diff --git a/frontend/src/app/common-test-bed.ts b/frontend/src/app/common-test-bed.ts index 888762621..678338635 100644 --- a/frontend/src/app/common-test-bed.ts +++ b/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, diff --git a/frontend/src/app/models/field-filter.spec.ts b/frontend/src/app/models/field-filter.spec.ts index 2ec71c117..a266b8563 100644 --- a/frontend/src/app/models/field-filter.spec.ts +++ b/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'; diff --git a/frontend/src/app/models/ngram.spec.ts b/frontend/src/app/models/ngram.spec.ts new file mode 100644 index 000000000..8f2748422 --- /dev/null +++ b/frontend/src/app/models/ngram.spec.ts @@ -0,0 +1,45 @@ +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, + } as NgramSettings; + const testParams = {ngramSettings: 's:3,p:first,c:true,a:clean,m:100,n:20'} + + 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, + } as NgramSettings; + const state = ngramParameters.storeToState({irrelevant: 'parameter'}) + expect(state).toEqual(defaultSettings); + }) + +}); diff --git a/frontend/src/app/models/ngram.ts b/frontend/src/app/models/ngram.ts new file mode 100644 index 000000000..1cc9670e5 --- /dev/null +++ b/frontend/src/app/models/ngram.ts @@ -0,0 +1,60 @@ +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; +} + +export class NgramParameters extends StoreSync { + + 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}`].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), + } + } + return { + size: 2, + positions: 'any', + freqCompensation: false, + analysis: 'none', + maxDocuments: 50, + numberOfNgrams: 10, + } as NgramSettings; + } + + findSetting(abbreviation: string, stringComponents: string[]): string | undefined{ + const setting = stringComponents.find(s => s[0] === abbreviation); + return setting.split(':')[1]; + } +} diff --git a/frontend/src/app/models/visualization.spec.ts b/frontend/src/app/models/visualization.spec.ts deleted file mode 100644 index b89edb49f..000000000 --- a/frontend/src/app/models/visualization.spec.ts +++ /dev/null @@ -1,37 +0,0 @@ -import { NgramParameters } from './visualization'; - -describe('NgramParameters', ()=> { - let ngramParameters: NgramParameters; - - beforeEach(() => { - ngramParameters = new NgramParameters( - 2, - 'any', - false, - 'none', - 50, - 10, - 'date' - ); - }); - - it('should convert itself to a param string', () => { - const paramString = ngramParameters.toRouteParam(); - expect(paramString).toEqual( - 's:2,p:any,c:false,a:none,m:50,n:10,f:date' - ); - }); - - it('should set itself from a param string', () => { - ngramParameters.fromRouteParam( - 's:3,p:first,c:true,a:none,m:50,n:20,f:date' - ); - expect(ngramParameters.size).toEqual(3); - expect(ngramParameters.positions).toEqual('first'); - expect(ngramParameters.freqCompensation).toEqual(true); - expect(ngramParameters.analysis).toEqual('none'); - expect(ngramParameters.maxDocuments).toEqual(50); - expect(ngramParameters.numberOfNgrams).toEqual(20); - expect(ngramParameters.dateField).toEqual('date'); - }); -}); diff --git a/frontend/src/app/models/visualization.ts b/frontend/src/app/models/visualization.ts index 6fe8fac7e..29c69b060 100644 --- a/frontend/src/app/models/visualization.ts +++ b/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; }; diff --git a/frontend/src/app/services/visualization.service.ts b/frontend/src/app/services/visualization.service.ts index 5ef5daeee..52ff1c4cc 100644 --- a/frontend/src/app/services/visualization.service.ts +++ b/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,8 @@ export class VisualizationService { corpus: Corpus, queryModel: QueryModel, field: string, - params: NgramParameters + params: NgramSettings, + dateField: string ): NGramRequestParameters { const query = queryModel.toAPIQuery(); return { @@ -109,7 +110,7 @@ export class VisualizationService { subfield: params.analysis, max_size_per_interval: params.maxDocuments, number_of_ngrams: params.numberOfNgrams, - date_field: params.dateField + date_field: dateField }; } @@ -121,8 +122,8 @@ export class VisualizationService { return this.apiService.getDateTermFrequency(params); } - getNgramTasks(queryModel: QueryModel, corpus: Corpus, field: string, params: NgramParameters): Promise { - const ngramRequestParams = this.makeNgramRequestParameters(corpus, queryModel, field, params); + getNgramTasks(queryModel: QueryModel, corpus: Corpus, field: string, params: NgramSettings, dateField: string): Promise { + const ngramRequestParams = this.makeNgramRequestParameters(corpus, queryModel, field, params, dateField); return this.apiService.ngramTasks(ngramRequestParams); } diff --git a/frontend/src/app/visualization/ngram/ngram.component.spec.ts b/frontend/src/app/visualization/ngram/ngram.component.spec.ts index 60e44a175..975d49098 100644 --- a/frontend/src/app/visualization/ngram/ngram.component.spec.ts +++ b/frontend/src/app/visualization/ngram/ngram.component.spec.ts @@ -6,14 +6,27 @@ 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; let apiService: ApiServiceMock; + let visualizationService: VisualizationService; + let cacheKey = 's:2,p:any,c:false,a:none,m:50,n:10'; + let defaultSettings = { + size: 2, + positions: 'any', + freqCompensation: false, + analysis: 'none', + maxDocuments: 50, + numberOfNgrams: 10, + } as NgramSettings; beforeEach(waitForAsync(() => { commonTestBed().testingModule.compileComponents(); @@ -21,11 +34,14 @@ describe('NgramComponent', () => { 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); @@ -35,7 +51,9 @@ describe('NgramComponent', () => { component.stopPolling$ = new Subject(); component.queryModel = queryModel; component.corpus = MockCorpusResponse[0] as any; - component.visualizedField = {name: 'speech'} as any; + component.visualizedField = {name: 'speech'} as any; + component.dateField = {name: 'date'} as any; + component.allDateFields = [component.dateField]; component.asTable = false; component.palette = ['yellow', 'blue']; fixture.detectChanges(); @@ -45,25 +63,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(); + }) + }); diff --git a/frontend/src/app/visualization/ngram/ngram.component.ts b/frontend/src/app/visualization/ngram/ngram.component.ts index 43f9d1887..d53c69fe7 100644 --- a/frontend/src/app/visualization/ngram/ngram.component.ts +++ b/frontend/src/app/visualization/ngram/ngram.component.ts @@ -1,5 +1,4 @@ import { Component, ElementRef, EventEmitter, HostBinding, Input, OnChanges, Output, SimpleChanges, ViewChild } from '@angular/core'; -import { ActivatedRoute, ParamMap, Params, Router } from '@angular/router'; import * as _ from 'lodash'; import { Subject } from 'rxjs'; @@ -10,23 +9,24 @@ import { QueryModel, CorpusField, NgramResults, - NgramParameters, SuccessfulTask, } from '@models'; import { ApiService, NotificationService, - ParamService, VisualizationService, } from '@services'; -import { ParamDirective } from '../../param/param-directive'; + +import { RouterStoreService } from '../../store/router-store.service'; +import { NgramParameters, NgramSettings } from '@models/ngram'; + @Component({ selector: 'ia-ngram', templateUrl: './ngram.component.html', styleUrls: ['./ngram.component.scss'], }) -export class NgramComponent extends ParamDirective implements OnChanges { +export class NgramComponent implements OnChanges { @HostBinding('style.display') display = 'block'; // needed for loading spinner positioning @Input() queryModel: QueryModel; @@ -39,6 +39,8 @@ export class NgramComponent extends ParamDirective implements OnChanges { @ViewChild('chart-container') chartContainer: ElementRef; + keysInStore = ['ngramSettings']; + allDateFields: CorpusField[]; dateField: CorpusField; @@ -85,10 +87,9 @@ export class NgramComponent extends ParamDirective implements OnChanges { tasksToCancel: string[]; resultsCache: { [parameters: string]: any } = {}; - currentParameters: NgramParameters; - lastParameters: NgramParameters; + ngramParameters: NgramParameters; + currentSettings: NgramSettings; parametersChanged = false; - ngramSettings: string[]; dataHasLoaded: boolean; isLoading = false; @@ -100,89 +101,64 @@ export class NgramComponent extends ParamDirective implements OnChanges { private apiService: ApiService, private visualizationService: VisualizationService, private notificationService: NotificationService, - route: ActivatedRoute, - router: Router, - paramService: ParamService + store: RouterStoreService, ) { - super(route, router, paramService); - this.currentParameters = new NgramParameters( - this.sizeOptions[0].value, - this.positionsOptions[0].value, - this.freqCompensationOptions[0].value, - 'none', - this.maxDocumentsOptions[0].value, - this.numberOfNgramsOptions[0].value, - 'date' + this.ngramParameters = new NgramParameters( + store, ); + this.currentSettings = _.clone(this.ngramParameters.state$.value); } get currentSizeOption() { - if (this.currentParameters) { - return this.sizeOptions.find( - (item) => item.value === this.currentParameters.size - ); - } + return this.sizeOptions.find( + (item) => item.value === this.currentSettings.size + ); } get currentPositionsOption() { - if (this.currentParameters) { - return this.positionsOptions.find( - (item) => item.value === this.currentParameters.positions - ); - } + return this.positionsOptions.find( + (item) => item.value === this.currentSettings.positions + ); } get currentFreqCompensationOption() { - if (this.currentParameters) { - return this.freqCompensationOptions.find( - (item) => item.value === this.currentParameters.freqCompensation - ); - } + return this.freqCompensationOptions.find( + (item) => item.value === this.currentSettings.freqCompensation + ); } get currentAnalysisOption() { - if (this.currentParameters) { - return this.analysisOptions.find( - (item) => item.value === this.currentParameters.analysis - ); - } + return this.analysisOptions.find( + (item) => item.value === this.currentSettings.analysis + ); } get currentMaxDocumentsOption() { - if (this.currentParameters) { - return this.maxDocumentsOptions.find( - (item) => item.value === this.currentParameters.maxDocuments - ); - } + return this.maxDocumentsOptions.find( + (item) => item.value === this.currentSettings.maxDocuments + ); } get currentNumberOfNgramsOption() { - if (this.currentParameters) { - return this.numberOfNgramsOptions.find( - (item) => item.value === this.currentParameters.numberOfNgrams - ); - } + return this.numberOfNgramsOptions.find( + (item) => item.value === this.currentSettings.numberOfNgrams + ); } - initialize() {} - - teardown(): void { + ngOnDestroy(): void { this.stopPolling$.next(); - } - - setStateFromParams(params: ParamMap) { - this.setParameters(params); - this.loadGraph(); + this.ngramParameters.complete(); } ngOnChanges(changes: SimpleChanges): void { - if (changes.queryModel || changes.visualizedField) { - this.resultsCache = {}; + if (changes.corpus) { this.allDateFields = this.corpus.fields.filter( (field) => field.mappingType === 'date' ); this.dateField = this.allDateFields[0]; - this.currentParameters.dateField = this.dateField.name; + } + if (changes.queryModel || changes.visualizedField) { + this.resultsCache = {}; if (this.visualizedField.multiFields) { this.analysisOptions = [ { label: 'None', value: 'none' }, @@ -203,30 +179,23 @@ export class NgramComponent extends ParamDirective implements OnChanges { } } - if (this.currentParameters) { + if (this.ngramParameters.state$.value) { this.loadGraph(); } - } - setParameters(params: Params) { - const ngramSettings = params.get('ngramSettings'); - if (ngramSettings) { - this.currentParameters.fromRouteParam(ngramSettings); - } } loadGraph() { this.isLoading = true; this.dataHasLoaded = false; - this.lastParameters = _.clone(this.currentParameters); - const cachedResult = this.getCachedResult(this.currentParameters); + const cachedResult = this.getCachedResult(); if (cachedResult) { this.onDataLoaded(cachedResult); } else { this.visualizationService.getNgramTasks( this.queryModel, this.corpus, this.visualizedField.name, - this.currentParameters).then( + this.currentSettings, this.dateField.name).then( response => { this.tasksToCancel = response.task_ids; // tasksToCancel contains ids of the parent task and its subtasks @@ -256,6 +225,7 @@ export class NgramComponent extends ParamDirective implements OnChanges { onDataLoaded(result: NgramResults) { this.dataHasLoaded = true; this.currentResults = result; + this.cacheResult(result); this.tableData = this.makeTableData(result); this.isLoading = false; } @@ -272,28 +242,41 @@ export class NgramComponent extends ParamDirective implements OnChanges { ); } - cacheResult(result: any, params: NgramParameters): void { - const key = params.toRouteParam(); - this.resultsCache[key] = result; + cacheResult(result: any): void { + const key = this.concatenateDateField(this.ngramParameters.stringifyNgramSettings(this.currentSettings)); + if (key) { + this.resultsCache[key] = result; + } } - getCachedResult(params: NgramParameters): any { - const key = params.toRouteParam(); - if (_.has(this.resultsCache, key)) { + getCachedResult(): any { + const key = this.concatenateDateField(this.ngramParameters.stringifyNgramSettings(this.currentSettings)); + if (key && _.has(this.resultsCache, key)) { return this.resultsCache[key]; } } + concatenateDateField(key: string): string { + // add the date field to the resultsCache key: it is currently not handled by ngramParameters + // TO DO: this is a workaround, remove if ngramParameters are implemented to handle date field + if (this.allDateFields.length) { + key.concat(`f:${this.dateField.name}`) + } + return key + } + setPositionsOptions(size) { // set positions dropdown options and reset its value this.positionsOptions = ['any'] .concat(['first', 'second', 'third', 'fourth'].slice(0, size)) .map((item) => ({ value: item, label: item })); - this.currentParameters.positions = this.positionsOptions[0].value; } onParameterChange(parameter: string, value: any) { - this.currentParameters[parameter] = value; + if (_.get(this.currentSettings, parameter) === value) { + return; + } + _.assign(this.currentSettings, {[parameter]: value}); if (parameter === 'size' && value) { this.setPositionsOptions(value); @@ -304,17 +287,14 @@ export class NgramComponent extends ParamDirective implements OnChanges { } cancelChanges() { - this.setPositionsOptions(this.lastParameters.size); - this.currentParameters = this.lastParameters; this.parametersChanged = false; } confirmChanges() { + this.ngramParameters.setParams(this.currentSettings); this.isLoading = true; this.parametersChanged = false; - this.setParams({ - ngramSettings: this.currentParameters.toRouteParam(), - }); + this.loadGraph(); } formatValue(value: number): string { @@ -338,7 +318,8 @@ export class NgramComponent extends ParamDirective implements OnChanges { this.corpus, this.queryModel, this.visualizedField.name, - this.currentParameters + this.ngramParameters.state$.value, + this.dateField.name ); this.apiService .requestFullData({ diff --git a/frontend/src/mock-data/visualization.ts b/frontend/src/mock-data/visualization.ts index 15aa34482..c81f9baa1 100644 --- a/frontend/src/mock-data/visualization.ts +++ b/frontend/src/mock-data/visualization.ts @@ -1,6 +1,8 @@ -export class visualizationServiceMock { - public async getRelatedWords() {} - public async getNgramTasks() { - return Promise.resolve({task_ids: ['ngram-task-id']}); +import { TaskResult } from '@models'; + +export class VisualizationServiceMock { + public getRelatedWords() {} + public async getNgramTasks(): Promise { + return Promise.resolve({task_ids: ['ngram-task-id', 'another-task-id']}); } }