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

Issue with ApplyObjectFromDictionary Method Handling Integer Values as Long #29

Closed
won5572 opened this issue May 22, 2024 · 13 comments
Closed

Comments

@won5572
Copy link

won5572 commented May 22, 2024

There is an issue with the ApplyObjectFromDictionary method where integer values in the IDictionary<string, object> source are being interpreted as long during debugging. This causes an InvalidCastException when attempting to set properties on the target object if the target property is of type int.

This issue is particularly problematic when working with JSON deserialization where numeric values are commonly interpreted as long. The proposed solution ensures compatibility with various numeric types and enhances the robustness of the method.

Example Code

public class TargetClass
{
    public int ExampleProperty { get; set; }
}

var target = new TargetClass();
var source = new Dictionary<string, object>
{
    { "ExampleProperty", 42 } // int value
};

ApplyObjectFromDictionary(target, source); // Exception occurs here

Modified Code

private void ApplyObjectFromDictionary<T>(T target, IDictionary<string, object> source)
{
    if (source == null || target == null)
    {
        return;
    }

    var targetType = target.GetType();

    foreach (var sourceProperty in source)
    {
        var targetProperty = targetType.GetProperty(sourceProperty.Key, BindingFlags.Instance | BindingFlags.Public | BindingFlags.IgnoreCase);

        if (targetProperty == null || !targetProperty.CanWrite)
        {
            continue;
        }

        var value = sourceProperty.Value;

        if (value != null)
        {
            // Convert ChangeType
            if (!targetProperty.PropertyType.IsInstanceOfType(value))
            {
                try
                {
                    value = Convert.ChangeType(value, targetProperty.PropertyType);
                }
                catch (Exception ex)
                {
                    throw new InvalidCastException($"Type mismatch in {sourceProperty.Key} parameter: {ex.Message}");
                }
            }
            
            targetProperty.SetValue(target, value);
        }
    }
}
@kjeske
Copy link
Contributor

kjeske commented May 22, 2024

Hi! From what I understood, you're doing a manual deserialization which interprets a integer as long instead of int, and then it fails when passed to a Hydro component, correct?

Could you show how you are passing such object to the component?

@kjeske
Copy link
Contributor

kjeske commented May 27, 2024

Could you give an update on this?

@kjeske
Copy link
Contributor

kjeske commented May 28, 2024

I'm closing for now. If you get back with more info, we can reopen.

@kjeske kjeske closed this as completed May 28, 2024
@won5572
Copy link
Author

won5572 commented Jun 18, 2024

I would like you to update the ApplyObjectFromDictionary method with the modified code.

I'll show you the code.

This code got the error "System.InvalidCastException: Type mismatch in uid parameter."

Test.cshtml

            var data = new Dictionary<string, object>
            {
                { "Uid", uid },
                { "Title", title }
            };
            
            //open a dialog with dispatch
            Dispatch(new OpenDialog("Popup", data, dialogOptions), Scope.Global);

Dialog.cshtml

                    @if (data is null)
                    {
                        <hydro name="@name" />
                    }
                    else
                    {
                        <hydro name="@name" params="@data" />
                    }

Popup.cshtml

@model Popup
    <div>
        @Model.uid
        @Model.title
    </div>

Popup.cshtml.cs

public class Popup : HydroComponent
{
    public int uid { get; set; }
    public string title { get; set; } = string.Empty;

    public override async Task MountAsync()
    {
        await base.MountAsync();
    }
}

@won5572
Copy link
Author

won5572 commented Jun 18, 2024

Popup.cshtml.cs

public class Popup : HydroComponent
{
    public int uid { get; set; } // if long uid { get; set;}, it's not mismatch.
    public string title { get; set; } = string.Empty;

    public override async Task MountAsync()
    {
        await base.MountAsync();
    }
}

@kjeske kjeske reopened this Jun 18, 2024
@kjeske
Copy link
Contributor

kjeske commented Jun 18, 2024

Is there any reason to use Dictionary instead of object?

Your code:

var data = new Dictionary<string, object>
{
    { "Uid", uid },
    { "Title", title }
};

//open a dialog with dispatch
Dispatch(new OpenDialog("Popup", data, dialogOptions), Scope.Global);

Could be:

var data = new { Uid = uid, Title = title };
Dispatch(new OpenDialog("Popup", data, dialogOptions), Scope.Global);

@won5572
Copy link
Author

won5572 commented Jun 19, 2024

I remember that the data variable was changed to Newtonsoft.Json.Linq.JObject type when the value was transferred through dispatch on the test page, and the parameter was not properly processed, so I used the dictionary type.

@kjeske
Copy link
Contributor

kjeske commented Jun 29, 2024

How do you get your data that you use in file Dialog.cshtml? Are you taking JObject from the event and converting to a dictionary? At least JObject is something that Hydro provides when tries to deserialize any structure into an object.

Passing data as a dictionary through params is an internal mechanism that will be obsolete at some point, so I suggest to use object. I would also wrap the data you pass to the Dialog in an object, like DialogData. Then your Dialog.cshtml could have:

<hydro name="@name" params="@(new { DialogData = data })" />

The reason of doing that is data in this case is an anonymous JObject coming from the event and to deal with it in Hydro, we need to put it in a container property, so it can be deserialized later properly into strongly typed property.

I could make it that params would accept JObject and then handle it natively, but it would be a bit hacky.

@won5572
Copy link
Author

won5572 commented Jul 1, 2024

[Dialogs.cshtml.cs]

public class Dialogs : HydroComponent
{
    //For Dialog
    public Dictionary<string, IDictionary<string, object>?> DialogsDictionary { get; set; } = default!;
    public bool IsOpen { get; set; }

    public Dialogs()
    {
        Subscribe<OpenDialog>(HandleOpenDialog);
        Subscribe<CloseDialog>(HandleCloseDialog);
    }

    private void HandleOpenDialog(OpenDialog dialog)
    {
        DialogsDictionary.Clear();
        DialogsDictionary.TryAdd(dialog.Name, dialog.Data);
        IsOpen = true;
        ViewBag.DialogOptions = dialog.DialogOptions;
    }

    private void HandleCloseDialog(CloseDialog dialog)
    {
        DialogsDictionary.Clear();
        IsOpen = false;
    }
}

[Dialogs.cshtml]

@model Dialogs
@using Hydro
@using Hydro.TagHelpers
@using System.Text.Json
@using Newtonsoft.Json
@using Newtonsoft.Json.Linq
@using System.Dynamic
@{
    var options = ViewBag.DialogOptions as DialogOptions ?? new DialogOptions();
    var shake = !Model.ModelState.IsValid;
}
<div>
    @foreach (var (name, data) in Model.DialogsDictionary)
    {
        <div class="overlay h-screen w-full bg-black bg-opacity-30 fixed top-0 left-0 z-10 flex items-center justify-center @(Model.IsOpen ? "" : "hidden")">
            <div class="modal z-20 bg-white rounded-md absolute overflow-y-auto @options.DialogWidth @options.DialogHeight @options.DialogClass">
                <div class="modal-header flex justify-between px-5 py-3 bg-gray-800 text-white font-bold @options.HeaderClass">
                    <p>
                        @options.Title
                    </p>
                    <p>
                        @if (!string.IsNullOrEmpty(options.RedirectUrl))
                        {
                            <button class="border px-2 py-[1px]" hydro-on:click="@(() => Model.CloseThisDialog(options.RedirectUrl))">X</button>
                        }
                        else
                        {
                            <button class="border px-2 py-[1px]" hydro-on:click="@(() => Model.CloseThisDialog(string.Empty))">X</button>
                        }
                    </p>
                </div>
                <div class="modal-body overflow-y-auto p-4 @options.BodyClass">
                    @if (data is null)
                    {
                        <hydro name="@name" />
                    }
                    else
                    {
                        <hydro name="@name" params="@data" />
                    }
                </div>
            </div>
        </div>
    }
</div>

When I turn over the data in the way below, it's converted to a Newtonsoft.Json.Linq.JObject type.
I don't know why it's like this. When I checked the value by using the breakpoint with a debugger, it was JObject type.
So I transferred it to the Dictionary <string, object> type and used it as hydro params.

var data = new { Uid = uid, Title = title };
Dispatch(new OpenDialog("Popup", data, dialogOptions), Scope.Global);

@kjeske
Copy link
Contributor

kjeske commented Jul 24, 2024

In order to make it work you would need to define DialogData property in your Popup.cshtml.cs:

public class Popup : HydroComponent
{
    public PopupData DialogData { get; set; }

    public override async Task MountAsync()
    {
        await base.MountAsync();
    }
}

public class PopupData
{
    public int Uid { get; set; }
    public string Title { get; set; } = string.Empty;
}

And then pass the data to it using

<hydro name="@name" params="@(new { DialogData = data })" />

Such code will work. The reason is that we can't pass a generic event's data object as a set of root properties to Hydro component. You need to create a root property on component to which you assign your event's data object, in this case DialogData.

@won5572
Copy link
Author

won5572 commented Jul 25, 2024

Okay, I understood.

Can't you update Method ApplyObjectFromDictionary the added code below?
I think the problem I have will be solved if it is updated.

private void ApplyObjectFromDictionary<T>(T target, IDictionary<string, object> source)
{
    if (source == null || target == null)
    {
        return;
    }

    var targetType = target.GetType();

    foreach (var sourceProperty in source)
    {
        var targetProperty = targetType.GetProperty(sourceProperty.Key, BindingFlags.Instance | BindingFlags.Public | BindingFlags.IgnoreCase);

        if (targetProperty == null || !targetProperty.CanWrite)
        {
            continue;
        }

        var value = sourceProperty.Value;

        if (value != null)
        {
            // Convert ChangeType
            if (value != null && !targetProperty.PropertyType.IsInstanceOfType(value))
            {
                try
                {
                    value = Convert.ChangeType(value, targetProperty.PropertyType);
                }
                catch (Exception ex)
                {
                    throw new InvalidCastException($"Type mismatch in {sourceProperty.Key} parameter: {ex.Message}");
                }
            }
            
            targetProperty.SetValue(target, value);
        }
    }
}

[Existing code]

            if (value != null && !targetProperty.PropertyType.IsInstanceOfType(value))
            {
                throw new InvalidCastException($"Type mismatch in {sourceProperty.Key} parameter.");
            }

[Modified code]

            if (value != null && !targetProperty.PropertyType.IsInstanceOfType(value))
            {
                try
                {
                    value = Convert.ChangeType(value, targetProperty.PropertyType);
                }
                catch (Exception ex)
                {
                    throw new InvalidCastException($"Type mismatch in {sourceProperty.Key} parameter: {ex.Message}");
                }
            }

@kjeske
Copy link
Contributor

kjeske commented Jul 26, 2024

Like I mentioned before, the method ApplyObjectFromDictionary will be soon obsolete, so there is no point in changing it, since in the next major version will be removed. Any reason you don't want to switch to the implementation I suggested with the PopupData class?

@kjeske
Copy link
Contributor

kjeske commented Jul 29, 2024

Closing for now, the issue can be fixed by suggested workaround, which is also recommended in similar situations. If you have any questions, feel free to open the discussion.

@kjeske kjeske closed this as completed Jul 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants