Refactor plugin factory creation

6 files changed, 115 insertions, 91 deletions

diff --git a/buildstream/_elementfactory.py b/buildstream/_elementfactory.py
index 9d5b258cb..00847e66f 100644
--- a/buildstream/_elementfactory.py
+++ b/buildstream/_elementfactory.py
@@ -31,9 +31,13 @@ from .element import Element
 #
 class ElementFactory(PluginContext):
 
-    def __init__(self, plugin_base, plugin_origins=None):
+    def __init__(self, plugin_base, *,
+                 format_versions={},
+                 plugin_origins=None):
 
-        super().__init__(plugin_base, Element, [_site.element_plugins], plugin_origins)
+        super().__init__(plugin_base, Element, [_site.element_plugins],
+                         plugin_origins=plugin_origins,
+                         format_versions=format_versions)
 
     # create():
     #
@@ -54,4 +58,7 @@ class ElementFactory(PluginContext):
     #
     def create(self, context, project, artifacts, meta):
         element_type, default_config = self.lookup(meta.kind)
-        return element_type(context, project, artifacts, meta, default_config)
+        element = element_type(context, project, artifacts, meta, default_config)
+        version = self._format_versions.get(meta.kind, 0)
+        self._assert_plugin_format(element, version)
+        return element
diff --git a/buildstream/_plugincontext.py b/buildstream/_plugincontext.py
index 38d2231ba..5a7097485 100644
--- a/buildstream/_plugincontext.py
+++ b/buildstream/_plugincontext.py
@@ -20,7 +20,7 @@
 import os
 import inspect
 
-from ._exceptions import PluginError
+from ._exceptions import PluginError, LoadError, LoadErrorReason
 from . import utils
 
 
@@ -41,7 +41,9 @@ from . import utils
 #
 class PluginContext():
 
-    def __init__(self, plugin_base, base_type, site_plugin_path, plugin_origins=None, dependencies=None):
+    def __init__(self, plugin_base, base_type, site_plugin_path, *,
+                 plugin_origins=None, dependencies=None,
+                 format_versions={}):
 
         # The plugin kinds which were loaded
         self.loaded_dependencies = []
@@ -58,6 +60,7 @@ class PluginContext():
         self._plugin_base = plugin_base
         self._site_source = plugin_base.make_plugin_source(searchpath=site_plugin_path)
         self._alternate_sources = {}
+        self._format_versions = format_versions
 
     # lookup():
     #
@@ -219,3 +222,14 @@ class PluginContext():
                                   self._base_type.__name__, kind,
                                   plugin_type.BST_REQUIRED_VERSION_MAJOR,
                                   plugin_type.BST_REQUIRED_VERSION_MINOR))
+
+    # _assert_plugin_format()
+    #
+    # Helper to raise a PluginError if the loaded plugin is of a lesser version then
+    # the required version for this plugin
+    #
+    def _assert_plugin_format(self, plugin, version):
+        if plugin.BST_FORMAT_VERSION < version:
+            raise LoadError(LoadErrorReason.UNSUPPORTED_PLUGIN,
+                            "{}: Format version {} is too old for requested version {}"
+                            .format(plugin, plugin.BST_FORMAT_VERSION, version))
diff --git a/buildstream/_project.py b/buildstream/_project.py
index 12f23e2e4..faecec87f 100644
--- a/buildstream/_project.py
+++ b/buildstream/_project.py
@@ -104,18 +104,17 @@ class Project():
         #
         self._context = context  # The invocation Context
         self._aliases = {}       # Aliases dictionary
-        self._plugin_source_origins = []   # Origins of custom sources
-        self._plugin_element_origins = []  # Origins of custom elements
 
         self._cli_options = cli_options
         self._cache_key = None
-        self._source_format_versions = {}
-        self._element_format_versions = {}
 
         self._shell_command = []      # The default interactive shell command
         self._shell_environment = {}  # Statically set environment vars
         self._shell_host_files = []   # A list of HostMount objects
 
