diff --git a/panoramapublic/src/org/labkey/panoramapublic/PanoramaPublicController.java b/panoramapublic/src/org/labkey/panoramapublic/PanoramaPublicController.java index e04e7e79..3dc17ea5 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; @@ -11233,34 +11234,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 19253a17..4cde5676 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 93e7b3bd..fcd7314c 100644 --- a/panoramapublic/test/src/org/labkey/test/tests/panoramapublic/PublicationSearchTest.java +++ b/panoramapublic/test/src/org/labkey/test/tests/panoramapublic/PublicationSearchTest.java @@ -5,7 +5,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; @@ -341,7 +341,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"); @@ -417,7 +417,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); @@ -447,7 +447,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 f406ed23..2fac3463 100644 --- a/testresults/src/org/labkey/testresults/SendTestResultsEmail.java +++ b/testresults/src/org/labkey/testresults/SendTestResultsEmail.java @@ -223,7 +223,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(""); } } @@ -237,7 +237,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("") @@ -267,13 +267,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 5b99149a..f35bfd32 100644 --- a/testresults/src/org/labkey/testresults/TestResultsController.java +++ b/testresults/src/org/labkey/testresults/TestResultsController.java @@ -68,7 +68,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; @@ -450,7 +449,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) @@ -1173,7 +1172,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 @@ -1228,23 +1227,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.get(0)) { - 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); } @@ -1779,6 +1781,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); } @@ -1848,6 +1852,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())); } } @@ -1890,15 +1896,14 @@ private static String SaveXML(MultipartFile file, Container c) { { File f = makeFile(c, fileName); if(f.exists()) { - _log.info("A file by the name " + fileName + " is already stored."); + _log.info("A file by the name {} is already stored.", fileName); return "File not saved - file already exists in file system."; } file.transferTo(f); } 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 bdd474ec..50af20ad 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; @@ -24,6 +25,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; @@ -42,6 +44,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"; @@ -127,7 +131,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 2a8a9a53..95878a89 100644 --- a/testresults/src/org/labkey/testresults/TestResultsWebPart.java +++ b/testresults/src/org/labkey/testresults/TestResultsWebPart.java @@ -1,7 +1,9 @@ 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; @@ -16,6 +18,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); @@ -32,7 +36,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 8202cdac..83c5cbae 100644 --- a/testresults/src/org/labkey/testresults/model/RunDetail.java +++ b/testresults/src/org/labkey/testresults/model/RunDetail.java @@ -15,8 +15,10 @@ */ package org.labkey.testresults.model; +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; @@ -34,6 +36,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; @@ -275,7 +279,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 b334fd3d..508f4b40 100644 --- a/testresults/src/org/labkey/testresults/view/failureDetail.jsp +++ b/testresults/src/org/labkey/testresults/view/failureDetail.jsp @@ -333,8 +333,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 c4895a38..b1c39187 100644 --- a/testresults/src/org/labkey/testresults/view/runDetail.jsp +++ b/testresults/src/org/labkey/testresults/view/runDetail.jsp @@ -46,7 +46,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 69d4d5cd..86390b17 100644 --- a/testresults/src/org/labkey/testresults/view/trainingdata.jsp +++ b/testresults/src/org/labkey/testresults/view/trainingdata.jsp @@ -288,7 +288,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() {