diff --git a/testresults/src/org/labkey/testresults/TestResultsController.java b/testresults/src/org/labkey/testresults/TestResultsController.java index 17faeb9f..5b99149a 100644 --- a/testresults/src/org/labkey/testresults/TestResultsController.java +++ b/testresults/src/org/labkey/testresults/TestResultsController.java @@ -142,7 +142,18 @@ public class TestResultsController extends SpringActionController { private static final Logger _log = LogManager.getLogger(TestResultsController.class); - private static final SimpleDateFormat MDYFormat = new SimpleDateFormat("MM/dd/yyyy"); + private static final String MDY_PATTERN = "MM/dd/yyyy"; + + // SimpleDateFormat is not thread-safe and is lenient by default, so we never share a + // single static instance. Each caller gets a fresh, strict formatter: strict parsing + // makes nonsense dates like 13/45/2026 throw ParseException instead of silently rolling + // over to a valid-but-wrong date (month 13 -> +1 year, day 45 -> spill into next month). + private static SimpleDateFormat mdyFormat() + { + SimpleDateFormat format = new SimpleDateFormat(MDY_PATTERN); + format.setLenient(false); + return format; + } private static final String KEY_SUCCESS = "Success"; @@ -175,7 +186,76 @@ public static String getTabClass(String tabName, String activeTab) private static Date parseDate(String dateStr) throws ParseException { - return StringUtils.isNotBlank(dateStr) ? MDYFormat.parse(dateStr) : null; + return StringUtils.isNotBlank(dateStr) ? mdyFormat().parse(dateStr) : null; + } + + /** + * Loads the run with the given id only if it belongs to the given container, otherwise returns + * null. Run ids are global and the raw testruns table has no query-layer container filter, so + * actions that look a run up by id must scope by container or a user could read or modify a run + * in another folder by guessing its id. + */ + private static RunDetail getRunInContainer(int runId, Container container) + { + SimpleFilter filter = new SimpleFilter(FieldKey.fromParts("id"), runId); + filter.addCondition(FieldKey.fromParts("container"), container.getEntityId()); + return new TableSelector(TestResultsSchema.getTableInfoTestRuns(), filter, null).getObject(RunDetail.class); + } + + /** + * Recomputes one user's training statistics (mean/stddev of tests-run and memory) in the + * given container from their remaining training runs. If the user has no training runs + * left, deletes their UserData row instead. + * + * The delete branch matters: the recompute groups by (userid, container), so an empty + * join produces zero output rows, ON CONFLICT never fires, and a stale UserData row would + * otherwise survive with its old mean/stddev values (leaving the user listed on the + * training-data page as if they still had training data). Callers must invoke this inside + * an open transaction. + */ + private static void recomputeUserData(DbScope scope, int userid, Container container) + { + SQLFragment remainingFragment = new SQLFragment(); + remainingFragment.append( + "SELECT COUNT(*) FROM " + TestResultsSchema.getTableInfoTestRuns() + " " + + "JOIN " + TestResultsSchema.getTableInfoTrain() + + " ON " + TestResultsSchema.getTableInfoTestRuns() + ".id = " + TestResultsSchema.getTableInfoTrain() + ".runid " + + "WHERE userid = ? AND container = ?"); + remainingFragment.add(userid); + remainingFragment.add(container.getEntityId()); + long remainingTrainRuns = new SqlSelector(TestResultsSchema.getSchema(), remainingFragment).getObject(Long.class); + + if (remainingTrainRuns == 0) + { + SQLFragment deleteFragment = new SQLFragment(); + deleteFragment.append("DELETE FROM " + TestResultsSchema.getTableInfoUserData() + " WHERE userid = ? AND container = ?"); + deleteFragment.add(userid); + deleteFragment.add(container.getEntityId()); + new SqlExecutor(scope).execute(deleteFragment); + } + else + { + SQLFragment sqlFragmentUpdate = new SQLFragment(); + sqlFragmentUpdate.append( + "INSERT INTO " + TestResultsSchema.getTableInfoUserData() + " " + + " (userid, container, meantestsrun, meanmemory, stddevtestsrun, stddevmemory) " + + "SELECT ?, ?, avg(passedtests), avg(averagemem), stddev_pop(passedtests), stddev_pop(averagemem) " + + "FROM " + TestResultsSchema.getTableInfoTestRuns() + " " + + "JOIN " + TestResultsSchema.getTableInfoTrain() + + " ON " + TestResultsSchema.getTableInfoTestRuns() + ".id = " + TestResultsSchema.getTableInfoTrain() + ".runid " + + "WHERE userid = ? AND container = ? " + + "GROUP BY userid, container " + + "ON CONFLICT(userid, container) DO UPDATE SET " + + " meantestsrun = excluded.meantestsrun, " + + " meanmemory = excluded.meanmemory, " + + " stddevtestsrun = excluded.stddevtestsrun, " + + " stddevmemory = excluded.stddevmemory"); + sqlFragmentUpdate.add(userid); + sqlFragmentUpdate.add(container.getEntityId()); + sqlFragmentUpdate.add(userid); + sqlFragmentUpdate.add(container.getEntityId()); + new SqlExecutor(scope).execute(sqlFragmentUpdate); + } } // Form class for RetrainAllAction @@ -529,16 +609,14 @@ public Object execute(TrainRunForm form, BindException errors) SqlSelector sqlSelector = new SqlSelector(TestResultsSchema.getSchema(), sqlFragment); List foundRuns = new ArrayList<>(); sqlSelector.forEach(rs -> foundRuns.add(rs.getInt("runid"))); - SimpleFilter filter = new SimpleFilter(); - filter.addCondition(FieldKey.fromParts("id"), runId); - RunDetail[] details = new TableSelector(TestResultsSchema.getTableInfoTestRuns(), filter, null).getArray(RunDetail.class); - if (!force) - { - if (details.length == 0) - return new ApiSimpleResponse(Map.of(KEY_SUCCESS, false, "error", "run does not exist: " + runId)); - else if ((train && !foundRuns.isEmpty()) || (!train && foundRuns.isEmpty())) - return new ApiSimpleResponse(Map.of(KEY_SUCCESS, false, "error", "no action necessary")); - } + // The run must exist (in this folder) even on the force path: recomputeUserData below + // uses run.getUserid(), so a missing run would otherwise throw. Scoping by container + // also stops a run in another folder from being trained from here. + RunDetail run = getRunInContainer(runId, getContainer()); + if (run == null) + return new ApiSimpleResponse(Map.of(KEY_SUCCESS, false, "error", "run does not exist: " + runId)); + if (!force && ((train && !foundRuns.isEmpty()) || (!train && foundRuns.isEmpty()))) + return new ApiSimpleResponse(Map.of(KEY_SUCCESS, false, "error", "no action necessary")); DbScope scope = TestResultsSchema.getSchema().getScope(); try (DbScope.Transaction transaction = scope.ensureTransaction()) { @@ -551,27 +629,8 @@ else if ((train && !foundRuns.isEmpty()) || (!train && foundRuns.isEmpty())) fragment.add(runId); new SqlExecutor(scope).execute(fragment); } - // update user table calculations - SQLFragment sqlFragmentUpdate = new SQLFragment(); - sqlFragmentUpdate.append( - "INSERT INTO " + TestResultsSchema.getTableInfoUserData() + " " + - " (userid, container, meantestsrun, meanmemory, stddevtestsrun, stddevmemory) " + - "SELECT ?, ?, avg(passedtests), avg(averagemem), stddev_pop(passedtests), stddev_pop(averagemem) " + - "FROM " + TestResultsSchema.getTableInfoTestRuns() + " " + - "JOIN " + TestResultsSchema.getTableInfoTrain() + - " ON " + TestResultsSchema.getTableInfoTestRuns() + ".id = " + TestResultsSchema.getTableInfoTrain() + ".runid " + - "WHERE userid = ? AND container = ? " + - "GROUP BY userid, container " + - "ON CONFLICT(userid, container) DO UPDATE SET " + - " meantestsrun = excluded.meantestsrun, " + - " meanmemory = excluded.meanmemory, " + - " stddevtestsrun = excluded.stddevtestsrun, " + - " stddevmemory = excluded.stddevmemory"); - sqlFragmentUpdate.add(details[0].getUserid()); - sqlFragmentUpdate.add(getContainer().getEntityId()); - sqlFragmentUpdate.add(details[0].getUserid()); - sqlFragmentUpdate.add(getContainer().getEntityId()); - new SqlExecutor(scope).execute(sqlFragmentUpdate); + // Refresh this user's training statistics from their remaining training runs. + recomputeUserData(scope, run.getUserid(), getContainer()); transaction.commit(); } return new ApiSimpleResponse(KEY_SUCCESS, true); @@ -990,14 +1049,39 @@ public Object execute(RunIdForm form, BindException errors) } int rowId = form.getRunId(); + // Look up the run (scoped to this folder) before deleting, both to reject a run in + // another folder and to refresh the owner's training stats afterward: the run may + // have been part of their training set, so their baseline can change (or disappear) + // once it is gone. + RunDetail run = getRunInContainer(rowId, getContainer()); + if (run == null) + { + response.put(KEY_SUCCESS, false); + response.put("error", "run does not exist: " + rowId); + return response; + } + // Child tables keyed by testrunid -> testruns(id). SimpleFilter filter = new SimpleFilter(); filter.addCondition(FieldKey.fromParts("testrunid"), rowId); - try (DbScope.Transaction transaction = TestResultsSchema.getSchema().getScope().ensureTransaction()) { + // trainruns keys the run via its "runid" column, not "testrunid". + SimpleFilter trainFilter = new SimpleFilter(); + trainFilter.addCondition(FieldKey.fromParts("runid"), rowId); + DbScope scope = TestResultsSchema.getSchema().getScope(); + try (DbScope.Transaction transaction = scope.ensureTransaction()) { + // Delete every child row that references this run before the parent, or the + // testruns delete fails on a foreign key constraint. handleleaks and trainruns + // were previously missed: deleting a run that had handle-leak records or was in + // the training set failed (e.g. fk_memoryleaks_testruns on table handleleaks). Table.delete(TestResultsSchema.getTableInfoTestFails(), filter); Table.delete(TestResultsSchema.getTableInfoTestPasses(), filter); Table.delete(TestResultsSchema.getTableInfoMemoryLeaks(), filter); + Table.delete(TestResultsSchema.getTableInfoHandleLeaks(), filter); Table.delete(TestResultsSchema.getTableInfoHangs(), filter); + Table.delete(TestResultsSchema.getTableInfoTrain(), trainFilter); Table.delete(TestResultsSchema.getTableInfoTestRuns(), rowId); // delete run last because of foreign key + // Refresh the owner's training stats now that the run (and any trainruns row for + // it) is gone, so a removed training run does not leave a stale UserData row behind. + recomputeUserData(scope, run.getUserid(), getContainer()); transaction.commit(); } catch (Exception x) { response.put(KEY_SUCCESS, false); @@ -1039,10 +1123,9 @@ public Object execute(FlagRunForm form, BindException errors) int rowId = form.getRunId(); boolean flag = form.getFlag() != null ? form.getFlag() : false; - SimpleFilter filter = new SimpleFilter(); - filter.addCondition(FieldKey.fromParts("id"), rowId); try (DbScope.Transaction transaction = TestResultsSchema.getSchema().getScope().ensureTransaction()) { - RunDetail detail = new TableSelector(TestResultsSchema.getTableInfoTestRuns(), filter, null).getObject(RunDetail.class); + // Scoped to this folder so a run in another folder cannot be flagged from here. + RunDetail detail = getRunInContainer(rowId, getContainer()); if (detail == null) { response.put(KEY_SUCCESS, false); @@ -1096,8 +1179,11 @@ public static class ShowFlaggedAction extends SimpleViewAction @Override public ModelAndView getView(Object o, BindException errors) throws Exception { + // Scoped to this folder so the Flags page lists only flagged runs in the current folder, + // not flagged runs across every folder. SimpleFilter filter = new SimpleFilter(); filter.addCondition(FieldKey.fromParts("flagged"), true); + filter.addCondition(FieldKey.fromParts("container"), getContainer().getEntityId()); RunDetail[] details = new TableSelector(TestResultsSchema.getTableInfoTestRuns(), filter, null).getArray(RunDetail.class); JspView view = new JspView<>("/org/labkey/testresults/view/flagged.jsp", new TestsDataBean(details, new User[0])); view.setFrame(WebPartView.FrameType.PORTAL); @@ -1196,9 +1282,11 @@ public Object execute(RunIdForm form, BindException errors) if (form.getRunId() == null) return new ApiSimpleResponse("log", null); int runId = form.getRunId(); + // Scoped to this folder so a run's log in another folder cannot be read by id. SQLFragment sqlFragment = new SQLFragment(); - sqlFragment.append("SELECT log FROM testresults.testruns WHERE id = ?"); + sqlFragment.append("SELECT log FROM testresults.testruns WHERE id = ? AND container = ?"); sqlFragment.add(runId); + sqlFragment.add(getContainer().getEntityId()); List logs = new ArrayList<>(); SqlSelector sqlSelector = new SqlSelector(TestResultsSchema.getSchema(), sqlFragment); sqlSelector.forEach(rs -> logs.add(rs.getBytes("log"))); @@ -1217,9 +1305,11 @@ public Object execute(RunIdForm form, BindException errors) if (form.getRunId() == null) return new ApiSimpleResponse("xml", null); int runId = form.getRunId(); + // Scoped to this folder so a run's XML in another folder cannot be read by id. SQLFragment sqlFragment = new SQLFragment(); - sqlFragment.append("SELECT xml FROM testresults.testruns WHERE id = ?"); + sqlFragment.append("SELECT xml FROM testresults.testruns WHERE id = ? AND container = ?"); sqlFragment.add(runId); + sqlFragment.add(getContainer().getEntityId()); List xmls = new ArrayList<>(); SqlSelector sqlSelector = new SqlSelector(TestResultsSchema.getSchema(), sqlFragment); sqlSelector.forEach(rs -> xmls.add(rs.getBytes("xml"))); @@ -1963,7 +2053,7 @@ private static void ParseAndStoreXML(String xml, Container c) throws Exception timestampDay = addDays(timestampDay, 1); } lastHour = hour; - String originalDate = MDYFormat.format(timestampDay); + String originalDate = mdyFormat().format(timestampDay); try { timestamp = new SimpleDateFormat("MM/dd/yyyy HH:mm").parse(originalDate + " " + ts); } catch (IllegalArgumentException e) { @@ -2015,7 +2105,7 @@ private static void ParseAndStoreXML(String xml, Container c) throws Exception timestampDay = addDays(timestampDay, 1); } lastHour = hour; - String originalDate = MDYFormat.format(timestampDay); + String originalDate = mdyFormat().format(timestampDay); try { timestamp = new SimpleDateFormat("MM/dd/yyyy HH:mm").parse(originalDate + " " + ts); } catch (IllegalArgumentException e) { diff --git a/testresults/src/org/labkey/testresults/view/failureDetail.jsp b/testresults/src/org/labkey/testresults/view/failureDetail.jsp index 3d675525..273f8f35 100644 --- a/testresults/src/org/labkey/testresults/view/failureDetail.jsp +++ b/testresults/src/org/labkey/testresults/view/failureDetail.jsp @@ -266,6 +266,19 @@ $(document).ready(function() { const problemData = <%=json(problemData, 0)%>; // Generate date chart. + // Only set explicit timeseries min/max when there are dates to plot. With an empty + // dates array, dates[0] and dates[length - 1] are undefined and c3/d3 throws while + // building the x-axis scale (e.g. when the active date range excludes all sample runs). + const chartDates = problemData.graphData.dates; + const xAxis = { + type: 'timeseries', + localtime: false, + tick: { fit: true, format: '%m/%d' } + }; + if (chartDates.length > 0) { + xAxis.min = chartDates[0]; + xAxis.max = chartDates[chartDates.length - 1]; + } let dateChart = c3.generate({ bindto: '#failGraph', data: { @@ -277,13 +290,7 @@ $(document).ready(function() { bar: { width: { ratio: 0.3 } }, subchart: { show: false, size: { height: 20 } }, axis: { - x: { - min: problemData.graphData.dates[0], - max: problemData.graphData.dates[problemData.graphData.dates.length - 1], - type: 'timeseries', - localtime: false, - tick: { fit: true, format: '%m/%d' } - }, + x: xAxis, // y: { tick: { values: %=graphYTicks.getJavaScriptFragment(0)% } } } }); diff --git a/testresults/test/src/org/labkey/test/tests/testresults/TestResultsTest.java b/testresults/test/src/org/labkey/test/tests/testresults/TestResultsTest.java index 49ee3345..1b1c37b9 100644 --- a/testresults/test/src/org/labkey/test/tests/testresults/TestResultsTest.java +++ b/testresults/test/src/org/labkey/test/tests/testresults/TestResultsTest.java @@ -28,6 +28,7 @@ import org.junit.experimental.categories.Category; import org.labkey.remoteapi.CommandException; import org.labkey.remoteapi.Connection; +import org.labkey.remoteapi.query.Filter; import org.labkey.remoteapi.query.SelectRowsCommand; import org.labkey.remoteapi.query.SelectRowsResponse; import org.labkey.remoteapi.query.Sort; @@ -50,10 +51,13 @@ import java.time.format.DateTimeFormatter; import java.util.List; import java.util.Map; +import java.util.Set; +import java.util.stream.Collectors; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotEquals; +import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; @Category({External.class, MacCossLabModules.class}) @@ -64,6 +68,10 @@ public class TestResultsTest extends BaseWebDriverTest implements PostgresOnlyTe static final String COMPUTER_NAME_1 = "TEST-PC-1"; static final String COMPUTER_NAME_2 = "TEST-PC-2"; + // Training stats are exact (avg/stddev of integer averagemem), so this only absorbs the + // floating round-trip through the query API. It is not a real tolerance. + private static final double EPSILON = 1e-6; + private static final Locator SUBMIT_BUTTON = Locator.css("input[type='submit'][value='Submit']"); // XPath for the problems matrix table (header cell contains "Fail: | Leak: | Hang:") @@ -466,19 +474,18 @@ public void testTrainingDataPage() toggleTrainingSet(); assertTextPresent("Add to training set"); - // After removal: the Remove link and the run's data row are gone, and - // the stats row now shows "RunCount:0". TEST-PC-1's section is still - // rendered in the training table though, because of a known bug in - // TrainRunAction: when the user's last training run is removed, the - // the stale UserData row (non-zero meanmemory / meantestsrun) is - // never cleared. trainingdata.jsp:158 only moves users to the - // "No Training Data --" list when both fields are 0, so the section - // stays. See TODO-LK-20260403_testresults-bugs.md. + // After removing the user's last training run, TrainRunAction deletes their + // UserData row, so TEST-PC-1 no longer has any training data: its section + // disappears from and the user moves to the + // "No Training Data --" list (it now behaves like TEST-PC-2). Previously a + // stale UserData row left a lingering RunCount:0 section here. clickAndWait(Locator.linkWithText("Training Data")); assertElementNotPresent(removeLink); assertElementNotPresent(trainingTable.containing("2026-01-16 06:00")); - assertElementPresent(statsRow.containing("RunCount:0")); - assertTextPresent(COMPUTER_NAME_2, "No Training Data --"); + assertElementNotPresent(trainingTable.descendant( + Locator.tagWithId("tr", "user-anchor-" + COMPUTER_NAME_1))); + assertElementNotPresent(statsRow); + assertTextPresent(COMPUTER_NAME_1, COMPUTER_NAME_2, "No Training Data --"); } @Test @@ -598,6 +605,14 @@ public void testApiErrorResponses() assertFalse(missingRun.optBoolean("Success", true)); assertEquals("run does not exist: 999999", missingRun.optString("error")); + // TrainRunAction: force path also requires the run to exist. The existence check runs + // before the recompute regardless of force, so a bad runId reports not-found instead of + // throwing on an empty lookup. + JSONObject forceMissingRun = postApi("trainRun", + Map.of("runId", "999999", "train", "force")); + assertFalse(forceMissingRun.optBoolean("Success", true)); + assertEquals("run does not exist: 999999", forceMissingRun.optString("error")); + // SetUserActive: missing userId JSONObject noUserId = postApi("setUserActive", Map.of("active", "true")); assertEquals("userId is required", noUserId.optString("Message")); @@ -640,6 +655,13 @@ public void testInvalidDateParameters() beginAt(WebTestHelper.buildRelativeUrl("testresults", PROJECT_NAME, "showFailures", Map.of("end", "03-24-2026"))); assertTextPresent("Invalid date format: 03-24-2026 (expected MM/dd/yyyy)"); + + // Strict parsing: an out-of-range but MM/dd/yyyy-shaped date like 13/45/2026 used to + // silently roll over to a valid date (02/14/2027) because the shared SimpleDateFormat + // was lenient. It must now be rejected like any other invalid date. + beginAt(WebTestHelper.buildRelativeUrl("testresults", PROJECT_NAME, "begin", + Map.of("end", "13/45/2026"))); + assertTextPresent("Invalid date format: 13/45/2026 (expected MM/dd/yyyy)"); } @Test @@ -666,6 +688,144 @@ public void testDeleteRun() assertTextNotPresent("TestDisposableOne"); } + @Test + public void testDeleteRunWithChildRecordsRecomputesTraining() + { + // Post a fresh run that has both handle-leak and memory-leak child rows, then add it + // to the training set. Deleting it must (1) succeed despite the handleleaks and + // trainruns child rows that previously caused a foreign key violation, and (2) refresh + // the user's training stats so no stale UserData row is left behind. + int baselineUserData = userDataRowCount(); + + int runId = postAndGetNewRunId("testresults/pc1-run-0117-leaks.xml", COMPUTER_NAME_1); + assertTrue("Posted run should have handle-leak child rows", childRowCount("handleleaks", runId) > 0); + assertTrue("Posted run should have memory-leak child rows", childRowCount("memoryleaks", runId) > 0); + + JSONObject trainResp = postApi("trainRun", Map.of("runId", String.valueOf(runId), "train", "true")); + assertTrue("trainRun should succeed: " + trainResp, trainResp.optBoolean("Success", false)); + assertTrue("A UserData row should exist after adding a training run", userDataRowCount() >= 1); + + // Without the fix this returned Success=false with a foreign key violation on + // handleleaks (and would also fail for the trainruns row). + JSONObject deleteResp = postApi("deleteRun", Map.of("runId", String.valueOf(runId))); + assertTrue("deleteRun should succeed without a foreign key violation: " + deleteResp, + deleteResp.optBoolean("Success", false)); + + // Child rows are cleaned up, and the training stats are refreshed: this was the run's + // only training run, so its UserData row is removed rather than left stale. + assertEquals("handleleaks child rows should be deleted", 0, childRowCount("handleleaks", runId)); + assertEquals("memoryleaks child rows should be deleted", 0, childRowCount("memoryleaks", runId)); + assertEquals("testpasses child rows should be deleted", 0, childRowCount("testpasses", runId)); + assertEquals("Deleting the only training run should remove its UserData row", + baselineUserData, userDataRowCount()); + } + + @Test + public void testRemoveTrainingRunRecomputesStatsWhenRunsRemain() + { + // Covers the recomputeUserData "update" branch: when a user still has training runs + // after one is removed, their UserData row must survive (not be deleted) and its stats + // must be recomputed from the remaining runs. The other tests only exercise the + // "delete" branch (removing a user's only/last training run). + int userId = getUserId(COMPUTER_NAME_1); + int baselineUserData = userDataRowCount(); + double cleanMem = runAverageMem(_cleanRunId); + double leakMem = runAverageMem(_leakRunId); + + try + { + // Add two TEST-PC-1 runs to the training set; both share one UserData row whose + // mean memory is the average of the two runs. + assertTrainRun(_cleanRunId, "true"); + assertTrainRun(_leakRunId, "true"); + assertEquals("Both training runs should share one UserData row", + baselineUserData + 1, userDataRowCount()); + assertEquals("Mean memory should be the average of both training runs", + (cleanMem + leakMem) / 2, userDataMeanMemory(userId), EPSILON); + // Population stddev of two values {a, b} is |a - b| / 2. + assertEquals("Stddev memory should reflect both training runs", + Math.abs(cleanMem - leakMem) / 2, userDataStdDevMemory(userId), EPSILON); + + // Remove one run: the row must remain (update branch, not delete) and its stats + // must be recomputed from the single remaining run. + assertTrainRun(_leakRunId, "false"); + assertEquals("Removing one of two training runs must not delete the UserData row", + baselineUserData + 1, userDataRowCount()); + assertEquals("Mean memory should be recomputed from the remaining run", + cleanMem, userDataMeanMemory(userId), EPSILON); + assertEquals("Stddev memory of a single remaining run should be zero", + 0.0, userDataStdDevMemory(userId), EPSILON); + } + finally + { + // Leave the training set empty for other tests (no-op if already removed). + postApi("trainRun", Map.of("runId", String.valueOf(_cleanRunId), "train", "false")); + postApi("trainRun", Map.of("runId", String.valueOf(_leakRunId), "train", "false")); + } + } + + @Test + public void testRunAccessIsContainerScoped() throws IOException, CommandException + { + // Run ids are global, so the actions that look a run up by id must reject a run that lives + // in another folder. Put a run in a subfolder, then from the PARENT folder confirm that + // every run-by-id action refuses it, and that the run is still reachable from its own folder. + final String subFolder = "CrossFolderAccessTest"; + final String subFolderPath = "/" + PROJECT_NAME + "/" + subFolder; + _containerHelper.createSubfolder(PROJECT_NAME, subFolder); + _containerHelper.enableModule(subFolderPath, "TestResults"); + postSampleXml("testresults/pc1-run-0116-failures.xml", subFolderPath); + + Connection conn = WebTestHelper.getRemoteApiConnection(); + SelectRowsCommand runsCmd = new SelectRowsCommand("testresults", "testruns"); + runsCmd.setColumns(List.of("id")); + SelectRowsResponse subRuns = runsCmd.execute(conn, subFolderPath); + assertEquals("Subfolder should have exactly 1 run", 1, subRuns.getRows().size()); + int subRunId = ((Number) subRuns.getRows().getFirst().get("id")).intValue(); + + // --- Negative: act on the subfolder's run by id from the PARENT folder (PROJECT_NAME) --- + + JSONObject del = postApi("deleteRun", Map.of("runId", String.valueOf(subRunId))); + assertFalse("Cross-folder deleteRun must fail: " + del, del.optBoolean("Success", true)); + assertEquals("run does not exist: " + subRunId, del.optString("error")); + + JSONObject train = postApi("trainRun", Map.of("runId", String.valueOf(subRunId), "train", "true")); + assertFalse("Cross-folder trainRun must fail: " + train, train.optBoolean("Success", true)); + assertEquals("run does not exist: " + subRunId, train.optString("error")); + + JSONObject flag = postApi("flagRun", Map.of("runId", String.valueOf(subRunId), "flag", "true")); + assertFalse("Cross-folder flagRun must fail: " + flag, flag.optBoolean("Success", true)); + assertEquals("run not found: " + subRunId, flag.optString("error")); + + assertNull("Cross-folder viewLog must not return content", + getApiString("viewLog", subRunId, "log")); + assertNull("Cross-folder viewXml must not return content", + getApiString("viewXml", subRunId, "xml")); + + // showRun in the parent folder shows the "enter run ID" prompt, not the subfolder run. + beginAt(WebTestHelper.buildRelativeUrl("testresults", PROJECT_NAME, "showRun", + Map.of("runId", String.valueOf(subRunId)))); + assertElementPresent(Locator.css("input[name='runId']")); + + // The run is untouched in its own folder (the cross-folder delete was a no-op there). + assertEquals("Subfolder run must be untouched", 1, + runsCmd.execute(conn, subFolderPath).getRows().size()); + + // --- Positive: the same run IS reachable from its own folder --- + // (deleteRun/trainRun/viewLog/viewXml in-folder are covered by the other tests, which all + // run in PROJECT_NAME and would fail if the container guard rejected same-folder access.) + JSONObject flagOwn = postApi("flagRun", + Map.of("runId", String.valueOf(subRunId), "flag", "true"), subFolderPath); + assertTrue("In-folder flagRun should succeed: " + flagOwn, flagOwn.optBoolean("Success", false)); + + // ShowFlaggedAction is folder-scoped: the flagged subfolder run shows on the subfolder's + // Flags page but not the parent's. + beginAt(WebTestHelper.buildRelativeUrl("testresults", subFolderPath, "showFlagged")); + assertElementPresent(Locator.tag("a").startsWith("id: " + subRunId + " /")); + beginAt(WebTestHelper.buildRelativeUrl("testresults", PROJECT_NAME, "showFlagged")); + assertElementNotPresent(Locator.tag("a").startsWith("id: " + subRunId + " /")); + } + /** * Verifies that child tables in the testresults schema are container-filtered * via a join to testruns. Creates a subfolder, posts a single run into it, @@ -1001,6 +1161,142 @@ private JSONObject postApi(String action, Map params, String con } } + /** + * Posts a sample XML run and returns the id of the run it created, identified as the + * single new testruns row for the given computer (compared against the runs present + * before the post). + */ + private int postAndGetNewRunId(String sampleDataRelativePath, String computerName) + { + Set before = runIdsForComputer(computerName); + postSampleXml(sampleDataRelativePath); + Set after = runIdsForComputer(computerName); + after.removeAll(before); + assertEquals("Expected exactly one new run for " + computerName, 1, after.size()); + return after.iterator().next(); + } + + /** + * Returns the set of testruns ids posted by the given computer in this container. + */ + private Set runIdsForComputer(String computerName) + { + return queryRuns().stream() + .filter(r -> computerName.equals(r.get("userid/username"))) + .map(r -> (Integer) r.get("id")) + .collect(Collectors.toSet()); + } + + /** + * Counts rows in a testresults child table that reference the given run via testrunid. + */ + private int childRowCount(String table, int runId) + { + try + { + Connection connection = WebTestHelper.getRemoteApiConnection(); + SelectRowsCommand cmd = new SelectRowsCommand("testresults", table); + cmd.addFilter(new Filter("testrunid", runId)); + return cmd.execute(connection, PROJECT_NAME).getRows().size(); + } + catch (Exception e) + { + throw new RuntimeException("Failed to count rows in " + table + " for run " + runId, e); + } + } + + /** + * Counts UserData (training stats) rows in this container. + */ + private int userDataRowCount() + { + try + { + Connection connection = WebTestHelper.getRemoteApiConnection(); + SelectRowsCommand cmd = new SelectRowsCommand("testresults", "userdata"); + return cmd.execute(connection, PROJECT_NAME).getRows().size(); + } + catch (Exception e) + { + throw new RuntimeException("Failed to count userdata rows", e); + } + } + + /** + * Posts trainRun for the given run and asserts it succeeded. {@code train} is "true" to add + * to the training set or "false" to remove. + */ + private void assertTrainRun(int runId, String train) + { + JSONObject resp = postApi("trainRun", Map.of("runId", String.valueOf(runId), "train", train)); + assertTrue("trainRun(" + runId + ", " + train + ") should succeed: " + resp, + resp.optBoolean("Success", false)); + } + + /** + * Returns the testresults.user id for the given computer name. + */ + private int getUserId(String computerName) + { + List> rows = selectRows("user", + new Filter("username", computerName), "id"); + assertEquals("Expected exactly one user row for " + computerName, 1, rows.size()); + return ((Number) rows.get(0).get("id")).intValue(); + } + + /** + * Returns the stored average managed memory for a run. + */ + private double runAverageMem(int runId) + { + List> rows = selectRows("testruns", + new Filter("id", runId), "averagemem"); + assertEquals("Expected exactly one testruns row for run " + runId, 1, rows.size()); + return ((Number) rows.get(0).get("averagemem")).doubleValue(); + } + + /** + * Returns the recomputed mean memory from the single UserData row for the given user. + */ + private double userDataMeanMemory(int userId) + { + List> rows = selectRows("userdata", + new Filter("userid", userId), "meanmemory"); + assertEquals("Expected exactly one userdata row for user " + userId, 1, rows.size()); + return ((Number) rows.get(0).get("meanmemory")).doubleValue(); + } + + /** + * Returns the recomputed population stddev of memory from the single UserData row for the + * given user. + */ + private double userDataStdDevMemory(int userId) + { + List> rows = selectRows("userdata", + new Filter("userid", userId), "stddevmemory"); + assertEquals("Expected exactly one userdata row for user " + userId, 1, rows.size()); + return ((Number) rows.get(0).get("stddevmemory")).doubleValue(); + } + + /** + * Runs a filtered SelectRows against a testresults query, returning the selected columns. + */ + private List> selectRows(String queryName, Filter filter, String... columns) + { + try + { + Connection connection = WebTestHelper.getRemoteApiConnection(); + SelectRowsCommand cmd = new SelectRowsCommand("testresults", queryName); + cmd.addFilter(filter); + cmd.setColumns(List.of(columns)); + return cmd.execute(connection, PROJECT_NAME).getRows(); + } + catch (Exception e) + { + throw new RuntimeException("Failed to query " + queryName, e); + } + } + // ------------------------------------------------------------------------- // Infrastructure // -------------------------------------------------------------------------