From 74810a148b004d2381a5296aa6e95fab9e02c7a1 Mon Sep 17 00:00:00 2001 From: Raquel Mencia Date: Mon, 3 Apr 2023 15:51:10 +0200 Subject: [PATCH 01/18] [OPT-761] Fix OTM schema inconsistencies --- otm/resources/schemas/otm_schema.json | 111 +++++++++++++------------- 1 file changed, 57 insertions(+), 54 deletions(-) diff --git a/otm/resources/schemas/otm_schema.json b/otm/resources/schemas/otm_schema.json index e12ae8f3..d11737cc 100644 --- a/otm/resources/schemas/otm_schema.json +++ b/otm/resources/schemas/otm_schema.json @@ -7,40 +7,40 @@ "required": ["project", "otmVersion"], "properties": { "project": { - "type": "object", + "type": ["object", "null"], "required": ["name", "id"], "properties": { "name": {"type": "string"}, "id": {"type": "string"}, - "description": {"type": "string"}, - "owner": {"type": "string"}, - "ownerContact": {"type": "string"}, + "description": {"type": ["string", "null"]}, + "owner": {"type": ["string", "null"]}, + "ownerContact": {"type": ["string", "null"]}, "tags": { - "type": "array", + "type": ["array", "null"], "items": { - "type": "string" + "type": ["string", "null"] } }, - "attributes": {"type": "object"} + "attributes": {"type": ["object", "null"]} } }, "representations": { - "type": "array", + "type": ["array", "null"], "required": ["name", "id", "type"], "properties": { "name": {"type": "string"}, "id": {"type": "string"}, "type": {"type": "string"}, - "description": {"type": "string"}, + "description": {"type": ["string", "null"]}, "size": {"$ref": "#/definitions/size"}, "repository": { - "type": "object", + "type": ["object", "null"], "required": ["url"], "properties": { - "url": {"type": "string"} + "url": {"type": ["string", "null"]} } }, - "attributes": {"type": "object"} + "attributes": {"type": ["object", "null"]} } }, "assets": { @@ -49,18 +49,18 @@ "properties": { "name": {"type": "string"}, "id": {"type": "string"}, - "description": {"type": "string"}, + "description": {"type": ["string", "null"]}, "risk": { - "type": "object", + "type": ["object", "null"], "required": ["confidentiality", "integrity", "availability"], "properties": { "confidentiality": {"type": "number"}, "integrity": {"type": "number"}, "availability": {"type": "number"}, - "comment": {"type": "string"} + "comment": {"type": ["string", "null"]} } }, - "attributes": {"type": "object"} + "attributes": {"type": ["object", "null"]} } }, "trustZones": { @@ -72,7 +72,7 @@ "id": {"type": "string"}, "name": {"type": "string"}, "type": {"type": "string"}, - "description": {"type": "string"}, + "description": {"type": ["string", "null"]}, "risk": { "type": "object", "required": ["trustRating"], @@ -85,7 +85,7 @@ "type": "array", "items": {"$ref": "#/definitions/representationElement"} }, - "attributes": {"type": "object"} + "attributes": {"type": ["object", "null"]} } } }, @@ -98,40 +98,40 @@ "id": {"type": "string"}, "name": {"type": "string"}, "type": {"type": "string"}, - "description": {"type": "string"}, + "description": {"type": ["string", "null"]}, "parent": {"$ref": "#/definitions/parent"}, "representations": { - "type": "array", + "type": ["array", "null"], "items": {"$ref": "#/definitions/representationElement"} }, "assets": { - "type": "object", + "type": ["object", "null"], "properties": { "stored": { - "type": "array", + "type": ["array", "null"], "items": { - "type": "string" + "type": ["string", "null"] } }, "processed": { - "type": "array", + "type": ["array", "null"], "items": { - "type": "string" + "type": ["string", "null"] } } } }, "threats": { - "type": "array", + "type": ["array", "null"], "items": {"$ref": "#/definitions/threat/"} }, "tags": { - "type": "array", + "type": ["array", "null"], "items": { - "type": "string" + "type": ["string", "null"] } }, - "attributes": {"type": "object"} + "attributes": {"type": ["object", "null"]} } } }, @@ -143,22 +143,25 @@ "properties": { "id": {"type": "string"}, "name": {"type": "string"}, - "description": {"type": "string"}, + "description": {"type": ["string", "null"]}, "bidirectional": {"type": "boolean"}, "source": {"type": "string"}, "destination": {"type": "string"}, "assets": { - "type": "array", - "items": {"type": "string"} + "type": ["array", "null"], + "items": {"type": ["string", "null"]} + }, + "threats": { + "type": ["array", "null"], + "items": {"$ref": "#/definitions/threat/"} }, - "threats": {"$ref": "#/definitions/threat"}, "tags": { - "type": "array", + "type": ["array", "null"], "items": { - "type": "string" + "type": ["string", "null"] } }, - "attributes": {"type": "object"} + "attributes": {"type": ["object", "null"]} } } }, @@ -170,14 +173,14 @@ "properties": { "id": {"type": "string"}, "name": {"type": "string"}, - "description": {"type": "string"}, + "description": {"type": ["string", "null"]}, "categories": { - "type": "array", - "items": {"type": "string"} + "type": ["array", "null"], + "items": {"type": ["string", "null"]} }, "cwes": { - "type": "array", - "items": {"type": "string"} + "type": ["array", "null"], + "items": {"type": ["string", "null"]} }, "risk": { "type": "object", @@ -190,12 +193,12 @@ } }, "tags": { - "type": "array", + "type": ["array", "null"], "items": { - "type": "string" + "type": ["string", "null"] } }, - "attributes": {"type": "object"} + "attributes": {"type": ["object", "null"]} } } }, @@ -207,9 +210,9 @@ "properties": { "id": {"type": "string"}, "name": {"type": "string"}, - "description": {"type": "string"}, + "description": {"type": ["string", "null"]}, "riskReduction": {"type": "number"}, - "attributes": {"type": "object"} + "attributes": {"type": ["object", "null"]} } } } @@ -247,14 +250,14 @@ "required": ["representation", "id"], "properties": { "representation": {"type": "string"}, - "name": {"type": "string"}, + "name": {"type": ["string", "null"]}, "id": {"type": "string"}, "position": {"$ref": "#/definitions/position"}, "size": {"$ref": "#/definitions/size"}, - "file": {"type": "string"}, - "line": {"type": "number"}, - "codeSnippet": {"type": "string"}, - "attributes": {"type": "object"} + "file": {"type": ["string", "null"]}, + "line": {"type": ["number", "null"]}, + "codeSnippet": {"type": ["string", "null"]}, + "attributes": {"type": ["object", "null"]} } }, "threat": { @@ -266,11 +269,11 @@ "mitigations": { "type": "array", "items": { - "type": "object", + "type": ["object", "null"], "required": ["mitigation", "state"], "properties": { - "mitigation": {"type": "string"}, - "state": {"type": "string"} + "mitigation": {"type": ["string", "null"]}, + "state": {"type": ["string", "null"]} } } } From 693ba88fffc512ab91a1862929d36e4c0beb5dd0 Mon Sep 17 00:00:00 2001 From: Santi Manero Date: Tue, 11 Apr 2023 15:16:46 +0200 Subject: [PATCH 02/18] [OPT-780] Extract strategy mapping dataflows --- slp_visio/slp_visio/load/__init__.py | 1 + .../load/objects/visio_diagram_factories.py | 50 +++-------------- .../dataflow/create_connector_strategy.py | 23 ++++++++ .../impl/create_connector_by_connects.py | 54 +++++++++++++++++++ .../impl/validate_connector_by_connects.py | 16 ++++++ .../dataflow/validate_connector_strategy.py | 22 ++++++++ .../slp_visio/load/strategies/strategy.py | 16 ++++++ slp_visio/slp_visio/load/vsdx_parser.py | 5 +- 8 files changed, 142 insertions(+), 45 deletions(-) create mode 100644 slp_visio/slp_visio/load/strategies/dataflow/create_connector_strategy.py create mode 100644 slp_visio/slp_visio/load/strategies/dataflow/impl/create_connector_by_connects.py create mode 100644 slp_visio/slp_visio/load/strategies/dataflow/impl/validate_connector_by_connects.py create mode 100644 slp_visio/slp_visio/load/strategies/dataflow/validate_connector_strategy.py create mode 100644 slp_visio/slp_visio/load/strategies/strategy.py diff --git a/slp_visio/slp_visio/load/__init__.py b/slp_visio/slp_visio/load/__init__.py index e69de29b..2418ddf2 100644 --- a/slp_visio/slp_visio/load/__init__.py +++ b/slp_visio/slp_visio/load/__init__.py @@ -0,0 +1 @@ +from .strategies.dataflow.impl import validate_connector_by_connects, create_connector_by_connects diff --git a/slp_visio/slp_visio/load/objects/visio_diagram_factories.py b/slp_visio/slp_visio/load/objects/visio_diagram_factories.py index c3f53bcd..e2308b80 100644 --- a/slp_visio/slp_visio/load/objects/visio_diagram_factories.py +++ b/slp_visio/slp_visio/load/objects/visio_diagram_factories.py @@ -1,37 +1,10 @@ from typing import Optional from slp_visio.slp_visio.load.objects.diagram_objects import DiagramComponent, DiagramConnector +from slp_visio.slp_visio.load.strategies.dataflow.create_connector_strategy import CreateConnectorStrategy from slp_visio.slp_visio.util.visio import get_shape_text, get_master_shape_text, normalize_label, get_unique_id_text -# if it has two shapes connected and is not pointing itself -def is_valid_connector(connected_shapes) -> bool: - if len(connected_shapes) < 2: - return False - if connected_shapes[0].shape_id == connected_shapes[1].shape_id: - return False - return True - - -# if its master name includes 'Double Arrow' or has arrows defined in both ends -def is_bidirectional_connector(shape) -> bool: - if shape.master_page.name is not None and 'Double Arrow' in shape.master_page.name: - return True - for arrow_value in [shape.cell_value(att) for att in ['BeginArrow', 'EndArrow']]: - if arrow_value is None or not str(arrow_value).isnumeric() or arrow_value == '0': - return False - return True - - -def is_created_from(connector) -> bool: - return connector.from_rel == 'BeginX' - - -def connector_has_arrow_in_origin(shape) -> bool: - begin_arrow_value = shape.cell_value('BeginArrow') - return begin_arrow_value is not None and str(begin_arrow_value).isnumeric() and begin_arrow_value != '0' - - class VisioComponentFactory: def create_component(self, shape, origin, representer) -> DiagramComponent: @@ -46,18 +19,9 @@ def create_component(self, shape, origin, representer) -> DiagramComponent: class VisioConnectorFactory: - def create_connector(self, shape) -> Optional[DiagramConnector]: - connected_shapes = shape.connects - if not is_valid_connector(connected_shapes): - return None - - if is_bidirectional_connector(shape): - return DiagramConnector(shape.ID, connected_shapes[0].shape_id, connected_shapes[1].shape_id, True) - - has_arrow_in_origin = connector_has_arrow_in_origin(shape) - - if (not has_arrow_in_origin and is_created_from(connected_shapes[0])) \ - or (has_arrow_in_origin and is_created_from(connected_shapes[1])): - return DiagramConnector(shape.ID, connected_shapes[0].shape_id, connected_shapes[1].shape_id) - else: - return DiagramConnector(shape.ID, connected_shapes[1].shape_id, connected_shapes[0].shape_id) + @staticmethod + def create_connector(shape) -> Optional[DiagramConnector]: + for strategy in CreateConnectorStrategy.get_strategies(): + connector = strategy.create_connector(shape) + if connector: + return connector diff --git a/slp_visio/slp_visio/load/strategies/dataflow/create_connector_strategy.py b/slp_visio/slp_visio/load/strategies/dataflow/create_connector_strategy.py new file mode 100644 index 00000000..f226ae19 --- /dev/null +++ b/slp_visio/slp_visio/load/strategies/dataflow/create_connector_strategy.py @@ -0,0 +1,23 @@ +import abc + +from vsdx import Shape + +from otm.otm.entity.dataflow import Dataflow +from slp_visio.slp_visio.load.strategies.strategy import Strategy + + +class CreateConnectorStrategy(Strategy): + """ + Formal Interface to create an OTM Dataflow from a vsdx shape + """ + + @classmethod + def __subclasshook__(cls, subclass): + return ( + hasattr(subclass, 'create_connector') and callable(subclass.process) + or NotImplemented) + + @abc.abstractmethod + def create_connector(self, shape: Shape) -> Dataflow: + """creates the OTM Dataflow from the vsdx shape""" + raise NotImplementedError diff --git a/slp_visio/slp_visio/load/strategies/dataflow/impl/create_connector_by_connects.py b/slp_visio/slp_visio/load/strategies/dataflow/impl/create_connector_by_connects.py new file mode 100644 index 00000000..f65abc9c --- /dev/null +++ b/slp_visio/slp_visio/load/strategies/dataflow/impl/create_connector_by_connects.py @@ -0,0 +1,54 @@ +from vsdx import Shape + +from slp_visio.slp_visio.load.objects.diagram_objects import DiagramConnector +from slp_visio.slp_visio.load.strategies.dataflow.create_connector_strategy import CreateConnectorStrategy + + +class CreateConnectorByConnects(CreateConnectorStrategy): + """ + Strategy to create a connector from the shape connects + """ + + def create_connector(self, shape: Shape) -> bool: + connected_shapes = shape.connects + if not self.are_two_different_shapes(connected_shapes): + return None + + if self.is_bidirectional_connector(shape): + return DiagramConnector(shape.ID, connected_shapes[0].shape_id, connected_shapes[1].shape_id, True) + + has_arrow_in_origin = self.connector_has_arrow_in_origin(shape) + + if (not has_arrow_in_origin and self.is_created_from(connected_shapes[0])) \ + or (has_arrow_in_origin and self.is_created_from(connected_shapes[1])): + return DiagramConnector(shape.ID, connected_shapes[0].shape_id, connected_shapes[1].shape_id) + else: + return DiagramConnector(shape.ID, connected_shapes[1].shape_id, connected_shapes[0].shape_id) + + # if it has two shapes connected and is not pointing itself + @staticmethod + def are_two_different_shapes(connected_shapes) -> bool: + if len(connected_shapes) < 2: + return False + if connected_shapes[0].shape_id == connected_shapes[1].shape_id: + return False + return True + + # if its master name includes 'Double Arrow' or has arrows defined in both ends + @staticmethod + def is_bidirectional_connector(shape) -> bool: + if shape.master_page.name is not None and 'Double Arrow' in shape.master_page.name: + return True + for arrow_value in [shape.cell_value(att) for att in ['BeginArrow', 'EndArrow']]: + if arrow_value is None or not str(arrow_value).isnumeric() or arrow_value == '0': + return False + return True + + @staticmethod + def connector_has_arrow_in_origin(shape) -> bool: + begin_arrow_value = shape.cell_value('BeginArrow') + return begin_arrow_value is not None and str(begin_arrow_value).isnumeric() and begin_arrow_value != '0' + + @staticmethod + def is_created_from(connector) -> bool: + return connector.from_rel == 'BeginX' diff --git a/slp_visio/slp_visio/load/strategies/dataflow/impl/validate_connector_by_connects.py b/slp_visio/slp_visio/load/strategies/dataflow/impl/validate_connector_by_connects.py new file mode 100644 index 00000000..62e7a007 --- /dev/null +++ b/slp_visio/slp_visio/load/strategies/dataflow/impl/validate_connector_by_connects.py @@ -0,0 +1,16 @@ +from vsdx import Shape + +from slp_visio.slp_visio.load.strategies.dataflow.validate_connector_strategy import ValidateConnectorStrategy + + +class ValidateConnectorByConnects(ValidateConnectorStrategy): + """ + Strategy to know if a shape is a connector + The shape must have connects and each connector_shape_id must match with the shape id + """ + + def is_connector(self, shape: Shape) -> bool: + for connect in shape.connects: + if shape.ID == connect.connector_shape_id: + return True + return False \ No newline at end of file diff --git a/slp_visio/slp_visio/load/strategies/dataflow/validate_connector_strategy.py b/slp_visio/slp_visio/load/strategies/dataflow/validate_connector_strategy.py new file mode 100644 index 00000000..6a2ad7f3 --- /dev/null +++ b/slp_visio/slp_visio/load/strategies/dataflow/validate_connector_strategy.py @@ -0,0 +1,22 @@ +import abc + +from vsdx import Shape + +from slp_visio.slp_visio.load.strategies.strategy import Strategy + + +class ValidateConnectorStrategy(Strategy): + """ + Formal Interface to validate if a shape is a connector + """ + + @classmethod + def __subclasshook__(cls, subclass): + return ( + hasattr(subclass, 'is_connector') and callable(subclass.process) + or NotImplemented) + + @abc.abstractmethod + def is_connector(self, shape: Shape) -> bool: + """return True if the Shape is a connector""" + raise NotImplementedError diff --git a/slp_visio/slp_visio/load/strategies/strategy.py b/slp_visio/slp_visio/load/strategies/strategy.py new file mode 100644 index 00000000..1c5e2792 --- /dev/null +++ b/slp_visio/slp_visio/load/strategies/strategy.py @@ -0,0 +1,16 @@ +import abc + + +class Strategy(metaclass=abc.ABCMeta): + """ + Formal Interface to build a strategy + """ + + @classmethod + def get_strategies(cls): + return [obj() for obj in cls.__get_subclasses()] + + @classmethod + def __get_subclasses(cls): + for subclass in cls.__subclasses__(): + yield subclass \ No newline at end of file diff --git a/slp_visio/slp_visio/load/vsdx_parser.py b/slp_visio/slp_visio/load/vsdx_parser.py index e0077ede..937a72de 100644 --- a/slp_visio/slp_visio/load/vsdx_parser.py +++ b/slp_visio/slp_visio/load/vsdx_parser.py @@ -5,6 +5,7 @@ from slp_visio.slp_visio.load.parent_calculator import ParentCalculator from slp_visio.slp_visio.load.representation.simple_component_representer import SimpleComponentRepresenter from slp_visio.slp_visio.load.representation.zone_component_representer import ZoneComponentRepresenter +from slp_visio.slp_visio.load.strategies.dataflow.validate_connector_strategy import ValidateConnectorStrategy from slp_visio.slp_visio.util.visio import get_limits, get_shape_text DIAGRAM_LIMITS_PADDING = 2 @@ -43,8 +44,8 @@ def parse(self, visio_diagram_filename) -> Diagram: @staticmethod def _is_connector(shape: Shape) -> bool: - for connect in shape.connects: - if shape.ID == connect.connector_shape_id: + for strategy in ValidateConnectorStrategy.get_strategies(): + if strategy.is_connector(shape): return True return False From 90f21d3f737ec78a7692ebbb034e1fecb2b95c45 Mon Sep 17 00:00:00 2001 From: Santi Manero Date: Thu, 13 Apr 2023 09:38:50 +0200 Subject: [PATCH 03/18] [OPT-780] Extract strategy mapping dataflows - additional tests --- slp_visio/slp_visio/load/__init__.py | 2 +- .../load/objects/visio_diagram_factories.py | 2 +- .../create_connector_strategy.py | 5 +- .../impl/create_connector_by_connects.py | 6 +- .../impl/validate_connector_by_connects.py | 2 +- .../validate_connector_strategy.py | 0 slp_visio/slp_visio/load/vsdx_parser.py | 2 +- .../objects/test_visio_diagram_factories.py | 137 +++++------------- .../impl/test_create_connector_by_connects.py | 122 ++++++++++++++++ .../test_validate_connector_by_connects.py | 47 ++++++ .../test_create_connector_strategy.py | 15 ++ .../test_validate_connector_strategy.py | 16 ++ slp_visio/tests/unit/load/test_vsdx_parser.py | 29 ++++ 13 files changed, 277 insertions(+), 108 deletions(-) rename slp_visio/slp_visio/load/strategies/{dataflow => connector}/create_connector_strategy.py (74%) rename slp_visio/slp_visio/load/strategies/{dataflow => connector}/impl/create_connector_by_connects.py (91%) rename slp_visio/slp_visio/load/strategies/{dataflow => connector}/impl/validate_connector_by_connects.py (79%) rename slp_visio/slp_visio/load/strategies/{dataflow => connector}/validate_connector_strategy.py (100%) create mode 100644 slp_visio/tests/unit/load/strategies/dataflow/impl/test_create_connector_by_connects.py create mode 100644 slp_visio/tests/unit/load/strategies/dataflow/impl/test_validate_connector_by_connects.py create mode 100644 slp_visio/tests/unit/load/strategies/dataflow/test_create_connector_strategy.py create mode 100644 slp_visio/tests/unit/load/strategies/dataflow/test_validate_connector_strategy.py create mode 100644 slp_visio/tests/unit/load/test_vsdx_parser.py diff --git a/slp_visio/slp_visio/load/__init__.py b/slp_visio/slp_visio/load/__init__.py index 2418ddf2..afead209 100644 --- a/slp_visio/slp_visio/load/__init__.py +++ b/slp_visio/slp_visio/load/__init__.py @@ -1 +1 @@ -from .strategies.dataflow.impl import validate_connector_by_connects, create_connector_by_connects +from .strategies.connector.impl import validate_connector_by_connects, create_connector_by_connects diff --git a/slp_visio/slp_visio/load/objects/visio_diagram_factories.py b/slp_visio/slp_visio/load/objects/visio_diagram_factories.py index e2308b80..0dd7dda3 100644 --- a/slp_visio/slp_visio/load/objects/visio_diagram_factories.py +++ b/slp_visio/slp_visio/load/objects/visio_diagram_factories.py @@ -1,7 +1,7 @@ from typing import Optional from slp_visio.slp_visio.load.objects.diagram_objects import DiagramComponent, DiagramConnector -from slp_visio.slp_visio.load.strategies.dataflow.create_connector_strategy import CreateConnectorStrategy +from slp_visio.slp_visio.load.strategies.connector.create_connector_strategy import CreateConnectorStrategy from slp_visio.slp_visio.util.visio import get_shape_text, get_master_shape_text, normalize_label, get_unique_id_text diff --git a/slp_visio/slp_visio/load/strategies/dataflow/create_connector_strategy.py b/slp_visio/slp_visio/load/strategies/connector/create_connector_strategy.py similarity index 74% rename from slp_visio/slp_visio/load/strategies/dataflow/create_connector_strategy.py rename to slp_visio/slp_visio/load/strategies/connector/create_connector_strategy.py index f226ae19..64646424 100644 --- a/slp_visio/slp_visio/load/strategies/dataflow/create_connector_strategy.py +++ b/slp_visio/slp_visio/load/strategies/connector/create_connector_strategy.py @@ -1,8 +1,9 @@ import abc +from typing import Optional from vsdx import Shape -from otm.otm.entity.dataflow import Dataflow +from slp_visio.slp_visio.load.objects.diagram_objects import DiagramConnector from slp_visio.slp_visio.load.strategies.strategy import Strategy @@ -18,6 +19,6 @@ def __subclasshook__(cls, subclass): or NotImplemented) @abc.abstractmethod - def create_connector(self, shape: Shape) -> Dataflow: + def create_connector(self, shape: Shape) -> Optional[DiagramConnector]: """creates the OTM Dataflow from the vsdx shape""" raise NotImplementedError diff --git a/slp_visio/slp_visio/load/strategies/dataflow/impl/create_connector_by_connects.py b/slp_visio/slp_visio/load/strategies/connector/impl/create_connector_by_connects.py similarity index 91% rename from slp_visio/slp_visio/load/strategies/dataflow/impl/create_connector_by_connects.py rename to slp_visio/slp_visio/load/strategies/connector/impl/create_connector_by_connects.py index f65abc9c..b224a3f2 100644 --- a/slp_visio/slp_visio/load/strategies/dataflow/impl/create_connector_by_connects.py +++ b/slp_visio/slp_visio/load/strategies/connector/impl/create_connector_by_connects.py @@ -1,7 +1,9 @@ +from typing import Optional + from vsdx import Shape from slp_visio.slp_visio.load.objects.diagram_objects import DiagramConnector -from slp_visio.slp_visio.load.strategies.dataflow.create_connector_strategy import CreateConnectorStrategy +from slp_visio.slp_visio.load.strategies.connector.create_connector_strategy import CreateConnectorStrategy class CreateConnectorByConnects(CreateConnectorStrategy): @@ -9,7 +11,7 @@ class CreateConnectorByConnects(CreateConnectorStrategy): Strategy to create a connector from the shape connects """ - def create_connector(self, shape: Shape) -> bool: + def create_connector(self, shape: Shape) -> Optional[DiagramConnector]: connected_shapes = shape.connects if not self.are_two_different_shapes(connected_shapes): return None diff --git a/slp_visio/slp_visio/load/strategies/dataflow/impl/validate_connector_by_connects.py b/slp_visio/slp_visio/load/strategies/connector/impl/validate_connector_by_connects.py similarity index 79% rename from slp_visio/slp_visio/load/strategies/dataflow/impl/validate_connector_by_connects.py rename to slp_visio/slp_visio/load/strategies/connector/impl/validate_connector_by_connects.py index 62e7a007..d66e47ba 100644 --- a/slp_visio/slp_visio/load/strategies/dataflow/impl/validate_connector_by_connects.py +++ b/slp_visio/slp_visio/load/strategies/connector/impl/validate_connector_by_connects.py @@ -1,6 +1,6 @@ from vsdx import Shape -from slp_visio.slp_visio.load.strategies.dataflow.validate_connector_strategy import ValidateConnectorStrategy +from slp_visio.slp_visio.load.strategies.connector.validate_connector_strategy import ValidateConnectorStrategy class ValidateConnectorByConnects(ValidateConnectorStrategy): diff --git a/slp_visio/slp_visio/load/strategies/dataflow/validate_connector_strategy.py b/slp_visio/slp_visio/load/strategies/connector/validate_connector_strategy.py similarity index 100% rename from slp_visio/slp_visio/load/strategies/dataflow/validate_connector_strategy.py rename to slp_visio/slp_visio/load/strategies/connector/validate_connector_strategy.py diff --git a/slp_visio/slp_visio/load/vsdx_parser.py b/slp_visio/slp_visio/load/vsdx_parser.py index 937a72de..90fb376f 100644 --- a/slp_visio/slp_visio/load/vsdx_parser.py +++ b/slp_visio/slp_visio/load/vsdx_parser.py @@ -5,7 +5,7 @@ from slp_visio.slp_visio.load.parent_calculator import ParentCalculator from slp_visio.slp_visio.load.representation.simple_component_representer import SimpleComponentRepresenter from slp_visio.slp_visio.load.representation.zone_component_representer import ZoneComponentRepresenter -from slp_visio.slp_visio.load.strategies.dataflow.validate_connector_strategy import ValidateConnectorStrategy +from slp_visio.slp_visio.load.strategies.connector.validate_connector_strategy import ValidateConnectorStrategy from slp_visio.slp_visio.util.visio import get_limits, get_shape_text DIAGRAM_LIMITS_PADDING = 2 diff --git a/slp_visio/tests/unit/load/objects/test_visio_diagram_factories.py b/slp_visio/tests/unit/load/objects/test_visio_diagram_factories.py index e827a83f..f0bbc7ce 100644 --- a/slp_visio/tests/unit/load/objects/test_visio_diagram_factories.py +++ b/slp_visio/tests/unit/load/objects/test_visio_diagram_factories.py @@ -1,111 +1,48 @@ -from unittest.mock import MagicMock - -import pytest +from unittest.mock import patch, MagicMock from slp_visio.slp_visio.load.objects.visio_diagram_factories import VisioConnectorFactory +from slp_visio.slp_visio.load.strategies.connector.create_connector_strategy import CreateConnectorStrategy class TestVisioConnectorFactory: - def test_create_connector_ok(self): - # GIVEN visio connector shape - shape = MagicMock( - ID=1001, - connects=[MagicMock(from_rel='BeginX', shape_id=1), MagicMock(from_rel='EndX', shape_id=2)]) - - # WHEN a connector is created - visio_connector = VisioConnectorFactory() - diagram_connector = visio_connector.create_connector(shape) - - # THEN a diagram connector has the following properties - assert diagram_connector.id == 1001 - assert diagram_connector.from_id == 1 - assert diagram_connector.to_id == 2 + @patch( + "slp_visio.slp_visio.load.strategies.connector.impl.create_connector_by_connects.CreateConnectorByConnects.create_connector") + def test_create_connector_when_strategy_returns_value(self, mock_strategy_impl1): + # GIVEN a visio connector shape + shape = MagicMock(ID=1001) - def test_create_connector_reversed_ok(self): - # GIVEN visio connector shape with reversed relationship - shape = MagicMock( - ID=1001, - connects=[MagicMock(from_rel='EndX', shape_id=1), MagicMock(from_rel='BeginX', shape_id=2)]) + # AND a diagram connector is returned by the strategy + diagram_connector = MagicMock(id=1001, from_id=1, to_id=2) + mock_strategy_impl1.return_value = diagram_connector # WHEN a connector is created - visio_connector = VisioConnectorFactory() - diagram_connector = visio_connector.create_connector(shape) - - # THEN a diagram connector has the following properties - assert diagram_connector.id == 1001 - assert diagram_connector.from_id == 2 - assert diagram_connector.to_id == 1 - - def test_create_connector_incomplete_connectors(self): - # GIVEN visio connector shape with incomplete connectors - shape = MagicMock( - ID=1001, - connects=[MagicMock(from_rel='BeginX', shape_id=1)]) + result = VisioConnectorFactory().create_connector(shape) + + # THEN the strategy implementations are the expected + assert CreateConnectorStrategy.get_strategies().__len__() == 1 + # AND the strategies method implementations are called once + mock_strategy_impl1.assert_called_once() + # AND the result is the expected + assert result.id == 1001 + assert result.to_id == 2 + assert result.from_id == 1 + + @patch( + "slp_visio.slp_visio.load.strategies.connector.impl.create_connector_by_connects.CreateConnectorByConnects.create_connector") + def test_create_connector_when_strategy_does_not_return_value(self, mock_strategy_impl1): + # GIVEN a visio connector shape + shape = MagicMock(ID=1001) + + # AND no diagram connector is returned by the strategy + mock_strategy_impl1.return_value = None # WHEN a connector is created - visio_connector = VisioConnectorFactory() - diagram_connector = visio_connector.create_connector(shape) - - # THEN None diagram connector is returned - assert not diagram_connector - - def test_create_connector_self_pointing(self): - # GIVEN visio connector shape self pointing - shape = MagicMock( - ID=1001, - connects=[MagicMock(from_rel='BeginX', shape_id=1), MagicMock(from_rel='EndX', shape_id=1)]) - - # WHEN a connector is created - visio_connector = VisioConnectorFactory() - diagram_connector = visio_connector.create_connector(shape) - - # THEN None diagram connector is returned - assert not diagram_connector - - @pytest.mark.parametrize('shape,cell_values,master_name,from_shape,to_shape,bidirectional', [ - # Control case - pytest.param(MagicMock(ID=1001, connects=[MagicMock(from_rel='BeginX', shape_id=1), MagicMock(from_rel='EndX', shape_id=2)]), {'BeginArrow': None, 'EndArrow': None}, None, 1, 2, False, id='control'), - pytest.param(MagicMock(ID=1001, connects=[MagicMock(from_rel='BeginX', shape_id=1), MagicMock(from_rel='EndX', shape_id=2)]), {'BeginArrow': None, 'EndArrow': None}, 'Custom Arrow', 1, 2, False, id='control_inverted'), - # Created left to right, arrow changed left to right - pytest.param(MagicMock(ID=1001, connects=[MagicMock(from_rel='BeginX', shape_id=1), MagicMock(from_rel='EndX', shape_id=2)]), {'BeginArrow': None, 'EndArrow': '13'}, None, 1, 2, False, id='left_to_right_arrow_changed_left_to_right'), - pytest.param(MagicMock(ID=1001, connects=[MagicMock(from_rel='EndX', shape_id=2), MagicMock(from_rel='BeginX', shape_id=1)]), {'BeginArrow': None, 'EndArrow': '13'}, None, 1, 2, False, id='left_to_right_arrow_changed_left_to_right_inverted'), - # Created left to right, arrow changed right to left - pytest.param(MagicMock(ID=1001, connects=[MagicMock(from_rel='BeginX', shape_id=1), MagicMock(from_rel='EndX', shape_id=2)]), {'BeginArrow': '13', 'EndArrow': None}, None, 2, 1, False, id='left_to_right_arrow_changed_right_to_left'), - pytest.param(MagicMock(ID=1001, connects=[MagicMock(from_rel='EndX', shape_id=2), MagicMock(from_rel='BeginX', shape_id=1)]), {'BeginArrow': '13', 'EndArrow': None}, None, 2, 1, False, id='left_to_right_arrow_changed_right_to_left_inverted'), - # Created right to left, arrow changed right to left - pytest.param(MagicMock(ID=1001, connects=[MagicMock(from_rel='BeginX', shape_id=2), MagicMock(from_rel='EndX', shape_id=1)]), {'BeginArrow': None, 'EndArrow': '13'}, None, 2, 1, False, id='right_to_left_arrow_changed_right_to_left'), - pytest.param(MagicMock(ID=1001, connects=[MagicMock(from_rel='EndX', shape_id=1), MagicMock(from_rel='BeginX', shape_id=2)]), {'BeginArrow': None, 'EndArrow': '13'}, None, 2, 1, False, id='right_to_left_arrow_changed_right_to_left_inverted'), - # Created right to left, arrow changed left to right - pytest.param(MagicMock(ID=1001, connects=[MagicMock(from_rel='BeginX', shape_id=2), MagicMock(from_rel='EndX', shape_id=1)]), {'BeginArrow': '13', 'EndArrow': None}, None, 1, 2, False, id='right_to_left_arrow_changed_left_to_right'), - pytest.param(MagicMock(ID=1001, connects=[MagicMock(from_rel='EndX', shape_id=1), MagicMock(from_rel='BeginX', shape_id=2)]), {'BeginArrow': '13', 'EndArrow': None}, None, 1, 2, False, id='right_to_left_arrow_changed_left_to_right_inverted'), - # Invalid BeginArrow/EndArrow combinations - pytest.param(MagicMock(ID=1001, connects=[MagicMock(from_rel='BeginX', shape_id=1), MagicMock(from_rel='EndX', shape_id=2)]), {'BeginArrow': '0', 'EndArrow': None}, None, 1, 2, False, id='invalid_begin_0_no_end'), - pytest.param(MagicMock(ID=1001, connects=[MagicMock(from_rel='BeginX', shape_id=1), MagicMock(from_rel='EndX', shape_id=2)]), {'BeginArrow': None, 'EndArrow': '0'}, None, 1, 2, False, id='invalid_no_begin_end_0'), - pytest.param(MagicMock(ID=1001, connects=[MagicMock(from_rel='BeginX', shape_id=1), MagicMock(from_rel='EndX', shape_id=2)]), {'BeginArrow': '0', 'EndArrow': '0'}, None, 1, 2, False, id='invalid_begin_0_end_0'), - # Created left to right, arrow changed to bidirectional - pytest.param(MagicMock(ID=1001, connects=[MagicMock(from_rel='BeginX', shape_id=1), MagicMock(from_rel='EndX', shape_id=2)]), {'BeginArrow': '13', 'EndArrow': '13'}, None, 1, 2, True, id='left_to_right_arrow_changed_bidirectional'), - pytest.param(MagicMock(ID=1001, connects=[MagicMock(from_rel='EndX', shape_id=2), MagicMock(from_rel='BeginX', shape_id=1)]), {'BeginArrow': '13', 'EndArrow': '13'}, None, 2, 1, True, id='left_to_right_arrow_changed_bidirectional_inverted'), - # Created right to left, arrow changed to bidirectional - pytest.param(MagicMock(ID=1001, connects=[MagicMock(from_rel='BeginX', shape_id=2), MagicMock(from_rel='EndX', shape_id=1)]), {'BeginArrow': '13', 'EndArrow': '13'}, None, 2, 1, True, id='right_to_left_arrow_changed_bidirectional'), - pytest.param(MagicMock(ID=1001, connects=[MagicMock(from_rel='EndX', shape_id=1), MagicMock(from_rel='BeginX', shape_id=2)]), {'BeginArrow': '13', 'EndArrow': '13'}, None, 1, 2, True, id='right_to_left_arrow_changed_bidirectional_inverted'), - # Created left to right, shape changed to bidirectional - pytest.param(MagicMock(ID=1001, connects=[MagicMock(from_rel='BeginX', shape_id=1), MagicMock(from_rel='EndX', shape_id=2)]), {'BeginArrow': None, 'EndArrow': None}, 'Simple Double Arrow', 1, 2, True, id='left_to_right_shape_changed_bidirectional'), - pytest.param(MagicMock(ID=1001, connects=[MagicMock(from_rel='EndX', shape_id=2), MagicMock(from_rel='BeginX', shape_id=1)]), {'BeginArrow': None, 'EndArrow': None}, 'Line Double Arrow', 2, 1, True, id='left_to_right_shape_changed_bidirectional_inverted'), - # Created right to left, shape changed to bidirectional - pytest.param(MagicMock(ID=1001, connects=[MagicMock(from_rel='BeginX', shape_id=2), MagicMock(from_rel='EndX', shape_id=1)]), {'BeginArrow': None, 'EndArrow': None}, 'Arced Line Double Arrow', 2, 1, True, id='right_to_left_shape_changed_bidirectional'), - pytest.param(MagicMock(ID=1001, connects=[MagicMock(from_rel='EndX', shape_id=1), MagicMock(from_rel='BeginX', shape_id=2)]), {'BeginArrow': None, 'EndArrow': None}, 'Block Double Arrow', 1, 2, True, id='right_to_left_shape_changed_bidirectional_inverted') - ]) - def test_create_connector_modified_manually(self, shape, cell_values, master_name, from_shape, to_shape, bidirectional): - # GIVEN a mocked visio connector - shape.cell_value.side_effect = cell_values.get - shape.master_page.name = master_name - - # WHEN a connector is created - diagram_connector = VisioConnectorFactory().create_connector(shape) - - # THEN a diagram connector has the following properties - assert diagram_connector.id == 1001 - assert diagram_connector.from_id == from_shape - assert diagram_connector.to_id == to_shape - assert diagram_connector.bidirectional == bidirectional + result = VisioConnectorFactory().create_connector(shape) + + # THEN the strategy implementations are the expected + assert CreateConnectorStrategy.get_strategies().__len__() == 1 + # AND the strategies method implementations are called once + mock_strategy_impl1.assert_called_once() + # AND no result is returned + assert not result diff --git a/slp_visio/tests/unit/load/strategies/dataflow/impl/test_create_connector_by_connects.py b/slp_visio/tests/unit/load/strategies/dataflow/impl/test_create_connector_by_connects.py new file mode 100644 index 00000000..cbfc395a --- /dev/null +++ b/slp_visio/tests/unit/load/strategies/dataflow/impl/test_create_connector_by_connects.py @@ -0,0 +1,122 @@ +from unittest.mock import MagicMock + +import pytest + +from slp_visio.slp_visio.load.strategies.connector.impl.create_connector_by_connects import CreateConnectorByConnects + + +class TestCreateConnectorByConnects: + + def test_create_connector_ok(self): + # GIVEN a visio connector shape + shape = MagicMock( + ID=1001, + connects=[MagicMock(from_rel='BeginX', shape_id=1), MagicMock(from_rel='EndX', shape_id=2)]) + + # WHEN the connector is created + strategy = CreateConnectorByConnects() + diagram_connector = strategy.create_connector(shape) + + # THEN the returned diagram connector has the following properties + assert diagram_connector.id == 1001 + assert diagram_connector.from_id == 1 + assert diagram_connector.to_id == 2 + + def test_create_connector_reversed_ok(self): + # GIVEN a visio connector shape with reversed relationship + shape = MagicMock( + ID=1001, + connects=[MagicMock(from_rel='EndX', shape_id=1), MagicMock(from_rel='BeginX', shape_id=2)]) + + # WHEN we try to create the connector + strategy = CreateConnectorByConnects() + diagram_connector = strategy.create_connector(shape) + + # THEN the returned diagram connector has the following properties + assert diagram_connector.id == 1001 + assert diagram_connector.from_id == 2 + assert diagram_connector.to_id == 1 + + def test_create_connector_incomplete_connectors(self): + # GIVEN a visio connector shape with incomplete connectors + shape = MagicMock( + ID=1001, + connects=[MagicMock(from_rel='BeginX', shape_id=1)]) + + # WHEN we try to create the connector + strategy = CreateConnectorByConnects() + diagram_connector = strategy.create_connector(shape) + + # THEN no diagram connector is returned + assert not diagram_connector + + def test_create_connector_self_pointing(self): + # GIVEN a visio connector shape self pointing + shape = MagicMock( + ID=1001, + connects=[MagicMock(from_rel='BeginX', shape_id=1), MagicMock(from_rel='EndX', shape_id=1)]) + + # WHEN we try to create the connector + strategy = CreateConnectorByConnects() + diagram_connector = strategy.create_connector(shape) + + # THEN no diagram connector is returned + assert not diagram_connector + + @pytest.mark.parametrize('shape,cell_values,master_name,from_shape,to_shape,bidirectional', [ + # Control case + pytest.param(MagicMock(ID=1001, connects=[MagicMock(from_rel='BeginX', shape_id=1), MagicMock(from_rel='EndX', shape_id=2)]), {'BeginArrow': None, 'EndArrow': None}, None, 1, 2, False, id='control'), + pytest.param(MagicMock(ID=1001, connects=[MagicMock(from_rel='BeginX', shape_id=1), MagicMock(from_rel='EndX', shape_id=2)]), {'BeginArrow': None, 'EndArrow': None}, 'Custom Arrow', 1, 2, False, id='control_inverted'), + # Created left to right, arrow changed left to right + pytest.param(MagicMock(ID=1001, connects=[MagicMock(from_rel='BeginX', shape_id=1), MagicMock(from_rel='EndX', shape_id=2)]), {'BeginArrow': None, 'EndArrow': '13'}, None, 1, 2, False, id='left_to_right_arrow_changed_left_to_right'), + pytest.param(MagicMock(ID=1001, connects=[MagicMock(from_rel='EndX', shape_id=2), MagicMock(from_rel='BeginX', shape_id=1)]), {'BeginArrow': None, 'EndArrow': '13'}, None, 1, 2, False, id='left_to_right_arrow_changed_left_to_right_inverted'), + # Created left to right, arrow changed right to left + pytest.param(MagicMock(ID=1001, connects=[MagicMock(from_rel='BeginX', shape_id=1), MagicMock(from_rel='EndX', shape_id=2)]), {'BeginArrow': '13', 'EndArrow': None}, None, 2, 1, False, id='left_to_right_arrow_changed_right_to_left'), + pytest.param(MagicMock(ID=1001, connects=[MagicMock(from_rel='EndX', shape_id=2), MagicMock(from_rel='BeginX', shape_id=1)]), {'BeginArrow': '13', 'EndArrow': None}, None, 2, 1, False, id='left_to_right_arrow_changed_right_to_left_inverted'), + # Created right to left, arrow changed right to left + pytest.param(MagicMock(ID=1001, connects=[MagicMock(from_rel='BeginX', shape_id=2), MagicMock(from_rel='EndX', shape_id=1)]), {'BeginArrow': None, 'EndArrow': '13'}, None, 2, 1, False, id='right_to_left_arrow_changed_right_to_left'), + pytest.param(MagicMock(ID=1001, connects=[MagicMock(from_rel='EndX', shape_id=1), MagicMock(from_rel='BeginX', shape_id=2)]), {'BeginArrow': None, 'EndArrow': '13'}, None, 2, 1, False, id='right_to_left_arrow_changed_right_to_left_inverted'), + # Created right to left, arrow changed left to right + pytest.param(MagicMock(ID=1001, connects=[MagicMock(from_rel='BeginX', shape_id=2), MagicMock(from_rel='EndX', shape_id=1)]), {'BeginArrow': '13', 'EndArrow': None}, None, 1, 2, False, id='right_to_left_arrow_changed_left_to_right'), + pytest.param(MagicMock(ID=1001, connects=[MagicMock(from_rel='EndX', shape_id=1), MagicMock(from_rel='BeginX', shape_id=2)]), {'BeginArrow': '13', 'EndArrow': None}, None, 1, 2, False, id='right_to_left_arrow_changed_left_to_right_inverted'), + # Invalid BeginArrow/EndArrow combinations + pytest.param(MagicMock(ID=1001, connects=[MagicMock(from_rel='BeginX', shape_id=1), MagicMock(from_rel='EndX', shape_id=2)]), {'BeginArrow': '0', 'EndArrow': None}, None, 1, 2, False, id='invalid_begin_0_no_end'), + pytest.param(MagicMock(ID=1001, connects=[MagicMock(from_rel='BeginX', shape_id=1), MagicMock(from_rel='EndX', shape_id=2)]), {'BeginArrow': None, 'EndArrow': '0'}, None, 1, 2, False, id='invalid_no_begin_end_0'), + pytest.param(MagicMock(ID=1001, connects=[MagicMock(from_rel='BeginX', shape_id=1), MagicMock(from_rel='EndX', shape_id=2)]), {'BeginArrow': '0', 'EndArrow': '0'}, None, 1, 2, False, id='invalid_begin_0_end_0'), + # Created left to right, arrow changed to bidirectional + pytest.param(MagicMock(ID=1001, connects=[MagicMock(from_rel='BeginX', shape_id=1), MagicMock(from_rel='EndX', shape_id=2)]), {'BeginArrow': '13', 'EndArrow': '13'}, None, 1, 2, True, id='left_to_right_arrow_changed_bidirectional'), + pytest.param(MagicMock(ID=1001, connects=[MagicMock(from_rel='EndX', shape_id=2), MagicMock(from_rel='BeginX', shape_id=1)]), {'BeginArrow': '13', 'EndArrow': '13'}, None, 2, 1, True, id='left_to_right_arrow_changed_bidirectional_inverted'), + # Created right to left, arrow changed to bidirectional + pytest.param(MagicMock(ID=1001, connects=[MagicMock(from_rel='BeginX', shape_id=2), MagicMock(from_rel='EndX', shape_id=1)]), {'BeginArrow': '13', 'EndArrow': '13'}, None, 2, 1, True, id='right_to_left_arrow_changed_bidirectional'), + pytest.param(MagicMock(ID=1001, connects=[MagicMock(from_rel='EndX', shape_id=1), MagicMock(from_rel='BeginX', shape_id=2)]), {'BeginArrow': '13', 'EndArrow': '13'}, None, 1, 2, True, id='right_to_left_arrow_changed_bidirectional_inverted'), + # Created left to right, shape changed to bidirectional + pytest.param(MagicMock(ID=1001, connects=[MagicMock(from_rel='BeginX', shape_id=1), MagicMock(from_rel='EndX', shape_id=2)]), {'BeginArrow': None, 'EndArrow': None}, 'Simple Double Arrow', 1, 2, True, id='left_to_right_shape_changed_bidirectional'), + pytest.param(MagicMock(ID=1001, connects=[MagicMock(from_rel='EndX', shape_id=2), MagicMock(from_rel='BeginX', shape_id=1)]), {'BeginArrow': None, 'EndArrow': None}, 'Line Double Arrow', 2, 1, True, id='left_to_right_shape_changed_bidirectional_inverted'), + # Created right to left, shape changed to bidirectional + pytest.param(MagicMock(ID=1001, connects=[MagicMock(from_rel='BeginX', shape_id=2), MagicMock(from_rel='EndX', shape_id=1)]), {'BeginArrow': None, 'EndArrow': None}, 'Arced Line Double Arrow', 2, 1, True, id='right_to_left_shape_changed_bidirectional'), + pytest.param(MagicMock(ID=1001, connects=[MagicMock(from_rel='EndX', shape_id=1), MagicMock(from_rel='BeginX', shape_id=2)]), {'BeginArrow': None, 'EndArrow': None}, 'Block Double Arrow', 1, 2, True, id='right_to_left_shape_changed_bidirectional_inverted') + ]) + def test_create_connector_modified_manually(self, shape, cell_values, master_name, from_shape, to_shape, bidirectional): + # GIVEN a mocked visio connector + shape.cell_value.side_effect = cell_values.get + shape.master_page.name = master_name + + # WHEN we try to create the connector + diagram_connector = CreateConnectorByConnects().create_connector(shape) + + # THEN the returned diagram connector has the following properties + assert diagram_connector.id == 1001 + assert diagram_connector.from_id == from_shape + assert diagram_connector.to_id == to_shape + assert diagram_connector.bidirectional == bidirectional + + def test_connector_without_connects(self): + # GIVEN a visio connector shape without connects + shape = MagicMock(ID=1001) + + # WHEN we try to create the connector + strategy = CreateConnectorByConnects() + diagram_connector = strategy.create_connector(shape) + + # THEN no diagram connector is created + assert not diagram_connector diff --git a/slp_visio/tests/unit/load/strategies/dataflow/impl/test_validate_connector_by_connects.py b/slp_visio/tests/unit/load/strategies/dataflow/impl/test_validate_connector_by_connects.py new file mode 100644 index 00000000..3556bb53 --- /dev/null +++ b/slp_visio/tests/unit/load/strategies/dataflow/impl/test_validate_connector_by_connects.py @@ -0,0 +1,47 @@ +from unittest.mock import MagicMock + +import pytest + +from slp_visio.slp_visio.load.strategies.connector.impl.validate_connector_by_connects import \ + ValidateConnectorByConnects + + +class TestValidateConnectorByConnects: + + @pytest.mark.parametrize('connector_shape_id_first,connector_shape_id_second', { + (1001, 1001), + (1001, 1002), + (1002, 1001) + }) + def test_validate_connector_ok(self, connector_shape_id_first, connector_shape_id_second): + # GIVEN a visio connector shape + shape = MagicMock( + ID=1001, + connects=[MagicMock(from_rel='BeginX', shape_id=1, connector_shape_id=connector_shape_id_first), + MagicMock(from_rel='EndX', shape_id=2, connector_shape_id=connector_shape_id_second)]) + + # WHEN the connector is validated + strategy = ValidateConnectorByConnects() + result = strategy.is_connector(shape) + + # THEN is a valid connector + assert result + + @pytest.mark.parametrize('connector_shape_id_first,connector_shape_id_second', { + (1002, 1003), + (0, 0), + (None, None) + }) + def test_validate_with_connects_wrong_connector_shape_id(self, connector_shape_id_first, connector_shape_id_second): + # GIVEN a visio connector shape + shape = MagicMock( + ID=1001, + connects=[MagicMock(from_rel='BeginX', shape_id=1, connector_shape_id=connector_shape_id_first), + MagicMock(from_rel='EndX', shape_id=2, connector_shape_id=connector_shape_id_second)]) + + # WHEN the connector is validated + strategy = ValidateConnectorByConnects() + result = strategy.is_connector(shape) + + # THEN is not a valid connector + assert not result diff --git a/slp_visio/tests/unit/load/strategies/dataflow/test_create_connector_strategy.py b/slp_visio/tests/unit/load/strategies/dataflow/test_create_connector_strategy.py new file mode 100644 index 00000000..af215e30 --- /dev/null +++ b/slp_visio/tests/unit/load/strategies/dataflow/test_create_connector_strategy.py @@ -0,0 +1,15 @@ +from slp_visio.slp_visio.load.strategies.connector.create_connector_strategy import CreateConnectorStrategy +from slp_visio.slp_visio.load.strategies.connector.impl.create_connector_by_connects import CreateConnectorByConnects + + +class TestCreateConnectorStrategy: + + def test_get_strategies(self): + # WHEN we get the strategies from CreateConnectorStrategy + strategies = CreateConnectorStrategy.get_strategies() + + # THEN we have the expected number of strategies + assert strategies.__len__() == 1 + + # AND we have the expected implementations + assert strategies[0].__class__ == CreateConnectorByConnects \ No newline at end of file diff --git a/slp_visio/tests/unit/load/strategies/dataflow/test_validate_connector_strategy.py b/slp_visio/tests/unit/load/strategies/dataflow/test_validate_connector_strategy.py new file mode 100644 index 00000000..1ac31d1f --- /dev/null +++ b/slp_visio/tests/unit/load/strategies/dataflow/test_validate_connector_strategy.py @@ -0,0 +1,16 @@ +from slp_visio.slp_visio.load.strategies.connector.impl.validate_connector_by_connects import \ + ValidateConnectorByConnects +from slp_visio.slp_visio.load.strategies.connector.validate_connector_strategy import ValidateConnectorStrategy + + +class TestValidateConnectorStrategy: + + def test_get_strategies(self): + # WHEN we get the strategies from CreateConnectorStrategy + strategies = ValidateConnectorStrategy.get_strategies() + + # THEN we have the expected number of strategies + assert strategies.__len__() == 1 + + # AND we have the expected implementations + assert strategies[0].__class__ == ValidateConnectorByConnects \ No newline at end of file diff --git a/slp_visio/tests/unit/load/test_vsdx_parser.py b/slp_visio/tests/unit/load/test_vsdx_parser.py new file mode 100644 index 00000000..36137478 --- /dev/null +++ b/slp_visio/tests/unit/load/test_vsdx_parser.py @@ -0,0 +1,29 @@ +from unittest.mock import patch, MagicMock + +import pytest + +from slp_visio.slp_visio.load.strategies.connector.validate_connector_strategy import ValidateConnectorStrategy +from slp_visio.slp_visio.load.vsdx_parser import VsdxParser + + +class TestVsdxParser: + @patch("slp_visio.slp_visio.load.strategies.connector.impl.validate_connector_by_connects.ValidateConnectorByConnects.is_connector") + @pytest.mark.parametrize('impl1_result', [True, False]) + def test_create_connector_when_strategy_returns_value(self, mock_strategy_impl1, impl1_result): + # GIVEN a visio connector shape + shape = MagicMock(ID=1001) + + # AND a diagram connector is returned by the strategy + mock_strategy_impl1.return_value = impl1_result + + # WHEN a connector is created + result = VsdxParser._is_connector(shape) + + # THEN the strategy implementations are the expected + assert ValidateConnectorStrategy.get_strategies().__len__() == 1 + # AND the strategies method implementations are called once + mock_strategy_impl1.assert_called_once() + # AND the result is the expected + assert result == impl1_result + + From 5b9e7fb0db36af69fa0884be7c78ff477265132b Mon Sep 17 00:00:00 2001 From: Santi Manero Date: Mon, 17 Apr 2023 17:46:57 +0200 Subject: [PATCH 04/18] [OPT-780] Extract strategy mapping dataflows - additional tests --- setup.py | 3 +- slp_visio/slp_visio/load/__init__.py | 2 +- .../slp_visio/load/connector_identifier.py | 14 ++ ...gy.py => connector_identifier_strategy.py} | 4 +- ...py => connector_identifier_by_connects.py} | 4 +- slp_visio/slp_visio/load/vsdx_parser.py | 14 +- .../slp_visio/lucid/load/lucid_vsdx_parser.py | 13 -- .../lucid/load/strategies/__init__.py | 0 .../load/strategies/connector/__init__.py | 0 .../strategies/connector/impl/__init__.py | 0 ...connector_identifier_by_lucid_line_name.py | 17 +++ .../objects/test_visio_diagram_factories.py | 120 +++++++++++++++--- ... test_connector_identifier_by_connects.py} | 10 +- .../test_connector_identifier_strategy.py | 19 +++ .../test_create_connector_strategy.py | 2 +- .../test_validate_connector_strategy.py | 16 --- .../unit/load/test_connector_identifier.py | 56 ++++++++ slp_visio/tests/unit/load/test_vsdx_parser.py | 29 ----- slp_visio/tests/unit/lucid/load/__init__.py | 0 .../unit/lucid/load/strategies/__init__.py | 0 .../load/strategies/connector/__init__.py | 0 .../strategies/connector/impl/__init__.py | 0 ...connector_identifier_by_lucid_line_name.py | 46 +++++++ 23 files changed, 271 insertions(+), 98 deletions(-) create mode 100644 slp_visio/slp_visio/load/connector_identifier.py rename slp_visio/slp_visio/load/strategies/connector/{validate_connector_strategy.py => connector_identifier_strategy.py} (82%) rename slp_visio/slp_visio/load/strategies/connector/impl/{validate_connector_by_connects.py => connector_identifier_by_connects.py} (67%) create mode 100644 slp_visio/slp_visio/lucid/load/strategies/__init__.py create mode 100644 slp_visio/slp_visio/lucid/load/strategies/connector/__init__.py create mode 100644 slp_visio/slp_visio/lucid/load/strategies/connector/impl/__init__.py create mode 100644 slp_visio/slp_visio/lucid/load/strategies/connector/impl/connector_identifier_by_lucid_line_name.py rename slp_visio/tests/unit/load/strategies/dataflow/impl/{test_validate_connector_by_connects.py => test_connector_identifier_by_connects.py} (84%) create mode 100644 slp_visio/tests/unit/load/strategies/dataflow/test_connector_identifier_strategy.py delete mode 100644 slp_visio/tests/unit/load/strategies/dataflow/test_validate_connector_strategy.py create mode 100644 slp_visio/tests/unit/load/test_connector_identifier.py delete mode 100644 slp_visio/tests/unit/load/test_vsdx_parser.py create mode 100644 slp_visio/tests/unit/lucid/load/__init__.py create mode 100644 slp_visio/tests/unit/lucid/load/strategies/__init__.py create mode 100644 slp_visio/tests/unit/lucid/load/strategies/connector/__init__.py create mode 100644 slp_visio/tests/unit/lucid/load/strategies/connector/impl/__init__.py create mode 100644 slp_visio/tests/unit/lucid/load/strategies/connector/impl/test_connector_identifier_by_lucid_line_name.py diff --git a/setup.py b/setup.py index 1baba429..e8d45a17 100644 --- a/setup.py +++ b/setup.py @@ -48,7 +48,8 @@ 'pytest==7.2.2', 'responses==0.22.0', 'deepdiff==6.2.3', - 'httpx==0.23.3' + 'httpx==0.23.3', + 'pytest-mock==3.10.0' ], "doc": [ 'mkdocs-material==9.1.1', diff --git a/slp_visio/slp_visio/load/__init__.py b/slp_visio/slp_visio/load/__init__.py index afead209..a6880c30 100644 --- a/slp_visio/slp_visio/load/__init__.py +++ b/slp_visio/slp_visio/load/__init__.py @@ -1 +1 @@ -from .strategies.connector.impl import validate_connector_by_connects, create_connector_by_connects +from .strategies.connector.impl import connector_identifier_by_connects, create_connector_by_connects diff --git a/slp_visio/slp_visio/load/connector_identifier.py b/slp_visio/slp_visio/load/connector_identifier.py new file mode 100644 index 00000000..7324d1ff --- /dev/null +++ b/slp_visio/slp_visio/load/connector_identifier.py @@ -0,0 +1,14 @@ +from vsdx import Shape + +from slp_visio.slp_visio.load.strategies.connector.connector_identifier_strategy import ConnectorIdentifierStrategy + + +class ConnectorIdentifier: + + @staticmethod + def is_connector(shape: Shape) -> bool: + for strategy in ConnectorIdentifierStrategy.get_strategies(): + if strategy.is_connector(shape): + return True + + return False diff --git a/slp_visio/slp_visio/load/strategies/connector/validate_connector_strategy.py b/slp_visio/slp_visio/load/strategies/connector/connector_identifier_strategy.py similarity index 82% rename from slp_visio/slp_visio/load/strategies/connector/validate_connector_strategy.py rename to slp_visio/slp_visio/load/strategies/connector/connector_identifier_strategy.py index 6a2ad7f3..06bc856d 100644 --- a/slp_visio/slp_visio/load/strategies/connector/validate_connector_strategy.py +++ b/slp_visio/slp_visio/load/strategies/connector/connector_identifier_strategy.py @@ -5,9 +5,9 @@ from slp_visio.slp_visio.load.strategies.strategy import Strategy -class ValidateConnectorStrategy(Strategy): +class ConnectorIdentifierStrategy(Strategy): """ - Formal Interface to validate if a shape is a connector + Formal Interface to check if a shape is a connector """ @classmethod diff --git a/slp_visio/slp_visio/load/strategies/connector/impl/validate_connector_by_connects.py b/slp_visio/slp_visio/load/strategies/connector/impl/connector_identifier_by_connects.py similarity index 67% rename from slp_visio/slp_visio/load/strategies/connector/impl/validate_connector_by_connects.py rename to slp_visio/slp_visio/load/strategies/connector/impl/connector_identifier_by_connects.py index d66e47ba..673b63c6 100644 --- a/slp_visio/slp_visio/load/strategies/connector/impl/validate_connector_by_connects.py +++ b/slp_visio/slp_visio/load/strategies/connector/impl/connector_identifier_by_connects.py @@ -1,9 +1,9 @@ from vsdx import Shape -from slp_visio.slp_visio.load.strategies.connector.validate_connector_strategy import ValidateConnectorStrategy +from slp_visio.slp_visio.load.strategies.connector.connector_identifier_strategy import ConnectorIdentifierStrategy -class ValidateConnectorByConnects(ValidateConnectorStrategy): +class ConnectorIdentifierByConnects(ConnectorIdentifierStrategy): """ Strategy to know if a shape is a connector The shape must have connects and each connector_shape_id must match with the shape id diff --git a/slp_visio/slp_visio/load/vsdx_parser.py b/slp_visio/slp_visio/load/vsdx_parser.py index 90fb376f..9320f8d9 100644 --- a/slp_visio/slp_visio/load/vsdx_parser.py +++ b/slp_visio/slp_visio/load/vsdx_parser.py @@ -1,11 +1,11 @@ from vsdx import Shape, VisioFile from slp_base import DiagramType +from slp_visio.slp_visio.load.connector_identifier import ConnectorIdentifier from slp_visio.slp_visio.load.objects.diagram_objects import Diagram, DiagramComponentOrigin, DiagramLimits from slp_visio.slp_visio.load.parent_calculator import ParentCalculator from slp_visio.slp_visio.load.representation.simple_component_representer import SimpleComponentRepresenter from slp_visio.slp_visio.load.representation.zone_component_representer import ZoneComponentRepresenter -from slp_visio.slp_visio.load.strategies.connector.validate_connector_strategy import ValidateConnectorStrategy from slp_visio.slp_visio.util.visio import get_limits, get_shape_text DIAGRAM_LIMITS_PADDING = 2 @@ -42,20 +42,12 @@ def parse(self, visio_diagram_filename) -> Diagram: return Diagram(DiagramType.VISIO, self._visio_components, self._visio_connectors, diagram_limits) - @staticmethod - def _is_connector(shape: Shape) -> bool: - for strategy in ValidateConnectorStrategy.get_strategies(): - if strategy.is_connector(shape): - return True - - return False - @staticmethod def _is_boundary(shape: Shape) -> bool: return shape.shape_name is not None and 'Curved panel' in shape.shape_name def _is_component(self, shape: Shape) -> bool: - return get_shape_text(shape) and not self._is_connector(shape) + return get_shape_text(shape) and not ConnectorIdentifier.is_connector(shape) def __calculate_diagram_limits(self) -> DiagramLimits: floor_coordinates = [None, None] @@ -80,7 +72,7 @@ def __calculate_diagram_limits(self) -> DiagramLimits: def _load_page_elements(self): for shape in self.page.child_shapes: - if self._is_connector(shape): + if ConnectorIdentifier.is_connector(shape): self._add_connector(shape) elif self._is_boundary(shape): self._add_boundary_component(shape) diff --git a/slp_visio/slp_visio/lucid/load/lucid_vsdx_parser.py b/slp_visio/slp_visio/lucid/load/lucid_vsdx_parser.py index ba67ec98..d0afb563 100644 --- a/slp_visio/slp_visio/lucid/load/lucid_vsdx_parser.py +++ b/slp_visio/slp_visio/lucid/load/lucid_vsdx_parser.py @@ -3,22 +3,9 @@ from slp_visio.slp_visio.load.parent_calculator import ParentCalculator from slp_visio.slp_visio.load.vsdx_parser import VsdxParser -LUCID_LINE = 'com.lucidchart.Line' - class LucidVsdxParser(VsdxParser): - @staticmethod - def _is_connector(shape: Shape) -> bool: - for connect in shape.connects: - if shape.ID == connect.connector_shape_id: - return True - - if shape.shape_name and shape.shape_name.startswith(f'{LUCID_LINE}'): - return True - - return False - def _add_connector(self, connector_shape: Shape): shape_components = [c for c in self.page.child_shapes if self._is_component(c) and not self._is_boundary(c)] diff --git a/slp_visio/slp_visio/lucid/load/strategies/__init__.py b/slp_visio/slp_visio/lucid/load/strategies/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/slp_visio/slp_visio/lucid/load/strategies/connector/__init__.py b/slp_visio/slp_visio/lucid/load/strategies/connector/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/slp_visio/slp_visio/lucid/load/strategies/connector/impl/__init__.py b/slp_visio/slp_visio/lucid/load/strategies/connector/impl/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/slp_visio/slp_visio/lucid/load/strategies/connector/impl/connector_identifier_by_lucid_line_name.py b/slp_visio/slp_visio/lucid/load/strategies/connector/impl/connector_identifier_by_lucid_line_name.py new file mode 100644 index 00000000..fee3b3aa --- /dev/null +++ b/slp_visio/slp_visio/lucid/load/strategies/connector/impl/connector_identifier_by_lucid_line_name.py @@ -0,0 +1,17 @@ +from vsdx import Shape + +from slp_visio.slp_visio.load.strategies.connector.connector_identifier_strategy import ConnectorIdentifierStrategy + +LUCID_LINE = 'com.lucidchart.Line' + + +class ConnectorIdentifierByLucidLineName(ConnectorIdentifierStrategy): + """ + Strategy to know if a shape is a connector + The shape name must start with "com.lucidchart.Line" + """ + + def is_connector(self, shape: Shape) -> bool: + name = shape.shape_name + if name and name.startswith(f'{LUCID_LINE}'): + return True diff --git a/slp_visio/tests/unit/load/objects/test_visio_diagram_factories.py b/slp_visio/tests/unit/load/objects/test_visio_diagram_factories.py index f0bbc7ce..18870fc1 100644 --- a/slp_visio/tests/unit/load/objects/test_visio_diagram_factories.py +++ b/slp_visio/tests/unit/load/objects/test_visio_diagram_factories.py @@ -1,20 +1,49 @@ -from unittest.mock import patch, MagicMock +from typing import List, Union +from unittest.mock import Mock +import pytest +from pytest import fixture, mark, param + +from slp_visio.slp_visio.load.objects.diagram_objects import DiagramConnector from slp_visio.slp_visio.load.objects.visio_diagram_factories import VisioConnectorFactory from slp_visio.slp_visio.load.strategies.connector.create_connector_strategy import CreateConnectorStrategy +def mocked_strategy(result: Union[DiagramConnector, None, Exception]): + strategy = Mock() + strategy.create_connector = Mock(side_effect=[result]) + return strategy + + +def mocked_strategies(results: List[Union[DiagramConnector, None, Exception]]): + return list(map(mocked_strategy, results)) + + +def mocked_connector(c_id: int = 1001, from_id: int = 1, to_id: int = 2): + return Mock(id=c_id, from_id=from_id, to_id=to_id) + + +@fixture +def strategies(): + return [] + + +@fixture(autouse=True) +def mock_get_strategies(mocker, strategies): + return mocker.patch( + 'slp_visio.slp_visio.load.strategies.connector.create_connector_strategy.CreateConnectorStrategy.get_strategies', + return_value=strategies) + + class TestVisioConnectorFactory: - @patch( - "slp_visio.slp_visio.load.strategies.connector.impl.create_connector_by_connects.CreateConnectorByConnects.create_connector") - def test_create_connector_when_strategy_returns_value(self, mock_strategy_impl1): + def test_create_connector_when_strategy_returns_value(self, mock_get_strategies): # GIVEN a visio connector shape - shape = MagicMock(ID=1001) + shape = Mock(ID=1001) - # AND a diagram connector is returned by the strategy - diagram_connector = MagicMock(id=1001, from_id=1, to_id=2) - mock_strategy_impl1.return_value = diagram_connector + # AND only one strategy that returns a connector + strategy = mocked_strategy(mocked_connector()) + mock_get_strategies.return_value = [strategy] # WHEN a connector is created result = VisioConnectorFactory().create_connector(shape) @@ -22,27 +51,84 @@ def test_create_connector_when_strategy_returns_value(self, mock_strategy_impl1) # THEN the strategy implementations are the expected assert CreateConnectorStrategy.get_strategies().__len__() == 1 # AND the strategies method implementations are called once - mock_strategy_impl1.assert_called_once() + strategy.create_connector.assert_called_once() # AND the result is the expected assert result.id == 1001 assert result.to_id == 2 assert result.from_id == 1 - @patch( - "slp_visio.slp_visio.load.strategies.connector.impl.create_connector_by_connects.CreateConnectorByConnects.create_connector") - def test_create_connector_when_strategy_does_not_return_value(self, mock_strategy_impl1): + @mark.parametrize('strategies,_id,_from,_to,valid_strategy', [ + param(mocked_strategies( + [mocked_connector(c_id=1001, from_id=1, to_id=2), + mocked_connector(c_id=2002, from_id=2, to_id=1)]), + 1001, 1, 2, 0), + param(mocked_strategies( + [mocked_connector(c_id=1001, from_id=1, to_id=2), + None]), + 1001, 1, 2, 0), + param(mocked_strategies( + [None, + mocked_connector(c_id=2002, from_id=9, to_id=21)]), + 2002, 9, 21, 1), + param(mocked_strategies( + [None, + None, + mocked_connector(c_id=3003, from_id=5, to_id=7)]), + 3003, 5, 7, 2) + ]) + def test_create_connector_when_some_strategy_return_value(self, strategies, _id, _from, _to, valid_strategy: int): # GIVEN a visio connector shape - shape = MagicMock(ID=1001) + shape = Mock(ID=1001) - # AND no diagram connector is returned by the strategy - mock_strategy_impl1.return_value = None + # WHEN a connector is created + result = VisioConnectorFactory().create_connector(shape) + + # THEN we call strategies until we find the first valid strategy + for i in range(0, valid_strategy): + strategies[i].create_connector.assert_called_once() + for i in range(valid_strategy + 1, len(strategies)): + strategies[i].create_connector.assert_not_called() + # AND the result is the returned by the first strategy + assert result.id == _id + assert result.to_id == _to + assert result.from_id == _from + + @mark.parametrize('strategies', [ + param(mocked_strategies([Exception("Some Error")]), id='one error'), + param(mocked_strategies([Exception("Some Error"), Exception("Other Error")]), id='two errors'), + param(mocked_strategies([Exception("Some Error"), mocked_connector()]), id='first error, second valid'), + ]) + def test_create_connector_when_some_strategy_return_error(self, strategies): + # GIVEN a visio connector shape + shape = Mock(ID=1001) + + # WHEN a connector is created + with pytest.raises(Exception) as error: + VisioConnectorFactory().create_connector(shape) + + # THEN the strategy implementations are the expected + assert CreateConnectorStrategy.get_strategies().__len__() == len(strategies) + + # AND the first strategy is called + strategies[0].create_connector.assert_called_once() + + # AND the error is propagated + assert error.value.args[0] == 'Some Error' + + @mark.parametrize('strategies', [ + param(mocked_strategies([None]), id='one strategy'), + param(mocked_strategies([None, None]), id='two strategies') + ]) + def test_create_connector_when_strategy_does_not_return_value(self, strategies): + # GIVEN a visio connector shape + shape = Mock(ID=1001) # WHEN a connector is created result = VisioConnectorFactory().create_connector(shape) # THEN the strategy implementations are the expected - assert CreateConnectorStrategy.get_strategies().__len__() == 1 + assert CreateConnectorStrategy.get_strategies().__len__() == len(strategies) # AND the strategies method implementations are called once - mock_strategy_impl1.assert_called_once() + strategies[0].create_connector.assert_called_once() # AND no result is returned assert not result diff --git a/slp_visio/tests/unit/load/strategies/dataflow/impl/test_validate_connector_by_connects.py b/slp_visio/tests/unit/load/strategies/dataflow/impl/test_connector_identifier_by_connects.py similarity index 84% rename from slp_visio/tests/unit/load/strategies/dataflow/impl/test_validate_connector_by_connects.py rename to slp_visio/tests/unit/load/strategies/dataflow/impl/test_connector_identifier_by_connects.py index 3556bb53..ca7b998c 100644 --- a/slp_visio/tests/unit/load/strategies/dataflow/impl/test_validate_connector_by_connects.py +++ b/slp_visio/tests/unit/load/strategies/dataflow/impl/test_connector_identifier_by_connects.py @@ -2,11 +2,11 @@ import pytest -from slp_visio.slp_visio.load.strategies.connector.impl.validate_connector_by_connects import \ - ValidateConnectorByConnects +from slp_visio.slp_visio.load.strategies.connector.impl.connector_identifier_by_connects import \ + ConnectorIdentifierByConnects -class TestValidateConnectorByConnects: +class TestConnectorIdentifierByConnects: @pytest.mark.parametrize('connector_shape_id_first,connector_shape_id_second', { (1001, 1001), @@ -21,7 +21,7 @@ def test_validate_connector_ok(self, connector_shape_id_first, connector_shape_i MagicMock(from_rel='EndX', shape_id=2, connector_shape_id=connector_shape_id_second)]) # WHEN the connector is validated - strategy = ValidateConnectorByConnects() + strategy = ConnectorIdentifierByConnects() result = strategy.is_connector(shape) # THEN is a valid connector @@ -40,7 +40,7 @@ def test_validate_with_connects_wrong_connector_shape_id(self, connector_shape_i MagicMock(from_rel='EndX', shape_id=2, connector_shape_id=connector_shape_id_second)]) # WHEN the connector is validated - strategy = ValidateConnectorByConnects() + strategy = ConnectorIdentifierByConnects() result = strategy.is_connector(shape) # THEN is not a valid connector diff --git a/slp_visio/tests/unit/load/strategies/dataflow/test_connector_identifier_strategy.py b/slp_visio/tests/unit/load/strategies/dataflow/test_connector_identifier_strategy.py new file mode 100644 index 00000000..ad3b637d --- /dev/null +++ b/slp_visio/tests/unit/load/strategies/dataflow/test_connector_identifier_strategy.py @@ -0,0 +1,19 @@ +from slp_visio.slp_visio.load.strategies.connector.impl.connector_identifier_by_connects import \ + ConnectorIdentifierByConnects +from slp_visio.slp_visio.load.strategies.connector.connector_identifier_strategy import ConnectorIdentifierStrategy +from slp_visio.slp_visio.lucid.load.strategies.connector.impl.connector_identifier_by_lucid_line_name import \ + ConnectorIdentifierByLucidLineName + + +class TestConnectorIdentifierStrategy: + + def test_get_strategies(self): + # WHEN we get the strategies from CreateConnectorStrategy + strategies = ConnectorIdentifierStrategy.get_strategies() + + # THEN we have the expected number of strategies + assert strategies.__len__() == 2 + + # AND we have the expected implementations + assert strategies[0].__class__ == ConnectorIdentifierByConnects + assert strategies[1].__class__ == ConnectorIdentifierByLucidLineName \ No newline at end of file diff --git a/slp_visio/tests/unit/load/strategies/dataflow/test_create_connector_strategy.py b/slp_visio/tests/unit/load/strategies/dataflow/test_create_connector_strategy.py index af215e30..ba37cb5e 100644 --- a/slp_visio/tests/unit/load/strategies/dataflow/test_create_connector_strategy.py +++ b/slp_visio/tests/unit/load/strategies/dataflow/test_create_connector_strategy.py @@ -12,4 +12,4 @@ def test_get_strategies(self): assert strategies.__len__() == 1 # AND we have the expected implementations - assert strategies[0].__class__ == CreateConnectorByConnects \ No newline at end of file + assert strategies[0].__class__ == CreateConnectorByConnects diff --git a/slp_visio/tests/unit/load/strategies/dataflow/test_validate_connector_strategy.py b/slp_visio/tests/unit/load/strategies/dataflow/test_validate_connector_strategy.py deleted file mode 100644 index 1ac31d1f..00000000 --- a/slp_visio/tests/unit/load/strategies/dataflow/test_validate_connector_strategy.py +++ /dev/null @@ -1,16 +0,0 @@ -from slp_visio.slp_visio.load.strategies.connector.impl.validate_connector_by_connects import \ - ValidateConnectorByConnects -from slp_visio.slp_visio.load.strategies.connector.validate_connector_strategy import ValidateConnectorStrategy - - -class TestValidateConnectorStrategy: - - def test_get_strategies(self): - # WHEN we get the strategies from CreateConnectorStrategy - strategies = ValidateConnectorStrategy.get_strategies() - - # THEN we have the expected number of strategies - assert strategies.__len__() == 1 - - # AND we have the expected implementations - assert strategies[0].__class__ == ValidateConnectorByConnects \ No newline at end of file diff --git a/slp_visio/tests/unit/load/test_connector_identifier.py b/slp_visio/tests/unit/load/test_connector_identifier.py new file mode 100644 index 00000000..e3804fb6 --- /dev/null +++ b/slp_visio/tests/unit/load/test_connector_identifier.py @@ -0,0 +1,56 @@ +from typing import List, Union +from unittest.mock import MagicMock, Mock + +import pytest +from pytest import fixture + +from slp_visio.slp_visio.load.connector_identifier import ConnectorIdentifier + + +def mocked_strategy(result: Union[bool, Exception]): + strategy = Mock() + strategy.is_connector = Mock(side_effect=[result]) + return strategy + + +def mocked_strategies(results: List[Union[bool, Exception]]): + return list(map(mocked_strategy, results)) + + +@fixture +def strategies(): + return [] + + +@fixture(autouse=True) +def mock_get_strategies(mocker, strategies): + return mocker.patch( + 'slp_visio.slp_visio.load.strategies.connector.connector_identifier_strategy.ConnectorIdentifierStrategy.get_strategies', + return_value=strategies) + + +class TestConnectorIdentifier: + + @pytest.mark.parametrize('strategies,valid_strategy,expected', [ + (mocked_strategies([True, True]), 0, True), + (mocked_strategies([True, False]), 0, True), + (mocked_strategies([False, True]), 1, True), + (mocked_strategies([False, False]), 2, False), + (mocked_strategies([True, False, False]), 0, True), + (mocked_strategies([False, False, True]), 2, True), + (mocked_strategies([False, False, False]), 3, False) + ]) + def test_create_connector_when_strategy_returns_value(self, strategies, valid_strategy: int, expected): + # GIVEN a visio connector shape + shape = MagicMock(ID=1001) + + # WHEN we check if the shape is a connector + result = ConnectorIdentifier.is_connector(shape) + + # THEN we call strategies until we find the first valid strategy + for i in range(0, valid_strategy): + strategies[i].is_connector.assert_called_once() + for i in range(valid_strategy + 1, len(strategies)): + strategies[i].create_connector.assert_not_called() + # AND the result is true if any strategy returns true + assert result == expected diff --git a/slp_visio/tests/unit/load/test_vsdx_parser.py b/slp_visio/tests/unit/load/test_vsdx_parser.py deleted file mode 100644 index 36137478..00000000 --- a/slp_visio/tests/unit/load/test_vsdx_parser.py +++ /dev/null @@ -1,29 +0,0 @@ -from unittest.mock import patch, MagicMock - -import pytest - -from slp_visio.slp_visio.load.strategies.connector.validate_connector_strategy import ValidateConnectorStrategy -from slp_visio.slp_visio.load.vsdx_parser import VsdxParser - - -class TestVsdxParser: - @patch("slp_visio.slp_visio.load.strategies.connector.impl.validate_connector_by_connects.ValidateConnectorByConnects.is_connector") - @pytest.mark.parametrize('impl1_result', [True, False]) - def test_create_connector_when_strategy_returns_value(self, mock_strategy_impl1, impl1_result): - # GIVEN a visio connector shape - shape = MagicMock(ID=1001) - - # AND a diagram connector is returned by the strategy - mock_strategy_impl1.return_value = impl1_result - - # WHEN a connector is created - result = VsdxParser._is_connector(shape) - - # THEN the strategy implementations are the expected - assert ValidateConnectorStrategy.get_strategies().__len__() == 1 - # AND the strategies method implementations are called once - mock_strategy_impl1.assert_called_once() - # AND the result is the expected - assert result == impl1_result - - diff --git a/slp_visio/tests/unit/lucid/load/__init__.py b/slp_visio/tests/unit/lucid/load/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/slp_visio/tests/unit/lucid/load/strategies/__init__.py b/slp_visio/tests/unit/lucid/load/strategies/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/slp_visio/tests/unit/lucid/load/strategies/connector/__init__.py b/slp_visio/tests/unit/lucid/load/strategies/connector/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/slp_visio/tests/unit/lucid/load/strategies/connector/impl/__init__.py b/slp_visio/tests/unit/lucid/load/strategies/connector/impl/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/slp_visio/tests/unit/lucid/load/strategies/connector/impl/test_connector_identifier_by_lucid_line_name.py b/slp_visio/tests/unit/lucid/load/strategies/connector/impl/test_connector_identifier_by_lucid_line_name.py new file mode 100644 index 00000000..f3a2ba9a --- /dev/null +++ b/slp_visio/tests/unit/lucid/load/strategies/connector/impl/test_connector_identifier_by_lucid_line_name.py @@ -0,0 +1,46 @@ +from _elementtree import Element + +import pytest +from vsdx import Shape + +from slp_visio.slp_visio.lucid.load.strategies.connector.impl.connector_identifier_by_lucid_line_name import \ + ConnectorIdentifierByLucidLineName + + +class TestConnectorIdentifierByLucidLineName: + + @pytest.mark.parametrize('name', { + 'com.lucidchart.Line', + 'com.lucidchart.Line.30', + 'com.lucidchart.Line.abc.def', + }) + def test_validate_connector_ok(self, name): + # GIVEN a visio connector shape + element = Element('Shape', {'NameU': name}) + shape = Shape(element, None, None) + + # WHEN the connector is validated + strategy = ConnectorIdentifierByLucidLineName() + result = strategy.is_connector(shape) + + # THEN is a valid connector + assert result + + @pytest.mark.parametrize('name', { + 'lucidchart.Line', + 'Line.30', + 'com_lucidchart_Line.abc.def', + '', + None + }) + def test_validate_connector_wrong_name(self, name): + # GIVEN a visio connector shape + element = Element('Shape', {'NameU': name}) + shape = Shape(element, None, None) + + # WHEN the connector is validated + strategy = ConnectorIdentifierByLucidLineName() + result = strategy.is_connector(shape) + + # THEN is a valid connector + assert not result From 3dcd59225402bdeb8c14646a80b2161da2922b8e Mon Sep 17 00:00:00 2001 From: Daniel Font Date: Tue, 25 Apr 2023 15:33:17 +0200 Subject: [PATCH 05/18] [OPT-828] Update python dependencies. --- setup.py | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/setup.py b/setup.py index 1baba429..1287e1fc 100644 --- a/setup.py +++ b/setup.py @@ -21,14 +21,14 @@ 'jmespath==1.0.1', 'python-hcl2==4.3.0', 'requests==2.28.2', - 'fastapi==0.93.0', + 'fastapi==0.95.1', 'python-multipart==0.0.5', 'click==8.1.3', - 'uvicorn==0.20.0', + 'uvicorn==0.21.1', 'shapely==2.0.1', 'vsdx==0.5.13', 'python-magic==0.4.27', - 'setuptools==65.5.1', + 'setuptools==67.7.2', 'defusedxml==0.7.1', 'networkx==3.0', 'pygraphviz==1.10' @@ -44,16 +44,15 @@ "pytest-runner==6.0.0", ], "test": [ - 'tox==4.4.6', - 'pytest==7.2.2', - 'responses==0.22.0', - 'deepdiff==6.2.3', + 'tox==4.5.0', + 'pytest==7.3.1', + 'responses==0.23.1', + 'deepdiff==6.3.0', 'httpx==0.23.3' ], "doc": [ - 'mkdocs-material==9.1.1', - 'pymdown-extensions==9.10', - 'mkdocs-glightbox==0.3.1' + 'mkdocs-material==9.1.8', + 'mkdocs-glightbox==0.3.3' ] }, entry_points=''' From 840c26f2df382e915775bce514ff8bcc5601a808 Mon Sep 17 00:00:00 2001 From: Santi Manero Date: Wed, 26 Apr 2023 12:20:25 +0200 Subject: [PATCH 06/18] [OPT-780] Added create_connector_by_line_coordinates strategy --- slp_visio/slp_visio/load/__init__.py | 3 +- .../connector/create_connector_strategy.py | 2 +- .../impl/create_connector_by_connects.py | 2 +- .../create_connector_by_line_coordinates.py | 48 +++++++ slp_visio/slp_visio/lucid/load/__init__.py | 1 + .../load/objects/lucid_diagram_factories.py | 35 +---- ...st_create_connector_by_line_coordinates.py | 96 ++++++++++++++ .../test_create_connector_strategy.py | 5 +- .../tests/unit/lucid/load/objects/__init__.py | 0 .../objects/test_lucid_diagram_factories.py | 120 ++++++++++++++++++ 10 files changed, 280 insertions(+), 32 deletions(-) create mode 100644 slp_visio/slp_visio/load/strategies/connector/impl/create_connector_by_line_coordinates.py create mode 100644 slp_visio/tests/unit/load/strategies/dataflow/impl/test_create_connector_by_line_coordinates.py create mode 100644 slp_visio/tests/unit/lucid/load/objects/__init__.py create mode 100644 slp_visio/tests/unit/lucid/load/objects/test_lucid_diagram_factories.py diff --git a/slp_visio/slp_visio/load/__init__.py b/slp_visio/slp_visio/load/__init__.py index a6880c30..630660db 100644 --- a/slp_visio/slp_visio/load/__init__.py +++ b/slp_visio/slp_visio/load/__init__.py @@ -1 +1,2 @@ -from .strategies.connector.impl import connector_identifier_by_connects, create_connector_by_connects +from .strategies.connector.impl import connector_identifier_by_connects, create_connector_by_connects, \ + create_connector_by_line_coordinates diff --git a/slp_visio/slp_visio/load/strategies/connector/create_connector_strategy.py b/slp_visio/slp_visio/load/strategies/connector/create_connector_strategy.py index 64646424..8b0f1948 100644 --- a/slp_visio/slp_visio/load/strategies/connector/create_connector_strategy.py +++ b/slp_visio/slp_visio/load/strategies/connector/create_connector_strategy.py @@ -19,6 +19,6 @@ def __subclasshook__(cls, subclass): or NotImplemented) @abc.abstractmethod - def create_connector(self, shape: Shape) -> Optional[DiagramConnector]: + def create_connector(self, shape: Shape, **kwargs) -> Optional[DiagramConnector]: """creates the OTM Dataflow from the vsdx shape""" raise NotImplementedError diff --git a/slp_visio/slp_visio/load/strategies/connector/impl/create_connector_by_connects.py b/slp_visio/slp_visio/load/strategies/connector/impl/create_connector_by_connects.py index b224a3f2..00153822 100644 --- a/slp_visio/slp_visio/load/strategies/connector/impl/create_connector_by_connects.py +++ b/slp_visio/slp_visio/load/strategies/connector/impl/create_connector_by_connects.py @@ -11,7 +11,7 @@ class CreateConnectorByConnects(CreateConnectorStrategy): Strategy to create a connector from the shape connects """ - def create_connector(self, shape: Shape) -> Optional[DiagramConnector]: + def create_connector(self, shape: Shape, **kwargs) -> Optional[DiagramConnector]: connected_shapes = shape.connects if not self.are_two_different_shapes(connected_shapes): return None diff --git a/slp_visio/slp_visio/load/strategies/connector/impl/create_connector_by_line_coordinates.py b/slp_visio/slp_visio/load/strategies/connector/impl/create_connector_by_line_coordinates.py new file mode 100644 index 00000000..7a63947a --- /dev/null +++ b/slp_visio/slp_visio/load/strategies/connector/impl/create_connector_by_line_coordinates.py @@ -0,0 +1,48 @@ +from typing import Optional + +from shapely.geometry import Point +from vsdx import Shape + +from slp_visio.slp_visio.load.objects.diagram_objects import DiagramConnector +from slp_visio.slp_visio.load.representation.simple_component_representer import SimpleComponentRepresenter +from slp_visio.slp_visio.load.strategies.connector.create_connector_strategy import CreateConnectorStrategy + + +class CreateConnectorByLineCoordinates(CreateConnectorStrategy): + """ + Strategy to create a connector from the shape begin and end coordinates + + We search a component shape that touch the beginning line coordinates of the given shape connector + We search a component shape that touch the ending line coordinates of the given shape connector + If we find both shapes at the end and at the beginning of the line, we create the connector + """ + + def __init__(self): + self.tolerance = 0.09 + self.representer: SimpleComponentRepresenter() = SimpleComponentRepresenter() + + def create_connector(self, shape: Shape, **kwargs) -> Optional[DiagramConnector]: + if not shape.begin_x or not shape.begin_y or not shape.end_x or not shape.end_y: + return None + begin_line = Point(shape.begin_x, shape.begin_y) + end_line = Point(shape.end_x, shape.end_y) + if not begin_line or not end_line: + return None + + components = kwargs.get('components') + if not components: + return None + origin = self.__match_component(begin_line, components) + target = self.__match_component(end_line, components) + + if not origin or not target: + return None + + return DiagramConnector(shape.ID, origin, target, name=shape.text) + + def __match_component(self, point, components): + for component in components: + polygon = self.representer.build_representation(component) + distance = polygon.exterior.distance(point) + if round(distance, 2) <= self.tolerance: + return component.ID diff --git a/slp_visio/slp_visio/lucid/load/__init__.py b/slp_visio/slp_visio/lucid/load/__init__.py index e69de29b..33f6d288 100644 --- a/slp_visio/slp_visio/lucid/load/__init__.py +++ b/slp_visio/slp_visio/lucid/load/__init__.py @@ -0,0 +1 @@ +from .strategies.connector.impl import connector_identifier_by_lucid_line_name \ No newline at end of file diff --git a/slp_visio/slp_visio/lucid/load/objects/lucid_diagram_factories.py b/slp_visio/slp_visio/lucid/load/objects/lucid_diagram_factories.py index d03cc4b5..5c7e1b88 100644 --- a/slp_visio/slp_visio/lucid/load/objects/lucid_diagram_factories.py +++ b/slp_visio/slp_visio/lucid/load/objects/lucid_diagram_factories.py @@ -1,10 +1,9 @@ from typing import Optional -from shapely.geometry import Point from vsdx import Shape from slp_visio.slp_visio.load.objects.diagram_objects import DiagramComponent, DiagramConnector -from slp_visio.slp_visio.load.representation.simple_component_representer import SimpleComponentRepresenter +from slp_visio.slp_visio.load.strategies.connector.create_connector_strategy import CreateConnectorStrategy from slp_visio.slp_visio.util.visio import get_shape_text, get_master_shape_text LUCID_COMPONENT_PREFIX = 'com.lucidchart' @@ -39,29 +38,9 @@ def create_component(shape, origin, representer) -> DiagramComponent: class LucidConnectorFactory: - def __init__(self): - self.tolerance = 0.09 - self.representer: SimpleComponentRepresenter() = SimpleComponentRepresenter() - - def create_connector(self, shape: Shape, components: [Shape]) -> Optional[DiagramConnector]: - - begin_line = Point(shape.begin_x, shape.begin_y) - end_line = Point(shape.end_x, shape.end_y) - if not begin_line or not end_line: - return None - - origin = self.__match_component(begin_line, components) - target = self.__match_component(end_line, components) - - if not origin or not target: - return None - - return DiagramConnector(shape.ID, origin, target, name=shape.text) - - def __match_component(self, point, components): - - for component in components: - polygon = self.representer.build_representation(component) - distance = polygon.exterior.distance(point) - if distance <= self.tolerance: - return component.ID + @staticmethod + def create_connector(shape: Shape, components: [Shape]) -> Optional[DiagramConnector]: + for strategy in CreateConnectorStrategy.get_strategies(): + connector = strategy.create_connector(shape, components=components) + if connector: + return connector diff --git a/slp_visio/tests/unit/load/strategies/dataflow/impl/test_create_connector_by_line_coordinates.py b/slp_visio/tests/unit/load/strategies/dataflow/impl/test_create_connector_by_line_coordinates.py new file mode 100644 index 00000000..081837fb --- /dev/null +++ b/slp_visio/tests/unit/load/strategies/dataflow/impl/test_create_connector_by_line_coordinates.py @@ -0,0 +1,96 @@ +from unittest.mock import MagicMock, Mock + +from pytest import mark + +from slp_visio.slp_visio.load.strategies.connector.impl.create_connector_by_line_coordinates import \ + CreateConnectorByLineCoordinates + + +def mock_component(_id, pos): + x, y = pos[0], pos[1] + width, height = pos[2], pos[3] + return MagicMock(ID=_id, begin_x=x, begin_y=y, + cells={'Width': Mock(value=width), 'Height': Mock(value=height)}, + center_x_y=(float(x) + float(width) / 2, float(y) + float(height) / 2)) + + +class TestCreateConnectorByLineCoordinates: + + @mark.parametrize('line,start,end', [ + (['1040', '-560', '1290', '-560'], ['960', '-600', '80', '80'], ['1290', '-590', '60', '60']), + ([1040, -560, 1290, -560], [960, -600, 80, 80], [1290, -590, 60, 60]), + (['1.04', '-0.56', '1.290', '-0.56'], ['0.96', '-0.6', '0.08', '0.08'], ['1.290', '-0.590', '0.06', '0.06']), + ([1.04, -0.56, 1.290, -0.56], [0.96, -0.6, 0.08, 0.08], [1.290, -0.590, 0.06, 0.06]), + ([1.04 + 0.09, -0.56, 1.290, -0.56], [0.96, -0.6, 0.08, 0.08], [1.290, -0.590, 0.06, 0.06]), + ([0.96 - 0.09, -0.56, 1.290, -0.56], [0.96, -0.6, 0.08, 0.08], [1.290, -0.590, 0.06, 0.06]), + ([1.04, -0.6 + 0.08 + 0.09, 1.290, -0.56], [0.96, -0.6, 0.08, 0.08], [1.290, -0.590, 0.06, 0.06]), + ([1.04, -0.56, 1.29 - 0.09, -0.56], [0.96, -0.6, 0.08, 0.08], [1.290, -0.590, 0.06, 0.06]), + ([1.04, -0.56, 1.29 + 0.06 + 0.09, -0.56], [0.96, -0.6, 0.08, 0.08], [1.290, -0.590, 0.06, 0.06]), + ([1.04, -0.56, 1.29, -0.59 - 0.09], [0.96, -0.6, 0.08, 0.08], [1.290, -0.590, 0.06, 0.06]), + ([1.04, -0.56, 1.29, -0.59 + 0.06 + 0.09], [0.96, -0.6, 0.08, 0.08], [1.290, -0.590, 0.06, 0.06]), + ]) + def test_create_connector_with_line_touching_component(self, line, start, end): + # GIVEN a visio connector shape + shape = Mock(ID=1111, begin_x=line[0], begin_y=line[1], end_x=line[2], end_y=line[3]) + # AND the start component + start_component = mock_component(222, start) + # AND the end component + end_component = mock_component(333, end) + + # WHEN the connector is created + strategy = CreateConnectorByLineCoordinates() + diagram_connector = strategy.create_connector(shape, components=[start_component, end_component]) + + # THEN the returned diagram connector has the following properties + assert diagram_connector.id == 1111 + assert diagram_connector.from_id == 222 + assert diagram_connector.to_id == 333 + assert not diagram_connector.bidirectional + + @mark.parametrize('line,start,end', [ + ([1.04 + 0.09 + 0.006, -0.56, 1.290, -0.56], [0.96, -0.6, 0.08, 0.08], [1.290, -0.590, 0.06, 0.06]), + ([1.04 + 0.09 + 0.006, -0.56, 1.290, -0.56], [0.96, -0.6, 0.08, 0.08], [1.290, -0.590, 0.06, 0.06]), + ([1.04 + 0.09 + 0.006, -0.56, 1.290, -0.56], [0.96, -0.6, 0.08, 0.08], [1.290, -0.590, 0.06, 0.06]), + ([0.96 - 0.09 - 0.006, -0.56, 1.290, -0.56], [0.96, -0.6, 0.08, 0.08], [1.290, -0.590, 0.06, 0.06]), + ([1.04, -0.6 + 0.08 + 0.09 + 0.006, 1.290, -0.56], [0.96, -0.6, 0.08, 0.08], [1.290, -0.590, 0.06, 0.06]), + ([1.04, -0.56, 1.29 - 0.09 - 0.006, -0.56], [0.96, -0.6, 0.08, 0.08], [1.290, -0.590, 0.06, 0.06]), + ([1.04, -0.56, 1.29 + 0.06 + 0.09 + 0.006, -0.56], [0.96, -0.6, 0.08, 0.08], [1.290, -0.590, 0.06, 0.06]), + ([1.04, -0.56, 1.29, -0.59 - 0.09 - 0.006], [0.96, -0.6, 0.08, 0.08], [1.290, -0.590, 0.06, 0.06]), + ([1.04, -0.56, 1.29, -0.59 + 0.06 + 0.09 + 0.006], [0.96, -0.6, 0.08, 0.08], [1.290, -0.590, 0.06, 0.06]), + ]) + def test_create_connector_with_line_not_touching(self, line, start, end): + # GIVEN a visio connector shape + shape = Mock(ID=1001, begin_x=line[0], begin_y=line[1], end_x=line[2], end_y=line[3]) + # AND the start component + start_component = mock_component(444, start) + # AND the end component + end_component = mock_component(555, end) + + # WHEN the connector is created + strategy = CreateConnectorByLineCoordinates() + diagram_connector = strategy.create_connector(shape, components=[start_component, end_component]) + + # THEN no diagram is returned + assert not diagram_connector + + def test_create_connector_without_components(self): + # GIVEN a visio connector shape + shape = Mock(ID=1001, begin_x=0, begin_y=0, end_x=0, end_y=0) + + # WHEN the connector is created + strategy = CreateConnectorByLineCoordinates() + diagram_connector = strategy.create_connector(shape) + + # THEN no diagram is returned + assert not diagram_connector + + def test_create_connector_invalid_line(self): + # GIVEN a visio connector shape + shape = Mock(ID=None, begin_x=None, begin_y=None, end_x=None, end_y=None) + + # WHEN the connector is created + strategy = CreateConnectorByLineCoordinates() + diagram_connector = strategy.create_connector(shape) + + # THEN no diagram is returned + assert not diagram_connector diff --git a/slp_visio/tests/unit/load/strategies/dataflow/test_create_connector_strategy.py b/slp_visio/tests/unit/load/strategies/dataflow/test_create_connector_strategy.py index ba37cb5e..fa1e5544 100644 --- a/slp_visio/tests/unit/load/strategies/dataflow/test_create_connector_strategy.py +++ b/slp_visio/tests/unit/load/strategies/dataflow/test_create_connector_strategy.py @@ -1,5 +1,7 @@ from slp_visio.slp_visio.load.strategies.connector.create_connector_strategy import CreateConnectorStrategy from slp_visio.slp_visio.load.strategies.connector.impl.create_connector_by_connects import CreateConnectorByConnects +from slp_visio.slp_visio.load.strategies.connector.impl.create_connector_by_line_coordinates import \ + CreateConnectorByLineCoordinates class TestCreateConnectorStrategy: @@ -9,7 +11,8 @@ def test_get_strategies(self): strategies = CreateConnectorStrategy.get_strategies() # THEN we have the expected number of strategies - assert strategies.__len__() == 1 + assert strategies.__len__() == 2 # AND we have the expected implementations assert strategies[0].__class__ == CreateConnectorByConnects + assert strategies[1].__class__ == CreateConnectorByLineCoordinates diff --git a/slp_visio/tests/unit/lucid/load/objects/__init__.py b/slp_visio/tests/unit/lucid/load/objects/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/slp_visio/tests/unit/lucid/load/objects/test_lucid_diagram_factories.py b/slp_visio/tests/unit/lucid/load/objects/test_lucid_diagram_factories.py new file mode 100644 index 00000000..61802917 --- /dev/null +++ b/slp_visio/tests/unit/lucid/load/objects/test_lucid_diagram_factories.py @@ -0,0 +1,120 @@ +from unittest.mock import Mock + +import pytest +from pytest import fixture, mark, param + +from slp_visio.slp_visio.load.strategies.connector.create_connector_strategy import CreateConnectorStrategy +from slp_visio.slp_visio.lucid.load.objects.lucid_diagram_factories import LucidConnectorFactory +from slp_visio.tests.unit.load.objects.test_visio_diagram_factories import mocked_strategy, mocked_connector, \ + mocked_strategies + + +@fixture +def strategies(): + return [] + + +@fixture(autouse=True) +def mock_get_strategies(mocker, strategies): + return mocker.patch( + 'slp_visio.slp_visio.load.strategies.connector.create_connector_strategy.CreateConnectorStrategy.get_strategies', + return_value=strategies) + + +class TestLucidConnectorFactory: + + def test_create_connector_when_strategy_returns_value(self, mock_get_strategies): + # GIVEN a visio connector shape + shape = Mock(ID=1001) + + # AND only one strategy that returns a connector + strategy = mocked_strategy(mocked_connector()) + mock_get_strategies.return_value = [strategy] + + # WHEN a connector is created + result = LucidConnectorFactory().create_connector(shape, []) + + # THEN the strategy implementations are the expected + assert CreateConnectorStrategy.get_strategies().__len__() == 1 + # AND the strategies method implementations are called once + strategy.create_connector.assert_called_once() + # AND the result is the expected + assert result.id == 1001 + assert result.to_id == 2 + assert result.from_id == 1 + + @mark.parametrize('strategies,_id,_from,_to,valid_strategy', [ + param(mocked_strategies( + [mocked_connector(c_id=1001, from_id=1, to_id=2), + mocked_connector(c_id=2002, from_id=2, to_id=1)]), + 1001, 1, 2, 0), + param(mocked_strategies( + [mocked_connector(c_id=1001, from_id=1, to_id=2), + None]), + 1001, 1, 2, 0), + param(mocked_strategies( + [None, + mocked_connector(c_id=2002, from_id=9, to_id=21)]), + 2002, 9, 21, 1), + param(mocked_strategies( + [None, + None, + mocked_connector(c_id=3003, from_id=5, to_id=7)]), + 3003, 5, 7, 2) + ]) + def test_create_connector_when_some_strategy_return_value(self, strategies, _id, _from, _to, valid_strategy: int): + # GIVEN a visio connector shape + shape = Mock(ID=1001) + + # WHEN a connector is created + result = LucidConnectorFactory().create_connector(shape, []) + + # THEN we call strategies until we find the first valid strategy + for i in range(0, valid_strategy): + strategies[i].create_connector.assert_called_once() + for i in range(valid_strategy + 1, len(strategies)): + strategies[i].create_connector.assert_not_called() + # AND the result is the returned by the first strategy + assert result.id == _id + assert result.to_id == _to + assert result.from_id == _from + + @mark.parametrize('strategies', [ + param(mocked_strategies([Exception("Some Error")]), id='one error'), + param(mocked_strategies([Exception("Some Error"), Exception("Other Error")]), id='two errors'), + param(mocked_strategies([Exception("Some Error"), mocked_connector()]), id='first error, second valid'), + ]) + def test_create_connector_when_some_strategy_return_error(self, strategies): + # GIVEN a visio connector shape + shape = Mock(ID=1001) + + # WHEN a connector is created + with pytest.raises(Exception) as error: + LucidConnectorFactory().create_connector(shape, []) + + # THEN the strategy implementations are the expected + assert CreateConnectorStrategy.get_strategies().__len__() == len(strategies) + + # AND the first strategy is called + strategies[0].create_connector.assert_called_once() + + # AND the error is propagated + assert error.value.args[0] == 'Some Error' + + @mark.parametrize('strategies', [ + param(mocked_strategies([None]), id='one strategy'), + param(mocked_strategies([None, None]), id='two strategies') + ]) + def test_create_connector_when_strategy_does_not_return_value(self, strategies): + # GIVEN a visio connector shape + shape = Mock(ID=1001) + + # WHEN a connector is created + result = LucidConnectorFactory().create_connector(shape, []) + + # THEN the strategy implementations are the expected + assert CreateConnectorStrategy.get_strategies().__len__() == len(strategies) + # AND the strategies method implementations are called once + strategies[0].create_connector.assert_called_once() + # AND no result is returned + assert not result From 384ae92cb95bfa5d0e431c5577da7d1288d72a84 Mon Sep 17 00:00:00 2001 From: Santi Manero Date: Thu, 27 Apr 2023 08:00:12 +0200 Subject: [PATCH 07/18] [OPT-780] test improvements, connector strategy refactor --- .../connector/create_connector_strategy.py | 2 +- .../impl/create_connector_by_connects.py | 2 +- .../create_connector_by_line_coordinates.py | 3 +- ...st_create_connector_by_line_coordinates.py | 64 ++++++++++++------- 4 files changed, 45 insertions(+), 26 deletions(-) diff --git a/slp_visio/slp_visio/load/strategies/connector/create_connector_strategy.py b/slp_visio/slp_visio/load/strategies/connector/create_connector_strategy.py index 8b0f1948..d2249739 100644 --- a/slp_visio/slp_visio/load/strategies/connector/create_connector_strategy.py +++ b/slp_visio/slp_visio/load/strategies/connector/create_connector_strategy.py @@ -19,6 +19,6 @@ def __subclasshook__(cls, subclass): or NotImplemented) @abc.abstractmethod - def create_connector(self, shape: Shape, **kwargs) -> Optional[DiagramConnector]: + def create_connector(self, shape: Shape, components=None) -> Optional[DiagramConnector]: """creates the OTM Dataflow from the vsdx shape""" raise NotImplementedError diff --git a/slp_visio/slp_visio/load/strategies/connector/impl/create_connector_by_connects.py b/slp_visio/slp_visio/load/strategies/connector/impl/create_connector_by_connects.py index 00153822..bc4cb83e 100644 --- a/slp_visio/slp_visio/load/strategies/connector/impl/create_connector_by_connects.py +++ b/slp_visio/slp_visio/load/strategies/connector/impl/create_connector_by_connects.py @@ -11,7 +11,7 @@ class CreateConnectorByConnects(CreateConnectorStrategy): Strategy to create a connector from the shape connects """ - def create_connector(self, shape: Shape, **kwargs) -> Optional[DiagramConnector]: + def create_connector(self, shape: Shape, components=None) -> Optional[DiagramConnector]: connected_shapes = shape.connects if not self.are_two_different_shapes(connected_shapes): return None diff --git a/slp_visio/slp_visio/load/strategies/connector/impl/create_connector_by_line_coordinates.py b/slp_visio/slp_visio/load/strategies/connector/impl/create_connector_by_line_coordinates.py index 7a63947a..1d74dbcd 100644 --- a/slp_visio/slp_visio/load/strategies/connector/impl/create_connector_by_line_coordinates.py +++ b/slp_visio/slp_visio/load/strategies/connector/impl/create_connector_by_line_coordinates.py @@ -21,7 +21,7 @@ def __init__(self): self.tolerance = 0.09 self.representer: SimpleComponentRepresenter() = SimpleComponentRepresenter() - def create_connector(self, shape: Shape, **kwargs) -> Optional[DiagramConnector]: + def create_connector(self, shape: Shape, components=None) -> Optional[DiagramConnector]: if not shape.begin_x or not shape.begin_y or not shape.end_x or not shape.end_y: return None begin_line = Point(shape.begin_x, shape.begin_y) @@ -29,7 +29,6 @@ def create_connector(self, shape: Shape, **kwargs) -> Optional[DiagramConnector] if not begin_line or not end_line: return None - components = kwargs.get('components') if not components: return None origin = self.__match_component(begin_line, components) diff --git a/slp_visio/tests/unit/load/strategies/dataflow/impl/test_create_connector_by_line_coordinates.py b/slp_visio/tests/unit/load/strategies/dataflow/impl/test_create_connector_by_line_coordinates.py index 081837fb..bd3cbe70 100644 --- a/slp_visio/tests/unit/load/strategies/dataflow/impl/test_create_connector_by_line_coordinates.py +++ b/slp_visio/tests/unit/load/strategies/dataflow/impl/test_create_connector_by_line_coordinates.py @@ -1,6 +1,6 @@ from unittest.mock import MagicMock, Mock -from pytest import mark +from pytest import mark, param from slp_visio.slp_visio.load.strategies.connector.impl.create_connector_by_line_coordinates import \ CreateConnectorByLineCoordinates @@ -15,19 +15,31 @@ def mock_component(_id, pos): class TestCreateConnectorByLineCoordinates: - @mark.parametrize('line,start,end', [ - (['1040', '-560', '1290', '-560'], ['960', '-600', '80', '80'], ['1290', '-590', '60', '60']), - ([1040, -560, 1290, -560], [960, -600, 80, 80], [1290, -590, 60, 60]), - (['1.04', '-0.56', '1.290', '-0.56'], ['0.96', '-0.6', '0.08', '0.08'], ['1.290', '-0.590', '0.06', '0.06']), - ([1.04, -0.56, 1.290, -0.56], [0.96, -0.6, 0.08, 0.08], [1.290, -0.590, 0.06, 0.06]), - ([1.04 + 0.09, -0.56, 1.290, -0.56], [0.96, -0.6, 0.08, 0.08], [1.290, -0.590, 0.06, 0.06]), - ([0.96 - 0.09, -0.56, 1.290, -0.56], [0.96, -0.6, 0.08, 0.08], [1.290, -0.590, 0.06, 0.06]), - ([1.04, -0.6 + 0.08 + 0.09, 1.290, -0.56], [0.96, -0.6, 0.08, 0.08], [1.290, -0.590, 0.06, 0.06]), - ([1.04, -0.56, 1.29 - 0.09, -0.56], [0.96, -0.6, 0.08, 0.08], [1.290, -0.590, 0.06, 0.06]), - ([1.04, -0.56, 1.29 + 0.06 + 0.09, -0.56], [0.96, -0.6, 0.08, 0.08], [1.290, -0.590, 0.06, 0.06]), - ([1.04, -0.56, 1.29, -0.59 - 0.09], [0.96, -0.6, 0.08, 0.08], [1.290, -0.590, 0.06, 0.06]), - ([1.04, -0.56, 1.29, -0.59 + 0.06 + 0.09], [0.96, -0.6, 0.08, 0.08], [1.290, -0.590, 0.06, 0.06]), + param(['1040', '-560', '1290', '-560'], ['960', '-600', '80', '80'], ['1290', '-590', '60', '60'], + id="perfect_match,strings,big_scale"), + param([1040, -560, 1290, -560], [960, -600, 80, 80], [1290, -590, 60, 60], + id="perfect_match,big_scale"), + param(['1.04', '-0.56', '1.29', '-0.56'], ['0.96', '-0.6', '0.08', '0.08'], ['1.29', '-0.59', '0.06', '0.06'], + id="perfect_match,strings"), + param([1.04, -0.56, 1.29, -0.56], [0.96, -0.6, 0.08, 0.08], [1.29, -0.59, 0.06, 0.06], + id="perfect_match"), + param([1.04 + 0.09, -0.56, 1.29, -0.56], [0.96, -0.6, 0.08, 0.08], [1.29, -0.59, 0.06, 0.06], + id="start connected by right tolerance, end connected"), + param([0.96 - 0.09, -0.56, 1.29, -0.56], [0.96, -0.6, 0.08, 0.08], [1.29, -0.59, 0.06, 0.06], + id="start connected by left tolerance, end connected"), + param([1.04, -0.6 - 0.09, 1.29, -0.56], [0.96, -0.6, 0.08, 0.08], [1.29, -0.59, 0.06, 0.06], + id="start connected by top tolerance, end connected"), + param([1.04, -0.6 + 0.08 + 0.09, 1.29, -0.56], [0.96, -0.6, 0.08, 0.08], [1.29, -0.59, 0.06, 0.06], + id="start connected by bottom tolerance, end connected"), + param([1.04, -0.56, 1.29 - 0.09, -0.56], [0.96, -0.6, 0.08, 0.08], [1.29, -0.59, 0.06, 0.06], + id="start connected, end connected by left tolerance"), + param([1.04, -0.56, 1.29 + 0.06 + 0.09, -0.56], [0.96, -0.6, 0.08, 0.08], [1.29, -0.59, 0.06, 0.06], + id="start connected, end connected by right tolerance"), + param([1.04, -0.56, 1.29, -0.59 - 0.09], [0.96, -0.6, 0.08, 0.08], [1.29, -0.59, 0.06, 0.06], + id="start connected, end connected by top tolerance"), + param([1.04, -0.56, 1.29, -0.59 + 0.06 + 0.09], [0.96, -0.6, 0.08, 0.08], [1.29, -0.59, 0.06, 0.06], + id="start connected, end connected by bottom tolerance"), ]) def test_create_connector_with_line_touching_component(self, line, start, end): # GIVEN a visio connector shape @@ -47,16 +59,24 @@ def test_create_connector_with_line_touching_component(self, line, start, end): assert diagram_connector.to_id == 333 assert not diagram_connector.bidirectional + @mark.parametrize('line,start,end', [ - ([1.04 + 0.09 + 0.006, -0.56, 1.290, -0.56], [0.96, -0.6, 0.08, 0.08], [1.290, -0.590, 0.06, 0.06]), - ([1.04 + 0.09 + 0.006, -0.56, 1.290, -0.56], [0.96, -0.6, 0.08, 0.08], [1.290, -0.590, 0.06, 0.06]), - ([1.04 + 0.09 + 0.006, -0.56, 1.290, -0.56], [0.96, -0.6, 0.08, 0.08], [1.290, -0.590, 0.06, 0.06]), - ([0.96 - 0.09 - 0.006, -0.56, 1.290, -0.56], [0.96, -0.6, 0.08, 0.08], [1.290, -0.590, 0.06, 0.06]), - ([1.04, -0.6 + 0.08 + 0.09 + 0.006, 1.290, -0.56], [0.96, -0.6, 0.08, 0.08], [1.290, -0.590, 0.06, 0.06]), - ([1.04, -0.56, 1.29 - 0.09 - 0.006, -0.56], [0.96, -0.6, 0.08, 0.08], [1.290, -0.590, 0.06, 0.06]), - ([1.04, -0.56, 1.29 + 0.06 + 0.09 + 0.006, -0.56], [0.96, -0.6, 0.08, 0.08], [1.290, -0.590, 0.06, 0.06]), - ([1.04, -0.56, 1.29, -0.59 - 0.09 - 0.006], [0.96, -0.6, 0.08, 0.08], [1.290, -0.590, 0.06, 0.06]), - ([1.04, -0.56, 1.29, -0.59 + 0.06 + 0.09 + 0.006], [0.96, -0.6, 0.08, 0.08], [1.290, -0.590, 0.06, 0.06]), + param([1.04 + 0.09 + 0.006, -0.56, 1.29, -0.56], [0.96, -0.6, 0.08, 0.08], [1.29, -0.59, 0.06, 0.06], + id="start not connected by right intolerance, end connected"), + param([0.96 - 0.09 - 0.006, -0.56, 1.29, -0.56], [0.96, -0.6, 0.08, 0.08], [1.29, -0.59, 0.06, 0.06], + id="start not connected by left intolerance, end connected"), + param([1.04, -0.6 + 0.08 + 0.09 + 0.006, 1.29, -0.56], [0.96, -0.6, 0.08, 0.08], [1.29, -0.59, 0.06, 0.06], + id="start not connected by bottom intolerance, end connected"), + param([1.04, -0.6 - 0.09 - 0.006, 1.29, -0.56], [0.96, -0.6, 0.08, 0.08], [1.29, -0.59, 0.06, 0.06], + id="start not connected by top intolerance, end connected", ), + param([1.04, -0.56, 1.29 - 0.09 - 0.006, -0.56], [0.96, -0.6, 0.08, 0.08], [1.29, -0.59, 0.06, 0.06], + id="start connected, end not connected by left intolerance"), + param([1.04, -0.56, 1.29 + 0.06 + 0.09 + 0.006, -0.56], [0.96, -0.6, 0.08, 0.08], [1.29, -0.59, 0.06, 0.06], + id="start connected, end not connected by right intolerance"), + param([1.04, -0.56, 1.29, -0.59 - 0.09 - 0.006], [0.96, -0.6, 0.08, 0.08], [1.29, -0.59, 0.06, 0.06], + id="start connected, end not connected by top intolerance"), + param([1.04, -0.56, 1.29, -0.59 + 0.06 + 0.09 + 0.006], [0.96, -0.6, 0.08, 0.08], [1.29, -0.59, 0.06, 0.06], + id="start connected, end not connected by bottom intolerance"), ]) def test_create_connector_with_line_not_touching(self, line, start, end): # GIVEN a visio connector shape From 07448d3c44d149b36d247378289acbcfc0805c51 Mon Sep 17 00:00:00 2001 From: Daniel Font Date: Mon, 8 May 2023 08:27:37 +0200 Subject: [PATCH 08/18] [OPT-761] Fix OTM schema inconsistencies. --- otm/resources/schemas/otm_schema.json | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/otm/resources/schemas/otm_schema.json b/otm/resources/schemas/otm_schema.json index d11737cc..bb9ceb43 100644 --- a/otm/resources/schemas/otm_schema.json +++ b/otm/resources/schemas/otm_schema.json @@ -7,7 +7,7 @@ "required": ["project", "otmVersion"], "properties": { "project": { - "type": ["object", "null"], + "type": "object", "required": ["name", "id"], "properties": { "name": {"type": "string"}, @@ -18,7 +18,7 @@ "tags": { "type": ["array", "null"], "items": { - "type": ["string", "null"] + "type": "string" } }, "attributes": {"type": ["object", "null"]} @@ -44,14 +44,14 @@ } }, "assets": { - "type": "array", + "type": ["array", "null"], "required": ["name", "id", "risk"], "properties": { "name": {"type": "string"}, "id": {"type": "string"}, "description": {"type": ["string", "null"]}, "risk": { - "type": ["object", "null"], + "type": "object", "required": ["confidentiality", "integrity", "availability"], "properties": { "confidentiality": {"type": "number"}, @@ -82,7 +82,7 @@ }, "parent": {"$ref": "#/definitions/parent"}, "representations": { - "type": "array", + "type": ["array", "null"], "items": {"$ref": "#/definitions/representationElement"} }, "attributes": {"type": ["object", "null"]} @@ -90,7 +90,7 @@ } }, "components": { - "type": "array", + "type": ["array", "null"], "items": { "type": "object", "required": ["id", "name", "type", "parent"], @@ -144,7 +144,7 @@ "id": {"type": "string"}, "name": {"type": "string"}, "description": {"type": ["string", "null"]}, - "bidirectional": {"type": "boolean"}, + "bidirectional": {"type": ["boolean", "null"]}, "source": {"type": "string"}, "destination": {"type": "string"}, "assets": { @@ -166,7 +166,7 @@ } }, "threats": { - "type": "array", + "type": ["array", "null"], "items": { "type": "object", "required": ["id", "name", "risk"], @@ -186,8 +186,8 @@ "type": "object", "required": ["likelihood", "impact"], "properties": { - "likelihood": {"type": "number"}, - "likelihoodComment": {"type": "string"}, + "likelihood": {"type": ["number", "null"]}, + "likelihoodComment": {"type": ["string", "null"]}, "impact": {"type": "number"}, "impactComment": {"type": "string"} } @@ -203,7 +203,7 @@ } }, "mitigations": { - "type": "array", + "type": ["array", "null"], "items": { "type": "object", "required": ["id", "name", "riskReduction"], @@ -219,7 +219,7 @@ }, "definitions": { "size": { - "type": "object", + "type": ["object", "null"], "required": ["width", "height"], "properties": { "width": {"type": "number"}, @@ -238,7 +238,7 @@ } }, "position": { - "type": "object", + "type": ["object", "null"], "required": ["x", "y"], "properties": { "x": {"type": "number"}, From 2ad935fe18bf7bf82b3220e740a1cd8464e9cbd7 Mon Sep 17 00:00:00 2001 From: Daniel Font Date: Mon, 8 May 2023 08:29:24 +0200 Subject: [PATCH 09/18] [OPT-761] Sync with OTM standard definition. --- slp_base/slp_base/otm_validator.py | 68 ++++++++++++++++-------------- 1 file changed, 36 insertions(+), 32 deletions(-) diff --git a/slp_base/slp_base/otm_validator.py b/slp_base/slp_base/otm_validator.py index fd4a1afc..8ea7a17d 100644 --- a/slp_base/slp_base/otm_validator.py +++ b/slp_base/slp_base/otm_validator.py @@ -46,54 +46,58 @@ def __check_otm_ids(self, otm): wrong_dataflow_from_ids = set() wrong_dataflow_to_ids = set() - for trustzone in otm['trustZones']: - if trustzone['id'] in all_valid_ids: - repeated_ids.add(trustzone['id']) - elif trustzone['id'] not in all_valid_ids: - all_valid_ids.add(trustzone['id']) + if 'trustZones' in otm: + for trustzone in otm['trustZones']: + if trustzone['id'] in all_valid_ids: + repeated_ids.add(trustzone['id']) + elif trustzone['id'] not in all_valid_ids: + all_valid_ids.add(trustzone['id']) - for component in otm['components']: - if component['id'] in all_valid_ids: - repeated_ids.add(component['id']) - elif component['id'] not in all_valid_ids: - all_valid_ids.add(component['id']) + if 'components' in otm: + for component in otm['components']: + if component['id'] in all_valid_ids: + repeated_ids.add(component['id']) + elif component['id'] not in all_valid_ids: + all_valid_ids.add(component['id']) - component_parent_id = self.__get_parent_id(component) - parent_ids.add(component_parent_id) + component_parent_id = self.__get_parent_id(component) + parent_ids.add(component_parent_id) - for parent_id in parent_ids: - if parent_id not in all_valid_ids: - wrong_component_parent_ids.add(parent_id) + for parent_id in parent_ids: + if parent_id not in all_valid_ids: + wrong_component_parent_ids.add(parent_id) - for dataflow in otm['dataflows']: - if dataflow['id'] in all_valid_ids: - repeated_ids.add(dataflow['id']) - elif dataflow['id'] not in all_valid_ids: - all_valid_ids.add(dataflow['id']) + if 'dataflows' in otm: + for dataflow in otm['dataflows']: + if dataflow['id'] in all_valid_ids: + repeated_ids.add(dataflow['id']) + elif dataflow['id'] not in all_valid_ids: + all_valid_ids.add(dataflow['id']) - if dataflow['source'] not in all_valid_ids: - wrong_dataflow_from_ids.add(dataflow['source']) + if dataflow['source'] not in all_valid_ids: + wrong_dataflow_from_ids.add(dataflow['source']) - if dataflow['destination'] not in all_valid_ids: - wrong_dataflow_to_ids.add(dataflow['destination']) + if dataflow['destination'] not in all_valid_ids: + wrong_dataflow_to_ids.add(dataflow['destination']) - if wrong_component_parent_ids: - logger.error(f"Component parent identifiers inconsistent: {wrong_component_parent_ids}") + if wrong_component_parent_ids: + logger.error(f"Component parent identifiers inconsistent: {wrong_component_parent_ids}") - if wrong_dataflow_from_ids: - logger.error(f"Dataflow 'source' identifiers inconsistent: {wrong_dataflow_from_ids}") + if wrong_dataflow_from_ids: + logger.error(f"Dataflow 'source' identifiers inconsistent: {wrong_dataflow_from_ids}") - if wrong_dataflow_to_ids: - logger.error(f"Dataflow 'destination' identifiers inconsistent: {wrong_dataflow_to_ids}") + if wrong_dataflow_to_ids: + logger.error(f"Dataflow 'destination' identifiers inconsistent: {wrong_dataflow_to_ids}") - if repeated_ids: - logger.error(f"Repeated identifiers inconsistent: {repeated_ids}") + if repeated_ids: + logger.error(f"Repeated identifiers inconsistent: {repeated_ids}") return (not wrong_component_parent_ids and not wrong_dataflow_from_ids and not wrong_dataflow_to_ids and not repeated_ids) + @staticmethod def __get_parent_id(trustzone: dict): parent = ParentType.TRUST_ZONE if ParentType.TRUST_ZONE in trustzone['parent'] else ParentType.COMPONENT From 2239c5ebb3f9d618887f2f644b3bf1df5fe4f3a3 Mon Sep 17 00:00:00 2001 From: Daniel Font Date: Wed, 10 May 2023 10:55:11 +0200 Subject: [PATCH 10/18] [OPT-850] Modify MAX SIZE parameter to 1Mb. --- slp_cft/slp_cft/validate/cft_validator.py | 2 +- slp_cft/tests/unit/validate/test_cft_validator.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/slp_cft/slp_cft/validate/cft_validator.py b/slp_cft/slp_cft/validate/cft_validator.py index 01a26a0f..19fc2658 100644 --- a/slp_cft/slp_cft/validate/cft_validator.py +++ b/slp_cft/slp_cft/validate/cft_validator.py @@ -8,7 +8,7 @@ logger = logging.getLogger(__name__) -MAX_SIZE = 20 * 1024 * 1024 +MAX_SIZE = 1 * 1024 * 1024 MIN_SIZE = 13 diff --git a/slp_cft/tests/unit/validate/test_cft_validator.py b/slp_cft/tests/unit/validate/test_cft_validator.py index c5f01a18..96c4a35f 100644 --- a/slp_cft/tests/unit/validate/test_cft_validator.py +++ b/slp_cft/tests/unit/validate/test_cft_validator.py @@ -5,8 +5,8 @@ from slp_cft.slp_cft.validate.cft_validator import CloudformationValidator VALID_MIME = 'text/plain' -MIN_SIZE = 20 -MAX_SIZE = 20 * 1024 * 1024 +MIN_SIZE = 13 +MAX_SIZE = 1 * 1024 * 1024 def create_cloudformation_file_data(size: int) -> bytes: From e260a5f5edfcd24caaf1dcc954e12c125074026b Mon Sep 17 00:00:00 2001 From: Daniel Font Date: Wed, 24 May 2023 12:39:20 +0200 Subject: [PATCH 11/18] [OPT-866] Add SonarCloud analysis on dev branch push events. --- .github/workflows/sonarcloud.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/sonarcloud.yml b/.github/workflows/sonarcloud.yml index 90c04984..5c2e8d9a 100644 --- a/.github/workflows/sonarcloud.yml +++ b/.github/workflows/sonarcloud.yml @@ -12,6 +12,8 @@ name: SonarCloud analysis on: pull_request: branches: [feature/*] + push: + branches: [dev] workflow_dispatch: permissions: From f415f89ca65f71218d9c4d3ebb1b4da3119236cb Mon Sep 17 00:00:00 2001 From: Daniel Font Date: Wed, 24 May 2023 13:07:38 +0200 Subject: [PATCH 12/18] [OPT-866] Add coverage dependency. --- setup.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/setup.py b/setup.py index af600010..cfefc33c 100644 --- a/setup.py +++ b/setup.py @@ -49,7 +49,8 @@ 'responses==0.23.1', 'deepdiff==6.3.0', 'httpx==0.23.3', - 'pytest-mock==3.10.0' + 'pytest-mock==3.10.0', + 'coverage==7.2.3' ] }, entry_points=''' From 671afffa7c15cb34b38b03dbfe1df45d7adc2106 Mon Sep 17 00:00:00 2001 From: Daniel Font Date: Wed, 24 May 2023 13:15:58 +0200 Subject: [PATCH 13/18] [OPT-866] Add SonarCloud analysis on dev branch pull request events. --- .github/workflows/sonarcloud.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/sonarcloud.yml b/.github/workflows/sonarcloud.yml index 5c2e8d9a..89dddc59 100644 --- a/.github/workflows/sonarcloud.yml +++ b/.github/workflows/sonarcloud.yml @@ -11,7 +11,7 @@ name: SonarCloud analysis on: pull_request: - branches: [feature/*] + branches: [dev,feature/*] push: branches: [dev] workflow_dispatch: From fe53900640b9eb11f8d7654d1ba121d741d0e2e8 Mon Sep 17 00:00:00 2001 From: Santi Manero Date: Thu, 25 May 2023 07:46:09 +0200 Subject: [PATCH 14/18] [OPT-781] Extracted strategy mapping components --- slp_visio/slp_visio/load/__init__.py | 1 + .../slp_visio/load/component_identifier.py | 14 +++++ .../load/objects/visio_diagram_factories.py | 17 +++--- .../component_identifier_strategy.py | 22 ++++++++ .../component/create_component_strategy.py | 26 +++++++++ .../component_identifier_by_shape_text.py | 17 ++++++ .../impl/create_component_by_shape_text.py | 42 ++++++++++++++ slp_visio/slp_visio/load/vsdx_parser.py | 8 +-- .../slp_visio/lucid/load/lucid_loader.py | 5 +- .../slp_visio/lucid/load/lucid_vsdx_parser.py | 3 +- .../load/objects/lucid_diagram_factories.py | 32 +---------- ...test_component_identifier_by_shape_text.py | 38 +++++++++++++ .../test_create_component_by_shape_text.py | 47 ++++++++++++++++ .../test_component_identifier_strategy.py | 16 ++++++ .../test_create_component_strategy.py | 15 +++++ .../unit/load/test_component_identifier.py | 56 +++++++++++++++++++ 16 files changed, 311 insertions(+), 48 deletions(-) create mode 100644 slp_visio/slp_visio/load/component_identifier.py create mode 100644 slp_visio/slp_visio/load/strategies/component/component_identifier_strategy.py create mode 100644 slp_visio/slp_visio/load/strategies/component/create_component_strategy.py create mode 100644 slp_visio/slp_visio/load/strategies/component/impl/component_identifier_by_shape_text.py create mode 100644 slp_visio/slp_visio/load/strategies/component/impl/create_component_by_shape_text.py create mode 100644 slp_visio/tests/unit/load/strategies/component/impl/test_component_identifier_by_shape_text.py create mode 100644 slp_visio/tests/unit/load/strategies/component/impl/test_create_component_by_shape_text.py create mode 100644 slp_visio/tests/unit/load/strategies/component/test_component_identifier_strategy.py create mode 100644 slp_visio/tests/unit/load/strategies/component/test_create_component_strategy.py create mode 100644 slp_visio/tests/unit/load/test_component_identifier.py diff --git a/slp_visio/slp_visio/load/__init__.py b/slp_visio/slp_visio/load/__init__.py index 630660db..e1845d11 100644 --- a/slp_visio/slp_visio/load/__init__.py +++ b/slp_visio/slp_visio/load/__init__.py @@ -1,2 +1,3 @@ from .strategies.connector.impl import connector_identifier_by_connects, create_connector_by_connects, \ create_connector_by_line_coordinates +from .strategies.component.impl import component_identifier_by_shape_text, create_component_by_shape_text diff --git a/slp_visio/slp_visio/load/component_identifier.py b/slp_visio/slp_visio/load/component_identifier.py new file mode 100644 index 00000000..d6ab6f2a --- /dev/null +++ b/slp_visio/slp_visio/load/component_identifier.py @@ -0,0 +1,14 @@ +from vsdx import Shape + +from slp_visio.slp_visio.load.strategies.component.component_identifier_strategy import ComponentIdentifierStrategy + + +class ComponentIdentifier: + + @staticmethod + def is_component(shape: Shape) -> bool: + for strategy in ComponentIdentifierStrategy.get_strategies(): + if strategy.is_component(shape): + return True + + return False diff --git a/slp_visio/slp_visio/load/objects/visio_diagram_factories.py b/slp_visio/slp_visio/load/objects/visio_diagram_factories.py index 0dd7dda3..d45cba2e 100644 --- a/slp_visio/slp_visio/load/objects/visio_diagram_factories.py +++ b/slp_visio/slp_visio/load/objects/visio_diagram_factories.py @@ -1,20 +1,19 @@ from typing import Optional from slp_visio.slp_visio.load.objects.diagram_objects import DiagramComponent, DiagramConnector +from slp_visio.slp_visio.load.strategies.component.create_component_strategy import CreateComponentStrategy from slp_visio.slp_visio.load.strategies.connector.create_connector_strategy import CreateConnectorStrategy -from slp_visio.slp_visio.util.visio import get_shape_text, get_master_shape_text, normalize_label, get_unique_id_text + class VisioComponentFactory: - def create_component(self, shape, origin, representer) -> DiagramComponent: - return DiagramComponent( - id=shape.ID, - name=normalize_label(get_shape_text(shape)), - type=normalize_label(get_master_shape_text(shape)), - origin=origin, - representation=representer.build_representation(shape), - unique_id=get_unique_id_text(shape)) + @staticmethod + def create_component(shape, origin, representer) -> DiagramComponent: + for strategy in CreateComponentStrategy.get_strategies(): + component = strategy.create_component(shape, origin=origin, representer=representer) + if component: + return component class VisioConnectorFactory: diff --git a/slp_visio/slp_visio/load/strategies/component/component_identifier_strategy.py b/slp_visio/slp_visio/load/strategies/component/component_identifier_strategy.py new file mode 100644 index 00000000..596b55bc --- /dev/null +++ b/slp_visio/slp_visio/load/strategies/component/component_identifier_strategy.py @@ -0,0 +1,22 @@ +import abc + +from vsdx import Shape + +from slp_visio.slp_visio.load.strategies.strategy import Strategy + + +class ComponentIdentifierStrategy(Strategy): + """ + Formal Interface to check if a shape is a component + """ + + @classmethod + def __subclasshook__(cls, subclass): + return ( + hasattr(subclass, 'is_component') and callable(subclass.process) + or NotImplemented) + + @abc.abstractmethod + def is_component(self, shape: Shape) -> bool: + """return True if the Shape is a component""" + raise NotImplementedError diff --git a/slp_visio/slp_visio/load/strategies/component/create_component_strategy.py b/slp_visio/slp_visio/load/strategies/component/create_component_strategy.py new file mode 100644 index 00000000..6245389a --- /dev/null +++ b/slp_visio/slp_visio/load/strategies/component/create_component_strategy.py @@ -0,0 +1,26 @@ +import abc +from typing import Optional + +from vsdx import Shape + +from slp_visio.slp_visio.load.objects.diagram_objects import DiagramComponent +from slp_visio.slp_visio.load.representation.visio_shape_representer import VisioShapeRepresenter +from slp_visio.slp_visio.load.strategies.strategy import Strategy + + +class CreateComponentStrategy(Strategy): + """ + Formal Interface to create an OTM Component from a vsdx shape + """ + + @classmethod + def __subclasshook__(cls, subclass): + return ( + hasattr(subclass, 'create_component') and callable(subclass.process) + or NotImplemented) + + @abc.abstractmethod + def create_component(self, shape: Shape, origin=None, representer: VisioShapeRepresenter = None) \ + -> Optional[DiagramComponent]: + """creates the OTM Component from the vsdx shape""" + raise NotImplementedError diff --git a/slp_visio/slp_visio/load/strategies/component/impl/component_identifier_by_shape_text.py b/slp_visio/slp_visio/load/strategies/component/impl/component_identifier_by_shape_text.py new file mode 100644 index 00000000..a071245b --- /dev/null +++ b/slp_visio/slp_visio/load/strategies/component/impl/component_identifier_by_shape_text.py @@ -0,0 +1,17 @@ +from vsdx import Shape + +from slp_visio.slp_visio.load.connector_identifier import ConnectorIdentifier +from slp_visio.slp_visio.load.strategies.component.component_identifier_strategy import ComponentIdentifierStrategy +from slp_visio.slp_visio.util.visio import get_shape_text + + +class ComponentIdentifierByShapeText(ComponentIdentifierStrategy): + """ + Strategy to know if a shape is a component + The shape must have the text property and must not be a connector + """ + + def is_component(self, shape: Shape) -> bool: + text = get_shape_text(shape) + is_connector = ConnectorIdentifier.is_connector(shape) + return text and not is_connector diff --git a/slp_visio/slp_visio/load/strategies/component/impl/create_component_by_shape_text.py b/slp_visio/slp_visio/load/strategies/component/impl/create_component_by_shape_text.py new file mode 100644 index 00000000..d6544222 --- /dev/null +++ b/slp_visio/slp_visio/load/strategies/component/impl/create_component_by_shape_text.py @@ -0,0 +1,42 @@ +from typing import Optional + +from vsdx import Shape + +from slp_visio.slp_visio.load.objects.diagram_objects import DiagramComponent +from slp_visio.slp_visio.load.representation.visio_shape_representer import VisioShapeRepresenter +from slp_visio.slp_visio.load.strategies.component.create_component_strategy import CreateComponentStrategy +from slp_visio.slp_visio.util.visio import get_shape_text, get_master_shape_text, normalize_label, get_unique_id_text + +LUCID_COMPONENT_PREFIX = 'com.lucidchart' + + +class CreateComponentByShapeText(CreateComponentStrategy): + """ + Strategy to create a component from the shape text + """ + + def create_component(self, shape: Shape, origin=None, representer: VisioShapeRepresenter = None) \ + -> Optional[DiagramComponent]: + name = get_shape_text(shape) + if name: + return DiagramComponent( + id=shape.ID, + name=normalize_label(name), + type=normalize_label(self.get_component_type(shape)), + origin=origin, + representation=representer.build_representation(shape), + unique_id=get_unique_id_text(shape)) + + def get_component_type(self, shape): + if self.is_lucid(shape): + return self.get_lucid_component_type(shape) + else: + return get_master_shape_text(shape) + + @staticmethod + def is_lucid(shape: Shape): + return shape.shape_name and shape.shape_name.startswith(LUCID_COMPONENT_PREFIX) + + @staticmethod + def get_lucid_component_type(shape: Shape): + return shape.shape_name.replace(f'{LUCID_COMPONENT_PREFIX}.', '').replace(f'.{shape.ID}', '') diff --git a/slp_visio/slp_visio/load/vsdx_parser.py b/slp_visio/slp_visio/load/vsdx_parser.py index 9320f8d9..f30d14aa 100644 --- a/slp_visio/slp_visio/load/vsdx_parser.py +++ b/slp_visio/slp_visio/load/vsdx_parser.py @@ -1,12 +1,13 @@ from vsdx import Shape, VisioFile from slp_base import DiagramType +from slp_visio.slp_visio.load.component_identifier import ComponentIdentifier from slp_visio.slp_visio.load.connector_identifier import ConnectorIdentifier from slp_visio.slp_visio.load.objects.diagram_objects import Diagram, DiagramComponentOrigin, DiagramLimits from slp_visio.slp_visio.load.parent_calculator import ParentCalculator from slp_visio.slp_visio.load.representation.simple_component_representer import SimpleComponentRepresenter from slp_visio.slp_visio.load.representation.zone_component_representer import ZoneComponentRepresenter -from slp_visio.slp_visio.util.visio import get_limits, get_shape_text +from slp_visio.slp_visio.util.visio import get_limits DIAGRAM_LIMITS_PADDING = 2 DEFAULT_DIAGRAM_LIMITS = DiagramLimits(((1000, 1000), (1000, 1000))) @@ -46,9 +47,6 @@ def parse(self, visio_diagram_filename) -> Diagram: def _is_boundary(shape: Shape) -> bool: return shape.shape_name is not None and 'Curved panel' in shape.shape_name - def _is_component(self, shape: Shape) -> bool: - return get_shape_text(shape) and not ConnectorIdentifier.is_connector(shape) - def __calculate_diagram_limits(self) -> DiagramLimits: floor_coordinates = [None, None] top_coordinates = [0, 0] @@ -76,7 +74,7 @@ def _load_page_elements(self): self._add_connector(shape) elif self._is_boundary(shape): self._add_boundary_component(shape) - elif self._is_component(shape): + elif ComponentIdentifier.is_component(shape): self._add_simple_component(shape) def _add_simple_component(self, component_shape: Shape): diff --git a/slp_visio/slp_visio/lucid/load/lucid_loader.py b/slp_visio/slp_visio/lucid/load/lucid_loader.py index e5369cb1..00b49f7d 100644 --- a/slp_visio/slp_visio/lucid/load/lucid_loader.py +++ b/slp_visio/slp_visio/lucid/load/lucid_loader.py @@ -1,7 +1,8 @@ import logging +from slp_visio.slp_visio.load.objects.visio_diagram_factories import VisioComponentFactory from slp_visio.slp_visio.load.visio_loader import VisioLoader -from slp_visio.slp_visio.lucid.load.objects.lucid_diagram_factories import LucidComponentFactory, LucidConnectorFactory +from slp_visio.slp_visio.lucid.load.objects.lucid_diagram_factories import LucidConnectorFactory from slp_visio.slp_visio.lucid.load.lucid_vsdx_parser import LucidVsdxParser logger = logging.getLogger(__name__) @@ -11,4 +12,4 @@ class LucidLoader(VisioLoader): def __init__(self, source): super().__init__(source) - self.parser = LucidVsdxParser(LucidComponentFactory(), LucidConnectorFactory()) + self.parser = LucidVsdxParser(VisioComponentFactory(), LucidConnectorFactory()) diff --git a/slp_visio/slp_visio/lucid/load/lucid_vsdx_parser.py b/slp_visio/slp_visio/lucid/load/lucid_vsdx_parser.py index d0afb563..515d9ad4 100644 --- a/slp_visio/slp_visio/lucid/load/lucid_vsdx_parser.py +++ b/slp_visio/slp_visio/lucid/load/lucid_vsdx_parser.py @@ -1,5 +1,6 @@ from vsdx import Shape +from slp_visio.slp_visio.load.component_identifier import ComponentIdentifier from slp_visio.slp_visio.load.parent_calculator import ParentCalculator from slp_visio.slp_visio.load.vsdx_parser import VsdxParser @@ -7,7 +8,7 @@ class LucidVsdxParser(VsdxParser): def _add_connector(self, connector_shape: Shape): - shape_components = [c for c in self.page.child_shapes if self._is_component(c) and not self._is_boundary(c)] + shape_components = [c for c in self.page.child_shapes if ComponentIdentifier.is_component(c) and not self._is_boundary(c)] visio_connector = self.connector_factory.create_connector(connector_shape, shape_components) if visio_connector: diff --git a/slp_visio/slp_visio/lucid/load/objects/lucid_diagram_factories.py b/slp_visio/slp_visio/lucid/load/objects/lucid_diagram_factories.py index 5c7e1b88..997fd36c 100644 --- a/slp_visio/slp_visio/lucid/load/objects/lucid_diagram_factories.py +++ b/slp_visio/slp_visio/lucid/load/objects/lucid_diagram_factories.py @@ -2,38 +2,8 @@ from vsdx import Shape -from slp_visio.slp_visio.load.objects.diagram_objects import DiagramComponent, DiagramConnector +from slp_visio.slp_visio.load.objects.diagram_objects import DiagramConnector from slp_visio.slp_visio.load.strategies.connector.create_connector_strategy import CreateConnectorStrategy -from slp_visio.slp_visio.util.visio import get_shape_text, get_master_shape_text - -LUCID_COMPONENT_PREFIX = 'com.lucidchart' - - -def is_lucid(shape: Shape): - return shape.shape_name and shape.shape_name.startswith(LUCID_COMPONENT_PREFIX) - - -def get_lucid_component_type(shape: Shape): - return shape.shape_name.replace(f'{LUCID_COMPONENT_PREFIX}.', '').replace(f'.{shape.ID}', '') - - -def get_component_type(shape): - if is_lucid(shape): - return get_lucid_component_type(shape) - else: - return get_master_shape_text(shape) - - -class LucidComponentFactory: - - @staticmethod - def create_component(shape, origin, representer) -> DiagramComponent: - return DiagramComponent( - id=shape.ID, - name=get_shape_text(shape), - type=get_component_type(shape), - origin=origin, - representation=representer.build_representation(shape)) class LucidConnectorFactory: diff --git a/slp_visio/tests/unit/load/strategies/component/impl/test_component_identifier_by_shape_text.py b/slp_visio/tests/unit/load/strategies/component/impl/test_component_identifier_by_shape_text.py new file mode 100644 index 00000000..78a7e966 --- /dev/null +++ b/slp_visio/tests/unit/load/strategies/component/impl/test_component_identifier_by_shape_text.py @@ -0,0 +1,38 @@ +from unittest.mock import MagicMock + +import pytest + +from slp_visio.slp_visio.load.strategies.component.impl.component_identifier_by_shape_text import \ + ComponentIdentifierByShapeText + + +class TestComponentIdentifierByShapeText: + + @pytest.mark.parametrize('shape_text', { + 'My EC2', + 'EC2' + }) + def test_validate_component_ok(self, shape_text): + # GIVEN a visio component shape + shape = MagicMock(ID=1001, text=shape_text, shape_name=None) + # WHEN the component is validated + strategy = ComponentIdentifierByShapeText() + result = strategy.is_component(shape) + + # THEN is a valid component + assert result + + @pytest.mark.parametrize('shape_text', { + ' ', + ' ' + }) + def test_validate_with_connects_wrong_component_shape_name(self, shape_text): + # GIVEN a visio component shape + shape = MagicMock(ID=1001, text=shape_text, shape_name=None) + + # WHEN the component is validated + strategy = ComponentIdentifierByShapeText() + result = strategy.is_component(shape) + + # THEN is not a valid component + assert not result diff --git a/slp_visio/tests/unit/load/strategies/component/impl/test_create_component_by_shape_text.py b/slp_visio/tests/unit/load/strategies/component/impl/test_create_component_by_shape_text.py new file mode 100644 index 00000000..cda532bf --- /dev/null +++ b/slp_visio/tests/unit/load/strategies/component/impl/test_create_component_by_shape_text.py @@ -0,0 +1,47 @@ +from unittest.mock import MagicMock + +import pytest + +from slp_visio.slp_visio.load.representation.simple_component_representer import SimpleComponentRepresenter +from slp_visio.slp_visio.load.strategies.component.impl.create_component_by_shape_text import CreateComponentByShapeText + + +class TestCreateComponentByShapeText: + + @pytest.mark.parametrize('shape_text,master_shape_text', { + ('My EC2', 'EC2'), + ('Another EC2', 'EC2'), + ('EC2', 'EC2'), + (None, 'EC2') + }) + def test_create_component_ok(self, shape_text, master_shape_text): + # GIVEN a visio component shape + shape = MagicMock(ID=1001, text=shape_text, shape_name=None, master_shape=MagicMock(text=master_shape_text), + master_page=MagicMock(master_unique_id='777'), center_x_y=(0.5, 2.5), + cells={'Width': MagicMock(value=8), 'Height': MagicMock(value=12)}) + + # WHEN the component is created + strategy = CreateComponentByShapeText() + diagram_component = strategy.create_component(shape, representer=SimpleComponentRepresenter()) + + # THEN the returned diagram component has the following properties + assert diagram_component.id == 1001 + assert diagram_component.name == shape_text or master_shape_text + assert diagram_component.type == 'EC2' + assert diagram_component.unique_id == '777' + + @pytest.mark.parametrize('shape_text,master_shape_text', { + (None, None) + }) + def test_create_component_without_text(self, shape_text, master_shape_text): + # GIVEN a visio component shape + shape = MagicMock(ID=1001, text=shape_text, shape_name=None, master_shape=MagicMock(text=master_shape_text), + master_page=MagicMock(master_unique_id='777'), center_x_y=(0.5, 2.5), + cells={'Width': MagicMock(value=8), 'Height': MagicMock(value=12)}) + + # WHEN the component is created + strategy = CreateComponentByShapeText() + diagram_component = strategy.create_component(shape, representer=SimpleComponentRepresenter()) + + # THEN no diagram is returned + assert not diagram_component diff --git a/slp_visio/tests/unit/load/strategies/component/test_component_identifier_strategy.py b/slp_visio/tests/unit/load/strategies/component/test_component_identifier_strategy.py new file mode 100644 index 00000000..f2a82117 --- /dev/null +++ b/slp_visio/tests/unit/load/strategies/component/test_component_identifier_strategy.py @@ -0,0 +1,16 @@ +from slp_visio.slp_visio.load.strategies.component.impl.component_identifier_by_shape_text import \ + ComponentIdentifierByShapeText +from slp_visio.slp_visio.load.strategies.component.component_identifier_strategy import ComponentIdentifierStrategy + + +class TestComponentIdentifierStrategy: + + def test_get_strategies(self): + # WHEN we get the strategies from CreateComponentStrategy + strategies = ComponentIdentifierStrategy.get_strategies() + + # THEN we have the expected number of strategies + assert strategies.__len__() == 1 + + # AND we have the expected implementations + assert strategies[0].__class__ == ComponentIdentifierByShapeText \ No newline at end of file diff --git a/slp_visio/tests/unit/load/strategies/component/test_create_component_strategy.py b/slp_visio/tests/unit/load/strategies/component/test_create_component_strategy.py new file mode 100644 index 00000000..c9e6a8e2 --- /dev/null +++ b/slp_visio/tests/unit/load/strategies/component/test_create_component_strategy.py @@ -0,0 +1,15 @@ +from slp_visio.slp_visio.load.strategies.component.create_component_strategy import CreateComponentStrategy +from slp_visio.slp_visio.load.strategies.component.impl.create_component_by_shape_text import CreateComponentByShapeText + + +class TestCreateComponentStrategy: + + def test_get_strategies(self): + # WHEN we get the strategies from CreateComponentStrategy + strategies = CreateComponentStrategy.get_strategies() + + # THEN we have the expected number of strategies + assert strategies.__len__() == 1 + + # AND we have the expected implementations + assert strategies[0].__class__ == CreateComponentByShapeText diff --git a/slp_visio/tests/unit/load/test_component_identifier.py b/slp_visio/tests/unit/load/test_component_identifier.py new file mode 100644 index 00000000..1f99cfd2 --- /dev/null +++ b/slp_visio/tests/unit/load/test_component_identifier.py @@ -0,0 +1,56 @@ +from typing import List, Union +from unittest.mock import MagicMock, Mock + +import pytest +from pytest import fixture + +from slp_visio.slp_visio.load.component_identifier import ComponentIdentifier + + +def mocked_strategy(result: Union[bool, Exception]): + strategy = Mock() + strategy.is_component = Mock(side_effect=[result]) + return strategy + + +def mocked_strategies(results: List[Union[bool, Exception]]): + return list(map(mocked_strategy, results)) + + +@fixture +def strategies(): + return [] + + +@fixture(autouse=True) +def mock_get_strategies(mocker, strategies): + return mocker.patch( + 'slp_visio.slp_visio.load.strategies.component.component_identifier_strategy.ComponentIdentifierStrategy.get_strategies', + return_value=strategies) + + +class TestComponentIdentifier: + + @pytest.mark.parametrize('strategies,valid_strategy,expected', [ + (mocked_strategies([True, True]), 0, True), + (mocked_strategies([True, False]), 0, True), + (mocked_strategies([False, True]), 1, True), + (mocked_strategies([False, False]), 2, False), + (mocked_strategies([True, False, False]), 0, True), + (mocked_strategies([False, False, True]), 2, True), + (mocked_strategies([False, False, False]), 3, False) + ]) + def test_create_component_when_strategy_returns_value(self, strategies, valid_strategy: int, expected): + # GIVEN a visio component shape + shape = MagicMock(ID=1001) + + # WHEN we check if the shape is a component + result = ComponentIdentifier.is_component(shape) + + # THEN we call strategies until we find the first valid strategy + for i in range(0, valid_strategy): + strategies[i].is_component.assert_called_once() + for i in range(valid_strategy + 1, len(strategies)): + strategies[i].create_component.assert_not_called() + # AND the result is true if any strategy returns true + assert result == expected From e468a4355d301f4560a6aa1e1ed44dad03a79d84 Mon Sep 17 00:00:00 2001 From: Santi Manero Date: Thu, 25 May 2023 08:31:42 +0200 Subject: [PATCH 15/18] [OPT-870] Updated dependencies --- setup.py | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/setup.py b/setup.py index cfefc33c..2dc1a46c 100644 --- a/setup.py +++ b/setup.py @@ -19,18 +19,18 @@ 'jsonschema==4.17.3', 'deepmerge==1.1.0', 'jmespath==1.0.1', - 'python-hcl2==4.3.0', - 'requests==2.28.2', - 'fastapi==0.95.1', - 'python-multipart==0.0.5', + 'python-hcl2==4.3.2', + 'requests==2.31.0', + 'fastapi==0.95.2', + 'python-multipart==0.0.6', 'click==8.1.3', - 'uvicorn==0.21.1', + 'uvicorn==0.22.0', 'shapely==2.0.1', 'vsdx==0.5.13', 'python-magic==0.4.27', - 'setuptools==67.7.2', + 'setuptools==67.8.0', 'defusedxml==0.7.1', - 'networkx==3.0', + 'networkx==3.1', 'pygraphviz==1.10' ], use_scm_version={ @@ -44,13 +44,13 @@ "pytest-runner==6.0.0", ], "test": [ - 'tox==4.5.0', + 'tox==4.5.1', 'pytest==7.3.1', 'responses==0.23.1', 'deepdiff==6.3.0', - 'httpx==0.23.3', + 'httpx==0.24.1', 'pytest-mock==3.10.0', - 'coverage==7.2.3' + 'coverage==7.2.6' ] }, entry_points=''' From 2e129258a4e8f66a9be278569fd2d892224cb6ed Mon Sep 17 00:00:00 2001 From: Santi Manero Date: Thu, 25 May 2023 10:59:32 +0200 Subject: [PATCH 16/18] [OPT-865] Fixed local documentation Docker deployment --- deployment/Dockerfile.docs | 10 ++++++---- docs/requirements.txt | 1 - 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/deployment/Dockerfile.docs b/deployment/Dockerfile.docs index d8244de2..d8f89130 100644 --- a/deployment/Dockerfile.docs +++ b/deployment/Dockerfile.docs @@ -1,8 +1,10 @@ FROM squidfunk/mkdocs-material -RUN pip install --upgrade pip - -RUN pip install -r requirements.txt +RUN adduser -D startleft +USER startleft COPY /docs ./docs -COPY mkdocs.yml . \ No newline at end of file +COPY mkdocs.yml . + +RUN pip install --upgrade pip +RUN pip install -r docs/requirements.txt diff --git a/docs/requirements.txt b/docs/requirements.txt index 305dce47..4a489694 100644 --- a/docs/requirements.txt +++ b/docs/requirements.txt @@ -1,3 +1,2 @@ mkdocs-material==9.1.1 -pymdown-extensions==9.10 mkdocs-glightbox==0.3.1 \ No newline at end of file From 4fdf566fa8cc6d66013b496c337f38c75b576c25 Mon Sep 17 00:00:00 2001 From: Santi Manero Date: Thu, 25 May 2023 11:48:01 +0200 Subject: [PATCH 17/18] [OPT-865] Upgraded versions in docs/requirements.txt --- docs/requirements.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/requirements.txt b/docs/requirements.txt index 4a489694..506e2c3b 100644 --- a/docs/requirements.txt +++ b/docs/requirements.txt @@ -1,2 +1,2 @@ -mkdocs-material==9.1.1 -mkdocs-glightbox==0.3.1 \ No newline at end of file +mkdocs-material==9.1.14 +mkdocs-glightbox==0.3.4 \ No newline at end of file From af2b5f95ce46b5d19368245fe9f3826a02759e68 Mon Sep 17 00:00:00 2001 From: David Antolin Alvarez Date: Tue, 6 Jun 2023 13:44:52 +0200 Subject: [PATCH 18/18] [OPT-879] pygraphviz lib version fixed to 1.10 --- .github/workflows/startleft-unit-integration-full.yml | 2 +- setup.py | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/.github/workflows/startleft-unit-integration-full.yml b/.github/workflows/startleft-unit-integration-full.yml index 7668b593..30da96ad 100644 --- a/.github/workflows/startleft-unit-integration-full.yml +++ b/.github/workflows/startleft-unit-integration-full.yml @@ -41,7 +41,7 @@ jobs: if: runner.os == 'Windows' shell: bash run: | - pip install --global-option=build_ext --global-option="-IC:\Program files\Graphviz\include" --global-option="-LC:\Program files\Graphviz\lib" pygraphviz + pip install --global-option=build_ext --global-option="-IC:\Program files\Graphviz\include" --global-option="-LC:\Program files\Graphviz\lib" pygraphviz==1.10 echo "C:\Program Files\Graphviz\bin" >> $GITHUB_PATH - name: Install dependencies diff --git a/setup.py b/setup.py index 2dc1a46c..d778b323 100644 --- a/setup.py +++ b/setup.py @@ -31,6 +31,7 @@ 'setuptools==67.8.0', 'defusedxml==0.7.1', 'networkx==3.1', + # Do not upgrade pygraphviz unless security issues because it is heavily dependent on the underlying OS 'pygraphviz==1.10' ], use_scm_version={