Skip to content

PER-10200 Web app display error on multi part uploads #574

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 62 additions & 0 deletions src/app/core/services/upload/uploader.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,17 @@ class MockApiService {
public static gotPresigned: boolean = false;
public static registeredRecord: RecordVO = null;
public static registeredDestination: string = null;
public static multipartRegistered: {
record: RecordVO;
uploadId: string;
key: string;
eTags: string[];
} = null;
public static reset() {
MockApiService.gotPresigned = false;
MockApiService.registeredRecord = null;
MockApiService.registeredDestination = null;
MockApiService.multipartRegistered = null;
}
public record = {
async registerRecord(record: RecordVO, url: string) {
Expand Down Expand Up @@ -53,6 +60,30 @@ class MockApiService {
],
});
},

async getMultipartUploadURLs(size: number) {
const partSize = 10 * 1024 * 1024;
const partCount = Math.ceil(size / partSize);
return {
urls: Array(partCount)
.fill(null)
.map((_, i) => `multipart-url-${i}`),
uploadId: 'test-upload-id',
key: 'test-file-key',
};
},
async registerMultipartRecord(
record: RecordVO,
uploadId: string,
key: string,
eTags: string[],
) {
MockApiService.multipartRegistered = { record, uploadId, key, eTags };
return new RecordVO({
displayName: 'multipart.txt',
parentFolderId: record.parentFolderId,
});
},
};
}

Expand Down Expand Up @@ -97,4 +128,35 @@ describe('Uploader', () => {
expect(MockApiService.registeredRecord.parentFolderId).toBe(1);
expect(MockApiService.registeredDestination).toBe('testurl');
});

it('can do a multipart upload using MockApiService', async () => {
const file = new File([new Uint8Array(200 * 1024 * 1024)], 'multipart.txt');
const uploadItem = new UploadItem(
file,
new FolderVO({ folderId: 2, folder_linkId: 2 }),
);

const progressSpy = jasmine.createSpy();

spyOn<any>(uploader, 'uploadToMultipartUrl').and.callFake(
async (
_url: string,
_item: UploadItem,
_pointer: number,
eTags: string[],
) => {
eTags.push('etag-mock');
},
);

const result = await (uploader as any).uploadMultipart(
uploadItem,
progressSpy,
);

expect(MockApiService.multipartRegistered.record.parentFolderId).toBe(2);
expect(MockApiService.multipartRegistered.eTags.length).toBe(20);
expect(progressSpy).toHaveBeenCalled();
expect(result.displayName).toBe('multipart.txt');
});
});
7 changes: 3 additions & 4 deletions src/app/core/services/upload/uploader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { HttpClient, HttpEvent, HttpEventType } from '@angular/common/http';
import { Injectable } from '@angular/core';
import { ApiService } from '@shared/services/api/api.service';
import { EventService } from '@shared/services/event/event.service';
import { RecordVO } from '@models/index';
import { UploadItem } from './uploadItem';

const buildForm = (fields: object, file: File) => {
Expand Down Expand Up @@ -125,22 +126,20 @@ export class Uploader {
emitProgress(progress);
}

const response = await this.api.record.registerMultipartRecord(
const record = await this.api.record.registerMultipartRecord(
item.RecordVO,
uploadId,
key,
eTags,
);

const record = response.getRecordVO();

this.event.dispatch({
action: 'submit',
entity: 'record',
record,
});

return response;
return record;
};

async uploadFile(
Expand Down
34 changes: 18 additions & 16 deletions src/app/shared/services/api/record.repo.ts
Original file line number Diff line number Diff line change
Expand Up @@ -138,23 +138,25 @@
uploadId: string,
key: string,
eTags: string[],
): Promise<RecordResponse> {
): Promise<RecordVO> {
const body = {

Check warning on line 142 in src/app/shared/services/api/record.repo.ts

View check run for this annotation

Codecov / codecov/patch

src/app/shared/services/api/record.repo.ts#L142

Added line #L142 was not covered by tests
displayName: record.displayName,
parentFolderId: record.parentFolderId,
uploadFileName: record.uploadFileName,
size: record.size,
multipartUploadData: {
uploadId,
key,
parts: eTags.map((ETag, index) => ({

Check warning on line 150 in src/app/shared/services/api/record.repo.ts

View check run for this annotation

Codecov / codecov/patch

src/app/shared/services/api/record.repo.ts#L150

Added line #L150 was not covered by tests
PartNumber: index + 1,
ETag,
})),
},
};

return getFirst(
this.httpV2.post('/record/registerRecord', {
displayName: record.displayName,
parentFolderId: record.parentFolderId,
uploadFileName: record.uploadFileName,
size: record.size,
multipartUploadData: {
uploadId,
key,
parts: eTags.map((ETag, index) => ({
PartNumber: index + 1,
ETag,
})),
},
}),
).toPromise() as unknown as RecordResponse;
this.httpV2.post<RecordVO>('/record/registerRecord', body),
).toPromise();
}

public async update(recordVOs: RecordVO[], archiveId: number) {
Expand Down