From c5f24eacc2ffa8695aaa82789c439a0abc365808 Mon Sep 17 00:00:00 2001 From: Victor Andreasson Date: Sun, 1 Sep 2024 14:01:23 +0200 Subject: [PATCH] Use a single SSH session for all operations And clean up code which is no longer called. --- .../android/git/GitFileSynchronizer.java | 314 +----------------- .../android/git/GitSshKeyTransportSetter.kt | 73 ++-- .../android/git/GitTransportSetter.java | 2 +- .../android/git/HTTPSTransportSetter.java | 4 + .../com/orgzly/android/repos/GitRepo.java | 229 ++++++------- .../orgzly/android/repos/TwoWaySyncRepo.kt | 4 - .../java/com/orgzly/android/sync/SyncUtils.kt | 61 ---- 7 files changed, 171 insertions(+), 516 deletions(-) diff --git a/app/src/main/java/com/orgzly/android/git/GitFileSynchronizer.java b/app/src/main/java/com/orgzly/android/git/GitFileSynchronizer.java index 215d0ecf4..93dd4764e 100644 --- a/app/src/main/java/com/orgzly/android/git/GitFileSynchronizer.java +++ b/app/src/main/java/com/orgzly/android/git/GitFileSynchronizer.java @@ -3,20 +3,17 @@ import static com.orgzly.android.ui.AppSnackbarUtils.showSnackbar; import android.app.Activity; -import android.content.Context; import android.net.Uri; import android.util.Log; import androidx.annotation.Nullable; import com.orgzly.BuildConfig; -import com.orgzly.R; import com.orgzly.android.App; import com.orgzly.android.util.LogUtils; import com.orgzly.android.util.MiscUtils; import org.eclipse.jgit.api.Git; -import org.eclipse.jgit.api.ListBranchCommand; import org.eclipse.jgit.api.MergeResult; import org.eclipse.jgit.api.RebaseCommand; import org.eclipse.jgit.api.RebaseResult; @@ -25,7 +22,6 @@ import org.eclipse.jgit.api.errors.GitAPIException; import org.eclipse.jgit.diff.DiffEntry; import org.eclipse.jgit.lib.Constants; -import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.ObjectReader; import org.eclipse.jgit.lib.Ref; import org.eclipse.jgit.lib.Repository; @@ -37,36 +33,25 @@ import org.eclipse.jgit.transport.RemoteRefUpdate; import org.eclipse.jgit.treewalk.AbstractTreeIterator; import org.eclipse.jgit.treewalk.CanonicalTreeParser; -import org.eclipse.jgit.treewalk.TreeWalk; import java.io.File; import java.io.FileInputStream; import java.io.FileNotFoundException; import java.io.IOException; import java.io.InputStream; -import java.text.SimpleDateFormat; -import java.util.Calendar; import java.util.List; -import java.util.TimeZone; public class GitFileSynchronizer { private final static String TAG = GitFileSynchronizer.class.getName(); - public final static String PRE_SYNC_MARKER_BRANCH = "orgzly-pre-sync-marker"; public final static String CONFLICT_BRANCH = "ORGZLY_CONFLICT"; private final Git git; private final GitPreferences preferences; - private final Context context; private final Activity currentActivity = App.getCurrentActivity(); public GitFileSynchronizer(Git g, GitPreferences prefs) { git = g; preferences = prefs; - context = App.getAppContext(); - } - - private GitTransportSetter transportSetter() { - return preferences.createTransportSetter(); } public void retrieveLatestVersionOfFile( @@ -104,16 +89,16 @@ public List getCommitDiff(RevCommit oldCommit, RevCommit newCommit) t .call(); } - public RevCommit fetch() throws IOException { + public RevCommit fetch(GitTransportSetter transportSetter) throws IOException { try { if (BuildConfig.LOG_DEBUG) { LogUtils.d(TAG, String.format("Fetching Git repo from %s", preferences.remoteUri())); } - transportSetter() - .setTransport(git.fetch() + transportSetter.setTransport( + git.fetch() .setRemote(preferences.remoteName()) - .setRemoveDeletedRefs(true)) - .call(); + .setRemoveDeletedRefs(true) + ).call(); } catch (GitAPIException e) { Log.e(TAG, e.toString()); throw new IOException(e.getMessage()); @@ -122,14 +107,10 @@ public RevCommit fetch() throws IOException { return getCommit("origin/" + currentBranch); } - public void checkoutSelected() throws GitAPIException { - git.checkout().setName(preferences.branchName()).call(); - } - - public boolean mergeWithRemote() throws IOException { + public boolean pull(GitTransportSetter transportSetter) throws IOException { ensureRepoIsClean(); try { - fetch(); + fetch(transportSetter); RevCommit mergeTarget = getCommit( String.format("%s/%s", preferences.remoteName(), git.getRepository().getBranch())); @@ -158,103 +139,6 @@ public RebaseResult rebase() throws IOException { return result; } - private String getShortHash(ObjectId hash) { - String shortHash = hash.getName(); - try { - shortHash = git.getRepository().newObjectReader().abbreviate(hash).name(); - } catch(IOException e) { - Log.e(TAG, "Error while abbreviating commit hash " + hash.getName() + ", falling back to full hash"); - } - return shortHash; - } - - private String createConflictBranchName(String repositoryPath, ObjectId commitHash) { - String shortCommitHash = getShortHash(commitHash); - repositoryPath = repositoryPath.replace(" ", "_"); - String now = new SimpleDateFormat("yyyy-MM-dd_HHmmss").format(Calendar.getInstance(TimeZone.getTimeZone("UTC")).getTime()); - return "merge-" + repositoryPath + "-" + shortCommitHash + "-" + now; - } - - public boolean updateAndCommitFileFromRevisionAndMerge( - File sourceFile, String repoRelativePath, - ObjectId fileRevision, RevCommit revision) - throws IOException { - ensureRepoIsClean(); - if (updateAndCommitFileFromRevision(sourceFile, repoRelativePath, fileRevision)) { - if (BuildConfig.LOG_DEBUG) { - LogUtils.d(TAG, String.format("File '%s' committed without conflicts.", repoRelativePath)); - } - return true; - } - - String originalBranch = git.getRepository().getFullBranch(); - if (BuildConfig.LOG_DEBUG) { - LogUtils.d(TAG, String.format("originalBranch is set to %s", originalBranch)); - } - String mergeBranch = createConflictBranchName(repoRelativePath, fileRevision); - if (BuildConfig.LOG_DEBUG) { - LogUtils.d(TAG, String.format("originalBranch is set to %s", originalBranch)); - LogUtils.d(TAG, String.format("Temporary mergeBranch is set to %s", mergeBranch)); - } - try { - git.branchDelete().setBranchNames(mergeBranch).call(); - } catch (GitAPIException ignored) {} - boolean mergeSucceeded = false; - try { - RevCommit mergeTarget = currentHead(); - // Try to use our "pre sync marker" to find a good point in history for branching off. - RevCommit branchStartPoint = getCommit(PRE_SYNC_MARKER_BRANCH); - if (branchStartPoint == null) { - branchStartPoint = revision; - } - if (BuildConfig.LOG_DEBUG) { - LogUtils.d(TAG, String.format("branchStartPoint is set to %s", branchStartPoint)); - } - git.checkout().setCreateBranch(true).setForceRefUpdate(true). - setStartPoint(branchStartPoint).setName(mergeBranch).call(); - if (!currentHead().equals(branchStartPoint)) - throw new IOException("Failed to create new branch at " + branchStartPoint.toString()); - if (!updateAndCommitFileFromRevision(sourceFile, repoRelativePath, fileRevision)) - throw new IOException( - String.format( - "The provided file revision %s for %s is " + - "not the same as the one found in the provided commit %s.", - fileRevision.toString(), repoRelativePath, revision.toString())); - mergeSucceeded = doMerge(mergeTarget); - if (mergeSucceeded) { - RevCommit merged = currentHead(); - git.checkout().setName(originalBranch).call(); - MergeResult result = git.merge().include(merged).call(); - if (!result.getMergeStatus().isSuccessful()) { - throw new IOException(String.format("Unexpected failure to merge '%s' into '%s'", merged.toString(), originalBranch)); - } - } - } catch (GitAPIException e) { - e.printStackTrace(); - throw new IOException("Failed to handle merge conflict: " + e.getMessage()); - } finally { - if (mergeSucceeded) { - try { - if (BuildConfig.LOG_DEBUG) { - LogUtils.d(TAG, String.format("Checking out originalBranch '%s'", originalBranch)); - } - git.checkout().setName(originalBranch).call(); - } catch (GitAPIException e) { - Log.w(TAG, String.format("Failed to checkout original branch '%s': %s", originalBranch, e.getMessage())); - } - try { - if (BuildConfig.LOG_DEBUG) { - LogUtils.d(TAG, String.format("Deleting temporary mergeBranch '%s'", mergeBranch)); - } - git.branchDelete().setBranchNames(mergeBranch).call(); - } catch (GitAPIException e) { - Log.w(TAG, String.format("Failed to delete temporary mergeBranch '%s': %s", mergeBranch, e.getMessage())); - } - } - } - return mergeSucceeded; - } - private boolean doMerge(RevCommit mergeTarget) throws IOException, GitAPIException { MergeResult result = git.merge().include(mergeTarget).call(); if (result.getMergeStatus().equals(MergeResult.MergeStatus.CONFLICTING)) { @@ -264,40 +148,9 @@ private boolean doMerge(RevCommit mergeTarget) throws IOException, GitAPIExcepti return true; } - /** - * Try to push to remote if local and remote HEADs for the current branch - * point to different commits. This method was added to allow pushing only - * once per sync occasion: right after the "for namesake in namesakes"-loop - * in SyncService.doInBackground(). - */ - public void tryPushIfHeadDiffersFromRemote() { - String branchName = null; - String remoteName = null; - RevCommit localHead = null; - RevCommit remoteHead = null; - Repository repo = git.getRepository(); - - try { - branchName = repo.getBranch(); - localHead = currentHead(); - remoteName = preferences.remoteName(); - } catch (IOException e) { - e.printStackTrace(); - } - - // Try to get the commit of the remote head with the same name as our local current head - try { - remoteHead = getCommit(remoteName + "/" + branchName); - } catch (IOException ignored) {} - - if (localHead != null && !localHead.equals(remoteHead)) { - push(); - } - } - - public void pushToConflictBranch() { + public void pushToConflictBranch(GitTransportSetter transportSetter) { RefSpec refSpec = new RefSpec("HEAD:refs/heads/" + CONFLICT_BRANCH); - final var pushCommand = transportSetter().setTransport(git.push().setRefSpecs(refSpec).setForce(true)); + final var pushCommand = transportSetter.setTransport(git.push().setRefSpecs(refSpec).setForce(true)); final Object monitor = new Object(); App.EXECUTORS.diskIO().execute(() -> { try { @@ -325,8 +178,8 @@ public void pushToConflictBranch() { } } - public RemoteRefUpdate pushWithResult() throws Exception { - final var pushCommand = transportSetter().setTransport( + public RemoteRefUpdate pushWithResult(GitTransportSetter transportSetter) throws Exception { + final var pushCommand = transportSetter.setTransport( git.push().setRemote(preferences.remoteName())); final Object monitor = new Object(); final RemoteRefUpdate[] result = new RemoteRefUpdate[1]; @@ -355,8 +208,8 @@ public RemoteRefUpdate pushWithResult() throws Exception { return result[0]; } - public void push() { - final var pushCommand = transportSetter().setTransport( + public void push(GitTransportSetter transportSetter) { + final var pushCommand = transportSetter.setTransport( git.push().setRemote(preferences.remoteName())); final Object monitor = new Object(); @@ -403,123 +256,10 @@ private void gitResetMerge() throws IOException, GitAPIException { git.reset().setMode(ResetCommand.ResetType.HARD).call(); } - public boolean updateAndCommitFileFromRevision( - File sourceFile, String repoRelativePath, ObjectId revision) throws IOException { - ensureRepoIsClean(); - ObjectId repositoryRevision = getFileRevision(repoRelativePath, currentHead()); - if (repositoryRevision.equals(revision)) { - updateAndCommitFile(sourceFile, repoRelativePath); - return true; - } - return false; - } - - public void setBranchAndGetLatest() throws IOException { - ensureRepoIsClean(); - try { - // Point a "marker" branch to the current head, so that we know a good starting commit - // for merge conflict branches. - git.branchCreate().setName(PRE_SYNC_MARKER_BRANCH).setForce(true).call(); - } catch (GitAPIException e) { - // We may end up here when syncing an empty Git repo for the first time. So don't - // panic, just log an info message. - Log.i(TAG, context.getString(R.string.git_sync_error_failed_set_marker_branch)); - } - fetch(); - try { - RevCommit current = currentHead(); - if (current == null) { - Log.i(TAG, "Git repo does not seem to have any commits."); - return; - } - RevCommit mergeTarget = getCommit( - String.format("%s/%s", preferences.remoteName(), git.getRepository().getBranch())); - if (mergeTarget != null) { - if (doMerge(mergeTarget)) { // Try to merge with the remote head of the current branch. - if (!git.getRepository().getBranch().equals(preferences.branchName())) { - // We are not on the main branch. Make an attempt to return to it. - attemptReturnToMainBranch(); - } - } else { - throw new IOException(String.format("Failed to merge %s and %s", - current.getName(), mergeTarget.getName())); - } - } else { - // We failed to find a corresponding remote head. Check if the repo is completely - // empty, and if so, push to it. - pushToRemoteIfEmpty(); - } - } catch (GitAPIException e) { - throw new IOException(e.getMessage()); - } - } - - private void pushToRemoteIfEmpty() throws GitAPIException { - List remoteBranches = git.branchList() - .setListMode(ListBranchCommand.ListMode.REMOTE) - .call(); - if (remoteBranches.isEmpty()) { - push(); - } - } - - public boolean attemptReturnToMainBranch() throws IOException { - ensureRepoIsClean(); - String originalBranch = git.getRepository().getBranch(); - RevCommit mergeTarget = getCommit( - String.format("%s/%s", preferences.remoteName(), preferences.branchName())); - boolean backOnMainBranch = false; - try { - if (doMerge(mergeTarget)) { - RevCommit merged = currentHead(); - checkoutSelected(); - if (doMerge(merged)) { - backOnMainBranch = true; - git.branchDelete().setBranchNames(originalBranch); - } - } - } catch (Exception e) { - e.printStackTrace(); - } - if (!backOnMainBranch) { - try { - git.checkout().setName(originalBranch).call(); - } catch (GitAPIException ge) { - ge.printStackTrace(); - throw new IOException("Error during checkout after failed merge attempt."); - } - } - return backOnMainBranch; - } - - public void updateAndCommitExistingFile(File sourceFile, String repositoryPath) throws IOException { - ensureRepoIsClean(); - File destinationFile = workTreeFile(repositoryPath); - if (!destinationFile.exists()) { - throw new FileNotFoundException("File " + destinationFile + " does not exist"); - } - updateAndCommitFile(sourceFile, repositoryPath); - } - - /** - * Add a new file to the repository, while ensuring that it didn't already exist. - * @param sourceFile This will become the contents of the added file - * @param repositoryPath Path inside the repo where the file should be added - * @throws IOException If the file already exists - */ - public void addAndCommitNewFile(File sourceFile, String repositoryPath) throws IOException { - ensureRepoIsClean(); - File destinationFile = workTreeFile(repositoryPath); - if (destinationFile.exists()) { - throw new IOException("Can't add new file " + repositoryPath + " that already exists."); - } - ensureDirectoryHierarchy(repositoryPath); - updateAndCommitFile(sourceFile, repositoryPath); - } - private void ensureDirectoryHierarchy(String repositoryPath) throws IOException { if (repositoryPath.contains("/")) { File targetDir = workTreeFile(repositoryPath).getParentFile(); + assert targetDir != null; if (!(targetDir.exists() || targetDir.mkdirs())) { throw new IOException("The directory " + targetDir.getAbsolutePath() + " could " + "not be created"); @@ -527,19 +267,6 @@ private void ensureDirectoryHierarchy(String repositoryPath) throws IOException } } - private void updateAndCommitFile( - File sourceFile, String repoRelativePath) throws IOException { - File destinationFile = workTreeFile(repoRelativePath); - MiscUtils.copyFile(sourceFile, destinationFile); - try { - git.add().addFilepattern(repoRelativePath).call(); - if (!gitRepoIsClean()) - commit(String.format("Orgzly update: %s", repoRelativePath)); - } catch (GitAPIException e) { - throw new IOException("Failed to commit changes."); - } - } - public void writeFileAndAddToIndex(File sourceFile, String repoRelativePath) throws IOException { if (repoHasUnstagedChanges()) throw new IOException("Git working tree is in an unclean state; refusing to update."); @@ -635,13 +362,8 @@ public boolean isEmptyRepo() throws IOException{ return git.getRepository().exactRef(Constants.HEAD).getObjectId() == null; } - public ObjectId getFileRevision(String pathString, RevCommit commit) throws IOException { - return TreeWalk.forPath( - git.getRepository(), pathString, commit.getTree()).getObjectId(0); - } - - public boolean deleteFileFromRepo(Uri uri) throws IOException { - if (mergeWithRemote()) { + public boolean deleteFileFromRepo(Uri uri, GitTransportSetter transportSetter) throws IOException { + if (pull(transportSetter)) { String repoRelativePath = uri.getPath(); try { git.rm().addFilepattern(repoRelativePath).call(); @@ -656,9 +378,9 @@ public boolean deleteFileFromRepo(Uri uri) throws IOException { } } - public boolean renameFileInRepo(String oldPath, String newPath) throws IOException { + public boolean renameFileInRepo(String oldPath, String newPath, GitTransportSetter transportSetter) throws IOException { ensureRepoIsClean(); - if (mergeWithRemote()) { + if (pull(transportSetter)) { File oldFile = workTreeFile(oldPath); File newFile = workTreeFile(newPath); // Abort if destination file exists diff --git a/app/src/main/java/com/orgzly/android/git/GitSshKeyTransportSetter.kt b/app/src/main/java/com/orgzly/android/git/GitSshKeyTransportSetter.kt index b13e0565b..71d9673a5 100644 --- a/app/src/main/java/com/orgzly/android/git/GitSshKeyTransportSetter.kt +++ b/app/src/main/java/com/orgzly/android/git/GitSshKeyTransportSetter.kt @@ -3,15 +3,19 @@ package com.orgzly.android.git import android.os.Build import androidx.annotation.RequiresApi import com.orgzly.android.App -import org.eclipse.jgit.annotations.NonNull import org.eclipse.jgit.api.TransportCommand import org.eclipse.jgit.api.TransportConfigCallback import org.eclipse.jgit.internal.transport.sshd.OpenSshServerKeyDatabase +import org.eclipse.jgit.transport.CredentialsProvider +import org.eclipse.jgit.transport.RemoteSession import org.eclipse.jgit.transport.SshSessionFactory import org.eclipse.jgit.transport.SshTransport import org.eclipse.jgit.transport.Transport +import org.eclipse.jgit.transport.URIish import org.eclipse.jgit.transport.sshd.ServerKeyDatabase +import org.eclipse.jgit.transport.sshd.SshdSession import org.eclipse.jgit.transport.sshd.SshdSessionFactory +import org.eclipse.jgit.util.FS import java.io.File import java.security.KeyPair @@ -19,35 +23,53 @@ class GitSshKeyTransportSetter: GitTransportSetter { private val configCallback: TransportConfigCallback private val context = App.getAppContext() - init { - val factory: SshSessionFactory = object : SshdSessionFactory(null, null) { + private val factory = object: SshdSessionFactory(null, null) { + private lateinit var session: SshdSession - override fun getHomeDirectory(): File { return context.filesDir } + override fun getDefaultPreferredAuthentications(): String { return "publickey" } - override fun getDefaultPreferredAuthentications(): String { return "publickey" } + override fun createServerKeyDatabase( + homeDir: File, + sshDir: File + ): ServerKeyDatabase { + // We override this method because we want to set "askAboutNewFile" to False. + return OpenSshServerKeyDatabase( + false, + getDefaultKnownHostsFiles(sshDir) + ) + } - override fun createServerKeyDatabase( - @NonNull homeDir: File, - @NonNull sshDir: File - ): ServerKeyDatabase { - // We override this method because we want to set "askAboutNewFile" to False. - return OpenSshServerKeyDatabase( - false, - getDefaultKnownHostsFiles(sshDir) - ) + @RequiresApi(Build.VERSION_CODES.N) + override fun getDefaultKeys(sshDir: File): Iterable? { + return if (SshKey.exists) { + listOf(SshKey.getKeyPair()) + } else { + SshKey.promptForKeyGeneration() + null } + } - @RequiresApi(Build.VERSION_CODES.N) - override fun getDefaultKeys(@NonNull sshDir: File): Iterable? { - return if (SshKey.exists) { - listOf(SshKey.getKeyPair()) - } else { - SshKey.promptForKeyGeneration() - null - } - } + override fun releaseSession(session: RemoteSession) { + // Do nothing. We want to leave SSH sessions open. } + override fun getSession( + uri: URIish?, + credentialsProvider: CredentialsProvider?, + fs: FS?, + tms: Int + ): SshdSession { + if (this::session.isInitialized) { return session } + session = super.getSession(uri, credentialsProvider, fs, tms) + return session + } + + fun disconnect() { + session.disconnect() + } + } + + init { SshSessionFactory.setInstance(factory) // org.apache.sshd.common.config.keys.IdentityUtils freaks out if user.home is not set @@ -59,10 +81,13 @@ class GitSshKeyTransportSetter: GitTransportSetter { } } + override fun close() { + factory.disconnect() + } + override fun setTransport(tc: TransportCommand<*, *>): TransportCommand<*, *> { tc.setTransportConfigCallback(configCallback) tc.setCredentialsProvider(SshCredentialsProvider()) return tc } - } diff --git a/app/src/main/java/com/orgzly/android/git/GitTransportSetter.java b/app/src/main/java/com/orgzly/android/git/GitTransportSetter.java index 8462471ed..e0e18d9d1 100644 --- a/app/src/main/java/com/orgzly/android/git/GitTransportSetter.java +++ b/app/src/main/java/com/orgzly/android/git/GitTransportSetter.java @@ -2,6 +2,6 @@ import org.eclipse.jgit.api.TransportCommand; -public interface GitTransportSetter { +public interface GitTransportSetter extends AutoCloseable { public TransportCommand setTransport(TransportCommand tc); } diff --git a/app/src/main/java/com/orgzly/android/git/HTTPSTransportSetter.java b/app/src/main/java/com/orgzly/android/git/HTTPSTransportSetter.java index febba22cd..83f31d5ee 100644 --- a/app/src/main/java/com/orgzly/android/git/HTTPSTransportSetter.java +++ b/app/src/main/java/com/orgzly/android/git/HTTPSTransportSetter.java @@ -18,4 +18,8 @@ public TransportCommand setTransport(TransportCommand tc) { tc.setCredentialsProvider(new UsernamePasswordCredentialsProvider(username, password)); return tc; } + + public void close() { + // Nothing to do here; the HTTPS transport does not use persistent connections. + } } diff --git a/app/src/main/java/com/orgzly/android/repos/GitRepo.java b/app/src/main/java/com/orgzly/android/repos/GitRepo.java index c04e3ea45..0e5c09233 100644 --- a/app/src/main/java/com/orgzly/android/repos/GitRepo.java +++ b/app/src/main/java/com/orgzly/android/repos/GitRepo.java @@ -6,7 +6,6 @@ import androidx.annotation.Nullable; -import com.orgzly.BuildConfig; import com.orgzly.R; import com.orgzly.android.App; import com.orgzly.android.NotesOrgExporter; @@ -25,7 +24,6 @@ import com.orgzly.android.sync.BookNamesake; import com.orgzly.android.sync.BookSyncStatus; import com.orgzly.android.sync.SyncState; -import com.orgzly.android.util.LogUtils; import org.eclipse.jgit.api.CloneCommand; import org.eclipse.jgit.api.Git; @@ -34,11 +32,9 @@ import org.eclipse.jgit.diff.DiffEntry; import org.eclipse.jgit.ignore.IgnoreNode; import org.eclipse.jgit.lib.FileMode; -import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.ProgressMonitor; import org.eclipse.jgit.lib.StoredConfig; import org.eclipse.jgit.revwalk.RevCommit; -import org.eclipse.jgit.revwalk.RevWalk; import org.eclipse.jgit.storage.file.FileRepositoryBuilder; import org.eclipse.jgit.transport.RemoteRefUpdate; import org.eclipse.jgit.treewalk.TreeWalk; @@ -53,6 +49,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Objects; public class GitRepo implements SyncRepo, TwoWaySyncRepo { private final static String TAG = GitRepo.class.getName(); @@ -80,11 +77,11 @@ public static GitRepo getInstance(RepoWithProps props, Context context) throws I // TODO: Build from info - return build(props.getRepo().getId(), prefs, false); + return build(props.getRepo().getId(), prefs); } - private static GitRepo build(long id, GitPreferences prefs, boolean clone) throws IOException { - Git git = ensureRepositoryExists(prefs, clone, null); + private static GitRepo build(long id, GitPreferences prefs) throws IOException { + Git git = ensureRepositoryExists(prefs, false, null); StoredConfig config = git.getRepository().getConfig(); config.setString("remote", prefs.remoteName(), "url", prefs.remoteUri().toString()); @@ -127,7 +124,7 @@ public static Git ensureRepositoryExists( */ private static Git verifyExistingRepo(File directoryFile) throws IOException { if (!directoryFile.exists()) { - throw new IOException(String.format("The directory %s does not exist", directoryFile.toString()), new FileNotFoundException()); + throw new IOException(String.format("The directory %s does not exist", directoryFile), new FileNotFoundException()); } FileRepositoryBuilder frb = new FileRepositoryBuilder(); @@ -152,13 +149,13 @@ private static Git verifyExistingRepo(File directoryFile) throws IOException { private static Git cloneRepo(Uri repoUri, File directoryFile, GitTransportSetter transportSetter, ProgressMonitor pm) throws IOException { if (!directoryFile.exists()) { - throw new IOException(String.format("The directory %s does not exist", directoryFile.toString()), new FileNotFoundException()); + throw new IOException(String.format("The directory %s does not exist", directoryFile), new FileNotFoundException()); } // Using list() can be resource intensive if there's many files, but since we just call it // at the time of cloning once we should be fine for now - if (directoryFile.list().length != 0) { - throw new IOException(String.format("The directory must be empty"), new DirectoryNotEmpty(directoryFile)); + if (Objects.requireNonNull(directoryFile.list()).length != 0) { + throw new IOException("The directory must be empty", new DirectoryNotEmpty(directoryFile)); } try { @@ -180,9 +177,9 @@ private static Git cloneRepo(Uri repoUri, File directoryFile, GitTransportSetter } } - private Git git; - private GitFileSynchronizer synchronizer; - private GitPreferences preferences; + private final Git git; + private final GitFileSynchronizer synchronizer; + private final GitPreferences preferences; public GitRepo(long id, Git g, GitPreferences prefs) { repoId = id; @@ -209,25 +206,17 @@ public boolean isAutoSyncSupported() { * @throws IOException */ public VersionedRook storeBook(File file, String repoRelativePath) throws IOException { - File destination = synchronizer.workTreeFile(repoRelativePath); - - if (destination.exists()) { - synchronizer.updateAndCommitExistingFile(file, repoRelativePath); - } else { - synchronizer.addAndCommitNewFile(file, repoRelativePath); + synchronizer.writeFileAndAddToIndex(file, repoRelativePath); + synchronizer.commitAnyStagedChanges(); + try (GitTransportSetter transportSetter = preferences.createTransportSetter()) { + synchronizer.push(transportSetter); + } catch (Exception e) { + Log.e(TAG, e.toString()); + throw new RuntimeException(e); } - synchronizer.push(); return currentVersionedRook(Uri.EMPTY.buildUpon().path(repoRelativePath).build()); } - private RevWalk walk() { - return new RevWalk(git.getRepository()); - } - - RevCommit getCommitFromRevisionString(String revisionString) throws IOException { - return walk().parseCommit(ObjectId.fromString(revisionString)); - } - /** * N.B: NOT called during regular GitRepo syncing, only during force-loading. * @param repoRelativePath @@ -238,9 +227,16 @@ RevCommit getCommitFromRevisionString(String revisionString) throws IOException @Override public VersionedRook retrieveBook(String repoRelativePath, File destination) throws IOException { Uri sourceUri = Uri.EMPTY.buildUpon().path(repoRelativePath).build(); - synchronizer.fetch(); + try (GitTransportSetter transportSetter = preferences.createTransportSetter()) { + synchronizer.fetch(transportSetter); + } catch (Exception e) { + Log.e(TAG, e.toString()); + throw new RuntimeException(e); + } try { // Reset our entire working tree to the remote head + // TODO: Introduce helper method for loading remote changes which can be called both + // here and during regular syncing. synchronizer.hardResetToRemoteHead(); } catch (GitAPIException e) { throw new RuntimeException(e); @@ -312,23 +308,36 @@ public Uri getUri() { } public void delete(Uri uri) throws IOException { - if (synchronizer.deleteFileFromRepo(uri)) synchronizer.push(); + try (GitTransportSetter transportSetter = preferences.createTransportSetter()) { + synchronizer.fetch(transportSetter); + if (synchronizer.deleteFileFromRepo(uri, transportSetter)) + synchronizer.push(transportSetter); + } catch (Exception e) { + Log.e(TAG, e.toString()); + throw new RuntimeException(e); + } } public VersionedRook renameBook(Uri oldFullUri, String newName) throws IOException { String oldPath = oldFullUri.getPath(); String newPath = BookName.repoRelativePath(newName, BookFormat.ORG); - if (synchronizer.renameFileInRepo(oldPath, newPath)) { - synchronizer.push(); - return currentVersionedRook(Uri.EMPTY.buildUpon().path(newPath).build()); - } else { - return null; + try (GitTransportSetter transportSetter = preferences.createTransportSetter()) { + if (synchronizer.renameFileInRepo(oldPath, newPath, transportSetter)) { + synchronizer.push(transportSetter); + return currentVersionedRook(Uri.EMPTY.buildUpon().path(newPath).build()); + } else { + return null; + } + } catch (Exception e) { + Log.e(TAG, e.toString()); + throw new RuntimeException(e); } } @Nullable @Override public SyncState syncRepo(DataRepository dataRepository) throws Exception { + // TODO: Add regression test for empty remote (no initial commit) RevCommit remoteHeadBeforeFetch = synchronizer.getRemoteHead(); RevCommit newRemoteHead = null; List allLinkedBooks = dataRepository.getBooksLinkedToRepo(repoId); @@ -397,65 +406,64 @@ public SyncState syncRepo(DataRepository dataRepository) throws Exception { namesake.setStatus(BookSyncStatus.BOOK_WITH_LINK_LOCAL_MODIFIED); } boolean rebaseWasAttempted = false; - if (!syncedBooks.isEmpty()) { - RevCommit newCommit = synchronizer.commitAnyStagedChanges(); - for (BookNamesake namesake : syncedBooks.values()) { - if (newCommit != null) { - Uri fileUri = - Uri.EMPTY.buildUpon().path(BookName.getRepoRelativePath(namesake.getBook())).build(); - VersionedRook vrook = new VersionedRook(repoId, repoType, getUri(), - fileUri, newCommit.name(), (long) newCommit.getCommitTime() * 1000); - dataRepository.updateBookLinkAndSync(namesake.getBook().getBook().getId(), - vrook); - } - dataRepository.setBookIsNotModified(namesake.getBook().getBook().getId()); - } - // Try pushing - RemoteRefUpdate pushResult = synchronizer.pushWithResult(); - if (pushResult == null) - throw new IOException("Git push failed unexpectedly"); - switch (pushResult.getStatus()) { - case OK: - case UP_TO_DATE: - for (BookNamesake namesake : syncedBooks.values()) { - storeBookStatus(dataRepository, namesake.getBook(), namesake.getStatus()); + // We know we must connect to the remote at some point, whether we have local changes or + // not. Let's do it here. + try (GitTransportSetter transportSetter = preferences.createTransportSetter()) { + if (!syncedBooks.isEmpty()) { + RevCommit newCommit = synchronizer.commitAnyStagedChanges(); + for (BookNamesake namesake : syncedBooks.values()) { + if (newCommit != null) { + Uri fileUri = Uri.EMPTY.buildUpon().path(BookName.getRepoRelativePath(namesake.getBook())).build(); + VersionedRook vrook = new VersionedRook(repoId, repoType, getUri(), fileUri, newCommit.name(), (long) newCommit.getCommitTime() * 1000); + dataRepository.updateBookLinkAndSync(namesake.getBook().getBook().getId(), vrook); } - break; - case REJECTED_NONFASTFORWARD: - case REJECTED_REMOTE_CHANGED: - // Try rebasing on latest remote head - newRemoteHead = synchronizer.fetch(); - switch (synchronizer.rebase().getStatus()) { - case FAST_FORWARD: // Only remote changes - case OK: // Remote and local changes - if (!syncedBooks.isEmpty()) { - synchronizer.push(); + dataRepository.setBookIsNotModified(namesake.getBook().getBook().getId()); + } + // Try pushing + RemoteRefUpdate pushResult = synchronizer.pushWithResult(transportSetter); + if (pushResult == null) throw new IOException("Git push failed unexpectedly"); + switch (pushResult.getStatus()) { + case OK: + case UP_TO_DATE: + for (BookNamesake namesake : syncedBooks.values()) { + storeBookStatus(dataRepository, namesake.getBook(), namesake.getStatus()); + } + break; + case REJECTED_NONFASTFORWARD: + case REJECTED_REMOTE_CHANGED: + // Try rebasing on latest remote head + newRemoteHead = synchronizer.fetch(transportSetter); + switch (synchronizer.rebase().getStatus()) { + case FAST_FORWARD: // Only remote changes + case OK: // Remote and local changes + if (!syncedBooks.isEmpty()) { + synchronizer.push(transportSetter); + for (BookNamesake namesake : syncedBooks.values()) { + storeBookStatus(dataRepository, namesake.getBook(), namesake.getStatus()); + } + } + break; + default: + // Rebase failed; push to conflict branch + synchronizer.pushToConflictBranch(transportSetter); for (BookNamesake namesake : syncedBooks.values()) { - storeBookStatus( - dataRepository, - namesake.getBook(), - namesake.getStatus() - ); + namesake.setStatus(BookSyncStatus.CONFLICT_SAVED_TO_TEMP_BRANCH); + storeBookStatus(dataRepository, namesake.getBook(), namesake.getStatus()); } - } - break; - default: - // Rebase failed; push to conflict branch - synchronizer.pushToConflictBranch(); - for (BookNamesake namesake : syncedBooks.values()) { - namesake.setStatus(BookSyncStatus.CONFLICT_SAVED_TO_TEMP_BRANCH); - storeBookStatus(dataRepository, namesake.getBook(), namesake.getStatus()); - } - } - rebaseWasAttempted = true; - break; - default: - throw new IOException("Error during git push: " + pushResult.getMessage()); + } + rebaseWasAttempted = true; + break; + default: + throw new IOException("Error during git push: " + pushResult.getMessage()); + } + } else { + // No local changes, but fetch is needed to discover remote changes + newRemoteHead = synchronizer.fetch(transportSetter); } - } else { - // No local changes, but fetch is needed to discover remote changes - newRemoteHead = synchronizer.fetch(); - } + } catch (Exception e) { + Log.e(TAG, e.toString()); + throw new RuntimeException(e); + } // Connection to remote is closed here; we have both pushed and fetched, if needed. if (newRemoteHead != null && !newRemoteHead.name().equals(remoteHeadBeforeFetch.name())) { // There are remote changes. // Ensure we have rebased on the remote head. @@ -602,43 +610,4 @@ private void storeBookStatus(DataRepository dataRepository, action, status.toString()); } - - @Override - public TwoWaySyncResult syncBook( - Uri uri, VersionedRook current, File fromDB) throws IOException { - String repoRelativePath = uri.getPath().replaceFirst("^/", ""); - boolean merged = true; - if (current != null) { - RevCommit rookCommit = getCommitFromRevisionString(current.getRevision()); - if (BuildConfig.LOG_DEBUG) { - LogUtils.d(TAG, String.format("Syncing file %s, rookCommit: %s", repoRelativePath, rookCommit)); - } - merged = synchronizer.updateAndCommitFileFromRevisionAndMerge( - fromDB, repoRelativePath, - synchronizer.getFileRevision(repoRelativePath, rookCommit), - rookCommit); - - if (merged) { - // Our change was successfully merged. Make an attempt - // to return to the main branch, if we are not on it. - if (!git.getRepository().getBranch().equals(preferences.branchName())) { - synchronizer.attemptReturnToMainBranch(); - } - } - } else { - Log.w(TAG, "Unable to find previous commit, loading from repository."); - } - File writeBackFile = synchronizer.workTreeFile(repoRelativePath); - return new TwoWaySyncResult( - currentVersionedRook(Uri.EMPTY.buildUpon().path(repoRelativePath).build()), merged, - writeBackFile); - } - - public void tryPushIfHeadDiffersFromRemote() { - synchronizer.tryPushIfHeadDiffersFromRemote(); - } - - public String getCurrentBranch() throws IOException { - return git.getRepository().getBranch(); - } } diff --git a/app/src/main/java/com/orgzly/android/repos/TwoWaySyncRepo.kt b/app/src/main/java/com/orgzly/android/repos/TwoWaySyncRepo.kt index d3d7dfa7e..8662bd4fc 100644 --- a/app/src/main/java/com/orgzly/android/repos/TwoWaySyncRepo.kt +++ b/app/src/main/java/com/orgzly/android/repos/TwoWaySyncRepo.kt @@ -5,10 +5,6 @@ import java.io.File import java.io.IOException interface TwoWaySyncRepo { - @Throws(IOException::class) - fun syncBook(uri: Uri, current: VersionedRook?, fromDB: File): TwoWaySyncResult - - fun tryPushIfHeadDiffersFromRemote() fun getUri(): Uri } \ No newline at end of file diff --git a/app/src/main/java/com/orgzly/android/sync/SyncUtils.kt b/app/src/main/java/com/orgzly/android/sync/SyncUtils.kt index a1419dba9..9c0487185 100644 --- a/app/src/main/java/com/orgzly/android/sync/SyncUtils.kt +++ b/app/src/main/java/com/orgzly/android/sync/SyncUtils.kt @@ -4,13 +4,10 @@ import androidx.core.net.toUri import com.orgzly.BuildConfig import com.orgzly.android.BookFormat import com.orgzly.android.BookName -import com.orgzly.android.NotesOrgExporter import com.orgzly.android.data.DataRepository import com.orgzly.android.db.entity.BookAction import com.orgzly.android.db.entity.Repo -import com.orgzly.android.repos.GitRepo import com.orgzly.android.repos.SyncRepo -import com.orgzly.android.repos.TwoWaySyncRepo import com.orgzly.android.repos.VersionedRook import com.orgzly.android.util.LogUtils import java.io.IOException @@ -85,23 +82,6 @@ object SyncUtils { val repositoryPath: String var bookAction: BookAction? = null - // FIXME: This is a pretty nasty hack that completely circumvents the existing code path - if (namesake.rooks.isNotEmpty()) { - val rook = namesake.rooks[0] - if (rook != null && namesake.status !== BookSyncStatus.NO_CHANGE) { - val repo = dataRepository.getRepoInstance( - rook.repoId, rook.repoType, rook.repoUri.toString()) - if (repo is GitRepo) { - if (!handleTwoWaySync(dataRepository, repo as TwoWaySyncRepo, namesake)) { - throw Exception("Merge conflict; saved to temporary branch.") - } - return BookAction.forNow( - BookAction.Type.INFO, - namesake.status.msg(String.format("branch '%s'", repo.currentBranch))) - } - } - } - when (namesake.status!!) { BookSyncStatus.NO_CHANGE -> bookAction = BookAction.forNow(BookAction.Type.INFO, namesake.status.msg()) @@ -177,45 +157,4 @@ object SyncUtils { return bookAction } - - @Throws(IOException::class) - private fun handleTwoWaySync(dataRepository: DataRepository, repo: TwoWaySyncRepo, namesake: BookNamesake): Boolean { - val (book, _, _, currentRook) = namesake.book - val someRook = currentRook ?: namesake.rooks[0] - val newRook: VersionedRook? - var noNewMergeConflicts = true - // If there are only local changes, the GitRepo.syncBook method is overly complicated. - if (namesake.status == BookSyncStatus.BOOK_WITH_LINK_LOCAL_MODIFIED) { - val repoRelativePath = BookName.getRepoRelativePath(repo.getUri(), namesake.book.syncedTo!!.uri) - dataRepository.saveBookToRepo(namesake.book.linkRepo!!, repoRelativePath, namesake.book, BookFormat.ORG) - } else { - val dbFile = dataRepository.getTempBookFile() - try { - NotesOrgExporter(dataRepository).exportBook(book, dbFile) - val (newRook1, merged, loadFile) = - repo.syncBook(someRook.uri, currentRook, dbFile) - noNewMergeConflicts = merged - newRook = newRook1 - // We only need to write it if syncback is needed - if (loadFile != null) { - val repoRelativePath = BookName.getRepoRelativePath(repo.getUri(), newRook.uri) - val bookName = BookName.fromRepoRelativePath(repoRelativePath) - if (BuildConfig.LOG_DEBUG) LogUtils.d(TAG, "Loading from file '$loadFile'") - dataRepository.loadBookFromFile( - bookName.name, - bookName.format, - loadFile, - newRook) - // TODO: db.book().updateIsModified(bookView.book.id, false) - // Instead of: - // dataRepository.updateBookMtime(loadedBook.getBook().getId(), 0); - } - } finally { - /* Delete temporary files. */ - dbFile.delete() - } - dataRepository.updateBookLinkAndSync(book.id, newRook!!) - } - return noNewMergeConflicts - } }