diff --git a/conf/locale/config.yaml b/conf/locale/config.yaml index 23cb9e0d8537..4f4d32ed2fb5 100644 --- a/conf/locale/config.yaml +++ b/conf/locale/config.yaml @@ -98,6 +98,8 @@ dummy_locales: # Directories we don't search for strings. ignore_dirs: + - common/static/xmodule/modules + - common/static/xmodule/descriptors # Directories with no user-facing code. - '*/migrations' - '*/envs' diff --git a/webpack-config/file-lists.js b/webpack-config/file-lists.js index ddb9cf4f7806..0ea827a10633 100644 --- a/webpack-config/file-lists.js +++ b/webpack-config/file-lists.js @@ -10,7 +10,9 @@ module.exports = { path.resolve(__dirname, '../common/static/common/js/components/views/paging_footer.js'), path.resolve(__dirname, '../cms/static/js/views/paging.js'), path.resolve(__dirname, '../common/static/common/js/components/utils/view_utils.js'), - /xmodule\/js\/src/, + /descriptors\/js/, + /modules\/js/, + /xmodule\/js\/src\//, path.resolve(__dirname, '../openedx/features/course_bookmarks/static/course_bookmarks/js/views/bookmark_button.js') ], diff --git a/webpack.common.config.js b/webpack.common.config.js index 1f84d78cd8e9..9a65796184ee 100644 --- a/webpack.common.config.js +++ b/webpack.common.config.js @@ -13,7 +13,9 @@ var xmoduleJS = require('./common/static/xmodule/webpack.xmodule.config.js'); var filesWithRequireJSBlocks = [ path.resolve(__dirname, 'common/static/common/js/components/utils/view_utils.js'), - /xmodule\/js\/src/ + /descriptors\/js/, + /modules\/js/, + /xmodule\/js\/src\// ]; var defineHeader = /\(function ?\(((define|require|requirejs|\$)(, )?)+\) ?\{/; @@ -309,124 +311,14 @@ module.exports = Merge.smart({ test: /xblock\/runtime.v1/, loader: 'exports-loader?window.XBlock!imports-loader?XBlock=xblock/core,this=>window' }, - /******************************************************************************************************* - /* BUILT-IN XBLOCK ASSETS WITH GLOBAL DEFINITIONS: - * - * The monstrous list of globally-namespace modules below is the result of a JS build refactoring. - * Originally, all of these modules were copied to common/static/xmodule/js/[module|descriptors]/, and - * this file simply contained the lines: - * - * { - * test: /descriptors\/js/, - * loader: 'imports-loader?this=>window' - * }, - * { - * test: /modules\/js/, - * loader: 'imports-loader?this=>window' - * }, - * - * We removed that asset copying because it added complexity to the build, but as a result, in order to - * preserve exact parity with the preexisting global namespace, we had to enumerate all formely-copied - * modules here. It is very likely that many of these modules do not need to be in this list. Future - * refactorings are welcome to try to prune the list down to the minimal set of modules. As far as - * we know, the only modules that absolutely need to be added to the global namespace are those - * which define module types, for example "Problem" (in xmodule/js/src/capa/display.js). - */ { - test: /xmodule\/assets\/word_cloud\/src\/js\/word_cloud.js/, + test: /descriptors\/js/, loader: 'imports-loader?this=>window' }, { - test: /xmodule\/js\/common_static\/js\/vendor\/draggabilly.js/, + test: /modules\/js/, loader: 'imports-loader?this=>window' }, - { - test: /xmodule\/js\/src\/annotatable\/display.js/, - loader: 'imports-loader?this=>window' - }, - { - test: /xmodule\/js\/src\/capa\/display.js/, - loader: 'imports-loader?this=>window' - }, - { - test: /xmodule\/js\/src\/capa\/imageinput.js/, - loader: 'imports-loader?this=>window' - }, - { - test: /xmodule\/js\/src\/capa\/schematic.js/, - loader: 'imports-loader?this=>window' - }, - { - test: /xmodule\/js\/src\/collapsible.js/, - loader: 'imports-loader?this=>window' - }, - { - test: /xmodule\/js\/src\/conditional\/display.js/, - loader: 'imports-loader?this=>window' - }, - { - test: /xmodule\/js\/src\/html\/display.js/, - loader: 'imports-loader?this=>window' - }, - { - test: /xmodule\/js\/src\/html\/edit.js/, - loader: 'imports-loader?this=>window' - }, - { - test: /xmodule\/js\/src\/html\/imageModal.js/, - loader: 'imports-loader?this=>window' - }, - { - test: /xmodule\/js\/src\/javascript_loader.js/, - loader: 'imports-loader?this=>window' - }, - { - test: /xmodule\/js\/src\/lti\/lti.js/, - loader: 'imports-loader?this=>window' - }, - { - test: /xmodule\/js\/src\/poll\/poll.js/, - loader: 'imports-loader?this=>window' - }, - { - test: /xmodule\/js\/src\/poll\/poll_main.js/, - loader: 'imports-loader?this=>window' - }, - { - test: /xmodule\/js\/src\/problem\/edit.js/, - loader: 'imports-loader?this=>window' - }, - { - test: /xmodule\/js\/src\/raw\/edit\/metadata-only.js/, - loader: 'imports-loader?this=>window' - }, - { - test: /xmodule\/js\/src\/raw\/edit\/xml.js/, - loader: 'imports-loader?this=>window' - }, - { - test: /xmodule\/js\/src\/sequence\/display.js/, - loader: 'imports-loader?this=>window' - }, - { - test: /xmodule\/js\/src\/sequence\/edit.js/, - loader: 'imports-loader?this=>window' - }, - { - test: /xmodule\/js\/src\/tabs\/tabs-aggregator.js/, - loader: 'imports-loader?this=>window' - }, - { - test: /xmodule\/js\/src\/vertical\/edit.js/, - loader: 'imports-loader?this=>window' - }, - { - test: /xmodule\/js\/src\/video\/10_main.js/, - loader: 'imports-loader?this=>window' - }, - /* - * END BUILT-IN XBLOCK ASSETS WITH GLOBAL DEFINITIONS - ******************************************************************************************************/ { test: /codemirror/, loader: 'exports-loader?window.CodeMirror' diff --git a/xmodule/assets/README.rst b/xmodule/assets/README.rst index 54ed8a92a078..618ba029246f 100644 --- a/xmodule/assets/README.rst +++ b/xmodule/assets/README.rst @@ -59,7 +59,8 @@ Currently, edx-platform XBlock JS is defined both here in `xmodule/assets`_ and * For many older blocks, their JS is: - * bundled using a generated Webpack config at ``common/static/xmodule/webpack.xmodule.config.js``, + * copied to ``common/static/xmodule`` by `static_content.py`_ (aka ``xmodule_assets``), + * then bundled using a generated Webpack config at ``common/static/xmodule/webpack.xmodule.config.js``, * which is included into `webpack.common.config.js`_, * allowing it to be included into XBlock fragments using ``add_webpack_js_to_fragment`` from `builtin_assets.py`_. @@ -76,6 +77,7 @@ Currently, edx-platform XBlock JS is defined both here in `xmodule/assets`_ and As part of an `active build refactoring`_: +* We update the older builtin XBlocks to reference their JS directly rather than using copies of it. * We will move ``webpack.xmodule.config.js`` here instead of generating it. * We will consolidate all edx-platform XBlock JS here in `xmodule/assets`_. * We will delete the ``xmodule_assets`` script. diff --git a/xmodule/static_content.py b/xmodule/static_content.py index ec2a6b2524a6..03014476737a 100755 --- a/xmodule/static_content.py +++ b/xmodule/static_content.py @@ -1,48 +1,22 @@ # /usr/bin/env python """ -Generate /webpack.xmodule.config.js, with a display & editor Webpack bundle for each builtin block. - -It looks like this: - - module.exports = { - "entry": { - "AboutBlockDisplay": [ - "./xmodule/js/src/xmodule.js", - "./xmodule/js/src/html/display.js", - "./xmodule/js/src/javascript_loader.js", - "./xmodule/js/src/collapsible.js", - "./xmodule/js/src/html/imageModal.js", - "./xmodule/js/common_static/js/vendor/draggabilly.js" - ], - "AboutBlockEditor": [ - "./xmodule/js/src/xmodule.js", - "./xmodule/js/src/html/edit.js" - ], - "AnnotatableBlockDisplay": [ - "./xmodule/js/src/xmodule.js", - "./xmodule/js/src/html/display.js", - "./xmodule/js/src/annotatable/display.js", - "./xmodule/js/src/javascript_loader.js", - "./xmodule/js/src/collapsible.js" - ], - ... etc. - } - } - -Don't add to this! It will soon be removed as part of: https://github.com/openedx/edx-platform/issues/32481 +This module has utility functions for gathering up the javascript +that is defined by XModules and XModuleDescriptors """ import errno +import hashlib import json import logging import os import sys import textwrap +from collections import defaultdict from pkg_resources import resource_filename import django -from pathlib import Path as path +from path import Path as path from xmodule.annotatable_block import AnnotatableBlock from xmodule.capa_block import ProblemBlock @@ -102,6 +76,16 @@ class VideoBlock(HTMLSnippet): # lint-amnesty, pylint: disable=abstract-method ] +def write_module_js(output_root): + """Write all registered XModule js and coffee files to output root.""" + return _write_js(output_root, XBLOCK_CLASSES, 'get_preview_view_js') + + +def write_descriptor_js(output_root): + """Write all registered XModuleDescriptor js and coffee files to output root.""" + return _write_js(output_root, XBLOCK_CLASSES, 'get_studio_view_js') + + def _ensure_dir(directory): """Ensure that `directory` exists.""" try: @@ -113,21 +97,110 @@ def _ensure_dir(directory): raise +def _write_js(output_root, classes, js_attribute): + """ + Write the javascript fragments from all XModules in `classes` + into `output_root` as individual files, hashed by the contents to remove + duplicates + + Returns a dictionary mapping class names to the files that they depend on. + """ + file_contents = {} + file_owners = defaultdict(list) + + fragment_owners = defaultdict(list) + for class_ in classes: + module_js = getattr(class_, js_attribute)() + with open(module_js.get('xmodule_js'), 'rb') as xmodule_js_file: + xmodule_js_fragment = xmodule_js_file.read() + # It will enforce 000 prefix for xmodule.js. + fragment_owners[(0, 'js', xmodule_js_fragment)].append(getattr(class_, js_attribute + '_bundle_name')()) + for filetype in ('coffee', 'js'): + for idx, fragment_path in enumerate(module_js.get(filetype, [])): + with open(fragment_path, 'rb') as fragment_file: + fragment = fragment_file.read() + fragment_owners[(idx + 1, filetype, fragment)].append(getattr(class_, js_attribute + '_bundle_name')()) + + for (idx, filetype, fragment), owners in sorted(fragment_owners.items()): + filename = "{idx:0=3d}-{hash}.{type}".format( + idx=idx, + hash=hashlib.md5(fragment).hexdigest(), + type=filetype) + file_contents[filename] = fragment + for owner in owners: + file_owners[owner].append(output_root / filename) + + _write_files(output_root, file_contents, {'.coffee': '.js'}) + + return file_owners + + +def _write_files(output_root, contents, generated_suffix_map=None): + """ + Write file contents to output root. + + Any files not listed in contents that exists in output_root will be deleted, + unless it matches one of the patterns in `generated_suffix_map`. + + output_root (path): The root directory to write the file contents in + contents (dict): A map from filenames to file contents to be written to the output_root + generated_suffix_map (dict): Optional. Maps file suffix to generated file suffix. + For any file in contents, if the suffix matches a key in `generated_suffix_map`, + then the same filename with the suffix replaced by the value from `generated_suffix_map` + will be ignored + """ + _ensure_dir(output_root) + to_delete = {file.basename() for file in output_root.files()} - set(contents.keys()) + + if generated_suffix_map: + for output_file in contents.keys(): + for suffix, generated_suffix in generated_suffix_map.items(): + if output_file.endswith(suffix): + to_delete.discard(output_file.replace(suffix, generated_suffix)) + + for extra_file in to_delete: + (output_root / extra_file).remove_p() + + for filename, file_content in contents.items(): + output_file = output_root / filename + + not_file = not output_file.isfile() + + # Sometimes content is already unicode and sometimes it's not + # so we add this conditional here to make sure that below we're + # always working with streams of bytes. + if not isinstance(file_content, bytes): + file_content = file_content.encode('utf-8') + + # not_file is included to short-circuit this check, because + # read_md5 depends on the file already existing + write_file = not_file or output_file.read_md5() != hashlib.md5(file_content).digest() + if write_file: + LOG.debug("Writing %s", output_file) + output_file.write_bytes(file_content) + else: + LOG.debug("%s unchanged, skipping", output_file) + + def write_webpack(output_file, module_files, descriptor_files): """ Write all xmodule and xmodule descriptor javascript into module-specific bundles. The output format should be suitable for smart-merging into an existing webpack configuration. """ - _ensure_dir(output_file.parent) + _ensure_dir(output_file.dirname()) config = { 'entry': {} } - for (owner, unique_files) in list(module_files.items()) + list(descriptor_files.items()): + for (owner, files) in list(module_files.items()) + list(descriptor_files.items()): + unique_files = sorted({f'./{file}' for file in files}) if len(unique_files) == 1: unique_files = unique_files[0] config['entry'][owner] = unique_files + # config['entry']['modules/js/all'] = sorted(set('./{}'.format(file) for file in sum(module_files.values(), []))) + # config['entry']['descriptors/js/all'] = sorted(set('./{}'.format(file) for file in sum(descriptor_files.values(), []))) # lint-amnesty, pylint: disable=line-too-long + with output_file.open('w') as outfile: outfile.write( textwrap.dedent("""\ @@ -144,8 +217,7 @@ def write_webpack(output_file, module_files, descriptor_files): def main(): """ - Generate the weback config. - + Generate Usage: static_content.py """ from django.conf import settings @@ -173,30 +245,8 @@ def main(): except IndexError: sys.exit(main.__doc__) - # We assume this module is located at edx-platform/xmodule/static_content.py. - # Not the most robust assumption, but this script will be gone soon. - repo_root = path(__file__).parent.parent - - module_files = { - class_.get_preview_view_js_bundle_name(): [ - "./" + str(path(fragment_path).relative_to(repo_root)) - for fragment_path in [ - class_.get_preview_view_js()['xmodule_js'], - *class_.get_preview_view_js().get('js', []), - ] - ] - for class_ in XBLOCK_CLASSES - } - descriptor_files = { - class_.get_studio_view_js_bundle_name(): [ - "./" + str(path(fragment_path).relative_to(repo_root)) - for fragment_path in [ - class_.get_studio_view_js()['xmodule_js'], - *class_.get_studio_view_js().get('js', []), - ] - ] - for class_ in XBLOCK_CLASSES - } + descriptor_files = write_descriptor_js(root / 'descriptors/js') + module_files = write_module_js(root / 'modules/js') write_webpack(root / 'webpack.xmodule.config.js', module_files, descriptor_files) diff --git a/xmodule/tests/test_content.py b/xmodule/tests/test_content.py index 0b566153ad7d..dabd6d752739 100644 --- a/xmodule/tests/test_content.py +++ b/xmodule/tests/test_content.py @@ -1,14 +1,17 @@ """Tests for contents""" +import os import unittest from unittest.mock import Mock, patch import ddt from opaque_keys.edx.keys import CourseKey from opaque_keys.edx.locator import AssetLocator, CourseLocator +from path import Path as path from xmodule.contentstore.content import ContentStore, StaticContent, StaticContentStream +from xmodule.static_content import XBLOCK_CLASSES, _write_js SAMPLE_STRING = """ This is a sample string with more than 1024 bytes, the default STREAM_DATA_CHUNK_SIZE @@ -202,3 +205,13 @@ def test_static_content_stream_stream_data_in_range(self): total_length += len(chunck) assert total_length == ((last_byte - first_byte) + 1) + + def test_static_content_write_js(self): + """ + Test that only one filename starts with 000. + """ + output_root = path('common/static/xmodule/descriptors/js') + file_owners = _write_js(output_root, XBLOCK_CLASSES, 'get_studio_view_js') + js_file_paths = {file_path for file_path in sum(list(file_owners.values()), []) if os.path.basename(file_path).startswith('000-')} # lint-amnesty, pylint: disable=line-too-long + assert len(js_file_paths) == 1 + assert 'XModule.Descriptor = (function() {' in open(js_file_paths.pop()).read()