Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;

import com.fasterxml.jackson.databind.JsonNode;
import es.co.elastic.clients.transport.rest5_client.low_level.Request;
import es.co.elastic.clients.transport.rest5_client.low_level.Response;
import es.co.elastic.clients.transport.rest5_client.low_level.Rest5Client;
Expand All @@ -25,6 +26,7 @@
import org.junit.jupiter.api.parallel.Execution;
import org.junit.jupiter.api.parallel.ExecutionMode;
import org.openmetadata.it.bootstrap.TestSuiteBootstrap;
import org.openmetadata.it.factories.UserTestFactory;
import org.openmetadata.it.util.SdkClients;
import org.openmetadata.it.util.TestNamespace;
import org.openmetadata.schema.api.data.CreateTable;
Expand All @@ -39,6 +41,7 @@
import org.openmetadata.schema.entity.services.ingestionPipelines.PipelineStatus;
import org.openmetadata.schema.entity.services.ingestionPipelines.PipelineStatusType;
import org.openmetadata.schema.entity.services.ingestionPipelines.PipelineType;
import org.openmetadata.schema.entity.teams.User;
import org.openmetadata.schema.metadataIngestion.SourceConfig;
import org.openmetadata.schema.metadataIngestion.TestSuitePipeline;
import org.openmetadata.schema.tests.DataQualityReport;
Expand Down Expand Up @@ -386,6 +389,53 @@ void test_testSuiteWithOwner(TestNamespace ns) {
assertFalse(fetched.getOwners().isEmpty());
}

@Test
void get_byName_honorsIncludeRelations_hidesSoftDeletedOwner(TestNamespace ns) {
// The Test Suite detail page fetches getByName with includeRelations=owners:non-deleted so a
// soft-deleted owner is hidden and the owner-edit diff stays aligned with the server's
// NON_DELETED PATCH base - otherwise a positional patch throws "array item index out of range".
// getByName previously ignored includeRelations (unlike getById); this guards that wiring.
// See issue #30117.
OpenMetadataClient client = SdkClients.adminClient();
User active = UserTestFactory.createUser(ns, "aaa_active");
User doomed = UserTestFactory.createUser(ns, "zzz_doomed");
CreateTestSuite request = new CreateTestSuite();
request.setName(ns.prefix("testsuite_getbyname_includerel"));
request.setOwners(List.of(active.getEntityReference(), doomed.getEntityReference()));
String fqn = createEntity(request).getFullyQualifiedName();
client.users().delete(doomed.getId().toString());

// include=all with no includeRelations still surfaces the soft-deleted owner...
assertTrue(
ownerIdsByName(fqn, "").contains(doomed.getId()),
"include=all should surface the soft-deleted owner");

// ...but getByName must honor includeRelations=owners:non-deleted and filter it out.
List<UUID> filtered = ownerIdsByName(fqn, "&includeRelations=owners:non-deleted");
assertFalse(
filtered.contains(doomed.getId()),
"getByName must hide the soft-deleted owner when includeRelations=owners:non-deleted");
assertTrue(filtered.contains(active.getId()), "the active owner must remain");
}

private List<UUID> ownerIdsByName(String fqn, String extraQuery) {
String body =
SdkClients.adminClient()
.getHttpClient()
.executeForString(
HttpMethod.GET,
"/v1/dataQuality/testSuites/name/"
+ fqn
+ "?fields=owners&include=all"
+ extraQuery,
null);
List<UUID> ids = new ArrayList<>();
for (JsonNode owner : JsonUtils.readTree(body).path("owners")) {
ids.add(UUID.fromString(owner.path("id").asText()));
}
return ids;
}

@Test
void test_testSuiteFQNFormat(TestNamespace ns) {
OpenMetadataClient client = SdkClients.adminClient();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -426,8 +426,18 @@ public TestSuite getByName(
schema = @Schema(implementation = Include.class))
@QueryParam("include")
@DefaultValue("non-deleted")
Include include) {
return getByNameInternal(uriInfo, securityContext, name, fieldsParam, include);
Include include,
@Parameter(
description =
"Per-relation include control. Format: field:value,field2:value2. "
+ "Example: owners:non-deleted,followers:all. "
+ "Valid values: all, deleted, non-deleted. "
+ "If not specified for a field, uses the entity's include value.",
schema = @Schema(type = "string", example = "owners:non-deleted,followers:all"))
@QueryParam("includeRelations")
String includeRelations) {
return getByNameInternal(
uriInfo, securityContext, name, fieldsParam, include, includeRelations);
}

@GET
Expand Down
35 changes: 30 additions & 5 deletions openmetadata-ui/src/main/resources/ui/src/rest/testAPI.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -883,7 +883,7 @@ describe('testAPI tests', () => {
});

describe('getTestSuiteByName', () => {
it('should fetch test suite by name with params', async () => {
it('should default owners/experts to non-deleted so soft-deleted owners stay hidden (issue #30117)', async () => {
const mockGet = jest.fn().mockResolvedValue({ data: mockTestSuite });
jest.mock('./index', () => ({
__esModule: true,
Expand All @@ -894,17 +894,42 @@ describe('testAPI tests', () => {

const { getTestSuiteByName } = require('./testAPI');

const params = { fields: ['owners', 'tests'] };

const result = await getTestSuiteByName('test.suite.name', params);
const result = await getTestSuiteByName('test.suite.name', {
fields: ['owners', 'tests'],
});

expect(mockGet).toHaveBeenCalledWith(
expect.stringContaining('/dataQuality/testSuites/name/'),
{ params }
{
params: {
fields: ['owners', 'tests'],
includeRelations: 'owners:non-deleted,experts:non-deleted',
},
}
);
expect(result).toEqual(mockTestSuite);
});

it('should respect a caller-provided includeRelations', async () => {
const mockGet = jest.fn().mockResolvedValue({ data: mockTestSuite });
jest.mock('./index', () => ({
__esModule: true,
default: {
get: mockGet,
},
}));

const { getTestSuiteByName } = require('./testAPI');

await getTestSuiteByName('test.suite.name', {
includeRelations: 'owners:all',
});

expect(mockGet).toHaveBeenCalledWith(expect.any(String), {
params: { includeRelations: 'owners:all' },
});
});

it('should encode FQN with special characters', async () => {
const mockGet = jest.fn().mockResolvedValue({ data: mockTestSuite });
jest.mock('./index', () => ({
Expand Down
12 changes: 11 additions & 1 deletion openmetadata-ui/src/main/resources/ui/src/rest/testAPI.ts
Original file line number Diff line number Diff line change
Expand Up @@ -413,7 +413,17 @@ export const getTestSuiteByName = async (
) => {
const response = await APIClient.get<TestSuite>(
`${testSuiteUrl}/name/${getEncodedFqn(name)}`,
{ params }
{
// Resolve owners/experts as non-deleted (consistent with data-asset detail pages), so a
// soft-deleted owner is not surfaced and the owner-edit diff stays aligned with the server's
// NON_DELETED PATCH base - otherwise a positional patch throws "array item index out of
// range". See issue #30117.
params: {
...params,
includeRelations:
params?.includeRelations ?? 'owners:non-deleted,experts:non-deleted',
},
Comment on lines +421 to +425
}
);

return response.data;
Expand Down
Loading