Skip to content

Commit 0974244

Browse files
xerialclaude
andcommitted
fix: ElapsedTime stays on string wire format (codex P2)
Codex flagged that the structured `{value, unit}` map encoding I switched to in the previous round was a wire-format change for `@Endpoint` methods returning `ElapsedTime` directly — the pre-PR toString-quoted body (`"2.50ms"`) became a JSON object. Revert to a JSON-string wire shape but use `${et.value}${unit}` rather than `et.toString` so the value is full-precision (`et.toString` is %.2f-formatted and rounds 2.555 → 2.56). To keep round-trips lossless for values that `Double.toString` emits in exponent form (e.g. `1.0E-9`), extend the `ElapsedTime.parse` regex to accept the exponent suffix. Drops the structured-map branch + the integer-literal handling that was specific to it. The simple stringBasedWeaver shape covers the new format. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent b1548f7 commit 0974244

3 files changed

Lines changed: 22 additions & 87 deletions

File tree

uni/src/main/scala/wvlet/uni/util/ElapsedTime.scala

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,11 @@ object ElapsedTime:
5656

5757
def units = List(NANOSECONDS, MICROSECONDS, MILLISECONDS, SECONDS, MINUTES, HOURS, DAYS)
5858

59-
private val strReprPattern = Pattern.compile("^\\s*(\\d+(?:\\.\\d+)?)\\s*([a-zA-Z]+)\\s*$")
59+
// Number pattern allows decimal, optional fractional part, and optional exponent so the
60+
// Weaver round-trip via Double.toString (which can emit "1.0E-9") parses back losslessly.
61+
private val strReprPattern = Pattern.compile(
62+
"^\\s*(\\d+(?:\\.\\d+)?(?:[eE][+-]?\\d+)?)\\s*([a-zA-Z]+)\\s*$"
63+
)
6064

6165
def nanosSince(start: Long): ElapsedTime = succinctNanos(System.nanoTime - start)
6266
def succinctNanos(nanos: Long): ElapsedTime = succinctDuration(nanos.toDouble, NANOSECONDS)

uni/src/main/scala/wvlet/uni/weaver/codec/PrimitiveWeaver.scala

Lines changed: 10 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -804,72 +804,16 @@ object PrimitiveWeaver:
804804

805805
given ulidWeaver: Weaver[ULID] = stringBasedWeaver("ULID", _.toString, ULID(_))
806806

807-
// ElapsedTime.toString is %.2f-formatted (lossy on the value), and a `${value}${unit}` string
808-
// form would only round-trip cleanly when Double.toString avoids exponential notation. Encode
809-
// as a structured `{value, unit}` map instead so the original (Double, TimeUnit) pair is
810-
// preserved exactly for any value the user could construct.
811-
given elapsedTimeWeaver: Weaver[ElapsedTime] =
812-
new Weaver[ElapsedTime]:
813-
override def pack(p: Packer, v: ElapsedTime, config: WeaverConfig): Unit =
814-
p.packMapHeader(2)
815-
p.packString("value")
816-
p.packDouble(v.value)
817-
p.packString("unit")
818-
p.packString(ElapsedTime.timeUnitToString(v.unit))
819-
820-
override def unpack(u: Unpacker, context: WeaverContext): Unit =
821-
u.getNextValueType match
822-
case ValueType.STRING =>
823-
// Backward-compatibility for legacy "<value><unit>" string form (e.g. "2.50ms").
824-
safeConvertFromString(
825-
context,
826-
u,
827-
ElapsedTime.parse(_),
828-
context.setObject,
829-
"ElapsedTime"
830-
)
831-
case ValueType.MAP =>
832-
try
833-
val mapSize = u.unpackMapHeader
834-
var value: Double = 0.0
835-
var hasValue = false
836-
var unit: java.util.concurrent.TimeUnit = null
837-
var i = 0
838-
while i < mapSize do
839-
u.unpackString match
840-
case "value" =>
841-
// Accept integer literals like `{"value":5,...}` from JSON producers that
842-
// omit the `.0` for whole-number durations.
843-
value =
844-
u.getNextValueType match
845-
case ValueType.INTEGER =>
846-
u.unpackLong.toDouble
847-
case _ =>
848-
u.unpackDouble
849-
hasValue = true
850-
case "unit" =>
851-
unit = ElapsedTime.valueOfTimeUnit(u.unpackString)
852-
case _ =>
853-
u.skipValue
854-
i += 1
855-
if !hasValue || unit == null then
856-
context.setError(
857-
IllegalArgumentException("ElapsedTime requires both 'value' and 'unit' fields")
858-
)
859-
else
860-
context.setObject(ElapsedTime(value, unit))
861-
catch
862-
case e: Exception =>
863-
context.setError(e)
864-
case ValueType.NIL =>
865-
safeUnpackNil(context, u)
866-
case other =>
867-
u.skipValue
868-
context.setError(
869-
IllegalArgumentException(
870-
s"Cannot convert ${other} to ElapsedTime, expected STRING or MAP"
871-
)
872-
)
807+
// Encode as `<value><unit>` (e.g. "2.555s", "1.0E-9ns") rather than `et.toString`, which is
808+
// %.2f-rounded and would lose precision on round-trip. ElapsedTime.parse accepts the value
809+
// in plain decimal *and* exponent form so any Double.toString output round-trips exactly,
810+
// and the wire shape stays a JSON string — matching the pre-PR Router fallback for this
811+
// type and avoiding a content-shape change for clients already on the wire.
812+
given elapsedTimeWeaver: Weaver[ElapsedTime] = stringBasedWeaver(
813+
"ElapsedTime",
814+
et => s"${et.value}${ElapsedTime.timeUnitToString(et.unit)}",
815+
ElapsedTime.parse(_)
816+
)
873817

874818
// Tuple support via recursive given resolution
875819
trait TupleElementWeaver[T <: Tuple]:

uni/src/test/scala/wvlet/uni/weaver/codec/AdditionalTypeWeaverTest.scala

Lines changed: 7 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -379,32 +379,19 @@ class AdditionalTypeWeaverTest extends UniTest:
379379
tinyBack.unit shouldBe tiny.unit
380380
}
381381

382-
test("ElapsedTime accepts legacy string form for backward compatibility") {
382+
test("ElapsedTime accepts legacy %.2f-formatted string form") {
383383
val legacy = "\"2.50ms\""
384384
val v = Weaver.fromJson[ElapsedTime](legacy)
385385
v.value shouldBe 2.5
386386
v.unit shouldBe TimeUnit.MILLISECONDS
387387
}
388388

389-
test("ElapsedTime rejects map missing value or unit") {
390-
val noValue = """{"unit":"ms"}"""
391-
val e1 = intercept[IllegalArgumentException] {
392-
Weaver.fromJson[ElapsedTime](noValue)
393-
}
394-
e1.getMessage shouldContain "value"
395-
396-
val noUnit = """{"value":5.0}"""
397-
val e2 = intercept[IllegalArgumentException] {
398-
Weaver.fromJson[ElapsedTime](noUnit)
399-
}
400-
e2.getMessage shouldContain "unit"
401-
}
402-
403-
test("ElapsedTime accepts integer value literal") {
404-
// JSON producers commonly emit whole numbers without a fractional part. Accept both.
405-
val v = Weaver.fromJson[ElapsedTime]("""{"value":5,"unit":"ms"}""")
406-
v.value shouldBe 5.0
407-
v.unit shouldBe TimeUnit.MILLISECONDS
389+
test("ElapsedTime parse accepts exponent form so Double.toString round-trips") {
390+
// 1.0E-9 is what `(1.0e-9).toString` produces — the encoder uses Double.toString so the
391+
// parse regex must accept this form to avoid losing very small values.
392+
val v = ElapsedTime.parse("1.0E-9ns")
393+
v.value shouldBe 1.0e-9
394+
v.unit shouldBe TimeUnit.NANOSECONDS
408395
}
409396

410397
// ====== Composite: case class with new types ======

0 commit comments

Comments
 (0)