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 4905ce92d8..c50d2e3edf 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 @@ -42,9 +42,4 @@ public interface ChangeDetailService extends RemoteJsonService { @SignInRequired void patchSetPublishDetail(PatchSet.Id key, AsyncCallback callback); - - @Audit - @SignInRequired - void alterTopic(Change.Id id, String topic, String message, - AsyncCallback callback); } diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeApi.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeApi.java index bcd755f7b3..679b81e1c4 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeApi.java +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeApi.java @@ -14,6 +14,7 @@ package com.google.gerrit.client.changes; +import com.google.gerrit.client.rpc.NativeString; import com.google.gerrit.client.rpc.RestApi; import com.google.gwt.core.client.JavaScriptObject; import com.google.gwtjsonrpc.common.AsyncCallback; @@ -25,10 +26,6 @@ import com.google.gwtjsonrpc.common.AsyncCallback; public class ChangeApi { private static final String URI = "/changes/"; - protected static class Message extends JavaScriptObject { - public final native void setMessage(String value) /*-{ this.message = value; }-*/; - } - /** * Sends a REST call to abandon a change and notify a callback. TODO: switch * to use the new id triplet (project~branch~change) once that data is @@ -36,8 +33,31 @@ public class ChangeApi { */ public static void abandon(int changeId, String message, AsyncCallback callback) { - Message msg = (Message) JavaScriptObject.createObject(); - msg.setMessage(message); - new RestApi(URI + changeId + "/abandon").data(msg).post(callback); + Input input = Input.create(); + input.setMessage(emptyToNull(message)); + new RestApi(URI + changeId + "/abandon").data(input).post(callback); + } + + public static void topic(int id, String topic, String msg, AsyncCallback cb) { + Input input = Input.create(); + input.setTopic(emptyToNull(topic)); + input.setMessage(emptyToNull(msg)); + new RestApi(URI + id + "/topic").data(input).put(NativeString.unwrap(cb)); + } + + private static class Input extends JavaScriptObject { + final native void setTopic(String t) /*-{ this.topic = t; }-*/; + final native void setMessage(String m) /*-{ this.message = m; }-*/; + + static Input create() { + return (Input) JavaScriptObject.createObject(); + } + + protected Input() { + } + } + + private static String emptyToNull(String str) { + return str == null || str.isEmpty() ? null : str; } } diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeInfoBlock.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeInfoBlock.java index c197af22ed..34e1bf23ef 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeInfoBlock.java +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeInfoBlock.java @@ -21,6 +21,7 @@ import com.google.gerrit.client.ui.AccountLink; import com.google.gerrit.client.ui.CommentedActionDialog; import com.google.gerrit.client.ui.BranchLink; import com.google.gerrit.client.ui.ProjectLink; +import com.google.gerrit.common.PageLinks; import com.google.gerrit.common.data.AccountInfoCache; import com.google.gerrit.common.data.ChangeDetail; import com.google.gerrit.common.data.SubmitTypeRecord; @@ -39,6 +40,7 @@ import com.google.gwt.user.client.ui.InlineLabel; import com.google.gwt.user.client.ui.TextBox; import com.google.gwt.user.client.ui.Widget; import com.google.gwtexpui.clippy.client.CopyableLabel; +import com.google.gwtjsonrpc.common.AsyncCallback; public class ChangeInfoBlock extends Composite { private static final int R_CHANGE_ID = 0; @@ -185,9 +187,19 @@ public class ChangeInfoBlock extends Composite { @Override public void onSend() { String topic = newTopic.getText(); - Util.DETAIL_SVC.alterTopic(change.getId(), topic, - getMessageText(), createCallback()); + ChangeApi.topic(change.getId().get(), topic, getMessageText(), + new AsyncCallback() { + @Override + public void onSuccess(String result) { + sent = true; + Gerrit.display(PageLinks.toChange(change.getId())); + hide(); + } + + @Override + public void onFailure(Throwable caught) { + enableButtons(true); + }}); } } - } diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/rpc/NativeString.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/rpc/NativeString.java new file mode 100644 index 0000000000..5242e9d99d --- /dev/null +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/rpc/NativeString.java @@ -0,0 +1,41 @@ +// 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.client.rpc; + +import com.google.gwt.core.client.JavaScriptObject; +import com.google.gwtjsonrpc.common.AsyncCallback; + +/** Wraps a String that was returned from a JSON API. */ +public final class NativeString extends JavaScriptObject { + public final native String asString() /*-{ return this; }-*/; + + public static final AsyncCallback + unwrap(final AsyncCallback cb) { + return new AsyncCallback() { + @Override + public void onSuccess(NativeString result) { + cb.onSuccess(result.asString()); + } + + @Override + public void onFailure(Throwable caught) { + cb.onFailure(caught); + } + }; + } + + protected NativeString() { + } +} diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/AlterTopicHandler.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/AlterTopicHandler.java deleted file mode 100644 index 3b095b97a3..0000000000 --- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/AlterTopicHandler.java +++ /dev/null @@ -1,79 +0,0 @@ -// 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.httpd.rpc.changedetail; - -import com.google.gerrit.common.data.ChangeDetail; -import com.google.gerrit.common.errors.NoSuchEntityException; -import com.google.gerrit.httpd.rpc.Handler; -import com.google.gerrit.reviewdb.client.Change; -import com.google.gerrit.server.changedetail.AlterTopic; -import com.google.gerrit.server.mail.EmailException; -import com.google.gerrit.server.patch.PatchSetInfoNotAvailableException; -import com.google.gerrit.server.project.InvalidChangeOperationException; -import com.google.gerrit.server.project.NoSuchChangeException; -import com.google.gwtorm.server.OrmException; -import com.google.inject.Inject; -import com.google.inject.Provider; -import com.google.inject.assistedinject.Assisted; - -import org.eclipse.jgit.errors.RepositoryNotFoundException; - -import java.io.IOException; - -import javax.annotation.Nullable; - -class AlterTopicHandler extends Handler { - - interface Factory { - AlterTopicHandler create(@Assisted Change.Id changeId, - @Assisted("topic") String topic, - @Assisted("message") @Nullable String message); - } - - private final Provider alterTopicProvider; - private final ChangeDetailFactory.Factory changeDetailFactory; - - private final Change.Id changeId; - private final String topic; - @Nullable - private final String message; - - @Inject - AlterTopicHandler(final Provider alterTopicProvider, - final ChangeDetailFactory.Factory changeDetailFactory, - @Assisted final Change.Id changeId, - @Assisted("topic") final String topic, - @Assisted("message") @Nullable final String message) { - this.alterTopicProvider = alterTopicProvider; - this.changeDetailFactory = changeDetailFactory; - - this.changeId = changeId; - this.topic = topic; - this.message = message; - } - - @Override - public ChangeDetail call() throws EmailException, IOException, - NoSuchChangeException, NoSuchEntityException, OrmException, - PatchSetInfoNotAvailableException, RepositoryNotFoundException, - InvalidChangeOperationException { - final AlterTopic alterTopic = alterTopicProvider.get(); - alterTopic.setChangeId(changeId); - alterTopic.setTopic(topic); - alterTopic.setMessage(message); - alterTopic.call(); - return changeDetailFactory.create(changeId).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 fd559b2b03..8090451604 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 @@ -30,19 +30,16 @@ class ChangeDetailServiceImpl implements ChangeDetailService { private final IncludedInDetailFactory.Factory includedInDetail; private final PatchSetDetailFactory.Factory patchSetDetail; private final PatchSetPublishDetailFactory.Factory patchSetPublishDetail; - private final AlterTopicHandler.Factory alterTopic; @Inject ChangeDetailServiceImpl(final ChangeDetailFactory.Factory changeDetail, final IncludedInDetailFactory.Factory includedInDetail, final PatchSetDetailFactory.Factory patchSetDetail, - final PatchSetPublishDetailFactory.Factory patchSetPublishDetail, - final AlterTopicHandler.Factory alterTopic) { + final PatchSetPublishDetailFactory.Factory patchSetPublishDetail) { this.changeDetail = changeDetail; this.includedInDetail = includedInDetail; this.patchSetDetail = patchSetDetail; this.patchSetPublishDetail = patchSetPublishDetail; - this.alterTopic = alterTopic; } public void changeDetail(final Change.Id id, @@ -69,9 +66,4 @@ class ChangeDetailServiceImpl implements ChangeDetailService { final AsyncCallback callback) { patchSetPublishDetail.create(id).to(callback); } - - public void alterTopic(final Change.Id id, final String topic, - final String message, final AsyncCallback callback) { - alterTopic.create(id, topic, message).to(callback); - } } diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/ChangeModule.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/ChangeModule.java index d8fb5a5f46..d3085aaf42 100644 --- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/ChangeModule.java +++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/ChangeModule.java @@ -28,7 +28,6 @@ public class ChangeModule extends RpcServletModule { install(new FactoryModule() { @Override protected void configure() { - factory(AlterTopicHandler.Factory.class); factory(RestoreChangeHandler.Factory.class); factory(RevertChange.Factory.class); factory(RebaseChangeHandler.Factory.class); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/GetTopic.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/GetTopic.java new file mode 100644 index 0000000000..96a5c769e7 --- /dev/null +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/GetTopic.java @@ -0,0 +1,26 @@ +// 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.Strings; +import com.google.gerrit.extensions.restapi.RestReadView; +import com.google.gwtorm.server.OrmException; + +class GetTopic implements RestReadView { + @Override + public Object apply(ChangeResource rsrc) throws OrmException { + return Strings.nullToEmpty(rsrc.getChange().getTopic()); + } +} 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 79814002eb..f0e7e468da 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 @@ -30,6 +30,9 @@ public class Module extends RestApiModule { DynamicMap.mapOf(binder(), REVISION_KIND); get(CHANGE_KIND).to(GetChange.class); + get(CHANGE_KIND, "topic").to(GetTopic.class); + put(CHANGE_KIND, "topic").to(PutTopic.class); + delete(CHANGE_KIND, "topic").to(PutTopic.class); post(CHANGE_KIND, "abandon").to(Abandon.class); child(CHANGE_KIND, "reviewers").to(Reviewers.class); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/PutTopic.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/PutTopic.java new file mode 100644 index 0000000000..50c0011701 --- /dev/null +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/PutTopic.java @@ -0,0 +1,120 @@ +// 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.Strings; +import com.google.gerrit.extensions.restapi.AuthException; +import com.google.gerrit.extensions.restapi.BadRequestException; +import com.google.gerrit.extensions.restapi.DefaultInput; +import com.google.gerrit.extensions.restapi.ResourceConflictException; +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.server.ReviewDb; +import com.google.gerrit.server.ChangeUtil; +import com.google.gerrit.server.IdentifiedUser; +import com.google.gerrit.server.change.PutTopic.Input; +import com.google.gerrit.server.project.ChangeControl; +import com.google.gwtorm.server.AtomicUpdate; +import com.google.inject.Inject; +import com.google.inject.Provider; + +import java.util.Collections; + +class PutTopic implements RestModifyView { + private final Provider dbProvider; + + static class Input { + @DefaultInput + String topic; + String message; + } + + @Inject + PutTopic(Provider dbProvider) { + this.dbProvider = dbProvider; + } + + @Override + public Class inputType() { + return Input.class; + } + + @Override + public Object apply(ChangeResource req, Input input) + throws BadRequestException, AuthException, + ResourceConflictException, Exception { + if (input == null) { + input = new Input(); + } + + ChangeControl control = req.getControl(); + Change change = req.getChange(); + if (!control.canEditTopicName()) { + throw new AuthException("changing topic not permitted"); + } else if (!change.getStatus().isOpen()) { + throw new ResourceConflictException("change is " + status(change)); + } + + ReviewDb db = dbProvider.get(); + final String newTopicName = Strings.nullToEmpty(input.topic); + String oldTopicName = Strings.nullToEmpty(change.getTopic()); + if (!oldTopicName.equals(newTopicName)) { + String summary; + if (oldTopicName.isEmpty()) { + summary = "Topic set to \"" + newTopicName + "\"."; + } else if (newTopicName.isEmpty()) { + summary = "Topic \"" + oldTopicName + "\" removed."; + } else { + summary = String.format( + "Topic updated from \"%s\" to \"%s\".", + oldTopicName, newTopicName); + } + + ChangeMessage cmsg = new ChangeMessage( + new ChangeMessage.Key(change.getId(), ChangeUtil.messageUUID(db)), + ((IdentifiedUser) control.getCurrentUser()).getAccountId(), + change.currentPatchSetId()); + StringBuilder msgBuf = new StringBuilder(summary); + if (!Strings.isNullOrEmpty(input.message)) { + msgBuf.append("\n\n"); + msgBuf.append(input.message); + } + cmsg.setMessage(msgBuf.toString()); + + Change updatedChange = db.changes().atomicUpdate(change.getId(), + new AtomicUpdate() { + @Override + public Change update(Change change) { + if (change.getStatus().isOpen()) { + change.setTopic(Strings.emptyToNull(newTopicName)); + return change; + } + return null; + } + }); + if (updatedChange == null) { + change = db.changes().get(change.getId()); + throw new ResourceConflictException("change is " + status(change)); + } + db.changeMessages().insert(Collections.singleton(cmsg)); + } + return Strings.nullToEmpty(newTopicName); + } + + private static String status(Change change) { + return change != null ? change.getStatus().name().toLowerCase() : "deleted"; + } +} diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/changedetail/AlterTopic.java b/gerrit-server/src/main/java/com/google/gerrit/server/changedetail/AlterTopic.java deleted file mode 100644 index ad3fc7d51b..0000000000 --- a/gerrit-server/src/main/java/com/google/gerrit/server/changedetail/AlterTopic.java +++ /dev/null @@ -1,135 +0,0 @@ -// 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.changedetail; - -import com.google.gerrit.common.data.ReviewResult; -import com.google.gerrit.reviewdb.client.Change; -import com.google.gerrit.reviewdb.client.ChangeMessage; -import com.google.gerrit.reviewdb.server.ReviewDb; -import com.google.gerrit.server.ChangeUtil; -import com.google.gerrit.server.IdentifiedUser; -import com.google.gerrit.server.mail.EmailException; -import com.google.gerrit.server.project.ChangeControl; -import com.google.gerrit.server.project.InvalidChangeOperationException; -import com.google.gerrit.server.project.NoSuchChangeException; -import com.google.gwtorm.server.AtomicUpdate; -import com.google.gwtorm.server.OrmException; -import com.google.inject.Inject; - -import java.util.Collections; -import java.util.concurrent.Callable; - -import org.kohsuke.args4j.Argument; -import org.kohsuke.args4j.Option; - -public class AlterTopic implements Callable { - - private final ChangeControl.Factory changeControlFactory; - private final ReviewDb db; - private final IdentifiedUser currentUser; - - @Argument(index = 0, required = true, multiValued = false, - usage = "change with topic to change") - private Change.Id changeId; - - public void setChangeId(final Change.Id changeId) { - this.changeId = changeId; - } - - @Argument(index = 1, required = true, multiValued = false, usage = "new topic") - private String newTopicName; - - public void setTopic(final String topic) { - this.newTopicName = topic.trim(); - } - - @Option(name = "--message", aliases = {"-m"}, - usage = "optional message to append to change") - private String message; - - public void setMessage(final String message) { - this.message = message; - } - - @Inject - AlterTopic(final ChangeControl.Factory changeControlFactory, final ReviewDb db, - final IdentifiedUser currentUser) { - this.changeControlFactory = changeControlFactory; - this.db = db; - this.currentUser = currentUser; - - changeId = null; - newTopicName = null; - message = null; - } - - @Override - public ReviewResult call() throws EmailException, - InvalidChangeOperationException, NoSuchChangeException, OrmException { - final ChangeControl control = changeControlFactory.validateFor(changeId); - final ReviewResult result = new ReviewResult(); - result.setChangeId(changeId); - - if (!control.canAddPatchSet()) { - throw new NoSuchChangeException(changeId); - } - if (!control.canEditTopicName()) { - result.addError(new ReviewResult.Error( - ReviewResult.Error.Type.EDIT_TOPIC_NAME_NOT_PERMITTED)); - return result; - } - - final Change change = db.changes().get(changeId); - final String oldTopicName = change.getTopic() != null ? change.getTopic() : ""; - if (!oldTopicName.equals(newTopicName)) { - String summary; - if (oldTopicName.isEmpty()) { - summary = "Topic set to \"" + newTopicName + "\""; - } else if (newTopicName.isEmpty()) { - summary = "Topic \"" + oldTopicName + "\" removed"; - } else { - summary = "Topic changed from \"" + oldTopicName // - + "\" to \"" + newTopicName + "\""; - } - final ChangeMessage cmsg = new ChangeMessage( - new ChangeMessage.Key(changeId, ChangeUtil.messageUUID(db)), - currentUser.getAccountId(), change.currentPatchSetId()); - final StringBuilder msgBuf = new StringBuilder(summary); - if (message != null && message.length() > 0) { - msgBuf.append("\n\n"); - msgBuf.append(message); - } - cmsg.setMessage(msgBuf.toString()); - - final Change updatedChange = db.changes().atomicUpdate(changeId, - new AtomicUpdate() { - @Override - public Change update(Change change) { - change.setTopic(newTopicName); - return change; - } - }); - - if (updatedChange == null) { - String err = "Patchset is not latest"; - throw new InvalidChangeOperationException(err); - } - db.changeMessages().insert(Collections.singleton(cmsg)); - } - - return result; - } -}