Skip to content

Commit

Permalink
Fixing support for null administrativeStatus (MID-3417)
Browse files Browse the repository at this point in the history
  • Loading branch information
@semancik
semancik committed Sep 23, 2016
1 parent 2d611a2 commit 3eaa962
Show file tree
Hide file tree
Showing 12 changed files with 218 additions and 35 deletions.
21 changes: 16 additions & 5 deletions ...ectors/dummy-connector/src/main/java/com/evolveum/icf/dummy/connector/DummyConnector.java
View file
Expand Up @@ -338,7 +338,7 @@ public Uid update(ObjectClass objectClass, Uid uid, Set<Attribute> replaceAttrib
account.setValidTo(getDate(attr));

} else if (attr.is(OperationalAttributes.LOCK_OUT_NAME)) {
account.setLockout(getBoolean(attr));
account.setLockout(getBooleanNotNull(attr));

} else if (PredefinedAttributes.AUXILIARY_OBJECT_CLASS_NAME.equalsIgnoreCase(attr.getName())) {
account.replaceAuxiliaryObjectClassNames(attr.getValue());
Expand Down Expand Up @@ -385,7 +385,7 @@ public Uid update(ObjectClass objectClass, Uid uid, Set<Attribute> replaceAttrib
throw new IllegalArgumentException("Attempt to change password on group");

} else if (attr.is(OperationalAttributes.ENABLE_NAME)) {
group.setEnabled(getBoolean(attr));
group.setEnabled(getBooleanNotNull(attr));

} else {
String name = attr.getName();
Expand Down Expand Up @@ -1691,7 +1691,7 @@ private DummyAccount convertToAccount(Set<Attribute> createAttributes) throws Co
}

} else if (attr.is(OperationalAttributeInfos.LOCK_OUT.getName())) {
Boolean lockout = getBoolean(attr);
Boolean lockout = getBooleanNotNull(attr);
newAccount.setLockout(lockout);

} else {
Expand Down Expand Up @@ -1733,7 +1733,7 @@ private DummyGroup convertToGroup(Set<Attribute> createAttributes) throws Connec
throw new IllegalArgumentException("Password specified for a group");

} else if (attr.is(OperationalAttributeInfos.ENABLE.getName())) {
enabled = getBoolean(attr);
enabled = getBooleanNotNull(attr);
newGroup.setEnabled(enabled);

} else if (attr.is(OperationalAttributeInfos.ENABLE_DATE.getName())) {
Expand Down Expand Up @@ -1829,7 +1829,18 @@ private DummyOrg convertToOrg(Set<Attribute> createAttributes) throws ConnectExc
return newOrg;
}

