Details
diff --git a/profiles/killbill/src/main/java/org/killbill/billing/server/notifications/PushNotificationListener.java b/profiles/killbill/src/main/java/org/killbill/billing/server/notifications/PushNotificationListener.java
index 16dc8db..f0dfcdd 100644
--- a/profiles/killbill/src/main/java/org/killbill/billing/server/notifications/PushNotificationListener.java
+++ b/profiles/killbill/src/main/java/org/killbill/billing/server/notifications/PushNotificationListener.java
@@ -75,6 +75,10 @@ public class PushNotificationListener {
final TenantContext context = contextFactory.createTenantContext(event.getTenantId());
try {
final List<String> callbacks = getCallbacksForTenant(context);
+ if (callbacks.isEmpty()) {
+ // Optimization - see https://github.com/killbill/killbill/issues/297
+ return;
+ }
dispatchCallback(event.getTenantId(), event, callbacks);
} catch (final TenantApiException e) {
log.warn("Failed to retrieve push notification callback for tenant {}", event.getTenantId());
diff --git a/tenant/src/main/java/org/killbill/billing/tenant/api/user/DefaultTenantUserApi.java b/tenant/src/main/java/org/killbill/billing/tenant/api/user/DefaultTenantUserApi.java
index 9ff60e6..f1fe45c 100644
--- a/tenant/src/main/java/org/killbill/billing/tenant/api/user/DefaultTenantUserApi.java
+++ b/tenant/src/main/java/org/killbill/billing/tenant/api/user/DefaultTenantUserApi.java
@@ -110,11 +110,11 @@ public class DefaultTenantUserApi implements TenantUserApi {
@Override
public List<String> getTenantValuesForKey(final String key, final TenantContext context) throws TenantApiException {
final InternalTenantContext internalContext = internalCallContextFactory.createInternalTenantContext(context);
- final String value = getCachedTenantValueForKey(key, internalContext);
- if (value != null) {
- return ImmutableList.<String>of(value);
+ if (!isCachedInTenantKVCache(key)) {
+ return tenantDao.getTenantValueForKey(key, internalContext);
+ } else {
+ return getCachedTenantValuesForKey(key, internalContext);
}
- return tenantDao.getTenantValueForKey(key, internalContext);
}
@Override
@@ -135,12 +135,15 @@ public class DefaultTenantUserApi implements TenantUserApi {
tenantKVCache.remove(tenantKey);
}
- private String getCachedTenantValueForKey(final String key, final InternalTenantContext internalContext) {
- if (!isCachedInTenantKVCache(key)) {
- return null;
- }
+ private List<String> getCachedTenantValuesForKey(final String key, final InternalTenantContext internalContext) {
final String tenantKey = getCacheKeyName(key, internalContext);
- return (String) tenantKVCache.get(tenantKey, new CacheLoaderArgument(ObjectType.TENANT_KVS));
+ final Object cachedTenantValues = tenantKVCache.get(tenantKey, new CacheLoaderArgument(ObjectType.TENANT_KVS));
+ if (cachedTenantValues == null) {
+ return ImmutableList.<String>of();
+ } else {
+ // Current, we only cache single-value keys
+ return ImmutableList.<String>of((String) cachedTenantValues);
+ }
}
private String getCacheKeyName(final String key, final InternalTenantContext internalContext) {
diff --git a/tenant/src/test/java/org/killbill/billing/tenant/api/user/TestDefaultTenantUserApi.java b/tenant/src/test/java/org/killbill/billing/tenant/api/user/TestDefaultTenantUserApi.java
index b39f4b4..9d70464 100644
--- a/tenant/src/test/java/org/killbill/billing/tenant/api/user/TestDefaultTenantUserApi.java
+++ b/tenant/src/test/java/org/killbill/billing/tenant/api/user/TestDefaultTenantUserApi.java
@@ -46,6 +46,30 @@ public class TestDefaultTenantUserApi extends TenantTestSuiteWithEmbeddedDb {
Assert.assertEquals(value.size(), 0);
}
+ @Test(groups = "slow", description = "https://github.com/killbill/killbill/issues/297")
+ public void testVerifyCacheOnAbsentValues() throws Exception {
+ final String tenantKey = TenantKey.PLUGIN_CONFIG_.toString() + "MyPluginName";
+
+ // Warm the cache with the empty value
+ List<String> value = tenantUserApi.getTenantValuesForKey(tenantKey, callContext);
+ Assert.assertEquals(value.size(), 0);
+
+ // Update the DAO directly (caching is done at the API layer)
+ tenantDao.addTenantKeyValue(tenantKey, "TheValue-hidden!", true, internalCallContext);
+
+ // Verify we still hit the cache
+ value = tenantUserApi.getTenantValuesForKey(tenantKey, callContext);
+ Assert.assertEquals(value.size(), 0);
+
+ // Update the cache
+ tenantUserApi.addTenantKeyValue(tenantKey, "TheValue", callContext);
+
+ // Verify the cache now has the right value
+ value = tenantUserApi.getTenantValuesForKey(tenantKey, callContext);
+ Assert.assertEquals(value.size(), 1);
+ Assert.assertEquals(value.get(0), "TheValue");
+ }
+
@Test(groups = "slow")
public void testSystemKeySingleValue() throws Exception {
final String tenantKey = TenantKey.PLUGIN_CONFIG_.toString() + "MyPluginName";
diff --git a/util/src/main/java/org/killbill/billing/util/cache/BaseCacheLoader.java b/util/src/main/java/org/killbill/billing/util/cache/BaseCacheLoader.java
index 7aa7472..c0044c6 100644
--- a/util/src/main/java/org/killbill/billing/util/cache/BaseCacheLoader.java
+++ b/util/src/main/java/org/killbill/billing/util/cache/BaseCacheLoader.java
@@ -1,7 +1,9 @@
/*
* Copyright 2010-2013 Ning, Inc.
+ * Copyright 2014-2015 Groupon, Inc
+ * Copyright 2014-2015 The Billing Project, LLC
*
- * Ning licenses this file to you under the Apache License, version 2.0
+ * The Billing Project licenses this file to you under the Apache License, version 2.0
* (the "License"); you may not use this file except in compliance with the
* License. You may obtain a copy of the License at:
*
@@ -30,6 +32,8 @@ import net.sf.ehcache.loader.CacheLoader;
public abstract class BaseCacheLoader implements CacheLoader {
+ static final String EMPTY_VALUE_PLACEHOLDER = "__#VALEUR!__";
+
private Status cacheLoaderStatus;
@Inject
diff --git a/util/src/main/java/org/killbill/billing/util/cache/EhCacheBasedCacheController.java b/util/src/main/java/org/killbill/billing/util/cache/EhCacheBasedCacheController.java
index ce26eb3..1f7998f 100644
--- a/util/src/main/java/org/killbill/billing/util/cache/EhCacheBasedCacheController.java
+++ b/util/src/main/java/org/killbill/billing/util/cache/EhCacheBasedCacheController.java
@@ -41,7 +41,7 @@ public class EhCacheBasedCacheController<K, V> implements CacheController<K, V>
@Override
public V get(final K key, final CacheLoaderArgument cacheLoaderArgument) {
final Element element = cache.getWithLoader(key, null, cacheLoaderArgument);
- if (element == null) {
+ if (element == null || element.getObjectValue() == null || element.getObjectValue().equals(BaseCacheLoader.EMPTY_VALUE_PLACEHOLDER)) {
return null;
}
return (V) element.getObjectValue();
diff --git a/util/src/main/java/org/killbill/billing/util/cache/TenantKVCacheLoader.java b/util/src/main/java/org/killbill/billing/util/cache/TenantKVCacheLoader.java
index 02b709f..a949445 100644
--- a/util/src/main/java/org/killbill/billing/util/cache/TenantKVCacheLoader.java
+++ b/util/src/main/java/org/killbill/billing/util/cache/TenantKVCacheLoader.java
@@ -59,7 +59,7 @@ public class TenantKVCacheLoader extends BaseCacheLoader {
final InternalTenantContext internalTenantContext = new InternalTenantContext(Long.valueOf(tenantRecordId));
final List<String> valuesForKey = tenantApi.getTenantValuesForKey(rawKey, internalTenantContext);
if (valuesForKey == null || valuesForKey.isEmpty()) {
- return null;
+ return EMPTY_VALUE_PLACEHOLDER;
}
if (valuesForKey.size() > 1) {
throw new IllegalStateException("TenantKVCacheLoader expecting no more than one value for key " + key);