From 28a09b5999dc1da8f0ea53ad3ad2d804c9e62212 Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Tue, 7 Apr 2026 09:19:22 -0700 Subject: [PATCH 01/13] Replace Graphviz/DOT with a Java implementation --- api/build.gradle | 26 ++++++++ .../api/module/ModuleDependencySorter.java | 43 ++++++------ .../core/security/SecurityController.java | 52 +++------------ .../pipeline/analysis/AnalysisController.java | 65 +++++++++++++------ 4 files changed, 99 insertions(+), 87 deletions(-) diff --git a/api/build.gradle b/api/build.gradle index 28eb3ee8d1e..ec41567f313 100644 --- a/api/build.gradle +++ b/api/build.gradle @@ -381,6 +381,32 @@ dependencies { ) ) + BuildUtils.addExternalDependency( + project, + new ExternalDependency( + "org.graphper:graph-support-core:${graphSupportVersion}", + "graph-support-core", + "graph-support", + "https://github.com/jamisonjiang/graph-support", + ExternalDependency.APACHE_2_LICENSE_NAME, + ExternalDependency.APACHE_2_LICENSE_URL, + "Graphviz Java API", + ) + ) + + BuildUtils.addExternalDependency( + project, + new ExternalDependency( + "org.graphper:graph-support-dot:${graphSupportVersion}", + "graph-support-dot", + "graph-support", + "https://github.com/jamisonjiang/graph-support", + ExternalDependency.APACHE_2_LICENSE_NAME, + ExternalDependency.APACHE_2_LICENSE_URL, + "DOT parsing support", + ) + ) + BuildUtils.addExternalDependency( project, new ExternalDependency( diff --git a/api/src/org/labkey/api/module/ModuleDependencySorter.java b/api/src/org/labkey/api/module/ModuleDependencySorter.java index aac12caf743..d3396ebd9ec 100644 --- a/api/src/org/labkey/api/module/ModuleDependencySorter.java +++ b/api/src/org/labkey/api/module/ModuleDependencySorter.java @@ -18,14 +18,16 @@ import org.apache.commons.collections4.MultiValuedMap; import org.apache.commons.collections4.multimap.ArrayListValuedHashMap; -import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; +import org.graphper.api.GraphResource; +import org.graphper.api.Graphviz; +import org.graphper.parser.DotParser; import org.junit.Assert; import org.junit.Test; import org.labkey.api.collections.CaseInsensitiveHashSet; -import org.labkey.api.util.DotRunner; import org.labkey.api.util.FileUtil; import org.labkey.api.util.Pair; +import org.labkey.api.util.logging.LogHelper; import java.io.File; import java.util.ArrayList; @@ -34,12 +36,12 @@ import java.util.stream.Collectors; /** - * Orders modules so that each module will always be after all of the modules it depends on. - * User: jeckels - * Date: Jun 6, 2006 + * Orders modules so that each module will always be after all the modules it depends on. */ public class ModuleDependencySorter { + private static final Logger LOG = LogHelper.getLogger(ModuleDependencySorter.class, "Module dependency information"); + public List sortModulesByDependencies(List modules) { List>> dependencies = new ArrayList<>(); @@ -101,7 +103,7 @@ public List sortModulesByDependencies(List modules) if (module.getName().equalsIgnoreCase("core")) { result.remove(i); - result.add(0, module); + result.addFirst(module); break; } } @@ -128,41 +130,36 @@ private Module findModuleWithoutDependencies(List>> dep throw new IllegalArgumentException("Module '" + moduleName + "' (" + entry.getKey().getClass().getName() + ") is listed as being dependent on itself."); } - StringBuilder sb = new StringBuilder(); - for (Pair> dependencyInfo : dependencies) - { - if (!sb.isEmpty()) - { - sb.append(", "); - } - sb.append(dependencyInfo.getKey().getName()); - } + String involved = dependencies.stream() + .map(pair -> pair.getKey().getName()) + .collect(Collectors.joining(", ")); // Generate an SVG diagram that shows all remaining dependencies graphModuleDependencies(dependencies, "involved"); - throw new IllegalArgumentException("Unable to resolve module dependencies. The following modules are somehow involved: " + sb); + throw new IllegalArgumentException("Unable to resolve module dependencies. The following modules are somehow involved: " + involved); } private void graphModuleDependencies(List>> dependencies, @SuppressWarnings("SameParameterValue") String adjective) { - Logger log = LogManager.getLogger(ModuleDependencySorter.class); - try { File dir = FileUtil.getTempDirectory(); String dot = buildDigraph(dependencies); + Graphviz graph = DotParser.parse(dot); File svgFile = FileUtil.createTempFile("modules", ".svg", dir); - DotRunner runner = new DotRunner(dir, dot); - runner.addSvgOutput(svgFile); - runner.execute(); - log.info("For a diagram of " + adjective + " module dependencies, see " + svgFile.getAbsolutePath()); + try (GraphResource resource = graph.toSvg()) + { + resource.save(svgFile.getParent(), svgFile.getName()); + } + + LOG.info("For a diagram of {} module dependencies, see {}", adjective, svgFile.getAbsolutePath()); } catch (Exception e) { - log.error("Error running dot", e); + LOG.error("Error running dot", e); } } diff --git a/core/src/org/labkey/core/security/SecurityController.java b/core/src/org/labkey/core/security/SecurityController.java index 6e74a24b6c3..62d4d375f46 100644 --- a/core/src/org/labkey/core/security/SecurityController.java +++ b/core/src/org/labkey/core/security/SecurityController.java @@ -20,6 +20,8 @@ import org.apache.commons.collections4.IteratorUtils; import org.apache.commons.lang3.StringUtils; import org.apache.poi.ss.usermodel.Sheet; +import org.graphper.api.Graphviz; +import org.graphper.parser.DotParser; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import org.json.JSONObject; @@ -119,9 +121,6 @@ import org.labkey.api.security.roles.RoleManager; import org.labkey.api.security.roles.SiteAdminRole; import org.labkey.api.settings.AppProps; -import org.labkey.api.util.DotRunner; -import org.labkey.api.util.FileUtil; -import org.labkey.api.util.HelpTopic; import org.labkey.api.util.HtmlString; import org.labkey.api.util.HtmlStringBuilder; import org.labkey.api.util.PageFlowUtil; @@ -149,7 +148,6 @@ import org.springframework.validation.ObjectError; import org.springframework.web.servlet.ModelAndView; -import java.io.File; import java.io.IOException; import java.io.Writer; import java.sql.SQLException; @@ -2099,50 +2097,16 @@ public ApiResponse execute(GroupDiagramForm form, BindException errors) throws E } else { - String graph = GroupManager.getGroupGraphDot(groups, getUser(), form.getHideUnconnected()); - File dir = FileUtil.getTempDirectory(); - File svgFile = null; - - try - { - svgFile = FileUtil.createTempFile("groups", ".svg", dir); - svgFile.deleteOnExit(); - DotRunner runner = new DotRunner(dir, graph); - runner.addSvgOutput(svgFile); - runner.execute(); - String svg = PageFlowUtil.getFileContentsAsString(svgFile); - - int idx = svg.indexOf(" + public static class FolderAccessAction extends SimpleViewAction { @Override public ModelAndView getView(FolderAccessForm form, BindException errors) @@ -2409,7 +2373,7 @@ controller.new GroupPermissionAction(), new UpdatePermissionsAction(), new ShowRegistrationEmailAction(), new GroupDiagramAction(), - controller.new FolderAccessAction() + new FolderAccessAction() ); // @RequiresPermission(UserManagementPermission.class) diff --git a/pipeline/src/org/labkey/pipeline/analysis/AnalysisController.java b/pipeline/src/org/labkey/pipeline/analysis/AnalysisController.java index 0a53f7d1062..5f664260d43 100644 --- a/pipeline/src/org/labkey/pipeline/analysis/AnalysisController.java +++ b/pipeline/src/org/labkey/pipeline/analysis/AnalysisController.java @@ -22,6 +22,8 @@ import org.apache.commons.lang3.StringUtils; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; +import org.graphper.api.Graphviz; +import org.graphper.parser.DotParser; import org.jetbrains.annotations.Nullable; import org.json.JSONArray; import org.json.JSONObject; @@ -658,25 +660,45 @@ private DOM.Renderable generateGraph(@Nullable TaskPipeline pipeline) } File svgFile = null; - try - { - File dir = FileUtil.getTempDirectory(); - String dot = buildDigraph(pipeline); - svgFile = FileUtil.createTempFile("pipeline", ".svg", dir); - DotRunner runner = new DotRunner(dir, dot); - runner.addSvgOutput(svgFile); - runner.execute(); - return HtmlString.unsafe(PageFlowUtil.getFileContentsAsString(svgFile)); - } - catch (Exception e) - { - LOG.error("Error running dot", e); - } - finally + File dir = FileUtil.getTempDirectory(); + String dot = buildDigraph(pipeline, true); + + if (null != dot) { - if (svgFile != null) - svgFile.delete(); + String htmlOld = "Oops... error with DotRunner!"; + try + { + svgFile = FileUtil.createTempFile("pipeline", ".svg", dir); + DotRunner runner = new DotRunner(dir, dot); + runner.addSvgOutput(svgFile); + runner.execute(); + htmlOld = PageFlowUtil.getFileContentsAsString(svgFile); + } + catch (Exception e) + { + LOG.error("Error running dot", e); + } + finally + { + if (svgFile != null) + svgFile.delete(); + } + + String html = "Oops... error with DotParser!"; + try + { + dot = buildDigraph(pipeline, false); + Graphviz graph = DotParser.parse(dot); + html = graph.toSvgStr(); + } + catch (Exception e) + { + LOG.error("Error with DotParser!", e); + } + + return HtmlString.unsafe(htmlOld + "
" + html); } + return null; } @@ -692,7 +714,7 @@ private DOM.Renderable generateGraph(@Nullable TaskPipeline pipeline) * +---------+----------+ * */ - private String buildDigraph(TaskPipeline pipeline) + private String buildDigraph(TaskPipeline pipeline, boolean dotRunner) { TaskId[] progression = pipeline.getTaskProgression(); if (progression == null) @@ -701,6 +723,9 @@ private String buildDigraph(TaskPipeline pipeline) StringBuilder sb = new StringBuilder(); sb.append("digraph pipeline {\n"); + if (!dotRunner) + sb.append("style=\"invis\";\nmargin=\"0,0\";\n"); // TODO: graph-support doesn't seem to respect "transparent" + // First, add all the nodes for (TaskId taskId : progression) { @@ -730,7 +755,7 @@ private String buildDigraph(TaskPipeline pipeline) if (factory instanceof CommandTaskImpl.Factory f) { sb.append(StringUtils.join( - Collections2.transform(f.getInputPaths().keySet(), (Function) input -> escapeDotFieldLabel(input) + "\\l"), + Collections2.transform(f.getInputPaths().keySet(), (Function) this::escapeDotFieldLabel), // + "\\l"), TODO: Add this back once graph-support supports it " | ")); } else @@ -747,7 +772,7 @@ private String buildDigraph(TaskPipeline pipeline) { sb.append(StringUtils.join( - Collections2.transform(f.getOutputPaths().keySet(), (Function) input -> escapeDotFieldLabel(input) + "\\r"), + Collections2.transform(f.getOutputPaths().keySet(), (Function) this::escapeDotFieldLabel), // + "\\r"), TODO: Add this back once graph-support supports it " | ")); } else From 7564bc2eb063590d76e862634bdf70ba2e4c1f22 Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Tue, 7 Apr 2026 11:03:47 -0700 Subject: [PATCH 02/13] Eliminate DotSvgAction --- core/src/org/labkey/core/CoreModule.java | 3 - .../labkey/core/portal/UtilController.java | 107 ------------------ 2 files changed, 110 deletions(-) delete mode 100644 core/src/org/labkey/core/portal/UtilController.java diff --git a/core/src/org/labkey/core/CoreModule.java b/core/src/org/labkey/core/CoreModule.java index 7b89ff030b2..c13e6a705ef 100644 --- a/core/src/org/labkey/core/CoreModule.java +++ b/core/src/org/labkey/core/CoreModule.java @@ -105,7 +105,6 @@ import org.labkey.api.notification.NotificationMenuView; import org.labkey.api.portal.ProjectUrls; import org.labkey.api.premium.AntiVirusProviderRegistry; -import org.labkey.api.products.Product; import org.labkey.api.products.ProductRegistry; import org.labkey.api.qc.DataStateManager; import org.labkey.api.query.DefaultSchema; @@ -267,7 +266,6 @@ import org.labkey.core.portal.CollaborationFolderType; import org.labkey.core.portal.PortalJUnitTest; import org.labkey.core.portal.ProjectController; -import org.labkey.core.portal.UtilController; import org.labkey.core.products.ProductController; import org.labkey.core.project.FolderNavigationForm; import org.labkey.core.qc.CoreQCStateHandler; @@ -443,7 +441,6 @@ protected void init() addController("core", CoreController.class); addController("analytics", AnalyticsController.class); addController("project", ProjectController.class); - addController("util", UtilController.class); addController("logger", LoggerController.class); addController("mini-profiler", MiniProfilerController.class); addController("notification", NotificationController.class); diff --git a/core/src/org/labkey/core/portal/UtilController.java b/core/src/org/labkey/core/portal/UtilController.java deleted file mode 100644 index a89e401d5bb..00000000000 --- a/core/src/org/labkey/core/portal/UtilController.java +++ /dev/null @@ -1,107 +0,0 @@ -/* - * Copyright (c) 2012-2019 LabKey Corporation - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package org.labkey.core.portal; - -import org.apache.logging.log4j.Logger; -import org.apache.logging.log4j.LogManager; -import org.labkey.api.action.SimpleViewAction; -import org.labkey.api.action.SpringActionController; -import org.labkey.api.security.RequiresPermission; -import org.labkey.api.security.permissions.ReadPermission; -import org.labkey.api.util.DotRunner; -import org.labkey.api.util.FileUtil; -import org.labkey.api.util.PageFlowUtil; -import org.labkey.api.view.HtmlView; -import org.labkey.api.view.NavTree; -import org.labkey.api.view.UnauthorizedException; -import org.labkey.api.view.WebPartView; -import org.labkey.api.view.template.PageConfig; -import org.springframework.validation.BindException; -import org.springframework.web.servlet.ModelAndView; - -import java.io.File; - -/** - * User: matthewb - * Date: 2011-11-18 - * Time: 10:26 AM - */ -public class UtilController extends SpringActionController -{ - private static final DefaultActionResolver _actionResolver = new DefaultActionResolver(UtilController.class); - private static final Logger _log = LogManager.getLogger(ProjectController.class); - - public UtilController() - { - setActionResolver(_actionResolver); - } - - - public static class DotForm - { - public String getDot() - { - return _text; - } - public void setDot(String text) - { - _text = text; - } - String _text; - } - - - @RequiresPermission(ReadPermission.class) - public static class DotSvgAction extends SimpleViewAction - { - @Override - public ModelAndView getView(DotForm form, BindException errors) throws Exception - { - // Don't allow GET to avoid security holes as this may inject script - if (!isPost()) - throw new UnauthorizedException("use POST"); - - getPageConfig().setTemplate(PageConfig.Template.None); - - String dot = form.getDot(); - File dir = FileUtil.getTempDirectory(); - File svgFile = null; - try - { - svgFile = FileUtil.createTempFile("groups", ".svg", dir); - svgFile.deleteOnExit(); - DotRunner runner = new DotRunner(dir, dot); - runner.addSvgOutput(svgFile); - runner.execute(); - String svg = PageFlowUtil.getFileContentsAsString(svgFile); - String html = svg.substring(svg.indexOf(" Date: Wed, 8 Apr 2026 14:05:38 -0700 Subject: [PATCH 03/13] Refactor experiment graphs to use graph-support --- .../src/org/labkey/experiment/DotGraph.java | 4 +- .../labkey/experiment/ExperimentRunGraph.java | 606 ++++-------------- .../FileBasedExperimentRunGraph.java | 392 +++++++++++ .../src/org/labkey/experiment/XarReader.java | 2 +- .../org/labkey/experiment/api/ExpRunImpl.java | 5 +- .../controllers/exp/ExperimentController.java | 13 +- .../exp/experimentRunGraphView.jsp | 14 +- .../pipeline/ExpGeneratorHelper.java | 4 +- 8 files changed, 539 insertions(+), 501 deletions(-) create mode 100644 experiment/src/org/labkey/experiment/FileBasedExperimentRunGraph.java diff --git a/experiment/src/org/labkey/experiment/DotGraph.java b/experiment/src/org/labkey/experiment/DotGraph.java index f11618bb6bb..61b52548d35 100644 --- a/experiment/src/org/labkey/experiment/DotGraph.java +++ b/experiment/src/org/labkey/experiment/DotGraph.java @@ -113,7 +113,7 @@ public Long getDGroupId(long rowIdD) return getGroupId(rowIdD, _pendingDNodes, _writtenDNodes); } - public @Nullable Long getGroupId(Long rowId, Map pendingNodes, Map writtenNodes) + private @Nullable Long getGroupId(Long rowId, Map pendingNodes, Map writtenNodes) { DotNode node = null; if (pendingNodes.containsKey(rowId)) @@ -348,7 +348,7 @@ public void flushPending() _groupPANodes.clear(); } - public void writePending(Map pendingMap, Map writtenMap) + private void writePending(Map pendingMap, Map writtenMap) { Set nodesToMove = new HashSet<>(); for (Long key : pendingMap.keySet()) diff --git a/experiment/src/org/labkey/experiment/ExperimentRunGraph.java b/experiment/src/org/labkey/experiment/ExperimentRunGraph.java index 4f7b8265315..d85bf75a665 100644 --- a/experiment/src/org/labkey/experiment/ExperimentRunGraph.java +++ b/experiment/src/org/labkey/experiment/ExperimentRunGraph.java @@ -1,22 +1,8 @@ -/* - * Copyright (c) 2008-2019 LabKey Corporation - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ package org.labkey.experiment; -import org.apache.logging.log4j.LogManager; -import org.apache.logging.log4j.Logger; +import org.graphper.api.Graphviz; +import org.graphper.draw.ExecuteException; +import org.graphper.parser.DotParser; import org.labkey.api.data.Container; import org.labkey.api.exp.ExperimentException; import org.labkey.api.exp.api.ExpData; @@ -25,27 +11,11 @@ import org.labkey.api.exp.api.ExpProtocolApplication; import org.labkey.api.exp.api.ExpRun; import org.labkey.api.exp.api.ExpRunItem; -import org.labkey.api.settings.AppProps; -import org.labkey.api.util.ConfigurationException; -import org.labkey.api.util.DotRunner; -import org.labkey.api.util.FileUtil; -import org.labkey.api.util.ImageUtil; -import org.labkey.api.view.ViewContext; import org.labkey.experiment.api.ExpDataImpl; import org.labkey.experiment.api.ExpMaterialImpl; import org.labkey.experiment.api.ExpProtocolApplicationImpl; import org.labkey.experiment.api.ExpRunImpl; -import javax.imageio.ImageIO; -import java.lang.ref.Cleaner; -import java.awt.image.BufferedImage; -import java.io.BufferedReader; -import java.io.File; -import java.io.FileInputStream; -import java.io.FileOutputStream; -import java.io.IOException; -import java.io.InputStreamReader; -import java.io.OutputStreamWriter; import java.io.PrintWriter; import java.io.StringWriter; import java.util.ArrayList; @@ -53,317 +23,171 @@ import java.util.HashMap; import java.util.List; import java.util.Map; -import java.util.Objects; import java.util.SortedMap; import java.util.TreeMap; -import java.util.concurrent.locks.Lock; -import java.util.concurrent.locks.ReentrantReadWriteLock; - -/** - * User: migra - * Date: Jun 15, 2005 - * Time: 12:57:02 AM - */ + public class ExperimentRunGraph { - private static File baseDirectory; - private static final Logger _log = LogManager.getLogger(ExperimentRunGraph.class); private static final int MAX_WIDTH_SMALL_FONT = 8; private static final int MAX_WIDTH_BIG_FONT = 3; private static final int MAX_SIBLINGS = 5; private static final int MIN_SIBLINGS = 3; - /** - * It's safe for lots of threads to be reading but only one should be creating or deleting at a time. - * We could make separate locks for each folder but leaving it with one global lock for now. - */ - private static final ReentrantReadWriteLock LOCK = new ReentrantReadWriteLock(); - - public synchronized static File getBaseDirectory() throws IOException + public static String getSvg(String dot) throws ExecuteException { - if (baseDirectory == null) - { - File tempDir = FileUtil.appendName(FileUtil.getTempDirectory(), "ExperimentRunGraphs"); - if (tempDir.exists()) - { - FileUtil.deleteDirectoryContents(tempDir); - } - else - { - if (!FileUtil.mkdirs(tempDir)) - { - throw new IOException("Unable to create temporary directory for experiment run graphs: " + tempDir.getPath()); - } - } - baseDirectory = tempDir; - } - return baseDirectory; - } + Graphviz graph = DotParser.parse(dot); + String svg = graph.toSvgStr(); - private synchronized static File getFolderDirectory(Container container) throws IOException - { - File result = FileUtil.appendName(getBaseDirectory(), "Folder" + container.getRowId()); - FileUtil.mkdirs(result); - for (int i = 0; i < 5; i++) - { - if (result.isDirectory()) - { - return result; - } - else - { - try - { - Thread.sleep(1); - } - catch (InterruptedException e) {} - FileUtil.mkdirs(result); - } - } - if (!result.isDirectory()) - { - throw new IOException("Failed to create directory " + result); - } - return result; + return svg; } - /** - * Creates a run graph, given the configuration parameters. Note that this creates a lock on the directory - * that contains the files, which must be cleared by calling release() on the resulting RunGraphFiles object. - */ - public static RunGraphFiles generateRunGraph(ViewContext ctx, ExpRunImpl run, boolean detail, String focus, String focusType) throws ExperimentException, IOException, InterruptedException + public static String getDotGraph(Container c, ExpRunImpl run, boolean detail, String focus, String focusType) { - boolean success = false; - - File imageFile = new File(getBaseFileName(run, detail, focus) + ".png"); - File mapFile = new File(getBaseFileName(run, detail, focus) + ".map"); + Integer focusId = null; + String typeCode = focusType; - // First acquire a read lock so we know that another thread won't be deleting these files out from under us - Lock readLock = LOCK.readLock(); - readLock.lock(); - - try + if (null != focus && !focus.isEmpty()) { - if (!AppProps.getInstance().isDevMode() && imageFile.exists() && mapFile.exists()) + if (!Character.isDigit(focus.charAt(0))) { - success = true; - return new RunGraphFiles(mapFile, imageFile, readLock); + typeCode = focus.substring(0, 1); + focus = focus.substring(1); } - } - finally - { - // If we found useful files, don't release the lock because the caller will want to read them - if (!success) + try { - readLock.unlock(); + focusId = Integer.parseInt(focus); + run.trimRunTree(focusId, typeCode); } + catch (NumberFormatException | ExperimentException ignored) {} } - // We need to create files to open up a write lock - Lock writeLock = LOCK.writeLock(); - writeLock.lock(); + StringWriter writer = new StringWriter(); - try + try (PrintWriter out = new PrintWriter(writer)) { - testDotPath(); - - Integer focusId = null; - String typeCode = focusType; - - if (null != focus && !focus.isEmpty()) - { - if (!Character.isDigit(focus.charAt(0))) - { - typeCode = focus.substring(0, 1); - focus = focus.substring(1); - } - try - { - focusId = Integer.parseInt(focus); - run.trimRunTree(focusId, typeCode); - } - catch (NumberFormatException ignored) {} - } + GraphCtrlProps ctrlProps = analyzeGraph(run); - StringWriter writer = new StringWriter(); + DotGraph dg = new DotGraph(out, c, ctrlProps.fUseSmallFonts); - try (PrintWriter out = new PrintWriter(writer)) + if (!detail) + generateSummaryGraph(run, dg, ctrlProps); + else { - GraphCtrlProps ctrlProps = analyzeGraph(run); - - DotGraph dg = new DotGraph(out, ctx.getContainer(), ctrlProps.fUseSmallFonts); - - if (!detail) - generateSummaryGraph(run, dg, ctrlProps); - else + if (null != focusId) + dg.setFocus(focusId, typeCode); + + // add starting inputs to graph if they need grouping + Map materialRoles = run.getMaterialInputs(); + List inputMaterials = new ArrayList<>(materialRoles.keySet()); + inputMaterials.sort(new RoleAndNameComparator<>(materialRoles)); + Map dataRoles = run.getDataInputs(); + List inputDatas = new ArrayList<>(dataRoles.keySet()); + inputDatas.sort(new RoleAndNameComparator<>(dataRoles)); + if (!run.getProtocolApplications().isEmpty()) { - if (null != focusId) - dg.setFocus(focusId, typeCode); - - // add starting inputs to graph if they need grouping - Map materialRoles = run.getMaterialInputs(); - List inputMaterials = new ArrayList<>(materialRoles.keySet()); - inputMaterials.sort(new RoleAndNameComparator<>(materialRoles)); - Map dataRoles = run.getDataInputs(); - List inputDatas = new ArrayList<>(dataRoles.keySet()); - inputDatas.sort(new RoleAndNameComparator<>(dataRoles)); - if (!run.getProtocolApplications().isEmpty()) - { - long groupId = run.getProtocolApplications().get(0).getRowId(); - addStartingInputs(inputMaterials, inputDatas, groupId, dg, run.getRowId(), ctrlProps); - generateDetailGraph(run, dg, ctrlProps); - } + long groupId = run.getProtocolApplications().getFirst().getRowId(); + addStartingInputs(inputMaterials, inputDatas, groupId, dg, run.getRowId(), ctrlProps); + generateDetailGraph(run, dg, ctrlProps); } - dg.dispose(); - } - - String dotInput = writer.getBuffer().toString(); - DotRunner runner = new DotRunner(getFolderDirectory(run.getContainer()), dotInput); - runner.addCmapOutput(mapFile); - runner.addPngOutput(imageFile); - runner.execute(); - - mapFile.deleteOnExit(); - imageFile.deleteOnExit(); - - BufferedImage originalImage = ImageIO.read(imageFile); - if (originalImage == null) - { - throw new IOException("Unable to read image file " + imageFile.getAbsolutePath() + " of size " + imageFile.length() + " - disk may be full?"); } - - try (FileOutputStream fOut = new FileOutputStream(imageFile)) - { - // Write it back out to disk - double scale = ImageUtil.resizeImage(originalImage, fOut, .85, 6, BufferedImage.TYPE_INT_RGB); - - // Need to rewrite the image map to change the coordinates according to the scaling factor - resizeImageMap(mapFile, scale); - } - - // Start the procedure of downgrade our lock from write to read so that the caller can use the files - readLock.lock(); - return new RunGraphFiles(mapFile, imageFile, readLock); - } - catch (UnsatisfiedLinkError | NoClassDefFoundError e) - { - throw new ConfigurationException("Unable to resize image, likely a problem with missing Java Runtime libraries not being available", e); - } - finally - { - writeLock.unlock(); + dg.dispose(); } + + return writer.getBuffer().toString(); } - /** - * Shrink all the coordinates in an image map by a fixed ratio - */ - private static void resizeImageMap(File mapFile, double finalScale) throws IOException + private static GraphCtrlProps analyzeGraph(ExpRunImpl exp) { - StringBuilder sb = new StringBuilder(); + int maxSiblingsPerParent = MAX_SIBLINGS; + int maxMD = MIN_SIBLINGS; + int iMaxLevelStart = 0; + int iMaxLevelEnd = 0; + int curMI = 0; + int curDI = 0; + int curMO = 0; + int curDO = 0; + int prevS = 0; + int iLevelStart = 0; + int iLevelEnd = 0; + GraphCtrlProps ctrlProps = new GraphCtrlProps(); - try (FileInputStream fIn = new FileInputStream(mapFile)) + int i = 0; + List steps = exp.getProtocolApplications(); + for (ExpProtocolApplicationImpl step : steps) { - // Read in the original file, line by line - BufferedReader reader = new BufferedReader(new InputStreamReader(fIn)); - String line; - while ((line = reader.readLine()) != null) + int curS = step.getActionSequence(); + + Integer countSeq = ctrlProps.mPANodesPerSequence.get(curS); + if (null == countSeq) + countSeq = Integer.valueOf(1); + else + countSeq++; + ctrlProps.mPANodesPerSequence.put(curS, countSeq); + + if (curS != prevS) { - int coordsIndex = line.indexOf("coords=\""); - if (coordsIndex != -1) + if (curMI + curDI > maxMD) { - int openIndex = coordsIndex + "coords=\"".length(); - int closeIndex = line.indexOf("\"", openIndex); - if (closeIndex != -1) - { - // Parse and scale the coordinates - String coordsOriginal = line.substring(openIndex, closeIndex); - String[] coords = coordsOriginal.split(",|(\\s)"); - StringBuilder newLine = new StringBuilder(); - newLine.append(line.substring(0, openIndex)); - String separator = ""; - for (String coord : coords) - { - newLine.append(separator); - separator = ","; - newLine.append((int) (Integer.parseInt(coord.trim()) * finalScale)); - } - newLine.append(line.substring(closeIndex)); - line = newLine.toString(); - } + maxMD = curMI + curDI; + iMaxLevelStart = iLevelStart; + iMaxLevelEnd = iLevelEnd; + } + if (curMO + curDO > maxMD) + { + maxMD = curMO + curDO; + iMaxLevelStart = iLevelStart; + iMaxLevelEnd = iLevelEnd; } - sb.append(line); - sb.append("\n"); + prevS = curS; + curMI = 0; + curDI = 0; + curMO = 0; + curDO = 0; + iLevelStart = i; } + iLevelEnd = i; + curMI += Math.min(step.getInputMaterials().size(), maxSiblingsPerParent); + curDI += Math.min(step.getInputDatas().size(), maxSiblingsPerParent); + curMO += Math.min(step.getOutputMaterials().size(), maxSiblingsPerParent); + curDO += Math.min(step.getOutputDatas().size(), maxSiblingsPerParent); + i++; } - // Write the file back to the disk - try (FileOutputStream mapOut = new FileOutputStream(mapFile)) - { - OutputStreamWriter mapWriter = new OutputStreamWriter(mapOut); - mapWriter.write(sb.toString()); - mapWriter.flush(); - } - } - - private static class CacheClearer implements Runnable - { - private final Container _container; - - public CacheClearer(Container container) - { - _container = container; - } - - @Override - public boolean equals(Object o) + if (maxMD > MAX_WIDTH_BIG_FONT) { - if (this == o) return true; - if (o == null || getClass() != o.getClass()) return false; - CacheClearer that = (CacheClearer) o; - return Objects.equals(_container, that._container); + ctrlProps.fUseSmallFonts = true; + ctrlProps.maxNodesWidth = MAX_WIDTH_SMALL_FONT; } - - @Override - public int hashCode() + else { - return Objects.hash(_container); + ctrlProps.fUseSmallFonts = false; + ctrlProps.maxNodesWidth = MAX_WIDTH_BIG_FONT; } - @Override - public void run() + // try to adjust the number of siblings to fit the levels within the max width + while ((maxMD > ctrlProps.maxNodesWidth) && (maxSiblingsPerParent > MIN_SIBLINGS)) { - clearCache(_container); + curMI = 0; + curDI = 0; + curMO = 0; + curDO = 0; + maxSiblingsPerParent--; + for (i = iMaxLevelStart; i <= (Math.min(iMaxLevelEnd, iMaxLevelStart + maxSiblingsPerParent - 1)); i++) + { + ExpProtocolApplication step = steps.get(i); + curMI += Math.min(step.getInputMaterials().size(), maxSiblingsPerParent); + curDI += Math.min(step.getInputDatas().size(), maxSiblingsPerParent); + curMO += Math.min(step.getOutputMaterials().size(), maxSiblingsPerParent); + curDO += Math.min(step.getOutputDatas().size(), maxSiblingsPerParent); + } + maxMD = Math.max(curMO + curDO, curMI + curDI); } - } - public static Runnable getCacheClearingCommitTask(Container c) - { - return new CacheClearer(c); - } + ctrlProps.maxSiblingNodes = maxSiblingsPerParent; + if (exp.getMaterialInputs().size() + exp.getDataInputs().size() > ctrlProps.maxNodesWidth) + ctrlProps.fGroupInputs = true; - /** - * Clears out the cache of files for this container. Must be called after any operation that changes the way a graph - * would be generated. Typically, this includes deleting or inserting any run in the container, because that - * can change the connections between the runs, which is reflected in the graphs. - */ - public static void clearCache(Container container) - { - Lock deleteLock = LOCK.writeLock(); - deleteLock.lock(); - try - { - FileUtil.deleteDir(getFolderDirectory(container)); - } - catch (IOException e) - { - // Non-fatal - _log.error("Failed to clear cached experiment run graphs for container " + container, e); - } - finally - { - deleteLock.unlock(); - } + return ctrlProps; } /** @@ -421,7 +245,7 @@ private static void generateDetailGraph(ExpRunImpl expRun, DotGraph dg, GraphCtr { continue; } - + List inputMaterials = protApp.getInputMaterials(); List inputDatas = protApp.getInputDatas(); List outputMaterials = protApp.getOutputMaterials(); @@ -643,198 +467,4 @@ public int getPACountForSequence(int seq) return c.intValue(); } } - - - private static GraphCtrlProps analyzeGraph(ExpRunImpl exp) - { - int maxSiblingsPerParent = MAX_SIBLINGS; - int maxMD = MIN_SIBLINGS; - int iMaxLevelStart = 0; - int iMaxLevelEnd = 0; - int curMI = 0; - int curDI = 0; - int curMO = 0; - int curDO = 0; - int prevS = 0; - int iLevelStart = 0; - int iLevelEnd = 0; - GraphCtrlProps ctrlProps = new GraphCtrlProps(); - - int i = 0; - List steps = exp.getProtocolApplications(); - for (ExpProtocolApplicationImpl step : steps) - { - int curS = step.getActionSequence(); - - Integer countSeq = ctrlProps.mPANodesPerSequence.get(curS); - if (null == countSeq) - countSeq = Integer.valueOf(1); - else - countSeq++; - ctrlProps.mPANodesPerSequence.put(curS, countSeq); - - if (curS != prevS) - { - if (curMI + curDI > maxMD) - { - maxMD = curMI + curDI; - iMaxLevelStart = iLevelStart; - iMaxLevelEnd = iLevelEnd; - } - if (curMO + curDO > maxMD) - { - maxMD = curMO + curDO; - iMaxLevelStart = iLevelStart; - iMaxLevelEnd = iLevelEnd; - } - prevS = curS; - curMI = 0; - curDI = 0; - curMO = 0; - curDO = 0; - iLevelStart = i; - } - iLevelEnd = i; - curMI += Math.min(step.getInputMaterials().size(), maxSiblingsPerParent); - curDI += Math.min(step.getInputDatas().size(), maxSiblingsPerParent); - curMO += Math.min(step.getOutputMaterials().size(), maxSiblingsPerParent); - curDO += Math.min(step.getOutputDatas().size(), maxSiblingsPerParent); - i++; - } - - if (maxMD > MAX_WIDTH_BIG_FONT) - { - ctrlProps.fUseSmallFonts = true; - ctrlProps.maxNodesWidth = MAX_WIDTH_SMALL_FONT; - } - else - { - ctrlProps.fUseSmallFonts = false; - ctrlProps.maxNodesWidth = MAX_WIDTH_BIG_FONT; - } - - // try to adjust the number of siblings to fit the levels within the max width - while ((maxMD > ctrlProps.maxNodesWidth) && (maxSiblingsPerParent > MIN_SIBLINGS)) - { - curMI = 0; - curDI = 0; - curMO = 0; - curDO = 0; - maxSiblingsPerParent--; - for (i = iMaxLevelStart; i <= (Math.min(iMaxLevelEnd, iMaxLevelStart + maxSiblingsPerParent - 1)); i++) - { - ExpProtocolApplication step = steps.get(i); - curMI += Math.min(step.getInputMaterials().size(), maxSiblingsPerParent); - curDI += Math.min(step.getInputDatas().size(), maxSiblingsPerParent); - curMO += Math.min(step.getOutputMaterials().size(), maxSiblingsPerParent); - curDO += Math.min(step.getOutputDatas().size(), maxSiblingsPerParent); - } - maxMD = Math.max(curMO + curDO, curMI + curDI); - } - - ctrlProps.maxSiblingNodes = maxSiblingsPerParent; - if (exp.getMaterialInputs().size() + exp.getDataInputs().size() > ctrlProps.maxNodesWidth) - ctrlProps.fGroupInputs = true; - - return ctrlProps; - } - - private static void testDotPath() throws ExperimentException - { - File dir; - - try - { - dir = getBaseDirectory(); - } - catch (IOException e) - { - throw new ExperimentException(DotRunner.getConfigurationError(e)); - } - - DotRunner.testDotPath(dir); - } - - - private static String getBaseFileName(ExpRun run, boolean detail, String focus) throws IOException - { - String fileName; - if (null != focus) - fileName = getFolderDirectory(run.getContainer()) + File.separator + "run" + run.getRowId() + "Focus" + focus; - else - fileName = getFolderDirectory(run.getContainer()) + File.separator + "run" + run.getRowId() + (detail ? "Detail" : ""); - return fileName; - } - - private static final Cleaner CLEANER = Cleaner.create(); - - private static class FileLockState implements Runnable - { - private final Throwable _allocation; - private Lock _lock; - - private FileLockState(Throwable allocation, Lock lock) - { - _allocation = allocation; - _lock = lock; - } - - @Override - public void run() - { - if (_lock != null) - { - _log.error("Lock was not released. Creation was at:", _allocation); - _lock.unlock(); - _lock = null; - } - } - - private void release() - { - if (_lock != null) - { - _lock.unlock(); - _lock = null; - } - } - } - - /** - * Results for run graph generation. Must be released once the files have been consumed by the caller. - */ - public static class RunGraphFiles - { - private final File _mapFile; - private final File _imageFile; - private final FileLockState _state; - private final Cleaner.Cleanable _cleanable; - - public RunGraphFiles(File mapFile, File imageFile, Lock lock) - { - _mapFile = mapFile; - _imageFile = imageFile; - _state = new FileLockState(new Throwable(), lock); - _cleanable = CLEANER.register(this, _state); - } - - public File getMapFile() - { - return _mapFile; - } - - public File getImageFile() - { - return _imageFile; - } - - /** - * Release the lock on the files. - */ - public void release() - { - _state.release(); - _cleanable.clean(); - } - } } diff --git a/experiment/src/org/labkey/experiment/FileBasedExperimentRunGraph.java b/experiment/src/org/labkey/experiment/FileBasedExperimentRunGraph.java new file mode 100644 index 00000000000..793a070f7a5 --- /dev/null +++ b/experiment/src/org/labkey/experiment/FileBasedExperimentRunGraph.java @@ -0,0 +1,392 @@ +/* + * Copyright (c) 2008-2019 LabKey Corporation + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.labkey.experiment; + +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; +import org.labkey.api.data.Container; +import org.labkey.api.exp.ExperimentException; +import org.labkey.api.exp.api.ExpRun; +import org.labkey.api.settings.AppProps; +import org.labkey.api.util.ConfigurationException; +import org.labkey.api.util.DotRunner; +import org.labkey.api.util.FileUtil; +import org.labkey.api.util.ImageUtil; +import org.labkey.api.view.ViewContext; +import org.labkey.experiment.api.ExpRunImpl; + +import javax.imageio.ImageIO; +import java.awt.image.BufferedImage; +import java.io.BufferedReader; +import java.io.File; +import java.io.FileInputStream; +import java.io.FileOutputStream; +import java.io.IOException; +import java.io.InputStreamReader; +import java.io.OutputStreamWriter; +import java.lang.ref.Cleaner; +import java.util.Objects; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantReadWriteLock; + +// TODO: This entire class can be eliminated once we're happy with the new graph-support Java implementation +public class FileBasedExperimentRunGraph extends ExperimentRunGraph +{ + private static File baseDirectory; + private static final Logger _log = LogManager.getLogger(FileBasedExperimentRunGraph.class); + + /** + * It's safe for lots of threads to be reading but only one should be creating or deleting at a time. + * We could make separate locks for each folder but leaving it with one global lock for now. + */ + private static final ReentrantReadWriteLock LOCK = new ReentrantReadWriteLock(); + + public synchronized static File getBaseDirectory() throws IOException + { + if (baseDirectory == null) + { + File tempDir = FileUtil.appendName(FileUtil.getTempDirectory(), "ExperimentRunGraphs"); + if (tempDir.exists()) + { + FileUtil.deleteDirectoryContents(tempDir); + } + else + { + if (!FileUtil.mkdirs(tempDir)) + { + throw new IOException("Unable to create temporary directory for experiment run graphs: " + tempDir.getPath()); + } + } + baseDirectory = tempDir; + } + return baseDirectory; + } + + private synchronized static File getFolderDirectory(Container container) throws IOException + { + File result = FileUtil.appendName(getBaseDirectory(), "Folder" + container.getRowId()); + FileUtil.mkdirs(result); + for (int i = 0; i < 5; i++) + { + if (result.isDirectory()) + { + return result; + } + else + { + try + { + Thread.sleep(1); + } + catch (InterruptedException e) {} + FileUtil.mkdirs(result); + } + } + if (!result.isDirectory()) + { + throw new IOException("Failed to create directory " + result); + } + return result; + } + + /** + * Creates a run graph, given the configuration parameters. Note that this creates a lock on the directory + * that contains the files, which must be cleared by calling release() on the resulting RunGraphFiles object. + */ + public static RunGraphFiles generateRunGraph(ViewContext ctx, ExpRunImpl run, boolean detail, String focus, String focusType) throws ExperimentException, IOException, InterruptedException + { + boolean success = false; + + File imageFile = new File(getBaseFileName(run, detail, focus) + ".png"); + File mapFile = new File(getBaseFileName(run, detail, focus) + ".map"); + + // First acquire a read lock so we know that another thread won't be deleting these files out from under us + Lock readLock = LOCK.readLock(); + readLock.lock(); + + try + { + if (!AppProps.getInstance().isDevMode() && imageFile.exists() && mapFile.exists()) + { + success = true; + return new RunGraphFiles(mapFile, imageFile, readLock); + } + } + finally + { + // If we found useful files, don't release the lock because the caller will want to read them + if (!success) + { + readLock.unlock(); + } + } + + // We need to create files to open up a write lock + Lock writeLock = LOCK.writeLock(); + writeLock.lock(); + + try + { + testDotPath(); + + String dotInput = getDotGraph(ctx.getContainer(), run, detail, focus, focusType); + DotRunner runner = new DotRunner(getFolderDirectory(run.getContainer()), dotInput); + runner.addCmapOutput(mapFile); + runner.addPngOutput(imageFile); + runner.execute(); + + mapFile.deleteOnExit(); + imageFile.deleteOnExit(); + + BufferedImage originalImage = ImageIO.read(imageFile); + if (originalImage == null) + { + throw new IOException("Unable to read image file " + imageFile.getAbsolutePath() + " of size " + imageFile.length() + " - disk may be full?"); + } + + try (FileOutputStream fOut = new FileOutputStream(imageFile)) + { + // Write it back out to disk + double scale = ImageUtil.resizeImage(originalImage, fOut, .85, 6, BufferedImage.TYPE_INT_RGB); + + // Need to rewrite the image map to change the coordinates according to the scaling factor + resizeImageMap(mapFile, scale); + } + + // Start the procedure of downgrade our lock from write to read so that the caller can use the files + readLock.lock(); + return new RunGraphFiles(mapFile, imageFile, readLock); + } + catch (UnsatisfiedLinkError | NoClassDefFoundError e) + { + throw new ConfigurationException("Unable to resize image, likely a problem with missing Java Runtime libraries not being available", e); + } + finally + { + writeLock.unlock(); + } + } + + /** + * Shrink all the coordinates in an image map by a fixed ratio + */ + private static void resizeImageMap(File mapFile, double finalScale) throws IOException + { + StringBuilder sb = new StringBuilder(); + + try (FileInputStream fIn = new FileInputStream(mapFile)) + { + // Read in the original file, line by line + BufferedReader reader = new BufferedReader(new InputStreamReader(fIn)); + String line; + while ((line = reader.readLine()) != null) + { + int coordsIndex = line.indexOf("coords=\""); + if (coordsIndex != -1) + { + int openIndex = coordsIndex + "coords=\"".length(); + int closeIndex = line.indexOf("\"", openIndex); + if (closeIndex != -1) + { + // Parse and scale the coordinates + String coordsOriginal = line.substring(openIndex, closeIndex); + String[] coords = coordsOriginal.split(",|(\\s)"); + StringBuilder newLine = new StringBuilder(); + newLine.append(line.substring(0, openIndex)); + String separator = ""; + for (String coord : coords) + { + newLine.append(separator); + separator = ","; + newLine.append((int) (Integer.parseInt(coord.trim()) * finalScale)); + } + newLine.append(line.substring(closeIndex)); + line = newLine.toString(); + } + } + sb.append(line); + sb.append("\n"); + } + } + + // Write the file back to the disk + try (FileOutputStream mapOut = new FileOutputStream(mapFile)) + { + OutputStreamWriter mapWriter = new OutputStreamWriter(mapOut); + mapWriter.write(sb.toString()); + mapWriter.flush(); + } + } + + private static class CacheClearer implements Runnable + { + private final Container _container; + + public CacheClearer(Container container) + { + _container = container; + } + + @Override + public boolean equals(Object o) + { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + CacheClearer that = (CacheClearer) o; + return Objects.equals(_container, that._container); + } + + @Override + public int hashCode() + { + return Objects.hash(_container); + } + + @Override + public void run() + { + clearCache(_container); + } + } + + public static Runnable getCacheClearingCommitTask(Container c) + { + return new CacheClearer(c); + } + + /** + * Clears out the cache of files for this container. Must be called after any operation that changes the way a graph + * would be generated. Typically, this includes deleting or inserting any run in the container, because that + * can change the connections between the runs, which is reflected in the graphs. + */ + public static void clearCache(Container container) + { + Lock deleteLock = LOCK.writeLock(); + deleteLock.lock(); + try + { + FileUtil.deleteDir(getFolderDirectory(container)); + } + catch (IOException e) + { + // Non-fatal + _log.error("Failed to clear cached experiment run graphs for container " + container, e); + } + finally + { + deleteLock.unlock(); + } + } + + private static void testDotPath() throws ExperimentException + { + File dir; + + try + { + dir = getBaseDirectory(); + } + catch (IOException e) + { + throw new ExperimentException(DotRunner.getConfigurationError(e)); + } + + DotRunner.testDotPath(dir); + } + + + private static String getBaseFileName(ExpRun run, boolean detail, String focus) throws IOException + { + String fileName; + if (null != focus) + fileName = getFolderDirectory(run.getContainer()) + File.separator + "run" + run.getRowId() + "Focus" + focus; + else + fileName = getFolderDirectory(run.getContainer()) + File.separator + "run" + run.getRowId() + (detail ? "Detail" : ""); + return fileName; + } + + private static final Cleaner CLEANER = Cleaner.create(); + + private static class FileLockState implements Runnable + { + private final Throwable _allocation; + private Lock _lock; + + private FileLockState(Throwable allocation, Lock lock) + { + _allocation = allocation; + _lock = lock; + } + + @Override + public void run() + { + if (_lock != null) + { + _log.error("Lock was not released. Creation was at:", _allocation); + _lock.unlock(); + _lock = null; + } + } + + private void release() + { + if (_lock != null) + { + _lock.unlock(); + _lock = null; + } + } + } + + /** + * Results for run graph generation. Must be released once the files have been consumed by the caller. + */ + public static class RunGraphFiles + { + private final File _mapFile; + private final File _imageFile; + private final FileLockState _state; + private final Cleaner.Cleanable _cleanable; + + public RunGraphFiles(File mapFile, File imageFile, Lock lock) + { + _mapFile = mapFile; + _imageFile = imageFile; + _state = new FileLockState(new Throwable(), lock); + _cleanable = CLEANER.register(this, _state); + } + + public File getMapFile() + { + return _mapFile; + } + + public File getImageFile() + { + return _imageFile; + } + + /** + * Release the lock on the files. + */ + public void release() + { + _state.release(); + _cleanable.clean(); + } + } +} diff --git a/experiment/src/org/labkey/experiment/XarReader.java b/experiment/src/org/labkey/experiment/XarReader.java index fe38014852d..17909154d0f 100644 --- a/experiment/src/org/labkey/experiment/XarReader.java +++ b/experiment/src/org/labkey/experiment/XarReader.java @@ -449,7 +449,7 @@ private void loadDoc() throws ExperimentException throw new XarFormatException(e); } - ExperimentRunGraph.clearCache(getContainer()); + FileBasedExperimentRunGraph.clearCache(getContainer()); try { diff --git a/experiment/src/org/labkey/experiment/api/ExpRunImpl.java b/experiment/src/org/labkey/experiment/api/ExpRunImpl.java index c55c241e91f..6dc83ea79a1 100644 --- a/experiment/src/org/labkey/experiment/api/ExpRunImpl.java +++ b/experiment/src/org/labkey/experiment/api/ExpRunImpl.java @@ -56,14 +56,13 @@ import org.labkey.api.query.QueryRowReference; import org.labkey.api.security.User; import org.labkey.api.security.permissions.DeletePermission; -import org.labkey.api.security.permissions.SampleWorkflowDeletePermission; import org.labkey.api.security.permissions.UpdatePermission; import org.labkey.api.util.FileUtil; import org.labkey.api.util.NetworkDrive; import org.labkey.api.view.ActionURL; import org.labkey.api.view.UnauthorizedException; import org.labkey.experiment.DotGraph; -import org.labkey.experiment.ExperimentRunGraph; +import org.labkey.experiment.FileBasedExperimentRunGraph; import java.io.File; import java.io.IOException; @@ -564,7 +563,7 @@ public void deleteProtocolApplications(List datasToDelete, User use // Clear the cache in a commit task, which allows us to do a single clear (which is semi-expensive) if multiple // runs are being deleted in the same transaction, like deleting a container - svc.getSchema().getScope().addCommitTask(ExperimentRunGraph.getCacheClearingCommitTask(getContainer()), DbScope.CommitTaskOption.POSTCOMMIT); + svc.getSchema().getScope().addCommitTask(FileBasedExperimentRunGraph.getCacheClearingCommitTask(getContainer()), DbScope.CommitTaskOption.POSTCOMMIT); } @Override diff --git a/experiment/src/org/labkey/experiment/controllers/exp/ExperimentController.java b/experiment/src/org/labkey/experiment/controllers/exp/ExperimentController.java index f2bdbe61ede..7474eebc46f 100644 --- a/experiment/src/org/labkey/experiment/controllers/exp/ExperimentController.java +++ b/experiment/src/org/labkey/experiment/controllers/exp/ExperimentController.java @@ -27,12 +27,13 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.apache.poi.ss.usermodel.Workbook; +import org.graphper.api.GraphResource; +import org.graphper.parser.DotParser; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import org.json.JSONArray; import org.json.JSONException; import org.json.JSONObject; -import org.jspecify.annotations.NonNull; import org.labkey.api.action.ApiJsonWriter; import org.labkey.api.action.ApiResponse; import org.labkey.api.action.ApiSimpleResponse; @@ -266,6 +267,7 @@ import org.labkey.experiment.ExpDataFileListener; import org.labkey.experiment.ExperimentRunDisplayColumn; import org.labkey.experiment.ExperimentRunGraph; +import org.labkey.experiment.FileBasedExperimentRunGraph; import org.labkey.experiment.LineageGraphDisplayColumn; import org.labkey.experiment.MissingFilesCheckInfo; import org.labkey.experiment.MoveRunsBean; @@ -325,6 +327,7 @@ import java.io.InputStream; import java.io.OutputStream; import java.net.URI; +import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; import java.sql.SQLException; @@ -1875,10 +1878,10 @@ public ModelAndView getView(ExperimentRunForm form, BindException errors) throws ExpRunImpl experimentRun = (ExpRunImpl) form.lookupRun(); ensureCorrectContainer(getContainer(), experimentRun, getViewContext()); - ExperimentRunGraph.RunGraphFiles files; + FileBasedExperimentRunGraph.RunGraphFiles files; try { - files = ExperimentRunGraph.generateRunGraph(getViewContext(), experimentRun, detail, focus, focusType); + files = FileBasedExperimentRunGraph.generateRunGraph(getViewContext(), experimentRun, detail, focus, focusType); } catch (ExperimentException e) { @@ -1898,6 +1901,10 @@ public ModelAndView getView(ExperimentRunForm form, BindException errors) throws { files.release(); } + + String dot = ExperimentRunGraph.getDotGraph(getContainer(), experimentRun, detail, focus, focusType); + getViewContext().getResponse().getOutputStream().write(ExperimentRunGraph.getSvg(dot).getBytes(StandardCharsets.UTF_8)); + return null; } diff --git a/experiment/src/org/labkey/experiment/controllers/exp/experimentRunGraphView.jsp b/experiment/src/org/labkey/experiment/controllers/exp/experimentRunGraphView.jsp index 6461ae484ca..640a2656ef7 100644 --- a/experiment/src/org/labkey/experiment/controllers/exp/experimentRunGraphView.jsp +++ b/experiment/src/org/labkey/experiment/controllers/exp/experimentRunGraphView.jsp @@ -16,6 +16,7 @@ */ %> <%@ page import="org.apache.commons.io.IOUtils" %> +<%@ page import="org.graphper.draw.ExecuteException" %> <%@ page import="org.labkey.api.exp.ExperimentException" %> <%@ page import="org.labkey.api.reader.Readers" %> <%@ page import="org.labkey.api.util.UniqueID" %> @@ -24,6 +25,7 @@ <%@ page import="org.labkey.api.view.ViewContext" %> <%@ page import="org.labkey.api.view.template.ClientDependencies" %> <%@ page import="org.labkey.experiment.ExperimentRunGraph" %> +<%@ page import="org.labkey.experiment.FileBasedExperimentRunGraph" %> <%@ page import="org.labkey.experiment.controllers.exp.ExperimentController" %> <%@ page import="org.labkey.experiment.controllers.exp.ExperimentRunGraphModel" %> <%@ page import="java.io.IOException" %> @@ -51,7 +53,7 @@ try { - ExperimentRunGraph.RunGraphFiles files = ExperimentRunGraph.generateRunGraph(context, + FileBasedExperimentRunGraph.RunGraphFiles files = FileBasedExperimentRunGraph.generateRunGraph(context, model.getRun(), model.isDetail(), model.getFocus(), @@ -94,9 +96,17 @@ { files.release(); } + + String dot = ExperimentRunGraph.getDotGraph(getContainer(), + model.getRun(), + model.isDetail(), + model.getFocus(), + model.getFocusType() + ); + out.write(ExperimentRunGraph.getSvg(dot)); } } - catch (ExperimentException | InterruptedException e) + catch (ExperimentException | InterruptedException | ExecuteException e) { %>

<%=h(e.getMessage(), true)%>

<% } diff --git a/experiment/src/org/labkey/experiment/pipeline/ExpGeneratorHelper.java b/experiment/src/org/labkey/experiment/pipeline/ExpGeneratorHelper.java index f31733af934..93aeb97c5bd 100644 --- a/experiment/src/org/labkey/experiment/pipeline/ExpGeneratorHelper.java +++ b/experiment/src/org/labkey/experiment/pipeline/ExpGeneratorHelper.java @@ -51,7 +51,7 @@ import org.labkey.api.query.ValidationException; import org.labkey.api.security.User; import org.labkey.api.util.FileUtil; -import org.labkey.experiment.ExperimentRunGraph; +import org.labkey.experiment.FileBasedExperimentRunGraph; import org.labkey.experiment.api.ExpDataImpl; import org.labkey.experiment.api.ExpMaterialImpl; import org.labkey.experiment.api.ExpRunImpl; @@ -274,7 +274,7 @@ static public ExpRunImpl insertRun(Container container, User user, throw new ExperimentException(e); } - ExperimentRunGraph.clearCache(run.getContainer()); + FileBasedExperimentRunGraph.clearCache(run.getContainer()); } catch (XarFormatException | BatchValidationException e) { From 61ba07ffc5803e9e2dabbf1df17cf1d4abe8c1c4 Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Sat, 11 Apr 2026 04:21:06 -0700 Subject: [PATCH 04/13] SvgUtil to read and set SVG sizes. Use it to scale experiment diagrams. --- api/src/org/labkey/api/ApiModule.java | 2 + .../org/labkey/api/attachments/SvgSource.java | 16 +- api/src/org/labkey/api/util/SvgUtil.java | 176 ++++++++++++++++++ .../labkey/experiment/ExperimentRunGraph.java | 5 +- 4 files changed, 188 insertions(+), 11 deletions(-) create mode 100644 api/src/org/labkey/api/util/SvgUtil.java diff --git a/api/src/org/labkey/api/ApiModule.java b/api/src/org/labkey/api/ApiModule.java index d613909b2b6..2d85ff99ba8 100644 --- a/api/src/org/labkey/api/ApiModule.java +++ b/api/src/org/labkey/api/ApiModule.java @@ -172,6 +172,7 @@ import org.labkey.api.util.SessionHelper; import org.labkey.api.util.StringExpressionFactory; import org.labkey.api.util.StringUtilsLabKey; +import org.labkey.api.util.SvgUtil; import org.labkey.api.util.SystemMaintenance; import org.labkey.api.util.SystemMaintenanceStartupListener; import org.labkey.api.util.URIUtil; @@ -439,6 +440,7 @@ public void registerServlets(ServletContext servletCtx) StringExpressionFactory.TestCase.class, StringUtilsLabKey.TestCase.class, SubfolderWriter.TestCase.class, + SvgUtil.TestCase.class, SwapQueue.TestCase.class, TSVMapWriter.Tests.class, TSVWriter.TestCase.class, diff --git a/api/src/org/labkey/api/attachments/SvgSource.java b/api/src/org/labkey/api/attachments/SvgSource.java index bf7c068a45d..388c5ae2d3d 100644 --- a/api/src/org/labkey/api/attachments/SvgSource.java +++ b/api/src/org/labkey/api/attachments/SvgSource.java @@ -5,6 +5,8 @@ import org.apache.commons.lang3.StringUtils; import org.apache.commons.lang3.Strings; import org.labkey.api.util.PageFlowUtil; +import org.labkey.api.util.SvgUtil; +import org.labkey.api.util.SvgUtil.Size; import org.labkey.api.view.NotFoundException; import java.io.BufferedReader; @@ -19,8 +21,7 @@ public class SvgSource { private final String _filteredSvg; - - private Float _height = null; + private final Float _height; public SvgSource(String svg) { @@ -32,16 +33,11 @@ public SvgSource(String svg) if (!svg.contains("xmlns=\"" + SVGDOMImplementation.SVG_NAMESPACE_URI + "\"") && !svg.contains("xmlns='" + SVGDOMImplementation.SVG_NAMESPACE_URI + "'")) svg = svg.replace("drt/CAexample_mini(DRT2)CAexample_mini.mzXMLBovine_mini1.fastaBovine_mini2.fastaBovine_mini3.fastaTandem SettingsScored SearchResultsProtein Prophetscoresdrt/CAexample_mini + (DRT2)->Scored Search + Resultsdrt/CAexample_mini + (DRT2)->Protein Prophet + scoresCAexample_mini.mzXML->drt/CAexample_mini + (DRT2)mzXMLBovine_mini1.fasta->drt/CAexample_mini + (DRT2)FASTABovine_mini2.fasta->drt/CAexample_mini + (DRT2)FASTABovine_mini3.fasta->drt/CAexample_mini + (DRT2)FASTATandem Settings->drt/CAexample_mini + (DRT2)SearchConfig"""; + + public static class TestCase extends Assert + { + @Test + public void testReadAttributes() + { + Size height = readHeight(svg); + Assert.assertNotNull(height); + Assert.assertEquals(785.3512f, height.value(), 0.0001f); + Assert.assertEquals("pt", height.units()); + + Size newHeight = new Size(123.4567f, "px"); + Assert.assertEquals(readHeight(setHeight(svg, newHeight)), newHeight); + + Size width = readWidth(svg); + Assert.assertNotNull(width); + Assert.assertEquals(1776.9770f, width.value(), 0.0001f); + Assert.assertEquals("pt", width.units()); + + Size newWidth = new Size(456.7890f, "px"); + Assert.assertEquals(readWidth(setWidth(svg, newWidth)), newWidth); + + // Now clear height and width, then test read and set scenarios when these attributes are missing + + String svgNoSize = setWidth(setHeight(svg, null), null); + Assert.assertNull(readHeight(svgNoSize)); + Assert.assertNull(readWidth(svgNoSize)); + + String changedSvg = setHeight(svgNoSize, new Size(375.1234f, "px")); + height = readHeight(changedSvg); + Assert.assertNotNull(height); + Assert.assertEquals(375.1234f, height.value(), 0.0001f); + Assert.assertEquals("px", height.units()); + + changedSvg = setWidth(svgNoSize, new Size(1023.9876f, "px")); + width = readWidth(changedSvg); + Assert.assertNotNull(width); + Assert.assertEquals(1023.9876f, width.value(), 0.0001f); + Assert.assertEquals("px", width.units()); + } + } +} diff --git a/experiment/src/org/labkey/experiment/ExperimentRunGraph.java b/experiment/src/org/labkey/experiment/ExperimentRunGraph.java index d85bf75a665..0ffd04f79a7 100644 --- a/experiment/src/org/labkey/experiment/ExperimentRunGraph.java +++ b/experiment/src/org/labkey/experiment/ExperimentRunGraph.java @@ -11,6 +11,7 @@ import org.labkey.api.exp.api.ExpProtocolApplication; import org.labkey.api.exp.api.ExpRun; import org.labkey.api.exp.api.ExpRunItem; +import org.labkey.api.util.SvgUtil; import org.labkey.experiment.api.ExpDataImpl; import org.labkey.experiment.api.ExpMaterialImpl; import org.labkey.experiment.api.ExpProtocolApplicationImpl; @@ -38,7 +39,9 @@ public static String getSvg(String dot) throws ExecuteException Graphviz graph = DotParser.parse(dot); String svg = graph.toSvgStr(); - return svg; + // Scale down to 50% of default size. This is arbitrary but seems reasonable. Diagrams are larger than + // the previous image-based ones, but monitors are much higher resolution than when those were scaled. + return SvgUtil.scaleSize(svg, 0.5f); } public static String getDotGraph(Container c, ExpRunImpl run, boolean detail, String focus, String focusType) From 3bb1cdf58f9ce2a93a16d2129d7a2bec73e506cc Mon Sep 17 00:00:00 2001 From: labkey-jeckels Date: Sat, 11 Apr 2026 09:50:25 -0600 Subject: [PATCH 05/13] Fix NPE --- experiment/src/org/labkey/experiment/api/ExpMaterialImpl.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/experiment/src/org/labkey/experiment/api/ExpMaterialImpl.java b/experiment/src/org/labkey/experiment/api/ExpMaterialImpl.java index e091339cb5b..645d0b3731b 100644 --- a/experiment/src/org/labkey/experiment/api/ExpMaterialImpl.java +++ b/experiment/src/org/labkey/experiment/api/ExpMaterialImpl.java @@ -647,7 +647,7 @@ else if (values.containsKey(dp.getPropertyURI())) } for (var entry : values.entrySet()) { - PropertyDescriptor pd = OntologyManager.getPropertyDescriptor(entry.getKey(), st.getContainer()); + PropertyDescriptor pd = OntologyManager.getPropertyDescriptor(entry.getKey(), st == null ? getContainer() : st.getContainer()); if (null != pd) super.setProperty(user, pd, entry.getValue(), insertNullValues); } From 417b122e2c59fadcc3022f41d7d92d1f9bc9e15b Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Sun, 12 Apr 2026 11:18:00 -0700 Subject: [PATCH 06/13] Code review feedback: Support % and other non-two letter units. Don't assume there are trailing spaces. --- api/src/org/labkey/api/util/SvgUtil.java | 62 +++++++++++++++---- .../src/org/labkey/experiment/DotGraph.java | 42 ++++++------- 2 files changed, 68 insertions(+), 36 deletions(-) diff --git a/api/src/org/labkey/api/util/SvgUtil.java b/api/src/org/labkey/api/util/SvgUtil.java index 6b7545ed63a..492392878c1 100644 --- a/api/src/org/labkey/api/util/SvgUtil.java +++ b/api/src/org/labkey/api/util/SvgUtil.java @@ -6,6 +6,9 @@ import org.junit.Assert; import org.junit.Test; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + public class SvgUtil { public record Size(Float value, @Nullable String units) @@ -32,19 +35,23 @@ public record Size(Float value, @Nullable String units) return ret; } + private static final Pattern floatingPointPattern = Pattern.compile("(?(\\d*\\.)?\\d+)(?[^\\d.]*)"); + public static @Nullable Size readSizeAttribute(String svg, String attribute) { Size ret = null; String value = readAttribute(svg, attribute); if (value != null) { - String units = value.substring(value.length() - 2); - if (StringUtils.isNumeric(units)) - units = null; + Matcher fbMatcher = floatingPointPattern.matcher(value); + if (fbMatcher.find()) + { + ret = new Size(Float.parseFloat(fbMatcher.group("value")), StringUtils.trimToNull(fbMatcher.group("units"))); + } else - value = value.substring(0, value.length() - 2); - - ret = new Size(Float.parseFloat(value), units); + { + throw new IllegalStateException("Couldn't match value " + value + " into a valid size pattern"); + } } return ret; @@ -95,12 +102,12 @@ private static String setSizeAttribute(String svg, String attribute, @Nullable S { if (size != null) { - int svgElement = svg.indexOf("drt/CAexample_mini(DRT2)CAexample_mini.mzXMLBovine_mini1.fastaBovine_mini2.fastaBovine_mini3.fastaTandem SettingsScored SearchResultsProtein Prophetscoresdrt/CAexample_mini + + drt/CAexample_mini(DRT2)CAexample_mini.mzXMLBovine_mini1.fastaBovine_mini2.fastaBovine_mini3.fastaTandem SettingsScored SearchResultsProtein Prophetscoresdrt/CAexample_mini (DRT2)->Scored Search Resultsdrt/CAexample_mini (DRT2)->Protein Prophet @@ -131,12 +139,13 @@ private static String setSizeAttribute(String svg, String attribute, @Nullable S (DRT2)FASTABovine_mini2.fasta->drt/CAexample_mini (DRT2)FASTABovine_mini3.fasta->drt/CAexample_mini (DRT2)FASTATandem Settings->drt/CAexample_mini - (DRT2)SearchConfig"""; + (DRT2)SearchConfig + """; public static class TestCase extends Assert { @Test - public void testReadAttributes() + public void testAttributes() { Size height = readHeight(svg); Assert.assertNotNull(height); @@ -172,5 +181,34 @@ public void testReadAttributes() Assert.assertEquals(1023.9876f, width.value(), 0.0001f); Assert.assertEquals("px", width.units()); } + + @Test + public void testCornerCases() + { + String svg = ""; + assertEquals(new Size(50f, "%"), readWidth(svg)); + assertEquals(new Size(75f, "%"), readHeight(svg)); + assertEquals("", setWidth(svg, new Size(83f, "%"))); + assertEquals("", setHeight(svg, new Size(47f, "%"))); + + svg = ""; + assertNull(readWidth(svg)); + Size size = readHeight(svg); + assertNotNull(size); + assertEquals(new Size(100f, null), size); + assertEquals("", setHeight(svg, new Size(200f, "px"))); + assertEquals("", setHeight(svg, null)); + + svg = ""; + assertNull(readHeight(svg)); + size = readWidth(svg); + assertNotNull(size); + assertEquals(new Size(200f, null), size); + assertEquals("", setWidth(svg, new Size(300f, "px"))); + assertEquals("", setWidth(svg, null)); + + assertEquals("", setWidth("", null)); + assertEquals("", setHeight("", null)); + } } } diff --git a/experiment/src/org/labkey/experiment/DotGraph.java b/experiment/src/org/labkey/experiment/DotGraph.java index 61b52548d35..6e18db7e8f6 100644 --- a/experiment/src/org/labkey/experiment/DotGraph.java +++ b/experiment/src/org/labkey/experiment/DotGraph.java @@ -35,8 +35,6 @@ /** * Represents the GraphViz format for building an experiment run "flowchart", connecting datas, materials through runs * and protocol applications. - * User: migra - * Date: Jun 13, 2005 */ public class DotGraph { @@ -79,17 +77,13 @@ public DotGraph(PrintWriter out, Container c, boolean bSmallFonts) { _pwOut = out; _c = c; - - if (bSmallFonts) - _pwOut.println("digraph G { node[fontname=\"" + LABEL_FONT + "\" fontsize=" + LABEL_SMALL_FONTSIZE + "]"); - else - _pwOut.println("digraph G { node[fontname=\"" + LABEL_FONT + "\" fontsize=" + LABEL_DEFAULT_FONTSIZE + "]"); + _pwOut.println("digraph G { margin=\"0,0\" node[fontname=\"" + LABEL_FONT + "\" fontsize=" + (bSmallFonts ? LABEL_SMALL_FONTSIZE : LABEL_DEFAULT_FONTSIZE) + "]"); } - public void setFocus(Integer focusid, String objtype) + public void setFocus(Integer focusId, String objType) { - _focusId = focusid; - _objectType = objtype; + _focusId = focusId; + _objectType = objType; } public void dispose() @@ -305,9 +299,9 @@ else if ((null != writtenTrgtMap) && writtenTrgtMap.containsKey(trgtRow)) { if (null == trgt._shape && src != null) // it's an output node, drawn just as an arrow to a label { - String outnodekey = src._key + "out"; - connect += outnodekey + " [arrowhead = diamond] "; - connect += "\n" + outnodekey + "[shape=plaintext label=\"Output\"]"; + String outNodeKey = src._key + "out"; + connect += outNodeKey + " [arrowhead = diamond] "; + connect += "\n" + outNodeKey + "[shape=plaintext label=\"Output\"]"; } else connect += trgt._key + "[arrowsize = 2]"; @@ -320,7 +314,7 @@ else if ((null != writtenTrgtMap) && writtenTrgtMap.containsKey(trgtRow)) { connect += " [ style=\"setlinewidth(3)\" ]"; } - if (!_writtenConnects.contains(connect) && !_pendingConnects.contains(connect)) + if (!_writtenConnects.contains(connect)) _pendingConnects.add(connect); } @@ -575,10 +569,10 @@ public GroupedNode(Long groupId, Integer actionseq, DotNode node) _nodeType = node._type; } - public void addNode(DotNode newnode) + public void addNode(DotNode newNode) { - assert (Objects.equals(_gMap.get(_gMap.firstKey())._type, newnode._type)); - _gMap.put(newnode._id, newnode); + assert (Objects.equals(_gMap.get(_gMap.firstKey())._type, newNode._type)); + _gMap.put(newNode._id, newNode); } @Override @@ -596,17 +590,17 @@ public void save(PrintWriter out) url.addParameter("rowId~in", sbIn.toString()); - _label += " (" + _gMap.keySet().size() + " entries)"; + _label += " (" + _gMap.size() + " entries)"; if (null != _shape) { out.println(_key + "[label=\"" + escape(_label) - + "\",style=\"filled\", fillcolor=\"" + _color + "\" shape=" + _shape - + ((null != _height) ? ", height=\"" + _height + "\"" : "") - + ((null != _width) ? ", width=\"" + _width + "\"" : "") - + ((null != _width) || (null != _height) ? ", fixedsize=true" : "") - + (", URL=\"" + escape(url.toString()) + "\"") - + "]"); + + "\",style=\"filled\", fillcolor=\"" + _color + "\" shape=" + _shape + + ((null != _height) ? ", height=\"" + _height + "\"" : "") + + ((null != _width) ? ", width=\"" + _width + "\"" : "") + + ((null != _width) || (null != _height) ? ", fixedsize=true" : "") + + (", URL=\"" + escape(url.toString()) + "\"") + + "]"); } } } From 66d1022ef640abd26d74e4c963c99be596b25d0c Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Mon, 13 Apr 2026 04:38:05 -0700 Subject: [PATCH 07/13] Remove all old graphviz code --- api/src/org/labkey/api/util/DotRunner.java | 200 --------- .../src/org/labkey/experiment/DotGraph.java | 42 +- .../labkey/experiment/ExperimentRunGraph.java | 14 +- .../FileBasedExperimentRunGraph.java | 392 ------------------ .../src/org/labkey/experiment/XarReader.java | 70 +++- .../org/labkey/experiment/api/ExpRunImpl.java | 9 +- .../controllers/exp/ExperimentController.java | 79 +--- .../exp/experimentRunGraphView.jsp | 74 +--- .../pipeline/ExpGeneratorHelper.java | 7 +- .../pipeline/analysis/AnalysisController.java | 40 +- 10 files changed, 115 insertions(+), 812 deletions(-) delete mode 100644 api/src/org/labkey/api/util/DotRunner.java delete mode 100644 experiment/src/org/labkey/experiment/FileBasedExperimentRunGraph.java diff --git a/api/src/org/labkey/api/util/DotRunner.java b/api/src/org/labkey/api/util/DotRunner.java deleted file mode 100644 index b975a34b2b7..00000000000 --- a/api/src/org/labkey/api/util/DotRunner.java +++ /dev/null @@ -1,200 +0,0 @@ -/* - * Copyright (c) 2013-2019 LabKey Corporation - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package org.labkey.api.util; - -import org.apache.commons.lang3.StringUtils; -import org.labkey.api.exp.ExperimentException; -import org.labkey.api.miniprofiler.CustomTiming; -import org.labkey.api.miniprofiler.MiniProfiler; - -import java.io.BufferedReader; -import java.io.File; -import java.io.IOException; -import java.io.InputStreamReader; -import java.io.PrintWriter; -import java.util.LinkedList; -import java.util.List; - -/** - * User: adam - * Date: 11/13/11 - * Time: 10:33 AM - */ - -// Handles the basics of testing the dot configuration, creating the dot command line, and invoking dot. This code -// originated in ExperimentRunGraph, but was pulled out to support group diagrams. -public class DotRunner -{ - private static final String dotExePath = "dot"; - - private final String _dotInput; - private final File _directory; - private final List _parameters = new LinkedList<>(); - - public DotRunner(File directory, String dotInput) - { - _directory = directory; - _dotInput = dotInput; - _parameters.add(dotExePath); - } - - public void addPngOutput(File pngFile) - { - addOutputParameter("png", pngFile); - } - - public void addCmapOutput(File cmapFile) - { - addOutputParameter("cmap", cmapFile); - } - - public void addSvgOutput(File svgFile) - { - addOutputParameter("svg", svgFile); - } - - private void addOutputParameter(String typeName, File file) - { - _parameters.add("-T" + typeName); - _parameters.add("-o" + file.getName()); - } - - public void execute() throws IOException, InterruptedException - { - ProcessBuilder pb = new ProcessBuilder(_parameters); - pb.directory(_directory); - ProcessResult result = executeProcess(pb, _dotInput); - - if (result._returnCode != 0) - { - throw new IOException("Graph generation failed with error code " + result._returnCode + ":\n" + result._output); - } - } - - public static void testDotPath(File baseDirectory) throws ExperimentException - { - try - { - File testVersion = FileUtil.appendName(baseDirectory, "dottest.txt"); - if (testVersion.exists()) - return; - - ProcessBuilder pb = new ProcessBuilder(dotExePath, "-V", "-o" + testVersion.getName()); - pb = pb.directory(baseDirectory); - pb.redirectErrorStream(true); - Process p = pb.start(); - int err = p.waitFor(); - - if (err != 0) - { - BufferedReader reader = new BufferedReader(new InputStreamReader(p.getInputStream())); - StringBuilder sb = new StringBuilder(); - String line; - while ((line = reader.readLine()) != null) - { - sb.append(line); - sb.append("\n"); - } - sb.append("(Exit code: "); - sb.append(err); - sb.append(")"); - throw new ExperimentException(getConfigurationError(sb.toString())); - } - } - catch (IOException e) - { - throw new ExperimentException(getConfigurationError(e)); - } - catch (InterruptedException e) - { - throw new ExperimentException("InterruptedException - web server may be shutting down"); - } - } - - public static String getConfigurationError(IOException e) - { - if (e.getMessage() != null) - { - return getConfigurationError(e.getMessage()); - } - else - { - return getConfigurationError(e.toString()); - } - } - - public static String getConfigurationError(String message) - { - StringBuilder sb = new StringBuilder(); - sb.append("Unable to display graph view: cannot run "); - sb.append(dotExePath); - sb.append(" due to an error.\n\n"); - sb.append(message); - sb.append("\n\n"); - sb.append("Regardless of the specific error message, please first verify that the program '" + dotExePath + "' is on your system PATH"); - if (!StringUtils.isEmpty(System.getenv("PATH"))) - { - sb.append(" ("); - sb.append(System.getenv("PATH")); - sb.append(")"); - } - sb.append(".\n\n"); - sb.append("For help on fixing your system configuration, please consult the Graphviz section of the LabKey Server documentation on third party components."); - - return sb.toString(); - } - - private static ProcessResult executeProcess(ProcessBuilder pb, String stdIn) throws IOException, InterruptedException - { - try (CustomTiming t = MiniProfiler.custom("exec", StringUtils.join(pb.command(), " "))) - { - StringBuilder sb = new StringBuilder(); - pb.redirectErrorStream(true); - Process p = pb.start(); - - try (PrintWriter writer = new PrintWriter(p.getOutputStream())) - { - writer.write(stdIn); - } - - try (BufferedReader procReader = new BufferedReader(new InputStreamReader(p.getInputStream()))) - { - String line; - - while ((line = procReader.readLine()) != null) - { - sb.append(line); - sb.append("\n"); - } - } - - int returnCode = p.waitFor(); - return new ProcessResult(returnCode, sb.toString()); - } - } - - private static class ProcessResult - { - private final int _returnCode; - private final String _output; - - public ProcessResult(int returnCode, String output) - { - _returnCode = returnCode; - _output = output; - } - } -} diff --git a/experiment/src/org/labkey/experiment/DotGraph.java b/experiment/src/org/labkey/experiment/DotGraph.java index 6e18db7e8f6..691fed0dc17 100644 --- a/experiment/src/org/labkey/experiment/DotGraph.java +++ b/experiment/src/org/labkey/experiment/DotGraph.java @@ -122,7 +122,7 @@ else if (writtenNodes.containsKey(rowId)) return null; } - public void addStartingMaterial(ExpMaterial m, Long groupId, Integer actionseq, long runId) + public void addStartingMaterial(ExpMaterial m, Long groupId, Integer actionSeq, long runId) { DotNode node = new MNode(m); node.setLinkURL(ExperimentController.getResolveLsidURL(_c, "material", m.getLSID())); @@ -130,13 +130,13 @@ public void addStartingMaterial(ExpMaterial m, Long groupId, Integer actionseq, node.setFocus(true); if (null != groupId) { - node = addNodeToGroup(node, groupId, actionseq, _groupMNodes); + node = addNodeToGroup(node, groupId, actionSeq, _groupMNodes); node.setLinkURL(ExperimentController.getShowGraphMoreListURL(_c, runId, TYPECODE_MATERIAL)); } _pendingMNodes.put(m.getRowId(), node); } - public void addStartingData(ExpData d, Long groupId, Integer actionseq, long runId) + public void addStartingData(ExpData d, Long groupId, Integer actionSeq, long runId) { DotNode node = new DNode(d); node.setLinkURL(ExperimentController.getResolveLsidURL(_c, "data", d.getLSID())); @@ -145,13 +145,13 @@ public void addStartingData(ExpData d, Long groupId, Integer actionseq, long run node.setFocus(true); if (null != groupId) { - node = addNodeToGroup(node, groupId, actionseq, _groupDNodes); + node = addNodeToGroup(node, groupId, actionSeq, _groupDNodes); node.setLinkURL(ExperimentController.getShowGraphMoreListURL(_c, runId, TYPECODE_DATA)); } _pendingDNodes.put(d.getRowId(), node); } - private GroupedNode addNodeToGroup(DotNode node, Long groupId, Integer actionseq, Map groupNodes) + private GroupedNode addNodeToGroup(DotNode node, Long groupId, Integer actionSeq, Map groupNodes) { GroupedNode gnode; if (groupNodes.containsKey(groupId)) @@ -161,14 +161,14 @@ private GroupedNode addNodeToGroup(DotNode node, Long groupId, Integer actionseq } else { - if (null == actionseq) actionseq = 0; - gnode = new GroupedNode(groupId, actionseq, node); + if (null == actionSeq) actionSeq = 0; + gnode = new GroupedNode(groupId, actionSeq, node); groupNodes.put(groupId, gnode); } return gnode; } - public void addMaterial(ExpMaterial m, Long groupId, Integer actionseq, boolean output) + public void addMaterial(ExpMaterial m, Long groupId, Integer actionSeq, boolean output) { if (_writtenMNodes.containsKey(m.getRowId()) || _pendingMNodes.containsKey(m.getRowId())) return; @@ -180,11 +180,11 @@ public void addMaterial(ExpMaterial m, Long groupId, Integer actionseq, boolean if (null != _focusId && TYPECODE_MATERIAL.equalsIgnoreCase(_objectType) && _focusId == m.getRowId()) node.setFocus(true); if (null != groupId) - node = addNodeToGroup(node, groupId, actionseq, _groupMNodes); + node = addNodeToGroup(node, groupId, actionSeq, _groupMNodes); _pendingMNodes.put(m.getRowId(), node); } - public void addData(ExpData d, Long groupId, Integer actionseq, boolean output) + public void addData(ExpData d, Long groupId, Integer actionSeq, boolean output) { if (_writtenDNodes.containsKey(d.getRowId()) || _pendingDNodes.containsKey(d.getRowId())) return; @@ -196,11 +196,11 @@ public void addData(ExpData d, Long groupId, Integer actionseq, boolean output) if (null != _focusId && TYPECODE_DATA.equalsIgnoreCase(_objectType) && _focusId == d.getRowId()) node.setFocus(true); if (null != groupId) - node = addNodeToGroup(node, groupId, actionseq, _groupDNodes); + node = addNodeToGroup(node, groupId, actionSeq, _groupDNodes); _pendingDNodes.put(d.getRowId(), node); } - public void addProtApp(Long groupId, long rowId, String name, Integer actionseq) + public void addProtApp(Long groupId, long rowId, String name, Integer actionSeq) { if (_writtenProcNodes.containsKey(rowId) || _pendingProcNodes.containsKey(rowId)) return; @@ -208,17 +208,17 @@ public void addProtApp(Long groupId, long rowId, String name, Integer actionseq) if (null != _focusId && TYPECODE_PROT_APP.equalsIgnoreCase(_objectType) && _focusId == rowId) node.setFocus(true); if (null != groupId) - node = addNodeToGroup(node, groupId, actionseq, _groupPANodes); + node = addNodeToGroup(node, groupId, actionSeq, _groupPANodes); _pendingProcNodes.put(rowId, node); } - public void addOutputNode(Long groupId, long rowId, String name, Integer actionseq) + public void addOutputNode(Long groupId, long rowId, String name, Integer actionSeq) { if (_writtenProcNodes.containsKey(rowId) || _pendingProcNodes.containsKey(rowId)) return; DotNode node = new OutputNode(rowId, name); if (null != groupId) - node = addNodeToGroup(node, groupId, actionseq, _groupPANodes); + node = addNodeToGroup(node, groupId, actionSeq, _groupPANodes); _pendingProcNodes.put(rowId, node); } @@ -352,11 +352,11 @@ private void writePending(Map pendingMap, Map writ node.save(_pwOut); if (node instanceof GroupedNode) { - for (Long memberkey : ((GroupedNode) node)._gMap.keySet()) + for (Long memberKey : ((GroupedNode) node)._gMap.keySet()) { - assert (pendingMap.containsKey(memberkey)); - writtenMap.put(memberkey, node); - nodesToMove.add(memberkey); + assert (pendingMap.containsKey(memberKey)); + writtenMap.put(memberKey, node); + nodesToMove.add(memberKey); } } else @@ -558,9 +558,9 @@ private class GroupedNode extends DotNode private final SortedMap _gMap = new TreeMap<>(); private final String _nodeType; - public GroupedNode(Long groupId, Integer actionseq, DotNode node) + public GroupedNode(Long groupId, Integer actionSeq, DotNode node) { - super(GROUP_ID_PREFIX + actionseq + node._type, node._id, "More... "); + super(GROUP_ID_PREFIX + actionSeq + node._type, node._id, "More... "); _gid = groupId; _gMap.put(node._id, node); //setShape(node.shape, node.color + GROUP_OPACITY); diff --git a/experiment/src/org/labkey/experiment/ExperimentRunGraph.java b/experiment/src/org/labkey/experiment/ExperimentRunGraph.java index 0ffd04f79a7..c880e8ae6b4 100644 --- a/experiment/src/org/labkey/experiment/ExperimentRunGraph.java +++ b/experiment/src/org/labkey/experiment/ExperimentRunGraph.java @@ -17,8 +17,10 @@ import org.labkey.experiment.api.ExpProtocolApplicationImpl; import org.labkey.experiment.api.ExpRunImpl; +import java.io.IOException; import java.io.PrintWriter; import java.io.StringWriter; +import java.io.Writer; import java.util.ArrayList; import java.util.Comparator; import java.util.HashMap; @@ -34,17 +36,23 @@ public class ExperimentRunGraph private static final int MAX_SIBLINGS = 5; private static final int MIN_SIBLINGS = 3; - public static String getSvg(String dot) throws ExecuteException + public static void renderSvg(Writer out, Container c, ExpRunImpl run, boolean detail, String focus, String focusType) throws ExecuteException, IOException + { + String dot = getDotGraph(c, run, detail, focus, focusType); + out.write(getSvg(dot)); + } + + private static String getSvg(String dot) throws ExecuteException { Graphviz graph = DotParser.parse(dot); String svg = graph.toSvgStr(); // Scale down to 50% of default size. This is arbitrary but seems reasonable. Diagrams are larger than - // the previous image-based ones, but monitors are much higher resolution than when those were scaled. + // the old image-based ones, but monitors are much higher resolution than when those were scaled. return SvgUtil.scaleSize(svg, 0.5f); } - public static String getDotGraph(Container c, ExpRunImpl run, boolean detail, String focus, String focusType) + private static String getDotGraph(Container c, ExpRunImpl run, boolean detail, String focus, String focusType) { Integer focusId = null; String typeCode = focusType; diff --git a/experiment/src/org/labkey/experiment/FileBasedExperimentRunGraph.java b/experiment/src/org/labkey/experiment/FileBasedExperimentRunGraph.java deleted file mode 100644 index 793a070f7a5..00000000000 --- a/experiment/src/org/labkey/experiment/FileBasedExperimentRunGraph.java +++ /dev/null @@ -1,392 +0,0 @@ -/* - * Copyright (c) 2008-2019 LabKey Corporation - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package org.labkey.experiment; - -import org.apache.logging.log4j.LogManager; -import org.apache.logging.log4j.Logger; -import org.labkey.api.data.Container; -import org.labkey.api.exp.ExperimentException; -import org.labkey.api.exp.api.ExpRun; -import org.labkey.api.settings.AppProps; -import org.labkey.api.util.ConfigurationException; -import org.labkey.api.util.DotRunner; -import org.labkey.api.util.FileUtil; -import org.labkey.api.util.ImageUtil; -import org.labkey.api.view.ViewContext; -import org.labkey.experiment.api.ExpRunImpl; - -import javax.imageio.ImageIO; -import java.awt.image.BufferedImage; -import java.io.BufferedReader; -import java.io.File; -import java.io.FileInputStream; -import java.io.FileOutputStream; -import java.io.IOException; -import java.io.InputStreamReader; -import java.io.OutputStreamWriter; -import java.lang.ref.Cleaner; -import java.util.Objects; -import java.util.concurrent.locks.Lock; -import java.util.concurrent.locks.ReentrantReadWriteLock; - -// TODO: This entire class can be eliminated once we're happy with the new graph-support Java implementation -public class FileBasedExperimentRunGraph extends ExperimentRunGraph -{ - private static File baseDirectory; - private static final Logger _log = LogManager.getLogger(FileBasedExperimentRunGraph.class); - - /** - * It's safe for lots of threads to be reading but only one should be creating or deleting at a time. - * We could make separate locks for each folder but leaving it with one global lock for now. - */ - private static final ReentrantReadWriteLock LOCK = new ReentrantReadWriteLock(); - - public synchronized static File getBaseDirectory() throws IOException - { - if (baseDirectory == null) - { - File tempDir = FileUtil.appendName(FileUtil.getTempDirectory(), "ExperimentRunGraphs"); - if (tempDir.exists()) - { - FileUtil.deleteDirectoryContents(tempDir); - } - else - { - if (!FileUtil.mkdirs(tempDir)) - { - throw new IOException("Unable to create temporary directory for experiment run graphs: " + tempDir.getPath()); - } - } - baseDirectory = tempDir; - } - return baseDirectory; - } - - private synchronized static File getFolderDirectory(Container container) throws IOException - { - File result = FileUtil.appendName(getBaseDirectory(), "Folder" + container.getRowId()); - FileUtil.mkdirs(result); - for (int i = 0; i < 5; i++) - { - if (result.isDirectory()) - { - return result; - } - else - { - try - { - Thread.sleep(1); - } - catch (InterruptedException e) {} - FileUtil.mkdirs(result); - } - } - if (!result.isDirectory()) - { - throw new IOException("Failed to create directory " + result); - } - return result; - } - - /** - * Creates a run graph, given the configuration parameters. Note that this creates a lock on the directory - * that contains the files, which must be cleared by calling release() on the resulting RunGraphFiles object. - */ - public static RunGraphFiles generateRunGraph(ViewContext ctx, ExpRunImpl run, boolean detail, String focus, String focusType) throws ExperimentException, IOException, InterruptedException - { - boolean success = false; - - File imageFile = new File(getBaseFileName(run, detail, focus) + ".png"); - File mapFile = new File(getBaseFileName(run, detail, focus) + ".map"); - - // First acquire a read lock so we know that another thread won't be deleting these files out from under us - Lock readLock = LOCK.readLock(); - readLock.lock(); - - try - { - if (!AppProps.getInstance().isDevMode() && imageFile.exists() && mapFile.exists()) - { - success = true; - return new RunGraphFiles(mapFile, imageFile, readLock); - } - } - finally - { - // If we found useful files, don't release the lock because the caller will want to read them - if (!success) - { - readLock.unlock(); - } - } - - // We need to create files to open up a write lock - Lock writeLock = LOCK.writeLock(); - writeLock.lock(); - - try - { - testDotPath(); - - String dotInput = getDotGraph(ctx.getContainer(), run, detail, focus, focusType); - DotRunner runner = new DotRunner(getFolderDirectory(run.getContainer()), dotInput); - runner.addCmapOutput(mapFile); - runner.addPngOutput(imageFile); - runner.execute(); - - mapFile.deleteOnExit(); - imageFile.deleteOnExit(); - - BufferedImage originalImage = ImageIO.read(imageFile); - if (originalImage == null) - { - throw new IOException("Unable to read image file " + imageFile.getAbsolutePath() + " of size " + imageFile.length() + " - disk may be full?"); - } - - try (FileOutputStream fOut = new FileOutputStream(imageFile)) - { - // Write it back out to disk - double scale = ImageUtil.resizeImage(originalImage, fOut, .85, 6, BufferedImage.TYPE_INT_RGB); - - // Need to rewrite the image map to change the coordinates according to the scaling factor - resizeImageMap(mapFile, scale); - } - - // Start the procedure of downgrade our lock from write to read so that the caller can use the files - readLock.lock(); - return new RunGraphFiles(mapFile, imageFile, readLock); - } - catch (UnsatisfiedLinkError | NoClassDefFoundError e) - { - throw new ConfigurationException("Unable to resize image, likely a problem with missing Java Runtime libraries not being available", e); - } - finally - { - writeLock.unlock(); - } - } - - /** - * Shrink all the coordinates in an image map by a fixed ratio - */ - private static void resizeImageMap(File mapFile, double finalScale) throws IOException - { - StringBuilder sb = new StringBuilder(); - - try (FileInputStream fIn = new FileInputStream(mapFile)) - { - // Read in the original file, line by line - BufferedReader reader = new BufferedReader(new InputStreamReader(fIn)); - String line; - while ((line = reader.readLine()) != null) - { - int coordsIndex = line.indexOf("coords=\""); - if (coordsIndex != -1) - { - int openIndex = coordsIndex + "coords=\"".length(); - int closeIndex = line.indexOf("\"", openIndex); - if (closeIndex != -1) - { - // Parse and scale the coordinates - String coordsOriginal = line.substring(openIndex, closeIndex); - String[] coords = coordsOriginal.split(",|(\\s)"); - StringBuilder newLine = new StringBuilder(); - newLine.append(line.substring(0, openIndex)); - String separator = ""; - for (String coord : coords) - { - newLine.append(separator); - separator = ","; - newLine.append((int) (Integer.parseInt(coord.trim()) * finalScale)); - } - newLine.append(line.substring(closeIndex)); - line = newLine.toString(); - } - } - sb.append(line); - sb.append("\n"); - } - } - - // Write the file back to the disk - try (FileOutputStream mapOut = new FileOutputStream(mapFile)) - { - OutputStreamWriter mapWriter = new OutputStreamWriter(mapOut); - mapWriter.write(sb.toString()); - mapWriter.flush(); - } - } - - private static class CacheClearer implements Runnable - { - private final Container _container; - - public CacheClearer(Container container) - { - _container = container; - } - - @Override - public boolean equals(Object o) - { - if (this == o) return true; - if (o == null || getClass() != o.getClass()) return false; - CacheClearer that = (CacheClearer) o; - return Objects.equals(_container, that._container); - } - - @Override - public int hashCode() - { - return Objects.hash(_container); - } - - @Override - public void run() - { - clearCache(_container); - } - } - - public static Runnable getCacheClearingCommitTask(Container c) - { - return new CacheClearer(c); - } - - /** - * Clears out the cache of files for this container. Must be called after any operation that changes the way a graph - * would be generated. Typically, this includes deleting or inserting any run in the container, because that - * can change the connections between the runs, which is reflected in the graphs. - */ - public static void clearCache(Container container) - { - Lock deleteLock = LOCK.writeLock(); - deleteLock.lock(); - try - { - FileUtil.deleteDir(getFolderDirectory(container)); - } - catch (IOException e) - { - // Non-fatal - _log.error("Failed to clear cached experiment run graphs for container " + container, e); - } - finally - { - deleteLock.unlock(); - } - } - - private static void testDotPath() throws ExperimentException - { - File dir; - - try - { - dir = getBaseDirectory(); - } - catch (IOException e) - { - throw new ExperimentException(DotRunner.getConfigurationError(e)); - } - - DotRunner.testDotPath(dir); - } - - - private static String getBaseFileName(ExpRun run, boolean detail, String focus) throws IOException - { - String fileName; - if (null != focus) - fileName = getFolderDirectory(run.getContainer()) + File.separator + "run" + run.getRowId() + "Focus" + focus; - else - fileName = getFolderDirectory(run.getContainer()) + File.separator + "run" + run.getRowId() + (detail ? "Detail" : ""); - return fileName; - } - - private static final Cleaner CLEANER = Cleaner.create(); - - private static class FileLockState implements Runnable - { - private final Throwable _allocation; - private Lock _lock; - - private FileLockState(Throwable allocation, Lock lock) - { - _allocation = allocation; - _lock = lock; - } - - @Override - public void run() - { - if (_lock != null) - { - _log.error("Lock was not released. Creation was at:", _allocation); - _lock.unlock(); - _lock = null; - } - } - - private void release() - { - if (_lock != null) - { - _lock.unlock(); - _lock = null; - } - } - } - - /** - * Results for run graph generation. Must be released once the files have been consumed by the caller. - */ - public static class RunGraphFiles - { - private final File _mapFile; - private final File _imageFile; - private final FileLockState _state; - private final Cleaner.Cleanable _cleanable; - - public RunGraphFiles(File mapFile, File imageFile, Lock lock) - { - _mapFile = mapFile; - _imageFile = imageFile; - _state = new FileLockState(new Throwable(), lock); - _cleanable = CLEANER.register(this, _state); - } - - public File getMapFile() - { - return _mapFile; - } - - public File getImageFile() - { - return _imageFile; - } - - /** - * Release the lock on the files. - */ - public void release() - { - _state.release(); - _cleanable.clean(); - } - } -} diff --git a/experiment/src/org/labkey/experiment/XarReader.java b/experiment/src/org/labkey/experiment/XarReader.java index 17909154d0f..a455b73c05e 100644 --- a/experiment/src/org/labkey/experiment/XarReader.java +++ b/experiment/src/org/labkey/experiment/XarReader.java @@ -26,7 +26,33 @@ import org.apache.xmlbeans.XmlException; import org.apache.xmlbeans.XmlObject; import org.apache.xmlbeans.XmlOptions; -import org.fhcrc.cpas.exp.xml.*; +import org.fhcrc.cpas.exp.xml.ContactType; +import org.fhcrc.cpas.exp.xml.DataBaseType; +import org.fhcrc.cpas.exp.xml.DataClassType; +import org.fhcrc.cpas.exp.xml.DataProtocolInputType; +import org.fhcrc.cpas.exp.xml.DataType; +import org.fhcrc.cpas.exp.xml.DomainDescriptorType; +import org.fhcrc.cpas.exp.xml.ExperimentArchiveDocument; +import org.fhcrc.cpas.exp.xml.ExperimentArchiveType; +import org.fhcrc.cpas.exp.xml.ExperimentLogEntryType; +import org.fhcrc.cpas.exp.xml.ExperimentRunType; +import org.fhcrc.cpas.exp.xml.ExperimentType; +import org.fhcrc.cpas.exp.xml.ImportAlias; +import org.fhcrc.cpas.exp.xml.InputOutputRefsType; +import org.fhcrc.cpas.exp.xml.MaterialBaseType; +import org.fhcrc.cpas.exp.xml.MaterialProtocolInputType; +import org.fhcrc.cpas.exp.xml.MaterialType; +import org.fhcrc.cpas.exp.xml.PropertyCollectionType; +import org.fhcrc.cpas.exp.xml.PropertyObjectDeclarationType; +import org.fhcrc.cpas.exp.xml.PropertyObjectType; +import org.fhcrc.cpas.exp.xml.ProtocolActionSetType; +import org.fhcrc.cpas.exp.xml.ProtocolActionType; +import org.fhcrc.cpas.exp.xml.ProtocolApplicationBaseType; +import org.fhcrc.cpas.exp.xml.ProtocolBaseType; +import org.fhcrc.cpas.exp.xml.SampleSetType; +import org.fhcrc.cpas.exp.xml.SimpleTypeNames; +import org.fhcrc.cpas.exp.xml.SimpleValueCollectionType; +import org.fhcrc.cpas.exp.xml.SimpleValueType; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import org.labkey.api.admin.FolderImportContext; @@ -92,7 +118,30 @@ import org.labkey.api.util.GUID; import org.labkey.api.util.Pair; import org.labkey.api.util.logging.LogHelper; -import org.labkey.experiment.api.*; +import org.labkey.experiment.api.AliasInsertHelper; +import org.labkey.experiment.api.Data; +import org.labkey.experiment.api.DataClass; +import org.labkey.experiment.api.DataInput; +import org.labkey.experiment.api.ExpDataClassImpl; +import org.labkey.experiment.api.ExpDataImpl; +import org.labkey.experiment.api.ExpMaterialImpl; +import org.labkey.experiment.api.ExpProtocolApplicationImpl; +import org.labkey.experiment.api.ExpProtocolImpl; +import org.labkey.experiment.api.ExpRunImpl; +import org.labkey.experiment.api.ExpSampleTypeImpl; +import org.labkey.experiment.api.Experiment; +import org.labkey.experiment.api.ExperimentRun; +import org.labkey.experiment.api.ExperimentServiceImpl; +import org.labkey.experiment.api.IdentifiableEntity; +import org.labkey.experiment.api.Material; +import org.labkey.experiment.api.MaterialInput; +import org.labkey.experiment.api.Protocol; +import org.labkey.experiment.api.ProtocolAction; +import org.labkey.experiment.api.ProtocolActionPredecessor; +import org.labkey.experiment.api.ProtocolActionStepDetail; +import org.labkey.experiment.api.ProtocolApplication; +import org.labkey.experiment.api.RunItem; +import org.labkey.experiment.api.SampleTypeServiceImpl; import org.labkey.experiment.api.property.DomainImpl; import org.labkey.experiment.pipeline.MoveRunsPipelineJob; import org.labkey.experiment.xar.AbstractXarImporter; @@ -127,11 +176,11 @@ import static org.labkey.api.dataiterator.SimpleTranslator.getContainerFileRootPath; import static org.labkey.api.dataiterator.SimpleTranslator.getFileRootSubstitutedFilePath; +import static org.labkey.api.exp.api.ColumnExporter.FILE_ROOT_SUBSTITUTION; import static org.labkey.api.exp.api.ExperimentService.SAMPLE_ALIQUOT_PROTOCOL_LSID; import static org.labkey.api.exp.api.ExperimentService.SAMPLE_DERIVATION_PROTOCOL_LSID; import static org.labkey.api.study.publish.StudyPublishService.STUDY_PUBLISH_PROTOCOL_LSID; import static org.labkey.experiment.XarExporter.GPAT_ASSAY_PROTOCOL_LSID_SUB; -import static org.labkey.api.exp.api.ColumnExporter.FILE_ROOT_SUBSTITUTION; public class XarReader extends AbstractXarImporter { @@ -449,19 +498,18 @@ private void loadDoc() throws ExperimentException throw new XarFormatException(e); } - FileBasedExperimentRunGraph.clearCache(getContainer()); - try { for (DeferredDataLoad deferredDataLoad : _deferredDataLoads) { Path path = deferredDataLoad.getData().getFilePath(); - if (path == null) - continue; - else if (Files.exists(path)) - deferredDataLoad.getData().importDataFile(_job, _xarSource); - else - getLog().warn("Data file " + FileUtil.getFileName(path) + " does not exist and could not be loaded."); + if (path != null) + { + if (Files.exists(path)) + deferredDataLoad.getData().importDataFile(_job, _xarSource); + else + getLog().warn("Data file {} does not exist and could not be loaded.", FileUtil.getFileName(path)); + } } } catch (SQLException e) diff --git a/experiment/src/org/labkey/experiment/api/ExpRunImpl.java b/experiment/src/org/labkey/experiment/api/ExpRunImpl.java index 6dc83ea79a1..4db94a4e583 100644 --- a/experiment/src/org/labkey/experiment/api/ExpRunImpl.java +++ b/experiment/src/org/labkey/experiment/api/ExpRunImpl.java @@ -62,7 +62,6 @@ import org.labkey.api.view.ActionURL; import org.labkey.api.view.UnauthorizedException; import org.labkey.experiment.DotGraph; -import org.labkey.experiment.FileBasedExperimentRunGraph; import java.io.File; import java.io.IOException; @@ -81,6 +80,8 @@ public class ExpRunImpl extends ExpIdentifiableEntityImpl implements ExpRun { + private static final Logger LOG = LogManager.getLogger(ExpRunImpl.class); + public static final String NAMESPACE_PREFIX = "Run"; private boolean _populated; @@ -91,8 +92,6 @@ public class ExpRunImpl extends ExpIdentifiableEntityImpl impleme private List _dataOutputs = new ArrayList<>(); private ExpRunImpl _replacedByRun; private Integer _maxOutputActionSequence = null; - private static final Logger LOG = LogManager.getLogger(ExpRunImpl.class); - private ExpProtocolApplication _workflowTask; static public List fromRuns(List runs) { @@ -560,10 +559,6 @@ public void deleteProtocolApplications(List datasToDelete, User use deleteRunProtocolApps(); clearCache(); - - // Clear the cache in a commit task, which allows us to do a single clear (which is semi-expensive) if multiple - // runs are being deleted in the same transaction, like deleting a container - svc.getSchema().getScope().addCommitTask(FileBasedExperimentRunGraph.getCacheClearingCommitTask(getContainer()), DbScope.CommitTaskOption.POSTCOMMIT); } @Override diff --git a/experiment/src/org/labkey/experiment/controllers/exp/ExperimentController.java b/experiment/src/org/labkey/experiment/controllers/exp/ExperimentController.java index 7474eebc46f..ec3248cf7b4 100644 --- a/experiment/src/org/labkey/experiment/controllers/exp/ExperimentController.java +++ b/experiment/src/org/labkey/experiment/controllers/exp/ExperimentController.java @@ -27,8 +27,6 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.apache.poi.ss.usermodel.Workbook; -import org.graphper.api.GraphResource; -import org.graphper.parser.DotParser; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import org.json.JSONArray; @@ -266,8 +264,6 @@ import org.labkey.experiment.DotGraph; import org.labkey.experiment.ExpDataFileListener; import org.labkey.experiment.ExperimentRunDisplayColumn; -import org.labkey.experiment.ExperimentRunGraph; -import org.labkey.experiment.FileBasedExperimentRunGraph; import org.labkey.experiment.LineageGraphDisplayColumn; import org.labkey.experiment.MissingFilesCheckInfo; import org.labkey.experiment.MoveRunsBean; @@ -317,17 +313,14 @@ import org.springframework.web.servlet.ModelAndView; import javax.imageio.ImageIO; -import java.awt.*; import java.awt.image.BufferedImage; import java.io.BufferedOutputStream; import java.io.ByteArrayOutputStream; import java.io.File; -import java.io.FileNotFoundException; import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; import java.net.URI; -import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; import java.sql.SQLException; @@ -1851,7 +1844,6 @@ public static ActionURL getRunGraphURL(Container c, long runId) return new ActionURL(ShowRunGraphAction.class, c).addParameter("rowId", runId); } - @RequiresPermission(ReadPermission.class) public class ShowRunGraphAction extends AbstractShowRunAction { @@ -1859,59 +1851,9 @@ public class ShowRunGraphAction extends AbstractShowRunAction protected VBox createLowerView(ExpRunImpl experimentRun, BindException errors) { return new VBox( - createRunViewTabs(experimentRun, false, true, true), - new ExperimentRunGraphView(experimentRun, false)); - } - } - - - @RequiresPermission(ReadPermission.class) - public static class DownloadGraphAction extends SimpleViewAction - { - @Override - public ModelAndView getView(ExperimentRunForm form, BindException errors) throws Exception - { - boolean detail = form.isDetail(); - String focus = form.getFocus(); - String focusType = form.getFocusType(); - - ExpRunImpl experimentRun = (ExpRunImpl) form.lookupRun(); - ensureCorrectContainer(getContainer(), experimentRun, getViewContext()); - - FileBasedExperimentRunGraph.RunGraphFiles files; - try - { - files = FileBasedExperimentRunGraph.generateRunGraph(getViewContext(), experimentRun, detail, focus, focusType); - } - catch (ExperimentException e) - { - PageFlowUtil.streamTextAsImage(getViewContext().getResponse(), "ERROR: " + e.getMessage(), 600, 150, Color.RED); - return null; - } - - try - { - PageFlowUtil.streamFile(getViewContext().getResponse(), files.getImageFile().toPath(), false); - } - catch (FileNotFoundException e) - { - throw new RedirectException(new URLHelper(getViewContext().getRequest().getContextPath() + "/experiment/ExperimentRunNotFound.gif")); - } - finally - { - files.release(); - } - - String dot = ExperimentRunGraph.getDotGraph(getContainer(), experimentRun, detail, focus, focusType); - getViewContext().getResponse().getOutputStream().write(ExperimentRunGraph.getSvg(dot).getBytes(StandardCharsets.UTF_8)); - - return null; - } - - @Override - public void addNavTrail(NavTree root) - { - throw new UnsupportedOperationException(); + createRunViewTabs(experimentRun, false, true, true), + new ExperimentRunGraphView(experimentRun, false) + ); } } @@ -7117,21 +7059,6 @@ public static ExperimentUrlsImpl get() return (ExperimentUrlsImpl) urlProvider(ExperimentUrls.class); } - public ActionURL getDownloadGraphURL(ExpRun run, boolean detail, String focus, String focusType) - { - ActionURL result = new ActionURL(DownloadGraphAction.class, run.getContainer()); - result.addParameter("rowId", run.getRowId()).addParameter("detail", detail); - if (focus != null) - { - result.addParameter("focus", focus); - } - if (focusType != null) - { - result.addParameter("focusType", focusType); - } - return result; - } - public ActionURL getBeginURL(Container container) { return new ActionURL(BeginAction.class, container); diff --git a/experiment/src/org/labkey/experiment/controllers/exp/experimentRunGraphView.jsp b/experiment/src/org/labkey/experiment/controllers/exp/experimentRunGraphView.jsp index 640a2656ef7..9067badd14c 100644 --- a/experiment/src/org/labkey/experiment/controllers/exp/experimentRunGraphView.jsp +++ b/experiment/src/org/labkey/experiment/controllers/exp/experimentRunGraphView.jsp @@ -15,21 +15,12 @@ * limitations under the License. */ %> -<%@ page import="org.apache.commons.io.IOUtils" %> <%@ page import="org.graphper.draw.ExecuteException" %> -<%@ page import="org.labkey.api.exp.ExperimentException" %> -<%@ page import="org.labkey.api.reader.Readers" %> <%@ page import="org.labkey.api.util.UniqueID" %> -<%@ page import="org.labkey.api.view.ActionURL" %> <%@ page import="org.labkey.api.view.HttpView" %> -<%@ page import="org.labkey.api.view.ViewContext" %> <%@ page import="org.labkey.api.view.template.ClientDependencies" %> <%@ page import="org.labkey.experiment.ExperimentRunGraph" %> -<%@ page import="org.labkey.experiment.FileBasedExperimentRunGraph" %> -<%@ page import="org.labkey.experiment.controllers.exp.ExperimentController" %> <%@ page import="org.labkey.experiment.controllers.exp.ExperimentRunGraphModel" %> -<%@ page import="java.io.IOException" %> -<%@ page import="java.io.Reader" %> <%@ page extends="org.labkey.api.jsp.JspBase" %> <%! @Override @@ -40,7 +31,6 @@ } %> <% - ViewContext context = getViewContext(); ExperimentRunGraphModel model = (ExperimentRunGraphModel)HttpView.currentModel(); boolean isSummaryView = !model.isDetail(); boolean isBetaViewEnabled = getActionURL().getParameter("betaGraph") != null; @@ -51,21 +41,8 @@ String graphTabId = "graph-tab-" + uniqueId; String graphTabBetaId = "graph-tab-beta-" + uniqueId; - try + if (isSummaryView) { - FileBasedExperimentRunGraph.RunGraphFiles files = FileBasedExperimentRunGraph.generateRunGraph(context, - model.getRun(), - model.isDetail(), - model.getFocus(), - model.getFocusType()); - - ActionURL imgSrc = ExperimentController.ExperimentUrlsImpl.get().getDownloadGraphURL(model.getRun(), - model.isDetail(), - model.getFocus(), - model.getFocusType()); - - if (isSummaryView) - { %> <%=button("Toggle Beta Graph (new!)").id(toggleBtnId).style("display: inline-block; float: right;")%>