This is an automated email from the ASF dual-hosted git repository.

bschuchardt pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/geode.git


The following commit(s) were added to refs/heads/develop by this push:
     new a3b6486  GEODE-5984 Address LGTM recommendations
a3b6486 is described below

commit a3b648613536d55376c5bc87509f0efc4f9f8f02
Author: Bruce Schuchardt <bschucha...@pivotal.io>
AuthorDate: Tue Nov 6 09:16:54 2018 -0800

    GEODE-5984 Address LGTM recommendations
    
    Addressing minor issues in javadocs and code reported by LooksGoodToMe.
    
    One change that I made wasn't reported by that tool: I removed
    two static variables from AcceptorImpl that duplicated variables in
    Handshake.
    
    This closes #2780
---
 .../gatewaydelta/GatewayDeltaCreateEvent.java      |   3 +-
 .../GatewayDeltaForwarderCacheListener.java        |   4 -
 .../internal/provider/CustomKeyManagerFactory.java |   3 +-
 .../provider/CustomTrustManagerFactory.java        |   3 +-
 .../geode/cache/PartitionAttributesFactory.java    |   9 -
 .../internal/AbstractDistributionConfig.java       |   2 +-
 .../geode/internal/HeapDataOutputStream.java       |   4 +-
 .../geode/internal/InternalDataSerializer.java     |  15 +-
 .../geode/internal/InternalInstantiator.java       |  16 +-
 .../apache/geode/internal/ObjIdConcurrentMap.java  | 278 ---------------------
 .../geode/internal/cache/GemFireCacheImpl.java     |   2 +-
 .../internal/cache/PartitionedRegionDataStore.java |  23 --
 .../geode/internal/cache/map/RegionMapDestroy.java |   4 +-
 .../internal/cache/tier/sockets/AcceptorImpl.java  |   4 +-
 .../tier/sockets/ClientDataSerializerMessage.java  |   7 +-
 .../tier/sockets/ClientInstantiatorMessage.java    |   7 +-
 .../cache/tier/sockets/ServerConnection.java       |   8 +-
 .../internal/datasource/AbstractPoolCache.java     |  11 -
 .../datasource/GemFireConnPooledDataSource.java    |  14 +-
 .../geode/internal/process/StartupStatus.java      |   4 +-
 .../management/internal/cli/i18n/CliStrings.java   |   4 +-
 .../geode/internal/cache/execute/data/Order.java   |   8 +
 .../internal/cache/execute/data/Shipment.java      |  10 +
 .../tools/pulse/internal/data/JMXDataUpdater.java  |   2 +-
 .../web/controllers/AddFreeItemToOrders.java       |  31 +--
 25 files changed, 81 insertions(+), 395 deletions(-)

diff --git 
a/extensions/geode-modules/src/main/java/org/apache/geode/modules/gatewaydelta/GatewayDeltaCreateEvent.java
 
b/extensions/geode-modules/src/main/java/org/apache/geode/modules/gatewaydelta/GatewayDeltaCreateEvent.java
index ee107f9..5b18012 100644
--- 
a/extensions/geode-modules/src/main/java/org/apache/geode/modules/gatewaydelta/GatewayDeltaCreateEvent.java
+++ 
b/extensions/geode-modules/src/main/java/org/apache/geode/modules/gatewaydelta/GatewayDeltaCreateEvent.java
@@ -17,6 +17,7 @@ package org.apache.geode.modules.gatewaydelta;
 import java.io.DataInput;
 import java.io.DataOutput;
 import java.io.IOException;
+import java.util.Arrays;
 
 import org.apache.geode.DataSerializer;
 import org.apache.geode.cache.Cache;
@@ -65,6 +66,6 @@ public class GatewayDeltaCreateEvent extends 
AbstractGatewayDeltaEvent {
   public String toString() {
     return new 
StringBuilder().append("GatewayDeltaCreateEvent[").append("regionName=")
         .append(this.regionName).append("; key=").append(this.key).append("; 
gatewayDelta=")
-        .append(this.gatewayDelta).append("]").toString();
+        .append(Arrays.toString(this.gatewayDelta)).append("]").toString();
   }
 }
diff --git 
a/extensions/geode-modules/src/main/java/org/apache/geode/modules/gatewaydelta/GatewayDeltaForwarderCacheListener.java
 
b/extensions/geode-modules/src/main/java/org/apache/geode/modules/gatewaydelta/GatewayDeltaForwarderCacheListener.java
index b500256..7ddc402 100644
--- 
a/extensions/geode-modules/src/main/java/org/apache/geode/modules/gatewaydelta/GatewayDeltaForwarderCacheListener.java
+++ 
b/extensions/geode-modules/src/main/java/org/apache/geode/modules/gatewaydelta/GatewayDeltaForwarderCacheListener.java
@@ -67,9 +67,6 @@ public class GatewayDeltaForwarderCacheListener extends 
CacheListenerAdapter<Str
         getGatewayDeltaRegion().put(sessionId, new 
GatewayDeltaCreateEvent(regionName, sessionId,
             EntryEventImpl.serialize(event.getNewValue())));
       } else {
-        System.out.println(
-            "GatewayDeltaForwarderCacheListener 
event.getSerializedNewValue().getSerializedValue(): "
-                + event.getSerializedNewValue().getSerializedValue());
         getGatewayDeltaRegion().put(sessionId,
             new GatewayDeltaCreateEvent(regionName, sessionId, 
scv.getSerializedValue()));
       }
@@ -85,7 +82,6 @@ public class GatewayDeltaForwarderCacheListener extends 
CacheListenerAdapter<Str
   }
 
   public void afterUpdate(EntryEvent<String, GatewayDelta> event) {
-    // System.out.println("GatewayDeltaForwarderCacheListener.afterUpdate: " + 
event);
     // If the event is from the local site, create an 'update' event and send 
it to the
     // gateway delta region
     if (event.getCallbackArgument() == null) {
diff --git 
a/geode-core/src/distributedTest/java/org/apache/geode/cache/client/internal/provider/CustomKeyManagerFactory.java
 
b/geode-core/src/distributedTest/java/org/apache/geode/cache/client/internal/provider/CustomKeyManagerFactory.java
index 75cf24a..13ec994 100644
--- 
a/geode-core/src/distributedTest/java/org/apache/geode/cache/client/internal/provider/CustomKeyManagerFactory.java
+++ 
b/geode-core/src/distributedTest/java/org/apache/geode/cache/client/internal/provider/CustomKeyManagerFactory.java
@@ -69,8 +69,7 @@ public abstract class CustomKeyManagerFactory extends 
KeyManagerFactorySpi {
     String SSL_KEYSTORE_TYPE = "JKS";
     String SSL_KEYSTORE_PASSWORD = "password";
 
-    try {
-      FileInputStream fileInputStream = new FileInputStream(keyStorePath);
+    try (FileInputStream fileInputStream = new FileInputStream(keyStorePath)) {
       KeyStore keyStore = KeyStore.getInstance(SSL_KEYSTORE_TYPE);
       keyStore.load(fileInputStream, SSL_KEYSTORE_PASSWORD.toCharArray());
       this.customKeyManagerFactory = 
KeyManagerFactory.getInstance(this.algorithm, "SunJSSE");
diff --git 
a/geode-core/src/distributedTest/java/org/apache/geode/cache/client/internal/provider/CustomTrustManagerFactory.java
 
b/geode-core/src/distributedTest/java/org/apache/geode/cache/client/internal/provider/CustomTrustManagerFactory.java
index 6d11455..c683cf9 100644
--- 
a/geode-core/src/distributedTest/java/org/apache/geode/cache/client/internal/provider/CustomTrustManagerFactory.java
+++ 
b/geode-core/src/distributedTest/java/org/apache/geode/cache/client/internal/provider/CustomTrustManagerFactory.java
@@ -81,8 +81,7 @@ public abstract class CustomTrustManagerFactory extends 
TrustManagerFactorySpi {
     String trustStoreType = "JKS";
     String trustStorePassword = "password";
 
-    try {
-      FileInputStream fileInputStream = new FileInputStream(trustStorePath);
+    try (FileInputStream fileInputStream = new 
FileInputStream(trustStorePath)) {
       KeyStore trustStore = KeyStore.getInstance(trustStoreType);
       trustStore.load(fileInputStream, trustStorePassword.toCharArray());
       this.customTrustManagerFactory = 
TrustManagerFactory.getInstance(this.algorithm, "SunJSSE");
diff --git 
a/geode-core/src/main/java/org/apache/geode/cache/PartitionAttributesFactory.java
 
b/geode-core/src/main/java/org/apache/geode/cache/PartitionAttributesFactory.java
index 198e557..7697b67 100644
--- 
a/geode-core/src/main/java/org/apache/geode/cache/PartitionAttributesFactory.java
+++ 
b/geode-core/src/main/java/org/apache/geode/cache/PartitionAttributesFactory.java
@@ -169,15 +169,6 @@ public class PartitionAttributesFactory<K, V> {
   }
 
   /**
-   * Sets the cache writer for the next <code>PartitionAttributes</code> 
created. <i>Currently
-   * unsupported for the early access release.</i>
-   *
-   * @param cacheWriter the cache writer or null if no cache writer
-   * @return this public PartitionAttributesFactory<K,V> 
setCacheWriter(CacheWriter cacheWriter) {
-   *         this.partitionAttributes.setCacheWriter(cacheWriter); return 
this; }
-   */
-
-  /**
    * Sets the maximum amount of memory, in megabytes, to be used by the region 
in this process. If
    * not set, a default of 90% of available heap is used.
    */
diff --git 
a/geode-core/src/main/java/org/apache/geode/distributed/internal/AbstractDistributionConfig.java
 
b/geode-core/src/main/java/org/apache/geode/distributed/internal/AbstractDistributionConfig.java
index da050b2..56e04f4 100644
--- 
a/geode-core/src/main/java/org/apache/geode/distributed/internal/AbstractDistributionConfig.java
+++ 
b/geode-core/src/main/java/org/apache/geode/distributed/internal/AbstractDistributionConfig.java
@@ -694,7 +694,7 @@ public abstract class AbstractDistributionConfig extends 
AbstractConfig
         default:
           throw new IllegalArgumentException(
               String.format("%s is not in the valid set of options %s",
-                  value,
+                  Arrays.toString(value),
                   StringUtils
                       .join(new String[] 
{SecurableCommunicationChannel.ALL.getConstant(),
                           SecurableCommunicationChannel.CLUSTER.getConstant(),
diff --git 
a/geode-core/src/main/java/org/apache/geode/internal/HeapDataOutputStream.java 
b/geode-core/src/main/java/org/apache/geode/internal/HeapDataOutputStream.java
index d07e7ef..1882f73 100755
--- 
a/geode-core/src/main/java/org/apache/geode/internal/HeapDataOutputStream.java
+++ 
b/geode-core/src/main/java/org/apache/geode/internal/HeapDataOutputStream.java
@@ -190,7 +190,7 @@ public class HeapDataOutputStream extends OutputStream
       return;
     checkIfWritable();
     ensureCapacity(1);
-    buffer.put((byte) b);
+    buffer.put((byte) (b & 0xff));
   }
 
   private void ensureCapacity(int amount) {
@@ -888,7 +888,7 @@ public class HeapDataOutputStream extends OutputStream
       return;
     checkIfWritable();
     ensureCapacity(2);
-    buffer.putShort((short) v);
+    buffer.putShort((short) (v & 0xffff));
   }
 
   /**
diff --git 
a/geode-core/src/main/java/org/apache/geode/internal/InternalDataSerializer.java
 
b/geode-core/src/main/java/org/apache/geode/internal/InternalDataSerializer.java
index 3fba509..1aa8ff7 100644
--- 
a/geode-core/src/main/java/org/apache/geode/internal/InternalDataSerializer.java
+++ 
b/geode-core/src/main/java/org/apache/geode/internal/InternalDataSerializer.java
@@ -855,14 +855,15 @@ public abstract class InternalDataSerializer extends 
DataSerializer {
       init = c.getDeclaredConstructor(new Class[0]);
 
     } catch (NoSuchMethodException ignored) {
-      String s = "Class %s does not have a zero-argument constructor.";
-      Object[] args = new Object[] {c.getName()};
       if (c.getDeclaringClass() != null) {
-        s =
-            "Class %s does not have a zero-argument constructor. It is an 
inner class of %s. Should it be a static inner class?";
-        args = new Object[] {c.getName(), c.getDeclaringClass()};
-      }
-      throw new IllegalArgumentException(String.format(s, args));
+        String message = String.format(
+            "Class %s does not have a zero-argument constructor. It is an 
inner class of %s. Should it be a static inner class?",
+            c.getName(), c.getDeclaringClass());
+        throw new IllegalArgumentException(message);
+      }
+      String message = String.format("Class %s does not have a zero-argument 
constructor.",
+          c.getName());
+      throw new IllegalArgumentException(message);
     }
 
     DataSerializer s;
diff --git 
a/geode-core/src/main/java/org/apache/geode/internal/InternalInstantiator.java 
b/geode-core/src/main/java/org/apache/geode/internal/InternalInstantiator.java
index 4448752..6512345 100644
--- 
a/geode-core/src/main/java/org/apache/geode/internal/InternalInstantiator.java
+++ 
b/geode-core/src/main/java/org/apache/geode/internal/InternalInstantiator.java
@@ -609,16 +609,16 @@ public class InternalInstantiator {
         types = new Class[] {Class.class, byte.class};
         init = instantiatorClass.getDeclaredConstructor(types);
       } catch (NoSuchMethodException ex2) {
-        String msg =
-            "Class %s does not have a two-argument (Class, int) constructor.";
-        Object[] msgArgs = new Object[] {instantiatorClass.getName()};
         if (instantiatorClass.getDeclaringClass() != null) {
-          msg =
-              "Class %s does not have a two-argument (Class, int) constructor. 
It is an inner class of %s. Should it be a static inner class?";
-          msgArgs =
-              new Object[] {instantiatorClass.getName(), 
instantiatorClass.getDeclaringClass()};
+          String msg = String.format(
+              "Class %s does not have a two-argument (Class, int) constructor. 
It is an inner class of %s. Should it be a static inner class?",
+              instantiatorClass.getName(), 
instantiatorClass.getDeclaringClass());
+          throw new IllegalArgumentException(msg);
         }
-        throw new IllegalArgumentException(String.format(msg, msgArgs));
+        String msg = String.format(
+            "Class %s does not have a two-argument (Class, int) constructor.",
+            instantiatorClass.getName());
+        throw new IllegalArgumentException(msg);
       }
     }
 
diff --git 
a/geode-core/src/main/java/org/apache/geode/internal/ObjIdConcurrentMap.java 
b/geode-core/src/main/java/org/apache/geode/internal/ObjIdConcurrentMap.java
index 2c4f9dc..b6eff87 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/ObjIdConcurrentMap.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/ObjIdConcurrentMap.java
@@ -21,7 +21,6 @@ import java.util.HashMap;
 import java.util.Hashtable;
 import java.util.Iterator;
 import java.util.Map;
-import java.util.NoSuchElementException;
 import java.util.concurrent.locks.ReentrantLock;
 
 /**
@@ -623,19 +622,6 @@ public class ObjIdConcurrentMap<V> /* extends 
AbstractMap<K, V> */
   }
 
   /**
-   * Creates a new map with the same mappings as the given map. The map is 
created with a capacity
-   * of 1.5 times the number of mappings in the given map or 16 (whichever is 
greater), and a
-   * default load factor (0.75) and concurrencyLevel (16).
-   *
-   * @param m the map
-   */
-  /*
-   * public ObjIdConcurrentMap(Map<? extends K, ? extends V> m) { 
this(Math.max((int) (m.size() /
-   * DEFAULT_LOAD_FACTOR) + 1, DEFAULT_INITIAL_CAPACITY), DEFAULT_LOAD_FACTOR,
-   * DEFAULT_CONCURRENCY_LEVEL); putAll(m); }
-   */
-
-  /**
    * Returns <tt>true</tt> if this map contains no key-value mappings.
    *
    * @return <tt>true</tt> if this map contains no key-value mappings
@@ -854,17 +840,6 @@ public class ObjIdConcurrentMap<V> /* extends 
AbstractMap<K, V> */
   }
 
   /**
-   * Copies all of the mappings from the specified map to this one. These 
mappings replace any
-   * mappings that this map had for any of the keys currently in the specified 
map.
-   *
-   * @param m mappings to be stored in this map
-   */
-  /*
-   * public void putAll(Map<? extends K, ? extends V> m) { for (Map.Entry<? 
extends K, ? extends V>
-   * e : m.entrySet()) put(e.getKey(), e.getValue()); }
-   */
-
-  /**
    * Removes the key (and its corresponding value) from this map. This method 
does nothing if the
    * key is not in the map.
    *
@@ -921,259 +896,6 @@ public class ObjIdConcurrentMap<V> /* extends 
AbstractMap<K, V> */
       segments[i].clear();
   }
 
-  /*
-   * Returns a {@link Set} view of the keys contained in this map. The set is 
backed by the map, so
-   * changes to the map are reflected in the set, and vice-versa. The set 
supports element removal,
-   * which removes the corresponding mapping from this map, via the 
<tt>Iterator.remove</tt>,
-   * <tt>Set.remove</tt>, <tt>removeAll</tt>, <tt>retainAll</tt>, and 
<tt>clear</tt> operations. It
-   * does not support the <tt>add</tt> or <tt>addAll</tt> operations.
-   *
-   * <p>The view's <tt>iterator</tt> is a "weakly consistent" iterator that 
will never throw {@link
-   * ConcurrentModificationException}, and guarantees to traverse elements as 
they existed upon
-   * construction of the iterator, and may (but is not guaranteed to) reflect 
any modifications
-   * subsequent to construction.
-   */
-  /*
-   * public Set<K> keySet() { Set<K> ks = keySet; return (ks != null) ? ks : 
(keySet = new
-   * KeySet()); }
-   *
-   * /** Returns a {@link Collection} view of the values contained in this 
map. The collection is
-   * backed by the map, so changes to the map are reflected in the collection, 
and vice-versa. The
-   * collection supports element removal, which removes the corresponding 
mapping from this map, via
-   * the <tt>Iterator.remove</tt>, <tt>Collection.remove</tt>, 
<tt>removeAll</tt>,
-   * <tt>retainAll</tt>, and <tt>clear</tt> operations. It does not support 
the <tt>add</tt> or
-   * <tt>addAll</tt> operations.
-   *
-   * <p>The view's <tt>iterator</tt> is a "weakly consistent" iterator that 
will never throw {@link
-   * ConcurrentModificationException}, and guarantees to traverse elements as 
they existed upon
-   * construction of the iterator, and may (but is not guaranteed to) reflect 
any modifications
-   * subsequent to construction.
-   */
-  /*
-   * public Collection<V> values() { Collection<V> vs = values; return (vs != 
null) ? vs : (values =
-   * new Values()); }
-   *
-   * /** Returns a {@link Set} view of the mappings contained in this map. The 
set is backed by the
-   * map, so changes to the map are reflected in the set, and vice-versa. The 
set supports element
-   * removal, which removes the corresponding mapping from the map, via the
-   * <tt>Iterator.remove</tt>, <tt>Set.remove</tt>, <tt>removeAll</tt>, 
<tt>retainAll</tt>, and
-   * <tt>clear</tt> operations. It does not support the <tt>add</tt> or 
<tt>addAll</tt> operations.
-   *
-   * <p>The view's <tt>iterator</tt> is a "weakly consistent" iterator that 
will never throw {@link
-   * ConcurrentModificationException}, and guarantees to traverse elements as 
they existed upon
-   * construction of the iterator, and may (but is not guaranteed to) reflect 
any modifications
-   * subsequent to construction.
-   */
-  /*
-   * public Set<Map.Entry<K,V>> entrySet() { Set<Map.Entry<K,V>> es = 
entrySet; return (es != null)
-   * ? es : (entrySet = new EntrySet()); }
-   *
-   * /** Returns an enumeration of the keys in this table.
-   *
-   * @return an enumeration of the keys in this table
-   *
-   * @see #keySet()
-   */
-  /*
-   * public Enumeration<K> keys() { return new KeyIterator(); }
-   *
-   * /** Returns an enumeration of the values in this table.
-   *
-   * @return an enumeration of the values in this table
-   *
-   * @see #values()
-   */
-  /*
-   * public Enumeration<V> elements() { return new ValueIterator(); }
-   */
-
-  /* ---------------- Iterator Support -------------- */
-
-  abstract class HashIterator {
-    int nextSegmentIndex;
-    int nextTableIndex;
-    HashEntry<V>[] currentTable;
-    HashEntry<V> nextEntry;
-    HashEntry<V> lastReturned;
-
-    HashIterator() {
-      nextSegmentIndex = segments.length - 1;
-      nextTableIndex = -1;
-      advance();
-    }
-
-    public boolean hasMoreElements() {
-      return hasNext();
-    }
-
-    void advance() {
-      if (nextEntry != null && (nextEntry = nextEntry.next) != null)
-        return;
-
-      while (nextTableIndex >= 0) {
-        if ((nextEntry = currentTable[nextTableIndex--]) != null)
-          return;
-      }
-
-      while (nextSegmentIndex >= 0) {
-        Segment<V> seg = segments[nextSegmentIndex--];
-        if (seg.count != 0) {
-          currentTable = seg.table;
-          for (int j = currentTable.length - 1; j >= 0; --j) {
-            if ((nextEntry = currentTable[j]) != null) {
-              nextTableIndex = j - 1;
-              return;
-            }
-          }
-        }
-      }
-    }
-
-    public boolean hasNext() {
-      return nextEntry != null;
-    }
-
-    HashEntry<V> nextEntry() {
-      if (nextEntry == null)
-        throw new NoSuchElementException();
-      lastReturned = nextEntry;
-      advance();
-      return lastReturned;
-    }
-
-    public void remove() {
-      if (lastReturned == null)
-        throw new IllegalStateException();
-      ObjIdConcurrentMap.this.remove(lastReturned.key);
-      lastReturned = null;
-    }
-  }
-
-  /*
-   * class KeyIterator extends HashIterator implements Iterator<K>, 
Enumeration<K> { public K next()
-   * { return super.nextEntry().key; } public K nextElement() { return 
super.nextEntry().key; } }
-   *
-   * class ValueIterator extends HashIterator implements Iterator<V>, 
Enumeration<V> { public V
-   * next() { return super.nextEntry().value; } public V nextElement() { return
-   * super.nextEntry().value; } }
-   */
-
-  // interface Entry<V> {
-  // int getKey();
-  // V getValue();
-  // V setValue(V value);
-  // boolean equals(Object o);
-  // int hashCode();
-  // }
-
-
-  // static class SimpleEntry<V> implements Entry<V> {
-  // int key;
-  // V value;
-  //
-  // public SimpleEntry(int key, V value) {
-  // this.key = key;
-  // this.value = value;
-  // }
-  //
-  // public SimpleEntry(Entry<V> e) {
-  // this.key = e.getKey();
-  // this.value = e.getValue();
-  // }
-  //
-  // public int getKey() {
-  // return key;
-  // }
-  //
-  // public V getValue() {
-  // return value;
-  // }
-  //
-  // public V setValue(V value) {
-  // V oldValue = this.value;
-  // this.value = value;
-  // return oldValue;
-  // }
-  //
-  // @Override
-  // public boolean equals(Object o) {
-  // if (!(o instanceof Map.Entry))
-  // return false;
-  // Map.Entry<?, ?> e = (Map.Entry<?, ?>)o;
-  // return eq(key, e.getKey()) && eq(value, e.getValue());
-  // }
-  //
-  // @Override
-  // public int hashCode() {
-  // return (key) ^
-  // ((value == null) ? 0 : value.hashCode());
-  // }
-  //
-  // @Override
-  // public String toString() {
-  // return key + "=" + value;
-  // }
-  //
-  // private static boolean eq(Object o1, Object o2) {
-  // return (o1 == null ? o2 == null : o1.equals(o2));
-  // }
-  // }
-  // /**
-  // * Custom Entry class used by EntryIterator.next(), that relays
-  // * setValue changes to the underlying map.
-  // */
-  // class WriteThroughEntry
-  // extends SimpleEntry<V>
-  // {
-  // WriteThroughEntry(int k, V v) {
-  // super(k,v);
-  // }
-  //
-  // /**
-  // * Set our entry's value and write through to the map. The
-  // * value to return is somewhat arbitrary here. Since a
-  // * WriteThroughEntry does not necessarily track asynchronous
-  // * changes, the most recent "previous" value could be
-  // * different from what we return (or could even have been
-  // * removed in which case the put will re-establish). We do not
-  // * and cannot guarantee more.
-  // */
-  // @Override
-  // public V setValue(V value) {
-  // if (value == null) throw new NullPointerException();
-  // V v = super.setValue(value);
-  // ObjIdConcurrentMap.this.put(getKey(), value);
-  // return v;
-  // }
-  // }
-
-  /*
-   * class EntryIterator extends HashIterator implements Iterator<Entry<V>> { 
public Entry<V> next()
-   * { HashEntry<V> e = super.nextEntry(); return new WriteThroughEntry(e.key, 
e.value); } }
-   *
-   * class KeySet extends AbstractSet<K> { public Iterator<K> iterator() { 
return new KeyIterator();
-   * } public int size() { return ObjIdConcurrentMap.this.size(); } public 
boolean isEmpty() {
-   * return ObjIdConcurrentMap.this.isEmpty(); } public boolean 
contains(Object o) { return
-   * ObjIdConcurrentMap.this.containsKey(o); } public boolean remove(Object o) 
{ return
-   * ObjIdConcurrentMap.this.remove(o) != null; } public void clear() {
-   * ObjIdConcurrentMap.this.clear(); } }
-   *
-   * class Values extends AbstractCollection<V> { public Iterator<V> 
iterator() { return new
-   * ValueIterator(); } public int size() { return 
ObjIdConcurrentMap.this.size(); } public boolean
-   * isEmpty() { return ObjIdConcurrentMap.this.isEmpty(); } public boolean 
contains(Object o) {
-   * return ObjIdConcurrentMap.this.containsValue(o); } public void clear() {
-   * ObjIdConcurrentMap.this.clear(); } }
-   *
-   * class EntrySet extends AbstractSet<Map.Entry<K,V>> { public 
Iterator<Map.Entry<K,V>> iterator()
-   * { return new EntryIterator(); } public boolean contains(Object o) { if 
(!(o instanceof
-   * Map.Entry)) return false; Map.Entry<?,?> e = (Map.Entry<?,?>)o; V v =
-   * ObjIdConcurrentMap.this.get(e.getKey()); return v != null && 
v.equals(e.getValue()); } public
-   * boolean remove(Object o) { if (!(o instanceof Map.Entry)) return false; 
Map.Entry<?,?> e =
-   * (Map.Entry<?,?>)o; return ObjIdConcurrentMap.this.remove(e.getKey(), 
e.getValue()); } public
-   * int size() { return ObjIdConcurrentMap.this.size(); } public boolean 
isEmpty() { return
-   * ObjIdConcurrentMap.this.isEmpty(); } public void clear() { 
ObjIdConcurrentMap.this.clear(); } }
-   */
-
   /* ---------------- Serialization Support -------------- */
 
   /**
diff --git 
a/geode-core/src/main/java/org/apache/geode/internal/cache/GemFireCacheImpl.java
 
b/geode-core/src/main/java/org/apache/geode/internal/cache/GemFireCacheImpl.java
index 0569d82..069adc1 100755
--- 
a/geode-core/src/main/java/org/apache/geode/internal/cache/GemFireCacheImpl.java
+++ 
b/geode-core/src/main/java/org/apache/geode/internal/cache/GemFireCacheImpl.java
@@ -834,7 +834,7 @@ public class GemFireCacheImpl implements InternalCache, 
InternalClientCache, Has
    * Creates a new instance of GemFireCache and populates it according to the 
{@code cache.xml}, if
    * appropriate.
    *
-   * @param typeRegistry: currently only unit tests set this parameter to a 
non-null value
+   * Currently only unit tests set the typeRegistry parameter to a non-null 
value
    */
   private GemFireCacheImpl(boolean isClient, PoolFactory pf, 
InternalDistributedSystem system,
       CacheConfig cacheConfig, boolean asyncEventListeners, TypeRegistry 
typeRegistry) {
diff --git 
a/geode-core/src/main/java/org/apache/geode/internal/cache/PartitionedRegionDataStore.java
 
b/geode-core/src/main/java/org/apache/geode/internal/cache/PartitionedRegionDataStore.java
index 4ef5128..ab644d0 100644
--- 
a/geode-core/src/main/java/org/apache/geode/internal/cache/PartitionedRegionDataStore.java
+++ 
b/geode-core/src/main/java/org/apache/geode/internal/cache/PartitionedRegionDataStore.java
@@ -1138,29 +1138,6 @@ public class PartitionedRegionDataStore implements 
HasCachePerfStats {
   }
 
   /**
-   * Querys the buckets in this data store for query specified by 
queryPredicate.
-   *
-   * @param parameters the parameters to be used to execute the query
-   * @param buckets to be queried
-   * @throws QueryException TODO-javadocs
-   */
-  /*
-   * public void queryLocalNode(DefaultQuery query, Object[] parameters, List 
buckets,
-   * PRQueryResultCollector resultCollector) throws QueryException, 
InterruptedException {
-   * Assert.assertTrue(!buckets.isEmpty(), "bucket list can not be empty. ");
-   * invokeBucketReadHook();
-   *
-   * // Check if QueryMonitor is enabled, if so add query to be monitored. 
QueryMonitor queryMonitor
-   * = null; if( GemFireCacheImpl.getInstance() != null) { queryMonitor =
-   * GemFireCacheImpl.getInstance().getQueryMonitor(); }
-   *
-   * try { if (queryMonitor != null) { // Add current thread to be monitored 
by QueryMonitor.
-   * queryMonitor.monitorQueryThread(Thread.currentThread(), query); } new 
PRQueryProcessor(this,
-   * query, parameters, buckets).executeQuery(resultCollector); } finally { if 
(queryMonitor !=
-   * null) { queryMonitor.stopMonitoringQueryThread(Thread.currentThread(), 
query); } } }
-   */
-
-  /**
    * This method returns name of this Partitioned Region
    *
    * @return Partitioned Region name
diff --git 
a/geode-core/src/main/java/org/apache/geode/internal/cache/map/RegionMapDestroy.java
 
b/geode-core/src/main/java/org/apache/geode/internal/cache/map/RegionMapDestroy.java
index 7e5365e..e4c90d1 100644
--- 
a/geode-core/src/main/java/org/apache/geode/internal/cache/map/RegionMapDestroy.java
+++ 
b/geode-core/src/main/java/org/apache/geode/internal/cache/map/RegionMapDestroy.java
@@ -16,6 +16,7 @@ package org.apache.geode.internal.cache.map;
 
 import org.apache.logging.log4j.Logger;
 
+import org.apache.geode.InternalGemFireError;
 import org.apache.geode.cache.CacheWriterException;
 import org.apache.geode.cache.EntryNotFoundException;
 import org.apache.geode.cache.TimeoutException;
@@ -85,7 +86,7 @@ public class RegionMapDestroy {
       throws CacheWriterException, EntryNotFoundException, TimeoutException {
 
     if (internalRegion == null) {
-      Assert.assertTrue(false, "The internalRegion for RegionMap " + this // 
"fix" for bug 32440
+      throw new InternalGemFireError("The internalRegion for RegionMap " + this
           + " is null for event " + event);
     }
 
@@ -107,7 +108,6 @@ public class RegionMapDestroy {
     cacheModificationLock.lockForCacheModification(internalRegion, event);
     final boolean locked = internalRegion.lockWhenRegionIsInitializing();
     try {
-
       while (retry) {
         retry = false;
         opCompleted = false;
diff --git 
a/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/AcceptorImpl.java
 
b/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/AcceptorImpl.java
index 16c701a..0308e7c 100755
--- 
a/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/AcceptorImpl.java
+++ 
b/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/AcceptorImpl.java
@@ -16,6 +16,7 @@ package org.apache.geode.internal.cache.tier.sockets;
 
 import static 
org.apache.geode.distributed.ConfigurationProperties.SECURITY_CLIENT_ACCESSOR_PP;
 import static 
org.apache.geode.internal.cache.tier.CommunicationMode.ClientToServerForQueue;
+import static 
org.apache.geode.internal.cache.tier.sockets.Handshake.REPLY_REFUSED;
 
 import java.io.DataOutputStream;
 import java.io.EOFException;
@@ -1437,9 +1438,6 @@ public class AcceptorImpl implements Acceptor, Runnable, 
CommBufferPool {
     }
   }
 
-  static final byte REPLY_REFUSED = (byte) 60;
-  static final byte REPLY_INVALID = (byte) 61;
-
   void refuseHandshake(OutputStream out, String message, byte exception) 
throws IOException {
 
     HeapDataOutputStream hdos = new HeapDataOutputStream(32, Version.CURRENT);
diff --git 
a/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/ClientDataSerializerMessage.java
 
b/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/ClientDataSerializerMessage.java
index 1b6a35c..13f8233 100644
--- 
a/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/ClientDataSerializerMessage.java
+++ 
b/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/ClientDataSerializerMessage.java
@@ -222,9 +222,10 @@ public class ClientDataSerializerMessage extends 
ClientUpdateMessageImpl {
 
   @Override
   public String toString() {
-    StringBuffer buffer = new StringBuffer();
-    buffer.append("ClientDataSerializerMessage[").append(";value=")
-        
.append((Arrays.toString(this.serializedDataSerializer))).append(";memberId=")
+    StringBuilder buffer = new StringBuilder();
+    buffer.append("ClientDataSerializerMessage[value=")
+        .append(Arrays.toString(this.serializedDataSerializer))
+        .append(";memberId=")
         
.append(getMembershipId()).append(";eventId=").append(getEventId()).append("]");
     return buffer.toString();
   }
diff --git 
a/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/ClientInstantiatorMessage.java
 
b/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/ClientInstantiatorMessage.java
index fd7bd0b..9399fe2 100755
--- 
a/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/ClientInstantiatorMessage.java
+++ 
b/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/ClientInstantiatorMessage.java
@@ -198,9 +198,10 @@ public class ClientInstantiatorMessage extends 
ClientUpdateMessageImpl {
   @Override
   public String toString() {
     StringBuffer buffer = new StringBuffer();
-    buffer.append("ClientInstantiatorMessage[").append(";value=")
-        
.append((Arrays.toString(this.serializedInstantiators))).append(";memberId=")
-        
.append(getMembershipId()).append(";eventId=").append(getEventId()).append(";notifyAll=")
+    buffer.append("ClientInstantiatorMessage[value=")
+        .append(Arrays.toString(this.serializedInstantiators))
+        .append(";memberId=")
+        .append(getMembershipId()).append(";eventId=").append(getEventId())
         .append("]");
     return buffer.toString();
   }
diff --git 
a/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/ServerConnection.java
 
b/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/ServerConnection.java
index 5c6b523..9fdb341 100644
--- 
a/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/ServerConnection.java
+++ 
b/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/ServerConnection.java
@@ -379,7 +379,7 @@ public abstract class ServerConnection implements Runnable {
           // is this branch ever taken?
           this.crHelper.checkCancelInProgress(null); // bug 37113?
           logger.warn("Received Unknown handshake reply code.");
-          refuseHandshake("Received Unknown handshake reply code.", 
AcceptorImpl.REPLY_INVALID);
+          refuseHandshake("Received Unknown handshake reply code.", 
Handshake.REPLY_INVALID);
           return false;
         }
       }
@@ -393,7 +393,7 @@ public abstract class ServerConnection implements Runnable {
   }
 
   private void handleHandshakeException(Exception ex) {
-    refuseHandshake(ex.getMessage(), AcceptorImpl.REPLY_REFUSED);
+    refuseHandshake(ex.getMessage(), Handshake.REPLY_REFUSED);
     failConnectionAttempt();
   }
 
@@ -896,7 +896,7 @@ public abstract class ServerConnection implements Runnable {
         MutableInt numRefs = getCleanupTable().get(this.handshake);
         if (numRefs != null) {
           numRefs.decrement();
-          if (numRefs.toInteger() <= 0) {
+          if (numRefs.intValue() <= 0) {
             clientDeparted = true;
             getCleanupTable().remove(this.handshake);
             this.stats.decCurrentClients();
@@ -916,7 +916,7 @@ public abstract class ServerConnection implements Runnable {
         MutableInt numRefs = getCleanupProxyIdTable().get(this.proxyId);
         if (numRefs != null) {
           numRefs.decrement();
-          if (numRefs.toInteger() <= 0) {
+          if (numRefs.intValue() <= 0) {
             unregisterClient = true;
             getCleanupProxyIdTable().remove(this.proxyId);
             // here we can remove entry multiuser map for client
diff --git 
a/geode-core/src/main/java/org/apache/geode/internal/datasource/AbstractPoolCache.java
 
b/geode-core/src/main/java/org/apache/geode/internal/datasource/AbstractPoolCache.java
index 8ab5a7a..24c5069 100644
--- 
a/geode-core/src/main/java/org/apache/geode/internal/datasource/AbstractPoolCache.java
+++ 
b/geode-core/src/main/java/org/apache/geode/internal/datasource/AbstractPoolCache.java
@@ -107,17 +107,6 @@ public abstract class AbstractPoolCache implements 
ConnectionPoolCache, Serializ
     }
   }
 
-  /**
-   * Authenticates the username and password for the database connection.
-   *
-   * @param user The username for the database connection
-   * @param pass The password for the database connection
-   *
-   *        public abstract void checkCredentials(String user, String pass)
-   */
-  /**
-   * @return ???
-   */
   public abstract Object getNewPoolConnection() throws PoolException;
 
   /**
diff --git 
a/geode-core/src/main/java/org/apache/geode/internal/datasource/GemFireConnPooledDataSource.java
 
b/geode-core/src/main/java/org/apache/geode/internal/datasource/GemFireConnPooledDataSource.java
index 523fc74..57fb648 100644
--- 
a/geode-core/src/main/java/org/apache/geode/internal/datasource/GemFireConnPooledDataSource.java
+++ 
b/geode-core/src/main/java/org/apache/geode/internal/datasource/GemFireConnPooledDataSource.java
@@ -41,14 +41,6 @@ public class GemFireConnPooledDataSource extends 
AbstractDataSource
   protected ConnectionProvider provider;
 
   /**
-   * Creates a new instance of GemFireConnPooledDataSource
-   *
-   * @param connPoolDS The ConnectionPoolDataSource object for the database 
driver.
-   * @param configs The ConfiguredDataSourceProperties containing the 
datasource properties.
-   */
-
-
-  /**
    * Place holder for abstract method isWrapperFor(java.lang.Class) in 
java.sql.Wrapper required by
    * jdk 1.6
    *
@@ -64,6 +56,12 @@ public class GemFireConnPooledDataSource extends 
AbstractDataSource
 
 
 
+  /**
+   * Creates a new instance of GemFireConnPooledDataSource
+   *
+   * @param connPoolDS The ConnectionPoolDataSource object for the database 
driver.
+   * @param configs The ConfiguredDataSourceProperties containing the 
datasource properties.
+   */
   public GemFireConnPooledDataSource(ConnectionPoolDataSource connPoolDS,
       ConfiguredDataSourceProperties configs) throws SQLException {
     super(configs);
diff --git 
a/geode-core/src/main/java/org/apache/geode/internal/process/StartupStatus.java 
b/geode-core/src/main/java/org/apache/geode/internal/process/StartupStatus.java
index 23ca291..00190ce 100644
--- 
a/geode-core/src/main/java/org/apache/geode/internal/process/StartupStatus.java
+++ 
b/geode-core/src/main/java/org/apache/geode/internal/process/StartupStatus.java
@@ -41,9 +41,9 @@ public class StartupStatus {
    */
   public static synchronized void startup(final String msg, final Object... 
params) {
     notNull(msg, "Invalid msgId '" + msg + "' specified");
-    notNull(params, "Invalid params '" + params + "' specified");
+    notNull(params, "Invalid params specified");
 
-    String message = (params == null) ? msg : String.format(msg, params);
+    String message = String.format(msg, params);
 
     if (listener != null) {
       listener.setStatus(message);
diff --git 
a/geode-core/src/main/java/org/apache/geode/management/internal/cli/i18n/CliStrings.java
 
b/geode-core/src/main/java/org/apache/geode/management/internal/cli/i18n/CliStrings.java
index 7c40805..0ec8230 100644
--- 
a/geode-core/src/main/java/org/apache/geode/management/internal/cli/i18n/CliStrings.java
+++ 
b/geode-core/src/main/java/org/apache/geode/management/internal/cli/i18n/CliStrings.java
@@ -2597,7 +2597,7 @@ public class CliStrings {
 
   public static final String START_SERVER__EVICTION__HEAP__PERCENTAGE = 
"eviction-heap-percentage";
   public static final String START_SERVER__EVICTION__HEAP__PERCENTAGE__HELP =
-      "Set the percentage of heap at or above which the eviction should begin 
on Regions configured for HeapLRU eviction. Changing this value may cause 
eviction to begin immediately."
+      "Set the percentage of heap at or above which the eviction should begin 
on Regions configured for HeapLRU eviction. Changing this value may cause 
eviction to begin immediately. "
           + "Only one change to this attribute or critical heap percentage 
will be allowed at any given time and its effect will be fully realized before 
the next change is allowed. This feature requires additional VM flags to 
perform properly. ";
 
   public static final String START_SERVER__CRITICAL_OFF_HEAP_PERCENTAGE =
@@ -3107,7 +3107,7 @@ public class CliStrings {
       "Set to true to have PDX deserialization produce a PdxInstance instead 
of an instance of the domain class. The default value for this options is 
\"false\"";
   public static final String CONFIGURE_PDX__IGNORE__UNREAD_FIELDS = 
"ignore-unread-fields";
   public static final String CONFIGURE_PDX__IGNORE__UNREAD_FIELDS__HELP =
-      "Control whether pdx ignores fields that were unread during 
deserialization. The default is to preserve unread fields be including their 
data during serialization. But if you configure the cache to ignore unread 
fields then their data will be lost during serialization."
+      "Control whether pdx ignores fields that were unread during 
deserialization. The default is to preserve unread fields be including their 
data during serialization. But if you configure the cache to ignore unread 
fields then their data will be lost during serialization. "
           + "You should only set this attribute to true if you know this 
member will only be reading cache data. In this use case you do not need to pay 
the cost of preserving the unread fields since you will never be reserializing 
pdx data.";
   public static final String CONFIGURE_PDX__PERSISTENT = "persistent";
   public static final String CONFIGURE_PDX__PERSISTENT__HELP =
diff --git 
a/geode-dunit/src/main/java/org/apache/geode/internal/cache/execute/data/Order.java
 
b/geode-dunit/src/main/java/org/apache/geode/internal/cache/execute/data/Order.java
index d23f0e2..e23c8b7 100644
--- 
a/geode-dunit/src/main/java/org/apache/geode/internal/cache/execute/data/Order.java
+++ 
b/geode-dunit/src/main/java/org/apache/geode/internal/cache/execute/data/Order.java
@@ -57,4 +57,12 @@ public class Order implements DataSerializable {
     }
     return false;
   }
+
+  @Override
+  public int hashCode() {
+    if (orderName == null) {
+      return super.hashCode();
+    }
+    return orderName.hashCode();
+  }
 }
diff --git 
a/geode-dunit/src/main/java/org/apache/geode/internal/cache/execute/data/Shipment.java
 
b/geode-dunit/src/main/java/org/apache/geode/internal/cache/execute/data/Shipment.java
index 25ff4c9..c546a52 100644
--- 
a/geode-dunit/src/main/java/org/apache/geode/internal/cache/execute/data/Shipment.java
+++ 
b/geode-dunit/src/main/java/org/apache/geode/internal/cache/execute/data/Shipment.java
@@ -57,4 +57,14 @@ public class Shipment implements DataSerializable {
     }
     return false;
   }
+
+
+  @Override
+  public int hashCode() {
+    if (shipmentName == null) {
+      return super.hashCode();
+    }
+    return shipmentName.hashCode();
+  }
+
 }
diff --git 
a/geode-pulse/src/main/java/org/apache/geode/tools/pulse/internal/data/JMXDataUpdater.java
 
b/geode-pulse/src/main/java/org/apache/geode/tools/pulse/internal/data/JMXDataUpdater.java
index fa5d9d5..a5cd770 100644
--- 
a/geode-pulse/src/main/java/org/apache/geode/tools/pulse/internal/data/JMXDataUpdater.java
+++ 
b/geode-pulse/src/main/java/org/apache/geode/tools/pulse/internal/data/JMXDataUpdater.java
@@ -887,7 +887,7 @@ public class JMXDataUpdater implements IClusterUpdater, 
NotificationListener {
   /**
    * Add member specific region information on the region
    *
-   * @param regionObjectName: used to construct the jmx objectname. For region 
name that has special
+   * @param regionObjectName used to construct the jmx objectname. For region 
name that has special
    *        characters in, it will have double quotes around it.
    */
   private void updateRegionOnMembers(String regionObjectName, String 
regionFullPath,
diff --git 
a/geode-web-api/src/integrationTest/java/org/apache/geode/rest/internal/web/controllers/AddFreeItemToOrders.java
 
b/geode-web-api/src/integrationTest/java/org/apache/geode/rest/internal/web/controllers/AddFreeItemToOrders.java
index 2e08343..c47a227 100644
--- 
a/geode-web-api/src/integrationTest/java/org/apache/geode/rest/internal/web/controllers/AddFreeItemToOrders.java
+++ 
b/geode-web-api/src/integrationTest/java/org/apache/geode/rest/internal/web/controllers/AddFreeItemToOrders.java
@@ -68,6 +68,7 @@ public class AddFreeItemToOrders implements Function {
     } catch (CacheClosedException ex) {
       vals.add("NoCacheFoundResult");
       context.getResultSender().lastResult(vals);
+      throw ex;
     }
 
     String oql =
@@ -80,33 +81,27 @@ public class AddFreeItemToOrders implements Function {
     SelectResults result = null;
     try {
       result = (SelectResults) query.execute(queryArgs);
-      int resultSize = result.size();
 
       if (result instanceof Collection<?>)
         for (Object item : result) {
           keys.add(item);
         }
     } catch (FunctionDomainException e) {
-      if (cache != null)
-        cache.getLogger()
-            .info("Caught FunctionDomainException while executing function 
AddFreeItemToOrders: "
-                + e.getMessage());
-
+      cache.getLogger()
+          .info("Caught FunctionDomainException while executing function 
AddFreeItemToOrders: "
+              + e.getMessage());
     } catch (TypeMismatchException e) {
-      if (cache != null)
-        cache.getLogger()
-            .info("Caught TypeMismatchException while executing function 
AddFreeItemToOrders: "
-                + e.getMessage());
+      cache.getLogger()
+          .info("Caught TypeMismatchException while executing function 
AddFreeItemToOrders: "
+              + e.getMessage());
     } catch (NameResolutionException e) {
-      if (cache != null)
-        cache.getLogger()
-            .info("Caught NameResolutionException while executing function 
AddFreeItemToOrders: "
-                + e.getMessage());
+      cache.getLogger()
+          .info("Caught NameResolutionException while executing function 
AddFreeItemToOrders: "
+              + e.getMessage());
     } catch (QueryInvocationTargetException e) {
-      if (cache != null)
-        cache.getLogger().info(
-            "Caught QueryInvocationTargetException while executing function 
AddFreeItemToOrders"
-                + e.getMessage());
+      cache.getLogger().info(
+          "Caught QueryInvocationTargetException while executing function 
AddFreeItemToOrders"
+              + e.getMessage());
     }
 
     // class has to be in classpath.

Reply via email to