From d40f42dd35a1e36ed143a7fa6635ebc85105905c Mon Sep 17 00:00:00 2001 From: bbimber Date: Mon, 23 Jun 2025 05:47:16 -0700 Subject: [PATCH 1/5] Initial commit for JSON-based study definition --- .../api/studies/study/StudyDefinition.java | 627 ++++++++++++++++++ Studies/resources/study/DemoStudy.json | 43 ++ .../org/labkey/studies/StudiesController.java | 51 +- .../org/labkey/studies/StudiesManager.java | 87 ++- .../src/org/labkey/studies/StudiesModule.java | 9 + 5 files changed, 791 insertions(+), 26 deletions(-) create mode 100644 Studies/api-src/org/labkey/api/studies/study/StudyDefinition.java create mode 100644 Studies/resources/study/DemoStudy.json diff --git a/Studies/api-src/org/labkey/api/studies/study/StudyDefinition.java b/Studies/api-src/org/labkey/api/studies/study/StudyDefinition.java new file mode 100644 index 000000000..2def2c865 --- /dev/null +++ b/Studies/api-src/org/labkey/api/studies/study/StudyDefinition.java @@ -0,0 +1,627 @@ +package org.labkey.api.studies.study; + +import com.fasterxml.jackson.annotation.JsonProperty; +import com.fasterxml.jackson.core.JsonProcessingException; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.databind.ObjectWriter; +import org.json.JSONObject; + +import java.util.Date; +import java.util.List; + +public class StudyDefinition +{ + private Integer _rowId; + private String _studyName; + private String _label; + private String _category; + private String _description; + + private String _container; + private Integer _createdBy; + private Date _created; + private Integer _modifiedBy; + private Date _modified; + + private List _cohorts; + private List _anchorEvents; + private List _timepoints; + + public StudyDefinition() + { + + } + + public Integer getRowId() + { + return _rowId; + } + + public void setRowId(Integer rowId) + { + _rowId = rowId; + } + + public String getStudyName() + { + return _studyName; + } + + public void setStudyName(String studyName) + { + _studyName = studyName; + } + + public String getLabel() + { + return _label; + } + + public void setLabel(String label) + { + _label = label; + } + + public String getCategory() + { + return _category; + } + + public void setCategory(String category) + { + _category = category; + } + + public String getDescription() + { + return _description; + } + + public void setDescription(String description) + { + _description = description; + } + + public String getContainer() + { + return _container; + } + + public void setContainer(String container) + { + _container = container; + } + + public Integer getCreatedBy() + { + return _createdBy; + } + + public void setCreatedBy(Integer createdBy) + { + _createdBy = createdBy; + } + + public Date getCreated() + { + return _created; + } + + public void setCreated(Date created) + { + _created = created; + } + + public Integer getModifiedBy() + { + return _modifiedBy; + } + + public void setModifiedBy(Integer modifiedBy) + { + _modifiedBy = modifiedBy; + } + + public Date getModified() + { + return _modified; + } + + public void setModified(Date modified) + { + _modified = modified; + } + + public List getCohorts() + { + return _cohorts; + } + + public void setCohorts(List cohorts) + { + _cohorts = cohorts; + } + + public List getAnchorEvents() + { + return _anchorEvents; + } + + public void setAnchorEvents(List anchorEvents) + { + _anchorEvents = anchorEvents; + } + + public List getTimepoints() + { + return _timepoints; + } + + public void setTimepoints(List timepoints) + { + _timepoints = timepoints; + } + + public static class StudyCohort + { + private Integer _rowId; + private Integer _studyId; + + private String _cohortName; + private String _label; + private String _category; + private String _description; + private Boolean _isControlGroup = false; + private Integer _sortOrder; + + private String _container; + private Integer _createdBy; + private Date _created; + + private Integer _modifiedBy; + private Date _modified; + + public StudyCohort() + { + + } + + public Integer getRowId() + { + return _rowId; + } + + public void setRowId(Integer rowId) + { + _rowId = rowId; + } + + public Integer getStudyId() + { + return _studyId; + } + + public void setStudyId(Integer studyId) + { + _studyId = studyId; + } + + public String getCohortName() + { + return _cohortName; + } + + public void setCohortName(String cohortName) + { + _cohortName = cohortName; + } + + public String getLabel() + { + return _label; + } + + public void setLabel(String label) + { + _label = label; + } + + public String getCategory() + { + return _category; + } + + public void setCategory(String category) + { + _category = category; + } + + public String getDescription() + { + return _description; + } + + public void setDescription(String description) + { + _description = description; + } + + public Boolean getControlGroup() + { + return _isControlGroup; + } + + public void setControlGroup(Boolean controlGroup) + { + _isControlGroup = controlGroup; + } + + public Integer getSortOrder() + { + return _sortOrder; + } + + public void setSortOrder(Integer sortOrder) + { + _sortOrder = sortOrder; + } + + public String getContainer() + { + return _container; + } + + public void setContainer(String container) + { + _container = container; + } + + public Integer getCreatedBy() + { + return _createdBy; + } + + public void setCreatedBy(Integer createdBy) + { + _createdBy = createdBy; + } + + public Date getCreated() + { + return _created; + } + + public void setCreated(Date created) + { + _created = created; + } + + public Integer getModifiedBy() + { + return _modifiedBy; + } + + public void setModifiedBy(Integer modifiedBy) + { + _modifiedBy = modifiedBy; + } + + public Date getModified() + { + return _modified; + } + + public void setModified(Date modified) + { + _modified = modified; + } + } + + public static class AnchorEvent + { + private Integer _rowId; + private Integer _studyId; + + private String _label; + private String _description; + private String _eventProviderName; + + private String _container; + private Integer _createdBy; + private Date _created; + + private Integer _modifiedBy; + private Date _modified; + + public AnchorEvent() + { + + } + + public Integer getRowId() + { + return _rowId; + } + + public void setRowId(Integer rowId) + { + _rowId = rowId; + } + + public Integer getStudyId() + { + return _studyId; + } + + public void setStudyId(Integer studyId) + { + _studyId = studyId; + } + + public String getLabel() + { + return _label; + } + + public void setLabel(String label) + { + _label = label; + } + + public String getDescription() + { + return _description; + } + + public void setDescription(String description) + { + _description = description; + } + + public String getEventProviderName() + { + return _eventProviderName; + } + + public void setEventProviderName(String eventProviderName) + { + _eventProviderName = eventProviderName; + } + + public String getContainer() + { + return _container; + } + + public void setContainer(String container) + { + _container = container; + } + + public Integer getCreatedBy() + { + return _createdBy; + } + + public void setCreatedBy(Integer createdBy) + { + _createdBy = createdBy; + } + + public Date getCreated() + { + return _created; + } + + public void setCreated(Date created) + { + _created = created; + } + + public Integer getModifiedBy() + { + return _modifiedBy; + } + + public void setModifiedBy(Integer modifiedBy) + { + _modifiedBy = modifiedBy; + } + + public Date getModified() + { + return _modified; + } + + public void setModified(Date modified) + { + _modified = modified; + } + } + + public static class Timepoint + { + private Integer _rowId; + private Integer _studyId; + private Integer _cohortId; + private String _label; + private String _labelShort; + private String _description; + + private Integer _anchorEvent; + private Integer _rangeMin; + private Integer _rangeMax; + + private String _container; + private Integer _createdBy; + private Date _created; + + private Integer _modifiedBy; + private Date _modified; + + public Timepoint() + { + + } + + public Integer getRowId() + { + return _rowId; + } + + public void setRowId(Integer rowId) + { + _rowId = rowId; + } + + public Integer getStudyId() + { + return _studyId; + } + + public void setStudyId(Integer studyId) + { + _studyId = studyId; + } + + public Integer getCohortId() + { + return _cohortId; + } + + public void setCohortId(Integer cohortId) + { + _cohortId = cohortId; + } + + public String getLabel() + { + return _label; + } + + public void setLabel(String label) + { + _label = label; + } + + public String getLabelShort() + { + return _labelShort; + } + + public void setLabelShort(String labelShort) + { + _labelShort = labelShort; + } + + public String getDescription() + { + return _description; + } + + public void setDescription(String description) + { + _description = description; + } + + public Integer getAnchorEvent() + { + return _anchorEvent; + } + + public void setAnchorEvent(Integer anchorEvent) + { + _anchorEvent = anchorEvent; + } + + public Integer getRangeMin() + { + return _rangeMin; + } + + public void setRangeMin(Integer rangeMin) + { + _rangeMin = rangeMin; + } + + public Integer getRangeMax() + { + return _rangeMax; + } + + public void setRangeMax(Integer rangeMax) + { + _rangeMax = rangeMax; + } + + public String getContainer() + { + return _container; + } + + public void setContainer(String container) + { + _container = container; + } + + public Integer getCreatedBy() + { + return _createdBy; + } + + public void setCreatedBy(Integer createdBy) + { + _createdBy = createdBy; + } + + public Date getCreated() + { + return _created; + } + + public void setCreated(Date created) + { + _created = created; + } + + public Integer getModifiedBy() + { + return _modifiedBy; + } + + public void setModifiedBy(Integer modifiedBy) + { + _modifiedBy = modifiedBy; + } + + public Date getModified() + { + return _modified; + } + + public void setModified(Date modified) + { + _modified = modified; + } + } + + public static StudyDefinition fromJson(JSONObject json) + { + ObjectMapper mapper = new ObjectMapper(); + + return mapper.convertValue(json, StudyDefinition.class); + } + + public String toJson() throws JsonProcessingException + { + ObjectWriter ow = new ObjectMapper().writer(); + return ow.writeValueAsString(this); + } + + public static StudyDefinition getForId(int studyId) + { + // TODO: implement this. This should query the DB and return a populated StudyDefinition + + return null; + } +} diff --git a/Studies/resources/study/DemoStudy.json b/Studies/resources/study/DemoStudy.json new file mode 100644 index 000000000..e0c8f515d --- /dev/null +++ b/Studies/resources/study/DemoStudy.json @@ -0,0 +1,43 @@ +{ + "studyName": "DemoStudy", + "label": "Demo Study", + "description": "This is a demo study", + "cohorts": [{ + "cohortName": "Group1", + "label": "Group 1", + "description": "This is the first group", + "sortOrder": 1 + },{ + "cohortName": "Control", + "label": "Control Group", + "description": "This is the control group", + "isControlGroup": true, + "sortOrder": 2 + }], + + "anchorEvents": [{ + "label": "Study Enrollment", + "description": "The represents Day 0 of the study", + "eventProviderName": "EnrollmentStart" + }], + "timepoints": [{ + "cohortId": null, + "label": "Day 0", + "labelShort": "D0", + "anchorEvent": "Study Enrollment" + },{ + "cohortName": "Group1", + "label": "Vaccination", + "labelShort": "V", + "anchorEvent": "Study Enrollment", + "minDate": 7, + "maxDate": 10 + },{ + "cohortName": "Control", + "label": "Mock-Vaccination", + "labelShort": "V", + "anchorEvent": "Study Enrollment", + "minDate": 7, + "maxDate": 10 + }] +} \ No newline at end of file diff --git a/Studies/src/org/labkey/studies/StudiesController.java b/Studies/src/org/labkey/studies/StudiesController.java index 58d6b51dd..8de8e05be 100644 --- a/Studies/src/org/labkey/studies/StudiesController.java +++ b/Studies/src/org/labkey/studies/StudiesController.java @@ -1,37 +1,16 @@ package org.labkey.studies; import org.apache.logging.log4j.Logger; -import org.jetbrains.annotations.NotNull; -import org.labkey.api.action.ConfirmAction; +import org.labkey.api.action.ApiSimpleResponse; +import org.labkey.api.action.MutatingApiAction; +import org.labkey.api.action.SimpleApiJsonForm; import org.labkey.api.action.SpringActionController; -import org.labkey.api.data.TableInfo; -import org.labkey.api.module.ModuleLoader; -import org.labkey.api.pipeline.PipelineUrls; -import org.labkey.api.query.BatchValidationException; -import org.labkey.api.query.DuplicateKeyException; -import org.labkey.api.query.QueryService; -import org.labkey.api.query.QueryUpdateService; -import org.labkey.api.query.QueryUpdateServiceException; -import org.labkey.api.reader.DataLoader; -import org.labkey.api.reader.TabLoader; -import org.labkey.api.resource.Resource; import org.labkey.api.security.RequiresPermission; import org.labkey.api.security.permissions.AdminPermission; -import org.labkey.api.studies.StudiesService; -import org.labkey.api.util.FileUtil; -import org.labkey.api.util.HtmlString; -import org.labkey.api.util.PageFlowUtil; -import org.labkey.api.util.Path; -import org.labkey.api.util.URLHelper; +import org.labkey.api.studies.study.StudyDefinition; import org.labkey.api.util.logging.LogHelper; -import org.labkey.api.view.HtmlView; import org.springframework.validation.BindException; -import org.springframework.validation.Errors; -import org.springframework.web.servlet.ModelAndView; -import java.io.IOException; -import java.sql.SQLException; -import java.util.List; import java.util.Map; public class StudiesController extends SpringActionController @@ -45,4 +24,26 @@ public StudiesController() { setActionResolver(_actionResolver); } + + + @RequiresPermission(AdminPermission.class) + public static class UpdateStudyDefinitionAction extends MutatingApiAction + { + @Override + public Object execute(SimpleApiJsonForm json, BindException errors) throws Exception + { + try + { + StudyDefinition sd = StudyDefinition.fromJson(json.getJsonObject()); + sd = StudiesManager.get().insertOrUpdateStudyDefinition(sd, getContainer(), getUser()); + + return new ApiSimpleResponse(Map.of("success", true, "studyDefinition", sd)); + } + catch (Exception e) + { + _log.error("Unable to import study definition", e); + return new ApiSimpleResponse("success", false); + } + } + } } diff --git a/Studies/src/org/labkey/studies/StudiesManager.java b/Studies/src/org/labkey/studies/StudiesManager.java index 469f52781..81d3dea15 100644 --- a/Studies/src/org/labkey/studies/StudiesManager.java +++ b/Studies/src/org/labkey/studies/StudiesManager.java @@ -1,16 +1,101 @@ package org.labkey.studies; +import org.apache.commons.io.IOUtils; +import org.json.JSONObject; +import org.junit.AfterClass; +import org.junit.BeforeClass; +import org.junit.Test; +import org.labkey.api.data.Container; +import org.labkey.api.data.ContainerManager; +import org.labkey.api.discvrcore.test.AbstractIntegrationTest; +import org.labkey.api.module.Module; +import org.labkey.api.module.ModuleLoader; +import org.labkey.api.resource.FileResource; +import org.labkey.api.resource.Resource; +import org.labkey.api.security.User; +import org.labkey.api.studies.study.StudyDefinition; +import org.labkey.api.util.StringUtilsLabKey; +import org.labkey.api.util.TestContext; + +import java.io.FileInputStream; +import java.io.InputStream; +import java.util.HashSet; +import java.util.Set; + public class StudiesManager { private static final StudiesManager _instance = new StudiesManager(); private StudiesManager() { - // prevent external construction with a private default constructor + } public static StudiesManager get() { return _instance; } + + public StudyDefinition insertOrUpdateStudyDefinition(StudyDefinition sd, Container c, User u) + { + // TODO: implement this. The goal is to have one entrypoint that takes the JSON and ensures DB records are accurate. + // This should handle situations where a cohort or timepoint is removed. + // Note that the idea of the JSONObject is to allow redundant nested values (like the studyId key on child records) to inherit from the parent. + + // return the modified StudyDefinition, which should include setting RowIds, if this is an import + return sd; + } + + public static class TestCase extends AbstractIntegrationTest + { + public static final String PROJECT_NAME = "StudiesIntegrationTestFolder"; + + @BeforeClass + public static void setup() throws Exception + { + doInitialSetUp(PROJECT_NAME); + + Container project = ContainerManager.getForPath(PROJECT_NAME); + Set active = new HashSet<>(project.getActiveModules()); + active.add(ModuleLoader.getInstance().getModule(StudiesModule.NAME)); + project.setActiveModules(active); + } + + @AfterClass + public static void cleanup() + { + doCleanup(PROJECT_NAME); + } + + @Test + public void testStudyInsert() throws Exception + { + Container c = ContainerManager.getForPath(PROJECT_NAME); + + Resource r = ModuleLoader.getInstance().getModule(StudiesModule.NAME).getModuleResource("study/DemoStudy.json"); + if (r instanceof FileResource fr) + { + try (InputStream is = new FileInputStream(fr.getFile())) + { + String jsonTxt = IOUtils.toString(is, StringUtilsLabKey.DEFAULT_CHARSET); + JSONObject json = new JSONObject(jsonTxt); + + StudyDefinition sd = StudyDefinition.fromJson(json); + sd = StudiesManager.get().insertOrUpdateStudyDefinition(sd, c, TestContext.get().getUser()); + + // TODO: ensure the sd now contains rowIds + // Make updates to values, repeat insertOrUpdateStudyDefinition. + // Verify values persisted + + // Perform more updates, including deleting cohorts or timepoints, verify + + // Finally export to JSON, verify + } + } + else + { + throw new IllegalStateException("Expected a FileResource"); + } + } + } } \ No newline at end of file diff --git a/Studies/src/org/labkey/studies/StudiesModule.java b/Studies/src/org/labkey/studies/StudiesModule.java index eb2d32cd4..df6449a3f 100644 --- a/Studies/src/org/labkey/studies/StudiesModule.java +++ b/Studies/src/org/labkey/studies/StudiesModule.java @@ -10,6 +10,7 @@ import org.labkey.api.query.QuerySchema; import org.labkey.api.security.roles.RoleManager; import org.labkey.api.studies.StudiesService; +import org.labkey.api.util.PageFlowUtil; import org.labkey.studies.query.StudiesUserSchema; import org.labkey.api.studies.security.StudiesDataAdminRole; import org.labkey.studies.study.StudyEnrollmentEventProvider; @@ -76,4 +77,12 @@ public QuerySchema createSchema(final DefaultSchema schema, Module module) } }); } + + @Override + public @NotNull Set getIntegrationTests() + { + return PageFlowUtil.set( + StudiesManager.TestCase.class + ); + } } \ No newline at end of file From 15857ac504814aaed45709cf9269b3b534d7d4af Mon Sep 17 00:00:00 2001 From: bbimber Date: Tue, 24 Jun 2025 10:23:29 -0700 Subject: [PATCH 2/5] Clean up generics --- .../org/labkey/api/cluster/ClusterResourceAllocator.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/cluster/api-src/org/labkey/api/cluster/ClusterResourceAllocator.java b/cluster/api-src/org/labkey/api/cluster/ClusterResourceAllocator.java index 8c014d064..ee4d41b0f 100644 --- a/cluster/api-src/org/labkey/api/cluster/ClusterResourceAllocator.java +++ b/cluster/api-src/org/labkey/api/cluster/ClusterResourceAllocator.java @@ -47,19 +47,19 @@ interface Factory /** * Additional lines to include in the condor submit script. These will be appended to the default script. */ - @Nullable void addExtraSubmitScriptLines(PipelineJob job, RemoteExecutionEngine engine, List existingExtraLines); + @Nullable void addExtraSubmitScriptLines(PipelineJob job, RemoteExecutionEngine engine, List existingExtraLines); /** * The memory, in GB, to use as -xmx for the LabKey java remote process */ @Nullable - default void processJavaOpts(PipelineJob job, RemoteExecutionEngine engine, @NotNull List existingJavaOpts) + default void processJavaOpts(PipelineJob job, RemoteExecutionEngine engine, @NotNull List existingJavaOpts) { } @NotNull - default Map getEnvironmentVars(PipelineJob job, RemoteExecutionEngine engine) + default Map getEnvironmentVars(PipelineJob job, RemoteExecutionEngine engine) { return Collections.emptyMap(); } From 8883e9dbcc62c8b67dba5d0bf45249502406c7c7 Mon Sep 17 00:00:00 2001 From: bbimber Date: Tue, 24 Jun 2025 12:13:22 -0700 Subject: [PATCH 3/5] Refinements to customizer code, and many new SIV-related ETLs --- .../studies/query/StudiesTableCustomizer.java | 22 ++++++++++++++++++- .../studies/query/StudiesUserSchema.java | 1 - 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/Studies/src/org/labkey/studies/query/StudiesTableCustomizer.java b/Studies/src/org/labkey/studies/query/StudiesTableCustomizer.java index 5b9311806..c5e630346 100644 --- a/Studies/src/org/labkey/studies/query/StudiesTableCustomizer.java +++ b/Studies/src/org/labkey/studies/query/StudiesTableCustomizer.java @@ -1,10 +1,14 @@ package org.labkey.studies.query; +import org.apache.commons.collections4.MultiValuedMap; +import org.apache.commons.collections4.SetValuedMap; import org.apache.logging.log4j.Logger; +import org.labkey.api.collections.CaseInsensitiveKeyedHashSetValuedMap; import org.labkey.api.data.AbstractTableInfo; import org.labkey.api.data.TableCustomizer; import org.labkey.api.data.TableInfo; import org.labkey.api.ldk.LDKService; +import org.labkey.api.study.DatasetTable; import org.labkey.api.util.logging.LogHelper; public class StudiesTableCustomizer implements TableCustomizer @@ -14,7 +18,23 @@ public class StudiesTableCustomizer implements TableCustomizer @Override public void customize(TableInfo tableInfo) { - LDKService.get().getDefaultTableCustomizer().customize(tableInfo); + MultiValuedMap props = new CaseInsensitiveKeyedHashSetValuedMap<>(); + if (tableInfo.getPkColumnNames().size() > 1) + { + if (tableInfo.getPkColumnNames().contains("objectId")) + { + props.put("primaryKeyField", "objectId"); + } + else if (tableInfo instanceof DatasetTable ds) + { + if (ds.getDataset().isDemographicData()) + { + props.put("primaryKeyField", ds.getDataset().getStudy().getSubjectColumnName()); + } + } + } + + LDKService.get().getDefaultTableCustomizer(props).customize(tableInfo); if (tableInfo instanceof AbstractTableInfo ati) { doCustomize(ati); diff --git a/Studies/src/org/labkey/studies/query/StudiesUserSchema.java b/Studies/src/org/labkey/studies/query/StudiesUserSchema.java index 71e26880b..707c37f6f 100644 --- a/Studies/src/org/labkey/studies/query/StudiesUserSchema.java +++ b/Studies/src/org/labkey/studies/query/StudiesUserSchema.java @@ -161,7 +161,6 @@ private TableInfo createStudyDesignTable(String name, ContainerFilter cf) ret.addPermissionMapping(InsertPermission.class, StudiesDataAdminPermission.class); ret.addPermissionMapping(UpdatePermission.class, StudiesDataAdminPermission.class); ret.addPermissionMapping(DeletePermission.class, StudiesDataAdminPermission.class); - ret.addPermissionMapping(ReadPermission.class, StudiesDataAdminPermission.class); ret.addTriggerFactory(new StudiesTriggerFactory()); return ret.init(); From 106c789dc5b42f66e4aef4682fb51461ed2eb9d6 Mon Sep 17 00:00:00 2001 From: Sebastian Benjamin Date: Thu, 17 Jul 2025 15:01:32 -0700 Subject: [PATCH 4/5] Implement upsert + test --- .../api/studies/study/StudyDefinition.java | 68 +++- Studies/resources/study/DemoStudy.json | 10 +- .../org/labkey/studies/StudiesManager.java | 361 ++++++++++++++++-- 3 files changed, 398 insertions(+), 41 deletions(-) diff --git a/Studies/api-src/org/labkey/api/studies/study/StudyDefinition.java b/Studies/api-src/org/labkey/api/studies/study/StudyDefinition.java index 2def2c865..cff4174ba 100644 --- a/Studies/api-src/org/labkey/api/studies/study/StudyDefinition.java +++ b/Studies/api-src/org/labkey/api/studies/study/StudyDefinition.java @@ -1,6 +1,9 @@ package org.labkey.api.studies.study; +import com.fasterxml.jackson.annotation.JsonGetter; +import com.fasterxml.jackson.annotation.JsonIgnore; import com.fasterxml.jackson.annotation.JsonProperty; +import com.fasterxml.jackson.annotation.JsonSetter; import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.databind.ObjectWriter; @@ -8,6 +11,9 @@ import java.util.Date; import java.util.List; +import java.util.Map; +import java.util.stream.Collectors; +import java.util.stream.IntStream; public class StudyDefinition { @@ -246,12 +252,12 @@ public void setDescription(String description) _description = description; } - public Boolean getControlGroup() + public Boolean getIsControlGroup() { return _isControlGroup; } - public void setControlGroup(Boolean controlGroup) + public void setIsControlGroup(Boolean controlGroup) { _isControlGroup = controlGroup; } @@ -448,7 +454,13 @@ public static class Timepoint private String _labelShort; private String _description; + @JsonIgnore private Integer _anchorEvent; + + @JsonIgnore + private String _anchorEventLabel; + + private String _cohortName; private Integer _rangeMin; private Integer _rangeMax; @@ -464,6 +476,25 @@ public Timepoint() } + // When reading from a JSON object, store the anchorEvent label + @JsonSetter("anchorEvent") + void readAnchorEvent(String lbl) { _anchorEventLabel = lbl; } + + @JsonGetter("anchorEvent") + String writeAnchorEvent() { return _anchorEventLabel; } + + // Call this to translate from label to index in order to fit the DB schema + void resolveAnchorEvent(Map idxByLabel) + { + Integer idx = idxByLabel.get(_anchorEventLabel); + if (idx == null) + throw new IllegalArgumentException( + "Unknown anchorEvent label '" + _anchorEventLabel + "'"); + _anchorEvent = idx; + } + + public Integer getAnchorEvent() { return _anchorEvent; } + public Integer getRowId() { return _rowId; @@ -484,6 +515,16 @@ public void setStudyId(Integer studyId) _studyId = studyId; } + public String getCohortName() + { + return _cohortName; + } + + public void setCohortName(String cohortName) + { + _cohortName = cohortName; + } + public Integer getCohortId() { return _cohortId; @@ -524,16 +565,6 @@ public void setDescription(String description) _description = description; } - public Integer getAnchorEvent() - { - return _anchorEvent; - } - - public void setAnchorEvent(Integer anchorEvent) - { - _anchorEvent = anchorEvent; - } - public Integer getRangeMin() { return _rangeMin; @@ -608,8 +639,19 @@ public void setModified(Date modified) public static StudyDefinition fromJson(JSONObject json) { ObjectMapper mapper = new ObjectMapper(); + StudyDefinition sd = mapper.convertValue(json.toMap(), StudyDefinition.class); + + // In our JSON, Timepoints store the anchorEvent label, not an ID. Since the DB schema requires an int, we need + // to do that translation manually. Here, we store the anchorEvent by its index in the anchorEvent list. + Map idxByLabel = IntStream.range(0, sd.getAnchorEvents().size()) + .boxed() + .collect(Collectors.toMap( + i -> sd.getAnchorEvents().get(i).getLabel(), + i -> i)); + + sd.getTimepoints().forEach(tp -> tp.resolveAnchorEvent(idxByLabel)); - return mapper.convertValue(json, StudyDefinition.class); + return sd; } public String toJson() throws JsonProcessingException diff --git a/Studies/resources/study/DemoStudy.json b/Studies/resources/study/DemoStudy.json index e0c8f515d..07f419b06 100644 --- a/Studies/resources/study/DemoStudy.json +++ b/Studies/resources/study/DemoStudy.json @@ -6,6 +6,7 @@ "cohortName": "Group1", "label": "Group 1", "description": "This is the first group", + "isControlGroup": false, "sortOrder": 1 },{ "cohortName": "Control", @@ -14,7 +15,6 @@ "isControlGroup": true, "sortOrder": 2 }], - "anchorEvents": [{ "label": "Study Enrollment", "description": "The represents Day 0 of the study", @@ -30,14 +30,14 @@ "label": "Vaccination", "labelShort": "V", "anchorEvent": "Study Enrollment", - "minDate": 7, - "maxDate": 10 + "rangeMin": 7, + "rangeMax": 10 },{ "cohortName": "Control", "label": "Mock-Vaccination", "labelShort": "V", "anchorEvent": "Study Enrollment", - "minDate": 7, - "maxDate": 10 + "rangeMin": 7, + "rangeMax": 10 }] } \ No newline at end of file diff --git a/Studies/src/org/labkey/studies/StudiesManager.java b/Studies/src/org/labkey/studies/StudiesManager.java index 81d3dea15..d7e657201 100644 --- a/Studies/src/org/labkey/studies/StudiesManager.java +++ b/Studies/src/org/labkey/studies/StudiesManager.java @@ -1,27 +1,57 @@ package org.labkey.studies; +import org.apache.commons.collections.map.CaseInsensitiveMap; import org.apache.commons.io.IOUtils; import org.json.JSONObject; import org.junit.AfterClass; import org.junit.BeforeClass; import org.junit.Test; +import org.junit.experimental.runners.Enclosed; +import org.junit.runner.RunWith; import org.labkey.api.data.Container; import org.labkey.api.data.ContainerManager; +import org.labkey.api.data.DbSchema; +import org.labkey.api.data.DbScope; +import org.labkey.api.data.SimpleFilter; +import org.labkey.api.data.TableInfo; +import org.labkey.api.data.TableSelector; import org.labkey.api.discvrcore.test.AbstractIntegrationTest; import org.labkey.api.module.Module; import org.labkey.api.module.ModuleLoader; +import org.labkey.api.query.BatchValidationException; +import org.labkey.api.query.DuplicateKeyException; +import org.labkey.api.query.FieldKey; +import org.labkey.api.query.InvalidKeyException; +import org.labkey.api.query.QueryService; +import org.labkey.api.query.QueryUpdateService; +import org.labkey.api.query.QueryUpdateServiceException; +import org.labkey.api.query.SimpleUserSchema; +import org.labkey.api.query.UserSchema; import org.labkey.api.resource.FileResource; import org.labkey.api.resource.Resource; import org.labkey.api.security.User; import org.labkey.api.studies.study.StudyDefinition; +import org.labkey.api.util.PageFlowUtil; import org.labkey.api.util.StringUtilsLabKey; import org.labkey.api.util.TestContext; +import org.labkey.studies.query.StudiesUserSchema; + import java.io.FileInputStream; import java.io.InputStream; +import java.sql.SQLException; +import java.util.ArrayList; +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; + +import static com.fasterxml.jackson.databind.type.LogicalType.Collection; +@RunWith(Enclosed.class) public class StudiesManager { private static final StudiesManager _instance = new StudiesManager(); @@ -38,14 +68,219 @@ public static StudiesManager get() public StudyDefinition insertOrUpdateStudyDefinition(StudyDefinition sd, Container c, User u) { - // TODO: implement this. The goal is to have one entrypoint that takes the JSON and ensures DB records are accurate. - // This should handle situations where a cohort or timepoint is removed. - // Note that the idea of the JSONObject is to allow redundant nested values (like the studyId key on child records) to inherit from the parent. + StudiesSchema ss = StudiesSchema.getInstance(); + DbSchema schema = ss.getSchema(); + DbScope scope = schema.getScope(); + + UserSchema us = QueryService.get().getUserSchema(u, c, StudiesSchema.NAME); + + TableInfo tblStudies = us.getTable(StudiesSchema.TABLE_STUDIES); + TableInfo tblCohorts = us.getTable(StudiesSchema.TABLE_COHORTS); + TableInfo tblAnchorEvents = us.getTable(StudiesSchema.TABLE_ANCHOR_EVENTS); + TableInfo tblTimepoints = us.getTable(StudiesSchema.TABLE_EXPECTED_TIMEPOINTS); + + try (DbScope.Transaction tx = scope.ensureTransaction()) + { + sd.setContainer(c.getEntityId().toString()); + sd = upsertStudy(sd, tblStudies, c, u); + + upsertChildRecords( + sd.getRowId(), + sd.getCohorts(), + tblCohorts, + c, + u, + this::cohortToMap, + StudyDefinition.StudyCohort::getRowId, + StudyDefinition.StudyCohort::setRowId + ); + + upsertChildRecords( + sd.getRowId(), + sd.getAnchorEvents(), + tblAnchorEvents, + c, + u, + this::anchorToMap, + StudyDefinition.AnchorEvent::getRowId, + StudyDefinition.AnchorEvent::setRowId + ); + + upsertChildRecords( + sd.getRowId(), + sd.getTimepoints(), + tblTimepoints, + c, + u, + this::timepointToMap, + StudyDefinition.Timepoint::getRowId, + StudyDefinition.Timepoint::setRowId + ); + + tx.commit(); + } + catch (Exception x) + { + throw new RuntimeException("Failed to up‑sert StudyDefinition", x); + } - // return the modified StudyDefinition, which should include setting RowIds, if this is an import return sd; } + private StudyDefinition upsertStudy(StudyDefinition sd, + TableInfo tbl, + Container c, + User u) throws Exception + { + Map map = studyToMap(sd); + BatchValidationException bve = new BatchValidationException(); + QueryUpdateService qus = tbl.getUpdateService(); + + List> rows = List.of(map); + List> ret; + + if (sd.getRowId() == null) + ret = qus.insertRows(u, c, rows, bve, null, null); + else + { + ret = qus.updateRows(u, c, rows, null, bve, null, null); + } + + if (bve.hasErrors()) + throw bve; + + sd.setRowId((Integer) ret.get(0).get("rowId")); + return sd; + } + + + private void upsertChildRecords(int studyRowId, + List incoming, + TableInfo tbl, + Container c, + User u, + Mapper mapper, + RowIdGetter getRowId, + RowIdSetter setRowId) throws Exception + { + QueryUpdateService qus = tbl.getUpdateService(); + + Set existing = new HashSet<>( + new TableSelector(tbl, PageFlowUtil.set("rowId"), + new SimpleFilter(FieldKey.fromString("studyId"), studyRowId), + null).getCollection(Integer.class) + ); + + List> inserts = new ArrayList<>(); + List insertBeans = new ArrayList<>(); + List> updates = new ArrayList<>(); + + for (T bean : incoming) + { + Map row = mapper.apply(bean); + row.put("studyId", studyRowId); + + Integer rk = getRowId.get(bean); + if (rk == null) + { + inserts.add(row); + insertBeans.add(bean); + } + else + { + updates.add(row); + existing.remove(rk); + } + } + + if (!existing.isEmpty()) + { + List> keys = existing.stream() + .map(rid -> Map.of("rowid", (Object) rid)) + .toList(); + + qus.deleteRows(u, c, keys, null, null); + } + + BatchValidationException bve = new BatchValidationException(); + + if (!inserts.isEmpty()) + { + List> ret = qus.insertRows(u, c, inserts, bve, null, null); + for (int i = 0; i < ret.size(); i++) + setRowId.set(insertBeans.get(i), (Integer) ret.get(i).get("rowId")); + } + + if (!updates.isEmpty()) + { + qus.updateRows(u, c, updates, null, bve, null, null); + } + + if (bve.hasErrors()) + throw bve; + } + + private Map studyToMap(StudyDefinition s) + { + Map m = new HashMap<>(); + if (s.getRowId() != null) + m.put("rowId", s.getRowId()); + m.put("name", s.getStudyName()); + m.put("label", s.getLabel()); + m.put("category", s.getCategory()); + m.put("description", s.getDescription()); + m.put("container", s.getContainer()); + return m; + } + + private Map cohortToMap(StudyDefinition.StudyCohort c) + { + Map m = new HashMap<>(); + if (c.getRowId() != null) + m.put("rowId", c.getRowId()); + m.put("cohortName", c.getCohortName()); + m.put("label", c.getLabel()); + m.put("category", c.getCategory()); + m.put("description", c.getDescription()); + m.put("isControlGroup",c.getIsControlGroup()); + m.put("sortOrder", c.getSortOrder()); + m.put("container", c.getContainer()); + return m; + } + + private Map anchorToMap(StudyDefinition.AnchorEvent a) + { + Map m = new HashMap<>(); + if (a.getRowId() != null) + m.put("rowId", a.getRowId()); + m.put("label", a.getLabel()); + m.put("description", a.getDescription()); + m.put("eventProviderName",a.getEventProviderName()); + m.put("container", a.getContainer()); + return m; + } + + private Map timepointToMap(StudyDefinition.Timepoint t) + { + Map m = new HashMap<>(); + if (t.getRowId() != null) + m.put("rowId", t.getRowId()); + m.put("cohortId", t.getCohortId()); + m.put("cohortName", t.getCohortName()); + m.put("label", t.getLabel()); + m.put("labelShort", t.getLabelShort()); + m.put("description", t.getDescription()); + m.put("anchorEvent", t.getAnchorEvent()); + m.put("rangeMin", t.getRangeMin()); + m.put("rangeMax", t.getRangeMax()); + m.put("container", t.getContainer()); + return m; + } + + @FunctionalInterface private interface Mapper { Map apply(T t); } + @FunctionalInterface private interface RowIdGetter { Integer get(T t); } + @FunctionalInterface private interface RowIdSetter { void set(T t, Integer id); } + public static class TestCase extends AbstractIntegrationTest { public static final String PROJECT_NAME = "StudiesIntegrationTestFolder"; @@ -71,31 +306,111 @@ public static void cleanup() public void testStudyInsert() throws Exception { Container c = ContainerManager.getForPath(PROJECT_NAME); + Resource r = ModuleLoader.getInstance() + .getModule(StudiesModule.NAME) + .getModuleResource("study/DemoStudy.json"); + + if (!(r instanceof FileResource fr)) + throw new IllegalStateException("Expected a FileResource; got " + r); - Resource r = ModuleLoader.getInstance().getModule(StudiesModule.NAME).getModuleResource("study/DemoStudy.json"); - if (r instanceof FileResource fr) + StudyDefinition sd; + try (InputStream is = new FileInputStream(fr.getFile())) { - try (InputStream is = new FileInputStream(fr.getFile())) - { - String jsonTxt = IOUtils.toString(is, StringUtilsLabKey.DEFAULT_CHARSET); - JSONObject json = new JSONObject(jsonTxt); + String jsonTxt = IOUtils.toString(is, StringUtilsLabKey.DEFAULT_CHARSET); + sd = StudyDefinition.fromJson(new JSONObject(jsonTxt)); + } - StudyDefinition sd = StudyDefinition.fromJson(json); - sd = StudiesManager.get().insertOrUpdateStudyDefinition(sd, c, TestContext.get().getUser()); + sd = StudiesManager.get().insertOrUpdateStudyDefinition(sd, c, TestContext.get().getUser()); - // TODO: ensure the sd now contains rowIds - // Make updates to values, repeat insertOrUpdateStudyDefinition. - // Verify values persisted + // 1. Verify insert + assertNotNull( "study rowId null after insert", sd.getRowId()); + sd.getCohorts().forEach(co -> assertNotNull("cohort rowId null", co.getRowId())); + sd.getAnchorEvents().forEach(ev -> assertNotNull("anchor rowId null", ev.getRowId())); + sd.getTimepoints().forEach(tp -> assertNotNull("timepoint rowId null", tp.getRowId())); - // Perform more updates, including deleting cohorts or timepoints, verify + StudiesSchema ss = StudiesSchema.getInstance(); + DbSchema schema = ss.getSchema(); + TableInfo tblStudies = schema.getTable(StudiesSchema.TABLE_STUDIES); + TableInfo tblCohorts = schema.getTable(StudiesSchema.TABLE_COHORTS); + TableInfo tblTP = schema.getTable(StudiesSchema.TABLE_EXPECTED_TIMEPOINTS); - // Finally export to JSON, verify - } - } - else - { - throw new IllegalStateException("Expected a FileResource"); - } + assertEquals(1, + new TableSelector(tblStudies, + PageFlowUtil.set("rowId"), + new SimpleFilter(FieldKey.fromString("rowId"), sd.getRowId()), + null).getRowCount()); + + int cohortCount = sd.getCohorts().size(); + int timepointCount = sd.getTimepoints().size(); + + assertEquals(cohortCount, + new TableSelector(tblCohorts, + PageFlowUtil.set("rowId"), + new SimpleFilter(FieldKey.fromString("studyId"), sd.getRowId()), + null).getRowCount()); + + assertEquals(timepointCount, + new TableSelector(tblTP, + PageFlowUtil.set("rowId"), + new SimpleFilter(FieldKey.fromString("studyId"), sd.getRowId()), + null).getRowCount()); + + // 2. Update some values, add a cohort, delete a timepoint + sd.setLabel(sd.getLabel() + " (updated)"); + + StudyDefinition.StudyCohort firstCohort = sd.getCohorts().get(0); + firstCohort.setLabel(firstCohort.getLabel() + "‑updated"); + + StudyDefinition.StudyCohort newCohort = new StudyDefinition.StudyCohort(); + newCohort.setCohortName("NEW"); + newCohort.setLabel("Brand‑new cohort"); + sd.getCohorts().add(newCohort); + + StudyDefinition.Timepoint removedTp = sd.getTimepoints().remove(0); + + sd = StudiesManager.get().insertOrUpdateStudyDefinition(sd, c, TestContext.get().getUser()); + + assertNotNull("new cohort did not receive rowId", newCohort.getRowId()); + + assertEquals(cohortCount + 1, + new TableSelector(tblCohorts, + PageFlowUtil.set("rowId"), + new SimpleFilter(FieldKey.fromString("studyId"), sd.getRowId()), + null).getRowCount()); + + assertEquals(timepointCount - 1, + new TableSelector(tblTP, + PageFlowUtil.set("rowId"), + new SimpleFilter(FieldKey.fromString("studyId"), sd.getRowId()), + null).getRowCount()); + + String dbLabel = new TableSelector(tblStudies, + PageFlowUtil.set("label"), + new SimpleFilter(FieldKey.fromString("rowId"), sd.getRowId()), + null).getObject(String.class); + assertEquals(sd.getLabel(), dbLabel); + + String dbCohortLabel = new TableSelector(tblCohorts, + PageFlowUtil.set("label"), + new SimpleFilter(FieldKey.fromString("rowId"), firstCohort.getRowId()), + null).getObject(String.class); + assertEquals(firstCohort.getLabel(), dbCohortLabel); + + // 3. Delete the new cohort + sd.getCohorts().remove(newCohort); + sd = StudiesManager.get().insertOrUpdateStudyDefinition(sd, c, TestContext.get().getUser()); + + assertEquals(cohortCount, + new TableSelector(tblCohorts, + PageFlowUtil.set("rowId"), + new SimpleFilter(FieldKey.fromString("studyId"), sd.getRowId()), + null).getRowCount()); + + // 4. Round-trip JSON export + JSONObject roundTrip = new JSONObject(sd.toJson()); + assertEquals(sd.getLabel(), roundTrip.getString("label")); + assertEquals(sd.getCohorts().size(), roundTrip.getJSONArray("cohorts").length()); + assertEquals(sd.getTimepoints().size(), roundTrip.getJSONArray("timepoints").length()); } } } \ No newline at end of file From 8e4d985a8dd0c61a2adba1be32bd2849cf60307d Mon Sep 17 00:00:00 2001 From: Sebastian Benjamin Date: Tue, 22 Jul 2025 14:31:09 -0700 Subject: [PATCH 5/5] Fix unicode hyphen --- Studies/src/org/labkey/studies/StudiesManager.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Studies/src/org/labkey/studies/StudiesManager.java b/Studies/src/org/labkey/studies/StudiesManager.java index d7e657201..bf7926dcc 100644 --- a/Studies/src/org/labkey/studies/StudiesManager.java +++ b/Studies/src/org/labkey/studies/StudiesManager.java @@ -359,11 +359,11 @@ public void testStudyInsert() throws Exception sd.setLabel(sd.getLabel() + " (updated)"); StudyDefinition.StudyCohort firstCohort = sd.getCohorts().get(0); - firstCohort.setLabel(firstCohort.getLabel() + "‑updated"); + firstCohort.setLabel(firstCohort.getLabel() + "-updated"); StudyDefinition.StudyCohort newCohort = new StudyDefinition.StudyCohort(); newCohort.setCohortName("NEW"); - newCohort.setLabel("Brand‑new cohort"); + newCohort.setLabel("Brand-new cohort"); sd.getCohorts().add(newCohort); StudyDefinition.Timepoint removedTp = sd.getTimepoints().remove(0);