fix/cookie jar delete

[jmschp]

What

I believe that the Rack::Test::CookieJar#delete and the Rack::Test::CookieJar#get_cookie should accept the cookie name as a Symbol. Just like Rack::Test::CookieJar#[] convert the argument to a sting delete and get_cookie should do the same.

For example the SessionTestHelper from Rails to be introduced in the next version uses cookies.delete(:session_id), with the current implementation this is not going to work.

[jmschp]

@rafaelfranca I made the necessary changes.

[jeremyevans]

I disagree that we should accept symbols for these methods. We shouldn't accept/convert them in Rack::Test::CookieJar#[]. We may be forced to due to backwards compatibility, but it is a bad idea. I know Rails tries to treat symbols and strings interchangeably, but that is a mistake, IMO. I'd be fine deprecating Rack::Test::CookieJar#[] accepting non-string.

[rafaelfranca]

What does deprecating/disallowing this give to the users of rack-tests? Or do you think deprecate/disallow would make this library easier to implement/maintain and that is what you are prioritizing in this situation?

Not particularly trying to push into any direction here, just want to understand the thought process so we can apply consistently in future PRs.

[rafaelfranca]

I removed the urgency from this decision by changing Rails to use the implementation we currently have in this gem in rails/rails@b164bb8.

Now we can decide what is best for this project.

As context, this was when we started to accept symbols in #[] #156

[jeremyevans]

As a general principle, I don't think you should automatically convert symbols to strings or vice-versa. Doing so leads to community confusion about the differences between symbols and strings. In many cases, this included, backwards compatibility means you may need continue to support previous design mistakes. However, that doesn't mean you need to allow for proliferation of such mistakes.

I'm not advocating for deprecating #[] taking a symbol, but if the argument is for consistency, I'd rather make #[] consistent with #delete/#get_cookie (no conversion), and not vice-versa. I am fine rejecting this and keeping #[] behavior the same for backwards compatibility. However, I am not in favor of further shifts towards symbol/string confusion.

Note that this particular issue is not only a convert symbol to string issue, as this accepts any object supporting to_s, so practically all objects.

[rafaelfranca]

I agree with your general principle. I also don't think this project specifically need to handle the design decision of Rails. Rails wraps most of the rack-test API anyway, and there is no reason to not wrap the CookieJar. If Rails desires to support strings and symbols, we could just wrap the rack-test cookie jar.

I'm fine with rejecting this change as well. And I'd not deprecate the behavior of #[] just because I don't think adding that burden to people will actually improve any problem in their applications.

Thank you for the pull request @jmschp and thank you for bringing the Rails issue to my attention.

[jmschp]

Thanks @rafaelfranca and @jeremyevans. Totally understand the String/Symbol concern, and actually agree with it.