Skip to content

JsonResponse hard to extend for purpose of overriding encoding defaults #14836

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.

Sign up for GitHub

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

Jump to bottom
Closed
Incognito opened this issue Jun 2, 2015 · 2 comments
Closed

JsonResponse hard to extend for purpose of overriding encoding defaults #14836

Incognito opened this issue Jun 2, 2015 · 2 comments
Labels
HttpFoundation

Comments

@Incognito
Copy link
Contributor

https://github.com/symfony/symfony/blob/2.8/src/Symfony/Component/HttpFoundation/JsonResponse.php#L38

There doesn't appear to be a way to override the default encoding through the constructor or inheritance.

Any objections to a PR where I move the flags into a protected property?
@hacfi
Copy link
Contributor

hacfi commented Jun 2, 2015

With #9915 you can extend the JsonResponse, overwrite __construct, call the parent and then $this-> setEncodingOptions(...);

@Incognito
Copy link
Contributor Author

I haven't run the code yet, but that appears to be destructive at first glance:

  1. set encoding A
  2. encodedA(data) == stored
  3. Modify encoding to be value B
  4. Decode without awareness of special encoding A rules.
  5. Redo encoding with encoding type B

This means in my override I call both setEncodingOptions and setData twice.

This was referenced Jun 9, 2015
Closed
Closed
fabpot pushed a commit that referenced this issue Jun 11, 2015
@Incognito @fabpot
Bug #14836 [HttpFoundation] Moves default JSON encoding assignment fr…
25e0d63 
…om constructor to property
fabpot added a commit that referenced this issue Jun 11, 2015
@fabpot
bug #14930 Bug #14836 [HttpFoundation] Moves default JSON encoding as…
88e99d2 
…signment fr… (Incognito)

This PR was submitted for the master branch but it was merged into the 2.6 branch instead (closes #14930).

Discussion
----------

Bug #14836 [HttpFoundation] Moves default JSON encoding assignment fr…

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

Moves the assignment of the encoding flags to a property so the class can be extended without needing to re-write the constructor and perform the encoding of data twice.

For more information please see #14836

Commits
-------

25e0d63 Bug #14836 [HttpFoundation] Moves default JSON encoding assignment from constructor to property
@fabpot fabpot closed this as completed Jun 11, 2015
nicolas-grekas added a commit that referenced this issue Jun 18, 2015
@nicolas-grekas
Merge branch '2.6' into 2.7
b1fabf5 
* 2.6:
  [Debug] Fix log level of stacked errors
  [VarDumper] Fix uninitialized id in HtmlDumper
  Fixed fluent interface
  [Debug] fix debug class loader case test on windows
  [Debug+VarDumper] Fix handling of PHP7 exception/error model
  [2.6][Security][Translation] #14920 update translations
  [VarDumper] Cherry-pick code style fixes from 2.7
  Bug #14836 [HttpFoundation] Moves default JSON encoding assignment from constructor to property

Conflicts:
	src/Symfony/Component/Debug/Tests/DebugClassLoaderTest.php
	src/Symfony/Component/VarDumper/Caster/DOMCaster.php
	src/Symfony/Component/VarDumper/Caster/ExceptionCaster.php
	src/Symfony/Component/VarDumper/Caster/PdoCaster.php
	src/Symfony/Component/VarDumper/Caster/SplCaster.php
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
HttpFoundation
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@fabpot @Incognito @hacfi @aitboudad