From 250f431fcd9d27aa8cf6e42faa54a7f87395e2d7 Mon Sep 17 00:00:00 2001 From: Jacob Lifshay Date: Mon, 7 Sep 2020 18:04:22 -0700 Subject: [PATCH] add code for parsing new cf_payees_list format --- src/budget_sync/budget_graph.py | 175 ++++++++++++++++--- src/budget_sync/test/test_budget_graph.py | 201 +++++++++++++++++++--- 2 files changed, 324 insertions(+), 52 deletions(-) diff --git a/src/budget_sync/budget_graph.py b/src/budget_sync/budget_graph.py index 1ebcd50..3b58127 100644 --- a/src/budget_sync/budget_graph.py +++ b/src/budget_sync/budget_graph.py @@ -1,12 +1,14 @@ from bugzilla.bug import Bug from bugzilla import Bugzilla -from typing import Set, Dict, Iterable, Optional, List +from typing import Set, Dict, Iterable, Optional, List, Union, Any from budget_sync.util import all_bugs from budget_sync.money import Money from functools import cached_property import toml import sys +import enum from collections import deque +from datetime import date, time, datetime class BudgetGraphBaseError(Exception): @@ -49,6 +51,140 @@ class _NodeSimpleReprWrapper: return self.node.bug.id < other.node.bug.id +class PayeeState(enum.Enum): + NotYetSubmitted = "not yet submitted" + Submitted = "submitted" + Paid = "paid" + + +_Date = Union[date, datetime] + + +def _parse_money_from_toml(value: Any) -> Money: + if not isinstance(value, (int, str)): + msg = f"monetary amount is not a string or integer " \ + f"(to use fractional amounts such as 123.45, write " \ + f"\"123.45\"): {value!r}" + raise ValueError(msg) + return Money(value) + + +def _parse_date_time_or_none_from_toml(value: Any) -> Optional[_Date]: + if value is None or isinstance(value, (date, datetime)): + return value + elif isinstance(value, time): + msg = f"just a time of day by itself is not enough," \ + f" a date must be included: {str(value)}" + raise ValueError(msg) + elif isinstance(value, bool): + msg = f"invalid date: {str(value).lower()}" + raise ValueError(msg) + elif isinstance(value, (str, int, float)): + msg = f"invalid date: {value!r}" + raise ValueError(msg) + else: + msg = f"invalid date" + raise ValueError(msg) + + +class Payment: + def __init__(self, + node: "Node", + payee_key: str, + amount: Money, + paid: Optional[_Date], + submitted: Optional[_Date]): + self.node = node + self.payee_key = payee_key + self.amount = amount + self.paid = paid + self.submitted = submitted + + @property + def state(self): + if self.paid is not None: + return PayeeState.Paid + if self.submitted is not None: + return PayeeState.Submitted + return PayeeState.NotYetSubmitted + + @staticmethod + def from_toml(node: "Node", payee_key: str, toml_value: Any): + paid = None + submitted = None + known_keys = ("paid", "submitted", "amount") + if isinstance(toml_value, dict): + try: + amount = toml_value['amount'] + except KeyError: + msg = f"value for key {payee_key!r} is missing the " \ + f"`amount` field which is required" + raise BudgetGraphPayeesParseError(node.bug.id, msg) \ + .with_traceback(sys.exc_info()[2]) + for k, v in toml_value.items(): + if k in ("paid", "submitted"): + try: + parsed_value = _parse_date_time_or_none_from_toml(v) + except ValueError as e: + msg = f"failed to parse `{k}` field for" \ + f" key {payee_key!r}: {e}" + raise BudgetGraphPayeesParseError( + node.bug.id, msg) \ + .with_traceback(sys.exc_info()[2]) + if k == "paid": + paid = parsed_value + else: + assert k == "submitted" + submitted = parsed_value + if k not in known_keys: + msg = f"value for key {payee_key!r} has an unknown" \ + f" field: `{k}`" + raise BudgetGraphPayeesParseError(node.bug.id, msg) \ + .with_traceback(sys.exc_info()[2]) + try: + paid = _parse_date_time_or_none_from_toml( + toml_value.get('paid')) + except ValueError as e: + msg = f"failed to parse `paid` field for" \ + f" key {payee_key!r}: {e}" + raise BudgetGraphPayeesParseError( + node.bug.id, msg) \ + .with_traceback(sys.exc_info()[2]) + try: + submitted = _parse_date_time_or_none_from_toml( + toml_value.get('submitted')) + except ValueError as e: + msg = f"failed to parse `submitted` field for" \ + f" key {payee_key!r}: {e}" + raise BudgetGraphPayeesParseError( + node.bug.id, msg) \ + .with_traceback(sys.exc_info()[2]) + elif isinstance(toml_value, (int, str, float)): + # float included for better error messages + amount = toml_value + else: + msg = f"value for key {payee_key!r} is invalid -- it should " \ + f"either be a monetary value or a table" + raise BudgetGraphPayeesParseError(node.bug.id, msg) + try: + amount = _parse_money_from_toml(amount) + except ValueError as e: + msg = f"failed to parse monetary amount for key {payee_key!r}: {e}" + raise BudgetGraphPayeesParseError( + node.bug.id, msg) \ + .with_traceback(sys.exc_info()[2]) + return Payment(node=node, payee_key=payee_key, amount=amount, + paid=paid, submitted=submitted) + + def __repr__(self): + return (f"Payment(node={_NodeSimpleReprWrapper(self.node)}, " + f"payee_key={self.payee_key!r}, " + f"amount={self.amount}, " + f"state={self.state.name}, " + f"paid={str(self.paid)}, " + f"submitted={str(self.submitted)})") + + class Node: graph: "BudgetGraph" bug: Bug @@ -74,7 +210,7 @@ class Node: self.nlnet_milestone = None @cached_property - def payees(self) -> Dict[str, Money]: + def payments(self) -> Dict[str, Payment]: try: parsed = toml.loads(self.bug.cf_payees_list) except toml.TomlDecodeError as e: @@ -86,19 +222,7 @@ class Node: if not isinstance(key, str): raise BudgetGraphPayeesParseError( self.bug.id, f"key is not a string: {key!r}") - if not isinstance(value, (int, str)): - msg = f"value for key {key!r} is not a string or integer " \ - f"(to use fractional values such as 123.45, write " \ - f"\"123.45\"): {value!r}" - raise BudgetGraphPayeesParseError(self.bug.id, msg) - try: - money_value = Money(value) - except ValueError as e: - msg = f"failed to parse Money value for key {key!r}: {e}" - raise BudgetGraphPayeesParseError( - self.bug.id, msg) \ - .with_traceback(sys.exc_info()[2]) - retval[key] = money_value + retval[key] = Payment.from_toml(self, key, value) return retval @property @@ -167,6 +291,7 @@ class Node: immediate_children.append(_NodeSimpleReprWrapper(i)) immediate_children.sort() parent = f"#{self.parent_id}" if self.parent_id is not None else None + payments = list(self.payments.values()) return (f"Node(graph=..., " f"id={_NodeSimpleReprWrapper(self)}, " f"root={root}, " @@ -177,7 +302,7 @@ class Node: f"fixed_budget_including_subtasks={self.fixed_budget_including_subtasks}, " f"nlnet_milestone={self.nlnet_milestone!r}, " f"immediate_children={immediate_children!r}, " - f"payees={self.payees!r}") + f"payments={payments!r}") class BudgetGraphError(BudgetGraphBaseError): @@ -301,11 +426,11 @@ class BudgetGraph: subtasks_total += child.fixed_budget_including_subtasks payees_total = Money(0) - for payee_key, payee_value in node.payees.items(): - if payee_value < 0: + for payment in node.payments.values(): + if payment.amount < 0: errors.append(BudgetGraphNegativePayeeMoney( - node.bug.id, root.bug.id, payee_key)) - payees_total += payee_value + node.bug.id, root.bug.id, payment.payee_key)) + payees_total += payment.amount def set_including_from_excluding_and_error(): node.fixed_budget_including_subtasks = \ @@ -367,7 +492,7 @@ class BudgetGraph: # can't have 2 match without all 3 matching assert not payees_matches_excluding assert not including_matches_excluding - if node.budget_including_subtasks == 0 and len(node.payees) == 0: + if node.budget_including_subtasks == 0 and len(node.payments) == 0: set_including_from_excluding_and_error() else: set_excluding_from_including_and_error() @@ -375,7 +500,7 @@ class BudgetGraph: # can't have 2 match without all 3 matching assert not payees_matches_including assert not including_matches_excluding - if node.budget_excluding_subtasks == 0 and len(node.payees) == 0: + if node.budget_excluding_subtasks == 0 and len(node.payments) == 0: if node.budget_including_subtasks == 0: set_including_from_excluding_and_error() else: @@ -386,7 +511,7 @@ class BudgetGraph: # can't have 2 match without all 3 matching assert not payees_matches_including assert not payees_matches_excluding - if len(node.payees) == 0: + if len(node.payments) == 0: pass # no error -- payees is just not set elif node.budget_excluding_subtasks == 0 \ and node.budget_including_subtasks == 0: @@ -396,7 +521,7 @@ class BudgetGraph: set_payees_from_excluding_and_error() else: # nothing matches - if len(node.payees) == 0: + if len(node.payments) == 0: # payees unset -- don't need to set payees if node.budget_including_subtasks == 0: set_including_from_excluding_and_error() @@ -440,6 +565,6 @@ class BudgetGraph: def payee_keys(self) -> Set[str]: retval = set() for node in self.nodes.values(): - for payee_key in node.payees.keys(): + for payee_key in node.payments.keys(): retval.add(payee_key) return retval diff --git a/src/budget_sync/test/test_budget_graph.py b/src/budget_sync/test/test_budget_graph.py index 16c107d..885a20e 100644 --- a/src/budget_sync/test/test_budget_graph.py +++ b/src/budget_sync/test/test_budget_graph.py @@ -133,7 +133,7 @@ class TestBudgetGraph(unittest.TestCase): self.assertEqual(node.budget_excluding_subtasks, Money(cents=0)) self.assertEqual(node.budget_including_subtasks, Money(cents=0)) self.assertIsNone(node.nlnet_milestone) - self.assertEqual(node.payees, {}) + self.assertEqual(node.payments, {}) def test_loop1(self): with self.assertRaises(BudgetGraphLoopError) as cm: @@ -166,7 +166,7 @@ class TestBudgetGraph(unittest.TestCase): self.assertEqual(node1.nlnet_milestone, "abc") self.assertEqual(list(node1.children()), [node2]) self.assertEqual(list(node1.children_breadth_first()), [node2]) - self.assertEqual(node1.payees, {}) + self.assertEqual(node1.payments, {}) self.assertIsInstance(node2, Node) self.assertIs(node2.graph, bg) self.assertIs(node2.bug, EXAMPLE_CHILD_BUG2) @@ -176,7 +176,7 @@ class TestBudgetGraph(unittest.TestCase): self.assertEqual(node2.budget_excluding_subtasks, Money(cents=1000)) self.assertEqual(node2.budget_including_subtasks, Money(cents=1000)) self.assertEqual(node2.nlnet_milestone, "abc") - self.assertEqual(node2.payees, {}) + self.assertEqual(node2.payments, {}) def test_children(self): bg = BudgetGraph([ @@ -652,7 +652,7 @@ class TestBudgetGraph(unittest.TestCase): self.assertEqual(errors[0].root_bug_id, 1) def test_payees_parse(self): - def check(cf_payees_list, expected_payees): + def check(cf_payees_list, expected_payments): bg = BudgetGraph([MockBug(bug_id=1, cf_budget_parent=None, cf_budget="0", @@ -662,43 +662,137 @@ class TestBudgetGraph(unittest.TestCase): ]) self.assertEqual(len(bg.nodes), 1) node: Node = bg.nodes[1] - self.assertEqual(node.payees, expected_payees) + self.assertEqual([str(i) for i in node.payments.values()], + expected_payments) check(""" abc = 123 """, - {"abc": Money(123)}) + ["Payment(node=#1, payee_key='abc', amount=123, " + + "state=NotYetSubmitted, paid=None, submitted=None)"]) check(""" abc = "123" """, - {"abc": Money(123)}) + ["Payment(node=#1, payee_key='abc', amount=123, " + + "state=NotYetSubmitted, paid=None, submitted=None)"]) check(""" abc = "123.45" """, - {"abc": Money("123.45")}) + ["Payment(node=#1, payee_key='abc', amount=123.45, " + + "state=NotYetSubmitted, paid=None, submitted=None)"]) check(""" abc = "123.45" "d e f" = "21.35" """, - { - "abc": Money("123.45"), - "d e f": Money("21.35"), - }) + ["Payment(node=#1, payee_key='abc', amount=123.45, " + + "state=NotYetSubmitted, paid=None, submitted=None)", + "Payment(node=#1, payee_key='d e f', amount=21.35, " + + "state=NotYetSubmitted, paid=None, submitted=None)"]) check(""" abc = "123.45" # my comments "AAA" = "-21.35" """, - { - "abc": Money("123.45"), - "AAA": Money("-21.35"), - }) + ["Payment(node=#1, payee_key='abc', amount=123.45, " + + "state=NotYetSubmitted, paid=None, submitted=None)", + "Payment(node=#1, payee_key='AAA', amount=-21.35, " + + "state=NotYetSubmitted, paid=None, submitted=None)"]) check(""" "not-an-email@example.com" = "-2345" """, - { - "not-an-email@example.com": Money(-2345), - }) + ["Payment(node=#1, payee_key='not-an-email@example.com', " + + "amount=-2345, state=NotYetSubmitted, paid=None, " + + "submitted=None)"]) + check(""" + payee = { amount = 123 } + """, + ["Payment(node=#1, payee_key='payee', " + + "amount=123, state=NotYetSubmitted, paid=None, " + + "submitted=None)"]) + check(""" + payee = { amount = 123, submitted = 2020-05-01 } + """, + ["Payment(node=#1, payee_key='payee', " + + "amount=123, state=Submitted, paid=None, " + + "submitted=2020-05-01)"]) + check(""" + payee = { amount = 123, submitted = 2020-05-01T00:00:00 } + """, + ["Payment(node=#1, payee_key='payee', " + + "amount=123, state=Submitted, paid=None, " + + "submitted=2020-05-01 00:00:00)"]) + check(""" + payee = { amount = 123, submitted = 2020-05-01T00:00:00Z } + """, + ["Payment(node=#1, payee_key='payee', " + + "amount=123, state=Submitted, paid=None, " + + "submitted=2020-05-01 00:00:00+00:00)"]) + check(""" + payee = { amount = 123, submitted = 2020-05-01T00:00:00-07:23 } + """, + ["Payment(node=#1, payee_key='payee', " + + "amount=123, state=Submitted, paid=None, " + + "submitted=2020-05-01 00:00:00-07:23)"]) + check(""" + payee = { amount = 123, paid = 2020-05-01 } + """, + ["Payment(node=#1, payee_key='payee', " + + "amount=123, state=Paid, paid=2020-05-01, " + + "submitted=None)"]) + check(""" + payee = { amount = 123, paid = 2020-05-01T00:00:00 } + """, + ["Payment(node=#1, payee_key='payee', " + + "amount=123, state=Paid, paid=2020-05-01 00:00:00, " + + "submitted=None)"]) + check(""" + payee = { amount = 123, paid = 2020-05-01T00:00:00Z } + """, + ["Payment(node=#1, payee_key='payee', " + + "amount=123, state=Paid, paid=2020-05-01 00:00:00+00:00, " + + "submitted=None)"]) + check(""" + payee = { amount = 123, paid = 2020-05-01T00:00:00-07:23 } + """, + ["Payment(node=#1, payee_key='payee', " + + "amount=123, state=Paid, paid=2020-05-01 00:00:00-07:23, " + + "submitted=None)"]) + check(""" + [payee] + amount = 123 + submitted = 2020-05-23 + paid = 2020-05-01 + """, + ["Payment(node=#1, payee_key='payee', " + + "amount=123, state=Paid, paid=2020-05-01, " + + "submitted=2020-05-23)"]) + check(""" + [payee] + amount = 123 + submitted = 2020-05-23 + paid = 2020-05-01T00:00:00 + """, + ["Payment(node=#1, payee_key='payee', " + + "amount=123, state=Paid, paid=2020-05-01 00:00:00, " + + "submitted=2020-05-23)"]) + check(""" + [payee] + amount = 123 + submitted = 2020-05-23 + paid = 2020-05-01T00:00:00Z + """, + ["Payment(node=#1, payee_key='payee', " + + "amount=123, state=Paid, paid=2020-05-01 00:00:00+00:00, " + + "submitted=2020-05-23)"]) + check(""" + [payee] + amount = 123 + submitted = 2020-05-23 + paid = 2020-05-01T00:00:00-07:23 + """, + ["Payment(node=#1, payee_key='payee', " + + "amount=123, state=Paid, paid=2020-05-01 00:00:00-07:23, " + + "submitted=2020-05-23)"]) def test_payees_money_mismatch(self): bg = BudgetGraph([ @@ -732,16 +826,15 @@ class TestBudgetGraph(unittest.TestCase): self.assertEqual(errors[0].msg, expected_msg) check_parse_error(""" - "payee 1" = {} + "payee 1" = [] """, - "value for key 'payee 1' is not a string or integer " - "(to use fractional values such as 123.45, write " - "\"123.45\"): {}") + "value for key 'payee 1' is invalid -- it should " + "either be a monetary value or a table") check_parse_error(""" payee = "ashjkf" """, - "failed to parse Money value for key 'payee': " + "failed to parse monetary amount for key 'payee': " "invalid Money string: characters after sign and " "before first `.` must be ascii digits") @@ -755,9 +848,63 @@ class TestBudgetGraph(unittest.TestCase): check_parse_error(""" payee = 123.45 """, - "value for key 'payee' is not a string or " - "integer (to use fractional values such as " - "123.45, write \"123.45\"): 123.45") + "failed to parse monetary amount for key 'payee': " + "monetary amount is not a string or integer (to " + "use fractional amounts such as 123.45, write " + "\"123.45\"): 123.45") + + check_parse_error(""" + payee = {} + """, + "value for key 'payee' is missing the `amount` " + "field which is required") + + check_parse_error(""" + payee = { amount = 123.45 } + """, + "failed to parse monetary amount for key 'payee': " + "monetary amount is not a string or integer (to " + "use fractional amounts such as 123.45, write " + "\"123.45\"): 123.45") + + check_parse_error(""" + payee = { amount = 123, blah = false } + """, + "value for key 'payee' has an unknown field: `blah`") + + check_parse_error(""" + payee = { amount = 123, submitted = false } + """, + "failed to parse `submitted` field for key " + "'payee': invalid date: false") + + check_parse_error(""" + payee = { amount = 123, submitted = 123 } + """, + "failed to parse `submitted` field for key 'payee':" + " invalid date: 123") + + check_parse_error( + """ + payee = { amount = 123, paid = 2020-01-01, submitted = "abc" } + """, + "failed to parse `submitted` field for key 'payee': " + "invalid date: 'abc'") + + check_parse_error( + """ + payee = { amount = 123, paid = 12:34:56 } + """, + "failed to parse `paid` field for key 'payee': just a time of " + "day by itself is not enough, a date must be included: 12:34:56") + + check_parse_error( + """ + payee = { amount = 123, submitted = 12:34:56.123456 } + """, + "failed to parse `submitted` field for key 'payee': just a time " + "of day by itself is not enough, a date must be included: " + "12:34:56.123456") def test_negative_payee_money(self): bg = BudgetGraph([ -- 2.30.2