diff --git a/dotCMS/src/main/java/com/dotcms/browser/BrowserAPIImpl.java b/dotCMS/src/main/java/com/dotcms/browser/BrowserAPIImpl.java index 1ecd66e1bd9d..7d63136e47ca 100644 --- a/dotCMS/src/main/java/com/dotcms/browser/BrowserAPIImpl.java +++ b/dotCMS/src/main/java/com/dotcms/browser/BrowserAPIImpl.java @@ -253,6 +253,27 @@ ContentUnderParent getContentUnderParentFromDB(final BrowserQuery browserQuery, * Pagination resumes from {@link BrowserQuery#contentCursor}, which is the DB row offset * returned by the previous page. On the first page it is 0. *

+ *

+ * The scan's cost cap depends on {@code applyESFilter}: when {@code true} (text filtering + * through Elasticsearch), it is bounded by elapsed time + * ({@code BROWSER_DB_MAX_SCAN_TIME_MILLIS}), so an unfiltered global search keeps scanning + * while still affordable instead of giving up at an arbitrary row count and silently + * dropping matches that sit later in DB order (see issue #37211), plus a much higher hard + * row ceiling ({@code BROWSER_DB_MAX_SCAN_ROWS_ES_HARD_CAP}) as a co-bound so a fast DB/ES + * pair -- or many concurrent unfiltered searches -- cannot turn the time budget into + * unbounded per-request work (found in review). When {@code false} (permission-only + * filtering), it stays bounded by row count ({@code BROWSER_DB_MAX_SCAN_ROWS}), unchanged + * from before. + *

+ *

+ * Completeness here is best-effort, not a hard guarantee. Because the ES-narrowed + * cutoff depends on wall-clock time, a slower or more heavily loaded node can exhaust the + * budget before reaching a match that an idle node would find in the same request, so two + * otherwise-identical requests against the same data can return different result sets + * depending on load (found in review). This trades the old cutoff's determinism for + * completeness under normal conditions; it does not eliminate the possibility of a dropped + * match under sustained load, only make it far less likely and no longer position-dependent. + *

* * @param browserQuery query containing search criteria, user context, and the current cursor * @param maxRows maximum number of permission-visible items to return @@ -269,13 +290,38 @@ private ContentUnderParent getContentByChunks(final BrowserQuery browserQuery, final int maxRows, final SelectQuery sqlQuery, final int chunkSize, final boolean applyESFilter) throws DotDataException, DotSecurityException { - final int scanLimit = Config.getIntProperty(BROWSER_DB_MAX_SCAN_ROWS_KEY, BROWSER_DB_MAX_SCAN_ROWS_DEFAULT); + final int scanRowLimit = Config.getIntProperty(BROWSER_DB_MAX_SCAN_ROWS_KEY, BROWSER_DB_MAX_SCAN_ROWS_DEFAULT); // Clamped against the guard rail: BROWSER_DB_MAX_SCAN_ROWS is configurable, and without // this the very first chunk fetch can already overshoot it whenever an operator lowers the // scan limit below the caller's chunk size (e.g. below BROWSER_SINGLE_PASS_CHUNK_SIZE's - // 7,000 default) -- the dbOffset >= scanLimit check only runs after a chunk is fetched, so + // 7,000 default) -- the dbOffset >= scanRowLimit check only runs after a chunk is fetched, so // nothing upstream of it would have caught that (found in review, issue #37184). - final int effectiveChunkSize = Math.min(chunkSize, scanLimit); + // NOTE: on the ES-narrowed path (applyESFilter=true), BROWSER_DB_MAX_SCAN_ROWS no longer + // bounds the *total* rows the scan may read -- BROWSER_DB_MAX_SCAN_ROWS_ES_HARD_CAP does + // that instead (see below). This clamp still shapes the ES path's working chunk size for + // the same reason it does on the permission-only path (bounding the SQL page size), so it + // intentionally stays in effect for both; lowering BROWSER_DB_MAX_SCAN_ROWS therefore + // still shrinks ES chunk size (more round trips) without limiting the ES scan itself + // (found in review, issue #37211) -- surprising if read as "the" scan limit, so calling + // it out explicitly here. + final int effectiveChunkSize = Math.min(chunkSize, scanRowLimit); + // The ES-narrowed scan (text filter) bounds cost by elapsed time instead of row count -- + // see BROWSER_DB_MAX_SCAN_TIME_MILLIS_KEY. The permission-only scan keeps the original + // row-count cutoff; it has no completeness gap to fix (every candidate row already + // matches the query's own SQL criteria). + final long scanTimeBudgetMillis = applyESFilter + ? Config.getLongProperty(BROWSER_DB_MAX_SCAN_TIME_MILLIS_KEY, BROWSER_DB_MAX_SCAN_TIME_MILLIS_DEFAULT) + : -1L; + // Co-bound alongside the time budget: without a row ceiling, a fast DB/ES pair (or many + // concurrent unfiltered searches sharing the DotSubmitter pool) could scan far more rows + // in scanTimeBudgetMillis than the old row-count cutoff ever allowed, multiplying real + // cost under load (found in review, issue #37211). Set high enough to comfortably cover + // the real-world scale this fix targets (~718,174 contentlets, per the issue) so it does + // not reintroduce the original completeness bug at that scale. + final int esRowHardCap = applyESFilter + ? Config.getIntProperty(BROWSER_DB_MAX_SCAN_ROWS_ES_HARD_CAP_KEY, BROWSER_DB_MAX_SCAN_ROWS_ES_HARD_CAP_DEFAULT) + : -1; + final long scanStartNanos = System.nanoTime(); final List accumulatedContent = new ArrayList<>(); List candidateChunkInodes; @@ -310,9 +356,9 @@ private ContentUnderParent getContentByChunks(final BrowserQuery browserQuery, // A satisfied page wins over the guard rail: when this chunk already produced enough // visible items we must exit through generateNextContentCursor so the next page resumes - // right after the last item returned. Checking the scan limit first would exit via the + // right after the last item returned. Checking the scan budget first would exit via the // warn path with a chunk-aligned cursor and silently skip whatever is left over in this - // chunk -- reachable whenever a chunk boundary lands exactly on the scan limit. + // chunk -- reachable whenever a chunk boundary lands exactly on the scan budget. if (accumulatedContent.size() >= maxRows) { hasMore = (candidateChunkInodes.size() == effectiveChunkSize); nextContentCursor = generateNextContentCursor(accumulatedContent, maxRows, @@ -321,13 +367,12 @@ private ContentUnderParent getContentByChunks(final BrowserQuery browserQuery, } // Natural DB exhaustion also wins over the guard rail, for the same reason: the scan - // limit exists to cut off a search that is NOT done, not to relabel a search that + // budget exists to cut off a search that is NOT done, not to relabel a search that // finished on its own. A partial last chunk (fewer rows than chunkSize) means there is - // nothing left to scan, regardless of how far dbOffset has climbed -- checking the scan - // limit first would report hasMore=true for a folder that is actually fully paged - // through whenever the last (partial) chunk's ending offset happens to land on or past - // the scan limit, which is reachable whenever chunkSize and the scan limit are close in - // size (found in review, issue #37184). + // nothing left to scan, regardless of how far dbOffset/elapsed time has climbed -- + // checking the scan budget first would report hasMore=true for a folder that is + // actually fully paged through whenever the last (partial) chunk's ending point happens + // to land on or past the budget (found in review, issue #37184). if (candidateChunkInodes.size() < effectiveChunkSize) { Logger.debug(this, String.format( "Reached end of results (partial chunk) - DB is exhausted. Total accumulated: %d", @@ -336,10 +381,30 @@ private ContentUnderParent getContentByChunks(final BrowserQuery browserQuery, break; } - if (dbOffset >= scanLimit) { + // Row-count-only cutoff dropped ES-narrowed matches that sit past it in DB order + // without ever sending them to ES (#37211) -- an unfiltered global search's SQL + // candidate set is essentially the whole site, so DB position says nothing about + // whether a match exists. Bound that scan by elapsed time instead, so it keeps + // going while still affordable rather than giving up at an arbitrary row count -- + // plus a much higher row hard cap as a co-bound, so a fast DB/ES pair (or many + // concurrent unfiltered searches) cannot turn the time budget into unbounded + // per-request work (found in review). The permission-only scan keeps the original + // row-count cutoff -- it has no completeness gap to fix (every candidate row + // already matches the query's own SQL criteria). + final boolean esTimeBudgetExhausted = applyESFilter + && TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - scanStartNanos) >= scanTimeBudgetMillis; + final boolean esRowHardCapExceeded = applyESFilter && dbOffset >= esRowHardCap; + final boolean scanBudgetExhausted = applyESFilter + ? (esTimeBudgetExhausted || esRowHardCapExceeded) + : dbOffset >= scanRowLimit; + + if (scanBudgetExhausted) { + final String exhaustedBoundDescription = !applyESFilter + ? scanRowLimit + " rows" + : (esRowHardCapExceeded ? esRowHardCap + " rows (hard cap)" : scanTimeBudgetMillis + "ms"); Logger.warn(BrowserAPIImpl.class, String.format( - "Scan limit reached (%d rows) after %d chunks. Returning %d accumulated items.", - dbOffset, chunkCount, accumulatedContent.size())); + "Scan budget reached (%s) after %d chunks, %d rows scanned. Returning %d accumulated items.", + exhaustedBoundDescription, chunkCount, dbOffset, accumulatedContent.size())); nextContentCursor = dbOffset; hasMore = true; break; @@ -834,9 +899,34 @@ private String buildPureESQuery(final BrowserQuery browserQuery) { // Maximum total DB rows to scan per request across all chunks. Acts as a safety cap to prevent // runaway queries when a restricted user has access to a small fraction of site content. // Default of 50,000 covers a worst-case ~5% permission pass rate for a full page of 300 items. + // Used as-is (row-count cutoff) for the permission-only scan (applyESFilter=false); see + // BROWSER_DB_MAX_SCAN_TIME_MILLIS_KEY for the ES-narrowed (text-filter) scan's cost bound. static final String BROWSER_DB_MAX_SCAN_ROWS_KEY = "BROWSER_DB_MAX_SCAN_ROWS"; static final int BROWSER_DB_MAX_SCAN_ROWS_DEFAULT = 50_000; + // Maximum wall-clock time to spend scanning DB chunks when text-filtering through ES + // (applyESFilter=true). A row-count cutoff here silently drops matches that fall later in + // DB order than the cutoff, even though they were never actually sent to ES for narrowing + // (issue #37211) -- a global search with no content-type filter has a broad, effectively + // unbounded-by-type candidate set, so match position in DB order says nothing about whether + // the match exists. Bounding by elapsed time instead lets the scan keep going as long as it + // is still affordable, rather than giving up at an arbitrary row count regardless of + // coverage. + static final String BROWSER_DB_MAX_SCAN_TIME_MILLIS_KEY = "BROWSER_DB_MAX_SCAN_TIME_MILLIS"; + static final long BROWSER_DB_MAX_SCAN_TIME_MILLIS_DEFAULT = 10_000L; + + // Co-bound alongside BROWSER_DB_MAX_SCAN_TIME_MILLIS for the ES-narrowed scan + // (applyESFilter=true): a hard ceiling on total rows read, independent of elapsed time. The + // time budget alone does not cap *work* -- a fast DB/ES pair could scan far more rows in + // BROWSER_DB_MAX_SCAN_TIME_MILLIS than the old row-count cutoff ever allowed, which under + // concurrent unfiltered searches multiplies real cost (found in review, issue #37211). Set + // well above the real-world scale this fix targets (~718,174 contentlets, per the issue) so + // it does not reintroduce the original completeness bug at that scale; it exists to cap the + // pathological case (a much larger site, or many concurrent requests) that the time budget + // alone cannot. + static final String BROWSER_DB_MAX_SCAN_ROWS_ES_HARD_CAP_KEY = "BROWSER_DB_MAX_SCAN_ROWS_ES_HARD_CAP"; + static final int BROWSER_DB_MAX_SCAN_ROWS_ES_HARD_CAP_DEFAULT = 1_000_000; + // Default DB chunk size for the hybrid DB+ES text-filtering loop. static final String BROWSER_CONTENT_CHUNK_SIZE_KEY = "BROWSER_CONTENT_CHUNK_SIZE"; static final int BROWSER_CONTENT_CHUNK_SIZE_DEFAULT = 900; diff --git a/dotcms-integration/src/test/java/com/dotcms/browser/BrowserAPITest.java b/dotcms-integration/src/test/java/com/dotcms/browser/BrowserAPITest.java index 51d33d8f56f8..f3a83cfb52a0 100644 --- a/dotcms-integration/src/test/java/com/dotcms/browser/BrowserAPITest.java +++ b/dotcms-integration/src/test/java/com/dotcms/browser/BrowserAPITest.java @@ -2238,6 +2238,444 @@ public void test_getPaginatedContents_scanLimitStopsLoop() throws Exception { } } + /** + * + */ + @Test + public void test_getPaginatedContents_unfilteredTextSearch_findsMatchPastScanLimit() throws Exception { + final int chunkSize = 5; + final int scanLimit = 15; + final int fillerCount = 20; + + Config.setProperty(BrowserAPIImpl.BROWSER_DB_MAX_SCAN_ROWS_KEY, scanLimit); + Config.setProperty("BROWSER_CONTENT_CHUNK_SIZE", chunkSize); + try { + final Host host = new SiteDataGen().nextPersisted(); + final Folder folder = new FolderDataGen().site(host).nextPersisted(); + final var contentType = new ContentTypeDataGen() + .host(host) + .folder(folder) + .field(new FieldDataGen().name("title").velocityVarName("title").next()) + .nextPersisted(); + + // Filler items created first -- oldest mod_date, scanned first under the default + // ascending order, all safely inside the scan limit. + for (int i = 0; i < fillerCount; i++) { + new ContentletDataGen(contentType) + .setProperty("title", "Filler " + i) + .host(host) + .folder(folder) + .setPolicy(IndexPolicy.WAIT_FOR) + .nextPersisted(); + } + + // The matching item is created LAST -- newest mod_date, scanned last, past the + // lowered scan limit -- reproducing "a just-uploaded item is missing from search". + final Contentlet capybara = new ContentletDataGen(contentType) + .setProperty("title", "Capybara Image") + .host(host) + .folder(folder) + .setPolicy(IndexPolicy.WAIT_FOR) + .nextPersisted(); + + final BrowserQuery query = BrowserQuery.builder() + .withHostOrFolderId(folder.getIdentifier()) + .withFilter("Capybara") + .useElasticsearchFiltering(true) // Content Drive always sets this (ContentDriveHelper) -- + // the bug only reproduces on the ES-routed path, never + // the plain DB ILIKE fallback used when this is false. + .showContent(true) + .showFiles(false) + .showFolders(false) + .showLinks(false) + .showDotAssets(false) + .showWorking(true) + .showArchived(false) + .maxResults(100) + .contentCursor(0) + .build(); + + final PaginatedContents result = browserAPI.getPaginatedContents(query); + + assertNotNull("Result must not be null", result); + final Set foundInodes = result.list.stream() + .map(item -> (String) item.get("inode")) + .collect(Collectors.toSet()); + assertTrue("Unfiltered global search must find a match that exists past the scan " + + "limit (issue #37211) -- found: " + foundInodes, + foundInodes.contains(capybara.getInode())); + } finally { + Config.setProperty(BrowserAPIImpl.BROWSER_DB_MAX_SCAN_ROWS_KEY, + BrowserAPIImpl.BROWSER_DB_MAX_SCAN_ROWS_DEFAULT); + Config.setProperty("BROWSER_CONTENT_CHUNK_SIZE", 900); + } + } + + /** + *
    + *
  • Method to Test: {@link BrowserAPI#getPaginatedContents(BrowserQuery)}
    • + *
  • Given Scenario: Two content types each hold one item matching the same + * free-text filter. An unfiltered global search is compared against the same search + * narrowed to a single content type.
    • + *
  • Expected Result: The content-type-filtered result is a subset of the + * unfiltered result -- it drops the other type's match but introduces nothing the + * unfiltered search didn't already find (AC-002 of + * #37211).
    • + *
+ */ + @Test + public void test_getPaginatedContents_contentTypeFilter_onlyNarrowsUnfilteredMatches() throws Exception { + final Host host = new SiteDataGen().nextPersisted(); + final Folder folder = new FolderDataGen().site(host).nextPersisted(); + + final var typeA = new ContentTypeDataGen() + .host(host) + .folder(folder) + .field(new FieldDataGen().name("title").velocityVarName("title").next()) + .nextPersisted(); + final var typeB = new ContentTypeDataGen() + .host(host) + .folder(folder) + .field(new FieldDataGen().name("title").velocityVarName("title").next()) + .nextPersisted(); + + final Contentlet matchA = new ContentletDataGen(typeA) + .setProperty("title", "Capybara from Type A") + .host(host) + .folder(folder) + .setPolicy(IndexPolicy.WAIT_FOR) + .nextPersisted(); + final Contentlet matchB = new ContentletDataGen(typeB) + .setProperty("title", "Capybara from Type B") + .host(host) + .folder(folder) + .setPolicy(IndexPolicy.WAIT_FOR) + .nextPersisted(); + + final BrowserQuery unfilteredQuery = BrowserQuery.builder() + .withHostOrFolderId(folder.getIdentifier()) + .withFilter("Capybara") + .useElasticsearchFiltering(true) + .showContent(true) + .showFiles(false) + .showFolders(false) + .showLinks(false) + .showDotAssets(false) + .showWorking(true) + .showArchived(false) + .maxResults(100) + .contentCursor(0) + .build(); + + final PaginatedContents unfiltered = browserAPI.getPaginatedContents(unfilteredQuery); + final Set unfilteredInodes = unfiltered.list.stream() + .map(item -> (String) item.get("inode")) + .collect(Collectors.toSet()); + + assertTrue("Unfiltered search must find the Type A match", unfilteredInodes.contains(matchA.getInode())); + assertTrue("Unfiltered search must find the Type B match", unfilteredInodes.contains(matchB.getInode())); + + final BrowserQuery typeAFilteredQuery = BrowserQuery.builder() + .withHostOrFolderId(folder.getIdentifier()) + .withFilter("Capybara") + .withContentTypes(Set.of(typeA.id())) + .useElasticsearchFiltering(true) + .showContent(true) + .showFiles(false) + .showFolders(false) + .showLinks(false) + .showDotAssets(false) + .showWorking(true) + .showArchived(false) + .maxResults(100) + .contentCursor(0) + .build(); + + final PaginatedContents typeAFiltered = browserAPI.getPaginatedContents(typeAFilteredQuery); + final Set filteredInodes = typeAFiltered.list.stream() + .map(item -> (String) item.get("inode")) + .collect(Collectors.toSet()); + + assertTrue("Type-A-filtered search must still find the Type A match", + filteredInodes.contains(matchA.getInode())); + assertFalse("Type-A-filtered search must drop the Type B match", + filteredInodes.contains(matchB.getInode())); + assertTrue("Filtered result must be a subset of the unfiltered result -- it must not " + + "introduce items the unfiltered search didn't find", + unfilteredInodes.containsAll(filteredInodes)); + } + + /** + *
    + *
  • Method to Test: {@link BrowserAPI#getPaginatedContents(BrowserQuery)}
    • + *
  • Given Scenario: A folder holds more content than the (lowered) scan limit, + * none of it matching the free-text filter at all.
    • + *
  • Expected Result: The search still completes within a bounded time and + * returns no matches -- fixing the silent-drop defect + * (#37211) must not regress + * into an unconditional full-table scan (see PR #37395 and siblings for the original + * scan-cost concern the row cap was introduced to address).
    • + *
+ */ + @Test + public void test_getPaginatedContents_unfilteredTextSearch_noMatchStaysBounded() throws Exception { + final int chunkSize = 5; + final int scanLimit = 15; + final int fillerCount = 20; + final long boundedMillis = 30_000L; + + Config.setProperty(BrowserAPIImpl.BROWSER_DB_MAX_SCAN_ROWS_KEY, scanLimit); + Config.setProperty("BROWSER_CONTENT_CHUNK_SIZE", chunkSize); + try { + final Host host = new SiteDataGen().nextPersisted(); + final Folder folder = new FolderDataGen().site(host).nextPersisted(); + final var contentType = new ContentTypeDataGen() + .host(host) + .folder(folder) + .field(new FieldDataGen().name("title").velocityVarName("title").next()) + .nextPersisted(); + + for (int i = 0; i < fillerCount; i++) { + new ContentletDataGen(contentType) + .setProperty("title", "Filler " + i) + .host(host) + .folder(folder) + .setPolicy(IndexPolicy.WAIT_FOR) + .nextPersisted(); + } + + final BrowserQuery query = BrowserQuery.builder() + .withHostOrFolderId(folder.getIdentifier()) + .withFilter("NoSuchTermAnywhere") + .useElasticsearchFiltering(true) + .showContent(true) + .showFiles(false) + .showFolders(false) + .showLinks(false) + .showDotAssets(false) + .showWorking(true) + .showArchived(false) + .maxResults(100) + .contentCursor(0) + .build(); + + final long start = System.currentTimeMillis(); + final PaginatedContents result = browserAPI.getPaginatedContents(query); + final long elapsed = System.currentTimeMillis() - start; + + assertNotNull("Result must not be null", result); + assertEquals("No item matches the filter term", 0, result.contentCount); + assertTrue("A non-matching scan must still complete within a bounded time (" + + elapsed + "ms) -- fixing #37211 must not reintroduce an " + + "unconditional full-table scan", + elapsed < boundedMillis); + } finally { + Config.setProperty(BrowserAPIImpl.BROWSER_DB_MAX_SCAN_ROWS_KEY, + BrowserAPIImpl.BROWSER_DB_MAX_SCAN_ROWS_DEFAULT); + Config.setProperty("BROWSER_CONTENT_CHUNK_SIZE", 900); + } + } + + /** + *
    + *
  • Method to Test: {@link BrowserAPI#getPaginatedContents(BrowserQuery)}
    • + *
  • Given Scenario: {@code BROWSER_DB_MAX_SCAN_TIME_MILLIS} is set to an + * effectively-zero budget (1 ms) so the ES-narrowed scan's time cutoff -- not + * DB exhaustion, not the row-count guard rail -- fires after the very first chunk, with + * a matching item still unscanned several chunks later. This exercises the mechanism + * itself (found in review: the earlier scan-limit tests only ever exhaust the DB + * naturally, so none of them actually drive {@code scanBudgetExhausted} to + * {@code true} via elapsed time).
    • + *
  • Expected Result: Page 1 stops after one chunk with {@code hasMoreContent} + * true and a partial-progress cursor; the match is not yet in that page. Resuming from + * that cursor with a normal time budget reaches the match -- proving the time-cutoff + * path produces a valid, resumable cursor rather than silently losing coverage.
    • + *
+ */ + @Test + public void test_getPaginatedContents_timeBudgetExhausted_resumesFromCursor() throws Exception { + final int chunkSize = 5; + final int fillerCount = 15; + final long tinyTimeBudgetMillis = 1L; + + Config.setProperty("BROWSER_CONTENT_CHUNK_SIZE", chunkSize); + Config.setProperty(BrowserAPIImpl.BROWSER_DB_MAX_SCAN_TIME_MILLIS_KEY, tinyTimeBudgetMillis); + try { + final Host host = new SiteDataGen().nextPersisted(); + final Folder folder = new FolderDataGen().site(host).nextPersisted(); + final var contentType = new ContentTypeDataGen() + .host(host) + .folder(folder) + .field(new FieldDataGen().name("title").velocityVarName("title").next()) + .nextPersisted(); + + for (int i = 0; i < fillerCount; i++) { + new ContentletDataGen(contentType) + .setProperty("title", "Filler " + i) + .host(host) + .folder(folder) + .setPolicy(IndexPolicy.WAIT_FOR) + .nextPersisted(); + } + final Contentlet match = new ContentletDataGen(contentType) + .setProperty("title", "Wombat Image") + .host(host) + .folder(folder) + .setPolicy(IndexPolicy.WAIT_FOR) + .nextPersisted(); + + final BrowserQuery firstQuery = BrowserQuery.builder() + .withHostOrFolderId(folder.getIdentifier()) + .withFilter("Wombat") + .useElasticsearchFiltering(true) + .showContent(true) + .showFiles(false) + .showFolders(false) + .showLinks(false) + .showDotAssets(false) + .showWorking(true) + .showArchived(false) + .maxResults(100) + .contentCursor(0) + .build(); + + final PaginatedContents firstPage = browserAPI.getPaginatedContents(firstQuery); + + assertNotNull("First page must not be null", firstPage); + assertTrue("The 1ms time budget must cut the scan short before the match's chunk " + + "is reached", firstPage.hasMoreContent); + assertTrue("nextContentCursor must reflect partial progress (> 0 and <= filler count, " + + "not the full dataset) -- was: " + firstPage.nextContentCursor, + firstPage.nextContentCursor > 0 && firstPage.nextContentCursor <= fillerCount); + final Set firstPageInodes = firstPage.list.stream() + .map(item -> (String) item.get("inode")) + .collect(Collectors.toSet()); + assertFalse("The match must not appear in the time-cutoff page", + firstPageInodes.contains(match.getInode())); + + // Resume from the returned cursor with a normal time budget so the scan can finish. + Config.setProperty(BrowserAPIImpl.BROWSER_DB_MAX_SCAN_TIME_MILLIS_KEY, + BrowserAPIImpl.BROWSER_DB_MAX_SCAN_TIME_MILLIS_DEFAULT); + final BrowserQuery secondQuery = BrowserQuery.builder() + .withHostOrFolderId(folder.getIdentifier()) + .withFilter("Wombat") + .useElasticsearchFiltering(true) + .showContent(true) + .showFiles(false) + .showFolders(false) + .showLinks(false) + .showDotAssets(false) + .showWorking(true) + .showArchived(false) + .maxResults(100) + .contentCursor(firstPage.nextContentCursor) + .build(); + + final PaginatedContents secondPage = browserAPI.getPaginatedContents(secondQuery); + final Set secondPageInodes = secondPage.list.stream() + .map(item -> (String) item.get("inode")) + .collect(Collectors.toSet()); + + assertTrue("Resuming from the time-cutoff cursor must reach the match in a later " + + "chunk", secondPageInodes.contains(match.getInode())); + } finally { + Config.setProperty(BrowserAPIImpl.BROWSER_DB_MAX_SCAN_TIME_MILLIS_KEY, + BrowserAPIImpl.BROWSER_DB_MAX_SCAN_TIME_MILLIS_DEFAULT); + Config.setProperty("BROWSER_CONTENT_CHUNK_SIZE", 900); + } + } + + /** + *
    + *
  • Method to Test: {@link BrowserAPI#getPaginatedContents(BrowserQuery)}
    • + *
  • Given Scenario: Every item in the fixture matches the free-text filter, and + * {@code BROWSER_DB_MAX_SCAN_ROWS_ES_HARD_CAP} is lowered well below the fixture's row + * count while {@code BROWSER_DB_MAX_SCAN_TIME_MILLIS} is left generous, so only the row + * hard cap -- not the time budget, not running out of matches -- can terminate the + * ES-narrowed scan (found in review: the co-bound added alongside the time budget had no + * test of its own).
    • + *
  • Expected Result: The scan stops at the hard cap with {@code hasMoreContent} + * true, proving the ES path is not left with an effectively unbounded row ceiling once + * the row-count-based cutoff was replaced by a time budget.
    • + *
+ */ + @Test + public void test_getPaginatedContents_esRowHardCap_stopsRunawayScanUnderGenerousTimeBudget() + throws Exception { + final int chunkSize = 5; + final int hardCap = 15; + final int fillerCount = 20; + + Config.setProperty("BROWSER_CONTENT_CHUNK_SIZE", chunkSize); + Config.setProperty(BrowserAPIImpl.BROWSER_DB_MAX_SCAN_ROWS_ES_HARD_CAP_KEY, hardCap); + Config.setProperty(BrowserAPIImpl.BROWSER_DB_MAX_SCAN_TIME_MILLIS_KEY, + BrowserAPIImpl.BROWSER_DB_MAX_SCAN_TIME_MILLIS_DEFAULT); + try { + final Host host = new SiteDataGen().nextPersisted(); + final Folder folder = new FolderDataGen().site(host).nextPersisted(); + final var contentType = new ContentTypeDataGen() + .host(host) + .folder(folder) + .field(new FieldDataGen().name("title").velocityVarName("title").next()) + .nextPersisted(); + + for (int i = 0; i < fillerCount; i++) { + new ContentletDataGen(contentType) + .setProperty("title", "AllMatchTerm " + i) + .host(host) + .folder(folder) + .setPolicy(IndexPolicy.WAIT_FOR) + .nextPersisted(); + } + + final BrowserQuery query = BrowserQuery.builder() + .withHostOrFolderId(folder.getIdentifier()) + .withFilter("AllMatchTerm") + .withContentTypes(Set.of(contentType.id())) + .useElasticsearchFiltering(true) + .showContent(true) + .showFiles(false) + .showFolders(false) + .showLinks(false) + .showDotAssets(false) + .showWorking(true) + .showArchived(false) + .maxResults(100) + .contentCursor(0) + .build(); + + final PaginatedContents result = browserAPI.getPaginatedContents(query); + + assertNotNull("Result must not be null", result); + assertTrue("The row hard cap must stop the scan before it reaches the end of the " + + "fixture (20 items) -- hasMoreContent should be true", + result.hasMoreContent); + assertTrue("nextContentCursor must reflect the hard cap having fired (>= hard cap, " + + "< full fixture size) -- was: " + result.nextContentCursor, + result.nextContentCursor >= hardCap && result.nextContentCursor < fillerCount); + } finally { + Config.setProperty(BrowserAPIImpl.BROWSER_DB_MAX_SCAN_ROWS_ES_HARD_CAP_KEY, + BrowserAPIImpl.BROWSER_DB_MAX_SCAN_ROWS_ES_HARD_CAP_DEFAULT); + Config.setProperty(BrowserAPIImpl.BROWSER_DB_MAX_SCAN_TIME_MILLIS_KEY, + BrowserAPIImpl.BROWSER_DB_MAX_SCAN_TIME_MILLIS_DEFAULT); + Config.setProperty("BROWSER_CONTENT_CHUNK_SIZE", 900); + } + } + /** *
    *
  • Method to Test: {@link BrowserAPI#getPaginatedContents(BrowserQuery)}
    • diff --git a/specs/37418-content-drive-global/spec.md b/specs/37418-content-drive-global/spec.md index 1aa749f40158..a3852490ed68 100644 --- a/specs/37418-content-drive-global/spec.md +++ b/specs/37418-content-drive-global/spec.md @@ -127,13 +127,16 @@ Two adjacent theories were checked and **refuted**, ruling out alternative root ever reaches ES. The legacy Search All portlet does not exhibit this bug because it does not go through this -DB-prescan/chunk mechanism — it queries ES directly. (Architecturally confirmed; the exact -backing class/line was not pinned down in this pass — candidates are -`ESSearchAPIImpl`/`ESContentResourcePortlet` under `dotCMS/src/main/java/com/dotcms/**` — -and should be confirmed during planning.) - -[NEEDS CLARIFICATION: Confirm the exact Search All REST resource/service class and line -reference, to formally document the working comparison path in the plan.] +DB-prescan/chunk mechanism — it queries ES directly. **Confirmed during planning**: the search +handler is `com.dotcms.rest.elasticsearch.ESContentResourcePortlet` +(`dotCMS/src/main/java/com/dotcms/rest/elasticsearch/ESContentResourcePortlet.java`) — +`search`/`searchPost` build a raw ES/OS Lucene query and execute it via +`ContentletAPI.search(...)` (`esapi.search(...)`, lines 133-135 and its `searchPost` +counterpart), with `esapi = APILocator.getContentletAPI()` (line 60); `searchRaw` +(line 293) does the same via `esapi.searchRaw(...)`. None of these paths build DB-order +paged candidate chunks first — the query goes straight to ES/OS, which matches and ranks +over the whole index in one shot, so a match is never dropped by a DB-page cutoff before ES +gets to see it. ## Fix Scope & Non-Goals *(mandatory)*