From 6543d105a6c255ebd28153257f4ddf4c74c5d23d Mon Sep 17 00:00:00 2001 From: Jacob Lifshay Date: Thu, 27 Aug 2020 17:01:28 -0700 Subject: [PATCH] start adding cf_payees_list handling --- setup.py | 1 + src/budget_sync/budget_graph.py | 113 +++++++++- src/budget_sync/test/mock_bug.py | 7 +- src/budget_sync/test/test_budget_graph.py | 240 ++++++++++++++++++++-- 4 files changed, 332 insertions(+), 29 deletions(-) diff --git a/setup.py b/setup.py index b863c1f..72ed1ac 100644 --- a/setup.py +++ b/setup.py @@ -2,6 +2,7 @@ from setuptools import setup, find_packages install_requires = [ "python-bugzilla", + "toml", ] setup( diff --git a/src/budget_sync/budget_graph.py b/src/budget_sync/budget_graph.py index 0ff5648..cb77a22 100644 --- a/src/budget_sync/budget_graph.py +++ b/src/budget_sync/budget_graph.py @@ -4,12 +4,28 @@ from typing import Set, Dict, Iterable, Optional, List from budget_sync.util import all_bugs from budget_sync.money import Money from functools import cached_property +import toml +import sys class BudgetGraphBaseError(Exception): pass +class BudgetGraphParseError(BudgetGraphBaseError): + def __init__(self, bug_id: int): + self.bug_id = bug_id + + +class BudgetGraphPayeesParseError(BudgetGraphParseError): + def __init__(self, bug_id: int, msg: str): + super().__init__(bug_id) + self.msg = msg + + def __str__(self): + return f"Failed to parse cf_payees_list field of bug #{self.bug_id}: {self.msg}" + + class BudgetGraphLoopError(BudgetGraphBaseError): def __init__(self, bug_ids: List[int]): self.bug_ids = bug_ids @@ -52,6 +68,34 @@ class Node: if self.nlnet_milestone == "---": self.nlnet_milestone = None + @cached_property + def payees(self) -> Dict[str, Money]: + try: + parsed = toml.loads(self.bug.cf_payees_list) + except toml.TomlDecodeError as e: + new_err = BudgetGraphPayeesParseError( + self.bug.id, f"TOML parse error: {e}") + raise new_err.with_traceback(sys.exc_info()[2]) + retval = {} + for key, value in parsed.items(): + 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 + return retval + @property def parent(self) -> Optional["Node"]: if self.parent_id is not None: @@ -115,11 +159,12 @@ class Node: f"budget_excluding_subtasks={self.budget_excluding_subtasks}, " f"budget_including_subtasks={self.budget_including_subtasks}, " f"nlnet_milestone={self.nlnet_milestone!r}, " - f"immediate_children={immediate_children!r}") + f"immediate_children={immediate_children!r}, " + f"payees={self.payees!r}") class BudgetGraphError(BudgetGraphBaseError): - def __init__(self, bug_id, root_bug_id): + def __init__(self, bug_id: int, root_bug_id: int): self.bug_id = bug_id self.root_bug_id = root_bug_id @@ -139,7 +184,8 @@ class BudgetGraphMilestoneMismatch(BudgetGraphError): class BudgetGraphMoneyMismatch(BudgetGraphError): - def __init__(self, bug_id, root_bug_id, expected_budget_excluding_subtasks): + def __init__(self, bug_id: int, root_bug_id: int, + expected_budget_excluding_subtasks: Money): super().__init__(bug_id, root_bug_id) self.expected_budget_excluding_subtasks = \ expected_budget_excluding_subtasks @@ -157,6 +203,29 @@ class BudgetGraphNegativeMoney(BudgetGraphError): f"bug #{self.bug_id}") +class BudgetGraphPayeesMoneyMismatch(BudgetGraphError): + def __init__(self, bug_id: int, root_bug_id: int, payees_total: Money): + super().__init__(bug_id, root_bug_id) + self.payees_total = payees_total + + def __str__(self): + return (f"Budget assigned to task excluding subtasks " + f"(cf_budget field) doesn't match total value " + f"assigned to payees (cf_payees_list): " + f"bug #{self.bug_id}, calculated total" + f" {self.payees_total}") + + +class BudgetGraphNegativePayeeMoney(BudgetGraphError): + def __init__(self, bug_id: int, root_bug_id: int, payee_key: str): + super().__init__(bug_id, root_bug_id) + self.payee_key = payee_key + + def __str__(self): + return (f"Budget assigned to payee for task is less than zero: " + f"bug #{self.bug_id}, payee {self.payee_key!r}") + + class BudgetGraph: nodes: Dict[int, Node] @@ -188,7 +257,8 @@ class BudgetGraph: if node.nlnet_milestone != root.nlnet_milestone: errors.append(BudgetGraphMilestoneMismatch( node.bug.id, root.bug.id)) - if node.budget_excluding_subtasks < 0 or node.budget_including_subtasks < 0: + if node.budget_excluding_subtasks < 0 \ + or node.budget_including_subtasks < 0: errors.append(BudgetGraphNegativeMoney( node.bug.id, root.bug.id)) budget = node.budget_including_subtasks @@ -197,14 +267,41 @@ class BudgetGraph: if node.budget_excluding_subtasks != budget: errors.append(BudgetGraphMoneyMismatch( node.bug.id, root.bug.id, budget)) + payees_total = Money(0) + for payee_key, payee_value in node.payees.items(): + if payee_value < 0: + errors.append(BudgetGraphNegativePayeeMoney( + node.bug.id, root.bug.id, payee_key)) + payees_total += payee_value + if node.budget_excluding_subtasks != payees_total \ + and len(node.payees) != 0: + errors.append(BudgetGraphPayeesMoneyMismatch( + node.bug.id, root.bug.id, payees_total)) def get_errors(self) -> List[BudgetGraphBaseError]: errors = [] try: - for root in self.roots: - self._get_node_errors(root, root, errors) - for child in root.children(): - self._get_node_errors(root, child, errors) + roots = self.roots except BudgetGraphBaseError as e: errors.append(e) + return errors + + for root in roots: + try: + self._get_node_errors(root, root, errors) + for child in root.children(): + try: + self._get_node_errors(root, child, errors) + except BudgetGraphBaseError as e: + errors.append(e) + except BudgetGraphBaseError as e: + errors.append(e) return errors + + @cached_property + def payee_keys(self) -> Set[str]: + retval = set() + for node in self.nodes.values(): + for payee_key in node.payees.keys(): + retval.add(payee_key) + return retval diff --git a/src/budget_sync/test/mock_bug.py b/src/budget_sync/test/mock_bug.py index eb9f05b..39a681d 100644 --- a/src/budget_sync/test/mock_bug.py +++ b/src/budget_sync/test/mock_bug.py @@ -7,7 +7,8 @@ class MockBug: cf_budget_parent: Optional[int], cf_budget: str, cf_total_budget: str, - cf_nlnet_milestone: Optional[str]): + cf_nlnet_milestone: Optional[str], + cf_payees_list: str): self.id = bug_id if cf_budget_parent is not None: self.cf_budget_parent = cf_budget_parent @@ -16,6 +17,7 @@ class MockBug: if cf_nlnet_milestone is None: cf_nlnet_milestone = "---" self.cf_nlnet_milestone = cf_nlnet_milestone + self.cf_payees_list = cf_payees_list def __repr__(self): cf_budget_parent = getattr(self, "cf_budget_parent", None) @@ -23,4 +25,5 @@ class MockBug: f"cf_budget_parent={cf_budget_parent!r}, " f"cf_budget={self.cf_budget!r}, " f"cf_total_budget={self.cf_total_budget!r}, " - f"cf_nlnet_milestone={self.cf_nlnet_milestone!r})") + f"cf_nlnet_milestone={self.cf_nlnet_milestone!r}, " + f"cf_payees_list={self.cf_payees_list!r})") diff --git a/src/budget_sync/test/test_budget_graph.py b/src/budget_sync/test/test_budget_graph.py index 7f08762..932d48e 100644 --- a/src/budget_sync/test/test_budget_graph.py +++ b/src/budget_sync/test/test_budget_graph.py @@ -4,7 +4,10 @@ from budget_sync.budget_graph import (BudgetGraphLoopError, BudgetGraph, BudgetGraphBaseError, BudgetGraphMoneyMismatch, BudgetGraphNegativeMoney, - BudgetGraphMilestoneMismatch) + BudgetGraphMilestoneMismatch, + BudgetGraphNegativePayeeMoney, + BudgetGraphPayeesParseError, + BudgetGraphPayeesMoneyMismatch) from budget_sync.money import Money from typing import List, Type import unittest @@ -39,37 +42,61 @@ class TestErrorFormatting(unittest.TestCase): self.assertEqual(str(BudgetGraphNegativeMoney(1, 5)), "Budget assigned to task is less than zero: bug #1") + def test_budget_graph_negative_payee_money(self): + self.assertEqual(str(BudgetGraphNegativePayeeMoney(1, 5, "payee1")), + "Budget assigned to payee for task is less than " + "zero: bug #1, payee 'payee1'") + + def test_budget_graph_payees_parse_error(self): + self.assertEqual(str( + BudgetGraphPayeesParseError(1, "my fake parse error")), + "Failed to parse cf_payees_list field of bug #1: " + "my fake parse error") + + def test_budget_graph_payees_money_mismatch(self): + self.assertEqual(str( + BudgetGraphPayeesMoneyMismatch(1, 5, Money(123))), + "Budget assigned to task excluding subtasks (cf_budget field) " + "doesn't match total value assigned to payees (cf_payees_list):" + " bug #1, calculated total 123") + EXAMPLE_BUG1 = MockBug(bug_id=1, cf_budget_parent=None, cf_budget="0", cf_total_budget="0", - cf_nlnet_milestone=None) + cf_nlnet_milestone=None, + cf_payees_list="") EXAMPLE_LOOP1_BUG1 = MockBug(bug_id=1, cf_budget_parent=1, cf_budget="0", cf_total_budget="0", - cf_nlnet_milestone=None) + cf_nlnet_milestone=None, + cf_payees_list="") EXAMPLE_LOOP2_BUG1 = MockBug(bug_id=1, cf_budget_parent=2, cf_budget="0", cf_total_budget="0", - cf_nlnet_milestone=None) + cf_nlnet_milestone=None, + cf_payees_list="") EXAMPLE_LOOP2_BUG2 = MockBug(bug_id=2, cf_budget_parent=1, cf_budget="0", cf_total_budget="0", - cf_nlnet_milestone=None) + cf_nlnet_milestone=None, + cf_payees_list="") EXAMPLE_PARENT_BUG1 = MockBug(bug_id=1, cf_budget_parent=None, cf_budget="10", cf_total_budget="20", - cf_nlnet_milestone="abc") + cf_nlnet_milestone="abc", + cf_payees_list="") EXAMPLE_CHILD_BUG2 = MockBug(bug_id=2, cf_budget_parent=1, cf_budget="10", cf_total_budget="10", - cf_nlnet_milestone="abc") + cf_nlnet_milestone="abc", + cf_payees_list="") class TestBudgetGraph(unittest.TestCase): @@ -98,6 +125,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, {}) def test_loop1(self): with self.assertRaises(BudgetGraphLoopError) as cm: @@ -129,6 +157,7 @@ class TestBudgetGraph(unittest.TestCase): self.assertEqual(node1.budget_including_subtasks, Money(cents=2000)) self.assertEqual(node1.nlnet_milestone, "abc") self.assertEqual(list(node1.children()), [node2]) + self.assertEqual(node1.payees, {}) self.assertIsInstance(node2, Node) self.assertIs(node2.graph, bg) self.assertIs(node2.bug, EXAMPLE_CHILD_BUG2) @@ -138,6 +167,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, {}) def test_money_with_no_milestone(self): bg = BudgetGraph([ @@ -145,7 +175,8 @@ class TestBudgetGraph(unittest.TestCase): cf_budget_parent=None, cf_budget="0", cf_total_budget="10", - cf_nlnet_milestone=None), + cf_nlnet_milestone=None, + cf_payees_list=""), ]) errors = bg.get_errors() self.assertErrorTypesMatches(errors, @@ -158,7 +189,8 @@ class TestBudgetGraph(unittest.TestCase): cf_budget_parent=None, cf_budget="10", cf_total_budget="0", - cf_nlnet_milestone=None), + cf_nlnet_milestone=None, + cf_payees_list=""), ]) errors = bg.get_errors() self.assertErrorTypesMatches(errors, @@ -171,7 +203,8 @@ class TestBudgetGraph(unittest.TestCase): cf_budget_parent=None, cf_budget="10", cf_total_budget="10", - cf_nlnet_milestone=None), + cf_nlnet_milestone=None, + cf_payees_list=""), ]) errors = bg.get_errors() self.assertErrorTypesMatches(errors, [BudgetGraphMoneyWithNoMilestone]) @@ -184,7 +217,8 @@ class TestBudgetGraph(unittest.TestCase): cf_budget_parent=None, cf_budget="0", cf_total_budget="10", - cf_nlnet_milestone="abc"), + cf_nlnet_milestone="abc", + cf_payees_list=""), ]) errors = bg.get_errors() self.assertErrorTypesMatches(errors, @@ -197,7 +231,8 @@ class TestBudgetGraph(unittest.TestCase): cf_budget_parent=None, cf_budget="10", cf_total_budget="0", - cf_nlnet_milestone="abc"), + cf_nlnet_milestone="abc", + cf_payees_list=""), ]) errors = bg.get_errors() self.assertErrorTypesMatches(errors, @@ -210,7 +245,8 @@ class TestBudgetGraph(unittest.TestCase): cf_budget_parent=None, cf_budget="10", cf_total_budget="10", - cf_nlnet_milestone="abc"), + cf_nlnet_milestone="abc", + cf_payees_list=""), ]) errors = bg.get_errors() self.assertEqual(errors, []) @@ -219,17 +255,20 @@ class TestBudgetGraph(unittest.TestCase): cf_budget_parent=None, cf_budget="10", cf_total_budget="10", - cf_nlnet_milestone="abc"), + cf_nlnet_milestone="abc", + cf_payees_list=""), MockBug(bug_id=2, cf_budget_parent=1, cf_budget="10", cf_total_budget="10", - cf_nlnet_milestone="abc"), + cf_nlnet_milestone="abc", + cf_payees_list=""), MockBug(bug_id=3, cf_budget_parent=1, cf_budget="1", cf_total_budget="10", - cf_nlnet_milestone="abc"), + cf_nlnet_milestone="abc", + cf_payees_list=""), ]) errors = bg.get_errors() self.assertErrorTypesMatches(errors, @@ -248,7 +287,8 @@ class TestBudgetGraph(unittest.TestCase): cf_budget_parent=None, cf_budget="0", cf_total_budget="-10", - cf_nlnet_milestone="abc"), + cf_nlnet_milestone="abc", + cf_payees_list=""), ]) errors = bg.get_errors() self.assertErrorTypesMatches(errors, @@ -264,7 +304,8 @@ class TestBudgetGraph(unittest.TestCase): cf_budget_parent=None, cf_budget="-10", cf_total_budget="0", - cf_nlnet_milestone="abc"), + cf_nlnet_milestone="abc", + cf_payees_list=""), ]) errors = bg.get_errors() self.assertErrorTypesMatches(errors, @@ -280,7 +321,8 @@ class TestBudgetGraph(unittest.TestCase): cf_budget_parent=None, cf_budget="-10", cf_total_budget="-10", - cf_nlnet_milestone="abc"), + cf_nlnet_milestone="abc", + cf_payees_list=""), ]) errors = bg.get_errors() self.assertErrorTypesMatches(errors, @@ -288,6 +330,166 @@ class TestBudgetGraph(unittest.TestCase): self.assertEqual(errors[0].bug_id, 1) self.assertEqual(errors[0].root_bug_id, 1) + def test_payees_parse(self): + def check(cf_payees_list, expected_payees): + bg = BudgetGraph([MockBug(bug_id=1, + cf_budget_parent=None, + cf_budget="0", + cf_total_budget="0", + cf_nlnet_milestone="abc", + cf_payees_list=cf_payees_list), + ]) + self.assertEqual(len(bg.nodes), 1) + node: Node = bg.nodes[1] + self.assertEqual(node.payees, expected_payees) + + check(""" + abc = 123 + """, + {"abc": Money(123)}) + check(""" + abc = "123" + """, + {"abc": Money(123)}) + check(""" + abc = "123.45" + """, + {"abc": Money("123.45")}) + check(""" + abc = "123.45" + "d e f" = "21.35" + """, + { + "abc": Money("123.45"), + "d e f": Money("21.35"), + }) + check(""" + abc = "123.45" + # my comments + "AAA" = "-21.35" + """, + { + "abc": Money("123.45"), + "AAA": Money("-21.35"), + }) + check(""" + "not-an-email@example.com" = "-2345" + """, + { + "not-an-email@example.com": Money(-2345), + }) + + def test_payees_money_mismatch(self): + bg = BudgetGraph([ + MockBug(bug_id=1, + cf_budget_parent=None, + cf_budget="10", + cf_total_budget="10", + cf_nlnet_milestone="abc", + cf_payees_list="payee = 5\npayee2 = 10"), + ]) + errors = bg.get_errors() + self.assertErrorTypesMatches(errors, + [BudgetGraphPayeesMoneyMismatch]) + self.assertEqual(errors[0].bug_id, 1) + self.assertEqual(errors[0].root_bug_id, 1) + self.assertEqual(errors[0].payees_total, 15) + bg = BudgetGraph([ + MockBug(bug_id=1, + cf_budget_parent=None, + cf_budget="0", + cf_total_budget="0", + cf_nlnet_milestone=None, + cf_payees_list="payee = 5\npayee2 = 10"), + ]) + errors = bg.get_errors() + self.assertErrorTypesMatches(errors, + [BudgetGraphPayeesMoneyMismatch]) + self.assertEqual(errors[0].bug_id, 1) + self.assertEqual(errors[0].root_bug_id, 1) + self.assertEqual(errors[0].payees_total, 15) + + def test_payees_parse_error(self): + def check_parse_error(cf_payees_list, expected_msg): + errors = BudgetGraph([ + MockBug(bug_id=1, + cf_budget_parent=None, + cf_budget="0", + cf_total_budget="0", + cf_nlnet_milestone="abc", + cf_payees_list=cf_payees_list), + ]).get_errors() + self.assertErrorTypesMatches(errors, + [BudgetGraphPayeesParseError]) + self.assertEqual(errors[0].bug_id, 1) + self.assertEqual(errors[0].msg, expected_msg) + + check_parse_error(""" + "payee 1" = {} + """, + "value for key 'payee 1' is not a string or integer " + "(to use fractional values such as 123.45, write " + "\"123.45\"): {}") + + check_parse_error(""" + payee = "ashjkf" + """, + "failed to parse Money value for key 'payee': " + "invalid Money string: characters after sign and " + "before first `.` must be ascii digits") + + check_parse_error(""" + payee = "1" + payee = "1" + """, + "TOML parse error: Duplicate keys! (line 3" + " column 1 char 39)") + + 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") + + def test_negative_payee_money(self): + bg = BudgetGraph([ + MockBug(bug_id=1, + cf_budget_parent=None, + cf_budget="10", + cf_total_budget="10", + cf_nlnet_milestone="abc", + cf_payees_list="""payee1 = -10"""), + ]) + errors = bg.get_errors() + self.assertErrorTypesMatches(errors, + [BudgetGraphNegativePayeeMoney, + BudgetGraphPayeesMoneyMismatch]) + self.assertEqual(errors[0].bug_id, 1) + self.assertEqual(errors[0].root_bug_id, 1) + self.assertEqual(errors[0].payee_key, "payee1") + self.assertEqual(errors[1].bug_id, 1) + self.assertEqual(errors[1].root_bug_id, 1) + self.assertEqual(errors[1].payees_total, -10) + + def test_payee_keys(self): + bg = BudgetGraph([ + MockBug(bug_id=1, + cf_budget_parent=None, + cf_budget="10", + cf_total_budget="10", + cf_nlnet_milestone="abc", + cf_payees_list="payee2 = 3\npayee1 = 7"), + MockBug(bug_id=2, + cf_budget_parent=None, + cf_budget="10", + cf_total_budget="10", + cf_nlnet_milestone="def", + cf_payees_list="""payee3 = 5\npayee2 = 5"""), + ]) + self.assertErrorTypesMatches(bg.get_errors(), []) + self.assertEqual(bg.payee_keys, {"payee1", "payee2", "payee3"}) + if __name__ == "__main__": unittest.main() -- 2.30.2