From 088aa8f00ecc8ed80c2df06bd7679218c9654388 Mon Sep 17 00:00:00 2001 From: Kirk Swenson Date: Tue, 20 Aug 2024 10:40:24 -0700 Subject: [PATCH] fix: date arithmetic; inequality comparisons involving dates, numbers, and strings (#1413) * fix: date arithmetic * fix: comparison operators support dates, strings, etc. * fix: eliminate time zone from serialized date strings * feat: don't show time by default for dates without times * chore: code review tweak --- v3/cypress/e2e/attribute-types.spec.ts | 16 +- v3/src/components/case-table/use-columns.tsx | 7 +- v3/src/models/data/attribute.test.ts | 2 +- v3/src/models/data/attribute.ts | 10 +- .../formula/functions/function-utils.test.ts | 4 + .../formula/functions/function-utils.ts | 8 +- .../formula/functions/operators.test.ts | 215 +++++++++++++++++- v3/src/models/formula/functions/operators.ts | 159 +++++++++---- v3/src/utilities/date-iso-utils.test.ts | 36 +++ v3/src/utilities/date-iso-utils.ts | 30 +++ v3/src/utilities/date-parser.test.ts | 36 +-- v3/src/utilities/date-parser.ts | 33 +-- v3/src/utilities/date-utils.ts | 27 ++- v3/src/utilities/math-utils.ts | 9 +- 14 files changed, 474 insertions(+), 118 deletions(-) create mode 100644 v3/src/utilities/date-iso-utils.test.ts create mode 100644 v3/src/utilities/date-iso-utils.ts diff --git a/v3/cypress/e2e/attribute-types.spec.ts b/v3/cypress/e2e/attribute-types.spec.ts index cf54dc5562..148328eb7b 100644 --- a/v3/cypress/e2e/attribute-types.spec.ts +++ b/v3/cypress/e2e/attribute-types.spec.ts @@ -19,14 +19,14 @@ context("attribute types", () => { }) it("verify date", () => { table.getCell("4", "2").should("contain", "8/7/2017, 12:01 PM") - table.getCell("4", "3").should("contain", "6/15/1966, 12:00 AM") - table.getCell("4", "4").should("contain", "12/7/1787, 12:00 AM") - table.getCell("4", "5").should("contain", "1/2/2003, 12:00 AM") - table.getCell("4", "6").should("contain", "12/3/1818, 12:00 AM") - table.getCell("4", "7").should("contain", "6/1/1992, 12:00 AM") - table.getCell("4", "8").should("contain", "11/2/1989, 12:00 AM") - table.getCell("4", "9").should("contain", "11/16/2007, 12:00 AM") - table.getCell("4", "10").should("contain", "11/30/2000, 12:00 AM") + table.getCell("4", "3").should("contain", "6/15/1966") + table.getCell("4", "4").should("contain", "12/7/1787") + table.getCell("4", "5").should("contain", "1/2/2003") + table.getCell("4", "6").should("contain", "12/3/1818") + table.getCell("4", "7").should("contain", "6/1/1992") + table.getCell("4", "8").should("contain", "11/2/1989") + table.getCell("4", "9").should("contain", "11/16/2007") + table.getCell("4", "10").should("contain", "11/30/2000") }) it.skip("verify boolean", () => { table.getCell("5", "2").should("contain", "false") diff --git a/v3/src/components/case-table/use-columns.tsx b/v3/src/components/case-table/use-columns.tsx index 9c1fd2a9a5..2f3beb06dc 100644 --- a/v3/src/components/case-table/use-columns.tsx +++ b/v3/src/components/case-table/use-columns.tsx @@ -9,14 +9,15 @@ import { IDataSet } from "../../models/data/data-set" import { symParent } from "../../models/data/data-set-types" import { getCollectionAttrs } from "../../models/data/data-set-utils" import { parseColor } from "../../utilities/color-utils" +import { isStdISODateString } from "../../utilities/date-iso-utils" +import { parseDate } from "../../utilities/date-parser" +import { DatePrecision, formatDate } from "../../utilities/date-utils" import { mstReaction } from "../../utilities/mst-reaction" import { isCaseEditable } from "../../utilities/plugin-utils" import { kDefaultColumnWidth, symDom, TColumn, TRenderCellProps } from "./case-table-types" import CellTextEditor from "./cell-text-editor" import ColorCellTextEditor from "./color-cell-text-editor" import { ColumnHeader } from "./column-header" -import { isBrowserISOString, parseDate } from "../../utilities/date-parser" -import { DatePrecision, formatDate } from "../../utilities/date-utils" // cache d3 number formatters so we don't have to generate them on every render type TNumberFormatter = (n: number) => string @@ -62,7 +63,7 @@ export function renderValue(str = "", num = NaN, attr?: IAttribute, key?: number // This is because CODAP v3 stores all the case values as strings natively, and we cannot simply check if the value // is an instance of the `Date` class (as it will never be). Date.toISOString() is the native way of serializing dates // in CODAP v3 (check `importValueToString` from attribute.ts). - if (isBrowserISOString(str) || userType === "date" && str !== "") { + if (isStdISODateString(str) || userType === "date" && str !== "") { const date = parseDate(str, true) if (date) { // TODO: add precision support for date formatting diff --git a/v3/src/models/data/attribute.test.ts b/v3/src/models/data/attribute.test.ts index 712a0d8d0c..de6ed65216 100644 --- a/v3/src/models/data/attribute.test.ts +++ b/v3/src/models/data/attribute.test.ts @@ -42,7 +42,7 @@ describe("Attribute", () => { expect(importValueToString(1e-6)).toBe("0.000001") expect(importValueToString(true)).toBe("true") expect(importValueToString(false)).toBe("false") - expect(importValueToString(new Date("2020-06-14T10:13:34.123Z"))).toBe("2020-06-14T10:13:34.123Z") + expect(importValueToString(new Date(2020, 5, 14, 10, 13, 34, 123))).toBe("2020-06-14T10:13:34.123Z") const attr = Attribute.create({ name: "a" }) expect(attr.toNumeric(null as any)).toBeNaN() diff --git a/v3/src/models/data/attribute.ts b/v3/src/models/data/attribute.ts index 4881cacd04..ddf1668dfc 100644 --- a/v3/src/models/data/attribute.ts +++ b/v3/src/models/data/attribute.ts @@ -26,14 +26,15 @@ */ import { Instance, SnapshotIn, types } from "mobx-state-tree" -import { parseColor } from "../../utilities/color-utils" import { kAttrIdPrefix, typeV3Id } from "../../utilities/codap-utils" +import { parseColor } from "../../utilities/color-utils" +import { formatStdISODateString } from "../../utilities/date-iso-utils" +import { isDateString } from "../../utilities/date-parser" import { cachedFnFactory } from "../../utilities/mst-utils" import { Formula, IFormula } from "../formula/formula" import { applyModelChange } from "../history/apply-model-change" import { withoutUndo } from "../history/without-undo" import { V2Model } from "./v2-model" -import { isDateString } from "../../utilities/date-parser" export const kDefaultFormatStr = ".3~f" @@ -54,10 +55,7 @@ export function importValueToString(value: IValueType): string { return value } if (value instanceof Date) { - // Convert Date to ISO string format. It's a consistent format that can be parsed back into a Date object - // without losing any information. Also, it's relatively compact and it can be easily recognized as a date string, - // in contrast to storing the date as a number (e.g. milliseconds since epoch). - return value.toISOString() + return formatStdISODateString(value) } return value.toString() } diff --git a/v3/src/models/formula/functions/function-utils.test.ts b/v3/src/models/formula/functions/function-utils.test.ts index c17d2113bf..f5bd0c3fe8 100644 --- a/v3/src/models/formula/functions/function-utils.test.ts +++ b/v3/src/models/formula/functions/function-utils.test.ts @@ -25,6 +25,8 @@ describe("equal", () => { expect(equal("1", "1")).toBe(true) expect(equal(true, "true")).toBe(true) expect(equal("true", true)).toBe(true) + const now = new Date() + expect(equal(now, new Date(now))).toBe(true) }) it("should return false for unequal values", () => { @@ -35,6 +37,8 @@ describe("equal", () => { expect(equal(true, "false")).toBe(false) expect(equal("true", false)).toBe(false) expect(equal(true, 1)).toBe(false) + const now = new Date() + expect(equal(now, new Date(Date.now() + 3600))).toBe(false) }) }) diff --git a/v3/src/models/formula/functions/function-utils.ts b/v3/src/models/formula/functions/function-utils.ts index cd72b438e8..7beea04248 100644 --- a/v3/src/models/formula/functions/function-utils.ts +++ b/v3/src/models/formula/functions/function-utils.ts @@ -1,6 +1,7 @@ import { MathNode } from "mathjs" -import { FormulaMathJsScope } from "../formula-mathjs-scope" +import { checkDate } from "../../../utilities/date-utils" import { isValueNonEmpty } from "../../../utilities/math-utils" +import { FormulaMathJsScope } from "../formula-mathjs-scope" import { CurrentScope, MathJSPartitionedMap } from "../formula-types" export { isNumber, isValueNonEmpty } from "../../../utilities/math-utils" @@ -12,6 +13,11 @@ export const isValueTruthy = (value: any) => isValueNonEmpty(value) && value !== export const equal = (a: any, b: any): boolean => { + // Date objects are compared numerically as seconds + const [isADate, aDate] = checkDate(a) + const [isBDate, bDate] = checkDate(b) + if (isADate) a = aDate.valueOf() / 1000 + if (isBDate) b = bDate.valueOf() / 1000 // Checks below might seem redundant once the data set cases start using typed values, but they are not. // Note that user might still compare a string with a number unintentionally, and it makes sense to try to cast // values when possible, so that the comparison can be performed without forcing users to think about types. diff --git a/v3/src/models/formula/functions/operators.test.ts b/v3/src/models/formula/functions/operators.test.ts index c25d108d32..0e0e78f99a 100644 --- a/v3/src/models/formula/functions/operators.test.ts +++ b/v3/src/models/formula/functions/operators.test.ts @@ -1,6 +1,188 @@ -import { formatDate } from "../../../utilities/date-utils" import { math } from "./math" +describe("= operator", () => { + it("compares dates for equality", () => { + const fn1 = math.compile(`today() == today()`) + expect(fn1.evaluate()).toBe(true) + const fn2 = math.compile(`today() == today() + 24 * 3600`) + expect(fn2.evaluate()).toBe(false) + }) + it("compares dates for inequality", () => { + const fn1 = math.compile(`today() != today()`) + expect(fn1.evaluate()).toBe(false) + const fn2 = math.compile(`today() != today() + 24 * 3600`) + expect(fn2.evaluate()).toBe(true) + }) +}) + +describe("< operator", () => { + it("compares numbers", () => { + const f1 = math.compile("1 < 2") + expect(f1.evaluate()).toBe(true) + const f2 = math.compile("1 < 1") + expect(f2.evaluate()).toBe(false) + const f3 = math.compile("2 < 1") + expect(f3.evaluate()).toBe(false) + // NaN comparisons are always false + const f4 = math.compile("0/0 < 0") + expect(f4.evaluate()).toBe(false) + const f5 = math.compile("0 < 0/0") + expect(f5.evaluate()).toBe(false) + }) + it("compares dates numerically", () => { + const f1 = math.compile("today() < today()") + expect(f1.evaluate()).toBe(false) + const f2 = math.compile("today() + 24 * 3600 < today()") + expect(f2.evaluate()).toBe(false) + const f3 = math.compile("today() < today() + 24 * 3600") + expect(f3.evaluate()).toBe(true) + }) + it("compares numeric strings", () => { + const f1 = math.compile("'1' < '2'") + expect(f1.evaluate()).toBe(true) + const f2 = math.compile("'1' < '1'") + expect(f2.evaluate()).toBe(false) + const f3 = math.compile("'2' < '1'") + expect(f3.evaluate()).toBe(false) + const f4 = math.compile("'1' < '12'") + expect(f4.evaluate()).toBe(true) + }) + it("compares strings", () => { + const f1 = math.compile("'abc' < 'def'") + expect(f1.evaluate()).toBe(true) + const f2 = math.compile("'abc' < 'abc'") + expect(f2.evaluate()).toBe(false) + const f3 = math.compile("'def' < 'abc'") + expect(f3.evaluate()).toBe(false) + }) +}) + +describe("<= operator", () => { + it("compares numbers", () => { + const f1 = math.compile("1 <= 2") + expect(f1.evaluate()).toBe(true) + const f2 = math.compile("1 <= 1") + expect(f2.evaluate()).toBe(true) + const f3 = math.compile("2 <= 1") + expect(f3.evaluate()).toBe(false) + // NaN comparisons are always false + const f4 = math.compile("0/0 <= 0") + expect(f4.evaluate()).toBe(false) + const f5 = math.compile("0 <= 0/0") + expect(f5.evaluate()).toBe(false) + }) + it("compares dates numerically", () => { + const f1 = math.compile("today() <= today()") + expect(f1.evaluate()).toBe(true) + const f2 = math.compile("today() + 24 * 3600 <= today()") + expect(f2.evaluate()).toBe(false) + const f3 = math.compile("today() <= today() + 24 * 3600") + expect(f3.evaluate()).toBe(true) + }) + it("compares numeric strings", () => { + const f1 = math.compile("'1' <= '2'") + expect(f1.evaluate()).toBe(true) + const f2 = math.compile("'1' <= '1'") + expect(f2.evaluate()).toBe(true) + const f3 = math.compile("'2' <= '1'") + expect(f3.evaluate()).toBe(false) + const f4 = math.compile("'1' <= '12'") + expect(f4.evaluate()).toBe(true) + }) + it("compares strings", () => { + const f1 = math.compile("'abc' <= 'def'") + expect(f1.evaluate()).toBe(true) + const f2 = math.compile("'abc' <= 'abc'") + expect(f2.evaluate()).toBe(true) + const f3 = math.compile("'def' < 'abc'") + expect(f3.evaluate()).toBe(false) + }) +}) + +describe("> operator", () => { + it("compares numbers", () => { + const f1 = math.compile("1 > 2") + expect(f1.evaluate()).toBe(false) + const f2 = math.compile("1 > 1") + expect(f2.evaluate()).toBe(false) + const f3 = math.compile("2 > 1") + expect(f3.evaluate()).toBe(true) + // NaN comparisons are always false + const f4 = math.compile("0/0 > 0") + expect(f4.evaluate()).toBe(false) + const f5 = math.compile("0 > 0/0") + expect(f5.evaluate()).toBe(false) + }) + it("compares dates numerically", () => { + const f1 = math.compile("today() > today()") + expect(f1.evaluate()).toBe(false) + const f2 = math.compile("today() + 24 * 3600 > today()") + expect(f2.evaluate()).toBe(true) + const f3 = math.compile("today() > today() + 24 * 3600") + expect(f3.evaluate()).toBe(false) + }) + it("compares numeric strings", () => { + const f1 = math.compile("'1' > '2'") + expect(f1.evaluate()).toBe(false) + const f2 = math.compile("'1' > '1'") + expect(f2.evaluate()).toBe(false) + const f3 = math.compile("'2' > '1'") + expect(f3.evaluate()).toBe(true) + const f4 = math.compile("'1' > '12'") + expect(f4.evaluate()).toBe(false) + }) + it("compares strings", () => { + const f1 = math.compile("'abc' > 'def'") + expect(f1.evaluate()).toBe(false) + const f2 = math.compile("'abc' > 'abc'") + expect(f2.evaluate()).toBe(false) + const f3 = math.compile("'def' > 'abc'") + expect(f3.evaluate()).toBe(true) + }) +}) + +describe(">= operator", () => { + it("compares numbers", () => { + const f1 = math.compile("1 >= 2") + expect(f1.evaluate()).toBe(false) + const f2 = math.compile("1 >= 1") + expect(f2.evaluate()).toBe(true) + const f3 = math.compile("2 >= 1") + expect(f3.evaluate()).toBe(true) + // NaN comparisons are always false + const f4 = math.compile("0/0 >= 0") + expect(f4.evaluate()).toBe(false) + const f5 = math.compile("0 >= 0/0") + expect(f5.evaluate()).toBe(false) + }) + it("compares dates numerically", () => { + const f1 = math.compile("today() >= today()") + expect(f1.evaluate()).toBe(true) + const f2 = math.compile("today() + 24 * 3600 >= today()") + expect(f2.evaluate()).toBe(true) + const f3 = math.compile("today() >= today() + 24 * 3600") + expect(f3.evaluate()).toBe(false) + }) + it("compares numeric strings", () => { + const f1 = math.compile("'1' >= '2'") + expect(f1.evaluate()).toBe(false) + const f2 = math.compile("'1' >= '1'") + expect(f2.evaluate()).toBe(true) + const f3 = math.compile("'2' >= '1'") + expect(f3.evaluate()).toBe(true) + const f4 = math.compile("'1' >= '12'") + expect(f4.evaluate()).toBe(false) + }) + it("compares strings", () => { + const f1 = math.compile("'abc' >= 'def'") + expect(f1.evaluate()).toBe(false) + const f2 = math.compile("'abc' >= 'abc'") + expect(f2.evaluate()).toBe(true) + const f3 = math.compile("'def' >= 'abc'") + expect(f3.evaluate()).toBe(true) + }) +}) + describe("+ operator", () => { it("adds two numbers", () => { const fn = math.compile("1 + 2") @@ -17,23 +199,26 @@ describe("+ operator", () => { expect(fn.evaluate()).toEqual(3) }) - it("adds dates", () => { + it("concatenates a number and a non-numeric string", () => { + const fn = math.compile("1 + 'a'") + expect(fn.evaluate()).toBe("1a") + }) + + it("throws an exception when adding dates", () => { const fn = math.compile("'1/1/2020' + '1/1/2020'") - const val1 = new Date(2020, 0, 1).valueOf() - const val2 = new Date(2020, 0, 1).valueOf() - expect(fn.evaluate()).toEqual(formatDate(new Date(val1 + val2))) + expect(() => fn.evaluate()).toThrow() }) it("adds seconds to dates", () => { const fn = math.compile("'1/1/2020' + 60 * 60") // + 1 hour - expect(fn.evaluate()).toEqual(formatDate(new Date(2020, 0, 1, 1))) + expect(fn.evaluate()).toEqual(new Date(2020, 0, 1, 1)) const fn2 = math.compile("'1/1/2020' + 60 * 60 * 24") // + 1 day - expect(fn2.evaluate()).toEqual(formatDate(new Date(2020, 0, 2))) + expect(fn2.evaluate()).toEqual(new Date(2020, 0, 2)) const fn3 = math.compile("60 * 60 + '1/1/2020'") // + 1 hour - expect(fn3.evaluate()).toEqual(formatDate(new Date(2020, 0, 1, 1))) + expect(fn3.evaluate()).toEqual(new Date(2020, 0, 1, 1)) const fn4 = math.compile("60 * 60 * 24 + '1/1/2020'") // + 1 day - expect(fn4.evaluate()).toEqual(formatDate(new Date(2020, 0, 2))) + expect(fn4.evaluate()).toEqual(new Date(2020, 0, 2)) }) it("concatenates strings", () => { @@ -62,7 +247,17 @@ describe("- operator", () => { const fn = math.compile("'1/2/2020' - '1/1/2020'") // 1 day const val1 = new Date(2020, 0, 2).valueOf() const val2 = new Date(2020, 0, 1).valueOf() - expect(fn.evaluate()).toEqual(formatDate(new Date(val1 - val2))) + expect(fn.evaluate()).toEqual(new Date(val1 - val2)) + }) + + it("subtracts a number from a date", () => { + const fn = math.compile("'1/2/2020' - 86400") // 1 day + expect(fn.evaluate()).toEqual(new Date(2020, 0, 1)) + }) + + it("throws an error when subtracting a date from a number", () => { + const fn = math.compile("86400 - '1/2/2020'") // 1 day + expect(() => fn.evaluate()).toThrow() }) it("throws an error when subtracting strings", () => { diff --git a/v3/src/models/formula/functions/operators.ts b/v3/src/models/formula/functions/operators.ts index 00b0af4aa3..f8a5af8a1f 100644 --- a/v3/src/models/formula/functions/operators.ts +++ b/v3/src/models/formula/functions/operators.ts @@ -1,6 +1,6 @@ -import { equal, isNumber, UNDEF_RESULT } from './function-utils' -import { parseDate } from '../../../utilities/date-parser' -import { formatDate } from '../../../utilities/date-utils' +import { checkDate } from '../../../utilities/date-utils' +import { checkNumber } from '../../../utilities/math-utils' +import { equal } from './function-utils' export const operators = { // equal(a, b) or a == b @@ -18,38 +18,114 @@ export const operators = { evaluateOperator: (a: any, b: any) => !equal(a, b) }, + smaller: { + isOperator: true, + numOfRequiredArguments: 2, + evaluateOperator: (a: any, b: any) => { + if (a == null || b == null || Number.isNaN(a) || Number.isNaN(b)) return false + const [isADate, aDate] = checkDate(a) + const [isBDate, bDate] = checkDate(b) + if (isADate) a = aDate.valueOf() / 1000 + if (isBDate) b = bDate.valueOf() / 1000 + // compare numerically if possible + const [isANumber, aNumber] = checkNumber(a) + const [isBNumber, bNumber] = checkNumber(b) + if (isANumber && isBNumber) return aNumber < bNumber + // compare as strings + return String(a) < String(b) + } + }, + + smallerEq: { + isOperator: true, + numOfRequiredArguments: 2, + evaluateOperator: (a: any, b: any) => { + if (a == null || b == null || Number.isNaN(a) || Number.isNaN(b)) return false + const [isADate, aDate] = checkDate(a) + const [isBDate, bDate] = checkDate(b) + if (isADate) a = aDate.valueOf() / 1000 + if (isBDate) b = bDate.valueOf() / 1000 + // compare numerically if possible + const [isANumber, aNumber] = checkNumber(a) + const [isBNumber, bNumber] = checkNumber(b) + if (isANumber && isBNumber) return aNumber <= bNumber + // compare as strings + return String(a) <= String(b) + } + }, + + larger: { + isOperator: true, + numOfRequiredArguments: 2, + evaluateOperator: (a: any, b: any) => { + if (a == null || b == null || Number.isNaN(a) || Number.isNaN(b)) return false + const [isADate, aDate] = checkDate(a) + const [isBDate, bDate] = checkDate(b) + if (isADate) a = aDate.valueOf() / 1000 + if (isBDate) b = bDate.valueOf() / 1000 + // compare numerically if possible + const [isANumber, aNumber] = checkNumber(a) + const [isBNumber, bNumber] = checkNumber(b) + if (isANumber && isBNumber) return aNumber > bNumber + // compare as strings + return String(a) > String(b) + } + }, + + largerEq: { + isOperator: true, + numOfRequiredArguments: 2, + evaluateOperator: (a: any, b: any) => { + if (a == null || b == null || Number.isNaN(a) || Number.isNaN(b)) return false + const [isADate, aDate] = checkDate(a) + const [isBDate, bDate] = checkDate(b) + if (isADate) a = aDate.valueOf() / 1000 + if (isBDate) b = bDate.valueOf() / 1000 + // compare numerically if possible + const [isANumber, aNumber] = checkNumber(a) + const [isBNumber, bNumber] = checkNumber(b) + if (isANumber && isBNumber) return aNumber >= bNumber + // compare as strings + return String(a) >= String(b) + } + }, + add: { isOperator: true, numOfRequiredArguments: 2, evaluateOperator: (a: any, b: any) => { - // Two numbers or numeric strings - const isANumber = isNumber(a) - const isBNumber = isNumber(b) - if (isANumber && isBNumber) { - return Number(a) + Number(b) - } + const addError = new Error(`Invalid arguments for add operator: ${a}, ${b}`) + + const [isADate, aDate] = checkDate(a) + const [isBDate, bDate] = checkDate(b) + const [isANumber, aNumber] = checkNumber(a) + const [isBNumber, bNumber] = checkNumber(b) - // Dates and numbers - const aDate = isANumber ? null : parseDate(a, true) - const bDate = isBNumber ? null : parseDate(b, true) - if (aDate != null && bDate != null) { - return formatDate(new Date(aDate.valueOf() + bDate.valueOf())) || UNDEF_RESULT + // both are dates + if (isADate && isBDate) { + throw addError } - // When one of the arguments is a date and the other is a number, we assume that the number is in seconds. - if (aDate != null && isBNumber) { - return formatDate(new Date(aDate.valueOf() + Number(b) * 1000)) || UNDEF_RESULT + // add a number in seconds to a date + if (isADate && isBNumber) { + return new Date(aDate.valueOf() + bNumber * 1000) } - if (isANumber && bDate != null) { - return formatDate(new Date(Number(a) * 1000 + bDate.valueOf())) || UNDEF_RESULT + if (isANumber && isBDate) { + return new Date(aNumber * 1000 + bDate.valueOf()) + } + + // Numbers + if (isANumber && isBNumber) { + return aNumber + bNumber } // Strings - if (typeof a === 'string' || typeof b === 'string') { + if (typeof a === "string" || typeof b === "string") { // Two non-date and non-numeric strings, so concatenate them. return a + b } - throw new Error(`Invalid arguments for add operator: ${a}, ${b}`) + /* istanbul ignore next */ + throw addError } }, @@ -57,28 +133,33 @@ export const operators = { isOperator: true, numOfRequiredArguments: 2, evaluateOperator: (a: any, b: any) => { - // Two numbers or numeric strings - const isANumber = isNumber(a) - const isBNumber = isNumber(b) - if (isANumber && isBNumber) { - return Number(a) - Number(b) - } + const subtractError = new Error(`Invalid arguments for subtract operator: ${a}, ${b}`) - // Dates and numbers - const aDate = isANumber ? null : parseDate(a, true) - const bDate = isBNumber ? null : parseDate(b, true) - if (aDate != null && bDate != null) { - return formatDate(new Date(aDate.valueOf() - bDate.valueOf())) || UNDEF_RESULT - } - // When one of the arguments is a date and the other is a number, we assume that the number is in seconds. - if (aDate != null && isBNumber) { - return formatDate(new Date(aDate.valueOf() - Number(b) * 1000)) || UNDEF_RESULT + const [isADate, aDate] = checkDate(a) + const [isBDate, bDate] = checkDate(b) + const [isANumber, aNumber] = checkNumber(a) + const [isBNumber, bNumber] = checkNumber(b) + + // Date objects + if (isADate || isBDate) { + // both are dates + if (isADate && isBDate) { + return new Date(aDate.valueOf() - bDate.valueOf()) + } + // subtract seconds from a date + if (isADate && isBNumber) { + return new Date(aDate.valueOf() - bNumber * 1000) + } + // can't subtract a date from seconds, etc. + throw subtractError } - if (isANumber && bDate != null) { - return formatDate(new Date(Number(a) * 1000 - bDate.valueOf())) || UNDEF_RESULT + + // Numbers + if (isANumber && isBNumber) { + return aNumber - bNumber } - throw new Error(`Invalid arguments for subtract operator: ${a}, ${b}`) + throw subtractError } } } diff --git a/v3/src/utilities/date-iso-utils.test.ts b/v3/src/utilities/date-iso-utils.test.ts new file mode 100644 index 0000000000..e3bab10cfa --- /dev/null +++ b/v3/src/utilities/date-iso-utils.test.ts @@ -0,0 +1,36 @@ +import { formatStdISODateString, isStdISODateString, parseStdISODateString } from "./date-iso-utils" + +describe('isStdISODateString', () => { + test('returns true for strings that were produced by native Date.toISOString() method', () => { + expect(isStdISODateString(new Date().toISOString())).toBe(true) + expect(isStdISODateString(new Date(2023, 7, 17, 15, 30, 45, 123).toISOString())).toBe(true) + expect(isStdISODateString(new Date(-2023, 7, 17, 15, 30, 45, 123).toISOString())).toBe(true) + expect(isStdISODateString('2023-08-17T15:30:45.123Z')).toBe(true) + expect(isStdISODateString('-002023-08-17T15:30:45.123Z')).toBe(true) + }) + test('returns false for strings that were not produced by native Date.toISOString() method', () => { + // Still valid ISO date strings, but not produced by native Date.toISOString() method + expect(isStdISODateString('2023-08-17T15:30:45.123')).toBe(false) + expect(isStdISODateString('2023-08-17T15:30:45.123Z+07:00')).toBe(false) + expect(isStdISODateString('2023-08-17T15:30:45.123+07:00')).toBe(false) + expect(isStdISODateString('2023-08-17T15:30:45.123-07:00')).toBe(false) + expect(isStdISODateString('002023-08-17T15:30:45.123Z')).toBe(false) + }) +}) + +describe('formatStdISODateString', () => { + test('works as expected', () => { + expect(formatStdISODateString(new Date(2023, 7, 17, 15, 30, 45, 123))).toBe('2023-08-17T15:30:45.123Z') + }) +}) + +describe('formatStdISODateString & parseStdISODateString', () => { + test('demonstrates round-trip fidelity', () => { + const date1Str = formatStdISODateString(new Date()) + expect(formatStdISODateString(parseStdISODateString(date1Str))).toBe(date1Str) + const date2Str = formatStdISODateString(new Date(2023, 7, 17, 15, 30, 45, 123)) + expect(formatStdISODateString(parseStdISODateString(date2Str))).toBe(date2Str) + const date3Str = formatStdISODateString(new Date(-2023, 7, 17, 15, 30, 45, 123)) + expect(formatStdISODateString(parseStdISODateString(date3Str))).toBe(date3Str) + }) +}) diff --git a/v3/src/utilities/date-iso-utils.ts b/v3/src/utilities/date-iso-utils.ts new file mode 100644 index 0000000000..e0c13f06f4 --- /dev/null +++ b/v3/src/utilities/date-iso-utils.ts @@ -0,0 +1,30 @@ +// Regular expression to match ISO 8601 date strings as produced by Date.toISOString. +// Note that this regular expression is more strict than the one used in parseDate (isoDateTimeRE) which supports +// additional formats. +const browserIsoDatePattern = /^([+-]\d{6}|\d{4})-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}.\d{3}Z$/ + +export function isStdISODateString(value: string): boolean { + return browserIsoDatePattern.test(value) +} + +export function getTimeZoneOffsetInMilliseconds(date: Date) { + return date.getTimezoneOffset() * 60000 +} + +export function formatStdISODateString(date: Date) { + // Convert Date to ISO string format. It's a consistent format that can be parsed back into a Date object + // without losing any information. Also, it's relatively compact and it can be easily recognized as a date string, + // in contrast to storing the date as a number (e.g. milliseconds since epoch). + // We subtract the effect of the time-zone offset, so that we're effectively storing an abstract + // time-zone-less value. The local time zone offset is re-applied on restore, so that for instance, + // midnight local time is serialized as midnight UTC time and restored as midnight local time, + // even if it is serialized/deserialized in different time zones. + return new Date(date.getTime() - getTimeZoneOffsetInMilliseconds(date)).toISOString() +} + +export function parseStdISODateString(dateStr: string) { + // local dates written out as ISO strings are implicitly converted to UTC, + // so we add back the local time zone information when converting to date. + const _utcDate = new Date(dateStr) + return new Date(_utcDate.getTime() + getTimeZoneOffsetInMilliseconds(_utcDate)) +} diff --git a/v3/src/utilities/date-parser.test.ts b/v3/src/utilities/date-parser.test.ts index 6bc7535b73..3f48be8a5f 100644 --- a/v3/src/utilities/date-parser.test.ts +++ b/v3/src/utilities/date-parser.test.ts @@ -1,4 +1,4 @@ -import { fixYear, isBrowserISOString, isDateString, isValidDateSpec, parseDate } from './date-parser' +import { fixYear, isDateString, isValidDateSpec, parseDate } from './date-parser' describe('Date Parser tests - V2 compatibility', () => { // These tests are ported from V2 and should always pass unchanged as long as we want to maintain compatibility. @@ -135,7 +135,7 @@ describe('isValidDateSpec', () => { test('returns null when month is out of range', () => { const invalidDateSpec = { year: 2023, - month: 13, + month: Infinity, day: 17, hour: 15, min: 30, @@ -148,20 +148,20 @@ describe('isValidDateSpec', () => { const invalidDateSpec = { year: 2023, month: 7, - day: 32, + day: null, hour: 15, min: 30, sec: 45, subsec: 123 } - expect(isValidDateSpec(invalidDateSpec)).toBeFalsy() + expect(isValidDateSpec(invalidDateSpec as any)).toBeFalsy() }) test('returns null when hour is out of range', () => { const invalidDateSpec = { year: 2023, month: 7, day: 17, - hour: 24, + hour: -Infinity, min: 30, sec: 45, subsec: 123 @@ -174,11 +174,11 @@ describe('isValidDateSpec', () => { month: 7, day: 17, hour: 15, - min: 60, + min: undefined, sec: 45, subsec: 123 } - expect(isValidDateSpec(invalidDateSpec)).toBeFalsy() + expect(isValidDateSpec(invalidDateSpec as any)).toBeFalsy() }) test('returns null when second is out of range', () => { const invalidDateSpec = { @@ -187,10 +187,10 @@ describe('isValidDateSpec', () => { day: 17, hour: 15, min: 30, - sec: 60, + sec: "", subsec: 123 } - expect(isValidDateSpec(invalidDateSpec)).toBeFalsy() + expect(isValidDateSpec(invalidDateSpec as any)).toBeFalsy() }) test('returns null when subsecond is NaN', () => { const invalidDateSpec = { @@ -223,21 +223,3 @@ describe('fixYear', () => { expect(fixYear(99)).toEqual(1999) }) }) - -describe('isBrowserISOString', () => { - test('returns true for strings that were produced by native Date.toISOString() method', () => { - expect(isBrowserISOString(new Date().toISOString())).toBe(true) - expect(isBrowserISOString(new Date(2023, 7, 17, 15, 30, 45, 123).toISOString())).toBe(true) - expect(isBrowserISOString(new Date(-2023, 7, 17, 15, 30, 45, 123).toISOString())).toBe(true) - expect(isBrowserISOString('2023-08-17T15:30:45.123Z')).toBe(true) - expect(isBrowserISOString('-002023-08-17T15:30:45.123Z')).toBe(true) - }) - test('returns false for strings that were not produced by native Date.toISOString() method', () => { - // Still valid ISO date strings, but not produced by native Date.toISOString() method - expect(isBrowserISOString('2023-08-17T15:30:45.123')).toBe(false) - expect(isBrowserISOString('2023-08-17T15:30:45.123Z+07:00')).toBe(false) - expect(isBrowserISOString('2023-08-17T15:30:45.123+07:00')).toBe(false) - expect(isBrowserISOString('2023-08-17T15:30:45.123-07:00')).toBe(false) - expect(isBrowserISOString('002023-08-17T15:30:45.123Z')).toBe(false) - }) -}) diff --git a/v3/src/utilities/date-parser.ts b/v3/src/utilities/date-parser.ts index 8f68916743..a8c0bc71cb 100644 --- a/v3/src/utilities/date-parser.ts +++ b/v3/src/utilities/date-parser.ts @@ -1,3 +1,5 @@ +import { isStdISODateString, parseStdISODateString } from "./date-iso-utils" +import { isFiniteNumber } from "./math-utils" import { t } from "./translation/translate" /** @@ -218,14 +220,19 @@ export function extractDateProps(match: string[], map: GroupMap): DateSpec { } export function isValidDateSpec(dateSpec: DateSpec) { + // Note: we're allowing out-of-range values with the overflow/underflow + // semantics defined by the Date constructor: + // https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Date/Date. + // This mirrors the v2 behavior (which was the result of an apparent coding bug) but has the + // advantage of giving reasonable interpretations to constructions that would otherwise fail. const isValid = - !isNaN(dateSpec.year) && - (!isNaN(dateSpec.month) && (1 <= dateSpec.month && dateSpec.month <= 12)) && - (!isNaN(dateSpec.day) && (1 <= dateSpec.day && dateSpec.day <= 31)) && - (!isNaN(dateSpec.hour) && (0 <= dateSpec.hour && dateSpec.hour <= 23)) && - (!isNaN(dateSpec.min) && (0 <= dateSpec.min && dateSpec.min <= 59)) && - (!isNaN(dateSpec.sec) && (0 <= dateSpec.sec && dateSpec.sec <= 59)) && - !isNaN(dateSpec.subsec) + isFiniteNumber(dateSpec.year) && + isFiniteNumber(dateSpec.month) && + isFiniteNumber(dateSpec.day) && + isFiniteNumber(dateSpec.hour) && + isFiniteNumber(dateSpec.min) && + isFiniteNumber(dateSpec.sec) && + isFiniteNumber(dateSpec.subsec) return isValid ? dateSpec : false } @@ -238,6 +245,9 @@ export function parseDateV2Compatible(iValue: any, iLoose?: boolean) { return iValue } iValue = String(iValue) + if (isStdISODateString(iValue)) { + return parseStdISODateString(iValue) + } let match let dateSpec: DateSpec | false let groupMap: GroupMap | null = null @@ -302,12 +312,3 @@ export function isDateString(iValue: any, iLoose?: boolean) { return spec.regex.test(iValue) }) || (!!iLoose && parseDateV3(iValue) != null) } - -// Regular expression to match ISO 8601 date strings as produced by Date.toISOString. -// Note that this regular expression is more strict than the one used in parseDate (isoDateTimeRE) which supports -// additional formats. -const browserIsoDatePattern = /^([+-]\d{6}|\d{4})-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}.\d{3}Z$/ - -export function isBrowserISOString(value: string): boolean { - return browserIsoDatePattern.test(value) -} diff --git a/v3/src/utilities/date-utils.ts b/v3/src/utilities/date-utils.ts index 7b0a876087..c483ae4d9a 100644 --- a/v3/src/utilities/date-utils.ts +++ b/v3/src/utilities/date-utils.ts @@ -122,6 +122,13 @@ export function isDate(iValue: any): iValue is Date { return iValue instanceof Date } +// returns whether the specified value is interpretable as a date, and if so its date value +export function checkDate(value: any): [false] | [true, Date] { + if (value instanceof Date) return [true, value] + const result = parseDate(value) + return result ? [true, result] : [false] +} + /** * Default formatting for Date objects. * @param date {Date | number | string | null } @@ -130,8 +137,7 @@ export function isDate(iValue: any): iValue is Date { */ export function formatDate(x: Date | number | string | null, precision: DatePrecision = DatePrecision.None): string | null { - const formatPrecisions: Record = { - [DatePrecision.None]: null, + const formatPrecisions: Partial> = { [DatePrecision.Year]: { year: 'numeric' }, [DatePrecision.Month]: { year: 'numeric', month: 'numeric' }, [DatePrecision.Day]: { year: 'numeric', month: 'numeric', day: 'numeric' }, @@ -140,11 +146,9 @@ export function formatDate(x: Date | number | string | null, precision: DatePrec [DatePrecision.Second]: { year: 'numeric', month: 'numeric', day: 'numeric', hour: 'numeric', minute: 'numeric', second: 'numeric' }, [DatePrecision.Millisecond]: { year: 'numeric', month: 'numeric', day: 'numeric', hour: 'numeric', - minute: 'numeric', second: 'numeric', fractionalSecondDigits: 3 } + minute: 'numeric', second: 'numeric', fractionalSecondDigits: 3 } as Intl.DateTimeFormatOptions } - const precisionFormat = formatPrecisions[precision] || formatPrecisions.minute - if (!(x && (isDate(x) || isDateString(x) || isFiniteNumber(x)))) { return null } @@ -159,8 +163,19 @@ export function formatDate(x: Date | number | string | null, precision: DatePrec x = new Date(x) } + // not convertible to a date + if (typeof x === "string") return null + + // default to minutes if the value contains time information, or days if it doesn't + let precisionFormat = formatPrecisions[precision] + if (!precisionFormat) { + precisionFormat = (x.getHours() > 0 || x.getMinutes() > 0) + ? formatPrecisions.minute + : formatPrecisions.day + } + const locale = getDefaultLanguage() - return new Intl.DateTimeFormat(locale, precisionFormat).format(x as Date) + return new Intl.DateTimeFormat(locale, precisionFormat).format(x) } /** diff --git a/v3/src/utilities/math-utils.ts b/v3/src/utilities/math-utils.ts index ac5c37adf5..017857615a 100644 --- a/v3/src/utilities/math-utils.ts +++ b/v3/src/utilities/math-utils.ts @@ -114,6 +114,14 @@ export const isValueNonEmpty = (value: any) => value !== "" && value != null // It allows for strings that can be converted to numbers and treats Infinity and -Infinity as valid numbers. export const isNumber = (v: any) => isValueNonEmpty(v) && !isNaN(Number(v)) +// returns whether the value can be interpreted as a number and if so, its value +export function checkNumber(value: any) : [false] | [true, number] { + if (typeof value === "number") return [true, value] + if (value == null || value === "") return [false] + const result = Number(value) + return isNaN(result) ? [false] : [true, result] +} + export const extractNumeric = (v: any) => { if (!isValueNonEmpty(v)) { return null @@ -257,4 +265,3 @@ export function fitGaussianGradientDescent(points: {x:number, y:number}[], amp:n return { mu: muSigma[0], sigma: muSigma[1] } } -