Skip to content

Commit

Permalink
Fix working with OperationResult in Handler
Browse files Browse the repository at this point in the history
OperationResult should be filled-in according to the code call
structure, i.e. called code should use the result of the parent.

The use of Handler interface, passing values to be processed but without
the corresponding OperationResult, is not correct in this respect.

So here are the following changes:

1. Introduced ObjectHandler interface that contains not only the value
but also the OperationResult instance. [TODO: better name?]

2. Used ObjectHandler for iterative processing in reports, instead of
Handler.

3. Renamed ObjectHandler in UCF API to UcfObjectHandler, to avoid naming
collisions.

Related to MID-7737.
  • Loading branch information
mederly committed Mar 16, 2022
1 parent d2a411a commit 27de9e7
Show file tree
Hide file tree
Showing 21 changed files with 105 additions and 82 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
/*
* Copyright (c) 2010-2013 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.schema;

import com.evolveum.midpoint.prism.PrismObject;
import com.evolveum.midpoint.schema.result.OperationResult;

/**
* Classes implementing this interface are used to handle arbitrary objects (not always {@link PrismObject} instances),
* typically - but not necessarily - coming from iterative search operation.
*
* TODO resolve class naming with {@link ResultHandler}; maybe ObjectHandler is not the best name at all?
*
* @param <T> type of the objects; intentionally general enough to cover both prism objects, containerables, and maybe
* others in the future
*
* @see ResultHandler
*/
@FunctionalInterface
public interface ObjectHandler<T> {

/**
* Handle a single object.
*
* @param object Object to handle.
* @param result Where to store the result of the processing.
*
* @return true if the operation should proceed, false if it should stop
*/
boolean handle(T object, OperationResult result);
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

import com.evolveum.midpoint.report.impl.ReportUtils;
import com.evolveum.midpoint.report.impl.controller.*;
import com.evolveum.midpoint.schema.ObjectHandler;
import com.evolveum.midpoint.task.api.RunningTask;

import com.evolveum.midpoint.xml.ns._public.common.common_3.*;
Expand All @@ -25,11 +26,8 @@
import com.evolveum.midpoint.prism.Containerable;
import com.evolveum.midpoint.repo.common.activity.run.ActivityRunException;
import com.evolveum.midpoint.schema.result.OperationResult;
import com.evolveum.midpoint.util.Handler;
import com.evolveum.midpoint.util.exception.CommonException;

import org.jetbrains.annotations.Nullable;

/**
* Activity execution specifics for classical (i.e. not distributed) collection report export.
*/
Expand Down Expand Up @@ -114,19 +112,19 @@ public Integer determineOverallSize(OperationResult result) throws CommonExcepti
}

@Override
public void iterateOverItemsInBucket(OperationResult result) throws CommonException {
public void iterateOverItemsInBucket(OperationResult gResult) throws CommonException {
// Issue the search to audit or model/repository
// And use the following handler to handle the results

AtomicInteger sequence = new AtomicInteger(0);

Handler<Containerable> handler = record -> {
ObjectHandler<Containerable> handler = (record, lResult) -> {
ItemProcessingRequest<Containerable> request =
ContainerableProcessingRequest.create(sequence.getAndIncrement(), record, this);
coordinator.submit(request, result);
coordinator.submit(request, lResult);
return true;
};
searchSpecificationHolder.run(handler, result);
searchSpecificationHolder.run(handler, gResult);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,9 @@
import com.evolveum.midpoint.report.impl.ReportServiceImpl;
import com.evolveum.midpoint.report.impl.ReportUtils;
import com.evolveum.midpoint.report.impl.controller.*;
import com.evolveum.midpoint.schema.ObjectHandler;
import com.evolveum.midpoint.schema.result.OperationResult;
import com.evolveum.midpoint.task.api.RunningTask;
import com.evolveum.midpoint.util.Handler;
import com.evolveum.midpoint.util.exception.CommonException;
import com.evolveum.midpoint.xml.ns._public.common.common_3.*;

Expand Down Expand Up @@ -130,7 +130,7 @@ public Integer determineOverallSize(OperationResult result) throws CommonExcepti
}

@Override
public void iterateOverItemsInBucket(OperationResult result) throws CommonException {
public void iterateOverItemsInBucket(OperationResult gResult) throws CommonException {
// Issue the search to audit or model/repository
// And use the following handler to handle the results

Expand All @@ -141,23 +141,23 @@ public void iterateOverItemsInBucket(OperationResult result) throws CommonExcept
ExportDashboardReportLine<Containerable> widgetLine = new ExportDashboardReportLine<>(widgetSequence.getAndIncrement(), widget);
ItemProcessingRequest<ExportDashboardReportLine<Containerable>> widgetRequest = new ExportDashboardReportLineProcessingRequest(
widgetLine, this);
coordinator.submit(widgetRequest, result);
coordinator.submit(widgetRequest, gResult);

if (support.isWidgetTableVisible()) {
AtomicInteger sequence = new AtomicInteger(1);
Handler<Containerable> handler = record -> {
ObjectHandler<Containerable> handler = (record, lResult) -> {
ExportDashboardReportLine<Containerable> line = new ExportDashboardReportLine<>(sequence.getAndIncrement(),
record,
widget.getIdentifier());
ItemProcessingRequest<ExportDashboardReportLine<Containerable>> request = new ExportDashboardReportLineProcessingRequest(
line, this);
coordinator.submit(request, result);
ItemProcessingRequest<ExportDashboardReportLine<Containerable>> request =
new ExportDashboardReportLineProcessingRequest(line, this);
coordinator.submit(request, lResult);
return true;
};

DashboardWidgetHolder holder = mapOfWidgetsController.get(widget.getIdentifier());
ContainerableReportDataSource searchSpecificationHolder = holder.getSearchSpecificationHolder();
searchSpecificationHolder.run(handler, result);
searchSpecificationHolder.run(handler, gResult);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,10 @@
import com.evolveum.midpoint.report.impl.controller.ExportedReportHeaderRow;
import com.evolveum.midpoint.report.impl.controller.ReportDataWriter;
import com.evolveum.midpoint.schema.GetOperationOptions;
import com.evolveum.midpoint.schema.ObjectHandler;
import com.evolveum.midpoint.schema.SearchResultList;
import com.evolveum.midpoint.schema.SelectorOptions;
import com.evolveum.midpoint.schema.result.OperationResult;
import com.evolveum.midpoint.util.Handler;
import com.evolveum.midpoint.xml.ns._public.common.audit_3.AuditEventRecordType;
import com.evolveum.midpoint.util.exception.CommonException;
import com.evolveum.midpoint.xml.ns._public.common.common_3.ObjectType;
Expand Down Expand Up @@ -71,30 +71,30 @@ public void saveReportFile(ReportDataWriter<? extends ExportedReportDataRow, ? e
public void searchRecordsIteratively(
Class<? extends Containerable> type,
ObjectQuery query,
Handler<Containerable> handler,
ObjectHandler<Containerable> handler,
Collection<SelectorOptions<GetOperationOptions>> options,
OperationResult result) throws CommonException {
if (AuditEventRecordType.class.equals(type)) {
modelAuditService.searchObjectsIterative(
query,
options,
(value, lResult) -> handler.handle(value),
handler::handle,
runningTask,
result);
} else if (ObjectType.class.isAssignableFrom(type)) {
Class<? extends ObjectType> objectType = type.asSubclass(ObjectType.class);
modelService.searchObjectsIterative(
objectType,
query,
(object, lResult) -> handler.handle(object.asObjectable()),
(object, lResult) -> handler.handle(object.asObjectable(), lResult),
options,
runningTask,
result);
} else {
// Temporary - until iterative search is available
SearchResultList<? extends Containerable> containers =
SearchResultList<? extends Containerable> values =
modelService.searchContainers(type, query, options, runningTask, result);
containers.forEach(handler::handle);
values.forEach(value -> handler.handle(value, result));
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import com.evolveum.midpoint.repo.common.activity.run.SearchSpecification;
import com.evolveum.midpoint.report.impl.controller.*;

import com.evolveum.midpoint.schema.ObjectHandler;
import com.evolveum.midpoint.schema.SearchResultList;
import com.evolveum.midpoint.xml.ns._public.common.audit_3.AuditEventRecordType;
import com.evolveum.midpoint.xml.ns._public.common.common_3.*;
Expand All @@ -38,7 +39,6 @@
import com.evolveum.midpoint.schema.SelectorOptions;
import com.evolveum.midpoint.schema.result.OperationResult;
import com.evolveum.midpoint.task.api.RunningTask;
import com.evolveum.midpoint.util.Handler;
import com.evolveum.midpoint.util.exception.*;
import com.evolveum.midpoint.util.logging.Trace;
import com.evolveum.midpoint.util.logging.TraceManager;
Expand Down Expand Up @@ -139,7 +139,7 @@ private void initializeController(OperationResult result) throws CommonException
}

private void initializeAuditReportBucketing(OperationResult result)
throws SchemaException, ObjectNotFoundException {
throws SchemaException {
ObjectFilter filter = masterSearchSpecification.getQuery().getFilter();
XMLGregorianCalendar reportFrom = null;
XMLGregorianCalendar reportTo = null;
Expand Down Expand Up @@ -188,10 +188,10 @@ private static XMLGregorianCalendar getTimestampFromFilter(ObjectFilter filter,

private static <T extends PrismValue> Collection<T> getTimestampsFromFilter(ObjectFilter filter, boolean greaterOrLess) {
if (greaterOrLess && filter instanceof GreaterFilter
&& AuditEventRecordType.F_TIMESTAMP.equivalent(((GreaterFilter) filter).getFullPath())) {
&& AuditEventRecordType.F_TIMESTAMP.equivalent(((GreaterFilter<?>) filter).getFullPath())) {
return ((GreaterFilter) filter).getValues();
} else if (!greaterOrLess && filter instanceof LessFilter
&& AuditEventRecordType.F_TIMESTAMP.equivalent(((LessFilter) filter).getFullPath())) {
&& AuditEventRecordType.F_TIMESTAMP.equivalent(((LessFilter<?>) filter).getFullPath())) {
return ((LessFilter) filter).getValues();
} else if (filter instanceof AndFilter || filter instanceof OrFilter) {
return getTimestampsFromFilter(((NaryLogicalFilter) filter).getConditions(), greaterOrLess);
Expand Down Expand Up @@ -259,7 +259,7 @@ public void initialize(Class<Containerable> type, ObjectQuery query, Collection<
}

@Override
public void run(Handler<Containerable> handler, OperationResult result) {
public void run(ObjectHandler<Containerable> handler, OperationResult result) {
// no-op
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,9 @@
import com.evolveum.midpoint.prism.query.ObjectQuery;
import com.evolveum.midpoint.report.impl.activity.ExportActivitySupport;
import com.evolveum.midpoint.schema.GetOperationOptions;
import com.evolveum.midpoint.schema.ObjectHandler;
import com.evolveum.midpoint.schema.SelectorOptions;
import com.evolveum.midpoint.schema.result.OperationResult;
import com.evolveum.midpoint.util.Handler;

import com.evolveum.midpoint.util.exception.CommonException;

Expand Down Expand Up @@ -56,7 +56,7 @@ public Collection<SelectorOptions<GetOperationOptions>> getOptions() {
}

@Override
public void run(Handler<Containerable> handler, OperationResult result) throws CommonException {
public void run(ObjectHandler<Containerable> handler, OperationResult result) throws CommonException {
support.searchRecordsIteratively(type, query, handler, options, result);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,9 @@
import com.evolveum.midpoint.prism.Containerable;
import com.evolveum.midpoint.prism.query.ObjectQuery;
import com.evolveum.midpoint.schema.GetOperationOptions;
import com.evolveum.midpoint.schema.ObjectHandler;
import com.evolveum.midpoint.schema.SelectorOptions;
import com.evolveum.midpoint.schema.result.OperationResult;
import com.evolveum.midpoint.util.Handler;
import com.evolveum.midpoint.util.exception.CommonException;

import java.util.Collection;
Expand All @@ -30,5 +30,5 @@ public interface ReportDataSource<C extends Containerable> {
/**
* Executes the search and feeds the handler with the data.
*/
void run(Handler<C> handler, OperationResult result) throws CommonException;
void run(ObjectHandler<C> handler, OperationResult result) throws CommonException;
}
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,7 @@ private <S extends ShadowType> void executeSearchForEntitlements(

SearchHierarchyConstraints searchHierarchyConstraints = determineSearchHierarchyConstraints(entitlementCtx, result);

ObjectHandler handler = (ucfObject, lResult) -> {
UcfObjectHandler handler = (ucfObject, lResult) -> {
PrismObject<ShadowType> entitlementResourceObject = ucfObject.getResourceObject();

try {
Expand Down Expand Up @@ -642,7 +642,7 @@ <T> void collectEntitlementsAsObjectOperationDelete(

SearchHierarchyConstraints searchHierarchyConstraints = determineSearchHierarchyConstraints(entitlementCtx, result);

ObjectHandler handler = (ucfObject, lResult) -> {
UcfObjectHandler handler = (ucfObject, lResult) -> {
PrismObject<ShadowType> entitlementShadow = ucfObject.getResourceObject();
Collection<? extends ResourceAttribute<?>> primaryIdentifiers = ShadowUtil.getPrimaryIdentifiers(entitlementShadow);
ResourceObjectDiscriminator disc = new ResourceObjectDiscriminator(entitlementDef.getTypeName(), primaryIdentifiers);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ public PrismObject<ShadowType> locateResourceObject(ProvisioningContext ctx,
.itemWithDef(secondaryIdentifierDef, ShadowType.F_ATTRIBUTES, secondaryIdentifierDef.getItemName()).eq(secondaryIdentifierValue)
.build();
final Holder<PrismObject<ShadowType>> shadowHolder = new Holder<>();
ObjectHandler handler = (ucfObject, result) -> {
UcfObjectHandler handler = (ucfObject, result) -> {
if (!shadowHolder.isEmpty()) {
throw new IllegalStateException("More than one value found for secondary identifier "+finalSecondaryIdentifier);
}
Expand Down Expand Up @@ -321,7 +321,7 @@ public AsynchronousOperationReturnValue<PrismObject<ShadowType>> addResourceObje
* which we want to add is already present in the backing store. In case of manual provisioning the resource
* itself will not indicate "already exist" error. We have to explicitly check for that.
*/
private void checkForAddConflicts(ProvisioningContext ctx, PrismObject<ShadowType> shadow, OperationResult result) throws ObjectAlreadyExistsException, SchemaException, CommunicationException, ConfigurationException, SecurityViolationException, ExpressionEvaluationException, ObjectNotFoundException {
private void checkForAddConflicts(ProvisioningContext ctx, PrismObject<ShadowType> shadow, OperationResult result) throws ObjectAlreadyExistsException, SchemaException, CommunicationException, ConfigurationException, SecurityViolationException, ExpressionEvaluationException {
if (LOGGER.isTraceEnabled()) {
LOGGER.trace("Checking for add conflicts for {}", ShadowUtil.shortDumpShadow(shadow));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,7 @@ PrismObject<ShadowType> fetchObject(ResourceObjectIdentification resourceObjectI
SearchResultMetadata search(
@NotNull ResourceObjectDefinition objectDefinition,
@Nullable ObjectQuery query,
@NotNull ObjectHandler handler,
@NotNull UcfObjectHandler handler,
@Nullable AttributesToReturn attributesToReturn,
@Nullable PagedSearchCapabilityType pagedSearchConfiguration,
@Nullable SearchHierarchyConstraints searchHierarchyConstraints,
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
import com.evolveum.midpoint.prism.PrismObject;
import com.evolveum.midpoint.prism.query.ObjectQuery;
import com.evolveum.midpoint.schema.internals.InternalsConfig;
import com.evolveum.midpoint.schema.processor.ResourceObjectClassDefinition;
import com.evolveum.midpoint.schema.processor.ResourceObjectDefinition;
import com.evolveum.midpoint.schema.processor.SearchHierarchyConstraints;
import com.evolveum.midpoint.schema.result.OperationResult;
import com.evolveum.midpoint.util.DebugDumpable;
Expand All @@ -24,8 +24,9 @@
import static com.evolveum.midpoint.util.MiscUtil.stateCheck;

/**
* Represents a resource object (e.g. an account) found by {@link ConnectorInstance#search(ResourceObjectClassDefinition, ObjectQuery, ObjectHandler, AttributesToReturn, PagedSearchCapabilityType, SearchHierarchyConstraints, UcfFetchErrorReportingMethod, Task, OperationResult)}.
* operation.
* Represents a resource object (e.g. an account) found by {@link ConnectorInstance#search(ResourceObjectDefinition, ObjectQuery,
* UcfObjectHandler, AttributesToReturn, PagedSearchCapabilityType, SearchHierarchyConstraints, UcfFetchErrorReportingMethod,
* UcfExecutionContext, OperationResult)} operation.
*
* See also {@link UcfChange}.
*/
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
/*
* Copyright (c) 2010-2017 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.provisioning.ucf.api;

import com.evolveum.midpoint.schema.ObjectHandler;

/**
* Handles UCF objects, typically coming from iterative search.
*
* @author Radovan Semancik
*/
@FunctionalInterface
public interface UcfObjectHandler extends ObjectHandler<UcfObjectFound> {

}
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,7 @@ public PrismObject<ShadowType> fetchObject(ResourceObjectIdentification resource
@Override
public SearchResultMetadata search(
@NotNull ResourceObjectDefinition objectDefinition, ObjectQuery query,
@NotNull ObjectHandler handler, @Nullable AttributesToReturn attributesToReturn,
@NotNull UcfObjectHandler handler, @Nullable AttributesToReturn attributesToReturn,
@Nullable PagedSearchCapabilityType pagedSearchConfiguration,
@Nullable SearchHierarchyConstraints searchHierarchyConstraints,
@Nullable UcfFetchErrorReportingMethod errorReportingMethod,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -351,7 +351,7 @@ public PrismObject<ShadowType> fetchObject(ResourceObjectIdentification resource

@Override
public SearchResultMetadata search(@NotNull ResourceObjectDefinition objectDefinition, ObjectQuery query,
@NotNull ObjectHandler handler, @Nullable AttributesToReturn attributesToReturn,
@NotNull UcfObjectHandler handler, @Nullable AttributesToReturn attributesToReturn,
@Nullable PagedSearchCapabilityType pagedSearchConfiguration, @Nullable SearchHierarchyConstraints searchHierarchyConstraints,
@Nullable UcfFetchErrorReportingMethod ucfErrorReportingMethod,
@NotNull UcfExecutionContext ctx, @NotNull OperationResult parentResult) {
Expand Down

0 comments on commit 27de9e7

Please sign in to comment.