8000 JsonResponse setContent, setEncodingOptions · Issue #16541 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

JsonResponse setContent, setEncodingOptions #16541

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
kanariezwart opened this issue Nov 13, 2015 · 6 comments
Closed

JsonResponse setContent, setEncodingOptions #16541

kanariezwart opened this issue Nov 13, 2015 · 6 comments
Labels
Enhancement HttpFoundation

Comments

@kanariezwart
Copy link

Consider when creating a JsonResponse and calling setContent() instead of setData() (bypassing this->data and meant for adding serialized content straight onto $this->content and skipping the added json_encode feature.

$jsonData = '{...}'; // something something
$response = new JsonResponse();
$response->setContent($jsonData)
$response->setEncodingOptions(JSON_PRETTY_PRINT); // some encoding option

return $response;

This will return an empty string as the JsonResponse content. Of course when setting content manually, there is no need for setting options anymore. However it's not always clear when data or content is passed to a JsonResponse. Inside the setEncodingOptions():

    // Symfony\Component\HttpFoundation/JsonReponse.php
    public function setEncodingOptions($encodingOptions)
    {
        $this->encodingOptions = (int) $encodingOptions;

        return $this->setData(json_decode($this->data));
    }

This also triggers the re-adding of this->data as content. I think this leads to enexpected behaviour. This proposal might not be the prettiest, but it works:

    // Symfony\Component\HttpFoundation/JsonReponse.php
    public function setEncodingOptions($encodingOptions)
    {
        $this->encodingOptions = (int)$encodingOptions;
        if ($this->content !== null) {
           $this->data = $this->content;
        }
        return $this->setData(json_decode($this->data));
    }

Another option to get rid of this issue is when calling setContent(), try to set and isFinal flag or something.

// Symfony\Component\HttpFoundation/JsonReponse.php
public function setContent($content)
{
  $this->data =  json_decode($content);
 // call Symfony\Component\HttpFoundation/Reponse.php
  parent::setContent($content);
}
@kanariezwart
Copy link
Author

Any thoughts?

@ro0NL
Copy link
Contributor
ro0NL commented Nov 16, 2015

I currently use the generic Response object for ready-to-use JSON strings, as I dont like the overhead of json_decode/encode in the JsonResponse object, ie; setData(json_decode($contents)), for something I already have.

Also combining setContent and setCallback leads to unexpected behaviour.. as the callback should be part of the content. Currently nothing happens :(

My2cents :)

@kanariezwart
Copy link
Author

@ro0NL I don't like it either, but re-decoding is the quickest solution with existing code.
I think the JsonResponse object should just be a container for data. And a ResponseBuilder object should responsible for adding/modifying data and returning it when called by a viewHandler

@kanariezwart
Copy link
Author

Please look at #16551

@kanariezwart kanariezwart mentioned this issue Nov 16, 2015
Closed
@ro0NL
Copy link
Contributor
ro0NL commented Nov 16, 2015

How about a JsonResponse::createFromString which parses callback from contents... getData decodes the content string lazy.

JsonResponse::create could also be renamed to createFromData so its signature doesnt changes..

@kanariezwart kanariezwart mentioned this issue Nov 16, 2015
Closed
@kanariezwart kanariezwart mentioned this issue Nov 26, 2015
Closed
@javiereguiluz
Copy link
Member

Closing it as fixed by #19552 where we added JsonResponse::fromJsonString()

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Enhancement HttpFoundation
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@javiereguiluz @kanariezwart @ro0NL @xabbuh
0