diff --git a/tools/abel/model/guid_to_entity_map.py b/tools/abel/model/guid_to_entity_map.py index c8010b40c..5576d3bec 100644 --- a/tools/abel/model/guid_to_entity_map.py +++ b/tools/abel/model/guid_to_entity_map.py @@ -65,10 +65,13 @@ def AddEntity(self, entity: ...) -> None: """ if entity is None: raise ValueError('Cannot add None values to the guid to entity map.') - if not entity.bc_guid: + elif not entity.bc_guid: raise AttributeError(f'{entity.code}: guid missing') - if entity.bc_guid not in self._guid_to_entity_map: + elif entity.bc_guid not in list(self._guid_to_entity_map): self._guid_to_entity_map[entity.bc_guid] = entity + elif self._guid_to_entity_map.get(entity.bc_guid) == entity: + # Do nothing, this mapping already exists + pass else: raise KeyError( f'{entity.bc_guid} maps to {self._guid_to_entity_map[entity.bc_guid]}' diff --git a/tools/abel/model/model_helper.py b/tools/abel/model/model_helper.py index edcdba07f..5b9ba89a6 100644 --- a/tools/abel/model/model_helper.py +++ b/tools/abel/model/model_helper.py @@ -20,6 +20,7 @@ from model import entity_field as ef from model import entity_operation from model import model_builder as mb +from model import site as site_lib from validate import field_translation as ft @@ -304,7 +305,9 @@ def GetConnectionDependencies(entity, dependencies, split_entities): A list of entities connected to entity. """ split_entities.append(entity) - if not entity.connections and entity not in split_entities: + if isinstance(entity, site_lib.Site): + return dependencies + elif not entity.connections and entity not in split_entities: dependencies.append(entity) return dependencies else: diff --git a/tools/abel/model/site.py b/tools/abel/model/site.py index 213e13892..4f1c43f50 100644 --- a/tools/abel/model/site.py +++ b/tools/abel/model/site.py @@ -78,6 +78,11 @@ def FromDict(cls, site_dict: Dict[str, object]) -> ...: ) return site_instance + @property + def bc_guid(self) -> str: + """Returns the BC GUID for a site.""" + return self.guid + @property def entities(self) -> List[str]: """Returns a list of entity guids contained in a site.""" @@ -104,10 +109,14 @@ def AddEntity(self, entity: Entity) -> None: # pylint: disable=unused-argument def GetSpreadsheetRowMapping(self, *args) -> Dict[str, str]: """Returns a dictionary of Site attributes by spreadsheet headers.""" - return { + site_row_mapping = { VALUES: [ {USER_ENTERED_VALUE: {STRING_VALUE: self.code}}, {USER_ENTERED_VALUE: {STRING_VALUE: self.guid}}, - {USER_ENTERED_VALUE: {STRING_VALUE: self.etag}}, ] } + if self.etag: + site_row_mapping.get(VALUES).append( + {USER_ENTERED_VALUE: {STRING_VALUE: self.etag}} + ) + return site_row_mapping diff --git a/tools/abel/tests/guid_to_entity_map_test.py b/tools/abel/tests/guid_to_entity_map_test.py index 5784e5cf8..634ba1c99 100644 --- a/tools/abel/tests/guid_to_entity_map_test.py +++ b/tools/abel/tests/guid_to_entity_map_test.py @@ -83,10 +83,13 @@ def testAddEntityRaisesAttributeError(self): _TEST_REPORTING_ENTITY.bc_guid = TEST_REPORTING_GUID def testAddEntityRaisesKeyError(self): + """Test adding a different entity with a already existing guid.""" self.guid_to_entity_map.AddEntity(_TEST_REPORTING_ENTITY) + test_virtual_entity = VirtualEntity.FromDict(TEST_VIRTUAL_ENTITY_DICT) + test_virtual_entity.bc_guid = TEST_REPORTING_GUID with self.assertRaises(KeyError): - self.guid_to_entity_map.AddEntity(_TEST_REPORTING_ENTITY) + self.guid_to_entity_map.AddEntity(test_virtual_entity) def testGetEntityByGuidRaisesKeyError(self): with self.assertRaises(KeyError): diff --git a/tools/validators/instance_validator/tests/entity_instance_test.py b/tools/validators/instance_validator/tests/entity_instance_test.py index cdbbfb4a8..8fed5174f 100644 --- a/tools/validators/instance_validator/tests/entity_instance_test.py +++ b/tools/validators/instance_validator/tests/entity_instance_test.py @@ -849,19 +849,31 @@ def testInstance_InvalidLinkFieldMissing_Fails(self): combination_validator.Validate(entity_instances.get('ENTITY-NAME-GUID')) ) + def testInstance_TypeInstanceWithNoFields_Fails(self): + parsed, default_operation = _Helper( + [path.join(_TESTCASE_PATH, 'BAD', 'type_expecting_fields.yaml')] + ) + + bad_entity_guid, bad_entity_parsed = next(iter(parsed.items())) + bad_entity = entity_instance.EntityInstance.FromYaml( + bad_entity_guid, bad_entity_parsed, default_operation=default_operation + ) + + self.assertFalse(self.init_validator.Validate(bad_entity)) + def testInstance_ValidLinkEntityName_Success(self): parsed, default_operation = _Helper( - [path.join(_TESTCASE_PATH, 'GOOD', 'links.yaml')] + [path.join(_TESTCASE_PATH, 'GOOD', 'links.yaml')] ) entity_instances = {} for entity_guid, entity_parsed in parsed.items(): entity = entity_instance.EntityInstance.FromYaml( - entity_guid, entity_parsed, default_operation=default_operation + entity_guid, entity_parsed, default_operation=default_operation ) entity_instances[entity.guid] = entity combination_validator = entity_instance.CombinationValidator( - self.config_universe, _INIT_CFG, entity_instances + self.config_universe, _INIT_CFG, entity_instances ) for _, instance in entity_instances.items(): diff --git a/tools/validators/instance_validator/tests/fake_instances/BAD/type_expecting_fields.yaml b/tools/validators/instance_validator/tests/fake_instances/BAD/type_expecting_fields.yaml new file mode 100644 index 000000000..ad1f7cb9b --- /dev/null +++ b/tools/validators/instance_validator/tests/fake_instances/BAD/type_expecting_fields.yaml @@ -0,0 +1,24 @@ +# Copyright 2023 Google LLC +# +# Licensed under the Apache License, Version 2.0 (the License); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# https://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an AS IS BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +a3c2fe35-aa03-48f1-a1ef-426203a74177: + type: HVAC/FAN_SS + code: FAN_1 + connections: + 46033ce6-7070-4434-98ab-0d8e6a463d46: CONTAINS + +# Building +46033ce6-7070-4434-98ab-0d8e6a463d46: + type: FACILITIES/BUILDING + code: US-SEA-BLDG1 diff --git a/tools/validators/instance_validator/validate/entity_instance.py b/tools/validators/instance_validator/validate/entity_instance.py index 6a70e363f..2f331d65b 100644 --- a/tools/validators/instance_validator/validate/entity_instance.py +++ b/tools/validators/instance_validator/validate/entity_instance.py @@ -498,7 +498,6 @@ def _ValidateTranslation( Returns: Returns true when the translation is valid on a reporting entity. """ - if entity.translation is None: return True @@ -1067,6 +1066,17 @@ def Validate(self, entity: EntityInstance, is_udmi: bool = True) -> bool: ) is_valid = False + entity_type = self.universe.GetEntityType( + entity.namespace, entity.type_name + ) + if entity_type: + if entity_type.GetAllFields(): + if not entity.translation and not entity.links: + print(f'[ERROR]\tEntity ({entity.guid}: {entity.code}) Has a type ' + 'which has defined fields but this instance has neither links ' + 'nor a translation.') + is_valid = False + if not self._EntityOperationAndConfigModeValid(entity): is_valid = False @@ -1085,8 +1095,6 @@ def Validate(self, entity: EntityInstance, is_udmi: bool = True) -> bool: if not self._IsFaciltitiesEntitiesMatchPattern(entity): is_valid = False - # TODO(berkoben): ADD entity needs transl'n or links if type has fields - return is_valid diff --git a/tools/validators/instance_validator/validate/handler.py b/tools/validators/instance_validator/validate/handler.py index 310471276..d8afdd9b4 100644 --- a/tools/validators/instance_validator/validate/handler.py +++ b/tools/validators/instance_validator/validate/handler.py @@ -37,7 +37,7 @@ def FileNameEnumerationHelper(filename: str) -> str: - """Adds an UTC timestamp enurmation prefix to the filename. + """Adds a UTC timestamp enumeration prefix to the filename. Args: filename: string representing the filename to be enumerated with a local