[7.x] Prevent rewrite of date_histogram interval (#91408)

src/plugins/data/common/search/aggs/buckets/lib/time_buckets/calc_es_interval.ts

@@ -28,12 +28,18 @@ export interface EsInterval {
  * @param  {moment.duration} duration
  * @return {object}
  */
-export function convertDurationToNormalizedEsInterval(duration: moment.Duration): EsInterval {
+export function convertDurationToNormalizedEsInterval(
+  duration: moment.Duration,
+  targetUnit?: Unit
+): EsInterval {
   for (let i = 0; i < unitsDesc.length; i++) {
     const unit = unitsDesc[i];
     const val = duration.as(unit);
     // find a unit that rounds neatly
     if (val >= 1 && Math.floor(val) === val) {
+      if (val === 1 && targetUnit && unit !== targetUnit) {
+        continue;
+      }
       // if the unit is "large", like years, but
       // isn't set to 1 ES will puke. So keep going until
       // we get out of the "large" units

src/plugins/data/common/search/aggs/buckets/lib/time_buckets/time_buckets.ts

@@ -5,11 +5,12 @@
  * in compliance with, at your election, the Elastic License 2.0 or the Server
  * Side Public License, v 1.
  */
-
+import { Assign } from 'utility-types';
 import { isString, isObject as isObjectLodash, isPlainObject, sortBy } from 'lodash';
 import moment, { Moment } from 'moment';
 
-import { parseInterval } from '../../../utils';
+import { Unit } from '@elastic/datemath';
+import { parseInterval, splitStringInterval } from '../../../utils';
 import { TimeRangeBounds } from '../../../../../query';
 import { calcAutoIntervalLessThan, calcAutoIntervalNear } from './calc_auto_interval';
 import {
@@ -38,6 +39,10 @@ function isValidMoment(m: any): boolean {
   return m && 'isValid' in m && m.isValid();
 }
 
+function isDurationInterval(i: any): i is moment.Duration {
+  return moment.isDuration(i) && Boolean(+i);
+}
+
 export interface TimeBucketsConfig extends Record<string, any> {
   'histogram:maxBars': number;
   'histogram:barTarget': number;
@@ -172,38 +177,26 @@ export class TimeBuckets {
    * @param {object|string|moment.duration} input - see desc
    */
   setInterval(input: null | string | Record<string, any>) {
-    let interval = input;
+    this._originalInterval = null;
+    this._i = isObject(input) ? input.val : input;
 
-    // selection object -> val
-    if (isObject(input) && !moment.isDuration(input)) {
-      interval = input.val;
-    }
-
-    if (!interval || interval === autoInterval) {
+    if (!this._i || this._i === autoInterval) {
       this._i = autoInterval;
       return;
     }
 
-    if (isString(interval)) {
-      input = interval;
-
-      // Preserve the original units because they're lost when the interval is converted to a
-      // moment duration object.
-      this._originalInterval = input;
+    if (isString(this._i)) {
+      const parsedInterval = parseInterval(this._i);
 
-      interval = parseInterval(interval);
-      if (interval === null || +interval === 0) {
-        interval = null;
+      if (isDurationInterval(parsedInterval)) {
+        this._originalInterval = this._i;
+        this._i = parsedInterval;
       }
     }
 
-    // if the value wasn't converted to a duration, and isn't
-    // already a duration, we have a problem
-    if (!moment.isDuration(interval)) {
-      throw new TypeError('"' + input + '" is not a valid interval.');
+    if (!isDurationInterval(this._i)) {
+      throw new TypeError('"' + this._i + '" is not a valid interval.');
     }
-
-    this._i = interval;
   }
 
   /**
@@ -235,15 +228,9 @@ export class TimeBuckets {
    */
   getInterval(useNormalizedEsInterval = true): TimeBucketsInterval {
     const duration = this.getDuration();
-
-    // either pull the interval from state or calculate the auto-interval
-    const readInterval = () => {
-      const interval = this._i;
-      if (moment.isDuration(interval)) return interval;
-      return calcAutoIntervalNear(this._timeBucketConfig['histogram:barTarget'], Number(duration));
-    };
-
-    const parsedInterval = readInterval();
+    const parsedInterval = isDurationInterval(this._i)
+      ? this._i
+      : calcAutoIntervalNear(this._timeBucketConfig['histogram:barTarget'], Number(duration));
 
     // check to see if the interval should be scaled, and scale it if so
     const maybeScaleInterval = (interval: moment.Duration) => {
@@ -271,10 +258,19 @@ export class TimeBuckets {
     };
 
     // append some TimeBuckets specific props to the interval
-    const decorateInterval = (interval: moment.Duration): TimeBucketsInterval => {
+    const decorateInterval = (
+      interval: Assign<moment.Duration, { scaled?: boolean }>
+    ): TimeBucketsInterval => {
+      let originalUnit: Unit | undefined;
+
+      if (!interval.scaled && this._originalInterval) {
+        originalUnit = splitStringInterval(this._originalInterval!)?.unit;
+      }
+
       const esInterval = useNormalizedEsInterval
-        ? convertDurationToNormalizedEsInterval(interval)
+        ? convertDurationToNormalizedEsInterval(interval, originalUnit)
         : convertIntervalToEsInterval(String(this._originalInterval));
+
       const prettyUnits = moment.normalizeUnits(esInterval.unit);
 
       return Object.assign(interval, {

src/plugins/data/common/search/aggs/utils/date_interval_utils/parse_interval.test.ts

@@ -7,17 +7,41 @@
  */
 
 import { Duration, unitOfTime } from 'moment';
-import { parseInterval } from './parse_interval';
+import { parseInterval, splitStringInterval } from './parse_interval';
+
+describe('splitStringInterval', () => {
+  test('should correctly split 1d interval', () => {
+    expect(splitStringInterval('1d')).toMatchInlineSnapshot(`
+      Object {
+        "unit": "d",
+        "value": 1,
+      }
+    `);
+  });
 
-const validateDuration = (duration: Duration | null, unit: unitOfTime.Base, value: number) => {
-  expect(duration).toBeDefined();
+  test('should correctly split 34m interval', () => {
+    expect(splitStringInterval('1d')).toMatchInlineSnapshot(`
+      Object {
+        "unit": "d",
+        "value": 1,
+      }
+    `);
+  });
 
-  if (duration) {
-    expect(duration.as(unit)).toBe(value);
-  }
-};
+  test('should return null if cannot parse interval', () => {
+    expect(splitStringInterval('wrong_value_1')).toMatchInlineSnapshot(`null`);
+  });
+});
 
 describe('parseInterval', () => {
+  const validateDuration = (duration: Duration | null, unit: unitOfTime.Base, value: number) => {
+    expect(duration).toBeDefined();
+
+    if (duration) {
+      expect(duration.as(unit)).toBe(value);
+    }
+  };
+
   describe('integer', () => {
     test('should correctly parse 1d interval', () => {
       validateDuration(parseInterval('1d'), 'd', 1);

src/plugins/data/common/search/aggs/utils/date_interval_utils/parse_interval.ts

@@ -7,21 +7,32 @@
  */
 
 import { find } from 'lodash';
-import moment, { unitOfTime } from 'moment';
-import dateMath from '@elastic/datemath';
+import moment from 'moment';
+import dateMath, { Unit } from '@elastic/datemath';
 
 // Assume interval is in the form (value)(unit), such as "1h"
 const INTERVAL_STRING_RE = new RegExp('^([0-9\\.]*)\\s*(' + dateMath.units.join('|') + ')$');
 
+export const splitStringInterval = (interval: string) => {
+  if (interval) {
+    const matches = interval.toString().trim().match(INTERVAL_STRING_RE);
+    if (matches) {
+      return {
+        value: parseFloat(matches[1]) || 1,
+        unit: matches[2] as Unit,
+      };
+    }
+  }
+  return null;
+};
+
 export function parseInterval(interval: string): moment.Duration | null {
-  const matches = String(interval).trim().match(INTERVAL_STRING_RE);
+  const parsedInterval = splitStringInterval(interval);
 
-  if (!matches) return null;
+  if (!parsedInterval) return null;
 
   try {
-    const value = parseFloat(matches[1]) || 1;
-    const unit = matches[2] as unitOfTime.Base;
-
+    const { value, unit } = parsedInterval;
     const duration = moment.duration(value, unit);
 
     // There is an error with moment, where if you have a fractional interval between 0 and 1, then when you add that
@@ -31,10 +42,7 @@ export function parseInterval(interval: string): moment.Duration | null {
     // adding 0.5 days until we hit the end date. However, since there is a bug in moment, when you add 0.5 days to
     // the start date, you get the same exact date (instead of being ahead by 12 hours). So instead of returning
     // a duration corresponding to 0.5 hours, we return a duration corresponding to 12 hours.
-    const selectedUnit = find(
-      dateMath.units,
-      (u) => Math.abs(duration.as(u)) >= 1
-    ) as unitOfTime.Base;
+    const selectedUnit = find(dateMath.units, (u) => Math.abs(duration.as(u)) >= 1) as Unit;
 
     // however if we do this fhe other way around it will also fail
     // go from 500m to hours as this will result in infinite number (dividing 500/60 = 8.3*)