tmenier / Flurl

Fluent URL builder and testable HTTP client for .NET
https://flurl.dev
MIT License
4.21k stars 387 forks source link

Support for nested objects in query strings #457

Open onliner10 opened 5 years ago

onliner10 commented 5 years ago

Hi, At work, we encountered a case where we have contract classes (DTOs) containing another DTOs, being nested objects therefore. It seemed that Flurl doesn't support this currently, while e.g. ASP.NET MVC model binder can correctly deserialize them from GET query parameters.

So, for example consider situation like:

public class Foo
{
  public Bar Bar {get;set;}
}

public class Bar
{
  public string A {get;set;}
  public string B {get;set;}
}

This can be deserialized by MVC model binder from the following form ?bar.a=sth&bar.b=sth_else.

This got implemented with my PR: https://github.com/tmenier/Flurl/pull/455 . What do you think about merging this ?

onliner10 commented 5 years ago

Any updates on this?

tmenier commented 5 years ago

I'm going to decline this. While I understand that ASP.NET knows how to deserialize that format to an object, I don't believe (correct me if I'm wrong) that this is such a common practice that Flurl should be changed to serialize this way by default. It's reasonable to assume that some people take advantage of the fact that Flurl just does a ToString, and override the method to have complete control of serialization. Maybe they convert the object to JSON or some other format. This change would break them.

I am, however, considering a new feature that adds some configurability to Flurl (the URL builder), similar to how configuring Flurl.Http works. Customizing the query string serializer would be a primary use case. That would be a 3.0 feature, and 3.0 of the URL builder is nearing completion so it would be fairly soon. In the mean time, I would suggest just adding your code to an extension method.

tmenier commented 5 years ago

As a side-note, if there's a good amount of interest in this feature, I might be interested in leveraging your code as some sort of opt-in switch on the default serializer, since serializing this way isn't exactly trivial to implement from scratch. (Can I get some thumbs-up for this?)

Or if nothing else, your code will stand as a very good example of custom serialization that others could use. Thanks!

STeeL835 commented 6 months ago

Is there an option to configure it this way now?

PeterOliverDev commented 1 month ago

Can we merge this pull request please?

tmenier commented 1 month ago

Thanks for the nudges, I think this fell off my radar about 5 years ago. As I've explained a few times, I'm against changing the default behavior because doing so would be breaking, and there's no "standard" for representing a complex object in a query string anyway. But adding a hook that lets you override the default behavior is something I'd entertain. I should have some time soon, and I'll try and make this idea a priority.