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