Skip to content
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.

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

InputTypeGenerator::mapFactoryMethod doesn't call field middleware. #439

Open
withinboredom opened this issue Feb 5, 2022 · 7 comments
Open

Comments

@withinboredom
Copy link

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
Copy link
Author

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

@oojacoboo
Copy link
Collaborator

oojacoboo commented Feb 5, 2022

@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?

@withinboredom
Copy link
Author

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.

@oojacoboo
Copy link
Collaborator

@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.

@withinboredom
Copy link
Author

withinboredom commented Feb 7, 2022

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 {}

@oojacoboo
Copy link
Collaborator

oojacoboo commented Feb 11, 2022

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.

@Lappihuan
Copy link
Contributor

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants