You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@geode.apache.org by GitBox <gi...@apache.org> on 2020/10/05 22:12:29 UTC

[GitHub] [geode] demery-pivotal opened a new pull request #5595: GEODE-8572: Make LogExporter not read dirs

demery-pivotal opened a new pull request #5595:
URL: https://github.com/apache/geode/pull/5595


   Changed LogExporter to refrain from attempting to read directories, even
   when a directory's path matches the exporter's file selector predicates.
   
   Changed LogExporterIntegrationTest to configure the server to write
   files in a unique subdirectory of the current working directory. Some
   tests were configuring the server to use a temporary folder that could
   be reused by other tests.
   
   Thank you for submitting a contribution to Apache Geode.
   
   In order to streamline the review of the contribution we ask you
   to ensure the following steps have been taken:
   
   ### For all changes:
   - [ ] Is there a JIRA ticket associated with this PR? Is it referenced in the commit message?
   
   - [ ] Has your PR been rebased against the latest commit within the target branch (typically `develop`)?
   
   - [ ] Is your initial contribution a single, squashed commit?
   
   - [ ] Does `gradlew build` run cleanly?
   
   - [ ] Have you written or updated unit tests to verify your changes?
   
   - [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)?
   
   ### Note:
   Please ensure that once the PR is submitted, check Concourse for build issues and
   submit an update to your PR as soon as possible. If you need help, please send an
   email to dev@geode.apache.org.
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [geode] demery-pivotal commented on a change in pull request #5595: GEODE-8572: Make LogExporter not read dirs

Posted by GitBox <gi...@apache.org>.
demery-pivotal commented on a change in pull request #5595:
URL: https://github.com/apache/geode/pull/5595#discussion_r500649130



##########
File path: geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/util/LogExporter.java
##########
@@ -181,12 +180,13 @@ private long filterAndSize(Path originalLogFile) throws IOException {
   }
 
   private List<Path> findFiles(Path workingDir, Predicate<Path> fileSelector) throws IOException {
-    Stream<Path> selectedFiles/* = null */;
     if (!workingDir.toFile().isDirectory()) {
       return Collections.emptyList();
     }
-    selectedFiles = Files.list(workingDir).filter(fileSelector).filter(this.logFilter::acceptsFile);
-
-    return selectedFiles.collect(toList());
+    return Files.list(workingDir)
+        .filter(Files::isRegularFile)

Review comment:
       Yes, that's what I thought, and I verified with some experiments (which did not involve the exporter).
   
   My question remains: If the path we're testing is a symbolic link, are you saying that you _want_ to ignore the file it links to? If so, what makes you want to skip the file in that case?
   
   One possibility: The link links to a file in the same dir, which would lead us to export the content twice (once via the file, and once via the link to the file). We could solve this by resolving the real path to the file, and collecting the matching files into a Set instead of a List.
   
   Another possibility: The link links to a file in some other directory, and some concern makes you not want to read the file from the other directory.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [geode] demery-pivotal commented on a change in pull request #5595: GEODE-8572: Make LogExporter not read dirs

Posted by GitBox <gi...@apache.org>.
demery-pivotal commented on a change in pull request #5595:
URL: https://github.com/apache/geode/pull/5595#discussion_r500530806



##########
File path: geode-gfsh/src/integrationTest/java/org/apache/geode/management/internal/cli/util/LogExporterIntegrationTest.java
##########
@@ -50,127 +48,146 @@
 
 @Category({GfshTest.class, LoggingTest.class})
 public class LogExporterIntegrationTest {
+  @Rule
+  public ServerStarterRule server = new ServerStarterRule();
 
   private final LogFilter filter = new LogFilter(Level.INFO, null, null);
 
-  private LogExporter logExporter;
-  private Properties properties;
-
-  @Rule
-  public ServerStarterRule server = new ServerStarterRule();
+  public Path serverFilesDir;
 
   @Before
-  public void before() {
-    properties = new Properties();
-    // make sure the server's working dir has no log files or stats file to begin with, since in
-    // some tests we are asserting on the # of log files and stats files created by the server
-    File workingDir = server.getWorkingDir();
-    Arrays.stream(workingDir.listFiles())
-        .filter(f -> (f.getName().endsWith(".log") || f.getName().endsWith(".gfs")))
-        .forEach(FileUtils::deleteQuietly);
+  public void createServerFilesDir() throws IOException {
+    // Name the directory after this test instance and the Gradle test worker, to ensure that tests
+    // running in parallel use different directories.
+    String testRunnerID = System.getProperty("org.gradle.test.worker", "standalone");
+    int testInstanceID = System.identityHashCode(this);
+    String className = getClass().getSimpleName();
+    String dirName = String.format("%s-%x-%s", className, testInstanceID, testRunnerID);
+    serverFilesDir = Files.createDirectories(Paths.get(dirName)).normalize().toAbsolutePath();
+  }
+
+  @After
+  public void deleteServerFilesDir() {
+    FileUtils.deleteQuietly(serverFilesDir.toFile());
   }
 
   @Test
   public void serverStartedWithWrongSuffix() throws Exception {
-    properties.setProperty(LOG_FILE, new File("test.txt").getAbsolutePath());
-    properties.setProperty(STATISTIC_ARCHIVE_FILE, "archive.archive");
-    server.withProperties(properties).startServer();
-    File serverWorkingDir = server.getWorkingDir();
-
-    logExporter = new LogExporter(filter, new File(serverWorkingDir, "test.log"),
-        new File(serverWorkingDir, "stats.gfs"));
-    List<Path> logFiles = logExporter.findLogFiles(serverWorkingDir.toPath());
-    assertThat(logFiles).isEmpty();
-
-    List<Path> statsFiles = logExporter.findStatFiles(serverWorkingDir.toPath());
-    assertThat(statsFiles).isEmpty();
+    String logFileNameWithWrongSuffix = "test.txt";
+    String statsFileNameWithWrongSuffix = "archive.archive";
+
+    Path logFile = serverFilesDir.resolve(logFileNameWithWrongSuffix);
+    Path statsFile = serverFilesDir.resolve(statsFileNameWithWrongSuffix);
+
+    server.withProperty(LOG_FILE, logFile.toString())
+        .withProperty(STATISTIC_ARCHIVE_FILE, statsFile.toString())
+        .startServer();
+
+    LogExporter logExporter = new LogExporter(filter, null, null);
+    List<Path> logFiles = logExporter.findLogFiles(serverFilesDir);
+
+    assertThat(logFiles)
+        .as("log files")
+        .isEmpty();
+
+    List<Path> statsFiles = logExporter.findStatFiles(serverFilesDir);
+    assertThat(statsFiles)
+        .as("stat files")
+        .isEmpty();
   }
 
   @Test
   public void serverStartedWithCorrectSuffix() throws Exception {
-    // ("relative log file is problematic in the test environment")
-    properties.setProperty(LOG_FILE, new File("test.log").getAbsolutePath());
-    properties.setProperty(STATISTIC_ARCHIVE_FILE, "archive.gfs");
-    server.withProperties(properties).startServer();
-    File serverWorkingDir = server.getWorkingDir();
-
-    logExporter = new LogExporter(filter, new File(serverWorkingDir, "test.log"),
-        new File(serverWorkingDir, "archive.gfs"));
-    List<Path> logFiles = logExporter.findLogFiles(serverWorkingDir.toPath());
-    assertThat(logFiles).hasSize(1);
-    assertThat(logFiles.get(0)).hasFileName("test.log");
-
-    List<Path> statsFiles = logExporter.findStatFiles(serverWorkingDir.toPath());
-    assertThat(statsFiles).hasSize(1);
-    assertThat(statsFiles.get(0)).hasFileName("archive.gfs");
+    String logFileName = "test.log";
+    String statsFileName = "archive.gfs";
+    Path logFile = serverFilesDir.resolve(logFileName);
+    Path statsFile = serverFilesDir.resolve(statsFileName);
+
+    server.withProperty(LOG_FILE, logFile.toString())
+        .withProperty(STATISTIC_ARCHIVE_FILE, statsFile.toString())
+        .startServer();
+
+    LogExporter logExporter = new LogExporter(filter, null, null);
+    List<Path> logFiles = logExporter.findLogFiles(serverFilesDir);
+
+    assertThat(logFiles)
+        .as("log files")
+        .hasSize(1);
+    assertThat(logFiles.get(0)).hasFileName(logFileName);
+
+    List<Path> statsFiles = logExporter.findStatFiles(serverFilesDir);
+    assertThat(statsFiles)
+        .as("stat files")
+        .hasSize(1);
+    assertThat(statsFiles.get(0)).hasFileName(statsFileName);
   }
 
   @Test
   @Ignore("GEODE-2574: fix .gz suffix")

Review comment:
       Though the test is ignored, I updated it to match the style of the tests I changed. I left it ignored because the system fails the test.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [geode] demery-pivotal commented on a change in pull request #5595: GEODE-8572: Make LogExporter not read dirs

Posted by GitBox <gi...@apache.org>.
demery-pivotal commented on a change in pull request #5595:
URL: https://github.com/apache/geode/pull/5595#discussion_r500664500



##########
File path: geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/util/LogExporter.java
##########
@@ -181,12 +180,13 @@ private long filterAndSize(Path originalLogFile) throws IOException {
   }
 
   private List<Path> findFiles(Path workingDir, Predicate<Path> fileSelector) throws IOException {
-    Stream<Path> selectedFiles/* = null */;
     if (!workingDir.toFile().isDirectory()) {
       return Collections.emptyList();
     }
-    selectedFiles = Files.list(workingDir).filter(fileSelector).filter(this.logFilter::acceptsFile);
-
-    return selectedFiles.collect(toList());
+    return Files.list(workingDir)
+        .filter(Files::isRegularFile)

Review comment:
       Okay. I'll merge this, because it fixes the problem of trying to read directories, but will otherwise behave as before. In particular, before and after this fix it follows symbolic links when exporting files.
   
   As for whether we _ought_ to follow symbolic links when we're exporting, I'm confident I have no more context than you do about that ;-)




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [geode] demery-pivotal commented on a change in pull request #5595: GEODE-8572: Make LogExporter not read dirs

Posted by GitBox <gi...@apache.org>.
demery-pivotal commented on a change in pull request #5595:
URL: https://github.com/apache/geode/pull/5595#discussion_r500529398



##########
File path: geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/util/LogExporter.java
##########
@@ -181,12 +180,13 @@ private long filterAndSize(Path originalLogFile) throws IOException {
   }
 
   private List<Path> findFiles(Path workingDir, Predicate<Path> fileSelector) throws IOException {
-    Stream<Path> selectedFiles/* = null */;
     if (!workingDir.toFile().isDirectory()) {
       return Collections.emptyList();
     }
-    selectedFiles = Files.list(workingDir).filter(fileSelector).filter(this.logFilter::acceptsFile);
-
-    return selectedFiles.collect(toList());
+    return Files.list(workingDir)
+        .filter(Files::isRegularFile)

Review comment:
       I hadn't thought about that. Is there a way to ensure that the path represents a regular file without following symbolic links?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [geode] jchen21 commented on a change in pull request #5595: GEODE-8572: Make LogExporter not read dirs

Posted by GitBox <gi...@apache.org>.
jchen21 commented on a change in pull request #5595:
URL: https://github.com/apache/geode/pull/5595#discussion_r500576818



##########
File path: geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/util/LogExporter.java
##########
@@ -181,12 +180,13 @@ private long filterAndSize(Path originalLogFile) throws IOException {
   }
 
   private List<Path> findFiles(Path workingDir, Predicate<Path> fileSelector) throws IOException {
-    Stream<Path> selectedFiles/* = null */;
     if (!workingDir.toFile().isDirectory()) {
       return Collections.emptyList();
     }
-    selectedFiles = Files.list(workingDir).filter(fileSelector).filter(this.logFilter::acceptsFile);
-
-    return selectedFiles.collect(toList());
+    return Files.list(workingDir)
+        .filter(Files::isRegularFile)

Review comment:
       You might want [`NOFOLLOW_LINKS`](https://docs.oracle.com/javase/8/docs/api/java/nio/file/Files.html#isRegularFile-java.nio.file.Path-java.nio.file.LinkOption...-). 




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [geode] demery-pivotal commented on a change in pull request #5595: GEODE-8572: Make LogExporter not read dirs

Posted by GitBox <gi...@apache.org>.
demery-pivotal commented on a change in pull request #5595:
URL: https://github.com/apache/geode/pull/5595#discussion_r500610779



##########
File path: geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/util/LogExporter.java
##########
@@ -181,12 +180,13 @@ private long filterAndSize(Path originalLogFile) throws IOException {
   }
 
   private List<Path> findFiles(Path workingDir, Predicate<Path> fileSelector) throws IOException {
-    Stream<Path> selectedFiles/* = null */;
     if (!workingDir.toFile().isDirectory()) {
       return Collections.emptyList();
     }
-    selectedFiles = Files.list(workingDir).filter(fileSelector).filter(this.logFilter::acceptsFile);
-
-    return selectedFiles.collect(toList());
+    return Files.list(workingDir)
+        .filter(Files::isRegularFile)

Review comment:
       Here's a scenario: Let's say the path is a symbolic link, and it links to a regular file.
   
   My understanding is:
   - If I use `NOFOLLOW_LINKS`, the exporter will skip the regular file.
   - If I omit `NOFOLLOW_LINKS`, the exporter will export the regular file.
   
   Is my understanding correct? If so, are you saying that it's best to ignore the regular file if the path we're testing is a symbolic link?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [geode] demery-pivotal merged pull request #5595: GEODE-8572: Make LogExporter not read dirs

Posted by GitBox <gi...@apache.org>.
demery-pivotal merged pull request #5595:
URL: https://github.com/apache/geode/pull/5595


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [geode] jchen21 commented on a change in pull request #5595: GEODE-8572: Make LogExporter not read dirs

Posted by GitBox <gi...@apache.org>.
jchen21 commented on a change in pull request #5595:
URL: https://github.com/apache/geode/pull/5595#discussion_r500475855



##########
File path: geode-gfsh/src/integrationTest/java/org/apache/geode/management/internal/cli/util/LogExporterIntegrationTest.java
##########
@@ -50,127 +48,146 @@
 
 @Category({GfshTest.class, LoggingTest.class})
 public class LogExporterIntegrationTest {
+  @Rule
+  public ServerStarterRule server = new ServerStarterRule();
 
   private final LogFilter filter = new LogFilter(Level.INFO, null, null);
 
-  private LogExporter logExporter;
-  private Properties properties;
-
-  @Rule
-  public ServerStarterRule server = new ServerStarterRule();
+  public Path serverFilesDir;
 
   @Before
-  public void before() {
-    properties = new Properties();
-    // make sure the server's working dir has no log files or stats file to begin with, since in
-    // some tests we are asserting on the # of log files and stats files created by the server
-    File workingDir = server.getWorkingDir();
-    Arrays.stream(workingDir.listFiles())
-        .filter(f -> (f.getName().endsWith(".log") || f.getName().endsWith(".gfs")))
-        .forEach(FileUtils::deleteQuietly);
+  public void createServerFilesDir() throws IOException {
+    // Name the directory after this test instance and the Gradle test worker, to ensure that tests
+    // running in parallel use different directories.
+    String testRunnerID = System.getProperty("org.gradle.test.worker", "standalone");
+    int testInstanceID = System.identityHashCode(this);
+    String className = getClass().getSimpleName();
+    String dirName = String.format("%s-%x-%s", className, testInstanceID, testRunnerID);
+    serverFilesDir = Files.createDirectories(Paths.get(dirName)).normalize().toAbsolutePath();
+  }
+
+  @After
+  public void deleteServerFilesDir() {
+    FileUtils.deleteQuietly(serverFilesDir.toFile());
   }
 
   @Test
   public void serverStartedWithWrongSuffix() throws Exception {
-    properties.setProperty(LOG_FILE, new File("test.txt").getAbsolutePath());
-    properties.setProperty(STATISTIC_ARCHIVE_FILE, "archive.archive");
-    server.withProperties(properties).startServer();
-    File serverWorkingDir = server.getWorkingDir();
-
-    logExporter = new LogExporter(filter, new File(serverWorkingDir, "test.log"),
-        new File(serverWorkingDir, "stats.gfs"));
-    List<Path> logFiles = logExporter.findLogFiles(serverWorkingDir.toPath());
-    assertThat(logFiles).isEmpty();
-
-    List<Path> statsFiles = logExporter.findStatFiles(serverWorkingDir.toPath());
-    assertThat(statsFiles).isEmpty();
+    String logFileNameWithWrongSuffix = "test.txt";
+    String statsFileNameWithWrongSuffix = "archive.archive";
+
+    Path logFile = serverFilesDir.resolve(logFileNameWithWrongSuffix);
+    Path statsFile = serverFilesDir.resolve(statsFileNameWithWrongSuffix);
+
+    server.withProperty(LOG_FILE, logFile.toString())
+        .withProperty(STATISTIC_ARCHIVE_FILE, statsFile.toString())
+        .startServer();
+
+    LogExporter logExporter = new LogExporter(filter, null, null);
+    List<Path> logFiles = logExporter.findLogFiles(serverFilesDir);
+
+    assertThat(logFiles)
+        .as("log files")
+        .isEmpty();
+
+    List<Path> statsFiles = logExporter.findStatFiles(serverFilesDir);
+    assertThat(statsFiles)
+        .as("stat files")
+        .isEmpty();
   }
 
   @Test
   public void serverStartedWithCorrectSuffix() throws Exception {
-    // ("relative log file is problematic in the test environment")
-    properties.setProperty(LOG_FILE, new File("test.log").getAbsolutePath());
-    properties.setProperty(STATISTIC_ARCHIVE_FILE, "archive.gfs");
-    server.withProperties(properties).startServer();
-    File serverWorkingDir = server.getWorkingDir();
-
-    logExporter = new LogExporter(filter, new File(serverWorkingDir, "test.log"),
-        new File(serverWorkingDir, "archive.gfs"));
-    List<Path> logFiles = logExporter.findLogFiles(serverWorkingDir.toPath());
-    assertThat(logFiles).hasSize(1);
-    assertThat(logFiles.get(0)).hasFileName("test.log");
-
-    List<Path> statsFiles = logExporter.findStatFiles(serverWorkingDir.toPath());
-    assertThat(statsFiles).hasSize(1);
-    assertThat(statsFiles.get(0)).hasFileName("archive.gfs");
+    String logFileName = "test.log";
+    String statsFileName = "archive.gfs";
+    Path logFile = serverFilesDir.resolve(logFileName);
+    Path statsFile = serverFilesDir.resolve(statsFileName);
+
+    server.withProperty(LOG_FILE, logFile.toString())
+        .withProperty(STATISTIC_ARCHIVE_FILE, statsFile.toString())
+        .startServer();
+
+    LogExporter logExporter = new LogExporter(filter, null, null);
+    List<Path> logFiles = logExporter.findLogFiles(serverFilesDir);
+
+    assertThat(logFiles)
+        .as("log files")
+        .hasSize(1);
+    assertThat(logFiles.get(0)).hasFileName(logFileName);
+
+    List<Path> statsFiles = logExporter.findStatFiles(serverFilesDir);
+    assertThat(statsFiles)
+        .as("stat files")
+        .hasSize(1);
+    assertThat(statsFiles.get(0)).hasFileName(statsFileName);
   }
 
   @Test
   @Ignore("GEODE-2574: fix .gz suffix")

Review comment:
       Not sure why this test is @Ignore. Any change to the test is not executed.

##########
File path: geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/util/LogExporter.java
##########
@@ -181,12 +180,13 @@ private long filterAndSize(Path originalLogFile) throws IOException {
   }
 
   private List<Path> findFiles(Path workingDir, Predicate<Path> fileSelector) throws IOException {
-    Stream<Path> selectedFiles/* = null */;
     if (!workingDir.toFile().isDirectory()) {
       return Collections.emptyList();
     }
-    selectedFiles = Files.list(workingDir).filter(fileSelector).filter(this.logFilter::acceptsFile);
-
-    return selectedFiles.collect(toList());
+    return Files.list(workingDir)
+        .filter(Files::isRegularFile)

Review comment:
       By default, this follows the symbolic link. Just make sure this is expected.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [geode] jchen21 commented on a change in pull request #5595: GEODE-8572: Make LogExporter not read dirs

Posted by GitBox <gi...@apache.org>.
jchen21 commented on a change in pull request #5595:
URL: https://github.com/apache/geode/pull/5595#discussion_r500642988



##########
File path: geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/util/LogExporter.java
##########
@@ -181,12 +180,13 @@ private long filterAndSize(Path originalLogFile) throws IOException {
   }
 
   private List<Path> findFiles(Path workingDir, Predicate<Path> fileSelector) throws IOException {
-    Stream<Path> selectedFiles/* = null */;
     if (!workingDir.toFile().isDirectory()) {
       return Collections.emptyList();
     }
-    selectedFiles = Files.list(workingDir).filter(fileSelector).filter(this.logFilter::acceptsFile);
-
-    return selectedFiles.collect(toList());
+    return Files.list(workingDir)
+        .filter(Files::isRegularFile)

Review comment:
       If you use `NOFOLLOW_LINKS`, for the scenario, `Files::isRegularFile` returns `false`. 
   If you omit `NOFOLLOW_LINKS`, `Files::isRegularFile` returns `true`. 
   I haven't tried it with the exporter though.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [geode] jchen21 commented on a change in pull request #5595: GEODE-8572: Make LogExporter not read dirs

Posted by GitBox <gi...@apache.org>.
jchen21 commented on a change in pull request #5595:
URL: https://github.com/apache/geode/pull/5595#discussion_r500657383



##########
File path: geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/util/LogExporter.java
##########
@@ -181,12 +180,13 @@ private long filterAndSize(Path originalLogFile) throws IOException {
   }
 
   private List<Path> findFiles(Path workingDir, Predicate<Path> fileSelector) throws IOException {
-    Stream<Path> selectedFiles/* = null */;
     if (!workingDir.toFile().isDirectory()) {
       return Collections.emptyList();
     }
-    selectedFiles = Files.list(workingDir).filter(fileSelector).filter(this.logFilter::acceptsFile);
-
-    return selectedFiles.collect(toList());
+    return Files.list(workingDir)
+        .filter(Files::isRegularFile)

Review comment:
       No, I am not saying I want to ignore the file it links to. Actually, I am not sure when it should follow the link, or when it should not. Based on my understanding, most likely, it should follow the link. You probably have more context than I have. What I am trying to say is that `Files::isRegularFile` has an option `NOFOLLOW_LINKS` for symbolic link. And symbolic link could be a tricky case. As long as it works for the scenario as you expect, it should be good.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org