Fix #1017 Do not hardcode the TransformerFactory implementation irrev… #1018
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
oscerd
approved these changes
Apr 2, 2020
lburgazzoli
reviewed
Apr 2, 2020
| * at the application startup. | ||
| */ | ||
| @ConfigItem(defaultValue = "org.apache.camel.quarkus.support.xalan.XalanTransformerFactory") | ||
| public Optional<String> transformerFactory; |
There was a problem hiding this comment.
We don't provide any substitution for other transformers so it should be clear that any value except the default is out of our support
There was a problem hiding this comment.
btw it probably does not need to be Optional as it has a default value
There was a problem hiding this comment.
it probably does not need to be Optional
Maybe. I was not sure what
camel.xalan.transformer-factory =
would do. I'd naively expect it to be null/nonPresent. I should test it.
There was a problem hiding this comment.
We don't provide any substitution for other transformers so it should be clear that any value except the default is out of our support
If you mean the text should be different, plz. propose a change.
There was a problem hiding this comment.
I don't have one in mind but with this seems to indicate one can swap the implementation to whatever transformer factory and that's all that is needed but that may not be true as the user has to take care of any substitution that may be needed
There was a problem hiding this comment.
I have tested with
quarkus.camel.xalan.transformer-factory =
and it makes the Optional.isPresent() to return false.
There was a problem hiding this comment.
what if you remove optional ? does it get the default ?
There was a problem hiding this comment.
what if you remove optional ? does it get the default ?
Strange enough it is neither null or default: it throws an exception
java.util.NoSuchElementException: Property quarkus.camel.xalan.transformer-factory not found
at io.quarkus.test.junit.QuarkusTestExtension.beforeEach(QuarkusTestExtension.java:191)
at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.lambda$invokeBeforeEachCallbacks$1(TestMethodTestDescriptor.java:154)
at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.lambda$invokeBeforeMethodsOrCallbacksUntilExceptionOccurs$5(TestMethodTestDescriptor.java:190)
at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.invokeBeforeMethodsOrCallbacksUntilExceptionOccurs(TestMethodTestDescriptor.java:190)
at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.invokeBeforeEachCallbacks(TestMethodTestDescriptor.java:153)
at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.execute(TestMethodTestDescriptor.java:131)
at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.execute(TestMethodTestDescriptor.java:71)
at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$5(NodeTestTask.java:135)
at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$7(NodeTestTask.java:125)
at org.junit.platform.engine.support.hierarchical.Node.around(Node.java:135)
at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$8(NodeTestTask.java:123)
at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
at org.junit.platform.engine.support.hierarchical.NodeTestTask.executeRecursively(NodeTestTask.java:122)
at org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:80)
at java.base/java.util.ArrayList.forEach(ArrayList.java:1540)
at org.junit.platform.engine.support.hierarchical.SameThreadHierarchicalTestExecutorService.invokeAll(SameThreadHierarchicalTestExecutorService.java:38)
at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$5(NodeTestTask.java:139)
at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$7(NodeTestTask.java:125)
at org.junit.platform.engine.support.hierarchical.Node.around(Node.java:135)
at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$8(NodeTestTask.java:123)
at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
at org.junit.platform.engine.support.hierarchical.NodeTestTask.executeRecursively(NodeTestTask.java:122)
at org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:80)
at java.base/java.util.ArrayList.forEach(ArrayList.java:1540)
at org.junit.platform.engine.support.hierarchical.SameThreadHierarchicalTestExecutorService.invokeAll(SameThreadHierarchicalTestExecutorService.java:38)
at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$5(NodeTestTask.java:139)
at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$7(NodeTestTask.java:125)
at org.junit.platform.engine.support.hierarchical.Node.around(Node.java:135)
at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$8(NodeTestTask.java:123)
at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
at org.junit.platform.engine.support.hierarchical.NodeTestTask.executeRecursively(NodeTestTask.java:122)
at org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:80)
at org.junit.platform.engine.support.hierarchical.SameThreadHierarchicalTestExecutorService.submit(SameThreadHierarchicalTestExecutorService.java:32)
at org.junit.platform.engine.support.hierarchical.HierarchicalTestExecutor.execute(HierarchicalTestExecutor.java:57)
at org.junit.platform.engine.support.hierarchical.HierarchicalTestEngine.execute(HierarchicalTestEngine.java:51)
at org.junit.platform.launcher.core.DefaultLauncher.execute(DefaultLauncher.java:220)
at org.junit.platform.launcher.core.DefaultLauncher.lambda$execute$6(DefaultLauncher.java:188)
at org.junit.platform.launcher.core.DefaultLauncher.withInterceptedStreams(DefaultLauncher.java:202)
at org.junit.platform.launcher.core.DefaultLauncher.execute(DefaultLauncher.java:181)
at org.junit.platform.launcher.core.DefaultLauncher.execute(DefaultLauncher.java:128)
at org.apache.maven.surefire.junitplatform.JUnitPlatformProvider.invokeAllTests(JUnitPlatformProvider.java:150)
at org.apache.maven.surefire.junitplatform.JUnitPlatformProvider.invoke(JUnitPlatformProvider.java:124)
at org.apache.maven.surefire.booter.ForkedBooter.invokeProviderInSameClassLoader(ForkedBooter.java:384)
at org.apache.maven.surefire.booter.ForkedBooter.runSuitesInProcess(ForkedBooter.java:345)
at org.apache.maven.surefire.booter.ForkedBooter.execute(ForkedBooter.java:126)
at org.apache.maven.surefire.booter.ForkedBooter.main(ForkedBooter.java:418)
Caused by: java.lang.RuntimeException: java.util.NoSuchElementException: Property quarkus.camel.xalan.transformer-factory not found
at io.quarkus.runner.bootstrap.AugmentActionImpl.runAugment(AugmentActionImpl.java:206)
at io.quarkus.runner.bootstrap.AugmentActionImpl.createInitialRuntimeApplication(AugmentActionImpl.java:95)
at io.quarkus.runner.bootstrap.AugmentActionImpl.createInitialRuntimeApplication(AugmentActionImpl.java:45)
at io.quarkus.test.junit.QuarkusTestExtension.doJavaStart(QuarkusTestExtension.java:96)
at io.quarkus.test.junit.QuarkusTestExtension.ensureStarted(QuarkusTestExtension.java:205)
at io.quarkus.test.junit.QuarkusTestExtension.beforeAll(QuarkusTestExtension.java:228)
at org.junit.jupiter.engine.descriptor.ClassBasedTestDescriptor.lambda$invokeBeforeAllCallbacks$7(ClassBasedTestDescriptor.java:359)
at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
at org.junit.jupiter.engine.descriptor.ClassBasedTestDescriptor.invokeBeforeAllCallbacks(ClassBasedTestDescriptor.java:359)
at org.junit.jupiter.engine.descriptor.ClassBasedTestDescriptor.before(ClassBasedTestDescriptor.java:189)
at org.junit.jupiter.engine.descriptor.ClassBasedTestDescriptor.before(ClassBasedTestDescriptor.java:78)
at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$5(NodeTestTask.java:132)
... 31 more
Caused by: java.util.NoSuchElementException: Property quarkus.camel.xalan.transformer-factory not found
at io.smallrye.config.SmallRyeConfig.propertyNotFound(SmallRyeConfig.java:209)
at io.smallrye.config.SmallRyeConfig.getValue(SmallRyeConfig.java:96)
at io.quarkus.deployment.configuration.BuildTimeConfigurationReader$ReadOperation.readConfigValue(BuildTimeConfigurationReader.java:616)
at io.quarkus.deployment.configuration.BuildTimeConfigurationReader$ReadOperation.readConfigGroup(BuildTimeConfigurationReader.java:561)
at io.quarkus.deployment.configuration.BuildTimeConfigurationReader$ReadOperation.run(BuildTimeConfigurationReader.java:284)
at io.quarkus.deployment.configuration.BuildTimeConfigurationReader.readConfiguration(BuildTimeConfigurationReader.java:241)
at io.quarkus.deployment.ExtensionLoader.loadStepsFrom(ExtensionLoader.java:204)
at io.quarkus.deployment.ExtensionLoader.loadStepsFrom(ExtensionLoader.java:136)
at io.quarkus.deployment.QuarkusAugmentor.run(QuarkusAugmentor.java:93)
at io.quarkus.runner.bootstrap.AugmentActionImpl.runAugment(AugmentActionImpl.java:204)
... 42 more
lburgazzoli
reviewed
Apr 2, 2020
| * wins over our factory on Java 11+ native | ||
| * I any case, the user has an option to pass his preferred factory instead of ours | ||
| */ | ||
| new SystemPropertyBuildItem("javax.xml.transform.TransformerFactory", val))); |
There was a problem hiding this comment.
It was an issue even before but this does not apply to JVM so we could have different behavior between native and jvm
There was a problem hiding this comment.
this does not apply to JVM
It does. There is NativeImageSystemPropertyBuildItem that applies to native only.
There was a problem hiding this comment.
ah sorry I misread it, my bad
ppalaga
commented
Apr 2, 2020
Comment on lines
+29
to
+30
| * A fully qualified class name to set as the {@code javax.xml.transform.TransformerFactory} system property early | ||
| * at the application startup. |
There was a problem hiding this comment.
How about this new variant of the text?
Suggested change
| * A fully qualified class name to set as the {@code javax.xml.transform.TransformerFactory} system property early | |
| * at the application startup. | |
| * A fully qualified class name to set as the {@code javax.xml.transform.TransformerFactory} system property early | |
| * at the application startup. The system property effectively overrides any service providers defined in | |
| * {@code META-INF/services/javax.xml.transform.TransformerFactory} files available in the class path. If you do not | |
| * set the option the default value is used and the service providers are overriden anyway. | |
| * <p> | |
| * Note that any custom transformer factory you pass will only work in native mode if all necessary classes are | |
| * registered for reflection and all necessary resources are included in the native image. This may already be the | |
| * case for {@code com.sun.org.apache.xalan.internal.xsltc.trax.TransformerFactoryImpl} if you depend on | |
| * {@code io.quarkus:quarkus-jaxb} or {@code org.apache.xalan.xsltc.trax.TransformerFactoryImpl} if you depend on | |
| * {@code org.apache.camel.quarkus:camel-quarkus-support-xalan}. |
lburgazzoli
approved these changes
Apr 2, 2020
|
Even better config docs in 349782f |
|
Fixed trailing whitespace in 349782f |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fix #1017