koemeet / JsonApiBundle

Integration of JSON API with Symfony using JMS Serializer
55 stars 26 forks source link

Circular reference is not being handled well right now, how can we check if relationship is already in `data`? #2

Closed koemeet closed 9 years ago

koemeet commented 9 years ago

Currently, we can keep track of all the included data, but not for the primary resources stored under data. How can we check if a relationship from an included record is already in the primary data?

silverqx commented 9 years ago

First, great progress, I really like how you resolved whole architecture and implementation, hats off :clap:

Can you pls paste here some json output of this scenario? Because I can't imagine what it exactly means.

I hope that I'm not tiresome. :grimacing:

koemeet commented 9 years ago

@szachara Thanks :smile:! Sure this is how it should output a circular relationship:

{
    "data": [
        {
            "type": "posts",
            "id": 1,
            "attributes": {
                "tiiitel": "My post",
                "description": "This is the first post"
            },
            "relationships": {
                "comments": {
                    "data": [
                        {
                            "type": "comments",
                            "id": 1
                        }
                    ]
                }
            }
        }
    ],
    "included": [
        {
            "type": "comments",
            "id": 1,
            "attributes": {
                "title": "This is a comment title",
                "body": "My body"
            },
            "relationships": {
                "post": {
                    "data": [
                        {
                            "type": "posts",
                            "id": 1
                        }
                    ]
                }
            }
        }
    ]
}

The problem is, is that in the JsonEventSubscriber it now always serializes the relationship. This is done by the following code:

 $this->includedRelationships[$relKey . ':' . $id] = $context->accept($item);

We can keep track of existing included relationships, but I need to fix it too with a circular reference to the primary data (in this case posts).

koemeet commented 9 years ago

@szachara I think I have a way of solving it. The best way I can think of right now is that we just serialize the duplicate (only for the primary resource) and filter those out in the JsonApiSerializationVisitor::getResult. Since there is a lot of recursion going on, we cannot do this in the listener (atleast none I can think of).

I will prepare a solution, so this issue can be closed ;)

silverqx commented 9 years ago

It's hacky, but good and fast solution and I think it's sufficient for now. Few weeks ago I solved the same problem, but in other context, I did it through jms serializer maxdepth exclusion strategy, but it doesn't fit this scenario.

I should fork this repo and debug it, but I'm so lazy now. ;-) sry

koemeet commented 9 years ago

@szachara I solved it by calculating a difference between the data and included. If the included got one that is already in data, it will filter it out correctly.

$included = array_udiff($included, $result, function ($a, $b) {
    return strcmp($a['type'] . $a['id'], $b['type'] . $b['id']);
});

Will push those changes now ;)

silverqx commented 9 years ago

One thing, relationships objects can't be in the included array's resource objects. Simply resource objects in the included array can't contain relationships objects. Is this true, right?

koemeet commented 9 years ago

@szachara If you mean that you can't have relationship objects nested in each other, then you are right. Everything is on the same level and are referenced to each other by ids.

silverqx commented 9 years ago

It's absolutely true, that you can't have relationship objects nested in each other.

But I mean that relationships objects can't be in included array anywhere. When I check json api docs, so I haven't seen any relationships objects in included array.

koemeet commented 9 years ago

@szachara Ah I think I know what you mean. But included relationships can have relationships of there own that are in included also. Btw, I also got a Gitter chat if you have simple questions or you need help. You can see the link in the README file.