Second time (is not the charm)

Author: labkey-nicka

api/src/org/labkey/api/data/AbstractTableInfo.java

@@ -27,6 +27,7 @@
 import org.junit.Test;
 import org.labkey.api.cache.CacheManager;
 import org.labkey.api.collections.CaseInsensitiveHashMap;
+import org.labkey.api.collections.CaseInsensitiveHashSet;
 import org.labkey.api.collections.CaseInsensitiveMapWrapper;
 import org.labkey.api.collections.CaseInsensitiveTreeSet;
 import org.labkey.api.collections.NamedObjectList;
@@ -1892,16 +1893,18 @@ public final boolean canStreamTriggers(Container c)
         if (triggers.isEmpty())
             return null;
 
-        var columns = triggers.stream()
-                .map(Trigger::getManagedColumns)
-                .filter(Objects::nonNull)
-                .flatMap(Collection::stream)
-                .toList();
+        var columns = new CaseInsensitiveHashSet();
+        for (var trigger : triggers)
+        {
+            var managedColumns = trigger.getManagedColumns();
+            if (managedColumns == null)
+                continue;
 
-        if (columns.isEmpty())
-            return null;
+            columns.addAll(managedColumns.insert());
+            columns.addAll(managedColumns.update());
+        }
 
-        return Sets.newCaseInsensitiveHashSet(columns);
+        return columns.isEmpty() ? null : columns;
     }
 
     private Collection<Trigger> _triggers = null;
