From 672f926eed0847dd040240fd270d10b3467f778c Mon Sep 17 00:00:00 2001
From: Stefano Borini <sborini@enthought.com>
Date: Mon, 31 Jul 2017 13:15:48 +0100
Subject: [PATCH] Changed ID strategy

---
 .../dummy/dummy_dakota/parameters.py          |  2 +-
 force_bdss/ids.py                             | 21 ++++++++++-------
 force_bdss/io/tests/test_workflow_writer.py   |  2 +-
 force_bdss/tests/fixtures/test_csv.json       | 10 ++++----
 .../tests/test_bundle_registry_plugin.py      | 23 +++++++++++++++----
 .../tests/test_core_evaluation_driver.py      |  2 +-
 force_bdss/tests/test_ids.py                  |  4 ++--
 7 files changed, 41 insertions(+), 23 deletions(-)

diff --git a/force_bdss/core_plugins/dummy/dummy_dakota/parameters.py b/force_bdss/core_plugins/dummy/dummy_dakota/parameters.py
index 6585312..d15c6c6 100644
--- a/force_bdss/core_plugins/dummy/dummy_dakota/parameters.py
+++ b/force_bdss/core_plugins/dummy/dummy_dakota/parameters.py
@@ -16,7 +16,7 @@ class RangedMCOParameter(BaseMCOParameter):
 
 class RangedMCOParameterFactory(BaseMCOParameterFactory):
     """The factory of the above model"""
-    id = mco_parameter_id("enthought", "ranged")
+    id = mco_parameter_id("enthought", "dummy_dakota", "ranged")
     model_class = RangedMCOParameter
     name = "Range"
     description = "A ranged parameter in floating point values."
diff --git a/force_bdss/ids.py b/force_bdss/ids.py
index 2af5c0b..4e5c414 100644
--- a/force_bdss/ids.py
+++ b/force_bdss/ids.py
@@ -29,23 +29,27 @@ def bundle_id(producer, identifier):
     -------
     str: an identifier to be used in the bundle.
     """
-    return _string_id("bundle", producer, identifier)
+    return _string_id(producer, "bundle", identifier)
 
 
-def mco_parameter_id(producer, identifier):
+def mco_parameter_id(producer, mco_identifier, parameter_identifier):
     """Creates an ID for an MCO parameter, so that it can be identified
     uniquely."""
-    return _string_id("mco_parameter", producer, identifier)
+    return _string_id(producer,
+                      "bundle",
+                      mco_identifier,
+                      "parameter",
+                      parameter_identifier)
 
 
 def plugin_id(producer, identifier):
     """Creates an ID for the plugins. These must be defined, otherwise
     the envisage system will complain (but not break)
     """
-    return _string_id("plugin", producer, identifier)
+    return _string_id(producer, "plugin", identifier)
 
 
-def _string_id(entity_namespace, producer, identifier):
+def _string_id(*args):
     """Creates an id for a generic entity.
 
     Parameters
@@ -68,7 +72,8 @@ def _string_id(entity_namespace, producer, identifier):
             " " not in entry and
             len(entry) != 0)
 
-    if not all(map(is_valid, [entity_namespace, producer, identifier])):
-        raise ValueError("Invalid parameters specified.")
+    if not all(map(is_valid, args)):
+        raise ValueError("One or more of the specified parameters was "
+                         "invalid: {}".format(str(args)))
 
-    return "force.bdss.{}.{}.{}".format(entity_namespace, producer, identifier)
+    return ".".join(["force", "bdss"]+list(args))
diff --git a/force_bdss/io/tests/test_workflow_writer.py b/force_bdss/io/tests/test_workflow_writer.py
index aaaa373..df70880 100644
--- a/force_bdss/io/tests/test_workflow_writer.py
+++ b/force_bdss/io/tests/test_workflow_writer.py
@@ -68,7 +68,7 @@ class TestWorkflowWriter(unittest.TestCase):
             BaseMCOParameter(
                 factory=mock.Mock(
                     spec=BaseMCOParameterFactory,
-                    id=mco_parameter_id("enthought", "mock")
+                    id=mco_parameter_id("enthought", "mock", "mock")
                 )
             )
         ]
diff --git a/force_bdss/tests/fixtures/test_csv.json b/force_bdss/tests/fixtures/test_csv.json
index 669ea67..726de30 100644
--- a/force_bdss/tests/fixtures/test_csv.json
+++ b/force_bdss/tests/fixtures/test_csv.json
@@ -2,11 +2,11 @@
   "version": "1",
   "workflow": {
     "mco": {
-      "id": "force.bdss.bundle.enthought.dummy_dakota",
+      "id": "force.bdss.enthought.bundle.dummy_dakota",
       "model_data": {
         "parameters" : [
           {
-            "id": "force.bdss.mco_parameter.enthought.ranged",
+            "id": "force.bdss.enthought.bundle.dummy_dakota.parameter.ranged",
             "model_data": {
                 "initial_value": 3,
                 "lower_bound": 0,
@@ -18,7 +18,7 @@
     },
     "data_sources": [
       {
-        "id": "force.bdss.bundle.enthought.csv_extractor",
+        "id": "force.bdss.enthought.bundle.csv_extractor",
         "model_data": {
           "filename": "foo.csv",
           "row": 3,
@@ -27,7 +27,7 @@
         }
       },
       {
-        "id": "force.bdss.bundle.enthought.csv_extractor",
+        "id": "force.bdss.enthought.bundle.csv_extractor",
         "model_data": {
           "filename": "foo.csv",
           "row": 3,
@@ -38,7 +38,7 @@
     ],
     "kpi_calculators": [
       {
-        "id": "force.bdss.bundle.enthought.kpi_adder",
+        "id": "force.bdss.enthought.bundle.kpi_adder",
         "model_data": {
           "cuba_type_in": "PRESSURE",
           "cuba_type_out": "TOTAL_PRESSURE"
diff --git a/force_bdss/tests/test_bundle_registry_plugin.py b/force_bdss/tests/test_bundle_registry_plugin.py
index b1ebd01..4bed99e 100644
--- a/force_bdss/tests/test_bundle_registry_plugin.py
+++ b/force_bdss/tests/test_bundle_registry_plugin.py
@@ -2,7 +2,9 @@ import unittest
 
 from force_bdss.base_extension_plugin import (
     BaseExtensionPlugin)
-from force_bdss.ids import bundle_id
+from force_bdss.ids import bundle_id, mco_parameter_id
+from force_bdss.mco.parameters.base_mco_parameter_factory import \
+    BaseMCOParameterFactory
 
 try:
     import mock
@@ -33,8 +35,17 @@ class TestBundleRegistry(unittest.TestCase):
 
 class MySuperPlugin(BaseExtensionPlugin):
     def _mco_bundles_default(self):
-        return [mock.Mock(spec=IMCOBundle,
-                          id=bundle_id("enthought", "mco1"))]
+        return [
+            mock.Mock(
+                spec=IMCOBundle,
+                id=bundle_id("enthought", "mco1"),
+                parameter_factories=mock.Mock(return_value=[
+                    mock.Mock(
+                        spec=BaseMCOParameterFactory,
+                        id=mco_parameter_id("enthought", "mco1", "ranged")
+                    )
+                ]),
+            )]
 
     def _data_source_bundles_default(self):
         return [mock.Mock(spec=IDataSourceBundle,
@@ -64,8 +75,10 @@ class TestBundleRegistryWithContent(unittest.TestCase):
         self.assertEqual(len(self.plugin.kpi_calculator_bundles), 3)
 
     def test_lookup(self):
-        id = bundle_id("enthought", "mco1")
-        self.assertEqual(self.plugin.mco_bundle_by_id(id).id, id)
+        mco_id = bundle_id("enthought", "mco1")
+        parameter_id = mco_parameter_id("enthought", "mco1", "ranged")
+        self.assertEqual(self.plugin.mco_bundle_by_id(mco_id).id, mco_id)
+        self.plugin.mco_parameter_factory_by_id(mco_id, parameter_id)
 
         for entry in ["ds1", "ds2"]:
             id = bundle_id("enthought", entry)
diff --git a/force_bdss/tests/test_core_evaluation_driver.py b/force_bdss/tests/test_core_evaluation_driver.py
index 504ba1c..5be1849 100644
--- a/force_bdss/tests/test_core_evaluation_driver.py
+++ b/force_bdss/tests/test_core_evaluation_driver.py
@@ -44,7 +44,7 @@ class NullParameter(BaseMCOParameter):
 
 
 class NullParameterFactory(BaseMCOParameterFactory):
-    id = mco_parameter_id("enthought", "ranged")
+    id = mco_parameter_id("enthought", "dummy_dakota", "ranged")
     model_class = NullParameter
 
 
diff --git a/force_bdss/tests/test_ids.py b/force_bdss/tests/test_ids.py
index 1ebff9a..e8dbbb7 100644
--- a/force_bdss/tests/test_ids.py
+++ b/force_bdss/tests/test_ids.py
@@ -6,7 +6,7 @@ from force_bdss.ids import bundle_id, plugin_id
 class TestIdGenerators(unittest.TestCase):
     def test_bundle_id(self):
         self.assertEqual(bundle_id("foo", "bar"),
-                         "force.bdss.bundle.foo.bar")
+                         "force.bdss.foo.bundle.bar")
 
         for bad_entry in ["", None, "   ", "foo bar"]:
             with self.assertRaises(ValueError):
@@ -15,7 +15,7 @@ class TestIdGenerators(unittest.TestCase):
                 bundle_id("foo", bad_entry)
 
     def test_plugin_id(self):
-        self.assertEqual(plugin_id("foo", "bar"), "force.bdss.plugin.foo.bar")
+        self.assertEqual(plugin_id("foo", "bar"), "force.bdss.foo.plugin.bar")
 
         for bad_entry in ["", None, "   ", "foo bar"]:
             with self.assertRaises(ValueError):
-- 
GitLab