diff --git a/dbt_semantic_interfaces/implementations/semantic_model.py b/dbt_semantic_interfaces/implementations/semantic_model.py index 37b18835..eb5c098b 100644 --- a/dbt_semantic_interfaces/implementations/semantic_model.py +++ b/dbt_semantic_interfaces/implementations/semantic_model.py @@ -5,7 +5,6 @@ from pydantic import validator from typing_extensions import override -from dbt_semantic_interfaces.enum_extension import assert_values_exhausted from dbt_semantic_interfaces.implementations.base import ( HashableBaseModel, ModelWithMetadataParsing, @@ -26,7 +25,6 @@ SemanticModelReference, TimeDimensionReference, ) -from dbt_semantic_interfaces.type_enums import EntityType class NodeRelation(HashableBaseModel): @@ -189,19 +187,4 @@ def checked_agg_time_dimension_for_measure( # noqa: D @property def primary_entity_reference(self) -> Optional[EntityReference]: # noqa: D - if self.primary_entity is not None: - return EntityReference(element_name=self.primary_entity) - - for entity in self.entities: - if entity.type is EntityType.PRIMARY: - return entity.reference - elif ( - entity.type is EntityType.UNIQUE - or entity.type is EntityType.FOREIGN - or entity.type is EntityType.NATURAL - ): - pass - else: - assert_values_exhausted(entity.type) - - return None + return EntityReference(element_name=self.primary_entity) if self.primary_entity is not None else None diff --git a/dbt_semantic_interfaces/protocols/semantic_model.py b/dbt_semantic_interfaces/protocols/semantic_model.py index adb5036b..44e7fac1 100644 --- a/dbt_semantic_interfaces/protocols/semantic_model.py +++ b/dbt_semantic_interfaces/protocols/semantic_model.py @@ -76,7 +76,7 @@ def node_relation(self) -> NodeRelation: # noqa: D @property @abstractmethod - def primary_entity(self) -> Optional[str]: # noqa: D + def primary_entity(self) -> Optional[str]: """The primary entity for dimensions listed in this model. This is for cases where there are dimensions in the model, but no entity with primary type. This allows those diff --git a/dbt_semantic_interfaces/validations/entities.py b/dbt_semantic_interfaces/validations/entities.py index 38db7a88..9d9755b4 100644 --- a/dbt_semantic_interfaces/validations/entities.py +++ b/dbt_semantic_interfaces/validations/entities.py @@ -1,6 +1,5 @@ import logging -from datetime import date -from typing import Generic, List, MutableSet, Sequence +from typing import Generic, List, Sequence from dbt_semantic_interfaces.protocols import SemanticManifestT, SemanticModel from dbt_semantic_interfaces.references import SemanticModelReference @@ -10,7 +9,6 @@ SemanticManifestValidationRule, SemanticModelContext, ValidationError, - ValidationFutureError, ValidationIssue, validate_safely, ) @@ -69,43 +67,3 @@ def validate_manifest(semantic_manifest: SemanticManifestT) -> Sequence[Validati ) return issues - - -class OnePrimaryEntityPerSemanticModelRule( - SemanticManifestValidationRule[SemanticManifestT], Generic[SemanticManifestT] -): - """Ensures that each semantic model has only one primary entity.""" - - @staticmethod - @validate_safely(whats_being_done="checking semantic model has only one primary entity") - def _only_one_primary_entity(semantic_model: SemanticModel) -> List[ValidationIssue]: - primary_entity_names: MutableSet[str] = set() - for entity in semantic_model.entities or []: - if entity.type == EntityType.PRIMARY: - primary_entity_names.add(entity.reference.element_name) - - if len(primary_entity_names) > 1: - return [ - ValidationFutureError( - context=SemanticModelContext( - file_context=FileContext.from_metadata(metadata=semantic_model.metadata), - semantic_model=SemanticModelReference(semantic_model_name=semantic_model.name), - ), - message=f"Semantic models can have only one primary entity. The semantic model" - f" `{semantic_model.name}` has {len(primary_entity_names)}: {', '.join(primary_entity_names)}", - error_date=date(2022, 1, 12), # Wed January 12th 2022 - ) - ] - return [] - - @staticmethod - @validate_safely( - whats_being_done="running model validation ensuring each semantic model has only one primary entity" - ) - def validate_manifest(semantic_manifest: SemanticManifestT) -> Sequence[ValidationIssue]: # noqa: D - issues = [] - - for semantic_model in semantic_manifest.semantic_models: - issues += OnePrimaryEntityPerSemanticModelRule._only_one_primary_entity(semantic_model=semantic_model) - - return issues diff --git a/dbt_semantic_interfaces/validations/primary_entity.py b/dbt_semantic_interfaces/validations/primary_entity.py index 3e8ef57f..fdb29783 100644 --- a/dbt_semantic_interfaces/validations/primary_entity.py +++ b/dbt_semantic_interfaces/validations/primary_entity.py @@ -4,7 +4,7 @@ from dbt_semantic_interfaces.implementations.semantic_manifest import ( PydanticSemanticManifest, ) -from dbt_semantic_interfaces.protocols import Entity, SemanticManifestT, SemanticModel +from dbt_semantic_interfaces.protocols import SemanticManifestT, SemanticModel from dbt_semantic_interfaces.references import SemanticModelReference from dbt_semantic_interfaces.type_enums import EntityType from dbt_semantic_interfaces.validations.validator_helpers import ( @@ -20,34 +20,18 @@ class PrimaryEntityRule(SemanticManifestValidationRule[SemanticManifestT], Generic[SemanticManifestT]): - """If a semantic model contains dimensions, the primary entity must be available. + """Checks that the primary entity has been properly defined in a semantic model. - The primary entity could be defined by the primary_entity field, or by one of the entities defined in a semantic - model. + * If a semantic model contains dimensions, the primary entity must be available. + * The primary entity could be defined by the primary_entity field, or by one of the entities defined in a semantic + model. + * There should only be one primary entity in the model. """ @staticmethod def _model_requires_primary_entity(semantic_model: SemanticModel) -> bool: return len(semantic_model.dimensions) > 0 - @staticmethod - def _model_has_entity_with_primary_type(semantic_model: SemanticModel) -> bool: - return any(entity.type == EntityType.PRIMARY for entity in semantic_model.entities) - - @staticmethod - def _entity_with_primary_type_in_model(semantic_model: SemanticModel) -> Entity: - for entity in semantic_model.entities: - if entity.type == EntityType.PRIMARY: - return entity - entities_with_primary_type = tuple( - entity for entity in semantic_model.entities if entity.type == EntityType.PRIMARY - ) - - if len(entities_with_primary_type) == 1: - return entities_with_primary_type[0] - - raise RuntimeError(f"Did not find exactly one entity with entity type {EntityType.PRIMARY} in {semantic_model}") - @staticmethod @validate_safely("Check that a semantic model has properly configured primary entities.") def _check_model(semantic_model: SemanticModel) -> Sequence[ValidationIssue]: @@ -56,19 +40,35 @@ def _check_model(semantic_model: SemanticModel) -> Sequence[ValidationIssue]: semantic_model=SemanticModelReference(semantic_model_name=semantic_model.name), ) - # Check that the primary entity field and the listed entities don't conflict. - if ( - semantic_model.primary_entity_reference is not None - and PrimaryEntityRule._model_has_entity_with_primary_type(semantic_model) - ): - entity_with_primary_type = PrimaryEntityRule._entity_with_primary_type_in_model(semantic_model) - if semantic_model.primary_entity_reference != entity_with_primary_type.reference: + # If there are entities defined in the model, check that there's only one primary entity. + entities_with_primary_time = tuple( + entity for entity in semantic_model.entities if entity.type is EntityType.PRIMARY + ) + + if len(entities_with_primary_time) > 0: + if len(entities_with_primary_time) > 1: + primary_entity_names = [primary_entity.name for primary_entity in entities_with_primary_time] + return ( + ValidationError( + message=( + f"Semantic models can have only one primary entity. The semantic model" + f" `{semantic_model.name}` has {len(primary_entity_names)}: " + f"{', '.join(primary_entity_names)}" + ), + context=context, + ), + ) + + entity_with_primary_type = entities_with_primary_time[0] + # If there is a primary entity, the primary entity field should not be set. + if semantic_model.primary_entity_reference is not None: return ( ValidationError( message=( - f"Semantic model {semantic_model.name} has an entity named {entity_with_primary_type.name} " - f"with type primary but it conflicts with the primary_entity field set to " - f"{semantic_model.primary_entity_reference.element_name}" + f"The semantic model `{semantic_model.name}` has an entity named " + f"`{entity_with_primary_type.name}` with type primary but it also has the `primary_entity` " + f"field set to `{semantic_model.primary_entity_reference.element_name}`. Both should not " + f"be present in the model." ), context=context, ), @@ -78,19 +78,20 @@ def _check_model(semantic_model: SemanticModel) -> Sequence[ValidationIssue]: if ( PrimaryEntityRule._model_requires_primary_entity(semantic_model) and semantic_model.primary_entity_reference is None - and not PrimaryEntityRule._model_has_entity_with_primary_type(semantic_model) + and len(entities_with_primary_time) == 0 ): return ( ValidationError( message=( f"The semantic model {semantic_model.name} contains dimensions, but it does not define a " - f"primary entity." + f"primary entity. Either add an entity with type PRIMARY or set a value for the " + f"primary_entity key." ), context=context, ), ) - return [] + return () @staticmethod @validate_safely("Check that semantic models in the manifest have properly configured primary entities.") diff --git a/dbt_semantic_interfaces/validations/semantic_manifest_validator.py b/dbt_semantic_interfaces/validations/semantic_manifest_validator.py index de11a5a6..47304e07 100644 --- a/dbt_semantic_interfaces/validations/semantic_manifest_validator.py +++ b/dbt_semantic_interfaces/validations/semantic_manifest_validator.py @@ -9,10 +9,7 @@ ) from dbt_semantic_interfaces.validations.dimension_const import DimensionConsistencyRule from dbt_semantic_interfaces.validations.element_const import ElementConsistencyRule -from dbt_semantic_interfaces.validations.entities import ( - NaturalEntityConfigurationRule, - OnePrimaryEntityPerSemanticModelRule, -) +from dbt_semantic_interfaces.validations.entities import NaturalEntityConfigurationRule from dbt_semantic_interfaces.validations.measures import ( CountAggregationExprRule, MeasureConstraintAliasesRule, @@ -26,6 +23,7 @@ DerivedMetricRule, ) from dbt_semantic_interfaces.validations.non_empty import NonEmptyRule +from dbt_semantic_interfaces.validations.primary_entity import PrimaryEntityRule from dbt_semantic_interfaces.validations.reserved_keywords import ReservedKeywordsRule from dbt_semantic_interfaces.validations.semantic_models import ( SemanticModelDefaultsRule, @@ -65,7 +63,6 @@ class SemanticManifestValidator(Generic[SemanticManifestT]): DimensionConsistencyRule[SemanticManifestT](), ElementConsistencyRule[SemanticManifestT](), NaturalEntityConfigurationRule[SemanticManifestT](), - OnePrimaryEntityPerSemanticModelRule[SemanticManifestT](), MeasureConstraintAliasesRule[SemanticManifestT](), MetricMeasuresRule[SemanticManifestT](), CumulativeMetricRule[SemanticManifestT](), @@ -75,6 +72,7 @@ class SemanticManifestValidator(Generic[SemanticManifestT]): ReservedKeywordsRule[SemanticManifestT](), MeasuresNonAdditiveDimensionRule[SemanticManifestT](), SemanticModelDefaultsRule[SemanticManifestT](), + PrimaryEntityRule[SemanticManifestT](), ) def __init__( diff --git a/tests/validations/test_entities.py b/tests/validations/test_entities.py index da4e2d97..2457a78f 100644 --- a/tests/validations/test_entities.py +++ b/tests/validations/test_entities.py @@ -17,10 +17,8 @@ find_semantic_model_with, ) from dbt_semantic_interfaces.type_enums import EntityType -from dbt_semantic_interfaces.validations.entities import ( - NaturalEntityConfigurationRule, - OnePrimaryEntityPerSemanticModelRule, -) +from dbt_semantic_interfaces.validations.entities import NaturalEntityConfigurationRule +from dbt_semantic_interfaces.validations.primary_entity import PrimaryEntityRule from dbt_semantic_interfaces.validations.semantic_manifest_validator import ( SemanticManifestValidator, ) @@ -49,22 +47,19 @@ def func(semantic_model: PydanticSemanticModel) -> bool: entity_references.add(entity.reference) model_issues = SemanticManifestValidator[PydanticSemanticManifest]( - [OnePrimaryEntityPerSemanticModelRule()] + [PrimaryEntityRule()] ).validate_semantic_manifest(model) - future_issue = ( + expected_issue_message = ( f"Semantic models can have only one primary entity. The semantic model" f" `{multiple_entity_semantic_model.name}` has {len(entity_references)}" ) - found_future_issue = False - - if model_issues is not None: - for issue in model_issues.all_issues: - if re.search(future_issue, issue.message): - found_future_issue = True - - assert found_future_issue + assert len( + tuple( + issue for issue in model_issues.all_issues if re.search(expected_issue_message, issue.message) is not None + ) + ) def test_multiple_natural_entities() -> None: diff --git a/tests/validations/test_metrics.py b/tests/validations/test_metrics.py index 6fe7d056..91afe196 100644 --- a/tests/validations/test_metrics.py +++ b/tests/validations/test_metrics.py @@ -53,6 +53,7 @@ def test_metric_no_time_dim_dim_only_source() -> None: # noqa:D name="sum_measure", measures=[], dimensions=[PydanticDimension(name=dim_name, type=DimensionType.CATEGORICAL)], + entities=[PydanticEntity(name="primary_entity2", type=EntityType.PRIMARY)], ), semantic_model_with_guaranteed_meta( name="sum_measure2", @@ -73,6 +74,7 @@ def test_metric_no_time_dim_dim_only_source() -> None: # noqa:D ), ), ], + entities=[PydanticEntity(name="primary_entity2", type=EntityType.PRIMARY)], ), ], metrics=[ diff --git a/tests/validations/test_primary_entity.py b/tests/validations/test_primary_entity.py index 5f372d78..9ae2e45c 100644 --- a/tests/validations/test_primary_entity.py +++ b/tests/validations/test_primary_entity.py @@ -41,4 +41,4 @@ def test_primary_entity_conflict(simple_semantic_manifest: PydanticSemanticManif model_validator = SemanticManifestValidator[PydanticSemanticManifest]([PrimaryEntityRule()]) errors = model_validator.validate_semantic_manifest(semantic_manifest_copy).errors assert len(errors) == 1 - assert errors[0].message.find("conflicts with the primary_entity field") != -1 + assert errors[0].message.find("Both should not be present in the model.") != -1 diff --git a/tests/validations/test_validity_param_definitions.py b/tests/validations/test_validity_param_definitions.py index 8ce82e7b..22ae7e78 100644 --- a/tests/validations/test_validity_param_definitions.py +++ b/tests/validations/test_validity_param_definitions.py @@ -33,6 +33,7 @@ def test_validity_window_configuration() -> None: node_relation: schema_name: some_schema alias: scd_table + primary_entity: some_primary_entity entities: - name: scd_key type: natural