Fix dynamic field value issues

[hyzx86]

fixes #16290
/cc @gvkries

[gvkries]

This approach is not feasible IMHO, because you loose the dynamic behavior of JsonDynamicValue all together by returning just string or number. We should find a different way.

[hyzx86]

Maybe refer to Nullable, this experiment didn't work for me~

[hyzx86]

This approach is not feasible IMHO, because you loose the dynamic behavior of JsonDynamicValue all together by returning just string or number. We should find a different way.

I remember that under version 1.8, ContentItem.Content.PartName.FieldName.Value doesn't support set value, only can get value.

[gvkries]

I think your problem is not in the dynamic wrappers itself, but the fact that they are serialized as well. In contrast to NSJ we now have a layer more, to support the dynamic behavior NSJ has by default. But when you serialize e.g. a JsonDynamicValue, you don't get the value only, like in NSJ, but the wrapper as well.
Maybe the solution is to add another JsonConverter for these dynamic wrappers, which will skip them and serialize the containing JSON only. I mean something like:

public override void Write(
    Utf8JsonWriter writer,
    object objectToWrite,
    JsonSerializerOptions options)
{
    if (objectToWrite is JsonDynamicValue value && value.JsonValue != null)
    {
        value.JsonValue.WriteTo(writer, options);
    }
    else
    {
        JsonSerializer.Serialize(writer, objectToWrite, objectToWrite.GetType(), options);
    }

[hyzx86]

Like the following unit test, actually in version 1.8 we can use contentItem2.Content.MyPart.myField.Value.FullYear directly in the js if I remember correctly.

It looks like if we have to use STJ in 2.0, we'll have to give up a few things.

            var contentItem2 = JConvert.DeserializeObject<ContentItem>(json);
            Assert.Equal(new DateTime(2024, 1, 1, 10, 42, 0, DateTimeKind.Utc), (DateTime?)contentItem2.Content.MyPart.myField.Value);

[hyzx86]

I've added some type conversion tests here, If this solution is acceptable, perhaps we need to add some abstraction converters for some custom development scenarios

[hyzx86]

If I remember correctly the only two types of fields that currently take values from strings in OC seem to be these two types, which are always wrapped in Value

[gvkries]

If I understand your approach correctly, you are moving parts of the value conversions from JsonDynamicValue to JsonDynamicObject. This is different from the current design, where we have the separation of value functionality and Json objects + arrays. One can argue that the JsonDynamicValue is not necessary, but I would not change that at this stage.

In summary, I strongly suggest to keep the current separation of value and object. Don't add conversion functionality into the JsonDynamicObject class at all.

[hyzx86]

Added a static collection to handle dynamic values for custom development environments

[hyzx86]

Custom fetch logic can be added here

[hyzx86]

If I understand your approach correctly, you are moving parts of the value conversions from JsonDynamicValue to JsonDynamicObject. This is different from the current design, where we have the separation of value functionality and Json objects + arrays. One can argue that the JsonDynamicValue is not necessary, but I would not change that at this stage.

Okay, if we have to do that, it's not impossible, we may have to pass the MemberName to the JsonDynamicValue object Mainly as I mentioned above, we can't rely on the code alone to detect the value in a way that determines the target type
#16294 (comment)

[gvkries]

Okay, if we have to do that, it's not impossible, we may have to pass the MemberName to the JsonDynamicValue object Mainly as I mentioned above, we can't rely on the code alone to detect the value in a way that determines the target type #16294 (comment)

I don't understand exactly what you mean by that, but I think we don't have to do a lot beside #16292. I'll add custom JsonConverter for the dynamic wrappers, to ensure they will not be serialized. This will almost be what you want. E.g. your test will pass after those changes:

[Fact]
    public void TestDynamicValueAssignToOther()
    {
        var contentItem = new ContentItem();
        contentItem.Alter<TestPart>(part =>
        {
            part.TextFieldProp = new TextField { Text = "test" };
            part.NumericFieldProp = new NumericField { Value = 123 };
            part.BooleanFieldProp = new BooleanField { Value = true };
        });

        dynamic expandoValue = new ExpandoObject();
        expandoValue.stringValue = contentItem.Content.TestPart.TextFieldProp.Text;
        expandoValue.numberValue = contentItem.Content.TestPart.NumericFieldProp.Value;
        expandoValue.booleanValue = contentItem.Content.TestPart.BooleanFieldProp.Value;

        var jsonStr = JConvert.SerializeObject((ExpandoObject)expandoValue);
        Assert.Equal("{\"stringValue\":\"test\",\"numberValue\":123,\"booleanValue\":true}", jsonStr);
    }

Note that we decided to not bring back support for .Value. Therefore the following example will not work without casting to DateTime: contentItem2.Content.MyPart.myField.Value.FullYear. This example is also wrong for NSJ, there you also needed to get the strongly typed value by accessing it like contentItem2.Content.MyPart.myField.Value.Value.FullYear (note the second Value). This Value will be a DateTime in NSJ, but in STJ we don't have that type information. I think we can ignore that and force users to cast to a strongly typed value, if they require it. Otherwise the JsonDynamicValue class will get too complex I think (it already is a mess).

[hyzx86]

If we don't seem to have a way to ask the user to force a type conversion in Jint
Jint use ExpandoObject ， That's why I use ExpandoObject to wrap the value in the test

[gvkries]

But it should be almost the same as with NSJ now. Only .Value is missing and can be replaced with a cast.

[gvkries]

Please check #16292 again if it is enough for you.

[hyzx86]

Please check #16292 again if it is enough for you.

Thanks, but I still need to verify the difference between it and NSJ in Jint to make sure I don't have any more problems with my existing programmes.

I need some time to verify them.