graphqlite icon indicating copy to clipboard operation
graphqlite copied to clipboard

InputTypeGenerator::mapFactoryMethod doesn't call field middleware.

Open withinboredom opened this issue 3 years ago • 7 comments

When generating fields from a factory method, the field middleware appears to never be called for the generated fields. I expected to be able to do something like:

<?php

// usings here

class Example {
  #[Factory]
  public function exampleFactory (
    #[AutoWire] 
    Something $something, 
    #[CustomAttribute] 
    string $field
  ) {
    // do stuff
  }
}

For now, the workaround appears to be to manually specify an input type instead of a factory?

withinboredom avatar Feb 05 '22 21:02 withinboredom

Would you accept a PR to fix this? I may have some time next weekend.

withinboredom avatar Feb 05 '22 21:02 withinboredom

@withinboredom Can you please describe this issue in more detail. What field middleware are you referring to, and in your example, what is it you're expecting to happen?

oojacoboo avatar Feb 05 '22 22:02 oojacoboo

This field middleware: https://graphqlite.thecodingmachine.io/docs/field-middlewares

I'd expect the input type's fields to recognize my custom attribute and apply whatever customization to the field.

withinboredom avatar Feb 06 '22 09:02 withinboredom

@withinboredom You're going to have to do a better job in explaining if you'd like some input on this. I still don't know what you're trying to do really.

oojacoboo avatar Feb 07 '22 02:02 oojacoboo

For example, I have the following attribute:

<?php

namespace Attributes;

use Attribute;
use TheCodingMachine\GraphQLite\Annotations\MiddlewareAnnotationInterface;

#[Attribute(Attribute::TARGET_FUNCTION |
    Attribute::TARGET_CLASS |
    Attribute::TARGET_PARAMETER |
    Attribute::TARGET_METHOD)]
class Description implements MiddlewareAnnotationInterface
{
    public function __construct(public readonly string $description)
    {
    }
}

and the following middleware:

<?php

namespace Middlewares;

use GraphQL\Type\Definition\FieldDefinition;
use Attributes\Description;
use TheCodingMachine\GraphQLite\Middlewares\FieldHandlerInterface;
use TheCodingMachine\GraphQLite\Middlewares\FieldMiddlewareInterface;
use TheCodingMachine\GraphQLite\QueryFieldDescriptor;

class DescriptionMiddleware implements FieldMiddlewareInterface
{
    public function process(
        QueryFieldDescriptor $queryFieldDescriptor,
        FieldHandlerInterface $fieldHandler
    ): ?FieldDefinition {
        $annotations = $queryFieldDescriptor->getMiddlewareAnnotations();
        
        /**
         * @var Description|null $description
         */
        $description = $annotations->getAnnotationByType(Description::class);
        
        if (null === $description) {
            return $fieldHandler->handle($queryFieldDescriptor);
        }
        
        $queryFieldDescriptor->setComment($description->description);
        
        return $fieldHandler->handle($queryFieldDescriptor);
    }
}

Fields allow you to do something like:

#[Field(description: 'The name of the user')]
public function getName(): string { }

But I also wanted them on things like controllers:

	#[Query]
	#[Logged]
	#[FailWith(value: null)]
	#[Description('This is everything about you.')]
	public function me(#[InjectUser] User $user): User
	{
		return $user;
	}

or on the fields in a mutation

    #[Mutation]
    #[Logged]
    #[Right(Rights::SubscriptionValue)]
    #[Description('Delete a contact')]
    public function deleteContact(
        #[Description('The contact to delete')]
        #[UseInputType(ContactRepository::TYPE_SELECTOR_REQ)] Contact $contact
    ): Contact {}

However, in factories, it doesn't work because the field middleware doesn't appear to be called at all for the generated fields:

    #[Factory(name: self::TYPE_INPUT, default: false)]
    public function createContact(
        #[Description('a contacts name')]
        string $name
    ): Contact {}

withinboredom avatar Feb 07 '22 09:02 withinboredom

Ah, thanks for the explanation. This is indeed poorly implemented at the moment. And not just in this scenario. There are multiple scenarios where the description isn't consistent across the API. For example, the PHPDoc is used if a Field description isn't provided. And support on queries and mutations could expose internals by relying on the PHPDoc.

A more comprehensive approach to this needs to be taken. I already have concerns about there being too many attributes and things not being as cohesive as they should.

As for a PR, absolutely. This would be great. If you'd like to review the API and put forward some further suggestions on this that'd be a good place to start. If not, I may have some time over this weekend to review this.

Personally, I think relying on the PHPDoc, while nice, is a bad approach that could expose internals. It also fragments the API/use of this lib and makes it confusing. IMO, we should just have a description on every attribute that can expose it over the GraphQL schema and only rely on that.

oojacoboo avatar Feb 11 '22 09:02 oojacoboo

@withinboredom have a look at this PR #466 this would be possible to implement when afterwards. alltought factories are probably still handled in a diffrent way, i did not touch those.

Lappihuan avatar Apr 22 '22 18:04 Lappihuan