[fix][client] Shade com.fasterxml.jackson.datatype.* to prevent ClassNotFoundException by massakam · Pull Request #19458 · apache/pulsar

massakam · 2023-02-08T04:26:03Z

Motivation

I tried to use the latest Java client with the authentication plugin, but it threw the following exception:

Exception in thread "main" org.apache.pulsar.client.api.PulsarClientException$UnsupportedAuthenticationException: java.lang.NoClassDefFoundError: org/apache/pulsar/shade/com/fasterxml/jackson/datatype/jdk8/Jdk8Module
        at org.apache.pulsar.client.api.AuthenticationFactory.create(AuthenticationFactory.java:110)
        at org.apache.pulsar.client.impl.ClientBuilderImpl.authentication(ClientBuilderImpl.java:138)
        at SampleConsumer.main(SampleConsumer.java:42)
Caused by: java.lang.NoClassDefFoundError: org/apache/pulsar/shade/com/fasterxml/jackson/datatype/jdk8/Jdk8Module
        at org.apache.pulsar.client.impl.AuthenticationUtil.<clinit>(AuthenticationUtil.java:35)
        at org.apache.pulsar.client.impl.PulsarClientImplementationBindingImpl.createAuthentication(PulsarClientImplementationBindingImpl.java:132)
        at org.apache.pulsar.client.api.AuthenticationFactory.create(AuthenticationFactory.java:108)
        ... 2 more
Caused by: java.lang.ClassNotFoundException: org.apache.pulsar.shade.com.fasterxml.jackson.datatype.jdk8.Jdk8Module
        at java.base/jdk.internal.loader.BuiltinClassLoader.loadClass(BuiltinClassLoader.java:641)
        at java.base/jdk.internal.loader.ClassLoaders$AppClassLoader.loadClass(ClassLoaders.java:188)
        at java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:520)
        ... 5 more

This is because the com.fasterxml.jackson.datatype.* classes are not shaded in pulsar-client.

Modifications

Fixed shade settings to use wildcards to specify Jackson classes to include. This also shades the com.fasterxml.jackson.datatype.* classes.

Verifying this change

Make sure that the change passes the CI checks.

Documentation

doc
doc-required
doc-not-needed
doc-complete

codecov-commenter · 2023-02-08T06:00:50Z

Codecov Report

Merging #19458 (1f18945) into master (5c8f929) will increase coverage by 0.58%.
The diff coverage is 53.67%.

@@             Coverage Diff              @@
##             master   #19458      +/-   ##
============================================
+ Coverage     63.30%   63.88%   +0.58%     
+ Complexity    26123     3491   -22632     
============================================
  Files          1836     1843       +7     
  Lines        134416   135088     +672     
  Branches      14772    14852      +80     
============================================
+ Hits          85087    86305    +1218     
+ Misses        41649    40941     -708     
- Partials       7680     7842     +162

Flag	Coverage Δ
inttests	`24.60% <5.51%> (-0.28%)`	⬇️
systests	`25.37% <0.00%> (-0.19%)`	⬇️
unittests	`61.20% <53.67%> (+0.56%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
...g/apache/pulsar/client/cli/AbstractCmdConsume.java	`20.45% <20.45%> (ø)`
.../java/org/apache/pulsar/client/cli/CmdConsume.java	`40.44% <25.00%> (+8.45%)`	⬆️
...ain/java/org/apache/pulsar/client/cli/CmdRead.java	`41.13% <41.13%> (ø)`
...r/impl/SnapshotSegmentAbortedTxnProcessorImpl.java	`64.28% <64.28%> (ø)`
...ransaction/buffer/impl/TopicTransactionBuffer.java	`74.27% <85.71%> (+11.77%)`	⬆️
...r/service/SystemTopicTxnBufferSnapshotService.java	`78.26% <100.00%> (ø)`
...er/systopic/NamespaceEventsSystemTopicFactory.java	`85.71% <100.00%> (+12.38%)`	⬆️
...ransactionBufferSnapshotBaseSystemTopicClient.java	`72.36% <100.00%> (+4.36%)`	⬆️
...er/impl/SingleSnapshotAbortedTxnProcessorImpl.java	`79.78% <100.00%> (+9.13%)`	⬆️
...er/metadata/v2/TransactionBufferSnapshotIndex.java	`77.77% <100.00%> (+77.77%)`	⬆️
... and 175 more

lhotari

Thanks for the PR, but I think we need to check the annotations compatibility.

lhotari · 2023-02-09T22:05:00Z

-                  <include>com.fasterxml.jackson.module</include>
-                  <include>com.fasterxml.jackson.core:jackson-core</include>
-                  <include>com.fasterxml.jackson.dataformat</include>
+                  <include>com.fasterxml.jackson.*:*</include>


I think that this breaks the usage of Jackson annotations in user compiled classes. The Jackson annotations are part of com.fasterxml.jackson.core:jackson-annotations dependency.
That's perhaps the reason why the wildcard rule was missing before. @massakam do you have a chance to check? An integration test would be nice.

Instead, revert the changes in this pom.xml and add

<include>com.fasterxml.jackson.module:jackson-module-parameter-names</include> <include>com.fasterxml.jackson.datatype:jackson-datatype-jsr310</include> <include>com.fasterxml.jackson.datatype:jackson-datatype-jdk8</include>

That would ensure that Jackson annotations are processed in the same way as before.

For the other pom.xml's the wildcard was already there and it raises some questions how unshaded Jackson annotations would work with shaded Jackson. Perhaps it does?

Related to #6902 . It seems that annotations should be unshaded. For example a Testcontainers comment: testcontainers/testcontainers-java#3972 (comment) . Found it by googling. Another one: https://github.com/hazelcast/hazelcast/blob/master/hazelcast/pom.xml#L158-L162

There's a bug in pulsar-client-admin-shaded/pom.xml and pulsar-client-all/pom.xml since there's already the wildcard for jackson. That doesn't make sense since annotations didn't seem to be shaded for pulsar-client-shaded, but it's shaded for the other 2. I believe that the wildcard shouldn't be used unless there's an exclusion for com.fasterxml.jackson.core:jackson-annotations . That could be a better solution so that it wouldn't be necessary to list all jackson deps. @massakam could you try if that would be a an option.

Letting it be
<include>com.fasterxml.jackson.*:*</include>
but also adding
<exclude>com.fasterxml.jackson.core:jackson-annotations</exclude> to <excludes>. I don't know if that works with maven. Perhaps it shades it if it gets included. It would have to be checked.

@lhotari The group id of jackson-annotations is com.fasterxml.jackson.core, so it seems to be already shaded to pulsar-client, pulsar-client-admin and pulsar-client-all.

@lhotari Excluded jackson-annotation from shading (42c72d1). I confirmed that jars such as pulsar-client no longer include jackson-annotation classes:

click to expand

$ diff -u <(jar tf BEFORE/pulsar-client-2.12.0-SNAPSHOT.jar | grep 'jackson.annotation') <(jar tf AFTER/pulsar-client-2.12.0-SNAPSHOT.jar | grep 'jackson.annotation') --- /dev/fd/63 2023-02-13 16:26:33.652605755 +0900 +++ /dev/fd/62 2023-02-13 16:26:33.652605755 +0900 @@ -1,75 +0,0 @@ -META-INF/maven/com.fasterxml.jackson.core/jackson-annotations/ -META-INF/maven/com.fasterxml.jackson.core/jackson-annotations/pom.properties -META-INF/maven/com.fasterxml.jackson.core/jackson-annotations/pom.xml -org/apache/pulsar/shade/com/fasterxml/jackson/annotation/ -org/apache/pulsar/shade/com/fasterxml/jackson/annotation/JacksonAnnotation.class -org/apache/pulsar/shade/com/fasterxml/jackson/annotation/JacksonAnnotationValue.class -org/apache/pulsar/shade/com/fasterxml/jackson/annotation/JacksonAnnotationsInside.class -org/apache/pulsar/shade/com/fasterxml/jackson/annotation/JacksonInject$Value.class -org/apache/pulsar/shade/com/fasterxml/jackson/annotation/JacksonInject.class -org/apache/pulsar/shade/com/fasterxml/jackson/annotation/JsonAlias.class -org/apache/pulsar/shade/com/fasterxml/jackson/annotation/JsonAnyGetter.class -org/apache/pulsar/shade/com/fasterxml/jackson/annotation/JsonAnySetter.class -org/apache/pulsar/shade/com/fasterxml/jackson/annotation/JsonAutoDetect$1.class -org/apache/pulsar/shade/com/fasterxml/jackson/annotation/JsonAutoDetect$Value.class -org/apache/pulsar/shade/com/fasterxml/jackson/annotation/JsonAutoDetect$Visibility.class -org/apache/pulsar/shade/com/fasterxml/jackson/annotation/JsonAutoDetect.class -org/apache/pulsar/shade/com/fasterxml/jackson/annotation/JsonBackReference.class -org/apache/pulsar/shade/com/fasterxml/jackson/annotation/JsonClassDescription.class -org/apache/pulsar/shade/com/fasterxml/jackson/annotation/JsonCreator$Mode.class -org/apache/pulsar/shade/com/fasterxml/jackson/annotation/JsonCreator.class -org/apache/pulsar/shade/com/fasterxml/jackson/annotation/JsonEnumDefaultValue.class -org/apache/pulsar/shade/com/fasterxml/jackson/annotation/JsonFilter.class -org/apache/pulsar/shade/com/fasterxml/jackson/annotation/JsonFormat$Feature.class -org/apache/pulsar/shade/com/fasterxml/jackson/annotation/JsonFormat$Features.class -org/apache/pulsar/shade/com/fasterxml/jackson/annotation/JsonFormat$Shape.class -org/apache/pulsar/shade/com/fasterxml/jackson/annotation/JsonFormat$Value.class -org/apache/pulsar/shade/com/fasterxml/jackson/annotation/JsonFormat.class -org/apache/pulsar/shade/com/fasterxml/jackson/annotation/JsonGetter.class -org/apache/pulsar/shade/com/fasterxml/jackson/annotation/JsonIdentityInfo.class -org/apache/pulsar/shade/com/fasterxml/jackson/annotation/JsonIdentityReference.class -org/apache/pulsar/shade/com/fasterxml/jackson/annotation/JsonIgnore.class -org/apache/pulsar/shade/com/fasterxml/jackson/annotation/JsonIgnoreProperties$Value.class -org/apache/pulsar/shade/com/fasterxml/jackson/annotation/JsonIgnoreProperties.class -org/apache/pulsar/shade/com/fasterxml/jackson/annotation/JsonIgnoreType.class -org/apache/pulsar/shade/com/fasterxml/jackson/annotation/JsonInclude$Include.class -org/apache/pulsar/shade/com/fasterxml/jackson/annotation/JsonInclude$Value.class -org/apache/pulsar/shade/com/fasterxml/jackson/annotation/JsonInclude.class -org/apache/pulsar/shade/com/fasterxml/jackson/annotation/JsonIncludeProperties$Value.class -org/apache/pulsar/shade/com/fasterxml/jackson/annotation/JsonIncludeProperties.class -org/apache/pulsar/shade/com/fasterxml/jackson/annotation/JsonKey.class -org/apache/pulsar/shade/com/fasterxml/jackson/annotation/JsonManagedReference.class -org/apache/pulsar/shade/com/fasterxml/jackson/annotation/JsonMerge.class -org/apache/pulsar/shade/com/fasterxml/jackson/annotation/JsonProperty$Access.class -org/apache/pulsar/shade/com/fasterxml/jackson/annotation/JsonProperty.class -org/apache/pulsar/shade/com/fasterxml/jackson/annotation/JsonPropertyDescription.class -org/apache/pulsar/shade/com/fasterxml/jackson/annotation/JsonPropertyOrder.class -org/apache/pulsar/shade/com/fasterxml/jackson/annotation/JsonRawValue.class -org/apache/pulsar/shade/com/fasterxml/jackson/annotation/JsonRootName.class -org/apache/pulsar/shade/com/fasterxml/jackson/annotation/JsonSetter$Value.class -org/apache/pulsar/shade/com/fasterxml/jackson/annotation/JsonSetter.class -org/apache/pulsar/shade/com/fasterxml/jackson/annotation/JsonSubTypes$Type.class -org/apache/pulsar/shade/com/fasterxml/jackson/annotation/JsonSubTypes.class -org/apache/pulsar/shade/com/fasterxml/jackson/annotation/JsonTypeId.class -org/apache/pulsar/shade/com/fasterxml/jackson/annotation/JsonTypeInfo$As.class -org/apache/pulsar/shade/com/fasterxml/jackson/annotation/JsonTypeInfo$Id.class -org/apache/pulsar/shade/com/fasterxml/jackson/annotation/JsonTypeInfo$None.class -org/apache/pulsar/shade/com/fasterxml/jackson/annotation/JsonTypeInfo.class -org/apache/pulsar/shade/com/fasterxml/jackson/annotation/JsonTypeName.class -org/apache/pulsar/shade/com/fasterxml/jackson/annotation/JsonUnwrapped.class -org/apache/pulsar/shade/com/fasterxml/jackson/annotation/JsonValue.class -org/apache/pulsar/shade/com/fasterxml/jackson/annotation/JsonView.class -org/apache/pulsar/shade/com/fasterxml/jackson/annotation/Nulls.class -org/apache/pulsar/shade/com/fasterxml/jackson/annotation/ObjectIdGenerator$IdKey.class -org/apache/pulsar/shade/com/fasterxml/jackson/annotation/ObjectIdGenerator.class -org/apache/pulsar/shade/com/fasterxml/jackson/annotation/ObjectIdGenerators$Base.class -org/apache/pulsar/shade/com/fasterxml/jackson/annotation/ObjectIdGenerators$IntSequenceGenerator.class -org/apache/pulsar/shade/com/fasterxml/jackson/annotation/ObjectIdGenerators$None.class -org/apache/pulsar/shade/com/fasterxml/jackson/annotation/ObjectIdGenerators$PropertyGenerator.class -org/apache/pulsar/shade/com/fasterxml/jackson/annotation/ObjectIdGenerators$StringIdGenerator.class -org/apache/pulsar/shade/com/fasterxml/jackson/annotation/ObjectIdGenerators$UUIDGenerator.class -org/apache/pulsar/shade/com/fasterxml/jackson/annotation/ObjectIdGenerators.class -org/apache/pulsar/shade/com/fasterxml/jackson/annotation/ObjectIdResolver.class -org/apache/pulsar/shade/com/fasterxml/jackson/annotation/OptBoolean.class -org/apache/pulsar/shade/com/fasterxml/jackson/annotation/PropertyAccessor.class -org/apache/pulsar/shade/com/fasterxml/jackson/annotation/SimpleObjectIdResolver.class

Excluded jackson-annotation from shading

This causes ConsumerBuilder#subscribe to get stuck in certain environments using jackson-annotation versions earlier than 2.12.0. See #21971 for details.

lhotari · 2023-02-09T22:07:24Z

Related to PIP-243: Register Jackson Java 8 support modules by default / #19243 changes made in #19161 .

lhotari

Good work @massakam!

tisonkun · 2023-07-14T06:54:11Z

This cause java.lang.NoClassDefFoundError: com/fasterxml/jackson/annotation/JsonView in apache/flink-connector-pulsar when bumping to Pulsar 3.0.0 (apache/flink-connector-pulsar#54).

I'm adding back the deps there but don't know if it cause further conflicts - apache/flink-connector-pulsar@dd56bb1

massakam added type/bug The PR fixed a bug or issue reported a bug component/client-java doc-not-needed Your PR changes do not impact docs ready-to-test labels Feb 8, 2023

massakam added this to the 3.0.0 milestone Feb 8, 2023

massakam self-assigned this Feb 8, 2023

lhotari requested review from michaeljmarshall and nodece February 9, 2023 21:50

lhotari requested changes Feb 9, 2023

View reviewed changes

massakam force-pushed the shade-jackson-datatype branch 2 times, most recently from 1f18945 to cbcf5ff Compare February 14, 2023 01:20

Masahiro Sakamoto added 2 commits February 16, 2023 10:27

Shade com.fasterxml.jackson.datatype.* to prevent ClassNotFoundException

6ee6f96

Exclude jackson-annotations from shading

42c72d1

massakam force-pushed the shade-jackson-datatype branch from cbcf5ff to 42c72d1 Compare February 16, 2023 01:28

lhotari approved these changes Feb 16, 2023

View reviewed changes

lhotari merged commit fe547c7 into apache:master Feb 16, 2023

massakam deleted the shade-jackson-datatype branch February 16, 2023 11:25

tisonkun mentioned this pull request Jul 17, 2023

[FLINK-32612] Upgrade Pulsar version to 3.0.0 apache/flink-connector-pulsar#54

Merged

4 tasks

Shawyeok mentioned this pull request Jan 25, 2024

ConsumerBuilderImpl#subscribe stuck when using pulsar-client:3.0.x with jackson-annotations prior to 2.12.0 #21971

Closed

2 tasks

Uh oh!

Conversation

massakam commented Feb 8, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation

Modifications

Verifying this change

Documentation

Uh oh!

codecov-commenter commented Feb 8, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

lhotari left a comment

Choose a reason for hiding this comment

Uh oh!

lhotari Feb 9, 2023

Choose a reason for hiding this comment

Uh oh!

lhotari Feb 9, 2023

Choose a reason for hiding this comment

Uh oh!

lhotari Feb 9, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lhotari Feb 9, 2023

Choose a reason for hiding this comment

Uh oh!

lhotari Feb 9, 2023

Choose a reason for hiding this comment

Uh oh!

massakam Feb 13, 2023

Choose a reason for hiding this comment

Uh oh!

massakam Feb 13, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Shawyeok Jan 25, 2024

Choose a reason for hiding this comment

Uh oh!

lhotari commented Feb 9, 2023

Uh oh!

lhotari left a comment

Choose a reason for hiding this comment

Uh oh!

tisonkun commented Jul 14, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

massakam commented Feb 8, 2023 •

edited

Loading

codecov-commenter commented Feb 8, 2023 •

edited

Loading

lhotari Feb 9, 2023 •

edited

Loading

massakam Feb 13, 2023 •

edited

Loading

tisonkun commented Jul 14, 2023 •

edited

Loading