Merge pull request #308 from iriusrisk/feature/OPT-992

[feature/OPT-992] to dev
iriusrisk · Sep 25, 2023 · ea544c2 · ea544c2
2 parents 1fcd1dc + fb47e2a
commit ea544c2
Show file tree

Hide file tree

Showing 4 changed files with 139 additions and 75 deletions.
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
@@ -6,6 +6,7 @@
 from slp_visio.slp_visio.load.objects.diagram_objects import DiagramConnector
 from slp_visio.slp_visio.load.strategies.connector.create_connector_strategy import CreateConnectorStrategy, \
     CreateConnectorStrategyContainer
+from slp_visio.slp_visio.util.visio import is_bidirectional_connector, connector_has_arrow_in_origin
 
 
 @register(CreateConnectorStrategyContainer.visio_strategies)
@@ -19,10 +20,10 @@ def create_connector(self, shape: Shape, components=None) -> Optional[DiagramCon
         if not self.are_two_different_shapes(connected_shapes):
             return None
 
-        if self.is_bidirectional_connector(shape):
+        if 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)
+        has_arrow_in_origin = 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])):
@@ -39,21 +40,6 @@ def are_two_different_shapes(connected_shapes) -> bool:
             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/connector/impl/create_connector_by_line_coordinates.py b/slp_visio/slp_visio/load/strategies/connector/impl/create_connector_by_line_coordinates.py
@@ -8,6 +8,7 @@
 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, \
     CreateConnectorStrategyContainer
+from slp_visio.slp_visio.util.visio import is_bidirectional_connector
 
 
 @register(CreateConnectorStrategyContainer.visio_strategies)
@@ -22,7 +23,7 @@ class CreateConnectorByLineCoordinates(CreateConnectorStrategy):
 
     def __init__(self):
         self.tolerance = 0.09
-        self.representer: SimpleComponentRepresenter() = SimpleComponentRepresenter()
+        self.representer: SimpleComponentRepresenter = SimpleComponentRepresenter()
 
     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:
@@ -40,11 +41,21 @@ def create_connector(self, shape: Shape, components=None) -> Optional[DiagramCon
         if not origin or not target:
             return None
 
-        return DiagramConnector(shape.ID, origin, target, name=shape.text)
+        return DiagramConnector(
+            id=shape.ID,
+            from_id=origin,
+            to_id=target,
+            bidirectional=is_bidirectional_connector(shape),
+            name=shape.text)
+
+    def __match_component(self, point, components) -> Optional[str]:
+        matching_component = {}
 
-    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
+            distance = round(polygon.exterior.distance(point), 2)
+            if distance <= matching_component.get('distance', self.tolerance):
+                matching_component['id'] = component.ID
+                matching_component['distance'] = distance
+
+        return matching_component.get('id', None)
diff --git a/slp_visio/slp_visio/util/visio.py b/slp_visio/slp_visio/util/visio.py
@@ -7,6 +7,7 @@
 
 from slp_visio.slp_visio.parse.shape_position_calculator import ShapePositionCalculator
 
+
 @singledispatch
 def get_shape_text(shape):
     if not shape:
@@ -18,14 +19,14 @@ def get_shape_text(shape):
 
     return (result or "").strip()
 
+
 @get_shape_text.register(list)
 def get_shape_text_from_list(shapes: [Shape]) -> str:
     if not shapes:
         return ""
     return "".join(shape.text or "" for shape in shapes)
 
 
-
 def get_master_shape_text(shape: Shape) -> str:
     if not shape.master_shape:
         return ""
@@ -42,6 +43,7 @@ def get_unique_id_text(shape: Shape) -> str:
     unique_id = shape.master_page.master_unique_id.strip()
     return normalize_unique_id(unique_id)
 
+
 def get_width(shape: Shape) -> float:
     if 'Width' in shape.cells:
         return float(shape.cells['Width'].value)
@@ -79,6 +81,7 @@ def get_limits(shape: Shape) -> tuple:
     return (center_x - (width / 2), center_y - (height / 2)), \
         (center_x + (width / 2), center_y + (height / 2))
 
+
 # These expressions are secure, so we can use the standard re lib by performance reason
 LUCID_AWS_YEARS_PATTERN = re.compile(r'_?(?:2017|AWS19|AWS19_v2|AWS2021)$')
 NON_PRINTABLE_CHARS_PATTERN = re.compile(f'[^{re.escape(string.printable)}]')
@@ -107,5 +110,22 @@ def normalize_label(label: str) -> str:
     return label
 
 
-def normalize_unique_id(unique_id):
+def normalize_unique_id(unique_id: str):
     return re.sub("[{}]", "", unique_id) if unique_id else ""
+
+
+def connector_has_arrow_in_origin(shape: 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'
+
+
+def is_bidirectional_connector(shape: Shape) -> bool:
+    """
+    If its master name includes 'Double Arrow' or has arrows defined in both ends
+    """
+    if shape.master_shape and shape.master_page.name 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
diff --git a/...sio/tests/unit/load/strategies/dataflow/impl/test_create_connector_by_line_coordinates.py b/...sio/tests/unit/load/strategies/dataflow/impl/test_create_connector_by_line_coordinates.py
@@ -1,84 +1,87 @@
-from unittest.mock import MagicMock, Mock
+from unittest.mock import MagicMock, Mock, patch
 
 from pytest import mark, param
 
 from slp_visio.slp_visio.load.strategies.connector.impl.create_connector_by_line_coordinates import \
     CreateConnectorByLineCoordinates
 
+TOLERANCE = 0.09
+
 
 def mock_component(_id, pos):
     x, y = pos[0], pos[1]
     width, height = pos[2], pos[3]
     mocked = 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))
