Skip to content
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

XmlPayload added #9

Merged
merged 6 commits into from
Aug 2, 2019
Merged

XmlPayload added #9

merged 6 commits into from
Aug 2, 2019

Conversation

filisko
Copy link
Contributor

@filisko filisko commented Jul 30, 2019

What do you think? 😅

This following code is just to convert the SimpleXML object to an array or a simple object (stdClass), this way it's more consistent to JSON payload and also if at some point we want to change the XML parser, it won't have any effect because we will still return simple arrays and objects (stdClass).

$json = json_encode($xml);
$array = json_decode($json, $this->associative);

@oscarotero
Copy link
Member

Sorry but I'm not convinced about this. XMLPayload should parse a XML code and generate a SimpleXMLElement object, but not an array. The array could be generated in a different middleware, but out of the scope of this, because there's many different ways to convert a xml structure into a json, or the user may not want an array.

If you remove the xml to array conversion, I'd merge this.

Thanks!

@filisko
Copy link
Contributor Author

filisko commented Aug 1, 2019

Great :) I didn't ask you 'What do you think?' by coincidence haha, I was with some doubts too.

What do you think about the tests? Should I create a separated method to test the empty case (it throws an exception)?

@oscarotero
Copy link
Member

The test is fine. Not sure if empty payload should return an excepcion, maybe just return null?

src/XmlPayload.php Outdated Show resolved Hide resolved
@oscarotero oscarotero merged commit 9ce74b0 into middlewares:master Aug 2, 2019
@oscarotero
Copy link
Member

Thanks! 👍

@filisko
Copy link
Contributor Author

filisko commented Aug 2, 2019

Thanks to you for sharing first!
I think you can update the repository description now "PSR-15 middleware to parse the body of the request with support for json, csv, url-encode and xml" :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants