Skip to content
This repository has been archived by the owner on Sep 15, 2023. It is now read-only.

Commit

Permalink
feat: after a merge that contained conflicting files, the commit wind…
Browse files Browse the repository at this point in the history
…ow is shown and contains a preformed commit message detailing the conflicting files
  • Loading branch information
L1nc0ln committed Aug 9, 2023
1 parent c3df3c9 commit 2a3622f
Show file tree
Hide file tree
Showing 4 changed files with 249 additions and 30 deletions.
10 changes: 9 additions & 1 deletion gui/src/main/java/de/adito/git/gui/actions/CommitAction.java
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ public void actionPerformed(ActionEvent pEvent)
{
saveUtil.saveUnsavedFiles();
Optional<IRepository> currentRepoOpt = repository.blockingFirst();
String prefStoreInstanceKey = COMMIT_MESSAGE_BASE_STORAGE_KEY + currentRepoOpt.map(pRepo -> pRepo.getTopLevelDirectory().getAbsolutePath()).orElse("");
String prefStoreInstanceKey = getCommitMessageStorageKey(repository);
Observable<Optional<IRepository>> repo = Observable.just(currentRepoOpt);
if (messageTemplate == null || messageTemplate.isEmpty())
{
Expand Down Expand Up @@ -101,6 +101,14 @@ public void actionPerformed(ActionEvent pEvent)
}
}

static String getCommitMessageStorageKey(@NonNull Observable<Optional<IRepository>> pRepository)
{
return COMMIT_MESSAGE_BASE_STORAGE_KEY + pRepository.blockingFirst(Optional.empty())
.map(IRepository::getTopLevelDirectory)
.map(File::getAbsolutePath)
.orElse("");
}

