ArC: fix subclass generation with not visible decorators

[jonasrutishauser]

This fixes several visibility issues in the generated subclass if decorators are not visible from the decorated bean or an other decorator in the chain.
An example error looks like this:

java.lang.IllegalAccessError: failed to access class io.quarkus.arc.test.decorators.NonPublicDecoratorTest$AdditionalConverterDecorator from class io.quarkus.arc.test.decorators.other.ToUpperCaseConverter_Subclass (io.quarkus.arc.test.decorators.NonPublicDecoratorTest$AdditionalConverterDecorator and io.quarkus.arc.test.decorators.other.ToUpperCaseConverter_Subclass are in unnamed module of loader 'app')
	at io.quarkus.arc.test.decorators.other.ToUpperCaseConverter_Subclass.<init>(Unknown Source)
	at io.quarkus.arc.test.decorators.other.ToUpperCaseConverter_Bean.doCreate(Unknown Source)
	at io.quarkus.arc.test.decorators.other.ToUpperCaseConverter_Bean.create(Unknown Source)
	at io.quarkus.arc.test.decorators.other.ToUpperCaseConverter_Bean.create(Unknown Source)
	at io.quarkus.arc.impl.AbstractSharedContext.createInstanceHandle(AbstractSharedContext.java:119)
	at io.quarkus.arc.impl.AbstractSharedContext$1.get(AbstractSharedContext.java:38)
	at io.quarkus.arc.impl.AbstractSharedContext$1.get(AbstractSharedContext.java:35)
	at io.quarkus.arc.impl.LazyValue.get(LazyValue.java:32)
	at io.quarkus.arc.impl.ComputingCache.computeIfAbsent(ComputingCache.java:69)
	at io.quarkus.arc.impl.ComputingCacheContextInstances.computeIfAbsent(ComputingCacheContextInstances.java:19)
	at io.quarkus.arc.impl.AbstractSharedContext.get(AbstractSharedContext.java:35)
	at io.quarkus.arc.impl.ClientProxies.getApplicationScopedDelegate(ClientProxies.java:23)
	at io.quarkus.arc.test.decorators.other.ToUpperCaseConverter_ClientProxy.arc$delegate(Unknown Source)
	at io.quarkus.arc.test.decorators.other.ToUpperCaseConverter_ClientProxy.convert(Unknown Source)
	at io.quarkus.arc.test.decorators.NonPublicDecoratorTest.testDecoration(NonPublicDecoratorTest.java:36)
	at java.base/java.lang.reflect.Method.invoke(Method.java:580)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1596)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1596)

The fix is to use the methods on the decorated types and not the decorator class.

A work around without the fix would be to make the decorator abstract (only works with a single decorator and not with a chain).

[Ladicek]

I take it you use the fact that decorated types are all interfaces, and even abuse that by storing the "next" decorator into a field of type Object, knowing that invokeinterface looks at the runtime class of the object and not the static class? Clever! I'm not 100% sure about this and will have to look deeper, but looks sensible on the first sight.

[gsmet]

@Ladicek friendly ping in case it fell from your radar :)

[Ladicek]

It didn't, though I didn't have the time to look properly yet. I'd also expect @mkouba and @manovotn to take a look :-)

[mkouba]

It didn't, though I didn't have the time to look properly yet. I'd also expect @mkouba and @manovotn to take a look :-)

I have it in my TODO list but it has very low priority given that @Ladicek started to look into it... 😉

[manovotn]

It's on my list as well. It's just that other items crept in... I will try to do it tomorrow though :)

[manovotn]

Just small notes on the tests, otherwise it looks good.

[manovotn]

Just as an FYI for others reviewing this. The failure in this test is different than the one reported in the issue.
Here I was getting:

[ERROR] io.quarkus.arc.test.decorators.NonPublicDecoratorTest.testDecoration -- Time elapsed: 0.235 s <<< ERROR!
java.lang.IllegalArgumentException: Must be a Class or String, got T
at io.quarkus.gizmo.DescriptorUtils.objectToDescriptor(DescriptorUtils.java:133)
at io.quarkus.gizmo.MethodDescriptor.ofMethod(MethodDescriptor.java:98)
at io.quarkus.arc.processor.SubclassGenerator.processDecorator(SubclassGenerator.java:770)
at io.quarkus.arc.processor.SubclassGenerator.createConstructor(SubclassGenerator.java:206)
at io.quarkus.arc.processor.SubclassGenerator.generate(SubclassGenerator.java:126)
at io.quarkus.arc.processor.BeanProcessor.generateResources(BeanProcessor.java:480)
at io.quarkus.arc.processor.BeanProcessor.process(BeanProcessor.java:583)
at io.quarkus.arc.test.ArcTestContainer.init(ArcTestContainer.java:503)
at io.quarkus.arc.test.ArcTestContainer.beforeEach(ArcTestContainer.java:342)
at java.base/java.util.ArrayList.forEach(ArrayList.java:1596)
at java.base/java.util.ArrayList.forEach(ArrayList.java:1596)

[manovotn]

I have added a backport label as I believe this could be worth considering but I will wait for what Martin and Lada say.

While this solution works, it might break some other cases we don't have covered although I haven't thought of any so far.

[mkouba]

Looks good but I'd like to run the SingleDecoratorBenchmark, just in case...

[Ladicek]

LGTM. I added an extra commit with a comment about decorators being interface-based and hence the decorator field can always be Object, but other than that, I have nothing to add. Great work here!

[quarkus-bot]

Status for workflow Quarkus CI

This is the status report for running Quarkus CI on commit 6e15235.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

You can consult the Develocity build scans.