Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Check fails on abstract superclass #716

Closed
smainz opened this issue Oct 2, 2022 · 5 comments
Closed

Check fails on abstract superclass #716

smainz opened this issue Oct 2, 2022 · 5 comments
Labels

Comments

@smainz
Copy link

smainz commented Oct 2, 2022

Describe the bug

EqualsVerifier tries to instantiate abstract base classes and fails if the euals method in this class calls an abstract method.

EqualsVerifier fails on every derived class with this error message:

[ERROR] Failures:
[ERROR]   EqualsTest.testEqualsAndHashCode:49 EqualsVerifier found a problem in class de.dynaware.vbank.cryptowallet.additionaldata.valueobject.AdditionalCompanyDataId.
-> Symmetry:
  AdditionalCompanyDataId{id=00000000-0000-0000-ffff-ffffffffffff}
does not equal superclass instance
  [EntityId]-throws AbstractMethodError(Missing implementation of resolved method 'abstract java.lang.Object getId()' of abstract class de.dynaware.vbank.cryptowallet.additionaldata.valueobject.EntityId.)

For more information, go to: https://www.jqno.nl/equalsverifier/errormessages

To Reproduce
Steps to reproduce the behavior

  1. Create the class EntityId
public abstract class EntityId<ID> implements Serializable {

    @NonNull
    public abstract ID getId();

    public boolean equals(Object other) {
        if( other == null)
            return false;
        if (this == other) {
            return true;
        } else if (this.getClass().isAssignableFrom( other.getClass())) {
            var otherId = ((EntityId) other).getId();
            return this.getId().equals(otherId);
        } else {
            return false;
        }
    }

    public int hashCode() {
        return Objects.hash(this.getId());
    }

    @Override
    public String toString() {
        return getClass().getSimpleName() + "{" +
               "id=" + getId() +
               '}';
    }
}

`2. Create a derived class like this

public class AdditionalCompanyDataId
    extends EntityId<UUID> {

    private UUID id;

    public AdditionalCompanyDataId() {
        this( UUID.randomUUID() );
    }

    private AdditionalCompanyDataId(UUID uuid) {
        setId(uuid);
    }

    @Override
    public UUID getId() {
        return id;
    }

    public void setId(UUID id) {
        this.id = id;
    }
}
  1. Run the equals test: EqualsVerifier.forClass(AdditionalCompanyDataId.class).verify();`

Code that triggers the behavior
See above,

Error message
Complete stacktrace:

EqualsVerifier found a problem in class de.dynaware.vbank.cryptowallet.additionaldata.valueobject.AdditionalCompanyDataId.
-> Symmetry:
  AdditionalCompanyDataId{id=00000000-0000-0000-ffff-ffffffffffff}
does not equal superclass instance
  [EntityId]-throws AbstractMethodError(Missing implementation of resolved method 'abstract java.lang.Object getId()' of abstract class de.dynaware.vbank.cryptowallet.additionaldata.valueobject.EntityId.)

