diff --git a/matrix-bom/bom.xml b/matrix-bom/bom.xml index c144770da..2dc4dbebd 100644 --- a/matrix-bom/bom.xml +++ b/matrix-bom/bom.xml @@ -46,7 +46,7 @@ 0.1.0-SNAPSHOT 0.5.0 0.1.1-SNAPSHOT - 2.3.1-SNAPSHOT + 2.4.0 2.4.0 2.4.0 0.3.0 diff --git a/matrix-spreadsheet/README.md b/matrix-spreadsheet/README.md index 268c357e0..d4402ec5c 100644 --- a/matrix-spreadsheet/README.md +++ b/matrix-spreadsheet/README.md @@ -9,9 +9,9 @@ for [Renjin R](https://github.com/bedatadriven/renjin). To use it, add the following to your gradle build script: ```groovy -implementation 'org.apache.groovy:groovy:5.0.4' -implementation 'se.alipsa.matrix:matrix-core:3.6.0' -implementation 'se.alipsa.matrix:matrix-spreadsheet:2.3.0' +implementation 'org.apache.groovy:groovy:5.0.5' +implementation 'se.alipsa.matrix:matrix-core:3.7.1' +implementation 'se.alipsa.matrix:matrix-spreadsheet:2.4.0' ``` or if you use maven: ```xml @@ -19,17 +19,17 @@ or if you use maven: org.apache.groovy groovy - 5.0.4 + 5.0.5 se.alipsa.matrix matrix-core - 3.6.0 + 3.7.1 se.alipsa.matrix matrix-spreadsheet - 2.3.0 + 2.4.0 ``` @@ -210,6 +210,7 @@ The following table illustrates the version compatibility of the matrix-spreadsh | Matrix spreadsheet | Matrix core | |-------------------:|---------------:| +| 2.4.0 | 3.7.1 | | 2.3.0 | 3.4.1 | | 2.2.0 | 3.2.0 -> 3.3.0 | | 2.1.0 | 3.1.0 | @@ -245,7 +246,5 @@ Used to define the data format i.e. the result from an import or the data to exp - URL: https://github.com/Alipsa/matrix - License: MIT -### Log4j -Used to handle logging -- URL: https://logging.apache.org/log4j/2.x/ -- License: Apache 2.0 +### matrix-core Logger +Uses the lightweight JDK System.Logger facade via matrix-core; no direct logging framework dependency. diff --git a/matrix-spreadsheet/build.gradle b/matrix-spreadsheet/build.gradle index 40c4c96d4..9e6e1c8b1 100644 --- a/matrix-spreadsheet/build.gradle +++ b/matrix-spreadsheet/build.gradle @@ -11,7 +11,7 @@ plugins { } group = 'se.alipsa.matrix' -version = '2.3.1-SNAPSHOT' +version = '2.4.0' description = 'Matrix spreadsheet import/export' codenarc { diff --git a/matrix-spreadsheet/release.md b/matrix-spreadsheet/release.md index c33ab291d..4ccfeff21 100644 --- a/matrix-spreadsheet/release.md +++ b/matrix-spreadsheet/release.md @@ -1,5 +1,34 @@ # Release history +## v2.4.0, 2026-05-05 +**Refactoring, code quality, and usability improvements** + +### Breaking Changes +- `Sheet.name` field type tightened from `Object` to `String` +- `Importer` interface: `startColumn`/`endColumn` parameters renamed to `startCol`/`endCol` for consistency + +### New Features +- add whole-sheet convenience imports with auto-detected dimensions: `SpreadsheetImporter.importSpreadsheet(file, sheetNumber)` and `importSpreadsheet(file, sheetName)` +- add `File`-accepting overloads to `SpreadsheetImporter` matching `SpreadsheetWriter`'s API shape + +### Bug Fixes +- remove dead I/O and wasted workbook allocation in `FExcelExporter.exportExcel` when target file already exists +- replace undeclared `commons-io` transitive dependency in `FOdsImporter` with Groovy's native `is.bytes` +- improve error messages when a named sheet does not exist (names the missing sheet instead of generic "No value present") +- standardise rounding mode in `TableUtil.round` to `HALF_EVEN` (banker's rounding) across all numeric column types + +### Code Quality +- add class-level GroovyDoc and fix `@param` mismatches in `SpreadsheetImporter` +- add GroovyDoc to `SpreadsheetWriter.writeSheets(Map)` +- fix stale/misleading GroovyDoc on `FExcelExporter.exportExcel` overloads +- make `SpreadsheetImporter.validateNotNull` private +- add `@CompileStatic` to `ValueExtractor` +- replace generic `Exception` throws in `FileUtil` with `SpreadsheetImportException` +- document why `FExcelExporter` cannot use `@CompileStatic` (fastexcel internal `GenericStyleSetter` access) + +### Test Coverage +- 124 tests passing (up from 105 in v2.3.0) + ## v2.3.0, 2026-01-31 **Major architectural refactoring with significant performance improvements** diff --git a/matrix-spreadsheet/req/v2.4.0-roadmap.md b/matrix-spreadsheet/req/v2.4.0-roadmap.md new file mode 100644 index 000000000..77149c12a --- /dev/null +++ b/matrix-spreadsheet/req/v2.4.0-roadmap.md @@ -0,0 +1,155 @@ +# Matrix Spreadsheet v2.4.0 Release Roadmap + +This document outlines the planned work for matrix-spreadsheet v2.4.0. + +## Summary + +v2.4.0 focuses on targeted refactoring, stronger test coverage (>= 70%), +performance benchmarking, release hygiene, and usability improvements identified +in the v2.3.1 code review. + +--- + +## Priority 1: Testing & Performance (Must Do) + +### 1.1 [x] Test coverage plan (target >= 70%) +- ✅ Completed in v2.3.0: Coverage achieved 79.74% (exceeds 70% target) +- See the detailed plan in v2.3.0 roadmap under "Test Coverage & Performance Plan". + +### 1.2 [x] Add performance benchmarks for XLSX/ODS read/write +- ✅ Completed in v2.3.0: SpreadsheetBenchmark added with 50k x 12 and Crime_Data tests +- Baseline read/write timing for realistic datasets documented in v2.3.0 roadmap. + +### 1.3 [x] Fix any performance regressions found in benchmarks +- ✅ Completed in v2.3.0: ODS performance improved 65-79% via Aalto parser and optimizations +- Before/after metrics captured: 4.86s → 1.70s (medium), 262s → 53s (large) + +--- + +## Priority 2: Refactoring (Should Do) + +### 2.1 [ ] Consider splitting FExcelAppender +- **File**: `src/main/groovy/se/alipsa/matrix/spreadsheet/fastexcel/FExcelAppender.groovy` +- **Issue**: ~750 lines with 11 inner classes +- **Consideration**: Could extract `WorkbookState`, `WorkbookPlan`, `SheetTemplate` to separate files +- **Note**: The class is well-structured internally; this is optional cleanup + +### 2.2 [ ] Consider using Optional in FExcelReader +- **File**: `src/main/groovy/se/alipsa/matrix/spreadsheet/fastexcel/FExcelReader.groovy` +- **Issue**: Returns -1 when sheet/content not found (lines 85, 97, 146) +- **Consideration**: Modern Java practice would use Optional +- **Note**: This is a breaking API change; defer to v3.0 if needed + +### 2.3 [x] Consider more specific exception types in FileUtil +- **File**: `src/main/groovy/se/alipsa/matrix/spreadsheet/FileUtil.groovy` +- **Issue**: Throws generic `Exception` in several places +- **Fix**: Replaced with `SpreadsheetImportException` (see Priority 4 below) + +--- + +## Priority 3: Bug Fixes & Code Quality (from v2.3.1 review) + +### 3.1 [x] Remove dead code and wasted I/O in `FExcelExporter.exportExcel` +- **File**: `src/main/groovy/se/alipsa/matrix/spreadsheet/fastexcel/FExcelExporter.groovy` lines 83–97 +- **Issue**: When the target file already exists, the method opens a `ReadableWorkbook` (allocating I/O resources), runs sheet-collision avoidance logic, and then unconditionally throws `IllegalArgumentException`. The collision-avoidance `if/while` block is completely dead — the throw always fires regardless. The workbook is opened and closed for nothing. +- **Fix**: Remove the entire `try` block. Replace the `if (file.exists() && file.length() > 0)` branch with a direct throw: + ```groovy + if (file.exists() && file.length() > 0) { + throw new IllegalArgumentException("Appending to an existing Excel file is not supported by FExcelExporter. Use FExcelAppender or SpreadsheetWriter for append/replace operations.") + } + ``` + +### 3.2 [x] Replace undeclared `commons-io` transitive dependency in `FOdsImporter` +- **File**: `src/main/groovy/se/alipsa/matrix/spreadsheet/fastods/FOdsImporter.groovy` line 5, 197 +- **Issue**: `IOUtils.toByteArray(is)` is used via `org.apache.commons.io.IOUtils`, which is only available as a transitive dependency from `fast.excel.reader`. It is not declared in `build.gradle`. If the transitive chain changes this dependency will break silently. +- **Fix**: Replace with Groovy's built-in `is.bytes` and remove the import: + ```groovy + // Before + import org.apache.commons.io.IOUtils + byte[] bytes = IOUtils.toByteArray(is) + // After + byte[] bytes = is.bytes + ``` + +### 3.3 [x] Make `SpreadsheetImporter.validateNotNull` private +- **File**: `src/main/groovy/se/alipsa/matrix/spreadsheet/SpreadsheetImporter.groovy` line 291 +- **Issue**: `static void validateNotNull(Object paramVal, String paramName)` has default (public) Groovy visibility and is therefore part of the public API surface, which is unintentional. +- **Fix**: Add `private` modifier. + +### 3.4 [x] Add `@CompileStatic` to `FExcelExporter` or document why it is absent +- **File**: `src/main/groovy/se/alipsa/matrix/spreadsheet/fastexcel/FExcelExporter.groovy` lines 21–22 +- **Issue**: `@CompileStatic` is commented out with only a brief note about a module access error on `GenericStyleSetter`. The entire exporter runs with dynamic dispatch as a result. +- **Fix**: Investigate whether the access error is avoidable (e.g. `--add-opens` JVM arg, or avoiding the offending call). If unavoidable, expand the comment to explain the root cause and link to the upstream fastexcel issue so the limitation is tracked rather than forgotten. + +### 3.5 [x] Add class-level GroovyDoc to `SpreadsheetImporter` +- **File**: `src/main/groovy/se/alipsa/matrix/spreadsheet/SpreadsheetImporter.groovy` +- **Issue**: `SpreadsheetImporter` is the primary import facade but has no class-level documentation. `SpreadsheetWriter` has thorough class doc including a usage example; `SpreadsheetImporter` should follow the same pattern. + +### 3.6 [x] Fix `@param` mismatches in `SpreadsheetImporter.importOds` / `importExcel` +- **File**: `src/main/groovy/se/alipsa/matrix/spreadsheet/SpreadsheetImporter.groovy` lines 41–123 +- **Issue**: The `importOds(URL url, ...)` and `importExcel(URL url, ...)` overloads copy-paste the doc block from the file-path variants and say `@param file the ods or excel file to import`, but the parameter is `url`, not `file`. Several overloads also have no GroovyDoc at all. +- **Fix**: Correct the `@param` tag to `@param url` with an appropriate description; add missing GroovyDoc to undocumented overloads. + +### 3.7 [x] Add GroovyDoc to `SpreadsheetWriter.writeSheets(Map params)` +- **File**: `src/main/groovy/se/alipsa/matrix/spreadsheet/SpreadsheetWriter.groovy` line 210 +- **Issue**: The map-params overload is undocumented — which keys are valid, what types, which are required? +- **Fix**: Add a `@param params` block listing `file` (File), `data` (List<Matrix>), and `sheetNames` (List<String>), matching the style used for the other overloads. + +### 3.8 [x] Standardise `startCol`/`endCol` parameter naming across `Importer` and `SpreadsheetImporter` +- **Files**: `src/main/groovy/se/alipsa/matrix/spreadsheet/Importer.groovy`, `SpreadsheetImporter.groovy` +- **Issue**: Some overloads use `startColumn`/`endColumn` (e.g. `importSpreadsheet(String, int, int, int, String, String, boolean)`) while the majority use `startCol`/`endCol`. The inconsistency appears in both the interface and the facade. +- **Fix**: Standardise on `startCol`/`endCol` throughout. + +### 3.9 [x] Fix misleading GroovyDoc on `FExcelExporter.exportExcel` overloads +- **File**: `src/main/groovy/se/alipsa/matrix/spreadsheet/fastexcel/FExcelExporter.groovy` lines 29–78 +- **Issue**: All four `exportExcel` overloads claim "if the excel file exists and is not empty, a new sheet will be added to the excel". Since v2.3.0 the method unconditionally throws `IllegalArgumentException` when the file already exists. The GroovyDoc is stale and misleading. +- **Fix**: Update the GroovyDoc to state that existing files cause an `IllegalArgumentException` and direct users to `SpreadsheetWriter` or `FExcelAppender` for append/replace behavior. + +### 3.10 [x] Add `@CompileStatic` to `ValueExtractor` +- **File**: `src/main/groovy/se/alipsa/matrix/spreadsheet/ValueExtractor.groovy` +- **Issue**: The abstract utility class lacks `@CompileStatic`, running with dynamic dispatch despite being pure type-conversion logic. This is inconsistent with the project's static-compilation preference. +- **Fix**: Add `@CompileStatic` and resolve any typing issues that surface. + +### 3.11 [x] Tighten `Sheet.name` field type in `fastods` +- **File**: `src/main/groovy/se/alipsa/matrix/spreadsheet/fastods/Sheet.groovy` +- **Issue**: The `name` field and `getName()` return type are `Object`, but the class only ever stores `String` or `Integer` (via the typed `setName` overloads). Using `Object` hides the actual domain from callers and skips IDE assistance. +- **Fix**: Store name as `String` internally ( callers can pass `Integer` and it is converted immediately in `setName(Number)` ), or split into `sheetName` / `sheetNumber` fields. + +### 3.12 [x] Improve error messages when a named sheet does not exist +- **Files**: `src/main/groovy/se/alipsa/matrix/spreadsheet/fastexcel/FExcelImporter.groovy` lines 160, 172; `fastods/FOdsImporter.groovy` equivalent paths +- **Issue**: `workbook.findSheet(sheetName).orElseThrow()` and `odsDataReader.readOds(..., sheetName, ...)` produce generic `NoSuchElementException: No value present` (or null-sheet errors) instead of naming the missing sheet. This compounds the `'Sheet1'` default problem (4.3) by giving users no clue which sheet name was sought. +- **Fix**: Supply explicit error messages such as `"Sheet '${sheetName}' does not exist in the workbook"`. + +--- + +## Priority 4: Usability Improvements + +### 4.1 [x] Add `File`-based overloads to `SpreadsheetImporter` +- **Files**: `src/main/groovy/se/alipsa/matrix/spreadsheet/SpreadsheetImporter.groovy`, `Importer.groovy` +- **Issue**: `SpreadsheetWriter.write` accepts `File` as its primary target type, but every `SpreadsheetImporter.importSpreadsheet` overload takes `String` file paths. Users with a `File` object must call `.absolutePath` at every call site. +- **Fix**: Add `File`-accepting overloads that delegate via `file.absolutePath`, matching the writer's API shape. + +### 4.2 [x] Add whole-sheet convenience import (auto-detect dimensions) +- **File**: `src/main/groovy/se/alipsa/matrix/spreadsheet/SpreadsheetImporter.groovy` +- **Issue**: Every import method requires explicit `endRow` and `endCol`. The `SpreadsheetFormatProvider` already auto-detects these via `SpreadsheetReader.findLastRow`/`findLastCol`, but this is not exposed through `SpreadsheetImporter`. Importing a whole sheet currently requires 3–4 extra method calls (open reader, find last row, find last col, close, then import). +- **Fix**: Add convenience overloads that use `SpreadsheetReader` internally to detect dimensions: + ```groovy + static Matrix importSpreadsheet(File file, int sheetNumber = 1, boolean firstRowAsColNames = true) + static Matrix importSpreadsheet(String file, int sheetNumber = 1, boolean firstRowAsColNames = true) + static Matrix importSpreadsheet(String file, String sheetName, boolean firstRowAsColNames = true) + ``` + +### 4.3 [N/A] Fix `'Sheet1'` hardcoded default in static overloads +- **Decision**: `'Sheet1'` is the universal default sheet name in both Excel and Calc; kept as-is. + +### 4.4 [x] Refresh README dependency versions and remove stale Log4j claim +- **File**: `matrix-spreadsheet/README.md` +- **Issue**: The README references outdated versions (`matrix-core:3.6.0`, `groovy:5.0.4`, `matrix-spreadsheet:2.3.0`). It also lists **Log4j** as a third-party library (line 248), but the module migrated to `matrix-core` Logger in v2.3.0 and has no direct Log4j dependency. +- **Fix**: Update version references to current aligned versions and remove the Log4j section. + +--- + +## Priority 5: Release Checklist + +- [x] Remove SNAPSHOT suffix from build.gradle version when cutting release +- [x] Update `release.md` v2.4.0 notes with current test count (124 tests) and completed fixes diff --git a/matrix-spreadsheet/src/main/groovy/se/alipsa/matrix/spreadsheet/FileUtil.groovy b/matrix-spreadsheet/src/main/groovy/se/alipsa/matrix/spreadsheet/FileUtil.groovy index e9c0af579..df5682178 100644 --- a/matrix-spreadsheet/src/main/groovy/se/alipsa/matrix/spreadsheet/FileUtil.groovy +++ b/matrix-spreadsheet/src/main/groovy/se/alipsa/matrix/spreadsheet/FileUtil.groovy @@ -18,21 +18,21 @@ class FileUtil { * Verify that the filePath exists and is reachable * @param filePath the path + file name of the resource to find * @return a File of found - * @throws Exception if the filePath cannot be found + * @throws SpreadsheetImportException if the filePath cannot be found */ - static File checkFilePath(String filePath) throws Exception { + static File checkFilePath(String filePath) throws SpreadsheetImportException { File excelFile URL url = getResourceUrl(filePath) if (url == null) { - throw new Exception(filePath + " does not exist") + throw new SpreadsheetImportException("'${filePath}' does not exist") } try { excelFile = Paths.get(url.toURI()).toFile() } catch (URISyntaxException | RuntimeException e) { - throw new Exception(filePath + " does not exist", e) + throw new SpreadsheetImportException("'${filePath}' does not exist", e) } if (!excelFile.exists()) { - throw new Exception(filePath + " does not exist") + throw new SpreadsheetImportException("'${filePath}' does not exist") } return excelFile } diff --git a/matrix-spreadsheet/src/main/groovy/se/alipsa/matrix/spreadsheet/Importer.groovy b/matrix-spreadsheet/src/main/groovy/se/alipsa/matrix/spreadsheet/Importer.groovy index 0993d68bc..f2019848c 100644 --- a/matrix-spreadsheet/src/main/groovy/se/alipsa/matrix/spreadsheet/Importer.groovy +++ b/matrix-spreadsheet/src/main/groovy/se/alipsa/matrix/spreadsheet/Importer.groovy @@ -55,7 +55,7 @@ interface Importer { Matrix importSpreadsheet(String file, int sheetNumber, int startRow, int endRow, - String startColumn, String endColumn, + String startCol, String endCol, boolean firstRowAsColNames) Matrix importSpreadsheet(String file, String sheetName, diff --git a/matrix-spreadsheet/src/main/groovy/se/alipsa/matrix/spreadsheet/SpreadsheetImporter.groovy b/matrix-spreadsheet/src/main/groovy/se/alipsa/matrix/spreadsheet/SpreadsheetImporter.groovy index 1da7ef01e..90c6f575f 100644 --- a/matrix-spreadsheet/src/main/groovy/se/alipsa/matrix/spreadsheet/SpreadsheetImporter.groovy +++ b/matrix-spreadsheet/src/main/groovy/se/alipsa/matrix/spreadsheet/SpreadsheetImporter.groovy @@ -8,153 +8,181 @@ import se.alipsa.matrix.spreadsheet.fastods.FOdsImporter import java.text.NumberFormat +/** + * Static facade for importing spreadsheet data into Matrix objects. + * + *

Auto-detects the file format (.ods vs .xlsx) and delegates to the + * appropriate importer ({@link FOdsImporter} or {@link FExcelImporter}).

+ * + *

Basic Usage

+ *
+ * // Import by sheet index (1-based)
+ * Matrix data = SpreadsheetImporter.importSpreadsheet("data.xlsx", 1, 1, 100, 1, 5)
+ *
+ * // Import by sheet name
+ * Matrix data = SpreadsheetImporter.importSpreadsheet("data.xlsx", "Sales", 1, 100, 1, 5)
+ *
+ * // Import using named parameters
+ * Matrix data = SpreadsheetImporter.importSpreadsheet(
+ *   file: "data.xlsx", sheet: 1, endRow: 100, endCol: 5)
+ * 
+ * + * @see SpreadsheetWriter + * @see FExcelImporter + * @see FOdsImporter + */ @CompileStatic class SpreadsheetImporter { /** + * Import a spreadsheet by file path and sheet index. * * @param file the ods or excel file to import * @param sheet the sheet index (index starting with 1) * @param startRow the first row to include (the first row is row 1) * @param endRow the last row to include - * @param startColumn the first column to include (index starting with 1) - * @param endColumn the last column to include + * @param startCol the first column to include (index starting with 1) + * @param endCol the last column to include * @param firstRowAsColNames whether to treat the first row as a header row for the column names or not, * defaults to true * @return A Matrix corresponding to the spreadsheet data. */ static Matrix importSpreadsheet(String file, int sheet, int startRow = 1, int endRow, - int startColumn = 1, int endColumn, + int startCol = 1, int endCol, boolean firstRowAsColNames = true) { if (isOdsFile(file)) { - return odsImporter().importSpreadsheet(file, sheet, startRow, endRow, startColumn, endColumn, firstRowAsColNames) + return odsImporter().importSpreadsheet(file, sheet, startRow, endRow, startCol, endCol, firstRowAsColNames) } SpreadsheetUtil.ensureXlsx(file) return excelImporter() - .importSpreadsheet(file, sheet, startRow, endRow, startColumn, endColumn, firstRowAsColNames) + .importSpreadsheet(file, sheet, startRow, endRow, startCol, endCol, firstRowAsColNames) } /** + * Import an ODS spreadsheet from a URL by sheet index. * - * @param file the ods or excel file to import + * @param url the URL of the ods file to import * @param sheet the sheet index (index starting with 1) * @param startRow the first row to include (the first row is row 1) * @param endRow the last row to include - * @param startColumn the first column to include (index starting with 1) - * @param endColumn the last column to include + * @param startCol the first column to include (index starting with 1) + * @param endCol the last column to include * @param firstRowAsColNames whether to treat the first row as a header row for the column names or not, * defaults to true * @return A Matrix corresponding to the spreadsheet data. */ static Matrix importOds(URL url, int sheet, int startRow = 1, int endRow, - int startColumn = 1, int endColumn, + int startCol = 1, int endCol, boolean firstRowAsColNames = true) { - odsImporter().importSpreadsheet(url, sheet, startRow, endRow, startColumn, endColumn, firstRowAsColNames) + odsImporter().importSpreadsheet(url, sheet, startRow, endRow, startCol, endCol, firstRowAsColNames) } static Matrix importOds(URL url, int sheet, int startRow = 1, int endRow, - String startColumn = 'A', String endColumn, + String startCol = 'A', String endCol, boolean firstRowAsColNames = true) { - odsImporter().importSpreadsheet(url, sheet, startRow, endRow, startColumn, endColumn, firstRowAsColNames) + odsImporter().importSpreadsheet(url, sheet, startRow, endRow, startCol, endCol, firstRowAsColNames) } /** + * Import an ODS spreadsheet from a URL by sheet name. * - * @param file the ods or excel file to import - * @param sheet the sheet name, defaults to Sheet1 + * @param url the URL of the ods file to import + * @param sheet the sheet name * @param startRow the first row to include (the first row is row 1) * @param endRow the last row to include - * @param startColumn the first column to include (index starting with 1) - * @param endColumn the last column to include + * @param startCol the first column to include (index starting with 1) + * @param endCol the last column to include * @param firstRowAsColNames whether to treat the first row as a header row for the column names or not, * defaults to true * @return A Matrix corresponding to the spreadsheet data. */ static Matrix importOds(URL url, String sheet, int startRow = 1, int endRow, - int startColumn = 1, int endColumn, + int startCol = 1, int endCol, boolean firstRowAsColNames = true) { - odsImporter().importSpreadsheet(url, sheet, startRow, endRow, startColumn, endColumn, firstRowAsColNames) + odsImporter().importSpreadsheet(url, sheet, startRow, endRow, startCol, endCol, firstRowAsColNames) } static Matrix importOds(URL url, String sheet, int startRow = 1, int endRow, - String startColumn = 'A', String endColumn, + String startCol = 'A', String endCol, boolean firstRowAsColNames = true) { - odsImporter().importSpreadsheet(url, sheet, startRow, endRow, startColumn, endColumn, firstRowAsColNames) + odsImporter().importSpreadsheet(url, sheet, startRow, endRow, startCol, endCol, firstRowAsColNames) } static Matrix importExcel(URL url, int sheet, int startRow = 1, int endRow, - int startColumn = 1, int endColumn, + int startCol = 1, int endCol, boolean firstRowAsColNames = true) { SpreadsheetUtil.ensureXlsx(url?.path) - excelImporter().importSpreadsheet(url, sheet, startRow, endRow, startColumn, endColumn, firstRowAsColNames) + excelImporter().importSpreadsheet(url, sheet, startRow, endRow, startCol, endCol, firstRowAsColNames) } static Matrix importExcel(URL url, String sheet, int startRow = 1, int endRow, - int startColumn = 1, int endColumn, + int startCol = 1, int endCol, boolean firstRowAsColNames = true) { SpreadsheetUtil.ensureXlsx(url?.path) - excelImporter().importSpreadsheet(url, sheet, startRow, endRow, startColumn, endColumn, firstRowAsColNames) + excelImporter().importSpreadsheet(url, sheet, startRow, endRow, startCol, endCol, firstRowAsColNames) } static Matrix importExcel(URL url, int sheet, int startRow = 1, int endRow, - String startColumn = 'A', String endColumn, + String startCol = 'A', String endCol, boolean firstRowAsColNames = true) { SpreadsheetUtil.ensureXlsx(url?.path) - excelImporter().importSpreadsheet(url, sheet, startRow, endRow, startColumn, endColumn, firstRowAsColNames) + excelImporter().importSpreadsheet(url, sheet, startRow, endRow, startCol, endCol, firstRowAsColNames) } static Matrix importExcel(URL url, String sheet, int startRow = 1, int endRow, - String startColumn = 'A', String endColumn, + String startCol = 'A', String endCol, boolean firstRowAsColNames = true) { SpreadsheetUtil.ensureXlsx(url?.path) - excelImporter().importSpreadsheet(url, sheet, startRow, endRow, startColumn, endColumn, firstRowAsColNames) + excelImporter().importSpreadsheet(url, sheet, startRow, endRow, startCol, endCol, firstRowAsColNames) } /** + * Import a spreadsheet by file path and sheet index using column names. * * @param file the ods or excel file to import * @param sheet the sheet index (index starting with 1) * @param startRow the first row to include (the first row is row 1) * @param endRow the last row to include - * @param startColumn the first column name to include (A is the first column name etc.) - * @param endColumn the last column name to include - * @param firstRowAsColNames whether to treat the first row as a header row for thecolumn names or not, + * @param startCol the first column name to include (A is the first column name etc.) + * @param endCol the last column name to include + * @param firstRowAsColNames whether to treat the first row as a header row for the column names or not, * defaults to true * @return A Matrix corresponding to the spreadsheet data. */ static Matrix importSpreadsheet(String file, int sheet, int startRow = 1, int endRow, - String startColumn = 'A', String endColumn, + String startCol = 'A', String endCol, boolean firstRowAsColNames = true) { if (isOdsFile(file)) { - return odsImporter().importSpreadsheet(file, sheet, startRow, endRow, startColumn, endColumn, firstRowAsColNames) + return odsImporter().importSpreadsheet(file, sheet, startRow, endRow, startCol, endCol, firstRowAsColNames) } SpreadsheetUtil.ensureXlsx(file) return excelImporter() - .importSpreadsheet(file, sheet, startRow, endRow, startColumn, endColumn, firstRowAsColNames) + .importSpreadsheet(file, sheet, startRow, endRow, startCol, endCol, firstRowAsColNames) } /** + * Import a spreadsheet by file path and sheet name. * * @param file the ods or excel file to import - * @param sheet the sheet name, defaults to Sheet1 + * @param sheet the sheet name, defaults to 'Sheet1' * @param startRow the first row to include (the first row is row 1) * @param endRow the last row to include - * @param startColumn the first column to include (index starting with 1) - * @param endColumn the last column to include - * @param firstRowAsColNames whether to treat the first row as a header row for thecolumn names or not, + * @param startCol the first column to include (index starting with 1) + * @param endCol the last column to include + * @param firstRowAsColNames whether to treat the first row as a header row for the column names or not, * defaults to true * @return A Matrix corresponding to the spreadsheet data. */ @@ -171,14 +199,15 @@ class SpreadsheetImporter { } /** + * Import a spreadsheet by file path and sheet name using column names. * * @param file the ods or excel file to import - * @param sheet the sheet name, defaults to Sheet1 + * @param sheet the sheet name, defaults to 'Sheet1' * @param startRow the first row to include (the first row is row 1) * @param endRow the last row to include - * @param startColumn the first column name to include (A is the first column name etc.) - * @param endColumn the last column name to include - * @param firstRowAsColNames whether to treat the first row as a header row for thecolumn names or not, + * @param startCol the first column name to include (A is the first column name etc.) + * @param endCol the last column name to include + * @param firstRowAsColNames whether to treat the first row as a header row for the column names or not, * defaults to true * @return A Matrix corresponding to the spreadsheet data. */ @@ -197,6 +226,141 @@ class SpreadsheetImporter { .importSpreadsheet(file, sheet, startRow, endRow, startCol, endCol, firstRowAsColNames) } + // --- File-based overloads --- + + /** + * Delegates to {@link #importSpreadsheet(String, int, int, int, int, int, boolean)} + * after resolving the absolute path. + */ + static Matrix importSpreadsheet(File file, int sheet, + int startRow = 1, int endRow, + int startCol = 1, int endCol, + boolean firstRowAsColNames = true) { + importSpreadsheet(file.absolutePath, sheet, startRow, endRow, startCol, endCol, firstRowAsColNames) + } + + /** + * Delegates to {@link #importSpreadsheet(String, String, int, int, int, int, boolean)} + * after resolving the absolute path. + */ + static Matrix importSpreadsheet(File file, String sheet, + int startRow = 1, int endRow, + int startCol = 1, int endCol, + boolean firstRowAsColNames = true) { + importSpreadsheet(file.absolutePath, sheet, startRow, endRow, startCol, endCol, firstRowAsColNames) + } + + /** + * Delegates to {@link #importSpreadsheet(String, int, int, int, String, String, boolean)} + * after resolving the absolute path. + */ + static Matrix importSpreadsheet(File file, int sheet, + int startRow = 1, int endRow, + String startCol = 'A', String endCol, + boolean firstRowAsColNames = true) { + importSpreadsheet(file.absolutePath, sheet, startRow, endRow, startCol, endCol, firstRowAsColNames) + } + + /** + * Delegates to {@link #importSpreadsheet(String, String, int, int, String, String, boolean)} + * after resolving the absolute path. + */ + static Matrix importSpreadsheet(File file, String sheet, + int startRow = 1, int endRow, + String startCol = 'A', String endCol, + boolean firstRowAsColNames = true) { + importSpreadsheet(file.absolutePath, sheet, startRow, endRow, startCol, endCol, firstRowAsColNames) + } + + // --- Whole-sheet convenience imports (auto-detect dimensions) --- + + /** + * Import the first sheet of a spreadsheet file using auto-detected dimensions. + * + * @param file the spreadsheet file to import + * @return A Matrix corresponding to the spreadsheet data. + */ + static Matrix importSpreadsheet(File file) { + importSpreadsheet(file, 1, true) + } + + /** + * Import an entire spreadsheet sheet using auto-detected dimensions. + * + * @param file the spreadsheet file to import + * @param sheetNumber the sheet index (1-based) + * @param firstRowAsColNames whether to treat the first row as column names + * @return A Matrix corresponding to the spreadsheet data. + */ + static Matrix importSpreadsheet(File file, int sheetNumber, boolean firstRowAsColNames = true) { + SpreadsheetReader.Factory.create(file).withCloseable { SpreadsheetReader reader -> + int endRow = reader.findLastRow(sheetNumber) + int endCol = reader.findLastCol(sheetNumber) + return importSpreadsheet(file.absolutePath, sheetNumber, 1, endRow, 1, endCol, firstRowAsColNames) + } + } + + /** + * Import the first sheet of a spreadsheet using auto-detected dimensions. + * + * @param file the spreadsheet file path to import + * @return A Matrix corresponding to the spreadsheet data. + */ + static Matrix importSpreadsheet(String file) { + importSpreadsheet(file, 1, true) + } + + /** + * Import an entire spreadsheet sheet using auto-detected dimensions. + * + * @param file the spreadsheet file path to import + * @param sheetNumber the sheet index (1-based) + * @param firstRowAsColNames whether to treat the first row as column names + * @return A Matrix corresponding to the spreadsheet data. + */ + static Matrix importSpreadsheet(String file, int sheetNumber, boolean firstRowAsColNames) { + File spreadsheetFile = FileUtil.checkFilePath(file) + SpreadsheetReader.Factory.create(spreadsheetFile).withCloseable { SpreadsheetReader reader -> + int endRow = reader.findLastRow(sheetNumber) + int endCol = reader.findLastCol(sheetNumber) + return importSpreadsheet(file, sheetNumber, 1, endRow, 1, endCol, firstRowAsColNames) + } + } + + /** + * Import an entire spreadsheet sheet by name using auto-detected dimensions. + * + *

For XLSX files, a missing sheet throws {@link NoSuchElementException}. + * For ODS files, a missing sheet throws {@link se.alipsa.matrix.spreadsheet.fastods.FastOdsException}.

+ * + * @param file the spreadsheet file path to import + * @param sheetName the sheet name + * @param firstRowAsColNames whether to treat the first row as column names + * @return A Matrix corresponding to the spreadsheet data. + */ + static Matrix importSpreadsheet(String file, String sheetName, boolean firstRowAsColNames) { + File spreadsheetFile = FileUtil.checkFilePath(file) + SpreadsheetReader.Factory.create(spreadsheetFile).withCloseable { SpreadsheetReader reader -> + int endRow = reader.findLastRow(sheetName) + int endCol = reader.findLastCol(sheetName) + return importSpreadsheet(file, sheetName, 1, endRow, 1, endCol, firstRowAsColNames) + } + } + + /** + * Import an entire spreadsheet sheet by name using auto-detected dimensions. + * + * @param file the spreadsheet file to import + * @param sheetName the sheet name + * @param firstRowAsColNames whether to treat the first row as column names + * @return A Matrix corresponding to the spreadsheet data. + */ + static Matrix importSpreadsheet(File file, String sheetName, boolean firstRowAsColNames = true) { + importSpreadsheet(file.absolutePath, sheetName, firstRowAsColNames) + } + + // --- Map-based import --- + /** * * @param params the named parameters i.e. @@ -263,6 +427,12 @@ class SpreadsheetImporter { ) } + static Map importSpreadsheets(File file, + List sheetParams, + NumberFormat... formatOpt) { + importSpreadsheets(file.absolutePath, sheetParams, formatOpt) + } + static Map importSpreadsheets(String fileName, List sheetParams, NumberFormat... formatOpt) { @@ -288,7 +458,7 @@ class SpreadsheetImporter { .importSpreadsheets(url, sheetParams, formatOpt) } - static void validateNotNull(Object paramVal, String paramName) { + private static void validateNotNull(Object paramVal, String paramName) { if (paramVal == null) { throw new IllegalArgumentException("$paramName cannot be null") } diff --git a/matrix-spreadsheet/src/main/groovy/se/alipsa/matrix/spreadsheet/SpreadsheetWriter.groovy b/matrix-spreadsheet/src/main/groovy/se/alipsa/matrix/spreadsheet/SpreadsheetWriter.groovy index 92fa9070e..289e65e44 100644 --- a/matrix-spreadsheet/src/main/groovy/se/alipsa/matrix/spreadsheet/SpreadsheetWriter.groovy +++ b/matrix-spreadsheet/src/main/groovy/se/alipsa/matrix/spreadsheet/SpreadsheetWriter.groovy @@ -204,7 +204,14 @@ class SpreadsheetWriter { /** * Write multiple Matrix objects using a parameter map. * - * @param params map with keys: 'file' (File), 'data' (List<Matrix>), 'sheetNames' (List<String>) + *

Valid keys in the params map:

+ *
    + *
  • {@code file} ({@link File}) — the target spreadsheet file (required)
  • + *
  • {@code data} ({@code List}) — the matrices to write, one per sheet (required)
  • + *
  • {@code sheetNames} ({@code List}) — names for each sheet (required)
  • + *
+ * + * @param params map of write parameters * @return list of actual sheet names created */ static List writeSheets(Map params) { diff --git a/matrix-spreadsheet/src/main/groovy/se/alipsa/matrix/spreadsheet/ValueExtractor.groovy b/matrix-spreadsheet/src/main/groovy/se/alipsa/matrix/spreadsheet/ValueExtractor.groovy index 12d61ac04..595aa8fd9 100644 --- a/matrix-spreadsheet/src/main/groovy/se/alipsa/matrix/spreadsheet/ValueExtractor.groovy +++ b/matrix-spreadsheet/src/main/groovy/se/alipsa/matrix/spreadsheet/ValueExtractor.groovy @@ -1,8 +1,11 @@ package se.alipsa.matrix.spreadsheet +import groovy.transform.CompileStatic + /** * A ValueExtractor is a helper class that makes it easier to get values from a spreadsheet. */ +@CompileStatic abstract class ValueExtractor { Double getDouble(Object val) { diff --git a/matrix-spreadsheet/src/main/groovy/se/alipsa/matrix/spreadsheet/fastexcel/FExcelExporter.groovy b/matrix-spreadsheet/src/main/groovy/se/alipsa/matrix/spreadsheet/fastexcel/FExcelExporter.groovy index 43d74c284..a406ccd9b 100644 --- a/matrix-spreadsheet/src/main/groovy/se/alipsa/matrix/spreadsheet/fastexcel/FExcelExporter.groovy +++ b/matrix-spreadsheet/src/main/groovy/se/alipsa/matrix/spreadsheet/fastexcel/FExcelExporter.groovy @@ -4,7 +4,6 @@ import groovy.transform.CompileStatic import org.dhatim.fastexcel.Workbook import org.dhatim.fastexcel.Worksheet -import org.dhatim.fastexcel.reader.ReadableWorkbook import se.alipsa.matrix.core.Column import se.alipsa.matrix.core.Matrix @@ -17,9 +16,12 @@ import java.time.LocalDateTime import java.time.ZonedDateTime import java.util.Collections -// Causes module access error (failed to access class org.dhatim.fastexcel.GenericStyleSetter) -// so go with dynamic for now -//@CompileStatic +// @CompileStatic is intentionally omitted. +// When enabled, Groovy generates direct bytecode for sheet.style(row, col).format(...).set() +// which triggers an IllegalAccessError at runtime because GenericStyleSetter is +// package-private inside org.dhatim.fastexcel. Dynamic dispatch uses reflection/ +// metaclass invocation which bypasses this restriction. Re-evaluate if fastexcel +// ever makes GenericStyleSetter public. class FExcelExporter { static final Logger logger = Logger.getLogger(FExcelExporter) @@ -27,9 +29,12 @@ class FExcelExporter { static final String VERSION = '02.1000' // must be in the format XX.YYYY /** - * Export to an Excel file. If the file does not exists, a new file will be created - * if the excel file exists and is not empty, a new sheet will be added to the excel - * The name of the sheet will correspond to the name of the Matrix + * Export a Matrix to a new Excel file. + * + *

If the file already exists and is not empty, an {@link IllegalArgumentException} + * is thrown. Use {@link FExcelAppender} or {@link SpreadsheetWriter} for append/replace + * operations.

+ * * @param filePath the path to the file to export * @param data the Matrix data to export * @return the actual name of the sheet created (illegal characters replaced by space) @@ -40,9 +45,12 @@ class FExcelExporter { } /** - * Export to an Excel file. If the file does not exists, a new file will be created - * if the excel file exists and is not empty, a new sheet will be added to the excel - * The name of the sheet will correspond to the name of the Matrix + * Export a Matrix to a new Excel file. + * + *

If the file already exists and is not empty, an {@link IllegalArgumentException} + * is thrown. Use {@link FExcelAppender} or {@link SpreadsheetWriter} for append/replace + * operations.

+ * * @param file the file to export * @param data the Matrix data to export * @return the actual name of the sheet created (illegal characters replaced by space) @@ -55,8 +63,11 @@ class FExcelExporter { } /** - * Export to an Excel file. If the file does not exists, a new file will be created - * if the excel file exists and is not empty, a new sheet will be added to the excel + * Export a Matrix to a new Excel file with a specific sheet name. + * + *

If the file already exists and is not empty, an {@link IllegalArgumentException} + * is thrown. Use {@link FExcelAppender} or {@link SpreadsheetWriter} for append/replace + * operations.

* * @param file the file to export * @param data the Matrix data to export @@ -68,7 +79,11 @@ class FExcelExporter { } /** - * Export to an Excel file with a specific start position. + * Export a Matrix to a new Excel file with a specific start position. + * + *

If the file already exists and is not empty, an {@link IllegalArgumentException} + * is thrown. Use {@link FExcelAppender} or {@link SpreadsheetWriter} for append/replace + * operations.

* * @param file the file to export * @param data the Matrix data to export @@ -81,26 +96,12 @@ class FExcelExporter { String validSheetName = SpreadsheetUtil.createValidSheetName(sheetName) SpreadsheetUtil.CellPosition position = SpreadsheetUtil.parseCellPosition(startPosition) if (file.exists() && file.length() > 0) { - try (FileInputStream fis = new FileInputStream(file) - ReadableWorkbook workbook = new ReadableWorkbook(fis, FExcelImporter.OPTIONS)) { - List sheetNames = FExcelReader.getSheetNames(workbook) - if (sheetNames.contains(validSheetName)) { - int index = 1 - while (true) { - validSheetName = SpreadsheetUtil.createValidSheetName(validSheetName + index++) - if (!sheetNames.contains(validSheetName)) { - break - } - } - } - throw new IllegalArgumentException("Appending to an existing Excel file is not supported by FExcelExporter. Use FExcelAppender or SpreadsheetWriter for append/replace operations.") - } - } else { - try (FileOutputStream fos = new FileOutputStream(file); Workbook workbook = new Workbook(fos, APP_NAME, VERSION)) { - Worksheet sheet = workbook.newWorksheet(validSheetName) - buildSheet(data, sheet, position) - workbook.finish() - } + throw new IllegalArgumentException("Appending to an existing Excel file is not supported by FExcelExporter. Use FExcelAppender or SpreadsheetWriter for append/replace operations.") + } + try (FileOutputStream fos = new FileOutputStream(file); Workbook workbook = new Workbook(fos, APP_NAME, VERSION)) { + Worksheet sheet = workbook.newWorksheet(validSheetName) + buildSheet(data, sheet, position) + workbook.finish() } return validSheetName } diff --git a/matrix-spreadsheet/src/main/groovy/se/alipsa/matrix/spreadsheet/fastexcel/FExcelImporter.groovy b/matrix-spreadsheet/src/main/groovy/se/alipsa/matrix/spreadsheet/fastexcel/FExcelImporter.groovy index 1d09d4f79..63beed9bb 100644 --- a/matrix-spreadsheet/src/main/groovy/se/alipsa/matrix/spreadsheet/fastexcel/FExcelImporter.groovy +++ b/matrix-spreadsheet/src/main/groovy/se/alipsa/matrix/spreadsheet/fastexcel/FExcelImporter.groovy @@ -135,7 +135,7 @@ class FExcelImporter implements Importer { @Override Matrix importSpreadsheet(String file, int sheet, int startRow = 1, int endRow, - String startColumn = 'A', String endColumn, + String startCol = 'A', String endCol, boolean firstRowAsColNames = true) { SpreadsheetUtil.ensureXlsx(file) return importSpreadsheet( @@ -143,8 +143,8 @@ class FExcelImporter implements Importer { sheet, startRow, endRow, - SpreadsheetUtil.asColumnNumber(startColumn), - SpreadsheetUtil.asColumnNumber(endColumn), + SpreadsheetUtil.asColumnNumber(startCol), + SpreadsheetUtil.asColumnNumber(endCol), firstRowAsColNames ) } @@ -157,7 +157,9 @@ class FExcelImporter implements Importer { SpreadsheetUtil.ensureXlsx(file) File excelFile = FileUtil.checkFilePath(file) try (ReadableWorkbook workbook = new ReadableWorkbook(excelFile, OPTIONS)) { - Sheet sheet = workbook.findSheet(sheetName).orElseThrow() + Sheet sheet = workbook.findSheet(sheetName).orElseThrow { + new NoSuchElementException("Sheet '${sheetName}' does not exist in the workbook") + } boolean isDate1904 = workbook.isDate1904() return importExcelSheet(sheet, startRow, endRow, startCol, endCol, firstRowAsColNames, isDate1904) } @@ -169,7 +171,9 @@ class FExcelImporter implements Importer { int startCol = 1, int endCol, boolean firstRowAsColNames = true) { try (ReadableWorkbook workbook = new ReadableWorkbook(is, OPTIONS)) { - Sheet sheet = workbook.findSheet(sheetName).orElseThrow() + Sheet sheet = workbook.findSheet(sheetName).orElseThrow { + new NoSuchElementException("Sheet '${sheetName}' does not exist in the workbook") + } boolean isDate1904 = workbook.isDate1904() def m = importExcelSheet(sheet, startRow, endRow, startCol, endCol, firstRowAsColNames, isDate1904) return m diff --git a/matrix-spreadsheet/src/main/groovy/se/alipsa/matrix/spreadsheet/fastods/FOdsImporter.groovy b/matrix-spreadsheet/src/main/groovy/se/alipsa/matrix/spreadsheet/fastods/FOdsImporter.groovy index 1977cb1c0..e0d67d8e9 100644 --- a/matrix-spreadsheet/src/main/groovy/se/alipsa/matrix/spreadsheet/fastods/FOdsImporter.groovy +++ b/matrix-spreadsheet/src/main/groovy/se/alipsa/matrix/spreadsheet/fastods/FOdsImporter.groovy @@ -2,8 +2,6 @@ package se.alipsa.matrix.spreadsheet.fastods import groovy.transform.CompileStatic -import org.apache.commons.io.IOUtils - import se.alipsa.matrix.core.ListConverter import se.alipsa.matrix.core.Matrix import se.alipsa.matrix.core.ValueConverter @@ -194,7 +192,7 @@ class FOdsImporter implements Importer { @Override Map importSpreadsheets(InputStream is, List sheetParams, NumberFormat... formatOpt) { - byte[] bytes = IOUtils.toByteArray(is) + byte[] bytes = is.bytes Map result = [:] sheetParams.each { try (InputStream is2 = new ByteArrayInputStream(bytes)) { diff --git a/matrix-spreadsheet/src/main/groovy/se/alipsa/matrix/spreadsheet/fastods/Sheet.groovy b/matrix-spreadsheet/src/main/groovy/se/alipsa/matrix/spreadsheet/fastods/Sheet.groovy index b115908a3..4c058f783 100644 --- a/matrix-spreadsheet/src/main/groovy/se/alipsa/matrix/spreadsheet/fastods/Sheet.groovy +++ b/matrix-spreadsheet/src/main/groovy/se/alipsa/matrix/spreadsheet/fastods/Sheet.groovy @@ -5,14 +5,14 @@ import groovy.transform.CompileStatic @CompileStatic class Sheet extends ArrayList> { - Object name + String name - Object getName() { + String getName() { name } String getSheetName() { - String.valueOf(name) + name } void setName(String name) { @@ -20,10 +20,10 @@ class Sheet extends ArrayList> { } void setName(Integer name) { - this.name = name + this.name = String.valueOf(name) } void setName(Number name) { - this.name = name.intValue() + this.name = String.valueOf(name.intValue()) } } diff --git a/matrix-spreadsheet/src/test/groovy/spreadsheet/SpreadsheetImporterTest.groovy b/matrix-spreadsheet/src/test/groovy/spreadsheet/SpreadsheetImporterTest.groovy index 50a450fe5..4cc4ef663 100644 --- a/matrix-spreadsheet/src/test/groovy/spreadsheet/SpreadsheetImporterTest.groovy +++ b/matrix-spreadsheet/src/test/groovy/spreadsheet/SpreadsheetImporterTest.groovy @@ -16,7 +16,11 @@ import se.alipsa.matrix.spreadsheet.Importer import se.alipsa.matrix.spreadsheet.SpreadsheetImporter import se.alipsa.matrix.spreadsheet.fastexcel.FExcelImporter +import se.alipsa.matrix.spreadsheet.fastods.FastOdsException + +import java.io.File import java.io.InputStream +import java.util.NoSuchElementException import java.math.RoundingMode import java.time.LocalDate import java.time.format.DateTimeFormatter @@ -199,4 +203,71 @@ class SpreadsheetImporterTest { stream.close() } } + + @Test + void testAutoDetectXlsxByFile() throws Exception { + File file = new File(SpreadsheetImporterTest.class.getResource("/Book1.xlsx").toURI()) + Matrix m = importSpreadsheet(file, 1, true) + book1ImportAssertions(m) + } + + @Test + void testAutoDetectXlsxByName() { + Matrix m = importSpreadsheet("Book1.xlsx", "Sheet1", true) + book1ImportAssertions(m) + } + + @Test + void testAutoDetectOdsByFile() throws Exception { + File file = new File(SpreadsheetImporterTest.class.getResource("/Book1.ods").toURI()) + Matrix m = importSpreadsheet(file, 1, true) + book1ImportAssertions(m) + } + + @Test + void testAutoDetectOdsByName() { + Matrix m = importSpreadsheet("Book1.ods", "Sheet1", true) + book1ImportAssertions(m) + } + + @Test + void testAutoDetectXlsxByFileAndSheetName() throws Exception { + File file = new File(SpreadsheetImporterTest.class.getResource("/Book1.xlsx").toURI()) + Matrix m = importSpreadsheet(file, "Sheet1", true) + book1ImportAssertions(m) + } + + @Test + void testFileBasedDelegationWithExplicitRange() throws Exception { + File file = new File(SpreadsheetImporterTest.class.getResource("/Book1.xlsx").toURI()) + Matrix m = importSpreadsheet(file, 1, 1, 12, 1, 4, true) + book1ImportAssertions(m) + } + + @Test + void testMissingSheetThrowsWithSheetName() { + assertThrows(NoSuchElementException.class, { + importSpreadsheet("Book1.xlsx", "NonExistentSheet", 1, 12, 1, 4, true) + }) + } + + @Test + void testMissingOdsSheetThrowsFastOdsException() { + assertThrows(FastOdsException.class, { + importSpreadsheet("Book1.ods", "NonExistentSheet", 1, 12, 1, 4, true) + }) + } + + @Test + void testImportSpreadsheetsWithFile() throws Exception { + File file = new File(SpreadsheetImporterTest.class.getResource("/Book2.xlsx").toURI()) + Map sheets = SpreadsheetImporter.importSpreadsheets(file, + [ + [sheetName: 'Sheet1', startRow: 3, endRow: 11, startCol: 2, endCol: 6, firstRowAsColNames: true], + [sheetName: 'Sheet2', startRow: 1, endRow: 12, startCol: 'A', endCol: 'D', firstRowAsColNames: true] + ]) + assertEquals(2, sheets.size()) + assertNotNull(sheets.Sheet1) + assertNotNull(sheets.Sheet2) + } } diff --git a/matrix-spreadsheet/v2.4.0-roadmap.md b/matrix-spreadsheet/v2.4.0-roadmap.md deleted file mode 100644 index 793ad097b..000000000 --- a/matrix-spreadsheet/v2.4.0-roadmap.md +++ /dev/null @@ -1,52 +0,0 @@ -# Matrix Spreadsheet v2.4.0 Release Roadmap - -This document outlines the planned work for matrix-spreadsheet v2.4.0. - -## Summary - -v2.4.0 focuses on targeted refactoring, stronger test coverage (>= 70%), -performance benchmarking, and release hygiene. - ---- - -## Priority 1: Testing & Performance (Must Do) - -### 1.1 [x] Test coverage plan (target >= 70%) -- ✅ Completed in v2.3.0: Coverage achieved 79.74% (exceeds 70% target) -- See the detailed plan in v2.3.0 roadmap under "Test Coverage & Performance Plan". - -### 1.2 [x] Add performance benchmarks for XLSX/ODS read/write -- ✅ Completed in v2.3.0: SpreadsheetBenchmark added with 50k x 12 and Crime_Data tests -- Baseline read/write timing for realistic datasets documented in v2.3.0 roadmap. - -### 1.3 [x] Fix any performance regressions found in benchmarks -- ✅ Completed in v2.3.0: ODS performance improved 65-79% via Aalto parser and optimizations -- Before/after metrics captured: 4.86s → 1.70s (medium), 262s → 53s (large) - ---- - -## Priority 2: Refactoring (Should Do) - -### 2.1 [ ] Consider splitting FExcelAppender -- **File**: `src/main/groovy/se/alipsa/matrix/spreadsheet/fastexcel/FExcelAppender.groovy` -- **Issue**: ~750 lines with 11 inner classes -- **Consideration**: Could extract `WorkbookState`, `WorkbookPlan`, `SheetTemplate` to separate files -- **Note**: The class is well-structured internally; this is optional cleanup - -### 2.2 [ ] Consider using Optional in FExcelReader -- **File**: `src/main/groovy/se/alipsa/matrix/spreadsheet/fastexcel/FExcelReader.groovy` -- **Issue**: Returns -1 when sheet/content not found (lines 85, 97, 146) -- **Consideration**: Modern Java practice would use Optional -- **Note**: This is a breaking API change; defer to v3.0 if needed - -### 2.3 [ ] Consider more specific exception types in FileUtil -- **File**: `src/main/groovy/se/alipsa/matrix/spreadsheet/FileUtil.groovy` -- **Issue**: Throws generic `Exception` in several places -- **Note**: Low priority; current behavior works - ---- - -## Priority 3: Release Checklist - -- [ ] Remove SNAPSHOT suffix from build.gradle version when cutting release -