diff --git a/openmetadata-integration-tests/src/test/java/org/openmetadata/it/tests/GlossaryTermRelationSettingsIT.java b/openmetadata-integration-tests/src/test/java/org/openmetadata/it/tests/GlossaryTermRelationSettingsIT.java index 9fd675fb70fa..b6729d702992 100644 --- a/openmetadata-integration-tests/src/test/java/org/openmetadata/it/tests/GlossaryTermRelationSettingsIT.java +++ b/openmetadata-integration-tests/src/test/java/org/openmetadata/it/tests/GlossaryTermRelationSettingsIT.java @@ -515,6 +515,88 @@ void test_systemDefinedRelationTypeCannotBeDeleted() throws Exception { "Should not be able to remove system-defined relation type 'relatedTo'. Got: " + status); } + @Test + @ResourceLock( + value = SharedResourceLocks.GLOSSARY_TERM_RELATION_SETTINGS, + mode = ResourceAccessMode.READ_WRITE) + void test_systemDefinedRelationTypeFieldCannotBeModified() throws Exception { + JsonNode currentSettings = getSettings(); + ArrayNode relationTypes = (ArrayNode) currentSettings.get("config_value").get("relationTypes"); + + boolean toggled = false; + boolean originalIsTransitive = false; + for (JsonNode type : relationTypes) { + if ("partOf".equals(type.get("name").asText())) { + JsonNode sys = type.get("isSystemDefined"); + assertTrue(sys != null && sys.asBoolean(), "'partOf' should be system-defined"); + originalIsTransitive = type.get("isTransitive").asBoolean(); + ((ObjectNode) type).put("isTransitive", !originalIsTransitive); + toggled = true; + break; + } + } + assertTrue(toggled, "'partOf' should exist in default settings"); + + ObjectNode modifiedSettings = MAPPER.createObjectNode(); + modifiedSettings.set("relationTypes", relationTypes); + int status = updateSettingsAndGetStatus(modifiedSettings); + assertTrue( + status >= 400, + "Editing a field of system-defined 'partOf' via settings PUT must be rejected. Got: " + + status); + + JsonNode after = getSettings(); + for (JsonNode type : after.get("config_value").get("relationTypes")) { + if ("partOf".equals(type.get("name").asText())) { + assertEquals( + originalIsTransitive, + type.get("isTransitive").asBoolean(), + "'partOf' isTransitive must be unchanged after a rejected edit"); + } + } + } + + @Test + @ResourceLock( + value = SharedResourceLocks.GLOSSARY_TERM_RELATION_SETTINGS, + mode = ResourceAccessMode.READ_WRITE) + void test_cannotCreateSystemDefinedRelationTypeViaSettingsPut() throws Exception { + String fakeName = "fakeSystem" + System.currentTimeMillis(); + + JsonNode currentSettings = getSettings(); + ArrayNode relationTypes = (ArrayNode) currentSettings.get("config_value").get("relationTypes"); + + ObjectNode fakeType = MAPPER.createObjectNode(); + fakeType.put("name", fakeName); + fakeType.put("displayName", "Fake System Type"); + fakeType.put("isSymmetric", false); + fakeType.put("isTransitive", false); + fakeType.put("isCrossGlossaryAllowed", true); + fakeType.put("category", "associative"); + fakeType.put("isSystemDefined", true); + fakeType.put("color", "#22c55e"); + fakeType.put("cardinality", "MANY_TO_MANY"); + relationTypes.add(fakeType); + + ObjectNode newSettings = MAPPER.createObjectNode(); + newSettings.set("relationTypes", relationTypes); + int status = updateSettingsAndGetStatus(newSettings); + assertTrue( + status >= 400, + "Creating a system-defined relation type via settings PUT must be rejected. Got: " + + status); + + JsonNode after = getSettings(); + boolean present = false; + for (JsonNode type : after.get("config_value").get("relationTypes")) { + if (fakeName.equals(type.get("name").asText())) { + present = true; + break; + } + } + assertFalse(present, "Fabricated system-defined type must not be persisted"); + } + @Test @ResourceLock( value = SharedResourceLocks.GLOSSARY_TERM_RELATION_SETTINGS, diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/util/GlossaryTermRelationSettingsUtil.java b/openmetadata-service/src/main/java/org/openmetadata/service/util/GlossaryTermRelationSettingsUtil.java index abb19188724c..c16639752834 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/util/GlossaryTermRelationSettingsUtil.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/util/GlossaryTermRelationSettingsUtil.java @@ -14,13 +14,17 @@ package org.openmetadata.service.util; import jakarta.ws.rs.core.Response; +import java.util.ArrayList; +import java.util.HashMap; import java.util.HashSet; import java.util.List; import java.util.Locale; +import java.util.Map; import java.util.Set; import org.openmetadata.schema.configuration.GlossaryTermRelationSettings; import org.openmetadata.schema.configuration.GlossaryTermRelationType; import org.openmetadata.schema.configuration.RelationCardinality; +import org.openmetadata.schema.utils.JsonUtils; import org.openmetadata.service.exception.SystemSettingsException; public final class GlossaryTermRelationSettingsUtil { @@ -87,32 +91,112 @@ public static void validateUniqueNames(GlossaryTermRelationSettings settings) { } } + /** + * Enforces the immutability contract for seeded (system-defined) relation types on the generic + * settings-update path. System-defined types cannot be removed or downgraded, no new type may be + * flagged as system-defined (create/promote), and an existing system-defined type's fields cannot + * be edited. Custom relation types are unaffected. The dedicated relationTypes endpoint and the UI + * already enforce this; this covers the remaining generic {@code PUT /system/settings} path. + */ public static void validateSystemDefinedRelationTypesPreserved( GlossaryTermRelationSettings current, GlossaryTermRelationSettings updated) { if (current == null || current.getRelationTypes() == null) { return; } - Set updatedSystemDefinedNames = new HashSet<>(); - if (updated != null && updated.getRelationTypes() != null) { - for (GlossaryTermRelationType relationType : updated.getRelationTypes()) { - if (relationType != null && Boolean.TRUE.equals(relationType.getIsSystemDefined())) { - updatedSystemDefinedNames.add(relationType.getName()); - } + Map updatedByName = indexByName(updated); + validateNoSystemDefinedRemoved(current, updatedByName); + validateNoUnsanctionedSystemDefined(current, updated); + validateSystemDefinedUnmodified(current, updatedByName); + } + + private static Map indexByName( + GlossaryTermRelationSettings settings) { + Map byName = new HashMap<>(); + if (settings == null || settings.getRelationTypes() == null) { + return byName; + } + for (GlossaryTermRelationType relationType : settings.getRelationTypes()) { + if (relationType != null && relationType.getName() != null) { + byName.put(relationType.getName(), relationType); } } + return byName; + } - List missingSystemDefinedNames = - current.getRelationTypes().stream() - .filter(relationType -> Boolean.TRUE.equals(relationType.getIsSystemDefined())) - .map(GlossaryTermRelationType::getName) - .filter(name -> !updatedSystemDefinedNames.contains(name)) - .toList(); - if (!missingSystemDefinedNames.isEmpty()) { + private static void validateNoSystemDefinedRemoved( + GlossaryTermRelationSettings current, Map updatedByName) { + List removed = new ArrayList<>(); + for (GlossaryTermRelationType currentType : current.getRelationTypes()) { + if (!Boolean.TRUE.equals(currentType.getIsSystemDefined())) { + continue; + } + GlossaryTermRelationType updatedType = updatedByName.get(currentType.getName()); + if (updatedType == null || !Boolean.TRUE.equals(updatedType.getIsSystemDefined())) { + removed.add(currentType.getName()); + } + } + if (!removed.isEmpty()) { throw new SystemSettingsException( - "Cannot delete system-defined relation types: " - + String.join(", ", missingSystemDefinedNames)); + "Cannot delete system-defined relation types: " + String.join(", ", removed)); + } + } + + private static void validateNoUnsanctionedSystemDefined( + GlossaryTermRelationSettings current, GlossaryTermRelationSettings updated) { + if (updated == null || updated.getRelationTypes() == null) { + return; } + Set currentSystemDefinedNames = new HashSet<>(); + for (GlossaryTermRelationType relationType : current.getRelationTypes()) { + if (Boolean.TRUE.equals(relationType.getIsSystemDefined())) { + currentSystemDefinedNames.add(relationType.getName()); + } + } + List unsanctioned = new ArrayList<>(); + for (GlossaryTermRelationType updatedType : updated.getRelationTypes()) { + if (updatedType != null + && Boolean.TRUE.equals(updatedType.getIsSystemDefined()) + && !currentSystemDefinedNames.contains(updatedType.getName())) { + unsanctioned.add(updatedType.getName()); + } + } + if (!unsanctioned.isEmpty()) { + throw new SystemSettingsException( + "Cannot create or promote system-defined relation types: " + + String.join(", ", unsanctioned)); + } + } + + private static void validateSystemDefinedUnmodified( + GlossaryTermRelationSettings current, Map updatedByName) { + List modified = new ArrayList<>(); + for (GlossaryTermRelationType currentType : current.getRelationTypes()) { + if (!Boolean.TRUE.equals(currentType.getIsSystemDefined())) { + continue; + } + GlossaryTermRelationType updatedType = updatedByName.get(currentType.getName()); + if (updatedType != null && isRelationTypeModified(currentType, updatedType)) { + modified.add(currentType.getName()); + } + } + if (!modified.isEmpty()) { + throw new SystemSettingsException( + "Cannot modify system-defined relation types: " + String.join(", ", modified)); + } + } + + private static boolean isRelationTypeModified( + GlossaryTermRelationType current, GlossaryTermRelationType updated) { + // Compare normalized copies so derived cardinality fields (sourceMax/targetMax) don't + // register as spurious edits when the stored value predates normalization. + GlossaryTermRelationType currentCopy = + JsonUtils.deepCopy(current, GlossaryTermRelationType.class); + GlossaryTermRelationType updatedCopy = + JsonUtils.deepCopy(updated, GlossaryTermRelationType.class); + normalize(currentCopy); + normalize(updatedCopy); + return !JsonUtils.valueToTree(currentCopy).equals(JsonUtils.valueToTree(updatedCopy)); } private static RelationCardinality deriveCardinality(Integer sourceMax, Integer targetMax) { diff --git a/openmetadata-service/src/test/java/org/openmetadata/service/util/GlossaryTermRelationSettingsUtilTest.java b/openmetadata-service/src/test/java/org/openmetadata/service/util/GlossaryTermRelationSettingsUtilTest.java index b54c9430aa25..5d16057851c9 100644 --- a/openmetadata-service/src/test/java/org/openmetadata/service/util/GlossaryTermRelationSettingsUtilTest.java +++ b/openmetadata-service/src/test/java/org/openmetadata/service/util/GlossaryTermRelationSettingsUtilTest.java @@ -93,6 +93,109 @@ void validateSystemDefinedRelationTypesRejectsDowngrade() { current, updated)); } + @Test + void validateSystemDefinedRelationTypesRejectsFieldModification() { + GlossaryTermRelationSettings current = + new GlossaryTermRelationSettings() + .withRelationTypes( + List.of(relationType("partOf").withIsSystemDefined(true).withIsTransitive(false))); + GlossaryTermRelationSettings updated = + new GlossaryTermRelationSettings() + .withRelationTypes( + List.of(relationType("partOf").withIsSystemDefined(true).withIsTransitive(true))); + + SystemSettingsException exception = + assertThrows( + SystemSettingsException.class, + () -> + GlossaryTermRelationSettingsUtil.validateSystemDefinedRelationTypesPreserved( + current, updated)); + + assertEquals("Cannot modify system-defined relation types: partOf", exception.getMessage()); + } + + @Test + void validateSystemDefinedRelationTypesRejectsNewSystemDefinedType() { + GlossaryTermRelationSettings current = + new GlossaryTermRelationSettings() + .withRelationTypes(List.of(relationType("partOf").withIsSystemDefined(true))); + GlossaryTermRelationSettings updated = + new GlossaryTermRelationSettings() + .withRelationTypes( + List.of( + relationType("partOf").withIsSystemDefined(true), + relationType("fakeSys").withIsSystemDefined(true))); + + SystemSettingsException exception = + assertThrows( + SystemSettingsException.class, + () -> + GlossaryTermRelationSettingsUtil.validateSystemDefinedRelationTypesPreserved( + current, updated)); + + assertEquals( + "Cannot create or promote system-defined relation types: fakeSys", exception.getMessage()); + } + + @Test + void validateSystemDefinedRelationTypesRejectsPromotionOfCustomType() { + GlossaryTermRelationSettings current = + new GlossaryTermRelationSettings() + .withRelationTypes( + List.of( + relationType("partOf").withIsSystemDefined(true), + relationType("dependsOn").withIsSystemDefined(false))); + GlossaryTermRelationSettings updated = + new GlossaryTermRelationSettings() + .withRelationTypes( + List.of( + relationType("partOf").withIsSystemDefined(true), + relationType("dependsOn").withIsSystemDefined(true))); + + SystemSettingsException exception = + assertThrows( + SystemSettingsException.class, + () -> + GlossaryTermRelationSettingsUtil.validateSystemDefinedRelationTypesPreserved( + current, updated)); + + assertEquals( + "Cannot create or promote system-defined relation types: dependsOn", + exception.getMessage()); + } + + @Test + void validateSystemDefinedRelationTypesAllowsCustomTypeModification() { + GlossaryTermRelationSettings current = + new GlossaryTermRelationSettings() + .withRelationTypes( + List.of( + relationType("dependsOn").withIsSystemDefined(false).withIsTransitive(false))); + GlossaryTermRelationSettings updated = + new GlossaryTermRelationSettings() + .withRelationTypes( + List.of( + relationType("dependsOn").withIsSystemDefined(false).withIsTransitive(true))); + + // Custom (non system-defined) relation types remain fully editable. + GlossaryTermRelationSettingsUtil.validateSystemDefinedRelationTypesPreserved(current, updated); + } + + @Test + void validateSystemDefinedRelationTypesAllowsUnchangedSystemType() { + GlossaryTermRelationSettings current = + new GlossaryTermRelationSettings() + .withRelationTypes( + List.of(relationType("partOf").withIsSystemDefined(true).withIsTransitive(false))); + GlossaryTermRelationSettings updated = + new GlossaryTermRelationSettings() + .withRelationTypes( + List.of(relationType("partOf").withIsSystemDefined(true).withIsTransitive(false))); + + // Re-saving system-defined types unchanged (e.g. alongside a new custom type) must be allowed. + GlossaryTermRelationSettingsUtil.validateSystemDefinedRelationTypesPreserved(current, updated); + } + private GlossaryTermRelationType relationType(String name) { return new GlossaryTermRelationType().withName(name); }