Skip to content

Allow @Factory annotation directly on input type classes #285

Closed

Description

@oojacoboo

I could have sworn there was a discussion on this topic some time back. I looked around and couldn't find the ticket or a pull request, but maybe I'm overlooking it. If anyone can link that other ticket that'd be great.

But basically, this ticket is to discuss the fact that factory methods are often times nothing but boilerplate. In fact, I've only had to modify input fields a handful of times to date. These methods mostly consist of taking in params and passing them directly to properties of the input type. In fact, in our case, they already have the same names as well. I'd assume this is similar for most.

What this proposes is making the @Factory annotation optionally defined on the input type class, itself. I don't even see any need for args to handle anything here, just a direct mapping, when the annotation is defined on the input type class.

As for how the properties are populated, it's probably best that, like the factory method today, all of the properties assignments are passed to the input type class' constructor. This would also allow for some light field modification if someone deemed that necessary. For example taking a date string and instantiating a Date object to assign to the class' property.

Assuming the above with the constructor, the question then becomes, do you put the @Factory annotation on the constructor or the class?

The other thing that I'll add to this, because I know it'd be an issue for us, is validation. We do also add a simple validation call within our input type factories. Assuming this gets implement, I think there should also be a way to register an InputTypeValidatorInterface with the schema, that would then be called when an input type is constructed, passing that newly constructed input type object.

Activity

  1. oojacoboo commented on Jul 16, 2020

    @oojacoboo
    CollaboratorAuthor

    After thinking on this a little further, I've progressed my recommendation for this issue. Instead of the aforementioned solution, which requires a more opinionated approach, I'd like to see a way to register an InputType Factory Service that can handle all InputType requests. By taking this route, effectively overriding the current logic which is out looking for @Factory annotations and corresponding method names, you'd register a service that can bypass all of this, allowing for you to instantiate and build your input types however you best see fit.

    @moufmouf thoughts?

  2. oojacoboo commented on Sep 25, 2020

    @oojacoboo
    CollaboratorAuthor

    We've been able to boil down, virtually all of our factory classes and methods, to the following example. There are only a handful of InputTypes that need a defined factory method on our API now. With a registered factory "service", we'd be able to delete almost every factory class.

    /**
     * Make UpdateVehicle InputType
     *
     * @GraphQLite\Factory()
     */
    public function makeUpdateVehicle(
        ID $id,
        int $year,
        string $make,
        string $model,
        ?string $color,
        ?string $licensePlateNumber,
        ?string $licensePlateState
    ): UpdateVehicle
    {
        return $this->make(new UpdateVehicle(), \get_defined_vars());
    }

    We're able to handle domain level mapping for common type/class mappings, validation and error response through the make method.

  3. oojacoboo commented on Mar 29, 2021

    @oojacoboo
    CollaboratorAuthor

    Issue resolved with #269

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions