In TypeBasedCandidateFilter.isCompatibleTypes -> ClassCastException: class TypeVariableImpl cannot be cast to class Class #3294

daniel-matheis-vivavis · 2024-03-11T07:55:28Z

Problem description

Given:
We are currenty migration from mocktio 5.1 and JDK 17 to mockito 5.11 and JDK 21.
We use annotation based mock injection.

Versions:
Mockito 5.11.0
Junit Jupiter 5.10.1
OpenJDK 64-Bit Server VM Temurin-21.0.2+13 (build 21.0.2+13-LTS, mixed mode, sharing)
Maven 3.9.6
Eclipse IDE 2023-12

Problematic scenario
Our problematic test case(s) extend a base test class with a type parameter
BaseTest<T>
This base test class also contains a field using the type variable

  @Mock
  Supplier<T> supplier;

When:
When the test is executed, either by Maven or via Eclipse IDE directly

Then
errors occur, the following stack trace can be seen

java.lang.ClassCastException: class sun.reflect.generics.reflectiveObjects.TypeVariableImpl cannot be cast to class java.lang.Class (sun.reflect.generics.reflectiveObjects.TypeVariableImpl and java.lang.Class are in module java.base of loader 'bootstrap')
	at org.mockito.internal.configuration.injection.filter.TypeBasedCandidateFilter.isCompatibleTypes(TypeBasedCandidateFilter.java:65)
	at org.mockito.internal.configuration.injection.filter.TypeBasedCandidateFilter.lambda$recurseOnTypeArguments$2(TypeBasedCandidateFilter.java:146)
	at java.base/java.util.stream.MatchOps$1MatchSink.accept(MatchOps.java:90)
	at java.base/java.util.stream.Streams$StreamBuilderImpl.tryAdvance(Streams.java:397)
	at java.base/java.util.stream.Streams$ConcatSpliterator.tryAdvance(Streams.java:723)
	at java.base/java.util.stream.ReferencePipeline.forEachWithCancel(ReferencePipeline.java:129)
	at java.base/java.util.stream.AbstractPipeline.copyIntoWithCancel(AbstractPipeline.java:527)
	at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:513)
	at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:499)
	at java.base/java.util.stream.MatchOps$MatchOp.evaluateSequential(MatchOps.java:230)
	at java.base/java.util.stream.MatchOps$MatchOp.evaluateSequential(MatchOps.java:196)
	at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
	at java.base/java.util.stream.ReferencePipeline.anyMatch(ReferencePipeline.java:632)
	at org.mockito.internal.configuration.injection.filter.TypeBasedCandidateFilter.recurseOnTypeArguments(TypeBasedCandidateFilter.java:144)
	at org.mockito.internal.configuration.injection.filter.TypeBasedCandidateFilter.isCompatibleTypes(TypeBasedCandidateFilter.java:52)
	at org.mockito.internal.configuration.injection.filter.TypeBasedCandidateFilter.filterCandidate(TypeBasedCandidateFilter.java:175)
	at org.mockito.internal.configuration.injection.PropertyAndSetterInjection.injectMockCandidatesOnFields(PropertyAndSetterInjection.java:136)
	at org.mockito.internal.configuration.injection.PropertyAndSetterInjection.injectMockCandidates(PropertyAndSetterInjection.java:113)
	at org.mockito.internal.configuration.injection.PropertyAndSetterInjection.processInjection(PropertyAndSetterInjection.java:81)
	at org.mockito.internal.configuration.injection.MockInjectionStrategy.process(MockInjectionStrategy.java:68)
	at org.mockito.internal.configuration.injection.MockInjectionStrategy.relayProcessToNextStrategy(MockInjectionStrategy.java:91)
	at org.mockito.internal.configuration.injection.MockInjectionStrategy.process(MockInjectionStrategy.java:71)
	at org.mockito.internal.configuration.injection.MockInjectionStrategy.relayProcessToNextStrategy(MockInjectionStrategy.java:91)
	at org.mockito.internal.configuration.injection.MockInjectionStrategy.process(MockInjectionStrategy.java:71)
	at org.mockito.internal.configuration.injection.MockInjection$OngoingMockInjection.apply(MockInjection.java:88)
	at org.mockito.internal.configuration.DefaultInjectionEngine.injectMocksOnFields(DefaultInjectionEngine.java:26)
	at org.mockito.internal.configuration.InjectingAnnotationEngine.injectCloseableMocks(InjectingAnnotationEngine.java:107)
	at org.mockito.internal.configuration.InjectingAnnotationEngine.process(InjectingAnnotationEngine.java:48)
	at org.mockito.MockitoAnnotations.openMocks(MockitoAnnotations.java:81)
	at org.mockito.internal.framework.DefaultMockitoSession.<init>(DefaultMockitoSession.java:43)
	at org.mockito.internal.session.DefaultMockitoSessionBuilder.startMocking(DefaultMockitoSessionBuilder.java:83)
	at org.mockito.junit.jupiter.MockitoExtension.beforeEach(MockitoExtension.java:160)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1596)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1596)
	Suppressed: java.lang.NullPointerException: Cannot invoke "java.util.Set.forEach(java.util.function.Consumer)" because the return value of "org.junit.jupiter.api.extension.ExtensionContext$Store.remove(Object, java.lang.Class)" is null
		at org.mockito.junit.jupiter.MockitoExtension.afterEach(MockitoExtension.java:194)
		... 2 more

