From 9040d2c1483a3eb54de7368edfb03000ef6bd149 Mon Sep 17 00:00:00 2001 From: Beto Dealmeida Date: Thu, 16 Apr 2026 11:22:44 -0400 Subject: [PATCH] Address comments --- .../src/superset_core/semantic_layers/daos.py | 169 ++++++++++++++++++ .../superset_core/semantic_layers/models.py | 85 +++++++++ superset/daos/semantic_layer.py | 52 +++--- ...7e0e21daa_add_semantic_layers_and_views.py | 29 ++- superset/semantic_layers/models.py | 17 +- .../unit_tests/semantic_layers/models_test.py | 3 +- 6 files changed, 316 insertions(+), 39 deletions(-) create mode 100644 superset-core/src/superset_core/semantic_layers/daos.py create mode 100644 superset-core/src/superset_core/semantic_layers/models.py diff --git a/superset-core/src/superset_core/semantic_layers/daos.py b/superset-core/src/superset_core/semantic_layers/daos.py new file mode 100644 index 00000000000..4b55231d0b2 --- /dev/null +++ b/superset-core/src/superset_core/semantic_layers/daos.py @@ -0,0 +1,169 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you 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 +# +# http://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. + +""" +Semantic layer DAO interfaces for superset-core. + +Provides abstract DAO classes for semantic layers and views that define the +interface contract. Host implementations replace these with concrete classes +backed by SQLAlchemy during initialization. + +Usage: + from superset_core.semantic_layers.daos import ( + AbstractSemanticLayerDAO, + AbstractSemanticViewDAO, + ) +""" + +from __future__ import annotations + +from abc import abstractmethod +from typing import Any, ClassVar + +from superset_core.common.daos import BaseDAO +from superset_core.semantic_layers.models import SemanticLayerModel, SemanticViewModel + + +class AbstractSemanticLayerDAO(BaseDAO[SemanticLayerModel]): + """ + Abstract DAO interface for SemanticLayer. + + Host implementations will replace this class during initialization + with a concrete DAO providing actual database access. + """ + + model_cls: ClassVar[type[Any] | None] = None + base_filter = None + id_column_name = "uuid" + uuid_column_name = "uuid" + + @classmethod + @abstractmethod + def validate_uniqueness(cls, name: str) -> bool: + """ + Validate that a semantic layer name is unique. + + :param name: Semantic layer name to validate + :return: True if the name is unique, False otherwise + """ + ... + + @classmethod + @abstractmethod + def validate_update_uniqueness(cls, layer_uuid: str, name: str) -> bool: + """ + Validate that a semantic layer name is unique for an update operation, + excluding the layer being updated. + + :param layer_uuid: UUID of the semantic layer being updated + :param name: New name to validate + :return: True if the name is unique, False otherwise + """ + ... + + @classmethod + @abstractmethod + def find_by_name(cls, name: str) -> SemanticLayerModel | None: + """ + Find a semantic layer by name. + + :param name: Semantic layer name + :return: SemanticLayerModel instance or None + """ + ... + + @classmethod + @abstractmethod + def get_semantic_views(cls, layer_uuid: str) -> list[SemanticViewModel]: + """ + Get all semantic views associated with a semantic layer. + + :param layer_uuid: UUID of the semantic layer + :return: List of SemanticViewModel instances + """ + ... + + +class AbstractSemanticViewDAO(BaseDAO[SemanticViewModel]): + """ + Abstract DAO interface for SemanticView. + + Host implementations will replace this class during initialization + with a concrete DAO providing actual database access. + """ + + model_cls: ClassVar[type[Any] | None] = None + base_filter = None + id_column_name = "id" + uuid_column_name = "uuid" + + @classmethod + @abstractmethod + def validate_uniqueness( + cls, + name: str, + layer_uuid: str, + configuration: dict[str, Any], + ) -> bool: + """ + Validate that a semantic view is unique within a semantic layer. + + Uniqueness is determined by the combination of name, layer UUID, and + configuration. + + :param name: View name + :param layer_uuid: UUID of the parent semantic layer + :param configuration: Configuration dict to compare + :return: True if unique, False otherwise + """ + ... + + @classmethod + @abstractmethod + def validate_update_uniqueness( + cls, + view_uuid: str, + name: str, + layer_uuid: str, + configuration: dict[str, Any], + ) -> bool: + """ + Validate that a semantic view is unique within a semantic layer for an + update operation, excluding the view being updated. + + :param view_uuid: UUID of the view being updated + :param name: New name to validate + :param layer_uuid: UUID of the parent semantic layer + :param configuration: Configuration dict to compare + :return: True if unique, False otherwise + """ + ... + + @classmethod + @abstractmethod + def find_by_name(cls, name: str, layer_uuid: str) -> SemanticViewModel | None: + """ + Find a semantic view by name within a semantic layer. + + :param name: View name + :param layer_uuid: UUID of the parent semantic layer + :return: SemanticViewModel instance or None + """ + ... + + +__all__ = ["AbstractSemanticLayerDAO", "AbstractSemanticViewDAO"] diff --git a/superset-core/src/superset_core/semantic_layers/models.py b/superset-core/src/superset_core/semantic_layers/models.py new file mode 100644 index 00000000000..e132a108499 --- /dev/null +++ b/superset-core/src/superset_core/semantic_layers/models.py @@ -0,0 +1,85 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you 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 +# +# http://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. + +""" +Semantic layer model interfaces for superset-core. + +Provides abstract model classes for semantic layers and views that will be +replaced by the host implementation's concrete SQLAlchemy models during +initialization. + +Usage: + from superset_core.semantic_layers.models import ( + SemanticLayerModel, + SemanticViewModel, + ) +""" + +from __future__ import annotations + +from datetime import datetime +from uuid import UUID + +from superset_core.common.models import CoreModel + + +class SemanticLayerModel(CoreModel): + """ + Abstract interface for the SemanticLayer database model. + + Host implementations will replace this class during initialization + with a concrete SQLAlchemy model providing actual persistence. + """ + + __abstract__ = True + + # Type hints for expected column attributes + uuid: UUID + name: str + description: str | None + type: str + configuration: str + configuration_version: int + cache_timeout: int | None + created_on: datetime | None + changed_on: datetime | None + + +class SemanticViewModel(CoreModel): + """ + Abstract interface for the SemanticView database model. + + Host implementations will replace this class during initialization + with a concrete SQLAlchemy model providing actual persistence. + """ + + __abstract__ = True + + # Type hints for expected column attributes + id: int + uuid: UUID + name: str + description: str | None + configuration: str + configuration_version: int + cache_timeout: int | None + semantic_layer_uuid: UUID + created_on: datetime | None + changed_on: datetime | None + + +__all__ = ["SemanticLayerModel", "SemanticViewModel"] diff --git a/superset/daos/semantic_layer.py b/superset/daos/semantic_layer.py index c9ad17b0cf3..3ae4d97c431 100644 --- a/superset/daos/semantic_layer.py +++ b/superset/daos/semantic_layer.py @@ -21,22 +21,25 @@ from __future__ import annotations from typing import Any -from superset.daos.base import BaseDAO +from superset_core.semantic_layers.daos import ( + AbstractSemanticLayerDAO, + AbstractSemanticViewDAO, +) + from superset.extensions import db from superset.semantic_layers.models import SemanticLayer, SemanticView from superset.utils import json -class SemanticLayerDAO(BaseDAO[SemanticLayer]): +class SemanticLayerDAO(AbstractSemanticLayerDAO): """ Data Access Object for SemanticLayer model. """ - # SemanticLayer uses 'uuid' as the primary key field - id_column_name = "uuid" + model_cls = SemanticLayer - @staticmethod - def validate_uniqueness(name: str) -> bool: + @classmethod + def validate_uniqueness(cls, name: str) -> bool: """ Validate that semantic layer name is unique. @@ -46,8 +49,8 @@ class SemanticLayerDAO(BaseDAO[SemanticLayer]): query = db.session.query(SemanticLayer).filter(SemanticLayer.name == name) return not db.session.query(query.exists()).scalar() - @staticmethod - def validate_update_uniqueness(layer_uuid: str, name: str) -> bool: + @classmethod + def validate_update_uniqueness(cls, layer_uuid: str, name: str) -> bool: """ Validate that semantic layer name is unique for updates. @@ -61,8 +64,8 @@ class SemanticLayerDAO(BaseDAO[SemanticLayer]): ) return not db.session.query(query.exists()).scalar() - @staticmethod - def find_by_name(name: str) -> SemanticLayer | None: + @classmethod + def find_by_name(cls, name: str) -> SemanticLayer | None: """ Find semantic layer by name. @@ -90,28 +93,14 @@ class SemanticLayerDAO(BaseDAO[SemanticLayer]): ) -class SemanticViewDAO(BaseDAO[SemanticView]): +class SemanticViewDAO(AbstractSemanticViewDAO): """Data Access Object for SemanticView model.""" - # SemanticView uses 'uuid' as the primary key field - id_column_name = "uuid" + model_cls = SemanticView - @staticmethod - def find_by_semantic_layer(layer_uuid: str) -> list[SemanticView]: - """ - Find all views for a semantic layer. - - :param layer_uuid: UUID of the semantic layer - :return: List of SemanticView instances - """ - return ( - db.session.query(SemanticView) - .filter(SemanticView.semantic_layer_uuid == layer_uuid) - .all() - ) - - @staticmethod + @classmethod def validate_uniqueness( + cls, name: str, layer_uuid: str, configuration: dict[str, Any], @@ -140,8 +129,9 @@ class SemanticViewDAO(BaseDAO[SemanticView]): ) return not any(json.loads(c.configuration) == configuration for c in candidates) - @staticmethod + @classmethod def validate_update_uniqueness( + cls, view_uuid: str, name: str, layer_uuid: str, @@ -170,8 +160,8 @@ class SemanticViewDAO(BaseDAO[SemanticView]): ) return not any(json.loads(c.configuration) == configuration for c in candidates) - @staticmethod - def find_by_name(name: str, layer_uuid: str) -> SemanticView | None: + @classmethod + def find_by_name(cls, name: str, layer_uuid: str) -> SemanticView | None: """ Find semantic view by name within a semantic layer. diff --git a/superset/migrations/versions/2025-11-04_11-26_33d7e0e21daa_add_semantic_layers_and_views.py b/superset/migrations/versions/2025-11-04_11-26_33d7e0e21daa_add_semantic_layers_and_views.py index 860b4aa21c0..b230e31a35e 100644 --- a/superset/migrations/versions/2025-11-04_11-26_33d7e0e21daa_add_semantic_layers_and_views.py +++ b/superset/migrations/versions/2025-11-04_11-26_33d7e0e21daa_add_semantic_layers_and_views.py @@ -46,6 +46,7 @@ def upgrade(): create_table( "semantic_layers", sa.Column("uuid", UUIDType(binary=True), default=uuid.uuid4, nullable=False), + # created_on and changed_on are nullable=True to match AuditMixinNullable sa.Column("created_on", sa.DateTime(), nullable=True), sa.Column("changed_on", sa.DateTime(), nullable=True), sa.Column("name", sa.String(length=250), nullable=False), @@ -56,6 +57,14 @@ def upgrade(): encrypted_field_factory.create(JSONType), nullable=True, ), + # configuration_version tracks the schema version of the configuration + # JSON field to aid with migrations as the schema evolves over time. + sa.Column( + "configuration_version", + sa.Integer(), + nullable=False, + server_default="1", + ), sa.Column("cache_timeout", sa.Integer(), nullable=True), sa.Column("created_by_fk", sa.Integer(), nullable=True), sa.Column("changed_by_fk", sa.Integer(), nullable=True), @@ -79,11 +88,16 @@ def upgrade(): ["id"], ) - # Create semantic_views table + # Create semantic_views table. + # The integer `id` is the primary key (auto-increment across all supported + # databases) and `uuid` is a secondary unique identifier. This follows the + # standard Superset model pattern and avoids using sa.Identity(), which is + # not supported in MySQL or SQLite. create_table( "semantic_views", + sa.Column("id", sa.Integer(), autoincrement=True, nullable=False), sa.Column("uuid", UUIDType(binary=True), default=uuid.uuid4, nullable=False), - sa.Column("id", sa.Integer(), sa.Identity(), unique=True, nullable=False), + # created_on and changed_on are nullable=True to match AuditMixinNullable sa.Column("created_on", sa.DateTime(), nullable=True), sa.Column("changed_on", sa.DateTime(), nullable=True), sa.Column("name", sa.String(length=250), nullable=False), @@ -93,6 +107,14 @@ def upgrade(): encrypted_field_factory.create(JSONType), nullable=True, ), + # configuration_version tracks the schema version of the configuration + # JSON field to aid with migrations as the schema evolves over time. + sa.Column( + "configuration_version", + sa.Integer(), + nullable=False, + server_default="1", + ), sa.Column("cache_timeout", sa.Integer(), nullable=True), sa.Column( "semantic_layer_uuid", @@ -102,7 +124,8 @@ def upgrade(): ), sa.Column("created_by_fk", sa.Integer(), nullable=True), sa.Column("changed_by_fk", sa.Integer(), nullable=True), - sa.PrimaryKeyConstraint("uuid"), + sa.PrimaryKeyConstraint("id"), + sa.UniqueConstraint("uuid"), ) # Create foreign key constraints for semantic_views diff --git a/superset/semantic_layers/models.py b/superset/semantic_layers/models.py index 13b0f5016f3..1db2708763b 100644 --- a/superset/semantic_layers/models.py +++ b/superset/semantic_layers/models.py @@ -27,7 +27,7 @@ from typing import Any, TYPE_CHECKING import pyarrow as pa from flask_appbuilder import Model -from sqlalchemy import Column, ForeignKey, Identity, Integer, String, Text +from sqlalchemy import Column, ForeignKey, Integer, String, Text from sqlalchemy.orm import relationship from sqlalchemy_utils import UUIDType from sqlalchemy_utils.types.json import JSONType @@ -117,6 +117,9 @@ class SemanticLayer(AuditMixinNullable, Model): type = Column(String(250), nullable=False) # snowflake, etc configuration = Column(encrypted_field_factory.create(JSONType), default="{}") + # Tracks the schema version of the configuration JSON field to aid with + # migrations as the configuration schema evolves over time. + configuration_version = Column(Integer, nullable=False, default=1) cache_timeout = Column(Integer, nullable=True) # Semantic views relationship @@ -152,14 +155,20 @@ class SemanticView(AuditMixinNullable, Model): __tablename__ = "semantic_views" - uuid = Column(UUIDType(binary=True), primary_key=True, default=uuid.uuid4) - id = Column(Integer, Identity(), unique=True) + # Use integer as the primary key for cross-database auto-increment + # compatibility (sa.Identity() is not supported in MySQL or SQLite). + # The uuid column is a secondary unique identifier used in URLs and perms. + id = Column(Integer, primary_key=True, autoincrement=True) + uuid = Column(UUIDType(binary=True), unique=True, default=uuid.uuid4) # Core fields name = Column(String(250), nullable=False) description = Column(Text, nullable=True) configuration = Column(encrypted_field_factory.create(JSONType), default="{}") + # Tracks the schema version of the configuration JSON field to aid with + # migrations as the configuration schema evolves over time. + configuration_version = Column(Integer, nullable=False, default=1) cache_timeout = Column(Integer, nullable=True) # Semantic layer relationship @@ -242,7 +251,7 @@ class SemanticView(AuditMixinNullable, Model): def data(self) -> ExplorableData: return { # core - "id": self.uuid.hex if self.uuid else uuid.uuid4().hex, + "id": self.id, "uid": self.uid, "type": "semantic_view", "name": self.name, diff --git a/tests/unit_tests/semantic_layers/models_test.py b/tests/unit_tests/semantic_layers/models_test.py index 6b9d878947c..dc846efcdca 100644 --- a/tests/unit_tests/semantic_layers/models_test.py +++ b/tests/unit_tests/semantic_layers/models_test.py @@ -556,6 +556,7 @@ def test_semantic_view_data( view = SemanticView() view.name = "Orders View" view.description = "View of order data" + view.id = 1 view.uuid = uuid.UUID("12345678-1234-5678-1234-567812345678") view.semantic_layer_uuid = uuid.UUID("87654321-4321-8765-4321-876543218765") view.cache_timeout = 3600 @@ -568,7 +569,7 @@ def test_semantic_view_data( data = view.data # Check core fields - assert data["id"] == "12345678123456781234567812345678" + assert data["id"] == 1 assert data["uid"] == "semantic_view_uid_123" assert data["type"] == "semantic_view" assert data["name"] == "Orders View"