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

Fix dictionary query request mapping #976

Merged
merged 5 commits into from
Oct 9, 2020

Conversation

folkcoder
Copy link
Contributor

What kind of change does this PR introduce?

Bug fix

What is the current behavior?

In the current version (5.2.1), dictionary query parameters with non-string keys throw an exception: 'Unable to cast object of type 'Example.Enum' to type 'System.String'.'.

Additionally, if the dictionary parameter is part of a complex object, the dictionary is serialized as ?DictionaryPropertyName=%5BKey%2C%20Value%5D.

This latter issue is reported here.

What is the new behavior?

The current query mapping code for IDictionary properties assumes that the dictionary key type is a string. This PR explicitly calls ToString() on the key before storing its value to the query map.

For dictionaries that are a property on a complex query object, the current behavior treats them the same as ordinary collections (e.g., implements IEnumerable<T>). This PR implements the quick fix offered by @ThomasSmeets to skip this handling and be handled by the appropriate query map method.

What might this PR break?

The change to handle IDictionary query parameters fixes an unhanded exception, this change is unlikely to break any existing assumptions.

The handling for IDictionary query properties that are part of a complex query objects may cause breaking changes for consumers who are expecting the previous serialization format (documented in this issue). However, prior to version 5.1.67, dictionary properties on query objects serialized as expected so this is likely a recent regression introduced by issue #896 and consumers are unlikely to be relying on this strange serialization behavior.

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)

@jamiehowarth0 jamiehowarth0 merged commit c62657b into reactiveui:master Oct 9, 2020
@github-actions
Copy link

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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 14, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants