refactor(retrofit): removed last few remaining references to retrofit1 (#2125)

alice485 · alice garcia · web-flow · commit 0f7a37d05cea · 2025-03-27T16:13:00.000Z
Co-authored-by: alice garcia &lt;alice_garcia@homedepot.com&gt;
diff --git a/keel-igor/src/test/kotlin/com/netflix/spinnaker/keel/services/ArtifactMetadataServiceTests.kt b/keel-igor/src/test/kotlin/com/netflix/spinnaker/keel/services/ArtifactMetadataServiceTests.kt
@@ -18,8 +18,9 @@ import dev.minutest.rootContext
 import io.mockk.coEvery
 import io.mockk.mockk
 import kotlinx.coroutines.runBlocking
-import retrofit.RetrofitError
-import retrofit.client.Response
+import okhttp3.MediaType.Companion.toMediaTypeOrNull
+import okhttp3.ResponseBody.Companion.toResponseBody
+import retrofit2.HttpException
 import strikt.api.expectCatching
 import strikt.api.expectThat
 import strikt.assertions.isEqualTo
@@ -141,24 +142,22 @@ class ArtifactMetadataServiceTests : JUnit5Minutests {
       }
 
       context("with HTTP error coming from igor") {
-        val retrofitError = RetrofitError.httpError(
-          "http://igor",
-          Response("http://igor", 404, "not found", emptyList(), null),
-          null, null
+        val notFoundError = HttpException(
+          retrofit2.Response.error<Any>(404, "".toResponseBody("application/json".toMediaTypeOrNull()))
         )
 
         before {
           coEvery {
             buildService.getArtifactMetadata(commitId = any(), buildNumber = any(), completionStatus = any())
-          } throws retrofitError
+          } throws notFoundError
         }
 
         test("throw an http error from fallback method") {
           expectCatching {
             artifactMetadataService.getArtifactMetadata("1", "a15p0")
           }
             .isFailure()
-            .isEqualTo(retrofitError)
+            .isEqualTo(notFoundError)
         }
       }
     }
diff --git a/keel-igor/src/test/kotlin/com/netflix/spinnaker/keel/services/DeliveryConfigImporterTests.kt b/keel-igor/src/test/kotlin/com/netflix/spinnaker/keel/services/DeliveryConfigImporterTests.kt
@@ -18,8 +18,9 @@ import com.netflix.spinnaker.keel.test.deliveryConfig
 import dev.minutest.junit.JUnit5Minutests
 import dev.minutest.rootContext
 import io.mockk.mockk
-import retrofit.RetrofitError
-import retrofit.client.Response
+import okhttp3.MediaType.Companion.toMediaTypeOrNull
+import okhttp3.ResponseBody.Companion.toResponseBody
+import retrofit2.HttpException
 import strikt.api.expectCatching
 import strikt.api.expectThat
 import strikt.assertions.all
@@ -121,16 +122,14 @@ class DeliveryConfigImporterTests : JUnit5Minutests {
       }
 
       context("with HTTP error retrieving delivery config from igor") {
-        val retrofitError = RetrofitError.httpError(
-          "http://igor",
-          Response("http://igor", 404, "not found", emptyList(), null),
-          null, null
+        val notFoundError = HttpException(
+          retrofit2.Response.error<Any>(404, "".toResponseBody("application/yaml".toMediaTypeOrNull()))
         )
 
         before {
           every {
             scmService.getDeliveryConfigManifest("stash", "proj", "repo", "spinnaker.yml", any(), any())
-          } throws retrofitError
+          } throws notFoundError
         }
 
         test("bubbles up HTTP error") {
@@ -144,7 +143,7 @@ class DeliveryConfigImporterTests : JUnit5Minutests {
             )
           }
             .isFailure()
-            .isEqualTo(retrofitError)
+            .isEqualTo(notFoundError)
         }
       }
 
