From d285fe7f0d79f7221eb6f25af807e9a439c22d66 Mon Sep 17 00:00:00 2001 From: ederag Date: Sat, 28 Dec 2019 18:56:59 +0100 Subject: [PATCH 1/3] check fact before inclusion Not only from the gui, but from the command line as well. --- src/hamster-cli.py | 1 + src/hamster/edit_activity.py | 40 +++++------------------------------ src/hamster/lib/fact.py | 41 ++++++++++++++++++++++++++++++++++++ 3 files changed, 47 insertions(+), 35 deletions(-) diff --git a/src/hamster-cli.py b/src/hamster-cli.py index ee7fda557..9dde0025c 100644 --- a/src/hamster-cli.py +++ b/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): diff --git a/src/hamster/edit_activity.py b/src/hamster/edit_activity.py index cff16dd5c..fa299cc22 100644 --- a/src/hamster/edit_activity.py +++ b/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: + fact.check(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("""\ - Working late ? - 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 diff --git a/src/hamster/lib/fact.py b/src/hamster/lib/fact.py index 3e7b7abbe..527499b56 100644 --- a/src/hamster/lib/fact.py +++ b/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, @@ -70,6 +75,42 @@ def category(self): def category(self, value): self._category = value.strip() if value else "" + def check(self, default_day=None): + """Check Fact validity for inclusion in the storage.""" + if self.start_time is None: + raise FactError("Missing start time") + + if self.end_time and (self.delta < dt.timedelta(0)): + fixed_fact = Fact(start_time=self.start_time, + end_time=self.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 self.activity: + raise FactError("Missing activity") + + if ',' in self.category: + raise FactError(dedent( + """\ + Forbidden comma in category: '{}' + Note: The description separator changed + from single comma to double comma ',,' (cf. PR #482). + """.format(self.category) + )) + def copy(self, **kwds): """Return an independent copy, with overrides as keyword arguments. From 98fb9b6453c215ed096d27ffe0c1a81f0c296973 Mon Sep 17 00:00:00 2001 From: ederag Date: Sun, 29 Dec 2019 18:01:27 +0100 Subject: [PATCH 2/3] move fact_check to storage itself --- src/hamster-service.py | 19 ++++++++++++++- src/hamster/client.py | 29 +++++++++++++++++++++-- src/hamster/edit_activity.py | 2 +- src/hamster/lib/dbus.py | 15 +++++++++++- src/hamster/lib/fact.py | 36 ----------------------------- src/hamster/storage/storage.py | 42 ++++++++++++++++++++++++++++++++-- 6 files changed, 100 insertions(+), 43 deletions(-) diff --git a/src/hamster-service.py b/src/hamster-service.py index c7d4c5e30..e303fb76b 100644 --- a/src/hamster-service.py +++ b/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) diff --git a/src/hamster/client.py b/src/hamster/client.py index 50e52ec27..66258552d 100644 --- a/src/hamster/client.py +++ b/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" diff --git a/src/hamster/edit_activity.py b/src/hamster/edit_activity.py index fa299cc22..83565ab14 100644 --- a/src/hamster/edit_activity.py +++ b/src/hamster/edit_activity.py @@ -345,7 +345,7 @@ def validate_fields(self): fact.end_time or default_dt) try: - fact.check(default_day=self.date) + runtime.storage.check_fact(fact, default_day=self.date) except FactError as error: self.update_status(status="wrong", markup=str(error)) return None diff --git a/src/hamster/lib/dbus.py b/src/hamster/lib/dbus.py index eaa4c57e2..87a3851e7 100644 --- a/src/hamster/lib/dbus.py +++ b/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) diff --git a/src/hamster/lib/fact.py b/src/hamster/lib/fact.py index 527499b56..817492c64 100644 --- a/src/hamster/lib/fact.py +++ b/src/hamster/lib/fact.py @@ -75,42 +75,6 @@ def category(self): def category(self, value): self._category = value.strip() if value else "" - def check(self, default_day=None): - """Check Fact validity for inclusion in the storage.""" - if self.start_time is None: - raise FactError("Missing start time") - - if self.end_time and (self.delta < dt.timedelta(0)): - fixed_fact = Fact(start_time=self.start_time, - end_time=self.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 self.activity: - raise FactError("Missing activity") - - if ',' in self.category: - raise FactError(dedent( - """\ - Forbidden comma in category: '{}' - Note: The description separator changed - from single comma to double comma ',,' (cf. PR #482). - """.format(self.category) - )) - def copy(self, **kwds): """Return an independent copy, with overrides as keyword arguments. diff --git a/src/hamster/storage/storage.py b/src/hamster/storage/storage.py index fad2f5f87..1cbe93bd8 100644 --- a/src/hamster/storage/storage.py +++ b/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,46 @@ def dispatch_overwrite(self): self.facts_changed() self.activities_changed() - # facts + def check_fact(self, 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. From 3df234dcedb3860f862cba6cbe4c38fa9721097a Mon Sep 17 00:00:00 2001 From: ederag Date: Mon, 30 Dec 2019 20:10:25 +0100 Subject: [PATCH 3/3] replace validate_fact with check_fact And check before transactions. --- src/hamster/storage/db.py | 3 +-- src/hamster/storage/storage.py | 12 ++++++------ 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/src/hamster/storage/db.py b/src/hamster/storage/db.py index b2a875dcd..52638c21f 100644 --- a/src/hamster/storage/db.py +++ b/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 diff --git a/src/hamster/storage/storage.py b/src/hamster/storage/storage.py index 1cbe93bd8..15042aa60 100644 --- a/src/hamster/storage/storage.py +++ b/src/hamster/storage/storage.py @@ -40,7 +40,8 @@ def dispatch_overwrite(self): self.activities_changed() # facts - def check_fact(self, fact, default_day=None): + @classmethod + def check_fact(cls, fact, default_day=None): """Check Fact validity for inclusion in the storage. Raise FactError(message) on failure. @@ -97,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() @@ -110,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. @@ -124,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()