Skip to content

Commit

Permalink
Stop caching of incomplete attributes
Browse files Browse the repository at this point in the history
  • Loading branch information
@mederly
mederly committed Mar 18, 2019
1 parent 1f2c7a8 commit 18cd459
Show file tree
Hide file tree
Showing 10 changed files with 203 additions and 86 deletions.
14 changes: 12 additions & 2 deletions ...ummy-connector/src/main/java/com/evolveum/icf/dummy/connector/AbstractDummyConnector.java
View file
Expand Up @@ -1213,8 +1213,18 @@ private ConnectorObjectBuilder createConnectorObjectBuilderCommon(DummyObject du
if (configuration.isVaryLetterCase()) {
name = varyLetterCase(name);
}
if (values != null && !values.isEmpty()) {
builder.addAttribute(name, values);
AttributeBuilder attributeBuilder = new AttributeBuilder();
attributeBuilder.setName(name);
attributeBuilder.addValue(values);
boolean store;
if (attrDef.isReturnedAsIncomplete()) {
attributeBuilder.setAttributeValueCompleteness(AttributeValueCompleteness.INCOMPLETE);
store = true;
} else {
store = values != null && !values.isEmpty();
}
if (store) {
builder.addAttribute(attributeBuilder.build());
}
}

Expand Down
8 changes: 8 additions & 0 deletions ...ummy-resource/src/main/java/com/evolveum/icf/dummy/resource/DummyAttributeDefinition.java
View file
Expand Up @@ -26,6 +26,7 @@ public class DummyAttributeDefinition {
private boolean isRequired;
private boolean isMulti;
private boolean isReturnedByDefault = true;
private boolean isReturnedAsIncomplete;

public DummyAttributeDefinition(String attributeName, Class<?> attributeType) {
super();
Expand Down Expand Up @@ -84,4 +85,11 @@ public void setReturnedByDefault(boolean isReturnedByDefault) {
this.isReturnedByDefault = isReturnedByDefault;
}

public boolean isReturnedAsIncomplete() {
return isReturnedAsIncomplete;
}

public void setReturnedAsIncomplete(boolean returnedAsIncomplete) {
isReturnedAsIncomplete = returnedAsIncomplete;
}
}
14 changes: 7 additions & 7 deletions ...onnectors/dummy-resource/src/main/java/com/evolveum/icf/dummy/resource/DummyResource.java
View file
Expand Up @@ -148,22 +148,22 @@ public class DummyResource implements DebugDumpable {
private static Map<String, DummyResource> instances = new HashMap<>();

DummyResource() {
allObjects = Collections.synchronizedMap(new LinkedHashMap<String,DummyObject>());
accounts = Collections.synchronizedMap(new LinkedHashMap<String, DummyAccount>());
groups = Collections.synchronizedMap(new LinkedHashMap<String, DummyGroup>());
privileges = Collections.synchronizedMap(new LinkedHashMap<String, DummyPrivilege>());
orgs = Collections.synchronizedMap(new LinkedHashMap<String, DummyOrg>());
allObjects = Collections.synchronizedMap(new LinkedHashMap<>());
accounts = Collections.synchronizedMap(new LinkedHashMap<>());
groups = Collections.synchronizedMap(new LinkedHashMap<>());
privileges = Collections.synchronizedMap(new LinkedHashMap<>());
orgs = Collections.synchronizedMap(new LinkedHashMap<>());
scriptHistory = new ArrayList<>();
accountObjectClass = new DummyObjectClass();
groupObjectClass = new DummyObjectClass();
privilegeObjectClass = new DummyObjectClass();
syncStyle = DummySyncStyle.NONE;
deltas = Collections.synchronizedList(new ArrayList<DummyDelta>());
deltas = Collections.synchronizedList(new ArrayList<>());
latestSyncToken = 0;
}

/**
* Clears everything, just like the resouce was just created.
* Clears everything, just like the resource was just created.
*/
public void reset() {
allObjects.clear();
Expand Down
4 changes: 4 additions & 0 deletions infra/prism-api/src/main/java/com/evolveum/midpoint/prism/util/PrismAsserts.java
View file
Expand Up @@ -33,6 +33,7 @@
import com.evolveum.midpoint.prism.query.*;
import com.evolveum.midpoint.prism.xnode.*;
import com.evolveum.midpoint.util.QNameUtil;
import org.apache.commons.collections4.CollectionUtils;
import org.jetbrains.annotations.NotNull;
import org.w3c.dom.Element;

Expand Down Expand Up @@ -880,6 +881,9 @@ public static <T> void assertSets(String message, MatchingRule<T> matchingRule,
}

public static <T> void assertSets(String message, MatchingRule<T> matchingRule, Collection<T> actualValues, Collection<T> expectedValues) throws SchemaException {
if (CollectionUtils.isEmpty(expectedValues) && CollectionUtils.isEmpty(actualValues)) {
return;
}
assertNotNull("Null set in " + message, actualValues);
assertEquals("Wrong number of values in " + message+ "; expected (real values) "
+PrettyPrinter.prettyPrint(expectedValues)+"; has (pvalues) "+actualValues,
Expand Down
3 changes: 1 addition & 2 deletions infra/prism-impl/src/main/java/com/evolveum/midpoint/prism/impl/PrismPropertyImpl.java
View file
Expand Up @@ -253,8 +253,7 @@ public void addValue(PrismPropertyValue<T> pValueToAdd) {
}

public void addRealValue(T valueToAdd) {
PrismPropertyValue<T> pval = new PrismPropertyValueImpl<>(valueToAdd);
addValue(pval);
addValue(new PrismPropertyValueImpl<>(valueToAdd));
}

public void addRealValues(T... valuesToAdd) {
Expand Down
122 changes: 72 additions & 50 deletions ...rovisioning-impl/src/main/java/com/evolveum/midpoint/provisioning/impl/ShadowManager.java
View file
Expand Up @@ -16,10 +16,7 @@

package com.evolveum.midpoint.provisioning.impl;

import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.List;
import java.util.*;

import javax.xml.datatype.XMLGregorianCalendar;
import javax.xml.namespace.QName;
Expand Down Expand Up @@ -1560,7 +1557,7 @@ private Collection<? extends ItemDelta> extractRepoShadowChanges(ProvisioningCon
PropertyDelta<PolyString> nameDelta = prismContext.deltaFactory().property().createReplaceDelta(shadow.getDefinition(), ShadowType.F_NAME, new PolyString(newName));
repoChanges.add(nameDelta);
}
if (!ProvisioningUtil.shouldStoreAtributeInShadow(objectClassDefinition, attrName, cachingStrategy)) {
if (!ProvisioningUtil.shouldStoreAttributeInShadow(objectClassDefinition, attrName, cachingStrategy)) {
continue;
}
} else if (ShadowType.F_ACTIVATION.equivalent(itemDelta.getParentPath())) {
Expand Down Expand Up @@ -1650,7 +1647,7 @@ private void computeUpdateShadowAttributeChanges(ProvisioningContext ctx, Collec
RefinedObjectClassDefinition objectClassDefinition = ctx.getObjectClassDefinition();
CachingStategyType cachingStrategy = ProvisioningUtil.getCachingStrategy(ctx);
for (RefinedAttributeDefinition attrDef: objectClassDefinition.getAttributeDefinitions()) {
if (ProvisioningUtil.shouldStoreAtributeInShadow(objectClassDefinition, attrDef.getName(), cachingStrategy)) {
if (ProvisioningUtil.shouldStoreAttributeInShadow(objectClassDefinition, attrDef.getName(), cachingStrategy)) {
ResourceAttribute<Object> resourceAttr = ShadowUtil.getAttribute(resourceShadow, attrDef.getName());
PrismProperty<Object> repoAttr = repoShadow.findProperty(ItemPath.create(ShadowType.F_ATTRIBUTES, attrDef.getName()));
PropertyDelta attrDelta;
Expand Down Expand Up @@ -1722,54 +1719,75 @@ private ObjectDelta<ShadowType> computeShadowDelta(@NotNull ProvisioningContext

CachingStategyType cachingStrategy = ProvisioningUtil.getCachingStrategy(ctx);

Collection<QName> incompleteCacheableItems = new HashSet<>();

for (Item<?, ?> currentResourceItem: currentResourceAttributesContainer.getValue().getItems()) {
if (currentResourceItem instanceof PrismProperty<?>) {
//noinspection unchecked
PrismProperty<Object> currentResourceAttrProperty = (PrismProperty<Object>) currentResourceItem;
RefinedAttributeDefinition<Object> attrDef = ocDef.findAttributeDefinition(currentResourceAttrProperty.getElementName());
if (ProvisioningUtil.shouldStoreAtributeInShadow(ocDef, attrDef.getName(), cachingStrategy)) {
MatchingRule matchingRule = matchingRuleRegistry.getMatchingRule(attrDef.getMatchingRuleQName(), attrDef.getTypeName());
PrismProperty<Object> oldRepoAttributeProperty = oldRepoAttributesContainer.findProperty(currentResourceAttrProperty.getElementName());
if (oldRepoAttributeProperty == null) {
PropertyDelta<Object> attrAddDelta = currentResourceAttrProperty.createDelta();
for (PrismPropertyValue<?> pval: currentResourceAttrProperty.getValues()) {
attrAddDelta.addRealValuesToAdd(matchingRule.normalize(pval.getValue()));
}
if (attrAddDelta.getDefinition().getTypeName() == null) {
throw new SchemaException("No definition in "+attrAddDelta);
}
shadowDelta.addModification(attrAddDelta);
} else {
if (attrDef.isSingleValue()) {
Object currentResourceRealValue = currentResourceAttrProperty.getRealValue();
Object currentResourceNormalizedRealValue;
currentResourceNormalizedRealValue = matchingRule.normalize(currentResourceRealValue);
if (!currentResourceNormalizedRealValue.equals(oldRepoAttributeProperty.getRealValue())) {
PropertyDelta delta = shadowDelta.addModificationReplaceProperty(currentResourceAttrProperty.getPath(), currentResourceNormalizedRealValue);
delta.setDefinition(currentResourceAttrProperty.getDefinition());
if (delta.getDefinition().getTypeName() == null) {
throw new SchemaException("No definition in "+delta);
}
if (attrDef == null) {
throw new SchemaException("No definition of " + currentResourceAttrProperty.getElementName() + " in " + ocDef);
}
if (ProvisioningUtil.shouldStoreAttributeInShadow(ocDef, attrDef.getName(), cachingStrategy)) {
if (!currentResourceItem.isIncomplete()) {
MatchingRule matchingRule = matchingRuleRegistry.getMatchingRule(attrDef.getMatchingRuleQName(), attrDef.getTypeName());
PrismProperty<Object> oldRepoAttributeProperty = oldRepoAttributesContainer.findProperty(currentResourceAttrProperty.getElementName());
if (oldRepoAttributeProperty == null) {
PropertyDelta<Object> attrAddDelta = currentResourceAttrProperty.createDelta();
for (PrismPropertyValue<?> pval : currentResourceAttrProperty.getValues()) {
//noinspection unchecked
attrAddDelta.addRealValuesToAdd(matchingRule.normalize(pval.getValue()));
}
} else {
PrismProperty<Object> normalizedCurrentResourceAttrProperty = currentResourceAttrProperty.clone();
for (PrismPropertyValue pval: normalizedCurrentResourceAttrProperty.getValues()) {
Object normalizedRealValue = matchingRule.normalize(pval.getValue());
pval.setValue(normalizedRealValue);
if (attrAddDelta.getDefinition().getTypeName() == null) {
throw new SchemaException("No definition in " + attrAddDelta);
}
PropertyDelta<Object> attrDiff = oldRepoAttributeProperty.diff(normalizedCurrentResourceAttrProperty);
// LOGGER.trace("DIFF:\n{}\n-\n{}\n=:\n{}",
// oldRepoAttributeProperty==null?null:oldRepoAttributeProperty.debugDump(1),
// normalizedCurrentResourceAttrProperty==null?null:normalizedCurrentResourceAttrProperty.debugDump(1),
// attrDiff==null?null:attrDiff.debugDump(1));
if (attrDiff != null && !attrDiff.isEmpty()) {
attrDiff.setParentPath(ShadowType.F_ATTRIBUTES);
if (attrDiff.getDefinition().getTypeName() == null) {
throw new SchemaException("No definition in "+attrDiff);
shadowDelta.addModification(attrAddDelta);
} else {
if (attrDef.isSingleValue()) {
Object currentResourceRealValue = currentResourceAttrProperty.getRealValue();
//noinspection unchecked
Object currentResourceNormalizedRealValue = matchingRule.normalize(currentResourceRealValue);
if (!Objects.equals(currentResourceNormalizedRealValue, oldRepoAttributeProperty.getRealValue())) {
PropertyDelta delta;
if (currentResourceNormalizedRealValue != null) {
delta = shadowDelta.addModificationReplaceProperty(currentResourceAttrProperty.getPath(),
currentResourceNormalizedRealValue);
} else {
delta = shadowDelta.addModificationReplaceProperty(currentResourceAttrProperty.getPath());
}
//noinspection unchecked
delta.setDefinition(currentResourceAttrProperty.getDefinition());
if (delta.getDefinition().getTypeName() == null) {
throw new SchemaException("No definition in " + delta);
}
}
} else {
PrismProperty<Object> normalizedCurrentResourceAttrProperty = currentResourceAttrProperty.clone();
for (PrismPropertyValue pval : normalizedCurrentResourceAttrProperty.getValues()) {
//noinspection unchecked
Object normalizedRealValue = matchingRule.normalize(pval.getValue());
//noinspection unchecked
pval.setValue(normalizedRealValue);
}
PropertyDelta<Object> attrDiff = oldRepoAttributeProperty.diff(normalizedCurrentResourceAttrProperty);
// LOGGER.trace("DIFF:\n{}\n-\n{}\n=:\n{}",
// oldRepoAttributeProperty==null?null:oldRepoAttributeProperty.debugDump(1),
// normalizedCurrentResourceAttrProperty==null?null:normalizedCurrentResourceAttrProperty.debugDump(1),
// attrDiff==null?null:attrDiff.debugDump(1));
if (attrDiff != null && !attrDiff.isEmpty()) {
attrDiff.setParentPath(ShadowType.F_ATTRIBUTES);
if (attrDiff.getDefinition().getTypeName() == null) {
throw new SchemaException("No definition in " + attrDiff);
}
shadowDelta.addModification(attrDiff);
}
shadowDelta.addModification(attrDiff);
}

}
} else {
LOGGER.trace("Resource item {} is incomplete, will not update the shadow with its content",
currentResourceItem.getElementName());
incompleteCacheableItems.add(currentResourceItem.getElementName());
}
}
}
Expand All @@ -1780,7 +1798,8 @@ private ObjectDelta<ShadowType> computeShadowDelta(@NotNull ProvisioningContext
PrismProperty<?> oldRepoAttrProperty = (PrismProperty<?>)oldRepoItem;
RefinedAttributeDefinition<Object> attrDef = ocDef.findAttributeDefinition(oldRepoAttrProperty.getElementName());
PrismProperty<Object> currentAttribute = currentResourceAttributesContainer.findProperty(oldRepoAttrProperty.getElementName());
if (attrDef == null || !ProvisioningUtil.shouldStoreAtributeInShadow(ocDef, attrDef.getName(), cachingStrategy) ||
// note: incomplete attributes with no values are not here: they are found in currentResourceAttributesContainer
if (attrDef == null || !ProvisioningUtil.shouldStoreAttributeInShadow(ocDef, attrDef.getName(), cachingStrategy) ||
currentAttribute == null) {
// No definition for this property it should not be there or no current value: remove it from the shadow
PropertyDelta<?> oldRepoAttrPropDelta = oldRepoAttrProperty.createDelta();
Expand Down Expand Up @@ -1828,10 +1847,13 @@ private ObjectDelta<ShadowType> computeShadowDelta(@NotNull ProvisioningContext
compareUpdateProperty(shadowDelta, SchemaConstants.PATH_ACTIVATION_VALID_TO, resourceShadowNew, repoShadowOld);
compareUpdateProperty(shadowDelta, SchemaConstants.PATH_ACTIVATION_LOCKOUT_STATUS, resourceShadowNew, repoShadowOld);

CachingMetadataType cachingMetadata = new CachingMetadataType();
cachingMetadata.setRetrievalTimestamp(clock.currentTimeXMLGregorianCalendar());
shadowDelta.addModificationReplaceProperty(ShadowType.F_CACHING_METADATA, cachingMetadata);

if (incompleteCacheableItems.isEmpty()) {
CachingMetadataType cachingMetadata = new CachingMetadataType();
cachingMetadata.setRetrievalTimestamp(clock.currentTimeXMLGregorianCalendar());
shadowDelta.addModificationReplaceProperty(ShadowType.F_CACHING_METADATA, cachingMetadata);
} else {
LOGGER.trace("Shadow has incomplete cacheable items; will not update caching timestamp: {}", incompleteCacheableItems);
}
} else {
throw new ConfigurationException("Unknown caching strategy "+cachingStrategy);
}
Expand Down
2 changes: 1 addition & 1 deletion ...isioning-impl/src/main/java/com/evolveum/midpoint/provisioning/util/ProvisioningUtil.java
View file
Expand Up @@ -351,7 +351,7 @@ public static void logWarning(Trace logger, OperationResult opResult, String mes
opResult.recordWarning(message, ex);
}

public static boolean shouldStoreAtributeInShadow(RefinedObjectClassDefinition objectClassDefinition, QName attributeName,
public static boolean shouldStoreAttributeInShadow(RefinedObjectClassDefinition objectClassDefinition, QName attributeName,
CachingStategyType cachingStrategy) throws ConfigurationException {
if (cachingStrategy == null || cachingStrategy == CachingStategyType.NONE) {
if (objectClassDefinition.isPrimaryIdentifier(attributeName) || objectClassDefinition.isSecondaryIdentifier(attributeName)) {
Expand Down
8 changes: 8 additions & 0 deletions ...visioning-impl/src/test/java/com/evolveum/midpoint/provisioning/impl/dummy/TestDummy.java
View file
Expand Up @@ -352,6 +352,14 @@ public void test107BGetModifiedAccountFromCacheHighStaleness() throws Exception
assertSteadyResource();
}

/**
* Incomplete attributes should not be cached.
*/
@Test
public void test107CSkipCachingForIncompleteAttributes() throws Exception {
// overridden in TestDummyCaching
}

/**
* Staleness of one millisecond is too small for the cache to work.
* Fresh data should be returned - both in case the cache is enabled and disabled.
Expand Down
69 changes: 69 additions & 0 deletions ...ng-impl/src/test/java/com/evolveum/midpoint/provisioning/impl/dummy/TestDummyCaching.java
View file
Expand Up @@ -228,6 +228,75 @@ public void test107BGetModifiedAccountFromCacheHighStaleness() throws Exception
assertSteadyResource();
}

/**
* Incomplete attributes should not be cached.
*/
@Test
public void test107CSkipCachingForIncompleteAttributes() throws Exception {
final String TEST_NAME = "test107CSkipCachingForIncompleteAttributes";
displayTestTitle(TEST_NAME);
// GIVEN
OperationResult result = new OperationResult(TestDummy.class.getName() + "." + TEST_NAME);
rememberCounter(InternalCounters.SHADOW_FETCH_OPERATION_COUNT);

DummyAccount accountWill = getDummyAccountAssert(transformNameFromResource(ACCOUNT_WILL_USERNAME), willIcfUid);
accountWill.replaceAttributeValue(DummyResourceContoller.DUMMY_ACCOUNT_ATTRIBUTE_TITLE_NAME, "Very Nice Pirate");
accountWill.replaceAttributeValue(DummyResourceContoller.DUMMY_ACCOUNT_ATTRIBUTE_SHIP_NAME, null);
accountWill.getAttributeDefinition(DummyResourceContoller.DUMMY_ACCOUNT_ATTRIBUTE_SHIP_NAME).setReturnedAsIncomplete(true);
accountWill.setEnabled(true);

try {
XMLGregorianCalendar startTs = clock.currentTimeXMLGregorianCalendar();

// WHEN
displayWhen(TEST_NAME);

PrismObject<ShadowType> shadow = provisioningService
.getObject(ShadowType.class, ACCOUNT_WILL_OID, null, null, result);

// THEN
displayThen(TEST_NAME);
assertSuccess(result);

assertCounterIncrement(InternalCounters.SHADOW_FETCH_OPERATION_COUNT, 1);

XMLGregorianCalendar endTs = clock.currentTimeXMLGregorianCalendar();

display("Retrieved account shadow", shadow);

assertNotNull("No dummy account", shadow);

assertAttribute(shadow, DummyResourceContoller.DUMMY_ACCOUNT_ATTRIBUTE_TITLE_NAME, "Very Nice Pirate");
assertAttribute(shadow, DummyResourceContoller.DUMMY_ACCOUNT_ATTRIBUTE_SHIP_NAME);
Collection<ResourceAttribute<?>> attributes = ShadowUtil.getAttributes(shadow);
assertEquals("Unexpected number of attributes", 7, attributes.size());

PrismObject<ShadowType> shadowRepo = getShadowRepo(ACCOUNT_WILL_OID);
checkRepoAccountShadowWillBasic(shadowRepo, null, startTs, null);

assertRepoShadowCachedAttributeValue(shadowRepo, DummyResourceContoller.DUMMY_ACCOUNT_ATTRIBUTE_TITLE_NAME,
"Very Nice Pirate");
assertRepoShadowCachedAttributeValue(shadowRepo, DummyResourceContoller.DUMMY_ACCOUNT_ATTRIBUTE_SHIP_NAME,
"Black Pearl");
assertRepoShadowCachedAttributeValue(shadowRepo, DummyResourceContoller.DUMMY_ACCOUNT_ATTRIBUTE_WEAPON_NAME, "sword",
"love");
assertRepoShadowCachedAttributeValue(shadowRepo, DummyResourceContoller.DUMMY_ACCOUNT_ATTRIBUTE_LOOT_NAME, 42);
assertRepoShadowCacheActivation(shadowRepo, ActivationStatusType.ENABLED);

checkUniqueness(shadow);

assertCachingMetadata(shadow, false, null, startTs);

assertCounterIncrement(InternalCounters.SHADOW_FETCH_OPERATION_COUNT, 0);

assertSteadyResource();
} finally {
// cleanup the state to allow other tests to pass
accountWill.replaceAttributeValue(DummyResourceContoller.DUMMY_ACCOUNT_ATTRIBUTE_SHIP_NAME, "Interceptor");
accountWill.getAttributeDefinition(DummyResourceContoller.DUMMY_ACCOUNT_ATTRIBUTE_SHIP_NAME).setReturnedAsIncomplete(false);
}
}

/**
* Search for all accounts with maximum staleness option.
* This is supposed to return only cached data. Therefore
Expand Down

0 comments on commit 18cd459

Please sign in to comment.