Skip to content

Commit

Permalink
fix(base64): Base64 artifacts not utilizing encode flag
Browse files Browse the repository at this point in the history
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 <[email protected]>
  • Loading branch information
nicolasff committed Jan 10, 2024
1 parent 8d633e5 commit da5c35b
Show file tree
Hide file tree
Showing 4 changed files with 53 additions and 8 deletions.
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ build
*.iml
*.ipr
*.iws
*lombok.config
.idea
.vscode
pipeline-builder/build
Expand All @@ -14,6 +15,7 @@ spinnaker-ci/build
**/.settings
*.hprof
**/bin
**/.idea

# generated by javadoc gradle task
**/lombok.config
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
Original file line number Diff line number Diff line change
@@ -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<Arguments> 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)
);
}
}

0 comments on commit da5c35b

Please sign in to comment.