ISSUE-71 Add branch "metadata" model attribute

[vasilievip]

No description provided.

[cdancy]

So the best we would ever know is the key for the map only? I'm worried that if there is no standard for the value then devs won't know what to do with the Object.

[vasilievip]

well, from what I'm observing there is no common structure under additional metadata plugins add into branch info, so, the best what I can do is use Map structure to at least allow to query info per plugin

[cdancy]

@vasilievip hmmm ... Ok. So how would you then use this? Once you get the Object the only thing I can think of is either get its toString and parse things that way or convert to some JSON object that devs can step through. That last idea might work here.

[vasilievip]

What I'm thinking is to create POJO with fields I'm interested in and add builder with fromMap() method. I assume this Object is map as well so I'll cast it to map and pass to the builder.

[vasilievip]

I'll update test to show example

[cdancy]

Sounds good. But if that doesn't work please consider using a JsonParser like we do here and convert the Map<String, Object> to Map<String, JsonObject>

[vasilievip]

I changed Map<String, Object> to Map<String, Map> to avoid usage of cast

[vasilievip]

Map<String, Map> will not work since gson is picky on array vs object parsing
It will be up to library user to cast to map or array once parsing is done

[cdancy]

This is too specific IMO and won't work across the board. I think the correct approach here would be to use JsonParser which is already on the classpath. Something like this might work:

    public abstract Map<String, JsonObject> metadata();

    Branch() {
    }

    @SerializedNames({ "id", "displayId", "type", "latestCommit", "latestChangeset", "isDefault", "errors", "metadata" })
    public static Branch create(String id, String displayId, String type,
                                String latestCommit, String latestChangeset, boolean isDefault, List<Error> errors, Map<String, Object> metadata) {
        JsonParser parser = new JsonParser(); // could probably just use the static instance from our fallbacks class
        Map<String, JsonObject> convertedMetadata = Maps.newHashMap();
        if (metadata != null) {
             metadata.each { // using groovy/pseudo-code here
                  convertedMetadata.put(key, parser.parse(value.toString()).getAsJsonObject();
             }
        }

        return new AutoValue_Branch(Utils.nullToEmpty(errors), id, displayId, type, latestCommit, latestChangeset, isDefault, convertedMetadata);
    }

[cdancy]

The below works as expected. No need to do any converting as jclouds behind the scenes will just pass along the JsonElement should you request it. I like this approach better as it allows for variable returned data and the user can then decided how they want to handle the json.

    public abstract Map<String, JsonElement> metadata();

    Branch() {
    }

    @SerializedNames({ "id", "displayId", "type", "latestCommit", "latestChangeset", "isDefault", "metadata", "errors" })
    public static Branch create(String id, String displayId, String type,
                                String latestCommit, String latestChangeset, 
                                boolean isDefault, Map<String, JsonElement> metadata, List<Error> errors) {
        
        return new AutoValue_Branch(Utils.nullToEmpty(errors), id, displayId, type, 
                latestCommit, latestChangeset, isDefault, Utils.nullToEmpty(metadata));
    }

You'll have to make a change to the mockTest you wrote but other than that this will work.

[vasilievip]

Not that I agree with json parsing internals leakage into API model, but I've done as you suggested.
BTW https://github.com/OpenFeign/feign from netflix + some lombok seems much cleaner to build client library

[cdancy]

@vasilievip I wouldn't call passing around a JsonElement as being anything "internal". It's simply an admission that the json returned can't be modeled for all use-cases.

Yeah I've looked at feign previously and the original author of that library is the original author of jclouds which this client is based on. At the time of starting this library they didn't have jax-rs support, which is what I liked about jclouds, but I believe they've provided a hook of sorts within the last year or so to make this possible. I like the opinionated and focused view of jclouds whereas with feign it seems to want to support any and everything.

[cdancy]

@vasilievip closing in favor of #80 which builds on top of this PR. Thanks for contribution!

EDIT: trying to get a release out today which is why I just fixed this.