From b3e02741e8e5414899a5fde891736c377814a39f Mon Sep 17 00:00:00 2001 From: sureshms Date: Sat, 6 Nov 2021 15:01:05 -0700 Subject: [PATCH] Fixes #1089 Code cleanup - Move comparators and matchers to EntityUtil --- .../catalog/jdbi3/DashboardRepository.java | 2 +- .../catalog/jdbi3/EntityRepository.java | 18 ++----- .../catalog/jdbi3/PipelineRepository.java | 5 +- .../catalog/jdbi3/TableRepository.java | 24 ++------- .../catalog/jdbi3/TeamRepository.java | 6 +-- .../catalog/jdbi3/UserRepository.java | 8 ++- .../openmetadata/catalog/util/EntityUtil.java | 54 +++++++++++++++++-- .../catalog/resources/EntityResourceTest.java | 5 +- .../dashboards/DashboardResourceTest.java | 2 +- .../databases/TableResourceTest.java | 32 +++++------ .../resources/models/ModelResourceTest.java | 2 +- .../pipelines/PipelineResourceTest.java | 4 +- .../resources/teams/TeamResourceTest.java | 7 ++- .../resources/teams/UserResourceTest.java | 7 ++- .../resources/topics/TopicResourceTest.java | 4 +- .../openmetadata/catalog/util/TestUtils.java | 6 +-- 16 files changed, 101 insertions(+), 85 deletions(-) diff --git a/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/DashboardRepository.java b/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/DashboardRepository.java index 3150fa6029e..b1519343b7e 100644 --- a/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/DashboardRepository.java +++ b/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/DashboardRepository.java @@ -360,7 +360,7 @@ public class DashboardRepository extends EntityRepository { List added = new ArrayList<>(); List deleted = new ArrayList<>(); - recordListChange("charts", origCharts, updatedCharts, added, deleted, entityReferenceMatch); + recordListChange("charts", origCharts, updatedCharts, added, deleted, EntityUtil.entityReferenceMatch); } } } 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 50e54bc6fb9..71533418994 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 @@ -27,7 +27,6 @@ import java.security.GeneralSecurityException; import java.text.ParseException; import java.util.ArrayList; import java.util.Collections; -import java.util.Comparator; import java.util.Date; import java.util.List; import java.util.Locale; @@ -139,7 +138,7 @@ public abstract class EntityRepository { T latest = setFields(dao.findEntityById(UUID.fromString(id)), putFields); String extensionPrefix = EntityUtil.getVersionExtensionPrefix(entityName); List oldVersions = daoCollection.entityExtensionDAO().getEntityVersions(id, extensionPrefix); - oldVersions.sort(Comparator.comparing(EntityVersionPair::getVersion).reversed()); + oldVersions.sort(EntityUtil.compareVersion.reversed()); final List allVersions = new ArrayList<>(); allVersions.add(JsonUtils.pojoToJson(latest)); @@ -162,7 +161,6 @@ public abstract class EntityRepository { } // Update the existing entity setFields(original, putFields); - validate(updated); EntityUpdater entityUpdater = getUpdater(original, updated, false); entityUpdater.update(); @@ -304,8 +302,8 @@ public abstract class EntityRepository { List addedTags = new ArrayList<>(); List deletedTags = new ArrayList<>(); - recordListChange(fieldName, origTags, updatedTags, addedTags, deletedTags, tagLabelMatch); - updatedTags.sort(Comparator.comparing(TagLabel::getTagFQN)); + recordListChange(fieldName, origTags, updatedTags, addedTags, deletedTags, EntityUtil.tagLabelMatch); + updatedTags.sort(EntityUtil.compareTagLabel); EntityUtil.applyTags(daoCollection.tagDAO(), updatedTags, fqn); } @@ -360,16 +358,15 @@ public abstract class EntityRepository { List deletedItems, BiPredicate typeMatch) throws JsonProcessingException { for (K stored : origList) { - // Find updated column matching name, data type and ordinal position + // If an entry in the original list is not in updated list, then it is deleted during update K updated = updatedList.stream().filter(c -> typeMatch.test(c, stored)).findAny().orElse(null); if (updated == null) { deletedItems.add(stored); } } - // Carry forward the user generated metadata from existing columns to new columns for (K updated : updatedList) { - // Find stored column matching name, data type and ordinal position + // If an entry in the updated list is not in original list, then it is added during update K stored = origList.stream().filter(c -> typeMatch.test(c, updated)).findAny().orElse(null); if (stored == null) { // New column added addedItems.add(updated); @@ -398,9 +395,4 @@ public abstract class EntityRepository { } } - public static BiPredicate entityReferenceMatch = (ref1, ref2) - -> ref1.getId().equals(ref2.getId()); - - public static BiPredicate tagLabelMatch = (tag1, tag2) - -> tag1.getTagFQN().equals(tag2.getTagFQN()); } diff --git a/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/PipelineRepository.java b/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/PipelineRepository.java index ab86ec6e5f9..2662a7826c2 100644 --- a/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/PipelineRepository.java +++ b/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/PipelineRepository.java @@ -42,7 +42,6 @@ import java.util.List; import java.util.Objects; import java.util.Optional; import java.util.UUID; -import java.util.function.BiPredicate; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -318,10 +317,8 @@ public class PipelineRepository extends EntityRepository { List added = new ArrayList<>(); List deleted = new ArrayList<>(); - recordListChange("tasks", origTasks, updatedTasks, added, deleted, taskMatch); + recordListChange("tasks", origTasks, updatedTasks, added, deleted, EntityUtil.taskMatch); } } - public static BiPredicate taskMatch = (task1, task2) - -> task1.getName().equals(task2.getName()); } 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 adb58738d5b..76152df84af 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 @@ -58,7 +58,6 @@ import java.util.List; import java.util.ListIterator; import java.util.Map; import java.util.Map.Entry; -import java.util.Objects; import java.util.Optional; import java.util.UUID; import java.util.function.BiPredicate; @@ -667,7 +666,7 @@ public class TableRepository extends EntityRepository { Table updatedTable = updated.getEntity(); updateConstraints(origTable, updatedTable); updateTableType(origTable, updatedTable); - updateColumns("columns", origTable.getColumns(), updated.getEntity().getColumns(), columnMatch); + updateColumns("columns", origTable.getColumns(), updated.getEntity().getColumns(), EntityUtil.columnMatch); } private void updateConstraints(Table origTable, Table updatedTable) throws JsonProcessingException { @@ -676,16 +675,16 @@ public class TableRepository extends EntityRepository
{ List updatedConstraints = Optional.ofNullable(updatedTable.getTableConstraints()) .orElse(Collections.emptyList()); - origConstraints.sort(Comparator.comparing(TableConstraint::getConstraintType)); + origConstraints.sort(EntityUtil.compareTableConstraint); origConstraints.stream().map(TableConstraint::getColumns).forEach(Collections::sort); - updatedConstraints.sort(Comparator.comparing(TableConstraint::getConstraintType)); + updatedConstraints.sort(EntityUtil.compareTableConstraint); updatedConstraints.stream().map(TableConstraint::getColumns).forEach(Collections::sort); List added = new ArrayList<>(); List deleted = new ArrayList<>(); recordListChange("tableConstraints", origConstraints, updatedConstraints, added, deleted, - tableConstraintMatch); + EntityUtil.tableConstraintMatch); } private void updateTableType(Table origTable, Table updatedTable) throws JsonProcessingException { @@ -709,7 +708,7 @@ public class TableRepository extends EntityRepository
{ // Carry forward the user generated metadata from existing columns to new columns for (Column updated : updatedColumns) { // Find stored column matching name, data type and ordinal position - Column stored = origColumns.stream().filter(c -> TableRepository.this.columnMatch.test(c, updated)).findAny().orElse(null); + Column stored = origColumns.stream().filter(c -> EntityUtil.columnMatch.test(c, updated)).findAny().orElse(null); if (stored == null) { // New column added continue; } @@ -753,17 +752,4 @@ public class TableRepository extends EntityRepository
{ } } - BiPredicate columnMatch = (column1, column2) -> { - /* you CAN compare apples and oranges */ - return column1.getName().equals(column2.getName()) && - column1.getDataType() == column2.getDataType() && - column1.getArrayDataType() == column2.getArrayDataType() && - Objects.equals(column1.getOrdinalPosition(), column2.getOrdinalPosition()); - }; - - BiPredicate tableConstraintMatch = (constraint1, constraint2) -> { - /* you CAN compare apples and oranges */ - return constraint1.getConstraintType() == constraint2.getConstraintType() && - constraint1.getColumns().equals(constraint2.getColumns()); - }; } diff --git a/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/TeamRepository.java b/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/TeamRepository.java index 9baf7fb05e2..bfc0d2d68ef 100644 --- a/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/TeamRepository.java +++ b/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/TeamRepository.java @@ -36,7 +36,6 @@ import java.net.URI; import java.text.ParseException; import java.util.ArrayList; import java.util.Collections; -import java.util.Comparator; import java.util.Date; import java.util.List; import java.util.Optional; @@ -276,9 +275,8 @@ public class TeamRepository extends EntityRepository { "team", "user", Relationship.CONTAINS.ordinal()); } - // Sort by user Id as string (as done in the database) - updatedUsers.sort(Comparator.comparing(entityReference -> entityReference.getId().toString())); - origUsers.sort(Comparator.comparing(entityReference -> entityReference.getId().toString())); + updatedUsers.sort(EntityUtil.compareEntityReference); + origUsers.sort(EntityUtil.compareEntityReference); recordChange("users", origUsers.isEmpty() ? null : origUsers, updatedUsers.isEmpty() ? null : updatedUsers); } } diff --git a/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/UserRepository.java b/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/UserRepository.java index 5be145d2ced..d67ca0f837b 100644 --- a/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/UserRepository.java +++ b/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/UserRepository.java @@ -37,7 +37,6 @@ import java.net.URI; import java.text.ParseException; import java.util.ArrayList; import java.util.Collections; -import java.util.Comparator; import java.util.Date; import java.util.List; import java.util.Optional; @@ -312,13 +311,12 @@ public class UserRepository extends EntityRepository { List origTeams = Optional.ofNullable(origUser.getTeams()).orElse(Collections.emptyList()); List updatedTeams = Optional.ofNullable(updatedUser.getTeams()).orElse(Collections.emptyList()); - // Sort by team Id as string (as done in the database) - origTeams.sort(Comparator.comparing(entityReference -> entityReference.getId().toString())); - updatedTeams.sort(Comparator.comparing(entityReference -> entityReference.getId().toString())); + origTeams.sort(EntityUtil.compareEntityReference); + updatedTeams.sort(EntityUtil.compareEntityReference); List added = new ArrayList<>(); List deleted = new ArrayList<>(); - recordListChange("teams", origTeams, updatedTeams, added, deleted, entityReferenceMatch); + recordListChange("teams", origTeams, updatedTeams, added, deleted, EntityUtil.entityReferenceMatch); } } } diff --git a/catalog-rest-service/src/main/java/org/openmetadata/catalog/util/EntityUtil.java b/catalog-rest-service/src/main/java/org/openmetadata/catalog/util/EntityUtil.java index 60c94a6d4e3..1d9aa92a9d9 100644 --- a/catalog-rest-service/src/main/java/org/openmetadata/catalog/util/EntityUtil.java +++ b/catalog-rest-service/src/main/java/org/openmetadata/catalog/util/EntityUtil.java @@ -25,6 +25,7 @@ import org.openmetadata.catalog.exception.CatalogExceptionMessage; import org.openmetadata.catalog.exception.EntityNotFoundException; import org.openmetadata.catalog.jdbi3.CollectionDAO; import org.openmetadata.catalog.jdbi3.CollectionDAO.EntityRelationshipDAO; +import org.openmetadata.catalog.jdbi3.CollectionDAO.EntityVersionPair; import org.openmetadata.catalog.jdbi3.CollectionDAO.TagDAO; import org.openmetadata.catalog.jdbi3.CollectionDAO.TeamDAO; import org.openmetadata.catalog.jdbi3.CollectionDAO.UsageDAO; @@ -47,10 +48,14 @@ import org.openmetadata.catalog.resources.services.storage.StorageServiceResourc import org.openmetadata.catalog.resources.teams.TeamResource; import org.openmetadata.catalog.resources.teams.UserResource; import org.openmetadata.catalog.resources.topics.TopicResource; +import org.openmetadata.catalog.type.Column; import org.openmetadata.catalog.type.EntityReference; +import org.openmetadata.catalog.type.FieldChange; +import org.openmetadata.catalog.type.TableConstraint; import org.openmetadata.catalog.type.Tag; import org.openmetadata.catalog.type.TagLabel; import org.openmetadata.catalog.type.TagLabel.LabelType; +import org.openmetadata.catalog.type.Task; import org.openmetadata.catalog.type.UsageDetails; import org.openmetadata.catalog.type.UsageStats; import org.openmetadata.catalog.type.Schedule; @@ -66,14 +71,54 @@ import java.util.Collections; import java.util.Comparator; import java.util.Date; import java.util.List; +import java.util.Objects; import java.util.Optional; import java.util.UUID; +import java.util.function.BiPredicate; import java.util.stream.Collectors; import java.util.stream.Stream; public final class EntityUtil { private static final Logger LOG = LoggerFactory.getLogger(EntityUtil.class); + // + // Comparators used for sorting list based on the given type + // + + // Note ordering is same as server side ordering by ID as string to ensure PATCH operations work + public static final Comparator compareEntityReference = + Comparator.comparing(entityReference -> entityReference.getId().toString()); + public static final Comparator compareVersion = + Comparator.comparing(EntityVersionPair::getVersion); + public static final Comparator compareTagLabel = + Comparator.comparing(TagLabel::getTagFQN); + public static final Comparator compareFieldChange = + Comparator.comparing(FieldChange::getName); + public static final Comparator compareTableConstraint = + Comparator.comparing(TableConstraint::getConstraintType); + + // + // Matchers used for matching two items in a list + // + public static BiPredicate entityReferenceMatch = (ref1, ref2) -> + ref1.getId().equals(ref2.getId()); + + public static BiPredicate tagLabelMatch = (tag1, tag2) -> + tag1.getTagFQN().equals(tag2.getTagFQN()); + + public static BiPredicate taskMatch = (task1, task2) -> + task1.getName().equals(task2.getName()); + + public static BiPredicate columnMatch = (column1, column2) -> + column1.getName().equals(column2.getName()) && + column1.getDataType() == column2.getDataType() && + column1.getArrayDataType() == column2.getArrayDataType() && + Objects.equals(column1.getOrdinalPosition(), column2.getOrdinalPosition()); + + public static BiPredicate tableConstraintMatch = (constraint1, constraint2) -> + constraint1.getConstraintType() == constraint2.getConstraintType() && + constraint1.getColumns().equals(constraint2.getColumns()); + private EntityUtil() { } @@ -200,7 +245,7 @@ public final class EntityUtil { if (ids.size() > 1) { LOG.warn("Possible database issues - multiple owners {} found for entity {}", ids, id); } - return ids.isEmpty() ? null : EntityUtil.populateOwner(userDAO, teamDAO, ids.get(0)); + return ids.isEmpty() ? null : populateOwner(userDAO, teamDAO, ids.get(0)); } public static EntityReference populateOwner(UserDAO userDAO, TeamDAO teamDAO, @@ -395,9 +440,9 @@ public final class EntityUtil { // Apply derived tags List derivedTags = getDerivedTags(tagLabel, tag); - updatedTagLabels = EntityUtil.mergeTags(updatedTagLabels, derivedTags); + updatedTagLabels = mergeTags(updatedTagLabels, derivedTags); } - updatedTagLabels.sort(Comparator.comparing(TagLabel::getTagFQN)); + updatedTagLabels.sort(compareTagLabel); return updatedTagLabels; } @@ -485,7 +530,7 @@ public final class EntityUtil { if (refList == null) { return null; } - return refList.stream().sorted(Comparator.comparing(EntityReference::getId)).map(EntityReference::getId) + return refList.stream().sorted(compareEntityReference).map(EntityReference::getId) .collect(Collectors.toList()); } @@ -514,4 +559,5 @@ public final class EntityUtil { localColumnName.append(s[s.length - 1]); return localColumnName.toString(); } + } diff --git a/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/EntityResourceTest.java b/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/EntityResourceTest.java index 3923a3396cc..2422bd82932 100644 --- a/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/EntityResourceTest.java +++ b/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/EntityResourceTest.java @@ -32,6 +32,7 @@ import org.openmetadata.catalog.type.EntityReference; import org.openmetadata.catalog.type.FieldChange; import org.openmetadata.catalog.type.TagLabel; import org.openmetadata.catalog.util.EntityInterface; +import org.openmetadata.catalog.util.EntityUtil; import org.openmetadata.catalog.util.JsonUtils; import org.openmetadata.catalog.util.ResultList; import org.openmetadata.catalog.util.TestUtils; @@ -710,8 +711,8 @@ public abstract class EntityResourceTest extends CatalogApplicationTest { protected final void assertFieldLists(List expectedList, List actualList) throws IOException { - expectedList.sort(Comparator.comparing(FieldChange::getName)); - actualList.sort(Comparator.comparing(FieldChange::getName)); + expectedList.sort(EntityUtil.compareFieldChange); + actualList.sort(EntityUtil.compareFieldChange); assertEquals(expectedList.size(), actualList.size()); for (int i = 0; i < expectedList.size(); i++) { diff --git a/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/dashboards/DashboardResourceTest.java b/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/dashboards/DashboardResourceTest.java index d81c153d6fd..065dd4c5cab 100644 --- a/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/dashboards/DashboardResourceTest.java +++ b/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/dashboards/DashboardResourceTest.java @@ -388,7 +388,7 @@ public class DashboardResourceTest extends EntityResourceTest { TestUtils.getPrincipal(authHeaders), createRequest.getOwner()); assertService(createRequest.getService(), dashboard.getService()); validateDashboardCharts(dashboard, createRequest.getCharts()); - TestUtils.validateTags(dashboard.getFullyQualifiedName(), createRequest.getTags(), dashboard.getTags()); + TestUtils.assertTags(dashboard.getFullyQualifiedName(), createRequest.getTags(), dashboard.getTags()); } @Override diff --git a/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/databases/TableResourceTest.java b/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/databases/TableResourceTest.java index 544f1e00b96..cfb031cd3d3 100644 --- a/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/databases/TableResourceTest.java +++ b/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/databases/TableResourceTest.java @@ -346,7 +346,7 @@ public class TableResourceTest extends EntityResourceTest
{ c2_c_d = c2_c.getChildren().get(0); c2_c_d.setTags(singletonList(USER_BANK_ACCOUNT_TAG_LABEL)); // c2.c.d new tag added table1 = patchEntity(table1.getId(), tableJson, table1, adminAuthHeaders()); - validateColumns(Arrays.asList(c1, c2), table1.getColumns()); + assertColumns(Arrays.asList(c1, c2), table1.getColumns()); } @Test @@ -586,7 +586,7 @@ public class TableResourceTest extends EntityResourceTest
{ // getTable and ensure the following column joins are correct table1 = getTable(table1.getId(), "joins", adminAuthHeaders()); - validateColumnJoins(expectedJoins1, table1.getJoins()); + assertColumnJoins(expectedJoins1, table1.getJoins()); // getTable and ensure the following column joins are correct table2 = getTable(table2.getId(), "joins", adminAuthHeaders()); @@ -600,7 +600,7 @@ public class TableResourceTest extends EntityResourceTest
{ // table2.c3 is joined with table1.c1 with join count 30 new ColumnJoin().withColumnName("c3").withJoinedWith(singletonList( new JoinedWith().withFullyQualifiedName(t1c3).withJoinCount(30 * i)))); - validateColumnJoins(expectedJoins2, table2.getJoins()); + assertColumnJoins(expectedJoins2, table2.getJoins()); // getTable and ensure the following column joins table3 = getTable(table3.getId(), "joins", adminAuthHeaders()); @@ -614,7 +614,7 @@ public class TableResourceTest extends EntityResourceTest
{ // table3.c3 is joined with table1.c1 with join count 30 new ColumnJoin().withColumnName("c3").withJoinedWith(singletonList( new JoinedWith().withFullyQualifiedName(t1c3).withJoinCount(30 * i)))); - validateColumnJoins(expectedJoins3, table3.getJoins()); + assertColumnJoins(expectedJoins3, table3.getJoins()); // Report again for the previous day and make sure aggregate counts are correct table1Joins = new TableJoins().withDayCount(1).withStartDate(RestUtil.today(-1)) @@ -666,7 +666,7 @@ public class TableResourceTest extends EntityResourceTest
{ assertResponse(exception, BAD_REQUEST, "Date range can only include past 30 days starting today"); } - public void validateColumnJoins(List expected, TableJoins actual) throws ParseException { + public void assertColumnJoins(List expected, TableJoins actual) throws ParseException { // Table reports last 30 days of aggregated join count assertEquals(actual.getStartDate(), getDateStringByOffset(DATE_FORMAT, RestUtil.today(0), -30)); assertEquals(actual.getDayCount(), 30); @@ -1010,7 +1010,7 @@ public class TableResourceTest extends EntityResourceTest
{ String originalJson = JsonUtils.pojoToJson(table); table.setColumns(columns); table = patchEntityAndCheck(table, originalJson, adminAuthHeaders(), MINOR_UPDATE, change); - validateColumns(columns, table.getColumns()); + assertColumns(columns, table.getColumns()); } @Test @@ -1123,7 +1123,7 @@ public class TableResourceTest extends EntityResourceTest
{ } - private static void validateColumn(Column expectedColumn, Column actualColumn) throws HttpResponseException { + private static void assertColumn(Column expectedColumn, Column actualColumn) throws HttpResponseException { assertNotNull(actualColumn.getFullyQualifiedName()); assertEquals(expectedColumn.getName(), actualColumn.getName()); assertEquals(expectedColumn.getDescription(), actualColumn.getDescription()); @@ -1133,13 +1133,13 @@ public class TableResourceTest extends EntityResourceTest
{ if (expectedColumn.getDataTypeDisplay() != null) { assertEquals(expectedColumn.getDataTypeDisplay().toLowerCase(Locale.ROOT), actualColumn.getDataTypeDisplay()); } - TestUtils.validateTags(actualColumn.getFullyQualifiedName(), expectedColumn.getTags(), actualColumn.getTags()); + TestUtils.assertTags(actualColumn.getFullyQualifiedName(), expectedColumn.getTags(), actualColumn.getTags()); // Check the nested columns - validateColumns(expectedColumn.getChildren(), actualColumn.getChildren()); + assertColumns(expectedColumn.getChildren(), actualColumn.getChildren()); } - private static void validateColumns(List expectedColumns, List actualColumns) + private static void assertColumns(List expectedColumns, List actualColumns) throws HttpResponseException { if (expectedColumns == null && actualColumns == null) { return; @@ -1148,7 +1148,7 @@ public class TableResourceTest extends EntityResourceTest
{ assertNotNull(expectedColumns); assertEquals(expectedColumns.size(), actualColumns.size()); for (int i = 0; i < expectedColumns.size(); i++) { - validateColumn(expectedColumns.get(i), actualColumns.get(i)); + assertColumn(expectedColumns.get(i), actualColumns.get(i)); } } @@ -1262,10 +1262,10 @@ public class TableResourceTest extends EntityResourceTest
{ // Entity specific validation assertEquals(createRequest.getTableType(), createdEntity.getTableType()); - validateColumns(createRequest.getColumns(), createdEntity.getColumns()); + assertColumns(createRequest.getColumns(), createdEntity.getColumns()); validateDatabase(createRequest.getDatabase(), createdEntity.getDatabase()); assertEquals(createRequest.getTableConstraints(), createdEntity.getTableConstraints()); - TestUtils.validateTags(createdEntity.getFullyQualifiedName(), createRequest.getTags(), createdEntity.getTags()); + TestUtils.assertTags(createdEntity.getFullyQualifiedName(), createRequest.getTags(), createdEntity.getTags()); TestUtils.validateEntityReference(createdEntity.getFollowers()); } @@ -1282,10 +1282,10 @@ public class TableResourceTest extends EntityResourceTest
{ // Entity specific validation assertEquals(expected.getTableType(), patched.getTableType()); - validateColumns(expected.getColumns(), patched.getColumns()); + assertColumns(expected.getColumns(), patched.getColumns()); validateDatabase(expected.getDatabase().getId(), patched.getDatabase()); assertEquals(expected.getTableConstraints(), patched.getTableConstraints()); - TestUtils.validateTags(expected.getFullyQualifiedName(), expected.getTags(), patched.getTags()); + TestUtils.assertTags(expected.getFullyQualifiedName(), expected.getTags(), patched.getTags()); TestUtils.validateEntityReference(expected.getFollowers()); } @@ -1316,7 +1316,7 @@ public class TableResourceTest extends EntityResourceTest
{ } else if (fieldName.contains("columns") && !fieldName.endsWith("tags") && !fieldName.endsWith("description")) { List expectedRefs = (List) expected; List actualRefs = JsonUtils.readObjects(actual.toString(), Column.class); - validateColumns(expectedRefs, actualRefs); + assertColumns(expectedRefs, actualRefs); } else if (fieldName.endsWith("tableType")) { TableType expectedTableType = (TableType) expected; TableType actualTableType = TableType.fromValue(actual.toString()); diff --git a/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/models/ModelResourceTest.java b/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/models/ModelResourceTest.java index fee67be050c..52120628182 100644 --- a/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/models/ModelResourceTest.java +++ b/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/models/ModelResourceTest.java @@ -527,7 +527,7 @@ public class ModelResourceTest extends CatalogApplicationTest { assertNotNull(model.getOwner().getHref()); } - TestUtils.validateTags(model.getFullyQualifiedName(), expectedTags, model.getTags()); + TestUtils.assertTags(model.getFullyQualifiedName(), expectedTags, model.getTags()); return model; } diff --git a/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/pipelines/PipelineResourceTest.java b/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/pipelines/PipelineResourceTest.java index 4a8091029ac..ba7f1a9705c 100644 --- a/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/pipelines/PipelineResourceTest.java +++ b/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/pipelines/PipelineResourceTest.java @@ -97,7 +97,7 @@ public class PipelineResourceTest extends EntityResourceTest { assertEquals(createRequest.getDisplayName(), pipeline.getDisplayName()); assertService(createRequest.getService(), pipeline.getService()); assertEquals(createRequest.getTasks(), pipeline.getTasks()); - TestUtils.validateTags(pipeline.getFullyQualifiedName(), createRequest.getTags(), pipeline.getTags()); + TestUtils.assertTags(pipeline.getFullyQualifiedName(), createRequest.getTags(), pipeline.getTags()); } @Override @@ -112,7 +112,7 @@ public class PipelineResourceTest extends EntityResourceTest { assertEquals(expected.getDisplayName(), updated.getDisplayName()); assertService(expected.getService(), updated.getService()); assertEquals(expected.getTasks(), updated.getTasks()); - TestUtils.validateTags(updated.getFullyQualifiedName(), expected.getTags(), updated.getTags()); + TestUtils.assertTags(updated.getFullyQualifiedName(), expected.getTags(), updated.getTags()); } @Override diff --git a/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/teams/TeamResourceTest.java b/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/teams/TeamResourceTest.java index d8c13137cb1..cba403930b8 100644 --- a/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/teams/TeamResourceTest.java +++ b/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/teams/TeamResourceTest.java @@ -33,6 +33,7 @@ import org.openmetadata.catalog.type.EntityReference; import org.openmetadata.catalog.type.ImageList; import org.openmetadata.catalog.type.Profile; import org.openmetadata.catalog.util.EntityInterface; +import org.openmetadata.catalog.util.EntityUtil; import org.openmetadata.catalog.util.JsonUtils; import org.openmetadata.catalog.util.TestUtils; import org.openmetadata.common.utils.JsonSchemaUtil; @@ -462,10 +463,8 @@ public class TeamResourceTest extends EntityResourceTest { actualUsers.forEach(TestUtils::validateEntityReference); actualUsers.forEach(user -> user.setHref(null)); - // Note ordering is same as server side ordering by ID as string - // Patch requests work only if the same ordering of users on the server side - actualUsers.sort(Comparator.comparing(entityReference -> entityReference.getId().toString())); - expectedUsers.sort(Comparator.comparing(entityReference -> entityReference.getId().toString())); + actualUsers.sort(EntityUtil.compareEntityReference); + expectedUsers.sort(EntityUtil.compareEntityReference); assertEquals(expectedUsers, actualUsers); TestUtils.validateEntityReference(updated.getOwns()); } diff --git a/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/teams/UserResourceTest.java b/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/teams/UserResourceTest.java index 44648fca599..2faae6c6d1d 100644 --- a/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/teams/UserResourceTest.java +++ b/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/teams/UserResourceTest.java @@ -38,6 +38,7 @@ import org.openmetadata.catalog.type.FieldChange; import org.openmetadata.catalog.type.ImageList; import org.openmetadata.catalog.type.Profile; import org.openmetadata.catalog.util.EntityInterface; +import org.openmetadata.catalog.util.EntityUtil; import org.openmetadata.catalog.util.JsonUtils; import org.openmetadata.catalog.util.TestUtils; import org.openmetadata.catalog.util.TestUtils.UpdateType; @@ -577,10 +578,8 @@ public class UserResourceTest extends EntityResourceTest { updatedTeams.forEach(TestUtils::validateEntityReference); - // Note ordering is same as server side ordering by ID as string - // Patch requests work only if the same ordering of users on the server side - expectedTeams.sort(Comparator.comparing(entityReference -> entityReference.getId().toString())); - updatedTeams.sort(Comparator.comparing(entityReference -> entityReference.getId().toString())); + expectedTeams.sort(EntityUtil.compareEntityReference); + updatedTeams.sort(EntityUtil.compareEntityReference); updatedTeams.forEach(t -> t.setHref(null)); assertEquals(expectedTeams, updatedTeams); if (expected.getProfile() != null) { diff --git a/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/topics/TopicResourceTest.java b/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/topics/TopicResourceTest.java index bf1c83f2944..3b6a5bd1f6c 100644 --- a/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/topics/TopicResourceTest.java +++ b/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/topics/TopicResourceTest.java @@ -295,7 +295,7 @@ public class TopicResourceTest extends EntityResourceTest { validateCommonEntityFields(getEntityInterface(topic), createRequest.getDescription(), TestUtils.getPrincipal(authHeaders), createRequest.getOwner()); assertService(createRequest.getService(), topic.getService()); - TestUtils.validateTags(topic.getFullyQualifiedName(), createRequest.getTags(), topic.getTags()); + TestUtils.assertTags(topic.getFullyQualifiedName(), createRequest.getTags(), topic.getTags()); } @Override @@ -308,7 +308,7 @@ public class TopicResourceTest extends EntityResourceTest { validateCommonEntityFields(getEntityInterface(updated), expected.getDescription(), TestUtils.getPrincipal(authHeaders), expected.getOwner()); assertService(expected.getService(), expected.getService()); - TestUtils.validateTags(expected.getFullyQualifiedName(), expected.getTags(), updated.getTags()); + TestUtils.assertTags(expected.getFullyQualifiedName(), expected.getTags(), updated.getTags()); } @Override diff --git a/catalog-rest-service/src/test/java/org/openmetadata/catalog/util/TestUtils.java b/catalog-rest-service/src/test/java/org/openmetadata/catalog/util/TestUtils.java index c077f313d56..51063fc61e8 100644 --- a/catalog-rest-service/src/test/java/org/openmetadata/catalog/util/TestUtils.java +++ b/catalog-rest-service/src/test/java/org/openmetadata/catalog/util/TestUtils.java @@ -241,7 +241,7 @@ public final class TestUtils { return headers; } - public static void validateTags(String fqn, List expectedList, List actualList) + public static void assertTags(String fqn, List expectedList, List actualList) throws HttpResponseException { if (expectedList == null) { return; @@ -256,8 +256,8 @@ public final class TestUtils { updatedExpectedList.addAll(derived); } updatedExpectedList = updatedExpectedList.stream().distinct().collect(Collectors.toList()); - updatedExpectedList.sort(Comparator.comparing(TagLabel::getTagFQN)); - actualList.sort(Comparator.comparing(TagLabel::getTagFQN)); + updatedExpectedList.sort(EntityUtil.compareTagLabel); + actualList.sort(EntityUtil.compareTagLabel); assertEquals(updatedExpectedList, actualList); }