Merge remote-tracking branch 'origin/master' into feature/adminlte3-u…

…pgrade
Evolveum · Apr 21, 2022 · 4d02eb6 · 4d02eb6
2 parents 1d69099 + c32beee
commit 4d02eb6
Show file tree

Hide file tree

Showing 11 changed files with 62 additions and 29 deletions.
diff --git a/...l-impl/src/main/java/com/evolveum/midpoint/model/impl/cleanup/CleanupActivityHandler.java b/...l-impl/src/main/java/com/evolveum/midpoint/model/impl/cleanup/CleanupActivityHandler.java
@@ -90,7 +90,7 @@ public void unregister() {
 
         ActivityStateDefinition<AbstractActivityWorkStateType> stateDef = ActivityStateDefinition.normal();
         children.add(EmbeddedActivity.create(
-                parentActivity.getDefinition().clone(),
+                parentActivity.getDefinition().cloneWithoutId(),
                 (context, result) ->
                         new CleanupPartialActivityRun<>(
                                 context, Part.AUDIT_RECORDS, CleanupPoliciesType::getAuditRecords,
@@ -101,7 +101,7 @@ public void unregister() {
                 parentActivity));
 
         children.add(EmbeddedActivity.create(
-                parentActivity.getDefinition().clone(),
+                parentActivity.getDefinition().cloneWithoutId(),
                 (context, result) ->
                         new CleanupPartialActivityRun<>(
                                 context, Part.CLOSED_TASKS, CleanupPoliciesType::getClosedTasks,
@@ -112,7 +112,7 @@ public void unregister() {
                 parentActivity));
 
         children.add(EmbeddedActivity.create(
-                parentActivity.getDefinition().clone(),
+                parentActivity.getDefinition().cloneWithoutId(),
                 (context, result) ->
                         new CleanupPartialActivityRun<>(
                                 context, Part.CLOSED_CASES, CleanupPoliciesType::getClosedCases,
@@ -123,7 +123,7 @@ public void unregister() {
                 parentActivity));
 
         children.add(EmbeddedActivity.create(
-                parentActivity.getDefinition().clone(),
+                parentActivity.getDefinition().cloneWithoutId(),
                 (context, result) ->
                         new CleanupPartialActivityRun<>(
                                 context, Part.DEAD_NODES, CleanupPoliciesType::getDeadNodes,
@@ -134,7 +134,7 @@ public void unregister() {
                 parentActivity));
 
         children.add(EmbeddedActivity.create(
-                parentActivity.getDefinition().clone(),
+                parentActivity.getDefinition().cloneWithoutId(),
                 (context, result) ->
                         new CleanupPartialActivityRun<>(
                                 context, Part.OUTPUT_REPORTS, CleanupPoliciesType::getOutputReports,
@@ -145,7 +145,7 @@ public void unregister() {
                 parentActivity));
 
         children.add(EmbeddedActivity.create(
-                parentActivity.getDefinition().clone(),
+                parentActivity.getDefinition().cloneWithoutId(),
                 (context, result) ->
                         new CleanupPartialActivityRun<>(
                                 context, Part.CLOSED_CERTIFICATION_CAMPAIGNS, CleanupPoliciesType::getClosedCertificationCampaigns,

diff --git a/...java/com/evolveum/midpoint/model/impl/sync/tasks/recon/ReconciliationActivityHandler.java b/...java/com/evolveum/midpoint/model/impl/sync/tasks/recon/ReconciliationActivityHandler.java
@@ -76,7 +76,7 @@ public void unregister() {
             Activity<ReconciliationWorkDefinition, ReconciliationActivityHandler> parentActivity) {
         ArrayList<Activity<?, ?>> children = new ArrayList<>();
         children.add(EmbeddedActivity.create(
-                parentActivity.getDefinition().clone(),
+                parentActivity.getDefinition().cloneWithoutId(),
                 (context, result) -> new OperationCompletionActivityRun(context,
                         "Reconciliation (operation completion)"),
                 null,
@@ -100,15 +100,15 @@ public void unregister() {
                 ActivityStateDefinition.normal(),
                 parentActivity));
         children.add(EmbeddedActivity.create(
-                parentActivity.getDefinition().clone(),
+                parentActivity.getDefinition().cloneWithoutId(),
                 (context, result) -> new ResourceObjectsReconciliationActivityRun(context,
                         "Reconciliation (on resource)" + modeSuffix(context)),
                 this::beforeResourceObjectsReconciliation,
                 (i) -> ModelPublicConstants.RECONCILIATION_RESOURCE_OBJECTS_ID,
                 ActivityStateDefinition.normal(),
                 parentActivity));
         children.add(EmbeddedActivity.create(
-                parentActivity.getDefinition().clone(),
+                parentActivity.getDefinition().cloneWithoutId(),
                 (context, result) -> new RemainingShadowsActivityRun(context,
                         "Reconciliation (remaining shadows)" + modeSuffix(context)),
                 null,
@@ -120,7 +120,7 @@ public void unregister() {
 
     private ActivityDefinition<ReconciliationWorkDefinition> createPreviewDefinition(
             @NotNull ActivityDefinition<ReconciliationWorkDefinition> original) {
-        ActivityDefinition<ReconciliationWorkDefinition> clone = original.clone();
+        ActivityDefinition<ReconciliationWorkDefinition> clone = original.cloneWithoutId();
         clone.getWorkDefinition().setExecutionMode(ExecutionModeType.PREVIEW);
         clone.getControlFlowDefinition().setSkip();
         return clone;

diff --git a/...java/com/evolveum/midpoint/model/impl/tasks/scanner/FocusValidityScanActivityHandler.java b/...java/com/evolveum/midpoint/model/impl/tasks/scanner/FocusValidityScanActivityHandler.java
@@ -63,7 +63,7 @@ public void unregister() {
         switch (queryStyle) {
             case SINGLE_QUERY:
                 children.add(EmbeddedActivity.create(
-                        parentActivity.getDefinition().clone(),
+                        parentActivity.getDefinition().cloneWithoutId(),
                         (context, result) -> new FocusValidityScanPartialRun(context, COMBINED),
                         null,
                         (i) -> ModelPublicConstants.FOCUS_VALIDITY_SCAN_FULL_ID,
@@ -72,14 +72,14 @@ public void unregister() {
                 break;
             case SEPARATE_OBJECT_AND_ASSIGNMENT_QUERIES:
                 children.add(EmbeddedActivity.create(
-                        parentActivity.getDefinition().clone(),
+                        parentActivity.getDefinition().cloneWithoutId(),
                         (context, result) -> new FocusValidityScanPartialRun(context, OBJECTS),
                         null,
                         (i) -> ModelPublicConstants.FOCUS_VALIDITY_SCAN_OBJECTS_ID,
                         stateDef,
                         parentActivity));
                 children.add(EmbeddedActivity.create(
-                        parentActivity.getDefinition().clone(),
+                        parentActivity.getDefinition().cloneWithoutId(),
                         (context, result) -> new FocusValidityScanPartialRun(context, ASSIGNMENTS),
                         null,
                         (i) -> ModelPublicConstants.FOCUS_VALIDITY_SCAN_ASSIGNMENTS_ID,

diff --git a/model/model-impl/src/test/resources/expr/expression-linkref-filter-defaults.xml b/model/model-impl/src/test/resources/expr/expression-linkref-filter-defaults.xml
@@ -7,9 +7,6 @@
   -->
 
 <filter xmlns="http://prism.evolveum.com/xml/ns/public/query-3" xmlns:c='http://midpoint.evolveum.com/xml/ns/public/common/common-3'>
-    <text>
-
-    </text>
     <ref>
         <path>linkRef</path>
         <c:expression>

diff --git a/model/model-intest/src/test/resources/tasks/recon/task-reconciliation.xml b/model/model-intest/src/test/resources/tasks/recon/task-reconciliation.xml
@@ -18,6 +18,7 @@
     <executionState>closed</executionState> <!-- run by the test code -->
 
     <activity>
+        <identifier>my-reconciliation</identifier>
         <work>
             <reconciliation>
                 <resourceObjects>

diff --git a/...va/com/evolveum/midpoint/report/impl/activity/DistributedReportExportActivityHandler.java b/...va/com/evolveum/midpoint/report/impl/activity/DistributedReportExportActivityHandler.java
@@ -88,14 +88,14 @@ public void unregister() {
             Activity<DistributedReportExportWorkDefinition, DistributedReportExportActivityHandler> parentActivity) {
         ArrayList<Activity<?, ?>> children = new ArrayList<>();
         children.add(EmbeddedActivity.create(
-                parentActivity.getDefinition().clone(),
+                parentActivity.getDefinition().cloneWithoutId(),
                 (context, result) -> new ReportDataCreationActivityRun(context),
                 this::createEmptyAggregatedDataObject,
                 (i) -> "data-creation",
                 ActivityStateDefinition.normal(),
                 parentActivity));
         children.add(EmbeddedActivity.create(
-                parentActivity.getDefinition().clone(),
+                parentActivity.getDefinition().cloneWithoutId(),
                 (context, result) -> new ReportDataAggregationActivityRun(context),
                 null,
                 (i) -> "data-aggregation",

diff --git a/repo/repo-common/src/main/java/com/evolveum/midpoint/repo/common/activity/Activity.java b/repo/repo-common/src/main/java/com/evolveum/midpoint/repo/common/activity/Activity.java
@@ -151,6 +151,12 @@ public void setLocalRoot(boolean localRoot) {
         }
     }
 
+    private @NotNull Map<String, Activity<?, ?>> getChildrenMapCopy() {
+        synchronized (childrenMap) {
+            return new HashMap<>(childrenMap);
+        }
+    }
+
     public @NotNull List<Activity<?, ?>> getChildrenCopyExceptSkipped() {
         synchronized (childrenMap) {
             return childrenMap.values().stream()
@@ -168,7 +174,8 @@ public String debugDump(int indent) {
         DebugUtil.debugDumpWithLabelLn(sb, "parent", String.valueOf(getParent()), indent + 1);
         DebugUtil.debugDumpWithLabelLn(sb, "path", String.valueOf(getPath()), indent + 1);
         DebugUtil.debugDumpWithLabelLn(sb, "local path", String.valueOf(getLocalPath()), indent + 1);
-        DebugUtil.debugDumpWithLabel(sb, "children (initialized=" + childrenMapInitialized + ")", childrenMap, indent + 1);
+        DebugUtil.debugDumpWithLabel(sb, "children (initialized=" + childrenMapInitialized + ")",
+                getChildrenMapCopy(), indent + 1);
         return sb.toString();
     }
 
@@ -248,24 +255,34 @@ private TaskRoleType getRoleOfTask(Task activityTask) {
             return child;
         } else {
             throw new IllegalArgumentException("Child with identifier " + identifier + " was not found among children of "
-                    + this + ". Known children are: " + childrenMap.keySet());
+                    + this + ". Known children are: " + getChildrenMapCopy().keySet());
         }
     }
 
     public void initializeChildrenMapIfNeeded() throws SchemaException {
-        if (!childrenMapInitialized) {
-            assert childrenMap.isEmpty();
-            createChildren();
-            childrenMapInitialized = true;
+        synchronized (childrenMap) { // just for sure
+            if (!childrenMapInitialized) {
+                assert childrenMap.isEmpty();
+                createChildren();
+                childrenMapInitialized = true;
+            }
         }
     }
 
+    /** Creates children in {@link #childrenMap}. The caller must hold the lock on {@link #childrenMap}. */
     private void createChildren() throws SchemaException {
         ArrayList<Activity<?, ?>> childrenList = getHandler().createChildActivities(this);
         setupIdentifiers(childrenList);
         tailorChildren(childrenList);
         setupIdentifiers(childrenList);
-        childrenList.forEach(child -> childrenMap.put(child.getIdentifier(), child));
+        childrenList.forEach(this::addChild);
+    }
+
+    private void addChild(@NotNull Activity<?, ?> child) {
+        var previous = childrenMap.put(child.getIdentifier(), child);
+        stateCheck(previous == null,
+                "Multiple child activities with the same identifier: %s (%s, %s)",
+                child.getIdentifier(), child, previous);
     }
 
     private void setupIdentifiers(List<Activity<?, ?>> childrenList) {
@@ -389,7 +406,7 @@ void applySubtaskTailoring(@NotNull ActivitySubtaskDefinitionType subtaskSpecifi
 
     public void accept(@NotNull ActivityVisitor visitor) {
         visitor.visit(this);
-        childrenMap.values()
+        getChildrenCopy()
                 .forEach(child -> child.accept(visitor));
     }
 

diff --git a/...epo-common/src/main/java/com/evolveum/midpoint/repo/common/activity/EmbeddedActivity.java b/...epo-common/src/main/java/com/evolveum/midpoint/repo/common/activity/EmbeddedActivity.java
@@ -49,7 +49,8 @@ private EmbeddedActivity(@NotNull ActivityDefinition<WD> definition, @NotNull Ac
     /**
      * Creates an embedded activity.
      *
-     * @param definition Definition to be used. Should be freely modifiable (typically cloned)!
+     * @param definition Definition to be used. Should be freely modifiable (typically cloned,
+     * e.g. via {@link ActivityDefinition#cloneWithoutId()})!
      */
     public static <WD extends WorkDefinition, AH extends ActivityHandler<WD, AH>> EmbeddedActivity<WD, AH> create(
             @NotNull ActivityDefinition<WD> definition,

diff --git a/...c/main/java/com/evolveum/midpoint/repo/common/activity/definition/ActivityDefinition.java b/...c/main/java/com/evolveum/midpoint/repo/common/activity/definition/ActivityDefinition.java
@@ -246,10 +246,25 @@ private void applyExecutionModeTailoring(@NotNull ActivityTailoringType tailorin
 
     /**
      * Does the deep clone. The goal is to be able to modify cloned definition freely.
+     *
+     * BEWARE: Do not use this method to create a definition clone to be used for child activities.
+     * Use {@link #cloneWithoutId()} instead. See MID-7894.
      */
     @SuppressWarnings({ "MethodDoesntCallSuperMethod" })
     @Override
     public ActivityDefinition<WD> clone() {
+        return cloneInternal(explicitlyDefinedIdentifier);
+    }
+
+    /**
+     * As {@link #clone()} but discards the {@link #explicitlyDefinedIdentifier} value.
+     */
+    public ActivityDefinition<WD> cloneWithoutId() {
+        return cloneInternal(null);
+    }
+
+    @NotNull
+    private ActivityDefinition<WD> cloneInternal(String explicitlyDefinedIdentifier) {
         //noinspection unchecked
         return new ActivityDefinition<>(
                 explicitlyDefinedIdentifier,

diff --git a/.../evolveum/midpoint/repo/common/tasks/handlers/composite/CompositeMockActivityHandler.java b/.../evolveum/midpoint/repo/common/tasks/handlers/composite/CompositeMockActivityHandler.java
@@ -79,7 +79,7 @@ public void unregister() {
         ArrayList<Activity<?, ?>> children = new ArrayList<>();
         if (workDefinition.isOpeningEnabled()) {
             children.add(EmbeddedActivity.create(
-                    parentActivity.getDefinition().clone(),
+                    parentActivity.getDefinition().cloneWithoutId(),
                     (context, result) -> new MockOpeningActivityRun(context),
                     this::runBeforeExecution,
                     (i) -> "opening",
@@ -88,7 +88,7 @@ public void unregister() {
         }
         if (workDefinition.isClosingEnabled()) {
             children.add(EmbeddedActivity.create(
-                    parentActivity.getDefinition().clone(),
+                    parentActivity.getDefinition().cloneWithoutId(),
                     (context, result) -> new MockClosingActivityRun(context),
                     this::runBeforeExecution,
                     (i) -> "closing",

diff --git a/repo/repo-sqale/src/main/java/com/evolveum/midpoint/repo/sqale/audit/SqaleAuditService.java b/repo/repo-sqale/src/main/java/com/evolveum/midpoint/repo/sqale/audit/SqaleAuditService.java
@@ -788,6 +788,8 @@ private ObjectFilter iterativeSearchCondition(
                 // "By default, null values sort as if larger than any non-null value; that is,
                 // NULLS FIRST is the default for DESC order, and NULLS LAST otherwise."
             } else {
+                // IMPORTANT: There is no fix for poly-strings here (MID-7860) like in SqaleRepositoryService.
+                // Although some poly-strings are stored in deltas and refs, they are never used for searched.
                 S_MatchingRuleEntry matchingRuleEntry =
                         asc ? filter.gt(item.getRealValue()) : filter.lt(item.getRealValue());
                 filter = matchingRuleEntry.or()