Submit: Fix ClassCastException for anonymous user

Change screen is using GET /changes/<iid>/revisions/<id>/actions REST
endpoint, that is always returning empty map for anonymous user. In
this code path UI action handlers don't get called, and getDescription()
methods from UI action classes aren't invoked.

However for unified change screen, the old code path in RPC code is
still active and calling PatchSetDetailFactory.call() and there
UiActions.sorted() is invoked, that iterates over all UI actions and
invokes getDescription() methods to figure out if this UI action should
be rendered or not.

However, since I7ec4a0beb5 all this code is dead and it was a mistake
not to remove it, because only there actions were rendered and not on
the old unified diff. That why we can safely remove all this code and
prevent that getDescription() methods from UI actions classes get
called for the anonymous users (and registered users) in the first
place.

Eclipse-Bug: https://bugs.eclipse.org/bugs/show_bug.cgi?id=479618
Bug: Issue 3531
Change-Id: I5480f0922cfe9a778472a3c99fd5631b3e5f2474
This commit is contained in:
David Ostrovsky
2015-10-13 13:42:17 +00:00
parent b9b5394c02
commit 8083f174ab
4 changed files with 0 additions and 96 deletions

View File

@@ -19,7 +19,6 @@ import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.client.PatchSetInfo;
import com.google.gerrit.reviewdb.client.Project;
import java.util.Collections;
import java.util.List;
public class PatchSetDetail {
@@ -27,7 +26,6 @@ public class PatchSetDetail {
protected PatchSetInfo info;
protected List<Patch> patches;
protected Project.NameKey project;
protected List<UiCommandDetail> commands;
public PatchSetDetail() {
}
@@ -63,15 +61,4 @@ public class PatchSetDetail {
public void setProject(final Project.NameKey p) {
project = p;
}
public List<UiCommandDetail> getCommands() {
if (commands != null) {
return commands;
}
return Collections.emptyList();
}
public void setCommands(List<UiCommandDetail> cmds) {
commands = cmds.isEmpty() ? null : cmds;
}
}

View File

@@ -1,24 +0,0 @@
// Copyright (C) 2013 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.google.gerrit.common.data;
/** Detail necessary to display an action. */
public class UiCommandDetail {
public String id;
public String method;
public String label;
public String title;
public boolean enabled;
}

View File

@@ -14,16 +14,11 @@
package com.google.gerrit.httpd.rpc.changedetail;
import com.google.common.base.Function;
import com.google.common.base.Optional;
import com.google.common.collect.Iterables;
import com.google.common.collect.Lists;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.common.data.PatchSetDetail;
import com.google.gerrit.common.data.UiCommandDetail;
import com.google.gerrit.common.errors.NoSuchEntityException;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.webui.UiAction;
import com.google.gerrit.httpd.rpc.Handler;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.AccountDiffPreference;
@@ -38,12 +33,8 @@ import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.PatchLineCommentsUtil;
import com.google.gerrit.server.change.ChangesCollection;
import com.google.gerrit.server.change.RevisionResource;
import com.google.gerrit.server.change.Revisions;
import com.google.gerrit.server.edit.ChangeEdit;
import com.google.gerrit.server.edit.ChangeEditUtil;
import com.google.gerrit.server.extensions.webui.UiActions;
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.patch.PatchList;
import com.google.gerrit.server.patch.PatchListCache;
@@ -57,7 +48,6 @@ import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.assistedinject.Assisted;
import com.google.inject.util.Providers;
import org.eclipse.jgit.lib.ObjectId;
import org.slf4j.Logger;
@@ -86,8 +76,6 @@ class PatchSetDetailFactory extends Handler<PatchSetDetail> {
private final PatchListCache patchListCache;
private final Provider<CurrentUser> userProvider;
private final ChangeControl.GenericFactory changeControlFactory;
private final ChangesCollection changes;
private final Revisions revisions;
private final PatchLineCommentsUtil plcUtil;
private final ChangeEditUtil editUtil;
@@ -107,8 +95,6 @@ class PatchSetDetailFactory extends Handler<PatchSetDetail> {
final PatchListCache patchListCache,
final Provider<CurrentUser> userProvider,
final ChangeControl.GenericFactory changeControlFactory,
final ChangesCollection changes,
final Revisions revisions,
final PatchLineCommentsUtil plcUtil,
ChangeEditUtil editUtil,
@Assisted("psIdBase") @Nullable final PatchSet.Id psIdBase,
@@ -119,8 +105,6 @@ class PatchSetDetailFactory extends Handler<PatchSetDetail> {
this.patchListCache = patchListCache;
this.userProvider = userProvider;
this.changeControlFactory = changeControlFactory;
this.changes = changes;
this.revisions = revisions;
this.plcUtil = plcUtil;
this.editUtil = editUtil;
@@ -216,23 +200,6 @@ class PatchSetDetailFactory extends Handler<PatchSetDetail> {
}
}
detail.setCommands(Lists.newArrayList(Iterables.transform(
UiActions.sorted(UiActions.plugins(UiActions.from(
revisions,
new RevisionResource(changes.parse(control), patchSet),
Providers.of(user)))),
new Function<UiAction.Description, UiCommandDetail>() {
@Override
public UiCommandDetail apply(UiAction.Description in) {
UiCommandDetail r = new UiCommandDetail();
r.method = in.getMethod();
r.id = in.getId();
r.label = in.getLabel();
r.title = in.getTitle();
r.enabled = in.isEnabled();
return r;
}
})));
return detail;
}

View File

@@ -18,7 +18,6 @@ import com.google.common.base.Function;
import com.google.common.base.Predicate;
import com.google.common.base.Predicates;
import com.google.common.collect.Iterables;
import com.google.common.collect.Lists;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.extensions.registration.DynamicMap;
import com.google.gerrit.extensions.restapi.AuthException;
@@ -34,10 +33,6 @@ import com.google.inject.Provider;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import java.util.Collections;
import java.util.Comparator;
import java.util.List;
public class UiActions {
private static final Logger log = LoggerFactory.getLogger(UiActions.class);
@@ -50,27 +45,6 @@ public class UiActions {
};
}
public static List<UiAction.Description> sorted(Iterable<UiAction.Description> in) {
List<UiAction.Description> s = Lists.newArrayList(in);
Collections.sort(s, new Comparator<UiAction.Description>() {
@Override
public int compare(UiAction.Description a, UiAction.Description b) {
return a.getId().compareTo(b.getId());
}
});
return s;
}
public static Iterable<UiAction.Description> plugins(Iterable<UiAction.Description> in) {
return Iterables.filter(in,
new Predicate<UiAction.Description>() {
@Override
public boolean apply(UiAction.Description input) {
return input.getId().indexOf('~') > 0;
}
});
}
public static <R extends RestResource> Iterable<UiAction.Description> from(
RestCollection<?, R> collection,
R resource,