MINOR: Fix Glossary Term Consolidation and Flaky Test (#18920)

* Fix Glossary Term Consolidation and Flaky Test

* Fix CheckStyle

* Implement the new entitySpecificUpdate in all repositories
This commit is contained in:
IceS2 2024-12-05 05:02:50 +01:00 committed by GitHub
parent 84566e3b21
commit 46d4e1c77c
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
44 changed files with 77 additions and 50 deletions

View File

@ -122,7 +122,7 @@ public class APICollectionRepository extends EntityRepository<APICollection> {
@Transaction
@Override
public void entitySpecificUpdate() {
public void entitySpecificUpdate(boolean consolidatingChanges) {
recordChange("sourceHash", original.getSourceHash(), updated.getSourceHash());
}
}

View File

@ -396,7 +396,7 @@ public class APIEndpointRepository extends EntityRepository<APIEndpoint> {
@Transaction
@Override
public void entitySpecificUpdate() {
public void entitySpecificUpdate(boolean consolidatingChanges) {
recordChange("endpointURL", original.getEndpointURL(), updated.getEndpointURL());
recordChange("requestMethod", original.getRequestMethod(), updated.getRequestMethod());

View File

@ -386,7 +386,7 @@ public class AppRepository extends EntityRepository<App> {
}
@Override
public void entitySpecificUpdate() {
public void entitySpecificUpdate(boolean consolidatingChanges) {
recordChange(
"appConfiguration", original.getAppConfiguration(), updated.getAppConfiguration());
recordChange("appSchedule", original.getAppSchedule(), updated.getAppSchedule());

View File

@ -97,7 +97,7 @@ public class BotRepository extends EntityRepository<Bot> {
@Transaction
@Override
public void entitySpecificUpdate() {
public void entitySpecificUpdate(boolean consolidatingChanges) {
updateUser(original, updated);
}

View File

@ -127,7 +127,7 @@ public class ChartRepository extends EntityRepository<Chart> {
@Transaction
@Override
public void entitySpecificUpdate() {
public void entitySpecificUpdate(boolean consolidatingChanges) {
recordChange("chartType", original.getChartType(), updated.getChartType());
recordChange("sourceUrl", original.getSourceUrl(), updated.getSourceUrl());
recordChange("sourceHash", original.getSourceHash(), updated.getSourceHash());

View File

@ -158,7 +158,7 @@ public class ClassificationRepository extends EntityRepository<Classification> {
@Transaction
@Override
public void entitySpecificUpdate() {
public void entitySpecificUpdate(boolean consolidatingChanges) {
// Mutually exclusive cannot be updated
updated.setMutuallyExclusive(original.getMutuallyExclusive());
recordChange("disabled", original.getDisabled(), updated.getDisabled());

View File

@ -270,7 +270,7 @@ public class ContainerRepository extends EntityRepository<Container> {
@Transaction
@Override
public void entitySpecificUpdate() {
public void entitySpecificUpdate(boolean consolidatingChanges) {
updateDataModel(original, updated);
recordChange("prefix", original.getPrefix(), updated.getPrefix());
List<ContainerFileFormat> addedItems = new ArrayList<>();

View File

@ -210,7 +210,7 @@ public class DashboardDataModelRepository extends EntityRepository<DashboardData
@Transaction
@Override
public void entitySpecificUpdate() {
public void entitySpecificUpdate(boolean consolidatingChanges) {
DatabaseUtil.validateColumns(original.getColumns());
updateColumns("columns", original.getColumns(), updated.getColumns(), EntityUtil.columnMatch);
recordChange("sourceHash", original.getSourceHash(), updated.getSourceHash());

View File

@ -209,7 +209,7 @@ public class DashboardRepository extends EntityRepository<Dashboard> {
@Transaction
@Override
public void entitySpecificUpdate() {
public void entitySpecificUpdate(boolean consolidatingChanges) {
update(
Entity.CHART,
"charts",

View File

@ -135,7 +135,7 @@ public class DataProductRepository extends EntityRepository<DataProduct> {
@Transaction
@Override
public void entitySpecificUpdate() {
public void entitySpecificUpdate(boolean consolidatingChanges) {
updateAssets();
}

View File

@ -246,7 +246,7 @@ public class DatabaseRepository extends EntityRepository<Database> {
@Transaction
@Override
public void entitySpecificUpdate() {
public void entitySpecificUpdate(boolean consolidatingChanges) {
recordChange("retentionPeriod", original.getRetentionPeriod(), updated.getRetentionPeriod());
recordChange("sourceUrl", original.getSourceUrl(), updated.getSourceUrl());
recordChange("sourceHash", original.getSourceHash(), updated.getSourceHash());

View File

@ -223,7 +223,7 @@ public class DatabaseSchemaRepository extends EntityRepository<DatabaseSchema> {
@Transaction
@Override
public void entitySpecificUpdate() {
public void entitySpecificUpdate(boolean consolidatingChanges) {
recordChange("retentionPeriod", original.getRetentionPeriod(), updated.getRetentionPeriod());
recordChange("sourceUrl", original.getSourceUrl(), updated.getSourceUrl());
recordChange("sourceHash", original.getSourceHash(), updated.getSourceHash());

View File

@ -144,7 +144,7 @@ public class DocumentRepository extends EntityRepository<Document> {
@Transaction
@Override
public void entitySpecificUpdate() {
public void entitySpecificUpdate(boolean consolidatingChanges) {
updateEmailTemplatePlaceholders(original, updated);
recordChange("data", original.getData(), updated.getData(), true);
}

View File

@ -147,7 +147,7 @@ public class DomainRepository extends EntityRepository<Domain> {
@Transaction
@Override
public void entitySpecificUpdate() {
public void entitySpecificUpdate(boolean consolidatingChanges) {
recordChange("domainType", original.getDomainType(), updated.getDomainType());
}
}

View File

@ -2668,7 +2668,7 @@ public abstract class EntityRepository<T extends EntityInterface> {
"In session change consolidation. Reverting to previous version {}",
previous.getVersion());
updated = previous;
updateInternal();
updateInternal(true);
LOG.info(
"In session change consolidation. Reverting to previous version {} completed",
previous.getVersion());
@ -2686,6 +2686,12 @@ public abstract class EntityRepository<T extends EntityInterface> {
/** Compare original and updated entities and perform updates. Update the entity version and track changes. */
@Transaction
private void updateInternal() {
updateInternal(false);
}
/** Compare original and updated entities and perform updates. Update the entity version and track changes. */
@Transaction
private void updateInternal(boolean consolidatingChanges) {
if (operation.isDelete()) { // Soft DELETE Operation
updateDeleted();
} else { // PUT or PATCH operations
@ -2704,11 +2710,11 @@ public abstract class EntityRepository<T extends EntityInterface> {
updateStyle();
updateLifeCycle();
updateCertification();
entitySpecificUpdate();
entitySpecificUpdate(consolidatingChanges);
}
}
protected void entitySpecificUpdate() {
protected void entitySpecificUpdate(boolean consolidatingChanges) {
// Default implementation. Override this to add any entity specific field updates
}

View File

@ -134,7 +134,7 @@ public class EventSubscriptionRepository extends EntityRepository<EventSubscript
}
@Override
public void entitySpecificUpdate() {
public void entitySpecificUpdate(boolean consolidatingChanges) {
recordChange("input", original.getInput(), updated.getInput(), true);
recordChange("batchSize", original.getBatchSize(), updated.getBatchSize());
if (!original.getAlertType().equals(CreateEventSubscription.AlertType.ACTIVITY_FEED)) {

View File

@ -386,7 +386,7 @@ public class GlossaryRepository extends EntityRepository<Glossary> {
@Transaction
@Override
public void entitySpecificUpdate() {
public void entitySpecificUpdate(boolean consolidatingChanges) {
updateName(original, updated);
// Mutually exclusive cannot be updated
updated.setMutuallyExclusive(original.getMutuallyExclusive());

View File

@ -934,9 +934,9 @@ public class GlossaryTermRepository extends EntityRepository<GlossaryTerm> {
@Transaction
@Override
public void entitySpecificUpdate() {
public void entitySpecificUpdate(boolean consolidatingChanges) {
validateParent();
updateStatus(original, updated);
updateStatus(original, updated, consolidatingChanges);
updateSynonyms(original, updated);
updateReferences(original, updated);
updateRelatedTerms(original, updated);
@ -1000,12 +1000,14 @@ public class GlossaryTermRepository extends EntityRepository<GlossaryTerm> {
}
}
private void updateStatus(GlossaryTerm origTerm, GlossaryTerm updatedTerm) {
private void updateStatus(
GlossaryTerm origTerm, GlossaryTerm updatedTerm, boolean consolidatingChanges) {
if (origTerm.getStatus() == updatedTerm.getStatus()) {
return;
}
// Only reviewers can change from IN_REVIEW status to APPROVED/REJECTED status
if (origTerm.getStatus() == Status.IN_REVIEW
if (!consolidatingChanges
&& origTerm.getStatus() == Status.IN_REVIEW
&& (updatedTerm.getStatus() == Status.APPROVED
|| updatedTerm.getStatus() == Status.REJECTED)) {
checkUpdatedByReviewer(origTerm, updatedTerm.getUpdatedBy());

View File

@ -300,7 +300,7 @@ public class IngestionPipelineRepository extends EntityRepository<IngestionPipel
@Transaction
@Override
public void entitySpecificUpdate() {
public void entitySpecificUpdate(boolean consolidatingChanges) {
updateSourceConfig();
updateAirflowConfig(original.getAirflowConfig(), updated.getAirflowConfig());
updateLogLevel(original.getLoggerLevel(), updated.getLoggerLevel());

View File

@ -142,7 +142,7 @@ public class KpiRepository extends EntityRepository<Kpi> {
@Transaction
@Override
public void entitySpecificUpdate() {
public void entitySpecificUpdate(boolean consolidatingChanges) {
updateToRelationship(
"dataInsightChart",
KPI,

View File

@ -109,7 +109,7 @@ public class MetricRepository extends EntityRepository<Metric> {
@Transaction
@Override
public void entitySpecificUpdate() {
public void entitySpecificUpdate(boolean consolidatingChanges) {
recordChange("granularity", original.getGranularity(), updated.getGranularity());
recordChange("metricType", original.getMetricType(), updated.getMetricType());
recordChange(

View File

@ -322,7 +322,7 @@ public class MlModelRepository extends EntityRepository<MlModel> {
@Transaction
@Override
public void entitySpecificUpdate() {
public void entitySpecificUpdate(boolean consolidatingChanges) {
updateAlgorithm(original, updated);
updateDashboard(original, updated);
updateMlFeatures(original, updated);

View File

@ -95,7 +95,7 @@ public class PersonaRepository extends EntityRepository<Persona> {
}
@Override
public void entitySpecificUpdate() {
public void entitySpecificUpdate(boolean consolidatingChanges) {
updateUsers(original, updated);
}

View File

@ -469,7 +469,7 @@ public class PipelineRepository extends EntityRepository<Pipeline> {
@Transaction
@Override
public void entitySpecificUpdate() {
public void entitySpecificUpdate(boolean consolidatingChanges) {
updateTasks(original, updated);
recordChange("sourceUrl", original.getSourceUrl(), updated.getSourceUrl());
recordChange("concurrency", original.getConcurrency(), updated.getConcurrency());

View File

@ -165,7 +165,7 @@ public class PolicyRepository extends EntityRepository<Policy> {
@Transaction
@Override
public void entitySpecificUpdate() {
public void entitySpecificUpdate(boolean consolidatingChanges) {
recordChange(ENABLED, original.getEnabled(), updated.getEnabled());
updateRules(original.getRules(), updated.getRules());
}

View File

@ -246,7 +246,7 @@ public class QueryRepository extends EntityRepository<Query> {
@Transaction
@Override
public void entitySpecificUpdate() {
public void entitySpecificUpdate(boolean consolidatingChanges) {
updateFromRelationships(
"users",
USER,

View File

@ -128,7 +128,7 @@ public class RoleRepository extends EntityRepository<Role> {
@Transaction
@Override
public void entitySpecificUpdate() {
public void entitySpecificUpdate(boolean consolidatingChanges) {
updatePolicies(listOrEmpty(original.getPolicies()), listOrEmpty(updated.getPolicies()));
}

View File

@ -376,7 +376,7 @@ public class SearchIndexRepository extends EntityRepository<SearchIndex> {
@Transaction
@Override
public void entitySpecificUpdate() {
public void entitySpecificUpdate(boolean consolidatingChanges) {
if (updated.getFields() != null) {
updateSearchIndexFields(
"fields",

View File

@ -117,7 +117,7 @@ public abstract class ServiceEntityRepository<
@Transaction
@Override
public void entitySpecificUpdate() {
public void entitySpecificUpdate(boolean consolidatingChanges) {
updateConnection();
}

View File

@ -130,7 +130,7 @@ public class StoredProcedureRepository extends EntityRepository<StoredProcedure>
@Transaction
@Override
public void entitySpecificUpdate() {
public void entitySpecificUpdate(boolean consolidatingChanges) {
// storedProcedureCode is a required field. Cannot be null.
if (updated.getStoredProcedureCode() != null) {
recordChange(

View File

@ -1156,7 +1156,7 @@ public class TableRepository extends EntityRepository<Table> {
}
@Override
public void entitySpecificUpdate() {
public void entitySpecificUpdate(boolean consolidatingChanges) {
Table origTable = original;
Table updatedTable = updated;
DatabaseUtil.validateColumns(updatedTable.getColumns());

View File

@ -307,7 +307,7 @@ public class TagRepository extends EntityRepository<Tag> {
@Transaction
@Override
public void entitySpecificUpdate() {
public void entitySpecificUpdate(boolean consolidatingChanges) {
recordChange(
"mutuallyExclusive", original.getMutuallyExclusive(), updated.getMutuallyExclusive());
recordChange("disabled", original.getDisabled(), updated.getDisabled());

View File

@ -876,7 +876,7 @@ public class TeamRepository extends EntityRepository<Team> {
@Transaction
@Override
public void entitySpecificUpdate() {
public void entitySpecificUpdate(boolean consolidatingChanges) {
if (original.getTeamType() != updated.getTeamType()) {
// A team of type 'Group' cannot be updated
if (GROUP.equals(original.getTeamType())) {

View File

@ -678,7 +678,7 @@ public class TestCaseRepository extends EntityRepository<TestCase> {
@Transaction
@Override
public void entitySpecificUpdate() {
public void entitySpecificUpdate(boolean consolidatingChanges) {
EntityLink origEntityLink = EntityLink.parse(original.getEntityLink());
EntityReference origTableRef = EntityUtil.validateEntityLink(origEntityLink);

View File

@ -79,7 +79,7 @@ public class TestConnectionDefinitionRepository extends EntityRepository<TestCon
@Transaction
@Override
public void entitySpecificUpdate() {
public void entitySpecificUpdate(boolean consolidatingChanges) {
recordChange("steps", original.getSteps(), updated.getSteps(), true);
}
}

View File

@ -62,7 +62,7 @@ public class TestDefinitionRepository extends EntityRepository<TestDefinition> {
@Transaction
@Override
public void entitySpecificUpdate() {
public void entitySpecificUpdate(boolean consolidatingChanges) {
recordChange("testPlatforms", original.getTestPlatforms(), updated.getTestPlatforms());
recordChange(
"supportedDataTypes", original.getSupportedDataTypes(), updated.getSupportedDataTypes());

View File

@ -508,7 +508,7 @@ public class TestSuiteRepository extends EntityRepository<TestSuite> {
@Transaction
@Override
public void entitySpecificUpdate() {
public void entitySpecificUpdate(boolean consolidatingChanges) {
List<EntityReference> origTests = listOrEmpty(original.getTests());
List<EntityReference> updatedTests = listOrEmpty(updated.getTests());
List<ResultSummary> origTestCaseResultSummary =

View File

@ -379,7 +379,7 @@ public class TopicRepository extends EntityRepository<Topic> {
@Transaction
@Override
public void entitySpecificUpdate() {
public void entitySpecificUpdate(boolean consolidatingChanges) {
recordChange(
"maximumMessageSize", original.getMaximumMessageSize(), updated.getMaximumMessageSize());
recordChange(

View File

@ -270,7 +270,7 @@ public class TypeRepository extends EntityRepository<Type> {
@Transaction
@Override
public void entitySpecificUpdate() {
public void entitySpecificUpdate(boolean consolidatingChanges) {
updateCustomProperties();
}

View File

@ -625,7 +625,7 @@ public class UserRepository extends EntityRepository<User> {
@Transaction
@Override
public void entitySpecificUpdate() {
public void entitySpecificUpdate(boolean consolidatingChanges) {
// LowerCase Email
updated.setEmail(original.getEmail().toLowerCase());

View File

@ -73,7 +73,7 @@ public class WorkflowDefinitionRepository extends EntityRepository<WorkflowDefin
@Transaction
@Override
public void entitySpecificUpdate() {
public void entitySpecificUpdate(boolean consolidatingChanges) {
updateType();
updateTrigger();
updateNodes();

View File

@ -84,7 +84,7 @@ public class WorkflowRepository extends EntityRepository<Workflow> {
@Transaction
@Override
public void entitySpecificUpdate() {
public void entitySpecificUpdate(boolean consolidatingChanges) {
recordChange("status", original.getStatus(), updated.getStatus());
recordChange("response", original.getResponse(), updated.getResponse(), true);
}

View File

@ -266,20 +266,29 @@ public class GlossaryTermResourceTest extends EntityResourceTest<GlossaryTerm, C
@Test
void patch_addDeleteReviewers(TestInfo test) throws IOException {
CreateGlossaryTerm create =
createRequest(getEntityName(test), "", "", null).withReviewers(null).withSynonyms(null);
createRequest(getEntityName(test), "desc", "", null).withReviewers(null).withSynonyms(null);
GlossaryTerm term = createEntity(create, ADMIN_AUTH_HEADERS);
// Add reviewer USER1, synonym1, reference1 in PATCH request
String origJson = JsonUtils.pojoToJson(term);
TermReference reference1 =
new TermReference().withName("reference1").withEndpoint(URI.create("http://reference1"));
// NOTE: We are patching outside the `patchEntityAndCheck` method in order to be able to wait
// for the Task to be Created.
// The Task is created asynchronously from the Glossary Approval Workflow.
// This allows us to be sure the Status will be updated to IN_REVIEW.
term.withReviewers(List.of(USER1_REF))
.withSynonyms(List.of("synonym1"))
.withReferences(List.of(reference1));
patchEntity(term.getId(), origJson, term, ADMIN_AUTH_HEADERS);
waitForTaskToBeCreated(term.getFullyQualifiedName());
ChangeDescription change = getChangeDescription(term, MINOR_UPDATE);
fieldAdded(change, "reviewers", List.of(USER1_REF));
fieldAdded(change, "synonyms", List.of("synonym1"));
fieldAdded(change, "references", List.of(reference1));
fieldUpdated(change, "status", Status.APPROVED, Status.IN_REVIEW);
term = patchEntityAndCheck(term, origJson, ADMIN_AUTH_HEADERS, MINOR_UPDATE, change);
// Add reviewer USER2, synonym2, reference2 in PATCH request
@ -294,6 +303,7 @@ public class GlossaryTermResourceTest extends EntityResourceTest<GlossaryTerm, C
fieldAdded(change, "reviewers", List.of(USER1_REF, USER2_REF));
fieldAdded(change, "synonyms", List.of("synonym1", "synonym2"));
fieldAdded(change, "references", List.of(reference1, reference2));
fieldUpdated(change, "status", Status.APPROVED, Status.IN_REVIEW);
term = patchEntityAndCheck(term, origJson, ADMIN_AUTH_HEADERS, CHANGE_CONSOLIDATED, change);
// Remove a reviewer USER1, synonym1, reference1 in PATCH request
@ -306,6 +316,7 @@ public class GlossaryTermResourceTest extends EntityResourceTest<GlossaryTerm, C
fieldAdded(change, "reviewers", List.of(USER2_REF));
fieldAdded(change, "synonyms", List.of("synonym2"));
fieldAdded(change, "references", List.of(reference2));
fieldUpdated(change, "status", Status.APPROVED, Status.IN_REVIEW);
patchEntityAndCheck(term, origJson, ADMIN_AUTH_HEADERS, CHANGE_CONSOLIDATED, change);
}

View File

@ -210,11 +210,18 @@ test.describe('Glossary tests', () => {
const { page, afterAction, apiContext } = await performAdminLogin(browser);
const glossary1 = new Glossary();
const glossaryTerm1 = new GlossaryTerm(glossary1);
const glossaryTerm2 = new GlossaryTerm(glossary1);
glossary1.data.terms = [glossaryTerm1, glossaryTerm2];
glossary1.data.terms = [glossaryTerm1];
/* We are creating another Glossary in order to avoid glossaryTerm2 to be moved to IN_REVIEW Status. */
const glossary2 = new Glossary();
const glossaryTerm2 = new GlossaryTerm(glossary2);
glossary2.data.terms = [glossaryTerm2];
const user3 = new UserClass();
const user4 = new UserClass();
await glossary1.create(apiContext);
await glossary2.create(apiContext);
await glossaryTerm1.create(apiContext);
await glossaryTerm2.create(apiContext);
await user3.create(apiContext);
@ -285,6 +292,7 @@ test.describe('Glossary tests', () => {
await glossaryTerm1.delete(apiContext);
await glossaryTerm2.delete(apiContext);
await glossary1.delete(apiContext);
await glossary2.delete(apiContext);
await user3.delete(apiContext);
await user4.delete(apiContext);
await afterAction();