From 3679fc9ac3d3bf8e5aa0328aea311cf1e5a703cd Mon Sep 17 00:00:00 2001 From: Ties de Kock Date: Fri, 8 Dec 2023 10:58:03 +0100 Subject: [PATCH] Actually test directory name generation --- .../ripe/rpki/rsyncit/config/AppConfig.java | 5 ++- .../net/ripe/rpki/rsyncit/config/Config.java | 3 +- .../ripe/rpki/rsyncit/rsync/RsyncWriter.java | 20 +++++---- src/test/java/net/ripe/rpki/TestDefaults.java | 3 +- .../rpki/rsyncit/rsync/RsyncWriterTest.java | 45 ++++++++++++------- 5 files changed, 48 insertions(+), 28 deletions(-) diff --git a/src/main/java/net/ripe/rpki/rsyncit/config/AppConfig.java b/src/main/java/net/ripe/rpki/rsyncit/config/AppConfig.java index 75be4ef..327d91d 100644 --- a/src/main/java/net/ripe/rpki/rsyncit/config/AppConfig.java +++ b/src/main/java/net/ripe/rpki/rsyncit/config/AppConfig.java @@ -11,6 +11,7 @@ import org.springframework.context.annotation.Bean; import org.springframework.stereotype.Component; +import java.nio.file.Path; import java.time.Duration; import java.util.Map; import java.util.function.Function; @@ -21,7 +22,7 @@ public class AppConfig implements InfoContributor { private final String rrdpUrl; private final String rrdpReplaceHostWith; - private final String rsyncPath; + private final Path rsyncPath; private final String cron; private final Duration requestTimeout; private final ApplicationInfo info; @@ -30,7 +31,7 @@ public class AppConfig implements InfoContributor { public AppConfig(@Value("${rrdpUrl}") String rrdpUrl, @Value("${rrdpReplaceHost:}") String rrdpReplaceHostWith, - @Value("${rsyncPath}") String rsyncPath, + @Value("${rsyncPath}") Path rsyncPath, // Run every 10 minutes @Value("${cron:0 0/10 * * * ?}") String cron, // 3 minutes by default diff --git a/src/main/java/net/ripe/rpki/rsyncit/config/Config.java b/src/main/java/net/ripe/rpki/rsyncit/config/Config.java index 32fa5a5..5c28667 100644 --- a/src/main/java/net/ripe/rpki/rsyncit/config/Config.java +++ b/src/main/java/net/ripe/rpki/rsyncit/config/Config.java @@ -2,6 +2,7 @@ import lombok.With; +import java.nio.file.Path; import java.time.Duration; import java.util.function.Function; @@ -9,7 +10,7 @@ public record Config( String rrdpUrl, Function substituteHost, - String rsyncPath, + Path rsyncPath, String cron, Duration requestTimeout, long targetDirectoryRetentionPeriodMs, diff --git a/src/main/java/net/ripe/rpki/rsyncit/rsync/RsyncWriter.java b/src/main/java/net/ripe/rpki/rsyncit/rsync/RsyncWriter.java index a9c6ba0..ef4d9da 100644 --- a/src/main/java/net/ripe/rpki/rsyncit/rsync/RsyncWriter.java +++ b/src/main/java/net/ripe/rpki/rsyncit/rsync/RsyncWriter.java @@ -6,7 +6,6 @@ import net.ripe.rpki.rsyncit.rrdp.RpkiObject; import org.apache.tomcat.util.http.fileupload.FileUtils; -import java.io.File; import java.io.IOException; import java.io.UncheckedIOException; import java.net.URI; @@ -55,10 +54,9 @@ public RsyncWriter(Config config) { public Path writeObjects(List objects) { try { final Instant now = Instant.now(); - var baseDirectory = Paths.get(config.rsyncPath()); final Path targetDirectory = writeObjectToNewDirectory(objects, now); - atomicallyReplacePublishedSymlink(Paths.get(config.rsyncPath()), targetDirectory); - cleanupOldTargetDirectories(now, baseDirectory); + atomicallyReplacePublishedSymlink(config.rsyncPath(), targetDirectory); + cleanupOldTargetDirectories(now, config.rsyncPath()); return targetDirectory; } catch (Exception e) { throw new RuntimeException(e); @@ -73,9 +71,7 @@ private Path writeObjectToNewDirectory(List objects, Instant now) th final Map> groupedByHost = objects.stream().collect(Collectors.groupingBy(o -> o.url().getHost())); - final String formattedNow = DateTimeFormatter.ISO_LOCAL_DATE_TIME.withZone(ZoneId.of("UTC")).format(now); - - final Path temporaryDirectory = Files.createTempDirectory(Paths.get(config.rsyncPath()), "tmp-" + formattedNow + "-"); + final Path temporaryDirectory = Files.createTempDirectory(config.rsyncPath(), "rsync-writer-tmp"); try { groupedByHost.forEach((hostName, os) -> { // create a directory per hostname (in realistic cases there will be just one) @@ -131,8 +127,8 @@ private Path writeObjectToNewDirectory(List objects, Instant now) th hostName); }); - // Calculate target directory after writing phase, to be sure variable is not used beforehand. - final Path targetDirectory = Paths.get(config.rsyncPath()).resolve("published-" + formattedNow); + // Calculate target directory after writing phase, to be sure it is not used beforehand. + final Path targetDirectory = generatePublicationDirectoryPath(config.rsyncPath(), now); // Directory write is fully complete, rename temporary to target directory name Files.setLastModifiedTime(temporaryDirectory, FileTime.from(now)); @@ -148,6 +144,12 @@ private Path writeObjectToNewDirectory(List objects, Instant now) th } } + static Path generatePublicationDirectoryPath(Path baseDir, Instant now) { + var timeSegment = DateTimeFormatter.ISO_LOCAL_DATE_TIME.withZone(ZoneId.of("UTC")).format(now); + + return baseDir.resolve("published-" + timeSegment); + } + static List filterOutBadUrls(Path hostBasedPath, Collection objects) { final String normalizedHostPath = hostBasedPath.normalize().toString(); return objects.stream().flatMap(object -> { diff --git a/src/test/java/net/ripe/rpki/TestDefaults.java b/src/test/java/net/ripe/rpki/TestDefaults.java index 53b8fc0..7a0b56d 100644 --- a/src/test/java/net/ripe/rpki/TestDefaults.java +++ b/src/test/java/net/ripe/rpki/TestDefaults.java @@ -3,6 +3,7 @@ import net.ripe.rpki.rsyncit.config.Config; import org.springframework.web.reactive.function.client.WebClient; +import java.nio.file.Paths; import java.time.Duration; import java.time.temporal.ChronoUnit; import java.util.function.Function; @@ -11,7 +12,7 @@ public class TestDefaults { public static Config defaultConfig() { return new Config("https://rrdp.ripe.net/notification.xml", Function.identity(), - "/tmp/rsync", + Paths.get("/tmp/rsync"), "0 0/10 * * * ?", Duration.of(1, ChronoUnit.MINUTES), 3600_000, 10); diff --git a/src/test/java/net/ripe/rpki/rsyncit/rsync/RsyncWriterTest.java b/src/test/java/net/ripe/rpki/rsyncit/rsync/RsyncWriterTest.java index 0728f05..82b6ab2 100644 --- a/src/test/java/net/ripe/rpki/rsyncit/rsync/RsyncWriterTest.java +++ b/src/test/java/net/ripe/rpki/rsyncit/rsync/RsyncWriterTest.java @@ -14,6 +14,7 @@ import java.time.Instant; import java.time.ZoneId; import java.time.format.DateTimeFormatter; +import java.time.temporal.ChronoUnit; import java.util.Arrays; import java.util.Collections; import java.util.Random; @@ -30,10 +31,24 @@ class RsyncWriterTest { @Test - public void testDateFormat() { + public void testPublicationDirectoryNaming(@TempDir Path tempDir) { Instant now = Instant.now(); - final String formattedNow = DateTimeFormatter.ISO_LOCAL_DATE_TIME.withZone(ZoneId.of("UTC")).format(now); - assertNotNull(formattedNow); + Function format = then -> DateTimeFormatter.ISO_LOCAL_DATE_TIME.withZone(ZoneId.of("UTC")).format(then); + + var d1 = RsyncWriter.generatePublicationDirectoryPath(tempDir, now); + var d2 = RsyncWriter.generatePublicationDirectoryPath(tempDir, now.plus(25, ChronoUnit.HOURS)); + + assertThat(d1.startsWith(tempDir)); + assertThat(d1).isNotEqualTo(d2); + // sibling dirs + assertThat(d1.getParent()).isEqualTo(d2.getParent()); + + assertThat(d1.toString()) + .contains("published-") + .contains(format.apply(now)); + + assertThat(d2.toString()) + .contains(format.apply(now.plus(25, ChronoUnit.HOURS))); } @Test @@ -59,13 +74,13 @@ public void testWriteMultipleObjects(@TempDir Path tmpPath) throws Exception { var o5 = new RpkiObject(URI.create("rsync://bla.net/path2/nested1/nested2/e.cer"), someBytes(), Instant.now()); var o6 = new RpkiObject(URI.create("rsync://different.net/path2/nested1/nested2/e.cer"), someBytes(), Instant.now()); rsyncWriter.writeObjects(Arrays.asList(o1, o2, o3, o4, o5, o6)); - final String root = rsyncWriter.getConfig().rsyncPath(); - checkFile(Paths.get(root, "published", "bla.net", "path1", "a.cer"), o1.bytes()); - checkFile(Paths.get(root, "published", "bla.net", "path1", "b.cer"), o2.bytes()); - checkFile(Paths.get(root, "published", "bla.net", "path1", "nested", "c.cer"), o3.bytes()); - checkFile(Paths.get(root, "published", "bla.net", "path2", "d.cer"), o4.bytes()); - checkFile(Paths.get(root, "published", "bla.net", "path2", "nested1", "nested2", "e.cer"), o5.bytes()); - checkFile(Paths.get(root, "published", "different.net", "path2", "nested1", "nested2", "e.cer"), o6.bytes()); + var root = rsyncWriter.getConfig().rsyncPath(); + checkFile(root.resolve("published/bla.net/path1/a.cer"), o1.bytes()); + checkFile(root.resolve("published/bla.net/path1/b.cer"), o2.bytes()); + checkFile(root.resolve("published/bla.net/path1/nested/c.cer"), o3.bytes()); + checkFile(root.resolve("published/bla.net/path2/d.cer"), o4.bytes()); + checkFile(root.resolve("published/bla.net/path2/nested1/nested2/e.cer"), o5.bytes()); + checkFile(root.resolve("published/different.net/path2/nested1/nested2/e.cer"), o6.bytes()); }); } @@ -76,10 +91,10 @@ public void testIgnoreBadUrls(@TempDir Path tmpPath) throws Exception { var o2 = new RpkiObject(URI.create("rsync://bla.net/path1/../../PATH_INJECTION.txt"), someBytes(), Instant.now()); var o3 = new RpkiObject(URI.create("rsync://bla.net/path1/path2/../NOT_REALLY_PATH_INJECTION.txt"), someBytes(), Instant.now()); rsyncWriter.writeObjects(Arrays.asList(o1, o2, o3)); - final String root = rsyncWriter.getConfig().rsyncPath(); - checkFile(Paths.get(root, "published", "bla.net", "path1", "a.cer"), o1.bytes()); - assertThat(Paths.get(root, "published", "PATH_INJECTION.txt").toFile().exists()).isFalse(); - checkFile(Paths.get(root, "published", "bla.net", "path1", "NOT_REALLY_PATH_INJECTION.txt"), o3.bytes()); + var root = rsyncWriter.getConfig().rsyncPath(); + checkFile(root.resolve("published/bla.net/path1/a.cer"), o1.bytes()); + assertThat(root.resolve( "published/PATH_INJECTION.txt").toFile().exists()).isFalse(); + checkFile(root.resolve( "published/bla.net/path1/NOT_REALLY_PATH_INJECTION.txt"), o3.bytes()); }); } @@ -145,7 +160,7 @@ private static void checkFile(Path path, byte[] bytes) throws IOException { } static void withRsyncWriter(Path tmpPath, Function transformConfig, ThrowingConsumer actualTest) throws Exception { - final Config config = defaultConfig().withRsyncPath(tmpPath.toString()); + final Config config = defaultConfig().withRsyncPath(tmpPath); actualTest.accept(new RsyncWriter(transformConfig.apply(config))); }