bschuchardt commented on pull request #5843:
URL: https://github.com/apache/geode/pull/5843#issuecomment-776071182


   @mivanac I think the best thing to do at this point is revert the merge and 
write some tests to fix the problems with
   
   1. membername being an empty string.  In this case all other fields are 
ignored.
   2. hashcode returning a value that places IDs having/not-having membernames 
in different hashmap buckets
   
   I noticed #1 when reviewing code when people were talking about #2.  I wrote 
a test for #1.  A test for #2 is harder to write.
   
   DistributionLocatorIdJUnitTest.java
   ```java
   @Test
   public void testEqualsWithMemberId() throws UnknownHostException {
     InetAddress address = InetAddress.getLocalHost();
     DistributionLocatorId dLI1 = new DistributionLocatorId(address, 40404, 
"127.0.0.1", null);
     DistributionLocatorId dLI2 = dLI1;
     @SuppressWarnings("RedundantStringConstructorCall")
     DistributionLocatorId dLI3 =
         new DistributionLocatorId(address, 40404, new String("127.0.0.1"), 
null);
     @SuppressWarnings("RedundantStringConstructorCall")
     DistributionLocatorId dLI4 = new 
DistributionLocatorId(InetAddress.getByName("localhost"),
         40405, new String("127.0.0.1"), null);
     dLI1.setMembername("");
     dLI3.setMembername("");
     dLI4.setMembername("");
     assertTrue(dLI1.equals(dLI2));
     assertTrue(dLI1.equals(dLI3));
     assertFalse(dLI1.equals(dLI4));
   }
   ```


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to