Skip to content

Commit

Permalink
Remove deprecated TaskType.policyRule
Browse files Browse the repository at this point in the history
There is a migration to focusValidityScan.validityConstraint,
although not a complete one: even if the ability to recompute
objects is retained, custom policy actions can no longer be defined.
However, 1) this functionality was undocumented, untested, and
experimental; 2) since approx. 4.4 it didn't work at all, due to
(unrelated) bug.

Hence, this commit:

1. Fixes the underlying bug. This involves an incompatible change
(improvement) of internal SearchBasedActivityRunSpecifics
interface. This is going to be reflected in the release notes.

2. Removes TaskType.policyRule. If needed, analogous functionality
can be added into FocusValidityScanWorkDefinitionType.
  • Loading branch information
mederly committed May 25, 2023
1 parent 235df5a commit 12f6f66
Show file tree
Hide file tree
Showing 29 changed files with 295 additions and 262 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ public List<O> getSelectedData() {
// Here we apply the distinct option. It is easier and more reliable to apply it here than to do at all the places
// where options for this provider are defined.
protected Collection<SelectorOptions<GetOperationOptions>> getOptionsToUse() {
return GetOperationOptions.merge(getPrismContext(), options, getDistinctRelatedOptions());
return GetOperationOptions.merge(options, getDistinctRelatedOptions());
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ public Class<? extends AssignmentHolderType> getType() {
// where options for this provider are defined.
protected Collection<SelectorOptions<GetOperationOptions>> getOptionsToUse() {
@NotNull Collection<SelectorOptions<GetOperationOptions>> rawOption = getOperationOptionsBuilder().raw().build();
return GetOperationOptions.merge(getPrismContext(), getOptions(), getDistinctRelatedOptions(), rawOption);
return GetOperationOptions.merge(getOptions(), getDistinctRelatedOptions(), rawOption);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -228,8 +228,7 @@ protected int internalSize() {
Task task = getPageBase().createSimpleTask(OPERATION_COUNT_OBJECTS);
OperationResult result = task.getResult();
try {
Collection<SelectorOptions<GetOperationOptions>> currentOptions = GetOperationOptions.merge(PrismContext.get(), getOptions(),
null);
Collection<SelectorOptions<GetOperationOptions>> currentOptions = GetOperationOptions.merge( getOptions(), null);
Integer counted = countObjects(getType(), getQuery(), currentOptions, task, result);
count = defaultIfNull(counted, defaultCountIfNull);
} catch (Exception ex) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1213,9 +1213,8 @@ public static GetOperationOptions fromRestOptions(List<String> options, Definiti

@NotNull
@SafeVarargs
public static Collection<SelectorOptions<GetOperationOptions>> merge(PrismContext prismContext,
Collection<SelectorOptions<GetOperationOptions>>... parts) {
final UniformItemPath emptyPath = prismContext.emptyPath();
public static Collection<SelectorOptions<GetOperationOptions>> merge(Collection<SelectorOptions<GetOperationOptions>>... parts) {
final UniformItemPath emptyPath = PrismContext.get().emptyPath();
Collection<SelectorOptions<GetOperationOptions>> merged = new ArrayList<>();
for (Collection<SelectorOptions<GetOperationOptions>> part : parts) {
for (SelectorOptions<GetOperationOptions> increment : CollectionUtils.emptyIfNull(part)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,11 @@
<a:version>4.8</a:version>
<a:operation>removed</a:operation>
</a:schemaMigration>
<a:schemaMigration>
<a:element>tns:policyRule</a:element>
<a:version>4.8</a:version>
<a:operation>removed</a:operation>
</a:schemaMigration>
</xsd:appinfo>
</xsd:annotation>
<xsd:complexContent>
Expand Down Expand Up @@ -470,19 +475,6 @@
</xsd:documentation>
</xsd:annotation>
</xsd:element>
<xsd:element name="policyRule" type="tns:PolicyRuleType" minOccurs="0">
<xsd:annotation>
<xsd:documentation>
Policy rule for the task. E.g notify user 5 days before their account expires.

DEPRECATED. Moved to the FocusValidityScanWorkDefinitionType.validityConstraint.
</xsd:documentation>
<xsd:appinfo>
<a:deprecated>true</a:deprecated>
<a:deprecatedSince>4.4</a:deprecatedSince>
</xsd:appinfo>
</xsd:annotation>
</xsd:element>
<xsd:element name="autoScaling" type="tns:TaskAutoScalingType" minOccurs="0">
<xsd:annotation>
<xsd:documentation>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,24 +6,20 @@
*/
package com.evolveum.midpoint.model.impl.cleanup;

import com.evolveum.midpoint.repo.common.activity.run.*;

import jakarta.annotation.PostConstruct;
import jakarta.annotation.PreDestroy;

import com.evolveum.midpoint.repo.common.activity.run.ActivityRunException;

import org.jetbrains.annotations.NotNull;
import org.springframework.stereotype.Component;

import com.evolveum.midpoint.model.api.ModelPublicConstants;
import com.evolveum.midpoint.model.impl.tasks.ModelActivityHandler;
import com.evolveum.midpoint.model.impl.tasks.scanner.ScanActivityRun;
import com.evolveum.midpoint.prism.query.ObjectQuery;
import com.evolveum.midpoint.repo.common.activity.run.state.ActivityStateDefinition;
import com.evolveum.midpoint.repo.common.activity.definition.AbstractWorkDefinition;
import com.evolveum.midpoint.repo.common.activity.definition.ObjectSetSpecificationProvider;
import com.evolveum.midpoint.repo.common.activity.run.AbstractActivityRun;
import com.evolveum.midpoint.repo.common.activity.run.ActivityRunInstantiationContext;
import com.evolveum.midpoint.repo.common.activity.run.ActivityReportingCharacteristics;
import com.evolveum.midpoint.repo.common.activity.run.processing.ItemProcessingRequest;
import com.evolveum.midpoint.schema.result.OperationResult;
import com.evolveum.midpoint.schema.util.ObjectQueryUtil;
Expand Down Expand Up @@ -112,15 +108,15 @@ public void beforeRun(OperationResult result) {
}

@Override
public ObjectQuery customizeQuery(ObjectQuery configuredQuery, OperationResult result) {
if (ObjectQueryUtil.hasFilter(configuredQuery)) {
return configuredQuery;
} else {
return ObjectQueryUtil.replaceFilter(
configuredQuery,
getBeans().prismContext.queryFor(ShadowType.class)
.exists(ShadowType.F_PENDING_OPERATION)
.buildFilter());
public void customizeQuery(SearchSpecification<ShadowType> searchSpecification, OperationResult result) {
var configuredQuery = searchSpecification.getQuery();
if (!ObjectQueryUtil.hasFilter(configuredQuery)) {
searchSpecification.setQuery(
ObjectQueryUtil.replaceFilter(
configuredQuery,
getBeans().prismContext.queryFor(ShadowType.class)
.exists(ShadowType.F_PENDING_OPERATION)
.buildFilter()));
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@

package com.evolveum.midpoint.model.impl.integrity.objects;

import java.util.Collection;
import java.util.Map;
import javax.xml.namespace.QName;

import com.evolveum.midpoint.repo.common.activity.run.SearchSpecification;

import com.google.common.base.MoreObjects;
import org.jetbrains.annotations.NotNull;
import org.springframework.stereotype.Component;
Expand Down Expand Up @@ -114,9 +115,13 @@ static final class MyRun extends
}

@Override
public Collection<SelectorOptions<GetOperationOptions>> customizeSearchOptions(
Collection<SelectorOptions<GetOperationOptions>> configuredOptions, OperationResult result) {
return SelectorOptions.updateRootOptions(configuredOptions, opt -> opt.setAttachDiagData(true), GetOperationOptions::new);
public void customizeSearchOptions(
SearchSpecification<ObjectType> searchSpecification, OperationResult result) {
searchSpecification.setSearchOptions(
SelectorOptions.updateRootOptions(
searchSpecification.getSearchOptions(),
opt -> opt.setAttachDiagData(true),
GetOperationOptions::new));
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,18 +10,14 @@
import java.util.Collection;

import com.evolveum.midpoint.model.impl.sync.tasks.ProcessingScope;
import com.evolveum.midpoint.repo.common.activity.run.ActivityRunException;
import com.evolveum.midpoint.repo.common.activity.run.SearchBasedActivityRun;
import com.evolveum.midpoint.repo.common.activity.run.*;

import com.evolveum.midpoint.xml.ns._public.common.common_3.*;

import org.jetbrains.annotations.NotNull;

import com.evolveum.midpoint.model.impl.ModelBeans;
import com.evolveum.midpoint.model.impl.sync.tasks.Synchronizer;
import com.evolveum.midpoint.prism.PrismContext;
import com.evolveum.midpoint.repo.common.activity.run.ActivityRunInstantiationContext;
import com.evolveum.midpoint.repo.common.activity.run.ActivityReportingCharacteristics;
import com.evolveum.midpoint.repo.common.activity.run.processing.ItemProcessingRequest;
import com.evolveum.midpoint.repo.common.activity.run.buckets.ItemDefinitionProvider;
import com.evolveum.midpoint.schema.GetOperationOptions;
Expand Down Expand Up @@ -77,16 +73,16 @@ private Synchronizer createSynchronizer() {
}

@Override
public Collection<SelectorOptions<GetOperationOptions>> customizeSearchOptions(
Collection<SelectorOptions<GetOperationOptions>> configuredOptions, OperationResult result) {
public void customizeSearchOptions(SearchSpecification<ShadowType> searchSpecification, OperationResult result) {
Collection<SelectorOptions<GetOperationOptions>> defaultOptions = SchemaService.get().getOperationOptionsBuilder()
.doNotDiscovery(false)
.errorReportingMethod(FetchErrorReportingMethodType.FETCH_RESULT)
.build();

// It is questionable if "do not discovery" and "error reporting" can be overridden from the task
// or not. Let us assume reasonable administrators and allow the overriding. Otherwise, we would swap the arguments below.
return GetOperationOptions.merge(PrismContext.get(), defaultOptions, configuredOptions);
searchSpecification.setSearchOptions(
GetOperationOptions.merge(defaultOptions, searchSpecification.getSearchOptions()));
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,12 @@

package com.evolveum.midpoint.model.impl.sync.tasks.recon;

import com.evolveum.midpoint.repo.common.activity.run.SearchSpecification;

import com.google.common.annotations.VisibleForTesting;
import org.jetbrains.annotations.NotNull;

import com.evolveum.midpoint.model.impl.util.ModelImplUtils;
import com.evolveum.midpoint.prism.query.ObjectQuery;
import com.evolveum.midpoint.provisioning.api.ProvisioningOperationOptions;
import com.evolveum.midpoint.repo.cache.RepositoryCache;
import com.evolveum.midpoint.repo.common.activity.run.ActivityRunInstantiationContext;
Expand Down Expand Up @@ -43,12 +44,13 @@ public boolean doesRequireDirectRepositoryAccess() {
* We ignore other parameters like kind, intent or object class. This is a behavior inherited from pre-4.4.
*/
@Override
public ObjectQuery customizeQuery(ObjectQuery configuredQuery, OperationResult result) {
return getBeans().prismContext.queryFor(ShadowType.class)
.item(ShadowType.F_RESOURCE_REF).ref(processingScope.getResourceOid())
.and()
.exists(ShadowType.F_PENDING_OPERATION)
.build();
public void customizeQuery(@NotNull SearchSpecification<ShadowType> searchSpecification, OperationResult result) {
searchSpecification.setQuery(
getBeans().prismContext.queryFor(ShadowType.class)
.item(ShadowType.F_RESOURCE_REF).ref(processingScope.getResourceOid())
.and()
.exists(ShadowType.F_PENDING_OPERATION)
.build());
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import java.util.Collection;
import javax.xml.datatype.XMLGregorianCalendar;

import com.evolveum.midpoint.repo.common.activity.run.SearchSpecification;
import com.evolveum.midpoint.xml.ns._public.common.common_3.*;

import com.google.common.annotations.VisibleForTesting;
Expand All @@ -21,7 +22,6 @@
import com.evolveum.midpoint.model.impl.util.ModelImplUtils;
import com.evolveum.midpoint.prism.PrismObject;
import com.evolveum.midpoint.prism.path.ItemName;
import com.evolveum.midpoint.prism.query.ObjectQuery;
import com.evolveum.midpoint.provisioning.api.ResourceObjectShadowChangeDescription;
import com.evolveum.midpoint.repo.common.activity.run.ActivityRunInstantiationContext;
import com.evolveum.midpoint.repo.common.activity.run.state.ActivityState;
Expand Down Expand Up @@ -73,7 +73,7 @@ public boolean doesRequireDirectRepositoryAccess() {
* TODO change it!
*/
@Override
public ObjectQuery customizeQuery(ObjectQuery configuredQuery, OperationResult result)
public void customizeQuery(@NotNull SearchSpecification<ShadowType> searchSpecification, OperationResult result)
throws SchemaException, ObjectNotFoundException {

// We doing dry run or preview, we must look after synchronizationTimestamp, because this is the one that
Expand All @@ -89,14 +89,15 @@ public ObjectQuery customizeQuery(ObjectQuery configuredQuery, OperationResult r
ShadowType.F_SYNCHRONIZATION_TIMESTAMP;

// TODO maybe we should filter on kind/intent here as well, and not rely on objectsFilter?
return getBeans().prismContext.queryFor(ShadowType.class)
.block()
.item(syncTimestampItem).le(getReconciliationStartTimestamp(result))
.or().item(syncTimestampItem).isNull()
.endBlock()
.and().item(ShadowType.F_RESOURCE_REF).ref(processingScope.getResourceOid())
.and().item(ShadowType.F_OBJECT_CLASS).eq(processingScope.getResolvedObjectClassName())
.build();
searchSpecification.setQuery(
getBeans().prismContext.queryFor(ShadowType.class)
.block()
.item(syncTimestampItem).le(getReconciliationStartTimestamp(result))
.or().item(syncTimestampItem).isNull()
.endBlock()
.and().item(ShadowType.F_RESOURCE_REF).ref(processingScope.getResourceOid())
.and().item(ShadowType.F_OBJECT_CLASS).eq(processingScope.getResolvedObjectClassName())
.build());
}

private @NotNull XMLGregorianCalendar getReconciliationStartTimestamp(OperationResult opResult)
Expand All @@ -111,11 +112,11 @@ public ObjectQuery customizeQuery(ObjectQuery configuredQuery, OperationResult r

// Ignoring configured search options. TODO ok?
@Override
public Collection<SelectorOptions<GetOperationOptions>> customizeSearchOptions(
Collection<SelectorOptions<GetOperationOptions>> configuredOptions, OperationResult result) {
return getBeans().schemaService.getOperationOptionsBuilder()
.errorReportingMethod(FetchErrorReportingMethodType.FETCH_RESULT)
.build();
public void customizeSearchOptions(SearchSpecification<ShadowType> searchSpecification, OperationResult result) {
searchSpecification.setSearchOptions(
getBeans().schemaService.getOperationOptionsBuilder()
.errorReportingMethod(FetchErrorReportingMethodType.FETCH_RESULT)
.build());
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,10 @@

package com.evolveum.midpoint.model.impl.sync.tasks.recon;

import java.util.Collection;

import com.evolveum.midpoint.repo.common.activity.run.ActivityRunException;

import com.evolveum.midpoint.repo.common.activity.run.SearchSpecification;

import com.google.common.annotations.VisibleForTesting;
import org.jetbrains.annotations.NotNull;

Expand All @@ -19,8 +19,6 @@
import com.evolveum.midpoint.repo.common.activity.run.ActivityReportingCharacteristics;
import com.evolveum.midpoint.repo.common.activity.run.processing.ItemProcessingRequest;
import com.evolveum.midpoint.repo.common.activity.run.buckets.ItemDefinitionProvider;
import com.evolveum.midpoint.schema.GetOperationOptions;
import com.evolveum.midpoint.schema.SelectorOptions;
import com.evolveum.midpoint.schema.constants.SchemaConstants;
import com.evolveum.midpoint.schema.result.OperationResult;
import com.evolveum.midpoint.task.api.RunningTask;
Expand Down Expand Up @@ -67,13 +65,13 @@ private Synchronizer createSynchronizer() {

// Ignoring configured search options. TODO ok?
@Override
public Collection<SelectorOptions<GetOperationOptions>> customizeSearchOptions(
Collection<SelectorOptions<GetOperationOptions>> configuredOptions, OperationResult result) {
public void customizeSearchOptions(SearchSpecification<ShadowType> searchSpecification, OperationResult result) {
// This is necessary to give ItemProcessingGatekeeper a chance to "see" errors in preprocessing.
// At the same time, it ensures that an exception in preprocessing does not kill the whole searchObjectsIterative call.
return getBeans().schemaService.getOperationOptionsBuilder()
.errorReportingMethod(FetchErrorReportingMethodType.FETCH_RESULT)
.build();
searchSpecification.setSearchOptions(
getBeans().schemaService.getOperationOptionsBuilder()
.errorReportingMethod(FetchErrorReportingMethodType.FETCH_RESULT)
.build());
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
import java.util.Objects;
import javax.xml.namespace.QName;

import com.evolveum.midpoint.repo.common.activity.run.SearchSpecification;
import com.evolveum.midpoint.schema.GetOperationOptions;
import com.evolveum.midpoint.schema.SelectorOptions;

Expand Down Expand Up @@ -114,7 +115,7 @@ static final class MyRun extends
* - in modern case the default of `true` is used.
*
* (Note that similar structure for the search options is not stored here. The search options
* are managed by the activity framework itself. See {@link #customizeSearchOptions(Collection, OperationResult)}.
* are managed by the activity framework itself. See {@link #customizeSearchOptions(SearchSpecification, OperationResult)}.
* But the principle of managing raw value in search is the same.)
*/
private ModelExecuteOptions effectiveModelExecuteOptions;
Expand Down Expand Up @@ -180,9 +181,10 @@ private void checkRawModeSettings() {
}

@Override
public Collection<SelectorOptions<GetOperationOptions>> customizeSearchOptions(
Collection<SelectorOptions<GetOperationOptions>> configuredOptions, OperationResult result) {
return getSearchOptionsWithRawSet(configuredOptions);
public void customizeSearchOptions(SearchSpecification<ObjectType> searchSpecification, OperationResult result) {
searchSpecification.setSearchOptions(
getSearchOptionsWithRawSet(
searchSpecification.getSearchOptions()));
}

/** Returns search options that have `raw` option set (provided or default). */
Expand Down

0 comments on commit 12f6f66

Please sign in to comment.