Skip to content

Commit

Permalink
GEODE-8572: Make LogExporter not read dirs (apache#5595)
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@demery-pivotal
demery-pivotal authored Oct 7, 2020
1 parent 8f4566c commit dbd1803
Show file tree
Hide file tree
Showing 3 changed files with 125 additions and 98 deletions.
10 changes: 10 additions & 0 deletions ...st/java/org/apache/geode/management/internal/cli/util/LogExporterFileIntegrationTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,9 @@
import java.io.BufferedReader;
import java.io.File;
import java.io.FileReader;
import java.io.IOException;
import java.nio.charset.Charset;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.Comparator;
import java.util.List;
Expand Down Expand Up @@ -152,4 +154,12 @@ public void findLogsWhichContainsTheWordLog() throws Exception {
assertThat(logExporter.findLogFiles(workingDir.toPath()))
.contains(gcRolledOverLogFile.toPath());
}

@Test
public void ignoresDirectoriesWhoseNamesMatchLogAndStatFilePredicates() throws IOException {
Files.createDirectories(workingDir.toPath().resolve("should-be-ignored.log"));
Files.createDirectories(workingDir.toPath().resolve("should-be-ignored.gfs"));

assertThat(logExporter.export()).isNull();
}
}
203 changes: 110 additions & 93 deletions ...onTest/java/org/apache/geode/management/internal/cli/util/LogExporterIntegrationTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,21 +18,19 @@
import static org.apache.geode.distributed.ConfigurationProperties.STATISTIC_ARCHIVE_FILE;
import static org.assertj.core.api.Assertions.assertThat;

import java.io.File;
import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.Arrays;
import java.nio.file.Paths;
import java.util.List;
import java.util.Properties;
import java.util.Set;
import java.util.stream.Collectors;
import java.util.zip.ZipEntry;
import java.util.zip.ZipFile;

import com.google.common.collect.Sets;
import org.apache.commons.io.FileUtils;
import org.apache.logging.log4j.Level;
import org.junit.After;
import org.junit.Before;
import org.junit.Ignore;
import org.junit.Rule;
Expand All @@ -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")
public void serverStartedWithGZSuffix() throws Exception {
properties.setProperty(LOG_FILE, "test.log.gz");
properties.setProperty(STATISTIC_ARCHIVE_FILE, "archive.gfs.gz");
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).hasSize(1);

List<Path> statsFiles = logExporter.findStatFiles(serverWorkingDir.toPath());
assertThat(statsFiles).hasSize(1);
}
Path gzLogFile = serverFilesDir.resolve("test.log.gz");
Path gzStatsFile = serverFilesDir.resolve("archive.gfs.gz");

@Test
public void testNoStatsFile() throws Throwable {
Path logsFile = Files.createTempFile("server", ".log");
properties.setProperty(LOG_FILE, logsFile.toString());
server.withProperties(properties).startServer();
server.withProperty(LOG_FILE, gzLogFile.toString())
.withProperty(STATISTIC_ARCHIVE_FILE, gzStatsFile.toString())
.startServer();

verifyExportLogsFunctionDoesNotBlowUp(server.getCache());
}
LogExporter logExporter = new LogExporter(filter, null, null);
List<Path> logFiles = logExporter.findLogFiles(serverFilesDir);

@Test
public void testWithRelativeStatsFile() throws Throwable {
Path logsFile = Files.createTempFile("server", ".log");
// Path statsFile = Files.createTempFile("stats", ".gfs");
properties.setProperty(LOG_FILE, logsFile.toString());
properties.setProperty(STATISTIC_ARCHIVE_FILE, "stats.gfs");
server.withProperties(properties).startServer();
assertThat(logFiles)
.as("log files")
.hasSize(1);

verifyExportLogsFunctionDoesNotBlowUp(server.getCache());
List<Path> statsFiles = logExporter.findStatFiles(serverFilesDir);
assertThat(statsFiles)
.as("stats files")
.hasSize(1);
}

@Test
public void testWithRelativeLogsFile() throws Throwable {
Path statsFile = Files.createTempFile("stats", ".gfs");
properties.setProperty(LOG_FILE, "sever.log");
properties.setProperty(STATISTIC_ARCHIVE_FILE, statsFile.toString());
server.withProperties(properties).startServer();
public void testNoStatsFile() throws Throwable {
Path logFile = serverFilesDir.resolve("server.log");

server.withProperty(LOG_FILE, logFile.toString())
.startServer();

verifyExportLogsFunctionDoesNotBlowUp(server.getCache());
}

@Test
public void testWithAbsoluteLogsStatsFile() throws Exception {
File logsDir = Files.createTempDirectory("logs").toFile();
File statsDir = Files.createTempDirectory("stats").toFile();

File logFile = new File(logsDir, "server.log");
File statsFile = new File(statsDir, "stats.gfs");

properties.setProperty(LOG_FILE, logFile.getAbsolutePath());
properties.setProperty(STATISTIC_ARCHIVE_FILE, statsFile.getAbsolutePath());
public void testWithRelativeFilePaths() throws Throwable {
Path serverWorkingDir = server.getWorkingDir().toPath().normalize().toAbsolutePath();
Path relativeLogFile = serverWorkingDir.relativize(serverFilesDir.resolve("server.log"));
Path relativeStatsFile = serverWorkingDir.relativize(serverFilesDir.resolve("stats.gfs"));

server.withProperties(properties).startServer();
server.withProperty(LOG_FILE, relativeLogFile.toString())
.withProperty(STATISTIC_ARCHIVE_FILE, relativeStatsFile.toString())
.startServer();

logExporter = new LogExporter(filter, logFile, statsFile);
Path exportedZip = logExporter.export();
Set<String> actualFiles = getZipEntries(exportedZip.toString());
Set<String> expectedFiles = Sets.newHashSet("server.log", "stats.gfs");
verifyExportLogsFunctionDoesNotBlowUp(server.getCache());
}

assertThat(actualFiles).isEqualTo(expectedFiles);
@Test
public void testWithAbsoluteFilePaths() throws Exception {
String logFileName = "server.log";
String statsFileName = "stats.gfs";
Path absoluteLogFile = serverFilesDir.resolve("logs").resolve(logFileName).toAbsolutePath();
Path absoluteStatsFile =
serverFilesDir.resolve("stats").resolve(statsFileName).toAbsolutePath();
Files.createDirectories(absoluteLogFile.getParent());
Files.createDirectories(absoluteLogFile.getParent());

server.withProperty(LOG_FILE, absoluteLogFile.toString())
.withProperty(STATISTIC_ARCHIVE_FILE, absoluteStatsFile.toString())
.startServer();

LogExporter logExporter =
new LogExporter(filter, absoluteLogFile.toFile(), absoluteStatsFile.toFile());
String exportedZip = logExporter.export().toString();

assertThat(zipEntriesIn(exportedZip))
.containsExactlyInAnyOrder(logFileName, statsFileName);
}

private static void verifyExportLogsFunctionDoesNotBlowUp(Cache cache) throws Throwable {
Expand All @@ -186,7 +203,7 @@ private static void verifyExportLogsFunctionDoesNotBlowUp(Cache cache) throws Th
}
}

private static Set<String> getZipEntries(String zipFilePath) throws IOException {
private static Set<String> zipEntriesIn(String zipFilePath) throws IOException {
return new ZipFile(zipFilePath).stream().map(ZipEntry::getName).collect(Collectors.toSet());
}

Expand Down
10 changes: 5 additions & 5 deletions geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/util/LogExporter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@
import java.util.Collections;
import java.util.List;
import java.util.function.Predicate;
import java.util.stream.Stream;

import org.apache.commons.io.FileUtils;
import org.apache.logging.log4j.Logger;
Expand Down Expand Up @@ -181,12 +180,13 @@ List<Path> findStatFiles(Path workingDir) 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)
.filter(fileSelector)
.filter(this.logFilter::acceptsFile)
.collect(toList());
}
}

0 comments on commit dbd1803

Please sign in to comment.