Skip to content

Commit

Permalink
Fix StringToArtifactPropertiesConverter for empty lines
Browse files Browse the repository at this point in the history
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
  • Loading branch information
artembilan authored and wilkinsona committed Nov 19, 2024
1 parent e813a63 commit db8b007
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
* {@link String}.
*
* @author Andy Wilkinson
* @author Artem Bilan
*/
class StringToArtifactPropertiesConverter implements Converter<String, List<ArtifactProperties>> {

Expand All @@ -46,11 +47,11 @@ public List<ArtifactProperties> 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 <includes>:<excludes>:<properties>");
return new ArtifactProperties(commaSeparatedList(components[0]), commaSeparatedList(components[1]),
commaSeparatedKeyValues(components[2]));
Expand All @@ -68,7 +69,7 @@ private Map<String, String> 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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
* Tests for {@link StringToArtifactPropertiesConverter}.
*
* @author Andy Wilkinson
* @author Artem Bilan
*/
class StringToArtifactPropertiesConverterTests {

Expand Down Expand Up @@ -75,7 +76,7 @@ void convertWithPropertyWithEqualsInValueHasProperty() {
@Test
void convertWithMultipleLinesProducesMultipleArtifactProperties() {
List<ArtifactProperties> 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");
Expand Down

0 comments on commit db8b007

Please sign in to comment.