From 2c93cd278b561805796f5d853cd2ec84162bff0c Mon Sep 17 00:00:00 2001 From: Suresh Srinivas Date: Sun, 10 Apr 2022 08:03:28 -0700 Subject: [PATCH] Fixes #3968 EntityReference should contain name, displayName and FQN as separate fields (#3996) --- .../catalog/jdbi3/ChartRepository.java | 2 +- .../catalog/jdbi3/DashboardRepository.java | 2 +- .../catalog/jdbi3/DatabaseRepository.java | 2 +- .../jdbi3/DatabaseSchemaRepository.java | 4 +- .../catalog/jdbi3/EntityRepository.java | 18 +++++-- .../catalog/jdbi3/GlossaryTermRepository.java | 5 +- .../jdbi3/IngestionPipelineRepository.java | 2 +- .../catalog/jdbi3/LocationRepository.java | 2 +- .../catalog/jdbi3/MetricsRepository.java | 2 +- .../catalog/jdbi3/MlModelRepository.java | 2 +- .../catalog/jdbi3/PipelineRepository.java | 2 +- .../catalog/jdbi3/TableRepository.java | 2 +- .../catalog/jdbi3/TopicRepository.java | 2 +- .../catalog/util/EntityInterface.java | 3 +- .../openmetadata/catalog/util/EntityUtil.java | 5 +- .../catalog/util/FullyQualifiedName.java | 2 +- .../json/schema/type/entityReference.json | 6 ++- .../dashboards/DashboardResourceTest.java | 2 +- .../databases/DatabaseResourceTest.java | 8 ++-- .../databases/DatabaseSchemaResourceTest.java | 6 ++- .../databases/TableResourceTest.java | 47 ++++++------------- .../locations/LocationResourceTest.java | 2 +- .../pipelines/PipelineResourceTest.java | 2 +- .../services/DatabaseServiceResourceTest.java | 3 +- .../IngestionPipelineResourceTest.java | 8 ++-- .../openmetadata/catalog/util/TestUtils.java | 16 +++++-- 26 files changed, 84 insertions(+), 73 deletions(-) diff --git a/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/ChartRepository.java b/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/ChartRepository.java index 6f350faa5d2..b9d11d441a9 100644 --- a/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/ChartRepository.java +++ b/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/ChartRepository.java @@ -54,7 +54,7 @@ public class ChartRepository extends EntityRepository { public static String getFQN(Chart chart) { return (chart != null && chart.getService() != null) - ? FullyQualifiedName.add(chart.getService().getName(), chart.getName()) + ? FullyQualifiedName.add(chart.getService().getFullyQualifiedName(), chart.getName()) : null; } 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 afc45d4d70f..10190be7710 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 @@ -54,7 +54,7 @@ public class DashboardRepository extends EntityRepository { public static String getFQN(Dashboard dashboard) { return (dashboard != null && dashboard.getService() != null) - ? FullyQualifiedName.add(dashboard.getService().getName(), dashboard.getName()) + ? FullyQualifiedName.add(dashboard.getService().getFullyQualifiedName(), dashboard.getName()) : null; } diff --git a/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/DatabaseRepository.java b/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/DatabaseRepository.java index 930c3b20295..4779eb9cdf1 100644 --- a/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/DatabaseRepository.java +++ b/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/DatabaseRepository.java @@ -54,7 +54,7 @@ public class DatabaseRepository extends EntityRepository { public static String getFQN(Database database) { return (database != null && database.getService() != null) - ? FullyQualifiedName.add(database.getService().getName(), database.getName()) + ? FullyQualifiedName.add(database.getService().getFullyQualifiedName(), database.getName()) : null; } diff --git a/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/DatabaseSchemaRepository.java b/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/DatabaseSchemaRepository.java index b1bc0e9dadc..17ec0a95305 100644 --- a/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/DatabaseSchemaRepository.java +++ b/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/DatabaseSchemaRepository.java @@ -51,7 +51,9 @@ public class DatabaseSchemaRepository extends EntityRepository { } public static String getFQN(DatabaseSchema schema) { - return (schema != null) ? FullyQualifiedName.add(schema.getDatabase().getName(), schema.getName()) : null; + return schema != null + ? FullyQualifiedName.add(schema.getDatabase().getFullyQualifiedName(), schema.getName()) + : null; } @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 feb270d87dd..1a78bb6fc1c 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 @@ -577,7 +577,7 @@ public abstract class EntityRepository { } /** Validate given list of tags and add derived tags to it */ - public final List addDerivedTags(List tagLabels) throws IOException { + public final List addDerivedTags(List tagLabels) { if (tagLabels == null || tagLabels.isEmpty()) { return tagLabels; } @@ -605,7 +605,7 @@ public abstract class EntityRepository { } /** Get tags associated with a given set of tags */ - private List getDerivedTags(TagLabel tagLabel, Tag tag) throws IOException { + private List getDerivedTags(TagLabel tagLabel, Tag tag) { List derivedTags = new ArrayList<>(); for (String fqn : listOrEmpty(tag.getAssociatedTags())) { Tag tempTag = daoCollection.tagDAO().findEntityByName(fqn); @@ -662,7 +662,7 @@ public abstract class EntityRepository { List followers = findFrom(entityInterface.getId(), entityType, Relationship.FOLLOWS); for (EntityReference follower : followers) { User user = daoCollection.userDAO().findEntityById(follower.getId(), ALL); - follower.withName(user.getName()).withDeleted(user.getDeleted()); + follower.withName(user.getName()).withDeleted(user.getDeleted()).withFullyQualifiedName(user.getName()); } return followers; } @@ -776,7 +776,11 @@ public abstract class EntityRepository { entityReferences.sort(EntityUtil.compareEntityReference); for (EntityReference entityReference : entityReferences) { EntityReference ref = daoCollection.userDAO().findEntityReferenceById(entityReference.getId()); - entityReference.withType(ref.getType()).withName(ref.getName()).withDisplayName(ref.getDisplayName()); + entityReference + .withType(ref.getType()) + .withName(ref.getName()) + .withDisplayName(ref.getDisplayName()) + .withFullyQualifiedName(ref.getFullyQualifiedName()); } } } @@ -786,7 +790,11 @@ public abstract class EntityRepository { entityReferences.sort(EntityUtil.compareEntityReference); for (EntityReference entityReference : entityReferences) { EntityReference ref = daoCollection.roleDAO().findEntityReferenceById(entityReference.getId()); - entityReference.withType(ref.getType()).withName(ref.getName()).withDisplayName(ref.getDisplayName()); + entityReference + .withType(ref.getType()) + .withName(ref.getName()) + .withDisplayName(ref.getDisplayName()) + .withFullyQualifiedName(ref.getFullyQualifiedName()); } } } diff --git a/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/GlossaryTermRepository.java b/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/GlossaryTermRepository.java index 191e921c94b..1102a41b9f4 100644 --- a/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/GlossaryTermRepository.java +++ b/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/GlossaryTermRepository.java @@ -103,10 +103,11 @@ public class GlossaryTermRepository extends EntityRepository { // Validate parent if (entity.getParent() == null) { - entity.setFullyQualifiedName(FullyQualifiedName.add(entity.getGlossary().getName(), entity.getName())); + entity.setFullyQualifiedName( + FullyQualifiedName.add(entity.getGlossary().getFullyQualifiedName(), entity.getName())); } else { EntityReference parent = Entity.getEntityReference(entity.getParent()); - entity.setFullyQualifiedName(FullyQualifiedName.add(parent.getName(), entity.getName())); + entity.setFullyQualifiedName(FullyQualifiedName.add(parent.getFullyQualifiedName(), entity.getName())); entity.setParent(parent); } diff --git a/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/IngestionPipelineRepository.java b/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/IngestionPipelineRepository.java index f00116b76df..8a1792ec558 100644 --- a/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/IngestionPipelineRepository.java +++ b/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/IngestionPipelineRepository.java @@ -50,7 +50,7 @@ public class IngestionPipelineRepository extends EntityRepository { public static String getFQN(Location location) { return (location != null && location.getService() != null) - ? FullyQualifiedName.add(location.getService().getName(), location.getName()) + ? FullyQualifiedName.add(location.getService().getFullyQualifiedName(), location.getName()) : null; } diff --git a/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/MetricsRepository.java b/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/MetricsRepository.java index e834459d43c..f551247ed7e 100644 --- a/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/MetricsRepository.java +++ b/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/MetricsRepository.java @@ -49,7 +49,7 @@ public class MetricsRepository extends EntityRepository { public static String getFQN(Metrics metrics) { return (metrics != null && metrics.getService() != null) - ? FullyQualifiedName.add(metrics.getService().getName(), metrics.getName()) + ? FullyQualifiedName.add(metrics.getService().getFullyQualifiedName(), metrics.getName()) : null; } diff --git a/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/MlModelRepository.java b/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/MlModelRepository.java index 0e2758f2c3d..dace2f4f62f 100644 --- a/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/MlModelRepository.java +++ b/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/MlModelRepository.java @@ -91,7 +91,7 @@ public class MlModelRepository extends EntityRepository { mlSources.forEach( s -> { if (s.getDataSource() != null) { - s.setFullyQualifiedName(FullyQualifiedName.add(s.getDataSource().getName(), s.getName())); + s.setFullyQualifiedName(FullyQualifiedName.add(s.getDataSource().getFullyQualifiedName(), s.getName())); } else { s.setFullyQualifiedName(s.getName()); } 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 cd0090351c4..bfbf7851636 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 @@ -63,7 +63,7 @@ public class PipelineRepository extends EntityRepository { public static String getFQN(Pipeline pipeline) { return (pipeline != null && pipeline.getService() != null) - ? FullyQualifiedName.add(pipeline.getService().getName(), pipeline.getName()) + ? FullyQualifiedName.add(pipeline.getService().getFullyQualifiedName(), pipeline.getName()) : null; } 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 a2fad310e61..e37db1af14d 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 @@ -144,7 +144,7 @@ public class TableRepository extends EntityRepository { public static String getFQN(Table table) { return (table != null && table.getDatabaseSchema() != null) - ? FullyQualifiedName.add(table.getDatabaseSchema().getName(), table.getName()) + ? FullyQualifiedName.add(table.getDatabaseSchema().getFullyQualifiedName(), table.getName()) : null; } diff --git a/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/TopicRepository.java b/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/TopicRepository.java index 60fc8462813..e081014dc30 100644 --- a/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/TopicRepository.java +++ b/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/TopicRepository.java @@ -42,7 +42,7 @@ public class TopicRepository extends EntityRepository { public static String getFQN(Topic topic) { return (topic != null && topic.getService() != null) - ? FullyQualifiedName.add(topic.getService().getName(), topic.getName()) + ? FullyQualifiedName.add(topic.getService().getFullyQualifiedName(), topic.getName()) : null; } diff --git a/catalog-rest-service/src/main/java/org/openmetadata/catalog/util/EntityInterface.java b/catalog-rest-service/src/main/java/org/openmetadata/catalog/util/EntityInterface.java index a618307a190..9351f92ea1f 100644 --- a/catalog-rest-service/src/main/java/org/openmetadata/catalog/util/EntityInterface.java +++ b/catalog-rest-service/src/main/java/org/openmetadata/catalog/util/EntityInterface.java @@ -67,7 +67,8 @@ public abstract class EntityInterface { public final EntityReference getEntityReference() { return new EntityReference() .withId(getId()) - .withName(getFullyQualifiedName()) + .withName(getName()) + .withFullyQualifiedName(getFullyQualifiedName()) .withDescription(getDescription()) .withDisplayName(getDisplayName()) .withType(entityType) 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 1a0b48d546e..cebe0405839 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 @@ -167,7 +167,10 @@ public final class EntityUtil { if (list != null) { for (EntityReference ref : list) { EntityReference ref2 = Entity.getEntityReferenceById(ref.getType(), ref.getId(), ALL); - ref.withDescription(ref2.getDescription()).withName(ref2.getName()).withDisplayName(ref2.getDisplayName()); + ref.withDescription(ref2.getDescription()) + .withName(ref2.getName()) + .withDisplayName(ref2.getDisplayName()) + .withFullyQualifiedName(ref2.getFullyQualifiedName()); } } return list; diff --git a/catalog-rest-service/src/main/java/org/openmetadata/catalog/util/FullyQualifiedName.java b/catalog-rest-service/src/main/java/org/openmetadata/catalog/util/FullyQualifiedName.java index aea33f37664..9ea6443e45a 100644 --- a/catalog-rest-service/src/main/java/org/openmetadata/catalog/util/FullyQualifiedName.java +++ b/catalog-rest-service/src/main/java/org/openmetadata/catalog/util/FullyQualifiedName.java @@ -66,7 +66,7 @@ public class FullyQualifiedName { public static String getTableFQN(String columnFQN) { // Split columnFQN of format databaseServiceName.databaseName.tableName.columnName String[] split = split(columnFQN); - if (split.length < 5) { + if (split.length != 5) { throw new IllegalArgumentException("Invalid fully qualified column name " + columnFQN); } // Return table FQN of format databaseService.tableName diff --git a/catalog-rest-service/src/main/resources/json/schema/type/entityReference.json b/catalog-rest-service/src/main/resources/json/schema/type/entityReference.json index 592b7ea3d42..8a490259402 100644 --- a/catalog-rest-service/src/main/resources/json/schema/type/entityReference.json +++ b/catalog-rest-service/src/main/resources/json/schema/type/entityReference.json @@ -24,7 +24,11 @@ "type": "string" }, "name": { - "description": "Name of the entity instance. For entities such as tables, databases where the name is not unique, fullyQualifiedName is returned in this field.", + "description": "Name of the entity instance.", + "type": "string" + }, + "fullyQualifiedName": { + "description": "Fully qualified name of the entity instance. For entities such as tables, databases fullyQualifiedName is returned in this field. For entities that don't have name hierarchy such as `user` and `team` this will be same as the `name` field.", "type": "string" }, "description": { 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 129cceb3102..eb326d946b9 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 @@ -113,7 +113,7 @@ public class DashboardResourceTest extends EntityResourceTest list = listEntities(queryParams, ADMIN_AUTH_HEADERS); for (Dashboard db : list.getData()) { assertEquals(service.getName(), db.getService().getName()); - String expectedFQN = FullyQualifiedName.add(service.getName(), db.getName()); + String expectedFQN = FullyQualifiedName.add(service.getFullyQualifiedName(), db.getName()); assertEquals(expectedFQN, db.getFullyQualifiedName()); } } diff --git a/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/databases/DatabaseResourceTest.java b/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/databases/DatabaseResourceTest.java index f5e4077de5d..18043f0cfd4 100644 --- a/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/databases/DatabaseResourceTest.java +++ b/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/databases/DatabaseResourceTest.java @@ -77,7 +77,7 @@ public class DatabaseResourceTest extends EntityResourceTest { // GET .../tables?fields=columns,tableConstraints final String fields = "tableConstraints"; - queryParams = - new HashMap<>() { - { - put("fields", fields); - } - }; + queryParams = new HashMap<>(); + queryParams.put("fields", fields); tableList = listEntities(queryParams, ADMIN_AUTH_HEADERS); assertEquals(2, tableList.getData().size()); assertFields(tableList.getData(), fields); // List tables with databaseFQN as filter - queryParams = - new HashMap<>() { - { - put("fields", fields); - put("database", DATABASE.getFullyQualifiedName()); - } - }; + queryParams = new HashMap<>(); + queryParams.put("fields", fields); + queryParams.put("database", DATABASE.getFullyQualifiedName()); tableList1 = listEntities(queryParams, ADMIN_AUTH_HEADERS); assertEquals(tableList.getData().size(), tableList1.getData().size()); assertFields(tableList1.getData(), fields); // GET .../tables?fields=usageSummary,owner final String fields1 = "usageSummary,owner"; - queryParams = - new HashMap<>() { - { - put("fields", fields1); - } - }; + queryParams = new HashMap<>(); + queryParams.put("fields", fields1); tableList = listEntities(queryParams, ADMIN_AUTH_HEADERS); assertEquals(2, tableList.getData().size()); assertFields(tableList.getData(), fields1); for (Table table : tableList.getData()) { - assertEquals(table.getOwner().getId(), USER_OWNER1.getId()); - assertEquals(table.getOwner().getType(), USER_OWNER1.getType()); - assertEquals(table.getDatabase().getId(), DATABASE.getId()); - assertEquals(table.getDatabase().getName(), DATABASE.getFullyQualifiedName()); + assertEquals(USER_OWNER1, table.getOwner()); + assertReference(DATABASE_REFERENCE, table.getDatabase()); } // List tables with databaseFQN as filter - queryParams = - new HashMap<>() { - { - put("fields", fields1); - put("database", DATABASE.getFullyQualifiedName()); - } - }; + queryParams = new HashMap<>(); + queryParams.put("fields", fields1); + queryParams.put("database", DATABASE.getFullyQualifiedName()); tableList1 = listEntities(queryParams, ADMIN_AUTH_HEADERS); assertEquals(tableList.getData().size(), tableList1.getData().size()); assertFields(tableList1.getData(), fields1); @@ -2012,7 +1993,7 @@ public class TableResourceTest extends EntityResourceTest { TestUtils.validateEntityReferences(createdEntity.getFollowers()); assertListNotNull(createdEntity.getService(), createdEntity.getServiceType()); assertEquals( - add(createdEntity.getDatabaseSchema().getName(), createdEntity.getName()), + FullyQualifiedName.add(createdEntity.getDatabaseSchema().getFullyQualifiedName(), createdEntity.getName()), createdEntity.getFullyQualifiedName()); } @@ -2050,7 +2031,7 @@ public class TableResourceTest extends EntityResourceTest { TestUtils.validateTags(expected.getTags(), patched.getTags()); TestUtils.validateEntityReferences(expected.getFollowers()); assertEquals( - FullyQualifiedName.add(patched.getDatabaseSchema().getName(), patched.getName()), + FullyQualifiedName.add(patched.getDatabaseSchema().getFullyQualifiedName(), patched.getName()), patched.getFullyQualifiedName()); } diff --git a/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/locations/LocationResourceTest.java b/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/locations/LocationResourceTest.java index 1a573117241..496bb91e8d7 100644 --- a/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/locations/LocationResourceTest.java +++ b/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/locations/LocationResourceTest.java @@ -143,7 +143,7 @@ public class LocationResourceTest extends EntityResourceTest list) { validateEntityReferences(list, false); }