From 7746bbe256b3ab0f4f8d33112f64355ceb8e8ea6 Mon Sep 17 00:00:00 2001 From: Andrey Loskutov Date: Wed, 2 Sep 2026 15:57:52 +0200 Subject: [PATCH] File search: don't re-evaluate match filters in the UI thread FileTreeContentProvider#elementsChanged(..) evaluated the active match filters again for every match of every updated file. The filter state of a match is however already computed once per match by AbstractTextSearchResult#didAddMatch(..) (and updated when the filters change) and is cached in Match#isFiltered(), which is also what initialize(..) and AbstractTextSearchViewPage#getDisplayedMatchCount(..) use. Re-evaluating the filters is not only redundant, it is expensive: OuterProjectFileFilter calls IWorkspaceRoot#findFilesForLocationURI(..), which iterates over all projects of the workspace. With a search producing thousands of matches this ran for every match in the UI thread on every batched update and froze the UI. The provider now reads the already computed filter state instead. The collection of the updated line elements was reworked as well: the matches of a file are enumerated only once, no matter how many lines of that file were updated, only the updated lines are remembered instead of all lines of the touched files, and the enumeration stops as soon as all updated lines are known to have matches. Since line elements have identity semantics (LineElement doesn't implement equals(..)/hashCode(), matches and updates refer to the very same instance), identity based sets are used. OuterProjectFileFilter is still evaluated once per match by the search result, so it now remembers the filter state per file instead of repeating the workspace lookup for every match of that file. The states are kept in a weak map keyed by the file handles the matches hold, so they are collected together with the search result they were computed for. The state doesn't depend on the filter instance, therefore the map and the resource change listener that invalidates it are static: at most one listener is registered, no matter how many filters are created. The listener discards all remembered states if projects are added, removed, opened, closed or moved, and if the description file of a project has changed. Linked resources, resource filters and virtual folders change which files represent a location, but they are not reported by the flags of a project delta (the platform's own AliasManager uses internal lifecycle events for them); they are however stored in the project description, which is written whenever they change. Looking for that single file in the delta is much cheaper than visiting the whole delta in the notification thread, and much less disruptive than discarding the states for every added or removed file. Since the file handles are equal for all searches, a remembered state must not outlive the search it was computed for: a change the listener cannot detect, like a file added or removed below a linked folder, would otherwise still be answered from the map when the search is run again. FileSearchQuery#run(..) therefore discards the remembered states when a search is started. The listener is registered before the state of the workspace is read, and the registration is published only after it is done, so that a state is never computed and remembered while the changes that would invalidate it are not reported yet. Outdated states are discarded by replacing the whole map: a state that is computed while the map is replaced is put into the replaced map and is therefore never seen again, so an invalidation cannot be lost. Added tests for OuterProjectFileFilter (missed in the original https://github.com/eclipse-platform/eclipse.platform.text/pull/144), including the invalidation of the remembered states, and a test that the tree of the search view doesn't evaluate the match filters again. Fixes https://github.com/eclipse-platform/eclipse.platform.ui/issues/4337 Assisted-by: Github Copilot (Claude Opus 5) --- .../internal/ui/text/FileSearchQuery.java | 4 + .../ui/text/FileTreeContentProvider.java | 97 ++-- .../ui/text/OuterProjectFileFilter.java | 201 ++++++++- .../tests/filesearch/AllFileSearchTests.java | 1 + .../filesearch/NestedProjectFilterTest.java | 416 ++++++++++++++++++ 5 files changed, 660 insertions(+), 59 deletions(-) create mode 100644 tests/org.eclipse.search.tests/src/org/eclipse/search/tests/filesearch/NestedProjectFilterTest.java diff --git a/bundles/org.eclipse.search/search/org/eclipse/search/internal/ui/text/FileSearchQuery.java b/bundles/org.eclipse.search/search/org/eclipse/search/internal/ui/text/FileSearchQuery.java index 16416f74bf5..9702c9818ce 100644 --- a/bundles/org.eclipse.search/search/org/eclipse/search/internal/ui/text/FileSearchQuery.java +++ b/bundles/org.eclipse.search/search/org/eclipse/search/internal/ui/text/FileSearchQuery.java @@ -231,6 +231,10 @@ public boolean canRunInBackground() { public IStatus run(final IProgressMonitor monitor) { AbstractTextSearchResult textResult= (AbstractTextSearchResult) getSearchResult(); textResult.removeAll(); + // the filter states are remembered for all search results and are keyed by + // the file handles, they must not be reused by this new search: not every + // change of the workspace that invalidates them can be detected + OuterProjectFileFilter.clearRememberedStates(); Pattern searchPattern= getSearchPattern(); diff --git a/bundles/org.eclipse.search/search/org/eclipse/search/internal/ui/text/FileTreeContentProvider.java b/bundles/org.eclipse.search/search/org/eclipse/search/internal/ui/text/FileTreeContentProvider.java index dc9e000cd91..1fae247d5fa 100644 --- a/bundles/org.eclipse.search/search/org/eclipse/search/internal/ui/text/FileTreeContentProvider.java +++ b/bundles/org.eclipse.search/search/org/eclipse/search/internal/ui/text/FileTreeContentProvider.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2000, 2023 IBM Corporation and others. + * Copyright (c) 2000, 2026 IBM Corporation and others. * * This program and the accompanying materials * are made available under the terms of the Eclipse Public License 2.0 @@ -17,18 +17,13 @@ *******************************************************************************/ package org.eclipse.search.internal.ui.text; -import java.util.Arrays; import java.util.Collections; import java.util.Enumeration; import java.util.HashMap; import java.util.HashSet; +import java.util.IdentityHashMap; import java.util.Map; import java.util.Set; -import java.util.Spliterator; -import java.util.Spliterators; -import java.util.stream.Collectors; -import java.util.stream.Stream; -import java.util.stream.StreamSupport; import org.eclipse.core.resources.IProject; import org.eclipse.core.resources.IResource; @@ -251,22 +246,6 @@ public boolean hasChildren(Object element) { return !children.isEmpty(); } - static Stream toStream(Enumeration e) { - return StreamSupport.stream(Spliterators.spliteratorUnknownSize(e.asIterator(), Spliterator.ORDERED), false); - } - - private boolean isUnfiltered(FileMatch m) { - MatchFilter[] filters = fResult.getActiveMatchFilters(); - if (filters != null) { - for (MatchFilter filter : filters) { - if (filter.filters(m)) { - return false; - } - } - } - return true; - } - /** * * Update the search contents. Screen out any results that are filtered via @@ -281,25 +260,7 @@ private boolean isUnfiltered(FileMatch m) { @Override public synchronized void elementsChanged(Object[] updatedElements) { boolean singleElement = updatedElements.length == 1; - Set lineMatches = Collections.emptySet(); - // if we have active match filters, we should only use non-filtered FileMatch - // objects to collect LineElements to update - if (hasActiveMatchFilters()) { - lineMatches = Arrays.stream(updatedElements).filter(LineElement.class::isInstance) - // only for distinct files: - .map(u -> ((LineElement) u).getParent()).distinct() - // query matches: - .map(fResult::getMatchSet).flatMap(FileTreeContentProvider::toStream) - .map(m -> ((FileMatch) m)).filter(this::isUnfiltered).map(m -> m.getLineElement()) - .collect(Collectors.toSet()); - } else { - lineMatches = Arrays.stream(updatedElements).filter(LineElement.class::isInstance) - // only for distinct files: - .map(u -> ((LineElement) u).getParent()).distinct() - // query matches: - .map(fResult::getMatchSet).flatMap(FileTreeContentProvider::toStream) - .map(m -> ((FileMatch) m).getLineElement()).collect(Collectors.toSet()); - } + Set lineMatches = getUpdatedLinesWithMatches(updatedElements); try { for (Object updatedElement : updatedElements) { if (!(updatedElement instanceof LineElement lineElement)) { @@ -337,6 +298,58 @@ private boolean hasActiveMatchFilters() { return activeMatchFilters != null && activeMatchFilters.length > 0; } + /** + * Collects the updated line elements that still have matches. The matches of a + * file are enumerated only once, no matter how many lines of that file have been + * updated, and only the given lines are remembered instead of all lines of the + * touched files. + * + * @param updatedElements the updated elements, may contain elements that are no + * line elements + * @return the line elements of updatedElements that have at least + * one match that is not hidden by an active match filter + */ + private Set getUpdatedLinesWithMatches(Object[] updatedElements) { + // LineElement doesn't implement equals(..)/hashCode(), matches refer to the + // very same instance the update is reported for + Set updatedLines = Collections.newSetFromMap(new IdentityHashMap<>()); + Set files = new HashSet<>(); + for (Object updatedElement : updatedElements) { + if (updatedElement instanceof LineElement lineElement) { + updatedLines.add(lineElement); + files.add(lineElement.getParent()); + } + } + if (updatedLines.isEmpty()) { + return Collections.emptySet(); + } + // if we have active match filters, we should only use non-filtered FileMatch + // objects to collect LineElements to update. The filter state is evaluated + // once per match by the search result (see AbstractTextSearchResult), it must + // not be computed again here: match filters can be expensive and this code + // runs in the UI thread for every batch of search results. + boolean useFilterState = hasActiveMatchFilters(); + Set linesWithMatches = Collections.newSetFromMap(new IdentityHashMap<>()); + for (IResource file : files) { + Enumeration matches = fResult.getMatchSet(file); + while (matches.hasMoreElements()) { + Match match = matches.nextElement(); + if (useFilterState && match.isFiltered()) { + continue; + } + LineElement lineElement = ((FileMatch) match).getLineElement(); + if (!updatedLines.contains(lineElement)) { + continue; // the line is not updated, no need to remember it + } + linesWithMatches.add(lineElement); + if (linesWithMatches.size() == updatedLines.size()) { + return linesWithMatches; // all updated lines have matches + } + } + } + return linesWithMatches; + } + @Override public void clear() { initialize(fResult); diff --git a/bundles/org.eclipse.search/search/org/eclipse/search/internal/ui/text/OuterProjectFileFilter.java b/bundles/org.eclipse.search/search/org/eclipse/search/internal/ui/text/OuterProjectFileFilter.java index 021c1cd8df2..fb8d61aa371 100644 --- a/bundles/org.eclipse.search/search/org/eclipse/search/internal/ui/text/OuterProjectFileFilter.java +++ b/bundles/org.eclipse.search/search/org/eclipse/search/internal/ui/text/OuterProjectFileFilter.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2023 Red Hat Inc. and others. + * Copyright (c) 2023, 2026 Red Hat Inc. and others. * * This program and the accompanying materials * are made available under the terms of the Eclipse Public License 2.0 @@ -15,9 +15,20 @@ import java.net.URI; import java.util.Arrays; +import java.util.Collections; import java.util.Comparator; +import java.util.Map; +import java.util.Optional; +import java.util.WeakHashMap; + +import org.eclipse.core.runtime.IPath; import org.eclipse.core.resources.IFile; +import org.eclipse.core.resources.IProjectDescription; +import org.eclipse.core.resources.IResourceChangeEvent; +import org.eclipse.core.resources.IResourceChangeListener; +import org.eclipse.core.resources.IResourceDelta; +import org.eclipse.core.resources.ResourcesPlugin; import org.eclipse.search.internal.ui.SearchMessages; import org.eclipse.search.ui.text.Match; @@ -25,25 +36,181 @@ public class OuterProjectFileFilter extends MatchFilter { + /** + * Remembers for the files of the reported matches whether they are filtered. + *

+ * The filter state is evaluated for every single match, but + * {@link org.eclipse.core.resources.IWorkspaceRoot#findFilesForLocationURI(URI)} + * iterates over all projects of the workspace and is therefore much too + * expensive to be called once per match: a file usually has many matches. + *

+ *

+ * The keys are the file handles held by the matches + * ({@link Match#getElement()}), and the values don't reference them, so the + * remembered states are garbage collected together with the search result they + * were computed for: the filter doesn't keep them alive. + *

+ *

+ * The state doesn't depend on the filter instance, so it is shared by all of + * them: the states are computed for all file search results anyway and one + * single resource change listener is sufficient to invalidate them. They are + * however not shared by subsequent searches: {@link #clearRememberedStates()} + * discards them whenever a search is started, so that a state can never be + * reused by a search that was started after the workspace has changed in a way + * the listener cannot detect. + *

+ *

+ * Outdated states are discarded by replacing the whole map. A state that is + * computed while the map is replaced is put into the replaced map and is + * therefore never seen again, so an invalidation cannot be lost. + *

+ */ + private static volatile Map filterStates = newFilterStates(); + + private static final Object listenerLock = new Object(); + + /** + * Whether {@link #PROJECT_CHANGE_LISTENER} is registered. Written only while + * {@link #listenerLock} is held and only after the registration, so + * that a thread which reads true is guaranteed to be notified + * about the changes that happen from now on. + */ + private static volatile boolean isListening; + + /** + * The files representing a location change if projects are added, removed, + * opened, closed or moved, and if linked resources are created, changed or + * removed. + *

+ * The links are stored in the description of their project, which is written + * whenever a link is added, changed or removed, so a change of the description + * file is a sufficient (and cheap to detect) indication: the whole delta must + * not be visited in the notification thread. The same is true for resource + * filters and for virtual folders. + *

+ *

+ * Not reported is the creation or deletion of a file below a linked folder, + * although such a file can be represented by more than one project as well. + * Invalidating the states for every added or removed file would discard them + * whenever a build or a refresh touches the workspace, which would defeat the + * purpose of remembering them. Since the states are remembered for all search + * results and are keyed by the file handles, which are equal for all searches, + * such a state must not be able to outlive the search it was computed for: + * {@link #clearRememberedStates()} discards them whenever a search is started. + *

+ */ + private static final IResourceChangeListener PROJECT_CHANGE_LISTENER = event -> { + if (affectsProjects(event.getDelta())) { + filterStates = newFilterStates(); + } + }; + + /** + * The links, filters and the location of a project are stored in this file. + */ + private static final IPath PROJECT_DESCRIPTION_FILE = IPath.fromOSString(IProjectDescription.DESCRIPTION_FILE_NAME); + + private static Map newFilterStates() { + return Collections.synchronizedMap(new WeakHashMap<>()); + } + + /** + * Discards all remembered filter states, so that they are computed again for + * the matches that are reported from now on. Called when a file search is + * started: the states are shared by all search results and are keyed by the + * file handles the matches hold, which are equal for all searches, so a state + * must not be reused by a subsequent search. + *

+ * This is what makes the changes that {@link #PROJECT_CHANGE_LISTENER} cannot + * detect - a file added or removed below a linked folder - recoverable by + * running the search again. + *

+ */ + public static void clearRememberedStates() { + filterStates= newFilterStates(); + } + + /** + * Registers the resource change listener on the first evaluation of a match and + * returns only after it is registered, so that no filter state is ever computed + * and remembered while the changes that invalidate it are not reported yet. The + * listener is registered only once and is never removed: it is as long-living + * as this class and doesn't hold on to any state. + */ + private static void ensureListeningToProjectChanges() { + // the volatile read avoids locking and a contended write for every match + if (isListening) { + return; + } + synchronized (listenerLock) { + if (!isListening) { + ResourcesPlugin.getWorkspace().addResourceChangeListener(PROJECT_CHANGE_LISTENER, + IResourceChangeEvent.POST_CHANGE); + // published after the registration: a thread that skips the lock + // because it reads 'true' has already missed no notification + isListening= true; + } + } + } + + private static boolean affectsProjects(IResourceDelta delta) { + if (delta == null) { + return false; + } + for (IResourceDelta projectDelta : delta.getAffectedChildren()) { + if (projectDelta.getKind() != IResourceDelta.CHANGED) { + return true; // project added or removed + } + int flags= projectDelta.getFlags(); + if ((flags & (IResourceDelta.OPEN | IResourceDelta.DESCRIPTION | IResourceDelta.MOVED_FROM + | IResourceDelta.MOVED_TO | IResourceDelta.LOCAL_CHANGED | IResourceDelta.REPLACED)) != 0) { + return true; + } + // linked resources, filters and virtual folders are stored in the + // description of their project: looking for that single file is much + // cheaper than visiting the whole delta + if (projectDelta.findMember(PROJECT_DESCRIPTION_FILE) != null) { + return true; + } + } + return false; + } + @Override public boolean filters(Match match) { - if (match instanceof FileMatch) { - IFile file = ((FileMatch) match).getFile(); - URI locationUri = file.getLocationURI(); - - IFile innermostFile = locationUri == null ? file : // - Arrays.stream(file.getWorkspace().getRoot().findFilesForLocationURI(locationUri)) // - // Don't consider the content of a closed project - // for filtering because the matches there cannot be - // shown - .filter(aFile -> aFile.getProject().isAccessible()) - .min(Comparator.comparingInt(aFile -> aFile.getFullPath().segments().length)) - // shortest workspace (project relative) full path - // means most nested project - .orElse(file); - return !file.equals(innermostFile); + if (!(match instanceof FileMatch fileMatch)) { + return false; } - return false; + IFile file= fileMatch.getFile(); + // the listener is registered before the state of the workspace is read, so + // that every change that invalidates the computed state is reported: the + // state of the workspace is read after the changes this thread may miss + ensureListeningToProjectChanges(); + Map states= filterStates; + Boolean isFiltered= states.get(file); + if (isFiltered == null) { + // computed without holding a lock: it may be computed twice for a file, + // but it must not block the other search threads + isFiltered= Boolean.valueOf(computeIsFiltered(file)); + states.put(file, isFiltered); + } + return isFiltered.booleanValue(); + } + + private static boolean computeIsFiltered(IFile file) { + URI locationUri= file.getLocationURI(); + if (locationUri == null) { + return false; + } + Optional innermostFile= Arrays + .stream(file.getWorkspace().getRoot().findFilesForLocationURI(locationUri)) // + // Don't consider the content of a closed project for filtering + // because the matches there cannot be shown + .filter(aFile -> aFile.getProject().isAccessible()) + // shortest workspace (project relative) full path means most + // nested project + .min(Comparator.comparingInt(aFile -> aFile.getFullPath().segments().length)); + return innermostFile.isPresent() && !file.equals(innermostFile.get()); } @Override diff --git a/tests/org.eclipse.search.tests/src/org/eclipse/search/tests/filesearch/AllFileSearchTests.java b/tests/org.eclipse.search.tests/src/org/eclipse/search/tests/filesearch/AllFileSearchTests.java index 5647b6ac2e9..36946714cf9 100644 --- a/tests/org.eclipse.search.tests/src/org/eclipse/search/tests/filesearch/AllFileSearchTests.java +++ b/tests/org.eclipse.search.tests/src/org/eclipse/search/tests/filesearch/AllFileSearchTests.java @@ -23,6 +23,7 @@ AnnotationManagerTest.class, FileSearchTests.class, LineAnnotationManagerTest.class, + NestedProjectFilterTest.class, PositionTrackerTest.class, ResultUpdaterTest.class, SearchResultPageTest.class, diff --git a/tests/org.eclipse.search.tests/src/org/eclipse/search/tests/filesearch/NestedProjectFilterTest.java b/tests/org.eclipse.search.tests/src/org/eclipse/search/tests/filesearch/NestedProjectFilterTest.java new file mode 100644 index 00000000000..88e1e25229d --- /dev/null +++ b/tests/org.eclipse.search.tests/src/org/eclipse/search/tests/filesearch/NestedProjectFilterTest.java @@ -0,0 +1,416 @@ +/******************************************************************************* + * Copyright (c) 2026 Andrey Loskutov and others. + * + * This program and the accompanying materials + * are made available under the terms of the Eclipse Public License 2.0 + * which accompanies this distribution, and is available at + * https://www.eclipse.org/legal/epl-2.0/ + * + * SPDX-License-Identifier: EPL-2.0 + * + * Contributors: + * Andrey Loskutov - initial API and implementation + *******************************************************************************/ +package org.eclipse.search.tests.filesearch; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.util.concurrent.atomic.AtomicInteger; + +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import org.eclipse.swt.widgets.Display; + +import org.eclipse.core.runtime.jobs.IJobManager; +import org.eclipse.core.runtime.jobs.Job; + +import org.eclipse.core.resources.IFile; +import org.eclipse.core.resources.IFolder; +import org.eclipse.core.resources.IProject; +import org.eclipse.core.resources.IProjectDescription; +import org.eclipse.core.resources.IResource; +import org.eclipse.core.resources.IWorkspace; +import org.eclipse.core.resources.ResourcesPlugin; + +import org.eclipse.jface.viewers.AbstractTreeViewer; + +import org.eclipse.search.internal.ui.text.FileMatch; +import org.eclipse.search.internal.ui.text.FileSearchPage; +import org.eclipse.search.internal.ui.text.FileSearchQuery; +import org.eclipse.search.internal.ui.text.FileSearchResult; +import org.eclipse.search.internal.ui.text.OuterProjectFileFilter; +import org.eclipse.search.tests.ResourceHelper; +import org.eclipse.search.tests.SearchTestUtil; +import org.eclipse.search.ui.ISearchResultViewPart; +import org.eclipse.search.ui.NewSearchUI; +import org.eclipse.search.ui.text.AbstractTextSearchViewPage; +import org.eclipse.search.ui.text.FileTextSearchScope; +import org.eclipse.search.ui.text.Match; +import org.eclipse.search.ui.text.MatchFilter; + +/** + * Tests the match filter that hides the matches of files which are reported for + * an outer project although they belong to a nested project, see + * https://github.com/eclipse-platform/eclipse.platform.text/issues/143 + *

+ * The tests use two projects that share the same file on disk: the location of + * the inner project is a folder of the outer project, so the very same file is + * represented by two resources and is reported twice by a search. + *

+ */ +public class NestedProjectFilterTest { + + private static final String OUTER_PROJECT_NAME= "nested-project-filter-outer"; + + private static final String INNER_PROJECT_NAME= "nested-project-filter-inner"; + + private static final String FILE_NAME= "test.txt"; + + private static final String SEARCH_STRING= "nestedProjectFilterNeedle"; + + private IProject outerProject; + + private IProject innerProject; + + /** The file as seen by the inner (innermost) project. */ + private IFile innerFile; + + /** The very same file on disk, as seen by the enclosing outer project. */ + private IFile outerFile; + + private FileSearchPage page; + + private int previousLayout; + + private MatchFilter[] lastUsedFilters; + + @BeforeEach + public void setUp() throws Exception { + SearchTestUtil.ensureWelcomePageClosed(); + // new search results pick up the last used filters, start without any + lastUsedFilters= FileSearchResult.getLastUsedFilters(); + FileSearchResult.setLastUsedFilters(new MatchFilter[0]); + + outerProject= ResourceHelper.createProject(OUTER_PROJECT_NAME); + IFolder nestedFolder= ResourceHelper.createFolder(outerProject.getFolder(INNER_PROJECT_NAME)); + outerFile= ResourceHelper.createFile(nestedFolder, FILE_NAME, SEARCH_STRING); + innerProject= createProjectAt(INNER_PROJECT_NAME, nestedFolder); + innerFile= innerProject.getFile(FILE_NAME); + + assertTrue(innerFile.exists(), "the nested project must see the file of the outer project"); + assertEquals(outerFile.getLocationURI(), innerFile.getLocationURI(), + "both resources must represent the same file on disk"); + assertTrue(innerFile.getFullPath().segmentCount() < outerFile.getFullPath().segmentCount(), + "the file of the innermost project must have the shortest path"); + } + + @AfterEach + public void tearDown() throws Exception { + if (page != null) { + // the layout is shared by all file search pages + page.setLayout(previousLayout); + page= null; + } + // setActiveMatchFilters(..) persists the filters in the dialog settings + FileSearchResult.setLastUsedFilters(lastUsedFilters); + // the inner project is located inside the outer one, delete it first + ResourceHelper.deleteProject(INNER_PROJECT_NAME); + ResourceHelper.deleteProject(OUTER_PROJECT_NAME); + } + + /** + * Only the file of the outer project is a duplicate, the file of the innermost + * project is the one to show. + */ + @Test + public void testDuplicateOfOuterProjectIsFiltered() { + OuterProjectFileFilter filter= new OuterProjectFileFilter(); + + assertTrue(filter.filters(new FileMatch(outerFile)), + "the file reported for the outer project must be filtered"); + assertFalse(filter.filters(new FileMatch(innerFile)), + "the file of the innermost project must not be filtered"); + } + + /** + * The filter remembers its answer per file, repeated evaluations must not + * change the result. + */ + @Test + public void testRepeatedEvaluationIsStable() { + OuterProjectFileFilter filter= new OuterProjectFileFilter(); + + for (int i= 0; i < 3; i++) { + assertTrue(filter.filters(new FileMatch(outerFile)), "evaluation " + i); + assertFalse(filter.filters(new FileMatch(innerFile)), "evaluation " + i); + } + } + + /** + * The matches of a closed project cannot be shown, so the file of the outer + * project is not a duplicate anymore once the inner project is closed. The + * filter must not answer with an outdated (remembered) state. + */ + @Test + public void testFilterIsUpdatedWhenInnerProjectIsClosed() throws Exception { + OuterProjectFileFilter filter= new OuterProjectFileFilter(); + assertTrue(filter.filters(new FileMatch(outerFile)), "precondition: the file is a duplicate"); + + innerProject.close(null); + + assertFiltersEventually(filter, outerFile, false, + "the file of the outer project is the only one that can be shown now"); + } + + /** + * The remembered filter states are shared by all instances of the filter, they + * must be invalidated for all of them if the inner project is deleted. + */ + @Test + public void testFilterIsUpdatedWhenInnerProjectIsDeleted() throws Exception { + OuterProjectFileFilter filter= new OuterProjectFileFilter(); + assertTrue(filter.filters(new FileMatch(outerFile)), "precondition: the file is a duplicate"); + + // deletes the project but keeps its content, which is owned by the outer + // project as well + innerProject.delete(false, true, null); + + assertFiltersEventually(new OuterProjectFileFilter(), outerFile, false, + "the file of the outer project is the only one left"); + assertFiltersEventually(filter, outerFile, false, "the remembered state must be invalidated"); + } + + /** + * A file that exists only once must never be filtered. + */ + @Test + public void testUniqueFileIsNotFiltered() throws Exception { + IFolder folder= ResourceHelper.createFolder(outerProject.getFolder("unique")); + IFile uniqueFile= ResourceHelper.createFile(folder, FILE_NAME, SEARCH_STRING); + OuterProjectFileFilter filter= new OuterProjectFileFilter(); + + assertFalse(filter.filters(new FileMatch(uniqueFile))); + } + + /** + * A linked resource represents the file of another project without any change + * of the project itself: the link is stored in the project description, the + * flags of the project delta don't report it. + */ + @Test + public void testFilterIsUpdatedWhenLinkIsCreated() throws Exception { + IFolder folder= ResourceHelper.createFolder(outerProject.getFolder("linkTarget")); + IFile linkTarget= ResourceHelper.createFile(folder, "linked.txt", SEARCH_STRING); + OuterProjectFileFilter filter= new OuterProjectFileFilter(); + assertFalse(filter.filters(new FileMatch(linkTarget)), "precondition: the file exists only once"); + + IFile link= innerProject.getFile("linked.txt"); + link.createLink(linkTarget.getLocationURI(), IResource.NONE, null); + + assertTrue(link.getFullPath().segmentCount() < linkTarget.getFullPath().segmentCount(), + "the link must be the innermost representation of the file"); + assertFiltersEventually(filter, linkTarget, true, + "the file is represented by the link of the inner project as well now"); + } + + /** + * Without the filter the same file is reported twice, with the filter only the + * matches of the innermost project are shown. + */ + @Test + public void testFilterStateOfSearchResult() throws Exception { + FileSearchQuery query= createQuery(); + NewSearchUI.runQueryInForeground(null, query); + FileSearchResult result= (FileSearchResult) query.getSearchResult(); + + assertEquals(2, result.getMatchCount(), "the same file must be found in both projects"); + assertEquals(1, result.getMatchCount(innerFile)); + assertEquals(1, result.getMatchCount(outerFile)); + + result.setActiveMatchFilters(new MatchFilter[] { getInnermostProjectFilter(result) }); + + assertTrue(isFiltered(result, outerFile), "the duplicate of the outer project must be filtered"); + assertFalse(isFiltered(result, innerFile), "the file of the innermost project must be shown"); + assertEquals(2, result.getMatchCount(), "filtered matches are still part of the result"); + } + + /** + * The filtered matches must not be shown in the tree of the search view. + */ + @Test + public void testFilteredFileIsNotShownInTree() throws Exception { + FileSearchQuery query= createQuery(); + NewSearchUI.runQueryInForeground(null, query); + FileSearchResult result= (FileSearchResult) query.getSearchResult(); + + ISearchResultViewPart view= NewSearchUI.getSearchResultView(); + page= (FileSearchPage) view.getActivePage(); + previousLayout= page.getLayout(); + page.setLayout(AbstractTextSearchViewPage.FLAG_LAYOUT_TREE); + result.setActiveMatchFilters(new MatchFilter[] { getInnermostProjectFilter(result) }); + consumeEvents(); + + AbstractTreeViewer viewer= (AbstractTreeViewer) page.getViewer(); + viewer.expandAll(); + + assertNotNull(viewer.testFindItem(innerFile), "the file of the innermost project must be shown"); + assertNull(viewer.testFindItem(outerFile), "the duplicate of the outer project must not be shown"); + } + + /** + * The remembered filter states are shared by all search results and are keyed + * by the file handles, which are equal for all searches: a state must never be + * reused by a search that is started later. + */ + @Test + public void testStatesAreNotReusedByNextSearch() throws Exception { + FileSearchQuery query= createQuery(); + NewSearchUI.runQueryInForeground(null, query); + FileSearchResult result= (FileSearchResult) query.getSearchResult(); + result.setActiveMatchFilters(new MatchFilter[] { getInnermostProjectFilter(result) }); + assertTrue(isFiltered(result, outerFile), "precondition: the duplicate is filtered"); + + innerProject.close(null); + NewSearchUI.runQueryInForeground(null, query); + + assertEquals(1, result.getMatchCount(), "only the file of the outer project can be found now"); + assertFalse(isFiltered(result, outerFile), "the file of the outer project must be shown now"); + } + + /** + * The filter state of a match is evaluated by the search result, the tree of the + * search view must not evaluate the (potentially expensive) match filters again + * in the UI thread, see + * https://github.com/eclipse-platform/eclipse.platform.ui/issues/4337 + */ + @Test + public void testTreeUpdateDoesNotReevaluateMatchFilters() throws Exception { + FileSearchQuery query= createQuery(); + NewSearchUI.runQueryInForeground(null, query); + FileSearchResult result= (FileSearchResult) query.getSearchResult(); + + ISearchResultViewPart view= NewSearchUI.getSearchResultView(); + page= (FileSearchPage) view.getActivePage(); + previousLayout= page.getLayout(); + page.setLayout(AbstractTextSearchViewPage.FLAG_LAYOUT_TREE); + CountingMatchFilter filter= new CountingMatchFilter(); + result.setActiveMatchFilters(new MatchFilter[] { filter }); + consumeEvents(); + + FileMatch existingMatch= (FileMatch) result.getMatches(innerFile)[0]; + filter.evaluations.set(0); + + // updates the line element of the match in the tree + result.addMatch(new FileMatch(innerFile, existingMatch.getOffset(), existingMatch.getLength(), + existingMatch.getLineElement())); + consumeEvents(); + + assertEquals(1, filter.evaluations.get(), + "the filter must be evaluated once for the added match and not again for the tree update"); + } + + private static void assertFiltersEventually(MatchFilter filter, IFile file, boolean expected, String message) { + // the resource change notification is sent when the operation that changed the + // workspace is finished, which may happen asynchronously + long timeout= System.currentTimeMillis() + 10_000; + boolean actual= filter.filters(new FileMatch(file)); + while (actual != expected && System.currentTimeMillis() < timeout) { + runEventLoop(); + Thread.yield(); + actual= filter.filters(new FileMatch(file)); + } + assertEquals(expected, actual, message); + } + + private static boolean isFiltered(FileSearchResult result, IFile file) { + Match[] matches= result.getMatches(file); + assertEquals(1, matches.length, "unexpected number of matches for " + file.getFullPath()); + return matches[0].isFiltered(); + } + + private static MatchFilter getInnermostProjectFilter(FileSearchResult result) { + for (MatchFilter filter : result.getAllMatchFilters()) { + if (filter instanceof OuterProjectFileFilter) { + return filter; + } + } + throw new AssertionError("the file search result must provide the innermost project filter"); + } + + private FileSearchQuery createQuery() { + FileTextSearchScope scope= FileTextSearchScope.newSearchScope( + new IResource[] { innerProject, outerProject }, new String[] { "*.txt" }, false); + return new FileSearchQuery(SEARCH_STRING, false, true, scope); + } + + /** + * Creates a project at the location of the given folder, so that the content of + * the folder belongs to two projects. + */ + private static IProject createProjectAt(String projectName, IFolder folder) throws Exception { + IWorkspace workspace= ResourcesPlugin.getWorkspace(); + IProject project= workspace.getRoot().getProject(projectName); + IProjectDescription description= workspace.newProjectDescription(projectName); + description.setLocation(folder.getLocation()); + project.create(description, null); + project.open(null); + project.refreshLocal(IResource.DEPTH_INFINITE, null); + folder.getProject().refreshLocal(IResource.DEPTH_INFINITE, null); + return project; + } + + private void consumeEvents() { + IJobManager manager= Job.getJobManager(); + while (manager.find(page).length > 0) { + runEventLoop(); + } + runEventLoop(); + } + + private static void runEventLoop() { + Display display= Display.getCurrent(); + while (display != null && display.readAndDispatch()) { + // process all pending events + } + } + + /** + * Counts how often the filter state of a match is evaluated. + */ + private static final class CountingMatchFilter extends MatchFilter { + + final AtomicInteger evaluations= new AtomicInteger(); + + @Override + public boolean filters(Match match) { + evaluations.incrementAndGet(); + return false; + } + + @Override + public String getName() { + return "counting"; + } + + @Override + public String getDescription() { + return "counts the evaluations of the filter"; + } + + @Override + public String getActionLabel() { + return getName(); + } + + @Override + public String getID() { + return "org.eclipse.search.tests.countingFilter"; + } + } +}