chore(git): Add push test + Fix failing unit test #12350

Author: wanling0000

docs/code-howtos/git.md

@@ -1,6 +1,7 @@
 # git
 
 ## Conflict Scenarios
+
 - **T1.** Remote changed a field, local did not  
   → No conflict.  
   The local version remained unchanged, so the remote change can be safely applied.
@@ -68,4 +69,3 @@
 - **T17.** Both added the same entry key with identical values  
   → No conflict.  
   Both sides created a new entry with the same citation key and identical fields, so it can be merged safely.
-

jabgui/src/main/java/org/jabref/gui/git/GitPullViewModel.java

@@ -29,6 +29,10 @@
 import org.eclipse.jgit.api.errors.GitAPIException;
 import org.eclipse.jgit.revwalk.RevCommit;
 
+/**
+ * ViewModel responsible for coordinating UI-bound Git Pull workflow,
+ * including conflict resolution.
+ */
 public class GitPullViewModel extends AbstractViewModel {
     private final ImportFormatPreferences importFormatPreferences;
     private final GitConflictResolver conflictResolver;

jablib/src/main/java/org/jabref/logic/git/GitHandler.java

@@ -215,7 +215,7 @@ public void fetchOnCurrentBranch() throws IOException {
 
     /**
      * Try to locate the Git repository root by walking up the directory tree starting from the given path.
-     * If a directory containing a `.git` folder is found, a new GitHandler is created and returned.
+     * If a directory containing a .git folder is found, a new GitHandler is created and returned.
      *
      * @param anyPathInsideRepo Any file or directory path that is assumed to be inside a Git repository
      * @return Optional containing a GitHandler initialized with the repository root, or empty if not found
@@ -230,4 +230,12 @@ public static Optional<GitHandler> fromAnyPath(Path anyPathInsideRepo) {
         }
         return Optional.empty();
     }
+
+    public File getRepositoryPathAsFile() {
+        return repositoryPathAsFile;
+    }
+
+    public Git open() throws IOException {
+        return Git.open(this.repositoryPathAsFile);
+    }
 }

jablib/src/main/java/org/jabref/logic/git/GitSyncService.java

@@ -16,6 +16,9 @@
 import org.jabref.logic.git.merge.MergePlan;
 import org.jabref.logic.git.merge.SemanticMerger;
 import org.jabref.logic.git.model.MergeResult;
+import org.jabref.logic.git.status.GitStatusChecker;
+import org.jabref.logic.git.status.GitStatusSnapshot;
+import org.jabref.logic.git.status.SyncStatus;
 import org.jabref.logic.importer.ImportFormatPreferences;
 import org.jabref.model.database.BibDatabaseContext;
 import org.jabref.model.entry.BibEntry;
@@ -27,11 +30,23 @@
 import org.slf4j.LoggerFactory;
 
 /**
- * Orchestrator for git sync service
+ * GitSyncService currently serves as an orchestrator for Git pull/push logic.
  * if (hasConflict)
  *     → UI merge;
  * else
  *     → autoMerge := local + remoteDiff
+ *
+ * NOTICE：
+ * - TODO：This class will be **deprecated** in the near future to avoid architecture violation (logic → gui)!
+ * - The underlying business logic will not change significantly.
+ * - Only the coordination responsibilities will shift to GUI/ViewModel layer.
+ *
+ * PLAN:
+ * - All orchestration logic (pull/push, merge, resolve, commit)
+ *   will be **moved into corresponding ViewModels**, such as:
+ *     - GitPullViewModel
+ *     - GitPushViewModel
+ *     - GitStatusViewModel
  */
 public class GitSyncService {
     private static final Logger LOGGER = LoggerFactory.getLogger(GitSyncService.class);
@@ -49,24 +64,42 @@ public GitSyncService(ImportFormatPreferences importFormatPreferences, GitHandle
      * Called when user clicks Pull
      */
     public MergeResult fetchAndMerge(Path bibFilePath) throws GitAPIException, IOException, JabRefException {
-        Git git = Git.open(bibFilePath.getParent().toFile());
+        GitStatusSnapshot status = GitStatusChecker.checkStatus(bibFilePath);
 
-        // 1. fetch latest remote branch
-        gitHandler.fetchOnCurrentBranch();
-
-        // 2. Locating the base / local / remote versions
-        GitRevisionLocator locator = new GitRevisionLocator();
-        RevisionTriple triple = locator.locateMergeCommits(git);
+        if (!status.tracking()) {
+            LOGGER.warn("Pull aborted: The file is not under Git version control.");
+            return MergeResult.failure();
+        }
 
-        // 3. Calling semantic merge logic
-        MergeResult result = performSemanticMerge(git, triple.base(), triple.local(), triple.remote(), bibFilePath);
+        if (status.conflict()) {
+            LOGGER.warn("Pull aborted: Local repository has unresolved merge conflicts.");
+            return MergeResult.failure();
+        }
 
-        // 4. Automatic merge
-        if (result.isSuccessful()) {
-            gitHandler.createCommitOnCurrentBranch("Auto-merged by JabRef", !AMEND);
+        if (status.syncStatus() == SyncStatus.UP_TO_DATE || status.syncStatus() == SyncStatus.AHEAD) {
+            LOGGER.info("Pull skipped: Local branch is already up to date with remote.");
+            return MergeResult.success();
         }
 
-        return result;
+        // Status is BEHIND or DIVERGED
+        try (Git git = gitHandler.open()) {
+            // 1. Fetch latest remote branch
+            gitHandler.fetchOnCurrentBranch();
+
+            // 2. Locate base / local / remote commits
+            GitRevisionLocator locator = new GitRevisionLocator();
+            RevisionTriple triple = locator.locateMergeCommits(git);
+
+            // 3. Perform semantic merge
+            MergeResult result = performSemanticMerge(git, triple.base(), triple.local(), triple.remote(), bibFilePath);
+
+            // 4. Auto-commit merge result if successful
+            if (result.isSuccessful()) {
+                gitHandler.createCommitOnCurrentBranch("Auto-merged by JabRef", !AMEND);
+            }
+
+            return result;
+        }
     }
 
     public MergeResult performSemanticMerge(Git git,
@@ -80,7 +113,6 @@ public MergeResult performSemanticMerge(Git git,
         Path relativePath;
 
         // TODO: Validate that the .bib file is inside the Git repository earlier in the workflow.
-        // This check might be better placed before calling performSemanticMerge.
         if (!bibPath.startsWith(workTree)) {
             throw new IllegalStateException("Given .bib file is not inside repository");
         }
@@ -122,17 +154,61 @@ public MergeResult performSemanticMerge(Git git,
         return MergeResult.success();
     }
 
-    // WIP
-    // TODO: add test
-    public void push(Path bibFilePath) throws GitAPIException, IOException {
-        // 1. Auto-commit: commit if there are changes
-        boolean committed = gitHandler.createCommitOnCurrentBranch("Changes committed by JabRef", !AMEND);
-
-        // 2. push to remote
-        if (committed) {
-            gitHandler.pushCommitsToRemoteRepository();
-        } else {
-            LOGGER.info("No changes to commit — skipping push");
+    public void push(Path bibFilePath) throws GitAPIException, IOException, JabRefException {
+        GitStatusSnapshot status = GitStatusChecker.checkStatus(bibFilePath);
+
+        if (!status.tracking()) {
+            LOGGER.warn("Push aborted: file is not tracked by Git");
+            return;
+        }
+
+        switch (status.syncStatus()) {
+            case UP_TO_DATE -> {
+                boolean committed = gitHandler.createCommitOnCurrentBranch("Changes committed by JabRef", !AMEND);
+                if (committed) {
+                    gitHandler.pushCommitsToRemoteRepository();
+                } else {
+                    LOGGER.info("No changes to commit — skipping push");
+                }
+            }
+
+            case AHEAD -> {
+                gitHandler.pushCommitsToRemoteRepository();
+            }
+
+            case BEHIND -> {
+                LOGGER.warn("Push aborted: Local branch is behind remote. Please pull first.");
+            }
+
+            case DIVERGED -> {
+                try (Git git = gitHandler.open()) {
+                    GitRevisionLocator locator = new GitRevisionLocator();
+                    RevisionTriple triple = locator.locateMergeCommits(git);
+
+                    MergeResult mergeResult = performSemanticMerge(git, triple.base(), triple.local(), triple.remote(), bibFilePath);
+
+                    if (!mergeResult.isSuccessful()) {
+                        LOGGER.warn("Semantic merge failed — aborting push");
+                        return;
+                    }
+
+                    boolean committed = gitHandler.createCommitOnCurrentBranch("Merged changes", !AMEND);
+
+                    if (committed) {
+                        gitHandler.pushCommitsToRemoteRepository();
+                    } else {
+                        LOGGER.info("Nothing to commit after semantic merge — skipping push");
+                    }
+                }
+            }
+
+            case CONFLICT -> {
+                LOGGER.warn("Push aborted: Local repository has unresolved merge conflicts.");
+            }
+
+            case UNTRACKED, UNKNOWN -> {
+                LOGGER.warn("Push aborted: Untracked or unknown Git status.");
+            }
         }
     }
 }

jablib/src/main/java/org/jabref/logic/git/io/GitFileWriter.java

@@ -6,8 +6,8 @@
 import java.nio.file.Path;
 
 import org.jabref.logic.exporter.AtomicFileWriter;
+import org.jabref.logic.exporter.BibDatabaseWriter;
 import org.jabref.logic.exporter.BibWriter;
-import org.jabref.logic.exporter.BibtexDatabaseWriter;
 import org.jabref.logic.exporter.SelfContainedSaveConfiguration;
 import org.jabref.logic.importer.ImportFormatPreferences;
 import org.jabref.model.database.BibDatabaseContext;
@@ -21,7 +21,7 @@ public static void write(Path file, BibDatabaseContext bibDatabaseContext, Impor
         synchronized (bibDatabaseContext) {
             try (AtomicFileWriter fileWriter = new AtomicFileWriter(file, encoding, saveConfiguration.shouldMakeBackup())) {
                 BibWriter bibWriter = new BibWriter(fileWriter, bibDatabaseContext.getDatabase().getNewLineSeparator());
-                BibtexDatabaseWriter writer = new BibtexDatabaseWriter(
+                BibDatabaseWriter writer = new BibDatabaseWriter(
                         bibWriter,
                         saveConfiguration,
                         importPrefs.fieldPreferences(),

jablib/src/main/java/org/jabref/logic/git/io/GitRevisionLocator.java

@@ -7,8 +7,10 @@
 import org.eclipse.jgit.api.Git;
 import org.eclipse.jgit.api.errors.GitAPIException;
 import org.eclipse.jgit.lib.ObjectId;
+import org.eclipse.jgit.lib.Repository;
 import org.eclipse.jgit.revwalk.RevCommit;
 import org.eclipse.jgit.revwalk.RevWalk;
+import org.eclipse.jgit.revwalk.filter.RevFilter;
 
 /**
  * Find the base/local/remote three commits:
@@ -21,26 +23,31 @@ public class GitRevisionLocator {
     private static final String REMOTE = "refs/remotes/origin/main";
 
     public RevisionTriple locateMergeCommits(Git git) throws GitAPIException, IOException, JabRefException {
+        Repository repo = git.getRepository();
         // assumes the remote branch is 'origin/main'
-        ObjectId headId = git.getRepository().resolve(HEAD);
+        ObjectId headId = repo.resolve(HEAD);
         // and uses the default remote tracking reference
         // does not support multiple remotes or custom remote branch names so far
-        ObjectId remoteId = git.getRepository().resolve(REMOTE);
+        ObjectId remoteId = repo.resolve(REMOTE);
         if (remoteId == null) {
             throw new IllegalStateException("Remote branch missing origin/main.");
         }
 
         try (RevWalk walk = new RevWalk(git.getRepository())) {
             RevCommit local = walk.parseCommit(headId);
             RevCommit remote = walk.parseCommit(remoteId);
-
-            walk.setRevFilter(org.eclipse.jgit.revwalk.filter.RevFilter.MERGE_BASE);
-            walk.markStart(local);
-            walk.markStart(remote);
-
-            RevCommit base = walk.next();
+            RevCommit base = findMergeBase(repo, local, remote);
 
             return new RevisionTriple(base, local, remote);
         }
     }
+
+    public static RevCommit findMergeBase(Repository repo, RevCommit a, RevCommit b) throws IOException {
+        try (RevWalk walk = new RevWalk(repo)) {
+            walk.setRevFilter(RevFilter.MERGE_BASE);
+            walk.markStart(a);
+            walk.markStart(b);
+            return walk.next();
+        }
+    }
 }

jablib/src/main/java/org/jabref/logic/git/model/MergeResult.java

@@ -14,6 +14,10 @@ public static MergeResult success() {
         return new MergeResult(SUCCESS, List.of());
     }
 
+    public static MergeResult failure() {
+        return new MergeResult(false, List.of());
+    }
+
     public boolean hasConflicts() {
         return !conflicts.isEmpty();
     }

jablib/src/main/java/org/jabref/logic/git/status/GitStatusChecker.java

@@ -0,0 +1,90 @@
+package org.jabref.logic.git.status;
+
+import java.io.IOException;
+import java.nio.file.Path;
+import java.util.Optional;
+
+import org.jabref.logic.git.GitHandler;
+import org.jabref.logic.git.io.GitRevisionLocator;
+
+import org.eclipse.jgit.api.Git;
+import org.eclipse.jgit.api.Status;
+import org.eclipse.jgit.api.errors.GitAPIException;
+import org.eclipse.jgit.lib.ObjectId;
+import org.eclipse.jgit.lib.Repository;
+import org.eclipse.jgit.revwalk.RevCommit;
+import org.eclipse.jgit.revwalk.RevWalk;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * This class is used to determine the status of a Git repository from any given path inside it.
+ * If no repository is found, it returns a {@link GitStatusSnapshot} with tracking = false.
+ * Otherwise, it returns a full snapshot including tracking status, sync status, and conflict state.
+ */
+public class GitStatusChecker {
+    private static final Logger LOGGER = LoggerFactory.getLogger(GitStatusChecker.class);
+
+    public static GitStatusSnapshot checkStatus(Path anyPathInsideRepo) {
+        Optional<GitHandler> maybeHandler = GitHandler.fromAnyPath(anyPathInsideRepo);
+
+        if (maybeHandler.isEmpty()) {
+            return new GitStatusSnapshot(false, SyncStatus.UNTRACKED, false, Optional.empty());
+        }
+        GitHandler handler = maybeHandler.get();
+
+        try (Git git = Git.open(handler.getRepositoryPathAsFile())) {
+            Repository repo = git.getRepository();
+            Status status = git.status().call();
+            boolean hasConflict = !status.getConflicting().isEmpty();
+
+            ObjectId localHead = repo.resolve("HEAD");
+            ObjectId remoteHead = repo.resolve("refs/remotes/origin/main");
+            SyncStatus syncStatus = determineSyncStatus(repo, localHead, remoteHead);
+
+            return new GitStatusSnapshot(
+                    true,
+                    syncStatus,
+                    hasConflict,
+                    Optional.ofNullable(localHead).map(ObjectId::getName)
+            );
+        } catch (IOException | GitAPIException e) {
+            LOGGER.warn("Failed to check Git status: " + e.getMessage());
+            return new GitStatusSnapshot(
+                    true,
+                    SyncStatus.UNKNOWN,
+                    false,
+                    Optional.empty()
+            );
+        }
+    }
+
+    private static SyncStatus determineSyncStatus(Repository repo, ObjectId localHead, ObjectId remoteHead) throws IOException {
+        if (localHead == null || remoteHead == null) {
+            return SyncStatus.UNKNOWN;
+        }
+
+        if (localHead.equals(remoteHead)) {
+            return SyncStatus.UP_TO_DATE;
+        }
+
+        try (RevWalk walk = new RevWalk(repo)) {
+            RevCommit localCommit = walk.parseCommit(localHead);
+            RevCommit remoteCommit = walk.parseCommit(remoteHead);
+            RevCommit mergeBase = GitRevisionLocator.findMergeBase(repo, localCommit, remoteCommit);
+
+            boolean ahead = !localCommit.equals(mergeBase);
+            boolean behind = !remoteCommit.equals(mergeBase);
+
+            if (ahead && behind) {
+                return SyncStatus.DIVERGED;
+            } else if (ahead) {
+                return SyncStatus.AHEAD;
+            } else if (behind) {
+                return SyncStatus.BEHIND;
+            } else {
+                return SyncStatus.UNKNOWN;
+            }
+        }
+    }
+}

jablib/src/main/java/org/jabref/logic/git/status/GitStatusSnapshot.java

@@ -0,0 +1,9 @@
+package org.jabref.logic.git.status;
+
+import java.util.Optional;
+
+public record GitStatusSnapshot(
+    boolean tracking,
+    SyncStatus syncStatus,
+    boolean conflict,
+    Optional<String> lastPulledCommit) { }