Support empty collections in _embedded section

[ghostwriter]

Q	A
Bugfix	yes

Description

The goal/convention is to include both non-empty and empty collections within the _embedded section of the response, to maintain consistency in the structure of the response, regardless of whether the collection is empty or not.

Discussion: https://laminas.slack.com/archives/C4RF57B0E/p1686214950164379

Maybe Related #4

---

empty-contacts-collection-not-embedded.json

{
  "contacts": []
}

empty-contacts-collection-embedded.json

{
  "_embedded": {
    "contacts": []
  }
}

non-empty-contacts-collection.json

{
  "_embedded": {
    "contacts": [
      {
        "id": 1,
        "name": "Jane",
        "email": "jane@example.com"
      },
      {
        "id": 2,
        "name": "John",
        "email": "john@example.com"
      }
    ]
  }
}

---

The user also explained that adding the code below to the constructor worked for them (in case we need to refactor the constructor)

if ($value === null || $value === []) {
    $this->embedded[$name] = [];
    return;
}

[ghostwriter]

Hey @weierophinney,

I would greatly appreciate your valuable insight to ensure its completeness, per your offer.

Thank you in advance. 🙏🏾

[froschdesign]

@ghostwriter
I would not consider the missing of the _embedded section as an error, because the specification explicitly marks this element as optional. And if nothing is present, then the entire section can be omitted.
The specification only assumes existing objects:

…values are either a Resource Object or an array of Resource Objects.

https://datatracker.ietf.org/doc/html/draft-kelly-json-hal#section-4.1.2

---

The problem I see here is that there is a change in the output and this can cause errors in existing applications and tests.

[ghostwriter]

@froschdesign I agree, the missing of the _embedded section is not the issue.

The pr/discussion is regarding: an empty array of Resource Objects.

non-empty and empty collections within the _embedded section of the response

Please look at the test fixtures for examples.

https://github.com/mezzio/mezzio-hal/pull/80/files

[froschdesign]

@ghostwriter
I understand your suggestion, but the problem is still that the output changes and this can potentially cause tests to fail for existing API endpoints.

[weierophinney]

the problem is still that the output changes and this can potentially cause tests to fail for existing API endpoints.

We have two competing issues:

• A number of users would like empty collections to appear in the _embedded section, as it simplifies consumers of their APIs. They can rely on the value being present, even if empty, which can ensure that payloads are valid, and no extra logic has to be created to create an empty collection based on absence of the collection in the payload.
• Doing so by default would cause issues for those who relied on empty collections either not being present or being in a different part of the payload.

I'd recommend we make this functionality opt-in, then. @ghostwriter , do you have any ideas on how you could adapt the PR to enable that?

[ghostwriter]

my idea was to introduce this change as a bug fix with a breaking change in a new minor version and communicate the breaking change to your users and provide documentation on how to handle the change in their applications.

based on our discussion in Slack, I was under the impression we all wanted to include both non-empty and empty collections within the _embedded section of the response, to maintain consistency in the structure of the response, regardless of whether the collection is empty or not.

[ghostwriter]

based on both of your feedbacks, it now uses the configuration key config.mezzio-hal.embed-empty-collections to optionally enable and disable this behavior.

[weierophinney]

This is looking great, @ghostwriter !

My primary concern at this point is that it looks like you rewrote existing tests that tested the existing behavior to test the behavior when the flag is toggled. Since we'll be doing both behaviors, we need to ensure we also test the default behavior.

[ghostwriter]

$resource === [] || because old behavior allows empty arrays.

[ghostwriter]

Ready for review.

[froschdesign]

Only minor changes to the documentation are necessary. Otherwise it looks good. 👍🏻

[weierophinney]

Found one little typo, but otherwise, all my feedback has been addressed! I'll apply that change and merge; thanks for the patch, @ghostwriter ! 🎉

[weierophinney]

ARGH - this was targeted at 2.7.x, but slated for 2.8.0. I'll figure out how to untangle it.