diff --git a/study/src/org/labkey/study/model/DatasetDefinition.java b/study/src/org/labkey/study/model/DatasetDefinition.java index 906b36a4fc1..175cfd40ec9 100644 --- a/study/src/org/labkey/study/model/DatasetDefinition.java +++ b/study/src/org/labkey/study/model/DatasetDefinition.java @@ -2785,6 +2785,11 @@ private void deleteProvenance(Container c, User u, Collection lsids) @Override public void deleteDatasetRows(User u, Collection lsids) + { + deleteDatasetRows(u, lsids, false); + } + + public void deleteDatasetRows(User u, Collection lsids, boolean isBulkLoad) { // Need to fetch the old item in order to log the deletion List> oldDatas = getDatasetRows(u, lsids); @@ -2794,7 +2799,10 @@ public void deleteDatasetRows(User u, Collection lsids) deleteProvenance(getContainer(), u, lsids); deleteRows(lsids); - new DatasetAuditHandler(this).addAuditEvent(u, getContainer(), getTableInfo(u), AuditBehaviorType.DETAILED, null, QueryService.AuditAction.DELETE, oldDatas, null); + if (!isBulkLoad) + { + new DatasetAuditHandler(this).addAuditEvent(u, getContainer(), getTableInfo(u), AuditBehaviorType.DETAILED, null, QueryService.AuditAction.DELETE, oldDatas, null); + } transaction.commit(); } diff --git a/study/src/org/labkey/study/query/DatasetUpdateService.java b/study/src/org/labkey/study/query/DatasetUpdateService.java index e9af4a66b50..6838e8f30f1 100644 --- a/study/src/org/labkey/study/query/DatasetUpdateService.java +++ b/study/src/org/labkey/study/query/DatasetUpdateService.java @@ -23,6 +23,7 @@ import org.junit.Assert; import org.junit.Before; import org.junit.Test; +import org.labkey.api.audit.AbstractAuditTypeProvider; import org.labkey.api.collections.CaseInsensitiveHashMap; import org.labkey.api.collections.CaseInsensitiveHashSet; import org.labkey.api.collections.ResultSetRowMapFactory; @@ -32,11 +33,11 @@ import org.labkey.api.data.ContainerManager; import org.labkey.api.data.DbScope; import org.labkey.api.data.JdbcType; -import org.labkey.api.data.MVDisplayColumn; import org.labkey.api.data.MvUtil; import org.labkey.api.data.RuntimeSQLException; import org.labkey.api.data.SQLFragment; import org.labkey.api.data.SimpleFilter; +import org.labkey.api.data.Sort; import org.labkey.api.data.SqlExecutor; import org.labkey.api.data.TableInfo; import org.labkey.api.data.TableSelector; @@ -78,7 +79,9 @@ import org.labkey.api.util.DateUtil; import org.labkey.api.util.GUID; import org.labkey.api.util.JunitUtil; +import org.labkey.api.util.PageFlowUtil; import org.labkey.api.util.TestContext; +import org.labkey.study.dataset.DatasetAuditProvider; import org.labkey.study.model.DatasetDefinition; import org.labkey.study.model.DatasetDomainKind; import org.labkey.study.model.SecurityType; @@ -88,6 +91,7 @@ import java.sql.SQLException; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collections; import java.util.Date; import java.util.HashMap; @@ -675,8 +679,11 @@ protected Map updateRow(User user, Container container, Map deleteRow(User user, Container container, Map 1) @TestWhen(TestWhen.When.BVT) public static class TestCase extends Assert { + private static final String SUBJECT_COLUMN_NAME = "SubjectID"; + private static final String DATASET_NAME = "DS1"; TestContext _context = null; User _user = null; Container _container = null; @@ -819,54 +828,194 @@ public static class TestCase extends Assert StudyManager _manager = StudyManager.getInstance(); String longName = "this is a very long name (with punctuation) that raises many questions \"?\" about your database design choices"; - @Test - public void updateRowTest() throws Exception + private void createDataset() throws Exception { - var dsd = new DatasetDefinition(_junitStudy, 1001, "DS1", "DS1", null, null, null); + if (DefaultSchema.get(_user, _container).getSchema(StudyQuerySchema.SCHEMA_NAME).getTable(DATASET_NAME) != null) + { + return; + } + + var dsd = new DatasetDefinition(_junitStudy, 1001, DATASET_NAME, DATASET_NAME, null, null, null); _manager.createDatasetDefinition(_user, dsd); dsd = _manager.getDatasetDefinition(_junitStudy, 1001); dsd.refreshDomain(); { - var domain = dsd.getDomain(); - DomainProperty p; - - p = domain.addProperty(); - p.setName("Field1"); - p.setPropertyURI(domain.getTypeURI() + "." + Lsid.encodePart(p.getName())); - p.setRangeURI(PropertyType.getFromJdbcType(JdbcType.VARCHAR).getTypeUri()); - - p = domain.addProperty(); - p.setName("SELECT"); - p.setPropertyURI(domain.getTypeURI() + "." + Lsid.encodePart(p.getName())); - p.setRangeURI(PropertyType.getFromJdbcType(JdbcType.VARCHAR).getTypeUri()); - - p = domain.addProperty(); - p.setName(longName); - p.setPropertyURI(domain.getTypeURI() + "." + Lsid.encodePart(p.getName())); - p.setRangeURI(PropertyType.getFromJdbcType(JdbcType.VARCHAR).getTypeUri()); - - p = domain.addProperty(); - p.setName("Value1"); - p.setPropertyURI(domain.getTypeURI() + "." + Lsid.encodePart(p.getName())); - p.setRangeURI(PropertyType.getFromJdbcType(JdbcType.DOUBLE).getTypeUri()); - p.setMvEnabled(true); - - p = domain.addProperty(); - p.setName("Value2"); - p.setPropertyURI(domain.getTypeURI() + "." + Lsid.encodePart(p.getName())); - p.setRangeURI(PropertyType.getFromJdbcType(JdbcType.DOUBLE).getTypeUri()); - p.setMvEnabled(true); - - p = domain.addProperty(); - p.setName("Value3"); - p.setPropertyURI(domain.getTypeURI() + "." + Lsid.encodePart(p.getName())); - p.setRangeURI(PropertyType.getFromJdbcType(JdbcType.DOUBLE).getTypeUri()); - p.setMvEnabled(true); - - domain.save(_user); + var domain = dsd.getDomain(); + DomainProperty p; + + p = domain.addProperty(); + p.setName("Field1"); + p.setPropertyURI(domain.getTypeURI() + "." + Lsid.encodePart(p.getName())); + p.setRangeURI(PropertyType.getFromJdbcType(JdbcType.VARCHAR).getTypeUri()); + + p = domain.addProperty(); + p.setName("SELECT"); + p.setPropertyURI(domain.getTypeURI() + "." + Lsid.encodePart(p.getName())); + p.setRangeURI(PropertyType.getFromJdbcType(JdbcType.VARCHAR).getTypeUri()); + + p = domain.addProperty(); + p.setName(longName); + p.setPropertyURI(domain.getTypeURI() + "." + Lsid.encodePart(p.getName())); + p.setRangeURI(PropertyType.getFromJdbcType(JdbcType.VARCHAR).getTypeUri()); + + p = domain.addProperty(); + p.setName("Value1"); + p.setPropertyURI(domain.getTypeURI() + "." + Lsid.encodePart(p.getName())); + p.setRangeURI(PropertyType.getFromJdbcType(JdbcType.DOUBLE).getTypeUri()); + p.setMvEnabled(true); + + p = domain.addProperty(); + p.setName("Value2"); + p.setPropertyURI(domain.getTypeURI() + "." + Lsid.encodePart(p.getName())); + p.setRangeURI(PropertyType.getFromJdbcType(JdbcType.DOUBLE).getTypeUri()); + p.setMvEnabled(true); + + p = domain.addProperty(); + p.setName("Value3"); + p.setPropertyURI(domain.getTypeURI() + "." + Lsid.encodePart(p.getName())); + p.setRangeURI(PropertyType.getFromJdbcType(JdbcType.DOUBLE).getTypeUri()); + p.setMvEnabled(true); + + domain.save(_user); + } + } + + private long getDatasetAuditRowCount() + { + return new TableSelector(QueryService.get().getUserSchema(_user, _container, AbstractAuditTypeProvider.QUERY_SCHEMA_NAME).getTable(DatasetAuditProvider.DATASET_AUDIT_EVENT)).getRowCount(); + } + + private String getLatestAuditMessage() + { + 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); + } + + @Test + public void testAuditing() throws Exception + { + createDataset(); + TableInfo t = DefaultSchema.get(_user, _container).getSchema(StudyQuerySchema.SCHEMA_NAME).getTable(DATASET_NAME); + t.getUpdateService().truncateRows(_user, _container, null, null); + + final QueryUpdateService qus = t.getUpdateService(); + BatchValidationException errors = new BatchValidationException(); + + long actualAuditRows = getDatasetAuditRowCount(); + long expectedAuditRows; + + List> insertedRows = qus.insertRows(_user, _container, + List.of(Map.of( + SUBJECT_COLUMN_NAME, "S1", + "SequenceNum", "1.2345", + longName, "NA"), + Map.of( + SUBJECT_COLUMN_NAME, "S2", + "SequenceNum", "1.2345", + longName, "WithoutBulkLoad")), + errors, null, null); + + if (errors.hasErrors()) + { + fail(errors.getMessage()); } - TableInfo t = DefaultSchema.get(_user, _container).getSchema("study").getTable("DS1"); + expectedAuditRows = actualAuditRows + 2; + actualAuditRows = getDatasetAuditRowCount(); + Assert.assertEquals("Incorrect number of audit records", expectedAuditRows, actualAuditRows); + Assert.assertEquals("Incorrect comment", "A new dataset record was inserted", getLatestAuditMessage()); + + qus.insertRows(_user, _container, + List.of(Map.of( + SUBJECT_COLUMN_NAME, "S3", + "SequenceNum", "1.2345", + longName, "WithoutBulkLoad")), + errors, null, null); + + if (errors.hasErrors()) + { + fail(errors.getMessage()); + } + + expectedAuditRows = actualAuditRows + 1; + actualAuditRows = getDatasetAuditRowCount(); + Assert.assertEquals("Incorrect number of audit records", expectedAuditRows, actualAuditRows); + + // Now update: + insertedRows.get(0).put(longName, "NewValue"); + insertedRows.get(1).put(longName, "NewValue"); + List> oldKeys = Arrays.asList( + Map.of("lsid", insertedRows.get(0).get("lsid")), + Map.of("lsid", insertedRows.get(1).get("lsid")) + ); + qus.updateRows(_user, _container, insertedRows, oldKeys, errors, null, null); + if (errors.hasErrors()) + { + fail(errors.getMessage()); + } + + expectedAuditRows = actualAuditRows + 2; + actualAuditRows = getDatasetAuditRowCount(); + Assert.assertEquals("Incorrect number of audit records", expectedAuditRows, actualAuditRows); + Assert.assertEquals("Incorrect comment", "A dataset record was modified", getLatestAuditMessage()); + + qus.deleteRows(_user, _container, oldKeys, null, null); + expectedAuditRows = actualAuditRows + 2; + actualAuditRows = getDatasetAuditRowCount(); + Assert.assertEquals("Incorrect number of audit records", expectedAuditRows, actualAuditRows); + Assert.assertEquals("Incorrect comment", "A dataset record was deleted", getLatestAuditMessage()); + + // Repeat using bulkLoad=true: + qus.setBulkLoad(true); + + insertedRows = qus.insertRows(_user, _container, + List.of(Map.of( + SUBJECT_COLUMN_NAME, "S4", + "SequenceNum", "1.2345", + longName, "WithBulkLoad"), + Map.of( + SUBJECT_COLUMN_NAME, "S5", + "SequenceNum", "1.2345", + longName, "WithBulkLoad")), + errors, null, null); + + if (errors.hasErrors()) + { + fail(errors.getMessage()); + } + + expectedAuditRows = actualAuditRows; + actualAuditRows = getDatasetAuditRowCount(); + Assert.assertEquals("Incorrect number of audit records", expectedAuditRows, actualAuditRows); + + // Now update: + insertedRows.get(0).put(longName, "NewValue"); + insertedRows.get(1).put(longName, "NewValue"); + oldKeys = Arrays.asList( + Map.of("lsid", insertedRows.get(0).get("lsid")), + Map.of("lsid", insertedRows.get(1).get("lsid")) + ); + qus.updateRows(_user, _container, insertedRows, oldKeys, errors, null, null); + if (errors.hasErrors()) + { + fail(errors.getMessage()); + } + + expectedAuditRows = actualAuditRows; + actualAuditRows = getDatasetAuditRowCount(); + Assert.assertEquals("Incorrect number of audit records", expectedAuditRows, actualAuditRows); + + qus.deleteRows(_user, _container, oldKeys, null, null); + expectedAuditRows = actualAuditRows; + actualAuditRows = getDatasetAuditRowCount(); + Assert.assertEquals("Incorrect number of audit records", expectedAuditRows, actualAuditRows); + } + + @Test + public void updateRowTest() throws Exception + { + createDataset(); + + TableInfo t = DefaultSchema.get(_user, _container).getSchema(StudyQuerySchema.SCHEMA_NAME).getTable(DATASET_NAME); assertNotNull(t); assertTrue("Field1".equalsIgnoreCase(t.getColumn("Field1").getAlias())); assertFalse("SELECT".equalsIgnoreCase(t.getColumn("SELECT").getAlias())); @@ -877,7 +1026,7 @@ public void updateRowTest() throws Exception var result = up.insertRows(_user, _container, List.of(Map.of( - "subjectid", "S1", + SUBJECT_COLUMN_NAME, "S1", "SequenceNum", "1.2345", "Field1", "f", "SELECT", "s", @@ -892,7 +1041,7 @@ public void updateRowTest() throws Exception assertNotNull(result); assertEquals(1, result.size()); var map = result.get(0); - assertEquals("S1", map.get("SubjectId")); + assertEquals("S1", map.get(SUBJECT_COLUMN_NAME)); assertEquals("f", map.get("Field1")); assertEquals("s", map.get("SELECT")); assertEquals("l", map.get(longName)); @@ -909,7 +1058,7 @@ public void updateRowTest() throws Exception // update subjectid column result = up.updateRows(_user, _container, - List.of(Map.of("subjectid", "S2")), + List.of(Map.of(SUBJECT_COLUMN_NAME, "S2")), List.of(Map.of("lsid", lsid)), errors, null, null); if (errors.hasErrors()) @@ -917,7 +1066,7 @@ public void updateRowTest() throws Exception assertNotNull(result); assertEquals(1, result.size()); map = result.get(0); - assertEquals("S2", map.get("SubjectId")); + assertEquals("S2", map.get(SUBJECT_COLUMN_NAME)); // All other columns are preserved assertEquals("f", map.get("Field1")); assertEquals("s", map.get("SELECT")); @@ -950,7 +1099,7 @@ public void updateRowTest() throws Exception assertNotNull(result); assertEquals(1, result.size()); map = result.get(0); - assertEquals("S2", map.get("SubjectId")); + assertEquals("S2", map.get(SUBJECT_COLUMN_NAME)); assertEquals("fUpdated", map.get("Field1")); assertEquals("sUpdated", map.get("SELECT")); assertEquals("lUpdated", map.get(longName)); @@ -977,7 +1126,7 @@ public void createStudy() StudyImpl s = new StudyImpl(c, "Junit Study"); s.setTimepointType(TimepointType.VISIT); s.setStartDate(new Date(DateUtil.parseDateTime(c, "2014-01-01"))); - s.setSubjectColumnName("SubjectID"); + s.setSubjectColumnName(SUBJECT_COLUMN_NAME); s.setSubjectNounPlural("Subjects"); s.setSubjectNounSingular("Subject"); s.setSecurityType(SecurityType.BASIC_WRITE);