Merge branch 'metadata-and-relationships-combined-in-submission' into w2p-70237_entities-orgunit-submission-fix

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

@@ -316,7 +316,9 @@ export abstract class DataService<T extends CacheableObject> {
     ).subscribe((href: string) => {
       this.requestService.removeByHrefSubstring(href);
       const request = new FindListRequest(requestId, href, options);
-      request.responseMsToLive = 10 * 1000;
+      if (hasValue(this.responseMsToLive)) {
+        request.responseMsToLive = this.responseMsToLive;
+      }
       this.requestService.configure(request);
     });
 
@@ -343,6 +345,9 @@ export abstract class DataService<T extends CacheableObject> {
       find((href: string) => hasValue(href)),
       map((href: string) => {
         const request = new PatchRequest(requestId, href, operations);
+        if (hasValue(this.responseMsToLive)) {
+          request.responseMsToLive = this.responseMsToLive;
+        }
         this.requestService.configure(request);
       })
     ).subscribe();
@@ -362,6 +367,11 @@ export abstract class DataService<T extends CacheableObject> {
     const requestId = this.requestService.generateRequestId();
     const serializedObject = new DSpaceSerializer(object.constructor as GenericConstructor<{}>).serialize(object);
     const request = new PutRequest(requestId, object._links.self.href, serializedObject);
+
+    if (hasValue(this.responseMsToLive)) {
+      request.responseMsToLive = this.responseMsToLive;
+    }
+
     this.requestService.configure(request);
 
     return this.requestService.getByUUID(requestId).pipe(
@@ -411,7 +421,13 @@ export abstract class DataService<T extends CacheableObject> {
 
     const request$ = endpoint$.pipe(
       take(1),
-      map((endpoint: string) => new CreateRequest(requestId, endpoint, JSON.stringify(serializedDso)))
+      map((endpoint: string) => {
+        const request = new CreateRequest(requestId, endpoint, JSON.stringify(serializedDso));
+        if (hasValue(this.responseMsToLive)) {
+          request.responseMsToLive = this.responseMsToLive;
+        }
+        return request
+      })
     );
 
     // Execute the post request
@@ -460,7 +476,13 @@ export abstract class DataService<T extends CacheableObject> {
 
     const request$ = endpoint$.pipe(
       take(1),
-      map((endpoint: string) => new CreateRequest(requestId, endpoint, JSON.stringify(serializedDso)))
+      map((endpoint: string) => {
+        const request = new CreateRequest(requestId, endpoint, JSON.stringify(serializedDso));
+        if (hasValue(this.responseMsToLive)) {
+          request.responseMsToLive = this.responseMsToLive;
+        }
+        return request
+      })
     );
 
     // Execute the post request
@@ -508,6 +530,9 @@ export abstract class DataService<T extends CacheableObject> {
           );
         }
         const request = new DeleteByIDRequest(requestId, href, dsoID);
+        if (hasValue(this.responseMsToLive)) {
+          request.responseMsToLive = this.responseMsToLive;
+        }
         this.requestService.configure(request);
       })
     ).subscribe();

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

@@ -52,7 +52,13 @@ import { DefaultChangeAnalyzer } from './default-change-analyzer.service';
 import { ItemDataService } from './item-data.service';
 import { PaginatedList } from './paginated-list';
 import { RemoteData, RemoteDataState } from './remote-data';
-import { DeleteRequest, FindListOptions, PostRequest, RestRequest } from './request.models';
+import {
+  DeleteRequest,
+  FindListOptions,
+  PostRequest,
+  RestRequest,
+  FindByIDRequest
+} from './request.models';
 import { RequestService } from './request.service';
 import has = Reflect.has;
 
@@ -86,6 +92,7 @@ const compareItemsByUUID = (itemCheck: Item) =>
 @dataService(RELATIONSHIP)
 export class RelationshipService extends DataService<Relationship> {
   protected linkPath = 'relationships';
+  protected responseMsToLive = 15 * 60 * 1000;
 
   constructor(protected itemService: ItemDataService,
               protected requestService: RequestService,

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

@@ -22,6 +22,7 @@ import {
 } from './request.models';
 import { RequestEntry } from './request.reducer';
 import { RequestService } from './request.service';
+import { parseJsonSchemaToCommandDescription } from '@angular/cli/utilities/json-schema';
 
 describe('RequestService', () => {
   let scheduler: TestScheduler;
@@ -139,13 +140,21 @@ describe('RequestService', () => {
   describe('getByUUID', () => {
     describe('if the request with the specified UUID exists in the store', () => {
       beforeEach(() => {
-        selectSpy.and.callFake(() => {
+        let callCounter = 0;
-          return () => {
+        const responses = [
-            return () => hot('a', {
+          cold('a', { // A direct hit in the request cache
             a: {
               completed: true
             }
-            });
+          }),
+          cold('b', { b: undefined }), // No hit in the index
+          cold('c', { c: undefined })  // So no mapped hit in the request cache
+        ];
+        selectSpy.and.callFake(() => {
+          return () => {
+            const response = responses[callCounter];
+            callCounter++;
+            return () => response;
           };
         });
       });
@@ -164,17 +173,57 @@ describe('RequestService', () => {
 
     describe(`if the request with the specified UUID doesn't exist in the store `, () => {
       beforeEach(() => {
+        let callCounter = 0;
+        const responses = [
+          cold('a', { a: undefined }), // No direct hit in the request cache
+          cold('b', { b: undefined }), // No hit in the index
+          cold('c', { c: undefined }), // So no mapped hit in the request cache
+        ];
         selectSpy.and.callFake(() => {
           return () => {
-            return () => hot('a', { a: undefined });
+            const response = responses[callCounter];
+            callCounter++;
+            return () => response;
           };
         });
       });
 
-      it(`it shouldn't return anything`, () => {
+      it('should return an Observable of undefined', () => {
         const result = service.getByUUID(testUUID);
 
-        scheduler.expectObservable(result).toBe('');
+        scheduler.expectObservable(result).toBe('a', { a: undefined });
+      });
+    });
+
+    describe(`if the request with the specified UUID wasn't sent, because it was already cached`, () => {
+      beforeEach(() => {
+        let callCounter = 0;
+        const responses = [
+          cold('a', { a: undefined }), // No direct hit in the request cache with that UUID
+          cold('b', { b: 'otherRequestUUID' }), // A hit in the index, which returns the uuid of the cached request
+          cold('c', {   // the call to retrieve the cached request using the UUID from the index
+            c: {
+              completed: true
+            }
+          })
+        ];
+        selectSpy.and.callFake(() => {
+          return () => {
+            const response = responses[callCounter];
+            callCounter++;
+            return () => response;
+          };
+        });
+      });
+
+      it(`it should return the cached request`, () => {
+        const result = service.getByUUID(testUUID);
+
+        scheduler.expectObservable(result).toBe('c', {
+          c: {
+            completed: true
+          }
+        });
       });
     });
 

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

@@ -2,8 +2,8 @@ import { Injectable } from '@angular/core';
 import { HttpHeaders } from '@angular/common/http';
 
 import { createSelector, MemoizedSelector, select, Store } from '@ngrx/store';
-import { Observable, race as observableRace } from 'rxjs';
+import { Observable, combineLatest as observableCombineLatest } from 'rxjs';
-import { filter, map, mergeMap, take, switchMap } from 'rxjs/operators';
+import { filter, map, mergeMap, take, switchMap, startWith } from 'rxjs/operators';
 import { cloneDeep, remove } from 'lodash';
 import { hasValue, isEmpty, isNotEmpty, hasValueOperator } from '../../shared/empty.util';
 import { CacheableObject } from '../cache/object-cache.reducer';
@@ -110,24 +110,19 @@ export class RequestService {
    * Retrieve a RequestEntry based on their uuid
    */
   getByUUID(uuid: string): Observable<RequestEntry> {
-    return observableRace(
+    return observableCombineLatest([
       this.store.pipe(
-        select(entryFromUUIDSelector(uuid)),
+        select(entryFromUUIDSelector(uuid))
-        hasValueOperator()
       ),
       this.store.pipe(
         select(originalRequestUUIDFromRequestUUIDSelector(uuid)),
         switchMap((originalUUID) => {
-            if (hasValue(originalUUID)) {
               return this.store.pipe(select(entryFromUUIDSelector(originalUUID)))
-            } else {
-              return []
-            }
           },
         ),
-        hasValueOperator()
+      ),
-      )
+    ]).pipe(
-    ).pipe(
+      map((entries: RequestEntry[]) => entries.find((entry: RequestEntry) => hasValue(entry))),
       map((entry: RequestEntry) => {
         // Headers break after being retrieved from the store (because of lazy initialization)
         // Combining them with a new object fixes this issue

src/app/shared/form/builder/ds-dynamic-form-ui/models/array-group/dynamic-form-array.component.ts

@@ -34,7 +34,7 @@ import {
 } from '../../../../../../core/shared/operators';
 import { SubmissionObject } from '../../../../../../core/submission/models/submission-object.model';
 import { SubmissionObjectDataService } from '../../../../../../core/submission/submission-object-data.service';
-import { hasValue } from '../../../../../empty.util';
+import { hasValue, isNotEmpty } from '../../../../../empty.util';
 import { FormFieldMetadataValueObject } from '../../../models/form-field-metadata-value.model';
 import {
   Reorderable,
@@ -47,6 +47,7 @@ import { Store } from '@ngrx/store';
 import { SubmissionService } from '../../../../../../submission/submission.service';
 import { AppState } from '../../../../../../app.reducer';
 import { followLink } from '../../../../../utils/follow-link-config.model';
+import { ObjectCacheService } from '../../../../../../core/cache/object-cache.service';
 
 @Component({
   selector: 'ds-dynamic-form-array',
@@ -74,6 +75,7 @@ export class DsDynamicFormArrayComponent extends DynamicFormArrayComponent imple
 
   constructor(protected layoutService: DynamicFormLayoutService,
               protected validationService: DynamicFormValidationService,
+              protected objectCacheService: ObjectCacheService,
               protected relationshipService: RelationshipService,
               protected changeDetectorRef: ChangeDetectorRef,
               protected submissionObjectService: SubmissionObjectDataService,
@@ -179,7 +181,7 @@ export class DsDynamicFormArrayComponent extends DynamicFormArrayComponent imple
           this.reorderables = reorderables;
 
           if (shouldPropagateChanges) {
-            const updatedReorderables: Array<Observable<any>> = [];
+            const movedReoRels: Array<Reorderable> = [];
             let hasMetadataField = false;
             this.reorderables.forEach((reorderable: Reorderable, index: number) => {
               if (reorderable.hasMoved) {
@@ -187,8 +189,9 @@ export class DsDynamicFormArrayComponent extends DynamicFormArrayComponent imple
                 const updatedReorderable = reorderable.update().pipe(take(1));
                 updatedReorderables.push(updatedReorderable);
                 if (reorderable instanceof ReorderableFormFieldMetadataValue) {
+                  const prevIndex = reorderable.oldIndex;
                   hasMetadataField = true;
-                  updatedReorderable.subscribe((v) => {
+                  reorderable.update().pipe(take(1)).subscribe((v) => {
                     const reoMD = reorderable as ReorderableFormFieldMetadataValue;
                     reoMD.model.value = reoMD.metadataValue;
                     this.onChange({
@@ -200,18 +203,22 @@ export class DsDynamicFormArrayComponent extends DynamicFormArrayComponent imple
                       type: DynamicFormControlEventType.Change
                     });
                   });
+                } else if (reorderable instanceof ReorderableRelationship) {
+                  movedReoRels.push(reorderable)
                 }
               }
             });
 
-            observableCombineLatest(updatedReorderables).pipe(
+            if (isNotEmpty(movedReoRels) && hasMetadataField && hasValue(this.model.relationshipConfig)) {
-            ).subscribe(() => {
-              if (hasMetadataField && hasValue(this.model.relationshipConfig)) {
               // if it's a mix between entities and regular metadata fields,
-                // we need to save after every operation, since they use different
+              // we need to save, since they use different endpoints and
-                // endpoints and otherwise they'll get out of sync.
+              // otherwise they'll get out of sync.
               this.submissionService.dispatchSave(this.model.submissionId);
             }
+
+            observableCombineLatest(
+              movedReoRels.map((movedReoRel) => movedReoRel.update().pipe(take(1)))
+            ).subscribe(() => {
               this.changeDetectorRef.detectChanges();
             });
           }