From c24009add312324b80173cd8406c8ce9a47eadbb Mon Sep 17 00:00:00 2001 From: "Katarina Valalikova (katkav)" Date: Tue, 22 Jul 2014 10:08:19 +0200 Subject: [PATCH] fixing MID-1975 (setting administrative status to undefined) --- .../impl/ResourceObjectConverter.java | 1 + .../ucf/impl/ConnectorInstanceIcfImpl.java | 47 +++++--- .../provisioning/test/impl/TestDummy.java | 103 +++++++++++++++++- 3 files changed, 136 insertions(+), 15 deletions(-) diff --git a/provisioning/provisioning-impl/src/main/java/com/evolveum/midpoint/provisioning/impl/ResourceObjectConverter.java b/provisioning/provisioning-impl/src/main/java/com/evolveum/midpoint/provisioning/impl/ResourceObjectConverter.java index 01a129c0c84..f420719c204 100644 --- a/provisioning/provisioning-impl/src/main/java/com/evolveum/midpoint/provisioning/impl/ResourceObjectConverter.java +++ b/provisioning/provisioning-impl/src/main/java/com/evolveum/midpoint/provisioning/impl/ResourceObjectConverter.java @@ -806,6 +806,7 @@ private Collection determineActivationChange(ShadowType shadow, Colle } XMLGregorianCalendar xmlCal = validFromPropertyDelta.getPropertyNew().getRealValue(); LOGGER.trace("Found activation validFrom change to: {}", xmlCal); + //TODO: why this if?? do we really want to not allow to set validFrom/validTo to null value?? the same for validTo if (xmlCal != null) { operations.add(new PropertyModificationOperation(validFromPropertyDelta)); } diff --git a/provisioning/provisioning-impl/src/main/java/com/evolveum/midpoint/provisioning/ucf/impl/ConnectorInstanceIcfImpl.java b/provisioning/provisioning-impl/src/main/java/com/evolveum/midpoint/provisioning/ucf/impl/ConnectorInstanceIcfImpl.java index 8c4cef26aed..86451d76741 100644 --- a/provisioning/provisioning-impl/src/main/java/com/evolveum/midpoint/provisioning/ucf/impl/ConnectorInstanceIcfImpl.java +++ b/provisioning/provisioning-impl/src/main/java/com/evolveum/midpoint/provisioning/ucf/impl/ConnectorInstanceIcfImpl.java @@ -1419,22 +1419,28 @@ public Set modifyObject(ObjectClassComplexTypeDef try { updateAttributes = convertFromResourceObject(updateValues, result); + + if (activationDeltas != null) { + // Activation change means modification of attributes + convertFromActivation(updateAttributes, activationDeltas); + } + + if (passwordDelta != null) { + // Activation change means modification of attributes + convertFromPassword(updateAttributes, passwordDelta); + } + } catch (SchemaException ex) { result.recordFatalError( "Error while converting resource object attributes. Reason: " + ex.getMessage(), ex); throw new SchemaException("Error while converting resource object attributes. Reason: " + ex.getMessage(), ex); + } catch (RuntimeException ex) { + result.recordFatalError("Error while converting resource object attributes. Reason: " + ex.getMessage(), ex); + throw ex; } - if (activationDeltas != null) { - // Activation change means modification of attributes - convertFromActivation(updateAttributes, activationDeltas); - } - - if (passwordDelta != null) { - // Activation change means modification of attributes - convertFromPassword(updateAttributes, passwordDelta); - } + OperationOptions options = new OperationOptionsBuilder().build(); icfResult = result.createSubresult(ConnectorFacade.class.getName() + ".update"); @@ -2190,17 +2196,18 @@ private void convertFromActivation(Set updateAttributes, for (PropertyDelta propDelta : activationDeltas) { if (propDelta.getElementName().equals(ActivationType.F_ADMINISTRATIVE_STATUS)) { - ActivationStatusType status = propDelta.getPropertyNew().getValue(ActivationStatusType.class).getValue(); + ActivationStatusType status = getPropertyNewValue(propDelta, ActivationStatusType.class); + // Not entirely correct, TODO: refactor later updateAttributes.add(AttributeBuilder.build(OperationalAttributes.ENABLE_NAME, status == ActivationStatusType.ENABLED)); } else if (propDelta.getElementName().equals(ActivationType.F_VALID_FROM)) { - XMLGregorianCalendar xmlCal = propDelta.getPropertyNew().getValue(XMLGregorianCalendar.class).getValue(); + XMLGregorianCalendar xmlCal = getPropertyNewValue(propDelta, XMLGregorianCalendar.class);//propDelta.getPropertyNew().getValue(XMLGregorianCalendar.class).getValue(); updateAttributes.add(AttributeBuilder.build(OperationalAttributes.ENABLE_DATE_NAME, XmlTypeConverter.toMillis(xmlCal))); } else if (propDelta.getElementName().equals(ActivationType.F_VALID_TO)) { - XMLGregorianCalendar xmlCal = propDelta.getPropertyNew().getValue(XMLGregorianCalendar.class).getValue(); + XMLGregorianCalendar xmlCal = getPropertyNewValue(propDelta, XMLGregorianCalendar.class);//propDelta.getPropertyNew().getValue(XMLGregorianCalendar.class).getValue(); updateAttributes.add(AttributeBuilder.build(OperationalAttributes.DISABLE_DATE_NAME, XmlTypeConverter.toMillis(xmlCal))); } else if (propDelta.getElementName().equals(ActivationType.F_LOCKOUT_STATUS)) { - LockoutStatusType status = propDelta.getPropertyNew().getValue(LockoutStatusType.class).getValue(); + LockoutStatusType status = getPropertyNewValue(propDelta, LockoutStatusType.class);//propDelta.getPropertyNew().getValue(LockoutStatusType.class).getValue(); updateAttributes.add(AttributeBuilder.build(OperationalAttributes.LOCK_OUT_NAME, status != LockoutStatusType.NORMAL)); } else { throw new SchemaException("Got unknown activation attribute delta " + propDelta.getElementName()); @@ -2209,6 +2216,20 @@ private void convertFromActivation(Set updateAttributes, } + private T getPropertyNewValue(PropertyDelta propertyDelta, Class clazz) throws SchemaException { + PrismProperty> prop = propertyDelta.getPropertyNew(); + if (prop == null){ + return null; + } + PrismPropertyValue propValue = prop.getValue(clazz); + + if (propValue == null){ + return null; + } + + return propValue.getValue(); + } + private void convertFromPassword(Set attributes, PropertyDelta passwordDelta) throws SchemaException { if (passwordDelta == null) { throw new IllegalArgumentException("No password was provided"); diff --git a/provisioning/provisioning-impl/src/test/java/com/evolveum/midpoint/provisioning/test/impl/TestDummy.java b/provisioning/provisioning-impl/src/test/java/com/evolveum/midpoint/provisioning/test/impl/TestDummy.java index 9857445012a..620fb003029 100644 --- a/provisioning/provisioning-impl/src/test/java/com/evolveum/midpoint/provisioning/test/impl/TestDummy.java +++ b/provisioning/provisioning-impl/src/test/java/com/evolveum/midpoint/provisioning/test/impl/TestDummy.java @@ -62,6 +62,7 @@ import com.evolveum.midpoint.prism.PrismContainer; import com.evolveum.midpoint.prism.PrismContainerDefinition; import com.evolveum.midpoint.prism.PrismObject; +import com.evolveum.midpoint.prism.PrismObjectDefinition; import com.evolveum.midpoint.prism.PrismProperty; import com.evolveum.midpoint.prism.PrismPropertyDefinition; import com.evolveum.midpoint.prism.PrismPropertyValue; @@ -1888,8 +1889,8 @@ public void test135ExecuteScript() throws Exception { assertSteadyResource(); } - - @Test + + @Test public void test150DisableAccount() throws Exception { final String TEST_NAME = "test150DisableAccount"; TestUtil.displayTestTile(TEST_NAME); @@ -1933,6 +1934,50 @@ public void test150DisableAccount() throws Exception { assertSteadyResource(); } + + @Test + public void test151ActivationStatusUndefinedAccount() throws Exception { + final String TEST_NAME = "test151ActivationStatusUndefinedAccount"; + TestUtil.displayTestTile(TEST_NAME); + // GIVEN + + Task task = taskManager.createTaskInstance(TestDummy.class.getName() + "." + TEST_NAME); + OperationResult result = task.getResult(); + + ShadowType accountType = provisioningService.getObject(ShadowType.class, ACCOUNT_WILL_OID, null, task, + result).asObjectable(); + assertNotNull(accountType); + display("Retrieved account shadow", accountType); + + DummyAccount dummyAccount = getDummyAccountAssert(ACCOUNT_WILL_USERNAME, willIcfUid); + assertFalse("Account is not disabled", dummyAccount.isEnabled()); + + syncServiceMock.reset(); + + ObjectDelta delta = ObjectDelta.createModificationDeleteProperty(ShadowType.class, + ACCOUNT_WILL_OID, SchemaConstants.PATH_ACTIVATION_ADMINISTRATIVE_STATUS, prismContext, + ActivationStatusType.DISABLED); + display("ObjectDelta", delta); + delta.checkConsistence(); + + // WHEN + provisioningService.modifyObject(ShadowType.class, delta.getOid(), delta.getModifications(), + new OperationProvisioningScriptsType(), null, task, result); + + // THEN + result.computeStatus(); + display("modifyObject result", result); + TestUtil.assertSuccess(result); + + delta.checkConsistence(); + // check if activation was changed + dummyAccount = getDummyAccountAssert(ACCOUNT_WILL_USERNAME, willIcfUid); + assertFalse("Dummy account "+ACCOUNT_WILL_USERNAME+" is enabled, expected disabled", dummyAccount.isEnabled()); + + syncServiceMock.assertNotifySuccessOnly(); + + assertSteadyResource(); + } @Test public void test151EnableAccount() throws Exception { @@ -1978,6 +2023,8 @@ public void test151EnableAccount() throws Exception { assertSteadyResource(); } + + @Test public void test152SetValidFrom() throws Exception { final String TEST_NAME = "test152SetValidFrom"; @@ -2073,6 +2120,58 @@ public void test153SetValidTo() throws Exception { assertSteadyResource(); } + //TODO: if this needed?? for now, we don't allow to set activation valid from and valid to to null value... + @Test(enabled = false) + public void test154DeleteValidToValidFrom() throws Exception { + final String TEST_NAME = "test154DeleteValidToValidFrom"; + TestUtil.displayTestTile(TEST_NAME); + // GIVEN + + Task task = taskManager.createTaskInstance(TestDummy.class.getName() + "." + TEST_NAME); + OperationResult result = task.getResult(); + + ShadowType accountType = provisioningService.getObject(ShadowType.class, ACCOUNT_WILL_OID, null, task, + result).asObjectable(); + assertNotNull(accountType); + + display("Retrieved account shadow", accountType); + + DummyAccount dummyAccount = getDummyAccountAssert(ACCOUNT_WILL_USERNAME, willIcfUid); + assertTrue(dummyAccount.isEnabled()); + + syncServiceMock.reset(); + +// long millis = VALID_TO_MILLIS; + + ObjectDelta delta = ObjectDelta.createModificationDeleteProperty(ShadowType.class, + ACCOUNT_WILL_OID, SchemaConstants.PATH_ACTIVATION_VALID_TO, prismContext, + XmlTypeConverter.createXMLGregorianCalendar(VALID_TO_MILLIS)); + PrismObjectDefinition def = accountType.asPrismObject().getDefinition(); + PropertyDelta validFromDelta = PropertyDelta.createModificationDeleteProperty(SchemaConstants.PATH_ACTIVATION_VALID_FROM, def.findPropertyDefinition(SchemaConstants.PATH_ACTIVATION_VALID_FROM), VALID_FROM_MILLIS); + delta.addModification(validFromDelta); + delta.checkConsistence(); + + // WHEN + provisioningService.modifyObject(ShadowType.class, delta.getOid(), + delta.getModifications(), new OperationProvisioningScriptsType(), null, task, result); + + // THEN + result.computeStatus(); + display("modifyObject result", result); + TestUtil.assertSuccess(result); + + delta.checkConsistence(); + // check if activation was changed + dummyAccount = getDummyAccountAssert(ACCOUNT_WILL_USERNAME, willIcfUid); + assertNull("Wrong account validTo in account "+ACCOUNT_WILL_USERNAME + ": " + dummyAccount.getValidTo(), dummyAccount.getValidTo()); + assertNull("Wrong account validFrom in account "+ACCOUNT_WILL_USERNAME + ": " + dummyAccount.getValidFrom(), dummyAccount.getValidFrom()); + assertTrue("Dummy account "+ACCOUNT_WILL_USERNAME+" is disabled, expected enabled", dummyAccount.isEnabled()); + + syncServiceMock.assertNotifySuccessOnly(); + + assertSteadyResource(); + } + @Test public void test155GetLockedoutAccount() throws Exception { final String TEST_NAME = "test155GetLockedoutAccount";