Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mapped NULL values are serialized outside of the _embedded key #4

Open
2 tasks done
weierophinney opened this issue Dec 31, 2019 · 9 comments
Open
2 tasks done

Comments

@weierophinney
Copy link
Contributor

Provide a narrative description of what you are trying to accomplish.

Code to reproduce the issue

User.php

namespace App;

class User
{
   /**
    * @var string
    */
   protected $id;

   /**
    * @var string
    */
   protected $name;

   /**
    * @var string
    */
   protected $email;

   /**
    * @var Avatar
    */
   protected $avatar;

   public function getName(): ?string
   {
       return $this->name;
   }

   public function setName(?string $name): User
   {
       $this->name = $name;
       return $this;
   }

   public function getEmail(): ?string
   {
       return $this->email;
   }

   public function setEmail(?string $email): User
   {
       $this->email = $email;
       return $this;
   }

   public function getAvatar(): ?Avatar
   {
       return $this->avatar;
   }

   public function setAvatar(?Avatar $avatar): User
   {
       $this->avatar = $avatar;
       return $this;
   }
}

Avatar.php

namespace App;

class Avatar
{
   /**
    * @var string|null
    */
   protected $id;

   /**
    * @var string|null
    */
   protected $url;

   public function getId(): ?string
   {
       return $this->id;
   }

   public function setId(?string $id): User
   {
       $this->id = $id;
       return $this;
   }

   public function getUrl(): ?string
   {
       return $this->url;
   }

   public function setUrl(?string $url): User
   {
       $this->url = $url;
       return $this;
   }
}

config.php

MetadataMap::class => [
    [
        '__class__' => RouteBasedResourceMetadata::class,
        'resource_class' => App\User::class,
        'route' => 'api.user',
        'extractor' => ClassMethodsHydrator::class,
    ],
    [
        '__class__' => RouteBasedResourceMetadata::class,
        'resource_class' => App\Avatar::class,
        'route' => 'api.avatar',
        'extractor' => ClassMethodsHydrator::class,
    ],
],
$renderer = new JsonRenderer();
$avatar = new Avatar();
$avatar->setId('1234');
$avatar->setUrl('https://superfastcdn.com/myavatar.png');
$user = new User();
$user->setId('5678');
$user->setName('John Doe');
$user->setEmail('john@doe.com');
$user->setAvatar($avatar);

Expected results

$resource = $resourceGenerator->fromObject($user, $request);
echo $renderer->render($resource);

$user->setAvatar(null);

$resource = $resourceGenerator->fromObject($user, $request);
echo $renderer->render($resource);
{
    "name": "John Doe",
    "email": "john@doe.com",
    "_embedded": {
        "avatar": {
            "id": "1234",
            "url": "https://superfastcdn.com/myavatar.png",
            "_links": {
                "self": {
                    "href": "https://api.acme.com/avatars/1234"
                }
            }
        }
    },
    "_links": {
        "self": {
            "href": "https://api.acme.com/users/5678"
        }
    }
}
{
    "name": "John Doe",
    "email": "john@doe.com",
    "_embedded": {
        "avatar": null
    },
    "_links": {
        "self": {
            "href": "https://api.acme.com/users/5678"
        }
    }
}

Actual results

{
    "name": "John Doe",
    "email": "john@doe.com",
    "_embedded": {
        "avatar": {
            "id": "1234",
            "url": "https://superfastcdn.com/myavatar.png",
            "_links": {
                "self": {
                    "href": "https://api.acme.com/avatars/1234"
                }
            }
        }
    },
    "_links": {
        "self": {
            "href": "https://api.acme.com/users/5678"
        }
    }
}
{
    "name": "John Doe",
    "email": "john@doe.com",
    "avatar": null,
    "_links": {
        "self": {
            "href": "https://api.acme.com/users/5678"
        }
    }
}

Originally posted by @jguittard at zendframework/zend-expressive-hal#52

