search

laminas-api-tools / api-tools-admin

Laminas API Tools Admin module
https://api-tools.getlaminas.org/documentation
BSD 3-Clause "New" or "Revised" License
13 stars 21 forks source link

Cannot create RPC-Service because of wrong InputFilter #81

Closed Lokilein closed 10 months ago

Lokilein commented 2 years ago

Bug Report

With v1.10.3, it is not possible to create a new service, as the wrong filter is taken for the POST action;

image

Q A
Version(s) 1.10.3

Summary

Current behavior

You cannot add a RPC-Service with the "New Service" Button. You receive an 422 error instead:

detail: "Failed Validation"
status: 422
title: "Unprocessable Entity"
type: "http://www.w3.org/Protocols/rfc2616/rfc2616-sec10.html"
validation_messages: {controller_class: ["The Controller Class must be a valid, fully qualified, PHP class name"],…}
accept_whitelist: ["The Accept Whitelist must be an array of valid media type expressions"]
content_type_whitelist: ["The Content-Type Whitelist must be an array of valid media type expressions"]
controller_class: ["The Controller Class must be a valid, fully qualified, PHP class name"]
http_methods: ["The HTTP Methods must be an array of valid HTTP method names"]

How to reproduce

Expected behavior

FXLima commented 2 years ago

Create a new service RPC in: Api-Tools Laminas

I have the same problem:

XHRPOSThttp://localhost/api-tools/api/module/myapp/rpc [HTTP/1.1 422 Unprocessable Entity 10175ms]

validation_messages Object { controller_class: […], accept_whitelist: […], content_type_whitelist: […], … } controller_class [ "The Controller Class must be a valid, fully qualified, PHP class name" ] 0 "The Controller Class must be a valid, fully qualified, PHP class name" accept_whitelist [ "The Accept Whitelist must be an array of valid media type expressions" ] 0 "The Accept Whitelist must be an array of valid media type expressions" content_type_whitelist [ "The Content-Type Whitelist must be an array of valid media type expressions" ] 0 "The Content-Type Whitelist must be an array of valid media type expressions" http_methods [ "The HTTP Methods must be an array of valid HTTP method names" ] 0 "The HTTP Methods must be an array of valid HTTP method names" type "http://www.w3.org/Protocols/rfc2616/rfc2616-sec10.html" title "Unprocessable Entity" status 422 detail "Failed Validation"

FXLima commented 2 years ago

I edited the file: module.config.php

vendor\laminas-api-tools\api-tools-admin\config\module.config.php

Following @Lokilein guidelines, we should edit the lines

'ZF\Apigility\Admin\InputFilter\RpcService\PATCH' => RpcPatchInputFilter::class, 'ZF\Apigility\Admin\InputFilter\RpcService\POST' => RpcPatchInputFilter::class, RpcPostInputFilter::class, 'ZF\Apigility\Admin\InputFilter\Version' => VersionInputFilter::class,

// Legacy Zend Framework aliases v2 'ZF\Apigility\Admin\InputFilter\RpcService\PatchInputFilter' => RpcPatchInputFilter::class, 'ZF\Apigility\Admin\InputFilter\RpcService\PostInputFilter' => RpcPatchInputFilter::class, RpcPostInputFilter::class,

~~### I edited the file: PatchInputFilter.php vendor\laminas-api-tools\api-tools-admin\src\InputFilter\RpcService\PatchInputFilter.php~~

~~And commented out the lines. It's not ideal, but it created the service. We need to investigate here.~~

class PatchInputFilter extends PostInputFilter
{
    public function init()
    {
        parent::init();

        // $this->add([
        //     'name'          => 'controller_class',
        //     'required'      => true,
        //     'error_message' => 'The Controller Class must be a valid, fully qualified, PHP class name',
        // ]);

        // $this->add([
        //     'name'          => 'accept_whitelist',
        //     'validators'    => [
        //         ['name' => MediaTypeArrayValidator::class],
        //     ],
        //     'error_message' => 'The Accept Whitelist must be an array of valid media type expressions',
        // ]);
        // $this->add([
        //     'name'          => 'content_type_whitelist',
        //     'validators'    => [
        //         ['name' => MediaTypeArrayValidator::class],
        //     ],
        //     'error_message' => 'The Content-Type Whitelist must be an array of valid media type expressions',
        // ]);
        // $this->add([
        //     'name'          => 'selector',
        //     'required'      => false,
        //     'allow_empty'   => true,
        //     'error_message' => 'The Content Negotiation Selector must be a valid,'
        //         . ' defined api-tools-content-negotiation selector name',
        // ]);

        // $this->add([
        //     'name'          => 'http_methods',
        //     'validators'    => [
        //         ['name' => HttpMethodArrayValidator::class],
        //     ],
        //     'error_message' => 'The HTTP Methods must be an array of valid HTTP method names',
        // ]);
    }
}
Lokilein commented 2 years ago

Instead of deactivating the filter, I would really recommand to adapt the wrong filter config. As you can see from my post, the PostFilter is used when asked for the PatchFilter. This is most likely a typo by CopyPaste. Replace RPCPostInputFilter with RPCPatchInputFilter at the two places and everything work as it should.

FXLima commented 2 years ago

Perfect! I didn't read your post carefully.

Zimhoo commented 2 years ago

I edited the file: module.config.php vendor\laminas-api-tools\api-tools-admin\config\module.config.php

Here is the file already corrected module.config.zip

robertvanlienden commented 1 year ago

Hi there,

I'm new to laminas and trying to learn the basics trough the documentation with the skeleton. Stumbled upon this issue when creating my first RPC-Service in Laminas API tools.

After editing vendor\laminas-api-tools\api-tools-admin\config\module.config.php as said by @Zimhoo I can create my first RPC-Service trough the web client and continue the getting-started guide.

I have too little knowledg about Laminas yet, but why would we not just submit the edited module.config.php submitted above as an PR on this package? As this edit will fix this issue and seems non-breaking.

Hoping that someone with some more knowledge about Laminas can answer this 😄 .

For me as Laminas-API-tools first time learner this issue was relative annoying... When diving into an new framework and going trough an "getting started" guide, you want to see things work as expected and not searching trough vendor/ code to fix framework-related bugs.

Update: Seems like there is an PR ready; https://github.com/laminas-api-tools/api-tools-admin/pull/88 Hoping this will get merged/released.

Lokilein commented 1 year ago

@robertvanlienden We are using the API tools since a year now and I cannot recommand it. We are planning to remove them again as they do more harm than good. Just to let you know some of our expierences if you want to consider it.

However, we find out that the REST is not the best option for us for most cases, so we will go to RPC instead, but in my opinion we might just use regular Controller for this and keep the API tools out of this.

ppaulis commented 1 year ago

@Lokilein I think it would be better to write down a general laminas review in the appropriate channels, and not in a particular issue that has nothing to do with points you are raising. Nevertheless, as the discussion is started, I'd like to reply to some of those points:

Generally speaking, the Laminas API Tools need some getting-used-to, but on the up-side you get a very powerful, customizable & stable framework, guiding you in a variety of best-practices. Sure, the admin package could need some work. At lot of features are not even editable in the Admin UI. But I've setup recently multiple new installations with our team in no time and the Admin package (& UI) worked as it should. What can make reading the documentation difficult, is, that Laminas API Tools are composed of a lot of smaller packages, and the documentation equally is split up in those packages.

We were not able to configure the returns like we wished

I think it's a misconception to link the input (to create a resource) 1-to-1 with the output. And this is not defined by the HAL component but rather by your Hydrators. The HAL component is just a renderer. If you want a fine-grained control on your resource's output, simply use a the ArraySerializableHydrator. Removing then the password from your resource becomes the easiest thing in the world. If, however, you use a ClassMethodsHydrator in combination with a getPassword() in your entity, then, sure, you end up having the password in your output.

We had a extremely hard time to stop it sending Exceptions to the frontend directly
...
One time, we didn't typed the getter/setter in the model correctly to nullable, but all you get is a generic error that the answer cannot be generated

I see multiple problems here: 1) Invalid requests should NEVER touch your database. If that happens, you input has probably not been filtered and validated correctly (which is perfectly doable with laminas validators (or even your ownn custom validators). 2) Verify you encapsulation : Create your own specific exception classes for the different components of your application. Each component should catch the exceptions of the invoked sub-components and handle them appropriately. You shouldn't expect the framework to handle low-level exceptions from the database out-of-the-box. As for not letting through exceptions to the API customer, you have always a catch(\Throwable $e) as a last resort if you didn't catch the exception before. 3) Have you written automated tests for your application? Are you doing TDD or even better BDD? What about Static code analysis? Using tools like Behat, PHPUnit, etc. lets you develop a lot faster with much more reliable code. Especially Static Analysis with PHPStan is a real no-brainer and does a lot of the debugging work for you.

Best regards, Pascal

Lokilein commented 1 year ago

@ppaulis I didn't want to critize the Laminas Framework at whole, but my experience with the API tools, when I was new to it a year ago, wasn't that good and I just wanted to feedback that to Robert, as he said he is new to it. Also, the thing, that this bug is still not fixed, as it is also essential to new developers to get in touch with these tools is a message, that can't be unseen.

Maybe there are good solution, your experience is a completly other, which might also be good to know. I also have to say that our API Tools were setup by a colleague in the "ivory tower", which are gone now and which made it hard for uns to understand, also, some concepts were implemented though they did not suit our solution. We tried to get it right, but then there was the documentation which was missing for us to find a workaround for our problems. I've read the documentation of the main API Tools several times up and down and haven't found a solution there - maybe, yes, I should have look for further documentation in sub-elements.

However, I agree that my experience might be a "how not to do it" expierence and based on missing documentation and/or knowledge, but, again, we haven't had so much problems with any other part of Laminas yet, and I am usually not that bad to work through stuff.

To your recommandations:

  1. True, these were experiences between development. We missed a typing, happens, but were not able to find out, what was happening, because based on how the connection is defined by us, the Laminas\Paginator\Paginator catches Throwables, but the getIterator get called somewhere in the framework. The way we configured it, we were not able to react at this point. But that is a personal detail and just a experience that I've wanted to share.
  2. We have these Exceptions, but see 1.
  3. We do and we don't. We have a huge legacy system and write test for new code, but there are still many areas that are not testable at all. We are working on it. However, TDD and static code analyser will not replace a proper documentation.

I can/will delete the comment from above, as it might not belong here, agree. Just wanted to explain my post to bring it in the right context.

tasmaniski commented 1 year ago

Hello, this issue is still present. PHP 8.2.10.

As others suggested, in ./vendor/laminas-api-tools/api-tools-admin/config/module.config.php I can see wrong aliases for POST. 

'ZF\Apigility\Admin\InputFilter\RpcService\POST' => RpcPatchInputFilter::class,  
... 
'ZF\Apigility\Admin\InputFilter\RpcService\PostInputFilter'     => RpcPatchInputFilter::class,

RpcPatchInputFilter is used instead of RestPostInputFilter.

When I make change to RestPostInputFilter::class it works. 

'ZF\Apigility\Admin\InputFilter\RpcService\POST' => RestPostInputFilter::class,  
... 
'ZF\Apigility\Admin\InputFilter\RpcService\PostInputFilter'     => RestPostInputFilter::class,
tasmaniski commented 1 year ago

Cleaner solution is to add new file in config/autoload/local.php and add correct aliases there: 

<?php

use Laminas\ApiTools\Admin\InputFilter\RestService\PostInputFilter as RestPostInputFilter;

return [
    'input_filters' => [
        'aliases' => [
            'ZF\Apigility\Admin\InputFilter\RpcService\POST'            => RestPostInputFilter::class,
            'ZF\Apigility\Admin\InputFilter\RpcService\PostInputFilter' => RestPostInputFilter::class,
        ],
    ],
];
christoferd commented 11 months ago

@tasmaniski Thanks, that worked with latest version 22-Nov-2023