Skip to content

TreatmentManager container scoping #7758

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
cnathe merged 3 commits into from
Jun 16, 2026
Merged

TreatmentManager container scoping #7758

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f855508
TreatmentManager container scoping
cnathe Jun 16, 2026
4450177
TreatmentManager container scoping, part 2
cnathe Jun 16, 2026
55dffc3
fix test case
cnathe Jun 16, 2026
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
141 changes: 138 additions & 3 deletions studydesign/src/org/labkey/studydesign/model/TreatmentManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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<DoseAndRoute> 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
Expand Down Expand Up @@ -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<Module> 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)
Expand Down