thephpleague / fractal

Output complex, flexible, AJAX/RESTful data structures.
fractal.thephpleague.com
MIT License
3.53k stars 349 forks source link

Paginator links property is either empty array or filled object #433

Open tomvo opened 6 years ago

tomvo commented 6 years ago

As per this question in a wrapper library https://github.com/spatie/fractalistic/issues/36#issuecomment-355804483, I'm noticing difference in the response for the ArraySerializer for when the pagination has links or not.

When there are no links for the pagination the meta property looks like this:

"meta": {
        "pagination": {
...
            "links": []
        }
    }

When there are, it looks like this:

"meta": {
        "pagination": {
...
            "links": {
                "next": "[url]"
            }
        }
    }

You can see that in an empty resultset the links property is an array and when set, it's an object. This causes issues for devices consuming our API.

This issue is caused by the default ArraySerializer creating the links property as an empty array ([]), see here: https://github.com/thephpleague/fractal/blob/master/src/Serializer/ArraySerializer.php#L105. The solution to this would be to create an empty ArrayObject instead of a normal Array.

Of course I can fix this for my case with overloading the paginator method but would you accept a tested PR for this or is this too breaking?

ejunker commented 6 years ago

I also have this issue. It would be nice if it was consistent and always returned an object for the links and returned {} instead of [] when there are no links.

As @tomvo mentioned it looks like this could be solved with a one line change in ArraySerializer.php

Change from:

$pagination['links'] = [];

to

$pagination['links'] = new \ArrayObject();
squareborg commented 6 years ago

I agree with this, One of our App guys has issues when consuming this with Swift. Is there a reason this cannot be fixed since it appears to be quite trivial?

willishq commented 6 years ago

It appears to be quite trivial until you see under the hood, this is using associative arrays. Because of that, its very difficult to determine if an empty array is an empty associative array or an empty object.

I did try to do a fix for this over a year ago using \ArrayObject() but it created more problems, it was like going down a rabbit hole and I didn’t have THAT much free time at the time.

I will revisit this, it is a known issue and one we’ve been pondering for way to long now.

willishq commented 6 years ago

Ignore me, I was talking about this issue for empty objects in the data namespace. Everywhere that data is created, it creates it as an empty array. Fixing that issue creates problems.

Fixing this issue does not, in my opinion. If you’ve written code to treat an empty array as an empty object, and you get an empty object, would that cause breaking changes? Only if somebody is relying on a property being an array to determine if an object is empty. How likely is that?

I would love to fix this but it’d probably need a major version bump to appease people who have hacked in workarounds that would break with the fix, but then it’s not worth doing this fix without the other \ArrayObject() foxes because people would just be unhappy at a half-finished job.

How about… I add another serializer: DataArrayObjectSerializer And put this functionality in there? Anybody have any issues with that?

lordrhodos commented 5 years ago

just hit the same issue when testing our OpenAPI specs against the API output. Would love to see an empty object here as well 👍

lordrhodos commented 5 years ago

How about… I add another serializer: DataArrayObjectSerializer And put this functionality in there? Anybody have any issues with that?

I have solved this by extending the ArraySerializer and overriding the pagination method, so if the library ships with an alternative serializer one could use to achieve these changes that would be great.

stale[bot] commented 2 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed after 4 weeks if no further activity occurs. Thank you for your contributions.