Skip to content

Commit

Permalink
build: include built-in XBlock JS directly rather than copying it
Browse files Browse the repository at this point in the history
TODO: will copy in commit message from PR when squash merging

Part of: #32481
  • Loading branch information
kdmccormick committed Jul 7, 2023
1 parent 17faca5 commit 5391623
Show file tree
Hide file tree
Showing 3 changed files with 60 additions and 125 deletions.
4 changes: 1 addition & 3 deletions xmodule/assets/README.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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`_.

Expand All @@ -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.
Expand Down
168 changes: 59 additions & 109 deletions xmodule/static_content.py
Original file line number Diff line number Diff line change
@@ -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 <output_root>/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
Expand Down Expand Up @@ -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:
Expand All @@ -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("""\
Expand All @@ -217,7 +144,8 @@ def write_webpack(output_file, module_files, descriptor_files):

def main():
"""
Generate
Generate the weback config.
Usage: static_content.py <output_root>
"""
from django.conf import settings
Expand Down Expand Up @@ -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)


Expand Down
13 changes: 0 additions & 13 deletions xmodule/tests/test_content.py
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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()

0 comments on commit 5391623

Please sign in to comment.