Expose 'Service User' tag on the REST API We expose an array of tags on AccountInfo. The idea is that we can expose tags like whether the account is a 'service user' or 'admininistrator'. For now, the only exposable tag is 'SERVICE_USER'. This builds upon and expands an existing concept in Gerrit: We had the 'Non-Interactive Users' group for a while. Despite being used by some installations, it wasn't a very prominent concept in Gerrit. A previous commit renamed this group to 'Service Users'. This commit iteratively expands on this by exposing weather a user is a service user on the REST API. Change-Id: I9542f0f3ad1c765f4bfde951a72f2b6128d4b00b
diff --git a/Documentation/rest-api-accounts.txt b/Documentation/rest-api-accounts.txt index 86f546d..32f8656 100644 --- a/Documentation/rest-api-accounts.txt +++ b/Documentation/rest-api-accounts.txt
@@ -60,7 +60,7 @@ [[details]] -- * `DETAILS`: Includes full name, preferred email, username, display -name, avatars, status and state for each account. +name, avatars, status, state and tags for each account. -- [[all-emails]] @@ -2302,6 +2302,12 @@ |`status` |optional|Status message of the account. |`inactive` |not set if `false`| Whether the account is inactive. +|`tags` |optional, not set if empty| +List of additional tags that this account has. The only + +current tag an account can have is `SERVICE_USER`. + +Only set if detailed account information is requested. + +See option link:rest-api-changes.html#detailed-accounts[ +DETAILED_ACCOUNTS] |=============================== [[account-input]]
diff --git a/java/com/google/gerrit/acceptance/AccountCreator.java b/java/com/google/gerrit/acceptance/AccountCreator.java index 44e2d2d..3ab1cec 100644 --- a/java/com/google/gerrit/acceptance/AccountCreator.java +++ b/java/com/google/gerrit/acceptance/AccountCreator.java
@@ -101,6 +101,7 @@ .setPreferredEmail(email) .addExternalIds(extIds)); + ImmutableList.Builder<String> tags = ImmutableList.builder(); if (groupNames != null) { for (String n : groupNames) { AccountGroup.NameKey k = AccountGroup.nameKey(n); @@ -109,10 +110,14 @@ throw new NoSuchGroupException(n); } addGroupMember(group.get().getGroupUUID(), id); + if ("Service Users".equals(n)) { + tags.add("SERVICE_USER"); + } } } - account = TestAccount.create(id, username, email, fullName, displayName, httpPass); + account = + TestAccount.create(id, username, email, fullName, displayName, httpPass, tags.build()); if (username != null) { accounts.put(username, account); }
diff --git a/java/com/google/gerrit/acceptance/TestAccount.java b/java/com/google/gerrit/acceptance/TestAccount.java index e9c0899..d5908f4 100644 --- a/java/com/google/gerrit/acceptance/TestAccount.java +++ b/java/com/google/gerrit/acceptance/TestAccount.java
@@ -48,8 +48,10 @@ @Nullable String email, @Nullable String fullName, @Nullable String displayName, - @Nullable String httpPassword) { - return new AutoValue_TestAccount(id, username, email, fullName, displayName, httpPassword); + @Nullable String httpPassword, + ImmutableList<String> tags) { + return new AutoValue_TestAccount( + id, username, email, fullName, displayName, httpPassword, tags); } public abstract Account.Id id(); @@ -69,6 +71,8 @@ @Nullable public abstract String httpPassword(); + public abstract ImmutableList<String> tags(); + public PersonIdent newIdent() { return new PersonIdent(fullName(), email()); }
diff --git a/java/com/google/gerrit/extensions/common/AccountInfo.java b/java/com/google/gerrit/extensions/common/AccountInfo.java index 2a3d260..60ba18d 100644 --- a/java/com/google/gerrit/extensions/common/AccountInfo.java +++ b/java/com/google/gerrit/extensions/common/AccountInfo.java
@@ -27,6 +27,12 @@ * are defined in {@link AccountDetailInfo}. */ public class AccountInfo { + /** Tags are additional properties of an account. */ + public enum Tag { + /** Tag indicating that this account is a service user. */ + SERVICE_USER + } + /** The numeric ID of the account. */ public Integer _accountId; @@ -67,6 +73,9 @@ /** Whether the account is inactive. */ public Boolean inactive; + /** Tags, such as whether this account is a service user. */ + public List<Tag> tags; + public AccountInfo(Integer id) { this._accountId = id; } @@ -89,7 +98,8 @@ && Objects.equals(username, accountInfo.username) && Objects.equals(avatars, accountInfo.avatars) && Objects.equals(_moreAccounts, accountInfo._moreAccounts) - && Objects.equals(status, accountInfo.status); + && Objects.equals(status, accountInfo.status) + && Objects.equals(tags, accountInfo.tags); } return false; } @@ -102,6 +112,7 @@ .add("displayname", displayName) .add("email", email) .add("username", username) + .add("tags", tags) .toString(); } @@ -116,7 +127,8 @@ username, avatars, _moreAccounts, - status); + status, + tags); } protected AccountInfo() {}
diff --git a/java/com/google/gerrit/server/account/AccountDirectory.java b/java/com/google/gerrit/server/account/AccountDirectory.java index 63fa551..98b2ca9 100644 --- a/java/com/google/gerrit/server/account/AccountDirectory.java +++ b/java/com/google/gerrit/server/account/AccountDirectory.java
@@ -51,7 +51,10 @@ STATE, /** Human friendly display name presented in the web interface chosen by the user. */ - DISPLAY_NAME + DISPLAY_NAME, + + /** Tags such as weather the account is a service user. */ + TAGS } public abstract void fillAccountInfo(Iterable<? extends AccountInfo> in, Set<FillOptions> options)
diff --git a/java/com/google/gerrit/server/account/AccountLoader.java b/java/com/google/gerrit/server/account/AccountLoader.java index 9acf078..c260401 100644 --- a/java/com/google/gerrit/server/account/AccountLoader.java +++ b/java/com/google/gerrit/server/account/AccountLoader.java
@@ -44,7 +44,8 @@ FillOptions.DISPLAY_NAME, FillOptions.STATUS, FillOptions.STATE, - FillOptions.AVATARS)); + FillOptions.AVATARS, + FillOptions.TAGS)); public interface Factory { AccountLoader create(boolean detailed);
diff --git a/java/com/google/gerrit/server/account/InternalAccountDirectory.java b/java/com/google/gerrit/server/account/InternalAccountDirectory.java index 3137c95..13b71cf 100644 --- a/java/com/google/gerrit/server/account/InternalAccountDirectory.java +++ b/java/com/google/gerrit/server/account/InternalAccountDirectory.java
@@ -18,6 +18,7 @@ import static java.util.stream.Collectors.toSet; import com.google.common.base.Strings; +import com.google.common.collect.ImmutableList; import com.google.common.collect.Sets; import com.google.common.collect.Streams; import com.google.gerrit.entities.Account; @@ -61,6 +62,7 @@ private final IdentifiedUser.GenericFactory userFactory; private final Provider<CurrentUser> self; private final PermissionBackend permissionBackend; + private final ServiceUserClassifier serviceUserClassifier; @Inject InternalAccountDirectory( @@ -68,12 +70,14 @@ DynamicItem<AvatarProvider> avatar, IdentifiedUser.GenericFactory userFactory, Provider<CurrentUser> self, - PermissionBackend permissionBackend) { + PermissionBackend permissionBackend, + ServiceUserClassifier serviceUserClassifier) { this.accountCache = accountCache; this.avatar = avatar; this.userFactory = userFactory; this.self = self; this.permissionBackend = permissionBackend; + this.serviceUserClassifier = serviceUserClassifier; } @Override @@ -155,6 +159,13 @@ info.inactive = account.inactive() ? true : null; } + if (options.contains(FillOptions.TAGS)) { + info.tags = + serviceUserClassifier.isServiceUser(account.id()) + ? ImmutableList.of(AccountInfo.Tag.SERVICE_USER) + : null; + } + if (options.contains(FillOptions.AVATARS)) { AvatarProvider ap = avatar.get(); if (ap != null) {
diff --git a/javatests/com/google/gerrit/acceptance/api/accounts/AccountIT.java b/javatests/com/google/gerrit/acceptance/api/accounts/AccountIT.java index aaca9e8..1b55652 100644 --- a/javatests/com/google/gerrit/acceptance/api/accounts/AccountIT.java +++ b/javatests/com/google/gerrit/acceptance/api/accounts/AccountIT.java
@@ -366,6 +366,7 @@ assertThat(accountInfo.name).isEqualTo(input.name); assertThat(accountInfo.email).isEqualTo(input.email); assertThat(accountInfo.status).isNull(); + assertThat(accountInfo.tags).isNull(); Account.Id accountId = Account.id(accountInfo._accountId); accountIndexedCounter.assertReindexOf(accountId, 1);
diff --git a/javatests/com/google/gerrit/acceptance/rest/account/AccountAssert.java b/javatests/com/google/gerrit/acceptance/rest/account/AccountAssert.java index c0feda9..0b0f2ec 100644 --- a/javatests/com/google/gerrit/acceptance/rest/account/AccountAssert.java +++ b/javatests/com/google/gerrit/acceptance/rest/account/AccountAssert.java
@@ -14,6 +14,7 @@ package com.google.gerrit.acceptance.rest.account; +import static com.google.common.collect.ImmutableList.toImmutableList; import static com.google.common.truth.Truth.assertThat; import com.google.common.collect.Iterables; @@ -35,6 +36,12 @@ assertThat(accountInfo.displayName).isEqualTo(testAccount.displayName()); assertThat(accountInfo.email).isEqualTo(testAccount.email()); assertThat(accountInfo.inactive).isNull(); + if (testAccount.tags().isEmpty()) { + assertThat(accountInfo.tags).isNull(); + } else { + assertThat(accountInfo.tags.stream().map(Enum::name).collect(toImmutableList())) + .containsExactlyElementsIn(testAccount.tags()); + } } /**
diff --git a/javatests/com/google/gerrit/acceptance/rest/account/GetAccountDetailIT.java b/javatests/com/google/gerrit/acceptance/rest/account/GetAccountDetailIT.java index 9202f42..a5cf3e1 100644 --- a/javatests/com/google/gerrit/acceptance/rest/account/GetAccountDetailIT.java +++ b/javatests/com/google/gerrit/acceptance/rest/account/GetAccountDetailIT.java
@@ -19,11 +19,19 @@ import com.google.gerrit.acceptance.AbstractDaemonTest; import com.google.gerrit.acceptance.RestResponse; +import com.google.gerrit.acceptance.testsuite.account.AccountOperations; +import com.google.gerrit.acceptance.testsuite.group.GroupOperations; import com.google.gerrit.entities.Account; +import com.google.gerrit.entities.AccountGroup; import com.google.gerrit.extensions.common.AccountDetailInfo; +import com.google.gerrit.extensions.common.AccountInfo; +import com.google.inject.Inject; import org.junit.Test; public class GetAccountDetailIT extends AbstractDaemonTest { + @Inject private GroupOperations groupOperations; + @Inject private AccountOperations accountOperations; + @Test public void getDetail() throws Exception { RestResponse r = adminRestSession.get("/accounts/" + admin.username() + "/detail/"); @@ -32,4 +40,17 @@ Account account = getAccount(admin.id()); assertThat(info.registeredOn).isEqualTo(account.registeredOn()); } + + @Test + public void getDetailForServiceUser() throws Exception { + Account.Id serviceUser = accountOperations.newAccount().create(); + groupOperations + .group(groupCache.get(AccountGroup.nameKey("Service Users")).get().getGroupUUID()) + .forUpdate() + .addMember(serviceUser) + .update(); + RestResponse r = adminRestSession.get("/accounts/" + serviceUser.get() + "/detail/"); + AccountDetailInfo info = newGson().fromJson(r.getReader(), AccountDetailInfo.class); + assertThat(info.tags).containsExactly(AccountInfo.Tag.SERVICE_USER); + } }
diff --git a/javatests/com/google/gerrit/acceptance/rest/account/GetAccountIT.java b/javatests/com/google/gerrit/acceptance/rest/account/GetAccountIT.java index 782638a..2e2f5d9 100644 --- a/javatests/com/google/gerrit/acceptance/rest/account/GetAccountIT.java +++ b/javatests/com/google/gerrit/acceptance/rest/account/GetAccountIT.java
@@ -67,6 +67,14 @@ assertThat(accountInfo.inactive).isTrue(); } + @Test + public void getServiceUserAccount() throws Exception { + TestAccount serviceUser = + accountCreator.create("robot1", "robot1@example.com", "Ro Bot", "Ro", "Service Users"); + assertThat(serviceUser.tags()).containsExactly("SERVICE_USER"); + testGetAccount(serviceUser.id().toString(), serviceUser); + } + private void testGetAccount(String id, TestAccount expectedAccount) throws Exception { assertAccountInfo(expectedAccount, gApi.accounts().id(id).get()); }