This is an automated email from the ASF dual-hosted git repository.
jamesbognar pushed a commit to branch release-manager
in repository https://gitbox.apache.org/repos/asf/juneau.git
The following commit(s) were added to refs/heads/release-manager by this push:
new 2f88995b03 SonarCloud cleanup + minor REST/UI updates
2f88995b03 is described below
commit 2f88995b03faddc7788e58de97420f7fec97c714
Author: James Bognar <[email protected]>
AuthorDate: Wed Aug 19 10:01:39 2026 -0400
SonarCloud cleanup + minor REST/UI updates
- rest: extract duplicated literals into constants (ReleaseRest,
CredentialRest); AdminRest/ReleaseRunRest updates
- tests: Mockito static imports, hoist assertThrows setup (S5778), reattach
dangling Javadoc, suppress S5961 on the full-pipeline run
- css: word-break -> overflow-wrap; e2e: node: import prefixes + add paging
assertion
- scripts: harden todo-status-audit.py regexes, reduce complexity, dedupe
literal
---
e2e/playwright.config.ts | 4 +-
e2e/tests/releases.spec.ts | 5 ++
scripts/todo-status-audit.py | 89 ++++++++++++++--------
.../org/apache/juneau/releng/rest/AdminRest.java | 37 +++++----
.../apache/juneau/releng/rest/CredentialRest.java | 15 ++--
.../org/apache/juneau/releng/rest/ReleaseRest.java | 39 +++++-----
.../apache/juneau/releng/rest/ReleaseRunRest.java | 31 +++++++-
src/main/resources/static/css/new-release.css | 2 +-
src/main/resources/templates/new-release.ftlh | 16 ++--
.../apache/juneau/releng/rest/AdminRestTest.java | 2 +-
.../releng/rest/CredentialWriteVectorTest.java | 10 ++-
.../apache/juneau/releng/rest/ReleaseRestTest.java | 17 +++--
.../juneau/releng/rest/ReleaseRunRestTest.java | 44 +++++++++++
13 files changed, 213 insertions(+), 98 deletions(-)
diff --git a/e2e/playwright.config.ts b/e2e/playwright.config.ts
index 3d84997d8c..84b875e24f 100644
--- a/e2e/playwright.config.ts
+++ b/e2e/playwright.config.ts
@@ -1,6 +1,6 @@
import { defineConfig, devices } from '@playwright/test';
-import path from 'path';
-import os from 'os';
+import path from 'node:path';
+import os from 'node:os';
/**
* Dedicated test port for the Spring Boot app this suite boots for itself.
MUST NOT be 8790 — that port is
diff --git a/e2e/tests/releases.spec.ts b/e2e/tests/releases.spec.ts
index 60947eb463..284ea85c71 100644
--- a/e2e/tests/releases.spec.ts
+++ b/e2e/tests/releases.spec.ts
@@ -144,6 +144,11 @@ test.describe('Releases table', () => {
await expect
.poll(async () => dataRows(page).first().innerText())
.not.toBe(firstRowBefore);
+
+ // Explicit assertion (the poll above exists only to retry until
DataTables' redraw settles): the
+ // last page's first row must genuinely differ from the first page's
first row.
+ const firstRowAfter = await dataRows(page).first().innerText();
+ expect(firstRowAfter).not.toBe(firstRowBefore);
});
test('the range segment doubles as a page-size menu button', async ({ page
}) => {
diff --git a/scripts/todo-status-audit.py b/scripts/todo-status-audit.py
index 74452ea123..7472f77c1e 100755
--- a/scripts/todo-status-audit.py
+++ b/scripts/todo-status-audit.py
@@ -97,10 +97,18 @@ DEFAULT_REPO_ROOT = Path(__file__).resolve().parent.parent
FILENAME_RE = re.compile(r"^(TODO|READY|MAYBE)-\d+[a-z]*-.*\.md$")
-STATUS_LINE_RE = re.compile(r"^\s*Current status:\s*(.*)$", re.IGNORECASE |
re.MULTILINE)
-COMPLEXITY_LINE_RE = re.compile(r"^\s*Complexity:\s*(.*)$", re.IGNORECASE |
re.MULTILINE)
-SECTION_HEADING_RE = re.compile(r"^##\s+(.*)$", re.MULTILINE)
-NUMBERED_ITEM_RE = re.compile(r"^\s*\d+[.)]\s+(.*)$", re.MULTILINE)
+# Each trailing capture used to be "\s*(.*)$"/"\s+(.*)$" -- a
mandatory-or-optional whitespace run directly
+# adjacent to a "rest of line" run, both of which can consume the same space
characters. That overlap gives
+# the regex engine multiple equivalent ways to split a run of whitespace
between the two groups, which is
+# super-linear (not just linear) to explore on a failing match. Every caller
below already re-derives the
+# "meaningful" text with .strip() (or a case-insensitive substring test on the
stripped text), so the leading
+# whitespace only ever needs to be consumed ONCE, deterministically -- as a
single non-repeating "\s" where a
+# separator is mandatory (SECTION_HEADING_RE, NUMBERED_ITEM_RE), or dropped
entirely where it was already
+# optional and redundant with the caller's own .strip() (STATUS_LINE_RE,
COMPLEXITY_LINE_RE).
+STATUS_LINE_RE = re.compile(r"^\s*Current status:(.*)$", re.IGNORECASE |
re.MULTILINE)
+COMPLEXITY_LINE_RE = re.compile(r"^\s*Complexity:(.*)$", re.IGNORECASE |
re.MULTILINE)
+SECTION_HEADING_RE = re.compile(r"^##\s(.*)$", re.MULTILINE)
+NUMBERED_ITEM_RE = re.compile(r"^\s*\d+[.)]\s(.*)$", re.MULTILINE)
# Matched on word boundaries. A bare substring test reads "unanswered" /
"unresolved" -- the
# most natural wording for an OPEN question -- as containing "answered" /
"resolved", which
@@ -111,9 +119,10 @@ RESOLVED_MARKER_RE =
re.compile(r"\b(?:resolved|answered|decided)\b")
# Recognized status prefixes (case-insensitive, checked with str.startswith
after lowercasing) per
# the skill's "Status wording rules" -- kept separate from TODO/READY vs MAYBE
since the two file
# families use disjoint wording.
+READY_TO_EXECUTE_STATUS = "ready to execute"
TODO_READY_STATUS_PREFIXES = (
"waiting for user input on open questions",
- "ready to execute",
+ READY_TO_EXECUTE_STATUS,
"in progress",
)
MAYBE_STATUS_PREFIX = "parked"
@@ -168,6 +177,45 @@ def status_prefix_ok(prefix: str, status: str) -> bool:
return any(normalized.startswith(p) for p in TODO_READY_STATUS_PREFIXES)
+def _flag_missing_headers(status_match, complexity_match, flags: list) -> None:
+ """Appends missing_status_header / missing_complexity_header for headers
absent from the file."""
+ if status_match is None:
+ flags.append(("missing_status_header", "No 'Current status:' line
found."))
+ if complexity_match is None:
+ flags.append(("missing_complexity_header", "No 'Complexity:' line
found."))
+
+
+def _flag_status_placement(text: str, status_match, flags: list) -> None:
+ """Appends status_header_misplaced if the status line appears at/after the
first '##' heading."""
+ first_heading = SECTION_HEADING_RE.search(text)
+ if first_heading is not None and status_match.start() >=
first_heading.start():
+ flags.append(("status_header_misplaced", "'Current status:' appears
at/after the first '##' section heading."))
+
+
+def _flag_open_questions_if_ready(prefix: str, normalized_status: str, text:
str, flags: list) -> None:
+ """Appends ready_but_has_open_questions if a Ready-to-execute file still
has an unresolved '## Open questions'."""
+ if prefix not in ("TODO", "READY") or not
normalized_status.startswith(READY_TO_EXECUTE_STATUS):
+ return
+ oq_section = extract_section(text, "Open questions")
+ if oq_section is not None and open_questions_are_unresolved(oq_section):
+ flags.append(("ready_but_has_open_questions", "Status says 'Ready to
execute' but '## Open questions' still has unresolved item(s)."))
+
+
+def _flag_status_prefix_transitions(prefix: str, status: str,
normalized_status: str, flags: list) -> None:
+ """Appends the READY/TODO/MAYBE prefix-vs-status-wording mismatch flags
(missed renames, wrong prefix)."""
+ if prefix == "READY" and normalized_status.startswith("waiting for user
input"):
+ flags.append(("ready_prefix_waiting_status", "READY-prefixed file but
status still says 'Waiting for user input'."))
+
+ if prefix == "TODO" and
normalized_status.startswith(READY_TO_EXECUTE_STATUS):
+ flags.append(("todo_prefix_marked_ready", "TODO-prefixed file already
marked 'Ready to execute' -- possible missed rename to READY-*.md."))
+
+ if prefix in ("TODO", "READY") and normalized_status.startswith("parked"):
+ flags.append(("parked_status_wrong_prefix", "Status says 'Parked...'
but filename is not MAYBE-prefixed."))
+
+ if prefix == "MAYBE" and not normalized_status.startswith("parked"):
+ flags.append(("maybe_prefix_non_parked", f"MAYBE-prefixed file but
status doesn't start with 'Parked': '{status}'"))
+
+
def audit_file(path: Path) -> list:
"""Return a list of (reason_code, detail) tuples for this plan file. Empty
list means no flags."""
text = path.read_text(encoding="utf-8")
@@ -176,38 +224,19 @@ def audit_file(path: Path) -> list:
status_match = STATUS_LINE_RE.search(text)
complexity_match = COMPLEXITY_LINE_RE.search(text)
-
- if status_match is None:
- flags.append(("missing_status_header", "No 'Current status:' line
found."))
- if complexity_match is None:
- flags.append(("missing_complexity_header", "No 'Complexity:' line
found."))
+ _flag_missing_headers(status_match, complexity_match, flags)
if status_match is not None:
status = status_match.group(1).strip()
- first_heading = SECTION_HEADING_RE.search(text)
- if first_heading is not None and status_match.start() >=
first_heading.start():
- flags.append(("status_header_misplaced", "'Current status:'
appears at/after the first '##' section heading."))
-
- if not status_prefix_ok(prefix, status):
- flags.append(("unrecognized_status_phrase", f"Status text doesn't
match the {prefix} wording rules: '{status}'"))
-
normalized = status.lower()
- if prefix in ("TODO", "READY") and normalized.startswith("ready to
execute"):
- oq_section = extract_section(text, "Open questions")
- if oq_section is not None and
open_questions_are_unresolved(oq_section):
- flags.append(("ready_but_has_open_questions", "Status says
'Ready to execute' but '## Open questions' still has unresolved item(s)."))
- if prefix == "READY" and normalized.startswith("waiting for user
input"):
- flags.append(("ready_prefix_waiting_status", "READY-prefixed file
but status still says 'Waiting for user input'."))
+ _flag_status_placement(text, status_match, flags)
- if prefix == "TODO" and normalized.startswith("ready to execute"):
- flags.append(("todo_prefix_marked_ready", "TODO-prefixed file
already marked 'Ready to execute' -- possible missed rename to READY-*.md."))
-
- if prefix in ("TODO", "READY") and normalized.startswith("parked"):
- flags.append(("parked_status_wrong_prefix", "Status says
'Parked...' but filename is not MAYBE-prefixed."))
+ if not status_prefix_ok(prefix, status):
+ flags.append(("unrecognized_status_phrase", f"Status text doesn't
match the {prefix} wording rules: '{status}'"))
- if prefix == "MAYBE" and not normalized.startswith("parked"):
- flags.append(("maybe_prefix_non_parked", f"MAYBE-prefixed file but
status doesn't start with 'Parked': '{status}'"))
+ _flag_open_questions_if_ready(prefix, normalized, text, flags)
+ _flag_status_prefix_transitions(prefix, status, normalized, flags)
return flags
diff --git a/src/main/java/org/apache/juneau/releng/rest/AdminRest.java
b/src/main/java/org/apache/juneau/releng/rest/AdminRest.java
index e0754e238f..6101fde966 100644
--- a/src/main/java/org/apache/juneau/releng/rest/AdminRest.java
+++ b/src/main/java/org/apache/juneau/releng/rest/AdminRest.java
@@ -21,6 +21,7 @@ import org.apache.juneau.commons.inject.Bean;
import org.apache.juneau.marshall.html.HtmlSerializer;
import org.apache.juneau.rest.server.Rest;
import org.apache.juneau.rest.server.RestGet;
+import org.apache.juneau.rest.server.RestRequest;
import org.apache.juneau.rest.server.servlet.BasicRestResource;
import org.apache.juneau.rest.server.view.View;
import org.apache.juneau.rest.server.view.freemarker.FreemarkerMixin;
@@ -31,10 +32,8 @@ import org.apache.juneau.rest.server.views.PageTable;
import org.apache.juneau.rest.server.views.Tab;
import org.apache.juneau.rest.server.views.ViewsMixin;
-import jakarta.servlet.http.HttpServletRequest;
-
/**
- * Admin tab (TODO-399 Phase C dogfood): a single multi-tab page composing the
app's existing
+ * Admin tab: a single multi-tab page composing the app's existing
* {@link ReleaseRest#releasesView() Releases} and {@link
CredentialRest#credentialsView() Credentials} rich views
* into one {@link PageDef}, rendered by {@link PageTable}.
*
@@ -44,16 +43,13 @@ import jakarta.servlet.http.HttpServletRequest;
* definitions, and each child view's {@code dataUrl} stays absolute (its
owning resource's own mount), so the
* ajax data draws still hit {@link ReleaseRest#data()} / {@link
CredentialRest#status()} exactly as they do from
* the standalone Releases/Credentials pages. Per {@link PageTable}'s
contract, the emitted per-view markup (marker
- * table + VIEW_META sidecar) is byte-for-byte identical to what {@link
ReleaseRest#page(HttpServletRequest)} /
+ * table + VIEW_META sidecar) is byte-for-byte identical to what {@link
ReleaseRest#page(RestRequest)} /
* {@code CredentialRest}'s own view would emit standalone — this
resource only adds the tab-bar/panel shell
* and the PAGE_META sidecar around them.
*/
@Rest(path = "/admin", title = "Admin", responseProcessors =
FreemarkerViewRenderer.class, mixins = ViewsMixin.class)
public class AdminRest extends BasicRestResource {
- /** This resource's absolute mount (RootRest {@code /rest/*} + {@code
/admin}), used to resolve asset URLs. */
- static final String MOUNT = "/rest/admin";
-
// Return type stays FreemarkerMixin - FreemarkerViewRenderer does an
exact-type bean lookup (see
// ConsoleFreemarkerMixin's class Javadoc).
@Bean
@@ -63,8 +59,7 @@ public class AdminRest extends BasicRestResource {
/**
* The composed page definition: one leaf tab per existing rich view.
{@code build()} validates unique tab ids
- * and unique referenced {@code ViewDef} ids across the page (TODO-399
Phase C {@code PageDef} validation
- * rules).
+ * and unique referenced {@code ViewDef} ids across the page.
*/
static PageDef adminPage() {
return PageDef.create("admin")
@@ -77,23 +72,25 @@ public class AdminRest extends BasicRestResource {
/** Human page — the composed tab/sub-tab page shell (emitted as
trusted markup) + PAGE_META sidecar. */
@RestGet("/")
- public View page(HttpServletRequest req) {
+ public View page(RestRequest req) {
var markup =
HtmlSerializer.DEFAULT_SIMPLE_SQ.toString(PageTable.of(adminPage()));
return ConsolePage.of("admin", req)
.attr("pageTable", markup)
- .attr("viewsCssUrl", asset(ViewsMixin.VIEWS_CSS_PATH))
- .attr("rendersJsUrl", asset(ViewsMixin.RENDERS_JS_PATH))
- .attr("iconsJsUrl", asset(ViewsMixin.ICONS_JS_PATH))
- .attr("ribbonJsUrl", asset(ViewsMixin.RIBBON_JS_PATH))
- .attr("viewsJsUrl", asset(ViewsMixin.VIEWS_JS_PATH))
- .attr("pagesJsUrl", asset(ViewsMixin.PAGES_JS_PATH));
+ .attr("viewsCssUrl", asset(req,
ViewsMixin.VIEWS_CSS_PATH))
+ .attr("rendersJsUrl", asset(req,
ViewsMixin.RENDERS_JS_PATH))
+ .attr("iconsJsUrl", asset(req,
ViewsMixin.ICONS_JS_PATH))
+ .attr("ribbonJsUrl", asset(req,
ViewsMixin.RIBBON_JS_PATH))
+ .attr("viewsJsUrl", asset(req,
ViewsMixin.VIEWS_JS_PATH))
+ .attr("pagesJsUrl", asset(req,
ViewsMixin.PAGES_JS_PATH));
}
/**
- * Resolves a toolkit asset to an absolute, cache-busted URL for the
FreeMarker head block, mirroring
- * {@code ReleaseRest#asset(String)}.
+ * Resolves a toolkit asset to an absolute, cache-busted URL for the
FreeMarker head block via the
+ * request-aware {@link ViewsMixin#viewAssetUrl(RestRequest, String)}
— resolved per-request
+ * against this resource's actual mount/context path, rather than a
hardcoded {@code MOUNT} constant string-
+ * replace, so moving this resource no longer silently breaks asset
loading.
*/
- private static String asset(String path) {
- return ViewsMixin.viewAssetUrl(path).replace("servlet:", MOUNT);
+ private static String asset(RestRequest req, String path) {
+ return ViewsMixin.viewAssetUrl(req, path);
}
}
diff --git a/src/main/java/org/apache/juneau/releng/rest/CredentialRest.java
b/src/main/java/org/apache/juneau/releng/rest/CredentialRest.java
index 290a4b45a8..fa7e14beeb 100644
--- a/src/main/java/org/apache/juneau/releng/rest/CredentialRest.java
+++ b/src/main/java/org/apache/juneau/releng/rest/CredentialRest.java
@@ -58,6 +58,9 @@ public class CredentialRest extends BasicRestResource {
/** This resource's absolute mount (RootRest {@code /rest/*} + {@code
/credentials}), used by {@link #credentialsView()}. */
static final String MOUNT = "/rest/credentials";
+ /** This resource's page/view id, shared by {@link
#credentialsView()}'s {@link ViewDef} id and {@link
#page(HttpServletRequest)}'s page/attribute name. */
+ static final String NAME = "credentials";
+
private final CredentialService service;
public CredentialRest(CredentialService service) {
@@ -65,14 +68,14 @@ public class CredentialRest extends BasicRestResource {
}
/**
- * The rich-view toolkit's declarative view of the Credentials list
(TODO-399 Phase C dogfood): a second,
- * independently-composable {@link ViewDef} alongside {@link
ReleaseRest#releasesView()}, wired into the RM
- * {@code Admin} tab page ({@code AdminRest}). Client-side data mode:
{@link #status()} already returns the
- * bare {@code List<CredentialStatus>} the toolkit's client-mode ajax
(({@code dataSrc: ""})) expects, so no new
+ * The rich-view toolkit's declarative view of the Credentials list: a
second, independently-composable
+ * {@link ViewDef} alongside {@link ReleaseRest#releasesView()}, wired
into the RM {@code Admin} tab page
+ * ({@code AdminRest}). Client-side data mode: {@link #status()}
already returns the bare
+ * {@code List<CredentialStatus>} the toolkit's client-mode ajax
(({@code dataSrc: ""})) expects, so no new
* server-side query wiring is needed.
*/
static ViewDef credentialsView() {
- return ViewDef.create("credentials")
+ return ViewDef.create(NAME)
.rowType(CredentialStatus.class)
.dataMode(DataMode.CLIENT)
.dataUrl(MOUNT + "/status")
@@ -97,7 +100,7 @@ public class CredentialRest extends BasicRestResource {
/** Human page. */
@RestGet("/")
public View page(HttpServletRequest req) {
- return ConsolePage.of("credentials", req).attr("credentials",
service.status());
+ return ConsolePage.of(NAME, req).attr(NAME, service.status());
}
/** JSON status for all credentials (no secrets). */
diff --git a/src/main/java/org/apache/juneau/releng/rest/ReleaseRest.java
b/src/main/java/org/apache/juneau/releng/rest/ReleaseRest.java
index 33a9b58cb6..c94f132ae4 100644
--- a/src/main/java/org/apache/juneau/releng/rest/ReleaseRest.java
+++ b/src/main/java/org/apache/juneau/releng/rest/ReleaseRest.java
@@ -24,6 +24,7 @@ import org.apache.juneau.http.response.NotFound;
import org.apache.juneau.marshall.html.HtmlSerializer;
import org.apache.juneau.rest.server.Rest;
import org.apache.juneau.rest.server.RestGet;
+import org.apache.juneau.rest.server.RestRequest;
import org.apache.juneau.rest.server.converter.ProtocolQueryable;
import org.apache.juneau.rest.server.converter.QueryableSettings;
import org.apache.juneau.rest.server.servlet.BasicRestResource;
@@ -48,7 +49,7 @@ import jakarta.servlet.http.HttpServletRequest;
*
* <p>
* Built on the {@code juneau-rest-server-views} rich-view toolkit: {@link
#releasesView()} declares the typed
- * {@link ViewDef} (columns + renderers + ribbon), {@link
#page(HttpServletRequest)} emits its {@link ViewTable} shell as trusted markup
+ * {@link ViewDef} (columns + renderers + ribbon), {@link #page(RestRequest)}
emits its {@link ViewTable} shell as trusted markup
* into the FreeMarker template, and {@link #data()} serves the {@code
DataTablesResults} envelope via
* {@link ProtocolQueryable} + the view's {@link ViewDef#queryableSettings()
queryable settings}. The four runtime
* assets are served by the composed {@link ViewsMixin} at this resource's
mount.
@@ -59,6 +60,9 @@ public class ReleaseRest extends BasicRestResource {
/** This resource's absolute mount (RootRest {@code /rest/*} + {@code
/releases}), used to resolve asset/data URLs. */
static final String MOUNT = "/rest/releases";
+ /** The rich-view toolkit's cell renderer id for a
clickable/href-bearing column (see {@link Column#render(String)}). */
+ static final String RENDER_LINKED = "linked";
+
private final ReleaseListService service;
public ReleaseRest(ReleaseListService service) {
@@ -86,8 +90,7 @@ public class ReleaseRest extends BasicRestResource {
* rendered Status/Stage pills (emitting the shared {@code
.tag.<domain>.<value>} classes the app's console-ui
* palette themes), timestamp/date columns, and a copy/csv export +
column-search + status quick-filter + refresh
* ribbon. Data arrives via ajax draws against {@link #data()}. Static
(no instance state) so {@code AdminRest}
- * can reuse this same declarative definition when composing the {@code
Admin} tab page (TODO-399 Phase C
- * dogfood).
+ * can reuse this same declarative definition when composing the {@code
Admin} tab page.
*/
static ViewDef releasesView() {
return ViewDef.create("releases")
@@ -96,14 +99,14 @@ public class ReleaseRest extends BasicRestResource {
.dataUrl(MOUNT + "/data")
.defaultOrder("version", Dir.DESC)
.columns(
-
Column.of("version").title("Version").render("linked").href(MOUNT +
"/{version}/1"),
+
Column.of("version").title("Version").render(RENDER_LINKED).href(MOUNT +
"/{version}/1"),
Column.of("rc").title("RC"),
Column.of("status").title("Status").render("tag:status"),
Column.of("stage").title("Stage").render("tag:stage"),
Column.of("voteCloses").title("Vote
closes").render("ts-zulu"),
Column.of("released").title("Released").render("date"),
-
Column.of("githubReleaseUrl").title("GitHub").render("linked").href("{githubReleaseUrl}").orderable(false),
-
Column.of("milestoneUrl").title("Milestone").render("linked").href("{milestoneUrl}").orderable(false))
+
Column.of("githubReleaseUrl").title("GitHub").render(RENDER_LINKED).href("{githubReleaseUrl}").orderable(false),
+
Column.of("milestoneUrl").title("Milestone").render(RENDER_LINKED).href("{milestoneUrl}").orderable(false))
.ribbon(
// "filters" clusters the column-search toggle
and the dropped-only quick-filter into one
// segmented ribbon group (visual-parity
control-row layout: filter-ribbon); "export" actions are
@@ -119,24 +122,26 @@ public class ReleaseRest extends BasicRestResource {
/** Human page — the rich-view table shell (emitted as trusted markup)
+ JSON sidecar, hydrated by the toolkit JS. */
@RestGet("/")
- public View page(HttpServletRequest req) {
+ public View page(RestRequest req) {
var markup =
HtmlSerializer.DEFAULT_SIMPLE_SQ.toString(ViewTable.of(releasesView()));
return ConsolePage.of("releases", req)
.attr("viewTable", markup)
- .attr("viewsCssUrl", asset(ViewsMixin.VIEWS_CSS_PATH))
- .attr("rendersJsUrl", asset(ViewsMixin.RENDERS_JS_PATH))
- .attr("iconsJsUrl", asset(ViewsMixin.ICONS_JS_PATH))
- .attr("ribbonJsUrl", asset(ViewsMixin.RIBBON_JS_PATH))
- .attr("viewsJsUrl", asset(ViewsMixin.VIEWS_JS_PATH));
+ .attr("viewsCssUrl", asset(req,
ViewsMixin.VIEWS_CSS_PATH))
+ .attr("rendersJsUrl", asset(req,
ViewsMixin.RENDERS_JS_PATH))
+ .attr("iconsJsUrl", asset(req,
ViewsMixin.ICONS_JS_PATH))
+ .attr("ribbonJsUrl", asset(req,
ViewsMixin.RIBBON_JS_PATH))
+ .attr("viewsJsUrl", asset(req,
ViewsMixin.VIEWS_JS_PATH));
}
/**
- * Resolves a toolkit asset to an absolute, cache-busted URL for the
FreeMarker head block. {@link ViewsMixin}'s
- * {@code servlet:}-relative form is rewritten to this resource's
absolute mount because the FreeMarker template
- * is rendered outside Juneau's {@code HtmlDoc} URL-resolution (which
would otherwise resolve {@code servlet:}).
+ * Resolves a toolkit asset to an absolute, cache-busted URL for the
FreeMarker head block via the
+ * request-aware {@link ViewsMixin#viewAssetUrl(RestRequest, String)},
resolved per-request against
+ * this resource's actual mount/context path rather than a hardcoded
string-replace of the {@code servlet:}
+ * scheme — the FreeMarker template is rendered outside Juneau's
{@code HtmlDoc} URL-resolution, which is
+ * why the URL must already be resolved before it reaches the template.
*/
- private static String asset(String path) {
- return ViewsMixin.viewAssetUrl(path).replace("servlet:", MOUNT);
+ private static String asset(RestRequest req, String path) {
+ return ViewsMixin.viewAssetUrl(req, path);
}
/**
diff --git a/src/main/java/org/apache/juneau/releng/rest/ReleaseRunRest.java
b/src/main/java/org/apache/juneau/releng/rest/ReleaseRunRest.java
index 366337ac64..a8af43840a 100644
--- a/src/main/java/org/apache/juneau/releng/rest/ReleaseRunRest.java
+++ b/src/main/java/org/apache/juneau/releng/rest/ReleaseRunRest.java
@@ -17,8 +17,12 @@
package org.apache.juneau.releng.rest;
+import static org.apache.juneau.commons.utils.StringUtils.escapeForScript;
+
+import java.util.LinkedHashMap;
import java.util.Map;
import org.apache.juneau.commons.inject.Bean;
+import org.apache.juneau.marshall.marshaller.Json;
import org.apache.juneau.http.Content;
import org.apache.juneau.http.Path;
import org.apache.juneau.http.response.Conflict;
@@ -74,7 +78,7 @@ public class ReleaseRunRest extends BasicRestResource {
var liveCapable = engine.mode() == ExecutionMode.LIVE;
var active = engine.displayRun().orElse(null);
var runMode = active == null ? ExecutionMode.SAFE :
engine.effectiveMode(active);
- var view = ConsolePage.of("new-release", req).attr("steps",
engine.registry().steps())
+ var view = ConsolePage.of("new-release", req).attr("stepMeta",
stepMetaJson(engine.registry().steps()))
.attr("mode", runMode.name()).attr("appMode",
engine.mode().name())
.attr("liveCapable",
Boolean.valueOf(liveCapable));
// FreemarkerView.attr() rejects null values by design; the
template only checks run??
@@ -83,6 +87,31 @@ public class ReleaseRunRest extends BasicRestResource {
: view.attr("run", active).attr("armed",
Boolean.valueOf(engine.isArmed(active.version)));
}
+ /**
+ * Serializes the step registry's {@code {id: {title, mutating}}} map
for the {@code nr-step-meta} sidecar.
+ *
+ * <p>Built and escaped Java-side rather than interpolated in the
{@code .ftlh}: the block is the raw-text content
+ * of a {@code <script type="application/json">} element, for which
FreeMarker's HTML auto-escaping is the wrong
+ * escaper (it entity-encodes {@code &}/{@code <}/{@code "} into forms
{@code JSON.parse} reads verbatim) and its
+ * incidental {@code </script>} break-out protection depends only on
the file extension. This serializes with the
+ * repo's JSON marshaller and hands the result to {@link
org.apache.juneau.commons.utils.StringUtils#escapeForScript(String)}
+ * — the same shared, hardened escaper the framework's {@code
ViewTable}/{@code PageTable} sidecars use
+ * — so a step title containing {@code </script>} cannot
terminate the element early.
+ *
+ * @param steps The registry's steps.
+ * @return The break-out-safe, {@code JSON.parse}-able sidecar payload.
+ */
+ static String stepMetaJson(Iterable<? extends
org.apache.juneau.releng.engine.ReleaseStep> steps) {
+ var meta = new LinkedHashMap<String,Object>();
+ for (var step : steps) {
+ var entry = new LinkedHashMap<String,Object>();
+ entry.put("title", step.title());
+ entry.put("mutating", Boolean.valueOf(step.mutating()));
+ meta.put(step.id(), entry);
+ }
+ return escapeForScript(Json.of(meta));
+ }
+
/** JSON RunState for polling / initial page data. */
@RestGet("/{version}")
public RunState state(@Path("version") String version) {
diff --git a/src/main/resources/static/css/new-release.css
b/src/main/resources/static/css/new-release.css
index 53275ce2ea..b7962bf29e 100644
--- a/src/main/resources/static/css/new-release.css
+++ b/src/main/resources/static/css/new-release.css
@@ -151,7 +151,7 @@
max-height: 240px;
overflow-y: auto;
white-space: pre-wrap;
- word-break: break-word;
+ overflow-wrap: break-word;
}
.rm-console .l-cmd { color: #7ec9ff; }
diff --git a/src/main/resources/templates/new-release.ftlh
b/src/main/resources/templates/new-release.ftlh
index 70ad3dd45a..bb94c31c88 100644
--- a/src/main/resources/templates/new-release.ftlh
+++ b/src/main/resources/templates/new-release.ftlh
@@ -173,14 +173,14 @@
</div>
</div>
- <!-- Step id -> {title, mutating} from the StepRegistry (page()'s
"steps" attr), for the
- detail pane's title/mutating-tag rendering. Not RunState — that's
already inline above. -->
- <script id="nr-step-meta" type="application/json">
- {
- <#list steps as reg>"${reg.id()}": {"title": "${reg.title()}",
"mutating": ${reg.mutating()?c}}<#if reg?has_next>,</#if>
- </#list>
- }
- </script>
+ <!-- Step id -> {title, mutating} from the StepRegistry, for the
detail pane's title/mutating-tag
+ rendering. Not RunState — that's already inline above. Serialized
Java-side by page() with the
+ repo's JSON marshaller and passed through
StringUtils.escapeForScript, then inserted here as raw
+ content (?no_esc): FreeMarker's HTML auto-escaping is the WRONG
escaper for a raw-text <script>
+ (it entity-encodes &/</" into forms JSON.parse reads verbatim,
corrupting the payload), and its
+ break-out protection is only incidental to the .ftlh extension.
escapeForScript neutralizes the
+ </script> / <!-- break-outs while keeping the JSON valid. -->
+ <script id="nr-step-meta"
type="application/json">${stepMeta?no_esc}</script>
</#if>
</section>
</#macro>
diff --git a/src/test/java/org/apache/juneau/releng/rest/AdminRestTest.java
b/src/test/java/org/apache/juneau/releng/rest/AdminRestTest.java
index 134bd115b9..c2435efc25 100644
--- a/src/test/java/org/apache/juneau/releng/rest/AdminRestTest.java
+++ b/src/test/java/org/apache/juneau/releng/rest/AdminRestTest.java
@@ -32,7 +32,7 @@ import org.apache.juneau.rest.server.views.ViewsMixin;
import org.junit.jupiter.api.Test;
/**
- * TODO-399 Phase C dogfood (tasks 10-12): the RM {@code Admin} tab composes
the existing Releases/Credentials
+ * The RM {@code Admin} tab composes the existing Releases/Credentials
* {@link org.apache.juneau.rest.server.views.ViewDef ViewDef}s into one
{@link org.apache.juneau.rest.server.views.PageDef PageDef}
* page, rendered by {@link PageTable} and served through {@link AdminRest}.
*/
diff --git
a/src/test/java/org/apache/juneau/releng/rest/CredentialWriteVectorTest.java
b/src/test/java/org/apache/juneau/releng/rest/CredentialWriteVectorTest.java
index 79b710413e..c8bed7f6aa 100644
--- a/src/test/java/org/apache/juneau/releng/rest/CredentialWriteVectorTest.java
+++ b/src/test/java/org/apache/juneau/releng/rest/CredentialWriteVectorTest.java
@@ -18,6 +18,8 @@
package org.apache.juneau.releng.rest;
import static org.junit.jupiter.api.Assertions.*;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
import java.nio.file.*;
import java.util.*;
@@ -152,10 +154,10 @@ class CredentialWriteVectorTest {
* check for the wrong reason -- which would make the refusals below
pass vacuously.
*/
private static jakarta.servlet.http.HttpServletRequest req(String
method, String contentType, Map<String,String> headers) {
- var r =
org.mockito.Mockito.mock(jakarta.servlet.http.HttpServletRequest.class);
- org.mockito.Mockito.when(r.getMethod()).thenReturn(method);
-
org.mockito.Mockito.when(r.getContentType()).thenReturn(contentType);
- headers.forEach((k, v) ->
org.mockito.Mockito.when(r.getHeader(k)).thenReturn(v));
+ var r = mock(jakarta.servlet.http.HttpServletRequest.class);
+ when(r.getMethod()).thenReturn(method);
+ when(r.getContentType()).thenReturn(contentType);
+ headers.forEach((k, v) -> when(r.getHeader(k)).thenReturn(v));
return r;
}
diff --git a/src/test/java/org/apache/juneau/releng/rest/ReleaseRestTest.java
b/src/test/java/org/apache/juneau/releng/rest/ReleaseRestTest.java
index 3b40717201..7a815551b4 100644
--- a/src/test/java/org/apache/juneau/releng/rest/ReleaseRestTest.java
+++ b/src/test/java/org/apache/juneau/releng/rest/ReleaseRestTest.java
@@ -71,7 +71,8 @@ class ReleaseRestTest {
@Test
void detailForAnUnknownVersionIs404() {
var rest = rest(List.of(release("9.2.1", "RELEASED")));
- var ex = assertThrows(NotFound.class, () ->
rest.detail("9.9.9", "1", req()));
+ var httpReq = req();
+ var ex = assertThrows(NotFound.class, () ->
rest.detail("9.9.9", "1", httpReq));
assertEquals(404, ex.getStatusCode());
}
@@ -103,13 +104,6 @@ class ReleaseRestTest {
}
}
- /**
- * The {@code /data} endpoint speaks the DataTables
server-side-processing contract: given a request carrying
- * DataTables params it returns a {@code DataTablesResults} envelope
({@code {draw, recordsTotal, recordsFiltered,
- * data}}) with server-side per-column filtering applied — not
the bare {@code List<Release>} array it used
- * to return. Wired via the {@code juneau-rest-server-views} toolkit
({@code ViewDef.queryableSettings()} +
- * {@code ProtocolQueryable}); this proves the envelope shape and that
filtering happens on the server.
- */
/**
* Regression: the rendered Releases page never included {@code
juneau-icons.js} (only renders/ribbon/views
* were wired up), so the icon registry was absent when the ribbon
built its buttons and every button fell back
@@ -133,6 +127,13 @@ class ReleaseRestTest {
}
}
+ /**
+ * The {@code /data} endpoint speaks the DataTables
server-side-processing contract: given a request carrying
+ * DataTables params it returns a {@code DataTablesResults} envelope
({@code {draw, recordsTotal, recordsFiltered,
+ * data}}) with server-side per-column filtering applied — not
the bare {@code List<Release>} array it used
+ * to return. Wired via the {@code juneau-rest-server-views} toolkit
({@code ViewDef.queryableSettings()} +
+ * {@code ProtocolQueryable}); this proves the envelope shape and that
filtering happens on the server.
+ */
@Test
void dataReturnsDataTablesEnvelopeWithServerSideFilterApplied() throws
Exception {
var releases = List.of(release("9.2.1", "RELEASED"),
release("9.3.0", "VOTING"));
diff --git
a/src/test/java/org/apache/juneau/releng/rest/ReleaseRunRestTest.java
b/src/test/java/org/apache/juneau/releng/rest/ReleaseRunRestTest.java
index f5170af0dd..3d2dddea4a 100644
--- a/src/test/java/org/apache/juneau/releng/rest/ReleaseRunRestTest.java
+++ b/src/test/java/org/apache/juneau/releng/rest/ReleaseRunRestTest.java
@@ -355,6 +355,12 @@ class ReleaseRunRestTest {
* every required step — including {@code nexus-release}, {@code
manual-followup-checklist}, and
* {@code compose-announcement-email} — before {@code finalize-run}
accepts it.
*/
+ @SuppressWarnings({
+ "java:S5961" // Deliberately one continuous end-to-end run
through every required pipeline step, in
+ // order, on a single mutable run;
splitting into separate @Test methods would re-derive
+ // (or fake) the intermediate run
state each time and weaken exactly the regression this
+ // test exists to catch -- that the
SAME run legitimately clears every required step.
+ })
@Test
void safeVoteResultAdvancesGateAndPipelineReachesFinalize(@TempDir Path
dir) throws IOException {
var model = new
NexusMockModel(NexusStagingClient.JUNEAU_PROFILE_ID);
@@ -498,4 +504,42 @@ class ReleaseRunRestTest {
assertEquals(StepStatus.PENDING,
rest.state("9.2.1").step("nexus-release").status,
"a rejected vote must not advance the linear
pipeline");
}
+
+ //
-----------------------------------------------------------------------------------------------------------
+ // nr-step-meta <script> sidecar: break-out neutralization
+ //
-----------------------------------------------------------------------------------------------------------
+
+ /**
+ * The {@code nr-step-meta} JSON is now built Java-side by {@link
ReleaseRunRest#stepMetaJson(Iterable)} and passed
+ * through {@code escapeForScript} rather than interpolated in the
{@code .ftlh}. A step title carrying a
+ * {@code </script>} break-out must be neutralized (no raw {@code <}
survives) yet remain valid, round-trippable
+ * JSON — the property FreeMarker's HTML auto-escaping would have
silently corrupted. Asserts the
+ * neutralization, not merely that a benign title round-trips.
+ */
+ @Test
+ void stepMetaJsonNeutralizesScriptBreakoutInAStepTitle() throws
Exception {
+ var evilTitle = "</script><script>alert(1)</script>\u2028x";
+ var step = new org.apache.juneau.releng.engine.ReleaseStep() {
+ @Override public String id() { return "evil"; }
+ @Override public String title() { return evilTitle; }
+ @Override public boolean mutating() { return true; }
+ @Override public
org.apache.juneau.releng.engine.Preview
preview(org.apache.juneau.releng.engine.StepContext c) { return null; }
+ @Override public
org.apache.juneau.releng.engine.StepResult
apply(org.apache.juneau.releng.engine.StepContext c) { return null; }
+ };
+
+ var json = ReleaseRunRest.stepMetaJson(List.of(step));
+
+ // Break-out neutralized: no raw '<' or raw U+2028 can survive
to close the raw-text <script> element early.
+ assertFalse(json.contains("<"), () -> "raw '<' survived into
the <script> sidecar: " + json);
+ assertFalse(json.contains("\u2028"), () -> "raw U+2028 survived
into the <script> sidecar: " + json);
+ assertTrue(json.contains("\\u003c"), () -> "expected escaped
'<' (\\u003c) in sidecar: " + json);
+
+ // Still valid, round-trippable JSON: a JSON parser decodes
\u003c back to the original title verbatim
+ // (this is exactly what FreeMarker's HTML entity-encoding
would have corrupted).
+ var parsed =
org.apache.juneau.marshall.marshaller.Json.to(json, Map.class);
+ @SuppressWarnings("unchecked")
+ var entry = (Map<String,Object>) parsed.get("evil");
+ assertEquals(evilTitle, entry.get("title"));
+ assertEquals(Boolean.TRUE, entry.get("mutating"));
+ }
}
