You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hbase.apache.org by md...@apache.org on 2017/08/24 19:11:50 UTC

hbase git commit: HBASE-18656 First issues found by error-prone

Repository: hbase
Updated Branches:
  refs/heads/branch-1 016ead793 -> 9c26a42ab


HBASE-18656 First issues found by error-prone


Project: http://git-wip-us.apache.org/repos/asf/hbase/repo
Commit: http://git-wip-us.apache.org/repos/asf/hbase/commit/9c26a42a
Tree: http://git-wip-us.apache.org/repos/asf/hbase/tree/9c26a42a
Diff: http://git-wip-us.apache.org/repos/asf/hbase/diff/9c26a42a

Branch: refs/heads/branch-1
Commit: 9c26a42ab19ad86262d16c21d78a905c94c6a5e1
Parents: 016ead7
Author: Mike Drob <md...@apache.org>
Authored: Wed Aug 23 16:43:50 2017 -0500
Committer: Mike Drob <md...@apache.org>
Committed: Thu Aug 24 13:04:45 2017 -0500

----------------------------------------------------------------------
 .../hadoop/hbase/util/ConcatenatedLists.java    | 77 +-------------------
 .../apache/hadoop/hbase/TestChoreService.java   | 12 +--
 .../hbase/util/TestConcatenatedLists.java       |  4 +-
 .../hadoop/hbase/util/TestDrainBarrier.java     |  6 +-
 4 files changed, 15 insertions(+), 84 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/hbase/blob/9c26a42a/hbase-common/src/main/java/org/apache/hadoop/hbase/util/ConcatenatedLists.java
----------------------------------------------------------------------
diff --git a/hbase-common/src/main/java/org/apache/hadoop/hbase/util/ConcatenatedLists.java b/hbase-common/src/main/java/org/apache/hadoop/hbase/util/ConcatenatedLists.java
index 8a3f6c5..f6fb4b9 100644
--- a/hbase-common/src/main/java/org/apache/hadoop/hbase/util/ConcatenatedLists.java
+++ b/hbase-common/src/main/java/org/apache/hadoop/hbase/util/ConcatenatedLists.java
@@ -18,10 +18,8 @@
  */
 package org.apache.hadoop.hbase.util;
 
-import java.lang.reflect.Array;
+import java.util.AbstractCollection;
 import java.util.ArrayList;
-import java.util.Collection;
-import java.util.Iterator;
 import java.util.List;
 import java.util.NoSuchElementException;
 
@@ -34,7 +32,7 @@ import org.apache.hadoop.hbase.classification.InterfaceAudience;
  * NOTE: Doesn't implement list as it is not necessary for current usage, feel free to add.
  */
 @InterfaceAudience.Private
