Skip to content

Commit

Permalink
fix(saving): exported packages now allows conform to the h5p.json sch…
Browse files Browse the repository at this point in the history
…ema (resolves #294) (#323)

* added test to find errors in metadata

* fix(saving): exported packages now follow the h5p.json schema even if the metadata received from the client is incomplete (resolves #294)

* fix(tests): added required property for mock of ContentMetadata
  • Loading branch information
sr258 authored and JPSchellenberg committed Dec 16, 2019
1 parent 1b62353 commit b2fd4e6
Show file tree
Hide file tree
Showing 9 changed files with 319 additions and 174 deletions.
9 changes: 1 addition & 8 deletions examples/express.ts
Original file line number Diff line number Diff line change
Expand Up @@ -134,19 +134,12 @@ const start = async () => {
return res.redirect('/');
}

const packageExporter = new h5pLib.PackageExporter(
h5pEditor.libraryManager,
h5pEditor.translationService,
h5pEditor.config,
h5pEditor.contentManager
);

// set filename for the package with .h5p extension
res.setHeader(
'Content-disposition',
`attachment; filename=${req.query.contentId}.h5p`
);
await packageExporter.createPackage(req.query.contentId, res, user);
await h5pEditor.exportPackage(req.query.contentId, res, user);
});

server.get('/examples/:key', (req, res) => {
Expand Down
7 changes: 6 additions & 1 deletion src/ContentManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import globPromise from 'glob-promise';
import * as path from 'path';
import { Stream } from 'stream';

import { ContentMetadata } from './ContentMetadata';
import H5pError from './helpers/H5pError';
import { streamToString } from './helpers/StreamHelpers';
import {
Expand Down Expand Up @@ -262,7 +263,11 @@ export default class ContentManager {
contentId: ContentId,
user: IUser
): Promise<IContentMetadata> {
return this.getFileJson(contentId, 'h5p.json', user);
// We don't directly return the h5p.json file content as
// we have to make sure it conforms to the schema.
return new ContentMetadata(
await this.getFileJson(contentId, 'h5p.json', user)
);
}

/**
Expand Down
53 changes: 53 additions & 0 deletions src/ContentMetadata.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
import {
IContentAuthor,
IContentChange,
IContentMetadata,
ILibraryName
} from './types';

/**
* Content metadata object with defaults for required values and
* sanitization to make sure it the metadata conforms to the schema.
*/
export class ContentMetadata implements IContentMetadata {
/**
* Creates an object conforming to the h5p.json schema.
* @param furtherMetadata these objects will be merged into the newly created object
*/
constructor(...furtherMetadata: any[]) {
for (const metadata of furtherMetadata) {
Object.assign(this, metadata);
}

// Remove empty arrays for authors and changes, as this validates the
// H5P schema.
if (this.authors && this.authors.length === 0) {
this.authors = undefined;
}
if (this.changes && this.changes.length === 0) {
this.changes = undefined;
}
}
public author?: string;
public authorComments?: string;
public authors?: IContentAuthor[];
public changes?: IContentChange[];
public contentType?: string;
public dynamicDependencies?: ILibraryName[];
public editorDependencies?: ILibraryName[];
public embedTypes: ('iframe' | 'div')[] = ['iframe'];
public h?: string;
public language: string = 'en';
public license?: string;
public licenseExtras?: string;
public licenseVersion?: string;
public mainLibrary: string;
public metaDescription?: string;
public metaKeywords?: string;
public preloadedDependencies: ILibraryName[];
public source?: string;
public title: string;
public w?: string;
public yearsFrom?: string;
public yearsTo?: string;
}
70 changes: 47 additions & 23 deletions src/H5PEditor.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { ReadStream } from 'fs';
import { ReadStream, WriteStream } from 'fs';
import fsExtra from 'fs-extra';
import promisepipe from 'promisepipe';
import stream from 'stream';
Expand All @@ -9,13 +9,14 @@ import defaultTranslation from '../assets/translations/en.json';
import defaultRenderer from './renderers/default';

import ContentManager from './ContentManager';
import { ContentMetadata } from './ContentMetadata';
import ContentStorer from './ContentStorer';
import ContentTypeCache from './ContentTypeCache';
import ContentTypeInformationRepository from './ContentTypeInformationRepository';
import H5pError from './helpers/H5pError';
import InstalledLibrary from './InstalledLibrary';
import LibraryManager from './LibraryManager';
import LibraryName from './LibraryName';
import PackageExporter from './PackageExporter';
import PackageImporter from './PackageImporter';
import TemporaryFileManager from './TemporaryFileManager';
import TranslationService from './TranslationService';
Expand Down Expand Up @@ -97,6 +98,12 @@ export default class H5PEditor {
this.contentManager,
this.contentStorer
);
this.packageExporter = new PackageExporter(
this.libraryManager,
translationService,
config,
this.contentManager
);
}

public contentManager: ContentManager;
Expand All @@ -112,9 +119,28 @@ export default class H5PEditor {
private contentTypeRepository: ContentTypeInformationRepository;
private filesPath: string;
private libraryUrl: string;
private packageExporter: PackageExporter;
private renderer: any;
private translation: any;

/**
* Creates a .h5p-package for the specified content file and pipes it to the stream.
* Throws H5pErrors if something goes wrong. The contents of the stream should be disregarded then.
* @param contentId The contentId for which the package should be created.
* @param outputStream The stream that the package is written to (e.g. the response stream fo Express)
*/
public async exportPackage(
contentId: ContentId,
outputStream: WriteStream,
user: IUser
): Promise<void> {
return this.packageExporter.createPackage(
contentId,
outputStream,
user
);
}

/**
* Returns a stream for a file that was uploaded for a content object.
* The requested content file can be a temporary file uploaded for unsaved content or a
Expand Down Expand Up @@ -637,32 +663,30 @@ export default class H5PEditor {
return Object.values(collect);
}

private generateH5PJSON(
private async generateH5PJSON(
metadata: IContentMetadata,
libraryName: ILibraryName,
contentDependencies: ILibraryName[] = []
): Promise<IContentMetadata> {
log.info(`generating h5p.json`);
return new Promise((resolve: (value: IContentMetadata) => void) => {
this.libraryManager
.loadLibrary(libraryName)
.then((library: InstalledLibrary) => {
const h5pJson: IContentMetadata = {
...metadata,
mainLibrary: library.machineName,
preloadedDependencies: [
...contentDependencies,
...(library.preloadedDependencies || []), // empty array should preloadedDependencies be undefined
{
machineName: library.machineName,
majorVersion: library.majorVersion,
minorVersion: library.minorVersion
}
]
};
resolve(h5pJson);
});
});

const library = await this.libraryManager.loadLibrary(libraryName);
const h5pJson: IContentMetadata = new ContentMetadata(
metadata,
{ mainLibrary: library.machineName },
{
preloadedDependencies: [
...contentDependencies,
...(library.preloadedDependencies || []), // empty array should preloadedDependencies be undefined
{
machineName: library.machineName,
majorVersion: library.majorVersion,
minorVersion: library.minorVersion
}
]
}
);
return h5pJson;
}

private integration(contentId: ContentId): IIntegration {
Expand Down
2 changes: 1 addition & 1 deletion src/PackageValidator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ export default class PackageValidator {
// we await the promise here because we want to catch the error and return undefined
return await yauzlPromise.open(file, { lazyEntries: false });
} catch (ignored) {
log.error(`zip file ${file} could not be opened`);
log.error(`zip file ${file} could not be opened: ${ignored}`);
return undefined;
}
}
Expand Down
Loading

0 comments on commit b2fd4e6

Please sign in to comment.