-
Notifications
You must be signed in to change notification settings - Fork 118
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
[WIP] Add JsonApiSerializer #108
Conversation
ping @adrienbrault @lsmith77 |
} | ||
|
||
$visitor->addData('links', $serializedLinks); | ||
$visitor->setRoot(array('links' => $topLevelSerializedLinks)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You need to check if there is already a root, otherwise you would override existing roots, cf: https://github.com/schmittjoh/serializer/blob/master/src/JMS/Serializer/Handler/FormErrorHandler.php#L128
I don't see where it is written that there has to be top level links. About your limitation 2: the issue is that someone that build an api for this spec, won't be able to switch to hal or something else because everything will be broken. (ids in href etc) Using About your limitation 1, why not use the Representation\SimpleCollection for this ? What about serializing the self link in both the href and id property ? |
yeah, i think it would be awesome if eventually we could get rid of these 2 limitations. in the serializer there are for example some xml specific mapping rules. this could be used to for example allow mapping the json api key so that some handler then wraps the value into an array with the key solving limitation 1) solving 2) seems to be a bit trickier, but here maybe here we can again figure out some output format specific mapping? |
For 2), we might be able to use exclusion strategies... |
I've tried something so that the id's for embedded resources are populated automatically, the only problem still, it's still depended on the method Also a problem is that the properties for the relations needs to be excluded, it would be nice if it could override them. If you do not exclude them, you will get an error saying that the property already contains data.
public function serializeEmbeddeds(array $embeddeds, JsonSerializationVisitor $visitor, SerializationContext $context)
{
$serializedEmbeds = array();
foreach ($embeddeds as $embed) {
$data = $embed->getData();
$ids = array();
if ($data instanceof \Traversable) {
foreach ($data as $object) {
if (is_callable(array($object, 'getId'))) {
$ids[] = $object->getId();
}
}
} else {
if (is_callable(array($data, 'getId'))) {
$ids[] = $data->getId();
}
}
if (!empty($ids)) {
$visitor->addData($embed->getRel(), $ids);
}
}
$serializedEmbeds[$embed->getRel()] = $context->accept($embed->getData());
$visitor->setRoot(array('linked' => $serializedEmbeds));
} Of course, we need to replace existing models inside the root, maybe just a simple |
$serializedEmbeds[$embed->getRel()] = $context->accept($embed->getData()); | ||
} | ||
|
||
$visitor->setRoot(array('linked' => $serializedEmbeds)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This needs an array_replace_recursive
, otherwise linked
is overwritten every time.
Hi, I've spent a while to take a look at Hateoas library and at issues mentioned above, and this is what I think:
To achieve that it need to be defined some common attributes for resource in every single format. I suppose such a list may look like this:
A "name" and "collectionName" is for naming conventions purposes (and some other stuff) in various formats.
This is about "resource", while "relation" could look like this:
A "class" attribute stands for a class where we can find some "@annotation\Resource", with other, earlier mentioned attributes as "name", "collectionName", "href", etc. A "type" attribute stands for type of relation (toOne, toMany). In this approach we have almost all needed information about resource and its relations. Of course "Annotation\Relation" could extends "Annotation\Resource", so attributes given here will override those ones defined in "class" class.
Maybe there is something that I've missed, but I'm pretty sure it approach may help to resolve almost all issues mentioned above. I would be grateful for Your opinion about that. Best regards, |
@Alabme I like the idea that you minimize that differences in configuration between hal and json-api. When I created my serializer for this bundle to support json-api I also had to override the LinkFactory for the same reason you mentioned. The current Link object does not contain very much information. Also, the "relation" idea has a better naming than "links". In HAL the name links would be fine, but as you said, json-api does also other things with links. This library already has a good way of seperating data formatting from serializing, but the naming of things and the intent of it should change. Also support for things like Currently I am at work, but I will think things through too. Like your ideas! |
I like the ideas too! |
(+1) |
Hi, I'd like to ask about the current state of this - I am planning a REST backend for a bigger project at the moment and i've to decide with which standard we want to go.
Best Regards, Uli |
+1 |
+1 - we're hoping to use this at http://www.gointegro.com. I was about to start work on the exact same thing. You rock, @willdurand. |
Hey, guys. I made this HATEOAS lib / bundle inspired by the effort on this PR. It's a bit different and more specific, but I thought you might want to check it out. |
Json api is stable now (1.0)! I might be able to work on some of this in about two or three months. |
URL based, see: http://jsonapi.org/format/#url-based-json-api
Limitations
This serializer works but it requires a few conditions:
href
values for link relations MUST be identifiers (still because of the JSON API spec):The output would be:
topLevel
attribute:The output would be:
I guess these requirements are acceptable.
WDYT?