private boolean getBoolean(Attribute attr) {
private Boolean getBoolean(Attribute attr) {
if (attr.getValue() == null || attr.getValue().isEmpty()) {
return null;
}
Object object = attr.getValue().get(0);
if (!(object instanceof Boolean)) {
throw new IllegalArgumentException("Attribute "+attr.getName()+" was provided as "+object.getClass().getName()+" while expecting boolean");
}
return ((Boolean)object).booleanValue();
}

private boolean getBooleanNotNull(Attribute attr) {
if (attr.getValue() == null || attr.getValue().isEmpty()) {
throw new IllegalArgumentException("Empty "+attr.getName()+" attribute was provided");
}
Expand Down
6 changes: 3 additions & 3 deletions icf-connectors/dummy-resource/src/main/java/com/evolveum/icf/dummy/resource/DummyObject.java
View file
Expand Up @@ -44,7 +44,7 @@ public abstract class DummyObject implements DebugDumpable {
// private int internalId = -1;
private String name;
private Map<String,Set<Object>> attributes = new HashMap<String, Set<Object>>();
private boolean enabled = true;
private Boolean enabled = true;
private Date validFrom = null;
private Date validTo = null;
protected DummyResource resource;
Expand Down Expand Up @@ -84,11 +84,11 @@ public void setName(String username) {
this.name = username;
}

public boolean isEnabled() {
public Boolean isEnabled() {
return enabled;
}

public void setEnabled(boolean enabled) throws ConnectException, FileNotFoundException, SchemaViolationException {
public void setEnabled(Boolean enabled) throws ConnectException, FileNotFoundException, SchemaViolationException {
checkModifyBreak();
this.enabled = enabled;
}
Expand Down
2 changes: 1 addition & 1 deletion model/model-impl/src/main/java/com/evolveum/midpoint/model/impl/lens/LensUtil.java
View file
Expand Up @@ -514,7 +514,7 @@ private static <V extends PrismValue, D extends ItemDefinition> Collection<ItemV
public static PropertyDelta<XMLGregorianCalendar> createActivationTimestampDelta(ActivationStatusType status, XMLGregorianCalendar now,
PrismContainerDefinition<ActivationType> activationDefinition, OriginType origin) {
QName timestampPropertyName;
if (status == ActivationStatusType.ENABLED) {
if (status == null || status == ActivationStatusType.ENABLED) {
timestampPropertyName = ActivationType.F_ENABLE_TIMESTAMP;
} else if (status == ActivationStatusType.DISABLED) {
timestampPropertyName = ActivationType.F_DISABLE_TIMESTAMP;
Expand Down
24 changes: 24 additions & 0 deletions ...-impl/src/main/java/com/evolveum/midpoint/model/impl/lens/projector/MappingEvaluator.java
View file
Expand Up @@ -183,6 +183,10 @@ public <V extends PrismValue, D extends ItemDefinition, T extends ObjectType, F
evaluateMapping(mapping, params.getContext(), task, result);

PrismValueDeltaSetTriple<V> mappingOutputTriple = mapping.getOutputTriple();
if (LOGGER.isTraceEnabled()) {
LOGGER.trace("Output triple of mapping {}\n{}", mapping.getContextDescription(),
mappingOutputTriple==null?null:mappingOutputTriple.debugDump(1));
}
if (mappingOutputTriple != null) {

MappingOutputStruct<V> mappingOutputStruct = outputTripleMap.get(mappingOutputPath);
Expand Down Expand Up @@ -279,6 +283,13 @@ public <V extends PrismValue, D extends ItemDefinition, T extends ObjectType, F
aPrioriTargetItem = aPrioriTargetObject.findItem(mappingOutputPath);
}

// WARNING
// Following code seems to be wrong. It is not very relativisic. It seems to always
// go for replace.
// It seems that it is only used for activation mappings (outbout and inbound). As
// these are quite special single-value properties then it seems to work fine
// (with the exception of MID-3418). Todo: make it more relativistic: MID-3419

if (targetContext.isAdd()) {

Collection<V> nonNegativeValues = outputTriple.getNonNegativeValues();
Expand All @@ -304,6 +315,12 @@ public <V extends PrismValue, D extends ItemDefinition, T extends ObjectType, F
} else {
valuesToReplace = outputTriple.getPlusSet();
}

if (LOGGER.isTraceEnabled()) {
LOGGER.trace("{}: hasFullTargetObject={}, isStrongMappingWasUsed={}, valuesToReplace={}",
new Object[]{mappingDesc, params.hasFullTargetObject(),
mappingOutputStruct.isStrongMappingWasUsed(), valuesToReplace});
}

if (valuesToReplace != null && !valuesToReplace.isEmpty()) {

Expand All @@ -318,6 +335,13 @@ public <V extends PrismValue, D extends ItemDefinition, T extends ObjectType, F
}
}
targetItemDelta.setValuesToReplace(PrismValue.cloneCollection(valuesToReplace));

} else if (outputTriple.hasMinusSet()) {
LOGGER.trace("{} resulted in null or empty value for {} and there is a minus set, resetting it (replace with empty)", mappingDesc, targetContext);
targetItemDelta.setValueToReplace();

} else {
LOGGER.trace("{} resulted in null or empty value for {}, skipping", mappingDesc, targetContext);
}

}
Expand Down
159 changes: 151 additions & 8 deletions model/model-intest/src/test/java/com/evolveum/midpoint/model/intest/TestActivation.java
View file
Expand Up @@ -91,11 +91,15 @@ public class TestActivation extends AbstractInitializedModelIntegrationTest {

private static final File TEST_DIR = new File("src/test/resources/activation");

// This resource does not support native activation. It has simulated activation instead.
// + unusual validTo and validFrom mappings
protected static final File RESOURCE_DUMMY_KHAKI_FILE = new File(TEST_DIR, "resource-dummy-khaki.xml");
protected static final String RESOURCE_DUMMY_KHAKI_OID = "10000000-0000-0000-0000-0000000a1004";
protected static final String RESOURCE_DUMMY_KHAKI_NAME = "khaki";
protected static final String RESOURCE_DUMMY_KHAKI_NAMESPACE = MidPointConstants.NS_RI;

// This resource does not support native activation. It has simulated activation instead.
// + unusual validTo and validFrom mappings
protected static final File RESOURCE_DUMMY_CORAL_FILE = new File(TEST_DIR, "resource-dummy-coral.xml");
protected static final String RESOURCE_DUMMY_CORAL_OID = "10000000-0000-0000-0000-0000000b1004";
protected static final String RESOURCE_DUMMY_CORAL_NAME = "coral";
Expand Down Expand Up @@ -194,8 +198,39 @@ public void test051ModifyUserJackDisable() throws Exception {
}

@Test
public void test052ModifyUserJackEnable() throws Exception {
final String TEST_NAME = "test052ModifyUserJackEnable";
public void test052ModifyUserJackNull() throws Exception {
final String TEST_NAME = "test052ModifyUserJackNull";
TestUtil.displayTestTile(this, TEST_NAME);

// GIVEN
Task task = taskManager.createTaskInstance(TestActivation.class.getName() + "." + TEST_NAME);
OperationResult result = task.getResult();
assumeAssignmentPolicy(AssignmentPolicyEnforcementType.FULL);

XMLGregorianCalendar start = clock.currentTimeXMLGregorianCalendar();

// WHEN
modifyUserReplace(USER_JACK_OID, ACTIVATION_ADMINISTRATIVE_STATUS_PATH, task, result);

// THEN
XMLGregorianCalendar end = clock.currentTimeXMLGregorianCalendar();
result.computeStatus();
TestUtil.assertSuccess(result);

PrismObject<UserType> userJack = getUser(USER_JACK_OID);
display("User after change execution", userJack);
assertUserJack(userJack, "Jack Sparrow");

assertAdministrativeStatus(userJack, null);
assertValidity(userJack, null);
assertEffectiveStatus(userJack, ActivationStatusType.ENABLED);

TestUtil.assertModifyTimestamp(userJack, start, end);
}

@Test
public void test055ModifyUserJackEnable() throws Exception {
final String TEST_NAME = "test055ModifyUserJackEnable";
TestUtil.displayTestTile(this, TEST_NAME);

// GIVEN
Expand Down Expand Up @@ -460,10 +495,10 @@ public void test114ModifyUserJackEnable() throws Exception {

PrismObject<UserType> userJack = getUser(USER_JACK_OID);
display("User after change execution", userJack);
assertUserJack(userJack, "Jack Sparrow");
assertUserJack(userJack, USER_JACK_FULL_NAME);

assertAdministrativeStatusEnabled(userJack);
assertDummyEnabled("jack");
assertDummyEnabled(ACCOUNT_JACK_DUMMY_USERNAME);
assertEnableTimestampFocus(userJack, startTime, endTime);

assertAccounts(USER_JACK_OID, 1);
Expand All @@ -473,13 +508,53 @@ public void test114ModifyUserJackEnable() throws Exception {
assertEnableTimestampShadow(account, startTime, endTime);
}

/**
* Re-enabling the user should enable the account as well. Even if the user is already enabled.
*/
@Test
public void test115ModifyUserJackAdministrativeStatusNull() throws Exception {
final String TEST_NAME = "test115ModifyUserJackAdministrativeStatusNull";
TestUtil.displayTestTile(this, TEST_NAME);

// GIVEN
Task task = taskManager.createTaskInstance(TestActivation.class.getName() + "." + TEST_NAME);
OperationResult result = task.getResult();
assumeAssignmentPolicy(AssignmentPolicyEnforcementType.FULL);
XMLGregorianCalendar startTime = clock.currentTimeXMLGregorianCalendar();

// WHEN
modifyUserReplace(USER_JACK_OID, ACTIVATION_ADMINISTRATIVE_STATUS_PATH, task, result);

// THEN
result.computeStatus();
TestUtil.assertSuccess("executeChanges result", result);
XMLGregorianCalendar endTime = clock.currentTimeXMLGregorianCalendar();

PrismObject<UserType> userJack = getUser(USER_JACK_OID);
display("User after change execution", userJack);
DummyAccount account = getDummyAccount(null, ACCOUNT_JACK_DUMMY_USERNAME);
display("Account after change", account);

assertUserJack(userJack, USER_JACK_FULL_NAME);

assertAdministrativeStatus(userJack, null);
// Dummy account should still be enabled. It does not support validity, therefore
// the account/administrativeStatus is mapped from user.effectiveStatus
assertDummyActivationEnabledState(ACCOUNT_JACK_DUMMY_USERNAME, true);

assertAccounts(USER_JACK_OID, 1);
PrismObject<ShadowType> shadow = getShadowModel(accountOid);
assertAccountShadowModel(shadow, accountOid, ACCOUNT_JACK_DUMMY_USERNAME, resourceDummyType);
assertAdministrativeStatus(shadow, ActivationStatusType.ENABLED);
}

/**
* Modify both user and account activation. As password outbound mapping is weak the user should have its own state
* and account should have its own state.
*/
@Test
public void test115ModifyJackActivationUserAndAccount() throws Exception {
final String TEST_NAME = "test115ModifyJackActivationUserAndAccount";
public void test118ModifyJackActivationUserAndAccount() throws Exception {
final String TEST_NAME = "test118ModifyJackActivationUserAndAccount";
TestUtil.displayTestTile(this, TEST_NAME);

// GIVEN
Expand Down Expand Up @@ -1549,6 +1624,74 @@ public void test350AssignMancombBlueAccount() throws Exception {
assertEquals("Wrong validTo in mancomb blue account", ACCOUNT_MANCOMB_VALID_TO_DATE, mancombBlueAccount.getValidTo());
}

@Test
public void test352AssignMancombBlackAccount() throws Exception {
final String TEST_NAME = "test352AssignMancombBlackAccount";
TestUtil.displayTestTile(this, TEST_NAME);

// GIVEN
Task task = taskManager.createTaskInstance(TestModelServiceContract.class.getName() + "." + TEST_NAME);
OperationResult result = task.getResult();

// WHEN
assignAccount(userMancombOid, RESOURCE_DUMMY_BLACK_OID, null, task, result);

// THEN
result.computeStatus();
TestUtil.assertSuccess(result);

PrismObject<UserType> userMancomb = getUser(userMancombOid);
display("User after change execution", userMancomb);
assertAccounts(userMancombOid, 3);

DummyAccount mancombBlueAccount = getDummyAccount(RESOURCE_DUMMY_BLUE_NAME, ACCOUNT_MANCOMB_DUMMY_USERNAME);
assertNotNull("No mancomb blue account", mancombBlueAccount);
assertTrue("mancomb blue account not enabled", mancombBlueAccount.isEnabled());
assertEquals("Wrong validFrom in mancomb blue account", ACCOUNT_MANCOMB_VALID_FROM_DATE, mancombBlueAccount.getValidFrom());
assertEquals("Wrong validTo in mancomb blue account", ACCOUNT_MANCOMB_VALID_TO_DATE, mancombBlueAccount.getValidTo());

DummyAccount mancombBlackAccount = getDummyAccount(RESOURCE_DUMMY_BLACK_NAME, ACCOUNT_MANCOMB_DUMMY_USERNAME);
assertNotNull("No mancomb black account", mancombBlackAccount);
assertTrue("mancomb black account not enabled", mancombBlackAccount.isEnabled());
assertEquals("Wrong validFrom in mancomb black account", ACCOUNT_MANCOMB_VALID_FROM_DATE, mancombBlackAccount.getValidFrom());
assertEquals("Wrong validTo in mancomb black account", ACCOUNT_MANCOMB_VALID_TO_DATE, mancombBlackAccount.getValidTo());
}

@Test
public void test355MancombModifyAdministrativeStatusNull() throws Exception {
final String TEST_NAME = "test355MancombModifyAdministrativeStatusNull";
TestUtil.displayTestTile(this, TEST_NAME);

// GIVEN
Task task = taskManager.createTaskInstance(TestModelServiceContract.class.getName() + "." + TEST_NAME);
OperationResult result = task.getResult();

// WHEN
modifyUserReplace(userMancombOid, ACTIVATION_ADMINISTRATIVE_STATUS_PATH, task, result);

// THEN
result.computeStatus();
TestUtil.assertSuccess(result);

PrismObject<UserType> userMancomb = getUser(userMancombOid);
display("User after change execution", userMancomb);
assertAccounts(userMancombOid, 3);

DummyAccount mancombBlueAccount = getDummyAccount(RESOURCE_DUMMY_BLUE_NAME, ACCOUNT_MANCOMB_DUMMY_USERNAME);
assertNotNull("No mancomb blue account", mancombBlueAccount);
// Blue resouce has only weak administrativeStatus mapping. The values is not reset to null.
// This does not work now: MID-3418
// assertEquals("Wring mancomb blue account enabled flag", Boolean.TRUE, mancombBlueAccount.isEnabled());
assertEquals("Wrong validFrom in mancomb blue account", ACCOUNT_MANCOMB_VALID_FROM_DATE, mancombBlueAccount.getValidFrom());
assertEquals("Wrong validTo in mancomb blue account", ACCOUNT_MANCOMB_VALID_TO_DATE, mancombBlueAccount.getValidTo());

DummyAccount mancombBlackAccount = getDummyAccount(RESOURCE_DUMMY_BLACK_NAME, ACCOUNT_MANCOMB_DUMMY_USERNAME);
assertNotNull("No mancomb black account", mancombBlackAccount);
assertEquals("Wring mancomb black account enabled flag", null, mancombBlackAccount.isEnabled());
assertEquals("Wrong validFrom in mancomb black account", ACCOUNT_MANCOMB_VALID_FROM_DATE, mancombBlackAccount.getValidFrom());
assertEquals("Wrong validTo in mancomb black account", ACCOUNT_MANCOMB_VALID_TO_DATE, mancombBlackAccount.getValidTo());
}

@Test
public void test400AddHerman() throws Exception {
final String TEST_NAME = "test400AddHerman";
Expand Down Expand Up @@ -1890,11 +2033,11 @@ public void test610EnableUser1() throws Exception {
// TODO check real state of the account and shadow
}

private void assertDummyActivationEnabledState(String userId, boolean expectedEnabled) throws SchemaViolationException {
private void assertDummyActivationEnabledState(String userId, Boolean expectedEnabled) throws SchemaViolationException {
assertDummyActivationEnabledState(null, userId, expectedEnabled);
}

private void assertDummyActivationEnabledState(String instance, String userId, boolean expectedEnabled) throws SchemaViolationException {
private void assertDummyActivationEnabledState(String instance, String userId, Boolean expectedEnabled) throws SchemaViolationException {
DummyAccount account = getDummyAccount(instance, userId);
assertNotNull("No dummy account "+userId, account);
assertEquals("Wrong enabled flag in dummy '"+instance+"' account "+userId, expectedEnabled, account.isEnabled());
Expand Down
2 changes: 1 addition & 1 deletion model/model-intest/src/test/resources/activation/resource-dummy-khaki.xml
View file
Expand Up @@ -47,7 +47,7 @@
xmlns:icfc="http://midpoint.evolveum.com/xml/ns/public/connector/icf-1/connector-schema-3">

<icfc:configurationProperties>
<icfi:instanceId>khaki</icfi:instanceId> <!-- Default instance. -->
<icfi:instanceId>khaki</icfi:instanceId>
<icfi:requireExplicitEnable>false</icfi:requireExplicitEnable>
<icfi:supportActivation>false</icfi:supportActivation>
</icfc:configurationProperties>
Expand Down
5 changes: 1 addition & 4 deletions model/model-intest/src/test/resources/common/resource-dummy-black.xml
View file
Expand Up @@ -47,7 +47,7 @@

<icfc:configurationProperties>
<icfi:instanceId>black</icfi:instanceId>
<icfi:supportValidity>black</icfi:supportValidity>
<icfi:supportValidity>true</icfi:supportValidity>
<icfi:addConnectorStateAttributes>true</icfi:addConnectorStateAttributes>
</icfc:configurationProperties>

Expand Down Expand Up @@ -151,23 +151,20 @@
<activation>
<administrativeStatus>
<outbound>
<strength>weak</strength>
<expression>
<asIs/>
</expression>
</outbound>
</administrativeStatus>
<validFrom>
<outbound>
<strength>weak</strength>
<expression>
<asIs/>
</expression>
</outbound>
</validFrom>
<validTo>
<outbound>
<strength>weak</strength>
<expression>
<asIs/>
</expression>
Expand Down

0 comments on commit 3eaa962

Please sign in to comment.