Code review comment for lp:~leonardr/wadllib/convert-datetime-309950

OK, this should address everyone's comments.

=== modified file 'wadllib/application.py'
--- wadllib/application.py 2009-01-21 19:12:41 +0000
+++ wadllib/application.py 2009-01-22 15:33:42 +0000
@@ -43,6 +43,7 @@
     'WADLError',
     ]

+from cStringIO import StringIO
 import datetime
 import time
 import urllib
@@ -58,6 +59,9 @@
 from iso_strptime import iso_strptime
 from wadllib._utils import uri

+NS_MAP = "xmlns:map"
+XML_SCHEMA_NS_URI = 'http://www.w3.org/2001/XMLSchema'
+
 def wadl_tag(tag_name):
     """Scope a tag name with the WADL namespace."""
     return '{http://research.sun.com/wadl/2006/10}' + tag_name
@@ -411,7 +415,11 @@
         return None

     def get_parameter_value(self, parameter):
- """Find the value of a parameter, given the Parameter object."""
+ """Find the value of a parameter, given the Parameter object.
+
+ :raise ValueError: If the parameter value can't be converted into
+ its defined type.
+ """

         if self.representation is None:
             raise NoBoundRepresentationError(
@@ -430,25 +438,46 @@
                     "type %s." % parameter.style)
             value = self.representation[parameter.name]
             if value is not None:
- if parameter.type in ['xsd:dateTime', 'xsd:date']:
+ namespace_url, data_type = self._dereference_namespace(
+ parameter.tag, parameter.type)
+ if (namespace_url == XML_SCHEMA_NS_URI
+ and data_type in ['dateTime', 'date']):
                     try:
- # Parse it as an ISO 1601 date and time.
+ # Parse it as an ISO 8601 date and time.
                         value = iso_strptime(value)
                     except ValueError:
- # Parse it as an ISO 1601 date.
+ # Parse it as an ISO 8601 date.
                         try:
                             value = datetime.datetime(
                                 *(time.strptime(value, "%Y-%m-%d")[0:6]))
                         except ValueError:
- # Oh well. Leave it as a string.
- pass
-
+ # Raise an unadored ValueError so the client
+ # can treat the value as a string if they
+ # want.
+ raise ValueError(value)
             return value

         raise NotImplementedError("Path traversal not implemented for "
                                   "a representation of media type %s."
                                   % self.media_type)

+
+ def _dereference_namespace(self, tag, value):
+ """Splits a value into namespace URI and value.
+
+ :param tag: A tag to use as context when mapping namespace
+ names to URIs.
+ """
+ namespace_url = None
+ if value is not None and ':' in value:
+ namespace, value = value.split(':', 1)
+ else:
+ namespace = ''
+ if namespace is not None:
+ ns_map = tag.get(NS_MAP)
+ namespace_url = ns_map.get(namespace)
+ return namespace_url, value
+
     def _definition_factory(self, id):
         """Given an ID, find a ResourceType for that ID."""
         return self.application.resource_types.get(id)
@@ -906,7 +935,7 @@
         self.markup_url = markup_url
         if hasattr(markup, 'read'):
             markup = markup.read()
- self.doc = ET.fromstring(markup)
+ self.doc = self._from_string(markup)
         self.resources = self.doc.find(wadl_xpath('resources'))
         self.resource_base = self.resources.attrib.get('base')
         self.representation_definitions = {}
@@ -921,6 +950,27 @@
             id = resource_type.attrib['id']
             self.resource_types[id] = ResourceType(resource_type)

+ def _from_string(self, markup):
+ """Turns markup into a document.
+
+ Just a wrapper around ElementTree which keeps track of namespaces.
+ """
+ events = "start", "start-ns", "end-ns"
+ root = None
+ ns_map = []
+
+ for event, elem in ET.iterparse(StringIO(markup), events):
+ if event == "start-ns":
+ ns_map.append(elem)
+ elif event == "end-ns":
+ ns_map.pop()
+ elif event == "start":
+ if root is None:
+ root = elem
+ elem.set(NS_MAP, dict(ns_map))
+ return ET.ElementTree(root)
+
+
     def get_resource_type(self, resource_type_url):
         """Retrieve a resource type by the URL of its description."""
         xml_id = self.lookup_xml_id(resource_type_url)
@@ -959,7 +1009,7 @@
         # representation of a non-root resource to its definition at
         # the server root.
         raise NotImplementedError("Can't look up definition in another "
- "url (%s)" % (url))
+ "url (%s)" % url)

     def get_resource_by_path(self, path):
         """Locate one of the resources described by this document.

=== modified file 'wadllib/docs/wadllib.txt'
--- wadllib/docs/wadllib.txt 2009-01-21 19:12:41 +0000
+++ wadllib/docs/wadllib.txt 2009-01-22 15:12:32 +0000
@@ -354,7 +354,7 @@
    >>> print method.response.get_representation_definition('text/html')
    None

-=== Data type converstion ===
+=== Data type conversion ===

 The values of date and dateTime parameters are automatically converted to
 Python datetime objects.
@@ -388,14 +388,17 @@
    >>> bound_root.get_parameter('a_date').get_value()
    datetime.datetime(2005, 6, 6, 8, ...)

-If a date or dateTime parameter has a value that can't be parsed to a
-datetime object, you get the original value.
+If a date or dateTime parameter has a null value, you get None. If the
+value is a string that can't be parsed to a datetime object, you get a
+ValueError.

    >>> representation = simplejson.dumps(
    ... {'a_date': 'foo', 'a_datetime': None})
    >>> bound_root = service_root.bind(representation, 'application/json')
    >>> bound_root.get_parameter('a_date').get_value()
- u'foo'
+ Traceback (most recent call last):
+ ...
+ ValueError: foo
    >>> print bound_root.get_parameter('a_datetime').get_value()
    None

=== modified file 'wadllib/iso_strptime.py'
--- wadllib/iso_strptime.py 2009-01-21 15:39:44 +0000
+++ wadllib/iso_strptime.py 2009-01-22 16:04:19 +0000
@@ -16,7 +16,7 @@
 # License along with wadllib. If not, see
 # <http://www.gnu.org/licenses/>.
 """
-Parser for ISO 1601 time strings
+Parser for ISO 8601 time strings
 ================================

 >>> d = iso_strptime("2008-01-07T05:30:30.345323+03:00")
@@ -41,23 +41,30 @@
 import datetime

 RE_TIME = re.compile(r"""^
- (?P<year>\d{4})\-(?P<month>\d{2})\-(?P<day>\d{2}) # pattern matching date
- T # seperator
- (?P<hour>\d{2})\:(?P<minutes>\d{2})\:(?P<seconds>\d{2}) # pattern matching time
- (\.(?P<microseconds>\d{6}))? # pattern matching optional microseconds
- (?P<tz_offset>[\-\+]\d{2}\:\d{2})? # pattern matching optional timezone offset
- $""", re.VERBOSE)
-
+ (?P<year>\d{4})\-(?P<month>\d{2})\-(?P<day>\d{2})
+ # pattern matching date
+ T
+ # seperator
+ (?P<hour>\d{2})\:(?P<minutes>\d{2})\:(?P<seconds>\d{2})
+ # pattern matching time
+ (\.(?P<microseconds>\d{6}))?
+ # pattern matching optional microseconds
+ (?P<tz_offset>[\-\+]\d{2}\:\d{2})?
+ # pattern matching optional timezone offset
+ $""", re.VERBOSE)
+
 class TimeZone(datetime.tzinfo):

     def __init__(self, tz_string):
         hours, minutes = tz_string.lstrip("-+").split(":")
- self.stdoffset = datetime.timedelta(hours=int(hours), minutes=int(minutes))
+ self.stdoffset = datetime.timedelta(hours=int(hours),
+ minutes=int(minutes))
         if tz_string.startswith("-"):
             self.stdoffset *= -1
-
+
     def __repr__(self):
- return "TimeZone(%s)" %(self.stdoffset.days*24*60*60 + self.stdoffset.seconds)
+ return "TimeZone(%s)" % (
+ self.stdoffset.days*24*60*60 + self.stdoffset.seconds)

     def utcoffset(self, dt):
         return self.stdoffset
@@ -79,7 +86,3 @@
     if x.group("tz_offset"):
         d = d.replace(tzinfo=TimeZone(x.group("tz_offset")))
     return d
-
-if __name__ == '__main__':
- import doctest
- doctest.testmod()