From e4e22d6768727865b699d9ae69360edb0097c6c4 Mon Sep 17 00:00:00 2001 From: CJ Cenizal Date: Wed, 16 Dec 2020 13:50:09 -0800 Subject: [PATCH] Fix bug in which Index Management data streams were being sorted incorrectly by storage size. They were previously being sorted alphabetically using the humanized size value. Now they're sorted numerically by the raw bytes value. --- data/.empty | 0 .../home/data_streams_tab.helpers.ts | 11 ++++++ .../home/data_streams_tab.test.ts | 38 +++++++++++++++++-- .../common/lib/data_stream_serialization.ts | 2 + .../common/types/data_streams.ts | 2 + .../data_stream_table/data_stream_table.tsx | 4 +- .../api/data_streams/register_get_route.ts | 4 +- .../index_management/data_streams.ts | 15 ++++---- 8 files changed, 63 insertions(+), 13 deletions(-) delete mode 100644 data/.empty diff --git a/data/.empty b/data/.empty deleted file mode 100644 index e69de29bb2d1d..0000000000000 diff --git a/x-pack/plugins/index_management/__jest__/client_integration/home/data_streams_tab.helpers.ts b/x-pack/plugins/index_management/__jest__/client_integration/home/data_streams_tab.helpers.ts index 0fa1ddeee820d..d7cf391376076 100644 --- a/x-pack/plugins/index_management/__jest__/client_integration/home/data_streams_tab.helpers.ts +++ b/x-pack/plugins/index_management/__jest__/client_integration/home/data_streams_tab.helpers.ts @@ -20,6 +20,7 @@ export interface DataStreamsTabTestBed extends TestBed { clickEmptyPromptIndexTemplateLink: () => void; clickIncludeStatsSwitch: () => void; toggleViewFilterAt: (index: number) => void; + sortTableOnStorageSize: () => void; clickReloadButton: () => void; clickNameAt: (index: number) => void; clickIndicesAt: (index: number) => void; @@ -94,6 +95,14 @@ export const setup = async (overridingDependencies: any = {}): Promise { + const { find, component } = testBed; + act(() => { + find('tableHeaderCell_storageSizeBytes_3.tableHeaderSortButton').simulate('click'); + }); + component.update(); + }; + const clickReloadButton = () => { const { find } = testBed; find('reloadButton').simulate('click'); @@ -205,6 +214,7 @@ export const setup = async (overridingDependencies: any = {}): Promise): DataSt health: 'green', indexTemplateName: 'indexTemplate', storageSize: '1b', + storageSizeBytes: 1, maxTimeStamp: 420, privileges: { delete_index: true, diff --git a/x-pack/plugins/index_management/__jest__/client_integration/home/data_streams_tab.test.ts b/x-pack/plugins/index_management/__jest__/client_integration/home/data_streams_tab.test.ts index 93899dece3308..b71e058e711cd 100644 --- a/x-pack/plugins/index_management/__jest__/client_integration/home/data_streams_tab.test.ts +++ b/x-pack/plugins/index_management/__jest__/client_integration/home/data_streams_tab.test.ts @@ -120,11 +120,21 @@ describe('Data Streams tab', () => { createNonDataStreamIndex('non-data-stream-index'), ]); - const dataStreamForDetailPanel = createDataStreamPayload({ name: 'dataStream1' }); + const dataStreamForDetailPanel = createDataStreamPayload({ + name: 'dataStream1', + storageSize: '5b', + storageSizeBytes: 5, + }); + setLoadDataStreamsResponse([ dataStreamForDetailPanel, - createDataStreamPayload({ name: 'dataStream2' }), + createDataStreamPayload({ + name: 'dataStream2', + storageSize: '1kb', + storageSizeBytes: 1000, + }), ]); + setLoadDataStreamResponse(dataStreamForDetailPanel); const indexTemplate = fixtures.getTemplate({ name: 'indexTemplate' }); @@ -181,8 +191,28 @@ describe('Data Streams tab', () => { // The table renders with the stats columns though. const { tableCellsValues } = table.getMetaData('dataStreamTable'); expect(tableCellsValues).toEqual([ - ['', 'dataStream1', 'green', 'December 31st, 1969 7:00:00 PM', '1b', '1', 'Delete'], - ['', 'dataStream2', 'green', 'December 31st, 1969 7:00:00 PM', '1b', '1', 'Delete'], + ['', 'dataStream1', 'green', 'December 31st, 1969 7:00:00 PM', '5b', '1', 'Delete'], + ['', 'dataStream2', 'green', 'December 31st, 1969 7:00:00 PM', '1kb', '1', 'Delete'], + ]); + }); + + test('sorting on stats sorts by bytes value instead of human readable value', async () => { + // Guards against regression of #86122. + const { actions, table, component } = testBed; + + await act(async () => { + actions.clickIncludeStatsSwitch(); + }); + component.update(); + + actions.sortTableOnStorageSize(); + + // The table sorts by the underlying byte values in ascending order, instead of sorting by + // the human-readable string values. + const { tableCellsValues } = table.getMetaData('dataStreamTable'); + expect(tableCellsValues).toEqual([ + ['', 'dataStream1', 'green', 'December 31st, 1969 7:00:00 PM', '5b', '1', 'Delete'], + ['', 'dataStream2', 'green', 'December 31st, 1969 7:00:00 PM', '1kb', '1', 'Delete'], ]); }); diff --git a/x-pack/plugins/index_management/common/lib/data_stream_serialization.ts b/x-pack/plugins/index_management/common/lib/data_stream_serialization.ts index 333cb4b97f2aa..c1a3a728d4238 100644 --- a/x-pack/plugins/index_management/common/lib/data_stream_serialization.ts +++ b/x-pack/plugins/index_management/common/lib/data_stream_serialization.ts @@ -16,6 +16,7 @@ export function deserializeDataStream(dataStreamFromEs: DataStreamFromEs): DataS template, ilm_policy: ilmPolicyName, store_size: storageSize, + store_size_bytes: storageSizeBytes, maximum_timestamp: maxTimeStamp, _meta, privileges, @@ -37,6 +38,7 @@ export function deserializeDataStream(dataStreamFromEs: DataStreamFromEs): DataS indexTemplateName: template, ilmPolicyName, storageSize, + storageSizeBytes, maxTimeStamp, _meta, privileges, diff --git a/x-pack/plugins/index_management/common/types/data_streams.ts b/x-pack/plugins/index_management/common/types/data_streams.ts index fca10f85ab63c..57abc0e188e16 100644 --- a/x-pack/plugins/index_management/common/types/data_streams.ts +++ b/x-pack/plugins/index_management/common/types/data_streams.ts @@ -36,6 +36,7 @@ export interface DataStreamFromEs { template: string; ilm_policy?: string; store_size?: string; + store_size_bytes?: number; maximum_timestamp?: number; privileges: PrivilegesFromEs; hidden: boolean; @@ -57,6 +58,7 @@ export interface DataStream { indexTemplateName: string; ilmPolicyName?: string; storageSize?: string; + storageSizeBytes?: number; maxTimeStamp?: number; _meta?: Meta; privileges: Privileges; diff --git a/x-pack/plugins/index_management/public/application/sections/home/data_stream_list/data_stream_table/data_stream_table.tsx b/x-pack/plugins/index_management/public/application/sections/home/data_stream_list/data_stream_table/data_stream_table.tsx index f74147f996701..dfad10ab62449 100644 --- a/x-pack/plugins/index_management/public/application/sections/home/data_stream_list/data_stream_table/data_stream_table.tsx +++ b/x-pack/plugins/index_management/public/application/sections/home/data_stream_list/data_stream_table/data_stream_table.tsx @@ -90,12 +90,14 @@ export const DataStreamTable: React.FunctionComponent = ({ }); columns.push({ - field: 'storageSize', + field: 'storageSizeBytes', name: i18n.translate('xpack.idxMgmt.dataStreamList.table.storageSizeColumnTitle', { defaultMessage: 'Storage size', }), truncateText: true, sortable: true, + render: (storageSizeBytes: DataStream['storageSizeBytes'], dataStream: DataStream) => + dataStream.storageSize, }); } diff --git a/x-pack/plugins/index_management/server/routes/api/data_streams/register_get_route.ts b/x-pack/plugins/index_management/server/routes/api/data_streams/register_get_route.ts index 4124d8e897b5b..4256107fc1818 100644 --- a/x-pack/plugins/index_management/server/routes/api/data_streams/register_get_route.ts +++ b/x-pack/plugins/index_management/server/routes/api/data_streams/register_get_route.ts @@ -23,6 +23,7 @@ interface PrivilegesFromEs { interface StatsFromEs { data_stream: string; store_size: string; + store_size_bytes: number; maximum_timestamp: number; } @@ -40,7 +41,7 @@ const enhanceDataStreams = ({ if (dataStreamsStats) { // eslint-disable-next-line @typescript-eslint/naming-convention - const { store_size, maximum_timestamp } = + const { store_size, store_size_bytes, maximum_timestamp } = dataStreamsStats.find( ({ data_stream: statsName }: { data_stream: string }) => statsName === dataStream.name ) || {}; @@ -48,6 +49,7 @@ const enhanceDataStreams = ({ enhancedDataStream = { ...enhancedDataStream, store_size, + store_size_bytes, maximum_timestamp, }; } diff --git a/x-pack/test/api_integration/apis/management/index_management/data_streams.ts b/x-pack/test/api_integration/apis/management/index_management/data_streams.ts index a8999fd065e75..e520a7c8f8e04 100644 --- a/x-pack/test/api_integration/apis/management/index_management/data_streams.ts +++ b/x-pack/test/api_integration/apis/management/index_management/data_streams.ts @@ -52,12 +52,14 @@ export default function ({ getService }: FtrProviderContext) { await deleteComposableIndexTemplate(name); }; - const assertDataStreamStorageSizeExists = (storageSize: string) => { - // Storage size of a document doesn't like it would be deterministic (could vary depending + const assertDataStreamStorageSizeExists = (storageSize: string, storageSizeBytes: number) => { + // Storage size of a document doesn't look like it would be deterministic (could vary depending // on how ES, Lucene, and the file system interact), so we'll just assert its presence and // type. expect(storageSize).to.be.ok(); expect(typeof storageSize).to.be('string'); + expect(storageSizeBytes).to.be.ok(); + expect(typeof storageSizeBytes).to.be('number'); }; describe('Data streams', function () { @@ -120,10 +122,9 @@ export default function ({ getService }: FtrProviderContext) { expect(testDataStream).to.be.ok(); // ES determines these values so we'll just echo them back. - const { name: indexName, uuid } = testDataStream!.indices[0]; - const { storageSize, ...dataStreamWithoutStorageSize } = testDataStream!; - assertDataStreamStorageSizeExists(storageSize); + const { storageSize, storageSizeBytes, ...dataStreamWithoutStorageSize } = testDataStream!; + assertDataStreamStorageSizeExists(storageSize, storageSizeBytes); expect(dataStreamWithoutStorageSize).to.eql({ name: testDataStreamName, @@ -153,8 +154,8 @@ export default function ({ getService }: FtrProviderContext) { // ES determines these values so we'll just echo them back. const { name: indexName, uuid } = dataStream.indices[0]; - const { storageSize, ...dataStreamWithoutStorageSize } = dataStream; - assertDataStreamStorageSizeExists(storageSize); + const { storageSize, storageSizeBytes, ...dataStreamWithoutStorageSize } = dataStream; + assertDataStreamStorageSizeExists(storageSize, storageSizeBytes); expect(dataStreamWithoutStorageSize).to.eql({ name: testDataStreamName,