scramble icon indicating copy to clipboard operation
scramble copied to clipboard

Swap order of default extensions and configured extensions

Open hn-seoai opened this issue 1 year ago • 7 comments

This allows overriding default extension responses, in case of multiple shouldHandle calls returning true.

We have a different set of pagination properties than the Laravel default, which currently forces us to use a customized fork of Scramble. With this change we won't need the fork anymore and can simply add an extension.

hn-seoai avatar Apr 15 '24 09:04 hn-seoai

Hey @hn-seoai

Thanks.

Can you please share your specific setup with some code examples? Like how your extension looks like and how you set them up.

Thanks.

romalytvynenko avatar Apr 19 '24 06:04 romalytvynenko

Extension and example controller comes here!

PaginatedResourceCollection extends AnonymousResourceCollection modifies the pagination properties. Our first workaround was to fork and change the code in LengthAwarePaginatorTypeToSchema.php.

<?php

namespace App\Support\Scramble\Extensions;

use App\Http\Resources\PaginatedResourceCollection;

use Dedoc\Scramble\Extensions\TypeToSchemaExtension;
use Dedoc\Scramble\Support\Generator\Response;
use Dedoc\Scramble\Support\Generator\Schema;
use Dedoc\Scramble\Support\Generator\Types\ArrayType;
use Dedoc\Scramble\Support\Generator\Types\BooleanType;
use Dedoc\Scramble\Support\Generator\Types\IntegerType;
use Dedoc\Scramble\Support\Generator\Types\ObjectType as OpenApiObjectType;
use Dedoc\Scramble\Support\Generator\Types\StringType;
use Dedoc\Scramble\Support\Type\Generic;
use Dedoc\Scramble\Support\Type\ObjectType;
use Dedoc\Scramble\Support\Type\Type;
use Illuminate\Http\Resources\Json\JsonResource;

class PaginatedResourceCollectionTypeToSchema extends TypeToSchemaExtension
{
    public function shouldHandle(Type $type)
    {
        return $type instanceof Generic
            && $type->name                 === PaginatedResourceCollection::class
            && count($type->templateTypes) === 1
            && $type->templateTypes[0] instanceof ObjectType
            && $type->templateTypes[0]->isInstanceOf(JsonResource::class);
    }

    /**
     * @param  Generic  $type
     */
    public function toResponse(Type $type)
    {
        $collectingClassType = $type->templateTypes[0];

        if (! $collectingType = $this->openApiTransformer->transform($collectingClassType)) {
            return null;
        }

        $type = new OpenApiObjectType;
        $type->addProperty('data', (new ArrayType())->setItems($collectingType));
        $type->addProperty(
            'meta',
            (new OpenApiObjectType)
                ->addProperty('current_page', new IntegerType)
                ->addProperty('from', (new IntegerType)->nullable(true))
                ->addProperty('last_page', new IntegerType)
                ->addProperty('path', (new StringType)->nullable(true)->setDescription('Base path for paginator generated URLs.'))
                ->addProperty('per_page', (new IntegerType)->setDescription('Number of items shown per page.'))
                ->addProperty('to', (new IntegerType)->nullable(true)->setDescription('Number of the last item in the slice.'))
                ->addProperty('total', (new IntegerType)->setDescription('Total number of items being paginated.'))
                ->addProperty('has_more', (new BooleanType)->setDescription('Whether there are more items after the current page.'))
                ->addProperty('has_less', (new BooleanType)->setDescription('Whether there are more items before the current page.'))
                ->addProperty('next_page', (new IntegerType)->nullable(true)->setDescription('Number of the next page.'))
                ->addProperty('previous_page', (new IntegerType)->nullable(true)->setDescription('Number of the previous page.'))
                ->setRequired(['current_page', 'from', 'last_page', 'path', 'per_page', 'to', 'total', 'has_more', 'has_less', 'next_page', 'previous_page'])
        );
        $type->setRequired(['data', 'meta']);

        return Response::make(200)
            ->description('Paginated set of `'.$this->components->uniqueSchemaName($collectingClassType->name).'`')
            ->setContent('application/json', Schema::fromType($type));
    }
}
<?php

declare(strict_types=1);

namespace App\Http\Controllers\Api;

use App\Http\Requests\PaginatedRequest;
use App\Http\Resources\PaginatedResourceCollection;
use App\Http\Resources\ProjectResource;
use Illuminate\Foundation\Auth\Access\AuthorizesRequests;

class ProjectController
{
    use AuthorizesRequests;

    /**
     * List projects accessible by the authenticated user.
     *
     * @return PaginatedResourceCollection<ProjectResource>
     */
    public function index(PaginatedRequest $request)
    {
        return new PaginatedResourceCollection(
            resource: $request->user()
                ->projects()
                ->orderBy(...$request->order())
                ->paginate($request->limit()),
            collects: ProjectResource::class,
        );
    }
}

hn-seoai avatar Apr 19 '24 08:04 hn-seoai

@hn-seoai thanks, got it. Can you please also share PaginatedResourceCollection source?

romalytvynenko avatar Apr 19 '24 11:04 romalytvynenko

<?php

namespace App\Http\Resources;

use Illuminate\Http\Resources\Json\AnonymousResourceCollection;
use Illuminate\Support\Arr;

class PaginatedResourceCollection extends AnonymousResourceCollection
{
    protected $preserveAllQueryParameters = true;

    public function paginationInformation($request, $paginated, $default)
    {
        $default['meta']['has_more']      = $default['meta']['current_page'] < $default['meta']['last_page'];
        $default['meta']['has_less']      = $default['meta']['current_page'] > 1;
        $default['meta']['next_page']     = $default['meta']['has_more']
            ? min($default['meta']['current_page'] + 1, $default['meta']['last_page'])
            : null;
        $default['meta']['previous_page'] = $default['meta']['has_less']
            ? max($default['meta']['current_page'] - 1, 1)
            : null;

        Arr::forget($default, ['links', 'meta.links']);

        return $default;
    }
}

hn-seoai avatar Apr 19 '24 11:04 hn-seoai

@hn-seoai thanks!

I'm wondering if a better solution here would be to allow you to modify previous extension's result. So Scramble would take care of creating paginated response description and you would be able to either rely on it and modify it, or return your own response type (think of reduce function).

<?php

// ...

class PaginatedResourceCollectionTypeToSchema extends TypeToSchemaExtension
{
    public function shouldHandle(Type $type)
    {
        return $type instanceof Generic
            && $type->name === PaginatedResourceCollection::class
            && count($type->templateTypes) === 1
            && $type->templateTypes[0] instanceof ObjectType
            && $type->templateTypes[0]->isInstanceOf(JsonResource::class);
    }

    /**
     * @param  Generic  $type
     */
    public function toResponse(Type $type, ?Response $previousResponse)
    {
        if (! $previousResponse) {
            return null;
        }

        $previousResponse
            ->content['application/json']
            ->removeProperties(['links', 'meta.links']);
    
        return $previousResponse;
    }
}

romalytvynenko avatar Apr 19 '24 12:04 romalytvynenko

That would indeed be a better solution!

hn-seoai avatar Apr 20 '24 07:04 hn-seoai

I tried working with the previous response, but this PR still seems to be necessary for it to work. Our custom extensions are run before everything else, so there is no previous response. Still using a fork.

hn-seoai avatar May 15 '24 11:05 hn-seoai

@hn-seoai sorry for the late reply. So I was thinking about this and for now I'd try to avoid changing extensions API (introducing prev response parameter).

But I'm still very open to help you achieve what you want. So let's forget about this "previous result" idea and stick to what we have here.

Scanning through the codebase I can see that custom extensions (provided by you) should be executed first. So whenever your extension returns true in shouldHandle and ACTUALLY handles the type (by returning non-null from handle) you should be good to go.

So what I'm trying to say is that I don't fully get how the suggested solution works and what problem does it solve. From what I can see, you can just add your extension and it will be used first, before the one from a lib.

I will close the PR for now, but please feel free to reply, I would be happy to help.

romalytvynenko avatar Jun 01 '24 18:06 romalytvynenko

@romalytvynenko @hn-seoai This is the same problem that I created a month before the same PR #347 for. I explained why this is a bug in more detail in the PR. Could you please look into this again?

If you want, I can create the PR again rebased to the current main branch state.

korridor avatar Jul 17 '24 14:07 korridor