Execution of CreateTriggerStatement should not rely on external state

patch by Maxwell Guo; reviewed by Sam Tunnicliffe for CASSANDRA-20287

Author: Maxwell-Guo

CHANGES.txt

@@ -1,4 +1,5 @@
 5.1
+ * Execution of CreateTriggerStatement should not rely on external state (CASSANDRA-20287)
  * Support LIKE expressions in filtering queries (CASSANDRA-17198)
  * Make legacy index rebuilds safe on Gossip -> TCM upgrades (CASSANDRA-20887)
  * Minor improvements and hardening for IndexHints (CASSANDRA-20888)

src/java/org/apache/cassandra/cql3/statements/schema/CreateTriggerStatement.java

@@ -17,6 +17,9 @@
  */
 package org.apache.cassandra.cql3.statements.schema;
 
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
 import org.apache.cassandra.audit.AuditLogContext;
 import org.apache.cassandra.audit.AuditLogEntryType;
 import org.apache.cassandra.cql3.CQLStatement;
@@ -33,6 +36,7 @@
 
 public final class CreateTriggerStatement extends AlterSchemaStatement
 {
+    private static final Logger logger = LoggerFactory.getLogger(CreateTriggerStatement.class);
     private final String tableName;
     private final String triggerName;
     private final String triggerClass;
@@ -47,6 +51,21 @@ public CreateTriggerStatement(String keyspaceName, String tableName, String trig
         this.ifNotExists = ifNotExists;
     }
 
+    @Override
+    public void validate(ClientState state)
+    {
+        try
+        {
+            TriggerExecutor.instance.loadTriggerClass(triggerClass);
+        }
+        catch (Exception e)
+        {
+            InvalidRequestException thrown = ire("Trigger class '%s' couldn't be loaded during validation. Reason : %s.", triggerClass, e.getMessage());
+            thrown.initCause(e);
+            throw thrown;
+        }
+    }
+
     @Override
     public Keyspaces apply(ClusterMetadata metadata)
     {
@@ -77,9 +96,7 @@ public Keyspaces apply(ClusterMetadata metadata)
         }
         catch (Exception e)
         {
-            InvalidRequestException thrown = ire("Trigger class '%s' couldn't be loaded", triggerClass);
-            thrown.initCause(e);
-            throw thrown;
+            logger.warn(String.format("Trigger class '%s' couldn't be loaded at apply stage.", triggerClass));
         }
 
         TableMetadata newTable = table.withSwapped(table.triggers.with(TriggerMetadata.create(triggerName, triggerClass)));

src/java/org/apache/cassandra/triggers/TriggerExecutor.java

@@ -40,6 +40,7 @@
 import org.apache.cassandra.db.Mutation;
 import org.apache.cassandra.db.partitions.PartitionUpdate;
 import org.apache.cassandra.exceptions.CassandraException;
+import org.apache.cassandra.exceptions.ConfigurationException;
 import org.apache.cassandra.exceptions.InvalidRequestException;
 import org.apache.cassandra.io.util.File;
 import org.apache.cassandra.schema.TableId;
@@ -245,11 +246,13 @@ private List<Mutation> executeInternal(PartitionUpdate update)
         }
         List<Mutation> tmutations = Lists.newLinkedList();
         Thread.currentThread().setContextClassLoader(customClassLoader);
+        String triggerClass = "";
         try
         {
             for (TriggerMetadata td : triggers)
             {
                 ITrigger trigger = cachedTriggers.get(td.classOption);
+                triggerClass = td.classOption;
                 if (trigger == null)
                 {
                     trigger = loadTriggerInstance(td.classOption);
@@ -265,6 +268,10 @@ private List<Mutation> executeInternal(PartitionUpdate update)
         {
             throw ex;
         }
+        catch (ClassNotFoundException ex)
+        {
+            throw new ConfigurationException("Trigger class " + triggerClass + " couldn't be found.");
+        }
         catch (Exception ex)
         {
             throw new RuntimeException(String.format("Exception while executing trigger on table with ID: %s", update.metadata().id), ex);

test/unit/org/apache/cassandra/triggers/TriggerExecutorTest.java

@@ -37,6 +37,7 @@
 import org.apache.cassandra.utils.FBUtilities;
 
 import static org.apache.cassandra.utils.ByteBufferUtil.bytes;
+import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNull;
 
@@ -91,6 +92,15 @@ public void differentKeyColumnFamilies() throws ConfigurationException, InvalidR
         TriggerExecutor.instance.execute(makeCf(metadata, "k1", "v1", null));
     }
 
+    @Test
+    public void triggerClassNotFound()
+    {
+        TableMetadata metadata = makeTableMetadata("ks1", "cf1", TriggerMetadata.create("test", "NotExistedTriggerClass"));
+        assertThatExceptionOfType(ConfigurationException.class)
+        .isThrownBy(()-> TriggerExecutor.instance.execute(makeCf(metadata, "k1", "v1", null)))
+        .withMessageContaining("Trigger class NotExistedTriggerClass couldn't be found.");
+    }
+
     @Test
     public void noTriggerMutations() throws ConfigurationException, InvalidRequestException
     {

test/unit/org/apache/cassandra/triggers/TriggersTest.java

@@ -21,6 +21,8 @@
 import java.util.Collections;
 import java.util.concurrent.atomic.AtomicBoolean;
 
+import org.apache.cassandra.exceptions.InvalidRequestException;
+
 import org.junit.After;
 import org.junit.Assert;
 import org.junit.Before;
@@ -101,6 +103,16 @@ public void after()
         DatabaseDescriptor.setTriggersPolicy(originalTriggersPolicy);
     }
 
+    @Test
+    public void testCreateNonExistTrigger()
+    {
+        Assertions.assertThatThrownBy(() -> {
+                    QueryProcessor.process(String.format("CREATE TRIGGER no_trigger_test ON %s.%s USING 'org.apache.cassandra.triggers.Noexisttrigger' ", ksName, cfName), ConsistencyLevel.ONE);
+        })
+        .isInstanceOf(InvalidRequestException.class)
+        .hasMessageContaining("Trigger class 'org.apache.cassandra.triggers.Noexisttrigger' couldn't be loaded during validation.");
+    }
+
     @Test
     public void testTriggersPolicy()
     {