Repository: consensys/tessera
Issue: AddPeer API accept duplicated url

tessera version: 0.7.3

I test `addPeer` API by below command:
```
curl -XPUT http://localhost:9002/config/peers -d '{"url":"http://localhost:9001"}' -H 'content-type:application/json'
curl -XPUT http://localhost:9002/config/peers -d '{"url":"http://localhost:9001"}' -H 'content-type:application/json'
curl -XPUT http://localhost:9002/config/peers -d '{"url":"http://localhost:9001"}' -H 'content-type:application/json'
```

then get peers by:
```
curl http://localhost:9002/config/peers
[{"type":"peer","url":"http://localhost:9000"},{"type":"peer","url":"http://localhost:9001"},{"type":"peer","url":"http://localhost:9001"},{"type":"peer","url":"http://localhost:9001"},{"type":"peer","url":"http://localhost:9001"},{"type":"peer","url":"http://localhost:9001"},{"type":"peer","url":"http://localhost:9001"},{"type":"peer","url":"http://localhost:9001"}]
```

I got lots of duplicated url in peers list, I think we should keep only one of these duplicated url.

Changed files:
jaxrs-service/src/main/java/com/quorum/tessera/p2p/ConfigResource.java, jaxrs-service/src/test/java/com/quorum/tessera/p2p/ConfigResourceTest.java

diff --git a/jaxrs-service/src/main/java/com/quorum/tessera/p2p/ConfigResource.java b/jaxrs-service/src/main/java/com/quorum/tessera/p2p/ConfigResource.java
index b4f4dcb91..142c5782b 100644
--- a/jaxrs-service/src/main/java/com/quorum/tessera/p2p/ConfigResource.java
+++ b/jaxrs-service/src/main/java/com/quorum/tessera/p2p/ConfigResource.java
@@ -12,11 +12,13 @@ import javax.ws.rs.core.MediaType;
 import javax.ws.rs.core.Response;
 import javax.ws.rs.core.UriBuilder;
 import java.net.URI;
-import java.util.Collections;
 import java.util.List;
 import java.util.Objects;
 import javax.ws.rs.core.GenericEntity;
 
+import static java.util.Collections.emptySet;
+import static java.util.Collections.singleton;
+
 @Path("/config")
 @Consumes(MediaType.APPLICATION_JSON)
 @Produces(MediaType.APPLICATION_JSON)
