diff --git a/backend/internal/chunk/chunktest/epub.go b/backend/internal/chunk/chunktest/epub.go index 5dd3f71a..ca344814 100644 --- a/backend/internal/chunk/chunktest/epub.go +++ b/backend/internal/chunk/chunktest/epub.go @@ -35,6 +35,9 @@ type Chapter struct { // Raw writes Body verbatim instead of wrapping it in the xhtml skeleton, for the nav document and // anything else that must control its own markup. Raw bool + // Declared writes the manifest item but NOT the file, for a book that promises a document it does not + // carry — the shape a scraped EPUB arrives in. + Declared bool } // SpineRef is one spine entry: the manifest id and its `linear` attribute ("" → linear, i.e. absent). @@ -51,6 +54,8 @@ type Nav struct { // InSpine puts the navigation document in the spine, which real books do and which is exactly the case // the service-page exclusion has to handle. InSpine bool + // Missing declares the navigation document in the manifest and omits its bytes. + Missing bool } // GuideRef is one EPUB 2 ``: how a book with no navigation document declares that a @@ -64,7 +69,11 @@ type EPUB struct { Spine []SpineRef Nav *Nav NCX []string // EPUB 2 navPoint hrefs, in order; nesting is not needed to test flattening - Guide []GuideRef + // NCXBroken writes bytes that are not an NCX, for a book whose spine names one it cannot parse. + NCXBroken bool + // NCXMissing names an NCX in the spine and omits its bytes. + NCXMissing bool + Guide []GuideRef } // entryName is where this chapter's bytes are written inside OEBPS/. @@ -132,13 +141,19 @@ func (e EPUB) BuildAt(t *testing.T, path string) { if href == "" { href = "nav.xhtml" } - chapters = append(chapters, Chapter{ID: "navdoc", Href: href, Properties: "nav", Raw: true, Body: navXHTML(*e.Nav)}) + chapters = append(chapters, Chapter{ID: "navdoc", Href: href, Properties: "nav", Raw: true, + Body: navXHTML(*e.Nav), Declared: e.Nav.Missing}) if e.Nav.InSpine { refs = append([]SpineRef{{ID: "navdoc"}}, refs...) } } - if len(e.NCX) > 0 { - chapters = append(chapters, Chapter{ID: "ncx", Href: "toc.ncx", MType: "application/x-dtbncx+xml", Raw: true, Body: ncxXML(e.NCX)}) + if len(e.NCX) > 0 || e.NCXBroken || e.NCXMissing { + body := ncxXML(e.NCX) + if e.NCXBroken { + body = "" // not an NCX: xml.Unmarshal refuses it + } + chapters = append(chapters, Chapter{ID: "ncx", Href: "toc.ncx", MType: "application/x-dtbncx+xml", + Raw: true, Body: body, Declared: e.NCXMissing}) } var manifest, spine strings.Builder @@ -161,7 +176,7 @@ func (e EPUB) BuildAt(t *testing.T, path string) { spine.WriteString(`` + "\n") } spineAttr := "" - if len(e.NCX) > 0 { + if len(e.NCX) > 0 || e.NCXBroken || e.NCXMissing { spineAttr = ` toc="ncx"` } var guide strings.Builder @@ -185,6 +200,9 @@ func (e EPUB) BuildAt(t *testing.T, path string) { // media-type labels it — real epubs mislabel xhtml as application/xml, etc. The entry // name (not the href) decides, because an href may be percent-encoded or fragment-bearing. entry := c.entryName() + if c.Declared { + continue // promised in the manifest, absent from the archive + } if c.Raw { add("OEBPS/"+entry, c.Body) continue diff --git a/backend/internal/chunk/epubtoc.go b/backend/internal/chunk/epubtoc.go index 21f4b753..7135708c 100644 --- a/backend/internal/chunk/epubtoc.go +++ b/backend/internal/chunk/epubtoc.go @@ -216,16 +216,20 @@ func (p ncxPoint) flatten(baseDir string, out *[]string, labels map[string]strin } // parseNCX reads an EPUB 2 NCX, returning its navPoint targets in document order. -func parseNCX(data []byte, baseDir string) (targets []string, labels map[string]string) { +// +// The parse error is RETURNED, not swallowed. A book whose spine names an NCX whose bytes will not parse +// has lost its whole statement about its chapters, and the cut then falls back to the spine — which looks +// exactly like a book that never had a table of contents at all. +func parseNCX(data []byte, baseDir string) (targets []string, labels map[string]string, err error) { labels = map[string]string{} var d ncxDoc if err := xml.Unmarshal(data, &d); err != nil { - return nil, labels + return nil, labels, err } for _, p := range d.Points { p.flatten(baseDir, &targets, labels) } - return targets, labels + return targets, labels, nil } // resolveTOC maps TOC hrefs onto spine positions. diff --git a/backend/internal/chunk/epubtoc_test.go b/backend/internal/chunk/epubtoc_test.go index b8dfccdb..e7b28172 100644 --- a/backend/internal/chunk/epubtoc_test.go +++ b/backend/internal/chunk/epubtoc_test.go @@ -695,7 +695,7 @@ func TestNavLabelIsTrimmedLikeItsSiblings(t *testing.T) { ncx := []byte(` Chapter One `) - _, ncxLabels := parseNCX(ncx, "OEBPS") + _, ncxLabels, _ := parseNCX(ncx, "OEBPS") const key, want = "OEBPS/c1.xhtml", "Chapter One" if navLabels[key] != want { @@ -705,3 +705,76 @@ func TestNavLabelIsTrimmedLikeItsSiblings(t *testing.T) { t.Fatalf("one title, two answers: nav %q vs ncx %q", navLabels[key], ncxLabels[key]) } } + +// TestADeclaredTableOfContentsThatCannotBeReadIsCounted closes the silence this reader had left in its own +// alarm: a book that DECLARES a nav or an NCX and whose bytes cannot be read falls back to the spine and is +// sold by characters, with nothing saying why. +// +// ⛔ THREE CASES, AND THE POINT IS THAT THEY STAY APART. "Declared and unreadable" is not "read, and some of +// its targets named nothing" — the first lost the book's whole statement about its chapters, the second lost +// part of it. And "declared nothing at all" is not a degradation: going to the spine is that book's correct +// path. Merging any two of the three moves the defect into new clothes instead of closing it. +func TestADeclaredTableOfContentsThatCannotBeReadIsCounted(t *testing.T) { + chapters := []chunktest.Chapter{ch("c1", "c1.xhtml", "one"), ch("c2", "c2.xhtml", "two")} + for _, tc := range []struct { + name string + book chunktest.EPUB + wantUnreadable, wantUnresolved int + wantStructure string + }{ + { + name: "a nav promised by the manifest and absent from the archive", + book: chunktest.EPUB{Chapters: chapters, Spine: refs("c1", "c2"), Nav: &chunktest.Nav{TOC: []string{"c1.xhtml", "c2.xhtml"}, Missing: true}}, + wantUnreadable: 1, wantStructure: StructureDelimited, + }, + { + name: "an NCX named by the spine and absent from the archive", + book: chunktest.EPUB{Chapters: chapters, Spine: refs("c1", "c2"), NCXMissing: true}, + wantUnreadable: 1, wantStructure: StructureDelimited, + }, + { + name: "an NCX that is present and is not an NCX", + book: chunktest.EPUB{Chapters: chapters, Spine: refs("c1", "c2"), NCXBroken: true}, + wantUnreadable: 1, wantStructure: StructureDelimited, + }, + { + // ⚠ NOT a degradation: this book never claimed to have a table of contents. + name: "no table of contents declared at all", + book: chunktest.EPUB{Chapters: chapters, Spine: refs("c1", "c2")}, + wantUnreadable: 0, wantStructure: StructureDelimited, + }, + { + // The neighbouring failure, kept distinct: the table WAS read, its targets named nothing. + name: "a nav that was read and whose targets dangle", + book: chunktest.EPUB{Chapters: chapters, Spine: refs("c1", "c2"), Nav: &chunktest.Nav{TOC: []string{"gone-a.xhtml", "gone-b.xhtml"}}}, + wantUnreadable: 0, wantUnresolved: 2, wantStructure: StructureDelimited, + }, + } { + t.Run(tc.name, func(t *testing.T) { + doc, err := ingest(tc.book.Build(t)) + if err != nil { + t.Fatal(err) + } + if doc.TOCUnreadable != tc.wantUnreadable { + t.Fatalf("toc unreadable = %d, want %d", doc.TOCUnreadable, tc.wantUnreadable) + } + if doc.TOCUnresolved != tc.wantUnresolved { + t.Fatalf("toc unresolved = %d, want %d — the two failures must not merge", doc.TOCUnresolved, tc.wantUnresolved) + } + if doc.Structure != tc.wantStructure { + t.Fatalf("structure = %q, want %q", doc.Structure, tc.wantStructure) + } + // And whatever it counted must reach a human. + notes := doc.IngestNotes() + named := false + for i := 0; i+1 < len(notes); i += 2 { + if notes[i] == "toc_unreadable" { + named = true + } + } + if named != (tc.wantUnreadable > 0) { + t.Fatalf("alarm names toc_unreadable = %v, want %v (notes: %v)", named, tc.wantUnreadable > 0, notes) + } + }) + } +} diff --git a/backend/internal/chunk/ingest.go b/backend/internal/chunk/ingest.go index 97fd935e..a15a9301 100644 --- a/backend/internal/chunk/ingest.go +++ b/backend/internal/chunk/ingest.go @@ -82,7 +82,21 @@ type Document struct { // WRONG exclusion — the document is accounted for, so the sum closes — which means a mistaken exclusion // is exactly the defect no gate can catch. What is left is a human reading the warning, and "excluded: 1" // gives them nothing to doubt. «excluded OEBPS/index_split_124.html as `toc`, 2019 runes» does. - Excluded []ExcludedDocument + Excluded []ExcludedDocument + // TOCUnreadable counts tables of contents the book DECLARED and this reader could not read at all — + // the manifest names a nav document, or the spine names an NCX, and the bytes are missing or will not + // parse. + // + // ⛔ IT IS NOT TOCUnresolved, and merging the two would hide the worse case inside the milder one. + // "Read it, and some of its targets named nothing" is a book with a partly broken table; "declared one + // and could not read it at all" is a book whose whole statement about its chapters was lost, after + // which the cut silently falls back to the spine and the book is sold by characters. A reader must be + // able to tell those apart, because only the second explains why an EPUB with a real nav came back + // `delimited`. + // + // ⚠ A book that declares NO table of contents is not counted here: going to the spine is its normal, + // correct path, not a degradation. + TOCUnreadable int TOCUnresolved int // table-of-contents targets that named nothing this book reads TargetsCollapsed int // targets that shared a document with an earlier one, so the cut is coarser ServiceRolesInsideDocuments int // toc/cover roles declared at a POINT inside a document, so NOT excluded @@ -113,6 +127,7 @@ func (d *Document) IngestNotes() []any { if n := len(d.Excluded); n > 0 { kv = append(kv, "documents_excluded", n, "excluded", d.Excluded) } + add("toc_unreadable", d.TOCUnreadable) add("toc_unresolved", d.TOCUnresolved) add("targets_collapsed", d.TargetsCollapsed) add("service_roles_inside_documents", d.ServiceRolesInsideDocuments) @@ -730,8 +745,14 @@ func ingestEPUB(p string) (*Document, error) { } var navTargets []string labels := map[string]string{} + tocUnreadable := 0 if navEntry != "" { - if data, rerr := readZipEntry(files, navEntry); rerr == nil { + data, rerr := readZipEntry(files, navEntry) + if rerr != nil { + // The manifest SAYS this book has a navigation document. Falling through to the spine without + // saying so is the silence this counter exists to end. + tocUnreadable++ + } else { targets, navLabels, landmarks, insideDoc := parseNavDoc(data, path.Dir(navEntry)) navTargets, labels = targets, navLabels serviceInsideDoc += insideDoc @@ -772,8 +793,15 @@ func ingestEPUB(p string) (*Document, error) { // book. Falling through to the NCX must not erase that count along with the failed table. navUnresolved := toc.unresolved if ncxEntry := ncxPath(opf, opfDir); ncxEntry != "" { - if data, rerr := readZipEntry(files, ncxEntry); rerr == nil { - targets, ncxLabels := parseNCX(data, path.Dir(ncxEntry)) + data, rerr := readZipEntry(files, ncxEntry) + switch { + case rerr != nil: + tocUnreadable++ // the spine names an NCX and its bytes are not there + default: + targets, ncxLabels, perr := parseNCX(data, path.Dir(ncxEntry)) + if perr != nil { + tocUnreadable++ // the bytes are there and are not an NCX + } if ncx := resolveTOC(tocNCX, targets, docs); ncx.kind == tocNCX { // Switching tables: carry the nav's dangling count across, or it vanishes with the table. toc, labels = ncx, ncxLabels @@ -835,7 +863,7 @@ func ingestEPUB(p string) (*Document, error) { } doc := &Document{ DocumentsAttached: attached, Excluded: excluded, - TOCUnresolved: toc.unresolved, TargetsCollapsed: toc.collapsed, + TOCUnreadable: tocUnreadable, TOCUnresolved: toc.unresolved, TargetsCollapsed: toc.collapsed, ServiceRolesInsideDocuments: serviceInsideDoc, } denseNo := 0 // matches SplitChunks: only a NON-empty chapter takes a number