H5P import (#1874)

* fix(thumbnail-upload): upload the thumbnails in parallel

* refactor(h5p-service): refactor and simplify the HTML and H5P upload and item creation

* feat(graasp-import): import the H5P files

Author: MatveyK

src/services/item/plugins/file/itemFile.controller.ts

@@ -138,7 +138,7 @@ const basePlugin: FastifyPluginAsyncTypebox<GraaspPluginFileOptions> = async (fa
             // othwerwise, we save it as a generic file
             let item: ItemRaw;
             if (getFileExtension(filename) === H5P_FILE_EXTENSION) {
-              item = await h5pService.createH5PItem(
+              item = await h5pService.uploadFileAndCreateItem(
                 tx,
                 member,
                 filename,

src/services/item/plugins/html/h5p/h5p.controller.test.ts

@@ -19,10 +19,10 @@ import { itemsRawTable } from '../../../../../drizzle/schema';
 import { assertIsDefined } from '../../../../../utils/assertions';
 import { H5P_LOCAL_CONFIG, H5P_PATH_PREFIX, TMP_FOLDER } from '../../../../../utils/config';
 import { H5PItem } from '../../../discrimination';
-import { ItemService } from '../../../item.service';
 import { HtmlImportError } from '../errors';
 import { H5P_FILE_DOT_EXTENSION } from './constants';
 import { H5PInvalidManifestError } from './errors';
+import { H5PService } from './h5p.service';
 import { H5P_PACKAGES } from './test/fixtures';
 import { expectH5PFiles, injectH5PImport } from './test/helpers';
 
@@ -302,9 +302,9 @@ describe('Service plugin', () => {
     });
     it('returns error and deletes extracted files on item creation failure', async () => {
       const { storageRootPath } = H5P_LOCAL_CONFIG.local;
-      const createItem = jest.spyOn(resolveDependency(ItemService), 'post');
-      createItem.mockImplementationOnce(() => {
-        throw new Error('mock error on create item');
+      const uploadPackage = jest.spyOn(resolveDependency(H5PService), 'uploadPackage');
+      uploadPackage.mockImplementationOnce(() => {
+        throw new Error('mock error on HTML package upload');
       });
 
       const {

src/services/item/plugins/html/h5p/h5p.controller.ts

@@ -122,7 +122,7 @@ const plugin: FastifyPluginAsyncTypebox<H5PPluginOptions> = async (fastify) => {
 
         const { filename, file: stream } = h5pFile;
 
-        return await h5pService.createH5PItem(
+        return await h5pService.uploadFileAndCreateItem(
           tx,
           member,
           filename,

src/services/item/plugins/html/h5p/h5p.service.ts

@@ -120,7 +120,25 @@ export class H5PService extends HtmlService {
     });
   }
 
-  async createH5PItem(
+  /**
+   * Upload an H5P file.
+   * @returns The H5P file metadata.
+   */
+  async uploadH5PFile(
+    dbConnection: DBConnection,
+    actor: MinimalMember,
+    filename: string,
+    stream: Readable,
+    log?: FastifyBaseLogger,
+  ) {
+    return await super.uploadFile(dbConnection, actor, filename, stream, log);
+  }
+
+  /**
+   * Upload the H5P file and create an item associated with it.
+   * @returns The newly created item.
+   */
+  async uploadFileAndCreateItem(
     dbConnection: DBConnection,
     actor: MinimalMember,
     filename: string,
@@ -129,48 +147,37 @@ export class H5PService extends HtmlService {
     previousItemId?: ItemRaw['id'],
     log?: FastifyBaseLogger,
   ): Promise<H5PItem> {
-    const item = await super.createItem(
+    const { remoteRootPath, baseName, contentId } = await this.uploadH5PFile(
       dbConnection,
       actor,
       filename,
       stream,
-      this.createItemForH5PFile,
-      parentId,
-      previousItemId,
       log,
     );
-    if (!isItemType(item, ItemType.H5P)) {
-      throw new Error('Expected item to be H5P but it was something else');
-    }
-    return item;
-  }
 
-  /**
-   * Creates a Graasp item for the uploaded H5P package
-   * @param filename Name of the original H5P file WITHOUT EXTENSION
-   * @param contentId Storage ID of the remote content
-   * @param remoteRootPath Root path on the remote storage
-   * @param member Actor member
-   * @param parentId Optional parent id of the newly created item
-   * !! it's important to keep this syntax because of the reference to this
-   */
-  private createItemForH5PFile = async (
-    dbConnection: DBConnection,
-    member: MinimalMember,
-    filename: string,
-    contentId: string,
-    parentId?: string,
-    previousItemId?: string,
-  ): Promise<ItemRaw> => {
     const metadata = {
-      name: this.buildH5PPath('', filename),
+      name: this.buildH5PPath('', baseName),
       type: ItemType.H5P,
-      extra: this.buildH5PExtra(contentId, filename),
+      extra: this.buildH5PExtra(contentId, baseName),
     };
-    return this.itemService.post(dbConnection, member, {
-      item: metadata,
-      parentId,
-      previousItemId,
-    });
-  };
+
+    try {
+      const item = await this.itemService.post(dbConnection, actor, {
+        item: metadata,
+        parentId,
+        previousItemId,
+      });
+
+      if (!isItemType(item, ItemType.H5P)) {
+        throw new Error('Expected item to be H5P but it was something else');
+      }
+      return item;
+    } catch (error) {
+      // delete storage folder of this html package if upload or creation fails
+      await this.fileService.deleteFolder(remoteRootPath);
+      // rethrow above
+      log?.error(error);
+      throw error;
+    }
+  }
 }

src/services/item/plugins/html/html.service.ts

@@ -112,7 +112,7 @@ export abstract class HtmlService {
    * are uploaded in parallel as soon as possible). Results aggregation can however
    * await in parallel (note: await in a map fn does not block the map iteration).
    */
-  async upload(
+  async uploadPackage(
     member: MinimalMember,
     folder: string,
     uploadPath: string,
@@ -128,7 +128,7 @@ export abstract class HtmlService {
 
       if (stats.isDirectory()) {
         // recursively upload child folder
-        return await this.upload(member, childPath, childUploadPath, log);
+        return await this.uploadPackage(member, childPath, childUploadPath, log);
       } else {
         // ignore this file if extension is not allowed
         const ext = path.extname(childPath);
@@ -154,24 +154,16 @@ export abstract class HtmlService {
     return (await Promise.all(uploads)).flat();
   }
 
-  async createItem(
+  /**
+   * Upload the HTML file and get the metadata of the file.
+   */
+  async uploadFile(
     dbConnection: DBConnection,
     actor: MinimalMember,
     filename: string,
     stream: Readable,
-    onComplete: (
-      dbConnection: DBConnection,
-      actor: MinimalMember,
-      baseName: string,
-      contentId: string,
-      parentId?: ItemRaw['id'],
-      previousItemId?: ItemRaw['id'],
-      log?: FastifyBaseLogger,
-    ) => Promise<ItemRaw>,
-    parentId?: ItemRaw['id'],
-    previousItemId?: ItemRaw['id'],
     log?: FastifyBaseLogger,
-  ): Promise<ItemRaw> {
+  ): Promise<{ remoteRootPath: string; baseName: string; contentId: string }> {
     // check member storage limit
     await this.storageService.checkRemainingStorage(dbConnection, actor);
     const contentId = v4();
@@ -197,17 +189,8 @@ export abstract class HtmlService {
       // try-catch block for remote storage cleanup
       try {
         // upload whole folder to public storage
-        await this.upload(actor, targetFolder, remoteRootPath, log);
-        const item = await onComplete(
-          dbConnection,
-          actor,
-          baseName,
-          contentId,
-          parentId,
-          previousItemId,
-          log,
-        );
-        return item;
+        await this.uploadPackage(actor, targetFolder, remoteRootPath, log);
+        return { remoteRootPath, baseName, contentId };
       } catch (error) {
         // delete storage folder of this html package if upload or creation fails
         await this.fileService.deleteFolder(remoteRootPath);

src/services/item/plugins/importExport/service.ts

@@ -301,6 +301,20 @@ export class ImportExportService {
           thumbnail = createReadStream(itemThumbnailPath);
         }
 
+        // Handle the H5P file upload
+        if (item.type === ItemType.H5P) {
+          const pathToGraaspFile = path.join(folderPath, item.id);
+          const h5pFileStream = createReadStream(pathToGraaspFile);
+          const h5pFileInfo = await this.h5pService.uploadH5PFile(
+            dbConnection,
+            actor,
+            item.id,
+            h5pFileStream,
+          );
+
+          extra = h5pFileInfo;
+        }
+
         // Handle the file upload
         if (item.type === ItemType.FILE) {
           if (!item.mimetype) {

src/services/item/plugins/importExport/specifications.md

@@ -35,8 +35,8 @@ The `description` field for all items is sanitized before item creation.
 - `FOLDER` - The children are recursively imported, if present.
 - `LINK` - Not currently supported.
 - `FILE` - The file is imported with the same procedure as in the raw file import. The item is then created with the `extra` field based on the properties extracted from the file.
+- `H5P` - The file is imported with the same procedure as the normal H5P upload. The `extra` field in the item is based on the properties extracted from the H5P file.
 - `SHORTCUT` - Not currently supported.
-- `H5P` - Not currently supported.
 - `ETHERPAD` - Not currently supported.
 
 # Raw ZIP Import

src/services/item/plugins/importExport/test/fixtures/graasp-archive.zip

@@ -425,6 +425,7 @@ describe('ZIP routes tests', () => {
       const pdfName = 'output.pdf';
       const pdfSize = 10024;
       const pdfContent = 'This is a real document.';
+      const h5pFilename = 'test-05-collage-54.h5p';
 
       // Create the actor and the parent item
       const {
@@ -457,12 +458,14 @@ describe('ZIP routes tests', () => {
         const folderItem = itemsInDB[0];
         const documentItem = itemsInDB[1];
         const fileItem = itemsInDB[2];
+        const h5pItem = itemsInDB[3];
 
         // Check that all the items have been imported and that their order is correct
-        expect(itemsInDB.length).toEqual(3);
+        expect(itemsInDB.length).toEqual(4);
         expect(folderItem.type).toEqual(ItemType.FOLDER);
         expect(documentItem.type).toEqual(ItemType.DOCUMENT);
         expect(fileItem.type).toEqual(ItemType.FILE);
+        expect(h5pItem.type).toEqual(ItemType.H5P);
         expect(Number(itemsInDB[1].order)).toBeLessThan(Number(itemsInDB[2].order));
 
         // Check that all the item properties have been assigned for the document type
@@ -489,6 +492,8 @@ describe('ZIP routes tests', () => {
         });
 
         expect(thumbnailURLResponse.statusCode).toBe(StatusCodes.OK);
+
+        expect(h5pItem.name).toEqual(h5pFilename);
       });
     });
   });

src/services/item/plugins/importExport/test/index.test.ts

@@ -3,6 +3,7 @@ import sharp from 'sharp';
 import { Readable } from 'stream';
 import { injectable } from 'tsyringe';
 
+import { BaseLogger } from '../../logger';
 import { AuthenticatedUser } from '../../types';
 import FileService from '../file/file.service';
 import { THUMBNAIL_FORMAT, THUMBNAIL_MIMETYPE, ThumbnailSizeFormat } from './constants';
@@ -13,10 +14,12 @@ export const ITEM_THUMBNAIL_PREFIX = 'thumbnails';
 @injectable()
 export class ThumbnailService {
   private readonly fileService: FileService;
+  private readonly logger: BaseLogger;
   private _prefix: string = ITEM_THUMBNAIL_PREFIX;
 
-  constructor(fileService: FileService) {
+  constructor(fileService: FileService, log: BaseLogger) {
     this.fileService = fileService;
+    this.logger = log;
   }
 
   public set prefix(prefix: string) {
@@ -33,7 +36,7 @@ export class ThumbnailService {
 
   async upload(authenticatedUser: AuthenticatedUser, id: string, file: Readable) {
     // upload all thumbnails in parallel
-    await Promise.all(
+    const filesToUpload = await Promise.all(
       Object.entries(ThumbnailSizeFormat).map(async ([sizeName, width]) => {
         // create thumbnail from image stream
         const pipeline = sharp().resize({ width }).toFormat(THUMBNAIL_FORMAT);
@@ -47,14 +50,20 @@ export class ThumbnailService {
             .on('error', reject),
         );
 
-        // upload file
-        await this.fileService.upload(authenticatedUser, {
+        return {
           file: pipeline,
           filepath: this.buildFilePath(id, sizeName),
           mimetype: THUMBNAIL_MIMETYPE,
-        });
+        };
       }),
     );
+
+    // upload the thumbnails
+    try {
+      await this.fileService.uploadMany(authenticatedUser, filesToUpload);
+    } catch (_err) {
+      this.logger.debug(`Could not upload the ${id} item thumbnails`);
+    }
   }
 
   async getUrl({ id, size }: { size: string; id: string }) {