From ac7e35f04db01e43cecedc77d2adf74a4b449c32 Mon Sep 17 00:00:00 2001 From: Pavol Mederly Date: Thu, 6 Jun 2019 07:43:15 +0200 Subject: [PATCH 1/6] Stop leaving parent case in open state This was a race condition, visible in tests. However, a more subtle race condition is still possible, see MID-5407. --- .../midpoint/wf/impl/execution/ExecutionHelper.java | 6 +++--- .../impl/processors/primary/PrimaryChangeProcessor.java | 8 ++++++-- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/model/workflow-impl/src/main/java/com/evolveum/midpoint/wf/impl/execution/ExecutionHelper.java b/model/workflow-impl/src/main/java/com/evolveum/midpoint/wf/impl/execution/ExecutionHelper.java index daeaac10d9b..9fdf67d57d5 100644 --- a/model/workflow-impl/src/main/java/com/evolveum/midpoint/wf/impl/execution/ExecutionHelper.java +++ b/model/workflow-impl/src/main/java/com/evolveum/midpoint/wf/impl/execution/ExecutionHelper.java @@ -102,8 +102,8 @@ public void setCaseStateInRepository(CaseType aCase, String newState, OperationR */ public void checkDependentCases(String rootOid, OperationResult result) throws SchemaException, ObjectNotFoundException, ObjectAlreadyExistsException, PreconditionViolationException { - CaseType aCase = repositoryService.getObject(CaseType.class, rootOid, null, result).asObjectable(); - if (CaseTypeUtil.isClosed(aCase)) { + CaseType rootCase = repositoryService.getObject(CaseType.class, rootOid, null, result).asObjectable(); + if (CaseTypeUtil.isClosed(rootCase)) { return; } List subcases = miscHelper.getSubcases(rootOid, result); @@ -117,7 +117,7 @@ public void checkDependentCases(String rootOid, OperationResult result) .collect(Collectors.toList()); LOGGER.debug("open cases OIDs: {}", openOids); if (openOids.isEmpty()) { - closeCaseInRepository(aCase, result); + closeCaseInRepository(rootCase, result); } else { ObjectQuery query = prismContext.queryFor(TaskType.class) .item(TaskType.F_OBJECT_REF).ref(openOids.toArray(new String[0])) diff --git a/model/workflow-impl/src/main/java/com/evolveum/midpoint/wf/impl/processors/primary/PrimaryChangeProcessor.java b/model/workflow-impl/src/main/java/com/evolveum/midpoint/wf/impl/processors/primary/PrimaryChangeProcessor.java index b544e258705..892e3fcf44d 100644 --- a/model/workflow-impl/src/main/java/com/evolveum/midpoint/wf/impl/processors/primary/PrimaryChangeProcessor.java +++ b/model/workflow-impl/src/main/java/com/evolveum/midpoint/wf/impl/processors/primary/PrimaryChangeProcessor.java @@ -456,6 +456,12 @@ public void onProcessEnd(EngineInvocationContext ctx, OperationResult result) private void submitExecutionTask(CaseType aCase, boolean waiting, OperationResult result) throws SchemaException, ObjectNotFoundException, ObjectAlreadyExistsException { + + // We must do this before the task is started, because as part of task completion we set state to CLOSED. + // So if we set state to EXECUTING after the task is started, the case might be already closed at that point. + // (If task is fast enough.) + executionHelper.setCaseStateInRepository(aCase, SchemaConstants.CASE_STATE_EXECUTING, result); + Task task = taskManager.createTaskInstance("execute"); task.setName("Execution of " + aCase.getName().getOrig()); task.setOwner(getExecutionTaskOwner(result)); @@ -465,8 +471,6 @@ private void submitExecutionTask(CaseType aCase, boolean waiting, OperationResul task.setInitialExecutionStatus(TaskExecutionStatus.WAITING); } taskManager.switchToBackground(task, result); - - executionHelper.setCaseStateInRepository(aCase, SchemaConstants.CASE_STATE_EXECUTING, result); } private PrismObject getExecutionTaskOwner(OperationResult result) throws SchemaException, ObjectNotFoundException { From d792a9b826329e14e471eeb59b603ea388b8273c Mon Sep 17 00:00:00 2001 From: skublik Date: Thu, 6 Jun 2019 09:45:35 +0200 Subject: [PATCH 2/6] fix for center composited icon css style --- .../impl/component/icon/CenterForColumnIconCssStyle.java | 4 ++-- .../gui/impl/component/icon/CenterIconCssStyle.java | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/gui/admin-gui/src/main/java/com/evolveum/midpoint/gui/impl/component/icon/CenterForColumnIconCssStyle.java b/gui/admin-gui/src/main/java/com/evolveum/midpoint/gui/impl/component/icon/CenterForColumnIconCssStyle.java index fdcc6016f07..9b06a9bbeb0 100644 --- a/gui/admin-gui/src/main/java/com/evolveum/midpoint/gui/impl/component/icon/CenterForColumnIconCssStyle.java +++ b/gui/admin-gui/src/main/java/com/evolveum/midpoint/gui/impl/component/icon/CenterForColumnIconCssStyle.java @@ -23,12 +23,12 @@ public class CenterForColumnIconCssStyle implements LayeredIconCssStyle { @Override public String getBasicCssClass() { - return "icon-basic-transparent"; + return "icon-basic-transparent-for-column"; } @Override public String getBasicLayerCssClass() { - return "icon-basic-layer"; + return "icon-basic-layer-for-column"; } @Override diff --git a/gui/admin-gui/src/main/java/com/evolveum/midpoint/gui/impl/component/icon/CenterIconCssStyle.java b/gui/admin-gui/src/main/java/com/evolveum/midpoint/gui/impl/component/icon/CenterIconCssStyle.java index 6daa4e075c5..b134f8a1b1f 100644 --- a/gui/admin-gui/src/main/java/com/evolveum/midpoint/gui/impl/component/icon/CenterIconCssStyle.java +++ b/gui/admin-gui/src/main/java/com/evolveum/midpoint/gui/impl/component/icon/CenterIconCssStyle.java @@ -23,17 +23,17 @@ public class CenterIconCssStyle implements LayeredIconCssStyle { @Override public String getBasicCssClass() { - return "icon-basic-transparent-for-column"; + return "icon-basic-transparent"; } @Override public String getBasicLayerCssClass() { - return "icon-basic-layer-for-column"; + return "icon-basic-layer"; } @Override public String getLayerCssClass() { - return "center-layer-for-column"; + return "center-layer"; } @Override From 67d84e323561d6888fa1c1d072ebe0913ff2d0f0 Mon Sep 17 00:00:00 2001 From: Pavol Mederly Date: Thu, 6 Jun 2019 11:06:38 +0200 Subject: [PATCH 3/6] Add test for H2 data corruption (MID-5409) --- .../midpoint/repo/sql/ConcurrencyTest.java | 182 ++++++++++++------ .../repo/sql/SqlRepositoryServiceImpl.java | 2 +- .../repo/sql/helpers/ObjectUpdater.java | 8 +- 3 files changed, 126 insertions(+), 66 deletions(-) diff --git a/repo/repo-sql-impl-test/src/test/java/com/evolveum/midpoint/repo/sql/ConcurrencyTest.java b/repo/repo-sql-impl-test/src/test/java/com/evolveum/midpoint/repo/sql/ConcurrencyTest.java index 65f40829703..178e9a03150 100644 --- a/repo/repo-sql-impl-test/src/test/java/com/evolveum/midpoint/repo/sql/ConcurrencyTest.java +++ b/repo/repo-sql-impl-test/src/test/java/com/evolveum/midpoint/repo/sql/ConcurrencyTest.java @@ -23,7 +23,9 @@ import com.evolveum.midpoint.prism.util.PrismTestUtil; import com.evolveum.midpoint.prism.xml.XmlTypeConverter; import com.evolveum.midpoint.repo.sql.testing.SqlRepoTestUtil; +import com.evolveum.midpoint.schema.constants.ObjectTypes; import com.evolveum.midpoint.schema.result.OperationResult; +import com.evolveum.midpoint.schema.util.ObjectTypeUtil; import com.evolveum.midpoint.util.logging.LoggingUtils; import com.evolveum.midpoint.util.logging.Trace; import com.evolveum.midpoint.util.logging.TraceManager; @@ -33,7 +35,6 @@ import org.hibernate.Session; import org.springframework.test.annotation.DirtiesContext; import org.springframework.test.context.ContextConfiguration; -import org.testng.AssertJUnit; import org.testng.annotations.Test; import javax.xml.namespace.QName; @@ -42,6 +43,10 @@ import java.util.Collection; import java.util.Date; import java.util.List; +import java.util.concurrent.atomic.AtomicInteger; +import java.util.stream.Collectors; + +import static org.testng.AssertJUnit.assertEquals; /** * @author Pavol Mederly @@ -58,8 +63,7 @@ public class ConcurrencyTest extends BaseSQLRepoTest { @Test public void test001TwoWriters_OneAttributeEach__NoReader() throws Exception { - - PropertyModifierThread[] mts = new PropertyModifierThread[]{ + PropertyModifierThread[] mts = new PropertyModifierThread[] { new PropertyModifierThread(1, UserType.F_GIVEN_NAME, true, null, true), new PropertyModifierThread(2, UserType.F_FAMILY_NAME, true, null, true), // new ModifierThread(3, oid, UserType.F_DESCRIPTION, false), @@ -69,33 +73,27 @@ public void test001TwoWriters_OneAttributeEach__NoReader() throws Exception { // new ModifierThread(8, oid, UserType.F_EMAIL_ADDRESS), // new ModifierThread(9, oid, UserType.F_EMPLOYEE_NUMBER) }; - concurrencyUniversal("Test1", 30000L, 500L, mts, null); } @Test public void test002FourWriters_OneAttributeEach__NoReader() throws Exception { - - PropertyModifierThread[] mts = new PropertyModifierThread[]{ + PropertyModifierThread[] mts = new PropertyModifierThread[] { new PropertyModifierThread(1, UserType.F_GIVEN_NAME, true, null, true), new PropertyModifierThread(2, UserType.F_FAMILY_NAME, true, null, true), new PropertyModifierThread(3, UserType.F_DESCRIPTION, false, null, true), new PropertyModifierThread(4, UserType.F_EMAIL_ADDRESS, false, null, true) }; - concurrencyUniversal("Test2", 60000L, 500L, mts, null); } @Test public void test003OneWriter_TwoAttributes__OneReader() throws Exception { - - PropertyModifierThread[] mts = new PropertyModifierThread[]{ + PropertyModifierThread[] mts = new PropertyModifierThread[] { new PropertyModifierThread(1, UserType.F_GIVEN_NAME, true, ItemPath.create(UserType.F_ASSIGNMENT, 1L, AssignmentType.F_DESCRIPTION), true) }; - - Checker checker = (iteration, oid) -> { PrismObject userRetrieved = repositoryService.getObject(UserType.class, oid, null, new OperationResult("dummy")); String givenName = userRetrieved.asObjectable().getGivenName().getOrig(); @@ -107,24 +105,19 @@ public void test003OneWriter_TwoAttributes__OneReader() throws Exception { throw new AssertionError(msg); } }; - concurrencyUniversal("Test3", 60000L, 0L, mts, checker); } @Test public void test004TwoWriters_TwoAttributesEach__OneReader() throws Exception { - - PropertyModifierThread[] mts = new PropertyModifierThread[]{ + PropertyModifierThread[] mts = new PropertyModifierThread[] { new PropertyModifierThread(1, UserType.F_GIVEN_NAME, true, ItemPath.create(UserType.F_ASSIGNMENT, 1L, AssignmentType.F_DESCRIPTION), true), - new PropertyModifierThread(2, UserType.F_FAMILY_NAME, true, ItemPath.create(UserType.F_ASSIGNMENT, 1L, AssignmentType.F_CONSTRUCTION), true), }; - - Checker checker = (iteration, oid) -> { PrismObject userRetrieved = repositoryService.getObject(UserType.class, oid, null, new OperationResult("dummy")); String givenName = userRetrieved.asObjectable().getGivenName().getOrig(); @@ -143,7 +136,6 @@ public void test004TwoWriters_TwoAttributesEach__OneReader() throws Exception { throw new AssertionError(msg); } }; - concurrencyUniversal("Test4", 60000L, 0L, mts, checker); } @@ -165,47 +157,38 @@ private void concurrencyUniversal(String name, long duration, long waitStep, Pro OperationResult result = new OperationResult("Concurrency Test"); String oid = repositoryService.addObject(user, null, result); - LOGGER.info("*** Object added: " + oid + " ***"); - + LOGGER.info("*** Object added: {} ***", oid); LOGGER.info("*** Starting modifier threads ***"); -// modifierThreads[1].setOid(oid); -// modifierThreads[1].runOnce(); -// if(true) return; - for (PropertyModifierThread mt : modifierThreads) { - mt.setOid(oid); + mt.setObject(UserType.class, oid); mt.start(); } - LOGGER.info("*** Waiting " + duration + " ms ***"); + LOGGER.info("*** Waiting {} ms ***", duration); long startTime = System.currentTimeMillis(); int readIteration = 1; main: while (System.currentTimeMillis() - startTime < duration) { - if (checker != null) { checker.check(readIteration, oid); } - if (waitStep > 0L) { Thread.sleep(waitStep); } - for (PropertyModifierThread mt : modifierThreads) { if (!mt.isAlive()) { LOGGER.error("At least one of threads died prematurely, finishing waiting."); break main; } } - readIteration++; } for (PropertyModifierThread mt : modifierThreads) { mt.stop = true; // stop the threads - System.out.println("Thread " + mt.id + " has done " + (mt.counter - 1) + " iterations"); - LOGGER.info("Thread " + mt.id + " has done " + (mt.counter - 1) + " iterations"); + System.out.println("Thread " + mt.id + " has done " + mt.counter.get() + " iterations"); + LOGGER.info("Thread " + mt.id + " has done " + mt.counter.get() + " iterations"); } // we do not have to wait for the threads to be stopped, just examine their results @@ -213,7 +196,7 @@ private void concurrencyUniversal(String name, long duration, long waitStep, Pro Thread.sleep(1000); // give the threads a chance to finish (before repo will be shut down) for (PropertyModifierThread mt : modifierThreads) { - LOGGER.info("Modifier thread " + mt.id + " finished with an exception: ", mt.threadResult); + LOGGER.info("Modifier thread {} finished with an exception", mt.id, mt.threadResult); } for (PropertyModifierThread mt : modifierThreads) { @@ -226,10 +209,11 @@ private void concurrencyUniversal(String name, long duration, long waitStep, Pro abstract class WorkerThread extends Thread { int id; - String oid; // object to modify + Class objectClass; // object to modify + String oid; // object to modify String lastVersion = null; volatile Throwable threadResult; - volatile int counter = 1; + AtomicInteger counter = new AtomicInteger(0); WorkerThread(int id) { this.id = id; @@ -242,8 +226,7 @@ public void run() { try { while (!stop) { OperationResult result = new OperationResult("run"); - counter++; - LOGGER.info(" --- Iteration number {} for {} ---", counter, description()); + LOGGER.info(" --- Iteration number {} for {} ---", counter.incrementAndGet(), description()); runOnce(result); } } catch (Throwable t) { @@ -254,17 +237,16 @@ public void run() { abstract void runOnce(OperationResult result) throws Exception; abstract String description(); - - public void setOid(String oid) { + public void setObject(Class objectClass, String oid) { + this.objectClass = objectClass; this.oid = oid; } - } class PropertyModifierThread extends WorkerThread { - ItemPath attribute1; // attribute to modify - ItemPath attribute2; // attribute to modify + final ItemPath attribute1; // attribute to modify + final ItemPath attribute2; // attribute to modify boolean poly; boolean checkValue; @@ -296,7 +278,7 @@ void runOnce(OperationResult result) { PrismObjectDefinition userPrismDefinition = prismContext.getSchemaRegistry().findObjectDefinitionByCompileTimeClass(UserType.class); String prefix = lastName(attribute1); - String dataWritten = "[" + prefix + ":" + Integer.toString(counter) + "]"; + String dataWritten = "[" + prefix + ":" + counter.get() + "]"; PrismPropertyDefinition propertyDefinition1 = userPrismDefinition.findPropertyDefinition(attribute1); if (propertyDefinition1 == null) { @@ -308,7 +290,7 @@ void runOnce(OperationResult result) { List deltas = new ArrayList<>(); deltas.add(delta1); - ItemDefinition propertyDefinition2 = null; + ItemDefinition propertyDefinition2; if (attribute2 != null) { propertyDefinition2 = userPrismDefinition.findItemDefinition(attribute2); if (propertyDefinition2 == null) { @@ -317,18 +299,24 @@ void runOnce(OperationResult result) { ItemDelta delta2; if (propertyDefinition2 instanceof PrismContainerDefinition) { - delta2 = prismContext.deltaFactory().container().create(attribute2, (PrismContainerDefinition) propertyDefinition2); + //noinspection unchecked + delta2 = prismContext.deltaFactory().container().create(attribute2, (PrismContainerDefinition) propertyDefinition2); } else { + //noinspection unchecked delta2 = prismContext.deltaFactory().property().create(attribute2, (PrismPropertyDefinition) propertyDefinition2); } if (ConstructionType.COMPLEX_TYPE.equals(propertyDefinition2.getTypeName())) { ConstructionType act = new ConstructionType(); act.setDescription(dataWritten); + //noinspection unchecked delta2.setValueToReplace(act.asPrismContainerValue()); } else { + //noinspection unchecked delta2.setValueToReplace(prismContext.itemFactory().createPropertyValue(dataWritten)); } deltas.add(delta2); + } else { + propertyDefinition2 = null; } try { @@ -348,6 +336,7 @@ void runOnce(OperationResult result) { try { Thread.sleep(100); } catch (InterruptedException e) { + // ignore } PrismObject user; @@ -400,20 +389,15 @@ void runOnce(OperationResult result) { lastVersion = currentVersion; } } - - public void setOid(String oid) { - this.oid = oid; - } - } abstract class DeltaExecutionThread extends WorkerThread { String description; - DeltaExecutionThread(int id, String oid, String description) { + DeltaExecutionThread(int id, Class objectClass, String oid, String description) { super(id); - this.oid = oid; + setObject(objectClass, oid); this.description = description; this.setName("Executor: " + description); } @@ -426,9 +410,7 @@ String description() { abstract Collection> getItemDeltas() throws Exception; void runOnce(OperationResult result) throws Exception { - - repositoryService.modifyObject(UserType.class, oid, getItemDeltas(), result); - + repositoryService.modifyObject(objectClass, oid, getItemDeltas(), result); } } @@ -454,6 +436,7 @@ public void test010SearchIterative() throws Exception { .createModificationReplaceProperty(UserType.class, object.getOid(), UserType.F_FULL_NAME, new PolyString(newFullName)); try { + //noinspection unchecked repositoryService.modifyObject(UserType.class, object.getOid(), delta.getModifications(), @@ -466,7 +449,7 @@ public void test010SearchIterative() throws Exception { null, true, result); PrismObject reloaded = repositoryService.getObject(UserType.class, oid, null, result); - AssertJUnit.assertEquals("Full name was not changed", newFullName, reloaded.asObjectable().getFullName().getOrig()); + assertEquals("Full name was not changed", newFullName, reloaded.asObjectable().getFullName().getOrig()); } @Test @@ -496,7 +479,7 @@ public void test100AddOperationExecution() throws Exception { for (int i = 0; i < THREADS; i++) { final int threadIndex = i; - DeltaExecutionThread thread = new DeltaExecutionThread(i, oid, "operationExecution adder #" + i) { + DeltaExecutionThread thread = new DeltaExecutionThread(i, UserType.class, oid, "operationExecution adder #" + i) { @Override Collection> getItemDeltas() throws Exception { return prismContext.deltaFor(UserType.class) @@ -518,6 +501,20 @@ public void test100AddOperationExecution() throws Exception { public void test110AddAssignments() throws Exception { if (getConfiguration().isUsingH2()) { + // Because of: + // Caused by: javax.persistence.EntityExistsException: A different object with the same identifier value was already associated with the session : [com.evolveum.midpoint.repo.sql.data.common.container.RAssignment#RContainerId{44c3e25d-e790-4142-958d-ff7ff3ff3a9f, 62}] + // at org.hibernate.internal.ExceptionConverterImpl.convert(ExceptionConverterImpl.java:118) + // at org.hibernate.internal.ExceptionConverterImpl.convert(ExceptionConverterImpl.java:157) + // at org.hibernate.internal.ExceptionConverterImpl.convert(ExceptionConverterImpl.java:164) + // at org.hibernate.internal.SessionImpl.doFlush(SessionImpl.java:1443) + // at org.hibernate.internal.SessionImpl.managedFlush(SessionImpl.java:493) + // at org.hibernate.internal.SessionImpl.flushBeforeTransactionCompletion(SessionImpl.java:3207) + // at org.hibernate.internal.SessionImpl.beforeTransactionCompletion(SessionImpl.java:2413) + // at org.hibernate.engine.jdbc.internal.JdbcCoordinatorImpl.beforeTransactionCompletion(JdbcCoordinatorImpl.java:473) + // at org.hibernate.resource.transaction.backend.jdbc.internal.JdbcResourceLocalTransactionCoordinatorImpl.beforeCompletionCallback(JdbcResourceLocalTransactionCoordinatorImpl.java:156) + // at org.hibernate.resource.transaction.backend.jdbc.internal.JdbcResourceLocalTransactionCoordinatorImpl.access$100(JdbcResourceLocalTransactionCoordinatorImpl.java:38) + // at org.hibernate.resource.transaction.backend.jdbc.internal.JdbcResourceLocalTransactionCoordinatorImpl$TransactionDriverControlImpl.commit(JdbcResourceLocalTransactionCoordinatorImpl.java:231) + // at org.hibernate.engine.transaction.internal.TransactionImpl.commit(TransactionImpl.java:68) return; // TODO } @@ -537,7 +534,7 @@ public void test110AddAssignments() throws Exception { for (int i = 0; i < THREADS; i++) { final int threadIndex = i; - DeltaExecutionThread thread = new DeltaExecutionThread(i, oid, "assignment adder #" + i) { + DeltaExecutionThread thread = new DeltaExecutionThread(i, UserType.class, oid, "assignment adder #" + i) { @Override Collection> getItemDeltas() throws Exception { return prismContext.deltaFor(UserType.class) @@ -556,7 +553,70 @@ public void test110AddAssignments() throws Exception { display("user after", userAfter); } - protected void waitForThreads(List threads, long DURATION) throws InterruptedException { + @Test + public void test120AddApproverRef() throws Exception { + + int THREADS = 4; + long DURATION = 30000L; + final String APPROVER_REF_FORMAT = "oid-%d-%s"; + + RoleType role = new RoleType(prismContext).name("judge"); + + OperationResult result = new OperationResult("test120AddApproverRef"); + String oid = repositoryService.addObject(role.asPrismObject(), null, result); + + PrismTestUtil.display("object added", oid); + + LOGGER.info("Starting worker threads"); + + List threads = new ArrayList<>(); + for (int i = 0; i < THREADS; i++) { + final int threadIndex = i; + + DeltaExecutionThread thread = new DeltaExecutionThread(i, RoleType.class, oid, "approverRef adder #" + i) { + @Override + Collection> getItemDeltas() throws Exception { + return prismContext.deltaFor(RoleType.class) + .item(RoleType.F_APPROVER_REF).add( + ObjectTypeUtil.createObjectRef(String.format(APPROVER_REF_FORMAT, threadIndex, counter), ObjectTypes.USER)) + .asItemDeltas(); + } + }; + thread.start(); + threads.add(thread); + } + + waitForThreads(threads, DURATION); + PrismObject roleAfter = repositoryService.getObject(RoleType.class, oid, null, result); + display("role after", roleAfter); + + int totalExecutions = threads.stream().mapToInt(t -> t.counter.get()).sum(); + int totalApprovers = roleAfter.asObjectable().getApproverRef().size(); + System.out.println("Total executions: " + totalExecutions); + System.out.println("Approvers: " + totalApprovers); + + List failures = new ArrayList<>(); + for (DeltaExecutionThread thread : threads) { + for (int i = 1; i <= thread.counter.get(); i++) { + String expected = String.format(APPROVER_REF_FORMAT, thread.id, i); + List matchingOids = roleAfter.asObjectable().getApproverRef().stream() + .map(ObjectReferenceType::getOid) + .filter(refOid -> refOid.equals(expected)) + .collect(Collectors.toList()); + if (matchingOids.size() != 1) { + failures.add("Wrong # of occurrences of " + expected + ": " + matchingOids); + } + } + } + + System.out.println("Failures:"); + failures.forEach(line -> System.out.println(" - " + line)); + + assertEquals("Wrong # of approvers", totalExecutions, totalApprovers); + assertEquals("Failures are there", 0, failures.size()); + } + + private void waitForThreads(List threads, long DURATION) throws InterruptedException { LOGGER.info("*** Waiting {} ms ***", DURATION); long startTime = System.currentTimeMillis(); main: @@ -574,8 +634,8 @@ protected void waitForThreads(List threads, long DURATIO for (WorkerThread thread : threads) { thread.stop = true; // stop the threads - System.out.println("Thread " + thread.id + " has done " + (thread.counter - 1) + " iterations"); - LOGGER.info("Thread " + thread.id + " has done " + (thread.counter - 1) + " iterations"); + System.out.println("Thread " + thread.id + " has done " + thread.counter.get() + " iterations"); + LOGGER.info("Thread " + thread.id + " has done " + thread.counter.get() + " iterations"); } // we do not have to wait for the threads to be stopped, just examine their results diff --git a/repo/repo-sql-impl/src/main/java/com/evolveum/midpoint/repo/sql/SqlRepositoryServiceImpl.java b/repo/repo-sql-impl/src/main/java/com/evolveum/midpoint/repo/sql/SqlRepositoryServiceImpl.java index 4689e8db0de..c4e9226d0a2 100644 --- a/repo/repo-sql-impl/src/main/java/com/evolveum/midpoint/repo/sql/SqlRepositoryServiceImpl.java +++ b/repo/repo-sql-impl/src/main/java/com/evolveum/midpoint/repo/sql/SqlRepositoryServiceImpl.java @@ -617,7 +617,7 @@ public ModifyObjectResult modifyObject(Class type, try { while (true) { try { - ModifyObjectResult rv = objectUpdater.modifyObjectAttempt(type, oid, modifications, precondition, options, subResult, this); + ModifyObjectResult rv = objectUpdater.modifyObjectAttempt(type, oid, modifications, precondition, options, attempt, subResult, this); invokeConflictWatchers((w) -> w.afterModifyObject(oid)); return rv; } catch (RestartOperationRequestedException ex) { diff --git a/repo/repo-sql-impl/src/main/java/com/evolveum/midpoint/repo/sql/helpers/ObjectUpdater.java b/repo/repo-sql-impl/src/main/java/com/evolveum/midpoint/repo/sql/helpers/ObjectUpdater.java index 94d7601c13e..7d8020e4222 100644 --- a/repo/repo-sql-impl/src/main/java/com/evolveum/midpoint/repo/sql/helpers/ObjectUpdater.java +++ b/repo/repo-sql-impl/src/main/java/com/evolveum/midpoint/repo/sql/helpers/ObjectUpdater.java @@ -365,7 +365,7 @@ public DeleteObjectResult deleteObjectAttempt(Class ty public ModifyObjectResult modifyObjectAttempt(Class type, String oid, Collection originalModifications, ModificationPrecondition precondition, - RepoModifyOptions modifyOptions, OperationResult result, SqlRepositoryServiceImpl sqlRepositoryService) + RepoModifyOptions modifyOptions, int attempt, OperationResult result, SqlRepositoryServiceImpl sqlRepositoryService) throws ObjectNotFoundException, SchemaException, ObjectAlreadyExistsException, SerializationRelatedException, PreconditionViolationException { @@ -374,8 +374,8 @@ public ModifyObjectResult modifyObjectAttempt(Class Collection modifications = CloneUtil.cloneCollectionMembers(originalModifications); //modifications = new ArrayList<>(modifications); - LOGGER.debug("Modifying object '{}' with oid '{}'.", type.getSimpleName(), oid); - LOGGER_PERFORMANCE.debug("> modify object {}, oid={}, modifications={}", type.getSimpleName(), oid, modifications); + LOGGER.debug("Modifying object '{}' with oid '{}' (attempt {})", type.getSimpleName(), oid, attempt); + LOGGER_PERFORMANCE.debug("> modify object {}, oid={} (attempt {}), modifications={}", type.getSimpleName(), oid, attempt, modifications); LOGGER.trace("Modifications:\n{}", DebugUtil.debugDumpLazily(modifications)); Session session = null; @@ -490,7 +490,7 @@ public ModifyObjectResult modifyObjectAttempt(Class LOGGER.trace("Before commit..."); session.getTransaction().commit(); - LOGGER.trace("Committed!"); + LOGGER.trace("Committed! (at attempt {})", attempt); return rv; } catch (ObjectNotFoundException | SchemaException ex) { baseHelper.rollbackTransaction(session, ex, result, true); From b790e83b8b6599002b79763af18e4fa0fe270fea Mon Sep 17 00:00:00 2001 From: Pavol Mederly Date: Thu, 6 Jun 2019 11:37:15 +0200 Subject: [PATCH 4/6] Fix thread stopping in ConcurrencyTest --- .../midpoint/repo/sql/ConcurrencyTest.java | 21 ++++++++++++------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/repo/repo-sql-impl-test/src/test/java/com/evolveum/midpoint/repo/sql/ConcurrencyTest.java b/repo/repo-sql-impl-test/src/test/java/com/evolveum/midpoint/repo/sql/ConcurrencyTest.java index 178e9a03150..cf9fd504bb7 100644 --- a/repo/repo-sql-impl-test/src/test/java/com/evolveum/midpoint/repo/sql/ConcurrencyTest.java +++ b/repo/repo-sql-impl-test/src/test/java/com/evolveum/midpoint/repo/sql/ConcurrencyTest.java @@ -46,7 +46,7 @@ import java.util.concurrent.atomic.AtomicInteger; import java.util.stream.Collectors; -import static org.testng.AssertJUnit.assertEquals; +import static org.testng.AssertJUnit.*; /** * @author Pavol Mederly @@ -58,7 +58,7 @@ public class ConcurrencyTest extends BaseSQLRepoTest { private static final Trace LOGGER = TraceManager.getTrace(ConcurrencyTest.class); - //private static final long WAIT_TIME = 60000; + private static final long WAIT_FOR_THREAD_NATURAL_STOP_TIME = 300000; //private static final long WAIT_STEP = 500; @Test @@ -616,11 +616,11 @@ public void test120AddApproverRef() throws Exception { assertEquals("Failures are there", 0, failures.size()); } - private void waitForThreads(List threads, long DURATION) throws InterruptedException { - LOGGER.info("*** Waiting {} ms ***", DURATION); + private void waitForThreads(List threads, long duration) throws InterruptedException { + LOGGER.info("*** Waiting {} ms ***", duration); long startTime = System.currentTimeMillis(); main: - while (System.currentTimeMillis() - startTime < DURATION) { + while (System.currentTimeMillis() - startTime < duration) { for (WorkerThread thread : threads) { if (!thread.isAlive()) { @@ -638,9 +638,14 @@ private void waitForThreads(List threads, long DURATION) LOGGER.info("Thread " + thread.id + " has done " + thread.counter.get() + " iterations"); } - // we do not have to wait for the threads to be stopped, just examine their results - - Thread.sleep(1000); // give the threads a chance to finish (before repo will be shut down) + long start = System.currentTimeMillis(); + boolean anyAlive = true; + while (anyAlive && System.currentTimeMillis() - start < WAIT_FOR_THREAD_NATURAL_STOP_TIME) { + anyAlive = threads.stream().anyMatch(Thread::isAlive); + Thread.sleep(100); + } + List alive = threads.stream().filter(Thread::isAlive).map(Thread::getName).collect(Collectors.toList()); + assertTrue("Some threads had not stopped in given time: " + alive, alive.isEmpty()); for (WorkerThread thread : threads) { LOGGER.info("Modifier thread " + thread.id + " finished with an exception: ", thread.threadResult); From 8706958507869d7d5911f7918f11cd1143ede92c Mon Sep 17 00:00:00 2001 From: Pavol Mederly Date: Thu, 6 Jun 2019 14:23:34 +0200 Subject: [PATCH 5/6] Tentatively fix H2 data corruption (MID-5409) It looks like SELECT ... FOR UPDATE helps here. Effects on performance and functionality are yet to be determined. --- .../repo/sql/ObjectDeltaUpdaterTest.java | 26 +++++++++---------- .../repo/sql/SqlRepositoryConfiguration.java | 2 +- .../repo/sql/SqlRepositoryFactory.java | 2 +- 3 files changed, 15 insertions(+), 15 deletions(-) diff --git a/repo/repo-sql-impl-test/src/test/java/com/evolveum/midpoint/repo/sql/ObjectDeltaUpdaterTest.java b/repo/repo-sql-impl-test/src/test/java/com/evolveum/midpoint/repo/sql/ObjectDeltaUpdaterTest.java index 978c02a19b2..7f66552e543 100644 --- a/repo/repo-sql-impl-test/src/test/java/com/evolveum/midpoint/repo/sql/ObjectDeltaUpdaterTest.java +++ b/repo/repo-sql-impl-test/src/test/java/com/evolveum/midpoint/repo/sql/ObjectDeltaUpdaterTest.java @@ -119,7 +119,7 @@ public void test100UpdateGivenNameAndActivation() throws Exception { repositoryService.modifyObject(UserType.class, userOid, delta.getModifications(), result); if (baseHelper.getConfiguration().isUsingH2()) { - AssertJUnit.assertEquals(4, queryCountInterceptor.getQueryCount()); + AssertJUnit.assertEquals(5, queryCountInterceptor.getQueryCount()); } Session session = factory.openSession(); @@ -155,7 +155,7 @@ public void test115DeleteActivation() throws Exception { repositoryService.modifyObject(UserType.class, userOid, delta.getModifications(), result); if (baseHelper.getConfiguration().isUsingH2()) { - AssertJUnit.assertEquals(3, queryCountInterceptor.getQueryCount()); + AssertJUnit.assertEquals(4, queryCountInterceptor.getQueryCount()); } Session session = factory.openSession(); @@ -180,7 +180,7 @@ public void test110ReplaceNonIndexedExtensionProperty() throws Exception { repositoryService.modifyObject(UserType.class, userOid, delta.getModifications(), result); if (baseHelper.getConfiguration().isUsingH2()) { - AssertJUnit.assertEquals(2, queryCountInterceptor.getQueryCount()); + AssertJUnit.assertEquals(3, queryCountInterceptor.getQueryCount()); } Session session = factory.openSession(); @@ -234,7 +234,7 @@ public void test120AddExtensionProperty() throws Exception { repositoryService.modifyObject(UserType.class, userOid, delta.getModifications(), result); if (baseHelper.getConfiguration().isUsingH2()) { - AssertJUnit.assertEquals(5, queryCountInterceptor.getQueryCount()); + AssertJUnit.assertEquals(6, queryCountInterceptor.getQueryCount()); } Session session = factory.openSession(); @@ -275,7 +275,7 @@ public void test140AddDeleteAssignment() throws Exception { // select createappr0_.owner_id as owner_id1_14_0_, createappr0_.owner_owner_oid as owner_ow2_14_0_, createappr0_.reference_type as referenc3_14_0_, createappr0_.relation as relation4_14_0_, createappr0_.targetOid as targetOi5_14_0_, createappr0_.owner_id as owner_id1_14_1_, createappr0_.owner_owner_oid as owner_ow2_14_1_, createappr0_.reference_type as referenc3_14_1_, createappr0_.relation as relation4_14_1_, createappr0_.targetOid as targetOi5_14_1_, createappr0_.targetType as targetTy6_14_1_ from m_assignment_reference createappr0_ where ( createappr0_.reference_type= 0) and createappr0_.owner_id=? and createappr0_.owner_owner_oid=? // select modifyappr0_.owner_id as owner_id1_14_0_, modifyappr0_.owner_owner_oid as owner_ow2_14_0_, modifyappr0_.reference_type as referenc3_14_0_, modifyappr0_.relation as relation4_14_0_, modifyappr0_.targetOid as targetOi5_14_0_, modifyappr0_.owner_id as owner_id1_14_1_, modifyappr0_.owner_owner_oid as owner_ow2_14_1_, modifyappr0_.reference_type as referenc3_14_1_, modifyappr0_.relation as relation4_14_1_, modifyappr0_.targetOid as targetOi5_14_1_, modifyappr0_.targetType as targetTy6_14_1_ from m_assignment_reference modifyappr0_ where ( modifyappr0_.reference_type= 1) and modifyappr0_.owner_id=? and modifyappr0_.owner_owner_oid=? if (baseHelper.getConfiguration().isUsingH2()) { - AssertJUnit.assertEquals(9, queryCountInterceptor.getQueryCount()); + AssertJUnit.assertEquals(10, queryCountInterceptor.getQueryCount()); } Session session = factory.openSession(); @@ -316,7 +316,7 @@ public void test145AddActivationToAssignment() throws Exception { repositoryService.modifyObject(UserType.class, userOid, delta.getModifications(), result); if (baseHelper.getConfiguration().isUsingH2()) { - AssertJUnit.assertEquals(4, queryCountInterceptor.getQueryCount()); + AssertJUnit.assertEquals(5, queryCountInterceptor.getQueryCount()); } Session session = factory.openSession(); @@ -352,7 +352,7 @@ public void test150AddDeleteLinkRef() throws Exception { repositoryService.modifyObject(UserType.class, userOid, delta.getModifications(), result); if (baseHelper.getConfiguration().isUsingH2()) { - AssertJUnit.assertEquals(5, queryCountInterceptor.getQueryCount()); + AssertJUnit.assertEquals(6, queryCountInterceptor.getQueryCount()); } Session session = factory.openSession(); @@ -385,7 +385,7 @@ public void test160AddDeleteParentRef() throws Exception { repositoryService.modifyObject(UserType.class, userOid, delta.getModifications(), result); if (baseHelper.getConfiguration().isUsingH2()) { - AssertJUnit.assertEquals(5, queryCountInterceptor.getQueryCount()); + AssertJUnit.assertEquals(6, queryCountInterceptor.getQueryCount()); } Session session = factory.openSession(); @@ -448,7 +448,7 @@ public void test170ModifyEmployeeTypeAndMetadataCreateChannel() throws Exception repositoryService.modifyObject(UserType.class, userOid, delta.getModifications(), result); if (baseHelper.getConfiguration().isUsingH2()) { - AssertJUnit.assertEquals(4, queryCountInterceptor.getQueryCount()); + AssertJUnit.assertEquals(5, queryCountInterceptor.getQueryCount()); } Session session = factory.openSession(); @@ -475,7 +475,7 @@ public void test180ModifyMetadataChannel() throws Exception { repositoryService.modifyObject(UserType.class, userOid, delta.getModifications(), result); if (baseHelper.getConfiguration().isUsingH2()) { - AssertJUnit.assertEquals(4, queryCountInterceptor.getQueryCount()); + AssertJUnit.assertEquals(5, queryCountInterceptor.getQueryCount()); } Session session = factory.openSession(); @@ -635,7 +635,7 @@ public void test280AddPhoto() throws Exception { repositoryService.modifyObject(UserType.class, userOid, delta.getModifications(), result); if (baseHelper.getConfiguration().isUsingH2()) { - AssertJUnit.assertEquals(5, queryCountInterceptor.getQueryCount()); + AssertJUnit.assertEquals(6, queryCountInterceptor.getQueryCount()); } LOGGER.info("test280AddPhoto check"); @@ -666,7 +666,7 @@ public void test290ReplacePhoto() throws Exception { repositoryService.modifyObject(UserType.class, userOid, delta.getModifications(), result); if (baseHelper.getConfiguration().isUsingH2()) { - AssertJUnit.assertEquals(5, queryCountInterceptor.getQueryCount()); + AssertJUnit.assertEquals(6, queryCountInterceptor.getQueryCount()); } LOGGER.info("test290ReplacePhoto check"); @@ -697,7 +697,7 @@ public void test300DeletePhoto() throws Exception { repositoryService.modifyObject(UserType.class, userOid, delta.getModifications(), result); if (baseHelper.getConfiguration().isUsingH2()) { - AssertJUnit.assertEquals(4, queryCountInterceptor.getQueryCount()); + AssertJUnit.assertEquals(5, queryCountInterceptor.getQueryCount()); } LOGGER.info("test300DeletePhoto check"); diff --git a/repo/repo-sql-impl/src/main/java/com/evolveum/midpoint/repo/sql/SqlRepositoryConfiguration.java b/repo/repo-sql-impl/src/main/java/com/evolveum/midpoint/repo/sql/SqlRepositoryConfiguration.java index d034643e344..53680622b7f 100644 --- a/repo/repo-sql-impl/src/main/java/com/evolveum/midpoint/repo/sql/SqlRepositoryConfiguration.java +++ b/repo/repo-sql-impl/src/main/java/com/evolveum/midpoint/repo/sql/SqlRepositoryConfiguration.java @@ -622,7 +622,7 @@ private void computeDefaultConcurrencyParameters() { if (isUsingH2()) { defaultTransactionIsolation = TransactionIsolation.SERIALIZABLE; defaultLockForUpdateViaHibernate = false; - defaultLockForUpdateViaSql = false; + defaultLockForUpdateViaSql = true; defaultUseReadOnlyTransactions = false; // h2 does not support "SET TRANSACTION READ ONLY" command } else if (isUsingMySqlCompatible()) { defaultTransactionIsolation = TransactionIsolation.SERIALIZABLE; diff --git a/repo/repo-sql-impl/src/main/java/com/evolveum/midpoint/repo/sql/SqlRepositoryFactory.java b/repo/repo-sql-impl/src/main/java/com/evolveum/midpoint/repo/sql/SqlRepositoryFactory.java index 29a503b51b9..4745aa2a98c 100644 --- a/repo/repo-sql-impl/src/main/java/com/evolveum/midpoint/repo/sql/SqlRepositoryFactory.java +++ b/repo/repo-sql-impl/src/main/java/com/evolveum/midpoint/repo/sql/SqlRepositoryFactory.java @@ -236,7 +236,7 @@ private String[] createArguments(SqlRepositoryConfiguration config) { args.add(Integer.toString(config.getPort())); } - return args.toArray(new String[args.size()]); + return args.toArray(new String[0]); } private void dropDatabaseIfExists(SqlRepositoryConfiguration config) throws RepositoryServiceFactoryException { From e6f59724799a6bfe88fa8bfe6df1613f69cfd5ab Mon Sep 17 00:00:00 2001 From: skublik Date: Thu, 6 Jun 2019 15:21:38 +0200 Subject: [PATCH 6/6] fix for sorting of properties --- ...oggingConfigurationWrapperFactoryImpl.java | 1 - .../PrismContainerWrapperFactoryImpl.java | 2 +- .../impl/prism/PrismContainerValuePanel.java | 46 ++++++++++++++++++- 3 files changed, 45 insertions(+), 4 deletions(-) diff --git a/gui/admin-gui/src/main/java/com/evolveum/midpoint/gui/impl/factory/LoggingConfigurationWrapperFactoryImpl.java b/gui/admin-gui/src/main/java/com/evolveum/midpoint/gui/impl/factory/LoggingConfigurationWrapperFactoryImpl.java index 3ff8bfa00e4..c1ebea87f1b 100644 --- a/gui/admin-gui/src/main/java/com/evolveum/midpoint/gui/impl/factory/LoggingConfigurationWrapperFactoryImpl.java +++ b/gui/admin-gui/src/main/java/com/evolveum/midpoint/gui/impl/factory/LoggingConfigurationWrapperFactoryImpl.java @@ -115,7 +115,6 @@ public PrismContainerValueWrapper createValueWrapper(PrismContainerWrapper } containerValueWrapper.getItems().addAll((Collection) wrappers); - containerValueWrapper.sort(); return containerValueWrapper; } diff --git a/gui/admin-gui/src/main/java/com/evolveum/midpoint/gui/impl/factory/PrismContainerWrapperFactoryImpl.java b/gui/admin-gui/src/main/java/com/evolveum/midpoint/gui/impl/factory/PrismContainerWrapperFactoryImpl.java index 35fb2fd0a5e..10a5454731b 100644 --- a/gui/admin-gui/src/main/java/com/evolveum/midpoint/gui/impl/factory/PrismContainerWrapperFactoryImpl.java +++ b/gui/admin-gui/src/main/java/com/evolveum/midpoint/gui/impl/factory/PrismContainerWrapperFactoryImpl.java @@ -96,7 +96,7 @@ public PrismContainerValueWrapper createValueWrapper(PrismContainerWrapper } containerValueWrapper.getItems().addAll((Collection) wrappers); - containerValueWrapper.sort(); +// containerValueWrapper.sort(); return containerValueWrapper; } diff --git a/gui/admin-gui/src/main/java/com/evolveum/midpoint/gui/impl/prism/PrismContainerValuePanel.java b/gui/admin-gui/src/main/java/com/evolveum/midpoint/gui/impl/prism/PrismContainerValuePanel.java index f746e93a3cf..b16571c9061 100644 --- a/gui/admin-gui/src/main/java/com/evolveum/midpoint/gui/impl/prism/PrismContainerValuePanel.java +++ b/gui/admin-gui/src/main/java/com/evolveum/midpoint/gui/impl/prism/PrismContainerValuePanel.java @@ -15,7 +15,11 @@ */ package com.evolveum.midpoint.gui.impl.prism; +import java.text.Collator; +import java.util.Collections; +import java.util.Comparator; import java.util.List; +import java.util.Locale; import javax.xml.namespace.QName; @@ -40,6 +44,8 @@ import com.evolveum.midpoint.gui.api.component.BasePanel; import com.evolveum.midpoint.gui.api.component.togglebutton.ToggleIconButton; import com.evolveum.midpoint.gui.api.prism.ItemWrapper; +import com.evolveum.midpoint.gui.api.prism.PrismContainerWrapper; +import com.evolveum.midpoint.gui.api.util.WebModelServiceUtils; import com.evolveum.midpoint.gui.impl.factory.WrapperContext; import com.evolveum.midpoint.prism.Containerable; import com.evolveum.midpoint.prism.Item; @@ -144,7 +150,43 @@ private , ID extends ItemDefinitio propertiesLabel.setOutputMarkupId(true); ListView properties = new ListView("properties", - new PropertyModel<>(getModel(), "nonContainers")) { + new IModel>() { + + @Override + public List getObject() { + List> nonContainers = getModelObject().getNonContainers(); + + Locale locale = WebModelServiceUtils.getLocale(); + if (locale == null) { + locale = Locale.getDefault(); + } + Collator collator = Collator.getInstance(locale); + collator.setStrength(Collator.SECONDARY); // e.g. "a" should be different from "รก" + collator.setDecomposition(Collator.FULL_DECOMPOSITION); + ItemWrapperComparator comparator = new ItemWrapperComparator<>(collator, getModelObject().isSorted()); + if (CollectionUtils.isNotEmpty(nonContainers)) { + nonContainers.sort((Comparator) comparator); + + int visibleProperties = 0; + + for (ItemWrapper item : nonContainers) { + if (item.isVisible(null)) { + visibleProperties++; + } + + if (visibleProperties % 2 == 0) { + item.setStripe(false); + } else { + item.setStripe(true); + } + + } + } + + return (List) nonContainers; + } + }) { +// new PropertyModel<>(getModel(), "nonContainers")) { private static final long serialVersionUID = 1L; @Override @@ -182,7 +224,7 @@ public boolean isVisible() { }; - properties.setReuseItems(true); +// properties.setReuseItems(true); properties.setOutputMarkupId(true); add(propertiesLabel); propertiesLabel.add(properties);