For more information, go to: https://www.jqno.nl/equalsverifier/errormessages
        at nl.jqno.equalsverifier.api.SingleTypeEqualsVerifierApi.verify(SingleTypeEqualsVerifierApi.java:314)
        at de.dynaware.vbank.cryptowallet.additionaldata.EqualsTest.testEqualsAndHashCode(EqualsTest.java:49)
        at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
        at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77)
        at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
        at java.base/java.lang.reflect.Method.invoke(Method.java:568)
        at org.junit.platform.commons.util.ReflectionUtils.invokeMethod(ReflectionUtils.java:725)
        at org.junit.jupiter.engine.execution.MethodInvocation.proceed(MethodInvocation.java:60)
        at org.junit.jupiter.engine.execution.InvocationInterceptorChain$ValidatingInvocation.proceed(InvocationInterceptorChain.java:131)
        at org.junit.jupiter.engine.extension.TimeoutExtension.intercept(TimeoutExtension.java:149)
        at org.junit.jupiter.engine.extension.TimeoutExtension.interceptTestableMethod(TimeoutExtension.java:140)
        at org.junit.jupiter.engine.extension.TimeoutExtension.interceptTestTemplateMethod(TimeoutExtension.java:92)
        at org.junit.jupiter.engine.execution.ExecutableInvoker$ReflectiveInterceptorCall.lambda$ofVoidMethod$0(ExecutableInvoker.java:115)
        at org.junit.jupiter.engine.execution.ExecutableInvoker.lambda$invoke$0(ExecutableInvoker.java:105)
        at org.junit.jupiter.engine.execution.InvocationInterceptorChain$InterceptedInvocation.proceed(InvocationInterceptorChain.java:106)
        at org.junit.jupiter.engine.execution.InvocationInterceptorChain.proceed(InvocationInterceptorChain.java:64)
        at org.junit.jupiter.engine.execution.InvocationInterceptorChain.chainAndInvoke(InvocationInterceptorChain.java:45)
        at org.junit.jupiter.engine.execution.InvocationInterceptorChain.invoke(InvocationInterceptorChain.java:37)
        at org.junit.jupiter.engine.execution.ExecutableInvoker.invoke(ExecutableInvoker.java:104)
        at org.junit.jupiter.engine.execution.ExecutableInvoker.invoke(ExecutableInvoker.java:98)
        at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.lambda$invokeTestMethod$7(TestMethodTestDescriptor.java:214)
        at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
        at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.invokeTestMethod(TestMethodTestDescriptor.java:210)
        at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.execute(TestMethodTestDescriptor.java:135)
        at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.execute(TestMethodTestDescriptor.java:66)
        at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$6(NodeTestTask.java:151)
        at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
        at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$8(NodeTestTask.java:141)
        at org.junit.platform.engine.support.hierarchical.Node.around(Node.java:137)
        at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$9(NodeTestTask.java:139)
        at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
        at org.junit.platform.engine.support.hierarchical.NodeTestTask.executeRecursively(NodeTestTask.java:138)
        at org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:95)
        at org.junit.platform.engine.support.hierarchical.SameThreadHierarchicalTestExecutorService.submit(SameThreadHierarchicalTestExecutorService.java:35)
        at org.junit.platform.engine.support.hierarchical.NodeTestTask$DefaultDynamicTestExecutor.execute(NodeTestTask.java:226)
        at org.junit.platform.engine.support.hierarchical.NodeTestTask$DefaultDynamicTestExecutor.execute(NodeTestTask.java:204)
        at org.junit.jupiter.engine.descriptor.TestTemplateTestDescriptor.execute(TestTemplateTestDescriptor.java:139)
        at org.junit.jupiter.engine.descriptor.TestTemplateTestDescriptor.lambda$execute$2(TestTemplateTestDescriptor.java:107)
        at java.base/java.util.stream.ForEachOps$ForEachOp$OfRef.accept(ForEachOps.java:183)
        at java.base/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:197)
        at java.base/java.util.stream.ReferencePipeline$2$1.accept(ReferencePipeline.java:179)
        at java.base/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:197)
        at java.base/java.util.stream.ForEachOps$ForEachOp$OfRef.accept(ForEachOps.java:183)
        at java.base/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:197)
        at java.base/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:197)
        at java.base/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:197)
        at java.base/java.util.stream.ForEachOps$ForEachOp$OfRef.accept(ForEachOps.java:183)
        at java.base/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:197)
        at java.base/java.util.Spliterators$ArraySpliterator.forEachRemaining(Spliterators.java:992)
        at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:509)
        at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:499)
        at java.base/java.util.stream.ForEachOps$ForEachOp.evaluateSequential(ForEachOps.java:150)
        at java.base/java.util.stream.ForEachOps$ForEachOp$OfRef.evaluateSequential(ForEachOps.java:173)
        at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
        at java.base/java.util.stream.ReferencePipeline.forEach(ReferencePipeline.java:596)
        at java.base/java.util.stream.ReferencePipeline$7$1.accept(ReferencePipeline.java:276)
        at java.base/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:197)
        at java.base/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:197)
        at java.base/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:197)
        at java.base/java.util.ArrayList$ArrayListSpliterator.forEachRemaining(ArrayList.java:1625)
        at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:509)
        at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:499)
        at java.base/java.util.stream.ForEachOps$ForEachOp.evaluateSequential(ForEachOps.java:150)
        at java.base/java.util.stream.ForEachOps$ForEachOp$OfRef.evaluateSequential(ForEachOps.java:173)
        at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
        at java.base/java.util.stream.ReferencePipeline.forEach(ReferencePipeline.java:596)
        at java.base/java.util.stream.ReferencePipeline$7$1.accept(ReferencePipeline.java:276)
        at java.base/java.util.ArrayList$ArrayListSpliterator.forEachRemaining(ArrayList.java:1625)
        at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:509)
        at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:499)
        at java.base/java.util.stream.ForEachOps$ForEachOp.evaluateSequential(ForEachOps.java:150)
        at java.base/java.util.stream.ForEachOps$ForEachOp$OfRef.evaluateSequential(ForEachOps.java:173)
        at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
        at java.base/java.util.stream.ReferencePipeline.forEach(ReferencePipeline.java:596)
        at org.junit.jupiter.engine.descriptor.TestTemplateTestDescriptor.execute(TestTemplateTestDescriptor.java:107)
        at org.junit.jupiter.engine.descriptor.TestTemplateTestDescriptor.execute(TestTemplateTestDescriptor.java:42)
        at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$6(NodeTestTask.java:151)
        at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
        at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$8(NodeTestTask.java:141)
        at org.junit.platform.engine.support.hierarchical.Node.around(Node.java:137)
        at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$9(NodeTestTask.java:139)
        at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
        at org.junit.platform.engine.support.hierarchical.NodeTestTask.executeRecursively(NodeTestTask.java:138)
        at org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:95)
        at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
        at org.junit.platform.engine.support.hierarchical.SameThreadHierarchicalTestExecutorService.invokeAll(SameThreadHierarchicalTestExecutorService.java:41)
        at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$6(NodeTestTask.java:155)
        at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
        at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$8(NodeTestTask.java:141)
        at org.junit.platform.engine.support.hierarchical.Node.around(Node.java:137)
        at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$9(NodeTestTask.java:139)
        at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
        at org.junit.platform.engine.support.hierarchical.NodeTestTask.executeRecursively(NodeTestTask.java:138)
        at org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:95)
        at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
        at org.junit.platform.engine.support.hierarchical.SameThreadHierarchicalTestExecutorService.invokeAll(SameThreadHierarchicalTestExecutorService.java:41)
        at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$6(NodeTestTask.java:155)
        at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
        at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$8(NodeTestTask.java:141)
        at org.junit.platform.engine.support.hierarchical.Node.around(Node.java:137)
        at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$9(NodeTestTask.java:139)
        at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
        at org.junit.platform.engine.support.hierarchical.NodeTestTask.executeRecursively(NodeTestTask.java:138)
        at org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:95)
        at org.junit.platform.engine.support.hierarchical.SameThreadHierarchicalTestExecutorService.submit(SameThreadHierarchicalTestExecutorService.java:35)
        at org.junit.platform.engine.support.hierarchical.HierarchicalTestExecutor.execute(HierarchicalTestExecutor.java:57)
        at org.junit.platform.engine.support.hierarchical.HierarchicalTestEngine.execute(HierarchicalTestEngine.java:54)
        at org.junit.platform.launcher.core.EngineExecutionOrchestrator.execute(EngineExecutionOrchestrator.java:107)
        at org.junit.platform.launcher.core.EngineExecutionOrchestrator.execute(EngineExecutionOrchestrator.java:88)
        at org.junit.platform.launcher.core.EngineExecutionOrchestrator.lambda$execute$0(EngineExecutionOrchestrator.java:54)
        at org.junit.platform.launcher.core.EngineExecutionOrchestrator.withInterceptedStreams(EngineExecutionOrchestrator.java:67)
        at org.junit.platform.launcher.core.EngineExecutionOrchestrator.execute(EngineExecutionOrchestrator.java:52)
        at org.junit.platform.launcher.core.DefaultLauncher.execute(DefaultLauncher.java:114)
        at org.junit.platform.launcher.core.DefaultLauncher.execute(DefaultLauncher.java:86)
        at org.junit.platform.launcher.core.DefaultLauncherSession$DelegatingLauncher.execute(DefaultLauncherSession.java:86)
        at org.apache.maven.surefire.junitplatform.LazyLauncher.execute(LazyLauncher.java:55)
        at org.apache.maven.surefire.junitplatform.JUnitPlatformProvider.execute(JUnitPlatformProvider.java:223)
        at org.apache.maven.surefire.junitplatform.JUnitPlatformProvider.invokeAllTests(JUnitPlatformProvider.java:175)
        at org.apache.maven.surefire.junitplatform.JUnitPlatformProvider.invoke(JUnitPlatformProvider.java:139)
        at org.apache.maven.surefire.booter.ForkedBooter.runSuitesInProcess(ForkedBooter.java:456)
        at org.apache.maven.surefire.booter.ForkedBooter.execute(ForkedBooter.java:169)
        at org.apache.maven.surefire.booter.ForkedBooter.run(ForkedBooter.java:595)
        at org.apache.maven.surefire.booter.ForkedBooter.main(ForkedBooter.java:581)
