Adapt to current Gerrit plugin API The plugin no longer compiled against current gerrit master. * OutgoingEmail is final and now delegates custom behavior to an EmailDecorator. Turn RateLimitReachedSender into a decorator and use EmailFactories to create the outgoing message, matching core and the checks plugin. Header, message id and recipient setup move to init(OutgoingEmail), the recipient check moves to shouldSendMessage(), and formatting moves to populateEmailContent(). The plugin-owned Soy templates are still rendered by the plugin, now with an explicit data map for email.userNameEmail and email.log instead of the removed protected soyContext. Add focused sender tests for the factory path, header/message-id/recipient setup, group gating, and text/HTML rendering. * PluginCommandModule requires the plugin name at construction; inject it via @PluginName. * Error Prone rejects the impossible null comparison in UserResolver.getUserName(); use flatMap over the optional user name instead. The discarded cloneProject() return values in RateLimitUploadPackIT are assigned to an unused variable to satisfy CheckReturnValue. Test Plan: bazelisk build //plugins/rate-limiter bazelisk test --cache_test_results=NO \ //plugins/rate-limiter:rate-limiter_tests Change-Id: If3b8a90c25e17d64f5c1e8a2047b9de6315a7f28
diff --git a/src/main/java/com/googlesource/gerrit/plugins/ratelimiter/RateLimitReachedSender.java b/src/main/java/com/googlesource/gerrit/plugins/ratelimiter/RateLimitReachedSender.java index 35b50e2..6a81feb 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/ratelimiter/RateLimitReachedSender.java +++ b/src/main/java/com/googlesource/gerrit/plugins/ratelimiter/RateLimitReachedSender.java
@@ -14,18 +14,21 @@ package com.googlesource.gerrit.plugins.ratelimiter; +import com.google.common.collect.ImmutableMap; import com.google.common.io.CharStreams; import com.google.gerrit.exceptions.EmailException; import com.google.gerrit.extensions.api.changes.RecipientType; import com.google.gerrit.server.IdentifiedUser; -import com.google.gerrit.server.mail.send.EmailArguments; +import com.google.gerrit.server.mail.EmailFactories; import com.google.gerrit.server.mail.send.MessageIdGenerator; import com.google.gerrit.server.mail.send.OutgoingEmail; +import com.google.gerrit.server.mail.send.OutgoingEmail.EmailDecorator; import com.google.gerrit.server.util.time.TimeUtil; import com.google.inject.ProvisionException; import com.google.inject.assistedinject.Assisted; import com.google.inject.assistedinject.AssistedInject; import com.google.template.soy.SoyFileSet; +import com.google.template.soy.data.SanitizedContent; import com.google.template.soy.jbcsrc.api.SoySauce; import java.io.BufferedReader; import java.io.IOException; @@ -33,26 +36,28 @@ import java.io.Reader; import java.util.Objects; -public class RateLimitReachedSender extends OutgoingEmail { +public class RateLimitReachedSender implements EmailDecorator { public interface Factory { RateLimitReachedSender create(IdentifiedUser user, String emailmessage, boolean acquirePermit); } + private final EmailFactories emailFactories; private final IdentifiedUser user; private final String emailMessage; private final MessageIdGenerator messageIdGenerator; private final Configuration configuration; private final boolean acquirePermit; + private OutgoingEmail email; @AssistedInject public RateLimitReachedSender( - EmailArguments args, + EmailFactories emailFactories, MessageIdGenerator messageIdGenerator, Configuration configuration, @Assisted IdentifiedUser user, @Assisted String emailMessage, @Assisted boolean acquirePermit) { - super(args, "RateLimitReached"); + this.emailFactories = emailFactories; this.messageIdGenerator = messageIdGenerator; this.configuration = configuration; this.acquirePermit = acquirePermit; @@ -60,26 +65,30 @@ this.emailMessage = emailMessage; } - @Override - protected void init() throws EmailException { - super.init(); - setHeader("Subject", "[Gerrit Code Review] " + emailMessage); - setMessageId( - messageIdGenerator.fromReasonAccountIdAndTimestamp( - "rate_limit_reached", user.getAccountId(), TimeUtil.now())); - add(RecipientType.TO, user.getAccountId()); + public void send() throws EmailException { + emailFactories.createOutgoingEmail("RateLimitReached", this).send(); } @Override - protected boolean shouldSendMessage() { + public void init(OutgoingEmail email) throws EmailException { + this.email = email; + email.setHeader("Subject", "[Gerrit Code Review] " + emailMessage); + email.setMessageId( + messageIdGenerator.fromReasonAccountIdAndTimestamp( + "rate_limit_reached", user.getAccountId(), TimeUtil.now())); + email.addByAccountId(RecipientType.TO, user.getAccountId()); + } + + @Override + public boolean shouldSendMessage() { return user.getEffectiveGroups().containsAnyOf(configuration.getRecipients()); } @Override - protected void format() throws EmailException { - appendText(soyUseTextTemplate("RateLimiterEmailFormat")); - if (useHtml()) { - appendHtml(soyUseHtmlTemplate("RateLimiterEmailFormatHTML")); + public void populateEmailContent() throws EmailException { + email.appendText(soyUseTextTemplate("RateLimiterEmailFormat")); + if (email.useHtml()) { + email.appendHtml(soyUseHtmlTemplate("RateLimiterEmailFormatHTML")); } } @@ -88,9 +97,9 @@ return renderer.renderText().get(); } - private String soyUseHtmlTemplate(String template) { + private SanitizedContent soyUseHtmlTemplate(String template) { SoySauce.Renderer renderer = getRenderer(template); - return renderer.renderHtml().get().toString(); + return renderer.renderHtml().get(); } private SoySauce.Renderer getRenderer(String template) { @@ -114,19 +123,18 @@ .append(".") .append(template) .toString(); + ImmutableMap<String, Object> soyData = + ImmutableMap.of( + "email", + ImmutableMap.of( + "email", getEmail(), + "userNameEmail", email.getUserNameEmailFor(user.getAccountId()), + "log", emailMessage)); SoySauce.Renderer renderer = - builder.build().compileTemplates().renderTemplate(renderedTemplate).setData(soyContext); + builder.build().compileTemplates().renderTemplate(renderedTemplate).setData(soyData); return renderer; } - @Override - protected void setupSoyContext() { - super.setupSoyContext(); - soyContextEmailData.put("email", getEmail()); - soyContextEmailData.put("userNameEmail", getUserNameEmailFor(user.getAccountId())); - soyContextEmailData.put("log", emailMessage); - } - private String getEmail() { return user.getAccount().preferredEmail(); }
diff --git a/src/main/java/com/googlesource/gerrit/plugins/ratelimiter/SshModule.java b/src/main/java/com/googlesource/gerrit/plugins/ratelimiter/SshModule.java index 85c0035..24a59d1 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/ratelimiter/SshModule.java +++ b/src/main/java/com/googlesource/gerrit/plugins/ratelimiter/SshModule.java
@@ -14,9 +14,15 @@ package com.googlesource.gerrit.plugins.ratelimiter; +import com.google.gerrit.extensions.annotations.PluginName; import com.google.gerrit.sshd.PluginCommandModule; +import com.google.inject.Inject; class SshModule extends PluginCommandModule { + @Inject + SshModule(@PluginName String pluginName) { + super(pluginName); + } @Override protected void configureCommands() {
diff --git a/src/main/java/com/googlesource/gerrit/plugins/ratelimiter/UserResolver.java b/src/main/java/com/googlesource/gerrit/plugins/ratelimiter/UserResolver.java index 0e79529..1e2d0e3 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/ratelimiter/UserResolver.java +++ b/src/main/java/com/googlesource/gerrit/plugins/ratelimiter/UserResolver.java
@@ -38,10 +38,7 @@ } Optional<String> getUserName(String key) { - Optional<IdentifiedUser> user = getIdentifiedUser(key); - return user.isPresent() - ? Optional.ofNullable(user.get().getUserName().get()) - : Optional.empty(); + return getIdentifiedUser(key).flatMap(IdentifiedUser::getUserName); } private static boolean isNumeric(String key) {
diff --git a/src/test/java/com/googlesource/gerrit/plugins/ratelimiter/RateLimitReachedSenderTest.java b/src/test/java/com/googlesource/gerrit/plugins/ratelimiter/RateLimitReachedSenderTest.java new file mode 100644 index 0000000..0cf6e91 --- /dev/null +++ b/src/test/java/com/googlesource/gerrit/plugins/ratelimiter/RateLimitReachedSenderTest.java
@@ -0,0 +1,137 @@ +// Copyright (C) 2026 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.googlesource.gerrit.plugins.ratelimiter; + +import static com.google.common.truth.Truth.assertThat; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import com.google.common.collect.ImmutableList; +import com.google.gerrit.entities.Account; +import com.google.gerrit.entities.AccountGroup; +import com.google.gerrit.exceptions.EmailException; +import com.google.gerrit.extensions.api.changes.RecipientType; +import com.google.gerrit.server.IdentifiedUser; +import com.google.gerrit.server.account.GroupMembership; +import com.google.gerrit.server.mail.EmailFactories; +import com.google.gerrit.server.mail.send.MessageIdGenerator; +import com.google.gerrit.server.mail.send.MessageIdGenerator.MessageId; +import com.google.gerrit.server.mail.send.OutgoingEmail; +import com.google.template.soy.data.SanitizedContent; +import java.time.Instant; +import org.junit.Before; +import org.junit.Test; +import org.mockito.ArgumentCaptor; + +public class RateLimitReachedSenderTest { + private static final Account.Id ACCOUNT_ID = Account.id(1000001); + private static final AccountGroup.UUID RECIPIENT_GROUP = AccountGroup.uuid("recipient-group"); + private static final String EMAIL = "user@example.com"; + private static final String USER_NAME_EMAIL = "User <user@example.com>"; + private static final String EMAIL_MESSAGE = "User user reached the warning limit"; + + private EmailFactories emailFactories; + private MessageIdGenerator messageIdGenerator; + private Configuration configuration; + private IdentifiedUser user; + private GroupMembership groupMembership; + private RateLimitReachedSender sender; + + @Before + public void setUp() { + emailFactories = mock(EmailFactories.class); + messageIdGenerator = mock(MessageIdGenerator.class); + configuration = mock(Configuration.class); + user = mock(IdentifiedUser.class); + groupMembership = mock(GroupMembership.class); + + when(configuration.getRecipients()).thenReturn(ImmutableList.of(RECIPIENT_GROUP)); + when(user.getAccountId()).thenReturn(ACCOUNT_ID); + when(user.getAccount()) + .thenReturn(Account.builder(ACCOUNT_ID, Instant.EPOCH).setPreferredEmail(EMAIL).build()); + when(user.getEffectiveGroups()).thenReturn(groupMembership); + + sender = + new RateLimitReachedSender( + emailFactories, + messageIdGenerator, + configuration, + user, + EMAIL_MESSAGE, + /* acquirePermit= */ true); + } + + @Test + public void sendUsesEmailFactories() throws Exception { + OutgoingEmail outgoingEmail = mock(OutgoingEmail.class); + when(emailFactories.createOutgoingEmail("RateLimitReached", sender)).thenReturn(outgoingEmail); + + sender.send(); + + verify(emailFactories).createOutgoingEmail("RateLimitReached", sender); + verify(outgoingEmail).send(); + } + + @Test + public void initSetsHeaderMessageIdAndRecipient() throws Exception { + OutgoingEmail email = mock(OutgoingEmail.class); + MessageId messageId = MessageId.create("message-id"); + when(messageIdGenerator.fromReasonAccountIdAndTimestamp( + eq("rate_limit_reached"), eq(ACCOUNT_ID), any(Instant.class))) + .thenReturn(messageId); + + sender.init(email); + + verify(email).setHeader("Subject", "[Gerrit Code Review] " + EMAIL_MESSAGE); + verify(email).setMessageId(messageId); + verify(email).addByAccountId(RecipientType.TO, ACCOUNT_ID); + } + + @Test + public void shouldSendMessageChecksConfiguredRecipientGroups() { + when(groupMembership.containsAnyOf(configuration.getRecipients())).thenReturn(true); + + assertThat(sender.shouldSendMessage()).isTrue(); + } + + @Test + public void shouldNotSendMessageIfUserIsNotInConfiguredRecipientGroups() { + when(groupMembership.containsAnyOf(configuration.getRecipients())).thenReturn(false); + + assertThat(sender.shouldSendMessage()).isFalse(); + } + + @Test + public void populateEmailContentRendersTextAndHtmlTemplates() throws EmailException { + OutgoingEmail email = mock(OutgoingEmail.class); + when(email.useHtml()).thenReturn(true); + when(email.getUserNameEmailFor(ACCOUNT_ID)).thenReturn(USER_NAME_EMAIL); + + sender.init(email); + sender.populateEmailContent(); + + ArgumentCaptor<String> textCaptor = ArgumentCaptor.forClass(String.class); + ArgumentCaptor<SanitizedContent> htmlCaptor = ArgumentCaptor.forClass(SanitizedContent.class); + verify(email).appendText(textCaptor.capture()); + verify(email).appendHtml(htmlCaptor.capture()); + assertThat(textCaptor.getValue()).contains(USER_NAME_EMAIL); + assertThat(textCaptor.getValue()).contains(EMAIL_MESSAGE); + assertThat(htmlCaptor.getValue().toString()).contains("User <user@example.com>"); + assertThat(htmlCaptor.getValue().toString()).contains(EMAIL_MESSAGE); + } +}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/ratelimiter/RateLimitUploadPackIT.java b/src/test/java/com/googlesource/gerrit/plugins/ratelimiter/RateLimitUploadPackIT.java index 035e45b..f8881e6 100644 --- a/src/test/java/com/googlesource/gerrit/plugins/ratelimiter/RateLimitUploadPackIT.java +++ b/src/test/java/com/googlesource/gerrit/plugins/ratelimiter/RateLimitUploadPackIT.java
@@ -16,10 +16,10 @@ import static com.google.gerrit.testing.GerritJUnit.assertThrows; -import com.google.gerrit.acceptance.config.GlobalPluginConfig; import com.google.gerrit.acceptance.LightweightPluginDaemonTest; import com.google.gerrit.acceptance.TestPlugin; import com.google.gerrit.acceptance.UseLocalDisk; +import com.google.gerrit.acceptance.config.GlobalPluginConfig; import com.google.gerrit.entities.Project; import com.google.gerrit.extensions.api.groups.GroupInput; import com.google.gerrit.extensions.api.projects.ProjectInput; @@ -57,7 +57,7 @@ createProjectWithChange(projectA); createProjectWithChange(projectB); - cloneProject(Project.nameKey(projectA), user); + var unused = cloneProject(Project.nameKey(projectA), user); assertThrows(TransportException.class, () -> cloneProject(Project.nameKey(projectB), user)); } @@ -82,9 +82,9 @@ createProjectWithChange(projectA); createProjectWithChange(projectB); - cloneProject(Project.nameKey(projectA), user); + var unused = cloneProject(Project.nameKey(projectA), user); Thread.sleep(PeriodicRateLimiter.DEFAULT_TIME_LAPSE_IN_MINUTES * 1000); - cloneProject(Project.nameKey(projectB), user); + unused = cloneProject(Project.nameKey(projectB), user); } private void addUserToNewGroup() throws RestApiException {