From da5c35b97dc568f51d25bc9628d1c988d428beeb Mon Sep 17 00:00:00 2001 From: Nicolas Favre-Felix Date: Tue, 9 Jan 2024 16:36:48 -0800 Subject: [PATCH] fix(base64): Base64 artifacts not utilizing encode flag This commit addresses an issue where Base64ArtifactDefinition was not properly utilizing the `shouldEncode` method/field. This was mainly due to having multiple `@Builder`s, and also `Boolean`s defaulting to false. To fix this we simply add a `null` check against the new flag that was added * update(gitignore): add some appropriate gitignore options * idea folder * lombok.config Authored by: benjamin-j-powell --- .gitignore | 2 + .../artifacts/Base64ArtifactDefinition.java | 7 +-- .../EmptyCustomArtifactDefinition.java | 4 +- .../Base64ArtifactDefinitionTest.java | 48 +++++++++++++++++++ 4 files changed, 53 insertions(+), 8 deletions(-) create mode 100644 pipeline-builder/src/test/java/io/spinnaker/pipelinebuilder/json/artifacts/Base64ArtifactDefinitionTest.java diff --git a/.gitignore b/.gitignore index 3859bdb..97fcf62 100644 --- a/.gitignore +++ b/.gitignore @@ -5,6 +5,7 @@ build *.iml *.ipr *.iws +*lombok.config .idea .vscode pipeline-builder/build @@ -14,6 +15,7 @@ spinnaker-ci/build **/.settings *.hprof **/bin +**/.idea # generated by javadoc gradle task **/lombok.config diff --git a/pipeline-builder/src/main/java/io/spinnaker/pipelinebuilder/json/artifacts/Base64ArtifactDefinition.java b/pipeline-builder/src/main/java/io/spinnaker/pipelinebuilder/json/artifacts/Base64ArtifactDefinition.java index 12e802f..aafe470 100644 --- a/pipeline-builder/src/main/java/io/spinnaker/pipelinebuilder/json/artifacts/Base64ArtifactDefinition.java +++ b/pipeline-builder/src/main/java/io/spinnaker/pipelinebuilder/json/artifacts/Base64ArtifactDefinition.java @@ -77,17 +77,12 @@ public class Base64ArtifactDefinition implements ArtifactDefinition { */ @JsonIgnore protected final Boolean shouldEncode; - @Builder // this creates a builder with just these two fields - public Base64ArtifactDefinition(String id, String name, String contents) { - this(id, name, contents, true); - } - @Builder // this creates a builder with just these three fields public Base64ArtifactDefinition(String id, String name, String contents, Boolean shouldEncode) { this.id = Objects.requireNonNullElse(id, UUID.randomUUID().toString()); this.name = name; this.contents = contents; - this.shouldEncode = shouldEncode; + this.shouldEncode = shouldEncode == null || shouldEncode; } @JsonProperty("reference") diff --git a/pipeline-builder/src/main/java/io/spinnaker/pipelinebuilder/json/artifacts/EmptyCustomArtifactDefinition.java b/pipeline-builder/src/main/java/io/spinnaker/pipelinebuilder/json/artifacts/EmptyCustomArtifactDefinition.java index 70351e1..cc3afda 100644 --- a/pipeline-builder/src/main/java/io/spinnaker/pipelinebuilder/json/artifacts/EmptyCustomArtifactDefinition.java +++ b/pipeline-builder/src/main/java/io/spinnaker/pipelinebuilder/json/artifacts/EmptyCustomArtifactDefinition.java @@ -32,11 +32,11 @@ @JsonInclude(Include.NON_NULL) // don't serialize null fields public class EmptyCustomArtifactDefinition extends Base64ArtifactDefinition { public EmptyCustomArtifactDefinition() { - super(UUID.randomUUID().toString(), null, ""); + super(UUID.randomUUID().toString(), null, "", true); } // @Builder comes from Base64ArtifactDefinition public EmptyCustomArtifactDefinition(String id) { - super(id, null, ""); + super(id, null, "", true); } } diff --git a/pipeline-builder/src/test/java/io/spinnaker/pipelinebuilder/json/artifacts/Base64ArtifactDefinitionTest.java b/pipeline-builder/src/test/java/io/spinnaker/pipelinebuilder/json/artifacts/Base64ArtifactDefinitionTest.java new file mode 100644 index 0000000..c4f510f --- /dev/null +++ b/pipeline-builder/src/test/java/io/spinnaker/pipelinebuilder/json/artifacts/Base64ArtifactDefinitionTest.java @@ -0,0 +1,48 @@ +package io.spinnaker.pipelinebuilder.json.artifacts; + +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; + +import java.util.Arrays; +import java.util.Base64; +import java.util.stream.Stream; + +import static org.junit.jupiter.api.Assertions.*; + +class Base64ArtifactDefinitionTest { + @ParameterizedTest + @MethodSource("referenceArguments") + public void reference(String content, String expected, Boolean shouldEncode) { + Base64ArtifactDefinition artifact = Base64ArtifactDefinition.builder() + .shouldEncode(shouldEncode) + .contents(content) + .build(); + + byte[] bytes = content.getBytes(); + String reference = artifact.getReference(); + // resolvedShouldEncode should consider whether we planned to encode, + // and account for it being null. + // + // The default value for shouldEncode is true, so if null was passed in, + // we need to ensure that resolvedShouldEncode is true. + Boolean resolvedShouldEncode = shouldEncode == null || shouldEncode; + // didContentBytesGetEncoded compares the bytes based on whether it + // encoded the reference or not. So if the reference was base64 encoded, + // they should be different, and same otherwise. + Boolean didContentBytesGetEncoded = !Arrays.equals(bytes, reference.getBytes()); + assertEquals(resolvedShouldEncode, didContentBytesGetEncoded); + assertEquals(expected, reference); + } + + public static Stream referenceArguments() { + String content = "hello world"; + String contentBase64 = Base64.getEncoder().encodeToString(content.getBytes()); + + return Stream.of( + Arguments.of(content, contentBase64, null), + Arguments.of(content, contentBase64, true), + Arguments.of(content, content, false) + ); + } +}