Caused by: nl.jqno.equalsverifier.internal.exceptions.AssertionException
        at nl.jqno.equalsverifier.internal.util.Assert.assertTrue(Assert.java:53)
        at nl.jqno.equalsverifier.internal.checkers.HierarchyChecker.checkSuperProperties(HierarchyChecker.java:130)
        at nl.jqno.equalsverifier.internal.checkers.HierarchyChecker.safelyCheckSuperProperties(HierarchyChecker.java:116)
        at nl.jqno.equalsverifier.internal.checkers.HierarchyChecker.checkSuperclass(HierarchyChecker.java:85)
        at nl.jqno.equalsverifier.internal.checkers.HierarchyChecker.check(HierarchyChecker.java:51)
        at nl.jqno.equalsverifier.api.SingleTypeEqualsVerifierApi.verifyWithExamples(SingleTypeEqualsVerifierApi.java:421)
        at nl.jqno.equalsverifier.api.SingleTypeEqualsVerifierApi.performVerification(SingleTypeEqualsVerifierApi.java:377)
        at nl.jqno.equalsverifier.api.SingleTypeEqualsVerifierApi.verify(SingleTypeEqualsVerifierApi.java:312)
        ... 122 more

The root couse is this:
Method threw 'java.lang.AbstractMethodError' exception. Cannot evaluate de.dynaware.vbank.cryptowallet.additionaldata.valueobject.EntityId$$DynamicSubclass$1827725498.toString()

