Typos, constant names, add context nesting.

docs/decisions/0003-plugin-contexts.rst

@@ -7,16 +7,24 @@ Draft
 
 Context
 =======
-edx-platform contains a plugin system which allows new Django apps to be installed inside the LMS and Studio without requiring the LMS/Studio to know about them. This is what enables us to move to a small and extensible core. While we had the ability to add settings, URLs, and signal handlers in our plugins, we wasn't a any way for a plugin to affect the commonly used pages that the core was delivering. Thus a plugin couldn't change any details on the dashboard, courseware, or any other rendered page that the platform delivered.
+edx-platform contains a plugin system which allows new Django apps to be installed inside the LMS and Studio without requiring the LMS/Studio to know about them. This is what enables us to move to a small and extensible core. While we had the ability to add settings, URLs, and signal handlers in our plugins, there wasn't any way for a plugin to affect the commonly used pages that the core was delivering. Thus a plugin couldn't change any details on the dashboard, courseware, or any other rendered page that the platform delivered.
 
 Decisions
 =========
-We have added the ability to add page context additions to the plugin system. This means that a plugin will be able to add context any view where it is enabled. To support this we have decided:
+We have added the ability to add page context additions to the plugin system. This means that a plugin will be able to add context to any view where it is enabled. To support this we have decided:
 # Plugins will define a callable function that the LMS and/or studio can import and call, which will return additional context to be added.
 # Every page that a plugin wants to add context to, must add a line to add the plugin contexts directly before the render.
