Repository: tonioo/sievelib

Issue:
Better support of relational and date extensions
The current implementation is buggy.

Discussion:

Patch:
diff --git a/README.rst b/README.rst
index 68ff377..e278c60 100644
--- a/README.rst
+++ b/README.rst
@@ -38,12 +38,13 @@ The following extensions are also supported:
 
 * Copying Without Side Effects (`RFC 3894 <https://tools.ietf.org/html/rfc3894>`_)
 * Body (`RFC 5173 <https://tools.ietf.org/html/rfc5173>`_)
-* Date and Index (`RFC 5260 <https://tools.ietf.org/html/rfc5260>`_)
 * Vacation (`RFC 5230 <http://tools.ietf.org/html/rfc5230>`_)
+* Relational (`RFC 5231 <https://tools.ietf.org/html/rfc5231>`_)
 * Imap4flags (`RFC 5232 <https://tools.ietf.org/html/rfc5232>`_)
 
 The following extensions are partially supported:
 
+* Date and Index (`RFC 5260 <https://tools.ietf.org/html/rfc5260>`_)
 * Checking Mailbox Status and Accessing Mailbox Metadata (`RFC 5490 <https://tools.ietf.org/html/rfc5490>`_)
 
 Extending the parser
diff --git a/sievelib/commands.py b/sievelib/commands.py
index 7f6df2e..d26bcb5 100644
--- a/sievelib/commands.py
+++ b/sievelib/commands.py
@@ -98,10 +98,21 @@ address_part = {"name": "address-part",
                 "values": [":localpart", ":domain", ":all"],
                 "type": ["tag"],
                 "required": False}
-match_type = {"name": "match-type",
-              "values": [":is", ":contains", ":matches"],
-              "type": ["tag"],
-              "required": False}
+match_type = {
+    "name": "match-type",
+    "values": [":is", ":contains", ":matches"],
+    "extension_values": {
+        ":count": "relational",
+        ":value": "relational"
+    },
+    "extra_arg": {
+        "type": "string",
+        "values": ['"gt"', '"ge"', '"lt"', '"le"', '"eq"', '"ne"'],
+        "valid_for": [":count", ":value"]
+    },
+    "type": ["tag"],
+    "required": False
+}
 
 
 class Command(object):
@@ -343,9 +354,17 @@ class Command(object):
         :param value: the value to check
         :return: True on succes, False otherwise
         """
-        if "values" not in arg:
+        if "values" not in arg and "extension_values" not in arg:
+            return True
+        if "values" in arg and value.lower() in arg["values"]:
             return True
-        return value.lower() in arg["values"]
+        if "extension_values" in arg:
+            extension = arg["extension_values"].get(value.lower())
+            if extension:
+                if extension not in RequireCommand.loaded_extensions:
+                    raise ExtensionNotLoaded(extension)
+                return True
+        return False
 
     def __is_valid_type(self, typ, typlist):
         """ Check if type is valid based on input type list
@@ -431,7 +450,6 @@ class Command(object):
 
             condition = (
                 atype in curarg["type"] and
-                ("values" not in curarg or avalue in curarg["values"]) and
                 self.__is_valid_value_for_arg(curarg, avalue)
             )
             if condition:
@@ -892,6 +910,59 @@ class HasflagCommand(TestCommand):
             self.rargs_cnt = 1
 
 
