Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make restoring patch attributes consistent across entities #13961

Merged
merged 3 commits into from
Nov 14, 2023
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 @@ -75,11 +75,8 @@ public Chart clearFields(Chart chart, Fields fields) {
@Override
public void restorePatchAttributes(Chart original, Chart updated) {
// Patch can't make changes to following fields. Ignore the changes
updated
.withFullyQualifiedName(original.getFullyQualifiedName())
.withName(original.getName())
.withService(original.getService())
.withId(original.getId());
super.restorePatchAttributes(original, updated);
updated.withService(original.getService());
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ public ClassificationRepository() {
"");
quoteFqn = true;
supportsSearch = true;
renameAllowed = true;
}

@Override
Expand Down Expand Up @@ -110,7 +111,6 @@ public ClassificationUpdater(Classification original, Classification updated, Op
@Transaction
@Override
public void entitySpecificUpdate() {
// TODO handle name change
// TODO mutuallyExclusive from false to true?
recordChange("mutuallyExclusive", original.getMutuallyExclusive(), updated.getMutuallyExclusive());
recordChange("disabled", original.getDisabled(), updated.getDisabled());
Expand All @@ -125,6 +125,7 @@ public void updateName(Classification original, Classification updated) {
}
// Classification name changed - update tag names starting from classification and all the children tags
LOG.info("Classification name changed from {} to {}", original.getName(), updated.getName());
setFullyQualifiedName(updated);
daoCollection.tagDAO().updateFqn(original.getName(), updated.getName());
daoCollection
.tagUsageDAO()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -146,12 +146,8 @@ public void storeEntity(Container container, boolean update) {
@Override
public void restorePatchAttributes(Container original, Container updated) {
// Patch can't make changes to following fields. Ignore the changes
updated
.withFullyQualifiedName(original.getFullyQualifiedName())
.withService(original.getService())
.withParent(original.getParent())
.withName(original.getName())
.withId(original.getId());
super.restorePatchAttributes(original, updated);
updated.withService(original.getService()).withParent(original.getParent());
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -174,11 +174,8 @@ public DashboardDataModel clearFields(DashboardDataModel dashboardDataModel, Fie
@Override
public void restorePatchAttributes(DashboardDataModel original, DashboardDataModel updated) {
// Patch can't make changes to following fields. Ignore the changes
updated
.withFullyQualifiedName(original.getFullyQualifiedName())
.withName(original.getName())
.withService(original.getService())
.withId(original.getId());
super.restorePatchAttributes(original, updated);
updated.withService(original.getService());
}

private void applyTags(List<Column> columns) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -118,11 +118,8 @@ public Dashboard clearFields(Dashboard dashboard, Fields fields) {
@Override
public void restorePatchAttributes(Dashboard original, Dashboard updated) {
// Patch can't make changes to following fields. Ignore the changes
updated
.withId(original.getId())
.withFullyQualifiedName(original.getFullyQualifiedName())
.withName(original.getName())
.withService(original.getService());
super.restorePatchAttributes(original, updated);
updated.withService(original.getService());
}

private void populateService(Dashboard dashboard) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ public EntityUpdater getUpdater(DataProduct original, DataProduct updated, Opera

@Override
public void restorePatchAttributes(DataProduct original, DataProduct updated) {
super.restorePatchAttributes(original, updated);
updated.withDomain(original.getDomain()); // Domain can't be changed
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -112,11 +112,8 @@ public Database clearFields(Database database, Fields fields) {
@Override
public void restorePatchAttributes(Database original, Database updated) {
// Patch can't make changes to following fields. Ignore the changes
updated
.withFullyQualifiedName(original.getFullyQualifiedName())
.withName(original.getName())
.withService(original.getService())
.withId(original.getId());
super.restorePatchAttributes(original, updated);
updated.withService(original.getService());
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -124,11 +124,8 @@ public DatabaseSchema setInheritedFields(DatabaseSchema schema, Fields fields) {
@Override
public void restorePatchAttributes(DatabaseSchema original, DatabaseSchema updated) {
// Patch can't make changes to following fields. Ignore the changes
updated
.withFullyQualifiedName(original.getFullyQualifiedName())
.withName(original.getName())
.withService(original.getService())
.withId(original.getId());
super.restorePatchAttributes(original, updated);
updated.withService(original.getService());
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,12 +43,6 @@ public void setFullyQualifiedName(Document doc) {
doc.setFullyQualifiedName(doc.getFullyQualifiedName());
}

@Override
public void restorePatchAttributes(Document original, Document updated) {
// Patch can't make changes to following fields. Ignore the changes
updated.withName(original.getName()).withId(original.getId());
}

@Override
public Document setFields(Document document, Fields fields) {
return document;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ public EntityUpdater getUpdater(Domain original, Domain updated, Operation opera

@Override
public void restorePatchAttributes(Domain original, Domain updated) {
super.restorePatchAttributes(original, updated);
updated.withParent(original.getParent()); // Parent can't be changed
updated.withChildren(original.getChildren()); // Children can't be changed
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,7 @@ public abstract class EntityRepository<T extends EntityInterface> {
@Getter protected final boolean supportsReviewers;
@Getter protected final boolean supportsExperts;
protected boolean quoteFqn = false; // Entity FQNS not hierarchical such user, teams, services need to be quoted
protected boolean renameAllowed = false; // Entity can be renamed

/** Fields that can be updated during PATCH operation */
@Getter private final Fields patchFields;
Expand Down Expand Up @@ -370,7 +371,10 @@ public T setInheritedFields(T entity, Fields fields) {
* stored in the original entity.
*/
public void restorePatchAttributes(T original, T updated) {
/* Nothing to restore during PATCH */
updated.setId(original.getId());
updated.setName(renameAllowed ? updated.getName() : original.getName());
updated.setFullyQualifiedName(original.getFullyQualifiedName());
updated.setChangeDescription(original.getChangeDescription());
}

/** Set fullyQualifiedName of an entity */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,11 +93,6 @@ public void storeRelationships(EventSubscription entity) {
// No relationships to store beyond what is stored in the super class
}

@Override
public void restorePatchAttributes(EventSubscription original, EventSubscription updated) {
updated.withId(original.getId()).withName(original.getName());
}

private SubscriptionPublisher getPublisher(UUID id) {
return subscriptionPublisherMap.get(id);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,7 @@ private String reviewerReferencesToRecord(List<EntityReference> reviewers) {
public class GlossaryUpdater extends EntityUpdater {
public GlossaryUpdater(Glossary original, Glossary updated, Operation operation) {
super(original, updated, operation);
renameAllowed = true;
}

@Transaction
Expand All @@ -285,6 +286,7 @@ public void updateName(Glossary original, Glossary updated) {
}
// Glossary name changed - update tag names starting from glossary and all the children tags
LOG.info("Glossary name changed from {} to {}", original.getName(), updated.getName());
setFullyQualifiedName(updated);
daoCollection.glossaryTermDAO().updateFqn(original.getName(), updated.getName());
daoCollection
.tagUsageDAO()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ public GlossaryTermRepository() {
PATCH_FIELDS,
UPDATE_FIELDS);
supportsSearch = true;
renameAllowed = true;
}

@Override
Expand Down Expand Up @@ -177,6 +178,7 @@ public void storeRelationships(GlossaryTerm entity) {
@Override
public void restorePatchAttributes(GlossaryTerm original, GlossaryTerm updated) {
// Patch can't update Children
super.restorePatchAttributes(original, updated);
updated.withChildren(original.getChildren());
}

Expand Down Expand Up @@ -435,6 +437,7 @@ public void updateName(GlossaryTerm original, GlossaryTerm updated) {
CatalogExceptionMessage.systemEntityRenameNotAllowed(original.getName(), entityType));
}
// Glossary term name changed - update the FQNs of the children terms to reflect this
setFullyQualifiedName(updated);
LOG.info("Glossary term name changed from {} to {}", original.getName(), updated.getName());
daoCollection.glossaryTermDAO().updateFqn(original.getFullyQualifiedName(), updated.getFullyQualifiedName());
daoCollection
Expand All @@ -455,6 +458,7 @@ private void updateParent(GlossaryTerm original, GlossaryTerm updated) {
UUID newGlossaryId = getId(updated.getGlossary());
boolean glossaryChanged = !Objects.equals(oldGlossaryId, newGlossaryId);

setFullyQualifiedName(updated); // Update the FQN since the parent has changed
daoCollection.glossaryTermDAO().updateFqn(original.getFullyQualifiedName(), updated.getFullyQualifiedName());
daoCollection
.tagUsageDAO()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,11 +103,8 @@ public MlModel clearFields(MlModel mlModel, Fields fields) {
@Override
public void restorePatchAttributes(MlModel original, MlModel updated) {
// Patch can't make changes to following fields. Ignore the changes
updated
.withFullyQualifiedName(original.getFullyQualifiedName())
.withService(original.getService())
.withName(original.getName())
.withId(original.getId());
super.restorePatchAttributes(original, updated);
updated.withService(original.getService());
}

private void setMlFeatureSourcesFQN(List<MlFeatureSource> mlSources) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,12 +56,6 @@ public Persona clearFields(Persona persona, Fields fields) {
return persona;
}

@Override
public void restorePatchAttributes(Persona original, Persona updated) {
// Patch can't make changes to following fields. Ignore the changes
updated.withName(original.getName()).withId(original.getId());
}

@Override
public void prepare(Persona persona, boolean update) {
validateUsers(persona.getUsers());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -210,11 +210,8 @@ private void validateTask(Pipeline pipeline, String taskName) {
@Override
public void restorePatchAttributes(Pipeline original, Pipeline updated) {
// Patch can't make changes to following fields. Ignore the changes
updated
.withFullyQualifiedName(original.getFullyQualifiedName())
.withName(original.getName())
.withService(original.getService())
.withId(original.getId());
super.restorePatchAttributes(original, updated);
updated.withService(original.getService());
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,12 +65,6 @@ private List<EntityReference> getTeams(@NonNull Role role) {
return findFrom(role.getId(), Entity.ROLE, Relationship.HAS, Entity.TEAM);
}

@Override
public void restorePatchAttributes(Role original, Role updated) {
// Patch can't make changes to following fields. Ignore the changes
updated.withName(original.getName()).withId(original.getId());
}

/**
* If policy does not exist for this role, create a new entity reference. The actual policy gets created within the
* storeEntity method call.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -170,12 +170,8 @@ private void setDefaultFields(Table table) {
@Override
public void restorePatchAttributes(Table original, Table updated) {
// Patch can't make changes to following fields. Ignore the changes.
updated
.withFullyQualifiedName(original.getFullyQualifiedName())
.withName(original.getName())
.withDatabase(original.getDatabase())
.withService(original.getService())
.withId(original.getId());
super.restorePatchAttributes(original, updated);
updated.withDatabase(original.getDatabase()).withService(original.getService());
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ public class TagRepository extends EntityRepository<Tag> {
public TagRepository() {
super(TagResource.TAG_COLLECTION_PATH, Entity.TAG, Tag.class, Entity.getCollectionDAO().tagDAO(), "", "");
supportsSearch = true;
renameAllowed = true;
}

@Override
Expand All @@ -67,6 +68,7 @@ public void storeEntity(Tag tag, boolean update) {

@Override
public void restorePatchAttributes(Tag original, Tag updated) {
super.restorePatchAttributes(original, updated);
updated.setChildren(original.getChildren());
}

Expand Down Expand Up @@ -153,6 +155,7 @@ public void updateName(Tag original, Tag updated) {
}
// Category name changed - update tag names starting from classification and all the children tags
LOG.info("Tag name changed from {} to {}", original.getName(), updated.getName());
setFullyQualifiedName(updated);
daoCollection.tagDAO().updateFqn(original.getFullyQualifiedName(), updated.getFullyQualifiedName());
daoCollection
.tagUsageDAO()
Expand All @@ -176,6 +179,7 @@ private void updateParent(Tag original, Tag updated) {
UUID newCategoryId = getId(updated.getClassification());
boolean classificationChanged = !Objects.equals(oldCategoryId, newCategoryId);

setFullyQualifiedName(updated);
daoCollection.tagDAO().updateFqn(original.getFullyQualifiedName(), updated.getFullyQualifiedName());
daoCollection
.tagUsageDAO()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ public Team clearFields(Team team, Fields fields) {
@Override
public void restorePatchAttributes(Team original, Team updated) {
// Patch can't make changes to following fields. Ignore the changes
updated.withName(original.getName()).withId(original.getId());
super.restorePatchAttributes(original, updated);
updated.withInheritedRoles(original.getInheritedRoles());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -136,9 +136,8 @@ public void prepare(User user, boolean update) {
@Override
public void restorePatchAttributes(User original, User updated) {
// Patch can't make changes to following fields. Ignore the changes
super.restorePatchAttributes(original, updated);
updated
.withId(original.getId())
.withName(original.getName())
.withInheritedRoles(original.getInheritedRoles())
.withAuthenticationMechanism(original.getAuthenticationMechanism());
}
Expand Down
Loading