[knox] branch master updated: KNOX-2314 - NPE from topology Service equals implementation when no URLs (#303)

[pzampino]

This is an automated email from the ASF dual-hosted git repository.

pzampino pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/knox.git


The following commit(s) were added to refs/heads/master by this push:
     new 52ca823  KNOX-2314 - NPE from topology Service equals implementation 
when no URLs (#303)
52ca823 is described below

commit 52ca823cb212566fab912c624e652858972214c8
Author: Phil Zampino <pzamp...@apache.org>
AuthorDate: Tue Mar 31 14:54:30 2020 -0400

    KNOX-2314 - NPE from topology Service equals implementation when no URLs 
(#303)
---
 .../org/apache/knox/gateway/topology/Service.java   |  4 ++--
 .../org/apache/knox/gateway/topology/Topology.java  |  2 +-
 .../apache/knox/gateway/topology/TopologyTest.java  | 21 +++++++++++++++++++++
 .../topology/simple/SimpleDescriptorHandler.java    |  4 ++--
 4 files changed, 26 insertions(+), 5 deletions(-)

diff --git 
a/gateway-spi/src/main/java/org/apache/knox/gateway/topology/Service.java 
b/gateway-spi/src/main/java/org/apache/knox/gateway/topology/Service.java
index a064039..39f51aa 100644
--- a/gateway-spi/src/main/java/org/apache/knox/gateway/topology/Service.java
+++ b/gateway-spi/src/main/java/org/apache/knox/gateway/topology/Service.java
@@ -128,8 +128,8 @@ public class Service {
     return (new EqualsBuilder()).append(name, that.name)
                                 .append(role, that.role)
                                 .append(version, that.version)
-                                
.append(urls.stream().sorted().collect(Collectors.toList()),
-                                        
that.urls.stream().sorted().collect(Collectors.toList()))
+                                
.append(getUrls().stream().sorted().collect(Collectors.toList()),
+                                        
that.getUrls().stream().sorted().collect(Collectors.toList()))
                                 .append(params, that.params)
                                 .build();
   }
diff --git 
a/gateway-spi/src/main/java/org/apache/knox/gateway/topology/Topology.java 
b/gateway-spi/src/main/java/org/apache/knox/gateway/topology/Topology.java
index 31cc314..15145fd 100644
--- a/gateway-spi/src/main/java/org/apache/knox/gateway/topology/Topology.java
+++ b/gateway-spi/src/main/java/org/apache/knox/gateway/topology/Topology.java
@@ -182,7 +182,7 @@ public class Topology {
     }
 
     Topology other = (Topology) obj;
-    if (Objects.equals(this.name,other.name)) {
+    if (Objects.equals(this.name, other.name)) {
       // Order is NOT significant for providers, services, and applications
 
       if (equalProviders(other)) {  // Providers
diff --git 
a/gateway-spi/src/test/java/org/apache/knox/gateway/topology/TopologyTest.java 
b/gateway-spi/src/test/java/org/apache/knox/gateway/topology/TopologyTest.java
index acacf6a..fec1f3e 100644
--- 
a/gateway-spi/src/test/java/org/apache/knox/gateway/topology/TopologyTest.java
+++ 
b/gateway-spi/src/test/java/org/apache/knox/gateway/topology/TopologyTest.java
@@ -133,6 +133,27 @@ public class TopologyTest {
   }
 
   @Test
+  public void testTopologiesWithSameServicesWithNullURLs() {
+    final String name = "topologyX";
+
+    final String serviceName1 = "service1";
+    final String serviceRole1 = "role1";
+    Service service1 = createService(serviceName1,
+                                     serviceRole1,
+                                     null,                    // Null URLs
+                                     Collections.emptyMap());
+
+    List<Service> services1 = Collections.singletonList(service1);
+    List<Service> services2 = Collections.singletonList(service1);
+
+    Topology t1 = createTopology(name, Collections.emptyList(), services1, 
Collections.emptyList());
+    Topology t2 = createTopology(name, Collections.emptyList(), services2, 
Collections.emptyList());
+
+    assertEquals(t1, t2);
+    assertEquals("hashcode must be equal if objects are equal.", 
t1.hashCode(), t2.hashCode());
+  }
+
+  @Test
   public void testTopologiesWithSameServicesWithDifferentURLOrder() {
     final String name = "topologyX";
 
diff --git 
a/gateway-topology-simple/src/main/java/org/apache/knox/gateway/topology/simple/SimpleDescriptorHandler.java
 
b/gateway-topology-simple/src/main/java/org/apache/knox/gateway/topology/simple/SimpleDescriptorHandler.java
index 162b0eb..f94fe3e 100644
--- 
a/gateway-topology-simple/src/main/java/org/apache/knox/gateway/topology/simple/SimpleDescriptorHandler.java
+++ 
b/gateway-topology-simple/src/main/java/org/apache/knox/gateway/topology/simple/SimpleDescriptorHandler.java
@@ -636,8 +636,8 @@ public class SimpleDescriptorHandler {
                 try (InputStream in = new 
ByteArrayInputStream(generatedContent.getBytes(StandardCharsets.UTF_8))) {
                     Topology generatedTopology = topologyService.parse(in);
                     generatedTopology.setName(topologyName);
-                    // If the generated toplogy is different from the 
existing, then it should be persisted
-                    result = !generatedTopology.equals(existing);
+                    // If the generated topology is different from the 
existing, then it should be persisted
+                    result = !existing.equals(generatedTopology);
                 } catch (Exception e) {
                     log.errorComparingGeneratedTopology(topologyName, e);
                 }