Address comments.

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

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

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

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.")

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__(

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:

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=[

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

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