image

called from:

private Object getEqualSuper(T reference) {
return ObjectAccessor.of(reference, type.getSuperclass()).copy();
}

Expected behavior

Expected behaviour was to ignore this check if the base class is abstract as it does here:

try {
checkSuperProperties(reference, equalSuper, scrambledAccessor.get());
} catch (AbstractMethodError | NullPointerException ignored) {
// In these cases, we'll assume all super properties hold.
// The problems we test for, can never occur anyway if you can't instantiate a super
// instance.
}
}

Version
3.10

Additional context

Workaround is to .suppress( Warning.STRICT_INHERITANCE)

I think it is wrong to try to instantiate an abstract base class.

Sory for beeing so verbose and thank you for thi snice library.

@jqno
Copy link
Owner

jqno commented Oct 3, 2022

Hm, I think your assessment is correct, but I'll have to investigate further. I'll let you know when I've done so.

Thanks for the verbosity btw, it's actually extremely helpful in investigating issues!

@jqno jqno added the accepted label Oct 3, 2022
@jqno
Copy link
Owner

jqno commented Dec 2, 2022

Took a while to get to this, sorry about that.

It turns out that the abstract class thing was a red herring, there is an actual Symmetry problem. On the failing check, the call to equals() actually goes through the else { return false; } branch at the end of the method. It doesn't touch the branch where the acutal check is performed, and therefore doesn't fail on calling the (still abstract) getId() method.

Then it tries to print an error message, and because toString() also tries to call getId(), that is where the AbstractMethodError occurs. This is consequently eaten by EqualsVerifier to produce a nicer looking error message, which is what you see.

You can check this for yourself in two ways. First, let's make the class non-abstract:

    public static class EntityId implements Serializable {

        private String id;

        public EntityId(String id) {
            this.id = id;
        }

        public String getId() {
            return id;
        };

        public boolean equals(Object obj) {
            if (obj == null) return false;
            if (this.getClass().isAssignableFrom(obj.getClass())) {
                String otherId = ((EntityId) obj).getId();
                return Objects.equals(getId(), otherId);
            }
            return false;
        }

        public int hashCode() {
            return Objects.hash(this.getId());
        }

        @Override
        public String toString() {
            return getClass().getSimpleName() + "{" +
               "id=" + getId() +
               '}';
        }
    }

    public static class AdditionalCompanyDataId extends EntityId {

        private AdditionalCompanyDataId(String id) {
            super(id);
        }
    }

You'll get the same error message as before, but without the abstract method stuff:

[ERROR]   AbstractHierarchyTest.issue716:78 EqualsVerifier found a problem in class nl.jqno.equalsverifier.integration.inheritance.AbstractHierarchyTest$AdditionalCompanyDataId.
-> Symmetry:
  AdditionalCompanyDataId{id=one}
does not equal superclass instance
  EntityId{id=one}

For more information, go to: https://www.jqno.nl/equalsverifier/errormessages

We can leave the class abstract, and change the equals() implementation slightly:

    public abstract class EntityId<ID> implements Serializable {

        @NonNull
        public abstract ID getId();

        public final boolean equals(Object other) {
            if (!(other instanceof EntityId)) {
                return false;
            }
            Object otherId = ((EntityId) other).getId();
            return Objects.equals(getId(), otherId);
        }

        public final int hashCode() {
            return Objects.hash(this.getId());
        }

        @Override
        public String toString() {
            return getClass().getSimpleName() + "{" +
                "id=" + getId() +
                '}';
        }
    }

With this implementation, the EqualsVerifier test passes. (Note that I've also made equals() and hashCode() final here.)

Now of course, this is not the implementation that you wanted. In order to make it work the way you want to, I'd suggest checking this part of the EqualsVerifier manual

For the next release, I'll change the way EqualsVerifier deals with exceptions thrown by toString(), to make things less confusing 😅

@jqno jqno closed this as completed Dec 2, 2022
@jqno
Copy link
Owner

jqno commented Dec 2, 2022

Better error messages are released in EqualsVerifier 3.12.1.

@smainz
Copy link
Author

smainz commented Dec 5, 2022

Thank you for this detailed analysis and sorry for putting you on the wrong track.

The whole idea of this is to create type save hibernate ids without having to re-type all the boilerplate code. My only requirement is that you can not pass an instance of class A with

class A extends EntityId<String> {
...
}

to a method which expects an id of class B with

class B extends EntityId<String> {
...
}

In hibernate this kind of id is an embeddable and somtimes is replaced by an - on the fly created - proxy.

While I do not consider new A("1") be equal to new B("1") if have to consider new B("1") equal to the proxy of B.
Sounds like there is no possible solution for this.

As it is unlikely to compare A to B i can make equals symmetric by using your approach for equals:

        public final boolean equals(Object other) {
            if (!(other instanceof EntityId)) {
                return false;
            }
            Object otherId = ((EntityId) other).getId();
            return Objects.equals(getId(), otherId);
        }

There will never be instances of EntityId<> (the abstract class) nor will there be derived classes of A´ or B` except maybe the generated proxies.

@jqno
Copy link
Owner

jqno commented Dec 6, 2022

Oh but it certainly is possible! It's not easy, but it is possible. Check out this link: https://www.artima.com/articles/how-to-write-an-equality-method-in-java (and skip to "Pitfall 4" if you're impatient).

Then you can check out this part of the EqualsVerifier manual to see how to translate this to EqualsVerifier: https://jqno.nl/equalsverifier/manual/inheritance/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants