Bring input setters on par with type getters

[Lappihuan]

The goal of this PR is to bring setters on Input Annotated Objects on par with getters.
Since the introduction of Input Types in #269 setters were very simple and did not bring any Field Middleware functionality that getters have (Autowire, Security, Right, Assert etc..).

With this Update setters have their own InputFieldMiddleware system that is pretty much 1:1 to the FieldMiddleware system allowing Autowire, Security and Right annotations to work.
Assertation would need to be added separately into the respective repositories for Symfony and Laravel.

While this seems like a relatively small addition, this allows for a very potent pattern with Doctrine Entities.
I will explain this pattern in depth as a example but this PR brings many more possibilities not outlined here.

Entities can now not only have computed fields on getters but also on setters leveraging the power of Autowire to set up references for example:

#[ORM\Entity]
#[GQL\Type(default: true)]
#[GQL\Input(name: "UpdateAccountInput", update: true, default: true)]
class Account {
//...
    #[ORM\ManyToOne]
    #[ORM\JoinColumn(name: "lang_id", referencedColumnName: "lang_id", nullable: false)]
    private Language $lang;
//...
    #[GQL\Field]
    public function setLang(LanguageEnum $lang, #[GQL\Autowire] EntityManagerInterface $entityManager): void
    {
        $this->lang = $entityManager->getReference(Language::class, $lang->getValue());
    }

    #[GQL\Field]
    public function getLang(): LanguageEnum
    {
        return LanguageEnum::from($this->lang->getLangId());
    }
}

This allows for a very clean separation of ORM responsibility around the private properties and the public API using the setter and getter functions without the need to map between DTOs and Entities, there is even potential for nested graph traversing in mutations but this would need to be figured out.

Additionally using Doctrines Embeddables one can fully abstract the public data structure while still working with Doctrine Entities that can be persisted without any mapping.

Unrelated to this lib we even have a merge service that allows to persist partially hydrated Entities like this:

    #[GQL\Mutation]
    public function updateAccount(
        ID $id,
        #[GQL\UseInputType(inputType: "UpdateAccountInput!")]
        Account $account
    ): Account
    {
        $account->setID($id);
        $merged = $this->mergeService->merge($account);
        $this->entityManager->flush();
        return $merge;
    }

The as for Validation directly on setters, i will make a PR on the Symfony Validator Bridge Repo asap.

There is still documentation to be made around this feature and maybe some edge cases to be discussed.

@oojacoboo there are some commits left over from your input improvements since we initially based our branch on your.
Let me know if this needs cleanup or can be ignored.

Overall working on this PR gave us a lot of insights into how this lib is working and i already have a few more ideas on how to improve it :)

Looking forward to your feedback!

[Lappihuan]

@oojacoboo the field annotation is fine on properties, as long as there exists a public setter method.
I removed the exceptions from the setters and the test is not failing now.

not sure whats you see wrong with the constructor

[oojacoboo]

Copied that over from another test, I wasn't able to test that it was properly failing. Looks like it'll have to be adjusted. We're getting the following error on our unit tests

TypeError: Foo\GraphQL\Operation\Employment\InputType\Employment::__construct(): Argument #1 ($startDate) must be of type ?Foo\Date, string given

Foo\Date is resolved with a custom root type mapper.

[Lappihuan]

@oojacoboo constructor hydration proves to be challenging in the way it does not integrate with the whole middleware pattern.

The middlewares can go and wrap a field with other logic like for example Security, or any other user defined middleware. Now in those middlewares need to have access to the resolver and from those resolvers you are always supposed to get the DTO you are working with. In the case of the Security middleware you can use it to access properties on the DTO in the expression via this so if you have a secret Field on the DTO you can check it in the expression via this.secret == "Foo".

This works well in the case of setting properties directly or via setters since the DTO is already instantiated. But with Fields that are supposed be hydrated via constructor this whole system does not make sense. 🐔🥚

I feel like constructor hydration has to be a second class citizen and not support any middlewares.

I will try and come up with a implementation that determines if a field is hydrated via constructor and if so it will skip all the fancy FieldDescriptor and middleware stuff and just be used to instantiate the DTO before its passed to hydration via setters and properties.

Any toughs?

[oojacoboo]

@Lappihuan I agree with skipping all the fancy business with the constructor. Let's just get it instantiating and then we can do all the other middleware on the other fields being hydrated. I'd be nice to check for fields being hydrated via the constructor to see if they're implementing any middleware logic, and if so, throw an exception.

There is one other option - fully constructing and hydrating the object and then post-processing all the middleware. I suspect that this isn't worth it and would require recursively resolving all the fields in 2 passes - performance hit.

[Lappihuan]

There is one other option - fully constructing and hydrating the object and then post-processing all the middleware. I suspect that this isn't worth it and would require recursively resolving all the fields in 2 passes - performance hit.

@oojacoboo Since i already do a second pass for setters and properties this wouldn't be a big deal.
But it assumes there is a way to do so. If the constructor is the only way to hydrate it would not work as expected.

I think is should be able to throw if middleware annotations are detected for constructor hydrated fields. 👍

Working on it right now.

[oojacoboo]

@Lappihuan if you're already having to do 2 passes, using reflection, you can check which fields are being hydrated via the constructor and simply re-resolve them... I don't love it, but it would retain functionality and I don't see any real issues with it.

[Lappihuan]

@oojacoboo now i only do one pass for all constructor fields and one for setters/property fields. I detect early if the property is hydrated via constructor and if so check if middleware annotations are used if so i throw a exception explaining its not supported, else i just skip all the middleware stuff and resolve the properties for instantiation, after instantiation the new stuff kicks in. fields that are used in the constructor are pretty close to how they are handled in the lib right now.

[Lappihuan]

@oojacoboo how would you like to proceed?
It seems like most bugs should be ironed out by now.

[oojacoboo]

@Lappihuan I'll give it a test again ASAP.

[oojacoboo]

@Lappihuan good news, I've had a chance to test the latest code with this PR on a large application - all tests are passing!

I've added a code review here. We probably should focus on getting phpcs rules updated to catch a lot of these code style comments I've made, then get that passing.

Looking forward to getting this merged in. Thanks again for all the work on this!

[Lappihuan]

@oojacoboo let me know if there is something to change on the unresolved points

[oojacoboo]

@Lappihuan thanks, added some more comments. In general, if we're leaving commented out code, we should at least have a comment saying why it's there so it doesn't cause any confusion.

[oojacoboo]

@Lappihuan I've merged in this PR. Documentation wasn't completed and I'm doing that now, in addition to versioning the docs for a 6.0 release.