ufal/s3-checksum-improvements (#518)

* Do not compute checksum on null object.. * The checksum is computed only after the download button is clicked. * Added tests to check the checksum won't be loaded on init * Fixed accidentally updated message key
dataquest-dev · Feb 20, 2024 · ebfe8dc · ebfe8dc
1 parent e4c2d91
commit ebfe8dc
Show file tree

Hide file tree

Showing 6 changed files with 109 additions and 22 deletions.
diff --git a/...aginated-drag-and-drop-bitstream-list/paginated-drag-and-drop-bitstream-list.component.ts b/...aginated-drag-and-drop-bitstream-list/paginated-drag-and-drop-bitstream-list.component.ts
@@ -60,8 +60,7 @@ export class PaginatedDragAndDropBitstreamListComponent extends AbstractPaginate
           switchMap(() => this.bundleService.getBitstreams(
             this.bundle.id,
             paginatedOptions,
-            followLink('format'),
-            followLink('checksum')
+            followLink('format')
           ))
         );
       })

diff --git a/...age/edit-item-page/item-bitstreams/item-edit-bitstream/item-edit-bitstream.component.html b/...age/edit-item-page/item-bitstreams/item-edit-bitstream/item-edit-bitstream.component.html
@@ -64,11 +64,43 @@
        (mouseenter)="showChecksumValues = true"
        (mouseleave)="showChecksumValues = false"
        *ngVar="(checkSum$ | async) as bitstreamChecksum">
-    <i [class]="checksumsAreEqual(bitstreamChecksum) ? 'fas fa-check' : 'fas fa-times'"></i>
-    <i class="pl-2 fas fa-info-circle"
-       triggers="mouseenter:mouseleave"
-       [ngbPopover]="checksumPopover"
-       popoverTitle="Checksums"></i>
+    <ng-container *ngIf="computedChecksum == false">
+      <ng-container *ngIf="loading">
+        <i class="fas fa-spinner fa-spin"></i>
+      </ng-container>
+      <ng-container *ngIf="!loading">
+        <i class="fas fa-question"
+           triggers="mouseenter:mouseleave"
+           [ngbPopover]="checksumWarning"></i>
+      </ng-container>
+      <a class="btn" (click)="computeChecksum()"
+         triggers="mouseenter:mouseleave"
+         [ngbPopover]="computeChecksumInfo">
+        <i class="fas fa-download"></i>
+      </a>
+    </ng-container>
+    <ng-container *ngIf="computedChecksum">
+      <i [class]="checksumsAreEqual(bitstreamChecksum) ? 'fas fa-check' : 'fas fa-times'"></i>
+      <i class="pl-2 fas fa-info-circle"
+         triggers="mouseenter:mouseleave"
+         [ngbPopover]="checksumPopover"
+         popoverTitle="Checksums"></i>
+    </ng-container>
+  </div>
+</ng-template>
+
+<ng-template #checksumWarning>
+  <div class="font-weight-bold text-decoration-underline">
+    {{ 'item.edit.bitstreams.checksum.popover.warning.header' | translate }}
+  </div>
+  <div>
+    {{ 'item.edit.bitstreams.checksum.popover.warning.body' | translate }}
+  </div>
+</ng-template>
+
+<ng-template #computeChecksumInfo>
+  <div>
+    {{ 'item.edit.bitstreams.checksum.popover.info.body' | translate }}
   </div>
 </ng-template>
 

diff --git a/.../edit-item-page/item-bitstreams/item-edit-bitstream/item-edit-bitstream.component.spec.ts b/.../edit-item-page/item-bitstreams/item-edit-bitstream/item-edit-bitstream.component.spec.ts
@@ -16,6 +16,7 @@ import { BrowserOnlyMockPipe } from '../../../../shared/testing/browser-only-moc
 import { RouterTestingModule } from '@angular/router/testing';
 import { RouterLinkDirectiveStub } from '../../../../shared/testing/router-link-directive.stub';
 import { BitstreamChecksum } from '../../../../core/shared/bitstream-checksum.model';
+import { BitstreamChecksumDataService } from '../../../../core/bitstream-checksum-data.service';
 
 let comp: ItemEditBitstreamComponent;
 let fixture: ComponentFixture<ItemEditBitstreamComponent>;
@@ -44,7 +45,8 @@ const checksum = Object.assign(new BitstreamChecksum(), {
   databaseChecksum: {
     checkSumAlgorithm: 'MD5',
     value: '789'
-  }
+  },
+  href: 'checksum-link'
 });
 
 const bitstream = Object.assign(new Bitstream(), {
@@ -67,8 +69,11 @@ const date = new Date();
 const url = 'thisUrl';
 
 let objectUpdatesService: ObjectUpdatesService;
-
+let bitstreamChecksumDataService: BitstreamChecksumDataService;
 describe('ItemEditBitstreamComponent', () => {
+
+  bitstreamChecksumDataService = jasmine.createSpyObj('bitstreamChecksumDataService',
+    { findByHref: observableOf(checksum) });
   beforeEach(waitForAsync(() => {
     objectUpdatesService = jasmine.createSpyObj('objectUpdatesService',
       {
@@ -104,7 +109,8 @@ describe('ItemEditBitstreamComponent', () => {
         RouterLinkDirectiveStub
       ],
       providers: [
-        { provide: ObjectUpdatesService, useValue: objectUpdatesService }
+        { provide: ObjectUpdatesService, useValue: objectUpdatesService },
+        { provide: BitstreamChecksumDataService, useValue: bitstreamChecksumDataService }
       ], schemas: [
         NO_ERRORS_SCHEMA
       ]
@@ -167,4 +173,19 @@ describe('ItemEditBitstreamComponent', () => {
       expect(comp.bitstreamDownloadUrl).toEqual(getBitstreamDownloadRoute(bitstream));
     });
   });
+
+  describe('when the bitstream checksum is null', () => {
+    it('should not throw any error', () => {
+      expect(comp.checksumsAreEqual(null)).toBeFalse();
+    });
+
+    it('checksum should be undefined on Init', () => {
+      expect(comp.checkSum$).toBeUndefined();
+    });
+
+    it('checksum should be loaded after click on download and compute checksum button', () => {
+      comp.computeChecksum();
+      expect(bitstreamChecksumDataService.findByHref).toHaveBeenCalled();
+    });
+  });
 });
diff --git a/...-page/edit-item-page/item-bitstreams/item-edit-bitstream/item-edit-bitstream.component.ts b/...-page/edit-item-page/item-bitstreams/item-edit-bitstream/item-edit-bitstream.component.ts
@@ -7,14 +7,16 @@ import { BitstreamFormat } from '../../../../core/shared/bitstream-format.model'
 import {
   getRemoteDataPayload,
   getFirstSucceededRemoteData,
-  getFirstCompletedRemoteData
 } from '../../../../core/shared/operators';
 import { ResponsiveTableSizes } from '../../../../shared/responsive-table-sizes/responsive-table-sizes';
 import { DSONameService } from '../../../../core/breadcrumbs/dso-name.service';
 import { FieldUpdate } from '../../../../core/data/object-updates/field-update.model';
 import { FieldChangeType } from '../../../../core/data/object-updates/field-change-type.model';
 import { getBitstreamDownloadRoute } from '../../../../app-routing-paths';
 import { BitstreamChecksum, CheckSum } from '../../../../core/shared/bitstream-checksum.model';
+import { hasNoValue } from '../../../../shared/empty.util';
+import { BitstreamChecksumDataService } from '../../../../core/bitstream-checksum-data.service';
+import { map } from 'rxjs/operators';
 
 @Component({
   selector: 'ds-item-edit-bitstream',
@@ -78,9 +80,20 @@ export class ItemEditBitstreamComponent implements OnChanges, OnInit {
    */
   checkSum$: Observable<BitstreamChecksum>;
 
+  /**
+   * Compute checksum - the whole file must be downloaded to compute the checksum
+   */
+  computedChecksum = false;
+
+  /**
+   * True if the bitstream is being downloaded and the checksum is being computed
+   */
+  loading = false;
+
   constructor(private objectUpdatesService: ObjectUpdatesService,
               private dsoNameService: DSONameService,
-              private viewContainerRef: ViewContainerRef) {
+              private viewContainerRef: ViewContainerRef,
+              private bitstreamChecksumDataService: BitstreamChecksumDataService) {
   }
 
   ngOnInit(): void {
@@ -99,10 +112,6 @@ export class ItemEditBitstreamComponent implements OnChanges, OnInit {
       getFirstSucceededRemoteData(),
       getRemoteDataPayload()
     );
-    this.checkSum$ = this.bitstream.checksum.pipe(
-      getFirstCompletedRemoteData(),
-      getRemoteDataPayload()
-    );
   }
 
   /**
@@ -140,7 +149,7 @@ export class ItemEditBitstreamComponent implements OnChanges, OnInit {
    * @param checksum2 e.g. Active store checksum (local or S3)
    */
   compareChecksums(checksum1: CheckSum, checksum2: CheckSum): boolean {
-    return checksum1.value === checksum2.value && checksum1.checkSumAlgorithm === checksum2.checkSumAlgorithm;
+    return checksum1?.value === checksum2?.value && checksum1?.checkSumAlgorithm === checksum2?.checkSumAlgorithm;
   }
 
   /**
@@ -149,6 +158,10 @@ export class ItemEditBitstreamComponent implements OnChanges, OnInit {
    * @param bitstreamChecksum which contains all checksums
    */
   checksumsAreEqual(bitstreamChecksum: BitstreamChecksum): boolean {
+    if (hasNoValue(bitstreamChecksum)){
+      return false;
+    }
+
     if (this.isBitstreamSynchronized()) {
       // Compare DB and Active store checksums
       // Compare DB and Synchronized and Active store checksums
@@ -166,4 +179,15 @@ export class ItemEditBitstreamComponent implements OnChanges, OnInit {
     return this.bitstream?.storeNumber === SYNCHRONIZED_STORES_NUMBER;
   }
 
+  computeChecksum() {
+    this.loading = true;
+    // Send request to get bitstream checksum
+    this.checkSum$ = this.bitstreamChecksumDataService.findByHref(this.bitstream?._links?.checksum?.href)
+      .pipe(getFirstSucceededRemoteData(), getRemoteDataPayload(),
+        map(value => {
+          this.computedChecksum = true;
+          this.loading = false;
+          return value;
+        }));
+  }
 }
diff --git a/src/assets/i18n/cs.json5 b/src/assets/i18n/cs.json5
@@ -2105,16 +2105,22 @@
   "item.edit.bitstreams.save-button": "Uložit",
   // "item.edit.bitstreams.upload-button": "Upload",
   "item.edit.bitstreams.upload-button": "Nahrát",
-  //  "item.edit.bitstreams.checksum.algorithm": "Algorithm: ",
+  // "item.edit.bitstreams.checksum.algorithm": "Algorithm: ",
   "item.edit.bitstreams.checksum.algorithm": "Algoritmus: ",
-  //  "item.edit.bitstreams.checksum.value": "Value: ",
+  // "item.edit.bitstreams.checksum.value": "Value: ",
   "item.edit.bitstreams.checksum.value": "Hodnota: ",
-  //  "item.edit.bitstreams.checksum.database": "DB",
+  // "item.edit.bitstreams.checksum.database": "DB",
   "item.edit.bitstreams.checksum.database": "DB",
-  //  "item.edit.bitstreams.checksum.active-store": "Active store",
+  // "item.edit.bitstreams.checksum.active-store": "Active store",
   "item.edit.bitstreams.checksum.active-store": "Aktivní úložiště",
-  //  "item.edit.bitstreams.checksum.sync-store": "Sync store",
+  // "item.edit.bitstreams.checksum.sync-store": "Sync store",
   "item.edit.bitstreams.checksum.sync-store": "Sync úložiště",
+  // "item.edit.bitstreams.checksum.popover.warning.header": "Warning:",
+  "item.edit.bitstreams.checksum.popover.warning.header": "Pozor:",
+  // "item.edit.bitstreams.checksum.popover.warning.body": "The file must be downloaded for the checksum to be computed.",
+  "item.edit.bitstreams.checksum.popover.warning.body": "Soubor musí být stažen, aby bylo možné vypočítat kontrolní součet (checksum).",
+  // "item.edit.bitstreams.checksum.popover.info.body": "Download the file and compute the checksum.",
+  "item.edit.bitstreams.checksum.popover.info.body": "Stáhněte soubor a vypočítejte kontrolní součet (checksum).",
   // "item.edit.delete.cancel": "Cancel",
   "item.edit.delete.cancel": "Zrušit",
   // "item.edit.delete.confirm": "Delete",

diff --git a/src/assets/i18n/en.json5 b/src/assets/i18n/en.json5
@@ -2210,6 +2210,11 @@
 
   "item.edit.bitstreams.checksum.sync-store": "Sync store",
 
+  "item.edit.bitstreams.checksum.popover.warning.header": "Warning:",
+
+  "item.edit.bitstreams.checksum.popover.warning.body": "The file must be downloaded for the checksum to be computed.:",
+
+  "item.edit.bitstreams.checksum.popover.info.body": "Download the file and compute the checksum.:",
 
 
   "item.edit.delete.cancel": "Cancel",