Add support for url form encoding collections #584
Conversation
When form encoding objects that have a property that is an IEnumerable type, use the QueryAttribute and it's CollectionFormat to determine how to encode the object. In general, this feature could be fleshed out much more deeply. Future work could include respecting custom formats and delimiters specified on the QueryAttribute, as well as support for collections stored in dictionaries that are form encoded.
Refit/FormValueDictionary.cs
Outdated
|
|
||
| var formattedValues = enumerable | ||
| .Cast<object>() | ||
| .Select(v => settings.FormUrlEncodedParameterFormatter.Format(v, null)); |
There was a problem hiding this comment.
Is there still value providing the attribute format string? Could these be combined? For example, for a specific double formatting in addition to the collection format?
There was a problem hiding this comment.
Wasn't sure before how the format strings fit into the picture. The attribute format string should definitely be respected
Refit/FormValueDictionary.cs
Outdated
| /// <remarks>Performs field renaming and value formatting as specified in <see cref="QueryAttribute"/>s and | ||
| /// <see cref="RefitSettings.FormUrlEncodedParameterFormatter"/>. Note that this is not a true dictionary, rather, a | ||
| /// form of MultiMap. A given key may appear multiple times with the same or different values.</remarks> | ||
| class FormValueDictionary : IEnumerable<KeyValuePair<string, string>> |
There was a problem hiding this comment.
Is having this be called a Dictionary confusing? Should I rename it? Outside of FormValueDictionaryTests there is only 1 line that would be affected by a rename.
There was a problem hiding this comment.
It can be renamed as it's an internal class
Refit/FormValueDictionary.cs
Outdated
| // see if there's a query attribute | ||
| var attrib = property.GetCustomAttribute<QueryAttribute>(true); | ||
|
|
||
| Add(GetFieldNameForProperty(property), settings.FormUrlEncodedParameterFormatter.Format(value, attrib?.Format)); | ||
| if (value is IEnumerable enumerable) { |
There was a problem hiding this comment.
Nit: please put braces on the next line
Refit/FormValueDictionary.cs
Outdated
| /// same key, but will not be considered. Returns <c>null</c> if no matching entry is found. | ||
| /// </summary> | ||
| public string this[string key] | ||
| { |
There was a problem hiding this comment.
Where is this currently used? Is there a danger in losing in this path that needs to be addressed? In other words, should this index 8000 er be removed and coded around a different way?
There was a problem hiding this comment.
Existing tests use it. I think there is a larger than necessary risk of future misuse, and the tests can be satisfied comfortably with a LINQ expression. Fixed in c63b62c
|
Thanks for this library and for taking the time to review the CL! |
|
Hi @MariusVolkhart first of all thanks for the PR. I was wandering if this PR fortunately addresses the following issue as well: [QueryParameters] Are broken for classes with IEnumerable properties #587 i took a quick look into the PR code and i couldn´t assert that it would work for non-decorated query parameter class definitions. I only use refit attributes on the Refit interface, and not on my POCOs/domain objects since they are shared with other libraries not referencing Refit. |
|
Here is an extract of the QueryParameter class: public class PostQueryOptions
{
public PostQueryOptions()
{
KeywordIds = new int[0];
MaxResults = 100;
}
[DataMember("tags", Order = 1)]
public int[] KeywordIds { get; set; }
[DataMember("limit", Order = 2)]
public int MaxResults { get; set; }
public string SearchPhrase { get; set; }
} |
When form encoding objects that have a property that is an IEnumerable type, use the QueryAttribute and it's CollectionFormat to determine how to encode the object.
In general, this feature could be fleshed out much more deeply. Future work could include respecting custom formats and delimiters specified on the QueryAttribute, as well as support for collections stored in dictionaries that are form encoded.