Fixes #4400 Glossary and GlossaryTerm Is not getting deleted with recursive=true (#4418)

This commit is contained in:
Suresh Srinivas 2022-04-23 12:58:22 -07:00 committed by GitHub
parent 3544527828
commit 4b12c52662
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 74 additions and 9 deletions

View File

@ -115,4 +115,8 @@ public final class CatalogExceptionMessage {
"Found multiple rules with operation %s within policy %s. Please ensure that operation across all rules within the policy are distinct",
operation, policy);
}
public static String entityIsNotEmpty(String entityType) {
return String.format("%s is not empty", entityType);
}
}

View File

@ -498,7 +498,7 @@ public abstract class EntityRepository<T> {
if (!contains.isEmpty()) {
if (!recursive) {
throw new IllegalArgumentException(entityType + " is not empty");
throw new IllegalArgumentException(CatalogExceptionMessage.entityIsNotEmpty(entityType));
}
// Soft delete all the contained entities
for (EntityReference entityReference : contains) {

View File

@ -77,12 +77,12 @@ public class GlossaryTermRepository extends EntityRepository<GlossaryTerm> {
}
private EntityReference getParent(GlossaryTerm entity) throws IOException {
List<String> ids = findFrom(entity.getId(), GLOSSARY_TERM, Relationship.PARENT_OF, GLOSSARY_TERM);
List<String> ids = findFrom(entity.getId(), GLOSSARY_TERM, Relationship.CONTAINS, GLOSSARY_TERM);
return ids.size() == 1 ? Entity.getEntityReferenceById(GLOSSARY_TERM, UUID.fromString(ids.get(0)), ALL) : null;
}
private List<EntityReference> getChildren(GlossaryTerm entity) throws IOException {
List<String> ids = findTo(entity.getId(), GLOSSARY_TERM, Relationship.PARENT_OF, GLOSSARY_TERM);
List<String> ids = findTo(entity.getId(), GLOSSARY_TERM, Relationship.CONTAINS, GLOSSARY_TERM);
return EntityUtil.populateEntityReferences(ids, GLOSSARY_TERM);
}
@ -157,7 +157,7 @@ public class GlossaryTermRepository extends EntityRepository<GlossaryTerm> {
addRelationship(
entity.getGlossary().getId(), entity.getId(), Entity.GLOSSARY, GLOSSARY_TERM, Relationship.CONTAINS);
if (entity.getParent() != null) {
addRelationship(entity.getParent().getId(), entity.getId(), GLOSSARY_TERM, GLOSSARY_TERM, Relationship.PARENT_OF);
addRelationship(entity.getParent().getId(), entity.getId(), GLOSSARY_TERM, GLOSSARY_TERM, Relationship.CONTAINS);
}
for (EntityReference relTerm : listOrEmpty(entity.getRelatedTerms())) {
// Make this bidirectional relationship
@ -273,7 +273,7 @@ public class GlossaryTermRepository extends EntityRepository<GlossaryTerm> {
@Override
public EntityReference getContainer() {
return null;
return entity.getParent() == null ? entity.getGlossary() : entity.getParent();
}
@Override

View File

@ -333,13 +333,17 @@ public class GlossaryResource extends EntityResource<Glossary, GlossaryRepositor
public Response delete(
@Context UriInfo uriInfo,
@Context SecurityContext securityContext,
@Parameter(description = "Recursively delete this entity and it's children. (Default `false`)")
@DefaultValue("false")
@QueryParam("recursive")
boolean recursive,
@Parameter(description = "Hard delete the entity. (Default = `false`)")
@QueryParam("hardDelete")
@DefaultValue("false")
boolean hardDelete,
@Parameter(description = "Chart Id", schema = @Schema(type = "string")) @PathParam("id") String id)
throws IOException {
return delete(uriInfo, securityContext, id, false, hardDelete, ADMIN | BOT);
return delete(uriInfo, securityContext, id, recursive, hardDelete, ADMIN | BOT);
}
private Glossary getGlossary(SecurityContext securityContext, CreateGlossary create) {

View File

@ -381,13 +381,17 @@ public class GlossaryTermResource extends EntityResource<GlossaryTerm, GlossaryT
public Response delete(
@Context UriInfo uriInfo,
@Context SecurityContext securityContext,
@Parameter(description = "Recursively delete this entity and it's children. (Default `false`)")
@DefaultValue("false")
@QueryParam("recursive")
boolean recursive,
@Parameter(description = "Hard delete the entity. (Default = `false`)")
@QueryParam("hardDelete")
@DefaultValue("false")
boolean hardDelete,
@Parameter(description = "Chart Id", schema = @Schema(type = "string")) @PathParam("id") String id)
throws IOException {
return delete(uriInfo, securityContext, id, false, hardDelete, ADMIN | BOT);
return delete(uriInfo, securityContext, id, recursive, hardDelete, ADMIN | BOT);
}
private GlossaryTerm getGlossaryTerm(SecurityContext securityContext, CreateGlossaryTerm create) {

View File

@ -30,6 +30,7 @@ import static org.openmetadata.catalog.Entity.FIELD_FOLLOWERS;
import static org.openmetadata.catalog.Entity.FIELD_OWNER;
import static org.openmetadata.catalog.Entity.FIELD_TAGS;
import static org.openmetadata.catalog.exception.CatalogExceptionMessage.ENTITY_ALREADY_EXISTS;
import static org.openmetadata.catalog.exception.CatalogExceptionMessage.entityIsNotEmpty;
import static org.openmetadata.catalog.exception.CatalogExceptionMessage.entityNotFound;
import static org.openmetadata.catalog.exception.CatalogExceptionMessage.noPermission;
import static org.openmetadata.catalog.exception.CatalogExceptionMessage.notAdmin;
@ -490,7 +491,7 @@ public abstract class EntityResourceTest<T, K> extends CatalogApplicationTest {
assertResponse(
() -> containerTest.deleteEntity(container.getId(), ADMIN_AUTH_HEADERS),
BAD_REQUEST,
container.getType() + " is not empty");
entityIsNotEmpty(container.getType()));
// Now soft-delete the container with recursive flag on
containerTest.deleteEntity(container.getId(), true, false, ADMIN_AUTH_HEADERS);
@ -1324,7 +1325,7 @@ public abstract class EntityResourceTest<T, K> extends CatalogApplicationTest {
target = recursive ? target.queryParam("recursive", true) : target;
target = hardDelete ? target.queryParam("hardDelete", true) : target;
T entity = TestUtils.delete(target, entityClass, authHeaders);
assertResponse(() -> getEntity(id, authHeaders), NOT_FOUND, entityNotFound(entityType, id));
assertEntityDeleted(id, hardDelete);
return entity;
}

View File

@ -21,6 +21,9 @@ import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.openmetadata.catalog.Entity.FIELD_TAGS;
import static org.openmetadata.catalog.Entity.GLOSSARY;
import static org.openmetadata.catalog.Entity.GLOSSARY_TERM;
import static org.openmetadata.catalog.exception.CatalogExceptionMessage.entityIsNotEmpty;
import static org.openmetadata.catalog.exception.CatalogExceptionMessage.glossaryTermMismatch;
import static org.openmetadata.catalog.util.TestUtils.ADMIN_AUTH_HEADERS;
import static org.openmetadata.catalog.util.TestUtils.UpdateType.MINOR_UPDATE;
@ -231,6 +234,55 @@ public class GlossaryTermResourceTest extends EntityResourceTest<GlossaryTerm, C
assertEquals(term1.getTags().get(0).getTagFQN(), PERSONAL_DATA_TAG_LABEL.getTagFQN());
}
@Test
void delete_recursive(TestInfo test) throws IOException {
GlossaryResourceTest glossaryResourceTest = new GlossaryResourceTest();
CreateGlossary createGlossary = glossaryResourceTest.createRequest(getEntityName(test), "", "", null);
Glossary g1 = glossaryResourceTest.createEntity(createGlossary, ADMIN_AUTH_HEADERS);
EntityReference g1Ref = glossaryResourceTest.getEntityInterface(g1).getEntityReference();
// Create glossary term t1 in glossary g1
CreateGlossaryTerm create = createRequest("t1", "", "", null).withGlossary(g1Ref);
GlossaryTerm t1 = createEntity(create, ADMIN_AUTH_HEADERS);
EntityReference tRef1 = new GlossaryTermEntityInterface(t1).getEntityReference();
// Create glossary term t11 under t1
create = createRequest("t11", "", "", null).withReviewers(null).withGlossary(g1Ref).withParent(tRef1);
GlossaryTerm t11 = createEntity(create, ADMIN_AUTH_HEADERS);
EntityReference tRef11 = new GlossaryTermEntityInterface(t11).getEntityReference();
// Create glossary term t111 under t11
create = createRequest("t111", "", "", null).withReviewers(null).withGlossary(g1Ref).withParent(tRef11);
GlossaryTerm t111 = createEntity(create, ADMIN_AUTH_HEADERS);
//
// Glossary that has terms and glossary terms that have children CAN'T BE DELETED without recursive flag
//
// g1 glossary is not empty and can't be deleted
assertResponse(
() -> glossaryResourceTest.deleteEntity(g1.getId(), ADMIN_AUTH_HEADERS),
BAD_REQUEST,
entityIsNotEmpty(GLOSSARY));
// t1 is not empty and can't be deleted
assertResponse(() -> deleteEntity(t1.getId(), ADMIN_AUTH_HEADERS), BAD_REQUEST, entityIsNotEmpty(GLOSSARY_TERM));
// t11 is not empty and can't be deleted
assertResponse(() -> deleteEntity(t11.getId(), ADMIN_AUTH_HEADERS), BAD_REQUEST, entityIsNotEmpty(GLOSSARY_TERM));
//
// Glossary that has terms and glossary terms that have children CAN BE DELETED recursive flag
//
deleteAndCheckEntity(t11, true, true, ADMIN_AUTH_HEADERS); // Delete both t11 and the child t11
assertEntityDeleted(t11.getId(), true);
assertEntityDeleted(t111.getId(), true);
glossaryResourceTest.deleteAndCheckEntity(g1, true, true, ADMIN_AUTH_HEADERS); // Delete the entire glossary
glossaryResourceTest.assertEntityDeleted(g1.getId(), true);
assertEntityDeleted(t1.getId(), true);
}
public GlossaryTerm createTerm(Glossary glossary, GlossaryTerm parent, String termName) throws HttpResponseException {
EntityReference glossaryRef = new GlossaryEntityInterface(glossary).getEntityReference();
EntityReference parentRef = parent != null ? new GlossaryTermEntityInterface(parent).getEntityReference() : null;