/**
* perform a commit based on the dialogResult
*
Expand Down
166 changes: 138 additions & 28 deletions gui/src/main/java/de/adito/git/gui/actions/MergeAction.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package de.adito.git.gui.actions;

import com.google.common.annotations.VisibleForTesting;
import com.google.inject.Inject;
import com.google.inject.assistedinject.Assisted;
import de.adito.git.api.INotifyUtil;
Expand All @@ -21,11 +22,15 @@
import de.adito.git.impl.Util;
import de.adito.git.impl.data.MergeDetailsImpl;
import io.reactivex.rxjava3.core.Observable;
import lombok.AllArgsConstructor;
import lombok.Getter;
import lombok.NonNull;

import java.awt.event.ActionEvent;
import java.text.MessageFormat;
import java.util.List;
import java.util.Optional;
import java.util.stream.Collectors;

/**
* @author m.kaspera 24.10.2018
Expand All @@ -36,6 +41,7 @@ class MergeAction extends AbstractTableAction
private static final String STASH_ID_KEY = "merge::stashCommitId";
private final ISaveUtil saveUtil;
private final INotifyUtil notifyUtil;
private final IActionProvider actionProvider;
private final MergeConflictSequence mergeConflictSequence;
final Observable<Optional<IRepository>> repositoryObservable;
private final IPrefStore prefStore;
Expand All @@ -45,14 +51,16 @@ class MergeAction extends AbstractTableAction

@Inject
MergeAction(IPrefStore pPrefStore, IAsyncProgressFacade pProgressFacade, IDialogProvider pDialogProvider, ISaveUtil pSaveUtil, INotifyUtil pNotifyUtil,
MergeConflictSequence pMergeConflictSequence, @Assisted Observable<Optional<IRepository>> pRepoObs, @Assisted Observable<Optional<IBranch>> pTargetBranch)
IActionProvider pActionProvider, MergeConflictSequence pMergeConflictSequence, @Assisted Observable<Optional<IRepository>> pRepoObs,
@Assisted Observable<Optional<IBranch>> pTargetBranch)
{
super("Merge into Current", _getIsEnabledObservable(pTargetBranch));
prefStore = pPrefStore;
progressFacade = pProgressFacade;
dialogProvider = pDialogProvider;
saveUtil = pSaveUtil;
notifyUtil = pNotifyUtil;
actionProvider = pActionProvider;
mergeConflictSequence = pMergeConflictSequence;
repositoryObservable = pRepoObs;
targetBranch = pTargetBranch;
Expand All @@ -76,10 +84,19 @@ public void actionPerformed(ActionEvent pEvent)
});
}

private void _doMerge(IProgressHandle pProgressHandle, IRepository pRepository, IBranch pSelectedBranch) throws AditoGitException
/**
* perform the actual merge
*
* @param pProgressHandle ProgressHandle that allows informing the user about the current work
* @param pRepository Repository of the project that the merge is done in
* @param pSelectedBranch The branch that is merged into the current branch
* @throws AditoGitException if the stash/unstash encounters a problem, an error is encountered during the merge or the result cannot be commited
*/
private void _doMerge(@NonNull IProgressHandle pProgressHandle, @NonNull IRepository pRepository, @NonNull IBranch pSelectedBranch) throws AditoGitException
{
saveUtil.saveUnsavedFiles();
boolean unstashChanges = true;
AfterMergeConflictsSteps afterMergeConflictsSteps = AfterMergeConflictsSteps.COMMIT_AND_UNSTASH;
try
{
if (pRepository.getStatus().blockingFirst().map(IFileStatus::hasUncommittedChanges).orElse(false)
Expand All @@ -90,35 +107,17 @@ private void _doMerge(IProgressHandle pProgressHandle, IRepository pRepository,
IBranch currentBranch = pRepository.getRepositoryState().blockingFirst().orElseThrow().getCurrentBranch();
pProgressHandle.setDescription("Merging branches");
List<IMergeData> mergeConflictDiffs = pRepository.merge(currentBranch, pSelectedBranch);
String suggestedCommitMessage = createSuggestedCommitMessage(pRepository, pSelectedBranch, mergeConflictDiffs);
if (!mergeConflictDiffs.isEmpty())
{
IMergeDetails mergeDetails = new MergeDetailsImpl(mergeConflictDiffs, currentBranch.getSimpleName(), pSelectedBranch.getSimpleName());
IMergeConflictDialogResult<?, ?> dialogResult = mergeConflictSequence.performMergeConflictSequence(Observable.just(Optional.of(pRepository)),
mergeDetails, true);
IUserPromptDialogResult<?, ?> promptDialogResult = null;
if (!(dialogResult.isAbortMerge() || dialogResult.isFinishMerge()))
{
promptDialogResult = dialogProvider.showMessageDialog(null, Util.getResource(this.getClass(), "mergeSaveStateQuestion"),
List.of(EButtons.SAVE, EButtons.ABORT),
List.of(EButtons.SAVE));
if (promptDialogResult.isOkay())
{
unstashChanges = false;
notifyUtil.notify("Saved merge state", Util.getResource(this.getClass(), "mergeSavedStateMsg"), false);
return;
}
}
if (!dialogResult.isFinishMerge() || (promptDialogResult != null && !promptDialogResult.isOkay()))
{
pProgressHandle.setDescription("Aborting merge");
pRepository.reset(currentBranch.getId(), EResetType.HARD);
// do not execute the "show commit dialog" part after this, the finally block should still be executed even if we return here
return;
}
afterMergeConflictsSteps = dealWithMergeConflicts(pProgressHandle, pRepository, currentBranch, mergeDetails);
}
pRepository.commit("merged " + pSelectedBranch.getSimpleName() + " into " + pRepository.getRepositoryState().blockingFirst()
.map(pState -> pState.getCurrentBranch().getSimpleName())
.orElse("current Branch"));
if (afterMergeConflictsSteps.isCommitWithDialog())
commitAfterMergeConflicts(pRepository, suggestedCommitMessage);
// in case there were no conflicts just commit the changes with the default message
else if (afterMergeConflictsSteps.isCommit)
pRepository.commit(suggestedCommitMessage);
}
catch (AlreadyUpToDateAditoGitException pE)
{
Expand All @@ -127,10 +126,90 @@ private void _doMerge(IProgressHandle pProgressHandle, IRepository pRepository,
}
finally
{
_performUnstash(pProgressHandle, pRepository, unstashChanges);
if (afterMergeConflictsSteps.isUnstash())
_performUnstash(pProgressHandle, pRepository, unstashChanges);
}
}

/**
* Create a commit message that contains information about the merge:
* - which branches were mergen
* - if there were conflicts, list which files had conflicts
*
* @param pRepository Repository of the project that the merge is done in
* @param pSelectedBranch The branch that is merged into the current branch
* @param pMergeConflictDiffs List of IMergeData that contains the conflicting files
* @return String with the message that should be given to the commit dialog as template for the user
*/
@VisibleForTesting
@NonNull
static String createSuggestedCommitMessage(@NonNull IRepository pRepository, @NonNull IBranch pSelectedBranch, @NonNull List<IMergeData> pMergeConflictDiffs)
{
String mergeMessage = "merged " + pSelectedBranch.getSimpleName() + " into " + pRepository.getRepositoryState().blockingFirst()
.map(pState -> pState.getCurrentBranch().getSimpleName())
.orElse("current Branch");
if (pMergeConflictDiffs.isEmpty())
{
return mergeMessage;
}
else
{
String conflictingFiles = pMergeConflictDiffs.stream().map(IMergeData::getFilePath).map(pPath -> "# " + pPath).collect(Collectors.joining("\n"));
return String.format("%s\n\n# merge had %d conflicting files:\n%s", mergeMessage, pMergeConflictDiffs.size(), conflictingFiles);
}
}

/**
* Perform the mergeConflictsSequence and handle the abort and save of the current merge
*
* @param pProgressHandle ProgressHandle that allows informing the user about the current work
* @param pRepository Repository of the project that the merge is done in
* @param pCurrentBranch Branch that the user is coming from (branch the user was on before he invoked the merge)
* @param pMergeDetails Details of the merge, includes conflicting files and such
* @return AfterMergeConflictsSteps that determine if a commit and unstash should happen
* @throws AditoGitException thrown if the user tries to abort the merge and HEAD cannot be reset to the tip of the current branch
*/
private AfterMergeConflictsSteps dealWithMergeConflicts(@NonNull IProgressHandle pProgressHandle, @NonNull IRepository pRepository, @NonNull IBranch pCurrentBranch,
@NonNull IMergeDetails pMergeDetails) throws AditoGitException
{
IMergeConflictDialogResult<?, ?> dialogResult = mergeConflictSequence.performMergeConflictSequence(Observable.just(Optional.of(pRepository)),
pMergeDetails, true);
IUserPromptDialogResult<?, ?> promptDialogResult = null;
if (!(dialogResult.isAbortMerge() || dialogResult.isFinishMerge()))
{
promptDialogResult = dialogProvider.showMessageDialog(null, Util.getResource(this.getClass(), "mergeSaveStateQuestion"),
List.of(EButtons.SAVE, EButtons.ABORT),
List.of(EButtons.SAVE));
if (promptDialogResult.isOkay())
{
notifyUtil.notify("Saved merge state", Util.getResource(this.getClass(), "mergeSavedStateMsg"), false);
return AfterMergeConflictsSteps.NO_COMMIT_NO_UNSTASH;
}
}
if (!dialogResult.isFinishMerge() || (promptDialogResult != null && !promptDialogResult.isOkay()))
{
pProgressHandle.setDescription("Aborting merge");
pRepository.reset(pCurrentBranch.getId(), EResetType.HARD);
// do not execute the "show commit dialog" part after this (no changes), but unstash the previously changed files
return AfterMergeConflictsSteps.NO_COMMIT_BUT_UNSTASH;
}
// there were conflicting files in the merge, so present the user with the dialog and a preset commit message and let him decide if the message suits him
return AfterMergeConflictsSteps.COMMIT_WITH_DIALOG_AND_UNSTASH;
}

/**
* Show the commit dialog, and based on the user input commit the selected changes (or leave them as-is if the user aborts)
*
* @param pRepository repository that contains the changes to commit
* @param suggestedCommitMessage message that should be preset for user convenience
*/
private void commitAfterMergeConflicts(@NonNull IRepository pRepository, @NonNull String suggestedCommitMessage)
{
actionProvider.getCommitAction(Observable.just(Optional.of(pRepository)), pRepository.getStatus()
.map(pStatus -> pStatus.map(IFileStatus::getUncommitted)),
suggestedCommitMessage).actionPerformed(null);
}

private void _performUnstash(IProgressHandle pProgressHandle, IRepository pRepository, boolean pUnstashChanges)
{
String stashedCommitId = prefStore.get(STASH_ID_KEY);
Expand All @@ -152,4 +231,35 @@ private static Observable<Optional<Boolean>> _getIsEnabledObservable(Observable<
{
return pTargetBranch.map(pBranch -> Optional.of(pBranch.isPresent()));
}

/**
* Enum that represents steps that should be done after a merge conflict resolution
*/
@AllArgsConstructor
private enum AfterMergeConflictsSteps
{
/**
* Do a fast commit (uses default commit message, does not ask the user for the commit message) and unstash the stashed changes afterwards
*/
COMMIT_AND_UNSTASH(true, false, true),
/**
* Do a commit with commit message approval (show the default commit dialog with preset commit message template) from the user and unstash the stashed changes afterwards
*/
COMMIT_WITH_DIALOG_AND_UNSTASH(false, true, true),
/**
* Do not commit but perform an unstash (used in conjunction with an aborted merge -> reset to state before the merge began and unstash changes)
*/
NO_COMMIT_BUT_UNSTASH(false, false, true),
/**
* Neither commit nor unstash, used if some conflicts were resolved and the current state of the merge should be kept for later
*/
NO_COMMIT_NO_UNSTASH(false, false, false);

@Getter
private final boolean isCommit;
@Getter
private final boolean isCommitWithDialog;
@Getter
private final boolean isUnstash;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ public class MergeRemoteAction extends MergeAction
MergeConflictSequence pMergeConflictSequence, IActionProvider pActionProvider, @Assisted Observable<Optional<IRepository>> pRepoObs,
@Assisted Observable<Optional<IBranch>> pTargetBranch)
{
super(pPrefStore, pProgressFacade, pDialogProvider, pSaveUtil, pNotifyUtil, pMergeConflictSequence, pRepoObs, pTargetBranch);
super(pPrefStore, pProgressFacade, pDialogProvider, pSaveUtil, pNotifyUtil, pActionProvider, pMergeConflictSequence, pRepoObs, pTargetBranch);
actionProvider = pActionProvider;
putValue(Action.NAME, "Fetch and merge into Current");
}
Expand Down
101 changes: 101 additions & 0 deletions gui/src/test/java/de/adito/git/gui/actions/MergeActionTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
package de.adito.git.gui.actions;

import de.adito.git.api.IRepository;
import de.adito.git.api.data.IBranch;
import de.adito.git.api.data.IRepositoryState;
import de.adito.git.api.data.diff.IMergeData;
import io.reactivex.rxjava3.core.Observable;
import lombok.NonNull;
import org.junit.jupiter.api.Nested;
import org.junit.jupiter.api.TestInstance;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.Arguments;
import org.junit.jupiter.params.provider.MethodSource;

import java.util.ArrayList;
import java.util.List;
import java.util.Optional;
import java.util.stream.Stream;

import static org.junit.jupiter.api.Assertions.assertAll;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;

/**
* Tests for the MergeAction class
*
* @author m.kaspera, 09.08.2023
*/
class MergeActionTest
{

@TestInstance(TestInstance.Lifecycle.PER_CLASS)
@Nested
class CreateSuggestedCommitMessage
{

/**
* @param pCurrentBranch Name of the current branch
* @param pMergeBranch Name of the branch that is merged into the current one
* @param pFilePaths List with conflicting filePaths
*/
@ParameterizedTest
@MethodSource("suggestedCommitMessageSource")
void doesMessageContainNecessaryInfos(@NonNull String pCurrentBranch, @NonNull String pMergeBranch, @NonNull List<String> pFilePaths)
{
IRepository repository = createMockRepo(pCurrentBranch);

IBranch mergeBranch = mock(IBranch.class);
when(mergeBranch.getSimpleName()).thenReturn(pMergeBranch);

List<IMergeData> mergeData = new ArrayList<>();
pFilePaths.forEach(pFilePath -> {
IMergeData data = mock(IMergeData.class);
when(data.getFilePath()).thenReturn(pFilePath);
mergeData.add(data);
});

String suggestedCommitMessage = MergeAction.createSuggestedCommitMessage(repository, mergeBranch, mergeData);

pFilePaths.forEach(filePath -> assertTrue(suggestedCommitMessage.contains(filePath)));
assertAll(() -> assertEquals(pFilePaths.isEmpty(), !suggestedCommitMessage.contains("\n")),
// check if the branch names are mentioned in the message
() -> assertTrue(suggestedCommitMessage.contains(pCurrentBranch)),
() -> assertTrue(suggestedCommitMessage.contains(pMergeBranch)));
}

/**
* create a mocked repository
*
* @param pCurrentBranch Name of the current branch
* @return Mocked IRepository that returns a mocked repositoryState that as the current Branch set (mocked again)
*/
@NonNull
private IRepository createMockRepo(@NonNull String pCurrentBranch)
{
IRepository repository = mock(IRepository.class);
IRepositoryState repositoryState = mock(IRepositoryState.class);
IBranch currentBranch = mock(IBranch.class);

when(repository.getRepositoryState()).thenReturn(Observable.just(Optional.of(repositoryState)));
when(repositoryState.getCurrentBranch()).thenReturn(currentBranch);
when(currentBranch.getSimpleName()).thenReturn(pCurrentBranch);
return repository;
}

/**
* @return Stream of Arguments for the doesMessageContainConflictingFiles test
*/
public Stream<Arguments> suggestedCommitMessageSource()
{
return Stream.of(Arguments.of("main", "dev", List.of()),
Arguments.of("main", "dev", List.of("entity/org/process/test.js")),
Arguments.of("main", "dev", List.of("package.json", "")),
Arguments.of("main", "dev", List.of("package.json", "entity/org/org_entity.aod")));
}

}

}

0 comments on commit 2a3622f

Please sign in to comment.