From 8ccf7ee489fccaeafbaf49a210f41d188922b113 Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Mon, 31 Aug 2020 13:21:30 -0700 Subject: [PATCH] Make file detection more resilient across restarts Retain file snapshot state across devtools restarts to help prevent detection failures. Closes gh-19543 --- .../LocalDevToolsAutoConfiguration.java | 3 +- .../devtools/filewatch/FileSystemWatcher.java | 44 ++++++++++--- .../filewatch/SnapshotStateRepository.java | 62 +++++++++++++++++++ .../StaticSnapshotStateRepository.java | 40 ++++++++++++ .../filewatch/FileSystemWatcherTests.java | 47 +++++++++++++- 5 files changed, 187 insertions(+), 9 deletions(-) create mode 100644 spring-boot-project/spring-boot-devtools/src/main/java/org/springframework/boot/devtools/filewatch/SnapshotStateRepository.java create mode 100644 spring-boot-project/spring-boot-devtools/src/main/java/org/springframework/boot/devtools/filewatch/StaticSnapshotStateRepository.java diff --git a/spring-boot-project/spring-boot-devtools/src/main/java/org/springframework/boot/devtools/autoconfigure/LocalDevToolsAutoConfiguration.java b/spring-boot-project/spring-boot-devtools/src/main/java/org/springframework/boot/devtools/autoconfigure/LocalDevToolsAutoConfiguration.java index 30c57271d1a..51a7a7809bd 100644 --- a/spring-boot-project/spring-boot-devtools/src/main/java/org/springframework/boot/devtools/autoconfigure/LocalDevToolsAutoConfiguration.java +++ b/spring-boot-project/spring-boot-devtools/src/main/java/org/springframework/boot/devtools/autoconfigure/LocalDevToolsAutoConfiguration.java @@ -31,6 +31,7 @@ import org.springframework.boot.devtools.classpath.ClassPathRestartStrategy; import org.springframework.boot.devtools.classpath.PatternClassPathRestartStrategy; import org.springframework.boot.devtools.filewatch.FileSystemWatcher; import org.springframework.boot.devtools.filewatch.FileSystemWatcherFactory; +import org.springframework.boot.devtools.filewatch.SnapshotStateRepository; import org.springframework.boot.devtools.livereload.LiveReloadServer; import org.springframework.boot.devtools.restart.ConditionalOnInitializedRestarter; import org.springframework.boot.devtools.restart.RestartScope; @@ -141,7 +142,7 @@ public class LocalDevToolsAutoConfiguration { private FileSystemWatcher newFileSystemWatcher() { Restart restartProperties = this.properties.getRestart(); FileSystemWatcher watcher = new FileSystemWatcher(true, restartProperties.getPollInterval(), - restartProperties.getQuietPeriod()); + restartProperties.getQuietPeriod(), SnapshotStateRepository.STATIC); String triggerFile = restartProperties.getTriggerFile(); if (StringUtils.hasLength(triggerFile)) { watcher.setTriggerFilter(new TriggerFileFilter(triggerFile)); diff --git a/spring-boot-project/spring-boot-devtools/src/main/java/org/springframework/boot/devtools/filewatch/FileSystemWatcher.java b/spring-boot-project/spring-boot-devtools/src/main/java/org/springframework/boot/devtools/filewatch/FileSystemWatcher.java index 06449c5e8b7..99e90802ecc 100644 --- a/spring-boot-project/spring-boot-devtools/src/main/java/org/springframework/boot/devtools/filewatch/FileSystemWatcher.java +++ b/spring-boot-project/spring-boot-devtools/src/main/java/org/springframework/boot/devtools/filewatch/FileSystemWatcher.java @@ -54,6 +54,8 @@ public class FileSystemWatcher { private final long quietPeriod; + private final SnapshotStateRepository snapshotStateRepository; + private final AtomicInteger remainingScans = new AtomicInteger(-1); private final Map directories = new HashMap<>(); @@ -79,6 +81,20 @@ public class FileSystemWatcher { * ensure that updates have completed */ public FileSystemWatcher(boolean daemon, Duration pollInterval, Duration quietPeriod) { + this(daemon, pollInterval, quietPeriod, null); + } + + /** + * Create a new {@link FileSystemWatcher} instance. + * @param daemon if a daemon thread used to monitor changes + * @param pollInterval the amount of time to wait between checking for changes + * @param quietPeriod the amount of time required after a change has been detected to + * ensure that updates have completed + * @param snapshotStateRepository the snapshot state repository + * @since 2.4.0 + */ + public FileSystemWatcher(boolean daemon, Duration pollInterval, Duration quietPeriod, + SnapshotStateRepository snapshotStateRepository) { Assert.notNull(pollInterval, "PollInterval must not be null"); Assert.notNull(quietPeriod, "QuietPeriod must not be null"); Assert.isTrue(pollInterval.toMillis() > 0, "PollInterval must be positive"); @@ -88,6 +104,8 @@ public class FileSystemWatcher { this.daemon = daemon; this.pollInterval = pollInterval.toMillis(); this.quietPeriod = quietPeriod.toMillis(); + this.snapshotStateRepository = (snapshotStateRepository != null) ? snapshotStateRepository + : SnapshotStateRepository.NONE; } /** @@ -150,11 +168,12 @@ public class FileSystemWatcher { */ public void start() { synchronized (this.monitor) { - saveInitialSnapshots(); + createOrRestoreInitialSnapshots(); if (this.watchThread == null) { Map localDirectories = new HashMap<>(this.directories); - this.watchThread = new Thread(new Watcher(this.remainingScans, new ArrayList<>(this.listeners), - this.triggerFilter, this.pollInterval, this.quietPeriod, localDirectories)); + Watcher watcher = new Watcher(this.remainingScans, new ArrayList<>(this.listeners), this.triggerFilter, + this.pollInterval, this.quietPeriod, localDirectories, this.snapshotStateRepository); + this.watchThread = new Thread(watcher); this.watchThread.setName("File Watcher"); this.watchThread.setDaemon(this.daemon); this.watchThread.start(); @@ -162,8 +181,13 @@ public class FileSystemWatcher { } } - private void saveInitialSnapshots() { - this.directories.replaceAll((f, v) -> new DirectorySnapshot(f)); + @SuppressWarnings("unchecked") + private void createOrRestoreInitialSnapshots() { + Map restored = (Map) this.snapshotStateRepository.restore(); + this.directories.replaceAll((f, v) -> { + DirectorySnapshot restoredSnapshot = (restored != null) ? restored.get(f) : null; + return (restoredSnapshot != null) ? restoredSnapshot : new DirectorySnapshot(f); + }); } /** @@ -213,14 +237,19 @@ public class FileSystemWatcher { private Map directories; + private SnapshotStateRepository snapshotStateRepository; + private Watcher(AtomicInteger remainingScans, List listeners, FileFilter triggerFilter, - long pollInterval, long quietPeriod, Map directories) { + long pollInterval, long quietPeriod, Map directories, + SnapshotStateRepository snapshotStateRepository) { this.remainingScans = remainingScans; this.listeners = listeners; this.triggerFilter = triggerFilter; this.pollInterval = pollInterval; this.quietPeriod = quietPeriod; this.directories = directories; + this.snapshotStateRepository = snapshotStateRepository; + } @Override @@ -288,10 +317,11 @@ public class FileSystemWatcher { changeSet.add(changedFiles); } } + this.directories = updated; + this.snapshotStateRepository.save(updated); if (!changeSet.isEmpty()) { fireListeners(Collections.unmodifiableSet(changeSet)); } - this.directories = updated; } private void fireListeners(Set changeSet) { diff --git a/spring-boot-project/spring-boot-devtools/src/main/java/org/springframework/boot/devtools/filewatch/SnapshotStateRepository.java b/spring-boot-project/spring-boot-devtools/src/main/java/org/springframework/boot/devtools/filewatch/SnapshotStateRepository.java new file mode 100644 index 00000000000..8a0b88c0f72 --- /dev/null +++ b/spring-boot-project/spring-boot-devtools/src/main/java/org/springframework/boot/devtools/filewatch/SnapshotStateRepository.java @@ -0,0 +1,62 @@ +/* + * Copyright 2012-2020 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.boot.devtools.filewatch; + +/** + * Repository used by {@link FileSystemWatcher} to save file/directory snapshots across + * restarts. + * + * @author Phillip Webb + * @since 2.4.0 + */ +public interface SnapshotStateRepository { + + /** + * A No-op {@link SnapshotStateRepository} that does not save state. + */ + SnapshotStateRepository NONE = new SnapshotStateRepository() { + + @Override + public void save(Object state) { + } + + @Override + public Object restore() { + return null; + } + + }; + + /** + * A {@link SnapshotStateRepository} that uses a static instance to keep state across + * restarts. + */ + SnapshotStateRepository STATIC = StaticSnapshotStateRepository.INSTANCE; + + /** + * Save the given state in the repository. + * @param state the state to save + */ + void save(Object state); + + /** + * Restore any previously saved state. + * @return the previously saved state or {@code null} + */ + Object restore(); + +} diff --git a/spring-boot-project/spring-boot-devtools/src/main/java/org/springframework/boot/devtools/filewatch/StaticSnapshotStateRepository.java b/spring-boot-project/spring-boot-devtools/src/main/java/org/springframework/boot/devtools/filewatch/StaticSnapshotStateRepository.java new file mode 100644 index 00000000000..69f5534d334 --- /dev/null +++ b/spring-boot-project/spring-boot-devtools/src/main/java/org/springframework/boot/devtools/filewatch/StaticSnapshotStateRepository.java @@ -0,0 +1,40 @@ +/* + * Copyright 2012-2020 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.boot.devtools.filewatch; + +/** + * {@link SnapshotStateRepository} that uses a single static instance. + * + * @author Phillip Webb + */ +class StaticSnapshotStateRepository implements SnapshotStateRepository { + + static final StaticSnapshotStateRepository INSTANCE = new StaticSnapshotStateRepository(); + + private volatile Object state; + + @Override + public void save(Object state) { + this.state = state; + } + + @Override + public Object restore() { + return this.state; + } + +} diff --git a/spring-boot-project/spring-boot-devtools/src/test/java/org/springframework/boot/devtools/filewatch/FileSystemWatcherTests.java b/spring-boot-project/spring-boot-devtools/src/test/java/org/springframework/boot/devtools/filewatch/FileSystemWatcherTests.java index c213312dcc9..51fbd20b4e2 100644 --- a/spring-boot-project/spring-boot-devtools/src/test/java/org/springframework/boot/devtools/filewatch/FileSystemWatcherTests.java +++ b/spring-boot-project/spring-boot-devtools/src/test/java/org/springframework/boot/devtools/filewatch/FileSystemWatcherTests.java @@ -273,8 +273,37 @@ class FileSystemWatcherTests { assertThat(actual).isEqualTo(expected); } + @Test + void withSnapshotRepository() throws Exception { + SnapshotStateRepository repository = new TestSnapshotStateRepository(); + setupWatcher(20, 10, repository); + File directory = new File(this.tempDir, UUID.randomUUID().toString()); + directory.mkdir(); + File file = touch(new File(directory, "file.txt")); + this.watcher.addSourceDirectory(directory); + this.watcher.start(); + file.delete(); + this.watcher.stopAfter(1); + this.changes.clear(); + File recreate = touch(new File(directory, "file.txt")); + setupWatcher(20, 10, repository); + this.watcher.addSourceDirectory(directory); + this.watcher.start(); + this.watcher.stopAfter(1); + ChangedFiles changedFiles = getSingleChangedFiles(); + Set actual = changedFiles.getFiles(); + Set expected = new HashSet<>(); + expected.add(new ChangedFile(directory, recreate, Type.ADD)); + assertThat(actual).isEqualTo(expected); + } + private void setupWatcher(long pollingInterval, long quietPeriod) { - this.watcher = new FileSystemWatcher(false, Duration.ofMillis(pollingInterval), Duration.ofMillis(quietPeriod)); + setupWatcher(pollingInterval, quietPeriod, null); + } + + private void setupWatcher(long pollingInterval, long quietPeriod, SnapshotStateRepository snapshotStateRepository) { + this.watcher = new FileSystemWatcher(false, Duration.ofMillis(pollingInterval), Duration.ofMillis(quietPeriod), + snapshotStateRepository); this.watcher.addListener((changeSet) -> FileSystemWatcherTests.this.changes.add(changeSet)); } @@ -304,4 +333,20 @@ class FileSystemWatcherTests { return file; } + private static class TestSnapshotStateRepository implements SnapshotStateRepository { + + private Object state; + + @Override + public void save(Object state) { + this.state = state; + } + + @Override + public Object restore() { + return this.state; + } + + } + }