YARN-11366. Improve equals, hashCode(), toString() methods of the Federation Base Object. (#5096)

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common/src/main/java/org/apache/hadoop/yarn/server/federation/store/records/ApplicationHomeSubCluster.java

@@ -17,6 +17,8 @@
 
 package org.apache.hadoop.yarn.server.federation.store.records;
 
+import org.apache.commons.lang3.builder.EqualsBuilder;
+import org.apache.commons.lang3.builder.HashCodeBuilder;
 import org.apache.hadoop.classification.InterfaceAudience.Private;
 import org.apache.hadoop.classification.InterfaceAudience.Public;
 import org.apache.hadoop.classification.InterfaceStability.Unstable;
@@ -123,32 +125,42 @@ public abstract class ApplicationHomeSubCluster {
 
   @Override
   public boolean equals(Object obj) {
+
     if (this == obj) {
       return true;
     }
+
     if (obj == null) {
       return false;
     }
-    if (getClass() != obj.getClass()) {
-      return false;
-    }
-    ApplicationHomeSubCluster other = (ApplicationHomeSubCluster) obj;
-    if (!this.getApplicationId().equals(other.getApplicationId())) {
-      return false;
+
+    if (obj instanceof ApplicationHomeSubCluster) {
+      ApplicationHomeSubCluster other = (ApplicationHomeSubCluster) obj;
+      return new EqualsBuilder()
+          .append(this.getApplicationId(), other.getApplicationId())
+          .append(this.getHomeSubCluster(), other.getHomeSubCluster())
+          .isEquals();
     }
-    return this.getHomeSubCluster().equals(other.getHomeSubCluster());
+
+    return false;
   }
 
   @Override
   public int hashCode() {
-    return getApplicationId().hashCode() * 31 + getHomeSubCluster().hashCode();
+    return new HashCodeBuilder().
+        append(this.getApplicationId()).
+        append(this.getHomeSubCluster()).
+        append(this.getCreateTime()).toHashCode();
   }
 
   @Override
   public String toString() {
-    return "ApplicationHomeSubCluster [getApplicationId()="
-        + getApplicationId() + ", getHomeSubCluster()=" + getHomeSubCluster()
-        + "]";
+    StringBuilder sb = new StringBuilder();
+    sb.append("ApplicationHomeSubCluster: [")
+        .append("ApplicationId: ").append(getApplicationId()).append(", ")
+        .append("HomeSubCluster: ").append(getHomeSubCluster()).append(", ")
+        .append("CreateTime: ").append(getCreateTime()).append(", ")
+        .append("]");
+    return sb.toString();
   }
-
 }

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common/src/main/java/org/apache/hadoop/yarn/server/federation/store/records/ReservationHomeSubCluster.java

@@ -94,21 +94,24 @@ public abstract class ReservationHomeSubCluster {
 
   @Override
   public boolean equals(Object obj) {
+
     if (this == obj) {
       return true;
     }
+
     if (obj == null) {
       return false;
     }
-    if (getClass() != obj.getClass()) {
-      return false;
+
+    if (obj instanceof ReservationHomeSubCluster) {
+      ReservationHomeSubCluster other = (ReservationHomeSubCluster) obj;
+      return new EqualsBuilder()
+          .append(this.getReservationId(), other.getReservationId())
+          .append(this.getHomeSubCluster(), other.getHomeSubCluster())
+          .isEquals();
     }
-    ReservationHomeSubCluster other = (ReservationHomeSubCluster) obj;
 
-    return new EqualsBuilder()
-        .append(this.getReservationId(), other.getReservationId())
-        .append(this.getHomeSubCluster(), other.getHomeSubCluster())
-        .isEquals();
+    return false;
   }
 
   @Override
@@ -121,9 +124,11 @@ public abstract class ReservationHomeSubCluster {
 
   @Override
   public String toString() {
-    return "ReservationHomeSubCluster [getReservationId()="
-        + getReservationId() + ", getApplicationHomeSubcluster()=" + getHomeSubCluster()
-        + "]";
+    StringBuilder sb = new StringBuilder();
+    sb.append("ReservationHomeSubCluster: [")
+        .append("ReservationId: ").append(getReservationId()).append(", ")
+        .append("HomeSubCluster: ").append(getHomeSubCluster())
+        .append("]");
+    return sb.toString();
   }
-
 }

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common/src/main/java/org/apache/hadoop/yarn/server/federation/store/records/RouterMasterKey.java

@@ -114,20 +114,36 @@ public abstract class RouterMasterKey {
   }
 
   @Override
-  public boolean equals(Object right) {
-    if (this == right) {
+  public boolean equals(Object obj) {
+
+    if (this == obj) {
       return true;
     }
 
-    if (right == null || getClass() != right.getClass()) {
+    if (obj == null) {
       return false;
     }
 
-    RouterMasterKey r = (RouterMasterKey) right;
-    return new EqualsBuilder()
-        .append(this.getKeyId().intValue(), r.getKeyId().intValue())
-        .append(this.getExpiryDate().longValue(), this.getExpiryDate().longValue())
-        .append(getKeyBytes().array(), r.getKeyBytes())
-        .isEquals();
+    if (obj instanceof RouterMasterKey) {
+      RouterMasterKey other = (RouterMasterKey) obj;
+      return new EqualsBuilder()
+          .append(this.getKeyId().intValue(), other.getKeyId().intValue())
+          .append(this.getExpiryDate().longValue(), other.getExpiryDate().longValue())
+          .append(this.getKeyBytes().array(), other.getKeyBytes())
+          .isEquals();
+    }
+
+    return false;
+  }
+
+  @Override
+  public String toString() {
+    StringBuilder sb = new StringBuilder();
+    sb.append("RouterMasterKey: [")
+        .append("KeyId: ").append(getKeyId()).append(", ")
+        .append("ExpiryDate: ").append(getExpiryDate()).append(", ")
+        .append("KeyBytes: ").append(getKeyBytes()).append(", ")
+        .append("]");
+    return sb.toString();
   }
 }

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common/src/main/java/org/apache/hadoop/yarn/server/federation/store/records/SubClusterId.java

@@ -17,6 +17,8 @@
 
 package org.apache.hadoop.yarn.server.federation.store.records;
 
+import org.apache.commons.lang3.builder.EqualsBuilder;
+import org.apache.commons.lang3.builder.HashCodeBuilder;
 import org.apache.hadoop.classification.InterfaceAudience.Private;
 import org.apache.hadoop.classification.InterfaceAudience.Public;
 import org.apache.hadoop.classification.InterfaceStability.Unstable;
@@ -78,19 +80,26 @@ public abstract class SubClusterId implements Comparable<SubClusterId> {
     if (this == obj) {
       return true;
     }
+
     if (obj == null) {
       return false;
     }
-    if (getClass() != obj.getClass()) {
-      return false;
+
+    if (obj instanceof SubClusterId) {
+      SubClusterId other = (SubClusterId) obj;
+      return new EqualsBuilder()
+          .append(this.getId(), other.getId())
+          .isEquals();
     }
-    SubClusterId other = (SubClusterId) obj;
-    return this.getId().equals(other.getId());
+
+    return false;
   }
 
   @Override
   public int hashCode() {
-    return getId().hashCode();
+    return new HashCodeBuilder().
+        append(this.getId()).
+        toHashCode();
   }
 
   @Override
@@ -104,5 +113,4 @@ public abstract class SubClusterId implements Comparable<SubClusterId> {
     sb.append(getId());
     return sb.toString();
   }
-
 }

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common/src/main/java/org/apache/hadoop/yarn/server/federation/store/records/SubClusterIdInfo.java

@@ -18,6 +18,8 @@
 
 package org.apache.hadoop.yarn.server.federation.store.records;
 
+import org.apache.commons.lang3.builder.EqualsBuilder;
+import org.apache.commons.lang3.builder.HashCodeBuilder;
 import org.apache.hadoop.classification.InterfaceAudience;
 import org.apache.hadoop.classification.InterfaceStability;
 
@@ -58,18 +60,28 @@ public class SubClusterIdInfo {
   }
 
   @Override
-  public boolean equals(Object other) {
-    if (other instanceof SubClusterIdInfo) {
-      if (((SubClusterIdInfo) other).id.equals(this.id)) {
-        return true;
-      }
+  public boolean equals(Object obj) {
+
+    if (this == obj) {
+      return true;
+    }
+
+    if (obj == null) {
+      return false;
     }
+
+    if (obj instanceof SubClusterIdInfo) {
+      SubClusterIdInfo other = (SubClusterIdInfo) obj;
+      return new EqualsBuilder()
+          .append(this.id, other.id)
+          .isEquals();
+    }
+
     return false;
   }
 
   @Override
   public int hashCode() {
-    return id.hashCode();
+    return new HashCodeBuilder().append(this.id).toHashCode();
   }
-
 }

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common/src/main/java/org/apache/hadoop/yarn/server/federation/store/records/SubClusterInfo.java

@@ -17,6 +17,8 @@
 
 package org.apache.hadoop.yarn.server.federation.store.records;
 
+import org.apache.commons.lang3.builder.EqualsBuilder;
+import org.apache.commons.lang3.builder.HashCodeBuilder;
 import org.apache.hadoop.classification.InterfaceAudience.Private;
 import org.apache.hadoop.classification.InterfaceAudience.Public;
 import org.apache.hadoop.classification.InterfaceStability.Unstable;
@@ -43,6 +45,7 @@ public abstract class SubClusterInfo {
 
   @Private
   @Unstable
+  @SuppressWarnings("checkstyle:ParameterNumber")
   public static SubClusterInfo newInstance(SubClusterId subClusterId,
       String amRMServiceAddress, String clientRMServiceAddress,
       String rmAdminServiceAddress, String rmWebServiceAddress,
@@ -54,6 +57,7 @@ public abstract class SubClusterInfo {
 
   @Private
   @Unstable
+  @SuppressWarnings("checkstyle:ParameterNumber")
   public static SubClusterInfo newInstance(SubClusterId subClusterId,
       String amRMServiceAddress, String clientRMServiceAddress,
       String rmAdminServiceAddress, String rmWebServiceAddress,
@@ -252,48 +256,49 @@ public abstract class SubClusterInfo {
 
   @Override
   public String toString() {
-    return "SubClusterInfo [getSubClusterId() = " + getSubClusterId()
-        + ", getAMRMServiceAddress() = " + getAMRMServiceAddress()
-        + ", getClientRMServiceAddress() = " + getClientRMServiceAddress()
-        + ", getRMAdminServiceAddress() = " + getRMAdminServiceAddress()
-        + ", getRMWebServiceAddress() = " + getRMWebServiceAddress()
-        + ", getState() = " + getState() + ", getLastStartTime() = "
-        + getLastStartTime() + ", getCapability() = " + getCapability() + "]";
+    StringBuilder sb = new StringBuilder();
+    sb.append("SubClusterInfo: [")
+        .append("SubClusterId: ").append(getSubClusterId()).append(", ")
+        .append("AMRMServiceAddress: ").append(getAMRMServiceAddress()).append(", ")
+        .append("ClientRMServiceAddress: ").append(getClientRMServiceAddress()).append(", ")
+        .append("RMAdminServiceAddress: ").append(getRMAdminServiceAddress()).append(", ")
+        .append("RMWebServiceAddress: ").append(getRMWebServiceAddress()).append(", ")
+        .append("State: ").append(getState()).append(", ")
+        .append("LastStartTime: ").append(getLastStartTime()).append(", ")
+        .append("Capability: ").append(getCapability())
+        .append("]");
+    return sb.toString();
   }
 
   @Override
   public boolean equals(Object obj) {
+
     if (this == obj) {
       return true;
     }
+
     if (obj == null) {
       return false;
     }
+
     if (getClass() != obj.getClass()) {
       return false;
     }
-    SubClusterInfo other = (SubClusterInfo) obj;
-    if (!this.getSubClusterId().equals(other.getSubClusterId())) {
-      return false;
-    }
-    if (!this.getAMRMServiceAddress().equals(other.getAMRMServiceAddress())) {
-      return false;
-    }
-    if (!this.getClientRMServiceAddress()
-        .equals(other.getClientRMServiceAddress())) {
-      return false;
-    }
-    if (!this.getRMAdminServiceAddress()
-        .equals(other.getRMAdminServiceAddress())) {
-      return false;
-    }
-    if (!this.getRMWebServiceAddress().equals(other.getRMWebServiceAddress())) {
-      return false;
-    }
-    if (!this.getState().equals(other.getState())) {
-      return false;
+
+    if (obj instanceof SubClusterInfo) {
+      SubClusterInfo other = (SubClusterInfo) obj;
+      return new EqualsBuilder()
+          .append(this.getSubClusterId(), other.getSubClusterId())
+          .append(this.getAMRMServiceAddress(), other.getAMRMServiceAddress())
+          .append(this.getClientRMServiceAddress(), other.getClientRMServiceAddress())
+          .append(this.getRMAdminServiceAddress(), other.getRMAdminServiceAddress())
+          .append(this.getRMWebServiceAddress(), other.getRMWebServiceAddress())
+          .append(this.getState(), other.getState())
+          .append(this.getLastStartTime(), other.getLastStartTime())
+          .isEquals();
     }
-    return this.getLastStartTime() == other.getLastStartTime();
+
+    return false;
     // Capability and HeartBeat fields are not included as they are temporal
     // (i.e. timestamps), so they change during the lifetime of the same
     // sub-cluster
@@ -301,23 +306,16 @@ public abstract class SubClusterInfo {
 
   @Override
   public int hashCode() {
-    final int prime = 31;
-    int result = 1;
-    result = prime * result
-        + ((getSubClusterId() == null) ? 0 : getSubClusterId().hashCode());
-    result = prime * result + ((getAMRMServiceAddress() == null) ? 0
-        : getAMRMServiceAddress().hashCode());
-    result = prime * result + ((getClientRMServiceAddress() == null) ? 0
-        : getClientRMServiceAddress().hashCode());
-    result = prime * result + ((getRMAdminServiceAddress() == null) ? 0
-        : getRMAdminServiceAddress().hashCode());
-    result = prime * result + ((getRMWebServiceAddress() == null) ? 0
-        : getRMWebServiceAddress().hashCode());
-    result =
-        prime * result + ((getState() == null) ? 0 : getState().hashCode());
-    result = prime * result
-        + (int) (getLastStartTime() ^ (getLastStartTime() >>> 32));
-    return result;
+
+    return new HashCodeBuilder()
+        .append(this.getSubClusterId())
+        .append(this.getAMRMServiceAddress())
+        .append(this.getClientRMServiceAddress())
+        .append(this.getRMAdminServiceAddress())
+        .append(this.getRMWebServiceAddress())
+        .append(this.getState())
+        .append(this.getLastStartTime())
+        .toHashCode();
     // Capability and HeartBeat fields are not included as they are temporal
     // (i.e. timestamps), so they change during the lifetime of the same
     // sub-cluster

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common/src/main/java/org/apache/hadoop/yarn/server/federation/store/records/SubClusterPolicyConfiguration.java

@@ -18,6 +18,8 @@
 
 package org.apache.hadoop.yarn.server.federation.store.records;
 
+import org.apache.commons.lang3.builder.EqualsBuilder;
+import org.apache.commons.lang3.builder.HashCodeBuilder;
 import org.apache.hadoop.classification.InterfaceAudience.Private;
 import org.apache.hadoop.classification.InterfaceAudience.Public;
 import org.apache.hadoop.classification.InterfaceStability.Unstable;
@@ -127,36 +129,44 @@ public abstract class SubClusterPolicyConfiguration {
 
   @Override
   public int hashCode() {
-    return 31 * getParams().hashCode() + getType().hashCode();
+    return new HashCodeBuilder()
+        .append(this.getType())
+        .append(this.getQueue())
+        .append(this.getParams()).
+        toHashCode();
   }
 
   @Override
   public boolean equals(Object obj) {
+
     if (this == obj) {
       return true;
     }
+
     if (obj == null) {
       return false;
     }
-    if (getClass() != obj.getClass()) {
-      return false;
-    }
-    SubClusterPolicyConfiguration other = (SubClusterPolicyConfiguration) obj;
-    if (!this.getType().equals(other.getType())) {
-      return false;
-    }
-    if (!this.getParams().equals(other.getParams())) {
-      return false;
+
+    if (obj instanceof SubClusterPolicyConfiguration) {
+      SubClusterPolicyConfiguration other = (SubClusterPolicyConfiguration) obj;
+      return new EqualsBuilder()
+          .append(this.getType(), other.getType())
+          .append(this.getQueue(), other.getQueue())
+          .append(this.getParams(), other.getParams())
+          .isEquals();
     }
-    return true;
+
+    return false;
   }
 
   @Override
   public String toString() {
     StringBuilder sb = new StringBuilder();
-    sb.append(getType())
-        .append(" : ")
-        .append(getParams());
+    sb.append("SubClusterPolicyConfiguration: [")
+        .append("Type: ").append(getType()).append(", ")
+        .append("Queue: ").append(getQueue()).append(", ")
+        .append("Params: ").append(getParams()).append(", ")
+        .append("]");
     return sb.toString();
   }
-}
+}

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common/src/test/java/org/apache/hadoop/yarn/server/federation/store/records/TestFederationProtocolRecords.java

@@ -17,6 +17,7 @@
 
 package org.apache.hadoop.yarn.server.federation.store.records;
 
+import org.apache.hadoop.util.Time;
 import org.apache.hadoop.yarn.api.BasePBImplRecordsTest;
 import org.apache.hadoop.yarn.api.records.ApplicationId;
 import org.apache.hadoop.yarn.api.records.ReservationId;
@@ -56,6 +57,7 @@ import org.apache.hadoop.yarn.federation.proto.YarnServerFederationProtos.Router
 import org.apache.hadoop.yarn.federation.proto.YarnServerFederationProtos.RouterMasterKeyRequestProto;
 import org.apache.hadoop.yarn.federation.proto.YarnServerFederationProtos.RouterMasterKeyResponseProto;
 import org.apache.hadoop.yarn.federation.proto.YarnServerFederationProtos.ApplicationHomeSubClusterProto;
+import org.apache.hadoop.yarn.server.federation.policies.dao.WeightedPolicyInfo;
 import org.apache.hadoop.yarn.server.federation.store.records.impl.pb.AddApplicationHomeSubClusterRequestPBImpl;
 import org.apache.hadoop.yarn.server.federation.store.records.impl.pb.AddApplicationHomeSubClusterResponsePBImpl;
 import org.apache.hadoop.yarn.server.federation.store.records.impl.pb.DeleteApplicationHomeSubClusterRequestPBImpl;
@@ -97,6 +99,11 @@ import org.apache.hadoop.yarn.server.records.Version;
 import org.junit.BeforeClass;
 import org.junit.Test;
 
+import java.nio.ByteBuffer;
+
+import static org.junit.Assert.assertEquals;
+import static org.mockito.Mockito.mock;
+
 /**
  * Test class for federation protocol records.
  */
@@ -326,4 +333,92 @@ public class TestFederationProtocolRecords extends BasePBImplRecordsTest {
     validatePBImplRecord(GetReservationHomeSubClusterRequestPBImpl.class,
         GetReservationHomeSubClusterRequestProto.class);
   }
-}
+
+  @Test
+  public void testValidateApplicationHomeSubClusterEqual() throws Exception {
+    long now = Time.now();
+
+    ApplicationId appId1 = ApplicationId.newInstance(now, 1);
+    SubClusterId subClusterId1 = SubClusterId.newInstance("SC-1");
+    ApplicationHomeSubCluster applicationHomeSubCluster1 =
+        ApplicationHomeSubCluster.newInstance(appId1, subClusterId1);
+
+    ApplicationId appId2 = ApplicationId.newInstance(now, 1);
+    SubClusterId subClusterId2 = SubClusterId.newInstance("SC-1");
+    ApplicationHomeSubCluster applicationHomeSubCluster2 =
+        ApplicationHomeSubCluster.newInstance(appId2, subClusterId2);
+
+    assertEquals(applicationHomeSubCluster1, applicationHomeSubCluster2);
+  }
+
+  @Test
+  public void testValidateReservationHomeSubClusterEqual() throws Exception {
+    long now = Time.now();
+
+    ReservationId reservationId1 = ReservationId.newInstance(now, 1);
+    SubClusterId subClusterId1 = SubClusterId.newInstance("SC-1");
+    ReservationHomeSubCluster reservationHomeSubCluster1 =
+        ReservationHomeSubCluster.newInstance(reservationId1, subClusterId1);
+
+    ReservationId reservationId2 = ReservationId.newInstance(now, 1);
+    SubClusterId subClusterId2 = SubClusterId.newInstance("SC-1");
+    ReservationHomeSubCluster reservationHomeSubCluster2 =
+        ReservationHomeSubCluster.newInstance(reservationId2, subClusterId2);
+
+    assertEquals(reservationHomeSubCluster1, reservationHomeSubCluster2);
+  }
+
+  @Test
+  public void testSubClusterIdEqual() throws Exception {
+    SubClusterId subClusterId1 = SubClusterId.newInstance("SC-1");
+    SubClusterId subClusterId2 = SubClusterId.newInstance("SC-1");
+    assertEquals(subClusterId1, subClusterId2);
+  }
+
+  @Test
+  public void testSubClusterIdInfoEqual() throws Exception {
+    SubClusterIdInfo subClusterIdInfo1 = new SubClusterIdInfo("SC-1");
+    SubClusterIdInfo subClusterIdInfo2 = new SubClusterIdInfo("SC-1");
+    assertEquals(subClusterIdInfo1, subClusterIdInfo2);
+  }
+
+  @Test
+  public void testSubClusterPolicyConfigurationEqual() throws Exception {
+
+    String queue1 = "queue1";
+    WeightedPolicyInfo policyInfo1 = mock(WeightedPolicyInfo.class);
+    ByteBuffer buf1 = policyInfo1.toByteBuffer();
+    SubClusterPolicyConfiguration configuration1 = SubClusterPolicyConfiguration
+        .newInstance(queue1, policyInfo1.getClass().getCanonicalName(), buf1);
+
+    String queue2 = "queue1";
+    WeightedPolicyInfo policyInfo2 = mock(WeightedPolicyInfo.class);
+    ByteBuffer buf2 = policyInfo1.toByteBuffer();
+    SubClusterPolicyConfiguration configuration2 = SubClusterPolicyConfiguration
+        .newInstance(queue2, policyInfo2.getClass().getCanonicalName(), buf2);
+
+    assertEquals(configuration1, configuration2);
+  }
+
+  @Test
+  public void testSubClusterInfoEqual() throws Exception {
+
+    String scAmRMAddress = "5.6.7.8:5";
+    String scClientRMAddress = "5.6.7.8:6";
+    String scRmAdminAddress = "5.6.7.8:7";
+    String scWebAppAddress = "127.0.0.1:8080";
+    String capabilityJson = "-";
+
+    SubClusterInfo sc1 =
+        SubClusterInfo.newInstance(SubClusterId.newInstance("SC-1"),
+        scAmRMAddress, scClientRMAddress, scRmAdminAddress, scWebAppAddress,
+        SubClusterState.SC_RUNNING, Time.now(), capabilityJson);
+
+    SubClusterInfo sc2 =
+        SubClusterInfo.newInstance(SubClusterId.newInstance("SC-1"),
+        scAmRMAddress, scClientRMAddress, scRmAdminAddress, scWebAppAddress,
+        SubClusterState.SC_RUNNING, Time.now(), capabilityJson);
+
+    assertEquals(sc1, sc2);
+  }
+}