Skip to content

Commit a60c1c7

Browse files
coopernetescoopernetes
authored
perf: return summary projection from GET /api/push list endpoint (#257)
## Summary - Adds `PushSummary` — a lightweight DTO with only the 12 fields the push list view needs (no `steps`, `commits`, or `attestation`) - `PushStore.findSummaries()` default implementation maps from `find()` in-memory (covers InMemory/Mongo stores); `JdbcPushStore` overrides with a lean `SELECT` of 12 columns and no `hydrate()` call, eliminating N×3 child-table queries per row - `GET /api/push` now returns `List<PushSummary>` — no frontend changes needed since field names are a subset of the existing `PushRecord` type ## Test plan - [ ] `JdbcPushStoreIntegrationTest` — two new tests: projection fields mapped correctly, filters pass through - [ ] `PushControllerTest` — existing list tests updated to verify `findSummaries()` is called with the correct query params - [ ] Coverage threshold verified locally (`jacocoTestCoverageVerification` passes) Closes #256 🤖 Generated with [Claude Code](https://claude.com/claude-code)
2 parents 350d39d + 5dd8f75 commit a60c1c7

6 files changed

Lines changed: 136 additions & 6 deletions

File tree

‎git-proxy-java-core/src/main/java/org/finos/gitproxy/db/PushStore.java‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
import org.finos.gitproxy.db.model.PushQuery;
99
import org.finos.gitproxy.db.model.PushRecord;
1010
import org.finos.gitproxy.db.model.PushStatus;
11+
import org.finos.gitproxy.db.model.PushSummary;
1112

1213
/**
1314
* Storage abstraction for push records. Implementations exist for in-memory, JDBC (H2, SQLite, Postgres), and MongoDB.
@@ -28,6 +29,30 @@ public interface PushStore {
2829
/** Query pushes with optional filters. */
2930
List<PushRecord> find(PushQuery query);
3031

32+
/**
33+
* Return lightweight summary projections for the list view. Omits all child collections (steps, commits,
34+
* attestation). Default implementation delegates to {@link #find} and projects in memory; JDBC override uses a lean
35+
* SELECT.
36+
*/
37+
default List<PushSummary> findSummaries(PushQuery query) {
38+
return find(query).stream()
39+
.map(r -> PushSummary.builder()
40+
.id(r.getId())
41+
.status(r.getStatus())
42+
.url(r.getUrl())
43+
.upstreamUrl(r.getUpstreamUrl())
44+
.project(r.getProject())
45+
.repoName(r.getRepoName())
46+
.branch(r.getBranch())
47+
.commitTo(r.getCommitTo())
48+
.author(r.getAuthor())
49+
.user(r.getUser())
50+
.resolvedUser(r.getResolvedUser())
51+
.timestamp(r.getTimestamp())
52+
.build())
53+
.toList();
54+
}
55+
3156
/** Delete a push record and all associated data. */
3257
void delete(String id);
3358

‎git-proxy-java-core/src/main/java/org/finos/gitproxy/db/jdbc/JdbcPushStore.java‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,39 @@ public List<PushRecord> find(PushQuery query) {
9696
return results;
9797
}
9898

99+
@Override
100+
public List<PushSummary> findSummaries(PushQuery query) {
101+
MapSqlParameterSource params = new MapSqlParameterSource();
102+
String where = buildWhere(query, params);
103+
String sql =
104+
"SELECT id, status, url, upstream_url, project, repo_name, branch, commit_to, author, push_user, resolved_user, timestamp"
105+
+ " FROM push_records" + where
106+
+ " ORDER BY timestamp " + (query.isNewestFirst() ? "DESC" : "ASC")
107+
+ " LIMIT :limit OFFSET :offset";
108+
params.addValue("limit", query.getLimit());
109+
params.addValue("offset", query.getOffset());
110+
return jdbc.query(
111+
sql,
112+
params,
113+
(rs, rowNum) -> PushSummary.builder()
114+
.id(rs.getString("id"))
115+
.status(PushStatus.valueOf(rs.getString("status")))
116+
.url(rs.getString("url"))
117+
.upstreamUrl(rs.getString("upstream_url"))
118+
.project(rs.getString("project"))
119+
.repoName(rs.getString("repo_name"))
120+
.branch(rs.getString("branch"))
121+
.commitTo(rs.getString("commit_to"))
122+
.author(rs.getString("author"))
123+
.user(rs.getString("push_user"))
124+
.resolvedUser(rs.getString("resolved_user"))
125+
.timestamp(
126+
rs.getTimestamp("timestamp") != null
127+
? rs.getTimestamp("timestamp").toInstant()
128+
: null)
129+
.build());
130+
}
131+
99132
@Override
100133
public List<RepoPushSummary> summarizeByRepo() {
101134
return jdbc.query(

‎git-proxy-java-core/src/main/java/org/finos/gitproxy/db/model/PushSummary.java‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
package org.finos.gitproxy.db.model;
2+
3+
import java.time.Instant;
4+
import lombok.Builder;
5+
import lombok.Value;
6+
7+
/**
8+
* Lightweight projection of a push record for list views. Omits all {@link PushStep}, {@link PushCommit}, and
9+
* {@link Attestation} data to keep list-endpoint responses small.
10+
*/
11+
@Value
12+
@Builder
13+
public class PushSummary {
14+
String id;
15+
PushStatus status;
16+
String url;
17+
String upstreamUrl;
18+
String project;
19+
String repoName;
20+
String branch;
21+
String commitTo;
22+
String author;
23+
String user;
24+
String resolvedUser;
25+
Instant timestamp;
26+
}

‎git-proxy-java-core/src/test/java/org/finos/gitproxy/db/jdbc/JdbcPushStoreIntegrationTest.java‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
import org.finos.gitproxy.db.model.PushRecord;
1313
import org.finos.gitproxy.db.model.PushStatus;
1414
import org.finos.gitproxy.db.model.PushStep;
15+
import org.finos.gitproxy.db.model.PushSummary;
1516
import org.finos.gitproxy.db.model.StepStatus;
1617
import org.junit.jupiter.api.BeforeEach;
1718
import org.junit.jupiter.api.Test;
@@ -349,6 +350,50 @@ void find_bySearch_matchesProjectAndRepoName() {
349350
assertEquals("widget-service", byRepo.get(0).getRepoName());
350351
}
351352

353+
// ---- findSummaries ----
354+
355+
@Test
356+
void findSummaries_returnsProjectionWithoutChildCollections() {
357+
PushRecord saved = PushRecord.builder()
358+
.commitTo("abc123")
359+
.branch("refs/heads/main")
360+
.repoName("repo")
361+
.project("acme")
362+
.upstreamUrl("https://github.com/acme/repo.git")
363+
.author("Alice")
364+
.user("alice")
365+
.resolvedUser("alice")
366+
.status(PushStatus.FORWARDED)
367+
.build();
368+
store.save(saved);
369+
370+
List<PushSummary> summaries = store.findSummaries(PushQuery.builder().build());
371+
372+
assertEquals(1, summaries.size());
373+
PushSummary s = summaries.get(0);
374+
assertEquals(saved.getId(), s.getId());
375+
assertEquals(PushStatus.FORWARDED, s.getStatus());
376+
assertEquals("https://github.com/acme/repo.git", s.getUpstreamUrl());
377+
assertEquals("refs/heads/main", s.getBranch());
378+
assertEquals("abc123", s.getCommitTo());
379+
assertEquals("Alice", s.getAuthor());
380+
assertEquals("alice", s.getUser());
381+
assertEquals("alice", s.getResolvedUser());
382+
assertNotNull(s.getTimestamp());
383+
}
384+
385+
@Test
386+
void findSummaries_filtersApplied() {
387+
store.save(record("a", "refs/heads/main", "repoA"));
388+
store.save(record("b", "refs/heads/main", "repoB"));
389+
390+
List<PushSummary> results =
391+
store.findSummaries(PushQuery.builder().repoName("repoA").build());
392+
393+
assertEquals(1, results.size());
394+
assertEquals("repoA", results.get(0).getRepoName());
395+
}
396+
352397
// ---- initialize idempotency ----
353398

354399
@Test

‎git-proxy-java-dashboard/src/main/java/org/finos/gitproxy/dashboard/controller/PushController.java‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
import org.finos.gitproxy.db.model.PushRecord;
1212
import org.finos.gitproxy.db.model.PushStatus;
1313
import org.finos.gitproxy.db.model.PushStep;
14+
import org.finos.gitproxy.db.model.PushSummary;
1415
import org.finos.gitproxy.jetty.config.AttestationQuestion;
1516
import org.finos.gitproxy.jetty.config.GitProxyConfig;
1617
import org.finos.gitproxy.jetty.reload.ConfigHolder;
@@ -55,7 +56,7 @@ private static String resolveReviewer(Map<String, String> body) {
5556
*/
5657
@Operation(operationId = "listPushes", summary = "List push records")
5758
@GetMapping
58-
public List<PushRecord> list(
59+
public List<PushSummary> list(
5960
@RequestParam(required = false) String status,
6061
@RequestParam(required = false) String project,
6162
@RequestParam(required = false) String repo,
@@ -82,7 +83,7 @@ public List<PushRecord> list(
8283
if (user != null && !user.isBlank()) query.user(user);
8384
if (search != null && !search.isBlank()) query.search(search);
8485

85-
return pushStore.find(query.build());
86+
return pushStore.findSummaries(query.build());
8687
}
8788

8889
/**

‎git-proxy-java-dashboard/src/test/java/org/finos/gitproxy/dashboard/controller/PushControllerTest.java‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -99,10 +99,10 @@ private void loginAs(String username, boolean admin, boolean selfCertify) {
9999
class List_ {
100100
@Test
101101
void noFilters_delegatesToStore() {
102-
when(pushStore.find(any())).thenReturn(java.util.List.of());
102+
when(pushStore.findSummaries(any())).thenReturn(java.util.List.of());
103103
var result = controller.list(null, null, null, null, null, 50, 0, true);
104104
assertEquals(0, result.size());
105-
verify(pushStore).find(argThat(q -> q.getLimit() == 50 && q.getOffset() == 0));
105+
verify(pushStore).findSummaries(argThat(q -> q.getLimit() == 50 && q.getOffset() == 0));
106106
}
107107

108108
@Test
@@ -115,9 +115,9 @@ void invalidStatus_throws400() {
115115

116116
@Test
117117
void validStatus_passedToQuery() {
118-
when(pushStore.find(any())).thenReturn(java.util.List.of());
118+
when(pushStore.findSummaries(any())).thenReturn(java.util.List.of());
119119
controller.list("PENDING", null, null, null, null, 50, 0, true);
120-
verify(pushStore).find(argThat(q -> q.getStatus() == PushStatus.PENDING));
120+
verify(pushStore).findSummaries(argThat(q -> q.getStatus() == PushStatus.PENDING));
121121
}
122122
}
123123

0 commit comments

Comments
 (0)