Skip to content

Split ZoneOffset into UtcOffset and FixedOffsetTimeZone #125

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 11 commits into from
Jul 2, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion core/common/src/Instant.kt
Original file line number Diff line number Diff line change
Expand Up @@ -483,4 +483,4 @@ internal const val DISTANT_FUTURE_SECONDS = 3093527980800
*
* Be careful: this function may throw for some values of the [Instant].
*/
internal expect fun Instant.toStringWithOffset(offset: ZoneOffset): String
internal expect fun Instant.toStringWithOffset(offset: UtcOffset): String
25 changes: 18 additions & 7 deletions core/common/src/TimeZone.kt
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,7 @@

package kotlinx.datetime

import kotlinx.datetime.serializers.TimeZoneSerializer
import kotlinx.datetime.serializers.ZoneOffsetSerializer
import kotlinx.datetime.serializers.*
import kotlinx.serialization.Serializable

@Serializable(with = TimeZoneSerializer::class)
Expand All @@ -32,7 +31,7 @@ public expect open class TimeZone {
/**
* Returns the time zone with the fixed UTC+0 offset.
*/
public val UTC: TimeZone
public val UTC: FixedOffsetTimeZone

/**
* Returns the time zone identified by the provided [zoneId].
Expand Down Expand Up @@ -85,18 +84,25 @@ public expect open class TimeZone {
public fun LocalDateTime.toInstant(): Instant
}

@Serializable(with = ZoneOffsetSerializer::class)
public expect class ZoneOffset : TimeZone {
@Serializable(with = FixedOffsetTimeZoneSerializer::class)
public expect class FixedOffsetTimeZone : TimeZone {
public constructor(offset: UtcOffset)
public val offset: UtcOffset

@Deprecated("Use offset.totalSeconds", ReplaceWith("offset.totalSeconds"))
public val totalSeconds: Int
}

@Deprecated("Use FixedOffsetTimeZone of UtcOffset instead", ReplaceWith("FixedOffsetTimeZone"))
public typealias ZoneOffset = FixedOffsetTimeZone

/**
* Finds the offset from UTC this time zone has at the specified [instant] of physical time.
*
* @see Instant.toLocalDateTime
* @see TimeZone.offsetAt
*/
public expect fun TimeZone.offsetAt(instant: Instant): ZoneOffset
public expect fun TimeZone.offsetAt(instant: Instant): UtcOffset

/**
* Return a civil date/time value that this instant has in the specified [timeZone].
Expand All @@ -110,13 +116,16 @@ public expect fun TimeZone.offsetAt(instant: Instant): ZoneOffset
*/
public expect fun Instant.toLocalDateTime(timeZone: TimeZone): LocalDateTime

internal expect fun Instant.toLocalDateTime(offset: UtcOffset): LocalDateTime


/**
* Finds the offset from UTC the specified [timeZone] has at this instant of physical time.
*
* @see Instant.toLocalDateTime
* @see TimeZone.offsetAt
*/
public fun Instant.offsetIn(timeZone: TimeZone): ZoneOffset =
public fun Instant.offsetIn(timeZone: TimeZone): UtcOffset =
timeZone.offsetAt(this)

/**
Expand All @@ -135,6 +144,8 @@ public fun Instant.offsetIn(timeZone: TimeZone): ZoneOffset =
*/
public expect fun LocalDateTime.toInstant(timeZone: TimeZone): Instant

public expect fun LocalDateTime.toInstant(offset: UtcOffset): Instant

/**
* Returns an instant that corresponds to the start of this date in the specified [timeZone].
*
Expand Down
25 changes: 25 additions & 0 deletions core/common/src/UtcOffset.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
/*
* Copyright 2019-2021 JetBrains s.r.o.
* Use of this source code is governed by the Apache 2.0 License that can be found in the LICENSE.txt file.
*/

package kotlinx.datetime

import kotlinx.datetime.serializers.UtcOffsetSerializer
import kotlinx.serialization.Serializable

@Serializable(with = UtcOffsetSerializer::class)
public expect class UtcOffset {
public val totalSeconds: Int

public companion object {
public val ZERO: UtcOffset
public fun parse(offsetString: String): UtcOffset
}
}
public expect fun UtcOffset(hours: Int? = null, minutes: Int? = null, seconds: Int? = null): UtcOffset
Copy link
Collaborator

Choose a reason for hiding this comment

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

This needs to be documented. The other new functions as well, but the rest of them are much more self-explanatory.


@Deprecated("Use UtcOffset.ZERO instead", ReplaceWith("UtcOffset.ZERO"), DeprecationLevel.ERROR)
public fun UtcOffset(): UtcOffset = UtcOffset.ZERO

public fun UtcOffset.asTimeZone(): FixedOffsetTimeZone = FixedOffsetTimeZone(this)
27 changes: 21 additions & 6 deletions core/common/src/serializers/TimeZoneSerializers.kt
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,9 @@

package kotlinx.datetime.serializers

import kotlinx.datetime.FixedOffsetTimeZone
import kotlinx.datetime.TimeZone
import kotlinx.datetime.ZoneOffset
import kotlinx.datetime.UtcOffset
import kotlinx.serialization.*
import kotlinx.serialization.descriptors.*
import kotlinx.serialization.encoding.*
Expand All @@ -23,21 +24,35 @@ public object TimeZoneSerializer: KSerializer<TimeZone> {

}

public object ZoneOffsetSerializer: KSerializer<ZoneOffset> {
public object FixedOffsetTimeZoneSerializer: KSerializer<FixedOffsetTimeZone> {

override val descriptor: SerialDescriptor = PrimitiveSerialDescriptor("ZoneOffset", PrimitiveKind.STRING)
override val descriptor: SerialDescriptor = PrimitiveSerialDescriptor("FixedOffsetTimeZone", PrimitiveKind.STRING)

override fun deserialize(decoder: Decoder): ZoneOffset {
override fun deserialize(decoder: Decoder): FixedOffsetTimeZone {
val zone = TimeZone.of(decoder.decodeString())
if (zone is ZoneOffset) {
if (zone is FixedOffsetTimeZone) {
return zone
} else {
throw SerializationException("Timezone identifier '$zone' does not correspond to a fixed-offset timezone")
Copy link
Member Author

Choose a reason for hiding this comment

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

May be a problem if a time zone is recognized as fixed-offset in one platform, but not as fixed-offset in other.

}
}

override fun serialize(encoder: Encoder, value: ZoneOffset) {
override fun serialize(encoder: Encoder, value: FixedOffsetTimeZone) {
encoder.encodeString(value.id)
}

}

public object UtcOffsetSerializer: KSerializer<UtcOffset> {
Copy link
Member Author

Choose a reason for hiding this comment

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

We could also provide an alternative serializer that stores offset as an integer primitive or a single component object.


override val descriptor: SerialDescriptor = PrimitiveSerialDescriptor("UtcOffset", PrimitiveKind.STRING)

override fun deserialize(decoder: Decoder): UtcOffset {
return UtcOffset.parse(decoder.decodeString())
}

override fun serialize(encoder: Encoder, value: UtcOffset) {
encoder.encodeString(value.toString())
}

}
33 changes: 7 additions & 26 deletions core/common/test/InstantTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -112,13 +112,13 @@ class InstantTest {
Instant.fromEpochSeconds(0, 0))

val offsets = listOf(
TimeZone.of("Z") as ZoneOffset,
TimeZone.of("+03:12:14") as ZoneOffset,
TimeZone.of("-03:12:14") as ZoneOffset,
TimeZone.of("+02:35") as ZoneOffset,
TimeZone.of("-02:35") as ZoneOffset,
TimeZone.of("+04") as ZoneOffset,
TimeZone.of("-04") as ZoneOffset,
UtcOffset.parse("Z"),
UtcOffset.parse("+03:12:14"),
UtcOffset.parse("-03:12:14"),
UtcOffset.parse("+02:35"),
UtcOffset.parse("-02:35"),
UtcOffset.parse("+04"),
UtcOffset.parse("-04"),
)

for (instant in instants) {
Expand Down Expand Up @@ -611,22 +611,3 @@ class InstantRangeTest {
}
}


@Suppress("INVISIBLE_REFERENCE", "INVISIBLE_MEMBER")
@kotlin.internal.InlineOnly
inline fun <T> assertArithmeticFails(message: String? = null, f: () -> T) {
assertFailsWith<DateTimeArithmeticException>(message) {
val result = f()
fail(result.toString())
}
}

@Suppress("INVISIBLE_REFERENCE", "INVISIBLE_MEMBER")
@kotlin.internal.InlineOnly
inline fun <T> assertInvalidFormat(message: String? = null, f: () -> T) {
assertFailsWith<DateTimeFormatException>(message) {
val result = f()
fail(result.toString())
}
}

135 changes: 84 additions & 51 deletions core/common/test/TimeZoneTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,12 @@ class TimeZoneTest {

@Test
fun utc() {
println(TimeZone.UTC)
assertEquals("Z", TimeZone.UTC.id)
val utc: FixedOffsetTimeZone = TimeZone.UTC
println(utc)
assertEquals("Z", utc.id)
assertEquals(UtcOffset.ZERO, utc.offset)
assertEquals(0, utc.offset.totalSeconds)
assertEquals(utc.offset, utc.offsetAt(Clock.System.now()))
}

@Test
Expand Down Expand Up @@ -55,7 +59,13 @@ class TimeZoneTest {

assertFailsWith<IllegalTimeZoneException> { TimeZone.of("Mars/Standard") }
assertFailsWith<IllegalTimeZoneException> { TimeZone.of("UTC+X") }
}

@Test
fun ofFailsOnInvalidOffset() {
for (v in UtcOffsetTest.invalidUtcOffsetStrings) {
assertFailsWith<IllegalTimeZoneException> { TimeZone.of(v) }
}
}

// from 310bp
Expand Down Expand Up @@ -93,73 +103,96 @@ class TimeZoneTest {
}
}

@Test
fun utcOffsetNormalization() {
val sameOffsetTZs = listOf("+04", "+04:00", "UTC+4", "UT+04", "GMT+04:00:00").map { TimeZone.of(it) }
val instant = Instant.fromEpochSeconds(0)
val offsets = sameOffsetTZs.map { it.offsetAt(instant) }

assertTrue(offsets.distinct().size == 1, "Expected all offsets to be equal: $offsets")
assertTrue(offsets.map { it.toString() }.distinct().size == 1, "Expected all offsets to have the same string representation: $offsets")
}

// from 310bp
@Test
fun newYorkOffset() {
val test = TimeZone.of("America/New_York")
val offset = TimeZone.of("-5")
assertEquals(TimeZone.of("-5"), createInstant(offset, 2008, 1, 1).offsetIn(test))
assertEquals(TimeZone.of("-5"), createInstant(offset, 2008, 2, 1).offsetIn(test))
assertEquals(TimeZone.of("-5"), createInstant(offset, 2008, 3, 1).offsetIn(test))
assertEquals(TimeZone.of("-4"), createInstant(offset, 2008, 4, 1).offsetIn(test))
assertEquals(TimeZone.of("-4"), createInstant(offset, 2008, 5, 1).offsetIn(test))
assertEquals(TimeZone.of("-4"), createInstant(offset, 2008, 6, 1).offsetIn(test))
assertEquals(TimeZone.of("-4"), createInstant(offset, 2008, 7, 1).offsetIn(test))
assertEquals(TimeZone.of("-4"), createInstant(offset, 2008, 8, 1).offsetIn(test))
assertEquals(TimeZone.of("-4"), createInstant(offset, 2008, 9, 1).offsetIn(test))
assertEquals(TimeZone.of("-4"), createInstant(offset, 2008, 10, 1).offsetIn(test))
assertEquals(TimeZone.of("-4"), createInstant(offset, 2008, 11, 1).offsetIn(test))
assertEquals(TimeZone.of("-5"), createInstant(offset, 2008, 12, 1).offsetIn(test))
assertEquals(TimeZone.of("-5"), createInstant(offset, 2008, 1, 28).offsetIn(test))
assertEquals(TimeZone.of("-5"), createInstant(offset, 2008, 2, 28).offsetIn(test))
assertEquals(TimeZone.of("-4"), createInstant(offset, 2008, 3, 28).offsetIn(test))
assertEquals(TimeZone.of("-4"), createInstant(offset, 2008, 4, 28).offsetIn(test))
assertEquals(TimeZone.of("-4"), createInstant(offset, 2008, 5, 28).offsetIn(test))
assertEquals(TimeZone.of("-4"), createInstant(offset, 2008, 6, 28).offsetIn(test))
assertEquals(TimeZone.of("-4"), createInstant(offset, 2008, 7, 28).offsetIn(test))
assertEquals(TimeZone.of("-4"), createInstant(offset, 2008, 8, 28).offsetIn(test))
assertEquals(TimeZone.of("-4"), createInstant(offset, 2008, 9, 28).offsetIn(test))
assertEquals(TimeZone.of("-4"), createInstant(offset, 2008, 10, 28).offsetIn(test))
assertEquals(TimeZone.of("-5"), createInstant(offset, 2008, 11, 28).offsetIn(test))
assertEquals(TimeZone.of("-5"), createInstant(offset, 2008, 12, 28).offsetIn(test))
val offset = UtcOffset.parse("-5")

fun check(expectedOffset: String, dateTime: LocalDateTime) {
assertEquals(UtcOffset.parse(expectedOffset), dateTime.toInstant(offset).offsetIn(test))
}

check("-5", LocalDateTime(2008, 1, 1))
check("-5", LocalDateTime(2008, 2, 1))
check("-5", LocalDateTime(2008, 3, 1))
check("-4", LocalDateTime(2008, 4, 1))
check("-4", LocalDateTime(2008, 5, 1))
check("-4", LocalDateTime(2008, 6, 1))
check("-4", LocalDateTime(2008, 7, 1))
check("-4", LocalDateTime(2008, 8, 1))
check("-4", LocalDateTime(2008, 9, 1))
check("-4", LocalDateTime(2008, 10, 1))
check("-4", LocalDateTime(2008, 11, 1))
check("-5", LocalDateTime(2008, 12, 1))
check("-5", LocalDateTime(2008, 1, 28))
check("-5", LocalDateTime(2008, 2, 28))
check("-4", LocalDateTime(2008, 3, 28))
check("-4", LocalDateTime(2008, 4, 28))
check("-4", LocalDateTime(2008, 5, 28))
check("-4", LocalDateTime(2008, 6, 28))
check("-4", LocalDateTime(2008, 7, 28))
check("-4", LocalDateTime(2008, 8, 28))
check("-4", LocalDateTime(2008, 9, 28))
check("-4", LocalDateTime(2008, 10, 28))
check("-5", LocalDateTime(2008, 11, 28))
check("-5", LocalDateTime(2008, 12, 28))
}

// from 310bp
@Test
fun newYorkOffsetToDST() {
val test = TimeZone.of("America/New_York")
val offset = TimeZone.of("-5")
assertEquals(TimeZone.of("-5"), createInstant(offset, 2008, 3, 8).offsetIn(test))
assertEquals(TimeZone.of("-5"), createInstant(offset, 2008, 3, 9).offsetIn(test))
assertEquals(TimeZone.of("-4"), createInstant(offset, 2008, 3, 10).offsetIn(test))
assertEquals(TimeZone.of("-4"), createInstant(offset, 2008, 3, 11).offsetIn(test))
assertEquals(TimeZone.of("-4"), createInstant(offset, 2008, 3, 12).offsetIn(test))
assertEquals(TimeZone.of("-4"), createInstant(offset, 2008, 3, 13).offsetIn(test))
assertEquals(TimeZone.of("-4"), createInstant(offset, 2008, 3, 14).offsetIn(test))
val offset = UtcOffset.parse("-5")

fun check(expectedOffset: String, dateTime: LocalDateTime) {
assertEquals(UtcOffset.parse(expectedOffset), dateTime.toInstant(offset).offsetIn(test))
}

check("-5", LocalDateTime(2008, 3, 8))
check("-5", LocalDateTime(2008, 3, 9))
check("-4", LocalDateTime(2008, 3, 10))
check("-4", LocalDateTime(2008, 3, 11))
check("-4", LocalDateTime(2008, 3, 12))
check("-4", LocalDateTime(2008, 3, 13))
check("-4", LocalDateTime(2008, 3, 14))
// cutover at 02:00 local
assertEquals(TimeZone.of("-5"), createInstant(offset, 2008, 3, 9, 1, 59, 59, 999999999).offsetIn(test))
assertEquals(TimeZone.of("-4"), createInstant(offset, 2008, 3, 9, 2, 0, 0, 0).offsetIn(test))
check("-5", LocalDateTime(2008, 3, 9, 1, 59, 59, 999999999))
check("-4", LocalDateTime(2008, 3, 9, 2, 0, 0, 0))
}

// from 310bp
@Test
fun newYorkOffsetFromDST() {
val test = TimeZone.of("America/New_York")
val offset = TimeZone.of("-4")
assertEquals(TimeZone.of("-4"), createInstant(offset, 2008, 11, 1).offsetIn(test))
assertEquals(TimeZone.of("-4"), createInstant(offset, 2008, 11, 2).offsetIn(test))
assertEquals(TimeZone.of("-5"), createInstant(offset, 2008, 11, 3).offsetIn(test))
assertEquals(TimeZone.of("-5"), createInstant(offset, 2008, 11, 4).offsetIn(test))
assertEquals(TimeZone.of("-5"), createInstant(offset, 2008, 11, 5).offsetIn(test))
assertEquals(TimeZone.of("-5"), createInstant(offset, 2008, 11, 6).offsetIn(test))
assertEquals(TimeZone.of("-5"), createInstant(offset, 2008, 11, 7).offsetIn(test))
val offset = UtcOffset.parse("-4")

fun check(expectedOffset: String, dateTime: LocalDateTime) {
assertEquals(UtcOffset.parse(expectedOffset), dateTime.toInstant(offset).offsetIn(test))
}

check("-4", LocalDateTime(2008, 11, 1))
check("-4", LocalDateTime(2008, 11, 2))
check("-5", LocalDateTime(2008, 11, 3))
check("-5", LocalDateTime(2008, 11, 4))
check("-5", LocalDateTime(2008, 11, 5))
check("-5", LocalDateTime(2008, 11, 6))
check("-5", LocalDateTime(2008, 11, 7))
// cutover at 02:00 local
assertEquals(TimeZone.of("-4"), createInstant(offset, 2008, 11, 2, 1, 59, 59, 999999999).offsetIn(test))
assertEquals(TimeZone.of("-5"), createInstant(offset, 2008, 11, 2, 2, 0, 0, 0).offsetIn(test))
check("-4", LocalDateTime(2008, 11, 2, 1, 59, 59, 999999999))
check("-5", LocalDateTime(2008, 11, 2, 2, 0, 0, 0))
}

// from 310bp
private fun createInstant(offset: TimeZone, year: Int, month: Int, day: Int, hour: Int = 0, min: Int = 0,
sec: Int = 0, nano: Int = 0): Instant =
LocalDateTime(year, month, day, hour, min, sec, nano).toInstant(offset)
private fun LocalDateTime(year: Int, monthNumber: Int, dayOfMonth: Int) = LocalDateTime(year, monthNumber, dayOfMonth, 0, 0)

}
Loading