Support ADD patch ops targeting existing values

In the 3.0.0 release, we implemented support for ADD patch operations
with a value filter. This was configured to always append a new value
to the array, since it is technically an "add". However, this does not
play well with SCIM provisioners that send multiple individual updates
that are meant to target the same value within a multi-valued attribute,
e.g., a work email. The intention is to add a new value to the field
that matches this path filter (e.g., "type eq \"work\""), not that the
"type" field should also be added. The exception to this is if the
provided type does not already exist on the resource; in this case, it
makes sense to add that data.

The new behavior is available via an opt-in setting in a static boolean
variable, PatchOperation.APPEND_NEW_PATCH_VALUES_PROPERTY. To opt into
this setting, set the value of this variable to false.

As a result of this change, the version of the SCIM SDK has been updated
to 3.2.0-SNAPSHOT.

Reviewer: vyhhuang
Reviewer: dougbulkley

JiraIssue: DS-49194

CHANGELOG.md

@@ -2,12 +2,33 @@
 All notable changes to this project will be documented in this file.
 This project adheres to [Semantic Versioning](https://semver.org/).
 
-## v3.1.1 - TBD
+## v3.2.0 - TBD
 Fixed an issue where `AndFilter.equals()` and `OrFilter.equals()` could incorrectly evaluate to
 true.
 
 Updated Jackson dependencies to 2.17.2.
 
+Added a property that allows ADD patch operations with value filters to target an existing value.
+For example, for the following patch request:
+```json
+{
+ "schemas": [ "urn:ietf:params:scim:api:messages:2.0:PatchOp" ],
+ "Operations": [
+ {
+ "op": "add",
+ "path": "emails[type eq \"work\"].display",
+ "value": "[email protected]"
+ }
+ ]
+}
+```
+When the new `PatchOperation.APPEND_NEW_PATCH_VALUES_PROPERTY` property is set to `false`, this
+operation will search the resource for an existing "work" email and add the value to that email.
+This allows for better integration with SCIM provisioners that send individual requests such as
+`emails[type eq "work"].display` followed by `emails[type eq "work"].value`, which are intended to
+target the same email. The default value of `PatchOperation.APPEND_NEW_PATCH_VALUES_PROPERTY` is
+`true`, which will always create a new value on the multi-valued attribute.
+
 ## v3.1.0 - 2024-Jun-25
 Updated all classes within the UnboundID SCIM 2 SDK to utilize `@Nullable` and `@NotNull`
 annotations for all non-primitive input parameters, member variables, and return values. These

pom.xml

@@ -18,7 +18,7 @@
  <modelVersion>4.0.0</modelVersion>
  <groupId>com.unboundid.product.scim2</groupId>
  <artifactId>scim2-parent</artifactId>
- <version>3.1.1-SNAPSHOT</version>
+ <version>3.2.0-SNAPSHOT</version>
  <packaging>pom</packaging>
  <name>UnboundID SCIM2 SDK Parent</name>
  <description>

scim2-assembly/pom.xml

@@ -19,7 +19,7 @@
  <parent>
  <artifactId>scim2-parent</artifactId>
  <groupId>com.unboundid.product.scim2</groupId>
- <version>3.1.1-SNAPSHOT</version>
+ <version>3.2.0-SNAPSHOT</version>
  </parent>
  <artifactId>scim2-assembly</artifactId>
  <packaging>pom</packaging>

scim2-sdk-client/pom.xml

@@ -19,7 +19,7 @@
  <parent>
  <artifactId>scim2-parent</artifactId>
  <groupId>com.unboundid.product.scim2</groupId>
- <version>3.1.1-SNAPSHOT</version>
+ <version>3.2.0-SNAPSHOT</version>
  </parent>
  <artifactId>scim2-sdk-client</artifactId>
  <packaging>jar</packaging>

scim2-sdk-common/pom.xml

@@ -19,7 +19,7 @@
  <parent>
  <artifactId>scim2-parent</artifactId>
  <groupId>com.unboundid.product.scim2</groupId>
- <version>3.1.1-SNAPSHOT</version>
+ <version>3.2.0-SNAPSHOT</version>
  </parent>
  <artifactId>scim2-sdk-common</artifactId>
  <packaging>jar</packaging>

scim2-sdk-common/src/main/java/com/unboundid/scim2/common/messages/PatchOperation.java

@@ -42,6 +42,7 @@
 import com.unboundid.scim2.common.filters.EqualFilter;
 import com.unboundid.scim2.common.filters.Filter;
 import com.unboundid.scim2.common.filters.FilterType;
+import com.unboundid.scim2.common.utils.FilterEvaluator;
 import com.unboundid.scim2.common.utils.JsonUtils;
 import com.unboundid.scim2.common.utils.SchemaUtils;
 
@@ -126,6 +127,39 @@ public abstract class PatchOperation
  @Nullable
  private final Path path;
 
+ /**
+ * This represents a property that, when enabled, will always append new
+ * values when processing ADD operations with a value filter. This is used for
+ * multi-valued attributes such as {@code emails}. For example, for the
+ * following patch request:
+ * <pre>
+ * {
+ * "schemas": [ "urn:ietf:params:scim:api:messages:2.0:PatchOp" ],
+ * "Operations": [
+ * {
+ * "op": "add",
+ * "path": "emails[type eq \"work\"].display",
+ * "value": "[email protected]"
+ * }
+ * ]
+ * }
+ * </pre>
+ *
+ * When this property is enabled and the above patch request is applied, the
+ * following JSON will be appended to the {@code emails} of the user resource:
+ * <pre>
+ * {
+ * "type": "work",
+ * "display": "[email protected]"
+ * }
+ * </pre>
+ *
+ * If this property is disabled, then the {@code display} field will be added
+ * to the resource's existing work email, if it exists. If the work email does
+ * not exist, then a new value will be appended.
+ */
+ public static boolean APPEND_NEW_PATCH_VALUES_PROPERTY = true;
+
 
  static final class AddOperation extends PatchOperation
  {
@@ -215,7 +249,6 @@ public void apply(@NotNull final ObjectNode node) throws ScimException
  Path path = (getPath() == null) ? Path.root() : getPath();
  if (hasValueFilter(path))
  {
- validateAddOpWithFilter(path, value);
  applyAddWithValueFilter(path, node, value);
  }
  else
@@ -377,8 +410,10 @@ private void applyAddWithValueFilter(
  @NotNull final Path path,
  @NotNull final ObjectNode existingResource,
  @NotNull final JsonNode value)
- throws BadRequestException
+ throws ScimException
  {
+ validateAddOpWithFilter(path, value);
+
  Filter valueFilter = path.getElement(0).getValueFilter();
  String filterAttributeName = valueFilter.getAttributePath().toString();
  ValueNode filterValue = valueFilter.getComparisonValue();
@@ -405,13 +440,95 @@ private void applyAddWithValueFilter(
  }
  ArrayNode attribute = (ArrayNode) jsonAttribute;
 
- // Construct the new attribute value that should be added to the resource.
- ObjectNode newValue = JsonUtils.getJsonNodeFactory().objectNode();
- newValue.set(subAttributeName, value);
- newValue.set(filterAttributeName, filterValue);
+ // When operations with a value filter add data, we can either append
+ // the data to a new value in the multi-valued attribute, or we can update
+ // an existing value.
+ //
+ // If the relevant property is enabled, the provided data should be added
+ // as a new value regardless of the existing resource's state (so we
+ // pretend that there are no matched values). Otherwise, any new data
+ // should update the existing value, if it is present.
+ ObjectNode matchedValue = null;
+ if (!APPEND_NEW_PATCH_VALUES_PROPERTY)
+ {
+ matchedValue = fetchExistingValue(attribute, valueFilter, attributeName);
+ }
+
+ // If there are no existing values that match the filter, then we should
+ // add the two attribute values to the array.
+ if (matchedValue == null)
+ {
+ ObjectNode newValue = JsonUtils.getJsonNodeFactory().objectNode();
+ newValue.set(subAttributeName, value);
+ newValue.set(filterAttributeName, filterValue);
+
+ attribute.add(newValue);
+ existingResource.replace(attributeName, attribute);
+ return;
+ }
+
+ // Ensure that the add does not conflict with an existing value.
+ if (FilterEvaluator.evaluate(Filter.pr(subAttributeName), matchedValue))
+ {
+ throw BadRequestException.invalidValue(String.format(
+ "The add operation attempted to add a new '%s' field, but the"
+ + " specified path already has a '%s' defined.",
+ subAttributeName,
+ subAttributeName
+ ));
+ }
+
+ matchedValue.set(subAttributeName, value);
+ }
+
+ /**
+ * Checks a multi-valued attribute for an existing value and returns a value
+ * based on the following conditions:
+ * <ul>
+ * <li> If a single existing value is present, it will be returned.
+ * <li> If no existing values are present, {@code null} will be returned.
+ * <li> If multiple existing values are present, a
+ * {@link BadRequestException} will be thrown, since it is unclear
+ * which value should be targeted.
+ * </ul>
+ *
+ * @param attribute The multi-valued attribute.
+ * @param valueFilter The value selection filter provided with the patch
+ * add operation.
+ * @param attributeName The name of {@code attribute}.
+ *
+ * @return An ObjectNode representing the single value that matched the
+ * criteria of the {@code valueFilter}, or {@code null} if no
+ * attributes matched the filter.
+ *
+ * @throws ScimException If there was an error processing the filter, or
+ * if multiple values were matched.
+ */
+ @Nullable
+ private static ObjectNode fetchExistingValue(
+ @NotNull final ArrayNode attribute,
+ @NotNull final Filter valueFilter,
+ @NotNull final String attributeName)
+ throws ScimException
+ {
+ ObjectNode matchedValue = null;
+
+ for (var arrayVal : attribute)
+ {
+ if (FilterEvaluator.evaluate(valueFilter, arrayVal))
+ {
+ if (matchedValue != null)
+ {
+ throw BadRequestException.noTarget(
+ "The operation could not be applied on the resource because the"
+ + " value filter matched more than one element in the '"
+ + attributeName + "' array of the resource.");
+ }
+ matchedValue = (ObjectNode) arrayVal;
+ }
+ }
 
- attribute.add(newValue);
- existingResource.replace(attributeName, attribute);
+ return matchedValue;
  }
 
  /**