Skip to content

Commit 66d6bf4

Browse files
bbimberlabkey-jeckels
bbimber
and
labkey-jeckels
authored
Make DatasetUpdateService respect isBulkLoad() for audit purposes (#6791)
* Make DatasetUpdateService respect isBulkLoad() for audit purposes * Add test case * Use constants * No need to register handler for bulk loads --------- Co-authored-by: labkey-jeckels <jeckels@labkey.com>
1 parent 099c775 commit 66d6bf4

File tree

2 files changed

+209
-52
lines changed

2 files changed

+209
-52
lines changed

‎study/src/org/labkey/study/model/DatasetDefinition.java

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2785,6 +2785,11 @@ private void deleteProvenance(Container c, User u, Collection<String> lsids)
27852785

27862786
@Override
27872787
public void deleteDatasetRows(User u, Collection<String> lsids)
2788+
{
2789+
deleteDatasetRows(u, lsids, false);
2790+
}
2791+
2792+
public void deleteDatasetRows(User u, Collection<String> lsids, boolean isBulkLoad)
27882793
{
27892794
// Need to fetch the old item in order to log the deletion
27902795
List<Map<String, Object>> oldDatas = getDatasetRows(u, lsids);
@@ -2794,7 +2799,10 @@ public void deleteDatasetRows(User u, Collection<String> lsids)
27942799
deleteProvenance(getContainer(), u, lsids);
27952800
deleteRows(lsids);
27962801

2797-
new DatasetAuditHandler(this).addAuditEvent(u, getContainer(), getTableInfo(u), AuditBehaviorType.DETAILED, null, QueryService.AuditAction.DELETE, oldDatas, null);
2802+
if (!isBulkLoad)
2803+
{
2804+
new DatasetAuditHandler(this).addAuditEvent(u, getContainer(), getTableInfo(u), AuditBehaviorType.DETAILED, null, QueryService.AuditAction.DELETE, oldDatas, null);
2805+
}
27982806

27992807
transaction.commit();
28002808
}

‎study/src/org/labkey/study/query/DatasetUpdateService.java

Lines changed: 200 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
import org.junit.Assert;
2424
import org.junit.Before;
2525
import org.junit.Test;
26+
import org.labkey.api.audit.AbstractAuditTypeProvider;
2627
import org.labkey.api.collections.CaseInsensitiveHashMap;
2728
import org.labkey.api.collections.CaseInsensitiveHashSet;
2829
import org.labkey.api.collections.ResultSetRowMapFactory;
@@ -32,11 +33,11 @@
3233
import org.labkey.api.data.ContainerManager;
3334
import org.labkey.api.data.DbScope;
3435
import org.labkey.api.data.JdbcType;
35-
import org.labkey.api.data.MVDisplayColumn;
3636
import org.labkey.api.data.MvUtil;
3737
import org.labkey.api.data.RuntimeSQLException;
3838
import org.labkey.api.data.SQLFragment;
3939
import org.labkey.api.data.SimpleFilter;
40+
import org.labkey.api.data.Sort;
4041
import org.labkey.api.data.SqlExecutor;
4142
import org.labkey.api.data.TableInfo;
4243
import org.labkey.api.data.TableSelector;
@@ -78,7 +79,9 @@
7879
import org.labkey.api.util.DateUtil;
7980
import org.labkey.api.util.GUID;
8081
import org.labkey.api.util.JunitUtil;
82+
import org.labkey.api.util.PageFlowUtil;
8183
import org.labkey.api.util.TestContext;
84+
import org.labkey.study.dataset.DatasetAuditProvider;
8285
import org.labkey.study.model.DatasetDefinition;
8386
import org.labkey.study.model.DatasetDomainKind;
8487
import org.labkey.study.model.SecurityType;
@@ -88,6 +91,7 @@
8891

8992
import java.sql.SQLException;
9093
import java.util.ArrayList;
94+
import java.util.Arrays;
9195
import java.util.Collections;
9296
import java.util.Date;
9397
import java.util.HashMap;
@@ -675,8 +679,11 @@ protected Map<String, Object> updateRow(User user, Container container, Map<Stri
675679
resetCreatedColumnsSQL.add(newLsid);
676680
new SqlExecutor(_dataset.getStorageTableInfo().getSchema()).execute(resetCreatedColumnsSQL);
677681

678-
new DatasetDefinition.DatasetAuditHandler(_dataset).addAuditEvent(user, container, target, AuditBehaviorType.DETAILED, null, QueryService.AuditAction.UPDATE,
679-
List.of(mergeData), List.of(oldData));
682+
if (!isBulkLoad())
683+
{
684+
new DatasetDefinition.DatasetAuditHandler(_dataset).addAuditEvent(user, container, target, AuditBehaviorType.DETAILED, null, QueryService.AuditAction.UPDATE,
685+
List.of(mergeData), List.of(oldData));
686+
}
680687

681688
// Successfully updated
682689
transaction.commit();
@@ -734,7 +741,7 @@ protected Map<String, Object> deleteRow(User user, Container container, Map<Stri
734741
{
735742
// Make sure we've found the original participant before doing the delete
736743
String participant = getParticipant(oldRow, user, container);
737-
_dataset.deleteDatasetRows(user, Collections.singleton(keyFromMap(oldRow)));
744+
_dataset.deleteDatasetRows(user, Collections.singleton(keyFromMap(oldRow)), isBulkLoad());
738745
_potentiallyDeletedParticipants.add(participant);
739746
_participantVisitResyncRequired = true;
740747
return oldRow;
@@ -812,61 +819,203 @@ else if (lsids.length > 1)
812819
@TestWhen(TestWhen.When.BVT)
813820
public static class TestCase extends Assert
814821
{
822+
private static final String SUBJECT_COLUMN_NAME = "SubjectID";
823+
private static final String DATASET_NAME = "DS1";
815824
TestContext _context = null;
816825
User _user = null;
817826
Container _container = null;
818827
StudyImpl _junitStudy = null;
819828
StudyManager _manager = StudyManager.getInstance();
820829
String longName = "this is a very long name (with punctuation) that raises many questions \"?\" about your database design choices";
821830

822-
@Test
823-
public void updateRowTest() throws Exception
831+
private void createDataset() throws Exception
824832
{
825-
var dsd = new DatasetDefinition(_junitStudy, 1001, "DS1", "DS1", null, null, null);
833+
if (DefaultSchema.get(_user, _container).getSchema(StudyQuerySchema.SCHEMA_NAME).getTable(DATASET_NAME) != null)
834+
{
835+
return;
836+
}
837+
838+
var dsd = new DatasetDefinition(_junitStudy, 1001, DATASET_NAME, DATASET_NAME, null, null, null);
826839
_manager.createDatasetDefinition(_user, dsd);
827840
dsd = _manager.getDatasetDefinition(_junitStudy, 1001);
828841
dsd.refreshDomain();
829842
{
830-
var domain = dsd.getDomain();
831-
DomainProperty p;
832-
833-
p = domain.addProperty();
834-
p.setName("Field1");
835-
p.setPropertyURI(domain.getTypeURI() + "." + Lsid.encodePart(p.getName()));
836-
p.setRangeURI(PropertyType.getFromJdbcType(JdbcType.VARCHAR).getTypeUri());
837-
838-
p = domain.addProperty();
839-
p.setName("SELECT");
840-
p.setPropertyURI(domain.getTypeURI() + "." + Lsid.encodePart(p.getName()));
841-
p.setRangeURI(PropertyType.getFromJdbcType(JdbcType.VARCHAR).getTypeUri());
842-
843-
p = domain.addProperty();
844-
p.setName(longName);
845-
p.setPropertyURI(domain.getTypeURI() + "." + Lsid.encodePart(p.getName()));
846-
p.setRangeURI(PropertyType.getFromJdbcType(JdbcType.VARCHAR).getTypeUri());
847-
848-
p = domain.addProperty();
849-
p.setName("Value1");
850-
p.setPropertyURI(domain.getTypeURI() + "." + Lsid.encodePart(p.getName()));
851-
p.setRangeURI(PropertyType.getFromJdbcType(JdbcType.DOUBLE).getTypeUri());
852-
p.setMvEnabled(true);
853-
854-
p = domain.addProperty();
855-
p.setName("Value2");
856-
p.setPropertyURI(domain.getTypeURI() + "." + Lsid.encodePart(p.getName()));
857-
p.setRangeURI(PropertyType.getFromJdbcType(JdbcType.DOUBLE).getTypeUri());
858-
p.setMvEnabled(true);
859-
860-
p = domain.addProperty();
861-
p.setName("Value3");
862-
p.setPropertyURI(domain.getTypeURI() + "." + Lsid.encodePart(p.getName()));
863-
p.setRangeURI(PropertyType.getFromJdbcType(JdbcType.DOUBLE).getTypeUri());
864-
p.setMvEnabled(true);
865-
866-
domain.save(_user);
843+
var domain = dsd.getDomain();
844+
DomainProperty p;
845+
846+
p = domain.addProperty();
847+
p.setName("Field1");
848+
p.setPropertyURI(domain.getTypeURI() + "." + Lsid.encodePart(p.getName()));
849+
p.setRangeURI(PropertyType.getFromJdbcType(JdbcType.VARCHAR).getTypeUri());
850+
851+
p = domain.addProperty();
852+
p.setName("SELECT");
853+
p.setPropertyURI(domain.getTypeURI() + "." + Lsid.encodePart(p.getName()));
854+
p.setRangeURI(PropertyType.getFromJdbcType(JdbcType.VARCHAR).getTypeUri());
855+
856+
p = domain.addProperty();
857+
p.setName(longName);
858+
p.setPropertyURI(domain.getTypeURI() + "." + Lsid.encodePart(p.getName()));
859+
p.setRangeURI(PropertyType.getFromJdbcType(JdbcType.VARCHAR).getTypeUri());
860+
861+
p = domain.addProperty();
862+
p.setName("Value1");
863+
p.setPropertyURI(domain.getTypeURI() + "." + Lsid.encodePart(p.getName()));
864+
p.setRangeURI(PropertyType.getFromJdbcType(JdbcType.DOUBLE).getTypeUri());
865+
p.setMvEnabled(true);
866+
867+
p = domain.addProperty();
868+
p.setName("Value2");
869+
p.setPropertyURI(domain.getTypeURI() + "." + Lsid.encodePart(p.getName()));
870+
p.setRangeURI(PropertyType.getFromJdbcType(JdbcType.DOUBLE).getTypeUri());
871+
p.setMvEnabled(true);
872+
873+
p = domain.addProperty();
874+
p.setName("Value3");
875+
p.setPropertyURI(domain.getTypeURI() + "." + Lsid.encodePart(p.getName()));
876+
p.setRangeURI(PropertyType.getFromJdbcType(JdbcType.DOUBLE).getTypeUri());
877+
p.setMvEnabled(true);
878+
879+
domain.save(_user);
880+
}
881+
}
882+
883+
private long getDatasetAuditRowCount()
884+
{
885+
return new TableSelector(QueryService.get().getUserSchema(_user, _container, AbstractAuditTypeProvider.QUERY_SCHEMA_NAME).getTable(DatasetAuditProvider.DATASET_AUDIT_EVENT)).getRowCount();
886+
}
887+
888+
private String getLatestAuditMessage()
889+
{
890+
return new TableSelector(QueryService.get().getUserSchema(_user, _container, AbstractAuditTypeProvider.QUERY_SCHEMA_NAME).getTable(DatasetAuditProvider.DATASET_AUDIT_EVENT), PageFlowUtil.set("Comment"), null, new Sort("-created")).setMaxRows(1).getObject(String.class);
891+
}
892+
893+
@Test
894+
public void testAuditing() throws Exception
895+
{
896+
createDataset();
897+
TableInfo t = DefaultSchema.get(_user, _container).getSchema(StudyQuerySchema.SCHEMA_NAME).getTable(DATASET_NAME);
898+
t.getUpdateService().truncateRows(_user, _container, null, null);
899+
900+
final QueryUpdateService qus = t.getUpdateService();
901+
BatchValidationException errors = new BatchValidationException();
902+
903+
long actualAuditRows = getDatasetAuditRowCount();
904+
long expectedAuditRows;
905+
906+
List<Map<String, Object>> insertedRows = qus.insertRows(_user, _container,
907+
List.of(Map.of(
908+
SUBJECT_COLUMN_NAME, "S1",
909+
"SequenceNum", "1.2345",
910+
longName, "NA"),
911+
Map.of(
912+
SUBJECT_COLUMN_NAME, "S2",
913+
"SequenceNum", "1.2345",
914+
longName, "WithoutBulkLoad")),
915+
errors, null, null);
916+
917+
if (errors.hasErrors())
918+
{
919+
fail(errors.getMessage());
867920
}
868921

869-
TableInfo t = DefaultSchema.get(_user, _container).getSchema("study").getTable("DS1");
922+
expectedAuditRows = actualAuditRows + 2;
923+
actualAuditRows = getDatasetAuditRowCount();
924+
Assert.assertEquals("Incorrect number of audit records", expectedAuditRows, actualAuditRows);
925+
Assert.assertEquals("Incorrect comment", "A new dataset record was inserted", getLatestAuditMessage());
926+
927+
qus.insertRows(_user, _container,
928+
List.of(Map.of(
929+
SUBJECT_COLUMN_NAME, "S3",
930+
"SequenceNum", "1.2345",
931+
longName, "WithoutBulkLoad")),
932+
errors, null, null);
933+
934+
if (errors.hasErrors())
935+
{
936+
fail(errors.getMessage());
937+
}
938+
939+
expectedAuditRows = actualAuditRows + 1;
940+
actualAuditRows = getDatasetAuditRowCount();
941+
Assert.assertEquals("Incorrect number of audit records", expectedAuditRows, actualAuditRows);
942+
943+
// Now update:
944+
insertedRows.get(0).put(longName, "NewValue");
945+
insertedRows.get(1).put(longName, "NewValue");
946+
List<Map<String, Object>> oldKeys = Arrays.asList(
947+
Map.of("lsid", insertedRows.get(0).get("lsid")),
948+
Map.of("lsid", insertedRows.get(1).get("lsid"))
949+
);
950+
qus.updateRows(_user, _container, insertedRows, oldKeys, errors, null, null);
951+
if (errors.hasErrors())
952+
{
953+
fail(errors.getMessage());
954+
}
955+
956+
expectedAuditRows = actualAuditRows + 2;
957+
actualAuditRows = getDatasetAuditRowCount();
958+
Assert.assertEquals("Incorrect number of audit records", expectedAuditRows, actualAuditRows);
959+
Assert.assertEquals("Incorrect comment", "A dataset record was modified", getLatestAuditMessage());
960+
961+
qus.deleteRows(_user, _container, oldKeys, null, null);
962+
expectedAuditRows = actualAuditRows + 2;
963+
actualAuditRows = getDatasetAuditRowCount();
964+
Assert.assertEquals("Incorrect number of audit records", expectedAuditRows, actualAuditRows);
965+
Assert.assertEquals("Incorrect comment", "A dataset record was deleted", getLatestAuditMessage());
966+
967+
// Repeat using bulkLoad=true:
968+
qus.setBulkLoad(true);
969+
970+
insertedRows = qus.insertRows(_user, _container,
971+
List.of(Map.of(
972+
SUBJECT_COLUMN_NAME, "S4",
973+
"SequenceNum", "1.2345",
974+
longName, "WithBulkLoad"),
975+
Map.of(
976+
SUBJECT_COLUMN_NAME, "S5",
977+
"SequenceNum", "1.2345",
978+
longName, "WithBulkLoad")),
979+
errors, null, null);
980+
981+
if (errors.hasErrors())
982+
{
983+
fail(errors.getMessage());
984+
}
985+
986+
expectedAuditRows = actualAuditRows;
987+
actualAuditRows = getDatasetAuditRowCount();
988+
Assert.assertEquals("Incorrect number of audit records", expectedAuditRows, actualAuditRows);
989+
990+
// Now update:
991+
insertedRows.get(0).put(longName, "NewValue");
992+
insertedRows.get(1).put(longName, "NewValue");
993+
oldKeys = Arrays.asList(
994+
Map.of("lsid", insertedRows.get(0).get("lsid")),
995+
Map.of("lsid", insertedRows.get(1).get("lsid"))
996+
);
997+
qus.updateRows(_user, _container, insertedRows, oldKeys, errors, null, null);
998+
if (errors.hasErrors())
999+
{
1000+
fail(errors.getMessage());
1001+
}
1002+
1003+
expectedAuditRows = actualAuditRows;
1004+
actualAuditRows = getDatasetAuditRowCount();
1005+
Assert.assertEquals("Incorrect number of audit records", expectedAuditRows, actualAuditRows);
1006+
1007+
qus.deleteRows(_user, _container, oldKeys, null, null);
1008+
expectedAuditRows = actualAuditRows;
1009+
actualAuditRows = getDatasetAuditRowCount();
1010+
Assert.assertEquals("Incorrect number of audit records", expectedAuditRows, actualAuditRows);
1011+
}
1012+
1013+
@Test
1014+
public void updateRowTest() throws Exception
1015+
{
1016+
createDataset();
1017+
1018+
TableInfo t = DefaultSchema.get(_user, _container).getSchema(StudyQuerySchema.SCHEMA_NAME).getTable(DATASET_NAME);
8701019
assertNotNull(t);
8711020
assertTrue("Field1".equalsIgnoreCase(t.getColumn("Field1").getAlias()));
8721021
assertFalse("SELECT".equalsIgnoreCase(t.getColumn("SELECT").getAlias()));
@@ -877,7 +1026,7 @@ public void updateRowTest() throws Exception
8771026

8781027
var result = up.insertRows(_user, _container,
8791028
List.of(Map.of(
880-
"subjectid", "S1",
1029+
SUBJECT_COLUMN_NAME, "S1",
8811030
"SequenceNum", "1.2345",
8821031
"Field1", "f",
8831032
"SELECT", "s",
@@ -892,7 +1041,7 @@ public void updateRowTest() throws Exception
8921041
assertNotNull(result);
8931042
assertEquals(1, result.size());
8941043
var map = result.get(0);
895-
assertEquals("S1", map.get("SubjectId"));
1044+
assertEquals("S1", map.get(SUBJECT_COLUMN_NAME));
8961045
assertEquals("f", map.get("Field1"));
8971046
assertEquals("s", map.get("SELECT"));
8981047
assertEquals("l", map.get(longName));
@@ -909,15 +1058,15 @@ public void updateRowTest() throws Exception
9091058

9101059
// update subjectid column
9111060
result = up.updateRows(_user, _container,
912-
List.of(Map.of("subjectid", "S2")),
1061+
List.of(Map.of(SUBJECT_COLUMN_NAME, "S2")),
9131062
List.of(Map.of("lsid", lsid)),
9141063
errors, null, null);
9151064
if (errors.hasErrors())
9161065
fail(errors.getMessage());
9171066
assertNotNull(result);
9181067
assertEquals(1, result.size());
9191068
map = result.get(0);
920-
assertEquals("S2", map.get("SubjectId"));
1069+
assertEquals("S2", map.get(SUBJECT_COLUMN_NAME));
9211070
// All other columns are preserved
9221071
assertEquals("f", map.get("Field1"));
9231072
assertEquals("s", map.get("SELECT"));
@@ -950,7 +1099,7 @@ public void updateRowTest() throws Exception
9501099
assertNotNull(result);
9511100
assertEquals(1, result.size());
9521101
map = result.get(0);
953-
assertEquals("S2", map.get("SubjectId"));
1102+
assertEquals("S2", map.get(SUBJECT_COLUMN_NAME));
9541103
assertEquals("fUpdated", map.get("Field1"));
9551104
assertEquals("sUpdated", map.get("SELECT"));
9561105
assertEquals("lUpdated", map.get(longName));
@@ -977,7 +1126,7 @@ public void createStudy()
9771126
StudyImpl s = new StudyImpl(c, "Junit Study");
9781127
s.setTimepointType(TimepointType.VISIT);
9791128
s.setStartDate(new Date(DateUtil.parseDateTime(c, "2014-01-01")));
980-
s.setSubjectColumnName("SubjectID");
1129+
s.setSubjectColumnName(SUBJECT_COLUMN_NAME);
9811130
s.setSubjectNounPlural("Subjects");
9821131
s.setSubjectNounSingular("Subject");
9831132
s.setSecurityType(SecurityType.BASIC_WRITE);

0 commit comments

Comments
 (0)