Skip to content

Commit

Permalink
wip
Browse files Browse the repository at this point in the history
  • Loading branch information
@jansorg
jansorg committed Feb 7, 2024
1 parent 730d651 commit 0064fe1
Show file tree
Hide file tree
Showing 4 changed files with 81 additions and 36 deletions.
59 changes: 35 additions & 24 deletions plugin-core/src/main/java/appland/cli/DefaultCommandLineService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import appland.config.AppMapConfigFile;
import appland.config.AppMapConfigFileListener;
import appland.files.AppMapFiles;
import appland.files.AppMapVfsUtils;
import appland.settings.AppMapApplicationSettingsService;
import com.intellij.execution.ExecutionException;
Expand All @@ -14,8 +15,8 @@
import com.intellij.openapi.Disposable;
import com.intellij.openapi.application.ApplicationManager;
import com.intellij.openapi.diagnostic.Logger;
import com.intellij.openapi.project.DumbService;
import com.intellij.openapi.project.ProjectManager;
import com.intellij.openapi.roots.ProjectRootManager;
import com.intellij.openapi.util.Key;
import com.intellij.openapi.util.SystemInfo;
import com.intellij.openapi.util.text.StringUtil;
Expand All @@ -39,10 +40,11 @@
import java.nio.file.InvalidPathException;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
import java.util.regex.Pattern;
import java.util.stream.Collectors;

public class DefaultCommandLineService implements AppLandCommandLineService {
private static final Logger LOG = Logger.getInstance(DefaultCommandLineService.class);
Expand All @@ -55,12 +57,13 @@ public class DefaultCommandLineService implements AppLandCommandLineService {
// a value of "0" indicates that process restart is disabled
private static final Key<Long> NEXT_RESTART_DELAY = Key.create("appmap.processRestartDelay");

// toString() uses unguarded access, synchronizing it could create deadlocks
@GuardedBy("this")
protected final Map<VirtualFile, CliProcesses> processes = new HashMap<>();
protected final Map<VirtualFile, CliProcesses> processes = new ConcurrentHashMap<>();

public DefaultCommandLineService() {
var connection = ApplicationManager.getApplication().getMessageBus().connect(this);
connection.subscribe(AppMapConfigFileListener.TOPIC, (AppMapConfigFileListener) this::refreshForOpenProjectsInBackground);
connection.subscribe(AppMapConfigFileListener.TOPIC, this::refreshForOpenProjectsInBackground);
}

@Override
Expand All @@ -74,7 +77,7 @@ public synchronized boolean isRunning(@NotNull VirtualFile directory, boolean st

@Override
public synchronized void start(@NotNull VirtualFile directory, boolean waitForProcessTermination) throws ExecutionException {
if (!isDirectoryEnabled(directory)) {
if (!AppMapFiles.isDirectoryEnabled(directory)) {
return;
}

Expand All @@ -101,15 +104,6 @@ public synchronized void start(@NotNull VirtualFile directory, boolean waitForPr
}
}

/**
* File appmap.yml is a marker file to tell that CLI processes may be launched for a directory.
*
* @return {@code true} if the directory may have CLI processes watching it
*/
private boolean isDirectoryEnabled(@NotNull VirtualFile directory) {
return directory.findChild("appmap.yml") != null;
}

@Override
public synchronized void stop(@NotNull VirtualFile directory, boolean waitForTermination) {
var entry = processes.remove(directory);
Expand Down Expand Up @@ -201,8 +195,10 @@ public void stopAll(boolean waitForTermination) {
return createAppMapCommand("stats", "--appmap-file", localPath.toString(), "--limit", String.valueOf(Long.MAX_VALUE), "--format", "json");
}

// We're not synchronizing, because some IDE threads display or use the result of toString
// and this must not create deadlocks.
@Override
public synchronized String toString() {
public /*synchronized*/ String toString() {
return "DefaultCommandLineService{" +
"processes=" + processes +
'}';
Expand Down Expand Up @@ -329,19 +325,34 @@ protected void requestVirtualFileRefresh(@NotNull Path path) {
}

private void doRefreshForOpenProjectsLocked() {
var topLevelRoots = new VfsUtilCore.DistinctVFilesRootsCollection(VirtualFile.EMPTY_ARRAY);
var enabledRoots = VfsUtilCore.createCompactVirtualFileSet();
for (var project : ProjectManager.getInstance().getOpenProjects()) {
if (!project.isDefault() && !project.isDisposed()) {
for (var contentRoot : ProjectRootManager.getInstance(project).getContentRoots()) {
if (isDirectoryEnabled(contentRoot)) {
topLevelRoots.add(contentRoot);
}
var projectRoots = DumbService.getInstance(project).runReadActionInSmartMode(() -> {
return AppMapFiles.findAppMapConfigFiles(project).stream()
.map(VirtualFile::getParent)
.filter(VirtualFile::isValid)
.collect(Collectors.toSet());
});
enabledRoots.addAll(projectRoots);
}

// Remove processes of roots, which no longer have a matching content root in a project or which don't match the
// settings anymore.
// We need to launch the scanner when "enableFindings" changes. We're iterating on a copy, because stop() is
// called inside the loop and modifies "processes"
for (var entry : List.copyOf(processes.entrySet())) {
var activeRoot = entry.getKey();
if (!enabledRoots.contains(activeRoot)) {
try {
stop(activeRoot, false);
} catch (Exception e) {
LOG.warn("Error stopping processes for root: " + activeRoot.getPath());
}
}
}

// launch missing cli processes
for (var root : topLevelRoots) {
// Launch missing processes.
for (var root : enabledRoots) {
try {
start(root, false);
} catch (ExecutionException e) {
Expand All @@ -355,7 +366,7 @@ private void doRefreshForOpenProjectsLocked() {
* This is either the directory configured in appmap.yml or the base directory itself.
*/
private static @NotNull Path findWatchedAppMapDirectory(@NotNull Path baseDirectory) {
var appMapConfig = AppMapConfigFile.parseConfigFile(baseDirectory.resolve("appmap.yml"));
var appMapConfig = AppMapConfigFile.parseConfigFile(baseDirectory.resolve(AppMapFiles.APPMAP_YML));
if (appMapConfig == null || appMapConfig.getAppMapDir() == null) {
return baseDirectory;
}
Expand Down
9 changes: 6 additions & 3 deletions plugin-core/src/main/java/appland/config/AppmapYamlAsyncFileListener.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,12 @@ public class AppmapYamlAsyncFileListener implements AsyncFileListener {
return new ChangeApplier() {
@Override
public void afterVfsChange() {
ApplicationManager.getApplication().getMessageBus()
.syncPublisher(AppMapConfigFileListener.TOPIC)
.refreshAppMapConfigs();
var application = ApplicationManager.getApplication();
application.executeOnPooledThread(() -> {
application.getMessageBus()
.syncPublisher(AppMapConfigFileListener.TOPIC)
.refreshAppMapConfigs();
});
}
};
}
Expand Down
33 changes: 32 additions & 1 deletion plugin-core/src/main/java/appland/files/AppMapFiles.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,22 +54,44 @@ public static boolean isAppMapConfigFileName(@NotNull String fileName) {
}

/**
* Locate the appmap.yml files located in the project.
* This method must be invoked in a {@link ReadAction}.
* The appmap.yml files are not searched inside excluded folders, libraries or dependencies of the project.
*
* @return The appmap.yaml files available in the current project.
* appmap.yml files are not searched inside excluded folders, libraries or dependencies of the project.
* @throws com.intellij.openapi.project.IndexNotReadyException If the filename index it unavailable
*/
@RequiresReadLock
public static @NotNull Collection<VirtualFile> findAppMapConfigFiles(@NotNull Project project) {
return findAppMapConfigFiles(project, AppMapSearchScopes.appMapConfigSearchScope(project));
}

/**
* Locate the appmap.yml files located in the project and scope.
* This method must be invoked in a {@link ReadAction}.
*
* @return The appmap.yaml files available in the current project and search scope.
* @throws com.intellij.openapi.project.IndexNotReadyException If the filename index it unavailable
*/
@RequiresReadLock
public static @NotNull Collection<VirtualFile> findAppMapConfigFiles(@NotNull Project project, @NotNull GlobalSearchScope scope) {
return FilenameIndex.getVirtualFilesByName(project, APPMAP_YML, false, scope);
}

/**
* @return The appmap.yaml files available in the current project and search scope.
*/
@RequiresReadLock
public static @NotNull Collection<VirtualFile> findAppMapConfigFilesByContentRoots(@NotNull Project project) {
var roots = VfsUtilCore.createCompactVirtualFileSet();
for (var contentRoot : ProjectRootManager.getInstance(project).getContentRoots()) {
if (isDirectoryEnabled(contentRoot)) {
roots.add(contentRoot);
}
}
return roots;
}

/**
* @param appMapConfigFile appmap.yml file
* @return The "appmap_dir" property value, if it's configured in the file
Expand Down Expand Up @@ -260,6 +282,15 @@ public static boolean updateMetadata(@NotNull Path appMapFile, @NotNull String n
return roots.toArray(VirtualFile.EMPTY_ARRAY);
}

/**
* File appmap.yml is a marker file to tell that CLI processes may be launched for a directory.
*
* @return {@code true} if the directory contains a appmap.yml file and may have CLI processes watching it.
*/
public static boolean isDirectoryEnabled(@NotNull VirtualFile directory) {
return directory.isValid() && directory.findChild(APPMAP_YML) != null;
}

/**
* Load AppMap with specific size threshold values.
*
Expand Down
16 changes: 8 additions & 8 deletions plugin-core/src/test/java/appland/cli/DefaultCommandLineServiceTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,11 +61,9 @@ public void directoryTree() throws Exception {
var nestedDir = myFixture.addFileToProject("parent/child/file.txt", "").getParent().getVirtualFile();
assertNotNull(nestedDir);

addContentRootAndLaunchService(nestedDir);
assertEmptyRoots();

// creating an appmap.yml file must trigger launch of the AppMap processes
// creating an appmap.yml file must trigger the launch of the matching AppMap processes
createAppMapYaml(nestedDir);
addContentRootAndLaunchService(nestedDir);
assertActiveRoots(nestedDir);

addContentRootAndLaunchService(parentDir);
Expand All @@ -83,10 +81,12 @@ public void directoryTree() throws Exception {

service.stop(parentDir, true);
service.stop(nestedDir, true);
assertFalse("Expected not to be running: " + service, service.isRunning(parentDir, true));
assertFalse("Expected not to be running: " + service, service.isRunning(parentDir, false));
assertFalse("Expected not to be running: " + service, service.isRunning(nestedDir, true));
assertFalse("Expected not to be running: " + service, service.isRunning(nestedDir, false));

var debugInfo = service.toString();
assertFalse("Expected not to be running: " + debugInfo, service.isRunning(parentDir, true));
assertFalse("Expected not to be running: " + debugInfo, service.isRunning(parentDir, false));
assertFalse("Expected not to be running: " + debugInfo, service.isRunning(nestedDir, true));
assertFalse("Expected not to be running: " + debugInfo, service.isRunning(nestedDir, false));
}

@Test
Expand Down

0 comments on commit 0064fe1

Please sign in to comment.