mbien commented on code in PR #7748:
URL: https://github.com/apache/netbeans/pull/7748#discussion_r1759645966


##########
java/java.lsp.server/src/org/netbeans/modules/java/lsp/server/protocol/TextDocumentServiceImpl.java:
##########
@@ -1759,9 +1762,34 @@ public CompletableFuture<List<TextEdit>> 
willSaveWaitUntil(WillSaveTextDocumentP
     }
 
     @Override
-    public void didSave(DidSaveTextDocumentParams arg0) {
-        //TODO: nothing for now?
-        LOG.log(Level.FINER, "didSave: {0}", arg0);
+    public void didSave(DidSaveTextDocumentParams savedParams) {
+        LOG.log(Level.FINER, "didSave: {0}", savedParams);
+        FileObject file = fromURI(savedParams.getTextDocument().getUri());
+        if (file == null) {
+            return;
+        }
+        // refresh the file systems, potentially fire events
+        file.refresh();
+        EditorCookie cake = file.getLookup().lookup(EditorCookie.class);
+        if (cake == null) {
+            return;
+        }
+        StyledDocument alreadyLoaded = cake.getDocument();
+        if (alreadyLoaded == null) {
+            return;
+        }
+        try {
+            // if the FileObject.refresh() only now discovered a change, it 
have fired an event and initiated a reload, which might
+            // be still pending. Grab the reload task and wait for it:
+            Method reload = 
CloneableEditorSupport.class.getDeclaredMethod("reloadDocument");
+            reload.setAccessible(true);
+            org.openide.util.Task t = 
(org.openide.util.Task)reload.invoke(cake);
+            // wait for a limited time, this could be enough for the reload to 
complete, blocking LSP queue. We do not want to block LSP queue indefinitely:
+            // in case of an error, the server could become unresponsive.
+            t.waitFinished(300);

Review Comment:
   check return value and log if false?



##########
ide/project.dependency/src/org/netbeans/modules/project/dependency/reload/Reloader.java:
##########
@@ -383,163 +385,259 @@ private void markInconsistentParts() {
             if (inc != null) {
                 LOG.log(Level.FINE, "{0}: loader reports inconsistencies", new 
Object[]{this, inc});
             }
-            if (d != null) {
-                Set<Class> inc2 = 
ReloadSpiAccessor.get().getInconsistencies(d);
-                if (inc != null) {
-                    if (inc2 != null) {
-                        inc.addAll(inc2);
-                    }
-                    LOG.log(Level.FINE, "{0}: part {1} reports 
inconsistencies", new Object[]{this, inc2});
-                } else {
-                    inc = inc2;
+            if (markInconsistencies(d, inc, parts, this)) {
+                ctx.reloadRequested = true;
+            }
+        }
+    }
+    
+    public static boolean markInconsistencies(ProjectStateData d, 
Collection<Class> inc, StateParts parts, Object logMe) {
+        boolean requested = false;
+        if (d != null) {
+            Set<Class> inc2 = ReloadSpiAccessor.get().getInconsistencies(d);
+            if (inc != null) {
+                if (inc2 != null) {
+                    inc.addAll(inc2);
                 }
+                LOG.log(Level.FINE, "{0}: part {1} reports inconsistencies", 
new Object[]{logMe, inc2});
+            } else {
+                inc = inc2;
             }
-            if (inc != null && !inc.isEmpty()) {
-                for (Class c : inc) {
-                    Lookup.Template t = new Lookup.Template<>(c);
-                    for (ProjectReloadImplementation.ProjectStateData d2 : 
parts.values()) {
-                        if (d2 == null) {
-                            continue;
-                        }
-                        if (d2.isConsistent() && d2.isValid()) {
-                            if (c.isInstance(d2.getProjectData()) || 
(d2.getLookup() != null && d2.getLookup().lookupItem(t) != null)) {
-                                if (LOG.isLoggable(Level.FINE)) {
-                                    LOG.log(Level.FINE, "{0}: part {1} 
provides {2}, mark inconsistent and reload", new Object[]{this, d2.toString(), 
c});
-                                }
-                                ctx.reloadRequested = true;
-                                d2.fireChanged(false, true);
+        }
+        if (inc != null && !inc.isEmpty()) {
+            for (Class c : inc) {
+                Lookup.Template t = new Lookup.Template<>(c);
+                for (ProjectReloadImplementation.ProjectStateData d2 : 
parts.values()) {
+                    if (d2 == null) {
+                        continue;
+                    }
+                    if (d2.isConsistent() && d2.isValid()) {
+                        if (c.isInstance(d2.getProjectData()) || 
(d2.getLookup() != null && d2.getLookup().lookupItem(t) != null)) {
+                            if (LOG.isLoggable(Level.FINE)) {
+                                LOG.log(Level.FINE, "{0}: part {1} provides 
{2}, mark inconsistent and reload", new Object[]{logMe, d2.toString(), c});
                             }
+                            requested = true;
+                            d2.fireChanged(false, true);
                         }
                     }
                 }
             }
         }
+        return requested;
     }
-
-    @NbBundle.Messages(value = {"# {0} - project name", "# {1} - list of 
providers in loop", "ERR_ReloadingLoop=Project is reloading repeatedly. See the 
log for more details.", "# {0} - project name", 
"ERR_ProjectModifiedWhileLoading=Project {0} has been modified while 
reloading", "# {0} - project name", "ERR_ProjectQualityLow=Project {0} could 
not be loaded."})
-    private synchronized CompletableFuture<ProjectReload.ProjectState> 
finishLoadingRound() {
+    
+    /**
+     * Checks and makes a retry load. If it does, it returns a Future, 
otherwise (the current load may terminate) return {@code null}.
+     * It protects against repeated loads: if the same files are modified, or 
the same implementations request reload as the last time, the
+     * operation fails with OUT_OF_SYNC (file inconsistencies) or 
+     * @param variant
+     * @return 
+     */
+    private CompletableFuture<ProjectState> maybeMakeRetry(Collection variant) 
{
+        // special case: if some of the phases reported ERROR = code error, 
bail out and do not even try to retry
+        if (loadData.stream().anyMatch(ctx -> ctx.reportLoadError)) {
+            return null;
+        }
         Collection<LoadContextImpl> retries;
         boolean forceReload = loadData.stream().anyMatch(d -> 
d.reloadRequested);
-        markInconsistentParts();
         retries = loadData.stream().filter(d -> 
d.reloadRequested).collect(Collectors.toList());
-        Throwable error = null;
+        Collection<LoadContextImpl> fileInconsistencies = new HashSet<>();
+        checkFileTimestamps(fileInconsistencies);
+        boolean inconsistentRetry = request.isConsistent() && 
!fileInconsistencies.isEmpty();
+        if (inconsistentRetry) {
+            if (!lastInconsistent.isEmpty() && 
fileInconsistencies.containsAll(lastInconsistent)) {
+                ProjectOperationException ex = new 
ProjectOperationException(project, ProjectOperationException.State.OUT_OF_SYNC, 
Bundle.ERR_ProjectModifiedWhileLoading(ProjectUtils.getInformation(project).getDisplayName()));
+                registry.createState(originalState, project, variant, parts, 
false, request);
+                return CompletableFuture.failedFuture(ex);
+            }
+        } else if (retries.isEmpty()) {
+            return null;
+        } else if (!lastRetries.isEmpty() && retries.containsAll(lastRetries)) 
{
+            // too bad: we seem to be in a retry cycle. Bail out.
+            LOG.log(Level.WARNING, "Project {0} is reloading repetadely. The 
following provider(s) reload in a loop: {1}", new 
Object[]{project.getProjectDirectory(), lastRetries});
+            // FIXME: create a ProjectState from the data available so far.
+            ProjectOperationException ex = new 
ProjectOperationException(project, ProjectOperationException.State.ERROR, 
Bundle.ERR_ReloadingLoop(ProjectUtils.getInformation(project).getDisplayName(), 
lastRetries));
+            registry.createState(originalState, project, variant, parts, 
false, request);
+            return CompletableFuture.failedFuture(ex);
+        } 
         if (LOG.isLoggable(Level.FINE)) {
-            LOG.log(Level.FINE, "{0} load round completed.", toString());
+            String s = 
Stream.concat(fileInconsistencies.stream(),retries.stream()).
+                    map(c -> 
c.impl.getClass().getName()).collect(Collectors.joining(", "));
+            if (LOG.isLoggable(Level.FINE)) {
+                LOG.log(Level.FINE, "{0} reloads again because of {1}", new 
Object[]{toString(), s});
+            }
         }

Review Comment:
   if i see this correctly on gh, the inner guard is redundant



##########
ide/project.dependency/src/org/netbeans/modules/project/dependency/ProjectReload.java:
##########
@@ -964,7 +965,7 @@ 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, "Reload {0}, request: {1}, state {2} - NOOP, 
finished", new Object[] { p, stateRequest, lastKnown });
+            LOG.log(Level.FINE, "FINISHED: Reload {0}, request: {1}, state {2} 
- NOOP, finished", new Object[] { p, stateRequest, lastKnown.toString() });

Review Comment:
   guard with isLoggable like a few lines below. Since `toString()` of 
ProjectState is going to do some work now.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscr...@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscr...@netbeans.apache.org
For additional commands, e-mail: notifications-h...@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists

Reply via email to