Skip to content

Commit 350d39d

Browse files
authored
fix: skip diff generation for tag pushes (#255)
## Summary - Skip diff generation in `DiffGenerationHook` (store-and-forward) when the pushed ref is under `refs/tags/` - Skip diff generation in `ScanDiffFilter` (transparent proxy) via the existing `isTagPush()` check on `GitRequestDetails` - Add `DiffGenerationHookTest` covering branch push, tag push skip, and delete skip - Add `tagPush_skipsDiffGeneration` test to `ScanDiffFilterTest` Closes #240 ## Test plan - [ ] `./gradlew :git-proxy-java-core:test` passes - [ ] Push a release tag through the proxy and confirm no diff record is created
2 parents d388e45 + 291d30c commit 350d39d

5 files changed

Lines changed: 114 additions & 4 deletions

File tree

git-proxy-java-core/src/main/java/org/finos/gitproxy/git/DiffGenerationHook.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,11 @@ public void onPreReceive(ReceivePack rp, Collection<ReceiveCommand> commands) {
5050
}
5151

5252
String refName = cmd.getRefName();
53+
54+
if (refName.startsWith("refs/tags/")) {
55+
log.debug("Skipping diff generation for tag push: {}", refName);
56+
continue;
57+
}
5358
String commitFrom = cmd.getOldId().name();
5459
String commitTo = cmd.getNewId().name();
5560
boolean isNewBranch = ObjectId.zeroId().equals(cmd.getOldId());

git-proxy-java-core/src/main/java/org/finos/gitproxy/servlet/filter/ScanDiffFilter.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,11 @@ public void doHttpFilter(HttpServletRequest request, HttpServletResponse respons
6363
return;
6464
}
6565

66+
if (requestDetails.isTagPush()) {
67+
log.debug("Skipping diff generation for tag push: {}", requestDetails.getBranch());
68+
return;
69+
}
70+
6671
String fromCommit = requestDetails.getCommitFrom();
6772
String toCommit = requestDetails.getCommitTo();
6873
if (toCommit == null || toCommit.isEmpty()) {
Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
package org.finos.gitproxy.git;
2+
3+
import static org.junit.jupiter.api.Assertions.*;
4+
5+
import java.io.File;
6+
import java.nio.file.Files;
7+
import java.nio.file.Path;
8+
import java.util.List;
9+
import org.eclipse.jgit.api.Git;
10+
import org.eclipse.jgit.lib.ObjectId;
11+
import org.eclipse.jgit.lib.PersonIdent;
12+
import org.eclipse.jgit.lib.Repository;
13+
import org.eclipse.jgit.revwalk.RevCommit;
14+
import org.eclipse.jgit.transport.ReceiveCommand;
15+
import org.eclipse.jgit.transport.ReceivePack;
16+
import org.junit.jupiter.api.BeforeEach;
17+
import org.junit.jupiter.api.Test;
18+
import org.junit.jupiter.api.io.TempDir;
19+
20+
class DiffGenerationHookTest {
21+
22+
@TempDir
23+
Path tempDir;
24+
25+
Git git;
26+
Repository repo;
27+
ObjectId c1;
28+
ObjectId c2;
29+
30+
@BeforeEach
31+
void setUp() throws Exception {
32+
git = Git.init().setDirectory(tempDir.toFile()).call();
33+
repo = git.getRepository();
34+
repo.getConfig().setBoolean("commit", null, "gpgsign", false);
35+
repo.getConfig().save();
36+
c1 = commit("init.txt", "initial content").getId();
37+
c2 = commit("change.txt", "second content").getId();
38+
}
39+
40+
private RevCommit commit(String filename, String content) throws Exception {
41+
File f = tempDir.resolve(filename).toFile();
42+
Files.writeString(f.toPath(), content + "\n");
43+
git.add().addFilepattern(".").call();
44+
return git.commit()
45+
.setAuthor(new PersonIdent("Dev", "dev@example.com"))
46+
.setCommitter(new PersonIdent("Dev", "dev@example.com"))
47+
.setMessage("add " + filename)
48+
.call();
49+
}
50+
51+
@Test
52+
void branchPush_generatesDiffStep() {
53+
ReceivePack rp = new ReceivePack(repo);
54+
ReceiveCommand cmd = new ReceiveCommand(c1, c2, "refs/heads/main", ReceiveCommand.Type.UPDATE);
55+
PushContext ctx = new PushContext();
56+
57+
new DiffGenerationHook(ctx).onPreReceive(rp, List.of(cmd));
58+
59+
boolean hasDiff =
60+
ctx.getSteps().stream().anyMatch(s -> DiffGenerationHook.STEP_NAME_PUSH_DIFF.equals(s.getStepName()));
61+
assertTrue(hasDiff, "branch push must generate a diff step");
62+
}
63+
64+
@Test
65+
void tagPush_skipsDiffGeneration() {
66+
ReceivePack rp = new ReceivePack(repo);
67+
ReceiveCommand cmd = new ReceiveCommand(ObjectId.zeroId(), c2, "refs/tags/v1.0.0");
68+
PushContext ctx = new PushContext();
69+
70+
new DiffGenerationHook(ctx).onPreReceive(rp, List.of(cmd));
71+
72+
boolean hasDiff =
73+
ctx.getSteps().stream().anyMatch(s -> DiffGenerationHook.STEP_NAME_PUSH_DIFF.equals(s.getStepName()));
74+
assertFalse(hasDiff, "tag push must not generate a diff step");
75+
}
76+
77+
@Test
78+
void deleteCommand_skipped() {
79+
ReceivePack rp = new ReceivePack(repo);
80+
ReceiveCommand cmd = new ReceiveCommand(c1, ObjectId.zeroId(), "refs/heads/main", ReceiveCommand.Type.DELETE);
81+
PushContext ctx = new PushContext();
82+
83+
new DiffGenerationHook(ctx).onPreReceive(rp, List.of(cmd));
84+
85+
assertTrue(ctx.getSteps().isEmpty(), "delete command must not generate any steps");
86+
}
87+
}

git-proxy-java-core/src/test/java/org/finos/gitproxy/servlet/filter/ScanDiffFilterTest.java

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -272,6 +272,22 @@ void blockedLiteral_stepHasSpecificErrorMessageAndMatchingLine() throws Exceptio
272272
"content should include the matching line, got: " + scanStep.getContent());
273273
}
274274

275+
// ---- tag push → skipped entirely, no diff step ----
276+
277+
@Test
278+
void tagPush_skipsDiffGeneration() throws Exception {
279+
GitRequestDetails details = pushDetails(baseCommit, cleanCommit);
280+
details.setBranch("refs/tags/v1.0.0");
281+
FakeResponse resp = new FakeResponse();
282+
283+
filterNoRules().doHttpFilter(mockRequest(details), resp.mock);
284+
285+
assertFalse(resp.committed.get());
286+
boolean hasDiffStep = details.getSteps().stream().anyMatch(s -> "diff".equals(s.getStepName()));
287+
assertFalse(hasDiffStep, "tag push must not generate a diff step");
288+
assertEquals(GitRequestDetails.GitResult.PENDING, details.getResult());
289+
}
290+
275291
// ---- diff step content contains the actual diff text ----
276292

277293
@Test

git-proxy-java-dashboard/frontend/src/pages/PushList.tsx

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -171,10 +171,7 @@ export function PushList({ currentUser }: PushListProps) {
171171
// Reload counts when filter-relevant state changes, with auto-refresh
172172
useEffect(() => {
173173
void Promise.resolve().then(() => loadCounts(filterRepo, myPushesOnly))
174-
const timer = setInterval(
175-
() => loadCounts(filterRepo, myPushesOnly),
176-
10_000,
177-
)
174+
const timer = setInterval(() => loadCounts(filterRepo, myPushesOnly), 10_000)
178175
return () => clearInterval(timer)
179176
}, [filterRepo, myPushesOnly, loadCounts])
180177

0 commit comments

Comments
 (0)