Bump gerrit plugins dep hashes, and for code-owners rebase it against master. Change-Id: If7da0ca391b4a5c0102560ca8d52b6f5a2dfd223 Reviewed-on: https://cl.tvl.fyi/c/depot/+/9734 Autosubmit: lukegb <lukegb@tvl.fyi> Tested-by: BuildkiteCI Reviewed-by: lukegb <lukegb@tvl.fyi>
		
			
				
	
	
		
			472 lines
		
	
	
	
		
			23 KiB
		
	
	
	
		
			Diff
		
	
	
	
	
	
			
		
		
	
	
			472 lines
		
	
	
	
		
			23 KiB
		
	
	
	
		
			Diff
		
	
	
	
	
	
| commit 29ace6c38ac513f7ec56ca425230d5712c081043
 | |
| Author: Luke Granger-Brown <git@lukegb.com>
 | |
| Date:   Wed Sep 21 03:15:38 2022 +0100
 | |
| 
 | |
|     Add support for usernames and groups
 | |
|     
 | |
|     Change-Id: I3ba8527f66216d08e555a6ac4451fe0d1e090de5
 | |
| 
 | |
| diff --git a/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerResolver.java b/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerResolver.java
 | |
| index 70009591..6dc596c9 100644
 | |
| --- a/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerResolver.java
 | |
| +++ b/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerResolver.java
 | |
| @@ -17,6 +17,8 @@ package com.google.gerrit.plugins.codeowners.backend;
 | |
|  import static com.google.common.base.Preconditions.checkState;
 | |
|  import static com.google.common.collect.ImmutableMap.toImmutableMap;
 | |
|  import static com.google.common.collect.ImmutableSet.toImmutableSet;
 | |
| +import static com.google.common.collect.ImmutableSetMultimap.flatteningToImmutableSetMultimap;
 | |
| +import static com.google.common.collect.ImmutableSetMultimap.toImmutableSetMultimap;
 | |
|  import static com.google.gerrit.plugins.codeowners.backend.CodeOwnersInternalServerErrorException.newInternalServerError;
 | |
|  import static java.util.Objects.requireNonNull;
 | |
|  
 | |
| @@ -25,6 +27,7 @@ import com.google.common.collect.ImmutableList;
 | |
|  import com.google.common.collect.ImmutableMap;
 | |
|  import com.google.common.collect.ImmutableMultimap;
 | |
|  import com.google.common.collect.ImmutableSet;
 | |
| +import com.google.common.collect.ImmutableSetMultimap;
 | |
|  import com.google.common.collect.Iterables;
 | |
|  import com.google.common.collect.Streams;
 | |
|  import com.google.common.flogger.FluentLogger;
 | |
| @@ -33,17 +36,24 @@ import com.google.gerrit.entities.Project;
 | |
|  import com.google.gerrit.metrics.Timer0;
 | |
|  import com.google.gerrit.plugins.codeowners.backend.config.CodeOwnersPluginConfiguration;
 | |
|  import com.google.gerrit.plugins.codeowners.metrics.CodeOwnerMetrics;
 | |
| +import com.google.gerrit.server.AnonymousUser;
 | |
|  import com.google.gerrit.server.CurrentUser;
 | |
|  import com.google.gerrit.server.IdentifiedUser;
 | |
|  import com.google.gerrit.server.account.AccountCache;
 | |
|  import com.google.gerrit.server.account.AccountControl;
 | |
|  import com.google.gerrit.server.account.AccountState;
 | |
| +import com.google.gerrit.server.account.GroupBackend;
 | |
| +import com.google.gerrit.server.account.GroupBackends;
 | |
| +import com.google.gerrit.server.account.InternalGroupBackend;
 | |
|  import com.google.gerrit.server.account.externalids.ExternalId;
 | |
|  import com.google.gerrit.server.account.externalids.ExternalIdCache;
 | |
|  import com.google.gerrit.server.permissions.GlobalPermission;
 | |
|  import com.google.gerrit.server.permissions.PermissionBackend;
 | |
|  import com.google.gerrit.server.permissions.PermissionBackendException;
 | |
| +import com.google.gerrit.server.util.RequestContext;
 | |
| +import com.google.gerrit.server.util.ThreadLocalRequestContext;
 | |
|  import com.google.inject.Inject;
 | |
| +import com.google.inject.OutOfScopeException;
 | |
|  import com.google.inject.Provider;
 | |
|  import java.io.IOException;
 | |
|  import java.nio.file.Path;
 | |
| @@ -102,6 +112,8 @@ public class CodeOwnerResolver {
 | |
|  
 | |
|    @VisibleForTesting public static final String ALL_USERS_WILDCARD = "*";
 | |
|  
 | |
| +  public static final String GROUP_PREFIX = "group:";
 | |
| +
 | |
|    private final CodeOwnersPluginConfiguration codeOwnersPluginConfiguration;
 | |
|    private final PermissionBackend permissionBackend;
 | |
|    private final Provider<CurrentUser> currentUser;
 | |
| @@ -112,6 +124,8 @@ public class CodeOwnerResolver {
 | |
|    private final CodeOwnerMetrics codeOwnerMetrics;
 | |
|    private final UnresolvedImportFormatter unresolvedImportFormatter;
 | |
|    private final TransientCodeOwnerCache transientCodeOwnerCache;
 | |
| +  private final InternalGroupBackend groupBackend;
 | |
| +  private final ThreadLocalRequestContext context;
 | |
|  
 | |
|    // Enforce visibility by default.
 | |
|    private boolean enforceVisibility = true;
 | |
| @@ -132,7 +146,9 @@ public class CodeOwnerResolver {
 | |
|        PathCodeOwners.Factory pathCodeOwnersFactory,
 | |
|        CodeOwnerMetrics codeOwnerMetrics,
 | |
|        UnresolvedImportFormatter unresolvedImportFormatter,
 | |
| -      TransientCodeOwnerCache transientCodeOwnerCache) {
 | |
| +      TransientCodeOwnerCache transientCodeOwnerCache,
 | |
| +      InternalGroupBackend groupBackend,
 | |
| +      ThreadLocalRequestContext context) {
 | |
|      this.codeOwnersPluginConfiguration = codeOwnersPluginConfiguration;
 | |
|      this.permissionBackend = permissionBackend;
 | |
|      this.currentUser = currentUser;
 | |
| @@ -143,6 +159,8 @@ public class CodeOwnerResolver {
 | |
|      this.codeOwnerMetrics = codeOwnerMetrics;
 | |
|      this.unresolvedImportFormatter = unresolvedImportFormatter;
 | |
|      this.transientCodeOwnerCache = transientCodeOwnerCache;
 | |
| +    this.groupBackend = groupBackend;
 | |
| +    this.context = context;
 | |
|    }
 | |
|  
 | |
|    /**
 | |
| @@ -361,6 +379,12 @@ public class CodeOwnerResolver {
 | |
|                "cannot resolve code owner email %s: no account with this email exists",
 | |
|                CodeOwnerResolver.ALL_USERS_WILDCARD));
 | |
|      }
 | |
| +    if (codeOwnerReference.email().startsWith(GROUP_PREFIX)) {
 | |
| +      return OptionalResultWithMessages.createEmpty(
 | |
| +          String.format(
 | |
| +              "cannot resolve code owner email %s: this is a group",
 | |
| +              codeOwnerReference.email()));
 | |
| +    }
 | |
|  
 | |
|      ImmutableList.Builder<String> messageBuilder = ImmutableList.builder();
 | |
|      AtomicBoolean ownedByAllUsers = new AtomicBoolean(false);
 | |
| @@ -405,9 +429,53 @@ public class CodeOwnerResolver {
 | |
|        ImmutableMultimap<CodeOwnerReference, CodeOwnerAnnotation> annotations) {
 | |
|      requireNonNull(codeOwnerReferences, "codeOwnerReferences");
 | |
|  
 | |
| +    ImmutableSet<String> groupsToResolve =
 | |
| +        codeOwnerReferences.stream()
 | |
| +            .map(CodeOwnerReference::email)
 | |
| +            .filter(ref -> ref.startsWith(GROUP_PREFIX))
 | |
| +            .map(ref -> ref.substring(GROUP_PREFIX.length()))
 | |
| +            .collect(toImmutableSet());
 | |
| +
 | |
| +    // When we call GroupBackends.findExactSuggestion we need to ensure that we
 | |
| +    // have a user in context.  This is because the suggestion backend is
 | |
| +    // likely to want to try to check that we can actually see the group it's
 | |
| +    // returning (which we also check for explicitly, because I have trust
 | |
| +    // issues).
 | |
| +    RequestContext oldCtx = context.getContext();
 | |
| +    // Check if we have a user in the context at all...
 | |
| +    try {
 | |
| +      oldCtx.getUser();
 | |
| +    } catch (OutOfScopeException | NullPointerException e) {
 | |
| +      // Nope.
 | |
| +      RequestContext newCtx = () -> {
 | |
| +        return new AnonymousUser();
 | |
| +      };
 | |
| +      context.setContext(newCtx);
 | |
| +    }
 | |
| +    ImmutableSetMultimap<String, CodeOwner> resolvedGroups = null;
 | |
| +    try {
 | |
| +      resolvedGroups =
 | |
| +          groupsToResolve.stream()
 | |
| +              .map(groupName -> GroupBackends.findExactSuggestion(groupBackend, groupName))
 | |
| +              .filter(groupRef -> groupRef != null)
 | |
| +              .filter(groupRef -> groupBackend.isVisibleToAll(groupRef.getUUID()))
 | |
| +              .map(groupRef -> groupBackend.get(groupRef.getUUID()))
 | |
| +              .collect(flatteningToImmutableSetMultimap(
 | |
| +                    groupRef -> GROUP_PREFIX + groupRef.getName(),
 | |
| +                    groupRef -> accountCache
 | |
| +                        .get(ImmutableSet.copyOf(groupRef.getMembers()))
 | |
| +                        .values().stream()
 | |
| +                        .map(accountState -> CodeOwner.create(accountState.account().id()))));
 | |
| +    } finally {
 | |
| +      context.setContext(oldCtx);
 | |
| +    }
 | |
| +    ImmutableSetMultimap<CodeOwner, String> usersToGroups =
 | |
| +        resolvedGroups.inverse();
 | |
| +
 | |
|      ImmutableSet<String> emailsToResolve =
 | |
|          codeOwnerReferences.stream()
 | |
|              .map(CodeOwnerReference::email)
 | |
| +            .filter(ref -> !ref.startsWith(GROUP_PREFIX))
 | |
|              .filter(filterOutAllUsersWildCard(ownedByAllUsers))
 | |
|              .collect(toImmutableSet());
 | |
|  
 | |
| @@ -442,7 +510,8 @@ public class CodeOwnerResolver {
 | |
|      ImmutableMap<String, CodeOwner> codeOwnersByEmail =
 | |
|          accountsByEmail.map(mapToCodeOwner()).collect(toImmutableMap(Pair::key, Pair::value));
 | |
|  
 | |
| -    if (codeOwnersByEmail.keySet().size() < emailsToResolve.size()) {
 | |
| +    if (codeOwnersByEmail.keySet().size() < emailsToResolve.size() ||
 | |
| +        resolvedGroups.keySet().size() < groupsToResolve.size()) {
 | |
|        hasUnresolvedCodeOwners.set(true);
 | |
|      }
 | |
|  
 | |
| @@ -456,7 +525,9 @@ public class CodeOwnerResolver {
 | |
|          cachedCodeOwnersByEmail.entrySet().stream()
 | |
|              .filter(e -> e.getValue().isPresent())
 | |
|              .map(e -> Pair.of(e.getKey(), e.getValue().get()));
 | |
| -    Streams.concat(newlyResolvedCodeOwnersStream, cachedCodeOwnersStream)
 | |
| +    Stream<Pair<String, CodeOwner>> resolvedGroupsCodeOwnersStream =
 | |
| +        resolvedGroups.entries().stream().map(e -> Pair.of(e.getKey(), e.getValue()));
 | |
| +    Streams.concat(Streams.concat(newlyResolvedCodeOwnersStream, cachedCodeOwnersStream), resolvedGroupsCodeOwnersStream)
 | |
|          .forEach(
 | |
|              p -> {
 | |
|                ImmutableSet.Builder<CodeOwnerAnnotation> annotationBuilder = ImmutableSet.builder();
 | |
| @@ -467,6 +538,12 @@ public class CodeOwnerResolver {
 | |
|                annotationBuilder.addAll(
 | |
|                    annotations.get(CodeOwnerReference.create(ALL_USERS_WILDCARD)));
 | |
|  
 | |
| +              // annotations for the groups this user is in apply as well
 | |
| +              for (String group : usersToGroups.get(p.value())) {
 | |
| +                annotationBuilder.addAll(
 | |
| +                    annotations.get(CodeOwnerReference.create(group)));
 | |
| +              }
 | |
| +
 | |
|                if (!codeOwnersWithAnnotations.containsKey(p.value())) {
 | |
|                  codeOwnersWithAnnotations.put(p.value(), new HashSet<>());
 | |
|                }
 | |
| @@ -570,7 +647,7 @@ public class CodeOwnerResolver {
 | |
|      }
 | |
|  
 | |
|      messages.add(String.format("email %s has no domain", email));
 | |
| -    return false;
 | |
| +    return true;  // TVL: we allow domain-less strings which are treated as usernames.
 | |
|    }
 | |
|  
 | |
|    /**
 | |
| @@ -585,11 +662,29 @@ public class CodeOwnerResolver {
 | |
|     */
 | |
|    private ImmutableMap<String, Collection<ExternalId>> lookupExternalIds(
 | |
|        ImmutableList.Builder<String> messages, ImmutableSet<String> emails) {
 | |
| +    String[] actualEmails = emails.stream()
 | |
| +      .filter(email -> email.contains("@"))
 | |
| +      .toArray(String[]::new);
 | |
| +    ImmutableSet<String> usernames = emails.stream()
 | |
| +      .filter(email -> !email.contains("@"))
 | |
| +      .collect(ImmutableSet.toImmutableSet());
 | |
|      try {
 | |
| -      ImmutableMap<String, Collection<ExternalId>> extIdsByEmail =
 | |
| -          externalIdCache.byEmails(emails.toArray(new String[0])).asMap();
 | |
| +      ImmutableMap<String, Collection<ExternalId>> extIds =
 | |
| +          new ImmutableMap.Builder<String, Collection<ExternalId>>()
 | |
| +              .putAll(externalIdCache.byEmails(actualEmails).asMap())
 | |
| +              .putAll(externalIdCache.allByAccount().entries().stream()
 | |
| +                  .map(entry -> entry.getValue())
 | |
| +                  .filter(externalId ->
 | |
| +                      externalId.key().scheme() != null &&
 | |
| +                      externalId.key().isScheme(ExternalId.SCHEME_USERNAME) &&
 | |
| +                      usernames.contains(externalId.key().id()))
 | |
| +                  .collect(toImmutableSetMultimap(
 | |
| +                      externalId -> externalId.key().id(),
 | |
| +                      externalId -> externalId))
 | |
| +                  .asMap())
 | |
| +              .build();
 | |
|        emails.stream()
 | |
| -          .filter(email -> !extIdsByEmail.containsKey(email))
 | |
| +          .filter(email -> !extIds.containsKey(email))
 | |
|            .forEach(
 | |
|                email -> {
 | |
|                  transientCodeOwnerCache.cacheNonResolvable(email);
 | |
| @@ -598,7 +693,7 @@ public class CodeOwnerResolver {
 | |
|                          "cannot resolve code owner email %s: no account with this email exists",
 | |
|                          email));
 | |
|                });
 | |
| -      return extIdsByEmail;
 | |
| +      return extIds;
 | |
|      } catch (IOException e) {
 | |
|        throw newInternalServerError(
 | |
|            String.format("cannot resolve code owner emails: %s", emails), e);
 | |
| @@ -815,6 +910,15 @@ public class CodeOwnerResolver {
 | |
|                  user != null ? user.getLoggableName() : currentUser.get().getLoggableName()));
 | |
|          return true;
 | |
|        }
 | |
| +      if (!email.contains("@")) {
 | |
| +        // the email is the username of the account, or a group, or something else.
 | |
| +        messages.add(
 | |
| +            String.format(
 | |
| +                "account %s is visible to user %s",
 | |
| +                accountState.account().id(),
 | |
| +                user != null ? user.getLoggableName() : currentUser.get().getLoggableName()));
 | |
| +        return true;
 | |
| +      }
 | |
|  
 | |
|        if (user != null) {
 | |
|          if (user.hasEmailAddress(email)) {
 | |
| diff --git a/java/com/google/gerrit/plugins/codeowners/backend/findowners/FindOwnersCodeOwnerConfigParser.java b/java/com/google/gerrit/plugins/codeowners/backend/findowners/FindOwnersCodeOwnerConfigParser.java
 | |
| index 5f350998..7977ba55 100644
 | |
| --- a/java/com/google/gerrit/plugins/codeowners/backend/findowners/FindOwnersCodeOwnerConfigParser.java
 | |
| +++ b/java/com/google/gerrit/plugins/codeowners/backend/findowners/FindOwnersCodeOwnerConfigParser.java
 | |
| @@ -149,7 +149,8 @@ public class FindOwnersCodeOwnerConfigParser implements CodeOwnerConfigParser {
 | |
|      private static final String EOL = "[\\s]*(#.*)?$"; // end-of-line
 | |
|      private static final String GLOB = "[^\\s,=]+"; // a file glob
 | |
|  
 | |
| -    private static final String EMAIL_OR_STAR = "([^\\s<>@,]+@[^\\s<>@#,]+|\\*)";
 | |
| +    // Also allows usernames, and group:$GROUP_NAME.
 | |
| +    private static final String EMAIL_OR_STAR = "([^\\s<>@,]+@[^\\s<>@#,]+?|\\*|[a-zA-Z0-9_\\-]+|group:[a-zA-Z0-9_\\-]+)";
 | |
|      private static final String EMAIL_LIST =
 | |
|          "(" + EMAIL_OR_STAR + "(" + COMMA + EMAIL_OR_STAR + ")*)";
 | |
|  
 | |
| diff --git a/javatests/com/google/gerrit/plugins/codeowners/backend/AbstractFileBasedCodeOwnerBackendTest.java b/javatests/com/google/gerrit/plugins/codeowners/backend/AbstractFileBasedCodeOwnerBackendTest.java
 | |
| index 7ec92959..59cf7e05 100644
 | |
| --- a/javatests/com/google/gerrit/plugins/codeowners/backend/AbstractFileBasedCodeOwnerBackendTest.java
 | |
| +++ b/javatests/com/google/gerrit/plugins/codeowners/backend/AbstractFileBasedCodeOwnerBackendTest.java
 | |
| @@ -424,7 +424,7 @@ public abstract class AbstractFileBasedCodeOwnerBackendTest extends AbstractCode
 | |
|                .commit()
 | |
|                .parent(head)
 | |
|                .message("Add invalid test code owner config")
 | |
| -              .add(JgitPath.of(codeOwnerConfigKey.filePath(getFileName())).get(), "INVALID"));
 | |
| +              .add(JgitPath.of(codeOwnerConfigKey.filePath(getFileName())).get(), "INVALID!"));
 | |
|      }
 | |
|  
 | |
|      // Try to update the code owner config.
 | |
| diff --git a/javatests/com/google/gerrit/plugins/codeowners/backend/CodeOwnerResolverTest.java b/javatests/com/google/gerrit/plugins/codeowners/backend/CodeOwnerResolverTest.java
 | |
| index 6171aca9..37699012 100644
 | |
| --- a/javatests/com/google/gerrit/plugins/codeowners/backend/CodeOwnerResolverTest.java
 | |
| +++ b/javatests/com/google/gerrit/plugins/codeowners/backend/CodeOwnerResolverTest.java
 | |
| @@ -24,8 +24,10 @@ import com.google.gerrit.acceptance.TestAccount;
 | |
|  import com.google.gerrit.acceptance.TestMetricMaker;
 | |
|  import com.google.gerrit.acceptance.config.GerritConfig;
 | |
|  import com.google.gerrit.acceptance.testsuite.account.AccountOperations;
 | |
| +import com.google.gerrit.acceptance.testsuite.group.GroupOperations;
 | |
|  import com.google.gerrit.acceptance.testsuite.request.RequestScopeOperations;
 | |
|  import com.google.gerrit.entities.Account;
 | |
| +import com.google.gerrit.entities.AccountGroup;
 | |
|  import com.google.gerrit.plugins.codeowners.acceptance.AbstractCodeOwnersTest;
 | |
|  import com.google.gerrit.server.ServerInitiated;
 | |
|  import com.google.gerrit.server.account.AccountsUpdate;
 | |
| @@ -51,6 +53,7 @@ public class CodeOwnerResolverTest extends AbstractCodeOwnersTest {
 | |
|    @Inject private RequestScopeOperations requestScopeOperations;
 | |
|    @Inject @ServerInitiated private Provider<AccountsUpdate> accountsUpdate;
 | |
|    @Inject private AccountOperations accountOperations;
 | |
| +  @Inject private GroupOperations groupOperations;
 | |
|    @Inject private ExternalIdNotes.Factory externalIdNotesFactory;
 | |
|    @Inject private TestMetricMaker testMetricMaker;
 | |
|    @Inject private ExternalIdFactory externalIdFactory;
 | |
| @@ -112,6 +115,18 @@ public class CodeOwnerResolverTest extends AbstractCodeOwnersTest {
 | |
|          .contains(String.format("account %s is visible to user %s", admin.id(), admin.username()));
 | |
|    }
 | |
|  
 | |
| +  @Test
 | |
| +  public void resolveCodeOwnerReferenceForUsername() throws Exception {
 | |
| +    OptionalResultWithMessages<CodeOwner> result =
 | |
| +        codeOwnerResolverProvider
 | |
| +            .get()
 | |
| +            .resolveWithMessages(CodeOwnerReference.create(admin.username()));
 | |
| +    assertThat(result.get()).hasAccountIdThat().isEqualTo(admin.id());
 | |
| +    assertThat(result)
 | |
| +        .hasMessagesThat()
 | |
| +        .contains(String.format("account %s is visible to user %s", admin.id(), admin.username()));
 | |
| +  }
 | |
| +
 | |
|    @Test
 | |
|    public void cannotResolveCodeOwnerReferenceForStarAsEmail() throws Exception {
 | |
|      OptionalResultWithMessages<CodeOwner> result =
 | |
| @@ -127,6 +142,18 @@ public class CodeOwnerResolverTest extends AbstractCodeOwnersTest {
 | |
|                  CodeOwnerResolver.ALL_USERS_WILDCARD));
 | |
|    }
 | |
|  
 | |
| +  @Test
 | |
| +  public void cannotResolveCodeOwnerReferenceForGroup() throws Exception {
 | |
| +    OptionalResultWithMessages<CodeOwner> result =
 | |
| +        codeOwnerResolverProvider
 | |
| +            .get()
 | |
| +            .resolveWithMessages(CodeOwnerReference.create("group:Administrators"));
 | |
| +    assertThat(result).isEmpty();
 | |
| +    assertThat(result)
 | |
| +        .hasMessagesThat()
 | |
| +        .contains("cannot resolve code owner email group:Administrators: this is a group");
 | |
| +  }
 | |
| +
 | |
|    @Test
 | |
|    public void resolveCodeOwnerReferenceForAmbiguousEmailIfOtherAccountIsInactive()
 | |
|        throws Exception {
 | |
| @@ -397,6 +424,64 @@ public class CodeOwnerResolverTest extends AbstractCodeOwnersTest {
 | |
|      assertThat(result.hasUnresolvedCodeOwners()).isFalse();
 | |
|    }
 | |
|  
 | |
| +  @Test
 | |
| +  public void resolvePathCodeOwnersWhenNonVisibleGroupIsUsed() throws Exception {
 | |
| +    CodeOwnerConfig codeOwnerConfig =
 | |
| +        CodeOwnerConfig.builder(CodeOwnerConfig.Key.create(project, "master", "/"), TEST_REVISION)
 | |
| +            .addCodeOwnerSet(
 | |
| +                CodeOwnerSet.createWithoutPathExpressions("group:Administrators"))
 | |
| +            .build();
 | |
| +
 | |
| +    CodeOwnerResolverResult result =
 | |
| +        codeOwnerResolverProvider
 | |
| +            .get()
 | |
| +            .resolvePathCodeOwners(codeOwnerConfig, Paths.get("/README.md"));
 | |
| +    assertThat(result.codeOwnersAccountIds()).isEmpty();
 | |
| +    assertThat(result.ownedByAllUsers()).isFalse();
 | |
| +    assertThat(result.hasUnresolvedCodeOwners()).isTrue();
 | |
| +  }
 | |
| +
 | |
| +  @Test
 | |
| +  public void resolvePathCodeOwnersWhenVisibleGroupIsUsed() throws Exception {
 | |
| +    AccountGroup.UUID createdGroupUUID = groupOperations
 | |
| +        .newGroup()
 | |
| +        .name("VisibleGroup")
 | |
| +        .visibleToAll(true)
 | |
| +        .addMember(admin.id())
 | |
| +        .create();
 | |
| +
 | |
| +    CodeOwnerConfig codeOwnerConfig =
 | |
| +        CodeOwnerConfig.builder(CodeOwnerConfig.Key.create(project, "master", "/"), TEST_REVISION)
 | |
| +            .addCodeOwnerSet(
 | |
| +                CodeOwnerSet.createWithoutPathExpressions("group:VisibleGroup"))
 | |
| +            .build();
 | |
| +
 | |
| +    CodeOwnerResolverResult result =
 | |
| +        codeOwnerResolverProvider
 | |
| +            .get()
 | |
| +            .resolvePathCodeOwners(codeOwnerConfig, Paths.get("/README.md"));
 | |
| +    assertThat(result.codeOwnersAccountIds()).containsExactly(admin.id());
 | |
| +    assertThat(result.ownedByAllUsers()).isFalse();
 | |
| +    assertThat(result.hasUnresolvedCodeOwners()).isFalse();
 | |
| +  }
 | |
| +
 | |
| +  @Test
 | |
| +  public void resolvePathCodeOwnersWhenUsernameIsUsed() throws Exception {
 | |
| +    CodeOwnerConfig codeOwnerConfig =
 | |
| +        CodeOwnerConfig.builder(CodeOwnerConfig.Key.create(project, "master", "/"), TEST_REVISION)
 | |
| +            .addCodeOwnerSet(
 | |
| +                CodeOwnerSet.createWithoutPathExpressions(admin.username()))
 | |
| +            .build();
 | |
| +
 | |
| +    CodeOwnerResolverResult result =
 | |
| +        codeOwnerResolverProvider
 | |
| +            .get()
 | |
| +            .resolvePathCodeOwners(codeOwnerConfig, Paths.get("/README.md"));
 | |
| +    assertThat(result.codeOwnersAccountIds()).containsExactly(admin.id());
 | |
| +    assertThat(result.ownedByAllUsers()).isFalse();
 | |
| +    assertThat(result.hasUnresolvedCodeOwners()).isFalse();
 | |
| +  }
 | |
| +
 | |
|    @Test
 | |
|    public void resolvePathCodeOwnersNonResolvableCodeOwnersAreFilteredOut() throws Exception {
 | |
|      CodeOwnerConfig codeOwnerConfig =
 | |
| @@ -655,7 +740,7 @@ public class CodeOwnerResolverTest extends AbstractCodeOwnersTest {
 | |
|          "domain example.com of email foo@example.org@example.com is allowed");
 | |
|      assertIsEmailDomainAllowed(
 | |
|          "foo@example.org", false, "domain example.org of email foo@example.org is not allowed");
 | |
| -    assertIsEmailDomainAllowed("foo", false, "email foo has no domain");
 | |
| +    assertIsEmailDomainAllowed("foo", true, "email foo has no domain");
 | |
|      assertIsEmailDomainAllowed(
 | |
|          "foo@example.com@example.org",
 | |
|          false,
 | |
| diff --git a/javatests/com/google/gerrit/plugins/codeowners/backend/findowners/FindOwnersCodeOwnerConfigParserTest.java b/javatests/com/google/gerrit/plugins/codeowners/backend/findowners/FindOwnersCodeOwnerConfigParserTest.java
 | |
| index 260e635e..7aab99d0 100644
 | |
| --- a/javatests/com/google/gerrit/plugins/codeowners/backend/findowners/FindOwnersCodeOwnerConfigParserTest.java
 | |
| +++ b/javatests/com/google/gerrit/plugins/codeowners/backend/findowners/FindOwnersCodeOwnerConfigParserTest.java
 | |
| @@ -158,16 +158,42 @@ public class FindOwnersCodeOwnerConfigParserTest extends AbstractCodeOwnerConfig
 | |
|                  codeOwnerConfigParser.parse(
 | |
|                      TEST_REVISION,
 | |
|                      CodeOwnerConfig.Key.create(project, "master", "/"),
 | |
| -                    getCodeOwnerConfig(EMAIL_1, "INVALID", "NOT_AN_EMAIL", EMAIL_2)));
 | |
| +                    getCodeOwnerConfig(EMAIL_1, "INVALID!", "NOT!AN_EMAIL", EMAIL_2)));
 | |
|      assertThat(exception.getFullMessage(FindOwnersBackend.CODE_OWNER_CONFIG_FILE_NAME))
 | |
|          .isEqualTo(
 | |
|              String.format(
 | |
|                  "invalid code owner config file '/OWNERS' (project = %s, branch = master):\n"
 | |
| -                    + "  invalid line: INVALID\n"
 | |
| -                    + "  invalid line: NOT_AN_EMAIL",
 | |
| +                    + "  invalid line: INVALID!\n"
 | |
| +                    + "  invalid line: NOT!AN_EMAIL",
 | |
|                  project));
 | |
|    }
 | |
|  
 | |
| +  @Test
 | |
| +  public void codeOwnerConfigWithUsernames() throws Exception {
 | |
| +    assertParseAndFormat(
 | |
| +        getCodeOwnerConfig(EMAIL_1, "USERNAME", EMAIL_2),
 | |
| +        codeOwnerConfig ->
 | |
| +            assertThat(codeOwnerConfig)
 | |
| +                .hasCodeOwnerSetsThat()
 | |
| +                .onlyElement()
 | |
| +                .hasCodeOwnersEmailsThat()
 | |
| +                .containsExactly(EMAIL_1, "USERNAME", EMAIL_2),
 | |
| +        getCodeOwnerConfig(EMAIL_1, "USERNAME", EMAIL_2));
 | |
| +  }
 | |
| +
 | |
| +  @Test
 | |
| +  public void codeOwnerConfigWithGroups() throws Exception {
 | |
| +    assertParseAndFormat(
 | |
| +        getCodeOwnerConfig(EMAIL_1, "group:tvl-employees", EMAIL_2),
 | |
| +        codeOwnerConfig ->
 | |
| +            assertThat(codeOwnerConfig)
 | |
| +                .hasCodeOwnerSetsThat()
 | |
| +                .onlyElement()
 | |
| +                .hasCodeOwnersEmailsThat()
 | |
| +                .containsExactly(EMAIL_1, "group:tvl-employees", EMAIL_2),
 | |
| +        getCodeOwnerConfig(EMAIL_1, "group:tvl-employees", EMAIL_2));
 | |
| +  }
 | |
| +
 | |
|    @Test
 | |
|    public void codeOwnerConfigWithComment() throws Exception {
 | |
|      assertParseAndFormat(
 |