diff --git a/docs/code-howtos/git.md b/docs/code-howtos/git.md index aa624c8cf85..135de1e4713 100644 --- a/docs/code-howtos/git.md +++ b/docs/code-howtos/git.md @@ -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 | diff --git a/jabgui/src/main/java/org/jabref/gui/git/GitPullAction.java b/jabgui/src/main/java/org/jabref/gui/git/GitPullAction.java index 5087d4d26c6..8cc48df17b8 100644 --- a/jabgui/src/main/java/org/jabref/gui/git/GitPullAction.java +++ b/jabgui/src/main/java/org/jabref/gui/git/GitPullAction.java @@ -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; @@ -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 @@ -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); diff --git a/jabgui/src/main/java/org/jabref/gui/git/GitStatusViewModel.java b/jabgui/src/main/java/org/jabref/gui/git/GitStatusViewModel.java index 89a97026b72..20486d19e2f 100644 --- a/jabgui/src/main/java/org/jabref/gui/git/GitStatusViewModel.java +++ b/jabgui/src/main/java/org/jabref/gui/git/GitStatusViewModel.java @@ -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()); diff --git a/jablib/src/main/java/module-info.java b/jablib/src/main/java/module-info.java index ef9f3d96406..5dc8b7d031c 100644 --- a/jablib/src/main/java/module-info.java +++ b/jablib/src/main/java/module-info.java @@ -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; diff --git a/jablib/src/main/java/org/jabref/logic/git/GitHandler.java b/jablib/src/main/java/org/jabref/logic/git/GitHandler.java index 6b6c87f1ca6..83914317c4d 100644 --- a/jablib/src/main/java/org/jabref/logic/git/GitHandler.java +++ b/jablib/src/main/java/org/jabref/logic/git/GitHandler.java @@ -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. + *

