Skip to content

Commit

Permalink
Issue #17012: Multi User/Team Ownership - Fix Tests - Part 6
Browse files Browse the repository at this point in the history
  • Loading branch information
harshach committed Jul 20, 2024
1 parent 7748155 commit 9765670
Show file tree
Hide file tree
Showing 15 changed files with 62 additions and 45 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ public final void addRecord(CsvFile csvFile, List<String> recordList) {
csvFile.withRecords(list);
}

/** Owner field is in entityType;entityName format */
/** Owner field is in entityType:entityName format */
public List<EntityReference> getOwners(CSVPrinter printer, CSVRecord csvRecord, int fieldNumber)
throws IOException {
if (!processRecord) {
Expand Down Expand Up @@ -524,7 +524,7 @@ public static String columnNotFound(int field, String columnFqn) {
}

public static String invalidOwner(int field) {
String error = "Owner should be of format user;userName or team;teamName";
String error = "Owner should be of format user:userName or team:teamName";
return String.format(FIELD_ERROR_MSG, CsvErrorType.INVALID_FIELD, field + 1, error);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import static org.openmetadata.csv.CsvUtil.addEntityReference;
import static org.openmetadata.csv.CsvUtil.addEntityReferences;
import static org.openmetadata.csv.CsvUtil.addField;
import static org.openmetadata.csv.CsvUtil.addOwners;
import static org.openmetadata.csv.CsvUtil.addTagLabels;
import static org.openmetadata.service.Entity.GLOSSARY;
import static org.openmetadata.service.Entity.GLOSSARY_TERM;
Expand Down Expand Up @@ -260,7 +261,7 @@ protected void addRecord(CsvFile csvFile, GlossaryTerm entity) {
addField(recordList, termReferencesToRecord(entity.getReferences()));
addTagLabels(recordList, entity.getTags());
addField(recordList, reviewerReferencesToRecord(entity.getReviewers()));
addField(recordList, reviewerOwnerReferencesToRecord(entity.getOwners()));
addOwners(recordList, entity.getOwners());
addField(recordList, entity.getStatus().value());
addRecord(csvFile, recordList);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -523,7 +523,7 @@ public void afterAllTests() throws Exception {
// Create request such as CreateTable, CreateChart returned by concrete implementation
public final K createRequest(TestInfo test) {
K createRequest = createRequest(getEntityName(test)).withDescription("").withDisplayName(null);
createRequest.setOwners(null);
createRequest.setOwners(emptyList());
return createRequest;
}

Expand Down Expand Up @@ -3671,7 +3671,10 @@ public static <T extends EntityInterface> List<EntityReference> reduceEntityRefe
List<EntityReference> entities) {
List<EntityReference> reducedEntities = new ArrayList<>();
for (EntityReference entity : listOrEmpty(entities)) {
reducedEntities.add(reduceEntityReference(entity));
EntityReference reducedRef = reduceEntityReference(entity);
if (reducedRef != null) {
reducedEntities.add(reducedRef);
}
}
return reducedEntities;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,7 @@ public DataProduct validateGetWithDifferentFields(DataProduct dataProduct, boole
? getEntityByName(dataProduct.getFullyQualifiedName(), null, ADMIN_AUTH_HEADERS)
: getEntity(dataProduct.getId(), null, ADMIN_AUTH_HEADERS);
assertListNull(getDataProduct.getOwners(), getDataProduct.getExperts());
String fields = "owner,domain,experts,assets";
String fields = "owners,domain,experts,assets";
getDataProduct =
byName
? getEntityByName(getDataProduct.getFullyQualifiedName(), fields, ADMIN_AUTH_HEADERS)
Expand All @@ -240,7 +240,7 @@ public DataProduct validateGetWithDifferentFields(DataProduct dataProduct, boole
assertEntityReferences(dataProduct.getExperts(), getDataProduct.getExperts());
assertEntityReferences(dataProduct.getAssets(), getDataProduct.getAssets());

// Checks for other owner, tags, and followers is done in the base class
// Checks for other owners, tags, and followers is done in the base class
return getDataProduct;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -847,11 +847,11 @@ void test_getSimplelistFromSearch(TestInfo testInfo) throws IOException, ParseEx
testCasesNum, allEntities.getData().size()); // Should return either values matching

queryParams.clear();
queryParams.put("owner", USER2_REF.getName());
queryParams.put("owners", USER2_REF.getName());
allEntities = listEntitiesFromSearch(queryParams, testCasesNum, 0, ADMIN_AUTH_HEADERS);
assertEquals(2, allEntities.getData().size()); // we have 2 test cases with USER2_REF as owner

queryParams.put("owner", USER_TEAM21.getName());
queryParams.put("owners", USER_TEAM21.getName());
allEntities = listEntitiesFromSearch(queryParams, testCasesNum, 0, ADMIN_AUTH_HEADERS);
assertEquals(
1,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -541,26 +541,26 @@ void testGlossaryImportExport() throws IOException {
List<String> createRecords =
listOf(
String.format(
",g1,dsp1,\"dsc1,1\",h1;h2;h3,,term1;http://term1,PII.None,%s;%s,user;%s,%s",
",g1,dsp1,\"dsc1,1\",h1;h2;h3,,term1;http://term1,PII.None,%s;%s,user:%s,%s",
user1, user2, user1, "Approved"),
String.format(
",g2,dsp2,dsc3,h1;h3;h3,,term2;https://term2,PII.NonSensitive,%s,user;%s,%s",
",g2,dsp2,dsc3,h1;h3;h3,,term2;https://term2,PII.NonSensitive,%s,user:%s,%s",
user1, user2, "Approved"),
String.format(
"importExportTest.g1,g11,dsp2,dsc11,h1;h3;h3,,,,%s;%s,team;%s,%s",
"importExportTest.g1,g11,dsp2,dsc11,h1;h3;h3,,,,%s;%s,team:%s,%s",
user1, user2, team11, "Draft"));

// Update terms with change in description
List<String> updateRecords =
listOf(
String.format(
",g1,dsp1,new-dsc1,h1;h2;h3,,term1;http://term1,PII.None,%s;%s,user;%s,%s",
",g1,dsp1,new-dsc1,h1;h2;h3,,term1;http://term1,PII.None,%s;%s,user:%s,%s",
user1, user2, user1, "Approved"),
String.format(
",g2,dsp2,new-dsc3,h1;h3;h3,,term2;https://term2,PII.NonSensitive,%s,user;%s,%s",
",g2,dsp2,new-dsc3,h1;h3;h3,,term2;https://term2,PII.NonSensitive,%s,user:%s,%s",
user1, user2, "Approved"),
String.format(
"importExportTest.g1,g11,dsp2,new-dsc11,h1;h3;h3,,,,%s;%s,team;%s,%s",
"importExportTest.g1,g11,dsp2,new-dsc11,h1;h3;h3,,,,%s;%s,team:%s,%s",
user1, user2, team11, "Draft"));

// Add new row to existing rows
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

package org.openmetadata.service.resources.glossary;

import static java.util.Collections.emptyList;
import static javax.ws.rs.core.Response.Status.BAD_REQUEST;
import static javax.ws.rs.core.Response.Status.FORBIDDEN;
import static javax.ws.rs.core.Response.Status.NOT_FOUND;
Expand Down Expand Up @@ -113,7 +114,7 @@ void get_listGlossaryTermsWithDifferentFilters() throws IOException {
// - term1
// - term11
// - term12
Glossary glossary1 = createGlossary("glossãry1", null, null);
Glossary glossary1 = createGlossary("glossãry1", null, emptyList());

GlossaryTerm term1 = createTerm(glossary1, null, "term1");
GlossaryTerm term11 = createTerm(glossary1, term1, "term11");
Expand Down Expand Up @@ -178,7 +179,7 @@ void test_inheritGlossaryReviewerAndOwner(TestInfo test) throws IOException {
//
// When reviewers are not set for a glossary term, carry it forward from the glossary
//
Glossary glossary = createGlossary(test, listOf(USER1_REF), USER2_REF);
Glossary glossary = createGlossary(test, listOf(USER1_REF), List.of(USER2_REF));

// Create term t1 in the glossary without reviewers and owner
CreateGlossaryTerm create =
Expand All @@ -187,7 +188,7 @@ void test_inheritGlossaryReviewerAndOwner(TestInfo test) throws IOException {
.withGlossary(glossary.getFullyQualifiedName())
.withDescription("desc");
GlossaryTerm t1 = assertOwnerInheritance(create, USER2_REF);
t1 = getEntity(t1.getId(), "reviewers,owner", ADMIN_AUTH_HEADERS);
t1 = getEntity(t1.getId(), "reviewers,owners", ADMIN_AUTH_HEADERS);
assertEntityReferences(glossary.getReviewers(), t1.getReviewers()); // Reviewers are inherited

// Create term t12 under t1 without reviewers and owner
Expand Down Expand Up @@ -571,19 +572,25 @@ void patch_usingFqn_addDeleteTags(TestInfo test) throws IOException {
@Test
void testInheritedPermissionFromParent(TestInfo test) throws IOException {
// Glossary g has owner dataConsumer
Glossary g = createGlossary(getEntityName(test), null, DATA_CONSUMER.getEntityReference());
Glossary g =
createGlossary(getEntityName(test), null, List.of(DATA_CONSUMER.getEntityReference()));
// dataConsumer as owner of g can create glossary term t1 under it
GlossaryTerm t1 =
createTerm(
g,
null,
"t1",
null,
DATA_STEWARD.getEntityReference(),
List.of(DATA_STEWARD.getEntityReference()),
authHeaders(DATA_CONSUMER.getName()));
// dataSteward who is owner of term t1 can create term t11 under it
createTerm(
g, t1, "t11", null, DATA_STEWARD.getEntityReference(), authHeaders(DATA_STEWARD.getName()));
g,
t1,
"t11",
null,
List.of(DATA_STEWARD.getEntityReference()),
authHeaders(DATA_STEWARD.getName()));
}

protected Thread assertApprovalTask(GlossaryTerm term, TaskStatus expectedTaskStatus)
Expand Down Expand Up @@ -697,7 +704,7 @@ void patch_addDeleteStyle(TestInfo test) throws IOException {

@Test
void delete_recursive(TestInfo test) throws IOException {
Glossary g1 = createGlossary(test, null, null);
Glossary g1 = createGlossary(test, null, emptyList());

// Create glossary term t1 in glossary g1
GlossaryTerm t1 = createTerm(g1, null, "t1");
Expand Down Expand Up @@ -850,23 +857,23 @@ public GlossaryTerm createTerm(Glossary glossary, GlossaryTerm parent, String te
public GlossaryTerm createTerm(
Glossary glossary, GlossaryTerm parent, String termName, List<EntityReference> reviewers)
throws IOException {
return createTerm(glossary, parent, termName, reviewers, null, ADMIN_AUTH_HEADERS);
return createTerm(glossary, parent, termName, reviewers, emptyList(), ADMIN_AUTH_HEADERS);
}

public GlossaryTerm createTerm(
Glossary glossary,
GlossaryTerm parent,
String termName,
List<EntityReference> reviewers,
EntityReference owner,
List<EntityReference> owners,
Map<String, String> createdBy)
throws IOException {
CreateGlossaryTerm createGlossaryTerm =
createRequest(termName, "", "", null)
.withGlossary(getFqn(glossary))
.withStyle(new Style().withColor("#FF5733").withIconURL("https://img"))
.withParent(getFqn(parent))
.withOwners(List.of(owner))
.withOwners(owners)
.withReviewers(reviewers);
return createAndCheckEntity(createGlossaryTerm, createdBy);
}
Expand Down Expand Up @@ -1145,14 +1152,16 @@ public void test_getImmediateChildrenGlossaryTermsWithParentFQN() throws IOExcep
}

public Glossary createGlossary(
TestInfo test, List<EntityReference> reviewers, EntityReference owner) throws IOException {
return createGlossary(glossaryTest.getEntityName(test), reviewers, owner);
TestInfo test, List<EntityReference> reviewers, List<EntityReference> owners)
throws IOException {
return createGlossary(glossaryTest.getEntityName(test), reviewers, owners);
}

public Glossary createGlossary(
String name, List<EntityReference> reviewers, EntityReference owner) throws IOException {
String name, List<EntityReference> reviewers, List<EntityReference> owners)
throws IOException {
CreateGlossary create =
glossaryTest.createRequest(name).withReviewers(reviewers).withOwners(List.of(owner));
glossaryTest.createRequest(name).withReviewers(reviewers).withOwners(owners);
return glossaryTest.createAndCheckEntity(create, ADMIN_AUTH_HEADERS);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import static org.openmetadata.service.Entity.FIELD_OWNERS;
import static org.openmetadata.service.Entity.TAG;
import static org.openmetadata.service.util.EntityUtil.fieldAdded;
import static org.openmetadata.service.util.EntityUtil.fieldDeleted;
import static org.openmetadata.service.util.EntityUtil.fieldUpdated;
import static org.openmetadata.service.util.TestUtils.ADMIN_AUTH_HEADERS;
import static org.openmetadata.service.util.TestUtils.UpdateType.MINOR_UPDATE;
Expand Down Expand Up @@ -156,7 +157,8 @@ void put_searchIndexAttributes_200_ok(TestInfo test) throws IOException {
SearchIndexField addedField = fields.get(2);
addedField.setFullyQualifiedName(
searchIndex.getFields().get(0).getFullyQualifiedName() + "." + addedField.getName());
fieldUpdated(change, FIELD_OWNERS, USER1_REF, TEAM11_REF);
fieldDeleted(change, FIELD_OWNERS, List.of(USER1_REF));
fieldAdded(change, FIELD_OWNERS, List.of(TEAM11_REF));
fieldUpdated(change, "description", "", "searchIndex");
fieldAdded(change, "fields.tableSearchIndex", JsonUtils.pojoToJson(List.of(addedField)));
updateAndCheckEntity(createSearchIndex, Status.OK, ADMIN_AUTH_HEADERS, MINOR_UPDATE, change);
Expand Down Expand Up @@ -221,7 +223,8 @@ void patch_searchIndexAttributes_200_ok(TestInfo test) throws IOException {
searchIndex.getFullyQualifiedName() + "." + addedField.getName());

ChangeDescription change = getChangeDescription(searchIndex, MINOR_UPDATE);
fieldUpdated(change, FIELD_OWNERS, USER1_REF, TEAM11_REF);
fieldDeleted(change, FIELD_OWNERS, List.of(USER1_REF));
fieldAdded(change, FIELD_OWNERS, List.of(TEAM11_REF));
fieldAdded(change, "fields", JsonUtils.pojoToJson(List.of(addedField)));
patchEntityAndCheck(searchIndex, origJson, ADMIN_AUTH_HEADERS, MINOR_UPDATE, change);
}
Expand Down Expand Up @@ -344,7 +347,8 @@ void patch_usingFqn_searchIndexAttributes_200_ok(TestInfo test) throws IOExcepti
searchIndex.getFullyQualifiedName() + "." + addedField.getName());

ChangeDescription change = getChangeDescription(searchIndex, MINOR_UPDATE);
fieldUpdated(change, FIELD_OWNERS, USER1_REF, TEAM11_REF);
fieldAdded(change, FIELD_OWNERS, List.of(TEAM11_REF));
fieldDeleted(change, FIELD_OWNERS, List.of(USER1_REF));
fieldAdded(change, "fields", JsonUtils.pojoToJson(List.of(addedField)));
patchEntityUsingFqnAndCheck(searchIndex, origJson, ADMIN_AUTH_HEADERS, MINOR_UPDATE, change);
}
Expand Down Expand Up @@ -419,7 +423,7 @@ public SearchIndex validateGetWithDifferentFields(SearchIndex searchIndex, boole
: getSearchIndex(searchIndex.getId(), fields, ADMIN_AUTH_HEADERS);
assertListNull(searchIndex.getOwners(), searchIndex.getFollowers(), searchIndex.getFollowers());

fields = "owner, followers, tags";
fields = "owners, followers, tags";
searchIndex =
byName
? getSearchIndexByName(searchIndex.getFullyQualifiedName(), fields, ADMIN_AUTH_HEADERS)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -672,7 +672,7 @@ void put_IngestionPipelineUpdate_200(TestInfo test) throws IOException {
// Add description and tasks
ChangeDescription change = getChangeDescription(ingestion, MINOR_UPDATE);
fieldAdded(change, "description", "newDescription");
fieldAdded(change, FIELD_OWNERS, USER1_REF);
fieldAdded(change, FIELD_OWNERS, List.of(USER1_REF));
updateAndCheckEntity(
request.withDescription("newDescription").withOwners(List.of(USER1_REF)),
OK,
Expand Down Expand Up @@ -880,7 +880,7 @@ public IngestionPipeline validateGetWithDifferentFields(
? getEntityByName(ingestion.getFullyQualifiedName(), fields, ADMIN_AUTH_HEADERS)
: getEntity(ingestion.getId(), fields, ADMIN_AUTH_HEADERS);
assertListNotNull(ingestion.getService());
assertListNull(ingestion.getOwner());
assertListNull(ingestion.getOwners());

fields = FIELD_OWNERS;
ingestion =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -803,7 +803,7 @@ public Container validateGetWithDifferentFields(Container container, boolean byN

// .../models?fields=dataModel - parent,children are not set in createEntity - these are tested
// separately
String fields = "dataModel,owner,tags,followers,extension";
String fields = "dataModel,owners,tags,followers,extension";
container =
byName
? getEntityByName(container.getFullyQualifiedName(), fields, ADMIN_AUTH_HEADERS)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@
"description": "Description of the testcase.",
"$ref": "../../type/basic.json#/definitions/markdown"
},
"owner": {
"description": "Owner of this TestCase definition.",
"owners": {
"description": "Owners of this TestCase definition.",
"$ref": "../../type/entityReferenceList.json"
},
"entityType": {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@
"openMetadataServerConnection": {
"$ref": "../services/connections/metadata/openMetadataConnection.json"
},
"owner": {
"owners": {
"description": "Owners of this workflow.",
"$ref": "../../type/entityReferenceList.json",
"default": null
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -142,8 +142,8 @@
"pipelineType": {
"$ref": "#/definitions/pipelineType"
},
"owner": {
"description": "Owner of this Pipeline.",
"owners": {
"description": "Owners of this Pipeline.",
"$ref": "../../../type/entityReferenceList.json",
"default": null
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@
"description": "A short description of the Alert, comprehensible to regular users.",
"$ref": "../../type/basic.json#/definitions/markdown"
},
"owner": {
"description": "Owner of this Alert.",
"owners": {
"description": "Owners of this Alert.",
"$ref": "../../type/entityReferenceList.json",
"default": null
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -273,8 +273,8 @@
"description": "A short description of the Event Subscription, comprehensible to regular users.",
"$ref": "../type/basic.json#/definitions/markdown"
},
"owner": {
"description": "Owner of this Event Subscription.",
"owners": {
"description": "Owners of this Event Subscription.",
"$ref": "../type/entityReferenceList.json",
"default": null
},
Expand Down

0 comments on commit 9765670

Please sign in to comment.