laminas-api-tools / api-tools

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

Input Filter Behavior Option/Toggle #69

Open michalbundyra opened 4 years ago

michalbundyra commented 4 years ago

Take Apigility out of the equation for a moment... Default Input Filter Behavior:

    $inputFilter = new InputFilter();
    $inputFilter->add(array(
                'name' => 'foo',
                'required'=>true,
                'allow_empty'=>false,
                'filters'=>array(
                    array('name'=>'StripNewLines'),
                    array('name'=>'StripTags'),
                    array('name'=>'StringTrim'),
                ),
                'validators'=>array(
                    array('name'=>'StringLength',
                    'options'=>array(
                        'min'=>3,
                        ),
                    ),
                ),
            )
        );

       $inputFilter->setData(array('foo'=>' bar<a>', 'baz'=>'boo'));

       $inputFilter->isValid();  // returns true
       $inputFilter->getValues(); // returns array('foo'=>'bar')

Even though we are passing a value for a field that was not defined, the input filter is valid and returns only the data that it has instructions for. This is expected behavior.

Now, let's say you compose the same input filter via Apigility for an API endpoint and submit the same array of data. The API will respond with a status 400 (Bad Request) with the error message of: Unrecognized Field "baz"

I understand the API throwing this error; I get it. However, here is a use case for having the API endpoint not throw that error and simply ignore the undefined fields just like the default behavior of the input filter:

Two API endpoints that represent two database tables: /artists (artists table: id, artists_name) /albums (albums table: id, artist_id, album_name)

When retrieving an album through /album/:id, you may want to include additional fields in the response via the way of a JOIN:

{ 
  id:1,
  artist_id:12,
  album:"Best Hits", 
  artist_name:"Dull Needle & The Statics",
}

It is much more efficient to include the artist's name in the album request instead of making two API calls.

Ok, so what does this GET response have to do with input filters?

We have several apps built using Angular.js since it is great at two way data binding and we can easily push the modified data models up to the server (PATCH, PUT) when the user edits a view. With the current behavior, we would have to clone the data model and filter it in the angular controller, then push it up to the API. In the example above, if you were to leave the model untouched, you would get a 400 response. If a new joined field was added to the response payload you would also have to update the js controller to filter that out for PATCH and PUT operations.

Had the API acted exactly as the InputFilter example, nothing would have to be done on the view layer or in the angular controllers. We could confidently know that extraneous data would be ignored and removed via way of the input filter. We could pass complex relational data via GET and not have to replicate filtering logic in the JS or view side of things for POST, PUT, PATCH.

There should be an option on how to handle this use case. At the very least, it should be up to the developer to decide wether the extra data constitutes an error or not.

Sorry for the length; just tried to explain it clearly.


Originally posted by @spectravp at https://github.com/zfcampus/zf-apigility/issues/47

michalbundyra commented 4 years ago

Responding from my tablet, so just a quick note: the behavior you describe should only be true of patch requests at this time - due to the fact that patch requests create a validation group based on the data values provided. We considered allowing arbitrary values, but this gets tricky once you start having nested input filters, as you then need to descend into each tree and to perform has() checks. We felt it would be too resource intensive - and with patch, you should only be sending the data you wish to change anyways.


Originally posted by @weierophinney at https://github.com/zfcampus/zf-apigility/issues/47#issuecomment-45286901

michalbundyra commented 4 years ago

Interesting. I'm not sure I actually tested it using anything other than PATCH. Maybe this whole thread is rendered moot. I will double check and get back to you. Thank you again for all your hard work.


Originally posted by @spectravp at https://github.com/zfcampus/zf-apigility/issues/47#issuecomment-45287419

michalbundyra commented 4 years ago

Tested and you are correct. It is only PATCH that exhibits this behavior. You can back burner my thread, but I do think this is something the developer should be able to control. Maybe for version 2... maybe version 12. Thanks again, Matt!


Originally posted by @spectravp at https://github.com/zfcampus/zf-apigility/issues/47#issuecomment-45287770

michalbundyra commented 4 years ago

Marking this as an enhancement request. It may be something we need to address in ZF2, in Zend\InputFilter and how it handles creation of validation groups.


Originally posted by @weierophinney at https://github.com/zfcampus/zf-apigility/issues/47#issuecomment-45341581