diff --git a/panoramapublic/src/org/labkey/panoramapublic/PanoramaPublicController.java b/panoramapublic/src/org/labkey/panoramapublic/PanoramaPublicController.java index 0f5abaf0..cef3ae75 100644 --- a/panoramapublic/src/org/labkey/panoramapublic/PanoramaPublicController.java +++ b/panoramapublic/src/org/labkey/panoramapublic/PanoramaPublicController.java @@ -32,6 +32,7 @@ import org.labkey.api.action.FormHandlerAction; import org.labkey.api.action.FormViewAction; import org.labkey.api.action.LabKeyError; +import org.labkey.api.action.MutatingApiAction; import org.labkey.api.action.ReadOnlyApiAction; import org.labkey.api.action.ReturnUrlForm; import org.labkey.api.action.SimpleErrorView; @@ -11219,34 +11220,47 @@ public static ActionURL getCatalogImageDownloadUrl(ExperimentAnnotations expAnno // ======================== Support actions for Selenium tests ======================== + // These actions swap the process-wide NCBI publication search service to a mock so that Selenium tests + // run without calling the live NCBI API. They must never be reachable on a production server (non-dev-mode) + private static void requireDevModeForMockNcbiService() + { + if (!AppProps.getInstance().isDevMode()) + { + throw new NotFoundException("Mock NCBI publication search service actions are only available on a server running in dev mode."); + } + } + @RequiresSiteAdmin - public static class SetupMockNcbiServiceAction extends ReadOnlyApiAction + public static class SetupMockNcbiServiceAction extends MutatingApiAction { @Override public Object execute(Object form, BindException errors) { + requireDevModeForMockNcbiService(); NcbiPublicationSearchServiceImpl.setInstance(new MockNcbiPublicationSearchService()); return new ApiSimpleResponse("mock", true); } } @RequiresSiteAdmin - public static class RestoreNcbiServiceAction extends ReadOnlyApiAction + public static class RestoreNcbiServiceAction extends MutatingApiAction { @Override public Object execute(Object form, BindException errors) { + requireDevModeForMockNcbiService(); NcbiPublicationSearchServiceImpl.setInstance(new NcbiPublicationSearchServiceImpl()); return new ApiSimpleResponse("restored", true); } } @RequiresSiteAdmin - public static class RegisterMockPublicationAction extends ReadOnlyApiAction + public static class RegisterMockPublicationAction extends MutatingApiAction { @Override public Object execute(RegisterMockPublicationForm form, BindException errors) { + requireDevModeForMockNcbiService(); NcbiPublicationSearchService service = NcbiPublicationSearchServiceImpl.getInstance(); if (!(service instanceof MockNcbiPublicationSearchService mock)) { diff --git a/panoramapublic/src/org/labkey/panoramapublic/proteomexchange/ExperimentModificationGetter.java b/panoramapublic/src/org/labkey/panoramapublic/proteomexchange/ExperimentModificationGetter.java index 95580587..aafb838d 100644 --- a/panoramapublic/src/org/labkey/panoramapublic/proteomexchange/ExperimentModificationGetter.java +++ b/panoramapublic/src/org/labkey/panoramapublic/proteomexchange/ExperimentModificationGetter.java @@ -345,8 +345,6 @@ public PxIsotopicMod(String skylineName, long dbModId, UnimodModification uMod, public static class TestCase extends Assert { - private static final boolean debug = false; - @Test public void testBuildIsotopeModFormula() { @@ -701,8 +699,6 @@ public void testStructuralMods() throws IOException { PxModification pxMod = getStructuralUnimodMod(mod, uMods); - printStructuralMod(mod, pxMod); - List matches = unimodMatches.get(pxMod.getSkylineName()); if (matches.isEmpty()) { @@ -727,32 +723,6 @@ else if (matches.size() == 1) } } - private void printStructuralMod(Modification mod, PxModification pxMod) - { - if (!debug) - { - return; - } - - if (pxMod.hasUnimodId()) - { - String term = mod.getTerminus() == null ? "" : (mod.getTerminus().equals("N") ? "N-term" : "C-term"); - String modInfo = pxMod.getSkylineName() + ", " + Formula.normalizeFormula(mod.getFormula()) + ", " + mod.getAminoAcid() + ", TERM: " + term; - System.out.print("Skyline: " + modInfo); - System.out.println(" --- " + pxMod.getUnimodId() + ", " + pxMod.getName()); - } - if (pxMod.hasPossibleUnimods()) - { - String term = mod.getTerminus() == null ? "" : (mod.getTerminus().equals("N") ? "N-term" : "C-term"); - String modInfo = pxMod.getSkylineName() + ", " + Formula.normalizeFormula(mod.getFormula()) + ", " + mod.getAminoAcid() + ", TERM: " + term; - System.out.println("Skyline: " + modInfo); - for (UnimodModification umod: pxMod.getPossibleUnimodMatches()) - { - System.out.println(" --- List.of(new UnimodModification(" + umod.getId() + ", \"" + umod.getName() + "\", null))"); - } - } - } - @Test public void testIsotopicMods() throws IOException { @@ -899,8 +869,6 @@ public void testIsotopicMods() throws IOException } PxModification pxMod = getIsotopicUnimodMod(mod, uMods); - printIsotopicMod(mod, pxMod); - List expectedMatches = matches.get(pxMod.getSkylineName()); if (expectedMatches.isEmpty()) @@ -926,31 +894,6 @@ else if (expectedMatches.size() > 1) } } - private void printIsotopicMod(IsotopeModification mod, PxModification pxMod) - { - if (!debug) - { - return; - } - if (pxMod.hasUnimodId()) - { - String term = mod.getTerminus() == null ? "" : (mod.getTerminus().equals("N") ? "N-term" : "C-term"); - String modInfo = pxMod.getSkylineName() + ", " + Formula.normalizeFormula(mod.getFormula()) + ", " + mod.getAminoAcid() + ", TERM: " + term; - System.out.print("Skyline: " + modInfo); - System.out.println(" --- " + pxMod.getUnimodId() + ", " + pxMod.getName()); - } - if (pxMod.hasPossibleUnimods()) - { - String term = mod.getTerminus() == null ? "" : (mod.getTerminus().equals("N") ? "N-term" : "C-term"); - String modInfo = pxMod.getSkylineName() + ", " + Formula.normalizeFormula(mod.getFormula()) + ", " + mod.getAminoAcid() + ", TERM: " + term; - System.out.println("Skyline: " + modInfo); - for (UnimodModification umod: pxMod.getPossibleUnimodMatches()) - { - System.out.println(" --- List.of(new UnimodModification(" + umod.getId() + ", \"" + umod.getName() + "\", null))"); - } - } - } - private Modification createMod(String name, String formula, String sites, String terminus) { Modification mod = new Modification(); diff --git a/panoramapublic/test/src/org/labkey/test/tests/panoramapublic/PublicationSearchTest.java b/panoramapublic/test/src/org/labkey/test/tests/panoramapublic/PublicationSearchTest.java index ef7e4eeb..555a193e 100644 --- a/panoramapublic/test/src/org/labkey/test/tests/panoramapublic/PublicationSearchTest.java +++ b/panoramapublic/test/src/org/labkey/test/tests/panoramapublic/PublicationSearchTest.java @@ -20,7 +20,7 @@ import org.junit.experimental.categories.Category; import org.labkey.remoteapi.CommandException; import org.labkey.remoteapi.Connection; -import org.labkey.remoteapi.SimpleGetCommand; +import org.labkey.remoteapi.SimplePostCommand; import org.labkey.remoteapi.query.Filter; import org.labkey.remoteapi.query.SelectRowsCommand; import org.labkey.remoteapi.query.SelectRowsResponse; @@ -356,7 +356,7 @@ private void initMockNcbiService() try { Connection connection = createDefaultConnection(); - SimpleGetCommand command = new SimpleGetCommand("panoramapublic", "setupMockNcbiService"); + SimplePostCommand command = new SimplePostCommand("panoramapublic", "setupMockNcbiService"); command.execute(connection, "/"); _useMockNcbi = true; log("Using mock NCBI service"); @@ -432,7 +432,7 @@ private void registerMockPublication(String database, String id, String searchKe try { Connection connection = createDefaultConnection(); - SimpleGetCommand command = new SimpleGetCommand("panoramapublic", "registerMockPublication"); + SimplePostCommand command = new SimplePostCommand("panoramapublic", "registerMockPublication"); Map params = new HashMap<>(); params.put("database", database); params.put("id", id); @@ -462,7 +462,7 @@ private void restoreNcbiService() try { Connection connection = createDefaultConnection(); - SimpleGetCommand command = new SimpleGetCommand("panoramapublic", "restoreNcbiService"); + SimplePostCommand command = new SimplePostCommand("panoramapublic", "restoreNcbiService"); command.execute(connection, "/"); log("Restored real NCBI service"); } diff --git a/testresults/src/org/labkey/testresults/SendTestResultsEmail.java b/testresults/src/org/labkey/testresults/SendTestResultsEmail.java index 51ce62d9..82e4c1aa 100644 --- a/testresults/src/org/labkey/testresults/SendTestResultsEmail.java +++ b/testresults/src/org/labkey/testresults/SendTestResultsEmail.java @@ -238,7 +238,7 @@ public Pair getHTMLEmail(org.labkey.api.security.User from) .append("\n" + run.getDuration() + (run.getHang() != null ? " (hang)" : "") + "") .append("\n" + run.getFailedtests() + "") .append("\n" + run.getLeaks().length + "") - .append("\n " + run.getGitHash() + "") + .append("\n " + PageFlowUtil.filter(run.getGitHash()) + "") .append(""); } } @@ -252,7 +252,7 @@ public Pair getHTMLEmail(org.labkey.api.security.User from) for (User u : missingUsers) { message.append("") - .append("\nMissing " + u.getUsername() + "") + .append("\nMissing " + PageFlowUtil.filter(u.getUsername()) + "") .append(""); } message.append("") @@ -282,13 +282,13 @@ public Pair getHTMLEmail(org.labkey.api.security.User from) ""); RunDetail[] problemRuns = problems.getRuns(); for (RunDetail run : problemRuns) - message.append("\n" + run.getUserName() + ""); + message.append("\n" + PageFlowUtil.filter(run.getUserName()) + ""); message.append("\n"); for (String test : problems.getTestNames()) { message.append("\n") - .append("\n" + test + ""); + .append("\n" + PageFlowUtil.filter(test) + ""); for (RunDetail run : problemRuns) { message.append("\n"); diff --git a/testresults/src/org/labkey/testresults/TestResultsController.java b/testresults/src/org/labkey/testresults/TestResultsController.java index 995e9380..9c140539 100644 --- a/testresults/src/org/labkey/testresults/TestResultsController.java +++ b/testresults/src/org/labkey/testresults/TestResultsController.java @@ -69,7 +69,6 @@ import org.labkey.api.view.JspView; import org.labkey.api.view.NavTree; import org.labkey.api.view.WebPartView; -import org.labkey.testresults.model.GlobalSettings; import org.labkey.testresults.model.RunDetail; import org.labkey.testresults.model.TestFailDetail; import org.labkey.testresults.model.TestHandleLeakDetail; @@ -451,7 +450,7 @@ public static void ensureRunDataCached(RunDetail[] runs, boolean keepObjData) { } catch (IOException e) { - e.printStackTrace(); + _log.error("Failed to encode run pass summary", e); } int avgMem = 0; if (passes.length != 0) @@ -1174,7 +1173,7 @@ public void setFlag(Boolean flag) /** * action to show all flagged runs flagged.jsp */ - @RequiresNoPermission + @RequiresPermission(ReadPermission.class) public static class ShowFlaggedAction extends SimpleViewAction { @Override @@ -1229,23 +1228,26 @@ public Object execute(BoundariesForm form, BindException errors) throws Exceptio return new ApiSimpleResponse(res); } - GlobalSettings settings = new GlobalSettings(warningB, errorB); - DbScope.Transaction transaction = TestResultsSchema.getSchema().getScope().ensureTransaction(); - SQLFragment sqlFragment = new SQLFragment(); - sqlFragment.append("select exists(select 1 from " + TestResultsSchema.getTableInfoGlobalSettings() + ") "); - SqlSelector sqlSelector = new SqlSelector(TestResultsSchema.getSchema(), sqlFragment); - List values = new ArrayList<>(); - sqlSelector.forEach(rs -> values.add(rs.getBoolean(1))); - - if (values.getFirst()) { - SQLFragment sqlFragmentDelete = new SQLFragment(); - sqlFragmentDelete.append("DELETE FROM " + TestResultsSchema.getTableInfoGlobalSettings()); - new SqlExecutor(TestResultsSchema.getSchema()).execute(sqlFragmentDelete); + try (DbScope.Transaction transaction = TestResultsSchema.getSchema().getScope().ensureTransaction()) + { + SQLFragment sqlFragment = new SQLFragment(); + sqlFragment.append("select exists(select 1 from " + TestResultsSchema.getTableInfoGlobalSettings() + ") "); + SqlSelector sqlSelector = new SqlSelector(TestResultsSchema.getSchema(), sqlFragment); + List values = new ArrayList<>(); + sqlSelector.forEach(rs -> values.add(rs.getBoolean(1))); + + if (values.get(0)) { + SQLFragment sqlFragmentDelete = new SQLFragment(); + sqlFragmentDelete.append("DELETE FROM " + TestResultsSchema.getTableInfoGlobalSettings()); + new SqlExecutor(TestResultsSchema.getSchema()).execute(sqlFragmentDelete); + } + SQLFragment sqlFragmentInsert = new SQLFragment(); + sqlFragmentInsert.append("INSERT INTO " + TestResultsSchema.getTableInfoGlobalSettings() + " (warningb, errorb) VALUES (?, ?)"); + sqlFragmentInsert.add(warningB); + sqlFragmentInsert.add(errorB); + new SqlExecutor(TestResultsSchema.getSchema()).execute(sqlFragmentInsert); + transaction.commit(); } - SQLFragment sqlFragmentInsert = new SQLFragment(); - sqlFragmentInsert.append("INSERT INTO " + TestResultsSchema.getTableInfoGlobalSettings() + " (warningb, errorb) VALUES (" + warningB + ", " + errorB +")"); - new SqlExecutor(TestResultsSchema.getSchema()).execute(sqlFragmentInsert); - transaction.commit(); res.put("Message", "success!"); return new ApiSimpleResponse(res); } @@ -1780,6 +1782,8 @@ else if (xml.isEmpty()) _log.info("Attempting to save file for a future post attempt"); res.put(KEY_SUCCESS, false); res.put("Message", "Error Parsing XML attempting to save the XML file... " + NIGHTLY_POSTER.SaveXML(file, getContainer())); + // The stack trace is intentionally returned to the caller (SkylineTester) so posting + // failures can be diagnosed from the client without server log access. res.put("Exception", e + NIGHTLY_POSTER.getStackTraceText(e)); return new ApiSimpleResponse(res); } @@ -1849,6 +1853,8 @@ public Object execute(Object o, BindException errors) f.delete(); res.put(f.getName(), "Success!"); } catch (Exception e) { + // The stack trace is intentionally returned to the caller so a failed re-post can be + // diagnosed from the client without server log access. res.put(f.getName(), Arrays.toString(e.getStackTrace())); } } @@ -1898,8 +1904,7 @@ private static String SaveXML(MultipartFile file, Container c) { } catch (IOException e) { - _log.error("Failed to save {}.", fileName); - e.printStackTrace(); + _log.error("Failed to save {}.", fileName, e); return "Failed to save the file."; } return "File saved to system."; diff --git a/testresults/src/org/labkey/testresults/TestResultsModule.java b/testresults/src/org/labkey/testresults/TestResultsModule.java index e8f37063..8819677b 100644 --- a/testresults/src/org/labkey/testresults/TestResultsModule.java +++ b/testresults/src/org/labkey/testresults/TestResultsModule.java @@ -16,6 +16,7 @@ package org.labkey.testresults; +import org.apache.logging.log4j.Logger; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import org.labkey.api.data.Container; @@ -23,6 +24,7 @@ import org.labkey.api.module.ModuleContext; import org.labkey.api.security.Directive; import org.labkey.api.security.SecurityManager; +import org.labkey.api.util.logging.LogHelper; import org.labkey.filters.ContentSecurityPolicyFilter; import org.labkey.api.view.WebPartFactory; import org.quartz.JobKey; @@ -41,6 +43,8 @@ public class TestResultsModule extends DefaultModule { + private static final Logger LOG = LogHelper.getLogger(TestResultsModule.class, "Test results module"); + public static final WebPartFactory _testResultsFactory = new TestResultsWebPart(); public static final String JOB_NAME = "TestResultsEmailTrigger"; public static final String JOB_GROUP = "TestResultsGroup"; @@ -124,7 +128,7 @@ public void startBackgroundThreads() } catch (SchedulerException e) { - e.printStackTrace(); + LOG.error("Failed to start the test results email scheduler", e); } } diff --git a/testresults/src/org/labkey/testresults/TestResultsWebPart.java b/testresults/src/org/labkey/testresults/TestResultsWebPart.java index 173ea0e9..f5726f40 100644 --- a/testresults/src/org/labkey/testresults/TestResultsWebPart.java +++ b/testresults/src/org/labkey/testresults/TestResultsWebPart.java @@ -15,8 +15,10 @@ */ package org.labkey.testresults; +import org.apache.logging.log4j.Logger; import org.jetbrains.annotations.NotNull; import org.labkey.api.data.Container; +import org.labkey.api.util.logging.LogHelper; import org.labkey.api.view.BaseWebPartFactory; import org.labkey.api.view.JspView; import org.labkey.api.view.Portal; @@ -30,6 +32,8 @@ public class TestResultsWebPart extends BaseWebPartFactory { + private static final Logger LOG = LogHelper.getLogger(TestResultsWebPart.class, "Test results web part"); + public TestResultsWebPart() { super("Test Results", true, false, WebPartFactory.LOCATION_BODY); @@ -46,7 +50,7 @@ public WebPartView getWebPartView(@NotNull ViewContext portalCtx, Portal.@Not } catch (ParseException | IOException e) { - e.printStackTrace(); + LOG.error("Failed to build the test results web part data", e); } JspView view = new JspView<>("/org/labkey/testresults/view/rundown.jsp", bean); view.setTitle("Test Results"); diff --git a/testresults/src/org/labkey/testresults/model/RunDetail.java b/testresults/src/org/labkey/testresults/model/RunDetail.java index c76680ee..e649bea4 100644 --- a/testresults/src/org/labkey/testresults/model/RunDetail.java +++ b/testresults/src/org/labkey/testresults/model/RunDetail.java @@ -16,8 +16,10 @@ package org.labkey.testresults.model; import org.jetbrains.annotations.NotNull; +import org.apache.logging.log4j.Logger; import org.labkey.api.data.Container; import org.labkey.api.reader.Readers; +import org.labkey.api.util.logging.LogHelper; import java.io.BufferedReader; import java.io.ByteArrayInputStream; @@ -35,6 +37,8 @@ */ public class RunDetail implements Comparable { + private static final Logger LOG = LogHelper.getLogger(RunDetail.class, "Test results run detail"); + public final static int HANG_MILLISECONDS = 30*60*1000; // 30 minutes private int id; @@ -276,7 +280,7 @@ public static String decode(byte[] bytes) { } catch (IOException e1) { - e1.printStackTrace(); + LOG.error("Error reading gzipped run log", e1); } return out.toString(); } diff --git a/testresults/src/org/labkey/testresults/view/failureDetail.jsp b/testresults/src/org/labkey/testresults/view/failureDetail.jsp index a8fab876..c21ca598 100644 --- a/testresults/src/org/labkey/testresults/view/failureDetail.jsp +++ b/testresults/src/org/labkey/testresults/view/failureDetail.jsp @@ -345,8 +345,8 @@ $(document).ready(function() { headerName = "Failures"; displayFunc = run => { return "
    " + run.failures.map(f => "
  • " + - "Pass " + f.pass.toString() + " (" + f.language + ") ---" + - '
    ' + f.trace + "
    "+ + "Pass " + f.pass.toString() + " (" + LABKEY.Utils.encodeHtml(f.language) + ") ---" + + '
    ' + LABKEY.Utils.encodeHtml(f.trace) + "
    "+ "
  • ").join() + "
"; }; jsonKey = "failures"; diff --git a/testresults/src/org/labkey/testresults/view/runDetail.jsp b/testresults/src/org/labkey/testresults/view/runDetail.jsp index 7574c272..f2d4fdf0 100644 --- a/testresults/src/org/labkey/testresults/view/runDetail.jsp +++ b/testresults/src/org/labkey/testresults/view/runDetail.jsp @@ -63,7 +63,11 @@ var win = window.open("", "Log file", "toolbar=no,location=no,directories=no,status=no,menubar=no,scrollbars=yes,resizable=yes," + "width=800,height=600"); - win.document.write('
' + data + '
'); + // The log/xml is stored run content. Set it as text rather than writing it as HTML so it cannot execute as + // markup in the popup. + var pre = win.document.createElement('pre'); + pre.textContent = data; + win.document.body.appendChild(pre); }; var showLog = function() { $.get('<%=h(new ActionURL(TestResultsController.ViewLogAction.class, c).addParameter("runId", runId))%>', csrf_header, diff --git a/testresults/src/org/labkey/testresults/view/trainingdata.jsp b/testresults/src/org/labkey/testresults/view/trainingdata.jsp index 25bb1dea..a5eebafb 100644 --- a/testresults/src/org/labkey/testresults/view/trainingdata.jsp +++ b/testresults/src/org/labkey/testresults/view/trainingdata.jsp @@ -305,7 +305,16 @@ var win = window.open("", data.subject, "toolbar=no,location=no,directories=no,status=no,menubar=no,scrollbars=yes,resizable=yes," + "width=800,height=600"); - win.document.write(data.HTML); + // data.HTML is a rendered email containing stored run/user/test names. Render it in a sandboxed iframe + // (no allow-scripts, opaque origin) so any markup it contains is displayed but cannot execute as script. + var iframe = win.document.createElement('iframe'); + iframe.setAttribute('sandbox', ''); + // Use viewport height: a percentage height would resolve against the popup body + // (which has no defined height) and collapse the iframe to zero, showing nothing. + iframe.style.cssText = 'border:0;width:100%;height:100vh;display:block;'; + iframe.srcdoc = data.HTML; + win.document.body.style.margin = '0'; + win.document.body.appendChild(iframe); }, "json") }); 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 1b1c37b9..720710a2 100644 --- a/testresults/test/src/org/labkey/test/tests/testresults/TestResultsTest.java +++ b/testresults/test/src/org/labkey/test/tests/testresults/TestResultsTest.java @@ -516,6 +516,28 @@ public void testViewXml() assertFalse("XML should not contain Log element (stripped before storage)", xmlContent.contains("")); } + @Test + public void testViewLogPopup() + { + // Verify the popup actually displays the stored log. + navigateToRunById(_cleanRunId); + click(Locator.lkButton("View Log")); + switchToWindow(1); + try + { + waitForElement(Locator.tag("pre")); + String popupText = getText(Locator.tag("pre")); + assertTrue("Log popup should show the nightly header, was: " + popupText, + popupText.contains("# Nightly started Thursday, January 15, 2026 9:00 PM")); + assertTrue("Log popup should show test entries", popupText.contains("TestAlpha")); + } + finally + { + closeExtraWindows(); + switchToMainWindow(); + } + } + @Test public void testChangeBoundaries() {