+class DateCommand(TestCommand):
+    """date command, part of the date extension.
+
+    https://tools.ietf.org/html/rfc5260#section-4
+    """
+
+    extension = "date"
+    args_definition = [
+        {"name": "zone",
+         "type": ["tag"],
+         "write_tag": True,
+         "values": [":zone", ":originalzone"],
+         "extra_arg": {"type": "string", "valid_for": [":zone"]},
+         "required": False},
+        comparator,
+        match_type,
+        {"name": "header-name",
+         "type": ["string"],
+         "required": True},
+        {"name": "date-part",
+         "type": ["string"],
+         "required": True},
+        {"name": "key-list",
+         "type": ["string", "stringlist"],
+         "required": True}
+    ]
+
+
+class CurrentdateCommand(TestCommand):
+    """currentdate command, part of the date extension.
+
+    http://tools.ietf.org/html/rfc5260#section-5
+    """
+
+    extension = "date"
+    args_definition = [
+        {"name": "zone",
+         "type": ["tag"],
+         "write_tag": True,
+         "values": [":zone"],
+         "extra_arg": {"type": "string"},
+         "required": False},
+        comparator,
+        match_type,
+        {"name": "date-part",
+         "type": ["string"],
+         "required": True},
+        {"name": "key-list",
+         "type": ["string", "stringlist"],
+         "required": True}
+    ]
+
+
 class VacationCommand(ActionCommand):
     args_definition = [
         {"name": "subject",
@@ -937,7 +1008,7 @@ class VacationCommand(ActionCommand):
 
 class SetCommand(ControlCommand):
 
-    """currentdate command, part of the variables extension
+    """set command, part of the variables extension
 
     http://tools.ietf.org/html/rfc5229
     """
@@ -953,37 +1024,6 @@ class SetCommand(ControlCommand):
     ]
 
 
-class CurrentdateCommand(ControlCommand):
-
-    """currentdate command, part of the date extension
-
-    http://tools.ietf.org/html/rfc5260#section-5
-    """
-
-    extension = "date"
-    accept_children = True
-    args_definition = [
-        {"name": "zone",
-         "type": ["tag"],
-         "write_tag": True,
-         "values": [":zone"],
-         "extra_arg": {"type": "string"},
-         "required": False},
-        {"name": "match-value",
-         "type": ["tag"],
-         "required": True},
-        {"name": "comparison",
-         "type": ["string"],
-         "required": True},
-        {"name": "match-against",
-         "type": ["string"],
-         "required": True},
-        {"name": "match-against-field",
-         "type": ["string"],
-         "required": True}
-    ]
-
-
 def add_commands(cmds):
     """
     Adds one or more commands to the module namespace.


Test patch:
diff --git a/sievelib/tests/test_parser.py b/sievelib/tests/test_parser.py
index 65e6393..430cdf1 100644
--- a/sievelib/tests/test_parser.py
+++ b/sievelib/tests/test_parser.py
@@ -779,10 +779,20 @@ if exists ["subject"]
 
 
 class DateCommands(SieveTest):
+
+    def test_date_command(self):
+        self.compilation_ok(b"""require ["date", "relational", "fileinto"];
+if allof(header :is "from" "boss@example.com",
+         date :value "ge" :originalzone "date" "hour" "09",
+         date :value "lt" :originalzone "date" "hour" "17")
+{ fileinto "urgent"; }
+""")
+
     def test_currentdate_command(self):
         self.compilation_ok(b"""require ["date", "relational"];
 
-if allof ( currentdate :value "ge" "date" "2013-10-23" , currentdate :value "le" "date" "2014-10-12" ) 
+if allof(currentdate :value "ge" "date" "2013-10-23",
+         currentdate :value "le" "date" "2014-10-12")
 {
     discard;
 }
@@ -791,7 +801,8 @@ if allof ( currentdate :value "ge" "date" "2013-10-23" , currentdate :value "le"
     def test_currentdate_command_timezone(self):
         self.compilation_ok(b"""require ["date", "relational"];
 
-if allof ( currentdate :zone "+0100" :value "ge" "date" "2013-10-23" , currentdate :value "le" "date" "2014-10-12" ) 
+if allof(currentdate :zone "+0100" :value "ge" "date" "2013-10-23",
+         currentdate :value "le" "date" "2014-10-12")
 {
     discard;
 }
@@ -800,13 +811,22 @@ if allof ( currentdate :zone "+0100" :value "ge" "date" "2013-10-23" , currentda
     def test_currentdate_norel(self):
         self.compilation_ok(b"""require ["date"];
 
-if allof ( 
-  currentdate :zone "+0100" :is "date" "2013-10-23"  
-) 
+if allof (
+  currentdate :zone "+0100" :is "date" "2013-10-23"
+)
 {
     discard;
 }""")
 
+    def test_currentdate_extension_not_loaded(self):
+        self.compilation_ko(b"""require ["date"];
+
+if allof ( currentdate :value "ge" "date" "2013-10-23" , currentdate :value "le" "date" "2014-10-12" )
+{
+    discard;
+}
+""")
+
 
 class VariablesCommands(SieveTest):
     def test_set_command(self):