From 046a2ab05d7d8b86934430bcef696ee03c9ff4c9 Mon Sep 17 00:00:00 2001 From: Jesse Wilson Date: Wed, 21 Jun 2023 22:24:07 -0400 Subject: [PATCH] Make API compatibility decisions (#1052) --- .../compatibility/ApiCompatibilityDecision.kt | 99 ++++++ .../ApiCompatibilityDecisionTest.kt | 321 ++++++++++++++++++ 2 files changed, 420 insertions(+) create mode 100644 zipline-kotlin-plugin/src/main/kotlin/app/cash/zipline/api/compatibility/ApiCompatibilityDecision.kt create mode 100644 zipline-kotlin-plugin/src/test/kotlin/app/cash/zipline/api/compatibility/ApiCompatibilityDecisionTest.kt diff --git a/zipline-kotlin-plugin/src/main/kotlin/app/cash/zipline/api/compatibility/ApiCompatibilityDecision.kt b/zipline-kotlin-plugin/src/main/kotlin/app/cash/zipline/api/compatibility/ApiCompatibilityDecision.kt new file mode 100644 index 0000000000..4b70c185b9 --- /dev/null +++ b/zipline-kotlin-plugin/src/main/kotlin/app/cash/zipline/api/compatibility/ApiCompatibilityDecision.kt @@ -0,0 +1,99 @@ +/* + * Copyright (C) 2023 Cash App + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package app.cash.zipline.api.compatibility + +import app.cash.zipline.api.fir.FirZiplineApi +import app.cash.zipline.api.toml.TomlZiplineApi +import app.cash.zipline.api.toml.TomlZiplineFunction +import app.cash.zipline.api.toml.TomlZiplineService + +sealed interface ApiCompatibilityDecision + +class ActualApiHasProblems( + val messages: List, +) : ApiCompatibilityDecision + +class ExpectedApiRequiresUpdates( + val updatedApi: TomlZiplineApi, +) : ApiCompatibilityDecision + +object ExpectedApiIsUpToDate : ApiCompatibilityDecision + +/** Compare the expected and actual API and decide what to do about it. */ +fun makeApiCompatibilityDecision( + expectedApi: TomlZiplineApi, + actualApi: FirZiplineApi, +): ApiCompatibilityDecision { + val problemMessages = mutableListOf() + var hasChanges = false + + val actualServices = actualApi.services.associateBy { it.name } + if (actualServices.size != expectedApi.services.size) hasChanges = true + + for (expectedService in expectedApi.services) { + val serviceName = expectedService.name + val actualService = actualServices[serviceName] + + if (actualService == null) { + problemMessages += + """ + |Expected service not found: + | $serviceName + """.trimMargin() + continue + } + + val actualFunctions = actualService.functions.associateBy { it.id } + if (actualFunctions.size != expectedService.functions.size) hasChanges = true + + for (expectedFunction in expectedService.functions) { + val functionId = expectedFunction.id + val actualFunction = actualFunctions[functionId] + if (actualFunction == null) { + val comment = expectedFunction.leadingComment + problemMessages += buildString { + append("Expected function $functionId of $serviceName not found") + if (comment.isNotEmpty()) { + append(":\n ") + append(comment.replace("\n", "\n ")) + } + } + } + } + } + + return when { + problemMessages.isNotEmpty() -> ActualApiHasProblems(problemMessages) + hasChanges -> ExpectedApiRequiresUpdates(actualApi.toToml()) + else -> ExpectedApiIsUpToDate + } +} + +private fun FirZiplineApi.toToml(): TomlZiplineApi { + return TomlZiplineApi( + services.map { service -> + TomlZiplineService( + name = service.name, + functions = service.functions.map { function -> + TomlZiplineFunction( + leadingComment = function.signature, + id = function.id, + ) + }, + ) + }, + ) +} diff --git a/zipline-kotlin-plugin/src/test/kotlin/app/cash/zipline/api/compatibility/ApiCompatibilityDecisionTest.kt b/zipline-kotlin-plugin/src/test/kotlin/app/cash/zipline/api/compatibility/ApiCompatibilityDecisionTest.kt new file mode 100644 index 0000000000..311f03be0d --- /dev/null +++ b/zipline-kotlin-plugin/src/test/kotlin/app/cash/zipline/api/compatibility/ApiCompatibilityDecisionTest.kt @@ -0,0 +1,321 @@ +/* + * Copyright (C) 2023 Cash App + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package app.cash.zipline.api.compatibility + +import app.cash.zipline.api.fir.FirZiplineApi +import app.cash.zipline.api.fir.FirZiplineFunction +import app.cash.zipline.api.fir.FirZiplineService +import app.cash.zipline.api.toml.TomlZiplineApi +import app.cash.zipline.api.toml.TomlZiplineFunction +import app.cash.zipline.api.toml.TomlZiplineService +import app.cash.zipline.kotlin.signatureHash +import assertk.assertThat +import assertk.assertions.containsExactly +import assertk.assertions.isEqualTo +import assertk.assertions.isInstanceOf +import org.junit.Test + +class ApiCompatibilityDecisionTest { + @Test + fun actualServiceIsMissing() { + val expectedApi = TomlZiplineApi( + listOf( + TomlZiplineService( + name = "com.example.EchoService", + functions = listOf( + TomlZiplineFunction( + "fun echo(kotlin.String): kotlin.String", + "fun echo(kotlin.String): kotlin.String".signatureHash(), + ), + ), + ), + ), + ) + val actualApi = FirZiplineApi(listOf()) + + val decision = makeApiCompatibilityDecision(expectedApi, actualApi) as ActualApiHasProblems + assertThat(decision.messages).containsExactly( + """ + |Expected service not found: + | com.example.EchoService + """.trimMargin(), + ) + } + + @Test + fun actualFunctionIsMissing() { + val expectedApi = TomlZiplineApi( + listOf( + TomlZiplineService( + name = "com.example.EchoService", + functions = listOf( + TomlZiplineFunction( + "fun echo(kotlin.String): kotlin.String", + "fun echo(kotlin.String): kotlin.String".signatureHash(), + ), + TomlZiplineFunction( + "fun echoTwo(kotlin.String): kotlin.String", + "fun echoTwo(kotlin.String): kotlin.String".signatureHash(), + ), + ), + ), + ), + ) + val actualApi = FirZiplineApi( + listOf( + FirZiplineService( + name = "com.example.EchoService", + functions = listOf( + FirZiplineFunction("fun echo(kotlin.String): kotlin.String"), + ), + ), + ), + ) + + val decision = makeApiCompatibilityDecision(expectedApi, actualApi) as ActualApiHasProblems + assertThat(decision.messages).containsExactly( + """ + |Expected function AFSfjQyQ of com.example.EchoService not found: + | fun echoTwo(kotlin.String): kotlin.String + """.trimMargin(), + ) + } + + @Test + fun serviceAdded() { + val expectedApi = TomlZiplineApi(listOf()) + val actualApi = FirZiplineApi( + listOf( + FirZiplineService( + name = "com.example.EchoService", + functions = listOf( + FirZiplineFunction("fun echo(kotlin.String): kotlin.String"), + ), + ), + ), + ) + + val decision = makeApiCompatibilityDecision(expectedApi, actualApi) + as ExpectedApiRequiresUpdates + assertThat(decision.updatedApi).isEqualTo( + TomlZiplineApi( + listOf( + TomlZiplineService( + name = "com.example.EchoService", + functions = listOf( + TomlZiplineFunction( + "fun echo(kotlin.String): kotlin.String", + "fun echo(kotlin.String): kotlin.String".signatureHash(), + ), + ), + ), + ), + ), + ) + } + + @Test + fun functionAdded() { + val expectedApi = TomlZiplineApi( + listOf( + TomlZiplineService( + name = "com.example.EchoService", + functions = listOf( + TomlZiplineFunction( + "fun echo(kotlin.String): kotlin.String", + "fun echo(kotlin.String): kotlin.String".signatureHash(), + ), + ), + ), + ), + ) + + val actualApi = FirZiplineApi( + listOf( + FirZiplineService( + name = "com.example.EchoService", + functions = listOf( + FirZiplineFunction("fun echo(kotlin.String): kotlin.String"), + FirZiplineFunction("fun echoTwo(kotlin.String): kotlin.String"), + ), + ), + ), + ) + + val decision = makeApiCompatibilityDecision(expectedApi, actualApi) + as ExpectedApiRequiresUpdates + assertThat(decision.updatedApi).isEqualTo( + TomlZiplineApi( + listOf( + TomlZiplineService( + name = "com.example.EchoService", + functions = listOf( + TomlZiplineFunction( + "fun echo(kotlin.String): kotlin.String", + "fun echo(kotlin.String): kotlin.String".signatureHash(), + ), + TomlZiplineFunction( + "fun echoTwo(kotlin.String): kotlin.String", + "fun echoTwo(kotlin.String): kotlin.String".signatureHash(), + ), + ), + ), + ), + ), + ) + } + + @Test + fun expectedApiIsUpToDate() { + val expectedApi = TomlZiplineApi( + listOf( + TomlZiplineService( + name = "com.example.EchoService", + functions = listOf( + TomlZiplineFunction( + "fun echo(kotlin.String): kotlin.String", + "fun echo(kotlin.String): kotlin.String".signatureHash(), + ), + ), + ), + ), + ) + + val actualApi = FirZiplineApi( + listOf( + FirZiplineService( + name = "com.example.EchoService", + functions = listOf( + FirZiplineFunction("fun echo(kotlin.String): kotlin.String"), + ), + ), + ), + ) + + val decision = makeApiCompatibilityDecision(expectedApi, actualApi) + assertThat(decision).isEqualTo(ExpectedApiIsUpToDate) + } + + /** + * Confirm [ActualApiHasProblems] takes precedence over [ExpectedApiRequiresUpdates] when both + * apply because the service names are disjoint. + */ + @Test + fun differentServicesDecidesActualApiHasProblems() { + val expectedApi = TomlZiplineApi( + listOf( + TomlZiplineService( + name = "com.example.EchoService", + functions = listOf( + TomlZiplineFunction( + "fun echo(kotlin.String): kotlin.String", + "fun echo(kotlin.String): kotlin.String".signatureHash(), + ), + ), + ), + ), + ) + + val actualApi = FirZiplineApi( + listOf( + FirZiplineService( + name = "com.example.HelloService", + functions = listOf( + FirZiplineFunction("fun echo(kotlin.String): kotlin.String"), + ), + ), + ), + ) + + val decision = makeApiCompatibilityDecision(expectedApi, actualApi) + assertThat(decision).isInstanceOf() + } + + /** + * Confirm [ActualApiHasProblems] takes precedence over [ExpectedApiRequiresUpdates] when both + * apply because the functions are disjoint. + */ + @Test + fun differentFunctionsDecidesActualApiHasProblems() { + val expectedApi = TomlZiplineApi( + listOf( + TomlZiplineService( + name = "com.example.EchoService", + functions = listOf( + TomlZiplineFunction( + "fun echo(kotlin.String): kotlin.String", + "fun echo(kotlin.String): kotlin.String".signatureHash(), + ), + ), + ), + ), + ) + + val actualApi = FirZiplineApi( + listOf( + FirZiplineService( + name = "com.example.EchoService", + functions = listOf( + FirZiplineFunction("fun echoTwo(kotlin.String): kotlin.String"), + ), + ), + ), + ) + + val decision = makeApiCompatibilityDecision(expectedApi, actualApi) + assertThat(decision).isInstanceOf() + } + + /** + * Although we have the function signature in both forms, only the function ID matters. This makes + * it possible to rename functions or their arguments without breaking compatibility. + * + * See https://github.com/cashapp/zipline/issues/1048 + */ + @Test + fun functionSignaturesMayDisagreeWithoutError() { + val expectedApi = TomlZiplineApi( + listOf( + TomlZiplineService( + name = "com.example.EchoService", + functions = listOf( + TomlZiplineFunction( + "fun echo(kotlin.String): kotlin.String", + "fun echo(kotlin.String): kotlin.String".signatureHash(), + ), + ), + ), + ), + ) + + val actualApi = FirZiplineApi( + listOf( + FirZiplineService( + name = "com.example.EchoService", + functions = listOf( + FirZiplineFunction( + "fun echo(kotlin.String): kotlin.String".signatureHash(), + "fun renamedFromEcho(kotlin.String): kotlin.String", // Renamed function! + ), + ), + ), + ), + ) + + val decision = makeApiCompatibilityDecision(expectedApi, actualApi) + assertThat(decision).isEqualTo(ExpectedApiIsUpToDate) + } +}