-public class ConcatenatedLists<T> implements Collection<T> {
+public class ConcatenatedLists<T> extends AbstractCollection<T> {
   protected final ArrayList<List<T>> components = new ArrayList<List<T>>();
   protected int size = 0;
 
@@ -57,77 +55,6 @@ public class ConcatenatedLists<T> implements Collection<T> {
   }
 
   @Override
-  public boolean isEmpty() {
-    return this.size == 0;
-  }
-
-  @Override
-  public boolean contains(Object o) {
-    for (List<T> component : this.components) {
-      if (component.contains(o)) return true;
-    }
-    return false;
-  }
-
-  @Override
-  public boolean containsAll(Collection<?> c) {
-    for (Object o : c) {
-      if (!contains(o)) return false;
-    }
-    return true;
-  }
-
-  @Override
-  public Object[] toArray() {
-    return toArray((Object[])Array.newInstance(Object.class, this.size));
-  }
-
-  @Override
-  @SuppressWarnings("unchecked")
-  public <U> U[] toArray(U[] a) {
-    U[] result = (a.length == this.size()) ? a
-        : (U[])Array.newInstance(a.getClass().getComponentType(), this.size);
-    int i = 0;
-    for (List<T> component : this.components) {
-      for (T t : component) {
-        result[i] = (U)t;
-        ++i;
-      }
-    }
-    return result;
-  }
-
-  @Override
-  public boolean add(T e) {
-    throw new UnsupportedOperationException();
-  }
-
-  @Override
-  public boolean remove(Object o) {
-    throw new UnsupportedOperationException();
-  }
-
-  @Override
-  public boolean addAll(Collection<? extends T> c) {
-    throw new UnsupportedOperationException();
-  }
-
-  @Override
-  public boolean removeAll(Collection<?> c) {
-    throw new UnsupportedOperationException();
-  }
-
-  @Override
-  public boolean retainAll(Collection<?> c) {
-    throw new UnsupportedOperationException();
-  }
-
-  @Override
-  public void clear() {
-    throw new UnsupportedOperationException();
-  }
-
-  @Override
   public java.util.Iterator<T> iterator() {
     return new Iterator();
   }

http://git-wip-us.apache.org/repos/asf/hbase/blob/9c26a42a/hbase-common/src/test/java/org/apache/hadoop/hbase/TestChoreService.java
----------------------------------------------------------------------
diff --git a/hbase-common/src/test/java/org/apache/hadoop/hbase/TestChoreService.java b/hbase-common/src/test/java/org/apache/hadoop/hbase/TestChoreService.java
index cc7b91f..06ce6d0 100644
--- a/hbase-common/src/test/java/org/apache/hadoop/hbase/TestChoreService.java
+++ b/hbase-common/src/test/java/org/apache/hadoop/hbase/TestChoreService.java
@@ -38,6 +38,8 @@ import org.junit.experimental.categories.Category;
 
 @Category(SmallTests.class)
 public class TestChoreService {
+  public static final Log log = LogFactory.getLog(TestChoreService.class);
+
   /**
    * A few ScheduledChore samples that are useful for testing with ChoreService
    */
@@ -77,7 +79,7 @@ public class TestChoreService {
         try {
           Thread.sleep(getPeriod() * 2);
         } catch (InterruptedException e) {
-          e.printStackTrace();
+          log.warn("", e);
         }
         return true;
       }
@@ -87,7 +89,7 @@ public class TestChoreService {
         try {
           Thread.sleep(getPeriod() * 2);
         } catch (InterruptedException e) {
-          //e.printStackTrace();
+          log.warn("", e);
         }
       }
     }
@@ -128,7 +130,7 @@ public class TestChoreService {
         try {
           Thread.sleep(sleepTime);
         } catch (InterruptedException e) {
-          e.printStackTrace();
+          log.warn("", e);
         }
         return true;
       }
@@ -138,7 +140,7 @@ public class TestChoreService {
         try {
           Thread.sleep(sleepTime);
         } catch (Exception e) {
-          System.err.println(e.getStackTrace());
+          log.warn("", e);
         }
       }
     }
@@ -176,7 +178,7 @@ public class TestChoreService {
       }
 
       private void outputTickCount() {
-        System.out.println("Chore: " + getName() + ". Count of chore calls: " + countOfChoreCalls);
+        log.info("Chore: " + getName() + ". Count of chore calls: " + countOfChoreCalls);
       }
 
       public int getCountOfChoreCalls() {

http://git-wip-us.apache.org/repos/asf/hbase/blob/9c26a42a/hbase-common/src/test/java/org/apache/hadoop/hbase/util/TestConcatenatedLists.java
----------------------------------------------------------------------
diff --git a/hbase-common/src/test/java/org/apache/hadoop/hbase/util/TestConcatenatedLists.java b/hbase-common/src/test/java/org/apache/hadoop/hbase/util/TestConcatenatedLists.java
index 54638d6..9b4ddb5 100644
--- a/hbase-common/src/test/java/org/apache/hadoop/hbase/util/TestConcatenatedLists.java
+++ b/hbase-common/src/test/java/org/apache/hadoop/hbase/util/TestConcatenatedLists.java
@@ -63,7 +63,7 @@ public class TestConcatenatedLists {
     } catch (UnsupportedOperationException ex) {
     }
     try {
-      c.retainAll(Arrays.asList(0L, 1L));
+      c.retainAll(Arrays.asList(0L, 2L));
       fail("Should throw");
     } catch (UnsupportedOperationException ex) {
     }
@@ -114,9 +114,11 @@ public class TestConcatenatedLists {
     verify(c, 7);
   }
 
+  @SuppressWarnings("ModifyingCollectionWithItself")
   private void verify(ConcatenatedLists<Long> c, int last) {
     assertEquals((last == -1), c.isEmpty());
     assertEquals(last + 1, c.size());
+    // This check is O(n^2), test with care
     assertTrue(c.containsAll(c));
     Long[] array = c.toArray(new Long[c.size()]);
     List<Long> all = new ArrayList<Long>();

http://git-wip-us.apache.org/repos/asf/hbase/blob/9c26a42a/hbase-common/src/test/java/org/apache/hadoop/hbase/util/TestDrainBarrier.java
----------------------------------------------------------------------
diff --git a/hbase-common/src/test/java/org/apache/hadoop/hbase/util/TestDrainBarrier.java b/hbase-common/src/test/java/org/apache/hadoop/hbase/util/TestDrainBarrier.java
index 7a8aa2b..a212e2d 100644
--- a/hbase-common/src/test/java/org/apache/hadoop/hbase/util/TestDrainBarrier.java
+++ b/hbase-common/src/test/java/org/apache/hadoop/hbase/util/TestDrainBarrier.java
@@ -43,7 +43,7 @@ public class TestDrainBarrier {
     DrainBarrier barrier = new DrainBarrier();
     try {
       barrier.endOp();
-      fail("Should have asserted");
+      throw new Error("Should have asserted");
     } catch (AssertionError e) {
     }
 
@@ -53,7 +53,7 @@ public class TestDrainBarrier {
     barrier.endOp();
     try {
       barrier.endOp();
-      fail("Should have asserted");
+      throw new Error("Should have asserted");
     } catch (AssertionError e) {
     }
   }
@@ -105,7 +105,7 @@ public class TestDrainBarrier {
     barrier.stopAndDrainOpsOnce();
     try {
       barrier.stopAndDrainOpsOnce();
-      fail("Should have asserted");
+      throw new Error("Should have asserted");
     } catch (AssertionError e) {
     }
   }