diff --git a/jdtls.ext/com.microsoft.jdtls.ext.core/src/com/microsoft/jdtls/ext/core/PackageCommand.java b/jdtls.ext/com.microsoft.jdtls.ext.core/src/com/microsoft/jdtls/ext/core/PackageCommand.java index cada8754..9baf72fb 100644 --- a/jdtls.ext/com.microsoft.jdtls.ext.core/src/com/microsoft/jdtls/ext/core/PackageCommand.java +++ b/jdtls.ext/com.microsoft.jdtls.ext.core/src/com/microsoft/jdtls/ext/core/PackageCommand.java @@ -16,6 +16,7 @@ import java.util.Collections; import java.util.EnumMap; import java.util.HashMap; +import java.util.LinkedHashSet; import java.util.LinkedList; import java.util.List; import java.util.Map; @@ -371,7 +372,7 @@ private static List getPackageRootChildren(PackageParams query, IPr throw new CoreException( new Status(IStatus.ERROR, JdtlsExtActivator.PLUGIN_ID, String.format("No package root found for %s", query.getPath()))); } - List result = getPackageFragmentRootContent(packageRoot, query.isHierarchicalView(), pm); + List result = getPackageFragmentRootContent(packageRoot, query.isHierarchicalView(), query.getSyncPaths(), pm); ResourceSet resourceSet = new ResourceSet(result, query.isHierarchicalView()); ResourceVisitor visitor = new JavaResourceVisitor(packageRoot.getJavaProject()); resourceSet.accept(visitor); @@ -508,8 +509,62 @@ private static List getFolderChildren(PackageParams query, IProgres * @param pm the progress monitor */ public static List getPackageFragmentRootContent(IPackageFragmentRoot root, boolean isHierarchicalView, IProgressMonitor pm) throws CoreException { + return getPackageFragmentRootContent(root, isHierarchicalView, null, pm); + } + + public static List getPackageFragmentRootContent(IPackageFragmentRoot root, boolean isHierarchicalView, List syncPaths, IProgressMonitor pm) throws CoreException { ArrayList result = new ArrayList<>(); - refreshLocal(root.getResource(), pm); + IResource rootResource = root.getResource(); + if (rootResource instanceof IContainer && rootResource.exists() + && root.getKind() == IPackageFragmentRoot.K_SOURCE) { + // Packages created out-of-band (e.g. by code generators or + // refactor-moves that write straight to disk) are otherwise never + // surfaced. A shallow DEPTH_ONE refresh only syncs the source root's + // immediate children and never discovers brand-new nested package + // folders. Even a DEPTH_INFINITE resource refresh is not enough on + // its own: the Java Model keeps a cached list of package fragments + // for the root, so closing it forces getChildren() below to rebuild + // that list from the freshly refreshed resource tree. + // + // On auto-refresh the client passes the changed resource URIs in + // syncPaths so we only deep-refresh those subtrees instead of the + // whole source tree. If any path cannot be resolved to an existing + // resource inside this root we conservatively fall back to a full + // DEPTH_INFINITE refresh so no package is ever missed. + // See https://github.com/microsoft/vscode-java-dependency/issues/914 + boolean refreshedTargets = false; + if (syncPaths != null && !syncPaths.isEmpty()) { + Set targets = new LinkedHashSet<>(); + boolean allResolved = true; + for (String syncPath : syncPaths) { + IResource target = findNearestExistingResource(syncPath, (IContainer) rootResource); + if (target == null) { + allResolved = false; + break; + } + // Multiple changed paths can resolve to the same existing + // ancestor (e.g. several new files in one new package); the + // set keeps each distinct subtree so it is refreshed once. + targets.add(target); + } + if (allResolved && !targets.isEmpty()) { + for (IResource target : targets) { + refreshLocal(target, IResource.DEPTH_INFINITE, pm); + } + refreshedTargets = true; + } + } + if (!refreshedTargets) { + refreshLocal(rootResource, IResource.DEPTH_INFINITE, pm); + } + try { + root.close(); + } catch (JavaModelException e) { + JdtlsExtActivator.log(e); + } + } else { + refreshLocal(rootResource, IResource.DEPTH_ONE, pm); + } if (isHierarchicalView) { Map map = new HashMap<>(); for (IJavaElement child : root.getChildren()) { @@ -599,13 +654,56 @@ public static IJavaProject getJavaProject(String projectUri) { } private static void refreshLocal(IResource resource, IProgressMonitor monitor) { + refreshLocal(resource, IResource.DEPTH_ONE, monitor); + } + + private static void refreshLocal(IResource resource, int depth, IProgressMonitor monitor) { if (resource == null || !resource.exists()) { return; } try { - resource.refreshLocal(IResource.DEPTH_ONE, monitor); + resource.refreshLocal(depth, monitor); } catch (CoreException e) { JdtlsExtActivator.log(e); } } + + /** + * Resolve a changed resource URI to the nearest ancestor that already exists + * in the workspace resource tree and lives inside the given source root. + * Used to scope an auto-refresh to only the affected subtree. Returns null + * when the URI cannot be mapped to a resource within the root, in which case + * the caller falls back to a full refresh so no package is missed. + */ + private static IResource findNearestExistingResource(String uriStr, IContainer root) { + if (StringUtils.isBlank(uriStr) || root == null) { + return null; + } + try { + URI uri = JDTUtils.toURI(uriStr); + if (uri == null) { + return null; + } + IWorkspaceRoot wsRoot = ResourcesPlugin.getWorkspace().getRoot(); + IResource resource = null; + IFile[] files = wsRoot.findFilesForLocationURI(uri); + if (files.length > 0) { + resource = files[0]; + } else { + IContainer[] containers = wsRoot.findContainersForLocationURI(uri); + if (containers.length > 0) { + resource = containers[0]; + } + } + while (resource != null && !resource.exists()) { + resource = resource.getParent(); + } + if (resource != null && root.getFullPath().isPrefixOf(resource.getFullPath())) { + return resource; + } + } catch (Exception e) { + JdtlsExtActivator.logException("Failed to resolve sync path " + uriStr, e); + } + return null; + } } diff --git a/jdtls.ext/com.microsoft.jdtls.ext.core/src/com/microsoft/jdtls/ext/core/PackageParams.java b/jdtls.ext/com.microsoft.jdtls.ext.core/src/com/microsoft/jdtls/ext/core/PackageParams.java index 4b6468ed..576563c5 100644 --- a/jdtls.ext/com.microsoft.jdtls.ext.core/src/com/microsoft/jdtls/ext/core/PackageParams.java +++ b/jdtls.ext/com.microsoft.jdtls.ext.core/src/com/microsoft/jdtls/ext/core/PackageParams.java @@ -11,6 +11,8 @@ package com.microsoft.jdtls.ext.core; +import java.util.List; + import com.microsoft.jdtls.ext.core.model.NodeKind; /** @@ -31,6 +33,14 @@ public class PackageParams { private boolean isHierarchicalView; + /** + * Optional list of resource URIs (sent by the client on auto-refresh) that + * have just changed on disk. When present, the server only refreshes the + * affected subtrees instead of deeply refreshing the whole source root. + * See https://github.com/microsoft/vscode-java-dependency/issues/914 + */ + private List syncPaths; + public PackageParams() { } @@ -97,4 +107,12 @@ public void setRootPath(String rootPath) { this.rootPath = rootPath; } + public List getSyncPaths() { + return syncPaths; + } + + public void setSyncPaths(List syncPaths) { + this.syncPaths = syncPaths; + } + } diff --git a/src/syncHandler.ts b/src/syncHandler.ts index cbdd05eb..57dd97f9 100644 --- a/src/syncHandler.ts +++ b/src/syncHandler.ts @@ -90,7 +90,18 @@ class SyncHandler implements Disposable { })); this.disposables.push(watcher.onDidCreate((uri: Uri) => { - this.refresh(this.getParentNodeInExplorer(uri)); + const node: ExplorerNode | undefined = this.getParentNodeInExplorer(uri); + // When the created resource lands in a package that is not currently + // rendered, getParentNodeInExplorer resolves to the source root. Tell + // that root which path changed so the server can refresh only that + // subtree instead of deeply refreshing the whole source tree. Gate on + // the node kind (not instanceof) to avoid importing PackageRootNode + // here, which would create a module cycle and break activation. + // See https://github.com/microsoft/vscode-java-dependency/issues/914 + if (node instanceof DataNode && node.nodeData?.kind === NodeKind.PackageRoot) { + (node as unknown as { pendingSyncPaths: Set }).pendingSyncPaths.add(uri.toString()); + } + this.refresh(node); })); this.disposables.push(watcher.onDidDelete((uri: Uri) => { diff --git a/src/views/packageRootNode.ts b/src/views/packageRootNode.ts index 63de2c65..01327579 100644 --- a/src/views/packageRootNode.ts +++ b/src/views/packageRootNode.ts @@ -16,6 +16,14 @@ import { NodeFactory } from "./nodeFactory"; export class PackageRootNode extends DataNode { + /** + * Resource URIs reported by the file watcher as newly created under this + * source root since the last load. Consumed on the next loadData() so the + * server can scope its filesystem refresh to only the changed subtrees. + * See https://github.com/microsoft/vscode-java-dependency/issues/914 + */ + public pendingSyncPaths: Set = new Set(); + constructor(nodeData: INodeData, parent: DataNode, protected _project: ProjectNode) { super(nodeData, parent); } @@ -25,13 +33,28 @@ export class PackageRootNode extends DataNode { } protected async loadData(): Promise { - return Jdtls.getPackageData({ - kind: NodeKind.PackageRoot, - projectUri: this._project.nodeData.uri, - rootPath: this.nodeData.path, - handlerIdentifier: this.nodeData.handlerIdentifier, - isHierarchicalView: Settings.isHierarchicalView(), - }); + let syncPaths: string[] | undefined; + if (this.pendingSyncPaths.size) { + // Snapshot and clear synchronously before the async server call so + // watcher events arriving during the await are not lost. + syncPaths = Array.from(this.pendingSyncPaths); + this.pendingSyncPaths.clear(); + } + try { + return await Jdtls.getPackageData({ + kind: NodeKind.PackageRoot, + projectUri: this._project.nodeData.uri, + rootPath: this.nodeData.path, + handlerIdentifier: this.nodeData.handlerIdentifier, + isHierarchicalView: Settings.isHierarchicalView(), + syncPaths, + }); + } catch (error) { + // Restore the snapshot so a transient server error does not drop the + // pending paths; the next refresh will retry the targeted sync. + syncPaths?.forEach((path) => this.pendingSyncPaths.add(path)); + throw error; + } } protected createChildNodeList(): ExplorerNode[] { diff --git a/test/e2e-plans/java-dep-autorefresh-targeted.yaml b/test/e2e-plans/java-dep-autorefresh-targeted.yaml new file mode 100644 index 00000000..124c8d77 --- /dev/null +++ b/test/e2e-plans/java-dep-autorefresh-targeted.yaml @@ -0,0 +1,105 @@ +# Validates the targeted (scoped) auto-refresh optimization for issue #914. +# Companion to java-dep-refresh-generated-files.yaml (which tests manual Refresh). +# +# On auto-refresh the extension passes the changed URI to the server +# (PackageParams.syncPaths); the server refreshes only the affected subtree (the +# nearest existing ancestor) instead of the whole source root, then closes the +# package root to rebuild its package-fragment list. This plan does NOT call +# java.view.package.refresh — it relies solely on the FileSystemWatcher, so it +# exercises the syncPaths path. Auto-refresh is on by default. +# +# Tree layout and verify policy are the same as the manual-Refresh plan: compact +# virtualized tree, deterministic verifyTreeItem assertions, no `verify:` on the +# file-write step (no reliable visual signal for the LLM judge). +# +# Usage: +# npx autotest run test/e2e-plans/java-dep-autorefresh-targeted.yaml \ +# --override extensionPath= + +name: "Java Dependency — Auto-refresh surfaces a new package (targeted, #914)" +description: | + Validates the targeted (scoped) auto-refresh optimization for issue #914: a + .java file written by an external generator into a brand-new sub-package must + appear in the Java Projects view via the file-watcher auto-refresh alone (no + manual Refresh), which routes the changed URI through PackageParams.syncPaths. + +setup: + extension: "redhat.java" + vscodeVersion: "stable" + workspace: "../maven" + timeout: 180 + settings: + java.configuration.checkProjectSettingsExclusions: false + workbench.startupEditor: "none" + explorer.autoReveal: false + +steps: + - id: "ls-ready" + action: "waitForLanguageServer" + timeout: 180 + + # Free vertical space so the Java Projects tree is not virtualized. + - id: "close-aux-bar" + action: "executeVSCodeCommand workbench.action.closeAuxiliaryBar" + + - id: "collapse-outline" + action: "collapseSidebarSection OUTLINE" + + - id: "collapse-timeline" + action: "collapseSidebarSection TIMELINE" + + - id: "collapse-explorer-folders" + action: "collapseSidebarSection maven" + + - id: "focus-java-projects" + action: "executeVSCodeCommand javaProjectExplorer.focus" + + - id: "wait-tree-load" + action: "wait 3 seconds" + + # The source root must be expanded so a PackageRootNode exists to target. + - id: "expand-project" + action: "expandTreeItem my-app" + verify: "my-app project expanded" + + - id: "expand-source-root" + action: "expandTreeItem src/main/java" + verify: "source root src/main/java expanded" + + - id: "baseline-existing-pkg" + action: "wait 1 seconds" + verifyTreeItem: + name: "com.mycompany.app" + visible: true + + # Negative baseline: the brand-new package must be ABSENT before the file is + # written, so check-new-pkg-autorefresh later observes a genuine appearance + # driven by the watcher, not a pre-existing node. + - id: "baseline-new-pkg-absent" + action: "wait 1 seconds" + verifyTreeItem: + name: "com.mycompany.app.autogen" + visible: false + timeout: 5 + + # Write a file straight to disk into a brand-new sub-package. + - id: "gen-file-new-pkg" + action: "insertLineInFile src/main/java/com/mycompany/app/autogen/Gen914AutoNewPkg.java 1 package com.mycompany.app.autogen;\n\npublic class Gen914AutoNewPkg {\n}\n" + + - id: "dismiss-overlay" + action: "pressKey Escape" + + # Let the file watcher + debounced auto-refresh fire. No manual Refresh. + - id: "wait-for-auto-refresh" + action: "wait 6 seconds" + + - id: "reexpand-source-root" + action: "expandTreeItem src/main/java" + + # The brand-new package appears via auto-refresh alone. + - id: "check-new-pkg-autorefresh" + action: "wait 1 seconds" + verifyTreeItem: + name: "com.mycompany.app.autogen" + visible: true + timeout: 20 diff --git a/test/e2e-plans/java-dep-refresh-generated-files.yaml b/test/e2e-plans/java-dep-refresh-generated-files.yaml new file mode 100644 index 00000000..cf3420d0 --- /dev/null +++ b/test/e2e-plans/java-dep-refresh-generated-files.yaml @@ -0,0 +1,124 @@ +# Regression test for issue #914: manual Refresh must surface externally written +# .java files (a brand-new sub-package and a class in an existing package). +# Pre-fix the shallow DEPTH_ONE refresh never discovered the new package folder. +# +# Tree layout: the Java Projects tree is virtualized, so off-screen rows are not +# in the DOM. The Explorer file tree is collapsed and explorer.autoReveal is off +# so assertions run while the tree is compact and every package node collapsed. +# +# insertLineInFile uses fs.writeFileSync, bypassing VS Code's file service — i.e. +# exactly an external generator. Assertions use the deterministic verifyTreeItem +# DOM check; file-write and refresh steps carry no `verify:` because the LLM +# screenshot judge has no reliable visual signal for them. +# +# Usage: +# npx autotest run test/e2e-plans/java-dep-refresh-generated-files.yaml \ +# --override extensionPath= + +name: "Java Dependency — Refresh surfaces externally generated files (#914)" +description: | + Regression for issue #914. Externally written .java files — one in a brand-new + sub-package, one in an existing package — must both appear in the Java Projects + view after an explicit Refresh. + +setup: + extension: "redhat.java" + vscodeVersion: "stable" + workspace: "../maven" + timeout: 180 + settings: + java.configuration.checkProjectSettingsExclusions: false + workbench.startupEditor: "none" + explorer.autoReveal: false + +steps: + - id: "ls-ready" + action: "waitForLanguageServer" + timeout: 180 + + # Free vertical space so the Java Projects tree is not virtualized. + - id: "close-aux-bar" + action: "executeVSCodeCommand workbench.action.closeAuxiliaryBar" + + - id: "collapse-outline" + action: "collapseSidebarSection OUTLINE" + + - id: "collapse-timeline" + action: "collapseSidebarSection TIMELINE" + + - id: "collapse-explorer-folders" + action: "collapseSidebarSection maven" + + - id: "focus-java-projects" + action: "executeVSCodeCommand javaProjectExplorer.focus" + + - id: "wait-tree-load" + action: "wait 3 seconds" + + - id: "expand-project" + action: "expandTreeItem my-app" + verify: "my-app project expanded" + + - id: "expand-source-root" + action: "expandTreeItem src/main/java" + verify: "source root src/main/java expanded" + + - id: "baseline-existing-pkg" + action: "wait 1 seconds" + verifyTreeItem: + name: "com.mycompany.app" + visible: true + + # Negative baseline: the brand-new package must be ABSENT before any file is + # written. This is the "before" half of the regression's before/after check — + # it proves check-new-pkg later observes a genuine appearance, not a leftover. + - id: "baseline-new-pkg-absent" + action: "wait 1 seconds" + verifyTreeItem: + name: "com.mycompany.app.gen" + visible: false + timeout: 5 + + # Write files straight to disk: one in a brand-new sub-package, one in an + # existing package. + - id: "gen-file-new-pkg" + action: "insertLineInFile src/main/java/com/mycompany/app/gen/Gen914InNewPkg.java 1 package com.mycompany.app.gen;\n\npublic class Gen914InNewPkg {\n}\n" + + - id: "gen-file-existing-pkg" + action: "insertLineInFile src/main/java/com/mycompany/app/Gen914InExisting.java 1 package com.mycompany.app;\n\npublic class Gen914InExisting {\n}\n" + + - id: "dismiss-overlay" + action: "pressKey Escape" + + - id: "settle" + action: "wait 3 seconds" + + # The behaviour under test: an explicit manual Refresh. + - id: "manual-refresh" + action: "executeVSCodeCommand java.view.package.refresh" + + - id: "wait-after-refresh" + action: "wait 4 seconds" + + - id: "reexpand-source-root" + action: "expandTreeItem src/main/java" + + # Core regression: the brand-new package appears after Refresh. + - id: "check-new-pkg" + action: "wait 1 seconds" + verifyTreeItem: + name: "com.mycompany.app.gen" + visible: true + timeout: 15 + + # Sanity: a new class in an existing package also appears. + - id: "expand-existing-pkg" + action: "expandTreeItem com.mycompany.app" + verify: "existing package com.mycompany.app expanded" + + - id: "check-existing-pkg-class" + action: "wait 1 seconds" + verifyTreeItem: + name: "Gen914InExisting" + visible: true + timeout: 15