rupadana / filament-api-service

A simple api service for supporting filamentphp
https://filamentphp.com/plugins/rupadana-api-service
MIT License
121 stars 26 forks source link

[Feature]: Allow multiple transformers and select transformer via X-HEADERS #56

Open eelco2k opened 3 months ago

eelco2k commented 3 months ago

What happened?

possibility to define multiple Transformers (in Resource) as array. And use a specific transformer via HTTP headers defined in the config:

small example code:

api-service.php

'api_transformer_header' => env('API_TRANSFORMER_HEADER', 'X-API-TRANSFORMER'),

$headerName = config('api-service.route.api_transformer_header');
$headerName = strtolower($headerName);
if (!request()->headers->has($headerName)) {
    return DefaultTransformer::class;
}

$transformer = request()->headers->get($headerName);

etc.. etc...

use $transformer as key of the resource apiTransformer Array defined list and return the corresponding transformer class,

How to reproduce the bug

new addition

Package Version

latest

PHP Version

8.3

Laravel Version

11

Which operating systems does with happen with?

No response

Notes

No response

rupadana commented 3 months ago

i don't think it is really needed, cause it will issued the security of the API.

eelco2k commented 3 months ago

@rupadana i do not understand what you mean with issues the security of the API.

This PR will add functionality to use / select a different transformer via an HTTP header. so the response can be different json layout

eelco2k commented 3 months ago

@rupadana i think another, but better approach to selecting a different response from an api request would be the: Accept Header / Media Type modification to something like this: application/vnd.<api name>.<service name>.v<version>+json in stead of the default:

application/json

so if a resource (BlogResource for example.) has 2 transformers (LargeTransformer::class, MinimumTransformer::class) you could request one or another transformer with the HTTP Accept header like so

application/vnd.blog.minimum-transformer+json or application/vnd.blog.large-transformer+json

what do you think?

rupadana commented 3 months ago

@rupadana i do not understand what you mean with issues the security of the API.

This PR will add functionality to use / select a different transformer via an HTTP header. so the response can be different json layout

yeah we need validation into it, to prevert internal server error if the user parsing a wrong transformer

eelco2k commented 3 months ago

Aah okay I understand but for now it will fallback to DefaultTransformer if specified transformer cannot be found. But will see if I can add validation and validation error responses

rupadana commented 3 months ago

@rupadana i think another, but better approach to selecting a different response from an api request would be the: Accept Header / Media Type modification to something like this: application/vnd.<api name>.<service name>.v<version>+json in stead of the default:

application/json

so if a resource (BlogResource for example.) has 2 transformers (LargeTransformer::class, MinimumTransformer::class) you could request one or another transformer with the HTTP Accept header like so

application/vnd.blog.minimum-transformer+json or application/vnd.blog.large-transformer+json

what do you think?

That goods, i think we need a method that stores the transformer, the api can have more than one transformer.

Maybe like this

public function transformers(): array {
     return [
            "md5hash" => Md5HashTransformer::class,
            "base64hash" => Base64HashTransormer::class,
            "other-things" => OtherThingsTransformer::class
      ];
}

and maybe we can bring it to v4 for it

eelco2k commented 3 months ago

If you check my changes in PR you will see that method (not transformers() but getApiTransformers() , I believe it is. ) already is there.

rupadana commented 3 months ago

If you check my changes in PR you will see that method (not transformers() but getApiTransformers() , I believe it is. ) already is there.

okay, let me review it