TRUNK-3675 Introduce caching to getSearchLocales() #1863
Conversation
pgutkowski
force-pushed
the
TRUNK-3675
branch
2 times, most recently
from
be845f9
to
2db9b51
Compare
| @@ -345,6 +358,7 @@ | |||
| * @should include currently selected full locale and langugage | |||
| * @should include users proficient locales | |||
| * @should exclude not allowed locales | |||
| * @should cache results for an user | |||
| * @verifies cache results for an user | ||
| */ | ||
| @Test | ||
| public void getSearchLocales_shouldCacheResultsForAnUser() throws Exception { |
pgutkowski
force-pushed
the
TRUNK-3675
branch
2 times, most recently
from
2b3aae8
to
501d622
Compare
| diskPersistent="false" | ||
| diskExpiryThreadIntervalSeconds="120" | ||
| memoryStoreEvictionPolicy="LRU" | ||
| /> |
There was a problem hiding this comment.
If I recall correctly, when you setup ehcache this way you don't have to declare caches for each name... If they are not declared the defaults are used... Am I wrong here?
There was a problem hiding this comment.
If I'm right modules can use caches based on the default cache config without having to explicitly declare names here. Sys admins seeing too much memory consumption could edit the file and disable some of caches by overriding their settings. We could provide more granular adjustments of config for module developers in a separate issue if needed.
There was a problem hiding this comment.
Yes, you are right, default caches are created.
We could provide more granular adjustments of config for module developers in a separate issue if needed.
I think it is a good idea
| @@ -9,7 +9,7 @@ | |||
| graphic logo is a trademark of OpenMRS Inc. | |||
|
|
|||
| --> | |||
| <ehcache> | |||
| <ehcache name="hibernateCache"> | |||
There was a problem hiding this comment.
Where do you tell Hibernate to use "hibernateCache"? Or is it the default name it looks for?
There was a problem hiding this comment.
As you said, ehcache.xml is default filename. In fact, we can rename it to hibernate-ehcache.xml by adding net.sf.ehcache.configurationResourceName parameter to hiebrnate.cfg, if you think we need to be more clear about it
There was a problem hiding this comment.
I just wondered why you added name="hibernateCache" here...
There was a problem hiding this comment.
I did it for debugging purposes, because default name is "DEFAULT". At first I had problem with those multiple CacheManager instances, so having explicit names was handy, and I think it can be useful in future if anybody will have to do something with those.
| diskPersistent="false" diskExpiryThreadIntervalSeconds="45"/> | ||
|
|
||
| <cache name="userSearchLocales" | ||
| maxElementsInMemory="5000" |
There was a problem hiding this comment.
Probably is, I've copied this configuration from commit which was reverted a couple months ago, I guess we can limit it to 500?
pgutkowski
force-pushed
the
TRUNK-3675
branch
from
501d622
to
898bf3b
Compare
TRUNK-3675
Description
This PR introduces Spring's caching functionality to OpenMRS. As proof of concept, results of
getSearchLocalesare cached.We couldn't use annotation driven cache configuration, because we are still using old
TransactionProxyFactoryBeanto handle transactions, so CacheInterceptor is added to preInterceptors ofTransactionProxyFactoryBean, to handle cache. More info can be found in Spring documentation.In long run, we probably will want to replace SimpleCacheManager with EhCacheManager, but I couldn't properly inject it, I guess HIbernate's second level cache ehCache is colliding with it, because when it was disabled, I was able to register my API EhCacheManagerI managed to set up ehcache manager for API instead of SimpleCacheManager. Previously it didn't work because Hibernate used singleton factory, Right now we will have 2 caches -
hibernateCacheandapiCache. I'm still working on making cache configuration extendable by modules.Related Issue
see https://issues.openmrs.org/browse/TRUNK-3675
Checklist:
git pull --rebase upstream master.mvn clean packageright before creating this pull request andadded all formatting changes to my commit.