feat: added a spreadsheet to PDF converter for Excel files with MIME …#940
Conversation
…type application/vnd.openxmlformats-officedocument.spreadsheetml.sheet (XLSX files)
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #940 +/- ##
============================================
+ Coverage 85.34% 85.45% +0.10%
- Complexity 2149 2175 +26
============================================
Files 264 265 +1
Lines 5945 6028 +83
Branches 378 390 +12
============================================
+ Hits 5074 5151 +77
- Misses 622 624 +2
- Partials 249 253 +4 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| } | ||
| case CONTENT_TYPE_DOCX, CONTENT_TYPE_TIKA_OOXML -> new DocxConverter(); | ||
| case CONTENT_TYPE_DOC, CONTENT_TYPE_TIKA_MSOFFICE -> new DocConverter(); | ||
| case CONTENT_TYPE_TIKA_XLSX -> new XlsxConverter(); | ||
| default -> { | ||
| throw new IllegalArgumentException("Unsupported content type"); |
There was a problem hiding this comment.
I haven't noticed this piece of code last time. IMHO this switch is a code smell. Each converter should have the file types it supports: boolean supports(String fileType)
and the AttachmentConversionService would just do something like this:
public FileToPdfConverter getConverter(String fileExtension) {
return converters.stream()
.filter(c -> c.supports(fileExtension))
.findFirst()
.orElseThrow(() -> new IllegalArgumentException("No converter for: " + fileExtension));
}There was a problem hiding this comment.
Will not be changed in this PR - opened an new CR for this refactoring:
[FEATURE] Refatoring of attachment conversion services to provide supported file types.
| PDDocument document = new PDDocument(); | ||
| ByteArrayOutputStream pdfOutputStream = new ByteArrayOutputStream()) { | ||
|
|
||
| // Process each sheet in the workbook |
There was a problem hiding this comment.
please remove all such comments, to explain the logic break down the functionality into appropriately named methods
|
|
||
| List<byte[]> result = conversionService.listToPdf(List.of(attachmentId)); | ||
|
|
||
| // Should convert successfully with boolean and blank cells |
There was a problem hiding this comment.
same problem with the comments as above :)
|
|
||
| // ============================================ | ||
| // XLSX Converter Tests | ||
| // ============================================ |
There was a problem hiding this comment.
Feel free to create a new test file, instead of separation with comments, this will get outdated really quickly
…pdf-and-include-in-merged-pdf
…pdf-and-include-in-merged-pdf
|
|
…pdf-and-include-in-merged-pdf
There was a problem hiding this comment.
Pull request overview
This PR adds XLSX to PDF conversion functionality to support Excel spreadsheet attachments. The implementation uses Apache POI to read XLSX files and PDFBox to generate PDF output with formatted tables, properly handling multiple sheets and various cell types.
Changes:
- Added XLSX to PDF conversion capability for spreadsheet attachments
- Implemented comprehensive test coverage for various XLSX scenarios
- Refactored test helper utilities to promote code reuse
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
| backend/pom.xml | Added Apache POI dependency (version 5.4.1) for XLSX support |
| backend/src/main/java/eu/bbmri_eric/negotiator/attachment/XlsxConverter.java | New converter implementing XLSX to PDF conversion with table rendering |
| backend/src/main/java/eu/bbmri_eric/negotiator/attachment/AttachmentConversionServiceImpl.java | Added XLSX content type mapping to route XLSX files to new converter |
| backend/src/test/java/eu/bbmri_eric/negotiator/attachment/XlsxConverterTest.java | Comprehensive test suite covering various XLSX conversion scenarios |
| backend/src/test/java/eu/bbmri_eric/negotiator/attachment/AttachmentTestHelper.java | Extracted shared test helper methods for creating test documents |
| backend/src/test/java/eu/bbmri_eric/negotiator/attachment/AttachmentConversionServiceTest.java | Updated to use centralized test helper methods |
|
|
Negotiator pull request:
Description:
Added a spreadsheet to PDF converter for Excel files with MIME type "application/vnd.openxmlformats-officedocument.spreadsheetml.sheet" (XLSX files) in the XlsxConverter.java file.
Features:
Changes Made:
New tests added:
Checklist:
Make sure you tick all the boxes below if they are true or do not apply before you ask for review