Fix case where multiple indexes with similar name seperated by _ …

…were interpreted as options. (#79)

* Fix case where multiple indexes with similar name seperated by ``_`` were interpreted as options.
Fixes #78.

* fix ambiguity

* raise ValueError again, when operator is not found ... this is intended behaviour

* add tests for ambiguity fix

* catch possible unpack error

Co-authored-by: Michael Howitz <[email protected]>
Co-authored-by: Peter Mathis <[email protected]>
Co-authored-by: agitator <[email protected]>

CHANGES.rst

@@ -47,6 +47,10 @@ Fixes
 - Fix case where index value is changed to None after previously being indexed.
   (`#100 <https://github.com/zopefoundation/Products.ZCatalog/issues/100>`_)
 
+- Fix case where multiple indexes with similar name seperated by ``_`` were interpreted as options.
+  Fixes #78.
+  [thet]
+
 
 5.1 (2020-04-20)
 ----------------

src/Products/ZCatalog/query.py

@@ -88,8 +88,7 @@ def __init__(self, request, iid, options=(), operators=('or', 'and'),
 
             for field in request.keys():
                 if field.startswith(iid + '_'):
-                    iid_tmp, op = field.split('_')
-
+                    op = field[len(iid) + 1:]
                     self.set(op, request[field])
 
         self.keys = keys

src/Products/ZCatalog/tests/test_query.py

@@ -69,32 +69,43 @@ def test_get_not_int(self):
 
     def test_operator_dict(self):
         request = {'path': {'query': 'foo', 'operator': 'bar'}}
-        self.assertRaises(ValueError,
-                          self._makeOne,
-                          request,
-                          'path',
-                          ('query', 'operator'),
-                          ('or', 'and'))
+        with self.assertRaises(ValueError):
+            self._makeOne(
+                request, 'path', ('query', 'operator'), ('or', 'and'))
 
     def test_operator_string(self):
         request = {'path': 'foo', 'path_operator': 'bar'}
-        self.assertRaises(ValueError,
-                          self._makeOne,
-                          request, 'path',
-                          ('query', 'operator'),
-                          ('or', 'and'))
+        with self.assertRaises(ValueError):
+            self._makeOne(
+                request, 'path', ('query', 'operator'), ('or', 'and'))
 
     def test_options_dict(self):
         request = {'path': {'query': 'foo', 'baropt': 'dummy'}}
-        self.assertRaises(ValueError,
-                          self._makeOne,
-                          request,
-                          'path',
-                          ('query', 'operator'))
+        with self.assertRaises(ValueError):
+            self._makeOne(
+                request, 'path', ('query', 'operator'))
 
     def test_options_string(self):
         request = {'path': 'foo', 'path_baropt': 'dummy'}
-        self.assertRaises(ValueError,
-                          self._makeOne,
-                          request, 'path',
-                          ('query', 'operator'))
+        with self.assertRaises(ValueError):
+            self._makeOne(
+                request, 'path', ('query', 'operator'))
+
+    def test_get_string_with_underscores(self):
+        request = {
+            'extra_path': 'foo',
+            'extra_path_level': 0,
+            'extra_path_operator': 'and',
+        }
+        parser = self._makeOne(
+            request, 'extra_path', ('query', 'level', 'operator'))
+
+        self.assertEqual(parser.get('keys'), ['foo'])
+        self.assertEqual(parser.get('level'), 0)
+        self.assertEqual(parser.get('operator'), 'and')
+
+    def test_operator_string_with_underscores(self):
+        request = {'extra_path': 'foo', 'extra_path_operator': 'bar'}
+        with self.assertRaises(ValueError):
+            self._makeOne(
+                request, 'extra_path', ('query', 'operator'), ('or', 'and'))