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..a66a8199166 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 @@ -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,55 @@ 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) && linesWithMatches.add(lineElement) + && 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..3123330a944 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,18 @@ 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 java.util.concurrent.atomic.AtomicBoolean; import org.eclipse.core.resources.IFile; +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 +34,102 @@ 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, + * which is shared by all file search results, doesn't keep them alive. + *

+ *

+ * 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 volatile Map filterStates = newFilterStates(); + + private final AtomicBoolean isListening = new AtomicBoolean(); + + /** + * The files representing a location only change if projects are added, removed, + * opened, closed or moved. + */ + private final IResourceChangeListener projectChangeListener = event -> { + if (affectsProjects(event.getDelta())) { + filterStates = newFilterStates(); + } + }; + + private static Map newFilterStates() { + return Collections.synchronizedMap(new WeakHashMap<>()); + } + + private void ensureListeningToProjectChanges() { + if (!isListening.get() && isListening.compareAndSet(false, true)) { + ResourcesPlugin.getWorkspace().addResourceChangeListener(projectChangeListener, + IResourceChangeEvent.POST_CHANGE); + } + } + + 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; + } + } + 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 + 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(computeFilters(file)); + states.put(file, isFiltered); + } + return isFiltered.booleanValue(); + } + + private static boolean computeFilters(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..cf3bb1a4282 --- /dev/null +++ b/tests/org.eclipse.search.tests/src/org/eclipse/search/tests/filesearch/NestedProjectFilterTest.java @@ -0,0 +1,276 @@ +/******************************************************************************* + * 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 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); + + assertFalse(filter.filters(new FileMatch(outerFile)), + "the file of the outer project is the only one that can be shown now"); + } + + /** + * 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))); + } + + /** + * 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"); + } + + 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 + } + } +}