check fact before inclusion

src/hamster-cli.py

@@ -149,6 +149,7 @@ def start(self, *args):
         fact = Fact.parse(" ".join(args), range_pos="tail")
         if fact.start_time is None:
             fact.start_time = stuff.hamster_now()
+        fact.check(default_day=stuff.hamster_today())
         self.storage.add_fact(fact)
 
     def stop(self, *args):

src/hamster-service.py

@@ -17,10 +17,11 @@
     DBusMainLoop,
     dbus,
     fact_signature,
+    from_dbus_date,
     from_dbus_fact,
     to_dbus_fact
 )
-from hamster.lib.fact import Fact
+from hamster.lib.fact import Fact, FactError
 
 logger = default_logger(__file__)
 
@@ -173,6 +174,22 @@ def AddFactVerbatim(self, dbus_fact):
         return self.add_fact(fact) or 0
 
 
+    @dbus.service.method("org.gnome.Hamster",
+                         in_signature="{}i".format(fact_signature),
+                         out_signature='bs')
+    def CheckFact(self, dbus_fact, dbus_default_day):
+        fact = from_dbus_fact(dbus_fact)
+        dd = from_dbus_date(dbus_default_day)
+        try:
+            self.check_fact(fact, default_day=dd)
+            success = True
+            message = ""
+        except FactError as error:
+            success = False
+            message = str(error)
+        return success, message
+
+
     @dbus.service.method("org.gnome.Hamster",
                          in_signature='i',
                          out_signature=fact_signature)

src/hamster/client.py

@@ -24,8 +24,14 @@
 
 from calendar import timegm
 from gi.repository import GObject as gobject
-from hamster.lib.dbus import DBusMainLoop, dbus, from_dbus_fact, to_dbus_fact
-from hamster.lib.fact import Fact
+from hamster.lib.dbus import (
+    DBusMainLoop,
+    dbus,
+    from_dbus_fact,
+    to_dbus_date,
+    to_dbus_fact,
+    )
+from hamster.lib.fact import Fact, FactError
 from hamster.lib.stuff import hamster_now
 
 
@@ -148,6 +154,25 @@ def get_fact(self, id):
         """returns fact by it's ID"""
         return from_dbus_fact(self.conn.GetFact(id))
 
+    def check_fact(self, fact, default_day=None):
+        """Check Fact validity for inclusion in the storage.
+
+        default_day (date): Default hamster day,
+                            used to simplify some hint messages
+                            (remove unnecessary dates).
+                            None is safe (always show dates).
+        """
+        if not fact.start_time:
+            # Do not even try to pass fact through D-Bus as
+            # conversions would fail in this case.
+            raise FactError("Missing start time")
+        dbus_fact = to_dbus_fact(fact)
+        dbus_day = to_dbus_date(default_day)
+        success, message = self.conn.CheckFact(dbus_fact, dbus_day)
+        if not success:
+            raise FactError(message)
+        return success, message
+
     def add_fact(self, fact, temporary_activity = False):
         """Add fact (Fact)."""
         assert fact.activity, "missing activity"

src/hamster/edit_activity.py

@@ -29,7 +29,7 @@
 """
 from hamster import widgets
 from hamster.lib.configuration import runtime, conf, load_ui_file
-from hamster.lib.fact import Fact
+from hamster.lib.fact import Fact, FactError
 from hamster.lib.stuff import (
     hamsterday_time_to_datetime, hamster_today, hamster_now, escape_pango)
 
@@ -344,47 +344,17 @@ def validate_fields(self):
         self.draw_preview(fact.start_time or default_dt,
                           fact.end_time or default_dt)
 
-        if fact.start_time is None:
-            self.update_status(status="wrong", markup="Missing start time")
+        try:
+            runtime.storage.check_fact(fact, default_day=self.date)
+        except FactError as error:
+            self.update_status(status="wrong", markup=str(error))
             return None
 
-        if not fact.activity:
-            self.update_status(status="wrong", markup="Missing activity")
-            return None
-
-        if (fact.delta < dt.timedelta(0)) and fact.end_time:
-            fact.end_time += dt.timedelta(days=1)
-            markup = dedent("""\
-                            <b>Working late ?</b>
-                            Duration would be negative.
-                            This happens when the activity crosses the
-                            hamster day start time ({:%H:%M} from tracking settings).
-
-                            Changing the end time date to the next day.
-                            Pressing the button would save
-                            an actvity going from
-                            {}
-                            to
-                            {}
-                            (in civil local time)
-                            """.format(conf.day_start, fact.start_time, fact.end_time))
-            self.update_status(status="warning", markup=markup)
-            return fact
-
         roundtrip_fact = Fact.parse(fact.serialized(), default_day=self.date)
         if roundtrip_fact != fact:
             self.update_status(status="wrong", markup="Fact could not be parsed back")
             return None
 
-        if ',' in fact.category:
-            markup = dedent("""\
-                            Commas ',' are forbidden in category.
-                            Note: the description separator changed
-                            from single comma to double comma (',,') in v3.0.
-                            """)
-            self.update_status(status="wrong", markup=markup)
-            return None
-
         # nothing unusual
         self.update_status(status="looks good", markup="")
         return fact

src/hamster/lib/dbus.py

@@ -8,6 +8,19 @@
 
 """D-Bus communication utilities."""
 
+# dates
+
+def from_dbus_date(dbus_date):
+    """Convert D-Bus timestamp (seconds since epoch) to date."""
+    return dt.date.fromtimestamp(dbus_date) if dbus_date else None
+
+
+def to_dbus_date(date):
+    """Convert date to D-Bus timestamp (seconds since epoch)."""
+    return timegm(date.timetuple()) if date else 0
+
+
+# facts
 
 """
 dbus_fact signature (types matching the to_dbus_fact output)
