killbill-aplcache

util: remove audit caches These were actually never plugged

2/5/2019 2:51:30 PM

Details

diff --git a/util/src/main/java/org/killbill/billing/util/cache/Cachable.java b/util/src/main/java/org/killbill/billing/util/cache/Cachable.java
index 240ff3c..3c64e76 100644
--- a/util/src/main/java/org/killbill/billing/util/cache/Cachable.java
+++ b/util/src/main/java/org/killbill/billing/util/cache/Cachable.java
@@ -1,7 +1,7 @@
 /*
  * Copyright 2010-2013 Ning, Inc.
- * Copyright 2014-2017 Groupon, Inc
- * Copyright 2014-2017 The Billing Project, LLC
+ * Copyright 2014-2019 Groupon, Inc
+ * Copyright 2014-2019 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
@@ -22,7 +22,6 @@ import java.lang.annotation.ElementType;
 import java.lang.annotation.Retention;
 import java.lang.annotation.RetentionPolicy;
 import java.lang.annotation.Target;
-import java.util.List;
 import java.util.UUID;
 
 import org.killbill.billing.account.api.ImmutableAccountData;
@@ -39,8 +38,6 @@ public @interface Cachable {
     String ACCOUNT_RECORD_ID_CACHE_NAME = "account-record-id";
     String TENANT_RECORD_ID_CACHE_NAME = "tenant-record-id";
     String OBJECT_ID_CACHE_NAME = "object-id";
-    String AUDIT_LOG_CACHE_NAME = "audit-log";
-    String AUDIT_LOG_VIA_HISTORY_CACHE_NAME = "audit-log-via-history";
     String TENANT_CATALOG_CACHE_NAME = "tenant-catalog";
     String TENANT_PAYMENT_STATE_MACHINE_CONFIG_CACHE_NAME = "tenant-payment-state-machine-config";
     String TENANT_OVERDUE_CONFIG_CACHE_NAME = "tenant-overdue-config";
@@ -70,12 +67,6 @@ public @interface Cachable {
         /* Mapping from object 'recordId (Long as String)' -> object 'id (UUID)'  */
         OBJECT_ID(OBJECT_ID_CACHE_NAME, String.class, UUID.class, true),
 
-        /* Mapping from object 'tableName::targetRecordId' -> matching objects 'List<AuditLogModelDao>' */
-        AUDIT_LOG(AUDIT_LOG_CACHE_NAME, String.class, List.class, true),
-
-        /* Mapping from object 'tableName::historyTableName::targetRecordId' -> matching objects 'List<AuditLogModelDao>' */
-        AUDIT_LOG_VIA_HISTORY(AUDIT_LOG_VIA_HISTORY_CACHE_NAME, String.class, List.class, true),
-
         /* Tenant catalog cache */
         TENANT_CATALOG(TENANT_CATALOG_CACHE_NAME, Long.class, Catalog.class, false),
 
diff --git a/util/src/main/java/org/killbill/billing/util/dao/AuditSqlDao.java b/util/src/main/java/org/killbill/billing/util/dao/AuditSqlDao.java
index 9e06d81..c980a98 100644
--- a/util/src/main/java/org/killbill/billing/util/dao/AuditSqlDao.java
+++ b/util/src/main/java/org/killbill/billing/util/dao/AuditSqlDao.java
@@ -24,9 +24,6 @@ import java.util.List;
 import org.killbill.billing.callcontext.InternalCallContext;
 import org.killbill.billing.callcontext.InternalTenantContext;
 import org.killbill.billing.util.audit.dao.AuditLogModelDao;
-import org.killbill.billing.util.cache.Cachable;
-import org.killbill.billing.util.cache.Cachable.CacheType;
-import org.killbill.billing.util.cache.CachableKey;
 import org.killbill.commons.jdbi.binder.SmartBindBean;
 import org.killbill.commons.jdbi.statement.SmartFetchSize;
 import org.killbill.commons.jdbi.template.KillBillSqlDaoStringTemplate;
@@ -37,13 +34,7 @@ import org.skife.jdbi.v2.sqlobject.customizers.BatchChunkSize;
 import org.skife.jdbi.v2.sqlobject.customizers.Define;
 
 /**
- * Note 1: cache invalidation has to happen for audit logs (which is tricky in the multi-nodes scenario).
- * For now, we're using a time-based eviction strategy (see ehcache.xml)
- * which is good enough: the cache will always get at least the initial CREATION audit log entry, which is the one
- * we really care about (both for Analytics and for Kaui's endpoints). Besides, we do cache invalidation properly
- * on our own node (see EntitySqlDaoWrapperInvocationHandler).
- * <p/>
- * Note 2: in the queries below, tableName always refers to the TableName enum, not the actual table name (TableName.getTableName()).
+ * Note: in the queries below, tableName always refers to the TableName enum, not the actual table name (TableName.getTableName()).
  */
 @KillBillSqlDaoStringTemplate("/org/killbill/billing/util/entity/dao/EntitySqlDao.sql.stg")
 // Note: @RegisterMapper annotation won't work here as we build the SqlObject via EntitySqlDao (annotations won't be inherited for JDBI)
@@ -64,15 +55,13 @@ public interface AuditSqlDao {
                                                                                  @SmartBindBean final InternalTenantContext context);
 
     @SqlQuery
-    @Cachable(CacheType.AUDIT_LOG)
-    public List<AuditLogModelDao> getAuditLogsForTargetRecordId(@CachableKey(1) @Bind("tableName") final String tableName,
-                                                                @CachableKey(2) @Bind("targetRecordId") final long targetRecordId,
+    public List<AuditLogModelDao> getAuditLogsForTargetRecordId(@Bind("tableName") final String tableName,
+                                                                @Bind("targetRecordId") final long targetRecordId,
                                                                 @SmartBindBean final InternalTenantContext context);
 
     @SqlQuery
-    @Cachable(CacheType.AUDIT_LOG_VIA_HISTORY)
-    public List<AuditLogModelDao> getAuditLogsViaHistoryForTargetRecordId(@CachableKey(1) @Bind("tableName") final String historyTableName, /* Uppercased - used to find entries in audit_log table */
-                                                                          @CachableKey(2) @Define("historyTableName") final String actualHistoryTableName, /* Actual table name, used in the inner join query */
-                                                                          @CachableKey(3) @Bind("targetRecordId") final long targetRecordId,
+    public List<AuditLogModelDao> getAuditLogsViaHistoryForTargetRecordId(@Bind("tableName") final String historyTableName, /* Uppercased - used to find entries in audit_log table */
+                                                                          @Define("historyTableName") final String actualHistoryTableName, /* Actual table name, used in the inner join query */
+                                                                          @Bind("targetRecordId") final long targetRecordId,
                                                                           @SmartBindBean final InternalTenantContext context);
 }
diff --git a/util/src/main/java/org/killbill/billing/util/entity/dao/EntitySqlDaoWrapperInvocationHandler.java b/util/src/main/java/org/killbill/billing/util/entity/dao/EntitySqlDaoWrapperInvocationHandler.java
index 1b3ec7a..21c7d38 100644
--- a/util/src/main/java/org/killbill/billing/util/entity/dao/EntitySqlDaoWrapperInvocationHandler.java
+++ b/util/src/main/java/org/killbill/billing/util/entity/dao/EntitySqlDaoWrapperInvocationHandler.java
@@ -66,7 +66,6 @@ import com.google.common.base.MoreObjects;
 import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableList.Builder;
-import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.Iterables;
 
 /**
@@ -449,41 +448,6 @@ public class EntitySqlDaoWrapperInvocationHandler<S extends EntitySqlDao<M, E>, 
 
         sqlDao.insertAuditsFromTransaction(audits, context);
         printSQLWarnings();
-
-        // We need to invalidate the caches. There is a small window of doom here where caches will be stale.
-        for (final M entityModelDao : entityModelDaoAndHistoryRecordIds.keySet()) {
-            final Long entityRecordId = entityModelDao.getRecordId();
-
-            // TODO Knowledge on how the key is constructed is also in AuditSqlDao
-            if (tableName.getHistoryTableName() != null) {
-                final CacheController<String, List> cacheController = cacheControllerDispatcher.getCacheController(CacheType.AUDIT_LOG_VIA_HISTORY);
-                if (cacheController != null) {
-                    final String key = buildCacheKey(ImmutableMap.<Integer, Object>of(0, tableName.getHistoryTableName(), 1, tableName.getHistoryTableName(), 2, entityRecordId));
-                    cacheController.remove(key);
-                }
-            } else {
-                final CacheController<String, List> cacheController = cacheControllerDispatcher.getCacheController(CacheType.AUDIT_LOG);
-                if (cacheController != null) {
-                    final String key = buildCacheKey(ImmutableMap.<Integer, Object>of(0, tableName, 1, entityRecordId));
-                    cacheController.remove(key);
-                }
-            }
-        }
-    }
-
-    private String buildCacheKey(final Map<Integer, Object> keyPieces) {
-        final StringBuilder cacheKey = new StringBuilder();
-        for (int i = 0; i < keyPieces.size(); i++) {
-            // To normalize the arguments and avoid casing issues, we make all pieces of the key uppercase.
-            // Since the database engine may be case insensitive and we use arguments of the SQL method call
-            // to build the key, the key has to be case insensitive as well.
-            final String str = String.valueOf(keyPieces.get(i)).toUpperCase();
-            cacheKey.append(str);
-            if (i < keyPieces.size() - 1) {
-                cacheKey.append(CacheControllerDispatcher.CACHE_KEY_SEPARATOR);
-            }
-        }
-        return cacheKey.toString();
     }
 
     private String getProfilingId(@Nullable final String prefix, @Nullable final Method method) {
diff --git a/util/src/main/java/org/killbill/billing/util/glue/CacheModule.java b/util/src/main/java/org/killbill/billing/util/glue/CacheModule.java
index 8289c70..5801a70 100644
--- a/util/src/main/java/org/killbill/billing/util/glue/CacheModule.java
+++ b/util/src/main/java/org/killbill/billing/util/glue/CacheModule.java
@@ -1,7 +1,7 @@
 /*
  * Copyright 2010-2013 Ning, Inc.
- * Copyright 2014-2018 Groupon, Inc
- * Copyright 2014-2018 The Billing Project, LLC
+ * Copyright 2014-2019 Groupon, Inc
+ * Copyright 2014-2019 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
@@ -24,8 +24,6 @@ import org.killbill.billing.platform.api.KillbillConfigSource;
 import org.killbill.billing.util.cache.AccountBCDCacheLoader;
 import org.killbill.billing.util.cache.AccountIdFromBundleIdCacheLoader;
 import org.killbill.billing.util.cache.AccountRecordIdCacheLoader;
-import org.killbill.billing.util.cache.AuditLogCacheLoader;
-import org.killbill.billing.util.cache.AuditLogViaHistoryCacheLoader;
 import org.killbill.billing.util.cache.BaseCacheLoader;
 import org.killbill.billing.util.cache.BundleIdFromSubscriptionIdCacheLoader;
 import org.killbill.billing.util.cache.CacheControllerDispatcher;
@@ -84,8 +82,6 @@ public class CacheModule extends KillBillModule {
         resultSetMapperSetBinder.addBinding().to(AccountRecordIdCacheLoader.class).asEagerSingleton();
         resultSetMapperSetBinder.addBinding().to(TenantRecordIdCacheLoader.class).asEagerSingleton();
         resultSetMapperSetBinder.addBinding().to(ObjectIdCacheLoader.class).asEagerSingleton();
-        resultSetMapperSetBinder.addBinding().to(AuditLogCacheLoader.class).asEagerSingleton();
-        resultSetMapperSetBinder.addBinding().to(AuditLogViaHistoryCacheLoader.class).asEagerSingleton();
         resultSetMapperSetBinder.addBinding().to(TenantCatalogCacheLoader.class).asEagerSingleton();
         resultSetMapperSetBinder.addBinding().to(TenantConfigCacheLoader.class).asEagerSingleton();
         resultSetMapperSetBinder.addBinding().to(TenantOverdueConfigCacheLoader.class).asEagerSingleton();
diff --git a/util/src/main/resources/ehcache.xml b/util/src/main/resources/ehcache.xml
index b2d732c..e7ec84b 100644
--- a/util/src/main/resources/ehcache.xml
+++ b/util/src/main/resources/ehcache.xml
@@ -2,8 +2,8 @@
 
 <!--
   ~ Copyright 2010-2014 Ning, Inc.
-  ~ Copyright 2014-2017 Groupon, Inc
-  ~ Copyright 2014-2017 The Billing Project, LLC
+  ~ Copyright 2014-2019 Groupon, Inc
+  ~ Copyright 2014-2019 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
@@ -30,22 +30,9 @@
                                     http://www.ehcache.org/schema/ehcache-clustered-ext-3.3.xsd">
     <ehcache:service>
         <jsr107:defaults default-template="defaultCacheConfiguration" enable-management="true" enable-statistics="true">
-            <!-- See AuditSqlDao -->
-            <jsr107:cache name="audit-log" template="defaultShortTTLCacheConfiguration"/>
-            <jsr107:cache name="audit-log-via-history" template="defaultShortTTLCacheConfiguration"/>
         </jsr107:defaults>
     </ehcache:service>
 
-    <ehcache:cache-template name="defaultShortTTLCacheConfiguration">
-        <ehcache:expiry>
-            <ehcache:ttl unit="seconds">20</ehcache:ttl>
-        </ehcache:expiry>
-
-        <ehcache:resources>
-            <ehcache:heap unit="entries">100000</ehcache:heap>
-        </ehcache:resources>
-    </ehcache:cache-template>
-
     <ehcache:cache-template name="defaultCacheConfiguration">
         <ehcache:expiry>
             <ehcache:none/>
diff --git a/util/src/test/java/org/killbill/billing/util/tag/dao/TestDefaultTagDao.java b/util/src/test/java/org/killbill/billing/util/tag/dao/TestDefaultTagDao.java
index 2d3f9b6..d65aa5b 100644
--- a/util/src/test/java/org/killbill/billing/util/tag/dao/TestDefaultTagDao.java
+++ b/util/src/test/java/org/killbill/billing/util/tag/dao/TestDefaultTagDao.java
@@ -1,7 +1,9 @@
 /*
- * Copyright 2010-2012 Ning, Inc.
+ * Copyright 2010-2013 Ning, Inc.
+ * Copyright 2014-2019 Groupon, Inc
+ * Copyright 2014-2019 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:
  *
@@ -20,6 +22,10 @@ import java.util.ArrayList;
 import java.util.List;
 import java.util.UUID;
 
+import org.killbill.billing.util.api.AuditLevel;
+import org.killbill.billing.util.audit.AuditLog;
+import org.killbill.billing.util.audit.ChangeType;
+import org.killbill.billing.util.dao.TableName;
 import org.testng.Assert;
 import org.testng.annotations.Test;
 
@@ -130,12 +136,20 @@ public class TestDefaultTagDao extends UtilTestSuiteWithEmbeddedDB {
         Assert.assertEquals(createdTagDefinition.getDescription(), description);
         assertListenerStatus();
 
+        final List<AuditLog> auditLogs1 = auditDao.getAuditLogsForId(TableName.TAG_DEFINITIONS, createdTagDefinition.getId(), AuditLevel.FULL, internalCallContext);
+        Assert.assertEquals(auditLogs1.size(), 1);
+        Assert.assertEquals(auditLogs1.get(0).getChangeType(), ChangeType.INSERT);
+
         // Make sure we can create a tag
         eventsListener.pushExpectedEvent(NextEvent.TAG);
         final Tag tag = new DescriptiveTag(createdTagDefinition.getId(), objectType, objectId, internalCallContext.getCreatedDate());
         tagDao.create(new TagModelDao(tag), internalCallContext);
         assertListenerStatus();
 
+        final List<AuditLog> auditLogs2 = auditDao.getAuditLogsForId(TableName.TAG, tag.getId(), AuditLevel.FULL, internalCallContext);
+        Assert.assertEquals(auditLogs2.size(), 1);
+        Assert.assertEquals(auditLogs2.get(0).getChangeType(), ChangeType.INSERT);
+
         // Make sure we can retrieve it via the DAO
         final List<TagModelDao> foundTags = tagDao.getTagsForObject(objectId, objectType, false, internalCallContext);
         Assert.assertEquals(foundTags.size(), 1);
@@ -149,6 +163,11 @@ public class TestDefaultTagDao extends UtilTestSuiteWithEmbeddedDB {
         tagDao.deleteTag(objectId, objectType, createdTagDefinition.getId(), internalCallContext);
         assertListenerStatus();
 
+        final List<AuditLog> auditLogs3 = auditDao.getAuditLogsForId(TableName.TAG, tag.getId(), AuditLevel.FULL, internalCallContext);
+        Assert.assertEquals(auditLogs3.size(), 2);
+        Assert.assertEquals(auditLogs3.get(0).getChangeType(), ChangeType.INSERT);
+        Assert.assertEquals(auditLogs3.get(1).getChangeType(), ChangeType.DELETE);
+
         // Make sure the tag is deleted
         Assert.assertEquals(tagDao.getTagsForObject(objectId, objectType, false, internalCallContext).size(), 0);
         Assert.assertEquals(tagDao.getTagsForAccount(false, internalCallContext).size(), 0);
@@ -184,5 +203,4 @@ public class TestDefaultTagDao extends UtilTestSuiteWithEmbeddedDB {
             Assert.assertEquals(ErrorCode.TAG_ALREADY_EXISTS.getCode(), e.getCode());
         }
     }
-
 }