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
diff --git a/ReleaseNotes/ReleaseNotes-2.1.7.2.txt b/ReleaseNotes/ReleaseNotes-2.1.7.2.txt
new file mode 100644
index 0000000..2d7622e
--- /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 1eeb5b8..89d8223 100644
--- a/ReleaseNotes/index.txt
+++ b/ReleaseNotes/index.txt
@@ -9,6 +9,7 @@
[[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 621f52d..2fbda21 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 @@
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 4caf8f8..f4862ee 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 @@
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 @@
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 9d3ac2f..df523b9 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 @@
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 @@
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 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 @@
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 d2d4ce0..075d24e 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 @@
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 f131123..fa0579e 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 @@
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 6865b2b..f478d66 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 @@
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 @@
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 @@
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 @@
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 4b11849..74ee893 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.assistedinject.Assisted;
import java.util.Collection;
+import java.util.Collections;
import java.util.HashSet;
import java.util.Set;
@@ -198,8 +199,24 @@
|| 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 boolean canRunUploadPack() {
- return isAnyIncludedIn(uploadGroups, user.getEffectiveGroups());
+ return isAnyIncludedIn(uploadGroups, getEffectiveUserGroups());
}
public boolean canRunReceivePack() {
- return isAnyIncludedIn(receiveGroups, user.getEffectiveGroups());
+ return isAnyIncludedIn(receiveGroups, getEffectiveUserGroups());
}
}
