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,99 +1,129 @@
 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;
+
+class ModuleIdTest {
+
+  @Test
+  void test() {
+    ModuleId module1 = new ModuleId("module-1");
+    assertEquals("module-1", module1.getId());
+    assertTrue(module1.hasSemVer());
+    assertFalse(module1.hasPreRelease());
+    assertEquals("module", module1.getProduct());
+    assertEquals("module-1", module1.toString());
+
+    ModuleId module1plus2 = new ModuleId("module-1-2+3");
+    assertEquals("module-1-2+3", module1plus2.getId());
+    assertTrue(module1plus2.hasSemVer());
+    assertTrue(module1plus2.hasPreRelease());
+    assertFalse(module1plus2.hasNpmSnapshot());
+    assertEquals("module", module1plus2.getProduct());
+    assertEquals("module-1-2+3", module1plus2.toString());
+  }
 
-public class ModuleIdTest {
+  static class ExtendedModuleId extends ModuleId {
+    public final String extension;
+
+    public ExtendedModuleId(String s, String extension) {
+      super(s);
+      this.extension = extension;
+    }
+  }
 
-  @java.lang.SuppressWarnings({"squid:S5961"}) // more than 25 assertions
   @Test
-  public void test() {
-    ModuleId module_1 = new ModuleId("module-1");
-    assertEquals("module-1", module_1.getId());
-    assertTrue(module_1.hasSemVer());
-    assertFalse(module_1.hasPreRelease());
-    assertEquals("module", module_1.getProduct());
-    assertEquals("module-1", module_1.toString());
-
-    ModuleId module_1plus2 = new ModuleId("module-1-2+3");
-    assertEquals("module-1-2+3", module_1plus2.getId());
-    assertTrue(module_1plus2.hasSemVer());
-    assertTrue(module_1plus2.hasPreRelease());
-    assertFalse(module_1plus2.hasNpmSnapshot());
-    assertEquals("module", module_1plus2.getProduct());
-    assertEquals("module-1-2+3", module_1plus2.toString());
-
-    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);
-
-    assertEquals(module_1plus2.hashCode(), module_1plus2copy.hashCode());
-
-    ModuleId foobar_1_2 = new ModuleId("foo-bar1-1.2");
-    assertEquals("foo-bar1-1.2", foobar_1_2.toString());
-
-    ModuleId module_1_9 = new ModuleId("module-1.9");
-    assertEquals("module-1.9", module_1_9.toString());
+  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);
+    var equals = module1.equals(null);
+    assertFalse(equals);
+    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 module1 = new ModuleId("module-1");
+    ModuleId foobar1dot2 = new ModuleId("foo-bar1-1.2");
+    assertEquals("foo-bar1-1.2", foobar1dot2.toString());
+
+    ModuleId module1dot9 = new ModuleId("module-1.9");
+    assertEquals("module-1.9", module1dot9.toString());
 
     ModuleId module = new ModuleId("module");
     assertEquals("module", module.toString());
 
-    assertTrue(module_1.hasPrefix(module));
-    assertFalse(module.hasPrefix(module_1));
-    assertTrue(module_1_9.hasPrefix(module));
-    assertTrue(module_1_9.hasPrefix(module_1));
-    assertFalse(module_1.hasPrefix(module_1_9));
-    assertFalse(foobar_1_2.hasPrefix(module));
+    assertTrue(module1.hasPrefix(module));
+    assertFalse(module.hasPrefix(module1));
+    assertTrue(module1dot9.hasPrefix(module));
+    assertTrue(module1dot9.hasPrefix(module1));
+    assertFalse(module1.hasPrefix(module1dot9));
+    assertFalse(foobar1dot2.hasPrefix(module));
 
-    assertEquals(5, module_1.compareTo(foobar_1_2));
-    assertEquals(-5, foobar_1_2.compareTo(module_1));
+    assertEquals(5, module1.compareTo(foobar1dot2));
+    assertEquals(-5, foobar1dot2.compareTo(module1));
 
-    assertEquals(-3, module_1.compareTo(module_1_9));
-    assertEquals(3, module_1_9.compareTo(module_1));
+    assertEquals(-3, module1.compareTo(module1dot9));
+    assertEquals(3, module1dot9.compareTo(module1));
 
-    assertEquals(-5, foobar_1_2.compareTo(module_1_9));
-    assertEquals(5, module_1_9.compareTo(foobar_1_2));
+    assertEquals(-5, foobar1dot2.compareTo(module1dot9));
+    assertEquals(5, module1dot9.compareTo(foobar1dot2));
 
-    assertEquals(-4, module.compareTo(module_1));
-    assertEquals(4, module_1.compareTo(module));
+    assertEquals(-4, module.compareTo(module1));
+    assertEquals(4, module1.compareTo(module));
     assertEquals(0, module.compareTo(module));
 
     assertEquals(-4, ModuleId.compare("abc-2.9", "abc-3.5"));
     assertEquals(-5, ModuleId.compare("abc-2", "abcd-3"));
 
-    assertNotEquals(module_1, foobar_1_2);
+    assertNotEquals(module1, foobar1dot2);
 
-    assertTrue(module_1.hasSemVer());
-    assertEquals("1", module_1.getSemVer().toString());
+    assertTrue(module1.hasSemVer());
+    assertEquals("1", module1.getSemVer().toString());
   }
 
   @Test
