From ab5383f88721fe69022d6df84e637db10cb3cef5 Mon Sep 17 00:00:00 2001 From: David Pursehouse Date: Fri, 14 Sep 2018 08:23:10 +0900 Subject: [PATCH 1/3] Fix broken link in documentation of receive.requireSignedPush Change-Id: Ic68c66d06c615626a6b3561fbd99fb88272a726a --- Documentation/config-project-config.txt | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/Documentation/config-project-config.txt b/Documentation/config-project-config.txt index b9f68ed95d..ed0b1515bb 100644 --- a/Documentation/config-project-config.txt +++ b/Documentation/config-project-config.txt @@ -182,9 +182,10 @@ the parent project. + Controls whether server-side signed push validation is required on the project. Only has an effect if signed push validation is enabled on the -server, and link:#receive.enableSignedPush is set on the project. See -the link:config-gerrit.html#receive.enableSignedPush[global -configuration] for details. +server, and link:#receive.enableSignedPush[`receive.enableSignedPush`] is +set on the project. See the +link:config-gerrit.html#receive.enableSignedPush[global configuration] +for details. + Default is `INHERIT`, which means that this property is inherited from the parent project. From cc9db0feba62978bee15e007e02e1d1e42f25667 Mon Sep 17 00:00:00 2001 From: David Pursehouse Date: Wed, 12 Sep 2018 11:43:14 +0900 Subject: [PATCH 2/3] Add a change deleted event/listener The new event is emitted on deletion of a draft change, and deletion of a regular change. Plugins may implement the ChangeDeletedListener to perform specific actions on change deletion. The event is included in the output of the stream-events ssh command. Bug: Issue 9711 Change-Id: I37424eafa73703f51072ff8ae6c2546a10ed5c7d --- Documentation/cmd-stream-events.txt | 10 +++ .../gerrit/acceptance/EventRecorder.java | 30 +++++++++ .../acceptance/api/change/ChangeIT.java | 13 ++-- .../events/ChangeDeletedListener.java | 25 +++++++ .../gerrit/server/change/DeleteChangeOp.java | 7 +- .../server/config/GerritGlobalModule.java | 2 + .../server/events/ChangeDeletedEvent.java | 28 ++++++++ .../gerrit/server/events/EventTypes.java | 1 + .../events/StreamEventsApiListener.java | 19 ++++++ .../extensions/events/ChangeDeleted.java | 67 +++++++++++++++++++ plugins/hooks | 2 +- 11 files changed, 198 insertions(+), 6 deletions(-) create mode 100644 gerrit-extension-api/src/main/java/com/google/gerrit/extensions/events/ChangeDeletedListener.java create mode 100644 gerrit-server/src/main/java/com/google/gerrit/server/events/ChangeDeletedEvent.java create mode 100644 gerrit-server/src/main/java/com/google/gerrit/server/extensions/events/ChangeDeleted.java diff --git a/Documentation/cmd-stream-events.txt b/Documentation/cmd-stream-events.txt index 85fb5e62b0..1fdf3a850e 100644 --- a/Documentation/cmd-stream-events.txt +++ b/Documentation/cmd-stream-events.txt @@ -90,6 +90,16 @@ reason:: Reason for abandoning the change. eventCreatedOn:: Time in seconds since the UNIX epoch when this event was created. +== Change Deleted + +Sent when a change has been deleted. + +type:: "change-deleted" + +change:: link:json.html#change[change attribute] + +deleter:: link:json.html#account[account attribute] + === Change Merged Sent when a change has been merged into the git repository. diff --git a/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/EventRecorder.java b/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/EventRecorder.java index 7348c8991b..9b77411db2 100644 --- a/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/EventRecorder.java +++ b/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/EventRecorder.java @@ -26,6 +26,7 @@ import com.google.gerrit.extensions.registration.RegistrationHandle; import com.google.gerrit.server.CurrentUser; import com.google.gerrit.server.IdentifiedUser; import com.google.gerrit.server.data.RefUpdateAttribute; +import com.google.gerrit.server.events.ChangeDeletedEvent; import com.google.gerrit.server.events.ChangeMergedEvent; import com.google.gerrit.server.events.Event; import com.google.gerrit.server.events.RefEvent; @@ -69,6 +70,8 @@ public class EventRecorder { public void onEvent(Event e) { if (e instanceof ReviewerDeletedEvent) { recordedEvents.put(ReviewerDeletedEvent.TYPE, (ReviewerDeletedEvent) e); + } else if (e instanceof ChangeDeletedEvent) { + recordedEvents.put(ChangeDeletedEvent.TYPE, (ChangeDeletedEvent) e); } else if (e instanceof RefEvent) { RefEvent event = (RefEvent) e; String key = @@ -138,6 +141,21 @@ public class EventRecorder { return events; } + private ImmutableList getChangeDeletedEvents(int expectedSize) { + String key = ChangeDeletedEvent.TYPE; + if (expectedSize == 0) { + assertThat(recordedEvents).doesNotContainKey(key); + return ImmutableList.of(); + } + assertThat(recordedEvents).containsKey(key); + ImmutableList events = + FluentIterable.from(recordedEvents.get(key)) + .transform(ChangeDeletedEvent.class::cast) + .toList(); + assertThat(events).hasSize(expectedSize); + return events; + } + public void assertNoRefUpdatedEvents(String project, String branch) throws Exception { getRefUpdatedEvents(project, branch, 0); } @@ -197,6 +215,18 @@ public class EventRecorder { } } + public void assertChangeDeletedEvents(String... expected) { + ImmutableList events = getChangeDeletedEvents(expected.length / 2); + int i = 0; + for (ChangeDeletedEvent event : events) { + String id = event.change.get().id; + assertThat(id).isEqualTo(expected[i]); + String reviewer = event.deleter.get().email; + assertThat(reviewer).isEqualTo(expected[i + 1]); + i += 2; + } + } + public void close() { eventListenerRegistration.remove(); } diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/change/ChangeIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/change/ChangeIT.java index 1bde86833a..56c85d170f 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/change/ChangeIT.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/change/ChangeIT.java @@ -437,10 +437,13 @@ public class ChangeIT extends AbstractDaemonTest { @Test public void deleteDraftChange() throws Exception { PushOneCommit.Result r = createChange("refs/drafts/master"); - assertThat(query(r.getChangeId())).hasSize(1); - assertThat(info(r.getChangeId()).status).isEqualTo(ChangeStatus.DRAFT); - gApi.changes().id(r.getChangeId()).delete(); - assertThat(query(r.getChangeId())).isEmpty(); + String changeId = r.getChangeId(); + assertThat(query(changeId)).hasSize(1); + assertThat(info(changeId).status).isEqualTo(ChangeStatus.DRAFT); + gApi.changes().id(changeId).delete(); + assertThat(query(changeId)).isEmpty(); + + eventRecorder.assertChangeDeletedEvents(changeId, admin.email); } @Test @@ -521,6 +524,8 @@ public class ChangeIT extends AbstractDaemonTest { gApi.changes().id(changeId).delete(); assertThat(query(changeId)).isEmpty(); + + eventRecorder.assertChangeDeletedEvents(changeId, deleteAs.email); } finally { removePermission(Permission.DELETE_OWN_CHANGES, project, "refs/*"); removePermission(Permission.DELETE_CHANGES, project, "refs/*"); diff --git a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/events/ChangeDeletedListener.java b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/events/ChangeDeletedListener.java new file mode 100644 index 0000000000..70014f3ee5 --- /dev/null +++ b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/events/ChangeDeletedListener.java @@ -0,0 +1,25 @@ +// Copyright (C) 2018 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.extensions.events; + +import com.google.gerrit.extensions.annotations.ExtensionPoint; + +/** Notified whenever a Change is deleted. */ +@ExtensionPoint +public interface ChangeDeletedListener { + interface Event extends ChangeEvent {} + + void onChangeDeleted(Event event); +} diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteChangeOp.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteChangeOp.java index 9d819b17dc..3db995ad9e 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteChangeOp.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteChangeOp.java @@ -28,6 +28,7 @@ import com.google.gerrit.reviewdb.server.ReviewDbUtil; import com.google.gerrit.server.PatchSetUtil; import com.google.gerrit.server.StarredChangesUtil; import com.google.gerrit.server.config.GerritServerConfig; +import com.google.gerrit.server.extensions.events.ChangeDeleted; import com.google.gerrit.server.project.NoSuchChangeException; import com.google.gerrit.server.update.BatchUpdateOp; import com.google.gerrit.server.update.BatchUpdateReviewDb; @@ -66,6 +67,7 @@ class DeleteChangeOp implements BatchUpdateOp { private final StarredChangesUtil starredChangesUtil; private final DynamicItem accountPatchReviewStore; private final boolean allowDrafts; + private final ChangeDeleted changeDeleted; private Change.Id id; @@ -74,11 +76,13 @@ class DeleteChangeOp implements BatchUpdateOp { PatchSetUtil psUtil, StarredChangesUtil starredChangesUtil, DynamicItem accountPatchReviewStore, - @GerritServerConfig Config cfg) { + @GerritServerConfig Config cfg, + ChangeDeleted changeDeleted) { this.psUtil = psUtil; this.starredChangesUtil = starredChangesUtil; this.accountPatchReviewStore = accountPatchReviewStore; this.allowDrafts = allowDrafts(cfg); + this.changeDeleted = changeDeleted; } @Override @@ -98,6 +102,7 @@ class DeleteChangeOp implements BatchUpdateOp { deleteChangeElementsFromDb(ctx, id); ctx.deleteChange(); + changeDeleted.fire(ctx.getChange(), ctx.getAccount(), ctx.getWhen()); return true; } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritGlobalModule.java b/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritGlobalModule.java index c7d7f8ad7a..4e0096b053 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritGlobalModule.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritGlobalModule.java @@ -35,6 +35,7 @@ import com.google.gerrit.extensions.events.AccountIndexedListener; import com.google.gerrit.extensions.events.AgreementSignupListener; import com.google.gerrit.extensions.events.AssigneeChangedListener; import com.google.gerrit.extensions.events.ChangeAbandonedListener; +import com.google.gerrit.extensions.events.ChangeDeletedListener; import com.google.gerrit.extensions.events.ChangeIndexedListener; import com.google.gerrit.extensions.events.ChangeMergedListener; import com.google.gerrit.extensions.events.ChangeRestoredListener; @@ -310,6 +311,7 @@ public class GerritGlobalModule extends FactoryModule { DynamicSet.setOf(binder(), GitReferenceUpdatedListener.class); DynamicSet.setOf(binder(), AssigneeChangedListener.class); DynamicSet.setOf(binder(), ChangeAbandonedListener.class); + DynamicSet.setOf(binder(), ChangeDeletedListener.class); DynamicSet.setOf(binder(), CommentAddedListener.class); DynamicSet.setOf(binder(), DraftPublishedListener.class); DynamicSet.setOf(binder(), HashtagsEditedListener.class); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/events/ChangeDeletedEvent.java b/gerrit-server/src/main/java/com/google/gerrit/server/events/ChangeDeletedEvent.java new file mode 100644 index 0000000000..63142fd149 --- /dev/null +++ b/gerrit-server/src/main/java/com/google/gerrit/server/events/ChangeDeletedEvent.java @@ -0,0 +1,28 @@ +// Copyright (C) 2018 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.events; + +import com.google.common.base.Supplier; +import com.google.gerrit.reviewdb.client.Change; +import com.google.gerrit.server.data.AccountAttribute; + +public class ChangeDeletedEvent extends ChangeEvent { + public static final String TYPE = "change-deleted"; + public Supplier deleter; + + public ChangeDeletedEvent(Change change) { + super(TYPE, change); + } +} diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/events/EventTypes.java b/gerrit-server/src/main/java/com/google/gerrit/server/events/EventTypes.java index 448e09f91a..19470ad90d 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/events/EventTypes.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/events/EventTypes.java @@ -24,6 +24,7 @@ public class EventTypes { static { register(AssigneeChangedEvent.TYPE, AssigneeChangedEvent.class); register(ChangeAbandonedEvent.TYPE, ChangeAbandonedEvent.class); + register(ChangeDeletedEvent.TYPE, ChangeDeletedEvent.class); register(ChangeMergedEvent.TYPE, ChangeMergedEvent.class); register(ChangeRestoredEvent.TYPE, ChangeRestoredEvent.class); register(CommentAddedEvent.TYPE, CommentAddedEvent.class); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/events/StreamEventsApiListener.java b/gerrit-server/src/main/java/com/google/gerrit/server/events/StreamEventsApiListener.java index b76cbefee3..de6cec142c 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/events/StreamEventsApiListener.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/events/StreamEventsApiListener.java @@ -26,6 +26,7 @@ import com.google.gerrit.extensions.common.ChangeInfo; import com.google.gerrit.extensions.common.RevisionInfo; import com.google.gerrit.extensions.events.AssigneeChangedListener; import com.google.gerrit.extensions.events.ChangeAbandonedListener; +import com.google.gerrit.extensions.events.ChangeDeletedListener; import com.google.gerrit.extensions.events.ChangeMergedListener; import com.google.gerrit.extensions.events.ChangeRestoredListener; import com.google.gerrit.extensions.events.CommentAddedListener; @@ -75,6 +76,7 @@ import org.slf4j.LoggerFactory; public class StreamEventsApiListener implements AssigneeChangedListener, ChangeAbandonedListener, + ChangeDeletedListener, ChangeMergedListener, ChangeRestoredListener, CommentAddedListener, @@ -94,6 +96,7 @@ public class StreamEventsApiListener protected void configure() { DynamicSet.bind(binder(), AssigneeChangedListener.class).to(StreamEventsApiListener.class); DynamicSet.bind(binder(), ChangeAbandonedListener.class).to(StreamEventsApiListener.class); + DynamicSet.bind(binder(), ChangeDeletedListener.class).to(StreamEventsApiListener.class); DynamicSet.bind(binder(), ChangeMergedListener.class).to(StreamEventsApiListener.class); DynamicSet.bind(binder(), ChangeRestoredListener.class).to(StreamEventsApiListener.class); DynamicSet.bind(binder(), CommentAddedListener.class).to(StreamEventsApiListener.class); @@ -497,4 +500,20 @@ public class StreamEventsApiListener log.error("Failed to dispatch event", e); } } + + @Override + public void onChangeDeleted(ChangeDeletedListener.Event ev) { + try { + ChangeNotes notes = getNotes(ev.getChange()); + Change change = notes.getChange(); + ChangeDeletedEvent event = new ChangeDeletedEvent(change); + + event.change = changeAttributeSupplier(change); + event.deleter = accountAttributeSupplier(ev.getWho()); + + dispatcher.get().postEvent(change, event); + } catch (OrmException e) { + log.error("Failed to dispatch event", e); + } + } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/extensions/events/ChangeDeleted.java b/gerrit-server/src/main/java/com/google/gerrit/server/extensions/events/ChangeDeleted.java new file mode 100644 index 0000000000..26bc2296af --- /dev/null +++ b/gerrit-server/src/main/java/com/google/gerrit/server/extensions/events/ChangeDeleted.java @@ -0,0 +1,67 @@ +// Copyright (C) 2018 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.extensions.events; + +import com.google.gerrit.extensions.api.changes.NotifyHandling; +import com.google.gerrit.extensions.common.AccountInfo; +import com.google.gerrit.extensions.common.ChangeInfo; +import com.google.gerrit.extensions.events.ChangeDeletedListener; +import com.google.gerrit.extensions.registration.DynamicSet; +import com.google.gerrit.reviewdb.client.Account; +import com.google.gerrit.reviewdb.client.Change; +import com.google.gwtorm.server.OrmException; +import com.google.inject.Inject; +import com.google.inject.Singleton; +import java.sql.Timestamp; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +@Singleton +public class ChangeDeleted { + private static final Logger log = LoggerFactory.getLogger(ChangeDeleted.class); + + private final DynamicSet listeners; + private final EventUtil util; + + @Inject + ChangeDeleted(DynamicSet listeners, EventUtil util) { + this.listeners = listeners; + this.util = util; + } + + public void fire(Change change, Account deleter, Timestamp when) { + if (!listeners.iterator().hasNext()) { + return; + } + try { + Event event = new Event(util.changeInfo(change), util.accountInfo(deleter), when); + for (ChangeDeletedListener l : listeners) { + try { + l.onChangeDeleted(event); + } catch (Exception e) { + util.logEventListenerError(this, l, e); + } + } + } catch (OrmException e) { + log.error("Couldn't fire event", e); + } + } + + private static class Event extends AbstractChangeEvent implements ChangeDeletedListener.Event { + Event(ChangeInfo change, AccountInfo deleter, Timestamp when) { + super(change, deleter, when, NotifyHandling.ALL); + } + } +} diff --git a/plugins/hooks b/plugins/hooks index 08c4186511..8b71877346 160000 --- a/plugins/hooks +++ b/plugins/hooks @@ -1 +1 @@ -Subproject commit 08c418651142c4d98946a050db039d0a892d7ac9 +Subproject commit 8b7187734639e41707f142ca67c7ecf21b9cf3bd From 879d7cf6a52cd579b40f6ca5f56d2d3c8ea72b09 Mon Sep 17 00:00:00 2001 From: Alice Kober-Sotzek Date: Fri, 14 Sep 2018 14:11:40 +0200 Subject: [PATCH 3/3] FormatUtil: Correctly fix the Math#round() error flagged by error-prone The fix applied by I1c88102d4 got rid of the error-prone warning but missed to address the actual issue in that line of code. The intention of the original code was to compute a double/float percentage and mathematically round it to the next integer/long value. However, as (long * int / long) == long, the computed percentage was a long value, for which the floor operation was automatically applied. Hence, the round function was even meaningless. The correct fix is to use a double value in the operation so that the result is also of type double and preserves all fractions. By doing so, we also get rid of the error-prone warning, which only indirectly hinted at that error. Change-Id: Iea07a05281faf00b0b52cf25a2fd35ca25b069c5 --- .../com/google/gerrit/client/FormatUtil.java | 22 ++----------------- 1 file changed, 2 insertions(+), 20 deletions(-) diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/FormatUtil.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/FormatUtil.java index 878abd2502..b30b3ece95 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/FormatUtil.java +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/FormatUtil.java @@ -137,25 +137,7 @@ public class FormatUtil { if (size == 0) { return Resources.C.notAvailable(); } - int p = Math.abs(saturatedCast(delta * 100 / size)); - return p + "%"; - } - - /** - * Returns the {@code int} nearest in value to {@code value}. - * - * @param value any {@code long} value - * @return the same value cast to {@code int} if it is in the range of the {@code int} type, - * {@link Integer#MAX_VALUE} if it is too large, or {@link Integer#MIN_VALUE} if it is too - * small - */ - private static int saturatedCast(long value) { - if (value > Integer.MAX_VALUE) { - return Integer.MAX_VALUE; - } - if (value < Integer.MIN_VALUE) { - return Integer.MIN_VALUE; - } - return (int) value; + long percentage = Math.abs(Math.round(delta * 100.0 / size)); + return percentage + "%"; } }