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

Added XML support #7

Open
wants to merge 1 commit into
base: craft3
Choose a base branch
from
Open

Conversation

jamiematrix
Copy link

Default set to JSON, but there's now support if the endpoint is returning XML.
Currently XML attributes are not returned (other than from the parent node), but that's next on the list.

Default set to JSON, but there's now support if the endpoint is returning XML.
Currenly XML attributes are not returned (other than from the parent node), but that's next on the list.
} elseif ($format == 'xml') {
$xmlbody = simplexml_load_string($response->getBody(), null, LIBXML_NOCDATA);

$json = json_encode($xmlbody);

Choose a reason for hiding this comment

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

The $json variable is unnecessary here. Put json_encode($xmlbody) on the next line instead where $json is.

Copy link
Author

Choose a reason for hiding this comment

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

Apologies, had no idea you replied.

I tried you suggestion and the body key had a string of the JSON object returned. So the XML needs encoding to JSON then decoding.
I suppose it could go $body = json_decode(json_encode($xmlbody), true); ?

Copy link
Owner

Choose a reason for hiding this comment

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

Yeah I think that snippet is what Patrik is suggesting. A bit DRYer I guess, but I personally prefer it as you have it for readability... (If I'm misunderstanding your point @patrikalienus feel free to correct me)

Copy link
Author

Choose a reason for hiding this comment

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

Yeh, it was more for readability. It was a bit rushed too at the time, so wasn't thinking to save a few lines. I guess where there isn't much in it, then @patrikalienus suggestion is still valid.

@jalendport
Copy link
Owner

I wonder if Symfony's XmlEncoder may be a better solution here than simplexml_load_string and json_encode/json_decode?

If I'm not mistaken it's already included in Craft, and the implementation would look something like this:

use Symfony\Component\Serializer\Encoder\XmlEncoder;

...

$xmlEncoder = new XmlEncoder();
$body = $xmlEncoder->decode($response->getBody(), 'xml');

@jamiematrix let me know what you think...

@jamiematrix
Copy link
Author

I'll give it a go @jalendport thanks.

I blame Google in a way :) The change was a quick search for a project, simple_load_string worked for me at the time. I think the key reason was for the LIBXML_NOCDATA usage as the endpoint has a multitude of CDATA usage

@jamiematrix
Copy link
Author

I got Class 'Symfony\Component\Serializer\Encoder\XmlEncoder' not found when usingXmlEncoder - not sure if it was added in a later version of Craft? This site is on 3.6.18 at the moment

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.

3 participants