util: make EhCacheBasedCacheController strongly typed

Cache operations on Ehcache 3 require the user to pass the expected
classes for both the key and the value.

To simplify the upgrade, do the same for our cache abstraction layer.

As part of this work, a bug was discovered in DefaultAccountApiBase:
the account id (UUID) was used to populate the ImmutableAccountData cache while
the account record id (Long) was expected instead. As a result, we probably
have a high miss rate on the ImmutableAccountData cache today.

See #587.

Signed-off-by: Pierre-Alexandre Meyer <pierre@mouraf.org>

account/src/main/java/org/killbill/billing/account/api/svcs/DefaultAccountInternalApi.java

@@ -56,7 +56,7 @@ public class DefaultAccountInternalApi extends DefaultAccountApiBase implements
 
     private final ImmutableAccountInternalApi immutableAccountInternalApi;
     private final AccountDao accountDao;
-    private final CacheController bcdCacheController;
+    private final CacheController<UUID, Integer> bcdCacheController;
 
     @Inject
     public DefaultAccountInternalApi(final ImmutableAccountInternalApi immutableAccountInternalApi,
@@ -106,7 +106,7 @@ public void updateBCD(final String externalKey, final int bcd,
     @Override
     public int getBCD(final UUID accountId, final InternalTenantContext context) throws AccountApiException {
         final CacheLoaderArgument arg = createBCDCacheLoaderArgument(context);
-        final Integer result = (Integer) bcdCacheController.get(accountId, arg);
+        final Integer result = bcdCacheController.get(accountId, arg);
         return result != null ? result : DefaultMutableAccountData.DEFAULT_BILLING_CYCLE_DAY_LOCAL;
     }
 

account/src/main/java/org/killbill/billing/account/api/svcs/DefaultImmutableAccountInternalApi.java

@@ -48,8 +48,8 @@ public class DefaultImmutableAccountInternalApi implements ImmutableAccountInter
 
     private final EntitySqlDaoTransactionalJdbiWrapper transactionalSqlDao;
     private final NonEntityDao nonEntityDao;
-    private final CacheControllerDispatcher cacheControllerDispatcher;
-    private final CacheController accountCacheController;
+    private final CacheController<Long, ImmutableAccountData> accountCacheController;
+    private final CacheController<String, Long> recordIdCacheController;
 
     @Inject
     public DefaultImmutableAccountInternalApi(final IDBI dbi,
@@ -59,20 +59,20 @@ public DefaultImmutableAccountInternalApi(final IDBI dbi,
         // This API will directly issue queries instead of relying on the DAO (introduced to avoid Guice circular dependencies with InternalCallContextFactory)
         this.transactionalSqlDao = new EntitySqlDaoTransactionalJdbiWrapper(dbi, clock, cacheControllerDispatcher, nonEntityDao, null);
         this.nonEntityDao = nonEntityDao;
-        this.cacheControllerDispatcher = cacheControllerDispatcher;
         this.accountCacheController = cacheControllerDispatcher.getCacheController(CacheType.ACCOUNT_IMMUTABLE);
+        this.recordIdCacheController = cacheControllerDispatcher.getCacheController(CacheType.RECORD_ID);
     }
 
     @Override
     public ImmutableAccountData getImmutableAccountDataById(final UUID accountId, final InternalTenantContext context) throws AccountApiException {
-        final Long recordId = nonEntityDao.retrieveRecordIdFromObject(accountId, ObjectType.ACCOUNT, cacheControllerDispatcher.getCacheController(CacheType.RECORD_ID));
+        final Long recordId = nonEntityDao.retrieveRecordIdFromObject(accountId, ObjectType.ACCOUNT, recordIdCacheController);
         return getImmutableAccountDataByRecordId(recordId, context);
     }
 
     @Override
     public ImmutableAccountData getImmutableAccountDataByRecordId(final Long recordId, final InternalTenantContext context) throws AccountApiException {
         final CacheLoaderArgument arg = createImmutableAccountCacheLoaderArgument(context);
-        return (ImmutableAccountData) accountCacheController.get(recordId, arg);
+        return accountCacheController.get(recordId, arg);
     }
 
     private CacheLoaderArgument createImmutableAccountCacheLoaderArgument(final InternalTenantContext context) {

account/src/main/java/org/killbill/billing/account/api/user/DefaultAccountApiBase.java

@@ -1,6 +1,6 @@
 /*
- * Copyright 2014-2015 Groupon, Inc
- * Copyright 2014-2015 The Billing Project, LLC
+ * Copyright 2014-2017 Groupon, Inc
+ * Copyright 2014-2017 The Billing Project, LLC
  *
  * 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
@@ -25,6 +25,7 @@
 import org.killbill.billing.account.api.AccountApiException;
 import org.killbill.billing.account.api.DefaultAccount;
 import org.killbill.billing.account.api.DefaultImmutableAccountData;
+import org.killbill.billing.account.api.ImmutableAccountData;
 import org.killbill.billing.account.dao.AccountDao;
 import org.killbill.billing.account.dao.AccountModelDao;
 import org.killbill.billing.callcontext.InternalTenantContext;
@@ -36,26 +37,26 @@
 public class DefaultAccountApiBase {
 
     private final AccountDao accountDao;
-    private final CacheControllerDispatcher cacheControllerDispatcher;
-    private final CacheController accountCacheController;
+    private final CacheController<Long, ImmutableAccountData> accountCacheController;
+    private final CacheController<String, Long> recordIdCacheController;
     private final NonEntityDao nonEntityDao;
 
     public DefaultAccountApiBase(final AccountDao accountDao,
                                  final NonEntityDao nonEntityDao,
                                  final CacheControllerDispatcher cacheControllerDispatcher) {
         this.accountDao = accountDao;
         this.nonEntityDao = nonEntityDao;
-        this.cacheControllerDispatcher = cacheControllerDispatcher;
         this.accountCacheController = cacheControllerDispatcher.getCacheController(CacheType.ACCOUNT_IMMUTABLE);
+        this.recordIdCacheController = cacheControllerDispatcher.getCacheController(CacheType.RECORD_ID);
     }
 
     protected Account getAccountById(final UUID accountId, final InternalTenantContext context) throws AccountApiException {
-        final Long recordId = nonEntityDao.retrieveRecordIdFromObject(accountId, ObjectType.ACCOUNT, cacheControllerDispatcher.getCacheController(CacheType.RECORD_ID));
+        final Long recordId = nonEntityDao.retrieveRecordIdFromObject(accountId, ObjectType.ACCOUNT, recordIdCacheController);
         final Account account = getAccountByRecordIdInternal(recordId, context);
         if (account == null) {
             throw new AccountApiException(ErrorCode.ACCOUNT_DOES_NOT_EXIST_FOR_ID, accountId);
         }
-        accountCacheController.putIfAbsent(accountId, new DefaultImmutableAccountData(account));
+        accountCacheController.putIfAbsent(recordId, new DefaultImmutableAccountData(account));
         return account;
     }
 
@@ -65,7 +66,8 @@ protected Account getAccountByKey(final String key, final InternalTenantContext
             throw new AccountApiException(ErrorCode.ACCOUNT_DOES_NOT_EXIST_FOR_KEY, key);
         }
         final Account account = new DefaultAccount(accountModelDao);
-        accountCacheController.putIfAbsent(account.getId(), new DefaultImmutableAccountData(account));
+        final Long recordId = nonEntityDao.retrieveRecordIdFromObject(account.getId(), ObjectType.ACCOUNT, recordIdCacheController);
+        accountCacheController.putIfAbsent(recordId, new DefaultImmutableAccountData(account));
         return account;
     }
 

beatrix/src/test/java/org/killbill/billing/beatrix/util/SubscriptionChecker.java

@@ -1,7 +1,9 @@
 /*
  * Copyright 2010-2013 Ning, Inc.
+ * Copyright 2014-2017 Groupon, Inc
+ * Copyright 2014-2017 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:
  *
@@ -22,6 +24,7 @@
 import javax.inject.Inject;
 
 import org.killbill.billing.util.cache.Cachable.CacheType;
+import org.killbill.billing.util.cache.CacheController;
 import org.killbill.billing.util.cache.CacheControllerDispatcher;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -46,14 +49,14 @@ public class SubscriptionChecker {
     private final SubscriptionBaseInternalApi subscriptionApi;
     private final AuditChecker auditChecker;
     private final NonEntityDao nonEntityDao;
-    private final CacheControllerDispatcher cacheControllerDispatcher;
+    private final CacheController<String, UUID> objectIdCacheController;
 
     @Inject
     public SubscriptionChecker(final SubscriptionBaseInternalApi subscriptionApi, final AuditChecker auditChecker, final NonEntityDao nonEntityDao, final CacheControllerDispatcher cacheControllerDispatcher) {
         this.subscriptionApi = subscriptionApi;
         this.auditChecker = auditChecker;
         this.nonEntityDao = nonEntityDao;
-        this.cacheControllerDispatcher = cacheControllerDispatcher;
+        objectIdCacheController = cacheControllerDispatcher.getCacheController(CacheType.OBJECT_ID);
     }
 
     public SubscriptionBaseBundle checkBundleNoAudits(final UUID bundleId, final UUID expectedAccountId, final String expectedKey, final InternalTenantContext context) throws SubscriptionBaseApiException {
@@ -65,7 +68,7 @@ public SubscriptionBaseBundle checkBundleNoAudits(final UUID bundleId, final UUI
     }
 
     public SubscriptionBase checkSubscriptionCreated(final UUID subscriptionId, final InternalCallContext context) throws SubscriptionBaseApiException {
-        final UUID tenantId = nonEntityDao.retrieveIdFromObject(context.getTenantRecordId(), ObjectType.TENANT, cacheControllerDispatcher.getCacheController(CacheType.OBJECT_ID));
+        final UUID tenantId = nonEntityDao.retrieveIdFromObject(context.getTenantRecordId(), ObjectType.TENANT, objectIdCacheController);
         final CallContext callContext = context.toCallContext(tenantId);
 
         final SubscriptionBase subscription = subscriptionApi.getSubscriptionFromId(subscriptionId, context);

catalog/src/main/java/org/killbill/billing/catalog/caching/EhCacheCatalogCache.java

@@ -53,7 +53,7 @@ public class EhCacheCatalogCache implements CatalogCache {
 
     private final Logger logger = LoggerFactory.getLogger(EhCacheCatalogCache.class);
 
-    private final CacheController cacheController;
+    private final CacheController<Long, Catalog> cacheController;
     private final VersionedCatalogLoader loader;
     private final CacheLoaderArgument cacheLoaderArgumentWithTemplateFiltering;
     private final CacheLoaderArgument cacheLoaderArgument;

catalog/src/main/java/org/killbill/billing/catalog/caching/EhCacheOverriddenPlanCache.java

@@ -48,7 +48,7 @@
 
 public class EhCacheOverriddenPlanCache implements OverriddenPlanCache {
 
-    private final CacheController cacheController;
+    private final CacheController<String, Plan> cacheController;
     private final LoaderCallback loaderCallback;
     private final CatalogOverrideDao overrideDao;
 

entitlement/src/main/java/org/killbill/billing/entitlement/dao/DefaultBlockingStateDao.java

@@ -1,7 +1,7 @@
 /*
  * Copyright 2010-2013 Ning, Inc.
- * Copyright 2014-2016 Groupon, Inc
- * Copyright 2014-2016 The Billing Project, LLC
+ * Copyright 2014-2017 Groupon, Inc
+ * Copyright 2014-2017 The Billing Project, LLC
  *
  * 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
@@ -36,7 +36,6 @@
 import org.killbill.billing.callcontext.InternalCallContext;
 import org.killbill.billing.callcontext.InternalTenantContext;
 import org.killbill.billing.entitlement.DefaultEntitlementService;
-import org.killbill.billing.entitlement.EntitlementService;
 import org.killbill.billing.entitlement.api.BlockingApiException;
 import org.killbill.billing.entitlement.api.BlockingState;
 import org.killbill.billing.entitlement.api.BlockingStateType;
@@ -46,6 +45,7 @@
 import org.killbill.billing.entitlement.block.StatelessBlockingChecker;
 import org.killbill.billing.entitlement.engine.core.BlockingTransitionNotificationKey;
 import org.killbill.billing.util.cache.Cachable.CacheType;
+import org.killbill.billing.util.cache.CacheController;
 import org.killbill.billing.util.cache.CacheControllerDispatcher;
 import org.killbill.billing.util.callcontext.InternalCallContextFactory;
 import org.killbill.billing.util.dao.NonEntityDao;
@@ -107,7 +107,7 @@ public int compare(final BlockingStateModelDao o1, final BlockingStateModelDao o
     private final Clock clock;
     private final NotificationQueueService notificationQueueService;
     private final PersistentBus eventBus;
-    private final CacheControllerDispatcher cacheControllerDispatcher;
+    private final CacheController<String, UUID> objectIdCacheController;
     private final NonEntityDao nonEntityDao;
 
     private final StatelessBlockingChecker statelessBlockingChecker = new StatelessBlockingChecker();
@@ -118,7 +118,7 @@ public DefaultBlockingStateDao(final IDBI dbi, final Clock clock, final Notifica
         this.clock = clock;
         this.notificationQueueService = notificationQueueService;
         this.eventBus = eventBus;
-        this.cacheControllerDispatcher = cacheControllerDispatcher;
+        this.objectIdCacheController = cacheControllerDispatcher.getCacheController(CacheType.OBJECT_ID);
         this.nonEntityDao = nonEntityDao;
     }
 
@@ -260,12 +260,12 @@ private BlockingAggregator getBlockedStatus(final BlockingStateSqlDao sqlDao, fi
         final List<BlockingState> bundleBlockingStates;
         final List<BlockingState> subscriptionBlockingStates;
         if (type == BlockingStateType.SUBSCRIPTION) {
-            final UUID accountId = nonEntityDao.retrieveIdFromObjectInTransaction(context.getAccountRecordId(), ObjectType.ACCOUNT, cacheControllerDispatcher.getCacheController(CacheType.OBJECT_ID), handle);
+            final UUID accountId = nonEntityDao.retrieveIdFromObjectInTransaction(context.getAccountRecordId(), ObjectType.ACCOUNT, objectIdCacheController, handle);
             accountBlockingStates = getBlockingState(sqlDao, accountId, BlockingStateType.ACCOUNT, upToDate, context);
             bundleBlockingStates = getBlockingState(sqlDao, bundleId, BlockingStateType.SUBSCRIPTION_BUNDLE, upToDate, context);
             subscriptionBlockingStates = getBlockingState(sqlDao, blockableId, BlockingStateType.SUBSCRIPTION, upToDate, context);
         } else if (type == BlockingStateType.SUBSCRIPTION_BUNDLE) {
-            final UUID accountId = nonEntityDao.retrieveIdFromObjectInTransaction(context.getAccountRecordId(), ObjectType.ACCOUNT, cacheControllerDispatcher.getCacheController(CacheType.OBJECT_ID), handle);
+            final UUID accountId = nonEntityDao.retrieveIdFromObjectInTransaction(context.getAccountRecordId(), ObjectType.ACCOUNT, objectIdCacheController, handle);
             accountBlockingStates = getBlockingState(sqlDao, accountId, BlockingStateType.ACCOUNT, upToDate, context);
             bundleBlockingStates = getBlockingState(sqlDao, blockableId, BlockingStateType.SUBSCRIPTION_BUNDLE, upToDate, context);
             subscriptionBlockingStates = ImmutableList.<BlockingState>of();

invoice/src/main/java/org/killbill/billing/invoice/dao/DefaultInvoiceDao.java

@@ -59,6 +59,7 @@
 import org.killbill.billing.tag.TagInternalApi;
 import org.killbill.billing.util.UUIDs;
 import org.killbill.billing.util.cache.Cachable.CacheType;
+import org.killbill.billing.util.cache.CacheController;
 import org.killbill.billing.util.cache.CacheControllerDispatcher;
 import org.killbill.billing.util.callcontext.InternalCallContextFactory;
 import org.killbill.billing.util.config.definition.InvoiceConfig;
@@ -115,7 +116,7 @@ public Comparable apply(final InvoiceModelDao invoice) {
     private final CBADao cbaDao;
     private final InvoiceConfig invoiceConfig;
     private final Clock clock;
-    private final CacheControllerDispatcher cacheControllerDispatcher;
+    private final CacheController<String, UUID> objectIdCacheController;
     private final NonEntityDao nonEntityDao;
     private final ParentInvoiceCommitmentPoster parentInvoiceCommitmentPoster;
     private final TagInternalApi tagInternalApi;
@@ -142,7 +143,7 @@ public DefaultInvoiceDao(final TagInternalApi tagInternalApi,
         this.invoiceDaoHelper = invoiceDaoHelper;
         this.cbaDao = cbaDao;
         this.clock = clock;
-        this.cacheControllerDispatcher = cacheControllerDispatcher;
+        this.objectIdCacheController = cacheControllerDispatcher.getCacheController(CacheType.OBJECT_ID);
         this.nonEntityDao = nonEntityDao;
         this.parentInvoiceCommitmentPoster = parentInvoiceCommitmentPoster;
     }
@@ -832,7 +833,7 @@ public boolean apply(final InvoicePaymentModelDao input) {
                 }
 
                 if (completion) {
-                    final UUID accountId = nonEntityDao.retrieveIdFromObjectInTransaction(context.getAccountRecordId(), ObjectType.ACCOUNT, cacheControllerDispatcher.getCacheController(CacheType.OBJECT_ID), entitySqlDaoWrapperFactory.getHandle());
+                    final UUID accountId = nonEntityDao.retrieveIdFromObjectInTransaction(context.getAccountRecordId(), ObjectType.ACCOUNT, objectIdCacheController, entitySqlDaoWrapperFactory.getHandle());
                     notifyBusOfInvoicePayment(entitySqlDaoWrapperFactory, invoicePayment, accountId, context.getUserToken(), context);
                 }
                 return null;

jaxrs/src/main/java/org/killbill/billing/jaxrs/resources/AdminResource.java

@@ -344,15 +344,17 @@ public Response invalidatesCache(@QueryParam("cacheName") final String cacheName
     @ApiResponses(value = {})
     public Response invalidatesCacheByAccount(@PathParam("accountId") final String accountIdStr,
                                               @javax.ws.rs.core.Context final HttpServletRequest request) {
+        final TenantContext tenantContext = context.createContext(request);
         final UUID accountId = UUID.fromString(accountIdStr);
+        final Long accountRecordId = recordIdApi.getRecordId(accountId, ObjectType.ACCOUNT, tenantContext);
 
         // clear account-record-id cache by accountId (note: String!)
         cacheControllerDispatcher.getCacheController(CacheType.ACCOUNT_RECORD_ID).remove(accountIdStr);
 
-        // clear account-immutable cache by accountId
-        cacheControllerDispatcher.getCacheController(CacheType.ACCOUNT_IMMUTABLE).remove(accountId);
+        // clear account-immutable cache by account record id
+        cacheControllerDispatcher.getCacheController(CacheType.ACCOUNT_IMMUTABLE).remove(accountRecordId);
 
-        // clear account-bcd cache by accountId
+        // clear account-bcd cache by accountId (note: UUID!)
         cacheControllerDispatcher.getCacheController(CacheType.ACCOUNT_BCD).remove(accountId);
 
         return Response.status(Status.OK).build();

overdue/src/main/java/org/killbill/billing/overdue/caching/EhCacheOverdueConfigCache.java

@@ -1,6 +1,6 @@
 /*
- * Copyright 2014-2015 Groupon, Inc
- * Copyright 2014-2015 The Billing Project, LLC
+ * Copyright 2014-2017 Groupon, Inc
+ * Copyright 2014-2017 The Billing Project, LLC
  *
  * 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
@@ -44,7 +44,7 @@ public class EhCacheOverdueConfigCache implements OverdueConfigCache {
 
     private static final Logger log = LoggerFactory.getLogger(EhCacheOverdueConfigCache.class);
 
-    private final CacheController cacheController;
+    private final CacheController<Long, OverdueConfig> cacheController;
     private final CacheLoaderArgument cacheLoaderArgument;
 
     private OverdueConfig defaultOverdueConfig;
@@ -97,7 +97,7 @@ public OverdueConfig getOverdueConfig(final InternalTenantContext tenantContext)
         // The cache loader might choke on some bad xml -- unlikely since we check its validity prior storing it,
         // but to be on the safe side;;
         try {
-            final OverdueConfig overdueConfig = (OverdueConfig) cacheController.get(tenantContext.getTenantRecordId(), cacheLoaderArgument);
+            final OverdueConfig overdueConfig = cacheController.get(tenantContext.getTenantRecordId(), cacheLoaderArgument);
             return (overdueConfig != null) ? overdueConfig : defaultOverdueConfig;
         } catch (final IllegalStateException e) {
             throw new OverdueApiException(ErrorCode.OVERDUE_INVALID_FOR_TENANT, tenantContext.getTenantRecordId());