Repository: Materials-Consortia/optimade-python-tools (python)

Issue:
NOT filter operation of mongo query for complex expressions
This implementation of  [NOT operation](https://docs.mongodb.com/manual/reference/operator/query/not/) fails in the case when it has been applied for a "complex" expression. Here are some example for the inputs and outputs of a `Transformer`:
```
"NOT a>1" -> {'a': {'$not': {'$gt': 1}}}
"NOT a>1 AND b>1" -> {'$and': [{'a': {'$not': {'$gt': 1}}}, {'b': {'$gt': 1}}]}
"NOT (a>1 AND b>1)" -> {'$and': {'$not': [{'a': {'$gt': 1}}, {'b': {'$gt': 1}}]}}
```
In the case of the last one the transformer gives you a rubish query.

Pull request:
Handle arbitrary nested NOT/AND/OR in queries
This PR allows us to recursively parse nested queries, but does NOT fix the problem described in #79 (yet...). Essentially the `expression_phrase` function fails to go deep enough through the tree and ends up flattening out the query at the first AND/OR, which breaks any negations. 

An example of the tests failing against master can be found [here](https://github.com/Materials-Consortia/optimade-python-tools/pull/221/checks?check_run_id=498692689#step:6:154), which amounts to `NOT (expr1 AND expr2)` becoming `NOT expr1 and expr2` instead of `NOT expr1 AND NOT expr2`. This PR fixes that by extending the negation to the second expression in this example, and hopefully to arbitrary expressions.

The discussion in #79 amounts to the distinction between the above and `NOT (expr1 AND expr2)` being tricky to handle in Mongo as it stands, which instead would need something like `(expr1 AND NOT expr2) OR (NOT expr1 AND expr2)`, which should probably be handle with care in another PR.

Patch:
diff --git a/optimade/filtertransformers/mongo.py b/optimade/filtertransformers/mongo.py
index a3f6c446..ac6ee6b0 100755
--- a/optimade/filtertransformers/mongo.py
+++ b/optimade/filtertransformers/mongo.py
@@ -88,16 +88,7 @@ class MongoTransformer(Transformer):
 
     def expression_phrase(self, arg):
         # expression_phrase: [ NOT ] ( comparison | "(" expression ")" )
-        if len(arg) == 1:
-            # without NOT
-            return arg[0]
-
-        if list(arg[1].keys()) == ["$or"]:
-            return {"$nor": arg[1]["$or"]}
-
-        # with NOT
-        # TODO: This implementation probably fails in the case of `"(" expression ")"`
-        return {prop: {"$not": expr} for prop, expr in arg[1].items()}
+        return self._recursive_expression_phrase(arg)
 
     @v_args(inline=True)
     def comparison(self, value):
@@ -208,3 +199,30 @@ class MongoTransformer(Transformer):
         raise NotImplementedError(
             f"Calling __default__, i.e., unknown grammar concept. data: {data}, children: {children}, meta: {meta}"
         )
+
+    def _recursive_expression_phrase(self, arg):
+        """ Helper function for parsing `expression_phrase`. Recursively sorts out
+        the correct precedence for $and, $or and $nor.
+
+        """
+        if len(arg) == 1:
+            # without NOT
+            return arg[0]
+
+        # handle the case of {"$not": {"$or": [expr1, expr2]}} using {"$nor": [expr1, expr2]}.
+        if "$or" in arg[1]:
+            return {"$nor": self._recursive_expression_phrase([arg[1]["$or"]])}
+
+        # handle the case of {"$not": {"$and": [expr1, expr2]}} using per-expression negation,
+        # e.g. {"$and": [{prop1: {"$not": expr1}}, {prop2: {"$not": ~expr2}}]}.
+        # Note that this is not the same as NOT (expr1 AND expr2)!
+        if "$and" in arg[1]:
+            return {
+                "$and": [
+                    self._recursive_expression_phrase(["NOT", subdict])
+                    for subdict in arg[1]["$and"]
+                ]
+            }
+
+        # simple case of negating one expression, from NOT (expr) to ~expr.
+        return {prop: {"$not": expr} for prop, expr in arg[1].items()}


Test patch:
diff --git a/tests/filtertransformers/test_mongo.py b/tests/filtertransformers/test_mongo.py
index 77ff633e..019dba22 100644
--- a/tests/filtertransformers/test_mongo.py
+++ b/tests/filtertransformers/test_mongo.py
@@ -226,6 +226,63 @@ class TestMongoTransformer(unittest.TestCase):
         # OPTIONAL
         # self.assertEqual(self.transform("((NOT (_exmpl_a>_exmpl_b)) AND _exmpl_x>0)"), {})
 
+        self.assertEqual(
+            self.transform("NOT (a>1 AND b>1)"),
+            {"$and": [{"a": {"$not": {"$gt": 1}}}, {"b": {"$not": {"$gt": 1}}}]},
+        )
+
+        self.assertEqual(
+            self.transform("NOT (a>1 AND b>1 OR c>1)"),
+            {
+                "$nor": [
+                    {"$and": [{"a": {"$gt": 1}}, {"b": {"$gt": 1}}]},
+                    {"c": {"$gt": 1}},
+                ]
+            },
+        )
+
+        self.assertEqual(
+            self.transform("NOT (a>1 AND ( b>1 OR c>1 ))"),
+            {
+                "$and": [
+                    {"a": {"$not": {"$gt": 1}}},
+                    {"$nor": [{"b": {"$gt": 1}}, {"c": {"$gt": 1}}]},
+                ]
+            },
+        )
+
+        self.assertEqual(
+            self.transform("NOT (a>1 AND ( b>1 OR (c>1 AND d>1 ) ))"),
+            {
+                "$and": [
+                    {"a": {"$not": {"$gt": 1}}},
+                    {
+                        "$nor": [
+                            {"b": {"$gt": 1}},
+                            {"$and": [{"c": {"$gt": 1}}, {"d": {"$gt": 1}}]},
+                        ]
+                    },
+                ]
+            },
+        )
+
+        self.assertEqual(
+            self.transform(
+                'elements HAS "Ag" AND NOT ( elements HAS "Ir" AND elements HAS "Ac" )'
+            ),
+            {
+                "$and": [
+                    {"elements": {"$in": ["Ag"]}},
+                    {
+                        "$and": [
+                            {"elements": {"$not": {"$in": ["Ir"]}}},
+                            {"elements": {"$not": {"$in": ["Ac"]}}},
+                        ]
+                    },
+                ]
+            },
+        )
+
         self.assertEqual(self.transform("5 < 7"), {7: {"$gt": 5}})
 
         with self.assertRaises(VisitError):