@@ -1978,15 +1981,6 @@ public void fireRowTrigger(Container c, User user, TriggerType type, boolean bef
 
         for (Trigger script : triggers)
         {
-            var managed = before ? script.getManagedColumns() : null;
-
-            // Inject sentinel for each declared column absent from the row; the trigger must handle it
-            if (newRow != null && managed != null && before)
-            {
-                for (var col : managed)
-                    newRow.putIfAbsent(col, Trigger.COLUMN_SENTINEL);
-            }
-
             // Snapshot keys after injection — trigger may update values but must not alter the key set
             var keysBeforeTrigger = newRow != null && before ? Set.copyOf(newRow.keySet()) : null;
 
@@ -1996,6 +1990,11 @@ public void fireRowTrigger(Container c, User user, TriggerType type, boolean bef
 
             if (newRow != null && before)
             {
+                var managed = script.getManagedColumns();
+                Set<String> managedCols = null;
+                if (managed != null)
+                    managedCols = managed.getColumns(type);
+
                 // Verify the trigger did not add or remove columns that are not managed
                 if (!newRow.keySet().equals(keysBeforeTrigger))
                 {
@@ -2006,10 +2005,15 @@ public void fireRowTrigger(Container c, User user, TriggerType type, boolean bef
                     removed.removeAll(newRow.keySet());
 
                     // managed column removals are intentional
-                    if (managed != null)
+                    if (managedCols != null)
                     {
-                        added.removeAll(managed);
-                        removed.removeAll(managed);
+                        added.removeAll(managedCols);
+                        removed.removeAll(managedCols);
+                        if (managed.ignored() != null)
+                        {
+                            added.removeAll(managed.ignored());
+                            removed.removeAll(managed.ignored());
+                        }
                     }
 
                     if (!added.isEmpty() || !removed.isEmpty())
@@ -2024,12 +2028,12 @@ public void fireRowTrigger(Container c, User user, TriggerType type, boolean bef
                     }
                 }
 
-                // Verify the trigger handled every declared column it was responsible for
-                if (managed != null)
+                // Verify that update triggers handle every managed column
+                if (managedCols != null && type == TriggerType.UPDATE)
                 {
-                    for (var col : managed)
+                    for (var col : managedCols)
                     {
-                        if (newRow.get(col) == Trigger.COLUMN_SENTINEL)
+                        if (!newRow.containsKey(col))
                             errors.addFieldError(col, "Trigger '" + script.getName() + "' declared column '" + col + "' in getManagedColumns() but did not set a value for it. Set null to clear or provide a value.");
                     }
                 }

api/src/org/labkey/api/data/triggers/Trigger.java

@@ -15,8 +15,10 @@
  */
 package org.labkey.api.data.triggers;
 
+import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 import org.json.JSONObject;
+import org.labkey.api.collections.Sets;
 import org.labkey.api.data.Container;
 import org.labkey.api.data.TableInfo;
 import org.labkey.api.query.BatchValidationException;
@@ -38,20 +40,6 @@
  */
 public interface Trigger
 {
-    /**
-     * Sentinel value placed into the row map for each column declared by {@link #getManagedColumns()}
-     * when that column is absent from the incoming row. The trigger must replace this with a real value
-     * or {@code null} before returning; leaving the sentinel in place is a validation error.
-     */
-    Object COLUMN_SENTINEL = new Object()
-    {
-        @Override
-        public String toString()
-        {
-            return "<trigger managed column - not yet handled>";
-        }
-    };
-
     /** The trigger name. */
     default String getName()
     {
@@ -104,41 +92,68 @@ default List<TableInfo.TriggerMethod> getEvents()
         }
     }
 
+    record ManagedColumns(@NotNull Set<String> insert, @NotNull Set<String> update, @Nullable Set<String> ignored)
+    {
+        public static ManagedColumns all(@NotNull Set<String> all)
+        {
+            return new ManagedColumns(all, all, null);
+        }
+
+        public static ManagedColumns all(@NotNull String... all)
+        {
+            return all(Sets.newCaseInsensitiveHashSet(all));
+        }
+
+        public @Nullable Set<String> getColumns(TableInfo.TriggerType type)
+        {
+            if (type == TableInfo.TriggerType.INSERT)
+                return insert;
+            if (type == TableInfo.TriggerType.UPDATE)
+                return update;
+            return null;
+        }
+    }
+
     /**
      * Returns the set of column names this trigger will read or write during row processing.
      * <p>
-     * For each row where a declared column is absent from the input, the data iterator initializes
-     * that column to {@link #COLUMN_SENTINEL} before the trigger fires. The trigger must then
-     * explicitly set each such column to {@code null} or a real value; failure to do so produces
+     * For each row where a declared column is absent from the input, the trigger must
+     * explicitly set each such column to null or a real value; failure to do so produces
      * a validation error naming this trigger and the unhandled column.
      * <p>
      * Columns that do not exist in the target table's schema (virtual/passthrough columns) may be
      * declared here and will work correctly — the database writer ignores them.
      */
-    default @Nullable Set<String> getManagedColumns()
+    default @Nullable ManagedColumns getManagedColumns()
     {
         return null;
     }
 
-    /**
-     * Convenience method for trigger implementations: replaces any {@link #COLUMN_SENTINEL} values
-     * in managed columns with {@code null}. Call this in early-return paths where the trigger will
-     * not produce a value for one or more of its declared columns.
-     */
-    default void clearManagedColumns(@Nullable Map<String, Object> row)
+    default void setInsertManagedColumns(@NotNull Map<String, Object> newRow)
+    {
+        setManagedColumns(newRow, null, TableInfo.TriggerType.INSERT);
+    }
+
+    default void setUpdateManagedColumns(@NotNull Map<String, Object> newRow, @NotNull Map<String, Object> oldRow)
     {
-        if (row == null)
+        if (oldRow == null)
+            throw new IllegalArgumentException("oldRow must be non-null for UPDATE triggers");
+
+        setManagedColumns(newRow, oldRow, TableInfo.TriggerType.UPDATE);
+    }
+
+    private void setManagedColumns(Map<String, Object> newRow, Map<String, Object> oldRow, TableInfo.TriggerType type)
+    {
+        var managedCols = getManagedColumns();
+        if (managedCols == null)
             return;
 
-        var managedColumns = getManagedColumns();
-        if (managedColumns == null || managedColumns.isEmpty())
+        var cols = managedCols.getColumns(type);
+        if (cols == null)
             return;
 
-        for (String col : managedColumns)
-        {
-            if (row.get(col) == COLUMN_SENTINEL)
-                row.remove(col);
-        }
+        for (var col : cols)
+            newRow.putIfAbsent(col, oldRow == null ? null : oldRow.get(col));
     }
 
     /**