[ambari] branch branch-2.7 updated: AMBARI-25638 FindBugs: Class defines equals() and uses Object.hashCode() (#3301)

[payert]

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

payert pushed a commit to branch branch-2.7
in repository https://gitbox.apache.org/repos/asf/ambari.git


The following commit(s) were added to refs/heads/branch-2.7 by this push:
     new 9260f12  AMBARI-25638 FindBugs: Class defines equals() and uses 
Object.hashCode() (#3301)
9260f12 is described below

commit 9260f1281e617e760740657927c3637e491a9edc
Author: Tamas Payer <35402259+pay...@users.noreply.github.com>
AuthorDate: Tue Apr 20 15:40:18 2021 +0200

    AMBARI-25638 FindBugs: Class defines equals() and uses Object.hashCode() 
(#3301)
    
    * Refactor equals() and add hashCode()
---
 .../ambari/server/state/alert/MetricSource.java    | 27 ++++++++-------
 .../metric/system/impl/MetricsSourceTest.java      | 38 ++++++++++++++++++++++
 2 files changed, 54 insertions(+), 11 deletions(-)

diff --git 
a/ambari-server/src/main/java/org/apache/ambari/server/state/alert/MetricSource.java
 
b/ambari-server/src/main/java/org/apache/ambari/server/state/alert/MetricSource.java
index cb6fd3b..d81deaf 100644
--- 
a/ambari-server/src/main/java/org/apache/ambari/server/state/alert/MetricSource.java
+++ 
b/ambari-server/src/main/java/org/apache/ambari/server/state/alert/MetricSource.java
@@ -20,13 +20,14 @@ package org.apache.ambari.server.state.alert;
 import static java.util.stream.Collectors.toMap;
 import static java.util.stream.IntStream.range;
 
-import java.util.ArrayList;
 import java.util.List;
 import java.util.Objects;
 import java.util.Optional;
 
 import org.apache.ambari.server.controller.jmx.JMXMetricHolder;
 import org.apache.ambari.server.state.UriInfo;
+import org.apache.commons.collections.CollectionUtils;
+import org.apache.commons.collections.ListUtils;
 import org.springframework.expression.spel.standard.SpelExpressionParser;
 import org.springframework.expression.spel.support.StandardEvaluationContext;
 
@@ -137,18 +138,22 @@ public class MetricSource extends Source {
 
     @Override
     public boolean equals(Object object) {
-      if (!JmxInfo.class.isInstance(object)) {
-        return false;
+      if (object instanceof JmxInfo) {
+        JmxInfo other = (JmxInfo) object;
+        if (propertyList == null) {
+          return other.propertyList == null;
+        } else {
+          // !!! even if out of order, this is enough to fail
+          return other.propertyList != null &&
+              
CollectionUtils.isEqualCollection(ListUtils.unmodifiableList(propertyList), 
ListUtils.unmodifiableList(other.propertyList));
+        }
       }
+      return false;
+    }
 
-      JmxInfo other = (JmxInfo)object;
-
-      List<String> list1 = new ArrayList<>(propertyList);
-      List<String> list2 = new ArrayList<>(other.propertyList);
-
-      // !!! even if out of order, this is enough to fail
-      return list1.equals(list2);
-
+    @Override
+    public int hashCode() {
+      return Objects.hash(propertyList);
     }
 
     public String getUrlSuffix() {
diff --git 
a/ambari-server/src/test/java/org/apache/ambari/server/metric/system/impl/MetricsSourceTest.java
 
b/ambari-server/src/test/java/org/apache/ambari/server/metric/system/impl/MetricsSourceTest.java
index 351d2aa..7411018 100644
--- 
a/ambari-server/src/test/java/org/apache/ambari/server/metric/system/impl/MetricsSourceTest.java
+++ 
b/ambari-server/src/test/java/org/apache/ambari/server/metric/system/impl/MetricsSourceTest.java
@@ -20,11 +20,15 @@ package org.apache.ambari.server.metric.system.impl;
 
 import static java.util.Collections.singletonList;
 import static junit.framework.Assert.assertEquals;
+import static junit.framework.Assert.assertFalse;
+import static junit.framework.Assert.assertTrue;
 import static org.easymock.EasyMock.capture;
 import static org.easymock.EasyMock.expectLastCall;
 import static org.easymock.EasyMock.replay;
 import static org.easymock.EasyMock.verify;
 
+import java.util.Arrays;
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
@@ -185,4 +189,38 @@ public class MetricsSourceTest {
     assertEquals("custom", deserialized.getValue().toString());
     assertEquals(singletonList("prop1"), deserialized.getPropertyList());
   }
+
+  @Test
+  public void testJmxInfoEquality() {
+    MetricSource.JmxInfo jmxInfo = new MetricSource.JmxInfo();
+    MetricSource.JmxInfo otherJmxInfo = null;
+
+    assertFalse(jmxInfo.equals(otherJmxInfo));
+
+    otherJmxInfo = new MetricSource.JmxInfo();
+
+    assertTrue(jmxInfo.equals(otherJmxInfo));
+    assertTrue(otherJmxInfo.equals(jmxInfo));
+    assertEquals(jmxInfo.hashCode(), otherJmxInfo.hashCode());
+
+    jmxInfo.setPropertyList(Arrays.asList("a", "b", "c", "d"));
+    assertFalse(jmxInfo.equals(otherJmxInfo));
+    assertFalse(otherJmxInfo.equals(jmxInfo));
+    assertFalse(jmxInfo.hashCode() == otherJmxInfo.hashCode());
+
+    otherJmxInfo.setPropertyList(Arrays.asList("a", "b", "c", "d"));
+    assertTrue(jmxInfo.equals(otherJmxInfo));
+    assertTrue(otherJmxInfo.equals(jmxInfo));
+    assertEquals(jmxInfo.hashCode(), otherJmxInfo.hashCode());
+
+    jmxInfo.setPropertyList(Collections.emptyList());
+    assertFalse(jmxInfo.equals(otherJmxInfo));
+    assertFalse(otherJmxInfo.equals(jmxInfo));
+    assertFalse(jmxInfo.hashCode() == otherJmxInfo.hashCode());
+
+    otherJmxInfo.setPropertyList(Arrays.asList("b", "c", "d", "a"));
+    assertFalse(jmxInfo.equals(otherJmxInfo));
+    assertFalse(otherJmxInfo.equals(jmxInfo));
+    assertFalse(jmxInfo.hashCode() == otherJmxInfo.hashCode());
+  }
 }