XMLHandlerImpl.update() removes an attribute's existing values before it checks the new ones, so an update that is rejected still changes the entry.
Where
OpenICF-xml-connector/src/main/java/org/forgerock/openicf/connectors/xml/XMLHandlerImpl.java, update(), on master: inside the loop over replaceAttributes, removeChildrenFromElement(entry, …) runs at line 280, and the single-valued check if (!attributeInfo.isMultiValued() && values.size() > 1) throw new IllegalArgumentException(…) runs after it, at line 285.
Scenario
update(ACCOUNT, uid-alice, {lastname = ["A", "B"]}), where lastname is single-valued, throws IllegalArgumentException: Data field: lastname is not multivalued can not have more than one value, but alice's lastname has already been removed from the document. On master dispose() saves after every call, so the removal also reaches the file.
The same happens across attributes. update() checks each attribute just before it replaces it, so when a later attribute is rejected (not supported, not updatable, a missing or blank required value, values of the wrong type, or the single-valued rule), the attributes before it in the set have already been replaced.
Expected
An update that throws leaves the entry as it was: check every attribute in replaceAttributes (supported, updatable, required values, value types, the single-valued rule) before the first change, as create() checks every attribute before it appends the new entry.
Notes
XMLHandlerImpl.update()removes an attribute's existing values before it checks the new ones, so an update that is rejected still changes the entry.Where
OpenICF-xml-connector/src/main/java/org/forgerock/openicf/connectors/xml/XMLHandlerImpl.java,update(), on master: inside the loop overreplaceAttributes,removeChildrenFromElement(entry, …)runs at line 280, and the single-valued checkif (!attributeInfo.isMultiValued() && values.size() > 1) throw new IllegalArgumentException(…)runs after it, at line 285.Scenario
update(ACCOUNT, uid-alice, {lastname = ["A", "B"]}), wherelastnameis single-valued, throwsIllegalArgumentException: Data field: lastname is not multivalued can not have more than one value, but alice'slastnamehas already been removed from the document. On masterdispose()saves after every call, so the removal also reaches the file.The same happens across attributes.
update()checks each attribute just before it replaces it, so when a later attribute is rejected (not supported, not updatable, a missing or blank required value, values of the wrong type, or the single-valued rule), the attributes before it in the set have already been replaced.Expected
An update that throws leaves the entry as it was: check every attribute in
replaceAttributes(supported, updatable, required values, value types, the single-valued rule) before the first change, ascreate()checks every attribute before it appends the new entry.Notes
markDirty()before the first change, so after a failed update the file agrees with memory.XMLHandlerReloadTests#failedUpdateLeavesTheFileInAgreementWithMemoryuses this failure for its mid-mutation case. Once this is fixed, that test needs another failure in the middle of an update. One that remains isremoveChildrenFromElementthrowing midway on an XSD-invalid store, as infailedDeleteOfANestedEntryLeavesTheFileAlone.