Ensure that a non-indexed entry doesn't break generic type resolution

[geoand]

This can happen if say some interface extends a JDK interface and there
is no reason the type resolution should fail because of it

[geoand]

cc @lburgazzoli who initially brought this to me

[quarkus-bot]

This workflow status is outdated as a new workflow run has been triggered.

Failing Jobs - Building 6f1268a

Status	Name	Step	Test failures	Logs	Raw logs
✖	JVM Tests - JDK 11	Build	Test failures	Logs	Raw logs
✔️	JVM Tests - JDK 15

Full information is available in the Build summary check run.

Test Failures

⚙️ JVM Tests - JDK 11 #

📦 extensions/jdbc/jdbc-mysql/deployment

✖ io.quarkus.jdbc.mysql.deployment.DevServicesMySQLDatasourceTestCase.testDatasource - More details - Source on GitHub

[FroMage]

Sorry, I don't understand the change.

What I think this is doing is that for external callers to findParametersRecursively this will still throw if not found, but if it recurses inside, it will return null instead.

So… I assume fetchFromIndex is the one doing the throwing, and then it will only throw for the first-level call, not for any recursion?

[geoand]

What I think this is doing is that for external callers to findParametersRecursively this will still throw if not found, but if it recurses inside, it will return null instead.

What external callers?

So… I assume fetchFromIndex is the one doing the throwing, and then it will only throw for the first-level call, not for any recursion?

Yes, fetchFromIndex is the method that throws.

My whole point is that I don't want th exception to terminal when looking up superclasses or interfaces

[FroMage]

What external callers?

The function that calls it the first time. It's not going to just call itself ;)

My whole point is that I don't want th exception to terminal when looking up superclasses or interfaces

OK. Got it.

My only worry is that this is changing the behaviour for all users of that method, and so it may be hiding the cause of a further error.

I think there are two cases where we can fail to find the parameter recursively:

• It's in a super class that is missing from the index, we won't find it anywhere else and the user must know that we failed to find it because the super class should be indexed.
• It's not in a super class, which is missing anyway, and we can go on and find it later from another super class that is indexed.

I think you've treated the second case. I worry about the first case, because the result is that we'll tell the user we could not find the parameter (it will be null) but we won't be able to tell him why (the class Foo was not indexed).

OTOH, if the contract of the external caller to findParametersRecursively is that it can return null and that's not an error, then perhaps we never want any exception. But I don't think that's the case. I think the null return value is only valid for recursion calls, to allow further lookup.

So perhaps the safer thing to do is to record which types are not indexed during recursion, and keep going. If at the end of the external call we have a result, then ignore all those not-indexed errors. If we don't, then raise an error explaining which types were not indexed that could be the cause of this error?

WDYT?

[geoand]

What external callers?

The function that calls it the first time. It's not going to just call itself ;)

My whole point is that I don't want th exception to terminal when looking up superclasses or interfaces

OK. Got it.

My only worry is that this is changing the behaviour for all users of that method, and so it may be hiding the cause of a further error.

I think there are two cases where we can fail to find the parameter recursively:

• It's in a super class that is missing from the index, we won't find it anywhere else and the user must know that we failed to find it because the super class should be indexed.
• It's not in a super class, which is missing anyway, and we can go on and find it later from another super class that is indexed.

I think you've treated the second case. I worry about the first case, because the result is that we'll tell the user we could not find the parameter (it will be null) but we won't be able to tell him why (the class Foo was not indexed).

OTOH, if the contract of the external caller to findParametersRecursively is that it can return null and that's not an error, then perhaps we never want any exception. But I don't think that's the case. I think the null return value is only valid for recursion calls, to allow further lookup.

Yes, I completely agree

So perhaps the safer thing to do is to record which types are not indexed during recursion, and keep going. If at the end of the external call we have a result, then ignore all those not-indexed errors. If we don't, then raise an error explaining which types were not indexed that could be the cause of this error?

WDYT?

Sure yeah, that makes sense. I'll update the PR on Monday

[FroMage]

OK great, no rush :)

[geoand]

PR updated as per your suggestion and tests added

[quarkus-bot]

This workflow status is outdated as a new workflow run has been triggered.

Failing Jobs - Building 553bf19

Status	Name	Step	Test failures	Logs	Raw logs
✖	JVM Tests - JDK 11	Build	Test failures	Logs	Raw logs
✖	JVM Tests - JDK 11 Windows	Build	⚠️ Check →	Logs	Raw logs
✖	JVM Tests - JDK 15	Build	Test failures	Logs	Raw logs
✖	MicroProfile TCKs Tests	Verify	Test failures	Logs	Raw logs
✖	Native Tests - Spring	Build	⚠️ Check →	Logs	Raw logs

Full information is available in the Build summary check run.

Test Failures

⚙️ JVM Tests - JDK 11 #

📦 integration-tests/spring-web

✖ io.quarkus.it.spring.web.TestSecurityTest.testPreAuthorizeWithDisabledAuth - More details - Source on GitHub

---

⚙️ JVM Tests - JDK 15 #

📦 integration-tests/spring-web

✖ io.quarkus.it.spring.web.TestSecurityTest.testPreAuthorizeWithDisabledAuth - More details - Source on GitHub

---

⚙️ MicroProfile TCKs Tests #

📦 tcks/resteasy-reactive/target/testsuite/tests

✖ com.sun.ts.tests.jaxrs.spec.filter.interceptor.JAXRSClient0021. - More details - Source on GitHub

[FroMage]

Great!

[geoand]

It seems like the failure in JAXRSClient0021 is actually caused by this...

[geoand]

Fixed

[quarkus-bot]

This workflow status is outdated as a new workflow run has been triggered.

🚫 This workflow run has been cancelled.

Failing Jobs - Building a49d919

⚠️ Artifacts of the workflow run were not available thus the report misses some details.

Status	Name	Step	Test failures	Logs	Raw logs
✖	Devtools Tests - JDK 11 Windows	Build	Test failures	Logs	Raw logs
⌛	JVM Tests - JDK 11	Build	⚠️ Check →	Logs	Raw logs
⌛	JVM Tests - JDK 11 Windows	Build	⚠️ Check →	Logs	Raw logs
⌛	JVM Tests - JDK 15	Build	⚠️ Check →	Logs	Raw logs
✖	MicroProfile TCKs Tests	Verify	Test failures	Logs	Raw logs
✖	Native Tests - Spring	Build	⚠️ Check →	Logs	Raw logs
⌛	Native Tests - gRPC	Build	⚠️ Check →	Logs	Raw logs

Full information is available in the Build summary check run.

Test Failures

⚙️ Devtools Tests - JDK 11 Windows #

📦 integration-tests/devtools

✖ io.quarkus.devtools.codestarts.quarkus.QuarkusCodestartBuildIT.testRunTogetherCodestartsKotlin line 61 - More details - Source on GitHub

---

⚙️ MicroProfile TCKs Tests #

📦 tcks/resteasy-reactive/target/testsuite/tests

✖ com.sun.ts.tests.jaxrs.spec.filter.interceptor.JAXRSClient0021. - More details - Source on GitHub

[quarkus-bot]

Failing Jobs - Building c06abd3

Status	Name	Step	Test failures	Logs	Raw logs
✔️	JVM Tests - JDK 11
✖	JVM Tests - JDK 11 Windows	Build	Test failures	Logs	Raw logs
✔️	JVM Tests - JDK 15

Full information is available in the Build summary check run.

Test Failures

⚙️ JVM Tests - JDK 11 Windows #

📦 extensions/arc/deployment

✖ io.quarkus.arc.test.config.ConfigMappingTest. - More details - Source on GitHub