[5.8] Check for preserveKeys when serializing a collection from a resource

[rodrigopedra]

As commented by @staudenmeir, when setting the preserveKeys in a resource, but generating a collection from it, the keys are not preserved.

Before this PR the example in the docs should not work:

use App\User;
use App\Http\Resources\User as UserResource;

Route::get('/user', function () {
    return UserResource::collection(User::all()->keyBy->id);
});

As the JsonResource::collection returns a AnonymousResourceCollection and this class does not have a preserveKeys attribute, it fails on keeping the keys when serializing as currently we only check the $preserveKeys property in the object being serialized.

This PR adds a method to the ConditionallyLoadsAttributes trait to handle both the cases of serializing a Resoruce or a Collection of resources.

I used reflection to accomplish this, maybe to 5.9 we could change to using an Interface, I think it would clearer.

Worth noticing that @staudenmeir found this bug while working on issue #27950

[rodrigopedra]

Ping @staudenmeir . Please take a look if you have the time

[staudenmeir]

An issue I see:

class UserResource extends JsonResource
{
    public $preserveKeys = true;
}

class UserCollection extends ResourceCollection
{
    public $collects = 'App\Http\Resources\UserResource';
}

return new UserCollection(User::all()->filter->active);

Here, the keys are preserved when they shouldn't be.

Wouldn't your other suggestion solve this?

Add dynamically the $preserveKeys to the AnonymousResourceCollection in instantiation if the collected resource has it.

[staudenmeir]

What if we move this to JsonResource::collection() and add $preserveKeys = false to AnonymousResourceCollection::__construct()? This wouldn't be a breaking change and we could avoid using reflection.

[rodrigopedra]

@staudenmeir , thanks for the suggestion, it is much nicer, can you please review it?

[staudenmeir]

Looks great!

[rodrigopedra]

Thanks! @taylorotwell

While fixing it I felt that this should have been implemented using an Interface, as we do with the ShouldQueue jobs, etc. As we expect the preserveKeys property to be true if this feature is needed.

If you think this is a good approach, I can send a PR to the 5.9 branch

[TBlindaruk]

@rodrigopedra ,
I`m not sure that @taylorotwell is reading comments after PR is merged or closed.

[rodrigopedra]

@TBlindaruk ok thanks, when I have the time I will try to send a PR so we can discuss there.