Minor code cleanup and correctness improvements (#1186)

Author: labkey-jeckels

api-src/org/labkey/api/targetedms/model/QCMetricConfiguration.java

@@ -16,10 +16,16 @@
 package org.labkey.api.targetedms.model;
 
 import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.Nullable;
 import org.json.JSONObject;
 
 public class QCMetricConfiguration implements Comparable<QCMetricConfiguration>
 {
+    public enum TimeValueOption
+    {
+        First, Last, Min, Max
+    }
+
     private int _id;
     private String _name;
     private String _queryName;
@@ -124,6 +130,21 @@ public void setTimeValueOption(String timeValueOption)
         _timeValueOption = timeValueOption;
     }
 
+    @Nullable
+    public TimeValueOption getParsedTimeValueOption()
+    {
+        if (_timeValueOption == null)
+            return null;
+        try
+        {
+            return TimeValueOption.valueOf(_timeValueOption);
+        }
+        catch (IllegalArgumentException e)
+        {
+            return null;
+        }
+    }
+
     public Double getTraceValue()
     {
         return _traceValue;

src/org/labkey/targetedms/TargetedMSManager.java

@@ -163,7 +163,7 @@ private TargetedMSManager()
      * client rendering the overview, all of which need to know the enabled configs.
      */
     private static final Cache<Container, List<QCMetricConfiguration>> _metricCache = CacheManager.getBlockingCache(1000, TimeUnit.HOURS.toMillis(1), "Enabled QC metric configs",
-            (c, argument) ->
+            (_, argument) ->
             {
                 try
                 {
@@ -1048,7 +1048,7 @@ public static TargetedMSRun getRunByFileName(String fileName, Container containe
         List<TargetedMSRun> matches = new TableSelector(TargetedMSManager.getTableInfoRuns(), filter, null).getArrayList(TargetedMSRun.class);
         if (matches.size() == 1)
         {
-            return matches.get(0);
+            return matches.getFirst();
         }
         return null;
     }
@@ -1084,7 +1084,7 @@ else if(representativeState == RunRepresentativeDataState.Representative_Peptide
         // for the documents not yet imported.
         updateSql.append(" AND StatusId = ? ");
         updateSql.add(SkylineDocImporter.STATUS_SUCCESS);
-        updateSql.append(" AND Id NOT IN ("+StringUtils.join(representativeRunIds, ",")+")");
+        updateSql.append(" AND Id NOT IN (").append(StringUtils.join(representativeRunIds, ",")).append(")");
 
         new SqlExecutor(TargetedMSManager.getSchema()).execute(updateSql);
     }
@@ -1125,7 +1125,7 @@ private static List<Long> getProteinRepresentativeRunIds(Container container, in
         reprRunIdSql.append(TargetedMSManager.getTableInfoRuns(), "runs");
         String states = StringUtils.join(stateArray, ',');
 
-        reprRunIdSql.append(" WHERE pepgrp.RepresentativeDataState IN (" + states + ")");
+        reprRunIdSql.append(" WHERE pepgrp.RepresentativeDataState IN (").append(states).append(")");
         reprRunIdSql.append(" AND runs.Container = ?");
         reprRunIdSql.add(container);
         reprRunIdSql.append(" AND runs.Id = pepgrp.RunId");
@@ -1160,7 +1160,7 @@ private static List<Long> getPeptideRepresentativeRunIds(Container container, in
 
         String states = StringUtils.join(stateArray, ',');
 
-        reprRunIdSql.append(" WHERE gp.RepresentativeDataState IN (" + states + ")");
+        reprRunIdSql.append(" WHERE gp.RepresentativeDataState IN (").append(states).append(")");
         reprRunIdSql.append(" AND gp.GeneralMoleculeId = gm.Id");
         reprRunIdSql.append(" AND gm.PeptideGroupId = pepgrp.Id");
         reprRunIdSql.append(" AND Container = ?");
@@ -1918,19 +1918,15 @@ private static void purgeDeletedRuns()
         // Delete from all list-related tables
         deleteListDependent();
 
+        // Get a list of runs to be deleted so that we can flush them from the cache
+        SQLFragment sql = new SQLFragment("SELECT Id FROM " + getTableInfoRuns() + " WHERE Deleted =  ?", true);
+        List<Long> deletedRunIds = new SqlSelector(getSchema(), sql).getArrayList(Long.class);
+
         // Delete from runs
         execute("DELETE FROM " + getTableInfoRuns() + " WHERE Deleted = ?", true);
 
         // Remove any cached results for the deleted runs
-        removeCachedResults();
-    }
-
-    private static void removeCachedResults()
-    {
-        // Get a list of deleted runs
-        SQLFragment sql = new SQLFragment("SELECT Id FROM " + getTableInfoRuns() + " WHERE Deleted =  ?", true);
-        List<Long> deletedRunIds = new SqlSelector(getSchema(), sql).getArrayList(Long.class);
-        if(!deletedRunIds.isEmpty())
+        if (!deletedRunIds.isEmpty())
         {
             ModificationManager.removeRunCachedResults(deletedRunIds);
             PeptideManager.removeRunCachedResults(deletedRunIds);
@@ -2155,18 +2151,22 @@ public static void renameRun(long runId, String newDescription, User user) throw
         if (newDescription == null || newDescription.isEmpty())
             return;
 
-        new SqlExecutor(getSchema()).execute("UPDATE " + getTableInfoRuns() + " SET Description=? WHERE Id = ?",
-                            newDescription, runId);
-        TargetedMSRun run = getRun(runId);
-        if (run != null)
+        try (DbScope.Transaction t = getSchema().getScope().ensureTransaction())
         {
-            // Keep the experiment run wrapper in sync
-            ExpRun expRun = ExperimentService.get().getExpRun(run.getExperimentRunLSID());
-            if (expRun != null)
+            new SqlExecutor(getSchema()).execute("UPDATE " + getTableInfoRuns() + " SET Description=? WHERE Id = ?",
+                    newDescription, runId);
+            TargetedMSRun run = getRun(runId);
+            if (run != null)
             {
-                expRun.setName(newDescription);
-                expRun.save(user);
+                // Keep the experiment run wrapper in sync
+                ExpRun expRun = ExperimentService.get().getExpRun(run.getExperimentRunLSID());
+                if (expRun != null)
+                {
+                    expRun.setName(newDescription);
+                    expRun.save(user);
+                }
             }
+            t.commit();
         }
     }
 
@@ -2199,7 +2199,7 @@ public static SampleFile getSampleFile(long id, Container container)
         {
             throw new IllegalStateException("More than one SampleFile for Id " + id);
         }
-        return matches.isEmpty() ? null : matches.get(0);
+        return matches.isEmpty() ? null : matches.getFirst();
     }
 
     public static List<SampleFile> getSampleFiles(Container container, @Nullable SQLFragment whereClause)
@@ -2813,13 +2813,6 @@ public static TransitionSettings.FullScanSettings getTransitionFullScanSettings(
                 .getObject(TransitionSettings.FullScanSettings.class);
     }
 
-    public static TransitionSettings.Predictor getReplicatePredictor(long predictorId)
-    {
-        return new TableSelector(TargetedMSManager.getTableInfoTransitionFullScanSettings(),
-                new SimpleFilter(FieldKey.fromParts("Id"), predictorId), null)
-                .getObject(TransitionSettings.Predictor.class);
-    }
-
     private static List<Long> getPredictorsToDelete()
     {
         SQLFragment sql = new SQLFragment("Select Id FROM " + getTableInfoPredictor() + " WHERE " +
@@ -2871,12 +2864,12 @@ public static List<QCTraceMetricValues> calculateTraceMetricValues(List<QCMetric
                         Double minTimeValue = qcMetricConfiguration.getMinTimeValue();
                         Double maxTimeValue = qcMetricConfiguration.getMaxTimeValue();
                         Double traceValue = qcMetricConfiguration.getTraceValue();
-                        String timeValueOption = qcMetricConfiguration.getTimeValueOption();
+                        QCMetricConfiguration.TimeValueOption timeValueOption = qcMetricConfiguration.getParsedTimeValueOption();
                         float minValue = Float.MAX_VALUE;
-                        float maxValue = Float.MIN_VALUE;
+                        float maxValue = -Float.MAX_VALUE;
                         for (int i = 0; i < times.length; i++)
                         {
-                            if (timeValueOption != null && timeValueOption.equals("First"))
+                            if (timeValueOption == QCMetricConfiguration.TimeValueOption.First)
                             {
                                 // first value after the minTimeValue
                                 if (minTimeValue != null && times[i] > minTimeValue)
@@ -2891,7 +2884,7 @@ else if (minTimeValue == null)
                                     break;
                                 }
                             }
-                            else if (timeValueOption != null && timeValueOption.equals("Last"))
+                            else if (timeValueOption == QCMetricConfiguration.TimeValueOption.Last)
                             {
                                 // last value before the maxTimeValue
                                 if (maxTimeValue != null && times[i] < maxTimeValue)
@@ -2904,7 +2897,7 @@ else if (maxTimeValue == null)
                                     valuesToStore.put(sampleFileChromInfo, values[i]);
                                 }
                             }
-                            else if (timeValueOption != null && timeValueOption.equals("Min"))
+                            else if (timeValueOption == QCMetricConfiguration.TimeValueOption.Min)
                             {
                                 // minValue between minTimeValue and maxTimeValue
                                 if (minTimeValue != null && maxTimeValue != null && times[i] >= minTimeValue && times[i] <= maxTimeValue)
@@ -2929,9 +2922,8 @@ else if (minTimeValue == null && maxTimeValue == null && values[i] < minValue)
                                 {
                                     minValue = values[i];
                                 }
-                                valuesToStore.put(sampleFileChromInfo, minValue);
                             }
-                            else if (timeValueOption != null && timeValueOption.equals("Max"))
+                            else if (timeValueOption == QCMetricConfiguration.TimeValueOption.Max)
                             {
                                 // maxValue between minTimeValue and maxTimeValue
                                 if (minTimeValue != null && maxTimeValue != null && times[i] >= minTimeValue && times[i] <= maxTimeValue)
@@ -2956,7 +2948,6 @@ else if (minTimeValue == null && maxTimeValue == null && values[i] > maxValue)
                                 {
                                     maxValue = values[i];
                                 }
-                                valuesToStore.put(sampleFileChromInfo, maxValue);
                             }
 
                             else if (traceValue != null && values[i] >= traceValue)
@@ -2965,6 +2956,16 @@ else if (traceValue != null && values[i] >= traceValue)
                                 break;
                             }
                         }
+
+                        // Store Min/Max only if at least one data point matched the time filter
+                        if (timeValueOption == QCMetricConfiguration.TimeValueOption.Min && minValue != Float.MAX_VALUE)
+                        {
+                            valuesToStore.put(sampleFileChromInfo, minValue);
+                        }
+                        else if (timeValueOption == QCMetricConfiguration.TimeValueOption.Max && maxValue != -Float.MAX_VALUE)
+                        {
+                            valuesToStore.put(sampleFileChromInfo, maxValue);
+                        }
                     }
                 }