feat(maven): search by GAV and surface POM in the UI#458
Conversation
Adds Maven GAV-aware search and POM access for Maven repositories. Search (#441): the GAVC tab previously sent groupId/artifactId/version as the backend's path/name/version params, which advanced-search ignores (it matches a single full-text query over name + path + version). The tab now folds the GAV, classifier, and a new Extension field into one query string scoped to the maven format, so coordinate searches return results. A shared buildMavenSearchQuery helper centralizes the term assembly. POM and GAV (#442): the grouped Maven browser now renders each component file as a download link (path derived from the GAV layout) and tags the .pom file with a POM badge. The artifact detail view gains a Maven section showing parsed groupId/artifactId/version and a copy/paste pom.xml dependency snippet. New src/lib/maven.ts holds the pure coordinate helpers (query building, path derivation, POM detection, GAV parsing, snippet rendering) with unit tests, plus a Playwright spec covering GAV search and POM reachability.
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
brandonrc
left a comment
There was a problem hiding this comment.
Test/E2E quality review (v1.2.0 batch). Verdict: strongest unit coverage in the batch; e2e has soft spots.
maven.test.ts (#441/#442). Excellent. buildMavenSearchQuery is covered for join, empty/whitespace skipping, trimming, leading-dot strip, and empty-input. parseMavenGav covers standard layout, leading slash, and the negative case (non-Maven paths return undefined). isPomFile covers the .pom.sha1/.pom.asc false-positive traps, which is exactly the kind of negative case that proves the fix is real. These tests would fail against the old code path (which fed path/name/version separately instead of folding into query).
#442 POM download e2e. Good and deterministic: PUTs the POM via API, then GET /download/${POM_PATH} asserts status 200 and body contains the artifactId. This is a pure contract assertion with no skip path.
#441 GAVC search e2e. Real: fills group/artifact/version/extension, clicks search, asserts the JAR row appears in results. No skip escape. The #search-gavc-extension visibility assertion ties directly to the fix. Reasonable.
#442 grouped-browser + detail e2e. Three test.skip paths here (GAV row not rendered, flat table not rendered, jar not visible). The detail-view test for maven-gav-section / maven-pom-snippet is the assertion that proves #442's GAV-surfacing fix, and it sits behind two skips. If the grouped view or flat table regresses, these skip silently green. The unit tests cover the helpers, but the UI-rendering of the GAV section is only exercised by this skippable e2e. Consider a vitest render test for MavenGavSection so the snippet rendering has a non-skippable guard.
Minor flake risk: page.waitForTimeout(2000) after the search fill is an arbitrary wait; prefer waiting on the row/response. Net: solid, not requesting changes.
brandonrc
left a comment
There was a problem hiding this comment.
Security/API review (path traversal + injection lens on Maven path construction).
Net: safe. No traversal sink and no HTML/XSS injection introduced.
mavenFilePath / parseMavenGav / groupIdToPath (src/lib/maven.ts) — SAFE. These build a repository-relative display/download path from component fields and parse coordinates back out. They are pure string transforms; the result is handed to artifactsApi.getDownloadUrl (which produces a same-origin /api/v1/repositories/<key>/download/<path> URL rendered as an href) or used as display text. A crafted GAV cannot escape the app origin because the path is always appended under the fixed /download/ route on the current origin; there is no filesystem access on the client. .. segments would simply be sent to the backend, which is the authoritative traversal-prevention boundary for download/metadata routes (out of scope for this web change, but worth confirming the backend rejects .. in the wildcard path — see note on #460).
pom snippet rendering — SAFE. buildPomDependencySnippet interpolates groupId/artifactId/version into an XML-looking string, but MavenGavSection renders it inside <pre data-testid="maven-pom-snippet">{snippet}</pre> as a React text child, so it is HTML-escaped, not parsed. No dangerouslySetInnerHTML. The copy button copies the raw string. No injection sink.
Encoded separators — see my note on #460: this PR's links go through getDownloadUrl, which on main does not per-segment-encode; #460 fixes that for the metadata get. Make sure #460 lands with/before this so generated links with spaces or reserved chars resolve correctly. Not a security defect, a correctness coupling.
No request-changes.
brandonrc
left a comment
There was a problem hiding this comment.
Frontend/React correctness review. One build-breaking blocker, otherwise the Maven helper logic is clean and well-tested. (Posting as a comment since the review tool refuses request-changes on a self-authored PR, but please treat the first item as a blocker.)
Blocker
Undefined repoFormat in repo-detail-content.tsx
The new Maven GAV section is gated on a repoFormat identifier that does not exist in this file:
{(repoFormat === "maven" || repoFormat === "gradle") && (
<MavenGavSection path={selectedArtifact.path} />
)}Everywhere else the file uses repository.format (around lines 440, 482, 590); there is no repoFormat in scope and the diff does not introduce one. This will not type-check, and would be a ReferenceError at runtime. Change to repository.format === "maven" || repository.format === "gradle" (or destructure a local const repoFormat = repository.format). The PR notes claim tsc --noEmit reports only two pre-existing errors, which is inconsistent with this usage, so please re-run tsc on the branch and confirm.
Suggestions
src/lib/maven.tsparseMavenGav: the "groupId = all segments before the last three" heuristic is reasonable and theif (!gav) return nullguard inMavenGavSectionkeeps a misparse harmless (section just hides). No change needed, flagging the limitation for snapshot/timestamped paths.- File-row links correctly use
artifactsApi.getDownloadUrl(relative path) for<a download>on the same origin. This is now intentionally distinct from #457'sgetAbsoluteDownloadUrl(relative for issuing downloads, absolute for copy-paste). Fine.
Merge sequencing (not a code defect)
This PR and #463 both rewrite the gavc branch of buildSearchParams in search-content.tsx. #463 reverts that branch to sending path/name/version (the very params this PR shows the backend ignores). Whichever merges second must preserve this PR's buildMavenSearchQuery + format: "maven" mapping for the gavc tab, or GAV search regresses. Please coordinate the rebase.
Tests are thorough. Once repoFormat is fixed this is good.
|
Shipped in v1.2.0. The changes from this PR were folded into the release integration branch #466, which was squash-merged to main and tagged as v1.2.0. The code is already on main, so closing this as superseded by #466 rather than merging it (re-applying would just duplicate what already shipped). No work lost. See #466 for the consolidated record. |
Summary
Two Maven UX fixes for v1.2.0.
GAV search (Closes #441). The Advanced Search GAVC tab sent groupId, artifactId, and version as the backend's
path/name/versionquery params. The advanced-search endpoint ignores those: it matches a single full-textqueryagainst name + path + version. Because Maven coordinates live in the artifact path, GAVC searches returned nothing. The tab now folds the GAV fields, classifier, and a new Extension field into one full-text query scoped to themavenformat. A sharedbuildMavenSearchQueryhelper assembles the terms.POM and GAV display (Closes #442). In the grouped Maven browser, each component file (jar, pom, checksums) now renders as a download link with the path derived from the GAV layout, and the
.pomfile is tagged with a POM badge so it is easy to find and download. The artifact detail view gains a Maven section that shows the parsed groupId / artifactId / version and a copy/pastepom.xmldependency snippet.New
src/lib/maven.tsholds the pure coordinate helpers (query building, GAV-to-path, POM detection, GAV parsing, dependency snippet).Test Checklist
UI Changes
Validation:
eslintclean on changed files,tsc --noEmitreports only two pre-existing errors in untouched test files, 16 new unit tests pass, existing maven-component-list (16) and search-content (48) suites still pass.