Merge remote-tracking branch 'origin/master'

Evolveum · Sep 14, 2020 · 3f70dfd · 3f70dfd
2 parents cf55277 + a9121df
commit 3f70dfd
Show file tree

Hide file tree

Showing 13 changed files with 997 additions and 66 deletions.
diff --git a/...i/src/main/java/com/evolveum/midpoint/gui/impl/prism/panel/DefaultContainerablePanel.java b/...i/src/main/java/com/evolveum/midpoint/gui/impl/prism/panel/DefaultContainerablePanel.java
@@ -181,7 +181,7 @@ private void populateNonContainer(ListItem<? extends ItemWrapper<?, ?>> item) {
             Panel panel = getPageBase().initItemPanel("property", typeName, item.getModel(), settings);
             panel.setOutputMarkupId(true);
             panel.add(AttributeModifier.append("class", appendStyleClassModel(item.getModel())));
-            panel.add(new VisibleEnableBehaviour() {
+            item.add(new VisibleEnableBehaviour() {
                 @Override
                 public boolean isVisible() {
                     return itemWrapper.isVisible(getModelObject(), getVisibilityHandler());

diff --git a/...odel-impl/src/main/java/com/evolveum/midpoint/model/impl/security/GuiProfileCompiler.java b/...odel-impl/src/main/java/com/evolveum/midpoint/model/impl/security/GuiProfileCompiler.java
@@ -302,11 +302,11 @@ private String determineViewIdentifier(GuiObjectListViewType objectListViewType)
         }
         CollectionRefSpecificationType collection = objectListViewType.getCollection();
         if (collection == null) {
-            return objectListViewType.getType().getLocalPart();
+            return objectListViewType.getType() != null ? objectListViewType.getType().getLocalPart() : null;
         }
         ObjectReferenceType collectionRef = collection.getCollectionRef();
         if (collectionRef == null) {
-            return objectListViewType.getType().getLocalPart();
+            return objectListViewType.getType() != null ? objectListViewType.getType().getLocalPart() : null;
         }
         return collectionRef.getOid();
     }

diff --git a/pom.xml b/pom.xml
@@ -182,12 +182,13 @@
         <!--
         Default Spring Boot 2.3.3 uses H2 1.4.200 but that one has NPE bug:
         https://github.com/h2database/h2database/issues/1808
-        Sometimes occurs on heavy concurrent test, e.g. on TestDummyParallelism.
+        Sometimes occurs on heavy concurrent test, e.g. on TestDummyParallelism, but also during app run.
         This bug was introduced in 1.4.198, which was major version that also changed queries in OrgClosureManager.
-        If h2<=1.4.197 is used, the OrgClosureManager must be changed to previous version.
+        Between 1.4.197 and 198 changes of OrgClosureManager and around MVCC must be made (see commit history).
         With 1.4.200 TestObjectLifecycleApprovalGlobal fails consistently.
+        197 seems to be last version that works OK with midPoint before very disruptive changes.
         -->
-        <h2.version>1.4.199</h2.version>
+        <h2.version>1.4.197</h2.version>
         <!-- Spring Boot parent declares most fresh versions for JDBC drivers for all our servers -->
         <wicket.version>8.6.0</wicket.version>
         <groovy.version>2.5.12</groovy.version>

diff --git a/...pl/src/main/java/com/evolveum/midpoint/provisioning/impl/shadowmanager/ShadowManager.java b/...pl/src/main/java/com/evolveum/midpoint/provisioning/impl/shadowmanager/ShadowManager.java
@@ -1926,7 +1926,13 @@ public PrismObject<ShadowType> recordDeleteResult(
     public void deleteShadow(ProvisioningContext ctx, PrismObject<ShadowType> oldRepoShadow, OperationResult parentResult)
             throws ObjectNotFoundException, SchemaException, CommunicationException, ConfigurationException, ExpressionEvaluationException {
         LOGGER.trace("Deleting repository {}", oldRepoShadow);
-        repositoryService.deleteObject(ShadowType.class, oldRepoShadow.getOid(), parentResult);
+        try {
+            repositoryService.deleteObject(ShadowType.class, oldRepoShadow.getOid(), parentResult);
+        } catch (ObjectNotFoundException e) {
+            // Attempt to delete shadow that is already deleted. No big deal.
+            parentResult.muteLastSubresultError();
+            LOGGER.trace("Attempt to delete repository {} that is already deleted. Ignoring error.", oldRepoShadow);
+        }
     }
 
 

diff --git a/...epo-sql-impl/src/main/java/com/evolveum/midpoint/repo/sql/SqlRepositoryConfiguration.java b/...epo-sql-impl/src/main/java/com/evolveum/midpoint/repo/sql/SqlRepositoryConfiguration.java
@@ -567,6 +567,8 @@ private String readFile(String filename) throws IOException {
 
     private String getDefaultEmbeddedJdbcUrl() {
         return getDefaultEmbeddedJdbcUrlPrefix()
+                // TODO: Was used for 1.4.193, but do we really need it?
+//                + ";MVCC=FALSE" // Turn off MVCC, revert to table locking.
                 // Disable database closing on exit. By default, a database is closed when the last connection is closed.
                 + ";DB_CLOSE_ON_EXIT=FALSE"
                 // Both read locks and write locks are kept until the transaction commits.

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
@@ -222,8 +222,8 @@ private String[] createArguments(SqlRepositoryConfiguration config) {
             args.add(Integer.toString(config.getPort()));
         }
         // Allows auto-creation of remote database, which is a security hole and was forbidden
-        // from 1.4.138, see https://h2database.com/html/tutorial.html#creating_new_databases
-        args.add("-ifNotExists");
+        // from 1.4.198, see https://h2database.com/html/tutorial.html#creating_new_databases
+//        args.add("-ifNotExists"); // we're using <198 version now
 
         return args.toArray(new String[0]);
     }

diff --git a/...repo-sql-impl/src/main/java/com/evolveum/midpoint/repo/sql/helpers/OrgClosureManager.java b/...repo-sql-impl/src/main/java/com/evolveum/midpoint/repo/sql/helpers/OrgClosureManager.java
@@ -593,7 +593,13 @@ private void addIndependentEdgesInternal(List<Edge> edges, Context context, Sess
 
                 long startUpdate = System.currentTimeMillis();
                 String updateInClosureQueryText;
-                if (isH2() || isPostgreSQL()) {
+                // Can/must be unified with PG after H2 > 1.4.200 if no other issues emerge.
+                if (isH2()) {
+                    updateInClosureQueryText = "update " + CLOSURE_TABLE_NAME + " " +
+                            "set val = val + (select val from " + deltaTempTableName + " td " +
+                            "where td.descendant_oid=" + CLOSURE_TABLE_NAME + ".descendant_oid and td.ancestor_oid=" + CLOSURE_TABLE_NAME + ".ancestor_oid) " +
+                            "where (descendant_oid, ancestor_oid) in (select (descendant_oid, ancestor_oid) from " + deltaTempTableName + ")";
+                } else if (isPostgreSQL()) {
                     updateInClosureQueryText = "update " + CLOSURE_TABLE_NAME + " " +
                             "set val = val + (select val from " + deltaTempTableName + " td " +
                             "where td.descendant_oid=" + CLOSURE_TABLE_NAME + ".descendant_oid and td.ancestor_oid=" + CLOSURE_TABLE_NAME + ".ancestor_oid) " +
@@ -616,7 +622,10 @@ private void addIndependentEdgesInternal(List<Edge> edges, Context context, Sess
                         "insert into " + CLOSURE_TABLE_NAME + " (descendant_oid, ancestor_oid, val) " +
                                 "select descendant_oid, ancestor_oid, val from " + deltaTempTableName + " delta ";
                 if (countUpdate > 0) {
-                    if (isH2() || isPostgreSQL()) {
+                    // Can/must be unified with PG after H2 > 1.4.200 if no other issues emerge.
+                    if (isH2()) {
+                        addQuery += " where (descendant_oid, ancestor_oid) not in (select (descendant_oid, ancestor_oid) from " + CLOSURE_TABLE_NAME + ")";
+                    } else if (isPostgreSQL()) {
                         addQuery += " where not exists (select 1 from " + CLOSURE_TABLE_NAME + " cl where cl.descendant_oid=delta.descendant_oid and cl.ancestor_oid=delta.ancestor_oid)";
                     } else {
                         throw new UnsupportedOperationException("Org. closure manager - unsupported database operation");
@@ -799,7 +808,19 @@ private void removeIndependentEdgesInternal(List<Edge> edges, Context context, S
             int count;
 
             String deleteFromClosureQueryText, updateInClosureQueryText;
-            if (isH2() || isPostgreSQL() || isOracle()) {
+            // Can/must be unified with PG after H2 > 1.4.200 if no other issues emerge.
+            if (isH2()) {
+                // delete with join is not supported by H2
+                // and the "postgresql/oracle version" does not work for some reasons
+                deleteFromClosureQueryText = "delete from " + CLOSURE_TABLE_NAME + " cl " +
+                        "where exists (" +
+                        "select 0 from " + deltaTempTableName + " delta " +
+                        "where cl.descendant_oid = delta.descendant_oid and cl.ancestor_oid = delta.ancestor_oid and cl.val = delta.val)";
+                updateInClosureQueryText = "update " + CLOSURE_TABLE_NAME + " " +
+                        "set val = val - (select val from " + deltaTempTableName + " td " +
+                        "where td.descendant_oid=" + CLOSURE_TABLE_NAME + ".descendant_oid and td.ancestor_oid=" + CLOSURE_TABLE_NAME + ".ancestor_oid) " +
+                        "where (descendant_oid, ancestor_oid) in (select (descendant_oid, ancestor_oid) from " + deltaTempTableName + ")";
+            } else if (isPostgreSQL() || isOracle()) {
                 deleteFromClosureQueryText = "delete from " + CLOSURE_TABLE_NAME + " " +
                         "where (descendant_oid, ancestor_oid, val) in " +
                         "(select descendant_oid, ancestor_oid, val from " + deltaTempTableName + ")";

diff --git a/...tz-impl/src/main/java/com/evolveum/midpoint/task/quartzimpl/TaskManagerConfiguration.java b/...tz-impl/src/main/java/com/evolveum/midpoint/task/quartzimpl/TaskManagerConfiguration.java
@@ -447,7 +447,7 @@ void setJdbcJobStoreInformation(MidpointConfiguration masterConfig, SqlRepositor
             if (explicitJdbcUrl == null) {
                 if (sqlConfig != null) {
                     if (sqlConfig.isEmbedded()) {
-                        jdbcUrl = defaultJdbcUrlPrefix + "-quartz;DB_CLOSE_ON_EXIT=FALSE";
+                        jdbcUrl = defaultJdbcUrlPrefix + "-quartz;MVCC=TRUE;DB_CLOSE_ON_EXIT=FALSE";
                     } else {
                         jdbcUrl = sqlConfig.getJdbcUrl();
                     }

diff --git a/...sting/story/TestConsistencyMechanism.java → ...consistency/TestConsistencyMechanism.java b/...sting/story/TestConsistencyMechanism.java → ...consistency/TestConsistencyMechanism.java
@@ -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.testing.story;
+package com.evolveum.midpoint.testing.story.consistency;
 
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.testng.AssertJUnit.*;
@@ -82,7 +82,7 @@
 @DirtiesContext(classMode = ClassMode.AFTER_CLASS)
 public class TestConsistencyMechanism extends AbstractModelIntegrationTest {
 
-    private static final String TEST_DIR = "src/test/resources/consistency/";
+    protected static final String TEST_DIR = "src/test/resources/consistency/";
 
     private static final String SYSTEM_CONFIGURATION_FILENAME = TEST_DIR + "system-configuration.xml";
 
@@ -94,7 +94,7 @@ public class TestConsistencyMechanism extends AbstractModelIntegrationTest {
 
     private static final String SAMPLE_CONFIGURATION_OBJECT_FILENAME = TEST_DIR + "sample-configuration-object.xml";
 
-    private static final String RESOURCE_OPENDJ_FILENAME = TEST_DIR + "resource-opendj.xml";
+    private static final File RESOURCE_OPENDJ_FILE = new File(TEST_DIR, "resource-opendj.xml");
     private static final String RESOURCE_OPENDJ_OID = "ef2bc95b-76e0-59e2-86d6-3d4f02d3ffff";
     private static final String RESOURCE_OPENDJ_NS = "http://midpoint.evolveum.com/xml/ns/public/resource/instance-3";
     private static final QName RESOURCE_OPENDJ_ACCOUNT_OBJECTCLASS = new QName(RESOURCE_OPENDJ_NS, "inetOrgPerson");
@@ -136,7 +136,7 @@ public class TestConsistencyMechanism extends AbstractModelIntegrationTest {
 
     private static final String USER_ELAINE_FILENAME = TEST_DIR + "user-elaine.xml";
     private static final File USER_ELAINE_FILE = new File(USER_ELAINE_FILENAME);
-    private static final String USER_ELAINE_OID = "c0c010c0-d34d-b33f-f00d-111111116666";
+    protected static final String USER_ELAINE_OID = "c0c010c0-d34d-b33f-f00d-111111116666";
 
     private static final String USER_HERMAN_FILENAME = TEST_DIR + "user-herman.xml";
     private static final String USER_HERMAN_OID = "c0c010c0-d34d-b33f-f00d-111111119999";
@@ -186,7 +186,7 @@ public class TestConsistencyMechanism extends AbstractModelIntegrationTest {
 
     private static final String ACCOUNT_DENIELS_FILENAME = TEST_DIR + "account-deniels.xml";
     private static final File ACCOUNT_DENIELS_FILE = new File(ACCOUNT_DENIELS_FILENAME);
-    private static final String ACCOUNT_DENIELS_OID = "a0c010c0-d34d-b33f-f00d-111111111555";
+    protected static final String ACCOUNT_DENIELS_OID = "a0c010c0-d34d-b33f-f00d-111111111555";
     private static final String ACCOUNT_DENIELS_LDAP_UID = "deniels";
     private static final String ACCOUNT_DENIELS_LDAP_DN = "uid=" + ACCOUNT_DENIELS_LDAP_UID + "," + OPENDJ_PEOPLE_SUFFIX;
 
@@ -252,14 +252,18 @@ public void initSystem(Task initTask, OperationResult initResult) throws Excepti
 
         // Need to import instead of add, so the (dynamic) connector reference
         // will be resolved correctly
-        importObjectFromFile(RESOURCE_OPENDJ_FILENAME, initResult);
+        importObjectFromFile(getResourceFile(), initResult);
 
         repoAddObjectFromFile(SAMPLE_CONFIGURATION_OBJECT_FILENAME, initResult);
         repoAddObjectFromFile(USER_TEMPLATE_FILENAME, initResult);
 
         assumeAssignmentPolicy(AssignmentPolicyEnforcementType.POSITIVE);
     }
 
+    protected File getResourceFile() {
+        return RESOURCE_OPENDJ_FILE;
+    }
+
 //    @Override
 //    protected TracingProfileType getTestMethodTracingProfile() {
 //        return createModelAndProvisioningLoggingTracingProfile()
@@ -690,25 +694,11 @@ public void test130DeleteObjectNotFound() throws Exception {
         executeChanges(deleteDelta, null, task, parentResult);
         parentResult.computeStatus();
 
-        PrismObject<ShadowType> shadowRepo = getShadowRepo(ACCOUNT_GUYBRUSH_OID);
-        ShadowAsserter.forShadow(shadowRepo)
-                .assertTombstone()
-                .assertDead()
-                .assertIsNotExists();
-
-        clockForward("PT20M");
-
+        // THEN
         then();
-        provisioningService.refreshShadow(shadowRepo, null, task, parentResult);
+        checkTest130DeadShadow(task, parentResult);
 
-        try {
-            repositoryService.getObject(ShadowType.class, ACCOUNT_GUYBRUSH_OID, null, parentResult);
-            fail("Unexpected object found");
-        } catch (Exception ex) {
-            if (!(ex instanceof ObjectNotFoundException)) {
-                fail("Expected ObjectNotFoundException but got " + ex);
-            }
-        }
+        assertNoRepoShadow(ACCOUNT_GUYBRUSH_OID);
 
         clock.resetOverride();
 
@@ -719,6 +709,19 @@ public void test130DeleteObjectNotFound() throws Exception {
         repositoryService.deleteObject(UserType.class, USER_GUYBRUSH_OID, parentResult);
     }
 
+    protected void checkTest130DeadShadow(Task task, OperationResult parentResult) throws CommonException {
+        PrismObject<ShadowType> shadowRepo = getShadowRepo(ACCOUNT_GUYBRUSH_OID);
+        ShadowAsserter.forShadow(shadowRepo)
+                .assertTombstone()
+                .assertDead()
+                .assertIsNotExists();
+
+        clockForward("PT20M");
+
+        then();
+        provisioningService.refreshShadow(shadowRepo, null, task, parentResult);
+    }
+
     /**
      * Modify account not found => reaction: Delete account
      * <p>
@@ -756,23 +759,9 @@ public void test140ModifyObjectNotFoundLinkedAccount() throws Exception {
 
         // THEN
         then();
-        PrismObject<ShadowType> shadowAfter = getShadowRepo(ACCOUNT_GUYBRUSH_OID);
-        ShadowAsserter.forShadow(shadowAfter)
-                .assertTombstone()
-                .assertDead()
-                .assertIsNotExists();
-
-        clockForward("PT20M");
-        provisioningService.refreshShadow(shadowAfter, null, task, result);
+        checkTest140DeadShadow(task, result);
 
-        try {
-            repositoryService.getObject(ShadowType.class, ACCOUNT_GUYBRUSH_OID, null, result);
-            fail("Expected ObjectNotFound but did not get one.");
-        } catch (Exception ex) {
-            if (!(ex instanceof ObjectNotFoundException)) {
-                fail("Expected ObjectNotFoundException but got " + ex);
-            }
-        }
+        assertNoRepoShadow(ACCOUNT_GUYBRUSH_OID);
 
         clock.resetOverride();
         PrismObject<UserType> userAfter = getUser(USER_GUYBRUSH_OID);
@@ -782,6 +771,17 @@ public void test140ModifyObjectNotFoundLinkedAccount() throws Exception {
 //        repositoryService.deleteObject(UserType.class, USER_GUYBRUSH_OID, result);
     }
 
+    protected void checkTest140DeadShadow(Task task, OperationResult result) throws CommonException {
+        PrismObject<ShadowType> shadowAfter = getShadowRepo(ACCOUNT_GUYBRUSH_OID);
+        ShadowAsserter.forShadow(shadowAfter)
+                .assertTombstone()
+                .assertDead()
+                .assertIsNotExists();
+
+        clockForward("PT20M");
+        provisioningService.refreshShadow(shadowAfter, null, task, result);
+    }
+
     /**
      * Modify account not found => reaction: Re-create account, apply changes.
      * <p>
@@ -2576,7 +2576,17 @@ public void test800Reconciliation() throws Exception {
         assertAttribute(modifiedAccount, "givenName", "Jackkk");
         assertAttribute(modifiedAccount, "employeeNumber", "emp4321");
 
-        // check if the account was marked as dead during the reconciliation process
+        // check if the account was marked as dead (or deleted) during the reconciliation process
+        assert800DeadShadows();
+
+        accountOid = assertUserOneAccountRef(USER_JACKIE_OID);
+        ShadowType jack2Shadow = checkNormalizedShadowBasic(accountOid, "jack2", true, SelectorOptions.createCollection(GetOperationOptions.createDoNotDiscovery()), null, result);
+        assertAttribute(jack2Shadow, "givenName", "jackNew2a");
+        assertAttribute(jack2Shadow, "cn", "jackNew2a");
+
+    }
+
+    protected void assert800DeadShadows() throws CommonException {
         assertRepoShadow(ACCOUNT_DENIELS_OID)
                 .assertDead();
 
@@ -2611,11 +2621,6 @@ public void test800Reconciliation() throws Exception {
                 .delta()
                 .assertAdd();
 
-        accountOid = assertUserOneAccountRef(USER_JACKIE_OID);
-        ShadowType jack2Shadow = checkNormalizedShadowBasic(accountOid, "jack2", true, SelectorOptions.createCollection(GetOperationOptions.createDoNotDiscovery()), null, result);
-        assertAttribute(jack2Shadow, "givenName", "jackNew2a");
-        assertAttribute(jack2Shadow, "cn", "jackNew2a");
-
     }
 
     //MID-5844