Merge "Use /changes/{id}/revisions/{sha1}/submit to submit changes"diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/Dispatcher.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/Dispatcher.java
index 5003286..6989339 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/Dispatcher.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/Dispatcher.java
@@ -432,8 +432,8 @@
}
c = dashboardId.indexOf(":");
if (0 <= c) {
- final String ref = URL.decodePathSegment(dashboardId.substring(0, c));
- final String path = URL.decodePathSegment(dashboardId.substring(c + 1));
+ final String ref = URL.decode(dashboardId.substring(0, c));
+ final String path = URL.decode(dashboardId.substring(c + 1));
DashboardList.get(new Project.NameKey(project), ref + ":" + path, cb);
return;
}
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/Gerrit.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/Gerrit.java
index 2081597..3298c3e 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/Gerrit.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/Gerrit.java
@@ -675,9 +675,9 @@
menuLeft.add(projectsBar, C.menuProjects());
if (signedIn) {
- final LinkMenuBar groupsBar = new LinkMenuBar();
- addLink(groupsBar, C.menuGroupsList(), PageLinks.ADMIN_GROUPS);
- menuLeft.add(groupsBar, C.menuGroups());
+ final LinkMenuBar peopleBar = new LinkMenuBar();
+ addLink(peopleBar, C.menuPeopleGroupsList(), PageLinks.ADMIN_GROUPS);
+ menuLeft.add(peopleBar, C.menuPeople());
final LinkMenuBar pluginsBar = new LinkMenuBar();
AccountCapabilities.all(new GerritCallback<AccountCapabilities>() {
@@ -687,12 +687,12 @@
addLink(projectsBar, C.menuProjectsCreate(), PageLinks.ADMIN_CREATE_PROJECT);
}
if (result.canPerform(CREATE_GROUP)) {
- addLink(groupsBar, C.menuGroupsCreate(), PageLinks.ADMIN_CREATE_GROUP);
+ addLink(peopleBar, C.menuPeopleGroupsCreate(), PageLinks.ADMIN_CREATE_GROUP);
}
if (result.canPerform(ADMINISTRATE_SERVER)) {
addLink(pluginsBar, C.menuPluginsInstalled(), PageLinks.ADMIN_PLUGINS);
menuLeft.insert(pluginsBar, C.menuPlugins(),
- menuLeft.getWidgetIndex(groupsBar) + 1);
+ menuLeft.getWidgetIndex(peopleBar) + 1);
}
}
}, CREATE_PROJECT, CREATE_GROUP, ADMINISTRATE_SERVER);
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/GerritConstants.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/GerritConstants.java
index a916964..8a57515 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/GerritConstants.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/GerritConstants.java
@@ -78,9 +78,9 @@
String menuProjectsCreate();
String menuPeople();
- String menuGroups();
- String menuGroupsList();
- String menuGroupsCreate();
+ String menuPeopleGroupsList();
+ String menuPeopleGroupsCreate();
+
String menuPlugins();
String menuPluginsInstalled();
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/GerritConstants.properties b/gerrit-gwtui/src/main/java/com/google/gerrit/client/GerritConstants.properties
index 3802ff5..4176051 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/GerritConstants.properties
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/GerritConstants.properties
@@ -61,9 +61,8 @@
menuProjectsCreate = Create New Project
menuPeople = People
-menuGroups = Groups
-menuGroupsList = List
-menuGroupsCreate = Create New Group
+menuPeopleGroupsList = List Groups
+menuPeopleGroupsCreate = Create New Group
menuPlugins = Plugins
menuPluginsInstalled = Installed
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/dashboards/DashboardList.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/dashboards/DashboardList.java
index 02ee2b1..9725ef0 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/dashboards/DashboardList.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/dashboards/DashboardList.java
@@ -50,11 +50,11 @@
private static String encodeDashboardId(String dashboardId) {
int c = dashboardId.indexOf(":");
if (0 <= c) {
- final String ref = URL.encodePathSegment(dashboardId.substring(0, c));
- final String path = URL.encodePathSegment(dashboardId.substring(c + 1));
+ final String ref = URL.encode(dashboardId.substring(0, c));
+ final String path = URL.encode(dashboardId.substring(c + 1));
return ref + ":" + path;
} else {
- return URL.encodePathSegment(dashboardId);
+ return URL.encode(dashboardId);
}
}
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/rpc/RestApi.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/rpc/RestApi.java
index 0445774..1764047 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/rpc/RestApi.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/rpc/RestApi.java
@@ -65,12 +65,33 @@
public void onResponseReceived(Request req, Response res) {
int status = res.getStatusCode();
if (status != 200) {
- RpcStatus.INSTANCE.onRpcComplete();
- if ((400 <= status && status < 600) && isTextBody(res)) {
- cb.onFailure(new RemoteJsonException(res.getText(), status, null));
+ String msg;
+ if (isTextBody(res)) {
+ msg = res.getText().trim();
+ } else if (isJsonBody(res)) {
+ JSONValue v;
+ try {
+ v = parseJson(res);
+ } catch (JSONException e) {
+ v = null;
+ }
+ if (v != null && v.isString() != null) {
+ msg = v.isString().stringValue();
+ } else {
+ msg = trimJsonMagic(res.getText()).trim();
+ }
} else {
- cb.onFailure(new StatusCodeException(status, res.getStatusText()));
+ msg = res.getStatusText();
}
+
+ Throwable error;
+ if (400 <= status && status < 600) {
+ error = new RemoteJsonException(msg, status, null);
+ } else {
+ error = new StatusCodeException(status, res.getStatusText());
+ }
+ RpcStatus.INSTANCE.onRpcComplete();
+ cb.onFailure(error);
return;
}
@@ -82,19 +103,9 @@
return;
}
- String json = res.getText();
- if (json.startsWith(JSON_MAGIC)) {
- json = json.substring(JSON_MAGIC.length());
- }
- if (json.isEmpty()) {
- RpcStatus.INSTANCE.onRpcComplete();
- cb.onFailure(new RemoteJsonException("JSON response was empty"));
- return;
- }
-
T data;
try {
- data = cast(JSONParser.parseStrict(json));
+ data = cast(parseJson(res));
} catch (JSONException e) {
RpcStatus.INSTANCE.onRpcComplete();
cb.onFailure(new RemoteJsonException("Invalid JSON: " + e.getMessage()));
@@ -105,21 +116,6 @@
RpcStatus.INSTANCE.onRpcComplete();
}
- @SuppressWarnings("unchecked")
- private T cast(JSONValue val) {
- if (val.isObject() != null) {
- return (T) val.isObject().getJavaScriptObject();
- } else if (val.isArray() != null) {
- return (T) val.isArray().getJavaScriptObject();
- } else if (val.isString() != null) {
- return (T) NativeString.wrap(val.isString().stringValue());
- } else if (val.isNull() != null) {
- return null;
- } else {
- throw new JSONException("Unsupported JSON response type");
- }
- }
-
@Override
public void onError(Request req, Throwable err) {
RpcStatus.INSTANCE.onRpcComplete();
@@ -268,4 +264,35 @@
}
return want.equals(type);
}
+
+ private static JSONValue parseJson(Response res)
+ throws JSONException {
+ String json = trimJsonMagic(res.getText());
+ if (json.isEmpty()) {
+ throw new JSONException("response was empty");
+ }
+ return JSONParser.parseStrict(json);
+ }
+
+ private static String trimJsonMagic(String json) {
+ if (json.startsWith(JSON_MAGIC)) {
+ json = json.substring(JSON_MAGIC.length());
+ }
+ return json;
+ }
+
+ @SuppressWarnings("unchecked")
+ private static <T extends JavaScriptObject> T cast(JSONValue val) {
+ if (val.isObject() != null) {
+ return (T) val.isObject().getJavaScriptObject();
+ } else if (val.isArray() != null) {
+ return (T) val.isArray().getJavaScriptObject();
+ } else if (val.isString() != null) {
+ return (T) NativeString.wrap(val.isString().stringValue());
+ } else if (val.isNull() != null) {
+ return null;
+ } else {
+ throw new JSONException("unsupported JSON type");
+ }
+ }
}
diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/restapi/RestApiServlet.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/restapi/RestApiServlet.java
index d472b1d..37c75dd 100644
--- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/restapi/RestApiServlet.java
+++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/restapi/RestApiServlet.java
@@ -28,6 +28,7 @@
import com.google.common.base.Joiner;
import com.google.common.base.Splitter;
import com.google.common.base.Strings;
+import com.google.common.collect.ImmutableMultimap;
import com.google.common.collect.Iterables;
import com.google.common.collect.LinkedHashMultimap;
import com.google.common.collect.Lists;
@@ -64,6 +65,7 @@
import com.google.gson.GsonBuilder;
import com.google.gson.JsonElement;
import com.google.gson.JsonParseException;
+import com.google.gson.JsonPrimitive;
import com.google.gson.stream.JsonReader;
import com.google.gson.stream.JsonToken;
import com.google.gwtjsonrpc.common.JsonConstants;
@@ -93,6 +95,7 @@
import java.util.List;
import java.util.Map;
import java.util.Set;
+import java.util.regex.Pattern;
import java.util.zip.GZIPOutputStream;
import javax.annotation.Nullable;
@@ -169,6 +172,7 @@
res.setHeader("Pragma", "no-cache");
res.setHeader("Cache-Control", "no-cache, must-revalidate");
res.setHeader("Content-Disposition", "attachment");
+ res.setHeader("X-Content-Type-Options", "nosniff");
try {
int status = SC_OK;
@@ -388,9 +392,10 @@
return c.newInstance();
}
- private static void replyJson(HttpServletRequest req,
+ private static void replyJson(@Nullable HttpServletRequest req,
HttpServletResponse res,
- Multimap<String, String> config, Object result)
+ Multimap<String, String> config,
+ Object result)
throws IOException {
final TemporaryBuffer.Heap buf = heap(Integer.MAX_VALUE);
buf.write(JSON_MAGIC);
@@ -421,7 +426,7 @@
FieldNamingPolicy.LOWER_CASE_WITH_UNDERSCORES;
private static Gson newGson(Multimap<String, String> config,
- HttpServletRequest req) {
+ @Nullable HttpServletRequest req) {
GsonBuilder gb = OutputFormat.JSON_COMPACT.newGsonBuilder()
.setFieldNamingPolicy(NAMING);
@@ -432,11 +437,12 @@
}
private static void enablePrettyPrint(GsonBuilder gb,
- Multimap<String, String> config, HttpServletRequest req) {
+ Multimap<String, String> config,
+ @Nullable HttpServletRequest req) {
String pp = Iterables.getFirst(config.get("pp"), null);
if (pp == null) {
pp = Iterables.getFirst(config.get("prettyPrint"), null);
- if (pp == null) {
+ if (pp == null && req != null) {
pp = acceptsJson(req) ? "0" : "1";
}
}
@@ -484,8 +490,10 @@
}
}
- private static void replyBinaryResult(HttpServletRequest req,
- HttpServletResponse res, BinaryResult bin) throws IOException {
+ private static void replyBinaryResult(
+ @Nullable HttpServletRequest req,
+ HttpServletResponse res,
+ BinaryResult bin) throws IOException {
try {
res.setContentType(bin.getContentType());
OutputStream dst = res.getOutputStream();
@@ -651,11 +659,20 @@
static void replyText(@Nullable HttpServletRequest req,
HttpServletResponse res, String text) throws IOException {
- if (!text.endsWith("\n")) {
- text += "\n";
+ if (isMaybeHTML(text)) {
+ replyJson(req, res, ImmutableMultimap.of("pp", "0"), new JsonPrimitive(text));
+ } else {
+ if (!text.endsWith("\n")) {
+ text += "\n";
+ }
+ replyBinaryResult(req, res,
+ BinaryResult.create(text).setContentType("text/plain"));
}
- replyBinaryResult(req, res,
- BinaryResult.create(text).setContentType("text/plain"));
+ }
+
+ private static final Pattern IS_HTML = Pattern.compile("[<&]");
+ private static boolean isMaybeHTML(String text) {
+ return IS_HTML.matcher(text).find();
}
private static boolean acceptsJson(HttpServletRequest req) {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/ApprovalsUtil.java b/gerrit-server/src/main/java/com/google/gerrit/server/ApprovalsUtil.java
index d7a487d..afbfd8d3 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/ApprovalsUtil.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/ApprovalsUtil.java
@@ -48,7 +48,7 @@
private final ApprovalTypes approvalTypes;
@Inject
- ApprovalsUtil(ReviewDb db, ApprovalTypes approvalTypes) {
+ public ApprovalsUtil(ReviewDb db, ApprovalTypes approvalTypes) {
this.db = db;
this.approvalTypes = approvalTypes;
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/ChangeUtil.java b/gerrit-server/src/main/java/com/google/gerrit/server/ChangeUtil.java
index 76ed67e..920299d 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/ChangeUtil.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/ChangeUtil.java
@@ -30,7 +30,6 @@
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.git.MergeOp;
import com.google.gerrit.server.mail.EmailException;
-import com.google.gerrit.server.mail.ReplyToChangeSender;
import com.google.gerrit.server.mail.RevertedSender;
import com.google.gerrit.server.patch.PatchSetInfoFactory;
import com.google.gerrit.server.patch.PatchSetInfoNotAvailableException;
@@ -56,8 +55,6 @@
import org.eclipse.jgit.util.Base64;
import org.eclipse.jgit.util.ChangeIdUtil;
import org.eclipse.jgit.util.NB;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
import java.io.IOException;
import java.sql.Timestamp;
@@ -70,8 +67,6 @@
import java.util.regex.Matcher;
public class ChangeUtil {
- private static final Logger log = LoggerFactory.getLogger(ChangeUtil.class);
-
private static int uuidPrefix;
private static int uuidSeq;
@@ -513,25 +508,6 @@
db.patchSets().delete(Collections.singleton(patch));
}
- public static <T extends ReplyToChangeSender> void updatedChange(
- final ReviewDb db, final IdentifiedUser user, final Change change,
- final ChangeMessage cmsg, ReplyToChangeSender.Factory<T> senderFactory)
- throws OrmException {
- db.changeMessages().insert(Collections.singleton(cmsg));
-
- new ApprovalsUtil(db, null).syncChangeStatus(change);
-
- // Email the reviewers
- try {
- final ReplyToChangeSender cm = senderFactory.create(change);
- cm.setFrom(user.getAccountId());
- cm.setChangeMessage(cmsg);
- cm.send();
- } catch (Exception e) {
- log.error("Cannot email update for change " + change.getChangeId(), e);
- }
- }
-
public static String sortKey(long lastUpdated, int id){
// The encoding uses minutes since Wed Oct 1 00:00:00 2008 UTC.
// We overrun approximately 4,085 years later, so ~6093.
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountsCollection.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountsCollection.java
index 7fa36b4..77a2504 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountsCollection.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountsCollection.java
@@ -15,10 +15,12 @@
package com.google.gerrit.server.account;
import com.google.gerrit.extensions.registration.DynamicMap;
+import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
-import com.google.gerrit.extensions.restapi.TopLevelResource;
import com.google.gerrit.extensions.restapi.RestCollection;
import com.google.gerrit.extensions.restapi.RestView;
+import com.google.gerrit.extensions.restapi.TopLevelResource;
+import com.google.gerrit.server.AnonymousUser;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.IdentifiedUser;
import com.google.inject.Inject;
@@ -38,15 +40,19 @@
@Override
public AccountResource parse(TopLevelResource root, String id)
- throws ResourceNotFoundException {
+ throws ResourceNotFoundException, AuthException {
if ("self".equals(id)) {
CurrentUser user = self.get();
if (user instanceof IdentifiedUser) {
return new AccountResource((IdentifiedUser) user);
+ } else if (user instanceof AnonymousUser) {
+ throw new AuthException("Authentication required");
+ } else {
+ throw new ResourceNotFoundException(id);
}
+ } else {
throw new ResourceNotFoundException(id);
}
- throw new ResourceNotFoundException(id);
}
@Override
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/Abandon.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/Abandon.java
index 1249e9b..3dc003b 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/Abandon.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/Abandon.java
@@ -23,18 +23,27 @@
import com.google.gerrit.extensions.restapi.RestModifyView;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.ChangeMessage;
-import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.server.ReviewDb;
+import com.google.gerrit.server.ApprovalsUtil;
import com.google.gerrit.server.ChangeUtil;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.change.Abandon.Input;
import com.google.gerrit.server.mail.AbandonedSender;
+import com.google.gerrit.server.mail.ReplyToChangeSender;
import com.google.gerrit.server.project.ChangeControl;
import com.google.gwtorm.server.AtomicUpdate;
+import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Provider;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.util.Collections;
+
public class Abandon implements RestModifyView<ChangeResource, Input> {
+ private static final Logger log = LoggerFactory.getLogger(Abandon.class);
+
private final ChangeHooks hooks;
private final AbandonedSender.Factory abandonedSenderFactory;
private final Provider<ReviewDb> dbProvider;
@@ -66,6 +75,7 @@
throws BadRequestException, AuthException,
ResourceConflictException, Exception {
ChangeControl control = req.getControl();
+ IdentifiedUser caller = (IdentifiedUser) control.getCurrentUser();
Change change = req.getChange();
if (!control.canAbandon()) {
throw new AuthException("abandon not permitted");
@@ -73,46 +83,68 @@
throw new ResourceConflictException("change is " + status(change));
}
- // Create a message to accompany the abandoned change
+ ChangeMessage message;
ReviewDb db = dbProvider.get();
- PatchSet.Id patchSetId = change.currentPatchSetId();
- IdentifiedUser currentUser = (IdentifiedUser) control.getCurrentUser();
- String message = Strings.emptyToNull(input.message);
- ChangeMessage cmsg = new ChangeMessage(
- new ChangeMessage.Key(change.getId(), ChangeUtil.messageUUID(db)),
- currentUser.getAccountId(), patchSetId);
- StringBuilder msg = new StringBuilder();
- msg.append(String.format("Patch Set %d: Abandoned", patchSetId.get()));
- if (message != null) {
- msg.append("\n\n");
- msg.append(message);
- }
- cmsg.setMessage(msg.toString());
-
- // Abandon the change
- Change updatedChange = db.changes().atomicUpdate(
- change.getId(),
- new AtomicUpdate<Change>() {
- @Override
- public Change update(Change change) {
- if (change.getStatus().isOpen()) {
- change.setStatus(Change.Status.ABANDONED);
- ChangeUtil.updated(change);
- return change;
+ db.changes().beginTransaction(change.getId());
+ try {
+ change = db.changes().atomicUpdate(
+ change.getId(),
+ new AtomicUpdate<Change>() {
+ @Override
+ public Change update(Change change) {
+ if (change.getStatus().isOpen()) {
+ change.setStatus(Change.Status.ABANDONED);
+ ChangeUtil.updated(change);
+ return change;
+ }
+ return null;
}
- return null;
- }
- });
- if (updatedChange == null) {
- throw new ResourceConflictException("change is "
- + status(db.changes().get(change.getId())));
+ });
+ if (change == null) {
+ throw new ResourceConflictException("change is "
+ + status(db.changes().get(req.getChange().getId())));
+ }
+ message = newMessage(input, caller, change);
+ db.changeMessages().insert(Collections.singleton(message));
+ new ApprovalsUtil(db, null).syncChangeStatus(change);
+ db.commit();
+ } finally {
+ db.rollback();
}
- ChangeUtil.updatedChange(db, currentUser, updatedChange, cmsg,
- abandonedSenderFactory);
- hooks.doChangeAbandonedHook(updatedChange, currentUser.getAccount(),
- message, db);
- return json.format(change.getId());
+ try {
+ ReplyToChangeSender cm = abandonedSenderFactory.create(change);
+ cm.setFrom(caller.getAccountId());
+ cm.setChangeMessage(message);
+ cm.send();
+ } catch (Exception e) {
+ log.error("Cannot email update for change " + change.getChangeId(), e);
+ }
+ hooks.doChangeAbandonedHook(change,
+ caller.getAccount(),
+ Strings.emptyToNull(input.message),
+ db);
+ return json.format(change);
+ }
+
+ private ChangeMessage newMessage(Input input, IdentifiedUser caller,
+ Change change) throws OrmException {
+ StringBuilder msg = new StringBuilder();
+ msg.append("Abandoned");
+ if (!Strings.nullToEmpty(input.message).trim().isEmpty()) {
+ msg.append("\n\n");
+ msg.append(input.message.trim());
+ }
+
+ ChangeMessage message = new ChangeMessage(
+ new ChangeMessage.Key(
+ change.getId(),
+ ChangeUtil.messageUUID(dbProvider.get())),
+ caller.getAccountId(),
+ change.getLastUpdatedOn(),
+ change.currentPatchSetId());
+ message.setMessage(msg.toString());
+ return message;
}
private static String status(Change change) {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteReviewer.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteReviewer.java
new file mode 100644
index 0000000..c58f78d
--- /dev/null
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteReviewer.java
@@ -0,0 +1,88 @@
+// Copyright (C) 2012 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.server.change;
+
+import com.google.common.base.Predicate;
+import com.google.common.collect.Iterables;
+import com.google.common.collect.Lists;
+import com.google.gerrit.extensions.restapi.AuthException;
+import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
+import com.google.gerrit.extensions.restapi.RestModifyView;
+import com.google.gerrit.reviewdb.client.Change;
+import com.google.gerrit.reviewdb.client.PatchSetApproval;
+import com.google.gerrit.reviewdb.server.ReviewDb;
+import com.google.gerrit.server.change.DeleteReviewer.Input;
+import com.google.gerrit.server.project.ChangeControl;
+import com.google.gwtorm.server.OrmException;
+import com.google.inject.Inject;
+import com.google.inject.Provider;
+
+import java.util.List;
+
+class DeleteReviewer implements RestModifyView<ReviewerResource, Input> {
+ static class Input {
+ }
+
+ private final Provider<ReviewDb> dbProvider;
+
+ @Inject
+ DeleteReviewer(Provider<ReviewDb> dbProvider) {
+ this.dbProvider = dbProvider;
+ }
+
+ @Override
+ public Class<Input> inputType() {
+ return Input.class;
+ }
+
+ @Override
+ public Object apply(ReviewerResource rsrc, Input input)
+ throws AuthException, ResourceNotFoundException, OrmException {
+ ChangeControl control = rsrc.getControl();
+ Change.Id changeId = rsrc.getChange().getId();
+ ReviewDb db = dbProvider.get();
+ db.changes().beginTransaction(changeId);
+ try {
+ List<PatchSetApproval> del = Lists.newArrayList();
+ for (PatchSetApproval a : approvals(db, rsrc)) {
+ if (control.canRemoveReviewer(a)) {
+ del.add(a);
+ } else {
+ throw new AuthException("delete not permitted");
+ }
+ }
+ if (del.isEmpty()) {
+ throw new ResourceNotFoundException();
+ }
+ db.patchSetApprovals().delete(del);
+ db.commit();
+ } finally {
+ db.rollback();
+ }
+ return new Object();
+ }
+
+ private Iterable<PatchSetApproval> approvals(ReviewDb db,
+ final ReviewerResource rsrc) throws OrmException {
+ return Iterables.filter(
+ db.patchSetApprovals().byChange(rsrc.getChange().getId()),
+ new Predicate<PatchSetApproval>() {
+ @Override
+ public boolean apply(PatchSetApproval input) {
+ return input.getAccountId().equals(rsrc.getAccount().getId());
+ }
+ });
+ }
+}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/ListDrafts.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/ListDrafts.java
index 208f271..e6442d12 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/ListDrafts.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/ListDrafts.java
@@ -56,6 +56,7 @@
list = Lists.newArrayList();
out.put(o.path, list);
}
+ o.path = null;
list.add(o);
}
for (List<Comment> list : out.values()) {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/Module.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/Module.java
index 861dac7..2330d34 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/Module.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/Module.java
@@ -46,6 +46,7 @@
post(CHANGE_KIND, "submit").to(Submit.CurrentRevision.class);
get(REVIEWER_KIND).to(GetReviewer.class);
+ delete(REVIEWER_KIND).to(DeleteReviewer.class);
child(CHANGE_KIND, "revisions").to(Revisions.class);
post(REVISION_KIND, "review").to(PostReview.class);
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReview.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReview.java
index 7bba253..f84670f 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReview.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReview.java
@@ -139,12 +139,14 @@
ChangeUtil.updated(change);
timestamp = change.getLastUpdatedOn();
- insertComments(revision, input.comments, input.drafts);
- updateLabels(revision, input.labels);
-
- insertMessage(revision, input.message);
- db.changes().update(Collections.singleton(change));
- db.commit();
+ boolean dirty = false;
+ dirty |= insertComments(revision, input.comments, input.drafts);
+ dirty |= updateLabels(revision, input.labels);
+ dirty |= insertMessage(revision, input.message);
+ if (dirty) {
+ db.changes().update(Collections.singleton(change));
+ db.commit();
+ }
} finally {
db.rollback();
}
@@ -247,7 +249,7 @@
}
}
- private void insertComments(RevisionResource rsrc,
+ private boolean insertComments(RevisionResource rsrc,
Map<String, List<Comment>> in, DraftHandling draftsHandling)
throws OrmException {
if (in == null) {
@@ -308,6 +310,7 @@
db.patchComments().update(upd);
comments.addAll(ins);
comments.addAll(upd);
+ return !del.isEmpty() || !ins.isEmpty() || !upd.isEmpty();
}
private Map<String, PatchLineComment> scanDraftComments(
@@ -321,7 +324,7 @@
return drafts;
}
- private void updateLabels(RevisionResource rsrc, Map<String, Short> labels)
+ private boolean updateLabels(RevisionResource rsrc, Map<String, Short> labels)
throws OrmException {
if (labels == null) {
labels = Collections.emptyMap();
@@ -379,6 +382,7 @@
db.patchSetApprovals().delete(del);
db.patchSetApprovals().insert(ins);
db.patchSetApprovals().update(upd);
+ return !del.isEmpty() || !ins.isEmpty() || !upd.isEmpty();
}
private void forceCallerAsReviewer(RevisionResource rsrc,
@@ -439,14 +443,11 @@
return sb.toString();
}
- private void insertMessage(RevisionResource rsrc, String msg)
+ private boolean insertMessage(RevisionResource rsrc, String msg)
throws OrmException {
msg = Strings.nullToEmpty(msg).trim();
StringBuilder buf = new StringBuilder();
- buf.append(String.format(
- "Patch Set %d:",
- rsrc.getPatchSet().getPatchSetId()));
for (String d : labelDelta) {
buf.append(" ").append(d);
}
@@ -458,14 +459,21 @@
if (!msg.isEmpty()) {
buf.append("\n\n").append(msg);
}
+ if (buf.length() == 0) {
+ return false;
+ }
message = new ChangeMessage(
new ChangeMessage.Key(change.getId(), ChangeUtil.messageUUID(db)),
rsrc.getAuthorId(),
timestamp,
rsrc.getPatchSet().getId());
- message.setMessage(buf.toString());
+ message.setMessage(String.format(
+ "Patch Set %d:%s",
+ rsrc.getPatchSet().getPatchSetId(),
+ buf.toString()));
db.changeMessages().insert(Collections.singleton(message));
+ return true;
}
@Deprecated
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/Restore.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/Restore.java
index 61e01db..5b78b5b 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/Restore.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/Restore.java
@@ -23,18 +23,27 @@
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.Change.Status;
import com.google.gerrit.reviewdb.client.ChangeMessage;
-import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.server.ReviewDb;
+import com.google.gerrit.server.ApprovalsUtil;
import com.google.gerrit.server.ChangeUtil;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.change.Restore.Input;
+import com.google.gerrit.server.mail.ReplyToChangeSender;
import com.google.gerrit.server.mail.RestoredSender;
import com.google.gerrit.server.project.ChangeControl;
import com.google.gwtorm.server.AtomicUpdate;
+import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Provider;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.util.Collections;
+
public class Restore implements RestModifyView<ChangeResource, Input> {
+ private static final Logger log = LoggerFactory.getLogger(Restore.class);
+
private final ChangeHooks hooks;
private final RestoredSender.Factory restoredSenderFactory;
private final Provider<ReviewDb> dbProvider;
@@ -65,6 +74,7 @@
public Object apply(ChangeResource req, Input input)
throws Exception {
ChangeControl control = req.getControl();
+ IdentifiedUser caller = (IdentifiedUser) control.getCurrentUser();
Change change = req.getChange();
if (!control.canRestore()) {
throw new AuthException("restore not permitted");
@@ -72,46 +82,68 @@
throw new ResourceConflictException("change is " + status(change));
}
- // Create a message to accompany the restore change
+ ChangeMessage message;
ReviewDb db = dbProvider.get();
- PatchSet.Id patchSetId = change.currentPatchSetId();
- IdentifiedUser currentUser = (IdentifiedUser) control.getCurrentUser();
- String message = Strings.emptyToNull(input.message);
- final ChangeMessage cmsg = new ChangeMessage(
- new ChangeMessage.Key(change.getId(), ChangeUtil.messageUUID(db)),
- currentUser.getAccountId(), patchSetId);
- StringBuilder msg = new StringBuilder();
- msg.append(String.format("Patch Set %d: Restored", patchSetId.get()));
- if (message != null) {
- msg.append("\n\n");
- msg.append(message);
- }
- cmsg.setMessage(msg.toString());
-
- // Restore the change
- final Change updatedChange = db.changes().atomicUpdate(
- change.getId(),
- new AtomicUpdate<Change>() {
- @Override
- public Change update(Change change) {
- if (change.getStatus() == Change.Status.ABANDONED) {
- change.setStatus(Change.Status.NEW);
- ChangeUtil.updated(change);
- return change;
+ db.changes().beginTransaction(change.getId());
+ try {
+ change = db.changes().atomicUpdate(
+ change.getId(),
+ new AtomicUpdate<Change>() {
+ @Override
+ public Change update(Change change) {
+ if (change.getStatus() == Change.Status.ABANDONED) {
+ change.setStatus(Change.Status.NEW);
+ ChangeUtil.updated(change);
+ return change;
+ }
+ return null;
}
- return null;
- }
- });
- if (updatedChange == null) {
- throw new ResourceConflictException("change is "
- + status(db.changes().get(change.getId())));
+ });
+ if (change == null) {
+ throw new ResourceConflictException("change is "
+ + status(db.changes().get(req.getChange().getId())));
+ }
+ message = newMessage(input, caller, change);
+ db.changeMessages().insert(Collections.singleton(message));
+ new ApprovalsUtil(db, null).syncChangeStatus(change);
+ db.commit();
+ } finally {
+ db.rollback();
}
- ChangeUtil.updatedChange(db, currentUser, updatedChange, cmsg,
- restoredSenderFactory);
- hooks.doChangeRestoredHook(updatedChange, currentUser.getAccount(),
- message, db);
- return json.format(change.getId());
+ try {
+ ReplyToChangeSender cm = restoredSenderFactory.create(change);
+ cm.setFrom(caller.getAccountId());
+ cm.setChangeMessage(message);
+ cm.send();
+ } catch (Exception e) {
+ log.error("Cannot email update for change " + change.getChangeId(), e);
+ }
+ hooks.doChangeRestoredHook(change,
+ caller.getAccount(),
+ Strings.emptyToNull(input.message),
+ dbProvider.get());
+ return json.format(change);
+ }
+
+ private ChangeMessage newMessage(Input input, IdentifiedUser caller,
+ Change change) throws OrmException {
+ StringBuilder msg = new StringBuilder();
+ msg.append("Restored");
+ if (!Strings.nullToEmpty(input.message).trim().isEmpty()) {
+ msg.append("\n\n");
+ msg.append(input.message.trim());
+ }
+
+ ChangeMessage message = new ChangeMessage(
+ new ChangeMessage.Key(
+ change.getId(),
+ ChangeUtil.messageUUID(dbProvider.get())),
+ caller.getAccountId(),
+ change.getLastUpdatedOn(),
+ change.currentPatchSetId());
+ message.setMessage(msg.toString());
+ return message;
}
private static String status(Change change) {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/Reviewers.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/Reviewers.java
index 37c1a5a..d1d0457 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/Reviewers.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/Reviewers.java
@@ -14,20 +14,23 @@
package com.google.gerrit.server.change;
+import com.google.common.collect.Sets;
import com.google.gerrit.extensions.registration.DynamicMap;
+import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.ChildCollection;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.extensions.restapi.RestView;
import com.google.gerrit.reviewdb.client.Account;
-import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.PatchSetApproval;
import com.google.gerrit.reviewdb.server.ReviewDb;
+import com.google.gerrit.server.AnonymousUser;
+import com.google.gerrit.server.CurrentUser;
+import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.account.AccountCache;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Provider;
-import java.util.HashSet;
import java.util.Set;
public class Reviewers implements
@@ -56,30 +59,38 @@
}
@Override
- public ReviewerResource parse(ChangeResource changeResource, String id)
- throws OrmException, ResourceNotFoundException {
- // Get the account id
- if (!id.matches("^[0-9]+$")) {
+ public ReviewerResource parse(ChangeResource rsrc, String id)
+ throws OrmException, ResourceNotFoundException, AuthException {
+ Account.Id accountId;
+ if (id.equals("self")) {
+ CurrentUser user = rsrc.getControl().getCurrentUser();
+ if (user instanceof IdentifiedUser) {
+ accountId = ((IdentifiedUser)user).getAccountId();
+ } else if (user instanceof AnonymousUser) {
+ throw new AuthException("Authentication required");
+ } else {
+ throw new ResourceNotFoundException(id);
+ }
+ } else if (id.matches("^[0-9]+$")) {
+ accountId = Account.Id.parse(id);
+ } else {
throw new ResourceNotFoundException(id);
}
- Account.Id accountId = Account.Id.parse(id);
// See if the id exists as a reviewer for this change
- if (fetchAccountIds(changeResource).contains(accountId)) {
+ if (fetchAccountIds(rsrc).contains(accountId)) {
Account account = accountCache.get(accountId).getAccount();
- return new ReviewerResource(changeResource, account);
+ return new ReviewerResource(rsrc, account);
}
throw new ResourceNotFoundException(id);
}
- private Set<Account.Id> fetchAccountIds(ChangeResource changeResource)
+ private Set<Account.Id> fetchAccountIds(ChangeResource rsrc)
throws OrmException {
- ReviewDb db = dbProvider.get();
- Change.Id changeId = changeResource.getChange().getId();
- Set<Account.Id> accountIds = new HashSet<Account.Id>();
- for (PatchSetApproval patchSetApproval
- : db.patchSetApprovals().byChange(changeId)) {
- accountIds.add(patchSetApproval.getAccountId());
+ Set<Account.Id> accountIds = Sets.newHashSet();
+ for (PatchSetApproval a
+ : dbProvider.get().patchSetApprovals().byChange(rsrc.getChange().getId())) {
+ accountIds.add(a.getAccountId());
}
return accountIds;
}
diff --git a/gerrit-war/src/main/resources/log4j.properties b/gerrit-war/src/main/resources/log4j.properties
index 45f630e..250fa4c 100644
--- a/gerrit-war/src/main/resources/log4j.properties
+++ b/gerrit-war/src/main/resources/log4j.properties
@@ -57,3 +57,6 @@
# Silence non-critical messages from Velocity
#
log4j.logger.velocity=WARN
+
+# Silence non-critical messages from apache.http
+log4j.logger.org.apache.http=WARN
