Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Add the mapping option for the PropelParamConverter, #156

Merged
merged 1 commit into from

4 participants

Jérémie Augustin Don't Add Me To Your Organization a.k.a The Travis Bot William Durand Tomasz Wójcik
Jérémie Augustin

Hi,

This Add the mapping option for the PropelParamConverter:

You can map route parameters directly to model column to be use for filtering.

If you have a route like /my-route/{postUniqueName}/{AuthorId}
Mapping option overwrite any other automatic mapping.

<?php

/**
 * @ParamConverter("post", class="BlogBundle\Model\Post", options={"mapping"={"postUniqueName":"name"}})
 * @ParamConverter("author", class="BlogBundle\Model\Author", options={"mapping"={"AuthorId":"id"}})
 */
public function myAction(Post $post, $author)
{
}

this enhance the pk handling to use the real PK name and not only 'id'

this also fix new sensioFrameworkBundle 2.1 requirement:
ParamConverterInterface::apply() method now must return a Boolean value indicating if a conversion was done

this cover #154

Jérémie Augustin jaugustin Add the mapping option for the PropelParamConverter,
enhance the pk handling to use the real PK name and not only 'id',
fix new sensioFrameworkBundle 2.1 requirement:
ParamConverterInterface::apply() method now must return a Boolean value indicating if a conversion was done
b03f6a5
Don't Add Me To Your Organization a.k.a The Travis Bot

This pull request passes (merged b03f6a5 into f675e39).

William Durand willdurand commented on the diff
Request/ParamConverter/PropelParamConverter.php
((35 lines not shown))
public function apply(Request $request, ConfigurationInterface $configuration)
{
$classQuery = $configuration->getClass() . 'Query';
+ $classPeer = $configuration->getClass() . 'Peer';
William Durand Owner

is it really necessary?

If we want it to work with PK not named "id" yes otherwise no

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
William Durand
Owner

Ok, seems ok then. I didn't dig more into this "ESI" issue, and I hope it's not a hack ;)

Jérémie Augustin

it's not a hack, but there is no real ESI issue.

With this you can handle more case like multiple parameters loaded at the same time, with there pks (that was not possible before)

William Durand willdurand merged commit 64a6b02 into from
William Durand
Owner

Alright, thanks!

Tomasz Wójcik

Using filters without resetting the array causes invalid behavior when converting parameters in subrequests in symfony. When master request uses this converter for f.ex. a $city in SomeController::someAction(City $city) with mapping "city": "slug", and agin in a subrequest with a different mapping the filters array has mapping from the master request, which causes to generate invalid queies to find a matching row from the database.

FIX (please review):

$this->filters = array();

at the beginning of the apply function

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on May 22, 2012
  1. Jérémie Augustin

    Add the mapping option for the PropelParamConverter,

    jaugustin authored
    enhance the pk handling to use the real PK name and not only 'id',
    fix new sensioFrameworkBundle 2.1 requirement:
    ParamConverterInterface::apply() method now must return a Boolean value indicating if a conversion was done
