Address the comments from @anuraaga and @ikhoon

line · trustin · Dec 20, 2019 · Dec 10, 2019 · Dec 11, 2019 · Dec 11, 2019
commit c3ea71c9550bf17eac9efd5c86ba99919f57436f
diff --git a/core/src/main/java/com/linecorp/armeria/common/HttpHeaderGetters.java b/core/src/main/java/com/linecorp/armeria/common/HttpHeaderGetters.java
@@ -15,6 +15,8 @@
  */
 package com.linecorp.armeria.common;
 
+import static java.util.Objects.requireNonNull;
+
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map.Entry;
@@ -334,6 +336,7 @@ default Stream<Entry<AsciiString, String>> stream() {
      */
     @Override
     default Stream<String> valueStream(CharSequence name) {
+        requireNonNull(name, "name");
         return Streams.stream(valueIterator(name));
     }
 }
diff --git a/core/src/main/java/com/linecorp/armeria/common/HttpHeaders.java b/core/src/main/java/com/linecorp/armeria/common/HttpHeaders.java
@@ -54,7 +54,7 @@
  * a new {@link HttpHeaders} derived from an existing one:</p>
  *
  * <pre>{@code
- * HttpHeaders headers = HttpHeaders.of("name1", "value0")
+ * HttpHeaders headers = HttpHeaders.of("name1", "value0");
  *
  * // Using toBuilder()
  * HttpHeaders headersWithToBuilder = headers.toBuilder()

diff --git a/core/src/main/java/com/linecorp/armeria/common/HttpHeadersBase.java b/core/src/main/java/com/linecorp/armeria/common/HttpHeadersBase.java
@@ -99,8 +99,8 @@ int hashName(CharSequence name) {
     }
 
     @Override
-    boolean nameEquals(@Nullable AsciiString a, @Nullable CharSequence b) {
-        return a != null && (a == b || a.contentEqualsIgnoreCase(b));
+    boolean nameEquals(AsciiString a, CharSequence b) {
+        return a.contentEqualsIgnoreCase(b);
     }
 
     @Override

diff --git a/core/src/main/java/com/linecorp/armeria/common/QueryParamGetters.java b/core/src/main/java/com/linecorp/armeria/common/QueryParamGetters.java
@@ -319,6 +319,7 @@ default Stream<Entry<String, String>> stream() {
      */
     @Override
     default Stream<String> valueStream(String name) {
+        requireNonNull(name, "name");
         return Streams.stream(valueIterator(name));
     }
 

diff --git a/core/src/main/java/com/linecorp/armeria/common/QueryParams.java b/core/src/main/java/com/linecorp/armeria/common/QueryParams.java
@@ -52,7 +52,7 @@
  * a new {@link QueryParams} derived from an existing one:</p>
  *
  * <pre>{@code
- * QueryParams params = QueryParams.of("name1", "value0")
+ * QueryParams params = QueryParams.of("name1", "value0");
  *
  * // Using toBuilder()
  * QueryParams paramsWithToBuilder = params.toBuilder()

diff --git a/core/src/main/java/com/linecorp/armeria/common/QueryParamsBase.java b/core/src/main/java/com/linecorp/armeria/common/QueryParamsBase.java
@@ -29,8 +29,6 @@
  */
 package com.linecorp.armeria.common;
 
-import java.util.Objects;
-
 import javax.annotation.Nullable;
 
 /**
@@ -56,8 +54,8 @@ int hashName(String s) {
     }
 
     @Override
-    boolean nameEquals(@Nullable String a, @Nullable String b) {
-        return Objects.equals(a, b);
+    boolean nameEquals(String a, String b) {
+        return a.equals(b);
     }
 
     @Override

diff --git a/core/src/main/java/com/linecorp/armeria/common/StringMultimap.java b/core/src/main/java/com/linecorp/armeria/common/StringMultimap.java
@@ -130,7 +130,7 @@ abstract class StringMultimap<IN_NAME extends CharSequence, NAME extends IN_NAME
 
     abstract int hashName(IN_NAME name);
 
-    abstract boolean nameEquals(@Nullable NAME a, @Nullable IN_NAME b);
+    abstract boolean nameEquals(NAME a, IN_NAME b);
 
     abstract boolean isFirstGroup(NAME name);
 
@@ -150,8 +150,11 @@ public final String get(IN_NAME name) {
         String value = null;
         // loop until the first entry was found
         while (e != null) {
-            if (e.hash == h && nameEquals(e.key, name)) {
-                value = e.value;
+            if (e.hash == h) {
+                final NAME currentName = e.key;
+                if (currentName != null && nameEquals(currentName, name)) {
+                    value = e.value;
+                }
             }
             e = e.next;
         }
@@ -182,8 +185,11 @@ private ImmutableList<String> getAllReversed(IN_NAME name) {
 
         final ImmutableList.Builder<String> builder = ImmutableList.builder();
         do {
-            if (e.hash == h && nameEquals(e.key, name)) {
-                builder.add(e.getValue());
+            if (e.hash == h) {
+                final NAME currentName = e.key;
+                if (currentName != null && nameEquals(currentName, name)) {
+                    builder.add(e.getValue());
+                }
             }
             e = e.next;
         } while (e != null);
@@ -263,8 +269,11 @@ public final boolean contains(IN_NAME name) {
         Entry e = entries[i];
         // loop until the first entry was found
         while (e != null) {
-            if (e.hash == h && nameEquals(e.key, name)) {
-                return true;
+            if (e.hash == h) {
+                final NAME currentName = e.key;
+                if (currentName != null && nameEquals(currentName, name)) {
+                    return true;
+                }
             }
             e = e.next;
         }
@@ -279,9 +288,12 @@ public final boolean contains(IN_NAME name, String value) {
         final int i = index(h);
         Entry e = entries[i];
         while (e != null) {
-            if (e.hash == h && nameEquals(e.key, name) &&
-                AsciiString.contentEquals(e.value, value)) {
-                return true;
+            if (e.hash == h) {
+                final NAME currentName = e.key;
+                if (currentName != null && nameEquals(currentName, name) &&
+                    AsciiString.contentEquals(e.value, value)) {
+                    return true;
+                }
             }
             e = e.next;
         }
@@ -785,11 +797,16 @@ private String remove0(int h, int i, IN_NAME name) {
         String value = null;
         Entry next = e.next;
         while (next != null) {
-            if (next.hash == h && nameEquals(next.key, name)) {
-                value = next.value;
-                e.next = next.next;
-                next.remove();
-                --size;
+            if (next.hash == h) {
+                final NAME currentName = next.key;
+                if (currentName != null && nameEquals(currentName, name)) {
+                    value = next.value;
+                    e.next = next.next;
+                    next.remove();
+                    --size;
+                } else {
+                    e = next;
+                }
             } else {
                 e = next;
             }
@@ -798,13 +815,16 @@ private String remove0(int h, int i, IN_NAME name) {
         }
 
         e = entries[i];
-        if (e.hash == h && nameEquals(e.key, name)) {
-            if (value == null) {
-                value = e.value;
+        if (e.hash == h) {
+            final NAME currentName = e.key;
+            if (currentName != null && nameEquals(currentName, name)) {
+                if (value == null) {
+                    value = e.value;
+                }
+                entries[i] = e.next;
+                e.remove();
+                --size;
             }
-            entries[i] = e.next;
-            e.remove();
-            --size;
         }
 
         return value;
@@ -1077,8 +1097,16 @@ public boolean equals(@Nullable Object o) {
 
             @SuppressWarnings("unchecked")
             final Map.Entry<IN_NAME, String> that = (Map.Entry<IN_NAME, String>) o;
+            final NAME thisKey = key;
             final IN_NAME thatKey = that.getKey();
-            return nameEquals(key, thatKey) && Objects.equals(value, that.getValue());
+            if (thisKey == null) {
+                return thatKey == null &&
+                       Objects.equals(value, that.getValue());
+            } else {
+                return thatKey != null &&
+                       nameEquals(thisKey, thatKey) &&
+                       Objects.equals(value, that.getValue());
+            }
         }
 
         @Override

diff --git a/core/src/main/java/com/linecorp/armeria/common/StringMultimapGetters.java b/core/src/main/java/com/linecorp/armeria/common/StringMultimapGetters.java
@@ -15,6 +15,8 @@
  */
 package com.linecorp.armeria.common;
 
+import static java.util.Objects.requireNonNull;
+
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map.Entry;
@@ -96,11 +98,7 @@ interface StringMultimapGetters<IN_NAME extends CharSequence, NAME extends IN_NA
 
     void forEachValue(IN_NAME name, Consumer<String> action);
 
-    default Stream<Entry<NAME, String>> stream() {
-        return Streams.stream(iterator());
-    }
+    Stream<Entry<NAME, String>> stream();
 
-    default Stream<String> valueStream(IN_NAME name) {
-        return Streams.stream(valueIterator(name));
-    }
+    Stream<String> valueStream(IN_NAME name);
 }