opendev/gerrit
Code Issues Proposed changes

Expose 'Service User' tag on the REST API

Browse Source 
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
This commit is contained in:
Patrick Hiesel
2020-08-07 16:15:54 +02:00
parent e587c40aaa
commit 90d26fd2c4
11 changed files with 88 additions and 9 deletions
Download Patch File Download Diff File Expand all files Collapse all files

8
Documentation/rest-api-accounts.txt
View File

@@ -60,7 +60,7 @@ generally disabled by default. Optional fields are:
[[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 @@ Only set on the last account that is returned.
|`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]]

7
java/com/google/gerrit/acceptance/AccountCreator.java
View File

@@ -101,6 +101,7 @@ public class AccountCreator {
.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 @@ public class AccountCreator {
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);
}

8
java/com/google/gerrit/acceptance/TestAccount.java
View File

@@ -48,8 +48,10 @@ public abstract class TestAccount {
@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 @@ public abstract class TestAccount {
@Nullable
public abstract String httpPassword();
public abstract ImmutableList<String> tags();
public PersonIdent newIdent() {
return new PersonIdent(fullName(), email());
}

16
java/com/google/gerrit/extensions/common/AccountInfo.java
View File

@@ -27,6 +27,12 @@ import java.util.Objects;
* 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 @@ public class AccountInfo {
/** 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 @@ public class AccountInfo {
&& 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 @@ public class AccountInfo {
.add("displayname", displayName)
.add("email", email)
.add("username", username)
.add("tags", tags)
.toString();
}
@@ -116,7 +127,8 @@ public class AccountInfo {
username,
avatars,
_moreAccounts,
status);
status,
tags);
}
protected AccountInfo() {}

5
java/com/google/gerrit/server/account/AccountDirectory.java
View File

@@ -51,7 +51,10 @@ public abstract class AccountDirectory {
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)

3
java/com/google/gerrit/server/account/AccountLoader.java
View File

@@ -44,7 +44,8 @@ public class AccountLoader {
FillOptions.DISPLAY_NAME,
FillOptions.STATUS,
FillOptions.STATE,
FillOptions.AVATARS));
FillOptions.AVATARS,
FillOptions.TAGS));
public interface Factory {
AccountLoader create(boolean detailed);

13
java/com/google/gerrit/server/account/InternalAccountDirectory.java
View File

@@ -18,6 +18,7 @@ import static java.util.stream.Collectors.toList;
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 @@ public class InternalAccountDirectory extends AccountDirectory {
private final IdentifiedUser.GenericFactory userFactory;
private final Provider<CurrentUser> self;
private final PermissionBackend permissionBackend;
private final ServiceUserClassifier serviceUserClassifier;
@Inject
InternalAccountDirectory(
@@ -68,12 +70,14 @@ public class InternalAccountDirectory extends AccountDirectory {
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 @@ public class InternalAccountDirectory extends AccountDirectory {
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) {

1
javatests/com/google/gerrit/acceptance/api/accounts/AccountIT.java
View File

@@ -366,6 +366,7 @@ public class AccountIT extends AbstractDaemonTest {
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);

7
javatests/com/google/gerrit/acceptance/rest/account/AccountAssert.java
View File

@@ -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 @@ public class AccountAssert {
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());
}
}
/**

21
javatests/com/google/gerrit/acceptance/rest/account/GetAccountDetailIT.java
View File

@@ -19,11 +19,19 @@ import static com.google.gerrit.acceptance.rest.account.AccountAssert.assertAcco
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 @@ public class GetAccountDetailIT extends AbstractDaemonTest {
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);
}
}

8
javatests/com/google/gerrit/acceptance/rest/account/GetAccountIT.java
View File

@@ -67,6 +67,14 @@ public class GetAccountIT extends AbstractDaemonTest {
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());
}