Skip to content

Commit

Permalink
Clean up and document activity framework code
Browse files Browse the repository at this point in the history
1. Now we prefer composition over subclassing. Activity execution
classes are no longer subclasses of an abstract execution class (except
for very special ones). Instead, IterativeActivityExecutionSpecifics
interface is implemented to provide the necessary functionality.

2. SimpleActivityHandler was aligned with these changes. The framework
itself is now sufficiently simple to make such adaptations
and simplifications unnecessary.

3. Tracing- and profiling-related methods in RunningTask interface
were removed. This functionality is related to activities, not to
tasks.

4. A lot of code was reviewed and cleaned up. Key classes and interfaces
of the framework are now documented.

Externally-visible changes:
- OperationExecutionType now contains activity path instead
of obsolete task part URI.
- Profiling/tracing, reporting options and failed objects selector
are now part of activity definition.
- Temporarily removed support for tracing of the pre-processing
stage for live sync and provisioning search-based activities
(like import or reconciliation). This is a result of fixes
in RunningTask interfaces.
- Removed "Last N failures" hack. It is no longer needed: the regular
failure-tracking mechanism should be good enough now.
  • Loading branch information
mederly committed Aug 4, 2021
1 parent 6ac8c4a commit 63812e8
Show file tree
Hide file tree
Showing 140 changed files with 3,489 additions and 3,286 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -354,7 +354,7 @@ public abstract class SchemaConstants {
public static final ItemName MODEL_EXTENSION_SEARCH_OPTIONS = new ItemName(NS_MODEL_EXTENSION, "searchOptions");
public static final ItemName MODEL_EXTENSION_USE_REPOSITORY_DIRECTLY = new ItemName(NS_MODEL_EXTENSION, "useRepositoryDirectly");
public static final ItemName MODEL_EXTENSION_ITERATION_METHOD = new ItemName(NS_MODEL_EXTENSION, "iterationMethod");
public static final ItemName MODEL_EXTENSION_FAILED_OBJECTS_SELECTOR = new ItemName(NS_MODEL_EXTENSION, "failedObjectsSelector"); // TODO!!!
public static final ItemName MODEL_EXTENSION_FAILED_OBJECTS_SELECTOR = new ItemName(NS_MODEL_EXTENSION, "failedObjectsSelector");
public static final ItemName MODEL_EXTENSION_OBJECT_DELTA = new ItemName(NS_MODEL_EXTENSION, "objectDelta"); // TODO ExecuteDeltasTaskHandler
public static final ItemName MODEL_EXTENSION_OBJECT_DELTAS = new ItemName(NS_MODEL_EXTENSION, "objectDeltas"); // TODO ExecuteDeltasTaskHandler
public static final ItemName MODEL_EXTENSION_WORKER_THREADS = new ItemName(NS_MODEL_EXTENSION, "workerThreads");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@

import com.evolveum.midpoint.util.annotation.Experimental;

import org.jetbrains.annotations.NotNull;

/**
* Represents data about iterative operation that starts.
*
Expand All @@ -17,7 +19,7 @@
@Experimental
public class IterativeOperationStartInfo {

private final IterationItemInformation item;
@NotNull private final IterationItemInformation item;

private final long startTimeMillis;
private final long startTimeNanos;
Expand All @@ -32,14 +34,14 @@ public class IterativeOperationStartInfo {
*/
private ProgressCollector progressCollector;

public IterativeOperationStartInfo(IterationItemInformation item) {
public IterativeOperationStartInfo(@NotNull IterationItemInformation item) {
this.item = item;

this.startTimeMillis = System.currentTimeMillis();
this.startTimeNanos = System.nanoTime();
}

public IterationItemInformation getItem() {
public @NotNull IterationItemInformation getItem() {
return item;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,12 @@ public static int getItemsProcessedShallow(ActivityItemProcessingStatisticsType
}
}

public static int getItemsProcessed(ActivityItemProcessingStatisticsType itemProcessingStatistics) {
return itemProcessingStatistics != null ?
getCounts(List.of(itemProcessingStatistics), set -> true) :
0;
}

public static int getItemsProcessed(@NotNull Collection<ActivityStateType> states) {
return getCounts(
getItemProcessingStatisticsCollection(states),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,13 +36,19 @@ public class ObjectSetUtil {
.type(getItemRealValue(extension, SchemaConstants.MODEL_EXTENSION_OBJECT_TYPE, QName.class))
.query(getQueryLegacy(source))
.searchOptions(getSearchOptionsLegacy(extension))
.useRepositoryDirectly(getUseRepositoryDirectly(extension));
.useRepositoryDirectly(getUseRepositoryDirectly(extension))
.failedObjectsSelector(getFailedObjectsSelector(extension));
}

private static Boolean getUseRepositoryDirectly(PrismContainerValue<?> extension) {
return getItemRealValue(extension, SchemaConstants.MODEL_EXTENSION_USE_REPOSITORY_DIRECTLY, Boolean.class);
}

static FailedObjectsSelectorType getFailedObjectsSelector(PrismContainerValue<?> extension) {
return getItemRealValue(extension, SchemaConstants.MODEL_EXTENSION_FAILED_OBJECTS_SELECTOR,
FailedObjectsSelectorType.class);
}

static QueryType getQueryLegacy(@NotNull LegacyWorkDefinitionSource source) {
QueryType fromObjectRef = getQueryFromTaskObjectRef(source.getObjectRef());
if (fromObjectRef != null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,7 @@

package com.evolveum.midpoint.schema.util.task.work;

import static com.evolveum.midpoint.schema.util.task.work.ObjectSetUtil.getQueryLegacy;
import static com.evolveum.midpoint.schema.util.task.work.ObjectSetUtil.getSearchOptionsLegacy;
import static com.evolveum.midpoint.schema.util.task.work.ObjectSetUtil.*;

import javax.xml.namespace.QName;

Expand Down Expand Up @@ -36,7 +35,8 @@ public class ResourceObjectSetUtil {
.kind(getItemRealValue(extension, SchemaConstants.MODEL_EXTENSION_KIND, ShadowKindType.class))
.intent(getItemRealValue(extension, SchemaConstants.MODEL_EXTENSION_INTENT, String.class))
.query(getQueryLegacy(source))
.searchOptions(getSearchOptionsLegacy(extension));
.searchOptions(getSearchOptionsLegacy(extension))
.failedObjectsSelector(getFailedObjectsSelector(extension));
}

// TODO move to PCV
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22565,6 +22565,20 @@
</xsd:documentation>
<xsd:appinfo>
<a:since>4.3</a:since>
<a:deprecated>true</a:deprecated>
<a:deprecatedSince>4.4</a:deprecatedSince>
</xsd:appinfo>
</xsd:annotation>
</xsd:element>
<xsd:element name="activityPath" type="tns:ActivityPathType" minOccurs="0">
<xsd:annotation>
<xsd:documentation>
Path to the activity whose execution resulted in this record. Necessary to distinguish between
results of various activities within a task. Currently supported only for complex operations.
</xsd:documentation>
<xsd:appinfo>
<a:since>4.4</a:since>
<a:experimental>true</a:experimental>
</xsd:appinfo>
</xsd:annotation>
</xsd:element>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2232,6 +2232,30 @@
</xsd:complexType>
<xsd:element name="processTracingConfiguration" type="tns:ProcessTracingConfigurationType"/>

<xsd:complexType name="ProcessProfilingConfigurationType">
<xsd:annotation>
<xsd:documentation>
</xsd:documentation>
<xsd:appinfo>
<a:since>4.4</a:since>
<a:container>true</a:container>
</xsd:appinfo>
</xsd:annotation>
<xsd:sequence>
<xsd:element name="interval" type="xsd:int" minOccurs="0">
<xsd:annotation>
<xsd:documentation>
Profiles processing of each N-th item.
</xsd:documentation>
<xsd:appinfo>
<a:displayName>Profiling interval</a:displayName>
</xsd:appinfo>
</xsd:annotation>
</xsd:element>
</xsd:sequence>
</xsd:complexType>
<xsd:element name="processProfilingConfiguration" type="tns:ProcessProfilingConfigurationType"/>

<xsd:complexType name="TracingProfileType">
<xsd:annotation>
<xsd:documentation>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1810,6 +1810,20 @@
</xsd:documentation>
</xsd:annotation>
</xsd:element>
<xsd:element name="monitoring" type="tns:ActivityMonitoringDefinitionType" minOccurs="0">
<xsd:annotation>
<xsd:documentation>
How the activity is monitored, e.g. using tracing or dynamic profiling.
</xsd:documentation>
</xsd:annotation>
</xsd:element>
<xsd:element name="reporting" type="tns:TaskReportingOptionsType" minOccurs="0">
<xsd:annotation>
<xsd:documentation>
How the activity is monitored, e.g. using tracing or dynamic profiling.
</xsd:documentation>
</xsd:annotation>
</xsd:element>
<xsd:element name="tailoring" type="tns:ActivitiesTailoringType" minOccurs="0">
<xsd:annotation>
<xsd:documentation>
Expand Down Expand Up @@ -1977,6 +1991,39 @@
<xsd:attribute name="id" type="xsd:long"/>
</xsd:complexType>

<xsd:complexType name="ActivityMonitoringDefinitionType">
<xsd:annotation>
<xsd:documentation>
How the activity is monitored, e.g. using tracing or dynamic profiling.
</xsd:documentation>
<xsd:appinfo>
<a:since>4.4</a:since>
<a:container>true</a:container>
<a:experimental>true</a:experimental>
</xsd:appinfo>
</xsd:annotation>
<xsd:sequence>
<xsd:element name="tracing" type="tns:ProcessTracingConfigurationType" minOccurs="0">
<xsd:annotation>
<xsd:documentation>
Specifies when and how should be the activity execution traced. Contains tracing interval
(i.e. how often a trace should be recorded), tracing profile to be used and tracing points
(roots) where the tracing should be done.
</xsd:documentation>
</xsd:annotation>
</xsd:element>
<xsd:element name="profiling" type="tns:ProcessProfilingConfigurationType" minOccurs="0">
<xsd:annotation>
<xsd:documentation>
Specifies when and how should be the activity execution dynamically profiled.
This functionality is deprecated. The tracing should be used instead.
</xsd:documentation>
</xsd:annotation>
</xsd:element>
</xsd:sequence>
<xsd:attribute name="id" type="xsd:long"/>
</xsd:complexType>

<xsd:complexType name="ActivitiesTailoringType">
<xsd:annotation>
<xsd:documentation>
Expand Down Expand Up @@ -5752,6 +5799,15 @@
</xsd:documentation>
</xsd:annotation>
</xsd:element>
<xsd:element name="failedObjectsSelector" type="tns:FailedObjectsSelectorType" minOccurs="0">
<xsd:annotation>
<xsd:documentation>
Selects objects that were failed to be processed in previous task run(s).
TODO To be decided if this element belongs to the work definition or to a separate
place in the activity definition (something like "error handling").
</xsd:documentation>
</xsd:annotation>
</xsd:element>
</xsd:sequence>
</xsd:complexType>
<xsd:element name="objectSet" type="tns:ObjectSetType"/>
Expand Down Expand Up @@ -5821,6 +5877,15 @@
</xsd:documentation>
</xsd:annotation>
</xsd:element>
<xsd:element name="failedObjectsSelector" type="tns:FailedObjectsSelectorType" minOccurs="0">
<xsd:annotation>
<xsd:documentation>
Selects objects that were failed to be processed in previous task run(s).
TODO To be decided if this element belongs to the work definition or to a separate
place in the activity definition (something like "error handling").
</xsd:documentation>
</xsd:annotation>
</xsd:element>
</xsd:sequence>
</xsd:complexType>
<xsd:element name="resourceObjectSet" type="tns:ResourceObjectSetType"/>
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import javax.annotation.PreDestroy;

import com.evolveum.midpoint.repo.common.activity.ActivityStateDefinition;
import com.evolveum.midpoint.repo.common.activity.execution.CompositeActivityExecution;
import com.evolveum.midpoint.util.exception.SchemaException;

import com.evolveum.midpoint.xml.ns._public.common.common_3.*;
Expand Down Expand Up @@ -59,10 +60,10 @@ public void unregister() {
}

@Override
public @NotNull CleanupActivityExecution createExecution(
public @NotNull CompositeActivityExecution<CleanupWorkDefinition, CleanupActivityHandler, ?> createExecution(
@NotNull ExecutionInstantiationContext<CleanupWorkDefinition, CleanupActivityHandler> context,
@NotNull OperationResult result) {
return new CleanupActivityExecution(context);
return new CompositeActivityExecution<>(context);
}

@Override
Expand All @@ -79,7 +80,8 @@ public void unregister() {
new CleanupPartialActivityExecution<>(
context, Part.AUDIT_RECORDS, CleanupPoliciesType::getAuditRecords,
(p, task, result1) -> auditService.cleanupAudit(p, result1)),
null, (i) -> Part.AUDIT_RECORDS.identifier,
null,
(i) -> Part.AUDIT_RECORDS.identifier,
stateDef,
parentActivity));

Expand All @@ -88,7 +90,8 @@ public void unregister() {
new CleanupPartialActivityExecution<>(
context, Part.CLOSED_TASKS, CleanupPoliciesType::getClosedTasks,
(p, task, result1) -> taskManager.cleanupTasks(p, task, result1)),
null, (i) -> Part.CLOSED_TASKS.identifier,
null,
(i) -> Part.CLOSED_TASKS.identifier,
stateDef,
parentActivity));

Expand All @@ -97,7 +100,8 @@ public void unregister() {
new CleanupPartialActivityExecution<>(
context, Part.CLOSED_CASES, CleanupPoliciesType::getClosedCases,
this::cleanupCases),
null, (i) -> Part.CLOSED_CASES.identifier,
null,
(i) -> Part.CLOSED_CASES.identifier,
stateDef,
parentActivity));

Expand All @@ -106,7 +110,8 @@ public void unregister() {
new CleanupPartialActivityExecution<>(
context, Part.DEAD_NODES, CleanupPoliciesType::getDeadNodes,
(p, task, result1) -> taskManager.cleanupNodes(p, task, result1)),
null, (i) -> Part.DEAD_NODES.identifier,
null,
(i) -> Part.DEAD_NODES.identifier,
stateDef,
parentActivity));

Expand All @@ -115,7 +120,8 @@ public void unregister() {
new CleanupPartialActivityExecution<>(
context, Part.OUTPUT_REPORTS, CleanupPoliciesType::getOutputReports,
(p, task, result1) -> cleanupReports(p, result1)),
null, (i) -> Part.OUTPUT_REPORTS.identifier,
null,
(i) -> Part.OUTPUT_REPORTS.identifier,
stateDef,
parentActivity));

Expand All @@ -124,7 +130,8 @@ public void unregister() {
new CleanupPartialActivityExecution<>(
context, Part.CLOSED_CERTIFICATION_CAMPAIGNS, CleanupPoliciesType::getClosedCertificationCampaigns,
(p, task, result1) -> certificationService.cleanupCampaigns(p, task, result1)),
null, (i) -> Part.CLOSED_CERTIFICATION_CAMPAIGNS.identifier,
null,
(i) -> Part.CLOSED_CERTIFICATION_CAMPAIGNS.identifier,
stateDef,
parentActivity));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,10 @@
import static com.evolveum.midpoint.task.api.TaskRunResult.TaskRunResultStatus.PERMANENT_ERROR;

/**
* Activity execution for an elementary cleanup part.
*
* TODO this class is not finished, e.g. statistics reporting is not complete
*
* @param <CP> Cleanup policy type
*/
public class CleanupPartialActivityExecution<CP>
Expand All @@ -51,22 +55,22 @@ public class CleanupPartialActivityExecution<CP>
}

@Override
public boolean supportsStatistics() {
public boolean doesSupportStatistics() {
return true;
}

@Override
public boolean supportsSynchronizationStatistics() {
public boolean doesSupportSynchronizationStatistics() {
return false;
}

@Override
public boolean supportsActionsExecuted() {
public boolean doesSupportActionsExecuted() {
return false; // Low-level direct repo operations are not reported in this way
}

@Override
public boolean supportsProgress() {
public boolean doesSupportProgress() {
return part.supportsProgress;
}

Expand Down

0 comments on commit 63812e8

Please sign in to comment.