Skip to content

Commit

Permalink
Replace custom caching with Gradle's incremental build
Browse files Browse the repository at this point in the history 
Replaced custom caching of the downloaded contracts with Gradle's
incremental build. After that `InitContractsTask` is not needed any
more, as its main purpose was to be a bridge between custom caching
and Gradle's one. Simplified code to download contracts is now part of
the `ContractsCopyTask`.
Also fixed `PublishStubsToScmTask` and removed incremental stuff from
there for now.

Fixes spring-cloudgh-1133
  • Loading branch information
Anatolii Balakiriev committed Aug 27, 2019
1 parent e54fe37 commit 7972177
Show file tree
Hide file tree
Showing 11 changed files with 388 additions and 576 deletions.
15 changes: 10 additions & 5 deletions ...roovy/org/springframework/cloud/contract/verifier/plugin/ContractVerifierExtension.groovy
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@

package org.springframework.cloud.contract.verifier.plugin

import org.gradle.api.tasks.Internal

import javax.inject.Inject

import groovy.transform.CompileStatic
Expand Down Expand Up @@ -517,10 +519,11 @@ class ContractVerifierExtension {
@Optional
Property<String> proxyHost
/**
* If set to true then will cache the folder where non snapshot contract artifacts got downloaded.
* Not used any more, as we switched to Gradle's incremental build.
*/
@Input
Property<Boolean> cacheDownloadedContracts
@Internal
@Deprecated
boolean cacheDownloadedContracts

@Inject
ContractRepository(ObjectFactory objects) {
Expand All @@ -529,7 +532,6 @@ class ContractVerifierExtension {
this.password = objects.property(String)
this.proxyHost = objects.property(String)
this.proxyPort = objects.property(Integer)
this.cacheDownloadedContracts = objects.property(Boolean).convention(true)
}

@Override
Expand All @@ -540,7 +542,6 @@ class ContractVerifierExtension {
", password=" + password.getOrNull() +
", proxyPort=" + proxyPort.getOrNull() +
", proxyHost=" + proxyHost.getOrNull() +
", cacheDownloadedContracts=" + cacheDownloadedContracts.get() +
'}'
}

Expand Down Expand Up @@ -575,5 +576,9 @@ class ContractVerifierExtension {
void setProxyHost(GString proxyHost) {
this.proxyHost.set(proxyHost.toString())
}

void setProxyPort(Integer proxyPort) {
this.proxyPort.set(proxyPort)
}
}
}
151 changes: 132 additions & 19 deletions ...c/main/groovy/org/springframework/cloud/contract/verifier/plugin/ContractsCopyTask.groovy
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ package org.springframework.cloud.contract.verifier.plugin


import groovy.transform.CompileStatic
import groovy.transform.Immutable
import groovy.transform.ImmutableOptions
import groovy.transform.PackageScope
import org.gradle.api.Action
import org.gradle.api.DefaultTask
Expand All @@ -26,18 +28,28 @@ import org.gradle.api.Project
import org.gradle.api.file.CopySpec
import org.gradle.api.file.Directory
import org.gradle.api.file.DirectoryProperty
import org.gradle.api.logging.Logger
import org.gradle.api.provider.MapProperty
import org.gradle.api.provider.Property
import org.gradle.api.provider.Provider
import org.gradle.api.tasks.Input
import org.gradle.api.tasks.InputDirectory
import org.gradle.api.tasks.Internal
import org.gradle.api.tasks.Nested
import org.gradle.api.tasks.Optional
import org.gradle.api.tasks.OutputDirectory
import org.gradle.api.tasks.TaskAction
import org.gradle.api.tasks.TaskProvider
import org.gradle.api.tasks.WorkResult

import org.springframework.cloud.contract.stubrunner.ContractDownloader
import org.springframework.cloud.contract.stubrunner.StubConfiguration
import org.springframework.cloud.contract.stubrunner.StubDownloader
import org.springframework.cloud.contract.stubrunner.StubDownloaderBuilderProvider
import org.springframework.cloud.contract.stubrunner.spring.StubRunnerProperties
import org.springframework.cloud.contract.verifier.converter.ToYamlConverter
import org.springframework.util.StringUtils

import java.time.Instant

// TODO: Convert to incremental task: https://docs.gradle.org/current/userguide/custom_tasks.html#incremental_tasks
/**
* Task that copies the contracts in order for the jar task to
Expand Down Expand Up @@ -65,12 +77,54 @@ class ContractsCopyTask extends DefaultTask {
Provider<Boolean> excludeBuildFolders
@Input
Provider<Boolean> failOnNoContracts
@Input
Provider<String> includedRootFolderAntPattern
@InputDirectory
Provider<Directory> contractsDirectory
// All fields inside `@Nested` one are properly marked as an `@Input` to work with incremental build:
@Nested
@Optional
ContractVerifierExtension.Dependency contractDependency
@Nested
@Optional
ContractVerifierExtension.ContractRepository contractRepository
@Input
@Optional
Property<StubRunnerProperties.StubsMode> contractsMode
@Input
Property<Boolean> deleteStubsAfterTest
@Input
Provider<String> contractsRepository
MapProperty<String, String> contractsProperties
@Input
@Optional
Property<String> contractsPath
@Input
@Optional
Instant getForceDownloadOfTheLatestContracts() {
// If we have `dynamic` version (`+` or `SNAPSHOT`) - we should mark this task as out of date for every run:
if (shouldDownloadContracts() && getStubConfiguration().isVersionChanging()) {
return Instant.now() // This will trigger re-download of contracts
} else {
return null // This will not trigger re-download of contracts
}
}
@Optional
@InputDirectory
Provider<Directory> getContractsDirectory() {
if (shouldDownloadContracts()) {
return null
} else {
return contractsDirectory
}
}

@Internal
boolean shouldDownloadContracts() {
return StringUtils.hasText(contractDependency.getArtifactId().getOrNull()) ||
StringUtils.hasText(contractDependency.getStringNotation().getOrNull()) ||
StringUtils.hasText(contractRepository.repositoryUrl.getOrNull())
}
@Internal
StubConfiguration getStubConfiguration() {
return GradleContractsDownloaderHelper.stubConfiguration(contractDependency)
}

@OutputDirectory
DirectoryProperty copiedContractsFolder
Expand All @@ -81,30 +135,44 @@ class ContractsCopyTask extends DefaultTask {

@TaskAction
void sync() {
File contractsDirectory = config.contractsDirectory.get().asFile
String contractsRepository = config.contractsRepository.get()
final DownloadedData downloadedData = downloadContractsIfNeeded()
final File contractsDirectory
final String antPattern
if (downloadedData) {
contractsDirectory = downloadedData.downloadedContracts
antPattern = "${downloadedData.inclusionProperties.includedRootFolderAntPattern}*.*"
} else {
contractsDirectory = config.contractsDirectory.get().asFile
antPattern = "**/"
}
logger.info("For project [{}] will use contracts provided in the folder [{}]", project.name, contractsDirectory)
final String contractsRepository = config.contractRepository.repositoryUrl.isPresent() ? config.contractRepository.repositoryUrl : ""
throwExceptionWhenFailOnNoContracts(contractsDirectory, contractsRepository)
String antPattern = "${config.includedRootFolderAntPattern.get()}*.*"
String slashSeparatedGroupId = project.group.toString().replace(".", File.separator)
String slashSeparatedAntPattern = antPattern.replace(slashSeparatedGroupId, project.group.toString())
File output = config.copiedContractsFolder.get().asFile

