Fixes #10907 Parent glossary term should not be allowed moved under its child term (#10908)

openmetadata-service/src/main/java/org/openmetadata/service/exception/CatalogExceptionMessage.java

@@ -202,4 +202,8 @@ public final class CatalogExceptionMessage {
   public static String userAlreadyBot(String userName, String botName) {
     return String.format("Bot user [%s] is already used by [%s] bot", userName, botName);
   }
+
+  public static String invalidGlossaryTermMove(String term, String newParent) {
+    return String.format("Can't move Glossary term %s to its child Glossary term %s", term, newParent);
+  }
 }

openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/GlossaryTermRepository.java

@@ -20,6 +20,7 @@ import static org.openmetadata.common.utils.CommonUtil.listOrEmpty;
 import static org.openmetadata.schema.type.Include.ALL;
 import static org.openmetadata.service.Entity.GLOSSARY;
 import static org.openmetadata.service.Entity.GLOSSARY_TERM;
+import static org.openmetadata.service.exception.CatalogExceptionMessage.invalidGlossaryTermMove;
 import static org.openmetadata.service.util.EntityUtil.entityReferenceMatch;
 import static org.openmetadata.service.util.EntityUtil.getId;
 import static org.openmetadata.service.util.EntityUtil.stringMatch;
@@ -254,6 +255,7 @@ public class GlossaryTermRepository extends EntityRepository<GlossaryTerm> {
 
     @Override
     public void entitySpecificUpdate() throws IOException {
+      validateParent();
       updateStatus(original, updated);
       updateSynonyms(original, updated);
       updateReferences(original, updated);
@@ -362,5 +364,14 @@ public class GlossaryTermRepository extends EntityRepository<GlossaryTerm> {
         recordChange("parent", original.getParent(), updated.getParent(), true, entityReferenceMatch);
       }
     }
+
+    private void validateParent() {
+      String fqn = original.getFullyQualifiedName();
+      String newParentFqn = updated.getParent() == null ? null : updated.getParent().getFullyQualifiedName();
+      // A glossary term can't be moved under its child
+      if (newParentFqn != null && FullyQualifiedName.isParent(newParentFqn, fqn)) {
+        throw new IllegalArgumentException(invalidGlossaryTermMove(fqn, newParentFqn));
+      }
+    }
   }
 }

openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/MlModelRepository.java

@@ -129,7 +129,6 @@ public class MlModelRepository extends EntityRepository<MlModel> {
   @Override
   public void prepare(MlModel mlModel) throws IOException {
     populateService(mlModel);
-    setFullyQualifiedName(mlModel);
     if (!nullOrEmpty(mlModel.getMlFeatures())) {
       validateReferences(mlModel.getMlFeatures());
     }

openmetadata-service/src/main/java/org/openmetadata/service/resources/tags/TagResource.java

@@ -498,12 +498,8 @@ public class TagResource extends EntityResource<Tag, TagRepository> {
 
   private Tag getTag(CreateTag create, String updateBy) throws IOException {
     String parentFQN = create.getParent() != null ? create.getParent() : create.getClassification();
-    EntityReference classification =
-        new EntityReference().withFullyQualifiedName(create.getClassification()).withType(CLASSIFICATION);
-    EntityReference parent =
-        create.getParent() == null
-            ? null
-            : new EntityReference().withFullyQualifiedName(create.getParent()).withType(TAG);
+    EntityReference classification = getEntityReference(CLASSIFICATION, create.getClassification());
+    EntityReference parent = create.getParent() == null ? null : getEntityReference(TAG, create.getParent());
     return copy(new Tag(), create, updateBy)
         .withFullyQualifiedName(FullyQualifiedName.add(parentFQN, create.getName()))
         .withParent(parent)

openmetadata-service/src/main/java/org/openmetadata/service/util/FullyQualifiedName.java

@@ -98,6 +98,11 @@ public class FullyQualifiedName {
     return split[0];
   }
 
+  public static boolean isParent(String childFqn, String parentFqn) {
+    // Returns true if the childFqn is indeed the child of parentFqn
+    return childFqn.startsWith(parentFqn) && childFqn.length() > parentFqn.length();
+  }
+
   private static class SplitListener extends FqnBaseListener {
     final List<String> list = new ArrayList<>();
 

openmetadata-service/src/test/java/org/openmetadata/service/resources/glossary/GlossaryResourceTest.java

@@ -314,8 +314,17 @@ public class GlossaryResourceTest extends EntityResourceTest<Glossary, CreateGlo
         copyGlossaryTerm(updatedTerm, termToMove);
       }
     }
+
+    // Move a parent term g1.t1 to its child g1.t1.t11 should be disallowed
+    assertResponse(
+        () -> glossaryTermResourceTest.moveGlossaryTerm(g.getEntityReference(), t11.getEntityReference(), t1),
+        Status.BAD_REQUEST,
+        CatalogExceptionMessage.invalidGlossaryTermMove(t1.getFullyQualifiedName(), t11.getFullyQualifiedName()));
   }
 
+  @Test
+  void patch_moveGlossaryTermParentToChild() {}
+
   @Test
   void testCsvDocumentation() throws HttpResponseException {
     assertEquals(GlossaryCsv.DOCUMENTATION, getCsvDocumentation());

openmetadata-service/src/test/java/org/openmetadata/service/util/FullyQualifiedNameTest.java

@@ -1,8 +1,10 @@
 package org.openmetadata.service.util;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertNull;
 import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
 
 import java.util.List;
 import org.antlr.v4.runtime.misc.ParseCancellationException;
@@ -96,4 +98,13 @@ class FullyQualifiedNameTest {
     assertEquals("a", FullyQualifiedName.getRoot("a.b"));
     assertNull(FullyQualifiedName.getRoot("a"));
   }
+
+  @Test
+  void test_isParent() {
+    assertTrue(FullyQualifiedName.isParent("a.b.c", "a.b"));
+    assertTrue(FullyQualifiedName.isParent("a.b.c", "a"));
+    assertFalse(FullyQualifiedName.isParent("a", "a.b.c"));
+    assertFalse(FullyQualifiedName.isParent("a.b", "a.b.c"));
+    assertFalse(FullyQualifiedName.isParent("a.b.c", "a.b.c"));
+  }
 }