Feat: improve sanity check for eqpt sheet

add cases of wrong sheets that were not captured and generate errors later in propagation: - if the eqpt line is duplicated - if the eqpt contains a link that is not present in Links sheet, but Nodes are OK - if the same link is defined but with opposite directions - if the ila is defined twice with opposite directions - if the service type or mode are not in the library Signed-off-by: EstherLerouzic <[email protected]> Change-Id: I4715886e19f07380bf02ed0e664559972bb39b71
Telecominfraproject · Nov 2, 2023 · 3bea4b3 · 3bea4b3
1 parent f2cc9f7
commit 3bea4b3
Show file tree

Hide file tree

Showing 8 changed files with 66 additions and 7 deletions.
diff --git a/gnpy/tools/convert.py b/gnpy/tools/convert.py
@@ -227,11 +227,10 @@ def sanity_check(nodes, links, nodes_by_city, links_by_city, eqpts_by_city):
                     link {l1.from_city}-{l1.to_city} is duplicate \
                     \nthe 1st duplicate link will be removed but you should check Links sheet input')
                 duplicate_links.append(l1)
-    for l in duplicate_links:
-        links.remove(l)
-        links_by_city[l.from_city].remove(l)
-        links_by_city[l.to_city].remove(l)
-
+    if duplicate_links:
+        msg = 'XLS error: ' \
+              + f'links {_format_items([(d.from_city, d.to_city) for d in duplicate_links])} are duplicate'
+        raise NetworkTopologyError(msg)
     unreferenced_nodes = [n for n in nodes_by_city if n not in links_by_city]
     if unreferenced_nodes:
         msg = 'XLS error: The following nodes are not ' \
@@ -250,6 +249,43 @@ def sanity_check(nodes, links, nodes_by_city, links_by_city, eqpts_by_city):
               + 'are not defined in the Nodes sheet:\n'\
               + _format_items(wrong_eqpt)
         raise NetworkTopologyError(msg)
+    # Now check links that are not listed in Links sheet, and duplicates
+    bad_eqpt = []
+    possible_links = [f'{e.from_city}|{e.to_city}' for e in links] + [f'{e.to_city}|{e.from_city}' for e in links]
+    possible_eqpt = []
+    duplicate_eqpt = []
+    duplicate_ila = []
+    for city, eqpts in eqpts_by_city.items():
+        for eqpt in eqpts:
+            # Check that each node_A-node_Z exists in links
+            nodea_nodez = f'{eqpt.from_city}|{eqpt.to_city}'
+            nodez_nodea = f'{eqpt.to_city}|{eqpt.from_city}'
+            if nodea_nodez not in possible_links \
+                    or nodez_nodea not in possible_links:
+                bad_eqpt.append([eqpt.from_city, eqpt.to_city])
+            else:
+                # Check that there are no duplicate lines in the Eqpt sheet
+                if nodea_nodez in possible_eqpt:
+                    duplicate_eqpt.append([eqpt.from_city, eqpt.to_city])
+                else:
+                    possible_eqpt.append(nodea_nodez)
+            # check that there are no two lines defining an ILA with different directions
+        if nodes_by_city[city].node_type == 'ILA' and len(eqpts) > 1:
+            duplicate_ila.append(city)
+    if bad_eqpt:
+        msg = 'XLS error: ' \
+              + 'The Eqpt sheet references links that ' \
+              + 'are not defined in the Links sheet:\n' \
+              + _format_items(f'{item[0]} -> {item[1]}' for item in bad_eqpt)
+        raise NetworkTopologyError(msg)
+    if duplicate_eqpt:
+        msg = 'XLS error: Duplicate lines in Eqpt sheet:' \
+              + _format_items(f'{item[0]} -> {item[1]}' for item in duplicate_eqpt)
+        raise NetworkTopologyError(msg)
+    if duplicate_ila:
+        msg = 'XLS error: Duplicate ILA eqpt definition in Eqpt sheet:' \
+              + _format_items(duplicate_ila)
+        raise NetworkTopologyError(msg)
 
     for city, link in links_by_city.items():
         if nodes_by_city[city].node_type.lower() == 'ila' and len(link) != 2:

