Support multiple query string values with the same key #70

Merged
merged 1 commit into from Sep 7, 2013

Conversation

Projects
None yet
2 participants
@tapmantwo
Contributor

tapmantwo commented Sep 2, 2013

Hi,

I would like to be able to have a handler that takes multiple values for the same query string key. (eg. /details?id=1&id=2&id=3).

I've added a test handler in the Sandbox project, and several tests to the HandlerBuilderFactory to cover the scenarios.

To get this to work, I made changes to the way the variables dictionary is built up - so now if there is already a value with the same key, it appends to the existing value using a tab delimiter. I'm not sure whether tab is the best delimiter to use in this scenario? - I followed a similar approach found in the Application::CombineQueryStringValues function.

The main bulk of the change was actually in the PropertySetterBuilder.cs file. I extended the 'PropertyIsPrimitive' function to also check whether the property was an enumerable of primitive properties, and then in the 'CreateTrySimpleAssign' method, I added an extra condition to deal with an enumerable property. In this scenario, it calls a different conversion method ('SafeConvertEnumerable') which creates a list and parses the (delimited) string to populate it.

The Activator.CreateInstance call might be expensive here - what are your thoughts? If there's a better way, I'd be glad to learn about it.

Finally, I spotted that in the Priority enum, Higher & High had the same value as Lower & Low - I assume that the Higher & High values should be negative?

If there is an alternative approach you would prefer for dealing with this scenario, let me know and I'll attempt it :)

Cheers,

Richard

@markrendle

This comment has been minimized.

Show comment
Hide comment
@markrendle

markrendle Sep 7, 2013

Owner

Thanks, this is definitely filling a gap. I'm sure there's a code-gen way around the Activator call, I'll pull this and then try to refactor.

Owner

markrendle commented Sep 7, 2013

Thanks, this is definitely filling a gap. I'm sure there's a code-gen way around the Activator call, I'll pull this and then try to refactor.

markrendle added a commit that referenced this pull request Sep 7, 2013

Merge pull request #70 from tapmantwo/ParseMultiStrings
Support multiple query string values with the same key

@markrendle markrendle merged commit 3f0ad90 into markrendle:master Sep 7, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment