Merge lp:~niemeyer/pyjuju/simplify-iface-schema into lp:pyjuju

[Gustavo Niemeyer]

The interface schema handling is doing unnecessary magic.

[William Reade (fwereade)]

LGTM

review: Approve

[Kapil Thangavelu (hazmat)]

LGTM

review: Approve

=== modified file 'ensemble/formula/metadata.py'
--- ensemble/formula/metadata.py	2011-08-26 17:28:45 +0000
+++ ensemble/formula/metadata.py	2011-08-26 20:38:25 +0000
@@ -6,7 +6,7 @@
     EnsembleError, InvalidEnsembleHeaderValue, FileNotFound)
 from ensemble.formula.errors import FormulaIdError
 from ensemble.lib.schema import (
-    SchemaError, Bool, Constant, Dict, Int,
+    SchemaError, Bool, Constant, Dict, String, Int,
     KeyDict, OneOf, UnicodeOrString)
 
 
@@ -50,8 +50,10 @@
     return (namespace, name, int(revision))
 
 
+UTF8_SCHEMA = UnicodeOrString("utf-8")
+
 INTERFACE_SCHEMA = KeyDict({
-        "interface": UnicodeOrString("utf-8"),
+        "interface": UTF8_SCHEMA,
         "limit": OneOf(Constant(None), Int()),
         "optional": Bool()})
 
@@ -81,8 +83,6 @@
     representation as seen in the mysql interface description above.
     """
 
-    value_converter = UnicodeOrString("utf-8")
-
     def __init__(self, limit):
         """Create relation interface reshaper.
 
@@ -91,18 +91,17 @@
         """
         self.limit = limit
 
-    def coerce_interface(self, interface_spec, path):
-        """Coerce `interface_spec` from shorthand form.
+    def coerce(self, value, path):
+        """Coerce `value` into an expanded interface.
 
         Helper method to support each of the variants, either the
         formula does not specify limit and optional, such as foobar in
         the above example; or the interface spec is just a string,
         such as the ``server: riak`` example.
         """
-        if not isinstance(interface_spec, dict):
+        if not isinstance(value, dict):
             return {
-                "interface":
-                    self.value_converter.coerce(interface_spec, path),
+                "interface": UTF8_SCHEMA.coerce(value, path),
                 "limit": self.limit,
                 "optional": False}
         else:
@@ -110,35 +109,22 @@
             # defaults, which is different than what KeyDict can
             # readily support. So just do it here first, then
             # coerce.
-            if "limit" not in interface_spec:
-                interface_spec["limit"] = self.limit
-            if "optional" not in interface_spec:
-                interface_spec["optional"] = False
-            return INTERFACE_SCHEMA.coerce(interface_spec, path)
-
-    def coerce(self, value, path):
-        """Converts an interface specification in shorthand form.
-
-        @value: the value to be coerced
-        @path: used in exception reporting"""
-        if not isinstance(value, dict):
-            raise SchemaError(path, "Invalid relation specification")
-
-        return dict(
-            (self.value_converter.coerce(K, path),
-             self.coerce_interface(V, path))
-            for K, V in value.iteritems())
+            if "limit" not in value:
+                value["limit"] = self.limit
+            if "optional" not in value:
+                value["optional"] = False
+            return INTERFACE_SCHEMA.coerce(value, path)
 
 
 SCHEMA = KeyDict({
     "ensemble": Constant("formula"),
-    "name": UnicodeOrString("utf-8"),
+    "name": UTF8_SCHEMA,
     "revision": Int(),
-    "summary": UnicodeOrString("utf-8"),
-    "description": UnicodeOrString("utf-8"),
-    "peers": InterfaceExpander(limit=1),
-    "provides": InterfaceExpander(limit=None),
-    "requires": InterfaceExpander(limit=1),
+    "summary": UTF8_SCHEMA,
+    "description": UTF8_SCHEMA,
+    "peers": Dict(UTF8_SCHEMA, InterfaceExpander(limit=1)),
+    "provides": Dict(UTF8_SCHEMA, InterfaceExpander(limit=None)),
+    "requires": Dict(UTF8_SCHEMA, InterfaceExpander(limit=1)),
     }, optional=set(["provides", "requires", "peers"]))
 
 

=== modified file 'ensemble/formula/tests/test_metadata.py'
--- ensemble/formula/tests/test_metadata.py	2011-08-26 17:28:45 +0000
+++ ensemble/formula/tests/test_metadata.py	2011-08-26 20:38:25 +0000
@@ -286,41 +286,39 @@
 
         # shorthand is properly rewritten
         self.assertEqual(
-            expander.coerce({"admin": "http"}, ["provides"]),
-            {"admin": {"interface": "http", "limit": None, "optional": False}})
+            expander.coerce("http", ["provides"]),
+            {"interface": "http", "limit": None, "optional": False})
 
         # defaults are properly applied
         self.assertEqual(
             expander.coerce(
-                {"admin": {"interface": "http"}}, ["provides"]),
-            {"admin": {"interface": "http", "limit": None, "optional": False}})
-        self.assertEqual(
-            expander.coerce(
-                {"foobar": {"interface": "http", "limit": 2}}, ["provides"]),
-            {"foobar": {"interface": "http", "limit": 2, "optional": False}})
-        self.assertEqual(
-            expander.coerce(
-                {"foobar": {"interface": "http", "optional": True}},
+                {"interface": "http"}, ["provides"]),
+            {"interface": "http", "limit": None, "optional": False})
+        self.assertEqual(
+            expander.coerce(
+                {"interface": "http", "limit": 2}, ["provides"]),
+            {"interface": "http", "limit": 2, "optional": False})
+        self.assertEqual(
+            expander.coerce(
+                {"interface": "http", "optional": True},
                 ["provides"]),
-            {"foobar": {"interface": "http", "limit": None, "optional": True}})
+            {"interface": "http", "limit": None, "optional": True})
 
         # invalid data raises SchemaError
         self.assertRaises(
             SchemaError,
-            expander.coerce, {"admin": 42}, ["provides"])
-        self.assertRaises(
-            SchemaError,
-            expander.coerce,
-            {"admin": {"interface": "http", "optional": None}},
-            ["provides"])
-        self.assertRaises(
-            SchemaError,
-            expander.coerce,
-            {"admin": {"interface": "http", "limit": "none, really"}},
-            ["provides"])
+            expander.coerce, 42, ["provides"])
+        self.assertRaises(
+            SchemaError,
+            expander.coerce,
+            {"interface": "http", "optional": None}, ["provides"])
+        self.assertRaises(
+            SchemaError,
+            expander.coerce,
+            {"interface": "http", "limit": "none, really"}, ["provides"])
 
         # can change `limit` default
         expander = InterfaceExpander(limit=1)
         self.assertEqual(
-            expander.coerce({"admin": "http"}, ["consumes"]),
-            {"admin": {"interface": "http", "limit": 1, "optional": False}})
+            expander.coerce("http", ["consumes"]),
+            {"interface": "http", "limit": 1, "optional": False})