diff --git a/tests/data/wrong_duplicate_eqpt_ila_reverse.xlsx b/tests/data/wrong_duplicate_eqpt_ila_reverse.xlsx
diff --git a/tests/data/wrong_duplicate_link_reverse.xlsx b/tests/data/wrong_duplicate_link_reverse.xlsx
diff --git a/tests/data/wrong_service.xlsx b/tests/data/wrong_service.xlsx
diff --git a/tests/data/wrong_service_type.xlsx b/tests/data/wrong_service_type.xlsx
diff --git a/tests/data/wrong_topo_bad_eqpt.xlsx b/tests/data/wrong_topo_bad_eqpt.xlsx
diff --git a/tests/data/wrong_topo_duplicate_eqpt.xlsx b/tests/data/wrong_topo_duplicate_eqpt.xlsx
diff --git a/tests/test_logger.py b/tests/test_logger.py
@@ -10,7 +10,8 @@
 
 from gnpy.core.exceptions import ConfigurationError, ServiceError, EquipmentConfigError, ParametersError, \
     NetworkTopologyError
-from gnpy.tools.json_io import SI, Roadm, Amp, load_equipment, requests_from_json, network_from_json
+from gnpy.tools.json_io import SI, Roadm, Amp, load_equipment, requests_from_json, network_from_json, \
+    load_network, load_requests
 from gnpy.tools.convert import xls_to_json_data
 
 TEST_DIR = Path(__file__).parent
@@ -85,6 +86,22 @@ def test_wrong_equipment(caplog, error, equipment, json_data, expected_msg):
         _ = equipment(**json_data)
 
 
+@pytest.mark.parametrize('xls_service_filename, xls_topo_filename, expected_msg',
+                         [('wrong_service.xlsx', 'testTopology.xls',
+                           "Service error: Request Id: 0 - could not find tsp : 'Voyager' with mode: 'Mode 10' "
+                           + "in eqpt library \nComputation stopped."),
+                          ('wrong_service_type.xlsx', 'testTopology.xls',
+                           "Service error: Request Id: 0 - could not find tsp : 'Galileo' with mode: 'mode 1' "
+                           + "in eqpt library \nComputation stopped.")])
+def test_wrong_xls_service(xls_service_filename, xls_topo_filename, expected_msg):
+    """
+    """
+    equipment = load_equipment(EQPT_FILENAME)
+    network = load_network(DATA_DIR / xls_topo_filename, equipment)
+    with pytest.raises(ServiceError, match=expected_msg):
+        _ = load_requests(DATA_DIR / xls_service_filename, equipment, False, network, DATA_DIR / xls_topo_filename)
+
+
 def wrong_amp():
     """Creates list of malformed equipments
     """
@@ -381,7 +398,13 @@ def test_json_network(error, json_data, expected_msg):
      (DATA_DIR / 'wrong_topo_link_header.xlsx', 'missing header Node Z'),
      (DATA_DIR / 'wrong_topo_eqpt.xlsx', 'XLS error: The Eqpt sheet refers to nodes that are not defined in the '
                                          + 'Nodes sheet:\n - toto'),
-     (DATA_DIR / 'wrong_topo_duplicate_node.xlsx', 'Duplicate city: Counter({\'ALB\': 2, \'CHA_3\': 1})')])
+     (DATA_DIR / 'wrong_topo_duplicate_node.xlsx', 'Duplicate city: Counter({\'ALB\': 2, \'CHA_3\': 1})'),
+     (DATA_DIR / 'wrong_topo_duplicate_eqpt.xlsx', 'XLS error: Duplicate lines in Eqpt sheet: - ALB -> CHA_3'),
+     (DATA_DIR / 'wrong_topo_bad_eqpt.xlsx', 'XLS error: The Eqpt sheet references links that are not defined '
+                                             + 'in the Links sheet:\n - toto -> CHA_3'),
+     (DATA_DIR / 'wrong_duplicate_link_reverse.xlsx', 'XLS error: links  - (\'ila\', \'siteb\') are duplicate'),
+     (DATA_DIR / 'wrong_duplicate_eqpt_ila_reverse.xlsx', 'XLS error: Duplicate ILA eqpt definition in Eqpt sheet:'
+                                                          + ' - ila')])
 def test_wrong_xlsx(input_filename, expected_msg):
     """Check that error and logs are correctly working
     """