TIBCOSoftware / jasperreports

JasperReports® - Free Java Reporting Library
https://community.jaspersoft.com/downloads/community-edition/
GNU Lesser General Public License v3.0
1.04k stars 401 forks source link

6.20.2 missing openpdf dependency #351

Closed Viserius closed 1 year ago

Viserius commented 1 year ago

After updating our version from

        <dependency>
            <groupId>net.sf.jasperreports</groupId>
            <artifactId>jasperreports</artifactId>
            <version>6.20.1</version>
        </dependency>

to

        <dependency>
            <groupId>net.sf.jasperreports</groupId>
            <artifactId>jasperreports</artifactId>
            <version>6.20.2</version>
        </dependency>

We obtain the following exception:

java.lang.NoClassDefFoundError: com/lowagie/text/DocumentException
    at net.sf.jasperreports.export.pdf.classic.ClassicPdfProducerFactory.createProducer(ClassicPdfProducerFactory.java:44)
    at net.sf.jasperreports.engine.export.JRPdfExporter.createPdfProducer(JRPdfExporter.java:821)
    at net.sf.jasperreports.engine.export.JRPdfExporter.initExport(JRPdfExporter.java:716)
    at net.sf.jasperreports.engine.export.JRPdfExporter.exportReport(JRPdfExporter.java:677)
    at net.sf.jasperreports.engine.JasperExportManager.exportToPdf(JasperExportManager.java:217)
    at net.sf.jasperreports.engine.JasperExportManager.exportReportToPdf(JasperExportManager.java:542)

After executing mvn dependency:tree, we see on v6.20.1:

[INFO] +- net.sf.jasperreports:jasperreports:jar:6.20.1:compile
[INFO] |  +- commons-beanutils:commons-beanutils:jar:1.9.4:compile
[INFO] |  |  \- commons-collections:commons-collections:jar:3.2.2:compile
[INFO] |  +- commons-digester:commons-digester:jar:2.1:compile
[INFO] |  +- commons-logging:commons-logging:jar:1.1.1:compile
[INFO] |  +- com.github.librepdf:openpdf:jar:1.3.30.jaspersoft.1:compile
[INFO] |  +- org.jfree:jcommon:jar:1.0.23:compile
[INFO] |  +- org.jfree:jfreechart:jar:1.0.19:compile
[INFO] |  +- org.eclipse.jdt:ecj:jar:3.21.0:compile
[INFO] |  +- com.fasterxml.jackson.core:jackson-core:jar:2.13.5:compile
[INFO] |  +- com.fasterxml.jackson.core:jackson-databind:jar:2.13.5:compile
[INFO] |  +- com.fasterxml.jackson.core:jackson-annotations:jar:2.13.5:compile
[INFO] |  \- com.fasterxml.jackson.dataformat:jackson-dataformat-xml:jar:2.13.5:compile
[INFO] |     +- org.codehaus.woodstox:stax2-api:jar:4.2.1:compile
[INFO] |     \- com.fasterxml.woodstox:woodstox-core:jar:6.4.0:compile

Whereas on version v6.20.2:

[INFO] +- net.sf.jasperreports:jasperreports:jar:6.20.2:compile
[INFO] |  +- commons-beanutils:commons-beanutils:jar:1.9.4:compile
[INFO] |  |  \- commons-collections:commons-collections:jar:3.2.2:compile
[INFO] |  +- commons-digester:commons-digester:jar:2.1:compile
[INFO] |  +- commons-logging:commons-logging:jar:1.1.1:compile
[INFO] |  +- org.jfree:jcommon:jar:1.0.23:compile
[INFO] |  +- org.jfree:jfreechart:jar:1.0.19:compile
[INFO] |  +- org.eclipse.jdt:ecj:jar:3.21.0:compile
[INFO] |  +- com.fasterxml.jackson.core:jackson-core:jar:2.13.5:compile
[INFO] |  +- com.fasterxml.jackson.core:jackson-databind:jar:2.13.5:compile
[INFO] |  +- com.fasterxml.jackson.core:jackson-annotations:jar:2.13.5:compile
[INFO] |  \- com.fasterxml.jackson.dataformat:jackson-dataformat-xml:jar:2.13.5:compile
[INFO] |     +- org.codehaus.woodstox:stax2-api:jar:4.2.1:compile
[INFO] |     \- com.fasterxml.woodstox:woodstox-core:jar:6.4.0:compile

Note: the dependency com.github.librepdf:openpdf:jar:1.3.30.jaspersoft.1:compile has been removed in version v6.20.2. To resolve the error, this dependency (but then version 1.3.30.jaspersoft.2) should be added back in.

Viserius commented 1 year ago

Pinpointed the breaking change to this commit https://github.com/TIBCOSoftware/jasperreports/commit/100b25889ca86d1475c7facd6f332d3071104d3e

Removing a required dependency introduces a breaking change and should not be introduced in a hotfix release, especially not considering it is not mentioned in the release notes, nor does it provide instructions that users should include this dependency themselves.

@teodord Can we make this dependency non-optional again?

teodord commented 1 year ago

@Viserius,

