Skip to content

Commit

Permalink
last review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
MajedAlaitwniCap committed Oct 27, 2023
1 parent 9f66fd9 commit aa9bd5b
Show file tree
Hide file tree
Showing 20 changed files with 117 additions and 80 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import {
} from '@shared/testing';
import { ObjectID } from 'bson';
import { Readable } from 'stream';
import { H5PContent, H5PContentParentType, IH5PContentProperties, BaseEntityWithTimestamp } from '../../entity';
import { H5PContent, H5PContentParentType, IH5PContentProperties, H5pEditorTempFile } from '../../entity';
import { H5PEditorTestModule } from '../../h5p-editor-test.module';
import { H5P_CONTENT_S3_CONNECTION, H5P_LIBRARIES_S3_CONNECTION } from '../../h5p-editor.config';
import { ContentStorage, LibraryStorage, TemporaryFileStorage } from '../../service';
Expand Down Expand Up @@ -270,7 +270,7 @@ describe('H5PEditor Controller (api)', () => {
content: 'File Content',
};

const mockTempFile = new BaseEntityWithTimestamp({
const mockTempFile = new H5pEditorTempFile({
filename: mockFile.name,
ownedByUserId: studentUser.id,
expiresAt: new Date(),
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { ContentParameters, IContentMetadata, IEditorModel, IIntegration } from '@lumieducation/h5p-server';
import { ApiProperty } from '@nestjs/swagger';
import { Readable } from 'stream';

export class H5PEditorModelResponse {
constructor(editorModel: IEditorModel) {
Expand All @@ -20,6 +21,15 @@ export class H5PEditorModelResponse {
styles: string[];
}

export interface GetH5PFileResponse {
data: Readable;
etag?: string;
contentType?: string;
contentLength?: number;
contentRange?: string;
name: string;
}

interface H5PContentResponse {
h5p: IContentMetadata;
library: string;
Expand Down Expand Up @@ -73,3 +83,12 @@ export class H5PSaveResponse {
@ApiProperty({ type: H5PContentMetadata })
metadata!: H5PContentMetadata;
}

export interface GetFileResponse {
data: Readable;
etag?: string;
contentType?: string;
contentLength?: number;
contentRange?: string;
name: string;
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { Readable } from 'stream';
import { File } from '@shared/infra/s3-client';

export class H5pFileDto {
export class H5pFileDto implements File {
constructor(file: H5pFileDto) {
this.name = file.name;
this.data = file.data;
Expand Down
2 changes: 2 additions & 0 deletions apps/server/src/modules/h5p-editor/controller/dto/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,5 @@ export * from './ajax';
export * from './content-file.url.params';
export * from './h5p-editor.params';
export * from './library-file.url.params';
export * from './h5p-file.dto';
export * from './h5p-editor.response';
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import {
import { FileFieldsInterceptor } from '@nestjs/platform-express';
import { ApiOperation, ApiResponse, ApiTags } from '@nestjs/swagger';
import { ApiValidationError } from '@shared/common';
import { ICurrentUser, CurrentUser } from '@src/modules/authentication';
import { ICurrentUser, CurrentUser } from '@modules/authentication';
import { Request, Response } from 'express';
import { Authenticate } from '@modules/authentication/decorator/auth.decorator';

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { Entity, Property } from '@mikro-orm/core';
import { ITemporaryFile, IFileStats } from '@lumieducation/h5p-server';
import { BaseEntity } from '@shared/domain';
import { BaseEntityWithTimestamps } from '@shared/domain';

export interface ITemporaryFileProperties {
filename: string;
Expand All @@ -11,7 +11,7 @@ export interface ITemporaryFileProperties {
}

@Entity({ tableName: 'h5p-editor-temp-file' })
export class BaseEntityWithTimestamp extends BaseEntity implements ITemporaryFile, IFileStats {
export class H5pEditorTempFile extends BaseEntityWithTimestamps implements ITemporaryFile, IFileStats {
/**
* The name by which the file can be identified; can be a path including subdirectories (e.g. 'images/xyz.png')
*/
Expand Down
4 changes: 2 additions & 2 deletions apps/server/src/modules/h5p-editor/h5p-editor.module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import { UserModule } from '@modules/user';
import { S3ClientModule } from '@shared/infra/s3-client';
import { AuthenticationModule } from '../authentication/authentication.module';
import { H5PEditorController } from './controller/h5p-editor.controller';
import { H5PContent, InstalledLibrary, BaseEntityWithTimestamp } from './entity';
import { H5PContent, InstalledLibrary, H5pEditorTempFile } from './entity';
import { config, s3ConfigContent, s3ConfigLibraries } from './h5p-editor.config';
import { H5PContentRepo, LibraryRepo, TemporaryFileRepo } from './repo';
import { ContentStorage, LibraryStorage, TemporaryFileStorage } from './service';
Expand All @@ -39,7 +39,7 @@ const imports = [
password: DB_PASSWORD,
user: DB_USERNAME,
// Needs ALL_ENTITIES for authorization
entities: [...ALL_ENTITIES, H5PContent, BaseEntityWithTimestamp, InstalledLibrary],
entities: [...ALL_ENTITIES, H5PContent, H5pEditorTempFile, InstalledLibrary],
}),
ConfigModule.forRoot(createConfigModuleOptions(config)),
S3ClientModule.register([s3ConfigContent, s3ConfigLibraries]),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,7 @@ import { H5PEditor, cacheImplementations } from '@lumieducation/h5p-server';

import { IH5PEditorOptions, ITranslationFunction } from '@lumieducation/h5p-server/build/src/types';
import { h5pConfig, h5pUrlGenerator } from '../service/config/h5p-service-config';
import { ContentStorage } from '../service/contentStorage.service';
import { Translator } from '../service/h5p-translator.service';
import { LibraryStorage } from '../service/libraryStorage.service';
import { TemporaryFileStorage } from '../service/temporary-file-storage.service';
import { ContentStorage, Translator, LibraryStorage, TemporaryFileStorage } from '../service';

export const H5PEditorProvider = {
provide: H5PEditor,
Expand All @@ -21,6 +18,7 @@ export const H5PEditorProvider = {
enableHubLocalization: true,
enableLibraryNameLocalization: true,
};
// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
const translationFunction: ITranslationFunction = await Translator.translate();
const h5pEditor = new H5PEditor(
cache,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { EntityManager } from '@mikro-orm/mongodb';
import { Test, TestingModule } from '@nestjs/testing';
import { MongoMemoryDatabaseModule } from '@shared/infra/database';
import { cleanupCollections, h5pTemporaryFileFactory } from '@shared/testing';
import { BaseEntityWithTimestamp } from '../entity';
import { H5pEditorTempFile } from '../entity';
import { TemporaryFileRepo } from './temporary-file.repo';

describe('TemporaryFileRepo', () => {
Expand All @@ -12,7 +12,7 @@ describe('TemporaryFileRepo', () => {

beforeAll(async () => {
module = await Test.createTestingModule({
imports: [MongoMemoryDatabaseModule.forRoot({ entities: [BaseEntityWithTimestamp] })],
imports: [MongoMemoryDatabaseModule.forRoot({ entities: [H5pEditorTempFile] })],
providers: [TemporaryFileRepo],
}).compile();

Expand All @@ -29,7 +29,7 @@ describe('TemporaryFileRepo', () => {
});

it('should implement entityName getter', () => {
expect(repo.entityName).toBe(BaseEntityWithTimestamp);
expect(repo.entityName).toBe(H5pEditorTempFile);
});

describe('createTemporaryFile', () => {
Expand Down
16 changes: 8 additions & 8 deletions apps/server/src/modules/h5p-editor/repo/temporary-file.repo.ts
Original file line number Diff line number Diff line change
@@ -1,32 +1,32 @@
import { Injectable } from '@nestjs/common';
import { EntityId } from '@shared/domain';
import { BaseRepo } from '@shared/repo/base.repo';
import { BaseEntityWithTimestamp } from '../entity';
import { H5pEditorTempFile } from '../entity';

@Injectable()
export class TemporaryFileRepo extends BaseRepo<BaseEntityWithTimestamp> {
export class TemporaryFileRepo extends BaseRepo<H5pEditorTempFile> {
get entityName() {
return BaseEntityWithTimestamp;
return H5pEditorTempFile;
}

async findByUserAndFilename(userId: EntityId, filename: string): Promise<BaseEntityWithTimestamp> {
async findByUserAndFilename(userId: EntityId, filename: string): Promise<H5pEditorTempFile> {
return this._em.findOneOrFail(this.entityName, { ownedByUserId: userId, filename });
}

async findAllByUserAndFilename(userId: EntityId, filename: string): Promise<BaseEntityWithTimestamp[]> {
async findAllByUserAndFilename(userId: EntityId, filename: string): Promise<H5pEditorTempFile[]> {
return this._em.find(this.entityName, { ownedByUserId: userId, filename });
}

async findExpired(): Promise<BaseEntityWithTimestamp[]> {
async findExpired(): Promise<H5pEditorTempFile[]> {
const now = new Date();
return this._em.find(this.entityName, { expiresAt: { $lt: now } });
}

async findByUser(userId: EntityId): Promise<BaseEntityWithTimestamp[]> {
async findByUser(userId: EntityId): Promise<H5pEditorTempFile[]> {
return this._em.find(this.entityName, { ownedByUserId: userId });
}

async findExpiredByUser(userId: EntityId): Promise<BaseEntityWithTimestamp[]> {
async findExpiredByUser(userId: EntityId): Promise<H5pEditorTempFile[]> {
const now = new Date();
return this._em.find(this.entityName, { $and: [{ ownedByUserId: userId }, { expiresAt: { $lt: now } }] });
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@ import { HttpException, InternalServerErrorException, NotFoundException } from '
import { Test, TestingModule } from '@nestjs/testing';
import { IEntity } from '@shared/domain';
import { S3ClientAdapter } from '@shared/infra/s3-client';
import { GetFileResponse } from '@src/modules/files-storage/interface';
import { ObjectID } from 'bson';
import { Readable } from 'stream';
import { GetH5PFileResponse } from '../controller/dto';
import { H5PContent, H5PContentParentType, IH5PContentProperties } from '../entity';
import { H5P_CONTENT_S3_CONNECTION } from '../h5p-editor.config';
import { H5PContentRepo } from '../repo';
Expand Down Expand Up @@ -497,7 +497,8 @@ describe('ContentStorage', () => {
describe('WHEN file does not exist', () => {
it('should return false', async () => {
const { contentID, filename } = setup();
s3ClientAdapter.head.mockRejectedValueOnce(new NotFoundException('NoSuchKey'));
// s3ClientAdapter.head.mockRejectedValueOnce(new NotFoundException('NoSuchKey'));
s3ClientAdapter.get.mockRejectedValue(new NotFoundException('NoSuchKey'));

const exists = await service.fileExists(contentID, filename);

Expand All @@ -508,7 +509,7 @@ describe('ContentStorage', () => {
describe('WHEN S3ClientAdapter.head throws error', () => {
it('should throw HttpException', async () => {
const { contentID, filename } = setup();
s3ClientAdapter.head.mockRejectedValueOnce(new Error());
s3ClientAdapter.get.mockRejectedValueOnce(new Error());

const existsPromise = service.fileExists(contentID, filename);

Expand Down Expand Up @@ -623,7 +624,7 @@ describe('ContentStorage', () => {
const filename = 'testfile.txt';
const fileStream = Readable.from('content');
const contentID = new ObjectID().toString();
const fileResponse = createMock<GetFileResponse>({ data: fileStream });
const fileResponse = createMock<GetH5PFileResponse>({ data: fileStream });
const user = helpers.createUser();

const getError = new Error('Could not get file');
Expand Down
Loading

0 comments on commit aa9bd5b

Please sign in to comment.