8000 [Serializer] XmlEncoder doesn't ignore PI nodes while encoding by maidmaid · Pull Request #27926 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content
/ symfony Public

[Serializer] XmlEncoder doesn't ignore PI nodes while encoding #27926

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
Merged
merged 1 commit into from
Aug 25, 2018
Merged

[Serializer] XmlEncoder doesn't ignore PI nodes while encoding #27926

merged 1 commit into from
Aug 25, 2018

Conversation

@maidmaid
Copy link
Contributor
@maidmaid maidmaid commented Jul 11, 2018 •
edited
Loading
Q A
Branch? master
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Tests pass? no
Fixed tickets /
License MIT
Doc PR /

It's sometimes important to get only the XML body without the processing instructions (like the <?xml version="1.0" ?> on the top of your XML doc). At the moment, it's possible to ignore them while decoding, but not while encoding.

$encoder = new XmlEncoder('response', null, $ignoredNodeTypes = [XML_PI_NODE]);
echo $encoder->encode([], 'xml');
Expected:
<response/>

Actual:
<?xml version="1.0"?>
<response/>

So, a new $encoderIgnoredNodeTypes arg is added to the constructor which will be:

public function __construct(string $rootNodeName = 'response', int $loadOptions = null, array $decoderIgnoredNodeTypes = array(XML_PI_NODE, XML_COMMENT_NODE), array $encoderIgnoredNodeTypes = array())

@dunglas
Copy link
Member
dunglas commented Jul 11, 2018

We follow the robustness principle "Be conservative in what you send, be liberal in what you accept", so while it's ok to ignore XML_PI_NODE by default when decoding, it must be generated by default when encoding.

I suggest to add a new constructor parameter:

public function __construct(string $rootNodeName = 'response', int $loadOptions = null, array $decoderIgnoredNodeTypes = array(XML_PI_NODE, XML_COMMENT_NODE), array $encoderIgnoredNodeTypes = array())

@nicolas-grekas nicolas-grekas added this to the next milestone Jul 23, 2018
@maidmaid maidmaid force-pushed the serializer-pi branch 2 times, most recently from d3d136d to 9c182a2 Compare August 6, 2018 22:53
@maidmaid
Copy link
Contributor Author
maidmaid commented Aug 6, 2018

I followed your suggestion @dunglas, could you review ?

@nicolas-grekas
Copy link
Member
nicolas-grekas commented Aug 24, 2018

Is the "BC break" label still valid? If not can you please update the PR description? Rebase needed btw.

@maidmaid
Copy link
Contributor Author
maidmaid commented Aug 24, 2018

@nicolas-grekas Done :)

@dunglas
Copy link
Member
dunglas commented Aug 25, 2018

Thank you @maidmaid.

@dunglas dunglas merged commit 2223fcc into symfony:master Aug 25, 2018
dunglas added a commit that referenced this pull request Aug 25, 2018
@dunglas
feature #27926 [Serializer] XmlEncoder doesn't ignore PI nodes while …
9ef362e 
…encoding (maidmaid)

This PR was merged into the 4.2-dev branch.

Discussion
----------

[Serializer] XmlEncoder doesn't ignore PI nodes while encoding

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

It's sometimes important to get only the XML body without the processing instructions (like the `<?xml version="1.0" ?>` on the top of your XML doc). At the moment, it's possible to ignore them while decoding, but not while encoding.

```php
$encoder = new XmlEncoder('response', null, $ignoredNodeTypes = [XML_PI_NODE]);
echo $encoder->encode([], 'xml');
```
```
Expected:
<response/>

Actual:
<?xml version="1.0"?>
<response/>
```

So, a new `$encoderIgnoredNodeTypes` arg is added to the constructor which will be:

```php
public function __construct(string $rootNodeName = 'response', int $loadOptions = null, array $decoderIgnoredNodeTypes = array(XML_PI_NODE, XML_COMMENT_NODE), array $encoderIgnoredNodeTypes = array())
```

Commits
-------

2223fcc Allow to ignore PI while encoding
@maidmaid maidmaid mentioned this pull request Aug 27, 2018
Merged
fabpot added a commit that referenced this pull request Aug 27, 2018
@fabpot
minor #28281 [Serializer] Update changelog about the new $encoderIgno…
6cc2bd6 
…redNodeTypes arg in XmlEncoder contrustor (maidmaid)

This PR was merged into the 4.2-dev branch.

Discussion
----------

[Serializer] Update changelog about the new $encoderIgnoredNodeTypes arg in XmlEncoder contrustor

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

Commits
-------

49f3bfc Update changelog
@maidmaid maidmaid deleted the serializer-pi branch August 27, 2018 22:05
@maidmaid maidmaid mentioned this pull request Aug 27, 2018
Merged
fabpot added a commit that referenced this pull request Aug 30, 2018
@fabpot
feature #28289 [Serializer] Add support for ignoring comments while X…
501212b 
…ML encoding (maidmaid)

This PR was merged into the 4.2-dev branch.

Discussion
----------

[Serializer] Add support for ignoring comments while XML encoding

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

In addition to #27926 which allowed to ignore XML processing instructions, this PR allows to ignore the XML comments while encoding.

Commits
-------

8f8230a Add support for ignoring comments while XML encoding
@javiereguiluz javiereguiluz mentioned this pull request Sep 7, 2018
Closed
@javiereguiluz
Copy link
Member
javiereguiluz commented Sep 7, 2018

I've created symfony/symfony-docs#10286 to document this new feature. Please, don't forget to create a doc issue for every new feature.

@nicolas-grekas nicolas-grekas modified the milestones: next, 4.2 Nov 1, 2018
This was referenced Nov 3, 2018
Closed
Merged
@maxhelias maxhelias mentioned this pull request Oct 9, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dunglas dunglas dunglas approved these changes

@nicolas-grekas nicolas-grekas nicolas-grekas approved these changes

Assignees

No one assigned

Labels

Serializer Status: Reviewed

Projects

None yet

Milestone

4.2

Development

Successfully merging this pull request may close these issues.

6 participants

@maidmaid @dunglas @nicolas-grekas @javiereguiluz @xabbuh @carsonbot
0