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

Allow custom processors through tagged services #2149

Merged
merged 14 commits into from
Jan 2, 2024

Conversation

Luehrsen
Copy link
Contributor

This pull request aims to provide a solution to register custom processors for swagger-php through services with the tag swagger.processor.

fixes #2146

Copy link
Contributor

@Nyholm Nyholm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you.

I like it. It works. But what if we want to add a processor in the middle of the list?

Could we register Generator as a service and use constructor injection to add that service to ApiDocGenerator? If so, we would be able to re-use Generator::addProcessor($$processor, ?string $before = null).

DependencyInjection/Compiler/CustomProcessorPass.php Outdated Show resolved Hide resolved
@Luehrsen Luehrsen marked this pull request as ready for review November 25, 2023 16:08
@Luehrsen
Copy link
Contributor Author

Luehrsen commented Nov 25, 2023

Thank you for your feedback!

The 6.3.* test keep failing on me, but I think that's a general issue at the moment.

I think as of now we have a generally working solution for custom processors in swagger-php.

One thing I noticed though, is that a lot of what the API Docs Bundle does is abstracted away from the processors, particularly anything relating the ModelRegistry and ModelRegister. There is a lot of logic happening there, that is inaccessible for custom processors at this point. (At least with my skills.)

That means, that while this is a nice addition for the Bundle in general, it would not solve #1721 for example, because you need access to the '@model' and refs to do that.

I'm currently thinking about if we could find a way to pipe the ModelRegistry into a processor if needed.

Copy link
Contributor

@Nyholm Nyholm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is awesome.

I think this is great. Thank you for working on this.

DependencyInjection/Compiler/CustomProcessorPass.php Outdated Show resolved Hide resolved
@Luehrsen
Copy link
Contributor Author

Luehrsen commented Nov 25, 2023

I think it should be in the scope of this PR to extend the ModelRegister a bit to attach some more useful information to the transformed annotations, so that the processors have some "meat" to work on.

Does that make sense?

@Luehrsen
Copy link
Contributor Author

I think it should be in the scope of this PR to extend the ModelRegister a bit to attach some more useful information to the transformed annotations, so that the processors have some "meat" to work on.

Does that make sense?

To answer my own question: Nope! I got it without touching the ModelRegister.

The secret sauce is to use Nelmio\ApiDocBundle\OpenApiPhp\Util::getSchema to get all the properties you desire.

So this would be an example of the __invoke function in a custom processor registered for swagger-php. (Dropping keywords here for search.)

    const X_QUERY_AGS_REF = 'query-args-explode';

    public function __invoke(Analysis $analysis)
    {
	$this->analysis = $analysis;

        /** @var OA\Parameter[] $schemas */
        $parameters = $analysis->getAnnotationsOfType(OA\Parameter::class);

        foreach ($parameters as $parameter) {
            if ($parameter->x !== GENERATOR::UNDEFINED && array_key_exists(self::X_QUERY_AGS_REF, $parameter->x)) {
		// Generate the name from the parameter schema.
		$name = str_replace(OA\Components::SCHEMA_REF, '', $parameter->schema->ref);
		$schema = Util::getSchema($this->analysis->openapi, $name);
            }
        }
    }

This would result into $schema being a proper OpenApi\Annotations\Schema class with all the relevant properties attached. (So essentially the object the ref is pointing at.

It was registered like this in a controller:

	#[OA\Parameter(
		in: 'query',
		name: 'query',
		explode: true,
		style: 'form',
		schema: new OA\Schema(
			type: 'object',
			ref: new Model(
				type: TeamFilter::class,
				groups: ['read']
			)
		),
		x: [
			'query-args-explode' => true
		]
	)]

So to reiterate: If you want to access Nelmio\ApiDocBundle\Annotation\Model data inside a custom processor do this:

  1. Find the property where the model was used.
  2. You'll see, that the model was replaced with an almost empty Schema object that now has a ref that points to your model, that has been processed by the ModelRegistry.
  3. To get the 'name' of your model as registered in the system, just remove the OA\Components::SCHEMA_REF from your ref.
  4. The base OpenAPI object can be found at $analysis->openapi.
  5. Use these two parameters inside Nelmio\ApiDocBundle\OpenApiPhp\Util::getSchema to retrieve the glory of your model.

/**
* @param DescriberInterface[]|iterable $describers
* @param ModelDescriberInterface[]|iterable $modelDescribers
*/
public function __construct($describers, $modelDescribers, CacheItemPoolInterface $cacheItemPool = null, string $cacheItemId = null)
public function __construct($describers, $modelDescribers, Generator $generator, CacheItemPoolInterface $cacheItemPool = null, string $cacheItemId = null)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the Generator parameter should be optional here aswell right? It would prevent making a potential breaking change.

Suggested change
public function __construct($describers, $modelDescribers, Generator $generator, CacheItemPoolInterface $cacheItemPool = null, string $cacheItemId = null)
public function __construct($describers, $modelDescribers, CacheItemPoolInterface $cacheItemPool = null, string $cacheItemId = null, Generator $generator = new Generator())

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then we have to update the container registration as well. Just changing this line leads to an error.

I'm not quite sure how one would do that. Would it just be:

                ->setArguments([
                    new TaggedIteratorArgument(sprintf('nelmio_api_doc.describer.%s', $area)),
                    new TaggedIteratorArgument('nelmio_api_doc.model_describer'),
                    null,
                    null,
                    new Reference('nelmio_api_doc.open_api.generator'),
                ]);

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ye that seems fine.

ApiDocGenerator.php Outdated Show resolved Hide resolved
Co-authored-by: Djordy Koert <[email protected]>
@Nyholm
Copy link
Contributor

Nyholm commented Nov 27, 2023

I've tested this in a SF 6 application. It works as expected.

Thank you!

@DjordyKoert DjordyKoert merged commit aa162b7 into nelmio:master Jan 2, 2024
12 checks passed
@DjordyKoert
Copy link
Collaborator

Thank you @Luehrsen 😄

@Luehrsen Luehrsen deleted the add/custom-processors branch January 2, 2024 14:29
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

Successfully merging this pull request may close these issues.

Allow custom Processors for swagger-php
3 participants