Repository: kapouille/mongoquery

Issue:
Checks against strings don't work properly
See the following case:

```
>>> import mongoquery
>>> a = {"a": "5", "b": 6}
>>> query = mongoquery.Query({"a": 2})
>>> query.match(a)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/local/lib/python2.7/site-packages/mongoquery/__init__.py", line 23, in match
    return self._match(self._definition, entry)
  File "/usr/local/lib/python2.7/site-packages/mongoquery/__init__.py", line 29, in _match
    for sub_operator, sub_condition in condition.items()
  File "/usr/local/lib/python2.7/site-packages/mongoquery/__init__.py", line 29, in <genexpr>
    for sub_operator, sub_condition in condition.items()
  File "/usr/local/lib/python2.7/site-packages/mongoquery/__init__.py", line 72, in _process_condition
    return self._match(condition, extracted_data)
  File "/usr/local/lib/python2.7/site-packages/mongoquery/__init__.py", line 33, in _match
    return condition in entry
TypeError: 'in <string>' requires string as left operand, not int
```

This happens because we have:
```
    def _match(self, condition, entry):
        if isinstance(condition, Mapping):
            return all(
                self._process_condition(sub_operator, sub_condition, entry)
                for sub_operator, sub_condition in condition.items()
            )
        else:
            if isinstance(entry, Sequence): <----- HERE
                return condition in entry
            else:
                return condition == entry
```

`str` is actually a subtype of `collections.Sequence`, and thus we try and do an "in" comparison, when we really want equality. This seems like a bug even if you had two strings, because this can happen:

```
>>> b = {"a": "567", "b": 6}
>>> query2 = mongoquery.Query({"a": "6"})
>>> query2.match(b)
True
```

Same reason - we're doing an `in` when you want to do a strict equality check. There are a couple of possible fixes:

1. Check against explicit types instead of `Sequence`
2. Ensure that it is not a string-like type (e.g. bytes, unicode, str, etc)

Discussion:
foolswood: The Travis failure is python 3.2 only (and resolved by https://www.python.org/dev/peps/pep-0414/).
There are a couple of options I can see to sort this out:
- Remove (or find another way to create) the unicode literal in the tests.
- Stop supporting python 3.2.
rajcze: > The Travis failure is python 3.2 only (and resolved by https://www.python.org/dev/peps/pep-0414/).
> There are a couple of options I can see to sort this out...

My bad for only testing on 2.7 and 3.5... It's not really my call, but I'd rather remove the unicode literal from the test suite. Even though we could go with `unicode()`, and something like:
```lang=python
try:
  unicode()
except NameError:
  unicode = lambda x: str(x)
```

It seems too nasty to just facilitate for what IMHO is quite a corner-case issue.

foolswood: I don't know many things using python3.2 (I don't have it locally either) so I don't blame you at all.

I was leaning towards the literal removal too.
Thought I'd put some options out, see if there was something I was missing.

Patch:
diff --git a/mongoquery/__init__.py b/mongoquery/__init__.py
index 9fa9482..83dc2ff 100644
--- a/mongoquery/__init__.py
+++ b/mongoquery/__init__.py
@@ -15,6 +15,10 @@ class _Undefined(object):
     pass
 
 
+def is_non_string_sequence(entry):
+    return isinstance(entry, Sequence) and not isinstance(entry, string_type)
+
+
 class Query(object):
     def __init__(self, definition):
         self._definition = definition
@@ -29,7 +33,7 @@ class Query(object):
                 for sub_operator, sub_condition in condition.items()
             )
         else:
-            if isinstance(entry, Sequence):
+            if is_non_string_sequence(entry):
                 return condition in entry
             else:
                 return condition == entry
@@ -39,7 +43,7 @@ class Query(object):
             return entry
         if entry is None:
             return entry
-        if isinstance(entry, Sequence) and not isinstance(entry, string_type):
+        if is_non_string_sequence(entry):
             try:
                 index = int(path[0])
                 return self._extract(entry[index], path[1:])
@@ -277,7 +281,7 @@ class Query(object):
                 )
             )
 
-        if isinstance(entry, Sequence) and not isinstance(entry, string_type):
+        if is_non_string_sequence(entry):
             return len(entry) == condition
 
         return False


Test patch:
diff --git a/tests/test_query.py b/tests/test_query.py
index 27d44e9..22ef7c8 100644
--- a/tests/test_query.py
+++ b/tests/test_query.py
@@ -103,6 +103,11 @@ class TestQuery(TestCase):
             self._query({"qty": {"$type": 'number'}})
         )
 
+        self.assertEqual(
+            _ALL,
+            self._query({"item": {"$type": 'string'}})
+        )
+
         self.assertEqual(
             [],
             self._query({"qty": {"$type": 'string'}})
@@ -385,3 +390,8 @@ class TestQuery(TestCase):
         collection = [{"turtles": "swim"}]
         self.assertEqual(
             [], self._query({"turtles": {"value": "swim"}}, collection))
+
+    def test_query_string(self):
+        collection = [{"a": "5"}, {"a": "567"}]
+        self.assertEqual([], self._query({"a": 5}, collection))
+        self.assertEqual([{"a": "5"}], self._query({"a": "5"}, collection))