You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@wink.apache.org by "Mike Rheinheimer (JIRA)" <ji...@apache.org> on 2009/09/10 17:53:57 UTC
[jira] Created: (WINK-179) test to ensure map types in
ProvidersRegistry
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
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.
[jira] Commented: (WINK-179) test to ensure map types in
ProvidersRegistry
Posted by "Michael Elman (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/WINK-179?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12753787#action_12753787 ]
Michael Elman commented on WINK-179:
------------------------------------
I didn't look at the patch yet, but IMO the field/method/class visibility should not be changed just to make it testable.
It's possible to use a reflection to access the restricted fields.
It's possible to use AOP (e.g. AspectJ) to see which methods were invoked and when.
> 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.
[jira] Updated: (WINK-179) test to ensure map types in
ProvidersRegistry
Posted by "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.
[jira] Commented: (WINK-179) test to ensure map types in
ProvidersRegistry
Posted by "Mike Rheinheimer (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/WINK-179?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12754141#action_12754141 ]
Mike Rheinheimer commented on WINK-179:
---------------------------------------
Fair points Michael. I'll rework the patch so that visibility is unchanged for the private class and private member and use reflection through the instance data for which I already have visibility to and instances of. Thanks for the good discussion and honest opinions.
> 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.
[jira] Resolved: (WINK-179) test to ensure map types in
ProvidersRegistry
Posted by "Bryant Luk (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/WINK-179?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Bryant Luk resolved WINK-179.
-----------------------------
Resolution: Fixed
Fix Version/s: 0.2
Thanks Mike.
> 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: 0.2
>
> 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.
[jira] Commented: (WINK-179) test to ensure map types in
ProvidersRegistry
Posted by "Nick Gallardo (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/WINK-179?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12753781#action_12753781 ]
Nick Gallardo commented on WINK-179:
------------------------------------
I'm ok with the general concept of making fields "protected" instead of "private" for testing purposes.
As for what this test itself should look like... you don't want to write something that's just re-testing the SimpleConcurrentMap. But, you should probably test it somewhere if you're going to guarantee that it's doing what you think.
> 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.
[jira] Updated: (WINK-179) test to ensure map types in
ProvidersRegistry
Posted by "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: WINK-179.patch
WINK-179.patch is attached. It makes NO changes to production code, but uses pure reflection to get to the private field in the nested private superclass of the nested private extended class of the private field of ProvidersRegistry. Whew. :)
I agree that the test should exercise the protection mechanism to also ensure that the production code does what it should. However, as quick and dirty insurance against changes, I think this is an ok thing to put into the test bucket. Tests are not necessarily set in stone; I think they can be considered a "caution" against changes, which is what I would consider this test.
> 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, 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.
[jira] Commented: (WINK-179) test to ensure map types in
ProvidersRegistry
Posted by "Nick Gallardo (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/WINK-179?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12753800#action_12753800 ]
Nick Gallardo commented on WINK-179:
------------------------------------
Yep, you could use both reflection and aspects. Although, I'd rather not pull in AspectJ just to test something like this.
My point was that I do like that the pattern of extending classes in some cases for testability. Maybe that's better for scenarios where you're testing APIs vs. testing instance data.
> 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.
[jira] Closed: (WINK-179) test to ensure map types in
ProvidersRegistry
Posted by "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 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.
[jira] Assigned: (WINK-179) test to ensure map types in
ProvidersRegistry
Posted by "Bryant Luk (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/WINK-179?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Bryant Luk reassigned WINK-179:
-------------------------------
Assignee: Bryant Luk
> 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
> 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.