Skip to content

Fix assay/experiment metrics bugs and modernize Java syntax #7613

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
labkey-jeckels merged 1 commit into from
Apr 23, 2026
Merged

Fix assay/experiment metrics bugs and modernize Java syntax #7613

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 21 additions & 31 deletions api/src/org/labkey/api/assay/AbstractAssayProvider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -186,6 +187,8 @@ public abstract class AbstractAssayProvider implements AssayProvider
// The result row LSID namespace prefix <code>_resultRowLSIDPrefix</code> 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;
Expand Down Expand Up @@ -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<Long> rowIds = rowIdsByTargetContainer.computeIfAbsent(targetStudyContainer, k -> new HashSet<>());
Set<Long> rowIds = rowIdsByTargetContainer.computeIfAbsent(targetStudyContainer, _ -> new HashSet<>());
rowIds.add(publishKey.getDataId());

dataMaps.add(dataMap);
Expand Down Expand Up @@ -645,7 +648,7 @@ public List<AssayDataCollector> getDataCollectors(@Nullable Map<String, org.labk
if (inputDatas.size() == 1)
{
// There's exactly one input, so just use it
addReusableData(reusableFiles, inputDatas.get(0));
addReusableData(reusableFiles, inputDatas.getFirst());
}
else if (inputDatas.size() > 1)
{
Expand Down Expand Up @@ -1238,7 +1241,7 @@ public static void addInputMaterials(ExpRun expRun, User user, Map<ExpMaterial,
role = st != null ? st.getName() : "Sample";
}

inputGroups.computeIfAbsent(role, r -> new HashSet<>()).add(newInput.getRowId());
inputGroups.computeIfAbsent(role, _ -> new HashSet<>()).add(newInput.getRowId());
}

for (var entry : inputGroups.entrySet())
Expand All @@ -1259,7 +1262,7 @@ public boolean hasUsefulDetailsPage()
public Pair<ValidationException, Pair<String, String>> setValidationAndAnalysisScripts(ExpProtocol protocol, @NotNull List<AnalysisScript> scripts) throws ExperimentException
{
Map<String, ObjectProperty> 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)
Expand All @@ -1272,7 +1275,7 @@ public Pair<ValidationException, Pair<String, String>> 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())
Expand Down Expand Up @@ -1320,23 +1323,8 @@ public Pair<ValidationException, Pair<String, String>> 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
Expand All @@ -1345,7 +1333,7 @@ public List<AnalysisScript> getValidationAndAnalysisScripts(ExpProtocol protocol
List<AnalysisScript> 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<AnalysisScript> scripts = AnalysisScript.fromJson(transformScripts.getStringValue());
Expand Down Expand Up @@ -1785,8 +1773,8 @@ public void moveRuns(List<ExpRun> 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();

Expand Down Expand Up @@ -1914,7 +1902,7 @@ private void moveRunsBatch(List<ExpRun> 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<? extends DomainProperty> fileDomainProps = getBatchDomain(assayProtocol)
.getProperties().stream()
.filter(prop -> PropertyType.FILE_LINK.getTypeUri().equals(prop.getRangeURI())).toList();
Expand Down Expand Up @@ -1996,7 +1984,7 @@ private void updateRunFiles(List<ExpRun> runs, Container sourceContainer, Contai
{
Map<Long, List<AssayFileMoveData>> movedFiles = assayMoveData.fileMovesByRunId();

ExpProtocol assayProtocol = runs.get(0).getProtocol();
ExpProtocol assayProtocol = runs.getFirst().getProtocol();

List<? extends DomainProperty> fileDomainProps = getRunDomain(assayProtocol)
.getProperties().stream()
Expand Down Expand Up @@ -2096,8 +2084,7 @@ private void updateDataFileUrl(List<ExpRun> 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)
Expand All @@ -2116,7 +2103,10 @@ private void updateDataFileUrl(List<ExpRun> runs, Container sourceContainer, Con
AuditLogService.get().addEvent(user, event);
}
}
catch (Exception ignored) {}
catch (Exception e)
{
LOG.warn("Failed to parse file URI {}", oldFileUrl, e);
}
}
}

Expand Down
64 changes: 4 additions & 60 deletions api/src/org/labkey/api/util/PageFlowUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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())
Expand Down Expand Up @@ -2129,7 +2122,7 @@ public static String sanitizeHtml(String html, Collection<String> 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");
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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')
Expand Down Expand Up @@ -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);
}


Expand All @@ -2728,7 +2720,6 @@ static public Map<String, MultipartFile> getFileMap(HttpServletRequest req)
{
if (!(req instanceof MultipartHttpServletRequest mpreq))
return Collections.emptyMap();
@SuppressWarnings("SSBasedInspection")
Map<String, MultipartFile> htmlMap = mpreq.getFileMap();
Map<String, MultipartFile> formMap = new LinkedHashMap<>();
htmlMap.forEach((key, value) -> formMap.put(PageFlowUtil.decodeFormName(key), value));
Expand All @@ -2740,7 +2731,6 @@ static public MultiValueMap<String, MultipartFile> getMultiFileMap(HttpServletRe
MultiValueMap<String, MultipartFile> formMap = new LinkedMultiValueMap<>();
if (!(req instanceof MultipartHttpServletRequest mpreq))
return formMap;
@SuppressWarnings("SSBasedInspection")
MultiValueMap<String, MultipartFile> htmlMap = mpreq.getMultiFileMap();
htmlMap.forEach((key, value) -> formMap.put(PageFlowUtil.decodeFormName(key), value));
return formMap;
Expand Down Expand Up @@ -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<ClientDependency> resources)
{
JSONObject ret = new JSONObject();
Expand Down
22 changes: 19 additions & 3 deletions experiment/src/org/labkey/experiment/ExperimentModule.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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("""
Expand Down
2 changes: 1 addition & 1 deletion experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9345,7 +9345,7 @@ private Map<String, Object> getParentAliasMetrics()
Map<String, Object> metrics = new HashMap<>();
Pair<Long, Long> samplesMetrics = getParentAliasMetrics(getTinfoSampleType(), "materialparentimportaliasmap");
metrics.put("RequiredSampleParentsForSampleTypes", samplesMetrics.first);
metrics.put("RequiredSourceParentsForSampleTypes", samplesMetrics.first);
metrics.put("RequiredSourceParentsForSampleTypes", samplesMetrics.second);
Pair<Long, Long> dataMetrics = getParentAliasMetrics(getTinfoDataClass(), "dataparentimportaliasmap");
metrics.put("RequiredSampleParentsForDataClasses", dataMetrics.first);
metrics.put("RequiredSourceParentsForDataClasses", dataMetrics.second);
Expand Down
Loading