Merge branch 'response-cache-refactoring' into w2p-55946_Item-mapping-on-item-level

Conflicts:
	src/app/core/cache/builders/remote-data-build.service.ts

package.json

@@ -25,7 +25,7 @@
     "prebuild:prod": "yarn run prebuild",
     "build": "webpack --progress --mode development",
     "build:aot": "webpack --env.aot --env.server --mode development && webpack --env.aot --env.client --mode development",
-    "build:prod": "webpack --env.aot --env.server --env.production && webpack --env.aot --env.client --env.production",
+    "build:prod": "webpack --env.aot --env.server --mode production && webpack --env.aot --env.client --mode production",
     "postbuild:prod": "yarn run rollup",
     "rollup": "rollup -c rollup.config.js",
     "prestart": "yarn run build:prod",

src/app/+item-page/field-components/metadata-field-wrapper/metadata-field-wrapper.component.html

@@ -1,7 +1,5 @@
-<div class="simple-view-element">
-  <span *ngIf="content.children.length != 0">
-    <h5 class="simple-view-element-header" *ngIf="label">{{ label }}</h5>
-  </span>
+<div class="simple-view-element" [class.d-none]="content.textContent.trim().length === 0">
+  <h5 class="simple-view-element-header" *ngIf="label">{{ label }}</h5>
   <div #content class="simple-view-element-body">
     <ng-content></ng-content>
   </div>

src/app/+item-page/field-components/metadata-field-wrapper/metadata-field-wrapper.component.spec.ts

@@ -7,7 +7,8 @@ import { MetadataFieldWrapperComponent } from './metadata-field-wrapper.componen
 @Component({
     selector: 'ds-component-with-content',
     template: '<ds-metadata-field-wrapper [label]="\'test label\'">\n' +
-      '    <div class="my content">\n' +
+      '    <div class="my-content">\n' +
+      '    <span></span>\n' +
       '    </div>\n' +
       '</ds-metadata-field-wrapper>'
 })
@@ -30,25 +31,37 @@ describe('MetadataFieldWrapperComponent', () => {
 
   const wrapperSelector = '.simple-view-element';
   const labelSelector = '.simple-view-element-header';
+  const contentSelector = '.my-content';
 
   it('should create', () => {
     expect(component).toBeDefined();
   });
 
-  it('should not show a label when there is no content', () => {
+  it('should not show the component when there is no content', () => {
     component.label = 'test label';
     fixture.detectChanges();
-    const debugLabel = fixture.debugElement.query(By.css(labelSelector));
-    expect(debugLabel).toBeNull();
+    const parentNative = fixture.nativeElement;
+    const nativeWrapper = parentNative.querySelector(wrapperSelector);
+    expect(nativeWrapper.classList.contains('d-none')).toBe(true);
   });
 
-  it('should show a label when there is content', () => {
+  it('should not show the component when there is DOM content but no text', () => {
     const parentFixture = TestBed.createComponent(ContentComponent);
     parentFixture.detectChanges();
-    const parentComponent = parentFixture.componentInstance;
     const parentNative = parentFixture.nativeElement;
-    const nativeLabel = parentNative.querySelector(labelSelector);
-    expect(nativeLabel.textContent).toContain('test label');
+    const nativeWrapper = parentNative.querySelector(wrapperSelector);
+    expect(nativeWrapper.classList.contains('d-none')).toBe(true);
+  });
+
+  it('should show the component when there is text content', () => {
+    const parentFixture = TestBed.createComponent(ContentComponent);
+    parentFixture.detectChanges();
+    const parentNative = parentFixture.nativeElement;
+    const nativeContent = parentNative.querySelector(contentSelector);
+    nativeContent.textContent = 'lorem ipsum';
+    const nativeWrapper = parentNative.querySelector(wrapperSelector);
+    parentFixture.detectChanges();
+    expect(nativeWrapper.classList.contains('d-none')).toBe(false);
   });
 
 });

src/app/+search-page/search-filters/search-filter/search-facet-filter/search-facet-filter.component.ts

@@ -6,7 +6,7 @@ import {
   Subject,
   Subscription
 } from 'rxjs';
-import { switchMap, distinctUntilChanged, first, map } from 'rxjs/operators';
+import { switchMap, distinctUntilChanged, first, map, take } from 'rxjs/operators';
 import { animate, state, style, transition, trigger } from '@angular/animations';
 import { Component, Inject, OnDestroy, OnInit } from '@angular/core';
 import { Router } from '@angular/router';
@@ -126,7 +126,7 @@ export class SearchFacetFilterComponent implements OnInit, OnDestroy {
         this.animationState = 'ready';
         this.filterValues$.next(rd);
       }));
-      this.subs.push(newValues$.pipe(first()).subscribe((rd) => {
+      this.subs.push(newValues$.pipe(take(1)).subscribe((rd) => {
         this.isLastPage$.next(hasNoValue(rd.payload.next))
       }));
     }));
@@ -189,7 +189,7 @@ export class SearchFacetFilterComponent implements OnInit, OnDestroy {
    * @param data The string from the input field
    */
   onSubmit(data: any) {
-    this.selectedValues.pipe(first()).subscribe((selectedValues) => {
+    this.selectedValues.pipe(take(1)).subscribe((selectedValues) => {
         if (isNotEmpty(data)) {
           this.router.navigate([this.getSearchLink()], {
             queryParams:
@@ -258,7 +258,7 @@ export class SearchFacetFilterComponent implements OnInit, OnDestroy {
    */
   findSuggestions(data): void {
     if (isNotEmpty(data)) {
-      this.searchConfigService.searchOptions.pipe(first()).subscribe(
+      this.searchConfigService.searchOptions.pipe(take(1)).subscribe(
         (options) => {
           this.filterSearchResults = this.searchService.getFacetValuesFor(this.filterConfig, 1, options, data.toLowerCase())
             .pipe(

src/app/+search-page/search-filters/search-filter/search-filter.component.ts

@@ -1,5 +1,5 @@
 
-import {first} from 'rxjs/operators';
+import { first, take } from 'rxjs/operators';
 import { Component, Input, OnInit } from '@angular/core';
 import { SearchFilterConfig } from '../../search-service/search-filter-config.model';
 import { SearchFilterService } from './search-filter.service';
@@ -37,7 +37,7 @@ export class SearchFilterComponent implements OnInit {
    * Else, the filter should initially be collapsed
    */
   ngOnInit() {
-    this.getSelectedValues().pipe(first()).subscribe((isActive) => {
+    this.getSelectedValues().pipe(take(1)).subscribe((isActive) => {
       if (this.filter.isOpenByDefault || isNotEmpty(isActive)) {
         this.initialExpand();
       } else {

src/app/app.component.ts

@@ -64,7 +64,7 @@ export class AppComponent implements OnInit, AfterViewInit {
 
     // Whether is not authenticathed try to retrieve a possible stored auth token
     this.store.pipe(select(isAuthenticated),
-      first(),
+      take(1),
       filter((authenticated) => !authenticated)
     ).subscribe((authenticated) => this.authService.checkAuthenticationToken());
 

src/app/core/auth/auth.effects.ts

@@ -47,7 +47,7 @@ export class AuthEffects {
       ofType(AuthActionTypes.AUTHENTICATE),
       switchMap((action: AuthenticateAction) => {
         return this.authService.authenticate(action.payload.email, action.payload.password).pipe(
-          first(),
+          take(1),
           map((response: AuthStatus) => new AuthenticationSuccessAction(response.token)),
           catchError((error) => observableOf(new AuthenticationErrorAction(error)))
         );
@@ -127,7 +127,7 @@ export class AuthEffects {
     switchMap(() => {
       return this.store.pipe(
         select(isAuthenticated),
-        first(),
+        take(1),
         filter((authenticated) => !authenticated),
         tap(() => this.authService.removeToken()),
         tap(() => this.authService.resetAuthenticationError())

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

@@ -148,7 +148,7 @@ describe('AuthService test', () => {
           (state as any).core = Object.create({});
           (state as any).core.auth = authenticatedState;
         });
-      authService = new AuthService({}, window, authReqService, router, cookieService, store, rdbService);
+      authService = new AuthService({}, window, undefined, authReqService, router, cookieService, store, rdbService);
     }));
 
     it('should return true when user is logged in', () => {
@@ -207,7 +207,7 @@ describe('AuthService test', () => {
           (state as any).core = Object.create({});
           (state as any).core.auth = authenticatedState;
         });
-      authService = new AuthService({}, window, authReqService, router, cookieService, store, rdbService);
+      authService = new AuthService({}, window, undefined, authReqService, router, cookieService, store, rdbService);
       storage = (authService as any).storage;
       spyOn(storage, 'get');
       spyOn(storage, 'remove');

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

@@ -9,10 +9,10 @@ import {
   take,
   withLatestFrom
 } from 'rxjs/operators';
-import { Inject, Injectable } from '@angular/core';
+import { Inject, Injectable, Optional } from '@angular/core';
 import { PRIMARY_OUTLET, Router, UrlSegmentGroup, UrlTree } from '@angular/router';
 import { HttpHeaders } from '@angular/common/http';
-import { REQUEST } from '@nguniversal/express-engine/tokens';
+import { REQUEST, RESPONSE } from '@nguniversal/express-engine/tokens';
 
 import { RouterReducerState } from '@ngrx/router-store';
 import { select, Store } from '@ngrx/store';
@@ -59,6 +59,7 @@ export class AuthService {
   constructor(@Inject(REQUEST) protected req: any,
               @Inject(NativeWindowService) protected _window: NativeWindowRef,
               protected authRequestService: AuthRequestService,
+              @Optional() @Inject(RESPONSE) private response: any,
               protected router: Router,
               protected storage: CookieService,
               protected store: Store<AppState>,
@@ -276,7 +277,7 @@ export class AuthService {
   public isTokenExpiring(): Observable<boolean> {
     return this.store.pipe(
       select(isTokenRefreshing),
-      first(),
+      take(1),
       map((isRefreshing: boolean) => {
         if (this.isTokenExpired() || isRefreshing) {
           return false;
@@ -345,6 +346,10 @@ export class AuthService {
     if (this._window.nativeWindow.location) {
       // Hard redirect to login page, so that all state is definitely lost
       this._window.nativeWindow.location.href = redirectUrl;
+    } else if (this.response) {
+      if (!this.response._headerSent) {
+        this.response.redirect(302, redirectUrl);
+      }
     } else {
       this.router.navigateByUrl(redirectUrl);
     }
@@ -355,16 +360,12 @@ export class AuthService {
    */
   public redirectToPreviousUrl() {
     this.getRedirectUrl().pipe(
-      first())
+      take(1))
       .subscribe((redirectUrl) => {
+
         if (isNotEmpty(redirectUrl)) {
           this.clearRedirectUrl();
-
-          // override the route reuse strategy
-          this.router.routeReuseStrategy.shouldReuseRoute = () => {
-            return false;
-          };
-          this.router.navigated = false;
+          this.router.onSameUrlNavigation = 'reload';
           const url = decodeURIComponent(redirectUrl);
           this.router.navigateByUrl(url);
           /* TODO Reenable hard redirect when REST API can handle x-forwarded-for, see https://github.com/DSpace/DSpace/pull/2207 */

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

@@ -1,4 +1,4 @@
-import { first, map, switchMap } from 'rxjs/operators';
+import { first, map, switchMap, take } from 'rxjs/operators';
 import { Injectable } from '@angular/core';
 
 import { Observable } from 'rxjs';
@@ -62,7 +62,7 @@ export class ServerAuthService extends AuthService {
    */
   public redirectToPreviousUrl() {
     this.getRedirectUrl().pipe(
-      first())
+      take(1))
       .subscribe((redirectUrl) => {
         if (isNotEmpty(redirectUrl)) {
           // override the route reuse strategy

src/app/core/cache/builders/remote-data-build.service.ts

@@ -5,8 +5,16 @@ import {
   race as observableRace
 } from 'rxjs';
 import { Injectable } from '@angular/core';
-import { distinctUntilChanged, first, flatMap, map, startWith, switchMap } from 'rxjs/operators';
-import { hasValue, hasValueOperator, isEmpty, isNotEmpty, isNotEmptyOperator } from '../../../shared/empty.util';
+import {
+  distinctUntilChanged,
+  first,
+  flatMap,
+  map,
+  startWith,
+  switchMap,
+  take
+} from 'rxjs/operators';
+import { hasValue, hasValueOperator, isEmpty, isNotEmpty } from '../../../shared/empty.util';
 import { PaginatedList } from '../../data/paginated-list';
 import { RemoteData } from '../../data/remote-data';
 import { RemoteDataError } from '../../data/remote-data-error';
@@ -44,7 +52,7 @@ export class RemoteDataBuildService {
       href$.pipe(getRequestFromRequestHref(this.requestService)),
       requestUUID$.pipe(getRequestFromRequestUUID(this.requestService)),
     ).pipe(
-      first()
+      take(1)
     );
 
     // always use self link if that is cached, only if it isn't, get it via the response.

src/app/core/cache/object-cache.service.ts

@@ -1,6 +1,6 @@
 import { combineLatest as observableCombineLatest, Observable } from 'rxjs';
 
-import { distinctUntilChanged, filter, first, map, mergeMap, } from 'rxjs/operators';
+import { distinctUntilChanged, filter, first, map, mergeMap, take, } from 'rxjs/operators';
 import { Injectable } from '@angular/core';
 import { MemoizedSelector, select, Store } from '@ngrx/store';
 import { IndexName } from '../index/index.reducer';
@@ -165,7 +165,7 @@ export class ObjectCacheService {
 
     this.store.pipe(
       select(selfLinkFromUuidSelector(uuid)),
-      first()
+      take(1)
     ).subscribe((selfLink: string) => result = this.hasBySelfLink(selfLink));
 
     return result;
@@ -184,7 +184,7 @@ export class ObjectCacheService {
     let result = false;
 
     this.store.pipe(select(entryFromSelfLinkSelector(selfLink)),
-      first()
+      take(1)
     ).subscribe((entry: ObjectCacheEntry) => result = this.isValid(entry));
 
     return result;

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

@@ -140,7 +140,7 @@ export class ErrorResponse extends RestResponse {
 
   constructor(error: RequestError) {
     super(false, error.statusText);
-    // console.error(error);
+    console.error(error);
     this.errorMessage = error.message;
   }
 }

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

@@ -1,4 +1,4 @@
-import { delay, exhaustMap, first, map, switchMap, tap } from 'rxjs/operators';
+import { delay, exhaustMap, first, map, switchMap, take, tap } from 'rxjs/operators';
 import { Inject, Injectable } from '@angular/core';
 import { Actions, Effect, ofType } from '@ngrx/effects';
 import {
@@ -56,7 +56,7 @@ export class ServerSyncBufferEffects {
       switchMap((action: CommitSSBAction) => {
         return this.store.pipe(
           select(serverSyncBufferSelector()),
-          first(), /* necessary, otherwise delay will not have any effect after the first run */
+          take(1), /* necessary, otherwise delay will not have any effect after the first run */
           switchMap((bufferState: ServerSyncBufferState) => {
             const actions: Array<Observable<Action>> = bufferState.buffer
               .filter((entry: ServerSyncBufferEntry) => {
@@ -95,7 +95,7 @@ export class ServerSyncBufferEffects {
    * @returns {Observable<Action>} ApplyPatchObjectCacheAction to be dispatched
    */
   private applyPatch(href: string): Observable<Action> {
-    const patchObject = this.objectCache.getBySelfLink(href).pipe(first());
+    const patchObject = this.objectCache.getBySelfLink(href).pipe(take(1));
 
     return patchObject.pipe(
       map((object) => {

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

@@ -49,23 +49,6 @@ export abstract class ComColDataService<TNormalized extends NormalizedObject, TD
           this.requestService.configure(request);
         }));
 
-      // return scopeCommunityHrefObs.pipe(
-      //   mergeMap((href: string) => this.responseCache.get(href)),
-      //   map((entry: ResponseCacheEntry) => entry.response),
-      //   mergeMap((response) => {
-      //     if (response.isSuccessful) {
-      //       const community$: Observable<NormalizedCommunity> = this.objectCache.getByUUID(scopeID);
-      //       return community$.pipe(
-      //         map((community) => community._links[linkPath]),
-      //         filter((href) => isNotEmpty(href)),
-      //         distinctUntilChanged()
-      //       );
-      //     } else if (!response.isSuccessful) {
-      //       return observableThrowError(new Error(`The Community with scope ${scopeID} couldn't be retrieved`))
-      //     }
-      //   }),
-      //   distinctUntilChanged()
-      // );
       const responses = scopeCommunityHrefObs.pipe(
         mergeMap((href: string) => this.requestService.getByHref(href)),
         getResponseFromEntry()

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

@@ -1,4 +1,4 @@
-import { delay, distinctUntilChanged, filter, first, map, take, tap } from 'rxjs/operators';
+import { delay, distinctUntilChanged, filter, find, first, map, take, tap } from 'rxjs/operators';
 import { Observable } from 'rxjs';
 import { Store } from '@ngrx/store';
 import { hasValue, isNotEmpty } from '../../shared/empty.util';
@@ -74,7 +74,7 @@ export abstract class DataService<TNormalized extends NormalizedObject, TDomain>
       map((endpoint: string) => this.getFindByIDHref(endpoint, id)));
 
     hrefObs.pipe(
-      first((href: string) => hasValue(href)))
+      find((href: string) => hasValue(href)))
       .subscribe((href: string) => {
         const request = new FindByIDRequest(this.requestService.generateRequestId(), href, id);
         this.requestService.configure(request);

src/app/core/metadata/metadata.service.ts

@@ -1,4 +1,12 @@
-import { catchError, distinctUntilKeyChanged, filter, first, map, take } from 'rxjs/operators';
+import {
+  catchError,
+  distinctUntilKeyChanged,
+  filter,
+  find,
+  first,
+  map,
+  take
+} from 'rxjs/operators';
 import { Inject, Injectable } from '@angular/core';
 import { ActivatedRoute, NavigationEnd, Router } from '@angular/router';
 
@@ -261,7 +269,7 @@ export class MetadataService {
       const item = this.currentObject.value as Item;
       item.getFiles()
         .pipe(
-          first((files) => isNotEmpty(files)),
+          find((files) => isNotEmpty(files)),
           catchError((error) => {
             console.debug(error.message);
             return []
@@ -269,7 +277,7 @@ export class MetadataService {
         .subscribe((bitstreams: Bitstream[]) => {
           for (const bitstream of bitstreams) {
             bitstream.format.pipe(
-              first(),
+              take(1),
               catchError((error: Error) => {
                 console.debug(error.message);
                 return []

src/app/core/shared/operators.ts

@@ -1,5 +1,5 @@
 import { Observable } from 'rxjs';
-import { filter, first, flatMap, map, tap } from 'rxjs/operators';
+import { filter, find, first, flatMap, map, tap } from 'rxjs/operators';
 import { hasValue, hasValueOperator, isNotEmpty } from '../../shared/empty.util';
 import { DSOSuccessResponse, RestResponse } from '../cache/response.models';
 import { RemoteData } from '../data/remote-data';
@@ -60,7 +60,7 @@ export const getRemoteDataPayload = () =>
 
 export const getSucceededRemoteData = () =>
   <T>(source: Observable<RemoteData<T>>): Observable<RemoteData<T>> =>
-    source.pipe(first((rd: RemoteData<T>) => rd.hasSucceeded));
+    source.pipe(find((rd: RemoteData<T>) => rd.hasSucceeded));
 
 export const toDSpaceObjectListRD = () =>
   <T extends DSpaceObject>(source: Observable<RemoteData<PaginatedList<SearchResult<T>>>>): Observable<RemoteData<PaginatedList<T>>> =>

src/app/shared/form/form.service.spec.ts

@@ -2,7 +2,11 @@ import { Store, StoreModule } from '@ngrx/store';
 import { async, inject, TestBed } from '@angular/core/testing';
 import { AbstractControl, FormControl, FormGroup, Validators } from '@angular/forms';
 
-import { DynamicFormControlModel, DynamicInputModel } from '@ng-dynamic-forms/core';
+import {
+  DynamicFormControlModel,
+  DynamicFormGroupModel,
+  DynamicInputModel
+} from '@ng-dynamic-forms/core';
 
 import { FormService } from './form.service';
 import { FormBuilderService } from './builder/form-builder.service';
@@ -37,30 +41,30 @@ describe('FormService test suite', () => {
     }),
     new DynamicInputModel({ id: 'date' }),
     new DynamicInputModel({ id: 'description' }),
-    // new DynamicFormGroupModel({
-    //
-    //   id: 'addressLocation',
-    //   group: [
-    //     new DynamicInputModel({
-    //
-    //       id: 'zipCode',
-    //       label: 'Zip Code',
-    //       placeholder: 'ZIP'
-    //     }),
-    //     new DynamicInputModel({
-    //
-    //       id: 'state',
-    //       label: 'State',
-    //       placeholder: 'State'
-    //     }),
-    //     new DynamicInputModel({
-    //
-    //       id: 'city',
-    //       label: 'City',
-    //       placeholder: 'City'
-    //     })
-    //   ]
-    // }),
+    new DynamicFormGroupModel({
+
+      id: 'addressLocation',
+      group: [
+        new DynamicInputModel({
+
+          id: 'zipCode',
+          label: 'Zip Code',
+          placeholder: 'ZIP'
+        }),
+        new DynamicInputModel({
+
+          id: 'state',
+          label: 'State',
+          placeholder: 'State'
+        }),
+        new DynamicInputModel({
+
+          id: 'city',
+          label: 'City',
+          placeholder: 'City'
+        })
+      ]
+    }),
   ];
 
   let controls;

src/server.ts

@@ -17,6 +17,7 @@ import { ngExpressEngine } from '@nguniversal/express-engine';
 
 import { ROUTES } from './routes';
 import { ENV_CONFIG } from './config';
+import { REQUEST, RESPONSE } from '@nguniversal/express-engine/tokens';
 
 export function startServer(bootstrap: Type<{}> | NgModuleFactory<{}>) {
   const app = express();
@@ -31,9 +32,21 @@ export function startServer(bootstrap: Type<{}> | NgModuleFactory<{}>) {
   app.use(cookieParser());
   app.use(bodyParser.json());
 
-  app.engine('html', ngExpressEngine({
-    bootstrap: bootstrap
-  }));
+  app.engine('html', (_, options, callback) =>
+    ngExpressEngine({
+      bootstrap: bootstrap,
+      providers: [
+        {
+          provide: REQUEST,
+          useValue: options.req,
+        },
+        {
+          provide: RESPONSE,
+          useValue: options.req.res,
+        },
+      ],
+    })(_, options, callback)
+  );
 
   app.set('view engine', 'html');
   app.set('views', 'src');
@@ -53,9 +66,10 @@ export function startServer(bootstrap: Type<{}> | NgModuleFactory<{}>) {
   function ngApp(req, res) {
 
     function onHandleError(parentZoneDelegate, currentZone, targetZone, error) {
-      console.error('Error:', error);
-      console.warn('Error in SSR, serving for direct CSR');
-      res.sendFile('index.csr.html', { root: './src' });
+      if (!res._headerSent)  {
+        console.warn('Error in SSR, serving for direct CSR');
+        res.sendFile('index.csr.html', { root: './src' });
+      }
     }
 
     if (ENV_CONFIG.universal.preboot) {

webpack.config.js

@@ -27,7 +27,7 @@ module.exports = function(env, options) {
       getAotPlugin('client', !!env.aot)
     ]
   });
-  if (env.production) {
+  if (options.mode === 'production') {
     serverConfig = webpackMerge({}, serverConfig, prodPartial);
     clientConfig = webpackMerge({}, clientConfig, prodPartial);
   }