Skip to content

Commit

Permalink
More validation for assumptions made in Enki (#203)
Browse files Browse the repository at this point in the history
* Check that slugs in books.xml match a slug in a book

* Check book filename matches assumption made in epub builds

Epub builds assume books will be named <slug>.collection.xml

* Only check the book defined on the same element as the slug

Match ranges to target specific book node for validation

* Do not allow bundle MISSING_BOOK and MISMATCHED_SLUG overlap

* Add tests for new validation; fix existing tests
  • Loading branch information
TylerZeroMaster authored Jul 1, 2024
1 parent 3a3f92d commit dbc0fb5
Show file tree
Hide file tree
Showing 4 changed files with 68 additions and 5 deletions.
15 changes: 13 additions & 2 deletions server/src/model/book.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ describe('Book validations', () => {
{ title: chapterTitle, children: [] },
{ title: chapterTitle, children: [] }
]
book.load(bookMaker({ toc }))
book.load(bookMaker({ toc, slug: 'test' }))
expectErrors(book, [BookValidationKind.DUPLICATE_CHAPTER_TITLE, BookValidationKind.DUPLICATE_CHAPTER_TITLE])
})
it(BookValidationKind.MISSING_PAGE.title, () => {
Expand All @@ -28,11 +28,22 @@ describe('Book validations', () => {
{ title: 'Chapter 1', children: ['m00001'] },
{ title: 'Chapter 2', children: ['m00001'] }
]
book.load(bookMaker({ toc }))
book.load(bookMaker({ toc, slug: 'test' }))
const page = first(book.pages)
page.load(pageMaker({}))
expectErrors(book, [BookValidationKind.DUPLICATE_PAGE, BookValidationKind.DUPLICATE_PAGE])
})
it(BookValidationKind.INVALID_BOOK_NAME.title, () => {
const bundle = makeBundle()
const book = first(loadSuccess(bundle).books)
const toc: BookMakerTocNode[] = [
{ title: 'Chapter 1', children: ['m00001'] }
]
book.load(bookMaker({ toc, slug: 'not-test' }))
const page = first(book.pages)
page.load(pageMaker({}))
expectErrors(book, [BookValidationKind.INVALID_BOOK_NAME])
})
})

describe('Book computed properties', () => {
Expand Down
11 changes: 10 additions & 1 deletion server/src/model/book.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import I from 'immutable'
import * as Quarx from 'quarx'
import { type PageNode } from './page'
import { type Opt, type WithRange, textWithRange, select, selectOne, findDuplicates, calculateElementPositions, expectValue, type HasRange, join, equalsOpt, equalsWithRange, tripleEq, equalsPos, equalsArray, PathKind, TocNodeKind } from './utils'
import { type Opt, type WithRange, textWithRange, select, selectOne, findDuplicates, calculateElementPositions, expectValue, type HasRange, join, equalsOpt, equalsWithRange, tripleEq, equalsPos, equalsArray, PathKind, TocNodeKind, NOWHERE } from './utils'
import { Fileish, type ValidationCheck, ValidationKind } from './fileish'
import { getCCLicense } from './cc-license'

Expand Down Expand Up @@ -157,12 +157,21 @@ export class BookNode extends Fileish {
message: BookValidationKind.DUPLICATE_PAGE,
nodesToLoad: I.Set(),
fn: () => I.Set(pageLeaves.filter(p => duplicatePages.has(p.page)).map(l => l.range))
},
{
message: BookValidationKind.INVALID_BOOK_NAME,
nodesToLoad: I.Set(),
fn: () => this.absPath.endsWith(`${this.slug}.collection.xml`)
? I.Set()
: I.Set([NOWHERE])
}
]
}
}

export class BookValidationKind extends ValidationKind {
// openstax/enki/bakery-js/src/epub/toc.tsx#L74
static INVALID_BOOK_NAME = new BookValidationKind('Book must be named <slug>.collection.xml')
static MISSING_PAGE = new BookValidationKind('Missing Page')
static DUPLICATE_CHAPTER_TITLE = new BookValidationKind('Duplicate chapter title')
static DUPLICATE_PAGE = new BookValidationKind('Duplicate page')
Expand Down
8 changes: 7 additions & 1 deletion server/src/model/bundle.spec.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { expect } from '@jest/globals'
import { type Bundle, BundleValidationKind } from './bundle'
import { bundleMaker, expectErrors, first, loadSuccess, makeBundle, read } from './spec-helpers.spec'
import { bookMaker, bundleMaker, expectErrors, first, loadSuccess, makeBundle, read } from './spec-helpers.spec'

describe('Bundle validations', () => {
it(BundleValidationKind.NO_BOOKS.title, () => {
Expand All @@ -14,6 +14,12 @@ describe('Bundle validations', () => {
book.load(undefined)
expectErrors(bundle, [BundleValidationKind.MISSING_BOOK])
})
it(BundleValidationKind.MISMATCHED_SLUG.title, () => {
const bundle = loadSuccess(makeBundle())
const book = first(bundle.books)
book.load(bookMaker({ slug: 'something' }))
expectErrors(bundle, [BundleValidationKind.MISMATCHED_SLUG])
})
})

describe('Happy path', () => {
Expand Down
39 changes: 38 additions & 1 deletion server/src/model/bundle.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ export class Bundle extends Fileish implements Bundleish {
public readonly allH5P: Factory<H5PExercise> = new Factory<H5PExercise>((absPath: string) => new H5PExercise(this, this.pathHelper, absPath), (x) => this.pathHelper.canonicalize(x))
public readonly allBooks = new Factory((absPath: string) => new BookNode(this, this.pathHelper, absPath), (x) => this.pathHelper.canonicalize(x))
private readonly _books = Quarx.observable.box<Opt<I.Set<WithRange<BookNode>>>>(undefined)
private readonly _booksXMLBooks = Quarx.observable.box<Opt<I.Set<WithRange<BooksXMLBook>>>>(undefined)
private readonly _duplicateFilePaths = Quarx.observable.box<I.Set<string>>(I.Set<string>())
private readonly _duplicateUUIDs = Quarx.observable.box<I.Set<string>>(I.Set<string>())
// TODO: parse these from META-INF/books.xml
Expand Down Expand Up @@ -52,9 +53,17 @@ export class Bundle extends Fileish implements Bundleish {

protected parseXML = (doc: Document) => {
const bookNodes = select('//bk:book', doc) as Element[]
this._books.set(I.Set(bookNodes.map(b => {
const booksXMLBooks = I.Set(bookNodes.map((b => {
const range = calculateElementPositions(b)
const href = expectValue(b.getAttribute('href'), 'ERROR: Missing @href attribute on book element')
const slug = expectValue(b.getAttribute('slug'), 'ERROR: Missing @slug attribute on book element')
const v: BooksXMLBook = { slug, href }
return { v, range }
})))
this._booksXMLBooks.set(booksXMLBooks)
this._books.set(I.Set(booksXMLBooks.map(b => {
const range = b.range
const href = b.v.href
const book = this.allBooks.getOrAdd(join(this.pathHelper, PathKind.ABS_TO_REL, this.absPath, href))
return {
v: book,
Expand Down Expand Up @@ -90,6 +99,7 @@ export class Bundle extends Fileish implements Bundleish {

protected getValidationChecks(): ValidationCheck[] {
const books = this.__books()
const booksXMLBooks = this.ensureLoaded(this._booksXMLBooks)
return [
{
message: BundleValidationKind.MISSING_BOOK,
Expand All @@ -100,12 +110,39 @@ export class Bundle extends Fileish implements Bundleish {
message: BundleValidationKind.NO_BOOKS,
nodesToLoad: I.Set(),
fn: () => books.isEmpty() ? I.Set([NOWHERE]) : I.Set()
},
{
message: BundleValidationKind.MISMATCHED_SLUG,
nodesToLoad: this.books,
fn: () => {
return booksXMLBooks
.filter(({ v: bx, range: rx }) => {
const maybeBookNode = books.find(
({ range: r }) => r.start === rx.start && r.end === rx.end
)
if (maybeBookNode !== undefined) {
const { v: book } = maybeBookNode
if (book.isValidXML && book.exists) {
return book.slug !== bx.slug
}
}
return false
})
.map(({ range }) => range)
}
}
]
}
}

export class BundleValidationKind extends ValidationKind {
// openstax/enki/bakery-src/scripts/link_single.py#L59
static MISMATCHED_SLUG = new BundleValidationKind('Slug does not match the one defined in the book')
static MISSING_BOOK = new BundleValidationKind('Missing book')
static NO_BOOKS = new BundleValidationKind('No books defined')
}

interface BooksXMLBook {
slug: string
href: string
}

0 comments on commit dbc0fb5

Please sign in to comment.