Merge remote-tracking branch 'refs/remotes/origin/master'

Evolveum · Mar 25, 2022 · 90d96fd · 90d96fd
2 parents c999fc8 + e185f54
commit 90d96fd
Show file tree

Hide file tree

Showing 15 changed files with 144 additions and 141 deletions.
diff --git a/model/model-intest/src/test/java/com/evolveum/midpoint/model/intest/misc/TestMisc.java b/model/model-intest/src/test/java/com/evolveum/midpoint/model/intest/misc/TestMisc.java
@@ -651,7 +651,7 @@ public void test601jackUnassignResourceAccount() throws Exception {
      * first assign role ship, the ship attribute in the role has mapping with calling midpoint.getLinkedShadow()
      */
     @Test
-    public void test602jackUnssigndRoleShip() throws Exception {
+    public void test602jackUnassignRoleShip() throws Exception {
         when();
         unassignRole(USER_JACK_OID, ROLE_SHIP_OID);
 
@@ -669,16 +669,16 @@ public void test700AddNodeArchetype() throws Exception {
         OperationResult result = task.getResult();
 
         when();
-        NodeType localNode = taskManager.getLocalNode();
+        String localNodeOid = taskManager.getLocalNodeOid();
         ObjectDelta<NodeType> delta = deltaFor(NodeType.class)
                 .item(NodeType.F_ASSIGNMENT)
                 .add(ObjectTypeUtil.createAssignmentTo(ARCHETYPE_NODE_GROUP_GUI.oid, ObjectTypes.ARCHETYPE, prismContext))
-                .asObjectDelta(localNode.getOid());
+                .asObjectDelta(localNodeOid);
 
         executeChanges(delta, null, task, result);
 
         then();
-        assertObject(NodeType.class, localNode.getOid(), "after")
+        assertObject(NodeType.class, localNodeOid, "after")
                 .display()
                 .assertArchetypeRef(ARCHETYPE_NODE_GROUP_GUI.oid);
 

diff --git a/...del-test/src/main/java/com/evolveum/midpoint/model/test/AbstractModelIntegrationTest.java b/...del-test/src/main/java/com/evolveum/midpoint/model/test/AbstractModelIntegrationTest.java
@@ -3277,7 +3277,11 @@ protected OperationResult waitForTaskActivityCompleted(final String taskOid, lon
             @Override
             public boolean check() throws CommonException {
                 var task = taskManager.getTaskWithResult(taskOid, waitResult);
-                var activity = task.getActivitiesStateOrClone().getActivity();
+                var activitiesState = task.getActivitiesStateOrClone();
+                if (activitiesState == null) {
+                    return false;
+                }
+                var activity = activitiesState.getActivity();
                 if (activity == null) {
                     return false;
                 }

diff --git a/...-impl/src/main/java/com/evolveum/midpoint/report/impl/activity/SaveReportFileSupport.java b/...-impl/src/main/java/com/evolveum/midpoint/report/impl/activity/SaveReportFileSupport.java
@@ -165,7 +165,7 @@ private String getDestinationFileName(ReportType reportType,
         String reportName = StringUtils.replace(reportType.getName().getOrig(), File.separator, "_");
         String fileNamePrefix = reportName + "-EXPORT " + getDateTime();
         String fileName = fileNamePrefix + dataWriter.getTypeSuffix();
-        return MiscUtil.replaceIllegalCharInFileNameOnWindows(new File(exportDir, fileName).getPath());
+        return new File(exportDir, MiscUtil.replaceIllegalCharInFileNameOnWindows(fileName)).getPath();
     }
 
     static String getNameOfExportedReportData(ReportType reportType, String type) {

diff --git a/...common/src/main/java/com/evolveum/midpoint/repo/common/util/OperationExecutionWriter.java b/...common/src/main/java/com/evolveum/midpoint/repo/common/util/OperationExecutionWriter.java
@@ -159,10 +159,12 @@ private <T> T select(OperationExecutionRecordTypeType recordType, T simpleVersio
     private <O extends ObjectType> void executeChanges(Request<O> request,
             List<OperationExecutionType> recordsToAdd, List<OperationExecutionType> recordsToDelete, OperationResult result)
             throws SchemaException, ObjectAlreadyExistsException, ObjectNotFoundException {
+        // Note that even recordToAdd can have a parent: e.g. if the write to primary owner fails, and we try
+        // the secondary one - the recordToAdd would be already inserted in the respective delta! Hence always cloning it.
         List<ItemDelta<?, ?>> deltas = prismContext.deltaFor(request.objectType)
                 .item(ObjectType.F_OPERATION_EXECUTION)
                 .delete(PrismContainerValue.toPcvList(CloneUtil.cloneCollectionMembers(recordsToDelete)))
-                .add(PrismContainerValue.toPcvList(recordsToAdd)) // assuming these are parent-less
+                .add(PrismContainerValue.toPcvList(CloneUtil.cloneCollectionMembers(recordsToAdd)))
                 .asItemDeltas();
         LOGGER.trace("Operation execution delta:\n{}", DebugUtil.debugDumpLazily(deltas));
         repositoryService.modifyObject(request.objectType, request.oid, deltas, result);

diff --git a/repo/repo-test-util/src/main/java/com/evolveum/midpoint/test/AbstractIntegrationTest.java b/repo/repo-test-util/src/main/java/com/evolveum/midpoint/test/AbstractIntegrationTest.java
@@ -4039,7 +4039,7 @@ private void deleteExistingExtraNodes(OperationResult result) throws SchemaExcep
         SearchResultList<PrismObject<NodeType>> existingNodes =
                 repositoryService.searchObjects(NodeType.class, null, null, result);
         for (PrismObject<NodeType> existingNode : existingNodes) {
-            if (!existingNode.getOid().equals(taskManager.getLocalNode().getOid())) {
+            if (!existingNode.getOid().equals(taskManager.getLocalNodeOid())) {
                 System.out.printf("Deleting extra node %s\n", existingNode);
                 repositoryService.deleteObject(NodeType.class, existingNode.getOid(), result);
             }

diff --git a/repo/task-api/src/main/java/com/evolveum/midpoint/task/api/TaskManager.java b/repo/task-api/src/main/java/com/evolveum/midpoint/task/api/TaskManager.java
@@ -666,7 +666,11 @@ boolean stopSchedulersAndTasks(Collection<String> nodeIdentifiers, long waitTime
 
     TaskHandler getHandler(String handlerUri);
 
-    NodeType getLocalNode();
+    /** Returns the local node object (immutable). */
+    @NotNull NodeType getLocalNode();
+
+    /** Returns the local node object OID. */
+    @NotNull String getLocalNodeOid();
 
     // A little bit of hack as well
     CacheConfigurationManager getCacheConfigurationManager();

diff --git a/...c/main/java/com/evolveum/midpoint/task/quartzimpl/LightweightIdentifierGeneratorImpl.java b/...c/main/java/com/evolveum/midpoint/task/quartzimpl/LightweightIdentifierGeneratorImpl.java
@@ -87,18 +87,7 @@ private int getHostIdentifier() {
     }
 
     private int getHostIdentifierFromNodeOid() {
-        NodeType localNode = taskManager.getLocalNode();
-        if (localNode == null) {
-            LOGGER.warn("Couldn't determine host identifier. No local node.");
-            return UNINITIALIZED;
-        }
-
-        String localNodeOid = localNode.getOid();
-        if (localNodeOid == null) {
-            LOGGER.warn("Couldn't determine host identifier. No local node OID.");
-            return UNINITIALIZED;
-        }
-
+        String localNodeOid = taskManager.getLocalNodeOid();
         try {
             String last4digits = localNodeOid.substring(localNodeOid.length() - 4);
             return Integer.parseInt(last4digits, 16);

diff --git a/.../task-quartz-impl/src/main/java/com/evolveum/midpoint/task/quartzimpl/LocalNodeState.java b/.../task-quartz-impl/src/main/java/com/evolveum/midpoint/task/quartzimpl/LocalNodeState.java
@@ -19,7 +19,7 @@
 
 import com.evolveum.midpoint.xml.ns._public.common.common_3.NodeType;
 
-import org.jetbrains.annotations.Nullable;
+import org.jetbrains.annotations.NotNull;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.stereotype.Component;
 
@@ -73,12 +73,8 @@ public void setErrorState(NodeErrorStateType errorState) {
      * @return current local node information, updated with local node execution and error status.
      * Returned value is fresh, so it can be modified as needed.
      */
-    @Nullable
-    public NodeType getLocalNode() {
-        PrismObject<NodeType> localNode = clusterManager.getLocalNodeObject();
-        if (localNode == null) {
-            return null;
-        }
+    @NotNull public NodeType getLocalNodeWithUpdatedState() {
+        PrismObject<NodeType> localNode = clusterManager.getLocalNodeObject().clone();
         NodeType node = localNode.clone().asObjectable();
         node.setExecutionState(getExecutionState());
         node.setErrorState(errorState);

diff --git a/...uartz-impl/src/main/java/com/evolveum/midpoint/task/quartzimpl/TaskManagerQuartzImpl.java b/...uartz-impl/src/main/java/com/evolveum/midpoint/task/quartzimpl/TaskManagerQuartzImpl.java
@@ -1198,10 +1198,13 @@ public RunningTaskQuartzImpl createFakeRunningTask(Task task) {
     }
 
     @Override
-    public NodeType getLocalNode() {
-        return ObjectTypeUtil.asObjectable(
-                CloneUtil.clone(
-                        nodeRegistrar.getCachedLocalNodeObject()));
+    public @NotNull NodeType getLocalNode() {
+        return nodeRegistrar.getCachedLocalNodeObjectRequired().asObjectable();
+    }
+
+    @Override
+    public @NotNull String getLocalNodeOid() {
+        return nodeRegistrar.getCachedLocalNodeObjectOid();
     }
 
     @Override
@@ -1252,13 +1255,8 @@ public boolean isCheckingIn(NodeType node) {
 
     @Override
     public Collection<ObjectReferenceType> getLocalNodeGroups() {
-        NodeType localNode = getLocalNode();
-        if (localNode == null) {
-            // should not occur during regular operation
-            return emptySet();
-        } else {
-            return Collections.unmodifiableCollection(localNode.getArchetypeRef());
-        }
+        return Collections.unmodifiableCollection(
+                getLocalNode().getArchetypeRef());
     }
 
     // TODO move to more appropriate place

diff --git a/repo/task-quartz-impl/src/main/java/com/evolveum/midpoint/task/quartzimpl/UpAndDown.java b/repo/task-quartz-impl/src/main/java/com/evolveum/midpoint/task/quartzimpl/UpAndDown.java
@@ -9,6 +9,7 @@
 import com.evolveum.midpoint.repo.api.CacheDispatcher;
 import com.evolveum.midpoint.task.api.TaskManager;
 import com.evolveum.midpoint.task.quartzimpl.cluster.ClusterManager;
+import com.evolveum.midpoint.task.quartzimpl.cluster.NodeRegistrar;
 import com.evolveum.midpoint.task.quartzimpl.execution.*;
 import com.evolveum.midpoint.task.quartzimpl.quartz.LocalScheduler;
 import com.evolveum.midpoint.task.quartzimpl.quartz.TaskSynchronizer;
@@ -46,6 +47,7 @@ public class UpAndDown implements BeanFactoryAware {
 
     @Autowired private TaskManagerQuartzImpl taskManager;
     @Autowired private ClusterManager clusterManager;
+    @Autowired private NodeRegistrar nodeRegistrar;
     @Autowired private TaskManagerConfiguration configuration;
     @Autowired private LocalScheduler localScheduler;
     @Autowired private LocalNodeState localNodeState;
@@ -124,9 +126,9 @@ private void initInternal(OperationResult result) throws TaskManagerInitializati
         }
 
         // register node
-        NodeType node = clusterManager.createOrUpdateNodeInRepo(result);     // may throw initialization exception
-        if (!configuration.isTestMode()) {  // in test mode do not start cluster manager thread nor verify cluster config
-            clusterManager.checkClusterConfiguration(result);      // Does not throw exceptions. Sets the ERROR state if necessary, however.
+        NodeType node = nodeRegistrar.initializeNode(result); // may throw initialization exception
+        if (!configuration.isTestMode()) { // in test mode do not start cluster manager thread nor verify cluster config
+            clusterManager.checkClusterConfiguration(result); // Does not throw exceptions. Sets the ERROR state if necessary, however.
         }
 
         JobExecutor.setTaskManagerQuartzImpl(taskManager); // unfortunately, there seems to be no clean way of letting jobs know the taskManager

diff --git a/...c/main/java/com/evolveum/midpoint/task/quartzimpl/cluster/ClusterExecutionHelperImpl.java b/...c/main/java/com/evolveum/midpoint/task/quartzimpl/cluster/ClusterExecutionHelperImpl.java
@@ -216,7 +216,7 @@ private WebClient createClient(NodeType node, ClusterExecutionOptions options, S
         }
         client.type(MediaType.APPLICATION_XML);
         NodeType localNode = taskManager.getLocalNode();
-        ProtectedStringType protectedSecret = localNode != null ? localNode.getSecret() : null;
+        ProtectedStringType protectedSecret = localNode.getSecret();
         if (protectedSecret == null) {
             throw new SchemaException("No secret is set for local node " + localNode);
         }

diff --git a/...artz-impl/src/main/java/com/evolveum/midpoint/task/quartzimpl/cluster/ClusterManager.java b/...artz-impl/src/main/java/com/evolveum/midpoint/task/quartzimpl/cluster/ClusterManager.java
@@ -8,15 +8,13 @@
 
 import com.evolveum.midpoint.prism.PrismContext;
 import com.evolveum.midpoint.prism.PrismObject;
-import com.evolveum.midpoint.prism.delta.ItemDelta;
 import com.evolveum.midpoint.prism.query.ObjectQuery;
 import com.evolveum.midpoint.repo.api.RepositoryService;
 import com.evolveum.midpoint.repo.api.SystemConfigurationChangeDispatcher;
 import com.evolveum.midpoint.schema.SearchResultList;
 import com.evolveum.midpoint.schema.result.OperationResult;
 import com.evolveum.midpoint.schema.util.ObjectQueryUtil;
 import com.evolveum.midpoint.task.api.Task;
-import com.evolveum.midpoint.task.api.TaskManagerInitializationException;
 import com.evolveum.midpoint.task.quartzimpl.TaskManagerConfiguration;
 import com.evolveum.midpoint.task.quartzimpl.TaskManagerQuartzImpl;
 import com.evolveum.midpoint.task.quartzimpl.TaskQuartzImpl;
@@ -110,15 +108,12 @@ public void deleteNode(String nodeOid, OperationResult result) throws SchemaExce
         nodeRegistrar.deleteNode(nodeOid, result);
     }
 
-    public NodeType createOrUpdateNodeInRepo(OperationResult result) throws TaskManagerInitializationException {
-        return nodeRegistrar.createOrUpdateNodeInRepo(result);
+    public @NotNull PrismObject<NodeType> getLocalNodeObject() {
+        return nodeRegistrar.getCachedLocalNodeObjectRequired();
     }
 
-    public PrismObject<NodeType> getLocalNodeObject() {
-        return nodeRegistrar.getCachedLocalNodeObject();
-    }
-
-    public NodeType getFreshVerifiedLocalNodeObject(OperationResult result) {
+    /** Returns null only in case of error. */
+    public @Nullable NodeType getFreshVerifiedLocalNodeObject(OperationResult result) {
         return nodeRegistrar.verifyNodeObject(result);
     }
 
@@ -139,7 +134,7 @@ public void registerNodeUp(OperationResult result) {
         // We do not want to query cluster nodes at this moment. We rely on the repository information.
         SearchResultList<PrismObject<NodeType>> nodes =
                 getRepositoryService().searchObjects(NodeType.class, null, null, result);
-        ClusterStateType clusterState = new ClusterStateType(PrismContext.get());
+        ClusterStateType clusterState = new ClusterStateType();
         // TODO use query after making operationalState indexed
         for (PrismObject<NodeType> node : nodes) {
             String nodeIdentifier = node.asObjectable().getNodeIdentifier();
@@ -209,7 +204,7 @@ public void run() {
                     LoggingUtils.logUnexpectedException(LOGGER, "Unexpected exception in ClusterManager thread; continuing execution.", t);
                 }
 
-                LOGGER.trace("ClusterManager thread sleeping for {} msec", delay);
+                LOGGER.trace("ClusterManager thread sleeping for {} ms", delay);
                 MiscUtil.sleepCatchingInterruptedException(delay);
             }
 
@@ -270,7 +265,7 @@ private boolean markNodeAsDown(NodeType node, OperationResult result) {
     }
 
     private boolean isRemoteNode(NodeType node) {
-        return taskManager.getNodeId() == null || !taskManager.getNodeId().equals(node.getNodeIdentifier());
+        return !taskManager.getNodeId().equals(node.getNodeIdentifier());
     }
 
     private boolean shouldBeMarkedAsDown(NodeType node) {