Merge lp:~radix/txaws/parameter-enrichment into lp:txaws

[Christopher Armstrong]

This is one part of schema enrichment. It adds List and Structure Parameter types, and significantly refactors the way that paramater parsing and formatting is done.

[Thomas Herve (therve)]

[1] In _secret_convert_old_schema, you should pass item.name as name parameter to List, to keep compatibility.

[2] You also need to forward the optional parameter.

[3] Still in the sample, it seems you should forward the default value, and use [] otherwise as default.

[4] In _convert_nest_to_flat, you return as soon as you get a dict: if you have more params, they get ignore. You should probably call _result.update instead.

[5]
+ {'foo.1.bar': 'value',
+ 'foo.2.baz': 'value'}
+
+ to::
+
+ {'foo': {'1': {'bar': 'value'},
+ '2': {'baz': 'value'}}}

I think we talked about that, but what's the reason we don't convert this to:

{'foo': [{'bar': 'value'}, {'baz': 'value'}]} ?

FWIW, I discovered most of this running Landscape/Clouddeck. We can deal with some small incompatibilities, but it'd be nice to narrow them down. Thanks!

review: Approve

[Thomas Herve (therve)]

review: Needs Fixing

[lp:~radix/txaws/parameter-enrichment]

150. By Christopher Armstrong on 2012-04-26

- fix bundling multiple list / structure arguments
- make default list value []
- give a name to Lists and structures that are automatically converted from the old schema system
- better test coverage for these things

[Christopher Armstrong (radix)]

Looks like I broke the second return value from 'extract' -- It's returning a nested structure when it should still be returning the flat structure. I'll fix that.

[lp:~radix/txaws/parameter-enrichment]

151. By Christopher Armstrong on 2012-04-26

- fix the 'rest' return value from extract so it returns the original format of arguments and not a nested structure.

[Christopher Armstrong (radix)]

I've fixed the 'rest' return format.

The only tests in landscape and clouddeck that are still failing are because of names in error messages.

I think this is a pretty tolerable incompatibility, but I'm going to see what I can do to do maintain compatibility here. Please take another look in the meantime (and if another reviewer wants to take a look, that'd be appreciated).

[Christopher Armstrong (radix)]

Oh, and to address your comments explicitly:

[1] fixed (name.split('.',1)[0], actually)
[2] fixed

[3] I've made [] the default default for List, but I'm not sure if 'default' should be forwarded from the item to the list. After investigating it for a while, the original behavior of default= on a list item (i.e. numbered parameter) seems broken... Given a schema of Unicode("foo.n", optional=True, default=u"foo"), after extracting an empty dict the value of arguments.foo == Arguments({}).

This is a little bit concerning given that we have code that assumes it works (or at least, specifies defaults). Given that the existing behavior is weird, I don't think it's worth putting any effort into it; everyone should specify lists as List(..., optional=True, default=[...]) from now on. Does this seem reasonable?

[4] Wow, that's a seriously dumb one. Fixed.
[5] The reason we don't convert it to a list is in this part of the code we don't know that it's meant to be interpreted as a list. Plus, it's just meant to be an internal data structure used during parsing and formatting, and never exposed to users. (however, it was accidentally being exposed to users as the 'rest' return value from extract(), which I've fixed).

Thanks!

[lp:~radix/txaws/parameter-enrichment]

152. By Christopher Armstrong on 2012-04-26

add an explicit test for specified defaults of Lists - already works, just
double-checking

[Thomas Herve (therve)]

[6]
+ _result.update(self._convert_nest_to_flat(v, _result=_result,
+ _prefix=path))

You can remove the update call, or remove passing the result value, but I don't think you need to do both.

Looks good to me, +1!

review: Approve

[lp:~radix/txaws/parameter-enrichment]

153. By Christopher Armstrong on 2012-04-26

we don't need to update the result dict, the function is doing it for us.

[Chad Smith (chad.smith)]

+1. It's nice to have the additional Schema classes defined.

[1] Do we want to alert the user as to the List argument format options for list parameters in our error messages? Or, would the following be too clunky?

Instead of the message:

The request must contain the parameter names

The error message could be:

The request must contain the parameter names or names.#.
OR
The request must contain the parameter names or names.N.

Maybe with something like this in coerce() with class attribute kind ="parameter" on Parameter:
- raise MissingParameterError(self.name)
+ if self.kind == "list":
+ raise MissingParameterError("%s or %s.#" %
+ (self.name, self.name))
+ else:
+ raise MissingParameterError(self.name)

[2] Should probably add a more explicit error message test in test_non_list:

        if not isinstance(value, dict):
            raise InvalidParameterValueError("%r should be a dict." % (value,))
+ self.assertEqual(400, error.status)
+ self.assertEqual("InvalidParameterValue", error.code)
+ self.assertEqual("'foo' should be a dict.",
+ error.message)

[3] Do we care about schema.extract error conditions if Schema is defined as Unicode("names.blah") and "names.10" and "names.20" are specified? Right now, this doesn't raise errors and schema extract gives us arguments.name[0] and arguments.name[1]. Should Unicode schema definition/declaration be constrained a bit more to prevent misuse in initial Schema definition? It seems we'll accept anything delimited by a single "." which we then interpret as a list.

[4] This branch does cause a couple of incompatibilities with error message tests for Unicode("blah.#") types in Landscape unit tests. You are already working it, but we should make sure Clouddeck & Landscape fix branches are committed at the same time.

review: Approve

[lp:~radix/txaws/parameter-enrichment]

154. By Christopher Armstrong on 2012-05-01

handle non-lists as list input.

155. By Christopher Armstrong on 2012-05-02

include the kind of the parameter in the MissingParameter error

[Christopher Armstrong (radix)]

[1] I've changed the error message to specify the kind of the parameter. That way users will know what they're expected to pass. Unfortunately this means a whole lot more unit tests will need to be updated :)

[2] I actually removed that exception, because it was what was preventing the "foo" -> "foo.1" from working. test_non_list is now gone.

[3] This was already the behavior of the schema system, so changing this would result in more incompatibilities. We should basically consider naming fields with dots in them to be deprecated; new schemas should all use List and Structure to be explicit about what child fields mean.

[4] Yep, I'll work on the incompatibility branch for landscape after your naming branch lands, so it minimizes conflicts.

=== modified file 'txaws/server/schema.py'
--- txaws/server/schema.py	2012-03-27 11:51:09 +0000
+++ txaws/server/schema.py	2012-05-02 15:45:23 +0000
@@ -21,11 +21,19 @@
     @param name: The name of the missing parameter.
     """
 
-    def __init__(self, name):
+    def __init__(self, name, kind=None):
         message = "The request must contain the parameter %s" % name
+        if kind is not None:
+            message += " (%s)" % (kind,)
         super(MissingParameterError, self).__init__(message)
 
 
+class InconsistentParameterError(SchemaError):
+    def __init__(self, name):
+        message = "Parameter %s is used inconsistently" % (name,)
+        super(InconsistentParameterError, self).__init__(message)
+
+
 class InvalidParameterValueError(SchemaError):
     """Raised when the value of a parameter is invalid."""
 
@@ -51,6 +59,20 @@
         super(UnknownParameterError, self).__init__(message)
 
 
+class UnknownParametersError(Exception):
+    """
+    Raised when extra unknown fields are passed to L{Structure.parse}.
+
+    @ivar result: The already coerced result representing the known parameters.
+    @ivar unknown: The unknown parameters.
+    """
+    def __init__(self, result, unknown):
+        self.result = result
+        self.unknown = unknown
+        message = "The parameters %s are not recognized" % (unknown,)
+        super(UnknownParametersError, self).__init__(message)
+
+
 class Parameter(object):
     """A single parameter in an HTTP request.
 
@@ -67,7 +89,10 @@
     @param validator: A callable to validate the parameter, returning a bool.
     """
 
-    def __init__(self, name, optional=False, default=None,
+    supports_multiple = False
+    kind = None
+
+    def __init__(self, name=None, optional=False, default=None,
                  min=None, max=None, allow_none=False, validator=None):
         self.name = name
         self.optional = optional
@@ -91,7 +116,7 @@
                 value = ""
         if value == "":
             if not self.allow_none:
-                raise MissingParameterError(self.name)
+                raise MissingParameterError(self.name, kind=self.kind)
             return self.default
         try:
             self._check_range(value)
@@ -182,7 +207,7 @@
     lower_than_min_template = "Value must be at least %s."
     greater_than_max_template = "Value exceeds maximum of %s."
 
-    def __init__(self, name, optional=False, default=None,
+    def __init__(self, name=None, optional=False, default=None,
                  min=0, max=None, allow_none=False, validator=None):
         super(Integer, self).__init__(name, optional, default, min, max,
                                       allow_none, validator)
@@ -228,8 +253,10 @@
 
     kind = "enum"
 
-    def __init__(self, name, mapping, optional=False, default=None):
+    def __init__(self, name=None, mapping=None, optional=False, default=None):
         super(Enum, self).__init__(name, optional=optional, default=default)
+        if mapping is None:
+            raise TypeError("Must provide mapping")
         self.mapping = mapping
         self.reverse = dict((value, key) for key, value in mapping.iteritems())
 
@@ -260,18 +287,149 @@
         return datetime.strftime(utc_value, "%Y-%m-%dT%H:%M:%SZ")
 
 
+class List(Parameter):
+    """
+    A homogenous list of instances of a parameterized type.
+
+    There is a strange behavior that lists can have any starting index and any
+    gaps are ignored.  Conventionally they are 1-based, and so indexes proceed
+    like 1, 2, 3...  However, any non-negative index can be used and the
+    ordering will be used to determine the true index. So::
+
+        {5: 'a', 7: 'b', 9: 'c'}
+
+    becomes::
+
+        ['a', 'b', 'c']
+    """
+
+    kind = "list"
+    supports_multiple = True
+
+    def __init__(self, name=None, item=None, optional=False, default=None):
+        """
+        @param item: A L{Parameter} instance which will be used to parse and
+            format the values in the list.
+        """
+        if item is None:
+            raise TypeError("Must provide item")
+        super(List, self).__init__(name, optional=optional, default=default)
+        self.item = item
+        if default is None:
+            self.default = []
+
+    def parse(self, value):
+        """
+        Convert a dictionary of {relative index: value} to a list of parsed
+        C{value}s.
+        """
+        indices = []
+        if not isinstance(value, dict):
+            # We interpret non-list inputs as a list of one element, for
+            # compatibility with certain EC2 APIs.
+            return [self.item.coerce(value)]
+        for index in value.keys():
+            try:
+                indices.append(int(index))
+            except ValueError:
+                raise UnknownParameterError(index)
+        result = [None] * len(value)
+        for index_index, index in enumerate(sorted(indices)):
+            v = value[str(index)]
+            if index < 0:
+                raise UnknownParameterError(index)
+            result[index_index] = self.item.coerce(v)
+        return result
+
+    def format(self, value):
+        """
+        Convert a list like::
+
+            ["a", "b", "c"]
+
+        to:
+
+            {"1": "a", "2": "b", "3": "c"}
+
+        C{value} may also be an L{Arguments} instance, mapping indices to
+        values. Who knows why.
+        """
+        if isinstance(value, Arguments):
+            return dict((str(i), self.item.format(v)) for i, v in value)
+        return dict((str(i + 1), self.item.format(v))
+                        for i, v in enumerate(value))
+
+
+class Structure(Parameter):
+    """
+    A structure with named fields of parameterized types.
+    """
+
+    kind = "structure"
+    supports_multiple = True
+
+    def __init__(self, name=None, fields=None, optional=False, default=None):
+        """
+        @param fields: A mapping of field name to field L{Parameter} instance.
+        """
+        if fields is None:
+            raise TypeError("Must provide fields")
+        super(Structure, self).__init__(name, optional=optional,
+                                        default=default)
+        self.fields = fields
+
+    def parse(self, value):
+        """
+        Convert a dictionary of raw values to a dictionary of processed values.
+        """
+        result = {}
+        rest = {}
+        for k, v in value.iteritems():
+            if k in self.fields:
+                if (isinstance(v, dict)
+                    and not self.fields[k].supports_multiple):
+                    if len(v) == 1:
+                        # We support "foo.1" as "foo" as long as there is only
+                        # one "foo.#" parameter provided.... -_-
+                        v = v.values()[0]
+                    else:
+                        raise InvalidParameterCombinationError(k)
+                result[k] = self.fields[k].coerce(v)
+            else:
+                rest[k] = v
+        for k, v in self.fields.iteritems():
+            if k not in result:
+                result[k] = v.coerce(None)
+        if rest:
+            raise UnknownParametersError(result, rest)
+        return result
+
+    def format(self, value):
+        """
+        Convert a dictionary of processed values to a dictionary of raw values.
+        """
+        if not isinstance(value, Arguments):
+            value = value.iteritems()
+        return dict((k, self.fields[k].format(v)) for k, v in value)
+
+
 class Arguments(object):
     """Arguments parsed from a request."""
 
     def __init__(self, tree):
         """Initialize a new L{Arguments} instance.
 
-        @param tree: The C{dict}-based structure of the L{Argument}instance
+        @param tree: The C{dict}-based structure of the L{Argument} instance
             to create.
         """
         for key, value in tree.iteritems():
             self.__dict__[key] = self._wrap(value)
 
+    def __str__(self):
+        return "Arguments(%s)" % (self.__dict__,)
+
+    __repr__ = __str__
+
     def __iter__(self):
         """Returns an iterator yielding C{(name, value)} tuples."""
         return self.__dict__.iteritems()
@@ -288,7 +446,7 @@
         """Wrap the given L{tree} with L{Arguments} as necessary.
 
         @param tree: A {dict}, containing L{dict}s and/or leaf values, nested
-        arbitrarily deep.
+            arbitrarily deep.
         """
         if isinstance(value, dict):
             if any(isinstance(name, int) for name in value.keys()):
@@ -299,6 +457,8 @@
                 return [self._wrap(value) for (name, value) in items]
             else:
                 return Arguments(value)
+        elif isinstance(value, list):
+            return [self._wrap(x) for x in value]
         else:
             return value
 
@@ -308,7 +468,7 @@
     The schema that the arguments of an HTTP request must be compliant with.
     """
 
-    def __init__(self, *parameters):
+    def __init__(self, *_parameters, **kwargs):
         """Initialize a new L{Schema} instance.
 
         Any number of L{Parameter} instances can be passed. The parameter path
@@ -323,60 +483,34 @@
 
         A more complex example::
 
-          schema = Schema(Unicode('Name.#'))
+          schema = Schema(List('Names', item=Unicode()))
 
-        means that the result of L{Schema.extract} would have a C{Name}
+        means that the result of L{Schema.extract} would have a C{Names}
         attribute, which would itself contain a list of names. Similarly,
-        L{Schema.bundle} would look for a C{Name} attribute.
+        L{Schema.bundle} would look for a C{Names} attribute.
         """
-        self._parameters = dict(
-            (self._get_template(parameter.name), parameter)
-            for parameter in parameters)
+        if 'parameters' in kwargs:
+            if len(_parameters) > 0:
+                raise TypeError("parameters= must only be passed "
+                                "without positional arguments")
+            self._parameters = kwargs['parameters']
+        else:
+            self._parameters = self._convert_old_schema(_parameters)
 
     def extract(self, params):
         """Extract parameters from a raw C{dict} according to this schema.
 
         @param params: The raw parameters to parse.
-        @return: An L{Arguments} object holding the extracted arguments.
-
-        @raises UnknownParameterError: If C{params} contains keys that this
-            schema doesn't know about.
+        @return: A tuple of an L{Arguments} object holding the extracted
+            arguments and any unparsed arguments.
         """
-        tree = {}
-        rest = {}
-
-        # Extract from the given arguments and parse according to the
-        # corresponding parameters.
-        for name, value in params.iteritems():
-            template = self._get_template(name)
-            parameter = self._parameters.get(template)
-
-            if template.endswith(".#") and parameter is None:
-                # If we were unable to find a direct match for a template that
-                # allows multiple values. Let's attempt to find it without the
-                # multiple value marker which Amazon allows. For example if the
-                # template is 'PublicIp', then a single key 'PublicIp.1' is
-                # allowed.
-                parameter = self._parameters.get(template[:-2])
-                if parameter is not None:
-                    name = name[:-2]
-
-                # At this point, we have a template that doesn't have the .#
-                # marker to indicate multiple values. We don't allow multiple
-                # "single" values for the same element.
-                if name in tree.keys():
-                    raise InvalidParameterCombinationError(name)
-
-            if parameter is None:
-                rest[name] = value
-            else:
-                self._set_value(tree, name, parameter.coerce(value))
-
-        # Ensure that the tree arguments are consistent with constraints
-        # defined in the schema.
-        for template, parameter in self._parameters.iteritems():
-            self._ensure_tree(tree, parameter, *template.split("."))
-
+        structure = Structure(fields=self._parameters)
+        try:
+            tree = structure.coerce(self._convert_flat_to_nest(params))
+            rest = {}
+        except UnknownParametersError, error:
+            tree = error.result
+            rest = self._convert_nest_to_flat(error.unknown)
         return Arguments(tree), rest
 
     def bundle(self, *arguments, **extra):
@@ -390,117 +524,85 @@
         params = {}
 
         for argument in arguments:
-            self._flatten(params, argument)
-        self._flatten(params, extra)
+            params.update(argument)
 
+        params.update(extra)
+        result = {}
         for name, value in params.iteritems():
-            parameter = self._parameters.get(self._get_template(name))
+            if value is None:
+                continue
+            segments = name.split('.')
+            first = segments[0]
+            parameter = self._parameters.get(first)
             if parameter is None:
                 raise RuntimeError("Parameter '%s' not in schema" % name)
             else:
                 if value is None:
-                    params[name] = ""
-                else:
-                    params[name] = parameter.format(value)
-
-        return params
-
-    def _get_template(self, key):
-        """Return the canonical template for a given parameter key.
-
-        For example::
-
-          'Child.1.Name.2'
-
-        becomes::
-
-          'Child.#.Name.#'
-
-        """
-        parts = key.split(".")
-        for index, part in enumerate(parts[1::2]):
-            parts[index * 2 + 1] = "#"
-        return ".".join(parts)
-
-    def _set_value(self, tree, path, value):
-        """Set C{value} at C{path} in the given C{tree}.
-
-        For example::
-
-          tree = {}
-          _set_value(tree, 'foo.1.bar.2', True)
-
-        results in C{tree} becoming::
-
-          {'foo': {1: {'bar': {2: True}}}}
-
-        @param tree: A L{dict}.
-        @param path: A L{str}.
-        @param value: The value to set. Can be anything.
-        """
-        nodes = []
-        for index, node in enumerate(path.split(".")):
-            if index % 2:
-                # Nodes with odd indexes must be non-negative integers
-                try:
-                    node = int(node)
-                except ValueError:
-                    raise UnknownParameterError(path)
-                if node < 0:
-                    raise UnknownParameterError(path)
-            nodes.append(node)
-        for node in nodes[:-1]:
-            tree = tree.setdefault(node, {})
-        tree[nodes[-1]] = value
-
-    def _ensure_tree(self, tree, parameter, node, *nodes):
-        """Check that C{node} exists in C{tree} and is followed by C{nodes}.
-
-        C{node} and C{nodes} should correspond to a template path (i.e. where
-        there are no absolute indexes, but C{#} instead).
-        """
-        if node == "#":
-            if len(nodes) == 0:
-                if len(tree.keys()) == 0 and not parameter.optional:
-                    raise MissingParameterError(parameter.name)
-            else:
-                for subtree in tree.itervalues():
-                    self._ensure_tree(subtree, parameter, *nodes)
-        else:
-            if len(nodes) == 0:
-                if node not in tree.keys():
-                    # No value for this parameter is present, if it's not
-                    # optional nor allow_none is set, the call below will
-                    # raise a MissingParameterError
-                    tree[node] = parameter.coerce(None)
-            else:
-                if node not in tree.keys():
-                    tree[node] = {}
-                self._ensure_tree(tree[node], parameter, *nodes)
-
-    def _flatten(self, params, tree, path=""):
-        """
-        For every element in L{tree}, set C{path} to C{value} in the given
-        L{params} dictionary.
-
-        @param params: A L{dict} which will be populated.
-        @param tree: A structure made up of L{Argument}s, L{list}s, L{dict}s
-            and leaf values.
-        """
-        if isinstance(tree, Arguments):
-            for name, value in tree:
-                self._flatten(params, value, "%s.%s" % (path, name))
-        elif isinstance(tree, dict):
-            for name, value in tree.iteritems():
-                self._flatten(params, value, "%s.%s" % (path, name))
-        elif isinstance(tree, list):
-            for index, value in enumerate(tree):
-                self._flatten(params, value, "%s.%d" % (path, index + 1))
-        elif tree is not None:
-            params[path.lstrip(".")] = tree
-        else:
-            # None is discarded.
-            pass
+                    result[name] = ""
+                else:
+                    result[name] = parameter.format(value)
+
+        return self._convert_nest_to_flat(result)
+
+    def _convert_flat_to_nest(self, params):
+        """
+        Convert a structure in the form of::
+
+            {'foo.1.bar': 'value',
+             'foo.2.baz': 'value'}
+
+        to::
+
+            {'foo': {'1': {'bar': 'value'},
+                     '2': {'baz': 'value'}}}
+
+        This is intended for use both during parsing of HTTP arguments like
+        'foo.1.bar=value' and when dealing with schema declarations that look
+        like 'foo.n.bar'.
+
+        This is the inverse of L{_convert_nest_to_flat}.
+        """
+        result = {}
+        for k, v in params.iteritems():
+            last = result
+            segments = k.split('.')
+            for index, item in enumerate(segments):
+                if index == len(segments) - 1:
+                    newd = v
+                else:
+                    newd = {}
+                if not isinstance(last, dict):
+                    raise InconsistentParameterError(k)
+                if type(last.get(item)) is dict and type(newd) is not dict:
+                    raise InconsistentParameterError(k)
+                last = last.setdefault(item, newd)
+        return result
+
+    def _convert_nest_to_flat(self, params, _result=None, _prefix=None):
+        """
+        Convert a data structure that looks like::
+
+            {"foo": {"bar": "baz", "shimmy": "sham"}}
+
+        to::
+
+            {"foo.bar": "baz",
+             "foo.shimmy": "sham"}
+
+        This is the inverse of L{_convert_flat_to_nest}.
+        """
+        if _result is None:
+            _result = {}
+        for k, v in params.iteritems():
+            if _prefix is None:
+                path = k
+            else:
+                path = _prefix + '.' + k
+            if isinstance(v, dict):
+                self._convert_nest_to_flat(v, _result=_result, _prefix=path)
+            else:
+                _result[path] = v
+        return _result
 
     def extend(self, *schema_items):
         """
@@ -513,3 +615,50 @@
             else:
                 raise TypeError("Illegal argument %s" % item)
         return Schema(*parameters)
+
+    def _convert_old_schema(self, parameters):
+        """
+        Convert an ugly old schema, using dotted names, to the hot new schema,
+        using List and Structure.
+
+        The old schema assumes that every other dot implies an array. So a list
+        of two parameters,
+
+            [Integer("foo.bar.baz.quux"), Integer("foo.bar.shimmy")]
+
+        becomes::
+
+            {"foo": List(
+                item=Structure(
+                    fields={"baz": List(item=Integer()),
+                            "shimmy": Integer()}))}
+
+        By design, the old schema syntax ignored the names "bar" and "quux".
+        """
+        crap = {}
+        for parameter in parameters:
+            crap[parameter.name] = parameter
+        nest = self._convert_flat_to_nest(crap)
+        return self._inner_convert_old_schema(nest, 0).fields
+
+    def _inner_convert_old_schema(self, mapping, depth):
+        """
+        Internal recursion helper for L{_convert_old_schema}.
+        """
+        if not isinstance(mapping, dict):
+            return mapping
+        if depth % 2 == 0:
+            fields = {}
+            for k, v in mapping.iteritems():
+                fields[k] = self._inner_convert_old_schema(v, depth + 1)
+            return Structure("anonymous_structure", fields=fields)
+        else:
+            if not isinstance(mapping, dict):
+                raise TypeError("mapping %r must be a dict" % (mapping,))
+            if not len(mapping) == 1:
+                raise ValueError("Multiple different index names specified: %r"
+                                 % (mapping.keys(),))
+            item = mapping.values()[0]
+            item = self._inner_convert_old_schema(item, depth + 1)
+            name = item.name.split('.', 1)[0]
+            return List(name=name, item=item, optional=item.optional)

=== modified file 'txaws/server/tests/test_resource.py'
--- txaws/server/tests/test_resource.py	2012-01-27 02:10:24 +0000
+++ txaws/server/tests/test_resource.py	2012-05-02 15:45:23 +0000
@@ -344,7 +344,8 @@
             errors = self.flushLoggedErrors()
             self.assertEquals(0, len(errors))
             self.assertEqual("MissingParameter - The request must contain "
-                             "the parameter Action", request.response)
+                             "the parameter Action (unicode)",
+                             request.response)
             self.assertEqual(400, request.code)
 
         return self.api.handle(request).addCallback(check)

=== modified file 'txaws/server/tests/test_schema.py'
--- txaws/server/tests/test_schema.py	2012-03-27 12:01:45 +0000
+++ txaws/server/tests/test_schema.py	2012-05-02 15:45:23 +0000
@@ -8,7 +8,9 @@
 
 from txaws.server.exception import APIError
 from txaws.server.schema import (
-    Arguments, Bool, Date, Enum, Integer, Parameter, RawStr, Schema, Unicode)
+    Arguments, Bool, Date, Enum, Integer, Parameter, RawStr, Schema, Unicode,
+    List, Structure,
+    InconsistentParameterError, InvalidParameterValueError)
 
 
 class ArgumentsTestCase(TestCase):
@@ -77,10 +79,12 @@
         required but not present in the request.
         """
         parameter = Parameter("Test")
+        parameter.kind = "testy kind"
         error = self.assertRaises(APIError, parameter.coerce, None)
         self.assertEqual(400, error.status)
         self.assertEqual("MissingParameter", error.code)
-        self.assertEqual("The request must contain the parameter Test",
+        self.assertEqual("The request must contain the parameter Test "
+                         "(testy kind)",
                          error.message)
 
     def test_coerce_with_default(self):
@@ -381,6 +385,11 @@
         _, rest = schema.extract({"name": "value"})
         self.assertEqual(rest, {"name": "value"})
 
+    def test_extract_with_nested_rest(self):
+        schema = Schema()
+        _, rest = schema.extract({"foo.1.bar": "hey", "foo.2.baz": "there"})
+        self.assertEqual({"foo.1.bar": "hey", "foo.2.baz": "there"}, rest)
+
     def test_extract_with_many_arguments(self):
         """L{Schema.extract} can handle multiple parameters."""
         schema = Schema(Unicode("name"), Integer("count"))
@@ -395,6 +404,26 @@
         self.assertEqual(u"value", arguments.name)
         self.assertEqual(None, arguments.count)
 
+    def test_extract_with_optional_default(self):
+        """
+        The value of C{default} on a parameter is used as the value when it is
+        not provided as an argument and the parameter is C{optional}.
+        """
+        schema = Schema(Unicode("name"),
+                        Integer("count", optional=True, default=5))
+        arguments, _ = schema.extract({"name": "value"})
+        self.assertEqual(u"value", arguments.name)
+        self.assertEqual(5, arguments.count)
+
+    def test_extract_structure_with_optional(self):
+        """L{Schema.extract} can handle optional parameters."""
+        schema = Schema(
+            Structure(
+                "struct",
+                fields={"name": Unicode(optional=True, default="radix")}))
+        arguments, _ = schema.extract({"struct": {}})
+        self.assertEqual(u"radix", arguments.struct.name)
+
     def test_extract_with_numbered(self):
         """
         L{Schema.extract} can handle parameters with numbered values.
@@ -404,12 +433,23 @@
         self.assertEqual("Joe", arguments.name[0])
         self.assertEqual("Tom", arguments.name[1])
 
+    def test_extract_with_goofy_numbered(self):
+        """
+        L{Schema.extract} only uses the relative values of indices to determine
+        the index in the resultant list.
+        """
+        schema = Schema(Unicode("name.n"))
+        arguments, _ = schema.extract({"name.5": "Joe", "name.10": "Tom"})
+        self.assertEqual("Joe", arguments.name[0])
+        self.assertEqual("Tom", arguments.name[1])
+
     def test_extract_with_single_numbered(self):
         """
-        L{Schema.extract} can handle a single parameter with a numbered value.
+        L{Schema.extract} can handle an un-numbered argument passed in to a
+        numbered parameter.
         """
         schema = Schema(Unicode("name.n"))
-        arguments, _ = schema.extract({"name.0": "Joe"})
+        arguments, _ = schema.extract({"name": "Joe"})
         self.assertEqual("Joe", arguments.name[0])
 
     def test_extract_complex(self):
@@ -458,8 +498,8 @@
         given without an index.
         """
         schema = Schema(Unicode("name.n"))
-        _, rest = schema.extract({"name": "foo", "name.1": "bar"})
-        self.assertEqual(rest, {"name": "foo"})
+        self.assertRaises(InconsistentParameterError,
+                          schema.extract, {"name": "foo", "name.1": "bar"})
 
     def test_extract_with_non_numbered_template(self):
         """
@@ -480,7 +520,7 @@
         error = self.assertRaises(APIError, schema.extract, params)
         self.assertEqual(400, error.status)
         self.assertEqual("UnknownParameter", error.code)
-        self.assertEqual("The parameter name.one is not recognized",
+        self.assertEqual("The parameter one is not recognized",
                          error.message)
 
     def test_extract_with_negative_index(self):
@@ -493,7 +533,7 @@
         error = self.assertRaises(APIError, schema.extract, params)
         self.assertEqual(400, error.status)
         self.assertEqual("UnknownParameter", error.code)
-        self.assertEqual("The parameter name.-1 is not recognized",
+        self.assertEqual("The parameter -1 is not recognized",
                          error.message)
 
     def test_bundle(self):
@@ -513,6 +553,16 @@
         params = schema.bundle(name=["foo", "bar"])
         self.assertEqual({"name.1": "foo", "name.2": "bar"}, params)
 
+    def test_bundle_with_two_numbered(self):
+        """
+        L{Schema.bundle} can bundle multiple numbered lists.
+        """
+        schema = Schema(Unicode("names.n"), Unicode("things.n"))
+        params = schema.bundle(names=["foo", "bar"], things=["baz", "quux"])
+        self.assertEqual({"names.1": "foo", "names.2": "bar",
+                          "things.1": "baz", "things.2": "quux"},
+                         params)
+
     def test_bundle_with_none(self):
         """L{None} values are discarded in L{Schema.bundle}."""
         schema = Schema(Unicode("name.n", optional=True))
@@ -524,7 +574,7 @@
         L{Schema.bundle} correctly handles an empty numbered arguments list.
         """
         schema = Schema(Unicode("name.n"))
-        params = schema.bundle(names=[])
+        params = schema.bundle(name=[])
         self.assertEqual({}, params)
 
     def test_bundle_with_numbered_not_supplied(self):
@@ -544,6 +594,41 @@
         self.assertEqual({"name.1": "Foo", "name.2": "Bar", "count": "123"},
                          params)
 
+    def test_bundle_with_structure(self):
+        """L{Schema.bundle} can bundle L{Structure}s."""
+        schema = Schema(
+            parameters={
+                "struct": Structure(fields={"field1": Unicode(),
+                                            "field2": Integer()})})
+        params = schema.bundle(struct={"field1": "hi", "field2": 59})
+        self.assertEqual({"struct.field1": "hi", "struct.field2": "59"},
+                         params)
+
+    def test_bundle_with_list(self):
+        """L{Schema.bundle} can bundle L{List}s."""
+        schema = Schema(parameters={"things": List(item=Unicode())})
+        params = schema.bundle(things=["foo", "bar"])
+        self.assertEqual({"things.1": "foo", "things.2": "bar"}, params)
+
+    def test_bundle_with_structure_with_arguments(self):
+        """
+        L{Schema.bundle} can bundle L{Structure}s (specified as L{Arguments}).
+        """
+        schema = Schema(
+            parameters={
+                "struct": Structure(fields={"field1": Unicode(),
+                                            "field2": Integer()})})
+        params = schema.bundle(struct=Arguments({"field1": "hi",
+                                                 "field2": 59}))
+        self.assertEqual({"struct.field1": "hi", "struct.field2": "59"},
+                         params)
+
+    def test_bundle_with_list_with_arguments(self):
+        """L{Schema.bundle} can bundle L{List}s (specified as L{Arguments})."""
+        schema = Schema(parameters={"things": List(item=Unicode())})
+        params = schema.bundle(things=Arguments({1: "foo", 2: "bar"}))
+        self.assertEqual({"things.1": "foo", "things.2": "bar"}, params)
+
     def test_bundle_with_arguments(self):
         """L{Schema.bundle} can bundle L{Arguments} too."""
         schema = Schema(Unicode("name.n"), Integer("count"))
@@ -590,3 +675,124 @@
         self.assertEqual(u"value", arguments.name)
         self.assertEqual("testing", arguments.computer)
         self.assertEqual(5, arguments.count)
+
+    def test_list(self):
+        """L{List}s can be extracted."""
+        schema = Schema(List("foo", Integer()))
+        arguments, _ = schema.extract({"foo.1": "1", "foo.2": "2"})
+        self.assertEqual([1, 2], arguments.foo)
+
+    def test_optional_list(self):
+        """
+        The default value of an optional L{List} is C{[]}.
+        """
+        schema = Schema(List("names", Unicode(), optional=True))
+        arguments, _ = schema.extract({})
+        self.assertEqual([], arguments.names)
+
+    def test_default_list(self):
+        """
+        The default of a L{List} can be specified as a list.
+        """
+        schema = Schema(List("names", Unicode(), optional=True, default=[u"foo", u"bar"]))
+        arguments, _ = schema.extract({})
+        self.assertEqual([u"foo", u"bar"], arguments.names)
+
+    def test_list_of_list(self):
+        """L{List}s can be nested."""
+        schema = Schema(List("foo", List(item=Unicode())))
+        arguments, _ = schema.extract(
+            {"foo.1.1": "first-first", "foo.1.2": "first-second",
+             "foo.2.1": "second-first", "foo.2.2": "second-second"})
+        self.assertEqual([["first-first", "first-second"],
+                          ["second-first", "second-second"]],
+                         arguments.foo)
+
+    def test_structure(self):
+        """
+        L{Schema}s with L{Structure} parameters can have arguments extracted.
+        """
+        schema = Schema(Structure("foo", {"a": Integer(), "b": Integer()}))
+        arguments, _ = schema.extract({"foo.a": "1", "foo.b": "2"})
+        self.assertEqual(1, arguments.foo.a)
+        self.assertEqual(2, arguments.foo.b)
+
+    def test_structure_of_structures(self):
+        """L{Structure}s can be nested."""
+        sub_struct = Structure(fields={"a": Unicode(), "b": Unicode()})
+        schema = Schema(Structure("foo", fields={"a": sub_struct,
+                                                 "b": sub_struct}))
+        arguments, _ = schema.extract({"foo.a.a": "a-a", "foo.a.b": "a-b",
+                                       "foo.b.a": "b-a", "foo.b.b": "b-b"})
+        self.assertEqual("a-a", arguments.foo.a.a)
+        self.assertEqual("a-b", arguments.foo.a.b)
+        self.assertEqual("b-a", arguments.foo.b.a)
+        self.assertEqual("b-b", arguments.foo.b.b)
+
+    def test_list_of_structures(self):
+        """L{List}s of L{Structure}s are extracted properly."""
+        schema = Schema(
+            List("foo", Structure(fields={"a": Integer(), "b": Integer()})))
+        arguments, _ = schema.extract({"foo.1.a": "1", "foo.1.b": "2",
+                                       "foo.2.a": "3", "foo.2.b": "4"})
+        self.assertEqual(1, arguments.foo[0]['a'])
+        self.assertEqual(2, arguments.foo[0]['b'])
+        self.assertEqual(3, arguments.foo[1]['a'])
+        self.assertEqual(4, arguments.foo[1]['b'])
+
+    def test_structure_of_list(self):
+        """L{Structure}s of L{List}s are extracted properly."""
+        schema = Schema(Structure("foo", fields={"l": List(item=Integer())}))
+        arguments, _ = schema.extract({"foo.l.1": "1", "foo.l.2": "2"})
+        self.assertEqual([1, 2], arguments.foo.l)
+
+    def test_new_parameters(self):
+        """
+        L{Schema} accepts a C{parameters} parameter to specify parameters in a
+        {name: field} format.
+        """
+        schema = Schema(
+            parameters={"foo": Structure(
+                    fields={"l": List(item=Integer())})})
+        arguments, _ = schema.extract({"foo.l.1": "1", "foo.l.2": "2"})
+        self.assertEqual([1, 2], arguments.foo.l)
+
+    def test_schema_conversion_list_name(self):
+        """
+        Backwards-compatibility conversion maintains the name of lists.
+        """
+        schema = Schema(Unicode("foos.N"))
+        self.assertEqual("foos", schema._parameters["foos"].name)
+
+    def test_schema_conversion_structure_name(self):
+        """
+        Backwards-compatibility conversion maintains the names of fields in
+        structures.
+        """
+        schema = Schema(Unicode("foos.N.field"),
+                        Unicode("foos.N.field2"))
+        self.assertEqual("anonymous_structure",
+                         schema._parameters["foos"].item.name)
+        self.assertEqual("foos.N.field",
+                         schema._parameters["foos"].item.fields["field"].name)
+        self.assertEqual("foos.N.field2",
+                         schema._parameters["foos"].item.fields["field2"].name)
+
+    def test_schema_conversion_optional_list(self):
+        """
+        Backwards-compatibility conversions maintains optional-ness of lists.
+        """
+        schema = Schema(Unicode("foos.N", optional=True))
+        arguments, _ = schema.extract({})
+        self.assertEqual([], arguments.foos)
+
+    def test_schema_conversion_optional_structure_field(self):
+        """
+        Backwards-compatibility conversion maintains optional-ness of structure
+        fields.
+        """
+        schema = Schema(Unicode("foos.N.field"),
+                        Unicode("foos.N.field2", optional=True, default=u"hi"))
+        arguments, _ = schema.extract({"foos.0.field": u"existent"})
+        self.assertEqual(u"existent", arguments.foos[0].field)
+        self.assertEqual(u"hi", arguments.foos[0].field2)