HC14 Strict RFC 3339 DateTime parsing should be opt-in/out - able?

[randyridge]

Product

Hot Chocolate

Is your feature request related to a problem?

In updating from HC 13 -> 14 we encountered a runtime behavior change, with respect to dates, I see that this is documented explicitly in the migration notes and I agree that having a common datetime spec is beneficial and rfc 3339 is a reasonable choice. I also realize that 13->14 is a major semver and could be breaking. All that said, I have clients sending me dates that are not strictly compatible, and while I agree they're not doing the correct thing and can and should be fixed, it's unfortunate this new spec enforcement is coupled directly with upgrading to 14.

My proposal would be to allow the ability to opt-in to the legacy datetime parser similar to how I can do the same for the legacy node ID serializer. It gives me time to migrate clients while the server can go from 13 to 14. For dates though, I have published clients, who from their point of view are sending the same query to the same schema and are now broken just for me having updated the server from 13 to 14. I need some way to decouple the 13 -> 14 upgrade from the strict enforcement of the datetime spec the same way I can decouple the upgrade from the new ID serializer.

I'm left with few options, either the client has to change, our clients go through app approval processes on various platforms, which again I can't always force (some platforms won't force an update even if one is available), especially at an exact time (like when the hc14 server starts taking traffic), or I have to upgrade and deploy elsewhere so new clients that are sending rfc compatible dates can hit an hc 14 instance (without a schema change) and those that haven't updated can still point to the hc13 version (with the same schema) and run dual processes for some period of time.

Specifically the error was:
DateTime cannot parse the given literal of type StringValueNode

For this date (missing seconds)
"2024-08-23T00:00-04:00"

The solution you'd like

allow the ability to opt-in to the legacy datetime parser

[randyridge]

I was able to work around this by modifying your private static scalar lookups using just the right amount of unholy reflection and replacing your datetime scalar with a child of your datetime scalar that doesn't call the DateTimeScalarRegex. This is a less-than-ideal solution.

[michaelstaib]

Why dont you use BindRuntimeType?

[magahl]

Is there a way to replace the DateTime scalar type that HotChcoalte register? BindRuntimeType adds a new scalar type. That cannot have the same name. This makes client generation harder down the line.

[glen-84]

builder.Services
    .AddGraphQLServer()
    .BindRuntimeType<DateTime, CustomDateTimeType>()
    .AddTypes();

CustomDateTimeType is a copy of DateTimeType, with the format check removed.

[magahl]

Yeah but that would end up with me having two types of scalars for date time?

Thats what i meant with problems generating clients down the line. Since they cant both have the name DateTime. It will throw an exception on startup. Key already exist. Thats why i wanted to remove the old one.

[glen-84]

I don't see that.

[willgittoes]

Same as magahl, I get this error when trying to do the workaround:

An item with the same key has already been added. Key: DateTime
at HotChocolate.Configuration.TypeInitializer.DiscoverTypes()
   at HotChocolate.Configuration.TypeInitializer.Initialize()
   at HotChocolate.SchemaBuilder.Setup.InitializeTypes(SchemaBuilder builder, IDescriptorContext context, IReadOnlyList`1 types)
   at HotChocolate.SchemaBuilder.Setup.Create(SchemaBuilder builder, LazySchema lazySchema, IDescriptorContext context)
   at HotChocolate.SchemaBuilder.Create(IDescriptorContext context)

[willgittoes]

I was able to work around this by modifying your private static scalar lookups using just the right amount of unholy reflection and replacing your datetime scalar with a child of your datetime scalar that doesn't call the DateTimeScalarRegex. This is a less-than-ideal solution.

Hey @randyridge could you tell me how you did this? My approach was to try and modify the static regex in DateTimeType, but newer versions of dotnet won't let me be that naughty (because it's static readonly and the class is initialised before I can get to it)

[randyridge]

In our startup something like so:

var lookupFieldValue = typeof(Scalars).GetField("_lookup", BindingFlags.NonPublic | BindingFlags.Static)?.GetValue(null);
if(lookupFieldValue == null) {
	throw new InvalidOperationException("Could not find _lookup field on Scalars type.");
}

var lookup = (Dictionary<Type, Type>) lookupFieldValue;
lookup.Remove(typeof(DateTime));
lookup.Remove(typeof(DateTimeOffset));
lookup.Add(typeof(DateTime), typeof(LooseyGooseyDateTimeType));
lookup.Add(typeof(DateTimeOffset), typeof(LooseyGooseyDateTimeType));

var nameLookupFieldValue = typeof(Scalars).GetField("_nameLookup", BindingFlags.NonPublic | BindingFlags.Static)?.GetValue(null);
if(nameLookupFieldValue == null) {
	throw new InvalidOperationException("Could not find _nameLookup field on Scalars type.");
}

var nameLookup = (Dictionary<string, Type>) nameLookupFieldValue;
nameLookup[ScalarNames.DateTime] = typeof(LooseyGooseyDateTimeType);

and then...

using System;
using System.Globalization;
using HotChocolate.Language;
using HotChocolate.Types;

public class LooseyGooseyDateTimeType : DateTimeType {
	public LooseyGooseyDateTimeType() : base(ScalarNames.DateTime) {
	}

	public override bool TryDeserialize(object? resultValue, out object? runtimeValue) {
		switch(resultValue) {
			case null:
				runtimeValue = null;
				return true;
			case string s when TryDeserializeFromString(s, out var d):
				runtimeValue = d;
				return true;
			case DateTimeOffset dto:
				runtimeValue = dto;
				return true;
			case DateTime dt:
				runtimeValue = new DateTimeOffset(dt.ToUniversalTime(), TimeSpan.Zero);
				return true;
			default:
				runtimeValue = null;
				return false;
		}
	}

	protected override DateTimeOffset ParseLiteral(StringValueNode valueSyntax) {
		if(TryDeserializeFromString(valueSyntax.Value, out var value)) {
			return value!.Value;
		}

		throw new SerializationException("LooseyGooseyDateTimeType: Cannot parse literal.", this);
	}

	private static bool TryDeserializeFromString(string? serialized, out DateTimeOffset? value) {
		// No "Unknown Local Offset Convention" - https://www.graphql-scalars.com/date-time/#no-unknown-local-offset-convention
		if(string.IsNullOrEmpty(serialized) || serialized.EndsWith("-00:00")) {
			value = null;
			return false;
		}

		if(serialized.EndsWith('Z') && DateTime.TryParse(serialized, CultureInfo.InvariantCulture, DateTimeStyles.AssumeUniversal, out var zuluTime)) {
			value = new DateTimeOffset(zuluTime.ToUniversalTime(), TimeSpan.Zero);
			return true;
		}

		if(DateTimeOffset.TryParse(serialized, out var dt)) {
			value = dt;
			return true;
		}

		value = null;
		return false;
	}
}

ymmv

[willgittoes]

@randyridge thanks for that, I figured all that out but what I also needed to do was replace the references to it in the filtering. Because we have .UseFiltering() there are some filters that reference the original DateTimeType.

Here's how I did that:

...
            .AddFiltering<ReplaceDateTimeStrictParsingHackConvention>()

public class ReplaceDateTimeStrictParsingHackConvention : FilterConvention
{
    private static int _lookupPatched = 0;

    protected override void Configure(IFilterConventionDescriptor descriptor)
    {
        // Use reflection to patch in our relaxed DateTime scalar type, gross I know.
        // Only do this once, lock-free so the tests are a bit faster.
        if (Interlocked.CompareExchange(ref _lookupPatched, 1, 0) == 0)
        {
            var type = typeof(HotChocolate.Types.Scalars);
            var lookupField = type.GetField("_lookup", BindingFlags.NonPublic | BindingFlags.Static);
            var lookupVal = lookupField?.GetValue(null) as Dictionary<Type, Type>;
            lookupVal!.Remove(typeof(DateTime));
            lookupVal!.Remove(typeof(DateTimeOffset));
            lookupVal![typeof(DateTime)] = typeof(RelaxedParsingDateTimeType);
            lookupVal![typeof(DateTimeOffset)] = typeof(RelaxedParsingDateTimeType);

            var nameField = type.GetField("_nameLookup", BindingFlags.NonPublic | BindingFlags.Static);
            var nameVal = nameField?.GetValue(null) as Dictionary<string, Type>;
            nameVal!.Remove(ScalarNames.DateTime);
            nameVal![ScalarNames.DateTime] = typeof(RelaxedParsingDateTimeType);
        }

        // Replace other references to the strict datetime type.
        descriptor.AddDefaults();
        descriptor.Operation(-420).Name("doesNotAppearInSchema");
        descriptor
            .BindRuntimeType<DateTime, RelaxedParsingDateTimeOperationFilterInputType>()
            .BindRuntimeType<DateTime?, RelaxedParsingDateTimeOperationFilterInputType>()
            .BindRuntimeType<DateTimeOffset, RelaxedParsingDateTimeOperationFilterInputType>()
            .BindRuntimeType<DateTimeOffset?, RelaxedParsingDateTimeOperationFilterInputType>();
    }
}