Improving re-loading support if the project keeps changing. Added tests.

ide/project.dependency/src/org/netbeans/modules/project/dependency/ProjectReload.java

@@ -965,7 +965,9 @@ public static CompletableFuture<ProjectState> withProjectState(Project p, final
         // special case if the state matches & is consistent: if the attempted quality is LESS 
         // than this request's target, the ReloadImplementation might give up 
         if (!doReload && lastKnown.target.isAtLeast(stateRequest.getTargetQuality())) {
-            LOG.log(Level.FINE, "FINISHED: Reload {0}, request: {1}, state {2} - NOOP, finished", new Object[] { p, stateRequest, lastKnown.toString() });
+            if (LOG.isLoggable(Level.FINE)) {
+                LOG.log(Level.FINE, "FINISHED: Reload {0}, request: {1}, state {2} - NOOP, finished", new Object[] { p, stateRequest, lastKnown.toString() });
+            }
             return CompletableFuture.completedFuture(lastKnown);
         }
         String reason = stateRequest.getReason();

ide/project.dependency/src/org/netbeans/modules/project/dependency/reload/ProjectReloadInternal.java

@@ -261,6 +261,8 @@ public StatePartsImpl(Map<? extends ProjectReloadImplementation<?>, ? extends Pr
         }
     }
 
+    public static final StateParts EMPTY_PARTS = new StatePartsImpl();
+
     static Collection variantKey(Project p, StateParts parts, Lookup context) {
         Collection c = new HashList();
         c.add(p);
@@ -383,7 +385,7 @@ boolean mergeStates(ProjectReload.ProjectState cached, ProjectReload.ProjectStat
     }
 
     Pair<StateRef, ProjectState> createState(ProjectReload.ProjectState previous, Project p, Collection variant, StateParts parts,
-            boolean force, StateRequest req) {
+            boolean rejectInconsistent, StateRequest req) {
         ProjectReload.ProjectState state = doCreateState(p, parts, req);
 
         ProjectReload.ProjectState cur = null;
@@ -416,6 +418,9 @@ Pair<StateRef, ProjectState> createState(ProjectReload.ProjectState previous, Pr
             if (!parts.isEmpty()) {
                 l.init();
             }
+            if (!state.isConsistent() && rejectInconsistent) {
+                return Pair.of(null, state);
+            }
             if (cur != previous && previous != null) {
                 if (LOG.isLoggable(Level.FINE)) {
                     LOG.log(Level.FINE, "Project {0}: obsolete previous state {1}", new Object[] { p, previous.toString() });
@@ -526,7 +531,7 @@ public Pair<StateRef, ProjectState> getProjectState0(Project p, Lookup context,
             }
         }
 
-        StateRef ref;
+        StateRef ref = null;
 
         // try to keep alive for the length of the operation.
         ProjectState oldS = null;
@@ -556,11 +561,9 @@ public Pair<StateRef, ProjectState> getProjectState0(Project p, Lookup context,
                 return Pair.of(ref, none);
             }
         } finally {
-            if (oldS != null) {
-                // some harmless thing that the optimizer cannot throw away and GC oldS prematurely.
-                synchronized (oldS) {
-                    oldS.notify();
-                }
+            if (oldS != null && ref.toDetach != null) {
+                // assume ref must not be null
+                ref.toDetach.checkFileTimestamps();
             }
             endOperation(p, null, null);
         }

ide/project.dependency/src/org/netbeans/modules/project/dependency/reload/StateDataListener.java

@@ -93,6 +93,28 @@ void init() {
         for (ProjectReloadImplementation.ProjectStateData<?> d : parts.values()) {
             ReloadSpiAccessor.get().addProjectStateListener(d, this);
         }
+        checkFileTimestamps();
+        checkInconsistencies();
+    }
+
+
+    public void checkFileTimestamps() {
+        ProjectState s = tracker.get();
+        if (s == null) {
+            return;
+        }
+        Map<FileObject, Collection<ProjectStateData>> wf = watchedFiles;
+        Reloader.checkFileTimestamps(s, wf);
+    }
+
+    void checkInconsistencies() {
+        ProjectState s = tracker.get();
+        if (s == null) {
+            return;
+        }
+        for (ProjectStateData d : parts.values()) {
+            Reloader.markInconsistencies(d, null, parts, s);
+        }
     }
 
     @Override
@@ -263,7 +285,7 @@ public void fileDeleted(FileEvent fe) {
 
     @Override
     public void fileChanged(FileEvent fe) {
-        reportFile(fe.getFile(), -1);
+        reportFile(fe.getFile(), fe.getFile().lastModified().getTime());
     }
 
     @Override
@@ -290,8 +312,8 @@ public void cookiesChanged(FileObject f, LookupEvent ev) {
                 }
                 ReloadApiAccessor.get().updateProjectState(state, true, false, ch, ed, null);
             }
+            watchedFiles.getOrDefault(f, Collections.emptyList()).forEach(d -> d.fireChanged(false, true));
         }
-        watchedFiles.getOrDefault(f, Collections.emptyList()).forEach(d -> d.fireChanged(false, true));
     }
 
 }

ide/project.dependency/src/org/netbeans/modules/project/dependency/spi/ProjectReloadImplementation.java

@@ -166,7 +166,7 @@ public final class ProjectStateData<D> {
             this.timestamp = timestamp;
             this.projectData = projectData;
             this.error = error;
-            if (!valid && LOG.isLoggable(Level.FINER)) {
+            if (!valid && quality != Quality.NONE && LOG.isLoggable(Level.FINER)) {
                 LOG.log(Level.FINER, "Created invalid state data {0}", toString());
                 LOG.log(Level.FINER, "Origin", new Throwable());
             }
@@ -334,17 +334,24 @@ public void fireChanged(boolean invalidate, boolean inconsistent) {
                 LOG.log(Level.FINER, "{0} got state change: invalidate={1}, inconsistent={2}", new Object[] { toString(), invalidate, inconsistent });
                 LOG.log(Level.FINEST, "Origin:", new Throwable());
             }
-            if (invalidate) {
-                this.valid = false;
+            boolean changed = false;
+            synchronized (this) {
+                if (invalidate && this.valid) {
+                    this.valid = false;
+                    changed = true;
+                }
+                if (inconsistent && this.consistent) {
+                    changed = true;
+                    this.consistent = false;
+                }
             }
-            if (inconsistent) {
-                this.consistent = false;
+            if (changed) {
+                fire();
             }
-            fire();
         }
 
         Set<Class> getInconsistencies() {
-            return inconsistencies;
+            return inconsistencies == null ? Collections.emptySet() : new HashSet<>(inconsistencies);
         }
 
         synchronized void addListener(ProjectStateListener l) {
@@ -368,12 +375,14 @@ void clear() {
                 }
             }
 
-            this.projectData = null;
-            this.changedFiles = null;
-            this.privateData = null;
-            this.lookup = null;
-            this.projectData = null;
-            this.valid = false;
+            synchronized (this) {
+                this.projectData = null;
+                this.changedFiles = null;
+                this.privateData = null;
+                this.lookup = null;
+                this.projectData = null;
+                this.valid = false;
+            }
             LOG.log(Level.FINER, "{0}: cleared.", toString());
             this.listeners.clear();
         }
@@ -500,6 +509,10 @@ public <X> ProjectStateBuilder privateData(Function<ProjectStateData<D>, X> crea
 
         public ProjectStateData build() {
             ProjectStateData d = new ProjectStateData(files == null ? Collections.emptyList() : files, valid, q, time, lkp, error, data);
+            if (!consistent) {
+                // make inconsistent
+                d.fireChanged(false, true);
+            }
             if (!d.isValid() || !d.isConsistent()) {
                 if (Reloader.LOG.isLoggable(Level.FINER)) {
                     Reloader.LOG.log(Level.FINER, "Creating obsolete StateData: {0}", d.toString());
@@ -754,7 +767,8 @@ public <T> T getLoadContext(Class<T> clazz) {
         }
 
         public <T> T ensureLoadContext(Class<T> clazz, Supplier<T> factory) {
-            if (loadContext == null) {
+            T lc = getLoadContext(clazz);
+            if (lc == null) {
                 loadContext = factory.get();
             }
             return (T)loadContext;

ide/project.dependency/test/unit/src/org/netbeans/modules/project/dependency/reload/MockProjectReloadImplementation.java

@@ -58,6 +58,7 @@ public void setProject(Project p) {
 
     public static class ProjectData {
         public String contents;
+        public volatile int counter;
 
         public ProjectData(String s) {
             this.contents = s;
@@ -118,10 +119,13 @@ public void projectDataReleased(ProjectStateData<ProjectData> data) {
 
     Reference<ProjectStateData> sdRef = new WeakReference<>(null);
 
+    volatile ProjectData loadProjectData;
+
     protected ProjectStateData doCreateStateData(Project project, ProjectReload.StateRequest request, LoadContext<ProjectData> context) {
         ProjectStateBuilder<ProjectData> b = ProjectStateData.builder(
                 request.getMinQuality().isAtLeast(ProjectReload.Quality.LOADED) ? request.getMinQuality() : ProjectReload.Quality.LOADED);
         b.timestamp(Instant.now().toEpochMilli());
+        loadProjectData = context.getLoadContext(ProjectData.class);
         ProjectStateData<ProjectData> psd = createStateData(b, request).
                 privateData(
                     (d) -> {

ide/project.dependency/test/unit/src/org/netbeans/modules/project/dependency/reload/ProjectReloadImplTest.java

@@ -55,6 +55,7 @@
 import org.netbeans.modules.project.dependency.ProjectOperationException;
 import org.netbeans.modules.project.dependency.ProjectReload;
 import org.netbeans.modules.project.dependency.ProjectReload.ProjectState;
+import org.netbeans.modules.project.dependency.ProjectReload.Quality;
 import org.netbeans.modules.project.dependency.ProjectReload.StateRequest;
 import org.netbeans.modules.project.dependency.reload.MockProjectReloadImplementation.ProjectData;
 import org.netbeans.modules.project.dependency.reload.ProjectReloadImplTest.Mock1;
@@ -80,21 +81,24 @@ public ProjectReloadImplTest(String name) {
         super(name);
     }
 
-    Logger logger;
+    Collection<Logger> loggers = new ArrayList<>();
 
     @Override
     protected void setUp() throws Exception {
         super.setUp();
         clearWorkDir();
 
-        logger = Logger.getLogger(ProjectReloadInternal.class.getName());
-        logger.setLevel(Level.FINER);
-
-        ConsoleHandler h = new ConsoleHandler();
-        h.setLevel(Level.FINER);
-        if (!Arrays.asList(logger.getHandlers()).stream().anyMatch(x -> x instanceof ConsoleHandler)) {
-            logger.addHandler(h);
-        }
+        loggers.add(Logger.getLogger(ProjectReloadInternal.class.getName()));
+        loggers.add(Logger.getLogger(Reloader.class.getName()));
+        loggers.forEach(logger -> {
+            logger.setLevel(Level.FINER);
+
+            ConsoleHandler h = new ConsoleHandler();
+            h.setLevel(Level.FINER);
+            if (!Arrays.asList(logger.getHandlers()).stream().anyMatch(x -> x instanceof ConsoleHandler)) {
+                logger.addHandler(h);
+            }
+        });
     }
 
     @Override
@@ -1400,6 +1404,9 @@ class WaitMock extends MockProjectReloadImplementation {
          * Released when reload is reached, for test synchronization
          */
         Semaphore reloadReached = new Semaphore(0);
+        Semaphore afterReloadReached = new Semaphore(0);
+
+        volatile StateRequest loadRequest;
 
         /**
          * Called will be used as a index to this
@@ -1416,6 +1423,7 @@ public WaitMock() {
 
         @Override
         public CompletableFuture<ProjectStateData<ProjectData>> reload(Project project, StateRequest request, ProjectReloadImplementation.LoadContext<ProjectData> context) {
+            this.loadRequest = request;
             synchronized (this) {
                 int n = called.getAndIncrement();
                 if (attemptReached != null && attemptReached.length > n) {
@@ -1424,6 +1432,7 @@ public CompletableFuture<ProjectStateData<ProjectData>> reload(Project project,
             }
             reloadReached.release();
             CompletableFuture<ProjectStateData<ProjectData>> f = super.reload(project, request, context);
+            afterReloadReached.release();
             return f.thenCombine(CompletableFuture.runAsync(() -> {
                 try {
                     latch.acquire();
@@ -1601,4 +1610,67 @@ public void stateChanged(ChangeEvent e) {
         // wait a litte, as these postponed actions are executed after the reload Future completes
         assertTrue(m1.dataReleaseLatch.tryAcquire(2, TimeUnit.SECONDS));
     }
+
+
+    /**
+     * Checks that if a file is modified during reload, the API will not report
+     * a failure, but attempts to retry the load.
+     */
+    public void testFileModifiedDuringLoad() throws Exception {
+        FileObject wd = FileUtil.toFileObject(getWorkDir());
+        FileObject o = FileUtil.toFileObject(getDataDir()).getFileObject("reload/Simple1._test");
+        FileObject f = FileUtil.copyFile(o, wd, wd.getName());
+
+        class WM extends WaitMock {
+            public WM() {
+            }
+
+            @Override
+            protected ProjectStateData doCreateStateData(Project project, StateRequest request, LoadContext<ProjectData> context) {
+                context.ensureLoadContext(ProjectData.class, () -> new ProjectData("")).counter++;
+                return super.doCreateStateData(project, request, context);
+            }
+
+            protected ProjectStateBuilder<ProjectData> createStateData(ProjectReloadImplementation.ProjectStateBuilder b, ProjectReload.StateRequest request) {
+                ProjectStateBuilder<ProjectData> b2 = super.createStateData(b, request);
+                return b2;
+            }
+        }
+
+        WM m1 = new WM();
+
+        TestProjectFactory.addToProject(f, (p) -> {
+            m1.project = p;
+            return m1;
+        });
+
+        Project p = ProjectManager.getDefault().findProject(f);
+
+        CompletableFuture<ProjectState> future = ProjectReload.withProjectState(p, StateRequest.refresh());
+
+        m1.afterReloadReached.acquire();
+
+        FileObject pf = f.getFileObject("project.txt");
+        Thread.sleep(2000);
+        Files.setLastModifiedTime(FileUtil.toFile(pf).toPath(), FileTime.from(Instant.now()));
+        // continue loading, this release will serve for the INITIAL non-NONE load state
+        assertSame(Quality.NONE, m1.loadRequest.getMinQuality());
+        assertEquals(1, m1.loadProjectData.counter);
+        m1.latch.release();
+
+        m1.afterReloadReached.acquire();
+        Thread.sleep(2000);
+        Files.setLastModifiedTime(FileUtil.toFile(pf).toPath(), FileTime.from(Instant.now()));
+        assertEquals(1, m1.loadProjectData.counter);
+        m1.latch.release();
+
+        m1.afterReloadReached.acquire();
+        assertEquals("The load goes through ReloadImplementation 2nd time", 2, m1.loadProjectData.counter);
+        m1.latch.release();
+
+        ProjectState ps = future.get();
+
+        assertTrue("The result is the most recent", ps.isValid());
+        assertTrue("Consistent after reload", ps.isConsistent());
+    }
 }

java/gradle.dependencies/src/org/netbeans/modules/gradle/reload/GradleReloadImplementation.java

@@ -74,6 +74,11 @@ public GradleReloadImplementation(Project project) {
         gp.addPropertyChangeListener(WeakListeners.propertyChange(reloadL, gp));
     }
 
+    // tests only
+    public ProjectStateData getLastCachedData() {
+        return last.get();
+    }
+
     private static void includeFile(File f, Collection<FileObject> into) {
         if (f == null) {
             return;
@@ -151,15 +156,9 @@ public ProjectStateData getProjectData() {
         ProjectStateData sd = ProjectStateData.builder(q).
                 files(files).
                 attachLookup(NbGradleProject.get(project).curretLookup()).
+                timestamp(time).
                 build();
-        /*
-        ProjectStateControl track = new ProjectStateControl(
-                "gradle", null, files, pq == NbGradleProject.Quality.FALLBACK || pq.betterThan(NbGradleProject.Quality.EVALUATED), 
-                q, time, NbGradleProject.get(project).projectDataLookup()
-        );
-        */
         synchronized (this) {
-//            states.add(new WeakReference<>(track));
             last = new WeakReference<>(sd);
         }
         return sd;