Clean up routing behaviour in ngram component

frontend/src/app/common-test-bed.ts

@@ -20,7 +20,7 @@ import { EntityService } from './services/entity.service';
 import { WordmodelsService } from './services/wordmodels.service';
 import { WordmodelsServiceMock } from '../mock-data/wordmodels';
 import { VisualizationService } from './services/visualization.service';
-import { visualizationServiceMock } from '../mock-data/visualization';
+import { VisualizationServiceMock } from '../mock-data/visualization';
 import { TagService } from './services/tag.service';
 import { TagServiceMock } from '../mock-data/tag';
 import { RouterStoreService } from './store/router-store.service';
@@ -72,7 +72,7 @@ export const commonTestBed = () => {
         },
         {
             provide: VisualizationService,
-            useValue: new visualizationServiceMock(),
+            useValue: new VisualizationServiceMock(),
         },
         {
             provide: TagService,

frontend/src/app/models/field-filter.spec.ts

@@ -1,9 +1,5 @@
-import { convertToParamMap } from '@angular/router';
 import { mockFieldMultipleChoice, mockFieldDate, mockField } from '../../mock-data/corpus';
-import { EsDateFilter, EsTermsFilter } from './elasticsearch';
 import { BooleanFilter, DateFilter, DateFilterData, MultipleChoiceFilter } from './field-filter';
-import { of } from 'rxjs';
-import { distinct } from 'rxjs/operators';
 import { SimpleStore } from '../store/simple-store';
 import { Store } from '../store/types';
 

frontend/src/app/models/ngram.spec.ts

@@ -0,0 +1,47 @@
+import { SimpleStore } from '../store/simple-store';
+import { RouterStoreService } from '../store/router-store.service';
+import { NgramParameters, NgramSettings } from './ngram';
+
+describe('NgramParameters', ()=> {
+    let store: RouterStoreService = new SimpleStore() as any;
+    let ngramParameters: NgramParameters;
+    const testState = {
+        size: 3,
+        positions: 'first',
+        freqCompensation: true,
+        analysis: 'clean',
+        maxDocuments: 100,
+        numberOfNgrams: 20,
+        dateField: 'releaseDate'
+    } as NgramSettings;
+    const testParams = {ngramSettings: 's:3,p:first,c:true,a:clean,m:100,n:20,f:releaseDate'}
+
+    beforeEach(() => {
+        ngramParameters = new NgramParameters(store);
+    });
+
+    it('should correctly convert internal state to a route parameter', () => {
+        const params = ngramParameters.stateToStore(testState);
+        expect(params).toEqual(testParams);
+    });
+
+    it('should correctly convert a route parameter to an internal state', () => {
+        const state = ngramParameters.storeToState(testParams);
+        expect(state).toEqual(testState);
+    });
+
+    it('should return default values if no relevant route parameter is present', () => {
+        const defaultSettings = {
+            size: 2,
+            positions: 'any',
+            freqCompensation: false,
+            analysis: 'none',
+            maxDocuments: 50,
+            numberOfNgrams: 10,
+            dateField: 'date'
+        } as NgramSettings;
+        const state = ngramParameters.storeToState({irrelevant: 'parameter'})
+        expect(state).toEqual(defaultSettings);
+    })
+
+});

frontend/src/app/models/ngram.ts

@@ -0,0 +1,64 @@
+import { Params } from '@angular/router';
+import * as _ from 'lodash';
+
+import { StoreSync } from '../store/store-sync';
+import { Store } from '../store/types';
+
+export interface NgramSettings {
+    size: number;
+    positions: string;
+    freqCompensation: boolean;
+    analysis: string;
+    maxDocuments: number;
+    numberOfNgrams: number;
+    dateField: string;
+}
+
+export class NgramParameters extends StoreSync<NgramSettings> {
+
+    keysInStore = ['ngramSettings'];
+
+    constructor(store: Store) {
+        super(store);
+        this.connectToStore();
+    }
+
+    stringifyNgramSettings(state: NgramSettings): string {
+        return [`s:${state.size}`,`p:${state.positions}`,`c:${state.freqCompensation}`,
+            `a:${state.analysis}`,`m:${state.maxDocuments}`,`n:${state.numberOfNgrams}`,
+            `f:${state.dateField}`].join(',')
+    }
+
+    stateToStore(state: NgramSettings): Params {
+        return { ngramSettings: this.stringifyNgramSettings(state)}
+    }
+
+    storeToState(params: Params): NgramSettings {
+        if (_.has(params, 'ngramSettings')) {
+            const stringComponents = params['ngramSettings'].split(',');
+            return {
+                size: parseInt(this.findSetting('s', stringComponents), 10),
+                positions: this.findSetting('p', stringComponents),
+                freqCompensation: this.findSetting('c', stringComponents) === 'true',
+                analysis: this.findSetting('a', stringComponents),
+                maxDocuments: parseInt(this.findSetting('m', stringComponents), 10),
+                numberOfNgrams: parseInt(this.findSetting('n', stringComponents), 10),
+                dateField: this.findSetting('f', stringComponents)
+            }
+        }
+        return {
+            size: 2,
+            positions: 'any',
+            freqCompensation: false,
+            analysis: 'none',
+            maxDocuments: 50,
+            numberOfNgrams: 10,
+            dateField: 'date'
+        } as NgramSettings;
+    }
+
+    findSetting(abbreviation: string, stringComponents: string[]): string | undefined{
+        const setting = stringComponents.find(s => s[0] === abbreviation);
+        return setting.split(':')[1];
+    }
+}

frontend/src/app/models/visualization.ts

@@ -75,6 +75,19 @@ export type WordcloudParameters = {
 } & APIQuery;
 
 
+export type NGramRequestParameters = {
+    corpus_name: string;
+    field: string;
+    ngram_size?: number;
+    term_position?: string;
+    freq_compensation?: boolean;
+    subfield?: string;
+    max_size_per_interval?: number;
+    number_of_ngrams?: number;
+    date_field: string;
+} & APIQuery;
+
+
 export interface FreqTableHeader {
     key: string;
     label: string;
@@ -96,69 +109,6 @@ export interface ChartParameters {
     chartType: ChartType;
 }
 
-export type NGramRequestParameters = {
-    corpus_name: string;
-    field: string;
-    ngram_size?: number;
-    term_position?: string;
-    freq_compensation?: boolean;
-    subfield?: string;
-    max_size_per_interval?: number;
-    number_of_ngrams?: number;
-    date_field: string;
-} & APIQuery;
-
-export class NgramParameters {
-    size: number;
-    positions: string;
-    freqCompensation: boolean;
-    analysis: string;
-    maxDocuments: number;
-    numberOfNgrams: number;
-    dateField: string;
-
-    ngramSettings: string [];
-
-    constructor(size: number,
-        positions: string,
-        freqCompensation: boolean,
-        analysis: string,
-        maxDocuments: number,
-        numberOfNgrams: number,
-        dateField: string
-    ) {
-        this.size = size;
-        this.positions = positions;
-        this.freqCompensation = freqCompensation;
-        this.analysis = analysis;
-        this.maxDocuments = maxDocuments;
-        this.numberOfNgrams = numberOfNgrams;
-        this.dateField = dateField;
-    }
-
-    toRouteParam(): string {
-       return [`s:${this.size}`,`p:${this.positions}`,`c:${this.freqCompensation}`,
-       `a:${this.analysis}`,`m:${this.maxDocuments}`,`n:${this.numberOfNgrams}`,
-       `f:${this.dateField}`].join(',');
-    }
-
-    fromRouteParam(paramString: string) {
-        this.ngramSettings = paramString.split(',');
-        this.size = parseInt(this.findSetting('s'), 10);
-        this.positions = this.findSetting('p');
-        this.freqCompensation = this.findSetting('c') === 'true';
-        this.analysis = this.findSetting('a');
-        this.maxDocuments = parseInt(this.findSetting('m'), 10);
-        this.numberOfNgrams = parseInt(this.findSetting('n'), 10);
-        this.dateField = this.findSetting('f');
-    }
-
-    findSetting(abbreviation: string): string | undefined{
-        const setting = this.ngramSettings.find(s => s[0] === abbreviation);
-        return setting.split(':')[1];
-    }
-}
-
 export interface FieldCoverage {
     [field: string]: number;
 };

frontend/src/app/services/visualization.service.ts

@@ -7,13 +7,13 @@ import {
     GeoLocation,
     MostFrequentWordsResult,
     NGramRequestParameters,
-    NgramParameters,
     QueryModel,
     TaskResult,
     TimeCategory,
 } from '@models';
 import { ApiService } from './api.service';
 import { Observable } from 'rxjs';
+import { NgramSettings } from '@models/ngram';
 
 @Injectable({
   providedIn: 'root'
@@ -96,7 +96,7 @@ export class VisualizationService {
         corpus: Corpus,
         queryModel: QueryModel,
         field: string,
-        params: NgramParameters
+        params: NgramSettings
     ): NGramRequestParameters {
         const query = queryModel.toAPIQuery();
         return {
@@ -121,7 +121,7 @@ export class VisualizationService {
         return this.apiService.getDateTermFrequency(params);
     }
 
-    getNgramTasks(queryModel: QueryModel, corpus: Corpus, field: string, params: NgramParameters): Promise<TaskResult> {
+    getNgramTasks(queryModel: QueryModel, corpus: Corpus, field: string, params: NgramSettings): Promise<TaskResult> {
         const ngramRequestParams = this.makeNgramRequestParameters(corpus, queryModel, field, params);
         return this.apiService.ngramTasks(ngramRequestParams);
     }

frontend/src/app/visualization/ngram/ngram.component.spec.ts

@@ -6,26 +6,43 @@ import { mockCorpus } from '../../../mock-data/corpus';
 import { MockCorpusResponse } from '../../../mock-data/corpus-response';
 import { commonTestBed } from '../../common-test-bed';
 import { NgramComponent } from './ngram.component';
-import { ApiService } from '@services';
-import { ApiServiceMock } from '../../../mock-data/api';
+import { ApiService, VisualizationService } from '@services';
+import { ApiServiceMock, fakeNgramResult } from '../../../mock-data/api';
+import { VisualizationServiceMock } from '../../../mock-data/visualization';
 import { Subject } from 'rxjs';
+import { NgramSettings } from '@models/ngram';
+
 
 describe('NgramComponent', () => {
   let component: NgramComponent;
   let fixture: ComponentFixture<NgramComponent>;
   let apiService: ApiServiceMock;
+  let visualizationService: VisualizationService;
+  let cacheKey = 's:2,p:any,c:false,a:none,m:50,n:10,f:date';
+  let defaultSettings = {
+    size: 2,
+    positions: 'any',
+    freqCompensation: false,
+    analysis: 'none',
+    maxDocuments: 50,
+    numberOfNgrams: 10,
+    dateField: 'date'
+  } as NgramSettings;
 
   beforeEach(waitForAsync(() => {
     commonTestBed().testingModule.compileComponents();
 }));
 
   beforeEach(() => {
     apiService = new ApiServiceMock({});
-    spyOn(apiService, 'abortTasks');
+    visualizationService = new VisualizationServiceMock() as any;
+    spyOn(visualizationService, 'getNgramTasks').and.callThrough();
+    spyOn(apiService, 'abortTasks').and.callThrough();
     fixture = TestBed.overrideComponent(NgramComponent, {
       set: {
         providers: [
-          { provide: ApiService, useValue: apiService}
+          { provide: ApiService, useValue: apiService },
+          { provide: VisualizationService, useValue: visualizationService }
         ]
       }
     }).createComponent(NgramComponent);
@@ -45,25 +62,46 @@ describe('NgramComponent', () => {
     expect(component).toBeTruthy();
   });
 
+  it('should initialize ngramParameters with default values', () => {
+    expect(component.ngramParameters.state$.value).toEqual(defaultSettings);
+  });
+
+  it('should not abort tasks when `onParameterChange` is triggered during initialization', () => {
+    spyOn(component.stopPolling$, 'next');
+    component.onParameterChange('size', 2);
+    expect(component.stopPolling$.next).not.toHaveBeenCalled();
+  })
+
   it('should stop polling and abort running tasks when changing settings', () => {
     const dropdown = fixture.debugElement.query(By.css('ia-dropdown'));
     const changeSizeDropdown = (value: number) => {
         const eventObj = { parameter: 'size', value };
         dropdown.triggerEventHandler('onChange', eventObj);
     };
-    spyOn(fixture.componentInstance.stopPolling$, 'next');
+    spyOn(component.stopPolling$, 'next');
     changeSizeDropdown(10);
-    expect(fixture.componentInstance.stopPolling$.next).toHaveBeenCalled();
+    expect(component.stopPolling$.next).toHaveBeenCalled();
     component.dataHasLoaded = false; // fake working response
     expect(component.tasksToCancel).toBeUndefined();
   });
 
   it('should stop polling and abort running tasks on destroy', () => {
     spyOn(component.stopPolling$, 'next');
-    component.teardown();
+    component.ngOnDestroy();
     expect(component.stopPolling$.next).toHaveBeenCalled();
     component.dataHasLoaded = false; // fake working response
     expect(component.tasksToCancel).toBeUndefined();
   });
 
+  it('should send a new ngram request after confirmed changes', () => {
+    component.confirmChanges();
+    expect(visualizationService.getNgramTasks).toHaveBeenCalled();
+  });
+
+  it('should not send a new ngram request when the result is cached', () => {
+    component.resultsCache = {[cacheKey]: fakeNgramResult};
+    component.confirmChanges();
+    expect(visualizationService.getNgramTasks).not.toHaveBeenCalled();
+  })
+
 });