-  public void testLatest() {
-    ModuleId module_1 = new ModuleId("module-1");
+  void testLatest() {
+    ModuleId module = new ModuleId("module-1");
     List<String> versionsL = new LinkedList<>();
     versionsL.add("module-1.0");
     versionsL.add("module-1.0-2");
     versionsL.add("module-0.9");
     versionsL.add("other-1.1");
     versionsL.add("other-0.9");
-    assertEquals("module-1.0", module_1.getLatest(versionsL));
-    assertEquals("module-1", module_1.getLatest(new LinkedList<>()));
+    assertEquals("module-1.0", module.getLatest(versionsL));
+    assertEquals("module-1", module.getLatest(new LinkedList<>()));
   }
 
   @Test
-  public 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());
+  void testNpmSnapshot() {
+    ModuleId module = new ModuleId("module-1.2.10000");
+    assertTrue(module.hasSemVer());
+    assertFalse(module.hasPreRelease());
+    assertTrue(module.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,9 @@ private void invalidVersion(String id, String exp) {
     assertNull(v);
   }
 
-  @java.lang.SuppressWarnings({"squid:S5961"}) // more than 25 assertions
   @Test
-  public void test() {
+  @SuppressWarnings("java:S117")  // allow underscore in local variables
+  void test() {
     SemVer v1 = createVersion("1", false, false);
     SemVer v2 = createVersion("2", false, false);
 
@@ -57,7 +59,11 @@ public void test() {
 
     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 v1dot0 = 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);
@@ -73,9 +79,9 @@ public void test() {
     assertTrue(p1.hasPrefix(v1));
     assertTrue(p7.hasPrefix(v1));
     assertTrue(p8.hasPrefix(v1));
-    assertTrue(p1.hasPrefix(v1_0));
-    assertTrue(p7.hasPrefix(v1_0));
-    assertTrue(p8.hasPrefix(v1_0));
+    assertTrue(p1.hasPrefix(v1dot0));
+    assertTrue(p7.hasPrefix(v1dot0));
+    assertTrue(p8.hasPrefix(v1dot0));
     assertFalse(v1.hasPrefix(p8));
     assertTrue(p1.hasPrefix(p8));
     assertFalse(p4.hasPrefix(p5));
@@ -98,7 +104,10 @@ public void test() {
     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 +125,70 @@ public void test() {
     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 +197,7 @@ public void test() {
   }
 
   @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 +209,7 @@ public void testMixedPrerelease() {
   }
 
   @Test
-  public void testInvalid() {
+  void testInvalid() {
     invalidVersion("", "missing major version: ");
     invalidVersion("x", "missing major version: x");
     invalidVersion("x.y", "missing major version: x.y");