Eliminate Eh107InternalCacheManager by resolving services from services#2346
Eliminate Eh107InternalCacheManager by resolving services from services#2346henri-tremblay merged 6 commits intoehcache:masterfrom
Conversation
chrisdennis
force-pushed
the
107-cleanup
branch
from
46577a3 to
1ff645f
Compare
|
I prefer #2339 which eliminates it as well. But without giving some special status to the |
|
There is no special status here though. The Jsr107Service already declares a dependency on the StatisticsService, this just makes sure we retrieve it via the correct channels. |
| ehcacheManager.init(); | ||
|
|
||
| return new Eh107CacheManager(this, ehcacheManager, properties, config.getClassLoader(), uri, | ||
| return new Eh107CacheManager(this, ehcacheManager, jsr107Service.getStatistics(), properties, config.getClassLoader(), uri, |
There was a problem hiding this comment.
It still feels weird to pass the StatisticsService since the ehcacheManager is passed and already has it.
There was a problem hiding this comment.
Why not passing jsr107service and retrieve the statistics inside the constructor? I would prefer that.
chrisdennis
force-pushed
the
107-cleanup
branch
from
1ff645f to
13ae0cf
Compare
chrisdennis
force-pushed
the
107-cleanup
branch
2 times, most recently
from
56833e9 to
ed4e97f
Compare
chrisdennis
force-pushed
the
107-cleanup
branch
2 times, most recently
from
887891b to
76326ab
Compare
|
@henri-tremblay could you re-review this? It's been tweaked and expanded since you last looked at it. |
chrisdennis
force-pushed
the
107-cleanup
branch
3 times, most recently
from
0c9a187 to
0c17e39
Compare
chrisdennis
force-pushed
the
107-cleanup
branch
2 times, most recently
from
0e24cc6 to
19381c8
Compare
| public class DefaultJsr107Service implements Jsr107Service { | ||
|
|
||
| private final Jsr107Configuration configuration; | ||
| private StatisticsService statisticsService; |
There was a problem hiding this comment.
Shouldn't that field be volatile?
There was a problem hiding this comment.
Probably not since there the ServiceLocator creates a happens before relationship between the start of a dependency and the start of it's dependents... but it's probably safer to add it anyway.
chrisdennis
force-pushed
the
107-cleanup
branch
2 times, most recently
from
c8c4359 to
e332743
Compare
4 participants
No description provided.