🐛 #23 Fix throw error when file doesn't exist

* #23 Fix throw error when file doesn't exist
Run all the tests in once without special runner
Add positive scenario
Move coverage to the backend build workflow
This commit is contained in:
Anton Shepilov
2023-05-01 19:45:28 +02:00
committed by GitHub
parent 11c0c35eb2
commit fd03040d73
15 changed files with 121 additions and 82 deletions
+9 -6
View File
@@ -19,12 +19,15 @@ jobs:
runs-on: ubuntu-20.04 runs-on: ubuntu-20.04
steps: steps:
- uses: actions/checkout@v2 - uses: actions/checkout@v2
- name: build-test
run: cd tdrive && docker-compose -f docker-compose.tests.yml run -e NODE_OPTIONS=--unhandled-rejections=warn node npm run build
- name: unit-test
run: cd tdrive && docker-compose -f docker-compose.tests.yml run -e NODE_OPTIONS=--unhandled-rejections=warn node npm run test:unit
- name: e2e-mongo-test - name: e2e-mongo-test
run: cd tdrive && docker-compose -f docker-compose.tests.yml run -e NODE_OPTIONS=--unhandled-rejections=warn -e SEARCH_DRIVER=mongodb -e DB_DRIVER=mongodb -e PUBSUB_TYPE=local node npm run test:e2e run: cd tdrive && docker-compose -f docker-compose.tests.yml run -e NODE_OPTIONS=--unhandled-rejections=warn -e SEARCH_DRIVER=mongodb -e DB_DRIVER=mongodb -e PUBSUB_TYPE=local node npm run test:all
- name: e2e-cassandra-test - name: e2e-cassandra-test
run: cd tdrive && docker-compose -f docker-compose.tests.yml up -d scylladb elasticsearch rabbitmq && sleep 60 && docker-compose -f docker-compose.tests.yml run -e NODE_OPTIONS=--unhandled-rejections=warn -e SEARCH_DRIVER=elasticsearch -e DB_DRIVER=cassandra node npm run test:e2e run: cd tdrive && docker-compose -f docker-compose.tests.yml up -d scylladb elasticsearch rabbitmq && sleep 60 && docker-compose -f docker-compose.tests.yml run -e NODE_OPTIONS=--unhandled-rejections=warn -e SEARCH_DRIVER=elasticsearch -e DB_DRIVER=cassandra node npm run test:all
- name: coverage
uses: adRise/jest-cov-reporter@main
with:
branch-coverage-report-path: ./tdrive/coverage/coverage-summary.json
base-coverage-report-path: ./tdrive/coverage/coverage-summary.json
delta: 0.3
fullCoverageDiff: true
-32
View File
@@ -1,32 +0,0 @@
name: backend-coverage
on:
pull_request_target:
types: [assigned, opened, synchronize, reopened]
branches: [main]
paths:
- "tdrive/backend/node/**"
jobs:
test:
runs-on: ubuntu-20.04
steps:
- uses: actions/checkout@v2
with:
ref: "refs/pull/${{ github.event.number }}/merge"
- name: unit-test
run: cd tdrive && docker-compose -f docker-compose.tests.yml run -e NODE_OPTIONS=--unhandled-rejections=warn node npm run test:unit
- name: e2e-mongo-test
run: cd tdrive && docker-compose -f docker-compose.tests.yml run -e NODE_OPTIONS=--unhandled-rejections=warn -e SEARCH_DRIVER=mongodb -e DB_DRIVER=mongodb -e PUBSUB_TYPE=local node npm run test:e2e
- name: generate coverage summary json
run: cd tdrive && docker-compose -f docker-compose.tests.yml run -e NODE_OPTIONS=--unhandled-rejections=warn node npm run test:merge:json
- name: generate coverage summary text
run: cd tdrive && docker-compose -f docker-compose.tests.yml run -e NODE_OPTIONS=--unhandled-rejections=warn node npm run test:merge:text
- name: Coverage
uses: adRise/jest-cov-reporter@main
with:
branch-coverage-report-path: ./tdrive/coverage/merged/coverage-summary.json
base-coverage-report-path: ./tdrive/coverage/merged/coverage-summary.json
delta: 0.3
fullCoverageDiff: true
+1
View File
@@ -28,6 +28,7 @@
"test:unit:watch": "npm run test:unit -- --watchAll --verbose false | pino-pretty", "test:unit:watch": "npm run test:unit -- --watchAll --verbose false | pino-pretty",
"test:merge:json": "npx istanbul report --dir coverage/merged --include 'coverage/**/coverage-final.json' json-summary", "test:merge:json": "npx istanbul report --dir coverage/merged --include 'coverage/**/coverage-final.json' json-summary",
"test:merge:text": "npx istanbul report --dir coverage/merged --include 'coverage/**/coverage-final.json' text > coverage/merged/coverage-report.txt", "test:merge:text": "npx istanbul report --dir coverage/merged --include 'coverage/**/coverage-final.json' text > coverage/merged/coverage-report.txt",
"test:all": "jest test --forceExit --coverage --detectOpenHandles --testTimeout=30000 --verbose false --runInBand",
"kill": "kill $(lsof -t -i:3000) | exit 0" "kill": "kill $(lsof -t -i:3000) | exit 0"
}, },
"jest": { "jest": {
@@ -44,14 +44,13 @@ export class MongoConnector extends AbstractConnector<MongoConnectionOptions, mo
async getDatabase(): Promise<mongo.Db> { async getDatabase(): Promise<mongo.Db> {
await this.connect(); await this.connect();
return this.client.db(this.options.database); return this.client.db(this.options.database);
} }
async drop(): Promise<this> { async drop(): Promise<this> {
const db = await this.getDatabase(); const db = await this.getDatabase();
db.dropDatabase(); await db.dropDatabase();
return this; return this;
} }
@@ -3,6 +3,7 @@ import { createWriteStream, createReadStream, existsSync, mkdirSync, statSync, r
import p from "path"; import p from "path";
import { rm } from "fs/promises"; // Do not change the import, this is not the same function import { rm } from "fs" import { rm } from "fs/promises"; // Do not change the import, this is not the same function import { rm } from "fs"
import { StorageConnectorAPI, WriteMetadata } from "../../provider"; import { StorageConnectorAPI, WriteMetadata } from "../../provider";
import fs from "fs";
export type LocalConfiguration = { export type LocalConfiguration = {
path: string; path: string;
@@ -42,7 +43,11 @@ export default class LocalConnectorService implements StorageConnectorAPI {
} }
async read(path: string): Promise<Readable> { async read(path: string): Promise<Readable> {
return createReadStream(this.getFullPath(path)); const fullPath = this.getFullPath(path);
if (!fs.existsSync(fullPath)) {
throw new Error("File doesn't not exists");
}
return createReadStream(fullPath);
} }
async remove(path: string): Promise<boolean> { async remove(path: string): Promise<boolean> {
@@ -73,7 +73,7 @@ export default class StorageService extends TdriveService<StorageAPI> implements
return await this.getConnector().write(path, stream); return await this.getConnector().write(path, stream);
} catch (err) { } catch (err) {
logger.error(err); logger.error(err);
return null; throw err;
} }
} }
@@ -107,8 +107,7 @@ export default class StorageService extends TdriveService<StorageAPI> implements
} }
} catch (err) { } catch (err) {
logger.error(err); logger.error(err);
callback(); callback(err, null);
return;
} }
callback(null, stream); callback(null, stream);
return; return;
@@ -117,7 +116,7 @@ export default class StorageService extends TdriveService<StorageAPI> implements
return new Multistream(factory); return new Multistream(factory);
} catch (err) { } catch (err) {
logger.error(err); logger.error(err);
return null; throw err;
} }
} }
@@ -67,7 +67,7 @@ export class FileServiceImpl {
entity.application_id = applicationId; entity.application_id = applicationId;
entity.upload_data = null; entity.upload_data = null;
this.repository.save(entity, context); await this.repository.save(entity, context);
} }
if (file) { if (file) {
@@ -89,7 +89,7 @@ export class FileServiceImpl {
size: options.totalSize, size: options.totalSize,
chunks: options.totalChunks || 1, chunks: options.totalChunks || 1,
}; };
this.repository.save(entity, context); await this.repository.save(entity, context);
} }
} }
@@ -109,7 +109,9 @@ export class FileServiceImpl {
} }
} }
return entity; return await this.getFile({ id: entity.id, company_id: entity.company_id }, context, {
waitForThumbnail: options.waitForThumbnail,
});
} }
async exists(id: string, companyId: string, context?: CompanyExecutionContext): Promise<boolean> { async exists(id: string, companyId: string, context?: CompanyExecutionContext): Promise<boolean> {
@@ -44,13 +44,18 @@ export class FileController {
): Promise<void> { ): Promise<void> {
const context = getCompanyExecutionContext(request); const context = getCompanyExecutionContext(request);
const params = request.params; const params = request.params;
const data = await gr.services.files.download(params.id, context); try {
const filename = data.name.replace(/[^a-zA-Z0-9 -_.]/g, ""); const data = await gr.services.files.download(params.id, context);
const filename = data.name.replace(/[^a-zA-Z0-9 -_.]/g, "");
response.header("Content-disposition", `attachment; filename="${filename}"`); response.header("Content-disposition", `attachment; filename="${filename}"`);
if (data.size) response.header("Content-Length", data.size); if (data.size) response.header("Content-Length", data.size);
response.type(data.mime); response.type(data.mime);
response.send(data.file); response.send(data.file);
} catch (e) {
console.log("!!!" + e);
throw e;
}
} }
async thumbnail( async thumbnail(
+3
View File
@@ -50,6 +50,9 @@ export async function formatUser(
const companies = await Promise.all( const companies = await Promise.all(
userCompanies.map(async uc => { userCompanies.map(async uc => {
const company = await gr.services.companies.getCompany({ id: uc.group_id }); const company = await gr.services.companies.getCompany({ id: uc.group_id });
if (!company) {
throw new Error(`Company with id ${uc.group_id} doesn't exists!`);
}
return { return {
role: uc.role as CompanyUserRole, role: uc.role as CompanyUserRole,
status: "active" as CompanyUserStatus, // FIXME: with real status status: "active" as CompanyUserStatus, // FIXME: with real status
@@ -8,15 +8,18 @@ import fs from "fs";
import { File } from "../../../src/services/files/entities/file"; import { File } from "../../../src/services/files/entities/file";
import { deserialize } from "class-transformer"; import { deserialize } from "class-transformer";
import formAutoContent from "form-auto-content"; import formAutoContent from "form-auto-content";
import LocalConnectorService from "../../../src/core/platform/services/storage/connectors/local/service";
describe.skip("The Files feature", () => {
describe("The Files feature", () => {
const url = "/internal/services/files/v1"; const url = "/internal/services/files/v1";
let platform: TestPlatform; let platform: TestPlatform;
beforeAll(async () => { beforeAll(async () => {
platform = await init({ platform = await init({
services: ["webserver", "database", "storage", "message-queue", "files", "previews"], services: ["webserver", "database", "storage", "files", "previews"],
}); });
await platform.database.getConnector().init();
}); });
afterAll(async done => { afterAll(async done => {
@@ -25,6 +28,22 @@ describe.skip("The Files feature", () => {
done(); done();
}); });
async function uploadFile(file: string) {
const form = formAutoContent({file: fs.createReadStream(file)});
form.headers["authorization"] = `Bearer ${await platform.auth.getJWTToken()}`;
const filesUploadRaw = await platform.app.inject({
method: "POST",
url: `${url}/companies/${platform.workspace.company_id}/files?thumbnail_sync=1`,
...form,
});
const filesUpload: ResourceUpdateResponse<File> = deserialize(
ResourceUpdateResponse,
filesUploadRaw.body,
);
return filesUpload;
}
describe("On user send files", () => { describe("On user send files", () => {
const files = [ const files = [
"assets/sample.png", "assets/sample.png",
@@ -36,22 +55,49 @@ describe.skip("The Files feature", () => {
].map(p => `${__dirname}/${p}`); ].map(p => `${__dirname}/${p}`);
const thumbnails = [1, 1, 2, 5, 0, 1]; const thumbnails = [1, 1, 2, 5, 0, 1];
it("should save file and generate previews", async done => { it("Download file should return 500 if file doesn't exists", async () => {
//given file
const filesUpload = await uploadFile(files[0]);
expect(filesUpload.resource.id).toBeTruthy();
//clean files directory
expect(platform.storage.getConnector()).toBeInstanceOf(LocalConnectorService)
const path = (<LocalConnectorService>platform.storage.getConnector()).configuration.path;
fs.readdirSync(path).forEach(f => fs.rmSync(`${path}/${f}`, {recursive: true, force: true}));
//when try to download the file
const fileDownloadResponse = await platform.app.inject({
method: "GET",
url: `${url}/companies/${platform.workspace.company_id}/files/${filesUpload.resource.id}/download`,
});
//then file should be not found with 404 error and "File not found message"
expect(fileDownloadResponse).toBeTruthy();
expect(fileDownloadResponse.statusCode).toBe(500);
}, 120000);
it("Download file should return 200 if file exists", async () => {
//given file
const filesUpload = await uploadFile(files[0]);
expect(filesUpload.resource.id).toBeTruthy();
//clean files directory
expect(platform.storage.getConnector()).toBeInstanceOf(LocalConnectorService)
//when try to download the file
const fileDownloadResponse = await platform.app.inject({
method: "GET",
url: `${url}/companies/${platform.workspace.company_id}/files/${filesUpload.resource.id}/download`,
});
//then file should be not found with 404 error and "File not found message"
expect(fileDownloadResponse).toBeTruthy();
expect(fileDownloadResponse.statusCode).toBe(200);
}, 120000);
it.skip("should save file and generate previews", async done => {
for (const i in files) { for (const i in files) {
const file = files[i]; const file = files[i];
const form = formAutoContent({ file: fs.createReadStream(file) }); const filesUpload = await uploadFile(file);
form.headers["authorization"] = `Bearer ${await platform.auth.getJWTToken()}`;
const filesUploadRaw = await platform.app.inject({
method: "POST",
url: `${url}/companies/${platform.workspace.company_id}/files?thumbnail_sync=1`,
...form,
});
const filesUpload: ResourceUpdateResponse<File> = deserialize(
ResourceUpdateResponse,
filesUploadRaw.body,
);
expect(filesUpload.resource.id).not.toBeFalsy(); expect(filesUpload.resource.id).not.toBeFalsy();
expect(filesUpload.resource.encryption_key).toBeFalsy(); //This must not be disclosed expect(filesUpload.resource.encryption_key).toBeFalsy(); //This must not be disclosed
@@ -59,6 +105,7 @@ describe.skip("The Files feature", () => {
for (const thumb of filesUpload.resource.thumbnails) { for (const thumb of filesUpload.resource.thumbnails) {
const thumbnails = await platform.app.inject({ const thumbnails = await platform.app.inject({
headers: {"authorization": `Bearer ${await platform.auth.getJWTToken()}`},
method: "GET", method: "GET",
url: `${url}/companies/${platform.workspace.company_id}/files/${filesUpload.resource.id}/thumbnails/${thumb.index}`, url: `${url}/companies/${platform.workspace.company_id}/files/${filesUpload.resource.id}/thumbnails/${thumb.index}`,
}); });
@@ -67,6 +114,7 @@ describe.skip("The Files feature", () => {
} }
done(); done();
}, 120000); }, 1200000);
}); });
}); });
+9 -1
View File
@@ -11,6 +11,8 @@ import { MessageQueueServiceAPI } from "../../../src/core/platform/services/mess
// @ts-ignore // @ts-ignore
import config from "config"; import config from "config";
import globalResolver from "../../../src/services/global-resolver"; import globalResolver from "../../../src/services/global-resolver";
import {FileServiceImpl} from "../../../src/services/files/services";
import StorageAPI from "../../../src/core/platform/services/storage/provider";
type TokenPayload = { type TokenPayload = {
sub: string; sub: string;
@@ -32,8 +34,10 @@ export interface TestPlatform {
workspace: Workspace; workspace: Workspace;
app: FastifyInstance; app: FastifyInstance;
database: DatabaseServiceAPI; database: DatabaseServiceAPI;
storage: StorageAPI;
messageQueue: MessageQueueServiceAPI; messageQueue: MessageQueueServiceAPI;
authService: AuthServiceAPI; authService: AuthServiceAPI;
filesService: FileServiceImpl;
auth: { auth: {
getJWTToken(payload?: TokenPayload): Promise<string>; getJWTToken(payload?: TokenPayload): Promise<string>;
}; };
@@ -68,17 +72,21 @@ export async function init(
await platform.start(); await platform.start();
const database = platform.getProvider<DatabaseServiceAPI>("database"); const database = platform.getProvider<DatabaseServiceAPI>("database");
await database.getConnector().drop();
const messageQueue = platform.getProvider<MessageQueueServiceAPI>("message-queue"); const messageQueue = platform.getProvider<MessageQueueServiceAPI>("message-queue");
const auth = platform.getProvider<AuthServiceAPI>("auth"); const auth = platform.getProvider<AuthServiceAPI>("auth");
const storage: StorageAPI = platform.getProvider<StorageAPI>("storage");
testPlatform = { testPlatform = {
platform, platform,
app, app,
messageQueue, messageQueue,
database, database,
storage,
workspace: { company_id: "", workspace_id: "" }, workspace: { company_id: "", workspace_id: "" },
currentUser: { id: "" }, currentUser: { id: "" },
authService: auth, authService: auth,
filesService: globalResolver.services.files,
auth: { auth: {
getJWTToken, getJWTToken,
}, },
@@ -104,7 +112,7 @@ export async function init(
payload.sub = testPlatform.currentUser.id; payload.sub = testPlatform.currentUser.id;
} }
if (testPlatform.currentUser.isWorkspaceModerator) { if (testPlatform .currentUser.isWorkspaceModerator) {
payload.org = {}; payload.org = {};
payload.org[testPlatform.workspace.company_id] = { payload.org[testPlatform.workspace.company_id] = {
role: "", role: "",
@@ -123,7 +123,7 @@ describe("The /users API", () => {
expect(resources.length).toBe(0); expect(resources.length).toBe(0);
done(); done();
}); }, 1200000);
}); });
async function search(search: string, companyId?: string): Promise<any[]> { async function search(search: string, companyId?: string): Promise<any[]> {
@@ -156,11 +156,13 @@ export class TestDbService {
} }
this.users.push(createdUser); this.users.push(createdUser);
await gr.services.companies.setUserRole( if (workspacesPk && workspacesPk.length) {
this.company ? this.company.id : workspacesPk[0].company_id, await gr.services.companies.setUserRole(
createdUser.id, this.company ? this.company.id : workspacesPk[0].company_id,
options.companyRole ? options.companyRole : "member", createdUser.id,
); options.companyRole ? options.companyRole : "member",
);
}
if (workspacesPk && workspacesPk.length) { if (workspacesPk && workspacesPk.length) {
for (const workspacePk of workspacesPk) { for (const workspacePk of workspacesPk) {
@@ -264,7 +264,6 @@ describe("The /workspace users API", () => {
const anotherUserId = testDbService.workspaces[0].users[0].id; const anotherUserId = testDbService.workspaces[0].users[0].id;
let workspaceUsersCount = await testDbService.getWorkspaceUsersCountFromDb(workspaceId); let workspaceUsersCount = await testDbService.getWorkspaceUsersCountFromDb(workspaceId);
let companyUsersCount = await testDbService.getCompanyUsersCountFromDb(companyId);
console.log(testDbService.workspaces[2].users); console.log(testDbService.workspaces[2].users);
console.log(workspaceUsersCount); console.log(workspaceUsersCount);
@@ -290,10 +289,7 @@ describe("The /workspace users API", () => {
checkUserObject(resource); checkUserObject(resource);
workspaceUsersCount = await testDbService.getWorkspaceUsersCountFromDb(workspaceId); workspaceUsersCount = await testDbService.getWorkspaceUsersCountFromDb(workspaceId);
companyUsersCount = await testDbService.getCompanyUsersCountFromDb(companyId);
expect(workspaceUsersCount).toBe(5); expect(workspaceUsersCount).toBe(5);
// expect(companyUsersCount).toBe(6);
done(); done();
}); });
}); });
@@ -35,7 +35,7 @@ describe("The /workspaces API", () => {
await platform.database.getConnector().init(); await platform.database.getConnector().init();
testDbService = new TestDbService(platform); testDbService = new TestDbService(platform);
await testDbService.createCompany(companyId); await testDbService.createCompany(companyId, "Company name");
const ws0pk = { id: uuidv1(), company_id: companyId }; const ws0pk = { id: uuidv1(), company_id: companyId };
const ws1pk = { id: uuidv1(), company_id: companyId }; const ws1pk = { id: uuidv1(), company_id: companyId };
const ws2pk = { id: uuidv1(), company_id: companyId }; const ws2pk = { id: uuidv1(), company_id: companyId };