[Doc] Change classname from DoctrineConverter to DoctrineParamConverter
merged branch franmomu/patch-1 (PR #133)

Commits
-------

f333e41 [Doc] Change classname from DoctrineConverter to DoctrineParamConverter

Discussion
----------

[Doc] Fixed class name from DoctrineConverter to DoctrineParamConverter
Backwards compatible refactoring that increases the use-cases for DoctrineParamConverter. Support for composite primary keys and for cases where multiple entities use have the same field names
merged branch beberlei/DoctrineParamConverterRefactoring (PR #141)

Commits
-------

ae620b8 Backwards compatible refactoring that increases the use-cases for DoctrineParamConverter. Support for composite primary keys and for cases where multiple entities use have the same field names

Discussion
----------

[ParamConverter] Doctrine Converter Refactoring

Backwards compatible refactoring that increases the use-cases for DoctrineParamConverter.

* Support for composite primary keys
* Allow explicit mapping and excludes in cases where multiple converters clash with the same field names.
* Fix bug where an exception is thrown even when the parameter is optional.
* Increase the code-coverage of the converter, covering find by primary key and by fields values.

This brings DoctrineConverter in line with Propel converter (https://github.com/propelorm/PropelBundle/blob/1.1/Request/ParamConverter/PropelParamConverter.php) which also has 'mapping' and 'exclude' keys.
fixed CS
[ParamConverter] Add support for named param converters. They preceed the iteration and make conversion much faster. Add existing converters as "doctrine.orm" and "datetime".
Add documentation on named converters
Fix a bug and a comment position
Refactor AddParamConverterPass loop over tags.
[ParamConverter][Doctrine] allow to find by single-valued associations.
Allow to disable priority registration with priority="false" attribute. Documentation of registration via DIC
Add UPGRADE.md notice of possible BC break
Fix bug introduced with CompilerPass refactoring
0 == "false" is true, so we rather check for ===
Fix error when using Symfony Routing component annotations

A controller using "Symfony\Component\Routing\Annotation\Route" instead of "Sensio\Bundle\FrameworkExtraBundle\Configuration\Route", with a prefix annotation on the class:

/**
 * @Route("/prefix")
 */
class SomeController { }

would cause an error:

Fatal error: Call to undefined method Symfony\Component\Routing\Annotation\Route::getService() in /vendor/sensio/framework-extra-bundle/Sensio/Bundle/FrameworkExtraBundle/Routing/AnnotatedRouteControllerLoader.php on line 40
merged branch beberlei/DoctrineFindByAssocations (PR #143)

Commits
-------

dd7e22b Add UPGRADE.md notice of possible BC break
56d98f3 [ParamConverter][Doctrine] allow to find by single-valued associations.

Discussion
----------

[ParamConverter][Doctrine] allow to find by single-valued associations.

Fixes GH-95

I initially thought this could lead to clashes when the following
conversions are defined:

    /**
     * @Route("/page/{user}/{group}")
     * @ParamConverter("user", {"id": "user"})
     * @ParamConverter("group", {"id": "group"})
     */
    public function showAction(User $user, Group $group) {}

    class User
    {
        /** @ManyToOne(targetEntity="Group") */
        private $group;
    }

However through the renaming via `id` its a find by primary key anyways. There is a slim chance before GH-141 to find two objects through `findOneBy()` which would break because of this patch, when an association was not found before and now is included in the `$criteria`.

Should I add a UPGRADE notice?

---------------------------------------------------------------------------

by fabpot at 2012-07-26T08:27:09Z

Can you indeed add an UPGRADE note?

---------------------------------------------------------------------------

by beberlei at 2012-07-26T10:52:52Z

Added the UPGRADE note.
merged branch beberlei/NamedParamConverters (PR #142)

Commits
-------

6efdeb0 0 == "false" is true, so we rather check for ===
a719ab4 Fix bug introduced with CompilerPass refactoring
85d7c3f Allow to disable priority registration with priority="false" attribute. Documentation of registration via DIC
349ad35 Refactor AddParamConverterPass loop over tags.
de202e4 Fix a bug and a comment position
55753d1 Add documentation on named converters
2ed098d [ParamConverter] Add support for named param converters. They preceed the iteration and make conversion much faster. Add existing converters as "doctrine.orm" and "datetime".

Discussion
----------

[ParamConverter] Add support for named param converters.

Named parameter converters will allow specific choice which param converter is chosen.

* avoid the iterations over many converters
* pick specific converters that otherwise need to be invoked at very late priorities for security reasons.
* It also makes the conversion more explicit, which could take away developer fears not understanding whats happening.
* Cases with different inputs (Query, Request, Raw-Data) might make explicit converter choice necessary.

Fixes GH-116

One open question with this patch is that now every converter automatically has a priority and not a name. You cannot give a converter no priority and only make it available via name. I am thinking to change this to allow registration with priority, name and with both.

* A BC solution would need to introduce <tag name="request.param_converter" priority="false" /> which needs special handling in the CompilerPAss.
* A non BC solution would change every converter without a priority attribute in the DIC tag without priority.

What do you prefer?

---------------------------------------------------------------------------

by beberlei at 2012-07-25T21:38:41Z

docs are missing.

---------------------------------------------------------------------------

by beberlei at 2012-07-25T21:44:33Z

docs are here now

---------------------------------------------------------------------------

by beberlei at 2012-07-25T21:54:20Z

This PR would close GH-116

---------------------------------------------------------------------------

by fabpot at 2012-07-26T06:20:45Z

About the question you have about priorities and names. A converter without a priority would have to be explicitly used via the annotation and the name provided, right? No automatic conversion would ever happen with such a converter?

Anyway, if it does not make the code more complex, I would prefer to have a BC solution, especially because this would be managed in the compiler pass.

---------------------------------------------------------------------------

by beberlei at 2012-07-26T06:47:28Z

Ok i updated it the BC way. Its not a complex solution, just one simple if condition. I added a documentation for DIC registration by tag as well now.

I haven't tested this code yet, i will check in some minutes at my work project and then give a +1/-1 on this PR.

---------------------------------------------------------------------------

by beberlei at 2012-07-26T09:52:50Z

Ok i "functionally" tested this in my application and it works after those two last fixes. You can use named converters and or priority based and the compiler pass works. Actual functionality is tested by the unitt-tests in ParamConverterManagerTest.php and works as advertised.
Make DateTime converter work on attributes only.
merged branch beberlei/FixDateTimeAttribute (PR #146)

Commits
-------

03760d7 Make DateTime converter work on attributes only.

Discussion
----------

[ParamConverter] Make DateTime converter work on attributes only.

Currently the DateTime converter works on all attributes. This can potentially pose problems, so this PR restricts the usage to attributes only. Additionally a 404 exception is thrown if a datetime cannot be converted succesfully.

Improved tests.
Skip Request when resolving. This improves performance since injecting the Request is already a feature of Symfony Core