Skip to content

Commit

Permalink
b/316247020 Rename Result class (#223)
Browse files Browse the repository at this point in the history
Use AnnotatedResult as a less generic name for the class
  • Loading branch information
jpassing authored Dec 17, 2023
1 parent 38f1365 commit 02b10b8
Show file tree
Hide file tree
Showing 5 changed files with 25 additions and 26 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
// under the License.
//

package com.google.solutions.jitaccess.core.entitlements;
package com.google.solutions.jitaccess.core;

import com.google.common.base.Preconditions;

Expand All @@ -29,7 +29,7 @@
/**
* Result list of T with an optional set of warnings.
*/
public class Result<T> {
public class AnnotatedResult<T> {
/**
* List of bindings. Might be incomplete if Warnings is non-empty.
*/
Expand All @@ -40,7 +40,7 @@ public class Result<T> {
*/
private final Set<String> warnings;

public Result(List<T> roleBindings, Set<String> warnings) {
public AnnotatedResult(List<T> roleBindings, Set<String> warnings) {
Preconditions.checkNotNull(roleBindings);

this.items = roleBindings;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ public record RoleBinding (
Preconditions.checkNotNull(fullResourceName, "fullResourceName");
Preconditions.checkNotNull(role, "role");
}

public RoleBinding(ProjectId project, String role) {
this(project.getFullResourceName(), role);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,7 @@
import com.google.api.services.cloudasset.v1.model.Expr;
import com.google.api.services.cloudasset.v1.model.IamPolicyAnalysis;
import com.google.common.base.Preconditions;
import com.google.solutions.jitaccess.core.AccessDeniedException;
import com.google.solutions.jitaccess.core.AccessException;
import com.google.solutions.jitaccess.core.ProjectId;
import com.google.solutions.jitaccess.core.UserId;
import com.google.solutions.jitaccess.core.*;
import com.google.solutions.jitaccess.core.clients.AssetInventoryClient;
import com.google.solutions.jitaccess.core.clients.ResourceManagerClient;
import jakarta.enterprise.context.ApplicationScoped;
Expand Down Expand Up @@ -161,7 +158,7 @@ public Set<ProjectId> listAvailableProjects(
/**
* List eligible role bindings for the given user.
*/
public Result<ProjectRole> listEligibleProjectRoles(
public AnnotatedResult<ProjectRole> listEligibleProjectRoles(
UserId user,
ProjectId projectId
) throws AccessException, IOException {
Expand All @@ -177,7 +174,7 @@ public Result<ProjectRole> listEligibleProjectRoles(
/**
* List eligible role bindings for the given user.
*/
public Result<ProjectRole> listEligibleProjectRoles(
public AnnotatedResult<ProjectRole> listEligibleProjectRoles(
UserId user,
ProjectId projectId,
EnumSet<ProjectRole.Status> statusesToInclude
Expand Down Expand Up @@ -288,7 +285,7 @@ public Result<ProjectRole> listEligibleProjectRoles(
.collect(Collectors.toList());
consolidatedRoles.addAll(activatedRoles);

return new Result<>(
return new AnnotatedResult<>(
consolidatedRoles,
Stream.ofNullable(analysisResult.getNonCriticalErrors())
.flatMap(Collection::stream)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
package com.google.solutions.jitaccess.core.entitlements;

import com.google.solutions.jitaccess.core.AccessDeniedException;
import com.google.solutions.jitaccess.core.AnnotatedResult;
import com.google.solutions.jitaccess.core.clients.ResourceManagerClient;
import com.google.solutions.jitaccess.core.ProjectId;
import com.google.solutions.jitaccess.core.UserId;
Expand Down Expand Up @@ -87,7 +88,7 @@ public void whenCallerLacksRoleBinding_ThenActivateProjectRoleForSelfThrowsExcep
var caller = SAMPLE_USER;

when(discoveryService.listEligibleProjectRoles(eq(caller), eq(SAMPLE_PROJECT_ID), any()))
.thenReturn(new Result<ProjectRole>(
.thenReturn(new AnnotatedResult<ProjectRole>(
List.of(new ProjectRole(
new RoleBinding(
SAMPLE_PROJECT_RESOURCE_1,
Expand Down Expand Up @@ -167,7 +168,7 @@ public void whenCallerIsJitEligible_ThenActivateProjectRoleForSelfAddsBinding()
var caller = SAMPLE_USER;

when(discoveryService.listEligibleProjectRoles(eq(caller), eq(SAMPLE_PROJECT_ID), any()))
.thenReturn(new Result<ProjectRole>(
.thenReturn(new AnnotatedResult<ProjectRole>(
List.of(new ProjectRole(
new RoleBinding(
SAMPLE_PROJECT_RESOURCE_1,
Expand Down Expand Up @@ -280,7 +281,7 @@ public void whenRoleNotMpaEligibleForCaller_ThenActivateProjectRoleForPeerThrows

var discoveryService = Mockito.mock(RoleDiscoveryService.class);
when(discoveryService.listEligibleProjectRoles(eq(caller), eq(SAMPLE_PROJECT_ID), any()))
.thenReturn(new Result<ProjectRole>(
.thenReturn(new AnnotatedResult<ProjectRole>(
List.of(new ProjectRole(
roleBinding,
ProjectRole.Status.ACTIVATED)),
Expand Down Expand Up @@ -321,7 +322,7 @@ public void whenRoleIsJitEligibleForCaller_ThenActivateProjectRoleForPeerThrowsE

var discoveryService = Mockito.mock(RoleDiscoveryService.class);
when(discoveryService.listEligibleProjectRoles(eq(caller), eq(SAMPLE_PROJECT_ID), any()))
.thenReturn(new Result<ProjectRole>(
.thenReturn(new AnnotatedResult<ProjectRole>(
List.of(new ProjectRole(
roleBinding,
ProjectRole.Status.ELIGIBLE_FOR_JIT)),
Expand Down Expand Up @@ -362,13 +363,13 @@ public void whenRoleNotEligibleForPeer_ThenActivateProjectRoleForPeerThrowsExcep

var discoveryService = Mockito.mock(RoleDiscoveryService.class);
when(discoveryService.listEligibleProjectRoles(eq(caller), eq(SAMPLE_PROJECT_ID), any()))
.thenReturn(new Result<ProjectRole>(
.thenReturn(new AnnotatedResult<ProjectRole>(
List.of(new ProjectRole(
roleBinding,
ProjectRole.Status.ELIGIBLE_FOR_MPA)),
Set.of()));
when(discoveryService.listEligibleProjectRoles(eq(peer), eq(SAMPLE_PROJECT_ID), any()))
.thenReturn(new Result<ProjectRole>(
.thenReturn(new AnnotatedResult<ProjectRole>(
List.of(),
Set.of()));

Expand Down Expand Up @@ -413,7 +414,7 @@ public void whenPeerAndCallerEligible_ThenActivateProjectRoleAddsBinding() throw
eq(EnumSet.of(
ProjectRole.Status.ELIGIBLE_FOR_JIT,
ProjectRole.Status.ELIGIBLE_FOR_MPA))))
.thenReturn(new Result<ProjectRole>(
.thenReturn(new AnnotatedResult<ProjectRole>(
List.of(new ProjectRole(
roleBinding,
ProjectRole.Status.ELIGIBLE_FOR_MPA)),
Expand All @@ -424,7 +425,7 @@ public void whenPeerAndCallerEligible_ThenActivateProjectRoleAddsBinding() throw
eq(EnumSet.of(
ProjectRole.Status.ELIGIBLE_FOR_JIT,
ProjectRole.Status.ELIGIBLE_FOR_MPA))))
.thenReturn(new Result<ProjectRole>(
.thenReturn(new AnnotatedResult<ProjectRole>(
List.of(new ProjectRole(
roleBinding,
ProjectRole.Status.ELIGIBLE_FOR_MPA)),
Expand Down Expand Up @@ -484,13 +485,13 @@ public void whenRoleAlreadyActivated_ThenActivateProjectRoleAddsBinding() throws
var resourceAdapter = Mockito.mock(ResourceManagerClient.class);
var discoveryService = Mockito.mock(RoleDiscoveryService.class);
when(discoveryService.listEligibleProjectRoles(eq(caller), eq(SAMPLE_PROJECT_ID), any()))
.thenReturn(new Result<ProjectRole>(
.thenReturn(new AnnotatedResult<ProjectRole>(
List.of(new ProjectRole(
roleBinding,
ProjectRole.Status.ELIGIBLE_FOR_MPA)),
Set.of()));
when(discoveryService.listEligibleProjectRoles(eq(peer), eq(SAMPLE_PROJECT_ID), any()))
.thenReturn(new Result<ProjectRole>(
.thenReturn(new AnnotatedResult<ProjectRole>(
List.of(new ProjectRole(
roleBinding,
ProjectRole.Status.ACTIVATED)), // Pretend someone else approved already
Expand Down Expand Up @@ -658,7 +659,7 @@ public void whenRoleNotMpaEligibleForCaller_ThenCreateActivationRequestForPeerTh

var discoveryService = Mockito.mock(RoleDiscoveryService.class);
when(discoveryService.listEligibleProjectRoles(eq(caller), eq(SAMPLE_PROJECT_ID), any()))
.thenReturn(new Result<ProjectRole>(
.thenReturn(new AnnotatedResult<ProjectRole>(
List.of(new ProjectRole(
roleBinding,
ProjectRole.Status.ACTIVATED)),
Expand Down Expand Up @@ -693,7 +694,7 @@ public void whenRoleIsJitEligibleForCaller_ThenCreateActivationRequestForPeerThr

var discoveryService = Mockito.mock(RoleDiscoveryService.class);
when(discoveryService.listEligibleProjectRoles(eq(caller), eq(SAMPLE_PROJECT_ID), any()))
.thenReturn(new Result<ProjectRole>(
.thenReturn(new AnnotatedResult<ProjectRole>(
List.of(new ProjectRole(
roleBinding,
ProjectRole.Status.ELIGIBLE_FOR_JIT)),
Expand Down Expand Up @@ -728,7 +729,7 @@ public void whenCallerEligible_ThenCreateActivationRequestForPeerSucceeds() thro

var discoveryService = Mockito.mock(RoleDiscoveryService.class);
when(discoveryService.listEligibleProjectRoles(eq(caller), eq(SAMPLE_PROJECT_ID), any()))
.thenReturn(new Result<ProjectRole>(
.thenReturn(new AnnotatedResult<ProjectRole>(
List.of(new ProjectRole(
roleBinding,
ProjectRole.Status.ELIGIBLE_FOR_MPA)),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
import com.google.solutions.jitaccess.core.notifications.NotificationService;
import com.google.solutions.jitaccess.core.entitlements.RoleActivationService;
import com.google.solutions.jitaccess.core.entitlements.RoleDiscoveryService;
import com.google.solutions.jitaccess.core.entitlements.Result;
import com.google.solutions.jitaccess.core.AnnotatedResult;

import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
Expand Down Expand Up @@ -371,7 +371,7 @@ public void whenRoleDiscoveryReturnsNoRoles_ThenListRolesReturnsEmptyList() thro
.listEligibleProjectRoles(
eq(SAMPLE_USER),
eq(new ProjectId("project-1"))))
.thenReturn(new Result<>(
.thenReturn(new AnnotatedResult<>(
List.of(),
Set.of("warning")));

Expand Down Expand Up @@ -401,7 +401,7 @@ public void whenRoleDiscoveryReturnsRoles_ThenListRolesReturnsList() throws Exce
.listEligibleProjectRoles(
eq(SAMPLE_USER),
eq(new ProjectId("project-1"))))
.thenReturn(new Result<>(
.thenReturn(new AnnotatedResult<>(
List.of(role1, role2),
null));

Expand Down

0 comments on commit 02b10b8

Please sign in to comment.