During debugging the type variable 'T' is seen which is not "instanceof Class" but "instanceof Type".
This finally provokes the ClassCastException.
See next section for affected code; I assume another case is missing here: either it is a parameterized type or a non-parameterized type or none of those two.

Affected code*

in org.mockito.internal.configuration.injection.filter.TypeBasedCandidateFilter.isCompatibleTypes(Type, Type, Field)


          if (mockType instanceof ParameterizedType) {
                // ParameterizedType.equals() is documented as:
                // "Instances of classes that implement this interface must implement
                // an equals() method that equates any two instances that share the
                // same generic type declaration and have equal type parameters."
                // Unfortunately, e.g. Wildcard parameter "?" doesn't equal java.lang.String,
                // and e.g. Set doesn't equal TreeSet, so roll our own comparison if
                // ParameterizedTypeImpl.equals() returns false
                if (typeToMock.equals(mockType)) {
                    result = true;
                } else {
                    ParameterizedType genericTypeToMock = (ParameterizedType) typeToMock;
                    ParameterizedType genericMockType = (ParameterizedType) mockType;
                    Type[] actualTypeArguments = genericTypeToMock.getActualTypeArguments();
                    Type[] actualTypeArguments2 = genericMockType.getActualTypeArguments();
                    if (actualTypeArguments.length == actualTypeArguments2.length) {
                        // Recurse on type parameters, so we properly test whether e.g. Wildcard
                        // bounds have a match
                        result =
                                recurseOnTypeArguments(
                                        injectMocksField,
                                        actualTypeArguments,
                                        actualTypeArguments2);
                    } else {
                        // the two ParameterizedTypes cannot match because they have unequal
                        // number of type arguments
                        result = false;
                    }
                }
            } else {
                // mockType is a non-parameterized Class, i.e. a concrete class.
                // so walk concrete class' type hierarchy
                **Class<?> concreteMockClass = (Class<?>) mockType;**
                Stream<Type> mockSuperTypes = getSuperTypes(concreteMockClass);
                result =
                        mockSuperTypes.anyMatch(
                                mockSuperType ->
                                        isCompatibleTypes(
                                                typeToMock, mockSuperType, injectMocksField));
            }

So any info/help is welcome to fix this problem!

Attached maven project can be run with 5.1 and JDK 17 (working) or with 5.11 (not working).
reproduce_3294.zip

colleague mentioned that #2958 resp. #3019 is probably related...

The text was updated successfully, but these errors were encountered:

daniel-matheis-vivavis · 2024-03-13T08:19:36Z

After a closer look and local playing around with the code the feature "support generics" #2923 released with v5.2.0 introduced the problems which several users have reported.
I have "fixed" some things locally (some instanceof checks missing etc., some change in the comparison of TypeVariables) but by far not everything is fine again - many NPEs in test execution because injection of mocks is still not working...

