Skip to content

[Serializer] Cast numeric/null into xml node value to php type instead of string #34901

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
wants to merge 3 commits into from

Conversation

jewome62
Copy link
Contributor

@jewome62 jewome62 commented Dec 9, 2019

Q A
Branch? master
Bug fix? no
New feature? yes
Deprecations? no
Tickets Fix partial #33849
License MIT
Doc PR none

XML example:

<?xml version="1.0"?>
<response>
  <foo>7643796</foo>
  <bar>3.14</bar>
</response>

Currently:

array(2) {
  ["foo"]=>
  string(7) "7643796",
  ["bar"]=>
  string(4) "3.14"
}

expected

array(2) {
  ["foo"]=>
  int(7643796),
  ["bar"]=>
  float(3.14)
}

@nicolas-grekas
Copy link
Member

This looks like a behavior change: not sure it qualifies as a new feature. It's either a bug fix or a potential BC break to me.

@jewome62
Copy link
Contributor Author

jewome62 commented Dec 10, 2019

@nicolas-grekas Yes i understand.

What do you think if i created a new context key TYPE_CAST_NODES to true value
to enable this ?

@jewome62 jewome62 force-pushed the xml-decode-numeric branch 2 times, most recently from 9840d27 to a26aaae Compare December 11, 2019 14:56
@jewome62 jewome62 changed the title [Serializer] Cast numeric into xml node value to php type instead of string [Serializer] Cast numeric/null into xml node value to php type instead of string Dec 11, 2019
@jewome62
Copy link
Contributor Author

Update to add in same way the null cast.

XML example:

<?xml version="1.0"?>
<response>
  <foo/>
</response>

Currently:

array(2) {
  ["foo"]=>
  string(0) ""
}

expected

array(2) {
  ["foo"]=>
 null
}

@nicolas-grekas nicolas-grekas added this to the next milestone Dec 11, 2019
@nicolas-grekas
Copy link
Member

nicolas-grekas commented Dec 15, 2019

I would rather change validateAndDenormalize to accept casting strings to int|float when a numeric string is encountered. Can you please have a look ?

@fabpot
Copy link
Member

fabpot commented Jan 7, 2020

Any news here?

@jewome62
Copy link
Contributor Author

@nicolas-grekas Hello, Sorry for answer time.
I have speak with @dunglas about it before start.
I have copied the operation already put in other encoder.
Here you want to change on normalisation for whatever the encoder, I'm a little afraid of breaking something else

@nicolas-grekas
Copy link
Member

I think I'm 👎 here: this is not the proper place to do this. Instead, the casting logic should happen in the code that uses the resulting array to hydrate the target structure - by looking at the metadata of the target structure (as suggested in api-platform/core#3192)

@fabpot
Copy link
Member

fabpot commented Feb 8, 2020

Let's close then.

@fabpot fabpot closed this Feb 8, 2020
@nicolas-grekas nicolas-grekas modified the milestones: next, 5.1 May 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants