From 5391623360dbc426776155c65b422b1643a7528c Mon Sep 17 00:00:00 2001 From: Kyle McCormick Date: Thu, 15 Jun 2023 10:17:47 -0400 Subject: [PATCH] build: include built-in XBlock JS directly rather than copying it TODO: will copy in commit message from PR when squash merging Part of: https://github.com/openedx/edx-platform/issues/32481 --- xmodule/assets/README.rst | 4 +- xmodule/static_content.py | 168 ++++++++++++---------------------- xmodule/tests/test_content.py | 13 --- 3 files changed, 60 insertions(+), 125 deletions(-) diff --git a/xmodule/assets/README.rst b/xmodule/assets/README.rst index 618ba029246f..54ed8a92a078 100644 --- a/xmodule/assets/README.rst +++ b/xmodule/assets/README.rst @@ -59,8 +59,7 @@ Currently, edx-platform XBlock JS is defined both here in `xmodule/assets`_ and * For many older blocks, their JS is: - * 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``, + * 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`_. @@ -77,7 +76,6 @@ 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 03014476737a..ec2a6b2524a6 100755 --- a/xmodule/static_content.py +++ b/xmodule/static_content.py @@ -1,22 +1,48 @@ # /usr/bin/env python """ -This module has utility functions for gathering up the javascript -that is defined by XModules and XModuleDescriptors +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 """ 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 path import Path as path +from pathlib import Path as path from xmodule.annotatable_block import AnnotatableBlock from xmodule.capa_block import ProblemBlock @@ -76,16 +102,6 @@ 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: @@ -97,110 +113,21 @@ 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.dirname()) + _ensure_dir(output_file.parent) config = { 'entry': {} } - for (owner, files) in list(module_files.items()) + list(descriptor_files.items()): - unique_files = sorted({f'./{file}' for file in files}) + for (owner, unique_files) in list(module_files.items()) + list(descriptor_files.items()): 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("""\ @@ -217,7 +144,8 @@ def write_webpack(output_file, module_files, descriptor_files): def main(): """ - Generate + Generate the weback config. + Usage: static_content.py """ from django.conf import settings @@ -245,8 +173,30 @@ def main(): except IndexError: sys.exit(main.__doc__) - descriptor_files = write_descriptor_js(root / 'descriptors/js') - module_files = write_module_js(root / 'modules/js') + # 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 + } 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 dabd6d752739..0b566153ad7d 100644 --- a/xmodule/tests/test_content.py +++ b/xmodule/tests/test_content.py @@ -1,17 +1,14 @@ """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 @@ -205,13 +202,3 @@ 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()