+        self._element_factory = None
+        self._source_factory = None
+
         profile_start(Topics.LOAD_PROJECT, self.directory.replace(os.sep, '-'))
         self._load()
         profile_end(Topics.LOAD_PROJECT, self.directory.replace(os.sep, '-'))
@@ -189,10 +188,7 @@ class Project():
     #    (Element): A newly created Element object of the appropriate kind
     #
     def create_element(self, artifacts, meta):
-        element = self._element_factory.create(self._context, self, artifacts, meta)
-        version = self._element_format_versions.get(meta.kind, 0)
-        self._assert_plugin_format(element, version)
-        return element
+        return self._element_factory.create(self._context, self, artifacts, meta)
 
     # create_source()
     #
@@ -205,10 +201,7 @@ class Project():
     #    (Source): A newly created Source object of the appropriate kind
     #
     def create_source(self, meta):
-        source = self._source_factory.create(self._context, self, meta)
-        version = self._source_format_versions.get(meta.kind, 0)
-        self._assert_plugin_format(source, version)
-        return source
+        return self._source_factory.create(self._context, self, meta)
 
     # get_alias_uri()
     #
@@ -394,49 +387,7 @@ class Project():
         # Load artifacts pull/push configuration for this project
         self.artifact_cache_specs = ArtifactCache.specs_from_config_node(config, self.directory)
 
-        # Plugin origins and versions
-        origins = _yaml.node_get(config, list, 'plugins', default_value=[])
-        for origin in origins:
-            allowed_origin_fields = [
-                'origin', 'sources', 'elements',
-                'package-name', 'path',
-            ]
-            allowed_origins = ['core', 'local', 'pip']
-            _yaml.node_validate(origin, allowed_origin_fields)
-
-            if origin['origin'] not in allowed_origins:
-                raise LoadError(
-                    LoadErrorReason.INVALID_YAML,
-                    "Origin '{}' is not one of the allowed types"
-                    .format(origin['origin']))
-
-            # Store source versions for checking later
-            source_versions = _yaml.node_get(origin, Mapping, 'sources', default_value={})
-            for key, _ in _yaml.node_items(source_versions):
-                if key in self._source_format_versions:
-                    raise LoadError(
-                        LoadErrorReason.INVALID_YAML,
-                        "Duplicate listing of source '{}'".format(key))
-                self._source_format_versions[key] = _yaml.node_get(source_versions, int, key)
-
-            # Store element versions for checking later
-            element_versions = _yaml.node_get(origin, Mapping, 'elements', default_value={})
-            for key, _ in _yaml.node_items(element_versions):
-                if key in self._element_format_versions:
-                    raise LoadError(
-                        LoadErrorReason.INVALID_YAML,
-                        "Duplicate listing of element '{}'".format(key))
-                self._element_format_versions[key] = _yaml.node_get(element_versions, int, key)
-
-            # Store the origins if they're not 'core'.
-            # core elements are loaded by default, so storing is unnecessary.
-            if _yaml.node_get(origin, str, 'origin') != 'core':
-                self._store_origin(origin, 'sources', self._plugin_source_origins)
-                self._store_origin(origin, 'elements', self._plugin_element_origins)
-
-        pluginbase = PluginBase(package='buildstream.plugins')
-        self._element_factory = ElementFactory(pluginbase, self._plugin_element_origins)
-        self._source_factory = SourceFactory(pluginbase, self._plugin_source_origins)
+        self._load_plugin_factories(config)
 
         # Source url aliases
         self._aliases = _yaml.node_get(config, Mapping, 'aliases', default_value={})
@@ -525,16 +476,59 @@ class Project():
             if not self.default_mirror:
                 self.default_mirror = mirror_name
 
