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

Sync select files #7127

Draft
wants to merge 5 commits into
base: master
Choose a base branch
from
Draft

Sync select files #7127

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
1812e1c
Request sync of particular files
odisseus Dec 4, 2024
ec50e6c
Queue for files that need to be synced
odisseus Dec 5, 2024
a4d450c
logging
odisseus Dec 5, 2024
564fa09
Synchronised add and clear
odisseus Dec 10, 2024
7187a8c
Unify the logic for requesting sync
odisseus Dec 12, 2024
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
43 changes: 30 additions & 13 deletions base/src/com/google/idea/blaze/base/qsync/QuerySyncAsyncFileListener.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import static com.google.common.collect.ImmutableList.toImmutableList;

import com.google.common.annotations.VisibleForTesting;
import com.google.common.collect.ImmutableCollection;
import com.google.common.collect.ImmutableList;
import com.google.idea.blaze.base.lang.buildfile.language.BuildFileType;
import com.google.idea.blaze.base.logging.utils.querysync.QuerySyncActionStatsScope;
Expand All @@ -28,6 +29,7 @@
import com.google.idea.blaze.base.sync.status.BlazeSyncStatus;
import com.intellij.openapi.Disposable;
import com.intellij.openapi.application.ApplicationManager;
import com.intellij.openapi.diagnostic.Logger;
import com.intellij.openapi.project.Project;
import com.intellij.openapi.vfs.AsyncFileListener;
import com.intellij.openapi.vfs.VirtualFile;
Expand All @@ -37,15 +39,21 @@
import com.intellij.openapi.vfs.newvfs.events.VFileMoveEvent;
import com.intellij.openapi.vfs.newvfs.events.VFilePropertyChangeEvent;
import com.intellij.ui.EditorNotifications;
import org.jetbrains.annotations.NotNull;

import java.nio.file.Path;
import java.util.Collection;
import java.util.List;
import java.util.Optional;
import java.util.concurrent.ConcurrentLinkedQueue;
import java.util.concurrent.atomic.AtomicBoolean;
import javax.annotation.Nullable;

