Merge pull request #294 from dhermes/query-operator-fix-dict-random-order

Makes operator checking deterministic in Query.filter.

Author: dhermes

gcloud/datastore/query.py

@@ -46,10 +46,10 @@ class Query(object):
     """
 
     OPERATORS = {
-        '<': datastore_pb.PropertyFilter.LESS_THAN,
         '<=': datastore_pb.PropertyFilter.LESS_THAN_OR_EQUAL,
-        '>': datastore_pb.PropertyFilter.GREATER_THAN,
         '>=': datastore_pb.PropertyFilter.GREATER_THAN_OR_EQUAL,
+        '<': datastore_pb.PropertyFilter.LESS_THAN,
+        '>': datastore_pb.PropertyFilter.GREATER_THAN,
         '=': datastore_pb.PropertyFilter.EQUAL,
     }
     """Mapping of operator strings and their protobuf equivalents."""
@@ -132,12 +132,16 @@ def filter(self, expression, value):
         property_name, operator = None, None
         expression = expression.strip()
 
-        for operator_string in self.OPERATORS:
-            if expression.endswith(operator_string):
-                operator = self.OPERATORS[operator_string]
-                property_name = expression[0:-len(operator_string)].strip()
+        # Use None to split on *any* whitespace.
+        expr_pieces = expression.rsplit(None, 1)
+        if len(expr_pieces) == 2:
+            property_name, operator = expr_pieces
+            property_name = property_name.strip()
 
-        if not operator or not property_name:
+        # If no whitespace in `expression`, `operator` will be `None` and
+        # self.OPERATORS[None] will be `None` as well.
+        pb_op_enum = self.OPERATORS.get(operator)
+        if pb_op_enum is None:
             raise ValueError('Invalid expression: "%s"' % expression)
 
         # Build a composite filter AND'd together.
@@ -147,7 +151,7 @@ def filter(self, expression, value):
         # Add the specific filter
         property_filter = composite_filter.filter.add().property_filter
         property_filter.property.name = property_name
-        property_filter.operator = operator
+        property_filter.operator = pb_op_enum
 
         # Set the value to filter on based on the type.
         helpers._set_protobuf_value(property_filter.value, value)

gcloud/datastore/test_query.py

@@ -63,21 +63,57 @@ def test_to_protobuf_w_kind(self):
         kq_pb, = list(q_pb.kind)
         self.assertEqual(kq_pb.name, _KIND)
 
+    def test_filter_w_no_operator(self):
+        query = self._makeOne()
+        self.assertRaises(ValueError, query.filter, 'firstname', 'John')
+
     def test_filter_w_unknown_operator(self):
         query = self._makeOne()
         self.assertRaises(ValueError, query.filter, 'firstname ~~', 'John')
 
     def test_filter_w_known_operator(self):
+        from gcloud.datastore import datastore_v1_pb2 as datastore_pb
+
         query = self._makeOne()
         after = query.filter('firstname =', u'John')
         self.assertFalse(after is query)
         self.assertTrue(isinstance(after, self._getTargetClass()))
         q_pb = after.to_protobuf()
-        self.assertEqual(q_pb.filter.composite_filter.operator, 1)  # AND
+        self.assertEqual(q_pb.filter.composite_filter.operator,
+                         datastore_pb.CompositeFilter.AND)
         f_pb, = list(q_pb.filter.composite_filter.filter)
         p_pb = f_pb.property_filter
         self.assertEqual(p_pb.property.name, 'firstname')
         self.assertEqual(p_pb.value.string_value, u'John')
+        self.assertEqual(p_pb.operator, datastore_pb.PropertyFilter.EQUAL)
+
+    def test_filter_w_all_operators(self):
+        from gcloud.datastore import datastore_v1_pb2 as datastore_pb
+
+        query = self._makeOne()
+        query = query.filter('leq_prop <=', u'val1')
+        query = query.filter('geq_prop >=', u'val2')
+        query = query.filter('lt_prop <', u'val3')
+        query = query.filter('gt_prop >', u'val4')
+        query = query.filter('eq_prop =', u'val5')
+
+        query_pb = query.to_protobuf()
+        pb_values = [
+            ('leq_prop', 'val1',
+             datastore_pb.PropertyFilter.LESS_THAN_OR_EQUAL),
+            ('geq_prop', 'val2',
+             datastore_pb.PropertyFilter.GREATER_THAN_OR_EQUAL),
+            ('lt_prop', 'val3', datastore_pb.PropertyFilter.LESS_THAN),
+            ('gt_prop', 'val4', datastore_pb.PropertyFilter.GREATER_THAN),
+            ('eq_prop', 'val5', datastore_pb.PropertyFilter.EQUAL),
+        ]
+        query_filter = query_pb.filter.composite_filter.filter
+        for filter_pb, pb_value in zip(query_filter, pb_values):
+            name, val, filter_enum = pb_value
+            prop_filter = filter_pb.property_filter
+            self.assertEqual(prop_filter.property.name, name)
+            self.assertEqual(prop_filter.value.string_value, val)
+            self.assertEqual(prop_filter.operator, filter_enum)
 
     def test_filter_w_known_operator_and_entity(self):
         import operator