Merge remote-tracking branch 'remotes/origin/master' into mydspace

# Conflicts:
#	src/app/core/data/base-response-parsing.service.ts
#	src/app/core/data/request.service.ts

package.json

@@ -108,7 +108,7 @@
     "jwt-decode": "^2.2.0",
     "methods": "1.1.2",
     "moment": "^2.22.1",
-    "morgan": "1.9.0",
+    "morgan": "^1.9.1",
     "ng-mocks": "^6.2.1",
     "ng2-file-upload": "1.2.1",
     "ng2-nouislider": "^1.7.11",

src/app/core/auth/auth-object-factory.ts

@@ -4,6 +4,7 @@ import { NormalizedAuthStatus } from './models/normalized-auth-status.model';
 import { NormalizedEPerson } from '../eperson/models/normalized-eperson.model';
 import { NormalizedObject } from '../cache/models/normalized-object.model';
 import { CacheableObject } from '../cache/object-cache.reducer';
+import { NormalizedGroup } from '../eperson/models/normalized-group.model';
 
 export class AuthObjectFactory {
   public static getConstructor(type): GenericConstructor<NormalizedObject<CacheableObject>> {
@@ -12,6 +13,10 @@ export class AuthObjectFactory {
         return NormalizedEPerson
       }
 
+      case AuthType.Group: {
+        return NormalizedGroup
+      }
+
       case AuthType.Status: {
         return NormalizedAuthStatus
       }

src/app/core/auth/auth-request.service.ts

@@ -6,11 +6,18 @@ import { RequestService } from '../data/request.service';
 import { GLOBAL_CONFIG } from '../../../config';
 import { GlobalConfig } from '../../../config/global-config.interface';
 import { isNotEmpty } from '../../shared/empty.util';
-import { AuthGetRequest, AuthPostRequest, PostRequest, RestRequest } from '../data/request.models';
+import {
+  AuthGetRequest,
+  AuthPostRequest,
+  GetRequest,
+  PostRequest,
+  RestRequest
+} from '../data/request.models';
 import { AuthStatusResponse, ErrorResponse } from '../cache/response.models';
 import { HttpOptions } from '../dspace-rest-v2/dspace-rest-v2.service';
 import { RequestEntry } from '../data/request.reducer';
 import { getResponseFromEntry } from '../shared/operators';
+import { RemoteDataBuildService } from '../cache/builders/remote-data-build.service';
 
 @Injectable()
 export class AuthRequestService {
@@ -56,8 +63,8 @@ export class AuthRequestService {
       map((endpointURL) => this.getEndpointByMethod(endpointURL, method)),
       distinctUntilChanged(),
       map((endpointURL: string) => new AuthGetRequest(this.requestService.generateRequestId(), endpointURL, options)),
-      tap((request: PostRequest) => this.requestService.configure(request, true)),
+      tap((request: GetRequest) => this.requestService.configure(request, true)),
-      mergeMap((request: PostRequest) => this.fetchRequest(request)),
+      mergeMap((request: GetRequest) => this.fetchRequest(request)),
       distinctUntilChanged());
   }
 }

src/app/core/auth/auth-response-parsing.service.ts

@@ -13,6 +13,8 @@ import { RestRequest } from '../data/request.models';
 import { AuthType } from './auth-type';
 import { AuthStatus } from './models/auth-status.model';
 import { NormalizedAuthStatus } from './models/normalized-auth-status.model';
+import { NormalizedObject } from '../cache/models/normalized-object.model';
+import { DSpaceObject } from '../shared/dspace-object.model';
 
 @Injectable()
 export class AuthResponseParsingService extends BaseResponseParsingService implements ResponseParsingService {
@@ -27,11 +29,10 @@ export class AuthResponseParsingService extends BaseResponseParsingService imple
 
   parse(request: RestRequest, data: DSpaceRESTV2Response): RestResponse {
     if (isNotEmpty(data.payload) && isNotEmpty(data.payload._links) && (data.statusCode === 200)) {
-      const response = this.process<NormalizedAuthStatus, AuthType>(data.payload, request.uuid);
+      const response = this.process<NormalizedObject<AuthStatus>, AuthType>(data.payload, request.uuid);
       return new AuthStatusResponse(response, data.statusCode, data.statusText);
     } else {
-      return new AuthStatusResponse(data.payload as AuthStatus, data.statusCode, data.statusText);
+      return new AuthStatusResponse(data.payload as NormalizedAuthStatus, data.statusCode, data.statusText);
     }
   }
-
 }

src/app/core/auth/auth-type.ts

@@ -1,4 +1,5 @@
 export enum AuthType {
   EPerson = 'eperson',
-  Status = 'status'
+  Status = 'status',
+  Group = 'group'
 }

src/app/core/auth/auth.service.spec.ts

@@ -26,21 +26,24 @@ import { getMockRemoteDataBuildService } from '../../shared/mocks/mock-remote-da
 
 describe('AuthService test', () => {
 
-  const mockStore: Store<AuthState> = jasmine.createSpyObj('store', {
+  let mockStore: Store<AuthState>;
-    dispatch: {},
-    pipe: observableOf(true)
-  });
   let authService: AuthService;
   let authRequest;
-  const window = new NativeWindowRef();
+  let window;
-  const routerStub = new RouterStub();
+  let routerStub;
   let routeStub;
   let storage: CookieService;
   let token: AuthTokenInfo;
   let authenticatedState;
-  const rdbService = getMockRemoteDataBuildService();
+  let rdbService;
 
   function init() {
+    mockStore = jasmine.createSpyObj('store', {
+      dispatch: {},
+      pipe: observableOf(true)
+    });
+    window = new NativeWindowRef();
+    routerStub = new RouterStub()
     token = new AuthTokenInfo('test_token');
     token.expires = Date.now() + (1000 * 60 * 60);
     authenticatedState = {
@@ -52,15 +55,14 @@ describe('AuthService test', () => {
     };
     authRequest = new AuthRequestServiceStub();
     routeStub = new ActivatedRouteStub();
-  }
+    rdbService = getMockRemoteDataBuildService();
+    spyOn(rdbService, 'build').and.returnValue({authenticated: true, eperson: observableOf({payload: {}})});
 
-  beforeEach(() => {
+  }
-    init();
-  });
 
   describe('', () => {
     beforeEach(() => {
-
+      init();
       TestBed.configureTestingModule({
         imports: [
           CommonModule,
@@ -137,7 +139,8 @@ describe('AuthService test', () => {
           { provide: REQUEST, useValue: {} },
           { provide: Router, useValue: routerStub },
           { provide: RemoteDataBuildService, useValue: rdbService },
-          CookieService
+          CookieService,
+          AuthService
         ]
       }).compileComponents();
     }));
@@ -176,8 +179,8 @@ describe('AuthService test', () => {
   });
 
   describe('', () => {
-
     beforeEach(async(() => {
+      init();
       TestBed.configureTestingModule({
         imports: [
           StoreModule.forRoot({ authReducer })
@@ -186,8 +189,10 @@ describe('AuthService test', () => {
           { provide: AuthRequestService, useValue: authRequest },
           { provide: REQUEST, useValue: {} },
           { provide: Router, useValue: routerStub },
+          { provide: RemoteDataBuildService, useValue: rdbService },
           ClientCookieService,
-          CookieService
+          CookieService,
+          AuthService
         ]
       }).compileComponents();
     }));

src/app/core/auth/auth.service.ts

@@ -130,14 +130,10 @@ export class AuthService {
     headers = headers.append('Authorization', `Bearer ${token.accessToken}`);
     options.headers = headers;
     return this.authRequestService.getRequest('status', options).pipe(
+      map((status) => this.rdbService.build(status)),
       switchMap((status: AuthStatus) => {
-
         if (status.authenticated) {
-          // TODO this should be cleaned up, AuthStatus could be parsed by the RemoteDataService as a whole...
+          return status.eperson.pipe(map((eperson) => eperson.payload));
-          // Review when https://jira.duraspace.org/browse/DS-4006 is fixed
-          // See https://github.com/DSpace/dspace-angular/issues/292
-          const person$ = this.rdbService.buildSingle<EPerson>(status.eperson.toString());
-          return person$.pipe(map((eperson) => eperson.payload));
         } else {
           throw(new Error('Not authenticated'));
         }
@@ -226,7 +222,6 @@ export class AuthService {
           throw(new Error('auth.errors.invalid-user'));
         }
       }))
-
   }
 
   /**

src/app/core/auth/models/auth-status.model.ts

@@ -3,8 +3,9 @@ import { AuthTokenInfo } from './auth-token-info.model';
 import { EPerson } from '../../eperson/models/eperson.model';
 import { RemoteData } from '../../data/remote-data';
 import { Observable } from 'rxjs';
+import { CacheableObject } from '../../cache/object-cache.reducer';
 
-export class AuthStatus {
+export class AuthStatus implements CacheableObject {
 
   id: string;
 

src/app/core/auth/server-auth.service.ts

@@ -34,15 +34,10 @@ export class ServerAuthService extends AuthService {
 
     options.headers = headers;
     return this.authRequestService.getRequest('status', options).pipe(
+      map((status) => this.rdbService.build(status)),
       switchMap((status: AuthStatus) => {
-
         if (status.authenticated) {
-
+          return status.eperson.pipe(map((eperson) => eperson.payload));
-          // TODO this should be cleaned up, AuthStatus could be parsed by the RemoteDataService as a whole...
-          const person$ = this.rdbService.buildSingle<EPerson>(status.eperson.toString());
-          return person$.pipe(
-            map((eperson) => eperson.payload)
-          );
         } else {
           throw(new Error('Not authenticated'));
         }

src/app/core/cache/models/normalized-object-factory.ts

@@ -20,6 +20,7 @@ import { CacheableObject } from '../object-cache.reducer';
 import { NormalizedSubmissionDefinitionsModel } from '../../config/models/normalized-config-submission-definitions.model';
 import { NormalizedSubmissionFormsModel } from '../../config/models/normalized-config-submission-forms.model';
 import { NormalizedSubmissionSectionModel } from '../../config/models/normalized-config-submission-section.model';
+import { NormalizedAuthStatus } from '../../auth/models/normalized-auth-status.model';
 
 export class NormalizedObjectFactory {
   public static getConstructor(type: ResourceType): GenericConstructor<NormalizedObject<CacheableObject>> {

src/app/core/cache/response.models.ts

@@ -14,6 +14,7 @@ import { MetadataField } from '../metadata/metadatafield.model';
 import { PaginatedList } from '../data/paginated-list';
 import { SubmissionObject } from '../submission/models/submission-object.model';
 import { DSpaceObject } from '../shared/dspace-object.model';
+import { NormalizedAuthStatus } from '../auth/models/normalized-auth-status.model';
 
 /* tslint:disable:max-classes-per-file */
 export class RestResponse {
@@ -202,7 +203,7 @@ export class AuthStatusResponse extends RestResponse {
   public toCache = false;
 
   constructor(
-    public response: AuthStatus,
+    public response: NormalizedAuthStatus,
     public statusCode: number,
     public statusText: string,
   ) {

src/app/core/data/base-response-parsing.service.ts

@@ -6,9 +6,8 @@ import { ObjectCacheService } from '../cache/object-cache.service';
 import { GlobalConfig } from '../../../config/global-config.interface';
 import { GenericConstructor } from '../shared/generic-constructor';
 import { PaginatedList } from './paginated-list';
-import { ResourceType } from '../shared/resource-type';
-import { RESTURLCombiner } from '../url-combiner/rest-url-combiner';
 import { isRestDataObject, isRestPaginatedList } from '../cache/builders/normalized-object-build.service';
+
 /* tslint:disable:max-classes-per-file */
 
 export abstract class BaseResponseParsingService {
@@ -26,7 +25,6 @@ export abstract class BaseResponseParsingService {
       } else if (Array.isArray(data)) {
         return this.processArray(data, requestUUID);
       } else if (isRestDataObject(data)) {
-        data = this.fixBadEPersonRestResponse(data);
         const object = this.deserialize(data);
         if (isNotEmpty(data._embedded)) {
           Object
@@ -142,25 +140,6 @@ export abstract class BaseResponseParsingService {
     return this.toCache ? obj.self : obj;
   }
 
-  // TODO Remove when https://jira.duraspace.org/browse/DS-4006 is fixed
-  // See https://github.com/DSpace/dspace-angular/issues/292
-  private fixBadEPersonRestResponse(obj: any): any {
-    if (obj.type === ResourceType.EPerson) {
-      const groups = obj.groups;
-      const normGroups = [];
-      if (isNotEmpty(groups)) {
-        groups.forEach((group) => {
-            const parts = ['eperson', 'groups', group.uuid];
-            const href = new RESTURLCombiner(this.EnvConfig, ...parts).toString();
-            normGroups.push(href);
-          }
-        )
-      }
-      return Object.assign({}, obj, { groups: normGroups });
-    }
-    return obj;
-  }
-
   protected isSuccessStatus(statusCode: number) {
     return statusCode >= 200 && statusCode < 300;
   }

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

@@ -1,9 +1,10 @@
 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 { filter, find, mergeMap, take } from 'rxjs/operators';
+import { filter, find, map, mergeMap, take } from 'rxjs/operators';
-import { remove } from 'lodash';
+import { cloneDeep, remove } from 'lodash';
 
 import { AppState } from '../../app.reducer';
 import { hasValue, isEmpty, isNotEmpty } from '../../shared/empty.util';
@@ -118,6 +119,16 @@ export class RequestService {
             return this.store.pipe(select(entryFromUUIDSelector(originalUUID)))
           },
         ))
+    ).pipe(
+      map((entry: RequestEntry) => {
+        // Headers break after being retrieved from the store (because of lazy initialization)
+        // Combining them with a new object fixes this issue
+        if (hasValue(entry) && hasValue(entry.request) && hasValue(entry.request.options) && hasValue(entry.request.options.headers)) {
+          entry = cloneDeep(entry);
+          entry.request.options.headers = Object.assign(new HttpHeaders(), entry.request.options.headers)
+        }
+        return entry;
+      })
     );
   }
 

src/app/core/dspace-rest-v2/dspace-rest-v2.service.spec.ts

@@ -1,8 +1,10 @@
 import { TestBed, inject } from '@angular/core/testing';
 import { HttpClientTestingModule, HttpTestingController } from '@angular/common/http/testing';
 
-import { DSpaceRESTv2Service } from './dspace-rest-v2.service';
+import { DEFAULT_CONTENT_TYPE, DSpaceRESTv2Service } from './dspace-rest-v2.service';
 import { DSpaceObject } from '../shared/dspace-object.model';
+import { RestRequestMethod } from '../data/rest-request-method';
+import { HttpHeaders } from '@angular/common/http';
 
 describe('DSpaceRESTv2Service', () => {
   let dSpaceRESTv2Service: DSpaceRESTv2Service;
@@ -49,8 +51,6 @@ describe('DSpaceRESTv2Service', () => {
       expect(req.request.method).toBe('GET');
       req.flush(mockPayload, { status: mockStatusCode, statusText: mockStatusText });
     });
-  });
-
     it('should throw an error', () => {
       dSpaceRESTv2Service.get(url).subscribe(() => undefined, (err) => {
         expect(err).toEqual(mockError);
@@ -72,6 +72,51 @@ describe('DSpaceRESTv2Service', () => {
       req.error(mockError);
     });
 
+    it('when no content-type header is provided, it should use application/json', () => {
+      dSpaceRESTv2Service.request(RestRequestMethod.POST, url, {}).subscribe();
+
+      const req = httpMock.expectOne(url);
+      expect(req.request.headers.get('Content-Type')).toContain(DEFAULT_CONTENT_TYPE);
+    });
+  });
+
+  describe('#request', () => {
+    it('should return an Observable<DSpaceRESTV2Response>', () => {
+      const mockPayload = {
+        page: 1
+      };
+      const mockStatusCode = 200;
+      const mockStatusText = 'GREAT';
+
+      dSpaceRESTv2Service.request(RestRequestMethod.POST, url, {}).subscribe((response) => {
+        expect(response).toBeTruthy();
+        expect(response.statusCode).toEqual(mockStatusCode);
+        expect(response.statusText).toEqual(mockStatusText);
+        expect(response.payload.page).toEqual(mockPayload.page);
+      });
+
+      const req = httpMock.expectOne(url);
+      expect(req.request.method).toBe('POST');
+      req.flush(mockPayload, { status: mockStatusCode, statusText: mockStatusText });
+    });
+
+    it('when a content-type header is provided, it should not use application/json', () => {
+      let headers = new HttpHeaders();
+      headers = headers.set('Content-Type', 'text/html');
+      dSpaceRESTv2Service.request(RestRequestMethod.POST, url, {}, { headers }).subscribe();
+
+      const req = httpMock.expectOne(url);
+      expect(req.request.headers.get('Content-Type')).not.toContain(DEFAULT_CONTENT_TYPE);
+    });
+
+    it('when no content-type header is provided, it should use application/json', () => {
+      dSpaceRESTv2Service.request(RestRequestMethod.POST, url, {}).subscribe();
+
+      const req = httpMock.expectOne(url);
+      expect(req.request.headers.get('Content-Type')).toContain(DEFAULT_CONTENT_TYPE);
+    });
+  });
+
   describe('buildFormData', () => {
     it('should return the correct data', () => {
       const name = 'testname';

src/app/core/dspace-rest-v2/dspace-rest-v2.service.ts

@@ -1,4 +1,4 @@
-import {throwError as observableThrowError,  Observable } from 'rxjs';
+import { Observable, throwError as observableThrowError } from 'rxjs';
 import { catchError, map } from 'rxjs/operators';
 import { Injectable } from '@angular/core';
 import { HttpClient, HttpHeaders, HttpParams, HttpResponse } from '@angular/common/http'
@@ -6,9 +6,10 @@ import { HttpClient, HttpHeaders, HttpParams, HttpResponse } from '@angular/comm
 import { DSpaceRESTV2Response } from './dspace-rest-v2-response.model';
 import { HttpObserve } from '@angular/common/http/src/client';
 import { RestRequestMethod } from '../data/rest-request-method';
-import { isNotEmpty } from '../../shared/empty.util';
+import { hasNoValue, isNotEmpty } from '../../shared/empty.util';
 import { DSpaceObject } from '../shared/dspace-object.model';
 
+export const DEFAULT_CONTENT_TYPE = 'application/json; charset=utf-8';
 export interface HttpOptions {
   body?: any;
   headers?: HttpHeaders;
@@ -38,11 +39,23 @@ export class DSpaceRESTv2Service {
    *      An Observable<string> containing the response from the server
    */
   get(absoluteURL: string): Observable<DSpaceRESTV2Response> {
-    return this.http.get(absoluteURL, { observe: 'response' }).pipe(
+    const requestOptions = {
-      map((res: HttpResponse<any>) => ({ payload: res.body, statusCode: res.status, statusText: res.statusText })),
+      observe: 'response' as any,
+      headers: new HttpHeaders({'Content-Type': DEFAULT_CONTENT_TYPE})
+    };
+    return this.http.get(absoluteURL, requestOptions).pipe(
+      map((res: HttpResponse<any>) => ({
+        payload: res.body,
+        statusCode: res.status,
+        statusText: res.statusText
+      })),
       catchError((err) => {
         console.log('Error: ', err);
-        return observableThrowError({statusCode: err.status, statusText: err.statusText, message: err.message});
+        return observableThrowError({
+          statusCode: err.status,
+          statusText: err.statusText,
+          message: err.message
+        });
       }));
   }
 
@@ -65,17 +78,35 @@ export class DSpaceRESTv2Service {
       requestOptions.body = this.buildFormData(body);
     }
     requestOptions.observe = 'response';
-    if (options && options.headers) {
+
-      requestOptions.headers = Object.assign(new HttpHeaders(),  options.headers);
-    }
     if (options && options.responseType) {
       requestOptions.responseType = options.responseType;
     }
+
+    if (hasNoValue(options) || hasNoValue(options.headers)) {
+      requestOptions.headers = new HttpHeaders();
+    } else {
+      requestOptions.headers = options.headers;
+    }
+
+    if (!requestOptions.headers.has('Content-Type')) {
+      // Because HttpHeaders is immutable, the set method returns a new object instead of updating the existing headers
+      requestOptions.headers = requestOptions.headers.set('Content-Type', DEFAULT_CONTENT_TYPE);
+    }
     return this.http.request(method, url, requestOptions).pipe(
-      map((res) => ({ payload: res.body, headers: res.headers, statusCode: res.status, statusText: res.statusText })),
+      map((res) => ({
+        payload: res.body,
+        headers: res.headers,
+        statusCode: res.status,
+        statusText: res.statusText
+      })),
       catchError((err) => {
         console.log('Error: ', err);
-        return observableThrowError({statusCode: err.status, statusText: err.statusText, message: err.message});
+        return observableThrowError({
+          statusCode: err.status,
+          statusText: err.statusText,
+          message: err.message
+        });
       }));
   }
 

src/app/shared/mocks/mock-remote-data-build.service.ts

@@ -4,6 +4,7 @@ import { RemoteDataBuildService } from '../../core/cache/builders/remote-data-bu
 import { RemoteData } from '../../core/data/remote-data';
 import { RequestEntry } from '../../core/data/request.reducer';
 import { hasValue } from '../empty.util';
+import { NormalizedObject } from '../../core/cache/models/normalized-object.model';
 
 export function getMockRemoteDataBuildService(toRemoteDataObservable$?: Observable<RemoteData<any>>): RemoteDataBuildService {
   return {
@@ -17,7 +18,8 @@ export function getMockRemoteDataBuildService(toRemoteDataObservable$?: Observab
         } as RemoteData<any>)))
       }
     },
-    buildSingle: (href$: string | Observable<string>) => observableOf(new RemoteData(false, false, true, undefined, {}))
+    buildSingle: (href$: string | Observable<string>) => observableOf(new RemoteData(false, false, true, undefined, {})),
+    build: (normalized: NormalizedObject<any>) => Object.create({})
   } as RemoteDataBuildService;
 
 }

yarn.lock

@@ -6530,14 +6530,14 @@ moment@^2.22.1:
   resolved "https://registry.yarnpkg.com/moment/-/moment-2.22.2.tgz#3c257f9839fc0e93ff53149632239eb90783ff66"
   integrity sha1-PCV/mDn8DpP/UxSWMiOeuQeD/2Y=
 
-morgan@1.9.0:
+morgan@^1.9.1:
-  version "1.9.0"
+  version "1.9.1"
-  resolved "https://registry.yarnpkg.com/morgan/-/morgan-1.9.0.tgz#d01fa6c65859b76fcf31b3cb53a3821a311d8051"
+  resolved "https://registry.yarnpkg.com/morgan/-/morgan-1.9.1.tgz#0a8d16734a1d9afbc824b99df87e738e58e2da59"
-  integrity sha1-0B+mxlhZt2/PMbPLU6OCGjEdgFE=
+  integrity sha512-HQStPIV4y3afTiCYVxirakhlCfGkI161c76kKFca7Fk1JusM//Qeo1ej2XaMniiNeaZklMVrh3vTtIzpzwbpmA==
   dependencies:
     basic-auth "~2.0.0"
     debug "2.6.9"
-    depd "~1.1.1"
+    depd "~1.1.2"
     on-finished "~2.3.0"
     on-headers "~1.0.1"