From 624e0ebf14db9065606d9c8fe9f0abe0dcd547ae Mon Sep 17 00:00:00 2001 From: Spyros Date: Sat, 14 Sep 2024 18:20:58 +0100 Subject: [PATCH 1/6] fix bug 552 by ensuring that ALL db cre links are from Higher->Lower and of type either contains or related, no more lower->higher of type partof since this is prone to bugs, part of is now an api construct --- application/cmd/cre_main.py | 1 + 1 file changed, 1 insertion(+) diff --git a/application/cmd/cre_main.py b/application/cmd/cre_main.py index f9287953..da04b3ee 100644 --- a/application/cmd/cre_main.py +++ b/application/cmd/cre_main.py @@ -117,6 +117,7 @@ def register_cre(cre: defs.CRE, collection: db.Node_collection) -> Tuple[db.CRE, dbcre: db.CRE = collection.add_cre(cre) for link in cre.links: if type(link.document) == defs.CRE: + logger.info(f"{link.document.id} {link.ltype} {cre.id}") other_cre, _ = register_cre(link.document, collection) # the following flips the PartOf relationship so that we only have contains relationship in the database From 930c7229217e7090c646ccc4959f00b8f8646843 Mon Sep 17 00:00:00 2001 From: Spyros Date: Fri, 20 Sep 2024 19:59:08 +0100 Subject: [PATCH 2/6] nit: replace variable with constant enum in register_cre, code quality only --- application/cmd/cre_main.py | 1 - 1 file changed, 1 deletion(-) diff --git a/application/cmd/cre_main.py b/application/cmd/cre_main.py index da04b3ee..f9287953 100644 --- a/application/cmd/cre_main.py +++ b/application/cmd/cre_main.py @@ -117,7 +117,6 @@ def register_cre(cre: defs.CRE, collection: db.Node_collection) -> Tuple[db.CRE, dbcre: db.CRE = collection.add_cre(cre) for link in cre.links: if type(link.document) == defs.CRE: - logger.info(f"{link.document.id} {link.ltype} {cre.id}") other_cre, _ = register_cre(link.document, collection) # the following flips the PartOf relationship so that we only have contains relationship in the database From c4cbf57a3caacf872bf6cf00eb2d1492325d04b9 Mon Sep 17 00:00:00 2001 From: Spyros Date: Sat, 28 Sep 2024 11:40:16 +0100 Subject: [PATCH 3/6] provide better exception errors when importing csvs and lines repeat --- application/defs/cre_defs.py | 17 ++++++++++++++--- application/defs/cre_exceptions.py | 5 +++++ application/web/web_main.py | 8 ++++++-- 3 files changed, 25 insertions(+), 5 deletions(-) diff --git a/application/defs/cre_defs.py b/application/defs/cre_defs.py index 3eedc800..9e8f00a3 100644 --- a/application/defs/cre_defs.py +++ b/application/defs/cre_defs.py @@ -366,11 +366,11 @@ def add_link(self, link: Link) -> "Document": if not self.links: self.links = [] if not isinstance(link, Link): - raise ValueError("add_link only takes Link() types") + raise ValueError(f"add_link only takes Link() types, instead it was provided with '{type(link)}', that's an internal bug, please open a ticket") if link.document.id == self.id: - raise ValueError("Cannot link a document to itself") + raise ValueError(f"Cannot link a document to itself, {self.id} is the same as the link") if link.document.id in [l.document.id for l in self.links]: - raise ValueError("Cannot link the same document twice") + raise cre_exceptions.DuplicateLinkException(f"Cannot link the same document twice, document {link.document.id} is already linked to {self.id}") self.links.append(link) return self @@ -453,6 +453,11 @@ def __post_init__(self): self.id += f":{self.section}" if self.subsection: self.id += f":{self.subsection}" + if self.hyperlink: + self.id += f":{self.hyperlink}" + if self.version: + self.id += f":{self.version}" + return super().__post_init__() def todict(self) -> Dict[Any, Any]: @@ -492,6 +497,12 @@ def __post_init__(self): self.id += f":{self.section}" if self.subsection: self.id += f":{self.subsection}" + if self.hyperlink: + self.id += f":{self.hyperlink}" + if self.version: + self.id += f":{self.version}" + if self.tooltype != ToolTypes.Unknown: + self.id += f":{self.tooltype.value}" return super().__post_init__() def __eq__(self, other: object) -> bool: diff --git a/application/defs/cre_exceptions.py b/application/defs/cre_exceptions.py index 95895004..4ef5f854 100644 --- a/application/defs/cre_exceptions.py +++ b/application/defs/cre_exceptions.py @@ -10,3 +10,8 @@ class InvalidCREIDException(DocumentFormatException): def __init__(self, cre): self.message = f"CRE ID '{cre.id}' does not fit pattern '\d\d\d-\d\d\d', cre name is {cre.name}" super().__init__(self.message) + + +class DuplicateLinkException(Exception): + def __init__(self, message): + super().__init__(message) diff --git a/application/web/web_main.py b/application/web/web_main.py index beab7f0e..d7225687 100644 --- a/application/web/web_main.py +++ b/application/web/web_main.py @@ -20,7 +20,8 @@ from application.database import db from application.cmd import cre_main from application.defs import cre_defs as defs -from application.defs import osib_defs as odefs +from application.defs import cre_exceptions + from application.utils import spreadsheet as sheet_utils from application.utils import mdutils, redirectors, gap_analysis from application.prompt_client import prompt_client as prompt_client @@ -733,7 +734,10 @@ def import_from_cre_csv() -> Any: abort(400, "No file provided") contents = file.read() csv_read = csv.DictReader(contents.decode("utf-8").splitlines()) - documents = spreadsheet_parsers.parse_export_format(list(csv_read)) + try: + documents = spreadsheet_parsers.parse_export_format(list(csv_read)) + except cre_exceptions.DuplicateLinkException as dle: + abort(500,f"error during parsing of the incoming CSV, err:{dle}") cres = documents.pop(defs.Credoctypes.CRE.value) standards = documents From 2a66eec54303bde8e2a4e057d385ab4e64c0ed58 Mon Sep 17 00:00:00 2001 From: Spyros Date: Sat, 28 Sep 2024 12:03:08 +0100 Subject: [PATCH 4/6] fixup error commit --- application/tests/cre_main_test.py | 12 ++++++------ application/tests/defs_test.py | 26 +++++++++++++------------- 2 files changed, 19 insertions(+), 19 deletions(-) diff --git a/application/tests/cre_main_test.py b/application/tests/cre_main_test.py index d4a93cc7..1912f04e 100644 --- a/application/tests/cre_main_test.py +++ b/application/tests/cre_main_test.py @@ -261,14 +261,14 @@ def test_register_cre(self) -> None: c_higher = self.collection.get_CREs(cre_higher.id)[0] c_lower = self.collection.get_CREs(cre_lower.id)[0] c_equal = self.collection.get_CREs(cre_equal.id)[0] - c = self.collection.get_CREs(cre.id)[0] + retrieved_cre = self.collection.get_CREs(cre.id)[0] self.assertCountEqual( c_higher.links, - [defs.Link(document=c.shallow_copy(), ltype=defs.LinkTypes.Contains)], + [defs.Link(document=retrieved_cre.shallow_copy(), ltype=defs.LinkTypes.Contains)], ) self.assertCountEqual( - c.links, + retrieved_cre.links, [ defs.Link(document=standard, ltype=defs.LinkTypes.LinkedTo), defs.Link(document=tool, ltype=defs.LinkTypes.LinkedTo), @@ -286,15 +286,15 @@ def test_register_cre(self) -> None: self.assertCountEqual( c_lower.links, - [defs.Link(document=c.shallow_copy(), ltype=defs.LinkTypes.PartOf)], + [defs.Link(document=retrieved_cre.shallow_copy(), ltype=defs.LinkTypes.PartOf)], ) self.assertCountEqual( c_higher.links, - [defs.Link(document=c.shallow_copy(), ltype=defs.LinkTypes.Contains)], + [defs.Link(document=retrieved_cre.shallow_copy(), ltype=defs.LinkTypes.Contains)], ) self.assertCountEqual( c_equal.links, - [defs.Link(document=c.shallow_copy(), ltype=defs.LinkTypes.Related)], + [defs.Link(document=retrieved_cre.shallow_copy(), ltype=defs.LinkTypes.Related)], ) def test_parse_file(self) -> None: diff --git a/application/tests/defs_test.py b/application/tests/defs_test.py index f99c08c0..33a866c8 100644 --- a/application/tests/defs_test.py +++ b/application/tests/defs_test.py @@ -18,7 +18,7 @@ def test_document_todict(self) -> None: version="0.0.0", ) standard_output = { - "id": "ASVS:SESSION-MGT-TOKEN-DIRECTIVES-DISCRETE-HANDLING:3.1.1", + "id": "ASVS:SESSION-MGT-TOKEN-DIRECTIVES-DISCRETE-HANDLING:3.1.1:0.0.0", "doctype": "Standard", "name": "ASVS", "section": "SESSION-MGT-TOKEN-DIRECTIVES-DISCRETE-HANDLING", @@ -35,14 +35,14 @@ def test_document_todict(self) -> None: ) cre_output = { "description": "CREdesc", - "doctype": defs.Credoctypes.CRE, + "doctype": defs.Credoctypes.CRE.value, "id": "100-100", "links": [ { - "ltype": defs.LinkTypes.LinkedTo, + "ltype": defs.LinkTypes.LinkedTo.value, "document": { - "id": "ASVS:SESSION-MGT-TOKEN-DIRECTIVES-DISCRETE-HANDLING:3.1.1", - "doctype": defs.Credoctypes.Standard, + "id": "ASVS:SESSION-MGT-TOKEN-DIRECTIVES-DISCRETE-HANDLING:3.1.1:0.0.0", + "doctype": defs.Credoctypes.Standard.value, "name": "ASVS", "section": "SESSION-MGT-TOKEN-DIRECTIVES-DISCRETE-HANDLING", "subsection": "3.1.1", @@ -75,17 +75,17 @@ def test_document_todict(self) -> None: "id": "500-500", "links": [ { - "ltype": defs.LinkTypes.Related, + "ltype": defs.LinkTypes.Related.value, "document": { "description": "CREdesc", - "doctype": defs.Credoctypes.CRE, + "doctype": defs.Credoctypes.CRE.value, "id": "100-100", "links": [ { - "ltype": defs.LinkTypes.LinkedTo, + "ltype": defs.LinkTypes.LinkedTo.value, "document": { - "id": "ASVS:SESSION-MGT-TOKEN-DIRECTIVES-DISCRETE-HANDLING:3.1.1", - "doctype": defs.Credoctypes.Standard, + "id": "ASVS:SESSION-MGT-TOKEN-DIRECTIVES-DISCRETE-HANDLING:3.1.1:0.0.0", + "doctype": defs.Credoctypes.Standard.value, "name": "ASVS", "section": ( "SESSION-MGT-TOKEN-DIRECTIVES-DISCRETE-HANDLING" @@ -100,10 +100,10 @@ def test_document_todict(self) -> None: }, }, { - "ltype": defs.LinkTypes.LinkedTo, + "ltype": defs.LinkTypes.LinkedTo.value, "document": { "id": "Standard:StandardSection:3.1.1", - "doctype": defs.Credoctypes.Standard, + "doctype": defs.Credoctypes.Standard.value, "name": "Standard", "section": "StandardSection", "subsection": "3.1.1", @@ -120,7 +120,7 @@ def test_document_todict(self) -> None: ) nested_output = { "id": "ASVS:SESSION-MGT-TOKEN-DIRECTIVES-DISCRETE-HANDLING:3.1.1", - "doctype": defs.Credoctypes.Standard, + "doctype": defs.Credoctypes.Standard.value, "name": "ASVS", "section": "SESSION-MGT-TOKEN-DIRECTIVES-DISCRETE-HANDLING", "subsection": "3.1.1", From a0bb1786e165b6d9c98bc9a23889720ee7abc84d Mon Sep 17 00:00:00 2001 From: Spyros Date: Sat, 28 Sep 2024 12:03:27 +0100 Subject: [PATCH 5/6] nit: styling --- application/cmd/cre_main.py | 2 +- application/tests/cre_main_test.py | 2 +- application/worker.py | 11 ++--------- 3 files changed, 4 insertions(+), 11 deletions(-) diff --git a/application/cmd/cre_main.py b/application/cmd/cre_main.py index f9287953..968d1c4f 100644 --- a/application/cmd/cre_main.py +++ b/application/cmd/cre_main.py @@ -643,7 +643,7 @@ def run(args: argparse.Namespace) -> None: # pragma: no cover if args.start_worker: from application.worker import start_worker - start_worker(args.cache_file) + start_worker() if args.preload_map_analysis_target_url: gap_analysis.preload(target_url=args.preload_map_analysis_target_url) diff --git a/application/tests/cre_main_test.py b/application/tests/cre_main_test.py index 1912f04e..46270fb5 100644 --- a/application/tests/cre_main_test.py +++ b/application/tests/cre_main_test.py @@ -367,7 +367,7 @@ def test_parse_file(self) -> None: "objects", "here", { - 1: 2, + "1": 2, }, ], scollection=self.collection, diff --git a/application/worker.py b/application/worker.py index 104e05eb..76b2057b 100644 --- a/application/worker.py +++ b/application/worker.py @@ -1,9 +1,5 @@ -import os -import redis from rq import Worker, Queue, Connection -from application.database import db import logging -from application.cmd.cre_main import db_connect from application.utils import redis logging.basicConfig() @@ -12,11 +8,8 @@ listen = ["high", "default", "low"] - -def start_worker(cache: str): - conn = redis.connect() +def start_worker(): logger.info(f"Worker Starting") - database = db_connect(path=cache) - with Connection(conn): + with Connection(redis.connect()): worker = Worker(map(Queue, listen)) worker.work() From 5459015ddf9e80c9e549e40eafd7b338e3adabfd Mon Sep 17 00:00:00 2001 From: Spyros Date: Sun, 29 Sep 2024 11:27:35 +0100 Subject: [PATCH 6/6] linting --- application/defs/cre_defs.py | 12 +++++++++--- application/tests/cre_main_test.py | 24 ++++++++++++++++++++---- application/web/web_main.py | 2 +- application/worker.py | 1 + 4 files changed, 31 insertions(+), 8 deletions(-) diff --git a/application/defs/cre_defs.py b/application/defs/cre_defs.py index 9e8f00a3..9cf2d4f4 100644 --- a/application/defs/cre_defs.py +++ b/application/defs/cre_defs.py @@ -366,11 +366,17 @@ def add_link(self, link: Link) -> "Document": if not self.links: self.links = [] if not isinstance(link, Link): - raise ValueError(f"add_link only takes Link() types, instead it was provided with '{type(link)}', that's an internal bug, please open a ticket") + raise ValueError( + f"add_link only takes Link() types, instead it was provided with '{type(link)}', that's an internal bug, please open a ticket" + ) if link.document.id == self.id: - raise ValueError(f"Cannot link a document to itself, {self.id} is the same as the link") + raise ValueError( + f"Cannot link a document to itself, {self.id} is the same as the link" + ) if link.document.id in [l.document.id for l in self.links]: - raise cre_exceptions.DuplicateLinkException(f"Cannot link the same document twice, document {link.document.id} is already linked to {self.id}") + raise cre_exceptions.DuplicateLinkException( + f"Cannot link the same document twice, document {link.document.id} is already linked to {self.id}" + ) self.links.append(link) return self diff --git a/application/tests/cre_main_test.py b/application/tests/cre_main_test.py index 46270fb5..6306a8b0 100644 --- a/application/tests/cre_main_test.py +++ b/application/tests/cre_main_test.py @@ -265,7 +265,11 @@ def test_register_cre(self) -> None: self.assertCountEqual( c_higher.links, - [defs.Link(document=retrieved_cre.shallow_copy(), ltype=defs.LinkTypes.Contains)], + [ + defs.Link( + document=retrieved_cre.shallow_copy(), ltype=defs.LinkTypes.Contains + ) + ], ) self.assertCountEqual( retrieved_cre.links, @@ -286,15 +290,27 @@ def test_register_cre(self) -> None: self.assertCountEqual( c_lower.links, - [defs.Link(document=retrieved_cre.shallow_copy(), ltype=defs.LinkTypes.PartOf)], + [ + defs.Link( + document=retrieved_cre.shallow_copy(), ltype=defs.LinkTypes.PartOf + ) + ], ) self.assertCountEqual( c_higher.links, - [defs.Link(document=retrieved_cre.shallow_copy(), ltype=defs.LinkTypes.Contains)], + [ + defs.Link( + document=retrieved_cre.shallow_copy(), ltype=defs.LinkTypes.Contains + ) + ], ) self.assertCountEqual( c_equal.links, - [defs.Link(document=retrieved_cre.shallow_copy(), ltype=defs.LinkTypes.Related)], + [ + defs.Link( + document=retrieved_cre.shallow_copy(), ltype=defs.LinkTypes.Related + ) + ], ) def test_parse_file(self) -> None: diff --git a/application/web/web_main.py b/application/web/web_main.py index d7225687..ff2da7e9 100644 --- a/application/web/web_main.py +++ b/application/web/web_main.py @@ -737,7 +737,7 @@ def import_from_cre_csv() -> Any: try: documents = spreadsheet_parsers.parse_export_format(list(csv_read)) except cre_exceptions.DuplicateLinkException as dle: - abort(500,f"error during parsing of the incoming CSV, err:{dle}") + abort(500, f"error during parsing of the incoming CSV, err:{dle}") cres = documents.pop(defs.Credoctypes.CRE.value) standards = documents diff --git a/application/worker.py b/application/worker.py index 76b2057b..96ba7da3 100644 --- a/application/worker.py +++ b/application/worker.py @@ -8,6 +8,7 @@ listen = ["high", "default", "low"] + def start_worker(): logger.info(f"Worker Starting") with Connection(redis.connect()):