Skip to content

http/common: streamline Headers.addAll, MediaType guards/toString/unescape, and ETag predicate #173

Description

@OmarAlJarrah

While reading through the http.common package I noticed a handful of small, behavior-preserving
simplifications. None of them change observable behavior or the public API surface — they drop a throwaway
allocation, collapse two duplicated string builds, untangle a double-negative guard, name an anonymous
15-line block, and remove an always-true term from a predicate. Grouping them in one ticket since they all
live in the same subsystem and are individually tiny.

sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/common/Headers.kt:278-283 — addAll should iterate the backing map directly

Old code:

        public fun addAll(headers: Headers): Builder =
            apply {
                headers.entries().forEach { (key, values) ->
                    headersMap.computeIfAbsent(key) { mutableListOf() }.addAll(values)
                }
            }

New code:

        public fun addAll(headers: Headers): Builder =
            apply {
                headers.headersMap.forEach { (key, values) ->
                    headersMap.computeIfAbsent(key) { mutableListOf() }.addAll(values)
                }
            }

Usage — before → after:

val merged = Headers.Builder()
    .add("Accept", "application/json")
    .addAll(other)
    .build()
val merged = Headers.Builder()
    .add("Accept", "application/json")
    .addAll(other)
    .build()

call sites unchanged — behavior-preserving.

Why: entries() exists to hand callers a defensively-copied snapshot: it allocates a LinkedHashMap,
wraps every value list in Collections.unmodifiableList, and wraps the whole thing in
Collections.unmodifiableMap — all of which is discarded immediately when we only want to iterate. Reading
headers.headersMap directly (exactly as the copy constructor at line 133 already does) skips those
allocations while preserving insertion order and multi-value semantics, since the built map is a
LinkedHashMap and .addAll(values) only reads from each list.

API / Build: No public-API change — addAll's signature is untouched, so apiCheck is unaffected. The
nested Builder already reads the outer class's private headersMap in its copy constructor, so this
introduces no new visibility coupling.

sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/common/MediaType.kt:87-93 — reuse the fullType getter in toString

Old code:

    override fun toString(): String {
        val formattedParams =
            parameters.entries.joinToString(separator = ";") { (key, value) ->
                "$key=${formatParameterValue(value)}"
            }
        return if (formattedParams.isEmpty()) "$type/$subtype" else "$type/$subtype;$formattedParams"
    }

New code:

    override fun toString(): String {
        val formattedParams =
            parameters.entries.joinToString(separator = ";") { (key, value) ->
                "$key=${formatParameterValue(value)}"
            }
        return if (formattedParams.isEmpty()) fullType else "$fullType;$formattedParams"
    }

Usage — before → after:

MediaType.parse("text/plain;charset=utf-8").toString() // "text/plain;charset=utf-8"
MediaType.parse("text/plain;charset=utf-8").toString() // "text/plain;charset=utf-8"

call sites unchanged — behavior-preserving.

Why: The fullType getter already computes "$type/$subtype"; reusing it removes the two inline rebuilds
of that string and keeps the bare-type wire form defined in exactly one place.

API / Build: No public-API or behavioral change; apiCheck is unaffected.

sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/common/MediaType.kt:145-147, 182-184 — drop the double-negative in the wildcard guard

Old code:

of() (lines 145-147):

            require(!(type == "*" && subtype != "*")) {
                "Invalid media type format: type=$type, subtype=$subtype"
            }

parse() (lines 182-184):

            require(!(type == "*" && subtype != "*")) {
                "Invalid media type format: $mediaType"
            }

New code:

of():

            require(type != "*" || subtype == "*") {
                "Invalid media type format: type=$type, subtype=$subtype"
            }

parse():

            require(type != "*" || subtype == "*") {
                "Invalid media type format: $mediaType"
            }

Usage — before → after:

MediaType.of("*", "*")    // ok
MediaType.of("*", "json") // throws IllegalArgumentException — wildcard type requires wildcard subtype
MediaType.of("*", "*")    // ok
MediaType.of("*", "json") // throws IllegalArgumentException — wildcard type requires wildcard subtype

call sites unchanged — behavior-preserving.

Why: By De Morgan's law !(type == "*" && subtype != "*") is type != "*" || subtype == "*", which reads
straight off as the documented invariant — "a wildcard type is only allowed when the subtype is also a
wildcard." Same truth table, no double negative to decode, and the error messages are unchanged.

API / Build: No public-API or behavioral change; apiCheck is unaffected.

sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/common/MediaType.kt:204-221 — extract the quoted-value unescape into a named function

Old code:

                        val value =
                            if (rawValue.startsWith("\"") && rawValue.endsWith("\"") && rawValue.length >= 2) {
                                val inner = rawValue.substring(1, rawValue.length - 1)
                                val sb = StringBuilder(inner.length)
                                var i = 0
                                while (i < inner.length) {
                                    if (inner[i] == '\\' && i + 1 < inner.length) {
                                        sb.append(inner[i + 1])
                                        i += 2
                                    } else {
                                        sb.append(inner[i])
                                        i++
                                    }
                                }
                                sb.toString()
                            } else {
                                rawValue
                            }

New code:

The inline block becomes a single call, and the unescape logic moves to a companion-level helper that mirrors
the existing formatParameterValue:

                        val value = unescapeQuotedValue(rawValue)
        /**
         * Strips surrounding double-quotes from a parameter [rawValue] and unescapes its
         * quoted-pair sequences (`\"` -> `"`, `\\` -> `\`), per RFC 7230 §3.2.6. A value that
         * is not a `quoted-string` is returned unchanged. Inverse of [formatParameterValue].
         */
        private fun unescapeQuotedValue(rawValue: String): String {
            if (!(rawValue.startsWith("\"") && rawValue.endsWith("\"") && rawValue.length >= 2)) {
                return rawValue
            }
            val inner = rawValue.substring(1, rawValue.length - 1)
            val sb = StringBuilder(inner.length)
            var i = 0
            while (i < inner.length) {
                if (inner[i] == '\\' && i + 1 < inner.length) {
                    sb.append(inner[i + 1])
                    i += 2
                } else {
                    sb.append(inner[i])
                    i++
                }
            }
            return sb.toString()
        }

Usage — before → after:

val value =
    if (rawValue.startsWith("\"") && rawValue.endsWith("\"") && rawValue.length >= 2) {
        // ... 15 lines of inline quoted-pair unescape ...
    } else {
        rawValue
    }
val value = unescapeQuotedValue(rawValue)

External MediaType.parse(...) results are unchanged — the extraction is internal to the parse loop.

Why: This 15-line block is the inverse of formatParameterValue but sits anonymously inside an
associate { } lambda, so the parameter-parsing loop is hard to skim. Lifting it to a named
private fun unescapeQuotedValue (next to its mirror image) names the operation and flattens the parse
body.

API / Build: The new symbol is private, so there is no public-API surface and apiCheck is unaffected.

sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/common/ETag.kt:92-94 — drop the redundant !startsWith("W/") term

Old code:

            val isStrongForm =
                trimmed.length >= 2 &&
                    !trimmed.startsWith("W/") && trimmed.startsWith("\"") && trimmed.endsWith("\"")

New code:

            val isStrongForm =
                trimmed.length >= 2 &&
                    trimmed.startsWith("\"") && trimmed.endsWith("\"")

Usage — before → after:

ETag.parse("\"abc\"")   // strong ETag
ETag.parse("W/\"abc\"") // weak ETag
ETag.parse("W/abc")     // throws IllegalArgumentException — malformed
ETag.parse("\"abc\"")   // strong ETag
ETag.parse("W/\"abc\"") // weak ETag
ETag.parse("W/abc")     // throws IllegalArgumentException — malformed

call sites unchanged — behavior-preserving.

Why: startsWith("\"") already requires the first character to be ", which a string starting with W/
can never be, so !startsWith("W/") is always true whenever the strong-form test reaches it. Dropping the
redundant term leaves the same result set and a clearer predicate.

API / Build: No public-API or behavioral change; apiCheck is unaffected.

Metadata

Metadata

Assignees

No one assigned

    Labels

    sdk-coresdk-core toolkittech-debtsimplification / cleanup

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions