refactor(integration)!: integration object more like PHP version (#821)

BREAKING CHANGE: In the player renderer, the customScripts property of IPlayerModel was removed, as the custom scripts should be added to the lists of JS scripts of the integration object (and IPlayermModel) to make sure that the scripts are also used in iframes created by the player client. You can remove all code that outputs the contents of IPlayerModel.customScripts.
Lumieducation · Oct 3, 2020 · 02c0be9 · 02c0be9
1 parent 48bcb84
commit 02c0be9
Show file tree

Hide file tree

Showing 8 changed files with 150 additions and 75 deletions.
diff --git a/build.sh b/build.sh
@@ -0,0 +1,6 @@
+#!/bin/bash
+rm -rf build
+npx tsc -p ./tsconfig.build.json
+npx tsc -p ./tsconfig.client.json
+cp -r src/schemas build/src/schemas
+cp -r assets build
diff --git a/package.json b/package.json
@@ -9,7 +9,7 @@
     },
     "scripts": {
         "build:watch": "npx tsc -w -p ./tsconfig.build.json",
-        "build": "rm -rf build && npx tsc -p ./tsconfig.build.json && npx tsc -p ./tsconfig.client.json && cp -r src/schemas build/src/schemas && cp -r assets build",
+        "build": "sh build.sh",
         "ci": "npm run build && npm run lint && npm run format:check && npm run test && npm run test:integration && npm run test:e2e",
         "clear": "rm -rf test/data/hub-content && rm -rf h5p && rm test/data/content-type-cache/real-content-types.json && rm -rf build && rm -rf coverage && rm -rf node_modules",
         "download:content-type-cache": "ts-node scripts/update-real-content-type-cache.ts",

diff --git a/src/H5PPlayer.ts b/src/H5PPlayer.ts
@@ -89,22 +89,6 @@ export default class H5PPlayer {
             throw new H5pError('h5p-player:content-missing', {}, 404);
         }
 
-        const model: IPlayerModel = {
-            contentId,
-            customScripts: this.globalCustomScripts
-                .map((script) => `<script src="${script}"></script>`)
-                .join('\n'),
-            downloadPath: this.getDownloadPath(contentId),
-            integration: this.generateIntegration(
-                contentId,
-                parameters,
-                metadata
-            ),
-            scripts: this.listCoreScripts(),
-            styles: this.listCoreStyles(),
-            translations: {}
-        };
-
         log.debug('Getting list of installed addons.');
         let installedAddons: ILibraryMetadata[] = [];
         if (this.libraryStorage?.listAddons) {
@@ -129,10 +113,28 @@ export default class H5PPlayer {
                     )
             )
         );
-        const libraries = {};
-        await this.getLibraries(dependencies, libraries);
 
-        this.aggregateAssets(dependencies, model, libraries);
+        // Getting lists of scripts and styles needed for the main library.
+        const libraries = await this.getMetadataRecursive(dependencies);
+        const assets = this.aggregateAssetsRecursive(dependencies, libraries);
+
+        const model: IPlayerModel = {
+            contentId,
+            downloadPath: this.getDownloadPath(contentId),
+            integration: this.generateIntegration(
+                contentId,
+                parameters,
+                metadata,
+                assets
+            ),
+            scripts: this.listCoreScripts()
+                .concat(this.globalCustomScripts)
+                .concat(assets.scripts),
+            styles: this.listCoreStyles().concat(assets.styles),
+            translations: {},
+            embedTypes: metadata.embedTypes // TODO: check if the library supports the embed type!
+        };
+
         return this.renderer(model);
     }
 
@@ -148,12 +150,24 @@ export default class H5PPlayer {
         return this;
     }
 