-# All view + plugin data will exist in the same dictionary. To better protect against dictionary key collisions, it is suggested that you prefix your new context items with your app name (e.g. return {"myapp__some_variable": True} vs {"some_variable": True})
+# Plugin context will live in a dictionary called "plugins" that will be passed into the context the templates receive. The structure will look like:
+::
+  {
+    ..existing context values
+    "plugins": {
+      "my_new_plugin": {... my_new_plugins's values ...},
+      "my_other_plugin": {... my_other_plugin's values ...},
+    }
+  }
 # Each view will have a constant name that will be defined within it's app's API.py which will be used by plugins. These must be globally unique. These will also be recorded in the rendering app's README.rst file.
-# Plugin app's may import the view name from the rendering app's api.py or just use it's string.
+# Plugin apps have the option to either use the view name strings directly or import the constants from the rendering app's api.py if the plugin is part of the edx-platform repo.
 # For now, in order to use these new context data items, we must use theming alongside this to keep the new context out of the core. This may be iterated on in the future.
 
 Implementation
@@ -26,7 +34,7 @@ In the plugin app
 ~~~~~~~~~~~~~~~~~
 Config
 ++++++
-Inside of your AppConfig your new plugin app, add a "context_config" item like below.
+Inside of your AppConfig your new plugin app, add a "view_context_config" item like below.
 * The format will be {"globally_unique_view_name": "function_inside_plugin_app"}
 * The function name & path don't need to be named anything specific, so long as they work
 * These functions will be called on **every** render of that view, so keep them efficient or memoize them if they aren't user specific.
@@ -36,7 +44,7 @@ Inside of your AppConfig your new plugin app, add a "context_config" item like b
         name = "my_app"
 
         plugin_app = {
-            "context_config": {
+            "view_context_config": {
                 "lms.djangoapp":  {
                     "course_dashboard": "my_app.context_api.get_dashboard_context"
                 }
@@ -50,9 +58,9 @@ The function that will be called by the plugin system should accept a single par
 Example:
 ::
     def my_context_function(existing_context):
-        additional_context = {"myapp__some_variable": 10}
-        if existing_context.get("some_value"):
-            additional_context.append({"myapp__some_other_variable": True})
+        additional_context = {"some_plugin_value": 10}
+        if existing_context.get("some_core_value"):
+            additional_context.append({"some_other_plugin_value": True})
         return additional_context
 
 

openedx/core/djangoapps/plugins/README.rst

@@ -231,7 +231,7 @@ OR use string constants when they cannot import from djangoapps.plugins::
                     }],
                 }
             },
-            u'context_config': {
+            u'view_context_config': {
                 u'lms.djangoapp': {
                     'student_dashboard': u'my_app.context_api.get_dashboard_context'
                 }

openedx/core/djangoapps/plugins/constants.py

@@ -83,4 +83,4 @@ class PluginContexts(object):
     that can be specified by a Plugin App in order to configure the
     additional views it would like to add context into.
     """
-    CONFIG = u"context_config"
+    CONFIG = u"view_context_config"

openedx/core/djangoapps/plugins/plugin_contexts.py

@@ -2,6 +2,8 @@ from importlib import import_module
 
 from logging import getLogger
 
+from openedx.core.lib.cache_utils import process_cached
+
 from . import constants, registry
 
 
@@ -10,11 +12,48 @@ log = getLogger(__name__)
 def get_plugins_view_context(project_type, view_name, existing_context={}):
     """
     Returns a dict of additonal view context. Will check if any plugin apps
-    have that view in their context_config, and if so will call their
+    have that view in their view_context_config, and if so will call their
     selected function to get their context dicts.
     """
-    aggregate_context = {}
+    aggregate_context = {"plugins": {}}
+
+    # This functionality is cached
+    context_functions = _get_context_functions_for_view(project_type, view_name)
+
+    for (context_function, plugin_name) in context_functions:
+        plugin_context = context_function(existing_context)
+        try:
+            plugin_context = context_function(existing_context)
+        except Exception as exc:
+            # We're catching this because we don't want the core to blow up when a
+            # plugin is broken. This exception will probably need some sort of
+            # monitoring hooked up to it to make sure that these errors don't go
+            # unseen.
+            log.exception("Failed to call plugin context function. Error: %s", exc)
+            continue
+
+        aggregate_context["plugins"][plugin_name] = plugin_context
+
+    return aggregate_context
+
+
+def _get_context_function(app_config, project_type, view_name):
+    plugin_config = getattr(app_config, constants.PLUGIN_APP_CLASS_ATTRIBUTE_NAME, {})
+    context_config = plugin_config.get(constants.PluginContexts.CONFIG, {})
+    project_type_settings = context_config.get(project_type, {})
+    return project_type_settings.get(view_name)
+
+@process_cached
+def _get_context_functions_for_view(project_type, view_name):
+    """
+    Returns a list of tuples where the first item is the context function
+    and the second item is the name of the plugin it's being called from.
 
+    NOTE: These will be functions will be cached (in RAM not memcache) on this unique
+    combination. If we enable many new views to use this system, we may notice an
+    increase in memory usage as the entirety of these functions will be held in memory.
+    """
+    context_functions = []
     for app_config in registry.get_app_configs(project_type):
         context_function_path = _get_context_function(app_config, project_type, view_name)
         if context_function_path:
@@ -22,37 +61,21 @@ def get_plugins_view_context(project_type, view_name, existing_context={}):
             try:
                 module = import_module(module_path)
             except (ImportError, ModuleNotFoundError):
-                log.exception("Failed to import %s plugin when creating %s context", module_path, view_name)
+                log.exception(
+                    "Failed to import %s plugin when creating %s context",
+                    module_path,
+                    view_name
+                )
                 continue
             context_function = getattr(module, name, None)
             if context_function:
-                plugin_context = context_function(existing_context)
+                plugin_name, _, _ = module_path.partition('.')
+                context_functions.append((context_function, plugin_name))
             else:
-                log.exception(
-                    "Failed to call %s function from %s plugin when creating %s context",
+                log.warning(
+                    "Failed to retrieve %s function from %s plugin when creating %s context",
                     name,
                     module_path,
                     view_name
                 )
-                continue
-
-            # NOTE: If two plugins have try to set the same context keys, the last one
-            # called will overwrite the others.
-            for key in plugin_context:
-                if key in aggregate_context:
-                    log.warning(
-                        "Plugin %s is overwriting the value of %s for view %s",
-                        app_config.__module__,
-                        key,
-                        view_name
-                    )
-            aggregate_context.update(plugin_context)
-
-    return aggregate_context
-
-
-def _get_context_function(app_config, project_type, view_name):
-    plugin_config = getattr(app_config, constants.PLUGIN_APP_CLASS_ATTRIBUTE_NAME, {})
-    context_config = plugin_config.get(constants.PluginContexts.CONFIG, {})
-    project_type_settings = context_config.get(project_type, {})
-    return project_type_settings.get(view_name)
+    return context_functions