killbill-memoizeit

account: subscription: make external key optional Default

1/22/2015 10:05:19 AM

Details

diff --git a/account/src/main/java/org/killbill/billing/account/api/user/DefaultAccountUserApi.java b/account/src/main/java/org/killbill/billing/account/api/user/DefaultAccountUserApi.java
index 0eb821e..283bec0 100644
--- a/account/src/main/java/org/killbill/billing/account/api/user/DefaultAccountUserApi.java
+++ b/account/src/main/java/org/killbill/billing/account/api/user/DefaultAccountUserApi.java
@@ -61,7 +61,7 @@ public class DefaultAccountUserApi implements AccountUserApi {
     @Override
     public Account createAccount(final AccountData data, final CallContext context) throws AccountApiException {
         // Not transactional, but there is a db constraint on that column
-        if (getIdFromKey(data.getExternalKey(), context) != null) {
+        if (data.getExternalKey() != null && getIdFromKey(data.getExternalKey(), context) != null) {
             throw new AccountApiException(ErrorCode.ACCOUNT_ALREADY_EXISTS, data.getExternalKey());
         }
 
diff --git a/account/src/main/java/org/killbill/billing/account/dao/AccountModelDao.java b/account/src/main/java/org/killbill/billing/account/dao/AccountModelDao.java
index 4f8cd7d..e63f7f3 100644
--- a/account/src/main/java/org/killbill/billing/account/dao/AccountModelDao.java
+++ b/account/src/main/java/org/killbill/billing/account/dao/AccountModelDao.java
@@ -22,15 +22,15 @@ import javax.annotation.Nullable;
 
 import org.joda.time.DateTime;
 import org.joda.time.DateTimeZone;
-
 import org.killbill.billing.account.api.Account;
 import org.killbill.billing.account.api.AccountData;
 import org.killbill.billing.catalog.api.Currency;
 import org.killbill.billing.util.dao.TableName;
-import org.killbill.billing.entity.EntityBase;
 import org.killbill.billing.util.entity.dao.EntityModelDao;
 import org.killbill.billing.util.entity.dao.EntityModelDaoBase;
 
+import com.google.common.base.Objects;
+
 public class AccountModelDao extends EntityModelDaoBase implements EntityModelDao<Account> {
 
     private String externalKey;
@@ -62,7 +62,7 @@ public class AccountModelDao extends EntityModelDaoBase implements EntityModelDa
                            final String city, final String stateOrProvince, final String country, final String postalCode,
                            final String phone, final Boolean migrated, final Boolean notifiedForInvoices) {
         super(id, createdDate, updatedDate);
-        this.externalKey = externalKey;
+        this.externalKey = Objects.firstNonNull(externalKey, id.toString());
         this.email = email;
         this.name = name;
         this.firstNameLength = firstNameLength;
diff --git a/account/src/test/java/org/killbill/billing/account/dao/TestAccountDao.java b/account/src/test/java/org/killbill/billing/account/dao/TestAccountDao.java
index 13ae31f..1ead069 100644
--- a/account/src/test/java/org/killbill/billing/account/dao/TestAccountDao.java
+++ b/account/src/test/java/org/killbill/billing/account/dao/TestAccountDao.java
@@ -29,6 +29,7 @@ import org.killbill.billing.account.api.AccountData;
 import org.killbill.billing.account.api.AccountEmail;
 import org.killbill.billing.account.api.DefaultAccount;
 import org.killbill.billing.account.api.DefaultAccountEmail;
+import org.killbill.billing.account.api.DefaultMutableAccountData;
 import org.killbill.billing.account.api.MutableAccountData;
 import org.killbill.billing.callcontext.InternalCallContext;
 import org.killbill.billing.callcontext.InternalTenantContext;
@@ -59,6 +60,24 @@ import static org.killbill.billing.account.AccountTestUtils.createTestAccount;
 
 public class TestAccountDao extends AccountTestSuiteWithEmbeddedDB {
 
+    @Test(groups = "slow", description = "Test Account: verify minimal set of required fields")
+    public void testMinimalFields() throws Exception {
+        final String email = UUID.randomUUID().toString();
+        final String name = UUID.randomUUID().toString();
+        final AccountData accountData = new DefaultMutableAccountData(null, email, name, 0, null,
+                                                                      0, null, null, null, null,
+                                                                      null, null, null, null, null,
+                                                                      null, null, false, true);
+        final AccountModelDao account = new AccountModelDao(UUID.randomUUID(), accountData);
+        accountDao.create(account, internalCallContext);
+
+        final AccountModelDao retrievedAccount = accountDao.getById(account.getId(), internalCallContext);
+        checkAccountsEqual(retrievedAccount, account);
+
+        // Verify a default external key was set
+        Assert.assertEquals(retrievedAccount.getExternalKey(), retrievedAccount.getId().toString());
+    }
+
     @Test(groups = "slow", description = "Test Account: basic DAO calls")
     public void testBasic() throws AccountApiException {
         final AccountModelDao account = createTestAccount();
@@ -133,7 +152,7 @@ public class TestAccountDao extends AccountTestSuiteWithEmbeddedDB {
     // Correct fix is to add a check at the API level instead, but today we are not testing very much the input
     // so seems weird to just add one check for that specific case.
     //
-    @Test(groups = "slow", description = "Test Account DAO: very long numbers", enabled=false)
+    @Test(groups = "slow", description = "Test Account DAO: very long numbers", enabled = false)
     public void testOverlyLongPhoneNumber() throws AccountApiException {
         final AccountModelDao account = createTestAccount("12345678901234567890123456");
         try {
diff --git a/jaxrs/src/main/java/org/killbill/billing/jaxrs/resources/AccountResource.java b/jaxrs/src/main/java/org/killbill/billing/jaxrs/resources/AccountResource.java
index 3257a7a..729d977 100644
--- a/jaxrs/src/main/java/org/killbill/billing/jaxrs/resources/AccountResource.java
+++ b/jaxrs/src/main/java/org/killbill/billing/jaxrs/resources/AccountResource.java
@@ -304,7 +304,6 @@ public class AccountResource extends JaxRsResourceBase {
                                   @javax.ws.rs.core.Context final UriInfo uriInfo) throws AccountApiException {
         verifyNonNullOrEmpty(json, "AccountJson body should be specified");
         verifyNonNullOrEmpty(json.getName(), "AccountJson name needs to be set");
-        verifyNonNullOrEmpty(json.getExternalKey(), "AccountJson externalKey needs to be set");
         verifyNonNullOrEmpty(json.getEmail(), "AccountJson email needs to be set");
 
         final AccountData data = json.toAccountData();

pom.xml 2(+1 -1)

diff --git a/pom.xml b/pom.xml
index f18c392..0cae2f2 100644
--- a/pom.xml
+++ b/pom.xml
@@ -20,7 +20,7 @@
     <parent>
         <artifactId>killbill-oss-parent</artifactId>
         <groupId>org.kill-bill.billing</groupId>
-        <version>0.9.7</version>
+        <version>0.9.8-SNAPSHOT</version>
     </parent>
     <artifactId>killbill</artifactId>
     <version>0.13.2-SNAPSHOT</version>
diff --git a/profiles/killbill/src/test/java/org/killbill/billing/jaxrs/TestAccount.java b/profiles/killbill/src/test/java/org/killbill/billing/jaxrs/TestAccount.java
index 294eec9..c033337 100644
--- a/profiles/killbill/src/test/java/org/killbill/billing/jaxrs/TestAccount.java
+++ b/profiles/killbill/src/test/java/org/killbill/billing/jaxrs/TestAccount.java
@@ -26,6 +26,7 @@ import java.util.UUID;
 
 import javax.annotation.Nullable;
 
+import org.killbill.billing.ErrorCode;
 import org.killbill.billing.ObjectType;
 import org.killbill.billing.client.KillBillClientException;
 import org.killbill.billing.client.model.Account;
@@ -48,6 +49,24 @@ import static org.testng.Assert.fail;
 
 public class TestAccount extends TestJaxrsBase {
 
+    @Test(groups = "slow", description = "Verify external key is unique")
+    public void testUniqueExternalKey() throws Exception {
+        // Verify the external key is not mandatory
+        final Account inputWithNoExternalKey = getAccount(UUID.randomUUID().toString(), null, UUID.randomUUID().toString());
+        Assert.assertNull(inputWithNoExternalKey.getExternalKey());
+
+        final Account account = killBillClient.createAccount(inputWithNoExternalKey, createdBy, reason, comment);
+        Assert.assertNotNull(account.getExternalKey());
+
+        final Account inputWithSameExternalKey = getAccount(UUID.randomUUID().toString(), account.getExternalKey(), UUID.randomUUID().toString());
+        try {
+            killBillClient.createAccount(inputWithSameExternalKey, createdBy, reason, comment);
+            Assert.fail();
+        } catch (final KillBillClientException e) {
+            Assert.assertEquals(e.getBillingException().getCode(), (Integer) ErrorCode.ACCOUNT_ALREADY_EXISTS.getCode());
+        }
+    }
+
     @Test(groups = "slow", description = "Can create, retrieve, search and update accounts")
     public void testAccountOk() throws Exception {
         final Account input = createAccount();
diff --git a/profiles/killbill/src/test/java/org/killbill/billing/jaxrs/TestBundle.java b/profiles/killbill/src/test/java/org/killbill/billing/jaxrs/TestBundle.java
index 5a5587a..23a9462 100644
--- a/profiles/killbill/src/test/java/org/killbill/billing/jaxrs/TestBundle.java
+++ b/profiles/killbill/src/test/java/org/killbill/billing/jaxrs/TestBundle.java
@@ -36,6 +36,14 @@ import static org.testng.Assert.assertNotEquals;
 
 public class TestBundle extends TestJaxrsBase {
 
+    @Test(groups = "slow", description = "Can create bundles without an external key")
+    public void testCreateBundleWithNoExternalKey() throws Exception {
+        final Account accountJson = createAccount();
+
+        final Subscription subscription = createEntitlement(accountJson.getAccountId(), null, "Shotgun", ProductCategory.BASE, BillingPeriod.MONTHLY, true);
+        Assert.assertNotNull(subscription.getExternalKey());
+    }
+
     @Test(groups = "slow", description = "Can retrieve bundles by external key")
     public void testBundleOk() throws Exception {
         final Account accountJson = createAccount();
diff --git a/subscription/src/main/java/org/killbill/billing/subscription/engine/dao/model/SubscriptionBundleModelDao.java b/subscription/src/main/java/org/killbill/billing/subscription/engine/dao/model/SubscriptionBundleModelDao.java
index 4b1f2c4..2f36acb 100644
--- a/subscription/src/main/java/org/killbill/billing/subscription/engine/dao/model/SubscriptionBundleModelDao.java
+++ b/subscription/src/main/java/org/killbill/billing/subscription/engine/dao/model/SubscriptionBundleModelDao.java
@@ -19,14 +19,14 @@ package org.killbill.billing.subscription.engine.dao.model;
 import java.util.UUID;
 
 import org.joda.time.DateTime;
-
 import org.killbill.billing.subscription.api.user.DefaultSubscriptionBaseBundle;
 import org.killbill.billing.subscription.api.user.SubscriptionBaseBundle;
 import org.killbill.billing.util.dao.TableName;
-import org.killbill.billing.entity.EntityBase;
 import org.killbill.billing.util.entity.dao.EntityModelDao;
 import org.killbill.billing.util.entity.dao.EntityModelDaoBase;
 
+import com.google.common.base.Objects;
+
 public class SubscriptionBundleModelDao extends EntityModelDaoBase implements EntityModelDao<SubscriptionBaseBundle> {
 
     private String externalKey;
@@ -39,7 +39,7 @@ public class SubscriptionBundleModelDao extends EntityModelDaoBase implements En
     public SubscriptionBundleModelDao(final UUID id, final String key, final UUID accountId, final DateTime lastSysUpdateDate,
                                       final DateTime createdDate, DateTime originalCreatedDate, final DateTime updateDate) {
         super(id, createdDate, updateDate);
-        this.externalKey = key;
+        this.externalKey = Objects.firstNonNull(key, id.toString());
         this.accountId = accountId;
         this.lastSysUpdateDate = lastSysUpdateDate;
         this.originalCreatedDate = originalCreatedDate;
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 f479f79..6e284d0 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,10 +20,6 @@ import java.util.List;
 
 import org.joda.time.DateTime;
 import org.joda.time.Interval;
-import org.testng.Assert;
-import org.testng.annotations.Test;
-
-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;
@@ -35,6 +31,8 @@ import org.killbill.billing.subscription.DefaultSubscriptionTestInitializer;
 import org.killbill.billing.subscription.SubscriptionTestSuiteWithEmbeddedDB;
 import org.killbill.billing.subscription.events.SubscriptionBaseEvent;
 import org.killbill.billing.subscription.events.phase.PhaseEvent;
+import org.testng.Assert;
+import org.testng.annotations.Test;
 
 import static org.testng.Assert.assertEquals;
 import static org.testng.Assert.assertNotNull;
@@ -43,6 +41,12 @@ import static org.testng.Assert.assertTrue;
 public class TestUserApiCreate extends SubscriptionTestSuiteWithEmbeddedDB {
 
     @Test(groups = "slow")
+    public void testCreateBundleWithNoExternalKey() throws Exception {
+        final SubscriptionBaseBundle newBundle = subscriptionInternalApi.createBundleForAccount(bundle.getAccountId(), null, internalCallContext);
+        assertNotNull(newBundle.getExternalKey());
+    }
+
+    @Test(groups = "slow")
     public void testCreateBundlesWithSameExternalKeys() throws SubscriptionBaseApiException {
         final DateTime init = clock.getUTCNow();
         final DateTime requestedDate = init.minusYears(1);