@@ -26,33 +28,38 @@ public class ConfigResource {
 
     private final PartyInfoService partyInfoService;
 
-    public ConfigResource(ConfigService configService, 
-            PartyInfoService partyInfoService) {
+    public ConfigResource(final ConfigService configService, final PartyInfoService partyInfoService) {
         this.configService = Objects.requireNonNull(configService);
         this.partyInfoService = Objects.requireNonNull(partyInfoService);
     }
-    
-
 
     @PUT
     @Path("/peers")
     public Response addPeer(@Valid final Peer peer) {
 
-        this.configService.addPeer(peer.getUrl());
+        final boolean existing = configService.getPeers().contains(peer);
 
-        partyInfoService.updatePartyInfo(
-                new PartyInfo(peer.getUrl(),Collections.EMPTY_SET, 
-                        Collections.singleton(new Party(peer.getUrl()))));
-        
-        //TODO: this seems a bit presumptuous, search for the peer instead?
-        final int index = this.configService.getPeers().size() - 1;
+        if (!existing) {
+            this.configService.addPeer(peer.getUrl());
+
+            this.partyInfoService.updatePartyInfo(
+                new PartyInfo(peer.getUrl(), emptySet(), singleton(new Party(peer.getUrl())))
+            );
+        }
+
+        final int index = this.configService.getPeers().indexOf(peer);
 
         final URI uri = UriBuilder.fromPath("config")
             .path("peers")
             .path(String.valueOf(index))
             .build();
 
-        return Response.created(uri).build();
+        if (!existing) {
+            return Response.created(uri).build();
+        } else {
+            return Response.ok().location(uri).build();
+        }
+
     }
 
     @GET
diff --git a/jaxrs-service/src/test/java/com/quorum/tessera/p2p/ConfigResourceTest.java b/jaxrs-service/src/test/java/com/quorum/tessera/p2p/ConfigResourceTest.java
index 68767a5c7..a4ba46337 100644
--- a/jaxrs-service/src/test/java/com/quorum/tessera/p2p/ConfigResourceTest.java
+++ b/jaxrs-service/src/test/java/com/quorum/tessera/p2p/ConfigResourceTest.java
@@ -4,22 +4,22 @@ import com.quorum.tessera.config.Peer;
 import com.quorum.tessera.core.config.ConfigService;
 import com.quorum.tessera.node.PartyInfoService;
 import com.quorum.tessera.node.model.PartyInfo;
-import java.util.ArrayList;
-import java.util.Arrays;
-import java.util.List;
-import javax.ws.rs.NotFoundException;
-import javax.ws.rs.core.Response;
-import static org.assertj.core.api.Assertions.*;
 import org.junit.After;
 import org.junit.Before;
 import org.junit.Test;
+import org.mockito.Mockito;
+
+import javax.ws.rs.NotFoundException;
+import javax.ws.rs.core.Response;
+import java.util.ArrayList;
+import java.util.List;
+
+import static java.util.Collections.singletonList;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.catchThrowable;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.anyString;
-import org.mockito.Mockito;
-import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.verify;
-import static org.mockito.Mockito.verifyNoMoreInteractions;
-import static org.mockito.Mockito.when;
+import static org.mockito.Mockito.*;
 
 public class ConfigResourceTest {
 
@@ -43,32 +43,61 @@ public class ConfigResourceTest {
     }
 
     @Test
-    public void addPeerIsSucessful() {
+    public void addPeerIsSuccessful() {
+
+        final Peer peer = new Peer("junit");
+        final List<Peer> peers = new ArrayList<>();
 
-        List<Peer> peers = new ArrayList<>();
-        Mockito.doAnswer((inv) -> {
+        Mockito.doAnswer(inv -> {
             peers.add(new Peer(inv.getArgument(0)));
             return null;
         }).when(configService).addPeer(anyString());
         when(configService.getPeers()).thenReturn(peers);
 
-        Peer peer = new Peer("junit");
+        final Response response = configResource.addPeer(peer);
 
-        Response response = configResource.addPeer(peer);
         assertThat(response.getStatus()).isEqualTo(201);
         assertThat(response.getLocation().toString()).isEqualTo("config/peers/0");
         assertThat(peers).containsExactly(peer);
+
         verify(configService).addPeer(peer.getUrl());
-        verify(configService).getPeers();
+        verify(configService, times(2)).getPeers();
         verify(partyInfoService).updatePartyInfo(any(PartyInfo.class));
     }
 
     @Test
-    public void getPeerIsSucessful() {
-        Peer peer = new Peer("getPeerIsSucessfulUrl");
-        when(configService.getPeers()).thenReturn(Arrays.asList(peer));
+    public void addExistingPeerIsSuccessful() {
 
-        Response response = configResource.getPeer(0);
+        final Peer peer = new Peer("junit");
+        final List<Peer> peers = new ArrayList<>();
+
+        Mockito.doAnswer(inv -> {
+            peers.add(new Peer(inv.getArgument(0)));
+            return null;
+        }).when(configService).addPeer(anyString());
+        when(configService.getPeers()).thenReturn(peers);
+
+        final Response responseOne = configResource.addPeer(peer);
+        assertThat(responseOne.getStatus()).isEqualTo(201);
+        assertThat(responseOne.getLocation().toString()).isEqualTo("config/peers/0");
+        assertThat(peers).containsExactly(peer);
+
+        final Response responseTwo = configResource.addPeer(peer);
+        assertThat(responseTwo.getStatus()).isEqualTo(200);
+        assertThat(responseTwo.getLocation().toString()).isEqualTo("config/peers/0");
+        assertThat(peers).containsExactly(peer);
+
+        verify(configService).addPeer(peer.getUrl());
+        verify(configService, times(4)).getPeers();
+        verify(partyInfoService).updatePartyInfo(any(PartyInfo.class));
+    }
+
+    @Test
+    public void getPeerIsSuccessful() {
+        final Peer peer = new Peer("getPeerIsSucessfulUrl");
+        when(configService.getPeers()).thenReturn(singletonList(peer));
+
+        final Response response = configResource.getPeer(0);
 
         assertThat(response.getStatus()).isEqualTo(200);
         assertThat(response.getEntity()).isEqualTo(peer);
@@ -78,22 +107,23 @@ public class ConfigResourceTest {
 
     @Test
     public void getPeerNotFound() {
-        Peer peer = new Peer("getPeerNoptFound");
-        when(configService.getPeers()).thenReturn(Arrays.asList(peer));
-
-        try {
-            configResource.getPeer(2);
-            failBecauseExceptionWasNotThrown(NotFoundException.class);
-        } catch (NotFoundException ex) {
-            verify(configService).getPeers();
-        }
+        final Peer peer = new Peer("getPeerNoptFound");
+        when(configService.getPeers()).thenReturn(singletonList(peer));
+
+        final Throwable throwable = catchThrowable(() -> this.configResource.getPeer(2));
+
+        assertThat(throwable).isInstanceOf(NotFoundException.class);
+
+        verify(configService).getPeers();
     }
 
     @Test
     public void getPeers() {
-        Peer peer = new Peer("somepeer");
-        when(configService.getPeers()).thenReturn(Arrays.asList(peer));
-        Response response = configResource.getPeers();
+        final Peer peer = new Peer("somepeer");
+        when(configService.getPeers()).thenReturn(singletonList(peer));
+
+        final Response response = configResource.getPeers();
+
         assertThat(response.getStatus()).isEqualTo(200);
         
         List<Peer> results = (List<Peer>) response.getEntity();