From a147bc2ebb50a108d8b0ab27d4a549345bfc7bd6 Mon Sep 17 00:00:00 2001 From: per Date: Sun, 3 May 2026 16:06:04 +0200 Subject: [PATCH 1/2] All 14 tasks from the v0.2.0 plan are implemented --- matrix-bom/bom.xml | 2 +- matrix-gsheets/build.gradle | 2 +- matrix-gsheets/release.md | 1 + matrix-gsheets/req/v0.2.0-plan.md | 491 ++++++++++++++++++ matrix-gsheets/req/v0.2.0-roadmap.md | 100 ++++ ...{BqAuthUtils.groovy => GsAuthUtils.groovy} | 41 +- ...nticator.groovy => GsAuthenticator.groovy} | 27 +- .../alipsa/matrix/gsheets/GsConverter.groovy | 27 +- .../alipsa/matrix/gsheets/GsExporter.groovy | 2 +- .../alipsa/matrix/gsheets/GsImporter.groovy | 2 +- .../se/alipsa/matrix/gsheets/GsUtil.groovy | 22 +- .../matrix/gsheets/GsheetsReader.groovy | 47 +- .../matrix/gsheets/GsheetsWriter.groovy | 149 +++--- ...tilsTest.groovy => GsAuthUtilsTest.groovy} | 24 +- ...Test.groovy => GsAuthenticatorTest.groovy} | 22 +- .../matrix/gsheets/GsConverterTest.groovy | 40 ++ .../matrix/gsheets/GsExporterTest.groovy | 125 +---- .../alipsa/matrix/gsheets/GsUtilTest.groovy | 121 +++++ .../matrix/gsheets/GsheetsReaderTest.groovy | 33 +- .../matrix/gsheets/GsheetsWriterTest.groovy | 18 +- 20 files changed, 963 insertions(+), 333 deletions(-) create mode 100644 matrix-gsheets/req/v0.2.0-plan.md create mode 100644 matrix-gsheets/req/v0.2.0-roadmap.md rename matrix-gsheets/src/main/groovy/se/alipsa/matrix/gsheets/{BqAuthUtils.groovy => GsAuthUtils.groovy} (83%) rename matrix-gsheets/src/main/groovy/se/alipsa/matrix/gsheets/{BqAuthenticator.groovy => GsAuthenticator.groovy} (94%) rename matrix-gsheets/src/test/groovy/test/alipsa/matrix/gsheets/{BqAuthUtilsTest.groovy => GsAuthUtilsTest.groovy} (56%) rename matrix-gsheets/src/test/groovy/test/alipsa/matrix/gsheets/{BqAuthenticatorTest.groovy => GsAuthenticatorTest.groovy} (84%) diff --git a/matrix-bom/bom.xml b/matrix-bom/bom.xml index 3a28c216..409b1297 100644 --- a/matrix-bom/bom.xml +++ b/matrix-bom/bom.xml @@ -41,7 +41,7 @@ 2.1.3-SNAPSHOT 0.5.0-SNAPSHOT 0.2.0 - 0.1.2-SNAPSHOT + 0.2.0-SNAPSHOT 2.2.0 0.1.0-SNAPSHOT 0.5.0 diff --git a/matrix-gsheets/build.gradle b/matrix-gsheets/build.gradle index 9edfe5d9..03235644 100644 --- a/matrix-gsheets/build.gradle +++ b/matrix-gsheets/build.gradle @@ -10,7 +10,7 @@ plugins { } group = 'se.alipsa.matrix' -version = '0.1.2-SNAPSHOT' +version = '0.2.0-SNAPSHOT' description = 'Google Sheets import/export' repositories { diff --git a/matrix-gsheets/release.md b/matrix-gsheets/release.md index 266021e8..5255472f 100644 --- a/matrix-gsheets/release.md +++ b/matrix-gsheets/release.md @@ -7,6 +7,7 @@ - org.mockito:mockito-core 5.22.0 -> 5.23.0 - org.mockito:mockito-junit-jupiter 5.22.0 -> 5.23.0 - se.alipsa.nexus-release-plugin:se.alipsa.nexus-release-plugin.gradle.plugin 2.1.1 -> 2.1.2 +- rename BqAuthenticator to GsAuthenticator and BqAuthUtils to GsAuthUtils ## 0.1.1, 2026-01-31 Move actual implementation for GsheetsReader and GsheetsWriter and utility methods to GsUtil so that GsImporter and GsExporter are just empty wrappers. diff --git a/matrix-gsheets/req/v0.2.0-plan.md b/matrix-gsheets/req/v0.2.0-plan.md new file mode 100644 index 00000000..f65151af --- /dev/null +++ b/matrix-gsheets/req/v0.2.0-plan.md @@ -0,0 +1,491 @@ +# Matrix-GSheets v0.2.0 Implementation Plan + +> **Target file:** `matrix-gsheets/req/v0.2.0-plan.md` + +## Overview +This plan addresses all 14 tasks (plus minor notes) from `matrix-gsheets/req/v0.2.0-roadmap.md`. The module compiles statically by default via `config/groovy/compileStatic.groovy`, so `@CompileStatic` is not required on individual classes. Changes span 6 production files and 4 test files. + +--- + +## 1. Remove Duplicated Code in GsheetsWriter + +**1.1** Remove the `MAX_SHEET_NAME_LENGTH` constant from `GsheetsWriter` (line 96). + +**1.2** Remove the `@PackageScope static String sanitizeSheetName(String)` method from `GsheetsWriter` (lines 215–223). + +**1.3** Remove the `@PackageScope static Object toCell(Object, boolean, boolean)` method from `GsheetsWriter` (lines 225–249). + +**1.4** Update `GsheetsWriter.write()` to call `GsUtil.sanitizeSheetName(...)` and `GsUtil.toCell(...)` directly. Replace the header row initialization with: +```groovy +List> values = [new ArrayList(matrix.columnNames())] +``` +Or follow the same collect pattern as the data rows. + +**1.5** Remove the now-unused imports: `groovy.transform.PackageScope`, `java.time.LocalDate`, `java.time.LocalDateTime`, `java.time.LocalTime`. + +**Test command:** +```bash +./gradlew :matrix-gsheets:test --tests 'test.alipsa.matrix.gsheets.GsheetsWriterTest' -g ./.gradle-user +``` + +--- + +## 2. Remove Explicit `return` on Final Expressions + +**2.1** `GsUtil.groovy` — remove `return` from final expressions: +- Line 109: `return endColIndex - startColIndex + 1` → `endColIndex - startColIndex + 1` +- Line 130: `return number` → `number` +- Line 152: `return getSheetNames(spreadsheetId, sheetsService)` → `getSheetNames(spreadsheetId, sheetsService)` +- Line 180: `return names` → `names` +- Line 193: `return s.trim().isEmpty() ? 'Sheet1' : s` → `s.trim().isEmpty() ? 'Sheet1' : s` +- Line 218: `return String.valueOf(v)` → `String.valueOf(v)` +- Line 248: `return list` → `list` +- Line 264: `return headers` → `headers` + +**2.2** `GsheetsReader.groovy` — remove `return` from final expressions: +- Line 80–81: `return readAsStrings(...)` → `readAsStrings(...)` +- Line 170–171: `return Matrix.builder(...)` → `Matrix.builder(...)` +- Line 245–246: `return Matrix.builder(...)` → `Matrix.builder(...)` + +**2.3** `GsheetsWriter.groovy` — remove `return` from final expressions: +- Line 131: `return write(...)` → `write(...)` +- Line 212: `return spreadsheetId` → `spreadsheetId` + +**2.4** `GsConverter.groovy` — remove `return` from final expressions: +- Line 56: `return EPOCH_DATE.plusDays(daysSinceEpoch)` → `EPOCH_DATE.plusDays(daysSinceEpoch)` +- Line 67: `return dates` → `dates` +- Line 90–91: `return asLocalDateTime(...)` → `asLocalDateTime(...)` +- Line 118: `return dateTimes` → `dateTimes` +- Line 141: `return asLocalTime(...)` → `asLocalTime(...)` +- Line 149: `return LocalTime.ofSecondOfDay(totalSeconds)` → `LocalTime.ofSecondOfDay(totalSeconds)` +- Line 160: `return times` → `times` +- Line 171: `return days` → `days` +- Line 186: `return days + fraction` → `days + fraction` +- Line 192: `return secondsSinceMidnight / totalSecondsInDay` → `secondsSinceMidnight / totalSecondsInDay` +- Line 199: `return asSerial(...)` → `asSerial(...)` +- Line 223: `return serials` → `serials` + +**Test command:** +```bash +./gradlew :matrix-gsheets:compileGroovy -g ./.gradle-user +``` + +--- + +## 3. Fix Logging Indentation Bug in GsAuthenticator + +**3.1** In `GsAuthenticator.groovy` lines 85–90, fix the indentation so `log.info` is inside the `if (verbose)` block: + +```groovy +if (qp) { + credentials = credentials.createWithQuotaProject(qp) + if (verbose) { + log.info("Using quota project: $qp") + } +} +``` + +**Test command:** +```bash +./gradlew :matrix-gsheets:compileGroovy -g ./.gradle-user +``` + +--- + +## 4. Fix Static Field Ordering Hazard in GsAuthenticator + +**4.1** Move `PROP_USER_HOME` declaration (currently line 42) above `ADC_FILE_PATH` (currently line 36) so the field initialization order is safe and explicit: + +```groovy +private static final String PROP_USER_HOME = 'user.home' +static final File ADC_FILE_PATH = new File(System.getProperty(PROP_USER_HOME), '.config/gcloud/application_default_credentials.json') +``` + +**Test command:** +```bash +./gradlew :matrix-gsheets:compileGroovy -g ./.gradle-user +``` + +--- + +## 5. Remove Commented-Out Code from GsAuthUtils + +**5.1** In `GsAuthUtils.groovy`, remove the old tokeninfo URL comment at line 112: +```groovy +//def url = new URI("https://www.googleapis.com/oauth2/v1/tokeninfo?access_token=${token}").toURL() // old url +``` + +**5.2** Convert the inline usage docs block (lines 27–36) into proper GroovyDoc on the `loginAndWriteAdc` method, then remove the inline block comment: + +```groovy +/** + * Drop-in replacement for `gcloud auth application-default login`. + * Writes `~/.config/gcloud/application_default_credentials.json`. + * + *

Usage: + *

{@code
+ * def creds = loginAndWriteAdc(
+ *     new File("$HOME/client_secret_desktop.json"),
+ *     [com.google.api.services.sheets.v4.SheetsScopes.SPREADSHEETS,
+ *      'https://www.googleapis.com/auth/drive.file',
+ *      'openid', 'email'],
+ *     "$PROJECT_ID" // optional; mainly needed if you still call Google OAuth2 userinfo via googleapis.com
+ * )
+ * }
+ */ +``` + +**Test command:** +```bash +./gradlew :matrix-gsheets:compileGroovy -g ./.gradle-user +``` + +--- + +## 6. Extract Duplicated Service Setup in GsheetsReader + +**6.1** Extract the identical 8-line transport/credentials/service setup from `readAsObject` (lines 128–139) and `readAsStrings` (lines 191–202) into a private helper: + +```groovy +private static Sheets buildSheetsService(GoogleCredentials credentials) { + def creds = credentials ?: GsAuthenticator.authenticate(GsAuthenticator.SCOPE_SHEETS_READONLY) + new Sheets.Builder( + GoogleNetHttpTransport.newTrustedTransport(), + GsonFactory.getDefaultInstance(), + new HttpCredentialsAdapter(creds)) + .setApplicationName(APP_NAME) + .build() +} +``` + +**6.2** Replace the duplicated blocks in both methods with `def sheetsService = buildSheetsService(credentials)`. + +**6.3** Add import for `com.google.api.client.http.HttpRequestInitializer` if needed (check if it becomes unused elsewhere). + +**Test command:** +```bash +./gradlew :matrix-gsheets:test --tests 'test.alipsa.matrix.gsheets.GsheetsReaderTest' -g ./.gradle-user +``` + +--- + +## 7. Add URL Convenience Method to GsheetsWriter + +**7.1** Add a static helper method to `GsheetsWriter`: + +```groovy +/** + * Returns the Google Sheets edit URL for a spreadsheet ID. + * + * @param spreadsheetId The spreadsheet ID + * @return The full URL to open the spreadsheet in a browser + */ +static String spreadsheetUrl(String spreadsheetId) { + "https://docs.google.com/spreadsheets/d/${spreadsheetId}/edit" +} +``` + +**7.2** Update the GroovyDoc `@return` on `write()` to reference this new method instead of showing the raw URL string. + +**7.3** Update the usage example in the class-level GroovyDoc to use the new helper: +```groovy +// Write to Google Sheets +String spreadsheetId = GsheetsWriter.write(employees) +println "View at: ${GsheetsWriter.spreadsheetUrl(spreadsheetId)}" +``` + +**Test command:** +```bash +./gradlew :matrix-gsheets:test --tests 'test.alipsa.matrix.gsheets.GsheetsWriterTest' -g ./.gradle-user +``` + +--- + +## 8. Add Write-to-Existing-Spreadsheet API + +**8.1** Add an `update` overload to `GsheetsWriter` that writes to an existing spreadsheet and range: + +```groovy +/** + * Updates an existing Google Spreadsheet with Matrix data. + * + * @param spreadsheetId The ID of the existing spreadsheet + * @param range The target range in A1 notation (e.g., 'Sheet1!A1') + * @param matrix The Matrix to write + * @param credentials Google Cloud credentials, or null to use ADC + * @param convertNullsToEmptyString If true, null values become empty strings + * @param convertDatesToSerial If true, date/time types are converted to serial numbers + * @return The spreadsheetId + * @throws IllegalArgumentException if any required parameter is null/empty + * @throws SheetOperationException if the update fails + */ +static String update(String spreadsheetId, String range, Matrix matrix, + GoogleCredentials credentials = null, + boolean convertNullsToEmptyString = true, + boolean convertDatesToSerial = false) { + // Validate inputs + if (spreadsheetId == null || spreadsheetId.trim().isEmpty()) { + throw new IllegalArgumentException('spreadsheetId must not be null or empty') + } + if (range == null || range.trim().isEmpty()) { + throw new IllegalArgumentException('range must not be null or empty') + } + if (matrix == null) { + throw new IllegalArgumentException('matrix must not be null') + } + if (matrix.columnCount() == 0) { + throw new IllegalArgumentException('matrix must have at least one column') + } + if (matrix.rowCount() == 0) { + throw new IllegalArgumentException('matrix must have at least one row') + } + + def transport = GoogleNetHttpTransport.newTrustedTransport() + def gsonFactory = GsonFactory.getDefaultInstance() + + if (credentials == null) { + credentials = authenticate(SCOPES) + } + HttpRequestInitializer cred = new HttpCredentialsAdapter(credentials) + + Sheets sheets = new Sheets.Builder(transport, gsonFactory, cred) + .setApplicationName('Matrix GSheets') + .build() + + // Build data: header row + data rows + List headers = (List) matrix.columnNames() + List> values = [new ArrayList(headers)] + + matrix.each { Row it -> + values << it.collect { GsUtil.toCell(it, convertNullsToEmptyString, convertDatesToSerial) } + } + + ValueRange vr = new ValueRange() + .setRange(range) + .setMajorDimension('ROWS') + .setValues(values) + + try { + sheets.spreadsheets().values() + .update(spreadsheetId, range, vr) + .setValueInputOption('RAW') + .execute() + } catch (IOException e) { + throw new SheetOperationException('update data', spreadsheetId, e) + } + + spreadsheetId +} +``` + +**Test command:** +```bash +./gradlew :matrix-gsheets:test --tests 'test.alipsa.matrix.gsheets.GsheetsWriterTest' -g ./.gradle-user +``` + +--- + +## 9. Document Bq* Naming in GroovyDoc + +**9.1** Add a historical-naming note to the class-level GroovyDoc of `GsAuthenticator`: + +``` +*

Note on naming: The "Bq" prefix (short for BigQuery) is historical. +* These classes handle authentication for Google Sheets, not BigQuery. +* The name is retained for backward compatibility. +``` + +**9.2** Add the same note to `GsAuthUtils`. + +**Test command:** +```bash +./gradlew :matrix-gsheets:compileGroovy -g ./.gradle-user +``` + +--- + +## 10. Fix Matrix Name from Range Without Sheet Prefix + +**10.1** In `GsheetsReader.readAsObject` (line 167) and `readAsStrings` (line 222), fix the sheet-name extraction: + +```groovy +def rangeParts = range.split(SHEET_NAME_SEPARATOR, 2) +def sheetName = rangeParts.size() > 1 ? rangeParts[0] : '' +``` + +**10.2** Use `''` (empty string) when no sheet name prefix is present, producing an unnamed matrix consistent with `Matrix.builder()` with no name. Document the behavior in the method GroovyDoc. + +**Test command:** +```bash +./gradlew :matrix-gsheets:test --tests 'test.alipsa.matrix.gsheets.GsheetsReaderTest' -g ./.gradle-user +``` + +--- + +## 11. Fix Misleading Test Comments and Remove Brittle Reflection Tests + +**11.1** In `GsheetsReaderTest`, fix the class-level comment (lines 14–16): +```groovy +/** + * Tests for GsheetsReader class. + * + * These tests verify input validation and method existence. + * Actual read operations are tested via external/integration tests. + */ +``` + +**11.2** Remove `testMethodsDelegateToGsImporter()` (lines 70–94) — reflection-based signature tests are brittle and add no value over compilation. + +**11.3** In `GsheetsWriterTest`, fix the class-level comment (lines 12–16) similarly. + +**11.4** Remove `testMethodDelegatesToGsExporter()` (lines 46–56) for the same reason. + +**Test command:** +```bash +./gradlew :matrix-gsheets:test --tests 'test.alipsa.matrix.gsheets.GsheetsReaderTest' --tests 'test.alipsa.matrix.gsheets.GsheetsWriterTest' -g ./.gradle-user +``` + +--- + +## 12. Move GsUtil Tests from GsExporterTest to GsUtilTest + +**12.1** Move the following test methods from `GsExporterTest` to `GsUtilTest`: +- `testSanitizeSheetNameRemovesInvalidCharacters` +- `testSanitizeSheetNameTruncatesLongNames` +- `testSanitizeSheetNameHandlesEmptyString` +- `testSanitizeSheetNameHandlesOnlyInvalidCharacters` +- `testSanitizeSheetNamePreservesValidCharacters` +- `testToCellWithNull` +- `testToCellWithNumbers` +- `testToCellWithBoolean` +- `testToCellWithString` +- `testToCellWithLocalDateAsString` +- `testToCellWithLocalDateAsSerial` +- `testToCellWithLocalDateTimeAsString` +- `testToCellWithLocalDateTimeAsSerial` +- `testToCellWithLocalTimeAsString` +- `testToCellWithLocalTimeAsSerial` +- `testToCellWithDateAsSerial` + +**12.2** Update imports in `GsUtilTest` to include `java.time.LocalDateTime`, `java.time.LocalTime`, `se.alipsa.matrix.gsheets.GsConverter`. + +**12.3** Remove the now-unused imports from `GsExporterTest` (keep only what's needed for the remaining `testExportSheet*` tests). + +**12.4** Rename `GsExporterTest` to something more accurate if desired, or keep it focused on export-sheet validation tests only. + +**Test command:** +```bash +./gradlew :matrix-gsheets:test --tests 'test.alipsa.matrix.gsheets.GsUtilTest' --tests 'test.alipsa.matrix.gsheets.GsExporterTest' -g ./.gradle-user +``` + +--- + +## 13. Add Positive-Case Tests for GsConverter.toSerials + +**13.1** Add the following tests to `GsConverterTest`: + +```groovy +@Test +void testToSerialsWithLocalDates() { + def dates = [LocalDate.parse('2024-06-24'), LocalDate.parse('2025-06-01')] + def serials = GsConverter.toSerials(dates) + assertEquals(2, serials.size()) + assertEquals(new BigDecimal(45467), serials[0]) + assertEquals(new BigDecimal(45809), serials[1]) +} + +@Test +void testToSerialsWithLocalDateTimes() { + def dateTimes = [LocalDateTime.parse('2022-01-14T12:33:20')] + def serials = GsConverter.toSerials(dateTimes) + assertEquals(1, serials.size()) + assertEquals(44575.5231481481, serials[0]) +} + +@Test +void testToSerialsWithLocalTimes() { + def times = [LocalTime.parse('18:25:44')] + def serials = GsConverter.toSerials(times) + assertEquals(1, serials.size()) + assertEquals(0.7678703704, serials[0]) +} + +@Test +void testToSerialsWithMixedDateTypes() { + def mixed = [LocalDate.parse('2024-06-24'), LocalDateTime.parse('2022-01-14T12:33:20')] + def serials = GsConverter.toSerials(mixed) + assertEquals(2, serials.size()) + assertEquals(new BigDecimal(45467), serials[0]) + assertEquals(44575.5231481481, serials[1]) +} + +@Test +void testToSerialsWithEmptyList() { + def serials = GsConverter.toSerials([]) + assertEquals([], serials) +} +``` + +**Test command:** +```bash +./gradlew :matrix-gsheets:test --tests 'test.alipsa.matrix.gsheets.GsConverterTest' -g ./.gradle-user +``` + +--- + +## 14. Minor Fixes + +**14.1** `GsConverter.asLocalDate(Number val)` — change `val.intValue()` to `val.longValue()`: +```groovy +static LocalDate asLocalDate(Number val) { + def daysSinceEpoch = val.longValue() + EPOCH_DATE.plusDays(daysSinceEpoch) +} +``` + +**14.2** `GsUtil.getSheetNames(String, Sheets)` — narrow catch from `Exception` to `IOException`: +```groovy +try { + // ... +} catch (IOException e) { + log.error("Failed to retrieve sheet names for spreadsheetId '$spreadsheetId': ${e.message}", e) + throw e +} +``` +_Note:_ `IOException` is a checked exception. After narrowing, the method signature should declare `throws IOException` (or callers must catch it). + +**14.3** `GsAuthenticator.main(String[])` — add a GroovyDoc note clarifying it is a dev convenience, without changing visibility (avoids potential `@CompileStatic` + `@PackageScope` issues on a method boundary): +```groovy +/** + * Dev convenience entry point — not part of the public API. + */ +static void main(String[] args) { +``` + +**14.4** `GsConverter` — add missing return type declarations on two public methods: +```groovy +@CompileDynamic +static LocalTime asLocalTime(Object o) { ... } + +static BigDecimal asSerial(Date date) { ... } +``` + +**Test command:** +```bash +./gradlew :matrix-gsheets:test -g ./.gradle-user +``` + +--- + +## Quality Gates + +After all tasks are complete, run the full module quality check: + +```bash +./gradlew :matrix-gsheets:spotlessApply :matrix-gsheets:spotlessCheck :matrix-gsheets:codenarcMain :matrix-gsheets:codenarcTest :matrix-gsheets:test -g ./.gradle-user +``` + +If external tests are available: +```bash +RUN_EXTERNAL_TESTS=true ./gradlew :matrix-gsheets:test -g ./.gradle-user +``` diff --git a/matrix-gsheets/req/v0.2.0-roadmap.md b/matrix-gsheets/req/v0.2.0-roadmap.md new file mode 100644 index 00000000..286034ea --- /dev/null +++ b/matrix-gsheets/req/v0.2.0-roadmap.md @@ -0,0 +1,100 @@ +# Matrix-gsheets 0.2.0 + +## Issues + +1. sanitizeSheetName, toCell, and MAX_SHEET_NAME_LENGTH are duplicated + + GsheetsWriter (lines 98, 218–251) has identical copies of sanitizeSheetName, toCell, and the constant MAX_SHEET_NAME_LENGTH = 100 that also exist in GsUtil (lines 28, 184–216). GsheetsWriter.write() calls its own @PackageScope versions instead of delegating to GsUtil. Tests in GsExporterTest already call GsUtil directly — the GsheetsWriter copies are dead weight that will drift. + + Fix: remove the @PackageScope copies from GsheetsWriter and call GsUtil.toCell(...) / GsUtil.sanitizeSheetName(...) directly in write(). + +2. Explicit return on final expressions (violates AGENTS.md idiom) + + Widespread use of return on the last line. Examples: + - GsUtil.groovy:106 — return endColIndex - startColIndex + 1 + - GsUtil.groovy:127 — return number + - GsUtil.groovy:149 — return getSheetNames(spreadsheetId, sheetsService) + - GsheetsReader.groovy:83 — return readAsStrings(...) + - GsheetsWriter.groovy:214 — return spreadsheetId + - GsConverter.groovy:56 — return EPOCH_DATE.plusDays(daysSinceEpoch) + + AGENTS.md: "Use the return keyword only when we need to return early." + + +3. Logging indentation bug in BqAuthenticator.groovy:91-93 + + if (qp) { + credentials = credentials.createWithQuotaProject(qp) + if (verbose) { + log.info("Using quota project: $qp") // <-- wrong indentation: inside the if block + } + + The log.info call is at the same indent level as the closing }, which is the outer if (qp) block, not the inner if (verbose). Formatting makes it hard to see the true structure. + + +4. Static field ordering hazard in BqAuthenticator.groovy + + ADC_FILE_PATH at line 38 references PROP_USER_HOME which is declared at line 44. In standard JVM field initialization order, PROP_USER_HOME would be null when ADC_FILE_PATH is initialized. This works in practice because 'user.home' is a compile-time constant that the Groovy compiler inlines, but the ordering is fragile and should be fixed by moving all constant strings before the fields that use them. + +5. Commented-out code should be removed + + - BqAuthUtils.groovy:114 — old tokeninfo URL left as a comment + - BqAuthUtils.groovy:29-38 — large block comment at the top of loginAndWriteAdc is usage docs that belong in the GroovyDoc, not inside the method body + +6. Code duplication in GsheetsReader — service setup repeated + + readAsStrings (lines 193–205) and readAsObject (lines 131–143) duplicate the identical 8-line transport/credentials/service setup block. This should be extracted to a private helper like buildSheetsService(GoogleCredentials). + + +## Usability Enhancements + +7. No URL convenience method after write + + The Javadoc for GsheetsWriter.write() shows users how to construct the URL manually: + https://docs.google.com/spreadsheets/d/${spreadsheetId}/edit + A simple static helper would significantly improve UX: + static String spreadsheetUrl(String spreadsheetId) { + "https://docs.google.com/spreadsheets/d/${spreadsheetId}/edit" + } + +8. No write-to-existing-spreadsheet API + + GsheetsWriter only creates new spreadsheets. There is no update(String spreadsheetId, String range, Matrix matrix) overload for users who want to push data to an existing sheet or update a specific range. This is a common workflow gap. + +9. Confusing Bq* naming + + BqAuthenticator and BqAuthUtils have Bq (BigQuery) prefixes but are used exclusively for Google Sheets authentication. For new users this is confusing. Consider + either renaming to GsAuthenticator / GsAuthUtils in a future version, or at minimum adding a GroovyDoc note explaining the historical naming. + +10. Matrix name derived from range when no sheet prefix is present + + In GsheetsReader.readAsStrings line 225 and readAsObject line 170: + def sheetName = range.split(SHEET_NAME_SEPARATOR)[0] + If the range has no ! (e.g., 'A1:D10'), sheetName becomes the range string itself ('A1:D10'), which makes a poor matrix name. Should default to an empty string or null when no sheet name is present. + + --- +## Test Issues + +11. Misleading test comments in GsheetsReaderTest and GsheetsWriterTest + + - GsheetsReaderTest.testMethodsDelegateToGsImporter says "GsheetsReader properly delegates to GsImporter" — this is backwards; GsImporter delegates to GsheetsReader, not vice versa. + - GsheetsWriterTest.testMethodDelegatesToGsExporter has the same inversion. + + Both tests use reflection to check method signatures — a brittle approach that doesn't add value over just calling the method. + +12. toCell and sanitizeSheetName tests belong in GsUtilTest, not GsExporterTest + + The test class GsExporterTest tests GsUtil.sanitizeSheetName and GsUtil.toCell — not anything specific to GsExporter. These should live in GsUtilTest. + +13. Missing positive-case tests for GsConverter.toSerials + + GsConverterTest only has testToSerialsWithNull. There are no tests for actual conversion via toSerials, unlike the equivalent toLocalDates, toLocalDateTimes, and toLocalTimes methods which have positive tests. + + --- +## Minor + +- GsConverter.asLocalDate(Number val) uses val.intValue() which is semantically correct but longValue() would be more semantically accurate for a day count. + +- GsUtil.getSheetNames(String, Sheets) catches Exception broadly on line 178 — should catch IOException specifically to avoid swallowing programming errors. + +- The main(String[]) method in BqAuthenticator is a convenient dev tool but could be moved to an example class to keep the API surface clean. diff --git a/matrix-gsheets/src/main/groovy/se/alipsa/matrix/gsheets/BqAuthUtils.groovy b/matrix-gsheets/src/main/groovy/se/alipsa/matrix/gsheets/GsAuthUtils.groovy similarity index 83% rename from matrix-gsheets/src/main/groovy/se/alipsa/matrix/gsheets/BqAuthUtils.groovy rename to matrix-gsheets/src/main/groovy/se/alipsa/matrix/gsheets/GsAuthUtils.groovy index 9f7aafcd..5768706e 100644 --- a/matrix-gsheets/src/main/groovy/se/alipsa/matrix/gsheets/BqAuthUtils.groovy +++ b/matrix-gsheets/src/main/groovy/se/alipsa/matrix/gsheets/GsAuthUtils.groovy @@ -1,6 +1,6 @@ package se.alipsa.matrix.gsheets -import static se.alipsa.matrix.gsheets.BqAuthenticator.* +import static se.alipsa.matrix.gsheets.GsAuthenticator.* import groovy.json.JsonOutput import groovy.json.JsonSlurper @@ -20,20 +20,32 @@ import com.google.auth.oauth2.GoogleCredentials import se.alipsa.matrix.core.util.Logger -class BqAuthUtils { +/** + * OAuth2 / ADC token utilities for Google Sheets authentication. + * + *

Note on naming: The "Bq" prefix (short for BigQuery) is historical. + * These classes handle authentication for Google Sheets, not BigQuery. + * The name is retained for backward compatibility. + */ +class GsAuthUtils { - private static final Logger log = Logger.getLogger(BqAuthUtils) + private static final Logger log = Logger.getLogger(GsAuthUtils) - /* drop-in replacement for gcloud auth application-default login - // Writes ~/.config/gcloud/application_default_credentials.json - // Usage: - def creds = loginAndWriteAdc( - new File("$HOME/client_secret_desktop.json"), - [com.google.api.services.sheets.v4.SheetsScopes.SPREADSHEETS, - 'https://www.googleapis.com/auth/drive.file', - 'openid', 'email'], - "$PROJECT_ID" // optional; mainly needed if you still call Google OAuth2 userinfo via googleapis.com - )*/ + /** + * Drop-in replacement for {@code gcloud auth application-default login}. + * Writes {@code ~/.config/gcloud/application_default_credentials.json}. + * + *

Usage: + *

{@code
+   * def creds = loginAndWriteAdc(
+   *     new File("$HOME/client_secret_desktop.json"),
+   *     [com.google.api.services.sheets.v4.SheetsScopes.SPREADSHEETS,
+   *      'https://www.googleapis.com/auth/drive.file',
+   *      'openid', 'email'],
+   *     "$PROJECT_ID" // optional; mainly needed if you still call Google OAuth2 userinfo via googleapis.com
+   * )
+   * }
+ */ static GoogleCredentials loginAndWriteAdc(File clientSecretJson, List scopes, String quotaProjectId = null) { def http = GoogleNetHttpTransport.newTrustedTransport() def json = GsonFactory.getDefaultInstance() @@ -73,7 +85,7 @@ class BqAuthUtils { if (quotaProjectId) adc.quota_project_id = quotaProjectId // Persist to the standard ADC path - File adcFile = BqAuthenticator.ADC_FILE_PATH + File adcFile = GsAuthenticator.ADC_FILE_PATH adcFile.parentFile.mkdirs() adcFile.text = JsonOutput.prettyPrint(JsonOutput.toJson(adc)) log.info("Wrote ADC to ${adcFile.absolutePath}") @@ -109,7 +121,6 @@ class BqAuthUtils { } def token = creds.accessToken?.tokenValue if (!token) return false - //def url = new URI("https://www.googleapis.com/oauth2/v1/tokeninfo?access_token=${token}").toURL() // old url def url = new URI("https://oauth2.googleapis.com/tokeninfo?access_token=${URLEncoder.encode(token,'UTF-8')}").toURL() def json = new JsonSlurper().parse(url) def granted = (json?.scope ?: '') diff --git a/matrix-gsheets/src/main/groovy/se/alipsa/matrix/gsheets/BqAuthenticator.groovy b/matrix-gsheets/src/main/groovy/se/alipsa/matrix/gsheets/GsAuthenticator.groovy similarity index 94% rename from matrix-gsheets/src/main/groovy/se/alipsa/matrix/gsheets/BqAuthenticator.groovy rename to matrix-gsheets/src/main/groovy/se/alipsa/matrix/gsheets/GsAuthenticator.groovy index e78129d6..41e80750 100755 --- a/matrix-gsheets/src/main/groovy/se/alipsa/matrix/gsheets/BqAuthenticator.groovy +++ b/matrix-gsheets/src/main/groovy/se/alipsa/matrix/gsheets/GsAuthenticator.groovy @@ -16,10 +16,14 @@ import java.util.concurrent.atomic.AtomicBoolean /** * Checks for Google Cloud authentication by checking for Application Default Credentials (ADC) * and delegates to the 'gcloud' SDK for an interactive login if needed. + * + *

Note on naming: The "Bq" prefix (short for BigQuery) is historical. + * These classes handle authentication for Google Sheets, not BigQuery. + * The name is retained for backward compatibility. */ -class BqAuthenticator { +class GsAuthenticator { - private static final Logger log = Logger.getLogger(BqAuthenticator) + private static final Logger log = Logger.getLogger(GsAuthenticator) static final String SCOPE_CLOUD_PLATFORM = 'https://www.googleapis.com/auth/cloud-platform' static final String SCOPE_SHEETS = SheetsScopes.SPREADSHEETS @@ -30,7 +34,9 @@ class BqAuthenticator { private static final AtomicBoolean LOGIN_ATTEMPTED = new AtomicBoolean(false) - private BqAuthenticator() { } + private GsAuthenticator() { } + + private static final String PROP_USER_HOME = 'user.home' // The location where gcloud stores Application Default Credentials static final File ADC_FILE_PATH = new File(System.getProperty(PROP_USER_HOME), '.config/gcloud/application_default_credentials.json') @@ -39,7 +45,6 @@ class BqAuthenticator { private static final String GCLOUD_AUTH = 'auth' private static final String GCLOUD_APP_DEFAULT = 'application-default' private static final String ENV_GOOGLE_CLOUD_PROJECT = 'GOOGLE_CLOUD_PROJECT' - private static final String PROP_USER_HOME = 'user.home' static final List SCOPES = [ SCOPE_CLOUD_PLATFORM, @@ -85,8 +90,8 @@ class BqAuthenticator { if (qp) { credentials = credentials.createWithQuotaProject(qp) if (verbose) { - log.info("Using quota project: $qp") - } + log.info("Using quota project: $qp") + } } // The refreshIfExpired() method will handle checking if a refresh is needed. @@ -151,7 +156,7 @@ class BqAuthenticator { def home = System.getProperty(PROP_USER_HOME) def clientSecretFile = new File("$home/client_secret_desktop.json") - def credentials = BqAuthUtils.loginAndWriteAdc(clientSecretFile, scopes, quotaProjectId) + def credentials = GsAuthUtils.loginAndWriteAdc(clientSecretFile, scopes, quotaProjectId) // If credentials are null, it means the process failed. return credentials != null @@ -172,7 +177,7 @@ class BqAuthenticator { try { def http = GoogleNetHttpTransport.newTrustedTransport() def json = GsonFactory.getDefaultInstance() - def init = BqAuthUtils.noUserProjectInitializer(credentials) + def init = GsAuthUtils.noUserProjectInitializer(credentials) def oauth2 = new Oauth2.Builder(http, json, init) .setApplicationName('Matrix GSheets') .build() @@ -212,7 +217,7 @@ class BqAuthenticator { } def creds = getCredentials(new ArrayList<>(scopes)) - if (creds == null || !BqAuthUtils.hasAllScopes(creds, scopes)) { + if (creds == null || !GsAuthUtils.hasAllScopes(creds, scopes)) { // Do ONE interactive login (ADC or programmatic), then reload creds if (LOGIN_ATTEMPTED.compareAndSet(false, true)) { boolean ok = isCommandAvailable(GCLOUD_CMD) @@ -301,7 +306,9 @@ class BqAuthenticator { } } - // --- Main execution --- + /** + * Dev convenience entry point — not part of the public API. + */ static void main(String[] args) { if (args.length > 0) { authenticate(args.collect() as List) diff --git a/matrix-gsheets/src/main/groovy/se/alipsa/matrix/gsheets/GsConverter.groovy b/matrix-gsheets/src/main/groovy/se/alipsa/matrix/gsheets/GsConverter.groovy index 4022b21f..f3601bca 100644 --- a/matrix-gsheets/src/main/groovy/se/alipsa/matrix/gsheets/GsConverter.groovy +++ b/matrix-gsheets/src/main/groovy/se/alipsa/matrix/gsheets/GsConverter.groovy @@ -52,8 +52,8 @@ class GsConverter { } static LocalDate asLocalDate(Number val) { - def daysSinceEpoch = val.intValue() - return EPOCH_DATE.plusDays(daysSinceEpoch) + def daysSinceEpoch = val.longValue() + EPOCH_DATE.plusDays(daysSinceEpoch) } static List toLocalDates(List list) { @@ -64,7 +64,7 @@ class GsConverter { for (Object o : list) { dates.add(asLocalDate(o)) } - return dates + dates } static LocalDateTime asLocalDateTime(Object o, DateTimeFormatter formatter = DateTimeFormatter.ISO_LOCAL_DATE_TIME) { @@ -78,7 +78,7 @@ class GsConverter { return asLocalDateTime((Number) o) } try { - return LocalDateTime.parse(o.toString(), formatter) + LocalDateTime.parse(o.toString(), formatter) } catch (DateTimeParseException e) { // Try parsing as a numeric serial value log.debug("Failed to parse '$o' as datetime with formatter $formatter, attempting numeric conversion: ${e.message}") @@ -115,9 +115,10 @@ class GsConverter { for (Object o : list) { dateTimes.add(asLocalDateTime(o)) } - return dateTimes + dateTimes } + @groovy.transform.CompileDynamic static LocalTime asLocalTime(Object o) { if (o == null) { return null @@ -129,7 +130,7 @@ class GsConverter { return asLocalTime((Number) o) } try { - return LocalTime.parse(o.toString()) + LocalTime.parse(o.toString()) } catch (DateTimeParseException e) { // Try parsing as a numeric serial value log.debug("Failed to parse '$o' as time, attempting numeric conversion: ${e.message}") @@ -146,7 +147,7 @@ class GsConverter { long totalSeconds = ((val as BigDecimal) * SECONDS_PER_DAY).round() as long // Create a LocalTime object from the total seconds - return LocalTime.ofSecondOfDay(totalSeconds) + LocalTime.ofSecondOfDay(totalSeconds) } static List toLocalTimes(List list) { @@ -157,7 +158,7 @@ class GsConverter { for (Object o : list) { times.add(asLocalTime(o)) } - return times + times } static BigDecimal asSerial(LocalDate date) { @@ -168,7 +169,7 @@ class GsConverter { // Calculate the number of days since the epoch long days = ChronoUnit.DAYS.between(EPOCH_DATE, date) - return days + days } static BigDecimal asSerial(LocalDateTime dateTime) { @@ -183,20 +184,20 @@ class GsConverter { def secondsSinceMidnight = dateTime.toLocalTime().toSecondOfDay() def fraction = secondsSinceMidnight / (SECONDS_PER_DAY as BigDecimal) - return days + fraction + days + fraction } static BigDecimal asSerial(LocalTime time) { long totalSecondsInDay = SECONDS_PER_DAY long secondsSinceMidnight = time.toSecondOfDay() - return secondsSinceMidnight / totalSecondsInDay + secondsSinceMidnight / totalSecondsInDay } static BigDecimal asSerial(Date date) { if (date == null) { return null } - return asSerial(new Timestamp(date.getTime()).toLocalDateTime()) + asSerial(new Timestamp(date.getTime()).toLocalDateTime()) } static BigDecimal asSerial(Object o) { @@ -220,7 +221,7 @@ class GsConverter { for (Object o : list) { serials.add(asSerial(o)) } - return serials + serials } } diff --git a/matrix-gsheets/src/main/groovy/se/alipsa/matrix/gsheets/GsExporter.groovy b/matrix-gsheets/src/main/groovy/se/alipsa/matrix/gsheets/GsExporter.groovy index 2407a22a..efed33b3 100644 --- a/matrix-gsheets/src/main/groovy/se/alipsa/matrix/gsheets/GsExporter.groovy +++ b/matrix-gsheets/src/main/groovy/se/alipsa/matrix/gsheets/GsExporter.groovy @@ -22,7 +22,7 @@ import java.time.LocalDateTime * *

Authentication

* If no credentials are provided, the exporter will attempt to use Application Default Credentials (ADC). - * For interactive authentication, use {@link BqAuthenticator#authenticate()}. + * For interactive authentication, use {@link GsAuthenticator#authenticate()}. * *

Setup Requirements

* Before using the exporter, ensure your Google Cloud project has the required APIs enabled: diff --git a/matrix-gsheets/src/main/groovy/se/alipsa/matrix/gsheets/GsImporter.groovy b/matrix-gsheets/src/main/groovy/se/alipsa/matrix/gsheets/GsImporter.groovy index 38c89aef..0d12238a 100644 --- a/matrix-gsheets/src/main/groovy/se/alipsa/matrix/gsheets/GsImporter.groovy +++ b/matrix-gsheets/src/main/groovy/se/alipsa/matrix/gsheets/GsImporter.groovy @@ -21,7 +21,7 @@ import se.alipsa.matrix.core.* * *

Authentication

* If no credentials are provided, the importer will attempt to use Application Default Credentials (ADC). - * For interactive authentication, use {@link BqAuthenticator#authenticate()}. + * For interactive authentication, use {@link GsAuthenticator#authenticate()}. * *

Usage Examples

*
{@code
diff --git a/matrix-gsheets/src/main/groovy/se/alipsa/matrix/gsheets/GsUtil.groovy b/matrix-gsheets/src/main/groovy/se/alipsa/matrix/gsheets/GsUtil.groovy
index e0d3a807..cf512271 100644
--- a/matrix-gsheets/src/main/groovy/se/alipsa/matrix/gsheets/GsUtil.groovy
+++ b/matrix-gsheets/src/main/groovy/se/alipsa/matrix/gsheets/GsUtil.groovy
@@ -34,8 +34,8 @@ class GsUtil {
       throw new IllegalArgumentException(SPREADSHEET_ID_ERROR)
     }
 
-    def scopes = ['https://www.googleapis.com/auth/drive'] + BqAuthenticator.SCOPES
-    def credentials = BqAuthenticator.authenticate(scopes)
+    def scopes = ['https://www.googleapis.com/auth/drive'] + GsAuthenticator.SCOPES
+    def credentials = GsAuthenticator.authenticate(scopes)
     HttpRequestInitializer cred = new HttpCredentialsAdapter(credentials)
     def transport = GoogleNetHttpTransport.newTrustedTransport()
     def gsonFactory = GsonFactory.getDefaultInstance()
@@ -106,7 +106,7 @@ class GsUtil {
     int endColIndex = asColumnNumber(endColumnLetters)
 
     // Calculate the number of columns
-    return endColIndex - startColIndex + 1
+    endColIndex - startColIndex + 1
   }
 
   /**
@@ -127,7 +127,7 @@ class GsUtil {
     for (int i = 0; i < colName.length(); i++) {
       number = number * 26 + (colName.charAt(i) - ('A' as char - 1))
     }
-    return number
+    number
   }
 
   static List getSheetNames(String spreadsheetId, GoogleCredentials credentials = null) {
@@ -139,7 +139,7 @@ class GsUtil {
     def gsonFactory = GsonFactory.getDefaultInstance()
 
     if (credentials == null) {
-      credentials = BqAuthenticator.authenticate(BqAuthenticator.SCOPE_SHEETS_READONLY)
+      credentials = GsAuthenticator.authenticate(GsAuthenticator.SCOPE_SHEETS_READONLY)
     }
 
     def sheetsService = new Sheets.Builder(
@@ -149,7 +149,7 @@ class GsUtil {
         .setApplicationName('Groovy Sheets Reader')
         .build()
 
-    return getSheetNames(spreadsheetId, sheetsService)
+    getSheetNames(spreadsheetId, sheetsService)
   }
 
   /**
@@ -160,7 +160,7 @@ class GsUtil {
    * @param sheetsService The Sheets service to use
    * @return List of sheet names
    */
-  static List getSheetNames(String spreadsheetId, Sheets sheetsService) {
+  static List getSheetNames(String spreadsheetId, Sheets sheetsService) throws IOException {
     if (spreadsheetId == null || spreadsheetId.trim().isEmpty()) {
       throw new IllegalArgumentException(SPREADSHEET_ID_ERROR)
     }
@@ -177,8 +177,8 @@ class GsUtil {
         names.add(sheet.getProperties().getTitle())
       }
 
-      return names
-    } catch (Exception e) {
+      names
+    } catch (IOException e) {
       log.error("Failed to retrieve sheet names for spreadsheetId '$spreadsheetId': ${e.message}", e)
       throw e
     }
@@ -190,7 +190,7 @@ class GsUtil {
     if (s.length() > MAX_SHEET_NAME_LENGTH) {
       s = s.substring(0, MAX_SHEET_NAME_LENGTH)
     }
-    return s.trim().isEmpty() ? 'Sheet1' : s
+    s.trim().isEmpty() ? 'Sheet1' : s
   }
 
   static Object toCell(Object v, boolean convertNullsToEmptyString, boolean convertDatesToSerial) {
@@ -215,7 +215,7 @@ class GsUtil {
         return GsConverter.asSerial(v as LocalTime)
       }
     }
-    return String.valueOf(v)
+    String.valueOf(v)
   }
 
   static void validateRange(String range) {
diff --git a/matrix-gsheets/src/main/groovy/se/alipsa/matrix/gsheets/GsheetsReader.groovy b/matrix-gsheets/src/main/groovy/se/alipsa/matrix/gsheets/GsheetsReader.groovy
index 91bada83..6a5e4602 100644
--- a/matrix-gsheets/src/main/groovy/se/alipsa/matrix/gsheets/GsheetsReader.groovy
+++ b/matrix-gsheets/src/main/groovy/se/alipsa/matrix/gsheets/GsheetsReader.groovy
@@ -16,7 +16,7 @@ import se.alipsa.matrix.core.Matrix
  *
  * 

Authentication

* If no credentials are provided, the reader will attempt to use Application Default Credentials (ADC). - * For interactive authentication, use {@link BqAuthenticator#authenticate()}. + * For interactive authentication, use {@link GsAuthenticator#authenticate()}. * *

Usage Examples

*
{@code
@@ -50,6 +50,7 @@ class GsheetsReader {
 
   private static final String APP_NAME = 'Groovy Sheets Reader'
   private static final String SHEET_NAME_SEPARATOR = '!'
+  private static final int RANGE_SPLIT_LIMIT = 2
 
   /**
    * Reads data from a Google Sheets spreadsheet as formatted strings.
@@ -77,7 +78,7 @@ class GsheetsReader {
    * @see #readAsObject(String, String, boolean, GoogleCredentials, boolean)
    */
   static Matrix read(String spreadsheetId, String range, boolean firstRowAsColumnNames, GoogleCredentials credentials = null) {
-    return readAsStrings(spreadsheetId, range, firstRowAsColumnNames, credentials)
+    readAsStrings(spreadsheetId, range, firstRowAsColumnNames, credentials)
   }
 
   /**
@@ -125,18 +126,7 @@ class GsheetsReader {
     GsUtil.validateSheetId(spreadsheetId)
     GsUtil.validateRange(range)
 
-    def transport = GoogleNetHttpTransport.newTrustedTransport()
-    def gsonFactory = GsonFactory.getDefaultInstance()
-
-    if (credentials == null) {
-      credentials = BqAuthenticator.authenticate(BqAuthenticator.SCOPE_SHEETS_READONLY)
-    }
-    def sheetsService = new Sheets.Builder(
-        transport,
-        gsonFactory,
-        new HttpCredentialsAdapter(credentials))
-        .setApplicationName(APP_NAME)
-        .build()
+    def sheetsService = buildSheetsService(credentials)
 
     def response = sheetsService
         .spreadsheets()
@@ -164,7 +154,8 @@ class GsheetsReader {
       GsUtil.fillListToSize(row, ncol)
     }
 
-    def sheetName = range.split(SHEET_NAME_SEPARATOR)[0]
+    def rangeParts = range.split(SHEET_NAME_SEPARATOR, RANGE_SPLIT_LIMIT)
+    def sheetName = rangeParts.size() > 1 ? rangeParts[0] : ''
     Matrix.builder(sheetName)
         .rows(values)
         .columnNames(headers)
@@ -188,18 +179,7 @@ class GsheetsReader {
     GsUtil.validateSheetId(spreadsheetId)
     GsUtil.validateRange(range)
 
-    def transport = GoogleNetHttpTransport.newTrustedTransport()
-    def gsonFactory = GsonFactory.getDefaultInstance()
-
-    if (credentials == null) {
-      credentials = BqAuthenticator.authenticate(BqAuthenticator.SCOPE_SHEETS_READONLY)
-    }
-    def sheetsService = new Sheets.Builder(
-        transport,
-        gsonFactory,
-        new HttpCredentialsAdapter(credentials))
-        .setApplicationName(APP_NAME)
-        .build()
+    def sheetsService = buildSheetsService(credentials)
 
     def request = sheetsService
         .spreadsheets()
@@ -219,7 +199,8 @@ class GsheetsReader {
       headers = Matrix.anonymousHeader(ncol)
     }
 
-    def sheetName = range.split(SHEET_NAME_SEPARATOR)[0]
+    def rangeParts = range.split(SHEET_NAME_SEPARATOR, RANGE_SPLIT_LIMIT)
+    def sheetName = rangeParts.size() > 1 ? rangeParts[0] : ''
     List> rows = []
     values.each { valueRow ->
       List row = []
@@ -245,6 +226,16 @@ class GsheetsReader {
         .build()
   }
 
+  private static Sheets buildSheetsService(GoogleCredentials credentials) {
+    def creds = credentials ?: GsAuthenticator.authenticate(GsAuthenticator.SCOPE_SHEETS_READONLY)
+    new Sheets.Builder(
+        GoogleNetHttpTransport.newTrustedTransport(),
+        GsonFactory.getDefaultInstance(),
+        new HttpCredentialsAdapter(creds))
+        .setApplicationName(APP_NAME)
+        .build()
+  }
+
   private static void convertEmptyToNull(List row) {
     for (int c = 0; c < row.size(); c++) {
       Object v = row.get(c)
diff --git a/matrix-gsheets/src/main/groovy/se/alipsa/matrix/gsheets/GsheetsWriter.groovy b/matrix-gsheets/src/main/groovy/se/alipsa/matrix/gsheets/GsheetsWriter.groovy
index 107a3c8a..c096224c 100644
--- a/matrix-gsheets/src/main/groovy/se/alipsa/matrix/gsheets/GsheetsWriter.groovy
+++ b/matrix-gsheets/src/main/groovy/se/alipsa/matrix/gsheets/GsheetsWriter.groovy
@@ -1,9 +1,7 @@
 package se.alipsa.matrix.gsheets
 
-import static se.alipsa.matrix.gsheets.BqAuthenticator.authenticate
-import static se.alipsa.matrix.gsheets.BqAuthenticator.getSCOPES
-
-import groovy.transform.PackageScope
+import static se.alipsa.matrix.gsheets.GsAuthenticator.authenticate
+import static se.alipsa.matrix.gsheets.GsAuthenticator.getSCOPES
 
 import com.google.api.client.googleapis.javanet.GoogleNetHttpTransport
 import com.google.api.client.http.HttpRequestInitializer
@@ -20,9 +18,7 @@ import com.google.auth.oauth2.GoogleCredentials
 import se.alipsa.matrix.core.Matrix
 import se.alipsa.matrix.core.Row
 
-import java.time.LocalDate
 import java.time.LocalDateTime
-import java.time.LocalTime
 
 /**
  * Writes Matrix data to Google Sheets spreadsheets.
@@ -32,7 +28,7 @@ import java.time.LocalTime
  *
  * 

Authentication

* If no credentials are provided, the writer will attempt to use Application Default Credentials (ADC). - * For interactive authentication, use {@link BqAuthenticator#authenticate()}. + * For interactive authentication, use {@link GsAuthenticator#authenticate()}. * *

Setup Requirements

* Before using the writer, ensure your Google Cloud project has the required APIs enabled: @@ -64,7 +60,7 @@ import java.time.LocalTime * * // Write to Google Sheets * String spreadsheetId = GsheetsWriter.write(employees) - * println "View at: https://docs.google.com/spreadsheets/d/${spreadsheetId}/edit" + * println "View at: ${GsheetsWriter.spreadsheetUrl(spreadsheetId)}" * * // Write with date conversion * Matrix withDates = Matrix.builder('Sales Data') @@ -93,7 +89,12 @@ import java.time.LocalTime */ class GsheetsWriter { - private static final int MAX_SHEET_NAME_LENGTH = 100 + private static final String MATRIX_NULL_ERROR = 'matrix must not be null' + private static final String MATRIX_NO_COLUMNS_ERROR = 'matrix must have at least one column' + private static final String MATRIX_NO_ROWS_ERROR = 'matrix must have at least one row' + private static final String APP_NAME = 'Matrix GSheets' + private static final String ROWS_DIMENSION = 'ROWS' + private static final String RAW_INPUT = 'RAW' /** * Creates a new Google Spreadsheet and writes the Matrix data to it. @@ -120,26 +121,25 @@ class GsheetsWriter { * @param convertNullsToEmptyString If true, null values become empty strings; if false, remain null * @param convertDatesToSerial If true, date/time types are converted to Google Sheets serial numbers; * if false, they are written as ISO-8601 strings - * @return The spreadsheet ID of the created spreadsheet (use with - * https://docs.google.com/spreadsheets/d/{spreadsheetId}/edit) + * @return The spreadsheet ID of the created spreadsheet (open with {@link #spreadsheetUrl(String)}) * @throws IllegalArgumentException if matrix is null, has no columns, or has no rows * @throws SheetOperationException if spreadsheet creation or data writing fails * @see GsConverter#asSerial(java.time.LocalDate) * @see GsConverter#asSerial(java.time.LocalDateTime) */ static String write(Matrix matrix, boolean convertNullsToEmptyString = true, boolean convertDatesToSerial = false) { - return write(matrix, null, convertNullsToEmptyString, convertDatesToSerial) + write(matrix, null, convertNullsToEmptyString, convertDatesToSerial) } static String write(Matrix matrix, GoogleCredentials credentials, boolean convertNullsToEmptyString = true, boolean convertDatesToSerial = false) { if (matrix == null) { - throw new IllegalArgumentException('matrix must not be null') + throw new IllegalArgumentException(MATRIX_NULL_ERROR) } if (matrix.columnCount() == 0) { - throw new IllegalArgumentException('matrix must have at least one column') + throw new IllegalArgumentException(MATRIX_NO_COLUMNS_ERROR) } if (matrix.rowCount() == 0) { - throw new IllegalArgumentException('matrix must have at least one row') + throw new IllegalArgumentException(MATRIX_NO_ROWS_ERROR) } def transport = GoogleNetHttpTransport.newTrustedTransport() @@ -152,11 +152,11 @@ class GsheetsWriter { HttpRequestInitializer cred = new HttpCredentialsAdapter(credentials) Sheets sheets = new Sheets.Builder(transport, gsonFactory, cred) - .setApplicationName('Matrix GSheets') + .setApplicationName(APP_NAME) .build() String titleBase = matrix.matrixName ?: "Matrix ${LocalDateTime.now().toString().replace('T', '_')}" - String sheetName = sanitizeSheetName(titleBase) + String sheetName = GsUtil.sanitizeSheetName(titleBase) String spreadsheetTitle = titleBase // 1) Create an empty spreadsheet with one sheet named after the matrix @@ -180,72 +180,105 @@ class GsheetsWriter { // 2) Build the data: header row + data rows List headers = (List) matrix.columnNames() - List> values = [] - - // header row - values.add(new ArrayList(headers)) + List> values = [new ArrayList(headers)] // data rows matrix.each { Row it -> - List row = new ArrayList<>(headers.size()) - it.each { v -> - row.add(toCell(v, convertNullsToEmptyString, convertDatesToSerial)) - } - values.add(row) + values << it.collect { GsUtil.toCell(it, convertNullsToEmptyString, convertDatesToSerial) } } // 3) Write all values starting at A1 ValueRange vr = new ValueRange() .setRange("${sheetName}!A1") - .setMajorDimension('ROWS') + .setMajorDimension(ROWS_DIMENSION) .setValues(values) try { sheets.spreadsheets().values() .update(spreadsheetId, "${sheetName}!A1", vr) - .setValueInputOption('RAW') // don't coerce; write exact values/strings + .setValueInputOption(RAW_INPUT) // don't coerce; write exact values/strings .execute() } catch (IOException e) { throw new SheetOperationException('write data', spreadsheetId, e) } - return spreadsheetId + spreadsheetId } - @PackageScope - static String sanitizeSheetName(String name) { - // Google Sheets sheet names cannot contain: : \ / ? * [ ] - String s = name.replaceAll('[:\\\\/?*\\[\\]]', ' ') - if (s.length() > MAX_SHEET_NAME_LENGTH) { - s = s.substring(0, MAX_SHEET_NAME_LENGTH) - } - return s.trim().isEmpty() ? 'Sheet1' : s + /** + * Returns the Google Sheets edit URL for a spreadsheet ID. + * + * @param spreadsheetId The spreadsheet ID + * @return The full URL to open the spreadsheet in a browser + */ + static String spreadsheetUrl(String spreadsheetId) { + "https://docs.google.com/spreadsheets/d/${spreadsheetId}/edit" } - @PackageScope - static Object toCell(Object v, boolean convertNullsToEmptyString, boolean convertDatesToSerial) { - if (v == null) { - return convertNullsToEmptyString ? '' : null + /** + * Updates an existing Google Spreadsheet with Matrix data. + * + * @param spreadsheetId The ID of the existing spreadsheet + * @param range The target range in A1 notation (e.g., 'Sheet1!A1') + * @param matrix The Matrix to write + * @param credentials Google Cloud credentials, or null to use ADC + * @param convertNullsToEmptyString If true, null values become empty strings + * @param convertDatesToSerial If true, date/time types are converted to serial numbers + * @return The spreadsheetId + * @throws IllegalArgumentException if any required parameter is null/empty + * @throws SheetOperationException if the update fails + */ + static String update(String spreadsheetId, String range, Matrix matrix, + GoogleCredentials credentials = null, + boolean convertNullsToEmptyString = true, + boolean convertDatesToSerial = false) { + GsUtil.validateSheetId(spreadsheetId) + GsUtil.validateRange(range) + if (matrix == null) { + throw new IllegalArgumentException(MATRIX_NULL_ERROR) + } + if (matrix.columnCount() == 0) { + throw new IllegalArgumentException(MATRIX_NO_COLUMNS_ERROR) + } + if (matrix.rowCount() == 0) { + throw new IllegalArgumentException(MATRIX_NO_ROWS_ERROR) + } + + def transport = GoogleNetHttpTransport.newTrustedTransport() + def gsonFactory = GsonFactory.getDefaultInstance() + + if (credentials == null) { + credentials = authenticate(SCOPES) } - if (v in Number || v in Boolean) { - return v + HttpRequestInitializer cred = new HttpCredentialsAdapter(credentials) + + Sheets sheets = new Sheets.Builder(transport, gsonFactory, cred) + .setApplicationName(APP_NAME) + .build() + + // Build data: header row + data rows + List headers = (List) matrix.columnNames() + List> values = [new ArrayList(headers)] + + matrix.each { Row it -> + values << it.collect { GsUtil.toCell(it, convertNullsToEmptyString, convertDatesToSerial) } } - // Dates/LocalDates/etc. are written as ISO strings unless you convert them to serial numbers yourself. - if (convertDatesToSerial) { - if (v in LocalDate) { - return GsConverter.asSerial(v as LocalDate) - } - if (v in LocalDateTime) { - return GsConverter.asSerial(v as LocalDateTime) - } - if (v in Date) { - return GsConverter.asSerial(v as Date) - } - if (v in LocalTime) { - return GsConverter.asSerial(v as LocalTime) - } + + ValueRange vr = new ValueRange() + .setRange(range) + .setMajorDimension(ROWS_DIMENSION) + .setValues(values) + + try { + sheets.spreadsheets().values() + .update(spreadsheetId, range, vr) + .setValueInputOption(RAW_INPUT) + .execute() + } catch (IOException e) { + throw new SheetOperationException('update data', spreadsheetId, e) } - return String.valueOf(v) + + spreadsheetId } } diff --git a/matrix-gsheets/src/test/groovy/test/alipsa/matrix/gsheets/BqAuthUtilsTest.groovy b/matrix-gsheets/src/test/groovy/test/alipsa/matrix/gsheets/GsAuthUtilsTest.groovy similarity index 56% rename from matrix-gsheets/src/test/groovy/test/alipsa/matrix/gsheets/BqAuthUtilsTest.groovy rename to matrix-gsheets/src/test/groovy/test/alipsa/matrix/gsheets/GsAuthUtilsTest.groovy index 3ce53d65..f40eab9d 100644 --- a/matrix-gsheets/src/test/groovy/test/alipsa/matrix/gsheets/BqAuthUtilsTest.groovy +++ b/matrix-gsheets/src/test/groovy/test/alipsa/matrix/gsheets/GsAuthUtilsTest.groovy @@ -1,43 +1,43 @@ package test.alipsa.matrix.gsheets import static org.junit.jupiter.api.Assertions.* -import static se.alipsa.matrix.gsheets.BqAuthenticator.* +import static se.alipsa.matrix.gsheets.GsAuthenticator.* import org.junit.jupiter.api.Test -import se.alipsa.matrix.gsheets.BqAuthUtils +import se.alipsa.matrix.gsheets.GsAuthUtils /** - * Unit tests for BqAuthUtils focusing on utility methods that don't require + * Unit tests for GsAuthUtils focusing on utility methods that don't require * external dependencies (OAuth2 token validation methods are excluded as they * require network access). */ -class BqAuthUtilsTest { +class GsAuthUtilsTest { @Test void testCanonScopeWithNull() { - assertNull(BqAuthUtils.canonScope(null)) + assertNull(GsAuthUtils.canonScope(null)) } @Test void testCanonScopeWithEmail() { // 'email' should be canonicalized to the full userinfo.email scope - assertEquals(SCOPE_USERINFO_EMAIL, BqAuthUtils.canonScope('email')) - assertEquals(SCOPE_USERINFO_EMAIL, BqAuthUtils.canonScope(SCOPE_USERINFO_EMAIL)) + assertEquals(SCOPE_USERINFO_EMAIL, GsAuthUtils.canonScope('email')) + assertEquals(SCOPE_USERINFO_EMAIL, GsAuthUtils.canonScope(SCOPE_USERINFO_EMAIL)) } @Test void testCanonScopeWithOtherScopes() { // Other scopes should pass through unchanged - assertEquals(SCOPE_SHEETS, BqAuthUtils.canonScope(SCOPE_SHEETS)) - assertEquals(SCOPE_CLOUD_PLATFORM, BqAuthUtils.canonScope(SCOPE_CLOUD_PLATFORM)) - assertEquals(SCOPE_DRIVE_FILE, BqAuthUtils.canonScope(SCOPE_DRIVE_FILE)) - assertEquals('custom-scope', BqAuthUtils.canonScope('custom-scope')) + assertEquals(SCOPE_SHEETS, GsAuthUtils.canonScope(SCOPE_SHEETS)) + assertEquals(SCOPE_CLOUD_PLATFORM, GsAuthUtils.canonScope(SCOPE_CLOUD_PLATFORM)) + assertEquals(SCOPE_DRIVE_FILE, GsAuthUtils.canonScope(SCOPE_DRIVE_FILE)) + assertEquals('custom-scope', GsAuthUtils.canonScope('custom-scope')) } @Test void testHasAllScopesWithNullCredentials() { - assertFalse(BqAuthUtils.hasAllScopes(null, [SCOPE_SHEETS])) + assertFalse(GsAuthUtils.hasAllScopes(null, [SCOPE_SHEETS])) } // Note: Testing hasAllScopes with real credentials requires network access diff --git a/matrix-gsheets/src/test/groovy/test/alipsa/matrix/gsheets/BqAuthenticatorTest.groovy b/matrix-gsheets/src/test/groovy/test/alipsa/matrix/gsheets/GsAuthenticatorTest.groovy similarity index 84% rename from matrix-gsheets/src/test/groovy/test/alipsa/matrix/gsheets/BqAuthenticatorTest.groovy rename to matrix-gsheets/src/test/groovy/test/alipsa/matrix/gsheets/GsAuthenticatorTest.groovy index dc0aa2f9..8f3b2b72 100644 --- a/matrix-gsheets/src/test/groovy/test/alipsa/matrix/gsheets/BqAuthenticatorTest.groovy +++ b/matrix-gsheets/src/test/groovy/test/alipsa/matrix/gsheets/GsAuthenticatorTest.groovy @@ -1,24 +1,24 @@ package test.alipsa.matrix.gsheets import static org.junit.jupiter.api.Assertions.* -import static se.alipsa.matrix.gsheets.BqAuthenticator.* +import static se.alipsa.matrix.gsheets.GsAuthenticator.* import org.junit.jupiter.api.Test -import se.alipsa.matrix.gsheets.BqAuthenticator +import se.alipsa.matrix.gsheets.GsAuthenticator /** - * Unit tests for BqAuthenticator focusing on scope normalization and utility methods. + * Unit tests for GsAuthenticator focusing on scope normalization and utility methods. * * Note: Full authentication flow testing requires external dependencies (gcloud, OAuth2, file system) * and should be done as integration tests tagged with @Tag('external'). */ -class BqAuthenticatorTest { +class GsAuthenticatorTest { @Test void testNormalizeScopesForGcloudAddsCloudPlatform() { def scopes = [SCOPE_SHEETS] - def normalized = BqAuthenticator.normalizeScopesForGcloud(scopes) + def normalized = GsAuthenticator.normalizeScopesForGcloud(scopes) assertTrue(normalized.contains(SCOPE_CLOUD_PLATFORM), 'Normalized scopes should always include cloud-platform') @@ -28,7 +28,7 @@ class BqAuthenticatorTest { @Test void testNormalizeScopesForGcloudUpgradesReadonly() { def scopes = [SCOPE_SHEETS_READONLY] - def normalized = BqAuthenticator.normalizeScopesForGcloud(scopes) + def normalized = GsAuthenticator.normalizeScopesForGcloud(scopes) assertTrue(normalized.contains(SCOPE_SHEETS), 'Should upgrade readonly to full sheets scope') @@ -39,7 +39,7 @@ class BqAuthenticatorTest { @Test void testNormalizeScopesForGcloudRemovesShortEmail() { def scopes = [SCOPE_SHEETS, 'email'] - def normalized = BqAuthenticator.normalizeScopesForGcloud(scopes) + def normalized = GsAuthenticator.normalizeScopesForGcloud(scopes) assertFalse(normalized.contains('email'), "Should remove short 'email' scope in favor of userinfo.email") @@ -47,7 +47,7 @@ class BqAuthenticatorTest { @Test void testNormalizeScopesForGcloudHandlesNull() { - def normalized = BqAuthenticator.normalizeScopesForGcloud(null) + def normalized = GsAuthenticator.normalizeScopesForGcloud(null) assertNotNull(normalized) assertTrue(normalized.contains(SCOPE_CLOUD_PLATFORM)) @@ -55,7 +55,7 @@ class BqAuthenticatorTest { @Test void testNormalizeScopesForGcloudHandlesEmpty() { - def normalized = BqAuthenticator.normalizeScopesForGcloud([]) + def normalized = GsAuthenticator.normalizeScopesForGcloud([]) assertNotNull(normalized) assertTrue(normalized.contains(SCOPE_CLOUD_PLATFORM)) @@ -64,7 +64,7 @@ class BqAuthenticatorTest { @Test void testNormalizeScopesForGcloudPreservesOrder() { def scopes = [SCOPE_DRIVE_FILE, SCOPE_OPENID, SCOPE_USERINFO_EMAIL] - def normalized = BqAuthenticator.normalizeScopesForGcloud(scopes) + def normalized = GsAuthenticator.normalizeScopesForGcloud(scopes) // Should preserve input scopes and add cloud-platform assertTrue(normalized.containsAll(scopes)) @@ -74,7 +74,7 @@ class BqAuthenticatorTest { @Test void testNormalizeScopesForGcloudRemovesDuplicates() { def scopes = [SCOPE_SHEETS, SCOPE_SHEETS, SCOPE_DRIVE_FILE, SCOPE_DRIVE_FILE] - def normalized = BqAuthenticator.normalizeScopesForGcloud(scopes) + def normalized = GsAuthenticator.normalizeScopesForGcloud(scopes) // Should have no duplicates (LinkedHashSet removes them) assertEquals(3, normalized.size(), diff --git a/matrix-gsheets/src/test/groovy/test/alipsa/matrix/gsheets/GsConverterTest.groovy b/matrix-gsheets/src/test/groovy/test/alipsa/matrix/gsheets/GsConverterTest.groovy index f5d06f82..53c5f73b 100644 --- a/matrix-gsheets/src/test/groovy/test/alipsa/matrix/gsheets/GsConverterTest.groovy +++ b/matrix-gsheets/src/test/groovy/test/alipsa/matrix/gsheets/GsConverterTest.groovy @@ -237,4 +237,44 @@ class GsConverterTest { assertEquals([], GsConverter.toSerials(null)) } + @Test + void testToSerialsWithLocalDates() { + def dates = [LocalDate.parse('2024-06-24'), LocalDate.parse('2025-06-01')] + def serials = GsConverter.toSerials(dates) + assertEquals(2, serials.size()) + assertEquals(new BigDecimal(45467), serials[0]) + assertEquals(new BigDecimal(45809), serials[1]) + } + + @Test + void testToSerialsWithLocalDateTimes() { + def dateTimes = [LocalDateTime.parse('2022-01-14T12:33:20')] + def serials = GsConverter.toSerials(dateTimes) + assertEquals(1, serials.size()) + assertEquals(44575.5231481481, serials[0]) + } + + @Test + void testToSerialsWithLocalTimes() { + def times = [LocalTime.parse('18:25:44')] + def serials = GsConverter.toSerials(times) + assertEquals(1, serials.size()) + assertEquals(0.7678703704, serials[0]) + } + + @Test + void testToSerialsWithMixedDateTypes() { + def mixed = [LocalDate.parse('2024-06-24'), LocalDateTime.parse('2022-01-14T12:33:20')] + def serials = GsConverter.toSerials(mixed) + assertEquals(2, serials.size()) + assertEquals(new BigDecimal(45467), serials[0]) + assertEquals(44575.5231481481, serials[1]) + } + + @Test + void testToSerialsWithEmptyList() { + def serials = GsConverter.toSerials([]) + assertEquals([], serials) + } + } diff --git a/matrix-gsheets/src/test/groovy/test/alipsa/matrix/gsheets/GsExporterTest.groovy b/matrix-gsheets/src/test/groovy/test/alipsa/matrix/gsheets/GsExporterTest.groovy index d195de10..c5c9961c 100644 --- a/matrix-gsheets/src/test/groovy/test/alipsa/matrix/gsheets/GsExporterTest.groovy +++ b/matrix-gsheets/src/test/groovy/test/alipsa/matrix/gsheets/GsExporterTest.groovy @@ -5,136 +5,13 @@ import static org.junit.jupiter.api.Assertions.* import org.junit.jupiter.api.Test import se.alipsa.matrix.core.Matrix -import se.alipsa.matrix.gsheets.GsConverter import se.alipsa.matrix.gsheets.GsExporter -import se.alipsa.matrix.gsheets.GsUtil - -import java.time.LocalDate -import java.time.LocalDateTime -import java.time.LocalTime /** - * Unit tests for GsExporter utility methods. + * Unit tests for GsExporter delegation to GsheetsWriter. */ class GsExporterTest { - @Test - void testSanitizeSheetNameRemovesInvalidCharacters() { - // Google Sheets doesn't allow: : \ / ? * [ ] - assertEquals('My Sheet Name', GsUtil.sanitizeSheetName('My:Sheet\\Name')) - assertEquals('Data Report', GsUtil.sanitizeSheetName('Data/Report')) - assertEquals('Query Results', GsUtil.sanitizeSheetName('Query?Results')) - assertEquals('Array Data', GsUtil.sanitizeSheetName('Array[*]Data')) // [*] becomes 2 spaces - } - - @Test - void testSanitizeSheetNameTruncatesLongNames() { - // Names longer than 100 characters should be truncated - String longName = 'A' * 150 - String result = GsUtil.sanitizeSheetName(longName) - assertEquals(100, result.length()) - } - - @Test - void testSanitizeSheetNameHandlesEmptyString() { - assertEquals('Sheet1', GsUtil.sanitizeSheetName('')) - assertEquals('Sheet1', GsUtil.sanitizeSheetName(' ')) - } - - @Test - void testSanitizeSheetNameHandlesOnlyInvalidCharacters() { - // If the result would be empty after removing invalid chars, use 'Sheet1' - assertEquals('Sheet1', GsUtil.sanitizeSheetName(':\\/?*[]')) - assertEquals('Sheet1', GsUtil.sanitizeSheetName('///')) - } - - @Test - void testSanitizeSheetNamePreservesValidCharacters() { - assertEquals('Employee Data 2024', GsUtil.sanitizeSheetName('Employee Data 2024')) - assertEquals('Sales_Q1', GsUtil.sanitizeSheetName('Sales_Q1')) - assertEquals('Report-Final', GsUtil.sanitizeSheetName('Report-Final')) - } - - @Test - void testToCellWithNull() { - // Test null handling with convertNullsToEmptyString - assertEquals('', GsUtil.toCell(null, true, false)) - assertNull(GsUtil.toCell(null, false, false)) - } - - @Test - void testToCellWithNumbers() { - assertEquals(42, GsUtil.toCell(42, true, false)) - assertEquals(3.14, GsUtil.toCell(3.14, true, false)) - assertEquals(123.45, GsUtil.toCell(123.45, true, false)) - } - - @Test - void testToCellWithBoolean() { - assertEquals(true, GsUtil.toCell(true, true, false)) - assertEquals(false, GsUtil.toCell(false, true, false)) - } - - @Test - void testToCellWithString() { - assertEquals('Hello, World!', GsUtil.toCell('Hello, World!', true, false)) - assertEquals('Test', GsUtil.toCell('Test', false, false)) - } - - @Test - void testToCellWithLocalDateAsString() { - def date = LocalDate.parse('2024-06-24') - // Without date conversion, should return ISO string - assertEquals('2024-06-24', GsUtil.toCell(date, true, false)) - } - - @Test - void testToCellWithLocalDateAsSerial() { - def date = LocalDate.parse('2024-06-24') - // With date conversion, should return serial number - def result = GsUtil.toCell(date, true, true) - assertEquals(GsConverter.asSerial(date), result) - } - - @Test - void testToCellWithLocalDateTimeAsString() { - def dateTime = LocalDateTime.parse('2022-01-14T12:33:20') - // Without date conversion, should return ISO string - assertEquals('2022-01-14T12:33:20', GsUtil.toCell(dateTime, true, false)) - } - - @Test - void testToCellWithLocalDateTimeAsSerial() { - def dateTime = LocalDateTime.parse('2022-01-14T12:33:20') - // With date conversion, should return serial number - def result = GsUtil.toCell(dateTime, true, true) - assertEquals(GsConverter.asSerial(dateTime), result) - } - - @Test - void testToCellWithLocalTimeAsString() { - def time = LocalTime.parse('18:25:44') - // Without date conversion, should return ISO string - assertEquals('18:25:44', GsUtil.toCell(time, true, false)) - } - - @Test - void testToCellWithLocalTimeAsSerial() { - def time = LocalTime.parse('18:25:44') - // With date conversion, should return serial number - def result = GsUtil.toCell(time, true, true) - assertEquals(GsConverter.asSerial(time), result) - } - - @Test - void testToCellWithDateAsSerial() { - def date = Date.from(LocalDateTime.parse('2022-01-14T12:33:20').atZone(java.time.ZoneId.systemDefault()).toInstant()) - // With date conversion, should return serial number - def result = GsUtil.toCell(date, true, true) - assertNotNull(result) - assertTrue(result instanceof BigDecimal) - } - @Test void testExportSheetWithNullMatrix() { assertThrows(IllegalArgumentException, () -> GsExporter.exportSheet(null)) diff --git a/matrix-gsheets/src/test/groovy/test/alipsa/matrix/gsheets/GsUtilTest.groovy b/matrix-gsheets/src/test/groovy/test/alipsa/matrix/gsheets/GsUtilTest.groovy index eb4e2518..6e50619e 100644 --- a/matrix-gsheets/src/test/groovy/test/alipsa/matrix/gsheets/GsUtilTest.groovy +++ b/matrix-gsheets/src/test/groovy/test/alipsa/matrix/gsheets/GsUtilTest.groovy @@ -8,9 +8,13 @@ import org.junit.jupiter.api.Tag import org.junit.jupiter.api.Test import se.alipsa.matrix.core.Matrix +import se.alipsa.matrix.gsheets.GsConverter import se.alipsa.matrix.gsheets.GsExporter +import se.alipsa.matrix.gsheets.GsUtil import java.time.LocalDate +import java.time.LocalDateTime +import java.time.LocalTime class GsUtilTest { @@ -171,6 +175,123 @@ class GsUtilTest { assertThrows(IllegalArgumentException, () -> getSheetNames(' ')) } + @Test + void testSanitizeSheetNameRemovesInvalidCharacters() { + // Google Sheets doesn't allow: : \ / ? * [ ] + assertEquals('My Sheet Name', GsUtil.sanitizeSheetName('My:Sheet\\Name')) + assertEquals('Data Report', GsUtil.sanitizeSheetName('Data/Report')) + assertEquals('Query Results', GsUtil.sanitizeSheetName('Query?Results')) + assertEquals('Array Data', GsUtil.sanitizeSheetName('Array[*]Data')) // [*] becomes 2 spaces + } + + @Test + void testSanitizeSheetNameTruncatesLongNames() { + // Names longer than 100 characters should be truncated + String longName = 'A' * 150 + String result = GsUtil.sanitizeSheetName(longName) + assertEquals(100, result.length()) + } + + @Test + void testSanitizeSheetNameHandlesEmptyString() { + assertEquals('Sheet1', GsUtil.sanitizeSheetName('')) + assertEquals('Sheet1', GsUtil.sanitizeSheetName(' ')) + } + + @Test + void testSanitizeSheetNameHandlesOnlyInvalidCharacters() { + // If the result would be empty after removing invalid chars, use 'Sheet1' + assertEquals('Sheet1', GsUtil.sanitizeSheetName(':\\/?*[]')) + assertEquals('Sheet1', GsUtil.sanitizeSheetName('///')) + } + + @Test + void testSanitizeSheetNamePreservesValidCharacters() { + assertEquals('Employee Data 2024', GsUtil.sanitizeSheetName('Employee Data 2024')) + assertEquals('Sales_Q1', GsUtil.sanitizeSheetName('Sales_Q1')) + assertEquals('Report-Final', GsUtil.sanitizeSheetName('Report-Final')) + } + + @Test + void testToCellWithNull() { + // Test null handling with convertNullsToEmptyString + assertEquals('', GsUtil.toCell(null, true, false)) + assertNull(GsUtil.toCell(null, false, false)) + } + + @Test + void testToCellWithNumbers() { + assertEquals(42, GsUtil.toCell(42, true, false)) + assertEquals(3.14, GsUtil.toCell(3.14, true, false)) + assertEquals(123.45, GsUtil.toCell(123.45, true, false)) + } + + @Test + void testToCellWithBoolean() { + assertEquals(true, GsUtil.toCell(true, true, false)) + assertEquals(false, GsUtil.toCell(false, true, false)) + } + + @Test + void testToCellWithString() { + assertEquals('Hello, World!', GsUtil.toCell('Hello, World!', true, false)) + assertEquals('Test', GsUtil.toCell('Test', false, false)) + } + + @Test + void testToCellWithLocalDateAsString() { + def date = LocalDate.parse('2024-06-24') + // Without date conversion, should return ISO string + assertEquals('2024-06-24', GsUtil.toCell(date, true, false)) + } + + @Test + void testToCellWithLocalDateAsSerial() { + def date = LocalDate.parse('2024-06-24') + // With date conversion, should return serial number + def result = GsUtil.toCell(date, true, true) + assertEquals(GsConverter.asSerial(date), result) + } + + @Test + void testToCellWithLocalDateTimeAsString() { + def dateTime = LocalDateTime.parse('2022-01-14T12:33:20') + // Without date conversion, should return ISO string + assertEquals('2022-01-14T12:33:20', GsUtil.toCell(dateTime, true, false)) + } + + @Test + void testToCellWithLocalDateTimeAsSerial() { + def dateTime = LocalDateTime.parse('2022-01-14T12:33:20') + // With date conversion, should return serial number + def result = GsUtil.toCell(dateTime, true, true) + assertEquals(GsConverter.asSerial(dateTime), result) + } + + @Test + void testToCellWithLocalTimeAsString() { + def time = LocalTime.parse('18:25:44') + // Without date conversion, should return ISO string + assertEquals('18:25:44', GsUtil.toCell(time, true, false)) + } + + @Test + void testToCellWithLocalTimeAsSerial() { + def time = LocalTime.parse('18:25:44') + // With date conversion, should return serial number + def result = GsUtil.toCell(time, true, true) + assertEquals(GsConverter.asSerial(time), result) + } + + @Test + void testToCellWithDateAsSerial() { + def date = Date.from(LocalDateTime.parse('2022-01-14T12:33:20').atZone(java.time.ZoneId.systemDefault()).toInstant()) + // With date conversion, should return serial number + def result = GsUtil.toCell(date, true, true) + assertNotNull(result) + assertTrue(result instanceof BigDecimal) + } + @Test void testAsColumnNumberValidInputs() { // Test valid column names diff --git a/matrix-gsheets/src/test/groovy/test/alipsa/matrix/gsheets/GsheetsReaderTest.groovy b/matrix-gsheets/src/test/groovy/test/alipsa/matrix/gsheets/GsheetsReaderTest.groovy index db1a451b..6057d962 100644 --- a/matrix-gsheets/src/test/groovy/test/alipsa/matrix/gsheets/GsheetsReaderTest.groovy +++ b/matrix-gsheets/src/test/groovy/test/alipsa/matrix/gsheets/GsheetsReaderTest.groovy @@ -2,17 +2,15 @@ package test.alipsa.matrix.gsheets import static org.junit.jupiter.api.Assertions.* -import com.google.auth.oauth2.GoogleCredentials import org.junit.jupiter.api.Test -import se.alipsa.matrix.core.Matrix import se.alipsa.matrix.gsheets.GsheetsReader /** * Tests for GsheetsReader class. * - * These tests verify that GsheetsReader properly delegates to GsImporter - * and maintains backward compatibility. + * These tests verify input validation and method existence. + * Actual read operations are tested via external/integration tests. */ class GsheetsReaderTest { @@ -66,31 +64,4 @@ class GsheetsReaderTest { }, 'Should throw on null range') } - @Test - void testMethodsDelegateToGsImporter() { - // These tests verify that methods exist and have correct signatures - // Actual functionality is tested in GsImporterTest since we delegate to it - - // Verify read method exists with correct parameter types - def readMethod = GsheetsReader.getDeclaredMethod( - 'read', String, String, boolean, GoogleCredentials - ) - assertNotNull(readMethod) - assertTrue(readMethod.returnType == Matrix) - - // Verify readAsObject method exists with correct parameter types - def readAsObjectMethod = GsheetsReader.getDeclaredMethod( - 'readAsObject', String, String, boolean, GoogleCredentials, boolean - ) - assertNotNull(readAsObjectMethod) - assertTrue(readAsObjectMethod.returnType == Matrix) - - // Verify readAsStrings method exists with correct parameter types - def readAsStringsMethod = GsheetsReader.getDeclaredMethod( - 'readAsStrings', String, String, boolean, GoogleCredentials - ) - assertNotNull(readAsStringsMethod) - assertTrue(readAsStringsMethod.returnType == Matrix) - } - } diff --git a/matrix-gsheets/src/test/groovy/test/alipsa/matrix/gsheets/GsheetsWriterTest.groovy b/matrix-gsheets/src/test/groovy/test/alipsa/matrix/gsheets/GsheetsWriterTest.groovy index f28ec225..004cbb4f 100644 --- a/matrix-gsheets/src/test/groovy/test/alipsa/matrix/gsheets/GsheetsWriterTest.groovy +++ b/matrix-gsheets/src/test/groovy/test/alipsa/matrix/gsheets/GsheetsWriterTest.groovy @@ -2,7 +2,6 @@ package test.alipsa.matrix.gsheets import static org.junit.jupiter.api.Assertions.* -import com.google.auth.oauth2.GoogleCredentials import org.junit.jupiter.api.Test import se.alipsa.matrix.core.Matrix @@ -11,8 +10,8 @@ import se.alipsa.matrix.gsheets.GsheetsWriter /** * Tests for GsheetsWriter class. * - * These tests verify that GsheetsWriter properly delegates to GsExporter - * and maintains backward compatibility. + * These tests verify input validation and method existence. + * Actual write operations are tested via external/integration tests. */ class GsheetsWriterTest { @@ -42,17 +41,4 @@ class GsheetsWriterTest { }, 'Should throw on matrix with no rows') } - @Test - void testMethodDelegatesToGsExporter() { - // This test verifies that the write method exists and has correct signature - // Actual functionality is tested in GsExporterTest since we delegate to it - - // Verify write method exists with correct parameter types - def writeMethod = GsheetsWriter.getDeclaredMethod( - 'write', Matrix, GoogleCredentials, boolean, boolean - ) - assertNotNull(writeMethod) - assertTrue(writeMethod.returnType == String) - } - } From 176a662d6831b653afe2069753d6b87206ecac79 Mon Sep 17 00:00:00 2001 From: per Date: Sun, 3 May 2026 16:42:25 +0200 Subject: [PATCH 2/2] =?UTF-8?q?1=20=20=20write()=20and=20update()=20duplic?= =?UTF-8?q?ate=20service=20setup=20=20=20=20=20=20=20Extracted=20buildShee?= =?UTF-8?q?tsService(GoogleCredentials)=20private=20helper=20in=20GsheetsW?= =?UTF-8?q?riter,=20identical=20pattern=20to=20GsheetsReader.=20=20=20=20?= =?UTF-8?q?=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20?= =?UTF-8?q?=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20?= =?UTF-8?q?=20=20=20=20=20=20=20=20=20=20=20=20=20Removed=20now-unused=20H?= =?UTF-8?q?ttpRequestInitializer=20import.=20=20=20=202=20=20=20Fully-qual?= =?UTF-8?q?ified=20@groovy.transform.CompileDynamic=20=20=20Added=20import?= =?UTF-8?q?=20groovy.transform.CompileDynamic=20to=20GsConverter=20and=20c?= =?UTF-8?q?hanged=20annotation=20to=20@CompileDynamic.=20=20=20=203=20=20?= =?UTF-8?q?=20update()=20has=20no=20tests=20=20=20=20=20=20=20=20=20=20=20?= =?UTF-8?q?=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20Added?= =?UTF-8?q?=205=20validation=20tests=20to=20GsheetsWriterTest:=20null=20sp?= =?UTF-8?q?readsheetId,=20null=20range,=20null=20matrix,=20empty=20matrix,?= =?UTF-8?q?=20no-row=20mat=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20?= =?UTF-8?q?=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20?= =?UTF-8?q?=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20?= =?UTF-8?q?=20rix.=20=20=20=204=20=20=20release.md=20rename=20entry=20unde?= =?UTF-8?q?r=20wrong=20version=20=20=20=20=20=20=20=20Already=20correctly?= =?UTF-8?q?=20placed=20under=200.2.0,=20In=20progress=20=E2=80=94=20no=20c?= =?UTF-8?q?hange=20needed.=20=20=20=205=20=20=20Breaking=20rename=20with?= =?UTF-8?q?=20no=20migration=20note=20=20=20=20=20=20=20=20=20=20=20=20=20?= =?UTF-8?q?Created=20docs/0.1-0.2-MIGRATION.md=20documenting=20the=20BqAut?= =?UTF-8?q?henticator=20=E2=86=92=20GsAuthenticator=20/=20BqAuthUtils=20?= =?UTF-8?q?=E2=86=92=20GsAuthUtils=20rena=20=20=20=20=20=20=20=20=20=20=20?= =?UTF-8?q?=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20?= =?UTF-8?q?=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20?= =?UTF-8?q?=20=20=20=20=20=20mes=20with=20before/after=20code=20examples,?= =?UTF-8?q?=20plus=20new=20update()=20and=20spreadsheetUrl()=20features.?= =?UTF-8?q?=20=20=20=206=20=20=20GsExporter=20Javadoc=20shows=20raw=20URL?= =?UTF-8?q?=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20Update?= =?UTF-8?q?d=20the=20deprecated=20class's=20example=20to=20use=20GsheetsWr?= =?UTF-8?q?iter.spreadsheetUrl(spreadsheetId).?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- matrix-gsheets/docs/0.1-0.2-MIGRATION.md | 64 +++++++++++++++++++ .../alipsa/matrix/gsheets/GsConverter.groovy | 4 +- .../alipsa/matrix/gsheets/GsExporter.groovy | 2 +- .../matrix/gsheets/GsheetsWriter.groovy | 36 ++++------- .../matrix/gsheets/GsheetsWriterTest.groovy | 40 ++++++++++++ 5 files changed, 120 insertions(+), 26 deletions(-) create mode 100644 matrix-gsheets/docs/0.1-0.2-MIGRATION.md diff --git a/matrix-gsheets/docs/0.1-0.2-MIGRATION.md b/matrix-gsheets/docs/0.1-0.2-MIGRATION.md new file mode 100644 index 00000000..20555d00 --- /dev/null +++ b/matrix-gsheets/docs/0.1-0.2-MIGRATION.md @@ -0,0 +1,64 @@ +# Migration Guide: matrix-gsheets 0.1 → 0.2 + +## Breaking Changes + +### Renamed Authentication Classes + +The `Bq*` prefix (short for BigQuery) was historically confusing because these classes are used exclusively for Google Sheets authentication. They have been renamed: + +| Old Name (0.1) | New Name (0.2) | +|----------------|----------------| +| `BqAuthenticator` | `GsAuthenticator` | +| `BqAuthUtils` | `GsAuthUtils` | + +#### Required Changes + +Update any direct references to the old class names: + +```groovy +// Before (0.1) +import se.alipsa.matrix.gsheets.BqAuthenticator +BqAuthenticator.authenticate(BqAuthenticator.SCOPE_SHEETS) + +// After (0.2) +import se.alipsa.matrix.gsheets.GsAuthenticator +GsAuthenticator.authenticate(GsAuthenticator.SCOPE_SHEETS) +``` + +```groovy +// Before (0.1) +import se.alipsa.matrix.gsheets.BqAuthUtils +BqAuthUtils.loginAndWriteAdc(...) + +// After (0.2) +import se.alipsa.matrix.gsheets.GsAuthUtils +GsAuthUtils.loginAndWriteAdc(...) +``` + +The static constants (`SCOPE_SHEETS`, `SCOPE_DRIVE_FILE`, `ADC_FILE_PATH`, etc.) have the same names and values — only the class name changed. + +## New Features + +### Write to Existing Spreadsheets + +`GsheetsWriter` now supports updating an existing spreadsheet: + +```groovy +GsheetsWriter.update( + '1BxiMVs0XRA5nFMdKvBdBZjgmUUqptlbs74OgvE2upms', + 'Sheet1!A1', + matrix, + null, // credentials + true, // convert nulls to empty strings + false // convert dates to serial +) +``` + +### Spreadsheet URL Helper + +A convenience method for generating the edit URL: + +```groovy +String url = GsheetsWriter.spreadsheetUrl(spreadsheetId) +// -> https://docs.google.com/spreadsheets/d/{spreadsheetId}/edit +``` diff --git a/matrix-gsheets/src/main/groovy/se/alipsa/matrix/gsheets/GsConverter.groovy b/matrix-gsheets/src/main/groovy/se/alipsa/matrix/gsheets/GsConverter.groovy index f3601bca..ff30d6b6 100644 --- a/matrix-gsheets/src/main/groovy/se/alipsa/matrix/gsheets/GsConverter.groovy +++ b/matrix-gsheets/src/main/groovy/se/alipsa/matrix/gsheets/GsConverter.groovy @@ -1,5 +1,7 @@ package se.alipsa.matrix.gsheets +import groovy.transform.CompileDynamic + import se.alipsa.matrix.core.util.Logger import java.sql.Timestamp @@ -118,7 +120,7 @@ class GsConverter { dateTimes } - @groovy.transform.CompileDynamic + @CompileDynamic static LocalTime asLocalTime(Object o) { if (o == null) { return null diff --git a/matrix-gsheets/src/main/groovy/se/alipsa/matrix/gsheets/GsExporter.groovy b/matrix-gsheets/src/main/groovy/se/alipsa/matrix/gsheets/GsExporter.groovy index efed33b3..bc2bbbe8 100644 --- a/matrix-gsheets/src/main/groovy/se/alipsa/matrix/gsheets/GsExporter.groovy +++ b/matrix-gsheets/src/main/groovy/se/alipsa/matrix/gsheets/GsExporter.groovy @@ -54,7 +54,7 @@ import java.time.LocalDateTime * * // Export to Google Sheets * String spreadsheetId = GsExporter.exportSheet(employees) - * println "View at: https://docs.google.com/spreadsheets/d/${spreadsheetId}/edit" + * println "View at: ${GsheetsWriter.spreadsheetUrl(spreadsheetId)}" * * // Export with date conversion * Matrix withDates = Matrix.builder('Sales Data') diff --git a/matrix-gsheets/src/main/groovy/se/alipsa/matrix/gsheets/GsheetsWriter.groovy b/matrix-gsheets/src/main/groovy/se/alipsa/matrix/gsheets/GsheetsWriter.groovy index c096224c..8698c7c5 100644 --- a/matrix-gsheets/src/main/groovy/se/alipsa/matrix/gsheets/GsheetsWriter.groovy +++ b/matrix-gsheets/src/main/groovy/se/alipsa/matrix/gsheets/GsheetsWriter.groovy @@ -4,7 +4,6 @@ import static se.alipsa.matrix.gsheets.GsAuthenticator.authenticate import static se.alipsa.matrix.gsheets.GsAuthenticator.getSCOPES import com.google.api.client.googleapis.javanet.GoogleNetHttpTransport -import com.google.api.client.http.HttpRequestInitializer import com.google.api.client.json.gson.GsonFactory import com.google.api.services.sheets.v4.Sheets import com.google.api.services.sheets.v4.model.Sheet @@ -142,18 +141,7 @@ class GsheetsWriter { throw new IllegalArgumentException(MATRIX_NO_ROWS_ERROR) } - def transport = GoogleNetHttpTransport.newTrustedTransport() - def gsonFactory = GsonFactory.getDefaultInstance() - - // Need write scope for creating/updating spreadsheets - if (credentials == null) { - credentials = authenticate(SCOPES) - } - HttpRequestInitializer cred = new HttpCredentialsAdapter(credentials) - - Sheets sheets = new Sheets.Builder(transport, gsonFactory, cred) - .setApplicationName(APP_NAME) - .build() + Sheets sheets = buildSheetsService(credentials) String titleBase = matrix.matrixName ?: "Matrix ${LocalDateTime.now().toString().replace('T', '_')}" String sheetName = GsUtil.sanitizeSheetName(titleBase) @@ -244,17 +232,7 @@ class GsheetsWriter { throw new IllegalArgumentException(MATRIX_NO_ROWS_ERROR) } - def transport = GoogleNetHttpTransport.newTrustedTransport() - def gsonFactory = GsonFactory.getDefaultInstance() - - if (credentials == null) { - credentials = authenticate(SCOPES) - } - HttpRequestInitializer cred = new HttpCredentialsAdapter(credentials) - - Sheets sheets = new Sheets.Builder(transport, gsonFactory, cred) - .setApplicationName(APP_NAME) - .build() + Sheets sheets = buildSheetsService(credentials) // Build data: header row + data rows List headers = (List) matrix.columnNames() @@ -281,4 +259,14 @@ class GsheetsWriter { spreadsheetId } + private static Sheets buildSheetsService(GoogleCredentials credentials) { + def creds = credentials ?: authenticate(SCOPES) + new Sheets.Builder( + GoogleNetHttpTransport.newTrustedTransport(), + GsonFactory.getDefaultInstance(), + new HttpCredentialsAdapter(creds)) + .setApplicationName(APP_NAME) + .build() + } + } diff --git a/matrix-gsheets/src/test/groovy/test/alipsa/matrix/gsheets/GsheetsWriterTest.groovy b/matrix-gsheets/src/test/groovy/test/alipsa/matrix/gsheets/GsheetsWriterTest.groovy index 004cbb4f..8f2357a5 100644 --- a/matrix-gsheets/src/test/groovy/test/alipsa/matrix/gsheets/GsheetsWriterTest.groovy +++ b/matrix-gsheets/src/test/groovy/test/alipsa/matrix/gsheets/GsheetsWriterTest.groovy @@ -41,4 +41,44 @@ class GsheetsWriterTest { }, 'Should throw on matrix with no rows') } + @Test + void testUpdateNullSpreadsheetIdThrows() { + assertThrows(IllegalArgumentException, () -> { + GsheetsWriter.update(null, 'Sheet1!A1', Matrix.builder().data(id: [1]).build()) + }, 'Should throw on null spreadsheetId') + } + + @Test + void testUpdateNullRangeThrows() { + assertThrows(IllegalArgumentException, () -> { + GsheetsWriter.update('some-id', null, Matrix.builder().data(id: [1]).build()) + }, 'Should throw on null range') + } + + @Test + void testUpdateNullMatrixThrows() { + assertThrows(IllegalArgumentException, () -> { + GsheetsWriter.update('some-id', 'Sheet1!A1', null) + }, 'Should throw on null matrix') + } + + @Test + void testUpdateEmptyMatrixThrows() { + Matrix emptyColumns = Matrix.builder().build() + assertThrows(IllegalArgumentException, () -> { + GsheetsWriter.update('some-id', 'Sheet1!A1', emptyColumns) + }, 'Should throw on matrix with no columns') + } + + @Test + void testUpdateMatrixWithNoRowsThrows() { + Matrix noRows = Matrix.builder() + .data(id: [], name: []) + .build() + + assertThrows(IllegalArgumentException, () -> { + GsheetsWriter.update('some-id', 'Sheet1!A1', noRows) + }, 'Should throw on matrix with no rows') + } + }