-
-
Notifications
You must be signed in to change notification settings - Fork 489
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
Switch from itext to openpdf for PDF map rendering #6343
Conversation
The build is failing:
|
Marking this as a draft as we see if we can recover a 2.x branch of mapfish-print. |
Hunting down mapfish print-lib 2.1.6:
This 2.1.x series uses gradle with a maven plugin; so some care is required restoring |
Interesting update, downloading 2.1.6 above and unpacking the source code over top of the 2.1.x branch shows ... no change. This makes the only change beyond 2.1.5 "upgrading to pdf box 2.0.7" GeoCat/mapfish-print@de53322 Which we should be able to see in the generated
I am now comfortable we have what was tagged as 2.1.6 😄 |
10de7d6
to
100e739
Compare
I have deployed a mapfish-print:2.2-SNAPSHOT to nexus.osgeo.org snapshots repository just so this branch has an opportunity to compile and pass tests. |
This build is failing with:
The repository Please do not list additional repositories in the |
Update: Was able to track the change to 953ef0d to obtain the Note you can update single third-party-dependencies dependencies to |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks all good Jody. Tested PDF record and map print, batch edits with diff.
Tested the following features and look fine: map print, metadata detail page, metadata single print, batch edit diffs. But the metadata selection print, I got for the attached metadata the map not printed. In GN 3.12 it works fine, but not sure if the issue is with GN 4 or with this change. For other metadata, it works. In the log I see the following exceptions:
|
The issue is not related to the change. |
The mapfish-print changes are available here mapfish/mapfish-print-v2#6 for review |
Thank you for the review @josegar74, we can merge this now or wait for mapfish print 2.2.0 to be available. |
…cy from maven central This is a fork of google code diff-match-patch:diff-match-patch:current adapted to java naming convetions and available from maven central. It is by far the most popular fork of this algorithm. By adopting this dependency we avoid the need to use https://maven.repository.redhat.com/ga/ improving build times.
@@ -65,24 +65,6 @@ | |||
</executions> | |||
</plugin> | |||
|
|||
<plugin> | |||
<artifactId>maven-assembly-plugin</artifactId> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This duplicated entry in the pom.xml produced warnings from maven
Updated to mapfish-print 2.2.0 (which is just released on osgeo repo), when this build is completed we can merge :) |
@jodygarnett I guess this need to be backported to |
) * Switch from iText to OpenPDF for PDF map rendering. * Change to mapfish-print 2.2.x based on OpenPDF. * Remove duplicate maven-assembly-plugin for dublin-core/pom.xml resolving warning. * Change Diff to use org.bitbucket.cowwoc:diff-match-patch:1.2 dependency from maven central. This is a fork of google code diff-match-patch:diff-match-patch:current adapted to java naming conventions and available from maven central. It is by far the most popular fork of this algorithm. By adopting this dependency we avoid the need to use https://maven.repository.redhat.com/ga/ improving build times. * Use mapfish-print 2.2.0 release. Backport of geonetwork#6343
* [BP] Switch from itext to openpdf for PDF map rendering (#6343) * Switch from iText to OpenPDF for PDF map rendering. * Change to mapfish-print 2.2.x based on OpenPDF. * Remove duplicate maven-assembly-plugin for dublin-core/pom.xml resolving warning. * Change Diff to use org.bitbucket.cowwoc:diff-match-patch:1.2 dependency from maven central. This is a fork of google code diff-match-patch:diff-match-patch:current adapted to java naming conventions and available from maven central. It is by far the most popular fork of this algorithm. By adopting this dependency we avoid the need to use https://maven.repository.redhat.com/ga/ improving build times. * Use mapfish-print 2.2.0 release. Backport of #6343 * Remove unused import Co-authored-by: Jody Garnett <jody.garnett@gmail.com>
* [BP] Switch from itext to openpdf for PDF map rendering Backport of #6343 * Remove unnecessary exclusions Remove itext exclusions becasue it isn't a depencency of mapfish's print-lib 2.2.0 anymore
On main, we still have the The issue was first introduced by #6364 and #6396 was supposed to fix it. But no. To have main deploying ok, I've to revert b2cfdca as well. Don't ask me why. So far, I've no idea why those library updates introduce that error. Any help or suggestions are welcomed @juanluisrp @jodygarnett ? :) |
I expect the different java environments have been configured with different logging systems wrapping each other. Do you have a stack trace please? So you are reverting the work I did? That is not so good. |
:) it is not but I've no clue why the camel update or this PR trigger that problem. And also sounds odd that jetty:run works and not deploying the WAR on the same Jetty version. Maybe the log4j update will solve it ? |
Is this error, isn't it? https://issues.apache.org/jira/browse/LOG4J2-3426 |
Francois the licenses are incompatible; reverting the work means you cannot legally release geonetwork. You know that right? |
It looks like your environment started using log4j2 2 some how and you have ended up with the log4j2 1.2 API ... bridging to log4j2 2 backend. Still we should collect details into a bug report; there is a PR to migrate to log4j2 2 outstanding also (held up on figuring out harvester logs). |
Going to move discussion to #6475 |
Initially focused on using dependency exclusion to swap iText to OpenPDF for map fish print.
Also found flying-saucer-pdf-itext5 was in use and changed to flying-saucer-pdf-openpdf.
The older iText library has known security vulnerabilities, OpenPDF is a drop-in replacement (forking from iText 4.2.0 to maintain an open-source license).
References:
Notes:
Update:
Fixes: