[
https://issues.apache.org/jira/browse/WINK-179?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Mike Rheinheimer closed WINK-179.
---------------------------------
> test to ensure map types in ProvidersRegistry
> ---------------------------------------------
>
> Key: WINK-179
> URL: https://issues.apache.org/jira/browse/WINK-179
> Project: Wink
> Issue Type: Wish
> Components: Common
> Reporter: Mike Rheinheimer
> Assignee: Bryant Luk
> Fix For: 1.0
>
> Attachments: file.patch, WINK-179.patch
>
>
> A question came up recently about the necessity for
> ProvidersRegistry.MediaTypeMap to use type SimpleConcurrentMap since some
> lock protection is already built into the ProvidersRegistry methods:
> getContextResolver(), getMessageBodyReader(), or getMessageBodyWriter().
> The question is, why not just use a normal HashMap instead of
> SimpleConcurrentMap? It could be faster to avoid the extra locking, which
> appears unnecessary here.
> As it turns out, the extra locking is necessary. From Bryant:
> "The reason why there are two protections is that the first protection is in
> cases where the providers are dynamically added.
> The second protection is for the cache itself which could be written to by
> two different threads even if they both were getting a single
> MessageBodyReader (i.e. a cache value may be dropped and then two threads
> come back later and try to write a new cache value). Due to some weird
> HashMap properties, this can blow up in weird ways."
> So, my wish is to have a test that ensures the map type is appropriate and
> remains appropriate. That way if someone comes along in the future and sees
> the same issue, any change they make to the code will be caught in the
> unittest and a careful reading the code comments and unittest would show why
> the MediaTypeMap is using the types it uses.
> The problem is, we need to inspect the instantiated providersCache object,
> and there is currently no way to get to that object.
> What's your opinion? Since this is a sensitive path, it needs to have a
> unittest or two around it. Do we want to change the inner class and field
> visibility just to make it testable? This is the easy way to test it; we
> can make the MediaTypeMap inner class and providersCache object "protected",
> then override the MediaTypeMap class in a test to implement a
> getProvidersCache method so we can inspect the object (see attached patch).
> I suppose the other way to test it is to build up a multi-threaded test that
> dynamically add providers, the way Bryant mentioned. Sounds fun. :)
> The attached patch is NOT FINAL. Please review and let's discuss. Consider
> it merely a suggestion.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
