Issue 52614: AssertionError deleting site group from its details page

api/src/org/labkey/api/audit/AuditTypeEvent.java

@@ -15,13 +15,17 @@
  */
 package org.labkey.api.audit;
 
+import org.apache.logging.log4j.Logger;
 import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.Nullable;
 import org.labkey.api.data.Container;
 import org.labkey.api.data.ContainerManager;
 import org.labkey.api.security.Group;
-import org.labkey.api.security.SecurityManager;
 import org.labkey.api.security.User;
 import org.labkey.api.security.UserManager;
+import org.labkey.api.util.ExceptionUtil;
+import org.labkey.api.util.logging.LogHelper;
+import org.slf4j.LoggerFactory;
 
 import java.util.Date;
 import java.util.LinkedHashMap;
@@ -46,8 +50,8 @@ public class AuditTypeEvent
     private long _rowId;
     private Integer _impersonatedBy;
     private String _comment;
-    private String _projectId;
-    private String _container;
+    private Container _projectId;
+    private Container _container;
     private String _eventType;
     private Date _created;
     private User _createdBy;
@@ -56,16 +60,16 @@ public class AuditTypeEvent
     private String userComment;
     private Long _transactionId;
 
-    public AuditTypeEvent(String eventType, Container container, String comment)
-    {
-        this(eventType, container.getId(), comment);
-    }
-
-    public AuditTypeEvent(String eventType, String container, String comment)
+    public AuditTypeEvent(@NotNull String eventType, @NotNull Container container, @Nullable String comment)
     {
         _eventType = eventType;
+        if (container == null)
+        {
+            ExceptionUtil.logExceptionToMothership(null, new IllegalStateException("Audit event container is null"));
+        }
         _container = container;
         _comment = comment;
+        _projectId = container.getProject();
     }
 
     public AuditTypeEvent(){}
@@ -100,22 +104,22 @@ public void setComment(String comment)
         _comment = comment;
     }
 
-    public String getProjectId()
+    public Container getProjectId()
     {
         return _projectId;
     }
 
-    public void setProjectId(String projectId)
+    public void setProjectId(Container projectId)
     {
         _projectId = projectId;
     }
 
-    public String getContainer()
+    public Container getContainer()
     {
         return _container;
     }
 
-    public void setContainer(String container)
+    public void setContainer(Container container)
     {
         _container = container;
     }
@@ -190,16 +194,19 @@ public void setTransactionId(Long transactionId)
         _transactionId = transactionId;
     }
 
-    protected String getContainerMessageElement(@NotNull String containerId)
+    protected String getContainerMessageElement(@NotNull Container container)
     {
-        String value = " (" + containerId + ")";
-        Container container = ContainerManager.getForId(containerId);
-        if (container != null)
-            value = container.getPath() + value;
+        String value = " (" + container.getId() + ")";
+        value = container.getPath() + value;
         return value;
     }
 
-    protected String getUserMessageElement(@NotNull Integer userId)
+    protected String getUserMessageElement(@NotNull User user)
+    {
+        return user.getEmail() + " (" + user.getUserId() + ")";
+    }
+
+    protected String getUserMessageElement(int userId)
     {
         String value = " (" + userId + ")";
         User user = UserManager.getUser(userId);
@@ -208,13 +215,9 @@ protected String getUserMessageElement(@NotNull Integer userId)
         return value;
     }
 
-    protected String getGroupMessageElement(@NotNull Integer groupId)
+    protected String getGroupMessageElement(@NotNull Group group)
     {
-        String value = " (" + groupId + ")";
-        Group group = SecurityManager.getGroup(groupId);
-        if (group != null)
-            value = group.getName() + value;
-        return value;
+        return group.getName() + " (" + group.getUserId() + ")";
     }
 
     public Map<String, Object> getAuditLogMessageElements()
@@ -230,10 +233,9 @@ public Map<String, Object> getAuditLogMessageElements()
         Integer impersonatorId = getImpersonatedBy();
         if (impersonatorId != null)
             elements.put(IMPERSONATED_BY_KEY, getUserMessageElement(impersonatorId));
-        String containerId = getContainer();
-        if (containerId != null)
-            elements.put(CONTAINER_KEY, getContainerMessageElement(containerId));
-        String projectId = getProjectId();
+        Container container = getContainer();
+        elements.put(CONTAINER_KEY, getContainerMessageElement(container));
+        Container projectId = getProjectId();
         if (projectId != null)
             elements.put(PROJECT_KEY, getContainerMessageElement(projectId));
         if (getComment() != null)

api/src/org/labkey/api/audit/ClientApiAuditProvider.java

@@ -18,6 +18,7 @@
 import org.jetbrains.annotations.NotNull;
 import org.labkey.api.audit.query.AbstractAuditDomainKind;
 import org.labkey.api.audit.query.DefaultAuditTypeTable;
+import org.labkey.api.data.Container;
 import org.labkey.api.data.ContainerFilter;
 import org.labkey.api.data.PropertyStorageSpec.Index;
 import org.labkey.api.data.TableInfo;
@@ -165,7 +166,7 @@ public ClientApiAuditEvent()
             super();
         }
 
-        public ClientApiAuditEvent(String container, String comment)
+        public ClientApiAuditEvent(Container container, String comment)
         {
             super(EVENT_TYPE, container, comment);
         }

api/src/org/labkey/api/audit/DetailedAuditTypeEvent.java

@@ -12,11 +12,6 @@ public DetailedAuditTypeEvent()
     }
 
     public DetailedAuditTypeEvent(String eventType, Container container, String comment)
-    {
-        super(eventType, container.getId(), comment);
-    }
-
-    public DetailedAuditTypeEvent(String eventType, String container, String comment)
     {
         super(eventType, container, comment);
     }

api/src/org/labkey/api/audit/ExperimentAuditEvent.java

@@ -1,5 +1,7 @@
 package org.labkey.api.audit;
 
+import org.labkey.api.data.Container;
+
 import java.util.LinkedHashMap;
 import java.util.Map;
 
@@ -19,7 +21,7 @@ public ExperimentAuditEvent()
         super();
     }
 
-    public ExperimentAuditEvent(String container, String comment)
+    public ExperimentAuditEvent(Container container, String comment)
     {
         super(EVENT_TYPE, container, comment);
     }

api/src/org/labkey/api/audit/SampleTimelineAuditEvent.java

@@ -93,7 +93,7 @@ public SampleTimelineAuditEvent()
         super();
     }
 
-    public SampleTimelineAuditEvent(String container, String comment)
+    public SampleTimelineAuditEvent(Container container, String comment)
     {
         super(EVENT_TYPE, container, comment);
     }

api/src/org/labkey/api/audit/TransactionAuditProvider.java

@@ -4,6 +4,7 @@
 import org.jetbrains.annotations.Nullable;
 import org.labkey.api.audit.query.AbstractAuditDomainKind;
 import org.labkey.api.audit.query.DefaultAuditTypeTable;
+import org.labkey.api.data.Container;
 import org.labkey.api.data.ContainerFilter;
 import org.labkey.api.data.JdbcType;
 import org.labkey.api.data.MutableColumnInfo;
@@ -111,7 +112,7 @@ public TransactionAuditEvent()
             super();
         }
 
-        public TransactionAuditEvent(String container, QueryService.AuditAction auditAction, long transactionId)
+        public TransactionAuditEvent(Container container, QueryService.AuditAction auditAction, long transactionId)
         {
             super(EVENT_TYPE, container, auditAction.getDefaultCommentSummary());
             _auditAction = auditAction;

api/src/org/labkey/api/audit/provider/FileSystemAuditProvider.java

@@ -19,6 +19,7 @@
 import org.labkey.api.audit.AuditTypeEvent;
 import org.labkey.api.audit.AuditTypeProvider;
 import org.labkey.api.audit.query.AbstractAuditDomainKind;
+import org.labkey.api.data.Container;
 import org.labkey.api.exp.PropertyDescriptor;
 import org.labkey.api.exp.PropertyType;
 import org.labkey.api.query.FieldKey;
@@ -111,7 +112,7 @@ public FileSystemAuditEvent()
             super();
         }
 
