Skip to content

Fix: Decouple GitHandler creation via registry and extend semantic conflict detection #13666

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 13 commits into from
Aug 7, 2025
Merged

Fix: Decouple GitHandler creation via registry and extend semantic conflict detection #13666

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
d2dc011
refactor GitHandlerRegistry
wanling0000 Aug 6, 2025
ba77395
Merge `upstream/main` into `fix/semantic-conflict-logic`
wanling0000 Aug 6, 2025
19d098b
refactor SemanticConflictDetector
wanling0000 Aug 6, 2025
7358c89
chore(rewrite)
wanling0000 Aug 6, 2025
93460f1
test
wanling0000 Aug 6, 2025
687698c
fix: add null check
wanling0000 Aug 6, 2025
09fd2ad
Merge `upstream/main` into `fix/semantic-conflict-logic`
wanling0000 Aug 6, 2025
8a7d1f6
fix: SemanticConflictDetector logic
wanling0000 Aug 6, 2025
8de1d4f
refactor: remove redundant deletion checks
wanling0000 Aug 6, 2025
88de9df
Replace redundant null checks and use equals() where safe
wanling0000 Aug 6, 2025
07d9632
Merge `upstream/main` into `fix/semantic-conflict-logic`
wanling0000 Aug 6, 2025
3da6327
chore(rewrite)
wanling0000 Aug 6, 2025
1ef4897
chore(docs): adjust table formatting
wanling0000 Aug 7, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
83 changes: 64 additions & 19 deletions docs/code-howtos/git.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,22 +84,67 @@ The semantic conflict detection and merge resolution logic is covered by:

The following table describes when semantic merge in JabRef should consider a situation as conflict or not during a three-way merge.

| ID | Base | Local Change | Remote Change | Result |
|------|----------------------------|------------------------------------|------------------------------------|--------|
| T1 | Field present | (unchanged) | Field modified | No conflict. The local version remained unchanged, so the remote change can be safely applied. |
| T2 | Field present | Field modified | (unchanged) | No conflict. The remote version did not touch the field, so the local change is preserved. |
| T3 | Field present | Field changed to same value | Field changed to same value | No conflict. Although both sides changed the field, the result is identical—therefore, no conflict. |
| T4 | Field present | Field changed to A | Field changed to B | Conflict. This is a true semantic conflict that requires resolution. |
| T5 | Field present | Field deleted | Field modified | Conflict. One side deleted the field while the other updated it—this is contradictory. |
| T6 | Field present | Field modified | Field deleted | Conflict. Similar to T5, one side deletes, the other edits—this is a conflict. |
| T7 | Field present | (unchanged) | Field deleted | No conflict. Local did not modify anything, so remote deletion is accepted. |
| T8 | Entry with fields A and B | Field A modified | Field B modified | No conflict. Changes are on separate fields, so they can be merged safely. |
| T9 | Entry with fields A and B | Field order changed | Field order changed differently | No conflict. Field order is not semantically meaningful, so no conflict is detected. |
| T10 | Entries A and B | Entry A modified | Entry B modified | No conflict. Modifications are on different entries, which are always safe to merge. |
| T11 | Entry with existing fields | (unchanged) | New field added | No conflict. Remote addition can be applied without issues. |
| T12 | Entry with existing fields | New field added with value A | New field added with value B | Conflict. One side added while the other side modified—there is a semantic conflict. |
| T13 | Entry with existing fields | New field added | (unchanged) | No conflict. Safe to preserve the local addition. |
| T14 | Entry with existing fields | New field added with value A | New field added with value A | No conflict. Even though both sides added it, the value is the same—no need for resolution. |
| T15 | Entry with existing fields | New field added with value A | New field added with value B | Conflict. The same field is introduced with different values, which creates a conflict. |
| T16 | (entry not present) | New entry with author A | New entry with author B | Conflict. Both sides created a new entry with the same citation key, but the fields differ. |
| T17 | (entry not present) | New entry with identical fields | New entry with identical fields | No conflict. Both sides created a new entry with the same citation key and identical fields, so it can be merged safely. |
### Entry-level Conflict Cases (E-series)

Each side (Base, Local, Remote) can take one of the following values:

* `–`: entry does not exist (null)
* `S`: same as base
* `M`: modified (fields changed)

> Note: Citation key is used as the entry identifier. Renaming a citation key is currently treated as deletion + addition and not supported as a standalone diff.

| TestID | Base | Local | Remote | Description | Common Scenario | Conflict |
| ------ | ------------- | ----------------- | ----------------- | -------------------------------------------------------------- | ------------------------------------------------------------------------------ | -------- |
| E01 | – | – | – | All null | Entry absent on all sides | No |
| E02 | – | – | M | Remote added entry | Accept remote addition | No |
| E03 | – | M | – | Local added entry | Keep local addition | No |
| E04 | – | M | M | Both added entry with same citation key | If content is identical: no conflict; else: compare fields | Depends |
| E05 | S | – | – | Both deleted | Safe deletion | No |
| E06 | S | – | S | Local deleted, remote unchanged | Respect local deletion | No |
| E07 | S | – | M | Local deleted, remote modified | One side deleted, one side changed | Yes |
| E08 | S | S | – | Remote deleted, local unchanged | Accept remote deletion as no conflict | No |
| E09 | S | S | S | All sides equal | No changes | No |
| E10 | S | S | M | Remote modified, local unchanged | Accept remote changes | No |
| E11 | S | M | – | Remote deleted, local modified | One side deleted, one side changed | Yes |
| E12 | S | M | S | Local modified, remote unchanged | Accept local changes | No |
| E13 | S | M | M | Both sides modified | If changes are equal or to different fields: no conflict; else: compare fields | Depends |
| E14a | `@article{a}` | `@article{b}` | unchanged | Local renamed citation key | Treated as deletion + addition | Yes |
| E14b | `@article{a}` | unchanged | `@article{b}` | Remote renamed citation key | Treated as deletion + addition | Yes |
| E14c | `@article{a}` | `@article{b}` | `@article{c}` | Both renamed to different keys | Treated as deletion + addition | Yes |
| E15 | – | `@article{a,...}` | `@article{a,...}` | Both added entry with same citation key, but different content | Duplicate citation key from both sides | Yes |

---

### Field-level Conflict Cases (F-series)

Each individual field (such as title, author, etc.) may have one of the following statuses relative to the base version:

* Unchanged: The field value is exactly the same as in the base version.
* Changed: The field value is different from the base version.
* Deleted: The field existed in the base version but is now missing (i.e., null).
* Added: The field did not exist in the base version but was added in the local or remote version.

| TestID | Base | Local | Remote | Description | Conflict |
| ------ | ----------------------------- | ------------------- | ------------------- | ------------------------------------------ |----------|
| F01 | U | U | U | All equal | No |
| F02 | U | U | C | Remote changed | No |
| F03 | U | C | U | Local changed | No |
| F04 | U | C | C (=) | Both changed to same value | No |
| F05 | U | C | C (≠) | Both changed same field, different values | Yes |
| F06 | U | D | U | Local deleted | No |
| F07 | U | U | D | Remote deleted | No |
| F08 | U | D | D | Both deleted | No |
| F09 | U | C | D | Local changed, remote deleted | Yes |
| F10 | U | D | C | Local deleted, remote changed | Yes |
| F11 | – | – | – | Field missing on all sides | No |
| F12 | – | A | – | Local added field | No |
| F13 | – | – | A | Remote added field | No |
| F14 | – | A | A (=) | Both added same field with same value | No |
| F15 | – | A | A (≠) | Both added same field with different values | Yes |
| F16 | U | C | D | Changed in local, deleted in remote | Yes |
| F17 | U | D | C | Deleted in local, changed in remote | Yes |
| F18 | – | A | C | No base, both sides added different values | Yes |
| F19 | `{title=Hello, author=Alice}` | reordered | unchanged | Field order changed only | No |
| F20 | `@article{a}` | `@inproceedings{a}` | unchanged | Entry type changed in local | No |
| F21 | `@article{a}` | `@book{a}` | `@inproceedings{a}` | Both changed entry type differently | Yes |
8 changes: 6 additions & 2 deletions jabgui/src/main/java/org/jabref/gui/git/GitPullAction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
import org.jabref.logic.git.conflicts.GitConflictResolverStrategy;
import org.jabref.logic.git.merge.GitSemanticMergeExecutor;
import org.jabref.logic.git.merge.GitSemanticMergeExecutorImpl;
import org.jabref.logic.git.util.GitHandlerRegistry;
import org.jabref.logic.l10n.Localization;
import org.jabref.logic.util.BackgroundTask;
import org.jabref.logic.util.TaskExecutor;
Expand All @@ -27,15 +28,18 @@ public class GitPullAction extends SimpleCommand {
private final StateManager stateManager;
private final GuiPreferences guiPreferences;
private final TaskExecutor taskExecutor;
private final GitHandlerRegistry handlerRegistry;

public GitPullAction(DialogService dialogService,
StateManager stateManager,
GuiPreferences guiPreferences,
TaskExecutor taskExecutor) {
TaskExecutor taskExecutor,
GitHandlerRegistry handlerRegistry) {
this.dialogService = dialogService;
this.stateManager = stateManager;
this.guiPreferences = guiPreferences;
this.taskExecutor = taskExecutor;
this.handlerRegistry = handlerRegistry;
}

@Override
Expand Down Expand Up @@ -65,7 +69,7 @@ public void execute() {
GitConflictResolverStrategy resolver = new GuiGitConflictResolverStrategy(dialog);
GitSemanticMergeExecutor mergeExecutor = new GitSemanticMergeExecutorImpl(guiPreferences.getImportFormatPreferences());

GitSyncService syncService = new GitSyncService(guiPreferences.getImportFormatPreferences(), handler, resolver, mergeExecutor);
GitSyncService syncService = new GitSyncService(guiPreferences.getImportFormatPreferences(), handlerRegistry, resolver, mergeExecutor);
GitStatusViewModel statusViewModel = new GitStatusViewModel(stateManager, bibFilePath);
GitPullViewModel viewModel = new GitPullViewModel(syncService, statusViewModel);

Expand Down
2 changes: 1 addition & 1 deletion jabgui/src/main/java/org/jabref/gui/git/GitStatusViewModel.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ protected void updateStatusFromContext(BibDatabaseContext context) {
}
this.activeHandler = gitHandlerOpt.get();

GitStatusSnapshot snapshot = GitStatusChecker.checkStatus(path);
GitStatusSnapshot snapshot = GitStatusChecker.checkStatus(activeHandler);
setTracking(snapshot.tracking());
setSyncStatus(snapshot.syncStatus());
setConflictDetected(snapshot.conflict());
Expand Down
1 change: 1 addition & 0 deletions jablib/src/main/java/module-info.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,7 @@
exports org.jabref.logic.git.model;
exports org.jabref.logic.git.status;
exports org.jabref.logic.command;
exports org.jabref.logic.git.util;

requires java.base;

Expand Down
15 changes: 10 additions & 5 deletions jablib/src/main/java/org/jabref/logic/git/GitHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -217,22 +217,27 @@ 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.
* <p>
* If a directory containing a .git folder is found, return that path.
*
* @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
* @param anyPathInsideRepo the file or directory path that is assumed to be located inside a Git repository
* @return an optional containing the path to the Git repository root if found
*/
public static Optional<GitHandler> fromAnyPath(Path anyPathInsideRepo) {
public static Optional<Path> findRepositoryRoot(Path anyPathInsideRepo) {
Path current = anyPathInsideRepo.toAbsolutePath();
while (current != null) {
if (Files.exists(current.resolve(".git"))) {
return Optional.of(new GitHandler(current));
return Optional.of(current);
}
current = current.getParent();
}
return Optional.empty();
}

public static Optional<GitHandler> fromAnyPath(Path anyPathInsideRepo) {
return findRepositoryRoot(anyPathInsideRepo).map(GitHandler::new);
}

public File getRepositoryPathAsFile() {
return repositoryPathAsFile;
}
Expand Down
28 changes: 19 additions & 9 deletions jablib/src/main/java/org/jabref/logic/git/GitSyncService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
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.git.util.GitHandlerRegistry;
import org.jabref.logic.importer.ImportFormatPreferences;
import org.jabref.model.database.BibDatabaseContext;
import org.jabref.model.entry.BibEntry;
Expand All @@ -39,25 +40,26 @@ public class GitSyncService {

private static final boolean AMEND = true;
private final ImportFormatPreferences importFormatPreferences;
private final GitHandler gitHandler;
private final GitHandlerRegistry gitHandlerRegistry;
private final GitConflictResolverStrategy gitConflictResolverStrategy;
private final GitSemanticMergeExecutor mergeExecutor;

public GitSyncService(ImportFormatPreferences importFormatPreferences, GitHandler gitHandler, GitConflictResolverStrategy gitConflictResolverStrategy, GitSemanticMergeExecutor mergeExecutor) {
public GitSyncService(ImportFormatPreferences importFormatPreferences, GitHandlerRegistry gitHandlerRegistry, GitConflictResolverStrategy gitConflictResolverStrategy, GitSemanticMergeExecutor mergeExecutor) {
this.importFormatPreferences = importFormatPreferences;
this.gitHandler = gitHandler;
this.gitHandlerRegistry = gitHandlerRegistry;
this.gitConflictResolverStrategy = gitConflictResolverStrategy;
this.mergeExecutor = mergeExecutor;
}

public MergeResult fetchAndMerge(BibDatabaseContext localDatabaseContext, Path bibFilePath) throws GitAPIException, IOException, JabRefException {
Optional<GitHandler> gitHandlerOpt = GitHandler.fromAnyPath(bibFilePath);
if (gitHandlerOpt.isEmpty()) {
LOGGER.warn("Pull aborted: The file is not inside a Git repository.");
Optional<Path> repoRoot = GitHandler.findRepositoryRoot(bibFilePath);
if (repoRoot.isEmpty()) {
LOGGER.warn("Path is not inside a Git repository.");
return MergeResult.failure();
}
GitHandler gitHandler = gitHandlerRegistry.get(repoRoot.get());

GitStatusSnapshot status = GitStatusChecker.checkStatus(bibFilePath);
GitStatusSnapshot status = GitStatusChecker.checkStatus(gitHandler);

if (!status.tracking()) {
LOGGER.warn("Pull aborted: The file is not under Git version control.");
Expand Down Expand Up @@ -151,10 +153,18 @@ public MergeResult performSemanticMerge(Git git,
}

public void push(BibDatabaseContext localDatabaseContext, Path bibFilePath) throws GitAPIException, IOException, JabRefException {
GitStatusSnapshot status = GitStatusChecker.checkStatus(bibFilePath);
Optional<Path> repoRoot = GitHandler.findRepositoryRoot(bibFilePath);

if (repoRoot.isEmpty()) {
LOGGER.warn("Path is not inside a Git repository");
return;
}
GitHandler gitHandler = gitHandlerRegistry.get(repoRoot.get());

GitStatusSnapshot status = GitStatusChecker.checkStatus(gitHandler);

if (!status.tracking()) {
LOGGER.warn("Push aborted: file is not tracked by Git");
LOGGER.warn("Push aborted: File is not tracked by Git");
return;
}

Expand Down
Loading