Repository: falconry/falcon

Issue:
Return if-modified-since, if-unmodified-since as datetime.datetime
Currently, the HTTP date string is returned. It might as well be parsed, since the app will need to do that anyway before it can use the value.

Discussion:

Patch:
diff --git a/doc/user/install.rst b/doc/user/install.rst
index 6633e0e..482becd 100644
--- a/doc/user/install.rst
+++ b/doc/user/install.rst
@@ -21,16 +21,6 @@ type:
 
     $ pip install --upgrade falcon
 
-.. note::
-
-    When using Cython, you should always recompile Falcon after
-    upgrading Python. To do this, simply run:
-
-    .. code:: bash
-
-        $ pip install --force-reinstall --upgrade cython
-        $ pip install --force-reinstall --upgrade falcon
-
 Installing Cython on OS X
 -------------------------
 
diff --git a/falcon/api.py b/falcon/api.py
index b12e16d..ff61dc1 100644
--- a/falcon/api.py
+++ b/falcon/api.py
@@ -53,7 +53,7 @@ class API(object):
                         \"""
 
                     def process_resource(self, req, resp, resource):
-                        \"""Process the request and resource *after* routing.
+                        \"""Process the request after routing.
 
                         Args:
                             req: Request object that will be passed to the
diff --git a/falcon/request.py b/falcon/request.py
index be8aabf..b52fe13 100644
--- a/falcon/request.py
+++ b/falcon/request.py
@@ -146,10 +146,10 @@ class Request(object):
             header is missing.
         if_none_match (str): Value of the If-None-Match header, or ``None``
             if the header is missing.
-        if_modified_since (str): Value of the If-Modified-Since header, or
-            ``None`` if the header is missing.
-        if_unmodified_since (str): Value of the If-Unmodified-Sinc header,
+        if_modified_since (datetime): Value of the If-Modified-Since header,
             or ``None`` if the header is missing.
+        if_unmodified_since (datetime): Value of the If-Unmodified-Since
+            header, or ``None`` if the header is missing.
         if_range (str): Value of the If-Range header, or ``None`` if the
             header is missing.
 
@@ -271,8 +271,6 @@ class Request(object):
 
     if_match = helpers.header_property('HTTP_IF_MATCH')
     if_none_match = helpers.header_property('HTTP_IF_NONE_MATCH')
-    if_modified_since = helpers.header_property('HTTP_IF_MODIFIED_SINCE')
-    if_unmodified_since = helpers.header_property('HTTP_IF_UNMODIFIED_SINCE')
     if_range = helpers.header_property('HTTP_IF_RANGE')
 
     @property
@@ -326,16 +324,15 @@ class Request(object):
 
     @property
     def date(self):
-        try:
-            http_date = self.env['HTTP_DATE']
-        except KeyError:
-            return None
+        return self.get_header_as_datetime('Date')
 
-        try:
-            return util.http_date_to_dt(http_date)
-        except ValueError:
-            msg = ('It must be formatted according to RFC 1123.')
-            raise HTTPInvalidHeader(msg, 'Date')
+    @property
+    def if_modified_since(self):
+        return self.get_header_as_datetime('If-Modified-Since')
+
+    @property
+    def if_unmodified_since(self):
+        return self.get_header_as_datetime('If-Unmodified-Since')
 
     @property
     def range(self):
@@ -343,9 +340,6 @@ class Request(object):
             value = self.env['HTTP_RANGE']
             if value.startswith('bytes='):
                 value = value[6:]
-            else:
-                msg = "The value must be prefixed with 'bytes='"
-                raise HTTPInvalidHeader(msg, 'Range')
         except KeyError:
             return None
 
@@ -574,6 +568,35 @@ class Request(object):
 
             raise HTTPMissingParam(name)
 
+    def get_header_as_datetime(self, header, required=False):
+        """Return an HTTP header with HTTP-Date values as a datetime.
+
+        Args:
+            name (str): Header name, case-insensitive (e.g., 'Date')
+            required (bool, optional): Set to ``True`` to raise
+                ``HTTPBadRequest`` instead of returning gracefully when the
+                header is not found (default ``False``).
+
+        Returns:
+            datetime: The value of the specified header if it exists,
+                or ``None`` if the header is not found and is not required.
+
+        Raises:
+            HTTPBadRequest: The header was not found in the request, but
+                it was required.
+            HttpInvalidHeader: The header contained a malformed/invalid value.
+        """
+
+        try:
+            http_date = self.get_header(header, required=required)
+            return util.http_date_to_dt(http_date)
+        except TypeError:
+            # When the header does not exist and isn't required
+            return None
+        except ValueError:
+            msg = ('It must be formatted according to RFC 1123.')
+            raise HTTPInvalidHeader(msg, header)
+
     def get_param(self, name, required=False, store=None, default=None):
         """Return the raw value of a query string parameter as a string.
 
diff --git a/tools/clean.sh b/tools/clean.sh
deleted file mode 100755
index 5d41474..0000000
--- a/tools/clean.sh
+++ /dev/null
@@ -1,3 +0,0 @@
-#!/usr/bin/env bash
-
-find $1 \( -name '*.c' -or -name '*.so' -or -name '*.pyc' \) -delete
diff --git a/tools/clean_cythoned.sh b/tools/clean_cythoned.sh
new file mode 100755
index 0000000..c0c97af
--- /dev/null
+++ b/tools/clean_cythoned.sh
@@ -0,0 +1,5 @@
+#!/usr/bin/env bash
+
+rm -f $1/*.c
+rm -f $1/*.so
+rm -f $1/*.pyc
diff --git a/tox.ini b/tox.ini
index 4cc43ed..b950eb4 100644
--- a/tox.ini
+++ b/tox.ini
@@ -24,13 +24,13 @@ envlist = py26,
 
 [testenv]
 deps = -r{toxinidir}/tools/test-requires
-commands = {toxinidir}/tools/clean.sh {toxinidir}/falcon
+commands = {toxinidir}/tools/clean_cythoned.sh {toxinidir}/falcon
            nosetests {posargs}
 
 [testenv:py27]
 deps = -r{toxinidir}/tools/test-requires
        nose-cprof
-commands = {toxinidir}/tools/clean.sh {toxinidir}/falcon
+commands = {toxinidir}/tools/clean_cythoned.sh {toxinidir}/falcon
            nosetests \
                --with-cprofile \
                --cprofile-stats-erase \
@@ -41,7 +41,7 @@ commands = {toxinidir}/tools/clean.sh {toxinidir}/falcon
 deps = -r{toxinidir}/tools/test-requires
        cython
 basepython = python2.7
-commands = {toxinidir}/tools/clean.sh {toxinidir}/falcon
+commands = {toxinidir}/tools/clean_cythoned.sh {toxinidir}/falcon
            nosetests \
                {posargs}
 
@@ -49,7 +49,7 @@ commands = {toxinidir}/tools/clean.sh {toxinidir}/falcon
 deps = -r{toxinidir}/tools/test-requires
        cython
 basepython = python3.3
-commands = {toxinidir}/tools/clean.sh {toxinidir}/falcon
+commands = {toxinidir}/tools/clean_cythoned.sh {toxinidir}/falcon
            nosetests \
                {posargs}
 
@@ -57,7 +57,7 @@ commands = {toxinidir}/tools/clean.sh {toxinidir}/falcon
 deps = -r{toxinidir}/tools/test-requires
        cython
 basepython = python3.4
-commands = {toxinidir}/tools/clean.sh {toxinidir}/falcon
+commands = {toxinidir}/tools/clean_cythoned.sh {toxinidir}/falcon
            nosetests \
                {posargs}
 


Test patch:
diff --git a/falcon/testing/helpers.py b/falcon/testing/helpers.py
index a292587..5e1dfb0 100644
--- a/falcon/testing/helpers.py
+++ b/falcon/testing/helpers.py
@@ -15,28 +15,15 @@
 import random
 import io
 import sys
-from datetime import datetime
 
 import six
 
-import falcon
 from falcon.util import uri
 
 # Constants
 DEFAULT_HOST = 'falconframework.org'
 
 
-def httpnow():
-    """Returns the current UTC time as an RFC 1123 date.
-
-    Returns:
-        str: An HTTP date string, e.g., "Tue, 15 Nov 1994 12:45:26 GMT".
-
-    """
-
-    return falcon.dt_to_http(datetime.utcnow())
-
-
 def rand_string(min, max):
     """Returns a randomly-generated string, of a random length.
 
diff --git a/tests/test_req_vars.py b/tests/test_req_vars.py
index 5ee1809..978770e 100644
--- a/tests/test_req_vars.py
+++ b/tests/test_req_vars.py
@@ -368,15 +368,15 @@ class TestReqVars(testing.TestBase):
         self.assertEqual(preferred_type, None)
 
     def test_range(self):
-        headers = {'Range': 'bytes=10-'}
+        headers = {'Range': '10-'}
         req = Request(testing.create_environ(headers=headers))
         self.assertEqual(req.range, (10, -1))
 
-        headers = {'Range': 'bytes=10-20'}
+        headers = {'Range': '10-20'}
         req = Request(testing.create_environ(headers=headers))
         self.assertEqual(req.range, (10, 20))
 
-        headers = {'Range': 'bytes=-10240'}
+        headers = {'Range': '-10240'}
         req = Request(testing.create_environ(headers=headers))
         self.assertEqual(req.range, (-10240, -1))
 
@@ -392,62 +392,62 @@ class TestReqVars(testing.TestBase):
         self.assertIs(req.range, None)
 
     def test_range_invalid(self):
-        headers = {'Range': 'bytes=10240'}
+        headers = {'Range': '10240'}
         req = Request(testing.create_environ(headers=headers))
         self.assertRaises(falcon.HTTPBadRequest, lambda: req.range)
 
-        headers = {'Range': 'bytes=-'}
+        headers = {'Range': '-'}
         expected_desc = ('The value provided for the Range header is '
                          'invalid. The byte offsets are missing.')
         self._test_error_details(headers, 'range',
                                  falcon.HTTPInvalidHeader,
                                  'Invalid header value', expected_desc)
 
-        headers = {'Range': 'bytes=--'}
+        headers = {'Range': '--'}
         req = Request(testing.create_environ(headers=headers))
         self.assertRaises(falcon.HTTPBadRequest, lambda: req.range)
 
-        headers = {'Range': 'bytes=-3-'}
+        headers = {'Range': '-3-'}
         req = Request(testing.create_environ(headers=headers))
         self.assertRaises(falcon.HTTPBadRequest, lambda: req.range)
 
-        headers = {'Range': 'bytes=-3-4'}
+        headers = {'Range': '-3-4'}
         req = Request(testing.create_environ(headers=headers))
         self.assertRaises(falcon.HTTPBadRequest, lambda: req.range)
 
-        headers = {'Range': 'bytes=3-3-4'}
+        headers = {'Range': '3-3-4'}
         req = Request(testing.create_environ(headers=headers))
         self.assertRaises(falcon.HTTPBadRequest, lambda: req.range)
 
-        headers = {'Range': 'bytes=3-3-'}
+        headers = {'Range': '3-3-'}
         req = Request(testing.create_environ(headers=headers))
         self.assertRaises(falcon.HTTPBadRequest, lambda: req.range)
 
-        headers = {'Range': 'bytes=3-3- '}
+        headers = {'Range': '3-3- '}
         req = Request(testing.create_environ(headers=headers))
         self.assertRaises(falcon.HTTPBadRequest, lambda: req.range)
 
-        headers = {'Range': 'bytes=fizbit'}
+        headers = {'Range': 'fizbit'}
         req = Request(testing.create_environ(headers=headers))
         self.assertRaises(falcon.HTTPBadRequest, lambda: req.range)
 
-        headers = {'Range': 'bytes=a-'}
+        headers = {'Range': 'a-'}
         req = Request(testing.create_environ(headers=headers))
         self.assertRaises(falcon.HTTPBadRequest, lambda: req.range)
 
-        headers = {'Range': 'bytes=a-3'}
+        headers = {'Range': 'a-3'}
         req = Request(testing.create_environ(headers=headers))
         self.assertRaises(falcon.HTTPBadRequest, lambda: req.range)
 
-        headers = {'Range': 'bytes=-b'}
+        headers = {'Range': '-b'}
         req = Request(testing.create_environ(headers=headers))
         self.assertRaises(falcon.HTTPBadRequest, lambda: req.range)
 
-        headers = {'Range': 'bytes=3-b'}
+        headers = {'Range': '3-b'}
         req = Request(testing.create_environ(headers=headers))
         self.assertRaises(falcon.HTTPBadRequest, lambda: req.range)
 
-        headers = {'Range': 'bytes=x-y'}
+        headers = {'Range': 'x-y'}
         expected_desc = ('The value provided for the Range header is '
                          'invalid. It must be a byte range formatted '
                          'according to RFC 2616.')
@@ -463,14 +463,6 @@ class TestReqVars(testing.TestBase):
                                  falcon.HTTPInvalidHeader,
                                  'Invalid header value', expected_desc)
 
-        headers = {'Range': '10-'}
-        expected_desc = ("The value provided for the Range "
-                         "header is invalid. The value must be "
-                         "prefixed with 'bytes='")
-        self._test_error_details(headers, 'range',
-                                 falcon.HTTPInvalidHeader,
-                                 'Invalid header value', expected_desc)
-
     def test_missing_attribute_header(self):
         req = Request(testing.create_environ())
         self.assertEqual(req.range, None)
@@ -505,28 +497,47 @@ class TestReqVars(testing.TestBase):
                                  falcon.HTTPInvalidHeader,
                                  'Invalid header value', expected_desc)
 
-    def test_date(self):
+    @ddt.data(('Date', 'date'),
+              ('If-Modified-since', 'if_modified_since'),
+              ('If-Unmodified-since', 'if_unmodified_since'),
+              )
+    @ddt.unpack
+    def test_date(self, header, attr):
         date = datetime.datetime(2013, 4, 4, 5, 19, 18)
-        headers = {'date': 'Thu, 04 Apr 2013 05:19:18 GMT'}
-        req = Request(testing.create_environ(headers=headers))
-        self.assertEqual(req.date, date)
+        date_str = 'Thu, 04 Apr 2013 05:19:18 GMT'
+
+        self._test_header_expected_value(header, date_str, attr, date)
 
-    @ddt.data('Thu, 04 Apr 2013', '')
-    def test_date_invalid(self, http_date):
-        headers = {'date': http_date}
-        expected_desc = ('The value provided for the Date '
+    @ddt.data(('Date', 'date'),
+              ('If-Modified-Since', 'if_modified_since'),
+              ('If-Unmodified-Since', 'if_unmodified_since'),
+              )
+    @ddt.unpack
+    def test_date_invalid(self, header, attr):
+
+        # Date formats don't conform to RFC 1123
+        headers = {header: 'Thu, 04 Apr 2013'}
+        expected_desc = ('The value provided for the {0} '
                          'header is invalid. It must be formatted '
                          'according to RFC 1123.')
-        self._test_error_details(headers, 'date',
+
+        self._test_error_details(headers, attr,
                                  falcon.HTTPInvalidHeader,
-                                 'Invalid header value', expected_desc)
+                                 'Invalid header value',
+                                 expected_desc.format(header))
+
+        headers = {header: ''}
+        self._test_error_details(headers, attr,
+                                 falcon.HTTPInvalidHeader,
+                                 'Invalid header value',
+                                 expected_desc.format(header))
 
-    def test_date_missing(self):
+    @ddt.data('date', 'if_modified_since', 'if_unmodified_since')
+    def test_date_missing(self, attr):
         req = Request(testing.create_environ())
-        self.assertIs(req.date, None)
+        self.assertIs(getattr(req, attr), None)
 
     def test_attribute_headers(self):
-        date = testing.httpnow()
         hash = 'fa0d1a60ef6616bb28038515c8ea4cb2'
         auth = 'HMAC_SHA1 c590afa9bb59191ffab30f223791e82d3fd3e3af'
         agent = 'testing/1.0.1'
@@ -542,12 +553,8 @@ class TestReqVars(testing.TestBase):
         self._test_attribute_header('Expect', '100-continue', 'expect')
 
         self._test_attribute_header('If-Match', hash, 'if_match')
-        self._test_attribute_header('If-Modified-Since', date,
-                                    'if_modified_since')
         self._test_attribute_header('If-None-Match', hash, 'if_none_match')
         self._test_attribute_header('If-Range', hash, 'if_range')
-        self._test_attribute_header('If-Unmodified-Since', date,
-                                    'if_unmodified_since')
 
         self._test_attribute_header('User-Agent', agent, 'user_agent',
                                     default=default_agent)
@@ -580,6 +587,11 @@ class TestReqVars(testing.TestBase):
         req = Request(testing.create_environ())
         self.assertEqual(getattr(req, attr), default)
 
+    def _test_header_expected_value(self, name, value, attr, expected_value):
+        headers = {name: value}
+        req = Request(testing.create_environ(headers=headers))
+        self.assertEqual(getattr(req, attr), expected_value)
+
     def _test_error_details(self, headers, attr_name,
                             error_type, title, description):
         req = Request(testing.create_environ(headers=headers))