handle serialization for query objects with IEnumerable properties

[folkcoder]

What kind of change does this PR introduce?
This PR modifies the Refit request builder to handle serialization of query objects that contain IEnumerable properties. This PR addresses the following issues:

#587
#522

What is the current behavior?
Currently, an IEnumerable property on a query object will translate to the query string as the type name. For example, the following query object

class QueryObject
{
    IEnumerable<int> TestIntCollection { get; set; }
}

will translate as ?testIntCollection=System.Int32%5B%5D.

What is the new behavior?
IEnumerable properties on query objects will be serialized in the same manner as IEnumerable parameters passed directly.

What might this PR break?
No foreseen impacts to existing uses.

Please check if the PR fulfills these requirements

• Tests for the changes have been added (for bug fixes / features)
• Docs have been added / updated (for bug fixes / features)

[ahmedalejo]

There is more
#847

Work Arround:
For your specific case
Use IEnumerable instead of IEnumerable<int>

class QueryObject
{
    IEnumerable TestIntCollection { get; set; }
}

[ahmedalejo]

What about doubles or floats new []{1.1,2.1,3.0}?
in some locale like pt and pt-br (1.2).ToString() results in 1,2.
We need to make sure that CultureInfo.InvariantCulture is used

[folkcoder]

@ahmedalejo You've certainly provided some food for thought. Regarding the related issue in #847, the current behavior, which continues in my PR, is that the query attribute from the parent query object is used; the request builder does not even look for an overriding query attribute on any child property. As you've noticed, this can certainly cause some issues as when, for example, you want a custom string format for a date property that should not be applied to an int property.

It's easy enough to add logic that will apply the most specific query attribute (e.g., use the query attribute on the parent class unless a child property defines its own). I'm going to defer to the project maintainers on whether or not this fits with their intent for query objects. If so, I'm happy to implement that behavior.

I'm not a huge fan of adding Refit-specific attributes to objects that, for my purposes, are distributed across various layers of our application (e.g., to Refit clients, to ASP.NET controllers, etc.), but it seems unavoidable if we want to encapsulate our query requests within an object. A more robust long-term solution may be configuration classes similar to how libraries such as AutoMapper or EF Core map object properties without muddying up the objects themselves. This is, obviously, a much bigger lift.

[folkcoder]

Work Arround:
For your specific case
Use IEnumerable instead of IEnumerable<int>

In my particular use case, the query object is passed as part of a library to .NET consumers and we want to enforce the type safety of the provided collection, so this workaround doesn't well suit our purposes. Thanks for the idea though!

[ahmedalejo]

@ahmedalejo
I'm not a huge fan of adding Refit-specific attributes to objects that, for my purposes, are distributed across various layers of our application (e.g., to Refit clients, to ASP.NET controllers, etc.), but it seems unavoidable if we want to encapsulate our query requests within an object. A more robust long-term solution may be configuration classes similar to how libraries such as AutoMapper or EF Core map object properties without muddying up the objects themselves. This is, obviously, a much bigger lift.a

Me neither that why i´ve been favoring things that allow to define an interface that both the client and server with implement without specific server or client decorations on the interface.

That´s why i have been considering Rpc libraries libraries like Grpc.Net and MagicOnion

[folkcoder]

It's easy enough to add logic that will apply the most specific query attribute (e.g., use the query attribute on the parent class unless a child property defines its own). I'm going to defer to the project maintainers on whether or not this fits with their intent for query objects. If so, I'm happy to implement that behavior.

Any feedback on this? I'm eager to make the changes and get this fix through (and thus avoid implementing workarounds in our current Refit clients).

[ahmedalejo]

Ping @bennor

[meeple142]

Thank you!!!

[github-actions]

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.