Skip to content

feat: support Instant string conversion - #1089

Open
56wj wants to merge 1 commit into
apache:mainfrom
56wj:feat/instant-string-converter
Open

feat: support Instant string conversion#1089
56wj wants to merge 1 commit into
apache:mainfrom
56wj:feat/instant-string-converter

Conversation

@56wj

@56wj 56wj commented Sep 9, 2026

Copy link
Copy Markdown

Purpose of the pull request

Related: #1017

Add JDK 8-compatible support for lossless conversion between java.time.Instant and ISO-8601 Excel string cells.

What's changed?

  • Add InstantStringConverter using Instant.parse() for reads and Instant.toString() for canonical writes.
  • Register the converter for default writes, explicit string writes, and string reads.
  • Add unit coverage for converter keys, UTC and offset parsing, nanosecond round trips, invalid input, and loader registration.
  • Keep the implementation string-only because Excel date/number cells do not preserve timezone/offset semantics.

Verification

  • mvn clean package -B -Dmaven.test.skip=false -pl fesod-common,fesod-shaded,fesod-sheet
  • mvn -pl fesod-sheet -DskipTests spotless:check

Results after rebasing onto current main on JDK 21: 923 tests, 0 failures, 0 errors, 0 skipped. The generated Surefire reports include 6 passing InstantStringConverterTest cases and 5 passing DefaultConverterLoaderTest cases.

Checklist

  • I have read the Contributor Guide.
  • I have written the necessary doc or comment.
  • I have added the necessary unit tests and all cases have passed.

@56wj
56wj force-pushed the feat/instant-string-converter branch from 9077a24 to 87ce5b2 Compare September 9, 2026 12:25
@56wj

56wj commented Sep 9, 2026

Copy link
Copy Markdown
Author

CI on JDK 8/11 exposed a runtime compatibility difference: Instant.parse(...) rejected the +08:00 test input there, although newer JDKs accepted it. I updated the read path to OffsetDateTime.parse(...).toInstant(), which preserves the intended ISO-8601 offset normalization and remains JDK 8 compatible. The focused converter/loader tests and Spotless check pass locally on the updated commit 87ce5b2.

Comment on lines +20 to +24
/*
* This file is part of the Apache Fesod (Incubating) project, which was derived from Alibaba EasyExcel.
*
* Copyright (C) 2018-2024 Alibaba Group Holding Ltd.
*/

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The newly added files in this PR are implemented from scratch and are not derived from Alibaba's EasyExcel. Therefore, no EasyExcel-related license header is required for these files.

Please refer to: https://github.com/apache/fesod/blob/main/fesod-sheet/src/main/java/org/apache/fesod/sheet/FesodSheet.java

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated — I removed the EasyExcel-derived header from the new converter file and retained only the ASF license header, following FesodSheet.java. The focused converter/loader tests and Spotless check pass on the updated commit 3b7ae8f.

@56wj
56wj force-pushed the feat/instant-string-converter branch from 87ce5b2 to 3b7ae8f Compare September 10, 2026 14:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants