Skip to content

Commit

Permalink
repo-sqale: added overwrite and first tests, ignoring version in add
Browse files Browse the repository at this point in the history 
Now addObject ignores provided version, both for initial (1 is set)
and for overwrite (updates version + 1).
  • Loading branch information
@virgo47
virgo47 committed May 19, 2021
1 parent a6ffcfb commit f28532f
Show file tree
Hide file tree
Showing 3 changed files with 174 additions and 44 deletions.
89 changes: 75 additions & 14 deletions repo/repo-sqale/src/main/java/com/evolveum/midpoint/repo/sqale/SqaleRepositoryService.java
View file
Expand Up @@ -26,17 +26,19 @@
import com.evolveum.midpoint.prism.PrismPropertyValue;
import com.evolveum.midpoint.prism.delta.ItemDelta;
import com.evolveum.midpoint.prism.delta.ItemDeltaCollectionsUtil;
import com.evolveum.midpoint.prism.delta.ObjectDelta;
import com.evolveum.midpoint.prism.delta.PropertyDelta;
import com.evolveum.midpoint.prism.equivalence.EquivalenceStrategy;
import com.evolveum.midpoint.prism.polystring.PolyString;
import com.evolveum.midpoint.prism.query.ObjectQuery;
import com.evolveum.midpoint.repo.api.*;
import com.evolveum.midpoint.repo.api.perf.OperationRecord;
import com.evolveum.midpoint.repo.api.query.ObjectFilterExpressionEvaluator;
import com.evolveum.midpoint.repo.sqale.operations.AddObjectOperation;
import com.evolveum.midpoint.repo.sqale.qmodel.SqaleTableMapping;
import com.evolveum.midpoint.repo.sqale.qmodel.object.MObject;
import com.evolveum.midpoint.repo.sqale.qmodel.object.MObjectType;
import com.evolveum.midpoint.repo.sqale.qmodel.object.QObject;
import com.evolveum.midpoint.repo.sqale.update.AddObjectContext;
import com.evolveum.midpoint.repo.sqale.update.RootUpdateContext;
import com.evolveum.midpoint.repo.sqlbase.*;
import com.evolveum.midpoint.repo.sqlbase.mapping.QueryTableMapping;
Expand Down Expand Up @@ -260,7 +262,11 @@ public <T extends ObjectType> String addObject(
object.setVersion("1");
}

