DeleteMembers: Do not fail if member input contains null or empty string
I'm not sure how to reproduce this via REST, but we saw in our logs that
this is happening [1]. Via the Java API it's easy to reproduce.
[1]
AutoRetry: restapi.group.DeleteMembers failed, retry with tracing enabled [CONTEXT forced=true TRACE_ID="retry-on-failure-1574282678933-f84acb17" ]
java.lang.NullPointerException
at com.google.common.base.Preconditions.checkNotNull(Preconditions.java:878)
at com.google.common.primitives.Longs.tryParse(Longs.java:396)
at com.google.common.primitives.Ints.tryParse(Ints.java:741)
at com.google.common.primitives.Ints.tryParse(Ints.java:716)
at com.google.gerrit.entities.Account$Id.tryParse(Account.java:58)
at com.google.gerrit.server.account.AccountResolver$ByExactAccountId.tryParse(AccountResolver.java:287)
at com.google.gerrit.server.account.AccountResolver$Searcher.trySearch(AccountResolver.java:226)
at com.google.gerrit.server.account.AccountResolver.searchImpl(AccountResolver.java:575)
at com.google.gerrit.server.account.AccountResolver.resolve(AccountResolver.java:519)
at com.google.gerrit.server.restapi.group.DeleteMembers.apply(DeleteMembers.java:71)
at com.google.gerrit.server.restapi.group.DeleteMembers.apply(DeleteMembers.java:44)
at com.google.gerrit.httpd.restapi.RestApiServlet.lambda$invokeRestModifyViewWithRetry$4(RestApiServlet.java:740)
at com.github.rholder.retry.AttemptTimeLimiters$NoAttemptTimeLimit.call(AttemptTimeLimiters.java:78)
at com.github.rholder.retry.Retryer.call(Retryer.java:160)
at com.google.gerrit.server.update.RetryHelper.executeWithTimeoutCount(RetryHelper.java:417)
at com.google.gerrit.server.update.RetryHelper.executeWithAttemptAndTimeoutCount(RetryHelper.java:368)
at com.google.gerrit.server.update.RetryHelper.execute(RetryHelper.java:271)
at com.google.gerrit.httpd.restapi.RestApiServlet.invokeRestEndpointWithRetry(RestApiServlet.java:820)
at com.google.gerrit.httpd.restapi.RestApiServlet.invokeRestModifyViewWithRetry(RestApiServlet.java:735)
at com.google.gerrit.httpd.restapi.RestApiServlet.service(RestApiServlet.java:511)
at javax.servlet.http.HttpServlet.service(HttpServlet.java:717)
...
Signed-off-by: Edwin Kempin <ekempin@google.com>
Change-Id: I0e136153cc38946f8c4f5abdf4aabc0ec4aa801a
diff --git a/java/com/google/gerrit/server/restapi/group/DeleteMembers.java b/java/com/google/gerrit/server/restapi/group/DeleteMembers.java
index 9bae2e2..fa1c5c6 100644
--- a/java/com/google/gerrit/server/restapi/group/DeleteMembers.java
+++ b/java/com/google/gerrit/server/restapi/group/DeleteMembers.java
@@ -14,6 +14,7 @@
package com.google.gerrit.server.restapi.group;
+import com.google.common.base.Strings;
import com.google.common.collect.Sets;
import com.google.gerrit.common.data.GroupDescription;
import com.google.gerrit.entities.Account;
@@ -68,6 +69,9 @@
Set<Account.Id> membersToRemove = new HashSet<>();
for (String nameOrEmail : input.members) {
+ if (Strings.isNullOrEmpty(nameOrEmail)) {
+ continue;
+ }
membersToRemove.add(accountResolver.resolve(nameOrEmail).asUnique().account().id());
}
AccountGroup.UUID groupUuid = internalGroup.getGroupUUID();
diff --git a/javatests/com/google/gerrit/acceptance/api/group/GroupsIT.java b/javatests/com/google/gerrit/acceptance/api/group/GroupsIT.java
index e543976..988580e 100644
--- a/javatests/com/google/gerrit/acceptance/api/group/GroupsIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/group/GroupsIT.java
@@ -187,6 +187,24 @@
}
@Test
+ public void removeMember_nullInMemberInputDoesNotCauseFailure() throws Exception {
+ AccountGroup.UUID group =
+ groupOperations.newGroup().addMember(admin.id()).addMember(user.id()).create();
+ gApi.groups().id(group.get()).removeMembers(user.id().toString(), null);
+ ImmutableSet<Account.Id> members = groupOperations.group(group).get().members();
+ assertThat(members).containsExactly(admin.id());
+ }
+
+ @Test
+ public void removeMember_emptyStringInMemberInputDoesNotCauseFailure() throws Exception {
+ AccountGroup.UUID group =
+ groupOperations.newGroup().addMember(admin.id()).addMember(user.id()).create();
+ gApi.groups().id(group.get()).removeMembers(user.id().toString(), "");
+ ImmutableSet<Account.Id> members = groupOperations.group(group).get().members();
+ assertThat(members).containsExactly(admin.id());
+ }
+
+ @Test
public void cachedGroupsForMemberAreUpdatedOnMemberAdditionAndRemoval() throws Exception {
String username = name("user");
Account.Id accountId = accountOperations.newAccount().username(username).create();
