From a6752b7c4d2d036f515c9da53609952b91f29818 Mon Sep 17 00:00:00 2001 From: David Ormsbee Date: Wed, 8 Nov 2017 17:37:00 -0500 Subject: [PATCH] Consolidate collected XBlock static assets. Before this commit, XBlock static assets were extracted during the collectstatic process by iterating through all installed XBlock classes and creating a separate directory for each. However, some packages install many XBlocks that actually share the same static assets. The most notable example is problem_builder, though we also see this with schoolyourself and google_drive. This commit uses the parent module name to do package asset lookup, collapsing those cases down and eliminating duplicates. For a default install of edx-platform XBlocks, this reduces assets generated from 31M to 14M. --- openedx/core/lib/xblock_pipeline/finder.py | 24 ++++++++++++++---- openedx/core/lib/xblock_utils/__init__.py | 29 +++++++++++++++++++++- 2 files changed, 47 insertions(+), 6 deletions(-) diff --git a/openedx/core/lib/xblock_pipeline/finder.py b/openedx/core/lib/xblock_pipeline/finder.py index 3dde96d757..aa1b5e8b34 100644 --- a/openedx/core/lib/xblock_pipeline/finder.py +++ b/openedx/core/lib/xblock_pipeline/finder.py @@ -11,6 +11,8 @@ from django.core.files.storage import Storage from pkg_resources import resource_exists, resource_filename, resource_isdir, resource_listdir from xblock.core import XBlock +from openedx.core.lib.xblock_utils import xblock_resource_pkg + class XBlockPackageStorage(Storage): """ @@ -109,13 +111,25 @@ class XBlockPipelineFinder(BaseFinder): A static files finder that gets static assets from xblocks. """ def __init__(self, *args, **kwargs): + """ + The XBlockPipelineFinder creates a separate XBlockPackageStorage for + every installed XBlock package when its initialized. After that + initialization happens, we just proxy all list()/find() requests by + iterating through the XBlockPackageStorage objects. + """ super(XBlockPipelineFinder, self).__init__(*args, **kwargs) - xblock_classes = set() - for __, xblock_class in XBlock.load_classes(): - xblock_classes.add(xblock_class) + + # xblock_resource_info holds (package_name, resources_dir) tuples. While + # it never happens in practice, the XBlock API does allow different + # XBlocks installed with the same setup.py to refer to their shared + # static assets using different prefixes. + xblock_resource_info = { + (xblock_resource_pkg(xblock_class), xblock_class.get_resources_dir()) + for __, xblock_class in XBlock.load_classes() + } self.package_storages = [ - XBlockPackageStorage(xblock_class.__module__, xblock_class.get_resources_dir()) - for xblock_class in xblock_classes + XBlockPackageStorage(pkg_name, resources_dir) + for pkg_name, resources_dir in xblock_resource_info ] def list(self, ignore_patterns): diff --git a/openedx/core/lib/xblock_utils/__init__.py b/openedx/core/lib/xblock_utils/__init__.py index a396fb4224..737201e5af 100644 --- a/openedx/core/lib/xblock_utils/__init__.py +++ b/openedx/core/lib/xblock_utils/__init__.py @@ -464,7 +464,7 @@ def xblock_local_resource_url(block, uri): xblock_class = getattr(block.__class__, 'unmixed_class', block.__class__) if settings.PIPELINE_ENABLED or not settings.REQUIRE_DEBUG: return staticfiles_storage.url('xblock/resources/{package_name}/{path}'.format( - package_name=xblock_class.__module__, + package_name=xblock_resource_pkg(xblock_class), path=uri )) else: @@ -472,3 +472,30 @@ def xblock_local_resource_url(block, uri): 'block_type': block.scope_ids.block_type, 'uri': uri, }) + + +def xblock_resource_pkg(block): + """ + Return the module name needed to find an XBlock's shared static assets. + + This method will return the full module name that is one level higher than + the one the block is in. For instance, problem_builder.answer.AnswerBlock + has a __module__ value of 'problem_builder.answer'. This method will return + 'problem_builder' instead. However, for edx-ora2's + openassessment.xblock.openassessmentblock.OpenAssessmentBlock, the value + returned is 'openassessment.xblock'. + + XModules are special cased because they're local to this repo and they + actually don't share their resource files when compiled out as part of the + XBlock asset pipeline. This only covers XBlocks and XModules using the + XBlock-style of asset specification. If they use the XModule bundling part + of the asset pipeline (xmodule_assets), their assets are compiled through an + entirely separate mechanism and put into lms-modules.js/css. + """ + # XModules are a special case because they map to different dirs for + # sub-modules. + module_name = block.__module__ + if module_name.startswith('xmodule.'): + return module_name + + return module_name.rsplit('.', 1)[0]