From fec1ce879507998a04088bf494cfd5a595e90160 Mon Sep 17 00:00:00 2001 From: Ghislain B Date: Tue, 5 Jan 2021 09:08:19 -0500 Subject: [PATCH] fix(backend): OData queries with input filter (#224) * fix(backend): OData queries with input filter * Improved escaping/normalizing search terms for string/text/readonly fields. * Invalid characters from integer/float/number fields with string search terms are now removed. * Added support for range filter `..` to function without an upper bound. When for example `2..` is input then `field GE 2` or `field GT 2` is send to the backend, depending on the `defaultFilterRangeOperator` * Added support for range filter `..` to function without a lower bound. When for example `..2` is input `field LE 2` or `field LT 2` is send, depending on the `defaultFilterRangeOperator` --- .../__tests__/grid-odata.service.spec.ts | 120 +++++++++- .../odata/src/services/grid-odata.service.ts | 223 +++++++++--------- 2 files changed, 228 insertions(+), 115 deletions(-) diff --git a/packages/odata/src/services/__tests__/grid-odata.service.spec.ts b/packages/odata/src/services/__tests__/grid-odata.service.spec.ts index 5c34f9347..fbc24678e 100644 --- a/packages/odata/src/services/__tests__/grid-odata.service.spec.ts +++ b/packages/odata/src/services/__tests__/grid-odata.service.spec.ts @@ -23,7 +23,7 @@ const DEFAULT_PAGE_SIZE = 20; const gridOptionMock = { enablePagination: true, - defaultFilterRangeOperator: 'RangeExclusive', + defaultFilterRangeOperator: OperatorType.rangeInclusive, backendServiceApi: { service: undefined, preProcess: jest.fn(), @@ -963,6 +963,62 @@ describe('GridOdataService', () => { expect(query).toBe(expectation); }); + it('should return a query to filter a search value between an inclusive range of numbers using the 2 dots (..) separator, the "RangeInclusive" operator and the range has an unbounded end', () => { + const expectation = `$top=10&$filter=(Duration ge 5)`; + const mockColumnDuration = { id: 'duration', field: 'duration', type: FieldType.number } as Column; + const mockColumnFilters = { + duration: { columnId: 'duration', columnDef: mockColumnDuration, searchTerms: ['5..'], operator: 'RangeInclusive' }, + } as ColumnFilters; + + service.init(serviceOptions, paginationOptions, gridStub); + service.updateFilters(mockColumnFilters, false); + const query = service.buildQuery(); + + expect(query).toBe(expectation); + }); + + it('should return a query to filter a search value between an inclusive range of numbers using the 2 dots (..) separator, the "RangeInclusive" operator and the range has an unbounded begin', () => { + const expectation = `$top=10&$filter=(Duration le 5)`; + const mockColumnDuration = { id: 'duration', field: 'duration', type: FieldType.number } as Column; + const mockColumnFilters = { + duration: { columnId: 'duration', columnDef: mockColumnDuration, searchTerms: ['..5'], operator: 'RangeInclusive' }, + } as ColumnFilters; + + service.init(serviceOptions, paginationOptions, gridStub); + service.updateFilters(mockColumnFilters, false); + const query = service.buildQuery(); + + expect(query).toBe(expectation); + }); + + it('should return a query to filter a search value between an inclusive range of numbers using the 2 dots (..) separator, the "RangeExclusive" operator and the range has an unbounded end', () => { + const expectation = `$top=10&$filter=(Duration gt 5)`; + const mockColumnDuration = { id: 'duration', field: 'duration', type: FieldType.number } as Column; + const mockColumnFilters = { + duration: { columnId: 'duration', columnDef: mockColumnDuration, searchTerms: ['5..'], operator: 'RangeExclusive' }, + } as ColumnFilters; + + service.init(serviceOptions, paginationOptions, gridStub); + service.updateFilters(mockColumnFilters, false); + const query = service.buildQuery(); + + expect(query).toBe(expectation); + }); + + it('should return a query to filter a search value between an inclusive range of numbers using the 2 dots (..) separator, the "RangeExclusive" operator and the range has an unbounded begin', () => { + const expectation = `$top=10&$filter=(Duration lt 5)`; + const mockColumnDuration = { id: 'duration', field: 'duration', type: FieldType.number } as Column; + const mockColumnFilters = { + duration: { columnId: 'duration', columnDef: mockColumnDuration, searchTerms: ['..5'], operator: 'RangeExclusive' }, + } as ColumnFilters; + + service.init(serviceOptions, paginationOptions, gridStub); + service.updateFilters(mockColumnFilters, false); + const query = service.buildQuery(); + + expect(query).toBe(expectation); + }); + it('should return a query to filter a search value between an exclusive range of numbers using 2 search terms and the "RangeExclusive" operator', () => { const expectation = `$top=10&$filter=(substringof('abc', Company) and (Duration gt 5 and Duration lt 22))`; const mockColumnCompany = { id: 'company', field: 'company' } as Column; @@ -1432,10 +1488,10 @@ describe('GridOdataService', () => { expect(currentFilters).toEqual(presetFilters); }); - it('should return a query with a filter with range of numbers with decimals when the preset is a filter range with 3 dots (...) separator', () => { + it('should return a query with a filter with range of numbers with decimals when the preset is a filter range with 2 dots (..) separator and range ends with a fraction', () => { const columns = [{ id: 'company', field: 'company' }, { id: 'gender', field: 'gender' }, { id: 'duration', field: 'duration', type: FieldType.number }]; jest.spyOn(gridStub, 'getColumns').mockReturnValue(columns); - const expectation = `$top=10&$filter=(Duration ge 0.5 and Duration le .88)`; + const expectation = `$top=10&$filter=(Duration ge 0.5 and Duration le 0.88)`; const presetFilters = [ { columnId: 'duration', searchTerms: ['0.5...88'] }, ] as CurrentFilter[]; @@ -1503,7 +1559,7 @@ describe('GridOdataService', () => { it('should return a query with a filter with range of dates inclusive when the preset is a filter range with 2 searchTerms without an operator', () => { const columns = [{ id: 'company', field: 'company' }, { id: 'gender', field: 'gender' }, { id: 'finish', field: 'finish', type: FieldType.date }]; jest.spyOn(gridStub, 'getColumns').mockReturnValue(columns); - const expectation = `$top=10&$filter=(Finish gt DateTime'2001-01-01T00:00:00Z' and Finish lt DateTime'2001-01-31T00:00:00Z')`; + const expectation = `$top=10&$filter=(Finish ge DateTime'2001-01-01T00:00:00Z' and Finish le DateTime'2001-01-31T00:00:00Z')`; const presetFilters = [ { columnId: 'finish', searchTerms: ['2001-01-01', '2001-01-31'] }, ] as CurrentFilter[]; @@ -1554,6 +1610,62 @@ describe('GridOdataService', () => { expect(query).toBe(expectation); expect(currentFilters).toEqual(presetFilters); }); + + it('should return a query to filter a search value with a fraction of a number that is missing a leading 0', () => { + const expectation = `$filter=(Duration eq 0.22)`; + const mockColumnDuration = { id: 'duration', field: 'duration', type: FieldType.float } as Column; + const mockColumnFilters = { + duration: { columnId: 'duration', columnDef: mockColumnDuration, searchTerms: ['.22'] }, + } as ColumnFilters; + + service.init(serviceOptions, paginationOptions, gridStub); + service.updateFilters(mockColumnFilters, false); + const query = service.buildQuery(); + + expect(query).toBe(expectation); + }); + + it('should return a query without invalid characters to filter a search value that does contains invalid characters', () => { + const expectation = `$filter=(Duration eq -22)`; + const mockColumnDuration = { id: 'duration', field: 'duration', type: FieldType.number } as Column; + const mockColumnFilters = { + duration: { columnId: 'duration', columnDef: mockColumnDuration, searchTerms: ['-2a2'] }, + } as ColumnFilters; + + service.init(serviceOptions, paginationOptions, gridStub); + service.updateFilters(mockColumnFilters, false); + const query = service.buildQuery(); + + expect(query).toBe(expectation); + }); + + it('should return a query without invalid characters to filter a search value with an integer that contains invalid characters', () => { + const expectation = `$filter=(Duration eq 22)`; + const mockColumnDuration = { id: 'duration', field: 'duration', type: FieldType.integer } as Column; + const mockColumnFilters = { + duration: { columnId: 'duration', columnDef: mockColumnDuration, searchTerms: ['22;'] }, + } as ColumnFilters; + + service.init(serviceOptions, paginationOptions, gridStub); + service.updateFilters(mockColumnFilters, false); + const query = service.buildQuery(); + + expect(query).toBe(expectation); + }); + + it('should return a query with 0 to filter a search value when the search value contains a minus', () => { + const expectation = `$filter=(Duration eq 0)`; + const mockColumnDuration = { id: 'duration', field: 'duration', type: FieldType.number } as Column; + const mockColumnFilters = { + duration: { columnId: 'duration', columnDef: mockColumnDuration, searchTerms: ['-'] }, + } as ColumnFilters; + + service.init(serviceOptions, paginationOptions, gridStub); + service.updateFilters(mockColumnFilters, false); + const query = service.buildQuery(); + + expect(query).toBe(expectation); + }); }); }); diff --git a/packages/odata/src/services/grid-odata.service.ts b/packages/odata/src/services/grid-odata.service.ts index dd6d112a2..8ef887050 100644 --- a/packages/odata/src/services/grid-odata.service.ts +++ b/packages/odata/src/services/grid-odata.service.ts @@ -261,7 +261,7 @@ export class GridOdataService implements BackendService { let fieldName = columnDef.filter?.queryField || columnDef.queryFieldFilter || columnDef.queryField || columnDef.field || columnDef.name || ''; const fieldType = columnDef.type || FieldType.string; - let searchTerms = (columnFilter ? columnFilter.searchTerms : null) || []; + let searchTerms = (columnFilter && columnFilter.searchTerms ? [...columnFilter.searchTerms] : null) || []; let fieldSearchValue = (Array.isArray(searchTerms) && searchTerms.length === 1) ? searchTerms[0] : ''; if (typeof fieldSearchValue === 'undefined') { fieldSearchValue = ''; @@ -284,16 +284,22 @@ export class GridOdataService implements BackendService { continue; } - if (Array.isArray(searchTerms) && searchTerms.length === 1 && typeof searchTerms[0] === 'string' && searchTerms[0].indexOf('..') > 0) { - searchTerms = searchTerms[0].split('..'); + if (Array.isArray(searchTerms) && searchTerms.length === 1 && typeof searchTerms[0] === 'string' && searchTerms[0].indexOf('..') >= 0) { if (!operator) { - operator = OperatorType.rangeInclusive; + operator = this._gridOptions.defaultFilterRangeOperator; } - } - // escaping the search value - searchValue = searchValue.replace(`'`, `''`); // escape single quotes by doubling them - searchValue = encodeURIComponent(searchValue); // encode URI of the final search value + searchTerms = searchTerms[0].split('..', 2); + if (searchTerms[0] === '') { + operator = operator === OperatorType.rangeInclusive ? '<=' : operator === OperatorType.rangeExclusive ? '<' : operator; + searchTerms = searchTerms.slice(1); + searchValue = searchTerms[0]; + } else if (searchTerms[1] === '') { + operator = operator === OperatorType.rangeInclusive ? '>=' : operator === OperatorType.rangeExclusive ? '>' : operator; + searchTerms = searchTerms.slice(0, 1); + searchValue = searchTerms[0]; + } + } // if we didn't find an Operator but we have a Column Operator inside the Filter (DOM Element), we should use its default Operator // multipleSelect is "IN", while singleSelect is "EQ", else don't map any operator @@ -301,9 +307,19 @@ export class GridOdataService implements BackendService { operator = columnDef.filter.operator; } + // No operator and 2 search terms should lead to default range operator. + if (!operator && Array.isArray(searchTerms) && searchTerms.length === 2 && searchTerms[0] && searchTerms[1]) { + operator = this._gridOptions.defaultFilterRangeOperator; + } + + // Range with 1 searchterm should lead to equals for a date field. + if ((operator === OperatorType.rangeInclusive || OperatorType.rangeExclusive) && Array.isArray(searchTerms) && searchTerms.length === 1 && fieldType === FieldType.date) { + operator = OperatorType.equal; + } + // if we still don't have an operator find the proper Operator to use by it's field type if (!operator) { - operator = mapOperatorByFieldType(columnDef.type || FieldType.string); + operator = mapOperatorByFieldType(fieldType); } // extra query arguments @@ -313,6 +329,14 @@ export class GridOdataService implements BackendService { this.saveColumnFilter(fieldName, fieldSearchValue, searchTerms); } } else { + // Normalize all search values + searchValue = this.normalizeSearchValue(fieldType, searchValue, odataVersion); + if (Array.isArray(searchTerms)) { + searchTerms.forEach((_part, index) => { + searchTerms[index] = this.normalizeSearchValue(fieldType, searchTerms[index], odataVersion); + }); + } + searchBy = ''; // titleCase the fieldName so that it matches the WebApi names @@ -320,63 +344,40 @@ export class GridOdataService implements BackendService { fieldName = titleCase(fieldName || ''); } - if (fieldType === FieldType.date) { - searchBy = this.filterBySearchDate(fieldName, operator, searchTerms, odataVersion); - } else if (searchTerms && searchTerms.length > 1 && (operator === 'IN' || operator === 'NIN' || operator === 'NOTIN' || operator === 'NOT IN' || operator === 'NOT_IN')) { + if (searchTerms && searchTerms.length > 1 && (operator === 'IN' || operator === 'NIN' || operator === 'NOTIN' || operator === 'NOT IN' || operator === 'NOT_IN')) { // when having more than 1 search term (then check if we have a "IN" or "NOT IN" filter search) - const tmpSearchTerms = []; - + const tmpSearchTerms: string[] = []; if (operator === 'IN') { // example:: (Stage eq "Expired" or Stage eq "Renewal") for (let j = 0, lnj = searchTerms.length; j < lnj; j++) { - if (fieldType === FieldType.string || fieldType === FieldType.text || fieldType === FieldType.readonly) { - const searchVal = encodeURIComponent(searchTerms[j].replace(`'`, `''`)); - tmpSearchTerms.push(`${fieldName} eq '${searchVal}'`); - } else { - // Single quote escape is not needed for non string type - tmpSearchTerms.push(`${fieldName} eq ${searchTerms[j]}`); - } + tmpSearchTerms.push(`${fieldName} eq ${searchTerms[j]}`); } searchBy = tmpSearchTerms.join(' or '); - if (!(typeof searchBy === 'string' && searchBy[0] === '(' && searchBy.slice(-1) === ')')) { - searchBy = `(${searchBy})`; - } } else { // example:: (Stage ne "Expired" and Stage ne "Renewal") for (let k = 0, lnk = searchTerms.length; k < lnk; k++) { - const searchVal = encodeURIComponent(searchTerms[k].replace(`'`, `''`)); - tmpSearchTerms.push(`${fieldName} ne '${searchVal}'`); + tmpSearchTerms.push(`${fieldName} ne ${searchTerms[k]}`); } searchBy = tmpSearchTerms.join(' and '); - if (!(typeof searchBy === 'string' && searchBy[0] === '(' && searchBy.slice(-1) === ')')) { - searchBy = `(${searchBy})`; - } + } + if (!(typeof searchBy === 'string' && searchBy[0] === '(' && searchBy.slice(-1) === ')')) { + searchBy = `(${searchBy})`; } } else if (operator === '*' || operator === 'a*' || operator === '*z' || lastValueChar === '*' || operator === OperatorType.startsWith || operator === OperatorType.endsWith) { // first/last character is a '*' will be a startsWith or endsWith - searchBy = (operator === '*' || operator === '*z' || operator === OperatorType.endsWith) ? `endswith(${fieldName}, '${searchValue}')` : `startswith(${fieldName}, '${searchValue}')`; - } else if (fieldType === FieldType.string || fieldType === FieldType.text || fieldType === FieldType.readonly) { - // string field needs to be in single quotes - if (operator === '' || operator === OperatorType.contains || operator === OperatorType.notContains) { - searchBy = this.odataQueryVersionWrapper('substring', odataVersion, fieldName, searchValue); - if (operator === OperatorType.notContains) { - searchBy = `not ${searchBy}`; - } - } else if (operator === OperatorType.rangeExclusive || operator === OperatorType.rangeInclusive) { - // example:: (Duration >= 5 and Duration <= 10) - searchBy = this.filterBySearchTermRange(fieldName, operator, searchTerms); - } else { - searchBy = `${fieldName} ${this.mapOdataOperator(operator)} '${searchValue}'`; + searchBy = (operator === '*' || operator === '*z' || operator === OperatorType.endsWith) ? `endswith(${fieldName}, ${searchValue})` : `startswith(${fieldName}, ${searchValue})`; + } else if (operator === OperatorType.rangeExclusive || operator === OperatorType.rangeInclusive) { + // example:: (Name >= 'Bob' and Name <= 'Jane') + searchBy = this.filterBySearchTermRange(fieldName, operator, searchTerms); + } else if ((operator === '' || operator === OperatorType.contains || operator === OperatorType.notContains) && + (fieldType === FieldType.string || fieldType === FieldType.text || fieldType === FieldType.readonly)) { + searchBy = odataVersion >= 4 ? `contains(${fieldName}, ${searchValue})` : `substringof(${searchValue}, ${fieldName})`; + if (operator === OperatorType.notContains) { + searchBy = `not ${searchBy}`; } } else { - if (operator === OperatorType.rangeExclusive || operator === OperatorType.rangeInclusive) { - // example:: (Duration >= 5 and Duration <= 10) - searchBy = this.filterBySearchTermRange(fieldName, operator, searchTerms); - } else { - // any other field type (or undefined type) - searchValue = (fieldType === FieldType.number || fieldType === FieldType.boolean) ? searchValue : `'${searchValue}'`; - searchBy = `${fieldName} ${this.mapOdataOperator(operator)} ${searchValue}`; - } + // any other field type (or undefined type) + searchBy = `${fieldName} ${this.mapOdataOperator(operator)} ${searchValue}`; } // push to our temp array and also trim white spaces @@ -403,7 +404,7 @@ export class GridOdataService implements BackendService { updatePagination(newPage: number, pageSize: number) { this._currentPagination = { pageNumber: newPage, - pageSize + pageSize, }; // unless user specifically set "enablePagination" to False, we'll update pagination options in every other cases @@ -453,7 +454,7 @@ export class GridOdataService implements BackendService { } } else if (sortColumns && !presetSorters) { // build the SortBy string, it could be multisort, example: customerNo asc, purchaserName desc - if (sortColumns && sortColumns.length === 0) { + if (sortColumns?.length === 0) { // TODO fix this line // currentSorters = new Array(this.defaultOptions.orderBy); // when empty, use the default sort } else { @@ -537,77 +538,77 @@ export class GridOdataService implements BackendService { }); } - private odataQueryVersionWrapper(queryType: 'dateTime' | 'substring', version: number, fieldName: string, searchValue = ''): string { - let query = ''; - switch (queryType) { - case 'dateTime': - query = version >= 4 ? searchValue : `DateTime'${searchValue}'`; - break; - case 'substring': - query = version >= 4 ? `contains(${fieldName}, '${searchValue}')` : `substringof('${searchValue}', ${fieldName})`; - break; - } - return query; - } - - /** - * Filter by a search date, the searchTerms might be a single value or range of dates (2 searchTerms OR 1 string separated by 2 dots "date1..date2") - * Also depending on the OData version number, the output will be different, previous version must wrap dates with DateTime - * - version 2-3:: Finish gt DateTime'2019-08-12T00:00:00Z' - * - version 4:: Finish gt 2019-08-12T00:00:00Z - */ - private filterBySearchDate(fieldName: string, operator: OperatorType | OperatorString, searchTerms: SearchTerm[], version: number): string { - let query = ''; - let searchValues: SearchTerm[] = []; - if (Array.isArray(searchTerms) && searchTerms.length > 1) { - searchValues = searchTerms; - if (operator !== OperatorType.rangeExclusive && operator !== OperatorType.rangeInclusive && this._gridOptions.defaultFilterRangeOperator) { - operator = this._gridOptions.defaultFilterRangeOperator; - } - } - - // single search value - if (searchValues.length === 0 && Array.isArray(searchTerms) && searchTerms.length === 1 && searchTerms[0]) { - const searchValue1 = this.odataQueryVersionWrapper('dateTime', version, fieldName, parseUtcDate(searchTerms[0] as string, true)); - if (searchValue1) { - return `${fieldName} ${this.mapOdataOperator(operator)} ${searchValue1}`; - } - } - - // multiple search value (date range) - if (Array.isArray(searchValues) && searchValues.length === 2 && searchValues[0] && searchValues[1]) { - // date field needs to be UTC and within DateTime function - const searchValue1 = this.odataQueryVersionWrapper('dateTime', version, fieldName, parseUtcDate(searchValues[0] as string, true)); - const searchValue2 = this.odataQueryVersionWrapper('dateTime', version, fieldName, parseUtcDate(searchValues[1] as string, true)); - - if (searchValue1 && searchValue2) { - if (operator === OperatorType.rangeInclusive) { - // example:: (Finish >= DateTime'2019-08-11T00:00:00Z' and Finish <= DateTime'2019-09-12T00:00:00Z') - query = `(${fieldName} ge ${searchValue1} and ${fieldName} le ${searchValue2})`; - } else if (operator === OperatorType.rangeExclusive) { - // example:: (Finish > DateTime'2019-08-11T00:00:00Z' and Finish < DateTime'2019-09-12T00:00:00Z') - query = `(${fieldName} gt ${searchValue1} and ${fieldName} lt ${searchValue2})`; - } - } - } - return query; - } - /** * Filter by a range of searchTerms (2 searchTerms OR 1 string separated by 2 dots "value1..value2") */ private filterBySearchTermRange(fieldName: string, operator: OperatorType | OperatorString, searchTerms: SearchTerm[]) { let query = ''; - if (Array.isArray(searchTerms) && searchTerms.length === 2) { if (operator === OperatorType.rangeInclusive) { // example:: (Duration >= 5 and Duration <= 10) - query = `(${fieldName} ge ${searchTerms[0]} and ${fieldName} le ${searchTerms[1]})`; + query = `(${fieldName} ge ${searchTerms[0]}`; + if (searchTerms[1] !== '') { + query += ` and ${fieldName} le ${searchTerms[1]}`; + } + query += ')'; } else if (operator === OperatorType.rangeExclusive) { // example:: (Duration > 5 and Duration < 10) - query = `(${fieldName} gt ${searchTerms[0]} and ${fieldName} lt ${searchTerms[1]})`; + query = `(${fieldName} gt ${searchTerms[0]}`; + if (searchTerms[1] !== '') { + query += ` and ${fieldName} lt ${searchTerms[1]}`; + } + query += ')'; } } return query; } + + /** + * Normalizes the search value according to field type and oData version. + */ + private normalizeSearchValue(fieldType: typeof FieldType[keyof typeof FieldType], searchValue: any, version: number) { + switch (fieldType) { + case FieldType.date: + searchValue = parseUtcDate(searchValue as string, true); + searchValue = version >= 4 ? searchValue : `DateTime'${searchValue}'`; + break; + case FieldType.string: + case FieldType.text: + case FieldType.readonly: + if (typeof searchValue === 'string') { + // escape single quotes by doubling them + searchValue = searchValue.replace(/'/g, `''`); + // encode URI of the final search value + searchValue = encodeURIComponent(searchValue); + // strings need to be quoted. + searchValue = `'${searchValue}'`; + } + break; + case FieldType.integer: + case FieldType.number: + case FieldType.float: + if (typeof searchValue === 'string') { + // Parse a valid decimal from the string. + + // Replace double dots with single dots + searchValue = searchValue.replace(/\.\./g, '.'); + // Remove a trailing dot + searchValue = searchValue.replace(/\.+$/g, ''); + // Prefix a leading dot with 0 + searchValue = searchValue.replace(/^\.+/g, '0.'); + // Prefix leading dash dot with -0. + searchValue = searchValue.replace(/^\-+\.+/g, '-0.'); + // Remove any non valid decimal characters from the search string + searchValue = searchValue.replace(/(?!^\-)[^\d\.]/g, ''); + + // if nothing left, search for 0 + if (searchValue === '' || searchValue === '-') { + searchValue = '0'; + } + } + break; + } + + return searchValue; + } }