8000 Allow Constructors with Relationships · Issue #2169 · api-platform/core · GitHub
[go: up one dir, main page]
Skip to content

Allow Constructors with Relationships #2169

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.

Sign up for GitHub

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

Jump to bottom
Closed
ragboyjr opened this issue Aug 21, 2018 · 5 comments
Closed

Allow Constructors with Relationships #2169

ragboyjr opened this issue Aug 21, 2018 · 5 comments

Comments

@ragboyjr
Copy link
Contributor

Currently, Resource constructors will fail if they contain a relationship entity.

class Book
{
    private $id;
    private $author;
    private $title;

    public function __construct(Author $author, string $title) {
        $this->author = $author;
        $this->title = $title;
    }

    public function getId(): ?int { return $this->id; }
    public function getAuthor(): Author { return $this->author; }
    public function getTitle(): string { return $this->title; }
}

This is because the ItemNormalizer in API Platform only denormalizes a relationship entity on the setAttributeValue method call. However, the instantiateObject method call in the normalizer is done before the relationship value has been denormalized.

I think this could be trivial to implement if we override the instantiateObject, check if any of the properties are relationships, and if so, we denormalize them ahead of time so that whether a value is set via setAttributeValue or instantiateObject, it will be denormalized properly.

Thoughts?
@ragboyjr
Copy link
Contributor Author

This is the same issue as #2123

@bendavies
Copy link
Contributor
bendavies commented Aug 23, 2018 •
edited
Loading

I think there are extensive tests for this here from @komik966:
#1991

I think it's very unfortunate that #1749 was merged without these tests.

@komik966
Copy link
Contributor

I've stopped working on this, because of lack of time. Now I've more time, so I'm planning PR for this to the end of this week.
Overriding instantiateObject is solution, but I think that this method should be refactored in symfony/serializer, we should extract logic of preparation of parameter. Without such refactoring we will be enforced to think about other aspects of instantiating object - e.g. discriminators (see https://github.com/symfony/serializer/blob/master/Normalizer/AbstractObjectNormalizer.php#L127)

@dunglas
Copy link
Member
dunglas commented Aug 23, 2018

I agree. I can also merge PRs on the SF Serializer, don't hesitate to ping me on the Symfony slack!

@komik966 komik966 mentioned this issue Aug 25, 2018
Merged
2 tasks
@ragboyjr
Copy link
Contributor Author
ragboyjr commented Oct 5, 2018

I think we can close this now that @komik966's changes have been merged.

Thanks!

@ragboyjr ragboyjr closed this as completed Oct 5, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@dunglas @bendavies @ragboyjr @komik966
0