Fixes #10740 Inherited ownership must not overwrite manually entered ownership (#10741)

This commit is contained in:
Suresh Srinivas 2023-03-23 17:15:55 -07:00 committed by GitHub
parent bfa0905093
commit b80fc39632
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 37 additions and 21 deletions

View File

@ -119,6 +119,8 @@ public class DatabaseSchemaRepository extends EntityRepository<DatabaseSchema> {
.withServiceType(database.getServiceType());
// Carry forward ownership from database, if necessary
schema.withOwner(schema.getOwner() == null ? database.getOwner() : schema.getOwner());
if (database.getOwner() != null && schema.getOwner() == null) {
schema.withOwner(database.getOwner().withDescription("inherited"));
}
}
}

View File

@ -474,17 +474,6 @@ public abstract class EntityRepository<T extends EntityInterface> {
return entity;
}
@Transaction
public final PutResponse<T> createOrUpdate(UriInfo uriInfo, T original, T updated) throws IOException {
prepareInternal(updated);
// Check if there is any original, deleted or not
original = JsonUtils.readValue(dao.findJsonByFqn(original.getFullyQualifiedName(), ALL), entityClass);
if (original == null) {
return new PutResponse<>(Status.CREATED, withHref(uriInfo, createNewEntity(updated)), RestUtil.ENTITY_CREATED);
}
return update(uriInfo, original, updated);
}
public final PutResponse<T> createOrUpdate(UriInfo uriInfo, T updated) throws IOException {
PutResponse<T> response = createOrUpdateInternal(uriInfo, updated);
if (response.getStatus() == Status.CREATED) {
@ -497,9 +486,8 @@ public abstract class EntityRepository<T extends EntityInterface> {
@Transaction
public final PutResponse<T> createOrUpdateInternal(UriInfo uriInfo, T updated) throws IOException {
// Check if there is any original, deleted or not
T original = JsonUtils.readValue(dao.findJsonByFqn(updated.getFullyQualifiedName(), ALL), entityClass);
if (original == null) {
if (original == null) { // If an original entity does not exist then create it, else update
return new PutResponse<>(Status.CREATED, withHref(uriInfo, createNewEntity(updated)), RestUtil.ENTITY_CREATED);
}
return update(uriInfo, original, updated);
@ -522,6 +510,12 @@ public abstract class EntityRepository<T extends EntityInterface> {
// Get all the fields in the original entity that can be updated during PUT operation
setFieldsInternal(original, putFields);
EntityReference updatedOwner = updated.getOwner();
if (updatedOwner != null && updatedOwner.getDescription().equals("inherited")) {
// Don't let inherited ownership overwrite existing ownership
updated.setOwner(original.getOwner() != null ? original.getOwner() : updatedOwner);
}
// If the entity state is soft-deleted, recursively undelete the entity and it's children
if (Boolean.TRUE.equals(original.getDeleted())) {
restoreEntity(updated.getUpdatedBy(), entityType, original.getId());

View File

@ -611,7 +611,9 @@ public class TableRepository extends EntityRepository<Table> {
.withServiceType(schema.getServiceType());
// Carry forward ownership from database schema
table.setOwner(table.getOwner() == null ? schema.getOwner() : table.getOwner());
if (table.getOwner() == null && schema.getOwner() != null) {
table.setOwner(schema.getOwner().withDescription("inherited"));
}
// Validate column tags
addDerivedColumnTags(table.getColumns());

View File

@ -63,6 +63,7 @@ import static org.openmetadata.service.util.TestUtils.assertResponse;
import static org.openmetadata.service.util.TestUtils.assertResponseContains;
import static org.openmetadata.service.util.TestUtils.validateEntityReference;
import com.fasterxml.jackson.core.JsonProcessingException;
import java.io.IOException;
import java.text.ParseException;
import java.util.ArrayList;
@ -1735,22 +1736,39 @@ public class TableResourceTest extends EntityResourceTest<Table, CreateTable> {
}
@Test
void test_ownershipInheritance(TestInfo test) throws HttpResponseException {
void test_ownershipInheritance(TestInfo test) throws HttpResponseException, JsonProcessingException {
// When a databaseSchema has no owner set, it inherits the ownership from database
// When a table has no owner set, it inherits the ownership from databaseSchema
DatabaseResourceTest dbTest = new DatabaseResourceTest();
Database db = dbTest.createEntity(dbTest.createRequest(test).withOwner(USER1_REF), ADMIN_AUTH_HEADERS);
// Ensure databaseSchema owner is inherited from database
DatabaseSchemaResourceTest schemaTest = new DatabaseSchemaResourceTest();
CreateDatabaseSchema createSchema =
schemaTest.createRequest(test).withDatabase(db.getFullyQualifiedName()).withOwner(null);
DatabaseSchema schema = schemaTest.createEntity(createSchema, ADMIN_AUTH_HEADERS);
assertEquals(USER1_REF, schema.getOwner()); // Ensure databaseSchema owner is inherited from database
assertReference(USER1_REF, schema.getOwner());
Table table =
createEntity(
createRequest(test).withOwner(null).withDatabaseSchema(schema.getFullyQualifiedName()), ADMIN_AUTH_HEADERS);
assertEquals(USER1_REF, table.getOwner()); // Ensure table owner is inherited from databaseSchema
// Ensure table owner is inherited from databaseSchema
CreateTable createTable = createRequest(test).withOwner(null).withDatabaseSchema(schema.getFullyQualifiedName());
Table table = createEntity(createTable, ADMIN_AUTH_HEADERS);
assertReference(USER1_REF, table.getOwner());
// Change the ownership of table and ensure further ingestion updates don't overwrite the ownership
String json = JsonUtils.pojoToJson(table);
table.setOwner(USER2_REF);
table = patchEntity(table.getId(), json, table, ADMIN_AUTH_HEADERS);
assertReference(USER2_REF, table.getOwner());
table = updateEntity(createTable.withOwner(null), OK, ADMIN_AUTH_HEADERS); // Simulate ingestion update
assertReference(USER2_REF, table.getOwner()); // Owner remains the same
// Change the ownership of schema and ensure further ingestion updates don't overwrite the ownership
json = JsonUtils.pojoToJson(schema);
schema.setOwner(USER2_REF);
schema = schemaTest.patchEntity(schema.getId(), json, schema, ADMIN_AUTH_HEADERS);
assertReference(USER2_REF, schema.getOwner());
schema = schemaTest.updateEntity(createSchema.withOwner(null), OK, ADMIN_AUTH_HEADERS); // Simulate ingestion update
assertReference(USER2_REF, schema.getOwner()); // Owner remains the same
}
private void deleteAndCheckLocation(Table table) throws HttpResponseException {