Skip to content

Commit

Permalink
[Lens] Fix sorting undefined, null and NaN values (#92575)
Browse files Browse the repository at this point in the history
  • Loading branch information
@flash1293
flash1293 authored Feb 24, 2021
1 parent 06418ae commit a3f7b1d
Show file tree
Hide file tree
Showing 2 changed files with 181 additions and 10 deletions.
146 changes: 145 additions & 1 deletion x-pack/plugins/lens/public/datatable_visualization/sorting.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,18 +19,20 @@ function testSorting({
direction,
type,
keepLast,
reverseOutput = true,
}: {
input: unknown[];
output: unknown[];
direction: 'asc' | 'desc';
type: DatatableColumnType | 'range';
keepLast?: boolean; // special flag to handle values that should always be last no matter the direction
reverseOutput?: boolean;
}) {
const datatable = input.map((v) => ({
a: v,
}));
const sorted = output.map((v) => ({ a: v }));
if (direction === 'desc') {
if (direction === 'desc' && reverseOutput) {
sorted.reverse();
if (keepLast) {
// Cycle shift of the first element
Expand Down Expand Up @@ -64,6 +66,44 @@ describe('Data sorting criteria', () => {
});
});
}

it(`should sort undefined and null to the end`, () => {
const now = Date.now();
testSorting({
input: [null, now, 0, undefined, null, now - 150000],
output: [0, now - 150000, now, null, undefined, null],
direction: 'asc',
type: 'date',
reverseOutput: false,
});

testSorting({
input: [null, now, 0, undefined, null, now - 150000],
output: [now, now - 150000, 0, null, undefined, null],
direction: 'desc',
type: 'date',
reverseOutput: false,
});
});

it(`should sort NaN to the end`, () => {
const now = Date.now();
testSorting({
input: [null, now, 0, undefined, Number.NaN, now - 150000],
output: [0, now - 150000, now, null, undefined, Number.NaN],
direction: 'asc',
type: 'number',
reverseOutput: false,
});

testSorting({
input: [null, now, 0, undefined, Number.NaN, now - 150000],
output: [now, now - 150000, 0, null, undefined, Number.NaN],
direction: 'desc',
type: 'number',
reverseOutput: false,
});
});
});

describe('String or anything else as string', () => {
Expand All @@ -86,6 +126,39 @@ describe('Data sorting criteria', () => {
});
});
}

it('should sort undefined and null to the end', () => {
testSorting({
input: ['a', null, 'b', 'c', undefined, 'd', '12'],
output: ['12', 'a', 'b', 'c', 'd', null, undefined],
direction: 'asc',
type: 'string',
reverseOutput: false,
});

testSorting({
input: ['a', null, 'b', 'c', undefined, 'd', '12'],
output: ['d', 'c', 'b', 'a', '12', null, undefined],
direction: 'desc',
type: 'string',
reverseOutput: false,
});

testSorting({
input: [true, null, false, undefined, false],
output: [false, false, true, null, undefined],
direction: 'asc',
type: 'boolean',
reverseOutput: false,
});
testSorting({
input: [true, null, false, undefined, false],
output: [true, false, false, null, undefined],
direction: 'desc',
type: 'boolean',
reverseOutput: false,
});
});
});

describe('IP sorting', () => {
Expand Down Expand Up @@ -154,6 +227,60 @@ describe('Data sorting criteria', () => {
});
});
}

it('should sort undefined and null to the end', () => {
testSorting({
input: [
'fc00::123',
'192.168.1.50',
null,
undefined,
'Other',
'10.0.1.76',
'8.8.8.8',
'::1',
],
output: [
'::1',
'8.8.8.8',
'10.0.1.76',
'192.168.1.50',
'fc00::123',
'Other',
null,
undefined,
],
direction: 'asc',
type: 'ip',
reverseOutput: false,
});

testSorting({
input: [
'fc00::123',
'192.168.1.50',
null,
undefined,
'Other',
'10.0.1.76',
'8.8.8.8',
'::1',
],
output: [
'fc00::123',
'192.168.1.50',
'10.0.1.76',
'8.8.8.8',
'::1',
'Other',
null,
undefined,
],
direction: 'desc',
type: 'ip',
reverseOutput: false,
});
});
});

describe('Range sorting', () => {
Expand Down Expand Up @@ -184,5 +311,22 @@ describe('Data sorting criteria', () => {
});
});
}

it('should sort undefined and null to the end', () => {
testSorting({
input: [{ gte: 1, lt: 5 }, undefined, { gte: 0, lt: 5 }, null, { gte: 0 }],
output: [{ gte: 0, lt: 5 }, { gte: 0 }, { gte: 1, lt: 5 }, undefined, null],
direction: 'asc',
type: 'range',
reverseOutput: false,
});
testSorting({
input: [{ gte: 1, lt: 5 }, undefined, { gte: 0, lt: 5 }, null, { gte: 0 }],
output: [{ gte: 1, lt: 5 }, { gte: 0 }, { gte: 0, lt: 5 }, undefined, null],
direction: 'desc',
type: 'range',
reverseOutput: false,
});
});
});
});
45 changes: 36 additions & 9 deletions x-pack/plugins/lens/public/datatable_visualization/sorting.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,8 @@ function getRangeCriteria(sortBy: string, directionFactor: number) {
};
}

type CompareFn = (rowA: Record<string, unknown>, rowB: Record<string, unknown>) => number;

export function getSortingCriteria(
type: string | undefined,
sortBy: string,
Expand All @@ -71,22 +73,47 @@ export function getSortingCriteria(
// handle the direction with a multiply factor.
const directionFactor = direction === 'asc' ? 1 : -1;

let criteria: CompareFn;

if (['number', 'date'].includes(type || '')) {
return (rowA: Record<string, unknown>, rowB: Record<string, unknown>) =>
criteria = (rowA: Record<string, unknown>, rowB: Record<string, unknown>) =>
directionFactor * ((rowA[sortBy] as number) - (rowB[sortBy] as number));
}
// this is a custom type, and can safely assume the gte and lt fields are all numbers or undefined
if (type === 'range') {
return getRangeCriteria(sortBy, directionFactor);
else if (type === 'range') {
criteria = getRangeCriteria(sortBy, directionFactor);
}
// IP have a special sorting
if (type === 'ip') {
return getIPCriteria(sortBy, directionFactor);
else if (type === 'ip') {
criteria = getIPCriteria(sortBy, directionFactor);
} else {
// use a string sorter for the rest
criteria = (rowA: Record<string, unknown>, rowB: Record<string, unknown>) => {
const aString = formatter.convert(rowA[sortBy]);
const bString = formatter.convert(rowB[sortBy]);
return directionFactor * aString.localeCompare(bString);
};
}
// use a string sorter for the rest
return getUndefinedHandler(sortBy, criteria);
}

function getUndefinedHandler(
sortBy: string,
sortingCriteria: (rowA: Record<string, unknown>, rowB: Record<string, unknown>) => number
) {
return (rowA: Record<string, unknown>, rowB: Record<string, unknown>) => {
const aString = formatter.convert(rowA[sortBy]);
const bString = formatter.convert(rowB[sortBy]);
return directionFactor * aString.localeCompare(bString);
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;
};
}

0 comments on commit a3f7b1d

Please sign in to comment.