OKAPI-1243: equals should support subclasses of ModuleId, SemVer, …

okapi-common/src/main/java/org/folio/okapi/common/ModuleId.java

@@ -114,7 +114,10 @@ public boolean equals(Object that) {
     if (this == that) {
       return true;
     }
-    if (!(that instanceof ModuleId)) {
+    if (that == null) {
+      return false;
+    }
+    if (that.getClass() != getClass()) {
       return false;
     }
     return compareTo((ModuleId) that) == 0;

okapi-common/src/main/java/org/folio/okapi/common/SemVer.java

@@ -259,7 +259,10 @@ public boolean equals(Object that) {
     if (this == that) {
       return true;
     }
-    if (!(that instanceof SemVer)) {
+    if (that == null) {
+      return false;
+    }
+    if (that.getClass() != getClass()) {
       return false;
     }
     return compareTo((SemVer) that) == 0;

okapi-common/src/main/java/org/folio/okapi/common/refreshtoken/tokencache/TenantUserCache.java

@@ -24,11 +24,14 @@ public boolean equals(Object o) {
       if (this == o) {
         return true;
       }
-      if (o instanceof TokenKey) {
-        TokenKey tokenKey = (TokenKey) o; // if on java17 we didn't have to do this
-        return tenant.equals(tokenKey.tenant) && user.equals(tokenKey.user);
+      if (o == null) {
+        return false;
       }
-      return false;
+      if (o.getClass() != getClass()) {
+        return false;
+      }
+      var tokenKey = (TokenKey) o;
+      return tenant.equals(tokenKey.tenant) && user.equals(tokenKey.user);
     }
 
     @Override

okapi-common/src/test/java/org/folio/okapi/common/ModuleIdTest.java

@@ -1,15 +1,15 @@
 package org.folio.okapi.common;
 
+import static org.junit.Assert.*;
+
 import java.util.LinkedList;
 import java.util.List;
-import org.junit.Test;
-import static org.junit.Assert.*;
+import org.junit.jupiter.api.Test;
 
-public class ModuleIdTest {
+class ModuleIdTest {
 
-  @java.lang.SuppressWarnings({"squid:S5961"}) // more than 25 assertions
   @Test
-  public void test() {
+  void test() {
     ModuleId module_1 = new ModuleId("module-1");
     assertEquals("module-1", module_1.getId());
     assertTrue(module_1.hasSemVer());
@@ -24,15 +24,44 @@
     assertFalse(module_1plus2.hasNpmSnapshot());
     assertEquals("module", module_1plus2.getProduct());
     assertEquals("module-1-2+3", module_1plus2.toString());
+  }
+
+  static class ExtendedModuleId extends ModuleId {
+    public final String extension;
 
-    assertNotEquals(module_1, module_1plus2);
-    ModuleId module_1_ref = module_1;
-    assertEquals(module_1, module_1_ref);
-    ModuleId module_1plus2copy = new ModuleId("module-1-2+3");
-    assertEquals(module_1plus2, module_1plus2copy);
+    public ExtendedModuleId(String s, String extension) {
+      super(s);
+      this.extension = extension;
+    }
+  }
 
-    assertEquals(module_1plus2.hashCode(), module_1plus2copy.hashCode());
+  @Test
+  void testEquals() {
+    var module1 = new ModuleId("module-1");
+    var module1plus2 = new ModuleId("module-1-2+3");
+    assertNotEquals(module1, module1plus2);
+    var module1ref = module1;
+    assertEquals(module1, module1ref);
+    var module1plus2copy = new ModuleId("module-1-2+3");
+    assertEquals(module1plus2, module1plus2copy);
+    assertFalse(module1.equals(null));
+    assertNotEquals(module1, "module-1");
+    var extendedModule1 = new ExtendedModuleId("module-1", "e");
+    var extendedModule2 = new ExtendedModuleId("module-2", "e");
+    var extendedModule1copy = new ExtendedModuleId("module-1", "f");
+    assertEquals(extendedModule1, extendedModule1);
+    assertNotEquals(extendedModule1, extendedModule2);
+    assertEquals(extendedModule1, extendedModule1copy);
+    assertNotEquals(extendedModule1, module1);
+    assertNotEquals(module1, extendedModule1);
+
+    assertEquals(module1plus2.hashCode(), module1plus2copy.hashCode());
+    assertNotEquals(module1.hashCode(), module1plus2.hashCode());
+  }
 
+  @Test
+  void testComparisons() {
+    ModuleId module_1 = new ModuleId("module-1");
     ModuleId foobar_1_2 = new ModuleId("foo-bar1-1.2");
     assertEquals("foo-bar1-1.2", foobar_1_2.toString());
 
@@ -72,7 +101,7 @@
   }
 
   @Test
-  public void testLatest() {
+  void testLatest() {
     ModuleId module_1 = new ModuleId("module-1");
     List<String> versionsL = new LinkedList<>();
     versionsL.add("module-1.0");
@@ -85,15 +114,15 @@
   }
 
   @Test
-  public void testNpmSnapshot() {
+  void testNpmSnapshot() {
     ModuleId module_1_10000 = new ModuleId("module-1.2.10000");
     assertTrue(module_1_10000.hasSemVer());
     assertFalse(module_1_10000.hasPreRelease());
     assertTrue(module_1_10000.hasNpmSnapshot());
   }
 
   @Test
-  public void testWithoutSemVer() {
+  void testWithoutSemVer() {
     ModuleId module = new ModuleId("module");
     assertFalse(module.hasSemVer());
     assertFalse(module.hasPreRelease());

okapi-common/src/test/java/org/folio/okapi/common/SemVerTest.java

@@ -1,9 +1,11 @@
 package org.folio.okapi.common;
 
-import org.junit.Test;
 import static org.junit.Assert.*;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.CsvSource;
 
-public class SemVerTest {
+class SemVerTest {
 
   private SemVer createVersion(String id, boolean isPrelease, boolean isNpmsnapshot) {
     SemVer v = new SemVer(id);
@@ -25,9 +27,8 @@
     assertNull(v);
   }
 
-  @java.lang.SuppressWarnings({"squid:S5961"}) // more than 25 assertions
   @Test
-  public void test() {
+  void test() {
     SemVer v1 = createVersion("1", false, false);
     SemVer v2 = createVersion("2", false, false);
 
@@ -57,7 +58,11 @@
 
     assertFalse(v1_10_0.hasPrefix(v1_10_1));
     assertFalse(v1_10_1.hasPrefix(v1_10_0));
+  }
 
+  void testPrefix() {
+    SemVer v1 = createVersion("1", false, false);
+    SemVer v1_0 = createVersion("1.0", false, false);
     SemVer p1 = createVersion("1.0.0-alpha", true, false);
     SemVer p2 = createVersion("1.0.0-alpha.1", true, false);
     SemVer p3 = createVersion("1.0.0-alpha.beta", true, false);
@@ -98,7 +103,10 @@
     assertEquals(1, p6.compareTo(p5));
     assertEquals(1, p7.compareTo(p6));
     assertEquals(1, p8.compareTo(p7));
+  }
 
+  @Test
+  void testSnapshot() {
     SemVer snap1 = createVersion("1.0.0-rc.1+snapshot-2017.1", true, false);
     SemVer snap2 = createVersion("1.0.0-rc.1+snapshot-2017.2", true, false);
     assertTrue(snap1.hasPrefix(snap1));
@@ -116,7 +124,70 @@
     assertEquals(1, snap2.compareTo(snap1));
     assertEquals(1, snap3.compareTo(snap1));
     assertEquals(-1, snap1.compareTo(snap3));
+  }
+
+  @Test
+  void testEquals() {
+    var semVer = new SemVer("1.2.0-SNAPSHOT");
+    var semVerCopy = new SemVer("1.2.0-SNAPSHOT");
+    assertEquals(semVer, semVer);
+    assertEquals(semVer, semVerCopy);
+  }
+
+  @ParameterizedTest
+  @CsvSource(textBlock = """
+      1, 1.0
+      1.0, 1
+      1, 1.0.0
+      1.0.0, 1
+      1.0.0, 1.0.1
+      1.0.0, 1.1.0
+      1.0.0, 1.0.0-foo
+      1.0.0-foo-1, 1.0.0-foo-2
+      """)
+  void testNotEquals(String string1, String string2) {
+    var semVer1 = new SemVer(string1);
+    var semVer2 = new SemVer(string2);
+    var equals = semVer1.equals(semVer2);
+    assertFalse(equals);
+    assertNotEquals(semVer1.hashCode(), semVer2.hashCode());
+  }
+
+  @Test
+  void testEqualsNull() {
+    var equals = new SemVer("1.0.0").equals(null);
+    assertFalse(equals);
+  }
+
+  @Test
+  void testEqualsIdent() {
+    var semVer = new SemVer("1.0.0");
+    assertEquals(semVer, semVer);
+  }
 
+  static class ExtendedSemVer extends SemVer {
+    public final String extension;
+
+    public ExtendedSemVer(String s, String extension) {
+      super(s);
+      this.extension = extension;
+    }
+  }
+
+  @Test
+  void testEqualsExtended() {
+    var semVer3 = new SemVer("1.2.3");
+    var extendedSemVer3e = new ExtendedSemVer("1.2.3", "e");
+    var extendedSemVer3f = new ExtendedSemVer("1.2.3", "f");
+    var extendedSemVer4e = new ExtendedSemVer("1.2.4", "e");
+    assertNotEquals(semVer3, extendedSemVer3e);
+    assertEquals(extendedSemVer3e, extendedSemVer3f);
+    assertNotEquals(extendedSemVer3e, extendedSemVer4e);
+  }
+
+  @Test
+  void testLong() {
+    SemVer snap1 = createVersion("1.0.0-rc.1+snapshot-2017.1", true, false);
     SemVer npmSnapshot = new SemVer("4000001006.1.0");
     assertEquals(-4, snap1.compareTo(npmSnapshot));
 
@@ -125,7 +196,7 @@
   }
 
   @Test
-  public void testMixedPrerelease() {
+  void testMixedPrerelease() {
     SemVer v2a3a = createVersion("1.0.0-2a-3a", true, false);
     SemVer v2a3 = createVersion("1.0.0-2a-3", true, false);
     SemVer v1234 = createVersion("1.0.0-1234", true, false);
@@ -137,7 +208,7 @@
   }
 
   @Test
-  public void testInvalid() {
+  void testInvalid() {
     invalidVersion("", "missing major version: ");
     invalidVersion("x", "missing major version: x");
     invalidVersion("x.y", "missing major version: x.y");

okapi-common/src/test/java/org/folio/okapi/common/refreshtoken/tokencache/TenantUserCacheTest.java

@@ -1,16 +1,16 @@
 package org.folio.okapi.common.refreshtoken.tokencache;
 
 import static org.hamcrest.CoreMatchers.is;
+import static org.hamcrest.CoreMatchers.not;
 import static org.hamcrest.CoreMatchers.nullValue;
 import static org.hamcrest.MatcherAssert.assertThat;
-
 import org.junit.Assert;
-import org.junit.Test;
+import org.junit.jupiter.api.Test;
 
-public class TenantUserCacheTest {
+class TenantUserCacheTest {
 
   @Test
-  public void lookup() {
+  void lookup() {
     TenantUserCache tk = new TenantUserCache(10);
     StringBuilder tenant = new StringBuilder("tenant1");
     StringBuilder user = new StringBuilder("user1");
@@ -24,20 +24,41 @@ public void lookup() {
     Assert.assertThrows(IllegalArgumentException.class, () -> tk.put(null, "user",  "v1",0));
   }
 
+  static class ExtendedTokenKey extends TenantUserCache.TokenKey {
+    public final String extension;
+
+    public ExtendedTokenKey(String tenant, String user, String extension) {
+      super(tenant, user);
+      this.extension = extension;
+    }
+  }
+
+
   @Test
-  public void tokenKey() {
-    TenantUserCache.TokenKey tokenKey1 = new TenantUserCache.TokenKey("a", "b");
+  void tokenKey() {
+    var tokenKey1 = new TenantUserCache.TokenKey("a", "b");
     assertThat(tokenKey1.equals(tokenKey1), is(true));
+    assertThat(tokenKey1.hashCode(), is(tokenKey1.hashCode()));
 
-    TenantUserCache.TokenKey tokenKey2 = new TenantUserCache.TokenKey("a", "b");
+    var tokenKey2 = new TenantUserCache.TokenKey("a", "b");
     assertThat(tokenKey1.equals(tokenKey2), is(true));
+    assertThat(tokenKey1.hashCode(), is(tokenKey2.hashCode()));
 
-    TenantUserCache.TokenKey tokenKey3 = new TenantUserCache.TokenKey("a", "c");
+    var tokenKey3 = new TenantUserCache.TokenKey("a", "c");
     assertThat(tokenKey1.equals(tokenKey3), is(false));
+    assertThat(tokenKey1.hashCode(), is(not(tokenKey3.hashCode())));
 
-    TenantUserCache.TokenKey tokenKey4 = new TenantUserCache.TokenKey("b", "c");
+    var tokenKey4 = new TenantUserCache.TokenKey("b", "c");
     assertThat(tokenKey1.equals(tokenKey4), is(false));
+    assertThat(tokenKey1.hashCode(), is(not(tokenKey4.hashCode())));
+
+    var tokenKey5 = new ExtendedTokenKey("a", "b", "e");
+    assertThat(tokenKey1.equals(tokenKey5), is(false));
+
+    var tokenKey6 = new ExtendedTokenKey("a", "b", "f");
+    assertThat(tokenKey5.equals(tokenKey6), is(true));
+    assertThat(tokenKey5.hashCode(), is(tokenKey6.hashCode()));
 
-    assertThat(tokenKey1.equals("a"), is(false));
+    assertThat(tokenKey6.equals(null), is(false));
   }
 }