Skip to content

Commit

Permalink
Update Headers factories to be idiomatic Kotlin
Browse files Browse the repository at this point in the history
  • Loading branch information
JakeWharton committed May 28, 2019
1 parent 1e14585 commit 0827fc6
Show file tree
Hide file tree
Showing 5 changed files with 57 additions and 33 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
package okhttp3.mockwebserver

import okhttp3.Headers
import okhttp3.Headers.Companion.headersOf
import okhttp3.HttpUrl
import okhttp3.Protocol
import okhttp3.Request
Expand Down Expand Up @@ -595,7 +596,7 @@ class MockWebServer : ExternalResource(), Closeable {
@Throws(InterruptedException::class)
private fun dispatchBookkeepingRequest(sequenceNumber: Int, socket: Socket) {
val request = RecordedRequest(
"", Headers.of(), emptyList(), 0L, Buffer(), sequenceNumber, socket)
"", headersOf(), emptyList(), 0L, Buffer(), sequenceNumber, socket)
requestCount.incrementAndGet()
requestQueue.add(request)
dispatcher.dispatch(request)
Expand Down
43 changes: 31 additions & 12 deletions okhttp/src/main/java/okhttp3/Headers.kt
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ import okhttp3.internal.http.toHttpDateString
import org.codehaus.mojo.animal_sniffer.IgnoreJRERequirement
import java.time.Instant
import java.util.ArrayList
import java.util.Arrays
import java.util.Collections
import java.util.Date
import java.util.Locale
Expand Down Expand Up @@ -175,10 +174,10 @@ class Headers private constructor(
* before comparing them for equality.
*/
override fun equals(other: Any?): Boolean {
return other is Headers && Arrays.equals(other.namesAndValues, namesAndValues)
return other is Headers && namesAndValues.contentEquals(other.namesAndValues)
}

override fun hashCode(): Int = Arrays.hashCode(namesAndValues)
override fun hashCode(): Int = namesAndValues.contentHashCode()

override fun toString(): String {
return buildString {
Expand Down Expand Up @@ -213,12 +212,12 @@ class Headers private constructor(
* or cache.
*/
internal fun addLenient(line: String) = apply {
val index = line.indexOf(":", 1)
val index = line.indexOf(':', 1)
when {
index != -1 -> {
addLenient(line.substring(0, index), line.substring(index + 1))
}
line.startsWith(":") -> {
line[0] == ':' -> {
// Work around empty header names and header names that start with a colon (created by old
// broken SPDY versions of the response cache).
addLenient("", line.substring(1)) // Empty header name.
Expand All @@ -232,7 +231,7 @@ class Headers private constructor(

/** Add an header line containing a field name, a literal colon, and a value. */
fun add(line: String) = apply {
val index = line.indexOf(":")
val index = line.indexOf(':')
require(index != -1) { "Unexpected header: $line" }
add(line.substring(0, index).trim(), line.substring(index + 1))
}
Expand Down Expand Up @@ -358,7 +357,8 @@ class Headers private constructor(
* arguments, and they must alternate between header names and values.
*/
@JvmStatic
fun of(vararg namesAndValues: String): Headers {
@JvmName("of")
fun headersOf(vararg namesAndValues: String): Headers {
require(namesAndValues.size % 2 == 0) { "Expected alternating header names and values" }

// Make a defensive copy and clean it up.
Expand All @@ -379,13 +379,23 @@ class Headers private constructor(
return Headers(namesAndValues)
}

@JvmName("-deprecated_of")
@Deprecated(
message = "function name changed",
replaceWith = ReplaceWith(expression = "headersOf(*namesAndValues)"),
level = DeprecationLevel.WARNING)
fun of(vararg namesAndValues: String): Headers {
return headersOf(*namesAndValues)
}

/** Returns headers for the header names and values in the [Map]. */
@JvmStatic
fun of(headers: Map<String, String>): Headers {
@JvmName("of")
fun Map<String, String>.toHeaders(): Headers {
// Make a defensive copy and clean it up.
val namesAndValues = arrayOfNulls<String>(headers.size * 2)
val namesAndValues = arrayOfNulls<String>(size * 2)
var i = 0
for ((k, v) in headers) {
for ((k, v) in this) {
val name = k.trim()
val value = v.trim()
checkName(name)
Expand All @@ -398,7 +408,16 @@ class Headers private constructor(
return Headers(namesAndValues as Array<String>)
}

internal fun checkName(name: String) {
@JvmName("-deprecated_of")
@Deprecated(
message = "function moved to extension",
replaceWith = ReplaceWith(expression = "headers.toHeaders()"),
level = DeprecationLevel.WARNING)
fun of(headers: Map<String, String>): Headers {
return headers.toHeaders()
}

private fun checkName(name: String) {
require(name.isNotEmpty()) { "name is empty" }
for (i in 0 until name.length) {
val c = name[i]
Expand All @@ -408,7 +427,7 @@ class Headers private constructor(
}
}

internal fun checkValue(value: String, name: String) {
private fun checkValue(value: String, name: String) {
for (i in 0 until value.length) {
val c = value[i]
require(c == '\t' || c in '\u0020'..'\u007e') {
Expand Down
3 changes: 2 additions & 1 deletion okhttp/src/main/java/okhttp3/internal/Util.kt
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ package okhttp3.internal

import okhttp3.EventListener
import okhttp3.Headers
import okhttp3.Headers.Companion.headersOf
import okhttp3.HttpUrl
import okhttp3.RequestBody.Companion.toRequestBody
import okhttp3.Response
Expand Down Expand Up @@ -56,7 +57,7 @@ import kotlin.text.Charsets.UTF_32LE
@JvmField
val EMPTY_BYTE_ARRAY = ByteArray(0)
@JvmField
val EMPTY_HEADERS = Headers.of()
val EMPTY_HEADERS = headersOf()

@JvmField
val EMPTY_RESPONSE = EMPTY_BYTE_ARRAY.toResponseBody()
Expand Down
5 changes: 3 additions & 2 deletions okhttp/src/test/java/okhttp3/HeadersKotlinTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -15,21 +15,22 @@
*/
package okhttp3

import okhttp3.Headers.Companion.headersOf
import org.assertj.core.api.Assertions.assertThat
import org.junit.Test
import java.time.Instant
import java.util.Date

class HeadersKotlinTest {
@Test fun getOperator() {
val headers = Headers.of("a", "b", "c", "d")
val headers = headersOf("a", "b", "c", "d")
assertThat(headers["a"]).isEqualTo("b")
assertThat(headers["c"]).isEqualTo("d")
assertThat(headers["e"]).isNull()
}

@Test fun iteratorOperator() {
val headers = Headers.of("a", "b", "c", "d")
val headers = headersOf("a", "b", "c", "d")

val pairs = mutableListOf<Pair<String, String>>()
for ((name, value) in headers) {
Expand Down
36 changes: 19 additions & 17 deletions okhttp/src/test/java/okhttp3/KotlinSourceModernTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@
package okhttp3

import okhttp3.Handshake.Companion.handshake
import okhttp3.Headers.Companion.headersOf
import okhttp3.Headers.Companion.toHeaders
import okhttp3.HttpUrl.Companion.toHttpUrl
import okhttp3.HttpUrl.Companion.toHttpUrlOrNull
import okhttp3.MediaType.Companion.toMediaType
Expand Down Expand Up @@ -149,7 +151,7 @@ class KotlinSourceModernTest {
val immutable: Boolean = cacheControl.immutable
val forceCache: CacheControl = CacheControl.FORCE_CACHE
val forceNetwork: CacheControl = CacheControl.FORCE_NETWORK
val parse: CacheControl = CacheControl.parse(Headers.of())
val parse: CacheControl = CacheControl.parse(headersOf())
}

@Test @Ignore
Expand Down Expand Up @@ -274,7 +276,7 @@ class KotlinSourceModernTest {
val secure: Boolean = cookie.secure
val matches: Boolean = cookie.matches("".toHttpUrl())
val parsedCookie: Cookie? = Cookie.parse("".toHttpUrl(), "")
val cookies: List<Cookie> = Cookie.parseAll("".toHttpUrl(), Headers.of())
val cookies: List<Cookie> = Cookie.parseAll("".toHttpUrl(), headersOf())
}

@Test @Ignore
Expand Down Expand Up @@ -444,8 +446,8 @@ class KotlinSourceModernTest {

@Test @Ignore
fun headers() {
var headers: Headers = Headers.of("", "")
headers = Headers.of(mapOf("" to ""))
var headers: Headers = headersOf("", "")
headers = mapOf("" to "").toHeaders()
val get: String? = headers.get("")
val date: Date? = headers.getDate("")
val instant: Instant? = headers.getInstant("")
Expand All @@ -465,7 +467,7 @@ class KotlinSourceModernTest {
builder = builder.add("")
builder = builder.add("", "")
builder = builder.addUnsafeNonAscii("", "")
builder = builder.addAll(Headers.of())
builder = builder.addAll(headersOf())
builder = builder.add("", Date(0L))
builder = builder.add("", Instant.EPOCH)
builder = builder.set("", "")
Expand Down Expand Up @@ -694,8 +696,8 @@ class KotlinSourceModernTest {
mockResponse = mockResponse.addHeader("", "")
mockResponse = mockResponse.addHeaderLenient("", Any())
mockResponse = mockResponse.setHeader("", Any())
mockResponse = mockResponse.setHeaders(Headers.of())
mockResponse = mockResponse.setTrailers(Headers.of())
mockResponse = mockResponse.setHeaders(headersOf())
mockResponse = mockResponse.setTrailers(headersOf())
mockResponse = mockResponse.removeHeader("")
var body: Buffer? = mockResponse.getBody()
body = mockResponse.getBody()
Expand All @@ -718,7 +720,7 @@ class KotlinSourceModernTest {
val bodyDelay: Long = mockResponse.getBodyDelay(TimeUnit.SECONDS)
mockResponse = mockResponse.setHeadersDelay(0L, TimeUnit.SECONDS)
val headersDelay: Long = mockResponse.getHeadersDelay(TimeUnit.SECONDS)
mockResponse = mockResponse.withPush(PushPromise("", "", Headers.of(), MockResponse()))
mockResponse = mockResponse.withPush(PushPromise("", "", headersOf(), MockResponse()))
var pushPromises: List<PushPromise> = mockResponse.pushPromises
pushPromises = mockResponse.pushPromises
mockResponse = mockResponse.withSettings(Settings())
Expand Down Expand Up @@ -786,7 +788,7 @@ class KotlinSourceModernTest {
fun multipartBodyPart() {
val requestBody: RequestBody = "".toRequestBody(null)
var part: MultipartBody.Part = MultipartBody.Part.create(null, requestBody)
part = MultipartBody.Part.create(Headers.of(), requestBody)
part = MultipartBody.Part.create(headersOf(), requestBody)
part = MultipartBody.Part.create(requestBody)
part = MultipartBody.Part.createFormData("", "")
part = MultipartBody.Part.createFormData("", "", requestBody)
Expand All @@ -802,7 +804,7 @@ class KotlinSourceModernTest {
builder = MultipartBody.Builder("")
builder = builder.setType("".toMediaType())
builder = builder.addPart(requestBody)
builder = builder.addPart(Headers.of(), requestBody)
builder = builder.addPart(headersOf(), requestBody)
builder = builder.addPart(null, requestBody)
builder = builder.addFormDataPart("", "")
builder = builder.addFormDataPart("", "", requestBody)
Expand Down Expand Up @@ -916,7 +918,7 @@ class KotlinSourceModernTest {

@Test @Ignore
fun pushPromise() {
val pushPromise: PushPromise = PushPromise("", "", Headers.of(), MockResponse())
val pushPromise: PushPromise = PushPromise("", "", headersOf(), MockResponse())
val method: String = pushPromise.method
val path: String = pushPromise.path
val headers: Headers = pushPromise.headers
Expand All @@ -935,7 +937,7 @@ class KotlinSourceModernTest {
}
queueDispatcher = QueueDispatcher()
var mockResponse: MockResponse = queueDispatcher.dispatch(
RecordedRequest("", Headers.of(), listOf(), 0L, Buffer(), 0, Socket()))
RecordedRequest("", headersOf(), listOf(), 0L, Buffer(), 0, Socket()))
mockResponse = queueDispatcher.peek()
queueDispatcher.enqueueResponse(MockResponse())
queueDispatcher.shutdown()
Expand All @@ -946,8 +948,8 @@ class KotlinSourceModernTest {
@Test @Ignore
fun recordedRequest() {
var recordedRequest: RecordedRequest = RecordedRequest(
"", Headers.of(), listOf(), 0L, Buffer(), 0, Socket())
recordedRequest = RecordedRequest("", Headers.of(), listOf(), 0L, Buffer(), 0, Socket())
"", headersOf(), listOf(), 0L, Buffer(), 0, Socket())
recordedRequest = RecordedRequest("", headersOf(), listOf(), 0L, Buffer(), 0, Socket())
var requestUrl: HttpUrl? = recordedRequest.requestUrl
var requestLine: String = recordedRequest.requestLine
var method: String? = recordedRequest.method
Expand Down Expand Up @@ -1001,7 +1003,7 @@ class KotlinSourceModernTest {
builder = builder.header("", "")
builder = builder.addHeader("", "")
builder = builder.removeHeader("")
builder = builder.headers(Headers.of())
builder = builder.headers(headersOf())
builder = builder.cacheControl(CacheControl.FORCE_CACHE)
builder = builder.get()
builder = builder.head()
Expand Down Expand Up @@ -1083,7 +1085,7 @@ class KotlinSourceModernTest {
builder = builder.header("", "")
builder = builder.addHeader("", "")
builder = builder.removeHeader("")
builder = builder.headers(Headers.of())
builder = builder.headers(headersOf())
builder = builder.body("".toResponseBody(null))
builder = builder.body(null)
builder = builder.networkResponse(Response.Builder().build())
Expand Down Expand Up @@ -1228,4 +1230,4 @@ class KotlinSourceModernTest {
private fun newRoute(): Route {
return Route(newAddress(), Proxy.NO_PROXY, InetSocketAddress.createUnresolved("", 0))
}
}
}

0 comments on commit 0827fc6

Please sign in to comment.