From cf416b662eaae7121e984b20e8fb42e601df0d60 Mon Sep 17 00:00:00 2001 From: sureshms Date: Thu, 14 Oct 2021 09:05:01 -0700 Subject: [PATCH] Fix #738 Add support for update time and updated by information to table entities on server backend --- .../mysql/v001__create_db_connection_info.sql | 7 +++-- .../catalog/jdbi3/TableRepository.java | 3 +- .../resources/databases/TableResource.java | 13 ++++++--- .../openmetadata/catalog/util/JsonUtils.java | 2 +- .../openmetadata/catalog/util/RestUtil.java | 2 +- .../databases/TableResourceTest.java | 29 ++++++++++++------- .../openmetadata/catalog/util/TestUtils.java | 5 ++++ 7 files changed, 42 insertions(+), 19 deletions(-) diff --git a/bootstrap/sql/mysql/v001__create_db_connection_info.sql b/bootstrap/sql/mysql/v001__create_db_connection_info.sql index 4a8a3dd3bfd..aa7c92ec7a2 100644 --- a/bootstrap/sql/mysql/v001__create_db_connection_info.sql +++ b/bootstrap/sql/mysql/v001__create_db_connection_info.sql @@ -107,9 +107,12 @@ CREATE TABLE IF NOT EXISTS table_entity ( id VARCHAR(36) GENERATED ALWAYS AS (json ->> '$.id') STORED NOT NULL, fullyQualifiedName VARCHAR(256) GENERATED ALWAYS AS (json ->> '$.fullyQualifiedName') NOT NULL, json JSON NOT NULL, - timestamp BIGINT, + updatedAt TIMESTAMP GENERATED ALWAYS AS (TIMESTAMP(STR_TO_DATE(json ->> '$.updatedAt', '%Y-%m-%dT%T.%fZ'))) NOT NULL, + updatedBy VARCHAR(256) GENERATED ALWAYS AS (json ->> '$.updatedBy') NOT NULL, PRIMARY KEY (id), - UNIQUE KEY unique_name(fullyQualifiedName) + UNIQUE KEY unique_name(fullyQualifiedName), + INDEX (updatedBy), + INDEX (updatedAt) ); CREATE TABLE IF NOT EXISTS metric_entity ( 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 d96a79760b2..0a6da4566ce 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 @@ -177,9 +177,10 @@ public abstract class TableRepository { } @Transaction - public Table patch(String id, JsonPatch patch) throws IOException, ParseException { + public Table patch(String id, String user, JsonPatch patch) throws IOException, ParseException { Table original = setFields(validateTable(id), TABLE_PATCH_FIELDS); Table updated = JsonUtils.applyPatch(original, patch, Table.class); + updated.withUpdatedBy(user).withUpdatedAt(new Date()); patch(original, updated); return updated; } diff --git a/catalog-rest-service/src/main/java/org/openmetadata/catalog/resources/databases/TableResource.java b/catalog-rest-service/src/main/java/org/openmetadata/catalog/resources/databases/TableResource.java index 6f06da479d8..1de634b65c7 100644 --- a/catalog-rest-service/src/main/java/org/openmetadata/catalog/resources/databases/TableResource.java +++ b/catalog-rest-service/src/main/java/org/openmetadata/catalog/resources/databases/TableResource.java @@ -70,6 +70,7 @@ import java.io.UnsupportedEncodingException; import java.security.GeneralSecurityException; import java.text.ParseException; import java.util.Arrays; +import java.util.Date; import java.util.List; import java.util.Objects; import java.util.UUID; @@ -218,12 +219,14 @@ public class TableResource { }) public Response create(@Context UriInfo uriInfo, @Context SecurityContext securityContext, - @Valid CreateTable create) throws IOException { + @Valid CreateTable create) throws IOException, ParseException { SecurityUtil.checkAdminOrBotRole(authorizer, securityContext); Table table = new Table().withId(UUID.randomUUID()).withName(create.getName()) .withColumns(create.getColumns()).withDescription(create.getDescription()) .withTableConstraints(create.getTableConstraints()).withTableType(create.getTableType()) - .withTags(create.getTags()).withViewDefinition(create.getViewDefinition()); + .withTags(create.getTags()).withViewDefinition(create.getViewDefinition()) + .withUpdatedBy(securityContext.getUserPrincipal().getName()) + .withUpdatedAt(new Date()); table = addHref(uriInfo, dao.create(validateNewTable(table), create.getOwner(), create.getDatabase())); return Response.created(table.getHref()).entity(table).build(); } @@ -243,7 +246,9 @@ public class TableResource { Table table = new Table().withId(UUID.randomUUID()).withName(create.getName()) .withColumns(create.getColumns()).withDescription(create.getDescription()) .withTableConstraints(create.getTableConstraints()).withTableType(create.getTableType()) - .withTags(create.getTags()).withViewDefinition(create.getViewDefinition()); + .withTags(create.getTags()).withViewDefinition(create.getViewDefinition()) + .withUpdatedBy(securityContext.getUserPrincipal().getName()) + .withUpdatedAt(new Date()); SecurityUtil.checkAdminRoleOrPermissions(authorizer, securityContext, dao.getOwnerReference(table)); PutResponse response = dao.createOrUpdate(validateNewTable(table), create.getOwner(), create.getDatabase()); table = addHref(uriInfo, response.getEntity()); @@ -271,7 +276,7 @@ public class TableResource { Fields fields = new Fields(FIELD_LIST, FIELDS); Table table = dao.get(id, fields); SecurityUtil.checkAdminRoleOrPermissions(authorizer, securityContext, dao.getOwnerReference(table)); - table = dao.patch(id, patch); + table = dao.patch(id, securityContext.getUserPrincipal().getName(), patch); return addHref(uriInfo, table); } diff --git a/catalog-rest-service/src/main/java/org/openmetadata/catalog/util/JsonUtils.java b/catalog-rest-service/src/main/java/org/openmetadata/catalog/util/JsonUtils.java index acb66192a6b..d12e01c5b12 100644 --- a/catalog-rest-service/src/main/java/org/openmetadata/catalog/util/JsonUtils.java +++ b/catalog-rest-service/src/main/java/org/openmetadata/catalog/util/JsonUtils.java @@ -64,7 +64,7 @@ public final class JsonUtils { OBJECT_MAPPER = new ObjectMapper(); // Ensure the date-time fields are serialized in ISO-8601 format OBJECT_MAPPER.disable(SerializationFeature.WRITE_DATES_AS_TIMESTAMPS); - OBJECT_MAPPER.setDateFormat(new StdDateFormat().withColonInTimeZone(true)); + OBJECT_MAPPER.setDateFormat(RestUtil.DATE_TIME_FORMAT); OBJECT_MAPPER.registerModule(new JSR353Module()); } diff --git a/catalog-rest-service/src/main/java/org/openmetadata/catalog/util/RestUtil.java b/catalog-rest-service/src/main/java/org/openmetadata/catalog/util/RestUtil.java index 816873285c2..a3708f58516 100644 --- a/catalog-rest-service/src/main/java/org/openmetadata/catalog/util/RestUtil.java +++ b/catalog-rest-service/src/main/java/org/openmetadata/catalog/util/RestUtil.java @@ -43,7 +43,7 @@ public final class RestUtil { static { // Quoted "Z" to indicate UTC, no timezone offset - DATE_TIME_FORMAT = new SimpleDateFormat("yyyy-MM-dd'T'HH:mm'Z'"); + DATE_TIME_FORMAT = new SimpleDateFormat("yyyy-MM-dd'T'HH:mm:ss.SSSSSS'Z'"); DATE_TIME_FORMAT.setTimeZone(TimeZone.getTimeZone("UTC")); DATE_FORMAT = new SimpleDateFormat("yyyy-MM-dd"); 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 7087c24b6f5..3c6221d5f79 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 @@ -366,7 +366,8 @@ public class TableResourceTest extends CatalogApplicationTest { public void put_tableCreate_200(TestInfo test) throws HttpResponseException { // Create a new table with put CreateTable request = create(test).withOwner(USER_OWNER1); - updateAndCheckTable(request.withName("newName").withDescription(null), CREATED, adminAuthHeaders()); + Table table = updateAndCheckTable(request.withName("newName").withDescription(null), CREATED, adminAuthHeaders()); + assertEquals(0.1, table.getVersion()); // First version } @Test @@ -1083,6 +1084,7 @@ public class TableResourceTest extends CatalogApplicationTest { TableType tableType, List tableConstraints, List tags, Map authHeaders) throws JsonProcessingException, HttpResponseException { + String updatedBy = TestUtils.getPrincipal(authHeaders); String tableJson = JsonUtils.pojoToJson(table); // Update the table attributes @@ -1095,11 +1097,12 @@ public class TableResourceTest extends CatalogApplicationTest { // Validate information returned in patch response has the updates Table updatedTable = patchTable(tableJson, table, authHeaders); validateTable(updatedTable, table.getDescription(), table.getColumns(), owner, null, tableType, - tableConstraints, tags); + tableConstraints, tags, updatedBy); // GET the table and Validate information returned Table getTable = getTable(table.getId(), "owner,tableConstraints,columns, tags", authHeaders); - validateTable(getTable, table.getDescription(), table.getColumns(), owner, null, tableType, tableConstraints, tags); + validateTable(getTable, table.getDescription(), table.getColumns(), owner, null, tableType, + tableConstraints, tags, updatedBy); return updatedTable; } @@ -1122,14 +1125,17 @@ public class TableResourceTest extends CatalogApplicationTest { public static Table createAndCheckTable(CreateTable create, Map authHeaders) throws HttpResponseException { // Validate table created has all the information set in create request + String updatedBy = TestUtils.getPrincipal(authHeaders); Table table = createTable(create, authHeaders); - validateTable(table, create.getDescription(), create.getColumns(), create.getOwner(), - create.getDatabase(), create.getTableType(), create.getTableConstraints(), create.getTags()); + validateTable(table, create.getDescription(), create.getColumns(), create.getOwner(), create.getDatabase(), + create.getTableType(), create.getTableConstraints(), create.getTags(), updatedBy); + assertEquals(0.1, table.getVersion()); // First version of the table // GET table created and ensure it has all the information set in create request Table getTable = getTable(table.getId(), "columns,owner,database,tags,tableConstraints", authHeaders); - validateTable(getTable, create.getDescription(), create.getColumns(), create.getOwner(), - create.getDatabase(), create.getTableType(), create.getTableConstraints(), create.getTags()); + validateTable(getTable, create.getDescription(), create.getColumns(), create.getOwner(), create.getDatabase(), + create.getTableType(), create.getTableConstraints(), create.getTags(), updatedBy); + assertEquals(0.1, table.getVersion()); // First version of the table // If owner information is set, GET and make sure the user or team has the table in owns list checkOwnerOwns(create.getOwner(), table.getId(), true); @@ -1210,13 +1216,15 @@ public class TableResourceTest extends CatalogApplicationTest { private static void validateTable(Table table, String expectedDescription, List expectedColumns, EntityReference expectedOwner, UUID expectedDatabaseId, TableType expectedTableType, - List expectedTableConstraints, List expectedTags) + List expectedTableConstraints, List expectedTags, + String expectedUpdatedByUser) throws HttpResponseException { assertNotNull(table.getId()); assertNotNull(table.getHref()); assertNotNull(table.getFullyQualifiedName()); assertEquals(expectedDescription, table.getDescription()); assertEquals(expectedTableType, table.getTableType()); + assertEquals(expectedUpdatedByUser, table.getUpdatedBy()); validateColumns(expectedColumns, table.getColumns()); @@ -1332,14 +1340,15 @@ public class TableResourceTest extends CatalogApplicationTest { public static Table updateAndCheckTable(CreateTable create, Status status, Map authHeaders) throws HttpResponseException { + String updatedBy = TestUtils.getPrincipal(authHeaders); Table updatedTable = updateTable(create, status, authHeaders); validateTable(updatedTable, create.getDescription(), create.getColumns(), create.getOwner(), create.getDatabase(), - create.getTableType(), create.getTableConstraints(), create.getTags()); + create.getTableType(), create.getTableConstraints(), create.getTags(), updatedBy); // GET the newly updated database and validate Table getTable = getTable(updatedTable.getId(), "columns,database,owner,tableConstraints,tags", authHeaders); validateTable(getTable, create.getDescription(), create.getColumns(), create.getOwner(), create.getDatabase(), - create.getTableType(), create.getTableConstraints(), create.getTags()); + create.getTableType(), create.getTableConstraints(), create.getTags(), updatedBy); // TODO columns check return updatedTable; } 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 82ac09369f8..f6901e851bd 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 @@ -275,4 +275,9 @@ public final class TestUtils { } assertEquals(expectedFollowing, following, "Follower list for the user is invalid"); } + + public static String getPrincipal(Map authHeaders) { + // Get user name from the email address + return authHeaders.get(CatalogOpenIdAuthorizationRequestFilter.X_AUTH_PARAMS_EMAIL_HEADER).split("@")[0]; + } }