Skip to content
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

[ME]: Trying to upload the same file as a resource or attachment twice fails #1009

Merged
merged 1 commit into from
Sep 30, 2024
Merged
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
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,7 @@ import {
someUserFeedbacksFixture,
userFeedbackFixture,
} from '@geonetwork-ui/common/fixtures'
import {
HttpClientTestingModule,
HttpTestingController,
} from '@angular/common/http/testing'
import { HttpClientTestingModule } from '@angular/common/http/testing'
import { HttpClient, HttpEventType } from '@angular/common/http'

let geonetworkVersion: string
Expand Down Expand Up @@ -187,7 +184,11 @@ class RecordsApiServiceMock {
},
])
)
putResource = jest.fn(() => of(undefined))
putResource = jest.fn(() =>
of({
type: HttpEventType.UploadProgress,
})
)
}

class UserfeedbackApiServiceMock {
Expand Down Expand Up @@ -257,7 +258,6 @@ describe('Gn4PlatformService', () => {
registriesApiService = TestBed.inject(RegistriesApiService)
userFeedbackApiService = TestBed.inject(UserfeedbackApiService as any)
recordsApiService = TestBed.inject(RecordsApiService)
TestBed.inject(HttpTestingController)
})

it('creates', () => {
Expand Down Expand Up @@ -775,7 +775,8 @@ describe('Gn4PlatformService', () => {
file = new File([''], 'filename')
})
it('calls api service', async () => {
service.attachFileToRecord('12345', file)
await firstValueFrom(service.attachFileToRecord('12345', file))
expect(recordsApiService.getAllResources).toHaveBeenCalledWith('12345')
expect(recordsApiService.putResource).toHaveBeenCalledWith(
'12345',
file,
Expand All @@ -785,6 +786,13 @@ describe('Gn4PlatformService', () => {
true
)
})
it('disambiguates file name if an identical file already exists', async () => {
file = new File([''], 'doge.jpg')
await firstValueFrom(service.attachFileToRecord('12345', file))
const fileSent = (recordsApiService.putResource as jest.Mock).mock
.calls[0][1]
expect(fileSent.name).not.toEqual('doge.jpg')
})
it('handles progress event', () => {
;(recordsApiService.putResource as jest.Mock).mockReturnValue(
of({
Expand Down
87 changes: 57 additions & 30 deletions libs/api/repository/src/lib/gn4/platform/gn4-platform.service.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { Injectable } from '@angular/core'
import { combineLatest, Observable, of, switchMap } from 'rxjs'
import { combineLatest, Observable, of, switchMap, throwError } from 'rxjs'
import { catchError, filter, map, shareReplay, tap } from 'rxjs/operators'
import {
MeApiService,
Expand All @@ -10,7 +10,10 @@ import {
UserfeedbackApiService,
UsersApiService,
} from '@geonetwork-ui/data-access/gn4'
import { PlatformServiceInterface } from '@geonetwork-ui/common/domain/platform.service.interface'
import {
PlatformServiceInterface,
UploadEvent,
} from '@geonetwork-ui/common/domain/platform.service.interface'
import { UserModel } from '@geonetwork-ui/common/domain/model/user/user.model'
import {
Keyword,
Expand All @@ -26,6 +29,7 @@ import {
ThesaurusApiResponse,
} from '@geonetwork-ui/api/metadata-converter'
import { KeywordType } from '@geonetwork-ui/common/domain/model/thesaurus'
import { noDuplicateFileName } from '@geonetwork-ui/util/shared'

const minApiVersion = '4.2.2'

Expand Down Expand Up @@ -288,34 +292,57 @@ export class Gn4PlatformService implements PlatformServiceInterface {
)
}

attachFileToRecord(recordUuid: string, file: File) {
attachFileToRecord(recordUuid: string, file: File): Observable<UploadEvent> {
let sizeBytes = -1
return this.recordsApiService
.putResource(recordUuid, file, 'public', undefined, 'events', true)
.pipe(
map((event) => {
if (event.type === HttpEventType.UploadProgress) {
sizeBytes = event.total
return {
type: 'progress',
progress: event.total
? Math.round((100 * event.loaded) / event.total)
: 0,
} as const
}
if (event.type === HttpEventType.Response) {
return {
type: 'success',
attachment: {
url: new URL(event.body.url),
fileName: event.body.filename,
},
sizeBytes,
} as const
}
return undefined
}),
filter((event) => !!event)
)

// Check if the resource already exists on the server and rename it if that's the case
return this.getRecordAttachments(recordUuid).pipe(
map((recordAttachement) => recordAttachement.map((r) => r.fileName)),
switchMap((fileNames) => {
const fileName = noDuplicateFileName(file.name, fileNames)

const fileCopy = new File([file], fileName, { type: file.type })

return this.recordsApiService
.putResource(
recordUuid,
fileCopy,
'public',
undefined,
'events',
true
)
.pipe(
map((event) => {
if (event.type === HttpEventType.UploadProgress) {
sizeBytes = event.total
return {
type: 'progress',
progress: event.total
? Math.round((100 * event.loaded) / event.total)
: 0,
} as UploadEvent
}
if (event.type === HttpEventType.Response) {
return {
type: 'success',
attachment: {
url: new URL(event.body.url),
fileName: event.body.filename,
},
sizeBytes,
} as UploadEvent
}
return undefined
}),
filter((event) => !!event)
)
}),
catchError((error) => {
return throwError(
() => new Error(error.error?.message ?? error.message)
)
})
)
}
}
4 changes: 2 additions & 2 deletions libs/common/domain/src/lib/platform.service.interface.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,11 @@ import type { Organization } from './model/record/organization.model'
import { Keyword, UserFeedback } from './model/record'
import { KeywordType } from './model/thesaurus'

interface RecordAttachment {
export interface RecordAttachment {
url: URL
fileName: string
}
type UploadEvent =
export type UploadEvent =
| {
type: 'progress'
progress: number // in percent
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,19 +3,32 @@ import { FormFieldOnlineLinkResourcesComponent } from './form-field-online-link-
import { aSetOfLinksFixture } from '@geonetwork-ui/common/fixtures'
import { MockBuilder, MockProvider } from 'ng-mocks'
import { TranslateModule } from '@ngx-translate/core'
import { PlatformServiceInterface } from '@geonetwork-ui/common/domain/platform.service.interface'
import {
PlatformServiceInterface,
RecordAttachment,
} from '@geonetwork-ui/common/domain/platform.service.interface'
import { NotificationsService } from '@geonetwork-ui/feature/notifications'
import { Subject } from 'rxjs'
import { BehaviorSubject, Subject } from 'rxjs'
import { MatDialog, MatDialogRef } from '@angular/material/dialog'
import { OnlineLinkResource } from '@geonetwork-ui/common/domain/model/record'
import { ModalDialogComponent } from '@geonetwork-ui/ui/layout'
import { ChangeDetectorRef } from '@angular/core'

let uploadSubject: Subject<any>

const recordAttachments = new BehaviorSubject<RecordAttachment[]>([
{
url: new URL('https://www.fakedomain.com/test.txt'),
fileName: 'test.txt',
},
])

class PlatformServiceInterfaceMock {
attachFileToRecord = jest.fn(() => {
uploadSubject = new Subject()
return uploadSubject
})
getRecordAttachments = jest.fn(() => recordAttachments)
}
export class MatDialogMock {
_subject = new Subject()
Expand Down Expand Up @@ -44,8 +57,13 @@ describe('FormFieldOnlineLinkResourcesComponent', () => {
PlatformServiceInterfaceMock,
'useClass'
),
MockProvider(NotificationsService),
MockProvider(NotificationsService, {
showNotification: jest.fn(),
}),
MockProvider(MatDialogRef),
MockProvider(ChangeDetectorRef, {
detectChanges: jest.fn(),
}),
MockProvider(MatDialog, MatDialogMock, 'useClass'),
],
}).compileComponents()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ export class FormFieldOnlineLinkResourcesComponent {

private allResources: OnlineResource[] = []
linkResources: OnlineLinkResource[] = []
uploadProgress = undefined
uploadProgress?: number = undefined
uploadSubscription: Subscription = null

protected MAX_UPLOAD_SIZE_MB = MAX_UPLOAD_SIZE_MB
Expand Down Expand Up @@ -96,7 +96,7 @@ export class FormFieldOnlineLinkResourcesComponent {
this.valueChange.emit([...this.allResources, newResource])
}
},
error: (error: Error) => this.handleError(error.message),
error: (error: Error) => this.handleError(error),
})
}

Expand All @@ -117,7 +117,7 @@ export class FormFieldOnlineLinkResourcesComponent {
}
this.valueChange.emit([...this.allResources, newLink])
} catch (e) {
this.handleError((e as Error).message)
this.handleError(e as Error)
}
}

Expand All @@ -134,16 +134,17 @@ export class FormFieldOnlineLinkResourcesComponent {
this.openEditDialog(resource, index)
}

private handleError(error: string) {
private handleError(error: Error) {
this.uploadProgress = undefined
this.cd.detectChanges()
this.notificationsService.showNotification({
type: 'error',
title: this.translateService.instant(
'editor.record.onlineResourceError.title'
),
text: `${this.translateService.instant(
'editor.record.onlineResourceError.body'
)} ${error}`,
)} ${error.message}`,
closeMessage: this.translateService.instant(
'editor.record.onlineResourceError.closeMessage'
),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ export class FormFieldOnlineResourcesComponent {
this.valueChange.emit([...this.allResources, newResource])
}
},
error: (error: Error) => this.handleError(error.message),
error: (error: Error) => this.handleError(error),
})
}

Expand All @@ -154,7 +154,7 @@ export class FormFieldOnlineResourcesComponent {
}
this.valueChange.emit([...this.allResources, newLink])
} catch (e) {
this.handleError((e as Error).message)
this.handleError(e as Error)
}
}

Expand Down Expand Up @@ -190,7 +190,7 @@ export class FormFieldOnlineResourcesComponent {
this.openEditDialog(resource, index)
}

private handleError(error: string) {
private handleError(error: Error) {
this.uploadProgress = undefined
this.notificationsService.showNotification({
type: 'error',
Expand All @@ -199,7 +199,7 @@ export class FormFieldOnlineResourcesComponent {
),
text: `${this.translateService.instant(
'editor.record.onlineResourceError.body'
)} ${error}`,
)} ${error.message}`,
closeMessage: this.translateService.instant(
'editor.record.onlineResourceError.closeMessage'
),
Expand Down
Original file line number Diff line number Diff line change
@@ -1,17 +1,29 @@
import { ComponentFixture, TestBed } from '@angular/core/testing'
import { TranslateModule } from '@ngx-translate/core'
import { FormFieldOverviewsComponent } from './form-field-overviews.component'
import { Subject } from 'rxjs'
import { BehaviorSubject, Subject } from 'rxjs'
import { NotificationsService } from '@geonetwork-ui/feature/notifications'
import { MockBuilder, MockProvider } from 'ng-mocks'
import { PlatformServiceInterface } from '@geonetwork-ui/common/domain/platform.service.interface'
import {
PlatformServiceInterface,
RecordAttachment,
} from '@geonetwork-ui/common/domain/platform.service.interface'

let uploadSubject: Subject<any>

const recordAttachments = new BehaviorSubject<RecordAttachment[]>([
{
url: new URL('https://www.fakedomain.com/test.txt'),
fileName: 'test.txt',
},
])

class PlatformServiceInterfaceMock {
attachFileToRecord = jest.fn(() => {
uploadSubject = new Subject()
return uploadSubject
})
getRecordAttachments = jest.fn(() => recordAttachments)
}

describe('FormFieldOverviewsComponent', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ export class FormFieldOverviewsComponent {
})
}
},
error: this.errorHandle,
error: (error: Error) => this.handleError(error),
})
}

Expand All @@ -87,7 +87,7 @@ export class FormFieldOverviewsComponent {
description: filename,
})
} catch (e) {
this.errorHandle(e)
this.handleError(e as Error)
}
}

Expand All @@ -106,8 +106,9 @@ export class FormFieldOverviewsComponent {
this.valueChange.emit(overView ? [overView] : [])
}

private errorHandle = (error) => {
private handleError = (error: Error) => {
this.uploadProgress = undefined
this.cd.markForCheck()
this.notificationsService.showNotification({
type: 'error',
title: this.translateService.instant('editor.record.resourceError.title'),
Expand Down
1 change: 1 addition & 0 deletions libs/util/shared/src/lib/utils/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ export * from './event'
export * from './fuzzy-filter'
export * from './geojson'
export * from './image-resize'
export * from './no-duplicate-file-name'
export * from './parse'
export * from './remove-whitespace'
export * from './sort-by'
Expand Down
Loading
Loading