From ed8d1fef89b75fe6aa4bc695d6113da73f7e66c4 Mon Sep 17 00:00:00 2001 From: 3gorke Date: Mon, 7 Sep 2026 16:24:23 +0200 Subject: [PATCH] feat: import only changed files when rebuilding the index --- ...orkspace-on-start-and-re-import-on-save.md | 8 +- ...changed-files-when-rebuilding-the-index.md | 48 +++++ doc/demo/example/src/CoffeeMaker.java | 3 +- doc/spec/requirements.md | 18 +- .../openfasttrace/lsp/OftLanguageServer.java | 8 +- .../lsp/OftTextDocumentService.java | 10 +- .../lsp/index/WorkspaceIndexer.java | 147 +++++++++++++++- .../lsp/OftLanguageServerTest.java | 11 +- .../OftTextDocumentServiceLiveBufferTest.java | 8 +- .../WorkspaceIndexerIncrementalTest.java | 164 ++++++++++++++++++ .../WorkspaceIndexerOpenDocumentsTest.java | 71 ++++++++ 11 files changed, 469 insertions(+), 27 deletions(-) create mode 100644 doc/decisions/0014-import-only-changed-files-when-rebuilding-the-index.md create mode 100644 src/test/java/org/itsallcode/openfasttrace/lsp/index/WorkspaceIndexerIncrementalTest.java create mode 100644 src/test/java/org/itsallcode/openfasttrace/lsp/index/WorkspaceIndexerOpenDocumentsTest.java diff --git a/doc/decisions/0006-import-the-workspace-on-start-and-re-import-on-save.md b/doc/decisions/0006-import-the-workspace-on-start-and-re-import-on-save.md index 071fa52..6939f09 100644 --- a/doc/decisions/0006-import-the-workspace-on-start-and-re-import-on-save.md +++ b/doc/decisions/0006-import-the-workspace-on-start-and-re-import-on-save.md @@ -25,7 +25,9 @@ Chosen option: **full import on server start, then file-level re-import on `didS A `didChange` (keystroke-level) event does **not** trigger a re-import. Only saves do. -This decision covers only the **OFT workspace index**: spec items and coverage links across the whole project. That is what `Oft.importItems()` rebuilds. It is genuinely expensive to redo on every keystroke. It does **not** mean the server ignores unsaved edits. The current text of every open document is still tracked in memory. It is updated on every `didChange` (`req~live-document-buffer~1`). So requests that only need the text of the file being edited see live content. That covers hover, definition, semantic tokens and especially completion, which almost always runs on text the user has not saved yet. Only cross-file concerns that depend on the *index* lag behind until the next save. An example is whether a given ID still exists workspace-wide. +This decision covers only the **OFT workspace index**: spec items and coverage links across the whole project. That is what `Oft.importItems()` rebuilds. It is genuinely expensive to redo on every keystroke. It does **not** mean the server ignores unsaved edits. The current text of every open document is still tracked in memory. It is updated on every `didChange` (`req~live-document-buffer~2`). So requests that only need the text of the file being edited see live content. That covers hover, definition, semantic tokens and especially completion, which almost always runs on text the user has not saved yet. Only cross-file concerns that depend on the *index* lag behind until the next save. An example is whether a given ID still exists workspace-wide. + +The rebuild itself reads open documents from that buffer rather than from disk (`req~index-reads-open-documents~1`). This decision fixes **when** the index is rebuilt, not which version of a file it is built from. Reading disk for a file the editor holds open would produce a state that exists nowhere, mixing saved and unsaved files. **Excluding build output** Handing the workspace root straight to `Oft.importItems()` also indexed build output. The indexer now does its own file walk. It prunes hidden paths and the common build output names `target`, `build`, `out`, `dist` and `node_modules` at every depth, then hands the surviving files to OFT. Files without a matching importer are skipped by OFT itself, so passing single files is safe. A `.oftignore` file in the workspace root adds further glob patterns. @@ -39,7 +41,7 @@ This decision covers only the **OFT workspace index**: spec items and coverage l * Good, because a large workspace no longer freezes the editor while it is indexed. The price is that requests before the first import finishes are answered from an empty index. * Good, because separating "live document text" from "workspace index" keeps the expensive part save-gated while the cheap part stays live. Completion would be unusable if it only saw saved content. * Neutral, because the *index* lags behind unsaved edits. A newly typed spec item ID does not resolve workspace-wide until saved. That matches most build-tool-backed language servers. -* Bad, because a large workspace re-imports everything on every save, not just the changed file. Worth revisiting if profiling later shows this is too slow. +* Neutral, because a rebuild covers the whole workspace. It no longer re-parses all of it: only changed files are imported, see [ADR 0014](0014-import-only-changed-files-when-rebuilding-the-index.md). * Bad, because the debounce adds up to 300 ms of deliberate delay before the index catches up. ### Confirmation @@ -48,4 +50,4 @@ Integration tests verify that after a file save, the next definition request ret ## More Information -A possible follow-up is file-scoped re-import: re-parse only the saved file and merge it into the existing index. That needs a better understanding of OFT's import pipeline than we have now. +File-scoped re-import was the follow-up named here and is decided in [ADR 0014](0014-import-only-changed-files-when-rebuilding-the-index.md). diff --git a/doc/decisions/0014-import-only-changed-files-when-rebuilding-the-index.md b/doc/decisions/0014-import-only-changed-files-when-rebuilding-the-index.md new file mode 100644 index 0000000..1e289d2 --- /dev/null +++ b/doc/decisions/0014-import-only-changed-files-when-rebuilding-the-index.md @@ -0,0 +1,48 @@ +--- +status: accepted +date: 2026-09-07 +decision-makers: Felix Gorke +--- + +# Import Only Changed Files When Rebuilding the Index +`adr~import-only-changed-files-when-rebuilding-the-index~1` + +Needs: impl + +## Context and Problem Statement + +Every save and every file change outside the editor rebuilds the whole index ([ADR 0006](0006-import-the-workspace-on-start-and-re-import-on-save.md)), which re-parses every file in the workspace. Measuring shows: + +| workspace | walk | import | link | build | +| --- | --- | --- | --- | --- | +| this repository | 12 ms | 52 ms | 2 ms | 2 ms | +| synthetic, 6000 files | 44 ms | 1170 ms | 4 ms | 6 ms | + +Parsing is the rebuild. Linking, which is the part that has to see the whole workspace, costs almost nothing. + +## Considered Options + +* Keep re-parsing everything +* Cache the imported items per file and re-import only what changed +* Update the linked graph incrementally as well + +## Decision Outcome + +Chosen option: **cache the imported items per file, re-import only changed files, link everything**. The expensive phase becomes proportional to what the user touched, while the phase that needs global knowledge stays a full pass and therefore stays correct by construction. + +A file counts as unchanged when its size and its modification time match the ones it had at the last import. Both come from the directory walk, which reads them anyway, so the check costs no extra file access. Hashing the content instead would be exact but was measured at 686 ms for those 6000 files, which is most of what the cache saves. + +A file the editor holds open is never cached and always imported again. Its text is the buffer, not the file, and the file says nothing about it (`req~index-reads-open-documents~1`). + +Updating the linked graph incrementally was rejected: a link status such as `ORPHANED` or `AMBIGUOUS` depends on items in other files, so a change would have to be propagated across the graph for 4 ms of savings. + +### Consequences + +* Good, because a rebuild after one edit no longer scales with the size of the workspace. Measured on 6000 files: 1253 ms for a full build, 73 ms for a rebuild. +* Good, because changes made outside the editor are picked up like any other, by their timestamp. A branch switch invalidates exactly the files it rewrote. +* Good, because the cache cannot drift into a different result: the items of an unchanged file are the ones it produced, and linking sees all of them. +* Bad, because an edit that restores both size and modification time is invisible to the rebuild. That takes deliberate effort, or a filesystem whose timestamps are coarser than the interval between two writes of equal length. + +### Confirmation + +Integration tests for different cases. diff --git a/doc/demo/example/src/CoffeeMaker.java b/doc/demo/example/src/CoffeeMaker.java index bc9f83c..637759b 100644 --- a/doc/demo/example/src/CoffeeMaker.java +++ b/doc/demo/example/src/CoffeeMaker.java @@ -15,7 +15,8 @@ private void grindBeans() { private void heatWater() { } - // Intentional defect: this tag references revision 1, the design item is at revision 2. + // Intentional defect: this tag references revision 1, the design item is at + // revision 2. // [impl->dsn~heat-water~1] private void keepWarm() { } diff --git a/doc/spec/requirements.md b/doc/spec/requirements.md index d4a3814..20d3926 100644 --- a/doc/spec/requirements.md +++ b/doc/spec/requirements.md @@ -96,12 +96,26 @@ Files changed outside the editor also trigger a reindex. Needs: impl, utest ## Live Document Buffer -req~live-document-buffer~1 +req~live-document-buffer~2 -The server keeps the current in-memory text of every open document. It updates that text on `textDocument/didOpen` and `textDocument/didChange`. Requests that work on document text read from this buffer while the document is open. Those requests are hover, go to definition, find references, semantic tokens and completion. When the document is not open, they fall back to the file on disk. This buffer is separate from the OFT workspace index, which is only rebuilt on save (`req~index-refresh-on-file-change~1`). +The server keeps the current in-memory text of every open document. It updates that text on `textDocument/didOpen` and `textDocument/didChange`. Requests that work on document text read from this buffer while the document is open. Those requests are hover, go to definition, find references, semantic tokens and completion. When the document is not open, they fall back to the file on disk. Needs: impl, utest, itest +## Index Reads Open Documents +req~index-reads-open-documents~1 + +When the index is rebuilt, a file the editor holds open is read from the document buffer rather than from disk. Every other file is read from disk as before. + +Needs: impl, itest + +## Index Imports Only Changed Files +req~index-imports-changed-files-only~1 + +A rebuild imports only the files that changed since the last one and takes the items of every other file from what that run produced. A file counts as unchanged when its size and its modification time are the ones it had then. A file the editor holds open is always imported again, because its text is not the one on disk, and it is not cached either. + +Needs: impl, itest + ## Highlight Specification Item Definitions req~highlight-specification-item~1 diff --git a/src/main/java/org/itsallcode/openfasttrace/lsp/OftLanguageServer.java b/src/main/java/org/itsallcode/openfasttrace/lsp/OftLanguageServer.java index 9f50366..e85715d 100644 --- a/src/main/java/org/itsallcode/openfasttrace/lsp/OftLanguageServer.java +++ b/src/main/java/org/itsallcode/openfasttrace/lsp/OftLanguageServer.java @@ -2,6 +2,7 @@ import java.net.URI; import java.nio.file.Path; +import java.util.Map; import java.util.List; import java.util.concurrent.CompletableFuture; @@ -68,7 +69,6 @@ public CompletableFuture initialize(final InitializeParams par final var syncOptions = new TextDocumentSyncOptions(); syncOptions.setOpenClose(true); - // [impl->req~live-document-buffer~1] syncOptions.setChange(TextDocumentSyncKind.Full); syncOptions.setSave(new SaveOptions(false)); @@ -162,11 +162,15 @@ private void buildInitialIndex() { } private void rebuildIndex() { + rebuildIndex(textDocumentService.openDocuments()); + } + + private void rebuildIndex(final Map openDocuments) { if (rootUri == null) { return; } final Path workspaceRoot = Path.of(URI.create(rootUri)); - final OftWorkspaceIndex index = indexer.buildIndex(workspaceRoot); + final OftWorkspaceIndex index = indexer.buildIndex(workspaceRoot, openDocuments); textDocumentService.updateIndex(index); workspaceService.updateIndex(index); Logger.info("Workspace index ready: " + index.specItemCount() + " item(s)"); diff --git a/src/main/java/org/itsallcode/openfasttrace/lsp/OftTextDocumentService.java b/src/main/java/org/itsallcode/openfasttrace/lsp/OftTextDocumentService.java index 9fb2df0..66d3f9d 100644 --- a/src/main/java/org/itsallcode/openfasttrace/lsp/OftTextDocumentService.java +++ b/src/main/java/org/itsallcode/openfasttrace/lsp/OftTextDocumentService.java @@ -122,6 +122,14 @@ private void publishWorkspaceDiagnostics() { .forEach(workspaceDiagnosticUris::add); } + // [impl->req~index-reads-open-documents~1] + Map openDocuments() { + final Map documents = new LinkedHashMap<>(); + openDocumentBuffers.forEach((uri, lines) -> LocationConverter.toPath(uri) + .ifPresent(file -> documents.put(file, String.join("\n", lines)))); + return documents; + } + private boolean isOpenFile(final String uri) { final String key = LocationConverter.toFileKey(uri); return openUris.stream().anyMatch(open -> LocationConverter.toFileKey(open).equals(key)); @@ -604,7 +612,7 @@ public void didOpen(final DidOpenTextDocumentParams params) { publishDiagnostics(uri); } - // [impl->req~live-document-buffer~1] + // [impl->req~live-document-buffer~2] @Override public void didChange(final DidChangeTextDocumentParams params) { final String uri = params.getTextDocument().getUri(); diff --git a/src/main/java/org/itsallcode/openfasttrace/lsp/index/WorkspaceIndexer.java b/src/main/java/org/itsallcode/openfasttrace/lsp/index/WorkspaceIndexer.java index ce2c163..99bdd12 100644 --- a/src/main/java/org/itsallcode/openfasttrace/lsp/index/WorkspaceIndexer.java +++ b/src/main/java/org/itsallcode/openfasttrace/lsp/index/WorkspaceIndexer.java @@ -1,18 +1,35 @@ package org.itsallcode.openfasttrace.lsp.index; +import java.io.BufferedReader; import java.io.IOException; +import java.io.StringReader; import java.nio.file.FileVisitResult; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.SimpleFileVisitor; import java.nio.file.attribute.BasicFileAttributes; +import static java.util.stream.Collectors.groupingBy; +import static java.util.stream.Collectors.toList; +import static java.util.stream.Collectors.toSet; + import java.util.ArrayList; +import java.util.LinkedHashMap; import java.util.List; +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.TimeUnit; import org.itsallcode.openfasttrace.api.core.LinkedSpecificationItem; import org.itsallcode.openfasttrace.api.core.SpecificationItem; import org.itsallcode.openfasttrace.api.importer.ImportSettings; +import org.itsallcode.openfasttrace.api.importer.ImporterContext; +import org.itsallcode.openfasttrace.api.importer.ImporterService; +import org.itsallcode.openfasttrace.api.importer.MultiFileImporter; +import org.itsallcode.openfasttrace.api.importer.input.InputFile; +import org.itsallcode.openfasttrace.api.importer.input.RealFileInput; import org.itsallcode.openfasttrace.core.OftRunner; +import org.itsallcode.openfasttrace.core.importer.ImporterFactoryLoader; +import org.itsallcode.openfasttrace.core.importer.ImporterServiceImpl; import org.tinylog.Logger; // [impl->req~index-refresh-on-file-change~1, req~index-on-startup~3] @@ -21,6 +38,8 @@ public class WorkspaceIndexer { private final OftRunner runner; + private final Map importCache = new ConcurrentHashMap<>(); + public WorkspaceIndexer() { this(new OftRunner()); } @@ -29,15 +48,18 @@ public WorkspaceIndexer() { this.runner = runner; } - // [impl->req~diagnostic-trace-defects~3] public OftWorkspaceIndex buildIndex(final Path workspaceRoot) { + return buildIndex(workspaceRoot, Map.of()); + } + + // [impl->req~diagnostic-trace-defects~3] + public OftWorkspaceIndex buildIndex(final Path workspaceRoot, + final Map openDocuments) { Logger.info("Indexing workspace: " + workspaceRoot); // [impl->req~index-ignore-file~1] final OftIgnore ignore = OftIgnore.load(workspaceRoot); - final ImportSettings settings = ImportSettings.builder() - .addInputs(collectInputs(workspaceRoot, ignore)) - .build(); - final List items = runner.importItems(settings); + final List files = collectInputs(workspaceRoot, ignore); + final List items = importItems(files, openDocuments); final List linkedItems = runner.link(items); Logger.info("Indexed " + items.size() + " specification item(s), " + linkedItems.stream().filter(LinkedSpecificationItem::isDefect).count() @@ -45,9 +67,116 @@ public OftWorkspaceIndex buildIndex(final Path workspaceRoot) { return OftWorkspaceIndex.ofLinkedItems(linkedItems, ignore); } + private record FileToImport(Path path, String fingerprint) { + + static FileToImport of(final Path path, final BasicFileAttributes attributes) { + return new FileToImport(path, attributes.size() + "@" + + attributes.lastModifiedTime().to(TimeUnit.NANOSECONDS)); + } + + static FileToImport unknown(final Path path) { + return new FileToImport(path, null); + } + } + + private record CachedImport(String fingerprint, List items) { + } + + // [impl->req~index-imports-changed-files-only~1] + // [impl->adr~import-only-changed-files-when-rebuilding-the-index~1] + private List importItems(final List files, + final Map openDocuments) { + importCache.keySet().retainAll(files.stream().map(FileToImport::path).collect(toSet())); + final List changed = files.stream() + .filter(file -> hasChanged(file, openDocuments)) + .toList(); + final Map> imported = importChanged(changed, openDocuments); + changed.forEach(file -> cache(file, imported, openDocuments)); + Logger.debug("Imported " + changed.size() + " of " + files.size() + " file(s)"); + return files.stream() + .map(file -> imported.containsKey(file.path()) + ? imported.get(file.path()) + : cachedItemsOf(file)) + .flatMap(List::stream) + .toList(); + } + + private boolean hasChanged(final FileToImport file, final Map openDocuments) { + if (file.fingerprint() == null || openDocuments.containsKey(file.path())) { + return true; + } + final CachedImport cached = importCache.get(file.path()); + return cached == null || !cached.fingerprint().equals(file.fingerprint()); + } + + private void cache(final FileToImport file, final Map> imported, + final Map openDocuments) { + if (file.fingerprint() == null || openDocuments.containsKey(file.path())) { + importCache.remove(file.path()); + return; + } + importCache.put(file.path(), new CachedImport(file.fingerprint(), + imported.getOrDefault(file.path(), List.of()))); + } + + private List cachedItemsOf(final FileToImport file) { + final CachedImport cached = importCache.get(file.path()); + return cached == null ? List.of() : cached.items(); + } + + // [impl->req~index-reads-open-documents~1] + private static Map> importChanged( + final List files, final Map openDocuments) { + if (files.isEmpty()) { + return Map.of(); + } + final List paths = files.stream().map(FileToImport::path).toList(); + final ImportSettings settings = ImportSettings.builder().addInputs(paths).build(); + final ImporterContext context = new ImporterContext(settings); + final ImporterService importerService = + new ImporterServiceImpl(new ImporterFactoryLoader(context), settings); + context.setImporterService(importerService); + final MultiFileImporter importer = importerService.createImporter(); + for (final Path file : paths) { + importer.importFile(inputFor(file, openDocuments)); + } + return importer.getImportedItems().stream() + .collect(groupingBy(item -> Path.of(item.getLocation().getPath()), + LinkedHashMap::new, toList())); + } + + private static InputFile inputFor(final Path file, final Map openDocuments) { + final String inEditor = openDocuments.get(file); + return inEditor == null ? RealFileInput.forPath(file) + : new OpenDocumentInput(file, inEditor); + } + + private record OpenDocumentInput(Path path, String content) implements InputFile { + @Override + public BufferedReader createReader() { + return new BufferedReader(new StringReader(content)); + } + + @Override + public String getPath() { + return path.toString(); + } + + @Override + public boolean isRealFile() { + return true; + } + + @Override + public Path toPath() { + return path; + } + } + @SuppressWarnings("NullableProblems") - private static List collectInputs(final Path workspaceRoot, final OftIgnore ignore) { - final List files = new ArrayList<>(); + private static List collectInputs(final Path workspaceRoot, + final OftIgnore ignore) { + final List files = new ArrayList<>(); try { Files.walkFileTree(workspaceRoot, new SimpleFileVisitor<>() { @Override @@ -61,7 +190,7 @@ public FileVisitResult preVisitDirectory(final Path dir, final BasicFileAttribut @Override public FileVisitResult visitFile(final Path file, final BasicFileAttributes attrs) { if (OftWorkspaceIndex.isIndexedFile(ignore, file)) { - files.add(file); + files.add(FileToImport.of(file, attrs)); } return FileVisitResult.CONTINUE; } @@ -75,7 +204,7 @@ public FileVisitResult visitFileFailed(final Path file, final IOException except } catch (final IOException exception) { Logger.warn("Workspace walk failed, indexing the full root instead: " + exception.getMessage()); - return List.of(workspaceRoot); + return List.of(FileToImport.unknown(workspaceRoot)); } return files; } diff --git a/src/test/java/org/itsallcode/openfasttrace/lsp/OftLanguageServerTest.java b/src/test/java/org/itsallcode/openfasttrace/lsp/OftLanguageServerTest.java index bad5c8e..c3caa0b 100644 --- a/src/test/java/org/itsallcode/openfasttrace/lsp/OftLanguageServerTest.java +++ b/src/test/java/org/itsallcode/openfasttrace/lsp/OftLanguageServerTest.java @@ -2,6 +2,7 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.timeout; import static org.mockito.Mockito.verify; @@ -47,7 +48,7 @@ void testGivenWorkspaceFoldersAndRootUriWhenInitializingThenIndexIsBuiltFromWork @TempDir final Path fromFolders, @TempDir final Path fromRootUri) throws Exception { // given final var indexer = mock(WorkspaceIndexer.class); - when(indexer.buildIndex(any())).thenReturn(OftWorkspaceIndex.empty()); + when(indexer.buildIndex(any(), any())).thenReturn(OftWorkspaceIndex.empty()); final var serverWithIndexer = new OftLanguageServer(indexer); final var params = new InitializeParams(); params.setWorkspaceFolders( @@ -59,7 +60,7 @@ void testGivenWorkspaceFoldersAndRootUriWhenInitializingThenIndexIsBuiltFromWork serverWithIndexer.initialized(new InitializedParams()); // then - verify(indexer, timeout(INDEX_TIMEOUT_MS)).buildIndex(fromFolders); + verify(indexer, timeout(INDEX_TIMEOUT_MS)).buildIndex(eq(fromFolders), any()); } // [utest->req~index-on-startup~3] @@ -68,7 +69,7 @@ void testGivenOnlyRootUriWhenInitializingThenIndexIsBuiltFromRootUri(@TempDir fi throws Exception { // given final var indexer = mock(WorkspaceIndexer.class); - when(indexer.buildIndex(any())).thenReturn(OftWorkspaceIndex.empty()); + when(indexer.buildIndex(any(), any())).thenReturn(OftWorkspaceIndex.empty()); final var serverWithIndexer = new OftLanguageServer(indexer); final var params = new InitializeParams(); setDeprecatedRootUri(params, root); @@ -78,7 +79,7 @@ void testGivenOnlyRootUriWhenInitializingThenIndexIsBuiltFromRootUri(@TempDir fi serverWithIndexer.initialized(new InitializedParams()); // then - verify(indexer, timeout(INDEX_TIMEOUT_MS)).buildIndex(root); + verify(indexer, timeout(INDEX_TIMEOUT_MS)).buildIndex(eq(root), any()); } private static void setDeprecatedRootUri(final InitializeParams params, final Path root) { @@ -140,7 +141,7 @@ void testGivenAClientThatWatchesFilesWhenInitializedThenTheWholeWorkspaceIsWatch private static OftLanguageServer startedServer(final LanguageClient client, final Path root, final boolean watchesFiles) throws Exception { final var indexer = mock(WorkspaceIndexer.class); - when(indexer.buildIndex(any())).thenReturn(OftWorkspaceIndex.empty()); + when(indexer.buildIndex(any(), any())).thenReturn(OftWorkspaceIndex.empty()); final var server = new OftLanguageServer(indexer); server.connect(client); final var params = new InitializeParams(); diff --git a/src/test/java/org/itsallcode/openfasttrace/lsp/OftTextDocumentServiceLiveBufferTest.java b/src/test/java/org/itsallcode/openfasttrace/lsp/OftTextDocumentServiceLiveBufferTest.java index 3de4ead..45868a2 100644 --- a/src/test/java/org/itsallcode/openfasttrace/lsp/OftTextDocumentServiceLiveBufferTest.java +++ b/src/test/java/org/itsallcode/openfasttrace/lsp/OftTextDocumentServiceLiveBufferTest.java @@ -40,7 +40,7 @@ void setUp() { .build()))); } - // [utest->req~live-document-buffer~1] + // [utest->req~live-document-buffer~2] @Test void testGivenUnsavedOpenedContentWhenCompletingThenBufferContentIsUsed() throws Exception { // given @@ -57,7 +57,7 @@ void testGivenUnsavedOpenedContentWhenCompletingThenBufferContentIsUsed() throws assertThat(items).extracting(CompletionItem::getLabel).containsExactly("req~my-req~1"); } - // [utest->req~live-document-buffer~1] + // [utest->req~live-document-buffer~2] @Test void testGivenBufferReplacedByDidChangeWhenCompletingThenNewContentIsUsed() throws Exception { // given @@ -77,7 +77,7 @@ void testGivenBufferReplacedByDidChangeWhenCompletingThenNewContentIsUsed() thro assertThat(items).extracting(CompletionItem::getLabel).containsExactly("req~my-req~1"); } - // [itest->req~live-document-buffer~1] + // [itest->req~live-document-buffer~2] @Test void testGivenClosedDocumentWhenHoveringThenContentFallsBackToDisk(@TempDir final Path tempDir) throws Exception { @@ -97,7 +97,7 @@ void testGivenClosedDocumentWhenHoveringThenContentFallsBackToDisk(@TempDir fina assertThat(hover.getContents().getRight().getValue()).contains("My Requirement"); } - // [utest->req~live-document-buffer~1] + // [utest->req~live-document-buffer~2] @Test void testGivenBufferedContentWithTrailingNewlineWhenReadingLinesThenLineNumberingMatchesDisk() throws Exception { diff --git a/src/test/java/org/itsallcode/openfasttrace/lsp/index/WorkspaceIndexerIncrementalTest.java b/src/test/java/org/itsallcode/openfasttrace/lsp/index/WorkspaceIndexerIncrementalTest.java new file mode 100644 index 0000000..3250432 --- /dev/null +++ b/src/test/java/org/itsallcode/openfasttrace/lsp/index/WorkspaceIndexerIncrementalTest.java @@ -0,0 +1,164 @@ +package org.itsallcode.openfasttrace.lsp.index; + +import static org.assertj.core.api.Assertions.assertThat; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.attribute.FileTime; +import java.time.Duration; +import java.time.Instant; +import java.util.Map; + +import org.itsallcode.openfasttrace.api.core.LinkedSpecificationItem; +import org.itsallcode.openfasttrace.api.core.SpecificationItemId; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +class WorkspaceIndexerIncrementalTest { + + @TempDir + Path workspace; + + private WorkspaceIndexer indexer; + + @BeforeEach + void setUp() { + indexer = new WorkspaceIndexer(); + } + + private static String spec(final String name) { + return "# Title\n\n`req~" + name + "~1`\n\nA requirement.\n"; + } + + private Path write(final String fileName, final String content, final Duration age) + throws IOException { + final Path file = workspace.resolve(fileName); + Files.writeString(file, content); + Files.setLastModifiedTime(file, FileTime.from(Instant.now().minus(age))); + return file; + } + + private OftWorkspaceIndex reindex() { + return indexer.buildIndex(workspace, Map.of()); + } + + private static boolean holds(final OftWorkspaceIndex index, final String id) { + return index.findSpecItem(SpecificationItemId.parseId(id)).isPresent(); + } + + // [itest->req~index-imports-changed-files-only~1] + @Test + void testGivenAChangedFileWhenReindexingThenItsItemsAreUpdated() throws Exception { + // given + write("spec.md", spec("login"), Duration.ofMinutes(5)); + assertThat(holds(reindex(), "req~login~1")).isTrue(); + + // when + write("spec.md", spec("auth"), Duration.ZERO); + + // then + final OftWorkspaceIndex index = reindex(); + assertThat(holds(index, "req~auth~1")).isTrue(); + assertThat(holds(index, "req~login~1")).isFalse(); + } + + // [itest->req~index-imports-changed-files-only~1] + @Test + void testGivenAnUntouchedFileWhenReindexingThenItsItemsSurvive() throws Exception { + // given + write("kept.md", spec("login"), Duration.ofMinutes(5)); + write("edited.md", spec("logout"), Duration.ofMinutes(5)); + reindex(); + + // when + write("edited.md", spec("sign-out"), Duration.ZERO); + + // then + final OftWorkspaceIndex index = reindex(); + assertThat(holds(index, "req~login~1")).isTrue(); + assertThat(holds(index, "req~sign-out~1")).isTrue(); + } + + // [itest->req~index-imports-changed-files-only~1] + @Test + void testGivenADeletedFileWhenReindexingThenItsItemsAreGone() throws Exception { + // given + final Path spec = write("spec.md", spec("login"), Duration.ofMinutes(5)); + reindex(); + + // when + Files.delete(spec); + + // then + assertThat(holds(reindex(), "req~login~1")).isFalse(); + } + + // [itest->req~index-imports-changed-files-only~1] + @Test + void testGivenAnAddedFileWhenReindexingThenItsItemsAppear() throws Exception { + // given + write("spec.md", spec("login"), Duration.ofMinutes(5)); + reindex(); + + // when + write("more.md", spec("logout"), Duration.ZERO); + + // then + assertThat(holds(reindex(), "req~logout~1")).isTrue(); + } + + // [itest->req~index-imports-changed-files-only~1] + @Test + void testGivenAFileTheEditorHoldsWhenReindexingThenItIsImportedAgain() throws Exception { + // given + final Path spec = write("spec.md", spec("login"), Duration.ofMinutes(5)); + reindex(); + + // when + final OftWorkspaceIndex index = indexer.buildIndex(workspace, Map.of(spec, spec("auth"))); + + // then + assertThat(holds(index, "req~auth~1")).isTrue(); + assertThat(holds(index, "req~login~1")).isFalse(); + } + + // [itest->req~index-imports-changed-files-only~1] + @Test + void testGivenTheEditorClosedTheFileWhenReindexingThenTheFileOnDiskCountsAgain() + throws Exception { + // given + final Path spec = write("spec.md", spec("login"), Duration.ofMinutes(5)); + indexer.buildIndex(workspace, Map.of(spec, spec("auth"))); + + // when + final OftWorkspaceIndex index = reindex(); + + // then + assertThat(holds(index, "req~login~1")).isTrue(); + assertThat(holds(index, "req~auth~1")).isFalse(); + } + + // [itest->req~index-imports-changed-files-only~1] + @Test + void testGivenASeriesOfChangesWhenReindexingThenTheResultMatchesAFullBuild() throws Exception { + // given + write("a.md", spec("login"), Duration.ofMinutes(5)); + write("b.md", spec("logout"), Duration.ofMinutes(5)); + write("Login.java", "// [impl->req~login~1]\n", Duration.ofMinutes(5)); + reindex(); + + // when + write("b.md", spec("sign-out"), Duration.ZERO); + write("c.md", spec("reset"), Duration.ZERO); + Files.delete(workspace.resolve("a.md")); + + // then + final OftWorkspaceIndex incremental = reindex(); + final OftWorkspaceIndex full = new WorkspaceIndexer().buildIndex(workspace); + assertThat(incremental.allSpecItems()).isEqualTo(full.allSpecItems()); + assertThat(incremental.allLinkedItems().stream().filter(LinkedSpecificationItem::isDefect).count()) + .isEqualTo(full.allLinkedItems().stream().filter(LinkedSpecificationItem::isDefect).count()); + } +} diff --git a/src/test/java/org/itsallcode/openfasttrace/lsp/index/WorkspaceIndexerOpenDocumentsTest.java b/src/test/java/org/itsallcode/openfasttrace/lsp/index/WorkspaceIndexerOpenDocumentsTest.java new file mode 100644 index 0000000..cb7a4d5 --- /dev/null +++ b/src/test/java/org/itsallcode/openfasttrace/lsp/index/WorkspaceIndexerOpenDocumentsTest.java @@ -0,0 +1,71 @@ +package org.itsallcode.openfasttrace.lsp.index; + +import static org.assertj.core.api.Assertions.assertThat; + +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.Map; + +import org.itsallcode.openfasttrace.api.core.LinkedSpecificationItem; +import org.itsallcode.openfasttrace.api.core.SpecificationItemId; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +class WorkspaceIndexerOpenDocumentsTest { + + private static final String SPEC_ON_DISK = "# Login\n\n`req~login~1`\n\nNeeds: impl\n"; + + private static final String SPEC_IN_EDITOR = "# Login\n\n`req~auth~1`\n\nNeeds: impl\n"; + + // [itest->req~index-reads-open-documents~1] + @Test + void testGivenAnOpenDocumentWhenIndexingThenItsUnsavedTextIsUsed(@TempDir final Path workspace) + throws Exception { + // given + final Path spec = workspace.resolve("spec.md"); + Files.writeString(spec, SPEC_ON_DISK); + final Map openInEditor = Map.of(spec, SPEC_IN_EDITOR); + + // when + final var index = new WorkspaceIndexer().buildIndex(workspace, openInEditor); + + // then + assertThat(index.findSpecItem(SpecificationItemId.parseId("req~auth~1"))).isPresent(); + assertThat(index.findSpecItem(SpecificationItemId.parseId("req~login~1"))).isEmpty(); + } + + // [itest->req~index-reads-open-documents~1] + @Test + void testGivenNoOpenDocumentWhenIndexingThenTheFileOnDiskIsUsed(@TempDir final Path workspace) + throws Exception { + // given + final Path spec = workspace.resolve("spec.md"); + Files.writeString(spec, SPEC_ON_DISK); + + // when + final var index = new WorkspaceIndexer().buildIndex(workspace, Map.of()); + + // then + assertThat(index.findSpecItem(SpecificationItemId.parseId("req~login~1"))).isPresent(); + } + + // [itest->req~index-reads-open-documents~1] + @Test + void testGivenARenameOnlyHalfWrittenToDiskWhenIndexingThenNoDefectIsReported( + @TempDir final Path workspace) throws Exception { + // given + final Path spec = workspace.resolve("spec.md"); + Files.writeString(spec, SPEC_ON_DISK); + final Path source = workspace.resolve("Main.java"); + Files.writeString(source, "// [impl->req~auth~1]\n"); + final Map specIsOpen = Map.of(spec, SPEC_IN_EDITOR); + + // when + final var withoutBuffer = new WorkspaceIndexer().buildIndex(workspace, Map.of()); + final var withBuffer = new WorkspaceIndexer().buildIndex(workspace, specIsOpen); + + // then + assertThat(withoutBuffer.allLinkedItems()).anyMatch(LinkedSpecificationItem::isDefect); + assertThat(withBuffer.allLinkedItems()).noneMatch(LinkedSpecificationItem::isDefect); + } +}