codeant-ai-for-open-source[bot] commented on code in PR #38370:
URL: https://github.com/apache/superset/pull/38370#discussion_r2880999878
##########
superset/daos/semantic_layer.py:
##########
@@ -32,6 +32,18 @@ class SemanticLayerDAO(BaseDAO[SemanticLayer]):
# SemanticLayer uses 'uuid' as the primary key field
id_column_name = "uuid"
+ @staticmethod
+ def find_by_uuid(uuid_str: str) -> SemanticLayer | None:
+ return (
+ db.session.query(SemanticLayer)
+ .filter(SemanticLayer.uuid == uuid_str)
+ .one_or_none()
+ )
Review Comment:
**Suggestion:** The `find_by_uuid` helper passes the raw `uuid_str` directly
into a `UUIDType(binary=True)` filter; if a client calls any semantic layer
endpoint with a malformed UUID (e.g. `/api/v1/semantic_layer/not-a-uuid`),
SQLAlchemy-Utils will attempt to coerce this string to a UUID and raise
`ValueError`, causing a 500 instead of a clean 404/400 response. [logic error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ GET /api/v1/semantic_layer/<uuid> can 500 on bad UUID.
- ❌ POST /api/v1/semantic_layer/<uuid>/schema/runtime can 500.
- ⚠️ Invalid client input can crash semantic layer detail retrieval.
- ⚠️ Invalid client input can crash semantic runtime schema retrieval.
- ⚠️ Error handling inconsistent with 404 behavior when layer missing.
```
</details>
```suggestion
@staticmethod
def find_by_uuid(uuid_str: str) -> SemanticLayer | None:
try:
return (
db.session.query(SemanticLayer)
.filter(SemanticLayer.uuid == uuid_str)
.one_or_none()
)
except ValueError:
# Malformed UUID strings should be treated as "not found" rather
than erroring
return None
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Start Superset with this PR code deployed and ensure the
`SEMANTIC_LAYERS` feature flag
is enabled (as used in tests in
`superset/tests/unit_tests/semantic_layers/api_test.py:37-41`).
2. Issue an HTTP GET request to the semantic layer detail endpoint with an
invalid UUID
string, e.g. `GET /api/v1/semantic_layer/not-a-uuid`. This endpoint is
implemented by
`SemanticLayerRestApi.get` in `superset/semantic_layers/api.py:457-481`,
where the
`<uuid>` path segment is passed as the `uuid: str` argument.
3. Inside `SemanticLayerRestApi.get`
(`superset/semantic_layers/api.py:479`), the code
calls `layer = SemanticLayerDAO.find_by_uuid(uuid)`, passing the raw string
`"not-a-uuid"`
to the DAO helper.
4. `SemanticLayerDAO.find_by_uuid` in
`superset/daos/semantic_layer.py:35-41` builds a
SQLAlchemy query `.filter(SemanticLayer.uuid == uuid_str)`. The
`SemanticLayer.uuid`
column is defined as `Column(UUIDType(binary=True), primary_key=True,
default=uuid.uuid4)`
in `superset/semantic_layers/models.py:120`, so SQLAlchemy-Utils attempts to
coerce the
string `"not-a-uuid"` into a UUID object during parameter binding and raises
`ValueError`
for the malformed value.
5. This `ValueError` is not caught in `find_by_uuid` or in
`SemanticLayerRestApi.get`
(which only checks `if not layer:` after the call), so the exception bubbles
out of the
view function wrapped by Flask-AppBuilder's `@safe` decorator
(`superset/semantic_layers/api.py:167-171, 457-461`). In practice this
results in the
request failing with an unhandled server-side error (500) instead of
returning a clean
404/400 response for the invalid UUID path parameter.
6. The same unhandled `ValueError` behavior occurs for `POST
/api/v1/semantic_layer/<uuid>/schema/runtime`, implemented by
`SemanticLayerRestApi.runtime_schema` in
`superset/semantic_layers/api.py:234-282`, which
also calls `SemanticLayerDAO.find_by_uuid(uuid)` at line 264 without
guarding against
malformed UUID strings.
```
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/daos/semantic_layer.py
**Line:** 35:41
**Comment:**
*Logic Error: The `find_by_uuid` helper passes the raw `uuid_str`
directly into a `UUIDType(binary=True)` filter; if a client calls any semantic
layer endpoint with a malformed UUID (e.g.
`/api/v1/semantic_layer/not-a-uuid`), SQLAlchemy-Utils will attempt to coerce
this string to a UUID and raise `ValueError`, causing a 500 instead of a clean
404/400 response.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38370&comment_hash=7b391236d4610c78e0cb0d7d26b2b57ca3a34fe75548871058b38f68efeeb0ea&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38370&comment_hash=7b391236d4610c78e0cb0d7d26b2b57ca3a34fe75548871058b38f68efeeb0ea&reaction=dislike'>👎</a>
##########
tests/unit_tests/semantic_layers/api_test.py:
##########
@@ -259,3 +265,624 @@ def test_put_semantic_view_empty_payload(
)
assert response.status_code == 200
+
+
+# =============================================================================
+# SemanticLayerRestApi tests
+# =============================================================================
+
+
+def test_get_types(
+ client: Any,
+ full_api_access: None,
+ mocker: MockerFixture,
+) -> None:
+ """Test GET /types returns registered semantic layer types."""
+ mock_cls = MagicMock()
+ mock_cls.name = "Snowflake Semantic Layer"
+ mock_cls.description = "Connect to Snowflake."
+
+ mocker.patch.dict(
+ "superset.semantic_layers.api.registry",
+ {"snowflake": mock_cls},
+ clear=True,
+ )
+
+ response = client.get("/api/v1/semantic_layer/types")
+
+ assert response.status_code == 200
+ result = response.json["result"]
+ assert len(result) == 1
+ assert result[0] == {
+ "id": "snowflake",
+ "name": "Snowflake Semantic Layer",
+ "description": "Connect to Snowflake.",
+ }
+
+
+def test_get_types_empty(
+ client: Any,
+ full_api_access: None,
+ mocker: MockerFixture,
+) -> None:
+ """Test GET /types returns empty list when no types registered."""
+ mocker.patch.dict(
+ "superset.semantic_layers.api.registry",
+ {},
+ clear=True,
+ )
+
+ response = client.get("/api/v1/semantic_layer/types")
+
+ assert response.status_code == 200
+ assert response.json["result"] == []
+
+
+def test_configuration_schema(
+ client: Any,
+ full_api_access: None,
+ mocker: MockerFixture,
+) -> None:
+ """Test POST /schema/configuration returns schema without partial
config."""
+ mock_cls = MagicMock()
+ mock_cls.get_configuration_schema.return_value = {"type": "object"}
+
+ mocker.patch.dict(
+ "superset.semantic_layers.api.registry",
+ {"snowflake": mock_cls},
+ clear=True,
+ )
+
+ response = client.post(
+ "/api/v1/semantic_layer/schema/configuration",
+ json={"type": "snowflake"},
+ )
+
+ assert response.status_code == 200
+ assert response.json["result"] == {"type": "object"}
+ mock_cls.get_configuration_schema.assert_called_once_with(None)
+
+
+def test_configuration_schema_with_partial_config(
+ client: Any,
+ full_api_access: None,
+ mocker: MockerFixture,
+) -> None:
+ """Test POST /schema/configuration enriches schema with partial config."""
+ mock_instance = MagicMock()
+ mock_instance.configuration = {"account": "test"}
+
+ mock_cls = MagicMock()
+ mock_cls.from_configuration.return_value = mock_instance
+ mock_cls.get_configuration_schema.return_value = {
+ "type": "object",
+ "properties": {"database": {"enum": ["db1", "db2"]}},
+ }
+
+ mocker.patch.dict(
+ "superset.semantic_layers.api.registry",
+ {"snowflake": mock_cls},
+ clear=True,
+ )
+
+ response = client.post(
+ "/api/v1/semantic_layer/schema/configuration",
+ json={"type": "snowflake", "configuration": {"account": "test"}},
+ )
+
+ assert response.status_code == 200
+ mock_cls.get_configuration_schema.assert_called_once_with({"account":
"test"})
+
+
+def test_configuration_schema_with_invalid_partial_config(
+ client: Any,
+ full_api_access: None,
+ mocker: MockerFixture,
+) -> None:
+ """Test /schema/configuration returns schema when partial config fails."""
+ mock_cls = MagicMock()
+ mock_cls.from_configuration.side_effect = ValueError("bad config")
+ mock_cls.get_configuration_schema.return_value = {"type": "object"}
+
+ mocker.patch.dict(
+ "superset.semantic_layers.api.registry",
+ {"snowflake": mock_cls},
+ clear=True,
+ )
+
+ response = client.post(
+ "/api/v1/semantic_layer/schema/configuration",
+ json={"type": "snowflake", "configuration": {"bad": "data"}},
+ )
+
+ assert response.status_code == 200
+ mock_cls.get_configuration_schema.assert_called_once_with(None)
+
+
+def test_configuration_schema_unknown_type(
+ client: Any,
+ full_api_access: None,
+ mocker: MockerFixture,
+) -> None:
+ """Test POST /schema/configuration returns 400 for unknown type."""
+ mocker.patch.dict(
+ "superset.semantic_layers.api.registry",
+ {},
+ clear=True,
+ )
+
+ response = client.post(
+ "/api/v1/semantic_layer/schema/configuration",
+ json={"type": "nonexistent"},
+ )
+
+ assert response.status_code == 400
+
+
+def test_runtime_schema(
+ client: Any,
+ full_api_access: None,
+ mocker: MockerFixture,
+) -> None:
+ """Test POST /<uuid>/schema/runtime returns runtime schema."""
+ test_uuid = str(uuid_lib.uuid4())
+ mock_layer = MagicMock()
+ mock_layer.type = "snowflake"
+ mock_layer.implementation.configuration = {"account": "test"}
+
+ mock_dao = mocker.patch("superset.semantic_layers.api.SemanticLayerDAO")
+ mock_dao.find_by_uuid.return_value = mock_layer
+
+ mock_cls = MagicMock()
+ mock_cls.get_runtime_schema.return_value = {"type": "object"}
+
+ mocker.patch.dict(
+ "superset.semantic_layers.api.registry",
+ {"snowflake": mock_cls},
+ clear=True,
+ )
+
+ response = client.post(
+ f"/api/v1/semantic_layer/{test_uuid}/schema/runtime",
+ json={"runtime_data": {"database": "mydb"}},
+ )
+
+ assert response.status_code == 200
+ assert response.json["result"] == {"type": "object"}
+ mock_cls.get_runtime_schema.assert_called_once_with(
+ {"account": "test"}, {"database": "mydb"}
+ )
+
+
+def test_runtime_schema_no_body(
+ client: Any,
+ full_api_access: None,
+ mocker: MockerFixture,
+) -> None:
+ """Test POST /<uuid>/schema/runtime works without a request body."""
+ test_uuid = str(uuid_lib.uuid4())
+ mock_layer = MagicMock()
+ mock_layer.type = "snowflake"
+ mock_layer.implementation.configuration = {"account": "test"}
+
+ mock_dao = mocker.patch("superset.semantic_layers.api.SemanticLayerDAO")
+ mock_dao.find_by_uuid.return_value = mock_layer
+
+ mock_cls = MagicMock()
+ mock_cls.get_runtime_schema.return_value = {"type": "object"}
+
+ mocker.patch.dict(
+ "superset.semantic_layers.api.registry",
+ {"snowflake": mock_cls},
+ clear=True,
+ )
+
+ response = client.post(
+ f"/api/v1/semantic_layer/{test_uuid}/schema/runtime",
+ )
+
+ assert response.status_code == 200
+ mock_cls.get_runtime_schema.assert_called_once_with({"account": "test"},
None)
+
+
+def test_runtime_schema_not_found(
+ client: Any,
+ full_api_access: None,
+ mocker: MockerFixture,
+) -> None:
+ """Test POST /<uuid>/schema/runtime returns 404 when layer not found."""
+ mock_dao = mocker.patch("superset.semantic_layers.api.SemanticLayerDAO")
+ mock_dao.find_by_uuid.return_value = None
+
+ response = client.post(
+ f"/api/v1/semantic_layer/{uuid_lib.uuid4()}/schema/runtime",
+ )
+
+ assert response.status_code == 404
+
+
+@SEMANTIC_LAYERS_APP
+def test_runtime_schema_unknown_type(
+ client: Any,
+ full_api_access: None,
+ mocker: MockerFixture,
+) -> None:
+ """Test POST /<uuid>/schema/runtime returns 400 for unknown type."""
+ test_uuid = str(uuid_lib.uuid4())
+ mock_layer = MagicMock()
+ mock_layer.type = "unknown_type"
+
+ mock_dao = mocker.patch("superset.semantic_layers.api.SemanticLayerDAO")
+ mock_dao.find_by_uuid.return_value = mock_layer
+
+ mocker.patch.dict(
+ "superset.semantic_layers.api.registry",
+ {},
+ clear=True,
+ )
+
+ response = client.post(
+ f"/api/v1/semantic_layer/{test_uuid}/schema/runtime",
+ )
+
+ assert response.status_code == 400
+ assert "Unknown type" in response.json["message"]
+
+
+@SEMANTIC_LAYERS_APP
+def test_runtime_schema_exception(
+ client: Any,
+ full_api_access: None,
+ mocker: MockerFixture,
+) -> None:
+ """Test POST /<uuid>/schema/runtime returns 400 when schema raises."""
+ test_uuid = str(uuid_lib.uuid4())
+ mock_layer = MagicMock()
+ mock_layer.type = "snowflake"
+ mock_layer.implementation.configuration = {"account": "test"}
+
+ mock_dao = mocker.patch("superset.semantic_layers.api.SemanticLayerDAO")
+ mock_dao.find_by_uuid.return_value = mock_layer
+
+ mock_cls = MagicMock()
+ mock_cls.get_runtime_schema.side_effect = ValueError("Bad config")
+
+ mocker.patch.dict(
+ "superset.semantic_layers.api.registry",
+ {"snowflake": mock_cls},
+ clear=True,
+ )
+
+ response = client.post(
+ f"/api/v1/semantic_layer/{test_uuid}/schema/runtime",
+ )
+
+ assert response.status_code == 400
+ assert "Bad config" in response.json["message"]
+
+
+def test_post_semantic_layer(
+ client: Any,
+ full_api_access: None,
+ mocker: MockerFixture,
+) -> None:
+ """Test POST / creates a semantic layer."""
+ test_uuid = uuid_lib.uuid4()
+ new_model = MagicMock()
+ new_model.uuid = test_uuid
+
+ mock_command = mocker.patch(
+ "superset.semantic_layers.api.CreateSemanticLayerCommand",
+ )
+ mock_command.return_value.run.return_value = new_model
+
+ payload = {
+ "name": "My Layer",
+ "type": "snowflake",
+ "configuration": {"account": "test"},
+ }
+ response = client.post("/api/v1/semantic_layer/", json=payload)
+
+ assert response.status_code == 201
+ assert response.json["result"]["uuid"] == str(test_uuid)
+ mock_command.assert_called_once_with(payload)
+
+
+def test_post_semantic_layer_invalid(
+ client: Any,
+ full_api_access: None,
+ mocker: MockerFixture,
+) -> None:
+ """Test POST / returns 422 when validation fails."""
+ mock_command = mocker.patch(
+ "superset.semantic_layers.api.CreateSemanticLayerCommand",
+ )
+ mock_command.return_value.run.side_effect = SemanticLayerInvalidError(
+ "Unknown type: bad"
+ )
+
+ payload = {
+ "name": "My Layer",
+ "type": "bad",
+ "configuration": {},
+ }
+ response = client.post("/api/v1/semantic_layer/", json=payload)
+
+ assert response.status_code == 422
+
+
+def test_post_semantic_layer_create_failed(
+ client: Any,
+ full_api_access: None,
+ mocker: MockerFixture,
+) -> None:
+ """Test POST / returns 422 when creation fails."""
+ mock_command = mocker.patch(
+ "superset.semantic_layers.api.CreateSemanticLayerCommand",
+ )
+ mock_command.return_value.run.side_effect =
SemanticLayerCreateFailedError()
+
+ payload = {
+ "name": "My Layer",
+ "type": "snowflake",
+ "configuration": {"account": "test"},
+ }
+ response = client.post("/api/v1/semantic_layer/", json=payload)
+
+ assert response.status_code == 422
+
+
+def test_post_semantic_layer_missing_required_fields(
+ client: Any,
+ full_api_access: None,
+ mocker: MockerFixture,
+) -> None:
+ """Test POST / returns 400 when required fields are missing."""
+ mocker.patch(
+ "superset.semantic_layers.api.CreateSemanticLayerCommand",
+ )
+
+ response = client.post(
+ "/api/v1/semantic_layer/",
+ json={"name": "Only name"},
+ )
+
+ assert response.status_code == 400
+
+
+def test_put_semantic_layer(
+ client: Any,
+ full_api_access: None,
+ mocker: MockerFixture,
+) -> None:
+ """Test PUT /<uuid> updates a semantic layer."""
+ test_uuid = uuid_lib.uuid4()
+ changed_model = MagicMock()
+ changed_model.uuid = test_uuid
+
+ mock_command = mocker.patch(
+ "superset.semantic_layers.api.UpdateSemanticLayerCommand",
+ )
+ mock_command.return_value.run.return_value = changed_model
+
+ payload = {"name": "Updated Name"}
+ response = client.put(
+ f"/api/v1/semantic_layer/{test_uuid}",
+ json=payload,
+ )
+
+ assert response.status_code == 200
+ assert response.json["result"]["uuid"] == str(test_uuid)
+ mock_command.assert_called_once_with(str(test_uuid), payload)
+
+
+def test_put_semantic_layer_not_found(
+ client: Any,
+ full_api_access: None,
+ mocker: MockerFixture,
+) -> None:
+ """Test PUT /<uuid> returns 404 when layer not found."""
+ mock_command = mocker.patch(
+ "superset.semantic_layers.api.UpdateSemanticLayerCommand",
+ )
+ mock_command.return_value.run.side_effect = SemanticLayerNotFoundError()
+
+ response = client.put(
+ f"/api/v1/semantic_layer/{uuid_lib.uuid4()}",
+ json={"name": "New"},
+ )
+
+ assert response.status_code == 404
+
+
+def test_put_semantic_layer_invalid(
+ client: Any,
+ full_api_access: None,
+ mocker: MockerFixture,
+) -> None:
+ """Test PUT /<uuid> returns 422 when validation fails."""
+ mock_command = mocker.patch(
+ "superset.semantic_layers.api.UpdateSemanticLayerCommand",
+ )
+ mock_command.return_value.run.side_effect = SemanticLayerInvalidError(
+ "Name already exists"
+ )
+
+ response = client.put(
+ f"/api/v1/semantic_layer/{uuid_lib.uuid4()}",
+ json={"name": "Duplicate"},
+ )
+
+ assert response.status_code == 422
+
+
Review Comment:
**Suggestion:** The semantic layer create and update tests call
`/api/v1/semantic_layer/` endpoints that are only registered when the
`SEMANTIC_LAYERS` feature flag is enabled, but these tests lack the
`SEMANTIC_LAYERS_APP` parametrization used elsewhere, so the endpoints may be
missing and the tests will fail with 404. [possible bug]
<details>
<summary><b>Severity Level:</b> Critical 🚨</summary>
```mdx
- ❌ Semantic layer create tests can fail with 404 responses.
- ❌ Update tests may fail when SEMANTIC_LAYERS is disabled.
- ⚠️ Create/update behavior for semantic layers not reliably tested.
```
</details>
```suggestion
@SEMANTIC_LAYERS_APP
def test_post_semantic_layer(
client: Any,
full_api_access: None,
mocker: MockerFixture,
) -> None:
"""Test POST / creates a semantic layer."""
test_uuid = uuid_lib.uuid4()
new_model = MagicMock()
new_model.uuid = test_uuid
mock_command = mocker.patch(
"superset.semantic_layers.api.CreateSemanticLayerCommand",
)
mock_command.return_value.run.return_value = new_model
payload = {
"name": "My Layer",
"type": "snowflake",
"configuration": {"account": "test"},
}
response = client.post("/api/v1/semantic_layer/", json=payload)
assert response.status_code == 201
assert response.json["result"]["uuid"] == str(test_uuid)
mock_command.assert_called_once_with(payload)
@SEMANTIC_LAYERS_APP
def test_post_semantic_layer_invalid(
client: Any,
full_api_access: None,
mocker: MockerFixture,
) -> None:
"""Test POST / returns 422 when validation fails."""
mock_command = mocker.patch(
"superset.semantic_layers.api.CreateSemanticLayerCommand",
)
mock_command.return_value.run.side_effect = SemanticLayerInvalidError(
"Unknown type: bad"
)
payload = {
"name": "My Layer",
"type": "bad",
"configuration": {},
}
response = client.post("/api/v1/semantic_layer/", json=payload)
assert response.status_code == 422
@SEMANTIC_LAYERS_APP
def test_post_semantic_layer_create_failed(
client: Any,
full_api_access: None,
mocker: MockerFixture,
) -> None:
"""Test POST / returns 422 when creation fails."""
mock_command = mocker.patch(
"superset.semantic_layers.api.CreateSemanticLayerCommand",
)
mock_command.return_value.run.side_effect =
SemanticLayerCreateFailedError()
payload = {
"name": "My Layer",
"type": "snowflake",
"configuration": {"account": "test"},
}
response = client.post("/api/v1/semantic_layer/", json=payload)
assert response.status_code == 422
@SEMANTIC_LAYERS_APP
def test_post_semantic_layer_missing_required_fields(
client: Any,
full_api_access: None,
mocker: MockerFixture,
) -> None:
"""Test POST / returns 400 when required fields are missing."""
mocker.patch(
"superset.semantic_layers.api.CreateSemanticLayerCommand",
)
response = client.post(
"/api/v1/semantic_layer/",
json={"name": "Only name"},
)
assert response.status_code == 400
@SEMANTIC_LAYERS_APP
def test_put_semantic_layer(
client: Any,
full_api_access: None,
mocker: MockerFixture,
) -> None:
"""Test PUT /<uuid> updates a semantic layer."""
test_uuid = uuid_lib.uuid4()
changed_model = MagicMock()
changed_model.uuid = test_uuid
mock_command = mocker.patch(
"superset.semantic_layers.api.UpdateSemanticLayerCommand",
)
mock_command.return_value.run.return_value = changed_model
payload = {"name": "Updated Name"}
response = client.put(
f"/api/v1/semantic_layer/{test_uuid}",
json=payload,
)
assert response.status_code == 200
assert response.json["result"]["uuid"] == str(test_uuid)
mock_command.assert_called_once_with(str(test_uuid), payload)
@SEMANTIC_LAYERS_APP
def test_put_semantic_layer_not_found(
client: Any,
full_api_access: None,
mocker: MockerFixture,
) -> None:
"""Test PUT /<uuid> returns 404 when layer not found."""
mock_command = mocker.patch(
"superset.semantic_layers.api.UpdateSemanticLayerCommand",
)
mock_command.return_value.run.side_effect = SemanticLayerNotFoundError()
response = client.put(
f"/api/v1/semantic_layer/{uuid_lib.uuid4()}",
json={"name": "New"},
)
assert response.status_code == 404
@SEMANTIC_LAYERS_APP
def test_put_semantic_layer_invalid(
client: Any,
full_api_access: None,
mocker: MockerFixture,
) -> None:
"""Test PUT /<uuid> returns 422 when validation fails."""
mock_command = mocker.patch(
"superset.semantic_layers.api.UpdateSemanticLayerCommand",
)
mock_command.return_value.run.side_effect = SemanticLayerInvalidError(
"Name already exists"
)
response = client.put(
f"/api/v1/semantic_layer/{uuid_lib.uuid4()}",
json={"name": "Duplicate"},
)
assert response.status_code == 422
@SEMANTIC_LAYERS_APP
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. When unit tests run, the `app` fixture in
`tests/unit_tests/conftest.py:76-125`
initializes Superset and calls `SupersetAppInitializer.init_views()`
(`superset/initialization/__init__.py:145-227`), which registers
`SemanticLayerRestApi`
only if `feature_flag_manager.is_feature_enabled("SEMANTIC_LAYERS")` is true
(lines
269-276).
2. Tests `test_post_semantic_layer*` and `test_put_semantic_layer*` in
`tests/unit_tests/semantic_layers/api_test.py:564-735` use the plain
`client` and
`full_api_access` fixtures but do not apply the `@SEMANTIC_LAYERS_APP`
parametrization
declared at lines 37-41, so the app is created with default feature flags
instead of
forcing `SEMANTIC_LAYERS` to `True`.
3. If `SEMANTIC_LAYERS` is disabled by default (as implied by it being gated
in
`init_views`), then the `/api/v1/semantic_layer/` POST and
`/api/v1/semantic_layer/<uuid>`
PUT routes are not added to the app, and calls like
`client.post("/api/v1/semantic_layer/", json=payload)` (line 584) or
`client.put(f"/api/v1/semantic_layer/{test_uuid}", json=payload)` (lines
668-672) return
404 instead of the expected 201/200/422/404 codes asserted in the tests.
4. As a result, these create/update tests fail (e.g. `assert
response.status_code == 201`
at line 586, `assert response.status_code == 200` at line 674, and status
422/404
assertions at lines 611, 632, 650, 695, 716, 735) when the semantic layers
feature flag is
not explicitly enabled in the test app, even though the API implementation in
`superset/semantic_layers/api.py:284-399` is correct.
```
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** tests/unit_tests/semantic_layers/api_test.py
**Line:** 564:718
**Comment:**
*Possible Bug: The semantic layer create and update tests call
`/api/v1/semantic_layer/` endpoints that are only registered when the
`SEMANTIC_LAYERS` feature flag is enabled, but these tests lack the
`SEMANTIC_LAYERS_APP` parametrization used elsewhere, so the endpoints may be
missing and the tests will fail with 404.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38370&comment_hash=42369c0a21403834de22bc157e3243b196bdbae4da236b102152e61232732d84&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38370&comment_hash=42369c0a21403834de22bc157e3243b196bdbae4da236b102152e61232732d84&reaction=dislike'>👎</a>
##########
tests/unit_tests/semantic_layers/api_test.py:
##########
@@ -259,3 +265,624 @@ def test_put_semantic_view_empty_payload(
)
assert response.status_code == 200
+
+
+# =============================================================================
+# SemanticLayerRestApi tests
+# =============================================================================
+
+
+def test_get_types(
+ client: Any,
+ full_api_access: None,
+ mocker: MockerFixture,
+) -> None:
+ """Test GET /types returns registered semantic layer types."""
+ mock_cls = MagicMock()
+ mock_cls.name = "Snowflake Semantic Layer"
+ mock_cls.description = "Connect to Snowflake."
+
+ mocker.patch.dict(
+ "superset.semantic_layers.api.registry",
+ {"snowflake": mock_cls},
+ clear=True,
+ )
+
+ response = client.get("/api/v1/semantic_layer/types")
+
+ assert response.status_code == 200
+ result = response.json["result"]
+ assert len(result) == 1
+ assert result[0] == {
+ "id": "snowflake",
+ "name": "Snowflake Semantic Layer",
+ "description": "Connect to Snowflake.",
+ }
+
+
+def test_get_types_empty(
+ client: Any,
+ full_api_access: None,
+ mocker: MockerFixture,
+) -> None:
+ """Test GET /types returns empty list when no types registered."""
+ mocker.patch.dict(
+ "superset.semantic_layers.api.registry",
+ {},
+ clear=True,
+ )
+
+ response = client.get("/api/v1/semantic_layer/types")
+
+ assert response.status_code == 200
+ assert response.json["result"] == []
+
+
+def test_configuration_schema(
+ client: Any,
+ full_api_access: None,
+ mocker: MockerFixture,
+) -> None:
+ """Test POST /schema/configuration returns schema without partial
config."""
+ mock_cls = MagicMock()
+ mock_cls.get_configuration_schema.return_value = {"type": "object"}
+
+ mocker.patch.dict(
+ "superset.semantic_layers.api.registry",
+ {"snowflake": mock_cls},
+ clear=True,
+ )
+
+ response = client.post(
+ "/api/v1/semantic_layer/schema/configuration",
+ json={"type": "snowflake"},
+ )
+
+ assert response.status_code == 200
+ assert response.json["result"] == {"type": "object"}
+ mock_cls.get_configuration_schema.assert_called_once_with(None)
+
+
+def test_configuration_schema_with_partial_config(
+ client: Any,
+ full_api_access: None,
+ mocker: MockerFixture,
+) -> None:
+ """Test POST /schema/configuration enriches schema with partial config."""
+ mock_instance = MagicMock()
+ mock_instance.configuration = {"account": "test"}
+
+ mock_cls = MagicMock()
+ mock_cls.from_configuration.return_value = mock_instance
+ mock_cls.get_configuration_schema.return_value = {
+ "type": "object",
+ "properties": {"database": {"enum": ["db1", "db2"]}},
+ }
+
+ mocker.patch.dict(
+ "superset.semantic_layers.api.registry",
+ {"snowflake": mock_cls},
+ clear=True,
+ )
+
+ response = client.post(
+ "/api/v1/semantic_layer/schema/configuration",
+ json={"type": "snowflake", "configuration": {"account": "test"}},
+ )
+
+ assert response.status_code == 200
+ mock_cls.get_configuration_schema.assert_called_once_with({"account":
"test"})
+
+
+def test_configuration_schema_with_invalid_partial_config(
+ client: Any,
+ full_api_access: None,
+ mocker: MockerFixture,
+) -> None:
+ """Test /schema/configuration returns schema when partial config fails."""
+ mock_cls = MagicMock()
+ mock_cls.from_configuration.side_effect = ValueError("bad config")
+ mock_cls.get_configuration_schema.return_value = {"type": "object"}
+
+ mocker.patch.dict(
+ "superset.semantic_layers.api.registry",
+ {"snowflake": mock_cls},
+ clear=True,
+ )
+
+ response = client.post(
+ "/api/v1/semantic_layer/schema/configuration",
+ json={"type": "snowflake", "configuration": {"bad": "data"}},
+ )
+
+ assert response.status_code == 200
+ mock_cls.get_configuration_schema.assert_called_once_with(None)
+
+
+def test_configuration_schema_unknown_type(
+ client: Any,
+ full_api_access: None,
+ mocker: MockerFixture,
+) -> None:
+ """Test POST /schema/configuration returns 400 for unknown type."""
+ mocker.patch.dict(
+ "superset.semantic_layers.api.registry",
+ {},
+ clear=True,
+ )
+
+ response = client.post(
+ "/api/v1/semantic_layer/schema/configuration",
+ json={"type": "nonexistent"},
+ )
+
+ assert response.status_code == 400
+
+
+def test_runtime_schema(
+ client: Any,
+ full_api_access: None,
+ mocker: MockerFixture,
+) -> None:
+ """Test POST /<uuid>/schema/runtime returns runtime schema."""
+ test_uuid = str(uuid_lib.uuid4())
+ mock_layer = MagicMock()
+ mock_layer.type = "snowflake"
+ mock_layer.implementation.configuration = {"account": "test"}
+
+ mock_dao = mocker.patch("superset.semantic_layers.api.SemanticLayerDAO")
+ mock_dao.find_by_uuid.return_value = mock_layer
+
+ mock_cls = MagicMock()
+ mock_cls.get_runtime_schema.return_value = {"type": "object"}
+
+ mocker.patch.dict(
+ "superset.semantic_layers.api.registry",
+ {"snowflake": mock_cls},
+ clear=True,
+ )
+
+ response = client.post(
+ f"/api/v1/semantic_layer/{test_uuid}/schema/runtime",
+ json={"runtime_data": {"database": "mydb"}},
+ )
+
+ assert response.status_code == 200
+ assert response.json["result"] == {"type": "object"}
+ mock_cls.get_runtime_schema.assert_called_once_with(
+ {"account": "test"}, {"database": "mydb"}
+ )
+
+
+def test_runtime_schema_no_body(
+ client: Any,
+ full_api_access: None,
+ mocker: MockerFixture,
+) -> None:
+ """Test POST /<uuid>/schema/runtime works without a request body."""
+ test_uuid = str(uuid_lib.uuid4())
+ mock_layer = MagicMock()
+ mock_layer.type = "snowflake"
+ mock_layer.implementation.configuration = {"account": "test"}
+
+ mock_dao = mocker.patch("superset.semantic_layers.api.SemanticLayerDAO")
+ mock_dao.find_by_uuid.return_value = mock_layer
+
+ mock_cls = MagicMock()
+ mock_cls.get_runtime_schema.return_value = {"type": "object"}
+
+ mocker.patch.dict(
+ "superset.semantic_layers.api.registry",
+ {"snowflake": mock_cls},
+ clear=True,
+ )
+
+ response = client.post(
+ f"/api/v1/semantic_layer/{test_uuid}/schema/runtime",
+ )
+
+ assert response.status_code == 200
+ mock_cls.get_runtime_schema.assert_called_once_with({"account": "test"},
None)
+
+
Review Comment:
**Suggestion:** These semantic layer configuration and runtime schema tests
call endpoints that are only registered when the `SEMANTIC_LAYERS` feature flag
is enabled, but unlike neighboring tests they don't apply the
`SEMANTIC_LAYERS_APP` parametrization, so the routes may not exist and the
tests will fail with 404. [possible bug]
<details>
<summary><b>Severity Level:</b> Critical 🚨</summary>
```mdx
- ❌ Semantic layer type listing tests fail with 404 responses.
- ❌ Configuration schema tests fail; routes not registered without flag.
- ❌ Runtime schema tests fail when SEMANTIC_LAYERS disabled by default.
- ⚠️ Semantic layer type/schema behavior not validated in CI.
```
</details>
```suggestion
@SEMANTIC_LAYERS_APP
def test_get_types(
client: Any,
full_api_access: None,
mocker: MockerFixture,
) -> None:
"""Test GET /types returns registered semantic layer types."""
mock_cls = MagicMock()
mock_cls.name = "Snowflake Semantic Layer"
mock_cls.description = "Connect to Snowflake."
mocker.patch.dict(
"superset.semantic_layers.api.registry",
{"snowflake": mock_cls},
clear=True,
)
response = client.get("/api/v1/semantic_layer/types")
assert response.status_code == 200
result = response.json["result"]
assert len(result) == 1
assert result[0] == {
"id": "snowflake",
"name": "Snowflake Semantic Layer",
"description": "Connect to Snowflake.",
}
@SEMANTIC_LAYERS_APP
def test_get_types_empty(
client: Any,
full_api_access: None,
mocker: MockerFixture,
) -> None:
"""Test GET /types returns empty list when no types registered."""
mocker.patch.dict(
"superset.semantic_layers.api.registry",
{},
clear=True,
)
response = client.get("/api/v1/semantic_layer/types")
assert response.status_code == 200
assert response.json["result"] == []
@SEMANTIC_LAYERS_APP
def test_configuration_schema(
client: Any,
full_api_access: None,
mocker: MockerFixture,
) -> None:
"""Test POST /schema/configuration returns schema without partial
config."""
mock_cls = MagicMock()
mock_cls.get_configuration_schema.return_value = {"type": "object"}
mocker.patch.dict(
"superset.semantic_layers.api.registry",
{"snowflake": mock_cls},
clear=True,
)
response = client.post(
"/api/v1/semantic_layer/schema/configuration",
json={"type": "snowflake"},
)
assert response.status_code == 200
assert response.json["result"] == {"type": "object"}
mock_cls.get_configuration_schema.assert_called_once_with(None)
@SEMANTIC_LAYERS_APP
def test_configuration_schema_with_partial_config(
client: Any,
full_api_access: None,
mocker: MockerFixture,
) -> None:
"""Test POST /schema/configuration enriches schema with partial
config."""
mock_instance = MagicMock()
mock_instance.configuration = {"account": "test"}
mock_cls = MagicMock()
mock_cls.from_configuration.return_value = mock_instance
mock_cls.get_configuration_schema.return_value = {
"type": "object",
"properties": {"database": {"enum": ["db1", "db2"]}},
}
mocker.patch.dict(
"superset.semantic_layers.api.registry",
{"snowflake": mock_cls},
clear=True,
)
response = client.post(
"/api/v1/semantic_layer/schema/configuration",
json={"type": "snowflake", "configuration": {"account": "test"}},
)
assert response.status_code == 200
mock_cls.get_configuration_schema.assert_called_once_with({"account":
"test"})
@SEMANTIC_LAYERS_APP
def test_configuration_schema_with_invalid_partial_config(
client: Any,
full_api_access: None,
mocker: MockerFixture,
) -> None:
"""Test /schema/configuration returns schema when partial config
fails."""
mock_cls = MagicMock()
mock_cls.from_configuration.side_effect = ValueError("bad config")
mock_cls.get_configuration_schema.return_value = {"type": "object"}
mocker.patch.dict(
"superset.semantic_layers.api.registry",
{"snowflake": mock_cls},
clear=True,
)
response = client.post(
"/api/v1/semantic_layer/schema/configuration",
json={"type": "snowflake", "configuration": {"bad": "data"}},
)
assert response.status_code == 200
mock_cls.get_configuration_schema.assert_called_once_with(None)
@SEMANTIC_LAYERS_APP
def test_configuration_schema_unknown_type(
client: Any,
full_api_access: None,
mocker: MockerFixture,
) -> None:
"""Test POST /schema/configuration returns 400 for unknown type."""
mocker.patch.dict(
"superset.semantic_layers.api.registry",
{},
clear=True,
)
response = client.post(
"/api/v1/semantic_layer/schema/configuration",
json={"type": "nonexistent"},
)
assert response.status_code == 400
@SEMANTIC_LAYERS_APP
def test_runtime_schema(
client: Any,
full_api_access: None,
mocker: MockerFixture,
) -> None:
"""Test POST /<uuid>/schema/runtime returns runtime schema."""
test_uuid = str(uuid_lib.uuid4())
mock_layer = MagicMock()
mock_layer.type = "snowflake"
mock_layer.implementation.configuration = {"account": "test"}
mock_dao = mocker.patch("superset.semantic_layers.api.SemanticLayerDAO")
mock_dao.find_by_uuid.return_value = mock_layer
mock_cls = MagicMock()
mock_cls.get_runtime_schema.return_value = {"type": "object"}
mocker.patch.dict(
"superset.semantic_layers.api.registry",
{"snowflake": mock_cls},
clear=True,
)
response = client.post(
f"/api/v1/semantic_layer/{test_uuid}/schema/runtime",
json={"runtime_data": {"database": "mydb"}},
)
assert response.status_code == 200
assert response.json["result"] == {"type": "object"}
mock_cls.get_runtime_schema.assert_called_once_with(
{"account": "test"}, {"database": "mydb"}
)
@SEMANTIC_LAYERS_APP
def test_runtime_schema_no_body(
client: Any,
full_api_access: None,
mocker: MockerFixture,
) -> None:
"""Test POST /<uuid>/schema/runtime works without a request body."""
test_uuid = str(uuid_lib.uuid4())
mock_layer = MagicMock()
mock_layer.type = "snowflake"
mock_layer.implementation.configuration = {"account": "test"}
mock_dao = mocker.patch("superset.semantic_layers.api.SemanticLayerDAO")
mock_dao.find_by_uuid.return_value = mock_layer
mock_cls = MagicMock()
mock_cls.get_runtime_schema.return_value = {"type": "object"}
mocker.patch.dict(
"superset.semantic_layers.api.registry",
{"snowflake": mock_cls},
clear=True,
)
response = client.post(
f"/api/v1/semantic_layer/{test_uuid}/schema/runtime",
)
assert response.status_code == 200
mock_cls.get_runtime_schema.assert_called_once_with({"account": "test"},
None)
@SEMANTIC_LAYERS_APP
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. The `app` fixture in `tests/unit_tests/conftest.py:76-125` builds a
Superset app from
`superset.config` and initializes views via
`SupersetAppInitializer.init_app()`, which
ultimately calls `SupersetAppInitializer.init_views()` in
`superset/initialization/__init__.py:145-227`.
2. Inside `init_views`, semantic layer APIs (`SemanticLayerRestApi`,
`SemanticViewRestApi`) are only registered when
`feature_flag_manager.is_feature_enabled("SEMANTIC_LAYERS")` is true (see
`superset/initialization/__init__.py:269-276`), so when the
`SEMANTIC_LAYERS` feature flag
is false or unset the `/api/v1/semantic_layer/...` routes are not added.
3. The tests `test_get_types`, `test_get_types_empty`,
`test_configuration_schema*`, and
`test_runtime_schema*` in
`tests/unit_tests/semantic_layers/api_test.py:275-501` call
these endpoints using the plain `client` fixture and `full_api_access`
fixture but do NOT
apply the `@SEMANTIC_LAYERS_APP` parametrization defined at lines 37-41, so
the app is
created with default feature flags where `SEMANTIC_LAYERS` is not forced on.
4. Running these tests with the default test configuration yields 404
responses for routes
such as `/api/v1/semantic_layer/types` and
`/api/v1/semantic_layer/schema/configuration`,
causing assertions like `assert response.status_code == 200` at lines 293,
317, 341, 373,
398, 419, 450, 484, and `assert response.status_code == 404` at line 501 to
fail because
they expect successful semantic layer API behavior rather than a
missing-route 404.
```
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** tests/unit_tests/semantic_layers/api_test.py
**Line:** 275:487
**Comment:**
*Possible Bug: These semantic layer configuration and runtime schema
tests call endpoints that are only registered when the `SEMANTIC_LAYERS`
feature flag is enabled, but unlike neighboring tests they don't apply the
`SEMANTIC_LAYERS_APP` parametrization, so the routes may not exist and the
tests will fail with 404.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38370&comment_hash=51488e88a8de19b63fe6b752891cbc37abe69eef166acbb9f1148e3426ba0cba&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38370&comment_hash=51488e88a8de19b63fe6b752891cbc37abe69eef166acbb9f1148e3426ba0cba&reaction=dislike'>👎</a>
##########
tests/unit_tests/semantic_layers/api_test.py:
##########
@@ -259,3 +265,624 @@ def test_put_semantic_view_empty_payload(
)
assert response.status_code == 200
+
+
+# =============================================================================
+# SemanticLayerRestApi tests
+# =============================================================================
+
+
+def test_get_types(
+ client: Any,
+ full_api_access: None,
+ mocker: MockerFixture,
+) -> None:
+ """Test GET /types returns registered semantic layer types."""
+ mock_cls = MagicMock()
+ mock_cls.name = "Snowflake Semantic Layer"
+ mock_cls.description = "Connect to Snowflake."
+
+ mocker.patch.dict(
+ "superset.semantic_layers.api.registry",
+ {"snowflake": mock_cls},
+ clear=True,
+ )
+
+ response = client.get("/api/v1/semantic_layer/types")
+
+ assert response.status_code == 200
+ result = response.json["result"]
+ assert len(result) == 1
+ assert result[0] == {
+ "id": "snowflake",
+ "name": "Snowflake Semantic Layer",
+ "description": "Connect to Snowflake.",
+ }
+
+
+def test_get_types_empty(
+ client: Any,
+ full_api_access: None,
+ mocker: MockerFixture,
+) -> None:
+ """Test GET /types returns empty list when no types registered."""
+ mocker.patch.dict(
+ "superset.semantic_layers.api.registry",
+ {},
+ clear=True,
+ )
+
+ response = client.get("/api/v1/semantic_layer/types")
+
+ assert response.status_code == 200
+ assert response.json["result"] == []
+
+
+def test_configuration_schema(
+ client: Any,
+ full_api_access: None,
+ mocker: MockerFixture,
+) -> None:
+ """Test POST /schema/configuration returns schema without partial
config."""
+ mock_cls = MagicMock()
+ mock_cls.get_configuration_schema.return_value = {"type": "object"}
+
+ mocker.patch.dict(
+ "superset.semantic_layers.api.registry",
+ {"snowflake": mock_cls},
+ clear=True,
+ )
+
+ response = client.post(
+ "/api/v1/semantic_layer/schema/configuration",
+ json={"type": "snowflake"},
+ )
+
+ assert response.status_code == 200
+ assert response.json["result"] == {"type": "object"}
+ mock_cls.get_configuration_schema.assert_called_once_with(None)
+
+
+def test_configuration_schema_with_partial_config(
+ client: Any,
+ full_api_access: None,
+ mocker: MockerFixture,
+) -> None:
+ """Test POST /schema/configuration enriches schema with partial config."""
+ mock_instance = MagicMock()
+ mock_instance.configuration = {"account": "test"}
+
+ mock_cls = MagicMock()
+ mock_cls.from_configuration.return_value = mock_instance
+ mock_cls.get_configuration_schema.return_value = {
+ "type": "object",
+ "properties": {"database": {"enum": ["db1", "db2"]}},
+ }
+
+ mocker.patch.dict(
+ "superset.semantic_layers.api.registry",
+ {"snowflake": mock_cls},
+ clear=True,
+ )
+
+ response = client.post(
+ "/api/v1/semantic_layer/schema/configuration",
+ json={"type": "snowflake", "configuration": {"account": "test"}},
+ )
+
+ assert response.status_code == 200
+ mock_cls.get_configuration_schema.assert_called_once_with({"account":
"test"})
+
+
+def test_configuration_schema_with_invalid_partial_config(
+ client: Any,
+ full_api_access: None,
+ mocker: MockerFixture,
+) -> None:
+ """Test /schema/configuration returns schema when partial config fails."""
+ mock_cls = MagicMock()
+ mock_cls.from_configuration.side_effect = ValueError("bad config")
+ mock_cls.get_configuration_schema.return_value = {"type": "object"}
+
+ mocker.patch.dict(
+ "superset.semantic_layers.api.registry",
+ {"snowflake": mock_cls},
+ clear=True,
+ )
+
+ response = client.post(
+ "/api/v1/semantic_layer/schema/configuration",
+ json={"type": "snowflake", "configuration": {"bad": "data"}},
+ )
+
+ assert response.status_code == 200
+ mock_cls.get_configuration_schema.assert_called_once_with(None)
+
+
+def test_configuration_schema_unknown_type(
+ client: Any,
+ full_api_access: None,
+ mocker: MockerFixture,
+) -> None:
+ """Test POST /schema/configuration returns 400 for unknown type."""
+ mocker.patch.dict(
+ "superset.semantic_layers.api.registry",
+ {},
+ clear=True,
+ )
+
+ response = client.post(
+ "/api/v1/semantic_layer/schema/configuration",
+ json={"type": "nonexistent"},
+ )
+
+ assert response.status_code == 400
+
+
+def test_runtime_schema(
+ client: Any,
+ full_api_access: None,
+ mocker: MockerFixture,
+) -> None:
+ """Test POST /<uuid>/schema/runtime returns runtime schema."""
+ test_uuid = str(uuid_lib.uuid4())
+ mock_layer = MagicMock()
+ mock_layer.type = "snowflake"
+ mock_layer.implementation.configuration = {"account": "test"}
+
+ mock_dao = mocker.patch("superset.semantic_layers.api.SemanticLayerDAO")
+ mock_dao.find_by_uuid.return_value = mock_layer
+
+ mock_cls = MagicMock()
+ mock_cls.get_runtime_schema.return_value = {"type": "object"}
+
+ mocker.patch.dict(
+ "superset.semantic_layers.api.registry",
+ {"snowflake": mock_cls},
+ clear=True,
+ )
+
+ response = client.post(
+ f"/api/v1/semantic_layer/{test_uuid}/schema/runtime",
+ json={"runtime_data": {"database": "mydb"}},
+ )
+
+ assert response.status_code == 200
+ assert response.json["result"] == {"type": "object"}
+ mock_cls.get_runtime_schema.assert_called_once_with(
+ {"account": "test"}, {"database": "mydb"}
+ )
+
+
+def test_runtime_schema_no_body(
+ client: Any,
+ full_api_access: None,
+ mocker: MockerFixture,
+) -> None:
+ """Test POST /<uuid>/schema/runtime works without a request body."""
+ test_uuid = str(uuid_lib.uuid4())
+ mock_layer = MagicMock()
+ mock_layer.type = "snowflake"
+ mock_layer.implementation.configuration = {"account": "test"}
+
+ mock_dao = mocker.patch("superset.semantic_layers.api.SemanticLayerDAO")
+ mock_dao.find_by_uuid.return_value = mock_layer
+
+ mock_cls = MagicMock()
+ mock_cls.get_runtime_schema.return_value = {"type": "object"}
+
+ mocker.patch.dict(
+ "superset.semantic_layers.api.registry",
+ {"snowflake": mock_cls},
+ clear=True,
+ )
+
+ response = client.post(
+ f"/api/v1/semantic_layer/{test_uuid}/schema/runtime",
+ )
+
+ assert response.status_code == 200
+ mock_cls.get_runtime_schema.assert_called_once_with({"account": "test"},
None)
+
+
+def test_runtime_schema_not_found(
+ client: Any,
+ full_api_access: None,
+ mocker: MockerFixture,
+) -> None:
+ """Test POST /<uuid>/schema/runtime returns 404 when layer not found."""
+ mock_dao = mocker.patch("superset.semantic_layers.api.SemanticLayerDAO")
+ mock_dao.find_by_uuid.return_value = None
+
+ response = client.post(
+ f"/api/v1/semantic_layer/{uuid_lib.uuid4()}/schema/runtime",
+ )
+
+ assert response.status_code == 404
+
+
+@SEMANTIC_LAYERS_APP
+def test_runtime_schema_unknown_type(
+ client: Any,
+ full_api_access: None,
+ mocker: MockerFixture,
+) -> None:
+ """Test POST /<uuid>/schema/runtime returns 400 for unknown type."""
+ test_uuid = str(uuid_lib.uuid4())
+ mock_layer = MagicMock()
+ mock_layer.type = "unknown_type"
+
+ mock_dao = mocker.patch("superset.semantic_layers.api.SemanticLayerDAO")
+ mock_dao.find_by_uuid.return_value = mock_layer
+
+ mocker.patch.dict(
+ "superset.semantic_layers.api.registry",
+ {},
+ clear=True,
+ )
+
+ response = client.post(
+ f"/api/v1/semantic_layer/{test_uuid}/schema/runtime",
+ )
+
+ assert response.status_code == 400
+ assert "Unknown type" in response.json["message"]
+
+
+@SEMANTIC_LAYERS_APP
+def test_runtime_schema_exception(
+ client: Any,
+ full_api_access: None,
+ mocker: MockerFixture,
+) -> None:
+ """Test POST /<uuid>/schema/runtime returns 400 when schema raises."""
+ test_uuid = str(uuid_lib.uuid4())
+ mock_layer = MagicMock()
+ mock_layer.type = "snowflake"
+ mock_layer.implementation.configuration = {"account": "test"}
+
+ mock_dao = mocker.patch("superset.semantic_layers.api.SemanticLayerDAO")
+ mock_dao.find_by_uuid.return_value = mock_layer
+
+ mock_cls = MagicMock()
+ mock_cls.get_runtime_schema.side_effect = ValueError("Bad config")
+
+ mocker.patch.dict(
+ "superset.semantic_layers.api.registry",
+ {"snowflake": mock_cls},
+ clear=True,
+ )
+
+ response = client.post(
+ f"/api/v1/semantic_layer/{test_uuid}/schema/runtime",
+ )
+
+ assert response.status_code == 400
+ assert "Bad config" in response.json["message"]
+
+
+def test_post_semantic_layer(
+ client: Any,
+ full_api_access: None,
+ mocker: MockerFixture,
+) -> None:
+ """Test POST / creates a semantic layer."""
+ test_uuid = uuid_lib.uuid4()
+ new_model = MagicMock()
+ new_model.uuid = test_uuid
+
+ mock_command = mocker.patch(
+ "superset.semantic_layers.api.CreateSemanticLayerCommand",
+ )
+ mock_command.return_value.run.return_value = new_model
+
+ payload = {
+ "name": "My Layer",
+ "type": "snowflake",
+ "configuration": {"account": "test"},
+ }
+ response = client.post("/api/v1/semantic_layer/", json=payload)
+
+ assert response.status_code == 201
+ assert response.json["result"]["uuid"] == str(test_uuid)
+ mock_command.assert_called_once_with(payload)
+
+
+def test_post_semantic_layer_invalid(
+ client: Any,
+ full_api_access: None,
+ mocker: MockerFixture,
+) -> None:
+ """Test POST / returns 422 when validation fails."""
+ mock_command = mocker.patch(
+ "superset.semantic_layers.api.CreateSemanticLayerCommand",
+ )
+ mock_command.return_value.run.side_effect = SemanticLayerInvalidError(
+ "Unknown type: bad"
+ )
+
+ payload = {
+ "name": "My Layer",
+ "type": "bad",
+ "configuration": {},
+ }
+ response = client.post("/api/v1/semantic_layer/", json=payload)
+
+ assert response.status_code == 422
+
+
+def test_post_semantic_layer_create_failed(
+ client: Any,
+ full_api_access: None,
+ mocker: MockerFixture,
+) -> None:
+ """Test POST / returns 422 when creation fails."""
+ mock_command = mocker.patch(
+ "superset.semantic_layers.api.CreateSemanticLayerCommand",
+ )
+ mock_command.return_value.run.side_effect =
SemanticLayerCreateFailedError()
+
+ payload = {
+ "name": "My Layer",
+ "type": "snowflake",
+ "configuration": {"account": "test"},
+ }
+ response = client.post("/api/v1/semantic_layer/", json=payload)
+
+ assert response.status_code == 422
+
+
+def test_post_semantic_layer_missing_required_fields(
+ client: Any,
+ full_api_access: None,
+ mocker: MockerFixture,
+) -> None:
+ """Test POST / returns 400 when required fields are missing."""
+ mocker.patch(
+ "superset.semantic_layers.api.CreateSemanticLayerCommand",
+ )
+
+ response = client.post(
+ "/api/v1/semantic_layer/",
+ json={"name": "Only name"},
+ )
+
+ assert response.status_code == 400
+
+
+def test_put_semantic_layer(
+ client: Any,
+ full_api_access: None,
+ mocker: MockerFixture,
+) -> None:
+ """Test PUT /<uuid> updates a semantic layer."""
+ test_uuid = uuid_lib.uuid4()
+ changed_model = MagicMock()
+ changed_model.uuid = test_uuid
+
+ mock_command = mocker.patch(
+ "superset.semantic_layers.api.UpdateSemanticLayerCommand",
+ )
+ mock_command.return_value.run.return_value = changed_model
+
+ payload = {"name": "Updated Name"}
+ response = client.put(
+ f"/api/v1/semantic_layer/{test_uuid}",
+ json=payload,
+ )
+
+ assert response.status_code == 200
+ assert response.json["result"]["uuid"] == str(test_uuid)
+ mock_command.assert_called_once_with(str(test_uuid), payload)
+
+
+def test_put_semantic_layer_not_found(
+ client: Any,
+ full_api_access: None,
+ mocker: MockerFixture,
+) -> None:
+ """Test PUT /<uuid> returns 404 when layer not found."""
+ mock_command = mocker.patch(
+ "superset.semantic_layers.api.UpdateSemanticLayerCommand",
+ )
+ mock_command.return_value.run.side_effect = SemanticLayerNotFoundError()
+
+ response = client.put(
+ f"/api/v1/semantic_layer/{uuid_lib.uuid4()}",
+ json={"name": "New"},
+ )
+
+ assert response.status_code == 404
+
+
+def test_put_semantic_layer_invalid(
+ client: Any,
+ full_api_access: None,
+ mocker: MockerFixture,
+) -> None:
+ """Test PUT /<uuid> returns 422 when validation fails."""
+ mock_command = mocker.patch(
+ "superset.semantic_layers.api.UpdateSemanticLayerCommand",
+ )
+ mock_command.return_value.run.side_effect = SemanticLayerInvalidError(
+ "Name already exists"
+ )
+
+ response = client.put(
+ f"/api/v1/semantic_layer/{uuid_lib.uuid4()}",
+ json={"name": "Duplicate"},
+ )
+
+ assert response.status_code == 422
+
+
+def test_put_semantic_layer_update_failed(
+ client: Any,
+ full_api_access: None,
+ mocker: MockerFixture,
+) -> None:
+ """Test PUT /<uuid> returns 422 when update fails."""
+ mock_command = mocker.patch(
+ "superset.semantic_layers.api.UpdateSemanticLayerCommand",
+ )
+ mock_command.return_value.run.side_effect =
SemanticLayerUpdateFailedError()
+
+ response = client.put(
+ f"/api/v1/semantic_layer/{uuid_lib.uuid4()}",
+ json={"name": "Test"},
+ )
+
+ assert response.status_code == 422
+
+
+@SEMANTIC_LAYERS_APP
+def test_put_semantic_layer_validation_error(
+ client: Any,
+ full_api_access: None,
+) -> None:
+ """Test PUT /<uuid> returns 400 when payload fails schema validation."""
+ response = client.put(
+ f"/api/v1/semantic_layer/{uuid_lib.uuid4()}",
+ json={"cache_timeout": "not_a_number"},
+ )
+
+ assert response.status_code == 400
+
+
+def test_delete_semantic_layer(
+ client: Any,
+ full_api_access: None,
+ mocker: MockerFixture,
+) -> None:
+ """Test DELETE /<uuid> deletes a semantic layer."""
+ test_uuid = str(uuid_lib.uuid4())
+ mock_command = mocker.patch(
+ "superset.semantic_layers.api.DeleteSemanticLayerCommand",
+ )
+ mock_command.return_value.run.return_value = None
+
+ response = client.delete(f"/api/v1/semantic_layer/{test_uuid}")
+
+ assert response.status_code == 200
+ mock_command.assert_called_once_with(test_uuid)
+
+
+def test_delete_semantic_layer_not_found(
+ client: Any,
+ full_api_access: None,
+ mocker: MockerFixture,
+) -> None:
+ """Test DELETE /<uuid> returns 404 when layer not found."""
+ mock_command = mocker.patch(
+ "superset.semantic_layers.api.DeleteSemanticLayerCommand",
+ )
+ mock_command.return_value.run.side_effect = SemanticLayerNotFoundError()
+
+ response = client.delete(f"/api/v1/semantic_layer/{uuid_lib.uuid4()}")
+
+ assert response.status_code == 404
+
+
+def test_delete_semantic_layer_failed(
+ client: Any,
+ full_api_access: None,
+ mocker: MockerFixture,
+) -> None:
+ """Test DELETE /<uuid> returns 422 when deletion fails."""
+ mock_command = mocker.patch(
+ "superset.semantic_layers.api.DeleteSemanticLayerCommand",
+ )
+ mock_command.return_value.run.side_effect =
SemanticLayerDeleteFailedError()
+
+ response = client.delete(f"/api/v1/semantic_layer/{uuid_lib.uuid4()}")
+
+ assert response.status_code == 422
+
+
+def test_get_list_semantic_layers(
+ client: Any,
+ full_api_access: None,
+ mocker: MockerFixture,
+) -> None:
+ """Test GET / returns list of semantic layers."""
+ layer1 = MagicMock()
+ layer1.uuid = uuid_lib.uuid4()
+ layer1.name = "Layer 1"
+ layer1.description = "First"
+ layer1.type = "snowflake"
+ layer1.cache_timeout = None
+
+ layer2 = MagicMock()
+ layer2.uuid = uuid_lib.uuid4()
+ layer2.name = "Layer 2"
+ layer2.description = None
+ layer2.type = "snowflake"
+ layer2.cache_timeout = 300
+
+ mock_dao = mocker.patch("superset.semantic_layers.api.SemanticLayerDAO")
+ mock_dao.find_all.return_value = [layer1, layer2]
+
+ response = client.get("/api/v1/semantic_layer/")
+
+ assert response.status_code == 200
+ result = response.json["result"]
+ assert len(result) == 2
+ assert result[0]["name"] == "Layer 1"
+ assert result[1]["name"] == "Layer 2"
+ assert result[1]["cache_timeout"] == 300
+
+
+def test_get_list_semantic_layers_empty(
+ client: Any,
+ full_api_access: None,
+ mocker: MockerFixture,
+) -> None:
+ """Test GET / returns empty list when no layers exist."""
+ mock_dao = mocker.patch("superset.semantic_layers.api.SemanticLayerDAO")
+ mock_dao.find_all.return_value = []
+
+ response = client.get("/api/v1/semantic_layer/")
+
+ assert response.status_code == 200
+ assert response.json["result"] == []
+
+
+def test_get_semantic_layer(
+ client: Any,
+ full_api_access: None,
+ mocker: MockerFixture,
+) -> None:
+ """Test GET /<uuid> returns a single semantic layer."""
+ test_uuid = uuid_lib.uuid4()
+ layer = MagicMock()
+ layer.uuid = test_uuid
+ layer.name = "My Layer"
+ layer.description = "A layer"
+ layer.type = "snowflake"
+ layer.cache_timeout = 600
+
+ mock_dao = mocker.patch("superset.semantic_layers.api.SemanticLayerDAO")
+ mock_dao.find_by_uuid.return_value = layer
+
+ response = client.get(f"/api/v1/semantic_layer/{test_uuid}")
+
+ assert response.status_code == 200
+ result = response.json["result"]
+ assert result["uuid"] == str(test_uuid)
+ assert result["name"] == "My Layer"
+ assert result["type"] == "snowflake"
+ assert result["cache_timeout"] == 600
+
Review Comment:
**Suggestion:** The semantic layer delete and fetch tests exercise routes
that are only added when the `SEMANTIC_LAYERS` feature flag is enabled, but
these tests don't use the `SEMANTIC_LAYERS_APP` parametrization used elsewhere,
so in default settings the endpoints may not exist and the tests will fail with
404 responses. [possible bug]
<details>
<summary><b>Severity Level:</b> Critical 🚨</summary>
```mdx
- ❌ Delete semantic layer tests may fail with 404 responses.
- ❌ List/get semantic layer tests can fail due to missing routes.
- ⚠️ Retrieval/deletion behavior for semantic layers not reliably covered.
```
</details>
```suggestion
@SEMANTIC_LAYERS_APP
def test_delete_semantic_layer(
client: Any,
full_api_access: None,
mocker: MockerFixture,
) -> None:
"""Test DELETE /<uuid> deletes a semantic layer."""
test_uuid = str(uuid_lib.uuid4())
mock_command = mocker.patch(
"superset.semantic_layers.api.DeleteSemanticLayerCommand",
)
mock_command.return_value.run.return_value = None
response = client.delete(f"/api/v1/semantic_layer/{test_uuid}")
assert response.status_code == 200
mock_command.assert_called_once_with(test_uuid)
@SEMANTIC_LAYERS_APP
def test_delete_semantic_layer_not_found(
client: Any,
full_api_access: None,
mocker: MockerFixture,
) -> None:
"""Test DELETE /<uuid> returns 404 when layer not found."""
mock_command = mocker.patch(
"superset.semantic_layers.api.DeleteSemanticLayerCommand",
)
mock_command.return_value.run.side_effect = SemanticLayerNotFoundError()
response = client.delete(f"/api/v1/semantic_layer/{uuid_lib.uuid4()}")
assert response.status_code == 404
@SEMANTIC_LAYERS_APP
def test_delete_semantic_layer_failed(
client: Any,
full_api_access: None,
mocker: MockerFixture,
) -> None:
"""Test DELETE /<uuid> returns 422 when deletion fails."""
mock_command = mocker.patch(
"superset.semantic_layers.api.DeleteSemanticLayerCommand",
)
mock_command.return_value.run.side_effect =
SemanticLayerDeleteFailedError()
response = client.delete(f"/api/v1/semantic_layer/{uuid_lib.uuid4()}")
assert response.status_code == 422
@SEMANTIC_LAYERS_APP
def test_get_list_semantic_layers(
client: Any,
full_api_access: None,
mocker: MockerFixture,
) -> None:
"""Test GET / returns list of semantic layers."""
layer1 = MagicMock()
layer1.uuid = uuid_lib.uuid4()
layer1.name = "Layer 1"
layer1.description = "First"
layer1.type = "snowflake"
layer1.cache_timeout = None
layer2 = MagicMock()
layer2.uuid = uuid_lib.uuid4()
layer2.name = "Layer 2"
layer2.description = None
layer2.type = "snowflake"
layer2.cache_timeout = 300
mock_dao = mocker.patch("superset.semantic_layers.api.SemanticLayerDAO")
mock_dao.find_all.return_value = [layer1, layer2]
response = client.get("/api/v1/semantic_layer/")
assert response.status_code == 200
result = response.json["result"]
assert len(result) == 2
assert result[0]["name"] == "Layer 1"
assert result[1]["name"] == "Layer 2"
assert result[1]["cache_timeout"] == 300
@SEMANTIC_LAYERS_APP
def test_get_list_semantic_layers_empty(
client: Any,
full_api_access: None,
mocker: MockerFixture,
) -> None:
"""Test GET / returns empty list when no layers exist."""
mock_dao = mocker.patch("superset.semantic_layers.api.SemanticLayerDAO")
mock_dao.find_all.return_value = []
response = client.get("/api/v1/semantic_layer/")
assert response.status_code == 200
assert response.json["result"] == []
@SEMANTIC_LAYERS_APP
def test_get_semantic_layer(
client: Any,
full_api_access: None,
mocker: MockerFixture,
) -> None:
"""Test GET /<uuid> returns a single semantic layer."""
test_uuid = uuid_lib.uuid4()
layer = MagicMock()
layer.uuid = test_uuid
layer.name = "My Layer"
layer.description = "A layer"
layer.type = "snowflake"
layer.cache_timeout = 600
mock_dao = mocker.patch("superset.semantic_layers.api.SemanticLayerDAO")
mock_dao.find_by_uuid.return_value = layer
response = client.get(f"/api/v1/semantic_layer/{test_uuid}")
assert response.status_code == 200
result = response.json["result"]
assert result["uuid"] == str(test_uuid)
assert result["name"] == "My Layer"
assert result["type"] == "snowflake"
assert result["cache_timeout"] == 600
@SEMANTIC_LAYERS_APP
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. The `SupersetAppInitializer.init_views()` method registers REST APIs in
`superset/initialization/__init__.py:236-281`, and only registers
`SemanticLayerRestApi`
(which defines DELETE and GET endpoints in
`superset/semantic_layers/api.py:401-455,
457-481`) when `feature_flag_manager.is_feature_enabled("SEMANTIC_LAYERS")`
is true
(condition at lines 269-276).
2. The delete and fetch tests in
`tests/unit_tests/semantic_layers/api_test.py:752-887`
(`test_delete_semantic_layer*`, `test_get_list_semantic_layers*`,
`test_get_semantic_layer*`) use `client` and `full_api_access` fixtures but
do NOT use the
`@SEMANTIC_LAYERS_APP` parametrization declared at lines 37-41, so the app
is created with
default feature flags and may not have `SEMANTIC_LAYERS` enabled.
3. With `SEMANTIC_LAYERS` disabled, routes like `DELETE
/api/v1/semantic_layer/<uuid>` and
`GET /api/v1/semantic_layer/` are not registered; issuing
`client.delete(f"/api/v1/semantic_layer/{test_uuid}")` (line 764) or
`client.get("/api/v1/semantic_layer/")` (line 825) returns 404,
contradicting the tests'
expectations (`assert response.status_code == 200` at lines 766, 827 and
`assert
response.status_code == 422`/`404` in the delete tests).
4. This causes the delete/list/get tests to fail whenever the semantic
layers feature flag
is off by default in the test configuration, even though the underlying
implementations in
`superset/semantic_layers/api.py` correctly handle normal, not-found, and
error cases when
the routes are actually registered.
```
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** tests/unit_tests/semantic_layers/api_test.py
**Line:** 752:875
**Comment:**
*Possible Bug: The semantic layer delete and fetch tests exercise
routes that are only added when the `SEMANTIC_LAYERS` feature flag is enabled,
but these tests don't use the `SEMANTIC_LAYERS_APP` parametrization used
elsewhere, so in default settings the endpoints may not exist and the tests
will fail with 404 responses.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38370&comment_hash=e8b8230b6b2a7e630529c56dab9d09e5bea44de06d93a45657d311dac3fe6f90&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38370&comment_hash=e8b8230b6b2a7e630529c56dab9d09e5bea44de06d93a45657d311dac3fe6f90&reaction=dislike'>👎</a>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
