From c7f8a233042690b53aba0591c1ba260ca46ce76c Mon Sep 17 00:00:00 2001 From: Tom Date: Wed, 26 Oct 2016 22:49:56 +0200 Subject: [PATCH] Do not serialize JGit commit objects JGit commit objects are a recursive data structure; they have links to their parent commits. Serializing a JGit commit will try to recursively serialize all reachable ancestors as faras they have been loaded. If that ancestor chain is too long, a StackOverflowError is thrown during Wicket's page serialization if a page has a reference to sucha JGit commit. Fixed by making sure that pages o not contain references to JGit commits. Use the (existing) wrapper object RepositoryCommit instead. * RepositoryCommit has a transient reference to the JGit commit and reads the commit from the repository upon de-serialization. * RefModel is a similar case (JGit tags/branches may also have links to the commits they point to). Solved a bit differently by making it a pure data object by transferring the interesting data from the JGit object in the constructor. * Change DataViews instantiated with RevCommit to use RepositoryCommit instead. * Change inner anonymous DataViews to ensure they do not have a synthesized field referencing the "allRefs" map. Such a synthesized field would also get serialized, and then serialize JGit commits again. Finally, remove non-transient logger instances in Wicket classes. Those might lead to NotSerializableException. These StackOverflowErrors have been reported in several places since 2014: * https://groups.google.com/forum/#!topic/gitblit/GH1d8WSlR6Q * https://bugs.chromium.org/p/gerrit/issues/detail?id=3316 * https://groups.google.com/d/msg/repo-discuss/Kcl0JIGNiGk/0DjH4mO8hA8J * https://groups.google.com/d/msg/repo-discuss/0_P6A3fjTec/2kcpVPIUAQAJ * https://github.com/gitblit/gitblit/issues/1011 * https://github.com/tomaswolf/gerrit-gitblit-plugin/issues/21 Issue: #1011 (cherry picked from commit 005e8e2) --- .../java/com/gitblit/models/RefModel.java | 93 +++++++++++++++---- .../com/gitblit/models/RepositoryCommit.java | 49 +++++++--- .../com/gitblit/wicket/SessionlessForm.java | 14 ++- .../com/gitblit/wicket/pages/BlamePage.java | 2 +- .../gitblit/wicket/pages/EditFilePage.java | 2 +- .../com/gitblit/wicket/pages/MetricsPage.java | 2 +- .../com/gitblit/wicket/pages/RawPage.java | 31 ++++--- .../gitblit/wicket/pages/RepositoryPage.java | 10 +- .../com/gitblit/wicket/pages/SummaryPage.java | 2 +- .../com/gitblit/wicket/pages/TicketPage.java | 21 +++-- .../gitblit/wicket/panels/HistoryPanel.java | 39 ++++---- .../com/gitblit/wicket/panels/LogPanel.java | 33 ++++--- .../gitblit/wicket/panels/SearchPanel.java | 29 ++++-- 13 files changed, 230 insertions(+), 97 deletions(-) diff --git a/src/main/java/com/gitblit/models/RefModel.java b/src/main/java/com/gitblit/models/RefModel.java index d20c2dc6b..86d984742 100644 --- a/src/main/java/com/gitblit/models/RefModel.java +++ b/src/main/java/com/gitblit/models/RefModel.java @@ -36,18 +36,42 @@ */ public class RefModel implements Serializable, Comparable { - private static final long serialVersionUID = 1L; + private static final long serialVersionUID = 876822269940583606L; + public final String displayName; - public final RevObject referencedObject; - public transient Ref reference; + + private final Date date; + private final String name; + private final int type; + private final String id; + private final String referencedId; + private final boolean annotated; + private final PersonIdent person; + private final String shortMessage; + private final String fullMessage; + + private transient ObjectId objectId; + private transient ObjectId referencedObjectId; + + public transient Ref reference; // Used in too many places. public RefModel(String displayName, Ref ref, RevObject refObject) { - this.displayName = displayName; this.reference = ref; - this.referencedObject = refObject; + this.displayName = displayName; + this.date = internalGetDate(refObject); + this.name = (ref != null) ? ref.getName() : displayName; + this.type = internalGetReferencedObjectType(refObject); + this.objectId = internalGetObjectId(reference); + this.id = this.objectId.getName(); + this.referencedObjectId = internalGetReferencedObjectId(refObject); + this.referencedId = this.referencedObjectId.getName(); + this.annotated = internalIsAnnotatedTag(ref, refObject); + this.person = internalGetAuthorIdent(refObject); + this.shortMessage = internalGetShortMessage(refObject); + this.fullMessage = internalGetFullMessage(refObject); } - public Date getDate() { + private Date internalGetDate(RevObject referencedObject) { Date date = new Date(0); if (referencedObject != null) { if (referencedObject instanceof RevTag) { @@ -64,14 +88,15 @@ public Date getDate() { return date; } + public Date getDate() { + return date; + } + public String getName() { - if (reference == null) { - return displayName; - } - return reference.getName(); + return name; } - public int getReferencedObjectType() { + private int internalGetReferencedObjectType(RevObject referencedObject) { int type = referencedObject.getType(); if (referencedObject instanceof RevTag) { type = ((RevTag) referencedObject).getObject().getType(); @@ -79,14 +104,25 @@ public int getReferencedObjectType() { return type; } - public ObjectId getReferencedObjectId() { + public int getReferencedObjectType() { + return type; + } + + private ObjectId internalGetReferencedObjectId(RevObject referencedObject) { if (referencedObject instanceof RevTag) { return ((RevTag) referencedObject).getObject().getId(); } return referencedObject.getId(); } - public String getShortMessage() { + public ObjectId getReferencedObjectId() { + if (referencedObjectId == null) { + referencedObjectId = ObjectId.fromString(referencedId); + } + return referencedObjectId; + } + + private String internalGetShortMessage(RevObject referencedObject) { String message = ""; if (referencedObject instanceof RevTag) { message = ((RevTag) referencedObject).getShortMessage(); @@ -96,7 +132,11 @@ public String getShortMessage() { return message; } - public String getFullMessage() { + public String getShortMessage() { + return shortMessage; + } + + private String internalGetFullMessage(RevObject referencedObject) { String message = ""; if (referencedObject instanceof RevTag) { message = ((RevTag) referencedObject).getFullMessage(); @@ -106,7 +146,11 @@ public String getFullMessage() { return message; } - public PersonIdent getAuthorIdent() { + public String getFullMessage() { + return fullMessage; + } + + private PersonIdent internalGetAuthorIdent(RevObject referencedObject) { if (referencedObject instanceof RevTag) { return ((RevTag) referencedObject).getTaggerIdent(); } else if (referencedObject instanceof RevCommit) { @@ -115,17 +159,32 @@ public PersonIdent getAuthorIdent() { return null; } - public ObjectId getObjectId() { + public PersonIdent getAuthorIdent() { + return person; + } + + private ObjectId internalGetObjectId(Ref reference) { return reference.getObjectId(); } - public boolean isAnnotatedTag() { + public ObjectId getObjectId() { + if (objectId == null) { + objectId = ObjectId.fromString(id); + } + return objectId; + } + + private boolean internalIsAnnotatedTag(Ref reference, RevObject referencedObject) { if (referencedObject instanceof RevTag) { return !getReferencedObjectId().equals(getObjectId()); } return reference.getPeeledObjectId() != null; } + public boolean isAnnotatedTag() { + return annotated; + } + @Override public int hashCode() { return getReferencedObjectId().hashCode() + getName().hashCode(); diff --git a/src/main/java/com/gitblit/models/RepositoryCommit.java b/src/main/java/com/gitblit/models/RepositoryCommit.java index 765b4898d..43f314a3a 100644 --- a/src/main/java/com/gitblit/models/RepositoryCommit.java +++ b/src/main/java/com/gitblit/models/RepositoryCommit.java @@ -15,6 +15,8 @@ */ package com.gitblit.models; +import java.io.IOException; +import java.io.ObjectInputStream; import java.io.Serializable; import java.text.MessageFormat; import java.util.Date; @@ -22,30 +24,36 @@ import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.PersonIdent; +import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.revwalk.RevCommit; +import org.eclipse.jgit.revwalk.RevWalk; + +import com.gitblit.wicket.GitBlitWebApp; /** - * Model class to represent a RevCommit, it's source repository, and the branch. - * This class is used by the activity page. + * Model class to represent a RevCommit, it's source repository, and the branch. This class is used by the activity page. * * @author James Moger */ public class RepositoryCommit implements Serializable, Comparable { - private static final long serialVersionUID = 1L; + private static final long serialVersionUID = -2214911650485772022L; - public final String repository; + public String repository; - public final String branch; + public String branch; - private final RevCommit commit; + private final String commitId; private List refs; + private transient RevCommit commit; + public RepositoryCommit(String repository, String branch, RevCommit commit) { this.repository = repository; this.branch = branch; this.commit = commit; + this.commitId = commit.getName(); } public void setRefs(List refs) { @@ -80,7 +88,7 @@ public int getParentCount() { return commit.getParentCount(); } - public RevCommit [] getParents() { + public RevCommit[] getParents() { return commit.getParents(); } @@ -92,10 +100,14 @@ public PersonIdent getCommitterIdent() { return commit.getCommitterIdent(); } + public RevCommit getCommit() { + return commit; + } + @Override public boolean equals(Object o) { if (o instanceof RepositoryCommit) { - RepositoryCommit commit = (RepositoryCommit) o; + final RepositoryCommit commit = (RepositoryCommit) o; return repository.equals(commit.repository) && getName().equals(commit.getName()); } return false; @@ -123,8 +135,23 @@ public RepositoryCommit clone(String withRef) { @Override public String toString() { - return MessageFormat.format("{0} {1} {2,date,yyyy-MM-dd HH:mm} {3} {4}", - getShortName(), branch, getCommitterIdent().getWhen(), getAuthorIdent().getName(), - getShortMessage()); + return MessageFormat.format("{0} {1} {2,date,yyyy-MM-dd HH:mm} {3} {4}", getShortName(), branch, getCommitterIdent().getWhen(), + getAuthorIdent().getName(), getShortMessage()); + } + + // Serialization: restore the JGit RevCommit on reading + + private void readObject(ObjectInputStream input) throws IOException, ClassNotFoundException { + // Read in fields and any hidden stuff + input.defaultReadObject(); + // Go find the commit again. + final Repository repo = GitBlitWebApp.get().repositories().getRepository(repository); + if (repo == null) { + throw new IOException("Cannot find repositoy " + repository); + } + try (RevWalk walk = new RevWalk(repo)) { + commit = walk.parseCommit(repo.resolve(commitId)); + } } + } \ No newline at end of file diff --git a/src/main/java/com/gitblit/wicket/SessionlessForm.java b/src/main/java/com/gitblit/wicket/SessionlessForm.java index 6f7907174..c53f095eb 100644 --- a/src/main/java/com/gitblit/wicket/SessionlessForm.java +++ b/src/main/java/com/gitblit/wicket/SessionlessForm.java @@ -58,7 +58,7 @@ public class SessionlessForm extends StatelessForm { protected final PageParameters pageParameters; - private final Logger log = LoggerFactory.getLogger(SessionlessForm.class); + private transient Logger logger; /** * Sessionless forms must have a bookmarkable page class. A bookmarkable @@ -118,7 +118,10 @@ protected void onComponentTagBody(final MarkupStream markupStream, final Compone if (c != null) { // this form has a field id which matches the // parameter name, skip embedding a hidden value - log.warn(MessageFormat.format("Skipping page parameter \"{0}\" from sessionless form hidden fields because it collides with a form child wicket:id", key)); + logger().warn( + MessageFormat + .format("Skipping page parameter \"{0}\" from sessionless form hidden fields because it collides with a form child wicket:id", + key)); continue; } String value = pageParameters.getString(key); @@ -156,4 +159,11 @@ protected String getAbsoluteUrl(Class pageClass, PageParamet String absoluteUrl = RequestUtils.toAbsolutePath(relativeUrl); return absoluteUrl; } + + private Logger logger() { + if (logger == null) { + logger = LoggerFactory.getLogger(SessionlessForm.class); + } + return logger; + } } diff --git a/src/main/java/com/gitblit/wicket/pages/BlamePage.java b/src/main/java/com/gitblit/wicket/pages/BlamePage.java index 2fcca0ae1..8465058f3 100644 --- a/src/main/java/com/gitblit/wicket/pages/BlamePage.java +++ b/src/main/java/com/gitblit/wicket/pages/BlamePage.java @@ -108,7 +108,7 @@ public BlamePage(PageParameters params) { if (pathModel == null) { final String notFound = MessageFormat.format("Blame page failed to find {0} in {1} @ {2}", blobPath, repositoryName, objectId); - logger.error(notFound); + logger().error(notFound); add(new Label("annotation").setVisible(false)); add(new Label("missingBlob", missingBlob(blobPath, commit)).setEscapeModelStrings(false)); return; diff --git a/src/main/java/com/gitblit/wicket/pages/EditFilePage.java b/src/main/java/com/gitblit/wicket/pages/EditFilePage.java index dbf8a79e2..5ec9b6ee8 100644 --- a/src/main/java/com/gitblit/wicket/pages/EditFilePage.java +++ b/src/main/java/com/gitblit/wicket/pages/EditFilePage.java @@ -123,7 +123,7 @@ protected void onSubmit() { try { ObjectId docAtLoad = getRepository().resolve(commitIdAtLoad.getObject()); - logger.trace("Commiting Edit File page: " + commitIdAtLoad.getObject()); + logger().trace("Commiting Edit File page: " + commitIdAtLoad.getObject()); DirCache index = DirCache.newInCore(); DirCacheBuilder builder = index.builder(); diff --git a/src/main/java/com/gitblit/wicket/pages/MetricsPage.java b/src/main/java/com/gitblit/wicket/pages/MetricsPage.java index 96113b0f6..77811687a 100644 --- a/src/main/java/com/gitblit/wicket/pages/MetricsPage.java +++ b/src/main/java/com/gitblit/wicket/pages/MetricsPage.java @@ -94,7 +94,7 @@ private void createLineChart(Charts charts, String id, List metrics) { try { date = df.parse(metric.name); } catch (ParseException e) { - logger.error("Unable to parse date: " + metric.name); + logger().error("Unable to parse date: " + metric.name); return; } chart.addValue(date, (int)metric.count); diff --git a/src/main/java/com/gitblit/wicket/pages/RawPage.java b/src/main/java/com/gitblit/wicket/pages/RawPage.java index c43574782..bd901f24f 100644 --- a/src/main/java/com/gitblit/wicket/pages/RawPage.java +++ b/src/main/java/com/gitblit/wicket/pages/RawPage.java @@ -45,7 +45,7 @@ public class RawPage extends SessionPage { - private final Logger logger = LoggerFactory.getLogger(getClass().getSimpleName()); + private transient Logger logger; String contentType; @@ -95,7 +95,7 @@ public void respond(RequestCycle requestCycle) { if (binary == null) { final String objectNotFound = MessageFormat.format("Raw page failed to find object {0} in {1}", objectId, repositoryName); - logger.error(objectNotFound); + logger().error(objectNotFound); throw new AbortWithWebErrorCodeException(HttpServletResponse.SC_NOT_FOUND, objectNotFound); } contentType = "application/octet-stream"; @@ -104,7 +104,7 @@ public void respond(RequestCycle requestCycle) { try { response.getOutputStream().write(binary); } catch (Exception e) { - logger.error("Failed to write binary response", e); + logger().error("Failed to write binary response", e); } } else { // standard raw blob view @@ -112,7 +112,7 @@ public void respond(RequestCycle requestCycle) { if (commit == null) { final String commitNotFound = MessageFormat.format("Raw page failed to find commit {0} in {1}", objectId, repositoryName); - logger.error(commitNotFound); + logger().error(commitNotFound); throw new AbortWithWebErrorCodeException(HttpServletResponse.SC_NOT_FOUND, commitNotFound); } @@ -148,7 +148,7 @@ public void respond(RequestCycle requestCycle) { // image blobs byte[] image = JGitUtils.getByteContent(r, commit.getTree(), blobPath, true); if (image == null) { - logger.error(blobNotFound); + logger().error(blobNotFound); throw new AbortWithWebErrorCodeException(HttpServletResponse.SC_NOT_FOUND, blobNotFound); } contentType = "image/" + extension.toLowerCase(); @@ -157,14 +157,14 @@ public void respond(RequestCycle requestCycle) { try { response.getOutputStream().write(image); } catch (IOException e) { - logger.error("Failed to write image response", e); + logger().error("Failed to write image response", e); } break; case 3: // binary blobs (download) byte[] binary = JGitUtils.getByteContent(r, commit.getTree(), blobPath, true); if (binary == null) { - logger.error(blobNotFound); + logger().error(blobNotFound); throw new AbortWithWebErrorCodeException(HttpServletResponse.SC_NOT_FOUND, blobNotFound); } contentType = "application/octet-stream"; @@ -193,7 +193,7 @@ public void respond(RequestCycle requestCycle) { try { response.getOutputStream().write(binary); } catch (IOException e) { - logger.error("Failed to write binary response", e); + logger().error("Failed to write binary response", e); } break; default: @@ -201,7 +201,7 @@ public void respond(RequestCycle requestCycle) { String content = JGitUtils.getStringContent(r, commit.getTree(), blobPath, encodings); if (content == null) { - logger.error(blobNotFound); + logger().error(blobNotFound); throw new AbortWithWebErrorCodeException(HttpServletResponse.SC_NOT_FOUND, blobNotFound); } contentType = "text/plain; charset=UTF-8"; @@ -209,7 +209,7 @@ public void respond(RequestCycle requestCycle) { try { response.getOutputStream().write(content.getBytes("UTF-8")); } catch (Exception e) { - logger.error("Failed to write text response", e); + logger().error("Failed to write text response", e); } } @@ -218,7 +218,7 @@ public void respond(RequestCycle requestCycle) { String content = JGitUtils.getStringContent(r, commit.getTree(), blobPath, encodings); if (content == null) { - logger.error(blobNotFound); + logger().error(blobNotFound); throw new AbortWithWebErrorCodeException(HttpServletResponse.SC_NOT_FOUND, blobNotFound); } contentType = "text/plain; charset=UTF-8"; @@ -226,7 +226,7 @@ public void respond(RequestCycle requestCycle) { try { response.getOutputStream().write(content.getBytes("UTF-8")); } catch (Exception e) { - logger.error("Failed to write text response", e); + logger().error("Failed to write text response", e); } } } @@ -235,6 +235,13 @@ public void respond(RequestCycle requestCycle) { }); } + protected Logger logger() { + if (logger == null) { + logger = LoggerFactory.getLogger(getClass()); + } + return logger; + } + @Override protected void setHeaders(WebResponse response) { super.setHeaders(response); diff --git a/src/main/java/com/gitblit/wicket/pages/RepositoryPage.java b/src/main/java/com/gitblit/wicket/pages/RepositoryPage.java index 36c5ae16c..4d30e049d 100644 --- a/src/main/java/com/gitblit/wicket/pages/RepositoryPage.java +++ b/src/main/java/com/gitblit/wicket/pages/RepositoryPage.java @@ -42,8 +42,6 @@ import org.eclipse.jgit.lib.PersonIdent; import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.revwalk.RevCommit; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; import com.gitblit.Constants; import com.gitblit.GitBlitException; @@ -78,8 +76,6 @@ public abstract class RepositoryPage extends RootPage { - protected final Logger logger = LoggerFactory.getLogger(getClass()); - private final String PARAM_STAR = "star"; protected final String projectName; @@ -93,7 +89,7 @@ public abstract class RepositoryPage extends RootPage { private Map submodules; private boolean showAdmin; - private boolean isOwner; + private final boolean isOwner; public RepositoryPage(PageParameters params) { super(params); @@ -144,7 +140,7 @@ public RepositoryPage(PageParameters params) { try { app().gitblit().reviseUser(user.username, user); } catch (GitBlitException e) { - logger.error("Failed to update user " + user.username, e); + logger().error("Failed to update user " + user.username, e); error(getString("gb.failedToUpdateUser"), false); } } @@ -579,7 +575,7 @@ protected String getShortObjectId(String objectId) { } protected void addRefs(Repository r, RevCommit c) { - add(new RefsPanel("refsPanel", repositoryName, c, JGitUtils.getAllRefs(r, getRepositoryModel().showRemoteBranches))); + add(new RefsPanel("refsPanel", repositoryName, JGitUtils.getAllRefs(r, getRepositoryModel().showRemoteBranches).get(c.getId()))); } protected void addFullText(String wicketId, String text) { diff --git a/src/main/java/com/gitblit/wicket/pages/SummaryPage.java b/src/main/java/com/gitblit/wicket/pages/SummaryPage.java index 3cfa152e8..1a5f05182 100644 --- a/src/main/java/com/gitblit/wicket/pages/SummaryPage.java +++ b/src/main/java/com/gitblit/wicket/pages/SummaryPage.java @@ -190,7 +190,7 @@ private Charts createCharts(List metrics) { try { date = df.parse(metric.name); } catch (ParseException e) { - logger.error("Unable to parse date: " + metric.name); + logger().error("Unable to parse date: " + metric.name); return charts; } chart.addValue(date, (int)metric.count); diff --git a/src/main/java/com/gitblit/wicket/pages/TicketPage.java b/src/main/java/com/gitblit/wicket/pages/TicketPage.java index cd049f4d2..a014d42b4 100644 --- a/src/main/java/com/gitblit/wicket/pages/TicketPage.java +++ b/src/main/java/com/gitblit/wicket/pages/TicketPage.java @@ -66,6 +66,7 @@ import com.gitblit.git.PatchsetReceivePack; import com.gitblit.models.PathModel.PathChangeModel; import com.gitblit.models.RegistrantAccessPermission; +import com.gitblit.models.RepositoryCommit; import com.gitblit.models.RepositoryModel; import com.gitblit.models.RepositoryUrl; import com.gitblit.models.SubmoduleModel; @@ -815,13 +816,17 @@ public void populateItem(final Item item) { // commits List commits = JGitUtils.getRevLog(getRepository(), currentPatchset.base, currentPatchset.tip); - ListDataProvider commitsDp = new ListDataProvider(commits); - DataView commitsView = new DataView("commit", commitsDp) { + List repoCommits = new ArrayList<>(commits.size()); + for (RevCommit c : commits) { + repoCommits.add(new RepositoryCommit(repositoryName, "", c)); + } + ListDataProvider commitsDp = new ListDataProvider(repoCommits); + DataView commitsView = new DataView("commit", commitsDp) { private static final long serialVersionUID = 1L; @Override - public void populateItem(final Item item) { - RevCommit commit = item.getModelObject(); + public void populateItem(final Item item) { + RepositoryCommit commit = item.getModelObject(); PersonIdent author = commit.getAuthorIdent(); item.add(new AvatarImage("authorAvatar", author.getName(), author.getEmailAddress(), null, 16, false)); item.add(new Label("author", commit.getAuthorIdent().getName())); @@ -830,7 +835,7 @@ public void populateItem(final Item item) { item.add(new LinkPanel("diff", "link", getString("gb.diff"), CommitDiffPage.class, WicketUtils.newObjectParameter(repositoryName, commit.getName()), true)); item.add(new Label("title", StringUtils.trimString(commit.getShortMessage(), Constants.LEN_SHORTLOG_REFS))); - item.add(WicketUtils.createDateLabel("commitDate", JGitUtils.getAuthorDate(commit), GitBlitWebSession + item.add(WicketUtils.createDateLabel("commitDate", author.getWhen(), GitBlitWebSession .get().getTimezone(), getTimeUtils(), false)); item.add(new DiffStatPanel("commitDiffStat", 0, 0, true)); } @@ -1451,7 +1456,7 @@ public void run() { } else { // merge failure String msg = MessageFormat.format("Failed to merge ticket {0,number,0}: {1}", ticket.number, result.name()); - logger.error(msg); + logger().error(msg); GitBlitWebSession.get().cacheErrorMessage(msg); } } @@ -1461,13 +1466,13 @@ public void run() { String msg = MessageFormat.format("Can not merge ticket {0,number,0}, patchset {1,number,0} has been vetoed!", ticket.number, patchset.number); GitBlitWebSession.get().cacheErrorMessage(msg); - logger.error(msg); + logger().error(msg); } } else { // not current patchset String msg = MessageFormat.format("Can not merge ticket {0,number,0}, the patchset has been updated!", ticket.number); GitBlitWebSession.get().cacheErrorMessage(msg); - logger.error(msg); + logger().error(msg); } redirectTo(TicketsPage.class, getPageParameters()); diff --git a/src/main/java/com/gitblit/wicket/panels/HistoryPanel.java b/src/main/java/com/gitblit/wicket/panels/HistoryPanel.java index 75fd70e76..737fe5aae 100644 --- a/src/main/java/com/gitblit/wicket/panels/HistoryPanel.java +++ b/src/main/java/com/gitblit/wicket/panels/HistoryPanel.java @@ -44,7 +44,9 @@ import com.gitblit.models.PathModel; import com.gitblit.models.PathModel.PathChangeModel; import com.gitblit.models.RefModel; +import com.gitblit.models.RepositoryCommit; import com.gitblit.models.SubmoduleModel; +import com.gitblit.utils.ArrayUtils; import com.gitblit.utils.JGitUtils; import com.gitblit.utils.MarkdownUtils; import com.gitblit.utils.StringUtils; @@ -61,7 +63,7 @@ public class HistoryPanel extends BasePanel { private static final long serialVersionUID = 1L; - private boolean hasMore; + private final boolean hasMore; public HistoryPanel(String wicketId, final String repositoryName, final String objectId, final String path, Repository r, int limit, int pageOffset, boolean showRemoteRefs) { @@ -101,10 +103,9 @@ public HistoryPanel(String wicketId, final String repositoryName, final String o if (matchingPath == null) { // path not in commit // manually locate path in tree - TreeWalk tw = new TreeWalk(r); - tw.reset(); - tw.setRecursive(true); - try { + try (TreeWalk tw = new TreeWalk(r)) { + tw.reset(); + tw.setRecursive(true); tw.addTree(commit.getTree()); tw.setFilter(PathFilterGroup.createFromStrings(Collections.singleton(path))); while (tw.next()) { @@ -115,8 +116,6 @@ public HistoryPanel(String wicketId, final String repositoryName, final String o } } } catch (Exception e) { - } finally { - tw.close(); } } } @@ -136,7 +135,7 @@ public HistoryPanel(String wicketId, final String repositoryName, final String o hasSubmodule = false; } - final Map> allRefs = JGitUtils.getAllRefs(r, showRemoteRefs); + Map> allRefs = JGitUtils.getAllRefs(r, showRemoteRefs); List commits; if (pageResults) { // Paging result set @@ -152,15 +151,23 @@ public HistoryPanel(String wicketId, final String repositoryName, final String o hasMore = commits.size() >= itemsPerPage; final int hashLen = app().settings().getInteger(Keys.web.shortCommitIdLength, 6); - ListDataProvider dp = new ListDataProvider(commits); - DataView logView = new DataView("commit", dp) { + List repoCommits = new ArrayList<>(commits.size()); + for (RevCommit c : commits) { + RepositoryCommit repoCommit = new RepositoryCommit(repositoryName, "", c); + if (allRefs.containsKey(c)) { + repoCommit.setRefs(allRefs.get(c)); + } + repoCommits.add(repoCommit); + } + ListDataProvider dp = new ListDataProvider(repoCommits); + DataView logView = new DataView("commit", dp) { private static final long serialVersionUID = 1L; int counter; @Override - public void populateItem(final Item item) { - final RevCommit entry = item.getModelObject(); - final Date date = JGitUtils.getAuthorDate(entry); + public void populateItem(final Item item) { + final RepositoryCommit entry = item.getModelObject(); + Date date = entry.getAuthorIdent().getWhen(); item.add(WicketUtils.createDateLabel("commitDate", date, getTimeZone(), getTimeUtils())); @@ -182,7 +189,7 @@ public void populateItem(final Item item) { String shortMessage = entry.getShortMessage(); String trimmedMessage = shortMessage; - if (allRefs.containsKey(entry.getId())) { + if (!ArrayUtils.isEmpty(entry.getRefs())) { trimmedMessage = StringUtils.trimString(shortMessage, Constants.LEN_SHORTLOG_REFS); } else { trimmedMessage = StringUtils.trimString(shortMessage, Constants.LEN_SHORTLOG); @@ -195,7 +202,7 @@ public void populateItem(final Item item) { } item.add(shortlog); - item.add(new RefsPanel("commitRefs", repositoryName, entry, allRefs)); + item.add(new RefsPanel("commitRefs", repositoryName, entry.getRefs())); if (isTree) { // tree @@ -214,7 +221,7 @@ public void populateItem(final Item item) { } else if (isSubmodule) { // submodule Repository repository = app().repositories().getRepository(repositoryName); - String submoduleId = JGitUtils.getSubmoduleCommitId(repository, path, entry); + String submoduleId = JGitUtils.getSubmoduleCommitId(repository, path, entry.getCommit()); repository.close(); if (StringUtils.isEmpty(submoduleId)) { // not a submodule at this commit, just a matching path diff --git a/src/main/java/com/gitblit/wicket/panels/LogPanel.java b/src/main/java/com/gitblit/wicket/panels/LogPanel.java index e9d240d09..4521d4307 100644 --- a/src/main/java/com/gitblit/wicket/panels/LogPanel.java +++ b/src/main/java/com/gitblit/wicket/panels/LogPanel.java @@ -15,6 +15,7 @@ */ package com.gitblit.wicket.panels; +import java.util.ArrayList; import java.util.Date; import java.util.List; import java.util.Map; @@ -35,7 +36,9 @@ import com.gitblit.Constants; import com.gitblit.Keys; import com.gitblit.models.RefModel; +import com.gitblit.models.RepositoryCommit; import com.gitblit.servlet.BranchGraphServlet; +import com.gitblit.utils.ArrayUtils; import com.gitblit.utils.JGitUtils; import com.gitblit.utils.StringUtils; import com.gitblit.wicket.ExternalImage; @@ -50,7 +53,7 @@ public class LogPanel extends BasePanel { private static final long serialVersionUID = 1L; - private boolean hasMore; + private final boolean hasMore; public LogPanel(String wicketId, final String repositoryName, final String objectId, Repository r, int limit, int pageOffset, boolean showRemoteRefs) { @@ -61,7 +64,7 @@ public LogPanel(String wicketId, final String repositoryName, final String objec itemsPerPage = 50; } - final Map> allRefs = JGitUtils.getAllRefs(r, showRemoteRefs); + Map> allRefs = JGitUtils.getAllRefs(r, showRemoteRefs); List commits; if (pageResults) { // Paging result set @@ -75,8 +78,8 @@ public LogPanel(String wicketId, final String repositoryName, final String objec // works unless commits.size() represents the exact end. hasMore = commits.size() >= itemsPerPage; - final String baseUrl = WicketUtils.getGitblitURL(getRequest()); - final boolean showGraph = app().settings().getBoolean(Keys.web.showBranchGraph, true); + String baseUrl = WicketUtils.getGitblitURL(getRequest()); + boolean showGraph = app().settings().getBoolean(Keys.web.showBranchGraph, true); MarkupContainer graph = new WebMarkupContainer("graph"); add(graph); @@ -101,15 +104,23 @@ public LogPanel(String wicketId, final String repositoryName, final String objec } final int hashLen = app().settings().getInteger(Keys.web.shortCommitIdLength, 6); - ListDataProvider dp = new ListDataProvider(commits); - DataView logView = new DataView("commit", dp) { + List repoCommits = new ArrayList<>(commits.size()); + for (RevCommit c : commits) { + RepositoryCommit repoCommit = new RepositoryCommit(repositoryName, "", c); + if (allRefs.containsKey(c)) { + repoCommit.setRefs(allRefs.get(c)); + } + repoCommits.add(repoCommit); + } + ListDataProvider dp = new ListDataProvider(repoCommits); + DataView logView = new DataView("commit", dp) { private static final long serialVersionUID = 1L; int counter; @Override - public void populateItem(final Item item) { - final RevCommit entry = item.getModelObject(); - final Date date = JGitUtils.getAuthorDate(entry); + public void populateItem(final Item item) { + final RepositoryCommit entry = item.getModelObject(); + final Date date = entry.getAuthorIdent().getWhen(); final boolean isMerge = entry.getParentCount() > 1; item.add(WicketUtils.createDateLabel("commitDate", date, getTimeZone(), getTimeUtils())); @@ -132,7 +143,7 @@ public void populateItem(final Item item) { // short message String shortMessage = entry.getShortMessage(); String trimmedMessage = shortMessage; - if (allRefs.containsKey(entry.getId())) { + if (!ArrayUtils.isEmpty(entry.getRefs())) { trimmedMessage = StringUtils.trimString(shortMessage, Constants.LEN_SHORTLOG_REFS); } else { trimmedMessage = StringUtils.trimString(shortMessage, Constants.LEN_SHORTLOG); @@ -145,7 +156,7 @@ public void populateItem(final Item item) { } item.add(shortlog); - item.add(new RefsPanel("commitRefs", repositoryName, entry, allRefs)); + item.add(new RefsPanel("commitRefs", repositoryName, entry.getRefs())); // commit hash link LinkPanel commitHash = new LinkPanel("hashLink", null, entry.getName().substring(0, hashLen), diff --git a/src/main/java/com/gitblit/wicket/panels/SearchPanel.java b/src/main/java/com/gitblit/wicket/panels/SearchPanel.java index 09322bc75..b29adcb57 100644 --- a/src/main/java/com/gitblit/wicket/panels/SearchPanel.java +++ b/src/main/java/com/gitblit/wicket/panels/SearchPanel.java @@ -15,6 +15,7 @@ */ package com.gitblit.wicket.panels; +import java.util.ArrayList; import java.util.Date; import java.util.List; import java.util.Map; @@ -31,6 +32,8 @@ import com.gitblit.Constants; import com.gitblit.Keys; import com.gitblit.models.RefModel; +import com.gitblit.models.RepositoryCommit; +import com.gitblit.utils.ArrayUtils; import com.gitblit.utils.JGitUtils; import com.gitblit.utils.StringUtils; import com.gitblit.wicket.WicketUtils; @@ -43,7 +46,7 @@ public class SearchPanel extends BasePanel { private static final long serialVersionUID = 1L; - private boolean hasMore; + private final boolean hasMore; public SearchPanel(String wicketId, final String repositoryName, final String objectId, final String value, Constants.SearchType searchType, Repository r, int limit, int pageOffset, @@ -57,7 +60,7 @@ public SearchPanel(String wicketId, final String repositoryName, final String ob RevCommit commit = JGitUtils.getCommit(r, objectId); - final Map> allRefs = JGitUtils.getAllRefs(r, showRemoteRefs); + Map> allRefs = JGitUtils.getAllRefs(r, showRemoteRefs); List commits; if (pageResults) { // Paging result set @@ -78,15 +81,23 @@ public SearchPanel(String wicketId, final String repositoryName, final String ob add(new Label("searchString", value)); add(new Label("searchType", searchType.toString())); - ListDataProvider dp = new ListDataProvider(commits); - DataView searchView = new DataView("commit", dp) { + List repoCommits = new ArrayList<>(commits.size()); + for (RevCommit c : commits) { + RepositoryCommit repoCommit = new RepositoryCommit(repositoryName, "", c); + if (allRefs.containsKey(c)) { + repoCommit.setRefs(allRefs.get(c)); + } + repoCommits.add(repoCommit); + } + ListDataProvider dp = new ListDataProvider(repoCommits); + DataView searchView = new DataView("commit", dp) { private static final long serialVersionUID = 1L; int counter; @Override - public void populateItem(final Item item) { - final RevCommit entry = item.getModelObject(); - final Date date = JGitUtils.getAuthorDate(entry); + public void populateItem(final Item item) { + final RepositoryCommit entry = item.getModelObject(); + final Date date = entry.getAuthorIdent().getWhen(); item.add(WicketUtils.createDateLabel("commitDate", date, getTimeZone(), getTimeUtils())); @@ -107,7 +118,7 @@ public void populateItem(final Item item) { String shortMessage = entry.getShortMessage(); String trimmedMessage = shortMessage; - if (allRefs.containsKey(entry.getId())) { + if (!ArrayUtils.isEmpty(entry.getRefs())) { trimmedMessage = StringUtils.trimString(shortMessage, Constants.LEN_SHORTLOG_REFS); } else { trimmedMessage = StringUtils.trimString(shortMessage, Constants.LEN_SHORTLOG); @@ -120,7 +131,7 @@ public void populateItem(final Item item) { } item.add(shortlog); - item.add(new RefsPanel("commitRefs", repositoryName, entry, allRefs)); + item.add(new RefsPanel("commitRefs", repositoryName, entry.getRefs())); item.add(new BookmarkablePageLink("commit", CommitPage.class, WicketUtils .newObjectParameter(repositoryName, entry.getName())));