HBASE-7579 HTableDescriptor equals method fails if results are returned in a different order; REAPPLPY

git-svn-id: https://svn.apache.org/repos/asf/hbase/trunk@1471054 13f79535-47bb-0310-9956-ffa450edef68

hbase-client/src/main/java/org/apache/hadoop/hbase/HColumnDescriptor.java

@@ -1125,12 +1125,10 @@ public class HColumnDescriptor implements WritableComparable<HColumnDescriptor>
   public int compareTo(HColumnDescriptor o) {
     int result = Bytes.compareTo(this.name, o.getName());
     if (result == 0) {
-      // punt on comparison for ordering, just calculate difference
-      result = this.values.hashCode() - o.values.hashCode();
-      if (result < 0)
-        result = -1;
-      else if (result > 0)
-        result = 1;
+      // The maps interface should compare values, even if they're in different orders
+      if (!this.values.equals(o.values)) {
+        return 1;
+      }
     }
     if (result == 0) {
       result = this.configuration.hashCode() - o.configuration.hashCode();

hbase-client/src/main/java/org/apache/hadoop/hbase/HTableDescriptor.java

@@ -225,9 +225,7 @@ public class HTableDescriptor implements WritableComparable<HTableDescriptor> {
    * catalog tables, <code>.META.</code> and <code>-ROOT-</code>.
    */
   protected HTableDescriptor(final byte [] name, HColumnDescriptor[] families) {
-    this.name = name.clone();
-    this.nameAsString = Bytes.toString(this.name);
-    setMetaFlags(name);
+    setName(name);
     for(HColumnDescriptor descriptor : families) {
       this.families.put(descriptor.getName(), descriptor);
     }
@@ -239,12 +237,7 @@ public class HTableDescriptor implements WritableComparable<HTableDescriptor> {
    */
   protected HTableDescriptor(final byte [] name, HColumnDescriptor[] families,
       Map<ImmutableBytesWritable,ImmutableBytesWritable> values) {
-    this.name = name.clone();
-    this.nameAsString = Bytes.toString(this.name);
-    setMetaFlags(name);
-    for(HColumnDescriptor descriptor : families) {
-      this.families.put(descriptor.getName(), descriptor);
-    }
+    this(name.clone(), families);
     for (Map.Entry<ImmutableBytesWritable, ImmutableBytesWritable> entry:
         values.entrySet()) {
       setValue(entry.getKey(), entry.getValue());
@@ -284,9 +277,7 @@ public class HTableDescriptor implements WritableComparable<HTableDescriptor> {
    */
   public HTableDescriptor(final byte [] name) {
     super();
-    setMetaFlags(this.name);
-    this.name = this.isMetaRegion()? name: isLegalTableName(name);
-    this.nameAsString = Bytes.toString(this.name);
+    setName(name);
   }
 
   /**
@@ -298,9 +289,7 @@ public class HTableDescriptor implements WritableComparable<HTableDescriptor> {
    */
   public HTableDescriptor(final HTableDescriptor desc) {
     super();
-    this.name = desc.name.clone();
-    this.nameAsString = Bytes.toString(this.name);
-    setMetaFlags(this.name);
+    setName(desc.name.clone());
     for (HColumnDescriptor c: desc.families.values()) {
       this.families.put(c.getName(), new HColumnDescriptor(c));
     }
@@ -650,9 +639,13 @@ public class HTableDescriptor implements WritableComparable<HTableDescriptor> {
    * Set the name of the table.
    *
    * @param name name of table
+   * @throws IllegalArgumentException if passed a table name
+   * that is made of other than 'word' characters, underscore or period: i.e.
+   * <code>[a-zA-Z_0-9.].
+   * @see <a href="HADOOP-1581">HADOOP-1581 HBASE: Un-openable tablename bug</a>
    */
   public void setName(byte[] name) {
-    this.name = name;
+    this.name = isMetaTable(name) ? name : isLegalTableName(name);
     this.nameAsString = Bytes.toString(this.name);
     setMetaFlags(this.name);
   }
@@ -987,39 +980,34 @@ public class HTableDescriptor implements WritableComparable<HTableDescriptor> {
    */
   @Override
   public int compareTo(final HTableDescriptor other) {
+    // Check name matches
     int result = Bytes.compareTo(this.name, other.name);
-    if (result == 0) {
-      result = families.size() - other.families.size();
-    }
-    if (result == 0 && families.size() != other.families.size()) {
-      result = Integer.valueOf(families.size()).compareTo(
-          Integer.valueOf(other.families.size()));
-    }
-    if (result == 0) {
-      for (Iterator<HColumnDescriptor> it = families.values().iterator(),
-          it2 = other.families.values().iterator(); it.hasNext(); ) {
-        result = it.next().compareTo(it2.next());
-        if (result != 0) {
-          break;
-        }
+    if (result != 0) return result;
+
+    // Check size matches
+    result = families.size() - other.families.size();
+    if (result != 0) return result;
+
+    // Compare that all column families
+    for (Iterator<HColumnDescriptor> it = families.values().iterator(),
+         it2 = other.families.values().iterator(); it.hasNext(); ) {
+      result = it.next().compareTo(it2.next());
+      if (result != 0) {
+        return result;
       }
     }
-    if (result == 0) {
-      // punt on comparison for ordering, just calculate difference
-      result = this.values.hashCode() - other.values.hashCode();
-      if (result < 0)
-        result = -1;
-      else if (result > 0)
-        result = 1;
+
+    // Compare values
+    if (!values.equals(other.values)) {
+      return 1;
     }
-    if (result == 0) {
-      result = this.configuration.hashCode() - other.configuration.hashCode();
-      if (result < 0)
-        result = -1;
-      else if (result > 0)
-        result = 1;
+
+    // Compare configuration
+    if (!configuration.equals(other.configuration)) {
+      return 1;
     }
-    return result;
+
+    return 0;
   }
 
   /**

hbase-server/src/test/java/org/apache/hadoop/hbase/TestHColumnDescriptor.java

@@ -18,12 +18,14 @@
 package org.apache.hadoop.hbase;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
 
 import org.apache.hadoop.hbase.exceptions.DeserializationException;
 import org.apache.hadoop.hbase.io.compress.Compression;
 import org.apache.hadoop.hbase.io.compress.Compression.Algorithm;
 import org.apache.hadoop.hbase.io.encoding.DataBlockEncoding;
+import org.apache.hadoop.hbase.io.hfile.BlockType;
 import org.apache.hadoop.hbase.regionserver.BloomType;
 import org.junit.experimental.categories.Category;
 
@@ -94,4 +96,71 @@ public class TestHColumnDescriptor {
     desc.removeConfiguration(key);
     assertEquals(null, desc.getConfigurationValue(key));
   }
+
+  @Test
+  public void testEqualsWithFamilyName() {
+    final String name1 = "someFamilyName";
+    HColumnDescriptor hcd1 = new HColumnDescriptor(name1);
+    HColumnDescriptor hcd2 = new HColumnDescriptor("someOtherFamilyName");
+    HColumnDescriptor hcd3 = new HColumnDescriptor(name1);
+
+    assertFalse(hcd1.equals(hcd2));
+    assertFalse(hcd2.equals(hcd1));
+
+    assertTrue(hcd3.equals(hcd1));
+    assertTrue(hcd1.equals(hcd3));
+  }
+
+  @Test
+  public void testEqualsWithAdditionalProperties() {
+    final String name1 = "someFamilyName";
+    HColumnDescriptor hcd1 = new HColumnDescriptor(name1);
+    HColumnDescriptor hcd2 = new HColumnDescriptor(name1);
+    hcd2.setBlocksize(4);
+
+    assertFalse(hcd1.equals(hcd2));
+    assertFalse(hcd2.equals(hcd1));
+
+    hcd1.setBlocksize(4);
+
+    assertTrue(hcd2.equals(hcd1));
+    assertTrue(hcd1.equals(hcd2));
+  }
+
+  @Test
+  public void testEqualsWithDifferentNumberOfProperties() {
+    final String name1 = "someFamilyName";
+    HColumnDescriptor hcd1 = new HColumnDescriptor(name1);
+    HColumnDescriptor hcd2 = new HColumnDescriptor(name1);
+    hcd2.setBlocksize(4);
+    hcd1.setBlocksize(4);
+
+    assertTrue(hcd2.equals(hcd1));
+    assertTrue(hcd1.equals(hcd2));
+
+    hcd2.setBloomFilterType(BloomType.ROW);
+
+    assertFalse(hcd1.equals(hcd2));
+    assertFalse(hcd2.equals(hcd1));
+  }
+
+  @Test
+  public void testEqualsWithDifferentOrderingOfProperties() {
+    final String name1 = "someFamilyName";
+    HColumnDescriptor hcd1 = new HColumnDescriptor(name1);
+    HColumnDescriptor hcd2 = new HColumnDescriptor(name1);
+    hcd2.setBlocksize(4);
+    hcd2.setBloomFilterType(BloomType.ROW);
+    hcd1.setBloomFilterType(BloomType.ROW);
+    hcd1.setBlocksize(4);
+
+    assertTrue(hcd2.equals(hcd1));
+    assertTrue(hcd1.equals(hcd2));
+  }
+
+  @Test
+  public void testEqualityWithSameObject() {
+    HColumnDescriptor hcd1 = new HColumnDescriptor("someName");
+    assertTrue(hcd1.equals(hcd1));
+  }
 }

hbase-server/src/test/java/org/apache/hadoop/hbase/TestHTableDescriptor.java

@@ -198,4 +198,173 @@ public class TestHTableDescriptor {
     desc.removeConfiguration(key);
     assertEquals(null, desc.getConfigurationValue(key));
   }
+
+  @Test
+  public void testEqualsWithDifferentProperties() {
+    // Test basic property difference
+    HTableDescriptor h1 = new HTableDescriptor();
+    h1.setName(Bytes.toBytes("n1"));
+
+    HTableDescriptor h2 = new HTableDescriptor();
+    h2.setName(Bytes.toBytes("n2"));
+
+    assertFalse(h2.equals(h1));
+    assertFalse(h1.equals(h2));
+
+    h2.setName(Bytes.toBytes("n1"));
+    assertTrue(h2.equals(h1));
+    assertTrue(h1.equals(h2));
+  }
+
+  @Test
+  public void testEqualsWithDifferentNumberOfItems() {
+    HTableDescriptor h1 = new HTableDescriptor();
+    HTableDescriptor h2 = new HTableDescriptor();
+
+    // Test diff # of items
+    h1 = new HTableDescriptor();
+    h1.setName(Bytes.toBytes("n1"));
+
+    h2 = new HTableDescriptor();
+    h2.setName(Bytes.toBytes("n1"));
+
+    HColumnDescriptor hcd1 = new HColumnDescriptor(Bytes.toBytes("someName"));
+    HColumnDescriptor hcd2 = new HColumnDescriptor(Bytes.toBytes("someOtherName"));
+
+    h1.addFamily(hcd1);
+    h2.addFamily(hcd1);
+    h1.addFamily(hcd2);
+
+    assertFalse(h2.equals(h1));
+    assertFalse(h1.equals(h2));
+
+    h2.addFamily(hcd2);
+
+    assertTrue(h2.equals(h1));
+    assertTrue(h1.equals(h2));
+  }
+
+  @Test
+  public void testNotEqualsWithDifferentHCDs() {
+    HTableDescriptor h1 = new HTableDescriptor();
+    HTableDescriptor h2 = new HTableDescriptor();
+
+    // Test diff # of items
+    h1 = new HTableDescriptor();
+    h1.setName(Bytes.toBytes("n1"));
+
+    h2 = new HTableDescriptor();
+    h2.setName(Bytes.toBytes("n1"));
+
+    HColumnDescriptor hcd1 = new HColumnDescriptor(Bytes.toBytes("someName"));
+    HColumnDescriptor hcd2 = new HColumnDescriptor(Bytes.toBytes("someOtherName"));
+
+    h1.addFamily(hcd1);
+    h2.addFamily(hcd2);
+
+    assertFalse(h2.equals(h1));
+    assertFalse(h1.equals(h2));
+  }
+
+  @Test
+  public void testEqualsWithDifferentHCDObjects() {
+    HTableDescriptor h1 = new HTableDescriptor();
+    HTableDescriptor h2 = new HTableDescriptor();
+
+    // Test diff # of items
+    h1 = new HTableDescriptor();
+    h1.setName(Bytes.toBytes("n1"));
+
+    h2 = new HTableDescriptor();
+    h2.setName(Bytes.toBytes("n1"));
+
+    HColumnDescriptor hcd1 = new HColumnDescriptor(Bytes.toBytes("someName"));
+    HColumnDescriptor hcd2 = new HColumnDescriptor(Bytes.toBytes("someName"));
+
+    h1.addFamily(hcd1);
+    h2.addFamily(hcd2);
+
+    assertTrue(h2.equals(h1));
+    assertTrue(h1.equals(h2));
+  }
+
+  @Test
+  public void testNotEqualsWithDifferentItems() {
+    HTableDescriptor h1 = new HTableDescriptor();
+    HTableDescriptor h2 = new HTableDescriptor();
+
+    // Test diff # of items
+    h1 = new HTableDescriptor();
+    h1.setName(Bytes.toBytes("n1"));
+
+    h2 = new HTableDescriptor();
+    h2.setName(Bytes.toBytes("n1"));
+
+    HColumnDescriptor hcd1 = new HColumnDescriptor(Bytes.toBytes("someName"));
+    HColumnDescriptor hcd2 = new HColumnDescriptor(Bytes.toBytes("someOtherName"));
+    h1.addFamily(hcd1);
+    h2.addFamily(hcd2);
+
+    assertFalse(h2.equals(h1));
+    assertFalse(h1.equals(h2));
+  }
+
+  @Test
+  public void testEqualsWithDifferentOrderingsOfItems() {
+    HTableDescriptor h1 = new HTableDescriptor();
+    HTableDescriptor h2 = new HTableDescriptor();
+
+    //Test diff # of items
+    h1 = new HTableDescriptor();
+    h1.setName(Bytes.toBytes("n1"));
+
+    h2 = new HTableDescriptor();
+    h2.setName(Bytes.toBytes("n1"));
+
+    HColumnDescriptor hcd1 = new HColumnDescriptor(Bytes.toBytes("someName"));
+    HColumnDescriptor hcd2 = new HColumnDescriptor(Bytes.toBytes("someOtherName"));
+    h1.addFamily(hcd1);
+    h2.addFamily(hcd2);
+    h1.addFamily(hcd2);
+    h2.addFamily(hcd1);
+
+    assertTrue(h2.equals(h1));
+    assertTrue(h1.equals(h2));
+  }
+
+  @Test
+  public void testSingleItemEquals() {
+    HTableDescriptor h1 = new HTableDescriptor();
+    HTableDescriptor h2 = new HTableDescriptor();
+
+    //Test diff # of items
+    h1 = new HTableDescriptor();
+    h1.setName(Bytes.toBytes("n1"));
+
+    h2 = new HTableDescriptor();
+    h2.setName(Bytes.toBytes("n1"));
+
+    HColumnDescriptor hcd1 = new HColumnDescriptor(Bytes.toBytes("someName"));
+    HColumnDescriptor hcd2 = new HColumnDescriptor(Bytes.toBytes("someName"));
+    h1.addFamily(hcd1);
+    h2.addFamily(hcd2);
+
+    assertTrue(h2.equals(h1));
+    assertTrue(h1.equals(h2));
+  }
+
+  @Test
+  public void testEmptyEquals() {
+    HTableDescriptor h1 = new HTableDescriptor();
+    HTableDescriptor h2 = new HTableDescriptor();
+
+    assertTrue(h2.equals(h1));
+    assertTrue(h1.equals(h2));
+  }
+
+  @Test
+  public void testEqualityWithSameObject() {
+    HTableDescriptor htd = new HTableDescriptor("someName");
+    assertTrue(htd.equals(htd));
+  }
 }