From 4a2fb11479c9b53ffe4e2e1cca82205ca122753b Mon Sep 17 00:00:00 2001 From: James Moger <james.moger@gitblit.com> Date: Fri, 31 Oct 2014 09:08:37 -0400 Subject: [PATCH] Gracefully handle missing integration branch in ticket page --- src/main/java/com/gitblit/wicket/pages/TicketPage.html | 1 - src/main/java/com/gitblit/utils/JGitUtils.java | 18 ++++++++++++++---- releases.moxie | 4 +++- src/main/java/com/gitblit/wicket/pages/TicketPage.java | 14 ++++++++++---- src/main/java/com/gitblit/wicket/GitBlitWebApp.properties | 1 + 5 files changed, 28 insertions(+), 10 deletions(-) diff --git a/releases.moxie b/releases.moxie index 145ea77..5fead38 100644 --- a/releases.moxie +++ b/releases.moxie @@ -14,7 +14,8 @@ design. '' security: ~ - fixes: ~ + fixes: + - Fix exception when viewing a ticket with a patchset where the integration branch does not exist (issue-521, ticket-212) changes: - Replaced Dagger with Guice (ticket-80) - Use release name as root directory in Gitblit GO artifacts (ticket-109) @@ -37,6 +38,7 @@ - Alex Lewis - Florian Zschocke - Paul Martin + - razzard } # diff --git a/src/main/java/com/gitblit/utils/JGitUtils.java b/src/main/java/com/gitblit/utils/JGitUtils.java index da51ea9..68c62ea 100644 --- a/src/main/java/com/gitblit/utils/JGitUtils.java +++ b/src/main/java/com/gitblit/utils/JGitUtils.java @@ -2270,7 +2270,7 @@ } public static enum MergeStatus { - NOT_MERGEABLE, FAILED, ALREADY_MERGED, MERGEABLE, MERGED; + MISSING_INTEGRATION_BRANCH, MISSING_SRC_BRANCH, NOT_MERGEABLE, FAILED, ALREADY_MERGED, MERGEABLE, MERGED; } /** @@ -2285,9 +2285,17 @@ public static MergeStatus canMerge(Repository repository, String src, String toBranch) { RevWalk revWalk = null; try { - revWalk = new RevWalk(repository); - RevCommit branchTip = revWalk.lookupCommit(repository.resolve(toBranch)); - RevCommit srcTip = revWalk.lookupCommit(repository.resolve(src)); + revWalk = new RevWalk(repository); + ObjectId branchId = repository.resolve(toBranch); + if (branchId == null) { + return MergeStatus.MISSING_INTEGRATION_BRANCH; + } + ObjectId srcId = repository.resolve(src); + if (srcId == null) { + return MergeStatus.MISSING_SRC_BRANCH; + } + RevCommit branchTip = revWalk.lookupCommit(branchId); + RevCommit srcTip = revWalk.lookupCommit(srcId); if (revWalk.isMergedInto(srcTip, branchTip)) { // already merged return MergeStatus.ALREADY_MERGED; @@ -2300,6 +2308,8 @@ if (canMerge) { return MergeStatus.MERGEABLE; } + } catch (NullPointerException e) { + LOGGER.error("Failed to determine canMerge", e); } catch (IOException e) { LOGGER.error("Failed to determine canMerge", e); } finally { diff --git a/src/main/java/com/gitblit/wicket/GitBlitWebApp.properties b/src/main/java/com/gitblit/wicket/GitBlitWebApp.properties index 5326042..ce7b522 100644 --- a/src/main/java/com/gitblit/wicket/GitBlitWebApp.properties +++ b/src/main/java/com/gitblit/wicket/GitBlitWebApp.properties @@ -749,3 +749,4 @@ gb.sortLowestPriority = lowest priority gb.sortHighestSeverity = highest severity gb.sortLowestSeverity = lowest severity +gb.missingIntegrationBranchMore = The target integration branch does not exist in the repository! \ No newline at end of file diff --git a/src/main/java/com/gitblit/wicket/pages/TicketPage.html b/src/main/java/com/gitblit/wicket/pages/TicketPage.html index f3c6b2a..5ae005e 100644 --- a/src/main/java/com/gitblit/wicket/pages/TicketPage.html +++ b/src/main/java/com/gitblit/wicket/pages/TicketPage.html @@ -557,7 +557,6 @@ </div> </wicket:fragment> - <!-- VETOED PATCHSET FRAGMENT --> <wicket:fragment wicket:id="vetoedFragment"> <div class="alert alert-error submit-info" style="padding:4px;"> diff --git a/src/main/java/com/gitblit/wicket/pages/TicketPage.java b/src/main/java/com/gitblit/wicket/pages/TicketPage.java index b140bfc..4890874 100644 --- a/src/main/java/com/gitblit/wicket/pages/TicketPage.java +++ b/src/main/java/com/gitblit/wicket/pages/TicketPage.java @@ -519,10 +519,10 @@ * TICKET METADATA */ add(new Label("ticketType", ticket.type.toString())); - + add(new Label("priority", ticket.priority.toString())); add(new Label("severity", ticket.severity.toString())); - + if (StringUtils.isEmpty(ticket.topic)) { add(new Label("ticketTopic").setVisible(false)); } else { @@ -531,8 +531,8 @@ String safeTopic = app().xssFilter().relaxed(topic); add(new Label("ticketTopic", safeTopic).setEscapeModelStrings(false)); } - - + + /* @@ -1431,6 +1431,12 @@ Fragment mergePanel = new Fragment("mergePanel", "alreadyMergedFragment", this); mergePanel.add(new Label("mergeTitle", MessageFormat.format(getString("gb.patchsetAlreadyMerged"), ticket.mergeTo))); return mergePanel; + } else if (MergeStatus.MISSING_INTEGRATION_BRANCH == mergeStatus) { + // target/integration branch is missing + Fragment mergePanel = new Fragment("mergePanel", "notMergeableFragment", this); + mergePanel.add(new Label("mergeTitle", MessageFormat.format(getString("gb.patchsetNotMergeable"), ticket.mergeTo))); + mergePanel.add(new Label("mergeMore", MessageFormat.format(getString("gb.missingIntegrationBranchMore"), ticket.mergeTo))); + return mergePanel; } else { // patchset can not be cleanly merged Fragment mergePanel = new Fragment("mergePanel", "notMergeableFragment", this); -- Gitblit v1.9.1