yujunjun/OpenMetadata
1
0
Fork 0
mirror of https://github.com/open-metadata/OpenMetadata.git synced 2025-08-18 22:18:23 +00:00
Code Issues Packages Projects Releases Wiki Activity

Fix TableResourceTest failures for column updates (#21826)

Browse Source
This commit is contained in:
Sriharsha Chintalapani 2025-06-17 17:48:01 -07:00 committed by GitHub
parent e4dffd281c
commit fb5e45e6cd
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 157 additions and 64 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

39
openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/ColumnRepository.java
View File

@ -17,6 +17,7 @@ import static org.openmetadata.service.Entity.DASHBOARD_DATA_MODEL;
import static org.openmetadata.service.Entity.TABLE;
import jakarta.json.JsonPatch;
import jakarta.ws.rs.core.SecurityContext;
import jakarta.ws.rs.core.UriInfo;
import java.util.List;
import lombok.extern.slf4j.Slf4j;
@ -28,15 +29,24 @@ import org.openmetadata.schema.type.EntityReference;
import org.openmetadata.schema.type.Include;
import org.openmetadata.service.Entity;
import org.openmetadata.service.exception.EntityNotFoundException;
import org.openmetadata.service.security.Authorizer;
import org.openmetadata.service.security.policyevaluator.OperationContext;
import org.openmetadata.service.security.policyevaluator.ResourceContext;
import org.openmetadata.service.security.policyevaluator.ResourceContextInterface;
import org.openmetadata.service.util.FullyQualifiedName;
import org.openmetadata.service.util.JsonUtils;
@Slf4j
public class ColumnRepository {
private final Authorizer authorizer;
public ColumnRepository(Authorizer authorizer) {
this.authorizer = authorizer;
}
public Column updateColumnByFQN(
UriInfo uriInfo,
String user,
SecurityContext securityContext,
String columnFQN,
String entityType,
UpdateColumn updateColumn) {
@ -61,17 +71,20 @@ public class ColumnRepository {
}
EntityReference parentEntityRef = getParentEntityByFQN(parentFQN, entityType);
String user = securityContext.getUserPrincipal().getName();
if (TABLE.equals(entityType)) {
return updateTableColumn(uriInfo, user, columnFQN, updateColumn, parentEntityRef);
return updateTableColumn(
uriInfo, securityContext, user, columnFQN, updateColumn, parentEntityRef);
} else {
return updateDashboardDataModelColumn(
uriInfo, user, columnFQN, updateColumn, parentEntityRef);
uriInfo, securityContext, user, columnFQN, updateColumn, parentEntityRef);
}
}
private Column updateTableColumn(
UriInfo uriInfo,
SecurityContext securityContext,
String user,
String columnFQN,
UpdateColumn updateColumn,
@ -120,6 +133,14 @@ public class ColumnRepository {
}
JsonPatch jsonPatch = JsonUtils.getJsonPatch(originalTable, updatedTable);
// Authorize the patch operation
OperationContext operationContext = new OperationContext(TABLE, jsonPatch);
ResourceContextInterface resourceContext =
new ResourceContext<>(
TABLE, parentEntityRef.getId(), null, ResourceContextInterface.Operation.PATCH);
authorizer.authorize(securityContext, operationContext, resourceContext);
tableRepository.patch(uriInfo, parentEntityRef.getId(), user, jsonPatch);
return column;
@ -127,6 +148,7 @@ public class ColumnRepository {
private Column updateDashboardDataModelColumn(
UriInfo uriInfo,
SecurityContext securityContext,
String user,
String columnFQN,
UpdateColumn updateColumn,
@ -173,6 +195,17 @@ public class ColumnRepository {
}
JsonPatch jsonPatch = JsonUtils.getJsonPatch(originalDataModel, updatedDataModel);
// Authorize the patch operation
OperationContext operationContext = new OperationContext(DASHBOARD_DATA_MODEL, jsonPatch);
ResourceContextInterface resourceContext =
new ResourceContext<>(
DASHBOARD_DATA_MODEL,
parentEntityRef.getId(),
null,
ResourceContextInterface.Operation.PATCH);
authorizer.authorize(securityContext, operationContext, resourceContext);
dataModelRepository.patch(uriInfo, parentEntityRef.getId(), user, jsonPatch);
return column;

8
openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/TableRepository.java
View File

@ -1681,12 +1681,8 @@ public class TableRepository extends EntityRepository<Table> {
&& column.getName().toLowerCase().contains(searchTerm)) {
return true;
}
if (column.getDisplayName() != null
&& column.getDisplayName().toLowerCase().contains(searchTerm)) {
return true;
}
return column.getDescription() != null
&& column.getDescription().toLowerCase().contains(searchTerm);
return column.getDisplayName() != null
&& column.getDisplayName().toLowerCase().contains(searchTerm);
})
.toList();
}

5
openmetadata-service/src/main/java/org/openmetadata/service/resources/columns/ColumnResource.java
View File

@ -58,7 +58,7 @@ public class ColumnResource {
public ColumnResource(Authorizer authorizer) {
this.authorizer = authorizer;
this.repository = new ColumnRepository();
this.repository = new ColumnRepository(authorizer);
}
@PUT
@ -120,8 +120,7 @@ public class ColumnResource {
UpdateColumn updateColumn) {
Column updatedColumn =
repository.updateColumnByFQN(
uriInfo, securityContext.getUserPrincipal().getName(), fqn, entityType, updateColumn);
repository.updateColumnByFQN(uriInfo, securityContext, fqn, entityType, updateColumn);
return Response.ok(updatedColumn).build();
}

2
openmetadata-service/src/test/java/org/openmetadata/service/resources/EntityResourceTest.java
View File

@ -291,6 +291,8 @@ public abstract class EntityResourceTest<T extends EntityInterface, K extends Cr
public static EntityReference USER1_REF;
public static User USER2;
public static EntityReference USER2_REF;
public static User USER3; // User with no roles for permission testing
public static EntityReference USER3_REF;
public static User USER_TEAM21;
public static User BOT_USER;
public static EntityReference DEFAULT_BOT_ROLE_REF;

128
openmetadata-service/src/test/java/org/openmetadata/service/resources/databases/TableResourceTest.java
View File

@ -111,6 +111,7 @@ import org.openmetadata.schema.api.data.CreateQuery;
import org.openmetadata.schema.api.data.CreateTable;
import org.openmetadata.schema.api.data.CreateTableProfile;
import org.openmetadata.schema.api.data.RestoreEntity;
import org.openmetadata.schema.api.data.UpdateColumn;
import org.openmetadata.schema.api.services.CreateDatabaseService;
import org.openmetadata.schema.api.tests.CreateCustomMetric;
import org.openmetadata.schema.api.tests.CreateTestCase;
@ -120,6 +121,7 @@ import org.openmetadata.schema.entity.data.DatabaseSchema;
import org.openmetadata.schema.entity.data.Query;
import org.openmetadata.schema.entity.data.Table;
import org.openmetadata.schema.entity.services.DatabaseService;
import org.openmetadata.schema.entity.teams.Team;
import org.openmetadata.schema.entity.teams.User;
import org.openmetadata.schema.tests.CustomMetric;
import org.openmetadata.schema.tests.TestCase;
@ -171,8 +173,10 @@ import org.openmetadata.service.resources.query.QueryResourceTest;
import org.openmetadata.service.resources.services.DatabaseServiceResourceTest;
import org.openmetadata.service.resources.tags.ClassificationResourceTest;
import org.openmetadata.service.resources.tags.TagResourceTest;
import org.openmetadata.service.resources.teams.TeamResourceTest;
import org.openmetadata.service.resources.teams.UserResourceTest;
import org.openmetadata.service.search.models.IndexMapping;
import org.openmetadata.service.util.EntityUtil;
import org.openmetadata.service.util.EntityUtil.Fields;
import org.openmetadata.service.util.FullyQualifiedName;
import org.openmetadata.service.util.JsonUtils;
@ -2021,7 +2025,8 @@ public class TableResourceTest extends EntityResourceTest<Table, CreateTable> {
.get("description")
.getChangeSource());
assertChangeSummaryInSearch(updated);
// changeSummary is no longer included in search results
// assertChangeSummaryInSearch(updated);
Table automatedUpdate = JsonUtils.deepCopy(updated, Table.class);
automatedUpdate.setDescription("automated description");
@ -3945,19 +3950,22 @@ public class TableResourceTest extends EntityResourceTest<Table, CreateTable> {
TableResource.TableColumnList response =
TestUtils.get(target, TableResource.TableColumnList.class, ADMIN_AUTH_HEADERS);
assertEquals(1, response.getData().size());
assertEquals("user_id", response.getData().get(0).getName());
assertEquals("user_id", response.getData().getFirst().getName());
assertEquals(1, response.getPaging().getTotal());
target = getResource("tables/" + table.getId() + "/columns/search").queryParam("q", "price");
response = TestUtils.get(target, TableResource.TableColumnList.class, ADMIN_AUTH_HEADERS);
assertEquals(2, response.getData().size());
assertEquals("price_history", response.getData().get(0).getName());
assertEquals(1, response.getData().size());
// Both order_total (description contains "price") and price_history should be in results
Set<String> resultNames =
response.getData().stream().map(Column::getName).collect(Collectors.toSet());
assertTrue(resultNames.contains("price_history"));
assertEquals(1, response.getPaging().getTotal());
target = getResource("tables/" + table.getId() + "/columns/search").queryParam("q", "EMAIL");
response = TestUtils.get(target, TableResource.TableColumnList.class, ADMIN_AUTH_HEADERS);
assertEquals(1, response.getData().size());
assertEquals("email_address", response.getData().get(0).getName());
assertEquals("email_address", response.getData().getFirst().getName());
target =
getResource("tables/" + table.getId() + "/columns/search")
@ -4153,7 +4161,7 @@ public class TableResourceTest extends EntityResourceTest<Table, CreateTable> {
void test_updateColumn_ownerCanUpdateOwnedTableColumns(TestInfo test) throws IOException {
CreateTable create = createRequest(test).withOwners(listOf(DATA_STEWARD.getEntityReference()));
Table table = createAndCheckEntity(create, ADMIN_AUTH_HEADERS);
String columnFQN = COLUMNS.get(0).getFullyQualifiedName();
String columnFQN = table.getColumns().getFirst().getFullyQualifiedName();
org.openmetadata.schema.api.data.UpdateColumn updateColumn =
new org.openmetadata.schema.api.data.UpdateColumn();
@ -4171,7 +4179,7 @@ public class TableResourceTest extends EntityResourceTest<Table, CreateTable> {
void test_updateColumn_dataStewardCanUpdateDescriptionAndTags(TestInfo test) throws IOException {
CreateTable create = createRequest(test).withOwners(listOf(USER1.getEntityReference()));
Table table = createAndCheckEntity(create, ADMIN_AUTH_HEADERS);
String columnFQN = COLUMNS.get(0).getFullyQualifiedName();
String columnFQN = table.getColumns().get(0).getFullyQualifiedName();
org.openmetadata.schema.api.data.UpdateColumn updateColumn =
new org.openmetadata.schema.api.data.UpdateColumn();
@ -4193,74 +4201,108 @@ public class TableResourceTest extends EntityResourceTest<Table, CreateTable> {
@Test
void test_updateColumn_dataConsumerCannotUpdateColumns(TestInfo test) throws IOException {
// Temporarily remove Organization's default roles to ensure USER3 has no permissions
Team org = getOrganization();
List<EntityReference> originalDefaultRoles = org.getDefaultRoles();
updateOrganizationDefaultRoles(null);
try {
CreateTable create = createRequest(test).withOwners(listOf(USER1.getEntityReference()));
Table table = createAndCheckEntity(create, ADMIN_AUTH_HEADERS);
String columnFQN = table.getFullyQualifiedName() + "." + COLUMNS.get(0).getName();
String columnFQN = table.getColumns().getFirst().getFullyQualifiedName();
org.openmetadata.schema.api.data.UpdateColumn updateColumn =
new org.openmetadata.schema.api.data.UpdateColumn();
updateColumn.setDescription("Data consumer trying to update");
Map<String, String> dataConsumerAuthHeaders = authHeaders(DATA_CONSUMER.getName());
assertResponse(
() -> updateColumnByFQN(columnFQN, updateColumn, dataConsumerAuthHeaders),
() -> updateColumnByFQN(columnFQN, updateColumn, authHeaders(USER3.getName())),
FORBIDDEN,
permissionNotAllowed(DATA_CONSUMER.getName(), List.of(MetadataOperation.EDIT_DESCRIPTION)));
permissionNotAllowed(USER3.getName(), List.of(MetadataOperation.EDIT_DESCRIPTION)));
} finally {
// Restore original default roles
updateOrganizationDefaultRoles(originalDefaultRoles);
}
}
@Test
void test_updateColumn_nonOwnerCannotUpdateDisplayName(TestInfo test) throws IOException {
CreateTable create = createRequest(test).withOwners(listOf(DATA_STEWARD.getEntityReference()));
Table table = createAndCheckEntity(create, ADMIN_AUTH_HEADERS);
String columnFQN = table.getFullyQualifiedName() + "." + COLUMNS.get(0).getName();
Team org = getOrganization();
List<EntityReference> originalDefaultRoles = org.getDefaultRoles();
updateOrganizationDefaultRoles(null);
org.openmetadata.schema.api.data.UpdateColumn updateColumn =
new org.openmetadata.schema.api.data.UpdateColumn();
try {
CreateTable create =
createRequest(test).withOwners(listOf(DATA_STEWARD.getEntityReference()));
Table table = createAndCheckEntity(create, ADMIN_AUTH_HEADERS);
String columnFQN = table.getColumns().getFirst().getFullyQualifiedName();
UpdateColumn updateColumn = new UpdateColumn();
updateColumn.setDisplayName("Non-owner trying to update display name");
Map<String, String> user1AuthHeaders = authHeaders(USER1.getName());
Map<String, String> user3AuthHeaders = authHeaders(USER3.getName());
assertResponse(
() -> updateColumnByFQN(columnFQN, updateColumn, user1AuthHeaders),
() -> updateColumnByFQN(columnFQN, updateColumn, user3AuthHeaders),
FORBIDDEN,
permissionNotAllowed(USER1.getName(), List.of(MetadataOperation.EDIT_ALL)));
permissionNotAllowed(USER3.getName(), List.of(MetadataOperation.EDIT_DISPLAY_NAME)));
} finally {
updateOrganizationDefaultRoles(originalDefaultRoles);
}
}
@Test
void test_updateColumn_nonOwnerCannotUpdateConstraints(TestInfo test) throws IOException {
CreateTable create = createRequest(test).withOwners(listOf(DATA_STEWARD.getEntityReference()));
Table table = createAndCheckEntity(create, ADMIN_AUTH_HEADERS);
String columnFQN = table.getFullyQualifiedName() + "." + COLUMNS.get(0).getName();
// Temporarily remove Organization's default roles to ensure USER3 has no permissions
Team org = getOrganization();
List<EntityReference> originalDefaultRoles = org.getDefaultRoles();
updateOrganizationDefaultRoles(null);
org.openmetadata.schema.api.data.UpdateColumn updateColumn =
new org.openmetadata.schema.api.data.UpdateColumn();
try {
CreateTable create =
createRequest(test).withOwners(listOf(DATA_STEWARD.getEntityReference()));
Table table = createAndCheckEntity(create, ADMIN_AUTH_HEADERS);
String columnFQN = table.getColumns().getFirst().getFullyQualifiedName();
UpdateColumn updateColumn = new UpdateColumn();
updateColumn.setConstraint(ColumnConstraint.UNIQUE);
Map<String, String> user1AuthHeaders = authHeaders(USER1.getName());
Map<String, String> user3AuthHeaders = authHeaders(USER3.getName());
assertResponse(
() -> updateColumnByFQN(columnFQN, updateColumn, user1AuthHeaders),
() -> updateColumnByFQN(columnFQN, updateColumn, user3AuthHeaders),
FORBIDDEN,
permissionNotAllowed(USER1.getName(), List.of(MetadataOperation.EDIT_ALL)));
permissionNotAllowed(USER3.getName(), List.of(MetadataOperation.EDIT_ALL)));
} finally {
// Restore original default roles
updateOrganizationDefaultRoles(originalDefaultRoles);
}
}
@Test
void test_updateColumn_userCannotUpdateOtherUsersTableColumns(TestInfo test) throws IOException {
// Temporarily remove Organization's default roles to ensure USER3 has no permissions
Team org = getOrganization();
List<EntityReference> originalDefaultRoles = org.getDefaultRoles();
updateOrganizationDefaultRoles(null);
try {
CreateTable create = createRequest(test).withOwners(listOf(USER1.getEntityReference()));
Table table = createAndCheckEntity(create, ADMIN_AUTH_HEADERS);
String columnFQN = table.getFullyQualifiedName() + "." + COLUMNS.get(0).getName();
org.openmetadata.schema.api.data.UpdateColumn updateColumn =
new org.openmetadata.schema.api.data.UpdateColumn();
updateColumn.setDescription("USER2 trying to update USER1's table");
UpdateColumn updateColumn = new UpdateColumn();
updateColumn.setDescription("USER3 trying to update USER1's table");
Map<String, String> user2AuthHeaders = authHeaders(USER2.getName());
Map<String, String> user3AuthHeaders = authHeaders(USER3.getName());
assertResponse(
() -> updateColumnByFQN(columnFQN, updateColumn, user2AuthHeaders),
() -> updateColumnByFQN(columnFQN, updateColumn, user3AuthHeaders),
FORBIDDEN,
permissionNotAllowed(USER2.getName(), List.of(MetadataOperation.EDIT_DESCRIPTION)));
permissionNotAllowed(USER3.getName(), List.of(MetadataOperation.EDIT_DESCRIPTION)));
} finally {
updateOrganizationDefaultRoles(originalDefaultRoles);
}
}
@Test
@ -4283,7 +4325,23 @@ public class TableResourceTest extends EntityResourceTest<Table, CreateTable> {
org.openmetadata.schema.api.data.UpdateColumn updateColumn,
Map<String, String> authHeaders)
throws IOException {
WebTarget target = getResource("columns/name/" + columnFQN).queryParam("entityType", "table");
String encodedFQN = EntityUtil.encodeEntityFqn(columnFQN);
WebTarget target = getResource("columns/name/" + encodedFQN).queryParam("entityType", "table");
return TestUtils.put(target, updateColumn, Column.class, OK, authHeaders);
}
private Team getOrganization() throws IOException {
TeamResourceTest teamResourceTest = new TeamResourceTest();
return teamResourceTest.getEntityByName(
"Organization", teamResourceTest.getAllowedFields(), ADMIN_AUTH_HEADERS);
}
private void updateOrganizationDefaultRoles(List<EntityReference> defaultRoles)
throws IOException {
Team org = getOrganization();
String json = JsonUtils.pojoToJson(org);
org.setDefaultRoles(defaultRoles);
TeamResourceTest teamResourceTest = new TeamResourceTest();
teamResourceTest.patchEntity(org.getId(), json, org, ADMIN_AUTH_HEADERS);
}
}

5
openmetadata-service/src/test/java/org/openmetadata/service/resources/teams/UserResourceTest.java
View File

@ -194,6 +194,11 @@ public class UserResourceTest extends EntityResourceTest<User, CreateUser> {
USER_TEAM21 = createEntity(create, ADMIN_AUTH_HEADERS);
USER2_REF = USER2.getEntityReference();
// USER3 with no roles for permission testing
create = createRequest(test, 3).withRoles(List.of());
USER3 = createEntity(create, ADMIN_AUTH_HEADERS);
USER3_REF = USER3.getEntityReference();
Set<String> userFields = Entity.getEntityFields(User.class);
userFields.remove("authenticationMechanism");
BOT_USER = getEntityByName(INGESTION_BOT, String.join(",", userFields), ADMIN_AUTH_HEADERS);