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 `<Buckets>` list. 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:
us...@infra.apache.org
