Merge branch 'tmp/mid-8659'

infra/schema/src/main/java/com/evolveum/midpoint/schema/util/SystemConfigurationTypeUtil.java

@@ -85,6 +85,6 @@ public static boolean isAccessesMetadataEnabled(SystemConfigurationType sysconfi
         if (accessesMetadataEnabled == null) {
             return defaultValue;
         }
-        return Boolean.TRUE.equals(roleManagement.isAccessesMetadataEnabled());
+        return Boolean.TRUE.equals(accessesMetadataEnabled);
     }
 }

infra/schema/src/main/java/com/evolveum/midpoint/schema/util/cid/ContainerValueIdGenerator.java

@@ -13,7 +13,6 @@
 import com.evolveum.midpoint.prism.*;
 import com.evolveum.midpoint.prism.delta.ItemDelta;
 import com.evolveum.midpoint.prism.equivalence.EquivalenceStrategy;
-import com.evolveum.midpoint.util.exception.SchemaException;
 import com.evolveum.midpoint.util.logging.Trace;
 import com.evolveum.midpoint.util.logging.TraceManager;
 import com.evolveum.midpoint.xml.ns._public.common.common_3.ObjectType;
@@ -32,22 +31,22 @@ public class ContainerValueIdGenerator {
     private final Set<Long> overwrittenIds = new HashSet<>(); // ids of PCV overwritten by ADD
     private final List<PrismContainerValue<?>> pcvsWithoutId = new ArrayList<>();
 
-    private long maxUsedId = 0; // tracks max CID (set to duplicate CID if found)
+    private long maxUsedId = 0; // tracks max CID
 
     private int generated;
 
-    public <S extends ObjectType> ContainerValueIdGenerator(@NotNull PrismObject<S> object) {
+    public ContainerValueIdGenerator(@NotNull PrismObject<? extends ObjectType> object) {
         this.object = object;
     }
 
     /**
      * Method inserts IDs for prism container values without IDs and returns highest CID.
      * This directly changes the object provided as a parameter, if necessary.
      */
-    public long generateForNewObject() throws SchemaException {
+    public long generateForNewObject() {
         checkExistingContainers(object);
-        LOGGER.trace("Generating " + pcvsWithoutId.size() + " missing container IDs for "
-                + object.toDebugType() + "/" + object.getOid());
+        LOGGER.trace("Generating {} missing container IDs for {}/{}",
+                pcvsWithoutId.size(), object.toDebugType(), object.getOid());
         assignMissingContainerIds();
         return maxUsedId;
     }
@@ -56,21 +55,20 @@ public long generateForNewObject() throws SchemaException {
      * Initializes the generator for modify object and checks that no previous CIDs are missing.
      * This is a critical step before calling {@link #processModification}.
      */
-    public ContainerValueIdGenerator forModifyObject(long containerIdSeq) throws SchemaException {
+    public ContainerValueIdGenerator forModifyObject(long containerIdSeq) {
         checkExistingContainers(object);
         if (!pcvsWithoutId.isEmpty()) {
-            LOGGER.warn("Generating missing container IDs in previously persisted prism object "
-                    + object.toDebugType() + "/" + object.getOid() + " for container values: "
-                    + pcvsWithoutId);
+            LOGGER.warn("Generating missing container IDs in previously persisted prism object {}/{} for container values: {}",
+                    object.toDebugType(), object.getOid(), pcvsWithoutId);
             assignMissingContainerIds();
         }
         if (containerIdSeq <= maxUsedId) {
-            LOGGER.warn("Current CID sequence (" + containerIdSeq + ") is not above max used CID ("
-                    + maxUsedId + ") for " + object.toDebugType() + "/" + object.getOid()
-                    + ". CID sequence will be fixed, but it's suspicious!");
+            LOGGER.warn("Current CID sequence ({}) is not above max used CID ({}) for {}/{}. "
+                    + "CID sequence will be fixed, but it's suspicious!",
+                    containerIdSeq, maxUsedId, object.toDebugType(), object.getOid());
         } else {
             // TODO: Is this correct? containerIdSeq is used to reboot sequence of cids (eg. if
-            // cids are not in full object, but other items
+            //  cids are not in full object, but other items
             maxUsedId = containerIdSeq != 0 ? containerIdSeq - 1 : 0;
         }
         return this;
@@ -84,24 +82,20 @@ public ContainerValueIdGenerator forModifyObject(long containerIdSeq) throws Sch
      * Theoretically, the changes may affect the prism object after the fact, but if any cloning
      * is involved this may not be true, so preferably use this *before* applying the modification.
      */
-    public void processModification(ItemDelta<?, ?> modification) throws SchemaException {
+    public void processModification(ItemDelta<?, ?> modification) {
         if (modification.isReplace()) {
             freeIdsFromReplacedContainer(modification);
         }
         if (modification.isAdd()) {
             identifyReplacedContainers(modification);
         }
-        try {
-            processModificationValues(modification.getValuesToAdd());
-            processModificationValues(modification.getValuesToReplace());
-            // values to delete are irrelevant
-        } catch (DuplicateContainerIdException e) {
-            throw new SchemaException("CID " + maxUsedId
-                    + " is used repeatedly in the object: " + object);
-        }
+        processModificationValues(modification.getValuesToAdd());
+        processModificationValues(modification.getValuesToReplace());
+        // values to delete are irrelevant
         assignMissingContainerIds();
     }
 
+    /** Currently does nothing. Consider removing. */
     private void freeIdsFromReplacedContainer(ItemDelta<?, ?> modification) {
         ItemDefinition<?> definition = modification.getDefinition();
         // we check all containers, even single-value container can contain multi-value ones
@@ -143,12 +137,10 @@ private void identifyReplacedContainers(ItemDelta<?, ?> modification) {
     private void freeContainerIds(PrismContainer<?> container) {
     }
 
-    private void processModificationValues(Collection<? extends PrismValue> values)
-            throws SchemaException {
+    private void processModificationValues(Collection<? extends PrismValue> values) {
         if (values != null) {
             for (PrismValue prismValue : values) {
-                if (prismValue instanceof PrismContainerValue) {
-                    PrismContainerValue<?> pcv = (PrismContainerValue<?>) prismValue;
+                if (prismValue instanceof PrismContainerValue<?> pcv) {
                     // the top level value is not covered by checkExistingContainers()
                     // FIXME: How to process here?
                     if (pcv.getDefinition().isMultiValue()) {
@@ -165,19 +157,14 @@ private void processModificationValues(Collection<? extends PrismValue> values)
      * Checks the provided container (possibly the whole object) and finds values requiring CID.
      * This does NOT cover top-level PCV if provided as parameter.
      */
-    private void checkExistingContainers(Visitable object) throws SchemaException {
-        try {
-            object.accept(visitable -> {
-                if (visitable instanceof PrismContainer
-                        && !(visitable instanceof PrismObject)
-                        && ((PrismContainer<?>) visitable).getDefinition().isMultiValue()) {
-                    processContainer((PrismContainer<?>) visitable);
-                }
-            });
-        } catch (DuplicateContainerIdException e) {
-            throw new SchemaException(
-                    "CID " + maxUsedId + " is used repeatedly in the object: " + object);
-        }
+    private void checkExistingContainers(Visitable object) {
+        object.accept(visitable -> {
+            if (visitable instanceof PrismContainer
+                    && !(visitable instanceof PrismObject)
+                    && ((PrismContainer<?>) visitable).getDefinition().isMultiValue()) {
+                processContainer((PrismContainer<?>) visitable);
+            }
+        });
     }
 
     private void processContainer(PrismContainer<?> container) {
@@ -190,10 +177,7 @@ private void processContainer(PrismContainer<?> container) {
     private void processContainerValue(PrismContainerValue<?> val, Set<Long> usedIds) {
         Long cid = val.getId();
         if (cid != null) {
-            if (!usedIds.add(cid)) {
-                //maxUsedId = cid;
-                //throw DuplicateContainerIdException.INSTANCE;
-            }
+            usedIds.add(cid);
             maxUsedId = Math.max(maxUsedId, cid);
         } else {
             pcvsWithoutId.add(val);
@@ -209,7 +193,7 @@ private void assignMissingContainerIds() {
         pcvsWithoutId.clear();
     }
 
-    public long nextId() {
+    private long nextId() {
         maxUsedId++;
         return maxUsedId;
     }
@@ -225,13 +209,4 @@ public long lastUsedId() {
     public boolean isOverwrittenId(Long id) {
         return overwrittenIds.contains(id);
     }
-
-    // static single-value runtime exception to be thrown from lambdas (used in visitor)
-    private static class DuplicateContainerIdException extends RuntimeException {
-        static final DuplicateContainerIdException INSTANCE = new DuplicateContainerIdException();
-
-        private DuplicateContainerIdException() {
-            super(null, null, false, false);
-        }
-    }
 }

model/model-api/src/main/java/com/evolveum/midpoint/model/api/context/AssignmentPath.java

@@ -84,7 +84,7 @@ public interface AssignmentPath extends DebugDumpable, ShortDumpable, Cloneable,
 
     AssignmentPath cloneFirst(int n);
 
-    AssignmentPathType toAssignmentPathType(boolean includeAssignmentsContent);
+    AssignmentPathType toAssignmentPathBean(boolean includeAssignmentsContent);
 
     ExtensionType collectExtensions(int startAt) throws SchemaException;
 

model/model-api/src/main/java/com/evolveum/midpoint/model/api/context/AssignmentPathSegment.java

@@ -71,8 +71,7 @@ public interface AssignmentPathSegment extends DebugDumpable, ShortDumpable, Ser
      */
     boolean isDelegation();
 
-    @NotNull
-    AssignmentPathSegmentType toAssignmentPathSegmentType(boolean includeAssignmentsContent);
+    @NotNull AssignmentPathSegmentType toAssignmentPathSegmentBean(boolean includeAssignmentsContent);
 
     /**
      * Returns true if the path segment matches specified order constraints. All of them must match.

model/model-api/src/main/java/com/evolveum/midpoint/model/api/context/EvaluatedExclusionTrigger.java

@@ -137,7 +137,7 @@ public EvaluatedExclusionTriggerType toEvaluatedPolicyRuleTriggerBean(
         if (options.isFullStorageStrategy()) {
             rv.setConflictingObjectRef(ObjectTypeUtil.createObjectRef(conflictingTarget));
             rv.setConflictingObjectDisplayName(ObjectTypeUtil.getDisplayName(conflictingTarget));
-            rv.setConflictingObjectPath(conflictingPath.toAssignmentPathType(options.isIncludeAssignmentsContent()));
+            rv.setConflictingObjectPath(conflictingPath.toAssignmentPathBean(options.isIncludeAssignmentsContent()));
             if (options.isIncludeAssignmentsContent() && conflictingAssignment.getAssignment() != null) {
                 rv.setConflictingAssignment(conflictingAssignment.getAssignment().clone());
             }

model/model-impl/src/main/java/com/evolveum/midpoint/model/impl/controller/ModelInteractionServiceImpl.java

@@ -1652,7 +1652,8 @@ private boolean determineDeputyValidity(
                 // TODO some special mode for verification of the validity - we don't need complete calculation here!
                 EvaluatedAssignment assignment = assignmentEvaluator
                         .evaluate(
-                                assignmentIdi, PlusMinusZero.ZERO, false,
+                                assignmentIdi, null,
+                                PlusMinusZero.ZERO, false,
                                 potentialDeputyBean, potentialDeputy.toString(),
                                 AssignmentOrigin.inObject(embedded(assignmentBean)),
                                 task, result);

model/model-impl/src/main/java/com/evolveum/midpoint/model/impl/lens/AssignmentIdStore.java

@@ -0,0 +1,55 @@
+/*
+ * Copyright (C) 2010-2023 Evolveum and contributors
+ *
+ * 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.model.impl.lens;
+
+import com.evolveum.midpoint.model.impl.ModelBeans;
+import com.evolveum.midpoint.model.impl.lens.assignments.EvaluatedAssignmentImpl;
+import com.evolveum.midpoint.schema.result.OperationResult;
+import com.evolveum.midpoint.xml.ns._public.common.common_3.AssignmentType;
+
+import org.jetbrains.annotations.NotNull;
+
+import java.util.HashMap;
+import java.util.Map;
+
+/**
+ * When new assignments are being created (either as part of focus "add" or "modify" operation), we need to know their PCV IDs
+ * beforehand. Otherwise, we are not able to create final `roleMembershipRef` value metadata for them (as they contain complete
+ * assignment paths), resulting in multiple execution-stage audit events: first one with no-ID assignment paths in metadata,
+ * and second one with correct assignment paths (having IDs). See MID-8659.
+ *
+ * The solution lies in acquiring assignment PCV IDs in earlier stages of the processing (from the repository).
+ *
+ * However, we _do not_ store the IDs in the assignments themselves, as their are sometimes re-generated (e.g. by mappings),
+ * and the IDs could be easily lost. Hence, we store these IDs in {@link #generatedIdsMap} here instead. They are stored
+ * in {@link EvaluatedAssignmentImpl#externalAssignmentId} (and at other similar places), and provided to appropriate places
+ * during the computations; and, of course, implanted into deltas before the execution, so they are persistently stored
+ * in the repository.
+ *
+ * NOTE: {@link TemporaryContainerIdStore} addresses a similar problem. However, in that case, IDs are not really important,
+ * and they can be safely discarded when the delta is executed. (Unlike this case, where we need to keep them.)
+ */
+public class AssignmentIdStore {
+
+    /** The map keys are parent-less assignments, immutable (to avoid hashCode being inadvertently changed). */
+    @NotNull private final Map<AssignmentType, Long> generatedIdsMap = new HashMap<>();
+
+    public Long getKnownExternalId(AssignmentType assignment) {
+        return generatedIdsMap.get(assignment);
+    }
+
+    public void put(AssignmentType assignment, Long newId) {
+        AssignmentType cloned = assignment.clone();
+        cloned.freeze();
+        generatedIdsMap.put(cloned, newId);
+    }
+
+    public boolean isEmpty() {
+        return generatedIdsMap.isEmpty();
+    }
+}

model/model-impl/src/main/java/com/evolveum/midpoint/model/impl/lens/Clockwork.java

@@ -402,12 +402,13 @@ private void exitDefaultSearchExpressionEvaluatorCache() {
         //DefaultSearchExpressionEvaluatorCache.exitCache();
     }
 
-    public <F extends ObjectType> @NotNull HookOperationMode click(@NotNull LensContext<F> context,
-            @NotNull Task task, @NotNull OperationResult parentResult)
+    public <F extends ObjectType> @NotNull HookOperationMode click(
+            @NotNull LensContext<F> context,
+            @NotNull Task task, @NotNull OperationResult result)
             throws SchemaException, ExpressionEvaluationException, CommunicationException, SecurityViolationException,
             ConflictDetectedException, ConfigurationException, ObjectNotFoundException, PolicyViolationException,
             ObjectAlreadyExistsException {
         return new ClockworkClick<>(context, beans, task)
-                .click(parentResult);
+                .click(result);
     }
 }

model/model-impl/src/main/java/com/evolveum/midpoint/model/impl/lens/ElementState.java

@@ -592,13 +592,13 @@ public void swallowToSecondaryDelta(LensElementContext<?> context, ItemDelta<?,
 
     private boolean isDeltaAlreadyPresent(ItemDelta<?, ?> itemDelta) {
         // This gradual check (primary then secondary) is there to avoid the computation of the current delta
-        return !wasPrimaryDeltaExecuted && isDeltaPresentIn(itemDelta, primaryDelta) ||
-                isDeltaPresentIn(itemDelta, secondaryDelta);
+        return !wasPrimaryDeltaExecuted && isDeltaPresentIn(itemDelta, primaryDelta)
+                || isDeltaPresentIn(itemDelta, secondaryDelta);
     }
 
     private boolean isDeltaPresentIn(ItemDelta<?, ?> itemDelta, ObjectDelta<O> objectDelta) {
-        return objectDelta != null &&
-                objectDelta.containsModification(itemDelta, EquivalenceStrategy.DATA.exceptForValueMetadata());
+        return objectDelta != null
+                && objectDelta.containsModification(itemDelta, EquivalenceStrategy.DATA.exceptForValueMetadata());
     }
 
     /** Creates the delta if needed. */

model/model-impl/src/main/java/com/evolveum/midpoint/model/impl/lens/EvaluatedPolicyRuleImpl.java

@@ -435,7 +435,7 @@ public void addToEvaluatedPolicyRuleBeansInternal(
         if (options.isFullStorageStrategy()) {
             if (assignmentPath != null) {
                 bean.setAssignmentPath(
-                        assignmentPath.toAssignmentPathType(options.isIncludeAssignmentsContent()));
+                        assignmentPath.toAssignmentPathBean(options.isIncludeAssignmentsContent()));
             }
             ObjectType directOwner = computeDirectOwner();
             if (directOwner != null) {

model/model-impl/src/main/java/com/evolveum/midpoint/model/impl/lens/LensContext.java

@@ -2030,6 +2030,10 @@ boolean isProjectionRecomputationRequested() {
         return projectionContexts.stream().anyMatch(LensProjectionContext::isProjectionRecomputationRequested);
     }
 
+    public boolean areAccessesMetadataEnabled() {
+        return SystemConfigurationTypeUtil.isAccessesMetadataEnabled(getSystemConfigurationBean());
+    }
+
     public enum ExportType {
         MINIMAL, REDUCED, OPERATIONAL, TRACE
     }

model/model-impl/src/main/java/com/evolveum/midpoint/model/impl/lens/LensFocusContext.java

@@ -97,11 +97,19 @@ public class LensFocusContext<O extends ObjectType> extends LensElementContext<O
     // this is not to be serialized into XML, but let's not mark it as transient
     @NotNull private PathKeyedMap<ObjectTemplateItemDefinitionType> itemDefinitionsMap = new PathKeyedMap<>();
 
+    /**
+     * Manages assignment PCV IDs that are allocated in advance. Created on demand.
+     *
+     * We don't assume that we want to use this store after the context is serialized and then restored.
+     * Hence, it's safe to be transient.
+     */
+    private transient AssignmentIdStore assignmentIdStore;
+
     public LensFocusContext(Class<O> objectTypeClass, LensContext<O> lensContext) {
         super(objectTypeClass, lensContext);
     }
 
-    public LensFocusContext(ElementState<O> elementState, LensContext<O> lensContext) {
+    private LensFocusContext(ElementState<O> elementState, LensContext<O> lensContext) {
         super(elementState, lensContext);
     }
 
@@ -221,6 +229,11 @@ public boolean isAdd() {
         return ObjectDelta.isAdd(state.getPrimaryDelta());
     }
 
+    /** Different from {@link #isAdd()} just to have a clear meaning. */
+    public boolean isPrimaryAdd() {
+        return ObjectDelta.isAdd(state.getPrimaryDelta());
+    }
+
     public boolean isDeleted() {
         return deleted;
     }
@@ -508,4 +521,11 @@ void updateDeltasAfterExecution() {
     public PrismObject<O> getStateBeforeSimulatedOperation() {
         return getObjectOld();
     }
+
+    public @NotNull AssignmentIdStore getAssignmentIdStore() {
+        if (assignmentIdStore == null) {
+            assignmentIdStore = new AssignmentIdStore();
+        }
+        return assignmentIdStore;
+    }
 }

model/model-impl/src/main/java/com/evolveum/midpoint/model/impl/lens/LoginAssignmentCollector.java

@@ -142,6 +142,7 @@ private <AH extends AssignmentHolderType> Collection<EvaluatedAssignment> evalua
                     evaluatedAssignments.add(
                             assignmentEvaluator.evaluate(
                                     assignmentIdi,
+                                    null,
                                     PlusMinusZero.ZERO,
                                     false,
                                     focus,

model/model-impl/src/main/java/com/evolveum/midpoint/model/impl/lens/OperationalDataManager.java

@@ -125,10 +125,12 @@ public <T extends ObjectType, F extends ObjectType, AH extends AssignmentHolderT
 
         MetadataType currentMetadata = getCurrentMetadata(elementContext);
 
-        ItemDeltaCollectionsUtil.mergeAll(objectDelta.getModifications(),
+        ItemDeltaCollectionsUtil.mergeAll(
+                objectDelta.getModifications(),
                 createModifyMetadataDeltas(context, currentMetadata, ObjectType.F_METADATA, objectTypeClass, now, task));
 
-        ItemDeltaCollectionsUtil.mergeAll(objectDelta.getModifications(),
+        ItemDeltaCollectionsUtil.mergeAll(
+                objectDelta.getModifications(),
                 createModifyApprovalMetadataDeltas(objectTypeClass, context));
 
         if (AssignmentHolderType.class.isAssignableFrom(objectTypeClass)) {