From f02e0fc13b49dc5b38924ba3ad8c485007a72cb2 Mon Sep 17 00:00:00 2001 From: Christian Heller Date: Fri, 21 Jun 2024 15:26:37 +0200 Subject: [PATCH 01/16] Refactor request handler identifying items by ID param on GET. --- plomtask/http.py | 69 ++++++++++++++++++++++++++++-------------------- tests/todos.py | 4 +-- 2 files changed, 42 insertions(+), 31 deletions(-) diff --git a/plomtask/http.py b/plomtask/http.py index 4c0d6a3..28812df 100644 --- a/plomtask/http.py +++ b/plomtask/http.py @@ -221,6 +221,25 @@ class TaskHandler(BaseHTTPRequestHandler): # GET handlers + @staticmethod + def _get_item(target_class: Any + ) -> Callable[..., Callable[[TaskHandler], + dict[str, object]]]: + def decorator(f: Callable[..., dict[str, object]] + ) -> Callable[[TaskHandler], dict[str, object]]: + def wrapper(self: TaskHandler) -> dict[str, object]: + # pylint: disable=protected-access + # (because pylint here fails to detect the use of wrapper as a + # method to self with respective access privileges) + id_ = self._params.get_int_or_none('id') + if target_class.can_create_by_id: + item = target_class.by_id_or_create(self.conn, id_) + else: + item = target_class.by_id(self.conn, id_) + return f(self, item) + return wrapper + return decorator + def do_GET_(self) -> str: """Return redirect target on GET /.""" return '/day' @@ -279,7 +298,8 @@ class TaskHandler(BaseHTTPRequestHandler): 'conditions_present': conditions_present, 'processes': Process.all(self.conn)} - def do_GET_todo(self) -> dict[str, object]: + @_get_item(Todo) + def do_GET_todo(self, todo: Todo) -> dict[str, object]: """Show single Todo of ?id=.""" @dataclass @@ -330,8 +350,6 @@ class TaskHandler(BaseHTTPRequestHandler): ids = ids | collect_adoptables_keys(node.children) return ids - id_ = self._params.get_int('id') - todo = Todo.by_id(self.conn, id_) todo_steps = [step.todo for step in todo.get_step_tree(set()).children] process_tree = todo.process.get_steps(self.conn, None) steps_todo_to_process: list[TodoStepsNode] = [] @@ -407,10 +425,9 @@ class TaskHandler(BaseHTTPRequestHandler): 'sort_by': sort_by, 'pattern': pattern} - def do_GET_condition(self) -> dict[str, object]: + @_get_item(Condition) + def do_GET_condition(self, c: Condition) -> dict[str, object]: """Show Condition of ?id=.""" - id_ = self._params.get_int_or_none('id') - c = Condition.by_id_or_create(self.conn, id_) ps = Process.all(self.conn) return {'condition': c, 'is_new': c.id_ is None, 'enabled_processes': [p for p in ps if c in p.conditions], @@ -418,22 +435,19 @@ class TaskHandler(BaseHTTPRequestHandler): 'enabling_processes': [p for p in ps if c in p.enables], 'disabling_processes': [p for p in ps if c in p.disables]} - def do_GET_condition_titles(self) -> dict[str, object]: + @_get_item(Condition) + def do_GET_condition_titles(self, c: Condition) -> dict[str, object]: """Show title history of Condition of ?id=.""" - id_ = self._params.get_int('id') - condition = Condition.by_id(self.conn, id_) - return {'condition': condition} + return {'condition': c} - def do_GET_condition_descriptions(self) -> dict[str, object]: + @_get_item(Condition) + def do_GET_condition_descriptions(self, c: Condition) -> dict[str, object]: """Show description historys of Condition of ?id=.""" - id_ = self._params.get_int('id') - condition = Condition.by_id(self.conn, id_) - return {'condition': condition} + return {'condition': c} - def do_GET_process(self) -> dict[str, object]: + @_get_item(Process) + def do_GET_process(self, process: Process) -> dict[str, object]: """Show Process of ?id=.""" - id_ = self._params.get_int_or_none('id') - process = Process.by_id_or_create(self.conn, id_) title_64 = self._params.get_str('title_b64') if title_64: title = b64decode(title_64.encode()).decode() @@ -451,23 +465,20 @@ class TaskHandler(BaseHTTPRequestHandler): 'process_candidates': Process.all(self.conn), 'condition_candidates': Condition.all(self.conn)} - def do_GET_process_titles(self) -> dict[str, object]: + @_get_item(Process) + def do_GET_process_titles(self, p: Process) -> dict[str, object]: """Show title history of Process of ?id=.""" - id_ = self._params.get_int('id') - process = Process.by_id(self.conn, id_) - return {'process': process} + return {'process': p} - def do_GET_process_descriptions(self) -> dict[str, object]: + @_get_item(Process) + def do_GET_process_descriptions(self, p: Process) -> dict[str, object]: """Show description historys of Process of ?id=.""" - id_ = self._params.get_int('id') - process = Process.by_id(self.conn, id_) - return {'process': process} + return {'process': p} - def do_GET_process_efforts(self) -> dict[str, object]: + @_get_item(Process) + def do_GET_process_efforts(self, p: Process) -> dict[str, object]: """Show default effort history of Process of ?id=.""" - id_ = self._params.get_int('id') - process = Process.by_id(self.conn, id_) - return {'process': process} + return {'process': p} def do_GET_processes(self) -> dict[str, object]: """Show all Processes.""" diff --git a/tests/todos.py b/tests/todos.py index 626b744..0998c69 100644 --- a/tests/todos.py +++ b/tests/todos.py @@ -414,8 +414,8 @@ class TestsWithServer(TestCaseWithServer): self.post_process() form_data = {'day_comment': '', 'new_todo': 1, 'make_type': 'full'} self.check_post(form_data, '/day?date=2024-01-01&make_type=full', 302) - self.check_get('/todo', 400) - self.check_get('/todo?id=', 400) + self.check_get('/todo', 404) + self.check_get('/todo?id=', 404) self.check_get('/todo?id=foo', 400) self.check_get('/todo?id=0', 404) self.check_get('/todo?id=1', 200) -- 2.30.2 From 39ae008d82dd325577fcebe1127e48ea82603606 Mon Sep 17 00:00:00 2001 From: Christian Heller Date: Fri, 21 Jun 2024 16:29:15 +0200 Subject: [PATCH 02/16] Clean up and extend /condition[s] GET/POST tests. --- .pylintrc | 2 +- tests/conditions.py | 181 ++++++++++++++++++++++++-------------------- 2 files changed, 98 insertions(+), 85 deletions(-) diff --git a/.pylintrc b/.pylintrc index b4814d1..50133a0 100644 --- a/.pylintrc +++ b/.pylintrc @@ -1,3 +1,3 @@ [BASIC] init-hook='import sys; sys.path.append(".")' -good-names-rgxs=.*_?do_(GET|POST)(_[a-z]+)?,test_[A-Z]+ +good-names-rgxs=(.*_)?(GET|POST)(_.+)?,,test_[A-Z]+ diff --git a/tests/conditions.py b/tests/conditions.py index 25a044f..9fcb68c 100644 --- a/tests/conditions.py +++ b/tests/conditions.py @@ -42,32 +42,55 @@ class TestsWithDB(TestCaseWithDB): class TestsWithServer(TestCaseWithServer): """Module tests against our HTTP server/handler (and database).""" + @classmethod + def GET_condition_dict(cls, cond: dict[str, object]) -> dict[str, object]: + """Return JSON of GET /condition to expect.""" + return {'is_new': False, + 'enabled_processes': [], + 'disabled_processes': [], + 'enabling_processes': [], + 'disabling_processes': [], + 'condition': cond['id'], + '_library': {'Condition': cls.as_refs([cond])}} + + @classmethod + def GET_conditions_dict(cls, conds: list[dict[str, object]] + ) -> dict[str, object]: + """Return JSON of GET /conditions to expect.""" + library = {'Condition': cls.as_refs(conds)} if conds else {} + d: dict[str, object] = {'conditions': cls.as_id_list(conds), + 'sort_by': 'title', + 'pattern': '', + '_library': library} + return d + + def test_fail_POST_condition(self) -> None: + """Test malformed/illegal POST /condition requests.""" + # check invalid POST payloads + url = '/condition' + self.check_post({}, url, 400) + self.check_post({'title': ''}, url, 400) + self.check_post({'title': '', 'description': ''}, url, 400) + self.check_post({'title': '', 'is_active': False}, url, 400) + self.check_post({'description': '', 'is_active': False}, url, 400) + # check valid POST payload on bad paths + valid_payload = {'title': '', 'description': '', 'is_active': False} + self.check_post(valid_payload, '/condition?id=foo', 400) + def test_do_POST_condition(self) -> None: """Test POST /condition and its effect on GET /condition[s].""" - # check empty POST fails - self.check_post({}, '/condition', 400) # test valid POST's effect on … post = {'title': 'foo', 'description': 'oof', 'is_active': False} self.check_post(post, '/condition', 302, '/condition?id=1') # … single /condition cond = self.cond_as_dict(titles=['foo'], descriptions=['oof']) assert isinstance(cond['_versioned'], dict) - expected_single: dict[str, object] - expected_single = {'is_new': False, - 'enabled_processes': [], - 'disabled_processes': [], - 'enabling_processes': [], - 'disabling_processes': [], - 'condition': cond['id'], - '_library': {'Condition': self.as_refs([cond])}} + expected_single = self.GET_condition_dict(cond) self.check_json_get('/condition?id=1', expected_single) # … full /conditions - expected_all: dict[str, object] - expected_all = {'conditions': self.as_id_list([cond]), - 'sort_by': 'title', 'pattern': '', - '_library': {'Condition': self.as_refs([cond])}} + expected_all = self.GET_conditions_dict([cond]) self.check_json_get('/conditions', expected_all) - # test effect of invalid POST to existing Condition on /condition + # test (no) effect of invalid POST to existing Condition on /condition self.check_post({}, '/condition?id=1', 400) self.check_json_get('/condition?id=1', expected_single) # test effect of POST changing title and activeness @@ -80,7 +103,6 @@ class TestsWithServer(TestCaseWithServer): self.check_post({'delete': ''}, '/condition?id=1', 302, '/conditions') cond = self.cond_as_dict() assert isinstance(expected_single['_library'], dict) - assert isinstance(expected_single['_library']['Condition'], dict) expected_single['_library']['Condition'] = self.as_refs([cond]) self.check_json_get('/condition?id=1', expected_single) # … full /conditions @@ -92,84 +114,75 @@ class TestsWithServer(TestCaseWithServer): """More GET /condition testing, especially for Process relations.""" # check expected default status codes self.check_get_defaults('/condition') - # check display of process relations - form_data = {'title': 'foo', 'description': 'oof', 'is_active': False} - self.check_post(form_data, '/condition', 302, '/condition?id=1') - proc_1_post = {'title': 'A', 'description': '', 'effort': 1.0, - 'conditions': [1], 'disables': [1]} - self.post_process(1, proc_1_post) - proc_2_post = {'title': 'B', 'description': '', 'effort': 1.0, - 'enables': [1], 'blockers': [1]} - self.post_process(2, proc_2_post) + # make Condition and two Processes that among them establish all + # possible ConditionsRelations to it, … + cond_post = {'title': 'foo', 'description': 'oof', 'is_active': False} + self.check_post(cond_post, '/condition', 302, '/condition?id=1') + proc1_post = {'title': 'A', 'description': '', 'effort': 1.0, + 'conditions': [1], 'disables': [1]} + proc2_post = {'title': 'B', 'description': '', 'effort': 1.0, + 'enables': [1], 'blockers': [1]} + self.post_process(1, proc1_post) + self.post_process(2, proc2_post) + # … then check /condition displays all these properly. cond = self.cond_as_dict(titles=['foo'], descriptions=['oof']) assert isinstance(cond['id'], int) - proc_1 = self.proc_as_dict(conditions=[cond['id']], - disables=[cond['id']]) - proc_2 = self.proc_as_dict(2, 'B', - blockers=[cond['id']], - enables=[cond['id']]) - expected = {'is_new': False, - 'enabled_processes': self.as_id_list([proc_1]), - 'disabled_processes': self.as_id_list([proc_2]), - 'enabling_processes': self.as_id_list([proc_2]), - 'disabling_processes': self.as_id_list([proc_1]), - 'condition': cond['id'], - '_library': {'Condition': self.as_refs([cond]), - 'Process': self.as_refs([proc_1, proc_2])}} + proc1 = self.proc_as_dict(conditions=[cond['id']], + disables=[cond['id']]) + proc2 = self.proc_as_dict(2, 'B', + blockers=[cond['id']], + enables=[cond['id']]) + expected = self.GET_condition_dict(cond) + assert isinstance(expected['_library'], dict) + expected['enabled_processes'] = self.as_id_list([proc1]) + expected['disabled_processes'] = self.as_id_list([proc2]) + expected['enabling_processes'] = self.as_id_list([proc2]) + expected['disabling_processes'] = self.as_id_list([proc1]) + expected['_library']['Process'] = self.as_refs([proc1, proc2]) self.check_json_get('/condition?id=1', expected) def test_do_GET_conditions(self) -> None: """Test GET /conditions.""" # test empty result on empty DB, default-settings on empty params - expected_json: dict[str, object] = {'conditions': [], - 'sort_by': 'title', - 'pattern': '', - '_library': {}} - self.check_json_get('/conditions', expected_json) - # test on meaningless non-empty params (incl. entirely un-used key) - expected_json = {'conditions': [], - 'sort_by': 'title', # nonsense "foo" defaulting - 'pattern': 'bar', # preserved despite zero effect - '_library': {}} - self.check_json_get('/conditions?sort_by=foo&pattern=bar&foo=x', - expected_json) + expected = self.GET_conditions_dict([]) + self.check_json_get('/conditions', expected) + # test on meaningless non-empty params (incl. entirely un-used key), + # that 'sort_by' default to 'title' (even if set to something else, as + # long as without handler) and 'pattern' get preserved + expected['pattern'] = 'bar' # preserved despite zero effect! + url = '/conditions?sort_by=foo&pattern=bar&foo=x' + self.check_json_get(url, expected) # test non-empty result, automatic (positive) sorting by title - post_1 = {'title': 'foo', 'description': 'oof', 'is_active': False} - self.check_post(post_1, '/condition', 302, '/condition?id=1') - post_2 = {'title': 'bar', 'description': 'rab', 'is_active': False} - self.check_post(post_2, '/condition', 302, '/condition?id=2') - post_3 = {'title': 'baz', 'description': 'zab', 'is_active': True} - self.check_post(post_3, '/condition', 302, '/condition?id=3') - cond_1 = self.cond_as_dict(titles=['foo'], descriptions=['oof']) - cond_2 = self.cond_as_dict(2, titles=['bar'], descriptions=['rab']) - cond_3 = self.cond_as_dict(3, True, ['baz'], ['zab']) - cons = [cond_2, cond_3, cond_1] - expected_json = {'conditions': self.as_id_list(cons), - 'sort_by': 'title', - 'pattern': '', - '_library': {'Condition': self.as_refs(cons)}} - self.check_json_get('/conditions', expected_json) + post1 = {'is_active': False, 'title': 'foo', 'description': 'oof'} + post2 = {'is_active': False, 'title': 'bar', 'description': 'rab'} + post3 = {'is_active': True, 'title': 'baz', 'description': 'zab'} + self.check_post(post1, '/condition', 302, '/condition?id=1') + self.check_post(post2, '/condition', 302, '/condition?id=2') + self.check_post(post3, '/condition', 302, '/condition?id=3') + cond1 = self.cond_as_dict(1, False, ['foo'], ['oof']) + cond2 = self.cond_as_dict(2, False, ['bar'], ['rab']) + cond3 = self.cond_as_dict(3, True, ['baz'], ['zab']) + expected = self.GET_conditions_dict([cond2, cond3, cond1]) + self.check_json_get('/conditions', expected) # test other sortings # (NB: by .is_active has two items of =False, their order currently # is not explicitly made predictable, so mail fail until we do) - expected_json['conditions'] = self.as_id_list([cond_1, cond_3, cond_2]) - expected_json['sort_by'] = '-title' - self.check_json_get('/conditions?sort_by=-title', expected_json) - expected_json['conditions'] = self.as_id_list([cond_1, cond_2, cond_3]) - expected_json['sort_by'] = 'is_active' - self.check_json_get('/conditions?sort_by=is_active', expected_json) - expected_json['conditions'] = self.as_id_list([cond_3, cond_1, cond_2]) - expected_json['sort_by'] = '-is_active' - self.check_json_get('/conditions?sort_by=-is_active', expected_json) + expected['conditions'] = self.as_id_list([cond1, cond3, cond2]) + expected['sort_by'] = '-title' + self.check_json_get('/conditions?sort_by=-title', expected) + expected['conditions'] = self.as_id_list([cond1, cond2, cond3]) + expected['sort_by'] = 'is_active' + self.check_json_get('/conditions?sort_by=is_active', expected) + expected['conditions'] = self.as_id_list([cond3, cond1, cond2]) + expected['sort_by'] = '-is_active' + self.check_json_get('/conditions?sort_by=-is_active', expected) # test pattern matching on title - cons = [cond_2, cond_3] - expected_json = {'conditions': self.as_id_list(cons), - 'sort_by': 'title', 'pattern': 'ba', - '_library': {'Condition': self.as_refs(cons)}} - self.check_json_get('/conditions?pattern=ba', expected_json) + expected = self.GET_conditions_dict([cond2, cond3]) + expected['pattern'] = 'ba' + self.check_json_get('/conditions?pattern=ba', expected) # test pattern matching on description - expected_json['conditions'] = self.as_id_list([cond_1]) - assert isinstance(expected_json['_library'], dict) - expected_json['_library']['Condition'] = self.as_refs([cond_1]) - expected_json['pattern'] = 'oo' - self.check_json_get('/conditions?pattern=oo', expected_json) + assert isinstance(expected['_library'], dict) + expected['conditions'] = self.as_id_list([cond1]) + expected['_library']['Condition'] = self.as_refs([cond1]) + expected['pattern'] = 'oo' + self.check_json_get('/conditions?pattern=oo', expected) -- 2.30.2 From 68d0abbe8563c6e778040b2d4f3f640858f5b968 Mon Sep 17 00:00:00 2001 From: Christian Heller Date: Fri, 21 Jun 2024 17:37:33 +0200 Subject: [PATCH 03/16] Improve POST /day input validation. --- plomtask/http.py | 27 +++++++++++++++++++++------ 1 file changed, 21 insertions(+), 6 deletions(-) diff --git a/plomtask/http.py b/plomtask/http.py index 28812df..2706603 100644 --- a/plomtask/http.py +++ b/plomtask/http.py @@ -137,6 +137,20 @@ class InputsParser: msg = f'cannot int a form field value for key {key} in: {all_str}' raise BadFormatException(msg) from e + def get_all_floats_or_nones(self, key: str) -> list[float | None]: + """Retrieve list of float value at key, None if empty strings.""" + ret: list[float | None] = [] + for val in self.get_all_str(key): + if '' == val: + ret += [None] + else: + try: + ret += [float(val)] + except ValueError as e: + msg = f'cannot float form field value for key {key}: {val}' + raise BadFormatException(msg) from e + return ret + class TaskHandler(BaseHTTPRequestHandler): """Handles single HTTP request.""" @@ -552,13 +566,14 @@ class TaskHandler(BaseHTTPRequestHandler): make_type = self._form_data.get_str('make_type') old_todos = self._form_data.get_all_int('todo_id') new_todos = self._form_data.get_all_int('new_todo') - is_done = [t_id in self._form_data.get_all_int('done') - for t_id in old_todos] comments = self._form_data.get_all_str('comment') - efforts = [float(effort) if effort else None - for effort in self._form_data.get_all_str('effort')] - if old_todos and 3*[len(old_todos)] != [len(is_done), len(comments), - len(efforts)]: + efforts = self._form_data.get_all_floats_or_nones('effort') + done_todos = self._form_data.get_all_int('done') + for _ in [id_ for id_ in done_todos if id_ not in old_todos]: + raise BadFormatException('"done" field refers to unknown Todo') + is_done = [t_id in done_todos for t_id in old_todos] + if not (len(old_todos) == len(is_done) == len(comments) + == len(efforts)): msg = 'not equal number each of number of todo_id, comments, ' +\ 'and efforts inputs' raise BadFormatException(msg) -- 2.30.2 From c49e55023d5e9d91bcf1433fe893f574184efcb5 Mon Sep 17 00:00:00 2001 From: Christian Heller Date: Fri, 21 Jun 2024 17:40:09 +0200 Subject: [PATCH 04/16] Extend POST /day testing. --- tests/days.py | 79 ++++++++++++++++++++++++++++++++++++++++----------- 1 file changed, 63 insertions(+), 16 deletions(-) diff --git a/tests/days.py b/tests/days.py index 008fc1e..52a3722 100644 --- a/tests/days.py +++ b/tests/days.py @@ -111,7 +111,7 @@ class TestsWithServer(TestCaseWithServer): return {'children': [], 'seen': False, 'todo': todo_id} @staticmethod - def get_day_dict(date: str) -> dict[str, object]: + def GET_day_dict(date: str) -> dict[str, object]: """Return JSON of GET /day to expect.""" day: dict[str, object] = {'id': date, 'comment': '', 'todos': []} d: dict[str, object] @@ -142,20 +142,20 @@ class TestsWithServer(TestCaseWithServer): """Test GET /day basics (no Todos).""" # check undefined day date = date_in_n_days(0) - expected = self.get_day_dict(date) + expected = self.GET_day_dict(date) self.check_json_get('/day', expected) # check "today", "yesterday", "tomorrow" days self.check_json_get('/day?date=today', expected) - expected = self.get_day_dict(date_in_n_days(1)) + expected = self.GET_day_dict(date_in_n_days(1)) self.check_json_get('/day?date=tomorrow', expected) - expected = self.get_day_dict(date_in_n_days(-1)) + expected = self.GET_day_dict(date_in_n_days(-1)) self.check_json_get('/day?date=yesterday', expected) # check wrong day strings self.check_get('/day?date=foo', 400) self.check_get('/day?date=2024-02-30', 400) # check defined day date = '2024-01-01' - expected = self.get_day_dict(date) + expected = self.GET_day_dict(date) assert isinstance(expected['_library'], dict) self.check_json_get(f'/day?date={date}', expected) # check saved day @@ -207,7 +207,7 @@ class TestsWithServer(TestCaseWithServer): procs_expected = self.post_batch(procs_data, [], ['title', 'description', 'effort'], self.proc_as_dict, self.post_process) - expected = self.get_day_dict(date) + expected = self.GET_day_dict(date) assert isinstance(expected['_library'], dict) expected['processes'] = self.as_id_list(procs_expected) expected['_library']['Process'] = self.as_refs(procs_expected) @@ -249,7 +249,7 @@ class TestsWithServer(TestCaseWithServer): ['title', 'description', 'effort'], self.proc_as_dict, self.post_process) date = '2024-01-01' - expected = self.get_day_dict(date) + expected = self.GET_day_dict(date) assert isinstance(expected['_library'], dict) expected['processes'] = self.as_id_list(procs_expected) expected['_library']['Process'] = self.as_refs(procs_expected) @@ -271,18 +271,65 @@ class TestsWithServer(TestCaseWithServer): self.post_day(f'date={date}', post_day) self.check_json_get(f'/day?date={date}', expected) - def test_do_GET(self) -> None: - """Test /day and /calendar response codes, and / redirect.""" + def test_do_GET_calendar(self) -> None: + """Test /calendar response codes, and / redirect.""" self.check_get('/calendar', 200) self.check_get('/calendar?start=&end=', 200) self.check_get('/calendar?start=today&end=today', 200) self.check_get('/calendar?start=2024-01-01&end=2025-01-01', 200) self.check_get('/calendar?start=foo', 400) - def test_do_POST_day(self) -> None: - """Test POST /day.""" - form_data = {'day_comment': '', 'make_type': 'full'} - self.check_post(form_data, '/day', 400) - self.check_post(form_data, '/day?date=foo', 400) - self.check_post(form_data, '/day?date=2024-01-01&make_type=full', 302) - self.check_post({'foo': ''}, '/day?date=2024-01-01', 400) + def test_fail_POST_day(self) -> None: + """Test malformed/illegal POST /condition requests.""" + # check payloads lacking minimum expecteds + url = '/day?date=2024-01-01' + self.check_post({}, url, 400) + self.check_post({'day_comment': ''}, url, 400) + self.check_post({'make_type': ''}, url, 400) + # to next check illegal new_todo values, we need an actual Process + self.post_process(1) + # check illegal new_todo values + post: dict[str, object] + post = {'make_type': '', 'day_comment': '', 'new_todo': ['foo']} + self.check_post(post, url, 400) + post['new_todo'] = [1, 2] # no Process of .id_=2 exists + # to next check illegal old_todo inputs, we need to first post Todo + post['new_todo'] = [1] + self.check_post(post, url, 302, '/day?date=2024-01-01&make_type=') + # check illegal old_todo inputs (equal list lengths though) + post = {'make_type': '', 'day_comment': '', 'comment': ['foo'], + 'effort': [3.3], 'done': [], 'todo_id': [1]} + self.check_post(post, url, 302, '/day?date=2024-01-01&make_type=') + post['todo_id'] = [2] # reference to non-existant Process + self.check_post(post, url, 404) + post['todo_id'] = ['a'] + self.check_post(post, url, 400) + post['todo_id'] = [1] + post['done'] = ['foo'] + self.check_post(post, url, 400) + post['done'] = [2] # reference to non-posted todo_id + self.check_post(post, url, 400) + post['done'] = [] + post['effort'] = ['foo'] + self.check_post(post, url, 400) + post['effort'] = [None] + self.check_post(post, url, 400) + post['effort'] = [3.3] + # check illegal old_todo inputs: unequal list lengths + post['comment'] = [] + self.check_post(post, url, 400) + post['comment'] = ['foo', 'foo'] + self.check_post(post, url, 400) + post['comment'] = ['foo'] + post['effort'] = [] + self.check_post(post, url, 400) + post['effort'] = [3.3, 3.3] + self.check_post(post, url, 400) + post['effort'] = [3.3] + post['todo_id'] = [1, 1] + self.check_post(post, url, 400) + post['todo_id'] = [1] + # # check valid POST payload on bad paths + self.check_post(post, '/day', 400) + self.check_post(post, '/day?date=', 400) + self.check_post(post, '/day?date=foo', 400) -- 2.30.2 From ef4dfff9ff3002c11ccfef4190999a5e4e513606 Mon Sep 17 00:00:00 2001 From: Christian Heller Date: Fri, 21 Jun 2024 18:26:04 +0200 Subject: [PATCH 05/16] Avoid confusing Day.by_id_or_create by always translating dating string first, so a look-up for "today" or so does not ignore the date equivalents. --- plomtask/days.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/plomtask/days.py b/plomtask/days.py index 92e44b2..7e6a5cc 100644 --- a/plomtask/days.py +++ b/plomtask/days.py @@ -34,6 +34,13 @@ class Day(BaseModel[str]): day.todos = Todo.by_date(db_conn, day.id_) return day + @classmethod + def by_id_or_create(cls, db_conn: DatabaseConnection, id_: str | None + ) -> Day: + """Extend BaseModel.by_id to ensure date name translation.""" + assert isinstance(id_, str) + return super().by_id_or_create(db_conn, valid_date(id_)) + @classmethod def by_id(cls, db_conn: DatabaseConnection, id_: str) -> Day: """Extend BaseModel.by_id checking for new/lost .todos.""" -- 2.30.2 From 3c9bff1fcd9f67baa7a13256a1cf49cad085d54e Mon Sep 17 00:00:00 2001 From: Christian Heller Date: Fri, 21 Jun 2024 18:50:53 +0200 Subject: [PATCH 06/16] Re-organize and extend POST/GET /day tests. --- tests/conditions.py | 2 +- tests/days.py | 85 +++++++++++++++++++++++++-------------------- 2 files changed, 49 insertions(+), 38 deletions(-) diff --git a/tests/conditions.py b/tests/conditions.py index 9fcb68c..bf04f7b 100644 --- a/tests/conditions.py +++ b/tests/conditions.py @@ -78,7 +78,7 @@ class TestsWithServer(TestCaseWithServer): self.check_post(valid_payload, '/condition?id=foo', 400) def test_do_POST_condition(self) -> None: - """Test POST /condition and its effect on GET /condition[s].""" + """Test (valid) POST /condition and its effect on GET /condition[s].""" # test valid POST's effect on … post = {'title': 'foo', 'description': 'oof', 'is_active': False} self.check_post(post, '/condition', 302, '/condition?id=1') diff --git a/tests/days.py b/tests/days.py index 52a3722..0b039bf 100644 --- a/tests/days.py +++ b/tests/days.py @@ -138,42 +138,6 @@ class TestsWithServer(TestCaseWithServer): redir_to = f'{target}&make_type={form_data["make_type"]}' self.check_post(form_data, target, status, redir_to) - def test_do_GET_day_basics(self) -> None: - """Test GET /day basics (no Todos).""" - # check undefined day - date = date_in_n_days(0) - expected = self.GET_day_dict(date) - self.check_json_get('/day', expected) - # check "today", "yesterday", "tomorrow" days - self.check_json_get('/day?date=today', expected) - expected = self.GET_day_dict(date_in_n_days(1)) - self.check_json_get('/day?date=tomorrow', expected) - expected = self.GET_day_dict(date_in_n_days(-1)) - self.check_json_get('/day?date=yesterday', expected) - # check wrong day strings - self.check_get('/day?date=foo', 400) - self.check_get('/day?date=2024-02-30', 400) - # check defined day - date = '2024-01-01' - expected = self.GET_day_dict(date) - assert isinstance(expected['_library'], dict) - self.check_json_get(f'/day?date={date}', expected) - # check saved day - post: dict[str, object] = {'day_comment': 'foo', 'make_type': ''} - self.post_day(f'date={date}', post) - expected['_library']['Day'][date]['comment'] = post['day_comment'] - self.check_json_get(f'/day?date={date}', expected) - # check GET parameter to GET requests affects immediate reply, but … - expected['make_type'] = 'bar' - self.check_json_get(f'/day?date={date}&make_type=bar', expected) - # … not any following, … - expected['make_type'] = '' - self.check_json_get(f'/day?date={date}', expected) - # … not even when part of a POST request - post['make_type'] = 'foo' - self.post_day(f'date={date}', post) - self.check_json_get(f'/day?date={date}', expected) - @staticmethod def post_batch(list_of_args: list[list[object]], names_of_simples: list[str], @@ -279,8 +243,33 @@ class TestsWithServer(TestCaseWithServer): self.check_get('/calendar?start=2024-01-01&end=2025-01-01', 200) self.check_get('/calendar?start=foo', 400) + def test_fail_GET_day(self) -> None: + """Test malformed/illegal GET /day requests.""" + self.check_get('/day?date=foo', 400) + self.check_get('/day?date=2024-02-30', 400) + + def test_basic_GET_day(self) -> None: + """Test basic (no Processes/Conditions/Todos) GET /day basics.""" + # check undefined day + date = date_in_n_days(0) + expected = self.GET_day_dict(date) + self.check_json_get('/day', expected) + # NB: GET ?date="today"/"yesterday"/"tomorrow" in test_basic_POST_day + # check 'make_type' GET parameter affects immediate reply, but … + date = '2024-01-01' + expected = self.GET_day_dict(date) + expected['make_type'] = 'bar' + self.check_json_get(f'/day?date={date}&make_type=bar', expected) + # … not any following, … + expected['make_type'] = '' + self.check_json_get(f'/day?date={date}', expected) + # … not even when part of a POST request + post: dict[str, object] = {'day_comment': '', 'make_type': 'foo'} + self.post_day(f'date={date}', post) + self.check_json_get(f'/day?date={date}', expected) + def test_fail_POST_day(self) -> None: - """Test malformed/illegal POST /condition requests.""" + """Test malformed/illegal POST /day requests.""" # check payloads lacking minimum expecteds url = '/day?date=2024-01-01' self.check_post({}, url, 400) @@ -333,3 +322,25 @@ class TestsWithServer(TestCaseWithServer): self.check_post(post, '/day', 400) self.check_post(post, '/day?date=', 400) self.check_post(post, '/day?date=foo', 400) + + def test_basic_POST_day(self) -> None: + """Test basic (no Todos) POST /day. + + Check POST (& GET!) requests properly parse 'today', 'tomorrow', + 'yesterday', and actual date strings; + preserve 'make_type' setting in redirect even if nonsensical; + and store 'day_comment' + """ + for name, dist, test_str in [('2024-01-01', None, 'a'), + ('today', 0, 'b'), + ('yesterday', -1, 'c'), + ('tomorrow', +1, 'd')]: + date = name if dist is None else date_in_n_days(dist) + post = {'day_comment': test_str, 'make_type': f'x:{test_str}'} + post_url = f'/day?date={name}' + redir_url = f'{post_url}&make_type={post["make_type"]}' + self.check_post(post, post_url, 302, redir_url) + expected = self.GET_day_dict(date) + assert isinstance(expected['_library'], dict) + expected['_library']['Day'][date]['comment'] = test_str + self.check_json_get(post_url, expected) -- 2.30.2 From 26b862b518df1cfff05fc9c3d749c03cb842d07a Mon Sep 17 00:00:00 2001 From: Christian Heller Date: Fri, 21 Jun 2024 20:18:24 +0200 Subject: [PATCH 07/16] Re-organize and extend/improve POST/GET /day tests. --- tests/days.py | 194 ++++++++++++++++++++++++-------------------------- 1 file changed, 94 insertions(+), 100 deletions(-) diff --git a/tests/days.py b/tests/days.py index 0b039bf..396fe05 100644 --- a/tests/days.py +++ b/tests/days.py @@ -106,30 +106,29 @@ class TestsWithServer(TestCaseWithServer): return d @staticmethod - def todo_node_as_dict(todo_id: int) -> dict[str, object]: + def _todo_node_as_dict(todo_id: int) -> dict[str, object]: """Return JSON of TodoNode to expect.""" return {'children': [], 'seen': False, 'todo': todo_id} - @staticmethod - def GET_day_dict(date: str) -> dict[str, object]: + @classmethod + def GET_day_dict(cls, date: str) -> dict[str, object]: """Return JSON of GET /day to expect.""" day: dict[str, object] = {'id': date, 'comment': '', 'todos': []} - d: dict[str, object] - d = {'day': date, - 'top_nodes': [], - 'make_type': '', - 'enablers_for': {}, - 'disablers_for': {}, - 'conditions_present': [], - 'processes': [], - '_library': {'Day': TestsWithServer.as_refs([day])}} + d: dict[str, object] = {'day': date, + 'top_nodes': [], + 'make_type': '', + 'enablers_for': {}, + 'disablers_for': {}, + 'conditions_present': [], + 'processes': [], + '_library': {'Day': cls.as_refs([day])}} return d - def post_day(self, params: str = '', - form_data: None | dict[str, object] = None, - redir_to: str = '', - status: int = 302, - ) -> None: + def _post_day(self, params: str = '', + form_data: None | dict[str, object] = None, + redir_to: str = '', + status: int = 302, + ) -> None: """POST /day?{params} with form_data.""" if not form_data: form_data = {'day_comment': '', 'make_type': ''} @@ -139,12 +138,12 @@ class TestsWithServer(TestCaseWithServer): self.check_post(form_data, target, status, redir_to) @staticmethod - def post_batch(list_of_args: list[list[object]], - names_of_simples: list[str], - names_of_versioneds: list[str], - f_as_dict: Callable[..., dict[str, object]], - f_to_post: Callable[..., None | dict[str, object]] - ) -> list[dict[str, object]]: + def _post_batch(list_of_args: list[list[object]], + names_of_simples: list[str], + names_of_versioneds: list[str], + f_as_dict: Callable[..., dict[str, object]], + f_to_post: Callable[..., None | dict[str, object]] + ) -> list[dict[str, object]]: """Post expected=f_as_dict(*args) as input to f_to_post, for many.""" expecteds = [] for args in list_of_args: @@ -159,82 +158,6 @@ class TestsWithServer(TestCaseWithServer): f_to_post(expected['id'], post) return expecteds - def post_cond(self, id_: int, form_data: dict[str, object]) -> None: - """POST Condition of id_ with form_data.""" - self.check_post(form_data, f'/condition?id={id_}', 302) - - def test_do_GET_day_with_processes_and_todos(self) -> None: - """Test GET /day displaying Processes and Todos.""" - date = '2024-01-01' - # check Processes get displayed in ['processes'] and ['_library'] - procs_data = [[1, 'foo', 'oof', 1.1], [2, 'bar', 'rab', 0.9]] - procs_expected = self.post_batch(procs_data, [], - ['title', 'description', 'effort'], - self.proc_as_dict, self.post_process) - expected = self.GET_day_dict(date) - assert isinstance(expected['_library'], dict) - expected['processes'] = self.as_id_list(procs_expected) - expected['_library']['Process'] = self.as_refs(procs_expected) - self.post_day(f'date={date}') - self.check_json_get(f'/day?date={date}', expected) - # post Todos of either process and check their display - post_day: dict[str, object] - post_day = {'day_comment': '', 'make_type': '', 'new_todo': [1, 2]} - todos = [self.todo_as_dict(1, 1, date), self.todo_as_dict(2, 2, date)] - expected['_library']['Todo'] = self.as_refs(todos) - expected['_library']['Day'][date]['todos'] = self.as_id_list(todos) - nodes = [self.todo_node_as_dict(1), self.todo_node_as_dict(2)] - expected['top_nodes'] = nodes - self.post_day(f'date={date}', post_day) - self.check_json_get(f'/day?date={date}', expected) - # add a comment to one Todo and set the other's doneness and effort - post_day['new_todo'] = [] - post_day['todo_id'] = [1, 2] - post_day['done'] = [2] - post_day['comment'] = ['FOO', ''] - post_day['effort'] = ['2.3', ''] - expected['_library']['Todo']['1']['comment'] = 'FOO' - expected['_library']['Todo']['1']['effort'] = 2.3 - expected['_library']['Todo']['2']['is_done'] = True - self.post_day(f'date={date}', post_day) - self.check_json_get(f'/day?date={date}', expected) - - def test_do_GET_day_with_conditions(self) -> None: - """Test GET /day displaying Conditions and their relations.""" - # add Process with Conditions and their Todos, check display - conds_data = [[1, False, ['A'], ['a']], [2, True, ['B'], ['b']]] - conds_expected = self.post_batch(conds_data, ['is_active'], - ['title', 'description'], - self.cond_as_dict, self.post_cond) - cond_names = ['conditions', 'disables', 'blockers', 'enables'] - procs_data = [[1, 'foo', 'oof', 1.1, [1], [1], [2], [2]], - [2, 'bar', 'rab', 0.9, [2], [2], [1], [1]]] - procs_expected = self.post_batch(procs_data, cond_names, - ['title', 'description', 'effort'], - self.proc_as_dict, self.post_process) - date = '2024-01-01' - expected = self.GET_day_dict(date) - assert isinstance(expected['_library'], dict) - expected['processes'] = self.as_id_list(procs_expected) - expected['_library']['Process'] = self.as_refs(procs_expected) - expected['_library']['Condition'] = self.as_refs(conds_expected) - self.post_day(f'date={date}') - self.check_json_get(f'/day?date={date}', expected) - # add Todos in relation to Conditions, check consequences - post_day: dict[str, object] - post_day = {'day_comment': '', 'make_type': '', 'new_todo': [1, 2]} - todos = [self.todo_as_dict(1, 1, date, [1], [1], [2], [2]), - self.todo_as_dict(2, 2, date, [2], [2], [1], [1])] - expected['_library']['Todo'] = self.as_refs(todos) - expected['_library']['Day'][date]['todos'] = self.as_id_list(todos) - nodes = [self.todo_node_as_dict(1), self.todo_node_as_dict(2)] - expected['top_nodes'] = nodes - expected['disablers_for'] = {'1': [1], '2': [2]} - expected['enablers_for'] = {'1': [2], '2': [1]} - expected['conditions_present'] = self.as_id_list(conds_expected) - self.post_day(f'date={date}', post_day) - self.check_json_get(f'/day?date={date}', expected) - def test_do_GET_calendar(self) -> None: """Test /calendar response codes, and / redirect.""" self.check_get('/calendar', 200) @@ -265,7 +188,7 @@ class TestsWithServer(TestCaseWithServer): self.check_json_get(f'/day?date={date}', expected) # … not even when part of a POST request post: dict[str, object] = {'day_comment': '', 'make_type': 'foo'} - self.post_day(f'date={date}', post) + self._post_day(f'date={date}', post) self.check_json_get(f'/day?date={date}', expected) def test_fail_POST_day(self) -> None: @@ -344,3 +267,74 @@ class TestsWithServer(TestCaseWithServer): assert isinstance(expected['_library'], dict) expected['_library']['Day'][date]['comment'] = test_str self.check_json_get(post_url, expected) + + def test_GET_day_with_processes_and_todos(self) -> None: + """Test GET /day displaying Processes and Todos (no trees).""" + date = '2024-01-01' + # check Processes get displayed in ['processes'] and ['_library'] + procs_data = [[1, 'foo', 'oof', 1.1], [2, 'bar', 'rab', 0.9]] + procs_expected = self._post_batch(procs_data, [], + ['title', 'description', 'effort'], + self.proc_as_dict, self.post_process) + expected = self.GET_day_dict(date) + assert isinstance(expected['_library'], dict) + expected['processes'] = self.as_id_list(procs_expected) + expected['_library']['Process'] = self.as_refs(procs_expected) + self._post_day(f'date={date}') + self.check_json_get(f'/day?date={date}', expected) + # post Todos of either process and check their display + post_day: dict[str, object] + post_day = {'day_comment': '', 'make_type': '', 'new_todo': [1, 2]} + todos = [self.todo_as_dict(1, 1, date), self.todo_as_dict(2, 2, date)] + expected['_library']['Todo'] = self.as_refs(todos) + expected['_library']['Day'][date]['todos'] = self.as_id_list(todos) + nodes = [self._todo_node_as_dict(1), self._todo_node_as_dict(2)] + expected['top_nodes'] = nodes + self._post_day(f'date={date}', post_day) + self.check_json_get(f'/day?date={date}', expected) + # add a comment to one Todo and set the other's doneness and effort + post_day = {'day_comment': '', 'make_type': '', 'new_todo': [], + 'todo_id': [1, 2], 'done': [2], 'comment': ['FOO', ''], + 'effort': [2.3, '']} + expected['_library']['Todo']['1']['comment'] = 'FOO' + expected['_library']['Todo']['1']['effort'] = 2.3 + expected['_library']['Todo']['2']['is_done'] = True + self._post_day(f'date={date}', post_day) + self.check_json_get(f'/day?date={date}', expected) + + def test_GET_day_with_conditions(self) -> None: + """Test GET /day displaying Conditions and their relations.""" + date = '2024-01-01' + # add Process with Conditions and their Todos, check display + conds_data = [[1, False, ['A'], ['a']], [2, True, ['B'], ['b']]] + conds_expected = self._post_batch( + conds_data, ['is_active'], ['title', 'description'], + self.cond_as_dict, + lambda x, y: self.check_post(y, f'/condition?id={x}', 302)) + cond_names = ['conditions', 'disables', 'blockers', 'enables'] + procs_data = [[1, 'foo', 'oof', 1.1, [1], [1], [2], [2]], + [2, 'bar', 'rab', 0.9, [2], [2], [1], [1]]] + procs_expected = self._post_batch(procs_data, cond_names, + ['title', 'description', 'effort'], + self.proc_as_dict, self.post_process) + expected = self.GET_day_dict(date) + assert isinstance(expected['_library'], dict) + expected['processes'] = self.as_id_list(procs_expected) + expected['_library']['Process'] = self.as_refs(procs_expected) + expected['_library']['Condition'] = self.as_refs(conds_expected) + self._post_day(f'date={date}') + self.check_json_get(f'/day?date={date}', expected) + # add Todos in relation to Conditions, check consequences + post_day: dict[str, object] + post_day = {'day_comment': '', 'make_type': '', 'new_todo': [1, 2]} + todos = [self.todo_as_dict(1, 1, date, [1], [1], [2], [2]), + self.todo_as_dict(2, 2, date, [2], [2], [1], [1])] + expected['_library']['Todo'] = self.as_refs(todos) + expected['_library']['Day'][date]['todos'] = self.as_id_list(todos) + nodes = [self._todo_node_as_dict(1), self._todo_node_as_dict(2)] + expected['top_nodes'] = nodes + expected['disablers_for'] = {'1': [1], '2': [2]} + expected['enablers_for'] = {'1': [2], '2': [1]} + expected['conditions_present'] = self.as_id_list(conds_expected) + self._post_day(f'date={date}', post_day) + self.check_json_get(f'/day?date={date}', expected) -- 2.30.2 From 0274be2e09f4b9c9cffa9e6737a8b128e0fab76d Mon Sep 17 00:00:00 2001 From: Christian Heller Date: Fri, 21 Jun 2024 23:29:18 +0200 Subject: [PATCH 08/16] Fuse Day.by_id_or_create and Day.by_id, as valid_date call in the first one fits better into the second. --- plomtask/days.py | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/plomtask/days.py b/plomtask/days.py index 7e6a5cc..2320130 100644 --- a/plomtask/days.py +++ b/plomtask/days.py @@ -34,17 +34,17 @@ class Day(BaseModel[str]): day.todos = Todo.by_date(db_conn, day.id_) return day - @classmethod - def by_id_or_create(cls, db_conn: DatabaseConnection, id_: str | None - ) -> Day: - """Extend BaseModel.by_id to ensure date name translation.""" - assert isinstance(id_, str) - return super().by_id_or_create(db_conn, valid_date(id_)) - @classmethod def by_id(cls, db_conn: DatabaseConnection, id_: str) -> Day: - """Extend BaseModel.by_id checking for new/lost .todos.""" - day = super().by_id(db_conn, id_) + """Extend BaseModel.by_id + + Checks Todo.days_to_update if we need to a retrieved Day's .todos, + and also ensures we're looking for proper dates and not strings like + "yesterday" by enforcing the valid_date translation. + """ + assert isinstance(id_, str) + possibly_translated_date = valid_date(id_) + day = super().by_id(db_conn, possibly_translated_date) if day.id_ in Todo.days_to_update: Todo.days_to_update.remove(day.id_) day.todos = Todo.by_date(db_conn, day.id_) -- 2.30.2 From 57364f285cc2d26f3f3984f0d71e76af4e79e3d3 Mon Sep 17 00:00:00 2001 From: Christian Heller Date: Fri, 21 Jun 2024 23:31:35 +0200 Subject: [PATCH 09/16] Greatly expand GET /calendar tests. --- tests/days.py | 135 +++++++++++++++++++++++++++++++------------------- 1 file changed, 85 insertions(+), 50 deletions(-) diff --git a/tests/days.py b/tests/days.py index 396fe05..8e3768c 100644 --- a/tests/days.py +++ b/tests/days.py @@ -79,16 +79,43 @@ class TestsWithDB(TestCaseWithDB): class TestsWithServer(TestCaseWithServer): """Tests against our HTTP server/handler (and database).""" + @classmethod + def GET_day_dict(cls, date: str) -> dict[str, object]: + """Return JSON of GET /day to expect.""" + # day: dict[str, object] = {'id': date, 'comment': '', 'todos': []} + day = cls._day_as_dict(date) + d: dict[str, object] = {'day': date, + 'top_nodes': [], + 'make_type': '', + 'enablers_for': {}, + 'disablers_for': {}, + 'conditions_present': [], + 'processes': [], + '_library': {'Day': cls.as_refs([day])}} + return d + + @classmethod + def GET_calendar_dict(cls, start: int, end: int) -> dict[str, object]: + """Return JSON of GET /calendar to expect.""" + today_date = date_in_n_days(0) + start_date = date_in_n_days(start) + end_date = date_in_n_days(end) + dates = [date_in_n_days(i) for i in range(start, end+1)] + days = [cls._day_as_dict(d) for d in dates] + library = {'Day': cls.as_refs(days)} if len(days) > 0 else {} + return {'today': today_date, 'start': start_date, 'end': end_date, + 'days': dates, '_library': library} + @staticmethod - def todo_as_dict(id_: int = 1, - process_id: int = 1, - date: str = '2024-01-01', - conditions: None | list[int] = None, - disables: None | list[int] = None, - blockers: None | list[int] = None, - enables: None | list[int] = None - ) -> dict[str, object]: - """Return JSON of Process to expect.""" + def _todo_as_dict(id_: int = 1, + process_id: int = 1, + date: str = '2024-01-01', + conditions: None | list[int] = None, + disables: None | list[int] = None, + blockers: None | list[int] = None, + enables: None | list[int] = None + ) -> dict[str, object]: + """Return JSON of Todo to expect.""" # pylint: disable=too-many-arguments d = {'id': id_, 'date': date, @@ -110,32 +137,9 @@ class TestsWithServer(TestCaseWithServer): """Return JSON of TodoNode to expect.""" return {'children': [], 'seen': False, 'todo': todo_id} - @classmethod - def GET_day_dict(cls, date: str) -> dict[str, object]: - """Return JSON of GET /day to expect.""" - day: dict[str, object] = {'id': date, 'comment': '', 'todos': []} - d: dict[str, object] = {'day': date, - 'top_nodes': [], - 'make_type': '', - 'enablers_for': {}, - 'disablers_for': {}, - 'conditions_present': [], - 'processes': [], - '_library': {'Day': cls.as_refs([day])}} - return d - - def _post_day(self, params: str = '', - form_data: None | dict[str, object] = None, - redir_to: str = '', - status: int = 302, - ) -> None: - """POST /day?{params} with form_data.""" - if not form_data: - form_data = {'day_comment': '', 'make_type': ''} - target = f'/day?{params}' - if not redir_to: - redir_to = f'{target}&make_type={form_data["make_type"]}' - self.check_post(form_data, target, status, redir_to) + @staticmethod + def _day_as_dict(date: str) -> dict[str, object]: + return {'id': date, 'comment': '', 'todos': []} @staticmethod def _post_batch(list_of_args: list[list[object]], @@ -158,21 +162,24 @@ class TestsWithServer(TestCaseWithServer): f_to_post(expected['id'], post) return expecteds - def test_do_GET_calendar(self) -> None: - """Test /calendar response codes, and / redirect.""" - self.check_get('/calendar', 200) - self.check_get('/calendar?start=&end=', 200) - self.check_get('/calendar?start=today&end=today', 200) - self.check_get('/calendar?start=2024-01-01&end=2025-01-01', 200) - self.check_get('/calendar?start=foo', 400) - - def test_fail_GET_day(self) -> None: - """Test malformed/illegal GET /day requests.""" - self.check_get('/day?date=foo', 400) - self.check_get('/day?date=2024-02-30', 400) + def _post_day(self, params: str = '', + form_data: None | dict[str, object] = None, + redir_to: str = '', + status: int = 302, + ) -> None: + """POST /day?{params} with form_data.""" + if not form_data: + form_data = {'day_comment': '', 'make_type': ''} + target = f'/day?{params}' + if not redir_to: + redir_to = f'{target}&make_type={form_data["make_type"]}' + self.check_post(form_data, target, status, redir_to) def test_basic_GET_day(self) -> None: """Test basic (no Processes/Conditions/Todos) GET /day basics.""" + # check illegal date parameters + self.check_get('/day?date=foo', 400) + self.check_get('/day?date=2024-02-30', 400) # check undefined day date = date_in_n_days(0) expected = self.GET_day_dict(date) @@ -285,7 +292,8 @@ class TestsWithServer(TestCaseWithServer): # post Todos of either process and check their display post_day: dict[str, object] post_day = {'day_comment': '', 'make_type': '', 'new_todo': [1, 2]} - todos = [self.todo_as_dict(1, 1, date), self.todo_as_dict(2, 2, date)] + todos = [self._todo_as_dict(1, 1, date), + self._todo_as_dict(2, 2, date)] expected['_library']['Todo'] = self.as_refs(todos) expected['_library']['Day'][date]['todos'] = self.as_id_list(todos) nodes = [self._todo_node_as_dict(1), self._todo_node_as_dict(2)] @@ -327,8 +335,8 @@ class TestsWithServer(TestCaseWithServer): # add Todos in relation to Conditions, check consequences post_day: dict[str, object] post_day = {'day_comment': '', 'make_type': '', 'new_todo': [1, 2]} - todos = [self.todo_as_dict(1, 1, date, [1], [1], [2], [2]), - self.todo_as_dict(2, 2, date, [2], [2], [1], [1])] + todos = [self._todo_as_dict(1, 1, date, [1], [1], [2], [2]), + self._todo_as_dict(2, 2, date, [2], [2], [1], [1])] expected['_library']['Todo'] = self.as_refs(todos) expected['_library']['Day'][date]['todos'] = self.as_id_list(todos) nodes = [self._todo_node_as_dict(1), self._todo_node_as_dict(2)] @@ -338,3 +346,30 @@ class TestsWithServer(TestCaseWithServer): expected['conditions_present'] = self.as_id_list(conds_expected) self._post_day(f'date={date}', post_day) self.check_json_get(f'/day?date={date}', expected) + + def test_GET_calendar(self) -> None: + """Test GET /calendar responses based on various inputs, DB states.""" + # check illegal date range delimiters + self.check_get('/calendar?start=foo', 400) + self.check_get('/calendar?end=foo', 400) + # check default range without saved days + expected = self.GET_calendar_dict(-1, 366) + self.check_json_get('/calendar', expected) + self.check_json_get('/calendar?start=&end=', expected) + # check named days as delimiters + expected = self.GET_calendar_dict(-1, +1) + self.check_json_get('/calendar?start=yesterday&end=tomorrow', expected) + # check zero-element range + expected = self.GET_calendar_dict(+1, 0) + self.check_json_get('/calendar?start=tomorrow&end=today', expected) + # check saved day shows up in results with proven by its comment + post_day: dict[str, object] = {'day_comment': 'foo', 'make_type': ''} + date1 = date_in_n_days(-2) + self._post_day(f'date={date1}', post_day) + start_date = date_in_n_days(-5) + end_date = date_in_n_days(+5) + url = f'/calendar?start={start_date}&end={end_date}' + expected = self.GET_calendar_dict(-5, +5) + assert isinstance(expected['_library'], dict) + expected['_library']['Day'][date1]['comment'] = post_day['day_comment'] + self.check_json_get(url, expected) -- 2.30.2 From 501b2ef5f6373807b7728e7b8539105aa9030809 Mon Sep 17 00:00:00 2001 From: Christian Heller Date: Fri, 21 Jun 2024 23:46:25 +0200 Subject: [PATCH 10/16] Minor reorganization of GET handlers code. --- plomtask/http.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/plomtask/http.py b/plomtask/http.py index 2706603..0f5e88e 100644 --- a/plomtask/http.py +++ b/plomtask/http.py @@ -424,8 +424,8 @@ class TaskHandler(BaseHTTPRequestHandler): def do_GET_conditions(self) -> dict[str, object]: """Show all Conditions.""" pattern = self._params.get_str('pattern') - conditions = Condition.matching(self.conn, pattern) sort_by = self._params.get_str('sort_by') + conditions = Condition.matching(self.conn, pattern) if sort_by == 'is_active': conditions.sort(key=lambda c: c.is_active) elif sort_by == '-is_active': @@ -462,15 +462,17 @@ class TaskHandler(BaseHTTPRequestHandler): @_get_item(Process) def do_GET_process(self, process: Process) -> dict[str, object]: """Show Process of ?id=.""" + owner_ids = self._params.get_all_int('step_to') + owned_ids = self._params.get_all_int('has_step') title_64 = self._params.get_str('title_b64') if title_64: title = b64decode(title_64.encode()).decode() process.title.set(title) owners = process.used_as_step_by(self.conn) - for step_id in self._params.get_all_int('step_to'): + for step_id in owner_ids: owners += [Process.by_id(self.conn, step_id)] preset_top_step = None - for process_id in self._params.get_all_int('has_step'): + for process_id in owned_ids: preset_top_step = process_id return {'process': process, 'is_new': process.id_ is None, 'preset_top_step': preset_top_step, @@ -497,8 +499,8 @@ class TaskHandler(BaseHTTPRequestHandler): def do_GET_processes(self) -> dict[str, object]: """Show all Processes.""" pattern = self._params.get_str('pattern') - processes = Process.matching(self.conn, pattern) sort_by = self._params.get_str('sort_by') + processes = Process.matching(self.conn, pattern) if sort_by == 'steps': processes.sort(key=lambda p: len(p.explicit_steps)) elif sort_by == '-steps': -- 2.30.2 From 692bfbac8d81ad5f1f0210e550dcabd15c58e8a5 Mon Sep 17 00:00:00 2001 From: Christian Heller Date: Sat, 22 Jun 2024 00:16:33 +0200 Subject: [PATCH 11/16] Refactor BaseModel sorting from GET handlers into class definitions. --- plomtask/conditions.py | 2 ++ plomtask/db.py | 19 ++++++++++++++++- plomtask/http.py | 46 +++--------------------------------------- plomtask/processes.py | 4 ++++ plomtask/todos.py | 4 ++++ 5 files changed, 31 insertions(+), 44 deletions(-) diff --git a/plomtask/conditions.py b/plomtask/conditions.py index b60d0af..15dcb9d 100644 --- a/plomtask/conditions.py +++ b/plomtask/conditions.py @@ -12,6 +12,8 @@ class Condition(BaseModel[int]): to_save_versioned = ['title', 'description'] to_search = ['title.newest', 'description.newest'] can_create_by_id = True + sorters = {'is_active': lambda c: c.is_active, + 'title': lambda c: c.title.newest} def __init__(self, id_: int | None, is_active: bool = False) -> None: super().__init__(id_) diff --git a/plomtask/db.py b/plomtask/db.py index 71c59d5..13cdaef 100644 --- a/plomtask/db.py +++ b/plomtask/db.py @@ -4,7 +4,7 @@ from os import listdir from os.path import isfile from difflib import Differ from sqlite3 import connect as sql_connect, Cursor, Row -from typing import Any, Self, TypeVar, Generic +from typing import Any, Self, TypeVar, Generic, Callable from plomtask.exceptions import HandledException, NotFoundException from plomtask.dating import valid_date @@ -241,6 +241,7 @@ class BaseModel(Generic[BaseModelId]): to_search: list[str] = [] can_create_by_id = False _exists = True + sorters: dict[str, Callable[..., Any]] = {} def __init__(self, id_: BaseModelId | None) -> None: if isinstance(id_, int) and id_ < 1: @@ -335,6 +336,22 @@ class BaseModel(Generic[BaseModelId]): """Convenience method to return cls' name in lowercase.""" return cls.__name__.lower() + @classmethod + def sort_by(cls, seq: list[Any], sort_key: str, default: str = 'title' + ) -> str: + """Sort cls list by cls.sorters[sort_key] (reverse if '-'-prefixed).""" + reverse = False + if len(sort_key) > 1 and '-' == sort_key[0]: + sort_key = sort_key[1:] + reverse = True + if sort_key not in cls.sorters: + sort_key = default + sorter: Callable[..., Any] = cls.sorters[sort_key] + seq.sort(key=sorter, reverse=reverse) + if reverse: + sort_key = f'-{sort_key}' + return sort_key + # cache management # (we primarily use the cache to ensure we work on the same object in # memory no matter where and how we retrieve it, e.g. we don't want diff --git a/plomtask/http.py b/plomtask/http.py index 0f5e88e..417c5a6 100644 --- a/plomtask/http.py +++ b/plomtask/http.py @@ -400,23 +400,7 @@ class TaskHandler(BaseHTTPRequestHandler): todos = [t for t in todos_by_date_range if comment_pattern in t.comment and ((not process_id) or t.process.id_ == process_id)] - if sort_by == 'doneness': - todos.sort(key=lambda t: t.is_done) - elif sort_by == '-doneness': - todos.sort(key=lambda t: t.is_done, reverse=True) - elif sort_by == 'title': - todos.sort(key=lambda t: t.title_then) - elif sort_by == '-title': - todos.sort(key=lambda t: t.title_then, reverse=True) - elif sort_by == 'comment': - todos.sort(key=lambda t: t.comment) - elif sort_by == '-comment': - todos.sort(key=lambda t: t.comment, reverse=True) - elif sort_by == '-date': - todos.sort(key=lambda t: t.date, reverse=True) - else: - todos.sort(key=lambda t: t.date) - sort_by = 'title' + sort_by = Todo.sort_by(todos, sort_by) return {'start': start, 'end': end, 'process_id': process_id, 'comment_pattern': comment_pattern, 'todos': todos, 'all_processes': Process.all(self.conn), 'sort_by': sort_by} @@ -426,15 +410,7 @@ class TaskHandler(BaseHTTPRequestHandler): pattern = self._params.get_str('pattern') sort_by = self._params.get_str('sort_by') conditions = Condition.matching(self.conn, pattern) - if sort_by == 'is_active': - conditions.sort(key=lambda c: c.is_active) - elif sort_by == '-is_active': - conditions.sort(key=lambda c: c.is_active, reverse=True) - elif sort_by == '-title': - conditions.sort(key=lambda c: c.title.newest, reverse=True) - else: - conditions.sort(key=lambda c: c.title.newest) - sort_by = 'title' + sort_by = Condition.sort_by(conditions, sort_by) return {'conditions': conditions, 'sort_by': sort_by, 'pattern': pattern} @@ -501,23 +477,7 @@ class TaskHandler(BaseHTTPRequestHandler): pattern = self._params.get_str('pattern') sort_by = self._params.get_str('sort_by') processes = Process.matching(self.conn, pattern) - if sort_by == 'steps': - processes.sort(key=lambda p: len(p.explicit_steps)) - elif sort_by == '-steps': - processes.sort(key=lambda p: len(p.explicit_steps), reverse=True) - elif sort_by == 'owners': - processes.sort(key=lambda p: p.n_owners or 0) - elif sort_by == '-owners': - processes.sort(key=lambda p: p.n_owners or 0, reverse=True) - elif sort_by == 'effort': - processes.sort(key=lambda p: p.effort.newest) - elif sort_by == '-effort': - processes.sort(key=lambda p: p.effort.newest, reverse=True) - elif sort_by == '-title': - processes.sort(key=lambda p: p.title.newest, reverse=True) - else: - processes.sort(key=lambda p: p.title.newest) - sort_by = 'title' + sort_by = Process.sort_by(processes, sort_by) return {'processes': processes, 'sort_by': sort_by, 'pattern': pattern} # POST handlers diff --git a/plomtask/processes.py b/plomtask/processes.py index 3615899..bb1de3a 100644 --- a/plomtask/processes.py +++ b/plomtask/processes.py @@ -36,6 +36,10 @@ class Process(BaseModel[int], ConditionsRelations): add_to_dict = ['explicit_steps'] to_search = ['title.newest', 'description.newest'] can_create_by_id = True + sorters = {'steps': lambda p: len(p.explicit_steps), + 'owners': lambda p: p.n_owners, + 'effort': lambda p: p.effort.newest, + 'title': lambda p: p.title.newest} def __init__(self, id_: int | None, calendarize: bool = False) -> None: BaseModel.__init__(self, id_) diff --git a/plomtask/todos.py b/plomtask/todos.py index 4d7e393..f5388b5 100644 --- a/plomtask/todos.py +++ b/plomtask/todos.py @@ -51,6 +51,10 @@ class Todo(BaseModel[int], ConditionsRelations): days_to_update: Set[str] = set() children: list[Todo] parents: list[Todo] + sorters = {'doneness': lambda t: t.is_done, + 'title': lambda t: t.title_then, + 'comment': lambda t: t.comment, + 'date': lambda t: t.date} # pylint: disable=too-many-arguments def __init__(self, id_: int | None, -- 2.30.2 From 7d5347d7bbbbfaf0cbec191ef2dc1ff9cda41ac3 Mon Sep 17 00:00:00 2001 From: Christian Heller Date: Sat, 22 Jun 2024 04:43:08 +0200 Subject: [PATCH 12/16] Empty cache more often to avoid race conditions. --- plomtask/http.py | 39 ++++++++++++++++++++++++++++++++++++--- 1 file changed, 36 insertions(+), 3 deletions(-) diff --git a/plomtask/http.py b/plomtask/http.py index 417c5a6..0626b4c 100644 --- a/plomtask/http.py +++ b/plomtask/http.py @@ -177,6 +177,38 @@ class TaskHandler(BaseHTTPRequestHandler): @staticmethod def _request_wrapper(http_method: str, not_found_msg: str ) -> Callable[..., Callable[[TaskHandler], None]]: + """Wrapper for do_GET… and do_POST… handlers, to init and clean up. + + Among other things, conditionally cleans all caches, but only on POST + requests, as only those are expected to change the states of objects + that may be cached, and certainly only those are expected to write any + changes to the database. We want to call them as early though as + possible here, either exactly after the specific request handler + returns successfully, or right after any exception is triggered – + otherwise, race conditions become plausible. + + Note that any POST attempt, even a failed one, may end in problematic + inconsistencies: + + - if the POST handler experiences an Exception, changes to objects + won't get written to the DB, but the changed objects may remain in + the cache and affect other objects despite their possibly illegal + state + + - even if an object was just saved to the DB, we cannot be sure its + current state is completely identical to what we'd get if loading it + fresh from the DB (e.g. currently Process.n_owners is only updated + when loaded anew via .from_table_row, nor is its state written to + the DB by .save; a questionable design choice, but proof that we + have no guarantee that objects' .save stores all their states we'd + prefer at their most up-to-date. + """ + + def clear_caches() -> None: + for cls in (Day, Todo, Condition, Process, ProcessStep): + assert hasattr(cls, 'empty_cache') + cls.empty_cache() + def decorator(f: Callable[..., str | None] ) -> Callable[[TaskHandler], None]: def wrapper(self: TaskHandler) -> None: @@ -193,6 +225,8 @@ class TaskHandler(BaseHTTPRequestHandler): if hasattr(self, handler_name): handler = getattr(self, handler_name) redir_target = f(self, handler) + if 'POST' != http_method: + clear_caches() if redir_target: self.send_response(302) self.send_header('Location', redir_target) @@ -201,9 +235,8 @@ class TaskHandler(BaseHTTPRequestHandler): msg = f'{not_found_msg}: {self._site}' raise NotFoundException(msg) except HandledException as error: - for cls in (Day, Todo, Condition, Process, ProcessStep): - assert hasattr(cls, 'empty_cache') - cls.empty_cache() + if 'POST' != http_method: + clear_caches() ctx = {'msg': error} self._send_page(ctx, 'msg', error.http_code) finally: -- 2.30.2 From a71dbd0fdbc8a03d400c59d0446595a995301d07 Mon Sep 17 00:00:00 2001 From: Christian Heller Date: Sat, 22 Jun 2024 04:49:29 +0200 Subject: [PATCH 13/16] In GET /process handler, catch malformed ?title_b64= params. --- plomtask/http.py | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/plomtask/http.py b/plomtask/http.py index 0626b4c..b7040f7 100644 --- a/plomtask/http.py +++ b/plomtask/http.py @@ -3,6 +3,7 @@ from __future__ import annotations from dataclasses import dataclass from typing import Any, Callable from base64 import b64encode, b64decode +from binascii import Error as binascii_Exception from http.server import BaseHTTPRequestHandler from http.server import HTTPServer from urllib.parse import urlparse, parse_qs @@ -187,8 +188,8 @@ class TaskHandler(BaseHTTPRequestHandler): returns successfully, or right after any exception is triggered – otherwise, race conditions become plausible. - Note that any POST attempt, even a failed one, may end in problematic - inconsistencies: + Note that otherwise any POST attempt, even a failed one, may end in + problematic inconsistencies: - if the POST handler experiences an Exception, changes to objects won't get written to the DB, but the changed objects may remain in @@ -225,8 +226,8 @@ class TaskHandler(BaseHTTPRequestHandler): if hasattr(self, handler_name): handler = getattr(self, handler_name) redir_target = f(self, handler) - if 'POST' != http_method: - clear_caches() + if 'POST' == http_method: + clear_caches() if redir_target: self.send_response(302) self.send_header('Location', redir_target) @@ -235,8 +236,8 @@ class TaskHandler(BaseHTTPRequestHandler): msg = f'{not_found_msg}: {self._site}' raise NotFoundException(msg) except HandledException as error: - if 'POST' != http_method: - clear_caches() + if 'POST' == http_method: + clear_caches() ctx = {'msg': error} self._send_page(ctx, 'msg', error.http_code) finally: @@ -475,13 +476,18 @@ class TaskHandler(BaseHTTPRequestHandler): owned_ids = self._params.get_all_int('has_step') title_64 = self._params.get_str('title_b64') if title_64: - title = b64decode(title_64.encode()).decode() + try: + title = b64decode(title_64.encode()).decode() + except binascii_Exception as exc: + msg = 'invalid base64 for ?title_b64=' + raise BadFormatException(msg) from exc process.title.set(title) + preset_top_step = None owners = process.used_as_step_by(self.conn) for step_id in owner_ids: owners += [Process.by_id(self.conn, step_id)] - preset_top_step = None for process_id in owned_ids: + Process.by_id(self.conn, process_id) # to ensure ID exists preset_top_step = process_id return {'process': process, 'is_new': process.id_ is None, 'preset_top_step': preset_top_step, -- 2.30.2 From f89df720c8d827624e3829b1c13bf9f76f301474 Mon Sep 17 00:00:00 2001 From: Christian Heller Date: Sat, 22 Jun 2024 04:51:52 +0200 Subject: [PATCH 14/16] Adapt tests to new cache emptying reality. --- tests/processes.py | 8 ++++---- tests/todos.py | 8 ++++++++ 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/tests/processes.py b/tests/processes.py index 9e266e0..2f3ebcc 100644 --- a/tests/processes.py +++ b/tests/processes.py @@ -337,11 +337,11 @@ class TestsWithServer(TestCaseWithServer): self.post_process(1, form_data_1) retrieved_process = Process.by_id(self.db_conn, 1) self.assertEqual(len(retrieved_process.explicit_steps), 2) - retrieved_step_0 = retrieved_process.explicit_steps[0] + retrieved_step_0 = retrieved_process.explicit_steps[1] self.assertEqual(retrieved_step_0.step_process_id, 3) self.assertEqual(retrieved_step_0.owner_id, 1) self.assertEqual(retrieved_step_0.parent_step_id, None) - retrieved_step_1 = retrieved_process.explicit_steps[1] + retrieved_step_1 = retrieved_process.explicit_steps[0] self.assertEqual(retrieved_step_1.step_process_id, 2) self.assertEqual(retrieved_step_1.owner_id, 1) self.assertEqual(retrieved_step_1.parent_step_id, None) @@ -369,11 +369,11 @@ class TestsWithServer(TestCaseWithServer): self.post_process(1, form_data_1) retrieved_process = Process.by_id(self.db_conn, 1) self.assertEqual(len(retrieved_process.explicit_steps), 3) - retrieved_step_0 = retrieved_process.explicit_steps[0] + retrieved_step_0 = retrieved_process.explicit_steps[1] self.assertEqual(retrieved_step_0.step_process_id, 2) self.assertEqual(retrieved_step_0.owner_id, 1) self.assertEqual(retrieved_step_0.parent_step_id, None) - retrieved_step_1 = retrieved_process.explicit_steps[1] + retrieved_step_1 = retrieved_process.explicit_steps[0] self.assertEqual(retrieved_step_1.step_process_id, 3) self.assertEqual(retrieved_step_1.owner_id, 1) self.assertEqual(retrieved_step_1.parent_step_id, None) diff --git a/tests/todos.py b/tests/todos.py index 0998c69..dd57ee4 100644 --- a/tests/todos.py +++ b/tests/todos.py @@ -245,19 +245,25 @@ class TestsWithServer(TestCaseWithServer): form_data = {'day_comment': '', 'make_type': 'full'} self.check_post(form_data, '/day?date=2024-01-01&make_type=full', 302) self.assertEqual(Todo.by_date(self.db_conn, '2024-01-01'), []) + proc = Process.by_id(self.db_conn, 1) form_data['new_todo'] = str(proc.id_) self.check_post(form_data, '/day?date=2024-01-01&make_type=full', 302) todos = Todo.by_date(self.db_conn, '2024-01-01') self.assertEqual(1, len(todos)) todo1 = todos[0] self.assertEqual(todo1.id_, 1) + proc = Process.by_id(self.db_conn, 1) self.assertEqual(todo1.process.id_, proc.id_) self.assertEqual(todo1.is_done, False) + proc2 = Process.by_id(self.db_conn, 2) form_data['new_todo'] = str(proc2.id_) self.check_post(form_data, '/day?date=2024-01-01&make_type=full', 302) todos = Todo.by_date(self.db_conn, '2024-01-01') todo1 = todos[1] self.assertEqual(todo1.id_, 2) + proc2 = Process.by_id(self.db_conn, 1) + todo1 = Todo.by_date(self.db_conn, '2024-01-01')[0] + self.assertEqual(todo1.id_, 1) self.assertEqual(todo1.process.id_, proc2.id_) self.assertEqual(todo1.is_done, False) @@ -403,10 +409,12 @@ class TestsWithServer(TestCaseWithServer): form_data = {'day_comment': '', 'todo_id': [1], 'make_type': 'full', 'comment': [''], 'done': [], 'effort': ['']} self.check_post(form_data, '/day?date=2024-01-01&make_type=full', 302) + todo = Todo.by_date(self.db_conn, '2024-01-01')[0] self.assertEqual(todo.is_done, False) form_data = {'day_comment': '', 'todo_id': [1], 'done': [1], 'make_type': 'full', 'comment': [''], 'effort': ['']} self.check_post(form_data, '/day?date=2024-01-01&make_type=full', 302) + todo = Todo.by_date(self.db_conn, '2024-01-01')[0] self.assertEqual(todo.is_done, True) def test_do_GET_todo(self) -> None: -- 2.30.2 From 7fe796242661ac9676bba745698f374a109b9440 Mon Sep 17 00:00:00 2001 From: Christian Heller Date: Sat, 22 Jun 2024 04:52:26 +0200 Subject: [PATCH 15/16] Expand GET /process and /processes tests. --- tests/processes.py | 122 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 122 insertions(+) diff --git a/tests/processes.py b/tests/processes.py index 2f3ebcc..1b20e21 100644 --- a/tests/processes.py +++ b/tests/processes.py @@ -1,4 +1,5 @@ """Test Processes module.""" +from typing import Any from tests.utils import TestCaseWithDB, TestCaseWithServer, TestCaseSansDB from plomtask.processes import Process, ProcessStep, ProcessStepsNode from plomtask.conditions import Condition @@ -384,5 +385,126 @@ class TestsWithServer(TestCaseWithServer): def test_do_GET(self) -> None: """Test /process and /processes response codes.""" + self.check_get('/process', 200) + self.check_get('/process?id=', 200) + self.check_get('/process?id=1', 200) self.check_get_defaults('/process') self.check_get('/processes', 200) + + def test_fail_GET_process(self) -> None: + """Test invalid GET /process params.""" + # check for invalid IDs + self.check_get('/process?id=foo', 400) + self.check_get('/process?id=0', 500) + # check we catch invalid base64 + self.check_get('/process?title_b64=foo', 400) + # check failure on references to unknown processes; we create Process + # of ID=1 here so we know the 404 comes from step_to=2 etc. (that tie + # the Process displayed by /process to others), not from not finding + # the main Process itself + self.post_process(1) + self.check_get('/process?id=1&step_to=2', 404) + self.check_get('/process?id=1&has_step=2', 404) + + @classmethod + def GET_processes_dict(cls, procs: list[dict[str, object]] + ) -> dict[str, object]: + """Return JSON of GET /processes to expect.""" + library = {'Process': cls.as_refs(procs)} if procs else {} + d: dict[str, object] = {'processes': cls.as_id_list(procs), + 'sort_by': 'title', + 'pattern': '', + '_library': library} + return d + + @staticmethod + def procstep_as_dict(id_: int, + owner_id: int, + step_process_id: int, + parent_step_id: int | None = None + ) -> dict[str, object]: + """Return JSON of Process to expect.""" + return {'id': id_, + 'owner_id': owner_id, + 'step_process_id': step_process_id, + 'parent_step_id': parent_step_id} + + def test_GET_processes(self) -> None: + """Test GET /processes.""" + # pylint: disable=too-many-statements + # test empty result on empty DB, default-settings on empty params + expected = self.GET_processes_dict([]) + self.check_json_get('/processes', expected) + # test on meaningless non-empty params (incl. entirely un-used key), + # that 'sort_by' default to 'title' (even if set to something else, as + # long as without handler) and 'pattern' get preserved + expected['pattern'] = 'bar' # preserved despite zero effect! + url = '/processes?sort_by=foo&pattern=bar&foo=x' + self.check_json_get(url, expected) + # test non-empty result, automatic (positive) sorting by title + post1: dict[str, Any] + post2: dict[str, Any] + post3: dict[str, Any] + post1 = {'title': 'foo', 'description': 'oof', 'effort': 1.0} + post2 = {'title': 'bar', 'description': 'rab', 'effort': 1.1} + post2['new_top_step'] = 1 + post3 = {'title': 'baz', 'description': 'zab', 'effort': 0.9} + post3['new_top_step'] = 1 + self.post_process(1, post1) + self.post_process(2, post2) + self.post_process(3, post3) + post3['new_top_step'] = 2 + post3['keep_step'] = 2 + post3['steps'] = [2] + post3['step_2_process_id'] = 1 + self.post_process(3, post3) + proc1 = self.proc_as_dict(1, post1['title'], + post1['description'], post1['effort']) + proc2 = self.proc_as_dict(2, post2['title'], + post2['description'], post2['effort']) + proc3 = self.proc_as_dict(3, post3['title'], + post3['description'], post3['effort']) + proc2['explicit_steps'] = [1] + proc3['explicit_steps'] = [2, 3] + step1 = self.procstep_as_dict(1, 2, 1) + step2 = self.procstep_as_dict(2, 3, 1) + step3 = self.procstep_as_dict(3, 3, 2) + expected = self.GET_processes_dict([proc2, proc3, proc1]) + assert isinstance(expected['_library'], dict) + expected['_library']['ProcessStep'] = self.as_refs([step1, step2, + step3]) + self.check_json_get('/processes', expected) + # test other sortings + expected['sort_by'] = '-title' + expected['processes'] = self.as_id_list([proc1, proc3, proc2]) + self.check_json_get('/processes?sort_by=-title', expected) + expected['sort_by'] = 'effort' + expected['processes'] = self.as_id_list([proc3, proc1, proc2]) + self.check_json_get('/processes?sort_by=effort', expected) + expected['sort_by'] = '-effort' + expected['processes'] = self.as_id_list([proc2, proc1, proc3]) + self.check_json_get('/processes?sort_by=-effort', expected) + expected['sort_by'] = 'steps' + expected['processes'] = self.as_id_list([proc1, proc2, proc3]) + self.check_json_get('/processes?sort_by=steps', expected) + expected['sort_by'] = '-steps' + expected['processes'] = self.as_id_list([proc3, proc2, proc1]) + self.check_json_get('/processes?sort_by=-steps', expected) + expected['sort_by'] = 'owners' + expected['processes'] = self.as_id_list([proc3, proc2, proc1]) + self.check_json_get('/processes?sort_by=owners', expected) + expected['sort_by'] = '-owners' + expected['processes'] = self.as_id_list([proc1, proc2, proc3]) + self.check_json_get('/processes?sort_by=-owners', expected) + # test pattern matching on title + expected = self.GET_processes_dict([proc2, proc3]) + assert isinstance(expected['_library'], dict) + expected['pattern'] = 'ba' + expected['_library']['ProcessStep'] = self.as_refs([step1, step2, + step3]) + self.check_json_get('/processes?pattern=ba', expected) + # test pattern matching on description + expected['processes'] = self.as_id_list([proc1]) + expected['_library'] = {'Process': self.as_refs([proc1])} + expected['pattern'] = 'of' + self.check_json_get('/processes?pattern=of', expected) -- 2.30.2 From 44812c864c635474d117d61a2ea56f8f10f743eb Mon Sep 17 00:00:00 2001 From: Christian Heller Date: Sat, 22 Jun 2024 04:53:16 +0200 Subject: [PATCH 16/16] Slightly improve and re-organize Condition tests. --- tests/conditions.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/tests/conditions.py b/tests/conditions.py index bf04f7b..9b3a403 100644 --- a/tests/conditions.py +++ b/tests/conditions.py @@ -77,7 +77,7 @@ class TestsWithServer(TestCaseWithServer): valid_payload = {'title': '', 'description': '', 'is_active': False} self.check_post(valid_payload, '/condition?id=foo', 400) - def test_do_POST_condition(self) -> None: + def test_POST_condition(self) -> None: """Test (valid) POST /condition and its effect on GET /condition[s].""" # test valid POST's effect on … post = {'title': 'foo', 'description': 'oof', 'is_active': False} @@ -110,7 +110,7 @@ class TestsWithServer(TestCaseWithServer): expected_all['_library'] = {} self.check_json_get('/conditions', expected_all) - def test_do_GET_condition(self) -> None: + def test_GET_condition(self) -> None: """More GET /condition testing, especially for Process relations.""" # check expected default status codes self.check_get_defaults('/condition') @@ -141,7 +141,7 @@ class TestsWithServer(TestCaseWithServer): expected['_library']['Process'] = self.as_refs([proc1, proc2]) self.check_json_get('/condition?id=1', expected) - def test_do_GET_conditions(self) -> None: + def test_GET_conditions(self) -> None: """Test GET /conditions.""" # test empty result on empty DB, default-settings on empty params expected = self.GET_conditions_dict([]) @@ -167,14 +167,14 @@ class TestsWithServer(TestCaseWithServer): # test other sortings # (NB: by .is_active has two items of =False, their order currently # is not explicitly made predictable, so mail fail until we do) - expected['conditions'] = self.as_id_list([cond1, cond3, cond2]) expected['sort_by'] = '-title' + expected['conditions'] = self.as_id_list([cond1, cond3, cond2]) self.check_json_get('/conditions?sort_by=-title', expected) - expected['conditions'] = self.as_id_list([cond1, cond2, cond3]) expected['sort_by'] = 'is_active' + expected['conditions'] = self.as_id_list([cond1, cond2, cond3]) self.check_json_get('/conditions?sort_by=is_active', expected) - expected['conditions'] = self.as_id_list([cond3, cond1, cond2]) expected['sort_by'] = '-is_active' + expected['conditions'] = self.as_id_list([cond3, cond1, cond2]) self.check_json_get('/conditions?sort_by=-is_active', expected) # test pattern matching on title expected = self.GET_conditions_dict([cond2, cond3]) @@ -184,5 +184,5 @@ class TestsWithServer(TestCaseWithServer): assert isinstance(expected['_library'], dict) expected['conditions'] = self.as_id_list([cond1]) expected['_library']['Condition'] = self.as_refs([cond1]) - expected['pattern'] = 'oo' - self.check_json_get('/conditions?pattern=oo', expected) + expected['pattern'] = 'of' + self.check_json_get('/conditions?pattern=of', expected) -- 2.30.2