From c2188a840bc4153ae92112b04b2e06a90d3944aa Mon Sep 17 00:00:00 2001 From: Paul Martin Date: Wed, 27 Apr 2016 23:58:06 +0100 Subject: [PATCH] Ticket Reference handling #1048 + Supports referencing: + Tickets from other tickets via comments + Tickets from commits on any branch + Common TicketLink class used for both commits and tickets + TicketLink is temporary and persisted to ticket as a Reference + Support deletion of ticket references + Rebasing patchsets/branches will generate new references + Deleting old patchsets/branches will remove the relevant references + Substantial testing of use cases + With and without patchsets, deleting, amending + BranchTicketService used during testing to allow end-to-end ref testing + Relocated common git helper functions to JGitUtils --- src/main/distrib/data/defaults.properties | 7 + .../com/gitblit/git/GitblitReceivePack.java | 243 +++++ .../com/gitblit/git/PatchsetReceivePack.java | 333 ++++--- .../java/com/gitblit/models/TicketModel.java | 218 +++- .../com/gitblit/tickets/ITicketService.java | 103 +- .../com/gitblit/tickets/TicketNotifier.java | 13 + .../java/com/gitblit/utils/JGitUtils.java | 171 +++- .../gitblit/wicket/GitBlitWebApp.properties | 4 +- .../com/gitblit/wicket/pages/TicketPage.html | 2 +- .../com/gitblit/wicket/pages/TicketPage.java | 43 +- src/main/resources/gitblit.css | 15 + src/test/config/test-gitblit.properties | 2 + .../java/com/gitblit/tests/GitBlitSuite.java | 2 +- .../gitblit/tests/TicketReferenceTest.java | 939 ++++++++++++++++++ 14 files changed, 1894 insertions(+), 201 deletions(-) create mode 100644 src/test/java/com/gitblit/tests/TicketReferenceTest.java diff --git a/src/main/distrib/data/defaults.properties b/src/main/distrib/data/defaults.properties index 403b7417..0c7d6cd4 100644 --- a/src/main/distrib/data/defaults.properties +++ b/src/main/distrib/data/defaults.properties @@ -574,6 +574,13 @@ tickets.requireApproval = false # SINCE 1.5.0 tickets.closeOnPushCommitMessageRegex = (?:fixes|closes)[\\s-]+#?(\\d+) +# The case-insensitive regular expression used to identify and link tickets on +# push to the commits based on commit message. In the case of a patchset +# self references are ignored +# +# SINCE 1.8.0 +tickets.linkOnPushCommitMessageRegex = (?:ref|task|issue|bug)?[\\s-]*#(\\d+) + # Specify the location of the Lucene Ticket index # # SINCE 1.4.0 diff --git a/src/main/java/com/gitblit/git/GitblitReceivePack.java b/src/main/java/com/gitblit/git/GitblitReceivePack.java index 34bbea27..f271f6f1 100644 --- a/src/main/java/com/gitblit/git/GitblitReceivePack.java +++ b/src/main/java/com/gitblit/git/GitblitReceivePack.java @@ -22,18 +22,28 @@ import groovy.util.GroovyScriptEngine; import java.io.File; import java.io.IOException; import java.text.MessageFormat; +import java.util.ArrayList; import java.util.Collection; +import java.util.LinkedHashMap; import java.util.LinkedHashSet; import java.util.List; +import java.util.Map; import java.util.Set; +import java.util.SortedMap; +import java.util.TreeMap; import java.util.concurrent.TimeUnit; +import org.eclipse.jgit.lib.AnyObjectId; import org.eclipse.jgit.lib.BatchRefUpdate; import org.eclipse.jgit.lib.NullProgressMonitor; +import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.PersonIdent; import org.eclipse.jgit.lib.ProgressMonitor; +import org.eclipse.jgit.lib.Ref; +import org.eclipse.jgit.lib.RefUpdate; import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.revwalk.RevCommit; +import org.eclipse.jgit.revwalk.RevWalk; import org.eclipse.jgit.transport.PostReceiveHook; import org.eclipse.jgit.transport.PreReceiveHook; import org.eclipse.jgit.transport.ReceiveCommand; @@ -50,14 +60,24 @@ import com.gitblit.client.Translation; import com.gitblit.extensions.ReceiveHook; import com.gitblit.manager.IGitblit; import com.gitblit.models.RepositoryModel; +import com.gitblit.models.TicketModel; import com.gitblit.models.UserModel; +import com.gitblit.models.TicketModel.Change; +import com.gitblit.models.TicketModel.Field; +import com.gitblit.models.TicketModel.Patchset; +import com.gitblit.models.TicketModel.Status; +import com.gitblit.models.TicketModel.TicketAction; +import com.gitblit.models.TicketModel.TicketLink; import com.gitblit.tickets.BranchTicketService; +import com.gitblit.tickets.ITicketService; +import com.gitblit.tickets.TicketNotifier; import com.gitblit.utils.ArrayUtils; import com.gitblit.utils.ClientLogger; import com.gitblit.utils.CommitCache; import com.gitblit.utils.JGitUtils; import com.gitblit.utils.RefLogUtils; import com.gitblit.utils.StringUtils; +import com.google.common.collect.Lists; /** @@ -92,6 +112,11 @@ public class GitblitReceivePack extends ReceivePack implements PreReceiveHook, P protected final IStoredSettings settings; protected final IGitblit gitblit; + + protected final ITicketService ticketService; + + protected final TicketNotifier ticketNotifier; + public GitblitReceivePack( IGitblit gitblit, @@ -114,6 +139,14 @@ public class GitblitReceivePack extends ReceivePack implements PreReceiveHook, P } catch (IOException e) { } + if (gitblit.getTicketService().isAcceptingTicketUpdates(repository)) { + this.ticketService = gitblit.getTicketService(); + this.ticketNotifier = this.ticketService.createNotifier(); + } else { + this.ticketService = null; + this.ticketNotifier = null; + } + // set advanced ref permissions setAllowCreates(user.canCreateRef(repository)); setAllowDeletes(user.canDeleteRef(repository)); @@ -500,6 +533,104 @@ public class GitblitReceivePack extends ReceivePack implements PreReceiveHook, P } } } + + // + // if there are ref update receive commands that were + // successfully processed and there is an active ticket service for the repository + // then process any referenced tickets + // + if (ticketService != null) { + List allUpdates = ReceiveCommand.filter(batch.getCommands(), Result.OK); + if (!allUpdates.isEmpty()) { + int ticketsProcessed = 0; + for (ReceiveCommand cmd : allUpdates) { + switch (cmd.getType()) { + case CREATE: + case UPDATE: + if (cmd.getRefName().startsWith(Constants.R_HEADS)) { + Collection tickets = processReferencedTickets(cmd); + ticketsProcessed += tickets.size(); + for (TicketModel ticket : tickets) { + ticketNotifier.queueMailing(ticket); + } + } + break; + + case UPDATE_NONFASTFORWARD: + if (cmd.getRefName().startsWith(Constants.R_HEADS)) { + String base = JGitUtils.getMergeBase(getRepository(), cmd.getOldId(), cmd.getNewId()); + List deletedRefs = JGitUtils.identifyTicketsBetweenCommits(getRepository(), settings, base, cmd.getOldId().name()); + for (TicketLink link : deletedRefs) { + link.isDelete = true; + } + Change deletion = new Change(user.username); + deletion.pendingLinks = deletedRefs; + ticketService.updateTicket(repository, 0, deletion); + + Collection tickets = processReferencedTickets(cmd); + ticketsProcessed += tickets.size(); + for (TicketModel ticket : tickets) { + ticketNotifier.queueMailing(ticket); + } + } + break; + case DELETE: + //Identify if the branch has been merged + SortedMap bases = new TreeMap(); + try { + ObjectId dObj = cmd.getOldId(); + Collection tips = getRepository().getRefDatabase().getRefs(Constants.R_HEADS).values(); + for (Ref ref : tips) { + ObjectId iObj = ref.getObjectId(); + String mergeBase = JGitUtils.getMergeBase(getRepository(), dObj, iObj); + if (mergeBase != null) { + int d = JGitUtils.countCommits(getRepository(), getRevWalk(), mergeBase, dObj.name()); + bases.put(d, mergeBase); + //All commits have been merged into some other branch + if (d == 0) { + break; + } + } + } + + if (bases.isEmpty()) { + //TODO: Handle orphan branch case + } else { + if (bases.firstKey() > 0) { + //Delete references from the remaining commits that haven't been merged + String mergeBase = bases.get(bases.firstKey()); + List deletedRefs = JGitUtils.identifyTicketsBetweenCommits(getRepository(), + settings, mergeBase, dObj.name()); + + for (TicketLink link : deletedRefs) { + link.isDelete = true; + } + Change deletion = new Change(user.username); + deletion.pendingLinks = deletedRefs; + ticketService.updateTicket(repository, 0, deletion); + } + } + + } catch (IOException e) { + LOGGER.error(null, e); + } + break; + + default: + break; + } + } + + if (ticketsProcessed == 1) { + sendInfo("1 ticket updated"); + } else if (ticketsProcessed > 1) { + sendInfo("{0} tickets updated", ticketsProcessed); + } + } + + // reset the ticket caches for the repository + ticketService.resetCaches(repository); + } } protected void setGitblitUrl(String url) { @@ -616,4 +747,116 @@ public class GitblitReceivePack extends ReceivePack implements PreReceiveHook, P public UserModel getUserModel() { return user; } + + /** + * Automatically closes open tickets and adds references to tickets if made in the commit message. + * + * @param cmd + */ + private Collection processReferencedTickets(ReceiveCommand cmd) { + Map changedTickets = new LinkedHashMap(); + + final RevWalk rw = getRevWalk(); + try { + rw.reset(); + rw.markStart(rw.parseCommit(cmd.getNewId())); + if (!ObjectId.zeroId().equals(cmd.getOldId())) { + rw.markUninteresting(rw.parseCommit(cmd.getOldId())); + } + + RevCommit c; + while ((c = rw.next()) != null) { + rw.parseBody(c); + List ticketLinks = JGitUtils.identifyTicketsFromCommitMessage(getRepository(), settings, c); + if (ticketLinks == null) { + continue; + } + + for (TicketLink link : ticketLinks) { + + TicketModel ticket = ticketService.getTicket(repository, link.targetTicketId); + if (ticket == null) { + continue; + } + + Change change = null; + String commitSha = c.getName(); + String branchName = Repository.shortenRefName(cmd.getRefName()); + + switch (link.action) { + case Commit: { + //A commit can reference a ticket in any branch even if the ticket is closed. + //This allows developers to identify and communicate related issues + change = new Change(user.username); + change.referenceCommit(commitSha); + } break; + + case Close: { + // As this isn't a patchset theres no merging taking place when closing a ticket + if (ticket.isClosed()) { + continue; + } + + change = new Change(user.username); + change.setField(Field.status, Status.Fixed); + + if (StringUtils.isEmpty(ticket.responsible)) { + // unassigned tickets are assigned to the closer + change.setField(Field.responsible, user.username); + } + } + + default: { + //No action + } break; + } + + if (change != null) { + ticket = ticketService.updateTicket(repository, ticket.number, change); + } + + if (ticket != null) { + sendInfo(""); + sendHeader("#{0,number,0}: {1}", ticket.number, StringUtils.trimString(ticket.title, Constants.LEN_SHORTLOG)); + + switch (link.action) { + case Commit: { + sendInfo("referenced by push of {0} to {1}", commitSha, branchName); + changedTickets.put(ticket.number, ticket); + } break; + + case Close: { + sendInfo("closed by push of {0} to {1}", commitSha, branchName); + changedTickets.put(ticket.number, ticket); + } break; + + default: { } + } + + sendInfo(ticketService.getTicketUrl(ticket)); + sendInfo(""); + } else { + switch (link.action) { + case Commit: { + sendError("FAILED to reference ticket {0} by push of {1}", link.targetTicketId, commitSha); + } break; + + case Close: { + sendError("FAILED to close ticket {0} by push of {1}", link.targetTicketId, commitSha); + } break; + + default: { } + } + } + } + } + + } catch (IOException e) { + LOGGER.error("Can't scan for changes to reference or close", e); + } finally { + rw.reset(); + } + + return changedTickets.values(); + } } diff --git a/src/main/java/com/gitblit/git/PatchsetReceivePack.java b/src/main/java/com/gitblit/git/PatchsetReceivePack.java index ef0b409b..33fa4705 100644 --- a/src/main/java/com/gitblit/git/PatchsetReceivePack.java +++ b/src/main/java/com/gitblit/git/PatchsetReceivePack.java @@ -30,7 +30,6 @@ import java.util.concurrent.TimeUnit; import java.util.regex.Matcher; import java.util.regex.Pattern; -import org.eclipse.jgit.lib.AnyObjectId; import org.eclipse.jgit.lib.BatchRefUpdate; import org.eclipse.jgit.lib.NullProgressMonitor; import org.eclipse.jgit.lib.ObjectId; @@ -60,6 +59,8 @@ import com.gitblit.models.TicketModel.Field; import com.gitblit.models.TicketModel.Patchset; import com.gitblit.models.TicketModel.PatchsetType; import com.gitblit.models.TicketModel.Status; +import com.gitblit.models.TicketModel.TicketAction; +import com.gitblit.models.TicketModel.TicketLink; import com.gitblit.models.UserModel; import com.gitblit.tickets.BranchTicketService; import com.gitblit.tickets.ITicketService; @@ -485,9 +486,27 @@ public class PatchsetReceivePack extends GitblitReceivePack { switch (cmd.getType()) { case CREATE: case UPDATE: + if (cmd.getRefName().startsWith(Constants.R_HEADS)) { + Collection tickets = processReferencedTickets(cmd); + ticketsProcessed += tickets.size(); + for (TicketModel ticket : tickets) { + ticketNotifier.queueMailing(ticket); + } + } + break; + case UPDATE_NONFASTFORWARD: if (cmd.getRefName().startsWith(Constants.R_HEADS)) { - Collection tickets = processMergedTickets(cmd); + String base = JGitUtils.getMergeBase(getRepository(), cmd.getOldId(), cmd.getNewId()); + List deletedRefs = JGitUtils.identifyTicketsBetweenCommits(getRepository(), settings, base, cmd.getOldId().name()); + for (TicketLink link : deletedRefs) { + link.isDelete = true; + } + Change deletion = new Change(user.username); + deletion.pendingLinks = deletedRefs; + ticketService.updateTicket(repository, 0, deletion); + + Collection tickets = processReferencedTickets(cmd); ticketsProcessed += tickets.size(); for (TicketModel ticket : tickets) { ticketNotifier.queueMailing(ticket); @@ -604,15 +623,17 @@ public class PatchsetReceivePack extends GitblitReceivePack { return null; } } - + // check to see if this commit is already linked to a ticket - long id = identifyTicket(tipCommit, false); - if (id > 0) { - sendError("{0} has already been pushed to ticket {1,number,0}.", shortTipId, id); + if (ticket != null && + JGitUtils.getTicketNumberFromCommitBranch(getRepository(), tipCommit) == ticket.number) { + sendError("{0} has already been pushed to ticket {1,number,0}.", shortTipId, ticket.number); sendRejection(cmd, "everything up-to-date"); return null; } - + + List ticketLinks = JGitUtils.identifyTicketsFromCommitMessage(getRepository(), settings, tipCommit); + PatchsetCommand psCmd; if (ticket == null) { /* @@ -802,6 +823,10 @@ public class PatchsetReceivePack extends GitblitReceivePack { } break; } + + Change change = psCmd.getChange(); + change.pendingLinks = ticketLinks; + return psCmd; } @@ -890,11 +915,11 @@ public class PatchsetReceivePack extends GitblitReceivePack { /** * Automatically closes open tickets that have been merged to their integration - * branch by a client. + * branch by a client and adds references to tickets if made in the commit message. * * @param cmd */ - private Collection processMergedTickets(ReceiveCommand cmd) { + private Collection processReferencedTickets(ReceiveCommand cmd) { Map mergedTickets = new LinkedHashMap(); final RevWalk rw = getRevWalk(); try { @@ -907,105 +932,151 @@ public class PatchsetReceivePack extends GitblitReceivePack { RevCommit c; while ((c = rw.next()) != null) { rw.parseBody(c); - long ticketNumber = identifyTicket(c, true); - if (ticketNumber == 0L || mergedTickets.containsKey(ticketNumber)) { - continue; - } - - TicketModel ticket = ticketService.getTicket(repository, ticketNumber); - if (ticket == null) { - continue; - } - String integrationBranch; - if (StringUtils.isEmpty(ticket.mergeTo)) { - // unspecified integration branch - integrationBranch = null; - } else { - // specified integration branch - integrationBranch = Constants.R_HEADS + ticket.mergeTo; - } - - // ticket must be open and, if specified, the ref must match the integration branch - if (ticket.isClosed() || (integrationBranch != null && !integrationBranch.equals(cmd.getRefName()))) { + List ticketLinks = JGitUtils.identifyTicketsFromCommitMessage(getRepository(), settings, c); + if (ticketLinks == null) { continue; } - String baseRef = PatchsetCommand.getBasePatchsetBranch(ticket.number); - boolean knownPatchset = false; - Set refs = getRepository().getAllRefsByPeeledObjectId().get(c.getId()); - if (refs != null) { - for (Ref ref : refs) { - if (ref.getName().startsWith(baseRef)) { - knownPatchset = true; - break; - } - } - } - - String mergeSha = c.getName(); - String mergeTo = Repository.shortenRefName(cmd.getRefName()); - Change change; - Patchset patchset; - if (knownPatchset) { - // identify merged patchset by the patchset tip - patchset = null; - for (Patchset ps : ticket.getPatchsets()) { - if (ps.tip.equals(mergeSha)) { - patchset = ps; - break; - } + for (TicketLink link : ticketLinks) { + + if (mergedTickets.containsKey(link.targetTicketId)) { + continue; } - - if (patchset == null) { - // should not happen - unless ticket has been hacked - sendError("Failed to find the patchset for {0} in ticket {1,number,0}?!", - mergeSha, ticket.number); + + TicketModel ticket = ticketService.getTicket(repository, link.targetTicketId); + if (ticket == null) { continue; } + String integrationBranch; + if (StringUtils.isEmpty(ticket.mergeTo)) { + // unspecified integration branch + integrationBranch = null; + } else { + // specified integration branch + integrationBranch = Constants.R_HEADS + ticket.mergeTo; + } + + Change change; + Patchset patchset = null; + String mergeSha = c.getName(); + String mergeTo = Repository.shortenRefName(cmd.getRefName()); + + if (link.action == TicketAction.Commit) { + //A commit can reference a ticket in any branch even if the ticket is closed. + //This allows developers to identify and communicate related issues + change = new Change(user.username); + change.referenceCommit(mergeSha); + } else { + // ticket must be open and, if specified, the ref must match the integration branch + if (ticket.isClosed() || (integrationBranch != null && !integrationBranch.equals(cmd.getRefName()))) { + continue; + } + + String baseRef = PatchsetCommand.getBasePatchsetBranch(ticket.number); + boolean knownPatchset = false; + Set refs = getRepository().getAllRefsByPeeledObjectId().get(c.getId()); + if (refs != null) { + for (Ref ref : refs) { + if (ref.getName().startsWith(baseRef)) { + knownPatchset = true; + break; + } + } + } + + if (knownPatchset) { + // identify merged patchset by the patchset tip + for (Patchset ps : ticket.getPatchsets()) { + if (ps.tip.equals(mergeSha)) { + patchset = ps; + break; + } + } + + if (patchset == null) { + // should not happen - unless ticket has been hacked + sendError("Failed to find the patchset for {0} in ticket {1,number,0}?!", + mergeSha, ticket.number); + continue; + } + + // create a new change + change = new Change(user.username); + } else { + // new patchset pushed by user + String base = cmd.getOldId().getName(); + patchset = newPatchset(ticket, base, mergeSha); + PatchsetCommand psCmd = new PatchsetCommand(user.username, patchset); + psCmd.updateTicket(c, mergeTo, ticket, null); + + // create a ticket patchset ref + updateRef(psCmd.getPatchsetBranch(), c.getId(), patchset.type); + RefUpdate ru = updateRef(psCmd.getTicketBranch(), c.getId(), patchset.type); + updateReflog(ru); + + // create a change from the patchset command + change = psCmd.getChange(); + } + + // set the common change data about the merge + change.setField(Field.status, Status.Merged); + change.setField(Field.mergeSha, mergeSha); + change.setField(Field.mergeTo, mergeTo); + + if (StringUtils.isEmpty(ticket.responsible)) { + // unassigned tickets are assigned to the closer + change.setField(Field.responsible, user.username); + } + } + + ticket = ticketService.updateTicket(repository, ticket.number, change); + + if (ticket != null) { + sendInfo(""); + sendHeader("#{0,number,0}: {1}", ticket.number, StringUtils.trimString(ticket.title, Constants.LEN_SHORTLOG)); + + switch (link.action) { + case Commit: { + sendInfo("referenced by push of {0} to {1}", c.getName(), mergeTo); + } + break; - // create a new change - change = new Change(user.username); - } else { - // new patchset pushed by user - String base = cmd.getOldId().getName(); - patchset = newPatchset(ticket, base, mergeSha); - PatchsetCommand psCmd = new PatchsetCommand(user.username, patchset); - psCmd.updateTicket(c, mergeTo, ticket, null); - - // create a ticket patchset ref - updateRef(psCmd.getPatchsetBranch(), c.getId(), patchset.type); - RefUpdate ru = updateRef(psCmd.getTicketBranch(), c.getId(), patchset.type); - updateReflog(ru); - - // create a change from the patchset command - change = psCmd.getChange(); - } + case Close: { + sendInfo("closed by push of {0} to {1}", patchset, mergeTo); + mergedTickets.put(ticket.number, ticket); + } + break; - // set the common change data about the merge - change.setField(Field.status, Status.Merged); - change.setField(Field.mergeSha, mergeSha); - change.setField(Field.mergeTo, mergeTo); + default: { + + } + } - if (StringUtils.isEmpty(ticket.responsible)) { - // unassigned tickets are assigned to the closer - change.setField(Field.responsible, user.username); - } + sendInfo(ticketService.getTicketUrl(ticket)); + sendInfo(""); - ticket = ticketService.updateTicket(repository, ticket.number, change); - if (ticket != null) { - sendInfo(""); - sendHeader("#{0,number,0}: {1}", ticket.number, StringUtils.trimString(ticket.title, Constants.LEN_SHORTLOG)); - sendInfo("closed by push of {0} to {1}", patchset, mergeTo); - sendInfo(ticketService.getTicketUrl(ticket)); - sendInfo(""); - mergedTickets.put(ticket.number, ticket); - } else { - String shortid = mergeSha.substring(0, settings.getInteger(Keys.web.shortCommitIdLength, 6)); - sendError("FAILED to close ticket {0,number,0} by push of {1}", ticketNumber, shortid); + } else { + String shortid = mergeSha.substring(0, settings.getInteger(Keys.web.shortCommitIdLength, 6)); + + switch (link.action) { + case Commit: { + sendError("FAILED to reference ticket {0,number,0} by push of {1}", link.targetTicketId, shortid); + } + break; + case Close: { + sendError("FAILED to close ticket {0,number,0} by push of {1}", link.targetTicketId, shortid); + } break; + + default: { + + } + } + } } } + } catch (IOException e) { - LOGGER.error("Can't scan for changes to close", e); + LOGGER.error("Can't scan for changes to reference or close", e); } finally { rw.reset(); } @@ -1013,75 +1084,9 @@ public class PatchsetReceivePack extends GitblitReceivePack { return mergedTickets.values(); } - /** - * Try to identify a ticket id from the commit. - * - * @param commit - * @param parseMessage - * @return a ticket id or 0 - */ - private long identifyTicket(RevCommit commit, boolean parseMessage) { - // try lookup by change ref - Map> map = getRepository().getAllRefsByPeeledObjectId(); - Set refs = map.get(commit.getId()); - if (!ArrayUtils.isEmpty(refs)) { - for (Ref ref : refs) { - long number = PatchsetCommand.getTicketNumber(ref.getName()); - if (number > 0) { - return number; - } - } - } - - if (parseMessage) { - // parse commit message looking for fixes/closes #n - String dx = "(?:fixes|closes)[\\s-]+#?(\\d+)"; - String x = settings.getString(Keys.tickets.closeOnPushCommitMessageRegex, dx); - if (StringUtils.isEmpty(x)) { - x = dx; - } - try { - Pattern p = Pattern.compile(x, Pattern.CASE_INSENSITIVE); - Matcher m = p.matcher(commit.getFullMessage()); - while (m.find()) { - String val = m.group(1); - return Long.parseLong(val); - } - } catch (Exception e) { - LOGGER.error(String.format("Failed to parse \"%s\" in commit %s", x, commit.getName()), e); - } - } - return 0L; - } + - private int countCommits(String baseId, String tipId) { - int count = 0; - RevWalk walk = getRevWalk(); - walk.reset(); - walk.sort(RevSort.TOPO); - walk.sort(RevSort.REVERSE, true); - try { - RevCommit tip = walk.parseCommit(getRepository().resolve(tipId)); - RevCommit base = walk.parseCommit(getRepository().resolve(baseId)); - walk.markStart(tip); - walk.markUninteresting(base); - for (;;) { - RevCommit c = walk.next(); - if (c == null) { - break; - } - count++; - } - } catch (IOException e) { - // Should never happen, the core receive process would have - // identified the missing object earlier before we got control. - LOGGER.error("failed to get commit count", e); - return 0; - } finally { - walk.close(); - } - return count; - } + /** * Creates a new patchset with metadata. @@ -1091,7 +1096,7 @@ public class PatchsetReceivePack extends GitblitReceivePack { * @param tip */ private Patchset newPatchset(TicketModel ticket, String mergeBase, String tip) { - int totalCommits = countCommits(mergeBase, tip); + int totalCommits = JGitUtils.countCommits(getRepository(), getRevWalk(), mergeBase, tip); Patchset newPatchset = new Patchset(); newPatchset.tip = tip; diff --git a/src/main/java/com/gitblit/models/TicketModel.java b/src/main/java/com/gitblit/models/TicketModel.java index 7495448f..d5345891 100644 --- a/src/main/java/com/gitblit/models/TicketModel.java +++ b/src/main/java/com/gitblit/models/TicketModel.java @@ -107,6 +107,7 @@ public class TicketModel implements Serializable, Comparable { TicketModel ticket; List effectiveChanges = new ArrayList(); Map comments = new HashMap(); + Map references = new HashMap(); Map latestRevisions = new HashMap(); int latestPatchsetNumber = -1; @@ -159,6 +160,18 @@ public class TicketModel implements Serializable, Comparable { effectiveChanges.add(change); } + } else if (change.reference != null){ + if (references.containsKey(change.reference.toString())) { + Change original = references.get(change.reference.toString()); + Change clone = copy(original); + clone.reference.deleted = change.reference.deleted; + int idx = effectiveChanges.indexOf(original); + effectiveChanges.remove(original); + effectiveChanges.add(idx, clone); + } else { + effectiveChanges.add(change); + references.put(change.reference.toString(), change); + } } else { effectiveChanges.add(change); } @@ -167,10 +180,16 @@ public class TicketModel implements Serializable, Comparable { // effective ticket ticket = new TicketModel(); for (Change change : effectiveChanges) { + //Ensure deleted items are not included if (!change.hasComment()) { - // ensure we do not include a deleted comment change.comment = null; } + if (!change.hasReference()) { + change.reference = null; + } + if (!change.hasPatchset()) { + change.patchset = null; + } ticket.applyChange(change); } return ticket; @@ -354,6 +373,15 @@ public class TicketModel implements Serializable, Comparable { return false; } + public boolean hasReferences() { + for (Change change : changes) { + if (change.hasReference()) { + return true; + } + } + return false; + } + public List getAttachments() { List list = new ArrayList(); for (Change change : changes) { @@ -364,6 +392,16 @@ public class TicketModel implements Serializable, Comparable { return list; } + public List getReferences() { + List list = new ArrayList(); + for (Change change : changes) { + if (change.hasReference()) { + list.add(change.reference); + } + } + return list; + } + public List getPatchsets() { List list = new ArrayList(); for (Change change : changes) { @@ -573,8 +611,12 @@ public class TicketModel implements Serializable, Comparable { } } - // add the change to the ticket - changes.add(change); + // add real changes to the ticket and ensure deleted changes are removed + if (change.isEmptyChange()) { + changes.remove(change); + } else { + changes.add(change); + } } protected String toString(Object value) { @@ -645,6 +687,8 @@ public class TicketModel implements Serializable, Comparable { public Comment comment; + public Reference reference; + public Map fields; public Set attachments; @@ -655,6 +699,10 @@ public class TicketModel implements Serializable, Comparable { private transient String id; + //Once links have been made they become a reference on the target ticket + //The ticket service handles promoting links to references + public transient List pendingLinks; + public Change(String author) { this(author, new Date()); } @@ -678,7 +726,7 @@ public class TicketModel implements Serializable, Comparable { } public boolean hasPatchset() { - return patchset != null; + return patchset != null && !patchset.isDeleted(); } public boolean hasReview() { @@ -688,11 +736,42 @@ public class TicketModel implements Serializable, Comparable { public boolean hasComment() { return comment != null && !comment.isDeleted() && comment.text != null; } + + public boolean hasReference() { + return reference != null && !reference.isDeleted(); + } + + public boolean hasPendingLinks() { + return pendingLinks != null && pendingLinks.size() > 0; + } public Comment comment(String text) { comment = new Comment(text); comment.id = TicketModel.getSHA1(date.toString() + author + text); + // parse comment looking for ref #n + //TODO: Ideally set via settings + String x = "(?:ref|task|issue|bug)?[\\s-]*#(\\d+)"; + + try { + Pattern p = Pattern.compile(x, Pattern.CASE_INSENSITIVE); + Matcher m = p.matcher(text); + while (m.find()) { + String val = m.group(1); + long targetTicketId = Long.parseLong(val); + + if (targetTicketId > 0) { + if (pendingLinks == null) { + pendingLinks = new ArrayList(); + } + + pendingLinks.add(new TicketLink(targetTicketId, TicketAction.Comment)); + } + } + } catch (Exception e) { + // ignore + } + try { Pattern mentions = Pattern.compile("\\s@([A-Za-z0-9-_]+)"); Matcher m = mentions.matcher(text); @@ -706,6 +785,16 @@ public class TicketModel implements Serializable, Comparable { return comment; } + public Reference referenceCommit(String commitHash) { + reference = new Reference(commitHash); + return reference; + } + + public Reference referenceTicket(long ticketId, String changeHash) { + reference = new Reference(ticketId, changeHash); + return reference; + } + public Review review(Patchset patchset, Score score, boolean addReviewer) { if (addReviewer) { plusList(Field.reviewers, author); @@ -876,6 +965,17 @@ public class TicketModel implements Serializable, Comparable { } return false; } + + /* + * Identify if this is an empty change. i.e. only an author and date is defined. + * This can occur when items have been deleted + * @returns true if the change is empty + */ + private boolean isEmptyChange() { + return ((comment == null) && (reference == null) && + (fields == null) && (attachments == null) && + (patchset == null) && (review == null)); + } @Override public String toString() { @@ -885,6 +985,8 @@ public class TicketModel implements Serializable, Comparable { sb.append(" commented on by "); } else if (hasPatchset()) { sb.append(MessageFormat.format(" {0} uploaded by ", patchset)); + } else if (hasReference()) { + sb.append(MessageFormat.format(" referenced in {0} by ", reference)); } else { sb.append(" changed by "); } @@ -1145,6 +1247,114 @@ public class TicketModel implements Serializable, Comparable { return text; } } + + + public static enum TicketAction { + Commit, Comment, Patchset, Close + } + + //Intentionally not serialized, links are persisted as "references" + public static class TicketLink { + public long targetTicketId; + public String hash; + public TicketAction action; + public boolean success; + public boolean isDelete; + + public TicketLink(long targetTicketId, TicketAction action) { + this.targetTicketId = targetTicketId; + this.action = action; + success = false; + isDelete = false; + } + + public TicketLink(long targetTicketId, TicketAction action, String hash) { + this.targetTicketId = targetTicketId; + this.action = action; + this.hash = hash; + success = false; + isDelete = false; + } + } + + public static enum ReferenceType { + Undefined, Commit, Ticket; + + @Override + public String toString() { + return name().toLowerCase().replace('_', ' '); + } + + public static ReferenceType fromObject(Object o, ReferenceType defaultType) { + if (o instanceof ReferenceType) { + // cast and return + return (ReferenceType) o; + } else if (o instanceof String) { + // find by name + for (ReferenceType type : values()) { + String str = o.toString(); + if (type.name().equalsIgnoreCase(str) + || type.toString().equalsIgnoreCase(str)) { + return type; + } + } + } else if (o instanceof Number) { + // by ordinal + int id = ((Number) o).intValue(); + if (id >= 0 && id < values().length) { + return values()[id]; + } + } + + return defaultType; + } + } + + public static class Reference implements Serializable { + + private static final long serialVersionUID = 1L; + + public String hash; + public Long ticketId; + + public Boolean deleted; + + Reference(String commitHash) { + this.hash = commitHash; + } + + Reference(long ticketId, String changeHash) { + this.ticketId = ticketId; + this.hash = changeHash; + } + + public ReferenceType getSourceType(){ + if (hash != null) { + if (ticketId != null) { + return ReferenceType.Ticket; + } else { + return ReferenceType.Commit; + } + } + + return ReferenceType.Undefined; + } + + public boolean isDeleted() { + return deleted != null && deleted; + } + + @Override + public String toString() { + switch (getSourceType()) { + case Commit: return hash; + case Ticket: return ticketId.toString() + "#" + hash; + default: {} break; + } + + return String.format("Unknown Reference Type"); + } + } public static class Attachment implements Serializable { diff --git a/src/main/java/com/gitblit/tickets/ITicketService.java b/src/main/java/com/gitblit/tickets/ITicketService.java index e8310039..20b6505b 100644 --- a/src/main/java/com/gitblit/tickets/ITicketService.java +++ b/src/main/java/com/gitblit/tickets/ITicketService.java @@ -50,9 +50,11 @@ import com.gitblit.models.TicketModel.Field; import com.gitblit.models.TicketModel.Patchset; import com.gitblit.models.TicketModel.PatchsetType; import com.gitblit.models.TicketModel.Status; +import com.gitblit.models.TicketModel.TicketLink; import com.gitblit.tickets.TicketIndexer.Lucene; import com.gitblit.utils.DeepCopier; import com.gitblit.utils.DiffUtils; +import com.gitblit.utils.JGitUtils; import com.gitblit.utils.DiffUtils.DiffStat; import com.gitblit.utils.StringUtils; import com.google.common.cache.Cache; @@ -1021,12 +1023,12 @@ public abstract class ITicketService implements IManager { } /** - * Updates a ticket. + * Updates a ticket and promotes pending links into references. * * @param repository - * @param ticketId + * @param ticketId, or 0 to action pending links in general * @param change - * @return the ticket model if successful + * @return the ticket model if successful, null if failure or using 0 ticketId * @since 1.4.0 */ public final TicketModel updateTicket(RepositoryModel repository, long ticketId, Change change) { @@ -1038,28 +1040,78 @@ public abstract class ITicketService implements IManager { throw new RuntimeException("must specify a change author!"); } - TicketKey key = new TicketKey(repository, ticketId); - ticketsCache.invalidate(key); - - boolean success = commitChangeImpl(repository, ticketId, change); + boolean success = true; + TicketModel ticket = null; + + if (ticketId > 0) { + TicketKey key = new TicketKey(repository, ticketId); + ticketsCache.invalidate(key); + + success = commitChangeImpl(repository, ticketId, change); + + if (success) { + ticket = getTicket(repository, ticketId); + ticketsCache.put(key, ticket); + indexer.index(ticket); + + // call the ticket hooks + if (pluginManager != null) { + for (TicketHook hook : pluginManager.getExtensions(TicketHook.class)) { + try { + hook.onUpdateTicket(ticket, change); + } catch (Exception e) { + log.error("Failed to execute extension", e); + } + } + } + } + } + if (success) { - TicketModel ticket = getTicket(repository, ticketId); - ticketsCache.put(key, ticket); - indexer.index(ticket); + //Now that the ticket has been successfully persisted add references to this ticket from linked tickets + if (change.hasPendingLinks()) { + for (TicketLink link : change.pendingLinks) { + TicketModel linkedTicket = getTicket(repository, link.targetTicketId); + Change dstChange = null; + + //Ignore if not available or self reference + if (linkedTicket != null && link.targetTicketId != ticketId) { + dstChange = new Change(change.author, change.date); + + switch (link.action) { + case Comment: { + if (ticketId == 0) { + throw new RuntimeException("must specify a ticket when linking a comment!"); + } + dstChange.referenceTicket(ticketId, change.comment.id); + } break; + + case Commit: { + dstChange.referenceCommit(link.hash); + } break; + + default: { + throw new RuntimeException( + String.format("must add persist logic for link of type %s", link.action)); + } + } + } + + if (dstChange != null) { + //If not deleted then remain null in journal + if (link.isDelete) { + dstChange.reference.deleted = true; + } - // call the ticket hooks - if (pluginManager != null) { - for (TicketHook hook : pluginManager.getExtensions(TicketHook.class)) { - try { - hook.onUpdateTicket(ticket, change); - } catch (Exception e) { - log.error("Failed to execute extension", e); + if (updateTicket(repository, link.targetTicketId, dstChange) != null) { + link.success = true; + } } } } - return ticket; } - return null; + + return ticket; } /** @@ -1232,9 +1284,18 @@ public abstract class ITicketService implements IManager { deletion.patchset.number = patchset.number; deletion.patchset.rev = patchset.rev; deletion.patchset.type = PatchsetType.Delete; + //Find and delete references to tickets by the removed commits + List patchsetTicketLinks = JGitUtils.identifyTicketsBetweenCommits( + repositoryManager.getRepository(ticket.repository), + settings, patchset.base, patchset.tip); - RepositoryModel repository = repositoryManager.getRepositoryModel(ticket.repository); - TicketModel revisedTicket = updateTicket(repository, ticket.number, deletion); + for (TicketLink link : patchsetTicketLinks) { + link.isDelete = true; + } + deletion.pendingLinks = patchsetTicketLinks; + + RepositoryModel repositoryModel = repositoryManager.getRepositoryModel(ticket.repository); + TicketModel revisedTicket = updateTicket(repositoryModel, ticket.number, deletion); return revisedTicket; } diff --git a/src/main/java/com/gitblit/tickets/TicketNotifier.java b/src/main/java/com/gitblit/tickets/TicketNotifier.java index 5979cf26..8c7fe6d4 100644 --- a/src/main/java/com/gitblit/tickets/TicketNotifier.java +++ b/src/main/java/com/gitblit/tickets/TicketNotifier.java @@ -317,6 +317,19 @@ public class TicketNotifier { // comment update sb.append(MessageFormat.format("**{0}** commented on this ticket.", user.getDisplayName())); sb.append(HARD_BRK); + } else if (lastChange.hasReference()) { + // reference update + String type = "?"; + + switch (lastChange.reference.getSourceType()) { + case Commit: { type = "commit"; } break; + case Ticket: { type = "ticket"; } break; + default: { } break; + } + + sb.append(MessageFormat.format("**{0}** referenced this ticket in {1} {2}", type, lastChange.toString())); + sb.append(HARD_BRK); + } else { // general update pattern = "**{0}** has updated this ticket."; diff --git a/src/main/java/com/gitblit/utils/JGitUtils.java b/src/main/java/com/gitblit/utils/JGitUtils.java index adcbb4de..a02fc3ff 100644 --- a/src/main/java/com/gitblit/utils/JGitUtils.java +++ b/src/main/java/com/gitblit/utils/JGitUtils.java @@ -28,6 +28,7 @@ import java.util.HashMap; import java.util.Iterator; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.Map.Entry; import java.util.regex.Matcher; import java.util.regex.Pattern; @@ -46,12 +47,15 @@ import org.eclipse.jgit.diff.DiffFormatter; import org.eclipse.jgit.diff.RawTextComparator; import org.eclipse.jgit.dircache.DirCache; import org.eclipse.jgit.dircache.DirCacheEntry; +import org.eclipse.jgit.errors.AmbiguousObjectException; import org.eclipse.jgit.errors.ConfigInvalidException; import org.eclipse.jgit.errors.IncorrectObjectTypeException; import org.eclipse.jgit.errors.LargeObjectException; import org.eclipse.jgit.errors.MissingObjectException; +import org.eclipse.jgit.errors.RevisionSyntaxException; import org.eclipse.jgit.errors.StopWalkException; import org.eclipse.jgit.internal.JGitText; +import org.eclipse.jgit.lib.AnyObjectId; import org.eclipse.jgit.lib.BlobBasedConfig; import org.eclipse.jgit.lib.CommitBuilder; import org.eclipse.jgit.lib.Constants; @@ -91,19 +95,22 @@ import org.eclipse.jgit.treewalk.filter.PathFilterGroup; import org.eclipse.jgit.treewalk.filter.PathSuffixFilter; import org.eclipse.jgit.treewalk.filter.TreeFilter; import org.eclipse.jgit.util.FS; +import org.jetbrains.annotations.NotNull; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import com.gitblit.GitBlit; import com.gitblit.GitBlitException; -import com.gitblit.manager.GitblitManager; +import com.gitblit.IStoredSettings; +import com.gitblit.Keys; +import com.gitblit.git.PatchsetCommand; import com.gitblit.models.FilestoreModel; import com.gitblit.models.GitNote; import com.gitblit.models.PathModel; import com.gitblit.models.PathModel.PathChangeModel; +import com.gitblit.models.TicketModel.TicketAction; +import com.gitblit.models.TicketModel.TicketLink; import com.gitblit.models.RefModel; import com.gitblit.models.SubmoduleModel; -import com.gitblit.servlet.FilestoreServlet; import com.google.common.base.Strings; /** @@ -2740,5 +2747,163 @@ public class JGitUtils { } return false; } + + /* + * Identify ticket by considering the branch the commit is on + * + * @param repository + * @param commit + * @return ticket number, or 0 if no ticket + */ + public static long getTicketNumberFromCommitBranch(Repository repository, RevCommit commit) { + // try lookup by change ref + Map> map = repository.getAllRefsByPeeledObjectId(); + Set refs = map.get(commit.getId()); + if (!ArrayUtils.isEmpty(refs)) { + for (Ref ref : refs) { + long number = PatchsetCommand.getTicketNumber(ref.getName()); + + if (number > 0) { + return number; + } + } + } + + return 0; + } + + + /** + * Try to identify all referenced tickets from the commit. + * + * @param commit + * @return a collection of TicketLinks + */ + @NotNull + public static List identifyTicketsFromCommitMessage(Repository repository, IStoredSettings settings, + RevCommit commit) { + List ticketLinks = new ArrayList(); + List linkedTickets = new ArrayList(); + + // parse commit message looking for fixes/closes #n + final String xFixDefault = "(?:fixes|closes)[\\s-]+#?(\\d+)"; + String xFix = settings.getString(Keys.tickets.closeOnPushCommitMessageRegex, xFixDefault); + if (StringUtils.isEmpty(xFix)) { + xFix = xFixDefault; + } + try { + Pattern p = Pattern.compile(xFix, Pattern.CASE_INSENSITIVE); + Matcher m = p.matcher(commit.getFullMessage()); + while (m.find()) { + String val = m.group(1); + long number = Long.parseLong(val); + + if (number > 0) { + ticketLinks.add(new TicketLink(number, TicketAction.Close)); + linkedTickets.add(number); + } + } + } catch (Exception e) { + LOGGER.error(String.format("Failed to parse \"%s\" in commit %s", xFix, commit.getName()), e); + } + + // parse commit message looking for ref #n + final String xRefDefault = "(?:ref|task|issue|bug)?[\\s-]*#(\\d+)"; + String xRef = settings.getString(Keys.tickets.linkOnPushCommitMessageRegex, xRefDefault); + if (StringUtils.isEmpty(xRef)) { + xRef = xRefDefault; + } + try { + Pattern p = Pattern.compile(xRef, Pattern.CASE_INSENSITIVE); + Matcher m = p.matcher(commit.getFullMessage()); + while (m.find()) { + String val = m.group(1); + long number = Long.parseLong(val); + //Most generic case so don't included tickets more precisely linked + if ((number > 0) && (!linkedTickets.contains(number))) { + ticketLinks.add( new TicketLink(number, TicketAction.Commit, commit.getName())); + linkedTickets.add(number); + } + } + } catch (Exception e) { + LOGGER.error(String.format("Failed to parse \"%s\" in commit %s", xRef, commit.getName()), e); + } + return ticketLinks; + } + + /** + * Try to identify all referenced tickets between two commits + * + * @param commit + * @param parseMessage + * @param currentTicketId, or 0 if not on a ticket branch + * @return a collection of TicketLink, or null if commit is already linked + */ + public static List identifyTicketsBetweenCommits(Repository repository, IStoredSettings settings, + String baseSha, String tipSha) { + List links = new ArrayList(); + if (repository == null) { return links; } + + RevWalk walk = new RevWalk(repository); + walk.sort(RevSort.TOPO); + walk.sort(RevSort.REVERSE, true); + try { + RevCommit tip = walk.parseCommit(repository.resolve(tipSha)); + RevCommit base = walk.parseCommit(repository.resolve(baseSha)); + walk.markStart(tip); + walk.markUninteresting(base); + for (;;) { + RevCommit commit = walk.next(); + if (commit == null) { + break; + } + links.addAll(JGitUtils.identifyTicketsFromCommitMessage(repository, settings, commit)); + } + } catch (IOException e) { + LOGGER.error("failed to identify tickets between commits.", e); + } finally { + walk.dispose(); + } + + return links; + } + + public static int countCommits(Repository repository, RevWalk walk, ObjectId baseId, ObjectId tipId) { + int count = 0; + walk.reset(); + walk.sort(RevSort.TOPO); + walk.sort(RevSort.REVERSE, true); + try { + RevCommit tip = walk.parseCommit(tipId); + RevCommit base = walk.parseCommit(baseId); + walk.markStart(tip); + walk.markUninteresting(base); + for (;;) { + RevCommit c = walk.next(); + if (c == null) { + break; + } + count++; + } + } catch (IOException e) { + // Should never happen, the core receive process would have + // identified the missing object earlier before we got control. + LOGGER.error("failed to get commit count", e); + return 0; + } finally { + walk.close(); + } + return count; + } + + public static int countCommits(Repository repository, RevWalk walk, String baseId, String tipId) { + int count = 0; + try { + count = countCommits(repository, walk, repository.resolve(baseId),repository.resolve(tipId)); + } catch (IOException e) { + LOGGER.error("failed to get commit count", e); + } + return count; + } } diff --git a/src/main/java/com/gitblit/wicket/GitBlitWebApp.properties b/src/main/java/com/gitblit/wicket/GitBlitWebApp.properties index cee7eaba..a215b4d6 100644 --- a/src/main/java/com/gitblit/wicket/GitBlitWebApp.properties +++ b/src/main/java/com/gitblit/wicket/GitBlitWebApp.properties @@ -778,4 +778,6 @@ gb.fileNotMergeable = Unable to commit {0}. This file can not be automatically gb.fileCommitted = Successfully committed {0}. gb.deletePatchset = Delete Patchset {0} gb.deletePatchsetSuccess = Deleted Patchset {0}. -gb.deletePatchsetFailure = Error deleting Patchset {0}. +gb.deletePatchsetFailure = Error deleting Patchset {0}. +gb.referencedByCommit = Referenced by commit. +gb.referencedByTicket = Referenced by ticket. diff --git a/src/main/java/com/gitblit/wicket/pages/TicketPage.html b/src/main/java/com/gitblit/wicket/pages/TicketPage.html index 974dcc03..46c0f7ee 100644 --- a/src/main/java/com/gitblit/wicket/pages/TicketPage.html +++ b/src/main/java/com/gitblit/wicket/pages/TicketPage.html @@ -461,7 +461,7 @@ pt push [revision type] - [R1] + [R1] diff --git a/src/main/java/com/gitblit/wicket/pages/TicketPage.java b/src/main/java/com/gitblit/wicket/pages/TicketPage.java index b2e63a60..cd049f4d 100644 --- a/src/main/java/com/gitblit/wicket/pages/TicketPage.java +++ b/src/main/java/com/gitblit/wicket/pages/TicketPage.java @@ -36,7 +36,6 @@ import org.apache.wicket.AttributeModifier; import org.apache.wicket.Component; import org.apache.wicket.MarkupContainer; import org.apache.wicket.PageParameters; -import org.apache.wicket.RequestCycle; import org.apache.wicket.RestartResponseException; import org.apache.wicket.ajax.AjaxRequestTarget; import org.apache.wicket.behavior.SimpleAttributeModifier; @@ -45,7 +44,6 @@ import org.apache.wicket.markup.html.image.ContextImage; import org.apache.wicket.markup.html.link.BookmarkablePageLink; import org.apache.wicket.markup.html.link.ExternalLink; import org.apache.wicket.markup.html.link.Link; -import org.apache.wicket.markup.html.link.StatelessLink; import org.apache.wicket.markup.html.pages.RedirectPage; import org.apache.wicket.markup.html.panel.Fragment; import org.apache.wicket.markup.repeater.Item; @@ -54,7 +52,6 @@ import org.apache.wicket.markup.repeater.data.ListDataProvider; import org.apache.wicket.model.Model; import org.apache.wicket.protocol.http.RequestUtils; import org.apache.wicket.protocol.http.WebRequest; -import org.apache.wicket.request.target.basic.RedirectRequestTarget; import org.eclipse.jgit.diff.DiffEntry.ChangeType; import org.eclipse.jgit.lib.PersonIdent; import org.eclipse.jgit.lib.Ref; @@ -863,9 +860,6 @@ public class TicketPage extends RepositoryPage { if (event.hasPatchset()) { // patchset Patchset patchset = event.patchset; - //In the case of using a cached change list - item.setVisible(!patchset.isDeleted()); - String what; if (event.isStatusChange() && (Status.New == event.getStatus())) { what = getString("gb.proposedThisChange"); @@ -883,6 +877,7 @@ public class TicketPage extends RepositoryPage { LinkPanel psr = new LinkPanel("patchsetRevision", null, patchset.number + "-" + patchset.rev, ComparePage.class, WicketUtils.newRangeParameter(repositoryName, patchset.parent == null ? patchset.base : patchset.parent, patchset.tip), true); WicketUtils.setHtmlTooltip(psr, patchset.toString()); + WicketUtils.setCssClass(psr, "aui-lozenge aui-lozenge-subtle"); item.add(psr); String typeCss = getPatchsetTypeCss(patchset.type); Label typeLabel = new Label("patchsetType", patchset.type.toString()); @@ -907,6 +902,42 @@ public class TicketPage extends RepositoryPage { // comment item.add(new Label("what", getString("gb.commented"))); item.add(new Label("patchsetRevision").setVisible(false)); + item.add(new Label("patchsetType").setVisible(false)); + item.add(new Label("deleteRevision").setVisible(false)); + item.add(new Label("patchsetDiffStat").setVisible(false)); + } else if (event.hasReference()) { + // reference + switch (event.reference.getSourceType()) { + case Commit: { + final int shaLen = app().settings().getInteger(Keys.web.shortCommitIdLength, 6); + + item.add(new Label("what", getString("gb.referencedByCommit"))); + LinkPanel psr = new LinkPanel("patchsetRevision", null, event.reference.toString().substring(0, shaLen), + CommitPage.class, WicketUtils.newObjectParameter(repositoryName, event.reference.toString()), true); + WicketUtils.setHtmlTooltip(psr, event.reference.toString()); + WicketUtils.setCssClass(psr, "ticketReference-commit shortsha1"); + item.add(psr); + + } break; + + case Ticket: { + final String text = MessageFormat.format("ticket/{0}", event.reference.ticketId); + + item.add(new Label("what", getString("gb.referencedByTicket"))); + //NOTE: Ideally reference the exact comment using reference.toString, + // however anchor hash is used and is escaped resulting in broken link + LinkPanel psr = new LinkPanel("patchsetRevision", null, text, + TicketsPage.class, WicketUtils.newObjectParameter(repositoryName, event.reference.ticketId.toString()), true); + WicketUtils.setCssClass(psr, "ticketReference-comment"); + item.add(psr); + } break; + + default: { + item.add(new Label("what").setVisible(false)); + item.add(new Label("patchsetRevision").setVisible(false)); + } + } + item.add(new Label("patchsetType").setVisible(false)); item.add(new Label("deleteRevision").setVisible(false)); item.add(new Label("patchsetDiffStat").setVisible(false)); diff --git a/src/main/resources/gitblit.css b/src/main/resources/gitblit.css index 3318441a..10c9a0e8 100644 --- a/src/main/resources/gitblit.css +++ b/src/main/resources/gitblit.css @@ -2391,4 +2391,19 @@ table.filestore-status { .delete-patchset { color:#D51900; font-size: 1.2em; +} + +.ticketReference-comment { + font-family: sans-serif; + font-weight: 200; + font-size: 1em; + font-variant: normal; + text-transform: none; +} + +.ticketReference-commit { + font-family: monospace; + font-weight: 200; + font-size: 1em; + font-variant: normal; } \ No newline at end of file diff --git a/src/test/config/test-gitblit.properties b/src/test/config/test-gitblit.properties index 78e9ab95..ef6a6c51 100644 --- a/src/test/config/test-gitblit.properties +++ b/src/test/config/test-gitblit.properties @@ -90,3 +90,5 @@ server.httpBindInterface = localhost server.httpsBindInterface = localhost server.storePassword = gitblit server.shutdownPort = 8081 + +tickets.service = com.gitblit.tickets.BranchTicketService diff --git a/src/test/java/com/gitblit/tests/GitBlitSuite.java b/src/test/java/com/gitblit/tests/GitBlitSuite.java index b01c82c4..133be77f 100644 --- a/src/test/java/com/gitblit/tests/GitBlitSuite.java +++ b/src/test/java/com/gitblit/tests/GitBlitSuite.java @@ -66,7 +66,7 @@ import com.gitblit.utils.JGitUtils; ModelUtilsTest.class, JnaUtilsTest.class, LdapSyncServiceTest.class, FileTicketServiceTest.class, BranchTicketServiceTest.class, RedisTicketServiceTest.class, AuthenticationManagerTest.class, SshKeysDispatcherTest.class, UITicketTest.class, PathUtilsTest.class, SshKerberosAuthenticationTest.class, - GravatarTest.class, FilestoreManagerTest.class, FilestoreServletTest.class }) + GravatarTest.class, FilestoreManagerTest.class, FilestoreServletTest.class, TicketReferenceTest.class }) public class GitBlitSuite { public static final File BASEFOLDER = new File("data"); diff --git a/src/test/java/com/gitblit/tests/TicketReferenceTest.java b/src/test/java/com/gitblit/tests/TicketReferenceTest.java new file mode 100644 index 00000000..934659cc --- /dev/null +++ b/src/test/java/com/gitblit/tests/TicketReferenceTest.java @@ -0,0 +1,939 @@ +/* + * Copyright 2016 gitblit.com. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.gitblit.tests; + +import java.io.BufferedWriter; +import java.io.File; +import java.io.FileOutputStream; +import java.io.OutputStreamWriter; +import java.text.MessageFormat; +import java.util.Date; +import java.util.List; + +import org.eclipse.jgit.api.CloneCommand; +import org.eclipse.jgit.api.Git; +import org.eclipse.jgit.api.MergeResult; +import org.eclipse.jgit.api.MergeCommand.FastForwardMode; +import org.eclipse.jgit.lib.Constants; +import org.eclipse.jgit.revwalk.RevCommit; +import org.eclipse.jgit.transport.CredentialsProvider; +import org.eclipse.jgit.transport.PushResult; +import org.eclipse.jgit.transport.RefSpec; +import org.eclipse.jgit.transport.RemoteRefUpdate; +import org.eclipse.jgit.transport.UsernamePasswordCredentialsProvider; +import org.eclipse.jgit.transport.RemoteRefUpdate.Status; +import org.eclipse.jgit.util.FileUtils; +import org.junit.AfterClass; +import org.junit.BeforeClass; +import org.junit.Test; + +import com.gitblit.Constants.AccessPermission; +import com.gitblit.Constants.AccessRestrictionType; +import com.gitblit.Constants.AuthorizationControl; +import com.gitblit.GitBlitException; +import com.gitblit.models.RepositoryModel; +import com.gitblit.models.TicketModel; +import com.gitblit.models.UserModel; +import com.gitblit.models.TicketModel.Change; +import com.gitblit.models.TicketModel.Field; +import com.gitblit.models.TicketModel.Reference; +import com.gitblit.tickets.ITicketService; + +/** + * Creates and deletes a range of ticket references via ticket comments and commits + */ +public class TicketReferenceTest extends GitblitUnitTest { + + static File workingCopy = new File(GitBlitSuite.REPOSITORIES, "working/TicketReferenceTest.git-wc"); + + static ITicketService ticketService; + + static final String account = "TicketRefTest"; + static final String password = GitBlitSuite.password; + + static final String url = GitBlitSuite.gitServletUrl; + + static UserModel user = null; + static RepositoryModel repo = null; + static CredentialsProvider cp = null; + static Git git = null; + + @BeforeClass + public static void configure() throws Exception { + File repositoryName = new File("TicketReferenceTest.git");; + + GitBlitSuite.close(repositoryName); + if (repositoryName.exists()) { + FileUtils.delete(repositoryName, FileUtils.RECURSIVE | FileUtils.RETRY); + } + repo = new RepositoryModel("TicketReferenceTest.git", null, null, null); + + if (gitblit().hasRepository(repo.name)) { + gitblit().deleteRepositoryModel(repo); + } + + gitblit().updateRepositoryModel(repo.name, repo, true); + + user = new UserModel(account); + user.displayName = account; + user.emailAddress = account + "@example.com"; + user.password = password; + + cp = new UsernamePasswordCredentialsProvider(user.username, user.password); + + if (gitblit().getUserModel(user.username) != null) { + gitblit().deleteUser(user.username); + } + + repo.authorizationControl = AuthorizationControl.NAMED; + repo.accessRestriction = AccessRestrictionType.PUSH; + gitblit().updateRepositoryModel(repo.name, repo, false); + + // grant user push permission + user.setRepositoryPermission(repo.name, AccessPermission.REWIND); + gitblit().updateUserModel(user); + + ticketService = gitblit().getTicketService(); + assertTrue(ticketService.deleteAll(repo)); + + GitBlitSuite.close(workingCopy); + if (workingCopy.exists()) { + FileUtils.delete(workingCopy, FileUtils.RECURSIVE | FileUtils.RETRY); + } + + CloneCommand clone = Git.cloneRepository(); + clone.setURI(MessageFormat.format("{0}/{1}", url, repo.name)); + clone.setDirectory(workingCopy); + clone.setBare(false); + clone.setBranch("master"); + clone.setCredentialsProvider(cp); + GitBlitSuite.close(clone.call()); + + git = Git.open(workingCopy); + git.getRepository().getConfig().setString("user", null, "name", user.displayName); + git.getRepository().getConfig().setString("user", null, "email", user.emailAddress); + git.getRepository().getConfig().save(); + + final RevCommit revCommit1 = makeCommit("initial commit"); + final String initialSha = revCommit1.name(); + Iterable results = git.push().setPushAll().setCredentialsProvider(cp).call(); + GitBlitSuite.close(git); + for (PushResult result : results) { + for (RemoteRefUpdate update : result.getRemoteUpdates()) { + assertEquals(Status.OK, update.getStatus()); + assertEquals(initialSha, update.getNewObjectId().name()); + } + } + } + + @AfterClass + public static void cleanup() throws Exception { + GitBlitSuite.close(git); + } + + @Test + public void noReferencesOnTicketCreation() throws Exception { + + TicketModel a = ticketService.createTicket(repo, newTicket("noReferencesOnCreation")); + assertNotNull(a); + assertFalse(a.hasReferences()); + + //Ensure retrieval process doesn't affect anything + a = ticketService.getTicket(repo, a.number); + assertNotNull(a); + assertFalse(a.hasReferences()); + } + + + @Test + public void commentNoUnexpectedReference() throws Exception { + + TicketModel a = ticketService.createTicket(repo, newTicket("commentNoUnexpectedReference-A")); + TicketModel b = ticketService.createTicket(repo, newTicket("commentNoUnexpectedReference-B")); + + assertNotNull(ticketService.updateTicket(repo, a.number, newComment("comment for 1 - no reference"))); + assertNotNull(ticketService.updateTicket(repo, a.number, newComment("comment for # - no reference"))); + assertNotNull(ticketService.updateTicket(repo, a.number, newComment("comment for #42 - ignores invalid reference"))); + + a = ticketService.getTicket(repo, a.number); + b = ticketService.getTicket(repo, b.number); + + assertFalse(a.hasReferences()); + assertFalse(b.hasReferences()); + } + + @Test + public void commentNoSelfReference() throws Exception { + + TicketModel a = ticketService.createTicket(repo, newTicket("commentNoSelfReference-A")); + + final Change comment = newComment(String.format("comment for #%d - no self reference", a.number)); + assertNotNull(ticketService.updateTicket(repo, a.number, comment)); + + a = ticketService.getTicket(repo, a.number); + + assertFalse(a.hasReferences()); + } + + @Test + public void commentSingleReference() throws Exception { + + TicketModel a = ticketService.createTicket(repo, newTicket("commentSingleReference-A")); + TicketModel b = ticketService.createTicket(repo, newTicket("commentSingleReference-B")); + + final Change comment = newComment(String.format("comment for #%d - single reference", b.number)); + assertNotNull(ticketService.updateTicket(repo, a.number, comment)); + + a = ticketService.getTicket(repo, a.number); + b = ticketService.getTicket(repo, b.number); + + assertFalse(a.hasReferences()); + assertTrue(b.hasReferences()); + + List cRefB = b.getReferences(); + assertNotNull(cRefB); + assertEquals(1, cRefB.size()); + assertEquals(a.number, cRefB.get(0).ticketId.longValue()); + assertEquals(comment.comment.id, cRefB.get(0).hash); + } + + @Test + public void commentSelfAndOtherReference() throws Exception { + TicketModel a = ticketService.createTicket(repo, newTicket("commentSelfAndOtherReference-A")); + TicketModel b = ticketService.createTicket(repo, newTicket("commentSelfAndOtherReference-B")); + + final Change comment = newComment(String.format("comment for #%d and #%d - self and other reference", a.number, b.number)); + assertNotNull(ticketService.updateTicket(repo, a.number, comment)); + + a = ticketService.getTicket(repo, a.number); + b = ticketService.getTicket(repo, b.number); + + assertFalse(a.hasReferences()); + assertTrue(b.hasReferences()); + + List cRefB = b.getReferences(); + assertNotNull(cRefB); + assertEquals(1, cRefB.size()); + assertEquals(a.number, cRefB.get(0).ticketId.longValue()); + assertEquals(comment.comment.id, cRefB.get(0).hash); + } + + @Test + public void commentMultiReference() throws Exception { + TicketModel a = ticketService.createTicket(repo, newTicket("commentMultiReference-A")); + TicketModel b = ticketService.createTicket(repo, newTicket("commentMultiReference-B")); + TicketModel c = ticketService.createTicket(repo, newTicket("commentMultiReference-C")); + + final Change comment = newComment(String.format("comment for #%d and #%d - multi reference", b.number, c.number)); + assertNotNull(ticketService.updateTicket(repo, a.number, comment)); + + a = ticketService.getTicket(repo, a.number); + b = ticketService.getTicket(repo, b.number); + c = ticketService.getTicket(repo, c.number); + + assertFalse(a.hasReferences()); + assertTrue(b.hasReferences()); + assertTrue(c.hasReferences()); + + List cRefB = b.getReferences(); + assertNotNull(cRefB); + assertEquals(1, cRefB.size()); + assertEquals(a.number, cRefB.get(0).ticketId.longValue()); + assertEquals(comment.comment.id, cRefB.get(0).hash); + + List cRefC = c.getReferences(); + assertNotNull(cRefC); + assertEquals(1, cRefC.size()); + assertEquals(a.number, cRefC.get(0).ticketId.longValue()); + assertEquals(comment.comment.id, cRefC.get(0).hash); + } + + + + @Test + public void commitMasterNoUnexpectedReference() throws Exception { + TicketModel a = ticketService.createTicket(repo, newTicket("commentMultiReference-A")); + + final String branchName = "master"; + git.checkout().setCreateBranch(false).setName(branchName).call(); + + makeCommit("commit for 1 - no reference"); + makeCommit("comment for # - no reference"); + final RevCommit revCommit1 = makeCommit("comment for #42 - ignores invalid reference"); + final String commit1Sha = revCommit1.name(); + + assertPushSuccess(commit1Sha, branchName); + + a = ticketService.getTicket(repo, a.number); + assertFalse(a.hasReferences()); + } + + @Test + public void commitMasterSingleReference() throws Exception { + TicketModel a = ticketService.createTicket(repo, newTicket("commitMasterSingleReference-A")); + + final String branchName = "master"; + git.checkout().setCreateBranch(false).setName(branchName).call(); + + final String message = String.format("commit for #%d - single reference", a.number); + final RevCommit revCommit1 = makeCommit(message); + final String commit1Sha = revCommit1.name(); + + assertPushSuccess(commit1Sha, branchName); + + a = ticketService.getTicket(repo, a.number); + assertTrue(a.hasReferences()); + + List cRefA = a.getReferences(); + assertNotNull(cRefA); + assertEquals(1, cRefA.size()); + assertNull(cRefA.get(0).ticketId); + assertEquals(commit1Sha, cRefA.get(0).hash); + } + + @Test + public void commitMasterMultiReference() throws Exception { + TicketModel a = ticketService.createTicket(repo, newTicket("commitMasterMultiReference-A")); + TicketModel b = ticketService.createTicket(repo, newTicket("commitMasterMultiReference-B")); + + final String branchName = "master"; + git.checkout().setCreateBranch(false).setName(branchName).call(); + + final String message = String.format("commit for #%d and #%d - multi reference", a.number, b.number); + final RevCommit revCommit1 = makeCommit(message); + final String commit1Sha = revCommit1.name(); + + assertPushSuccess(commit1Sha, branchName); + + a = ticketService.getTicket(repo, a.number); + b = ticketService.getTicket(repo, b.number); + assertTrue(a.hasReferences()); + assertTrue(b.hasReferences()); + + List cRefA = a.getReferences(); + assertNotNull(cRefA); + assertEquals(1, cRefA.size()); + assertNull(cRefA.get(0).ticketId); + assertEquals(commit1Sha, cRefA.get(0).hash); + + List cRefB = a.getReferences(); + assertNotNull(cRefB); + assertEquals(1, cRefB.size()); + assertNull(cRefB.get(0).ticketId); + assertEquals(commit1Sha, cRefB.get(0).hash); + } + + @Test + public void commitMasterAmendReference() throws Exception { + TicketModel a = ticketService.createTicket(repo, newTicket("commitMasterAmendReference-A")); + TicketModel b = ticketService.createTicket(repo, newTicket("commitMasterAmendReference-B")); + + final String branchName = "master"; + git.checkout().setCreateBranch(false).setName(branchName).call(); + + String message = String.format("commit before amend for #%d and #%d", a.number, b.number); + final RevCommit revCommit1 = makeCommit(message); + final String commit1Sha = revCommit1.name(); + assertPushSuccess(commit1Sha, branchName); + + a = ticketService.getTicket(repo, a.number); + b = ticketService.getTicket(repo, b.number); + assertTrue(a.hasReferences()); + assertTrue(b.hasReferences()); + + List cRefA = a.getReferences(); + assertNotNull(cRefA); + assertEquals(1, cRefA.size()); + assertNull(cRefA.get(0).ticketId); + assertEquals(commit1Sha, cRefA.get(0).hash); + + List cRefB = b.getReferences(); + assertNotNull(cRefB); + assertEquals(1, cRefB.size()); + assertNull(cRefB.get(0).ticketId); + assertEquals(commit1Sha, cRefB.get(0).hash); + + //Confirm that old invalid references removed for both tickets + //and new reference added for one referenced ticket + message = String.format("commit after amend for #%d", a.number); + final String commit2Sha = amendCommit(message); + + assertForcePushSuccess(commit2Sha, branchName); + + a = ticketService.getTicket(repo, a.number); + b = ticketService.getTicket(repo, b.number); + assertTrue(a.hasReferences()); + assertFalse(b.hasReferences()); + + cRefA = a.getReferences(); + assertNotNull(cRefA); + assertEquals(1, cRefA.size()); + assertNull(cRefA.get(0).ticketId); + assertEquals(commit2Sha, cRefA.get(0).hash); + } + + + @Test + public void commitPatchsetNoUnexpectedReference() throws Exception { + setPatchsetAvailable(true); + TicketModel a = ticketService.createTicket(repo, newTicket("commitPatchsetNoUnexpectedReference-A")); + + String branchName = String.format("ticket/%d", a.number); + git.checkout().setCreateBranch(true).setName(branchName).call(); + + makeCommit("commit for 1 - no reference"); + makeCommit("commit for # - no reference"); + final String message = "commit for #42 - ignores invalid reference"; + final RevCommit revCommit1 = makeCommit(message); + final String commit1Sha = revCommit1.name(); + + assertPushSuccess(commit1Sha, branchName); + + a = ticketService.getTicket(repo, a.number); + assertFalse(a.hasReferences()); + } + + @Test + public void commitPatchsetNoSelfReference() throws Exception { + setPatchsetAvailable(true); + TicketModel a = ticketService.createTicket(repo, newTicket("commitPatchsetNoSelfReference-A")); + + String branchName = String.format("ticket/%d", a.number); + git.checkout().setCreateBranch(true).setName(branchName).call(); + + final String message = String.format("commit for #%d - patchset self reference", a.number); + final RevCommit revCommit1 = makeCommit(message); + final String commit1Sha = revCommit1.name(); + + assertPushSuccess(commit1Sha, branchName); + + a = ticketService.getTicket(repo, a.number); + assertFalse(a.hasReferences()); + } + + @Test + public void commitPatchsetSingleReference() throws Exception { + setPatchsetAvailable(true); + TicketModel a = ticketService.createTicket(repo, newTicket("commitPatchsetSingleReference-A")); + TicketModel b = ticketService.createTicket(repo, newTicket("commitPatchsetSingleReference-B")); + + String branchName = String.format("ticket/%d", a.number); + git.checkout().setCreateBranch(true).setName(branchName).call(); + + final String message = String.format("commit for #%d - patchset single reference", b.number); + final RevCommit revCommit1 = makeCommit(message); + final String commit1Sha = revCommit1.name(); + + assertPushSuccess(commit1Sha, branchName); + + a = ticketService.getTicket(repo, a.number); + b = ticketService.getTicket(repo, b.number); + assertFalse(a.hasReferences()); + assertTrue(b.hasReferences()); + + List cRefB = b.getReferences(); + assertNotNull(cRefB); + assertEquals(1, cRefB.size()); + assertNull(cRefB.get(0).ticketId); + assertEquals(commit1Sha, cRefB.get(0).hash); + } + + @Test + public void commitPatchsetMultiReference() throws Exception { + setPatchsetAvailable(true); + TicketModel a = ticketService.createTicket(repo, newTicket("commitPatchsetMultiReference-A")); + TicketModel b = ticketService.createTicket(repo, newTicket("commitPatchsetMultiReference-B")); + TicketModel c = ticketService.createTicket(repo, newTicket("commitPatchsetMultiReference-C")); + + String branchName = String.format("ticket/%d", a.number); + git.checkout().setCreateBranch(true).setName(branchName).call(); + + final String message = String.format("commit for #%d and #%d- patchset multi reference", b.number, c.number); + final RevCommit revCommit1 = makeCommit(message); + final String commit1Sha = revCommit1.name(); + + assertPushSuccess(commit1Sha, branchName); + + a = ticketService.getTicket(repo, a.number); + b = ticketService.getTicket(repo, b.number); + c = ticketService.getTicket(repo, c.number); + assertFalse(a.hasReferences()); + assertTrue(b.hasReferences()); + assertTrue(c.hasReferences()); + + List cRefB = b.getReferences(); + assertNotNull(cRefB); + assertEquals(1, cRefB.size()); + assertNull(cRefB.get(0).ticketId); + assertEquals(commit1Sha, cRefB.get(0).hash); + + List cRefC = c.getReferences(); + assertNotNull(cRefC); + assertEquals(1, cRefC.size()); + assertNull(cRefC.get(0).ticketId); + assertEquals(commit1Sha, cRefC.get(0).hash); + } + + @Test + public void commitPatchsetAmendReference() throws Exception { + setPatchsetAvailable(true); + + TicketModel a = ticketService.createTicket(repo, newTicket("commitPatchsetAmendReference-A")); + TicketModel b = ticketService.createTicket(repo, newTicket("commitPatchsetAmendReference-B")); + TicketModel c = ticketService.createTicket(repo, newTicket("commitPatchsetAmendReference-C")); + assertFalse(c.hasPatchsets()); + + String branchName = String.format("ticket/%d", c.number); + git.checkout().setCreateBranch(true).setName(branchName).call(); + + String message = String.format("commit before amend for #%d and #%d", a.number, b.number); + final RevCommit revCommit1 = makeCommit(message); + final String commit1Sha = revCommit1.name(); + assertPushSuccess(commit1Sha, branchName); + + a = ticketService.getTicket(repo, a.number); + b = ticketService.getTicket(repo, b.number); + c = ticketService.getTicket(repo, c.number); + assertTrue(a.hasReferences()); + assertTrue(b.hasReferences()); + assertFalse(c.hasReferences()); + + assertTrue(c.hasPatchsets()); + assertNotNull(c.getPatchset(1, 1)); + + List cRefA = a.getReferences(); + assertNotNull(cRefA); + assertEquals(1, cRefA.size()); + assertNull(cRefA.get(0).ticketId); + assertEquals(commit1Sha, cRefA.get(0).hash); + + List cRefB = b.getReferences(); + assertNotNull(cRefB); + assertEquals(1, cRefB.size()); + assertNull(cRefB.get(0).ticketId); + assertEquals(commit1Sha, cRefB.get(0).hash); + + //As a new patchset is created the references will remain until deleted + message = String.format("commit after amend for #%d", a.number); + final String commit2Sha = amendCommit(message); + + assertForcePushSuccess(commit2Sha, branchName); + + a = ticketService.getTicket(repo, a.number); + b = ticketService.getTicket(repo, b.number); + c = ticketService.getTicket(repo, c.number); + assertTrue(a.hasReferences()); + assertTrue(b.hasReferences()); + assertFalse(c.hasReferences()); + + assertNotNull(c.getPatchset(1, 1)); + assertNotNull(c.getPatchset(2, 1)); + + cRefA = a.getReferences(); + assertNotNull(cRefA); + assertEquals(2, cRefA.size()); + assertNull(cRefA.get(0).ticketId); + assertNull(cRefA.get(1).ticketId); + assertEquals(commit1Sha, cRefA.get(0).hash); + assertEquals(commit2Sha, cRefA.get(1).hash); + + cRefB = b.getReferences(); + assertNotNull(cRefB); + assertEquals(1, cRefB.size()); + assertNull(cRefB.get(0).ticketId); + assertEquals(commit1Sha, cRefB.get(0).hash); + + //Delete the original patchset and confirm old references are removed + ticketService.deletePatchset(c, c.getPatchset(1, 1), user.username); + + a = ticketService.getTicket(repo, a.number); + b = ticketService.getTicket(repo, b.number); + c = ticketService.getTicket(repo, c.number); + assertTrue(a.hasReferences()); + assertFalse(b.hasReferences()); + assertFalse(c.hasReferences()); + + assertNull(c.getPatchset(1, 1)); + assertNotNull(c.getPatchset(2, 1)); + + cRefA = a.getReferences(); + assertNotNull(cRefA); + assertEquals(1, cRefA.size()); + assertNull(cRefA.get(0).ticketId); + assertEquals(commit2Sha, cRefA.get(0).hash); + } + + + @Test + public void commitTicketBranchNoUnexpectedReference() throws Exception { + setPatchsetAvailable(false); + TicketModel a = ticketService.createTicket(repo, newTicket("commitTicketBranchNoUnexpectedReference-A")); + + String branchName = String.format("ticket/%d", a.number); + git.checkout().setCreateBranch(true).setName(branchName).call(); + + makeCommit("commit for 1 - no reference"); + makeCommit("commit for # - no reference"); + final String message = "commit for #42 - ignores invalid reference"; + final RevCommit revCommit1 = makeCommit(message); + final String commit1Sha = revCommit1.name(); + assertPushSuccess(commit1Sha, branchName); + + a = ticketService.getTicket(repo, a.number); + assertFalse(a.hasReferences()); + } + + @Test + public void commitTicketBranchSelfReference() throws Exception { + setPatchsetAvailable(false); + TicketModel a = ticketService.createTicket(repo, newTicket("commitTicketBranchSelfReference-A")); + + String branchName = String.format("ticket/%d", a.number); + git.checkout().setCreateBranch(true).setName(branchName).call(); + + final String message = String.format("commit for #%d - patchset self reference", a.number); + final RevCommit revCommit1 = makeCommit(message); + final String commit1Sha = revCommit1.name(); + + assertPushSuccess(commit1Sha, branchName); + + a = ticketService.getTicket(repo, a.number); + assertTrue(a.hasReferences()); + + List cRefA = a.getReferences(); + assertNotNull(cRefA); + assertEquals(1, cRefA.size()); + assertNull(cRefA.get(0).ticketId); + assertEquals(commit1Sha, cRefA.get(0).hash); + } + + @Test + public void commitTicketBranchSingleReference() throws Exception { + setPatchsetAvailable(false); + TicketModel a = ticketService.createTicket(repo, newTicket("commitTicketBranchSingleReference-A")); + TicketModel b = ticketService.createTicket(repo, newTicket("commitTicketBranchSingleReference-B")); + + String branchName = String.format("ticket/%d", a.number); + git.checkout().setCreateBranch(true).setName(branchName).call(); + + final String message = String.format("commit for #%d - patchset single reference", b.number); + final RevCommit revCommit1 = makeCommit(message); + final String commit1Sha = revCommit1.name(); + + assertPushSuccess(commit1Sha, branchName); + + a = ticketService.getTicket(repo, a.number); + b = ticketService.getTicket(repo, b.number); + assertFalse(a.hasReferences()); + assertTrue(b.hasReferences()); + + List cRefB = b.getReferences(); + assertNotNull(cRefB); + assertEquals(1, cRefB.size()); + assertNull(cRefB.get(0).ticketId); + assertEquals(commit1Sha, cRefB.get(0).hash); + } + + @Test + public void commitTicketBranchMultiReference() throws Exception { + setPatchsetAvailable(false); + TicketModel a = ticketService.createTicket(repo, newTicket("commitTicketBranchMultiReference-A")); + TicketModel b = ticketService.createTicket(repo, newTicket("commitTicketBranchMultiReference-B")); + TicketModel c = ticketService.createTicket(repo, newTicket("commitTicketBranchMultiReference-C")); + + String branchName = String.format("ticket/%d", a.number); + git.checkout().setCreateBranch(true).setName(branchName).call(); + + final String message = String.format("commit for #%d and #%d- patchset multi reference", b.number, c.number); + final RevCommit revCommit1 = makeCommit(message); + final String commit1Sha = revCommit1.name(); + + assertPushSuccess(commit1Sha, branchName); + + a = ticketService.getTicket(repo, a.number); + b = ticketService.getTicket(repo, b.number); + c = ticketService.getTicket(repo, c.number); + assertFalse(a.hasReferences()); + assertTrue(b.hasReferences()); + assertTrue(c.hasReferences()); + + List cRefB = b.getReferences(); + assertNotNull(cRefB); + assertEquals(1, cRefB.size()); + assertNull(cRefB.get(0).ticketId); + assertEquals(commit1Sha, cRefB.get(0).hash); + + List cRefC = c.getReferences(); + assertNotNull(cRefC); + assertEquals(1, cRefC.size()); + assertNull(cRefC.get(0).ticketId); + assertEquals(commit1Sha, cRefC.get(0).hash); + } + + @Test + public void commitTicketBranchAmendReference() throws Exception { + setPatchsetAvailable(false); + + TicketModel a = ticketService.createTicket(repo, newTicket("commitTicketBranchAmendReference-A")); + TicketModel b = ticketService.createTicket(repo, newTicket("commitTicketBranchAmendReference-B")); + TicketModel c = ticketService.createTicket(repo, newTicket("commitTicketBranchAmendReference-C")); + assertFalse(c.hasPatchsets()); + + String branchName = String.format("ticket/%d", c.number); + git.checkout().setCreateBranch(true).setName(branchName).call(); + + String message = String.format("commit before amend for #%d and #%d", a.number, b.number); + final RevCommit revCommit1 = makeCommit(message); + final String commit1Sha = revCommit1.name(); + assertPushSuccess(commit1Sha, branchName); + + a = ticketService.getTicket(repo, a.number); + b = ticketService.getTicket(repo, b.number); + c = ticketService.getTicket(repo, c.number); + assertTrue(a.hasReferences()); + assertTrue(b.hasReferences()); + assertFalse(c.hasReferences()); + assertFalse(c.hasPatchsets()); + + List cRefA = a.getReferences(); + assertNotNull(cRefA); + assertEquals(1, cRefA.size()); + assertNull(cRefA.get(0).ticketId); + assertEquals(commit1Sha, cRefA.get(0).hash); + + List cRefB = b.getReferences(); + assertNotNull(cRefB); + assertEquals(1, cRefB.size()); + assertNull(cRefB.get(0).ticketId); + assertEquals(commit1Sha, cRefB.get(0).hash); + + //Confirm that old invalid references removed for both tickets + //and new reference added for one referenced ticket + message = String.format("commit after amend for #%d", a.number); + final String commit2Sha = amendCommit(message); + + assertForcePushSuccess(commit2Sha, branchName); + + a = ticketService.getTicket(repo, a.number); + b = ticketService.getTicket(repo, b.number); + c = ticketService.getTicket(repo, c.number); + assertTrue(a.hasReferences()); + assertFalse(b.hasReferences()); + assertFalse(c.hasReferences()); + assertFalse(c.hasPatchsets()); + + cRefA = a.getReferences(); + assertNotNull(cRefA); + assertEquals(1, cRefA.size()); + assertNull(cRefA.get(0).ticketId); + assertEquals(commit2Sha, cRefA.get(0).hash); + } + + @Test + public void commitTicketBranchDeleteNoMergeReference() throws Exception { + setPatchsetAvailable(false); + + TicketModel a = ticketService.createTicket(repo, newTicket("commitTicketBranchDeleteNoMergeReference-A")); + TicketModel b = ticketService.createTicket(repo, newTicket("commitTicketBranchDeleteNoMergeReference-B")); + TicketModel c = ticketService.createTicket(repo, newTicket("commitTicketBranchDeleteNoMergeReference-C")); + assertFalse(c.hasPatchsets()); + + String branchName = String.format("ticket/%d", c.number); + git.checkout().setCreateBranch(true).setName(branchName).call(); + + String message = String.format("commit before amend for #%d and #%d", a.number, b.number); + final RevCommit revCommit1 = makeCommit(message); + final String commit1Sha = revCommit1.name(); + assertPushSuccess(commit1Sha, branchName); + + a = ticketService.getTicket(repo, a.number); + b = ticketService.getTicket(repo, b.number); + c = ticketService.getTicket(repo, c.number); + assertTrue(a.hasReferences()); + assertTrue(b.hasReferences()); + assertFalse(c.hasReferences()); + + List cRefA = a.getReferences(); + assertNotNull(cRefA); + assertEquals(1, cRefA.size()); + assertNull(cRefA.get(0).ticketId); + assertEquals(commit1Sha, cRefA.get(0).hash); + + List cRefB = b.getReferences(); + assertNotNull(cRefB); + assertEquals(1, cRefB.size()); + assertNull(cRefB.get(0).ticketId); + assertEquals(commit1Sha, cRefB.get(0).hash); + + //Confirm that old invalid references removed for both tickets + assertDeleteBranch(branchName); + + a = ticketService.getTicket(repo, a.number); + b = ticketService.getTicket(repo, b.number); + c = ticketService.getTicket(repo, c.number); + assertFalse(a.hasReferences()); + assertFalse(b.hasReferences()); + assertFalse(c.hasReferences()); + } + + @Test + public void commitTicketBranchDeletePostMergeReference() throws Exception { + setPatchsetAvailable(false); + + TicketModel a = ticketService.createTicket(repo, newTicket("commitTicketBranchDeletePostMergeReference-A")); + TicketModel b = ticketService.createTicket(repo, newTicket("commitTicketBranchDeletePostMergeReference-B")); + TicketModel c = ticketService.createTicket(repo, newTicket("commitTicketBranchDeletePostMergeReference-C")); + assertFalse(c.hasPatchsets()); + + String branchName = String.format("ticket/%d", c.number); + git.checkout().setCreateBranch(true).setName(branchName).call(); + + String message = String.format("commit before amend for #%d and #%d", a.number, b.number); + final RevCommit revCommit1 = makeCommit(message); + final String commit1Sha = revCommit1.name(); + assertPushSuccess(commit1Sha, branchName); + + a = ticketService.getTicket(repo, a.number); + b = ticketService.getTicket(repo, b.number); + c = ticketService.getTicket(repo, c.number); + assertTrue(a.hasReferences()); + assertTrue(b.hasReferences()); + assertFalse(c.hasReferences()); + + List cRefA = a.getReferences(); + assertNotNull(cRefA); + assertEquals(1, cRefA.size()); + assertNull(cRefA.get(0).ticketId); + assertEquals(commit1Sha, cRefA.get(0).hash); + + List cRefB = b.getReferences(); + assertNotNull(cRefB); + assertEquals(1, cRefB.size()); + assertNull(cRefB.get(0).ticketId); + assertEquals(commit1Sha, cRefB.get(0).hash); + + git.checkout().setCreateBranch(false).setName("refs/heads/master").call(); + + // merge the tip of the branch into master + MergeResult mergeResult = git.merge().setFastForward(FastForwardMode.NO_FF).include(revCommit1.getId()).call(); + assertEquals(MergeResult.MergeStatus.MERGED, mergeResult.getMergeStatus()); + + // push the merged master to the origin + Iterable results = git.push().setCredentialsProvider(cp).setRemote("origin").call(); + for (PushResult result : results) { + RemoteRefUpdate ref = result.getRemoteUpdate("refs/heads/master"); + assertEquals(Status.OK, ref.getStatus()); + } + + //As everything has been merged no references should be changed + assertDeleteBranch(branchName); + + a = ticketService.getTicket(repo, a.number); + b = ticketService.getTicket(repo, b.number); + c = ticketService.getTicket(repo, c.number); + assertTrue(a.hasReferences()); + assertTrue(b.hasReferences()); + assertFalse(c.hasReferences()); + + cRefA = a.getReferences(); + assertNotNull(cRefA); + assertEquals(1, cRefA.size()); + assertNull(cRefA.get(0).ticketId); + assertEquals(commit1Sha, cRefA.get(0).hash); + + cRefB = b.getReferences(); + assertNotNull(cRefB); + assertEquals(1, cRefB.size()); + assertNull(cRefB.get(0).ticketId); + assertEquals(commit1Sha, cRefB.get(0).hash); + } + + private static Change newComment(String text) { + Change change = new Change("JUnit"); + change.comment(text); + return change; + } + + private static Change newTicket(String title) { + Change change = new Change("JUnit"); + change.setField(Field.title, title); + change.setField(Field.type, TicketModel.Type.Bug ); + return change; + } + + private static RevCommit makeCommit(String message) throws Exception { + File file = new File(workingCopy, "testFile.txt"); + OutputStreamWriter os = new OutputStreamWriter(new FileOutputStream(file, true), Constants.CHARSET); + BufferedWriter w = new BufferedWriter(os); + w.write("// " + new Date().toString() + "\n"); + w.close(); + git.add().addFilepattern(file.getName()).call(); + RevCommit rev = git.commit().setMessage(message).call(); + return rev; + } + + private static String amendCommit(String message) throws Exception { + File file = new File(workingCopy, "testFile.txt"); + OutputStreamWriter os = new OutputStreamWriter(new FileOutputStream(file, true), Constants.CHARSET); + BufferedWriter w = new BufferedWriter(os); + w.write("// " + new Date().toString() + "\n"); + w.close(); + git.add().addFilepattern(file.getName()).call(); + RevCommit rev = git.commit().setAmend(true).setMessage(message).call(); + return rev.getId().name(); + } + + + private void setPatchsetAvailable(boolean state) throws GitBlitException { + repo.acceptNewPatchsets = state; + gitblit().updateRepositoryModel(repo.name, repo, false); + } + + + private void assertPushSuccess(String commitSha, String branchName) throws Exception { + Iterable results = git.push().setRemote("origin").setCredentialsProvider(cp).call(); + + for (PushResult result : results) { + RemoteRefUpdate ref = result.getRemoteUpdate("refs/heads/" + branchName); + assertEquals(Status.OK, ref.getStatus()); + assertEquals(commitSha, ref.getNewObjectId().name()); + } + } + + private void assertForcePushSuccess(String commitSha, String branchName) throws Exception { + Iterable results = git.push().setForce(true).setRemote("origin").setCredentialsProvider(cp).call(); + + for (PushResult result : results) { + RemoteRefUpdate ref = result.getRemoteUpdate("refs/heads/" + branchName); + assertEquals(Status.OK, ref.getStatus()); + assertEquals(commitSha, ref.getNewObjectId().name()); + } + } + + private void assertDeleteBranch(String branchName) throws Exception { + + RefSpec refSpec = new RefSpec() + .setSource(null) + .setDestination("refs/heads/" + branchName); + + Iterable results = git.push().setRefSpecs(refSpec).setRemote("origin").setCredentialsProvider(cp).call(); + + for (PushResult result : results) { + RemoteRefUpdate ref = result.getRemoteUpdate("refs/heads/" + branchName); + assertEquals(Status.OK, ref.getStatus()); + } + } +} \ No newline at end of file -- 2.39.5