You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@jclouds.apache.org by "Andrea Turli (JIRA)" <ji...@apache.org> on 2014/10/10 13:00:40 UTC

[jira] [Commented] (JCLOUDS-750) Replace hand-written domain classes with Auto-Value ones

    [ https://issues.apache.org/jira/browse/JCLOUDS-750?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14166636#comment-14166636 ] 

Andrea Turli commented on JCLOUDS-750:
--------------------------------------

Thanks [~adriancole] for this suggestion

I personally think our value classes are something that a tool can do certainly better than a human. In my experience, writing those it is the most time consuming and boring task to me.

I've revamped an old tool created by Adam Lowe to generate "jclouds-domain" objects which is able to generate the value class + the builder

i.e. from this incomplete class 

package com.example;

import java.util.List;

import org.jclouds.javax.annotation.Nullable;

import com.google.common.base.MoreObjects;
import com.google.gson.annotations.SerializedName;

public class Acl {

   @SerializedName("UserGroup")
   @Nullable
   private final String userGroup;
   @SerializedName("ResourceRole")
   private final String resourceRole;
   @SerializedName("Permissions")
   @Nullable
   private final List<String> permissions;

to 

package com.example;

import static com.google.common.base.Preconditions.checkNotNull;

import java.util.List;
import java.beans.ConstructorProperties;

import javax.inject.Named;

import org.jclouds.javax.annotation.Nullable;

import com.google.common.collect.ImmutableMap;
import com.google.gson.annotations.SerializedName;
import com.google.common.base.MoreObjects;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Sets;
import com.google.common.base.Objects;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMultimap;
import com.google.common.base.Objects.ToStringHelper;

import com.google.common.base.MoreObjects;
import com.google.common.base.Objects;
/**
 * Class Acl
*/
public class Acl {

   @SerializedName("UserGroup")
   private final String userGroup;
   @SerializedName("ResourceRole")
   private final String resourceRole;
   @SerializedName("Permissions")
   private final List<String> permissions;

   @ConstructorProperties({
      "UserGroup", "ResourceRole", "Permissions"
   })
   protected Acl(@Nullable String userGroup, String resourceRole, @Nullable List<String> permissions) {
      this.userGroup = userGroup;
      this.resourceRole = checkNotNull(resourceRole, "resourceRole");
      this.permissions = permissions == null ? ImmutableList.<String>of() : ImmutableList.copyOf(permissions);      
   }

   @Nullable
   public String getUserGroup() {
      return this.userGroup;
   }

   public String getResourceRole() {
      return this.resourceRole;
   }

   public List<String> getPermissions() {
      return this.permissions;
   }

   @Override
   public int hashCode() {
      return Objects.hashCode(userGroup, resourceRole, permissions);
   }

   @Override
   public boolean equals(Object obj) {
      if (this == obj) return true;
      if (obj == null || getClass() != obj.getClass()) return false;
      Acl that = Acl.class.cast(obj);
      return Objects.equal(this.userGroup, that.userGroup)
               && Objects.equal(this.resourceRole, that.resourceRole)
               && Objects.equal(this.permissions, that.permissions);
   }

   @Override
   public String toString() {
      return MoreObjects.toStringHelper(this)
                        .add("userGroup", userGroup)
                        .add("resourceRole", resourceRole)
                        .add("permissions", permissions)
                        .toString();
   }

   public static Builder builder() {
      return new Builder();
   }

   public Builder toBuilder() {
      return builder().fromAcl(this);
   }

   public static final class Builder {
      protected String userGroup;
      protected String resourceRole;
      protected List<String> permissions = ImmutableList.of();

      /**
      * @see Acl#getUserGroup()
      */
      public Builder userGroup(String userGroup) {
         this.userGroup = userGroup;
         return this;
      }

      /**
      * @see Acl#getResourceRole()
      */
      public Builder resourceRole(String resourceRole) {
         this.resourceRole = resourceRole;
         return this;
      }

      /**
      * @see Acl#getPermissions()
      */
      public Builder permissions(List<String> permissions) {
         this.permissions = ImmutableList.copyOf(checkNotNull(permissions, "permissions"));
         return this;
      }

      public Builder permissions(String... in) {
         return permissions(ImmutableList.copyOf(in));
      }

      public Acl build() {
         return new Acl(userGroup, resourceRole, permissions);
      }

      public Builder fromAcl(Acl in) {
         return this
         .userGroup(in.getUserGroup())
         .resourceRole(in.getResourceRole())
         .permissions(in.getPermissions());
      }
   }

}

Of course this needs to be used offline, but it could be a nice helper.
Thoughts?

> Replace hand-written domain classes with Auto-Value ones
> --------------------------------------------------------
>
>                 Key: JCLOUDS-750
>                 URL: https://issues.apache.org/jira/browse/JCLOUDS-750
>             Project: jclouds
>          Issue Type: New Feature
>            Reporter: Adrian Cole
>
> In doing maintenance and ports, I've noticed that we have drift related to using guava to implement hashCode/equals on domain classes. Having an opportunity for a guava incompatibility on something like this is not high value, in my opinion. Moreover, we have a lot of other inconsistency in our value classes, which have caused bugs, and extra review time on pull requests.
> Auto-Value generates concrete implementations and takes out the possibility of inconsistency of field names, Nullability, etc. It is handled at compile time, so doesn't introduce a dependency of note, nor a chance of guava version conflict for our users.
> https://github.com/google/auto/tree/master/value
> While it may be the case that we need custom gson adapters (ex opposed to the ConstructorAnnotation approach), or a revision to our approach, I believe that this work is worthwhile.
> While it is the case that our Builders won't be generated, I still think this is valuable. For example, in many cases, we shouldn't be making Builders anyway (ex. they are read-only objects never instantiated, such as lists). Even if we choose to still write Builders, the problem is isolated there.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)