Fix for inconsistent timezone handling (#5931)

* Consistent creation of zoned temporal values

* Remove comments in scalars

* Fix tests

* Fix test

.changeset/good-sheep-refuse.md

@@ -0,0 +1,5 @@
+---
+"@neo4j/graphql": major
+---
+
+`DateTime` and `Time` values are now converted from strings into temporal types in the generated Cypher instead of in server code using the driver. This could result in different values when the database is in a different timezone to the GraphQL server.

packages/graphql/src/graphql/scalars/DateTime.ts

@@ -19,7 +19,8 @@
 
 import type { ValueNode } from "graphql";
 import { GraphQLError, GraphQLScalarType, Kind } from "graphql";
-import neo4j, { isDateTime } from "neo4j-driver";
+import type neo4j from "neo4j-driver";
+import { isDateTime } from "neo4j-driver";
 
 export const GraphQLDateTime = new GraphQLScalarType({
     name: "DateTime",
@@ -43,10 +44,6 @@ export const GraphQLDateTime = new GraphQLScalarType({
                 throw new GraphQLError(`DateTime cannot represent non temporal value: ${inputValue}`);
             }
 
-            return neo4j.types.DateTime.fromStandardDate(date);
-        }
-
-        if (isDateTime(inputValue)) {
             return inputValue;
         }
 
@@ -57,6 +54,6 @@ export const GraphQLDateTime = new GraphQLScalarType({
             throw new GraphQLError("DateTime cannot represent non string value.");
         }
 
-        return neo4j.types.DateTime.fromStandardDate(new Date(ast.value));
+        return ast.value;
     },
 });

packages/graphql/src/graphql/scalars/Time.ts

@@ -19,30 +19,12 @@
 
 import type { ValueNode } from "graphql";
 import { GraphQLError, GraphQLScalarType, Kind } from "graphql";
-import neo4j, { isTime } from "neo4j-driver";
+import neo4j from "neo4j-driver";
 
 export const TIME_REGEX =
     /^(?<hour>[01]\d|2[0-3]):(?<minute>[0-5]\d)(:(?<second>[0-5]\d)(\.(?<fraction>\d{1}(?:\d{0,8})))?((?:[Zz])|((?<offsetDirection>[-|+])(?<offsetHour>[01]\d|2[0-3]):(?<offsetMinute>[0-5]\d)))?)?$/;
 
-type TimeRegexMatchGroups = {
-    hour: string;
-    minute: string;
-    second: string;
-    fraction: string;
-    offsetDirection: string;
-    offsetHour: string;
-    offsetMinute: string;
-};
-
-type ParsedTime = {
-    hour: number;
-    minute: number;
-    second: number;
-    nanosecond: number;
-    timeZoneOffsetSeconds: number;
-};
-
-export const parseTime = (value: unknown): ParsedTime => {
+export const validateTime = (value: unknown): string => {
     if (typeof value !== "string") {
         throw new TypeError(`Value must be of type string: ${value}`);
     }
@@ -53,39 +35,7 @@ export const parseTime = (value: unknown): ParsedTime => {
         throw new TypeError(`Value must be formatted as Time: ${value}`);
     }
 
-    const { hour, minute, second, fraction, offsetDirection, offsetHour, offsetMinute } =
-        match.groups as TimeRegexMatchGroups;
-    // Calculate the number of nanoseconds by padding the fraction of seconds with zeroes to nine digits
-    let nanosecond = 0;
-    if (fraction) {
-        nanosecond = +`${fraction}000000000`.substring(0, 9);
-    }
-
-    // Calculate the timeZoneOffsetSeconds by calculating the offset in seconds with the appropriate sign
-    let timeZoneOffsetSeconds = 0;
-    if (offsetDirection && offsetHour && offsetMinute) {
-        const offsetInMinutes = +offsetMinute + +offsetHour * 60;
-        const offsetInSeconds = offsetInMinutes * 60;
-        timeZoneOffsetSeconds = +`${offsetDirection}${offsetInSeconds}`;
-    }
-
-    return {
-        hour: +hour,
-        minute: +minute,
-        second: +(second || 0),
-        nanosecond,
-        timeZoneOffsetSeconds,
-    };
-};
-
-const parse = (value: unknown) => {
-    if (isTime(value)) {
-        return value;
-    }
-
-    const { hour, minute, second, nanosecond, timeZoneOffsetSeconds } = parseTime(value);
-
-    return new neo4j.types.Time(hour, minute, second, nanosecond, timeZoneOffsetSeconds);
+    return value;
 };
 
 export const GraphQLTime = new GraphQLScalarType({
@@ -105,12 +55,12 @@ export const GraphQLTime = new GraphQLScalarType({
         return stringifiedValue;
     },
     parseValue: (value: unknown) => {
-        return parse(value);
+        return validateTime(value);
     },
     parseLiteral: (ast: ValueNode) => {
         if (ast.kind !== Kind.STRING) {
             throw new GraphQLError(`Only strings can be validated as Time, but received: ${ast.kind}`);
         }
-        return parse(ast.value);
+        return validateTime(ast.value);
     },
 });

packages/graphql/src/schema/validation/custom-rules/directives/default.ts

@@ -21,7 +21,7 @@ import { Kind } from "graphql";
 import { GRAPHQL_BUILTIN_SCALAR_TYPES } from "../../../../constants";
 import { GraphQLDate, GraphQLDateTime, GraphQLLocalDateTime } from "../../../../graphql/scalars";
 import { GraphQLLocalTime, parseLocalTime } from "../../../../graphql/scalars/LocalTime";
-import { GraphQLTime, parseTime } from "../../../../graphql/scalars/Time";
+import { GraphQLTime, validateTime } from "../../../../graphql/scalars/Time";
 import { DocumentValidationError } from "../utils/document-validation-error";
 import type { ObjectOrInterfaceWithExtensions } from "../utils/path-parser";
 import { assertArgumentHasSameTypeAsField } from "../utils/same-type-argument-as-field";
@@ -60,7 +60,7 @@ export function verifyDefault(enums: EnumTypeDefinitionNode[]) {
                 }
             } else if (expectedType === GraphQLTime.name) {
                 try {
-                    parseTime((defaultArg?.value as StringValueNode).value);
+                    validateTime((defaultArg?.value as StringValueNode).value);
                 } catch {
                     throw new DocumentValidationError(
                         `@default.${defaultArg.name.value} is not a valid ${expectedType}`,

packages/graphql/src/translate/create-create-and-params.ts

@@ -77,6 +77,7 @@ function createCreateAndParams({
         const relationField = node.relationFields.find((x) => key === x.fieldName);
         const primitiveField = node.primitiveFields.find((x) => key === x.fieldName);
         const pointField = node.pointFields.find((x) => key === x.fieldName);
+        const temporalField = node.temporalFields.find((x) => key === x.fieldName);
         const dbFieldName = mapToDbProperty(node, key);
 
         if (primitiveField) {
@@ -259,6 +260,22 @@ function createCreateAndParams({
             return res;
         }
 
+        if (temporalField && ["DateTime", "Time"].includes(temporalField.typeMeta.name)) {
+            if (temporalField.typeMeta.array) {
+                res.creates.push(
+                    `SET ${varName}.${dbFieldName} = [t in $${varNameKey} | ${temporalField.typeMeta.name.toLowerCase()}(t)]`
+                );
+            } else {
+                res.creates.push(
+                    `SET ${varName}.${dbFieldName} = ${temporalField.typeMeta.name.toLowerCase()}($${varNameKey})`
+                );
+            }
+
+            res.params[varNameKey] = value;
+
+            return res;
+        }
+
         res.creates.push(`SET ${varName}.${dbFieldName} = $${varNameKey}`);
         res.params[varNameKey] = value;
 

packages/graphql/src/translate/queryAST/ast/filters/aggregation/AggregationDateTimePropertyFilter.ts

@@ -0,0 +1,50 @@
+/*
+ * Copyright (c) "Neo4j"
+ * Neo4j Sweden AB [http://neo4j.com]
+ *
+ * This file is part of Neo4j.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import Cypher from "@neo4j/cypher-builder";
+import type { AggregationLogicalOperator } from "../../../factory/parsers/parse-where-field";
+import { AggregationPropertyFilter } from "./AggregationPropertyFilter";
+
+export class AggregationDateTimeFilter extends AggregationPropertyFilter {
+    protected getOperation(expr: Cypher.Expr): Cypher.ComparisonOp {
+        return this.createDateTimeOperation({
+            operator: this.logicalOperator,
+            property: expr,
+            param: new Cypher.Param(this.comparisonValue),
+        });
+    }
+
+    private createDateTimeOperation({
+        operator,
+        property,
+        param,
+    }: {
+        operator: AggregationLogicalOperator;
+        property: Cypher.Expr;
+        param: Cypher.Expr;
+    }) {
+        const variable = Cypher.datetime(param);
+
+        return this.createBaseOperation({
+            operator,
+            property,
+            param: variable,
+        });
+    }
+}

packages/graphql/src/translate/queryAST/ast/filters/aggregation/AggregationDurationPropertyFilter.ts

@@ -18,8 +18,8 @@
  */
 
 import Cypher from "@neo4j/cypher-builder";
-import { AggregationPropertyFilter } from "./AggregationPropertyFilter";
 import type { AggregationLogicalOperator } from "../../../factory/parsers/parse-where-field";
+import { AggregationPropertyFilter } from "./AggregationPropertyFilter";
 
 export class AggregationDurationFilter extends AggregationPropertyFilter {
     protected getOperation(expr: Cypher.Expr): Cypher.ComparisonOp {

packages/graphql/src/translate/queryAST/ast/filters/aggregation/AggregationTimePropertyFilter.ts

@@ -0,0 +1,50 @@
+/*
+ * Copyright (c) "Neo4j"
+ * Neo4j Sweden AB [http://neo4j.com]
+ *
+ * This file is part of Neo4j.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import Cypher from "@neo4j/cypher-builder";
+import type { AggregationLogicalOperator } from "../../../factory/parsers/parse-where-field";
+import { AggregationPropertyFilter } from "./AggregationPropertyFilter";
+
+export class AggregationTimeFilter extends AggregationPropertyFilter {
+    protected getOperation(expr: Cypher.Expr): Cypher.ComparisonOp {
+        return this.createTimeOperation({
+            operator: this.logicalOperator,
+            property: expr,
+            param: new Cypher.Param(this.comparisonValue),
+        });
+    }
+
+    private createTimeOperation({
+        operator,
+        property,
+        param,
+    }: {
+        operator: AggregationLogicalOperator;
+        property: Cypher.Expr;
+        param: Cypher.Expr;
+    }) {
+        const variable = Cypher.time(param);
+
+        return this.createBaseOperation({
+            operator,
+            property,
+            param: variable,
+        });
+    }
+}

packages/graphql/src/translate/queryAST/ast/filters/property-filters/CypherFilter.ts

@@ -26,8 +26,10 @@ import type { CustomCypherSelection } from "../../selection/CustomCypherSelectio
 import type { FilterOperator } from "../Filter";
 import { Filter } from "../Filter";
 import { coalesceValueIfNeeded } from "../utils/coalesce-if-needed";
+import { createDateTimeOperation } from "../utils/create-date-time-operation";
 import { createDurationOperation } from "../utils/create-duration-operation";
 import { createPointOperation } from "../utils/create-point-operation";
+import { createTimeOperation } from "../utils/create-time-operation";
 
 /** A property which comparison has already been parsed into a Param */
 export class CypherFilter extends Filter {
@@ -127,6 +129,24 @@ export class CypherFilter extends Filter {
             });
         }
 
+        if (this.attribute.typeHelper.isDateTime()) {
+            return createDateTimeOperation({
+                operator,
+                property: coalesceProperty,
+                param: this.comparisonValue,
+                attribute: this.attribute,
+            });
+        }
+
+        if (this.attribute.typeHelper.isTime()) {
+            return createTimeOperation({
+                operator,
+                property: coalesceProperty,
+                param: this.comparisonValue,
+                attribute: this.attribute,
+            });
+        }
+
         return createComparisonOperation({ operator, property: coalesceProperty, param });
     }
 }