tolerates Consul providing duplicate keys

src/main/java/com/orbitz/consul/cache/ConsulCache.java

@@ -11,8 +11,10 @@
 import org.slf4j.LoggerFactory;
 
 import java.math.BigInteger;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 import java.util.concurrent.CopyOnWriteArrayList;
 import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.Executors;
@@ -135,17 +137,25 @@ public ImmutableMap<K, V> getMap() {
         return lastResponse.get();
     }
 
-    private ImmutableMap<K, V> convertToMap(ConsulResponse<List<V>> response) {
+    @VisibleForTesting
+    ImmutableMap<K, V> convertToMap(final ConsulResponse<List<V>> response) {
         if (response == null || response.getResponse() == null || response.getResponse().isEmpty()) {
             return ImmutableMap.of();
         }
 
-        ImmutableMap.Builder<K, V> builder = ImmutableMap.builder();
-        for (V v : response.getResponse()) {
-            K key = keyConversion.apply(v);
+        final ImmutableMap.Builder<K, V> builder = ImmutableMap.builder();
+        final Set<K> keySet = new HashSet<>();
+        for (final V v : response.getResponse()) {
+            final K key = keyConversion.apply(v);
             if (key != null) {
-                builder.put(key, v);
+                if (!keySet.contains(key)) {
+                    builder.put(key, v);
+                } else {
+                    System.out.println(key.toString());
+                    LOGGER.warn("Duplicate service encountered. May differ by tags. Try using more specific tags? " + key.toString());
+                }
             }
+            keySet.add(key);
         }
         return builder.build();
     }

src/test/java/com/orbitz/consul/cache/ConsulCacheTest.java

@@ -1,23 +1,30 @@
 package com.orbitz.consul.cache;
 
+import com.google.common.base.Function;
+import com.google.common.base.Optional;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.net.HostAndPort;
 import com.orbitz.consul.Consul;
 import com.orbitz.consul.HealthClient;
 import com.orbitz.consul.KeyValueClient;
+import com.orbitz.consul.model.ConsulResponse;
 import com.orbitz.consul.model.agent.Agent;
 import com.orbitz.consul.model.health.ServiceHealth;
 import com.orbitz.consul.model.kv.Value;
 import org.junit.Test;
+import org.mockito.Mockito;
 
+import java.math.BigInteger;
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.List;
 import java.util.Map;
 import java.util.UUID;
 import java.util.concurrent.TimeUnit;
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.fail;
 
 public class ConsulCacheTest {
@@ -253,4 +260,26 @@ public void notify(Map<String, Value> newValues) {
         kvClient.deleteKeys(root);
 
     }
+
+    /**
+     * Test that if Consul for some reason returns a duplicate service or keyvalue entry
+     * that we recover gracefully by taking the first value, ignoring duplicates, and warning
+     * user of the condition 
+     */
+    @Test
+    public void testDuplicateServicesDontCauseFailure() {
+        final Function<Value, String> keyExtractor = new Function<Value, String>() {
+            @Override
+            public String apply(final Value input) {
+                return "SAME_KEY";
+            }
+        };
+        final List<Value> response = Arrays.asList(Mockito.mock(Value.class), Mockito.mock(Value.class));
+        final ConsulCache<String, Value> consulCache = new ConsulCache<>(keyExtractor, null);
+        final ConsulResponse<List<Value>> consulResponse = new ConsulResponse<>(response, 0, false, BigInteger.ONE); 
+        final ImmutableMap<String, Value> map = consulCache.convertToMap(consulResponse);
+        assertNotNull(map);
+        // Second copy has been weeded out
+        assertEquals(1, map.size());
+    }
 }