[vfs] LFS / file watcher interaction refined

- FileWatcher.isOperational() switched from "all-of" to "any-of" (IDEA-152485)
- "is-watched" check removed from RefreshSession.scan() as supposedly superfluous
- FileWatcher.isWatched() dropped, along with a corresponding path map flag

platform/platform-impl/src/com/intellij/openapi/vfs/impl/local/CanonicalPathMap.java

@@ -121,7 +121,7 @@ class CanonicalPathMap {
    * of the recursive root because if the root itself was changed, we need to know about it.
    */
   @NotNull
-  public Collection<String> getWatchedPaths(@NotNull String reportedPath, boolean isExact, boolean fastPath) {
+  public Collection<String> getWatchedPaths(@NotNull String reportedPath, boolean isExact) {
     if (myFlatWatchRoots.isEmpty() && myRecursiveWatchRoots.isEmpty()) return Collections.emptyList();
 
     Collection<String> affectedPaths = applyMapping(reportedPath);
@@ -129,8 +129,6 @@ class CanonicalPathMap {
 
     ext:
     for (String path : affectedPaths) {
-      if (fastPath && !changedPaths.isEmpty()) break;
-
       for (String root : myFlatWatchRoots) {
         if (FileUtil.namesEqual(path, root)) {
           changedPaths.add(path);
@@ -160,7 +158,7 @@ class CanonicalPathMap {
       }
     }
 
-    if (!fastPath && changedPaths.isEmpty() && LOG.isDebugEnabled()) {
+    if (changedPaths.isEmpty() && LOG.isDebugEnabled()) {
       LOG.debug("Not watchable, filtered: " + reportedPath);
     }
 
@@ -190,4 +188,4 @@ class CanonicalPathMap {
 
     return results;
   }
-}
+}

platform/platform-impl/src/com/intellij/openapi/vfs/impl/local/FileWatcher.java

@@ -22,7 +22,6 @@ import com.intellij.openapi.application.ModalityState;
 import com.intellij.openapi.diagnostic.Logger;
 import com.intellij.openapi.util.NotNullLazyValue;
 import com.intellij.openapi.util.Pair;
-import com.intellij.openapi.vfs.VirtualFile;
 import com.intellij.openapi.vfs.local.FileWatcherNotificationSink;
 import com.intellij.openapi.vfs.local.PluggableFileWatcher;
 import com.intellij.openapi.vfs.newvfs.ManagingFS;
@@ -99,9 +98,9 @@ public class FileWatcher {
 
   public boolean isOperational() {
     for (PluggableFileWatcher watcher : myWatchers) {
-      if (!watcher.isOperational()) return false;
+      if (watcher.isOperational()) return true;
     }
-    return true;
+    return false;
   }
 
   public boolean isSettingRoots() {
@@ -147,23 +146,13 @@ public class FileWatcher {
     }
   }
 
-  public boolean isWatched(@NotNull VirtualFile file) {
-    // At the moment, "watched" means "monitored by at least one operational watcher".
-    // The following condition matches the above statement only for a single watcher, but this should work for a moment.
-    // todo[r.sh] reconsider usages of isWatched() and getManualWatchRoots() in LFS and refresh session
-    return isOperational() && !myPathMap.getWatchedPaths(file.getPath(), true, true).isEmpty();
-  }
-
-  public void notifyOnFailure(final String cause, @Nullable final NotificationListener listener) {
+  public void notifyOnFailure(@NotNull String cause, @Nullable NotificationListener listener) {
     LOG.warn(cause);
 
     if (myFailureShown.compareAndSet(false, true)) {
-      ApplicationManager.getApplication().invokeLater(new Runnable() {
-        @Override
-        public void run() {
-          String title = ApplicationBundle.message("watcher.slow.sync");
-          Notifications.Bus.notify(NOTIFICATION_GROUP.getValue().createNotification(title, cause, NotificationType.WARNING, listener));
-        }
+      String title = ApplicationBundle.message("watcher.slow.sync");
+      ApplicationManager.getApplication().invokeLater(() -> {
+        Notifications.Bus.notify(NOTIFICATION_GROUP.getValue().createNotification(title, cause, NotificationType.WARNING, listener));
       }, ModalityState.NON_MODAL);
     }
   }
@@ -207,7 +196,7 @@ public class FileWatcher {
 
     @Override
     public void notifyDirtyPath(@NotNull String path) {
-      Collection<String> paths = myPathMap.getWatchedPaths(path, true, false);
+      Collection<String> paths = myPathMap.getWatchedPaths(path, true);
       if (!paths.isEmpty()) {
         synchronized (myLock) {
           for (String eachPath : paths) {
@@ -220,7 +209,7 @@ public class FileWatcher {
 
     @Override
     public void notifyPathCreatedOrDeleted(@NotNull String path) {
-      Collection<String> paths = myPathMap.getWatchedPaths(path, true, false);
+      Collection<String> paths = myPathMap.getWatchedPaths(path, true);
       if (!paths.isEmpty()) {
         synchronized (myLock) {
           for (String p : paths) {
@@ -237,7 +226,7 @@ public class FileWatcher {
 
     @Override
     public void notifyDirtyDirectory(@NotNull String path) {
-      Collection<String> paths = myPathMap.getWatchedPaths(path, false, false);
+      Collection<String> paths = myPathMap.getWatchedPaths(path, false);
       if (!paths.isEmpty()) {
         synchronized (myLock) {
           myDirtyPaths.dirtyDirectories.addAll(paths);
@@ -248,7 +237,7 @@ public class FileWatcher {
 
     @Override
     public void notifyDirtyPathRecursive(@NotNull String path) {
-      Collection<String> paths = myPathMap.getWatchedPaths(path, false, false);
+      Collection<String> paths = myPathMap.getWatchedPaths(path, false);
       if (!paths.isEmpty()) {
         synchronized (myLock) {
           for (String each : paths) {

platform/platform-impl/src/com/intellij/openapi/vfs/impl/local/LocalFileSystemImpl.java

@@ -280,7 +280,7 @@ public final class LocalFileSystemImpl extends LocalFileSystemBase implements Ap
 
     if (myWatcher.isOperational()) {
       for (String root : myWatcher.getManualWatchRoots()) {
-        final VirtualFile suspiciousRoot = findFileByPathIfCached(root);
+        VirtualFile suspiciousRoot = findFileByPathIfCached(root);
         if (suspiciousRoot != null) {
           ((NewVirtualFile)suspiciousRoot).markDirtyRecursively();
         }

platform/platform-impl/src/com/intellij/openapi/vfs/newvfs/RefreshSessionImpl.java

@@ -24,7 +24,6 @@ import com.intellij.openapi.vfs.LocalFileSystem;
 import com.intellij.openapi.vfs.VirtualFile;
 import com.intellij.openapi.vfs.VirtualFileManager;
 import com.intellij.openapi.vfs.ex.VirtualFileManagerEx;
-import com.intellij.openapi.vfs.impl.local.FileWatcher;
 import com.intellij.openapi.vfs.impl.local.LocalFileSystemImpl;
 import com.intellij.openapi.vfs.newvfs.events.VFileEvent;
 import com.intellij.openapi.vfs.newvfs.persistent.PersistentFS;
@@ -124,15 +123,9 @@ public class RefreshSessionImpl extends RefreshSession {
     boolean haveEventsToFire = myFinishRunnable != null || !myEvents.isEmpty();
 
     if (!workQueue.isEmpty()) {
-      final LocalFileSystem fileSystem = LocalFileSystem.getInstance();
-      final FileWatcher watcher;
-      if (fileSystem instanceof LocalFileSystemImpl) {
-        LocalFileSystemImpl fs = (LocalFileSystemImpl)fileSystem;
-        fs.markSuspiciousFilesDirty(workQueue);
-        watcher = fs.getFileWatcher();
-      }
-      else {
-        watcher = null;
+      LocalFileSystem fs = LocalFileSystem.getInstance();
+      if (fs instanceof LocalFileSystemImpl) {
+        ((LocalFileSystemImpl)fs).markSuspiciousFilesDirty(workQueue);
       }
 
       long t = 0;
@@ -145,9 +138,8 @@ public class RefreshSessionImpl extends RefreshSession {
         if (myCancelled) break;
 
         NewVirtualFile nvf = (NewVirtualFile)file;
-        if (!myIsRecursive && (!myIsAsync || (watcher != null && !watcher.isWatched(nvf)))) {
-          // we're unable to definitely refresh synchronously by means of file watcher.
-          nvf.markDirty();
+        if (!myIsRecursive && !myIsAsync) {
+          nvf.markDirty();  // always scan when non-recursive AND synchronous - needed e.g. when refreshing project files on open
         }
 
         RefreshWorker worker = new RefreshWorker(nvf, myIsRecursive);

platform/platform-tests/testSrc/com/intellij/openapi/vfs/impl/local/CanonicalPathMapTest.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2000-2015 JetBrains s.r.o.
+ * Copyright 2000-2016 JetBrains s.r.o.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -43,7 +43,7 @@ public class CanonicalPathMapTest {
     // REMAP from native file watcher: /?/root/mapped -> /?/root/real
     pathMap.addMapping(Collections.singletonList(pair(mappedDir.getPath(), realDir.getPath())));
 
-    Collection<String> watchedPaths = pathMap.getWatchedPaths(new File(mappedDir, "file.txt").getPath(), true, false);
+    Collection<String> watchedPaths = pathMap.getWatchedPaths(new File(mappedDir, "file.txt").getPath(), true);
     assertThat(watchedPaths).containsExactly(new File(symLink, "file.txt").getPath());
   }
 }

platform/platform-tests/testSrc/com/intellij/openapi/vfs/local/FileWatcherTest.kt

@@ -36,6 +36,7 @@ import com.intellij.testFramework.runInEdtAndWait
 import com.intellij.util.Alarm
 import com.intellij.util.TimeoutUtil
 import com.intellij.util.concurrency.Semaphore
+import org.assertj.core.api.Assertions.assertThat
 import org.junit.After
 import org.junit.Assume.assumeTrue
 import org.junit.Before
@@ -48,6 +49,8 @@ import kotlin.test.assertFalse
 import kotlin.test.assertTrue
 
 class FileWatcherTest : BareTestFixtureTestCase() {
+  //<editor-fold desc="Set up / tear down">
+
   private val LOG: Logger by lazy { Logger.getInstance(NativeFileWatcherImpl::class.java) }
 
   private val START_STOP_DELAY = 10000L      // time to wait for the watcher spin up/down
@@ -102,7 +105,7 @@ class FileWatcherTest : BareTestFixtureTestCase() {
     LOG.debug("================== tearing down up " + getTestName(false) + " ==================")
   }
 
-  // test cases
+  //</editor-fold>
 
   @Test fun testWatchRequestConvention() {
     val dir = tempDir.newFolder("dir")
@@ -511,7 +514,14 @@ class FileWatcherTest : BareTestFixtureTestCase() {
     assertTrue(vFile.isWritable)
   }
 
-  // helpers
+  @Test fun testSyncRefreshNonWatchedFile() {
+    val file = tempDir.newFile("test.txt")
+    val vFile = refresh(file)
+    file.writeText("new content")
+    assertThat(VfsTestUtil.print(VfsTestUtil.getEvents { vFile.refresh(false, false) })).containsOnly("U : ${vFile.path}")
+  }
+
+  //<editor-fold desc="Helpers">
 
   private fun wait(timeout: Long = START_STOP_DELAY, condition: () -> Boolean) {
     val stopAt = System.currentTimeMillis() + timeout
@@ -557,4 +567,6 @@ class FileWatcherTest : BareTestFixtureTestCase() {
     val actual = VfsTestUtil.print(events).sorted()
     assertEquals(expected, actual)
   }
+
+  //</editor-fold>
 }