From a0b50d328a74d429cee85309cee9c81e15c1000c Mon Sep 17 00:00:00 2001 From: Suresh Srinivas Date: Wed, 6 Apr 2022 17:10:29 -0700 Subject: [PATCH] Fixes #3807 Add DELETE api support for tag and tagCategory (#3898) --- .../mysql/v005__create_db_connection_info.sql | 13 +++ .../mysql/v006__create_db_connection_info.sql | 5 -- .../catalog/jdbi3/CollectionDAO.java | 12 ++- .../catalog/jdbi3/EntityRepository.java | 2 +- .../catalog/jdbi3/TableRepository.java | 3 +- .../catalog/jdbi3/TagCategoryRepository.java | 11 +++ .../catalog/jdbi3/TagRepository.java | 12 +++ .../catalog/resources/tags/TagResource.java | 32 +++++++ .../resources/tags/TagResourceTest.java | 89 +++++++++++++++++-- 9 files changed, 163 insertions(+), 16 deletions(-) delete mode 100644 bootstrap/sql/mysql/v006__create_db_connection_info.sql diff --git a/bootstrap/sql/mysql/v005__create_db_connection_info.sql b/bootstrap/sql/mysql/v005__create_db_connection_info.sql index 1455621448c..a37de9cb320 100644 --- a/bootstrap/sql/mysql/v005__create_db_connection_info.sql +++ b/bootstrap/sql/mysql/v005__create_db_connection_info.sql @@ -145,8 +145,21 @@ CREATE TABLE IF NOT EXISTS database_schema_entity ( INDEX (updatedAt) ); +-- +-- Drop indexes for deleted boolean column +-- Drop unused indexes for updatedAt and updatedBy +-- +RENAME TABLE airflow_pipeline_entity to ingestion_pipeline_entity; + ALTER TABLE tag_category ADD COLUMN id VARCHAR(36) GENERATED ALWAYS AS (json ->> '$.id') STORED NOT NULL FIRST; +UPDATE tag_category +SET json = JSON_SET(json, '$.id', UUID()); + ALTER TABLE tag ADD COLUMN id VARCHAR(36) GENERATED ALWAYS AS (json ->> '$.id') STORED NOT NULL FIRST; + +UPDATE tag +SET json = JSON_SET(json, '$.id', UUID()); + diff --git a/bootstrap/sql/mysql/v006__create_db_connection_info.sql b/bootstrap/sql/mysql/v006__create_db_connection_info.sql deleted file mode 100644 index 707c4815726..00000000000 --- a/bootstrap/sql/mysql/v006__create_db_connection_info.sql +++ /dev/null @@ -1,5 +0,0 @@ --- --- Drop indexes for deleted boolean column --- Drop unused indexes for updatedAt and updatedBy --- -RENAME TABLE airflow_pipeline_entity to ingestion_pipeline_entity; diff --git a/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/CollectionDAO.java b/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/CollectionDAO.java index fd55de7f98b..0e934a0c1ec 100644 --- a/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/CollectionDAO.java +++ b/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/CollectionDAO.java @@ -1252,6 +1252,9 @@ public interface CollectionDAO { default EntityReference getEntityReference(Tag entity) { return null; } + + @SqlUpdate("DELETE FROM tag where fullyQualifiedName LIKE CONCAT(:fqnPrefix, '.%')") + void deleteTagsByPrefix(@Bind("fqnPrefix") String fqnPrefix); } @RegisterRowMapper(TagLabelMapper.class) @@ -1276,10 +1279,13 @@ public interface CollectionDAO { int getTagCount(@Bind("source") int source, @Bind("fqnPrefix") String fqnPrefix); @SqlUpdate("DELETE FROM tag_usage where targetFQN = :targetFQN") - void deleteTags(@Bind("targetFQN") String targetFQN); + void deleteTagsByTarget(@Bind("targetFQN") String targetFQN); - @SqlUpdate("DELETE FROM tag_usage where targetFQN LIKE CONCAT(:fqnPrefix, '%')") - void deleteTagsByPrefix(@Bind("fqnPrefix") String fqnPrefix); + @SqlUpdate("DELETE FROM tag_usage where tagFQN = :tagFQN AND source = :source") + void deleteTagLabels(@Bind("source") int source, @Bind("tagFQN") String tagFQN); + + @SqlUpdate("DELETE FROM tag_usage where tagFQN LIKE CONCAT(:tagFQN, '.%') AND source = :source") + void deleteTagLabelsByPrefix(@Bind("source") int source, @Bind("tagFQN") String tagFQN); class TagLabelMapper implements RowMapper { @Override diff --git a/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/EntityRepository.java b/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/EntityRepository.java index d8c74a0168a..feb270d87dd 100644 --- a/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/EntityRepository.java +++ b/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/EntityRepository.java @@ -993,7 +993,7 @@ public abstract class EntityRepository { } // Remove current entity tags in the database. It will be added back later from the merged tag list. - daoCollection.tagUsageDAO().deleteTags(fqn); + daoCollection.tagUsageDAO().deleteTagsByTarget(fqn); if (operation.isPut()) { // PUT operation merges tags in the request with what already exists diff --git a/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/TableRepository.java b/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/TableRepository.java index 3a5f4913596..a2fad310e61 100644 --- a/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/TableRepository.java +++ b/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/TableRepository.java @@ -1079,7 +1079,8 @@ public class TableRepository extends EntityRepository { } // Delete tags related to deleted columns - deletedColumns.forEach(deleted -> daoCollection.tagUsageDAO().deleteTags(deleted.getFullyQualifiedName())); + deletedColumns.forEach( + deleted -> daoCollection.tagUsageDAO().deleteTagsByTarget(deleted.getFullyQualifiedName())); // Add tags related to newly added columns for (Column added : addedColumns) { diff --git a/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/TagCategoryRepository.java b/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/TagCategoryRepository.java index f9c6aa495dd..e84f537d050 100644 --- a/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/TagCategoryRepository.java +++ b/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/TagCategoryRepository.java @@ -20,6 +20,7 @@ import java.sql.SQLException; import java.util.ArrayList; import java.util.List; import java.util.UUID; +import javax.ws.rs.core.UriInfo; import lombok.extern.slf4j.Slf4j; import org.jdbi.v3.core.mapper.RowMapper; import org.jdbi.v3.sqlobject.transaction.Transaction; @@ -117,6 +118,16 @@ public class TagCategoryRepository extends EntityRepository { return daoCollection.tagUsageDAO().getTagCount(Source.TAG.ordinal(), category.getName()); } + @Transaction + public TagCategory delete(UriInfo uriInfo, String id) throws IOException { + TagCategory category = get(uriInfo, id, Fields.EMPTY_FIELDS, Include.NON_DELETED); + dao.delete(id); + daoCollection.tagDAO().deleteTagsByPrefix(category.getName()); + daoCollection.tagUsageDAO().deleteTagLabels(Source.TAG.ordinal(), category.getName()); + daoCollection.tagUsageDAO().deleteTagLabelsByPrefix(Source.TAG.ordinal(), category.getName()); + return category; + } + public static class TagLabelMapper implements RowMapper { @Override public TagLabel map(ResultSet r, org.jdbi.v3.core.statement.StatementContext ctx) throws SQLException { diff --git a/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/TagRepository.java b/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/TagRepository.java index 1b5a450cfa8..33f5987bb7f 100644 --- a/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/TagRepository.java +++ b/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/TagRepository.java @@ -20,7 +20,9 @@ import java.net.URI; import java.util.ArrayList; import java.util.List; import java.util.UUID; +import javax.ws.rs.core.UriInfo; import lombok.extern.slf4j.Slf4j; +import org.jdbi.v3.sqlobject.transaction.Transaction; import org.openmetadata.catalog.Entity; import org.openmetadata.catalog.resources.tags.TagResource; import org.openmetadata.catalog.type.ChangeDescription; @@ -124,6 +126,16 @@ public class TagRepository extends EntityRepository { return daoCollection.tagUsageDAO().getTagCount(Source.TAG.ordinal(), tag.getFullyQualifiedName()); } + @Transaction + public Tag delete(UriInfo uriInfo, String id) throws IOException { + Tag tag = get(uriInfo, id, Fields.EMPTY_FIELDS, Include.NON_DELETED); + dao.delete(id); + daoCollection.tagDAO().deleteTagsByPrefix(tag.getFullyQualifiedName()); + daoCollection.tagUsageDAO().deleteTagLabels(Source.TAG.ordinal(), tag.getFullyQualifiedName()); + daoCollection.tagUsageDAO().deleteTagLabelsByPrefix(Source.TAG.ordinal(), tag.getFullyQualifiedName()); + return tag; + } + public static class TagEntityInterface extends EntityInterface { public TagEntityInterface(Tag entity) { super(Entity.TAG, entity); diff --git a/catalog-rest-service/src/main/java/org/openmetadata/catalog/resources/tags/TagResource.java b/catalog-rest-service/src/main/java/org/openmetadata/catalog/resources/tags/TagResource.java index e697ecc6f31..1c63a684d4c 100644 --- a/catalog-rest-service/src/main/java/org/openmetadata/catalog/resources/tags/TagResource.java +++ b/catalog-rest-service/src/main/java/org/openmetadata/catalog/resources/tags/TagResource.java @@ -31,6 +31,7 @@ import java.util.Objects; import java.util.UUID; import java.util.regex.Pattern; import javax.validation.Valid; +import javax.ws.rs.DELETE; import javax.ws.rs.GET; import javax.ws.rs.POST; import javax.ws.rs.PUT; @@ -465,6 +466,37 @@ public class TagResource { return Response.ok(tag).build(); } + @DELETE + @Path("/{id}") + @Operation( + summary = "Delete tag category", + tags = "tags", + description = "Delete a tag category and all the tags under it.") + public TagCategory deleteCategory( + @Context UriInfo uriInfo, + @Context SecurityContext securityContext, + @Parameter(description = "Tag category id", schema = @Schema(type = "string")) @PathParam("id") String id) + throws IOException { + SecurityUtil.authorizeAdmin(authorizer, securityContext, ADMIN | BOT); + TagCategory tagCategory = daoCategory.delete(uriInfo, id); + return addHref(uriInfo, tagCategory); + } + + @DELETE + @Path("/{category}/{id}") + @Operation(summary = "Delete tag", tags = "tags", description = "Delete a tag and all the tags under it.") + public Tag deleteTags( + @Context UriInfo uriInfo, + @Context SecurityContext securityContext, + @Parameter(description = "Tag id", schema = @Schema(type = "string")) @PathParam("category") String category, + @Parameter(description = "Tag id", schema = @Schema(type = "string")) @PathParam("id") String id) + throws IOException { + SecurityUtil.authorizeAdmin(authorizer, securityContext, ADMIN | BOT); + Tag tag = dao.delete(uriInfo, id); + URI categoryHref = RestUtil.getHref(uriInfo, TAG_COLLECTION_PATH, category); + return addHref(categoryHref, tag); + } + private TagCategory addHref(UriInfo uriInfo, TagCategory category) { category.setHref(RestUtil.getHref(uriInfo, TAG_COLLECTION_PATH, category.getName())); addHref(category.getHref(), category.getChildren()); diff --git a/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/tags/TagResourceTest.java b/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/tags/TagResourceTest.java index fb268c01120..0e0336da535 100644 --- a/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/tags/TagResourceTest.java +++ b/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/tags/TagResourceTest.java @@ -128,16 +128,91 @@ public class TagResourceTest extends CatalogApplicationTest { } @Test - void post_validTagCategory_as_admin_201(TestInfo test) throws HttpResponseException { + void post_delete_validTagCategory_as_admin_201(TestInfo test) throws HttpResponseException, JsonProcessingException { // POST .../tags/{newCategory} returns 201 - String categoryName = test.getDisplayName().substring(0, 10); // Form a unique category name based on the test name + String categoryName = test.getDisplayName().substring(0, 20); // Form a unique category name based on the test name CreateTagCategory create = new CreateTagCategory() .withName(categoryName) .withDescription("description") .withCategoryType(TagCategoryType.Descriptive); - TagCategory newCategory = createAndCheckCategory(create, ADMIN_AUTH_HEADERS); - assertEquals(0, newCategory.getChildren().size()); + TagCategory category = createAndCheckCategory(create, ADMIN_AUTH_HEADERS); + assertEquals(0, category.getChildren().size()); + + CreateTag createTag = new CreateTag().withName("PrimaryTag").withDescription("description"); + Tag primaryTag = createPrimaryTag(category.getName(), createTag, ADMIN_AUTH_HEADERS); + + // POST .../tags/{category}/{primaryTag}/{secondaryTag} to create secondary tag + createTag = new CreateTag().withName("SecondaryTag").withDescription("description"); + Tag secondaryTag = createSecondaryTag(category.getName(), "PrimaryTag", createTag, ADMIN_AUTH_HEADERS); + + // Now delete Tag category and ensure the tag category and tags under it are all deleted + deleteCategory(category, ADMIN_AUTH_HEADERS); + assertResponse( + () -> getTag(primaryTag.getFullyQualifiedName(), ADMIN_AUTH_HEADERS), + NOT_FOUND, + entityNotFound(Entity.TAG, primaryTag.getFullyQualifiedName())); + assertResponse( + () -> getTag(secondaryTag.getFullyQualifiedName(), ADMIN_AUTH_HEADERS), + NOT_FOUND, + entityNotFound(Entity.TAG, secondaryTag.getFullyQualifiedName())); + } + + @Test + void post_delete_validTags_as_admin_201(TestInfo test) throws HttpResponseException, JsonProcessingException { + // Create tag category + String categoryName = test.getDisplayName().substring(0, 20); + CreateTagCategory create = + new CreateTagCategory() + .withName(categoryName) + .withDescription("description") + .withCategoryType(TagCategoryType.Descriptive); + TagCategory category = createAndCheckCategory(create, ADMIN_AUTH_HEADERS); + assertEquals(0, category.getChildren().size()); + + // Create primaryTag + CreateTag createTag = new CreateTag().withName("PrimaryTag").withDescription("description"); + Tag primaryTag = createPrimaryTag(category.getName(), createTag, ADMIN_AUTH_HEADERS); + + // Create secondaryTag1 + createTag = new CreateTag().withName("SecondaryTag1").withDescription("description"); + Tag secondaryTag1 = createSecondaryTag(category.getName(), "PrimaryTag", createTag, ADMIN_AUTH_HEADERS); + + // Create secondaryTag2 + createTag = new CreateTag().withName("SecondaryTag2").withDescription("description"); + Tag secondaryTag2 = createSecondaryTag(category.getName(), "PrimaryTag", createTag, ADMIN_AUTH_HEADERS); + + // Delete and verify secondaryTag2 is deleted + deleteTag(category.getName(), secondaryTag2, ADMIN_AUTH_HEADERS); + + // Delete primaryTag and ensure secondaryTag1 under it is deleted + deleteTag(category.getName(), primaryTag, ADMIN_AUTH_HEADERS); + assertResponse( + () -> getTag(secondaryTag1.getFullyQualifiedName(), ADMIN_AUTH_HEADERS), + NOT_FOUND, + entityNotFound(Entity.TAG, secondaryTag1.getFullyQualifiedName())); + } + + public final TagCategory deleteCategory(TagCategory category, Map authHeaders) + throws HttpResponseException { + WebTarget target = getResource("tags/" + category.getId()); + TagCategory deletedCategory = TestUtils.delete(target, TagCategory.class, authHeaders); + assertResponse( + () -> getCategory(category.getName(), ADMIN_AUTH_HEADERS), + NOT_FOUND, + entityNotFound(Entity.TAG_CATEGORY, category.getName())); + return deletedCategory; + } + + public final Tag deleteTag(String categoryName, Tag tag, Map authHeaders) + throws HttpResponseException { + WebTarget target = getResource("tags/" + categoryName + "/" + tag.getId()); + Tag deletedTag = TestUtils.delete(target, Tag.class, authHeaders); + assertResponse( + () -> getTag(tag.getFullyQualifiedName(), ADMIN_AUTH_HEADERS), + NOT_FOUND, + entityNotFound(Entity.TAG, tag.getFullyQualifiedName())); + return deletedTag; } @Test @@ -323,7 +398,7 @@ public class TagResourceTest extends CatalogApplicationTest { return category; } - private void createPrimaryTag(String category, CreateTag create, Map authHeaders) + private Tag createPrimaryTag(String category, CreateTag create, Map authHeaders) throws HttpResponseException, JsonProcessingException { String updatedBy = TestUtils.getPrincipal(authHeaders); WebTarget target = getResource("tags/" + category); @@ -347,9 +422,10 @@ public class TagResourceTest extends CatalogApplicationTest { create.getDescription(), create.getAssociatedTags(), updatedBy); + return returnedTag; } - private void createSecondaryTag(String category, String primaryTag, CreateTag create, Map authHeaders) + private Tag createSecondaryTag(String category, String primaryTag, CreateTag create, Map authHeaders) throws HttpResponseException, JsonProcessingException { String updatedBy = TestUtils.getPrincipal(authHeaders); WebTarget target = getResource("tags/" + category + "/" + primaryTag); @@ -373,6 +449,7 @@ public class TagResourceTest extends CatalogApplicationTest { create.getDescription(), create.getAssociatedTags(), updatedBy); + return returnedTag; } @SneakyThrows