Remove UnbannedReflectiveBuildItem + policy extension ?

[johnpoth]

When a component uses the camel-api-component-maven-plugin (olingo4, box, fhir, zendesk etc...) one has to register the Configuration classes for reflection using ReflectiveClassBuildItem.
With the Policy extension, one also has to register the same classes with UnbannedReflectiveBuildItem or else the build fails.

Since these are valid uses of ReflectiveClassBuildItem I really don't think the build should fail.

[ppalaga]

We currently ban classes annotated with @org.apache.camel.spi.UriParams that actually should not need reflection, because there are means on the Camel site how to avoid it. If those means do not work, one should file a Camel issue or even better fix it on the Camel side. Unbanning should be seen only as a temporary measure to make things work. Registering for reflection comes at some cost (image size, runtime efficiency) and we want to avoid it whereever possible.

[lburgazzoli]

I'd say that we should trust a little extensions developers and thus evaluation should be done mainly through the PR review process.

[johnpoth]

I'm not sure I understand, some classes annotated with @org.apache.camel.spi.UriParams do need reflection, so why make the build fail ?

[oscerd]

Because on the camel side there were efforts to avoid reflection and there are multiple commits where this is visible on each component. It wasn't possible to do this on all the components btw, so I agree the build should not fail.

[johnpoth]

I agree with @lburgazzoli, @oscerd
Build failures should really only occur in critical situations. IMO this hurts the overall developer experience here

[ppalaga]

I am sorry for the inconvenience, but I still think that registering a class for reflection that belongs to a well known group that is expected not to need it, should not go without checking it properly.

Speaking for me, I do not feel able to eval during the review by pure sight whether some Camel class has some specific annotation. I think the policy does some useful work there. If we really want to tackle this, we should keep the policy.

Going through tens of extensions manually afterwards to check whether we register for reflection only what is necessary is a lot of work, that I did twice and I would not wish anybody to that again.

However, if we actually do not have the ambition to do this properly, I won't be against removing the policy.

By no means this inconvenience can impact Camel Quarkus application developers unless they consciously depend on the policy extension. This is only the case in our integration tests. Failing is the right thing to do there because what does not fail the build gets ignored.

[jamesnetherton]

As a compromise, can we not just log a message at WARN or DEBUG level if UriParams stuff is registered for reflection? At least that'll give an easy way to track stuff down for investigation.

[lburgazzoli]

I think the warning is the right thing as it also avoid to keep a list of white listed classes (that needs to be maintained)

[ppalaga]

OK, accepting that the policy is not seen as useful by the majority and thus also switching to passive mode for the solving of the underlying problem.

[davsclaus]

All the Camel components now do not use reflection when configuring themselves. We have those generated Configurer classes today. Today we fixed one (hopefully last) issue in the api components that @jamesnetherton discovered. Thats fixed in Camel 3.5 onwards (backported to 3.4.3 also)