-    private aggregateAssets(
+    /**
+     *
+     * @param dependencies
+     * @param libraries
+     * @param assets
+     * @param loaded
+     * @returns aggregated asset lists
+     */
+    private aggregateAssetsRecursive(
         dependencies: ILibraryName[],
-        assets: IAssets,
-        libraries: object = {},
-        loaded: object = {}
-    ): void {
+        libraries: { [ubername: string]: IInstalledLibrary },
+        assets: IAssets = ({
+            scripts: [],
+            styles: [],
+            translations: []
+        } = { scripts: [], styles: [], translations: [] }),
+        loaded: { [ubername: string]: boolean } = {}
+    ): IAssets {
         log.verbose(
             `loading assets from dependencies: ${dependencies
                 .map((dep) => LibraryName.toUberName(dep))
@@ -166,10 +180,10 @@ export default class H5PPlayer {
             loaded[key] = true;
             const lib = libraries[key];
             if (lib) {
-                this.aggregateAssets(
+                this.aggregateAssetsRecursive(
                     lib.preloadedDependencies || [],
-                    assets,
                     libraries,
+                    assets,
                     loaded
                 );
                 (lib.preloadedCss || []).forEach((asset) =>
@@ -184,6 +198,7 @@ export default class H5PPlayer {
                 );
             }
         });
+        return assets;
     }
 
     /**
@@ -220,7 +235,8 @@ export default class H5PPlayer {
     private generateIntegration(
         contentId: ContentId,
         parameters: ContentParameters,
-        metadata: IContentMetadata
+        metadata: IContentMetadata,
+        assets: IAssets
     ): IIntegration {
         // see https://h5p.org/creating-your-own-h5p-plugin
         log.info(`generating integration for ${contentId}`);
@@ -246,7 +262,11 @@ export default class H5PPlayer {
                         license: metadata.license || 'U',
                         title: metadata.title || '',
                         defaultLanguage: metadata.language || 'en'
-                    }
+                    },
+                    scripts: this.listCoreScripts()
+                        .concat(this.globalCustomScripts)
+                        .concat(assets.scripts),
+                    styles: this.listCoreStyles().concat(assets.styles)
                 }
             },
             core: {
@@ -262,6 +282,7 @@ export default class H5PPlayer {
             postUserStatistics: false,
             saveFreq: false,
             url: this.config.baseUrl,
+            hubIsEnabled: true,
             ...this.integrationObjectDefaults
         };
     }
@@ -393,10 +414,28 @@ export default class H5PPlayer {
         return this.urlGenerator.downloadPackage(contentId);
     }
 
-    private async getLibraries(
+    private async getMetadata(
+        machineName: string,
+        majorVersion: number,
+        minorVersion: number
+    ): Promise<IInstalledLibrary> {
+        log.verbose(
+            `loading library ${machineName}-${majorVersion}.${minorVersion}`
+        );
+        return this.libraryStorage.getLibrary(
+            new LibraryName(machineName, majorVersion, minorVersion)
+        );
+    }
+
+    /**
+     *
+     * @param dependencies
+     * @param loaded can be left out in initial call
+     */
+    private async getMetadataRecursive(
         dependencies: ILibraryName[],
-        loaded: object
-    ): Promise<void> {
+        loaded: { [ubername: string]: IInstalledLibrary } = {}
+    ): Promise<{ [ubername: string]: IInstalledLibrary }> {
         log.verbose(
             `loading libraries from dependencies: ${dependencies
                 .map((dep) => LibraryName.toUberName(dep))
@@ -414,34 +453,22 @@ export default class H5PPlayer {
                 }
                 let lib;
                 try {
-                    lib = await this.getLibrary(name, majVer, minVer);
+                    lib = await this.getMetadata(name, majVer, minVer);
                 } catch {
                     log.info(
                         `Could not find library ${name}-${majVer}.${minVer} in storage. Silently ignoring...`
                     );
                 }
                 if (lib) {
                     loaded[key] = lib;
-                    await this.getLibraries(
+                    await this.getMetadataRecursive(
                         lib.preloadedDependencies || [],
                         loaded
                     );
                 }
             })
         );
-    }
-
-    private async getLibrary(
-        machineName: string,
-        majorVersion: number,
-        minorVersion: number
-    ): Promise<IInstalledLibrary> {
-        log.verbose(
-            `loading library ${machineName}-${majorVersion}.${minorVersion}`
-        );
-        return this.libraryStorage.getLibrary(
-            new LibraryName(machineName, majorVersion, minorVersion)
-        );
+        return loaded;
     }
 
     private listCoreScripts(): string[] {

diff --git a/src/implementation/H5PConfig.ts b/src/implementation/H5PConfig.ts
@@ -88,6 +88,7 @@ export default class H5PConfig implements IH5PConfig {
         await this.loadSettingFromStorage('siteType');
         await this.loadSettingFromStorage('uuid');
         await this.loadSettingFromStorage('exportMaxContentPathLength');
+        await this.loadSettingFromStorage('baseUrl');
         return this;
     }
 

diff --git a/src/index.ts b/src/index.ts
@@ -20,9 +20,13 @@ import {
     ContentParameters,
     IContentMetadata,
     IContentStorage,
+    IEditorModel,
+    IFileStats,
     IH5PConfig,
     IInstalledLibrary,
+    IIntegration,
     IKeyValueStorage,
+    ILibraryAdministrationOverviewItem,
     ILibraryFileUrlResolver,
     ILibraryMetadata,
     ILibraryName,
@@ -32,9 +36,7 @@ import {
     ITemporaryFileStorage,
     ITranslationFunction,
     IUser,
-    Permission,
-    ILibraryAdministrationOverviewItem,
-    IFileStats
+    Permission
 } from './types';
 
 // Adapters
@@ -62,21 +64,23 @@ export {
     ContentParameters,
     IContentMetadata,
     IContentStorage,
+    IEditorModel,
+    IFileStats,
     IH5PConfig,
     IInstalledLibrary,
+    IIntegration,
     IKeyValueStorage,
+    ILibraryAdministrationOverviewItem,
     ILibraryFileUrlResolver,
     ILibraryMetadata,
     ILibraryName,
     ILibraryStorage,
-    ILibraryAdministrationOverviewItem,
     IPlayerModel,
     ITemporaryFile,
     ITemporaryFileStorage,
     ITranslationFunction,
     IUser,
     Permission,
-    IFileStats,
     // implementations
     H5PConfig,
     fs,

diff --git a/src/renderers/player.ts b/src/renderers/player.ts
@@ -2,7 +2,7 @@ export default (model) => `<!doctype html>
 <html class="h5p-iframe">
 <head>
     <meta charset="utf-8">
-    
+   
     ${model.styles
         .map((style) => `<link rel="stylesheet" href="${style}"/>`)
         .join('\n    ')}
@@ -11,9 +11,8 @@ export default (model) => `<!doctype html>
         .join('\n    ')}
 
     <script>
-        H5PIntegration = ${JSON.stringify(model.integration, null, 2)};
+        window.H5PIntegration = ${JSON.stringify(model.integration, null, 2)};
     </script>
-    ${model.customScripts}
 </head>
 <body>
     <div class="h5p-content" data-content-id="${model.contentId}"></div>

diff --git a/src/types.ts b/src/types.ts
@@ -231,6 +231,16 @@ export interface IIntegration {
              * Example: <script src=\"https://example.org/h5p/library/js/h5p-resizer.js\" charset=\"UTF-8\"></script>
              */
             resizeCode?: string;
+            /**
+             * A complete list of scripts required to display the content.
+             * Includes core scripts and content type specific scripts.
+             */
+            scripts?: string[];
+            /**
+             * A complete list of styles required to display the content.
+             * Includes core scripts and content type specific styles.
+             */
+            styles?: string[];
             /**
              * The absolute URL to the current content.
              */
@@ -278,14 +288,10 @@ export interface IIntegration {
         [namespace: string]: any;
     };
     /**
-     * Can be null. Usage is unknown. the server might be able to customize
-     * library behavior by setting the library config for certain machine names,
-     * as the H5P client allows it to be called by executing
-     * H5P.getLibraryConfig(machineName). This means that libraries can retrieve
-     * configuration values from the server that way. The core never calls the
-     * method and none of the content types on the H5P hub do so...
-     * The Moodle implementation simply passed through a configuration value
-     * in this case.
+     * Can be null. The server can customize library behavior by setting the
+     * library config for certain machine names, as the H5P client allows it to
+     * be called by executing H5P.getLibraryConfig(machineName). This means that
+     * libraries can retrieve configuration values from the server that way.
      */
     libraryConfig?: {
         [machineName: string]: any;
@@ -1616,8 +1622,8 @@ export interface IHubInfo {
 
 export interface IPlayerModel {
     contentId: ContentParameters;
-    customScripts: string;
     downloadPath: string;
+    embedTypes: ('iframe' | 'div')[];
     integration: IIntegration;
     scripts: string[];
     styles: string[];