Skip to content

Commit

Permalink
Merge pull request #228 from shamlymhd/shamly-fix-i
Browse files Browse the repository at this point in the history 
Make severity of scanning empty files configurable
  • Loading branch information
@uhafner
uhafner authored Nov 30, 2022
2 parents 7ab4980 + 7fb1d38 commit 9eec644
Show file tree
Hide file tree
Showing 2 changed files with 53 additions and 19 deletions.
21 changes: 15 additions & 6 deletions src/main/java/io/jenkins/plugins/util/AgentFileVisitor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,9 @@ public abstract class AgentFileVisitor<T extends Serializable>
private final String filePattern;
private final String encoding;
private final boolean followSymbolicLinks;
private final boolean errorOnEmptyFiles;
private final FileSystemFacade fileSystemFacade;
private static final String EMPTY_FILE = "Skipping file '%s' because it's empty";

/**
* Creates a new instance of {@link AgentFileVisitor}.
Expand All @@ -52,20 +54,22 @@ public abstract class AgentFileVisitor<T extends Serializable>
* @param encoding
* encoding of the files to parse
* @param followSymbolicLinks
* if the scanner should traverse symbolic links
* determines whether the visitor should traverse symbolic links
* @param errorOnEmptyFiles
* determines whether the visitor should log errors if a file is empty
*/
protected AgentFileVisitor(final String filePattern, final String encoding, final boolean followSymbolicLinks) {
this(filePattern, encoding, followSymbolicLinks, new FileSystemFacade());
protected AgentFileVisitor(final String filePattern, final String encoding, final boolean followSymbolicLinks, final boolean errorOnEmptyFiles) {
this(filePattern, encoding, followSymbolicLinks, errorOnEmptyFiles, new FileSystemFacade());
}

@VisibleForTesting
AgentFileVisitor(final String filePattern, final String encoding, final boolean followSymbolicLinks,
final FileSystemFacade fileSystemFacade) {
AgentFileVisitor(final String filePattern, final String encoding, final boolean followSymbolicLinks, final boolean errorOnEmptyFiles, final FileSystemFacade fileSystemFacade) {
super();

this.filePattern = filePattern;
this.encoding = encoding;
this.followSymbolicLinks = followSymbolicLinks;
this.errorOnEmptyFiles = errorOnEmptyFiles;
this.fileSystemFacade = fileSystemFacade;
}

Expand Down Expand Up @@ -98,7 +102,12 @@ private List<T> scanFiles(final File workspace, final String[] fileNames, final
log.logError("Skipping file '%s' because Jenkins has no permission to read the file", fileName);
}
else if (fileSystemFacade.isEmpty(file)) {
log.logError("Skipping file '%s' because it's empty", fileName);
if (errorOnEmptyFiles) {
log.logError(EMPTY_FILE, fileName);
}
else {
log.logInfo(EMPTY_FILE, fileName);
}
}
else {
results.add(processFile(file, new CharsetValidation().getCharset(encoding), log));
Expand Down
51 changes: 38 additions & 13 deletions src/test/java/io/jenkins/plugins/util/AgentFileVisitorTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,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,
StringScanner scanner = new StringScanner(PATTERN, "UTF-8", followLinks, true,
createFileSystemFacade(followLinks));

FileVisitorResult<String> actualResult = scanner.invoke(workspace, null);
Expand All @@ -57,7 +57,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,
StringScanner scanner = new StringScanner(PATTERN, "UTF-8", followLinks, true,
createFileSystemFacade(followLinks, "/one.txt"));

FileVisitorResult<String> actualResult = scanner.invoke(workspace, null);
Expand All @@ -75,7 +75,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,
StringScanner scanner = new StringScanner(PATTERN, "UTF-8", followLinks, true,
createFileSystemFacade(followLinks, "/one.txt", "/two.txt"));

FileVisitorResult<String> actualResult = scanner.invoke(workspace, null);
Expand All @@ -91,20 +91,20 @@ void shouldReturnMultipleResults(final boolean followLinks, final String message
}

@Test
@DisplayName("Should handle empty or forbidden files")
void shouldReturnMultipleResults() {
@DisplayName("Should log error for empty or forbidden files")
void shouldLogErrorForEmptyAndForbiddenFiles() {
FileSystemFacade fileSystemFacade = createFileSystemFacade(true,
"/one.txt", "/two.txt", "empty.txt", "not-readable.txt");

Path empty = workspace.toPath().resolve("empty.txt");
when(fileSystemFacade.resolve(workspace, "empty.txt")).thenReturn(empty);
when(fileSystemFacade.isNotReadable(empty)).thenReturn(true);
when(fileSystemFacade.isEmpty(empty)).thenReturn(true);

Path notReadable = workspace.toPath().resolve("not-readable.txt");
when(fileSystemFacade.resolve(workspace, "not-readable.txt")).thenReturn(notReadable);
when(fileSystemFacade.isEmpty(notReadable)).thenReturn(true);
when(fileSystemFacade.isNotReadable(notReadable)).thenReturn(true);

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

FileVisitorResult<String> actualResult = scanner.invoke(workspace, null);
Expand All @@ -116,8 +116,33 @@ void shouldReturnMultipleResults() {
"Successfully processed file '/two.txt'");
assertThat(actualResult.hasErrors()).isTrue();
assertThat(actualResult.getLog().getErrorMessages()).containsExactly("Errors during parsing",
"Skipping file 'empty.txt' because Jenkins has no permission to read the file",
"Skipping file 'not-readable.txt' because it's empty");
"Skipping file 'empty.txt' because it's empty",
"Skipping file 'not-readable.txt' because Jenkins has no permission to read the file");
}

@Test
@DisplayName("Should skip logging of errors when parsing empty files")
void shouldSkipLoggingOfErrorsForEmptyFiles() {
FileSystemFacade fileSystemFacade = createFileSystemFacade(true,
"/one.txt", "/two.txt", "empty.txt");

Path empty = workspace.toPath().resolve("empty.txt");
when(fileSystemFacade.resolve(workspace, "empty.txt")).thenReturn(empty);
when(fileSystemFacade.isEmpty(empty)).thenReturn(true);

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

FileVisitorResult<String> actualResult = scanner.invoke(workspace, null);
assertThat(actualResult.getResults()).containsExactly(CONTENT + 1, CONTENT + 2);
assertThat(actualResult.getLog().getInfoMessages()).contains(
"Searching for all files in '/absolute/path' that match the pattern '**/*.txt'",
"-> found 3 files",
"Successfully processed file '/one.txt'",
"Successfully processed file '/two.txt'",
"Skipping file 'empty.txt' because it's empty");
assertThat(actualResult.hasErrors()).isFalse();
assertThat(actualResult.getLog().getErrorMessages()).isEmpty();
}

private FileSystemFacade createFileSystemFacade(final boolean followLinks, final String... files) {
Expand All @@ -131,16 +156,16 @@ private FileSystemFacade createFileSystemFacade(final boolean followLinks, final

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

static class StringScanner extends AgentFileVisitor<String> {
private static final long serialVersionUID = -6902473746775046311L;
private int counter = 1;

@VisibleForTesting
protected StringScanner(final String filePattern, final String encoding, final boolean followSymbolicLinks, final FileSystemFacade fileSystemFacade) {
super(filePattern, encoding, followSymbolicLinks, fileSystemFacade);
protected StringScanner(final String filePattern, final String encoding, final boolean followSymbolicLinks, final boolean errorOnEmptyFiles, final FileSystemFacade fileSystemFacade) {
super(filePattern, encoding, followSymbolicLinks, errorOnEmptyFiles, fileSystemFacade);
}

@Override
Expand Down

0 comments on commit 9eec644

Please sign in to comment.