-    # _assert_plugin_format()
-    #
-    # Helper to raise a PluginError if the loaded plugin is of a lesser version then
-    # the required version for this plugin
-    #
-    def _assert_plugin_format(self, plugin, version):
-        if plugin.BST_FORMAT_VERSION < version:
-            raise LoadError(LoadErrorReason.UNSUPPORTED_PLUGIN,
-                            "{}: Format version {} is too old for requested version {}"
-                            .format(plugin, plugin.BST_FORMAT_VERSION, version))
+    def _load_plugin_factories(self, config):
+        plugin_source_origins = []   # Origins of custom sources
+        plugin_element_origins = []  # Origins of custom elements
+
+        # Plugin origins and versions
+        origins = _yaml.node_get(config, list, 'plugins', default_value=[])
+        source_format_versions = {}
+        element_format_versions = {}
+        for origin in origins:
+            allowed_origin_fields = [
+                'origin', 'sources', 'elements',
+                'package-name', 'path',
+            ]
+            allowed_origins = ['core', 'local', 'pip']
+            _yaml.node_validate(origin, allowed_origin_fields)
+
+            if origin['origin'] not in allowed_origins:
+                raise LoadError(
+                    LoadErrorReason.INVALID_YAML,
+                    "Origin '{}' is not one of the allowed types"
+                    .format(origin['origin']))
+
+            # Store source versions for checking later
+            source_versions = _yaml.node_get(origin, Mapping, 'sources', default_value={})
+            for key, _ in _yaml.node_items(source_versions):
+                if key in source_format_versions:
+                    raise LoadError(
+                        LoadErrorReason.INVALID_YAML,
+                        "Duplicate listing of source '{}'".format(key))
+                source_format_versions[key] = _yaml.node_get(source_versions, int, key)
+
+            # Store element versions for checking later
+            element_versions = _yaml.node_get(origin, Mapping, 'elements', default_value={})
+            for key, _ in _yaml.node_items(element_versions):
+                if key in element_format_versions:
+                    raise LoadError(
+                        LoadErrorReason.INVALID_YAML,
+                        "Duplicate listing of element '{}'".format(key))
+                element_format_versions[key] = _yaml.node_get(element_versions, int, key)
+
+            # Store the origins if they're not 'core'.
+            # core elements are loaded by default, so storing is unnecessary.
+            if _yaml.node_get(origin, str, 'origin') != 'core':
+                self._store_origin(origin, 'sources', plugin_source_origins)
+                self._store_origin(origin, 'elements', plugin_element_origins)
+
+        pluginbase = PluginBase(package='buildstream.plugins')
+        self._element_factory = ElementFactory(pluginbase,
+                                               plugin_origins=plugin_element_origins,
+                                               format_versions=element_format_versions)
+        self._source_factory = SourceFactory(pluginbase,
+                                             plugin_origins=plugin_source_origins,
+                                             format_versions=source_format_versions)
 
     # _store_origin()
     #
diff --git a/buildstream/_sourcefactory.py b/buildstream/_sourcefactory.py
index 88a130e10..1d959a140 100644
--- a/buildstream/_sourcefactory.py
+++ b/buildstream/_sourcefactory.py
@@ -31,9 +31,13 @@ from .source import Source
 #
 class SourceFactory(PluginContext):
 
-    def __init__(self, plugin_base, plugin_origins=None):
+    def __init__(self, plugin_base, *,
+                 format_versions={},
+                 plugin_origins=None):
 
-        super().__init__(plugin_base, Source, [_site.source_plugins], plugin_origins)
+        super().__init__(plugin_base, Source, [_site.source_plugins],
+                         format_versions=format_versions,
+                         plugin_origins=plugin_origins)
 
     # create():
     #
@@ -54,4 +58,7 @@ class SourceFactory(PluginContext):
     #
     def create(self, context, project, meta):
         source_type, _ = self.lookup(meta.kind)
