accumulo-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ctubb...@apache.org
Subject [2/6] accumulo git commit: ACCUMULO-4685 Remove more warnings
Date Fri, 28 Jul 2017 22:42:40 GMT
ACCUMULO-4685 Remove more warnings

Fix some more obscure warnings by ensuring type safety when using
collections methods which take "Object" instead of the expected type.
This prevents coding problems masked by the fact that a generic
Collection can never contain objects which aren't of the type in that
Collection.

Avoid use of unsafe (non-commutative) equals implementations which can
compare "apples with apple seeds". For example:
  new Value(b = new byte[]{...}).equals(b)
or
  new ProcessReference(p = new Process(...)).equals(p)

Remove some unnecessary casts of char to int.


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

Branch: refs/heads/1.8
Commit: a1b39552059f25cd41e26ae569e77252b72bb6a5
Parents: f88f5cf
Author: Christopher Tubbs <ctubbsii@apache.org>
Authored: Fri Jul 28 17:18:16 2017 -0400
Committer: Christopher Tubbs <ctubbsii@apache.org>
Committed: Fri Jul 28 17:18:16 2017 -0400

----------------------------------------------------------------------
 .../security/tokens/AuthenticationToken.java    | 12 +++++++----
 .../impl/MiniAccumuloClusterControl.java        | 10 ++++-----
 .../minicluster/impl/ProcessReference.java      | 22 +++++++++++---------
 .../apache/accumulo/server/util/DefaultMap.java |  5 +++--
 .../balancer/DefaultLoadBalancerTest.java       |  4 ++--
 .../accumulo/test/functional/LargeRowIT.java    |  2 +-
 6 files changed, 31 insertions(+), 24 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/accumulo/blob/a1b39552/core/src/main/java/org/apache/accumulo/core/client/security/tokens/AuthenticationToken.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/accumulo/core/client/security/tokens/AuthenticationToken.java
b/core/src/main/java/org/apache/accumulo/core/client/security/tokens/AuthenticationToken.java
index a623e0d..8a0e968 100644
--- a/core/src/main/java/org/apache/accumulo/core/client/security/tokens/AuthenticationToken.java
+++ b/core/src/main/java/org/apache/accumulo/core/client/security/tokens/AuthenticationToken.java
@@ -181,19 +181,22 @@ public interface AuthenticationToken extends Writable, Destroyable,
Cloneable {
     @Override
     public boolean containsKey(Object key) {
       checkDestroyed();
-      return map.containsKey(key);
+      String k = (String) key;
+      return map.containsKey(k);
     }
 
     @Override
     public boolean containsValue(Object value) {
       checkDestroyed();
-      return map.containsValue(value);
+      char[] v = (char[]) value;
+      return map.containsValue(v);
     }
 
     @Override
     public char[] get(Object key) {
       checkDestroyed();
-      return map.get(key);
+      String k = (String) key;
+      return map.get(k);
     }
 
     @Override
@@ -205,7 +208,8 @@ public interface AuthenticationToken extends Writable, Destroyable, Cloneable
{
     @Override
     public char[] remove(Object key) {
       checkDestroyed();
-      return map.remove(key);
+      String k = (String) key;
+      return map.remove(k);
     }
 
     @Override

http://git-wip-us.apache.org/repos/asf/accumulo/blob/a1b39552/minicluster/src/main/java/org/apache/accumulo/minicluster/impl/MiniAccumuloClusterControl.java
----------------------------------------------------------------------
diff --git a/minicluster/src/main/java/org/apache/accumulo/minicluster/impl/MiniAccumuloClusterControl.java
b/minicluster/src/main/java/org/apache/accumulo/minicluster/impl/MiniAccumuloClusterControl.java
index 8cc7950..53a4f76 100644
--- a/minicluster/src/main/java/org/apache/accumulo/minicluster/impl/MiniAccumuloClusterControl.java
+++ b/minicluster/src/main/java/org/apache/accumulo/minicluster/impl/MiniAccumuloClusterControl.java
@@ -297,11 +297,11 @@ public class MiniAccumuloClusterControl implements ClusterControl {
     throw new UnsupportedOperationException();
   }
 
-  public void killProcess(ServerType type, ProcessReference proc) throws ProcessNotFoundException,
InterruptedException {
+  public void killProcess(ServerType type, ProcessReference procRef) throws ProcessNotFoundException,
InterruptedException {
     boolean found = false;
     switch (type) {
       case MASTER:
-        if (proc.equals(masterProcess)) {
+        if (procRef.getProcess().equals(masterProcess)) {
           try {
             cluster.stopProcessWithTimeout(masterProcess, 30, TimeUnit.SECONDS);
           } catch (ExecutionException e) {
@@ -316,7 +316,7 @@ public class MiniAccumuloClusterControl implements ClusterControl {
       case TABLET_SERVER:
         synchronized (tabletServerProcesses) {
           for (Process tserver : tabletServerProcesses) {
-            if (proc.equals(tserver)) {
+            if (procRef.getProcess().equals(tserver)) {
               tabletServerProcesses.remove(tserver);
               try {
                 cluster.stopProcessWithTimeout(tserver, 30, TimeUnit.SECONDS);
@@ -332,7 +332,7 @@ public class MiniAccumuloClusterControl implements ClusterControl {
         }
         break;
       case ZOOKEEPER:
-        if (proc.equals(zooKeeperProcess)) {
+        if (procRef.getProcess().equals(zooKeeperProcess)) {
           try {
             cluster.stopProcessWithTimeout(zooKeeperProcess, 30, TimeUnit.SECONDS);
           } catch (ExecutionException e) {
@@ -345,7 +345,7 @@ public class MiniAccumuloClusterControl implements ClusterControl {
         }
         break;
       case GARBAGE_COLLECTOR:
-        if (proc.equals(gcProcess)) {
+        if (procRef.getProcess().equals(gcProcess)) {
           try {
             cluster.stopProcessWithTimeout(gcProcess, 30, TimeUnit.SECONDS);
           } catch (ExecutionException e) {

http://git-wip-us.apache.org/repos/asf/accumulo/blob/a1b39552/minicluster/src/main/java/org/apache/accumulo/minicluster/impl/ProcessReference.java
----------------------------------------------------------------------
diff --git a/minicluster/src/main/java/org/apache/accumulo/minicluster/impl/ProcessReference.java
b/minicluster/src/main/java/org/apache/accumulo/minicluster/impl/ProcessReference.java
index ff58869..d2859af 100644
--- a/minicluster/src/main/java/org/apache/accumulo/minicluster/impl/ProcessReference.java
+++ b/minicluster/src/main/java/org/apache/accumulo/minicluster/impl/ProcessReference.java
@@ -17,38 +17,40 @@
 
 package org.apache.accumulo.minicluster.impl;
 
+import java.util.Objects;
+
 /**
  * Opaque handle to a process.
  */
 public class ProcessReference {
-  private Process process;
+  private final Process process;
 
   ProcessReference(Process process) {
-    this.process = process;
+    this.process = Objects.requireNonNull(process);
   }
 
-  /**
-   * Visible for testing, not intended for client consumption
-   */
   public Process getProcess() {
     return process;
   }
 
   @Override
   public String toString() {
-    return process.toString();
+    return getProcess().toString();
   }
 
   @Override
   public int hashCode() {
-    return process.hashCode();
+    return getProcess().hashCode();
   }
 
   @Override
   public boolean equals(Object obj) {
-    if (obj instanceof Process) {
-      return process == obj;
+    if (this == obj) {
+      return true;
+    }
+    if (obj instanceof ProcessReference) {
+      return getProcess().equals(((ProcessReference) obj).getProcess());
     }
-    return this == obj;
+    throw new IllegalArgumentException(String.valueOf(obj) + " is not of type " + this.getClass().getName());
   }
 }

http://git-wip-us.apache.org/repos/asf/accumulo/blob/a1b39552/server/base/src/main/java/org/apache/accumulo/server/util/DefaultMap.java
----------------------------------------------------------------------
diff --git a/server/base/src/main/java/org/apache/accumulo/server/util/DefaultMap.java b/server/base/src/main/java/org/apache/accumulo/server/util/DefaultMap.java
index 66c5d8e..c32a292 100644
--- a/server/base/src/main/java/org/apache/accumulo/server/util/DefaultMap.java
+++ b/server/base/src/main/java/org/apache/accumulo/server/util/DefaultMap.java
@@ -34,10 +34,11 @@ public class DefaultMap<K,V> extends HashMap<K,V> {
   @SuppressWarnings("unchecked")
   @Override
   public V get(Object key) {
-    V result = super.get(key);
+    K k = (K) key; // fail early that key is correct type, rather than during put
+    V result = super.get(k);
     if (result == null) {
       try {
-        super.put((K) key, result = construct());
+        super.put(k, result = construct());
       } catch (Exception ex) {
         throw new RuntimeException(ex);
       }

http://git-wip-us.apache.org/repos/asf/accumulo/blob/a1b39552/server/base/src/test/java/org/apache/accumulo/server/master/balancer/DefaultLoadBalancerTest.java
----------------------------------------------------------------------
diff --git a/server/base/src/test/java/org/apache/accumulo/server/master/balancer/DefaultLoadBalancerTest.java
b/server/base/src/test/java/org/apache/accumulo/server/master/balancer/DefaultLoadBalancerTest.java
index ba19d9a..91fb826 100644
--- a/server/base/src/test/java/org/apache/accumulo/server/master/balancer/DefaultLoadBalancerTest.java
+++ b/server/base/src/test/java/org/apache/accumulo/server/master/balancer/DefaultLoadBalancerTest.java
@@ -169,7 +169,7 @@ public class DefaultLoadBalancerTest {
   public void testUnevenAssignment() {
     for (char c : "abcdefghijklmnopqrstuvwxyz".toCharArray()) {
       String cString = Character.toString(c);
-      HostAndPort fakeAddress = HostAndPort.fromParts("127.0.0.1", (int) c);
+      HostAndPort fakeAddress = HostAndPort.fromParts("127.0.0.1", c);
       String fakeInstance = cString;
       TServerInstance tsi = new TServerInstance(fakeAddress, fakeInstance);
       FakeTServer fakeTServer = new FakeTServer();
@@ -210,7 +210,7 @@ public class DefaultLoadBalancerTest {
     // make 26 servers
     for (char c : "abcdefghijklmnopqrstuvwxyz".toCharArray()) {
       String cString = Character.toString(c);
-      HostAndPort fakeAddress = HostAndPort.fromParts("127.0.0.1", (int) c);
+      HostAndPort fakeAddress = HostAndPort.fromParts("127.0.0.1", c);
       String fakeInstance = cString;
       TServerInstance tsi = new TServerInstance(fakeAddress, fakeInstance);
       FakeTServer fakeTServer = new FakeTServer();

http://git-wip-us.apache.org/repos/asf/accumulo/blob/a1b39552/test/src/test/java/org/apache/accumulo/test/functional/LargeRowIT.java
----------------------------------------------------------------------
diff --git a/test/src/test/java/org/apache/accumulo/test/functional/LargeRowIT.java b/test/src/test/java/org/apache/accumulo/test/functional/LargeRowIT.java
index 06dcfe0..b3e224a 100644
--- a/test/src/test/java/org/apache/accumulo/test/functional/LargeRowIT.java
+++ b/test/src/test/java/org/apache/accumulo/test/functional/LargeRowIT.java
@@ -202,7 +202,7 @@ public class LargeRowIT extends AccumuloClusterIT {
         if (!entry.getKey().getRow().equals(new Text(rowData))) {
           throw new Exception("verification failed, unexpected row i =" + i);
         }
-        if (!entry.getValue().equals(Integer.toString(i).getBytes(UTF_8))) {
+        if (!entry.getValue().equals(new Value(Integer.toString(i).getBytes(UTF_8)))) {
           throw new Exception("verification failed, unexpected value i =" + i + " value =
" + entry.getValue());
         }
         count++;


Mime
View raw message