return executeAddObject(object, options, operationResult);
return object.getOid() == null || !options.isOverwrite()
? executeAddObject(object, options, operationResult)
: executeOverwriteObject(object, options, operationResult);
} catch (RepositoryException | RuntimeException e) {
throw handledGeneralException(e, operationResult);
} catch (Throwable t) {
operationResult.recordFatalError(t);
throw t;
Expand All @@ -275,9 +281,7 @@ private <T extends ObjectType> String executeAddObject(
@NotNull RepoAddOptions options,
@NotNull OperationResult operationResult)
throws SchemaException, ObjectAlreadyExistsException {
long opHandle = registerOperationStart(
options.isOverwrite() ? OP_ADD_OBJECT_OVERWRITE : OP_ADD_OBJECT,
object);
long opHandle = registerOperationStart(OP_ADD_OBJECT, object);
/* old repo code missing in new repo:
int attempt = 1;
int restarts = 0;
Expand All @@ -287,8 +291,8 @@ private <T extends ObjectType> String executeAddObject(
// TODO use executeAttempts

try {
String oid = new AddObjectOperation<>(object, options, operationResult)
.execute(repositoryContext);
String oid = new AddObjectContext<>(repositoryContext, object, options, operationResult)
.execute();
invokeConflictWatchers((w) -> w.afterAddObject(oid, object));
return oid;
/*
Expand All @@ -310,6 +314,52 @@ private <T extends ObjectType> String executeAddObject(
}
}

/** Overwrite is more like update than add. */
private <T extends ObjectType> String executeOverwriteObject(
@NotNull PrismObject<T> newObject,
@NotNull RepoAddOptions options,
@NotNull OperationResult operationResult)
throws SchemaException, RepositoryException, ObjectAlreadyExistsException {

String oid = newObject.getOid();
UUID oidUuid = UUID.fromString(oid);
long opHandle = registerOperationStart(OP_ADD_OBJECT_OVERWRITE, newObject);
// TODO use executeAttempts
try (JdbcSession jdbcSession = repositoryContext.newJdbcSession().startTransaction()) {
try {
//noinspection ConstantConditions
RootUpdateContext<T, QObject<MObject>, MObject> updateContext =
prepareUpdateContext(jdbcSession, newObject.getCompileTimeClass(), oidUuid);
PrismObject<T> prismObject = updateContext.getPrismObject();
// no precondition check for overwrite

// TODO beforeModify? then afterModify lower too...
// invokeConflictWatchers(w -> w.beforeModifyObject(prismObject));
newObject.setUserData(RepositoryService.KEY_ORIGINAL_OBJECT, prismObject.clone());
ObjectDelta<T> delta = prismObject.diff(newObject, EquivalenceStrategy.LITERAL);
Collection<? extends ItemDelta<?, ?>> modifications = delta.getModifications();

LOGGER.trace("overwriteAddObjectAttempt: originalOid={}, modifications={}",
oid, modifications);

updateContext.execute(modifications);

// TODO do we want this conflict watcher or modify one?
// invokeConflictWatchers((w) -> w.afterAddObject(oid, object));
LOGGER.trace("OBJECT after:\n{}", prismObject.debugDumpLazily());
} catch (ObjectNotFoundException e) {
// so it is just plain addObject after all
new AddObjectContext<>(repositoryContext, newObject, options, operationResult)
.execute();
invokeConflictWatchers((w) -> w.afterAddObject(oid, newObject));
}
jdbcSession.commit();
return oid;
} finally {
registerOperationFinish(opHandle, 1); // TODO attempt
}
}

@Override
@NotNull
public <T extends ObjectType> ModifyObjectResult<T> modifyObject(
Expand Down Expand Up @@ -425,6 +475,7 @@ private <T extends ObjectType> ModifyObjectResult<T> executeModifyObject(

LOGGER.trace("OBJECT after:\n{}", prismObject.debugDumpLazily());

invokeConflictWatchers((w) -> w.afterModifyObject(prismObject.getOid()));
return new ModifyObjectResult<>(originalObject, prismObject, modifications);
} finally {
registerOperationFinish(opHandle, 1); // TODO attempt
Expand Down Expand Up @@ -500,13 +551,7 @@ private void logTraceModifications(@NotNull Collection<? extends ItemDelta<?, ?>
.addParam("oid", oid)
.build();
try {
try (JdbcSession jdbcSession = repositoryContext.newJdbcSession().startTransaction()) {
DeleteObjectResult result = deleteObjectAttempt(type, oidUuid, jdbcSession);
invokeConflictWatchers((w) -> w.afterDeleteObject(oid));

jdbcSession.commit();
return result;
}
return executeDeleteObject(type, oid, oidUuid);
} catch (RuntimeException e) {
throw handledGeneralException(e, operationResult);
} catch (Throwable t) {
Expand All @@ -517,6 +562,22 @@ private void logTraceModifications(@NotNull Collection<? extends ItemDelta<?, ?>
}
}

@NotNull
private <T extends ObjectType> DeleteObjectResult executeDeleteObject(
Class<T> type, String oid, UUID oidUuid) throws ObjectNotFoundException {

long opHandle = registerOperationStart(OP_DELETE_OBJECT, type);
try (JdbcSession jdbcSession = repositoryContext.newJdbcSession().startTransaction()) {
DeleteObjectResult result = deleteObjectAttempt(type, oidUuid, jdbcSession);
invokeConflictWatchers((w) -> w.afterDeleteObject(oid));

jdbcSession.commit();
return result;
} finally {
registerOperationFinish(opHandle, 1); // TODO attempt
}
}

private <T extends ObjectType, Q extends QObject<R>, R extends MObject>
DeleteObjectResult deleteObjectAttempt(Class<T> type, UUID oid, JdbcSession jdbcSession)
throws ObjectNotFoundException {
Expand Down
44 changes: 17 additions & 27 deletions .../sqale/operations/AddObjectOperation.java → ...t/repo/sqale/update/AddObjectContext.java
View file
Expand Up @@ -4,7 +4,7 @@
* This work is dual-licensed under the Apache License 2.0
* and European Union Public License. See LICENSE file for details.
*/
package com.evolveum.midpoint.repo.sqale.operations;
package com.evolveum.midpoint.repo.sqale.update;

import java.util.Objects;
import java.util.UUID;
Expand All @@ -24,35 +24,31 @@
import com.evolveum.midpoint.repo.sqale.qmodel.object.QObject;
import com.evolveum.midpoint.repo.sqale.qmodel.object.QObjectMapping;
import com.evolveum.midpoint.repo.sqlbase.JdbcSession;
import com.evolveum.midpoint.repo.sqlbase.SqlRepoContext;
import com.evolveum.midpoint.schema.result.OperationResult;
import com.evolveum.midpoint.util.exception.ObjectAlreadyExistsException;
import com.evolveum.midpoint.util.exception.SchemaException;
import com.evolveum.midpoint.xml.ns._public.common.common_3.ObjectType;

/*
TODO: implementation note:
Typically I'd use "technical" dependencies in a constructor and then the object/options/result
would be parameters of execute(). Unfortunately I don't know how to do that AND capture
the parametric types in the operations object. I could hide it behind this object and then capture
it in another "actual operation" object, but that does not make any sense.
That's why the creation is with actual parameters and execute() takes technical ones (possibly
some richer "context" object later to provide more dependencies if necessary).
The "context" could go to construction too, but than it would be all mixed too much. Sorry.
*/
public class AddObjectOperation<S extends ObjectType, Q extends QObject<R>, R extends MObject> {
/**
* Add object operation context; used only for true add, not overwrite which is more like modify.
*/
public class AddObjectContext<S extends ObjectType, Q extends QObject<R>, R extends MObject> {

private final SqaleRepoContext repositoryContext;
private final PrismObject<S> object;
private final RepoAddOptions options;
private final OperationResult result;

private SqlRepoContext repositoryContext;
private Q root;
private QObjectMapping<S, Q, R> rootMapping;
private MObjectType objectType;

public AddObjectOperation(@NotNull PrismObject<S> object,
@NotNull RepoAddOptions options, @NotNull OperationResult result) {
public AddObjectContext(
@NotNull SqaleRepoContext repositoryContext,
@NotNull PrismObject<S> object,
@NotNull RepoAddOptions options,
@NotNull OperationResult result) {
this.repositoryContext = repositoryContext;
this.object = object;
this.options = options;
this.result = result;
Expand All @@ -61,24 +57,20 @@ public AddObjectOperation(@NotNull PrismObject<S> object,
/**
* Inserts the object provided to the constructor and returns its OID.
*/
public String execute(SqaleRepoContext repositoryContext)
public String execute()
throws SchemaException, ObjectAlreadyExistsException {
try {
// TODO utilize options and result
this.repositoryContext = repositoryContext;
object.setVersion("1"); // initial add always uses 1 as version number
Class<S> schemaObjectClass = object.getCompileTimeClass();
objectType = MObjectType.fromSchemaType(schemaObjectClass);
rootMapping = repositoryContext.getMappingBySchemaType(schemaObjectClass);
root = rootMapping.defaultAlias();

// we don't want CID generation here, because overwrite works different then normal add

if (object.getOid() == null) {
return addObjectWithoutOid();
} else if (options.isOverwrite()) {
return overwriteObject();
} else {
// OID is not null, but it's not overwrite either
// this also handles overwrite after ObjectNotFoundException
return addObjectWithOid();
}
} catch (QueryException e) {
Expand All @@ -90,10 +82,6 @@ public String execute(SqaleRepoContext repositoryContext)
}
}

private String overwriteObject() {
throw new UnsupportedOperationException(); // TODO
}

private String addObjectWithOid() throws SchemaException {
long lastCid = new ContainerValueIdGenerator().generateForNewObject(object);
try (JdbcSession jdbcSession = repositoryContext.newJdbcSession().startTransaction()) {
Expand Down Expand Up @@ -150,6 +138,8 @@ private String addObjectWithoutOid() throws SchemaException {
}
}

// TODO can be static. Should it move to other SQL exception handling code?

/** Throws more specific exception or returns and then original exception should be rethrown. */
private void handlePostgresException(PSQLException psqlException)
throws ObjectAlreadyExistsException {
Expand Down
85 changes: 82 additions & 3 deletions ...ale/src/test/java/com/evolveum/midpoint/repo/sqale/func/SqaleRepoAddDeleteObjectTest.java
View file
Expand Up @@ -77,7 +77,8 @@ public void test100AddNamedUserWithoutOidWorksOk()
given("user with a name");
String userName = "user" + getTestNumber();
UserType userType = new UserType(prismContext)
.name(userName);
.name(userName)
.version("5"); // version will be ignored and set to 1

when("adding it to the repository");
String returnedOid = repositoryService.addObject(userType.asPrismObject(), null, result);
Expand All @@ -90,7 +91,7 @@ public void test100AddNamedUserWithoutOidWorksOk()
MUser row = selectOne(u, u.nameOrig.eq(userName));
assertThat(row.oid).isEqualTo(UUID.fromString(returnedOid));
assertThat(row.nameNorm).isNotNull(); // normalized name is stored
assertThat(row.version).isEqualTo(1); // initial version is set
assertThat(row.version).isEqualTo(1); // initial version is set, ignoring provided version
// read-only column with value generated/stored in the database
assertThat(row.objectType).isEqualTo(MObjectType.USER);
assertThat(row.subtypes).isNull(); // we don't store empty lists as empty arrays
Expand Down Expand Up @@ -139,6 +140,65 @@ public void test102AddWithoutOidIgnoresOverwriteOption()
assertThat(users.get(0).oid).isNotNull();
}

@Test
public void test105AddWithOverwriteOption()
throws ObjectAlreadyExistsException, SchemaException {
OperationResult result = createOperationResult();

given("user already in the repository");
long baseCount = count(QUser.class);
String userName = "user" + getTestNumber();
UserType userType = new UserType(prismContext)
.name(userName);
repositoryService.addObject(userType.asPrismObject(), null, result);
assertThat(count(QUser.class)).isEqualTo(baseCount + 1);

when("adding it to the repository again with overwrite option");
userType.setFullName(PolyStringType.fromOrig("Overwritten User"));
userType.setVersion("5"); // should be ignored
repositoryService.addObject(userType.asPrismObject(), createOverwrite(), result);

then("operation is successful");
assertThatOperationResult(result).isSuccess();

and("existing user row is modified/overwritten");
assertThat(count(QUser.class)).isEqualTo(baseCount + 1); // no change in count
MUser row = selectObjectByOid(QUser.class, userType.getOid());
assertThat(row.fullNameOrig).isEqualTo("Overwritten User");

and("provided version for overwrite is ignored");
assertThat(row.version).isEqualTo(2);
}

@Test
public void test106AddWithOverwriteOptionWithNewOidActsLikeNormalAdd()
throws ObjectAlreadyExistsException, SchemaException {
OperationResult result = createOperationResult();

given("user with random OID is not in the repository");
long baseCount = count(QUser.class);
UUID oid = UUID.randomUUID();
assertThat(selectNullableObjectByOid(QUser.class, oid)).isNull();

when("adding it to the repository again with overwrite option");
String userName = "user" + getTestNumber();
UserType userType = new UserType(prismContext)
.oid(oid.toString())
.name(userName)
.version("5");
repositoryService.addObject(userType.asPrismObject(), createOverwrite(), result);

then("operation is successful");
assertThatOperationResult(result).isSuccess();

and("existing user row is modified/overwritten");
assertThat(count(QUser.class)).isEqualTo(baseCount + 1); // no change in count
MUser row = selectObjectByOid(QUser.class, userType.getOid());

and("provided version for overwrite is ignored");
assertThat(row.version).isEqualTo(1);
}

@Test
public void test110AddUserWithProvidedOidWorksOk()
throws ObjectAlreadyExistsException, SchemaException {
Expand Down Expand Up @@ -1258,7 +1318,26 @@ public void test911DeleteUserUsingFocusWorksOk() throws Exception {
}

@Test
public void test920DeleteAllOtherObjects() throws Exception {
public void test920DeleteOperationUpdatesPerformanceMonitor()
throws ObjectNotFoundException {
OperationResult result = createOperationResult();

given("object to delete and cleared performance information");
UUID userOid = randomExistingOid(QUser.class);
SqlPerformanceMonitorImpl pm = repositoryService.getPerformanceMonitor();
pm.clearGlobalPerformanceInformation();
assertThat(pm.getGlobalPerformanceInformation().getAllData()).isEmpty();

when("object is deleted from the repository");
repositoryService.deleteObject(FocusType.class, userOid.toString(), result);

then("performance monitor is updated");
assertThatOperationResult(result).isSuccess();
assertSingleOperationRecorded(pm, RepositoryService.OP_DELETE_OBJECT);
}

@Test
public void test999DeleteAllOtherObjects() throws Exception {
// this doesn't follow given-when-then, sorry
OperationResult result = createOperationResult();

Expand Down

0 comments on commit f28532f

Please sign in to comment.