-    mocked.parent=None
+                       cells={'Width': Mock(value=width), 'Height': Mock(value=height)},
+                       center_x_y=(float(x) + float(width) / 2, float(y) + float(height) / 2))
+    mocked.parent = None
     return mocked
 
 
 class TestCreateConnectorByLineCoordinates:
     @mark.parametrize('line,start,end', [
         param(['1040', '-560', '1290', '-560'], ['960', '-600', '80', '80'], ['1290', '-590', '60', '60'],
-                     id="perfect_match,strings,big_scale"),
+              id="perfect_match,strings,big_scale"),
         param([1040, -560, 1290, -560], [960, -600, 80, 80], [1290, -590, 60, 60],
-                     id="perfect_match,big_scale"),
+              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"),
+              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"),
+              id="perfect_match"),
+        param([1.04 + TOLERANCE, -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 - TOLERANCE, -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 - TOLERANCE, 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 + TOLERANCE, 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 - TOLERANCE, -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 + TOLERANCE, -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 - TOLERANCE], [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 + TOLERANCE], [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
-        shape = Mock(ID=1111, begin_x=line[0], begin_y=line[1], end_x=line[2], end_y=line[3])
+        shape = MagicMock(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])
+        diagram_connector = CreateConnectorByLineCoordinates() \
+            .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', [
-        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"),
+        param([1.04 + TOLERANCE + 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 - TOLERANCE - 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 + TOLERANCE + 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 - TOLERANCE - 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 - TOLERANCE - 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 + TOLERANCE + 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 - TOLERANCE - 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 + TOLERANCE + 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
@@ -89,19 +92,64 @@ def test_create_connector_with_line_not_touching(self, line, start, end):
         end_component = mock_component(555, end)
 
         # WHEN the connector is created
-        strategy = CreateConnectorByLineCoordinates()
-        diagram_connector = strategy.create_connector(shape, components=[start_component, end_component])
+        diagram_connector = CreateConnectorByLineCoordinates() \
+            .create_connector(shape, components=[start_component, end_component])
 
         # THEN no diagram is returned
         assert not diagram_connector
 
+    def test_create_connector_with_two_components_inside_tolerance_area(self):
+        # GIVEN a visio mocked connector shape
+        shape = MagicMock(ID=11, begin_x=1.04, begin_y=-0.6 - TOLERANCE, end_x=1.29, end_y=-0.59 - TOLERANCE)
+
+        # AND the start_component inside a valid tolerance area
+        start_component = mock_component(44, [0.96, -0.6, 0.08, 0.08])
+        # AND another component inside the start tolerance area, but further away than the other
+        close_to_start_component = mock_component(444, [0.96, -0.6 + 0.01, 0.08, 0.08])
+
+        # AND the end_component inside a valid tolerance area
+        end_component = mock_component(55, [1.29, -0.59, 0.06, 0.06])
+        # AND another component inside the end tolerance area, but further away than the other
+        close_to_end_component = mock_component(555, [1.29, -0.59 + 0.01, 0.06, 0.06])
+
+        # WHEN the connector is created
+        diagram_connector = CreateConnectorByLineCoordinates().create_connector(
+            shape=shape,
+            components=[start_component, close_to_start_component, end_component, close_to_end_component])
+
+        # THEN the returned diagram connector has the following properties
+        assert diagram_connector.id == 11
+        assert diagram_connector.from_id == 44
+        assert diagram_connector.to_id == 55
+        assert not diagram_connector.bidirectional
+
+    def test_create_bidirectional_connector(self, mocker):
+        # GIVEN a visio mocked connector shape
+        shape = MagicMock(ID=1, begin_x=1.04, begin_y=-0.56, end_x=1.29, end_y=-0.56)
+
+        # AND two connected components
+        component_a = mock_component(4, [0.96, -0.6, 0.08, 0.08])
+        component_b = mock_component(5, [1.29, -0.59, 0.06, 0.06])
+
+        # AND a mock for the is_bidirectional_connector function
+        # WHEN the connector is created
+        with patch('slp_visio.slp_visio.load.strategies.connector.impl.create_connector_by_line_coordinates.'
+                   'is_bidirectional_connector', return_value=True):
+            diagram_connector = CreateConnectorByLineCoordinates() \
+                .create_connector(shape=shape, components=[component_a, component_b])
+
+        # THEN the returned diagram connector has the following properties
+        assert diagram_connector.id == 1
+        assert diagram_connector.from_id == 4
+        assert diagram_connector.to_id == 5
+        assert diagram_connector.bidirectional
+
     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)
+        diagram_connector = CreateConnectorByLineCoordinates().create_connector(shape)
 
         # THEN no diagram is returned
         assert not diagram_connector
@@ -111,8 +159,7 @@ def test_create_connector_invalid_line(self):
         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)
+        diagram_connector = CreateConnectorByLineCoordinates().create_connector(shape)
 
         # THEN no diagram is returned
         assert not diagram_connector