Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Only update file time once #16707

Merged
merged 1 commit into from
Apr 27, 2021
Merged

Only update file time once #16707

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 53 additions & 12 deletions test-framework/junit5-internal/src/main/java/io/quarkus/test/QuarkusDevModeTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -539,11 +539,13 @@ public void modifyTestSourceFile(Class<?> sourceFile, Function<String, String> m
* @param sourceFile
*/
public void addSourceFile(Class<?> sourceFile) {
Path path = copySourceFilesForClass(projectSourceRoot, deploymentSourcePath, testLocation,
Path path = findTargetSourceFilesForPath(projectSourceRoot, deploymentSourcePath, testLocation,
testLocation.resolve(sourceFile.getName().replace(".", "/") + ".class"));
long old = modTime(path.getParent());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

findTargetSourceFilesForPath can return null and you don't test it AFAICS. Maybe it's a corner case but it seems to be an open door for NPEs.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The original copySourceFilesForClass could also return null. I will make them throw an exception instead.

copySourceFilesForClass(projectSourceRoot, deploymentSourcePath, testLocation,
testLocation.resolve(sourceFile.getName().replace(".", "/") + ".class"));
sleepForFileChanges(path);
// since this is a new file addition, even wait for the parent dir's last modified timestamp to change
sleepForFileChanges(path.getParent());
sleepForFileChanges(path.getParent(), old);
}

public String[] getCommandLineArgs() {
Expand Down Expand Up @@ -575,6 +577,8 @@ void modifyFile(String name, Function<String, String> mutator, Path path) {

private void modifyPath(Function<String, String> mutator, Path sourceDirectory, Path input) {
try {
long old = modTime(input);
long oldSrc = modTime(sourceDirectory);
byte[] data;
try (InputStream in = Files.newInputStream(input)) {
data = FileUtil.readFileContents(in);
Expand All @@ -586,23 +590,31 @@ private void modifyPath(Function<String, String> mutator, Path sourceDirectory,
throw new RuntimeException("File was not modified, mutator function had no effect");
}

sleepForFileChanges(sourceDirectory);
Files.write(input, content.getBytes(StandardCharsets.UTF_8));
sleepForFileChanges(sourceDirectory, oldSrc);
sleepForFileChanges(input, old);
} catch (IOException e) {
throw new UncheckedIOException(e);
}
}

public void sleepForFileChanges(Path path) {
private void sleepForFileChanges(Path path, long oldTime) {
try {
//we avoid modifying the file twice
//this can cause intermittent failures in the continous testing tests
long fm = modTime(path);
if (fm > oldTime) {
return;
}
//we want to make sure the last modified time is larger than both the current time
//and the current last modified time. Some file systems only resolve file
//time to the nearest second, so this is necessary for dev mode to pick up the changes
long timeToBeat = Math.max(System.currentTimeMillis(), Files.getLastModifiedTime(path).toMillis());
long timeToBeat = Math.max(System.currentTimeMillis(), modTime(path));
for (;;) {
Thread.sleep(1000);
Files.setLastModifiedTime(path, FileTime.fromMillis(System.currentTimeMillis()));
long fm = Files.getLastModifiedTime(path).toMillis();
Thread.sleep(10);
fm = modTime(path);
Thread.sleep(100);
if (fm > timeToBeat) {
return;
}
Expand All @@ -612,13 +624,22 @@ public void sleepForFileChanges(Path path) {
}
}

private long modTime(Path path) {
try {
return Files.getLastModifiedTime(path).toMillis();
} catch (IOException e) {
throw new RuntimeException(e);
}
}

/**
* Adds or overwrites a resource file with the given data. The path is an absolute path into to
* the deployment resources directory
*/
public void modifyResourceFile(String path, Function<String, String> mutator) {
try {
Path resourcePath = deploymentResourcePath.resolve(path);
long old = modTime(resourcePath);
byte[] data;
try (InputStream in = Files.newInputStream(resourcePath)) {
data = FileUtil.readFileContents(in);
Expand All @@ -628,7 +649,7 @@ public void modifyResourceFile(String path, Function<String, String> mutator) {
content = mutator.apply(content);

Files.write(resourcePath, content.getBytes(StandardCharsets.UTF_8));
sleepForFileChanges(resourcePath);
sleepForFileChanges(resourcePath, old);
} catch (IOException e) {
throw new UncheckedIOException(e);
}
Expand All @@ -640,14 +661,14 @@ public void modifyResourceFile(String path, Function<String, String> mutator) {
*/
public void addResourceFile(String path, byte[] data) {
final Path resourceFilePath = deploymentResourcePath.resolve(path);
long oldParent = modTime(resourceFilePath.getParent());
try {
Files.write(resourceFilePath, data);
} catch (IOException e) {
throw new UncheckedIOException(e);
}
sleepForFileChanges(resourceFilePath);
// since this is a new file addition, even wait for the parent dir's last modified timestamp to change
sleepForFileChanges(resourceFilePath.getParent());
sleepForFileChanges(resourceFilePath.getParent(), oldParent);
}

/**
Expand All @@ -656,6 +677,7 @@ public void addResourceFile(String path, byte[] data) {
*/
public void deleteResourceFile(String path) {
final Path resourceFilePath = deploymentResourcePath.resolve(path);
long old = modTime(resourceFilePath.getParent());
long timeout = System.currentTimeMillis() + 5000;
//in general there is a potential race here
//if you serve a file you will send the data to the client, then close the resource
Expand All @@ -679,7 +701,7 @@ public void deleteResourceFile(String path) {
}
}
// wait for last modified time of the parent to get updated
sleepForFileChanges(resourceFilePath.getParent());
sleepForFileChanges(resourceFilePath.getParent(), old);
}

/**
Expand Down Expand Up @@ -724,4 +746,23 @@ private Path copySourceFilesForClass(Path projectSourcesDir, Path deploymentSour
return null;
}

private Path findTargetSourceFilesForPath(Path projectSourcesDir, Path deploymentSourcesDir, Path classesDir,
Path classFile) {
for (CompilationProvider provider : compilationProviders) {
Path source = provider.getSourcePath(classFile,
Collections.singleton(projectSourcesDir.toAbsolutePath().toString()),
classesDir.toAbsolutePath().toString());
if (source != null) {
String relative = projectSourcesDir.relativize(source).toString();
try {
Path resolved = deploymentSourcesDir.resolve(relative);
Files.createDirectories(resolved.getParent());
return resolved;
} catch (IOException e) {
throw new UncheckedIOException(e);
}
}
}
throw new RuntimeException("Could not find source file for " + classFile);
}
}