Non-functional changes

7 files changed, 44 insertions, 62 deletions

diff --git a/documentapi/src/main/java/com/yahoo/documentapi/messagebus/protocol/ContentPolicy.java b/documentapi/src/main/java/com/yahoo/documentapi/messagebus/protocol/ContentPolicy.java
index c03231543df..fdb59941a31 100644
--- a/documentapi/src/main/java/com/yahoo/documentapi/messagebus/protocol/ContentPolicy.java
+++ b/documentapi/src/main/java/com/yahoo/documentapi/messagebus/protocol/ContentPolicy.java
@@ -221,18 +221,20 @@ public class ContentPolicy extends SlobrokPolicy {
 
     /** Class parsing the semicolon separated parameter string and exposes the appropriate value to the policy. */
     public static class Parameters {
+
         protected final String clusterName;
         protected final String distributionConfigId;
         protected final SlobrokHostPatternGenerator slobrokHostPatternGenerator;
 
         public Parameters(Map<String, String> params) {
             clusterName = params.get("cluster");
+            if (clusterName == null)
+                throw new IllegalArgumentException("Required parameter 'cluster', the name of the content cluster, not set");
             distributionConfigId = params.get("clusterconfigid"); // TODO jonmv: remove
             slobrokHostPatternGenerator = createPatternGenerator();
-            if (clusterName == null) throw new IllegalArgumentException("Required parameter 'cluster', the name of the content cluster, not set");
         }
 
-        String getDistributionConfigId() {
+        private String getDistributionConfigId() {
             return distributionConfigId == null ? clusterName : distributionConfigId;
         }
         public String getClusterName() {
diff --git a/documentapi/src/main/java/com/yahoo/documentapi/messagebus/protocol/RoutingPolicyFactories.java b/documentapi/src/main/java/com/yahoo/documentapi/messagebus/protocol/RoutingPolicyFactories.java
index 7b44a1a4f0d..a09faea73d2 100755
--- a/documentapi/src/main/java/com/yahoo/documentapi/messagebus/protocol/RoutingPolicyFactories.java
+++ b/documentapi/src/main/java/com/yahoo/documentapi/messagebus/protocol/RoutingPolicyFactories.java
@@ -3,34 +3,32 @@ package com.yahoo.documentapi.messagebus.protocol;
 
 /**
  * @author Simon Thoresen Hult
+ * @author jonmv
  */
-public abstract class RoutingPolicyFactories {
+class RoutingPolicyFactories {
+
+    private RoutingPolicyFactories() { }
 
     static class AndPolicyFactory implements RoutingPolicyFactory {
         public DocumentProtocolRoutingPolicy createPolicy(String param) {
             return new ANDPolicy(param);
         }
-
-
-        public void destroy() {
-        }
     }
 
     static class ContentPolicyFactory implements RoutingPolicyFactory {
         public DocumentProtocolRoutingPolicy createPolicy(String param) {
             return new ContentPolicy(param);
         }
-
-        public void destroy() {
-        }
     }
 
     static class MessageTypePolicyFactory implements RoutingPolicyFactory {
+
         private final String configId;
 
         public MessageTypePolicyFactory(String configId) {
             this.configId = configId;
         }
+
         public DocumentProtocolRoutingPolicy createPolicy(String param) {
             return new MessageTypePolicy((param == null || param.isEmpty()) ? configId : param);
         }
@@ -56,10 +54,6 @@ public abstract class RoutingPolicyFactories {
             }
             return ret;
         }
-
-
-        public void destroy() {
-        }
     }
 
     static class ExternPolicyFactory implements RoutingPolicyFactory {
@@ -71,49 +65,30 @@ public abstract class RoutingPolicyFactories {
             }
             return ret;
         }
-
-
-        public void destroy() {
-        }
     }
 
     static class LocalServicePolicyFactory implements RoutingPolicyFactory {
         public DocumentProtocolRoutingPolicy createPolicy(String param) {
             return new LocalServicePolicy(param);
         }
-
-
-        public void destroy() {
-        }
     }
 
     static class RoundRobinPolicyFactory implements RoutingPolicyFactory {
         public DocumentProtocolRoutingPolicy createPolicy(String param) {
             return new RoundRobinPolicy();
         }
-
-
-        public void destroy() {
-        }
     }
 
     static class LoadBalancerPolicyFactory implements RoutingPolicyFactory {
         public DocumentProtocolRoutingPolicy createPolicy(String param) {
             return new LoadBalancerPolicy(param);
         }
-
-
-        public void destroy() {
-        }
     }
 
     static class SubsetServicePolicyFactory implements RoutingPolicyFactory {
         public DocumentProtocolRoutingPolicy createPolicy(String param) {
             return new SubsetServicePolicy(param);
         }
-
-
-        public void destroy() {
-        }
     }
+
 }
diff --git a/documentapi/src/main/java/com/yahoo/documentapi/messagebus/protocol/RoutingPolicyFactory.java b/documentapi/src/main/java/com/yahoo/documentapi/messagebus/protocol/RoutingPolicyFactory.java
index 6ea5020607e..3e368832c98 100755
--- a/documentapi/src/main/java/com/yahoo/documentapi/messagebus/protocol/RoutingPolicyFactory.java
+++ b/documentapi/src/main/java/com/yahoo/documentapi/messagebus/protocol/RoutingPolicyFactory.java
@@ -24,11 +24,6 @@ public interface RoutingPolicyFactory {
      * @param param The parameter to use when creating the policy.
      * @return The created routing policy.
      */
-    public DocumentProtocolRoutingPolicy createPolicy(String param);
+    DocumentProtocolRoutingPolicy createPolicy(String param);
 
-    /**
-     * Destroys this factory and frees up any resources it has held. Making further calls on a destroyed
-     * factory causes a runtime exception.
-     */
-    public void destroy();
 }
diff --git a/documentapi/src/main/java/com/yahoo/documentapi/messagebus/protocol/SlobrokPolicy.java b/documentapi/src/main/java/com/yahoo/documentapi/messagebus/protocol/SlobrokPolicy.java
index f4e1bb33dd1..1ffce622d78 100644
--- a/documentapi/src/main/java/com/yahoo/documentapi/messagebus/protocol/SlobrokPolicy.java
+++ b/documentapi/src/main/java/com/yahoo/documentapi/messagebus/protocol/SlobrokPolicy.java
@@ -10,10 +10,10 @@ import java.util.Map;
 import java.util.TreeMap;
 
 /**
- * Abstract class for policies that allow you to specify which slobrok to use for the
- * routing.
+ * Abstract class for policies that allow you to specify which slobrok to use for the routing.
  */
 public abstract class SlobrokPolicy implements DocumentProtocolRoutingPolicy {
+
     private boolean firstTry = true;
 
     protected List<Mirror.Entry> lookup(RoutingContext context, String pattern) {
diff --git a/documentapi/src/test/java/com/yahoo/documentapi/messagebus/protocol/test/storagepolicy/ContentPolicyTest.java b/documentapi/src/test/java/com/yahoo/documentapi/messagebus/protocol/test/storagepolicy/ContentPolicyTest.java
index f324245b612..5aa3994a757 100644
--- a/documentapi/src/test/java/com/yahoo/documentapi/messagebus/protocol/test/storagepolicy/ContentPolicyTest.java
+++ b/documentapi/src/test/java/com/yahoo/documentapi/messagebus/protocol/test/storagepolicy/ContentPolicyTest.java
@@ -5,6 +5,7 @@ import org.junit.Ignore;
 import org.junit.Test;
 
 public class ContentPolicyTest extends Simulator {
+
     /**
      * Verify that a resent message with failures doesn't ruin overall performance. (By dumping the cached state too often
      * so other requests are sent to wrong target)
@@ -17,6 +18,7 @@ public class ContentPolicyTest extends Simulator {
                     + "Last correctnode 99, wrongnode 1, downnode 0, worked 92, failed 8",
                       new PersistentFailureTestParameters().addBadNode(new BadNode(3, FailureType.TRANSIENT_ERROR)));
     }
+
     /**
      * Verify that a resent message with failures doesn't ruin overall performance. (By dumping the cached state too often
      * so other requests are sent to wrong target)
@@ -29,6 +31,7 @@ public class ContentPolicyTest extends Simulator {
                     + "Last correctnode 99, wrongnode 1, downnode 0, worked 92, failed 8",
                       new PersistentFailureTestParameters().addBadNode(new BadNode(3, FailureType.FATAL_ERROR)));
     }
+
     /**
      * Verify that a node responding with old cluster state doesn't ruin overall performance (By dumping/switching cached
      * state too often)
@@ -41,6 +44,7 @@ public class ContentPolicyTest extends Simulator {
                     + "Last correctnode 100, wrongnode 0, downnode 0, worked 100, failed 0",
                       new PersistentFailureTestParameters().addBadNode(new BadNode(3, FailureType.OLD_CLUSTER_STATE).setDownInCurrentState()));
     }
+
     /**
      * Verify that a reset cluster state version doesn't keep sending requests to the wrong node.
      * We expect a few failures in first half. We should have detected the issue before second half, so there all should be fine.
@@ -52,6 +56,7 @@ public class ContentPolicyTest extends Simulator {
                     + "Last correctnode .*, wrongnode 0, downnode 0, worked .*, failed 0",
                       new PersistentFailureTestParameters().addBadNode(new BadNode(3, FailureType.RESET_CLUSTER_STATE).setDownInCurrentState()));
     }
+
     /**
      * Verify that a reset cluster state version doesn't keep sending requests to the wrong node.
      * We expect a few failures in first half. We should have detected the issue before second half, so there all should be fine.
@@ -70,6 +75,7 @@ public class ContentPolicyTest extends Simulator {
                     + "Last correctnode .*, wrongnode 100, downnode 100, worked 0, failed 100",
                       new PersistentFailureTestParameters().addBadNode(new BadNode(3, FailureType.RESET_CLUSTER_STATE_NO_GOOD_NODES).setDownInCurrentState()));
     }
+
     /**
      * Verify that a reset cluster state version doesn't keep sending requests to the wrong node.
      * We expect a few failures in first half. We should have detected the issue before second half, so there all should be fine.
@@ -86,6 +92,7 @@ public class ContentPolicyTest extends Simulator {
                     + "Last correctnode .*, wrongnode 91, downnode 0, worked 0, failed 100",
                       new PersistentFailureTestParameters().addBadNode(new BadNode(3, FailureType.RESET_CLUSTER_STATE_NO_GOOD_NODES)));
     }
+
     /**
      * Verify that a reset cluster state version doesn't keep sending requests to the wrong node.
      * Another scenario where we have a node coming up in correct state.
@@ -98,6 +105,7 @@ public class ContentPolicyTest extends Simulator {
                     + "Last correctnode .*, wrongnode 0, downnode 0, worked .*, failed 0",
                       new PersistentFailureTestParameters().newNodeAdded().addBadNode(new BadNode(3, FailureType.RESET_CLUSTER_STATE).setDownInCurrentState()));
     }
+
     /** Test node that is not in slobrok. Until fleetcontroller detects this, we expect 10% of the requests to go to wrong node. */
     @Test
     @Ignore // FIXME test has been implicitly disabled for ages, figure out and fix
diff --git a/documentapi/src/test/java/com/yahoo/documentapi/messagebus/protocol/test/storagepolicy/ContentPolicyTestEnvironment.java b/documentapi/src/test/java/com/yahoo/documentapi/messagebus/protocol/test/storagepolicy/ContentPolicyTestEnvironment.java
index 479e0b0f422..6d2477e1871 100644
--- a/documentapi/src/test/java/com/yahoo/documentapi/messagebus/protocol/test/storagepolicy/ContentPolicyTestEnvironment.java
+++ b/documentapi/src/test/java/com/yahoo/documentapi/messagebus/protocol/test/storagepolicy/ContentPolicyTestEnvironment.java
@@ -34,11 +34,13 @@ import java.util.TreeSet;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNotSame;
 import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
 
 public abstract class ContentPolicyTestEnvironment {
 
-    protected StoragePolicyTestFactory policyFactory;
+    protected ContentPolicyTestFactory policyFactory;
     protected PolicyTestFrame frame;
     private Set<Integer> nodes;
     protected static int[] bucketOneNodePreference = new int[]{ 3, 5, 7, 6, 8, 0, 9, 2, 1, 4 };
@@ -51,7 +53,7 @@ public abstract class ContentPolicyTestEnvironment {
         frame = new PolicyTestFrame(manager);
         nodes = new TreeSet<>();
         DocumentProtocol protocol = (DocumentProtocol) frame.getMessageBus().getProtocol((Utf8Array)DocumentProtocol.NAME);
-        policyFactory = new StoragePolicyTestFactory(nodes);
+        policyFactory = new ContentPolicyTestFactory(nodes);
         protocol.putRoutingPolicyFactory("storage", policyFactory);
         frame.setMessage(createMessage("id:ns:testdoc:n=1:foo"));
         frame.setHop(new HopSpec("test", "[storage:cluster=foo]"));
@@ -104,7 +106,7 @@ public abstract class ContentPolicyTestEnvironment {
 
     public static class TestHostFetcher extends ContentPolicy.HostFetcher {
         private final String clusterName;
-        private RandomGen randomizer = new RandomGen(1234);
+        private final RandomGen randomizer = new RandomGen(1234);
         private final Set<Integer> nodes;
         private Integer avoidPickingAtRandom = null;
 
@@ -121,13 +123,14 @@ public abstract class ContentPolicyTestEnvironment {
             try{
                 if (distributor == null) {
                     if (nodes.size() == 1) {
-                        assertTrue(avoidPickingAtRandom != nodes.iterator().next());
+                        assertNotSame(avoidPickingAtRandom, nodes.iterator().next());
                         distributor = nodes.iterator().next();
                     } else {
                         Iterator<Integer> it = nodes.iterator();
                         for (int i = 0, n = randomizer.nextInt(nodes.size() - 1); i<n; ++i) it.next();
                         distributor = it.next();
-                        if (avoidPickingAtRandom != null && distributor == avoidPickingAtRandom) distributor = it.next();
+                        if (avoidPickingAtRandom != null && avoidPickingAtRandom.equals(distributor))
+                            distributor = it.next();
                     }
                 }
                 if (nodes.contains(distributor)) {
@@ -137,8 +140,7 @@ public abstract class ContentPolicyTestEnvironment {
                 }
             } catch (RuntimeException e) {
                 e.printStackTrace();
-                assertTrue(e.getMessage(), false);
-                throw e;
+                throw new AssertionError(e.getMessage());
             }
         }
     }
@@ -160,12 +162,12 @@ public abstract class ContentPolicyTestEnvironment {
         public Distribution createDistribution(SlobrokPolicy policy) { return distribution; }
     }
 
-    public static class StoragePolicyTestFactory implements RoutingPolicyFactory {
+    public static class ContentPolicyTestFactory implements RoutingPolicyFactory {
         private Set<Integer> nodes;
-        private final LinkedList<TestParameters> parameterInstances = new LinkedList<TestParameters>();
+        private final LinkedList<TestParameters> parameterInstances = new LinkedList<>();
         private Integer avoidPickingAtRandom = null;
 
-        public StoragePolicyTestFactory(Set<Integer> nodes) {
+        public ContentPolicyTestFactory(Set<Integer> nodes) {
             this.nodes = nodes;
         }
         public DocumentProtocolRoutingPolicy createPolicy(String parameters) {
diff --git a/documentapi/src/test/java/com/yahoo/documentapi/messagebus/protocol/test/storagepolicy/Simulator.java b/documentapi/src/test/java/com/yahoo/documentapi/messagebus/protocol/test/storagepolicy/Simulator.java
index d23dd9ea998..be880e69781 100644
--- a/documentapi/src/test/java/com/yahoo/documentapi/messagebus/protocol/test/storagepolicy/Simulator.java
+++ b/documentapi/src/test/java/com/yahoo/documentapi/messagebus/protocol/test/storagepolicy/Simulator.java
@@ -31,7 +31,7 @@ public abstract class Simulator extends ContentPolicyTestEnvironment {
         RESET_CLUSTER_STATE_NO_GOOD_NODES,
         NODE_NOT_IN_SLOBROK
     };
-    private Integer getIdealTarget(String idString, String clusterState) {
+    private int getIdealTarget(String idString, String clusterState) {
         DocumentId did = new DocumentId(idString);
         BucketIdFactory factory = new BucketIdFactory();
         BucketId bid = factory.getBucketId(did);
@@ -145,6 +145,7 @@ public abstract class Simulator extends ContentPolicyTestEnvironment {
             return currentClusterState;
         }
     }
+
     public void runSimulation(String expected, PersistentFailureTestParameters params) {
         params.validate();
         // Set nodes in slobrok
@@ -157,16 +158,16 @@ public abstract class Simulator extends ContentPolicyTestEnvironment {
             replyWrongDistribution(target, "foo", null, params.getInitialClusterState().toString());
         }
         RandomGen randomizer = new RandomGen(432121);
-        int correctnode[] = new int[2],
-                wrongnode[] = new int[2],
-                failed[] = new int[2],
-                worked[] = new int[2],
-                downnode[] = new int[2];
+        int[] correctnode = new int[2],
+                wrongnode = new int[2],
+                   failed = new int[2],
+                   worked = new int[2],
+                 downnode = new int[2];
         for (int step = 0, steps = (params.getTotalRequests() / params.getParallellRequests()); step < steps; ++step) {
             int half = (step < steps / 2 ? 0 : 1);
             if (debug) System.err.println("Starting step " + step + " in half " + half);
-            String docId[] = new String[params.getParallellRequests()];
-            RoutingNode targets[] = new RoutingNode[params.getParallellRequests()];
+            String[] docId = new String[params.getParallellRequests()];
+            RoutingNode[] targets = new RoutingNode[params.getParallellRequests()];
             for (int i=0; i<params.getParallellRequests(); ++i) {
                 docId[i] = "id:ns:testdoc::" + (step * params.getParallellRequests() + i);
                 frame.setMessage(createMessage(docId[i]));
@@ -206,7 +207,6 @@ public abstract class Simulator extends ContentPolicyTestEnvironment {
             }
         }
         StringBuilder actual = new StringBuilder();
-        String result[][] = new String[2][];
         for (int i=0; i<2; ++i) {
             actual.append(i == 0 ? "First " : " Last ")
                     .append("correctnode ").append(correctnode[i])
@@ -215,7 +215,7 @@ public abstract class Simulator extends ContentPolicyTestEnvironment {
                     .append(", worked ").append(worked[i])
                     .append(", failed ").append(failed[i]);
         }
-        if (!Pattern.matches(expected, actual.toString())) {
+        if ( ! Pattern.matches(expected, actual.toString())) {
             assertEquals(expected, actual.toString());
         }
     }