diff --git a/Source/ApplicationServices/DbContexts.cs b/Source/ApplicationServices/DbContexts.cs index 09246f75..1b21b960 100644 --- a/Source/ApplicationServices/DbContexts.cs +++ b/Source/ApplicationServices/DbContexts.cs @@ -132,6 +132,13 @@ public static class DbContexts return context.GetDeletedLibraryBooks(); } + /// How many books belong in the search index. A row count only; no entities are loaded. + public static int GetIndexableBookCount() + { + using var context = GetContext(); + return context.GetIndexableBookCount(); + } + /// /// How many rows are in the trash. A row count only; no entities are loaded, /// so this is cheap enough to refresh whenever the library changes. diff --git a/Source/ApplicationServices/SearchEngineCommands.cs b/Source/ApplicationServices/SearchEngineCommands.cs index 00d1089b..26a436da 100644 --- a/Source/ApplicationServices/SearchEngineCommands.cs +++ b/Source/ApplicationServices/SearchEngineCommands.cs @@ -23,6 +23,7 @@ public static class SearchEngineCommands lock (IndexLock) { var engine = new SearchEngine(); + repairShortIndex(engine); try { return func(engine); @@ -40,6 +41,59 @@ public static class SearchEngineCommands } } } + + /// Set once the index has been measured against the library, so the check costs one query per run. + private static bool indexCounted; + + /// + /// Rebuilds an index that holds fewer books than the library does. + /// + /// A book the index never received is not merely unsearchable. Filtering intersects the grid with the + /// query's hits, so a positive term such as Absent cannot return it, while the negated -Absent + /// resolves to "every document in the index" and therefore drops it from the grid - which looks exactly + /// like the negated filter working correctly. Reported as issue #1989. + /// + /// + /// Nothing else notices: the index is only ever written as a whole, and + /// deliberately swallows a rebuild that fails so a bad index cannot fail a good scan. A short index + /// therefore stays short until something happens to change the library again. + /// + /// + private static void repairShortIndex(SearchEngine engine) + { + if (indexCounted) + return; + + // before the work, not after: a check that throws must not run on every query for the rest of the session + indexCounted = true; + + try + { + var indexed = engine.GetIndexedBookCount(); + + // no index yet, or one too damaged to read. Both already have their own recovery. + if (indexed < 0) + return; + + var expected = DbContexts.GetIndexableBookCount(); + + // more documents than books is stale rather than harmful: an extra document matches no grid row. + if (indexed >= expected) + return; + + Log.Warning("The search index holds {Indexed} of {Expected} books. Rebuilding it: search cannot find the rest, and a negated filter hides them.", indexed, expected); + + var failures = fullReIndex(engine); + + if (failures > 0) + Log.Error("{Failures} book(s) could not be added to the search index. Search cannot find them, and a negated filter hides them.", failures); + } + catch (Exception ex) + { + // Searching with a short index still beats not searching at all. + Log.Error(ex, "Could not check the search index against the library."); + } + } #endregion public static event EventHandler? SearchEngineUpdated; @@ -92,7 +146,7 @@ public static class SearchEngineCommands } } - public static void FullReIndex() => performSafeCommand(fullReIndex); + public static void FullReIndex() => performSafeCommand(e => fullReIndex(e)); public static void FullReIndex(List libraryBooks) => performSafeCommand(se => fullReIndex(se, libraryBooks.WithoutParents())); @@ -147,13 +201,15 @@ public static class SearchEngineCommands } } - private static void fullReIndex(SearchEngine engine) + /// How many books could not be indexed. + private static int fullReIndex(SearchEngine engine) { var library = DbContexts.GetLibrary_Flat_NoTracking(); - fullReIndex(engine, library); + return fullReIndex(engine, library); } - private static void fullReIndex(SearchEngine engine, IEnumerable libraryBooks) + /// How many books could not be indexed. + private static int fullReIndex(SearchEngine engine, IEnumerable libraryBooks) => engine.CreateNewIndex(libraryBooks); #endregion } diff --git a/Source/DataLayer/QueryObjects/LibraryBookQueries.cs b/Source/DataLayer/QueryObjects/LibraryBookQueries.cs index 185d4077..7d90fff0 100644 --- a/Source/DataLayer/QueryObjects/LibraryBookQueries.cs +++ b/Source/DataLayer/QueryObjects/LibraryBookQueries.cs @@ -69,6 +69,17 @@ public static class LibraryBookQueries .getLibrary() .ToList(); + /// + /// How many books a full search re-index writes: the same rows as + /// without the podcast parents, which are stripped before + /// indexing. A row count only; no entities are loaded, so this is cheap enough to check the index against. + /// + public int GetIndexableBookCount() + => context + .LibraryBooks + .AsNoTracking() + .Count(lb => !lb.IsDeleted && lb.Book.ContentType != ContentType.Parent); + /// Counts rows by (no related entities loaded). public (int NotInTrash, int InTrash) GetLibraryBookCountsByTrashFlag() { diff --git a/Source/LibationSearchEngine/SearchEngine.cs b/Source/LibationSearchEngine/SearchEngine.cs index 41bf10c2..beae8b66 100644 --- a/Source/LibationSearchEngine/SearchEngine.cs +++ b/Source/LibationSearchEngine/SearchEngine.cs @@ -88,7 +88,14 @@ public class SearchEngine #region create and update index /// create new. ie: full re-index - public void CreateNewIndex(IEnumerable library, bool overwrite = true) + /// How many books could not be indexed, and so cannot be found by search or filter. + /// + /// One book is not allowed to cost the rest their place in the index. Disposing the writer commits + /// whatever it already holds, so letting a single bad book escape the loop used to leave a silently + /// truncated index behind: every book after the failure was missing from it, and a missing book is + /// worse than an unsearchable one, because a negated filter drops it from the grid entirely. + /// + public int CreateNewIndex(IEnumerable library, bool overwrite = true) { var libraryList = library.ToList(); @@ -99,10 +106,58 @@ public class SearchEngine using var analyzer = new StandardAnalyzer(Version); using var ixWriter = openWriterForRebuild(index, analyzer, overwrite); + var failures = 0; + foreach (var libraryBook in libraryList) { - var doc = createBookIndexDocument(libraryBook); - ixWriter.AddDocument(doc); + try + { + var doc = createBookIndexDocument(libraryBook); + ixWriter.AddDocument(doc); + } + catch (Exception ex) + { + failures++; + Serilog.Log.Logger.Error(ex, "Could not add {Book} to the search index. It will not be found by search or filter, and a negated filter will hide it.", describeForLog(libraryBook)); + } + } + + return failures; + } + + /// Name a book for a log line without risking a second exception from the book that just threw. + private static string describeForLog(LibraryBook libraryBook) + { + try + { + return libraryBook?.Book?.AudibleProductId ?? "a book with no product id"; + } + catch + { + return "an unreadable book"; + } + } + + /// + /// How many books the index holds, or -1 when there is no index to read. Every write to the index is a + /// full rebuild, so this should always equal the number of books a rebuild would write; when it is short, + /// the missing books are invisible to search and are hidden from the grid by any negated filter. + /// + public int GetIndexedBookCount() + { + try + { + using var index = getIndex(); + if (!IndexReader.IndexExists(index)) + return -1; + + using var reader = IndexReader.Open(index, readOnly: true); + return reader.NumDocs(); + } + catch (Exception ex) + { + Serilog.Log.Logger.Debug(ex, "Could not count the documents in the search index"); + return -1; } } diff --git a/Source/_Tests/LibationSearchEngine.Tests/IndexCompletenessTests.cs b/Source/_Tests/LibationSearchEngine.Tests/IndexCompletenessTests.cs new file mode 100644 index 00000000..159cf744 --- /dev/null +++ b/Source/_Tests/LibationSearchEngine.Tests/IndexCompletenessTests.cs @@ -0,0 +1,121 @@ +using System; +using System.Collections.Generic; +using System.IO; +using System.Linq; +using AssertionHelper; +using DataLayer; +using LibationSearchEngine; +using Microsoft.VisualStudio.TestTools.UnitTesting; +using Directory = System.IO.Directory; + +namespace SearchEngineTests; + +/// +/// A book the index does not hold is worse off than a book the index gets wrong. Filtering keeps the grid rows +/// whose product id the query returned, so a missing book cannot be found by any positive term, and every +/// negated term - which resolves to "every document in the index" - drops it from the grid instead. Reported as +/// issue #1989, where Absent found nothing while -Absent appeared to work perfectly. +/// +[TestClass] +public class IndexCompletenessTests +{ + private const string PRESENT = "B0PRESENT01"; + private const string ABSENT = "B0ABSENT001"; + private const string THIRD = "B0THIRD0001"; + + private string indexDirectory = null!; + + [TestInitialize] + public void Initialize() + { + indexDirectory = Path.Combine(Path.GetTempPath(), "LibationSearchEngineTests", Guid.NewGuid().ToString("N")); + Directory.CreateDirectory(indexDirectory); + } + + [TestCleanup] + public void Cleanup() + { + try + { + if (Directory.Exists(indexDirectory)) + Directory.Delete(indexDirectory, recursive: true); + } + catch (IOException) + { + // Windows refuses to delete a file Lucene still holds open, and a leftover temp directory is + // not worth failing a test over + } + } + + private static LibraryBook book(string asin, bool absentFromLastScan = false) + { + var contributor = Contributor.GetEmpty(); + var b = new Book(new AudibleProductId(asin), $"Title {asin}", null, null, 1, ContentType.Product, [contributor], [contributor], "us"); + return new LibraryBook(b, new DateTime(2026, 8, 15), "account") { AbsentFromLastScan = absentFromLastScan }; + } + + /// + /// EF materializes entities through the private parameterless constructor, so a + /// row whose BookId matches nothing in the Books table arrives with a null . + /// says as much where it marks those rows absent. + /// Every index rule reads through that property, so such a row throws while it is being indexed. + /// + private static LibraryBook bookThatCannotBeIndexed() + => (LibraryBook)Activator.CreateInstance(typeof(LibraryBook), nonPublic: true)!; + + private string[] search(string query) + => [.. new SearchEngine(indexDirectory).Search(query).Docs.Select(d => d.ProductId).Order()]; + + [TestMethod] + public void an_unindexable_book_does_not_cost_the_books_after_it_their_place() + { + List library = [book(PRESENT), bookThatCannotBeIndexed(), book(ABSENT, absentFromLastScan: true), book(THIRD)]; + + var failures = new SearchEngine(indexDirectory).CreateNewIndex(library); + + failures.Should().Be(1); + search("*:*").Should().BeEquivalentTo([PRESENT, ABSENT, THIRD]); + search("Absent").Should().BeEquivalentTo([ABSENT]); + search("-Absent").Should().BeEquivalentTo([PRESENT, THIRD]); + } + + [TestMethod] + public void a_library_that_indexes_cleanly_reports_no_failures() + => new SearchEngine(indexDirectory).CreateNewIndex([book(PRESENT), book(ABSENT)]).Should().Be(0); + + [TestMethod] + public void the_indexed_book_count_is_minus_one_before_there_is_an_index() + => new SearchEngine(indexDirectory).GetIndexedBookCount().Should().Be(-1); + + [TestMethod] + public void the_indexed_book_count_is_what_the_index_holds() + { + var engine = new SearchEngine(indexDirectory); + engine.CreateNewIndex([book(PRESENT), book(ABSENT), book(THIRD)]); + + engine.GetIndexedBookCount().Should().Be(3); + } + + /// + /// The signature from issue #1989, and the reason a short index has to be repaired rather than tolerated: + /// the two halves disagree, so the filter looks half-broken instead of looking like a stale index. + /// + [TestMethod] + public void a_book_the_index_never_received_reads_as_a_half_working_filter() + { + // the absent book is in the library but never made it into the index + new SearchEngine(indexDirectory).CreateNewIndex([book(PRESENT), book(THIRD)]); + + // nothing to show, even though the library has an absent book + search("Absent").Should().BeEquivalentTo([]); + + // and the negation quietly leaves it out, which is indistinguishable from working + search("-Absent").Should().BeEquivalentTo([PRESENT, THIRD]); + + // once the index holds the whole library, both halves agree + new SearchEngine(indexDirectory).CreateNewIndex([book(PRESENT), book(ABSENT, absentFromLastScan: true), book(THIRD)]); + + search("Absent").Should().BeEquivalentTo([ABSENT]); + search("-Absent").Should().BeEquivalentTo([PRESENT, THIRD]); + } +}