Skip to content

Commit

Permalink
Reload lens context if there's a focus change
Browse files Browse the repository at this point in the history
If the focus is changed during lens context loading - presumably
because of the discovery process - we will rot the context and re-load
it. This is to avoid continuing with the obsolete data, because that
can lead to inconsistent results, as demonstrated
by TestVeryAuthoritativeSource (and MID-7725).

This resolves MID-7725.

(cherry picked from commit bb2a191)
  • Loading branch information
mederly committed Mar 23, 2022
1 parent 557aea9 commit f4a0fde
Show file tree
Hide file tree
Showing 4 changed files with 84 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import com.evolveum.midpoint.schema.result.OperationResult;
import com.evolveum.midpoint.task.api.Task;
import com.evolveum.midpoint.util.DebugUtil;
import com.evolveum.midpoint.util.annotation.Experimental;
import com.evolveum.midpoint.util.exception.*;
import com.evolveum.midpoint.util.logging.Trace;
import com.evolveum.midpoint.util.logging.TraceManager;
Expand All @@ -35,15 +36,13 @@
import org.jetbrains.annotations.NotNull;

import javax.xml.datatype.XMLGregorianCalendar;
import java.util.Collection;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.*;

import static com.evolveum.midpoint.model.api.ProgressInformation.ActivityType.FOCUS_OPERATION;
import static com.evolveum.midpoint.model.api.ProgressInformation.StateType.ENTERING;
import static com.evolveum.midpoint.model.impl.lens.ChangeExecutor.OPERATION_EXECUTE_FOCUS;
import static com.evolveum.midpoint.prism.PrismContainerValue.asContainerables;
import static com.evolveum.midpoint.util.MiscUtil.stateCheck;

/**
* Executes changes in the focus context.
Expand All @@ -70,6 +69,12 @@ public class FocusChangeExecution<O extends ObjectType> {
*/
private ObjectDelta<O> focusDelta;

/** Just to detect leaking of the listeners. */
private static final int MAX_LISTENERS_PER_THREAD = 100;

private static final ThreadLocal<Set<ChangeExecutionListener>> CHANGE_EXECUTION_LISTENERS_TL =
ThreadLocal.withInitial(HashSet::new);

public FocusChangeExecution(@NotNull LensContext<O> context, @NotNull LensFocusContext<O> focusContext,
@NotNull Task task, @NotNull ModelBeans modelBeans) {
this.context = context;
Expand Down Expand Up @@ -125,6 +130,7 @@ public void execute(OperationResult parentResult) throws SchemaException,
} finally {
result.computeStatusIfUnknown();
context.reportProgress(new ProgressInformation(FOCUS_OPERATION, result));
notifyChangeExecutionListeners();
}
}

Expand Down Expand Up @@ -305,4 +311,37 @@ private void processItemConstraint(PrismObject<O> objectNew, ItemConstraintType
focusContext.setOid(String.valueOf(prop.getRealValue()));
}
}

private void notifyChangeExecutionListeners() {
String oid = focusContext.getOid(); // Even for ADD operations the new OID should be here.
if (oid == null) {
return; // Something must have gone wrong
}

for (ChangeExecutionListener listener : CHANGE_EXECUTION_LISTENERS_TL.get()) {
// We don't expect any exceptions to be thrown
listener.onFocusChange(oid);
}
}

/** Must be accompanied by respective unregister call! */
public static void registerChangeExecutionListener(@NotNull ChangeExecutionListener listener) {
Set<ChangeExecutionListener> listeners = CHANGE_EXECUTION_LISTENERS_TL.get();
listeners.add(listener);
stateCheck(listeners.size() <= MAX_LISTENERS_PER_THREAD,
"Leaking change execution listeners in %s: %s", Thread.currentThread(), listeners);
}

public static void unregisterChangeExecutionListener(@NotNull ChangeExecutionListener listener) {
CHANGE_EXECUTION_LISTENERS_TL.get().remove(listener);
}

/**
* Receives notifications when focus object is modified (or added, or deleted).
* Should be fast and should throw no exceptions.
*/
@Experimental
public interface ChangeExecutionListener {
void onFocusChange(@NotNull String oid);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,13 @@
import static com.evolveum.midpoint.schema.GetOperationOptions.createReadOnlyCollection;
import static com.evolveum.midpoint.util.DebugUtil.debugDumpLazily;

import java.util.HashSet;
import java.util.List;
import java.util.Objects;
import java.util.Set;
import javax.xml.datatype.XMLGregorianCalendar;

import com.evolveum.midpoint.model.impl.lens.executor.FocusChangeExecution;
import com.evolveum.midpoint.prism.delta.ObjectDelta;

import org.jetbrains.annotations.NotNull;
Expand Down Expand Up @@ -64,13 +67,45 @@ public class ContextLoader implements ProjectorProcessor {

public static final String CLASS_DOT = ContextLoader.class.getName() + ".";

/**
* How many times do we try to load the context: the load is repeated if the focus
* is updated by an embedded clockwork run (presumably due to discovery process).
*/
private static final int MAX_LOAD_ATTEMPTS = 3;

/**
* Loads the whole context.
*
* The loading is repeated if the focus is modified during the `load` operation. See MID-7725.
*/
@ProcessorMethod
<F extends ObjectType> void load(@NotNull LensContext<F> context, @NotNull String activityDescription,
@SuppressWarnings("unused") XMLGregorianCalendar now, @NotNull Task task, @NotNull OperationResult parentResult)
throws SchemaException, ObjectNotFoundException, CommunicationException, ConfigurationException,
SecurityViolationException, PolicyViolationException, ExpressionEvaluationException {
new ContextLoadOperation<>(context, activityDescription, task)
.load(parentResult);

for (int attempt = 1; attempt <= MAX_LOAD_ATTEMPTS; attempt++) {
Set<String> modifiedOids = new HashSet<>();
FocusChangeExecution.ChangeExecutionListener listener = modifiedOids::add;
FocusChangeExecution.registerChangeExecutionListener(listener);
try {
new ContextLoadOperation<>(context, activityDescription, task)
.load(parentResult);
} finally {
FocusChangeExecution.unregisterChangeExecutionListener(listener);
}
LOGGER.trace("Focus OID/OIDs modified during load operation in this thread: {}", modifiedOids);
LensFocusContext<F> focusContext = context.getFocusContext();
if (focusContext != null && focusContext.getOid() != null && modifiedOids.contains(focusContext.getOid())) {
LOGGER.debug("Detected modification of the focus during 'load' operation, retrying the loading (#{})", attempt);
context.rot("focus modification during loading");
} else {
LOGGER.trace("No modification of the focus during 'load' operation, continuing");
return;
}
}
LOGGER.warn("Focus was repeatedly modified during loading ({} times) - continuing, but it's suspicious",
MAX_LOAD_ATTEMPTS);
}

public <O extends ObjectType> void loadFocusContext(LensContext<O> context, Task task, OperationResult parentResult)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@
</expression>
<target>
<path>description</path>
<set>
<predefined>all</predefined>
</set>
</target>
</mapping>
</objectTemplate>
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,6 @@
<name>template-global</name>
<documentation>
Standard object template. It creates an assignment to `role-target` if `description` is not null.

Workaround for MID-7725: If there is no source account, `description` is considered to be effectively null.
</documentation>
<mapping>
<documentation>Non-null description leads to assignment of `role-target`.</documentation>
Expand Down Expand Up @@ -39,8 +37,7 @@
<condition>
<script>
<code>
// Here is the workaround for MID-7725
description != null &amp;&amp; midpoint.hasLinkedAccount('de33fd4e-753a-43a6-bda7-cf1f2576c885')
description != null
</code>
</script>
</condition>
Expand Down

0 comments on commit f4a0fde

Please sign in to comment.