@weierophinney
Copy link
Contributor Author

Since you have the scenario mapped here, could you please submit a pull request demonstrating the issue? I can work on a solution this week if so.


Originally posted by @weierophinney at zendframework/zend-expressive-hal#52 (comment)

@weierophinney
Copy link
Contributor Author

@weierophinney demonstrating the issue = failing unit test ?


Originally posted by @jguittard at zendframework/zend-expressive-hal#52 (comment)

@weierophinney
Copy link
Contributor Author

Indeed!

On Mon, Jan 7, 2019, 5:47 PM Julien Guittard <notifications@github.com
wrote:

@weierophinney https://github.com/weierophinney demonstrating the
issue
= failing unit test ?


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
zendframework/zend-expressive-hal#52 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AABlVyIfRiJ4156tgWfJ0Gmg66A6L5qkks5vA9yigaJpZM4Zy6QM
.


Originally posted by @weierophinney at zendframework/zend-expressive-hal#52 (comment)

@weierophinney
Copy link
Contributor Author

@weierophinney
Copy link
Contributor Author

After seeing #53, I took a closer look at the expected/actual results you had above, and I'm not 100% convinced that what you expect is correct.

In the specification description for _embedded, it notes:

The reserved "_embedded" property is OPTIONAL

It is an object whose property names are link relation types (as defined by [RFC5988]) and values are either a Resource Object or an array of Resource Objects.

Embedded Resources MAY be a full, partial, or inconsistent version of the representation served from the target URI.

The specification is explicitly stating that the values for each relation must be an object or array of objects (what we call collections in this package).

As such, because you have assigned a null value, it cannot be considered an embedded resource, which is why the current code pushes it back into the parent resource.

One way I've looked at HAL: the complete resource is the combination of its properties, and the various _embedded properties, where the latter indicate related resources. A null value indicates there is no related resource, and thus would not be in the _embedded section. I think pushing it into the _embedded relations would be a violation of the specification, no?


Originally posted by @weierophinney at zendframework/zend-expressive-hal#52 (comment)

@weierophinney
Copy link
Contributor Author

Two things before going deeper into business considerations:

  • In my above example, avatar is a resource whose value is in the latter case set to null. The fact that it's null should not push it back to a regular property.
  • How about simply set an empty array of properties upon serialization of the embedded null object?

Originally posted by @jguittard at zendframework/zend-expressive-hal#52 (comment)

@weierophinney
Copy link
Contributor Author

The point is that, for purposes of HAL, a resource MUST be an object, and, specifically, an object with relational links. null does not fall under that category.

Neither does an empty object - UNLESS you provide relational links for it (e.g., a POST URL to which one can make a request to create a resource of that type). Even then, I'd argue the relational link goes in the parent:

{
  "_links": {
    "avatar": { "href": "/users" }
    "self": { "href": "..." }
  },
}

Originally posted by @weierophinney at zendframework/zend-expressive-hal#52 (comment)

@weierophinney
Copy link
Contributor Author

How does HAL then represent the property of an object whose value is not set? It just not represents it?
When the client of the API parses the JSON object, following my exemple, it expects the avatar property to be read into the _embedded key. Following your POV, it should first parses this entry, then if it doesn’t find the key, it should try to match it in the parent entry? Not very consistent, IMHO...


Originally posted by @jguittard at zendframework/zend-expressive-hal#52 (comment)

@weierophinney
Copy link
Contributor Author

I think the problem is assuming that a value is either under the top-level or an _embedded key. The idea on the client-side should be to create an object that merges the values from both:

.then(function(payload) {
  let user = Object.assign(payload, payload._embedded);
  delete user._embedded;
  return user;
})

If you do pre-processing of the returned payload such as the above, you will not have to worry about where in the representation a property lies; your code will just see a unified "user" object, where some of the properties just happen to be objects that also contain links.


Originally posted by @weierophinney at zendframework/zend-expressive-hal#52 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant