Merge branch 'stable'

* stable:
  Fix ChangeDetailFactory's invocation of PatchSetDetailFactory
  Release notes for 2.1.7.1
  Fix API breakage on ChangeDetailService
  Do not reset Patch History selection on navigation to next file diff
  Resolve Project Owners when checking access right on any ref

Conflicts:
	gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectControl.java
	gerrit-server/src/test/java/com/google/gerrit/server/project/RefControlTest.java

Change-Id: I6e956625cb4648df35035b9be2d32e6e431fb8f3

9 files changed, 82 insertions, 21 deletions

diff --git a/ReleaseNotes/ReleaseNotes-2.1.7.2.txt b/ReleaseNotes/ReleaseNotes-2.1.7.2.txt
new file mode 100644
index 0000000000..2d7622e5bb
--- /dev/null
+++ b/ReleaseNotes/ReleaseNotes-2.1.7.2.txt
@@ -0,0 +1,32 @@
+Release notes for Gerrit 2.1.7.1
+================================
+
+Gerrit 2.1.7.1 is now available:
+
+link:http://code.google.com/p/gerrit/downloads/detail?name=gerrit-2.1.7.1.war[http://code.google.com/p/gerrit/downloads/detail?name=gerrit-2.1.7.1.war]
+
+Bug Fixes
+---------
+* issue 997 Resolve Project Owners when checking access rights
++
+Members of the 'Project Owners' magical group did not always have
+their project owner privileges when Gerrit Code Review was looking
+for "access to any ref" at the project level. This was caused by
+not expanding the 'Project Owner's group to the actual ownership
+list. Fixed.
+
+* issue 999 Do not reset Patch History selection on navigation
++
+Navigating to the next/previous file lost the setting of the
+"Old Version" made under the "Patch History" expandable control
+on a specific file view. This was accidentally broken when the
+"Old Version History" control was added to the change page. Fixed.
+
+* Fix API breakage on ChangeDetailService
++
+Version 2.1.7 broke the Gerrit Code Review plugin for Mylyn Reviews
+due to an accidential signature change of one of the remote JSON
+APIs. The ChangeDetailService.patchSetDetail() method is back to the
+old signature and a new patchSetDetail2() method has been added to
+handle the newer calling convention used in some contexts of the
+web UI.
diff --git a/ReleaseNotes/index.txt b/ReleaseNotes/index.txt
index 1eeb5b8e49..89d82231a6 100644
--- a/ReleaseNotes/index.txt
+++ b/ReleaseNotes/index.txt
@@ -9,6 +9,7 @@ Version 2.2.x
 [[2_1]]
 Version 2.1.x
 -------------
+* link:ReleaseNotes-2.1.7.2.html[2.1.7.2],
 * link:ReleaseNotes-2.1.7.html[2.1.7],
 * link:ReleaseNotes-2.1.6.html[2.1.6],
   link:ReleaseNotes-2.1.6.1.html[2.1.6.1]
diff --git a/gerrit-common/src/main/java/com/google/gerrit/common/data/ChangeDetailService.java b/gerrit-common/src/main/java/com/google/gerrit/common/data/ChangeDetailService.java
index 621f52df3a..2fbda21af9 100644
--- a/gerrit-common/src/main/java/com/google/gerrit/common/data/ChangeDetailService.java
+++ b/gerrit-common/src/main/java/com/google/gerrit/common/data/ChangeDetailService.java
@@ -29,7 +29,9 @@ public interface ChangeDetailService extends RemoteJsonService {
 
   void includedInDetail(Change.Id id, AsyncCallback<IncludedInDetail> callback);
 
-  void patchSetDetail(PatchSet.Id keyA, PatchSet.Id keyB,
+  void patchSetDetail(PatchSet.Id key, AsyncCallback<PatchSetDetail> callback);
+
+  void patchSetDetail2(PatchSet.Id baseId, PatchSet.Id key,
       AccountDiffPreference diffPrefs, AsyncCallback<PatchSetDetail> callback);
 
   @SignInRequired
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/PatchSetComplexDisclosurePanel.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/PatchSetComplexDisclosurePanel.java
index 4caf8f88fc..f4862ee2d4 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/PatchSetComplexDisclosurePanel.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/PatchSetComplexDisclosurePanel.java
@@ -533,7 +533,7 @@ class PatchSetComplexDisclosurePanel extends ComplexDisclosurePanel implements O
       diffPrefs = patchTable.getPreferences().get();
     }
 
-    Util.DETAIL_SVC.patchSetDetail(patchSet.getId(), diffBaseId, diffPrefs,
+    Util.DETAIL_SVC.patchSetDetail2(diffBaseId, patchSet.getId(), diffPrefs,
         new GerritCallback<PatchSetDetail>() {
           @Override
           public void onSuccess(PatchSetDetail result) {
@@ -569,7 +569,7 @@ class PatchSetComplexDisclosurePanel extends ComplexDisclosurePanel implements O
         diffPrefs = new ListenableAccountDiffPreference().get();
       }
 
-      Util.DETAIL_SVC.patchSetDetail(patchSet.getId(), diffBaseId, diffPrefs,
+      Util.DETAIL_SVC.patchSetDetail2(diffBaseId, patchSet.getId(), diffPrefs,
           new GerritCallback<PatchSetDetail>() {
             public void onSuccess(final PatchSetDetail result) {
               ensureLoaded(result);
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/patches/PatchScreen.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/patches/PatchScreen.java
index 9d3ac2ff58..df523b9c6c 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/patches/PatchScreen.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/patches/PatchScreen.java
@@ -320,7 +320,7 @@ public abstract class PatchScreen extends Screen implements
   protected void onLoad() {
     super.onLoad();
     if (patchSetDetail == null) {
-      Util.DETAIL_SVC.patchSetDetail(idSideB, null, null,
+      Util.DETAIL_SVC.patchSetDetail(idSideB,
           new GerritCallback<PatchSetDetail>() {
             @Override
             public void onSuccess(PatchSetDetail result) {
@@ -403,7 +403,7 @@ public abstract class PatchScreen extends Screen implements
       commitMessageBlock.display(patchSetDetail.getInfo().getMessage());
     } else {
       commitMessageBlock.setVisible(false);
-      Util.DETAIL_SVC.patchSetDetail(idSideB, null, null,
+      Util.DETAIL_SVC.patchSetDetail(idSideB,
           new GerritCallback<PatchSetDetail>() {
             @Override
             public void onSuccess(PatchSetDetail result) {
@@ -479,6 +479,9 @@ public abstract class PatchScreen extends Screen implements
   public void setSideA(PatchSet.Id patchSetId) {
     idSideA = patchSetId;
     diffSideA = patchSetId;
+    if (fileList != null) {
+      fileList.setPatchSetIdToCompareWith(patchSetId);
+    }
   }
 
   public void setSideB(PatchSet.Id patchSetId) {
@@ -497,7 +500,7 @@ public abstract class PatchScreen extends Screen implements
         final PatchSet.Id psid = patchKey.getParentKey();
         fileList = new PatchTable(prefs);
         fileList.setSavePointerId("PatchTable " + psid);
-        Util.DETAIL_SVC.patchSetDetail(psid, null, null,
+        Util.DETAIL_SVC.patchSetDetail(psid,
             new GerritCallback<PatchSetDetail>() {
               public void onSuccess(final PatchSetDetail result) {
                 fileList.display(result);
diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/ChangeDetailFactory.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/ChangeDetailFactory.java
index d2d4ce0ed4..075d24ea18 100644
--- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/ChangeDetailFactory.java
+++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/ChangeDetailFactory.java
@@ -187,7 +187,7 @@ public class ChangeDetailFactory extends Handler<ChangeDetail> {
       NoSuchEntityException, PatchSetInfoNotAvailableException,
       NoSuchChangeException {
     final PatchSet.Id psId = detail.getChange().currentPatchSetId();
-    final PatchSetDetailFactory loader = patchSetDetail.create(psId, null, null);
+    final PatchSetDetailFactory loader = patchSetDetail.create(null, psId, null);
     loader.patchSet = detail.getCurrentPatchSet();
     loader.control = control;
     detail.setCurrentPatchSetDetail(loader.call());
diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/ChangeDetailServiceImpl.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/ChangeDetailServiceImpl.java
index f131123bab..fa0579e3d8 100644
--- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/ChangeDetailServiceImpl.java
+++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/ChangeDetailServiceImpl.java
@@ -52,10 +52,14 @@ class ChangeDetailServiceImpl implements ChangeDetailService {
     includedInDetail.create(id).to(callback);
   }
 
-  public void patchSetDetail(final PatchSet.Id idA, final PatchSet.Id idB,
-      final AccountDiffPreference diffPrefs,
-      final AsyncCallback<PatchSetDetail> callback) {
-    patchSetDetail.create(idA, idB, diffPrefs).to(callback);
+  public void patchSetDetail(PatchSet.Id id,
+      AsyncCallback<PatchSetDetail> callback) {
+    patchSetDetail2(null, id, null, callback);
+  }
+
+  public void patchSetDetail2(PatchSet.Id baseId, PatchSet.Id id,
+      AccountDiffPreference diffPrefs, AsyncCallback<PatchSetDetail> callback) {
+    patchSetDetail.create(baseId, id, diffPrefs).to(callback);
   }
 
   public void patchSetPublishDetail(final PatchSet.Id id,
diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/PatchSetDetailFactory.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/PatchSetDetailFactory.java
index 6865b2b0e7..f478d667a6 100644
--- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/PatchSetDetailFactory.java
+++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/PatchSetDetailFactory.java
@@ -56,8 +56,10 @@ class PatchSetDetailFactory extends Handler<PatchSetDetail> {
     LoggerFactory.getLogger(PatchSetDetailFactory.class);
 
   interface Factory {
-    PatchSetDetailFactory create(@Assisted("psIdNew") PatchSet.Id psIdA,
-        @Assisted("psIdOld") PatchSet.Id psIdB, AccountDiffPreference diffPrefs);
+    PatchSetDetailFactory create(
+        @Assisted("psIdBase") @Nullable PatchSet.Id psIdBase,
+        @Assisted("psIdNew") PatchSet.Id psIdNew,
+        @Nullable AccountDiffPreference diffPrefs);
   }
 
   private final PatchSetInfoFactory infoFactory;
@@ -66,8 +68,8 @@ class PatchSetDetailFactory extends Handler<PatchSetDetail> {
   private final ChangeControl.Factory changeControlFactory;
 
   private Project.NameKey projectKey;
+  private final PatchSet.Id psIdBase;
   private final PatchSet.Id psIdNew;
-  private final PatchSet.Id psIdOld;
   private final AccountDiffPreference diffPrefs;
   private ObjectId oldId;
   private ObjectId newId;
@@ -80,16 +82,16 @@ class PatchSetDetailFactory extends Handler<PatchSetDetail> {
   PatchSetDetailFactory(final PatchSetInfoFactory psif, final ReviewDb db,
       final PatchListCache patchListCache,
       final ChangeControl.Factory changeControlFactory,
+      @Assisted("psIdBase") @Nullable final PatchSet.Id psIdBase,
       @Assisted("psIdNew") final PatchSet.Id psIdNew,
-      @Assisted("psIdOld") @Nullable final PatchSet.Id psIdOld,
       @Assisted @Nullable final AccountDiffPreference diffPrefs) {
     this.infoFactory = psif;
     this.db = db;
     this.patchListCache = patchListCache;
     this.changeControlFactory = changeControlFactory;
 
+    this.psIdBase = psIdBase;
     this.psIdNew = psIdNew;
-    this.psIdOld = psIdOld;
     this.diffPrefs = diffPrefs;
   }
 
@@ -106,9 +108,9 @@ class PatchSetDetailFactory extends Handler<PatchSetDetail> {
 
     final PatchList list;
 
-    if (psIdOld != null) {
+    if (psIdBase != null) {
+      oldId = toObjectId(psIdBase);
       newId = toObjectId(psIdNew);
-      oldId = psIdOld != null ? toObjectId(psIdOld) : null;
 
       projectKey = control.getProject().getNameKey();
 
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectControl.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectControl.java
index 4b11849af4..74ee893e0c 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectControl.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectControl.java
@@ -32,6 +32,7 @@ import com.google.inject.Provider;
 import com.google.inject.assistedinject.Assisted;
 
 import java.util.Collection;
+import java.util.Collections;
 import java.util.HashSet;
 import java.util.Set;
 
@@ -198,8 +199,24 @@ public class ProjectControl {
         || canPerformOnAnyRef(Permission.PUSH_TAG);
   }
 
+  /**
+   * @return the effective groups of the current user for this project
+   */
+  private Set<AccountGroup.UUID> getEffectiveUserGroups() {
+    final Set<AccountGroup.UUID> userGroups = user.getEffectiveGroups();
+    if (isOwner()) {
+      final Set<AccountGroup.UUID> userGroupsOnProject =
+          new HashSet<AccountGroup.UUID>(userGroups.size() + 1);
+      userGroupsOnProject.addAll(userGroups);
+      userGroupsOnProject.add(AccountGroup.PROJECT_OWNERS);
+      return Collections.unmodifiableSet(userGroupsOnProject);
+    } else {
+      return userGroups;
+    }
+  }
+
   private boolean canPerformOnAnyRef(String permissionName) {
-    final Set<AccountGroup.UUID> groups = user.getEffectiveGroups();
+    final Set<AccountGroup.UUID> groups = getEffectiveUserGroups();
 
     for (AccessSection section : access()) {
       Permission permission = section.getPermission(permissionName);
@@ -264,10 +281,10 @@ public class ProjectControl {
   }
 
   public boolean canRunUploadPack() {
-    return isAnyIncludedIn(uploadGroups, user.getEffectiveGroups());
+    return isAnyIncludedIn(uploadGroups, getEffectiveUserGroups());
   }
 
   public boolean canRunReceivePack() {
-    return isAnyIncludedIn(receiveGroups, user.getEffectiveGroups());
+    return isAnyIncludedIn(receiveGroups, getEffectiveUserGroups());
   }
 }