Skip to content

Commit

Permalink
Make result of file visitor optional.
Browse files Browse the repository at this point in the history 
Then we can simplify the error handling in the base class.
  • Loading branch information
@uhafner
uhafner committed Nov 30, 2022
1 parent d23109c commit 60a4ef2
Show file tree
Hide file tree
Showing 2 changed files with 65 additions and 11 deletions.
13 changes: 10 additions & 3 deletions src/main/java/io/jenkins/plugins/util/AgentFileVisitor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import java.util.Optional;

import org.apache.commons.lang3.StringUtils;
import org.apache.tools.ant.BuildException;
Expand Down Expand Up @@ -110,8 +111,14 @@ else if (fileSystemFacade.isEmpty(file)) {
}
}
else {
results.add(processFile(file, new CharsetValidation().getCharset(encoding), log));
log.logInfo("Successfully processed file '%s'", fileName);
Optional<T> result = processFile(file, new CharsetValidation().getCharset(encoding), log);
if (result.isPresent()) {
results.add(result.get());
log.logInfo("Successfully processed file '%s'", fileName);
}
else {
log.logError("No result created for file '%s' due to some errors", fileName);
}
}
}
return results;
Expand All @@ -131,7 +138,7 @@ protected String plural(final int count, @SuppressWarnings("SameParameterValue")
return String.format("%d %s%s", count, itemName, count == 1 ? "" : "s");
}

protected abstract T processFile(Path file, Charset charset, FilteredLog log);
protected abstract Optional<T> processFile(Path file, Charset charset, FilteredLog log);

/**
* File system facade that can be replaced by a stub in unit tests.
Expand Down
63 changes: 55 additions & 8 deletions src/test/java/io/jenkins/plugins/util/AgentFileVisitorTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import java.io.File;
import java.nio.charset.Charset;
import java.nio.file.Path;
import java.util.Optional;

import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Test;
Expand Down Expand Up @@ -30,6 +31,7 @@
class AgentFileVisitorTest extends SerializableTest<StringScanner> {
private static final String CONTENT = "Hello World!";
private static final String PATTERN = "**/*.txt";
private static final String ENCODING = "UTF-8";

@TempDir
private File workspace;
Expand All @@ -38,7 +40,7 @@ class AgentFileVisitorTest extends SerializableTest<StringScanner> {
@CsvSource({"true, enabled", "false, disabled"})
@ParameterizedTest(name = "{index} => followSymbolicLinks={0}, message={1}")
void shouldReportErrorOnEmptyResults(final boolean followLinks, final String message) {
StringScanner scanner = new StringScanner(PATTERN, "UTF-8", followLinks, true,
StringScanner scanner = new StringScanner(PATTERN, ENCODING, followLinks, true,
createFileSystemFacade(followLinks));

FileVisitorResult<String> actualResult = scanner.invoke(workspace, null);
Expand All @@ -57,7 +59,7 @@ void shouldReportErrorOnEmptyResults(final boolean followLinks, final String mes
@CsvSource({"true, enabled", "false, disabled"})
@ParameterizedTest(name = "{index} => followSymbolicLinks={0}, message={1}")
void shouldReturnSingleResult(final boolean followLinks, final String message) {
StringScanner scanner = new StringScanner(PATTERN, "UTF-8", followLinks, true,
StringScanner scanner = new StringScanner(PATTERN, ENCODING, followLinks, true,
createFileSystemFacade(followLinks, "/one.txt"));

FileVisitorResult<String> actualResult = scanner.invoke(workspace, null);
Expand All @@ -75,7 +77,7 @@ void shouldReturnSingleResult(final boolean followLinks, final String message) {
@CsvSource({"true, enabled", "false, disabled"})
@ParameterizedTest(name = "{index} => followSymbolicLinks={0}, message={1}")
void shouldReturnMultipleResults(final boolean followLinks, final String message) {
StringScanner scanner = new StringScanner(PATTERN, "UTF-8", followLinks, true,
StringScanner scanner = new StringScanner(PATTERN, ENCODING, followLinks, true,
createFileSystemFacade(followLinks, "/one.txt", "/two.txt"));

FileVisitorResult<String> actualResult = scanner.invoke(workspace, null);
Expand Down Expand Up @@ -104,7 +106,7 @@ void shouldLogErrorForEmptyAndForbiddenFiles() {
when(fileSystemFacade.resolve(workspace, "not-readable.txt")).thenReturn(notReadable);
when(fileSystemFacade.isNotReadable(notReadable)).thenReturn(true);

StringScanner scanner = new StringScanner(PATTERN, "UTF-8", true, true,
StringScanner scanner = new StringScanner(PATTERN, ENCODING, true, true,
fileSystemFacade);

FileVisitorResult<String> actualResult = scanner.invoke(workspace, null);
Expand All @@ -130,7 +132,7 @@ void shouldSkipLoggingOfErrorsForEmptyFiles() {
when(fileSystemFacade.resolve(workspace, "empty.txt")).thenReturn(empty);
when(fileSystemFacade.isEmpty(empty)).thenReturn(true);

StringScanner scanner = new StringScanner(PATTERN, "UTF-8", true, false,
StringScanner scanner = new StringScanner(PATTERN, ENCODING, true, false,
fileSystemFacade);

FileVisitorResult<String> actualResult = scanner.invoke(workspace, null);
Expand All @@ -145,6 +147,24 @@ void shouldSkipLoggingOfErrorsForEmptyFiles() {
assertThat(actualResult.getLog().getErrorMessages()).isEmpty();
}

@Test
@DisplayName("Should log error if no results are returned")
void shouldLogErrorIfNoResultIsAvailable() {
FileSystemFacade fileSystemFacade = createFileSystemFacade(false, "/one.txt");

EmptyScanner scanner = new EmptyScanner(fileSystemFacade);

FileVisitorResult<String> actualResult = scanner.invoke(workspace, null);
assertThat(actualResult.getResults()).isEmpty();
assertThat(actualResult.getLog().getInfoMessages()).containsExactly(
"Searching for all files in '/absolute/path' that match the pattern '**/*.txt'",
"Traversing of symbolic links: disabled",
"-> found 1 file");
assertThat(actualResult.hasErrors()).isTrue();
assertThat(actualResult.getLog().getErrorMessages()).containsExactly("Errors during parsing",
"No result created for file '/one.txt' due to some errors");
}

private FileSystemFacade createFileSystemFacade(final boolean followLinks, final String... files) {
FileSystemFacade fileSystem = mock(FileSystemFacade.class);

Expand All @@ -156,7 +176,7 @@ private FileSystemFacade createFileSystemFacade(final boolean followLinks, final

@Override
protected StringScanner createSerializable() {
return new StringScanner(PATTERN, "UTF-8", true, true, createFileSystemFacade(true));
return new StringScanner(PATTERN, ENCODING, true, true, createFileSystemFacade(true));
}

static class StringScanner extends AgentFileVisitor<String> {
Expand All @@ -169,8 +189,35 @@ protected StringScanner(final String filePattern, final String encoding, final b
}

@Override
protected String processFile(final Path file, final Charset charset, final FilteredLog log) {
return CONTENT + counter++;
protected Optional<String> processFile(final Path file, final Charset charset, final FilteredLog log) {
return Optional.of(CONTENT + counter++);
}

@SuppressWarnings("EqualsWhichDoesntCheckParameterClass")
@SuppressFBWarnings(value = "EQ_ALWAYS_TRUE", justification = "Required for serializable test")
@Override
public boolean equals(final Object o) {
return true;
}

@Override
public int hashCode() {
return 0;
}
}

private static class EmptyScanner extends AgentFileVisitor<String> {

private static final long serialVersionUID = 3700448215163706213L;

@VisibleForTesting
protected EmptyScanner(final FileSystemFacade fileSystemFacade) {
super(PATTERN, ENCODING, false, false, fileSystemFacade);
}

@Override
protected Optional<String> processFile(final Path file, final Charset charset, final FilteredLog log) {
return Optional.empty();
}

@SuppressWarnings("EqualsWhichDoesntCheckParameterClass")
Expand Down

0 comments on commit 60a4ef2

Please sign in to comment.