Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TimeField , DateTimeField and DateField cannot be deserialised from Json #16203

Closed
hyzx86 opened this issue May 31, 2024 · 10 comments · Fixed by #16205
Closed

TimeField , DateTimeField and DateField cannot be deserialised from Json #16203

hyzx86 opened this issue May 31, 2024 · 10 comments · Fixed by #16205
Labels
Milestone

Comments

@hyzx86
Copy link
Contributor

hyzx86 commented May 31, 2024

Describe the bug

I found this in my custom code, but if someone has enabled the OrchardCore.ContentFields.Indexing.SQL feature on OC 2.0 and is using TimeField they will run into this problem!

To Reproduce

     [Fact]
    public void TimeSpanFiledTests()
    {
        var jsonStr = """
            {
                "Value":"13:05"
            }
            """;
        var jobject = JsonNode.Parse(jsonStr);
        var timeFiled = jobject.ToObject<TimeField>();

        Assert.Equal("13:05", timeFiled.Value.ToString());
    }
IContentHandler thrown from OrchardCore.ContentManagement.Handlers.ContentPartHandlerCoordinator by JsonException System.Text.Json.JsonException: The JSON value could not be converted to System.Nullable`1[System.TimeSpan]. Path: $.Value | LineNumber: 0 | BytePositionInLine: 16.
 ---> System.FormatException: The JSON value is not in a supported TimeSpan format.
   at System.Text.Json.ThrowHelper.ThrowFormatException(DataType dataType)
   at System.Text.Json.Serialization.Converters.TimeSpanConverter.ReadCore(Utf8JsonReader& reader)
   at System.Text.Json.Serialization.Converters.NullableConverter`1.Read(Utf8JsonReader& reader, Type typeToConvert, JsonSerializerOptions options)
   at System.Text.Json.Serialization.Metadata.JsonPropertyInfo`1.ReadJsonAndSetMember(Object obj, ReadStack& state, Utf8JsonReader& reader)
   at System.Text.Json.Serialization.Converters.ObjectDefaultConverter`1.OnTryRead(Utf8JsonReader& reader, Type typeToConvert, JsonSerializerOptions options, ReadStack& state, T& value)
   at System.Text.Json.Serialization.JsonConverter`1.TryRead(Utf8JsonReader& reader, Type typeToConvert, JsonSerializerOptions options, ReadStack& state, T& value, Boolean& isPopulatedValue)
   at System.Text.Json.Serialization.JsonConverter`1.ReadCore(Utf8JsonReader& reader, JsonSerializerOptions options, ReadStack& state)
   --- End of inner exception stack trace ---
   at System.Text.Json.ThrowHelper.ReThrowWithPath(ReadStack& state, Utf8JsonReader& reader, Exception ex)
   at System.Text.Json.Serialization.JsonConverter`1.ReadCore(Utf8JsonReader& reader, JsonSerializerOptions options, ReadStack& state)
   at System.Text.Json.Serialization.Metadata.JsonTypeInfo`1.DeserializeAsObject(Utf8JsonReader& reader, ReadStack& state)
   at System.Text.Json.JsonSerializer.ReadFromSpanAsObject(ReadOnlySpan`1 utf8Json, JsonTypeInfo jsonTypeInfo, Nullable`1 actualByteCount)
   at System.Text.Json.JsonSerializer.ReadFromNodeAsObject(JsonNode node, JsonTypeInfo jsonTypeInfo)
   at OrchardCore.ContentManagement.ContentExtensions.Get(ContentElement contentElement, Type contentElementType, String name)
   at OrchardCore.ContentManagement.Handlers.ContentPartHandlerCoordinator.InvokeHandlers[TContext,TFieldContext](TContext context, TFieldContext fieldContext, Func`4 partHandlerAction, Func`4 fieldHandlerAction, Boolean createPartIfNotExists, Boolean createFieldIfNotExists)
   at OrchardCore.Modules.InvokeExtensions.InvokeAsync[TEvents,T1](IEnumerable`1 events, Func`3 dispatch, T1 arg1, ILogger logger)    at System.Text.Json.ThrowHelper.ReThrowWithPath(ReadStack& state, Utf8JsonReader& reader, Exception ex)
   at System.Text.Json.Serialization.JsonConverter`1.ReadCore(Utf8JsonReader& reader, JsonSerializerOptions options, ReadStack& state)
   at System.Text.Json.Serialization.Metadata.JsonTypeInfo`1.DeserializeAsObject(Utf8JsonReader& reader, ReadStack& state)
   at System.Text.Json.JsonSerializer.ReadFromSpanAsObject(ReadOnlySpan`1 utf8Json, JsonTypeInfo jsonTypeInfo, Nullable`1 actualByteCount)
   at System.Text.Json.JsonSerializer.ReadFromNodeAsObject(JsonNode node, JsonTypeInfo jsonTypeInfo)
   at OrchardCore.ContentManagement.ContentExtensions.Get(ContentElement contentElement, Type contentElementType, String name)
   at OrchardCore.ContentManagement.Handlers.ContentPartHandlerCoordinator.InvokeHandlers[TContext,TFieldContext](TContext context, TFieldContext fieldContext, Func`4 partHandlerAction, Func`4 fieldHandlerAction, Boolean createPartIfNotExists, Boolean createFieldIfNotExists)
   at OrchardCore.Modules.InvokeExtensions.InvokeAsync[TEvents,T1](IEnumerable`1 events, Func`3 dispatch, T1 arg1, ILogger logger)
@Piedone
Copy link
Member

Piedone commented May 31, 2024

Thanks for the test!

How exactly do you reproduce this with Field Indexing?

@hyzx86
Copy link
Contributor Author

hyzx86 commented May 31, 2024

Because this code will call the ToObject function

return fieldDefinitions
.GetContentFields<TimeField>(contentItem)
.Select(pair =>
new TimeFieldIndex
{
Latest = contentItem.Latest,
Published = contentItem.Published,
ContentItemId = contentItem.ContentItemId,
ContentItemVersionId = contentItem.ContentItemVersionId,
ContentType = contentItem.ContentType,
ContentPart = pair.Definition.ContentTypePartDefinition.Name,
ContentField = pair.Definition.Name,
Time = pair.Field.Value,
});

if (((JsonObject)contentItem.Content)[fieldDefinition.ContentTypePartDefinition.Name] is not JsonObject jPart ||
jPart[fieldDefinition.Name] is not JsonObject jField)
{
return null;
}
return jField.ToObject<TField>();

@Piedone
Copy link
Member

Piedone commented May 31, 2024

I mean, how do you use Field Indexing that brings this bug out?

@hyzx86
Copy link
Contributor Author

hyzx86 commented May 31, 2024

I mean, how do you use Field Indexing that brings this bug out?

I've created a recipe that, if you import it, you'll see detailed error messages in the logs, and I'm going to add it to the unit test

{
  "name": "dynamicIndexTest",
  "displayName": "dynamicIndexTests",
  "description": "Test dynamicIndex",
  "author": "Tony Han",
  "website": "",
  "version": "1.0.0",
  "issetuprecipe": false,
  "categories": [ "test" ],
  "tags": [],
  "steps": [
    {
      "name": "Feature",
      "enable": [ "OrchardCore.ContentFields.Indexing.SQL" ]
    },
    {
      "name": "ContentDefinition",
      "ContentTypes": [
        {
          "Name": "Message",
          "DisplayName": "Message",
          "Settings": {
            "ContentTypeSettings": {
              "Creatable": true,
              "Draftable": true,
              "Versionable": true,
              "Listable": true,
              "Securable": true
            }
          },
          "ContentTypePartDefinitionRecords": [
            {
              "PartName": "Message",
              "Name": "Message",
              "Settings": {
                "ContentTypePartSettings": {
                  "Position": "5"
                }
              }
            },
            {
              "PartName": "DIndexPart",
              "Name": "DIndexPart"
            }
          ]
        }
      ],
      "ContentParts": [
        {
          "Name": "Message",
          "Settings": {},
          "ContentPartFieldDefinitionRecords": [
            {
              "FieldName": "TextField",
              "Name": "TextFieldTest"
            },
            {
              "FieldName": "MultiTextField",
              "Name": "MultiTextFieldTest"
            },
            {
              "FieldName": "HtmlField",
              "Name": "HtmlFieldTest"
            },
            {
              "FieldName": "MediaField",
              "Name": "MediaFieldTest"
            },
            {
              "FieldName": "BooleanField",
              "Name": "BooleanFieldTest"
            },
            {
              "FieldName": "TimeField",
              "Name": "TimeFieldTest"
            },
            {
              "FieldName": "DateTimeField",
              "Name": "DateTimeFieldTest"
            },
            {
              "FieldName": "DateField",
              "Name": "DateFieldTest"
            },
            {
              "FieldName": "NumericField",
              "Name": "NumericFieldTest"
            },
            {
              "FieldName": "ContentPickerField",
              "Name": "ContentPickerFieldTest"
            },
            {
              "FieldName": "UserPickerField",
              "Name": "UserPickerFieldTest"
            }

          ]
        }
      ]
    },
    {
      "name": "content",
      "data": [
        {
          "ContentItemId": "testId1",
          "ContentType": "Message",
          "DisplayText": "Message",
          "Latest": true,
          "Published": true, 
          "Owner": "[js: parameters('AdminUserId')]",
          "Author": "[js: parameters('AdminUsername')]",
          "Message": {
            "TextFieldTest": { "Text": "TextFieldTestValue" },
            "MultiTextFieldTest": { "Values": [ "MultiTextFieldValue1", "MultiTextFieldValue2" ] },
            "HtmlFieldTest": { "Html": "HtmlFieldTestValue" },
            "MediaFieldTest": { "Paths": [ "Path1", "Path2" ] },
            "BooleanFieldTest": { "Value": true },
            "TimeFieldTest": { "Value": "12:05" },
            "DateTimeFieldTest": { "Value": "2024-5-5 22:50:58" },
            "DateFieldTest": { "Value": "2024-5-5" },
            "NumericFieldTest": { "Value": 123 },
            "ContentPickerFieldTest": { "ContentItemIds": [ "testContentItemId" ] },
            "UserPickerFieldTest": { "UserIds": [ "TestUserID" ] }
          },
          "TitlePart": {
            "Title": "[locale en]You have successfully registered![/locale][locale fr]Vous vous êtes inscrit avec succès![/locale]"
          }
        }
      ]
    }
  ]
}

@hyzx86
Copy link
Contributor Author

hyzx86 commented May 31, 2024

STJ only supports 13:05:00 , but NSJ also supports 13:05 ~~ Maybe it can't be considered as a bug.

@Piedone Piedone added this to the 2.0 milestone May 31, 2024
@Piedone Piedone changed the title TimeField cannot be deserialised from Json. TimeField cannot be deserialised from Json May 31, 2024
Copy link
Contributor

We triaged this issue and set the milestone according to the priority we think is appropriate (see the docs on how we triage and prioritize issues).

This indicates when the core team may start working on it. However, if you'd like to contribute, we'd warmly welcome you to do that anytime. See our guide on contributions here.

@Piedone
Copy link
Member

Piedone commented May 31, 2024

Thanks for the details! All clear now.

Since this breaks existing installations that use TimeField (right?), it's a bug. I suggest two things:

  • Foremost, see if you can get STJ to deserialize this with some serializer settings. If yes, then all good.
  • If not, or perhaps additionally, change TimeField to also save the seconds (as 00).

Please let me know once the PR is ready.

@hyzx86
Copy link
Contributor Author

hyzx86 commented May 31, 2024

It looks like there is a similar problem with DateField and DateTimeField . It looks like there's more formatting requirements in STJ.

    System.Text.Json.JsonException : The JSON value could not be converted to System.Nullable`1[System.DateTime]. Path: $.Value | LineNumber: 0 | BytePositionInLine: 26.
    ---- System.FormatException : The JSON value is not in a supported DateTime format.

@hyzx86 hyzx86 changed the title TimeField cannot be deserialised from Json TimeField , DateTimeField and DateField cannot be deserialised from Json May 31, 2024
@sebastienros
Copy link
Member

So it's not related to SQL Indexing, right? It's just that indexing triggered the code path that was deserializing the document to a TimeField, but this should be reproduced without it?

Could you share what JSON is saved with 1.8.3 (NSJ) in a document when using such fields? Just to confirm that it's NSJ that created this format, and that the DateTime format it used is also incompatible with STJ. This might be related to the culture when it saved the valus with NSJ.

@hyzx86
Copy link
Contributor Author

hyzx86 commented Jun 5, 2024

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants