From f5483f419bfc214f93f9d49895afef25df36500f Mon Sep 17 00:00:00 2001 From: Paul Campbell Date: Mon, 25 Jun 2018 20:03:29 +0100 Subject: [PATCH] GitDBBranch returns Result values --- .../java/net/kemitix/gitdb/GitDBBranch.java | 17 +-- .../kemitix/gitdb/impl/GitDBBranchImpl.java | 76 +++++++----- .../net/kemitix/gitdb/impl/GitDBRepo.java | 82 +++++++------ .../net/kemitix/gitdb/impl/KeyRemover.java | 54 ++++---- .../kemitix/gitdb/impl/LocalGitDBImpl.java | 6 +- .../net/kemitix/gitdb/test/GitDBTest.java | 116 ++++++++++++------ 6 files changed, 205 insertions(+), 146 deletions(-) diff --git a/src/main/java/net/kemitix/gitdb/GitDBBranch.java b/src/main/java/net/kemitix/gitdb/GitDBBranch.java index e1fa7cf..fe08fb9 100644 --- a/src/main/java/net/kemitix/gitdb/GitDBBranch.java +++ b/src/main/java/net/kemitix/gitdb/GitDBBranch.java @@ -22,9 +22,8 @@ package net.kemitix.gitdb; import com.github.zafarkhaja.semver.Version; - -import java.io.IOException; -import java.util.Optional; +import net.kemitix.mon.maybe.Maybe; +import net.kemitix.mon.result.Result; /** * API for interacting with a branch in a GirDB. @@ -38,9 +37,8 @@ public interface GitDBBranch { * * @param key the key to lookup * @return an Optional containing the value, if it exists, or empty if not - * @throws IOException if there was an error reading the value */ - Optional get(String key) throws IOException; + Result> get(String key); /** * Put a value into the store for the key. @@ -48,18 +46,16 @@ public interface GitDBBranch { * @param key the key to place the value under * @param value the value (must be Serializable) * @return an updated branch containing the new key/value - * @throws IOException if there was an error writing the key/value */ - GitDBBranch put(String key, String value) throws IOException; + Result put(String key, String value); /** * Removes a key and its value from the store. * * @param key the key to remove * @return an updated branch without the key, or the original if the key was not found - * @throws IOException if there was an error removing the key/value */ - GitDBBranch remove(String key) throws IOException; + Result remove(String key); /** * Returns the GitDB format for the current branch. @@ -67,8 +63,7 @@ public interface GitDBBranch { *

Different branches can have different versions.

* * @return the format as per semantic versioning, i.e. "x.y.z" within an Optional - * @throws IOException error reading version */ - Optional getFormatVersion() throws IOException; + Result> getFormatVersion(); } diff --git a/src/main/java/net/kemitix/gitdb/impl/GitDBBranchImpl.java b/src/main/java/net/kemitix/gitdb/impl/GitDBBranchImpl.java index 097c646..9b90a36 100644 --- a/src/main/java/net/kemitix/gitdb/impl/GitDBBranchImpl.java +++ b/src/main/java/net/kemitix/gitdb/impl/GitDBBranchImpl.java @@ -25,12 +25,13 @@ import com.github.zafarkhaja.semver.Version; import lombok.AccessLevel; import lombok.RequiredArgsConstructor; import net.kemitix.gitdb.GitDBBranch; +import net.kemitix.mon.maybe.Maybe; +import net.kemitix.mon.result.Result; import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.Ref; import org.eclipse.jgit.lib.Repository; import java.io.IOException; -import java.util.Optional; import java.util.function.Function; /** @@ -48,15 +49,6 @@ class GitDBBranchImpl implements GitDBBranch { private final String userEmailAddress; private final String name; - private static GitDBBranch select( - final Ref branchRef, - final GitDBRepo gitDBRepo, - final String userName, - final String userEmailAddress - ) { - return new GitDBBranchImpl(branchRef, gitDBRepo, userName, userEmailAddress, branchRef.getName()); - } - /** * Initialise the creation of new GitDBBranch instances. * @@ -65,7 +57,7 @@ class GitDBBranchImpl implements GitDBBranch { * @param userEmailAddress the user's email address to record against changes * @return a Function for creating a GitDBBranch when supplied with a Ref for a branch */ - static Function init( + static Function> init( final Repository repository, final String userName, final String userEmailAddress @@ -73,38 +65,56 @@ class GitDBBranchImpl implements GitDBBranch { return ref -> select(ref, new GitDBRepo(repository), userName, userEmailAddress); } + private static Result select( + final Ref branchRef, + final GitDBRepo gitDBRepo, + final String userName, + final String userEmailAddress + ) { + return Result.ok(new GitDBBranchImpl(branchRef, gitDBRepo, userName, userEmailAddress, branchRef.getName())); + } + @Override - public Optional get(final String key) throws IOException { + public Result> get(final String key) { return gitDBRepo.readValue(branchRef, KEY_PREFIX + key); } @Override - public GitDBBranch put(final String key, final String value) throws IOException { - final ObjectId newTree = gitDBRepo.writeValue(branchRef, KEY_PREFIX + key, value); - final String message = String.format("Add key [%s] = [%s]", key, value); - final Ref newBranch = gitDBRepo.writeCommit(branchRef, newTree, message, userName, userEmailAddress); - return select(newBranch, gitDBRepo, userName, userEmailAddress); - } - - @Override - public GitDBBranch remove(final String key) throws IOException { - final Optional newTree = gitDBRepo.removeKey(branchRef, KEY_PREFIX + key); - if (newTree.isPresent()) { - final Ref newBranch = - gitDBRepo.writeCommit( - branchRef, newTree.get(), - String.format("Remove Key [%s]", key), - userName, - userEmailAddress); - return select(newBranch, gitDBRepo, userName, userEmailAddress); + public Result put(final String key, final String value) { + try { + final ObjectId newTree = gitDBRepo.writeValue(branchRef, KEY_PREFIX + key, value); + final String message = String.format("Add key [%s] = [%s]", key, value); + final Result newBranch = gitDBRepo.writeCommit(branchRef, newTree, message, userName, userEmailAddress); + return newBranch.flatMap(b -> select(b, gitDBRepo, userName, userEmailAddress)); + } catch (IOException e) { + return Result.error(e); } - return this; } @Override - public Optional getFormatVersion() throws IOException { + public Result remove(final String key) { + return gitDBRepo.removeKey(branchRef, KEY_PREFIX + key).flatMap(treeId -> + writeRemoveKeyCommit(key, treeId) + .map(selectUpdatedBranch()) + .orElse(Result.ok(this))); + } + + private Maybe> writeRemoveKeyCommit(final String key, final Maybe idMaybe) { + return idMaybe.map(objectId -> { + final String message = String.format("Remove Key [%s]", key); + return gitDBRepo.writeCommit(branchRef, objectId, message, userName, userEmailAddress); + }); + } + + private Function, Result> selectUpdatedBranch() { + return refResult -> refResult.flatMap(ref -> + select(ref, gitDBRepo, userName, userEmailAddress)); + } + + @Override + public Result> getFormatVersion() { return gitDBRepo.readValue(branchRef, "GitDB.Version") - .map(Version::valueOf); + .map(version -> version.map(Version::valueOf)); } } diff --git a/src/main/java/net/kemitix/gitdb/impl/GitDBRepo.java b/src/main/java/net/kemitix/gitdb/impl/GitDBRepo.java index 1efbe7c..7af698b 100644 --- a/src/main/java/net/kemitix/gitdb/impl/GitDBRepo.java +++ b/src/main/java/net/kemitix/gitdb/impl/GitDBRepo.java @@ -22,13 +22,14 @@ package net.kemitix.gitdb.impl; import lombok.val; +import net.kemitix.mon.maybe.Maybe; +import net.kemitix.mon.result.Result; import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.Ref; import org.eclipse.jgit.lib.Repository; import java.io.IOException; import java.nio.charset.StandardCharsets; -import java.util.Optional; /** * Wrapper for interacting with the GitDB Repository. @@ -47,7 +48,7 @@ class GitDBRepo { /** * Creates a new instance of this class. * - * @param repository the Git Repository + * @param repository the Git Repository */ GitDBRepo(final Repository repository) { this.repository = repository; @@ -73,47 +74,29 @@ class GitDBRepo { return keyWriter.writeFirst(key, valueId); } - /** - * Insert a commit into the store, returning its unique id. - * - * @param treeId id of the tree - * @param branchRef the branch to commit to - * @param message the message - * @param userName the user name - * @param userEmailAddress the user email address - * @return the id of the commit - * @throws IOException the commit could not be stored - */ - ObjectId insertCommit( - final ObjectId treeId, - final String message, - final String userName, - final String userEmailAddress, - final Ref branchRef - ) throws IOException { - return commitWriter.write(treeId, branchRef, message, userName, userEmailAddress); - } - /** * Reads a value from the branch with the given key. * * @param branchRef the branch to select from * @param key the key to get the value for * @return an Optional containing the value if found, or empty - * @throws IOException if there was an error reading the value */ - Optional readValue( + Result> readValue( final Ref branchRef, final String key - ) throws IOException { - val blob = new GitTreeReader(repository) - .treeFilter(key) - .stream(branchRef) - .findFirst(); - if (blob.isPresent()) { - return Optional.of(blob.get().blobAsString()); + ) { + try { + val blob = new GitTreeReader(repository) + .treeFilter(key) + .stream(branchRef) + .findFirst(); + if (blob.isPresent()) { + return Result.ok(Maybe.just(blob.get().blobAsString())); + } + return Result.ok(Maybe.nothing()); + } catch (IOException e) { + return Result.error(e); } - return Optional.empty(); } /** @@ -143,15 +126,40 @@ class GitDBRepo { * @return the Ref of the updated branch * @throws IOException if there was an error writing the branch */ - Ref writeCommit( + Result writeCommit( final Ref branchRef, final ObjectId tree, final String message, final String userName, final String userEmailAddress + ) { + try { + final ObjectId commitId = insertCommit(tree, message, userName, userEmailAddress, branchRef); + return Result.ok(headWriter.write(branchRef.getName(), commitId)); + } catch (IOException e) { + return Result.error(e); + } + } + + /** + * Insert a commit into the store, returning its unique id. + * + * @param treeId id of the tree + * @param branchRef the branch to commit to + * @param message the message + * @param userName the user name + * @param userEmailAddress the user email address + * @return the id of the commit + * @throws IOException the commit could not be stored + */ + ObjectId insertCommit( + final ObjectId treeId, + final String message, + final String userName, + final String userEmailAddress, + final Ref branchRef ) throws IOException { - final ObjectId commitId = insertCommit(tree, message, userName, userEmailAddress, branchRef); - return headWriter.write(branchRef.getName(), commitId); + return commitWriter.write(treeId, branchRef, message, userName, userEmailAddress); } /** @@ -184,7 +192,7 @@ class GitDBRepo { * empty Optional if there key was not found, the there was no changes made * @throws IOException if there was an error writing the value */ - Optional removeKey(final Ref branchRef, final String key) throws IOException { + Result> removeKey(final Ref branchRef, final String key) { return keyRemover.remove(branchRef, key); } diff --git a/src/main/java/net/kemitix/gitdb/impl/KeyRemover.java b/src/main/java/net/kemitix/gitdb/impl/KeyRemover.java index f8cd7d0..6146f0c 100644 --- a/src/main/java/net/kemitix/gitdb/impl/KeyRemover.java +++ b/src/main/java/net/kemitix/gitdb/impl/KeyRemover.java @@ -22,10 +22,11 @@ package net.kemitix.gitdb.impl; import lombok.RequiredArgsConstructor; +import net.kemitix.mon.maybe.Maybe; +import net.kemitix.mon.result.Result; import org.eclipse.jgit.lib.*; import java.io.IOException; -import java.util.Optional; import java.util.concurrent.atomic.AtomicBoolean; import java.util.function.Consumer; import java.util.function.Predicate; @@ -40,6 +41,28 @@ class KeyRemover { private final Repository repository; + /** + * Remove a key from the repository. + * + * @param branchRef the branch to update + * @param key the key to remove + * @return the id of the updated tree + */ + Result> remove(final Ref branchRef, final String key) { + final TreeFormatter treeFormatter = new TreeFormatter(); + final AtomicBoolean removed = new AtomicBoolean(false); + try { + new GitTreeReader(repository) + .stream(branchRef) + .peek(flagIfFound(key, removed)) + .filter(isNotKey(key)) + .forEach(addToTree(treeFormatter)); + return insertTree(treeFormatter).maybe(oi -> removed.get()); + } catch (IOException e) { + return Result.error(e); + } + } + /** * Sets the boolean to true if the key matches a NamedRevBlob's name. * @@ -75,38 +98,17 @@ class KeyRemover { return item -> treeFormatter.append(item.getName(), item.getRevBlob()); } - /** - * Remove a key from the repository. - * - * @param branchRef the branch to update - * @param key the key to remove - * @return the id of the updated tree - * @throws IOException if there is an error writing the value - */ - Optional remove(final Ref branchRef, final String key) throws IOException { - final TreeFormatter treeFormatter = new TreeFormatter(); - final AtomicBoolean removed = new AtomicBoolean(false); - new GitTreeReader(repository) - .stream(branchRef) - .peek(flagIfFound(key, removed)) - .filter(isNotKey(key)) - .forEach(addToTree(treeFormatter)); - if (removed.get()) { - return Optional.of(insertTree(treeFormatter)); - } - return Optional.empty(); - } - /** * Insert a tree into the repo, returning its id. * * @param treeFormatter the formatter containing the proposed tree's data. * @return the name of the tree object. - * @throws IOException the object could not be stored. */ - private ObjectId insertTree(final TreeFormatter treeFormatter) throws IOException { + private Result insertTree(final TreeFormatter treeFormatter) { try (ObjectInserter inserter = repository.getObjectDatabase().newInserter()) { - return inserter.insert(treeFormatter); + return Result.ok(inserter.insert(treeFormatter)); + } catch (IOException e) { + return Result.error(e); } } } diff --git a/src/main/java/net/kemitix/gitdb/impl/LocalGitDBImpl.java b/src/main/java/net/kemitix/gitdb/impl/LocalGitDBImpl.java index 549e0d6..9164a96 100644 --- a/src/main/java/net/kemitix/gitdb/impl/LocalGitDBImpl.java +++ b/src/main/java/net/kemitix/gitdb/impl/LocalGitDBImpl.java @@ -46,7 +46,7 @@ final class LocalGitDBImpl implements GitDB, LocalGitDB { private final Repository repository; - private final Function branchInit; + private final Function> branchInit; private LocalGitDBImpl( final Repository repository, @@ -113,7 +113,9 @@ final class LocalGitDBImpl implements GitDB, LocalGitDB { @Override public Result> branch(final String name) { try { - return Result.ok(Maybe.maybe(repository.findRef(name)).map(branchInit)); + return Result.invert(Maybe.maybe( + repository.findRef(name)) + .map(branchInit::apply)); } catch (IOException e) { return Result.error(e); } diff --git a/src/test/java/net/kemitix/gitdb/test/GitDBTest.java b/src/test/java/net/kemitix/gitdb/test/GitDBTest.java index 8a703da..23428e8 100644 --- a/src/test/java/net/kemitix/gitdb/test/GitDBTest.java +++ b/src/test/java/net/kemitix/gitdb/test/GitDBTest.java @@ -24,8 +24,8 @@ import java.nio.file.DirectoryNotEmptyException; import java.nio.file.Files; import java.nio.file.NotDirectoryException; import java.nio.file.Path; -import java.util.Optional; import java.util.UUID; +import java.util.function.Consumer; import java.util.function.Supplier; @ExtendWith(MockitoExtension.class) @@ -80,7 +80,7 @@ class GitDBTest implements WithAssertions { final Result gitDBResult = GitDB.initLocal(dir, userName, userEmailAddress); //then gitDBResult.match( - success -> fail("Is a file not a directory"), + failOnSuccess("Is a file not a directory"), error -> assertThat(error) .isInstanceOf(NotDirectoryException.class) .hasMessageContaining(dir.toString()) @@ -91,6 +91,10 @@ class GitDBTest implements WithAssertions { return Files.createTempFile("gitdb", "file"); } + private Consumer failOnSuccess(String message) { + return success -> fail(message); + } + // When initialising a repo in a non-empty dir then an exception is thrown @Test void initRepo_whenNotEmptyDir_thenThrowException() throws IOException { @@ -101,7 +105,7 @@ class GitDBTest implements WithAssertions { final Result gitDBResult = GitDB.initLocal(dir, userName, userEmailAddress); //then gitDBResult.match( - success -> fail("Directory is not empty"), + failOnSuccess("Directory is not empty"), error -> assertThat(error) .isInstanceOf(DirectoryNotEmptyException.class) .hasMessageContaining(dir.toString()) @@ -137,7 +141,7 @@ class GitDBTest implements WithAssertions { final Result gitDBResult = GitDB.openLocal(dir, userName, userEmailAddress); //then gitDBResult.match( - success -> fail("Not a bare repo"), + failOnSuccess("Not a bare repo"), error -> assertThat(error) .isInstanceOf(InvalidRepositoryException.class) .hasMessageContaining(dir.toString()) @@ -154,7 +158,7 @@ class GitDBTest implements WithAssertions { final Result gitDBResult = GitDB.openLocal(dir, userName, userEmailAddress); //then gitDBResult.match( - success -> fail("Directory is a file"), + failOnSuccess("Directory is a file"), error -> assertThat(error) .isInstanceOf(InvalidRepositoryException.class) .hasMessageContaining(dir.toString()) @@ -170,7 +174,7 @@ class GitDBTest implements WithAssertions { final Result gitDBResult = GitDB.openLocal(dir, userName, userEmailAddress); //then gitDBResult.match( - success -> fail("Directory does not exist"), + failOnSuccess("Directory does not exist"), error -> assertThat(error) .isInstanceOf(InvalidRepositoryException.class) .hasMessageContaining(dir.toString()) @@ -186,7 +190,7 @@ class GitDBTest implements WithAssertions { final Result gitDBResult = GitDB.openLocal(dir, userName, userEmailAddress); //then gitDBResult.match( - success -> fail("Not a bare repo"), + failOnSuccess("Not a bare repo"), error -> assertThat(error) .isInstanceOf(InvalidRepositoryException.class) .hasMessageContaining("Invalid GitDB repo") @@ -259,33 +263,50 @@ class GitDBTest implements WithAssertions { //given final GitDBBranch branch = gitDBBranch(); //when - final Optional value = branch.get("unknown"); + final Result> value = branch.get("unknown"); //then - assertThat(value).isEmpty(); + value.match( + success -> assertThat(success.toOptional()).isEmpty(), + failOnError() + ); } private GitDBBranch gitDBBranch() { try { - return gitDB(dirDoesNotExist()) - .flatMap(db -> db.branch("master")) - .orElseThrow() - .orElse(null); + final Result gitDBResult = gitDB(dirDoesNotExist()); + System.out.println("gitDBResult = " + gitDBResult); + final Result> master = gitDBResult.flatMap(db -> { + final Result> maybeResult = db.branch("master"); + System.out.println("maybeResult = " + maybeResult); + return maybeResult; + }); + assert master != null; + System.out.println("master = " + master); + final Maybe gitDBBranchMaybe = master.orElseThrow(); + final GitDBBranch gitDBBranch = gitDBBranchMaybe.orElse(null); + return gitDBBranch; } catch (Throwable throwable) { - throw new RuntimeException("Couldn't create master branch"); + throw new RuntimeException("Couldn't create master branch", throwable); } } + private Consumer failOnError() { + return error -> fail("Not an error"); + } + // When getting the format version it matches expected @Test void getVersionFormat_thenFormatIsSet() throws IOException { //given final GitDBBranch gitDBBranch = gitDBBranch(); - //when - final Optional formatVersion = gitDBBranch.getFormatVersion(); - //then final Version version = new FormatVersion().getVersion(); - assertThat(formatVersion).contains(version); - assertThat(formatVersion.get()).isNotSameAs(version); + //when + final Result> formatVersion = gitDBBranch.getFormatVersion(); + //then + formatVersion.match( + success -> success.peek(v -> assertThat(v).isEqualTo(version).isNotSameAs(version)), + failOnError() + ); } // When putting a key/value pair then a GitDbBranch is returned @@ -294,10 +315,14 @@ class GitDBTest implements WithAssertions { //given final GitDBBranch originalBranch = gitDBBranch(); //when - final GitDBBranch updatedBranch = originalBranch.put("key-name", "value"); + final Result updatedBranch = originalBranch.put("key-name", "value"); //then - assertThat(updatedBranch).isNotNull(); - assertThat(updatedBranch).isNotSameAs(originalBranch); + updatedBranch.match( + success -> assertThat(success).isNotNull().isNotSameAs(originalBranch), + failOnError() + ); + assertThat(updatedBranch).isNotNull() + .isNotSameAs(originalBranch); } // When getting a key that does exist then the value is returned inside an Optional @@ -307,11 +332,14 @@ class GitDBTest implements WithAssertions { final String key = stringSupplier.get(); final String value = stringSupplier.get(); final GitDBBranch originalBranch = gitDBBranch(); - final GitDBBranch updatedBranch = originalBranch.put(key, value); + final Result updatedBranch = originalBranch.put(key, value); //when - final Optional result = updatedBranch.get(key); + final Result> result = updatedBranch.flatMap(b -> b.get(key)); //then - assertThat(result).contains(value); + result.match( + success -> success.map(v -> assertThat(v).contains(value)), + failOnError() + ); } // When removing a key that does not exist then the GitDbBranch is returned @@ -320,9 +348,12 @@ class GitDBTest implements WithAssertions { //given final GitDBBranch gitDBBranch = gitDBBranch(); //when - final GitDBBranch result = gitDBBranch.remove("unknown"); + final Result result = gitDBBranch.remove("unknown"); //then - assertThat(result).isSameAs(gitDBBranch); + result.match( + success -> assertThat(success).isSameAs(gitDBBranch), + failOnError() + ); } // When removing a key that does exist then a GitDbBranch is returned @@ -331,14 +362,17 @@ class GitDBTest implements WithAssertions { //given final String key = stringSupplier.get(); final String value = stringSupplier.get(); - final GitDBBranch originalBranch = gitDBBranchWithKeyValue(key, value); + final Result originalBranch = gitDBBranchWithKeyValue(key, value); //when - final GitDBBranch updatedBranch = originalBranch.remove(key); + final Result updatedBranch = originalBranch.flatMap(b -> b.remove(key)); //then - assertThat(updatedBranch).isNotSameAs(originalBranch); + updatedBranch.match( + success -> assertThat(success).isNotSameAs(originalBranch), + failOnError() + ); } - private GitDBBranch gitDBBranchWithKeyValue(final String key, final String value) throws IOException { + private Result gitDBBranchWithKeyValue(final String key, final String value) throws IOException { return gitDBBranch().put(key, value); } @@ -348,11 +382,15 @@ class GitDBTest implements WithAssertions { //given final String key = stringSupplier.get(); final String value = stringSupplier.get(); - final GitDBBranch originalBranch = gitDBBranchWithKeyValue(key, value); + final Result originalBranch = gitDBBranchWithKeyValue(key, value); //when - final GitDBBranch updatedBranch = originalBranch.remove(key); + final Result updatedBranch = originalBranch.flatMap(b -> b.remove(key)); //then - assertThat(originalBranch.get(key)).contains(value); + originalBranch.flatMap(b -> b.get(key)) + .match( + success -> assertThat(success.toOptional()).contains(value), + failOnError() + ); } // When removing a key that does exist then the updated GitDbBranch can't find it @@ -361,11 +399,15 @@ class GitDBTest implements WithAssertions { //given final String key = stringSupplier.get(); final String value = stringSupplier.get(); - final GitDBBranch originalBranch = gitDBBranchWithKeyValue(key, value); + final Result originalBranch = gitDBBranchWithKeyValue(key, value); //when - final GitDBBranch updatedBranch = originalBranch.remove(key); + final Result updatedBranch = originalBranch.flatMap(b -> b.remove(key)); //then - assertThat(updatedBranch.get(key)).isEmpty(); + updatedBranch.flatMap(b -> b.get(key)) + .match( + success -> assertThat(success.toOptional()).isEmpty(), + failOnError() + ); } }