killbill-memoizeit

See #351 Defer serialization of plugin properties for the

8/5/2015 8:27:32 PM

Details

diff --git a/payment/src/main/java/org/killbill/billing/payment/core/sm/control/DefaultControlCompleted.java b/payment/src/main/java/org/killbill/billing/payment/core/sm/control/DefaultControlCompleted.java
index 86a22f1..6655210 100644
--- a/payment/src/main/java/org/killbill/billing/payment/core/sm/control/DefaultControlCompleted.java
+++ b/payment/src/main/java/org/killbill/billing/payment/core/sm/control/DefaultControlCompleted.java
@@ -26,13 +26,17 @@ import org.killbill.automaton.State;
 import org.killbill.automaton.State.EnteringStateCallback;
 import org.killbill.automaton.State.LeavingStateCallback;
 import org.killbill.billing.ObjectType;
+import org.killbill.billing.payment.api.PluginProperty;
 import org.killbill.billing.payment.core.sm.PluginControlPaymentAutomatonRunner;
 import org.killbill.billing.payment.api.TransactionStatus;
 import org.killbill.billing.payment.dao.PaymentAttemptModelDao;
 import org.killbill.billing.payment.dao.PaymentTransactionModelDao;
+import org.killbill.billing.payment.dao.PluginPropertySerializer;
+import org.killbill.billing.payment.dao.PluginPropertySerializer.PluginPropertySerializerException;
 import org.killbill.billing.payment.retry.BaseRetryService.RetryServiceScheduler;
 
 import com.google.common.base.Predicate;
+import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Iterables;
 
 public class DefaultControlCompleted implements EnteringStateCallback {
@@ -56,14 +60,26 @@ public class DefaultControlCompleted implements EnteringStateCallback {
         final UUID transactionId = paymentStateContext.getCurrentTransaction() != null ?
                                    paymentStateContext.getCurrentTransaction().getId() :
                                    null;
-        retryablePaymentAutomatonRunner.getPaymentDao().updatePaymentAttempt(attempt.getId(), transactionId, state.getName(), paymentStateContext.getInternalCallContext());
+
 
         if (retriedState.getName().equals(state.getName()) && !isUnknownTransaction()) {
+            retryablePaymentAutomatonRunner.getPaymentDao().updatePaymentAttemptWithProperties(attempt.getId(), transactionId, state.getName(), getSerializedProperties(), paymentStateContext.getInternalCallContext());
             retryServiceScheduler.scheduleRetry(ObjectType.PAYMENT_ATTEMPT, attempt.getId(), attempt.getId(), attempt.getTenantRecordId(),
                                                 paymentStateContext.getPaymentControlPluginNames(), paymentStateContext.getRetryDate());
+        } else {
+            retryablePaymentAutomatonRunner.getPaymentDao().updatePaymentAttempt(attempt.getId(), transactionId, state.getName(), paymentStateContext.getInternalCallContext());
+        }
+    }
+
+    private byte[] getSerializedProperties() {
+        try {
+            return PluginPropertySerializer.serialize(paymentStateContext.getProperties());
+        } catch (final PluginPropertySerializerException e) {
+            throw new IllegalStateException(e);
         }
     }
 
+
     //
     // If we see an UNKNOWN transaction we prevent it to be rescheduled as the Janitor will *try* to fix it, and that could lead to infinite retries from a badly behaved plugin
     // (In other words, plugin should ONLY retry 'known' transaction)
diff --git a/payment/src/main/java/org/killbill/billing/payment/core/sm/control/DefaultControlInitiated.java b/payment/src/main/java/org/killbill/billing/payment/core/sm/control/DefaultControlInitiated.java
index 8086967..ea31f37 100644
--- a/payment/src/main/java/org/killbill/billing/payment/core/sm/control/DefaultControlInitiated.java
+++ b/payment/src/main/java/org/killbill/billing/payment/core/sm/control/DefaultControlInitiated.java
@@ -21,6 +21,7 @@ import org.joda.time.DateTime;
 import org.killbill.automaton.OperationException;
 import org.killbill.automaton.State;
 import org.killbill.automaton.State.LeavingStateCallback;
+import org.killbill.billing.payment.api.PluginProperty;
 import org.killbill.billing.payment.api.TransactionType;
 import org.killbill.billing.payment.core.sm.PaymentStateContext;
 import org.killbill.billing.payment.core.sm.PluginControlPaymentAutomatonRunner;
@@ -33,6 +34,7 @@ import org.killbill.billing.payment.dao.PluginPropertySerializer.PluginPropertyS
 import org.killbill.billing.util.UUIDs;
 
 import com.google.common.base.Preconditions;
+import com.google.common.collect.ImmutableList;
 
 public class DefaultControlInitiated implements LeavingStateCallback {
 
@@ -74,8 +76,13 @@ public class DefaultControlInitiated implements LeavingStateCallback {
 
         if (state.getName().equals(initialState.getName()) || state.getName().equals(retriedState.getName())) {
             try {
-                final byte[] serializedProperties = PluginPropertySerializer.serialize(stateContext.getProperties());
-
+                //
+                // We don't serialize any properties at this stage to avoid serializing sensitive information.
+                // However, if after going through the control plugins, the attempt end up in RETRIED state,
+                // the properties will be serialized in the enteringState() callback (any plugin that sets a
+                // retried date is responsible to correctly remove sensitive information such as CVV, ...)
+                //
+                final byte[] serializedProperties = PluginPropertySerializer.serialize(ImmutableList.<PluginProperty>of());
                 final PaymentAttemptModelDao attempt = new PaymentAttemptModelDao(stateContext.getAccount().getId(), stateContext.getPaymentMethodId(),
                                                                                   utcNow, utcNow, stateContext.getPaymentExternalKey(), stateContext.getTransactionId(),
                                                                                   stateContext.getPaymentTransactionExternalKey(), transactionType, initialState.getName(),
diff --git a/payment/src/main/java/org/killbill/billing/payment/dao/DefaultPaymentDao.java b/payment/src/main/java/org/killbill/billing/payment/dao/DefaultPaymentDao.java
index ff64501..a03d1eb 100644
--- a/payment/src/main/java/org/killbill/billing/payment/dao/DefaultPaymentDao.java
+++ b/payment/src/main/java/org/killbill/billing/payment/dao/DefaultPaymentDao.java
@@ -109,7 +109,6 @@ public class DefaultPaymentDao implements PaymentDao {
     @Override
     public void updatePaymentAttempt(final UUID paymentAttemptId, @Nullable final UUID transactionId, final String state, final InternalCallContext context) {
         transactionalSqlDao.execute(new EntitySqlDaoTransactionWrapper<Void>() {
-
             @Override
             public Void inTransaction(final EntitySqlDaoWrapperFactory entitySqlDaoWrapperFactory) throws Exception {
                 final String transactionIdStr = transactionId != null ? transactionId.toString() : null;
@@ -121,6 +120,20 @@ public class DefaultPaymentDao implements PaymentDao {
     }
 
     @Override
+    public void updatePaymentAttemptWithProperties(final UUID paymentAttemptId, final UUID transactionId, final String state, final byte[] pluginProperties, final InternalCallContext context) {
+        transactionalSqlDao.execute(new EntitySqlDaoTransactionWrapper<Void>() {
+
+            @Override
+            public Void inTransaction(final EntitySqlDaoWrapperFactory entitySqlDaoWrapperFactory) throws Exception {
+                final String transactionIdStr = transactionId != null ? transactionId.toString() : null;
+                final PaymentAttemptSqlDao transactional = entitySqlDaoWrapperFactory.become(PaymentAttemptSqlDao.class);
+                transactional.updateAttemptWithProperties(paymentAttemptId.toString(), transactionIdStr, state, pluginProperties, context);
+                return null;
+            }
+        });
+    }
+
+    @Override
     public Pagination<PaymentAttemptModelDao> getPaymentAttemptsByStateAcrossTenants(final String stateName, final DateTime createdBeforeDate, final Long offset, final Long limit) {
 
         final Date createdBefore = createdBeforeDate.toDate();
diff --git a/payment/src/main/java/org/killbill/billing/payment/dao/PaymentAttemptSqlDao.java b/payment/src/main/java/org/killbill/billing/payment/dao/PaymentAttemptSqlDao.java
index cc2959e..4b25a00 100644
--- a/payment/src/main/java/org/killbill/billing/payment/dao/PaymentAttemptSqlDao.java
+++ b/payment/src/main/java/org/killbill/billing/payment/dao/PaymentAttemptSqlDao.java
@@ -42,6 +42,14 @@ public interface PaymentAttemptSqlDao extends EntitySqlDao<PaymentAttemptModelDa
                        @Bind("stateName") final String stateName,
                        @BindBean final InternalCallContext context);
 
+    @SqlUpdate
+    @Audited(ChangeType.UPDATE)
+    void updateAttemptWithProperties(@Bind("id") final String attemptId,
+                                     @Bind("transactionId") final String transactionId,
+                                     @Bind("stateName") final String stateName,
+                                     @Bind("pluginProperties") final byte[] pluginProperties,
+                                     @BindBean final InternalCallContext context);
+
     @SqlQuery
     List<PaymentAttemptModelDao> getByTransactionExternalKey(@Bind("transactionExternalKey") final String transactionExternalKey,
                                                              @BindBean final InternalTenantContext context);
diff --git a/payment/src/main/java/org/killbill/billing/payment/dao/PaymentDao.java b/payment/src/main/java/org/killbill/billing/payment/dao/PaymentDao.java
index 535a623..69b6941 100644
--- a/payment/src/main/java/org/killbill/billing/payment/dao/PaymentDao.java
+++ b/payment/src/main/java/org/killbill/billing/payment/dao/PaymentDao.java
@@ -36,6 +36,8 @@ public interface PaymentDao {
 
     public void updatePaymentAttempt(UUID paymentAttemptId, UUID transactionId, String state, InternalCallContext context);
 
+    public void updatePaymentAttemptWithProperties(UUID paymentAttemptId, UUID transactionId, String state, final byte[] pluginProperties, InternalCallContext context);
+
     public Pagination<PaymentAttemptModelDao> getPaymentAttemptsByStateAcrossTenants(String stateName, DateTime createdBeforeDate, final Long offset, final Long limit);
 
     public List<PaymentAttemptModelDao> getPaymentAttempts(String paymentExternalKey, InternalTenantContext context);
diff --git a/payment/src/main/resources/org/killbill/billing/payment/dao/PaymentAttemptSqlDao.sql.stg b/payment/src/main/resources/org/killbill/billing/payment/dao/PaymentAttemptSqlDao.sql.stg
index 154bb91..614e2fc 100644
--- a/payment/src/main/resources/org/killbill/billing/payment/dao/PaymentAttemptSqlDao.sql.stg
+++ b/payment/src/main/resources/org/killbill/billing/payment/dao/PaymentAttemptSqlDao.sql.stg
@@ -99,6 +99,18 @@ where id = :id
 ;
 >>
 
+updateAttemptWithProperties() ::= <<
+update <tableName()>
+set state_name = :stateName
+, transaction_id = :transactionId
+, plugin_properties = :pluginProperties
+, updated_by = :updatedBy
+, updated_date = :createdDate
+where id = :id
+<AND_CHECK_TENANT()>
+;
+>>
+
 
 
 
diff --git a/payment/src/test/java/org/killbill/billing/payment/dao/MockPaymentDao.java b/payment/src/test/java/org/killbill/billing/payment/dao/MockPaymentDao.java
index 7109927..b0ac283 100644
--- a/payment/src/test/java/org/killbill/billing/payment/dao/MockPaymentDao.java
+++ b/payment/src/test/java/org/killbill/billing/payment/dao/MockPaymentDao.java
@@ -112,6 +112,11 @@ public class MockPaymentDao implements PaymentDao {
     }
 
     @Override
+    public void updatePaymentAttemptWithProperties(final UUID paymentAttemptId, final UUID transactionId, final String state, final byte[] pluginProperties, final InternalCallContext context) {
+        updatePaymentAttempt(paymentAttemptId, transactionId, state, context);
+    }
+
+    @Override
     public Pagination<PaymentAttemptModelDao> getPaymentAttemptsByStateAcrossTenants(final String stateName, final DateTime createdBeforeDate, final Long offset, final Long limit) {
         return null;
     }
diff --git a/payment/src/test/java/org/killbill/billing/payment/dao/TestPaymentDao.java b/payment/src/test/java/org/killbill/billing/payment/dao/TestPaymentDao.java
index 9afbb45..93e85a2 100644
--- a/payment/src/test/java/org/killbill/billing/payment/dao/TestPaymentDao.java
+++ b/payment/src/test/java/org/killbill/billing/payment/dao/TestPaymentDao.java
@@ -269,7 +269,7 @@ public class TestPaymentDao extends PaymentTestSuiteWithEmbeddedDB {
                                                                                        TransactionStatus.PENDING, BigDecimal.TEN, Currency.AED,
                                                                                        "pending", "");
 
-        final PaymentModelDao payment  = paymentDao.insertPaymentWithFirstTransaction(paymentModelDao, transaction1, internalCallContext);
+        final PaymentModelDao payment = paymentDao.insertPaymentWithFirstTransaction(paymentModelDao, transaction1, internalCallContext);
 
         final PaymentTransactionModelDao transaction2 = new PaymentTransactionModelDao(initialTime, initialTime, null, transactionExternalKey2,
                                                                                        paymentModelDao.getId(), TransactionType.AUTHORIZE, initialTime,
@@ -375,7 +375,6 @@ public class TestPaymentDao extends PaymentTestSuiteWithEmbeddedDB {
                                                                      createdDate1);
         paymentDao.insertPaymentWithFirstTransaction(paymentModelDao1, transaction1, context1);
 
-
         // Right after createdAfterDate, so it should  be returned
         final DateTime createdDate2 = createdAfterDate.plusHours(1);
         final PaymentModelDao paymentModelDao2 = new PaymentModelDao(createdDate2, createdDate2, accountId, paymentMethodId, externalKey2);
@@ -419,7 +418,6 @@ public class TestPaymentDao extends PaymentTestSuiteWithEmbeddedDB {
 
         paymentDao.insertPaymentWithFirstTransaction(paymentModelDao3, transaction3, context3);
 
-
         // Right before createdBeforeDate but with a SUCCESS state so it should NOT be returned
         final DateTime createdDate4 = createdBeforeDate.minusDays(1);
         final PaymentModelDao paymentModelDao4 = new PaymentModelDao(createdDate4, createdDate4, accountId, paymentMethodId, externalKey4);
@@ -469,7 +467,6 @@ public class TestPaymentDao extends PaymentTestSuiteWithEmbeddedDB {
         assertEquals(result.size(), 2);
     }
 
-
     @Test(groups = "slow")
     public void testPaginationForPaymentByStatesAcrossTenants() {
         // Right before createdAfterDate, so should not be returned
@@ -496,7 +493,7 @@ public class TestPaymentDao extends PaymentTestSuiteWithEmbeddedDB {
             paymentDao.insertPaymentWithFirstTransaction(paymentModelDao1, transaction1, context1);
         }
 
-        final Pagination<PaymentTransactionModelDao> result =  paymentDao.getByTransactionStatusAcrossTenants(ImmutableList.of(TransactionStatus.UNKNOWN), clock.getUTCNow(), createdDate1, 0L, new Long(NB_ENTRIES));
+        final Pagination<PaymentTransactionModelDao> result = paymentDao.getByTransactionStatusAcrossTenants(ImmutableList.of(TransactionStatus.UNKNOWN), clock.getUTCNow(), createdDate1, 0L, new Long(NB_ENTRIES));
         Assert.assertEquals(result.getTotalNbRecords(), new Long(NB_ENTRIES));
 
         final Iterator<PaymentTransactionModelDao> iterator = result.iterator();
@@ -508,7 +505,7 @@ public class TestPaymentDao extends PaymentTestSuiteWithEmbeddedDB {
         }
     }
 
-        @Test(groups = "slow")
+    @Test(groups = "slow")
     public void testPaymentAttemptsByStateAcrossTenants() {
 
         final UUID paymentMethodId = UUID.randomUUID();
@@ -526,13 +523,12 @@ public class TestPaymentDao extends PaymentTestSuiteWithEmbeddedDB {
         final String pluginName = "miraculous";
 
         final PaymentAttemptModelDao attempt1 = new PaymentAttemptModelDao(accountId, paymentMethodId, createdAfterDate, createdAfterDate, externalKey1,
-                                                                     UUID.randomUUID(), transactionExternalKey1, TransactionType.AUTHORIZE, stateName, BigDecimal.ONE, Currency.USD,
-                                                                     ImmutableList.<String>of(pluginName), null);
-
+                                                                           UUID.randomUUID(), transactionExternalKey1, TransactionType.AUTHORIZE, stateName, BigDecimal.ONE, Currency.USD,
+                                                                           ImmutableList.<String>of(pluginName), null);
 
         final PaymentAttemptModelDao attempt2 = new PaymentAttemptModelDao(accountId, paymentMethodId, createdAfterDate, createdAfterDate, externalKey2,
-                                                                     UUID.randomUUID(), transactionExternalKey2, TransactionType.AUTHORIZE, stateName, BigDecimal.ONE, Currency.USD,
-                                                                     ImmutableList.<String>of(pluginName), null);
+                                                                           UUID.randomUUID(), transactionExternalKey2, TransactionType.AUTHORIZE, stateName, BigDecimal.ONE, Currency.USD,
+                                                                           ImmutableList.<String>of(pluginName), null);
 
         final InternalCallContext context1 = new InternalCallContext(1L,
                                                                      1L,
@@ -546,7 +542,6 @@ public class TestPaymentDao extends PaymentTestSuiteWithEmbeddedDB {
                                                                      createdAfterDate);
         paymentDao.insertPaymentAttemptWithProperties(attempt1, context1);
 
-
         final InternalCallContext context2 = new InternalCallContext(2L,
                                                                      2L,
                                                                      internalCallContext.getUserToken(),
@@ -559,11 +554,64 @@ public class TestPaymentDao extends PaymentTestSuiteWithEmbeddedDB {
                                                                      createdAfterDate);
         paymentDao.insertPaymentAttemptWithProperties(attempt2, context2);
 
-
         final Pagination<PaymentAttemptModelDao> result = paymentDao.getPaymentAttemptsByStateAcrossTenants(stateName, createdBeforeDate, 0L, 2L);
         Assert.assertEquals(result.getTotalNbRecords().longValue(), 2L);
     }
 
+    @Test(groups = "slow")
+    public void testUpdatePaymentAttempt() throws PluginPropertySerializerException {
+
+        final UUID paymentMethodId = UUID.randomUUID();
+        final UUID accountId = UUID.randomUUID();
+        final String externalKey1 = "2354";
+        final String transactionExternalKey1 = "jkjkjk";
+
+        final DateTime createdAfterDate = clock.getUTCNow().minusDays(10);
+
+        final String stateName = "RRRRR";
+        final String pluginName = "elated";
+
+        final PaymentAttemptModelDao attempt = new PaymentAttemptModelDao(accountId, paymentMethodId, createdAfterDate, createdAfterDate, externalKey1,
+                                                                          UUID.randomUUID(), transactionExternalKey1, TransactionType.AUTHORIZE, stateName, BigDecimal.ONE, Currency.USD,
+                                                                          ImmutableList.<String>of(pluginName), null);
+
+        final PaymentAttemptModelDao rehydratedAttempt = paymentDao.insertPaymentAttemptWithProperties(attempt, internalCallContext);
+
+        final UUID transactionId = UUID.randomUUID();
+        final String newStateName = "YYYYYYY";
+        paymentDao.updatePaymentAttempt(rehydratedAttempt.getId(), transactionId, newStateName, internalCallContext);
+
+        final PaymentAttemptModelDao attempt1 = paymentDao.getPaymentAttempt(rehydratedAttempt.getId(), internalCallContext);
+        assertEquals(attempt1.getStateName(), newStateName);
+        assertEquals(attempt1.getTransactionId(), transactionId);
+
+        final List<PluginProperty> properties = new ArrayList<PluginProperty>();
+        properties.add(new PluginProperty("prop1", "value1", false));
+        properties.add(new PluginProperty("prop2", "value2", false));
+
+
+        final byte [] serializedProperties = PluginPropertySerializer.serialize(properties);
+        paymentDao.updatePaymentAttemptWithProperties(rehydratedAttempt.getId(), transactionId, newStateName, serializedProperties, internalCallContext);
+        final PaymentAttemptModelDao attempt2 = paymentDao.getPaymentAttempt(rehydratedAttempt.getId(), internalCallContext);
+        assertEquals(attempt2.getStateName(), newStateName);
+        assertEquals(attempt2.getTransactionId(), transactionId);
+
+        final Iterable<PluginProperty> properties2 = PluginPropertySerializer.deserialize(attempt2.getPluginProperties());
+        checkProperty(properties2, new PluginProperty("prop1", "value1", false));
+        checkProperty(properties2, new PluginProperty("prop2", "value2", false));
+
+    }
+
+    private void checkProperty(final Iterable<PluginProperty> properties, final PluginProperty expected) {
+        final PluginProperty found = Iterables.tryFind(properties, new Predicate<PluginProperty>() {
+            @Override
+            public boolean apply(final PluginProperty input) {
+                return input.getKey().equals(expected.getKey());
+            }
+        }).orNull();
+        assertNotNull(found, "Did not find property key = " + expected.getKey());
+        assertEquals(found.getValue(), expected.getValue());
+    }
 
     private List<PaymentTransactionModelDao> getPendingTransactions(final UUID paymentId) {
         final List<PaymentTransactionModelDao> total = paymentDao.getTransactionsForPayment(paymentId, internalCallContext);