Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 17 additions & 2 deletions backend/app/controllers/api/PagesController.scala
Original file line number Diff line number Diff line change
Expand Up @@ -19,17 +19,32 @@ import utils.controller.{AuthApiController, AuthControllerComponents}
class PagesController(val controllerComponents: AuthControllerComponents, manifest: Manifest,
index: Index, pagesService: Pages2, annotations: Annotations, previewStorage: ObjectStorage) extends AuthApiController {

def getPageCount(uri: Uri) = ApiAction.attempt { req =>
def getPageCount(uri: Uri, q: Option[String]) = ApiAction.attempt { req =>
val countAttempt = pagesService.getPageCount(uri)
val dimensionsAttempt = pagesService.getFirstPageDimensions(uri)
.recoverWith { case _ => Attempt.Right(None) }
// Answers "would the Combined view show a match for this search?" alongside
// the count, so the frontend can choose the initial view for a document
// opened from search results in a single round trip (issues #733 / #760).
// A failure here (e.g. a query elasticsearch can't parse) must not stop the
// document opening: it reports as "no match", which keeps the search-forced
// flat view.
val searchMatchAttempt = Attempt.sequenceOption(
q.filter(_.trim.nonEmpty).map { query =>
Attempt.catchNonFatalBlasé(Chips.parseQueryString(query).query)
.flatMap(parsed => pagesService.hasSearchMatch(uri, parsed))
.recoverWith { case _ => Attempt.Right(false) }
}
)
for {
count <- countAttempt
dimensions <- dimensionsAttempt
searchMatchesInPages <- searchMatchAttempt
} yield {
Ok(Json.obj(
"pageCount" -> count,
"dimensions" -> dimensions
"dimensions" -> dimensions,
"searchMatchesInPages" -> searchMatchesInPages
))
}
}
Expand Down
17 changes: 17 additions & 0 deletions backend/app/services/index/Pages2.scala
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,23 @@ class Pages2(val client: ElasticClient, indexNamePrefix: String)(implicit val ex
}
}

// Cheap existence check: does the query match anywhere in this document's
// pages? A single count query — unlike findInPages + the geometry pipeline
// behind findInDocument/searchInDocument, this never touches S3 or computes
// highlight geometry. Used to decide the initial view for a document opened
// from search results (issues #733 / #760).
def hasSearchMatch(uri: Uri, query: String): Attempt[Boolean] = {
execute {
count(textIndexName).query(
must(buildQuery(query)).filter(
termQuery(PagesFields.resourceId, uri.value)
)
)
}.map { resp =>
resp.count > 0
}
}

private def buildQuery(query: String) =
queryStringQuery(query)
.defaultOperator("and")
Expand Down
2 changes: 1 addition & 1 deletion backend/conf/routes
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ DELETE /api/comments/:commentId cont
GET /api/pages/text/:uri controllers.api.Resource.getTextPages(uri: model.Uri, top: Double, bottom: Double, q: Option[String], language: Option[model.Language])
GET /api/pages/preview/:language/:uri/:pageNumber controllers.api.Resource.getPagePreview(uri: model.Uri, language: model.Language, pageNumber: Int)

GET /api/pages2/:uri/pageCount controllers.api.PagesController.getPageCount(uri: model.Uri)
GET /api/pages2/:uri/pageCount controllers.api.PagesController.getPageCount(uri: model.Uri, q: Option[String])
GET /api/pages2/:uri/find controllers.api.PagesController.findInDocument(uri: model.Uri, q: String)
GET /api/pages2/:uri/search controllers.api.PagesController.searchInDocument(uri: model.Uri, q: String)
GET /api/pages2/:uri/:pageNumber/preview controllers.api.PagesController.getPagePreview(uri: model.Uri, pageNumber: Int)
Expand Down
38 changes: 38 additions & 0 deletions backend/test/services/index/ElasticsearchPagesITest.scala
Original file line number Diff line number Diff line change
Expand Up @@ -126,5 +126,43 @@ class ElasticsearchPagesITest extends AnyFreeSpec with Matchers with BeforeAndAf
result shouldBe None
}
}

"hasSearchMatch" - {
val uri = Uri("has-search-match-test")
val page = Page(page = 1, Map(English -> "brown fox jumped near hedge"), PageDimensions.A4_PORTRAIT)

"return true when the query matches the document's pages" in {
elasticsearchTestService.elasticPages.addPageContents(uri, Seq(page)).successValue

pages2.hasSearchMatch(uri, "fox").successValue shouldBe true
}

"return false when the query does not match" in {
elasticsearchTestService.elasticPages.addPageContents(uri, Seq(page)).successValue

pages2.hasSearchMatch(uri, "badger").successValue shouldBe false
}

"respect quoted phrases" in {
elasticsearchTestService.elasticPages.addPageContents(uri, Seq(page)).successValue

pages2.hasSearchMatch(uri, "\"brown fox\"").successValue shouldBe true
pages2.hasSearchMatch(uri, "\"fox brown\"").successValue shouldBe false
}

"not match content from other documents" in {
val otherUri = Uri("has-search-match-other-doc")
val otherPage = Page(page = 1, Map(English -> "badger slept here"), PageDimensions.A4_PORTRAIT)
elasticsearchTestService.elasticPages.addPageContents(uri, Seq(page)).successValue
elasticsearchTestService.elasticPages.addPageContents(otherUri, Seq(otherPage)).successValue

pages2.hasSearchMatch(uri, "badger").successValue shouldBe false
pages2.hasSearchMatch(otherUri, "badger").successValue shouldBe true
}

"return false for a document with no pages" in {
pages2.hasSearchMatch(Uri("has-search-match-no-pages"), "fox").successValue shouldBe false
}
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,116 @@
import {
COMBINED_VIEW,
isTextLikeView,
resolveInitialPagedView,
} from "./resolveInitialPagedView";

describe("isTextLikeView", () => {
it("treats text and any ocr.* variant as text-like", () => {
expect(isTextLikeView("text")).toBe(true);
expect(isTextLikeView("ocr")).toBe(true);
expect(isTextLikeView("ocr.english")).toBe(true);
});

it("does not treat combined, transcript or other views as text-like", () => {
expect(isTextLikeView(COMBINED_VIEW)).toBe(false);
expect(isTextLikeView("transcript.english")).toBe(false);
expect(isTextLikeView("vttTranscript.english")).toBe(false);
expect(isTextLikeView("preview")).toBe(false);
expect(isTextLikeView("table")).toBe(false);
});
});

describe("resolveInitialPagedView", () => {
const base = {
pageCount: 10,
currentView: undefined as string | undefined,
hasSearchQuery: false,
searchMatchesInPages: null as boolean | null | undefined,
};

it("does nothing for non-paged documents (no Combined view exists)", () => {
expect(
resolveInitialPagedView({
...base,
pageCount: 0,
currentView: "text",
hasSearchQuery: true,
searchMatchesInPages: true,
}),
).toBeUndefined();
});

it("defaults a paged document with no view to Combined", () => {
expect(resolveInitialPagedView(base)).toBe(COMBINED_VIEW);
});

describe("when search has forced a text-like view", () => {
const forced = {
...base,
currentView: "text",
hasSearchQuery: true,
};

it("switches to Combined when the match is reachable in the pages", () => {
expect(
resolveInitialPagedView({ ...forced, searchMatchesInPages: true }),
).toBe(COMBINED_VIEW);
});

it("honours the forced view when the match is not in the pages", () => {
// e.g. lossy Tesseract OCR, cross-field, or a future translation match
expect(
resolveInitialPagedView({ ...forced, searchMatchesInPages: false }),
).toBeUndefined();
});

it("honours the forced view when the answer is unknown", () => {
// e.g. a backend that predates the searchMatchesInPages parameter
expect(
resolveInitialPagedView({ ...forced, searchMatchesInPages: null }),
).toBeUndefined();
expect(
resolveInitialPagedView({ ...forced, searchMatchesInPages: undefined }),
).toBeUndefined();
});

it("applies the same logic to an ocr.* view", () => {
expect(
resolveInitialPagedView({
...forced,
currentView: "ocr.english",
searchMatchesInPages: true,
}),
).toBe(COMBINED_VIEW);
});
});

it("honours a text view opened without a search query", () => {
expect(
resolveInitialPagedView({
...base,
currentView: "text",
hasSearchQuery: false,
// no query is sent with the pageCount request, so this stays null
searchMatchesInPages: null,
}),
).toBeUndefined();
});

it("honours a non-text view such as transcript even when the pages match", () => {
expect(
resolveInitialPagedView({
...base,
currentView: "transcript.english",
hasSearchQuery: true,
searchMatchesInPages: true,
}),
).toBeUndefined();
});

it("honours an explicit combined view", () => {
expect(
resolveInitialPagedView({ ...base, currentView: COMBINED_VIEW }),
).toBeUndefined();
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
export const COMBINED_VIEW = "combined";

/**
* Views whose content is derived from the page text that backs the Combined
* view, and which Elasticsearch therefore highlights against fields the page
* index also contains (`text`, `ocr`). A match in one of these is normally also
* reachable in Combined.
*
* Transcript / vttTranscript views are deliberately excluded: that content does
* not live in the page index, and audio/video documents have no pages anyway.
* The same will apply to any future translation view.
*/
export function isTextLikeView(view: string): boolean {
return view === "text" || view.startsWith("ocr");
}

/**
* Decide which view a paged document should open in, given its pageCount
* response (which also answers whether the search query matches in the page
* index — see getPageCount).
*
* Background (issues #733 / #760): Elasticsearch has no concept of the
* "Combined" view — it is assembled in the frontend from the page index. So
* search picks the document field with the most highlights (e.g. `text`,
* `ocr.english`) and forces that view via `?view=…`, which then suppresses the
* Combined default and lands the user in flat text/OCR even when Combined would
* show the same match in the view we actually want people to use.
*
* This resolves that: prefer Combined whenever the search query is also
* reachable there (`searchMatchesInPages`), and only fall back to the
* search-forced flat view when it is not — e.g. lossy Tesseract OCR,
* cross-field matches, or future translation matches that don't exist in the
* page index. A document opened without a search query, or with an explicit
* non-text view, is left untouched.
*
* Returns the view to switch to, or undefined to leave the current view alone.
* The caller runs this once per pageCount response, so a user manually
* switching tabs afterwards is never overridden.
*/
export function resolveInitialPagedView({
pageCount,
currentView,
hasSearchQuery,
searchMatchesInPages,
}: {
pageCount: number;
currentView: string | undefined;
hasSearchQuery: boolean;
// true/false from the backend when a query was sent; null or undefined when
// no query was sent, or from a backend that predates the parameter.
searchMatchesInPages: boolean | null | undefined;
}): string | undefined {
// No pages means there is no Combined view to prefer.
if (pageCount <= 0) {
return undefined;
}

// Paged document with no view yet → default to Combined (existing behaviour,
// e.g. opening from an ingestion or workspace).
if (!currentView) {
return COMBINED_VIEW;
}

// Search forced a text-like view: prefer Combined when the match is known to
// be reachable there; otherwise honour the forced view so the user still
// lands on a real match.
if (
hasSearchQuery &&
isTextLikeView(currentView) &&
searchMatchesInPages === true
) {
return COMBINED_VIEW;
}

// Honour any other explicit view (Combined, preview, table, transcript, or a
// text-like view without a search query or without a page match).
return undefined;
}
Original file line number Diff line number Diff line change
Expand Up @@ -48,13 +48,19 @@ export function useHighlightNavigation(

setQuery(q);
setIsPending(true);
// TODO: handle error
return authFetch(`/api/pages2/${uri}/${endpoint}?${params.toString()}`)
.then((res) => res.json())
.then((results: HighlightForSearchNavigation[]) => {
setIsPending(false);
setHighlights(results);
setFocusedIndex(results.length > 0 ? 0 : null);
})
.catch(() => {
// e.g. the document has no page index; treat as no highlights rather
// than letting the rejection surface as an uncaught runtime error.
setIsPending(false);
setHighlights([]);
setFocusedIndex(null);
});
},
[uri, endpoint],
Expand Down
Loading
Loading