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

Query param fetcher #185

Merged
merged 49 commits into from May 16, 2012
Merged
Show file tree
Hide file tree
Changes from 48 commits
Commits
Show all changes
49 commits
Select commit Hold shift + click to select a range
631fe13
Proof-of-concept for @QueryParam validation
asm89 Jan 31, 2012
8c04a84
Add tests
asm89 Feb 1, 2012
192f3ae
requires -> requirements to be consistent with sf2
asm89 Feb 7, 2012
bfa891e
Merge branch 'master' into query_param-proof-of-concept
lsmith77 Feb 7, 2012
e32e078
added support for defaults via annotations
lsmith77 Feb 7, 2012
80d0da4
added support for controllers as a service, moved logic out of the co…
lsmith77 Feb 7, 2012
7cfa318
added support for passing the QueryFetcher as an action parameter
lsmith77 Feb 7, 2012
25517a6
fix tests
lsmith77 Feb 7, 2012
fd1ae02
cosmetic tweaks
lsmith77 Feb 7, 2012
b03414b
fixed regexp plus a few cosmetics
lsmith77 Feb 7, 2012
937e273
cover all possible controller definition cases
lsmith77 Feb 7, 2012
e7fe284
use a controller listener
lsmith77 Feb 8, 2012
df59842
tweaked error handling
lsmith77 Feb 8, 2012
ebbfb5c
Merge pull request #186 from FriendsOfSymfony/query_param-proof-of-co…
lsmith77 Feb 8, 2012
a52e85d
tweaked error handling
lsmith77 Feb 8, 2012
1cc4ccc
Merge branch 'master' into query_param-proof-of-concept
lsmith77 Feb 22, 2012
e430694
Merge branch 'use_rest_lib' into query_param-proof-of-concept
lsmith77 Mar 2, 2012
bf75439
Merge branch 'master' into query_param-proof-of-concept
lsmith77 Mar 2, 2012
8e19e8f
Merge branch 'master' into query_param-proof-of-concept
lsmith77 Mar 24, 2012
370e806
updated config reference
lsmith77 Mar 24, 2012
e41dd47
Merge remote-tracking branch 'origin/master' into query_param-proof-o…
lsmith77 Mar 29, 2012
8dfb9c4
Merge branch 'master' into query_param-proof-of-concept
lsmith77 Mar 29, 2012
f81bac8
Merge branch 'master' into query_param-proof-of-concept
lsmith77 Apr 2, 2012
f39ea93
Merge branch 'master' into query_param-proof-of-concept
lsmith77 Apr 3, 2012
b784c43
Merge remote-tracking branch 'origin/master' into query_param-proof-o…
lsmith77 Apr 10, 2012
49f68b2
Merge remote-tracking branch 'origin/master' into query_param-proof-o…
lsmith77 Apr 11, 2012
89ece6e
Merge branch 'master' into query_param-proof-of-concept
lsmith77 Apr 12, 2012
f07bd7a
Merge branch 'master' into query_param-proof-of-concept
lsmith77 Apr 13, 2012
ed4aaae
Merge branch 'master' into query_param-proof-of-concept
lsmith77 Apr 13, 2012
a6d434f
Merge branch 'master' into query_param-proof-of-concept
lsmith77 Apr 15, 2012
e65bb81
Merge branch 'master' into query_param-proof-of-concept
lsmith77 Apr 18, 2012
ddcc3fa
Merge branch 'master' into query_param-proof-of-concept
lsmith77 Apr 19, 2012
3dc09ba
Merge branch 'master' into query_param-proof-of-concept
lsmith77 Apr 19, 2012
9d4c66a
Merge branch 'master' into query_param-proof-of-concept
lsmith77 May 13, 2012
39da182
renamed QueryFetcher::getParameter() to QueryFetcher::get() and added…
lsmith77 May 15, 2012
9cbb25d
made it possible to set the query parameters as request attributes
lsmith77 May 15, 2012
0671c11
ignore query params when generating routes
lsmith77 May 15, 2012
199f42e
cosmetics
lsmith77 May 15, 2012
3406ebd
added documentation
lsmith77 May 15, 2012
87ccc2b
fix tests
lsmith77 May 15, 2012
3535e86
added strict mode
lsmith77 May 16, 2012
4137c3e
some more docs
lsmith77 May 16, 2012
07ae8fe
validate parameters
lsmith77 May 16, 2012
f6d019d
simplified code
lsmith77 May 16, 2012
af142df
added interfaces
lsmith77 May 16, 2012
37661de
validate configs
lsmith77 May 16, 2012
822af06
undo previous commit .. no need to make things harder
lsmith77 May 16, 2012
3e20eac
cosmetics
lsmith77 May 16, 2012
5c88435
ws tweak
lsmith77 May 16, 2012
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
30 changes: 30 additions & 0 deletions Controller/Annotations/QueryParam.php
@@ -0,0 +1,30 @@
<?php

/*
* This file is part of the FOSRestBundle package.
*
* (c) FriendsOfSymfony <http://friendsofsymfony.github.com/>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace FOS\RestBundle\Controller\Annotations;

/**
* QueryParam annotation class.
*
* @Annotation
* @author Alexander <iam.asm89@gmail.com>
*/
class QueryParam
{
/** @var string */
public $name;
/** @var string */
public $requirements;
/** @var string */
public $default;
/** @var string */
public $description;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you should probably add the annotations allowing the AnnotationReader to validate annotations as of Common 2.2 (see the ORM or @schmittjoh's bundles for instance)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed.

}
15 changes: 14 additions & 1 deletion DependencyInjection/Configuration.php
Expand Up @@ -27,6 +27,8 @@
*/
class Configuration implements ConfigurationInterface
{
private $forceOptionValues = array(false, true, 'force');

/**
* Generates the configuration tree.
*
Expand All @@ -39,6 +41,12 @@ public function getConfigTreeBuilder()

$rootNode
->children()
->scalarNode('query_fetcher_listener')->defaultFalse()
->validate()
->ifNotInArray($this->forceOptionValues)
->thenInvalid('The query_fetcher_listener option does not support %s. Please choose one of '.json_encode($this->forceOptionValues))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

missing one indentation level

->end()
->end()
->arrayNode('routing_loader')
->addDefaultsIfNotSet()
->children()
Expand Down Expand Up @@ -111,7 +119,12 @@ private function addViewSection(ArrayNodeDefinition $rootNode)
->defaultValue(array('html' => true))
->prototype('boolean')->end()
->end()
->scalarNode('view_response_listener')->defaultValue('force')->end()
->scalarNode('view_response_listener')->defaultValue('force')
->validate()
->ifNotInArray($this->forceOptionValues)
->thenInvalid('The view_response_listener option does not support %s. Please choose one of '.json_encode($this->forceOptionValues))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here

->end()
->end()
->scalarNode('failed_validation')->defaultValue(Codes::HTTP_BAD_REQUEST)->end()
->end()
->end()
Expand Down
9 changes: 9 additions & 0 deletions DependencyInjection/FOSRestExtension.php
Expand Up @@ -39,6 +39,7 @@ public function load(array $configs, ContainerBuilder $container)
$loader->load('view.xml');
$loader->load('routing.xml');
$loader->load('util.xml');
$loader->load('request.xml');

if (version_compare(FOSRestBundle::getSymfonyVersion(Kernel::VERSION), '2.1.0', '<')) {
$container->setParameter('fos_rest.routing.loader.controller.class', $container->getParameter('fos_rest.routing.loader_2_0.controller.class'));
Expand Down Expand Up @@ -128,6 +129,14 @@ public function load(array $configs, ContainerBuilder $container)
} else {
$container->setParameter($this->getAlias().'.mime_types', array());
}

if (!empty($config['query_fetcher_listener'])) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you could use if ($config['query_fetcher_listener']) { as you will always have a boolean (the node is a booleanNode and it has a default value)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this would mean i need to write a lot more code into the tests .. its fine like it is.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

well, my comment is not really accurate anymore as it is not a boolean anymore (because of force)

$loader->load('query_fetcher_listener.xml');

if ('force' === $config['query_fetcher_listener']) {
$container->setParameter($this->getAlias().'.query_fetch_listener.set_params_as_attributes', true);
}
}
}

/**
Expand Down
71 changes: 71 additions & 0 deletions EventListener/QueryFetcherListener.php
@@ -0,0 +1,71 @@
<?php

/*
* This file is part of the FOSRestBundle package.
*
* (c) FriendsOfSymfony <http://friendsofsymfony.github.com/>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace FOS\RestBundle\EventListener;

use Symfony\Component\HttpKernel\Event\FilterControllerEvent,
Symfony\Component\HttpKernel\HttpKernelInterface,
Symfony\Component\DependencyInjection\ContainerInterface;

/**
* This listener handles various setup tasks related to the query fetcher
*
* Setting the controller callable on the query fetcher
* Setting the query fetcher as a request attribute
*
* @author Lukas Kahwe Smith <smith@pooteeweet.org>
*/
class QueryFetcherListener
{
/**
* @var ContainerInterface
*/
private $container;

private $setParamsAsAttributes;

/**
* Constructor.
*
* @param ContainerInterface $container container
*/
public function __construct(ContainerInterface $container, $setParamsAsAttributes = false)
{
$this->container = $container;
$this->setParamsAsAttributes = $setParamsAsAttributes;
}

/**
* Core controller handler
*
* @param FilterControllerEvent $event The event
*/
public function onKernelController(FilterControllerEvent $event)
{
$request = $event->getRequest();
$queryFetcher = $this->container->get('fos_rest.request.query_fetcher');

$queryFetcher->setController($event->getController());
$request->attributes->set('queryFetcher', $queryFetcher);

if ($this->setParamsAsAttributes) {
$params = $queryFetcher->all();
foreach ($params as $name => $param) {
if ($request->attributes->has($name)) {
$msg = sprintf("QueryFetcher parameter conflicts with a path parameter '$name' for route '%s'", $request->attributes->get('_route'));
throw new \InvalidArgumentException($msg);
}

$request->attributes->set($name, $param);
}
}
}
}
135 changes: 135 additions & 0 deletions Request/QueryFetcher.php
@@ -0,0 +1,135 @@
<?php

/*
* This file is part of the FOSRestBundle package.
*
* (c) FriendsOfSymfony <http://friendsofsymfony.github.com/>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace FOS\RestBundle\Request;

use Symfony\Component\HttpFoundation\Request;

/**
* Helper to validate query parameters from the active request.
*
* @author Alexander <iam.asm89@gmail.com>
* @author Lukas Kahwe Smith <smith@pooteeweet.org>
*/
class QueryFetcher implements QueryFetcherInterface
{
/**
* @var QueryParamReader
*/
private $queryParamReader;

/**
* @var Request
*/
private $request;

/**
* @var array
*/
private $params;

/**
* @var callable
*/
private $controller;

/**
* Initializes fetcher.
*
* @param QueryParamReader $queryParamReader Query param reader
* @param Request $request Active request
*/
public function __construct(QueryParamReader $queryParamReader, Request $request)
{
$this->queryParamReader = $queryParamReader;
$this->request = $request;
}

/**
* @abstract
* @param callable $controller
*
* @return void
*/
public function setController($controller)
{
$this->controller = $controller;
}

/**
* Get a validated query parameter.
*
* @param string $name Name of the query parameter
* @param Boolean $strict If a requirement mismatch should cause an exception
*
* @return mixed Value of the parameter.
*/
public function get($name, $strict = false)
{
if (null === $this->params) {
$this->initParams();
}

if (!array_key_exists($name, $this->params)) {
throw new \InvalidArgumentException(sprintf("No @QueryParam configuration for parameter '%s'.", $name));
}

$config = $this->params[$name];
$default = $config->default;
$param = $this->request->query->get($name, $default);

// Set default if the requirements do not match
if ($param !== $default && !preg_match('#^'.$config->requirements.'$#xs', $param)) {
if ($strict) {
throw new \RuntimeException("Query parameter value '$param', does not match requirements '{$config->requirements}'");
}

$param = $default;
}

return $param;
}

/**
* Get all validated query parameter.
*
* @param Boolean $strict If a requirement mismatch should cause an exception
*
* @return array Values of all the parameters.
*/
public function all($strict = false)
{
$params = array();
foreach ($this->params as $name => $config) {
$params[$name] = $this->get($name, $strict);
}

return $params;
}

/**
* Initialize the parameters
*
* @throws \InvalidArgumentException
*/
private function initParams()
{
if (empty($this->controller)) {
throw new \InvalidArgumentException('Controller and method needs to be set via setController');
}

if (!is_array($this->controller) || empty($this->controller[0]) || !is_object($this->controller[0])) {
throw new \InvalidArgumentException('Controller needs to be set as a class instance (closures/functions are not supported)');
}

$this->params = $this->queryParamReader->read(new \ReflectionClass($this->controller[0]), $this->controller[1]);
}
}
50 changes: 50 additions & 0 deletions Request/QueryFetcherInterface.php
@@ -0,0 +1,50 @@
<?php

/*
* This file is part of the FOSRestBundle package.
*
* (c) FriendsOfSymfony <http://friendsofsymfony.github.com/>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace FOS\RestBundle\Request;

use Symfony\Component\HttpFoundation\Request;

/**
* Helper interface to validate query parameters from the active request.
*
* @author Alexander <iam.asm89@gmail.com>
* @author Lukas Kahwe Smith <smith@pooteeweet.org>
*/
interface QueryFetcherInterface
{
/**
* @abstract
* @param callable $controller
*
* @return void
*/
function setController($controller);

/**
* Get a validated query parameter.
*
* @param string $name Name of the query parameter
* @param Boolean $strict If a requirement mismatch should cause an exception
*
* @return mixed Value of the parameter.
*/
function get($name, $strict = false);

/**
* Get all validated query parameter.
*
* @param Boolean $strict If a requirement mismatch should cause an exception
*
* @return array Values of all the parameters.
*/
function all($strict = false);
}