yin icon indicating copy to clipboard operation
yin copied to clipboard

Problem with the resource transformer design pattern and cross relations

Open gfemorris opened this issue 7 years ago • 9 comments

When using cross relations between 2 resources the current design pattern used by the yin resource transformers has the issue of circular dependencies. I think it is necessary to cross relate objects. For example:

  • You have a book object and want to know the author(s)
  • You have an author object and want two know books written by the author

I solved this in my case outside of the lib with injecting just a container into the transformers which can require the transformers on demand. But in my opinion this should be solved by the lib itself.

The neomerx lib solves this by mapping the transformers "hard" with the models.. but i liked your modular "loose" approach here.. so maybe a factory patter where one needs to register all transformers and a factory where you can get them whenever you need them could solve the problem.

gfemorris avatar Dec 18 '17 10:12 gfemorris

Hi, sorry to bother you again... it would be very nice if you could check this, because i think it is a bigger issue which everyone will face sooner or later. It would be best if it would be solved inside the lib, everything else feels like a dirty workaround.

Thanks anyway for this great lib and your hard work!!!

gfemorris avatar Mar 05 '18 14:03 gfemorris

Hey,

Sorry for leaving your issue unanswered so long, and thanks for bringing this up again. In the current months I am overwhelmed with other tasks and really busy at work. :/ So I didn't have possibility to think about the solution and respond you.

I'll answer you in detail in some days (and definitely not in many days)!

kocsismate avatar Mar 09 '18 17:03 kocsismate

Hey, thanks for your reply. You just answer whenever you have time.. I just wanted to bring it up again so that it is not forgotten.

gfemorris avatar Mar 09 '18 17:03 gfemorris

I have thought about the problem since our last messages, and I believe, it should be solved outside of the library indeed. Here are the solutions which might work for you in my opinion:

  • The quick and dirty solution might be using property or setter injection if you have a DI Container. These methods effectively mitigate the problem of circular dependencies. Usually, I don't recommend these dependency injection types because they make a class not easy to unit test, but in this case, it's not a serious issue as JSON:API transformation is not usually unit tested.

  • A child class of an AbstractTransformer can have any dependency so you can implement your own BookTransfomerFactory which can create an AuthorTransformer or BookTransformer class. I admit that a Factory like that might not be easy to implement.

What do you think about these ideas? To be honest, I don't like the idea to register transformers because that would require an own Service Locator to be implemented and a plain old DI Container is just perfect for this purpose.

Sorry for the late response again.

kocsismate avatar Mar 19 '18 18:03 kocsismate

Sorry didn't have the time to think it through.. but i will reply in detail soon... thanks for your ideas.

gfemorris avatar Apr 09 '18 13:04 gfemorris

I don't think creating the graph of how transformers are related is the best alternative. I think you should specify the Transformer class when you create relationships. So it would looks like:

public function getRelationships($book): array
    {
        return [
            "authors" => function (array $book) {
                return ToManyRelationship::create()
                    ->setLinks(
                        Links::createWithoutBaseUri()->setSelf(new Link("/books/relationships/authors"))
                    )
                    ->setData($book["authors"], \AuthorTransformer)
                ;
            },
            "publisher" => function ($book) {
                return ToOneRelationship::create()
                    ->setLinks(
                        Links::createWithoutBaseUri()->setSelf(new Link("/books/relationships/publisher"))
                    )
                    ->setData($book["publisher"], \PublisherTransformer)
                ;
            }
        ];
    }

You could instantiate them as needed and remove the need for this:

// Instantiating a book document
    $document = new BookDocument(
        new BookResourceTransformer(
            new AuthorResourceTransformer(),
            new PublisherResourceTransformer()
        )
    );

guilhermeaiolfi avatar Jun 06 '18 18:06 guilhermeaiolfi

@guilhermeaiolfi Yes i came up with that idea as well and actually solved it like that in another project. Sometimes the easy solutions are the hardest to find.. :D And i totally agree, that this is the best approach here. The only thing considering would be, that we will end up with a lot of transformer instances. Transformers are stateless so they could be singletons without any issues. Maybe @kocsismate should just change the examples to that easy approach because i'm sure a lot of people are running into the same issues here.

gfemorris avatar Oct 25 '18 14:10 gfemorris

I know it's been a while, but I came across the same problem yesterday. I have an Offer entity that has many Images and each Image entity is related back to an Offer. I wanted to be able to include both Images with Offer and Offer with Image. After trying and failing for a while, I got it to work using Symfony's setter injection. The main trick is not to inject anything in the constructor because this is what makes the reference circle.

In OfferResourceTransformer add the setter for ImageResourceTransformer:

    private $imageResourceTransformer;

    public function setImageResourceTransformer(ImageResourceTransformer $imageResourceTransformer): void
    {
        $this->imageResourceTransformer = $imageResourceTransformer;
    }

In ImageResourceTransformer add the setter for OfferResourceTransformer:

    private $offerResourceTransformer;

    public function setOfferResourceTransformer(OfferResourceTransformer $offerResourceTransformer): void
    {
        $this->offerResourceTransformer = $offerResourceTransformer;
    }

In services.yaml add:

    App\JsonApi\Transformer\OfferResourceTransformer:
      calls:
        - method: setImageResourceTransformer

    App\JsonApi\Transformer\ImageResourceTransformer:
      calls:
        - method: setOfferResourceTransformer

And now it works like a charm!

jakagacic avatar Mar 20 '19 09:03 jakagacic

Hi @jakagacic,

Thanks for sharing your example!

To give an answer to the prior discussion: as @guilhermeaiolfi suggested, transformers are not at all needed to be instantiated ahead of time, a DI container can easily do the job on-the-fly.

I don't plan to add better support for service location of transformers in Yin as I believe it can be still conveniently done in user-land code. However, I agree with @gfemorris , it could be noted in the examples and also in the read me that you have a choice when to instantiate the transformers (ahead-of-time via Dependency Injection or just-in-time via Service Location).

kocsismate avatar Mar 29 '19 13:03 kocsismate