fix(semantic-layer): preserve metadata generation on unchanged configuration

superset/commands/semantic_layer/update.py

@@ -219,7 +219,19 @@ def run(self) -> Model:
                 self._properties["configuration"]
             )
         if "configuration" in self._properties:
-            self._model.clear_metadata_cache()
+            try:
+                unchanged: bool = json.dumps(
+                    json.loads(self._model.configuration), sort_keys=True
+                ) == json.dumps(
+                    json.loads(self._properties["configuration"]), sort_keys=True
+                )
+            except (TypeError, ValueError):
+                unchanged = False
+            if unchanged:
+                # Avoid overwriting a concurrent edit with this reader's old config.
+                self._properties.pop("configuration")
+            else:
+                self._model.clear_metadata_cache()
         return SemanticLayerDAO.update(self._model, attributes=self._properties)
 
     def validate(self) -> None:

tests/unit_tests/commands/semantic_layer/update_test.py

@@ -35,7 +35,6 @@
 )
 from superset.constants import PASSWORD_MASK
 from superset.exceptions import SupersetSecurityException
-from superset.utils import json
 
 
 def test_update_semantic_view_success(mocker: MockerFixture) -> None:
@@ -648,11 +647,11 @@ def test_update_semantic_layer_preserves_masked_secret_end_to_end(
 ) -> None:
     """A name-only PUT that round-trips the masked GET response does not
     overwrite the stored credential with the literal mask."""
-    mock_model = MagicMock()
+    mock_model: MagicMock = MagicMock()
     mock_model.type = "snowflake"
     mock_model.configuration = '{"account": "test", "password": "hunter2"}'
 
-    dao = mocker.patch(
+    dao: MagicMock = mocker.patch(
         "superset.commands.semantic_layer.update.SemanticLayerDAO",
     )
     dao.find_by_uuid.return_value = mock_model
@@ -662,7 +661,7 @@ def test_update_semantic_layer_preserves_masked_secret_end_to_end(
         "superset.commands.semantic_layer.update.current_user_can_modify_object",
     )
 
-    mock_cls = MagicMock()
+    mock_cls: MagicMock = MagicMock()
     mock_cls.get_configuration_schema.return_value = {
         "properties": {"password": {"type": "string", "writeOnly": True}}
     }
@@ -672,7 +671,7 @@ def test_update_semantic_layer_preserves_masked_secret_end_to_end(
         clear=True,
     )
 
-    data = {
+    data: dict[str, Any] = {
         "name": "Renamed",
         "configuration": {"account": "test", "password": PASSWORD_MASK},
     }
@@ -683,11 +682,10 @@ def test_update_semantic_layer_preserves_masked_secret_end_to_end(
     )
     dao.update.assert_called_once_with(
         mock_model,
-        attributes={
-            "name": "Renamed",
-            "configuration": json.dumps({"account": "test", "password": "hunter2"}),
-        },
+        attributes={"name": "Renamed"},
     )
+    assert mock_model.configuration == '{"account": "test", "password": "hunter2"}'
+    mock_model.clear_metadata_cache.assert_not_called()
 
 
 @pytest.mark.parametrize("registered", [True, False])

tests/unit_tests/semantic_layers/simple_cache_clear_test.py

@@ -69,18 +69,19 @@ def layer(
 @pytest.mark.parametrize(
     "properties,expected",
     [
-        ({"configuration": {}}, 1),
+        ({"configuration": {}}, 0),
+        ({"configuration": {"value": "new"}}, 1),
         ({"description": "changed"}, 0),
     ],
 )
-def test_save_rotates_only_submitted_configuration(
+def test_save_rotates_only_changed_configuration(
     app: Flask,
     session: Session,
     layer: SemanticLayer,
     properties: dict[str, Any],
     expected: int,
 ) -> None:
-    """Save writes its generation in the same transaction, even for unchanged config."""
+    """Save rotates generation only when the effective configuration changes."""
     command: UpdateSemanticLayerCommand = UpdateSemanticLayerCommand(
         str(layer.uuid), properties
     )
@@ -91,6 +92,67 @@ def test_save_rotates_only_submitted_configuration(
     assert session.get(SemanticLayer, layer.uuid).cache_version == expected
 
 
+@pytest.mark.parametrize(
+    "stored,submitted,expected",
+    [
+        ('{ "b": 2, "a": {"x": 1} }', {"a": {"x": 1}, "b": 2}, 0),
+        ('{"value": true}', {"value": 1}, 1),
+        ("invalid JSON", {"value": "repaired"}, 1),
+        (None, {}, 1),
+        ("", {}, 1),
+        ('{"items": [1, 2]}', {"items": [2, 1]}, 1),
+    ],
+)
+def test_save_compares_configuration_content(
+    app: Flask,
+    session: Session,
+    layer: SemanticLayer,
+    stored: str | None,
+    submitted: dict[str, Any],
+    expected: int,
+) -> None:
+    """Ignore JSON formatting/key order, but preserve scalar types and list order."""
+    layer.configuration = stored
+    session.commit()
+    session.expire_all()
+    assert layer.configuration == stored
+    command: UpdateSemanticLayerCommand = UpdateSemanticLayerCommand(
+        str(layer.uuid), {"configuration": submitted}
+    )
+    command._model = layer
+    with app.app_context(), patch.object(command, "validate"):
+        command.run()
+    session.expire_all()
+    assert session.get(SemanticLayer, layer.uuid).cache_version == expected
+    if stored is None or stored == "":
+        assert layer.configuration == "{}"
+
+
+def test_noop_save_preserves_a_concurrent_configuration_change(
+    app: Flask,
+    session: Session,
+    layer: SemanticLayer,
+) -> None:
+    """A stale no-op reader must not overwrite a newer configuration/generation."""
+    layer.configuration = '{ "a": 1, "b": 2 }'
+    session.commit()
+    assert layer.configuration == '{ "a": 1, "b": 2 }'
+    session.execute(
+        SemanticLayer.__table__.update()
+        .where(SemanticLayer.uuid == layer.uuid)
+        .values(configuration='{"new": true}', cache_version=1)
+    )
+    command: UpdateSemanticLayerCommand = UpdateSemanticLayerCommand(
+        str(layer.uuid), {"configuration": {"b": 2, "a": 1}}
+    )
+    command._model = layer
+    with app.app_context(), patch.object(command, "validate"):
+        command.run()
+    session.expire_all()
+    assert layer.configuration == '{"new": true}'
+    assert layer.cache_version == 1
+
+
 def test_failed_save_rolls_back_configuration_and_version(
     app: Flask,
     session: Session,