-        return source_type(context, project, meta)
+        source = source_type(context, project, meta)
+        version = self._format_versions.get(meta.kind, 0)
+        self._assert_plugin_format(source, version)
+        return source
diff --git a/tests/plugins/basics.py b/tests/plugins/basics.py
index 740a20c66..4c9cbafe4 100644
--- a/tests/plugins/basics.py
+++ b/tests/plugins/basics.py
@@ -49,7 +49,7 @@ def test_custom_source(plugin_fixture, datafiles):
                              datafiles.basename),
         'plugins': {'foo': 0}
     }]
-    factory = SourceFactory(plugin_fixture['base'], plugins)
+    factory = SourceFactory(plugin_fixture['base'], plugin_origins=plugins)
     assert(isinstance(factory, SourceFactory))
 
     foo_type, _ = factory.lookup('foo')
@@ -64,7 +64,7 @@ def test_custom_element(plugin_fixture, datafiles):
                              datafiles.basename),
         'plugins': {'foo': 0}
     }]
-    factory = ElementFactory(plugin_fixture['base'], plugins)
+    factory = ElementFactory(plugin_fixture['base'], plugin_origins=plugins)
     assert(isinstance(factory, ElementFactory))
 
     foo_type, _ = factory.lookup('foo')
@@ -101,7 +101,7 @@ def test_source_notatype(plugin_fixture, datafiles):
                              datafiles.basename),
         'plugins': {'foo': 0}
     }]
-    factory = SourceFactory(plugin_fixture['base'], plugins)
+    factory = SourceFactory(plugin_fixture['base'], plugin_origins=plugins)
     with pytest.raises(PluginError) as exc:
         foo_type = factory.lookup('foo')
 
@@ -115,7 +115,7 @@ def test_element_notatype(plugin_fixture, datafiles):
                              datafiles.basename),
         'plugins': {'foo': 0}
     }]
-    factory = ElementFactory(plugin_fixture['base'], plugins)
+    factory = ElementFactory(plugin_fixture['base'], plugin_origins=plugins)
     with pytest.raises(PluginError) as exc:
         foo_type = factory.lookup('foo')
 
@@ -130,7 +130,7 @@ def test_source_wrongtype(plugin_fixture, datafiles):
                              datafiles.basename),
         'plugins': {'foo': 0}
     }]
-    factory = SourceFactory(plugin_fixture['base'], plugins)
+    factory = SourceFactory(plugin_fixture['base'], plugin_origins=plugins)
     with pytest.raises(PluginError) as exc:
         foo_type = factory.lookup('foo')
 
@@ -145,7 +145,7 @@ def test_element_wrongtype(plugin_fixture, datafiles):
                              datafiles.basename),
         'plugins': {'foo': 0}
     }]
-    factory = ElementFactory(plugin_fixture['base'], plugins)
+    factory = ElementFactory(plugin_fixture['base'], plugin_origins=plugins)
     with pytest.raises(PluginError) as exc:
         foo_type = factory.lookup('foo')
 
@@ -159,7 +159,7 @@ def test_source_missing_setup(plugin_fixture, datafiles):
                              datafiles.basename),
         'plugins': {'foo': 0}
     }]
-    factory = SourceFactory(plugin_fixture['base'], plugins)
+    factory = SourceFactory(plugin_fixture['base'], plugin_origins=plugins)
     with pytest.raises(PluginError) as exc:
         foo_type = factory.lookup('foo')
 
@@ -173,7 +173,7 @@ def test_element_missing_setup(plugin_fixture, datafiles):
                              datafiles.basename),
         'plugins': {'foo': 0}
     }]
-    factory = ElementFactory(plugin_fixture['base'], plugins)
+    factory = ElementFactory(plugin_fixture['base'], plugin_origins=plugins)
     with pytest.raises(PluginError) as exc:
         foo_type = factory.lookup('foo')
 
@@ -188,7 +188,7 @@ def test_source_bad_setup(plugin_fixture, datafiles):
                              datafiles.basename),
         'plugins': {'foo': 0}
     }]
