killbill-memoizeit

util: Harden logic around so-called system tags so users can't

12/29/2016 9:01:09 PM

Details

diff --git a/profiles/killbill/src/test/java/org/killbill/billing/jaxrs/TestTag.java b/profiles/killbill/src/test/java/org/killbill/billing/jaxrs/TestTag.java
index 2fb7072..0ab1bfd 100644
--- a/profiles/killbill/src/test/java/org/killbill/billing/jaxrs/TestTag.java
+++ b/profiles/killbill/src/test/java/org/killbill/billing/jaxrs/TestTag.java
@@ -35,6 +35,7 @@ import org.killbill.billing.client.model.TagDefinition;
 import org.killbill.billing.client.model.Tags;
 import org.killbill.billing.util.api.AuditLevel;
 import org.killbill.billing.util.tag.ControlTagType;
+import org.killbill.billing.util.tag.dao.SystemTags;
 import org.testng.Assert;
 import org.testng.annotations.Test;
 
@@ -61,6 +62,8 @@ public class TestTag extends TestJaxrsBase {
         }
     }
 
+
+
     @Test(groups = "slow", description = "Can create a TagDefinition")
     public void testTagDefinitionOk() throws Exception {
         final TagDefinition input = new TagDefinition(null, false, "blue", "relaxing color", ImmutableList.<ObjectType>of());
@@ -76,6 +79,10 @@ public class TestTag extends TestJaxrsBase {
         List<TagDefinition> objFromJson = killBillClient.getTagDefinitions();
         final int sizeSystemTag = objFromJson.isEmpty() ? 0 : objFromJson.size();
 
+        for (final TagDefinition cur : objFromJson) {
+            Assert.assertFalse(SystemTags.isSystemTag(cur.getId()));
+        }
+
         final TagDefinition inputBlue = new TagDefinition(null, false, "blue", "relaxing color", ImmutableList.<ObjectType>of());
         killBillClient.createTagDefinition(inputBlue, createdBy, reason, comment);
 
@@ -148,6 +155,19 @@ public class TestTag extends TestJaxrsBase {
         }
     }
 
+    @Test(groups = "slow", description = "Can create a TagDefinition")
+    public void testNotAllowedSystemTag() throws Exception {
+
+        final Account account = createAccount();
+
+        try {
+            killBillClient.createAccountTag(account.getAccountId(), SystemTags.PARK_TAG_DEFINITION_ID, requestOptions);
+            Assert.fail("Creating a tag associated with a system tag should fail");
+        } catch (final Exception e) {
+            Assert.assertTrue(true);
+        }
+    }
+
     @Test(groups = "slow", description = "Can paginate through all tags")
     public void testTagsPagination() throws Exception {
         final Account account = createAccount();
diff --git a/util/src/main/java/org/killbill/billing/util/tag/api/DefaultTagUserApi.java b/util/src/main/java/org/killbill/billing/util/tag/api/DefaultTagUserApi.java
index 068c90a..b461cc2 100644
--- a/util/src/main/java/org/killbill/billing/util/tag/api/DefaultTagUserApi.java
+++ b/util/src/main/java/org/killbill/billing/util/tag/api/DefaultTagUserApi.java
@@ -37,6 +37,7 @@ import org.killbill.billing.util.tag.DefaultTagDefinition;
 import org.killbill.billing.util.tag.DescriptiveTag;
 import org.killbill.billing.util.tag.Tag;
 import org.killbill.billing.util.tag.TagDefinition;
+import org.killbill.billing.util.tag.dao.SystemTags;
 import org.killbill.billing.util.tag.dao.TagDao;
 import org.killbill.billing.util.tag.dao.TagDefinitionDao;
 import org.killbill.billing.util.tag.dao.TagDefinitionModelDao;
@@ -74,7 +75,7 @@ public class DefaultTagUserApi implements TagUserApi {
 
     @Override
     public List<TagDefinition> getTagDefinitions(final TenantContext context) {
-        return ImmutableList.<TagDefinition>copyOf(Collections2.transform(tagDefinitionDao.getTagDefinitions(internalCallContextFactory.createInternalTenantContextWithoutAccountRecordId(context)),
+        return ImmutableList.<TagDefinition>copyOf(Collections2.transform(tagDefinitionDao.getTagDefinitions(false, internalCallContextFactory.createInternalTenantContextWithoutAccountRecordId(context)),
                                                                           new Function<TagDefinitionModelDao, TagDefinition>() {
                                                                               @Override
                                                                               public TagDefinition apply(final TagDefinitionModelDao input) {
@@ -125,6 +126,12 @@ public class DefaultTagUserApi implements TagUserApi {
 
     @Override
     public void addTag(final UUID objectId, final ObjectType objectType, final UUID tagDefinitionId, final CallContext context) throws TagApiException {
+
+        if (SystemTags.isSystemTag(tagDefinitionId)) {
+            // TODO Create a proper ErrorCode instaed
+            throw new IllegalStateException(String.format("Failed to add tag for tagDefinitionId='%s': System tags are reserved for the system.", tagDefinitionId));
+        }
+
         final InternalCallContext internalContext = internalCallContextFactory.createInternalCallContext(objectId, objectType, context);
         final TagModelDao tag = new TagModelDao(context.getCreatedDate(), tagDefinitionId, objectId, objectType);
         try {
diff --git a/util/src/main/java/org/killbill/billing/util/tag/dao/DefaultTagDefinitionDao.java b/util/src/main/java/org/killbill/billing/util/tag/dao/DefaultTagDefinitionDao.java
index e406380..6c78854 100644
--- a/util/src/main/java/org/killbill/billing/util/tag/dao/DefaultTagDefinitionDao.java
+++ b/util/src/main/java/org/killbill/billing/util/tag/dao/DefaultTagDefinitionDao.java
@@ -68,7 +68,7 @@ public class DefaultTagDefinitionDao extends EntityDaoBase<TagDefinitionModelDao
     }
 
     @Override
-    public List<TagDefinitionModelDao> getTagDefinitions(final InternalTenantContext context) {
+    public List<TagDefinitionModelDao> getTagDefinitions(final boolean includeSystemTags, final InternalTenantContext context) {
         return transactionalSqlDao.execute(new EntitySqlDaoTransactionWrapper<List<TagDefinitionModelDao>>() {
             @Override
             public List<TagDefinitionModelDao> inTransaction(final EntitySqlDaoWrapperFactory entitySqlDaoWrapperFactory) throws Exception {
@@ -79,7 +79,7 @@ public class DefaultTagDefinitionDao extends EntityDaoBase<TagDefinitionModelDao
                 Iterators.addAll(definitionList, all);
 
                 // Add invoice tag definitions
-                definitionList.addAll(SystemTags.all());
+                definitionList.addAll(SystemTags.get(includeSystemTags));
                 return definitionList;
             }
         });
diff --git a/util/src/main/java/org/killbill/billing/util/tag/dao/SystemTags.java b/util/src/main/java/org/killbill/billing/util/tag/dao/SystemTags.java
index adede89..5879654 100644
--- a/util/src/main/java/org/killbill/billing/util/tag/dao/SystemTags.java
+++ b/util/src/main/java/org/killbill/billing/util/tag/dao/SystemTags.java
@@ -22,9 +22,13 @@ import java.util.LinkedList;
 import java.util.List;
 import java.util.UUID;
 
+import javax.annotation.Nullable;
+
 import org.killbill.billing.util.tag.ControlTagType;
 
+import com.google.common.base.Predicate;
 import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Iterables;
 
 public class SystemTags {
 
@@ -35,8 +39,10 @@ public class SystemTags {
     // Note! TagSqlDao.sql.stg needs to be kept in sync (see userAndSystemTagDefinitions)
     private static final List<TagDefinitionModelDao> SYSTEM_DEFINED_TAG_DEFINITIONS = ImmutableList.<TagDefinitionModelDao>of(new TagDefinitionModelDao(PARK_TAG_DEFINITION_ID, null, null, PARK_TAG_DEFINITION_NAME, "Accounts with invalid invoicing state"));
 
-    public static Collection<TagDefinitionModelDao> all() {
-        final Collection<TagDefinitionModelDao> all = new LinkedList<TagDefinitionModelDao>(SYSTEM_DEFINED_TAG_DEFINITIONS);
+    public static Collection<TagDefinitionModelDao> get(final boolean includeSystemTags) {
+        final Collection<TagDefinitionModelDao> all = includeSystemTags ?
+                                                      new LinkedList<TagDefinitionModelDao>(SYSTEM_DEFINED_TAG_DEFINITIONS) :
+                                                      new LinkedList<TagDefinitionModelDao>();
         for (final ControlTagType controlTag : ControlTagType.values()) {
             all.add(new TagDefinitionModelDao(controlTag));
         }
@@ -59,6 +65,15 @@ public class SystemTags {
         return null;
     }
 
+    public static boolean isSystemTag(final UUID tagDefinitionId) {
+        return Iterables.any(SYSTEM_DEFINED_TAG_DEFINITIONS, new Predicate<TagDefinitionModelDao>() {
+            @Override
+            public boolean apply(final TagDefinitionModelDao input) {
+                return input.getId().equals(tagDefinitionId);
+            }
+        });
+    }
+
     public static TagDefinitionModelDao lookup(final UUID tagDefinitionId) {
         for (final ControlTagType t : ControlTagType.values()) {
             if (t.getId().equals(tagDefinitionId)) {
diff --git a/util/src/main/java/org/killbill/billing/util/tag/dao/TagDefinitionDao.java b/util/src/main/java/org/killbill/billing/util/tag/dao/TagDefinitionDao.java
index cfa2447..84f3bed 100644
--- a/util/src/main/java/org/killbill/billing/util/tag/dao/TagDefinitionDao.java
+++ b/util/src/main/java/org/killbill/billing/util/tag/dao/TagDefinitionDao.java
@@ -28,7 +28,7 @@ import org.killbill.billing.util.tag.TagDefinition;
 
 public interface TagDefinitionDao extends EntityDao<TagDefinitionModelDao, TagDefinition, TagDefinitionApiException> {
 
-    public List<TagDefinitionModelDao> getTagDefinitions(InternalTenantContext context);
+    public List<TagDefinitionModelDao> getTagDefinitions(boolean includeSystemTags, InternalTenantContext context);
 
     public TagDefinitionModelDao getByName(String definitionName, InternalTenantContext context);
 
diff --git a/util/src/main/java/org/killbill/billing/util/tag/DefaultTagInternalApi.java b/util/src/main/java/org/killbill/billing/util/tag/DefaultTagInternalApi.java
index 9db3025..e4af036 100644
--- a/util/src/main/java/org/killbill/billing/util/tag/DefaultTagInternalApi.java
+++ b/util/src/main/java/org/killbill/billing/util/tag/DefaultTagInternalApi.java
@@ -51,7 +51,7 @@ public class DefaultTagInternalApi implements TagInternalApi {
 
     @Override
     public List<TagDefinition> getTagDefinitions(final InternalTenantContext context) {
-        return ImmutableList.<TagDefinition>copyOf(Collections2.transform(tagDefinitionDao.getTagDefinitions(context),
+        return ImmutableList.<TagDefinition>copyOf(Collections2.transform(tagDefinitionDao.getTagDefinitions(true, context),
                                                                           new Function<TagDefinitionModelDao, TagDefinition>() {
                                                                               @Override
                                                                               public TagDefinition apply(final TagDefinitionModelDao input) {
diff --git a/util/src/test/java/org/killbill/billing/util/tag/dao/MockTagDefinitionDao.java b/util/src/test/java/org/killbill/billing/util/tag/dao/MockTagDefinitionDao.java
index c602091..f382b4f 100644
--- a/util/src/test/java/org/killbill/billing/util/tag/dao/MockTagDefinitionDao.java
+++ b/util/src/test/java/org/killbill/billing/util/tag/dao/MockTagDefinitionDao.java
@@ -34,7 +34,7 @@ public class MockTagDefinitionDao extends MockEntityDaoBase<TagDefinitionModelDa
     private final Map<String, TagDefinitionModelDao> tags = new ConcurrentHashMap<String, TagDefinitionModelDao>();
 
     @Override
-    public List<TagDefinitionModelDao> getTagDefinitions(final InternalTenantContext context) {
+    public List<TagDefinitionModelDao> getTagDefinitions(final boolean dummy, final InternalTenantContext context) {
         return new ArrayList<TagDefinitionModelDao>(tags.values());
     }
 
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 b110370..d648fcd 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
@@ -68,8 +68,8 @@ public class TestDefaultTagDao extends UtilTestSuiteWithEmbeddedDB {
         result = tagDefinitionDao.getByIds(uuids, internalCallContext);
         assertEquals(result.size(), 4);
 
-        result = tagDefinitionDao.getTagDefinitions(internalCallContext);
-        assertEquals(result.size(), 3 + SystemTags.all().size());
+        result = tagDefinitionDao.getTagDefinitions(true, internalCallContext);
+        assertEquals(result.size(), 3 + SystemTags.get(true).size());
     }
 
     @Test(groups = "slow")
diff --git a/util/src/test/java/org/killbill/billing/util/tag/TestTagStore.java b/util/src/test/java/org/killbill/billing/util/tag/TestTagStore.java
index 6ff114f..8de750a 100644
--- a/util/src/test/java/org/killbill/billing/util/tag/TestTagStore.java
+++ b/util/src/test/java/org/killbill/billing/util/tag/TestTagStore.java
@@ -144,7 +144,7 @@ public class TestTagStore extends UtilTestSuiteWithEmbeddedDB {
 
     @Test(groups = "slow")
     public void testGetTagDefinitions() {
-        final List<TagDefinitionModelDao> definitionList = tagDefinitionDao.getTagDefinitions(internalCallContext);
+        final List<TagDefinitionModelDao> definitionList = tagDefinitionDao.getTagDefinitions(false, internalCallContext);
         assertTrue(definitionList.size() >= ControlTagType.values().length);
     }
 }