Add new outputformat geotiff-cog - #4166
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
sbrunner
changed the title
There was a problem hiding this comment.
Pull request overview
Adds a new cog output format intended to generate Cloud Optimized GeoTIFF (COG) results from MapFish Print, wiring the format into the Spring output-format registry and introducing a dedicated JasperReports-backed GeoTIFF exporter.
Changes:
- Registers new
cogoutput format beans for both standard (JasperReport) and map-export templates. - Introduces
JasperReportGeoTiffOutputFormatto render a Jasper report to an in-memory image and write it as a GeoTIFF-like output via GeoTools.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 7 comments.
| File | Description |
|---|---|
| core/src/main/resources/mapfish-spring-config-output-formats.xml | Adds cog output format bean registrations for standard and map-export modes. |
| core/src/main/java/org/mapfish/print/output/JasperReportGeoTiffOutputFormat.java | New output format implementation that renders JasperPrint pages to an image and writes a tiled/compressed GeoTIFF to the response. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
c5e09e1 to
aa9e583
Compare
d6fa689 to
66092e5
Compare
b73494e to
016893c
Compare
sebr72
left a comment
sebr72
left a comment
There was a problem hiding this comment.
Great work. Thanks.
Do not hesitate to ask any question/help to implement these comments.
| || "jpg".equals(format.getFileSuffix())) { | ||
| || "jpg".equals(format.getFileSuffix()) | ||
| || format instanceof MapCogExportOutputFormat) { | ||
| // BMP and JPEG do not support transparency |
There was a problem hiding this comment.
Please update comment to match your change
|
|
||
| @Override | ||
| public final Processor.ExecutionContext print( | ||
| @Nonnull final Map<String, String> mdcContext, |
There was a problem hiding this comment.
Could you please split this method into several smaller ones to avoid that one has to read so much to work out what is happening (I know it has been done in other parts of the code base, but this is not a good practice).
| coverage = factory.create("coverage", image, envelope); | ||
|
|
||
| } else { | ||
| throw new IOException("COG export requires either center + scale or bbox"); |
There was a problem hiding this comment.
Please use a more appropriate Exception. Suggestion: IllegalArgumentException
| params.parameter(AbstractGridFormat.GEOTOOLS_WRITE_PARAMS.getName().toString()).setValue(wp); | ||
| params.parameter(GeoTiffFormat.RETAIN_AXES_ORDER.getName().toString()).setValue(true); | ||
|
|
||
| final OutputStream nonClosingOutputStream = |
There was a problem hiding this comment.
Please elaborate why you need a non Closing stream.
|
|
||
| writer = format.getWriter(nonClosingOutputStream); | ||
| if (writer == null) { | ||
| throw new IOException("Could not create GeoTIFF writer"); |
There was a problem hiding this comment.
Please use a more appropriate exception. Not sure why an IO.
| } finally { | ||
| if (writer != null) { | ||
| try { | ||
| writer.dispose(); |
There was a problem hiding this comment.
Since your output stream is a non closing one, this dispose is not sufficient. You will need to manually call the close in the superclass of your nonClosingOutputStream.
| try { | ||
| writer.dispose(); | ||
| } catch (Exception e) { | ||
| /* ignore */ |
There was a problem hiding this comment.
Why do you want to hide that you could not release resources ? At the bare minimum please log it.
|
|
||
| @Test | ||
| public void testCogExport() throws Exception { | ||
| final Configuration config = configurationFactory.getConfig(getFile(BASE_DIR + "config.yaml")); |
There was a problem hiding this comment.
Please use GIVEN WHEN THEN as comments to clarify what you are trying to validate/test
Use methods if you favor it over using comments.
|
|
||
| @Test | ||
| public void testCogExportWithCenterAndScale() throws Exception { | ||
| final Configuration config = configurationFactory.getConfig(getFile(BASE_DIR + "config.yaml")); |
|
|
||
| @Test | ||
| public void testCogExportWithRotation() throws Exception { | ||
| final Configuration config = configurationFactory.getConfig(getFile(BASE_DIR + "config.yaml")); |
This PR adds support for a new GeoTIFF COG output format. (#3805)
The implementation has been reworked to follow the project’s contribution workflow. The changes are based on the latest codebase, target the master branch, and comply with the pre-commit hooks and formatting rules.
All checks pass locally.