Zucchi/router tweaks #902

Closed
wants to merge 2 commits into
from

Projects

None yet

3 participants

@zucchi

Added new querystring route to allow appending of query strings

// Setup the router and routes
'Zend\Mvc\Router\RouteStack' => array(
    'parameters' => array(
        'routes' => array(
            'hyperion' => array(
                'type'    => 'Zend\Mvc\Router\Http\Segment',
                'options' => array(
                    'route'    => '/hyperion[/:controller[/:action]]',
                    'constraints' => array(
                        'controller' => '[a-zA-Z][a-zA-Z0-9_-]*',
                        'action'     => '[a-zA-Z][a-zA-Z0-9_-]*',
                    ),
                    'defaults' => array(
                        'controller' => 'hyperion',
                        'action'     => 'index',
                    ),
                ),
                'may_terminate' => true,
                'child_routes'  => array(
                    'query' => array(
                        'type' => 'Zend\Mvc\Router\Http\QueryString',
                    ),
                ),
            ),
        ),
    ),
),

which will now add params as a query string. to a url.

url('hyperion/query', array('controller'=>'domains', 'action' => 'detail', 'id' => $domain->id));?>
@DASPRiD DASPRiD commented on the diff Mar 7, 2012
library/Zend/Mvc/Router/Http/QueryString.php
+ /**
+ * List of assembled parameters.
+ *
+ * @var array
+ */
+ protected $assembledParams = array();
+
+ /**
+ * Create a new wildcard route.
+ *
+ * @param string $keyValueDelimiter
+ * @param string $paramDelimiter
+ * @param array $defaults
+ * @return void
+ */
+ public function __construct($keyValueDelimiter = '=', $paramDelimiter = '&', array $defaults = array(), $queryBoundary = '?')
@DASPRiD
DASPRiD Mar 7, 2012

I see no need to change delimiters and boundaries, as they are defined as is in the URI standard (plus: docblock is wrong).

@DASPRiD DASPRiD commented on the diff Mar 7, 2012
library/Zend/Mvc/Router/Http/QueryString.php
+ $options['defaults'] = array();
+ }
+
+ return new static($options['key_value_delimiter'], $options['param_delimiter'], $options['defaults']);
+ }
+
+ /**
+ * match(): defined by Route interface.
+ *
+ * @see Route::match()
+ * @param Request $request
+ * @return RouteMatch
+ */
+ public function match(Request $request, $pathOffset = null)
+ {
+ // return null as not matching query string
@DASPRiD
DASPRiD Mar 7, 2012

I'd actually add matching here, so that one can:

  • define which params are required to match
  • define constraints for the match
@zucchi
zucchi Mar 7, 2012

The reason for creating a "query" route was so I could pass arbitrary get value pairs without needing to define a route for every combination.

I would argue against this level of checking in the router by saying it query params should be tested and validated as required in the business logic

@DASPRiD DASPRiD commented on the diff Mar 7, 2012
library/Zend/Mvc/Router/Http/QueryString.php
+ */
+ public function assemble(array $params = array(), array $options = array())
+ {
+ $elements = array();
+ $mergedParams = array_merge($this->defaults, $params);
+ $this->assembledParams = array();
+
+ if ($mergedParams) {
+ foreach ($mergedParams as $key => $value) {
+ $elements[] = urlencode($key) . $this->keyValueDelimiter . urlencode($value);
+
+ $this->assembledParams[] = $key;
+ }
+
+ return $this->queryBoundary . implode($this->paramDelimiter, $elements);
+ }
@DASPRiD
DASPRiD Mar 7, 2012

This should work with the URI object, and not return a string (see e.g. Scheme or Hostname route).
Also, one may want to limit which parameters are actually used in the query string.

@DASPRiD DASPRiD commented on the diff Mar 7, 2012
library/Zend/Mvc/Router/Http/QueryString.php
+ * Delimiter between keys and values.
+ *
+ * @var string
+ */
+ protected $keyValueDelimiter;
+
+ /**
+ * Delimtier before parameters.
+ *
+ * @var array
+ */
+ protected $paramDelimiter;
+
+ /**
+ * Boundary character for query string
+ * @var string
@DASPRiD
DASPRiD Mar 7, 2012

Period after description missing. New line after description required.

@DASPRiD DASPRiD commented on the diff Mar 7, 2012
library/Zend/Mvc/Router/Http/QueryString.php
+
+use Traversable,
+ Zend\Stdlib\IteratorToArray,
+ Zend\Stdlib\RequestDescription as Request,
+ Zend\Mvc\Router\Exception;
+
+/**
+ * QueryString route.
+ *
+ * @package Zend_Mvc_Router
+ * @subpackage Http
+ * @copyright Copyright (c) 2005-2010 Zend Technologies USA Inc. (http://www.zend.com)
+ * @license http://framework.zend.com/license/new-bsd New BSD License
+ * @see http://manuals.rubyonrails.com/read/chapter/65
+ */
+class QueryString implements Route
@DASPRiD
DASPRiD Mar 7, 2012

Should actually be named Query, as this route works against a query (key value parameters), not the string (or is "Query" a reserved keyword?).

@DASPRiD
Zend Framework member

Also: Unit tests and documentation is missing.

@DASPRiD DASPRiD closed this Mar 7, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment