From 59887e6856932d00b7d451cd6bc51f143d4a61a1 Mon Sep 17 00:00:00 2001 From: Yogesh Ananda Nikam Date: Mon, 26 Aug 2024 12:25:28 +0530 Subject: [PATCH] Bring back the existing backwardCompatibilityCheck command Add deprecation notice to all the b/w compatibility related commands which will be eventually removed --- .../BackwardCompatibilityCheckCommand.kt | 329 ++++++++++++++++++ .../main/kotlin/application/CompareCommand.kt | 7 +- .../kotlin/application/CompatibleCommand.kt | 7 +- .../kotlin/application/DifferenceCommand.kt | 8 +- .../main/kotlin/application/PushCommand.kt | 13 +- .../kotlin/application/SpecmaticCommand.kt | 27 +- .../BackwardCompatibilityCheckBaseCommand.kt | 9 +- ...=> BackwardCompatibilityCheckCommandV2.kt} | 5 +- ...ackwardCompatibilityCheckCommandV2Test.kt} | 12 +- 9 files changed, 400 insertions(+), 17 deletions(-) create mode 100644 application/src/main/kotlin/application/BackwardCompatibilityCheckCommand.kt rename application/src/main/kotlin/application/backwardCompatibility/{BackwardCompatibilityCheckCommand.kt => BackwardCompatibilityCheckCommandV2.kt} (96%) rename application/src/test/kotlin/application/{BackwardCompatibilityCheckCommandTest.kt => BackwardCompatibilityCheckCommandV2Test.kt} (88%) diff --git a/application/src/main/kotlin/application/BackwardCompatibilityCheckCommand.kt b/application/src/main/kotlin/application/BackwardCompatibilityCheckCommand.kt new file mode 100644 index 000000000..6b4fbbf0d --- /dev/null +++ b/application/src/main/kotlin/application/BackwardCompatibilityCheckCommand.kt @@ -0,0 +1,329 @@ +package application + +import application.BackwardCompatibilityCheckCommand.CompatibilityResult.* +import io.specmatic.conversions.OpenApiSpecification +import io.specmatic.core.* +import io.specmatic.core.git.GitCommand +import io.specmatic.core.git.SystemGit +import io.specmatic.core.log.logger +import io.specmatic.core.utilities.exitWithMessage +import io.specmatic.stub.isOpenAPI +import org.springframework.stereotype.Component +import picocli.CommandLine.Command +import java.io.File +import java.nio.file.Files +import java.nio.file.Path +import java.nio.file.Paths +import java.util.concurrent.Callable +import java.util.regex.Pattern +import kotlin.io.path.extension +import kotlin.io.path.pathString +import kotlin.system.exitProcess + +const val ONE_INDENT = " " +const val TWO_INDENTS = "${ONE_INDENT}${ONE_INDENT}" + +@Component +@Command( + name = "backwardCompatibilityCheck", + mixinStandardHelpOptions = true, + description = [ +""" +Checks backward compatibility of a directory across the current HEAD and the main branch. +DEPRECATED: This command will be removed in the next major release. Use 'backward-compatibility-check' command instead. +""" + ] +) +class BackwardCompatibilityCheckCommand( + private val gitCommand: GitCommand = SystemGit(), +) : Callable { + + private val newLine = System.lineSeparator() + + companion object { + private const val HEAD = "HEAD" + private const val MARGIN_SPACE = " " + } + + override fun call() { + val filesChangedInCurrentBranch: Set = getOpenAPISpecFilesChangedInCurrentBranch() + + if (filesChangedInCurrentBranch.isEmpty()) exitWithMessage("${newLine}No OpenAPI spec files were changed, skipping the check.$newLine") + + val filesReferringToChangedSchemaFiles = filesReferringToChangedSchemaFiles(filesChangedInCurrentBranch) + + val specificationsOfChangedExternalisedExamples: Set = getSpecificationsOfChangedExternalisedExamples(filesChangedInCurrentBranch) + + logFilesToBeCheckedForBackwardCompatibility( + filesChangedInCurrentBranch, + filesReferringToChangedSchemaFiles, + specificationsOfChangedExternalisedExamples + ) + + val specificationsToCheck: Set = filesChangedInCurrentBranch + filesReferringToChangedSchemaFiles + specificationsOfChangedExternalisedExamples + + val result = try { + runBackwardCompatibilityCheckFor(specificationsToCheck) + } catch(e: Throwable) { + logger.newLine() + logger.newLine() + logger.log(e) + exitProcess(1) + } + + println() + println(result.report) + exitProcess(result.exitCode) + } + + private fun getSpecificationsOfChangedExternalisedExamples(filesChangedInCurrentBranch: Set): Set { + data class CollectedFiles( + val specifications: MutableSet = mutableSetOf(), + val examplesMissingSpecifications: MutableList = mutableListOf(), + val ignoredFiles: MutableList = mutableListOf() + ) + + val collectedFiles = filesChangedInCurrentBranch.fold(CollectedFiles()) { acc, filePath -> + val path = Paths.get(filePath) + val examplesDir = path.find { it.toString().endsWith("_examples") || it.toString().endsWith("_tests") } + + if (examplesDir == null) { + acc.ignoredFiles.add(filePath) + } else { + val parentPath = examplesDir.parent + val strippedPath = parentPath.resolve(examplesDir.fileName.toString().removeSuffix("_examples")) + val specFiles = findSpecFiles(strippedPath) + + if (specFiles.isNotEmpty()) { + acc.specifications.addAll(specFiles.map { it.toString() }) + } else { + acc.examplesMissingSpecifications.add(filePath) + } + } + acc + } + + val result = collectedFiles.specifications.toMutableSet() + + collectedFiles.examplesMissingSpecifications.forEach { filePath -> + val path = Paths.get(filePath) + val examplesDir = path.find { it.toString().endsWith("_examples") || it.toString().endsWith("_tests") } + if (examplesDir != null) { + val parentPath = examplesDir.parent + val strippedPath = parentPath.resolve(examplesDir.fileName.toString().removeSuffix("_examples")) + val specFiles = findSpecFiles(strippedPath) + if (specFiles.isNotEmpty()) { + result.addAll(specFiles.map { it.toString() }) + } else { + result.add("${strippedPath}.yaml") + } + } + } + + return result + } + + private fun Path.find(predicate: (Path) -> Boolean): Path? { + var current: Path? = this + while (current != null) { + if (predicate(current)) { + return current + } + current = current.parent + } + return null + } + + private fun findSpecFiles(path: Path): List { + val extensions = CONTRACT_EXTENSIONS + return extensions.map { path.resolveSibling(path.fileName.toString() + it) } + .filter { Files.exists(it) && (isOpenAPI(it.pathString) || it.extension in listOf(WSDL, CONTRACT_EXTENSION)) } + } + + private fun runBackwardCompatibilityCheckFor(files: Set): CompatibilityReport { + val branchWithChanges = gitCommand.currentBranch() + val treeishWithChanges = if (branchWithChanges == HEAD) gitCommand.detachedHEAD() else branchWithChanges + + try { + val results = files.mapIndexed { index, specFilePath -> + try { + println("${index.inc()}. Running the check for $specFilePath:") + + // newer => the file with changes on the branch + val (newer, unusedExamples) = OpenApiSpecification.fromFile(specFilePath).toFeature().loadExternalisedExamplesAndListUnloadableExamples() + + val olderFile = gitCommand.getFileInTheBaseBranch( + specFilePath, + treeishWithChanges, + gitCommand.defaultBranch() + ) + if (olderFile == null) { + println("$specFilePath is a new file.$newLine") + return@mapIndexed PASSED + } + + gitCommand.checkout(gitCommand.defaultBranch()) + + // older => the same file on the default (e.g. main) branch + val older = OpenApiSpecification.fromFile(olderFile.path).toFeature() + + val backwardCompatibilityResult = testBackwardCompatibility(older, newer) + + if (backwardCompatibilityResult.success()) { + println( + "$newLine The file $specFilePath is backward compatible.$newLine".prependIndent( + MARGIN_SPACE + ) + ) + + println() + + var errorsFound = false + + if(!examplesAreValid(newer, "newer")) { + println( + "$newLine *** Examples in $specFilePath are not valid. ***$newLine".prependIndent( + MARGIN_SPACE + ) + ) + + println() + + errorsFound = true + } + + if(unusedExamples.isNotEmpty()) { + println( + "$newLine *** Some examples for $specFilePath could not be loaded. ***$newLine".prependIndent( + MARGIN_SPACE + ) + ) + + println() + + errorsFound = true + + } + + if(errorsFound) { + FAILED + } + else + PASSED + } else { + println("$newLine ${backwardCompatibilityResult.report().prependIndent(MARGIN_SPACE)}") + println( + "$newLine *** The file $specFilePath is NOT backward compatible. ***$newLine".prependIndent( + MARGIN_SPACE + ) + ) + + println() + + FAILED + } + } finally { + gitCommand.checkout(treeishWithChanges) + } + } + + return CompatibilityReport(results) + } finally { + gitCommand.checkout(treeishWithChanges) + } + } + + private fun examplesAreValid(feature: Feature, which: String): Boolean { + return try { + feature.validateExamplesOrException() + true + } catch (t: Throwable) { + println() + false + } + } + + private fun logFilesToBeCheckedForBackwardCompatibility( + changedFiles: Set, + filesReferringToChangedFiles: Set, + specificationsOfChangedExternalisedExamples: Set + ) { + + println("Checking backward compatibility of the following files: $newLine") + println("${ONE_INDENT}Files that have changed:") + changedFiles.forEach { println(it.prependIndent(TWO_INDENTS)) } + println() + + if(filesReferringToChangedFiles.isNotEmpty()) { + println("${ONE_INDENT}Files referring to the changed files - ") + filesReferringToChangedFiles.forEach { println(it.prependIndent(TWO_INDENTS)) } + println() + } + + if(specificationsOfChangedExternalisedExamples.isNotEmpty()) { + println("${ONE_INDENT}Specifications whose externalised examples were changed - ") + filesReferringToChangedFiles.forEach { println(it.prependIndent(TWO_INDENTS)) } + println() + } + + println("-".repeat(20)) + println() + } + + internal fun filesReferringToChangedSchemaFiles(inputFiles: Set): Set { + if (inputFiles.isEmpty()) return emptySet() + + val inputFileNames = inputFiles.map { File(it).name } + val result = allOpenApiSpecFiles().filter { + it.readText().trim().let { specContent -> + inputFileNames.any { inputFileName -> + val pattern = Pattern.compile("\\b$inputFileName\\b") + val matcher = pattern.matcher(specContent) + matcher.find() + } + } + }.map { it.path }.toSet() + + return result.flatMap { + filesReferringToChangedSchemaFiles(setOf(it)).ifEmpty { setOf(it) } + }.toSet() + } + + internal fun allOpenApiSpecFiles(): List { + return File(".").walk().toList().filterNot { + ".git" in it.path + }.filter { it.isFile && it.isOpenApiSpec() } + } + + private fun getOpenAPISpecFilesChangedInCurrentBranch(): Set { + return gitCommand.getFilesChangedInCurrentBranch( + gitCommand.defaultBranch() + ).filter { + File(it).exists() && File(it).isOpenApiSpec() + }.toSet() + } + + private fun File.isOpenApiSpec(): Boolean { + if (this.extension !in CONTRACT_EXTENSIONS) return false + return OpenApiSpecification.isParsable(this.path) + } + + class CompatibilityReport(results: List) { + val report: String + val exitCode: Int + + init { + val failed: Boolean = results.any { it == FAILED } + val failedCount = results.count { it == FAILED } + val passedCount = results.count { it == PASSED } + + report = "Files checked: ${results.size} (Passed: ${passedCount}, Failed: $failedCount)" + exitCode = if(failed) 1 else 0 + } + + } + + enum class CompatibilityResult { + PASSED, FAILED + } +} diff --git a/application/src/main/kotlin/application/CompareCommand.kt b/application/src/main/kotlin/application/CompareCommand.kt index bdb0b7dbc..61918d226 100644 --- a/application/src/main/kotlin/application/CompareCommand.kt +++ b/application/src/main/kotlin/application/CompareCommand.kt @@ -12,7 +12,12 @@ import kotlin.system.exitProcess @Command(name = "compare", mixinStandardHelpOptions = true, - description = ["Checks if two contracts are equivalent"]) + description = [ +""" +Checks if two contracts are equivalent. +DEPRECATED: This command will be removed in the next major release. Use 'backward-compatibility-check' command instead. +""" + ]) class CompareCommand : Callable { @Parameters(index = "0", description = ["Older contract file path"]) lateinit var olderContractFilePath: String diff --git a/application/src/main/kotlin/application/CompatibleCommand.kt b/application/src/main/kotlin/application/CompatibleCommand.kt index e5c32dd27..234e28546 100644 --- a/application/src/main/kotlin/application/CompatibleCommand.kt +++ b/application/src/main/kotlin/application/CompatibleCommand.kt @@ -180,7 +180,12 @@ class GitCompatibleCommand : Callable { @Command(name = "compatible", mixinStandardHelpOptions = true, - description = ["Checks if the newer contract is backward compatible with the older one"], + description = [ +""" +Checks if the newer contract is backward compatible with the older one +DEPRECATED: This command will be removed in the next major release. Use 'backward-compatibility-check' command instead. +""" + ], subcommands = [ GitCompatibleCommand::class ]) internal class CompatibleCommand : Callable { override fun call() { diff --git a/application/src/main/kotlin/application/DifferenceCommand.kt b/application/src/main/kotlin/application/DifferenceCommand.kt index 1effdcc73..e6ce1ee61 100644 --- a/application/src/main/kotlin/application/DifferenceCommand.kt +++ b/application/src/main/kotlin/application/DifferenceCommand.kt @@ -11,7 +11,13 @@ import kotlin.system.exitProcess @Command(name = "similar", mixinStandardHelpOptions = true, - description = ["Show the difference between two contracts"]) + description = [ +""" +Show the difference between two contracts. +DEPRECATED: This command will be removed in the next major release. Use 'backward-compatibility-check' command instead. +""" + ] +) class DifferenceCommand : Callable { @Parameters(index = "0", description = ["Older contract file path"]) lateinit var olderContractFilePath: String diff --git a/application/src/main/kotlin/application/PushCommand.kt b/application/src/main/kotlin/application/PushCommand.kt index 9293a631f..771882d46 100644 --- a/application/src/main/kotlin/application/PushCommand.kt +++ b/application/src/main/kotlin/application/PushCommand.kt @@ -18,7 +18,16 @@ import kotlin.system.exitProcess private const val pipelineKeyInSpecmaticConfig = "pipeline" -@CommandLine.Command(name = "push", description = ["Check the new contract for backward compatibility with the specified version, then overwrite the old one with it."], mixinStandardHelpOptions = true) +@CommandLine.Command( + name = "push", + description = [ +""" +Check the new contract for backward compatibility with the specified version, then overwrite the old one with it. +DEPRECATED: This command will be removed in the next major release. Use 'backward-compatibility-check' command instead. +""" + ], + mixinStandardHelpOptions = true +) class PushCommand: Callable { override fun call() { val userHome = File(System.getProperty("user.home")) @@ -160,4 +169,4 @@ fun registerPipelineCredentials(manifestData: JSONObjectValue, contractPath: Str sourceGit.add() } } -} \ No newline at end of file +} diff --git a/application/src/main/kotlin/application/SpecmaticCommand.kt b/application/src/main/kotlin/application/SpecmaticCommand.kt index 170d188e1..d6591fe32 100644 --- a/application/src/main/kotlin/application/SpecmaticCommand.kt +++ b/application/src/main/kotlin/application/SpecmaticCommand.kt @@ -1,6 +1,6 @@ package application -import application.backwardCompatibility.BackwardCompatibilityCheckCommand +import application.backwardCompatibility.BackwardCompatibilityCheckCommandV2 import org.springframework.stereotype.Component import picocli.AutoComplete.GenerateCompletion import picocli.CommandLine.Command @@ -11,7 +11,30 @@ import java.util.concurrent.Callable name = "specmatic", mixinStandardHelpOptions = true, versionProvider = VersionProvider::class, - subcommands = [BackwardCompatibilityCheckCommand::class, BundleCommand::class, CompareCommand::class, CompatibleCommand::class, DifferenceCommand::class, GenerateCompletion::class, GraphCommand::class, MergeCommand::class, ToOpenAPICommand::class, ImportCommand::class, InstallCommand::class, ProxyCommand::class, PushCommand::class, ReDeclaredAPICommand::class, ExamplesCommand::class, SamplesCommand::class, StubCommand::class, SubscribeCommand::class, TestCommand::class, ValidateViaLogs::class, CentralContractRepoReportCommand::class] + subcommands = [ + BackwardCompatibilityCheckCommandV2::class, + BackwardCompatibilityCheckCommand::class, + BundleCommand::class, + CompareCommand::class, + CompatibleCommand::class, + DifferenceCommand::class, + GenerateCompletion::class, + GraphCommand::class, + MergeCommand::class, + ToOpenAPICommand::class, + ImportCommand::class, + InstallCommand::class, + ProxyCommand::class, + PushCommand::class, + ReDeclaredAPICommand::class, + ExamplesCommand::class, + SamplesCommand::class, + StubCommand::class, + SubscribeCommand::class, + TestCommand::class, + ValidateViaLogs::class, + CentralContractRepoReportCommand::class + ] ) class SpecmaticCommand : Callable { override fun call(): Int { diff --git a/application/src/main/kotlin/application/backwardCompatibility/BackwardCompatibilityCheckBaseCommand.kt b/application/src/main/kotlin/application/backwardCompatibility/BackwardCompatibilityCheckBaseCommand.kt index 8b3c330b3..11241eeb7 100644 --- a/application/src/main/kotlin/application/backwardCompatibility/BackwardCompatibilityCheckBaseCommand.kt +++ b/application/src/main/kotlin/application/backwardCompatibility/BackwardCompatibilityCheckBaseCommand.kt @@ -16,7 +16,14 @@ abstract class BackwardCompatibilityCheckBaseCommand : Callable { private val gitCommand: GitCommand = SystemGit() private val newLine = System.lineSeparator() - @Option(names = ["--base-branch"], description = ["Base branch to compare the changes against"], required = false) + @Option( + names = ["--base-branch"], + description = [ + "Base branch to compare the changes against", + "Default value is the local origin HEAD of the current branch" + ], + required = false + ) var baseBranch: String? = null @Option(names = ["--target-path"], description = ["Specification file or folder to run the check against"], required = false) diff --git a/application/src/main/kotlin/application/backwardCompatibility/BackwardCompatibilityCheckCommand.kt b/application/src/main/kotlin/application/backwardCompatibility/BackwardCompatibilityCheckCommandV2.kt similarity index 96% rename from application/src/main/kotlin/application/backwardCompatibility/BackwardCompatibilityCheckCommand.kt rename to application/src/main/kotlin/application/backwardCompatibility/BackwardCompatibilityCheckCommandV2.kt index 29914abe4..4c59f7244 100644 --- a/application/src/main/kotlin/application/backwardCompatibility/BackwardCompatibilityCheckCommand.kt +++ b/application/src/main/kotlin/application/backwardCompatibility/BackwardCompatibilityCheckCommandV2.kt @@ -20,12 +20,11 @@ import kotlin.io.path.pathString @Component @Command( - name = "backwardCompatibilityCheck", - aliases = ["backward-compatibility-check"], + name = "backward-compatibility-check", mixinStandardHelpOptions = true, description = ["Checks backward compatibility of OpenAPI specifications"] ) -class BackwardCompatibilityCheckCommand: BackwardCompatibilityCheckBaseCommand() { +class BackwardCompatibilityCheckCommandV2: BackwardCompatibilityCheckBaseCommand() { override fun checkBackwardCompatibility(oldFeature: IFeature, newFeature: IFeature): Results { return testBackwardCompatibility(oldFeature as Feature, newFeature as Feature) diff --git a/application/src/test/kotlin/application/BackwardCompatibilityCheckCommandTest.kt b/application/src/test/kotlin/application/BackwardCompatibilityCheckCommandV2Test.kt similarity index 88% rename from application/src/test/kotlin/application/BackwardCompatibilityCheckCommandTest.kt rename to application/src/test/kotlin/application/BackwardCompatibilityCheckCommandV2Test.kt index 6873da05c..18fd33bf7 100644 --- a/application/src/test/kotlin/application/BackwardCompatibilityCheckCommandTest.kt +++ b/application/src/test/kotlin/application/BackwardCompatibilityCheckCommandV2Test.kt @@ -1,6 +1,6 @@ package application -import application.backwardCompatibility.BackwardCompatibilityCheckCommand +import application.backwardCompatibility.BackwardCompatibilityCheckCommandV2 import io.mockk.every import io.mockk.spyk import org.junit.jupiter.api.AfterEach @@ -9,18 +9,18 @@ import org.junit.jupiter.api.Assertions.assertTrue import org.junit.jupiter.api.Test import java.io.File -class BackwardCompatibilityCheckCommandTest { +class BackwardCompatibilityCheckCommandV2Test { @Test fun `getSpecsReferringTo returns empty set when input is empty`() { - val command = BackwardCompatibilityCheckCommand() + val command = BackwardCompatibilityCheckCommandV2() val result = command.getSpecsReferringTo(emptySet()) assertTrue(result.isEmpty()) } @Test fun `getSpecsReferringTo returns empty set when no files refer to changed schema files`() { - val command = spyk() + val command = spyk() every { command.allSpecFiles() } returns listOf( File("file1.yaml").apply { writeText("content1") }, File("file2.yaml").apply { writeText("content2") } @@ -31,7 +31,7 @@ class BackwardCompatibilityCheckCommandTest { @Test fun `getSpecsReferringTo returns set of files that refer to changed schema files`() { - val command = spyk() + val command = spyk() every { command.allSpecFiles() } returns listOf( File("file1.yaml").apply { writeText("file3.yaml") }, File("file2.yaml").apply { writeText("file4.yaml") } @@ -42,7 +42,7 @@ class BackwardCompatibilityCheckCommandTest { @Test fun `getSpecsReferringTo returns set of files which are referring to a changed schema that is one level down`() { - val command = spyk() + val command = spyk() every { command.allSpecFiles() } returns listOf( File("file1.yaml").apply { referTo("schema_file1.yaml") }, File("schema_file2.yaml").apply { referTo("schema_file1.yaml") }, // schema within a schema