Merge pull request #1465 from atmire/move-correlation-id-to-store

Move correlation ID to store

src/app/app.module.ts

@@ -55,9 +55,6 @@ import { ThemedBreadcrumbsComponent } from './breadcrumbs/themed-breadcrumbs.com
 import { ThemedHeaderNavbarWrapperComponent } from './header-nav-wrapper/themed-header-navbar-wrapper.component';
 import { IdleModalComponent } from './shared/idle-modal/idle-modal.component';
 
-import { UUIDService } from './core/shared/uuid.service';
-import { CookieService } from './core/services/cookie.service';
-
 import { AppConfig, APP_CONFIG } from '../config/app-config.interface';
 
 export function getConfig() {
@@ -156,21 +153,6 @@ const PROVIDERS = [
     useClass: LogInterceptor,
     multi: true
   },
-  // insert the unique id of the user that is using the application utilizing cookies
-  {
-    provide: APP_INITIALIZER,
-    useFactory: (cookieService: CookieService, uuidService: UUIDService) => {
-      const correlationId = cookieService.get('CORRELATION-ID');
-
-      // Check if cookie exists, if don't, set it with unique id
-      if (!correlationId) {
-        cookieService.set('CORRELATION-ID', uuidService.generate());
-      }
-      return () => true;
-    },
-    multi: true,
-    deps: [CookieService, UUIDService]
-  },
   {
     provide: DYNAMIC_ERROR_MESSAGES_MATCHER,
     useValue: ValidateEmailErrorStateMatcher

src/app/app.reducer.ts

@@ -49,6 +49,7 @@ import {
 import { sidebarReducer, SidebarState } from './shared/sidebar/sidebar.reducer';
 import { truncatableReducer, TruncatablesState } from './shared/truncatable/truncatable.reducer';
 import { ThemeState, themeReducer } from './shared/theme-support/theme.reducer';
+import { correlationIdReducer } from './correlation-id/correlation-id.reducer';
 
 export interface AppState {
   router: fromRouter.RouterReducerState;
@@ -69,6 +70,7 @@ export interface AppState {
   communityList: CommunityListState;
   epeopleRegistry: EPeopleRegistryState;
   groupRegistry: GroupRegistryState;
+  correlationId: string;
 }
 
 export const appReducers: ActionReducerMap<AppState> = {
@@ -90,6 +92,7 @@ export const appReducers: ActionReducerMap<AppState> = {
   communityList: CommunityListReducer,
   epeopleRegistry: ePeopleRegistryReducer,
   groupRegistry: groupRegistryReducer,
+  correlationId: correlationIdReducer
 };
 
 export const routerStateSelector = (state: AppState) => state.router;

src/app/core/log/log.interceptor.spec.ts

@@ -9,12 +9,17 @@ import { RestRequestMethod } from '../data/rest-request-method';
 import { CookieService } from '../services/cookie.service';
 import { CookieServiceMock } from '../../shared/mocks/cookie.service.mock';
 import { RouterStub } from '../../shared/testing/router.stub';
+import { CorrelationIdService } from '../../correlation-id/correlation-id.service';
+import { UUIDService } from '../shared/uuid.service';
+import { StoreModule } from '@ngrx/store';
+import { appReducers, storeModuleConfig } from '../../app.reducer';
 
 
 describe('LogInterceptor', () => {
   let service: DspaceRestService;
   let httpMock: HttpTestingController;
   let cookieService: CookieService;
+  let correlationIdService: CorrelationIdService;
   const router = Object.assign(new RouterStub(),{url : '/statistics'});
 
   // Mock payload/statuses are dummy content as we are not testing the results
@@ -28,7 +33,10 @@ describe('LogInterceptor', () => {
 
   beforeEach(() => {
     TestBed.configureTestingModule({
-      imports: [HttpClientTestingModule],
+      imports: [
+        HttpClientTestingModule,
+        StoreModule.forRoot(appReducers, storeModuleConfig),
+      ],
       providers: [
         DspaceRestService,
         // LogInterceptor,
@@ -39,14 +47,18 @@ describe('LogInterceptor', () => {
         },
         { provide: CookieService, useValue: new CookieServiceMock() },
         { provide: Router, useValue: router },
+        { provide: CorrelationIdService, useClass: CorrelationIdService },
+        { provide: UUIDService, useClass: UUIDService },
       ],
     });
 
-    service = TestBed.get(DspaceRestService);
+    service = TestBed.inject(DspaceRestService);
-    httpMock = TestBed.get(HttpTestingController);
+    httpMock = TestBed.inject(HttpTestingController);
-    cookieService = TestBed.get(CookieService);
+    cookieService = TestBed.inject(CookieService);
+    correlationIdService = TestBed.inject(CorrelationIdService);
 
     cookieService.set('CORRELATION-ID','123455');
+    correlationIdService.initCorrelationId();
   });
 
 

src/app/core/log/log.interceptor.ts

@@ -3,9 +3,8 @@ import { HttpEvent, HttpHandler, HttpInterceptor, HttpRequest } from '@angular/c
 import { Router } from '@angular/router';
 
 import { Observable } from 'rxjs';
-
-import { CookieService } from '../services/cookie.service';
 import { hasValue } from '../../shared/empty.util';
+import { CorrelationIdService } from '../../correlation-id/correlation-id.service';
 
 /**
  * Log Interceptor intercepting Http Requests & Responses to
@@ -15,12 +14,12 @@ import { hasValue } from '../../shared/empty.util';
 @Injectable()
 export class LogInterceptor implements HttpInterceptor {
 
-  constructor(private cookieService: CookieService, private router: Router) {}
+  constructor(private cidService: CorrelationIdService, private router: Router) {}
 
   intercept(request: HttpRequest<any>, next: HttpHandler): Observable<HttpEvent<any>> {
 
-    // Get Unique id of the user from the cookies
+    // Get the correlation id for the user from the store
-    const correlationId = this.cookieService.get('CORRELATION-ID');
+    const correlationId = this.cidService.getCorrelationId();
 
     // Add headers from the intercepted request
     let headers = request.headers;

src/app/correlation-id/correlation-id.actions.ts

@@ -0,0 +1,21 @@
+import { type } from '../shared/ngrx/type';
+import { Action } from '@ngrx/store';
+
+export const CorrelationIDActionTypes = {
+  SET: type('dspace/core/correlationId/SET')
+};
+
+/**
+ * Action for setting a new correlation ID
+ */
+export class SetCorrelationIdAction implements Action {
+  type = CorrelationIDActionTypes.SET;
+
+  constructor(public payload: string) {
+  }
+}
+
+/**
+ * Type alias for all correlation ID actions
+ */
+export type CorrelationIdAction = SetCorrelationIdAction;

src/app/correlation-id/correlation-id.reducer.spec.ts

@@ -0,0 +1,23 @@
+import { correlationIdReducer } from './correlation-id.reducer';
+import { SetCorrelationIdAction } from './correlation-id.actions';
+
+describe('correlationIdReducer', () => {
+  it('should set the correlatinId with SET action', () => {
+    const initialState = null;
+    const currentState = correlationIdReducer(initialState, new SetCorrelationIdAction('new ID'));
+
+    expect(currentState).toBe('new ID');
+  });
+
+  it('should leave correlatinId unchanged otherwise', () => {
+    const initialState = null;
+
+    let currentState = correlationIdReducer(initialState, { type: 'unknown' } as any);
+    expect(currentState).toBe(null);
+
+    currentState = correlationIdReducer(currentState, new SetCorrelationIdAction('new ID'));
+    currentState = correlationIdReducer(currentState, { type: 'unknown' } as any);
+
+    expect(currentState).toBe('new ID');
+  });
+});

src/app/correlation-id/correlation-id.reducer.ts

@@ -0,0 +1,27 @@
+import {
+  CorrelationIdAction,
+  CorrelationIDActionTypes,
+  SetCorrelationIdAction
+} from './correlation-id.actions';
+import { AppState } from '../app.reducer';
+
+const initialState = null;
+
+export const correlationIdSelector = (state: AppState) => state.correlationId;
+
+/**
+ * Reducer that handles actions to update the correlation ID
+ * @param {string} state                the previous correlation ID (null if unset)
+ * @param {CorrelationIdAction} action  the action to perform
+ * @return {string}                     the new correlation ID
+ */
+export const correlationIdReducer = (state = initialState, action: CorrelationIdAction): string => {
+  switch (action.type) {
+    case CorrelationIDActionTypes.SET: {
+      return (action as SetCorrelationIdAction).payload;
+    }
+    default: {
+      return state;
+    }
+  }
+};

src/app/correlation-id/correlation-id.service.spec.ts

@@ -0,0 +1,83 @@
+import { CorrelationIdService } from './correlation-id.service';
+import { CookieServiceMock } from '../shared/mocks/cookie.service.mock';
+import { UUIDService } from '../core/shared/uuid.service';
+import { MockStore, provideMockStore } from '@ngrx/store/testing';
+import { TestBed } from '@angular/core/testing';
+import { Store, StoreModule } from '@ngrx/store';
+import { appReducers, AppState, storeModuleConfig } from '../app.reducer';
+import { SetCorrelationIdAction } from './correlation-id.actions';
+
+describe('CorrelationIdService', () => {
+  let service: CorrelationIdService;
+
+  let cookieService;
+  let uuidService;
+  let store;
+
+  beforeEach(async () => {
+    await TestBed.configureTestingModule({
+      imports: [
+        StoreModule.forRoot(appReducers, storeModuleConfig),
+      ],
+    }).compileComponents();
+  });
+
+  beforeEach(() => {
+    cookieService = new CookieServiceMock();
+    uuidService = new UUIDService();
+    store = TestBed.inject(Store) as MockStore<AppState>;
+    service = new CorrelationIdService(cookieService, uuidService, store);
+  });
+
+  describe('getCorrelationId', () => {
+    it('should get from from store', () => {
+      expect(service.getCorrelationId()).toBe(null);
+      store.dispatch(new SetCorrelationIdAction('some value'));
+      expect(service.getCorrelationId()).toBe('some value');
+    });
+  });
+
+
+  describe('initCorrelationId', () => {
+    const cookieCID = 'cookie CID';
+    const storeCID = 'store CID';
+
+    it('should set cookie and store values to a newly generated value if neither ex', () => {
+      service.initCorrelationId();
+
+      expect(cookieService.get('CORRELATION-ID')).toBeTruthy();
+      expect(service.getCorrelationId()).toBeTruthy();
+      expect(cookieService.get('CORRELATION-ID')).toEqual(service.getCorrelationId());
+    });
+
+    it('should set store value to cookie value if present', () => {
+      expect(service.getCorrelationId()).toBe(null);
+
+      cookieService.set('CORRELATION-ID', cookieCID);
+
+      service.initCorrelationId();
+
+      expect(cookieService.get('CORRELATION-ID')).toBe(cookieCID);
+      expect(service.getCorrelationId()).toBe(cookieCID);
+    });
+
+    it('should set cookie value to store value if present', () => {
+      store.dispatch(new SetCorrelationIdAction(storeCID));
+
+      service.initCorrelationId();
+
+      expect(cookieService.get('CORRELATION-ID')).toBe(storeCID);
+      expect(service.getCorrelationId()).toBe(storeCID);
+    });
+
+    it('should set store value to cookie value if both are present', () => {
+      cookieService.set('CORRELATION-ID', cookieCID);
+      store.dispatch(new SetCorrelationIdAction(storeCID));
+
+      service.initCorrelationId();
+
+      expect(cookieService.get('CORRELATION-ID')).toBe(cookieCID);
+      expect(service.getCorrelationId()).toBe(cookieCID);
+    });
+  });
+});

src/app/correlation-id/correlation-id.service.ts

@@ -0,0 +1,64 @@
+import { CookieService } from '../core/services/cookie.service';
+import { UUIDService } from '../core/shared/uuid.service';
+import { Store, select } from '@ngrx/store';
+import { AppState } from '../app.reducer';
+import { isEmpty } from '../shared/empty.util';
+import { correlationIdSelector } from './correlation-id.reducer';
+import { take } from 'rxjs/operators';
+import { SetCorrelationIdAction } from './correlation-id.actions';
+import { Injectable } from '@angular/core';
+
+/**
+ * Service to manage the correlation id, an id used to give context to server side logs
+ */
+@Injectable({
+  providedIn: 'root'
+})
+export class CorrelationIdService {
+
+  constructor(
+    protected cookieService: CookieService,
+    protected uuidService: UUIDService,
+    protected store: Store<AppState>,
+  ) {
+  }
+
+  /**
+   * Initialize the correlation id based on the cookie or the ngrx store
+   */
+  initCorrelationId(): void {
+    // first see of there's a cookie with a correlation-id
+    let correlationId = this.cookieService.get('CORRELATION-ID');
+
+    // if there isn't see if there's an ID in the store
+    if (isEmpty(correlationId)) {
+      correlationId = this.getCorrelationId();
+    }
+
+    // if no id was found, create a new id
+    if (isEmpty(correlationId)) {
+      correlationId = this.uuidService.generate();
+    }
+
+    // Store the correct id both in the store and as a cookie to ensure they're in sync
+    this.store.dispatch(new SetCorrelationIdAction(correlationId));
+    this.cookieService.set('CORRELATION-ID', correlationId);
+  }
+
+  /**
+   * Get the correlation id from the store
+   */
+  getCorrelationId(): string {
+    let correlationId;
+
+    this.store.pipe(
+      select(correlationIdSelector),
+      take(1)
+    ).subscribe((storeId: string) => {
+      // we can do this because ngrx selects are synchronous
+      correlationId = storeId;
+    });
+
+    return correlationId;
+  }
+}

src/modules/app/browser-app.module.ts

@@ -36,6 +36,7 @@ import { BrowserAuthRequestService } from '../../app/core/auth/browser-auth-requ
 import { AppConfig, APP_CONFIG_STATE } from '../../config/app-config.interface';
 import { DefaultAppConfig } from '../../config/default-app-config';
 import { extendEnvironmentWithAppConfig } from '../../config/config.util';
+import { CorrelationIdService } from '../../app/correlation-id/correlation-id.service';
 
 import { environment } from '../../environments/environment';
 
@@ -81,16 +82,21 @@ export function getRequest(transferState: TransferState): any {
   providers: [
     {
       provide: APP_INITIALIZER,
-      useFactory: (transferState: TransferState) => {
+      useFactory: (
+        transferState: TransferState,
+        dspaceTransferState: DSpaceTransferState,
+        correlationIdService: CorrelationIdService
+      ) => {
         if (transferState.hasKey<AppConfig>(APP_CONFIG_STATE)) {
           const appConfig = transferState.get<AppConfig>(APP_CONFIG_STATE, new DefaultAppConfig());
           // extend environment with app config for browser
           extendEnvironmentWithAppConfig(environment, appConfig);
         }
-
+        dspaceTransferState.transfer();
+        correlationIdService.initCorrelationId();
         return () => true;
       },
-      deps: [TransferState],
+      deps: [TransferState, DSpaceTransferState, CorrelationIdService],
       multi: true
     },
     {
@@ -137,9 +143,4 @@ export function getRequest(transferState: TransferState): any {
   ]
 })
 export class BrowserAppModule {
-  constructor(
-    private transferState: DSpaceTransferState,
-  ) {
-    this.transferState.transfer();
-  }
 }

src/modules/app/server-app.module.ts

@@ -32,6 +32,7 @@ import { ServerHardRedirectService } from '../../app/core/services/server-hard-r
 import { Angulartics2Mock } from '../../app/shared/mocks/angulartics2.service.mock';
 import { AuthRequestService } from '../../app/core/auth/auth-request.service';
 import { ServerAuthRequestService } from '../../app/core/auth/server-auth-request.service';
+import { CorrelationIdService } from '../../app/correlation-id/correlation-id.service';
 import { AppConfig, APP_CONFIG_STATE } from '../../config/app-config.interface';
 
 import { environment } from '../../environments/environment';
@@ -65,11 +66,17 @@ export function createTranslateLoader() {
     // Initialize app config and extend environment
     {
       provide: APP_INITIALIZER,
-      useFactory: (transferState: TransferState) => {
+      useFactory: (
+        transferState: TransferState,
+        dspaceTransferState: DSpaceTransferState,
+        correlationIdService: CorrelationIdService,
+      ) => {
         transferState.set<AppConfig>(APP_CONFIG_STATE, environment as AppConfig);
+        dspaceTransferState.transfer();
+        correlationIdService.initCorrelationId();
         return () => true;
       },
-      deps: [TransferState],
+      deps: [TransferState, DSpaceTransferState, CorrelationIdService],
       multi: true
     },
     {
@@ -117,9 +124,4 @@ export function createTranslateLoader() {
   ]
 })
 export class ServerAppModule {
-  constructor(
-    private transferState: DSpaceTransferState,
-  ) {
-    this.transferState.transfer();
-  }
 }