diff --git a/api/src/org/labkey/api/exp/api/DefaultExperimentSaveHandler.java b/api/src/org/labkey/api/exp/api/DefaultExperimentSaveHandler.java index 6f9b777b52a..c2704721e6c 100644 --- a/api/src/org/labkey/api/exp/api/DefaultExperimentSaveHandler.java +++ b/api/src/org/labkey/api/exp/api/DefaultExperimentSaveHandler.java @@ -584,7 +584,7 @@ else if (materialObject.has(ExperimentJSONConverter.LSID)) if (sampleTypeJson.has(ExperimentJSONConverter.ID)) { int sampleTypeRowId = sampleTypeJson.getInt(ExperimentJSONConverter.ID); - sampleType = SampleTypeService.get().getSampleType(sampleTypeRowId); + sampleType = SampleTypeService.get().getSampleType(context.getContainer(), sampleTypeRowId, true); if (sampleType == null) throw new NotFoundException("A sample type with row id '" + sampleTypeRowId + "' doesn't exist."); } @@ -598,7 +598,7 @@ else if (sampleTypeJson.has(ExperimentJSONConverter.NAME)) else if (sampleTypeJson.has(ExperimentJSONConverter.LSID)) { String lsid = sampleTypeJson.getString(ExperimentJSONConverter.LSID); - sampleType = SampleTypeService.get().getSampleType(lsid); + sampleType = SampleTypeService.get().getSampleTypeByLsid(context.getContainer(), lsid, true); if (sampleType == null) throw new NotFoundException("A sample type with LSID '" + lsid + "' doesn't exist."); } diff --git a/api/src/org/labkey/api/exp/api/SampleTypeService.java b/api/src/org/labkey/api/exp/api/SampleTypeService.java index 42acc471241..86ed77fd53c 100644 --- a/api/src/org/labkey/api/exp/api/SampleTypeService.java +++ b/api/src/org/labkey/api/exp/api/SampleTypeService.java @@ -204,6 +204,11 @@ ExpSampleType createSampleType(Container c, User u, String name, String descript */ ExpSampleType getSampleType(@NotNull Container scope, long rowId, boolean includeOtherContainers); + /** + * Get a SampleType by LSID within scope -- current plus, if includeOtherContainers is true, project and shared + */ + ExpSampleType getSampleTypeByLsid(@NotNull Container scope, @NotNull String lsid, boolean includeOtherContainers); + Lsid getSampleTypeLsid(String name, Container container); Pair getSampleTypeSamplePrefixLsids(Container container); diff --git a/experiment/src/org/labkey/experiment/api/SampleTypeServiceImpl.java b/experiment/src/org/labkey/experiment/api/SampleTypeServiceImpl.java index 49aa7cafdac..a2fad787551 100644 --- a/experiment/src/org/labkey/experiment/api/SampleTypeServiceImpl.java +++ b/experiment/src/org/labkey/experiment/api/SampleTypeServiceImpl.java @@ -125,6 +125,7 @@ import org.labkey.api.util.GUID; import org.labkey.api.util.Pair; import org.labkey.api.util.StringUtilsLabKey; +import org.labkey.api.util.TestContext; import org.labkey.api.util.logging.LogHelper; import org.labkey.api.view.ViewBackgroundInfo; import org.labkey.experiment.SampleTypeAuditProvider; @@ -513,6 +514,12 @@ public ExpSampleTypeImpl getSampleType(@NotNull Container c, long rowId, boolean return getSampleType(c, includeOtherContainers, (materialSource -> materialSource.getRowId() == rowId)); } + @Override + public ExpSampleTypeImpl getSampleTypeByLsid(@NotNull Container c, @NotNull String lsid, boolean includeOtherContainers) + { + return getSampleType(c, includeOtherContainers, (materialSource -> lsid.equals(materialSource.getLSID()))); + } + private ExpSampleTypeImpl getSampleType(@NotNull Container c, boolean includeOtherContainers, Predicate predicate) { for (Container scopeContainer : ExperimentServiceImpl.get().createContainerList(c, includeOtherContainers)) @@ -2518,5 +2525,62 @@ public void testGetValidatedUnit() } } + + @Test + public void testSampleTypeLookupIsContainerScoped() throws Exception + { + final User user = TestContext.get().getUser(); + final SampleTypeServiceImpl svc = get(); + final Container root = ContainerManager.getRoot(); + final Container shared = ContainerManager.getSharedContainer(); + final String suffix = GUID.makeHash(); + + Container projA = null; + Container projB = null; + ExpSampleType stShared = null; + try + { + projA = ContainerManager.createContainer(root, "SampleScopeA" + suffix, user); + projB = ContainerManager.createContainer(root, "SampleScopeB" + suffix, user); + Container subA = ContainerManager.createContainer(projA, "sub", user); + + ExpSampleType stLocal = svc.createSampleType(subA, user, "ScopeLocal", null, nameProp(), Collections.emptyList(), -1, -1, -1, -1, null, null); + ExpSampleType stProject = svc.createSampleType(projA, user, "ScopeProject", null, nameProp(), Collections.emptyList(), -1, -1, -1, -1, null, null); + stShared = svc.createSampleType(shared, user, "ScopeShared" + suffix, null, nameProp(), Collections.emptyList(), -1, -1, -1, -1, null, null); + ExpSampleType stForeign = svc.createSampleType(projB, user, "ScopeForeign", null, nameProp(), Collections.emptyList(), -1, -1, -1, -1, null, null); + + // In scope from subA (current, project, Shared) -- resolves by rowId and by LSID. + assertNotNull("Local sample type should resolve by rowId", svc.getSampleType(subA, stLocal.getRowId(), true)); + assertNotNull("Local sample type should resolve by LSID", svc.getSampleTypeByLsid(subA, stLocal.getLSID(), true)); + assertNotNull("Project sample type should resolve by rowId", svc.getSampleType(subA, stProject.getRowId(), true)); + assertNotNull("Project sample type should resolve by LSID", svc.getSampleTypeByLsid(subA, stProject.getLSID(), true)); + assertNotNull("Shared sample type should resolve by rowId", svc.getSampleType(subA, stShared.getRowId(), true)); + assertNotNull("Shared sample type should resolve by LSID", svc.getSampleTypeByLsid(subA, stShared.getLSID(), true)); + + // An unrelated project's sample type must NOT resolve from subA, by rowId or LSID. + assertNull("Foreign sample type must not resolve by rowId", svc.getSampleType(subA, stForeign.getRowId(), true)); + assertNull("Foreign sample type must not resolve by LSID", svc.getSampleTypeByLsid(subA, stForeign.getLSID(), true)); + + // The unscoped overloads still find it, confirming scoping is what blocks the cross-container reference. + assertNotNull("Unscoped rowId lookup still finds the foreign type", svc.getSampleType(stForeign.getRowId())); + assertNotNull("Unscoped LSID lookup still finds the foreign type", svc.getSampleType(stForeign.getLSID())); + } + finally + { + if (stShared != null) + stShared.delete(user); + if (projA != null) + ContainerManager.deleteAll(projA, user); + if (projB != null) + ContainerManager.deleteAll(projB, user); + } + } + + private List nameProp() + { + List props = new ArrayList<>(); + props.add(new GWTPropertyDescriptor("name", "string")); + return props; + } } }