Can you add the OpenPDF dependency in your own application build, where you already have the JasperReports dependency? Make sure you add the following repository where we keep our fork of OpenPDF until they accept the fix we need from them: https://github.com/TIBCOSoftware/jasperreports/blob/master/jasperreports/pom-parent.xml#L100

Thank you, Teodor

Viserius commented 1 year ago

Thank you for the quick reply @teodord.

While I was able to resolve the issue by adding the OpenPDF dependency myself, I am disappointed that this breaking change was introduced in a minor/patch update without any mention in the installation guide or changelog. This caused us to change our project structure to accommodate this update, which we believe is not a good practice. Moreover, it seems that quite a few other users did not expect this change, or were not informed that this required action on their part (#347 #348 @yu-shiba @DManstrator).

I strongly suggest that the developers follow the semantic versioning (semver.org) guidelines and consider reverting this change and reintroducing it in a major version like 7.0.0. Additionally, it is essential to communicate such changes in the changelog and set-up instructions to prevent confusion for users who may need to package OpenPDF themselves. Especially since a non-standard OpenPDF version is required, this change is highly unexpected and I would like to ask the maintainers to announce or communicate such changes clearly in the future.

teodord commented 1 year ago

@Viserius

Should I also make Batik dependencies mandatory? There are 44 optional dependencies in JasperReports today. Should they all be mandatory? I am curious if in your application build you have Batik dependencies referenced directly, or you rely on them being brought in transiently. If you do have Batik referenced directly, how did you know you need it? If you don't have Batik, how do you know your reports would not crash one day because you are missing it?

I want to learn.

Thank you, Teodor

Viserius commented 1 year ago

@teodord

I see your point, and I agree with you that if you have such a large application with many optional use cases that are not always used by everyone, you do not want to package it all in one super fat JAR. My point primarily being that it was removed in a patch version without notice.

Still, I completely understand the dilemma. On the one hand, you do not want to package many JARs you do not use so your JAR becomes very large. On the other hand, requiring users to import the optional dependencies themselves without knowing what exactly they need, can also be confusing. I believe a common solution that is being applied to similar situations is to apply the microkernel design pattern. This would mean you'd not have 1 Jasperreports maven dependency that contains everything, but the basic jasperreports dependency is small with core functionality. By including extra plug-in jasperreports modules, you enable additional functionality. I.e., jasperreports-core, jasperreports-pdf, jasperreports-svg etc. This is one possible method of how you can be small, while being easily extendable at the same time. Another benefit is that you completely control the underlying dependencies being imported by the various module (i.e. the proprietary openpdf version), so users are not required to know if they need version 1.3.30, 1.3.30.jaspersoft.1, 1.3.30.jaspersoft.1, or 1.3.31 etc.

teodord commented 1 year ago

We are going to consider splitting JasperReports into smaller jars.

For now, this tracker remains open so that others encountering the same problem can see what the solution is. We are not going to revert the change that makes OpenPDF optional, especially since the damage is done and once everybody takes action, we are not going to hear about in the future.

Thank you, Teodor

soc commented 1 year ago

@Viserius I agree with your concern, though I also believe that JasperReports authors can run their project any way they like (that includes "unprofessional").

teodord commented 1 year ago

I want to solve this in a professional way, so please help me achieve it.

Since there is no way to take the release back from Maven repositories, you want us to do either of the following, or both:

1) I apologise to everyone and ask for forgiveness.

2) Make a new release, probably called 6.20.4, in which we put OpenPDF dependency back to mandatory, so that when you upgrade to it, your build systems remain intact;

2.a) If we ever want to make OpenPDF optional, we should only do it again in a 7.0.0 release, but announce it to everybody in every possible way we can, so that there is no surprise to anyone that they need to add OpenPDF and the JFrog repository to their builds. The error you encountered this time around would come again, but at least you would not be able to complain you did not know about it (you probably wont log this again, because you read the release notes and announcements, but I'm sure there would be others who do not read these things and would log it again anyway, so we would still need to put some fires out by telling them it was properly announced, they just did not pay attention to it).

2.b) We never ever attempt to make OpenPDF optional again.

I am doing 1) right now: Please accept my apologies for the mistake I have done when publishing 6.20.3 and made OpenPDF optional without properly announcing it. Please forgive me and I promise it will not happen again.

Please let me know how would you like us to proceed with item 2).

Thank you, Teodor

Viserius commented 1 year ago

Dear @teodord ,

Thank you for your prompt response and for the constructive discussion. I appreciate that you take this issue seriously and want to address it in a professional manner.

I understand that I could have added the missing dependency myself, but my intention was to help you and the project by highlighting the potential problems this could cause for other users who rely on PDF exporting. This was also supported by other open issues.

I want to emphasize that the decision to keep or remove the dependency in a breaking release is entirely up to the maintainers, and I don't want to provide any unsolicited advice on this matter. Still, introducing breaking changes is sometimes inevitable and even a good decision, as long as the user does not need to resort to issue trackers for instructions on how to upgrade. Applying these changes in a major version (such as 7.0.0) helps to signal to the user that updating to the new version might break their code and that action on their part is required. Often, users consult the changelog for major releases, but not so often for patch updates.

In the end, we are all trying to do our work the best we can, and sometimes mistakes happen. Thank you again for your prompt and thoughtful response. We like and appreciate the jasperreports project, and are thankful we can use it to generate PDFs.

Best regards, Mark

DManstrator commented 1 year ago

I also want to thank you for the response and the discussion.

A release 6.20.4 with OpenPDF being provided again seems a great solution to me (for now).

About your second item:

In my opinion, making OpenPDF optional again in the next major seems completely fair to me. As you argued multiple times already, Jasper can also work without a pdf export so it seems fair that it will be removed again, this time with notice and within a major update.

The only problem most of us had were that you made that breaking change unnoticed and with a patch release. As already explained by @Viserius, a major update indicates that there may be breaking changes with the release where a patch update is most likely auto-merged by most users.

Another issue regarding OpenPDF is that you use a special version of it (as you have explained in https://github.com/TIBCOSoftware/jasperreports/issues/339#issuecomment-1480703665). This makes it even harder for users since they not only need to add OpenPDF but also an additional repository to be able to use it. I hope you can resolve that in the near future so when you potentially remove it with version 7.0.0, users only need to add OpenPDF in the latest public version.

soc commented 1 year ago

I think the core concern is that the approach to handle the iText/OpenPDF dependency is simply not working out: In fact more than 15% of all bugs ever filed in this repo contain either "itext" or "openpdf".

We have gone from "this uses some custom iText", "this uses iText, but you can substitute OpenPDF", "this uses OpenPDF", "this uses a custom OpenPDF fork, but you can substitute the upstream one" to "this uses a custom OpenPDF fork, and you cannot substitute the upstream one" and every time stuff needlessly broke.

I don't even care about optional vs. non-optional at this point, it's just another bit of churn.

It simply doesn't have to be this way, and I don't know any other project that suffers from this problem this much.

There is a solution that has been pointed out a few times in the past: When you publish a JasperReports version to Maven Central, publish all of it to Maven Central (including your custom OpenPDF if you need it, under the Jasper GroupId).

I don't even understand why/how Maven Central accepts artifact uploads with missing dependencies in the first place, but this doesn't mean it has to be used, especially if it keeps creating problems for everyone.

teodord commented 1 year ago

Feel free to write me directly at teodord@users.sourceforge.net about this topic or any other topic related to this troublesome project.

Thank you, Teodor

yu-shiba commented 1 year ago

Hello to all JasperReports lovers. I have raised a similar issue in #347.

It is useful to be able to discuss ideas to avoid problems caused by software upgrades.

I hope the dependency on OpenPDF will return in 6.20.4. That way we can at least avoid the possibility that future patch releases will break the build. And options 2-a and 2-b, I think it would be desirable to have both options. For example, the following configuration

Artifacts Contents
jasperreports same as 6.x (OpenPDF not optional), perhaps BOM only
jasperreports-core common to all dependencies
jasperreports-pdf PDF features only, include OpenPDF dependencies
jasperreports-html HTML features only
jasperreports-office Office features only (.xls)
jasperreports-ooxml Office Open XML feature only (.xlsx)
jasperreports-odf OpenDocument Format features only

I think this configuration can maintain backward compatibility and optimize library dependencies.

teodord commented 1 year ago

The release 6.20.4 was published to put back mandatory OpenPDF dependency in pom.xml.

gourigs123 commented 1 year ago

I have been trying to upgrade jasper report version from 6.12.2->6.20.2 but facing session destroyed and please let me know how to add openpdf dependency is this rite dependencyResolutionManagement { versionCatalogs { commonLibs { version('jasperreports.version', '6.15.0') version('itext.version','7.1.16')

                alias('jasperreports-metadata').to('net.sf.jasperreports','jasperreports-metadata').versionRef('jasperreports.version')
                alias('jasperreports-annotation-processors').to('net.sf.jasperreports','jasperreports-annotation-processors').versionRef('jasperreports.version')
                alias('jasperreports-fonts').to('net.sf.jasperreports','jasperreports-fonts').versionRef('jasperreports.version')
                alias('jasperreports').to('net.sf.jasperreports','jasperreports').versionRef('jasperreports.version')
                alias('itext').to('com.itextpdf:itext7-core').versionRef('itext.version')
                }

                alias('itext').to('com.itextpdf','itext7-core').versionRef('itext.version')
                alias('openpdf').to('com.github.librepdf','openpdf').versionRef('openpdf.version')

                version('itext.version','7.1.16')
                version('openpdf.version','1.3.30')
gourigs123 commented 1 year ago

Could not GET 'https://repo.spring.io/plugins-snapshot/com/github/librepdf/openpdf/1.3.30.jaspersoft.2/openpdf-1.3.30.jaspersoft.2.pom'. Received status code 401 from server:

soc commented 1 year ago

version('jasperreports.version', '6.15.0')

@gourigs123 Why are you using this version, if you want to update to 6.20.2?

daniel-shuy commented 11 months ago

For others facing this issue, you can exclude openpdf and import openpdf version 1.3.32 (see #398)