diff --git a/keel-web/keel-web.gradle b/keel-web/keel-web.gradle
@@ -63,6 +63,7 @@ dependencies {
   testImplementation(project(":keel-clouddriver"))
   testImplementation("io.strikt:strikt-jackson")
   testImplementation("com.fasterxml.jackson.module:jackson-module-kotlin")
+  testImplementation("io.spinnaker.kork:kork-retrofit")
   testImplementation("io.spinnaker.kork:kork-security")
   testImplementation("com.squareup.okhttp3:mockwebserver")
   testImplementation("org.testcontainers:mysql:${testContainersVersion}")
diff --git a/keel-web/src/test/kotlin/com/netflix/spinnaker/keel/rest/DeliveryConfigControllerTests.kt b/keel-web/src/test/kotlin/com/netflix/spinnaker/keel/rest/DeliveryConfigControllerTests.kt
@@ -27,10 +27,12 @@ import com.netflix.spinnaker.keel.persistence.DismissibleNotificationRepository
 import com.netflix.spinnaker.keel.persistence.KeelRepository
 import com.netflix.spinnaker.keel.persistence.NoDeliveryConfigForApplication
 import com.netflix.spinnaker.keel.persistence.NoSuchDeliveryConfigName
+import com.netflix.spinnaker.keel.retrofit.InstrumentedJacksonConverter
 import com.netflix.spinnaker.keel.spring.test.MockEurekaConfiguration
 import com.netflix.spinnaker.keel.test.DummyResourceSpec
 import com.netflix.spinnaker.keel.test.TEST_API_V1
 import com.netflix.spinnaker.keel.yaml.APPLICATION_YAML
+import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerHttpException
 import com.ninjasquad.springmockk.MockkBean
 import dev.minutest.junit.JUnit5Minutests
 import dev.minutest.rootContext
@@ -41,6 +43,10 @@ import io.mockk.coVerify
 import io.mockk.every
 import io.mockk.just
 import io.mockk.verify
+import okhttp3.MediaType.Companion.toMediaTypeOrNull
+import okhttp3.Protocol
+import okhttp3.Request
+import okhttp3.ResponseBody.Companion.toResponseBody
 import org.springframework.beans.factory.annotation.Autowired
 import org.springframework.boot.test.autoconfigure.web.servlet.AutoConfigureMockMvc
 import org.springframework.boot.test.context.SpringBootTest
@@ -54,8 +60,8 @@ import org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get
 import org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post
 import org.springframework.test.web.servlet.result.MockMvcResultMatchers.content
 import org.springframework.test.web.servlet.result.MockMvcResultMatchers.status
-import retrofit.RetrofitError
-import retrofit.client.Response
+import retrofit2.Response
+import retrofit2.Retrofit
 import strikt.api.expectThat
 import strikt.jackson.at
 import strikt.jackson.isMissing
@@ -454,16 +460,26 @@ internal class DeliveryConfigControllerTests
       }
 
       context("when manifest retrieval fails") {
-        val retrofitError = RetrofitError.httpError(
-          "http://igor",
-          Response("http://igor", 404, "not found", emptyList(), null),
-          null, null
-        )
+        val ok3Resp =
+          okhttp3.Response.Builder()
+            .code(404)
+            .message("not found")
+            .protocol(Protocol.HTTP_1_1)
+            .request(Request.Builder().url("http://igor/").build())
+            .build()
+
+        val retrofit =
+          Retrofit.Builder()
+            .baseUrl("http://igor/")
+            .addConverterFactory(InstrumentedJacksonConverter.Factory("Igor", ObjectMapper()))
+            .build()
+
+        val notFoundError = SpinnakerHttpException(Response.error<Any>("".toResponseBody("application/yaml".toMediaTypeOrNull()), ok3Resp), retrofit)
 
         before {
           every {
             importer.import("stash", "proj", "repo", "spinnaker.yml", "refs/heads/master")
-          } throws retrofitError
+          } throws notFoundError
         }
 
         test("the error from the downstream service is returned") {

Original file line number	Diff line number	Diff line change
`@@ -18,8 +18,9 @@ import dev.minutest.rootContext`
`18`	`18`	`import io.mockk.coEvery`
`19`	`19`	`import io.mockk.mockk`
`20`	`20`	`import kotlinx.coroutines.runBlocking`
`21`		`-import retrofit.RetrofitError`
`22`		`-import retrofit.client.Response`
	`21`	`+import okhttp3.MediaType.Companion.toMediaTypeOrNull`
	`22`	`+import okhttp3.ResponseBody.Companion.toResponseBody`
	`23`	`+import retrofit2.HttpException`
`23`	`24`	`import strikt.api.expectCatching`
`24`	`25`	`import strikt.api.expectThat`
`25`	`26`	`import strikt.assertions.isEqualTo`
`@@ -141,24 +142,22 @@ class ArtifactMetadataServiceTests : JUnit5Minutests {`
`141`	`142`	`}`
`142`	`143`
`143`	`144`	`context("with HTTP error coming from igor") {`
`144`		`- val retrofitError = RetrofitError.httpError(`
`145`		`- "http://igor",`
`146`		`- Response("http://igor", 404, "not found", emptyList(), null),`
`147`		`- null, null`
	`145`	`+ val notFoundError = HttpException(`
	`146`	`+ retrofit2.Response.error<Any>(404, "".toResponseBody("application/json".toMediaTypeOrNull()))`
`148`	`147`	`)`
`149`	`148`
`150`	`149`	`before {`
`151`	`150`	`coEvery {`
`152`	`151`	`buildService.getArtifactMetadata(commitId = any(), buildNumber = any(), completionStatus = any())`
`153`		`- } throws retrofitError`
	`152`	`+ } throws notFoundError`
`154`	`153`	`}`
`155`	`154`
`156`	`155`	`test("throw an http error from fallback method") {`
`157`	`156`	`expectCatching {`
`158`	`157`	`artifactMetadataService.getArtifactMetadata("1", "a15p0")`
`159`	`158`	`}`
`160`	`159`	`.isFailure()`
`161`		`- .isEqualTo(retrofitError)`
	`160`	`+ .isEqualTo(notFoundError)`
`162`	`161`	`}`
`163`	`162`	`}`
`164`	`163`	`}`
Original file line number	Diff line number	Diff line change
`@@ -18,8 +18,9 @@ import com.netflix.spinnaker.keel.test.deliveryConfig`
`18`	`18`	`import dev.minutest.junit.JUnit5Minutests`
`19`	`19`	`import dev.minutest.rootContext`
`20`	`20`	`import io.mockk.mockk`
`21`		`-import retrofit.RetrofitError`
`22`		`-import retrofit.client.Response`
	`21`	`+import okhttp3.MediaType.Companion.toMediaTypeOrNull`
	`22`	`+import okhttp3.ResponseBody.Companion.toResponseBody`
	`23`	`+import retrofit2.HttpException`
`23`	`24`	`import strikt.api.expectCatching`
`24`	`25`	`import strikt.api.expectThat`
`25`	`26`	`import strikt.assertions.all`
`@@ -121,16 +122,14 @@ class DeliveryConfigImporterTests : JUnit5Minutests {`
`121`	`122`	`}`
`122`	`123`
`123`	`124`	`context("with HTTP error retrieving delivery config from igor") {`
`124`		`- val retrofitError = RetrofitError.httpError(`
`125`		`- "http://igor",`
`126`		`- Response("http://igor", 404, "not found", emptyList(), null),`
`127`		`- null, null`
	`125`	`+ val notFoundError = HttpException(`
	`126`	`+ retrofit2.Response.error<Any>(404, "".toResponseBody("application/yaml".toMediaTypeOrNull()))`
`128`	`127`	`)`
`129`	`128`
`130`	`129`	`before {`
`131`	`130`	`every {`
`132`	`131`	`scmService.getDeliveryConfigManifest("stash", "proj", "repo", "spinnaker.yml", any(), any())`
`133`		`- } throws retrofitError`
	`132`	`+ } throws notFoundError`
`134`	`133`	`}`
`135`	`134`
`136`	`135`	`test("bubbles up HTTP error") {`
`@@ -144,7 +143,7 @@ class DeliveryConfigImporterTests : JUnit5Minutests {`
`144`	`143`	`)`
`145`	`144`	`}`
`146`	`145`	`.isFailure()`
`147`		`- .isEqualTo(retrofitError)`
	`146`	`+ .isEqualTo(notFoundError)`
`148`	`147`	`}`
`149`	`148`	`}`
`150`	`149`