Deduplicate SequenceHelper code

Also: 1. Renamed SequenceHelper to SequenceUtil (no instantiation is there). 2. Other minor code improvements.
Evolveum · Nov 22, 2022 · 34d30b4 · 34d30b4
1 parent 9173f33
commit 34d30b4
Show file tree

Hide file tree

Showing 11 changed files with 68 additions and 86 deletions.
diff --git a/...lveum/midpoint/common/SequenceHelper.java → ...volveum/midpoint/common/SequenceUtil.java b/...lveum/midpoint/common/SequenceHelper.java → ...volveum/midpoint/common/SequenceUtil.java
@@ -12,29 +12,29 @@
 import com.evolveum.midpoint.util.logging.TraceManager;
 import com.evolveum.midpoint.xml.ns._public.common.common_3.SequenceType;
 
+import java.util.List;
+import java.util.Objects;
+
 /**
  * Created by Viliam Repan (lazyman).
  */
-public class SequenceHelper {
+public class SequenceUtil {
 
-    private static final Trace LOGGER = TraceManager.getTrace(SequenceHelper.class);
+    private static final Trace LOGGER = TraceManager.getTrace(SequenceUtil.class);
 
     public static long advanceSequence(SequenceType sequence) {
-        long returnValue;
-        if (!sequence.getUnusedValues().isEmpty()) {
-            returnValue = sequence.getUnusedValues().remove(0);
+        List<Long> unusedValues = sequence.getUnusedValues();
+        if (!unusedValues.isEmpty()) {
+            return unusedValues.remove(0);
         } else {
-            returnValue = advanceSequence(sequence, sequence.getOid());
+            return reallyAdvanceSequence(sequence);
         }
-
-        return returnValue;
     }
 
-    private static long advanceSequence(SequenceType sequence, String oid) {
+    private static long reallyAdvanceSequence(SequenceType sequence) {
         long returnValue;
-        long counter = sequence.getCounter() != null ? sequence.getCounter() : 0L;
-        long maxCounter = sequence.getMaxCounter() != null
-                ? sequence.getMaxCounter() : Long.MAX_VALUE;
+        long counter = Objects.requireNonNullElse(sequence.getCounter(), 0L);
+        long maxCounter = Objects.requireNonNullElse(sequence.getMaxCounter(), Long.MAX_VALUE);
         boolean allowRewind = Boolean.TRUE.equals(sequence.isAllowRewind());
 
         if (counter < maxCounter) {
@@ -47,15 +47,16 @@ private static long advanceSequence(SequenceType sequence, String oid) {
             } else {
                 sequence.setCounter(counter + 1); // will produce exception during next run
             }
-        } else { // i.e. counter > maxCounter
+        } else {
+            assert counter > maxCounter;
             if (allowRewind) { // shouldn't occur but...
-                LOGGER.warn("Sequence {} overflown with allowRewind set to true. Rewinding.", oid);
+                LOGGER.warn("Sequence {} overflown with allowRewind set to true. Rewinding.", sequence.getOid());
                 returnValue = 0;
                 sequence.setCounter(1L);
             } else {
-                throw new SystemException("No (next) value available from sequence " + oid
-                        + ". Current counter = " + sequence.getCounter()
-                        + ", max value = " + sequence.getMaxCounter());
+                throw new SystemException(
+                        String.format("No (next) value available from sequence %s. Current counter = %d, max value = %d.",
+                                sequence.getOid(), sequence.getCounter(), sequence.getMaxCounter()));
             }
         }
         return returnValue;

diff --git a/infra/schema/src/main/java/com/evolveum/midpoint/schema/TaskExecutionMode.java b/infra/schema/src/main/java/com/evolveum/midpoint/schema/TaskExecutionMode.java
@@ -36,10 +36,24 @@ private TaskExecutionMode(String name, boolean persistent, boolean productionCon
         this.productionConfiguration = productionConfiguration;
     }
 
+    /** Should the effects of this task be persistent or not? The latter means "simulation", "preview", etc. */
     public boolean isPersistent() {
         return persistent;
     }
 
+    /**
+     * What configuration should the actions take into account? Production or "development" one?
+     *
+     * - Production usually means `active` and `deprecated` lifecycle states.
+     * - Development usually means `active` and `proposed` states.
+     *
+     * However, in the future we may provide more customization options here (e.g. explicit enumeration of livecycle states
+     * to use, or even a set of specific deltas to apply).
+     *
+     * If {@link #persistent} is `true` then {@link #productionConfiguration} should be `true` as well.
+     *
+     * See https://docs.evolveum.com/midpoint/devel/design/simulations/ for more information.
+     */
     public boolean isProductionConfiguration() {
         return productionConfiguration;
     }

diff --git a/model/model-api/src/main/java/com/evolveum/midpoint/model/api/ModelExecuteOptions.java b/model/model-api/src/main/java/com/evolveum/midpoint/model/api/ModelExecuteOptions.java
@@ -25,6 +25,8 @@
 import com.evolveum.midpoint.util.exception.SchemaException;
 import com.evolveum.midpoint.xml.ns._public.common.common_3.*;
 
+import org.jetbrains.annotations.Nullable;
+
 /**
  * Options for execution of Model operations. These options influence the way how the operations are executed.
  * The options are not mandatory. All options have reasonable default values. They may not be specified at all.
@@ -420,6 +422,15 @@ public SimulationOptionsType getSimulationOptions() {
         return content.getSimulation();
     }
 
+    public static boolean isAdvanceSequenceSafe(@Nullable ModelExecuteOptions options) {
+        if (options == null) {
+            return false;
+        }
+        SimulationOptionsType simulationOptions = options.getSimulationOptions();
+        return simulationOptions != null
+                && simulationOptions.getSequence() == SimulationOptionType.SAFE;
+    }
+
     public ModelExecuteOptions simulationOptions(SimulationOptionsType options) {
         content.setSimulation(options);
         return this;

diff --git a/...rc/main/java/com/evolveum/midpoint/model/impl/controller/ModelInteractionServiceImpl.java b/...rc/main/java/com/evolveum/midpoint/model/impl/controller/ModelInteractionServiceImpl.java
@@ -1319,7 +1319,9 @@ private <T, O extends ObjectType> boolean validateValue(PrismObject<O> object, V
 
         for (String newValue : valuesToValidate) {
             OperationResult result = parentResult.createSubresult(OPERATION_VALIDATE_VALUE + ".value");
-            if (path != null) {result.addParam("path", path.toString());}
+            if (path != null) {
+                result.addParam("path", path.toString());
+            }
             result.addParam("valueToValidate", newValue);
 
             ObjectValuePolicyEvaluator.Builder evaluatorBuilder = new ObjectValuePolicyEvaluator.Builder()
@@ -1742,8 +1744,8 @@ public ExecuteCredentialResetResponseType executeCredentialsReset(PrismObject<Us
         try {
             Collection<ObjectDeltaOperation<? extends ObjectType>> result = modelService.executeChanges(
                     MiscUtil.createCollection(userDelta), ModelExecuteOptions.create().raw(), task, parentResult);
-        } catch (ObjectNotFoundException | SchemaException | CommunicationException | ConfigurationException
-                | SecurityViolationException | ExpressionEvaluationException | ObjectAlreadyExistsException |
+        } catch (ObjectNotFoundException | SchemaException | CommunicationException | ConfigurationException |
+                SecurityViolationException | ExpressionEvaluationException | ObjectAlreadyExistsException |
                 PolicyViolationException e) {
             response.message(LocalizationUtil.createForFallbackMessage("Failed to reset credential: " + e.getMessage()));
             throw e;
@@ -2050,7 +2052,7 @@ public void processObjectsFromCollection(CollectionRefSpecificationType collecti
                     query, options, task, result);
             processContainerByHandler(auditRecords, handler);
         } else if (ObjectType.class.isAssignableFrom(type)) {
-            ResultHandler<ObjectType> resultHandler = (value, operationResult) -> handler.test((PrismContainer) value);
+            ResultHandler<ObjectType> resultHandler = (value, operationResult) -> handler.test(value);
             checkOrdering(query, ObjectType.F_NAME);
             modelService.searchObjectsIterative((Class<ObjectType>) type, query, resultHandler, options, task, result);
         } else {

diff --git a/...c/main/java/com/evolveum/midpoint/model/impl/expr/SequentialValueExpressionEvaluator.java b/...c/main/java/com/evolveum/midpoint/model/impl/expr/SequentialValueExpressionEvaluator.java
@@ -6,13 +6,11 @@
  */
 package com.evolveum.midpoint.model.impl.expr;
 
-import java.util.Collection;
-import java.util.Collections;
 import javax.xml.namespace.QName;
 
 import org.jetbrains.annotations.NotNull;
 
-import com.evolveum.midpoint.common.SequenceHelper;
+import com.evolveum.midpoint.common.SequenceUtil;
 import com.evolveum.midpoint.model.api.ModelExecuteOptions;
 import com.evolveum.midpoint.model.api.context.ModelContext;
 import com.evolveum.midpoint.model.common.expression.ModelExpressionThreadLocalHolder;
@@ -24,8 +22,6 @@
 import com.evolveum.midpoint.repo.common.expression.ExpressionEvaluationContext;
 import com.evolveum.midpoint.repo.common.expression.ExpressionUtil;
 import com.evolveum.midpoint.repo.common.expression.evaluator.AbstractExpressionEvaluator;
-import com.evolveum.midpoint.schema.GetOperationOptions;
-import com.evolveum.midpoint.schema.SelectorOptions;
 import com.evolveum.midpoint.schema.result.OperationResult;
 import com.evolveum.midpoint.util.exception.ExpressionEvaluationException;
 import com.evolveum.midpoint.util.exception.ObjectNotFoundException;
@@ -86,9 +82,11 @@ static long getSequenceCounterValue(String sequenceOid, RepositoryService reposi
             if (!isAdvanceSequenceSafe()) {
                 freshValue = repositoryService.advanceSequence(sequenceOid, result);
             } else {
-                Collection<SelectorOptions<GetOperationOptions>> options = Collections.emptyList();
-                SequenceType seq = repositoryService.getObject(SequenceType.class, sequenceOid, options, result).asObjectable();
-                freshValue = SequenceHelper.advanceSequence(seq);
+                SequenceType seq =
+                        repositoryService
+                                .getObject(SequenceType.class, sequenceOid, null, result)
+                                .asObjectable();
+                freshValue = SequenceUtil.advanceSequence(seq);
             }
 
             ctx.setSequenceCounter(sequenceOid, freshValue);
@@ -98,23 +96,13 @@ static long getSequenceCounterValue(String sequenceOid, RepositoryService reposi
     }
 
     private static boolean isAdvanceSequenceSafe() {
-        return isAdvanceSequenceSafe(ModelExpressionThreadLocalHolder.getLensContextRequired());
+        return isAdvanceSequenceSafe(
+                ModelExpressionThreadLocalHolder.getLensContextRequired());
     }
 
-    public static boolean isAdvanceSequenceSafe(ModelContext context) {
-        boolean isAdvanceSequenceSafe = false;
-
-        ModelExecuteOptions options = context.getOptions();
-        if (options == null || options.getSimulationOptions() == null) {
-            return isAdvanceSequenceSafe;
-        }
-
-        SimulationOptionsType simulation = options.getSimulationOptions();
-        if (simulation.getSequence() == null) {
-            return isAdvanceSequenceSafe;
-        }
-
-        return SimulationOptionType.SAFE.equals(simulation.getSequence());
+    public static boolean isAdvanceSequenceSafe(ModelContext<?> context) {
+        return ModelExecuteOptions.isAdvanceSequenceSafe(
+                context.getOptions());
     }
 
     @NotNull

diff --git a/model/model-impl/src/main/java/com/evolveum/midpoint/model/impl/util/ModelImplUtils.java b/model/model-impl/src/main/java/com/evolveum/midpoint/model/impl/util/ModelImplUtils.java
@@ -112,7 +112,7 @@ public static void recordException(OperationResult result, Throwable e) {
         recordException(result, e.getMessage(), e);
     }
 
-    public static void recordException(OperationResult result, String message, Throwable e) {
+    private static void recordException(OperationResult result, String message, Throwable e) {
         // Do not log at ERROR level. This is too harsh. Especially in object not found case.
         // What model considers an error may be just a normal situation for the code is using model API.
         // If this is really an error then it should be logged by the invoking code.

diff --git a/...com/evolveum/midpoint/provisioning/impl/resourceobjects/ResourceObjectCountOperation.java b/...com/evolveum/midpoint/provisioning/impl/resourceobjects/ResourceObjectCountOperation.java
@@ -9,9 +9,6 @@
 
 import java.util.concurrent.atomic.AtomicInteger;
 
-import com.evolveum.midpoint.schema.internals.InternalCounters;
-import com.evolveum.midpoint.schema.internals.InternalMonitor;
-
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 

diff --git a/...n/java/com/evolveum/midpoint/provisioning/impl/resources/ResourceCompletionOperation.java b/...n/java/com/evolveum/midpoint/provisioning/impl/resources/ResourceCompletionOperation.java
@@ -191,9 +191,7 @@ private void applyConnectorSchemaAndExpressions() throws StopException {
 
         // Now we need to re-read the resource from the repository and re-apply the schemas. This ensures that we will
         // cache the correct version and that we avoid race conditions, etc.
-        ResourceType reloaded =
-                beans.resourceManager
-                        .readResourceFromRepository(resource.getOid(), result);
+        ResourceType reloaded = beans.resourceManager.readResourceFromRepository(resource.getOid(), result);
 
         expand(reloaded);
 

diff --git a/...n/java/com/evolveum/midpoint/provisioning/impl/shadows/errors/SchemaExceptionHandler.java b/...n/java/com/evolveum/midpoint/provisioning/impl/shadows/errors/SchemaExceptionHandler.java
@@ -28,5 +28,4 @@ protected void throwException(@Nullable ShadowProvisioningOperation<?> operation
             throw new SchemaException(cause.getMessage(), cause);
         }
     }
-
 }
diff --git a/repo/repo-sqale/src/main/java/com/evolveum/midpoint/repo/sqale/SqaleRepositoryService.java b/repo/repo-sqale/src/main/java/com/evolveum/midpoint/repo/sqale/SqaleRepositoryService.java
@@ -20,7 +20,7 @@
 import java.util.stream.Collectors;
 import javax.xml.namespace.QName;
 
-import com.evolveum.midpoint.common.SequenceHelper;
+import com.evolveum.midpoint.common.SequenceUtil;
 
 import com.google.common.base.Strings;
 import com.google.common.collect.ObjectArrays;
@@ -1407,7 +1407,7 @@ private long executeAdvanceSequence(UUID oid)
 
             logger.trace("OBJECT before:\n{}", sequence.debugDumpLazily());
 
-            long returnValue = SequenceHelper.advanceSequence(sequence);
+            long returnValue = SequenceUtil.advanceSequence(sequence);
 
             logger.trace("Return value = {}, OBJECT after:\n{}",
                     returnValue, sequence.debugDumpLazily());

diff --git a/repo/repo-sql-impl/src/main/java/com/evolveum/midpoint/repo/sql/helpers/SequenceHelper.java b/repo/repo-sql-impl/src/main/java/com/evolveum/midpoint/repo/sql/helpers/SequenceHelper.java
@@ -9,6 +9,8 @@
 import java.util.Collection;
 import java.util.Iterator;
 
+import com.evolveum.midpoint.common.SequenceUtil;
+
 import org.hibernate.Session;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.stereotype.Component;
@@ -60,40 +62,10 @@ public long advanceSequenceAttempt(String oid, OperationResult result) throws Ob
             if (LOGGER.isTraceEnabled()) {
                 LOGGER.trace("OBJECT before:\n{}", prismObject.debugDump());
             }
-            SequenceType sequence = prismObject.asObjectable();
 
-            if (!sequence.getUnusedValues().isEmpty()) {
-                returnValue = sequence.getUnusedValues().remove(0);
-            } else {
-                long counter = sequence.getCounter() != null ? sequence.getCounter() : 0L;
-                long maxCounter = sequence.getMaxCounter() != null ? sequence.getMaxCounter() : Long.MAX_VALUE;
-                boolean allowRewind = Boolean.TRUE.equals(sequence.isAllowRewind());
-
-                if (counter < maxCounter) {
-                    returnValue = counter;
-                    sequence.setCounter(counter + 1);
-                } else if (counter == maxCounter) {
-                    returnValue = counter;
-                    if (allowRewind) {
-                        sequence.setCounter(0L);
-                    } else {
-                        sequence.setCounter(counter + 1);       // will produce exception during next run
-                    }
-                } else {        // i.e. counter > maxCounter
-                    if (allowRewind) {          // shouldn't occur but...
-                        LOGGER.warn("Sequence {} overflown with allowRewind set to true. Rewinding.", oid);
-                        returnValue = 0;
-                        sequence.setCounter(1L);
-                    } else {
-                        // TODO some better exception...
-                        throw new SystemException("No (next) value available from sequence " + oid + ". Current counter = " + sequence.getCounter() + ", max value = " + sequence.getMaxCounter());
-                    }
-                }
-            }
+            returnValue = SequenceUtil.advanceSequence(prismObject.asObjectable());
 
-            if (LOGGER.isTraceEnabled()) {
-                LOGGER.trace("Return value = {}, OBJECT after:\n{}", returnValue, prismObject.debugDump());
-            }
+            LOGGER.trace("Return value = {}, OBJECT after:\n{}", returnValue, prismObject.debugDumpLazily());
 
             // merge and update object
             LOGGER.trace("Translating JAXB to data type.");