Implementation of equals and hashCode on IdentityContentCodec (#…

…1525)

Motivation:

[Issue-1455](#1455)
It is pretty trivial for a user to specify their own codec with a similar name, therefore it would be better to support equals/hashCode to allow for better equality checks.

Modifications:

- Implemented `equals` and `hashCode` on `IdentityContent`;
- Changed all equality checks that were done using == and != to `equals`;
- Add tests to verify new behavior;

Result:

`IdentityContentCodec` now implements `equals` and `hashCode`,
and all equality checks are done using `equals()`.

servicetalk-encoding-api-internal/src/main/java/io/servicetalk/encoding/api/internal/HeaderUtils.java

@@ -89,7 +89,7 @@ public static ContentCodec negotiateAcceptedEncoding(final List<ContentCodec> cl
         }
 
         for (ContentCodec encoding : serverSupportedEncodings) {
-            if (encoding != identity() && clientSupportedEncodings.contains(encoding)) {
+            if (!identity().equals(encoding) && clientSupportedEncodings.contains(encoding)) {
                 return encoding;
             }
         }

servicetalk-encoding-api/build.gradle

@@ -1,5 +1,5 @@
 /*
- * Copyright © 2020 Apple Inc. and the ServiceTalk project authors
+ * Copyright © 2020-2021 Apple Inc. and the ServiceTalk project authors
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -24,4 +24,10 @@ dependencies {
     implementation project(":servicetalk-concurrent-internal")
     implementation "com.google.code.findbugs:jsr305:$jsr305Version"
     implementation "org.slf4j:slf4j-api:$slf4jVersion"
+
+    testImplementation "org.junit.jupiter:junit-jupiter-api:$junit5Version"
+    testImplementation "org.junit.jupiter:junit-jupiter-params:$junit5Version"
+    testImplementation "org.hamcrest:hamcrest-library:$hamcrestVersion"
+
+    testRuntimeOnly "org.junit.jupiter:junit-jupiter-engine:$junit5Version"
 }

servicetalk-encoding-api/src/main/java/io/servicetalk/encoding/api/AbstractContentCodec.java

@@ -16,7 +16,7 @@
 package io.servicetalk.encoding.api;
 
 import static io.servicetalk.buffer.api.CharSequences.caseInsensitiveHashCode;
-import static io.servicetalk.buffer.api.CharSequences.contentEquals;
+import static io.servicetalk.buffer.api.CharSequences.contentEqualsIgnoreCase;
 
 @Deprecated
 abstract class AbstractContentCodec implements ContentCodec {
@@ -40,7 +40,7 @@ public boolean equals(Object o) {
             return false;
         }
         final AbstractContentCodec that = (AbstractContentCodec) o;
-        return contentEquals(name, that.name);
+        return contentEqualsIgnoreCase(name, that.name);
     }
 
     @Override

servicetalk-encoding-api/src/main/java/io/servicetalk/encoding/api/IdentityContentCodec.java

@@ -19,6 +19,8 @@
 import io.servicetalk.buffer.api.BufferAllocator;
 import io.servicetalk.concurrent.api.Publisher;
 
+import static io.servicetalk.buffer.api.CharSequences.caseInsensitiveHashCode;
+import static io.servicetalk.buffer.api.CharSequences.contentEqualsIgnoreCase;
 import static io.servicetalk.buffer.api.CharSequences.newAsciiString;
 
 /**
@@ -27,6 +29,7 @@
 final class IdentityContentCodec implements ContentCodec {
 
     private static final CharSequence NAME = newAsciiString("identity");
+    private static final int HASH_CODE = caseInsensitiveHashCode(NAME);
 
     @Override
     public CharSequence name() {
@@ -62,4 +65,23 @@ public Publisher<Buffer> encode(final Publisher<Buffer> from, final BufferAlloca
     public Publisher<Buffer> decode(final Publisher<Buffer> from, final BufferAllocator allocator) {
         return from;
     }
+
+    @Override
+    public boolean equals(Object o) {
+        if (this == o) {
+            return true;
+        }
+
+        if (!(o instanceof ContentCodec)) {
+            return false;
+        }
+
+        final ContentCodec that = (ContentCodec) o;
+        return contentEqualsIgnoreCase(name(), that.name());
+    }
+
+    @Override
+    public int hashCode() {
+        return HASH_CODE;
+    }
 }

servicetalk-encoding-api/src/test/java/io/servicetalk/encoding/api/CustomIdentityContentCodec.java

@@ -0,0 +1,29 @@
+/*
+ * Copyright © 2021 Apple Inc. and the ServiceTalk project authors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package io.servicetalk.encoding.api;
+
+/**
+ * Extension of {@link NoopContentCodec} with custom implementations for {@link #equals(Object)} and {@link #hashCode()}
+ * based on {@link #name()}.
+ * This class can be used as is or can be extended to create a {@link ContentCodec} with custom behaviour.
+ */
+class CustomIdentityContentCodec extends NoopContentCodec {
+
+    CustomIdentityContentCodec() {
+        super(Identity.identity().name());
+    }
+}

servicetalk-encoding-api/src/test/java/io/servicetalk/encoding/api/IdentityContentCodecTest.java

@@ -0,0 +1,73 @@
+/*
+ * Copyright © 2021 Apple Inc. and the ServiceTalk project authors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package io.servicetalk.encoding.api;
+
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
+
+import java.util.stream.Stream;
+
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.is;
+import static org.hamcrest.Matchers.not;
+
+class IdentityContentCodecTest {
+
+    private static final IdentityContentCodec IDENTITY_CODEC = new IdentityContentCodec();
+
+    private static Stream<Arguments> equalsParams() {
+        return Stream.of(
+                Arguments.of(IDENTITY_CODEC, new IdentityContentCodec()),
+                Arguments.of(IDENTITY_CODEC, new CustomIdentityContentCodec()),
+                Arguments.of(IDENTITY_CODEC, new CustomIdentityContentCodec() {
+                    @Override
+                    public int hashCode() {
+                        return IDENTITY_CODEC.hashCode() + 1;
+                    }
+
+                    @SuppressWarnings("RedundantMethodOverride")
+                    @Override
+                    public boolean equals(Object other) {
+                        return super.equals(other);
+                    }
+                }),
+                Arguments.of(IDENTITY_CODEC, new NoopContentCodec(IDENTITY_CODEC.name().toString().toUpperCase()))
+        );
+    }
+
+    private static Stream<Arguments> notEqualsParams() {
+        return Stream.of(
+                Arguments.of(IDENTITY_CODEC, new Object()),
+                Arguments.of(IDENTITY_CODEC, new NoopContentCodec(IDENTITY_CODEC.name() + "different")),
+                Arguments.of(IDENTITY_CODEC, null)
+        );
+    }
+
+    @ParameterizedTest(name = "{displayName} [{index}] {arguments}")
+    @MethodSource("equalsParams")
+    void equals(IdentityContentCodec identity, Object other) {
+        assertThat(identity, is(equalTo(other)));
+    }
+
+    @ParameterizedTest(name = "{displayName} [{index}] {arguments}")
+    @MethodSource("notEqualsParams")
+    void notEquals(IdentityContentCodec identity, Object other) {
+        assertThat(identity, is(not(equalTo(other))));
+    }
+}

servicetalk-encoding-api/src/test/java/io/servicetalk/encoding/api/NoopContentCodec.java

@@ -0,0 +1,68 @@
+/*
+ * Copyright © 2021 Apple Inc. and the ServiceTalk project authors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package io.servicetalk.encoding.api;
+
+import io.servicetalk.buffer.api.Buffer;
+import io.servicetalk.buffer.api.BufferAllocator;
+import io.servicetalk.concurrent.api.Publisher;
+
+/**
+ * Implementation of {@link ContentCodec} that doesn't modify the source {@link Buffer}.
+ */
+class NoopContentCodec implements ContentCodec {
+
+    private final CharSequence name;
+
+    NoopContentCodec(CharSequence name) {
+        this.name = name;
+    }
+
+    @Override
+    public CharSequence name() {
+        return name;
+    }
+
+    @Override
+    public Buffer encode(Buffer src, BufferAllocator allocator) {
+        return src;
+    }
+
+    @Override
+    public Buffer encode(Buffer src, int offset, int length, BufferAllocator allocator) {
+        return src;
+    }
+
+    @Override
+    public Buffer decode(Buffer src, BufferAllocator allocator) {
+        return src;
+    }
+
+    @Override
+    public Buffer decode(Buffer src, int offset, int length, BufferAllocator allocator) {
+        return src;
+    }
+
+    @Override
+    public Publisher<Buffer> encode(Publisher<Buffer> from, BufferAllocator allocator) {
+        return from;
+    }
+
+    @Override
+    public Publisher<Buffer> decode(Publisher<Buffer> from, BufferAllocator allocator) {
+        return from;
+    }
+}

servicetalk-encoding-netty/src/main/java/io/servicetalk/encoding/netty/AbstractContentCodec.java

@@ -18,7 +18,7 @@
 import io.servicetalk.encoding.api.ContentCodec;
 
 import static io.servicetalk.buffer.api.CharSequences.caseInsensitiveHashCode;
-import static io.servicetalk.buffer.api.CharSequences.contentEquals;
+import static io.servicetalk.buffer.api.CharSequences.contentEqualsIgnoreCase;
 
 abstract class AbstractContentCodec implements ContentCodec {
 
@@ -47,7 +47,7 @@ public final boolean equals(final Object o) {
         }
 
         final AbstractContentCodec that = (AbstractContentCodec) o;
-        return contentEquals(name, that.name);
+        return contentEqualsIgnoreCase(name, that.name);
     }
 
     @Override

servicetalk-grpc-api/src/main/java/io/servicetalk/grpc/api/GrpcUtils.java

@@ -404,7 +404,7 @@ private static CharSequence acceptedEncodingsHeaderValueOrCached(final List<Cont
     private static CharSequence acceptedEncodingsHeaderValue0(final List<ContentCodec> codings) {
         StringBuilder builder = new StringBuilder(codings.size() * (12 + CONTENT_ENCODING_SEPARATOR.length()));
         for (ContentCodec codec : codings) {
-            if (codec == identity()) {
+            if (identity().equals(codec)) {
                 continue;
             }
             builder.append(codec.name()).append(CONTENT_ENCODING_SEPARATOR);

servicetalk-grpc-netty/src/test/java/io/servicetalk/grpc/netty/GrpcMessageEncodingTest.java

@@ -200,15 +200,15 @@ public Single<StreamingHttpResponse> handle(final HttpServiceContext ctx,
                                     assertEquals(GZIP_MAGIC, actualHeader);
                                 }
 
-                                if (reqEncoding != identity()) {
+                                if (!identity().equals(reqEncoding)) {
                                     assertTrue("Compressed content length should be less than the " +
                                                     "original payload size", buffer.readableBytes() < PAYLOAD_SIZE);
                                 } else {
                                     assertTrue("Uncompressed content length should be more than the " +
                                                     "original payload size", buffer.readableBytes() > PAYLOAD_SIZE);
                                 }
 
-                                assertEquals(reqEncoding != identity() ? 1 : 0, compressedFlag);
+                                assertEquals(!identity().equals(reqEncoding) ? 1 : 0, compressedFlag);
                             } catch (Throwable t) {
                                 errors.add(t);
                                 throw t;
@@ -217,7 +217,7 @@ public Single<StreamingHttpResponse> handle(final HttpServiceContext ctx,
                         })));
 
                         assertValidCodingHeader(clientSupportedEncodings, request.headers());
-                        if (reqEncoding == identity()) {
+                        if (identity().equals(reqEncoding)) {
                             assertThat("Message-Encoding should NOT be present in the headers if identity",
                                     request.headers().contains(MESSAGE_ENCODING), is(false));
                         } else {
@@ -260,7 +260,7 @@ private void handle0(final List<ContentCodec> clientSupportedEncodings,
                             }
                         }
 
-                        if (expected == identity()) {
+                        if (identity().equals(expected)) {
                             assertThat("Response shouldn't contain Message-Encoding header if identity",
                                     response.headers().contains(MESSAGE_ENCODING), is(false));
                         } else {
@@ -494,7 +494,7 @@ private static void assertValidCodingHeader(final List<ContentCodec> supportedEn
     @Nonnull
     private static List<String> encodingsAsStrings(final List<ContentCodec> supportedEncodings) {
         return supportedEncodings.stream()
-                .filter(enc -> enc != identity())
+                .filter(enc -> !identity().equals(enc))
                 .map(ContentCodec::name)
                 .map(CharSequence::toString)
                 .collect(toList());

servicetalk-grpc-protobuf/src/main/java/io/servicetalk/grpc/protobuf/ProtoBufSerializationProvider.java

@@ -1,5 +1,5 @@
 /*
- * Copyright © 2019-2020 Apple Inc. and the ServiceTalk project authors
+ * Copyright © 2019-2021 Apple Inc. and the ServiceTalk project authors
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -275,7 +275,7 @@ private static final class ProtoSerializer implements StreamingSerializer {
 
         ProtoSerializer(final ContentCodec codec) {
             this.codec = codec;
-            this.encode = codec != identity();
+            this.encode = !identity().equals(codec);
         }
 
         @Override

servicetalk-grpc-protobuf/src/main/java/io/servicetalk/grpc/protobuf/ProtoBufSerializationProviderBuilder.java

@@ -258,7 +258,7 @@ public void flush() throws IOException {
 
         private void addContentHeaders(final HttpHeaders headers) {
             headers.set(CONTENT_TYPE, APPLICATION_GRPC_PROTO);
-            if (codec != identity()) {
+            if (!identity().equals(codec)) {
                 headers.set(GRPC_MESSAGE_ENCODING_KEY, codec.name());
             }
         }

servicetalk-http-api/src/main/java/io/servicetalk/http/api/ContentCodingHttpRequesterFilter.java

@@ -115,7 +115,7 @@ private Single<StreamingHttpResponse> decodePayloadContentIfEncoded(
     private static CharSequence buildAcceptEncodingsHeader(final List<ContentCodec> codecs) {
         StringBuilder builder = new StringBuilder();
         for (ContentCodec enc : codecs) {
-            if (enc == identity()) {
+            if (identity().equals(enc)) {
                 continue;
             }
 
@@ -132,7 +132,7 @@ private static CharSequence buildAcceptEncodingsHeader(final List<ContentCodec>
     private static void encodePayloadContentIfAvailable(final StreamingHttpRequest request,
                                                         final BufferAllocator allocator) {
         ContentCodec coding = request.encoding();
-        if (coding != null && !coding.equals(identity())) {
+        if (coding != null && !identity().equals(coding)) {
             setContentEncoding(request.headers(), coding.name());
             request.transformPayloadBody(pub -> coding.encode(pub, allocator));
         }

servicetalk-http-api/src/main/java/io/servicetalk/http/api/ContentCodingHttpServiceFilter.java

@@ -1,5 +1,5 @@
 /*
- * Copyright © 2020 Apple Inc. and the ServiceTalk project authors
+ * Copyright © 2020-2021 Apple Inc. and the ServiceTalk project authors
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -183,6 +183,6 @@ private static ContentCodec codingForResponse(final HttpHeaders requestHeaders,
             encoding = negotiateAcceptedEncoding(requestHeaders.get(ACCEPT_ENCODING), supportedEncodings);
         }
 
-        return encoding == identity() ? null : encoding;
+        return identity().equals(encoding) ? null : encoding;
     }
 }

servicetalk-http-api/src/main/java/io/servicetalk/http/api/HeaderUtils.java

@@ -686,7 +686,7 @@ static ContentCodec identifyContentEncodingOrNullIfIdentity(
             throw new UnsupportedContentEncodingException(encoding.toString());
         }
 
-        return enc == identity() ? null : enc;
+        return identity().equals(enc) ? null : enc;
     }
 
     /**