Validate grant types

[tfrommen]

Hi again,

@rmccue didn't comment on my What's Left list in #8, but since this was no big deal, I just created this pull request with a proposed validation for grant type( handler)s.

The result of the grant type filter, oauth2.grant_types, will be validated, and only WP\OAuth2\Types\Type implementations make it to the consumer.

Comments?

Cheers,
Thorsten

[rmccue]

Whoops, let me go back and answer :)

[rmccue]

Could we instead just specify function ( Type $type ) and have PHP validate for us? 🤔

[rmccue]

No need for the blank line here.

[tfrommen]

Removed (aka fixed).

[tfrommen]

Could we instead just specify function ( Type $type ) and have PHP validate for us? 🤔

No, not really. PHP would not validate (and discard invalid) types, like the code that I suggested, but rather expect correct implementations, and fatal error if encountered anything else.
If this is the desired behavior, then sure, we could add a type hint. But the array_filter() would then get pretty awkward to look at, in my opinion. We do not want to do anything to valid types, so the code could look something like this:

return array_filter( $types, function ( Type $type )  {
    return true;
} );

So, in fact, we wouldn't really need the callback('s body), but just PHP that kicks in for checking (and fatal erroring because of) the type.

To be honest, I would rather keep my code (or something along the lines). But like I said, the bahavior would be discard invalid types. If you want PHP to jump the user into their face, we can do that. 😀

Does this make sense? 🤔

[rmccue]

I'd rather not silently drop objects; it makes debugging a pain for developers. If not the fatal error, we should at least fire a _doing_it_wrong when we drop it.

[tfrommen]

Makes sense. Should I adapt the closure accordingly?

[rmccue]

That'd be great, thanks :)

[tfrommen]

I just updated the validation code, again.

If only we could use PHP 5.6+... :)
I decided to use a regular foreach loop now, because then I'm able to include the type in the message. If this is not required, I can refactor this to use array_filter(). Another option would be to use array_walk() in combination with another variable, or array_filter() - which both might not be faster than the foreach.

Comments?

[rmccue]

Looks good to me 👍