@@ -51,5 +64,5 @@ def to_dbus_fact(fact):
             fact.activity_id or 0,
             fact.category or '',
             dbus.Array(fact.tags, signature = 's'),
-            timegm(fact.date.timetuple()),
+            to_dbus_date(fact.date),
             fact.delta.days * 24 * 60 * 60 + fact.delta.seconds)

src/hamster/lib/fact.py

@@ -2,8 +2,10 @@
 logger = logging.getLogger(__name__)   # noqa: E402
 
 import calendar
+import datetime as dt
 
 from copy import deepcopy
+from textwrap import dedent
 
 from hamster.lib.parsing import TIME_FMT, DATETIME_FMT, parse_fact
 from hamster.lib.stuff import (
@@ -13,6 +15,9 @@
     hamster_today,
 )
 
+class FactError(Exception):
+    """Generic Fact error."""
+
 
 class Fact(object):
     def __init__(self, activity="", category=None, description=None, tags=None,

src/hamster/storage/db.py

@@ -517,6 +517,7 @@ def __solve_overlaps(self, start_time, end_time):
                                 description=fact["description"],
                                 start_time=end_time,
                                 end_time=fact_end_time)
+                storage.Storage.check_fact(new_fact)
                 new_fact_id = self.__add_fact(new_fact)
 
                 # copy tags
@@ -554,8 +555,6 @@ def __add_fact(self, fact, temporary=False):
         """
         logger.info("adding fact {}".format(fact))
 
-        self.validate_fact(fact)  # sanity check
-
         start_time = fact.start_time
         end_time = fact.end_time
 

src/hamster/storage/storage.py

@@ -22,7 +22,7 @@
 logger = logging.getLogger(__name__)   # noqa: E402
 
 import datetime as dt
-from hamster.lib.fact import Fact
+from hamster.lib.fact import Fact, FactError
 from hamster.lib.stuff import hamster_now
 
 class Storage(object):
@@ -39,8 +39,47 @@ def dispatch_overwrite(self):
         self.facts_changed()
         self.activities_changed()
 
-
     # facts
+    @classmethod
+    def check_fact(cls, fact, default_day=None):
+        """Check Fact validity for inclusion in the storage.
+
+        Raise FactError(message) on failure.
+        """
+        if fact.start_time is None:
+            raise FactError("Missing start time")
+
+        if fact.end_time and (fact.delta < dt.timedelta(0)):
+            fixed_fact = Fact(start_time=fact.start_time,
+                              end_time=fact.end_time + dt.timedelta(days=1))
+            suggested_range_str = fixed_fact.serialized_range(default_day=default_day)
+            # work around cyclic imports
+            from hamster.lib.configuration import conf
+            raise FactError(dedent(
+                """\
+                Duration would be negative.
+                Working late ?
+                This happens when the activity crosses the
+                hamster day start time ({:%H:%M} from tracking settings).
+
+                Suggestion: move the end to the next day; the range would become:
+                {}
+                (in civil local time)
+                """.format(conf.day_start, suggested_range_str)
+                ))
+
+        if not fact.activity:
+            raise FactError("Missing activity")
+
+        if ',' in fact.category:
+            raise FactError(dedent(
+                """\
+                Forbidden comma in category: '{}'
+                Note: The description separator changed
+                      from single comma to double comma ',,' (cf. PR #482).
+                """.format(fact.category)
+                ))
+
     def add_fact(self, fact, start_time=None, end_time=None, temporary=False):
         """Add fact.
 
@@ -59,6 +98,8 @@ def add_fact(self, fact, start_time=None, end_time=None, temporary=False):
             fact.start_time = start_time
             fact.end_time = end_time
 
+        # better fail before opening the transaction
+        self.check_fact(fact)
         self.start_transaction()
         result = self.__add_fact(fact, temporary)
         self.end_transaction()
@@ -72,6 +113,8 @@ def get_fact(self, fact_id):
         return self.__get_fact(fact_id)
 
     def update_fact(self, fact_id, fact, start_time=None, end_time=None, temporary=False):
+        # better fail before opening the transaction
+        self.check_fact(fact)
         self.start_transaction()
         self.__remove_fact(fact_id)
         # to be removed once update facts use Fact directly.
@@ -86,11 +129,6 @@ def update_fact(self, fact_id, fact, start_time=None, end_time=None, temporary=F
             self.facts_changed()
         return result
 
-    def validate_fact(self, fact):
-        """Check fact validity for inclusion into storage."""
-        assert fact.activity, "missing activity"
-        assert fact.start_time, "missing start_time"
-
     def stop_tracking(self, end_time):
         """Stops tracking the current activity"""
         facts = self.__get_todays_facts()