fix(endpoints): endpoints with same name are allowed

fix gravitee-io/issues#436
gravitee-io · Feb 20, 2017 · a9c8a3b · a9c8a3b
1 parent 6376480
commit a9c8a3b
Show file tree

Hide file tree

Showing 7 changed files with 61 additions and 16 deletions.
diff --git a/...on/src/main/java/io/gravitee/definition/jackson/datatype/api/deser/ProxyDeserializer.java b/...on/src/main/java/io/gravitee/definition/jackson/datatype/api/deser/ProxyDeserializer.java
@@ -17,6 +17,7 @@
 
 import com.fasterxml.jackson.core.JsonParser;
 import com.fasterxml.jackson.databind.DeserializationContext;
+import com.fasterxml.jackson.databind.JsonMappingException;
 import com.fasterxml.jackson.databind.JsonNode;
 import com.fasterxml.jackson.databind.deser.std.StdScalarDeserializer;
 import io.gravitee.definition.model.Endpoint;
@@ -25,6 +26,9 @@
 import io.gravitee.definition.model.Proxy;
 
 import java.io.IOException;
+import java.util.HashSet;
+import java.util.List;
+import java.util.stream.Collectors;
 
 /**
  * @author David BRASSELY (david.brassely at graviteesource.com)
@@ -53,14 +57,14 @@ public Proxy deserialize(JsonParser jp, DeserializationContext ctxt)
         final JsonNode nodeEndpoints = node.get("endpoints");
 
         if (nodeEndpoints != null && nodeEndpoints.isArray()) {
-            nodeEndpoints.elements().forEachRemaining(jsonNode -> {
-                try {
-                    Endpoint endpoint = jsonNode.traverse(jp.getCodec()).readValueAs(Endpoint.class);
-                    proxy.getEndpoints().add(endpoint);
-                } catch (IOException e) {
-                    e.printStackTrace();
+            proxy.setEndpoints(new HashSet<>(nodeEndpoints.size()));
+            for (JsonNode jsonNode : nodeEndpoints) {
+                Endpoint endpoint = jsonNode.traverse(jp.getCodec()).readValueAs(Endpoint.class);
+                boolean added = proxy.getEndpoints().add(endpoint);
+                if(!added) {
+                    throw ctxt.mappingException("[api] API must have single endpoint names");
                 }
-            });
+            }
         }
 
         JsonNode stripContextNode = node.get("strip_context_path");

diff --git a/jackson/src/main/java/io/gravitee/definition/jackson/datatype/api/ser/ProxySerializer.java b/jackson/src/main/java/io/gravitee/definition/jackson/datatype/api/ser/ProxySerializer.java
@@ -23,6 +23,7 @@
 
 import java.io.IOException;
 import java.util.List;
+import java.util.Set;
 
 /**
  * @author David BRASSELY (david.brassely at graviteesource.com)
@@ -42,7 +43,7 @@ public void serialize(Proxy proxy, JsonGenerator jgen, SerializerProvider provid
         jgen.writeBooleanField("dumpRequest", proxy.isDumpRequest());
         jgen.writeBooleanField("multiTenant", proxy.isMultiTenant());
 
-        final List<Endpoint> endpoints = proxy.getEndpoints();
+        final Set<Endpoint> endpoints = proxy.getEndpoints();
 
         jgen.writeArrayFieldStart("endpoints");
         endpoints.forEach(endpoint -> {

diff --git a/jackson/src/test/java/io/gravitee/definition/jackson/api/ApiDeserializerTest.java b/jackson/src/test/java/io/gravitee/definition/jackson/api/ApiDeserializerTest.java
@@ -230,7 +230,7 @@ public void definition_singleEndpoint() throws Exception {
     public void definition_singleEndpoint_backup() throws Exception {
         Api api = load("/io/gravitee/definition/jackson/api-singleendpoint.json", Api.class);
         Assert.assertEquals(1, api.getProxy().getEndpoints().size());
-        Assert.assertFalse(api.getProxy().getEndpoints().get(0).isBackup());
+        Assert.assertFalse(api.getProxy().getEndpoints().iterator().next().isBackup());
     }
 
     @Test
@@ -249,7 +249,7 @@ public void definition_multipleEndpoints() throws Exception {
     public void definition_singleEndpoint_inArray_backup() throws Exception {
         Api api = load("/io/gravitee/definition/jackson/api-singleendpoint-inarray.json", Api.class);
         Assert.assertEquals(1, api.getProxy().getEndpoints().size());
-        Assert.assertFalse(api.getProxy().getEndpoints().get(0).isBackup());
+        Assert.assertFalse(api.getProxy().getEndpoints().iterator().next().isBackup());
     }
 
     @Test
@@ -356,8 +356,13 @@ public void definition_failover_singlecase() throws Exception {
     public void definition_failover_singlecase_backup() throws Exception {
         Api api = load("/io/gravitee/definition/jackson/api-failover-singlecase.json", Api.class);
 
-        Assert.assertFalse(api.getProxy().getEndpoints().get(0).isBackup());
-        Assert.assertTrue(api.getProxy().getEndpoints().get(1).isBackup());
+        api.getProxy().getEndpoints().forEach(endpoint -> {
+            if ("endpoint_0".equals(endpoint.getName())) {
+                Assert.assertFalse(endpoint.isBackup());
+            } else {
+                Assert.assertTrue(endpoint.isBackup());
+            }
+        });
     }
 
     @Test
@@ -372,6 +377,12 @@ public void definition_multiTenant_enable() throws Exception {
         Api api = load("/io/gravitee/definition/jackson/api-multitenant.json", Api.class);
 
         Assert.assertTrue(api.getProxy().isMultiTenant());
-        Assert.assertEquals("europe", api.getProxy().getEndpoints().get(0).getTenant());
+        Assert.assertEquals("europe", api.getProxy().getEndpoints().iterator().next().getTenant());
+    }
+
+    @Test(expected = JsonMappingException.class)
+    public void shouldFailWithSameEndpointNames() throws Exception {
+        load("/io/gravitee/definition/jackson/api-multiplesameendpoints.json", Api.class);
+        Assert.fail("should throw deser exception");
     }
 }
diff --git a/jackson/src/test/resources/io/gravitee/definition/jackson/api-default-failover.json b/jackson/src/test/resources/io/gravitee/definition/jackson/api-default-failover.json
@@ -6,8 +6,10 @@
     "context_path": "/team",
     "endpoints": [
       {
+        "name": "endpoint_0",
         "target": "http://host1:8083/myapi"
       }, {
+        "name": "endpoint_1",
         "target": "http://host2:8083/myapi"
       }
     ],

diff --git a/jackson/src/test/resources/io/gravitee/definition/jackson/api-multiplesameendpoints.json b/jackson/src/test/resources/io/gravitee/definition/jackson/api-multiplesameendpoints.json
@@ -0,0 +1,23 @@
+{
+  "id": "my-api",
+  "name": "my-team-api",
+
+  "proxy": {
+    "context_path": "/team",
+    "endpoints": [
+      {
+        "name": "endpoint_0",
+        "target": "http://host1:8083/myapi"
+      }, {
+        "name": "endpoint_0",
+        "target": "http://host2:8083/myapi"
+      }
+    ],
+    "strip_context_path": false
+  },
+
+  "paths": {
+    "/*": [
+    ]
+  }
+}
diff --git a/jackson/src/test/resources/io/gravitee/definition/jackson/api-override-failover.json b/jackson/src/test/resources/io/gravitee/definition/jackson/api-override-failover.json
@@ -6,8 +6,10 @@
     "context_path": "/team",
     "endpoints": [
       {
+        "name": "endpoint_0",
         "target": "http://host1:8083/myapi"
       }, {
+        "name": "endpoint_1",
         "target": "http://host2:8083/myapi"
       }
     ],

diff --git a/model/src/main/java/io/gravitee/definition/model/Proxy.java b/model/src/main/java/io/gravitee/definition/model/Proxy.java
@@ -16,7 +16,9 @@
 package io.gravitee.definition.model;
 
 import java.util.ArrayList;
+import java.util.HashSet;
 import java.util.List;
+import java.util.Set;
 
 /**
  * @author David BRASSELY (david.brassely at graviteesource.com)
@@ -30,7 +32,7 @@ public class Proxy {
 
     private String contextPath;
 
-    private List<Endpoint> endpoints = new ArrayList<>();
+    private Set<Endpoint> endpoints = null;
 
     private LoadBalancer loadBalancer = new LoadBalancer();
 
@@ -50,11 +52,11 @@ public void setContextPath(String contextPath) {
         this.contextPath = contextPath;
     }
 
-    public List<Endpoint> getEndpoints() {
+    public Set<Endpoint> getEndpoints() {
         return endpoints;
     }
 
-    public void setEndpoints(List<Endpoint> endpoints) {
+    public void setEndpoints(Set<Endpoint> endpoints) {
         this.endpoints = endpoints;
     }