http/sse: shared optional-space strip, stdlib retry parsing, leaner ByteArrayBuilder.grow

[OmarAlJarrah]

Reading through the SSE parser I noticed a few small redundancies that have built up in ServerSentEventReader. None of them touch parsing behavior — the WHATWG SSE semantics, and the byte-identical output the test suite pins down via exact ServerSentEvent equality, stay exactly as they are. They just collapse duplicated logic and remove some dead code. Grouping them together since they all live in this one file.

sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/sse/ServerSentEventReader.kt:79-102 — share one optional-leading-space strip between comments and fields

The comment path and the field path each implement the "drop one optional U+0020 after the colon" rule from WHATWG SSE §9.2.6, but with two different code shapes. Fold them into a single helper.

Old code:

            if (line[0] == ':') {
                comment = line.substring(1).removePrefix(" ")
                hasField = true
                continue
            }

            val colon = line.indexOf(':')
            val field: String
            val rawValue: String
            if (colon < 0) {
                // No colon at all — entire line is the field name; value is empty.
                field = line
                rawValue = ""
            } else {
                field = line.substring(0, colon)
                // Per spec: if the value starts with U+0020 SPACE, drop one.
                val afterColon = colon + 1
                rawValue =
                    if (afterColon < line.length && line[afterColon] == ' ') {
                        line.substring(afterColon + 1)
                    } else {
                        line.substring(afterColon)
                    }
            }

New code:

            if (line[0] == ':') {
                comment = valueFrom(line, 1)
                hasField = true
                continue
            }

            val colon = line.indexOf(':')
            val field: String
            val rawValue: String
            if (colon < 0) {
                // No colon at all — entire line is the field name; value is empty.
                field = line
                rawValue = ""
            } else {
                field = line.substring(0, colon)
                rawValue = valueFrom(line, colon + 1)
            }

    // + new private method on the class (placed next to parseRetryMillis):
    /**
     * Returns the value portion of [line] starting at [start], dropping a single
     * leading U+0020 SPACE if present. WHATWG SSE §9.2.6 applies this optional
     * single-space strip to both comment text and field values, so both code
     * paths now go through this one implementation.
     */
    private fun valueFrom(line: String, start: Int): String =
        if (start < line.length && line[start] == ' ') {
            line.substring(start + 1)
        } else {
            line.substring(start)
        }

Usage — before → after:

val reader = ServerSentEventReader(source)
val event = reader.next()

val reader = ServerSentEventReader(source)
val event = reader.next()

call sites unchanged — behavior-preserving. valueFrom is a private method; nothing outside the class can observe the refactor. For the comment path, valueFrom(line, 1) reproduces substring(1).removePrefix(" ") for every input (only-colon, leading space, two spaces, no space), and for the field path it is the same index check moved behind a shared name.

Why: One implementation of the optional-leading-space strip instead of two, so the §9.2.6 rule lives in a single place. The comment path also drops the redundant intermediate allocation that removePrefix(" ") adds on top of the substring(1).

sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/sse/ServerSentEventReader.kt:202-213 — parse retry: with the stdlib behind a digit guard

The hand-rolled loop is a base-10 parse with manual overflow detection, which String.toLongOrNull() already does. Keep an explicit digit guard in front of it so spec semantics are preserved exactly.

Old code:

    private fun parseRetryMillis(value: String): Long? {
        if (value.isEmpty()) return null
        var result = 0L
        for (c in value) {
            if (c !in '0'..'9') return null
            val digit = (c - '0').toLong()
            // Detect overflow before it happens so we don't wrap around.
            if (result > (Long.MAX_VALUE - digit) / DECIMAL_BASE) return null
            result = result * DECIMAL_BASE + digit
        }
        return result
    }

New code:

    private fun parseRetryMillis(value: String): Long? {
        // Spec allows ASCII digits only; this guard rejects signs and any other
        // non-digit up front (toLongOrNull would otherwise accept a leading +/-).
        // toLongOrNull then parses base-10 and returns null on empty input or on
        // values past Long.MAX_VALUE — the same overflow boundary as before.
        if (value.any { it !in '0'..'9' }) return null
        return value.toLongOrNull()
    }

Usage — before → after:

// source yields: "retry: 2500\n\n"
val event = ServerSentEventReader(source).next()
// event?.retry == Duration.ofMillis(2500)

// source yields: "retry: 2500\n\n"
val event = ServerSentEventReader(source).next()
// event?.retry == Duration.ofMillis(2500)

call sites unchanged — behavior-preserving. Empty input still returns null ("".toLongOrNull() is null; the guard is vacuously false on an empty string); a value above Long.MAX_VALUE is still ignored, since toLongOrNull returns null at exactly that boundary.

Why: The standard-library parse already covers base-10 conversion and the overflow rejection the manual loop was written for, at the identical boundary. The leading check must stay a guard (if (value.any { it !in '0'..'9' }) return null), not a filter transform: filtering non-digits would silently parse "1a2" as 12, and toLongOrNull on its own would accept a leading +/- that the spec forbids — both wrong. The guard rejects any non-digit (including signs) before the parse runs.

Build: DECIMAL_BASE is referenced only by the manual multiply/overflow math being removed, so it becomes an unused private const and trips detekt. Delete it from the companion object:

        // Numeric base for `retry:` digit parsing — SSE retry values are unsigned decimals.
        private const val DECIMAL_BASE = 10L

sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/sse/ServerSentEventReader.kt:239-255 — drop the unused parameter and dead branch in ByteArrayBuilder.grow

grow is only ever called from append, and always with minCapacity == bytes.size + 1. Under that single contract the else -> oldCap arm can never be reached and the parameter carries no information.

Old code:

        fun append(b: Byte) {
            if (count == bytes.size) grow(count + 1)
            bytes[count++] = b
        }

        fun toUtf8(): String = if (count == 0) "" else String(bytes, 0, count, Charsets.UTF_8)

        private fun grow(minCapacity: Int) {
            val oldCap = bytes.size
            val newCap =
                when {
                    oldCap == 0 -> INITIAL_CAP
                    oldCap < minCapacity -> maxOf(oldCap * 2, minCapacity)
                    else -> oldCap
                }
            bytes = bytes.copyOf(newCap)
        }

New code:

        fun append(b: Byte) {
            if (count == bytes.size) grow()
            bytes[count++] = b
        }

        fun toUtf8(): String = if (count == 0) "" else String(bytes, 0, count, Charsets.UTF_8)

        private fun grow() {
            val newCap = if (bytes.isEmpty()) INITIAL_CAP else bytes.size * 2
            bytes = bytes.copyOf(newCap)
        }

Usage — before → after:

            if (count == bytes.size) grow(count + 1)

            if (count == bytes.size) grow()

ByteArrayBuilder is a private nested class and this is its only call site — there is no external usage. Behavior-preserving: grow() allocates INITIAL_CAP (64) on the first append and doubles bytes.size thereafter, identical to the old when under its only caller.

Why: Since the sole caller passes bytes.size + 1, oldCap < minCapacity is always true and maxOf(oldCap * 2, oldCap + 1) is always oldCap * 2, so the else arm is dead and the parameter is noise. Reducing it to "first allocation is INITIAL_CAP, otherwise double" keeps the same 0 → 64 → 128 → … growth sequence with less code.