Update use of md5 and xxhash64 in more places

Set md5 and xxhash64 only when non-null in request and response.
Update hasEqualConfig methods.
Always return xxhash64 in response.

6 files changed, 54 insertions, 27 deletions

diff --git a/config/src/main/java/com/yahoo/vespa/config/PayloadChecksums.java b/config/src/main/java/com/yahoo/vespa/config/PayloadChecksums.java
index d30e5b055bc..be935043dc0 100644
--- a/config/src/main/java/com/yahoo/vespa/config/PayloadChecksums.java
+++ b/config/src/main/java/com/yahoo/vespa/config/PayloadChecksums.java
@@ -24,14 +24,20 @@ public class PayloadChecksums {
     private final Map<PayloadChecksum.Type, PayloadChecksum> checksums = new LinkedHashMap<>();
 
     private PayloadChecksums() {
-        Arrays.stream(PayloadChecksum.Type.values()).forEach(type -> checksums.put(type, PayloadChecksum.empty(type)));
+        this(false);
     }
 
-    public static PayloadChecksums empty() { return new PayloadChecksums(); }
+    private PayloadChecksums(boolean addEmptyChecksumForAllTypes) {
+        if (addEmptyChecksumForAllTypes)
+            Arrays.stream(PayloadChecksum.Type.values())
+                  .forEach(type -> checksums.put(type, PayloadChecksum.empty(type)));
+    }
+
+    public static PayloadChecksums empty() { return new PayloadChecksums(true); }
 
     public static PayloadChecksums from(PayloadChecksum... checksums) {
         PayloadChecksums payloadChecksums = new PayloadChecksums();
-        Arrays.stream(checksums).forEach(payloadChecksums::add);
+        Arrays.stream(checksums).filter(Objects::nonNull).forEach(payloadChecksums::add);
         return payloadChecksums;
     }
 
diff --git a/config/src/main/java/com/yahoo/vespa/config/RawConfig.java b/config/src/main/java/com/yahoo/vespa/config/RawConfig.java
index 78c3fefc936..30e169e2597 100755
--- a/config/src/main/java/com/yahoo/vespa/config/RawConfig.java
+++ b/config/src/main/java/com/yahoo/vespa/config/RawConfig.java
@@ -141,7 +141,15 @@ public class RawConfig extends ConfigInstance {
      * @return  true if this config is equal to the config in the given request.
      */
     public boolean hasEqualConfig(JRTServerConfigRequest req) {
-        return getConfigMd5().equals(req.getRequestConfigMd5());
+        PayloadChecksums payloadChecksums = getPayloadChecksums();
+        PayloadChecksum xxhash64 = payloadChecksums.getForType(PayloadChecksum.Type.XXHASH64);
+        PayloadChecksum md5 = payloadChecksums.getForType(PayloadChecksum.Type.MD5);
+        if (xxhash64 != null)
+            return xxhash64.equals(req.getRequestConfigChecksums().getForType(PayloadChecksum.Type.XXHASH64));
+        if (md5 != null)
+            return md5.equals(req.getRequestConfigChecksums().getForType(PayloadChecksum.Type.MD5));
+
+        return true;
     }
 
     /**
diff --git a/config/src/main/java/com/yahoo/vespa/config/protocol/ConfigResponse.java b/config/src/main/java/com/yahoo/vespa/config/protocol/ConfigResponse.java
index 98fc7f7a50e..47d3203b32e 100644
--- a/config/src/main/java/com/yahoo/vespa/config/protocol/ConfigResponse.java
+++ b/config/src/main/java/com/yahoo/vespa/config/protocol/ConfigResponse.java
@@ -1,6 +1,7 @@
-// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.vespa.config.protocol;
 
+import com.yahoo.vespa.config.PayloadChecksum;
 import com.yahoo.vespa.config.PayloadChecksums;
 import com.yahoo.text.AbstractUtf8Array;
 
@@ -28,7 +29,15 @@ public interface ConfigResponse {
     void serialize(OutputStream os, CompressionType uncompressed) throws IOException;
 
     default boolean hasEqualConfig(JRTServerConfigRequest request) {
-        return (getConfigMd5().equals(request.getRequestConfigMd5()));
+        PayloadChecksums payloadChecksums = getPayloadChecksums();
+        PayloadChecksum xxhash64 = payloadChecksums.getForType(PayloadChecksum.Type.XXHASH64);
+        PayloadChecksum md5 = payloadChecksums.getForType(PayloadChecksum.Type.MD5);
+        if (xxhash64 != null)
+            return xxhash64.equals(request.getRequestConfigChecksums().getForType(PayloadChecksum.Type.XXHASH64));
+        if (md5 != null)
+            return md5.equals(request.getRequestConfigChecksums().getForType(PayloadChecksum.Type.MD5));
+
+        return true;
     }
 
     default boolean hasNewerGeneration(JRTServerConfigRequest request) {
diff --git a/config/src/main/java/com/yahoo/vespa/config/protocol/JRTServerConfigRequestV3.java b/config/src/main/java/com/yahoo/vespa/config/protocol/JRTServerConfigRequestV3.java
index 13d0ca1119a..d342e42ec03 100644
--- a/config/src/main/java/com/yahoo/vespa/config/protocol/JRTServerConfigRequestV3.java
+++ b/config/src/main/java/com/yahoo/vespa/config/protocol/JRTServerConfigRequestV3.java
@@ -82,8 +82,10 @@ public class JRTServerConfigRequestV3 implements JRTServerConfigRequest {
             JsonGenerator jsonGenerator = createJsonGenerator(byteArrayOutputStream);
             jsonGenerator.writeStartObject();
             addCommonReturnValues(jsonGenerator);
-            setResponseField(jsonGenerator, SlimeResponseData.RESPONSE_CONFIG_MD5, payloadChecksums.getForType(MD5).asString());
-            setResponseField(jsonGenerator, SlimeResponseData.RESPONSE_CONFIG_XXHASH64, payloadChecksums.getForType(XXHASH64).asString());
+            if (payloadChecksums.getForType(MD5) != null)
+                setResponseField(jsonGenerator, SlimeResponseData.RESPONSE_CONFIG_MD5, payloadChecksums.getForType(MD5).asString());
+            if (payloadChecksums.getForType(XXHASH64) != null)
+                setResponseField(jsonGenerator, SlimeResponseData.RESPONSE_CONFIG_XXHASH64, payloadChecksums.getForType(XXHASH64).asString());
             setResponseField(jsonGenerator, SlimeResponseData.RESPONSE_CONFIG_GENERATION, generation);
             setResponseField(jsonGenerator, SlimeResponseData.RESPONSE_APPLY_ON_RESTART, applyOnRestart);
             jsonGenerator.writeObjectFieldStart(SlimeResponseData.RESPONSE_COMPRESSION_INFO);
diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/rpc/ConfigResponseFactory.java b/configserver/src/main/java/com/yahoo/vespa/config/server/rpc/ConfigResponseFactory.java
index 8c1cdeb753a..0afab85df12 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/rpc/ConfigResponseFactory.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/rpc/ConfigResponseFactory.java
@@ -50,22 +50,23 @@ public interface ConfigResponseFactory {
     default PayloadChecksums generatePayloadChecksums(AbstractUtf8Array rawPayload, PayloadChecksums requestsPayloadChecksums) {
         PayloadChecksum requestChecksumMd5 = requestsPayloadChecksums.getForType(MD5);
         PayloadChecksum requestChecksumXxhash64 = requestsPayloadChecksums.getForType(XXHASH64);
+        PayloadChecksum xxhash64 = new PayloadChecksum(ConfigUtils.getXxhash64(rawPayload), XXHASH64);
 
-        PayloadChecksum md5 = PayloadChecksum.empty(MD5);
-        PayloadChecksum xxhash64 = PayloadChecksum.empty(XXHASH64);
-        // Response contains same checksum type as in request, except when both are empty,
-        // then use both checksum types in response
-        if (requestChecksumMd5.isEmpty() && requestChecksumXxhash64.isEmpty()
-                || ( ! requestChecksumMd5.isEmpty() && ! requestChecksumXxhash64.isEmpty())) {
-            md5 = new PayloadChecksum(ConfigUtils.getMd5(rawPayload), MD5);
-            xxhash64 = new PayloadChecksum(ConfigUtils.getXxhash64(rawPayload), XXHASH64);
-        } else if ( ! requestChecksumMd5.isEmpty()) {
+        if (requestChecksumMd5 == null)
+            return PayloadChecksums.from(xxhash64);
+
+        // Return xxhash64 checksum, even if none in request
+        if (requestChecksumXxhash64 == null)
+            return PayloadChecksums.from(new PayloadChecksum(ConfigUtils.getMd5(rawPayload), MD5), xxhash64);
+
+        // Response always contains xxhash64 checksum, md5 checksum is included
+        // when both are present, both are absent or only md5 is present in request
+        PayloadChecksum md5 = null;
+        if (( ! requestChecksumMd5.isEmpty() && ! requestChecksumXxhash64.isEmpty())
+                || (requestChecksumMd5.isEmpty() && requestChecksumXxhash64.isEmpty())) {
             md5 = new PayloadChecksum(ConfigUtils.getMd5(rawPayload), MD5);
-        } else if (requestChecksumMd5.isEmpty() && !requestChecksumXxhash64.isEmpty()) {
-            xxhash64 = new PayloadChecksum(ConfigUtils.getXxhash64(rawPayload), XXHASH64);
-        } else {
+        } else if (! requestChecksumMd5.isEmpty() && requestChecksumXxhash64.isEmpty()) {
             md5 = new PayloadChecksum(ConfigUtils.getMd5(rawPayload), MD5);
-            xxhash64 = new PayloadChecksum(ConfigUtils.getXxhash64(rawPayload), XXHASH64);
         }
 
         return PayloadChecksums.from(md5, xxhash64);
diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/rpc/ConfigResponseFactoryTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/rpc/ConfigResponseFactoryTest.java
index b164c3e5cd5..47094abb910 100644
--- a/configserver/src/test/java/com/yahoo/vespa/config/server/rpc/ConfigResponseFactoryTest.java
+++ b/configserver/src/test/java/com/yahoo/vespa/config/server/rpc/ConfigResponseFactoryTest.java
@@ -12,6 +12,7 @@ import org.junit.Test;
 import static com.yahoo.vespa.config.PayloadChecksum.Type.MD5;
 import static com.yahoo.vespa.config.PayloadChecksum.Type.XXHASH64;
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNull;
 
 /**
  * @author Ulf Lilleengen
@@ -39,30 +40,30 @@ public class ConfigResponseFactoryTest {
 
     @Test
     public void testLZ4CompressedFactory() {
-        // Both checksums in request
+        // md5 and xxhash64 checksums in request, both md5 and xxhash64 checksums should be in response
         {
             ConfigResponse response = createResponse(payloadChecksums);
             assertEquals(payloadChecksums, response.getPayloadChecksums());
         }
 
-        // No checksums in request (empty checksums), both checksums should be in response
+        // Empty md5 and xxhash64 checksums in request, both md5 and xxhash64 checksum should be in response
         {
             ConfigResponse response = createResponse(payloadChecksumsEmpty);
             assertEquals(payloadChecksums.getForType(MD5), response.getPayloadChecksums().getForType(MD5));
             assertEquals(payloadChecksums.getForType(XXHASH64), response.getPayloadChecksums().getForType(XXHASH64));
         }
 
-        // Only md5 checksums in request
+        // md5 checksum and no xxhash64 checksum in request, md5 and xxhash6 checksum in response
         {
             ConfigResponse response = createResponse(payloadChecksumsOnlyMd5);
             assertEquals(payloadChecksumsOnlyMd5.getForType(MD5), response.getPayloadChecksums().getForType(MD5));
-            assertEquals(payloadChecksumsOnlyMd5.getForType(XXHASH64), response.getPayloadChecksums().getForType(XXHASH64));
+            assertEquals(payloadChecksums.getForType(XXHASH64), response.getPayloadChecksums().getForType(XXHASH64));
         }
 
-        // Only xxhash64 checksums in request
+        // Only xxhash64 checksum in request, only xxhash64 checksums in response
         {
             ConfigResponse response = createResponse(payloadChecksumsOnlyXxhash64);
-            assertEquals(payloadChecksumsOnlyXxhash64.getForType(MD5), response.getPayloadChecksums().getForType(MD5));
+            assertNull(response.getPayloadChecksums().getForType(MD5));
             assertEquals(payloadChecksumsOnlyXxhash64.getForType(XXHASH64), response.getPayloadChecksums().getForType(XXHASH64));
         }
     }