final String slashSeparatedGroupId = project.group.toString().replace(".", File.separator)
final String slashSeparatedAntPattern = antPattern.replace(slashSeparatedGroupId, project.group.toString())
final File output = config.copiedContractsFolder.get().asFile
logger.info("Downloading and unpacking files from [${contractsDirectory}] to [$output]. The inclusion ant patterns are [${antPattern}] and [${slashSeparatedAntPattern}]")
sync(contractsDirectory, antPattern, slashSeparatedAntPattern, config.excludeBuildFolders.get(), output)
if (config.convertToYaml.get()) {
convertBackedUpDslsToYaml(contractsDirectory, antPattern, slashSeparatedAntPattern, output, config.excludeBuildFolders.get())
}
}

static Config fromExtension(ContractVerifierExtension extension, TaskProvider<InitContractsTask> initContractsTask, String root, Project project) {
static Config fromExtension(ContractVerifierExtension extension, String root, Project project) {
return new Config(
convertToYaml: extension.convertToYaml,
excludeBuildFolders: extension.excludeBuildFolders,
failOnNoContracts: extension.failOnNoContracts,
includedRootFolderAntPattern: initContractsTask.flatMap { it.config.includedRootFolderAntPattern },
contractsDirectory: initContractsTask.flatMap { it.config.initialisedContractsDirectory },
contractsRepository: extension.contractRepository.repositoryUrl.isPresent() ? extension.contractRepository.repositoryUrl : project.provider({ String s -> "" }),
contractsDirectory: extension.contractsDslDir,
copiedContractsFolder: createTaskOutput(root, extension.stubsOutputDir, ContractsCopyTask.CONTRACTS, project),
backupContractsFolder: createTaskOutput(root, extension.stubsOutputDir, ContractsCopyTask.BACKUP, project)
backupContractsFolder: createTaskOutput(root, extension.stubsOutputDir, ContractsCopyTask.BACKUP, project),
contractDependency: extension.contractDependency,
contractRepository: extension.contractRepository,
contractsMode: extension.contractsMode,
deleteStubsAfterTest: extension.deleteStubsAfterTest,
contractsProperties: extension.contractsProperties,
contractsPath: extension.contractsPath
)
}

Expand All @@ -115,8 +183,6 @@ class ContractsCopyTask extends DefaultTask {
}

protected WorkResult sync(File file, String antPattern, String slashSeparatedAntPattern, boolean excludeBuildFolders, File outputContractsFolder) {
// TODO: Is there any better way to make it statically compiled, avoiding explicit creation of new Action?
// sync will remove files from target if they are removed from source. So using it here instead of copy:
return project.sync(new Action<CopySpec>() {
@Override
void execute(final CopySpec spec) {
Expand All @@ -135,6 +201,35 @@ class ContractsCopyTask extends DefaultTask {
})
}

private DownloadedData downloadContractsIfNeeded() {
if (config.shouldDownloadContracts()) {
logger.info("Project has group id [{}], artifact id [{}]", project.group, project.name)
logger.info("For project [${project.name}] Download dependency is provided - will download contract jars")
logger.info("Contract dependency [{}]", config.contractDependency)
StubConfiguration configuration = config.getStubConfiguration()
logger.info("Got the following contract dependency to download [{}]", configuration)
logger.info("The contract dependency is a changing one [{}]", configuration.isVersionChanging())

final StubDownloader downloader = new StubDownloaderBuilderProvider().get(
StubRunnerOptionsFactory.createStubRunnerOptions(config.contractRepository,
config.contractsMode.getOrNull(), config.deleteStubsAfterTest.get(),
config.contractsProperties.get(), config.failOnNoContracts.get()))
final ContractDownloader contractDownloader = new ContractDownloader(downloader, configuration,
config.contractsPath.getOrNull(), project.group as String, project.name, project.version as String)
final File downloadedContracts = contractDownloader.unpackAndDownloadContracts();
final ContractDownloader.InclusionProperties inclusionProperties =
contractDownloader.createNewInclusionProperties(downloadedContracts)

// TODO: inclusionProperties.includedContracts is never used eventually. Review this:
return new DownloadedData(
downloadedContracts: contractsSubDirIfPresent(downloadedContracts, logger),
inclusionProperties: inclusionProperties
)
} else {
return null
}
}

private static DirectoryProperty createTaskOutput(String root, DirectoryProperty stubsOutputDir, String suffix, Project project) {
Provider<Directory> provider = stubsOutputDir.flatMap {
Directory dir = it
Expand Down Expand Up @@ -162,4 +257,22 @@ class ContractsCopyTask extends DefaultTask {
+ "] .\nPlease make sure that the contracts were defined, or set the [failOnNoContracts] flag to [false]")
}
}

private static File contractsSubDirIfPresent(File contractsDirectory, Logger logger) {
File contracts = new File(contractsDirectory, "contracts")
if (contracts.exists()) {
if (logger.isDebugEnabled()) {
logger.debug("Contracts folder found [" + contracts + "]")
}
contractsDirectory = contracts
}
return contractsDirectory
}

@ImmutableOptions(knownImmutableClasses = [File, ContractDownloader.InclusionProperties])
@Immutable
private static class DownloadedData {
final File downloadedContracts
final ContractDownloader.InclusionProperties inclusionProperties
}
}
12 changes: 1 addition & 11 deletions .../groovy/org/springframework/cloud/contract/verifier/plugin/GenerateServerTestsTask.groovy
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,6 @@ class GenerateServerTestsTask extends DefaultTask {
@InputDirectory
Provider<Directory> contractsDslDir
@Input
Provider<String> includedContracts
@Input
@Optional
Provider<String> nameSuffixForTests
@Input
Expand Down Expand Up @@ -101,12 +99,6 @@ class GenerateServerTestsTask extends DefaultTask {
logger.info("Generated test sources dir [${generatedTestSources}]")
logger.info("Generated test resources dir [${generatedTestResources}]")
File contractsDslDir = config.contractsDslDir.get().asFile
// There used to be some bug in old code, which was always setting `includedContracts` to ".*" instead of
// getting it from the config (which is coming from contracts downloader). In fact, that `correct` behaviour
// doesn't even make sense here as contracts are already copied, so that inclusion filter will not include
// anything. So for now just restoring this old behaviour, but it must be reviewed. Probably that
// `includedContracts` should be used in the `copyContracts` task instead?
// String includedContracts = config.includedContracts.get()
String includedContracts = ".*"
project.logger.info("Spring Cloud Contract Verifier Plugin: Invoking test sources generation")
project.logger.info("Contracts are unpacked to [${contractsDslDir}]")
Expand Down Expand Up @@ -145,11 +137,9 @@ class GenerateServerTestsTask extends DefaultTask {
}
}

static Config fromExtension(ContractVerifierExtension extension, TaskProvider<InitContractsTask> initContractsTask,
TaskProvider<ContractsCopyTask> copyContractsTask) {
static Config fromExtension(ContractVerifierExtension extension, TaskProvider<ContractsCopyTask> copyContractsTask) {
return new Config(
contractsDslDir: copyContractsTask.flatMap { it.config.copiedContractsFolder },
includedContracts: initContractsTask.flatMap { it.config.includedContracts },
nameSuffixForTests: extension.nameSuffixForTests,
basePackageForTests: extension.basePackageForTests,
baseClassForTests: extension.baseClassForTests,
Expand Down
Loading

0 comments on commit 7972177

Please sign in to comment.