diff --git a/scm-core/src/main/java/sonia/scm/repository/Tag.java b/scm-core/src/main/java/sonia/scm/repository/Tag.java index 0262888e55..49f81c7516 100644 --- a/scm-core/src/main/java/sonia/scm/repository/Tag.java +++ b/scm-core/src/main/java/sonia/scm/repository/Tag.java @@ -73,7 +73,7 @@ public final class Tag { /** * The date is retrieved in a best-effort fashion. * In certain situations it might not be available. - * In these cases, this method returns null. + * In these cases, this method returns an empty optional. * * @since 2.5.0 */ diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/GitUtil.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/GitUtil.java index 97fc29163a..5426422def 100644 --- a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/GitUtil.java +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/GitUtil.java @@ -390,45 +390,27 @@ public final class GitUtil } /** - * Method description - * - * @param repository - * @param ref - * @return - * @throws IOException * @since 2.5.0 */ - public static Long getTagTime(org.eclipse.jgit.lib.Repository repository, Ref ref) throws IOException { + public static Long getTagTime(org.eclipse.jgit.lib.Repository repository, ObjectId objectId) throws IOException { try (RevWalk walk = new RevWalk(repository)) { - return GitUtil.getTagTime(repository, walk, ref); + return GitUtil.getTagTime(repository, walk, objectId); } } /** - * Method description - * - * @param repository - * @param revWalk - * @param ref - * @return - * @throws IOException * @since 2.5.0 */ public static Long getTagTime(org.eclipse.jgit.lib.Repository repository, - RevWalk revWalk, Ref ref) + RevWalk revWalk, ObjectId objectId) throws IOException { - if (ref == null) { - return null; - } - ObjectId id = ref.getObjectId(); - - if (id != null) { + if (objectId != null) { if (revWalk == null) { revWalk = new RevWalk(repository); } - final RevObject revObject = revWalk.parseAny(id); + final RevObject revObject = revWalk.parseAny(objectId); if (revObject instanceof RevTag) { return ((RevTag) revObject).getTaggerIdent().getWhen().getTime(); } else if (revObject instanceof RevCommit) { diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/api/GitHookTagProvider.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/api/GitHookTagProvider.java index c1a333400b..7889badc30 100644 --- a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/api/GitHookTagProvider.java +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/api/GitHookTagProvider.java @@ -26,18 +26,16 @@ package sonia.scm.repository.api; import com.google.common.base.Strings; import com.google.common.collect.ImmutableList; - -import java.io.IOException; -import java.util.List; - import org.eclipse.jgit.lib.Repository; -import org.eclipse.jgit.revwalk.RevWalk; import org.eclipse.jgit.transport.ReceiveCommand; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import sonia.scm.repository.GitUtil; import sonia.scm.repository.Tag; +import java.io.IOException; +import java.util.List; + /** * Git provider implementation of {@link HookTagProvider}. * @@ -67,20 +65,18 @@ public class GitHookTagProvider implements HookTagProvider { if (Strings.isNullOrEmpty(tag)) { LOG.debug("received ref name {} is not a tag", refName); } else { - Long tagTime = null; try { - tagTime = GitUtil.getTagTime(repository, rc.getRef()); + if (isCreate(rc)) { + createdTagBuilder.add(createTagFromNewId(rc, tag, GitUtil.getTagTime(repository, rc.getNewId()))); + } else if (isDelete(rc)) { + deletedTagBuilder.add(createTagFromOldId(rc, tag, GitUtil.getTagTime(repository, rc.getOldId()))); + } else if (isUpdate(rc)) { + createdTagBuilder.add(createTagFromNewId(rc, tag, GitUtil.getTagTime(repository, rc.getNewId()))); + deletedTagBuilder.add(createTagFromOldId(rc, tag, GitUtil.getTagTime(repository, rc.getOldId()))); + } } catch (IOException e) { LOG.error("Could not read tag time", e); } - if (isCreate(rc)) { - createdTagBuilder.add(createTagFromNewId(rc, tag, tagTime)); - } else if (isDelete(rc)) { - deletedTagBuilder.add(createTagFromOldId(rc, tag, tagTime)); - } else if (isUpdate(rc)) { - createdTagBuilder.add(createTagFromNewId(rc, tag, tagTime)); - deletedTagBuilder.add(createTagFromOldId(rc, tag, tagTime)); - } } } diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitTagsCommand.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitTagsCommand.java index 97b9c91be1..b8c000ed03 100644 --- a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitTagsCommand.java +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitTagsCommand.java @@ -132,7 +132,7 @@ public class GitTagsCommand extends AbstractGitCommand implements TagsCommand { if (revObject != null) { String name = GitUtil.getTagName(ref); - tag = new Tag(name, revObject.getId().name(), GitUtil.getTagTime(repository, revWalk, ref)); + tag = new Tag(name, revObject.getId().name(), GitUtil.getTagTime(repository, revWalk, ref.getObjectId())); } } catch (IOException ex) { diff --git a/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/api/GitHookTagProviderTest.java b/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/api/GitHookTagProviderTest.java index e0a83e1272..ccf21b83c9 100644 --- a/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/api/GitHookTagProviderTest.java +++ b/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/api/GitHookTagProviderTest.java @@ -26,7 +26,6 @@ package sonia.scm.repository.api; import com.google.common.collect.Lists; import org.eclipse.jgit.lib.ObjectId; -import org.eclipse.jgit.lib.Ref; import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.transport.ReceiveCommand; import org.junit.Before; @@ -64,12 +63,6 @@ public class GitHookTagProviderTest { @Mock private Repository repository; - @Mock - private Ref gitRef; - - @Mock - private ObjectId objectId; - private List commands; /** @@ -91,7 +84,7 @@ public class GitHookTagProviderTest { String tagName = "1.0.0"; String ref = "refs/tags/" + tagName; - dummy.when(() -> GitUtil.getTagTime(repository, gitRef)).thenReturn(timestamp); + dummy.when(() -> GitUtil.getTagTime(repository, ObjectId.fromString(revision))).thenReturn(timestamp); dummy.when(() -> GitUtil.getTagName(ref)).thenReturn(tagName); dummy.when(() -> GitUtil.getId(ObjectId.fromString(revision))).thenReturn(revision); @@ -113,7 +106,7 @@ public class GitHookTagProviderTest { String tagName = "1.0.0"; String ref = "refs/tags/" + tagName; - dummy.when(() -> GitUtil.getTagTime(repository, gitRef)).thenReturn(timestamp); + dummy.when(() -> GitUtil.getTagTime(repository, ObjectId.fromString(revision))).thenReturn(timestamp); dummy.when(() -> GitUtil.getTagName(ref)).thenReturn(tagName); dummy.when(() -> GitUtil.getId(ObjectId.fromString(revision))).thenReturn(revision); @@ -137,29 +130,54 @@ public class GitHookTagProviderTest { } /** - * Tests {@link GitHookTagProvider} with update command. + * Tests {@link GitHookTagProvider} with update command pre receive. */ @Test - public void testUpdateTags() { + public void testUpdateTagsPreReceive() { try (MockedStatic dummy = Mockito.mockStatic(GitUtil.class)) { - String newRevision = "b2002b64013e54b78eac251df0672bd5d6a83aa7"; - Long newTimestamp = 1339416344000L; - String newTagName = "1.0.0"; - String newRef = "refs/tags/" + newTagName; - String oldRevision = "e0f2be968b147ff7043684a7715d2fe852553db4"; - String oldTagName = "0.9.0"; + String newRevision = "b2002b64013e54b78eac251df0672bd5d6a83aa7"; - dummy.when(() -> GitUtil.getTagTime(repository, gitRef)).thenReturn(newTimestamp); - dummy.when(() -> GitUtil.getTagName(newRef)).thenReturn(newTagName); + Long timestamp = 1339416344000L; + String tagName = "1.0.0"; + String ref = "refs/tags/" + tagName; + + dummy.when(() -> GitUtil.getTagTime(repository, ObjectId.fromString(oldRevision))).thenReturn(timestamp); + dummy.when(() -> GitUtil.getTagTime(repository, ObjectId.fromString(newRevision))).thenReturn(null); + dummy.when(() -> GitUtil.getTagName(ref)).thenReturn(tagName); + dummy.when(() -> GitUtil.getId(ObjectId.fromString(oldRevision))).thenReturn(oldRevision); dummy.when(() -> GitUtil.getId(ObjectId.fromString(newRevision))).thenReturn(newRevision); + GitHookTagProvider provider = createProvider(ReceiveCommand.Type.UPDATE, ref, newRevision, oldRevision); + + assertTag(tagName, newRevision, null, provider.getCreatedTags()); + assertTag(tagName, oldRevision, timestamp, provider.getDeletedTags()); + } + } + + /** + * Tests {@link GitHookTagProvider} with update command post receive. + */ + @Test + public void testUpdateTagsPostReceive() { + try (MockedStatic dummy = Mockito.mockStatic(GitUtil.class)) { + String oldRevision = "e0f2be968b147ff7043684a7715d2fe852553db4"; + String newRevision = "b2002b64013e54b78eac251df0672bd5d6a83aa7"; + + Long timestamp = 1339416344000L; + String tagName = "1.0.0"; + String ref = "refs/tags/" + tagName; + + dummy.when(() -> GitUtil.getTagTime(repository, ObjectId.fromString(newRevision))).thenReturn(timestamp); + dummy.when(() -> GitUtil.getTagTime(repository, ObjectId.fromString(oldRevision))).thenReturn(null); + dummy.when(() -> GitUtil.getTagName(ref)).thenReturn(tagName); dummy.when(() -> GitUtil.getId(ObjectId.fromString(oldRevision))).thenReturn(oldRevision); + dummy.when(() -> GitUtil.getId(ObjectId.fromString(newRevision))).thenReturn(newRevision); - GitHookTagProvider provider = createProvider(ReceiveCommand.Type.UPDATE, newRef, newRevision, oldRevision); + GitHookTagProvider provider = createProvider(ReceiveCommand.Type.UPDATE, ref, newRevision, oldRevision); - assertTag(newTagName, newRevision, newTimestamp, provider.getCreatedTags()); - assertTag(oldTagName, oldRevision, null, provider.getDeletedTags()); + assertTag(tagName, newRevision, timestamp, provider.getCreatedTags()); + assertTag(tagName, oldRevision, null, provider.getDeletedTags()); } } @@ -178,8 +196,6 @@ public class GitHookTagProviderTest { when(command.getOldId()).thenReturn(ObjectId.fromString(oldId)); when(command.getType()).thenReturn(type); when(command.getRefName()).thenReturn(ref); - when(command.getRef()).thenReturn(gitRef); - when(gitRef.getObjectId()).thenReturn(objectId); return new GitHookTagProvider(commands, repository); } diff --git a/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/client/spi/GitTagCommand.java b/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/client/spi/GitTagCommand.java index 19b93d0775..28664c463d 100644 --- a/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/client/spi/GitTagCommand.java +++ b/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/client/spi/GitTagCommand.java @@ -89,7 +89,7 @@ public class GitTagCommand implements TagCommand { walk = new RevWalk(git.getRepository()); revObject = walk.parseAny(id); - tagTime = GitUtil.getTagTime(git.getRepository(), walk, GitUtil.getRefForCommit(git.getRepository(), id)); + tagTime = GitUtil.getTagTime(git.getRepository(), walk, id); } finally {