diff --git a/packages/h5p-server/src/ContentStorer.ts b/packages/h5p-server/src/ContentStorer.ts index 5595d7703..b61a9ae33 100644 --- a/packages/h5p-server/src/ContentStorer.ts +++ b/packages/h5p-server/src/ContentStorer.ts @@ -236,12 +236,18 @@ export default class ContentStorer { ) ); + const filePathToNewFilenameMap = new Map(); for (const reference of fileReferencesInParams) { const filepath = path.join( packageDirectory, 'content', reference.filePath ); + let newFilename = filePathToNewFilenameMap.get(filepath); + if (newFilename) { + reference.context.params.path = `${newFilename}#tmp`; + continue; + } // If the file referenced in the parameters isn't included in the // package, we first check if the path is actually a URL and if not, // we delete the reference. @@ -257,12 +263,13 @@ export default class ContentStorer { continue; } const readStream = fsExtra.createReadStream(filepath); - const newFilename = await this.temporaryFileManager.addFile( + newFilename = await this.temporaryFileManager.addFile( reference.filePath, readStream, user ); reference.context.params.path = `${newFilename}#tmp`; + filePathToNewFilenameMap.set(filepath, newFilename); } return { metadata, parameters }; } @@ -487,6 +494,7 @@ export default class ContentStorer { oldFiles: string[] ): Promise { const filesToCopyFromTemporaryStorage: IFileReference[] = []; + const hashSet = new Set(); for (const ref of fileReferencesInNewParams) { // We mark the file to be copied over from temporary storage if the @@ -495,11 +503,15 @@ export default class ContentStorer { // We only save temporary file for later copying, however, if // the there isn't already a file with the exact name. This // might be the case if the user presses "save" twice. - if (!oldFiles.some((f) => f === ref.filePath)) { + if ( + !hashSet.has(ref.filePath) && + !oldFiles.some((f) => f === ref.filePath) + ) { filesToCopyFromTemporaryStorage.push(ref); } // remove temporary file marker from parameters ref.context.params.path = ref.filePath; + hashSet.add(ref.filePath); } } return filesToCopyFromTemporaryStorage; diff --git a/packages/h5p-server/test/H5PEditor.uploadingPackages.test.ts b/packages/h5p-server/test/H5PEditor.uploadingPackages.test.ts index affeb8fe6..f3df526ae 100644 --- a/packages/h5p-server/test/H5PEditor.uploadingPackages.test.ts +++ b/packages/h5p-server/test/H5PEditor.uploadingPackages.test.ts @@ -127,7 +127,7 @@ describe('H5PEditor', () => { await packageFinishedPromise; writeStream.close(); - // CHeck if filename remains short + // Check if filename remains short expect( /^earth-[0-9a-z]+\.jpg$/i.test(parameters?.image?.path) ).toBe(true); @@ -136,4 +136,119 @@ describe('H5PEditor', () => { { keep: false, unsafeCleanup: true } ); }); + + // context: activity contains 3 images; image1 uploaded, image2 + // copy/pasted from image1, image3 uploaded + it( + 'returns parameters containing items 1 and 2 referencing file 1, and ' + + 'item 3 referencing file 2', + async () => { + await withDir( + async ({ path: tempDirPath }) => { + const { h5pEditor, temporaryStorage } = + createH5PEditor(tempDirPath); + const user = new User(); + + const fileBuffer = fsExtra.readFileSync( + path.resolve('test/data/validator/valid3-3-images.h5p') + ); + const { parameters } = await h5pEditor.uploadPackage( + fileBuffer, + user + ); + + const tmpFilePath1 = + parameters.items[0].image.params.file.path; + const tmpFilePath2 = + parameters.items[1].image.params.file.path; + const tmpFilePath3 = + parameters.items[2].image.params.file.path; + expect(tmpFilePath1).toEqual(tmpFilePath2); + expect(tmpFilePath2).not.toEqual(tmpFilePath3); + + const files = await temporaryStorage.listFiles(user); + expect(files).toHaveLength(2); + + expect( + temporaryStorage.fileExists( + tmpFilePath1.substr(0, tmpFilePath1.length - 4), + user + ) + ).resolves.toEqual(true); + + expect( + temporaryStorage.fileExists( + tmpFilePath3.substr(0, tmpFilePath3.length - 4), + user + ) + ).resolves.toEqual(true); + }, + { keep: false, unsafeCleanup: true } + ); + } + ); + + // context: activity contains 3 images; image1 uploaded, image2 + // copy/pasted from image1, image3 uploaded + it('saves uploaded package as new content; 2 images saved to permanent storage', async () => { + await withDir( + async ({ path: tempDirPath }) => { + const { h5pEditor, contentStorage } = + createH5PEditor(tempDirPath); + const user = new User(); + + const fileBuffer = fsExtra.readFileSync( + path.resolve('test/data/validator/valid3-3-images.h5p') + ); + const { metadata, parameters } = await h5pEditor.uploadPackage( + fileBuffer, + user + ); + + const tmpFilePath1 = parameters.items[0].image.params.file.path; + const tmpFilePath3 = parameters.items[2].image.params.file.path; + + const contentId = await h5pEditor.saveOrUpdateContent( + undefined, + parameters, + metadata, + ContentMetadata.toUbername(metadata), + user + ); + + const files = await contentStorage.listFiles(contentId, user); + expect(files).toHaveLength(2); + + // get data we've stored and check if the #tmp tag has been removed from the image + const { params } = await h5pEditor.getContent(contentId, user); + + // FIRST IMAGE ======================= + const newFilename1 = tmpFilePath1.substr( + 0, + tmpFilePath1.length - 4 + ); + expect(params.params.items[0].image.params.file.path).toEqual( + newFilename1 + ); + // check if image is now in permanent storage + await expect( + contentStorage.fileExists(contentId, newFilename1) + ).resolves.toEqual(true); + + // SECOND IMAGE ======================= + const newFilename2 = tmpFilePath3.substr( + 0, + tmpFilePath3.length - 4 + ); + expect(params.params.items[2].image.params.file.path).toEqual( + newFilename2 + ); + // check if image is now in permanent storage + await expect( + contentStorage.fileExists(contentId, newFilename2) + ).resolves.toEqual(true); + }, + { keep: false, unsafeCleanup: true } + ); + }); }); diff --git a/test/data/validator/valid3-3-images.h5p b/test/data/validator/valid3-3-images.h5p new file mode 100644 index 000000000..6fb926281 Binary files /dev/null and b/test/data/validator/valid3-3-images.h5p differ