diff --git a/x-pack/plugins/lens/common/expressions/datatable/sorting.test.tsx b/x-pack/plugins/lens/common/expressions/datatable/sorting.test.tsx index 10b37acd94117..042a3e22f7bb8 100644 --- a/x-pack/plugins/lens/common/expressions/datatable/sorting.test.tsx +++ b/x-pack/plugins/lens/common/expressions/datatable/sorting.test.tsx @@ -31,7 +31,7 @@ function testSorting({ const datatable = input.map((v) => ({ a: v, })); - const sorted = output.map((v) => ({ a: v })); + const sorted = [...output]; if (direction === 'desc' && reverseOutput) { sorted.reverse(); if (keepLast) { @@ -41,7 +41,7 @@ function testSorting({ } } const criteria = getSortingCriteria(type, 'a', getMockFormatter(), direction); - expect(datatable.sort(criteria)).toEqual(sorted); + expect(datatable.sort(criteria).map((row) => row.a)).toEqual(sorted); } describe('Data sorting criteria', () => { @@ -65,6 +65,25 @@ describe('Data sorting criteria', () => { type: 'date', }); }); + + it(`should provide the number criteria of array numeric values (${direction})`, () => { + testSorting({ + input: [7, [7, 1], [7, 2], [6], -Infinity, Infinity], + output: [-Infinity, [6], 7, [7, 1], [7, 2], Infinity], + direction, + type: 'number', + }); + }); + + it(`should provide the number criteria for array date values (${direction})`, () => { + const now = Date.now(); + testSorting({ + input: [now, [0, now], [0], now - 150000], + output: [[0], [0, now], now - 150000, now], + direction, + type: 'date', + }); + }); } it(`should sort undefined and null to the end`, () => { @@ -116,6 +135,15 @@ describe('Data sorting criteria', () => { type: 'version', }); }); + + it(`should provide the array version criteria for terms values (${direction})`, () => { + testSorting({ + input: [['1.21.0'], ['1.1.0', '1.1.1'], ['1.21.1', '1.1.1'], '1.112.0', '1.0.0'], + output: ['1.0.0', ['1.1.0', '1.1.1'], ['1.21.0'], ['1.21.1', '1.1.1'], '1.112.0'], + direction, + type: 'version', + }); + }); } it('should sort non-version stuff to the end', () => { @@ -148,6 +176,15 @@ describe('Data sorting criteria', () => { }); }); + it(`should provide the string criteria for array terms values (${direction})`, () => { + testSorting({ + input: ['a', 'b', 'c', 'd', '12', ['a', 'b'], ['b', 'c']], + output: ['12', 'a', ['a', 'b'], 'b', ['b', 'c'], 'c', 'd'], + direction, + type: 'string', + }); + }); + it(`should provide the string criteria for other types of values (${direction})`, () => { testSorting({ input: [true, false, false], @@ -156,6 +193,15 @@ describe('Data sorting criteria', () => { type: 'boolean', }); }); + + it(`should provide the string criteria for other types of array values (${direction})`, () => { + testSorting({ + input: [true, false, false, [true, false]], + output: [false, false, true, [true, false]], + direction, + type: 'boolean', + }); + }); } it('should sort undefined and null to the end', () => { @@ -257,6 +303,34 @@ describe('Data sorting criteria', () => { keepLast: true, }); }); + + it(`should provide the IP criteria for array IP values (mixed values with invalid "Other" field) - ${direction}`, () => { + testSorting({ + input: [ + 'fc00::123', + '192.168.1.50', + 'Other', + '10.0.1.76', + '8.8.8.8', + '::1', + ['fc00::123', '192.168.1.50'], + ['::1', '10.0.1.76'], + ], + output: [ + '::1', + ['::1', '10.0.1.76'], + '8.8.8.8', + '10.0.1.76', + '192.168.1.50', + 'fc00::123', + ['fc00::123', '192.168.1.50'], + 'Other', + ], + direction, + type: 'ip', + keepLast: true, + }); + }); } it('should sort undefined and null to the end', () => { @@ -341,6 +415,34 @@ describe('Data sorting criteria', () => { type: 'range', }); }); + + it(`should sort array values for both open and closed ranges - ${direction}`, () => { + testSorting({ + input: [ + { gte: 1, lt: 5 }, + , + [ + { gte: 0, lt: 5 }, + { gte: 1, lt: 5 }, + ], + [{ gte: 0 }, { gte: 1, lt: 5 }], + { gte: 0, lt: 5 }, + { gte: 0 }, + ], + output: [ + { gte: 0, lt: 5 }, + [ + { gte: 0, lt: 5 }, + { gte: 1, lt: 5 }, + ], + { gte: 0 }, + [{ gte: 0 }, { gte: 1, lt: 5 }], + { gte: 1, lt: 5 }, + ], + direction, + type: 'range', + }); + }); } it('should sort undefined and null to the end', () => { diff --git a/x-pack/plugins/lens/common/expressions/datatable/sorting.tsx b/x-pack/plugins/lens/common/expressions/datatable/sorting.tsx index e3db99d69dc78..643e119e6ef7f 100644 --- a/x-pack/plugins/lens/common/expressions/datatable/sorting.tsx +++ b/x-pack/plugins/lens/common/expressions/datatable/sorting.tsx @@ -8,82 +8,174 @@ import versionCompare from 'compare-versions'; import valid from 'semver/functions/valid'; import ipaddr, { type IPv4, type IPv6 } from 'ipaddr.js'; -import type { FieldFormat } from '@kbn/field-formats-plugin/common'; +import { FieldFormat } from '@kbn/field-formats-plugin/common'; + +type CompareFn = ( + v1: T | undefined, + v2: T | undefined, + direction: number, + formatter: FieldFormat +) => number; + +const numberCompare: CompareFn = (v1, v2) => (v1 ?? -Infinity) - (v2 ?? -Infinity); + +const stringComparison: CompareFn = (v1, v2, _, formatter) => { + const aString = formatter.convert(v1); + const bString = formatter.convert(v2); + if (v1 == null) { + return -1; + } + if (v2 == null) { + return 1; + } + return aString.localeCompare(bString); +}; + +// The maximum length of a IP is 39 chars for a IPv6 +// therefore set 40 for null values to always be longer +const MAX_IP_LENGTH = 40; + +const ipComparison: CompareFn = (v1, v2, direction) => { + const ipA = getSafeIpAddress(v1, direction); + const ipB = getSafeIpAddress(v2, direction); + + // Now compare each part of the IPv6 address and exit when a value != 0 is found + let i = 0; + let diff = ipA.parts[i] - ipB.parts[i]; + while (!diff && i < 7) { + i++; + diff = ipA.parts[i] - ipB.parts[i]; + } + + // in case of same address but written in different styles, sort by string length + if (diff === 0) { + const v1Length = v1 ? v1.length : MAX_IP_LENGTH; + const v2Length = v2 ? v2.length : MAX_IP_LENGTH; + return v1Length - v2Length; + } + return diff; +}; + +/** + * This is a comparison utility for array + * It performs a comparison for each pair of value and exists on the first comparison value != 0 + * @param array1 T[] + * @param array2 T[] + * @param directionFactor +1 / -1 + * @param compareFn + * @returns + */ +function compareArrays( + array1: T[], + array2: T[], + directionFactor: number, + formatter: FieldFormat, + compareFn: CompareFn +): number { + // sort by each pair of values + const maxLength = Math.max(array1.length, array2.length); + for (let i = 0; i < maxLength; i++) { + const comparisonValue = compareFn(array1[i], array2[i], directionFactor, formatter); + if (comparisonValue !== 0) { + return comparisonValue; + } + } + return 0; +} function isIPv6Address(ip: IPv4 | IPv6): ip is IPv6 { return ip.kind() === 'ipv6'; } -function getSafeIpAddress(ip: string, directionFactor: number) { - if (!ipaddr.isValid(ip)) { +function getSafeIpAddress(ip: string | undefined, directionFactor: number) { + if (ip == null || !ipaddr.isValid(ip)) { + // if ip is null, then it's a part of an array ip value + // therefore the comparison might be between a single value [ipA, undefined] vs multiple values ip [ipA, ipB] + // set in this case -1 for the undefined of the former to force it to be always before + const forceSortingFactor = ip == null ? -1 : directionFactor; // for non valid IPs have the same behaviour as for now (we assume it's only the "Other" string) // create a mock object which has all a special value to keep them always at the bottom of the list - return { parts: Array(8).fill(directionFactor * Infinity) }; + return { parts: Array(8).fill(forceSortingFactor * Infinity) }; } const parsedIp = ipaddr.parse(ip); return isIPv6Address(parsedIp) ? parsedIp : parsedIp.toIPv4MappedAddress(); } -function getIPCriteria(sortBy: string, directionFactor: number) { - // Create a set of 8 function to sort based on the 8 IPv6 slots of an address - // For IPv4 bring them to the IPv6 "mapped" format and then sort - return (rowA: Record, rowB: Record) => { - const ipAString = rowA[sortBy] as string; - const ipBString = rowB[sortBy] as string; - const ipA = getSafeIpAddress(ipAString, directionFactor); - const ipB = getSafeIpAddress(ipBString, directionFactor); - - // Now compare each part of the IPv6 address and exit when a value != 0 is found - let i = 0; - let diff = ipA.parts[i] - ipB.parts[i]; - while (!diff && i < 7) { - i++; - diff = ipA.parts[i] - ipB.parts[i]; - } - - // in case of same address but written in different styles, sort by string length - if (diff === 0) { - return directionFactor * (ipAString.length - ipBString.length); - } - return directionFactor * diff; +const versionComparison: CompareFn = (v1, v2, direction) => { + const valueA = String(v1 ?? ''); + const valueB = String(v2 ?? ''); + const aInvalid = !valueA || !valid(valueA); + const bInvalid = !valueB || !valid(valueB); + if (aInvalid && bInvalid) { + return 0; + } + // need to fight the direction multiplication of the parent function + if (aInvalid) { + return direction * 1; + } + if (bInvalid) { + return direction * -1; + } + return versionCompare(valueA, valueB); +}; + +const openRange = { gte: -Infinity, lt: Infinity }; +const rangeComparison: CompareFn> = (v1, v2) => { + const rangeA = { ...openRange, ...v1 }; + const rangeB = { ...openRange, ...v2 }; + + const fromComparison = rangeA.gte - rangeB.gte; + const toComparison = rangeA.lt - rangeB.lt; + + return fromComparison || toComparison || 0; +}; + +function createArrayValuesHandler(sortBy: string, directionFactor: number, formatter: FieldFormat) { + return function (criteriaFn: CompareFn) { + return (rowA: Record, rowB: Record) => { + // if either side of the comparison is an array, make it also the other one become one + // then perform an array comparison + if (Array.isArray(rowA[sortBy]) || Array.isArray(rowB[sortBy])) { + return ( + directionFactor * + compareArrays( + (Array.isArray(rowA[sortBy]) ? rowA[sortBy] : [rowA[sortBy]]) as T[], + (Array.isArray(rowB[sortBy]) ? rowB[sortBy] : [rowB[sortBy]]) as T[], + directionFactor, + formatter, + criteriaFn + ) + ); + } + return ( + directionFactor * + criteriaFn(rowA[sortBy] as T, rowB[sortBy] as T, directionFactor, formatter) + ); + }; }; } -function getVersionCriteria(sortBy: string, directionFactor: number) { +function getUndefinedHandler( + sortBy: string, + sortingCriteria: (rowA: Record, rowB: Record) => number +) { return (rowA: Record, rowB: Record) => { - const valueA = String(rowA[sortBy] ?? ''); - const valueB = String(rowB[sortBy] ?? ''); - const aInvalid = !valueA || !valid(valueA); - const bInvalid = !valueB || !valid(valueB); - if (aInvalid && bInvalid) { - return 0; + const valueA = rowA[sortBy]; + const valueB = rowB[sortBy]; + if (valueA != null && valueB != null && !Number.isNaN(valueA) && !Number.isNaN(valueB)) { + return sortingCriteria(rowA, rowB); } - if (aInvalid) { + if (valueA == null || Number.isNaN(valueA)) { return 1; } - if (bInvalid) { + if (valueB == null || Number.isNaN(valueB)) { return -1; } - return directionFactor * versionCompare(valueA, valueB); - }; -} - -function getRangeCriteria(sortBy: string, directionFactor: number) { - // fill missing fields with these open bounds to perform number sorting - const openRange = { gte: -Infinity, lt: Infinity }; - return (rowA: Record, rowB: Record) => { - const rangeA = { ...openRange, ...(rowA[sortBy] as Omit) }; - const rangeB = { ...openRange, ...(rowB[sortBy] as Omit) }; - - const fromComparison = rangeA.gte - rangeB.gte; - const toComparison = rangeA.lt - rangeB.lt; - return directionFactor * (fromComparison || toComparison); + return 0; }; } -type CompareFn = (rowA: Record, rowB: Record) => number; - export function getSortingCriteria( type: string | undefined, sortBy: string, @@ -93,50 +185,23 @@ export function getSortingCriteria( // handle the direction with a multiply factor. const directionFactor = direction === 'asc' ? 1 : -1; - let criteria: CompareFn; + const arrayValueHandler = createArrayValuesHandler(sortBy, directionFactor, formatter); if (['number', 'date'].includes(type || '')) { - criteria = (rowA: Record, rowB: Record) => - directionFactor * ((rowA[sortBy] as number) - (rowB[sortBy] as number)); + return getUndefinedHandler(sortBy, arrayValueHandler(numberCompare)); } // this is a custom type, and can safely assume the gte and lt fields are all numbers or undefined - else if (type === 'range') { - criteria = getRangeCriteria(sortBy, directionFactor); + if (type === 'range') { + return getUndefinedHandler(sortBy, arrayValueHandler(rangeComparison)); } // IP have a special sorting - else if (type === 'ip') { - criteria = getIPCriteria(sortBy, directionFactor); - } else if (type === 'version') { - // do not wrap in undefined behandler because of special invalid-case handling - return getVersionCriteria(sortBy, directionFactor); - } else { - // use a string sorter for the rest - criteria = (rowA: Record, rowB: Record) => { - const aString = formatter.convert(rowA[sortBy]); - const bString = formatter.convert(rowB[sortBy]); - return directionFactor * aString.localeCompare(bString); - }; + if (type === 'ip') { + return getUndefinedHandler(sortBy, arrayValueHandler(ipComparison)); } - return getUndefinedHandler(sortBy, criteria); -} - -function getUndefinedHandler( - sortBy: string, - sortingCriteria: (rowA: Record, rowB: Record) => number -) { - return (rowA: Record, rowB: Record) => { - const valueA = rowA[sortBy]; - const valueB = rowB[sortBy]; - if (valueA != null && valueB != null && !Number.isNaN(valueA) && !Number.isNaN(valueB)) { - return sortingCriteria(rowA, rowB); - } - if (valueA == null || Number.isNaN(valueA)) { - return 1; - } - if (valueB == null || Number.isNaN(valueB)) { - return -1; - } - - return 0; - }; + if (type === 'version') { + // do not wrap in undefined handler because of special invalid-case handling + return arrayValueHandler(versionComparison); + } + // use a string sorter for the rest + return getUndefinedHandler(sortBy, arrayValueHandler(stringComparison)); } diff --git a/x-pack/plugins/lens/common/types.ts b/x-pack/plugins/lens/common/types.ts index ee89551dfc36a..647bac719186f 100644 --- a/x-pack/plugins/lens/common/types.ts +++ b/x-pack/plugins/lens/common/types.ts @@ -35,7 +35,7 @@ export interface PersistableFilter extends Filter { meta: PersistableFilterMeta; } -export type SortingHint = 'version'; +export type SortingHint = string; export type LayerType = typeof layerTypes[keyof typeof layerTypes]; diff --git a/x-pack/plugins/lens/public/datasources/form_based/form_based.tsx b/x-pack/plugins/lens/public/datasources/form_based/form_based.tsx index 34ad0400e951d..f650fe36b4426 100644 --- a/x-pack/plugins/lens/public/datasources/form_based/form_based.tsx +++ b/x-pack/plugins/lens/public/datasources/form_based/form_based.tsx @@ -17,7 +17,7 @@ import { flatten, isEqual } from 'lodash'; import type { DataViewsPublicPluginStart, DataView } from '@kbn/data-views-plugin/public'; import type { IndexPatternFieldEditorStart } from '@kbn/data-view-field-editor-plugin/public'; import { KibanaContextProvider, KibanaThemeProvider } from '@kbn/kibana-react-plugin/public'; -import { DataPublicPluginStart, ES_FIELD_TYPES, UI_SETTINGS } from '@kbn/data-plugin/public'; +import { DataPublicPluginStart, UI_SETTINGS } from '@kbn/data-plugin/public'; import { VisualizeFieldContext } from '@kbn/ui-actions-plugin/public'; import { ChartsPluginSetup } from '@kbn/charts-plugin/public'; import { UiActionsStart } from '@kbn/ui-actions-plugin/public'; @@ -100,7 +100,7 @@ import { getPrecisionErrorWarningMessages } from './utils'; import { DOCUMENT_FIELD_NAME } from '../../../common/constants'; import { isColumnOfType } from './operations/definitions/helpers'; import { LayerSettingsPanel } from './layer_settings'; -import { FormBasedLayer } from '../..'; +import { FormBasedLayer, LastValueIndexPatternColumn } from '../..'; import { filterAndSortUserMessages } from '../../app_plugin/get_application_user_messages'; export type { OperationType, GenericIndexPatternColumn } from './operations'; export { deleteColumn } from './operations'; @@ -112,24 +112,31 @@ function wrapOnDot(str?: string) { return str ? str.replace(/\./g, '.\u200B') : ''; } +function getSortingHint(column: GenericIndexPatternColumn, dataView?: IndexPattern | DataView) { + if (column.dataType === 'string') { + const fieldTypes = + 'sourceField' in column ? dataView?.getFieldByName(column.sourceField)?.esTypes : undefined; + return fieldTypes?.[0] || undefined; + } + if (isColumnOfType('last_value', column)) { + return column.dataType; + } +} + export function columnToOperation( column: GenericIndexPatternColumn, uniqueLabel?: string, dataView?: IndexPattern | DataView ): OperationDescriptor { const { dataType, label, isBucketed, scale, operationType, timeShift, reducedTimeRange } = column; - const fieldTypes = - 'sourceField' in column ? dataView?.getFieldByName(column.sourceField)?.esTypes : undefined; + return { dataType: normalizeOperationDataType(dataType), isBucketed, scale, label: uniqueLabel || label, isStaticValue: operationType === 'static_value', - sortingHint: - column.dataType === 'string' && fieldTypes?.includes(ES_FIELD_TYPES.VERSION) - ? 'version' - : undefined, + sortingHint: getSortingHint(column, dataView), hasTimeShift: Boolean(timeShift), hasReducedTimeRange: Boolean(reducedTimeRange), interval: isColumnOfType('date_histogram', column) diff --git a/x-pack/test/functional/apps/lens/group2/table.ts b/x-pack/test/functional/apps/lens/group2/table.ts index 2f55ed6204412..a10fa546a3c20 100644 --- a/x-pack/test/functional/apps/lens/group2/table.ts +++ b/x-pack/test/functional/apps/lens/group2/table.ts @@ -42,6 +42,53 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { }); }); + it('should able to sort a last_value column correctly in a table', async () => { + // configure last_value with a keyword field + await PageObjects.lens.configureDimension({ + dimension: 'lnsDatatable_metrics > lns-empty-dimension', + operation: 'last_value', + field: 'geo.dest', + }); + + await PageObjects.lens.changeTableSortingBy(3, 'ascending'); + await PageObjects.lens.waitForVisualization(); + expect(await PageObjects.lens.getDatatableCellText(0, 3)).to.eql('CN'); + + await PageObjects.lens.changeTableSortingBy(3, 'descending'); + await PageObjects.lens.waitForVisualization(); + expect(await PageObjects.lens.getDatatableCellText(0, 3)).to.eql('PH'); + + // now configure a new one with an ip field + await PageObjects.lens.configureDimension({ + dimension: 'lnsDatatable_metrics > lns-empty-dimension', + operation: 'last_value', + field: 'ip', + }); + await PageObjects.lens.changeTableSortingBy(4, 'ascending'); + await PageObjects.lens.waitForVisualization(); + expect(await PageObjects.lens.getDatatableCellText(0, 4)).to.eql('78.83.247.30'); + // Change the sorting + await PageObjects.lens.changeTableSortingBy(4, 'descending'); + await PageObjects.lens.waitForVisualization(); + expect(await PageObjects.lens.getDatatableCellText(0, 0)).to.eql('169.228.188.120'); + + await retry.try(async () => { + await PageObjects.lens.changeTableSortingBy(4, 'none'); + await PageObjects.lens.waitForVisualization(); + expect(await PageObjects.lens.isDatatableHeaderSorted(0)).to.eql(false); + }); + + // clear all metrics and reconfigure the default + await PageObjects.lens.removeDimension('lnsDatatable_metrics'); + await PageObjects.lens.removeDimension('lnsDatatable_metrics'); + await PageObjects.lens.removeDimension('lnsDatatable_metrics'); + await PageObjects.lens.configureDimension({ + dimension: 'lnsDatatable_metrics > lns-empty-dimension', + operation: 'average', + field: 'bytes', + }); + }); + it('should able to use filters cell actions in table', async () => { const firstCellContent = await PageObjects.lens.getDatatableCellText(0, 0); await retry.try(async () => {