From 75902ac8d896e3cc282f5baeb90ba8bc4f0cbee4 Mon Sep 17 00:00:00 2001 From: Pere Miquel Brull Date: Tue, 17 Oct 2023 12:58:40 +0200 Subject: [PATCH] =?UTF-8?q?Revert=20"Fixes=20#13595=20Consolidate=20change?= =?UTF-8?q?s=20by=20a=20user=20in=20a=20single=20session=20to=20a=20s?= =?UTF-8?q?=E2=80=A6=20(#13596)"=20(#13598)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit 45ad6bd05deccd279b3345270701881f2b07ede5. --- .../service/jdbi3/CollectionDAO.java | 4 + .../service/jdbi3/EntityRepository.java | 37 +------- .../service/jdbi3/TestCaseRepository.java | 4 +- .../openmetadata/service/util/EntityUtil.java | 4 - .../service/OpenMetadataApplicationTest.java | 2 - .../service/resources/EntityResourceTest.java | 94 ------------------- .../policyevaluator/PolicyEvaluatorTest.java | 4 +- 7 files changed, 13 insertions(+), 136 deletions(-) diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/CollectionDAO.java b/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/CollectionDAO.java index 6d8d33cc7e0..93540494760 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/CollectionDAO.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/CollectionDAO.java @@ -633,6 +633,10 @@ public interface CollectionDAO { + "ORDER BY extension") List getExtensions(@BindUUID("id") UUID id, @Bind("extensionPrefix") String extensionPrefix); + @RegisterRowMapper(ExtensionMapper.class) + @SqlQuery("SELECT json FROM entity_extension WHERE id = :id AND extension = :extension " + "ORDER BY extension") + List getExtensionJsons(@BindUUID("id") UUID id, @Bind("extension") String extension); + @SqlUpdate("DELETE FROM entity_extension WHERE id = :id AND extension = :extension") void delete(@BindUUID("id") UUID id, @Bind("extension") String extension); diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/EntityRepository.java b/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/EntityRepository.java index a67e84b8146..2f5a34e0ab2 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/EntityRepository.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/EntityRepository.java @@ -53,12 +53,10 @@ import static org.openmetadata.service.util.EntityUtil.getExtensionField; import static org.openmetadata.service.util.EntityUtil.nextMajorVersion; import static org.openmetadata.service.util.EntityUtil.nextVersion; import static org.openmetadata.service.util.EntityUtil.objectMatch; -import static org.openmetadata.service.util.EntityUtil.previousVersion; import static org.openmetadata.service.util.EntityUtil.tagLabelMatch; import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.node.ObjectNode; -import com.google.common.annotations.VisibleForTesting; import com.google.common.cache.CacheBuilder; import com.google.common.cache.CacheLoader; import com.google.common.cache.LoadingCache; @@ -1253,8 +1251,8 @@ public abstract class EntityRepository { } } - /** Apply tags {@code tagLabels} to the entity or field identified by {@code targetFQN} */ @Transaction + /** Apply tags {@code tagLabels} to the entity or field identified by {@code targetFQN} */ public void applyTags(List tagLabels, String targetFQN) { for (TagLabel tagLabel : listOrEmpty(tagLabels)) { if (tagLabel.getSource() == TagSource.CLASSIFICATION) { @@ -1639,8 +1637,8 @@ public abstract class EntityRepository { } } - /** Remove owner relationship for a given entity */ @Transaction + /** Remove owner relationship for a given entity */ private void removeOwner(T entity, EntityReference owner) { if (EntityUtil.getId(owner) != null) { LOG.info("Removing owner {}:{} for entity {}", owner.getType(), owner.getFullyQualifiedName(), entity.getId()); @@ -1792,8 +1790,6 @@ public abstract class EntityRepository { * @see TableRepository.TableUpdater#entitySpecificUpdate() for example. */ public class EntityUpdater { - private static volatile long sessionTimeoutMillis = 10 * 60 * 1000; // 10 minutes - private static boolean disableConsolidateChanges = false; protected final T original; protected final T updated; protected final Operation operation; @@ -1804,8 +1800,7 @@ public abstract class EntityRepository { private boolean entityChanged = false; public EntityUpdater(T original, T updated, Operation operation) { - boolean consolidateChanges = consolidateChanges(original, updated); - this.original = consolidateChanges ? getPreviousVersion(original) : original; + this.original = original; this.updated = updated; this.operation = operation; this.updatingUser = @@ -1814,32 +1809,8 @@ public abstract class EntityRepository { : getEntityByName(Entity.USER, updated.getUpdatedBy(), "", NON_DELETED); } - @VisibleForTesting - public static void disableConsolidateChanges(boolean disable) { - disableConsolidateChanges = disable; - } - - @VisibleForTesting - public static void setSessionTimeout(long timeout) { - sessionTimeoutMillis = timeout; - } - - private boolean consolidateChanges(T original, T updated) { - // If user is the same and the new update is with in the user session timeout - return !disableConsolidateChanges - && original.getVersion() > 0.1 - && original.getUpdatedBy().equals(updated.getUpdatedBy()) - && updated.getUpdatedAt() - original.getUpdatedAt() <= sessionTimeoutMillis; - } - - private T getPreviousVersion(T original) { - String extensionName = EntityUtil.getVersionExtension(entityType, previousVersion(original.getVersion())); - String json = daoCollection.entityExtensionDAO().getExtension(original.getId(), extensionName); - return JsonUtils.readValue(json, entityClass); - } - - /** Compare original and updated entities and perform updates. Update the entity version and track changes. */ @Transaction + /** Compare original and updated entities and perform updates. Update the entity version and track changes. */ public final void update() { if (operation.isDelete()) { // DELETE Operation updateDeleted(); diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/TestCaseRepository.java b/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/TestCaseRepository.java index 35e13c865e1..73d2f4ff1ab 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/TestCaseRepository.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/TestCaseRepository.java @@ -280,7 +280,7 @@ public class TestCaseRepository extends EntityRepository { ResultSummary storedResultSummary = findMatchingResultSummary(resultSummaries, resultSummary.getTestCaseName()); - if (!shouldUpdateResultSummary(storedResultSummary, timestamp)) { + if (!shouldUpdateResultSummary(storedResultSummary, timestamp, isDeleted)) { continue; // if the state should not be updated then nothing to do } @@ -317,7 +317,7 @@ public class TestCaseRepository extends EntityRepository { .orElse(null); } - private boolean shouldUpdateResultSummary(ResultSummary storedResultSummary, Long timestamp) { + private boolean shouldUpdateResultSummary(ResultSummary storedResultSummary, Long timestamp, boolean isDeleted) { return storedResultSummary == null || timestamp >= storedResultSummary.getTimestamp(); } diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/util/EntityUtil.java b/openmetadata-service/src/main/java/org/openmetadata/service/util/EntityUtil.java index f11740c172c..e523142db87 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/util/EntityUtil.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/util/EntityUtil.java @@ -381,10 +381,6 @@ public final class EntityUtil { return Math.round((version + 0.1) * 10.0) / 10.0; } - public static Double previousVersion(Double version) { - return Math.round((version - 0.1) * 10.0) / 10.0; - } - public static Double nextMajorVersion(Double version) { return Math.round((version + 1.0) * 10.0) / 10.0; } diff --git a/openmetadata-service/src/test/java/org/openmetadata/service/OpenMetadataApplicationTest.java b/openmetadata-service/src/test/java/org/openmetadata/service/OpenMetadataApplicationTest.java index f79b9cbd906..4a444588107 100644 --- a/openmetadata-service/src/test/java/org/openmetadata/service/OpenMetadataApplicationTest.java +++ b/openmetadata-service/src/test/java/org/openmetadata/service/OpenMetadataApplicationTest.java @@ -40,7 +40,6 @@ import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.TestInstance; import org.openmetadata.common.utils.CommonUtil; import org.openmetadata.service.fernet.Fernet; -import org.openmetadata.service.jdbi3.EntityRepository.EntityUpdater; import org.openmetadata.service.jdbi3.locator.ConnectionAwareAnnotationSqlLocator; import org.openmetadata.service.jdbi3.locator.ConnectionType; import org.openmetadata.service.resources.CollectionRegistry; @@ -78,7 +77,6 @@ public abstract class OpenMetadataApplicationTest { @BeforeAll public static void createApplication() throws Exception { - EntityUpdater.disableConsolidateChanges(true); String jdbcContainerClassName = System.getProperty("jdbcContainerClassName"); String jdbcContainerImage = System.getProperty("jdbcContainerImage"); String elasticSearchContainerImage = System.getProperty("elasticSearchContainerClassName"); diff --git a/openmetadata-service/src/test/java/org/openmetadata/service/resources/EntityResourceTest.java b/openmetadata-service/src/test/java/org/openmetadata/service/resources/EntityResourceTest.java index 7a3d800763a..c383f002352 100644 --- a/openmetadata-service/src/test/java/org/openmetadata/service/resources/EntityResourceTest.java +++ b/openmetadata-service/src/test/java/org/openmetadata/service/resources/EntityResourceTest.java @@ -164,7 +164,6 @@ import org.openmetadata.schema.type.csv.CsvImportResult; import org.openmetadata.service.Entity; import org.openmetadata.service.OpenMetadataApplicationTest; import org.openmetadata.service.exception.CatalogExceptionMessage; -import org.openmetadata.service.jdbi3.EntityRepository.EntityUpdater; import org.openmetadata.service.resources.bots.BotResourceTest; import org.openmetadata.service.resources.databases.TableResourceTest; import org.openmetadata.service.resources.domains.DataProductResourceTest; @@ -1327,55 +1326,6 @@ public abstract class EntityResourceTest permissions, final MetadataOperation operation, Access access) { + public static List updateAccess( + List permissions, final MetadataOperation operation, Access access) { permissions.forEach( permission -> { if (permission.getOperation().equals(operation)) permission.setAccess(access); }); + return permissions; } public static ResourcePermission getResourcePermission(