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/8834a9ee Tree: http://git-wip-us.apache.org/repos/asf/hbase/tree/8834a9ee Diff: http://git-wip-us.apache.org/repos/asf/hbase/diff/8834a9ee Branch: refs/heads/branch-1.2 Commit: 8834a9ee60c9fae2754ab92cbf855541ef419f18 Parents: 863c2f7 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 14:12:22 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/8834a9ee/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/8834a9ee/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 bb81cc2..cf68601 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/8834a9ee/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/8834a9ee/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) { } }