On 28/11/2014 18:39, Christopher Schultz wrote:
> Mark,
>
> On 11/28/14 12:06 PM, Mark Thomas wrote:
>> On 28/11/2014 15:44, Rémy Maucherat wrote:
>>> 2014-11-28 16:20 GMT+01:00 Martin Grigorov <mgrigo...@apache.org>:I'm not
>>> sure I follow you.
>>>
>>>> Assuming 'headers' is an instance of CaseInsensitiveKeyMap then:
>>>> - headers.get("BLAH") will delegate to innerMap.get(key{"blah"}) //
>>>> Perl-ish syntax to express what I mean
>>>> - headers.get("bLAh") will also delegate to innerMap.get(key{"blah"})
>>>>
>>>> so everything should be OK
>>
>> Part of the problem here is that this class is trying to meet a
>> currently undocumented specification. The WebSocket's TCK appears to be
>> testing (I say appears as I don't have access to it - I am just trying
>> to fix the associated issue) a requirement that is not documented in the
>> WebSocket spec.
>>
>> Until the actual requirement is documented I simply don't know what
>> requirement this implementation has to meet.
>>
>> The current implementation aims to be case insensitive while conserving
>> (as far as is possible) the original case of the key. Whether this is
>> overkill is TBD.
>
> Maintaining the original case only really apples for the toString() and
> keySet() methods (and friends). It's nice to have and doesn't cost much
> to maintain.
Early indications from the WebSocket EG are that the original case needs
to be retained everywhere.
> Another strategy is to keep a map of toLowerCase(key) -> originalKey
> which can be used for those methods.
I tried that first. It gets messier, faster than the current approach.
>>> I am talking about the impl as is in websockets right now, the case
>>> insensitivity is not done right. Besides that, it is possible this new impl
>>> could be optimized (or not).
>>
>> I am almost certain that there will be scope for optimisation. I was
>> aiming for minimal, clear code rather than performance. I doubt - given
>> the way that this class will be used - that performance will be an issue
>> but we can always come back to it if it is.
>>
>> I suspect that much more optimisation will be possible once we know what
>> the real requirement is.
>
> I have implemented a CaseInsensitiveKeyMap in the past and was able to
> do it by extending AbstractMap and implementing only these methods:
>
> - clear
> - clone
> - containsKey
> - containsValue
> - entrySet
> - get
> - isEmpty
> - keySet
> - put
> - readObject (for java.io.Serializable; I implement an original-case key
> cache)
> - remove
> - size
> - values
>
> I wasn't optimizing mine for speed, so I'm not sure if Mark's additional
> method implementations improve speed or just increase code. I suspect
> that many of the inner classes are not necessary (other than Key, of
> course).
I believe that the inner classes are all necessary in order to expose
the original String value for the key in the key set and entry set
iterators.
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
