Require start / end at if doing a period query

This is the requirement and what the error message is claiming but not
what the validation is checking. Spotlight always performs queries with
a time range so this should not cause a problem.

backdrop/read/validation.py

@@ -72,8 +72,8 @@ def validate(self, request_args, context):
 
 class PeriodQueryValidator(Validator):
     def validate(self, request_args, context):
-        if 'start_at' in request_args or 'end_at' in request_args:
-            if not ('start_at' in request_args and 'end_at' in request_args):
+        if 'period' in request_args and 'delta' not in request_args:
+            if 'start_at' not in request_args or 'end_at' not in request_args:
                 self.add_error("both 'start_at' and 'end_at' are required "
                                "for a period query")
 

tests/read/test_read_api_query_endpoint.py

@@ -26,14 +26,6 @@ class QueryingApiTestCase(unittest.TestCase):
     def setUp(self):
         self.app = api.app.test_client()
 
-    @stub_bucket_retrieve_by_name("foo")
-    @patch('backdrop.core.bucket.Bucket.query')
-    def test_period_query_is_executed(self, mock_query):
-        mock_query.return_value = NoneData()
-        self.app.get('/foo?period=week')
-        mock_query.assert_called_with(
-            Query.create(period=WEEK))
-
     @stub_bucket_retrieve_by_name("foo", raw_queries_allowed=True)
     @patch('backdrop.core.bucket.Bucket.query')
     def test_filter_by_query_is_executed(self, mock_query):
@@ -65,16 +57,6 @@ def test_query_with_start_and_end_is_executed(self, mock_query):
         mock_query.assert_called_with(
             Query.create(start_at=expected_start_at, end_at=expected_end_at))
 
-    @stub_bucket_retrieve_by_name("foo")
-    @patch('backdrop.core.bucket.Bucket.query')
-    def test_group_by_with_period_is_executed(self, mock_query):
-        mock_query.return_value = NoneData()
-        self.app.get(
-            '/foo?period=week&group_by=stuff'
-        )
-        mock_query.assert_called_with(
-            Query.create(period=WEEK, group_by="stuff"))
-
     @stub_bucket_retrieve_by_name("foo", raw_queries_allowed=True)
     @patch('backdrop.core.bucket.Bucket.query')
     def test_sort_query_is_executed(self, mock_query):

tests/read/test_read_api_service_data_endpoint.py

@@ -8,7 +8,7 @@
 from backdrop.read import api
 from backdrop.read.query import Query
 from tests.support.bucket import setup_bucket
-from tests.support.test_helpers import has_status, has_header
+from tests.support.test_helpers import has_status, has_header, d_tz
 
 
 class NoneData(object):
@@ -24,9 +24,15 @@ def setUp(self):
     @patch('backdrop.core.bucket.Bucket.query')
     def test_period_query_is_executed(self, mock_query):
         mock_query.return_value = NoneData()
-        self.app.get('/data/some-group/some-type?period=week')
+
+        self.app.get(
+            '/data/some-group/some-type?period=week&' +
+            'start_at=' + urllib.quote("2012-11-05T00:00:00Z") + '&' +
+            'end_at=' + urllib.quote("2012-12-03T00:00:00Z"))
         mock_query.assert_called_with(
-            Query.create(period=WEEK))
+            Query.create(period=WEEK,
+                         start_at=d_tz(2012, 11, 5),
+                         end_at=d_tz(2012, 12, 3)))
 
     @setup_bucket("foo", data_group="some-group", data_type="some-type", raw_queries_allowed=True)
     @patch('backdrop.core.bucket.Bucket.query')
@@ -64,11 +70,17 @@ def test_query_with_start_and_end_is_executed(self, mock_query):
     @patch('backdrop.core.bucket.Bucket.query')
     def test_group_by_with_period_is_executed(self, mock_query):
         mock_query.return_value = NoneData()
+
         self.app.get(
-            '/data/some-group/some-type?period=week&group_by=stuff'
-        )
+            '/data/some-group/some-type?period=week&group_by=stuff&' +
+            'start_at=' + urllib.quote("2012-11-05T00:00:00Z") + '&' +
+            'end_at=' + urllib.quote("2012-12-03T00:00:00Z"))
+
         mock_query.assert_called_with(
-            Query.create(period=WEEK, group_by="stuff"))
+            Query.create(period=WEEK,
+                         group_by="stuff",
+                         start_at=d_tz(2012, 11, 5),
+                         end_at=d_tz(2012, 12, 3)))
 
     @setup_bucket("foo", data_group="some-group", data_type="some-type", raw_queries_allowed=True)
     @patch('backdrop.core.bucket.Bucket.query')
@@ -122,14 +134,14 @@ def test_cors_requests_can_cache_control(self):
         response = self.app.open('/data/some-group/some-type', method='OPTIONS')
         assert_that(response, has_header('Access-Control-Allow-Headers', 'cache-control'))
 
-    @setup_bucket("bucket", data_group="some-group", data_type="some-type")
+    @setup_bucket("bucket", data_group="some-group", data_type="some-type", raw_queries_allowed=True)
     def test_max_age_is_30_min_for_non_realtime_buckets(self):
-        response = self.app.get('/data/some-group/some-type?period=week')
+        response = self.app.get('/data/some-group/some-type')
 
         assert_that(response, has_header('Cache-Control', 'max-age=1800, must-revalidate'))
 
-    @setup_bucket("bucket", data_group="some-group", data_type="some-type", realtime=True)
+    @setup_bucket("bucket", data_group="some-group", data_type="some-type", realtime=True, raw_queries_allowed=True)
     def test_max_age_is_2_min_for_realtime_buckets(self):
-        response = self.app.get('/data/some-group/some-type?period=week')
+        response = self.app.get('/data/some-group/some-type')
 
         assert_that(response, has_header('Cache-Control', 'max-age=120, must-revalidate'))

tests/read/test_validation.py

@@ -151,7 +151,9 @@ def test_queries_with_a_negative_limit_value_are_disallowed(self):
     def test_queries_with_sort_by_and_period_are_disallowed(self):
         validation_result = validate_request_args({
             "sort_by": "foo:ascending",
-            "period": "week"
+            "period": "week",
+            "start_at": "2012-11-05T00:00:00Z",
+            "end_at": "2012-12-03T00:00:00Z",
         })
         assert_that( validation_result, is_invalid_with_message(
             "Cannot sort for period queries without group_by. "
@@ -161,7 +163,9 @@ def test_queries_with_sort_by_and_period_and_group_by_are_allowed(self):
         validation_result = validate_request_args({
             "sort_by": "foo:ascending",
             "period": "week",
-            "group_by": "foo"
+            "group_by": "foo",
+            "start_at": "2012-11-12T00:00:00+00:00",
+            "end_at": "2012-12-12T00:00:00+00:00",
         })
         assert_that( validation_result, is_valid() )
 
@@ -255,7 +259,9 @@ def test_queries_grouping_and_collecting_equal_fields_are_disallowed(self):
     def test_queries_grouping_week_start_at_with_period_are_disallowed(self):
         validation_result = validate_request_args({
             'period': 'week',
-            'group_by': '_week_start_at'
+            'group_by': '_week_start_at',
+            'start_at': '2012-11-05T00:00:00Z',
+            'end_at': '2012-12-03T00:00:00Z',
         })
 
         assert_that(validation_result, is_invalid_with_message(
@@ -264,7 +270,9 @@ def test_queries_grouping_week_start_at_with_period_are_disallowed(self):
 
     def test_queries_with_period_values_must_be_certain_values(self):
         validation_result = validate_request_args({
-            'period': 'fortnight'
+            'period': 'fortnight',
+            'start_at': '2012-11-12T00:00:00+00:00',
+            'end_at':  '2012-12-12T00:00:00+00:00',
         })
 
         assert_that(validation_result, is_invalid_with_message(
@@ -324,9 +332,9 @@ def test_that_collect_queries_with_invalid_method_are_disallowed(self):
             'collect': 'field:infinity',
         })
 
-        assert_that(validation_result, is_invalid_with_message((
+        assert_that(validation_result, is_invalid_with_message(
             "Unknown collection method"
-        )))
+        ))
 
     def test_period_with_just_positive_delta(self):
         validation_result = validate_request_args({

tests/read/test_validation_of_queries_accessing_raw_data.py

@@ -16,7 +16,11 @@ def test_that_grouped_queries_are_allowed(self):
         assert_that(validation_result, is_valid())
 
     def test_that_periodic_queries_are_allowed(self):
-        validation_result = validate_request_args({'period': 'week'})
+        validation_result = validate_request_args({
+            'period': 'day',
+            'start_at': '2012-11-01T00:00:00Z',
+            'end_at': '2012-12-01T00:00:00Z',
+        })
         assert_that(validation_result, is_valid())
 
     def test_that_querying_for_less_than_7_days_of_data_is_disallowed(self):