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.

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

Debounce requests for a filesystem refresh #805

Merged
merged 1 commit into from
Oct 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package appland.cli;

import appland.config.AppMapConfigFile;
import appland.config.AppMapConfigFileListener;
import appland.files.AppMapFiles;
import appland.files.AppMapVfsUtils;
import appland.settings.AppMapApplicationSettingsService;
Expand All @@ -21,7 +20,6 @@
import com.intellij.openapi.util.Key;
import com.intellij.openapi.util.SystemInfo;
import com.intellij.openapi.util.text.StringUtil;
import com.intellij.openapi.vfs.VfsUtil;
import com.intellij.openapi.vfs.VfsUtilCore;
import com.intellij.openapi.vfs.VirtualFile;
import com.intellij.openapi.vfs.ex.temp.TempFileSystemMarker;
Expand Down Expand Up @@ -272,11 +270,6 @@ public void dispose() {
}
}

// extracted as method to allow overriding and testing it
protected void requestVirtualFileRefresh(@NotNull Path path) {
VfsUtil.markDirtyAndRefresh(true, false, false, path.toFile());
}

@TestOnly
synchronized @Nullable CliProcesses getProcesses(@NotNull VirtualFile directory) {
return processes.get(directory);
Expand Down Expand Up @@ -658,7 +651,7 @@ boolean isEmpty() {
/**
* Listen to index events of the indexer.
*/
private class IndexEventsProcessListener extends ProcessAdapter {
private static class IndexEventsProcessListener extends ProcessAdapter {
@Override
public void onTextAvailable(@NotNull ProcessEvent event, @NotNull Key outputType) {
if (LOG.isTraceEnabled()) {
Expand All @@ -675,7 +668,7 @@ public void onTextAvailable(@NotNull ProcessEvent event, @NotNull Key outputType

// Refresh parent directory of the indexed AppMap, because it contains both
// myAppMap.appmap.json file and the corresponding metadata directory myAppMap/
requestVirtualFileRefresh(Path.of(filePath).getParent());
VfsRefreshService.getInstance().requestVirtualFileRefresh(Path.of(filePath).getParent());
} catch (InvalidPathException e) {
if (LOG.isDebugEnabled()) {
LOG.debug("Error parsing indexed file path: " + filePath, e);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
package appland.cli;

import com.intellij.openapi.Disposable;
import com.intellij.openapi.vfs.LocalFileSystem;
import com.intellij.openapi.vfs.VfsUtil;
import com.intellij.openapi.vfs.VirtualFile;
import com.intellij.util.Alarm;
import com.intellij.util.SingleAlarm;
import com.intellij.util.containers.ContainerUtil;
import org.jetbrains.annotations.NotNull;

import java.nio.file.Path;
import java.util.Set;
import java.util.concurrent.CopyOnWriteArraySet;
import java.util.concurrent.atomic.AtomicReference;

public class DefaultVfsRefreshService implements VfsRefreshService, Disposable {
// Cumulate requests for filesystem refreshes of the last 5s.
// A new requests during the 5s delay restarts the delay.
private final @NotNull SingleAlarm refreshAlarm = new SingleAlarm(
this::debouncedRefresh,
5_000,
this,
Alarm.ThreadToUse.POOLED_THREAD,
null);
// The Set<Path> value must be thread-safe.
private final @NotNull AtomicReference<@NotNull Set<Path>> pendingRefresh = new AtomicReference<>(new CopyOnWriteArraySet<>());

@Override
public void dispose() {
pendingRefresh.getAndSet(Set.of());
}

@Override
public void requestVirtualFileRefresh(@NotNull Path path) {
pendingRefresh.get().add(path);
refreshAlarm.cancelAndRequest();
}

private void debouncedRefresh() {
var pending = pendingRefresh.getAndSet(new CopyOnWriteArraySet<>());
if (pending.isEmpty()) {
return;
}

var fs = LocalFileSystem.getInstance();
var pendingVirtualFiles = ContainerUtil.mapNotNull(pending, fs::refreshAndFindFileByNioFile);
if (!pendingVirtualFiles.isEmpty()) {
VfsUtil.markDirtyAndRefresh(true, false, false, pendingVirtualFiles.toArray(new VirtualFile[0]));
}
}
}
23 changes: 23 additions & 0 deletions plugin-core/src/main/java/appland/cli/VfsRefreshService.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
package appland.cli;

import com.intellij.openapi.application.ApplicationManager;
import org.jetbrains.annotations.NotNull;

import java.nio.file.Path;

/**
* Service interface to allow testing of our Vfs refresh handing of AppMap data.
* Requests for a filesystem refresh must be debounced by the service implementation.
*/
public interface VfsRefreshService {
static @NotNull VfsRefreshService getInstance() {
return ApplicationManager.getApplication().getService(VfsRefreshService.class);
}

/**
* Request a refresh of the given path.
*
* @param path File or directory to refresh.
*/
void requestVirtualFileRefresh(@NotNull Path path);
}
8 changes: 7 additions & 1 deletion plugin-core/src/main/resources/META-INF/appmap-core.xml
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,8 @@
<fileEditorProvider implementation="appland.installGuide.InstallGuideEditorProvider" order="first"/>

<fileEditorProvider implementation="appland.webviews.findings.FindingsOverviewEditorProvider" order="first"/>
<fileEditorProvider implementation="appland.webviews.findingDetails.FindingDetailsEditorProvider" order="first"/>
<fileEditorProvider implementation="appland.webviews.findingDetails.FindingDetailsEditorProvider"
order="first"/>
<fileEditorProvider implementation="appland.webviews.navie.NavieEditorProvider" order="first"/>

<fileIconProvider implementation="appland.webviews.WebviewEditorIconProvider"/>
Expand Down Expand Up @@ -80,6 +81,11 @@
serviceImplementation="appland.cli.DefaultAppLandDownloadService"
testServiceImplementation="appland.cli.TestAppLandDownloadService"/>

<applicationService
serviceInterface="appland.cli.VfsRefreshService"
serviceImplementation="appland.cli.DefaultVfsRefreshService"
testServiceImplementation="appland.cli.TestVfsRefreshService"/>

<applicationService id="appmap.settings"
serviceImplementation="appland.settings.AppMapApplicationSettingsService"/>
<projectService serviceInterface="appland.remote.RemoteRecordingStatusService"
Expand Down
4 changes: 2 additions & 2 deletions plugin-core/src/test/java/appland/AppMapBaseTest.java
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
package appland;

import appland.cli.AppLandCommandLineService;
import appland.cli.TestCommandLineService;
import appland.cli.TestVfsRefreshService;
import appland.config.AppMapConfigFile;
import appland.files.AppMapFiles;
import appland.problemsView.TestFindingsManager;
Expand Down Expand Up @@ -302,7 +302,7 @@ private String createClassMap(int functionCount) {
}

private void resetState() {
TestCommandLineService.getInstance().reset();
TestVfsRefreshService.getInstance().reset();
}

private void shutdownAppMapProcesses() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -230,11 +230,11 @@ public void directoryRefreshAfterAppMapIndexing() throws Throwable {
var appMapConfig = myFixture.copyFileToProject("projects/without_existing_index/appmap.yml", "test-project/appmap.yml");
ModuleTestUtils.withContentRoot(getModule(), appMapConfig.getParent(), () -> {
// copying AppMaps into a watched directory must trigger a file refresh based on the output of the AppMap process
var refreshCondition = TestCommandLineService.newVfsRefreshCondition(getProject(), getTestRootDisposable());
var refreshCondition = TestVfsRefreshService.newVfsRefreshCondition(getProject(), getTestRootDisposable());
myFixture.copyDirectoryToProject("projects/without_existing_index", "test-project");
assertTrue(refreshCondition.await(30, TimeUnit.SECONDS));

var refreshedFiles = TestCommandLineService.getInstance().getRefreshedFiles();
var refreshedFiles = TestVfsRefreshService.getInstance().getRefreshedFiles();
assertSize(1, refreshedFiles);
var refreshedPath = refreshedFiles.iterator().next();
assertTrue("The parent directory of the AppMap must be refreshed", refreshedPath.endsWith("appmap"));
Expand Down
Original file line number Diff line number Diff line change
@@ -1,53 +1,14 @@
package appland.cli;

import com.intellij.openapi.Disposable;
import com.intellij.openapi.application.ApplicationManager;
import com.intellij.openapi.project.Project;
import com.intellij.util.containers.ContainerUtil;
import com.intellij.util.messages.Topic;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.TestOnly;

import java.nio.file.Path;
import java.util.Collections;
import java.util.Set;
import java.util.concurrent.CountDownLatch;

@TestOnly
public class TestCommandLineService extends DefaultCommandLineService {
private static final Topic<Runnable> VFS_REFRESH_TOPIC = Topic.create("appmap.test.vfsRefresh", Runnable.class);

private final Set<Path> refreshedFiles = ContainerUtil.newConcurrentSet();

/**
* Condition to wait for the request to refresh a file path based on STDOUT of the AppMap indexer.
* We're exposing this to allow tests to access fields of DefaultCommandLineService.
*/
public static @NotNull CountDownLatch newVfsRefreshCondition(@NotNull Project project,
@NotNull Disposable parentDisposable) {
var latch = new CountDownLatch(1);
project.getMessageBus().connect(parentDisposable).subscribe(VFS_REFRESH_TOPIC, (Runnable) latch::countDown);
return latch;
}

public static @NotNull TestCommandLineService getInstance() {
return (TestCommandLineService) AppLandCommandLineService.getInstance();
}

public Set<Path> getRefreshedFiles() {
return Collections.unmodifiableSet(refreshedFiles);
}

public void reset() {
refreshedFiles.clear();
}

@Override
protected void requestVirtualFileRefresh(@NotNull Path filePath) {
try {
refreshedFiles.add(filePath);
super.requestVirtualFileRefresh(filePath);
} finally {
ApplicationManager.getApplication().getMessageBus().syncPublisher(VFS_REFRESH_TOPIC).run();
}
}
}
59 changes: 59 additions & 0 deletions plugin-core/src/test/java/appland/cli/TestVfsRefreshService.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
package appland.cli;

import com.intellij.openapi.Disposable;
import com.intellij.openapi.application.ApplicationManager;
import com.intellij.openapi.project.Project;
import com.intellij.util.containers.ContainerUtil;
import com.intellij.util.messages.Topic;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.TestOnly;

import java.nio.file.Path;
import java.util.Collections;
import java.util.Set;
import java.util.concurrent.CountDownLatch;

@TestOnly
public class TestVfsRefreshService extends DefaultVfsRefreshService {
private static final Topic<Runnable> VFS_REFRESH_TOPIC = Topic.create("appmap.test.vfsRefresh", Runnable.class);

private final Set<Path> refreshedFiles = ContainerUtil.newConcurrentSet();

public static @NotNull TestVfsRefreshService getInstance() {
return (TestVfsRefreshService) VfsRefreshService.getInstance();
}

/**
* Condition to wait for the request to refresh a file path based on STDOUT of the AppMap indexer.
*/
public static @NotNull CountDownLatch newVfsRefreshCondition(@NotNull Project project,
@NotNull Disposable parentDisposable) {
var latch = new CountDownLatch(1);
project.getMessageBus().connect(parentDisposable).subscribe(VFS_REFRESH_TOPIC, (Runnable) latch::countDown);
return latch;
}

public @NotNull Set<Path> getRefreshedFiles() {
return Collections.unmodifiableSet(refreshedFiles);
}

@Override
public void dispose() {
reset();
super.dispose();
}

public void reset() {
refreshedFiles.clear();
}

@Override
public void requestVirtualFileRefresh(@NotNull Path path) {
try {
refreshedFiles.add(path);
super.requestVirtualFileRefresh(path);
} finally {
ApplicationManager.getApplication().getMessageBus().syncPublisher(VFS_REFRESH_TOPIC).run();
}
}
}
Loading