[GitHub] [jclouds] timuralp commented on a change in pull request #103: Do not assume Owner ID comes first in XML for S3

[GitBox <gi...@apache.org>]

timuralp commented on a change in pull request #103:
URL: https://github.com/apache/jclouds/pull/103#discussion_r606745485



##########
File path: apis/s3/src/main/java/org/jclouds/s3/xml/ListAllMyBucketsHandler.java
##########
@@ -56,9 +57,18 @@ public ListAllMyBucketsHandler(DateService dateParser) {
 
    public void endElement(String uri, String name, String qName) {
       if (qName.equals("ID")) { // owner stuff
-         currentOwner = new CanonicalUser(currentOrNull(currentText));
+         if (currentDisplayName == null) {
+            currentOwner = new CanonicalUser(currentOrNull(currentText));
+         } else {
+            currentOwner = new CanonicalUser(
+               currentOrNull(currentText), currentDisplayName);
+         }
       } else if (qName.equals("DisplayName")) {
-         currentOwner.setDisplayName(currentOrNull(currentText));
+         if (currentOwner == null) {
+            currentDisplayName = currentOrNull(currentText);
+         } else {
+            currentOwner.setDisplayName(currentOrNull(currentText));

Review comment:
       I see, yes, that's better, although in the current patch and suggested patch there is the latent bug of having the `<Owner>` element appear _after_ the bucket listing. What do you think about adding an empty constructor for the `CanonicalUser` class and setting the values once they are parsed?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org