Skip to content

Commit

Permalink
2209 h5p exercise url (#194)
Browse files Browse the repository at this point in the history 
* Add root path names to bundle

To be defined in and parsed from books.xml

* Add h5p exercises to bundle and model manager

* Add support for h5p url autocompletion

* Update tests

* Typo: Resource validations -> H5P validations

* Add H5P to allNodes

* Only list orphaned H5P for autocompletion
  • Loading branch information
@TylerZeroMaster
TylerZeroMaster authored May 9, 2024
1 parent 227feb0 commit f5de166
Show file tree
Hide file tree
Showing 12 changed files with 298 additions and 35 deletions.
14 changes: 12 additions & 2 deletions client/src/utils.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -165,10 +165,20 @@ export function launchLanguageServer(context: vscode.ExtensionContext): Language
documentSelector: [{ scheme: 'file', language: 'xml' }],
synchronize: {
fileEvents: [
// FIXME: media, modules, collections, etc. should not be hardcoded here
// NOTE: these patterns do not support matching directories without matching files
// '**/*/' and '**/**/' both match all directories AND files
// Additionally, events are sent for each pattern that matches. This can cause
// massively degraded performance if there are many overlapping patterns
// It is a known issue and a completely new watcher system is being created to address
// the shortcomings of this one
// See: https://github.com/microsoft/vscode/wiki/File-Watcher-Internals
vscode.workspace.createFileSystemWatcher('**/META-INF/books.xml'),
vscode.workspace.createFileSystemWatcher('**/media/**'),
vscode.workspace.createFileSystemWatcher('**/*.cnxml'),
vscode.workspace.createFileSystemWatcher('**/*.collection.xml')
vscode.workspace.createFileSystemWatcher('**/modules/**'),
vscode.workspace.createFileSystemWatcher('**/*.collection.xml'),
vscode.workspace.createFileSystemWatcher('**/interactives/*/'),
vscode.workspace.createFileSystemWatcher('**/interactives/*/h5p.json')
]
}
}
Expand Down
79 changes: 78 additions & 1 deletion server/src/model-manager.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,14 @@ import xmlFormat from 'xml-formatter'
import { expectValue, type Opt, join, PathKind } from './model/utils'
import { Bundle, BundleValidationKind } from './model/bundle'
import { ModelManager } from './model-manager'
import { bookMaker, bundleMaker, first, FS_PATH_HELPER, ignoreConsoleWarnings, loadSuccess, makeBundle, type PageInfo, pageMaker } from './model/spec-helpers.spec'
import { bookMaker, bundleMaker, first, FS_PATH_HELPER, ignoreConsoleWarnings, loadSuccess, makeBundle, type PageInfo, pageMaker, newH5PPath } from './model/spec-helpers.spec'
import { type Job, JobRunner } from './job-runner'

import { PageNode, PageValidationKind } from './model/page'
import { type TocModification, TocModificationKind, TocNodeKind } from '../../common/src/toc'
import { type BooksAndOrphans, DiagnosticSource } from '../../common/src/requests'
import { URI, Utils } from 'vscode-uri'
import { H5PExercise } from './model/h5p-exercise'

