From 7235fa392ae21d0c0f0d5270d2a5eefefe963f1f Mon Sep 17 00:00:00 2001 From: thinkh Date: Mon, 30 Mar 2020 13:37:08 +0200 Subject: [PATCH 01/26] Export serialize and restore RegExp function --- src/lineup/internal/cmds.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/lineup/internal/cmds.ts b/src/lineup/internal/cmds.ts index 47ea2ecd9..769bb12df 100644 --- a/src/lineup/internal/cmds.ts +++ b/src/lineup/internal/cmds.ts @@ -394,11 +394,11 @@ interface IRegExpFilter { * ``` * JSON.stringify(/^123$/gm); // result: {} * ``` - * + * @internal * @param value Input string or RegExp object * @returns {string | IRegExpFilter} Returns the input string or a plain `IRegExpFilter` object */ -function serializeRegExp(value: string | RegExp): string | IRegExpFilter { +export function serializeRegExp(value: string | RegExp): string | IRegExpFilter { if (!(value instanceof RegExp)) { return value; } @@ -409,10 +409,11 @@ function serializeRegExp(value: string | RegExp): string | IRegExpFilter { * Restores a RegExp object from a given IRegExpFilter object. * In case a string is passed to this function no deserialization is applied. * + * @internal * @param filter Filter as string or plain object matching the IRegExpFilter * @returns {string | RegExp| null} Returns the input string or the restored RegExp object */ -function restoreRegExp(filter: string | IRegExpFilter): string | RegExp { +export function restoreRegExp(filter: string | IRegExpFilter): string | RegExp { if (filter === null || !(filter).isRegExp) { return filter; } From c92f0e90b44d40fbd79aadc64e14d56b8324bd18 Mon Sep 17 00:00:00 2001 From: thinkh Date: Mon, 30 Mar 2020 13:37:34 +0200 Subject: [PATCH 02/26] Add jest tests for serialize/restoreRegExp --- tests/lineup/internal/cmds.test.ts | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) create mode 100644 tests/lineup/internal/cmds.test.ts diff --git a/tests/lineup/internal/cmds.test.ts b/tests/lineup/internal/cmds.test.ts new file mode 100644 index 000000000..accd5ec73 --- /dev/null +++ b/tests/lineup/internal/cmds.test.ts @@ -0,0 +1,29 @@ +/// +import {serializeRegExp, restoreRegExp} from '../../../src/lineup/internal/cmds'; + +describe('Serialize LineUp filter for provenance graph', () => { + it('filter as string', () => { + expect(serializeRegExp('abc')).toBe('abc'); + expect(typeof serializeRegExp('abc')).toBe('string'); + }); + + // The following tests worked with LineUp v3. + // With LineUp v4 the filter object changed + it('filter as RegExp', () => { + expect(serializeRegExp(/abc/gm)).toMatchObject({value: '/abc/gm', isRegExp: true}); + expect(serializeRegExp(/abc/gm)).not.toMatchObject({value: '/12345/gm', isRegExp: true}); + expect(serializeRegExp(/abc/gm)).not.toMatchObject({value: '/abc/gm', isRegExp: false}); + }); +}); + +describe('Restore LineUp filter from provenance graph', () => { + it('filter as string', () => { + expect(restoreRegExp('abc')).toBe('abc'); + expect(typeof restoreRegExp('abc')).toBe('string'); + }); + + it('filter as IRegExpFilter', () => { + expect(restoreRegExp({value: '/abc/gm', isRegExp: true})).toMatchObject(/abc/gm); + }); +}); + From ed4e0ca7d2a5616914ef10e7d13e592a8cdf860c Mon Sep 17 00:00:00 2001 From: thinkh Date: Mon, 30 Mar 2020 14:03:42 +0200 Subject: [PATCH 03/26] Refactor `serializeLineUpFilter` for LineUp v4 --- src/lineup/internal/cmds.ts | 62 ++++++++++++++++++++++++------ tests/lineup/internal/cmds.test.ts | 34 +++++++++++----- 2 files changed, 75 insertions(+), 21 deletions(-) diff --git a/src/lineup/internal/cmds.ts b/src/lineup/internal/cmds.ts index 769bb12df..53f06fb1e 100644 --- a/src/lineup/internal/cmds.ts +++ b/src/lineup/internal/cmds.ts @@ -349,7 +349,9 @@ function recordPropertyChange(source: Column | Ranking, provider: LocalDataProvi return; } - const newSerializedValue = serializeRegExp(newValue); // serialize possible RegExp object to be properly stored as provenance graph + if(property === 'filter') { + newValue = serializeLineUpFilter(newValue); // serialize possible RegExp object to be properly stored as provenance graph + } if (source instanceof Column) { // assert ALineUpView and update the stats @@ -357,12 +359,12 @@ function recordPropertyChange(source: Column | Ranking, provider: LocalDataProvi const rid = rankingId(provider, source.findMyRanker()); const path = source.fqpath; - graph.pushWithResult(setColumn(lineupViewWrapper, rid, path, property, newSerializedValue), { + graph.pushWithResult(setColumn(lineupViewWrapper, rid, path, property, newValue), { inverse: setColumn(lineupViewWrapper, rid, path, property, old) }); } else if (source instanceof Ranking) { const rid = rankingId(provider, source); - graph.pushWithResult(setColumn(lineupViewWrapper, rid, null, property, newSerializedValue), { + graph.pushWithResult(setColumn(lineupViewWrapper, rid, null, property, newValue), { inverse: setColumn(lineupViewWrapper, rid, null, property, old) }); } @@ -386,23 +388,59 @@ interface IRegExpFilter { } /** - * Serializes RegExp objects to an IRegexFilter object, which can be stored in the provenance graph. - * In case a string is passed to this function no serialization is applied. + * This interface combines the `IStringFilter` from `StringColumn` + * and `ICategoricalFilter` from `ICategoricalColumn`. + */ +interface ILineUpStringFilter { + /** + * Filter value + */ + filter: string[] | string | RegExp | null; + + /** + * Filter for missing values + */ + filterMissing: boolean; +} + +/** + * Similar to the `ILineUpStringFilter`, but the RegExp is replaced with `IRegExpFilter` + */ +interface ISerializableLineUpFilter { + /** + * Filter value + * Note that the RegExp is replaced with IRegExpFilter (compared to the `ILineUpStringFilter` interface) + */ + filter: string[] | string | IRegExpFilter | null; + + /** + * Filter for missing values + */ + filterMissing: boolean; +} + +/** + * Serializes LineUp string filter, which can contain RegExp objects to an IRegexFilter object. + * The return value of this function can be passed to `JSON.stringify()` and stored in the provenance graph. * * Background information: * The serialization step is necessary, because RegExp objects are converted into an empty object `{}` on `JSON.stringify`. * ``` * JSON.stringify(/^123$/gm); // result: {} * ``` + * * @internal - * @param value Input string or RegExp object - * @returns {string | IRegExpFilter} Returns the input string or a plain `IRegExpFilter` object + * @param filter LineUp filter object + * @returns Returns the `ISerializableLineUpFilter` object */ -export function serializeRegExp(value: string | RegExp): string | IRegExpFilter { - if (!(value instanceof RegExp)) { - return value; - } - return {value: value.toString(), isRegExp: true}; +export function serializeLineUpFilter(filter: ILineUpStringFilter): ISerializableLineUpFilter { + return { + filter: { + value: filter.filter.toString(), + isRegExp: (filter.filter instanceof RegExp) + }, + filterMissing: filter.filterMissing + }; } /** diff --git a/tests/lineup/internal/cmds.test.ts b/tests/lineup/internal/cmds.test.ts index accd5ec73..571cbca84 100644 --- a/tests/lineup/internal/cmds.test.ts +++ b/tests/lineup/internal/cmds.test.ts @@ -1,18 +1,34 @@ /// -import {serializeRegExp, restoreRegExp} from '../../../src/lineup/internal/cmds'; +import {serializeLineUpFilter, restoreRegExp} from '../../../src/lineup/internal/cmds'; -describe('Serialize LineUp filter for provenance graph', () => { +// The following tests worked with LineUp v3. +// With LineUp v4 the filter object changed +// describe('Serialize LineUp v3 filter for provenance graph', () => { +// it('filter as string', () => { +// expect(serializeRegExp('abc')).toBe('abc'); +// expect(typeof serializeRegExp('abc')).toBe('string'); +// }); + +// it('filter as RegExp', () => { +// expect(serializeRegExp(/abc/gm)).toMatchObject({value: '/abc/gm', isRegExp: true}); +// expect(serializeRegExp(/abc/gm)).not.toMatchObject({value: '/12345/gm', isRegExp: true}); +// expect(serializeRegExp(/abc/gm)).not.toMatchObject({value: '/abc/gm', isRegExp: false}); +// }); +// }); + +describe('Serialize LineUp v4 filter for provenance graph', () => { it('filter as string', () => { - expect(serializeRegExp('abc')).toBe('abc'); - expect(typeof serializeRegExp('abc')).toBe('string'); + const lineUpFilter = {filter: 'abc', filterMissing: false}; + + expect(serializeLineUpFilter(lineUpFilter)).toMatchObject({filter: {value: 'abc', isRegExp: false}, filterMissing: false}); }); - // The following tests worked with LineUp v3. - // With LineUp v4 the filter object changed it('filter as RegExp', () => { - expect(serializeRegExp(/abc/gm)).toMatchObject({value: '/abc/gm', isRegExp: true}); - expect(serializeRegExp(/abc/gm)).not.toMatchObject({value: '/12345/gm', isRegExp: true}); - expect(serializeRegExp(/abc/gm)).not.toMatchObject({value: '/abc/gm', isRegExp: false}); + const lineUpFilter = {filter: /abc/gm, filterMissing: false}; + + expect(serializeLineUpFilter(lineUpFilter)).toMatchObject({filter: {value: '/abc/gm', isRegExp: true}, filterMissing: false}); + expect(serializeLineUpFilter(lineUpFilter)).not.toMatchObject({filter: {value: '/12345/gm', isRegExp: true}, filterMissing: false}); + expect(serializeLineUpFilter(lineUpFilter)).not.toMatchObject({filter: {value: '/abc/gm', isRegExp: false}, filterMissing: false}); }); }); From fcd5d7d6bb6288510db150cffd8eee2c35e7fca5 Mon Sep 17 00:00:00 2001 From: oltionchampari Date: Tue, 31 Mar 2020 12:53:02 +0200 Subject: [PATCH 04/26] Ignore number columns filters datavisyn/tdp_core#331 --- src/lineup/internal/cmds.ts | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/lineup/internal/cmds.ts b/src/lineup/internal/cmds.ts index 53f06fb1e..628c33f0e 100644 --- a/src/lineup/internal/cmds.ts +++ b/src/lineup/internal/cmds.ts @@ -193,6 +193,13 @@ export function setGroupCriteria(provider: IObjectRef, rid: number, columns }); } +/** + * Check if filter has the `filter` property + * Necessary since number columns filter has properties `min`, `max` and no filter property, + * @param filter + */ +const serialize = (filter: any) => filter.hasOwnProperty('filter'); + export async function setColumnImpl(inputs: IObjectRef[], parameter: any) { const p: LocalDataProvider = await resolveImmediately((await inputs[0].v).data); const ranking = p.getRankings()[parameter.rid]; From a5a1a9c8b6b43f6d752884640a83e3099ed58732 Mon Sep 17 00:00:00 2001 From: oltionchampari Date: Tue, 31 Mar 2020 12:55:06 +0200 Subject: [PATCH 05/26] Serialize and restore only regex filters datavisyn/tdp_core#331 --- src/lineup/internal/cmds.ts | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/lineup/internal/cmds.ts b/src/lineup/internal/cmds.ts index 628c33f0e..66f2a4710 100644 --- a/src/lineup/internal/cmds.ts +++ b/src/lineup/internal/cmds.ts @@ -229,7 +229,9 @@ export async function setColumnImpl(inputs: IObjectRef[], parameter: any) { break; default: bak = source[`get${prop}`](); - source[`set${prop}`].call(source, restoreRegExp(parameter.value)); // restore serialized regular expression before passing to LineUp + // restore serialized regular expression before passing to LineUp + const value = parameter.prop === 'filter' && serialize(parameter.value) ? restoreRegExp(parameter.value) : parameter.value; + source[`set${prop}`].call(source, value); break; } } @@ -356,8 +358,8 @@ function recordPropertyChange(source: Column | Ranking, provider: LocalDataProvi return; } - if(property === 'filter') { - newValue = serializeLineUpFilter(newValue); // serialize possible RegExp object to be properly stored as provenance graph + if (property === 'filter') { + newValue = serialize(newValue) ? serializeLineUpFilter(newValue) : newValue; // serialize possible RegExp object to be properly stored as provenance graph } if (source instanceof Column) { From 103e90903fb03e045e4d025e0560fc20ce3b3e1c Mon Sep 17 00:00:00 2001 From: oltionchampari Date: Tue, 31 Mar 2020 12:57:43 +0200 Subject: [PATCH 06/26] Add ILineUpStringFilterValue type datavisyn/tdp_core#331 --- src/lineup/internal/cmds.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/lineup/internal/cmds.ts b/src/lineup/internal/cmds.ts index 66f2a4710..26aae4495 100644 --- a/src/lineup/internal/cmds.ts +++ b/src/lineup/internal/cmds.ts @@ -404,7 +404,7 @@ interface ILineUpStringFilter { /** * Filter value */ - filter: string[] | string | RegExp | null; + filter: ILineUpStringFilterValue | RegExp; /** * Filter for missing values @@ -420,7 +420,7 @@ interface ISerializableLineUpFilter { * Filter value * Note that the RegExp is replaced with IRegExpFilter (compared to the `ILineUpStringFilter` interface) */ - filter: string[] | string | IRegExpFilter | null; + filter: ILineUpStringFilterValue | IRegExpFilter; /** * Filter for missing values @@ -428,6 +428,7 @@ interface ISerializableLineUpFilter { filterMissing: boolean; } +type ILineUpStringFilterValue = string[] | string | null; /** * Serializes LineUp string filter, which can contain RegExp objects to an IRegexFilter object. * The return value of this function can be passed to `JSON.stringify()` and stored in the provenance graph. From 502eaceba312cb70d6b56c2f3b702832b0eb6fb8 Mon Sep 17 00:00:00 2001 From: oltionchampari Date: Tue, 31 Mar 2020 12:59:01 +0200 Subject: [PATCH 07/26] Check if filter is a regExp before turning it to a string datavisyn/tdp_core#331 --- src/lineup/internal/cmds.ts | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/lineup/internal/cmds.ts b/src/lineup/internal/cmds.ts index 26aae4495..23d354bca 100644 --- a/src/lineup/internal/cmds.ts +++ b/src/lineup/internal/cmds.ts @@ -389,7 +389,7 @@ interface IRegExpFilter { /** * RegExp as string */ - value: string; + value: ILineUpStringFilterValue; /** * Flag to indicate the value should be restored as RegExp */ @@ -429,6 +429,7 @@ interface ISerializableLineUpFilter { } type ILineUpStringFilterValue = string[] | string | null; + /** * Serializes LineUp string filter, which can contain RegExp objects to an IRegexFilter object. * The return value of this function can be passed to `JSON.stringify()` and stored in the provenance graph. @@ -444,10 +445,12 @@ type ILineUpStringFilterValue = string[] | string | null; * @returns Returns the `ISerializableLineUpFilter` object */ export function serializeLineUpFilter(filter: ILineUpStringFilter): ISerializableLineUpFilter { + const value = filter.filter; + const isRegexp = value instanceof RegExp; return { filter: { - value: filter.filter.toString(), - isRegExp: (filter.filter instanceof RegExp) + value: isRegexp ? value.toString() : value as ILineUpStringFilterValue, + isRegExp: isRegexp }, filterMissing: filter.filterMissing }; From a6b177785e9ce0f7f8adeadefb7a6a538f8e8383 Mon Sep 17 00:00:00 2001 From: oltionchampari Date: Tue, 31 Mar 2020 13:01:11 +0200 Subject: [PATCH 08/26] Adapt restoreRegexp to pass tests datavisyn/tdp_core#331 --- src/lineup/internal/cmds.ts | 27 ++++++++++++++++++--------- 1 file changed, 18 insertions(+), 9 deletions(-) diff --git a/src/lineup/internal/cmds.ts b/src/lineup/internal/cmds.ts index 23d354bca..a8f118bd8 100644 --- a/src/lineup/internal/cmds.ts +++ b/src/lineup/internal/cmds.ts @@ -462,17 +462,26 @@ export function serializeLineUpFilter(filter: ILineUpStringFilter): ISerializabl * * @internal * @param filter Filter as string or plain object matching the IRegExpFilter - * @returns {string | RegExp| null} Returns the input string or the restored RegExp object + * @returns Returns the input string or the restored RegExp object */ -export function restoreRegExp(filter: string | IRegExpFilter): string | RegExp { - if (filter === null || !(filter).isRegExp) { - return filter; +export function restoreRegExp(filter: ILineUpStringFilterValue | IRegExpFilter | ISerializableLineUpFilter, filterMissing = false): ILineUpStringFilter { + + const isIRegExpFilter = ((filter: IRegExpFilter | ISerializableLineUpFilter): filter is IRegExpFilter => filter.hasOwnProperty('isRegExp')); + const isISerializableLineUpFilter = (filter: IRegExpFilter | ISerializableLineUpFilter): filter is ISerializableLineUpFilter => filter.hasOwnProperty('filterMissing'); + + if (filter === null || typeof filter === 'string' || Array.isArray(filter)) { + return {filter: filter, filterMissing}; + } else if (isIRegExpFilter(filter)) { + if (filter.isRegExp) { + const serializedRegexParser = /^\/(.+)\/(\w+)?$/; // from https://gist.github.com/tenbits/ec7f0155b57b2d61a6cc90ef3d5f8b49 + const matches = serializedRegexParser.exec(filter.value as string); + const [_full, regexString, regexFlags] = matches; + return {filter: new RegExp(regexString, regexFlags), filterMissing}; + } + return restoreRegExp(filter.value, filterMissing); + } else if (isISerializableLineUpFilter(filter)) { + return restoreRegExp(filter.filter, filter.filterMissing); } - - const serializedRegexParser = /^\/(.+)\/(\w+)?$/; // from https://gist.github.com/tenbits/ec7f0155b57b2d61a6cc90ef3d5f8b49 - const matches = serializedRegexParser.exec((filter).value); - const [_full, regexString, regexFlags] = matches; - return new RegExp(regexString, regexFlags); } function trackColumn(provider: LocalDataProvider, lineup: IObjectRef, graph: ProvenanceGraph, col: Column) { From 942e1d265cacf5c64a78af636ab4a05c2e4e7eda Mon Sep 17 00:00:00 2001 From: oltionchampari Date: Tue, 31 Mar 2020 13:01:36 +0200 Subject: [PATCH 09/26] Add addittional tests datavisyn/tdp_core#331 --- tests/lineup/internal/cmds.test.ts | 51 +++++++++++++++++++++++++++--- 1 file changed, 47 insertions(+), 4 deletions(-) diff --git a/tests/lineup/internal/cmds.test.ts b/tests/lineup/internal/cmds.test.ts index 571cbca84..fe6436e3f 100644 --- a/tests/lineup/internal/cmds.test.ts +++ b/tests/lineup/internal/cmds.test.ts @@ -30,16 +30,59 @@ describe('Serialize LineUp v4 filter for provenance graph', () => { expect(serializeLineUpFilter(lineUpFilter)).not.toMatchObject({filter: {value: '/12345/gm', isRegExp: true}, filterMissing: false}); expect(serializeLineUpFilter(lineUpFilter)).not.toMatchObject({filter: {value: '/abc/gm', isRegExp: false}, filterMissing: false}); }); + + it('filter as array', () => { + const lineUpFilter = {filter: ['chromosome', 'gender'], filterMissing: false}; + expect(serializeLineUpFilter(lineUpFilter)).toMatchObject({filter: {value: ['chromosome', 'gender'], isRegExp: false}, filterMissing: false}); + }); + + it('filter as null', () => { + const lineUpFilter = {filter: null, filterMissing: false}; + expect(serializeLineUpFilter(lineUpFilter)).toMatchObject({filter: {value: null, isRegExp: false}, filterMissing: false}); + }); + + it('filter as null and use regular expression true', () => { + const lineUpFilter = {filter: /null/, filterMissing: false}; + expect(serializeLineUpFilter(lineUpFilter)).toMatchObject({filter: {value: /null/, isRegExp: true}, filterMissing: false}); + }); }); + describe('Restore LineUp filter from provenance graph', () => { it('filter as string', () => { - expect(restoreRegExp('abc')).toBe('abc'); - expect(typeof restoreRegExp('abc')).toBe('string'); + expect(restoreRegExp('abc')).toMatchObject({filter: 'abc', filterMissing: false}); + expect(restoreRegExp('abc').hasOwnProperty('filterMissing')).toBeTruthy(); + }); + + it('filter as null', () => { + expect(restoreRegExp(null)).toMatchObject({filter: null, filterMissing: false}); + }); + + it('filter as IRegExpFilter with `value:abc`, `isRegexp:false`', () => { + expect(restoreRegExp({value: 'abc', isRegExp: false})).toMatchObject({filter: 'abc', filterMissing: false}); + }); + + it('filter as IRegExpFilter with `value:null`, `isRegexp:false`', () => { + expect(restoreRegExp({value: null, isRegExp: false})).toMatchObject({filter: null, filterMissing: false}); + }); + + it('filter as IRegExpFilter with `value:/^abc/gm`, `isRegexp:true`', () => { + expect(restoreRegExp({value: '/^abc/gm', isRegExp: true})).toMatchObject({filter: /^abc/gm, filterMissing: false}); + }); + + it('filter as ISerializableLineUpFilter', () => { + const fromGraphFilter = {filter: {value: '/abc$/gm', isRegExp: true}, filterMissing: false}; + expect(restoreRegExp(fromGraphFilter)).toMatchObject({filter: /abc$/gm, filterMissing: false}); + }); + + it('filter as ISerializableLineUpFilter with property `filterMissing: true`', () => { + const fromGraphFilter = {filter: {value: '/abc$/gm', isRegExp: true}, filterMissing: true}; + expect(restoreRegExp(fromGraphFilter)).toMatchObject({filter: /abc$/gm, filterMissing: true}); }); - it('filter as IRegExpFilter', () => { - expect(restoreRegExp({value: '/abc/gm', isRegExp: true})).toMatchObject(/abc/gm); + it('filter as ISerializableLineUpFilter with property `value: null`', () => { + const fromGraphFilter = {filter: {value: null, isRegExp: false}, filterMissing: true}; + expect(restoreRegExp(fromGraphFilter)).toMatchObject({filter: null, filterMissing: true}); }); }); From 9f7b02231e03be234fd30f418b7fc12548da10a3 Mon Sep 17 00:00:00 2001 From: oltionchampari Date: Tue, 31 Mar 2020 13:39:48 +0200 Subject: [PATCH 10/26] Add comment --- src/lineup/internal/cmds.ts | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/lineup/internal/cmds.ts b/src/lineup/internal/cmds.ts index a8f118bd8..06a5e84d1 100644 --- a/src/lineup/internal/cmds.ts +++ b/src/lineup/internal/cmds.ts @@ -428,6 +428,9 @@ interface ISerializableLineUpFilter { filterMissing: boolean; } +/** + * Filter value + */ type ILineUpStringFilterValue = string[] | string | null; /** @@ -459,16 +462,12 @@ export function serializeLineUpFilter(filter: ILineUpStringFilter): ISerializabl /** * Restores a RegExp object from a given IRegExpFilter object. * In case a string is passed to this function no deserialization is applied. - * - * @internal * @param filter Filter as string or plain object matching the IRegExpFilter * @returns Returns the input string or the restored RegExp object */ export function restoreRegExp(filter: ILineUpStringFilterValue | IRegExpFilter | ISerializableLineUpFilter, filterMissing = false): ILineUpStringFilter { - const isIRegExpFilter = ((filter: IRegExpFilter | ISerializableLineUpFilter): filter is IRegExpFilter => filter.hasOwnProperty('isRegExp')); const isISerializableLineUpFilter = (filter: IRegExpFilter | ISerializableLineUpFilter): filter is ISerializableLineUpFilter => filter.hasOwnProperty('filterMissing'); - if (filter === null || typeof filter === 'string' || Array.isArray(filter)) { return {filter: filter, filterMissing}; } else if (isIRegExpFilter(filter)) { From ab6b1642c7ddfe6bf99e9111c08e1c52f174371c Mon Sep 17 00:00:00 2001 From: oltionchampari Date: Wed, 1 Apr 2020 08:12:41 +0200 Subject: [PATCH 11/26] Rename function datavisyn/tdp_core#345 --- src/lineup/internal/cmds.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/lineup/internal/cmds.ts b/src/lineup/internal/cmds.ts index 06a5e84d1..7a2ecf9e3 100644 --- a/src/lineup/internal/cmds.ts +++ b/src/lineup/internal/cmds.ts @@ -198,7 +198,7 @@ export function setGroupCriteria(provider: IObjectRef, rid: number, columns * Necessary since number columns filter has properties `min`, `max` and no filter property, * @param filter */ -const serialize = (filter: any) => filter.hasOwnProperty('filter'); +const isSerializedFilter = (filter: any) => filter.hasOwnProperty('filter'); export async function setColumnImpl(inputs: IObjectRef[], parameter: any) { const p: LocalDataProvider = await resolveImmediately((await inputs[0].v).data); @@ -230,7 +230,7 @@ export async function setColumnImpl(inputs: IObjectRef[], parameter: any) { default: bak = source[`get${prop}`](); // restore serialized regular expression before passing to LineUp - const value = parameter.prop === 'filter' && serialize(parameter.value) ? restoreRegExp(parameter.value) : parameter.value; + const value = parameter.prop === 'filter' && isSerializedFilter(parameter.value) ? restoreRegExp(parameter.value) : parameter.value; source[`set${prop}`].call(source, value); break; } @@ -359,7 +359,7 @@ function recordPropertyChange(source: Column | Ranking, provider: LocalDataProvi } if (property === 'filter') { - newValue = serialize(newValue) ? serializeLineUpFilter(newValue) : newValue; // serialize possible RegExp object to be properly stored as provenance graph + newValue = isSerializedFilter(newValue) ? serializeLineUpFilter(newValue) : newValue; // serialize possible RegExp object to be properly stored as provenance graph } if (source instanceof Column) { From a4c34f2f5fbddb21ff342837156c2d781179a304 Mon Sep 17 00:00:00 2001 From: oltionchampari Date: Wed, 1 Apr 2020 08:17:30 +0200 Subject: [PATCH 12/26] Restore filter only when cmd is filter datavisyn/tdp_core#345 --- src/lineup/internal/cmds.ts | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/lineup/internal/cmds.ts b/src/lineup/internal/cmds.ts index 7a2ecf9e3..ab1b9881f 100644 --- a/src/lineup/internal/cmds.ts +++ b/src/lineup/internal/cmds.ts @@ -227,12 +227,16 @@ export async function setColumnImpl(inputs: IObjectRef[], parameter: any) { bak = source[`getRenderer`](); source[`setRenderer`].call(source, parameter.value); break; - default: + case 'filter': bak = source[`get${prop}`](); // restore serialized regular expression before passing to LineUp - const value = parameter.prop === 'filter' && isSerializedFilter(parameter.value) ? restoreRegExp(parameter.value) : parameter.value; + const value = isSerializedFilter(parameter.value) ? restoreRegExp(parameter.value) : parameter.value; source[`set${prop}`].call(source, value); break; + default: + bak = source[`get${prop}`](); + source[`set${prop}`].call(source, parameter.value); + break; } } From e508584fa8ba2f1b0f311be78613757d69c1269f Mon Sep 17 00:00:00 2001 From: oltionchampari Date: Wed, 1 Apr 2020 08:26:56 +0200 Subject: [PATCH 13/26] Refactoring datavisyn/tdp_core#345 --- src/lineup/internal/cmds.ts | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/src/lineup/internal/cmds.ts b/src/lineup/internal/cmds.ts index ab1b9881f..abd8436f8 100644 --- a/src/lineup/internal/cmds.ts +++ b/src/lineup/internal/cmds.ts @@ -230,7 +230,7 @@ export async function setColumnImpl(inputs: IObjectRef[], parameter: any) { case 'filter': bak = source[`get${prop}`](); // restore serialized regular expression before passing to LineUp - const value = isSerializedFilter(parameter.value) ? restoreRegExp(parameter.value) : parameter.value; + const value = isSerializedFilter(parameter.value) ? restoreLineUpFilter(parameter.value) : parameter.value; source[`set${prop}`].call(source, value); break; default: @@ -469,11 +469,14 @@ export function serializeLineUpFilter(filter: ILineUpStringFilter): ISerializabl * @param filter Filter as string or plain object matching the IRegExpFilter * @returns Returns the input string or the restored RegExp object */ -export function restoreRegExp(filter: ILineUpStringFilterValue | IRegExpFilter | ISerializableLineUpFilter, filterMissing = false): ILineUpStringFilter { +export function restoreLineUpFilter(filter: ILineUpStringFilterValue | IRegExpFilter | ISerializableLineUpFilter, filterMissing = false): ILineUpStringFilter { + const isSimpleFilter = (filter: ILineUpStringFilterValue | IRegExpFilter | ISerializableLineUpFilter): filter is ILineUpStringFilterValue => filter === null || typeof filter === 'string' || Array.isArray(filter); const isIRegExpFilter = ((filter: IRegExpFilter | ISerializableLineUpFilter): filter is IRegExpFilter => filter.hasOwnProperty('isRegExp')); const isISerializableLineUpFilter = (filter: IRegExpFilter | ISerializableLineUpFilter): filter is ISerializableLineUpFilter => filter.hasOwnProperty('filterMissing'); - if (filter === null || typeof filter === 'string' || Array.isArray(filter)) { - return {filter: filter, filterMissing}; + + if (isSimpleFilter(filter)) { + return {filter, filterMissing}; + } else if (isIRegExpFilter(filter)) { if (filter.isRegExp) { const serializedRegexParser = /^\/(.+)\/(\w+)?$/; // from https://gist.github.com/tenbits/ec7f0155b57b2d61a6cc90ef3d5f8b49 @@ -481,9 +484,11 @@ export function restoreRegExp(filter: ILineUpStringFilterValue | IRegExpFilter | const [_full, regexString, regexFlags] = matches; return {filter: new RegExp(regexString, regexFlags), filterMissing}; } - return restoreRegExp(filter.value, filterMissing); + + return restoreLineUpFilter(filter.value, filterMissing); + } else if (isISerializableLineUpFilter(filter)) { - return restoreRegExp(filter.filter, filter.filterMissing); + return restoreLineUpFilter(filter.filter, filter.filterMissing); } } From 77f5def9633772c810af1d7e4d4996faa21446c0 Mon Sep 17 00:00:00 2001 From: oltionchampari Date: Wed, 1 Apr 2020 09:26:08 +0200 Subject: [PATCH 14/26] Include case filter being null datavisyn/tdp_core#345 --- src/lineup/internal/cmds.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lineup/internal/cmds.ts b/src/lineup/internal/cmds.ts index abd8436f8..338c5411c 100644 --- a/src/lineup/internal/cmds.ts +++ b/src/lineup/internal/cmds.ts @@ -198,7 +198,7 @@ export function setGroupCriteria(provider: IObjectRef, rid: number, columns * Necessary since number columns filter has properties `min`, `max` and no filter property, * @param filter */ -const isSerializedFilter = (filter: any) => filter.hasOwnProperty('filter'); +const isSerializedFilter = (filter: any) => filter && filter.hasOwnProperty('filter'); export async function setColumnImpl(inputs: IObjectRef[], parameter: any) { const p: LocalDataProvider = await resolveImmediately((await inputs[0].v).data); From 0aa6348c935f706e42917577bf1a8bc2c2f6db20 Mon Sep 17 00:00:00 2001 From: oltionchampari Date: Wed, 1 Apr 2020 09:26:47 +0200 Subject: [PATCH 15/26] Adapt tests datavisyn/tdp_core#345 --- src/lineup/internal/OverviewColumn.ts | 6 ++--- tests/lineup/internal/cmds.test.ts | 32 ++++++++++++--------------- 2 files changed, 17 insertions(+), 21 deletions(-) diff --git a/src/lineup/internal/OverviewColumn.ts b/src/lineup/internal/OverviewColumn.ts index 8c7aa7f38..122154324 100644 --- a/src/lineup/internal/OverviewColumn.ts +++ b/src/lineup/internal/OverviewColumn.ts @@ -18,9 +18,9 @@ export default class OverviewColumn extends BooleanColumn { super(id, Object.assign(desc, { label: i18n.t('tdp:core.lineup.OverviewColumn.overviewSelection') })); - (this).setDefaultRenderer('boolean'); - (this).setDefaultGroupRenderer('boolean'); - (this).setDefaultSummaryRenderer('categorical'); + // (this).setDefaultRenderer('boolean'); + // (this).setDefaultGroupRenderer('boolean'); + // (this).setDefaultSummaryRenderer('categorical'); (this).setWidthImpl(0); // hide } diff --git a/tests/lineup/internal/cmds.test.ts b/tests/lineup/internal/cmds.test.ts index fe6436e3f..cb893c3e3 100644 --- a/tests/lineup/internal/cmds.test.ts +++ b/tests/lineup/internal/cmds.test.ts @@ -1,5 +1,5 @@ /// -import {serializeLineUpFilter, restoreRegExp} from '../../../src/lineup/internal/cmds'; +import {serializeLineUpFilter, restoreLineUpFilter} from '../../../src/lineup/internal/cmds'; // The following tests worked with LineUp v3. // With LineUp v4 the filter object changed @@ -36,53 +36,49 @@ describe('Serialize LineUp v4 filter for provenance graph', () => { expect(serializeLineUpFilter(lineUpFilter)).toMatchObject({filter: {value: ['chromosome', 'gender'], isRegExp: false}, filterMissing: false}); }); - it('filter as null', () => { - const lineUpFilter = {filter: null, filterMissing: false}; - expect(serializeLineUpFilter(lineUpFilter)).toMatchObject({filter: {value: null, isRegExp: false}, filterMissing: false}); - }); - it('filter as null and use regular expression true', () => { - const lineUpFilter = {filter: /null/, filterMissing: false}; - expect(serializeLineUpFilter(lineUpFilter)).toMatchObject({filter: {value: /null/, isRegExp: true}, filterMissing: false}); + const lineUpFilter = {filter: null, filterMissing: true}; + expect(serializeLineUpFilter(lineUpFilter)).toMatchObject({filter: {value: null, isRegExp: false}, filterMissing: true}); }); }); describe('Restore LineUp filter from provenance graph', () => { it('filter as string', () => { - expect(restoreRegExp('abc')).toMatchObject({filter: 'abc', filterMissing: false}); - expect(restoreRegExp('abc').hasOwnProperty('filterMissing')).toBeTruthy(); + expect(restoreLineUpFilter('abc')).toMatchObject({filter: 'abc', filterMissing: false}); + expect(restoreLineUpFilter('abc').hasOwnProperty('filterMissing')).toBeTruthy(); }); - it('filter as null', () => { - expect(restoreRegExp(null)).toMatchObject({filter: null, filterMissing: false}); + it('filter as null and `filterMissing: true`', () => { + const fromGraphFilter = {filter: {value: null, isRegExp: false}, filterMissing: true}; + expect(restoreLineUpFilter(fromGraphFilter)).toMatchObject({filter: null, filterMissing: true}); }); it('filter as IRegExpFilter with `value:abc`, `isRegexp:false`', () => { - expect(restoreRegExp({value: 'abc', isRegExp: false})).toMatchObject({filter: 'abc', filterMissing: false}); + expect(restoreLineUpFilter({value: 'abc', isRegExp: false})).toMatchObject({filter: 'abc', filterMissing: false}); }); it('filter as IRegExpFilter with `value:null`, `isRegexp:false`', () => { - expect(restoreRegExp({value: null, isRegExp: false})).toMatchObject({filter: null, filterMissing: false}); + expect(restoreLineUpFilter({value: null, isRegExp: false})).toMatchObject({filter: null, filterMissing: false}); }); it('filter as IRegExpFilter with `value:/^abc/gm`, `isRegexp:true`', () => { - expect(restoreRegExp({value: '/^abc/gm', isRegExp: true})).toMatchObject({filter: /^abc/gm, filterMissing: false}); + expect(restoreLineUpFilter({value: '/^abc/gm', isRegExp: true})).toMatchObject({filter: /^abc/gm, filterMissing: false}); }); it('filter as ISerializableLineUpFilter', () => { const fromGraphFilter = {filter: {value: '/abc$/gm', isRegExp: true}, filterMissing: false}; - expect(restoreRegExp(fromGraphFilter)).toMatchObject({filter: /abc$/gm, filterMissing: false}); + expect(restoreLineUpFilter(fromGraphFilter)).toMatchObject({filter: /abc$/gm, filterMissing: false}); }); it('filter as ISerializableLineUpFilter with property `filterMissing: true`', () => { const fromGraphFilter = {filter: {value: '/abc$/gm', isRegExp: true}, filterMissing: true}; - expect(restoreRegExp(fromGraphFilter)).toMatchObject({filter: /abc$/gm, filterMissing: true}); + expect(restoreLineUpFilter(fromGraphFilter)).toMatchObject({filter: /abc$/gm, filterMissing: true}); }); it('filter as ISerializableLineUpFilter with property `value: null`', () => { const fromGraphFilter = {filter: {value: null, isRegExp: false}, filterMissing: true}; - expect(restoreRegExp(fromGraphFilter)).toMatchObject({filter: null, filterMissing: true}); + expect(restoreLineUpFilter(fromGraphFilter)).toMatchObject({filter: null, filterMissing: true}); }); }); From d4280ef1e7581cbcbcf2c6e755b19aa3931ce6ef Mon Sep 17 00:00:00 2001 From: oltionchampari Date: Wed, 1 Apr 2020 09:34:06 +0200 Subject: [PATCH 16/26] Remove comments datavisyn/tdp_core#345 --- src/lineup/internal/OverviewColumn.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/lineup/internal/OverviewColumn.ts b/src/lineup/internal/OverviewColumn.ts index 122154324..8c7aa7f38 100644 --- a/src/lineup/internal/OverviewColumn.ts +++ b/src/lineup/internal/OverviewColumn.ts @@ -18,9 +18,9 @@ export default class OverviewColumn extends BooleanColumn { super(id, Object.assign(desc, { label: i18n.t('tdp:core.lineup.OverviewColumn.overviewSelection') })); - // (this).setDefaultRenderer('boolean'); - // (this).setDefaultGroupRenderer('boolean'); - // (this).setDefaultSummaryRenderer('categorical'); + (this).setDefaultRenderer('boolean'); + (this).setDefaultGroupRenderer('boolean'); + (this).setDefaultSummaryRenderer('categorical'); (this).setWidthImpl(0); // hide } From e117ac9255d8f40cede89638d4d56ca9f1cd1222 Mon Sep 17 00:00:00 2001 From: oltionchampari Date: Wed, 1 Apr 2020 09:47:21 +0200 Subject: [PATCH 17/26] Minor change datavisyn/tdp_core#345 --- src/lineup/internal/cmds.ts | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/src/lineup/internal/cmds.ts b/src/lineup/internal/cmds.ts index 338c5411c..4d2a427ed 100644 --- a/src/lineup/internal/cmds.ts +++ b/src/lineup/internal/cmds.ts @@ -193,13 +193,6 @@ export function setGroupCriteria(provider: IObjectRef, rid: number, columns }); } -/** - * Check if filter has the `filter` property - * Necessary since number columns filter has properties `min`, `max` and no filter property, - * @param filter - */ -const isSerializedFilter = (filter: any) => filter && filter.hasOwnProperty('filter'); - export async function setColumnImpl(inputs: IObjectRef[], parameter: any) { const p: LocalDataProvider = await resolveImmediately((await inputs[0].v).data); const ranking = p.getRankings()[parameter.rid]; @@ -437,6 +430,16 @@ interface ISerializableLineUpFilter { */ type ILineUpStringFilterValue = string[] | string | null; + +/** + * Check if filter has the `filter` property + * Necessary since number columns filter has properties `min`, `max` and no filter property, + * @param filter + */ +function isSerializedFilter(filter: any): ISerializableLineUpFilter { + return filter && filter.hasOwnProperty('filter'); +} + /** * Serializes LineUp string filter, which can contain RegExp objects to an IRegexFilter object. * The return value of this function can be passed to `JSON.stringify()` and stored in the provenance graph. From 23c9e03afca8a7076c27835c23bc4ac5742f6898 Mon Sep 17 00:00:00 2001 From: oltionchampari Date: Thu, 2 Apr 2020 08:08:19 +0200 Subject: [PATCH 18/26] Merge branch lineupjs4 --- package.json | 2 +- src/lineup/internal/OverviewColumn.ts | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/package.json b/package.json index 35407b5d7..aa008dabc 100644 --- a/package.json +++ b/package.json @@ -116,7 +116,7 @@ "ts-loader": "4.0.1", "tslib": "~1.11.0", "tslint": "5.9.1", - "ts-jest": "^25.2.1", + "ts-jest": "25.2.1", "typedoc": "~0.16.10", "typescript": "~3.8.2", "url-loader": "0.5.8", diff --git a/src/lineup/internal/OverviewColumn.ts b/src/lineup/internal/OverviewColumn.ts index 8c7aa7f38..059407304 100644 --- a/src/lineup/internal/OverviewColumn.ts +++ b/src/lineup/internal/OverviewColumn.ts @@ -16,12 +16,12 @@ export default class OverviewColumn extends BooleanColumn { constructor(id: string, desc: IBooleanColumnDesc) { super(id, Object.assign(desc, { - label: i18n.t('tdp:core.lineup.OverviewColumn.overviewSelection') + label: i18n.t('tdp:core.lineup.OverviewColumn.overviewSelection'), + renderer: 'boolean', + groupRenderer: 'boolean', + summaryRenderer: 'categorical' })); - (this).setDefaultRenderer('boolean'); - (this).setDefaultGroupRenderer('boolean'); - (this).setDefaultSummaryRenderer('categorical'); - (this).setWidthImpl(0); // hide + this.setWidthImpl(0); // hide } getValue(row: IDataRow) { From ddb0f56f775795db42bbc7ca801803e8e23ffa72 Mon Sep 17 00:00:00 2001 From: thinkh Date: Thu, 2 Apr 2020 09:56:27 +0200 Subject: [PATCH 19/26] Use enum to get filter value --- src/lineup/internal/cmds.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/lineup/internal/cmds.ts b/src/lineup/internal/cmds.ts index 4d2a427ed..e6a08430a 100644 --- a/src/lineup/internal/cmds.ts +++ b/src/lineup/internal/cmds.ts @@ -220,7 +220,7 @@ export async function setColumnImpl(inputs: IObjectRef[], parameter: any) { bak = source[`getRenderer`](); source[`setRenderer`].call(source, parameter.value); break; - case 'filter': + case LineUpTrackAndUntrackActions.filter: bak = source[`get${prop}`](); // restore serialized regular expression before passing to LineUp const value = isSerializedFilter(parameter.value) ? restoreLineUpFilter(parameter.value) : parameter.value; @@ -355,7 +355,7 @@ function recordPropertyChange(source: Column | Ranking, provider: LocalDataProvi return; } - if (property === 'filter') { + if (property === LineUpTrackAndUntrackActions.filter) { newValue = isSerializedFilter(newValue) ? serializeLineUpFilter(newValue) : newValue; // serialize possible RegExp object to be properly stored as provenance graph } From f9b7e4396c8f7e037209cdd410296847203f4fb4 Mon Sep 17 00:00:00 2001 From: thinkh Date: Thu, 2 Apr 2020 09:56:55 +0200 Subject: [PATCH 20/26] Throw error when restoring unknown filter values --- src/lineup/internal/cmds.ts | 3 +++ tests/lineup/internal/cmds.test.ts | 12 +++++++++--- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/src/lineup/internal/cmds.ts b/src/lineup/internal/cmds.ts index e6a08430a..d7114b2f6 100644 --- a/src/lineup/internal/cmds.ts +++ b/src/lineup/internal/cmds.ts @@ -492,7 +492,10 @@ export function restoreLineUpFilter(filter: ILineUpStringFilterValue | IRegExpFi } else if (isISerializableLineUpFilter(filter)) { return restoreLineUpFilter(filter.filter, filter.filterMissing); + } + + throw new Error('Unknown LineUp filter format. Unable to restore the given filter.'); } function trackColumn(provider: LocalDataProvider, lineup: IObjectRef, graph: ProvenanceGraph, col: Column) { diff --git a/tests/lineup/internal/cmds.test.ts b/tests/lineup/internal/cmds.test.ts index cb893c3e3..7a6512e77 100644 --- a/tests/lineup/internal/cmds.test.ts +++ b/tests/lineup/internal/cmds.test.ts @@ -54,15 +54,15 @@ describe('Restore LineUp filter from provenance graph', () => { expect(restoreLineUpFilter(fromGraphFilter)).toMatchObject({filter: null, filterMissing: true}); }); - it('filter as IRegExpFilter with `value:abc`, `isRegexp:false`', () => { + it('filter as IRegExpFilter with `value: abc`, `isRegexp: false`', () => { expect(restoreLineUpFilter({value: 'abc', isRegExp: false})).toMatchObject({filter: 'abc', filterMissing: false}); }); - it('filter as IRegExpFilter with `value:null`, `isRegexp:false`', () => { + it('filter as IRegExpFilter with `value: null`, `isRegexp: false`', () => { expect(restoreLineUpFilter({value: null, isRegExp: false})).toMatchObject({filter: null, filterMissing: false}); }); - it('filter as IRegExpFilter with `value:/^abc/gm`, `isRegexp:true`', () => { + it('filter as IRegExpFilter with `value: /^abc/gm`, `isRegexp: true`', () => { expect(restoreLineUpFilter({value: '/^abc/gm', isRegExp: true})).toMatchObject({filter: /^abc/gm, filterMissing: false}); }); @@ -80,5 +80,11 @@ describe('Restore LineUp filter from provenance graph', () => { const fromGraphFilter = {filter: {value: null, isRegExp: false}, filterMissing: true}; expect(restoreLineUpFilter(fromGraphFilter)).toMatchObject({filter: null, filterMissing: true}); }); + + it('unknown filter format throws error', () => { + expect(() => { + restoreLineUpFilter(123456789); // typecast to pass unknown format + }).toThrowError(new Error('Unknown LineUp filter format. Unable to restore the given filter.')); + }); }); From 5f5bae1509c4f4823209af29bc39fbb185e3eddc Mon Sep 17 00:00:00 2001 From: thinkh Date: Thu, 2 Apr 2020 10:05:25 +0200 Subject: [PATCH 21/26] Extract cmd filter impl to own file + move test --- src/lineup/internal/cmds.ts | 121 +---------------- src/lineup/internal/cmds/filter.ts | 126 ++++++++++++++++++ .../{cmds.test.ts => cmds/filter.test.ts} | 2 +- 3 files changed, 128 insertions(+), 121 deletions(-) create mode 100644 src/lineup/internal/cmds/filter.ts rename tests/lineup/internal/{cmds.test.ts => cmds/filter.test.ts} (99%) diff --git a/src/lineup/internal/cmds.ts b/src/lineup/internal/cmds.ts index d7114b2f6..0e4a0ac80 100644 --- a/src/lineup/internal/cmds.ts +++ b/src/lineup/internal/cmds.ts @@ -7,6 +7,7 @@ import {IObjectRef, action, meta, cat, op, ProvenanceGraph, ICmdResult} from 'ph import {NumberColumn, LocalDataProvider, StackColumn, ScriptColumn, OrdinalColumn, CompositeColumn, Ranking, ISortCriteria, Column, isMapAbleColumn, mappingFunctions} from 'lineupjs'; import {resolveImmediately} from 'phovea_core/src'; import i18n from 'phovea_core/src/i18n'; +import {isSerializedFilter, restoreLineUpFilter, serializeLineUpFilter} from './cmds/filter'; // used for function calls in the context of tracking or untracking actions in the provenance graph in order to get a consistent defintion of the used strings @@ -378,126 +379,6 @@ function recordPropertyChange(source: Column | Ranking, provider: LocalDataProvi source.on(`${property}Changed.track`, delayed > 0 ? delayedCall(f, delayed) : f); } -/** - * Serialize RegExp objects from LineUp string columns as plain object - * that can be stored in the provenance graph - */ -interface IRegExpFilter { - /** - * RegExp as string - */ - value: ILineUpStringFilterValue; - /** - * Flag to indicate the value should be restored as RegExp - */ - isRegExp: boolean; -} - -/** - * This interface combines the `IStringFilter` from `StringColumn` - * and `ICategoricalFilter` from `ICategoricalColumn`. - */ -interface ILineUpStringFilter { - /** - * Filter value - */ - filter: ILineUpStringFilterValue | RegExp; - - /** - * Filter for missing values - */ - filterMissing: boolean; -} - -/** - * Similar to the `ILineUpStringFilter`, but the RegExp is replaced with `IRegExpFilter` - */ -interface ISerializableLineUpFilter { - /** - * Filter value - * Note that the RegExp is replaced with IRegExpFilter (compared to the `ILineUpStringFilter` interface) - */ - filter: ILineUpStringFilterValue | IRegExpFilter; - - /** - * Filter for missing values - */ - filterMissing: boolean; -} - -/** - * Filter value - */ -type ILineUpStringFilterValue = string[] | string | null; - - -/** - * Check if filter has the `filter` property - * Necessary since number columns filter has properties `min`, `max` and no filter property, - * @param filter - */ -function isSerializedFilter(filter: any): ISerializableLineUpFilter { - return filter && filter.hasOwnProperty('filter'); -} - -/** - * Serializes LineUp string filter, which can contain RegExp objects to an IRegexFilter object. - * The return value of this function can be passed to `JSON.stringify()` and stored in the provenance graph. - * - * Background information: - * The serialization step is necessary, because RegExp objects are converted into an empty object `{}` on `JSON.stringify`. - * ``` - * JSON.stringify(/^123$/gm); // result: {} - * ``` - * - * @internal - * @param filter LineUp filter object - * @returns Returns the `ISerializableLineUpFilter` object - */ -export function serializeLineUpFilter(filter: ILineUpStringFilter): ISerializableLineUpFilter { - const value = filter.filter; - const isRegexp = value instanceof RegExp; - return { - filter: { - value: isRegexp ? value.toString() : value as ILineUpStringFilterValue, - isRegExp: isRegexp - }, - filterMissing: filter.filterMissing - }; -} - -/** - * Restores a RegExp object from a given IRegExpFilter object. - * In case a string is passed to this function no deserialization is applied. - * @param filter Filter as string or plain object matching the IRegExpFilter - * @returns Returns the input string or the restored RegExp object - */ -export function restoreLineUpFilter(filter: ILineUpStringFilterValue | IRegExpFilter | ISerializableLineUpFilter, filterMissing = false): ILineUpStringFilter { - const isSimpleFilter = (filter: ILineUpStringFilterValue | IRegExpFilter | ISerializableLineUpFilter): filter is ILineUpStringFilterValue => filter === null || typeof filter === 'string' || Array.isArray(filter); - const isIRegExpFilter = ((filter: IRegExpFilter | ISerializableLineUpFilter): filter is IRegExpFilter => filter.hasOwnProperty('isRegExp')); - const isISerializableLineUpFilter = (filter: IRegExpFilter | ISerializableLineUpFilter): filter is ISerializableLineUpFilter => filter.hasOwnProperty('filterMissing'); - - if (isSimpleFilter(filter)) { - return {filter, filterMissing}; - - } else if (isIRegExpFilter(filter)) { - if (filter.isRegExp) { - const serializedRegexParser = /^\/(.+)\/(\w+)?$/; // from https://gist.github.com/tenbits/ec7f0155b57b2d61a6cc90ef3d5f8b49 - const matches = serializedRegexParser.exec(filter.value as string); - const [_full, regexString, regexFlags] = matches; - return {filter: new RegExp(regexString, regexFlags), filterMissing}; - } - - return restoreLineUpFilter(filter.value, filterMissing); - - } else if (isISerializableLineUpFilter(filter)) { - return restoreLineUpFilter(filter.filter, filter.filterMissing); - - } - - throw new Error('Unknown LineUp filter format. Unable to restore the given filter.'); -} - function trackColumn(provider: LocalDataProvider, lineup: IObjectRef, graph: ProvenanceGraph, col: Column) { recordPropertyChange(col, provider, lineup, graph, LineUpTrackAndUntrackActions.metaData); recordPropertyChange(col, provider, lineup, graph, LineUpTrackAndUntrackActions.filter); diff --git a/src/lineup/internal/cmds/filter.ts b/src/lineup/internal/cmds/filter.ts new file mode 100644 index 000000000..1a1c5b614 --- /dev/null +++ b/src/lineup/internal/cmds/filter.ts @@ -0,0 +1,126 @@ + +/** + * Serialize RegExp objects from LineUp string columns as plain object + * that can be stored in the provenance graph + */ +interface IRegExpFilter { + /** + * RegExp as string + */ + value: ILineUpStringFilterValue; + /** + * Flag to indicate the value should be restored as RegExp + */ + isRegExp: boolean; +} + +/** + * This interface combines the `IStringFilter` from `StringColumn` + * and `ICategoricalFilter` from `ICategoricalColumn`. + */ +interface ILineUpStringFilter { + /** + * Filter value + */ + filter: ILineUpStringFilterValue | RegExp; + + /** + * Filter for missing values + */ + filterMissing: boolean; +} + +/** + * Similar to the `ILineUpStringFilter`, but the RegExp is replaced with `IRegExpFilter` + */ +interface ISerializableLineUpFilter { + /** + * Filter value + * Note that the RegExp is replaced with IRegExpFilter (compared to the `ILineUpStringFilter` interface) + */ + filter: ILineUpStringFilterValue | IRegExpFilter; + + /** + * Filter for missing values + */ + filterMissing: boolean; +} + +/** + * Filter value + */ +type ILineUpStringFilterValue = string[] | string | null; + + +/** + * This type guard checks if a given parameter has the `filter` property. + * Necessary since number columns filter has properties `min`, `max` and no filter property, + * @interal + * + * @param filter Any value that could be a filter + */ +export function isSerializedFilter(filter: any): ISerializableLineUpFilter { + return filter && filter.hasOwnProperty('filter'); +} + +/** + * Serializes LineUp string filter, which can contain RegExp objects to an IRegexFilter object. + * The return value of this function can be passed to `JSON.stringify()` and stored in the provenance graph. + * + * Background information: + * The serialization step is necessary, because RegExp objects are converted into an empty object `{}` on `JSON.stringify`. + * ``` + * JSON.stringify(/^123$/gm); // result: {} + * ``` + * + * @internal + * + * @param filter LineUp filter object + * @returns Returns the `ISerializableLineUpFilter` object + */ +export function serializeLineUpFilter(filter: ILineUpStringFilter): ISerializableLineUpFilter { + const value = filter.filter; + const isRegexp = value instanceof RegExp; + return { + filter: { + value: isRegexp ? value.toString() : value as ILineUpStringFilterValue, + isRegExp: isRegexp + }, + filterMissing: filter.filterMissing + }; +} + +/** + * Restores a RegExp object from a given IRegExpFilter object. + * In case a string is passed to this function no deserialization is applied. + * + * @interal + * @param filter Filter as string or plain object matching the IRegExpFilter + * @param filterMissing The flag indicates if missing values should be filtered (default = `false`) + * @returns Returns the input string or the restored RegExp object + */ +export function restoreLineUpFilter(filter: ILineUpStringFilterValue | IRegExpFilter | ISerializableLineUpFilter, filterMissing = false): ILineUpStringFilter { + const isSimpleFilter = (filter: ILineUpStringFilterValue | IRegExpFilter | ISerializableLineUpFilter): filter is ILineUpStringFilterValue => filter === null || typeof filter === 'string' || Array.isArray(filter); + const isIRegExpFilter = ((filter: IRegExpFilter | ISerializableLineUpFilter): filter is IRegExpFilter => filter.hasOwnProperty('isRegExp')); + const isISerializableLineUpFilter = (filter: IRegExpFilter | ISerializableLineUpFilter): filter is ISerializableLineUpFilter => filter.hasOwnProperty('filterMissing'); + + if (isSimpleFilter(filter)) { + return {filter, filterMissing}; + + } else if (isIRegExpFilter(filter)) { + if (filter.isRegExp) { + const serializedRegexParser = /^\/(.+)\/(\w+)?$/; // from https://gist.github.com/tenbits/ec7f0155b57b2d61a6cc90ef3d5f8b49 + const matches = serializedRegexParser.exec(filter.value as string); + const [_full, regexString, regexFlags] = matches; + return {filter: new RegExp(regexString, regexFlags), filterMissing}; + } + + return restoreLineUpFilter(filter.value, filterMissing); + + } else if (isISerializableLineUpFilter(filter)) { + return restoreLineUpFilter(filter.filter, filter.filterMissing); + + } + + throw new Error('Unknown LineUp filter format. Unable to restore the given filter.'); +} diff --git a/tests/lineup/internal/cmds.test.ts b/tests/lineup/internal/cmds/filter.test.ts similarity index 99% rename from tests/lineup/internal/cmds.test.ts rename to tests/lineup/internal/cmds/filter.test.ts index 7a6512e77..1239d67bb 100644 --- a/tests/lineup/internal/cmds.test.ts +++ b/tests/lineup/internal/cmds/filter.test.ts @@ -1,5 +1,5 @@ /// -import {serializeLineUpFilter, restoreLineUpFilter} from '../../../src/lineup/internal/cmds'; +import {serializeLineUpFilter, restoreLineUpFilter} from '../../../../src/lineup/internal/cmds/filter'; // The following tests worked with LineUp v3. // With LineUp v4 the filter object changed From 1d72e896a21c9bccfee5cee62e7bc1825858a4e8 Mon Sep 17 00:00:00 2001 From: oltionchampari Date: Fri, 3 Apr 2020 09:32:09 +0200 Subject: [PATCH 22/26] Test restore for regex filter values datavisyn/tdp_core#345 --- tests/lineup/internal/cmds/filter.test.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/lineup/internal/cmds/filter.test.ts b/tests/lineup/internal/cmds/filter.test.ts index 1239d67bb..3f5c3e943 100644 --- a/tests/lineup/internal/cmds/filter.test.ts +++ b/tests/lineup/internal/cmds/filter.test.ts @@ -81,6 +81,11 @@ describe('Restore LineUp filter from provenance graph', () => { expect(restoreLineUpFilter(fromGraphFilter)).toMatchObject({filter: null, filterMissing: true}); }); + it('filter as RegExp `filterMissing: false', () => { + const fromGraphFilter = {filter: /abc/, filterMissing: false}; + expect(restoreLineUpFilter(fromGraphFilter)).toMatchObject({filter: /abc/, filterMissing: false}); + }); + it('unknown filter format throws error', () => { expect(() => { restoreLineUpFilter(123456789); // typecast to pass unknown format From 09c65c5a0d8ae65634b1ef1351635482381e1660 Mon Sep 17 00:00:00 2001 From: oltionchampari Date: Fri, 3 Apr 2020 09:59:20 +0200 Subject: [PATCH 23/26] Adapt implementation to account for edge case datavisyn/tdp_core#345 --- src/lineup/internal/cmds.ts | 4 ++-- src/lineup/internal/cmds/filter.ts | 12 ++++++------ tests/lineup/internal/cmds/filter.test.ts | 4 ++-- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/lineup/internal/cmds.ts b/src/lineup/internal/cmds.ts index 0e4a0ac80..3816159cb 100644 --- a/src/lineup/internal/cmds.ts +++ b/src/lineup/internal/cmds.ts @@ -7,7 +7,7 @@ import {IObjectRef, action, meta, cat, op, ProvenanceGraph, ICmdResult} from 'ph import {NumberColumn, LocalDataProvider, StackColumn, ScriptColumn, OrdinalColumn, CompositeColumn, Ranking, ISortCriteria, Column, isMapAbleColumn, mappingFunctions} from 'lineupjs'; import {resolveImmediately} from 'phovea_core/src'; import i18n from 'phovea_core/src/i18n'; -import {isSerializedFilter, restoreLineUpFilter, serializeLineUpFilter} from './cmds/filter'; +import {isSerializedFilter, restoreLineUpFilter, serializeLineUpFilter, ILineUpStringFilter} from './cmds/filter'; // used for function calls in the context of tracking or untracking actions in the provenance graph in order to get a consistent defintion of the used strings @@ -357,7 +357,7 @@ function recordPropertyChange(source: Column | Ranking, provider: LocalDataProvi } if (property === LineUpTrackAndUntrackActions.filter) { - newValue = isSerializedFilter(newValue) ? serializeLineUpFilter(newValue) : newValue; // serialize possible RegExp object to be properly stored as provenance graph + newValue = isSerializedFilter(newValue) ? serializeLineUpFilter(newValue as ILineUpStringFilter) : newValue; // serialize possible RegExp object to be properly stored as provenance graph } if (source instanceof Column) { diff --git a/src/lineup/internal/cmds/filter.ts b/src/lineup/internal/cmds/filter.ts index 1a1c5b614..b6fb0685e 100644 --- a/src/lineup/internal/cmds/filter.ts +++ b/src/lineup/internal/cmds/filter.ts @@ -18,7 +18,7 @@ interface IRegExpFilter { * This interface combines the `IStringFilter` from `StringColumn` * and `ICategoricalFilter` from `ICategoricalColumn`. */ -interface ILineUpStringFilter { +export interface ILineUpStringFilter { /** * Filter value */ @@ -59,7 +59,7 @@ type ILineUpStringFilterValue = string[] | string | null; * * @param filter Any value that could be a filter */ -export function isSerializedFilter(filter: any): ISerializableLineUpFilter { +export function isSerializedFilter(filter: any): filter is ISerializableLineUpFilter | ILineUpStringFilter { return filter && filter.hasOwnProperty('filter'); } @@ -99,10 +99,10 @@ export function serializeLineUpFilter(filter: ILineUpStringFilter): ISerializabl * @param filterMissing The flag indicates if missing values should be filtered (default = `false`) * @returns Returns the input string or the restored RegExp object */ -export function restoreLineUpFilter(filter: ILineUpStringFilterValue | IRegExpFilter | ISerializableLineUpFilter, filterMissing = false): ILineUpStringFilter { - const isSimpleFilter = (filter: ILineUpStringFilterValue | IRegExpFilter | ISerializableLineUpFilter): filter is ILineUpStringFilterValue => filter === null || typeof filter === 'string' || Array.isArray(filter); - const isIRegExpFilter = ((filter: IRegExpFilter | ISerializableLineUpFilter): filter is IRegExpFilter => filter.hasOwnProperty('isRegExp')); - const isISerializableLineUpFilter = (filter: IRegExpFilter | ISerializableLineUpFilter): filter is ISerializableLineUpFilter => filter.hasOwnProperty('filterMissing'); +export function restoreLineUpFilter(filter: ILineUpStringFilterValue | IRegExpFilter | ISerializableLineUpFilter | ILineUpStringFilter, filterMissing = false): ILineUpStringFilter { + const isSimpleFilter = (filter: ILineUpStringFilterValue | IRegExpFilter | ISerializableLineUpFilter | ILineUpStringFilter): filter is ILineUpStringFilterValue => filter === null || typeof filter === 'string' || Array.isArray(filter) || filter instanceof RegExp; + const isIRegExpFilter = ((filter: IRegExpFilter | ISerializableLineUpFilter | ILineUpStringFilter): filter is IRegExpFilter => filter.hasOwnProperty('isRegExp')); + const isISerializableLineUpFilter = (filter: IRegExpFilter | ISerializableLineUpFilter | ILineUpStringFilter): filter is ISerializableLineUpFilter => filter.hasOwnProperty('filterMissing'); if (isSimpleFilter(filter)) { return {filter, filterMissing}; diff --git a/tests/lineup/internal/cmds/filter.test.ts b/tests/lineup/internal/cmds/filter.test.ts index 3f5c3e943..bbb6becab 100644 --- a/tests/lineup/internal/cmds/filter.test.ts +++ b/tests/lineup/internal/cmds/filter.test.ts @@ -1,5 +1,5 @@ /// -import {serializeLineUpFilter, restoreLineUpFilter} from '../../../../src/lineup/internal/cmds/filter'; +import {serializeLineUpFilter, restoreLineUpFilter, ILineUpStringFilter} from '../../../../src/lineup/internal/cmds/filter'; // The following tests worked with LineUp v3. // With LineUp v4 the filter object changed @@ -83,7 +83,7 @@ describe('Restore LineUp filter from provenance graph', () => { it('filter as RegExp `filterMissing: false', () => { const fromGraphFilter = {filter: /abc/, filterMissing: false}; - expect(restoreLineUpFilter(fromGraphFilter)).toMatchObject({filter: /abc/, filterMissing: false}); + expect(restoreLineUpFilter(fromGraphFilter)).toMatchObject({filter: /abc/, filterMissing: false}); }); it('unknown filter format throws error', () => { From 2f169459b03778e8c33454669edb0bf514a756b0 Mon Sep 17 00:00:00 2001 From: oltionchampari Date: Fri, 3 Apr 2020 10:10:01 +0200 Subject: [PATCH 24/26] Comment --- src/lineup/internal/cmds/filter.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/lineup/internal/cmds/filter.ts b/src/lineup/internal/cmds/filter.ts index b6fb0685e..4af638af2 100644 --- a/src/lineup/internal/cmds/filter.ts +++ b/src/lineup/internal/cmds/filter.ts @@ -55,9 +55,9 @@ type ILineUpStringFilterValue = string[] | string | null; /** * This type guard checks if a given parameter has the `filter` property. * Necessary since number columns filter has properties `min`, `max` and no filter property, - * @interal - * + * @internal * @param filter Any value that could be a filter + * @returns Returns true if filter should be serialized/restored or false if not. */ export function isSerializedFilter(filter: any): filter is ISerializableLineUpFilter | ILineUpStringFilter { return filter && filter.hasOwnProperty('filter'); From a4bf429bb475b5b5575a609cd256bb5a1dc6da36 Mon Sep 17 00:00:00 2001 From: thinkh Date: Fri, 3 Apr 2020 14:22:38 +0200 Subject: [PATCH 25/26] Improve type guards and refactor code #331 --- src/lineup/internal/cmds/filter.ts | 72 ++++++++++++++--------- tests/lineup/internal/cmds/filter.test.ts | 5 -- 2 files changed, 45 insertions(+), 32 deletions(-) diff --git a/src/lineup/internal/cmds/filter.ts b/src/lineup/internal/cmds/filter.ts index 4af638af2..23fc09a76 100644 --- a/src/lineup/internal/cmds/filter.ts +++ b/src/lineup/internal/cmds/filter.ts @@ -1,3 +1,16 @@ +/** + * Basic LineUp string filter values + */ +type LineUpStringFilterValue = string[] | string | null; + +/** + * This type guard checks if `filter` parameter matches the `LineUpStringFilterValue` type + * @param filter Any filterable value that should be checked + * @returns Returns true if filter applies to the `LineUpStringFilterValue` + */ +function isLineUpStringFilterValue(filter: any): filter is LineUpStringFilterValue { + return filter === null || typeof filter === 'string' || Array.isArray(filter); +} /** * Serialize RegExp objects from LineUp string columns as plain object @@ -7,13 +20,22 @@ interface IRegExpFilter { /** * RegExp as string */ - value: ILineUpStringFilterValue; + value: LineUpStringFilterValue; /** * Flag to indicate the value should be restored as RegExp */ isRegExp: boolean; } +/** + * This type guard checks if `filter` parameter matches the `IRegExpFilter` type. + * @param filter Any filterable value that should be checked + * @returns Returns true if filter applies to the `IRegExpFilter` + */ +function isIRegExpFilter(filter: any): filter is IRegExpFilter { + return filter && filter.hasOwnProperty('value') && isLineUpStringFilterValue(filter.value) && filter.hasOwnProperty('isRegExp'); +} + /** * This interface combines the `IStringFilter` from `StringColumn` * and `ICategoricalFilter` from `ICategoricalColumn`. @@ -22,7 +44,7 @@ export interface ILineUpStringFilter { /** * Filter value */ - filter: ILineUpStringFilterValue | RegExp; + filter: LineUpStringFilterValue | RegExp; /** * Filter for missing values @@ -38,7 +60,7 @@ interface ISerializableLineUpFilter { * Filter value * Note that the RegExp is replaced with IRegExpFilter (compared to the `ILineUpStringFilter` interface) */ - filter: ILineUpStringFilterValue | IRegExpFilter; + filter: LineUpStringFilterValue | IRegExpFilter; /** * Filter for missing values @@ -47,20 +69,15 @@ interface ISerializableLineUpFilter { } /** - * Filter value - */ -type ILineUpStringFilterValue = string[] | string | null; - - -/** - * This type guard checks if a given parameter has the `filter` property. - * Necessary since number columns filter has properties `min`, `max` and no filter property, + * This type guard checks if the `filter` parameter matches the `ISerializableLineUpFilter` type. + * Necessary since number columns filter has properties `min`, `max` and no filter property. + * * @internal * @param filter Any value that could be a filter * @returns Returns true if filter should be serialized/restored or false if not. */ -export function isSerializedFilter(filter: any): filter is ISerializableLineUpFilter | ILineUpStringFilter { - return filter && filter.hasOwnProperty('filter'); +export function isSerializedFilter(filter: any): filter is ISerializableLineUpFilter { + return filter && filter.hasOwnProperty('filter') && (isLineUpStringFilterValue(filter.filter) || isIRegExpFilter(filter.filter)) && filter.hasOwnProperty('filterMissing'); } /** @@ -80,31 +97,32 @@ export function isSerializedFilter(filter: any): filter is ISerializableLineUpFi */ export function serializeLineUpFilter(filter: ILineUpStringFilter): ISerializableLineUpFilter { const value = filter.filter; - const isRegexp = value instanceof RegExp; + const isRegExp = value instanceof RegExp; return { filter: { - value: isRegexp ? value.toString() : value as ILineUpStringFilterValue, - isRegExp: isRegexp + value: isRegExp ? value.toString() : value as LineUpStringFilterValue, + isRegExp }, filterMissing: filter.filterMissing }; } /** - * Restores a RegExp object from a given IRegExpFilter object. - * In case a string is passed to this function no deserialization is applied. + * Restores filter values from the provenance graph and returns an `ILineUpStringFilter` + * which can be passed to the LineUp instance (using LineUp > 4.0.0). + * + * Valid seralized filter values are: + * - `LineUpStringFilterValue` used with LineUp < 4.0.0 and tdp_core < 9.0.0 + * - `IRegExpFilter` used with LineUp < 4.0.0 and tdp_core >= 9.0.0 + * - `ISerializableLineUpFilter` used with LineUp > 4.0.0 and tdp_core > 9.1.0 * * @interal - * @param filter Filter as string or plain object matching the IRegExpFilter + * @param filter Filter with one of the types described above * @param filterMissing The flag indicates if missing values should be filtered (default = `false`) - * @returns Returns the input string or the restored RegExp object + * @returns Returns an `ILineUpStringFilter` which can be passed to LineUp */ -export function restoreLineUpFilter(filter: ILineUpStringFilterValue | IRegExpFilter | ISerializableLineUpFilter | ILineUpStringFilter, filterMissing = false): ILineUpStringFilter { - const isSimpleFilter = (filter: ILineUpStringFilterValue | IRegExpFilter | ISerializableLineUpFilter | ILineUpStringFilter): filter is ILineUpStringFilterValue => filter === null || typeof filter === 'string' || Array.isArray(filter) || filter instanceof RegExp; - const isIRegExpFilter = ((filter: IRegExpFilter | ISerializableLineUpFilter | ILineUpStringFilter): filter is IRegExpFilter => filter.hasOwnProperty('isRegExp')); - const isISerializableLineUpFilter = (filter: IRegExpFilter | ISerializableLineUpFilter | ILineUpStringFilter): filter is ISerializableLineUpFilter => filter.hasOwnProperty('filterMissing'); - - if (isSimpleFilter(filter)) { +export function restoreLineUpFilter(filter: LineUpStringFilterValue | IRegExpFilter | ISerializableLineUpFilter, filterMissing = false): ILineUpStringFilter { + if (isLineUpStringFilterValue(filter)) { return {filter, filterMissing}; } else if (isIRegExpFilter(filter)) { @@ -117,7 +135,7 @@ export function restoreLineUpFilter(filter: ILineUpStringFilterValue | IRegExpFi return restoreLineUpFilter(filter.value, filterMissing); - } else if (isISerializableLineUpFilter(filter)) { + } else if (isSerializedFilter(filter)) { return restoreLineUpFilter(filter.filter, filter.filterMissing); } diff --git a/tests/lineup/internal/cmds/filter.test.ts b/tests/lineup/internal/cmds/filter.test.ts index bbb6becab..45d10c305 100644 --- a/tests/lineup/internal/cmds/filter.test.ts +++ b/tests/lineup/internal/cmds/filter.test.ts @@ -81,11 +81,6 @@ describe('Restore LineUp filter from provenance graph', () => { expect(restoreLineUpFilter(fromGraphFilter)).toMatchObject({filter: null, filterMissing: true}); }); - it('filter as RegExp `filterMissing: false', () => { - const fromGraphFilter = {filter: /abc/, filterMissing: false}; - expect(restoreLineUpFilter(fromGraphFilter)).toMatchObject({filter: /abc/, filterMissing: false}); - }); - it('unknown filter format throws error', () => { expect(() => { restoreLineUpFilter(123456789); // typecast to pass unknown format From 304e51fee28754ecbf4a5e72b03fe53c6f7da28a Mon Sep 17 00:00:00 2001 From: thinkh Date: Fri, 3 Apr 2020 15:31:39 +0200 Subject: [PATCH 26/26] Add type guard filter + refactore restore regexp #331 --- src/lineup/internal/cmds.ts | 4 +- src/lineup/internal/cmds/filter.ts | 45 ++++++++++++++++---- tests/lineup/internal/cmds/filter.test.ts | 51 ++++++++++++++++++++++- 3 files changed, 90 insertions(+), 10 deletions(-) diff --git a/src/lineup/internal/cmds.ts b/src/lineup/internal/cmds.ts index 3816159cb..325c6d6f7 100644 --- a/src/lineup/internal/cmds.ts +++ b/src/lineup/internal/cmds.ts @@ -7,7 +7,7 @@ import {IObjectRef, action, meta, cat, op, ProvenanceGraph, ICmdResult} from 'ph import {NumberColumn, LocalDataProvider, StackColumn, ScriptColumn, OrdinalColumn, CompositeColumn, Ranking, ISortCriteria, Column, isMapAbleColumn, mappingFunctions} from 'lineupjs'; import {resolveImmediately} from 'phovea_core/src'; import i18n from 'phovea_core/src/i18n'; -import {isSerializedFilter, restoreLineUpFilter, serializeLineUpFilter, ILineUpStringFilter} from './cmds/filter'; +import {isSerializedFilter, restoreLineUpFilter, serializeLineUpFilter, isLineUpStringFilter} from './cmds/filter'; // used for function calls in the context of tracking or untracking actions in the provenance graph in order to get a consistent defintion of the used strings @@ -357,7 +357,7 @@ function recordPropertyChange(source: Column | Ranking, provider: LocalDataProvi } if (property === LineUpTrackAndUntrackActions.filter) { - newValue = isSerializedFilter(newValue) ? serializeLineUpFilter(newValue as ILineUpStringFilter) : newValue; // serialize possible RegExp object to be properly stored as provenance graph + newValue = isLineUpStringFilter(newValue) ? serializeLineUpFilter(newValue) : newValue; // serialize possible RegExp object to be properly stored as provenance graph } if (source instanceof Column) { diff --git a/src/lineup/internal/cmds/filter.ts b/src/lineup/internal/cmds/filter.ts index 23fc09a76..400855c8f 100644 --- a/src/lineup/internal/cmds/filter.ts +++ b/src/lineup/internal/cmds/filter.ts @@ -40,7 +40,7 @@ function isIRegExpFilter(filter: any): filter is IRegExpFilter { * This interface combines the `IStringFilter` from `StringColumn` * and `ICategoricalFilter` from `ICategoricalColumn`. */ -export interface ILineUpStringFilter { +interface ILineUpStringFilter { /** * Filter value */ @@ -52,6 +52,18 @@ export interface ILineUpStringFilter { filterMissing: boolean; } +/** + * This type guard checks if the `filter` parameter matches the `isLineUpStringFilter` type. + * + * @internal + * @param filter Any value that could be a filter + * @returns Returns true if filter should be serialized/restored or false if not. + */ +export function isLineUpStringFilter(filter: any): filter is ILineUpStringFilter { + return filter && filter.hasOwnProperty('filter') && (isLineUpStringFilterValue(filter.filter) || filter.filter instanceof RegExp) && filter.hasOwnProperty('filterMissing'); +} + + /** * Similar to the `ILineUpStringFilter`, but the RegExp is replaced with `IRegExpFilter` */ @@ -107,6 +119,25 @@ export function serializeLineUpFilter(filter: ILineUpStringFilter): ISerializabl }; } +/** + * Coverts a RegExp string to a RegExp instance + * + * @internal + * @param value RegExp string + * @returns The RegExp instance + */ +export function restoreRegExp(value: string): RegExp { + const serializedRegexParser = /^\/(.+)\/(\w+)?$/; // from https://gist.github.com/tenbits/ec7f0155b57b2d61a6cc90ef3d5f8b49 + const matches = serializedRegexParser.exec(value); + + if(matches === null) { + throw new Error('Unable to parse regular expression from string. The string does not seem to be a valid RegExp.'); + } + + const [_full, regexString, regexFlags] = matches; + return new RegExp(regexString, regexFlags); +} + /** * Restores filter values from the provenance graph and returns an `ILineUpStringFilter` * which can be passed to the LineUp instance (using LineUp > 4.0.0). @@ -125,14 +156,14 @@ export function restoreLineUpFilter(filter: LineUpStringFilterValue | IRegExpFil if (isLineUpStringFilterValue(filter)) { return {filter, filterMissing}; - } else if (isIRegExpFilter(filter)) { - if (filter.isRegExp) { - const serializedRegexParser = /^\/(.+)\/(\w+)?$/; // from https://gist.github.com/tenbits/ec7f0155b57b2d61a6cc90ef3d5f8b49 - const matches = serializedRegexParser.exec(filter.value as string); - const [_full, regexString, regexFlags] = matches; - return {filter: new RegExp(regexString, regexFlags), filterMissing}; + } else if (isIRegExpFilter(filter) && filter.isRegExp === true) { + if(typeof filter.value === 'string') { + return {filter: restoreRegExp(filter.value), filterMissing}; } + throw new Error('Wrong type of filter value. Unable to restore RegExp instance from the given filter value.'); + + } else if (isIRegExpFilter(filter) && filter.isRegExp === false) { return restoreLineUpFilter(filter.value, filterMissing); } else if (isSerializedFilter(filter)) { diff --git a/tests/lineup/internal/cmds/filter.test.ts b/tests/lineup/internal/cmds/filter.test.ts index 45d10c305..5154be53f 100644 --- a/tests/lineup/internal/cmds/filter.test.ts +++ b/tests/lineup/internal/cmds/filter.test.ts @@ -1,5 +1,5 @@ /// -import {serializeLineUpFilter, restoreLineUpFilter, ILineUpStringFilter} from '../../../../src/lineup/internal/cmds/filter'; +import {serializeLineUpFilter, restoreLineUpFilter, isSerializedFilter, isLineUpStringFilter, restoreRegExp} from '../../../../src/lineup/internal/cmds/filter'; // The following tests worked with LineUp v3. // With LineUp v4 the filter object changed @@ -16,6 +16,55 @@ import {serializeLineUpFilter, restoreLineUpFilter, ILineUpStringFilter} from '. // }); // }); +describe('Type guard isLineUpStringFilter', () => { + it('isLineUpStringFilter with string', () => { + expect(isLineUpStringFilter({filter: 'abc', filterMissing: false})).toBe(true); + }); + + it('isLineUpStringFilter with string[]', () => { + expect(isLineUpStringFilter({filter: ['abc', 'def'], filterMissing: false})).toBe(true); + }); + + it('isLineUpStringFilter with null', () => { + expect(isLineUpStringFilter({filter: null, filterMissing: false})).toBe(true); + }); + + it('isLineUpStringFilter with RegExp', () => { + expect(isLineUpStringFilter({filter: /abc/gm, filterMissing: false})).toBe(true); + }); +}); + +describe('Type guard isSerializedFilter', () => { + it('isSerializedFilter with string', () => { + expect(isSerializedFilter({filter: {value: 'abc', isRegExp: false}, filterMissing: false})).toBe(true); + }); + + it('isSerializedFilter with string[]', () => { + expect(isSerializedFilter({filter: {value: ['abc', 'def'], isRegExp: false}, filterMissing: false})).toBe(true); + }); + + it('isSerializedFilter with null', () => { + expect(isSerializedFilter({filter: {value: null, isRegExp: false}, filterMissing: false})).toBe(true); + }); + + it('isSerializedFilter with RegExp', () => { + expect(isSerializedFilter({filter: {value: '/abc/gm', isRegExp: true}, filterMissing: false})).toBe(true); + }); +}); + +describe('Restore RegExp from string', () => { + it('valid RegExp string', () => { + expect(restoreRegExp('/abc/gm').toString()).toBe('/abc/gm'); + expect(restoreRegExp('/def/gm').toString()).not.toBe('/abc/gm'); + }); + + it('invalid RegExp string', () => { + expect(() => { + restoreRegExp('invalid regexp string').toString(); + }).toThrow(new Error('Unable to parse regular expression from string. The string does not seem to be a valid RegExp.')); + }); +}); + describe('Serialize LineUp v4 filter for provenance graph', () => { it('filter as string', () => { const lineUpFilter = {filter: 'abc', filterMissing: false};