-        public FileSystemAuditEvent(String container, String comment)
+        public FileSystemAuditEvent(Container container, String comment)
         {
             super(EVENT_TYPE, container, comment);
         }

api/src/org/labkey/api/audit/provider/GroupAuditProvider.java

@@ -58,6 +58,7 @@
 import java.io.Writer;
 import java.util.ArrayList;
 import java.util.Collections;
+import java.util.HashMap;
 import java.util.LinkedHashMap;
 import java.util.LinkedHashSet;
 import java.util.List;
@@ -221,15 +222,43 @@ public static class GroupAuditEvent extends AuditTypeEvent
         Integer _group;
         String _resourceEntityId;
 
+        private final Map<String, Object> _messageElements = new HashMap<>();
+
         public GroupAuditEvent()
         {
             super();
         }
 
-        public GroupAuditEvent(String container, String comment)
+        public GroupAuditEvent(Container container, String comment)
         {
             super(GroupManager.GROUP_AUDIT_EVENT, container, comment);
         }
+        public GroupAuditEvent(Container c, String comment, Group group)
+        {
+            this(c, comment, group, null);
+        }
+        public GroupAuditEvent(Container c, String comment, Group group, UserPrincipal userOrGroup)
+        {
+            this(c, comment);
+            if (group != null)
+            {
+                _messageElements.put("group", getGroupMessageElement(group));
+                _group = group.getUserId();
+            }
+            if (userOrGroup != null)
+            {
+                if (userOrGroup instanceof User u)
+                {
+                    _messageElements.put("user", getUserMessageElement(u));
+                }
+                _user = userOrGroup.getUserId();
+            }
+        }
+
+        public GroupAuditEvent(Container c, String comment, UserPrincipal userOrGroup)
+        {
+            this(c, comment, null, userOrGroup);
+        }
 
         public Integer getUser()
         {
@@ -264,11 +293,7 @@ public void setResourceEntityId(String resourceEntityId)
         @Override
         public Map<String, Object> getAuditLogMessageElements()
         {
-            Map<String, Object> elements = new LinkedHashMap<>();
-            if (getUser() != null)
-                elements.put("user", getUserMessageElement(getUser()));
-            if (getGroup() != null)
-                elements.put("group", getGroupMessageElement(getGroup()));
+            Map<String, Object> elements = new LinkedHashMap<>(_messageElements);
             elements.putAll(super.getAuditLogMessageElements());
             return elements;
         }

api/src/org/labkey/api/audit/provider/MessageAuditProvider.java

@@ -19,6 +19,7 @@
 import org.labkey.api.audit.AuditTypeEvent;
 import org.labkey.api.audit.AuditTypeProvider;
 import org.labkey.api.audit.query.AbstractAuditDomainKind;
+import org.labkey.api.data.Container;
 import org.labkey.api.exp.PropertyDescriptor;
 import org.labkey.api.exp.PropertyType;
 import org.labkey.api.query.FieldKey;
@@ -111,7 +112,7 @@ public MessageAuditEvent()
             super();
         }
 
-        public MessageAuditEvent(String container, String comment)
+        public MessageAuditEvent(Container container, String comment)
         {
             super(MailHelper.MESSAGE_AUDIT_EVENT, container, comment);
         }

api/src/org/labkey/api/audit/provider/SiteSettingsAuditProvider.java

@@ -21,6 +21,7 @@
 import org.labkey.api.audit.AuditTypeProvider;
 import org.labkey.api.audit.query.AbstractAuditDomainKind;
 import org.labkey.api.audit.query.DefaultAuditTypeTable;
+import org.labkey.api.data.Container;
 import org.labkey.api.data.ContainerFilter;
 import org.labkey.api.data.HtmlDisplayColumnFactory;
 import org.labkey.api.data.TableInfo;
@@ -117,7 +118,7 @@ public SiteSettingsAuditEvent()
             super();
         }
 
-        public SiteSettingsAuditEvent(String container, String comment)
+        public SiteSettingsAuditEvent(Container container, String comment)
         {
             super(AUDIT_EVENT_TYPE, container, comment);
         }

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

@@ -2027,10 +2027,7 @@ private static void addAuditEvent(User user, Container c, String comment)
     {
         if (user != null)
         {
-            AuditTypeEvent event = new AuditTypeEvent(ContainerAuditProvider.CONTAINER_AUDIT_EVENT, c.getId(), comment);
-            if (c.getProject() != null)
-                event.setProjectId(c.getProject().getId());
-
+            AuditTypeEvent event = new AuditTypeEvent(ContainerAuditProvider.CONTAINER_AUDIT_EVENT, c, comment);
             AuditLogService.get().addEvent(user, event);
         }
     }

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

@@ -40,7 +40,7 @@ public SelectQueryAuditEvent()
 
     public SelectQueryAuditEvent(Container container, String comment)
     {
-        super(SelectQueryAuditProvider.EVENT_NAME, container.getId(), comment);
+        super(SelectQueryAuditProvider.EVENT_NAME, container, comment);
     }
 
     public SelectQueryAuditEvent(QueryLogging queryLogging)

api/src/org/labkey/api/exp/property/DomainAuditProvider.java

@@ -145,7 +145,7 @@ public DomainAuditEvent()
             super();
         }
 
-        public DomainAuditEvent(String container, String comment)
+        public DomainAuditEvent(Container container, String comment)
         {
             super(EVENT_TYPE, container, comment);
         }

api/src/org/labkey/api/exp/property/DomainPropertyAuditProvider.java

@@ -19,6 +19,7 @@
 import org.labkey.api.audit.AuditTypeEvent;
 import org.labkey.api.audit.query.AbstractAuditDomainKind;
 import org.labkey.api.audit.query.DefaultAuditTypeTable;
+import org.labkey.api.data.Container;
 import org.labkey.api.data.ContainerFilter;
 import org.labkey.api.data.ContainerManager;
 import org.labkey.api.data.MutableColumnInfo;
@@ -190,7 +191,7 @@ public DomainPropertyAuditEvent()
             super();
         }
 
-        public DomainPropertyAuditEvent(String container, String propertyUri, String propertyName, String action,
+        public DomainPropertyAuditEvent(Container container, String propertyUri, String propertyName, String action,
                                         Long domainEventId, String domainName, String comment)
         {
             super(EVENT_NAME, container, comment);

api/src/org/labkey/api/premium/AntiVirusService.java

@@ -103,7 +103,7 @@ default void warnAndAuditLog(Logger log, String logmessage, ViewBackgroundInfo i
     {
         log.warn((null != info.getUser() ? info.getUser().getEmail() + " " : "") + logmessage);
         FileSystemAuditProvider.FileSystemAuditEvent event = new FileSystemAuditProvider.FileSystemAuditEvent(
-                info.getContainer().getId(), logmessage
+                info.getContainer(), logmessage
         );
         if (null != info.getURL())
             event.setDirectory(info.getURL().getPath());

api/src/org/labkey/api/query/AbstractQueryUpdateService.java

@@ -218,7 +218,7 @@ public boolean hasExistingRowsInOtherContainers(Container container, Map<Integer
     public static TransactionAuditProvider.TransactionAuditEvent createTransactionAuditEvent(Container container, QueryService.AuditAction auditAction)
     {
         long auditId = DbSequenceManager.get(ContainerManager.getRoot(), DB_SEQUENCE_NAME).next();
-        return new TransactionAuditProvider.TransactionAuditEvent(container.getId(), auditAction, auditId);
+        return new TransactionAuditProvider.TransactionAuditEvent(container, auditAction, auditId);
     }
 
     public static void addTransactionAuditEvent(DbScope.Transaction transaction, User user, TransactionAuditProvider.TransactionAuditEvent auditEvent)

api/src/org/labkey/api/security/AuthenticationManager.java

@@ -909,7 +909,7 @@ public static void addAuditEvent(@NotNull User user, HttpServletRequest request,
         AUTH_MESSAGES.put(key, msg);
         if (user.isGuest())
         {
-            UserManager.UserAuditEvent event = new UserManager.UserAuditEvent(ContainerManager.getRoot().getId(), msg, user);
+            UserManager.UserAuditEvent event = new UserManager.UserAuditEvent(ContainerManager.getRoot(), msg, user);
             AuditLogService.get().addEvent(user, event);
         }
         else