diff --git a/changelog/@unreleased/pr-1662.v2.yml b/changelog/@unreleased/pr-1662.v2.yml new file mode 100644 index 000000000..faff15c79 --- /dev/null +++ b/changelog/@unreleased/pr-1662.v2.yml @@ -0,0 +1,5 @@ +type: fix +fix: + description: Allow `ArtifactLocator`s to have inputs derived from task outputs. + links: + - https://github.com/palantir/sls-packaging/pull/1662 diff --git a/gradle-sls-packaging/src/main/java/com/palantir/gradle/dist/BaseDistributionExtension.java b/gradle-sls-packaging/src/main/java/com/palantir/gradle/dist/BaseDistributionExtension.java index 78af1c056..6f16cec54 100644 --- a/gradle-sls-packaging/src/main/java/com/palantir/gradle/dist/BaseDistributionExtension.java +++ b/gradle-sls-packaging/src/main/java/com/palantir/gradle/dist/BaseDistributionExtension.java @@ -80,7 +80,6 @@ public BaseDistributionExtension(Project project) { optionalProductDependencies = project.getObjects().setProperty(ProductId.class); ignoredProductDependencies = project.getObjects().setProperty(ProductId.class); artifacts = project.getObjects().domainObjectSet(ArtifactLocator.class); - artifacts.whenObjectAdded(ArtifactLocator::isValid); serviceGroup.set(project.provider(() -> project.getGroup().toString())); serviceName.set(project.provider(project::getName)); diff --git a/gradle-sls-packaging/src/main/java/com/palantir/gradle/dist/artifacts/ArtifactLocator.java b/gradle-sls-packaging/src/main/java/com/palantir/gradle/dist/artifacts/ArtifactLocator.java index f2fae7267..5d2c7b8d3 100644 --- a/gradle-sls-packaging/src/main/java/com/palantir/gradle/dist/artifacts/ArtifactLocator.java +++ b/gradle-sls-packaging/src/main/java/com/palantir/gradle/dist/artifacts/ArtifactLocator.java @@ -16,9 +16,6 @@ package com.palantir.gradle.dist.artifacts; -import com.google.common.base.Preconditions; -import com.palantir.logsafe.exceptions.SafeIllegalArgumentException; -import java.net.URI; import org.gradle.api.provider.Property; import org.gradle.api.tasks.Input; @@ -28,19 +25,4 @@ public interface ArtifactLocator { @Input Property getUri(); - - default void isValid() { - Preconditions.checkNotNull(getType().get(), "type must be specified"); - Preconditions.checkNotNull(getUri().get(), "uri must be specified"); - uriIsValid(getUri().get()); - } - - private static void uriIsValid(String uri) { - try { - // Throws IllegalArgumentException if URI does not conform to RFC 2396 - URI.create(uri); - } catch (IllegalArgumentException e) { - throw new SafeIllegalArgumentException("uri is not valid", e); - } - } } diff --git a/gradle-sls-packaging/src/main/java/com/palantir/gradle/dist/artifacts/JsonArtifactLocator.java b/gradle-sls-packaging/src/main/java/com/palantir/gradle/dist/artifacts/JsonArtifactLocator.java index 3abfe2c31..905cb4122 100644 --- a/gradle-sls-packaging/src/main/java/com/palantir/gradle/dist/artifacts/JsonArtifactLocator.java +++ b/gradle-sls-packaging/src/main/java/com/palantir/gradle/dist/artifacts/JsonArtifactLocator.java @@ -16,27 +16,47 @@ package com.palantir.gradle.dist.artifacts; -import com.fasterxml.jackson.annotation.JsonIgnoreProperties; -import com.fasterxml.jackson.annotation.JsonProperty; +import com.fasterxml.jackson.databind.annotation.JsonDeserialize; +import com.fasterxml.jackson.databind.annotation.JsonSerialize; +import java.net.URI; +import java.net.URISyntaxException; +import org.immutables.value.Value; -@JsonIgnoreProperties(ignoreUnknown = true) -public final class JsonArtifactLocator { +@Value.Immutable +@JsonDeserialize(as = ImmutableJsonArtifactLocator.class) +@JsonSerialize(as = ImmutableJsonArtifactLocator.class) +public interface JsonArtifactLocator { - @JsonProperty("type") - @SuppressWarnings("unused") - private String type; + String URI_IS_NOT_VALID_PREAMBLE = "uri is not valid: "; - @JsonProperty("uri") - @SuppressWarnings("unused") - private String uri; + String type(); - public JsonArtifactLocator(String type, String uri) { - this.type = type; - this.uri = uri; + String uri(); + + @Value.Check + default void isValid() { + try { + // Throws IllegalArgumentException if URI does not conform to RFC 2396 + URI.create(uri()); + } catch (IllegalArgumentException e) { + if (!(e.getCause() instanceof URISyntaxException)) { + throw new IllegalArgumentException("uri is invalid for some other reason", e); + } + + URISyntaxException cause = (URISyntaxException) e.getCause(); + int problemIndex = cause.getIndex(); + String highlight = + String.format(" %s^%s", "-".repeat(problemIndex), "-".repeat(uri().length() - (problemIndex + 1))); + throw new IllegalArgumentException( + String.format("%s\n'%s'\n%s", URI_IS_NOT_VALID_PREAMBLE, uri(), highlight), e); + } + } + + static JsonArtifactLocator from(ArtifactLocator artifactLocator) { + return from(artifactLocator.getType().get(), artifactLocator.getUri().get()); } - public static JsonArtifactLocator from(ArtifactLocator artifactLocator) { - return new JsonArtifactLocator( - artifactLocator.getType().get(), artifactLocator.getUri().get()); + static JsonArtifactLocator from(String type, String uri) { + return ImmutableJsonArtifactLocator.builder().type(type).uri(uri).build(); } } diff --git a/gradle-sls-packaging/src/main/java/com/palantir/gradle/dist/tasks/CreateManifestTask.java b/gradle-sls-packaging/src/main/java/com/palantir/gradle/dist/tasks/CreateManifestTask.java index 8a1412a12..5b56bb35a 100644 --- a/gradle-sls-packaging/src/main/java/com/palantir/gradle/dist/tasks/CreateManifestTask.java +++ b/gradle-sls-packaging/src/main/java/com/palantir/gradle/dist/tasks/CreateManifestTask.java @@ -60,6 +60,7 @@ import org.gradle.api.specs.Spec; import org.gradle.api.tasks.Input; import org.gradle.api.tasks.InputFile; +import org.gradle.api.tasks.Nested; import org.gradle.api.tasks.OutputFile; import org.gradle.api.tasks.TaskAction; import org.gradle.api.tasks.TaskProvider; @@ -84,7 +85,7 @@ public abstract class CreateManifestTask extends DefaultTask { @Input public abstract MapProperty getManifestExtensions(); - @Input + @Nested public abstract SetProperty getArtifacts(); @InputFile diff --git a/gradle-sls-packaging/src/test/groovy/com/palantir/gradle/dist/tasks/CreateManifestTaskIntegrationSpec.groovy b/gradle-sls-packaging/src/test/groovy/com/palantir/gradle/dist/tasks/CreateManifestTaskIntegrationSpec.groovy index 6d08e10f1..753b4b5cc 100644 --- a/gradle-sls-packaging/src/test/groovy/com/palantir/gradle/dist/tasks/CreateManifestTaskIntegrationSpec.groovy +++ b/gradle-sls-packaging/src/test/groovy/com/palantir/gradle/dist/tasks/CreateManifestTaskIntegrationSpec.groovy @@ -16,7 +16,10 @@ package com.palantir.gradle.dist.tasks +import com.fasterxml.jackson.core.type.TypeReference import com.palantir.gradle.dist.ObjectMappers +import com.palantir.gradle.dist.SlsManifest +import com.palantir.gradle.dist.artifacts.JsonArtifactLocator import com.palantir.gradle.dist.pdeps.ResolveProductDependenciesIntegrationSpec import nebula.test.IntegrationSpec import spock.lang.Unroll @@ -217,13 +220,75 @@ class CreateManifestTaskIntegrationSpec extends IntegrationSpec { then: buildResult.wasExecuted('createManifest') - def manifest = ObjectMappers.jsonMapper.readValue(file('build/deployment/manifest.yml').text, Map) - manifest.get("extensions").get("artifacts") == [ - [ - "type": "oci", - "uri" : "registry.example.io/foo/bar:v1.3.0" - ] - ] + readArtifactsExtension() == [JsonArtifactLocator.from("oci", "registry.example.io/foo/bar:v1.3.0")] + } + + def 'fails if invalid'() { + buildFile << """ + distribution { + artifact { + type = "oci" + uri = "registry.example[.io/foo/bar:v1.3.0" + } + } + """.stripIndent() + + when: + def buildResult = runTasksWithFailure('createManifest') + + then: + buildResult.failure.cause.cause.message.contains("uri is not valid") + } + + def "write artifacts to manifest from task output"() { + given: + buildFile << """ + + import org.gradle.api.file.RegularFileProperty + import org.gradle.api.provider.Provider + import org.gradle.api.tasks.OutputFile + import org.gradle.api.tasks.TaskAction + import java.nio.file.Files + + abstract class SomeTask extends DefaultTask { + @OutputFile + abstract RegularFileProperty getOutput(); + + final Provider artifactUri() { + return getOutput().getAsFile().map { + return Files.readString(it.toPath()) + } + } + + @TaskAction + final void action() throws Exception { + Files.writeString(getOutput().getAsFile().get().toPath(), "registry.example.io/foo/bar:v1.3.0") + } + } + + def artifactOutput = tasks.register('produceArtifactUrl', SomeTask.class) { + output.fileValue(file("build/artifact-url")) + } + + distribution { + artifact { + type = "oci" + uri = artifactOutput.flatMap { it.output }.map { Files.readString(it.getAsFile().toPath())} + } + } + """.stripIndent() + + when: + def buildResult = runTasksSuccessfully('createManifest') + println buildResult.standardOutput + + then: + readArtifactsExtension() == [JsonArtifactLocator.from("oci", "registry.example.io/foo/bar:v1.3.0")] + } + + private List readArtifactsExtension() { + def manifest = ObjectMappers.jsonMapper.readValue(file('build/deployment/manifest.yml').text, SlsManifest) + ObjectMappers.jsonMapper.convertValue(manifest.extensions().get("artifacts"), new TypeReference>() {}) } def "check depends on createManifest"() {