Deprecate Shared Instant and replace uses of it with Kotlin Instant#781
Conversation
There was a problem hiding this comment.
Pull request overview
This PR migrates the toolkit away from the custom org.readium.r2.shared.util.Instant wrapper by adopting the Kotlin standard library kotlin.time.Instant, adding a shared parsing helper (String.toInstant()), updating call sites, and deprecating the legacy wrapper API.
Changes:
- Added
String.toInstant()to parse ISO-8601-like date strings (with fallback parsing) into Kotlin instants. - Replaced usages of the legacy Shared
Instantacross Shared/OPDS/Streamer/LCP/Test app modules withkotlin.time.Instantand epoch-millis conversions where needed. - Deprecated the legacy
Instantwrapper and documented migration guidance in the changelog.
Reviewed changes
Copilot reviewed 26 out of 26 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| test-app/src/main/java/org/readium/r2/testapp/reader/PublicationMetadataDialogFragment.kt | Switches publication date handling to kotlin.time.Clock/System.now() and epoch-millis to java.util.Date. |
| test-app/src/main/java/org/readium/r2/testapp/drm/LcpManagementViewModel.kt | Replaces legacy toJavaDate() with Date(instant.toEpochMilliseconds()) conversions. |
| readium/streamer/src/test/java/org/readium/r2/streamer/parser/epub/MetadataTest.kt | Updates imports after Instant migration (currently introduces an unused import). |
| readium/streamer/src/main/java/org/readium/r2/streamer/parser/epub/MetadataAdapter.kt | Uses the new String.toInstant() when parsing EPUB metadata dates. |
| readium/shared/src/test/java/org/readium/r2/shared/util/InstantTest.kt | Suppresses deprecation warnings for tests that still cover the legacy wrapper. |
| readium/shared/src/test/java/org/readium/r2/shared/publication/MetadataTest.kt | Migrates test data from Instant.parse(...) to String.toInstant(). |
| readium/shared/src/test/java/org/readium/r2/shared/opds/AvailabilityTest.kt | Migrates test data from Instant.parse(...) to String.toInstant(). |
| readium/shared/src/main/java/org/readium/r2/shared/util/Instant.kt | Deprecates the legacy Instant wrapper and suggests replacements. |
| readium/shared/src/main/java/org/readium/r2/shared/publication/Metadata.kt | Switches modified/published to Kotlin Instant and parses via toInstant(). |
| readium/shared/src/main/java/org/readium/r2/shared/opds/OpdsMetadata.kt | Switches OPDS metadata modified to Kotlin Instant. |
| readium/shared/src/main/java/org/readium/r2/shared/opds/Availability.kt | Switches OPDS availability instants and parsing to Kotlin Instant/toInstant(). |
| readium/shared/src/main/java/org/readium/r2/shared/extensions/String.kt | Introduces String.toInstant() extension for permissive parsing. |
| readium/opds/src/test/java/org/readium/r2/opds/OPDS1ParserTest.kt | Updates OPDS1 parser tests to use String.toInstant() for dates. |
| readium/opds/src/main/java/org/readium/r2/opds/OPDS2Parser.kt | Updates OPDS2 parsing to use String.toInstant() for modified. |
| readium/opds/src/main/java/org/readium/r2/opds/OPDS1Parser.kt | Updates OPDS1 parsing to use String.toInstant() for modified/published. |
| readium/lcp/src/main/java/org/readium/r2/lcp/license/model/components/lsd/PotentialRights.kt | Switches LSD rights end date parsing to Kotlin Instant via toInstant(). |
| readium/lcp/src/main/java/org/readium/r2/lcp/license/model/components/lsd/Event.kt | Switches event timestamp parsing to Kotlin Instant via toInstant(). |
| readium/lcp/src/main/java/org/readium/r2/lcp/license/model/components/lcp/Rights.kt | Switches start/end parsing to Kotlin Instant via toInstant(). |
| readium/lcp/src/main/java/org/readium/r2/lcp/license/model/StatusDocument.kt | Switches updated timestamps parsing to Kotlin Instant via toInstant(). |
| readium/lcp/src/main/java/org/readium/r2/lcp/license/model/LicenseDocument.kt | Switches issued/updated parsing to Kotlin Instant via toInstant(). |
| readium/lcp/src/main/java/org/readium/r2/lcp/license/LicenseValidation.kt | Switches “now” to Clock.System.now() for license validation logic. |
| readium/lcp/src/main/java/org/readium/r2/lcp/license/License.kt | Migrates license API surface to Kotlin Instant. |
| readium/lcp/src/main/java/org/readium/r2/lcp/MaterialRenewListener.kt | Uses Clock.System.now() and epoch millis for the date picker integration. |
| readium/lcp/src/main/java/org/readium/r2/lcp/LcpLicense.kt | Migrates LCP license API to Kotlin Instant. |
| readium/lcp/src/main/java/org/readium/r2/lcp/LcpError.kt | Migrates error model references to Kotlin Instant. |
| CHANGELOG.md | Documents the deprecation and migration guidance for instants. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ?: tryOrNull { LocalDateTime.parse(this).toInstant(TimeZone.UTC) } | ||
| ?: tryOrNull { LocalDate.parse(this).atStartOfDayIn(TimeZone.UTC) } |
There was a problem hiding this comment.
LocalDateTime.toInstant and LocalDate.atStartOfDayIn both return kotlin.time.Instant.
| @@ -50,24 +62,58 @@ public class Instant private constructor( | |||
| return instant?.let { Instant(it) } | |||
There was a problem hiding this comment.
Same comment as String.kt.
|
Where possible, yes. Since Kotlin's
Yes, I just didn't want to put too much effort into the TestApp given it will go away eventually. kotlinx-datetime isn't an existing dependency in it. |
I understand that.
Okay, as you like. |
qnga
left a comment
There was a problem hiding this comment.
Take a look at my changes. If I haven't missed anything we can merge it.
|
Looks good to me. |
Since Kotlin 2.1.20, Instant has been available in the Kotlin standard library. This PR does the following:
String.toInstant(), to parse strings into Kotlin InstanttoInstantwhere necessary.