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

Incorrect Serialization of Enums #293

Open
cruiserkernan opened this issue Dec 5, 2023 · 9 comments
Open

Incorrect Serialization of Enums #293

cruiserkernan opened this issue Dec 5, 2023 · 9 comments

Comments

@cruiserkernan
Copy link
Contributor

Description

The BlazorGoogleMaps library does not always use the EnumMember attribute when serializing enums to JSON. This causes map controls to not be correctly positioned on the map. For instance, the enum TOP_LEFT gets serialized as topLeft, leading to incorrect behavior.

Additional Details

  • There exists a JsonStringEnumConverterEx class for the correct behavior but it is not used on the ControlPosition enum.
  • The second issue is that Helper.cs had serialization that was not adding the converter to the JsonSerializerOptions. This might have been affecting more than the ControlPosition enum.

Expected Behavior

  • Enums should be serialized considering the EnumMember attribute.
  • Correct serialization should ensure that map controls are positioned as expected according to their specified enum values.

Current Behavior

  • Enums are serialized incorrectly, not respecting the EnumMember attribute.
  • This results in map controls being incorrectly positioned, impacting the usability of the map.

Steps to Reproduce

  1. Define a map control with a specific position using enum, like TOP_LEFT.
  2. Observe that the control is not positioned as TOP_LEFT but as BOTTOM_CENTER.

Suggested Fix

  • Modify the serialization process to respect the EnumMember attribute for enums.
  • Ensure that all enums are serialized correctly.
  • Utilize the existing JsonStringEnumConverterEx class for the ControlPosition enum and other enums missing the converter.
  • Update Helper.cs to include the converter in JsonSerializerOptions.

Additional Information

This serialization issue significantly impacts the functionality and usability of the BlazorGoogleMaps library, as controls and other enum-dependent features do not behave as expected.

Example of Incorrect Code

// Incorrect serialization of Enums in various classes
[JsonConverter(typeof(JsonStringEnumConverter))]

Potential Impact

  • This issue might lead to incorrect map behavior and visual inconsistencies.
  • Developers might have to find workarounds or wait for an official fix to use the library effectively.

Contribution and Proposed Changes

I am willing to contribute to the resolution of this issue and will submit a pull request with the necessary changes. The proposed changes include updating the JsonConverter attribute to respect the EnumMember attribute in enum serialization and correctly implementing the JsonStringEnumConverterEx in all the enums and add the converter in Helper.cs.

Thank you for your attention to this matter.

cruiserkernan pushed a commit to cruiserkernan/BlazorGoogleMaps that referenced this issue Dec 5, 2023
@valentasm1
Copy link
Collaborator

control is not positioned as TOP_LEFT but as BOTTOM_CENTER. this is different bug. Not related to serialization. Solved with #281

@cruiserkernan
Copy link
Contributor Author

I believe it is another bug. Upon running the Blazor Server Demo and examining the MapLegendPage, I observed that its intended position, TOP_LEFT, is not reflected in the actual placement; it appears at BOTTOM_CENTER instead. During debugging, I noticed that the addControl method in the objectManager is receiving "topLeft" as the enum value. However, this doesn't seem to match any case in the switch statement, causing it to revert to the default case of BOTTOM_CENTER.

I initially noticed this in my current application when trying to position the control on RIGHT_CENTER.

using System.Threading.Tasks;
using GoogleMapsComponents;
using GoogleMapsComponents.Maps;
using Microsoft.AspNetCore.Components;
using Microsoft.JSInterop;

namespace ServerSideDemo.Pages;

public partial class MapLegendPage
{
    private GoogleMap map1;

    private MapOptions mapOptions;

    [Inject] private IJSRuntime jsRuntime { get; set; }

    protected ElementReference legendReference { get; set; }

    protected override void OnInitialized()
    {
        mapOptions = new MapOptions()
        {
            Zoom = 13,
            Center = new LatLngLiteral()
            {
                Lat = 13.505892,
                Lng = 100.8162
            },
            MapTypeId = MapTypeId.Roadmap
        };
    }

    protected override async Task OnAfterRenderAsync(bool firstRender)
    {
        if (firstRender)
        {
            await jsRuntime.InvokeAsync<object>("initMapLegend");
        }
    }

    private async Task AfterMapInit()
    {
        await map1.InteropObject.AddControl(ControlPosition.TopLeft, legendReference);
    }
}

image

@valentasm1
Copy link
Collaborator

Please register new issue then. One bug one issue.

@cruiserkernan
Copy link
Contributor Author

I can do that, I just thought that since it was a caused by the serialization it was one bigger issue, that control position was just one case where I noticed it. But I understand that it might be a separate thing and could be fixed with less of a code change.
I will create a issue for only the control position and and a pull request only for that.

@valentasm1
Copy link
Collaborator

No need PR. Just sample of code how it should is enough since change is very small

valentasm1 added a commit that referenced this issue Dec 7, 2023
…d_using_enummember

Fix bug #293 Incorrect serialization of Enums
valentasm1 pushed a commit that referenced this issue Dec 7, 2023
@valentasm1
Copy link
Collaborator

I merger PR. It dont work. I tried for 30min and give up. Reverted.
If you do functionality. Startup server demo and try markers diff func, drawing polygons, polylines. Maybe smth else.
I am a bit disappointed.

@cruiserkernan
Copy link
Contributor Author

Sry my bad, I should have tested it more. I will update the code so that it works and thoroughly test it.

@Enritski
Copy link

Enritski commented Jan 8, 2024

I've been digging into this, and experimenting with a different custom serializer for enums.
I've come to the conclusion that a big part of the problem is due to the use of discriminated unions (OneOf) for various properties. If a type has a property that uses OneOf, the OneOfConverterFactory is used to serialize that property and does so by using the default serializer for whichever of the underlying types is being used. So, for example, in the case of the Symbol class, the Path property is a discriminated union: OneOf<SymbolPath, string>. The SymbolPath type is an enum and will serialize properly using the custom serializer (e.g., JsonStringEnumConverterEx), but not when it is part of a OneOf as is the case here.
I experimented with making the Path property just a simple SymbolPath instead of OneOf<SymbolPath, string> and that worked.
I've been trying to work out how to have the OneOfConverterFactory use the appropriate serializer for each of the possible types in the discriminated union, but haven't got there yet.
I just thought I'd add this comment here in case anyone else has an idea how to achieve this.

@valentasm1
Copy link
Collaborator

Sharing knowledge is always great. Thank you.

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

3 participants