From fc6fd17b88d2dbc0530b7abfb36e8915ca5976cb Mon Sep 17 00:00:00 2001 From: OmarAlJarrah Date: Tue, 7 Jul 2026 16:32:21 +0300 Subject: [PATCH] refactor: share Uri/Url decoded-path projection and single-source builder gates MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two verbatim duplications between the deliberately-separate Uri and Url profiles, factored without coupling the two: - The decoded-path-segments projection (a `when` over the shared UrlPath that percent-decodes each segment) was byte-identical in Uri.computeDecodedPathSegments and Url.pathSegments. Move it to a `decodedSegments` helper in UrlPath, alongside the existing shared fileNameOf / toUriPathString / appendPathSegments projections, following the same decode-lambda convention that keeps UrlPath percent-codec-free. Each caller's caching is unchanged (Uri still memoizes via `by lazy`, Url still recomputes per access). - Each builder stated its composability rules twice — throwing `require`s for build() and a boolean chain for buildOrNull() — so a rule change had to be mirrored by hand or the two would silently diverge. Replace each with one ordered, private, per-profile `composabilityError(): String?`; build() throws IllegalArgumentException with the first failure's message (identical to the prior `require`), buildOrNull() returns null when it is non-null. The two forms were verified exactly equivalent beforehand. Per-profile throughout: no shared supertype, no cross-profile coupling. Pure refactor, public API unchanged. --- .../commonMain/kotlin/org/dexpace/kuri/Uri.kt | 54 ++++++++----------- .../commonMain/kotlin/org/dexpace/kuri/Url.kt | 49 ++++++++--------- .../kotlin/org/dexpace/kuri/parser/UrlPath.kt | 17 ++++++ 3 files changed, 65 insertions(+), 55 deletions(-) diff --git a/kuri/src/commonMain/kotlin/org/dexpace/kuri/Uri.kt b/kuri/src/commonMain/kotlin/org/dexpace/kuri/Uri.kt index 794e371..cf33f6a 100644 --- a/kuri/src/commonMain/kotlin/org/dexpace/kuri/Uri.kt +++ b/kuri/src/commonMain/kotlin/org/dexpace/kuri/Uri.kt @@ -13,6 +13,7 @@ import org.dexpace.kuri.parser.ParsedComponents import org.dexpace.kuri.parser.Resolver import org.dexpace.kuri.parser.UriParser import org.dexpace.kuri.parser.UrlPath +import org.dexpace.kuri.parser.decodedSegments import org.dexpace.kuri.parser.fileExtensionOf import org.dexpace.kuri.parser.fileNameOf import org.dexpace.kuri.parser.toUriPathString @@ -540,10 +541,7 @@ public class Uri internal constructor( /** The decoded segments backing [pathSegments]; an opaque path yields its single decoded value. */ private fun computeDecodedPathSegments(): List = - when (val storedPath = components.path) { - is UrlPath.Opaque -> listOf(PercentCodec.decode(storedPath.path)) - is UrlPath.Segments -> storedPath.segments.map { PercentCodec.decode(it) } - } + decodedSegments(components.path) { PercentCodec.decode(it) } /** * True for the RFC 3986 opaque shape: an absolute URI with no authority and a rootless path. @@ -960,7 +958,7 @@ public class Uri internal constructor( */ public fun build(): Uri { val path = effectivePath() - validateComposable(path) + composabilityError(path)?.let { throw IllegalArgumentException(it) } return buildResult(path).getOrThrow() } @@ -977,7 +975,7 @@ public class Uri internal constructor( */ public fun buildOrNull(): Uri? { val path = effectivePath() - if (!isComposable(path)) return null + if (composabilityError(path) != null) return null return buildResult(path).getOrNull() } @@ -993,34 +991,28 @@ public class Uri internal constructor( UriParser.parse(recompose(path), options).map { Uri(it) } /** - * Rejects component combinations no RFC 3986 recomposition can represent (RFC 3986 §3.2/§3.3). + * The RFC 3986 §3.2/§3.3 message for the first component combination no recomposition can + * represent, or `null` when the accumulated components are composable. * - * `userinfo`/`port` are authority sub-components, so they require a host; a caller wanting an - * empty authority passes `host("")`. A present authority forbids a rootless [path], which - * would otherwise merge into the authority on re-parse; a segment-built path is already rooted - * by [effectivePath] when a host is present, so only a verbatim rootless path can trip this. + * The single ordered source of the composability rules [build] and [buildOrNull] share, so a + * rule cannot drift between the throwing and the non-throwing path. `userinfo`/`port` are + * authority sub-components, so they require a host; a caller wanting an empty authority passes + * `host("")`. A present authority forbids a rootless [path], which would otherwise merge into + * the authority on re-parse; a segment-built path is already rooted by [effectivePath] when a + * host is present, so only a verbatim rootless path can trip that rule. */ - private fun validateComposable(path: String) { - require(authorityHasHost()) { - "userInfo/port require a host: set host(\"\") for an empty-authority URI, or drop them" - } - require(pathFitsAuthority(path)) { - "a path with an authority must be empty or start with '/': $path" - } - require(host != null || !path.startsWith("//")) { - "an authority-less path cannot begin with '//': $path" + private fun composabilityError(path: String): String? = + when { + !authorityHasHost() -> + "userInfo/port require a host: set host(\"\") for an empty-authority URI, or drop them" + !pathFitsAuthority(path) -> + "a path with an authority must be empty or start with '/': $path" + host == null && path.startsWith("//") -> + "an authority-less path cannot begin with '//': $path" + !this.path.wellFormed() -> + "a rootless path cannot begin with an empty segment; the edit would re-root it: $path" + else -> null } - require(this.path.wellFormed()) { - "a rootless path cannot begin with an empty segment; the edit would re-root it: $path" - } - } - - /** True iff [build] would pass [validateComposable] for [path] without throwing; for [buildOrNull]. */ - private fun isComposable(path: String): Boolean = - authorityHasHost() && - pathFitsAuthority(path) && - (host != null || !path.startsWith("//")) && - this.path.wellFormed() /** True when a [host] is present, or neither [userInfo] nor [port] (which require one) is set. */ private fun authorityHasHost(): Boolean = host != null || (userInfo.isNullOrEmpty() && port == null) diff --git a/kuri/src/commonMain/kotlin/org/dexpace/kuri/Url.kt b/kuri/src/commonMain/kotlin/org/dexpace/kuri/Url.kt index e7064a3..f0fea3b 100644 --- a/kuri/src/commonMain/kotlin/org/dexpace/kuri/Url.kt +++ b/kuri/src/commonMain/kotlin/org/dexpace/kuri/Url.kt @@ -13,6 +13,7 @@ import org.dexpace.kuri.parser.BuilderPath import org.dexpace.kuri.parser.ParsedComponents import org.dexpace.kuri.parser.UrlParser import org.dexpace.kuri.parser.UrlPath +import org.dexpace.kuri.parser.decodedSegments import org.dexpace.kuri.parser.fileExtensionOf import org.dexpace.kuri.parser.fileNameOf import org.dexpace.kuri.percent.PercentCodec @@ -136,11 +137,7 @@ public class Url internal constructor( /** The decoded path segments in order (read-only); an opaque path yields its single decoded value. */ @get:JvmName("pathSegments") public val pathSegments: List - get() = - when (val path = components.path) { - is UrlPath.Opaque -> listOf(PercentCodec.decode(path.path)) - is UrlPath.Segments -> path.segments.map { PercentCodec.decode(it) } - } + get() = decodedSegments(components.path) { PercentCodec.decode(it) } /** The canonical encoded path string (e.g. `/a/b`, or the opaque path verbatim). */ @get:JvmName("encodedPath") @@ -806,18 +803,7 @@ public class Url internal constructor( */ public fun build(): Url { val path = effectivePath() - require(hasRequiredHost()) { - "a special scheme requires a host: set host(...) before build(): $scheme" - } - require(pathMatchesAuthority(path)) { - "a path with an authority must be empty or start with '/': $path" - } - require(pathAllowedWithoutAuthority(path)) { - "an authority-less path cannot begin with '//': $path" - } - require(this.path.wellFormed()) { - "a rootless path cannot begin with an empty segment; the edit would re-root it: $path" - } + composabilityError(path)?.let { throw IllegalArgumentException(it) } return buildResult(path).getOrThrow() } @@ -835,7 +821,7 @@ public class Url internal constructor( */ public fun buildOrNull(): Url? { val path = effectivePath() - if (!isComposable(path)) return null + if (composabilityError(path) != null) return null return buildResult(path).getOrNull() } @@ -891,11 +877,26 @@ public class Url internal constructor( */ private fun pathAllowedWithoutAuthority(path: String): Boolean = host != null || !path.startsWith("//") - /** True iff [build] would pass its composability requires for [path] without throwing; for [buildOrNull]. */ - private fun isComposable(path: String): Boolean = - hasRequiredHost() && - pathMatchesAuthority(path) && - pathAllowedWithoutAuthority(path) && - this.path.wellFormed() + /** + * The message for the first component combination that cannot form a valid URL, or `null` when + * the accumulated components are composable. + * + * The single ordered source of the composability rules [build] and [buildOrNull] share, so a + * rule cannot drift between the throwing and the non-throwing path: a special (non-`file`) + * scheme needs a host (whose absence the parser would misread as the first path segment), a + * present authority forbids a rootless [path], and a rootless path must not re-root at build. + */ + private fun composabilityError(path: String): String? = + when { + !hasRequiredHost() -> + "a special scheme requires a host: set host(...) before build(): $scheme" + !pathMatchesAuthority(path) -> + "a path with an authority must be empty or start with '/': $path" + !pathAllowedWithoutAuthority(path) -> + "an authority-less path cannot begin with '//': $path" + !this.path.wellFormed() -> + "a rootless path cannot begin with an empty segment; the edit would re-root it: $path" + else -> null + } } } diff --git a/kuri/src/commonMain/kotlin/org/dexpace/kuri/parser/UrlPath.kt b/kuri/src/commonMain/kotlin/org/dexpace/kuri/parser/UrlPath.kt index b4f0c93..cb2ad22 100644 --- a/kuri/src/commonMain/kotlin/org/dexpace/kuri/parser/UrlPath.kt +++ b/kuri/src/commonMain/kotlin/org/dexpace/kuri/parser/UrlPath.kt @@ -156,6 +156,23 @@ internal fun fileExtensionOf(name: String): String { return if (hasNonDotStem) name.substring(dot + 1) else "" } +/** + * The decoded path segments of [path], each percent-decoded through [decode]; an + * [Opaque][UrlPath.Opaque] path has no segment structure and yields its single decoded value as a + * one-element list. Shared by the `Uri`/`Url` `pathSegments` projections. + * + * Takes the decoder as a lambda so this helper stays free of the percent-codec dependency, the same + * convention [appendPathSegments] follows with its `encode` lambda. + */ +internal fun decodedSegments( + path: UrlPath, + decode: (String) -> String, +): List = + when (path) { + is UrlPath.Opaque -> listOf(decode(path.path)) + is UrlPath.Segments -> path.segments.map(decode) + } + /** * Appends the '/'-separated [input] onto [current] as decoded path segments (OkHttp * `HttpUrl.Builder.addPathSegments` semantics), encoding each piece with [encode].