From b6ad15ac37edff74c474e53002e1c5c81cf16509 Mon Sep 17 00:00:00 2001 From: labkey-jeckels Date: Tue, 21 Apr 2026 17:14:00 -0700 Subject: [PATCH] Fix assay/experiment metrics bugs and modernize Java syntax - Fix RequiredSourceParentsForSampleTypes metric using .first (sample count) instead of .second (source/data count) in ExperimentServiceImpl - Fix protocolsWithTransformScriptRunOnEditCount using INSERT LIKE pattern instead of UPDATE; both edit and import metrics were counting the same thing - Parameterize SQL queries in ExperimentModule assay metrics - Extract ScriptType private enum to public TRANSFORM_SCRIPT_PROPERTY_NAME constant so ExperimentModule can reference it without duplication - Remove dead PageFlowUtil methods: streamTextAsImage() and getSessionId() - Modernize Java syntax: unnamed lambda vars, getFirst(), pattern variables, URI.create(), isEmpty() - Improve XML serialization error to use logger instead of printStackTrace() Co-Authored-By: Claude Sonnet 4.6 --- .../api/assay/AbstractAssayProvider.java | 52 ++++++--------- api/src/org/labkey/api/util/PageFlowUtil.java | 64 ++----------------- .../labkey/experiment/ExperimentModule.java | 22 ++++++- .../experiment/api/ExperimentServiceImpl.java | 2 +- 4 files changed, 45 insertions(+), 95 deletions(-) diff --git a/api/src/org/labkey/api/assay/AbstractAssayProvider.java b/api/src/org/labkey/api/assay/AbstractAssayProvider.java index c620c02341b..724af962d08 100644 --- a/api/src/org/labkey/api/assay/AbstractAssayProvider.java +++ b/api/src/org/labkey/api/assay/AbstractAssayProvider.java @@ -20,6 +20,7 @@ import org.apache.commons.collections4.MapUtils; import org.apache.commons.io.IOUtils; import org.apache.commons.lang3.StringUtils; +import org.apache.logging.log4j.Logger; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import org.json.JSONArray; @@ -111,6 +112,7 @@ import org.labkey.api.util.PageFlowUtil; import org.labkey.api.util.Pair; import org.labkey.api.util.StringUtilsLabKey; +import org.labkey.api.util.logging.LogHelper; import org.labkey.api.view.ActionURL; import org.labkey.api.view.DetailsView; import org.labkey.api.view.NavTree; @@ -126,7 +128,6 @@ import java.io.IOException; import java.io.InputStream; import java.net.URI; -import java.net.URL; import java.sql.ResultSet; import java.sql.SQLException; import java.util.ArrayList; @@ -186,6 +187,8 @@ public abstract class AbstractAssayProvider implements AssayProvider // The result row LSID namespace prefix _resultRowLSIDPrefix should end with this constant. public static final String RESULT_LSID_PREFIX_PART = "AssayResultRow"; + private static final Logger LOG = LogHelper.getLogger(AbstractAssayProvider.class, "Base functionality for all assay types"); + protected final String _protocolLSIDPrefix; protected final String _runLSIDPrefix; protected final String _resultRowLSIDPrefix; @@ -285,7 +288,7 @@ public ActionURL linkToStudy(User user, Container assayDataContainer, ExpProtoco dataMap.put(StudyPublishService.TARGET_STUDY_PROPERTY_NAME, targetStudyContainer); // Remember which rows we're planning to link, partitioned by the target study - Set rowIds = rowIdsByTargetContainer.computeIfAbsent(targetStudyContainer, k -> new HashSet<>()); + Set rowIds = rowIdsByTargetContainer.computeIfAbsent(targetStudyContainer, _ -> new HashSet<>()); rowIds.add(publishKey.getDataId()); dataMaps.add(dataMap); @@ -645,7 +648,7 @@ public List getDataCollectors(@Nullable Map 1) { @@ -1238,7 +1241,7 @@ public static void addInputMaterials(ExpRun expRun, User user, Map new HashSet<>()).add(newInput.getRowId()); + inputGroups.computeIfAbsent(role, _ -> new HashSet<>()).add(newInput.getRowId()); } for (var entry : inputGroups.entrySet()) @@ -1259,7 +1262,7 @@ public boolean hasUsefulDetailsPage() public Pair> setValidationAndAnalysisScripts(ExpProtocol protocol, @NotNull List scripts) throws ExperimentException { Map props = new HashMap<>(protocol.getObjectProperties()); - String propertyURI = ScriptType.TRANSFORM.getPropertyURI(protocol); + String propertyURI = createPropertyURI(protocol, TRANSFORM_SCRIPT_PROPERTY_NAME); ValidationException validationErrors = new ValidationException(); for (AnalysisScript script : scripts) @@ -1272,7 +1275,7 @@ public Pair> setValidationAndAnalysisS if (engine != null) { // check for deprecated tokens in text scripts - if (!(engine instanceof ExternalScriptEngine && ((ExternalScriptEngine) engine).isBinary(scriptFile))) + if (!(engine instanceof ExternalScriptEngine ese && ese.isBinary(scriptFile))) { String scriptText; try (InputStream is = scriptFile.openInputStream()) @@ -1320,23 +1323,8 @@ public Pair> setValidationAndAnalysisS return new Pair<>(validationErrors, new Pair<>(oldJson, json == null ? null : json.toString())); } - /** For migrating legacy assay designs that have separate transform and validation script properties */ - private enum ScriptType - { - TRANSFORM("TransformScript"); - - private final String _uriSuffix; - - ScriptType(String uriSuffix) - { - _uriSuffix = uriSuffix; - } - - public String getPropertyURI(ExpProtocol protocol) - { - return protocol.getLSID() + "#" + _uriSuffix; - } - } + /** Property name suffix used when storing transform scripts on assay protocol objects */ + public static final String TRANSFORM_SCRIPT_PROPERTY_NAME = "TransformScript"; @NotNull @Override @@ -1345,7 +1333,7 @@ public List getValidationAndAnalysisScripts(ExpProtocol protocol List result = new ArrayList<>(); if (scope == Scope.ASSAY_DEF || scope == Scope.ALL) { - ObjectProperty transformScripts = protocol.getObjectProperties().get(ScriptType.TRANSFORM.getPropertyURI(protocol)); + ObjectProperty transformScripts = protocol.getObjectProperties().get(createPropertyURI(protocol, TRANSFORM_SCRIPT_PROPERTY_NAME)); if (transformScripts != null) { List scripts = AnalysisScript.fromJson(transformScripts.getStringValue()); @@ -1785,8 +1773,8 @@ public void moveRuns(List runs, Container targetContainer, User user, As if (runs.isEmpty()) return; - Container sourceContainer = runs.get(0).getContainer(); - ExpProtocol assayProtocol = runs.get(0).getProtocol(); + Container sourceContainer = runs.getFirst().getContainer(); + ExpProtocol assayProtocol = runs.getFirst().getProtocol(); ExperimentService experimentService = ExperimentService.get(); @@ -1914,7 +1902,7 @@ private void moveRunsBatch(List runs, Container sourceContainer, Contain // move batch files // batch property files needs to be moved before updating exp.object for the batch // objectpropertiesview joins exp.object for finding properties with matched container, so the old container needs to be used for finding file props - ExpProtocol assayProtocol = runs.get(0).getProtocol(); + ExpProtocol assayProtocol = runs.getFirst().getProtocol(); List fileDomainProps = getBatchDomain(assayProtocol) .getProperties().stream() .filter(prop -> PropertyType.FILE_LINK.getTypeUri().equals(prop.getRangeURI())).toList(); @@ -1996,7 +1984,7 @@ private void updateRunFiles(List runs, Container sourceContainer, Contai { Map> movedFiles = assayMoveData.fileMovesByRunId(); - ExpProtocol assayProtocol = runs.get(0).getProtocol(); + ExpProtocol assayProtocol = runs.getFirst().getProtocol(); List fileDomainProps = getRunDomain(assayProtocol) .getProperties().stream() @@ -2096,8 +2084,7 @@ private void updateDataFileUrl(List runs, Container sourceContainer, Con try { - URL url = new URL(oldFileUrl); - URI uri = url.toURI(); + URI uri = URI.create(oldFileUrl); File sourceFile = new File(uri); File updatedFile = fileContentService.getMoveTargetFile(sourceFile.getAbsolutePath(), sourceContainer, targetContainer); if (updatedFile != null) @@ -2116,7 +2103,10 @@ private void updateDataFileUrl(List runs, Container sourceContainer, Con AuditLogService.get().addEvent(user, event); } } - catch (Exception ignored) {} + catch (Exception e) + { + LOG.warn("Failed to parse file URI {}", oldFileUrl, e); + } } } diff --git a/api/src/org/labkey/api/util/PageFlowUtil.java b/api/src/org/labkey/api/util/PageFlowUtil.java index 1e6fa06dc12..7be3cb5496f 100644 --- a/api/src/org/labkey/api/util/PageFlowUtil.java +++ b/api/src/org/labkey/api/util/PageFlowUtil.java @@ -36,8 +36,6 @@ import org.apache.tika.mime.MimeTypes; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; -import org.jfree.chart.encoders.EncoderUtil; -import org.jfree.chart.encoders.ImageFormat; import org.json.JSONObject; import org.junit.Assert; import org.junit.Test; @@ -89,7 +87,6 @@ import org.springframework.util.MultiValueMap; import org.springframework.web.multipart.MultipartFile; import org.springframework.web.multipart.MultipartHttpServletRequest; -import org.springframework.web.util.WebUtils; import org.w3c.dom.Document; import org.w3c.dom.Element; import org.w3c.dom.Node; @@ -111,7 +108,6 @@ import javax.xml.transform.dom.DOMSource; import javax.xml.transform.stream.StreamResult; import java.awt.*; -import java.awt.image.BufferedImage; import java.io.BufferedReader; import java.io.ByteArrayOutputStream; import java.io.File; @@ -147,7 +143,6 @@ import java.util.Map; import java.util.Objects; import java.util.Set; -import java.util.StringTokenizer; import java.util.concurrent.atomic.AtomicInteger; import java.util.regex.Matcher; import java.util.regex.Pattern; @@ -569,8 +564,6 @@ public static String toQueryString(PropertyValues pvs) for (PropertyValue entry : pvs.getPropertyValues()) { Object key = entry.getName(); - if (null == key) - continue; String encKey = encodeURIComponent(String.valueOf(key)); Object v = entry.getValue(); if (v == null || v instanceof String || !v.getClass().isArray()) @@ -2129,7 +2122,7 @@ public static String sanitizeHtml(String html, Collection errors) if ("_blank".equals(target)) { String rel = href.getAttribute("rel"); - if (rel == null || !rel.contains("noopener") || !rel.contains("noreferrer")) + if (!rel.contains("noopener") || !rel.contains("noreferrer")) { modified = true; href.setAttribute("rel", "noopener noreferrer"); @@ -2169,7 +2162,7 @@ public static String documentToString(Document document) } catch (TransformerException tEx) { - tEx.printStackTrace(); + _log.error("Failed to convert XML document to string", tEx); } return null; } @@ -2625,7 +2618,7 @@ else if (c == ',' || c == '\0') } case AFTERTOKEN -> { - assert currentToken.length() == 0; + assert currentToken.isEmpty(); if (Character.isWhitespace(c)) continue; if (c == ',' || c == '\0') @@ -2710,8 +2703,7 @@ public static String encodeFormName(String name) if (!StringUtils.containsAny(name, unclean)) return name; // CONSIDER: use encode(name) for simplicity or only encode the unclean chars? - var ret = FIELD_ENCODED_PREFIX + encode(name); - return ret; + return FIELD_ENCODED_PREFIX + encode(name); } @@ -2728,7 +2720,6 @@ static public Map getFileMap(HttpServletRequest req) { if (!(req instanceof MultipartHttpServletRequest mpreq)) return Collections.emptyMap(); - @SuppressWarnings("SSBasedInspection") Map htmlMap = mpreq.getFileMap(); Map formMap = new LinkedHashMap<>(); htmlMap.forEach((key, value) -> formMap.put(PageFlowUtil.decodeFormName(key), value)); @@ -2740,7 +2731,6 @@ static public MultiValueMap getMultiFileMap(HttpServletRe MultiValueMap formMap = new LinkedMultiValueMap<>(); if (!(req instanceof MultipartHttpServletRequest mpreq)) return formMap; - @SuppressWarnings("SSBasedInspection") MultiValueMap htmlMap = mpreq.getMultiFileMap(); htmlMap.forEach((key, value) -> formMap.put(PageFlowUtil.decodeFormName(key), value)); return formMap; @@ -3297,52 +3287,6 @@ public static ActionURL addLastFilterParameter(ActionURL url, String scope) { return url.addParameter(scope + DataRegion.LAST_FILTER_PARAM, "true"); } - - public static String getSessionId(HttpServletRequest request) - { - return WebUtils.getSessionId(request); - } - - /** - * Stream the text back to the browser as a PNG - */ - public static void streamTextAsImage(HttpServletResponse response, String text, int width, int height, Color textColor) throws IOException - { - Font font = new Font("SansSerif", Font.PLAIN, 12); - - BufferedImage buffer = new BufferedImage(width, height, BufferedImage.TYPE_INT_RGB); - Graphics2D g2 = buffer.createGraphics(); - g2.setColor(Color.WHITE); - g2.fillRect(0, 0, width, height); - g2.setRenderingHint(RenderingHints.KEY_ANTIALIASING, RenderingHints.VALUE_ANTIALIAS_ON); - g2.setColor(textColor); - g2.setFont(font); - FontMetrics metrics = g2.getFontMetrics(); - int fontHeight = metrics.getHeight(); - int spaceWidth = metrics.stringWidth(" "); - - int x = 5; - int y = fontHeight + 5; - - StringTokenizer st = new StringTokenizer(text, " "); - // Line wrap to fit - while (st.hasMoreTokens()) - { - String token = st.nextToken(); - int tokenWidth = metrics.stringWidth(token); - if (x != 5 && tokenWidth + x > width) - { - x = 5; - y += fontHeight; - } - g2.drawString(token, x, y); - x += tokenWidth + spaceWidth; - } - - response.setContentType("image/png"); - EncoderUtil.writeBufferedImage(buffer, ImageFormat.PNG, response.getOutputStream()); - } - public static JSONObject getModuleClientContext(ContainerUser context, @Nullable LinkedHashSet resources) { JSONObject ret = new JSONObject(); diff --git a/experiment/src/org/labkey/experiment/ExperimentModule.java b/experiment/src/org/labkey/experiment/ExperimentModule.java index e96b25ce651..efcb9f2c1fc 100644 --- a/experiment/src/org/labkey/experiment/ExperimentModule.java +++ b/experiment/src/org/labkey/experiment/ExperimentModule.java @@ -20,8 +20,10 @@ import org.jetbrains.annotations.Nullable; import org.json.JSONObject; import org.labkey.api.admin.FolderSerializationRegistry; +import org.labkey.api.assay.AbstractAssayProvider; import org.labkey.api.assay.AssayProvider; import org.labkey.api.assay.AssayService; +import org.labkey.api.assay.transform.DataTransformService; import org.labkey.api.attachments.AttachmentService; import org.labkey.api.audit.AuditLogService; import org.labkey.api.audit.SampleTimelineAuditEvent; @@ -631,9 +633,23 @@ public void containerDeleted(Container c, User user) assayMetrics.put(assayProvider.getName(), protocolMetrics); } assayMetrics.put("autoLinkedAssayCount", new SqlSelector(schema, "SELECT COUNT(*) FROM exp.protocol EP JOIN exp.objectPropertiesView OP ON EP.lsid = OP.objecturi WHERE OP.propertyuri = 'terms.labkey.org#AutoCopyTargetContainer'").getObject(Long.class)); - assayMetrics.put("protocolsWithTransformScriptCount", new SqlSelector(schema, "SELECT COUNT(*) FROM exp.protocol EP JOIN exp.objectPropertiesView OP ON EP.lsid = OP.objecturi WHERE OP.name = 'TransformScript' AND status = 'Active'").getObject(Long.class)); - assayMetrics.put("protocolsWithTransformScriptRunOnEditCount", new SqlSelector(schema, "SELECT COUNT(*) FROM exp.protocol EP JOIN exp.objectPropertiesView OP ON EP.lsid = OP.objecturi WHERE OP.name = 'TransformScript' AND status = 'Active' AND OP.stringvalue LIKE '%\"INSERT\"%'").getObject(Long.class)); - assayMetrics.put("protocolsWithTransformScriptRunOnImportCount", new SqlSelector(schema, "SELECT COUNT(*) FROM exp.protocol EP JOIN exp.objectPropertiesView OP ON EP.lsid = OP.objecturi WHERE OP.name = 'TransformScript' AND status = 'Active' AND OP.stringvalue LIKE '%\"INSERT\"%'").getObject(Long.class)); + assayMetrics.put("protocolsWithTransformScriptCount", new SqlSelector(schema, + "SELECT COUNT(*) FROM exp.protocol EP JOIN exp.objectPropertiesView OP ON EP.lsid = OP.objecturi WHERE OP.name = ? AND status = ?", + AbstractAssayProvider.TRANSFORM_SCRIPT_PROPERTY_NAME, + ExpProtocol.Status.Active.toString() + ).getObject(Long.class)); + assayMetrics.put("protocolsWithTransformScriptRunOnEditCount", new SqlSelector(schema, + "SELECT COUNT(*) FROM exp.protocol EP JOIN exp.objectPropertiesView OP ON EP.lsid = OP.objecturi WHERE OP.name = ? AND status = ? AND OP.stringvalue LIKE ?", + AbstractAssayProvider.TRANSFORM_SCRIPT_PROPERTY_NAME, + ExpProtocol.Status.Active.toString(), + "%\"" + DataTransformService.TransformOperation.UPDATE + "\"%" + ).getObject(Long.class)); + assayMetrics.put("protocolsWithTransformScriptRunOnImportCount", new SqlSelector(schema, + "SELECT COUNT(*) FROM exp.protocol EP JOIN exp.objectPropertiesView OP ON EP.lsid = OP.objecturi WHERE OP.name = ? AND status = ? AND OP.stringvalue LIKE ?", + AbstractAssayProvider.TRANSFORM_SCRIPT_PROPERTY_NAME, + ExpProtocol.Status.Active.toString(), + "%\"" + DataTransformService.TransformOperation.INSERT + "\"%" + ).getObject(Long.class)); assayMetrics.put("standardAssayWithPlateSupportCount", new SqlSelector(schema, "SELECT COUNT(*) FROM exp.protocol EP JOIN exp.objectPropertiesView OP ON EP.lsid = OP.objecturi WHERE OP.name = 'PlateMetadata' AND floatValue = 1").getObject(Long.class)); SQLFragment runsWithPlateSQL = new SQLFragment(""" diff --git a/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java b/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java index 26699c33010..427cf6f2f42 100644 --- a/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java +++ b/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java @@ -9345,7 +9345,7 @@ private Map getParentAliasMetrics() Map metrics = new HashMap<>(); Pair samplesMetrics = getParentAliasMetrics(getTinfoSampleType(), "materialparentimportaliasmap"); metrics.put("RequiredSampleParentsForSampleTypes", samplesMetrics.first); - metrics.put("RequiredSourceParentsForSampleTypes", samplesMetrics.first); + metrics.put("RequiredSourceParentsForSampleTypes", samplesMetrics.second); Pair dataMetrics = getParentAliasMetrics(getTinfoDataClass(), "dataparentimportaliasmap"); metrics.put("RequiredSampleParentsForDataClasses", dataMetrics.first); metrics.put("RequiredSourceParentsForDataClasses", dataMetrics.second);