Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unclear integration of Multi-Release Jar of jackson in cucumber-core #2786

Closed
holzig opened this issue Aug 23, 2023 · 2 comments · Fixed by #2793
Closed

Unclear integration of Multi-Release Jar of jackson in cucumber-core #2786

holzig opened this issue Aug 23, 2023 · 2 comments · Fixed by #2793

Comments

@holzig
Copy link

holzig commented Aug 23, 2023

🤔 What's the problem you've observed?

I think that the Multi Release jar related jackson classes are not included correctly in the cucumber-core module. As far as I can see, is the maven-shade plugin used by cucumber-core to include the jackson classes into the own jar under a different package. But jackson-core is a Multi Release jar and the corresponding classes reside in the META-INF/versions directory inside the jar. These classes are also "shaded" into the cucumber-core jar. Decompiling the classes shows that the package declarion has changed to the new subpackage. But inside the jar they still reside in the old directory/package "com.faster.xml.jackson.core.io.doubleparser". Also the "Multi-Release: true" property inside the MANIFEST.MF file is missing so that even if they classes would exist in the correct directory the "MultiRelease" features would now work as expected.

Why is this a problem / How did we notice it?

I am sure jackson has its reason for supporting MRJAR which is effectively negated by the way it is integrated into cucumber. We only noticed this because we have the enforcer plugin checking for duplicate classes in a projects that has a dependency on cucumber (7.13.0) and jackson (2.15.2).

Executing mvn clean verify a project with the following simplified example pom shows that the maven enforcer plugin does detect duplicate classes, which should, if I understand it correctly not be duplicated.

<?xml version="1.0" encoding="UTF-8"?>
<project xmlns="http://maven.apache.org/POM/4.0.0"
         xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
         xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
    <modelVersion>4.0.0</modelVersion>

    <groupId>org.example</groupId>
    <artifactId>cucumber-jackson-reproducer</artifactId>
    <version>1.0-SNAPSHOT</version>
    <packaging>jar</packaging>

    <properties>
        <maven.compiler.source>17</maven.compiler.source>
        <maven.compiler.target>17</maven.compiler.target>
        <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
    </properties>

    <dependencies>
        <dependency>
            <groupId>com.fasterxml.jackson.core</groupId>
            <artifactId>jackson-core</artifactId>
            <version>2.15.2</version>
        </dependency>
        <dependency>
            <groupId>io.cucumber</groupId>
            <artifactId>cucumber-core</artifactId>
            <version>7.13.0</version>
        </dependency>
    </dependencies>
    <build>
        <pluginManagement>
            <plugins>
                <plugin>
                    <groupId>org.apache.maven.plugins</groupId>
                    <artifactId>maven-enforcer-plugin</artifactId>
                    <version>3.4.0</version>
                    <executions>
                        <execution>
                            <goals>
                                <goal>enforce</goal>
                            </goals>
                            <phase>validate</phase>
                        </execution>
                    </executions>
                    <dependencies>
                        <dependency>
                            <groupId>org.codehaus.mojo</groupId>
                            <artifactId>extra-enforcer-rules</artifactId>
                            <version>1.3</version>
                        </dependency>
                    </dependencies>
                </plugin>
            </plugins>
        </pluginManagement>
        <plugins>
            <plugin>
                <groupId>org.apache.maven.plugins</groupId>
                <artifactId>maven-enforcer-plugin</artifactId>
                <executions>
                    <execution>
                        <goals>
                            <goal>enforce</goal>
                        </goals>
                    </execution>
                </executions>
                <configuration>
                    <rules>
                        <banDuplicateClasses>
                            <findAllDuplicates>true</findAllDuplicates>
                        </banDuplicateClasses>
                    </rules>
                </configuration>
            </plugin>
        </plugins>
    </build>

</project>

We could configure the enforcer to exclude the related classes but that does not change our impression that something is not quite right.

                        <banDuplicateClasses>
                            <ignoreClasses>
                                <ignoreClass>META-INF/versions/*/com/fasterxml/jackson/core/io/doubleparser/*</ignoreClass>
                            </ignoreClasses>
                            <findAllDuplicates>true</findAllDuplicates>
                        </banDuplicateClasses>

✨ Do you have a proposal for making it better?

If the shading of jackson is a requirement I think there are two options.

  1. Stop shading / including the "multi release" related classes of jackson
    • e.g. by excluding the META-INF/versions directory from the shade plug
  2. Do the shading correctly
    • I am unsure how the wrong package directory could be solved
    • Regarding the "Multi-Release" property this issue of the shade plugin might be helpful

I am unsure if this is really a problem of cucumber or the shade plugin or jackson or a combination of all but maybe you can say something about this. That is the reason why I created a "Developer experience" issues instead of a bug.

📚 Any additional context?


This text was originally generated from a template, then edited by hand. You can modify the template here.

@mpkorstanje
Copy link
Contributor

mpkorstanje commented Aug 23, 2023

Your impression is correct. As a test framework, Cucumber shouldn't interfere with the system under test. So Jackson is shaded in to avoid just that.

Unfortunately Jackson is a moving target, and rather quick at that. We've had comparable problems in the past e.g. #2620.

I would welcome a PR that resolves this. Unfortunately I can't currently investigate the issue in more detail. The correct solution however would ensure that the shaded Jackson works for Cucumber -- which uses a rather limited feature set -- so excluding META-INF/versions could be a solution depending on the functionality provided by META-INF/versions .

It would also be nice if we could configure something to check this doesn't happen in the future.

@nhojpatrick
Copy link
Member

I'm getting issues with Jackson v2.15.2 on another project, but v2.15.1 works fine.
Can you try again with jackson v2.15.1 and see if that fixes the issues.

mpkorstanje added a commit that referenced this issue Sep 9, 2023
Cucumber-JVM uses Jackson Databind to write json. Because Jackson is a very
common dependency, to ensure we do not interfere with the system under test,
we shade it.

Unfortunately the Shade plugin occasionally gets updates and now includes
multi-releases. These were not included in the relocation pattern and
result in duplicate classes when Cucumber itself is shaded.

As `cucumber-core` is not a multi-release jar, we can exclude the multi-
release files altogether.

Fixes: #2786
mpkorstanje added a commit that referenced this issue Sep 9, 2023
Cucumber-JVM uses Jackson Databind to write json. Because Jackson is a very
common dependency, to ensure we do not interfere with the system under test,
we shade it.

Unfortunately the Shade plugin occasionally gets updates and now includes
multi-releases. These were not included in the relocation pattern and
result in duplicate classes when Cucumber itself is shaded.

As `cucumber-core` is not a multi-release jar, we can exclude the multi-
release files altogether.

Fixes: #2786
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants