ank19 commented on a change in pull request #6728:
URL: https://github.com/apache/camel/pull/6728#discussion_r783904381
##########
File path:
components/camel-caffeine/src/main/java/org/apache/camel/component/caffeine/cache/CaffeineCacheEndpoint.java
##########
@@ -64,27 +64,37 @@ public Producer createProducer() throws Exception {
@Override
protected void doStart() throws Exception {
- cache = CamelContextHelper.lookup(getCamelContext(), cacheName,
Cache.class);
- if (cache == null) {
- Caffeine<?, ?> builder = Caffeine.newBuilder();
- if (configuration.getEvictionType() == EvictionType.SIZE_BASED) {
- builder.initialCapacity(configuration.getInitialCapacity());
- builder.maximumSize(configuration.getMaximumSize());
- } else if (configuration.getEvictionType() ==
EvictionType.TIME_BASED) {
-
builder.expireAfterAccess(configuration.getExpireAfterAccessTime(),
TimeUnit.SECONDS);
-
builder.expireAfterWrite(configuration.getExpireAfterWriteTime(),
TimeUnit.SECONDS);
- }
- if (configuration.isStatsEnabled()) {
- if (ObjectHelper.isEmpty(configuration.getStatsCounter())) {
- builder.recordStats();
+
+ synchronized (this) {
+ cache = CamelContextHelper.lookup(getCamelContext(), cacheName,
Cache.class);
+ if (cache == null) {
+ if (configuration.isCreateCacheIfNotExist()) {
+ Caffeine<?, ?> builder = Caffeine.newBuilder();
+ if (configuration.getEvictionType() ==
EvictionType.SIZE_BASED) {
+
builder.initialCapacity(configuration.getInitialCapacity());
+ builder.maximumSize(configuration.getMaximumSize());
+ } else if (configuration.getEvictionType() ==
EvictionType.TIME_BASED) {
+
builder.expireAfterAccess(configuration.getExpireAfterAccessTime(),
TimeUnit.SECONDS);
+
builder.expireAfterWrite(configuration.getExpireAfterWriteTime(),
TimeUnit.SECONDS);
+ }
+ if (configuration.isStatsEnabled()) {
+ if
(ObjectHelper.isEmpty(configuration.getStatsCounter())) {
+ builder.recordStats();
+ } else {
+
builder.recordStats(configuration::getStatsCounter);
+ }
+ }
+ if
(ObjectHelper.isNotEmpty(configuration.getRemovalListener())) {
+
builder.removalListener(configuration.getRemovalListener());
+ }
+ cache = builder.build();
+ getCamelContext().getRegistry().bind(cacheName,
Cache.class, cache);
Review comment:
Ah, ok - I think I got your point. However, might it be that the
documentation is misleading and the associated test case is wrong? As it just
works with .toF() without prior initialization? The problem with the test case
is IMHO that if the GET command returns null, the body is not set, as there's a
null check in that action. That in turn means that the body value from the PUT
action before is still set and therefore the test succeeds, but not because the
same cache is used.
Basically
```
camelContext.getRegistry().bind("cache",
Caffeine.newBuilder().build());
from("direct://start")
.to("caffeine-cache://cache?action=PUT&key=1")
.setBody(constant("")) // test case fix to illustrate the issue
.to("caffeine-cache://cache?key=1&action=GET")
.log("Test! ${body}")
.to("mock:result");
```
instead of
```
from("direct://start")
.to("caffeine-cache://cache?action=PUT&key=1")
.to("caffeine-cache://cache?key=1&action=GET")
.log("Test! ${body}")
.to("mock:result");
```I don't dare to say whether or not binding a cache to the registry within
the endpoint fits to the Camel concept, but it doesn't, the synchronization
indeed doesn't make sense at all, of course. Otherwise a synchronization would
be required I guess, but according to my tests it has to be something like
synchronized (getCamelContext().getRegistry()) instead of a synchronized(this),
which is probably a very expensive lock and would have to be mitigated by
double-check idiom or so. Thinking of it, I guess the binding looks somehow
complicated - you're right and the cache has to be created before using it. Can
you confirm? Then I try to update the pull request asap.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@camel.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
