Merge lp:~wallyworld/lazr.restful/dict-marshalling-default-key-value-types into lp:lazr.restful

[Ian Booth]

Improve the collection field marshallers so that if the fields haven't specified key and/or value types, the default marshallers are used for the collection elements.

== Implementation ==

Improve the collection field marshallers so that if the fields haven't specified key and/or value types, the default marshallers are used for the collection elements. This was needed because of an error unmarshalling a return value of type Dict in Launchpad. While I was fixing this, I also added the improvement to AbstractCollectionMarshaller.

Some drive-by lint also.

== Tests ==

Add new tests to webservice-marshallers.txt

== Lint ==

Linting changed files:
  src/lazr/restful/NEWS.txt
  src/lazr/restful/marshallers.py
  src/lazr/restful/version.txt
  src/lazr/restful/docs/webservice-marshallers.txt

[Steve Kowalik (stevenk)]

Looks broadly good, I'm a little unhappy about the amount you have repeated getMultiAdapter with similar arguments in the marshaling code -- I'm not going to block on it, but I'd like you to have a think about a cleaner way to implement your changes.

review: Approve (code)

[Ian Booth (wallyworld)]

> Looks broadly good, I'm a little unhappy about the amount you have repeated
> getMultiAdapter with similar arguments in the marshaling code -- I'm not going
> to block on it, but I'd like you to have a think about a cleaner way to
> implement your changes.

Yes, good point.

    self.value_marshaller = getMultiAdapter(
        (field.value_type or Field(), request), IFieldMarshaller)

is much nicer.

=== modified file 'src/lazr/restful/NEWS.txt'
--- src/lazr/restful/NEWS.txt	2012-03-13 02:03:47 +0000
+++ src/lazr/restful/NEWS.txt	2012-03-15 03:35:20 +0000
@@ -2,6 +2,13 @@
 NEWS for lazr.restful
 =====================
 
+0.19.6 (2012-03-15)
+===================
+
+Fixed bug 955668: make marshallers work correctly for  collection fields
+(Set, List, Dict) where the key and/or value types have not been specified.
+In such cases, the default marshaller is used for the collection elements.
+
 0.19.5 (2012-03-13)
 ===================
 

=== modified file 'src/lazr/restful/docs/webservice-marshallers.txt'
--- src/lazr/restful/docs/webservice-marshallers.txt	2012-03-13 23:47:22 +0000
+++ src/lazr/restful/docs/webservice-marshallers.txt	2012-03-15 03:35:20 +0000
@@ -701,7 +701,8 @@
 collection marshaller will take care of marshalling to the proper
 collection type, and of marshalling the individual items using the
 marshaller for its value_type. Dictionaries may specify separate
-marshallers for their keys and values.
+marshallers for their keys and values. If no key and/or value marshallers
+are specified, the default SimpleFieldMarshaller is used.
 
     >>> from zope.schema import Dict, List, Tuple, Set
     >>> list_of_strings_field = List(value_type=Text())
@@ -709,17 +710,27 @@
     >>> tuple_of_ints_field = Tuple(value_type=Int())
     >>> list_of_choices_field = List(
     ...     value_type=Choice(vocabulary=Cuisine))
+    >>> simple_list_field = List()
     >>> set_of_choices_field = Set(
     ...  value_type=Choice(vocabulary=Cuisine)).bind(None)
     >>> dict_of_choices_field = Dict(
     ...     key_type=Text(),
     ...     value_type=Choice(vocabulary=Cuisine))
+    >>> simple_dict_field = Dict()
 
     >>> list_marshaller = getMultiAdapter(
     ...     (list_of_strings_field, request), IFieldMarshaller)
     >>> verifyObject(IFieldMarshaller, list_marshaller)
     True
 
+    >>> simple_list_marshaller = getMultiAdapter(
+    ...     (simple_list_field, request), IFieldMarshaller)
+    >>> verifyObject(IFieldMarshaller, simple_list_marshaller)
+    True
+    >>> verifyObject(
+    ...     IFieldMarshaller, simple_list_marshaller.value_marshaller)
+    True
+
     >>> tuple_marshaller = getMultiAdapter(
     ...     (tuple_of_ints_field, request), IFieldMarshaller)
     >>> verifyObject(IFieldMarshaller, tuple_marshaller)
@@ -739,13 +750,19 @@
     ...     (dict_of_choices_field, request), IFieldMarshaller)
     >>> verifyObject(IFieldMarshaller, dict_marshaller)
     True
-    >>> dict_key_marshaller = getMultiAdapter(
-    ...     (dict_of_choices_field.key_type, request), IFieldMarshaller)
-    >>> verifyObject(IFieldMarshaller, dict_key_marshaller)
-    True
-    >>> dict_value_marshaller = getMultiAdapter(
-    ...     (dict_of_choices_field.value_type, request), IFieldMarshaller)
-    >>> verifyObject(IFieldMarshaller, dict_value_marshaller)
+    >>> verifyObject(IFieldMarshaller, dict_marshaller.key_marshaller)
+    True
+    >>> verifyObject(IFieldMarshaller, dict_marshaller.value_marshaller)
+    True
+
+    >>> simple_dict_marshaller = getMultiAdapter(
+    ...     (simple_dict_field, request), IFieldMarshaller)
+    >>> verifyObject(IFieldMarshaller, simple_dict_marshaller)
+    True
+    >>> verifyObject(IFieldMarshaller, simple_dict_marshaller.key_marshaller)
+    True
+    >>> verifyObject(
+    ...     IFieldMarshaller, simple_dict_marshaller.value_marshaller)
     True
 
 For sequences, the only JSON representation for the collection itself is a

=== modified file 'src/lazr/restful/marshallers.py'
--- src/lazr/restful/marshallers.py	2012-03-13 23:47:22 +0000
+++ src/lazr/restful/marshallers.py	2012-03-15 03:35:20 +0000
@@ -39,6 +39,7 @@
     )
 from zope.interface import implements
 from zope.publisher.interfaces import NotFound
+from zope.schema import Field
 from zope.security.proxy import removeSecurityProxy
 from zope.traversing.browser import absoluteURL
 
@@ -403,7 +404,7 @@
         super(AbstractCollectionFieldMarshaller, self).__init__(
             field, request)
         self.value_marshaller = getMultiAdapter(
-            (field.value_type, request), IFieldMarshaller)
+            (field.value_type or Field(), request), IFieldMarshaller)
 
     def _marshall_from_json_data(self, value):
         """See `SimpleFieldMarshaller`.
@@ -475,14 +476,15 @@
         """See `SimpleFieldMarshaller`.
 
         This also looks for the appropriate marshaller for key_type and
-        value_type.
+        value_type. If key_type or value_type are not specified, the default
+        field marshaller is used.
         """
         super(DictFieldMarshaller, self).__init__(
             field, request)
         self.key_marshaller = getMultiAdapter(
-            (field.key_type, request), IFieldMarshaller)
+            (field.key_type or Field(), request), IFieldMarshaller)
         self.value_marshaller = getMultiAdapter(
-            (field.value_type, request), IFieldMarshaller)
+            (field.value_type or Field(), request), IFieldMarshaller)
 
     def _marshall_from_request(self, value):
         """See `IFieldMarshaller`.
@@ -492,8 +494,8 @@
 
         When coming in via HTML request parameters, the dict is sent as a list
         of comma separated "key,value" strings.
-        If the value isn't a list or dict, transform it into a one-element list.
-        That allows web client to submit one-element list of strings
+        If the value isn't a list or dict, transform it into a one-element
+        list. That allows web client to submit one-element list of strings
         without having to JSON-encode it.
         """
         if not isinstance(value, list) and not isinstance(value, dict):

=== modified file 'src/lazr/restful/version.txt'
--- src/lazr/restful/version.txt	2012-03-13 02:03:47 +0000
+++ src/lazr/restful/version.txt	2012-03-15 03:35:20 +0000
@@ -1,1 +1,1 @@
-0.19.5
+0.19.6