[jira] Updated: (WINK-179) test to ensure map types in ProvidersRegistry

["Mike Rheinheimer (JIRA)" <ji...@apache.org>]

     [ https://issues.apache.org/jira/browse/WINK-179?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Mike Rheinheimer updated WINK-179:
----------------------------------

    Attachment: file.patch

Patch can be applied at wink-common/ directory level.

> 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
>         Attachments: file.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.