64961: server-sync-buffer bugfix, data-service update revert changes + edit-bitstream onSubmit refactoring

src/app/+bitstream-page/edit-bitstream-page/edit-bitstream-page.component.ts

@@ -2,7 +2,7 @@ import { ChangeDetectionStrategy, ChangeDetectorRef, Component, OnDestroy, OnIni
 import { Bitstream } from '../../core/shared/bitstream.model';
 import { ActivatedRoute } from '@angular/router';
 import { map, switchMap, take, tap } from 'rxjs/operators';
-import { combineLatest as observableCombineLatest, concat as observableConcat } from 'rxjs';
+import { combineLatest as observableCombineLatest, of as observableOf } from 'rxjs';
 import { Subscription } from 'rxjs/internal/Subscription';
 import {
   DynamicFormControlModel,
@@ -18,13 +18,17 @@ import { TranslateService } from '@ngx-translate/core';
 import { DynamicCustomSwitchModel } from '../../shared/form/builder/ds-dynamic-form-ui/models/custom-switch/custom-switch.model';
 import { cloneDeep } from 'lodash';
 import { BitstreamDataService } from '../../core/data/bitstream-data.service';
-import { getRemoteDataPayload, getSucceededRemoteData } from '../../core/shared/operators';
+import {
+  getFirstSucceededRemoteDataPayload,
+  getRemoteDataPayload,
+  getSucceededRemoteData
+} from '../../core/shared/operators';
 import { NotificationsService } from '../../shared/notifications/notifications.service';
 import { BitstreamFormatDataService } from '../../core/data/bitstream-format-data.service';
 import { BitstreamFormat } from '../../core/shared/bitstream-format.model';
 import { BitstreamFormatSupportLevel } from '../../core/shared/bitstream-format-support-level';
 import { RestResponse } from '../../core/cache/response.models';
-import { hasValue, isNotEmpty, isNotEmptyOperator } from '../../shared/empty.util';
+import { hasValue, isNotEmpty } from '../../shared/empty.util';
 import { Metadata } from '../../core/shared/metadata.utils';
 import { Location } from '@angular/common';
 
@@ -311,9 +315,7 @@ export class EditBitstreamPageComponent implements OnInit, OnDestroy {
       }
     });
     this.bitstream.format.pipe(
-      getSucceededRemoteData(),
+      getFirstSucceededRemoteDataPayload()
-      getRemoteDataPayload(),
-      take(1)
     ).subscribe((format: BitstreamFormat) => {
       this.originalFormat = format;
       this.formGroup.patchValue({
@@ -399,20 +401,10 @@ export class EditBitstreamPageComponent implements OnInit, OnDestroy {
     const selectedFormat = this.formats.find((f: BitstreamFormat) => f.id === updatedValues.formatContainer.selectedFormat);
     const isNewFormat = selectedFormat.id !== this.originalFormat.id;
 
-    const extraOperations = [];
+    let bitstream$;
-    if (isNewFormat) {
-      const operation = Object.assign({op: 'replace', path: '/format', value: selectedFormat.self});
-      extraOperations.push(operation);
-    }
-
-    const updatedBitstream$ = this.bitstreamService.update(this.bitstream, extraOperations).pipe(
-      getSucceededRemoteData(),
-      getRemoteDataPayload(),
-      tap(() => this.bitstreamService.commitUpdates())
-    );
 
     if (isNewFormat) {
-      this.bitstreamService.updateFormat(this.bitstream, selectedFormat).pipe(
+      bitstream$ = this.bitstreamService.updateFormat(this.bitstream, selectedFormat).pipe(
         switchMap((formatResponse: RestResponse) => {
           if (hasValue(formatResponse) && !formatResponse.isSuccessful) {
             this.notificationsService.error(
@@ -420,17 +412,37 @@ export class EditBitstreamPageComponent implements OnInit, OnDestroy {
               formatResponse.statusText
             );
           } else {
-            return updatedBitstream$;
+            return this.bitstreamService.findById(this.bitstream.id).pipe(
+              getFirstSucceededRemoteDataPayload()
+            );
           }
         })
+      )
+    } else {
+      bitstream$ = observableOf(this.bitstream);
+    }
+
+    bitstream$.pipe(
+      switchMap(() => {
+        if (isNewFormat) {
+          const operation = Object.assign({op: 'replace', path: '/format', value: selectedFormat.self});
+          this.bitstreamService.patch(this.bitstream.self, [operation]);
+        }
+
+        return this.bitstreamService.update(this.bitstream).pipe(
+          getFirstSucceededRemoteDataPayload(),
+          switchMap(() => {
+            this.bitstreamService.commitUpdates();
+            return this.bitstreamService.findById(this.bitstream.id).pipe(
+              getFirstSucceededRemoteDataPayload()
+            );
+          })
+        );
+      })
     ).subscribe((bitstream: Bitstream) => {
       this.onSuccess(bitstream);
     });
-    } else {
+
-      updatedBitstream$.subscribe((bitstream: Bitstream) => {
-        this.onSuccess(bitstream);
-      });
-    }
   }
 
   /**

src/app/core/cache/server-sync-buffer.reducer.ts

@@ -68,6 +68,8 @@ function addToServerSyncQueue(state: ServerSyncBufferState, action: AddToSSBActi
   const actionEntry = action.payload as ServerSyncBufferEntry;
   if (hasNoValue(state.buffer.find((entry) => entry.href === actionEntry.href && entry.method === actionEntry.method))) {
     return Object.assign({}, state, { buffer: state.buffer.concat(actionEntry) });
+  } else {
+    return state;
   }
 }
 

src/app/core/data/data.service.ts

@@ -211,18 +211,13 @@ export abstract class DataService<T extends CacheableObject> {
    * Add a new patch to the object cache
    * The patch is derived from the differences between the given object and its version in the object cache
    * @param {DSpaceObject} object The given object
-   * @param extraOperations
    */
-  update(object: T, extraOperations?: Operation[]): Observable<RemoteData<T>> {
+  update(object: T): Observable<RemoteData<T>> {
     const oldVersion$ = this.objectCache.getObjectBySelfLink(object.self);
     return oldVersion$.pipe(take(1), mergeMap((oldVersion: T) => {
         const operations = this.comparator.diff(oldVersion, object);
-        let combinedOperations = operations || extraOperations;
+        if (isNotEmpty(operations)) {
-        if (isNotEmpty(extraOperations)) {
+          this.objectCache.addPatch(object.self, operations);
-          combinedOperations = [...operations, ...extraOperations];
-        }
-        if (isNotEmpty(combinedOperations)) {
-          this.objectCache.addPatch(object.self, combinedOperations);
         }
         return this.findById(object.uuid);
       }
@@ -316,7 +311,7 @@ export abstract class DataService<T extends CacheableObject> {
    * @param dso The DSpace Object to be removed
    * Return the delete request's ID
    */
-  deleteAndReturnRequestId(dso: T): string {
+  private deleteAndReturnRequestId(dso: T): string {
     const requestId = this.requestService.generateRequestId();
 
     const hrefObs = this.halService.getEndpoint(this.linkPath).pipe(

src/app/core/shared/operators.ts

@@ -126,3 +126,15 @@ export const getFirstOccurrence = () =>
     source.pipe(
       map((rd) => Object.assign(rd, { payload: rd.payload.page.length > 0 ? rd.payload.page[0] : undefined }))
     );
+
+/**
+ * Get the first succeeded RemoteData's payload
+ */
+export const getFirstSucceededRemoteDataPayload = () =>
+  <T>(source: Observable<RemoteData<T>>): Observable<T> =>
+    source.pipe(
+      getSucceededRemoteData(),
+      getRemoteDataPayload(),
+      hasValueOperator(),
+      take(1)
+    );