8000 [Serializer] Add options to JsonDecode and JsonEncode to wrap/unwrap json data by Simperfit · Pull Request #30894 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[Serializer] Add options to JsonDecode and JsonEncode to wrap/unwrap json data #30894

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
wants to merge 1 commit into from
Closed

[Serializer] Add options to JsonDecode and JsonEncode to wrap/unwrap json data #30894

wants to merge 1 commit into from

Conversation

Simperfit
Copy link
Contributor
@Simperfit Simperfit commented Apr 6, 2019 •
edited
Loading

from @nonanerz

Add options to JsonDecode and JsonEncode to wrap/unwrap json data
Q A
Branch? master
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets n/a
License MIT
Doc PR n/a

Add options to JsonDecode and JsonEncode to wrap/unwrap json data.

So you can use:

$serialiser->deserialize("{object: {value: 1, other: 2}}", Object::class, ['json_root_key' => 'object'])

I've just rebase the PR with master without the merge cc @fabpot
from #28887
#FOSSHackathons

@fabpot fabpot mentioned this pull request Apr 6, 2019
Closed
@fabpot
Copy link
Member
fabpot commented Apr 6, 2019

@ogizanagi was not sure if that's suitable for core.

@nicolas-grekas nicolas-grekas added this to the next milestone Apr 6, 2019
nicolas-grekas
nicolas-grekas reviewed Apr 6, 2019
View reviewed changes
src/Symfony/Component/Serializer/Encoder/JsonDecode.php
}

/**
* Decodes data.
*
* @param string $data The encoded JSON string to decode
* @param string $format Must be set to JsonEncoder::FORMAT
* @param array $context An optional set of options for the JSON decoder; see below
* @param array $context an optional set of options for the JSON decoder; see below
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should be reverted

src/Symfony/Component/Serializer/Encoder/JsonDecode.php
@@ -80,6 +84,9 @@ public function __construct($defaultContext = [], int $depth = 512)
* json_decode_options: integer
* Specifies additional options as per documentation for json_decode
*
* JSON_PROPERTY_PATH: string
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lower case: that's the value of the constant, not its name

src/Symfony/Component/Serializer/Encoder/JsonEncode.php
* @param string $propertyPath
* @param mixed $data
*
* @return array
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the docblock should be removed in favor of real types

src/Symfony/Component/Serializer/Tests/Encoder/JsonDecodeTest.php
array('{"baz": {"foo": "bar"}}', null, array(JsonEncoder::JSON_PROPERTY_PATH => 'baz.inner')),
array('{"baz": {"foo": "bar"}}', $assoc, array(JsonEncoder::JSON_PROPERTY_PATH => '[baz]', 'json_decode_associative' => true)),
array('{"baz": {"foo": "bar"}}', $assoc, array(JsonEncoder::JSON_PROPERTY_PATH => '[baz]', 'json_decode_associative' => true)),
array('{"baz": {"foo": "bar", "inner": {"key": "value"}}}', array('key' => 'value'), array(JsonEncoder::JSON_PROPERTY_PATH => '[baz][inner]', 'json_decode_associative' => true)),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see fabbot

joelwurtz
joelwurtz reviewed Apr 7, 2019
View reviewed changes
Copy link
Contributor
@joelwurtz joelwurtz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm also not sure if it's something that should be in the core, use case seems to be very special, cannot this be done with a specific normalizer instead ?

Like having a ROOT_OBJECT context option that can be interpreted by a specific normalizer and which will call the underlying denormalizer (DenormalizerAwareInterface)

src/Symfony/Component/Serializer/Encoder/JsonDecode.php
@@ -57,14 +60,15 @@ public function __construct($defaultContext = [], int $depth = 512)
}
8000
$this->defaultContext = array_merge($this->defaultContext, $defaultContext);
$this->propertyAccessor = PropertyAccess::createPropertyAccessor();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It adds a hard dependency on property access which is still a suggested dependency in the composer

@Simperfit
Copy link
Contributor Author

I don't think we want to commit that to the core, like @joelwurtz said it should be in a specific normalizer.

@Simperfit Simperfit closed this Apr 7, 2019
@Simperfit Simperfit deleted the feature/nonanerz/add_option_to_json_decoder branch April 18, 2019 05:31
@nicolas-grekas nicolas-grekas modified the milestones: next, 4.3 Apr 30, 2019
@nonanerz nonanerz mentioned this pull request May 6, 2019
Merged
fabpot added a commit that referenced this pull request Apr 12, 2020
@fabpot
feature #31390 [Serializer] UnwrappingDenormalizer (nonanerz)
7c98954 
This PR was merged into the 5.1-dev branch.

Discussion
----------

[Serializer] UnwrappingDenormalizer

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

UnwrappingDenormalizer, registered with very high priority. Unwrapping the data if UNWRAP_PATH is provided.

Very often some APIs give nested responses in which we need only the child object. With UnwrappingDenormalizer we can get the needed object without creating unnecessary Model class that we don't really need.

Regarding to #28887 and #30894

Usage:

`$serialiser->deserialize('{"baz": {"foo": "bar", "inner": {"title": "value", "numbers": [5,3]}}}', Object::class, ['UnwrappingDenormalizer::UNWRAP_PATH' => '[baz][inner]'])`

Commits
-------

00d103d UnwrappingDenormalizer
symfony-splitter pushed a commit to symfony/serializer that referenced this pull request Apr 12, 2020
@fabpot
feature #31390 [Serializer] UnwrappingDenormalizer (nonanerz)
0d866f1 
This PR was merged into the 5.1-dev branch.

Discussion
----------

[Serializer] UnwrappingDenormalizer

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

UnwrappingDenormalizer, registered with very high priority. Unwrapping the data if UNWRAP_PATH is provided.

Very often some APIs give nested responses in which we need only the child object. With UnwrappingDenormalizer we can get the needed object without creating unnecessary Model class that we don't really need.

Regarding to symfony/symfony#28887 and symfony/symfony#30894

Usage:

`$serialiser->deserialize('{"baz": {"foo": "bar", "inner": {"title": "value", "numbers": [5,3]}}}', Object::class, ['UnwrappingDenormalizer::UNWRAP_PATH' => '[baz][inner]'])`

Commits
-------

00d103d5f7 UnwrappingDenormalizer
symfony-splitter pushed a commit to symfony/framework-bundle that referenced this pull request Apr 12, 2020
@fabpot
feature #31390 [Serializer] UnwrappingDenormalizer (nonanerz)
2e07d4f 
This PR was merged into the 5.1-dev branch.

Discussion
----------

[Serializer] UnwrappingDenormalizer

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

UnwrappingDenormalizer, registered with very high priority. Unwrapping the data if UNWRAP_PATH is provided.

Very often some APIs give nested responses in which we need only the child object. With UnwrappingDenormalizer we can get the needed object without creating unnecessary Model class that we don't really need.

Regarding to symfony/symfony#28887 and symfony/symfony#30894

Usage:

`$serialiser->deserialize('{"baz": {"foo": "bar", "inner": {"title": "value", "numbers": [5,3]}}}', Object::class, ['UnwrappingDenormalizer::UNWRAP_PATH' => '[baz][inner]'])`

Commits
-------

00d103d5f7 UnwrappingDenormalizer
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@joelwurtz joelwurtz joelwurtz left review comments

@nicolas-grekas nicolas-grekas nicolas-grekas left review comments

@maxhelias maxhelias maxhelias approved these changes

Assignees
No one assigned
Labels
Serializer Status: Reviewed
Projects
None yet
Milestone
4.3
Development

Successfully merging this pull request may close these issues.
7 participants
@Simperfit @fabpot @joelwurtz @nicolas-grekas @maxhelias @carsonbot @nonanerz
0