SimpleFieldStorage doesn't handle Nullable<DateTime> correctly

[orchardbot]

flew2bits created: https://orchard.codeplex.com/workitem/20247

I'm trying to use the Line20.DateTimeRange module and have found that I can add the field to content types, but it doesn't save the values correctly. The field contains two Nullable properties. In SimpleFieldStorage, the Set method handles DateTime objects as a special case, but doesn't deal with Nullable. On retrieving the field values, the Get method ultimately fails with an exception because the value that's stored is not in the expected format. For some reason, the DateTimeRange field retrieves the value from the database as Get(...), so Get handles the value as a special case -- I think this needs to be fixed in the DateTimeRange module, but I don't believe it will work properly until the SimpleFieldStorage issue is fixed.

For the setter, I propose the additional clause in the if statement in Set: if (typeof(T) == typeof(DateTime)) { var text = ((DateTime)(object)value).ToString("o", CultureInfo.InvariantCulture); Setter(name, typeof(T), text); } // ADDED CLAUSE else if (typeof(T) == typeof(Nullable) && ((Nullable)(object)value).HasValue) { var text = (((Nullable)(object)value).Value).ToString("o", CultureInfo.InvariantCulture); Setter(name, typeof(DateTime), text); } // END OF ADDED CLAUSE else { Setter(name, typeof(T), Convert.ToString(value, CultureInfo.InvariantCulture)); }

I haven't worked out yet what needs to be changed in the Get method yet since right now it works as is, but I have a feeling that if the stored DateTime? value is null, it likely throws an exception on trying to retrieve a DateTime value.

[orchardbot]

flew2bits commented:

It looks like changing the line

if (typeof(T) == typeof(DateTime)) {

to

if (t == typeof(DateTime)) {

will make the Get method handle Nullable correctly.

[orchardbot]

@bleroy commented:

I'm confused: where is that code? In Orchard or in that Line20.DateTimeRange module?

[orchardbot]

AimOrchard commented:

In SimpleFieldStorage.

Original bits of code (from 1.6):

        // using a special case for DateTime as it would lose milliseconds otherwise
        if (typeof(T) == typeof(DateTime)) {
            var result = XmlConvert.ToDateTime(value, XmlDateTimeSerializationMode.Utc);
            return (T) (object)result;
        }

[orchardbot]

flew2bits commented:

I'm looking at 1.7.1 now, and it's in lines 29 (for Get) and 37-48 (for Set) of Orchard.ContentManagement.FieldStorage.SimpleFieldStorage

[orchardbot]

flew2bits commented:

Line20.DateRangeRange was just where I was seeing symptoms. If it's added to a content type, you can set values in it when creating a new content item, but if you try to edit that item, the editor for the field just doesn't show. It's because an exception is occurring in retrieving the field data since the formatting is not as expected due to Set not handling the Nullable.

[orchardbot]

flew2bits commented:

From what I can tell Line20.DateTimeRange was written for Orchard 1.4.x which doesn't have code to specifically handle DateTime values in SimpleFieldStorage, so it most likely worked just fine in 1.4.

[orchardbot]

@bleroy commented:

Thanks.

[orchardbot]

@sebastienros commented:

Checked, the code is wrong, as explained.