-    factory = SourceFactory(plugin_fixture['base'], plugins)
+    factory = SourceFactory(plugin_fixture['base'], plugin_origins=plugins)
     with pytest.raises(PluginError) as exc:
         foo_type = factory.lookup('foo')
 
@@ -203,7 +203,7 @@ def test_element_bad_setup(plugin_fixture, datafiles):
                              datafiles.basename),
         'plugins': {'foo': 0}
     }]
-    factory = ElementFactory(plugin_fixture['base'], plugins)
+    factory = ElementFactory(plugin_fixture['base'], plugin_origins=plugins)
     with pytest.raises(PluginError) as exc:
         foo_type = factory.lookup('foo')
 
@@ -218,7 +218,7 @@ def test_source_badversion(plugin_fixture, datafiles):
                              datafiles.basename),
         'plugins': {'foo': 0}
     }]
-    factory = SourceFactory(plugin_fixture['base'], plugins)
+    factory = SourceFactory(plugin_fixture['base'], plugin_origins=plugins)
     with pytest.raises(PluginError) as exc:
         foo_type = factory.lookup('foo')
 
@@ -233,7 +233,7 @@ def test_element_badversion(plugin_fixture, datafiles):
                              datafiles.basename),
         'plugins': {'foo': 0}
     }]
-    factory = ElementFactory(plugin_fixture['base'], plugins)
+    factory = ElementFactory(plugin_fixture['base'], plugin_origins=plugins)
     with pytest.raises(PluginError) as exc:
         foo_type = factory.lookup('foo')
 
@@ -260,8 +260,8 @@ def test_source_multicontext(plugin_fixture, datafiles):
         'plugins': {'foo': 0}
     }
 
-    factory1 = SourceFactory(plugin_fixture['base'], [plugins1])
-    factory2 = SourceFactory(plugin_fixture['base'], [plugins2])
+    factory1 = SourceFactory(plugin_fixture['base'], plugin_origins=[plugins1])
+    factory2 = SourceFactory(plugin_fixture['base'], plugin_origins=[plugins2])
     assert(isinstance(factory1, SourceFactory))
     assert(isinstance(factory2, SourceFactory))
 
@@ -289,8 +289,8 @@ def test_element_multicontext(plugin_fixture, datafiles):
         'plugins': {'foo': 0}
     }
 
-    factory1 = ElementFactory(plugin_fixture['base'], [plugins1])
-    factory2 = ElementFactory(plugin_fixture['base'], [plugins2])
+    factory1 = ElementFactory(plugin_fixture['base'], plugin_origins=[plugins1])
+    factory2 = ElementFactory(plugin_fixture['base'], plugin_origins=[plugins2])
     assert(isinstance(factory1, ElementFactory))
     assert(isinstance(factory2, ElementFactory))
 
diff --git a/tests/plugins/third_party.py b/tests/plugins/third_party.py
index a786fc7c2..b7791a2e4 100644
--- a/tests/plugins/third_party.py
+++ b/tests/plugins/third_party.py
@@ -33,7 +33,8 @@ def test_custom_pip_element(plugin_fixture, entry_fixture, datafiles):
         'path': str(datafiles),
         'plugins': {'foop': 0}
     }]
-    factory = ElementFactory(plugin_fixture['base'], origin_data)
+    factory = ElementFactory(plugin_fixture['base'],
+                             plugin_origins=origin_data)
     assert(isinstance(factory, ElementFactory))
 
     entry_fixture(datafiles, 'buildstream.plugins', 'third_party_element:foop')
@@ -50,7 +51,8 @@ def test_custom_pip_source(plugin_fixture, entry_fixture, datafiles):
         'path': str(datafiles),
         'plugins': {'foop': 0}
     }]
-    factory = SourceFactory(plugin_fixture['base'], origin_data)
+    factory = SourceFactory(plugin_fixture['base'],
+                            plugin_origins=origin_data)
     assert(isinstance(factory, SourceFactory))
 
     entry_fixture(datafiles, 'buildstream.plugins', 'third_party_source:foop')