diff --git a/Documentation/config-gerrit.txt b/Documentation/config-gerrit.txt index f8a4159ad8..bd2263c133 100644 --- a/Documentation/config-gerrit.txt +++ b/Documentation/config-gerrit.txt @@ -910,6 +910,76 @@ keyboard shortcut is appended. Default is "Reply and score". In the user interface it becomes "Reply and score (Shortcut: a)". +[[changeCleanup]] +=== Section changeCleanup + +This section allows to configure change cleanups and schedules them to +run periodically. + +[[changeCleanup.abandonAfter]]changeCleanup.abandonAfter:: ++ +Period of inactivity after which open changes should be abandoned +automatically. ++ +By default `0`, never abandon open changes. ++ +[WARNING] Auto-Abandoning changes may confuse/annoy users. When +enabling this, make sure to choose a reasonably large grace period and +inform users in advance. ++ +The following suffixes are supported to define the time unit: ++ +* `d, day, days` +* `w, week, weeks` (`1 week` is treated as `7 days`) +* `mon, month, months` (`1 month` is treated as `30 days`) +* `y, year, years` (`1 year` is treated as `365 days`) + +[[changeCleanup.abandonMessage]]changeCleanup.abandonMessage:: ++ +Change message that should be posted when a change is abandoned. ++ +'${URL}' can be used as a placeholder for the Gerrit web URL. ++ +By default "Auto-Abandoned due to inactivity, see +${URL}Documentation/user-change-cleanup.html#auto-abandon\n\n +If this change is still wanted it should be restored.". + +[[changeCleanup.startTime]]changeCleanup.startTime:: ++ +Start time to define the first execution of the change cleanups. +If the configured `'changeCleanup.interval'` is shorter than +`'changeCleanup.startTime - now'` the start time will be preponed by +the maximum integral multiple of `'changeCleanup.interval'` so that the +start time is still in the future. ++ +---- + : +or +: + + : Mon, Tue, Wed, Thu, Fri, Sat, Sun + : 00-23 + : 0-59 +---- + + +[[changeCleanup.interval]]changeCleanup.interval:: ++ +Interval for periodic repetition of triggering the change cleanups. +The interval must be larger than zero. The following suffixes are supported +to define the time unit for the interval: ++ +* `s, sec, second, seconds` +* `m, min, minute, minutes` +* `h, hr, hour, hours` +* `d, day, days` +* `w, week, weeks` (`1 week` is treated as `7 days`) +* `mon, month, months` (`1 month` is treated as `30 days`) +* `y, year, years` (`1 year` is treated as `365 days`) + +link:#schedule-examples[Schedule examples] can be found in the +link:#gc[gc] section. + [[changeMerge]] === Section changeMerge @@ -1519,6 +1589,7 @@ to define the time unit for the interval: * `mon, month, months` (`1 month` is treated as `30 days`) * `y, year, years` (`1 year` is treated as `365 days`) +[[schedule-examples]] Examples:: + ---- diff --git a/Documentation/index.txt b/Documentation/index.txt index 084160a9bb..1a2ed6b57f 100644 --- a/Documentation/index.txt +++ b/Documentation/index.txt @@ -21,6 +21,7 @@ .. Changes ... link:user-changeid.html[Change-Id Lines] ... link:user-signedoffby.html[Signed-off-by Lines] +... link:user-change-cleanup.html[Change Cleanup] == Project Management . link:project-configuration.html[Project Configuration] diff --git a/Documentation/user-change-cleanup.txt b/Documentation/user-change-cleanup.txt new file mode 100644 index 0000000000..e569f3887f --- /dev/null +++ b/Documentation/user-change-cleanup.txt @@ -0,0 +1,28 @@ += Gerrit Code Review - Change Cleanup + +Gerrit administrators may configure +link:config-gerrit.html#changeCleanup[change cleanups] that are +executed periodically. + +[[auto-abandon]] +== Auto-Abandon + +This cleanup job automatically abandons open changes that have been +inactive for a defined time. + +Abandoning old inactive changes has the following advantages: + +* it signals change authors that changes are considered outdated +* it keeps dashboards clean +* it reduces the load on the server (for open changes the mergeability + flag is recomputed whenever a change is merged) + +If a change is still wanted it can be restored by clicking on the +`Restore` button. + +GERRIT +------ +Part of link:index.html[Gerrit Code Review] + +SEARCHBOX +--------- diff --git a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/Daemon.java b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/Daemon.java index 1afa243512..cb8e9ba030 100644 --- a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/Daemon.java +++ b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/Daemon.java @@ -44,6 +44,7 @@ import com.google.gerrit.pgm.util.SiteProgram; import com.google.gerrit.reviewdb.client.AuthType; import com.google.gerrit.server.account.InternalAccountDirectory; import com.google.gerrit.server.cache.h2.DefaultCacheFactory; +import com.google.gerrit.server.change.ChangeCleanupRunner; import com.google.gerrit.server.config.AuthConfig; import com.google.gerrit.server.config.AuthConfigModule; import com.google.gerrit.server.config.CanonicalWebUrlModule; @@ -373,6 +374,7 @@ public class Daemon extends SiteProgram { } }); modules.add(new GarbageCollectionModule()); + modules.add(new ChangeCleanupRunner.Module()); return cfgInjector.createChildInjector(modules); } 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 a73f8ba6f5..5a6b274919 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 @@ -22,6 +22,7 @@ import com.google.gerrit.extensions.restapi.AuthException; import com.google.gerrit.extensions.restapi.ResourceConflictException; import com.google.gerrit.extensions.restapi.RestModifyView; import com.google.gerrit.extensions.webui.UiAction; +import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.reviewdb.client.Change; import com.google.gerrit.reviewdb.client.ChangeMessage; import com.google.gerrit.reviewdb.server.ReviewDb; @@ -88,14 +89,23 @@ public class Abandon implements RestModifyView, } else if (change.getStatus() == Change.Status.DRAFT) { throw new ResourceConflictException("draft changes cannot be abandoned"); } + change = abandon(control, input.message, caller.getAccount()); + ChangeInfo result = json.format(change); + return result; + } + public Change abandon(ChangeControl control, + String msgTxt, Account acc) throws ResourceConflictException, + OrmException, IOException { + Change change; ChangeMessage message; ChangeUpdate update; + Change.Id changeId = control.getChange().getId(); ReviewDb db = dbProvider.get(); - db.changes().beginTransaction(change.getId()); + db.changes().beginTransaction(changeId); try { change = db.changes().atomicUpdate( - change.getId(), + changeId, new AtomicUpdate() { @Override public Change update(Change change) { @@ -109,12 +119,12 @@ public class Abandon implements RestModifyView, }); if (change == null) { throw new ResourceConflictException("change is " - + status(db.changes().get(req.getChange().getId()))); + + status(db.changes().get(changeId))); } //TODO(yyonas): atomic update was not propagated update = updateFactory.create(control, change.getLastUpdatedOn()); - message = newMessage(input, caller, change); + message = newMessage(msgTxt, acc != null ? acc.getId() : null, change); cmUtil.addChangeMessage(db, update, message); db.commit(); } finally { @@ -125,19 +135,20 @@ public class Abandon implements RestModifyView, indexer.index(db, change); try { ReplyToChangeSender cm = abandonedSenderFactory.create(change.getId()); - cm.setFrom(caller.getAccountId()); + if (acc != null) { + cm.setFrom(acc.getId()); + } cm.setChangeMessage(message); cm.send(); } catch (Exception e) { log.error("Cannot email update for change " + change.getChangeId(), e); } hooks.doChangeAbandonedHook(change, - caller.getAccount(), + acc, db.patchSets().get(change.currentPatchSetId()), - Strings.emptyToNull(input.message), + Strings.emptyToNull(msgTxt), db); - ChangeInfo result = json.format(change); - return result; + return change; } @Override @@ -150,20 +161,20 @@ public class Abandon implements RestModifyView, && resource.getControl().canAbandon()); } - private ChangeMessage newMessage(AbandonInput input, IdentifiedUser caller, + private ChangeMessage newMessage(String msgTxt, Account.Id accId, Change change) throws OrmException { StringBuilder msg = new StringBuilder(); msg.append("Abandoned"); - if (!Strings.nullToEmpty(input.message).trim().isEmpty()) { + if (!Strings.nullToEmpty(msgTxt).trim().isEmpty()) { msg.append("\n\n"); - msg.append(input.message.trim()); + msg.append(msgTxt.trim()); } ChangeMessage message = new ChangeMessage( new ChangeMessage.Key( change.getId(), ChangeUtil.messageUUID(dbProvider.get())), - caller.getAccountId(), + accId, change.getLastUpdatedOn(), change.currentPatchSetId()); message.setMessage(msg.toString()); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/AbandonUtil.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/AbandonUtil.java new file mode 100644 index 0000000000..c6e16be035 --- /dev/null +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/AbandonUtil.java @@ -0,0 +1,97 @@ +// Copyright (C) 2015 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.gerrit.extensions.restapi.ResourceConflictException; +import com.google.gerrit.reviewdb.client.Change; +import com.google.gerrit.server.IdentifiedUser; +import com.google.gerrit.server.config.ChangeCleanupConfig; +import com.google.gerrit.server.project.ChangeControl; +import com.google.gerrit.server.project.NoSuchChangeException; +import com.google.gerrit.server.query.QueryParseException; +import com.google.gerrit.server.query.change.ChangeData; +import com.google.gerrit.server.query.change.ChangeQueryBuilder; +import com.google.gerrit.server.query.change.QueryProcessor; +import com.google.gwtorm.server.OrmException; +import com.google.inject.Inject; +import com.google.inject.Singleton; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.util.List; +import java.util.concurrent.TimeUnit; + +@Singleton +public class AbandonUtil { + private static final Logger log = LoggerFactory.getLogger(AbandonUtil.class); + + private final ChangeCleanupConfig cfg; + private final IdentifiedUser.GenericFactory identifiedUserFactory; + private final QueryProcessor queryProcessor; + private final ChangeQueryBuilder queryBuilder; + private final ChangeControl.GenericFactory changeControlFactory; + private final Abandon abandon; + + @Inject + AbandonUtil( + ChangeCleanupConfig cfg, + IdentifiedUser.GenericFactory identifiedUserFactory, + QueryProcessor queryProcessor, + ChangeQueryBuilder queryBuilder, + ChangeControl.GenericFactory changeControlFactory, + Abandon abandon) { + this.cfg = cfg; + this.identifiedUserFactory = identifiedUserFactory; + this.queryProcessor = queryProcessor; + this.queryBuilder = queryBuilder; + this.changeControlFactory = changeControlFactory; + this.abandon = abandon; + } + + public void abandonInactiveOpenChanges() { + if (cfg.getAbandonAfter() <= 0) { + return; + } + + try { + String query = "status:new age:" + + TimeUnit.MILLISECONDS.toMinutes(cfg.getAbandonAfter()) + + "m"; + List changesToAbandon = queryProcessor.enforceVisibility(false) + .queryChanges(queryBuilder.parse(query)).changes(); + for (ChangeData cd : changesToAbandon) { + try { + abandon.abandon(changeControl(cd), cfg.getAbandonMessage(), null); + } catch (ResourceConflictException e) { + // Change was already merged or abandoned. + } catch (Throwable e) { + log.error(String.format( + "Failed to auto-abandon inactive open change %d.", + cd.getId().get()), e); + } + } + } catch (QueryParseException | OrmException e) { + log.error("Failed to query inactive open changes for auto-abandoning.", e); + } + } + + private ChangeControl changeControl(ChangeData cd) + throws NoSuchChangeException, OrmException { + Change c = cd.change(); + return changeControlFactory.controlFor(c, + identifiedUserFactory.create(c.getOwner())); + } +} diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeCleanupRunner.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeCleanupRunner.java new file mode 100644 index 0000000000..310c8cbef6 --- /dev/null +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeCleanupRunner.java @@ -0,0 +1,108 @@ +// Copyright (C) 2015 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 static com.google.gerrit.server.config.ScheduleConfig.MISSING_CONFIG; + +import com.google.gerrit.extensions.events.LifecycleListener; +import com.google.gerrit.lifecycle.LifecycleModule; +import com.google.gerrit.server.config.ChangeCleanupConfig; +import com.google.gerrit.server.config.ScheduleConfig; +import com.google.gerrit.server.git.WorkQueue; +import com.google.gerrit.server.util.ManualRequestContext; +import com.google.gerrit.server.util.OneOffRequestContext; +import com.google.gwtorm.server.OrmException; +import com.google.inject.Inject; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.util.concurrent.TimeUnit; + +/** Runnable to enable scheduling change cleanups to run periodically */ +public class ChangeCleanupRunner implements Runnable { + private static final Logger log = LoggerFactory + .getLogger(ChangeCleanupRunner.class); + + public static class Module extends LifecycleModule { + @Override + protected void configure() { + listener().to(Lifecycle.class); + } + } + + static class Lifecycle implements LifecycleListener { + private final WorkQueue queue; + private final ChangeCleanupRunner runner; + private final ChangeCleanupConfig cfg; + + @Inject + Lifecycle(WorkQueue queue, + ChangeCleanupRunner runner, + ChangeCleanupConfig cfg) { + this.queue = queue; + this.runner = runner; + this.cfg = cfg; + } + + @Override + public void start() { + ScheduleConfig scheduleConfig = cfg.getScheduleConfig(); + long interval = scheduleConfig.getInterval(); + long delay = scheduleConfig.getInitialDelay(); + if (delay == MISSING_CONFIG && interval == MISSING_CONFIG) { + log.info("Ignoring missing changeCleanup schedule configuration"); + } else if (delay < 0 || interval <= 0) { + log.warn(String.format( + "Ignoring invalid changeCleanup schedule configuration: %s", + scheduleConfig)); + } else { + queue.getDefaultQueue().scheduleAtFixedRate(runner, delay, + interval, TimeUnit.MILLISECONDS); + } + } + + @Override + public void stop() { + // handled by WorkQueue.stop() already + } + } + + private final OneOffRequestContext oneOffRequestContext; + private final AbandonUtil abandonUtil; + + @Inject + ChangeCleanupRunner( + OneOffRequestContext oneOffRequestContext, + AbandonUtil abandonUtil) { + this.oneOffRequestContext = oneOffRequestContext; + this.abandonUtil = abandonUtil; + } + + @Override + public void run() { + log.info("Running change cleanups."); + try (ManualRequestContext ctx = oneOffRequestContext.open()) { + abandonUtil.abandonInactiveOpenChanges(); + } catch (OrmException e) { + log.error("Failed to cleanup changes.", e); + } + } + + @Override + public String toString() { + return "change cleanup runner"; + } +} diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/config/ChangeCleanupConfig.java b/gerrit-server/src/main/java/com/google/gerrit/server/config/ChangeCleanupConfig.java new file mode 100644 index 0000000000..9d2ede86ce --- /dev/null +++ b/gerrit-server/src/main/java/com/google/gerrit/server/config/ChangeCleanupConfig.java @@ -0,0 +1,75 @@ +// Copyright (C) 2015 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.config; + +import com.google.common.base.Strings; +import com.google.inject.Inject; +import com.google.inject.Singleton; + +import org.eclipse.jgit.lib.Config; + +import java.util.concurrent.TimeUnit; + +@Singleton +public class ChangeCleanupConfig { + private static String SECTION = "changeCleanup"; + private static String KEY_ABANDON_AFTER = "abandonAfter"; + private static String KEY_ABANDON_MESSAGE = "abandonMessage"; + private static String DEFAULT_ABANDON_MESSAGE = + "Auto-Abandoned due to inactivity, see " + + "${URL}Documentation/user-change-cleanup.html#auto-abandon\n" + + "\n" + + "If this change is still wanted it should be restored."; + + private final ScheduleConfig scheduleConfig; + private final long abandonAfter; + private final String abandonMessage; + + @Inject + ChangeCleanupConfig(@GerritServerConfig Config cfg, + @CanonicalWebUrl String canonicalWebUrl) { + scheduleConfig = new ScheduleConfig(cfg, SECTION); + abandonAfter = readAbandonAfter(cfg); + abandonMessage = readAbandonMessage(cfg, canonicalWebUrl); + } + + private long readAbandonAfter(Config cfg) { + long abandonAfter = + ConfigUtil.getTimeUnit(cfg, SECTION, null, KEY_ABANDON_AFTER, 0, + TimeUnit.MILLISECONDS); + return abandonAfter >= 0 ? abandonAfter : 0; + } + + private String readAbandonMessage(Config cfg, String webUrl) { + String abandonMessage = cfg.getString(SECTION, null, KEY_ABANDON_MESSAGE); + if (Strings.isNullOrEmpty(abandonMessage)) { + abandonMessage = DEFAULT_ABANDON_MESSAGE; + } + abandonMessage = abandonMessage.replaceAll("\\$\\{URL\\}", webUrl); + return abandonMessage; + } + + public ScheduleConfig getScheduleConfig() { + return scheduleConfig; + } + + public long getAbandonAfter() { + return abandonAfter; + } + + public String getAbandonMessage() { + return abandonMessage; + } +} 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 a6acb997d7..b4f744d5ca 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 @@ -219,6 +219,7 @@ public class GerritGlobalModule extends FactoryModule { bind(GitWebConfig.class); bind(GcConfig.class); + bind(ChangeCleanupConfig.class); bind(ApprovalsUtil.class); bind(ChangeMergeQueue.class).in(SINGLETON); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/config/ScheduleConfig.java b/gerrit-server/src/main/java/com/google/gerrit/server/config/ScheduleConfig.java index d19f063b9a..91a20dce8e 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/config/ScheduleConfig.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/config/ScheduleConfig.java @@ -38,6 +38,11 @@ public class ScheduleConfig { private static final String KEY_INTERVAL = "interval"; private static final String KEY_STARTTIME = "startTime"; + private final Config rc; + private final String section; + private final String subsection; + private final String keyInterval; + private final String keyStartTime; private final long initialDelay; private final long interval; @@ -62,6 +67,11 @@ public class ScheduleConfig { @VisibleForTesting ScheduleConfig(Config rc, String section, String subsection, String keyInterval, String keyStartTime, DateTime now) { + this.rc = rc; + this.section = section; + this.subsection = subsection; + this.keyInterval = keyInterval; + this.keyStartTime = keyStartTime; this.interval = interval(rc, section, subsection, keyInterval); if (interval > 0) { this.initialDelay = initialDelay(rc, section, subsection, keyStartTime, now, @@ -150,4 +160,31 @@ public class ScheduleConfig { return delay; } + @Override + public String toString() { + StringBuilder b = new StringBuilder(); + b.append(formatValue(keyInterval)); + b.append(", "); + b.append(formatValue(keyStartTime)); + return b.toString(); + } + + private String formatValue(String key) { + StringBuilder b = new StringBuilder(); + b.append(section); + if (subsection != null) { + b.append("."); + b.append(subsection); + } + b.append("."); + b.append(key); + String value = rc.getString(section, subsection, key); + if (value != null) { + b.append(" = "); + b.append(value); + } else { + b.append(": NA"); + } + return b.toString(); + } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/events/EventFactory.java b/gerrit-server/src/main/java/com/google/gerrit/server/events/EventFactory.java index 4a61e3e1aa..098a0cc883 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/events/EventFactory.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/events/EventFactory.java @@ -489,6 +489,10 @@ public class EventFactory { * @return object suitable for serialization to JSON */ public AccountAttribute asAccountAttribute(final Account account) { + if (account == null) { + return null; + } + AccountAttribute who = new AccountAttribute(); who.name = account.getFullName(); who.email = account.getPreferredEmail(); diff --git a/gerrit-war/src/main/java/com/google/gerrit/httpd/WebAppInitializer.java b/gerrit-war/src/main/java/com/google/gerrit/httpd/WebAppInitializer.java index 3bc8b58fc4..dde4923454 100644 --- a/gerrit-war/src/main/java/com/google/gerrit/httpd/WebAppInitializer.java +++ b/gerrit-war/src/main/java/com/google/gerrit/httpd/WebAppInitializer.java @@ -28,6 +28,7 @@ import com.google.gerrit.lucene.LuceneIndexModule; import com.google.gerrit.reviewdb.client.AuthType; import com.google.gerrit.server.account.InternalAccountDirectory; import com.google.gerrit.server.cache.h2.DefaultCacheFactory; +import com.google.gerrit.server.change.ChangeCleanupRunner; import com.google.gerrit.server.config.AuthConfig; import com.google.gerrit.server.config.AuthConfigModule; import com.google.gerrit.server.config.CanonicalWebUrlModule; @@ -323,6 +324,7 @@ public class WebAppInitializer extends GuiceServletContextListener } }); modules.add(new GarbageCollectionModule()); + modules.add(new ChangeCleanupRunner.Module()); return cfgInjector.createChildInjector(modules); }