diff --git a/client/src/utils.ts b/client/src/utils.ts index 9cfa4be3..6476231a 100644 --- a/client/src/utils.ts +++ b/client/src/utils.ts @@ -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') ] } } diff --git a/server/src/model-manager.spec.ts b/server/src/model-manager.spec.ts index cd3a1666..800af9bc 100644 --- a/server/src/model-manager.spec.ts +++ b/server/src/model-manager.spec.ts @@ -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 @@ -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([]) @@ -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 @@ -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 + ' 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) { @@ -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) { @@ -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) { @@ -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 @@ -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}`)) }) @@ -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 => { @@ -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) }) @@ -382,26 +402,53 @@ 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 or