Now that more and more projects will have to upgrade their dependency to the latest mockito to have JDK 21 support more will arrive at this situation of being blocked by this "broken" behaviour in Mockito in comparison to before 5.2.0.
I propose to introduce a "legacy feature toogle" in org.mockito.internal.configuration.injection.filter.TypeBasedCandidateFilter.filterCandidate(Collection, Field, List, Object, Field) (or before) to disable the new feature for those projects which don't work anymore and to stabilize the feature in the meantime.
What do you think, @jfrantzius / @TimvdLippe ?

If you agree with such approach, how would you define such a feature toggle/option? Where is the best "place" for it for the user? What would you usually prefer?
I could give it a try with your guidance...

jfrantzius · 2024-03-13T08:27:50Z

Sorry I haven't been looking at issues lately, what other issues pointing to a bug in that feature would come to your mind?

If there was a sudden wave of related issues due to JDK LTS changes, with a variety of different exceptions, then IMHO a feature toggle would be good. On the other hand, if it is only the same or very few underlying cases, we better fix those :)

daniel-matheis-vivavis · 2024-03-13T09:43:46Z

It is more a feeling when reading release notes and other issues that this feature has yet not reached full "maturity" for all cases.
And I think not all are always directly upgrading to latest mockito versions if not needed. Therefore this "wave" might be flattened.

Of course, fixing the different cases is an option and I would be happy to assist that.
However, it seems that until now no mockito expert has had the time to fix previous/related issues around that feature.
Which is totally ok if it would not be such a blocker now for us.

I will fork and commit my play around version of the TypeBaseCandidateFilter now so that you could see some places which might need some workover.
I have reduced failing tests from 696 to 338 at my place for one of our projects, now many NPEs (mocks not injected) but also " there were multiple matching mocks" for cases which should not happen now.
So my "fixes" surely are not correct/complete ...

Update:
see main...daniel-matheis-vivavis:mockito:main

…class TypeVariableImpl cannot be cast to class Class mockito#3294 Playing around to minimize errors in tests: - checked with "instanceof" to avoid class cast exception - sysouts for some "simple tracing" - widened comparison from TypeVariable#equals to allow "super types" in generic declaration as it seemed right to me

daniel-matheis-vivavis · 2024-03-13T12:17:40Z

During lunch break I have tried out to introduce such a simple feature toggle via System property in org.mockito.internal.configuration.injection.filter.TypeBasedCandidateFilter.filterCandidate(Collection, Field, List, Object, Field)
When the "pre-5.2.0" code/behaviour was activated, the number of test errors were reduced further from 338 to now 96.
Still "NPEs", i.e. mock not inject, and "multiple matching mocks".

So either just switching on that old behaviour in that single place is not enough "rollback" or there is another problem which I don't know the location yet...

…class TypeVariableImpl cannot be cast to class Class mockito#3294 introduced simple system property based feature toggle to disable "generics support" e.g. pass "-Dmockito.typeBasedCandidateFilter.genericsSupport.disabled" to surefire/tycho-surefire maven plugin to disable it generics support is still switched on per default

…class TypeVariableImpl cannot be cast to class Class mockito#3294 added test case to verify first case of mockito#3294: generics in combintation with inheritance in test and under test classes led to a ClassCastException (java.lang.ClassCastException: class sun.reflect.generics.reflectiveObjects.TypeVariableImpl cannot be cast to class java.lang.Class (sun.reflect.generics.reflectiveObjects.TypeVariableImpl and java.lang.Class are in module java.base of loader 'bootstrap') at org.mockito.internal.configuration.injection.filter.TypeBasedCandidateFilter.isCompatibleTypes(TypeBasedCandidateFilter.java:64))

daniel-matheis-vivavis · 2024-03-13T15:54:49Z

Looking at the rest of the failing tests in our project I think we are in many cases hit by the behaviour change #2978 / #2603
we are relying on <= 5.1 behaviour that constructor and field injection strategies are applied both which is different to the current official behaviour - can be read here: https://javadoc.io/doc/org.mockito/mockito-core/latest/org/mockito/InjectMocks.html

"Mockito will try to inject mocks only either by constructor injection, property injection or setter injection in order and as described below. If any of the following strategy fail, then Mockito won't report failure; i.e. you will have to provide dependencies yourself."

I assume we are out of luck here and that the official recommendation is to either provide a constructor with all parameters to be mocked or to use setter / field injection (remove any constructor with parameter)...

Actually we were quite used to the "combined injection" behaviour and it worked well for us and currently I don't get what was so bad about it.

…class TypeVariableImpl cannot be cast to class Class mockito#3294 - just ignore Object typed fields during injection target analysis, because it will almost always lead to unwanted "multiple matches" problem - moved knowledge about feature toggle GENERICS_SUPPORT_DISABLED_KEY back to TypeBasedCandidateFilter private scope and printing it just once

…class TypeVariableImpl cannot be cast to class Class mockito#3294 introduced simple feature toggle via system property "mockito.constructorInjection.alwaysTryNextStrategy.enabled" to in effect pass injection on to PropertyAndSetterInjection to optionally "restore" a bit of the pre 5.2.0 behaviour. Per default this legacy behaviour is turned off.

daniel-matheis-vivavis · 2024-03-14T10:01:48Z

Now test cases of that one project back to green. Achieved this by changing code back towards the legacy pre-5.2.0 behaviour.
Still prototypical code of course but sufficient at the moment, that we are not blocked anymore.

TimvdLippe · 2024-03-15T11:16:48Z

Based on my understanding of the various issues that have popped up after 5.2.0, the @InjectMocks is both ambiguously used and a lot of users relied on the magic behavior that it provided. While we did officially state what the feature would do, in practice it turned out to be difficult to align with what we wanted. The fixes that @jfrantzius provided were both necessary, but also introduced user friction as we didn't anticipate that so many of our users relied on the magic behavior.

We have had several proposals from community members to resolve this issue and I reviewed all PRs with suggestions. The most dedicated effort was in #3123 which is also a massive PR introducing more than 1500 new lines of code. Unfortunately we cannot make such massive overhaul and complications into our system, as users also rely on the performance and ease of mocks.

Therefore, I do think that we now align with what we originally promised. But the frustrating bit for users is that they have grown used to the previous behavior. Whether the previous behavior is better or not is a subjective question that truthfully, I don't know the answer to.

If you do have any suggestions on how we can improve the status quo while also ensure we maintain existing behavior and have minimal impact on the overall system, please send us a PR. One thing that I do want to make explicit: we will not introduce runtime flags for changing Mockito behavior. This has been suggested multiple times (not just for @InjectMocks) and we do not want to introduce a flag-based system to change behavior. Simply for the fact that so many other libraries build on top of Mockito, that they will rely on a specific value for the flag and become incompatible with each other.

daniel-matheis-vivavis · 2024-03-15T13:53:10Z

Thanks a lot for your reply and insights it provides to us externals ;)
You should know we users appreciate Mockito and the maintainers'/contributors' work a lot and couldn't really code without it!
And I fully agree that we don't want to have "feature toggle"/configuration hell... This was just a quick "private" workaround solution to unblock our own migration efforts.

So about

suggestions on how we can improve the status quo while also ensure we maintain existing behavior and have minimal impact on the overall system

I think what could help here is to make more mechanisms of Mockito exchangeable like JUnit 5 vs JUnit 4 or Eclipse E4 vs E3.
The principal: if the user of the framework is "unhappy" with some aspect, make this aspect exchangeable (if possible) so that not Mockito has an issue to solve but the user. Less tickets for you, more happy users.

I would see one important extension/variation point which probably would help a lot of pre-5.2.0 projects/users like us (relying on "do some more injection like in the 'old days'") to configure Mockito to their own needs:
Make the 'mock injection engine' exchangeable with own implementations.
The fixes I had to make were all located in this "area" of Mockito.
Is it really needed to "prescribe"/predefine in which fields and how Mockito should inject mocks or not?
Of course, Mockito can and should offer sensible defaults and recommend certain things which work for most cases and if a user is starting from scratch and he or she "feels" the wanted/strategical restrictions right away during writing test code usually there should not be any need to exchange something.
But for smoother test code/mechanism evolution and certain use cases of the framework such extension points would make our lives easier - probably.

Let me for example tell you why we need/rely on that "constructor plus field injection" mechanism:
Eclipse E4 RCP has its own injection framework and we rely on it of course. It works as well with constructor injection and with field injection at the same time.
You don't need setters or constructor parameters for "everything" you want to mock and Mockito worked well in injecting the mocks so far.
With EJBs / CDI it is a bit different: there we don't use such mix but only field injection. Therefore the current way of Mockito works well during injection without modifications - at least if that ClassCastException is fixed...

So far I have only scratched the surface of Mockito code and just checked quickly if I could make that injection engine exchangeable.
I jumped "backward" from the injection strategies and via DefaultInjectionEngine arriving at the InjectingAnnotationEngine.
It looked as if AnnotationEngine could be exchanged via such "Plugin" mechanism, so maybe it would be possible to go for this in our case, but it seems a bit "too much and too far away" to "just fix that injection".

Probably you insiders know best where to create such extension resp. if it is at all feasible.

TimvdLippe · 2024-03-15T14:16:43Z

Luckily that was my suggestion as well on the linked PR: #3123 (comment) As you can see in the PR, it introduces a new org.mockito.inject-generics artifact to achieve this with a newer extension that fixes several issues, while also allowing users to override it for their own. The extension point would be MockCandidateFilter.

If you can help us revive that PR and make it ready for merging (based on the feedback I gave on it), that would be amazing 😄

…class TypeVariableImpl cannot be cast to class Class mockito#3294 - "inverted" and renamed feature toggles to have them active per default which is the pre 5.2.0 behaviour more or less - to have unchanged tests still runnable, set the feature toggles to enable "new", unwanted behaviour

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

In TypeBasedCandidateFilter.isCompatibleTypes -> ClassCastException: class TypeVariableImpl cannot be cast to class Class #3294

In TypeBasedCandidateFilter.isCompatibleTypes -> ClassCastException: class TypeVariableImpl cannot be cast to class Class #3294

daniel-matheis-vivavis commented Mar 11, 2024 •

edited

daniel-matheis-vivavis commented Mar 13, 2024 •

edited

jfrantzius commented Mar 13, 2024

daniel-matheis-vivavis commented Mar 13, 2024 •

edited

daniel-matheis-vivavis commented Mar 13, 2024 •

edited

daniel-matheis-vivavis commented Mar 13, 2024 •

edited

daniel-matheis-vivavis commented Mar 14, 2024

TimvdLippe commented Mar 15, 2024

daniel-matheis-vivavis commented Mar 15, 2024

TimvdLippe commented Mar 15, 2024

In TypeBasedCandidateFilter.isCompatibleTypes -> ClassCastException: class TypeVariableImpl cannot be cast to class Class #3294

In TypeBasedCandidateFilter.isCompatibleTypes -> ClassCastException: class TypeVariableImpl cannot be cast to class Class #3294

Comments

daniel-matheis-vivavis commented Mar 11, 2024 • edited

Problem description

Affected code*

daniel-matheis-vivavis commented Mar 13, 2024 • edited

jfrantzius commented Mar 13, 2024

daniel-matheis-vivavis commented Mar 13, 2024 • edited

daniel-matheis-vivavis commented Mar 13, 2024 • edited

daniel-matheis-vivavis commented Mar 13, 2024 • edited

daniel-matheis-vivavis commented Mar 14, 2024

TimvdLippe commented Mar 15, 2024

daniel-matheis-vivavis commented Mar 15, 2024

TimvdLippe commented Mar 15, 2024

daniel-matheis-vivavis commented Mar 11, 2024 •

edited

daniel-matheis-vivavis commented Mar 13, 2024 •

edited

daniel-matheis-vivavis commented Mar 13, 2024 •

edited

daniel-matheis-vivavis commented Mar 13, 2024 •

edited

daniel-matheis-vivavis commented Mar 13, 2024 •

edited