From 1ceae322de6c6254203494ccf7a2a75b4f0392ed Mon Sep 17 00:00:00 2001 From: lotte Date: Fri, 5 Apr 2019 09:13:42 +0200 Subject: [PATCH 01/10] 61561: fixed ui issue for DS-4006 --- src/app/core/auth/auth-object-factory.ts | 5 +++++ src/app/core/auth/auth-type.ts | 3 ++- .../data/base-response-parsing.service.ts | 20 ------------------- 3 files changed, 7 insertions(+), 21 deletions(-) diff --git a/src/app/core/auth/auth-object-factory.ts b/src/app/core/auth/auth-object-factory.ts index e37475d94c..02458f4e3e 100644 --- a/src/app/core/auth/auth-object-factory.ts +++ b/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> { @@ -12,6 +13,10 @@ export class AuthObjectFactory { return NormalizedEPerson } + case AuthType.Group: { + return NormalizedGroup + } + case AuthType.Status: { return NormalizedAuthStatus } diff --git a/src/app/core/auth/auth-type.ts b/src/app/core/auth/auth-type.ts index 9a248da91f..f0460449ea 100644 --- a/src/app/core/auth/auth-type.ts +++ b/src/app/core/auth/auth-type.ts @@ -1,4 +1,5 @@ export enum AuthType { EPerson = 'eperson', - Status = 'status' + Status = 'status', + Group = 'group' } diff --git a/src/app/core/data/base-response-parsing.service.ts b/src/app/core/data/base-response-parsing.service.ts index 6102f930b0..334aadd379 100644 --- a/src/app/core/data/base-response-parsing.service.ts +++ b/src/app/core/data/base-response-parsing.service.ts @@ -26,7 +26,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 @@ -141,23 +140,4 @@ export abstract class BaseResponseParsingService { protected retrieveObjectOrUrl(obj: any): any { 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; - } } From b44389084865db0a30c6a745326a76fc3147421d Mon Sep 17 00:00:00 2001 From: lotte Date: Mon, 8 Apr 2019 15:55:09 +0200 Subject: [PATCH 02/10] 61561: resolving second todo in auth srvice --- src/app/core/auth/auth-request.service.ts | 13 ++++++++++--- src/app/core/auth/auth-response-parsing.service.ts | 7 ++++--- src/app/core/auth/auth.service.ts | 8 ++------ src/app/core/auth/models/auth-status.model.ts | 3 ++- src/app/core/auth/server-auth.service.ts | 9 ++------- .../core/cache/models/normalized-object-factory.ts | 4 ++++ src/app/core/cache/response.models.ts | 3 ++- src/app/core/data/request.effects.ts | 1 + .../core/dspace-rest-v2/dspace-rest-v2.service.ts | 1 + src/app/core/shared/resource-type.ts | 1 + 10 files changed, 29 insertions(+), 21 deletions(-) diff --git a/src/app/core/auth/auth-request.service.ts b/src/app/core/auth/auth-request.service.ts index 6d782cbbe2..14784d1b55 100644 --- a/src/app/core/auth/auth-request.service.ts +++ b/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)), - mergeMap((request: PostRequest) => this.fetchRequest(request)), + tap((request: GetRequest) => this.requestService.configure(request, true)), + mergeMap((request: GetRequest) => this.fetchRequest(request)), distinctUntilChanged()); } } diff --git a/src/app/core/auth/auth-response-parsing.service.ts b/src/app/core/auth/auth-response-parsing.service.ts index 3cb00789f6..1993f6d162 100644 --- a/src/app/core/auth/auth-response-parsing.service.ts +++ b/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(data.payload, request.uuid); + const response = this.process, 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); } } - } diff --git a/src/app/core/auth/auth.service.ts b/src/app/core/auth/auth.service.ts index fdb372f643..d0547a1b9c 100644 --- a/src/app/core/auth/auth.service.ts +++ b/src/app/core/auth/auth.service.ts @@ -146,14 +146,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... - // 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(status.eperson.toString()); - return person$.pipe(map((eperson) => eperson.payload)); + return status.eperson.pipe(map((eperson) => eperson.payload)); } else { throw(new Error('Not authenticated')); } diff --git a/src/app/core/auth/models/auth-status.model.ts b/src/app/core/auth/models/auth-status.model.ts index 37f8d76672..6e722a80c9 100644 --- a/src/app/core/auth/models/auth-status.model.ts +++ b/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; diff --git a/src/app/core/auth/server-auth.service.ts b/src/app/core/auth/server-auth.service.ts index b61b11a4f2..c344683e38 100644 --- a/src/app/core/auth/server-auth.service.ts +++ b/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) { - - // TODO this should be cleaned up, AuthStatus could be parsed by the RemoteDataService as a whole... - const person$ = this.rdbService.buildSingle(status.eperson.toString()); - return person$.pipe( - map((eperson) => eperson.payload) - ); + return status.eperson.pipe(map((eperson) => eperson.payload)); } else { throw(new Error('Not authenticated')); } diff --git a/src/app/core/cache/models/normalized-object-factory.ts b/src/app/core/cache/models/normalized-object-factory.ts index 53d7f475fc..15dfe08e86 100644 --- a/src/app/core/cache/models/normalized-object-factory.ts +++ b/src/app/core/cache/models/normalized-object-factory.ts @@ -18,6 +18,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> { @@ -52,6 +53,9 @@ export class NormalizedObjectFactory { case ResourceType.Group: { return NormalizedGroup } + case ResourceType.Status: { + return NormalizedAuthStatus + } case ResourceType.MetadataSchema: { return NormalizedMetadataSchema } diff --git a/src/app/core/cache/response.models.ts b/src/app/core/cache/response.models.ts index a734eba812..810dcf1799 100644 --- a/src/app/core/cache/response.models.ts +++ b/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, ) { diff --git a/src/app/core/data/request.effects.ts b/src/app/core/data/request.effects.ts index 5e7bec698b..506cfd5f8d 100644 --- a/src/app/core/data/request.effects.ts +++ b/src/app/core/data/request.effects.ts @@ -48,6 +48,7 @@ export class RequestEffects { const serializer = new DSpaceRESTv2Serializer(NormalizedObjectFactory.getConstructor(request.body.type)); body = serializer.serialize(request.body); } + console.log(JSON.stringify(request)); return this.restApi.request(request.method, request.href, body, request.options).pipe( map((data: DSpaceRESTV2Response) => this.injector.get(request.getResponseParser()).parse(request, data)), addToResponseCacheAndCompleteAction(request, this.EnvConfig), diff --git a/src/app/core/dspace-rest-v2/dspace-rest-v2.service.ts b/src/app/core/dspace-rest-v2/dspace-rest-v2.service.ts index a2a9f2530c..6e700f68d7 100644 --- a/src/app/core/dspace-rest-v2/dspace-rest-v2.service.ts +++ b/src/app/core/dspace-rest-v2/dspace-rest-v2.service.ts @@ -71,6 +71,7 @@ export class DSpaceRESTv2Service { if (options && options.responseType) { requestOptions.responseType = options.responseType; } + console.log('url', url); return this.http.request(method, url, requestOptions).pipe( map((res) => ({ payload: res.body, headers: res.headers, statusCode: res.status, statusText: res.statusText })), catchError((err) => { diff --git a/src/app/core/shared/resource-type.ts b/src/app/core/shared/resource-type.ts index 484f1ea6e2..451e4c3324 100644 --- a/src/app/core/shared/resource-type.ts +++ b/src/app/core/shared/resource-type.ts @@ -20,4 +20,5 @@ export enum ResourceType { SubmissionForms = 'submissionforms', SubmissionSections = 'submissionsections', SubmissionSection = 'submissionsection', + Status = 'status', } From d89e639e7693f4728cc2097721cf68f08bfc92dd Mon Sep 17 00:00:00 2001 From: lotte Date: Mon, 8 Apr 2019 17:17:12 +0200 Subject: [PATCH 03/10] 61561: clean up --- src/app/core/cache/models/normalized-object-factory.ts | 3 --- src/app/core/data/request.effects.ts | 1 - src/app/core/dspace-rest-v2/dspace-rest-v2.service.ts | 1 - src/app/core/shared/resource-type.ts | 1 - src/app/shared/mocks/mock-remote-data-build.service.ts | 4 +++- 5 files changed, 3 insertions(+), 7 deletions(-) diff --git a/src/app/core/cache/models/normalized-object-factory.ts b/src/app/core/cache/models/normalized-object-factory.ts index 15dfe08e86..2a0db803df 100644 --- a/src/app/core/cache/models/normalized-object-factory.ts +++ b/src/app/core/cache/models/normalized-object-factory.ts @@ -53,9 +53,6 @@ export class NormalizedObjectFactory { case ResourceType.Group: { return NormalizedGroup } - case ResourceType.Status: { - return NormalizedAuthStatus - } case ResourceType.MetadataSchema: { return NormalizedMetadataSchema } diff --git a/src/app/core/data/request.effects.ts b/src/app/core/data/request.effects.ts index 506cfd5f8d..5e7bec698b 100644 --- a/src/app/core/data/request.effects.ts +++ b/src/app/core/data/request.effects.ts @@ -48,7 +48,6 @@ export class RequestEffects { const serializer = new DSpaceRESTv2Serializer(NormalizedObjectFactory.getConstructor(request.body.type)); body = serializer.serialize(request.body); } - console.log(JSON.stringify(request)); return this.restApi.request(request.method, request.href, body, request.options).pipe( map((data: DSpaceRESTV2Response) => this.injector.get(request.getResponseParser()).parse(request, data)), addToResponseCacheAndCompleteAction(request, this.EnvConfig), diff --git a/src/app/core/dspace-rest-v2/dspace-rest-v2.service.ts b/src/app/core/dspace-rest-v2/dspace-rest-v2.service.ts index 6e700f68d7..a2a9f2530c 100644 --- a/src/app/core/dspace-rest-v2/dspace-rest-v2.service.ts +++ b/src/app/core/dspace-rest-v2/dspace-rest-v2.service.ts @@ -71,7 +71,6 @@ export class DSpaceRESTv2Service { if (options && options.responseType) { requestOptions.responseType = options.responseType; } - console.log('url', url); return this.http.request(method, url, requestOptions).pipe( map((res) => ({ payload: res.body, headers: res.headers, statusCode: res.status, statusText: res.statusText })), catchError((err) => { diff --git a/src/app/core/shared/resource-type.ts b/src/app/core/shared/resource-type.ts index 451e4c3324..484f1ea6e2 100644 --- a/src/app/core/shared/resource-type.ts +++ b/src/app/core/shared/resource-type.ts @@ -20,5 +20,4 @@ export enum ResourceType { SubmissionForms = 'submissionforms', SubmissionSections = 'submissionsections', SubmissionSection = 'submissionsection', - Status = 'status', } diff --git a/src/app/shared/mocks/mock-remote-data-build.service.ts b/src/app/shared/mocks/mock-remote-data-build.service.ts index 675e539d90..3cab439581 100644 --- a/src/app/shared/mocks/mock-remote-data-build.service.ts +++ b/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>): RemoteDataBuildService { return { @@ -17,7 +18,8 @@ export function getMockRemoteDataBuildService(toRemoteDataObservable$?: Observab } as RemoteData))) } }, - buildSingle: (href$: string | Observable) => observableOf(new RemoteData(false, false, true, undefined, {})) + buildSingle: (href$: string | Observable) => observableOf(new RemoteData(false, false, true, undefined, {})), + build: (normalized: NormalizedObject) => Object.create({}) } as RemoteDataBuildService; } From cd131d4eeb1057f73f0d1e13f1634cb2b180d5b2 Mon Sep 17 00:00:00 2001 From: lotte Date: Tue, 9 Apr 2019 11:10:47 +0200 Subject: [PATCH 04/10] Fixed test issues --- src/app/core/auth/auth.service.spec.ts | 35 +++++++++++++++----------- src/app/core/auth/auth.service.ts | 1 - 2 files changed, 20 insertions(+), 16 deletions(-) diff --git a/src/app/core/auth/auth.service.spec.ts b/src/app/core/auth/auth.service.spec.ts index c461148eea..ebb4026265 100644 --- a/src/app/core/auth/auth.service.spec.ts +++ b/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 = jasmine.createSpyObj('store', { - dispatch: {}, - pipe: observableOf(true) - }); + let mockStore: Store; let authService: AuthService; let authRequest; - const window = new NativeWindowRef(); - const routerStub = new RouterStub(); + let window; + 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(); })); diff --git a/src/app/core/auth/auth.service.ts b/src/app/core/auth/auth.service.ts index d0547a1b9c..7874eebea8 100644 --- a/src/app/core/auth/auth.service.ts +++ b/src/app/core/auth/auth.service.ts @@ -238,7 +238,6 @@ export class AuthService { throw(new Error('auth.errors.invalid-user')); } })) - } /** From 1bf239f8464fa5886cfaf61b491fb4ac97965f4e Mon Sep 17 00:00:00 2001 From: lotte Date: Mon, 15 Apr 2019 16:31:24 +0200 Subject: [PATCH 05/10] 61873: content type fallback --- .../dspace-rest-v2.service.spec.ts | 76 +++++++++++++++---- .../dspace-rest-v2/dspace-rest-v2.service.ts | 61 ++++++++++++--- 2 files changed, 111 insertions(+), 26 deletions(-) diff --git a/src/app/core/dspace-rest-v2/dspace-rest-v2.service.spec.ts b/src/app/core/dspace-rest-v2/dspace-rest-v2.service.spec.ts index 18b9090844..5cb2fb459e 100644 --- a/src/app/core/dspace-rest-v2/dspace-rest-v2.service.spec.ts +++ b/src/app/core/dspace-rest-v2/dspace-rest-v2.service.spec.ts @@ -3,6 +3,8 @@ import { HttpClientTestingModule, HttpTestingController } from '@angular/common/ import { 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; @@ -47,29 +49,71 @@ describe('DSpaceRESTv2Service', () => { const req = httpMock.expectOne(url); expect(req.request.method).toBe('GET'); - req.flush(mockPayload, { status: mockStatusCode, statusText: mockStatusText}); + req.flush(mockPayload, { status: mockStatusCode, statusText: mockStatusText }); + }); + it('should throw an error', () => { + dSpaceRESTv2Service.get(url).subscribe(() => undefined, (err) => { + expect(err).toEqual(mockError); + }); + const req = httpMock.expectOne(url); + expect(req.request.method).toBe('GET'); + req.error(mockError); + }); + + it('should log an error', () => { + spyOn(console, 'log'); + + dSpaceRESTv2Service.get(url).subscribe(() => undefined, (err) => { + expect(console.log).toHaveBeenCalled(); + }); + + const req = httpMock.expectOne(url); + expect(req.request.method).toBe('GET'); + 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('application/json; charset=utf-8'); }); }); - it('should throw an error', () => { - dSpaceRESTv2Service.get(url).subscribe(() => undefined, (err) => { - expect(err).toEqual(mockError); - }); - const req = httpMock.expectOne(url); - expect(req.request.method).toBe('GET'); - req.error(mockError); - }); + describe('#request', () => { + it('should return an Observable', () => { + const mockPayload = { + page: 1 + }; + const mockStatusCode = 200; + const mockStatusText = 'GREAT'; - it('should log an error', () => { - spyOn(console, 'log'); + 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); + }); - dSpaceRESTv2Service.get(url).subscribe(() => undefined, (err) => { - expect(console.log).toHaveBeenCalled(); + const req = httpMock.expectOne(url); + expect(req.request.method).toBe('POST'); + req.flush(mockPayload, { status: mockStatusCode, statusText: mockStatusText }); }); - const req = httpMock.expectOne(url); - expect(req.request.method).toBe('GET'); - req.error(mockError); + it('when a content-type header is provided, it should not use application/json', () => { + const headers = new HttpHeaders({'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('application/json; charset=utf-8'); + }); + + 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('application/json; charset=utf-8'); + }); }); describe('buildFormData', () => { diff --git a/src/app/core/dspace-rest-v2/dspace-rest-v2.service.ts b/src/app/core/dspace-rest-v2/dspace-rest-v2.service.ts index a2a9f2530c..1d72f5e4fe 100644 --- a/src/app/core/dspace-rest-v2/dspace-rest-v2.service.ts +++ b/src/app/core/dspace-rest-v2/dspace-rest-v2.service.ts @@ -1,5 +1,5 @@ -import {throwError as observableThrowError, Observable } from 'rxjs'; -import {catchError, map} from 'rxjs/operators'; +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' @@ -8,6 +8,7 @@ import { HttpObserve } from '@angular/common/http/src/client'; import { RestRequestMethod } from '../data/rest-request-method'; import { isNotEmpty } from '../../shared/empty.util'; import { DSpaceObject } from '../shared/dspace-object.model'; +import { cloneDeep } from 'lodash'; export interface HttpOptions { body?: any; @@ -38,11 +39,23 @@ export class DSpaceRESTv2Service { * An Observable containing the response from the server */ get(absoluteURL: string): Observable { - return this.http.get(absoluteURL, { observe: 'response' }).pipe( - map((res: HttpResponse) => ({ payload: res.body, statusCode: res.status, statusText: res.statusText })), + const requestOptions = { + observe: 'response' as any, + headers: new HttpHeaders({ 'Content-Type': 'application/json; charset=utf-8' }) + }; + return this.http.get(absoluteURL, requestOptions).pipe( + map((res: HttpResponse) => ({ + 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,45 @@ 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; } + // WORKING OPTION + // requestOptions.headers = ((options && options.headers) || new HttpHeaders()).set('blaat', 'bla').delete('blaat'); + + // OPTION 1 + // requestOptions.headers = ((options && options.headers) || new HttpHeaders()); + + // OPTION 2 + // requestOptions.headers = new HttpHeaders(); + // if (options && options.headers) { + // options.headers.keys().forEach((key) => { + // options.headers.getAll(key).forEach((header) => { + // // Because HttpHeaders is immutable, the set method returns a new object instead of updating the existing headers + // requestOptions.headers = requestOptions.headers.set(key, header); + // }) + // }); + // } + // + // 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', 'application/json; charset=utf-8'); + // } 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 + }); })); } From 987e5abc5810933da8765ce774e96f8d594afe51 Mon Sep 17 00:00:00 2001 From: lotte Date: Tue, 16 Apr 2019 11:47:37 +0200 Subject: [PATCH 06/10] fixed bug with lazy initialization --- .../dspace-rest-v2.service.spec.ts | 5 +-- .../dspace-rest-v2/dspace-rest-v2.service.ts | 32 +++++++------------ 2 files changed, 14 insertions(+), 23 deletions(-) diff --git a/src/app/core/dspace-rest-v2/dspace-rest-v2.service.spec.ts b/src/app/core/dspace-rest-v2/dspace-rest-v2.service.spec.ts index 5cb2fb459e..98813d8df3 100644 --- a/src/app/core/dspace-rest-v2/dspace-rest-v2.service.spec.ts +++ b/src/app/core/dspace-rest-v2/dspace-rest-v2.service.spec.ts @@ -6,7 +6,7 @@ import { DSpaceObject } from '../shared/dspace-object.model'; import { RestRequestMethod } from '../data/rest-request-method'; import { HttpHeaders } from '@angular/common/http'; -describe('DSpaceRESTv2Service', () => { +fdescribe('DSpaceRESTv2Service', () => { let dSpaceRESTv2Service: DSpaceRESTv2Service; let httpMock: HttpTestingController; const url = 'http://www.dspace.org/'; @@ -101,7 +101,8 @@ describe('DSpaceRESTv2Service', () => { }); it('when a content-type header is provided, it should not use application/json', () => { - const headers = new HttpHeaders({'Content-Type': 'text/html'}); + let headers = new HttpHeaders(); + headers = headers.set('Content-Type', 'text/html'); dSpaceRESTv2Service.request(RestRequestMethod.POST, url, {}, { headers }).subscribe(); const req = httpMock.expectOne(url); diff --git a/src/app/core/dspace-rest-v2/dspace-rest-v2.service.ts b/src/app/core/dspace-rest-v2/dspace-rest-v2.service.ts index 1d72f5e4fe..f93e40e403 100644 --- a/src/app/core/dspace-rest-v2/dspace-rest-v2.service.ts +++ b/src/app/core/dspace-rest-v2/dspace-rest-v2.service.ts @@ -8,7 +8,6 @@ import { HttpObserve } from '@angular/common/http/src/client'; import { RestRequestMethod } from '../data/rest-request-method'; import { isNotEmpty } from '../../shared/empty.util'; import { DSpaceObject } from '../shared/dspace-object.model'; -import { cloneDeep } from 'lodash'; export interface HttpOptions { body?: any; @@ -41,8 +40,9 @@ export class DSpaceRESTv2Service { get(absoluteURL: string): Observable { const requestOptions = { observe: 'response' as any, - headers: new HttpHeaders({ 'Content-Type': 'application/json; charset=utf-8' }) + headers: new HttpHeaders() }; + requestOptions.headers = requestOptions.headers.set( 'Content-Type', 'application/json; charset=utf-8' ); return this.http.get(absoluteURL, requestOptions).pipe( map((res: HttpResponse) => ({ payload: res.body, @@ -82,27 +82,17 @@ export class DSpaceRESTv2Service { if (options && options.responseType) { requestOptions.responseType = options.responseType; } - // WORKING OPTION - // requestOptions.headers = ((options && options.headers) || new HttpHeaders()).set('blaat', 'bla').delete('blaat'); - // OPTION 1 - // requestOptions.headers = ((options && options.headers) || new HttpHeaders()); + if (options && options.headers) { + requestOptions.headers = Object.assign(new HttpHeaders(), options.headers); + } else { + requestOptions.headers = new HttpHeaders(); + } - // OPTION 2 - // requestOptions.headers = new HttpHeaders(); - // if (options && options.headers) { - // options.headers.keys().forEach((key) => { - // options.headers.getAll(key).forEach((header) => { - // // Because HttpHeaders is immutable, the set method returns a new object instead of updating the existing headers - // requestOptions.headers = requestOptions.headers.set(key, header); - // }) - // }); - // } - // - // 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', 'application/json; charset=utf-8'); - // } + 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', 'application/json; charset=utf-8'); + } return this.http.request(method, url, requestOptions).pipe( map((res) => ({ payload: res.body, From 57cd177247e412130d9dac9294bdc87afffc26c3 Mon Sep 17 00:00:00 2001 From: lotte Date: Tue, 16 Apr 2019 11:50:33 +0200 Subject: [PATCH 07/10] cleanup --- .../core/dspace-rest-v2/dspace-rest-v2.service.spec.ts | 8 ++++---- src/app/core/dspace-rest-v2/dspace-rest-v2.service.ts | 6 +++--- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/app/core/dspace-rest-v2/dspace-rest-v2.service.spec.ts b/src/app/core/dspace-rest-v2/dspace-rest-v2.service.spec.ts index 98813d8df3..d8827297b3 100644 --- a/src/app/core/dspace-rest-v2/dspace-rest-v2.service.spec.ts +++ b/src/app/core/dspace-rest-v2/dspace-rest-v2.service.spec.ts @@ -1,7 +1,7 @@ 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'; @@ -76,7 +76,7 @@ fdescribe('DSpaceRESTv2Service', () => { dSpaceRESTv2Service.request(RestRequestMethod.POST, url, {}).subscribe(); const req = httpMock.expectOne(url); - expect(req.request.headers.get('Content-Type')).toContain('application/json; charset=utf-8'); + expect(req.request.headers.get('Content-Type')).toContain(DEFAULT_CONTENT_TYPE); }); }); @@ -106,14 +106,14 @@ fdescribe('DSpaceRESTv2Service', () => { dSpaceRESTv2Service.request(RestRequestMethod.POST, url, {}, { headers }).subscribe(); const req = httpMock.expectOne(url); - expect(req.request.headers.get('Content-Type')).not.toContain('application/json; charset=utf-8'); + 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('application/json; charset=utf-8'); + expect(req.request.headers.get('Content-Type')).toContain(DEFAULT_CONTENT_TYPE); }); }); diff --git a/src/app/core/dspace-rest-v2/dspace-rest-v2.service.ts b/src/app/core/dspace-rest-v2/dspace-rest-v2.service.ts index f93e40e403..84d4d559b8 100644 --- a/src/app/core/dspace-rest-v2/dspace-rest-v2.service.ts +++ b/src/app/core/dspace-rest-v2/dspace-rest-v2.service.ts @@ -9,6 +9,7 @@ import { RestRequestMethod } from '../data/rest-request-method'; import { 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; @@ -40,9 +41,8 @@ export class DSpaceRESTv2Service { get(absoluteURL: string): Observable { const requestOptions = { observe: 'response' as any, - headers: new HttpHeaders() + headers: new HttpHeaders({'Content-Type': DEFAULT_CONTENT_TYPE}) }; - requestOptions.headers = requestOptions.headers.set( 'Content-Type', 'application/json; charset=utf-8' ); return this.http.get(absoluteURL, requestOptions).pipe( map((res: HttpResponse) => ({ payload: res.body, @@ -91,7 +91,7 @@ export class DSpaceRESTv2Service { 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', 'application/json; charset=utf-8'); + requestOptions.headers = requestOptions.headers.set('Content-Type', DEFAULT_CONTENT_TYPE); } return this.http.request(method, url, requestOptions).pipe( map((res) => ({ From 9dbd2eec710c4c7e1ac21278de07cc8ecc5484e5 Mon Sep 17 00:00:00 2001 From: lotte Date: Tue, 16 Apr 2019 13:51:58 +0200 Subject: [PATCH 08/10] moved fix to request service --- src/app/core/data/request.service.ts | 14 +++++++++++++- .../core/dspace-rest-v2/dspace-rest-v2.service.ts | 8 ++++---- 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/src/app/core/data/request.service.ts b/src/app/core/data/request.service.ts index fd463047f1..9f79d7539b 100644 --- a/src/app/core/data/request.service.ts +++ b/src/app/core/data/request.service.ts @@ -2,7 +2,7 @@ import { Injectable } from '@angular/core'; import { createSelector, MemoizedSelector, select, Store } from '@ngrx/store'; import { Observable, race as observableRace } from 'rxjs'; -import { filter, mergeMap, take } from 'rxjs/operators'; +import { filter, map, mergeMap, take } from 'rxjs/operators'; import { AppState } from '../../app.reducer'; import { hasValue, isNotEmpty } from '../../shared/empty.util'; @@ -23,6 +23,8 @@ import { CommitSSBAction } from '../cache/server-sync-buffer.actions'; import { RestRequestMethod } from './rest-request-method'; import { AddToIndexAction, RemoveFromIndexBySubstringAction } from '../index/index.actions'; import { coreSelector } from '../core.selectors'; +import { HttpHeaders } from '@angular/common/http'; +import { cloneDeep } from 'lodash'; /** * The base selector function to select the request state in the store @@ -118,6 +120,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; + }) ); } diff --git a/src/app/core/dspace-rest-v2/dspace-rest-v2.service.ts b/src/app/core/dspace-rest-v2/dspace-rest-v2.service.ts index 84d4d559b8..204c782e79 100644 --- a/src/app/core/dspace-rest-v2/dspace-rest-v2.service.ts +++ b/src/app/core/dspace-rest-v2/dspace-rest-v2.service.ts @@ -6,7 +6,7 @@ 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'; @@ -83,10 +83,10 @@ export class DSpaceRESTv2Service { requestOptions.responseType = options.responseType; } - if (options && options.headers) { - requestOptions.headers = Object.assign(new HttpHeaders(), options.headers); - } else { + if (hasNoValue(options) || hasNoValue(options.headers)) { requestOptions.headers = new HttpHeaders(); + } else { + requestOptions.headers = options.headers; } if (!requestOptions.headers.has('Content-Type')) { From 1e7bc8b3013de5840c71a325ef35c6fa90962942 Mon Sep 17 00:00:00 2001 From: lotte Date: Tue, 16 Apr 2019 13:53:24 +0200 Subject: [PATCH 09/10] removed focus from test --- src/app/core/dspace-rest-v2/dspace-rest-v2.service.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/app/core/dspace-rest-v2/dspace-rest-v2.service.spec.ts b/src/app/core/dspace-rest-v2/dspace-rest-v2.service.spec.ts index d8827297b3..a7aba56a3b 100644 --- a/src/app/core/dspace-rest-v2/dspace-rest-v2.service.spec.ts +++ b/src/app/core/dspace-rest-v2/dspace-rest-v2.service.spec.ts @@ -6,7 +6,7 @@ import { DSpaceObject } from '../shared/dspace-object.model'; import { RestRequestMethod } from '../data/rest-request-method'; import { HttpHeaders } from '@angular/common/http'; -fdescribe('DSpaceRESTv2Service', () => { +describe('DSpaceRESTv2Service', () => { let dSpaceRESTv2Service: DSpaceRESTv2Service; let httpMock: HttpTestingController; const url = 'http://www.dspace.org/'; From 37e0afc2d5ba96762ded76e7e2123ac51d909138 Mon Sep 17 00:00:00 2001 From: Art Lowel Date: Tue, 23 Apr 2019 13:56:49 +0200 Subject: [PATCH 10/10] upgrade morgan to 1.9.1 to fix CVE-2019-5413 --- package.json | 2 +- yarn.lock | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/package.json b/package.json index 46eeb7be2f..1f75da6c8b 100644 --- a/package.json +++ b/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", diff --git a/yarn.lock b/yarn.lock index 786c9ade3e..50cf67c8d8 100644 --- a/yarn.lock +++ b/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: - version "1.9.0" - resolved "https://registry.yarnpkg.com/morgan/-/morgan-1.9.0.tgz#d01fa6c65859b76fcf31b3cb53a3821a311d8051" - integrity sha1-0B+mxlhZt2/PMbPLU6OCGjEdgFE= +morgan@^1.9.1: + version "1.9.1" + resolved "https://registry.yarnpkg.com/morgan/-/morgan-1.9.1.tgz#0a8d16734a1d9afbc824b99df87e738e58e2da59" + 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"