killbill-aplcache

diff --git a/subscription/src/main/java/org/killbill/billing/subscription/api/svcs/DefaultSubscriptionInternalApi.java b/subscription/src/main/java/org/killbill/billing/subscription/api/svcs/DefaultSubscriptionInternalApi.java
index 9a5095e..0fd3a0b 100644
--- a/subscription/src/main/java/org/killbill/billing/subscription/api/svcs/DefaultSubscriptionInternalApi.java
+++ b/subscription/src/main/java/org/killbill/billing/subscription/api/svcs/DefaultSubscriptionInternalApi.java
@@ -381,37 +381,8 @@ public class DefaultSubscriptionInternalApi extends SubscriptionApiBase implemen
     @Override
     public SubscriptionBaseBundle createBundleForAccount(final UUID accountId, final String bundleKey, final InternalCallContext context) throws SubscriptionBaseApiException {
 
-        final List<SubscriptionBaseBundle> existingBundles = dao.getSubscriptionBundlesForKey(bundleKey, context);
-
-        //
-        // Because the creation of the SubscriptionBundle is not atomic (with creation of Subscription/SubscriptionEvent), we verify if we were left
-        // with an empty SubscriptionBaseBundle form a past failing operation (See #684). We only allow reuse if such SubscriptionBaseBundle is fully
-        // empty (and don't allow use case where all Subscription are cancelled, which is the condition for that key to be re-used)
-        // Such condition should have been checked upstream (to decide whether that key is valid or not)
-        //
-        final SubscriptionBaseBundle existingBundleForAccount = Iterables.tryFind(existingBundles, new Predicate<SubscriptionBaseBundle>() {
-            @Override
-            public boolean apply(final SubscriptionBaseBundle input) {
-                return input.getAccountId().equals(accountId);
-            }
-        }).orNull();
-
-        // If Bundle already exists, and there is 0 Subscription, we reuse
-        if (existingBundleForAccount != null) {
-            try {
-                final Map<UUID, List<SubscriptionBase>> accountSubscriptions = dao.getSubscriptionsForAccount(context);
-                final List<SubscriptionBase> subscriptions = accountSubscriptions.get(existingBundleForAccount.getId());
-                if (subscriptions == null || subscriptions.size() == 0) {
-                    return existingBundleForAccount;
-                }
-            } catch (final CatalogApiException e) {
-                throw new SubscriptionBaseApiException(e);
-            }
-        }
-
         final DateTime now = clock.getUTCNow();
-        final DateTime originalCreatedDate = !existingBundles.isEmpty() ? existingBundles.get(0).getCreatedDate() : now;
-        final DefaultSubscriptionBaseBundle bundle = new DefaultSubscriptionBaseBundle(bundleKey, accountId, now, originalCreatedDate, now, now);
+        final DefaultSubscriptionBaseBundle bundle = new DefaultSubscriptionBaseBundle(bundleKey, accountId, now, now, now, now);
 
         if (null != bundleKey && bundleKey.length() > 255) {
             throw new SubscriptionBaseApiException(ErrorCode.EXTERNAL_KEY_LIMIT_EXCEEDED);

diff --git a/subscription/src/main/java/org/killbill/billing/subscription/engine/dao/DefaultSubscriptionDao.java b/subscription/src/main/java/org/killbill/billing/subscription/engine/dao/DefaultSubscriptionDao.java
index 5fe34c7..36a9afe 100644
--- a/subscription/src/main/java/org/killbill/billing/subscription/engine/dao/DefaultSubscriptionDao.java
+++ b/subscription/src/main/java/org/killbill/billing/subscription/engine/dao/DefaultSubscriptionDao.java
@@ -33,7 +33,6 @@ import java.util.UUID;
 import javax.annotation.Nullable;
 import javax.inject.Inject;
 
-import com.google.common.base.Preconditions;
 import org.joda.time.DateTime;
 import org.killbill.billing.ErrorCode;
 import org.killbill.billing.callcontext.InternalCallContext;
@@ -43,6 +42,7 @@ import org.killbill.billing.catalog.api.CatalogApiException;
 import org.killbill.billing.catalog.api.CatalogService;
 import org.killbill.billing.catalog.api.Plan;
 import org.killbill.billing.catalog.api.ProductCategory;
+import org.killbill.billing.entitlement.api.Entitlement.EntitlementState;
 import org.killbill.billing.entitlement.api.SubscriptionApiException;
 import org.killbill.billing.entity.EntityPersistenceException;
 import org.killbill.billing.subscription.api.SubscriptionBase;
@@ -56,6 +56,7 @@ import org.killbill.billing.subscription.api.user.DefaultEffectiveSubscriptionEv
 import org.killbill.billing.subscription.api.user.DefaultRequestedSubscriptionEvent;
 import org.killbill.billing.subscription.api.user.DefaultSubscriptionBase;
 import org.killbill.billing.subscription.api.user.DefaultSubscriptionBaseBundle;
+import org.killbill.billing.subscription.api.user.SubscriptionBaseApiException;
 import org.killbill.billing.subscription.api.user.SubscriptionBaseBundle;
 import org.killbill.billing.subscription.api.user.SubscriptionBaseTransitionData;
 import org.killbill.billing.subscription.api.user.SubscriptionBuilder;
@@ -76,7 +77,6 @@ import org.killbill.billing.subscription.events.user.ApiEvent;
 import org.killbill.billing.subscription.events.user.ApiEventBuilder;
 import org.killbill.billing.subscription.events.user.ApiEventCancel;
 import org.killbill.billing.subscription.events.user.ApiEventChange;
-import org.killbill.billing.subscription.events.user.ApiEventCreate;
 import org.killbill.billing.subscription.events.user.ApiEventType;
 import org.killbill.billing.subscription.exceptions.SubscriptionBaseError;
 import org.killbill.billing.util.cache.CacheControllerDispatcher;
@@ -104,6 +104,7 @@ import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 import com.google.common.base.Function;
+import com.google.common.base.Preconditions;
 import com.google.common.base.Predicate;
 import com.google.common.collect.ArrayListMultimap;
 import com.google.common.collect.Collections2;
@@ -256,11 +257,58 @@ public class DefaultSubscriptionDao extends EntityDaoBase<SubscriptionBundleMode
     }
 
     @Override
-    public SubscriptionBaseBundle createSubscriptionBundle(final DefaultSubscriptionBaseBundle bundle, final InternalCallContext context) {
-        return transactionalSqlDao.execute(new EntitySqlDaoTransactionWrapper<SubscriptionBaseBundle>() {
+    public SubscriptionBaseBundle createSubscriptionBundle(final DefaultSubscriptionBaseBundle bundle, final InternalCallContext context) throws SubscriptionBaseApiException {
+
+        return transactionalSqlDao.execute(SubscriptionBaseApiException.class, new EntitySqlDaoTransactionWrapper<SubscriptionBaseBundle>() {
             @Override
-            public SubscriptionBaseBundle inTransaction(final EntitySqlDaoWrapperFactory entitySqlDaoWrapperFactory) throws EntityPersistenceException {
+            public SubscriptionBaseBundle inTransaction(final EntitySqlDaoWrapperFactory entitySqlDaoWrapperFactory) throws Exception {
+                final List<SubscriptionBundleModelDao> existingBundles = entitySqlDaoWrapperFactory.become(BundleSqlDao.class).getBundlesForKey(bundle.getExternalKey(), context);
+                //
+                // Because the creation of the SubscriptionBundle is not atomic (with creation of Subscription/SubscriptionEvent), we verify if we were left
+                // with an empty SubscriptionBaseBundle form a past failing operation (See #684). We only allow reuse if such SubscriptionBaseBundle is fully
+                // empty (and don't allow use case where all Subscription are cancelled, which is the condition for that key to be re-used)
+                // Such condition should have been checked upstream (to decide whether that key is valid or not)
+                //
+                final SubscriptionBundleModelDao existingBundleForAccount = Iterables.tryFind(existingBundles, new Predicate<SubscriptionBundleModelDao>() {
+                    @Override
+                    public boolean apply(final SubscriptionBundleModelDao input) {
+                        return input.getAccountId().equals(bundle.getAccountId());
+                    }
+                }).orNull();
+
+                // If Bundle already exists, and there is 0 Subscription, we reuse
+                if (existingBundleForAccount != null) {
+                    final List<SubscriptionModelDao> accountSubscriptions = entitySqlDaoWrapperFactory.become(SubscriptionSqlDao.class).getByAccountRecordId(context);
+                    if (accountSubscriptions == null || accountSubscriptions.size() == 0) {
+                        return SubscriptionBundleModelDao.toSubscriptionbundle(existingBundleForAccount);
+                    }
+                }
+
+                for (SubscriptionBundleModelDao cur : existingBundles) {
+                    final List<SubscriptionModelDao> subscriptions = entitySqlDaoWrapperFactory.become(SubscriptionSqlDao.class).getSubscriptionsFromBundleId(cur.getId().toString(), context);
+                    final Iterable<SubscriptionModelDao> filtered = subscriptions != null ? Iterables.filter(subscriptions, new Predicate<SubscriptionModelDao>() {
+                        @Override
+                        public boolean apply(@Nullable final SubscriptionModelDao input) {
+                            return input.getCategory() != ProductCategory.ADD_ON;
+                        }
+                    }) : ImmutableList.<SubscriptionModelDao>of();
+                    for (SubscriptionModelDao f : filtered) {
+                        try {
+                            final SubscriptionBase s = buildSubscription(SubscriptionModelDao.toSubscription(f, cur.getExternalKey()), context);
+                            if (s.getState() != EntitlementState.CANCELLED) {
+                                throw new SubscriptionBaseApiException(ErrorCode.SUB_CREATE_ACTIVE_BUNDLE_KEY_EXISTS, bundle.getExternalKey());
+                            }
+                        } catch (CatalogApiException e) {
+                            throw new SubscriptionBaseApiException(e);
+                        }
+                    }
+                }
+
                 final SubscriptionBundleModelDao model = new SubscriptionBundleModelDao(bundle);
+                // Preserve Original created date
+                if (!existingBundles.isEmpty()) {
+                    model.setOriginalCreatedDate(existingBundles.get(0).getCreatedDate());
+                }
                 final BundleSqlDao bundleSqlDao = entitySqlDaoWrapperFactory.become(BundleSqlDao.class);
                 final SubscriptionBundleModelDao result = createAndRefresh(bundleSqlDao, model, context);
                 return SubscriptionBundleModelDao.toSubscriptionbundle(result);

diff --git a/subscription/src/main/java/org/killbill/billing/subscription/engine/dao/SubscriptionDao.java b/subscription/src/main/java/org/killbill/billing/subscription/engine/dao/SubscriptionDao.java
index 516d80c..886e43a 100644
--- a/subscription/src/main/java/org/killbill/billing/subscription/engine/dao/SubscriptionDao.java
+++ b/subscription/src/main/java/org/killbill/billing/subscription/engine/dao/SubscriptionDao.java
@@ -31,6 +31,7 @@ import org.killbill.billing.subscription.api.transfer.TransferCancelData;
 import org.killbill.billing.subscription.api.user.DefaultSubscriptionBase;
 import org.killbill.billing.subscription.api.user.DefaultSubscriptionBaseBundle;
 import org.killbill.billing.subscription.api.user.DefaultSubscriptionBaseWithAddOns;
+import org.killbill.billing.subscription.api.user.SubscriptionBaseApiException;
 import org.killbill.billing.subscription.api.user.SubscriptionBaseBundle;
 import org.killbill.billing.subscription.engine.dao.model.SubscriptionBundleModelDao;
 import org.killbill.billing.subscription.events.SubscriptionBaseEvent;
@@ -52,7 +53,7 @@ public interface SubscriptionDao extends EntityDao<SubscriptionBundleModelDao, S
 
     public SubscriptionBaseBundle getSubscriptionBundleFromId(UUID bundleId, InternalTenantContext context);
 
-    public SubscriptionBaseBundle createSubscriptionBundle(DefaultSubscriptionBaseBundle bundle, InternalCallContext context);
+    public SubscriptionBaseBundle createSubscriptionBundle(DefaultSubscriptionBaseBundle bundle, InternalCallContext context) throws SubscriptionBaseApiException;
 
     public SubscriptionBase getSubscriptionFromId(UUID subscriptionId, InternalTenantContext context) throws CatalogApiException;
 

diff --git a/subscription/src/test/java/org/killbill/billing/subscription/api/user/TestUserApiCreate.java b/subscription/src/test/java/org/killbill/billing/subscription/api/user/TestUserApiCreate.java
index 4776379..36167c8 100644
--- a/subscription/src/test/java/org/killbill/billing/subscription/api/user/TestUserApiCreate.java
+++ b/subscription/src/test/java/org/killbill/billing/subscription/api/user/TestUserApiCreate.java
@@ -20,6 +20,7 @@ import java.util.List;
 
 import org.joda.time.DateTime;
 import org.joda.time.Interval;
+import org.killbill.billing.ErrorCode;
 import org.killbill.billing.api.TestApiListener.NextEvent;
 import org.killbill.billing.catalog.api.BillingPeriod;
 import org.killbill.billing.catalog.api.PhaseType;
@@ -62,6 +63,15 @@ public class TestUserApiCreate extends SubscriptionTestSuiteWithEmbeddedDB {
         assertListenerStatus();
         assertNotNull(subscription);
 
+
+        // Verify we can't create a second bundle with the same key
+        try {
+            subscriptionInternalApi.createBundleForAccount(bundle.getAccountId(), DefaultSubscriptionTestInitializer.DEFAULT_BUNDLE_KEY, internalCallContext);
+            Assert.fail("Should not be able to create a bundle with same externalKey");
+        } catch (final SubscriptionBaseApiException e) {
+            Assert.assertEquals(e.getCode(), ErrorCode.SUB_CREATE_ACTIVE_BUNDLE_KEY_EXISTS.getCode());
+        }
+
         testListener.pushExpectedEvent(NextEvent.CANCEL);
         subscription.cancelWithDate(clock.getUTCNow(), callContext);
         assertListenerStatus();
@@ -70,6 +80,7 @@ public class TestUserApiCreate extends SubscriptionTestSuiteWithEmbeddedDB {
         assertNotNull(newBundle);
         assertEquals(newBundle.getOriginalCreatedDate().compareTo(bundle.getCreatedDate()), 0);
 
+
         testListener.pushExpectedEvents(NextEvent.CREATE, NextEvent.PHASE);
         final DefaultSubscriptionBase newSubscription = (DefaultSubscriptionBase) subscriptionInternalApi.createSubscription(newBundle.getId(),
                                                                                                                              testUtil.getProductSpecifier(productName, planSetName, term, null), null, requestedDate, false, internalCallContext);