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

New middleware feature: use API method parameter type hints to move parsing/validation outside of API methods #8852

Closed
diosmosis opened this issue Sep 25, 2015 · 1 comment
Labels
c: Platform For Matomo platform changes that aren't impacting any of our APIs but improve the core itself. wontfix If you can reproduce this issue, please reopen the issue or create a new one describing it.

Comments

@diosmosis
Copy link
Member

Currently API methods accept string values and are themselves responsible for parsing those values into a data structure and validating those values. Many API methods use common inputs though, for example, a list of site IDs or a list of dates. At the moment this results in code redundancy and often times code that isn't fully tested.

We can solve this for 3.0 by introducing a feature that is available in many MVC frameworks (including, eg Spring: http://stackoverflow.com/questions/16942193/spring-mvc-complex-object-as-get-requestparam). In core/API/Proxy.php, we can detect the type hints of API methods, construct those types w/ the query parameter values and pass them to the API methods, thus moving the responsibility of parsing & validating to Piwik's middleware.

For example, an API method that might've looked like this before:

public function doSomething($sites, $dates)
{
    $sites = explode(',', $sites);
    $sites = array_map('intval', $sites);
    Piwik::checkUserHasAdminAccess($sites);

    $dates = explode(',', $dates);
    $dates = array_map('\Piwik\Date::factory', $dates);

    // ...
}

would instead look like:

public function doSomething(SitesList $sites, DatesList $dates)
{
    Piwik::checkUserHasAdminAccess($sites->getIdSites());

    // ...
}

Should increase code reuse, increase code testability, and well, gosh darnit, just look better.

A related possibility, is allowing results to be complex types as well, but this isn't quite as important to Piwik since most of the time the result is a DataTable.

@diosmosis diosmosis added the c: Platform For Matomo platform changes that aren't impacting any of our APIs but improve the core itself. label Sep 25, 2015
@diosmosis diosmosis added this to the 3.0.0 milestone Sep 25, 2015
@diosmosis diosmosis changed the title New middleware feature: use API method parameter type hints to construct move parsing/validation outside of API methods New middleware feature: use API method parameter type hints to move parsing/validation outside of API methods Sep 25, 2015
@diosmosis
Copy link
Member Author

Docblock type hints could also be used, eg, to validate $idSite vars, ie @param int $idSite.

@mattab mattab modified the milestones: Mid term, 3.0.0 Feb 8, 2016
@mattab mattab closed this as completed Jul 8, 2016
@mattab mattab added the wontfix If you can reproduce this issue, please reopen the issue or create a new one describing it. label Jul 8, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c: Platform For Matomo platform changes that aren't impacting any of our APIs but improve the core itself. wontfix If you can reproduce this issue, please reopen the issue or create a new one describing it.
Projects
None yet
Development

No branches or pull requests

2 participants