Skip to content

Commit d10fecf

Browse files
coopernetesclaude
andcommitted
refactor: replace raw JDBC in JdbcPushStore with Spring JDBC abstractions
- Add spring-jdbc and spring-tx dependencies to jgit-proxy-core - Replace PreparedStatement / ResultSet / manual tx with NamedParameterJdbcTemplate and TransactionTemplate (DataSourceTransactionManager built internally) - Extract four RowMapper<T> singletons: PushRecordRowMapper, PushStepRowMapper, PushCommitRowMapper, AttestationRowMapper - Add hydrate() helper to load child collections after primary row fetch - Add integration tests: commit round-trip (PushCommit + signed-off-by list) and LIKE search (find by project/repoName substring) Closes #9 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 908460e commit d10fecf

7 files changed

Lines changed: 424 additions & 326 deletions

File tree

jgit-proxy-core/build.gradle

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,10 @@ dependencies {
6969
api "org.eclipse.jetty.ee11:jetty-ee11-servlet:${jettyVersion}"
7070
api "org.eclipse.jetty.ee11:jetty-ee11-proxy:${jettyVersion}"
7171

72+
// Spring JDBC (NamedParameterJdbcTemplate, RowMapper) + transaction abstraction
73+
implementation "org.springframework:spring-jdbc:${springVersion}"
74+
implementation "org.springframework:spring-tx:${springVersion}"
75+
7276
// Database - connection pooling
7377
api "com.zaxxer:HikariCP:${hikariVersion}"
7478

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

Lines changed: 181 additions & 326 deletions
Large diffs are not rendered by default.
Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
package org.finos.gitproxy.db.jdbc.mapper;
2+
3+
import java.sql.ResultSet;
4+
import java.sql.SQLException;
5+
import java.sql.Timestamp;
6+
import java.time.Instant;
7+
import org.finos.gitproxy.db.model.Attestation;
8+
import org.springframework.jdbc.core.RowMapper;
9+
10+
/** Maps a {@code push_attestations} result-set row to an {@link Attestation}. */
11+
public final class AttestationRowMapper implements RowMapper<Attestation> {
12+
13+
public static final AttestationRowMapper INSTANCE = new AttestationRowMapper();
14+
15+
private AttestationRowMapper() {}
16+
17+
@Override
18+
public Attestation mapRow(ResultSet rs, int rowNum) throws SQLException {
19+
return Attestation.builder()
20+
.pushId(rs.getString("push_id"))
21+
.type(Attestation.Type.valueOf(rs.getString("type")))
22+
.reviewerUsername(rs.getString("reviewer_username"))
23+
.reviewerEmail(rs.getString("reviewer_email"))
24+
.reason(rs.getString("reason"))
25+
.automated(rs.getBoolean("automated"))
26+
.timestamp(toInstant(rs.getTimestamp("timestamp")))
27+
.build();
28+
}
29+
30+
private static Instant toInstant(Timestamp ts) {
31+
return ts != null ? ts.toInstant() : null;
32+
}
33+
}
Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
package org.finos.gitproxy.db.jdbc.mapper;
2+
3+
import java.sql.ResultSet;
4+
import java.sql.SQLException;
5+
import java.sql.Timestamp;
6+
import java.time.Instant;
7+
import java.util.ArrayList;
8+
import java.util.Arrays;
9+
import java.util.List;
10+
import org.finos.gitproxy.db.model.PushCommit;
11+
import org.springframework.jdbc.core.RowMapper;
12+
13+
/** Maps a {@code push_commits} result-set row to a {@link PushCommit}. */
14+
public final class PushCommitRowMapper implements RowMapper<PushCommit> {
15+
16+
public static final PushCommitRowMapper INSTANCE = new PushCommitRowMapper();
17+
18+
private PushCommitRowMapper() {}
19+
20+
@Override
21+
public PushCommit mapRow(ResultSet rs, int rowNum) throws SQLException {
22+
String sobRaw = rs.getString("signed_off_by");
23+
List<String> signedOffBy =
24+
(sobRaw != null && !sobRaw.isBlank()) ? Arrays.asList(sobRaw.split("\n")) : new ArrayList<>();
25+
return PushCommit.builder()
26+
.pushId(rs.getString("push_id"))
27+
.sha(rs.getString("sha"))
28+
.parentSha(rs.getString("parent_sha"))
29+
.authorName(rs.getString("author_name"))
30+
.authorEmail(rs.getString("author_email"))
31+
.committerName(rs.getString("committer_name"))
32+
.committerEmail(rs.getString("committer_email"))
33+
.message(rs.getString("message"))
34+
.commitDate(toInstant(rs.getTimestamp("commit_date")))
35+
.signature(rs.getString("signature"))
36+
.signedOffBy(signedOffBy)
37+
.build();
38+
}
39+
40+
private static Instant toInstant(Timestamp ts) {
41+
return ts != null ? ts.toInstant() : null;
42+
}
43+
}
Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
package org.finos.gitproxy.db.jdbc.mapper;
2+
3+
import java.sql.ResultSet;
4+
import java.sql.SQLException;
5+
import java.sql.Timestamp;
6+
import java.time.Instant;
7+
import org.finos.gitproxy.db.model.PushRecord;
8+
import org.finos.gitproxy.db.model.PushStatus;
9+
import org.springframework.jdbc.core.RowMapper;
10+
11+
/** Maps a {@code push_records} result-set row to a {@link PushRecord}. */
12+
public final class PushRecordRowMapper implements RowMapper<PushRecord> {
13+
14+
public static final PushRecordRowMapper INSTANCE = new PushRecordRowMapper();
15+
16+
private PushRecordRowMapper() {}
17+
18+
@Override
19+
public PushRecord mapRow(ResultSet rs, int rowNum) throws SQLException {
20+
return PushRecord.builder()
21+
.id(rs.getString("id"))
22+
.timestamp(toInstant(rs.getTimestamp("timestamp")))
23+
.url(rs.getString("url"))
24+
.upstreamUrl(rs.getString("upstream_url"))
25+
.project(rs.getString("project"))
26+
.repoName(rs.getString("repo_name"))
27+
.branch(rs.getString("branch"))
28+
.commitFrom(rs.getString("commit_from"))
29+
.commitTo(rs.getString("commit_to"))
30+
.message(rs.getString("message"))
31+
.author(rs.getString("author"))
32+
.authorEmail(rs.getString("author_email"))
33+
.committer(rs.getString("committer"))
34+
.committerEmail(rs.getString("committer_email"))
35+
.user(rs.getString("push_user"))
36+
.userEmail(rs.getString("user_email"))
37+
.method(rs.getString("method"))
38+
.status(PushStatus.valueOf(rs.getString("status")))
39+
.errorMessage(rs.getString("error_message"))
40+
.blockedMessage(rs.getString("blocked_message"))
41+
.autoApproved(rs.getBoolean("auto_approved"))
42+
.autoRejected(rs.getBoolean("auto_rejected"))
43+
.build();
44+
}
45+
46+
private static Instant toInstant(Timestamp ts) {
47+
return ts != null ? ts.toInstant() : null;
48+
}
49+
}
Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
package org.finos.gitproxy.db.jdbc.mapper;
2+
3+
import java.sql.ResultSet;
4+
import java.sql.SQLException;
5+
import java.sql.Timestamp;
6+
import java.time.Instant;
7+
import java.util.ArrayList;
8+
import java.util.Arrays;
9+
import java.util.List;
10+
import org.finos.gitproxy.db.model.PushStep;
11+
import org.finos.gitproxy.db.model.StepStatus;
12+
import org.springframework.jdbc.core.RowMapper;
13+
14+
/** Maps a {@code push_steps} result-set row to a {@link PushStep}. */
15+
public final class PushStepRowMapper implements RowMapper<PushStep> {
16+
17+
public static final PushStepRowMapper INSTANCE = new PushStepRowMapper();
18+
19+
private static final String LOG_SEPARATOR = "\n---LOG---\n";
20+
21+
private PushStepRowMapper() {}
22+
23+
@Override
24+
public PushStep mapRow(ResultSet rs, int rowNum) throws SQLException {
25+
return PushStep.builder()
26+
.id(rs.getString("id"))
27+
.pushId(rs.getString("push_id"))
28+
.stepName(rs.getString("step_name"))
29+
.stepOrder(rs.getInt("step_order"))
30+
.status(StepStatus.valueOf(rs.getString("status")))
31+
.content(rs.getString("content"))
32+
.errorMessage(rs.getString("error_message"))
33+
.blockedMessage(rs.getString("blocked_message"))
34+
.logs(splitLogs(rs.getString("logs")))
35+
.timestamp(toInstant(rs.getTimestamp("timestamp")))
36+
.build();
37+
}
38+
39+
public static String joinLogs(List<String> logs) {
40+
if (logs == null || logs.isEmpty()) return null;
41+
return String.join(LOG_SEPARATOR, logs);
42+
}
43+
44+
private static List<String> splitLogs(String logs) {
45+
if (logs == null || logs.isEmpty()) return new ArrayList<>();
46+
return new ArrayList<>(Arrays.asList(logs.split(LOG_SEPARATOR)));
47+
}
48+
49+
private static Instant toInstant(Timestamp ts) {
50+
return ts != null ? ts.toInstant() : null;
51+
}
52+
}

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

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -287,6 +287,68 @@ void multipleSaves_allVisible() {
287287
assertEquals(20, all.size());
288288
}
289289

290+
// ---- commits persistence ----
291+
292+
@Test
293+
void save_withCommits_commitsRoundTripCorrectly() {
294+
PushRecord r = record("abc", "refs/heads/main", "repo");
295+
org.finos.gitproxy.db.model.PushCommit commit = org.finos.gitproxy.db.model.PushCommit.builder()
296+
.pushId(r.getId())
297+
.sha("deadbeef")
298+
.parentSha("cafebabe")
299+
.authorName("Alice")
300+
.authorEmail("alice@example.com")
301+
.committerName("Alice")
302+
.committerEmail("alice@example.com")
303+
.message("feat: add thing")
304+
.signedOffBy(List.of("Alice <alice@example.com>", "Bob <bob@example.com>"))
305+
.build();
306+
r.setCommits(List.of(commit));
307+
store.save(r);
308+
309+
PushRecord loaded = store.findById(r.getId()).orElseThrow();
310+
311+
assertEquals(1, loaded.getCommits().size());
312+
org.finos.gitproxy.db.model.PushCommit loadedCommit =
313+
loaded.getCommits().get(0);
314+
assertEquals("deadbeef", loadedCommit.getSha());
315+
assertEquals("cafebabe", loadedCommit.getParentSha());
316+
assertEquals("Alice", loadedCommit.getAuthorName());
317+
assertEquals("alice@example.com", loadedCommit.getAuthorEmail());
318+
assertEquals("feat: add thing", loadedCommit.getMessage());
319+
assertEquals(List.of("Alice <alice@example.com>", "Bob <bob@example.com>"), loadedCommit.getSignedOffBy());
320+
}
321+
322+
// ---- find with search ----
323+
324+
@Test
325+
void find_bySearch_matchesProjectAndRepoName() {
326+
PushRecord r1 = PushRecord.builder()
327+
.project("finos")
328+
.repoName("git-proxy")
329+
.commitTo("a")
330+
.branch("refs/heads/main")
331+
.build();
332+
PushRecord r2 = PushRecord.builder()
333+
.project("acme")
334+
.repoName("widget-service")
335+
.commitTo("b")
336+
.branch("refs/heads/main")
337+
.build();
338+
store.save(r1);
339+
store.save(r2);
340+
341+
List<PushRecord> results =
342+
store.find(PushQuery.builder().search("finos").build());
343+
assertEquals(1, results.size());
344+
assertEquals("finos", results.get(0).getProject());
345+
346+
List<PushRecord> byRepo =
347+
store.find(PushQuery.builder().search("widget").build());
348+
assertEquals(1, byRepo.size());
349+
assertEquals("widget-service", byRepo.get(0).getRepoName());
350+
}
351+
290352
// ---- initialize idempotency ----
291353

292354
@Test

0 commit comments

Comments
 (0)