Skip to content

Commit

Permalink
feat: add codegen wrappers for retries (#490)
Browse files Browse the repository at this point in the history
  • Loading branch information
ianbotsf authored Oct 12, 2021
1 parent 74af3aa commit d887259
Show file tree
Hide file tree
Showing 27 changed files with 576 additions and 132 deletions.
1 change: 1 addition & 0 deletions gradle.properties
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ jacocoVersion=0.8.7
kotlinxBenchmarkVersion=0.3.1

# serialization
kamlVersion=0.36.0
xmlpullVersion=1.1.3.1
xpp3Version=1.1.6

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,7 @@
*/
package aws.smithy.kotlin.runtime.http

import aws.smithy.kotlin.runtime.http.util.StringValuesMap
import aws.smithy.kotlin.runtime.http.util.StringValuesMapBuilder
import aws.smithy.kotlin.runtime.http.util.*
import aws.smithy.kotlin.runtime.http.util.StringValuesMapImpl

/**
Expand Down Expand Up @@ -35,12 +34,13 @@ private object EmptyHeaders : Headers {
/**
* Build an immutable HTTP header map
*/
class HeadersBuilder : StringValuesMapBuilder(true, 8) {
class HeadersBuilder : StringValuesMapBuilder(true, 8), CanDeepCopy<HeadersBuilder> {
override fun toString(): String = "HeadersBuilder ${entries()} "
override fun build(): Headers {
require(!built) { "HeadersBuilder can only build a single Headers instance" }
built = true
return HeadersImpl(values)
override fun build(): Headers = HeadersImpl(values)

override fun deepCopy(): HeadersBuilder {
val originalValues = values.deepCopy()
return HeadersBuilder().apply { values.putAll(originalValues) }
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import aws.smithy.kotlin.runtime.io.SdkByteReadChannel
* HTTP payload to be sent to a peer
*/
sealed class HttpBody {

/**
* Specifies the length of this [HttpBody] content
* If null it is assumed to be a streaming source using e.g. `Transfer-Encoding: Chunked`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,7 @@
*/
package aws.smithy.kotlin.runtime.http

import aws.smithy.kotlin.runtime.http.util.StringValuesMap
import aws.smithy.kotlin.runtime.http.util.StringValuesMapBuilder
import aws.smithy.kotlin.runtime.http.util.StringValuesMapImpl
import aws.smithy.kotlin.runtime.http.util.*
import aws.smithy.kotlin.runtime.util.text.urlEncodeComponent

/**
Expand All @@ -33,12 +31,13 @@ private object EmptyQueryParameters : QueryParameters {
override fun isEmpty(): Boolean = true
}

class QueryParametersBuilder : StringValuesMapBuilder(true, 8) {
class QueryParametersBuilder : StringValuesMapBuilder(true, 8), CanDeepCopy<QueryParametersBuilder> {
override fun toString(): String = "QueryParametersBuilder ${entries()} "
override fun build(): QueryParameters {
require(!built) { "QueryParametersBuilder can only build a single instance" }
built = true
return QueryParametersImpl(values)
override fun build(): QueryParameters = QueryParametersImpl(values)

override fun deepCopy(): QueryParametersBuilder {
val originalValues = values.deepCopy()
return QueryParametersBuilder().apply { values.putAll(originalValues) }
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
*/
package aws.smithy.kotlin.runtime.http

import aws.smithy.kotlin.runtime.http.util.CanDeepCopy
import aws.smithy.kotlin.runtime.util.InternalApi
import aws.smithy.kotlin.runtime.util.text.encodeUrlPath

Expand Down Expand Up @@ -105,7 +106,7 @@ data class UserInfo(val username: String, val password: String)
/**
* Construct a URL by it's individual components
*/
class UrlBuilder {
class UrlBuilder : CanDeepCopy<UrlBuilder> {
var scheme = Protocol.HTTPS
var host: String = ""
var port: Int? = null
Expand All @@ -130,6 +131,20 @@ class UrlBuilder {
forceQuery
)

override fun deepCopy(): UrlBuilder {
val builder = this
return UrlBuilder().apply {
scheme = builder.scheme
host = builder.host
port = builder.port
path = builder.path
parameters = builder.parameters.deepCopy()
fragment = builder.fragment
userInfo = builder.userInfo?.copy()
forceQuery = builder.forceQuery
}
}

override fun toString(): String =
"UrlBuilder(scheme=$scheme, host='$host', port=$port, path='$path', parameters=$parameters, fragment=$fragment, userInfo=$userInfo, forceQuery=$forceQuery)"
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
/*
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
* SPDX-License-Identifier: Apache-2.0.
*/

package aws.smithy.kotlin.runtime.http.middleware

import aws.smithy.kotlin.runtime.http.Feature
import aws.smithy.kotlin.runtime.http.FeatureKey
import aws.smithy.kotlin.runtime.http.HttpBody
import aws.smithy.kotlin.runtime.http.HttpClientFeatureFactory
import aws.smithy.kotlin.runtime.http.operation.SdkHttpOperation
import aws.smithy.kotlin.runtime.http.operation.deepCopy
import aws.smithy.kotlin.runtime.http.request.HttpRequestBuilder
import aws.smithy.kotlin.runtime.retries.RetryPolicy
import aws.smithy.kotlin.runtime.retries.RetryStrategy

class RetryFeature(private val strategy: RetryStrategy, private val policy: RetryPolicy<Any?>) : Feature {
class Config {
var strategy: RetryStrategy? = null
var policy: RetryPolicy<Any?>? = null
}

companion object Feature : HttpClientFeatureFactory<Config, RetryFeature> {
override val key: FeatureKey<RetryFeature> = FeatureKey("RetryFeature")

override fun create(block: Config.() -> Unit): RetryFeature {
val config = Config().apply(block)
val strategy = requireNotNull(config.strategy) { "strategy is required" }
val policy = requireNotNull(config.policy) { "policy is required" }
return RetryFeature(strategy, policy)
}
}

override fun <I, O> install(operation: SdkHttpOperation<I, O>) {
operation.execution.finalize.intercept { req, next ->
if (req.subject.isRetryable) {
strategy.retry(policy) {
// Deep copy the request because later middlewares (e.g., signing) mutate it
val reqCopy = req.deepCopy()

when (val body = reqCopy.subject.body) {
// Reset streaming bodies back to beginning
is HttpBody.Streaming -> body.reset()
}

next.call(reqCopy)
}
} else {
next.call(req)
}
}
}
}

/**
* Indicates whether this HTTP request could be retried. Some requests with streaming bodies are unsuitable for
* retries.
*/
val HttpRequestBuilder.isRetryable: Boolean
get() = when (val body = this.body) {
is HttpBody.Empty, is HttpBody.Bytes -> true
is HttpBody.Streaming -> body.isReplayable
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
package aws.smithy.kotlin.runtime.http.operation

import aws.smithy.kotlin.runtime.client.ExecutionContext
import aws.smithy.kotlin.runtime.http.util.CanDeepCopy

/**
* Wrapper around a type [subject] with an execution context.
Expand All @@ -17,3 +18,11 @@ import aws.smithy.kotlin.runtime.client.ExecutionContext
* @param subject The input type
*/
data class OperationRequest<T>(val context: ExecutionContext, val subject: T)

/**
* Deep copy an [OperationRequest] to a new request. Note that, because [context] is...well, context...it's considered
* transient and is not part of the copy. The subject itself, however, is deeply copied.
* @return A new [OperationRequest] with the same context and a deeply-copied subject.
*/
internal fun <T : CanDeepCopy<T>> OperationRequest<T>.deepCopy(): OperationRequest<T> =
OperationRequest(context, subject.deepCopy())
Original file line number Diff line number Diff line change
Expand Up @@ -6,35 +6,30 @@ package aws.smithy.kotlin.runtime.http.request

import aws.smithy.kotlin.runtime.http.*
import aws.smithy.kotlin.runtime.http.content.ByteArrayContent
import aws.smithy.kotlin.runtime.http.util.CanDeepCopy
import aws.smithy.kotlin.runtime.io.*
import aws.smithy.kotlin.runtime.util.InternalApi

/**
* Used to construct an HTTP request
* @param method The HTTP method (verb) to use when making the request
* @param url Endpoint to make request to
* @param headers HTTP headers
* @param body Outgoing payload. Initially empty
*/
class HttpRequestBuilder {
/**
* The HTTP method (verb) to use when making the request
*/
var method: HttpMethod = HttpMethod.GET

/**
* Endpoint to make request to
*/
val url: UrlBuilder = UrlBuilder()

/**
* HTTP headers
*/
val headers: HeadersBuilder = HeadersBuilder()

/**
* Outgoing payload. Initially empty
*/
var body: HttpBody = HttpBody.Empty
class HttpRequestBuilder private constructor(
var method: HttpMethod,
val url: UrlBuilder,
val headers: HeadersBuilder,
var body: HttpBody,
) : CanDeepCopy<HttpRequestBuilder> {
constructor() : this(HttpMethod.GET, UrlBuilder(), HeadersBuilder(), HttpBody.Empty)

fun build(): HttpRequest = HttpRequest(method, url.build(), if (headers.isEmpty()) Headers.Empty else headers.build(), body)

override fun deepCopy(): HttpRequestBuilder =
HttpRequestBuilder(method, url.deepCopy(), headers.deepCopy(), body)

override fun toString(): String = buildString {
append("HttpRequestBuilder(method=$method, url=$url, headers=$headers, body=$body)")
}
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
/*
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
* SPDX-License-Identifier: Apache-2.0.
*/

package aws.smithy.kotlin.runtime.http.util

/**
* Indicates that an object supports a [deepCopy] operation which will return a copy that can be safely mutated without
* affecting other instances.
*/
interface CanDeepCopy<out T> {
/**
* Returns a deep copy of this object.
*/
fun deepCopy(): T
}
Original file line number Diff line number Diff line change
Expand Up @@ -64,8 +64,10 @@ internal open class StringValuesMapImpl(
override val caseInsensitiveName: Boolean = false,
initialValues: Map<String, List<String>> = emptyMap()
) : StringValuesMap {
protected val values: Map<String, List<String>> by lazy {
if (caseInsensitiveName) CaseInsensitiveMap<List<String>>().apply { putAll(initialValues) } else values.toMap()
protected val values: Map<String, List<String>> = run {
// Make a defensive copy so modifications to the initialValues don't mutate our internal copy
val copiedValues = initialValues.deepCopy()
if (caseInsensitiveName) CaseInsensitiveMap<List<String>>().apply { putAll(copiedValues) } else copiedValues
}

override fun getAll(name: String): List<String>? = values[name]
Expand All @@ -81,13 +83,17 @@ internal open class StringValuesMapImpl(
override fun isEmpty(): Boolean = values.isEmpty()
}

/**
* Perform a deep copy of this map, specifically duplicating the value lists so that they're insulated from changes.
* @return A new map instance with copied value lists.
*/
internal fun Map<String, List<String>>.deepCopy() = mapValues { (_, v) -> v.toMutableList() }

@InternalApi
open class StringValuesMapBuilder(val caseInsensitiveName: Boolean = false, size: Int = 8) {
protected val values: MutableMap<String, MutableList<String>> =
if (caseInsensitiveName) CaseInsensitiveMap() else LinkedHashMap(size)

protected var built = false

fun getAll(name: String): List<String>? = values[name]

operator fun contains(name: String): Boolean = name in values
Expand Down Expand Up @@ -154,16 +160,8 @@ open class StringValuesMapBuilder(val caseInsensitiveName: Boolean = false, size

fun clear() = values.clear()

open fun build(): StringValuesMap {
require(!built) { "StringValuesMapBuilder can only build a single instance" }
built = true
return StringValuesMapImpl(caseInsensitiveName, values)
}
open fun build(): StringValuesMap = StringValuesMapImpl(caseInsensitiveName, values)

private fun ensureListForKey(name: String, size: Int): MutableList<String> {
if (built) {
throw IllegalStateException("Cannot modify a builder when final structure has already been built")
}
return values[name] ?: ArrayList<String>(size).also { values[name] = it }
}
private fun ensureListForKey(name: String, size: Int): MutableList<String> =
values[name] ?: ArrayList<String>(size).also { values[name] = it }
}
Original file line number Diff line number Diff line change
Expand Up @@ -28,4 +28,31 @@ class HeadersTest {
}
assertEquals("Headers [key=[value]]", "$actual2")
}

@Test
fun testSubsequentModificationsDontAffectOriginal() {
val builder = HeadersBuilder()

builder.append("a", "alligator")
builder.append("b", "bunny")
builder.append("c", "chinchilla")
val first = builder.build()
val firstExpected = mapOf(
"a" to listOf("alligator"),
"b" to listOf("bunny"),
"c" to listOf("chinchilla"),
)

builder.append("a", "anteater")
builder.remove("b")
builder["c"] = "crocodile"
val second = builder.build()
val secondExpected = mapOf(
"a" to listOf("alligator", "anteater"),
"c" to listOf("crocodile"),
)

assertEquals(firstExpected.entries, first.entries())
assertEquals(secondExpected.entries, second.entries())
}
}
Loading

0 comments on commit d887259

Please sign in to comment.