bumped Symfony version to 2.0.23-DEV
[Yaml] fixed #6770
[Yaml] fixed #6773
merged branch ezzatron/yaml-negative-integers (PR #6784)

This PR was merged into the 2.0 branch.

Commits
-------

fea20b7 [Yaml] fixed #6770

Discussion
----------

[Yaml] fixed parsing of negative integers (2.0 branch)

| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #6770
| License       | MIT
| Doc PR        | n/a

Note that an unrelated test fixture for large integers had to be changed to work on systems with 64-bit integer support because of the change from `assertEquals()` to `assertSame()`. Please see the diff for clarification.
[Yaml] fixed ignored text when parsing an inlined mapping or sequence (closes #6786)
KNOWN_ISSUES with php 5.3.16
Merge branch '2.0' into 2.1

* 2.0:
  [DependencyInjection] fixed the creation of synthetic services in ContainerBuilder
  [Security] PHPDoc in SecurityEvents
  [FrameworkBundle] fixed Client::doRequest that must call its parent method (closes #6737)
  [Yaml] fixed ignored text when parsing an inlined mapping or sequence (closes #6786)
  [Yaml] fixed #6773
  [Yaml] fixed #6770
  bumped Symfony version to 2.0.23-DEV

Conflicts:
	src/Symfony/Component/DependencyInjection/ContainerBuilder.php
	src/Symfony/Component/HttpKernel/Kernel.php
	src/Symfony/Component/Yaml/Inline.php
	src/Symfony/Component/Yaml/Tests/InlineTest.php
merged branch fabpot/content-renderer-request (PR #6829)

This PR was merged into the master branch.

Commits
-------

23f5145 renamed proxy to router_proxy
e5135f6 [HttpKernel] renamed path to _path to avoid collision
3193a90 made the proxy path configurable
ad82893 removed the need for a proxy route for rendering strategies
b9f0e17 [HttpKernel] made the Request required when using rendering strategies

Discussion
----------

Content renderer simplification

| Q             | A
| ------------- | ---
| Bug fix?      | no
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | n/a
| License       | MIT
| Doc PR        | symfony/symfony-docs#2179

Todo:

  - [x] submit a PR for documentation update

The first commit makes the Request required when dealing with rendering strategies (see the commit why this was a bad idea to make it optional in the first place).

The second commit removes the need for a proxy route and replaces it with the same system we have in the security component.

The third commit makes the proxy path configurable (default to `/_proxy`).

This PR has been triggered by a discussion on #6791.

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

by fabpot at 2013-01-22T09:49:37Z

My opinion:

  * The first commit should be merged.

  * For the second and third one, I don't have a strong opinion. One of the benefits that the content renderer and its strategies do not rely on the Routing component anymore.

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

by fabpot at 2013-01-22T15:22:47Z

Any comments? ping @Tobion @vicb

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

by Tobion at 2013-01-22T16:07:15Z

Shouldn't the class name be like `SubRequestRenderingStrategyInterface` because currently it does not say anything about what is rendered. `RenderingStrategyInterface` makes it look like it's for rendering a normal master request, i.e. templating.

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

by fabpot at 2013-01-22T16:19:26Z

@Tobion: This was actually the first name I had but I found it too long. It is indeed rendering a normal request, but only in the context of a master request.

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

by Tobion at 2013-01-22T16:23:25Z

I found the correct term for what this is about: http://en.wikipedia.org/wiki/Transclusion
So it should probably be like `Symfony/Component/HttpKernel/Transclusion/TransclusionInterface`
or `TransclusionStrategyInterface`.
So the term `rendering` is misleading as it does not really render the subrequest, but only prints a reference to the subrequest. The rendering is done by ESI processor or hinclude etc.

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

by fabpot at 2013-01-22T16:37:00Z

The `RenderingStrategyInterface` does render a request and returns a Response. One of the strategy consist of returning an ESI tag, but this is still a Response.

I don't like introducing the `Transclusion` word as (at least for me) it does not evoke anything.

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

by Tobion at 2013-01-22T16:46:10Z

Also `DefaultRenderingStrategy` is not saying anything. What is default? It should express that it directly includes the resource in the other (term `integrate` comes to my mind).

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

by kriswallsmith at 2013-01-22T17:23:21Z

How about `InlineRenderingStrategy`?

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

by vicb at 2013-01-22T17:25:17Z

@Tobion @kriswallsmith :+1:

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

by kriswallsmith at 2013-01-22T17:26:17Z

Also, `SubRequestStrategyInterface` may be more apparent (`InlineSubRequestStrategy`, `EsiSubRequestStrategy`…)

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

by Tobion at 2013-01-22T18:10:19Z

`SubRequestStrategyInterface` is missing the verb somehow. A strategy for what? @kriswallsmith as an English native speaker, is transclusion also not convenient for you?

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

by fabpot at 2013-01-22T18:11:41Z

Thanks for all your suggestions, I appreciate them, but what about the whole approach? Do you agree that it is better than the current one? I'd like to avoid the bikeshedding if the approach is not better.

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

by kriswallsmith at 2013-01-22T18:22:47Z

:+1: for removing the router dependency.

@Tobion perhaps request is the verb there?

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

by Tobion at 2013-01-22T19:51:20Z

I'm also fine with making it independent from the routing component because routing is about making routes configurable with placeholders etc. for nice URLs. But this is not needed for a proxy feature.

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

by kriswallsmith at 2013-01-22T20:13:48Z

@fabpot Do you anticipate ever wanting a sub request to be handled differently based on HTTP method? Just thinking of possible reasons to continue using the routing component here…

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

by fabpot at 2013-01-22T20:40:06Z

No, sub-requests only make sense for GET requests. In fact, we even enforce that in HttpContentRenderer.

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

by fabpot at 2013-01-23T06:51:54Z

I'm not going to discuss names further in the context of this PR as it has already been discussed in the initial PR that introduced these classes and because this PR does not change anything to the meaning of these classes. If you think the names can be better, feel free to open a ticket and discuss names there.

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

by vicb at 2013-01-23T07:48:36Z

If I understand correctly, both hsi and esi will generate path starting with "/_proxy", isn't it a problem wrt access_control ? should it be possible to configure a per strategy path ?

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

by fabpot at 2013-01-23T07:56:11Z

@vicb: Yes, all strategies use the `/_proxy` path when the developer uses a controller reference. But the router proxy takes care of securing the route, so there is no need to do it yourself.

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

by vicb at 2013-01-23T08:07:36Z

@fabpot that's smart, I've missed it.

Some questions though (they should be answered by UT I think - and might already have been, I have not checked)
- Isn't there a pb with urlencoding in the listener ?
- Would the listener work with fragments (`#...') ?

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

by vicb at 2013-01-23T08:31:37Z

Some more points:

- Should we validate that the router_proxy is enabled when esi are enabled (early failure ?)
- Should we be able to enable each strategy individually (ie no need to expose the signer when hsi are not used)

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

by fabpot at 2013-01-23T09:58:45Z

Enabling the router proxy when using ESI si not required. The router proxy is "only" required when you use the `controller` Twig function (or the equivalent in PHP -> `ControllerReference`). But we can probably throw an exception in the `ControllerReference` constructor if the proxy is not enabled.

Enabling each strategy individually is indeed a good idea (and that's more a general question as we could do the same for the translator loaders, or the service container loaders). Let's create another issue on this global topic.

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

by vicb at 2013-01-23T10:10:29Z

> But we can probably throw an exception in the ControllerReference constructor if the proxy is not enabled.

It should probably be in a wrapper class then ?

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

by fabpot at 2013-01-23T12:45:36Z

The listener does not need to work with fragments as URLs as they are never part of the generated URL.
Merge branch '2.1'

* 2.1:
  [DependencyInjection] fixed the creation of synthetic services in ContainerBuilder
  [Security] PHPDoc in SecurityEvents
  Fix typos in README
  Added an error message in the DebugClassLoader when using / instead of \.
  KNOWN_ISSUES with php 5.3.16
  [FrameworkBundle] fixed Client::doRequest that must call its parent method (closes #6737)
  [Yaml] fixed ignored text when parsing an inlined mapping or sequence (closes #6786)
  [Yaml] fixed #6773
  [Yaml] fixed #6770
  bumped Symfony version to 2.1.8-DEV
  bumped Symfony version to 2.0.23-DEV

Conflicts:
	src/Symfony/Bundle/FrameworkBundle/Client.php
	src/Symfony/Component/HttpKernel/Kernel.php
[Yaml] fixed wrong merge (indentation default is 4 as of 2.1)
Merge branch '2.1'

* 2.1:
  [Yaml] fixed wrong merge (indentation default is 4 as of 2.1)
  Fixed missing class argument when throwing exception
updated the branch alias in composer files
remove dead code in yaml component
Merge branch '2.1' into 2.2

* 2.1:
  [FrameworkBundle] Fixes invalid serialized objects in cache
  remove dead code in yaml component
  fixed typo
  RedisProfilerStorage wrong db-number/index-number selected
  [DependencyInjection] added a test for the previous merge (refs #7261)
  Unset loading[$id] in ContainerBuilder on exception
  [Console] fixed StringInput binding
  [Console] added string input test
  Revert "merged branch jfsimon/issue-6749 (PR #7220)"
  fixed CS

Conflicts:
	src/Symfony/Component/Console/Tests/Input/StringInputTest.php
	src/Symfony/Component/HttpKernel/Profiler/RedisProfilerStorage.php
Merge branch '2.2'

* 2.2: (26 commits)
  [FrameworkBundle] Fixes invalid serialized objects in cache
  remove dead code in yaml component
  Fixed typo in UPGRADE-2.2
  fixed typo
  RedisProfilerStorage wrong db-number/index-number selected
  [DependencyInjection] added a test for the previous merge (refs #7261)
  Unset loading[$id] in ContainerBuilder on exception
  Default validation message translation fix.
  remove() should not use deprecated getParent() so it does not trigger deprecation internally
  adjust routing tests to not use prefix in addCollection
  add test for uniqueness of resources
  added tests for addDefaults, addRequirements, addOptions
  adjust RouteCollectionTest for the addCollection change and refactor the tests to only skip the part that really needs the config component
  added tests for remove() that wasnt covered yet and special route name
  refactor interator test that was still assuming a tree
  adjust tests to no use addPrefix with options
  adjusted tests to not use RouteCollection::getPrefix
  [Routing] trigger deprecation warning for deprecated features that will be removed in 2.3
  [Console] fixed StringInput binding
  [Console] added string input test
  ...