/** {@link AsyncFileListener} for monitoring project changes requiring a re-sync */
public class QuerySyncAsyncFileListener implements AsyncFileListener {

private static final Logger logger = Logger.getInstance(QuerySyncAsyncFileListener.class);

private final Project project;
private final SyncRequester syncRequester;

Expand Down Expand Up @@ -119,7 +127,8 @@ public void afterVfsChange() {
}

if (syncOnFileChanges()) {
syncRequester.requestSync();
syncRequester.requestSync(eventsRequiringSync.stream()
.map(VFileEvent::getFile).toList());
}
EditorNotifications.getInstance(project).updateAllNotifications();
});
Expand Down Expand Up @@ -154,7 +163,7 @@ private boolean requiresSync(VFileEvent event) {

/** Interface for requesting project syncs. */
public interface SyncRequester {
void requestSync();
void requestSync(@NotNull Collection<VirtualFile> files);
}

/**
Expand All @@ -164,7 +173,7 @@ public interface SyncRequester {
private static class QueueingSyncRequester implements SyncRequester {
private final Project project;

private final AtomicBoolean changePending = new AtomicBoolean(false);
private final ConcurrentLinkedQueue<VirtualFile> unprocessedChanges = new ConcurrentLinkedQueue<>();

public QueueingSyncRequester(Project project) {
this.project = project;
Expand All @@ -182,30 +191,38 @@ public void afterQuerySync(Project project, BlazeContext context) {
if (!requester.project.equals(project)) {
return;
}
if (requester.changePending.get()) {
requester.requestSyncInternal();
}
requester.requestSync(ImmutableList.of());
}
},
parentDisposable);
return requester;
}

@Override
public void requestSync() {
if (changePending.compareAndSet(false, true)) {
if (!BlazeSyncStatus.getInstance(project).syncInProgress()) {
requestSyncInternal();
public void requestSync(@NotNull Collection<VirtualFile> files) {
logger.info(String.format("Putting %d files into sync queue", files.size()));
ImmutableList<VirtualFile> changesToProcess = ImmutableList.of();
synchronized (unprocessedChanges) {
// TODO aggregate multiple events into one sync request
unprocessedChanges.addAll(files);
if (!unprocessedChanges.isEmpty()) {
if (!BlazeSyncStatus.getInstance(project).syncInProgress()) {
changesToProcess = ImmutableList.copyOf(unprocessedChanges);
unprocessedChanges.clear();
}
}
}
if(!changesToProcess.isEmpty()) {
requestSyncInternal(changesToProcess);
}
}

private void requestSyncInternal() {
private void requestSyncInternal(ImmutableCollection<VirtualFile> files) {
logger.info(String.format("Requesting sync of files: %s", files));
QuerySyncManager.getInstance(project)
.deltaSync(
QuerySyncActionStatsScope.create(QuerySyncAsyncFileListener.class, null),
QuerySyncActionStatsScope.createForFiles(QuerySyncAsyncFileListener.class, null, files),
TaskOrigin.AUTOMATIC);
changePending.set(false);
}
}

Expand Down
57 changes: 37 additions & 20 deletions ...sts/integrationtests/com/google/idea/blaze/base/qsync/QuerySyncAsyncFileListenerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

import static com.google.common.truth.Truth.assertThat;
import static java.nio.charset.StandardCharsets.UTF_8;
import static org.mockito.ArgumentMatchers.*;
import static org.mockito.Mockito.atLeastOnce;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.verify;
Expand All @@ -32,10 +33,14 @@
import com.intellij.testFramework.fixtures.LightJavaCodeInsightFixtureTestCase4;
import com.intellij.testFramework.PlatformTestUtil;
import java.nio.file.Path;
import java.util.Arrays;
import java.util.Collection;

import org.junit.Rule;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;
import org.mockito.ArgumentMatcher;
import org.mockito.Mock;
import org.mockito.junit.MockitoJUnit;
import org.mockito.junit.MockitoRule;
Expand All @@ -61,15 +66,15 @@ public void projectFileAdded_requestsSync() {
.setAutoSync(true);
VirtualFileManager.getInstance()
.addAsyncFileListener(fileListener, getFixture().getTestRootDisposable());
verify(mockSyncRequester, never()).requestSync();
verify(mockSyncRequester, never()).requestSync(any());

getFixture()
.addFileToProject(
INCLUDED_DIRECTORY.resolve("java/com/example/Class1.java").toString(),
"package com.example;public class Class1{}");
ApplicationManager.getApplication()
.invokeAndWait(PlatformTestUtil::dispatchAllEventsInIdeEventQueue);
verify(mockSyncRequester, atLeastOnce()).requestSync();
verify(mockSyncRequester, atLeastOnce()).requestSync(argThat(hasFiles("/src/my/project")));
}

@Test
Expand All @@ -87,7 +92,7 @@ public void projectFileAdded_autoSyncDisabled_neverRequestsSync() {
"package com.example;public class Class1{}");
ApplicationManager.getApplication()
.invokeAndWait(PlatformTestUtil::dispatchAllEventsInIdeEventQueue);
verify(mockSyncRequester, never()).requestSync();
verify(mockSyncRequester, never()).requestSync(any());
}

@Test
Expand All @@ -98,14 +103,14 @@ public void nonProjectFileAdded_neverRequestsSync() {
.setAutoSync(true);
VirtualFileManager.getInstance()
.addAsyncFileListener(fileListener, getFixture().getTestRootDisposable());
verify(mockSyncRequester, never()).requestSync();
verify(mockSyncRequester, never()).requestSync(any());

getFixture()
.addFileToProject(
"some/other/path/Class1.java", "package some.other.path;public class Class1{}");
ApplicationManager.getApplication()
.invokeAndWait(PlatformTestUtil::dispatchAllEventsInIdeEventQueue);
verify(mockSyncRequester, never()).requestSync();
verify(mockSyncRequester, never()).requestSync(any());
}

@Test
Expand All @@ -124,17 +129,15 @@ public void projectFileMoved_requestsSync() throws Exception {
.setAutoSync(true);
VirtualFileManager.getInstance()
.addAsyncFileListener(fileListener, getFixture().getTestRootDisposable());
verify(mockSyncRequester, never()).requestSync();
verify(mockSyncRequester, never()).requestSync(any());

String sourcePath = INCLUDED_DIRECTORY.resolve("java/com/example/Class1.java").toString();
String destinationPath = INCLUDED_DIRECTORY.resolve("submodule/java/com/example").toString();
WriteAction.runAndWait(
() ->
getFixture()
.moveFile(
INCLUDED_DIRECTORY.resolve("java/com/example/Class1.java").toString(),
INCLUDED_DIRECTORY.resolve("submodule/java/com/example").toString()));
() -> getFixture().moveFile(sourcePath, destinationPath));
ApplicationManager.getApplication()
.invokeAndWait(PlatformTestUtil::dispatchAllEventsInIdeEventQueue);
verify(mockSyncRequester, atLeastOnce()).requestSync();
verify(mockSyncRequester, atLeastOnce()).requestSync(argThat(hasFiles("/src/" + destinationPath + "/Class1.java")));
}

@Test
Expand All @@ -150,7 +153,7 @@ public void projectFileModified_nonBuildFile_doesNotRequestSync() throws Excepti
.setAutoSync(true);
VirtualFileManager.getInstance()
.addAsyncFileListener(fileListener, getFixture().getTestRootDisposable());
verify(mockSyncRequester, never()).requestSync();
verify(mockSyncRequester, never()).requestSync(any());

VirtualFile vf =
getFixture()
Expand All @@ -163,36 +166,50 @@ public void projectFileModified_nonBuildFile_doesNotRequestSync() throws Excepti
ApplicationManager.getApplication()
.invokeAndWait(PlatformTestUtil::dispatchAllEventsInIdeEventQueue);

verify(mockSyncRequester, never()).requestSync();
verify(mockSyncRequester, never()).requestSync(any());
assertThat(fileListener.hasModifiedBuildFiles()).isFalse();
}

@Test
public void projectFileModified_buildFile_requestsSync() throws Exception {
getFixture()
.addFileToProject(
INCLUDED_DIRECTORY.resolve("BUILD").toString(), "java_library(name=\"java\",srcs=[])");
String buildFilePath = INCLUDED_DIRECTORY.resolve("BUILD").toString();
getFixture().addFileToProject(buildFilePath, "java_library(name=\"java\",srcs=[])");

TestListener fileListener =
new TestListener(getFixture().getProject(), mockSyncRequester)
.setProjectInclude(projectRootPath().resolve(INCLUDED_DIRECTORY))
.setAutoSync(true);
VirtualFileManager.getInstance()
.addAsyncFileListener(fileListener, getFixture().getTestRootDisposable());
verify(mockSyncRequester, never()).requestSync();
verify(mockSyncRequester, never()).requestSync(any());

VirtualFile vf = getFixture().findFileInTempDir(INCLUDED_DIRECTORY.resolve("BUILD").toString());
VirtualFile vf = getFixture().findFileInTempDir(buildFilePath);
WriteAction.runAndWait(
() ->
vf.setBinaryContent(
"/**LICENSE-TEXT*/java_library(name=\"javalib\",srcs=[])".getBytes(UTF_8)));
ApplicationManager.getApplication()
.invokeAndWait(PlatformTestUtil::dispatchAllEventsInIdeEventQueue);

verify(mockSyncRequester, atLeastOnce()).requestSync();
verify(mockSyncRequester, atLeastOnce()).requestSync(argThat(hasFiles("/src/" + buildFilePath)));
assertThat(fileListener.hasModifiedBuildFiles()).isTrue();
}

private static ArgumentMatcher<Collection<VirtualFile>> hasFiles(String... paths) {
return new ArgumentMatcher<>() {
@Override
public boolean matches(Collection<VirtualFile> virtualFiles) {
var actualPaths = virtualFiles.stream().map(VirtualFile::getPath).sorted().toList();
return actualPaths.equals(Arrays.stream(paths).sorted().toList());
}

@Override
public Class<?> type() {
return Collection.class;
}
};
}

/**
* Implementation of {@link QuerySyncAsyncFileListener} for testing. Has a configurable single
* project include directory and setting for requesting syncs.
Expand Down