[5.8] Fix how resources handle arrays with mixed numeric and string keys

[rodrigopedra]

Since PR #23414 the way Resources handle payload with numeric keys has changed breaking the way some users expected them to be returned.

After this PR, any array in a Resource that had a numeric value in its first key was converted to a plain JSON array. For example, assume we have this route:

Route::get('/', function () {
    return new \Illuminate\Http\Resources\Json\JsonResource([
        1 => 10,
        2 => 20,
        'total' => 30
    ]);
});

Currently the response would be:

{"data":[10,20,30]}

While as a user might excpect it to be:

{"data":{"1": 10, "2": 20, "total": 30}}

This problem was tried to be solved in some other PR's ( #24339 , #27325 , #27384 ). And some use-cases were described in the comments within these PR's.

The example above (having a total key and the payload) was described by @hotmeteor in the PR #23414 comments ( link to the comment ).

Other valid use case, described by @SethTompkins in the comments of PR #24339 ( link to the comment ) was to adopt a recommendation from the Redux JavaScript framework to "normalize frontend state to allow for easy transaction and lookups with api resources". ( Reference: https://redux.js.org/recipes/structuring-reducers/normalizing-state-shape#relationships-and-tables )

In the example from Redux, linked above, it suggests to return a lookup-table shaped like this (simplified):

{
    entities: {
        authorBook : {
            byId : {
                1 : { id : 1, authorId : 5, bookId : 22 },
                2 : { id : 2, authorId : 5, bookId : 15 },
                3 : { id : 3, authorId : 42, bookId : 12 }
            },
            allIds : [1, 2, 3]
        }
    }
}

Note that the entities.authorBook.byId property is a JavaScript object with numeric keys.

In the current form we could not return the data shaped like this example. Currently a Resource when serialize its data it only checks if the first key of an array is numeric and, if so, converts all the array to a plain array. The data returned would look like this:

{
    entities: {
        authorBook : {
            byId : [
                { id : 1, authorId : 5, bookId : 22 },
                { id : 2, authorId : 5, bookId : 15 },
                { id : 3, authorId : 42, bookId : 12 }
            ],
            allIds : [1, 2, 3]
        }
    }
}

Which is different than expected.

If this PR gets merged, a user could define a $preserveKeys = true property in the Resource returning this data structure and the keys within the entities.authorBook.byId would be preserved.

This PR does the following changes:

1. Change how the ConditionallyLoadsAttributes@removeMissingValues methods detects if an array has only numeric keys, by using an sentinel flag while it iterates to the array element.

   This is a breaking change, as it would check all keys in a array before converting it to a 'plain' without further modification from a user (see first use-case above).

2. A user can now define a $preserveKeys to a resource, so only actual plain arrays (array_values($data) === $data) are converted to a JS array, anything different than that have their keys preserved.

---

Much of the code in this PR was possible summing up on the previous work and comments from @staudenmeir in PR's #24339 , #27325 .

[taylorotwell]

Please give a better summary of breaking changes.

I don't understand why "1." is a breaking change at all. Needs much more explanation.

Regarding "2.", I don't know what you mean by "actual plain arrays". Also needs much more explanation.

[rodrigopedra]

Hi @taylorotwell , I'll try my best, to clarify the changes, English is not my native language, if any doubts are left please do not hesitate to ask.

1. Change on how arrays were serialized.

The previous code on ConditionallyLoadsAttributes@removeMissingValues method, just checked if the first key on an array was a numeric.

If the first key was numeric it returned the result of array_values($arr).

This a BC, if we consider the following code:

Route::get('/', function () {
    return new \Illuminate\Http\Resources\Json\JsonResource([
        1 => 10, // note that the first key is_numeric()
        2 => 20,
        'total' => 30 // this array has a string key, which was ignored before
    ]);
});

Before we would have:

{"data":[10,20,30]}

After this PR:

{"data":{"1": 10, "2": 20, "total": 30}}

This changed beahavior happens as now we use a sentinel to check if the array being evaluated by the ConditionallyLoadsAttributes@removeMissingValues method, has any non-numeric keys. I chose to use a sentinel as this method already loops through each array element, so this approached added a minimal overhead.

As the output changes for the same input before and after this PR, I think it might be documented as a BC.

2. "Plain" arrays

By "plain" array, I meant an array that the following expression evaluates to true:

array_values($arr) === $arr;

Which means (of course you know), that the array is empty, or that the first key is 0 and all the following keys are numeric keys in ascending order.

As internally all PHP arrays are associative, I didn't find a better term to differentiate between arrays that might have non-numeric or non-contiguous keys and the kind of array described above.

Considering this definition, this PR introduces the possibility to a user define a $preserveKeys property on a resource. When this property exists and is equal to true, the way arrays are serialized changes.

Only arrays which the expression above (array_values($arr) === $arr) evaluates to true are going to be outputted as JSON arrays. All the other arrays are going to be outputted as JSON objects.

So, if you have this situation:

Route::get('/', function () {
    // MyResource has $preserveKeys = true;
    return new MyResource([
        'foo' => [10, 20, 30], // array_values($foo) === $foo
        'bar' => [1 => 10, 20, 30], // first key is not zero, so array_values($bar) !== $bar
    ]);
});

The output would be:

{"foo":[10, 20, 30], "bar": {"1": 10, "2": 20, "3": 30}}

Although weird, this allows the user to use techniques such as described in the Redux docs, which require objects as lookup-tables.

Note that this behavior should be explicitly enabled by the user by adding $preserveKeys = true to a resource.

If the $preserveKeys is not set to true the behavior described in the first topic is applied.

[staudenmeir]

@rodrigopedra While working on #27950, I noticed that the example from the documentation ("Preserving Collection Keys") doesn't actually work.

$preserveKeys = true; only works with new UserResource() (as tested), but not with UserResource::collection(). Presevering keys in a collection currently requires a separate UserResourceCollection class: #27950 (comment)

How do we fix this?

[rodrigopedra]

I am taking a look right now and, if needed, will open a new PR

[rodrigopedra]

Hi @staudenmeir

The problem is when the AnonymousResourceCollection hits the ConditionallyLoadsAttributes@removeMissingValues method, the check verify if the current class has a $preserveKeys, which of course it doesn't.

I think of two approaches:

1. Add dynamically the $preserveKeys to the AnonymousResourceCollection in instantiation if the collected resource has it
2. Add a method to verify it in the ConditionallyLoadsAttributes

I will submit method 2 as a new PR, as it seems to me the better one, but please send me your opinions and suggestions.

Will also add tests