CodeOwnerResolver: Return Optional from resolve method rather than Stream
The resolve method either returns a stream with exactly one code owner
or an empty stream. It's easier and more readable to just return an
Optional. Returning a stream is a left-over from when the resolve method
was resolving "*" to all users.
Signed-off-by: Edwin Kempin <ekempin@google.com>
Change-Id: I1a575727855e72dfea876facf142ef4534982315
diff --git a/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerResolver.java b/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerResolver.java
index ad239f0..c6b5f55 100644
--- a/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerResolver.java
+++ b/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerResolver.java
@@ -45,7 +45,6 @@
import java.util.Optional;
import java.util.Set;
import java.util.concurrent.atomic.AtomicBoolean;
-import java.util.stream.Stream;
/** Class to resolve {@link CodeOwnerReference}s to {@link CodeOwner}s. */
public class CodeOwnerResolver {
@@ -188,7 +187,9 @@
}
return true;
})
- .flatMap(this::resolve)
+ .map(this::resolve)
+ .filter(Optional::isPresent)
+ .map(Optional::get)
.collect(toImmutableSet());
return CodeOwnerResolverResult.create(codeOwners, ownedByAllUsers.get());
}
@@ -233,34 +234,34 @@
* @return the {@link CodeOwner} for the code owner reference if it was resolved, otherwise {@link
* Optional#empty()}
*/
- public Stream<CodeOwner> resolve(CodeOwnerReference codeOwnerReference) {
+ public Optional<CodeOwner> resolve(CodeOwnerReference codeOwnerReference) {
String email = requireNonNull(codeOwnerReference, "codeOwnerReference").email();
logger.atFine().log("resolving code owner reference %s", codeOwnerReference);
if (!isEmailDomainAllowed(email)) {
logger.atFine().log("domain of email %s is not allowed", email);
- return Stream.of();
+ return Optional.empty();
}
Optional<AccountState> accountState =
lookupEmail(email).flatMap(accountId -> lookupAccount(accountId, email));
if (!accountState.isPresent()) {
logger.atFine().log("no account for email %s", email);
- return Stream.of();
+ return Optional.empty();
}
if (!accountState.get().account().isActive()) {
logger.atFine().log("account for email %s is inactive", email);
- return Stream.of();
+ return Optional.empty();
}
if (enforceVisibility && !isVisible(accountState.get(), email)) {
logger.atFine().log(
"account %d or email %s not visible", accountState.get().account().id().get(), email);
- return Stream.of();
+ return Optional.empty();
}
CodeOwner codeOwner = CodeOwner.create(accountState.get().account().id());
logger.atFine().log("resolved to code owner %s", codeOwner);
- return Stream.of(codeOwner);
+ return Optional.of(codeOwner);
}
/** Whether the given account can be seen. */
diff --git a/java/com/google/gerrit/plugins/codeowners/restapi/RenameEmail.java b/java/com/google/gerrit/plugins/codeowners/restapi/RenameEmail.java
index e8092b3..0ed505b 100644
--- a/java/com/google/gerrit/plugins/codeowners/restapi/RenameEmail.java
+++ b/java/com/google/gerrit/plugins/codeowners/restapi/RenameEmail.java
@@ -14,13 +14,10 @@
package com.google.gerrit.plugins.codeowners.restapi;
-import static com.google.common.collect.ImmutableSet.toImmutableSet;
import static java.util.Objects.requireNonNull;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Strings;
-import com.google.common.collect.ImmutableSet;
-import com.google.common.collect.Iterables;
import com.google.gerrit.entities.Account;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.BadRequestException;
@@ -158,19 +155,14 @@
}
}
- private Account.Id resolveEmail(String email)
- throws ResourceConflictException, UnprocessableEntityException {
+ private Account.Id resolveEmail(String email) throws UnprocessableEntityException {
requireNonNull(email, "email");
- ImmutableSet<CodeOwner> codeOwners =
- codeOwnerResolver.resolve(CodeOwnerReference.create(email)).collect(toImmutableSet());
- if (codeOwners.isEmpty()) {
+ Optional<CodeOwner> codeOwner = codeOwnerResolver.resolve(CodeOwnerReference.create(email));
+ if (!codeOwner.isPresent()) {
throw new UnprocessableEntityException(String.format("cannot resolve email %s", email));
}
- if (codeOwners.size() > 1) {
- throw new ResourceConflictException(String.format("email %s is ambigious", email));
- }
- return Iterables.getOnlyElement(codeOwners).accountId();
+ return codeOwner.get().accountId();
}
/**
diff --git a/javatests/com/google/gerrit/plugins/codeowners/backend/CodeOwnerResolverTest.java b/javatests/com/google/gerrit/plugins/codeowners/backend/CodeOwnerResolverTest.java
index 6e19e28..cefe297 100644
--- a/javatests/com/google/gerrit/plugins/codeowners/backend/CodeOwnerResolverTest.java
+++ b/javatests/com/google/gerrit/plugins/codeowners/backend/CodeOwnerResolverTest.java
@@ -34,8 +34,8 @@
import com.google.inject.Key;
import com.google.inject.Provider;
import java.nio.file.Paths;
+import java.util.Optional;
import java.util.Set;
-import java.util.stream.Stream;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.Repository;
import org.junit.Before;
@@ -89,14 +89,14 @@
@Test
public void resolveCodeOwnerReferenceForEmail() throws Exception {
- Stream<CodeOwner> codeOwner =
+ Optional<CodeOwner> codeOwner =
codeOwnerResolver.get().resolve(CodeOwnerReference.create(admin.email()));
- assertThat(codeOwner).onlyElement().hasAccountIdThat().isEqualTo(admin.id());
+ assertThat(codeOwner).value().hasAccountIdThat().isEqualTo(admin.id());
}
@Test
public void cannotResolveCodeOwnerReferenceForStarAsEmail() throws Exception {
- Stream<CodeOwner> codeOwner =
+ Optional<CodeOwner> codeOwner =
codeOwnerResolver
.get()
.resolve(CodeOwnerReference.create(CodeOwnerResolver.ALL_USERS_WILDCARD));
@@ -160,14 +160,14 @@
// admin has the "Modify Account" global capability and hence can see the secondary email of the
// user account.
- Stream<CodeOwner> codeOwner =
+ Optional<CodeOwner> codeOwner =
codeOwnerResolver.get().resolve(CodeOwnerReference.create(secondaryEmail));
- assertThat(codeOwner).onlyElement().hasAccountIdThat().isEqualTo(user.id());
+ assertThat(codeOwner).value().hasAccountIdThat().isEqualTo(user.id());
// user can see its own secondary email.
requestScopeOperations.setApiUser(user.id());
codeOwner = codeOwnerResolver.get().resolve(CodeOwnerReference.create(secondaryEmail));
- assertThat(codeOwner).onlyElement().hasAccountIdThat().isEqualTo(user.id());
+ assertThat(codeOwner).value().hasAccountIdThat().isEqualTo(user.id());
}
@Test
@@ -297,9 +297,9 @@
assertThat(codeOwnerResolver.get().resolve(adminCodeOwnerReference)).isEmpty();
// if visibility is not enforced the code owner reference can be resolved regardless
- Stream<CodeOwner> codeOwner =
+ Optional<CodeOwner> codeOwner =
codeOwnerResolver.get().enforceVisibility(false).resolve(adminCodeOwnerReference);
- assertThat(codeOwner).onlyElement().hasAccountIdThat().isEqualTo(admin.id());
+ assertThat(codeOwner).value().hasAccountIdThat().isEqualTo(admin.id());
}
@Test
@@ -311,13 +311,13 @@
// admin is the current user and can see the account
assertThat(codeOwnerResolver.get().resolve(CodeOwnerReference.create(user.email())))
- .isNotEmpty();
+ .isPresent();
assertThat(
codeOwnerResolver
.get()
.forUser(identifiedUserFactory.create(admin.id()))
.resolve(CodeOwnerReference.create(user.email())))
- .isNotEmpty();
+ .isPresent();
// user2 cannot see the account
assertThat(
