From 8bb396b578b81e10c71e3b5bb9a4ed0170d5373f Mon Sep 17 00:00:00 2001 From: labkey-jeckels Date: Mon, 25 May 2026 15:17:02 -0700 Subject: [PATCH 1/3] Remove unused actions: CycleCheckAction and CheckEdgesAction --- .../controllers/exp/ExperimentController.java | 139 +----------------- 1 file changed, 5 insertions(+), 134 deletions(-) diff --git a/experiment/src/org/labkey/experiment/controllers/exp/ExperimentController.java b/experiment/src/org/labkey/experiment/controllers/exp/ExperimentController.java index 27f2e4eaf57..813c25e8c9e 100644 --- a/experiment/src/org/labkey/experiment/controllers/exp/ExperimentController.java +++ b/experiment/src/org/labkey/experiment/controllers/exp/ExperimentController.java @@ -79,7 +79,6 @@ import org.labkey.api.data.ConvertHelper; import org.labkey.api.data.DataRegion; import org.labkey.api.data.DataRegionSelection; -import org.labkey.api.data.DbSchema; import org.labkey.api.data.DbScope; import org.labkey.api.data.DisplayColumn; import org.labkey.api.data.ExcelWriter; @@ -90,7 +89,6 @@ import org.labkey.api.data.SimpleDisplayColumn; import org.labkey.api.data.SimpleFilter; import org.labkey.api.data.Sort; -import org.labkey.api.data.SqlSelector; import org.labkey.api.data.TSVJSONWriter; import org.labkey.api.data.TSVWriter; import org.labkey.api.data.TableInfo; @@ -201,7 +199,6 @@ import org.labkey.api.security.permissions.ReadPermission; import org.labkey.api.security.permissions.SampleWorkflowDeletePermission; import org.labkey.api.security.permissions.SiteAdminPermission; -import org.labkey.api.security.permissions.TroubleshooterPermission; import org.labkey.api.security.permissions.UpdatePermission; import org.labkey.api.settings.AppProps; import org.labkey.api.settings.ConceptURIProperties; @@ -218,7 +215,6 @@ import org.labkey.api.util.ExceptionUtil; import org.labkey.api.util.FileStream; import org.labkey.api.util.FileUtil; -import org.labkey.api.util.HtmlString; import org.labkey.api.util.HtmlStringBuilder; import org.labkey.api.util.ImageUtil; import org.labkey.api.util.JSoupUtil; @@ -292,7 +288,6 @@ import org.labkey.experiment.api.ExpSampleTypeImpl; import org.labkey.experiment.api.Experiment; import org.labkey.experiment.api.ExperimentServiceImpl; -import org.labkey.experiment.api.GraphAlgorithms; import org.labkey.experiment.api.ProtocolActionStepDetail; import org.labkey.experiment.api.SampleTypeServiceImpl; import org.labkey.experiment.api.SampleTypeUpdateServiceDI; @@ -356,18 +351,15 @@ import static org.labkey.api.util.DOM.Attribute.name; import static org.labkey.api.util.DOM.Attribute.size; import static org.labkey.api.util.DOM.Attribute.src; -import static org.labkey.api.util.DOM.Attribute.target; import static org.labkey.api.util.DOM.Attribute.type; import static org.labkey.api.util.DOM.Attribute.value; import static org.labkey.api.util.DOM.Attribute.width; import static org.labkey.api.util.DOM.DIV; import static org.labkey.api.util.DOM.IMG; import static org.labkey.api.util.DOM.INPUT; -import static org.labkey.api.util.DOM.LI; import static org.labkey.api.util.DOM.TABLE; import static org.labkey.api.util.DOM.TD; import static org.labkey.api.util.DOM.TR; -import static org.labkey.api.util.DOM.UL; import static org.labkey.api.util.DOM.at; import static org.labkey.api.util.DOM.cl; import static org.labkey.experiment.ExpDataIterators.setContainerFilterForImport; @@ -797,7 +789,7 @@ private Pair> createViews(ExpObjectForm form, ExperimentRunType selectedType = ExperimentRunType.getSelectedFilter(types, getViewContext().getRequest().getParameter("experimentRunFilter")); ChooseExperimentTypeBean bean = new ChooseExperimentTypeBean(types, selectedType, getViewContext().getActionURL().clone(), protocols); - JspView chooserView = new JspView<>("/org/labkey/experiment/experimentRunQueryHeader.jsp", bean, errors); + JspView chooserView = new JspView<>("/org/labkey/experiment/experimentRunQueryHeader.jsp", bean, errors); ExperimentRunListView runListView = ExperimentRunListView.createView(getViewContext(), bean.getSelectedFilter(), true); runListView.getRunTable().setExperiment(_experiment); @@ -2517,8 +2509,10 @@ protected ModelAndView getDataView(DataForm form, BindException errors) throws I try { // Try to write the exception back to the caller if we haven't already flushed the buffer - ApiJsonWriter writer = new ApiJsonWriter(getViewContext().getResponse()); - writer.writeResponse(e); + try(ApiJsonWriter writer = new ApiJsonWriter(getViewContext().getResponse())) + { + writer.writeResponse(e); + } } catch (IllegalStateException ise) { @@ -7373,43 +7367,6 @@ public Object execute(Object o, BindException errors) throws Exception } } - @Marshal(Marshaller.Jackson) - @RequiresPermission(AdminPermission.class) - public static class CheckEdgesAction extends ReadOnlyApiAction - { - @Override - public Object execute(Object o, BindException errors) throws Exception - { - List result; - DbSchema schema = ExperimentService.get().getSchema(); - TableInfo edgeTable = schema.getTable("Edge"); - - if (null != edgeTable.getColumn("fromObjectId")) - { - var edges = new SqlSelector(ExperimentService.get().getSchema(), "SELECT fromObjectId, toObjectId FROM exp.Edge") - .resultSetStream() - .map(r -> { try { return new Pair<>(r.getInt(1), r.getInt(2)); } catch (SQLException x) { throw new RuntimeException(x); } }) - .collect(toList()); - var cycles = (new GraphAlgorithms()).detectCycleInDirectedGraph(edges); - result = cycles.stream().map(e -> new Integer[]{e.first, e.second}).collect(toList()); - } - else - { - var edges = new SqlSelector(ExperimentService.get().getSchema(), "SELECT fromLsid, toLsid FROM exp.Edge") - .resultSetStream() - .map(r -> { try { return new Pair<>(r.getString(1), r.getString(2)); } catch (SQLException x) { throw new RuntimeException(x); } }) - .collect(toList()); - var cycles = (new GraphAlgorithms()).detectCycleInDirectedGraph(edges); - result = cycles.stream().map(e -> new String[]{e.first, e.second}).collect(toList()); - } - - JSONObject ret = new JSONObject(); - ret.put("result", result); - ret.put("success", true); - return ret; - } - } - @RequiresPermission(UpdatePermission.class) public static class UpdateMaterialQueryRowAction extends UserSchemaAction { @@ -8190,92 +8147,6 @@ public ModelAndView getView(Object o, BindException errors) throws SQLException } } - /* Also see API CheckEdgesAction */ - @RequiresPermission(TroubleshooterPermission.class) - public static class CycleCheckAction extends FormViewAction - { - List cycleObjectIds = null; - - @Override - public void validateCommand(Object target, Errors errors) - { - - } - - @Override - public ModelAndView getView(Object o, boolean reshow, BindException errors) - { - if (!reshow) - { - return new HtmlView( - DIV("This operation can use a lot of memory.", - LK.FORM(at(method,"POST"), - PageFlowUtil.button("Continue").submit(true))) - ); - } - - if (null == cycleObjectIds) - return new HtmlView(HtmlString.of("No cycles found")); - - Map map = new LongHashMap<>(); - var cf = new ContainerFilter.AllFolders(getUser()); - var materials = ExperimentServiceImpl.get().getExpMaterialsByObjectId(cf, cycleObjectIds); - materials.forEach( (m) -> map.put(m.getObjectId(), m)); - var datas = ExperimentServiceImpl.get().getExpDatasByObjectId(cf, cycleObjectIds); - datas.forEach( (d) -> map.put(d.getObjectId(), d)); - var runs = ExperimentServiceImpl.get().getRunsByObjectId(cf, cycleObjectIds); - runs.forEach( (r) -> map.put(r.getObjectId(), r)); - - ExperimentUrls urls = ExperimentUrls.get(); - return new HtmlView( - DIV("Cycle found involving these objects.", - UL(cycleObjectIds.stream().map((objectid) -> - { - ExpObject exp = map.get(objectid); - if (exp instanceof ExpMaterial mat) - return LI(A(at(target, "_blank", href, urls.getMaterialDetailsURL(mat)), objectid + " : material - " + mat.getName())); - else if (exp instanceof ExpRun run) - return LI(A(at(target, "_blank", href, urls.getRunTextURL(run)), objectid + " : run - " + run.getName())); - else if (exp instanceof ExpData data) - return LI(A(at(target, "_blank", href, urls.getDataDetailsURL(data)), objectid + " : run - " + data.getName())); - else - return LI(String.valueOf(objectid)); - })) - ) - ); - } - - @Override - public boolean handlePost(Object o, BindException errors) - { - var edges = new SqlSelector(ExperimentService.get().getSchema(), "SELECT fromObjectId, toObjectId FROM exp.Edge") - .resultSetStream() - .map(r -> { try { return new Pair<>(r.getInt(1), r.getInt(2)); } catch (SQLException x) { throw new RuntimeException(x); } }) - .collect(toList()); - var cyclesEdges = (new GraphAlgorithms()).detectCycleInDirectedGraph(edges); - - var set = new LinkedHashSet(); - cyclesEdges.forEach( (edge) -> { - set.add(edge.first); - set.add(edge.second); - }); - cycleObjectIds = set.stream().toList(); - return false; - } - - @Override - public URLHelper getSuccessURL(Object o) - { - return null; - } - - @Override - public void addNavTrail(NavTree root) - { - - } - } - @RequiresPermission(AdminPermission.class) public static class MissingFilesCheckAction extends ReadOnlyApiAction { From 662c9f149b841107e154ba015267c447bb6f53c5 Mon Sep 17 00:00:00 2001 From: labkey-jeckels Date: Tue, 26 May 2026 20:28:59 -0700 Subject: [PATCH 2/3] Fix int/long type mismatch in CycleCheckAction and CheckEdgesAction; add EdgeDiagnosticsTestCase MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CycleCheckAction and CheckEdgesAction read fromObjectId/toObjectId from exp.Edge using JDBC getInt() (Integer), but ExpObject.getObjectId() returns Long. This caused a silent failure in CycleCheckAction.getView() where LongHashMap lookups with Integer keys always returned null, so cycle-involved objects were never resolved to their names or links. Fix: use getLong() throughout both actions and carry Long consistently through all type parameters. Also fixed a null-check bug where cycleObjectIds was always set to an empty list (not null) when no cycles were found, making the "No cycles found" branch unreachable. Refactored the core logic of both actions into public static helpers (detectCycleEdges, detectCycleObjectIds, resolveCycleObjects) to enable direct testing. Changed getExpMaterialsByObjectId, getExpDatasByObjectId, and getRunsByObjectId in ExperimentServiceImpl from Collection to Collection to match. Added EdgeDiagnosticsTestCase (org.labkey.experiment.api) with integration tests that insert real synthetic A↔B cycle edges and verify both actions correctly detect and resolve cycle-involved objects by Long key — the assertion that would fail with the pre-fix Integer-keyed code. Co-Authored-By: Claude Sonnet 4.6 --- .../labkey/experiment/ExperimentModule.java | 2 + .../api/EdgeDiagnosticsTestCase.java | 134 +++++++++++++++ .../experiment/api/ExperimentServiceImpl.java | 10 +- .../controllers/exp/ExperimentController.java | 152 +++++++++++++++++- 4 files changed, 287 insertions(+), 11 deletions(-) create mode 100644 experiment/src/org/labkey/experiment/api/EdgeDiagnosticsTestCase.java diff --git a/experiment/src/org/labkey/experiment/ExperimentModule.java b/experiment/src/org/labkey/experiment/ExperimentModule.java index 5c9c65763ae..8bc6e44d4da 100644 --- a/experiment/src/org/labkey/experiment/ExperimentModule.java +++ b/experiment/src/org/labkey/experiment/ExperimentModule.java @@ -117,6 +117,7 @@ import org.labkey.api.webdav.WebdavService; import org.labkey.api.writer.ContainerUser; import org.labkey.experiment.api.DataClassDomainKind; +import org.labkey.experiment.api.EdgeDiagnosticsTestCase; import org.labkey.experiment.api.ExpDataClassImpl; import org.labkey.experiment.api.ExpDataClassTableImpl; import org.labkey.experiment.api.ExpDataClassType; @@ -1114,6 +1115,7 @@ public Collection getSummary(Container c) public @NotNull Set> getIntegrationTests() { return Set.of( + EdgeDiagnosticsTestCase.class, DomainImpl.TestCase.class, DomainPropertyImpl.TestCase.class, ExpDataTableImpl.TestCase.class, diff --git a/experiment/src/org/labkey/experiment/api/EdgeDiagnosticsTestCase.java b/experiment/src/org/labkey/experiment/api/EdgeDiagnosticsTestCase.java new file mode 100644 index 00000000000..e7ebaa2d14f --- /dev/null +++ b/experiment/src/org/labkey/experiment/api/EdgeDiagnosticsTestCase.java @@ -0,0 +1,134 @@ +package org.labkey.experiment.api; + +import org.junit.After; +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; +import org.labkey.api.collections.CaseInsensitiveHashMap; +import org.labkey.api.data.ContainerFilter; +import org.labkey.api.data.DbSchema; +import org.labkey.api.data.SQLFragment; +import org.labkey.api.data.SqlExecutor; +import org.labkey.api.data.TableInfo; +import org.labkey.api.exp.api.ExpMaterial; +import org.labkey.api.exp.api.ExpObject; +import org.labkey.api.exp.api.ExperimentService; +import org.labkey.api.exp.api.SampleTypeService; +import org.labkey.api.gwt.client.model.GWTPropertyDescriptor; +import org.labkey.api.query.BatchValidationException; +import org.labkey.api.query.QueryService; +import org.labkey.api.query.QueryUpdateService; +import org.labkey.api.query.SchemaKey; +import org.labkey.api.query.UserSchema; +import org.labkey.api.security.User; +import org.labkey.api.util.JunitUtil; +import org.labkey.api.util.Pair; +import org.labkey.api.util.TestContext; +import org.labkey.experiment.controllers.exp.ExperimentController; + +import java.util.ArrayList; +import java.util.Collection; +import java.util.Collections; +import java.util.List; +import java.util.Map; + +public class EdgeDiagnosticsTestCase extends Assert +{ + private User _user; + private ExpMaterial _sampleA; + private ExpMaterial _sampleB; + + @Before + public void setUp() throws Exception + { + JunitUtil.deleteTestContainer(); + _user = TestContext.get().getUser(); + var container = JunitUtil.getTestContainer(); + + List props = new ArrayList<>(); + props.add(new GWTPropertyDescriptor("name", "string")); + var sampleType = SampleTypeService.get().createSampleType(container, _user, "EdgeDiagSamples", null, props, Collections.emptyList(), -1, -1, -1, -1, null); + + UserSchema schema = QueryService.get().getUserSchema(_user, container, SchemaKey.fromParts("Samples")); + TableInfo table = schema.getTable("EdgeDiagSamples"); + QueryUpdateService svc = table.getUpdateService(); + BatchValidationException errors = new BatchValidationException(); + svc.insertRows(_user, container, List.of( + CaseInsensitiveHashMap.of("name", "edgeA"), + CaseInsensitiveHashMap.of("name", "edgeB") + ), errors, null, null); + if (errors.hasErrors()) + throw errors; + + _sampleA = sampleType.getSample(container, "edgeA"); + _sampleB = sampleType.getSample(container, "edgeB"); + } + + @After + public void tearDown() + { + JunitUtil.deleteTestContainer(); + } + + @Test + public void testCycleCheckActionDetectsAndResolves() + { + Long idA = _sampleA.getObjectId(); + Long idB = _sampleB.getObjectId(); + DbSchema expSchema = ExperimentService.get().getSchema(); + try + { + insertCycleEdges(expSchema, idA, idB); + + List cycleIds = ExperimentController.CycleCheckAction.detectCycleObjectIds(expSchema); + assertNotNull("Cycle should be detected", cycleIds); + assertTrue("Cycle should include idA", cycleIds.contains(idA)); + assertTrue("Cycle should include idB", cycleIds.contains(idB)); + + Map resolved = ExperimentController.CycleCheckAction.resolveCycleObjects(new ContainerFilter.AllFolders(_user), cycleIds); + assertEquals(_sampleA.getName(), resolved.get(idA).getName()); + assertEquals(_sampleB.getName(), resolved.get(idB).getName()); + } + finally + { + deleteCycleEdges(expSchema, idA, idB); + } + } + + @Test + public void testCheckEdgesActionDetectsCycles() + { + Long idA = _sampleA.getObjectId(); + Long idB = _sampleB.getObjectId(); + DbSchema expSchema = ExperimentService.get().getSchema(); + try + { + insertCycleEdges(expSchema, idA, idB); + + Collection> cycleEdges = ExperimentController.CheckEdgesAction.detectCycleEdges(expSchema); + assertFalse("Cycle edges should be detected", cycleEdges.isEmpty()); + assertTrue("idA should appear in a cycle edge", + cycleEdges.stream().anyMatch(e -> e.first.equals(idA) || e.second.equals(idA))); + assertTrue("idB should appear in a cycle edge", + cycleEdges.stream().anyMatch(e -> e.first.equals(idB) || e.second.equals(idB))); + } + finally + { + deleteCycleEdges(expSchema, idA, idB); + } + } + + private static void insertCycleEdges(DbSchema schema, Long idA, Long idB) + { + var executor = new SqlExecutor(schema); + executor.execute(new SQLFragment("INSERT INTO exp.Edge (FromObjectId, ToObjectId) VALUES (?,?)", idA, idB)); + executor.execute(new SQLFragment("INSERT INTO exp.Edge (FromObjectId, ToObjectId) VALUES (?,?)", idB, idA)); + } + + private static void deleteCycleEdges(DbSchema schema, Long idA, Long idB) + { + new SqlExecutor(schema).execute(new SQLFragment( + "DELETE FROM exp.Edge WHERE (FromObjectId=? AND ToObjectId=?) OR (FromObjectId=? AND ToObjectId=?)", + idA, idB, idB, idA)); + } +} diff --git a/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java b/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java index bf51266dc3f..ac0a3b9dbcd 100644 --- a/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java +++ b/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java @@ -991,7 +991,7 @@ public List getExpMaterials(Container container, User user, Col return getExpMaterials(filter); } - public List getExpMaterialsByObjectId(ContainerFilter containerFilter, Collection objectIds) + public List getExpMaterialsByObjectId(ContainerFilter containerFilter, Collection objectIds) { if (objectIds.isEmpty()) return emptyList(); @@ -1871,7 +1871,7 @@ public List getExpDatas(ExpDataClass dataClass) return datas.stream().map(ExpDataImpl::new).collect(toList()); } - public List getExpDatasByObjectId(ContainerFilter containerFilter, Collection objectIds) + public List getExpDatasByObjectId(ContainerFilter containerFilter, Collection objectIds) { SimpleFilter filter = new SimpleFilter(); filter.addInClause(FieldKey.fromParts("ObjectId"), objectIds); @@ -5711,7 +5711,7 @@ public List getRunsUsingDataIds(List ids) return ExpRunImpl.fromRuns(new SqlSelector(getExpSchema(), sql).getArrayList(ExperimentRun.class)); } - public List getRunsByObjectId(ContainerFilter containerFilter, Collection objectIds) + public List getRunsByObjectId(ContainerFilter containerFilter, Collection objectIds) { SimpleFilter filter = new SimpleFilter(); filter.addInClause(FieldKey.fromParts("ObjectId"), objectIds); @@ -7550,13 +7550,13 @@ else if (_aliquotRootCache.containsKey(parent.getLSID())) _aliquotRootCache.put(outputAliquot.getLSID(), rootMaterialRowId); // add self's root to cache sql.addAll(rec._protApp.getRowId(), rec._protApp._object.getRunId(), rootMaterialRowId, parent.getLSID(), outputAliquot.getRowId()); - + new SqlExecutor(tableInfo.getSchema()).execute(sql); } } } } - + private void saveExpMaterialOutputs(List protAppRecords) { for (ProtocolAppRecord rec : protAppRecords) diff --git a/experiment/src/org/labkey/experiment/controllers/exp/ExperimentController.java b/experiment/src/org/labkey/experiment/controllers/exp/ExperimentController.java index 813c25e8c9e..eab0d71f2b6 100644 --- a/experiment/src/org/labkey/experiment/controllers/exp/ExperimentController.java +++ b/experiment/src/org/labkey/experiment/controllers/exp/ExperimentController.java @@ -79,6 +79,7 @@ import org.labkey.api.data.ConvertHelper; import org.labkey.api.data.DataRegion; import org.labkey.api.data.DataRegionSelection; +import org.labkey.api.data.DbSchema; import org.labkey.api.data.DbScope; import org.labkey.api.data.DisplayColumn; import org.labkey.api.data.ExcelWriter; @@ -89,6 +90,7 @@ import org.labkey.api.data.SimpleDisplayColumn; import org.labkey.api.data.SimpleFilter; import org.labkey.api.data.Sort; +import org.labkey.api.data.SqlSelector; import org.labkey.api.data.TSVJSONWriter; import org.labkey.api.data.TSVWriter; import org.labkey.api.data.TableInfo; @@ -199,6 +201,7 @@ import org.labkey.api.security.permissions.ReadPermission; import org.labkey.api.security.permissions.SampleWorkflowDeletePermission; import org.labkey.api.security.permissions.SiteAdminPermission; +import org.labkey.api.security.permissions.TroubleshooterPermission; import org.labkey.api.security.permissions.UpdatePermission; import org.labkey.api.settings.AppProps; import org.labkey.api.settings.ConceptURIProperties; @@ -215,6 +218,7 @@ import org.labkey.api.util.ExceptionUtil; import org.labkey.api.util.FileStream; import org.labkey.api.util.FileUtil; +import org.labkey.api.util.HtmlString; import org.labkey.api.util.HtmlStringBuilder; import org.labkey.api.util.ImageUtil; import org.labkey.api.util.JSoupUtil; @@ -288,6 +292,7 @@ import org.labkey.experiment.api.ExpSampleTypeImpl; import org.labkey.experiment.api.Experiment; import org.labkey.experiment.api.ExperimentServiceImpl; +import org.labkey.experiment.api.GraphAlgorithms; import org.labkey.experiment.api.ProtocolActionStepDetail; import org.labkey.experiment.api.SampleTypeServiceImpl; import org.labkey.experiment.api.SampleTypeUpdateServiceDI; @@ -351,15 +356,18 @@ import static org.labkey.api.util.DOM.Attribute.name; import static org.labkey.api.util.DOM.Attribute.size; import static org.labkey.api.util.DOM.Attribute.src; +import static org.labkey.api.util.DOM.Attribute.target; import static org.labkey.api.util.DOM.Attribute.type; import static org.labkey.api.util.DOM.Attribute.value; import static org.labkey.api.util.DOM.Attribute.width; import static org.labkey.api.util.DOM.DIV; import static org.labkey.api.util.DOM.IMG; import static org.labkey.api.util.DOM.INPUT; +import static org.labkey.api.util.DOM.LI; import static org.labkey.api.util.DOM.TABLE; import static org.labkey.api.util.DOM.TD; import static org.labkey.api.util.DOM.TR; +import static org.labkey.api.util.DOM.UL; import static org.labkey.api.util.DOM.at; import static org.labkey.api.util.DOM.cl; import static org.labkey.experiment.ExpDataIterators.setContainerFilterForImport; @@ -789,7 +797,7 @@ private Pair> createViews(ExpObjectForm form, ExperimentRunType selectedType = ExperimentRunType.getSelectedFilter(types, getViewContext().getRequest().getParameter("experimentRunFilter")); ChooseExperimentTypeBean bean = new ChooseExperimentTypeBean(types, selectedType, getViewContext().getActionURL().clone(), protocols); - JspView chooserView = new JspView<>("/org/labkey/experiment/experimentRunQueryHeader.jsp", bean, errors); + JspView chooserView = new JspView<>("/org/labkey/experiment/experimentRunQueryHeader.jsp", bean, errors); ExperimentRunListView runListView = ExperimentRunListView.createView(getViewContext(), bean.getSelectedFilter(), true); runListView.getRunTable().setExperiment(_experiment); @@ -2509,10 +2517,8 @@ protected ModelAndView getDataView(DataForm form, BindException errors) throws I try { // Try to write the exception back to the caller if we haven't already flushed the buffer - try(ApiJsonWriter writer = new ApiJsonWriter(getViewContext().getResponse())) - { - writer.writeResponse(e); - } + ApiJsonWriter writer = new ApiJsonWriter(getViewContext().getResponse()); + writer.writeResponse(e); } catch (IllegalStateException ise) { @@ -7367,6 +7373,47 @@ public Object execute(Object o, BindException errors) throws Exception } } + @Marshal(Marshaller.Jackson) + @RequiresPermission(AdminPermission.class) + public static class CheckEdgesAction extends ReadOnlyApiAction + { + @Override + public Object execute(Object o, BindException errors) throws Exception + { + List result; + DbSchema schema = ExperimentService.get().getSchema(); + TableInfo edgeTable = schema.getTable("Edge"); + + if (null != edgeTable.getColumn("fromObjectId")) + { + result = detectCycleEdges(schema).stream().map(e -> new Long[]{e.first, e.second}).collect(toList()); + } + else + { + var edges = new SqlSelector(ExperimentService.get().getSchema(), "SELECT fromLsid, toLsid FROM exp.Edge") + .resultSetStream() + .map(r -> { try { return new Pair<>(r.getString(1), r.getString(2)); } catch (SQLException x) { throw new RuntimeException(x); } }) + .collect(toList()); + var cycles = (new GraphAlgorithms()).detectCycleInDirectedGraph(edges); + result = cycles.stream().map(e -> new String[]{e.first, e.second}).collect(toList()); + } + + JSONObject ret = new JSONObject(); + ret.put("result", result); + ret.put("success", true); + return ret; + } + + public static Collection> detectCycleEdges(DbSchema schema) + { + var edges = new SqlSelector(schema, "SELECT fromObjectId, toObjectId FROM exp.Edge") + .resultSetStream() + .map(r -> { try { return new Pair<>(r.getLong(1), r.getLong(2)); } catch (SQLException x) { throw new RuntimeException(x); } }) + .collect(toList()); + return (new GraphAlgorithms()).detectCycleInDirectedGraph(edges); + } + } + @RequiresPermission(UpdatePermission.class) public static class UpdateMaterialQueryRowAction extends UserSchemaAction { @@ -7402,7 +7449,7 @@ private static long getSampleId(QueryUpdateForm tableForm) { sampleId = ConvertHelper.convert(tableForm.getPkVal(), Long.class); } - catch (ConversionException e) + catch (ConversionException _) { } if (null == sampleId) @@ -8147,6 +8194,99 @@ public ModelAndView getView(Object o, BindException errors) throws SQLException } } + /* Also see API CheckEdgesAction */ + @RequiresPermission(TroubleshooterPermission.class) + public static class CycleCheckAction extends FormViewAction + { + List cycleObjectIds = null; + + @Override + public void validateCommand(Object target, Errors errors) + { + + } + + @Override + public ModelAndView getView(Object o, boolean reshow, BindException errors) + { + if (!reshow) + { + return new HtmlView( + DIV("This operation can use a lot of memory.", + LK.FORM(at(method,"POST"), + PageFlowUtil.button("Continue").submit(true))) + ); + } + + if (null == cycleObjectIds) + return new HtmlView(HtmlString.of("No cycles found")); + + var cf = new ContainerFilter.AllFolders(getUser()); + Map map = resolveCycleObjects(cf, cycleObjectIds); + + ExperimentUrls urls = ExperimentUrls.get(); + return new HtmlView( + DIV("Cycle found involving these objects.", + UL(cycleObjectIds.stream().map((objectid) -> + { + ExpObject exp = map.get(objectid); + if (exp instanceof ExpMaterial mat) + return LI(A(at(target, "_blank", href, urls.getMaterialDetailsURL(mat)), objectid + " : material - " + mat.getName())); + else if (exp instanceof ExpRun run) + return LI(A(at(target, "_blank", href, urls.getRunTextURL(run)), objectid + " : run - " + run.getName())); + else if (exp instanceof ExpData data) + return LI(A(at(target, "_blank", href, urls.getDataDetailsURL(data)), objectid + " : run - " + data.getName())); + else + return LI(String.valueOf(objectid)); + })) + ) + ); + } + + @Override + public boolean handlePost(Object o, BindException errors) + { + cycleObjectIds = detectCycleObjectIds(ExperimentService.get().getSchema()); + return false; + } + + @Override + public URLHelper getSuccessURL(Object o) + { + return null; + } + + @Override + public void addNavTrail(NavTree root) + { + root.addChild("Cycle check"); + } + + public static @Nullable List detectCycleObjectIds(DbSchema schema) + { + var edges = new SqlSelector(schema, "SELECT fromObjectId, toObjectId FROM exp.Edge") + .resultSetStream() + .map(r -> { try { return new Pair<>(r.getLong(1), r.getLong(2)); } catch (SQLException x) { throw new RuntimeException(x); } }) + .collect(toList()); + var cyclesEdges = (new GraphAlgorithms()).detectCycleInDirectedGraph(edges); + if (cyclesEdges.isEmpty()) + return null; + var set = new LinkedHashSet(); + cyclesEdges.forEach(e -> { set.add(e.first); set.add(e.second); }); + return set.stream().toList(); + } + + public static Map resolveCycleObjects(ContainerFilter cf, List ids) + { + Map map = new LongHashMap<>(); + ExperimentServiceImpl.get().getExpMaterialsByObjectId(cf, ids).forEach(m -> map.put(m.getObjectId(), m)); + ExperimentServiceImpl.get().getExpDatasByObjectId(cf, ids).forEach(d -> map.put(d.getObjectId(), d)); + ExperimentServiceImpl.get().getRunsByObjectId(cf, ids).forEach(r -> map.put(r.getObjectId(), r)); + return map; + } + + } + @RequiresPermission(AdminPermission.class) public static class MissingFilesCheckAction extends ReadOnlyApiAction { From d844fa4eba9f804b8efd0f87ccaefa046ae61b6e Mon Sep 17 00:00:00 2001 From: labkey-jeckels Date: Tue, 26 May 2026 20:43:48 -0700 Subject: [PATCH 3/3] Cleanup --- .../controllers/exp/ExperimentController.java | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/experiment/src/org/labkey/experiment/controllers/exp/ExperimentController.java b/experiment/src/org/labkey/experiment/controllers/exp/ExperimentController.java index eab0d71f2b6..9a188ba7177 100644 --- a/experiment/src/org/labkey/experiment/controllers/exp/ExperimentController.java +++ b/experiment/src/org/labkey/experiment/controllers/exp/ExperimentController.java @@ -8230,14 +8230,16 @@ public ModelAndView getView(Object o, boolean reshow, BindException errors) UL(cycleObjectIds.stream().map((objectid) -> { ExpObject exp = map.get(objectid); - if (exp instanceof ExpMaterial mat) - return LI(A(at(target, "_blank", href, urls.getMaterialDetailsURL(mat)), objectid + " : material - " + mat.getName())); - else if (exp instanceof ExpRun run) - return LI(A(at(target, "_blank", href, urls.getRunTextURL(run)), objectid + " : run - " + run.getName())); - else if (exp instanceof ExpData data) - return LI(A(at(target, "_blank", href, urls.getDataDetailsURL(data)), objectid + " : run - " + data.getName())); - else - return LI(String.valueOf(objectid)); + return switch (exp) + { + case ExpMaterial mat -> + LI(A(at(target, "_blank", href, urls.getMaterialDetailsURL(mat)), objectid + " : material - " + mat.getName())); + case ExpRun run -> + LI(A(at(target, "_blank", href, urls.getRunTextURL(run)), objectid + " : run - " + run.getName())); + case ExpData data -> + LI(A(at(target, "_blank", href, urls.getDataDetailsURL(data)), objectid + " : run - " + data.getName())); + case null, default -> LI(objectid + " : unknown object"); + }; })) ) );