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

Regression of Issue #51 - XmlLocation response not handling multiple tags of the same name correctly #82

Closed
sprak3000 opened this issue Apr 25, 2015 · 3 comments

Comments

@sprak3000
Copy link
Contributor

Issue #51 was closed as fixed, but it appears there is a regression that has cropped up around it. It is back to the same behavior seen before; the first node is off on its own instead of encapsulated as item 0 in the array for the tag.

It looks like a bit of code from my original fix got nerfed at some point; the else if out of the fix (src/ResponseLocation/XmlLocation.php method xmlToArray) got nerfed:

// A child element with this name exists so we're assuming
// that the node contains a list of elements
if (!is_array($result[$name])) {
  $result[$name] = [$result[$name]];
}

should be

// A child element with this name exists so we're assuming
// that the node contains a list of elements
if (!is_array($result[$name])) {
  $result[$name] = [$result[$name]];
}  else if (!isset($result[$name][0])) {
  // Convert the first child into the first element of a numerically indexed array
  $firstResult = $result[$name];
  $result[$name] = [];
  $result[$name][] = $firstResult;
}

I've tried patching this locally, and it clears up my issue. I'll get a PR going.

sprak3000 added a commit to sprak3000/guzzle-services that referenced this issue Apr 25, 2015
…th the same name being in a proper numerically indexed array rather than having the first element having its child elements as non-numeric keys.
mtdowling added a commit that referenced this issue Aug 30, 2015
Fixing issue #82 to address regression for handling elements with the sa...
@fillup
Copy link

fillup commented Nov 7, 2015

Was this ever merged back in? I'm using version 0.5 now and this issue is present. I can confirm that the fix above resolves the issue. @sprak3000 did you submit another PR?

@sprak3000
Copy link
Contributor Author

@fillup I did submit a PR, and it was pulled into master (dbd8aef).

They just haven't tagged an official release yet. Looks like they might be pulling more stuff in prior to a 0.6 tagged release. You can grab my change from that specific commit or pull directly from head of master for now.

Hope that helps.

@Konafets
Copy link
Contributor

I will close this issue. If your problem still exist with latest versions of Guzzle and Guzzle Service, feel free to reopen it.

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

No branches or pull requests

3 participants