killbill-aplcache

diff --git a/jaxrs/src/main/java/com/ning/billing/jaxrs/json/TagJson.java b/jaxrs/src/main/java/com/ning/billing/jaxrs/json/TagJson.java
index fa62e86..12581cb 100644
--- a/jaxrs/src/main/java/com/ning/billing/jaxrs/json/TagJson.java
+++ b/jaxrs/src/main/java/com/ning/billing/jaxrs/json/TagJson.java
@@ -13,26 +13,28 @@
  * License for the specific language governing permissions and limitations
  * under the License.
  */
+
 package com.ning.billing.jaxrs.json;
 
 import java.util.List;
 
 import javax.annotation.Nullable;
 
-import com.fasterxml.jackson.annotation.JsonCreator;
-import com.fasterxml.jackson.annotation.JsonProperty;
 import com.ning.billing.util.audit.AuditLog;
 import com.ning.billing.util.tag.TagDefinition;
 
+import com.fasterxml.jackson.annotation.JsonCreator;
+import com.fasterxml.jackson.annotation.JsonProperty;
+
 public class TagJson extends JsonBase {
 
     private final String tagDefinitionId;
     private final String tagDefinitionName;
 
     @JsonCreator
-    public TagJson(@JsonProperty("tagDefinitionId")  final String tagDefinitionId,
-            @JsonProperty("tagDefinitionName") final String tagDefinitionName,
-            @JsonProperty("auditLogs") @Nullable List<AuditLogJson> auditLogs) {
+    public TagJson(@JsonProperty("tagDefinitionId") final String tagDefinitionId,
+                   @JsonProperty("tagDefinitionName") final String tagDefinitionName,
+                   @JsonProperty("auditLogs") @Nullable final List<AuditLogJson> auditLogs) {
         super(auditLogs);
         this.tagDefinitionId = tagDefinitionId;
         this.tagDefinitionName = tagDefinitionName;
@@ -49,4 +51,41 @@ public class TagJson extends JsonBase {
     public String getTagDefinitionName() {
         return tagDefinitionName;
     }
+
+    @Override
+    public String toString() {
+        final StringBuilder sb = new StringBuilder("TagJson{");
+        sb.append("tagDefinitionId='").append(tagDefinitionId).append('\'');
+        sb.append(", tagDefinitionName='").append(tagDefinitionName).append('\'');
+        sb.append('}');
+        return sb.toString();
+    }
+
+    @Override
+    public boolean equals(final Object o) {
+        if (this == o) {
+            return true;
+        }
+        if (o == null || getClass() != o.getClass()) {
+            return false;
+        }
+
+        final TagJson tagJson = (TagJson) o;
+
+        if (tagDefinitionId != null ? !tagDefinitionId.equals(tagJson.tagDefinitionId) : tagJson.tagDefinitionId != null) {
+            return false;
+        }
+        if (tagDefinitionName != null ? !tagDefinitionName.equals(tagJson.tagDefinitionName) : tagJson.tagDefinitionName != null) {
+            return false;
+        }
+
+        return true;
+    }
+
+    @Override
+    public int hashCode() {
+        int result = tagDefinitionId != null ? tagDefinitionId.hashCode() : 0;
+        result = 31 * result + (tagDefinitionName != null ? tagDefinitionName.hashCode() : 0);
+        return result;
+    }
 }

diff --git a/jaxrs/src/main/java/com/ning/billing/jaxrs/resources/JaxRsResourceBase.java b/jaxrs/src/main/java/com/ning/billing/jaxrs/resources/JaxRsResourceBase.java
index 40092a2..7af0b3d 100644
--- a/jaxrs/src/main/java/com/ning/billing/jaxrs/resources/JaxRsResourceBase.java
+++ b/jaxrs/src/main/java/com/ning/billing/jaxrs/resources/JaxRsResourceBase.java
@@ -130,6 +130,7 @@ public abstract class JaxRsResourceBase implements JaxrsResource {
                                   final CallContext context) throws TagApiException {
         final Collection<UUID> input = getTagDefinitionUUIDs(tagList);
         tagUserApi.addTags(id, getObjectType(), input, context);
+        // TODO This will always return 201, even if some (or all) tags already existed (in which case we don't do anything)
         return uriBuilder.buildResponse(this.getClass(), "getTags", id, uriInfo.getBaseUri().toString());
     }
 

diff --git a/server/src/test/java/com/ning/billing/jaxrs/TestAccount.java b/server/src/test/java/com/ning/billing/jaxrs/TestAccount.java
index f469837..4c90eab 100644
--- a/server/src/test/java/com/ning/billing/jaxrs/TestAccount.java
+++ b/server/src/test/java/com/ning/billing/jaxrs/TestAccount.java
@@ -34,13 +34,11 @@ import com.ning.billing.jaxrs.json.CustomFieldJson;
 import com.ning.billing.jaxrs.json.PaymentJsonSimple;
 import com.ning.billing.jaxrs.json.PaymentMethodJson;
 import com.ning.billing.jaxrs.json.RefundJson;
-import com.ning.billing.jaxrs.json.TagDefinitionJson;
 import com.ning.billing.jaxrs.json.TagJson;
 import com.ning.billing.jaxrs.resources.JaxrsResource;
 import com.ning.http.client.Response;
 
 import com.fasterxml.jackson.core.type.TypeReference;
-import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 
 import static org.testng.Assert.assertEquals;
@@ -252,21 +250,32 @@ public class TestAccount extends TestJaxrsBase {
 
     @Test(groups = "slow")
     public void testTags() throws Exception {
-
+        final String accountId = UUID.randomUUID().toString();
+        final String uri = JaxrsResource.ACCOUNTS_PATH + "/" + accountId + "/" + JaxrsResource.TAGS;
+        final String accountTagsUrl = getUrlFromUri(uri);
         // Use tag definition for AUTO_PAY_OFF
-        final TagDefinitionJson input = new TagDefinitionJson(new UUID(0, 1).toString(), false, "AUTO_PAY_OFF",
-                                                              "nothing more to say", ImmutableList.<String>of());
+        final String autoPayOffId = new UUID(0, 1).toString();
 
-        final Map<String, String> queryParams = new HashMap<String, String>();
-        queryParams.put(JaxrsResource.QUERY_TAGS, input.getId());
-        final String uri = JaxrsResource.ACCOUNTS_PATH + "/" + UUID.randomUUID().toString() + "/" + JaxrsResource.TAGS;
-        Response response = doPost(uri, null, queryParams, DEFAULT_HTTP_TIMEOUT_SEC);
+        // Add a tag
+        Response response = doPost(uri, null, ImmutableMap.<String, String>of(JaxrsResource.QUERY_TAGS, autoPayOffId), DEFAULT_HTTP_TIMEOUT_SEC);
         assertEquals(response.getStatusCode(), Status.CREATED.getStatusCode());
 
-        // Retrieves by Id based on Location returned
-        final String url = getUrlFromUri(uri);
-        response = doGetWithUrl(url, DEFAULT_EMPTY_QUERY, DEFAULT_HTTP_TIMEOUT_SEC);
+        // Retrieves all tags
+        response = doGetWithUrl(accountTagsUrl, DEFAULT_EMPTY_QUERY, DEFAULT_HTTP_TIMEOUT_SEC);
+        Assert.assertEquals(response.getStatusCode(), Status.OK.getStatusCode());
+        final List<TagJson> tags1 = mapper.readValue(response.getResponseBody(), new TypeReference<List<TagJson>>() {});
+        Assert.assertEquals(tags1.size(), 1);
+        Assert.assertEquals(tags1.get(0).getTagDefinitionId(), autoPayOffId);
+
+        // Verify adding the same tag a second time doesn't do anything
+        response = doPost(uri, null, ImmutableMap.<String, String>of(JaxrsResource.QUERY_TAGS, autoPayOffId), DEFAULT_HTTP_TIMEOUT_SEC);
+        assertEquals(response.getStatusCode(), Status.CREATED.getStatusCode());
+
+        // Retrieves all tags again
+        response = doGetWithUrl(accountTagsUrl, DEFAULT_EMPTY_QUERY, DEFAULT_HTTP_TIMEOUT_SEC);
         Assert.assertEquals(response.getStatusCode(), Status.OK.getStatusCode());
+        final List<TagJson> tags2 = mapper.readValue(response.getResponseBody(), new TypeReference<List<TagJson>>() {});
+        Assert.assertEquals(tags2, tags1);
     }
 
     @Test(groups = "slow")

diff --git a/util/src/main/java/com/ning/billing/util/entity/dao/EntityDaoBase.java b/util/src/main/java/com/ning/billing/util/entity/dao/EntityDaoBase.java
index e4718c3..e69c388 100644
--- a/util/src/main/java/com/ning/billing/util/entity/dao/EntityDaoBase.java
+++ b/util/src/main/java/com/ning/billing/util/entity/dao/EntityDaoBase.java
@@ -38,12 +38,16 @@ public abstract class EntityDaoBase<M extends EntityModelDao<E>, E extends Entit
 
     @Override
     public void create(final M entity, final InternalCallContext context) throws U {
-        transactionalSqlDao.execute(new EntitySqlDaoTransactionWrapper<Void>() {
+        transactionalSqlDao.execute(getCreateEntitySqlDaoTransactionWrapper(entity, context));
+    }
+
+    protected EntitySqlDaoTransactionWrapper<Void> getCreateEntitySqlDaoTransactionWrapper(final M entity, final InternalCallContext context) {
+        return new EntitySqlDaoTransactionWrapper<Void>() {
             @Override
             public Void inTransaction(final EntitySqlDaoWrapperFactory<EntitySqlDao> entitySqlDaoWrapperFactory) throws Exception {
                 final EntitySqlDao<M, E> transactional = entitySqlDaoWrapperFactory.become(realSqlDao);
 
-                if (transactional.getById(entity.getId().toString(), context) != null) {
+                if (checkEntityAlreadyExists(transactional, entity, context)) {
                     throw generateAlreadyExistsException(entity, context);
                 }
                 transactional.create(entity, context);
@@ -53,7 +57,11 @@ public abstract class EntityDaoBase<M extends EntityModelDao<E>, E extends Entit
                 postBusEventFromTransaction(entity, refreshedEntity, ChangeType.INSERT, entitySqlDaoWrapperFactory, context);
                 return null;
             }
-        });
+        };
+    }
+
+    protected boolean checkEntityAlreadyExists(final EntitySqlDao<M, E> transactional, final M entity, final InternalCallContext context) {
+        return transactional.getById(entity.getId().toString(), context) != null;
     }
 
     protected void postBusEventFromTransaction(final M entity, final M savedEntity, final ChangeType changeType,

diff --git a/util/src/main/java/com/ning/billing/util/entity/dao/EntitySqlDaoTransactionalJdbiWrapper.java b/util/src/main/java/com/ning/billing/util/entity/dao/EntitySqlDaoTransactionalJdbiWrapper.java
index 9b6933b..0f31ecd 100644
--- a/util/src/main/java/com/ning/billing/util/entity/dao/EntitySqlDaoTransactionalJdbiWrapper.java
+++ b/util/src/main/java/com/ning/billing/util/entity/dao/EntitySqlDaoTransactionalJdbiWrapper.java
@@ -70,4 +70,24 @@ public class EntitySqlDaoTransactionalJdbiWrapper {
         final EntitySqlDao<EntityModelDao<Entity>, Entity> entitySqlDao = dbi.onDemand(InitialEntitySqlDao.class);
         return entitySqlDao.inTransaction(TransactionIsolationLevel.READ_COMMITTED, new JdbiTransaction<ReturnType, EntityModelDao<Entity>, Entity>(entitySqlDaoTransactionWrapper));
     }
+
+    /**
+     * @param entitySqlDaoTransactionWrapper transaction to execute
+     * @param <ReturnType>                   object type to return from the transaction
+     * @return result from the transaction fo type ReturnType
+     */
+    public <ReturnType, E extends Exception> ReturnType execute(final Class<E> exception, final EntitySqlDaoTransactionWrapper<ReturnType> entitySqlDaoTransactionWrapper) throws E {
+        final EntitySqlDao<EntityModelDao<Entity>, Entity> entitySqlDao = dbi.onDemand(InitialEntitySqlDao.class);
+        try {
+            return execute(entitySqlDaoTransactionWrapper);
+        } catch (RuntimeException e) {
+            if (e.getCause() != null && e.getCause().getClass().isAssignableFrom(exception)) {
+                throw (E) e.getCause();
+            } else if (e.getCause() != null && e.getCause() instanceof RuntimeException) {
+                throw (RuntimeException) e.getCause();
+            } else {
+                throw e;
+            }
+        }
+    }
 }

diff --git a/util/src/main/java/com/ning/billing/util/tag/api/DefaultTagUserApi.java b/util/src/main/java/com/ning/billing/util/tag/api/DefaultTagUserApi.java
index de54c35..6c5f150 100644
--- a/util/src/main/java/com/ning/billing/util/tag/api/DefaultTagUserApi.java
+++ b/util/src/main/java/com/ning/billing/util/tag/api/DefaultTagUserApi.java
@@ -20,6 +20,7 @@ import java.util.Collection;
 import java.util.List;
 import java.util.UUID;
 
+import com.ning.billing.ErrorCode;
 import com.ning.billing.ObjectType;
 import com.ning.billing.util.api.TagApiException;
 import com.ning.billing.util.api.TagDefinitionApiException;
@@ -110,7 +111,14 @@ public class DefaultTagUserApi implements TagUserApi {
     public void addTag(final UUID objectId, final ObjectType objectType, final UUID tagDefinitionId, final CallContext context) throws TagApiException {
         final InternalCallContext internalContext = internalCallContextFactory.createInternalCallContext(objectId, objectType, context);
         final TagModelDao tag = new TagModelDao(context.getCreatedDate(), tagDefinitionId, objectId, objectType);
-        tagDao.create(tag, internalContext);
+        try {
+            tagDao.create(tag, internalContext);
+        } catch (TagApiException e) {
+            // Be lenient here and make the addTag method idempotent
+            if (ErrorCode.TAG_ALREADY_EXISTS.getCode() != e.getCode()) {
+                throw e;
+            }
+        }
     }
 
     @Override

diff --git a/util/src/main/java/com/ning/billing/util/tag/dao/DefaultTagDao.java b/util/src/main/java/com/ning/billing/util/tag/dao/DefaultTagDao.java
index 6ae85a9..206fe2b 100644
--- a/util/src/main/java/com/ning/billing/util/tag/dao/DefaultTagDao.java
+++ b/util/src/main/java/com/ning/billing/util/tag/dao/DefaultTagDao.java
@@ -30,7 +30,6 @@ import com.ning.billing.ErrorCode;
 import com.ning.billing.ObjectType;
 import com.ning.billing.util.api.TagApiException;
 import com.ning.billing.util.audit.ChangeType;
-import com.ning.billing.util.cache.Cachable.CacheType;
 import com.ning.billing.util.cache.CacheControllerDispatcher;
 import com.ning.billing.util.callcontext.InternalCallContext;
 import com.ning.billing.util.callcontext.InternalTenantContext;
@@ -50,6 +49,7 @@ import com.ning.billing.util.tag.api.user.TagEventBuilder;
 import com.google.common.base.Predicate;
 import com.google.common.collect.Collections2;
 import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Iterables;
 import com.google.inject.Inject;
 
 public class DefaultTagDao extends EntityDaoBase<TagModelDao, Tag, TagApiException> implements TagDao {
@@ -132,8 +132,22 @@ public class DefaultTagDao extends EntityDaoBase<TagModelDao, Tag, TagApiExcepti
     }
 
     @Override
+    protected boolean checkEntityAlreadyExists(final EntitySqlDao<TagModelDao, Tag> transactional, final TagModelDao entity, final InternalCallContext context) {
+        return Iterables.find(transactional.getByAccountRecordId(context),
+                              new Predicate<TagModelDao>() {
+                                  @Override
+                                  public boolean apply(final TagModelDao existingTag) {
+                                      return entity.equals(existingTag) || entity.isSame(existingTag);
+                                  }
+                              },
+                              null) != null;
+    }
+
+    @Override
     protected TagApiException generateAlreadyExistsException(final TagModelDao entity, final InternalCallContext context) {
-        return new TagApiException(ErrorCode.TAG_ALREADY_EXISTS, entity.getId());
+        // Print the tag details, not the id here, as we throw this exception when checking if a tag already exists
+        // by using the isSame(TagModelDao) method (see above)
+        return new TagApiException(ErrorCode.TAG_ALREADY_EXISTS, entity.toString());
     }
 
     private TagDefinitionModelDao getTagDefinitionFromTransaction(final UUID tagDefinitionId, final EntitySqlDaoWrapperFactory<EntitySqlDao> entitySqlDaoWrapperFactory, final InternalTenantContext context) throws TagApiException {
@@ -156,6 +170,11 @@ public class DefaultTagDao extends EntityDaoBase<TagModelDao, Tag, TagApiExcepti
     }
 
     @Override
+    public void create(final TagModelDao entity, final InternalCallContext context) throws TagApiException {
+        transactionalSqlDao.execute(TagApiException.class, getCreateEntitySqlDaoTransactionWrapper(entity, context));
+    }
+
+    @Override
     public void deleteTag(final UUID objectId, final ObjectType objectType, final UUID tagDefinitionId, final InternalCallContext context) throws TagApiException {
 
         transactionalSqlDao.execute(new EntitySqlDaoTransactionWrapper<Void>() {

diff --git a/util/src/main/java/com/ning/billing/util/tag/dao/TagModelDao.java b/util/src/main/java/com/ning/billing/util/tag/dao/TagModelDao.java
index ea6ce3f..d88163f 100644
--- a/util/src/main/java/com/ning/billing/util/tag/dao/TagModelDao.java
+++ b/util/src/main/java/com/ning/billing/util/tag/dao/TagModelDao.java
@@ -95,6 +95,10 @@ public class TagModelDao extends EntityBase implements EntityModelDao<Tag> {
 
         final TagModelDao that = (TagModelDao) o;
 
+        return isSame(that);
+    }
+
+    public boolean isSame(final TagModelDao that) {
         if (isActive != null ? !isActive.equals(that.isActive) : that.isActive != null) {
             return false;
         }

diff --git a/util/src/main/resources/com/ning/billing/util/entity/dao/EntitySqlDao.sql.stg b/util/src/main/resources/com/ning/billing/util/entity/dao/EntitySqlDao.sql.stg
index af975cd..5ee87d6 100644
--- a/util/src/main/resources/com/ning/billing/util/entity/dao/EntitySqlDao.sql.stg
+++ b/util/src/main/resources/com/ning/billing/util/entity/dao/EntitySqlDao.sql.stg
@@ -162,11 +162,12 @@ where <recordIdField("t.")> = :recordId
 ;
 >>
 
+/** Use NULL-safe equal to operator in case account_record_id is NULL **/
 getByAccountRecordId(accountRecordId) ::= <<
 select
 <allTableFields("t.")>
 from <tableName()> t
-where <accountRecordIdField("t.")> = :accountRecordId
+where <accountRecordIdField("t.")> \<=\> :accountRecordId
 <AND_CHECK_TENANT("t.")>
 ;
 >>