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

JsonProperty ignored when deserialising response #706

Closed
c-lamont opened this issue Jun 23, 2022 · 8 comments
Closed

JsonProperty ignored when deserialising response #706

c-lamont opened this issue Jun 23, 2022 · 8 comments

Comments

@c-lamont
Copy link

c-lamont commented Jun 23, 2022

I feel I must be doing something wrong as this would be an obvious bug.
When deserialising the response with Flurl, the JsonProperty is ignored and the property is not set.
If I name the property to be exactly the same as the json, then it does serialise correctly.

Unit test that fails;

[Fact]
public async Task ReceiveJson_WithJson_ShouldReturnStickerColorDtoWithStickerColorSet()
{
    // Arrange
    using var httpTest = new HttpTest();
    var original = new StickerColorDto(){StickerColor = "green"};
    var json = JsonConvert.SerializeObject(original);
    const string url = "https://api.com";
    httpTest.ForCallsTo(url).RespondWith(json);
    
    // Act
    var response = await url.GetJsonAsync<StickerColorDto>();

    // Assert
    Assert.NotNull(response.StickerColor);
    Assert.Equal("green", response.StickerColor);
}

public class StickerColorDto
{
    [JsonProperty("sticker_color")] public string StickerColor { get; set; }
}

And changing the StickerColorDto to this, the test passes;

public class StickerColorDto
{
    [JsonProperty("sticker_color")] public string sticker_color { get; set; }
}
@c-lamont c-lamont added the bug label Jun 23, 2022
@c-lamont c-lamont changed the title ReceiveJson ignored JsonProperty of object it is deserialising to. JsonProperty ignored when deserialising response Jun 23, 2022
@tmenier
Copy link
Owner

tmenier commented Jun 23, 2022

What version of Flurl.Http are you using?

@c-lamont
Copy link
Author

Flurl.Http 4.0.0-pre2
Flurl 3.0.6

@tmenier
Copy link
Owner

tmenier commented Jun 24, 2022

Json.NET is being removed in 4.0, so its attributes won't work.

https://github.com/tmenier/Flurl/releases/tag/Flurl.Http.4.0.0-pre1

Keep an eye on release notes before upgrading prereleases and the final 4.0.0, there's likely to be breaking changes with each iteration.

@tmenier tmenier removed the bug label Jun 24, 2022
@c-lamont
Copy link
Author

c-lamont commented Jun 24, 2022

Thanks for the response. Interested to know what the reason is to remove Json.NET?

I saw the workaround in the link you sent on how to create your own NewtonsoftJsonSerializer and register it. It should be fine to use that when the app is running but I'll have to look for a solution to initialise it for my unit tests 🤔

@tmenier
Copy link
Owner

tmenier commented Jun 24, 2022

The rationale is all laid out in #517. In short, lots of well-reasoned arguments coupled with tons of demand (more than any other feature request in recent memory). It was broadly applauded when I finally did it. I'm not sure why the serializer is any harder to use in tests than in the main app, can you explain?

@tmenier
Copy link
Owner

tmenier commented Jun 24, 2022

Of course another alternative is to embrace the switch and replace all your JsonPropertys with JsonPropertyName, but I get that a lot of people are probably using more Newtonsoft-specific features than just that (including myself in my own projects).

@c-lamont
Copy link
Author

Okay thanks for the links, interesting reading.

I'm not sure why the serializer is any harder to use in tests than in the main app, can you explain?

I don't have a global setup/teardown or a unit test base class that I could simply initialise the serialiser. Do you have a hint on how you do it in your project?

In the mean time I'll look into the possibility of using JsonPropertyName.

@tmenier
Copy link
Owner

tmenier commented Jun 24, 2022

Off the top of my head, you could do it in the test class constructor, or even a static constructor of the test subject. Doesn't really matter, and it's not important that it only happens once.

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