Skip to content

Commit

Permalink
Support replace ops with an empty array value
Browse files Browse the repository at this point in the history 
This commit resolves an issue with replace operations that contained an
empty array as the value. If a filter is not included in the path, the
SDK now treats this as a request to delete all values of the targeted
multi-valued attribute. If a filter is included, then all attribute
values that match the filter will be removed.

Reviewer: vyhhuang
Reviewer: dougbulkley

JiraIssue: DS-48754
  • Loading branch information
@kqarryzada
kqarryzada committed Mar 13, 2024
1 parent 74d04e1 commit e2d492f
Show file tree
Hide file tree
Showing 4 changed files with 332 additions and 56 deletions.
5 changes: 4 additions & 1 deletion CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,10 @@ All notable changes to this project will be documented in this file.
This project adheres to [Semantic Versioning](https://semver.org/).

## v3.0.1 - TBD
Placeholder for the next release.
Resolved an issue with replace operations that set the `value` field to an empty array. When these
operations are applied, the SCIM SDK now clears all matching values of the targeted multi-valued
attribute. If the `path` of the replace operation does not have a filter, then the multi-valued
attribute will be deleted from the resource.

## v3.0.0 - 2023-Oct-03
Removed support for Java 8. The UnboundID SCIM 2 SDK now requires Java 11 or a later release.
Expand Down
4 changes: 2 additions & 2 deletions scim2-sdk-common/src/main/java/com/unboundid/scim2/common/messages/PatchOperation.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -594,8 +594,8 @@ public <T> List<T> getValues(final Class<T> cls)
@Override
public void apply(final ObjectNode node) throws ScimException
{
JsonUtils.replaceValue(getPath() == null ? Path.root() :
getPath(), node, value);
Path path = (getPath() == null) ? Path.root() : getPath();
JsonUtils.replaceValue(path, node, value);
addMissingSchemaUrns(node);
}

Expand Down
202 changes: 152 additions & 50 deletions scim2-sdk-common/src/main/java/com/unboundid/scim2/common/utils/JsonUtils.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,13 +28,16 @@
import com.fasterxml.jackson.databind.node.ObjectNode;
import com.fasterxml.jackson.databind.type.CollectionType;
import com.unboundid.scim2.common.Path;
import com.unboundid.scim2.common.annotations.NotNull;
import com.unboundid.scim2.common.annotations.Nullable;
import com.unboundid.scim2.common.exceptions.BadRequestException;
import com.unboundid.scim2.common.exceptions.ScimException;
import com.unboundid.scim2.common.filters.Filter;
import com.unboundid.scim2.common.messages.PatchOperation;
import com.unboundid.scim2.common.types.AttributeDefinition;

import java.io.IOException;
import java.util.ArrayList;
import java.util.Date;
import java.util.Iterator;
import java.util.LinkedList;
Expand Down Expand Up @@ -254,55 +257,131 @@ else if(node.isArray())
/**
* {@inheritDoc}
*/
protected void visitLeafNode(final ObjectNode parent,
final String field,
final Filter valueFilter)
protected void visitLeafNode(@NotNull final ObjectNode parent,
@Nullable final String field,
@Nullable final Filter valueFilter)
throws ScimException
{
if(field != null)
boolean matchesFound = false;

if (field == null)
{
updateNode(parent, null, value);
return;
}

// Operations without a value filter can be updated normally without any
// special considerations. Note that add operations with value filters are
// already handled in PatchOperation#applyAddWithValueFilter.
if (appendValues || valueFilter == null)
{
updateNode(parent, field, value);
return;
}

// in replace mode, a value filter requires that the target node
// be an array and that we can find matching value(s)
JsonNode node = parent.path(field);
if (node.isArray())
{
JsonNode node = parent.path(field);
if (!appendValues && valueFilter != null)
var array = (ArrayNode) node;
matchesFound = processArrayReplace(parent, field, array, valueFilter);
}
// exception: this allows filters on singular values if
// and only if the filter uses the "value" attribute to
// reference the value of the value node.
else if (FilterEvaluator.evaluate(valueFilter, node))
{
matchesFound = true;
updateNode(parent, field, value);
}

if (!matchesFound)
{
throw BadRequestException.noTarget("Attribute " +
field + " does not have a value matching " +
"the filter " + valueFilter);
}
}

/**
* This method processes a replace operation for a multi-valued attribute
* (e.g., {@code emails, addresses}) with a value selection filter. If the
* {@code value} of the patch operation is an empty array, then attribute
* values that match the filter will be removed from the target resource.
*
* @param parent The resource that contains the multi-valued
* attribute.
* @param field The name of the multi-valued attribute (e.g.,
* {@code emails}).
* @param array The current value of the multi-valued attribute.
* @param valueFilter The value filter indicating the subset of attribute
* values that should be modified.
*
* @return {@code true} if at least one value in the {@code array} was
* updated with the new value.
*
* @throws ScimException If an error occurred while processing the filter.
*/
private boolean processArrayReplace(@NotNull final ObjectNode parent,
@NotNull final String field,
@NotNull final ArrayNode array,
@NotNull final Filter valueFilter)
throws ScimException
{
boolean nodeUpdated = false;

// If the 'value' is an empty array, then the client is actually
// requesting that we delete values that match the filter.
boolean isDelete = isNullNodeOrEmptyArray(value);
List<Integer> indexesToDelete = new ArrayList<>();

for (int i = 0; i < array.size(); i++)
{
JsonNode attributeValue = array.get(i);

// We should only perform processing on this inner node if it matches
// the filter.
if (!FilterEvaluator.evaluate(valueFilter, attributeValue))
{
// in replace mode, a value filter requires that the target node
// be an array and that we can find matching value(s)
boolean matchesFound = false;
if (node.isArray())
{
for(int i = 0; i < node.size(); i++)
{
if(FilterEvaluator.evaluate(valueFilter, node.get(i)))
{
matchesFound = true;
if(node.get(i).isObject() && value.isObject())
{
updateNode((ObjectNode) node.get(i), null, value);
}
else
{
((ArrayNode) node).set(i, value);
}
}
}
}
// exception: this allows filters on singular values if
// and only if the filter uses the "value" attribute to
// reference the value of the value node.
else if (FilterEvaluator.evaluate(valueFilter, node))
{
matchesFound = true;
updateNode(parent, field, value);
}
if(!matchesFound)
{
throw BadRequestException.noTarget("Attribute " +
field + " does not have a value matching " +
"the filter " + valueFilter);
}
return;
continue;
}
nodeUpdated = true;


if (attributeValue.isObject() && value.isObject())
{
updateNode((ObjectNode) attributeValue, null, value);
}
else if (isDelete && attributeValue.isObject())
{
// Mark the current index as a child that must be deleted. We cannot
// delete this yet because it would modify the 'i' index, which we are
// currently iterating over.
indexesToDelete.add(i);
}
else
{
array.set(i, value);
}
}
updateNode(parent, field, value);

for (int j = indexesToDelete.size() - 1; j >= 0; j--)
{
var nodeIndex = indexesToDelete.get(j);
array.remove(nodeIndex);
}

// If we have pruned all remaining values on the field, then we should
// completely delete the attribute. For example, if a resource only has a
// single email that is deleted, the 'emails' field in the JsonNode should
// explicitly be set to null.
if (array.isEmpty())
{
updateNode(parent, field, NullNode.getInstance());
}

return nodeUpdated;
}

/**
Expand All @@ -313,17 +392,27 @@ else if (FilterEvaluator.evaluate(valueFilter, node))
* @param key The key of the field to update.
* @param value The update value.
*/
protected void updateNode(final ObjectNode parent, final String key,
final JsonNode value)
protected void updateNode(@NotNull final ObjectNode parent,
@Nullable final String key,
@Nullable final JsonNode value)
{
if(value.isNull() || value.isArray() && value.size() == 0)
// RFC 7643 section 2.5 states:
// "Unassigned attributes, the null value, or empty array (in the case of
// a multi-valued attribute) SHALL be considered to be equivalent in
// 'state'. Assigning an attribute with the value 'null' or an empty
// array... has the effect of making the attribute 'unassigned'."
if (isNullNodeOrEmptyArray(value))
{
// draft-ietf-scim-core-schema section 2.4 states "Unassigned
// attributes, the null value, or empty array (in the case of
// a multi-valued attribute) SHALL be considered to be
// equivalent in 'state'".
// Replace operations should remove the field. If this is an add
// operation, there is no update to make.
if (!appendValues && key != null)
{
parent.remove(key);
}

return;
}

// When key is null, the node to update is the parent itself.
JsonNode node = key == null ? parent : parent.path(key);
if(node.isObject())
Expand Down Expand Up @@ -1056,4 +1145,17 @@ public static void setCustomMapperFactory(final MapperFactory customMapperFactor
SDK_OBJECT_MAPPER = customMapperFactory.createObjectMapper();
}


/**
* Validates whether the provided node is null or an empty array.
*/
private static boolean isNullNodeOrEmptyArray(@Nullable final JsonNode node)
{
if (node == null || node.isNull())
{
return true;
}

return (node.isArray() && node.isEmpty());
}
}
Loading

0 comments on commit e2d492f

Please sign in to comment.