From db8b0071a40727027e287f5c1691dd04168983db Mon Sep 17 00:00:00 2001 From: Artem Bilan Date: Fri, 19 Jul 2024 17:50:54 -0400 Subject: [PATCH] Fix StringToArtifactPropertiesConverter for empty lines The problem with action configuration in YAML: ``` artifact-properties: | /**/*.zip::zip.name=${{ github.event.repository.name }},zip.deployed=false /**/*docs.zip::zip.type=docs /**/*dist.zip::zip.type=dist ``` It visually not obvious that there are some empty lines in the end. Plus end-user may decide to separate those entries with empty lines as well. * Fix `StringToArtifactPropertiesConverter` to use `StringUtils.hasText()`. The `hasLength()` does count whitespaces as content * Some other suggested by IDEA simple refactoring for the `StringToArtifactPropertiesConverter`: - `String.split()` never returns null - `String.substring()` without second argument yields to `length()` internally --- .../artifactory/StringToArtifactPropertiesConverter.java | 7 ++++--- .../StringToArtifactPropertiesConverterTests.java | 3 ++- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/main/java/io/spring/github/actions/artifactorydeploy/artifactory/StringToArtifactPropertiesConverter.java b/src/main/java/io/spring/github/actions/artifactorydeploy/artifactory/StringToArtifactPropertiesConverter.java index 7d34531..01b7a89 100644 --- a/src/main/java/io/spring/github/actions/artifactorydeploy/artifactory/StringToArtifactPropertiesConverter.java +++ b/src/main/java/io/spring/github/actions/artifactorydeploy/artifactory/StringToArtifactPropertiesConverter.java @@ -36,6 +36,7 @@ * {@link String}. * * @author Andy Wilkinson + * @author Artem Bilan */ class StringToArtifactPropertiesConverter implements Converter> { @@ -46,11 +47,11 @@ public List convert(String source) { } private ArtifactProperties toArtifactProperties(String line) { - if (!StringUtils.hasLength(line)) { + if (!StringUtils.hasText(line)) { return null; } String[] components = line.split(":"); - Assert.state(components != null && components.length == 3, + Assert.state(components.length == 3, "Artifact properties must be configured in the form ::"); return new ArtifactProperties(commaSeparatedList(components[0]), commaSeparatedList(components[1]), commaSeparatedKeyValues(components[2])); @@ -68,7 +69,7 @@ private Map commaSeparatedKeyValues(String input) { for (String pair : input.split(",")) { int equalsIndex = pair.indexOf('='); String key = pair.substring(0, equalsIndex); - String value = pair.substring(equalsIndex + 1, pair.length()); + String value = pair.substring(equalsIndex + 1); properties.put(key, value); } return properties; diff --git a/src/test/java/io/spring/github/actions/artifactorydeploy/artifactory/StringToArtifactPropertiesConverterTests.java b/src/test/java/io/spring/github/actions/artifactorydeploy/artifactory/StringToArtifactPropertiesConverterTests.java index 042e12c..2cd3b1b 100644 --- a/src/test/java/io/spring/github/actions/artifactorydeploy/artifactory/StringToArtifactPropertiesConverterTests.java +++ b/src/test/java/io/spring/github/actions/artifactorydeploy/artifactory/StringToArtifactPropertiesConverterTests.java @@ -27,6 +27,7 @@ * Tests for {@link StringToArtifactPropertiesConverter}. * * @author Andy Wilkinson + * @author Artem Bilan */ class StringToArtifactPropertiesConverterTests { @@ -75,7 +76,7 @@ void convertWithPropertyWithEqualsInValueHasProperty() { @Test void convertWithMultipleLinesProducesMultipleArtifactProperties() { List artifactProperties = this.converter - .convert("one,two:three:a=alpha,b=bravo%nfour:five:c=charlie%nsix:seven:d=delta".formatted()); + .convert("one,two:three:a=alpha,b=bravo%n %nfour:five:c=charlie%nsix:seven:d=delta".formatted()); assertThat(artifactProperties).hasSize(3).first().satisfies((properties) -> { assertThat(properties.include()).containsExactly("one", "two"); assertThat(properties.exclude()).containsExactly("three");