8000 Dual PSR-7 + HttpFoundation support on a component by component basis · Issue #15414 · symfony/symfony · GitHub
[go: up one dir, main page]

Skip to content

Dual PSR-7 + HttpFoundation support on a component by component basis #15414

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

Closed
6 of 20 tasks
nicolas-grekas opened this issue Jul 30, 2015 · 10 comments
Closed
6 of 20 tasks

Comments

@nicolas-grekas
Copy link
Member

Naïve list gathered with grep. Let's make using each of these usable, standalone, in a PSR-7 centric app, or just check if there is something to do, or decide to not do it.

May handle PSR-7:

May not handle PSR-7:

  • Component/HttpFoundation
  • Component/HttpKernel
  • Bundle/DebugBundle
  • Bundle/FrameworkBundle
  • Bundle/SecurityBundle
  • Bundle/TwigBundle
  • Bundle/WebProfilerBundle
@stof
Copy link
Member
stof commented Jul 30, 2015

PSR-7 support for Monolog and Switfmailer should maybe live outside the bridge, in libraries themselves, depending of what relies on HttpFoundation in them.

The Doctrine bridge only relies on HttpFoundation for its Session part, which is out of the scope of PSR-7. The other place using HttpFoundation is the DataCollector, but changing this is a matter of refactoring the profiler, not of refactoring the bridge. So I'm marking the Doctrine bridge as covered.

@stof
Copy link
Member
stof commented Jul 30, 2015

btw, many places are relying on the RequestStack to access the Request. But we don't have a RequestStack equivalent based on PSR-7

@wouterj
Copy link
Member
wouterj commented Jul 30, 2015

Routing has its own issue for PSR-7: #14723

@wouterj
Copy link
Member
wouterj commented Jul 30, 2015

Security Core 2.8 will have only one usage of Request in the ExpressionVoter (for access_control expressions). I think it's a better solution to register a new global expression language function: request_to_psr7() or the like.

Security Csrf only relies on HttpFoundation for the session part, so it can be marked as done as well.

Security Http has big dependencies on HttpFoundation. I don't think we should want PSR-7 support over there, or we should rewrite almost all classes in there.

@stof
Copy link
Member
stof commented Jul 31, 2015

The swiftmailer bridge is only about the profiler integration, so nothing to do here

@stof
Copy link
Member
stof commented Jul 31, 2015

and symfony/security-core also depends on HttpFoundation on simple authenticators, which should actually have been provided in Http rather than Core (because this is the distinction between Core and Http)

@stof
Copy link
Member
stof commented Jul 31, 2015

The translation component usage is only about the profiler integration, so nothing to do here (until the profiler is rewritten)

@wouterj
Copy link
Member
wouterj commented Jul 31, 2015

@stof the simple authenticators are moved to Security\Http in 2.8

@wouterj
Copy link
Member
wouterj commented Jul 31, 2015

I'm not so sure about the TwigBridge. A Bridge means that it's providing some classes that integrate Symfony in 3rd party libs (like Twig). So it's logical that they implement HttpFoundation and I'm not sure if it's logical to implement PSR-7 here. Seems like a TwigPsr7Extension in the twigphp organization can do a better job. For the record, HttpFoundation usage in the TwigBridge:

  • HttpFoundationExtension - to implement the absolute_url and relative_path functions
  • AppVariable - the global app var in Symfony full-stack templates
  • The data collector

fabpot added a commit that referenced this issue Aug 1, 2015
…colas-grekas)

This PR was merged into the 2.8 branch.

Discussion
----------

[Debug] Deprecate ExceptionHandler::createResponse

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

This does deprecate a behavior that might be specialized by someone somewhere, but I seriously doubt anyone did. It complicated the implementation for no gain IMHO.

Commits
-------

a4d2d31 [Debug] Deprecate ExceptionHandler::createResponse
@nicolas-grekas
Copy link
Member Author

Closing because this is not something with a well defined target (we don't know which/how components need psr-7 support). If someone want psr-7 support for a component, please open a PR :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants
0