yujunjun/OpenMetadata
1
0
Fork 0
mirror of https://github.com/open-metadata/OpenMetadata.git synced 2025-09-27 09:55:36 +00:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #658 from open-metadata/issue632

Browse Source 
Fix #632 PATCH API is fails to update for column description for tabl…
This commit is contained in:
Suresh Srinivas 2021-10-04 10:56:08 -07:00 committed by GitHub
commit 5bc7e8fbc2
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 26 additions and 25 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

23
catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/TableRepository.java
View File

@ -202,7 +202,7 @@ public abstract class TableRepository {
updatedTable.setId(storedTable.getId()); updatedTable.setId(storedTable.getId());
validateRelationships(updatedTable, database, newOwner); validateRelationships(updatedTable, database, newOwner);
// Carry forward non empty description // Carry forward non-empty description
if (storedTable.getDescription() != null && !storedTable.getDescription().isEmpty()) { if (storedTable.getDescription() != null && !storedTable.getDescription().isEmpty()) {
// Update description only when it is empty // Update description only when it is empty
updatedTable.setDescription(storedTable.getDescription()); updatedTable.setDescription(storedTable.getDescription());
@ -211,7 +211,7 @@ public abstract class TableRepository {
EntityUtil.removeTagsByPrefix(tagDAO(), storedTable.getFullyQualifiedName()); EntityUtil.removeTagsByPrefix(tagDAO(), storedTable.getFullyQualifiedName());
updatedTable.setTags(EntityUtil.mergeTags(updatedTable.getTags(), storedTable.getTags())); updatedTable.setTags(EntityUtil.mergeTags(updatedTable.getTags(), storedTable.getTags()));
updateColumns(storedTable, updatedTable); updateColumns(storedTable, updatedTable, false);
storeTable(updatedTable, true); storeTable(updatedTable, true);
updateRelationships(storedTable, updatedTable); updateRelationships(storedTable, updatedTable);
@ -468,7 +468,7 @@ public abstract class TableRepository {
// Remove previous tags. Merge tags from the update and the existing tags // Remove previous tags. Merge tags from the update and the existing tags
EntityUtil.removeTags(tagDAO(), original.getFullyQualifiedName()); EntityUtil.removeTags(tagDAO(), original.getFullyQualifiedName());
updateColumns(original, updated); updateColumns(original, updated, true);
storeTable(updated, true); storeTable(updated, true);
updateRelationships(original, updated); updateRelationships(original, updated);
@ -515,8 +515,7 @@ public abstract class TableRepository {
} }
//TODO modified columns //TODO modified columns
private void updateColumns(List<Column> storedColumns, List<Column> updatedColumns, List<Column> addedColumns, private void updateColumns(List<Column> storedColumns, List<Column> updatedColumns, boolean patchOperation) {
List<Column> deletedColumns) {
// Carry forward the user generated metadata from existing columns to new columns // Carry forward the user generated metadata from existing columns to new columns
for (Column updated : updatedColumns) { for (Column updated : updatedColumns) {
// Find stored column matching name, data type and ordinal position // Find stored column matching name, data type and ordinal position
@ -529,13 +528,14 @@ public abstract class TableRepository {
.orElse(null); .orElse(null);
if (stored == null) { if (stored == null) {
// TODO versioning of schema // TODO versioning of schema
addedColumns.add(updated);
LOG.info("Column {} was newly added", updated.getFullyQualifiedName()); LOG.info("Column {} was newly added", updated.getFullyQualifiedName());
continue; continue;
} }
// Carry forward user generated metadata to the columns from the update // For patch operation don't overwrite updated column with stored column description
if (stored.getDescription() != null && !stored.getDescription().isEmpty()) { // For put operation overwrite updated column with non-empty or non-null stored column description
if (!patchOperation &&
stored.getDescription() != null && !stored.getDescription().isEmpty()) {
updated.setDescription(stored.getDescription()); // Carry forward non-empty description updated.setDescription(stored.getDescription()); // Carry forward non-empty description
} }
@ -546,7 +546,7 @@ public abstract class TableRepository {
updated.setTags(updated.getTags()); updated.setTags(updated.getTags());
if (updated.getChildren() != null && stored.getChildren() != null) { if (updated.getChildren() != null && stored.getChildren() != null) {
updateColumns(stored.getChildren(), updated.getChildren(), addedColumns, deletedColumns); updateColumns(stored.getChildren(), updated.getChildren(), patchOperation);
} }
} }
@ -562,14 +562,13 @@ public abstract class TableRepository {
.orElse(null); .orElse(null);
if (updated == null) { if (updated == null) {
// TODO versioning of schema addedColumns.add(stored); // TODO versioning of schema addedColumns.add(stored);
deletedColumns.add(stored);
LOG.info("Column {} was deleted", stored.getFullyQualifiedName()); LOG.info("Column {} was deleted", stored.getFullyQualifiedName());
} }
} }
} }
private void updateColumns(Table storedTable, Table updatedTable) { private void updateColumns(Table storedTable, Table updatedTable, boolean patchOperation) {
updateColumns(storedTable.getColumns(), updatedTable.getColumns(), new ArrayList<>(), new ArrayList<>()); updateColumns(storedTable.getColumns(), updatedTable.getColumns(), patchOperation);
storedTable.setColumns(updatedTable.getColumns()); storedTable.setColumns(updatedTable.getColumns());
} }

28
catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/databases/TableResourceTest.java
View File

@ -92,6 +92,7 @@ import static org.openmetadata.catalog.resources.databases.DatabaseResourceTest.
import static org.openmetadata.catalog.resources.services.DatabaseServiceResourceTest.createService; import static org.openmetadata.catalog.resources.services.DatabaseServiceResourceTest.createService;
import static org.openmetadata.catalog.type.ColumnDataType.ARRAY; import static org.openmetadata.catalog.type.ColumnDataType.ARRAY;
import static org.openmetadata.catalog.type.ColumnDataType.BIGINT; import static org.openmetadata.catalog.type.ColumnDataType.BIGINT;
import static org.openmetadata.catalog.type.ColumnDataType.BINARY;
import static org.openmetadata.catalog.type.ColumnDataType.CHAR; import static org.openmetadata.catalog.type.ColumnDataType.CHAR;
import static org.openmetadata.catalog.type.ColumnDataType.FLOAT; import static org.openmetadata.catalog.type.ColumnDataType.FLOAT;
import static org.openmetadata.catalog.type.ColumnDataType.INT; import static org.openmetadata.catalog.type.ColumnDataType.INT;
@ -229,13 +230,12 @@ public class TableResourceTest extends CatalogApplicationTest {
} }
private static Column getColumn(String name, ColumnDataType columnDataType, TagLabel tag) { private static Column getColumn(String name, ColumnDataType columnDataType, TagLabel tag) {
return getColumn(name, columnDataType, null, tag); return getColumn(name, columnDataType, null, tag);
} }
private static Column getColumn(String name, ColumnDataType columnDataType, String dataTypeDisplay, TagLabel tag) { private static Column getColumn(String name, ColumnDataType columnDataType, String dataTypeDisplay, TagLabel tag) {
List<TagLabel> tags = tag == null ? new ArrayList<>() : singletonList(tag); List<TagLabel> tags = tag == null ? new ArrayList<>() : singletonList(tag);
return new Column().withName(name).withDataType(columnDataType) return new Column().withName(name).withDataType(columnDataType).withDescription(name)
.withDataTypeDisplay(dataTypeDisplay).withTags(tags); .withDataTypeDisplay(dataTypeDisplay).withTags(tags);
} }
@ -438,9 +438,10 @@ public class TableResourceTest extends CatalogApplicationTest {
// //
// Create a table with column c1, type BIGINT, description c1 and tag USER_ADDRESS_TAB_LABEL // Create a table with column c1, type BIGINT, description c1 and tag USER_ADDRESS_TAB_LABEL
// //
List<TagLabel> tags = new ArrayList<>(singletonList(USER_ADDRESS_TAG_LABEL)); List<TagLabel> tags = new ArrayList<>();
List<Column> columns = singletonList(new Column().withName("c1").withDataType(BIGINT) tags.add(USER_ADDRESS_TAG_LABEL);
.withOrdinalPosition(1).withDescription("c1").withTags(tags)); List<Column> columns = new ArrayList<>();
columns.add(getColumn("c1", BIGINT, null).withTags(tags));
CreateTable request = create(test).withColumns(columns); CreateTable request = create(test).withColumns(columns);
Table table = createAndCheckTable(request, adminAuthHeaders()); Table table = createAndCheckTable(request, adminAuthHeaders());
@ -458,8 +459,7 @@ public class TableResourceTest extends CatalogApplicationTest {
// //
tags.add(USER_BANK_ACCOUNT_TAG_LABEL); tags.add(USER_BANK_ACCOUNT_TAG_LABEL);
List<Column> updatedColumns = new ArrayList<>(); List<Column> updatedColumns = new ArrayList<>();
updatedColumns.add(new Column().withName("c1").withDataType(BIGINT) updatedColumns.add(getColumn("c1", BIGINT, null).withTags(tags));
.withTags(tags).withOrdinalPosition(1));
table = updateAndCheckTable(request.withColumns(updatedColumns), OK, adminAuthHeaders()); table = updateAndCheckTable(request.withColumns(updatedColumns), OK, adminAuthHeaders());
// Ensure tag usage counts are updated // Ensure tag usage counts are updated
@ -470,7 +470,7 @@ public class TableResourceTest extends CatalogApplicationTest {
// //
// Add a new column and make sure it is added by PUT // Add a new column and make sure it is added by PUT
// //
updatedColumns.add(new Column().withName("c2").withDataType(ColumnDataType.BINARY).withOrdinalPosition(2) updatedColumns.add(getColumn("c2", BINARY, null).withOrdinalPosition(2)
.withDataLength(10).withFullyQualifiedName(table.getFullyQualifiedName() + ".c2").withTags(tags)); .withDataLength(10).withFullyQualifiedName(table.getFullyQualifiedName() + ".c2").withTags(tags));
table = updateAndCheckTable(request.withColumns(updatedColumns), OK, adminAuthHeaders()); table = updateAndCheckTable(request.withColumns(updatedColumns), OK, adminAuthHeaders());
assertEquals(2, table.getColumns().size()); assertEquals(2, table.getColumns().size());
@ -1024,7 +1024,7 @@ public class TableResourceTest extends CatalogApplicationTest {
} }
@Test @Test
public void patch_tableColumnTags_200_ok(TestInfo test) throws HttpResponseException, JsonProcessingException { public void patch_tableColumns_200_ok(TestInfo test) throws HttpResponseException, JsonProcessingException {
// Create table without description, table tags, tier, owner, tableType, and tableConstraints // Create table without description, table tags, tier, owner, tableType, and tableConstraints
List<Column> columns = new ArrayList<>(); List<Column> columns = new ArrayList<>();
columns.add(getColumn("c1", INT, USER_ADDRESS_TAG_LABEL)); columns.add(getColumn("c1", INT, USER_ADDRESS_TAG_LABEL));
@ -1033,10 +1033,11 @@ public class TableResourceTest extends CatalogApplicationTest {
Table table = createTable(create(test).withColumns(columns), adminAuthHeaders()); Table table = createTable(create(test).withColumns(columns), adminAuthHeaders());
// Update the columns // Update the column tags and description
columns.get(0).setTags(List.of(USER_ADDRESS_TAG_LABEL, USER_BANK_ACCOUNT_TAG_LABEL)); // Add a tag columns.get(0).withDescription("new0")
columns.get(1).setTags(List.of(USER_ADDRESS_TAG_LABEL)); // No change in tag .withTags(List.of(USER_ADDRESS_TAG_LABEL, USER_BANK_ACCOUNT_TAG_LABEL)); // Add a tag
columns.get(2).setTags(new ArrayList<>()); // Remove tag columns.get(1).withDescription("new1").withTags(List.of(USER_ADDRESS_TAG_LABEL));// No change in tag
columns.get(2).withDescription("new3").withTags(new ArrayList<>()); // Remove tag
table = patchTableColumnAttributesAndCheck(table, columns, adminAuthHeaders()); table = patchTableColumnAttributesAndCheck(table, columns, adminAuthHeaders());
validateColumns(columns, table.getColumns()); validateColumns(columns, table.getColumns());
@ -1242,6 +1243,7 @@ public class TableResourceTest extends CatalogApplicationTest {
private static void validateColumn(Column expectedColumn, Column actualColumn) throws HttpResponseException { private static void validateColumn(Column expectedColumn, Column actualColumn) throws HttpResponseException {
assertNotNull(actualColumn.getFullyQualifiedName()); assertNotNull(actualColumn.getFullyQualifiedName());
assertEquals(expectedColumn.getName(), actualColumn.getName()); assertEquals(expectedColumn.getName(), actualColumn.getName());
assertEquals(expectedColumn.getDescription(), actualColumn.getDescription());
assertEquals(expectedColumn.getDataType(), actualColumn.getDataType()); assertEquals(expectedColumn.getDataType(), actualColumn.getDataType());
assertEquals(expectedColumn.getArrayDataType(), actualColumn.getArrayDataType()); assertEquals(expectedColumn.getArrayDataType(), actualColumn.getArrayDataType());
if (expectedColumn.getDataTypeDisplay() != null) { if (expectedColumn.getDataTypeDisplay() != null) {