+ * 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 fromAnyPath(Path anyPathInsideRepo) { + public static Optional 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 fromAnyPath(Path anyPathInsideRepo) { + return findRepositoryRoot(anyPathInsideRepo).map(GitHandler::new); + } + public File getRepositoryPathAsFile() { return repositoryPathAsFile; } diff --git a/jablib/src/main/java/org/jabref/logic/git/GitSyncService.java b/jablib/src/main/java/org/jabref/logic/git/GitSyncService.java index fbef280fc3b..ad13d01b816 100644 --- a/jablib/src/main/java/org/jabref/logic/git/GitSyncService.java +++ b/jablib/src/main/java/org/jabref/logic/git/GitSyncService.java @@ -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; @@ -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 gitHandlerOpt = GitHandler.fromAnyPath(bibFilePath); - if (gitHandlerOpt.isEmpty()) { - LOGGER.warn("Pull aborted: The file is not inside a Git repository."); + Optional 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."); @@ -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 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; } diff --git a/jablib/src/main/java/org/jabref/logic/git/conflicts/SemanticConflictDetector.java b/jablib/src/main/java/org/jabref/logic/git/conflicts/SemanticConflictDetector.java index 1ffd468e11e..f501d0055e6 100644 --- a/jablib/src/main/java/org/jabref/logic/git/conflicts/SemanticConflictDetector.java +++ b/jablib/src/main/java/org/jabref/logic/git/conflicts/SemanticConflictDetector.java @@ -6,6 +6,7 @@ import java.util.Map; import java.util.Objects; import java.util.Optional; +import java.util.Set; import java.util.function.Function; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -17,6 +18,8 @@ import org.jabref.model.entry.BibEntry; import org.jabref.model.entry.field.Field; +import static com.google.common.collect.Sets.union; + /// Detects semantic merge conflicts between base, local, and remote. /// /// Strategy: @@ -26,53 +29,177 @@ /// Caveats: /// - Only entries with the same citation key are considered matching. /// - Entries without citation keys are currently ignored. -/// - TODO: Improve handling of such entries. -/// See: `BibDatabaseDiffTest#compareOfTwoEntriesWithSameContentAndMixedLineEndingsReportsNoDifferences` /// - Changing a citation key is not supported and is treated as deletion + addition. public class SemanticConflictDetector { public static List detectConflicts(BibDatabaseContext base, BibDatabaseContext local, BibDatabaseContext remote) { - // 1. get diffs between base and remote - List remoteDiffs = BibDatabaseDiff.compare(base, remote).getEntryDifferences(); + // 1. get diffs between base, local and remote + BibDatabaseDiff localDiff = BibDatabaseDiff.compare(base, local); + BibDatabaseDiff remoteDiff = BibDatabaseDiff.compare(base, remote); - // 2. map citation key to entry for local/remote diffs - Map baseEntries = getCitationKeyToEntryMap(base); - Map localEntries = getCitationKeyToEntryMap(local); + // 2. Union of all citationKeys that were changed (on either side) + Map baseMap = getCitationKeyToEntryMap(base); + Map localMap = getCitationKeyToEntryMap(local); + Map remoteMap = getCitationKeyToEntryMap(remote); + + // 3. Build a map from citationKey -> BibEntryDiff for both local and remote diffs + Map localDiffMap = indexByCitationKey(localDiff.getEntryDifferences()); + Map remoteDiffMap = indexByCitationKey(remoteDiff.getEntryDifferences()); + Set allKeys = union(localDiffMap.keySet(), remoteDiffMap.keySet()); List conflicts = new ArrayList<>(); - // 3. look for entries modified in both local and remote - for (BibEntryDiff remoteDiff : remoteDiffs) { - Optional keyOpt = remoteDiff.newEntry().getCitationKey(); - if (keyOpt.isEmpty()) { - continue; + // 4. Build full 3-way entry maps (key -> entry) from each database + for (String key : allKeys) { + BibEntry baseEntry = baseMap.get(key); + BibEntry localEntry = resolveEntry(key, localDiffMap.get(key), localMap); + BibEntry remoteEntry = resolveEntry(key, remoteDiffMap.get(key), remoteMap); + + // 5. If this triplet results in a conflict, collect it + detectEntryConflict(baseEntry, localEntry, remoteEntry).ifPresent(conflicts::add); + } + + return conflicts; + } + + /** + * Detect entry-level conflicts among base, local, and remote versions of an entry. + *

+ * + * @param base the entry in the common ancestor + * @param local the entry in the local version + * @param remote the entry in the remote version + * @return optional conflict (if detected) + */ + private static Optional detectEntryConflict(BibEntry base, + BibEntry local, + BibEntry remote) { + // Case 1: Both local and remote added same citation key -> compare their fields + if (base == null && local != null && remote != null) { + if (hasConflictingFields(new BibEntry(), local, remote)) { + return Optional.of(new ThreeWayEntryConflict(null, local, remote)); + } else { + return Optional.empty(); } + } - String citationKey = keyOpt.get(); - BibEntry baseEntry = baseEntries.get(citationKey); - BibEntry localEntry = localEntries.get(citationKey); - BibEntry remoteEntry = remoteDiff.newEntry(); + // Case 2: base exists, one side deleted, other modified -> conflict + if (base != null) { + boolean localDeleted = local == null; + boolean remoteDeleted = remote == null; - // Case 1: if the entry exists in all 3 versions - if (baseEntry != null && localEntry != null && remoteEntry != null) { - if (hasConflictingFields(baseEntry, localEntry, remoteEntry)) { - conflicts.add(new ThreeWayEntryConflict(baseEntry, localEntry, remoteEntry)); - } - // Case 2: base missing, but local + remote both added same citation key with different content - } else if (baseEntry == null && localEntry != null && remoteEntry != null) { - if (!Objects.equals(localEntry, remoteEntry)) { - conflicts.add(new ThreeWayEntryConflict(null, localEntry, remoteEntry)); - } - // Case 3: one side deleted, other side modified - } else if (baseEntry != null) { - if (localEntry != null && remoteEntry == null && !Objects.equals(baseEntry, localEntry)) { - conflicts.add(new ThreeWayEntryConflict(baseEntry, localEntry, null)); - } - if (localEntry == null && remoteEntry != null && !Objects.equals(baseEntry, remoteEntry)) { - conflicts.add(new ThreeWayEntryConflict(baseEntry, null, remoteEntry)); - } + boolean localChanged = !localDeleted && !base.getFieldMap().equals(local.getFieldMap()); + boolean remoteChanged = !remoteDeleted && !base.getFieldMap().equals(remote.getFieldMap()); + + if ((localChanged && remoteDeleted) || (remoteChanged && localDeleted)) { + return Optional.of(new ThreeWayEntryConflict(base, local, remote)); } } - return conflicts; + + // Case 3: base exists, both sides modified the entry -> check field-level diff + if (base != null && local != null && remote != null) { + boolean localChanged = !base.getFieldMap().equals(local.getFieldMap()); + boolean remoteChanged = !base.getFieldMap().equals(remote.getFieldMap()); + + if (localChanged && remoteChanged && hasConflictingFields(base, local, remote)) { + return Optional.of(new ThreeWayEntryConflict(base, local, remote)); + } + } + + return Optional.empty(); + } + + private static boolean hasConflictingFields(BibEntry base, BibEntry local, BibEntry remote) { + if (entryTypeChangedDifferently(base, local, remote)) { + return true; + } + + Set allFields = Stream.of(base, local, remote) + .flatMap(entry -> entry.getFields().stream()) + .collect(Collectors.toSet()); + + for (Field field : allFields) { + String baseVal = base.getField(field).orElse(null); + String localVal = local.getField(field).orElse(null); + String remoteVal = remote.getField(field).orElse(null); + + // Case 1: Both local and remote modified the same field from base, and the values differ + if (modifiedOnBothSidesWithDisagreement(baseVal, localVal, remoteVal)) { + return true; + } + + // Case 2: One side deleted the field, the other side modified it + if (oneSideDeletedOneSideModified(baseVal, localVal, remoteVal)) { + return true; + } + + // Case 3: Both sides added the field with different values + if (addedOnBothSidesWithDisagreement(baseVal, localVal, remoteVal)) { + return true; + } + } + + return false; + } + + private static boolean entryTypeChangedDifferently(BibEntry base, BibEntry local, BibEntry remote) { + if (base == null || local == null || remote == null) { + return false; + } + + boolean localChanged = !base.getType().equals(local.getType()); + boolean remoteChanged = !base.getType().equals(remote.getType()); + boolean changedToDifferentTypes = !local.getType().equals(remote.getType()); + + return localChanged && remoteChanged && changedToDifferentTypes; + } + + private static boolean modifiedOnBothSidesWithDisagreement(String baseVal, String localVal, String remoteVal) { + return notEqual(baseVal, localVal) && notEqual(baseVal, remoteVal) && notEqual(localVal, remoteVal); + } + + private static boolean oneSideDeletedOneSideModified(String baseVal, String localVal, String remoteVal) { + if (localVal == null && remoteVal == null) { + return false; + } + + return (baseVal != null) + && ((localVal == null && notEqual(baseVal, remoteVal)) + || (remoteVal == null && notEqual(baseVal, localVal))); + } + + private static boolean addedOnBothSidesWithDisagreement(String baseVal, String localVal, String remoteVal) { + return baseVal == null && localVal != null && remoteVal != null && notEqual(localVal, remoteVal); + } + + private static boolean notEqual(String a, String b) { + return !Objects.equals(a, b); + } + + /** + * Converts a List of BibEntryDiff into a Map where the key is the citation key, + * and the value is the corresponding BibEntryDiff. + *

+ * Notes: + * - Only entries with a citation key are included (entries without a key cannot be uniquely identified during merge). + * - Entries that represent additions (base == null) or deletions (new == null) are also included. + * - If multiple BibEntryDiffs share the same citation key (rare), the latter one will overwrite the former. + *

+ * + * @param entryDiffs A list of entry diffs produced by BibDatabaseDiff + * @return A map from citation key to corresponding BibEntryDiff + */ + private static Map indexByCitationKey(List entryDiffs) { + Map result = new LinkedHashMap<>(); + + for (BibEntryDiff diff : entryDiffs) { + Optional citationKey = Optional.ofNullable(diff.newEntry()) + .flatMap(BibEntry::getCitationKey) + .or(() -> Optional.ofNullable(diff.originalEntry()) + .flatMap(BibEntry::getCitationKey)); + citationKey.ifPresent(key -> result.put(key, diff)); + } + + return result; } private static Map getCitationKeyToEntryMap(BibDatabaseContext context) { @@ -86,20 +213,11 @@ private static Map getCitationKeyToEntryMap(BibDatabaseContext )); } - private static boolean hasConflictingFields(BibEntry base, BibEntry local, BibEntry remote) { - return Stream.of(base, local, remote) - .flatMap(entry -> entry.getFields().stream()) - .distinct() - .anyMatch(field -> { - String baseVal = base.getField(field).orElse(null); - String localVal = local.getField(field).orElse(null); - String remoteVal = remote.getField(field).orElse(null); - - boolean localChanged = !Objects.equals(baseVal, localVal); - boolean remoteChanged = !Objects.equals(baseVal, remoteVal); - - return localChanged && remoteChanged && !Objects.equals(localVal, remoteVal); - }); + private static BibEntry resolveEntry(String key, BibEntryDiff diff, Map fullMap) { + if (diff == null) { + return fullMap.get(key); + } + return diff.newEntry(); } /** diff --git a/jablib/src/main/java/org/jabref/logic/git/status/GitStatusChecker.java b/jablib/src/main/java/org/jabref/logic/git/status/GitStatusChecker.java index 97e1c118a19..d19419814c4 100644 --- a/jablib/src/main/java/org/jabref/logic/git/status/GitStatusChecker.java +++ b/jablib/src/main/java/org/jabref/logic/git/status/GitStatusChecker.java @@ -26,21 +26,8 @@ public class GitStatusChecker { private static final Logger LOGGER = LoggerFactory.getLogger(GitStatusChecker.class); - public static GitStatusSnapshot checkStatus(Path anyPathInsideRepo) { - Optional gitHandlerOpt = GitHandler.fromAnyPath(anyPathInsideRepo); - - if (gitHandlerOpt.isEmpty()) { - return new GitStatusSnapshot( - !GitStatusSnapshot.TRACKING, - SyncStatus.UNTRACKED, - !GitStatusSnapshot.CONFLICT, - !GitStatusSnapshot.UNCOMMITTED, - Optional.empty() - ); - } - GitHandler handler = gitHandlerOpt.get(); - - try (Git git = Git.open(handler.getRepositoryPathAsFile())) { + public static GitStatusSnapshot checkStatus(GitHandler gitHandler) { + try (Git git = Git.open(gitHandler.getRepositoryPathAsFile())) { Repository repo = git.getRepository(); Status status = git.status().call(); boolean hasConflict = !status.getConflicting().isEmpty(); @@ -71,6 +58,21 @@ public static GitStatusSnapshot checkStatus(Path anyPathInsideRepo) { } } + public static GitStatusSnapshot checkStatus(Path anyPathInsideRepo) { + Optional handlerOpt = GitHandler.fromAnyPath(anyPathInsideRepo); + if (handlerOpt.isEmpty()) { + return new GitStatusSnapshot( + !GitStatusSnapshot.TRACKING, + SyncStatus.UNTRACKED, + !GitStatusSnapshot.CONFLICT, + !GitStatusSnapshot.UNCOMMITTED, + Optional.empty() + ); + } + + return checkStatus(handlerOpt.get()); + } + private static SyncStatus determineSyncStatus(Repository repo, ObjectId localHead, ObjectId remoteHead) throws IOException { if (localHead == null || remoteHead == null) { LOGGER.debug("localHead or remoteHead null"); diff --git a/jablib/src/main/java/org/jabref/logic/git/util/GitHandlerRegistry.java b/jablib/src/main/java/org/jabref/logic/git/util/GitHandlerRegistry.java new file mode 100644 index 00000000000..4936213f5b9 --- /dev/null +++ b/jablib/src/main/java/org/jabref/logic/git/util/GitHandlerRegistry.java @@ -0,0 +1,34 @@ +package org.jabref.logic.git.util; + +import java.nio.file.Path; +import java.util.Map; +import java.util.Objects; +import java.util.Optional; +import java.util.concurrent.ConcurrentHashMap; + +import org.jabref.logic.git.GitHandler; + +/** + * A registry that manages {@link GitHandler} instances per Git repository. + *

+ * Ensures that each Git repository (identified by its root path) has a single {@code GitHandler} instance shared across the application. + *

+ * Usage: + * - {@link #get(Path)} — for known repository root paths (must contain a .git folder). + * - {@link #fromAnyPath(Path)} — for arbitrary paths inside a Git repo; will locate the repo root first. + */ +public class GitHandlerRegistry { + + private final Map handlerCache = new ConcurrentHashMap<>(); + + public GitHandler get(Path repoPath) { + Path normalized = Objects.requireNonNull(repoPath, "Path must not be null") + .toAbsolutePath().normalize(); + return handlerCache.computeIfAbsent(normalized, GitHandler::new); + } + + public Optional fromAnyPath(Path anyPathInsideRepo) { + return GitHandler.findRepositoryRoot(Objects.requireNonNull(anyPathInsideRepo, "Path must not be null")) + .map(this::get); + } +} diff --git a/jablib/src/test/java/org/jabref/logic/git/GitSyncServiceTest.java b/jablib/src/test/java/org/jabref/logic/git/GitSyncServiceTest.java index 729c5af1c26..5cc0296518a 100644 --- a/jablib/src/test/java/org/jabref/logic/git/GitSyncServiceTest.java +++ b/jablib/src/test/java/org/jabref/logic/git/GitSyncServiceTest.java @@ -13,6 +13,7 @@ import org.jabref.logic.git.merge.GitSemanticMergeExecutor; import org.jabref.logic.git.merge.GitSemanticMergeExecutorImpl; import org.jabref.logic.git.model.MergeResult; +import org.jabref.logic.git.util.GitHandlerRegistry; import org.jabref.logic.importer.ImportFormatPreferences; import org.jabref.logic.importer.ParserResult; import org.jabref.logic.importer.fileformat.BibtexParser; @@ -60,6 +61,7 @@ class GitSyncServiceTest { private GitConflictResolverStrategy gitConflictResolverStrategy; private GitSemanticMergeExecutor mergeExecutor; private BibDatabaseContext context; + private GitHandlerRegistry gitHandlerRegistry; // These are setup by aliceBobSetting private RevCommit baseCommit; @@ -123,6 +125,7 @@ void aliceBobSimple(@TempDir Path tempDir) throws Exception { gitConflictResolverStrategy = mock(GitConflictResolverStrategy.class); mergeExecutor = new GitSemanticMergeExecutorImpl(importFormatPreferences); + gitHandlerRegistry = new GitHandlerRegistry(); // create fake remote repo remoteDir = tempDir.resolve("remote.git"); @@ -205,8 +208,7 @@ void cleanup() { @Test void pullTriggersSemanticMergeWhenNoConflicts() throws Exception { - GitHandler gitHandler = new GitHandler(library.getParent()); - GitSyncService syncService = new GitSyncService(importFormatPreferences, gitHandler, gitConflictResolverStrategy, mergeExecutor); + GitSyncService syncService = new GitSyncService(importFormatPreferences, gitHandlerRegistry, gitConflictResolverStrategy, mergeExecutor); MergeResult result = syncService.fetchAndMerge(context, library); assertTrue(result.isSuccessful()); @@ -229,8 +231,7 @@ void pullTriggersSemanticMergeWhenNoConflicts() throws Exception { @Test void pushTriggersMergeAndPushWhenNoConflicts() throws Exception { - GitHandler gitHandler = new GitHandler(library.getParent()); - GitSyncService syncService = new GitSyncService(importFormatPreferences, gitHandler, gitConflictResolverStrategy, mergeExecutor); + GitSyncService syncService = new GitSyncService(importFormatPreferences, gitHandlerRegistry, gitConflictResolverStrategy, mergeExecutor); syncService.push(context, library); String pushedContent = GitFileReader @@ -308,8 +309,7 @@ void mergeConflictOnSameFieldTriggersDialogAndUsesUserResolution() throws Except return List.of(resolved); }); - GitHandler handler = new GitHandler(aliceDir); - GitSyncService service = new GitSyncService(importFormatPreferences, handler, resolver, mergeExecutor); + GitSyncService service = new GitSyncService(importFormatPreferences, gitHandlerRegistry, resolver, mergeExecutor); MergeResult result = service.fetchAndMerge(context, library); assertTrue(result.isSuccessful()); diff --git a/jablib/src/test/java/org/jabref/logic/git/status/GitStatusCheckerTest.java b/jablib/src/test/java/org/jabref/logic/git/status/GitStatusCheckerTest.java index bacc6d56579..5160280f59f 100644 --- a/jablib/src/test/java/org/jabref/logic/git/status/GitStatusCheckerTest.java +++ b/jablib/src/test/java/org/jabref/logic/git/status/GitStatusCheckerTest.java @@ -5,6 +5,9 @@ import java.nio.file.Path; import java.util.List; +import org.jabref.logic.git.GitHandler; +import org.jabref.logic.git.util.GitHandlerRegistry; + import org.eclipse.jgit.api.CreateBranchCommand; import org.eclipse.jgit.api.Git; import org.eclipse.jgit.internal.storage.file.WindowCache; @@ -28,6 +31,7 @@ class GitStatusCheckerTest { private Git localGit; private Git remoteGit; private Git seedGit; + private GitHandlerRegistry gitHandlerRegistry; private final PersonIdent author = new PersonIdent("Tester", "tester@example.org"); @@ -69,6 +73,7 @@ class GitStatusCheckerTest { @BeforeEach void setup(@TempDir Path tempDir) throws Exception { + gitHandlerRegistry = new GitHandlerRegistry(); Path remoteDir = tempDir.resolve("remote.git"); remoteGit = Git.init().setBare(true).setDirectory(remoteDir.toFile()).call(); @@ -132,13 +137,16 @@ void tearDown() { void untrackedStatusWhenNotGitRepo(@TempDir Path tempDir) { Path nonRepoPath = tempDir.resolve("somefile.bib"); GitStatusSnapshot snapshot = GitStatusChecker.checkStatus(nonRepoPath); + assertFalse(snapshot.tracking()); assertEquals(SyncStatus.UNTRACKED, snapshot.syncStatus()); } @Test void upToDateStatusAfterInitialSync() { - GitStatusSnapshot snapshot = GitStatusChecker.checkStatus(localLibrary); + GitHandler gitHandler = gitHandlerRegistry.get(localLibrary.getParent()); + GitStatusSnapshot snapshot = GitStatusChecker.checkStatus(gitHandler); + assertTrue(snapshot.tracking()); assertEquals(SyncStatus.UP_TO_DATE, snapshot.syncStatus()); } @@ -159,14 +167,17 @@ void behindStatusWhenRemoteHasNewCommit(@TempDir Path tempDir) throws Exception .call(); } localGit.fetch().setRemote("origin").call(); - GitStatusSnapshot snapshot = GitStatusChecker.checkStatus(localLibrary); + GitHandler gitHandler = gitHandlerRegistry.get(localLibrary.getParent()); + GitStatusSnapshot snapshot = GitStatusChecker.checkStatus(gitHandler); + assertEquals(SyncStatus.BEHIND, snapshot.syncStatus()); } @Test void aheadStatusWhenLocalHasNewCommit() throws Exception { commitFile(localGit, localUpdatedContent, "Local update"); - GitStatusSnapshot snapshot = GitStatusChecker.checkStatus(localLibrary); + GitHandler gitHandler = gitHandlerRegistry.get(localLibrary.getParent()); + GitStatusSnapshot snapshot = GitStatusChecker.checkStatus(gitHandler); assertEquals(SyncStatus.AHEAD, snapshot.syncStatus()); } @@ -188,7 +199,8 @@ void divergedStatusWhenBothSidesHaveCommits(@TempDir Path tempDir) throws Except .call(); } localGit.fetch().setRemote("origin").call(); - GitStatusSnapshot snapshot = GitStatusChecker.checkStatus(localLibrary); + GitHandler gitHandler = gitHandlerRegistry.get(localLibrary.getParent()); + GitStatusSnapshot snapshot = GitStatusChecker.checkStatus(gitHandler); assertEquals(SyncStatus.DIVERGED, snapshot.syncStatus()); } diff --git a/jablib/src/test/java/org/jabref/logic/git/util/GitHandlerRegistryTest.java b/jablib/src/test/java/org/jabref/logic/git/util/GitHandlerRegistryTest.java new file mode 100644 index 00000000000..d7d8eb254a6 --- /dev/null +++ b/jablib/src/test/java/org/jabref/logic/git/util/GitHandlerRegistryTest.java @@ -0,0 +1,74 @@ +package org.jabref.logic.git.util; + +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.Optional; + +import org.jabref.logic.git.GitHandler; + +import org.eclipse.jgit.api.Git; +import org.eclipse.jgit.internal.storage.file.WindowCache; +import org.eclipse.jgit.lib.RepositoryCache; +import org.eclipse.jgit.storage.file.WindowCacheConfig; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertTrue; + +class GitHandlerRegistryTest { + @TempDir + Path tempDir; + + private GitHandlerRegistry registry; + private Git testGit; + + @BeforeEach + void setUp() { + registry = new GitHandlerRegistry(); + } + + @AfterEach + void tearDown() { + if (testGit != null) { + testGit.close(); + } + + RepositoryCache.clear(); + WindowCache.reconfigure(new WindowCacheConfig()); + } + + @Test + void returnsSameHandlerForSameRepoPath() throws Exception { + testGit = Git.init().setDirectory(tempDir.toFile()).call(); + Path repoPath = tempDir.toAbsolutePath().normalize(); + + GitHandler handler1 = registry.get(repoPath); + GitHandler handler2 = registry.get(repoPath); + + assertSame(handler1, handler2); + } + + @Test + void returnsEmptyForNonGitPath() { + Path nonGitPath = tempDir.resolve("non-git-folder"); + assertFalse(registry.fromAnyPath(nonGitPath).isPresent()); + } + + @Test + void resolvesHandlerFromNestedPath() throws Exception { + testGit = Git.init().setDirectory(tempDir.toFile()).call(); + + Path subDir = Files.createDirectory(tempDir.resolve("nested")); + Optional handlerOpt = registry.fromAnyPath(subDir); + + assertTrue(handlerOpt.isPresent(), "Should resolve handler from subdirectory inside repo"); + + GitHandler handler1 = registry.get(tempDir); + GitHandler handler2 = handlerOpt.get(); + assertSame(handler1, handler2, "Should return same cached handler"); + } +} diff --git a/jablib/src/test/java/org/jabref/logic/git/util/SemanticConflictDetectorTest.java b/jablib/src/test/java/org/jabref/logic/git/util/SemanticConflictDetectorTest.java index 31da078d777..ef42db0c191 100644 --- a/jablib/src/test/java/org/jabref/logic/git/util/SemanticConflictDetectorTest.java +++ b/jablib/src/test/java/org/jabref/logic/git/util/SemanticConflictDetectorTest.java @@ -99,369 +99,759 @@ private RevCommit writeAndCommit(String content, String message, PersonIdent aut // See docs/code-howtos/git.md for testing patterns static Stream semanticConflicts() { return Stream.of( - Arguments.of("T1 - remote changed a field, local unchanged", + Arguments.of("E01 - entry a does not exist anywhere", + "", + "", + "", + false), + + Arguments.of("E02 - entry a added remotely only", + "", + "", """ - @article{a, - author = {Test Author}, - doi = {xya}, - } + @article{a, + author = {remote}, + title = {A}, + } + """, + false), + + Arguments.of("E03 - entry a added locally only", + "", + """ + @article{a, + author = {local}, + title = {A}, + } + """, + "", + false), + + Arguments.of("E04 - entry a added on both sides with different content", + "", + """ + @article{a, + author = {local}, + title = {A}, + } + """, + """ + @article{a, + author = {remote}, + title = {A}, + } """, + true), + + Arguments.of("E04a - both sides added entry a with identical content", + "", """ @article{a, - author = {Test Author}, - doi = {xya}, + author = {same}, + title = {A}, } """, """ @article{a, - author = {bob}, - doi = {xya}, + author = {same}, + title = {A}, } """, false ), - Arguments.of("T2 - local changed a field, remote unchanged", + + Arguments.of("E04b - both added entry a but changed different fields", + "", """ @article{a, - author = {Test Author}, - doi = {xya}, + author = {local}, } """, """ @article{a, - author = {alice}, - doi = {xya}, + journal = {Remote Journal}, } """, + false + ), + + Arguments.of("E04c - both added entry a with conflicting field values", + "", """ @article{a, - author = {Test Author}, - doi = {xya}, + author = {local}, + title = {A}, } """, - false + """ + @article{a, + author = {remote}, + title = {A}, + } + """, + true ), - Arguments.of("T3 - both changed to same value", + + Arguments.of("E05 - entry a was deleted by both", """ @article{a, - author = {Test Author}, - doi = {xya}, + author = {base}, + title = {A}, } """, + "", + "", + false), + + Arguments.of("E06 - local deleted entry a, remote kept it unchanged", """ @article{a, - author = {bob}, - doi = {xya}, + author = {base}, + title = {A}, } """, + "", """ @article{a, - author = {bob}, - doi = {xya}, + author = {base}, + title = {A}, } """, - false - ), - Arguments.of("T4 - both changed to different values", + false), + + Arguments.of("E07 - local deleted entry a, remote modified it", """ @article{a, - author = {Test Author}, - doi = {xya}, + author = {base}, + title = {A}, } """, + "", """ @article{a, - author = {alice}, - doi = {xya}, + author = {remote}, + title = {A}, } """, + true), + + Arguments.of("E08 - remote deleted entry a, local kept it unchanged", """ @article{a, - author = {bob}, - doi = {xya}, + author = {base}, + title = {A}, } """, - true - ), - Arguments.of("T5 - local deleted field, remote changed it", """ @article{a, - author = {Test Author}, - doi = {xya}, + author = {base}, + title = {A}, } """, + "", + false), + + Arguments.of("E09 - entry a unchanged in all three", + """ + @article{a, + author = {base}, + title = {A}, + }""", """ @article{a, + author = {base}, + title = {A}, } """, """ @article{a, - author = {bob}, - doi = {xya}, + author = {base}, + title = {A}, } """, - true + false), + + Arguments.of("E10a - remote modified a different field, local unchanged", + """ + @article{a, + author = {base}, + title = {A}, + } + """, + """ + @article{a, + author = {base}, + title = {A}, + } + """, + """ + @article{a, + author = {base}, + year = {2025}, + } + """, + false ), - Arguments.of("T6 - local changed, remote deleted", + + Arguments.of("E10b - remote modified the same field, local unchanged", """ - @article{a, - author = {Test Author}, - doi = {xya}, - } + @article{a, + author = {base}, + title = {A}, + } + """, + """ + @article{a, + author = {base}, + title = {A}, + } + """, + """ + @article{a, + author = {remote}, + title = {A}, + } """, + false + ), + + Arguments.of("E11 - remote deleted entry a, local modified it", """ @article{a, - author = {alice}, - doi = {xya}, + author = {base}, + title = {A}, } """, """ @article{a, + author = {local}, + title = {A}, } """, - true - ), - Arguments.of("T7 - remote deleted, local unchanged", + "", + true), + + Arguments.of("E12 - local modified entry a, remote unchanged", """ @article{a, - author = {Test Author}, - doi = {xya}, + author = {base}, + title = {A}, } """, """ @article{a, - author = {Test Author}, - doi = {xya}, + author = {local}, + title = {A}, } """, """ @article{a, + author = {base}, + title = {A}, } """, - false - ), - Arguments.of("T8 - local changed field A, remote changed field B", + false), + + Arguments.of("E13a - both modified entry a but changed different fields", """ @article{a, - author = {Test Author}, - doi = {xya}, + author = {base}, + title = {A}, } """, """ @article{a, - author = {alice}, - doi = {xya}, + author = {local}, + title = {A}, } """, """ @article{a, - author = {Test Author}, - doi = {xyz}, + author = {base}, + title = {B}, } """, false ), - Arguments.of("T9 - field order changed only", + + Arguments.of("E13b - both changed same field to same value", """ @article{a, - author = {Test Author}, - doi = {xya}, + author = {base}, } """, """ @article{a, - doi = {xya}, - author = {Test Author}, + author = {common}, } """, """ @article{a, - author = {Test Author}, - doi = {xya}, + author = {common}, } """, false ), - Arguments.of("T10 - local changed entry a, remote changed entry b", + + Arguments.of("E13c - both changed same field differently", """ @article{a, - author = {Test Author}, - doi = {xya}, - } - - @article{b, - author = {Test Author}, - doi = {xyz}, + author = {base}, } """, """ @article{a, - author = {author-a}, - doi = {xya}, + author = {local}, } - @article{b, - author = {Test Author}, - doi = {xyz}, + """, + """ + @article{a, + author = {remote}, + } + """, + true + ), + Arguments.of("E14a - citationKey changed in local", + """ + @article{a, + author = {base}, } """, """ @article{b, - author = {author-b}, - doi = {xyz}, + author = {base}, } - + """, + """ @article{a, - author = {Test Author}, - doi = {xya}, + author = {base}, } """, false ), - Arguments.of("T11 - remote added field, local unchanged", + Arguments.of("E14b - citationKey changed in remote", """ @article{a, - author = {Test Author}, - doi = {xya}, + author = {base}, } """, """ @article{a, - author = {Test Author}, - doi = {xya}, + author = {base}, } """, """ - @article{a, - author = {Test Author}, - doi = {xya}, - year = {2025}, + @article{b, + author = {base}, } """, false ), - Arguments.of("T12 - both added same field with different values", + Arguments.of("E14c - citationKey renamed differently in local and remote", """ @article{a, - author = {Test Author}, - doi = {xya}, + author = {base}, + } + """, + """ + @article{b, + author = {base}, + } + """, + """ + @article{c, + author = {base}, } """, + false + ), + Arguments.of("E15 - both added same citationKey with different content", + """ + """, """ @article{a, - author = {Test Author}, - doi = {xya}, - year = {2023}, + title = {local}, } """, """ @article{a, - author = {Test Author}, - doi = {xya}, - year = {2025}, + title = {remote}, } """, true ), - Arguments.of("T13 - local added field, remote unchanged", + Arguments.of("F01 - identical field value on all sides", + """ + @article{a, + author = {same}, + } + """, + """ + @article{a, + author = {same}, + } + """, + """ + @article{a, + author = {same}, + } + """, + false + ), + Arguments.of("F02 - remote changed, local same as base", + """ + @article{a, + author = {base}, + } + """, + """ + @article{a, + author = {base}, + } + """, + """ + @article{a, + author = {remote}, + } + """, + false + ), + Arguments.of("F03 - local changed, remote same as base", + """ + @article{a, + author = {base}, + } + """, + """ + @article{a, + author = {local}, + } + """, + """ + @article{a, + author = {base}, + } + """, + false + ), + Arguments.of("F04 - both changed to same value", + """ + @article{a, + author = {base}, + } + """, + """ + @article{a, + author = {common}, + } + """, + """ + @article{a, + author = {common}, + } + """, + false + ), + Arguments.of("F05 - both changed same field differently", + """ + @article{a, + author = {base}, + } + """, + """ + @article{a, + author = {local}, + } + """, + """ + @article{a, + author = {remote}, + } + """, + true + ), + Arguments.of("F06 - Local deleted, remote unchanged", """ @article{a, - author = {Test Author}, - doi = {xya}, - } + author = {base} + } + """, + """ + @article{a, + } """, """ @article{a, - author = {Test Author}, - doi = {newfield}, - } + author = {base} + } """, + false + ), + Arguments.of("F07 - Remote deleted, local unchanged", """ @article{a, - author = {Test Author}, - doi = {xya}, - } + author = {base} + } + """, + """ + @article{a, + author = {base} + } + """, + """ + @article{a, + } """, false ), - Arguments.of("T14 - both added same field with same value", + Arguments.of("F08 - Both deleted", + """ + @article{a, + author = {base} + } + """, + """ + @article{a, + } + """, + """ + @article{a, + } + """, + false + ), + Arguments.of("F09 - Local changed, remote deleted", + """ + @article{a, + author = {base} + } + """, + """ + @article{a, + author = {local} + } + """, + """ + @article{a, + }""", + true + ), + + Arguments.of("F10 - Local deleted, remote changed", + """ + @article{a, + author = {base} + } + """, + """ + @article{a, + } + """, + """ + @article{a, + author = {remote} + } + """, + true + ), + + Arguments.of("F11 - All missing", """ @article{a, - author = {Test Author}, - doi = {xya}, } """, """ @article{a, - author = {Test Author}, - doi = {value}, } """, """ @article{a, - author = {Test Author}, - doi = {value}, } """, false ), - Arguments.of("T15 - both added same field with different values", + + Arguments.of("F12 - Local added", + """ + @article{a, + } + """, """ @article{a, - author = {Test Author}, - doi = {xya}, + author = {local} + } + """, + """ + @article{a, + } + """, + false + ), + + Arguments.of("F13 - Remote added", + """ + @article{a, + } + """, + """ + @article{a, + } + """, + """ + @article{a, + author = {remote} + } + """, + false + ), + + Arguments.of("F14 - Both added same value", + """ + @article{a, + } + """, + """ + @article{a, + author = {same} + } + """, + """ + @article{a, + author = {same} + } + """, + false + ), + + Arguments.of("F15 - Both added different values", + """ + @article{a, + } + """, + """ + @article{a, + author = {local} + } + """, + """ + @article{a, + author = {remote} + } + """, + true + ), + + Arguments.of("F16 - Local modified, remote deleted (conflict)", + """ + @article{a, + author = {base} + } + """, + """ + @article{a, + author = {local} + } + """, + """ + @article{a, + }""", + true + ), + + Arguments.of("F17 - Local deleted and remote modified (conflict)", + """ + @article{a, + author = {base} } """, """ @article{a, - author = {Test Author}, - doi = {value1}, } """, """ @article{a, - author = {Test Author}, - doi = {value2}, + author = {remote} } """, true ), - Arguments.of("T16 - both sides added entry a with different values", - "", + + Arguments.of("F18 - No base, both added different", + """ + @article{a, + }""", """ @article{a, - author = {alice}, - doi = {xya}, + author = {local} } """, """ @article{a, - author = {bob}, - doi = {xya}, + author = {remote} } """, true ), - Arguments.of("T17 - both added same content", - "", + Arguments.of("F19 - field order changed, same content", """ @article{a, - author = {same}, - doi = {123}, + title = {Hello}, + author = {Alice}, } """, """ @article{a, - author = {same}, - doi = {123}, + author = {Alice}, + title = {Hello}, + } + """, + """ + @article{a, + title = {Hello}, + author = {Alice}, + } + """, + false + ), + Arguments.of("F20 - entryType changed in local", + """ + @article{a, + author = {base}, + } + """, + """ + @book{a, + author = {base}, + } + """, + """ + @article{a, + author = {base}, } """, false + ), + Arguments.of("F21 - entryType changed differently on both sides", + """ + + """, + """ + @book{a, + author = {base}, + } + """, + """ + @inproceedings{a, + author = {base}, + } + """, + true ) ); } @Test - void extractMergePlanT10OnlyRemoteChangedEntryB() throws Exception { + void extractMergePlanOnlyRemoteChangedEntryB() throws Exception { String base = """ @article{a, author = {Test Author}, @@ -498,7 +888,7 @@ void extractMergePlanT10OnlyRemoteChangedEntryB() throws Exception { } @Test - void extractMergePlanT11RemoteAddsField() throws Exception { + void extractMergePlanRemoteAddsField() throws Exception { String base = """ @article{a, author = {Test Author}, @@ -524,4 +914,39 @@ void extractMergePlanT11RemoteAddsField() throws Exception { Map patch = plan.fieldPatches().get("a"); assertEquals("2025", patch.get(StandardField.YEAR)); } + + @Test + void noConflictWhenOnlyLineEndingsDiffer() throws Exception { + String base = """ + @article{a, + comment = {line1\n\nline3\n\nline5}, + } + """; + String local = """ + @article{a, + comment = {line1\r\n\r\nline3\r\n\r\nline5}, + } + """; + String remote = """ + @article{a, + comment = {line1\n\nline3\n\nline5}, + } + """; + + RevCommit baseCommit = writeAndCommit(base, "base", alice); + RevCommit localCommit = writeAndCommit(local, "local", alice); + RevCommit remoteCommit = writeAndCommit(remote, "remote", bob); + + BibDatabaseContext baseDatabaseContext = parse(baseCommit); + BibDatabaseContext localDatabaseContext = parse(localCommit); + BibDatabaseContext remoteDatabaseContext = parse(remoteCommit); + + List diffs = SemanticConflictDetector.detectConflicts( + baseDatabaseContext, + localDatabaseContext, + remoteDatabaseContext + ); + + assertTrue(diffs.isEmpty(), "Line ending differences should not be considered conflicts"); + } }