feat: implement HKP op=index (#137) - #152
Merged
Merged
Conversation
- Add KeyIndexResult and UidIndexEntry as top-level records in application-api so the web layer only depends on primary ports - Add package-info.java (@NullMarked) to application-api root package - Extend KeyRepository secondary port with findManyBySearch() - Add keyserver-application-api dependency to application-port-repository - Add searchForIndex() to KeyRepositoryService and implement in PersistentKeyRepositoryService - Implement findManyBySearch() in JpaKeyRepository; refactor private search helpers (queryEntitiesByEmail, queryEntitiesByUidSubstring) to eliminate duplicated JPQL strings; both single- and multi-result paths share one query definition - Add HkpIndexRenderer (ApplicationScoped): renderMachineReadable() produces HKP info:/pub:/uid: format; renderHtml() a simple table - Wire op=index routing in LookupEndpoint (options=mr -> text/plain, else HTML; 404 when no results) - Add LookupEndpointIndexTest (7 tests): 404 on empty, info: header, pub: line format, UID percent-encoding, revoked/expired flags, HTML Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #152 +/- ##
============================================
+ Coverage 66.83% 72.86% +6.02%
- Complexity 100 122 +22
============================================
Files 27 28 +1
Lines 392 479 +87
Branches 40 52 +12
============================================
+ Hits 262 349 +87
+ Misses 111 104 -7
- Partials 19 26 +7 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Contributor
There was a problem hiding this comment.
Pull request overview
Implements HKP op=index end-to-end (closes #137). Adds a new application-API DTO pair (KeyIndexResult, UidIndexEntry), extends the outbound KeyRepository port and KeyRepositoryService with multi-result search, plumbs it through the JPA adapter (extracting shared JPQL helpers), and adds a new HkpIndexRenderer plus routing in LookupEndpoint for machine-readable (options=mr) and HTML output, with unit-test coverage.
Changes:
- New
application-apirecordsKeyIndexResult/UidIndexEntry(with@NullMarkedpackage-info) and a newsearchForIndexservice method, mirrored byfindManyBySearchon the secondary repository port. JpaKeyRepositorygainsfindManyBySearchwith the same routing asfindBySearch, sharing extracted email/UID query helpers, and maps verified UIDs into the new DTO.LookupEndpointroutesop=indexthrough the newHkpIndexRenderer, which emits theinfo:/pub:/uid:machine-readable format (flagsr,e) or a minimal HTML table.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| application/application-api/.../KeyIndexResult.java | New DTO for a key's index entry. |
| application/application-api/.../UidIndexEntry.java | New DTO for a verified UID. |
| application/application-api/.../package-info.java | Adds @NullMarked to root api package. |
| application/application-api/.../KeyRepositoryService.java | Adds searchForIndex. |
| application/application-ports/.../KeyRepository.java | Adds findManyBySearch and updates KeySearchResult javadoc. |
| application/application-ports/application-port-repository/pom.xml | Depends on application-api for the new DTO. |
| application/application-core/.../PersistentKeyRepositoryService.java | Implements searchForIndex by delegation. |
| application/application-core/.../VerifyUidCommandHandlerTest.java | Updates fake repo to satisfy new port method. |
| repository/.../JpaKeyRepository.java | Adds multi-result search paths, extracts shared JPQL helpers, maps KeyIndexResult. |
| web/.../LookupEndpoint.java | Routes op=index, selects mr vs HTML, returns 404 on empty. |
| web/.../HkpIndexRenderer.java | New renderer for machine-readable and HTML index output. |
| web/.../LookupEndpointIndexTest.java | Seven unit tests covering routing, formatting, flags, and HTML. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Cap multi-result index queries at 5_000 rows (INDEX_RESULT_LIMIT constant) to prevent unbounded memory use on broad search terms; applies to both email and UID-substring paths - Replace '+' with '%20' in UID percent-encoding: URLEncoder uses form-encoding (space→'+') but '+' is a legal literal in UID strings and would be mis-decoded by strict RFC 3986 clients - Strengthen expired-key test: assert the full pub: line including the expiration epoch and ':e' flag position, not just ':e\n', so the test targets the key-level flag specifically rather than the uid: line Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
3 tasks
- Add DB migration V010: 'disabled' BOOLEAN column on keys table (default FALSE)
- Add 'disabled' field to KeyEntity with getter/setter
- Add 'disabled' component to KeyIndexResult record
- HkpIndexRenderer.computeFlags now emits 'd' between 'r' and 'e' per HKP spec
- Key-level pub: line passes key.disabled(); uid: lines always pass false
- Fix N+1 query for multi-result index paths (findManyByEmail, findManyByUidSubstring):
- Two-query pattern: first query selects fingerprints only with setMaxResults()
so the SQL LIMIT applies cleanly on a scalar query without pagination/JOIN FETCH conflict
- Second query does JOIN FETCH k.uids WHERE fingerprint IN :fps for correct eager loading
- Fix findManyByKeyIdOrFingerprint: use JOIN FETCH k.uids with DISTINCT on all paths
(fingerprint, long key ID, short key ID) to avoid N+1 per key
- Add JPA provider read-only hints (Hibernate, EclipseLink, Apache OpenJPA) on all
multi-result index queries to skip dirty tracking and lock acquisition
- Add test setsFlagDForDisabledKey() in LookupEndpointIndexTest
- Update all existing KeyIndexResult constructions for new 'disabled' component
Closes #137 review comments (N+1 query, 'd' flag)
Related: #153 (search result ranking, filed separately)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…lter, HTML label - Fix V010 migration comment: disabled keys are shown with 'd' flag in op=index, not hidden from results (HKP spec behaviour) - Filter keys with no verified UIDs from all findManyBy* result paths: toIndexResults() now drops KeyIndexResult entries with empty verifiedUids, making fingerprint/keyid lookups consistent with email/UID-substring paths which already gate matching on u.verified = true - HTML renderer: rename 'Algorithm' column to 'Algorithm (OpenPGP code)' so users understand the integer value (e.g. 22 = EdDSA, 1 = RSA) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…d HTML DOCTYPE - AGENTS.adoc: document exception to 'no Optional in fields' rule — immutable transient DTO records (never persisted, never serialized) may use Optional components when optionality is intrinsic to the domain; KeyIndexResult and UidIndexEntry are the canonical examples - LookupEndpoint: fix options=mr detection — split on ',' and compare tokens case-insensitively via Arrays.stream().anyMatch() instead of String.contains(), preventing false matches on tokens like 'nomr' or 'mrtg' - HkpIndexRenderer.renderHtml: add DOCTYPE and <meta charset=utf-8> so browsers render in standards mode; fixes potential mis-rendering of UTF-8 UIDs via 'save as' workflows and legacy intermediaries - LookupEndpointIndexTest: add assertions for DOCTYPE/charset in HTML test and new recognisesMrTokenAmongCommaDelimitedOptions test (nm,mr -> machine-readable) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Implements
GET /pks/lookup?op=indexas described in #137.What changed
Architecture:
KeyIndexResultandUidIndexEntryare top-level records inapplication-api— the web layer only depends on primary ports, no secondary port types cross the boundary@NullMarkedpackage-info.javato theapplication.apiroot packageapplication-port-repositorypom gains a dependency onapplication-api(one-way, no cycle)Port layer (
KeyRepository):findManyBySearch(String, boolean)returningList<KeyIndexResult>Service layer:
KeyRepositoryService.searchForIndex()delegates to the new port methodPersistentKeyRepositoryServiceimplements itRepository adapter (
JpaKeyRepository):findManyBySearch()with the same routing (fingerprint → email → UID substring)queryEntitiesByEmail()andqueryEntitiesByUidSubstring()helpers that both the single-result and multi-result paths share — eliminates duplicated JPQLtoIndexResult(KeyEntity)maps to the new record; onlyverified=trueUIDs includedWeb layer:
HkpIndexRenderer(ApplicationScoped):renderMachineReadable()produces the HKPinfo:/pub:/uid:format;renderHtml()a simple tableLookupEndpointroutesop=index:options=mr→text/plain, elsetext/html; returns 404 when no resultsTests (
LookupEndpointIndexTest, 7 cases):info:1:header with correct countpub:line format (fingerprint, algorithm, keylen, epoch seconds)rflag for revoked keyseflag for expired keysCloses #137