Fix preview diff on first change To compute the diff against the base for a patch set refs/changes/X/Y/Z the preview diff screens used refs/changes/X/Y/Z^1 as base revison. This failed for the first change in a project because in this case refs/changes/X/Y/Z^1 does not exist. Instead of using refs/changes/X/Y/Z^1 as base revision retrieve the commit info for the patch set and lookup the parent commit in it. If there is no parent commit then a diff for the first change should be displayed and the diff is requested from the server without setting an A revision. Adapt the XDocServlet and the XDocLoader to handle diff requests where no A revision is given. Change-Id: I78cff82ff6daafbe8b9cb6781ae76f454ff3a280 Signed-off-by: Edwin Kempin <edwin.kempin@sap.com>
diff --git a/src/main/java/com/googlesource/gerrit/plugins/xdocs/XDocLoader.java b/src/main/java/com/googlesource/gerrit/plugins/xdocs/XDocLoader.java index 15c1ce5..eb44305 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/xdocs/XDocLoader.java +++ b/src/main/java/com/googlesource/gerrit/plugins/xdocs/XDocLoader.java
@@ -115,12 +115,14 @@ try { RevWalk rw = new RevWalk(repo); try { - ObjectId revId = checkRevId(key.getRevId()); - String html = loadHtml(formatter, repo, rw, key, revId); + String html = null; + if (key.getRevId() != null) { + html = loadHtml(formatter, repo, rw, key, key.getRevId()); + } if (key.getDiffMode() != DiffMode.NO_DIFF) { - ObjectId revIdB = checkRevId(key.getRevIdB()); - String htmlB = loadHtml(formatter, repo, rw, key, revIdB); + String htmlB = + loadHtml(formatter, repo, rw, key, checkRevId(key.getRevIdB())); if (html == null && htmlB == null) { throw new ResourceNotFoundException(); } @@ -131,7 +133,8 @@ } } - RevCommit commit = rw.parseCommit(revId); + RevCommit commit = rw.parseCommit( + MoreObjects.firstNonNull(key.getRevIdB(), key.getRevId())); return getAsHtmlResource(html, commit.getCommitTime()); } finally { rw.release();
diff --git a/src/main/java/com/googlesource/gerrit/plugins/xdocs/XDocServlet.java b/src/main/java/com/googlesource/gerrit/plugins/xdocs/XDocServlet.java index 9d34b0c..1170a34 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/xdocs/XDocServlet.java +++ b/src/main/java/com/googlesource/gerrit/plugins/xdocs/XDocServlet.java
@@ -19,6 +19,7 @@ import com.google.common.base.CharMatcher; import com.google.common.base.MoreObjects; +import com.google.common.base.Strings; import com.google.common.hash.Hasher; import com.google.common.hash.Hashing; import com.google.common.net.HttpHeaders; @@ -130,15 +131,20 @@ ProjectControl projectControl = projectControlFactory.validateFor(key.project); String rev = getRevision(cfg, - MoreObjects.firstNonNull(key.revision, cfg.getIndexRef()), + key.diffMode == DiffMode.NO_DIFF + ? MoreObjects.firstNonNull(key.revision, cfg.getIndexRef()) + : key.revision, projectControl); String revB = getRevision(cfg, key.revisionB, projectControl); Repository repo = repoManager.openRepository(key.project); try { ObjectId revId = - resolveRevision(repo, MoreObjects.firstNonNull(rev, Constants.HEAD)); - if (ObjectId.isId(rev)) { + resolveRevision(repo, + key.diffMode == DiffMode.NO_DIFF + ? MoreObjects.firstNonNull(rev, Constants.HEAD) + : rev); + if (revId != null && ObjectId.isId(rev)) { validateCanReadCommit(repo, projectControl, revId); } @@ -322,9 +328,11 @@ private static String computeETag(Project.NameKey project, ObjectId revId, String file, ObjectId revIdB, DiffMode diffMode) { Hasher hasher = Hashing.md5().newHasher(); - hasher.putUnencodedChars(project.get()) - .putUnencodedChars(revId.getName()) - .putUnencodedChars(file); + hasher.putUnencodedChars(project.get()); + if (revId != null) { + hasher.putUnencodedChars(revId.getName()); + } + hasher.putUnencodedChars(file); if (diffMode != DiffMode.NO_DIFF) { hasher.putUnencodedChars(revIdB.getName()).putUnencodedChars( diffMode.name()); @@ -400,7 +408,7 @@ diffMode = DiffMode.UNIFIED; int p = revision.indexOf("<->"); revisionB = revision.substring(p + 3); - revision = revision.substring(0, p); + revision = Strings.emptyToNull(revision.substring(0, p)); } else if (revision.contains("<-")) { diffMode = DiffMode.SIDEBYSIDE_A; int p = revision.indexOf("<-"); @@ -410,7 +418,7 @@ diffMode = DiffMode.SIDEBYSIDE_B; int p = revision.indexOf("->"); revisionB = revision.substring(p + 2); - revision = revision.substring(0, p); + revision = Strings.emptyToNull(revision.substring(0, p)); } }
diff --git a/src/main/java/com/googlesource/gerrit/plugins/xdocs/client/ChangeInfo.java b/src/main/java/com/googlesource/gerrit/plugins/xdocs/client/ChangeInfo.java index 9f28d99..9aece0a 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/xdocs/client/ChangeInfo.java +++ b/src/main/java/com/googlesource/gerrit/plugins/xdocs/client/ChangeInfo.java
@@ -21,6 +21,7 @@ public final native String project() /*-{ return this.project; }-*/; public final native NativeMap<RevisionInfo> revisions() /*-{ return this.revisions; }-*/; public final native int _number() /*-{ return this._number; }-*/; + public final native String current_revision() /*-{ return this.current_revision; }-*/; protected ChangeInfo() { }
diff --git a/src/main/java/com/googlesource/gerrit/plugins/xdocs/client/CommitInfo.java b/src/main/java/com/googlesource/gerrit/plugins/xdocs/client/CommitInfo.java new file mode 100644 index 0000000..00b2641 --- /dev/null +++ b/src/main/java/com/googlesource/gerrit/plugins/xdocs/client/CommitInfo.java
@@ -0,0 +1,27 @@ +// Copyright (C) 2014 The Android Open Source Project +// +// 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.googlesource.gerrit.plugins.xdocs.client; + +import com.google.gwt.core.client.JavaScriptObject; +import com.google.gwt.core.client.JsArray; + +public class CommitInfo extends JavaScriptObject { + + public final native String commit() /*-{ return this.commit; }-*/; + public final native JsArray<CommitInfo> parents() /*-{ return this.parents; }-*/; + + protected CommitInfo() { + } +}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/xdocs/client/ProjectApi.java b/src/main/java/com/googlesource/gerrit/plugins/xdocs/client/ProjectApi.java new file mode 100644 index 0000000..85060ad --- /dev/null +++ b/src/main/java/com/googlesource/gerrit/plugins/xdocs/client/ProjectApi.java
@@ -0,0 +1,29 @@ +// Copyright (C) 2014 The Android Open Source Project +// +// 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.googlesource.gerrit.plugins.xdocs.client; + +import com.google.gerrit.plugin.client.rpc.RestApi; +import com.google.gwt.user.client.rpc.AsyncCallback; + +public class ProjectApi { + public static void getCommitInfo(String project, String ref, + AsyncCallback<CommitInfo> callback) { + project(project).view("commits").id(ref).get(callback); + } + + private static RestApi project(String id) { + return new RestApi("/projects/").id(id); + } +}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/xdocs/client/XDocDiffScreen.java b/src/main/java/com/googlesource/gerrit/plugins/xdocs/client/XDocDiffScreen.java index cff00ea..59fef0c 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/xdocs/client/XDocDiffScreen.java +++ b/src/main/java/com/googlesource/gerrit/plugins/xdocs/client/XDocDiffScreen.java
@@ -30,6 +30,8 @@ import com.googlesource.gerrit.plugins.xdocs.client.ChangeInfo.RevisionInfo; +import java.util.List; + public abstract class XDocDiffScreen extends VerticalPanel { protected final String changeId; protected final String path; @@ -49,8 +51,33 @@ ChangeApi.getChangeInfo(changeId, new AsyncCallback<ChangeInfo>() { @Override - public void onSuccess(ChangeInfo change) { - setRevisions(change, patchSet); + public void onSuccess(final ChangeInfo change) { + parseRevisions(change, patchSet); + if (revisionA == null) { + ProjectApi.getCommitInfo(change.project(), change.current_revision(), + new AsyncCallback<CommitInfo>() { + @Override + public void onSuccess(CommitInfo commit) { + if (commit.parents() != null) { + List<CommitInfo> parents = Natives.asList(commit.parents()); + if (!parents.isEmpty()) { + revisionA = parents.get(0).commit(); + } + } + show(change); + } + + @Override + public void onFailure(Throwable caught) { + // never invoked + } + }); + } else { + show(change); + } + } + + private void show(ChangeInfo change) { addHeader(change); init(); display(change); @@ -78,7 +105,7 @@ return revisionB; } - private void setRevisions(ChangeInfo change, String patchSetString) { + private void parseRevisions(ChangeInfo change, String patchSetString) { int i = patchSetString.indexOf(".."); if (i > 0) { base = parsePatchSet(patchSetString.substring(0, i)); @@ -92,7 +119,6 @@ } else { patchSet = parsePatchSet(patchSetString); revisionB = getRevision(change, patchSet); - revisionA = this.revisionB + "^1"; } }
diff --git a/src/main/java/com/googlesource/gerrit/plugins/xdocs/client/XDocSideBySideDiffScreen.java b/src/main/java/com/googlesource/gerrit/plugins/xdocs/client/XDocSideBySideDiffScreen.java index e9b0861..3e57b4e 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/xdocs/client/XDocSideBySideDiffScreen.java +++ b/src/main/java/com/googlesource/gerrit/plugins/xdocs/client/XDocSideBySideDiffScreen.java
@@ -38,8 +38,9 @@ @Override protected void display(ChangeInfo change) { String frameIdA = "xdoc_sidebyside_diff_a_iframe"; - Frame frameA = - new Frame(XDocApi.getUrl(change.project(), getRevisionSideA(), getPath())); + Frame frameA = getRevisionA() != null + ? new Frame(XDocApi.getUrl(change.project(), getRevisionSideA(), getPath())) + : new Frame(); frameA.getElement().setId(frameIdA); XDocScreen.resize(frameA, frameIdA); @@ -62,7 +63,7 @@ } private String getRevisionSideB() { - return getRevisionA() + "->" + getRevisionB(); + return (getRevisionA() != null ? getRevisionA() : "") + "->" + getRevisionB(); } @Override
diff --git a/src/main/java/com/googlesource/gerrit/plugins/xdocs/client/XDocUnifiedDiffScreen.java b/src/main/java/com/googlesource/gerrit/plugins/xdocs/client/XDocUnifiedDiffScreen.java index 9004ad7..d1c6548 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/xdocs/client/XDocUnifiedDiffScreen.java +++ b/src/main/java/com/googlesource/gerrit/plugins/xdocs/client/XDocUnifiedDiffScreen.java
@@ -45,7 +45,7 @@ } private String getRevision() { - return getRevisionA() + "<->" + getRevisionB(); + return (getRevisionA() != null ? getRevisionA() : "") + "<->" + getRevisionB(); } @Override