diff --git a/studydesign/src/org/labkey/studydesign/model/TreatmentManager.java b/studydesign/src/org/labkey/studydesign/model/TreatmentManager.java index 8bfae0273ed..1c441aef779 100644 --- a/studydesign/src/org/labkey/studydesign/model/TreatmentManager.java +++ b/studydesign/src/org/labkey/studydesign/model/TreatmentManager.java @@ -54,6 +54,7 @@ import org.labkey.api.util.GUID; import org.labkey.api.util.JunitUtil; import org.labkey.api.util.TestContext; +import org.labkey.api.view.NotFoundException; import java.math.BigDecimal; import java.util.ArrayList; @@ -318,7 +319,10 @@ public void deleteStudyProduct(Container container, User user, int rowId) deleteProductAntigens(container, user, rowId); // delete the associated doses and routes for this product - Table.delete(StudyDesignSchema.getInstance().getTableInfoDoseAndRoute(), new SimpleFilter(FieldKey.fromParts("ProductId"), rowId)); + // GitHub Kanban #1929: scope to the container in addition to ProductId + SimpleFilter doseAndRouteFilter = SimpleFilter.createContainerFilter(container); + doseAndRouteFilter.addCondition(FieldKey.fromParts("ProductId"), rowId); + Table.delete(StudyDesignSchema.getInstance().getTableInfoDoseAndRoute(), doseAndRouteFilter); // delete the associated treatment study product mappings (provision table) SimpleFilter filter = SimpleFilter.createContainerFilter(container); @@ -363,19 +367,31 @@ public DoseAndRoute saveStudyProductDoseAndRoute(Container container, User user, if (doseAndRoute.isNew()) return Table.insert(user, StudyDesignSchema.getInstance().getTableInfoDoseAndRoute(), doseAndRoute); else + { + // GitHub Kanban #1929: verify the existing row is in this container before updating + SimpleFilter filter = SimpleFilter.createContainerFilter(container); + filter.addCondition(FieldKey.fromParts("RowId"), doseAndRoute.getRowId()); + if (!new TableSelector(StudyDesignSchema.getInstance().getTableInfoDoseAndRoute(), filter, null).exists()) + throw new NotFoundException("No dose and route found for rowId: " + doseAndRoute.getRowId()); + return Table.update(user, StudyDesignSchema.getInstance().getTableInfoDoseAndRoute(), doseAndRoute, doseAndRoute.getRowId()); + } } public Collection getStudyProductsDoseAndRoute(Container container, User user, int productId) { - SimpleFilter filter = new SimpleFilter(FieldKey.fromParts("ProductId"), productId); + // GitHub Kanban #1929: scope to the container in addition to ProductId + SimpleFilter filter = SimpleFilter.createContainerFilter(container); + filter.addCondition(FieldKey.fromParts("ProductId"), productId); return new TableSelector(StudyDesignSchema.getInstance().getTableInfoDoseAndRoute(), filter, null).getCollection(DoseAndRoute.class); } @Nullable public DoseAndRoute getDoseAndRoute(Container container, String dose, String route, int productId) { - SimpleFilter filter = new SimpleFilter(FieldKey.fromParts("ProductId"), productId); + // GitHub Kanban #1929: scope to the container in addition to ProductId + SimpleFilter filter = SimpleFilter.createContainerFilter(container); + filter.addCondition(FieldKey.fromParts("ProductId"), productId); if (dose != null) filter.addCondition(FieldKey.fromParts("Dose"), dose); else @@ -989,6 +1005,125 @@ private void tearDown() assertTrue(ContainerManager.delete(_junitStudy.getContainer(), _context.getUser())); } } + + // GitHub Kanban #1929: getStudyProductsDoseAndRoute / getDoseAndRoute / deleteStudyProduct container scoping checks + @Test + public void testDoseAndRouteContainerScoping() throws Exception + { + TestContext context = TestContext.get(); + User user = context.getUser(); + Container containerA = null; + Container containerB = null; + try + { + containerA = createStudyContainer(context, GUID.makeHash()); + containerB = createStudyContainer(context, GUID.makeHash()); + + int productIdA = insertStudyProduct(containerA, user, "Immunogen A", "Immunogen"); + int productIdB = insertStudyProduct(containerB, user, "Immunogen B", "Immunogen"); + + _manager.saveStudyProductDoseAndRoute(containerA, user, new DoseAndRoute("Dose A", "Route A", productIdA, containerA)); + _manager.saveStudyProductDoseAndRoute(containerB, user, new DoseAndRoute("Dose B", "Route B", productIdB, containerB)); + + // getStudyProductsDoseAndRoute: scoped by container, not ProductId alone + assertEquals("Dose/route should be returned within its own container", 1, + _manager.getStudyProductsDoseAndRoute(containerB, user, productIdB).size()); + assertEquals("Dose/route must not be returned from another container", 0, + _manager.getStudyProductsDoseAndRoute(containerA, user, productIdB).size()); + + // getDoseAndRoute: same scoping + assertNotNull("getDoseAndRoute should find the row within its own container", + _manager.getDoseAndRoute(containerB, "Dose B", "Route B", productIdB)); + assertNull("getDoseAndRoute must not find the row from another container", + _manager.getDoseAndRoute(containerA, "Dose B", "Route B", productIdB)); + + // deleteStudyProduct's dose/route delete is now container scoped; deleting the product in its own + // container removes its dose/route (the cross-container case is unreachable since ProductId is unique). + _manager.deleteStudyProduct(containerB, user, productIdB); + assertEquals("deleteStudyProduct should remove the dose/route within its own container", 0, + _manager.getStudyProductsDoseAndRoute(containerB, user, productIdB).size()); + } + finally + { + if (null != containerB) + ContainerManager.delete(containerB, user); + if (null != containerA) + ContainerManager.delete(containerA, user); + } + } + + // GitHub Kanban #1929: saveStudyProductDoseAndRoute rejects updating a dose/route row from another container + @Test + public void testCrossContainerDoseAndRouteUpdateDenied() + { + TestContext context = TestContext.get(); + User user = context.getUser(); + Container containerA = null; + Container containerB = null; + try + { + containerA = createStudyContainer(context, GUID.makeHash()); + containerB = createStudyContainer(context, GUID.makeHash()); + + int productIdA = insertStudyProduct(containerA, user, "Immunogen A", "Immunogen"); + int productIdB = insertStudyProduct(containerB, user, "Immunogen B", "Immunogen"); + + // a dose/route owned by container B + DoseAndRoute savedB = _manager.saveStudyProductDoseAndRoute(containerB, user, new DoseAndRoute("Dose B", "Route B", productIdB, containerB)); + int rowIdB = savedB.getRowId(); + + // an editor in container A submits an update carrying container B's RowId + DoseAndRoute foreign = new DoseAndRoute("Rejected", "Rejected", productIdA, containerA); + foreign.setRowId(rowIdB); + try + { + _manager.saveStudyProductDoseAndRoute(containerA, user, foreign); + fail("Expected NotFoundException updating a dose/route row from another container"); + } + catch (NotFoundException expected) + { + // expected + } + + // container B's row must be untouched (neither overwritten nor repointed into container A) + assertNotNull("Cross-container update must not modify container B's dose/route", + _manager.getDoseAndRoute(containerB, "Dose B", "Route B", productIdB)); + assertNull("Cross-container update must not have repointed the row into container A", + _manager.getDoseAndRoute(containerA, "Rejected", "Rejected", productIdA)); + + // positive control: updating the row from within its own container succeeds + DoseAndRoute updateB = new DoseAndRoute("Dose B2", "Route B2", productIdB, containerB); + updateB.setRowId(rowIdB); + _manager.saveStudyProductDoseAndRoute(containerB, user, updateB); + assertNotNull("In-container update should succeed", + _manager.getDoseAndRoute(containerB, "Dose B2", "Route B2", productIdB)); + } + finally + { + if (null != containerB) + ContainerManager.delete(containerB, user); + if (null != containerA) + ContainerManager.delete(containerA, user); + } + } + + private Container createStudyContainer(TestContext context, String name) + { + Container junit = JunitUtil.getTestContainer(); + Container c = ContainerManager.createContainer(junit, name, context.getUser()); + Set modules = new HashSet<>(c.getActiveModules()); + modules.add(ModuleLoader.getInstance().getModule("studydesign")); + c.setActiveModules(modules); + StudyService.get().createStudy(c, context.getUser(), "Junit Study " + name, TimepointType.VISIT, true); + return c; + } + + private int insertStudyProduct(Container c, User user, String label, String role) + { + UserSchema schema = QueryService.get().getUserSchema(user, c, StudyDesignQuerySchema.STUDY_SCHEMA_NAME); + TableInfo ti = ((FilteredTable) schema.getTable(StudyDesignQuerySchema.PRODUCT_TABLE_NAME)).getRealTable(); + return Table.insert(user, ti, new ProductImpl(c, label, role)).getRowId(); + } } @TestWhen(TestWhen.When.BVT)