Skip to content

Commit

Permalink
Allow deserializing null Resources in ListResponse
Browse files Browse the repository at this point in the history
RFC 7644 states that the Resources array in a ListResponse is permitted
to be null when the value of totalResults is 0. Previously, the SCIM SDK
would throw an exception on any attempt to convert a JSON string to a
ListResponse if the Resources array was missing.

The SDK has been updated to successfully process JSON strings with a
null array, as long as the totalResults or itemsPerPage fields are
explicitly set to 0. The latter is not technically requested in the
specification. but it seems probable that a service provider could
return a null array in this case even though it is not correct. There
is arguably little benefit to rejecting the case where itemsPerPage is
0 and the Resources array is null, so we will be permissive of this to
prevent further deserialization errors, as they can be difficult for
application developers to work around.

Note that this commit does not change any behavior for serializing a
ListResponse object to a JSON string. By default, the SCIM SDK will
always print this case as an explicit empty array.

Reviewer: vyhhuang
Reviewer: dougbulkley

JiraIssue: DS-49416
  • Loading branch information
kqarryzada authored Nov 19, 2024
1 parent 67986bf commit dc8144f
Show file tree
Hide file tree
Showing 3 changed files with 258 additions and 11 deletions.
9 changes: 9 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,15 @@ can be used to define resource types for objects that don't have a strongly defi
class-level Javadoc describes how to interface with the object effectively, and the methods now
provide clearer examples of how they can be used.

Updated the class-level documentation of `ListResponse` to provide more background about the
resource type and how to interface with the object.

Updated the `ListResponse` class to prevent deserialization errors when the `Resources` array is
`null`. This is now permitted when `totalResults` and/or `itemsPerPage` is set to 0.
[RFC 7644 Section 3.4.2](https://datatracker.ietf.org/doc/html/rfc7644#section-3.4.2)
explicitly states that the `Resources` array may be null when `totalResults` is 0, so the SCIM SDK
will no longer throw deserialization exceptions when processing JSON of this form.

## 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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,73 @@
import java.util.TreeMap;

/**
* Class representing a SCIM 2 list response.
* This class represents a SCIM 2.0 list response. A list response represents a
* list of results with some additional metadata. This resource type is used as
* a response to search requests and "list" requests (e.g., a GET request on
* {@code /Users}).
* <br><br>
*
* A list response can be broken down into pages, where each page contains a
* subset of the overall results. Pagination allows the SCIM service provider to
* return reasonably-sized JSON responses and avoid expensive computations. The
* next page of results can be retrieved by leveraging the "startIndex" field,
* which represents the page number. Pagination is not a hard requirement of the
* SCIM 2.0 protocol, so some SCIM services do not support it.
* <br><br>
*
* List responses contain the following fields:
* <ul>
* <li> {@code Resources}: A list of SCIM resource objects.
* <li> {@code itemsPerPage}: Indicates the number of results that are present
* in the {@code Resources} array.
* <li> {@code totalResults}: Indicates the total number of results that match
* the list or query operation. This value may be larger than the value
* of {@code itemsPerPage} if all of the matched resources are not
* present in the provided {@code Resources} array.
* <li> {@code startIndex}: The index indicating the page number, if
* pagination is supported by the SCIM service.
* </ul>
*
* An example list response takes the following form:
* <pre>
* {
* "schemas": [ "urn:ietf:params:scim:api:messages:2.0:ListResponse" ],
* "totalResults": 100,
* "itemsPerPage": 1,
* "Resources": [
* {
* "schemas": [ "urn:ietf:params:scim:schemas:core:2.0:User" ],
* "userName": "muhammad.ali",
* "title": "Champ"
* }
* ]
* }
* </pre>
*
* To create the above list response, use the following Java code:
* <pre>
* UserResource muhammad = new UserResource()
* .setUserName("muhammad.ali")
* .setTitle("Champ");
* ListResponse&lt;UserResource&gt; response =
* new ListResponse&lt;&gt;(100, List.of(muhammad), 1, null);
* </pre>
*
* Any Collection may be passed directly into the alternate constructor.
* <pre>
* List&lt;UserResource&gt; list = getUserList();
* ListResponse&lt;UserResource&gt; response = new ListResponse&lt;&gt;(list);
* </pre>
*
* When iterating over the elements in a list response's {@code Resources} list,
* it is possible to iterate directly over the ListResponse object:
* <pre>
* ListResponse&lt;BaseScimResource&gt; listResponse = getResponse();
* for (BaseScimResource resource : listResponse)
* {
* System.out.println(resource.getId());
* }
* </pre>
*
* @param <T> The type of the returned resources.
*/
Expand Down Expand Up @@ -74,27 +140,36 @@ public final class ListResponse<T> extends BaseScimResource
@JsonProperty(value = "Resources", required = true)
private final List<T> resources;

@NotNull
private static final Integer ZERO = 0;

/**
* Create a new List Response.
* <br><br>
*
* This constructor is utilized by Jackson when converting JSON strings into
* ListResponse objects. To create a ListResponse in code, it is suggested to
* use {@link ListResponse#ListResponse(int, List, Integer, Integer)}.
*
* @param props Properties to construct the List Response.
*/
@JsonCreator(mode = JsonCreator.Mode.DELEGATING)
@SuppressWarnings("unchecked")
public ListResponse(@NotNull final Map<String, Object> props)
throws IllegalArgumentException, IllegalStateException
{
final Map<String, Object> properties =
new TreeMap<String, Object>(String.CASE_INSENSITIVE_ORDER);
new TreeMap<>(String.CASE_INSENSITIVE_ORDER);
properties.putAll(props);

checkRequiredProperties(properties, "totalResults", "resources");

checkRequiredProperties(properties, "totalResults");
this.totalResults = (Integer) properties.get("totalResults");
this.resources = (List<T>) properties.get("resources");
this.startIndex = properties.containsKey("startIndex") ?
(Integer) properties.get("startIndex") : null;
this.itemsPerPage = properties.containsKey("itemsPerPage") ?
(Integer) properties.get("itemsPerPage") : null;
this.itemsPerPage = (Integer) properties.get("itemsPerPage");
this.startIndex = (Integer) properties.get("startIndex");

var initList = (List<T>) properties.get("Resources");
this.resources = resourcesOrEmptyList(initList, itemsPerPage, totalResults);

if (properties.containsKey("schemas"))
{
this.setSchemaUrns((Collection<String>) properties.get("schemas"));
Expand All @@ -116,6 +191,7 @@ public ListResponse(final int totalResults,
@NotNull final List<T> resources,
@Nullable final Integer startIndex,
@Nullable final Integer itemsPerPage)
throws IllegalArgumentException
{
this.totalResults = totalResults;
this.startIndex = startIndex;
Expand Down Expand Up @@ -231,7 +307,7 @@ public boolean equals(@Nullable final Object o)
return false;
}

ListResponse that = (ListResponse) o;
ListResponse<?> that = (ListResponse<?>) o;

if (totalResults != that.totalResults)
{
Expand Down Expand Up @@ -274,6 +350,7 @@ public int hashCode()
private void checkRequiredProperties(
@NotNull final Map<String, Object> properties,
@NotNull final String... requiredProperties)
throws IllegalStateException
{
for (final String property : requiredProperties)
{
Expand All @@ -284,4 +361,48 @@ private void checkRequiredProperties(
}
}
}

/**
* Fetches a non-null representation of the {@code Resources} list, or throws
* an exception if the ListResponse object would be invalid.
* <br><br>
*
* A JSON list response may contain a null value for {@code Resources} only
* if there are no results to display. RFC 7644 states:
* <pre>
* Resources A multi-valued list of complex objects containing the
* requested resources... REQUIRED if "totalResults" is non-zero.
* </pre>
*
* This method only permits {@code null} arrays when the provided list should
* have been empty (i.e., when either integer value is 0).
*
* @param resources The list that should be analyzed.
* @param itemsPerPage The value of {@code itemsPerPage} on the ListResponse.
* @param totalResults The value of {@code totalResults} on the ListResponse.
* @return A non-null list of resources.
*
* @throws IllegalStateException If {@code Resources} is {@code null} but
* neither integer is 0.
*/
@NotNull
private List<T> resourcesOrEmptyList(@Nullable final List<T> resources,
@Nullable final Integer itemsPerPage,
final int totalResults)
throws IllegalStateException
{
if (resources != null)
{
return resources;
}

if (totalResults == 0 || ZERO.equals(itemsPerPage))
{
return Collections.emptyList();
}

throw new IllegalStateException(
"Failed to create the ListResponse since it is missing the 'Resources'"
+ " property, which must be present if totalResults is non-zero.");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,10 @@

package com.unboundid.scim2.common;

import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.core.type.TypeReference;
import com.fasterxml.jackson.databind.JsonMappingException;
import com.fasterxml.jackson.databind.ObjectReader;
import com.fasterxml.jackson.databind.node.ObjectNode;
import com.unboundid.scim2.common.messages.ListResponse;
import com.unboundid.scim2.common.types.ResourceTypeResource;
Expand All @@ -30,6 +32,8 @@
import java.util.Collections;
import java.util.List;

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
import static org.testng.Assert.assertEquals;
import static org.testng.Assert.assertTrue;
import static org.testng.Assert.fail;
Expand Down Expand Up @@ -150,7 +154,8 @@ public void testListResponse()
* @throws Exception If an unexpected error occurs.
*/
@Test
public void testListResponseFormat() throws Exception {
public void testListResponseFormat() throws Exception
{
// Reformat the expected JSON to a standardized form.
String expectedJSON =
JsonUtils.getObjectReader().readTree(SINGLE_ELEMENT_LIST_RESPONSE)
Expand All @@ -163,4 +168,116 @@ public void testListResponseFormat() throws Exception {

assertEquals(listResponseJSON, expectedJSON);
}


/**
* Ensures {@code null} resources are permitted during deserialization when
* {@code totalResults} or {@code itemsPerPage} are zero.
*/
@Test
public void testDeserializingNullResourcesArray() throws Exception
{
// The object reader that will be used to serialize JSON strings into
// ListResponse objects.
final ObjectReader reader = JsonUtils.getObjectReader();

// When 'totalResults' is 0, a missing "Resources" property should be
// permitted and translated into an empty list.
String noResource = "{ "
+ " \"schemas\": ["
+ " \"urn:ietf:params:scim:api:messages:2.0:ListResponse\""
+ " ],"
+ " \"totalResults\": 0,"
+ " \"itemsPerPage\": 0"
+ "}";
ListResponse<?> object = reader.readValue(noResource, ListResponse.class);
assertThat(object.getTotalResults()).isEqualTo(0);
assertThat(object.getItemsPerPage()).isEqualTo(0);
assertThat(object.getStartIndex()).isNull();
assertThat(object.getResources())
.isNotNull()
.isEmpty();

// Test the emptiest possible valid ListResponse object. There should be no
// problems with setting the other fields to null or empty.
String smallResponse = "{ "
+ " \"schemas\": ["
+ " \"urn:ietf:params:scim:api:messages:2.0:ListResponse\""
+ " ],"
+ " \"totalResults\": 0"
+ "}";
ListResponse<?> small = reader.readValue(smallResponse, ListResponse.class);
assertThat(small.getTotalResults()).isEqualTo(0);
assertThat(small.getItemsPerPage()).isNull();
assertThat(small.getStartIndex()).isNull();
assertThat(small.getResources())
.isNotNull()
.isEmpty();

// An explicit empty array should still be permitted if totalResults == 0.
String emptyArray = "{ "
+ " \"schemas\": ["
+ " \"urn:ietf:params:scim:api:messages:2.0:ListResponse\""
+ " ],"
+ " \"totalResults\": 0,"
+ " \"Resources\": []"
+ "}";
ListResponse<?> response = reader.readValue(emptyArray, ListResponse.class);
assertThat(response.getTotalResults()).isEqualTo(0);
assertThat(response.getItemsPerPage()).isNull();
assertThat(response.getStartIndex()).isNull();
assertThat(response.getResources())
.isNotNull()
.isEmpty();

// This is illegal since the Resources array is missing when totalResults is
// non-zero.
String invalidJSON = "{ "
+ " \"schemas\": ["
+ " \"urn:ietf:params:scim:api:messages:2.0:ListResponse\""
+ " ],"
+ " \"totalResults\": 1"
+ "}";
assertThatThrownBy(()-> reader.readValue(invalidJSON, ListResponse.class))
.isInstanceOf(JsonProcessingException.class)
.hasMessageContaining("Failed to create the ListResponse since it is")
.hasMessageContaining("missing the 'Resources' property");

// The following is a valid list response since itemsPerPage restricts the
// resource list size to 0.
String newObj = "{ "
+ " \"schemas\": ["
+ " \"urn:ietf:params:scim:api:messages:2.0:ListResponse\""
+ " ],"
+ " \"totalResults\": 100,"
+ " \"itemsPerPage\": 0,"
+ " \"Resources\": []"
+ "}";
ListResponse<?> response2 = reader.readValue(newObj, ListResponse.class);
assertThat(response2.getTotalResults()).isEqualTo(100);
assertThat(response2.getItemsPerPage()).isEqualTo(0);
assertThat(response2.getStartIndex()).isNull();
assertThat(response2.getResources())
.isNotNull()
.isEmpty();

// This response is technically invalid since it should use an empty array.
// However, if a SCIM service is willing to return null when totalResults is
// 0, they might do the same for itemsPerPage. The SCIM SDK permits this
// operation in order to make it easier to work with these SCIM services.
String itemsJSON = "{ "
+ " \"schemas\": ["
+ " \"urn:ietf:params:scim:api:messages:2.0:ListResponse\""
+ " ],"
+ " \"totalResults\": 100,"
+ " \"itemsPerPage\": 0"
+ "}";
ListResponse<?> response3 = reader.readValue(itemsJSON, ListResponse.class);
assertThat(response3.getTotalResults()).isEqualTo(100);
assertThat(response3.getItemsPerPage()).isEqualTo(0);
assertThat(response3.getStartIndex()).isNull();
assertThat(response3.getResources())
.isNotNull()
.isEmpty();
}
}

0 comments on commit dc8144f

Please sign in to comment.