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

Input Filter Behavior Option/Toggle #69

Open
michalbundyra opened this issue Jan 10, 2020 · 4 comments
Open

Input Filter Behavior Option/Toggle #69

michalbundyra opened this issue Jan 10, 2020 · 4 comments
Labels
Enhancement New feature or request

Comments

@michalbundyra
Copy link
Member

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 zfcampus/zf-apigility#47

@michalbundyra michalbundyra added the Enhancement New feature or request label Jan 10, 2020
@michalbundyra
Copy link
Member Author

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 zfcampus/zf-apigility#47 (comment)

@michalbundyra
Copy link
Member Author

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 zfcampus/zf-apigility#47 (comment)

@michalbundyra
Copy link
Member Author

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 zfcampus/zf-apigility#47 (comment)

@michalbundyra
Copy link
Member Author

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 zfcampus/zf-apigility#47 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

1 participant