Merge pull request #1009 from geonetwork/me-upload-same-file-twice

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

libs/api/repository/src/lib/gn4/platform/gn4-platform.service.spec.ts

@@ -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
@@ -187,7 +184,11 @@ class RecordsApiServiceMock {
       },
     ])
   )
-  putResource = jest.fn(() => of(undefined))
+  putResource = jest.fn(() =>
+    of({
+      type: HttpEventType.UploadProgress,
+    })
+  )
 }
 
 class UserfeedbackApiServiceMock {
@@ -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', () => {
@@ -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,
@@ -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({

libs/api/repository/src/lib/gn4/platform/gn4-platform.service.ts

@@ -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,
@@ -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,
@@ -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'
 
@@ -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)
+        )
+      })
+    )
   }
 }

libs/common/domain/src/lib/platform.service.interface.ts

@@ -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

libs/feature/editor/src/lib/components/record-form/form-field/form-field-online-link-resources/form-field-online-link-resources.component.spec.ts

@@ -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()
@@ -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()

libs/feature/editor/src/lib/components/record-form/form-field/form-field-online-link-resources/form-field-online-link-resources.component.ts

@@ -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
@@ -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),
       })
   }
 
@@ -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)
     }
   }
 
@@ -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'
       ),

libs/feature/editor/src/lib/components/record-form/form-field/form-field-online-resources/form-field-online-resources.component.ts

@@ -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),
       })
   }
 
@@ -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)
     }
   }
 
@@ -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',
@@ -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'
       ),

libs/feature/editor/src/lib/components/record-form/form-field/form-field-overviews/form-field-overviews.component.spec.ts

@@ -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', () => {

libs/feature/editor/src/lib/components/record-form/form-field/form-field-overviews/form-field-overviews.component.ts

@@ -68,7 +68,7 @@ export class FormFieldOverviewsComponent {
             })
           }
         },
-        error: this.errorHandle,
+        error: (error: Error) => this.handleError(error),
       })
   }
 
@@ -87,7 +87,7 @@ export class FormFieldOverviewsComponent {
         description: filename,
       })
     } catch (e) {
-      this.errorHandle(e)
+      this.handleError(e as Error)
     }
   }
 
@@ -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'),

libs/util/shared/src/lib/utils/index.ts

@@ -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'