ModelManager.debug = () => {} // Turn off logging
JobRunner.debug = () => {} // Turn off logging
Expand Down Expand Up @@ -293,6 +294,7 @@ describe('processFilesystemChange()', () => {
expect((await fireChange(FileChangeType.Created, 'modules/m1234/index.cnxml')).size).toBe(1)
expect((await fireChange(FileChangeType.Created, 'media/newpic.png')).size).toBe(1)
expect((await fireChange(FileChangeType.Created, 'media/does-not-exist.png')).size).toBe(1)
expect((await fireChange(FileChangeType.Created, `${manager.bundle.paths.publicRoot}/does-not-exist/h5p.json`)).size).toBe(1)
})
it('does not create things it does not understand', async () => {
expect((await fireChange(FileChangeType.Created, 'README.md')).toArray()).toEqual([])
Expand All @@ -309,6 +311,7 @@ describe('processFilesystemChange()', () => {
expect(sendDiagnosticsStub.callCount).toBe(1)

expect((await fireChange(FileChangeType.Changed, 'media/newpic.png')).toArray()).toEqual([]) // Since the model was not aware of the file yet
expect((await fireChange(FileChangeType.Changed, `${manager.bundle.paths.publicRoot}/does-not-exist/h5p.json`)).toArray()).toEqual([]) // Since the model was not aware of the file yet
})
it('deletes Files and directories', async () => {
// Load the Bundle, Book, and Page
Expand Down Expand Up @@ -434,6 +437,71 @@ describe('Image Autocomplete', () => {
})
})

describe('URL Autocomplete', () => {
const sinon = SinonRoot.createSandbox()
const h5pName = 'abc'
const orphanedH5PName = '123'
let manager = null as unknown as ModelManager
let page: PageNode
let h5p: H5PExercise
let orphanedH5P: H5PExercise

beforeEach(() => {
const bundle = makeBundle()
manager = new ModelManager(bundle, conn)
page = first(loadSuccess(first(loadSuccess(manager.bundle).books)).pages)
h5p = manager.bundle.allH5P.getOrAdd(newH5PPath(manager.bundle, h5pName))
h5p.load('any-string')
orphanedH5P = manager.bundle.allH5P.getOrAdd(newH5PPath(manager.bundle, orphanedH5PName))
orphanedH5P.load('any-string')
manager.updateFileContents(page.absPath, pageMaker({ pageLinks: [{ url: `${H5PExercise.PLACEHOLDER}/abc` }] }))
})

afterEach(() => {
sinon.restore()
})

it('Returns orphaned H5P interactives', () => {
expect(page.validationErrors.nodesToLoad.toArray()).toEqual([])
expect(page.validationErrors.errors.toArray()).toEqual([])

expect(page.pageLinks.size).toBe(1)
const firstLink = first(page.pageLinks)
const results = manager.autocompleteUrls(
page,
{
line: firstLink.range.start.line,
character: firstLink.range.start.character + '<link url="X'.length
}
)
expect(results).not.toEqual([])
expect(results[0].label).toBe(`${H5PExercise.PLACEHOLDER}/${orphanedH5PName}`)
})

it('Returns no results outside url tag', () => {
expect(page.validationErrors.nodesToLoad.toArray()).toEqual([])
expect(page.validationErrors.errors.toArray()).toEqual([])

const cursor = { line: 0, character: 0 }
const results = manager.autocompleteResources(page, cursor)
expect(results).toEqual([])
})

it('Returns no results outside replacement range', () => {
expect(page.validationErrors.nodesToLoad.toArray()).toEqual([])
expect(page.validationErrors.errors.toArray()).toEqual([])

const firstLink = first(page.pageLinks)
const results = manager.autocompleteResources(
page, {
line: firstLink.range.start.line,
character: firstLink.range.start.character + '<url'.length
}
)
expect(results).toEqual([])
})
})

describe('documentLinks()', () => {
let manager = null as unknown as ModelManager

Expand All @@ -452,6 +520,7 @@ describe('documentLinks()', () => {
const nonexistentButLoadedPath = FS_PATH_HELPER.join(FS_PATH_HELPER.dirname(FS_PATH_HELPER.dirname(page.absPath)), nonexistentButLoadedId, 'index.cnxml')
const otherPage = manager.bundle.allPages.getOrAdd(otherPagePath)
const nonexistentButLoadedPage = manager.bundle.allPages.getOrAdd(nonexistentButLoadedPath)
const h5p = manager.bundle.allH5P.getOrAdd(newH5PPath(manager.bundle, 'abcd'))

otherPage.load(pageMaker({
elementIds: ['other-el-id']
Expand Down Expand Up @@ -482,6 +551,14 @@ describe('documentLinks()', () => {
await testPageLink({ pageLinks: [{ targetPage: otherId, targetId: 'nonexistent-id' }] }, `modules/${otherId}/index.cnxml`)
await testPageLink({ pageLinks: [{ targetPage: otherId, targetId: 'other-el-id' }] }, `modules/${otherId}/index.cnxml#9:0`)
await testPageLink({ pageLinks: [{ targetPage: nonexistentButLoadedId }] }, undefined)
await testPageLink(
{
pageLinks: [
{ url: `${H5PExercise.PLACEHOLDER}/abcd` }
]
},
path.relative(URI.parse(manager.bundle.workspaceRootUri).fsPath, h5p.absPath)
)
})
})

Expand Down
122 changes: 98 additions & 24 deletions server/src/model-manager.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import { type BooksAndOrphans, DiagnosticSource, ExtensionServerNotification } f
import { type BookNode, type TocSubbookWithRange } from './model/book'
import { mkdirp } from 'fs-extra'
import { DOMParser, XMLSerializer } from 'xmldom'
import { H5PExercise } from './model/h5p-exercise'

// Note: `[^/]+` means "All characters except slash"
const IMAGE_RE = /\/media\/[^/]+\.[^.]+$/
Expand All @@ -27,6 +28,11 @@ const BOOK_RE = /\/collections\/[^/]+\.collection\.xml$/
const PATH_SEP = path.sep

interface NodeAndParent { node: ClientTocNode, parent: BookToc | ClientTocNode }
interface Autocompleter {
hasLinkNearCursor: (page: PageNode, cursor: Position) => boolean
getRange: (cursor: Position, content: string) => Range | undefined
getCompletionItems: (page: PageNode, range: Range) => CompletionItem[]
}
function childrenOf(n: ClientTocNode) {
/* istanbul ignore else */
if (n.type === TocNodeKind.Subbook) {
Expand All @@ -49,7 +55,10 @@ function findOrCreateNode(bundle: Bundle, absPath: string) {
return bundle.allPages.getOrAdd(absPath)
} else if (BOOK_RE.test(absPath)) {
return bundle.allBooks.getOrAdd(absPath)
} else if (absPath.endsWith('h5p.json')) {
return bundle.allH5P.getOrAdd(absPath)
}
return undefined
}

function findNode(bundle: Bundle, absPath: string) {
Expand All @@ -58,7 +67,8 @@ function findNode(bundle: Bundle, absPath: string) {
: (
bundle.allBooks.get(absPath) ??
bundle.allPages.get(absPath) ??
bundle.allResources.get(absPath))
bundle.allResources.get(absPath)) ??
bundle.allH5P.get(absPath)
}

export function pageToModuleId(page: PageNode) {
Expand Down Expand Up @@ -176,6 +186,12 @@ export class ModelManager {
return this.bundle.allResources.all.filter(loadedAndExists).subtract(pages.flatMap(p => p.resources))
}

public get orphanedH5P() {
const books = this.bundle.books.filter(loadedAndExists)
const pages = books.flatMap(b => b.pages)
return this.bundle.allH5P.all.filter(loadedAndExists).subtract(pages.flatMap(p => p.h5p))
}

public async loadEnoughForToc() {
// The only reason this is not implemented as a Job is because we need to send a timely response to the client
// and there is no code for being notified when a Job completes
Expand All @@ -191,8 +207,9 @@ export class ModelManager {
public async loadEnoughForOrphans() {
if (this.didLoadOrphans) return
await this.loadEnoughForToc()
const { pagesRoot, mediaRoot, booksRoot, publicRoot } = this.bundle.paths
// Add all the orphaned Images/Pages/Books dangling around in the filesystem without loading them
const files = glob.sync('{modules/*/index.cnxml,media/*.*,collections/*.collection.xml}', { cwd: URI.parse(this.bundle.workspaceRootUri).fsPath, absolute: true })
const files = glob.sync(`{${pagesRoot}/*/*.cnxml,${mediaRoot}/*.*,${booksRoot}/*.collection.xml,${publicRoot}/*/h5p.json}`, { cwd: URI.parse(this.bundle.workspaceRootUri).fsPath, absolute: true })
Quarx.batch(() => {
files.forEach(absPath => expectValue(findOrCreateNode(this.bundle, this.bundle.pathHelper.canonicalize(absPath)), `BUG? We found files that the bundle did not recognize: ${absPath}`))
})
Expand Down Expand Up @@ -258,13 +275,15 @@ export class ModelManager {
// Remove if it was a file
const removedNode = bundle.allBooks.remove(uri) ??
bundle.allPages.remove(uri) ??
bundle.allResources.remove(uri)
bundle.allResources.remove(uri) ??
bundle.allH5P.remove(uri)
if (removedNode !== undefined) s.add(removedNode)
// Remove if it was a directory
const filePathDir = `${uri}${PATH_SEP}`
s.union(bundle.allBooks.removeByKeyPrefix(filePathDir))
s.union(bundle.allPages.removeByKeyPrefix(filePathDir))
s.union(bundle.allResources.removeByKeyPrefix(filePathDir))
s.union(bundle.allH5P.removeByKeyPrefix(filePathDir))
})
// Unload all removed nodes so users do not think the files still exist
removedNodes.forEach(n => {
Expand Down Expand Up @@ -353,6 +372,7 @@ export class ModelManager {
{ slow: true, type: 'INITIAL_LOAD_ALL_BOOK_ERRORS', context: this.bundle, fn: () => { this.bundle.allBooks.all.forEach(b => { this.sendFileDiagnostics(b) }) } },
{ slow: true, type: 'INITIAL_LOAD_ALL_PAGES', context: this.bundle, fn: () => this.bundle.allPages.all.forEach(enqueueLoadJob) },
{ slow: true, type: 'INITIAL_LOAD_ALL_RESOURCES', context: this.bundle, fn: () => this.bundle.allResources.all.forEach(enqueueLoadJob) },
{ slow: true, type: 'INITIAL_LOAD_ALL_H5P', context: this.bundle, fn: () => this.bundle.allH5P.all.forEach(enqueueLoadJob) },
{ slow: true, type: 'INITIAL_LOAD_REPORT_VALIDATION', context: this.bundle, fn: async () => this.bundle.allNodes.forEach(f => { this.sendFileDiagnostics(f) }) }
]
jobs.reverse().forEach(j => { this.jobRunner.enqueue(j) })
Expand Down Expand Up @@ -382,36 +402,88 @@ export class ModelManager {
jobs.reverse().forEach(j => { this.jobRunner.enqueue(j) })
}

public autocompleteResources(page: PageNode, cursor: Position) {
const foundLinks = page.resourceLinks.toArray().filter((l) => {
return inRange(l.range, cursor)
})
private autocomplete(
page: PageNode,
cursor: Position,
autocompleter: Autocompleter
) {
if (!autocompleter.hasLinkNearCursor(page, cursor)) { return [] }

if (foundLinks.length === 0) { return [] }

// We're inside an <image> or <iframe> element.
// Now check and see if we are right at the src=" point
const content = expectValue(this.getOpenDocContents(page.absPath), 'BUG: This file should be open and have been sent from the vscode client').split('\n')
const beforeCursor = content[cursor.line].substring(0, cursor.character)
const afterCursor = content[cursor.line].substring(cursor.character)
const startQuoteOffset = beforeCursor.lastIndexOf('src="')
const endQuoteOffset = afterCursor.indexOf('"')
if (startQuoteOffset >= 0 && endQuoteOffset >= 0) {
const range: Range = {
start: { line: cursor.line, character: startQuoteOffset + 'src="'.length },
end: { line: cursor.line, character: endQuoteOffset + cursor.character }
}
const ret = this.orphanedResources.toArray().map(i => {
const content = expectValue(
this.getOpenDocContents(page.absPath),
'BUG: This file should be open and have been sent from the vscode client'
)

const range = autocompleter.getRange(cursor, content)
if (range === undefined) { return [] }

return autocompleter.getCompletionItems(page, range)
}

private rangeFinderFactory(start: string, end: string) {
return (cursor: Position, content: string) => {
const lines = content.split('\n')
// We're in an autocomplete context
// Now check and see if we are right at the start of the thing we
// want to autocomplete (src, url, etc.)
const beforeCursor = lines[cursor.line].substring(0, cursor.character)
const afterCursor = lines[cursor.line].substring(cursor.character)
const startOffset = beforeCursor.lastIndexOf(start)
const endOffset = afterCursor.indexOf(end)
return startOffset < 0 || endOffset < 0
? undefined
: {
start: { line: cursor.line, character: startOffset + start.length },
end: { line: cursor.line, character: endOffset + cursor.character }
}
}
}

public autocompleteResources(page: PageNode, cursor: Position) {
const resourceAutocompleter: Autocompleter = {
hasLinkNearCursor: (page, cursor) => {
return page.resourceLinks
.toArray()
.filter((l) => inRange(l.range, cursor))
.length > 0
},
getRange: this.rangeFinderFactory('src="', '"'),
getCompletionItems: (page, range) => this.orphanedResources.toArray().map(i => {
const insertText = path.relative(path.dirname(page.absPath), i.absPath)
const item = CompletionItem.create(insertText)
item.textEdit = TextEdit.replace(range, insertText)
item.kind = CompletionItemKind.File
item.detail = 'Orphaned Resource'
return item
})
return ret
}
return []
return this.autocomplete(page, cursor, resourceAutocompleter)
}

public autocompleteUrls(page: PageNode, cursor: Position) {
const urlAutocompleter: Autocompleter = {
hasLinkNearCursor: (page, cursor) => {
return page.pageLinks
.toArray()
.filter((l) => inRange(l.range, cursor))
.length > 0
},
getRange: this.rangeFinderFactory('url="', '"'),
getCompletionItems: (_page, range) => {
return this.orphanedH5P.toArray().filter((h) => h.exists)
.map((h) => path.dirname(h.absPath))
.map((p) => path.basename(p))
.map((name) => {
const text = `${H5PExercise.PLACEHOLDER}/${name}`
const item = CompletionItem.create(text)
item.textEdit = TextEdit.replace(range, text)
item.kind = CompletionItemKind.File
item.detail = 'H5P interactive'
return item
})
}
}
return this.autocomplete(page, cursor, urlAutocompleter)
}

async getDocumentLinks(page: PageNode) {
Expand All @@ -424,6 +496,8 @@ export class ModelManager {
}
if (pageLink.type === PageLinkKind.URL) {
ret.push(DocumentLink.create(pageLink.range, pageLink.url))
} else if (pageLink.type === PageLinkKind.H5P) {
ret.push(DocumentLink.create(pageLink.range, pageLink.h5p.absPath))
} else {
const targetPage = pageLink.page
if (targetPage.isLoaded && !targetPage.exists) {
Expand Down
12 changes: 11 additions & 1 deletion server/src/model/bundle.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,24 @@ import { PageNode } from './page'
import { BookNode } from './book'
import { Fileish, type ValidationCheck, ValidationKind } from './fileish'
import { ResourceNode } from './resource'
import { H5PExercise } from './h5p-exercise'

export class Bundle extends Fileish implements Bundleish {
public readonly allResources: Factory<ResourceNode> = new Factory<ResourceNode>((absPath: string) => new ResourceNode(this, this.pathHelper, absPath), (x) => this.pathHelper.canonicalize(x))
public readonly allPages: Factory<PageNode> = new Factory<PageNode>((absPath: string) => new PageNode(this, this.pathHelper, absPath), (x) => this.pathHelper.canonicalize(x))
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 _duplicateResourcePaths = 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
public readonly paths = {
publicRoot: 'interactives',
privateRoot: 'private',
booksRoot: 'collections',
pagesRoot: 'modules',
mediaRoot: 'media'
}

constructor(pathHelper: PathHelper<string>, public readonly workspaceRootUri: string) {
super(undefined, pathHelper, pathHelper.join(workspaceRootUri, 'META-INF/books.xml'))
Expand Down Expand Up @@ -53,7 +63,7 @@ export class Bundle extends Fileish implements Bundleish {
}

public get allNodes() {
return I.Set([this]).union(this.allBooks.all).union(this.allPages.all).union(this.allResources.all)
return I.Set([this]).union(this.allBooks.all).union(this.allPages.all).union(this.allResources.all).union(this.allH5P.all)
}

public get books() {
Expand Down
Loading

0 comments on commit f5de166

Please sign in to comment.