This page is out of date. Refresh to see the latest.
70 Request/ParamConverter/PropelParamConverter.php
View
@@ -10,27 +10,75 @@
/**
- * PropelConverter.
+ * PropelParamConverter
+ *
+ * This convert action parameter to a Propel Object
+ * there is two option for this converter:
+ *
+ * mapping : take an array of routeParam => column
+ * exclude : take an array of routeParam to exclude from the conversion process
+ *
*
* @author Jérémie Augustin <jeremie.augustin@pixel-cookers.com>
*/
class PropelParamConverter implements ParamConverterInterface
{
+ /**
+ * the pk column (e.g. id)
+ * @var string
+ */
+ protected $pk;
+
+ /**
+ * list of column/value to use with filterBy
+ * @var array
+ */
+ protected $filters = array();
+
+ /**
+ * list of route parameters to exclude from the conversion process
+ * @var array
+ */
+ protected $exclude = array();
public function apply(Request $request, ConfigurationInterface $configuration)
{
$classQuery = $configuration->getClass() . 'Query';
+ $classPeer = $configuration->getClass() . 'Peer';
William Durand Owner

is it really necessary?

If we want it to work with PK not named "id" yes otherwise no

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
if (!class_exists($classQuery)) {
throw new \Exception(sprintf('The %s Query class does not exist', $classQuery));
}
+
+ $tableMap = $classPeer::getTableMap();
+ $pkColumns = $tableMap->getPrimaryKeyColumns();
+
+ if (count($pkColumns) == 1) {
+ $this->pk = strtolower($pkColumns[0]->getName());
+ }
+
$options = $configuration->getOptions();
- $exclude = isset($options['exclude'])? $options['exclude'] : array();
+
+ if (isset($options['mapping'])) {
+ // We use the mapping for calling findPk or filterBy
+ foreach ($options['mapping'] as $routeParam => $column) {
+ if ($request->attributes->has($routeParam)) {
+ if ($this->pk === $column) {
+ $this->pk = $routeParam;
+ } else {
+ $this->filters[$column] = $request->attributes->get($routeParam);
+ }
+ }
+ }
+ } else {
+ $this->exclude = isset($options['exclude'])? $options['exclude'] : array();
+ $this->filters = $request->attributes->all();
+ }
// find by Pk
- if (in_array('id', $exclude) || false === $object = $this->findPk($classQuery, $request)) {
+ if (false === $object = $this->findPk($classQuery, $request)) {
// find by criteria
- if (false === $object = $this->findOneBy($classQuery, $request, $exclude)) {
+ if (false === $object = $this->findOneBy($classQuery, $request)) {
if ($configuration->isOptional()) {
//we find nothing but the object is optional
$object = null;
@@ -45,25 +93,27 @@ public function apply(Request $request, ConfigurationInterface $configuration)
}
$request->attributes->set($configuration->getName(), $object);
+
+ return true;
}
protected function findPk($classQuery, Request $request)
{
- if (!$request->attributes->has('id')) {
+ if (in_array($this->pk, $this->exclude) || !$request->attributes->has($this->pk)) {
return false;
}
- return $classQuery::create()->findPk($request->attributes->get('id'));
+ return $classQuery::create()->findPk($request->attributes->get($this->pk));
}
- protected function findOneBy($classQuery, Request $request, $exclude)
+ protected function findOneBy($classQuery, Request $request)
{
$query = $classQuery::create();
$hasCriteria = false;
- foreach ($request->attributes->all() as $key => $value) {
- if (!in_array($key, $exclude)) {
+ foreach ($this->filters as $column => $value) {
+ if (!in_array($column, $this->exclude)) {
try {
- $query->{'filterBy' . PropelInflector::camelize($key)}($value);
+ $query->{'filterBy' . PropelInflector::camelize($column)}($value);
$hasCriteria = true;
} catch (\PropelException $e) { }
}
18 Resources/doc/param_converter.markdown
View
@@ -45,5 +45,23 @@ public function myAction(Post $post)
}
```
+#### Custom mapping ####
+
+You can map route parameters directly to model column to be use for filtering.
+
+If you have a route like `/my-route/{postUniqueName}/{AuthorId}`
+Mapping option overwrite any other automatic mapping.
+
+``` php
+<?php
+
+/**
+ * @ParamConverter("post", class="BlogBundle\Model\Post", options={"mapping"={"postUniqueName":"name"}})
+ * @ParamConverter("author", class="BlogBundle\Model\Author", options={"mapping"={"AuthorId":"id"}})
+ */
+public function myAction(Post $post, $author)
+{
+}
+```
[Back to index](index.markdown)
26 Tests/Request/ParamConverter/PropelParamConverterTest.php
View
@@ -152,4 +152,30 @@ public function testParamConverterFindWithOptionalParam()
$this->assertNull($request->attributes->get('book'),
'param "book" should be null if book is not found and the parameter is optional');
}
+
+ public function testParamConverterFindWithMapping()
+ {
+ $paramConverter = new PropelParamConverter();
+ $request = new Request(array(), array(), array('toto' => 1, 'book' => null));
+ $configuration = new ParamConverter(array('class' => 'Propel\PropelBundle\Tests\Fixtures\Model\Book',
+ 'name' => 'book',
+ 'options' => array('mapping' => array('toto' => 'id'))
+ ));
+ $paramConverter->apply($request, $configuration);
+ $this->assertInstanceOf('Propel\PropelBundle\Tests\Fixtures\Model\Book',$request->attributes->get('book'),
+ 'param "book" should be an instance of "Propel\PropelBundle\Tests\Fixtures\Model\Book"');
+ }
+
+ public function testParamConverterFindSlugWithMapping()
+ {
+ $paramConverter = new PropelParamConverter();
+ $request = new Request(array(), array(), array('slugParam_special' => 'my-book', 'book' => null));
+ $configuration = new ParamConverter(array('class' => 'Propel\PropelBundle\Tests\Fixtures\Model\Book',
+ 'name' => 'book',
+ 'options' => array('mapping' => array('slugParam_special' => 'slug'))
+ ));
+ $paramConverter->apply($request, $configuration);
+ $this->assertInstanceOf('Propel\PropelBundle\Tests\Fixtures\Model\Book',$request->attributes->get('book'),
+ 'param "book" should be an instance of "Propel\PropelBundle\Tests\Fixtures\Model\Book"');
+ }
}
Something went wrong with that request. Please try again.