Skip to content

AuditLogHelper: return null instead of index error #2610

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
labkey-nicka merged 1 commit into from
Aug 11, 2025
Merged

AuditLogHelper: return null instead of index error #2610

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/org/labkey/test/tests/DomainDesignerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -282,7 +282,7 @@ public void testInvalidLookupDomainField() throws IOException, CommandException
.clickSave();

AuditLogHelper.DetailedAuditEventRow expectedDomainEvent = new AuditLogHelper.DetailedAuditEventRow(null, listName, null,
"The name of the list domain 'InvalidLookUpNameList' was changed to 'InvalidLookUpNameList_edited'. The descriptor of domain InvalidLookUpNameList_edited was updated.",
String.format("The name of the list domain '%s' was changed to '%s'. The descriptor of domain %s was updated.", listName, editedListName, editedListName),
"", null, null, "Name: " + listName + " > " + editedListName);
boolean pass = _auditLogHelper.validateLastDomainAuditEvents(editedListName, getProjectName(), expectedDomainEvent, Collections.emptyMap());
checker().verifyTrue("The comment logged for the list renaming was not as expected", pass);
Expand Down
20 changes: 7 additions & 13 deletions src/org/labkey/test/tests/TextChoiceSampleTypeTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,8 @@
import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.stream.Collectors;

@Category({Daily.class})
Expand Down Expand Up @@ -330,10 +328,6 @@ public void testUpdatingAndDeletingValuesInSampleType() throws IOException, Comm

TestDataGenerator dataGenerator = createSampleType(sampleTypeName, namePrefix, textChoiceFieldName, expectedUnLockedValues);

// Add the list of the event ids to an ignore list so future tests don't look at them again.
Set<Integer> ignoreIds = new HashSet<>();
ignoreIds.addAll(_auditLogHelper.getDomainEventIds(getProjectName(), sampleTypeName, null));

log("Create some samples that have TextChoice values set.");

// Only assign a few of the values to samples (i.e. lock them).
Expand All @@ -347,7 +341,7 @@ public void testUpdatingAndDeletingValuesInSampleType() throws IOException, Comm
Map<String, String> samplesWithTC = new HashMap<>();

int index = 0;
for(int i = 1; i <= 20; i++)
for (int i = 1; i <= 20; i++)
{

String sampleName = String.format("%s%d", namePrefix, i);
Expand All @@ -357,10 +351,10 @@ public void testUpdatingAndDeletingValuesInSampleType() throws IOException, Comm
String strFieldValue;

// Give a TextChoice value to every other sample.
if(i%2 == 0)
if (i%2 == 0)
{

if(index >= expectedLockedValues.size())
if (index >= expectedLockedValues.size())
index = 0;

String tcValue = expectedLockedValues.get(index);
Expand Down Expand Up @@ -413,7 +407,7 @@ public void testUpdatingAndDeletingValuesInSampleType() throws IOException, Comm
value = expectedUnLockedValues.get(0);
fieldRow.selectTextChoiceValue(value);

if(checker().verifyTrue(String.format("Delete button is not enabled for value '%s', it should be.", value),
if (checker().verifyTrue(String.format("Delete button is not enabled for value '%s', it should be.", value),
fieldRow.isTextChoiceDeleteButtonEnabled()))
{
fieldRow.deleteTextChoiceValue(value);
Expand Down Expand Up @@ -474,13 +468,13 @@ public void testUpdatingAndDeletingValuesInSampleType() throws IOException, Comm
// Construct a list of samples that have TextChoice set and what they are expected to be.
List<Map<String, String>> expectedSamples = new ArrayList<>();

for(Map.Entry<String, String> entry : samplesWithTC.entrySet())
for (Map.Entry<String, String> entry : samplesWithTC.entrySet())
{
String sampleId = entry.getKey();

// Need to special case for the TC value that was just updated.
String sampleValue;
if(entry.getValue().equals(valueToUpdate))
if (entry.getValue().equals(valueToUpdate))
{
sampleValue = updatedValue;
}
Expand Down Expand Up @@ -618,7 +612,7 @@ public void testSetTextChoiceValueForSample() throws IOException, CommandExcepti

List<String> availableSamples = new ArrayList<>();

for(int i = 1; i <= 5; i++)
for (int i = 1; i <= 5; i++)
{
Map<String, Object> sample = new HashMap<>();
String sampleName = String.format("%s%d", namePrefix, i);
Expand Down
109 changes: 57 additions & 52 deletions src/org/labkey/test/util/AuditLogHelper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package org.labkey.test.util;

import org.apache.commons.lang3.StringUtils;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
import org.json.JSONException;
import org.labkey.remoteapi.CommandException;
Expand Down Expand Up @@ -165,10 +166,12 @@ public void checkTimelineAuditEventDiffCount(String containerPath, List<Integer>
{
checkAuditEventDiffCount(containerPath, getAuditEventNameFromURL(), expectedDiffCounts);
}

public void checkAuditEventDiffCount(String containerPath, AuditEvent auditEventName, List<Integer> expectedDiffCounts) throws IOException, CommandException
{
checkAuditEventDiffCount(containerPath, auditEventName, Collections.emptyList(), expectedDiffCounts);
}

public void checkAuditEventDiffCount(String containerPath, AuditEvent auditEventName, List<Filter> filters, List<Integer> expectedDiffCounts) throws IOException, CommandException
{
Integer maxRows = expectedDiffCounts.size();
Expand Down Expand Up @@ -287,12 +290,13 @@ public boolean validateDomainPropertiesAuditLog(String domainName, Integer domai
return true;
Map<String, DetailedAuditEventRow> actualAuditDetails = getDomainPropertyEvents(domainName, domainEventId);
boolean pass = true;
if (expectedAuditDetails.keySet().size() != actualAuditDetails.keySet().size())
if (expectedAuditDetails.size() != actualAuditDetails.size())
{
pass = false;
TestLogger.log("Number of DomainPropertyAuditEvent events not as expected.");
TestLogger.log(String.format("Number of DomainPropertyAuditEvent events not as expected. Expected %d, Actual %d.", expectedAuditDetails.size(), actualAuditDetails.size()));
}
for (String key : expectedAuditDetails.keySet())

for (String key : expectedAuditDetails.keySet())
{
DetailedAuditEventRow expectedAuditDetail = expectedAuditDetails.get(key);
DetailedAuditEventRow actualAuditDetail = actualAuditDetails.get(key);
Expand All @@ -311,13 +315,19 @@ public boolean validateDomainPropertiesAuditLog(String domainName, Integer domai
public boolean validateLastDomainAuditEvents(String domainName, String projectName, DetailedAuditEventRow expectedDomainEvent, Map<String, DetailedAuditEventRow> expectedDomainPropertyEvents)
{
DetailedAuditEventRow latestDomainEvent = getLastDomainEvent(projectName, domainName);
if (latestDomainEvent == null)
{
TestLogger.log(String.format("No DomainAuditEvent found for domain '%s' in project '%s'.", domainName, projectName));
return false;
}

boolean pass = validateDetailAuditLog(expectedDomainEvent, latestDomainEvent);
return pass && validateDomainPropertiesAuditLog(domainName, latestDomainEvent.rowId, expectedDomainPropertyEvents);
}

public List<Integer> getDomainEventIds(String projectName, String domainName, @Nullable Collection<Integer> ignoreIds)
{
List<DetailedAuditEventRow> domainAuditEventAllRows = getDomainEventLog(projectName, domainName, ignoreIds);
List<DetailedAuditEventRow> domainAuditEventAllRows = getDomainAuditEventLog(projectName, domainName, ignoreIds, null);

List<Integer> domainEventIds = new ArrayList<>();
domainAuditEventAllRows.forEach((event)->domainEventIds.add(event.rowId));
Expand All @@ -327,14 +337,20 @@ public List<Integer> getDomainEventIds(String projectName, String domainName, @N
return domainEventIds;
}

public DetailedAuditEventRow getLastDomainEvent(String projectName, String domainName)
public @Nullable DetailedAuditEventRow getLastDomainEvent(String projectName, String domainName)
{
return getDomainEventLog(projectName, domainName, null).get(0);
List<DetailedAuditEventRow> eventLog = getDomainAuditEventLog(projectName, domainName, null, 1);
if (eventLog.isEmpty())
return null;
return eventLog.get(0);
}

public Integer getLastDomainEventId(String projectName, String domainName)
public @Nullable Integer getLastDomainEventId(String projectName, String domainName)
{
return getLastDomainEvent(projectName, domainName).rowId;
DetailedAuditEventRow event = getLastDomainEvent(projectName, domainName);
if (event == null)
return null;
return event.rowId;
}

public static List<String> propertyAuditColumns = List.of("type", "comment", "usercomment", "oldvalues", "newvalues", "datachanges");
Expand Down Expand Up @@ -363,8 +379,11 @@ public String getLogString()
}
}

public Map<String, DetailedAuditEventRow> getDomainPropertyEvents(String domainName, Integer domainEventId)
public @NotNull Map<String, DetailedAuditEventRow> getDomainPropertyEvents(String domainName, Integer domainEventId)
{
if (domainEventId == null)
return Collections.emptyMap();

List<Map<String, Object>> allRows = getDomainPropertyEventLog(domainName, Collections.singletonList(domainEventId));
Map<String, DetailedAuditEventRow> domainPropEventComments = new HashMap<>();
allRows.forEach((event)->{
Expand All @@ -378,8 +397,8 @@ public Map<String, DetailedAuditEventRow> getDomainPropertyEvents(String domainN
String dataChanges = getLogColumnDisplayValue(event, "dataChanges");
domainPropEventComments.put(propertyName, new DetailedAuditEventRow(rowId, propertyName, action, comment, userComment, oldValue, newValue, dataChanges));
});
return domainPropEventComments;

return domainPropEventComments;
}

public Map<String, DetailedAuditEventRow> getLastDomainPropertyEvents(String projectName, String domainName)
Expand All @@ -395,67 +414,58 @@ public List<String> getLastDomainPropertyValues(String projectName, String domai

public List<String> getDomainEventComments(String projectName, String domainName, @Nullable Collection<Integer> ignoreIds)
{
List<DetailedAuditEventRow> domainAuditEventAllRows = getDomainEventLog(projectName, domainName, ignoreIds);

List<String> domainEventComments = new ArrayList<>();
domainAuditEventAllRows.forEach((event)->domainEventComments.add(event.comment));
return domainEventComments;
return getDomainAuditEventLog(projectName, domainName, ignoreIds, null).stream().map(event -> event.comment).toList();
}

public Set<Integer> getDomainEventIdsFromPropertyEvents(List<Map<String, Object>> domainPropertyEventRows)
{
Set<Integer> domainEventIds = new HashSet<>();

for(Map<String, Object> row : domainPropertyEventRows)
for (Map<String, Object> row : domainPropertyEventRows)
{
domainEventIds.add(getLogColumnIntValue(row, "domaineventid"));
}

return domainEventIds;
}

private List<DetailedAuditEventRow> getDomainEventLog(String projectName, String domainName, @Nullable Collection<Integer> ignoreIds)
private List<DetailedAuditEventRow> getDomainAuditEventLog(String projectName, String domainName, @Nullable Collection<Integer> ignoreIds, @Nullable Integer maxRows)
{
TestLogger.log("Get a list of the Domain Events for project '" + projectName + "'. ");
domainName = domainName.trim();

Connection cn = WebTestHelper.getRemoteApiConnection();
SelectRowsCommand cmd = new SelectRowsCommand("auditLog", "DomainAuditEvent");
cmd.setRequiredVersion(9.1);
cmd.setColumns(Arrays.asList("rowid", "domainuri", "domainname", "comment", "usercomment", "oldvalues", "newvalues", "datachanges"));
cmd.addFilter("projectid/DisplayName", projectName, Filter.Operator.EQUAL);
if(null != ignoreIds)
cmd.addFilter("domainname", domainName, Filter.Operator.EQUAL);
if (null != ignoreIds)
{
StringBuilder stringBuilder = new StringBuilder();
ignoreIds.forEach((id)->{
if(!stringBuilder.isEmpty())
stringBuilder.append(";");
stringBuilder.append(id);
});
cmd.addFilter("rowId", stringBuilder, Filter.Operator.NOT_IN);
String rowIds = StringUtils.join(ignoreIds, ";");
cmd.addFilter("rowId", rowIds, Filter.Operator.NOT_IN);
}
cmd.setContainerFilter(ContainerFilter.AllFolders);
cmd.setSorts(Arrays.asList(new Sort("RowId", Sort.Direction.DESCENDING)));
cmd.setSorts(List.of(new Sort("RowId", Sort.Direction.DESCENDING)));

if (maxRows != null)
cmd.setMaxRows(maxRows);

List<Map<String, Object>> domainAuditEventAllRows = executeSelectCommand(cn, cmd);
TestLogger.log("Number of 'Domain Event' log entries for '" + projectName + "': " + domainAuditEventAllRows.size());
TestLogger.log(String.format("Number of Domain Event log entries for domain '%s' in '%s': %d", domainName, projectName, domainAuditEventAllRows.size()));

TestLogger.log("Filter the list to look only at '" + domainName + "'.");
List<DetailedAuditEventRow> domainAuditEventRows = new ArrayList<>();

for(Map<String, Object> row : domainAuditEventAllRows)
for (Map<String, Object> row : domainAuditEventAllRows)
{
String domainName_ = getLogColumnValue(row, "domainname");

if(domainName_.trim().equalsIgnoreCase(domainName.trim()))
{
Integer rowId = getLogColumnIntValue(row, "rowid");
String comment = getLogColumnValue(row, "comment");
String userComment = getLogColumnValue(row, "usercomment");
String oldValue = getLogColumnValue(row, "oldvalues");
String newValue = getLogColumnValue(row, "newvalues");
String dataChanges = getLogColumnDisplayValue(row, "dataChanges");
domainAuditEventRows.add(new DetailedAuditEventRow(rowId, domainName, null, comment, userComment, oldValue, newValue, dataChanges));
}
String eventDomainName = getLogColumnValue(row, "domainname");
Integer rowId = getLogColumnIntValue(row, "rowid");
String comment = getLogColumnValue(row, "comment");
String userComment = getLogColumnValue(row, "usercomment");
String oldValue = getLogColumnValue(row, "oldvalues");
String newValue = getLogColumnValue(row, "newvalues");
String dataChanges = getLogColumnDisplayValue(row, "dataChanges");
domainAuditEventRows.add(new DetailedAuditEventRow(rowId, eventDomainName, null, comment, userComment, oldValue, newValue, dataChanges));
}

return domainAuditEventRows;
Expand All @@ -469,15 +479,10 @@ private List<Map<String, Object>> getDomainPropertyEventLog(String domainName, @
cmd.setColumns(Arrays.asList("Created", "CreatedBy", "ImpersonatedBy", "propertyname", "action", "domainname", "domaineventid", "Comment", "UserComment", "oldvalues", "newvalues", "datachanges"));
cmd.addFilter("domainname", domainName, Filter.Operator.EQUAL);

if(null != eventIds)
if (null != eventIds)
{
StringBuilder stringBuilder = new StringBuilder();
eventIds.forEach((id)->{
if(!stringBuilder.isEmpty())
stringBuilder.append(";");
stringBuilder.append(id);
});
cmd.addFilter("domaineventid/rowid", stringBuilder, Filter.Operator.IN);
String rowIds = StringUtils.join(eventIds, ";");
cmd.addFilter("domaineventid/rowid", rowIds, Filter.Operator.IN);
}

cmd.setContainerFilter(ContainerFilter.AllFolders);
Expand All @@ -494,7 +499,7 @@ private List<Map<String, Object>> executeSelectCommand(Connection cn, SelectRows
TestLogger.log("Number of rows: " + response.getRowCount());
rowsReturned.addAll(response.getRows());
}
catch(IOException | CommandException ex)
catch (IOException | CommandException ex)
{
// Just fail here, don't toss the exception up the stack.
fail("There was a command exception when getting the log: " + ex);
Expand Down Expand Up @@ -551,7 +556,7 @@ private String getLogColumnValue(Map<String, Object> rowEntry, String columnName
return null;
return value.toString();
}
catch(JSONException je)
catch (JSONException je)
{
// Just fail here, don't toss the exception up the stack.
throw new IllegalArgumentException(je);
Expand All @@ -577,7 +582,7 @@ private Integer getLogColumnIntValue(Map<String, Object> rowEntry, String column
return null;
return parseInt(strVal);
}
catch(JSONException je)
catch (JSONException je)
{
// Just fail here, don't toss the exception up the stack.
throw new IllegalArgumentException(je);
Expand Down