Revert "Fixes #13595 Consolidate changes by a user in a single session to a s… (#13596)" (#13598)

This reverts commit 45ad6bd05deccd279b3345270701881f2b07ede5.
This commit is contained in:
Pere Miquel Brull 2023-10-17 12:58:40 +02:00 committed by GitHub
parent 10ef83779e
commit 75902ac8d8
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 13 additions and 136 deletions

View File

@ -633,6 +633,10 @@ public interface CollectionDAO {
+ "ORDER BY extension")
List<ExtensionRecord> 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<String> 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);

View File

@ -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<T extends EntityInterface> {
}
}
/** 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<TagLabel> tagLabels, String targetFQN) {
for (TagLabel tagLabel : listOrEmpty(tagLabels)) {
if (tagLabel.getSource() == TagSource.CLASSIFICATION) {
@ -1639,8 +1637,8 @@ public abstract class EntityRepository<T extends EntityInterface> {
}
}
/** 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<T extends EntityInterface> {
* @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<T extends EntityInterface> {
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<T extends EntityInterface> {
: 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();

View File

@ -280,7 +280,7 @@ public class TestCaseRepository extends EntityRepository<TestCase> {
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<TestCase> {
.orElse(null);
}
private boolean shouldUpdateResultSummary(ResultSummary storedResultSummary, Long timestamp) {
private boolean shouldUpdateResultSummary(ResultSummary storedResultSummary, Long timestamp, boolean isDeleted) {
return storedResultSummary == null || timestamp >= storedResultSummary.getTimestamp();
}

View File

@ -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;
}

View File

@ -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");

View File

@ -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<T extends EntityInterface, K extends Cr
entityNotFound(Entity.USER, NON_EXISTENT_ENTITY));
}
@Test
void put_entityUpdatesInASession(TestInfo test) throws IOException, InterruptedException {
EntityUpdater.disableConsolidateChanges(false);
try {
K create = createRequest(getEntityName(test), "description", null, null);
T entity = createEntity(create, ADMIN_AUTH_HEADERS);
Double previousVersion = entity.getVersion();
// Update description with a new description and the version changes
create = create.withDescription("description1");
ChangeDescription change = getChangeDescription(previousVersion);
fieldUpdated(change, "description", "description", "description1");
updateAndCheckEntity(create, OK, ADMIN_AUTH_HEADERS, MINOR_UPDATE, change);
// Update description with a new description and the version changes
create = create.withDescription("description2");
change = getChangeDescription(previousVersion); // New version remains the same
fieldUpdated(change, "description", "description", "description2");
updateAndCheckEntity(create, OK, ADMIN_AUTH_HEADERS, MINOR_UPDATE, change);
// Update null displayName with a new displayName - version remains the same.
// ChangeDescription includes all the changes so far.
create = create.withDisplayName("displayName");
change = getChangeDescription(previousVersion); // New version remains the same
fieldUpdated(change, "description", "description", "description2");
fieldAdded(change, "displayName", "displayName");
updateAndCheckEntity(create, OK, ADMIN_AUTH_HEADERS, MINOR_UPDATE, change);
// Delete the displayName - version remains the same.
// ChangeDescription includes all the changes so far.
create = create.withDisplayName(null);
change = getChangeDescription(previousVersion); // New version remains the same
fieldUpdated(change, "description", "description", "description2");
entity = updateAndCheckEntity(create, OK, ADMIN_AUTH_HEADERS, MINOR_UPDATE, change);
// Set the user session timeout to 1 seconds from 10 minutes. Updating display should result in new change
EntityUpdater.setSessionTimeout(1000);
java.lang.Thread.sleep(1001L);
create = create.withDisplayName("displayName");
change = getChangeDescription(entity.getVersion()); // New version remains the same
fieldAdded(change, "displayName", "displayName");
updateAndCheckEntity(create, OK, ADMIN_AUTH_HEADERS, MINOR_UPDATE, change);
} finally {
// Finally, disable consolidate changes for tests to work
EntityUpdater.setSessionTimeout(10 * 60 * 1000);
EntityUpdater.disableConsolidateChanges(true);
}
}
///////////////////////////////////////////////////////////////////////////////////////////////////////////////////
// Common entity tests for PATCH operations
///////////////////////////////////////////////////////////////////////////////////////////////////////////////////
@ -1520,50 +1470,6 @@ public abstract class EntityResourceTest<T extends EntityInterface, K extends Cr
readOnlyAttribute(entityType, FIELD_DELETED));
}
@Test
void patch_entityUpdatesInASession(TestInfo test) throws IOException {
EntityUpdater.disableConsolidateChanges(false);
try {
K create = createRequest(getEntityName(test), "description", null, null);
T entity = createEntity(create, ADMIN_AUTH_HEADERS);
Double previousVersion = entity.getVersion();
// Update description with a new description and the version changes
String json = JsonUtils.pojoToJson(entity);
entity.setDescription("description1");
ChangeDescription change = getChangeDescription(previousVersion);
fieldUpdated(change, "description", "description", "description1");
entity = patchEntityAndCheck(entity, json, ADMIN_AUTH_HEADERS, MINOR_UPDATE, change);
// Update description with a new description and the version changes
json = JsonUtils.pojoToJson(entity);
entity.setDescription("description2");
change = getChangeDescription(previousVersion); // New version remains the same
fieldUpdated(change, "description", "description", "description2");
entity = patchEntityAndCheck(entity, json, ADMIN_AUTH_HEADERS, MINOR_UPDATE, change);
// Update null displayName with a new displayName - version remains the same.
// ChangeDescription includes all the changes so far.
json = JsonUtils.pojoToJson(entity);
entity.setDisplayName("displayName");
change = getChangeDescription(previousVersion); // New version remains the same
fieldUpdated(change, "description", "description", "description2");
fieldAdded(change, "displayName", "displayName");
entity = patchEntityAndCheck(entity, json, ADMIN_AUTH_HEADERS, MINOR_UPDATE, change);
// Delete the displayName - version remains the same.
// ChangeDescription includes all the changes so far.
json = JsonUtils.pojoToJson(entity);
entity.setDisplayName(null);
change = getChangeDescription(previousVersion); // New version remains the same
fieldUpdated(change, "description", "description", "description2");
patchEntityAndCheck(entity, json, ADMIN_AUTH_HEADERS, MINOR_UPDATE, change);
} finally {
// Finally, disable consolidate changes for tests to work
EntityUpdater.disableConsolidateChanges(true);
}
}
@Test
@Execution(ExecutionMode.CONCURRENT)
void put_addEntityCustomAttributes(TestInfo test) throws IOException {

View File

@ -173,11 +173,13 @@ class PolicyEvaluatorTest {
return permissions;
}
public static void updateAccess(List<Permission> permissions, final MetadataOperation operation, Access access) {
public static List<Permission> updateAccess(
List<Permission> permissions, final MetadataOperation operation, Access access) {
permissions.forEach(
permission -> {
if (permission.getOperation().equals(operation)) permission.setAccess(access);
});
return permissions;
}
public static ResourcePermission getResourcePermission(