SONAR-8704 add UserSession#getGroups()

Contrary to getUserGroups(), this method returns GroupDto
but not only the group names. It also doesn't return the
virtual group "Anyone"

diff --git a/server/sonar-ce/src/main/java/org/sonar/ce/user/CeUserSession.java b/server/sonar-ce/src/main/java/org/sonar/ce/user/CeUserSession.java
index fa61a25058447c8034a6eb250641b5310b1f1f58..ec52e02dc406c7989a296e984972bb23ff9f379a 100644 (file)
--- a/server/sonar-ce/src/main/java/org/sonar/ce/user/CeUserSession.java
+++ b/server/sonar-ce/src/main/java/org/sonar/ce/user/CeUserSession.java
@@ -22,6 +22,7 @@ package org.sonar.ce.user;
 import java.util.Collection;
 import java.util.List;
 import java.util.Set;
+import org.sonar.db.user.GroupDto;
 import org.sonar.server.user.UserSession;
 
 /**
@@ -49,6 +50,11 @@ public class CeUserSession implements UserSession {
     return notImplemented();
   }
 
+  @Override
+  public Collection<GroupDto> getGroups() {
+    return notImplemented();
+  }
+
   @Override
   public Set<String> getUserGroups() {
     return notImplemented();

diff --git a/server/sonar-server/src/main/java/org/sonar/server/user/DoPrivileged.java b/server/sonar-server/src/main/java/org/sonar/server/user/DoPrivileged.java
index fa1c399e547f5c26f5e3948bcd8b7879f5441b64..3d08fc487e50fc4097fe2fd5674bbe8bfbc1b315 100644 (file)
--- a/server/sonar-server/src/main/java/org/sonar/server/user/DoPrivileged.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/user/DoPrivileged.java
@@ -19,11 +19,13 @@
  */
 package org.sonar.server.user;
 
+import java.util.Collection;
 import java.util.Collections;
 import java.util.List;
 import java.util.Set;
 import org.sonar.api.security.DefaultGroups;
 import org.sonar.core.permission.GlobalPermissions;
+import org.sonar.db.user.GroupDto;
 
 /**
  * Allow code to be executed with the highest privileges possible, as if executed by a {@link GlobalPermissions#SYSTEM_ADMIN} account.
@@ -81,6 +83,11 @@ public final class DoPrivileged {
         return null;
       }
 
+      @Override
+      public Collection<GroupDto> getGroups() {
+        return Collections.emptyList();
+      }
+
       @Override
       public Set<String> getUserGroups() {
         return Collections.singleton(DefaultGroups.ANYONE);

diff --git a/server/sonar-server/src/main/java/org/sonar/server/user/ServerUserSession.java b/server/sonar-server/src/main/java/org/sonar/server/user/ServerUserSession.java
index ce379734db82870f590ede6982fb3cb6a10571f2..d837b8754d1690d85d8a10a66651b28e5bcb1fa0 100644 (file)
--- a/server/sonar-server/src/main/java/org/sonar/server/user/ServerUserSession.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/user/ServerUserSession.java
@@ -19,6 +19,8 @@
  */
 package org.sonar.server.user;
 
+import com.google.common.base.Supplier;
+import com.google.common.base.Suppliers;
 import com.google.common.collect.HashMultimap;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.SetMultimap;
@@ -53,7 +55,7 @@ public class ServerUserSession extends AbstractUserSession {
   private final UserDto userDto;
   private final DbClient dbClient;
   private final ResourceDao resourceDao;
-  private final Set<String> userGroups;
+  private final Supplier<List<GroupDto>> groups;
   private List<String> globalPermissions = null;
   private SetMultimap<String, String> projectKeyByPermission = HashMultimap.create();
   private SetMultimap<String, String> projectUuidByPermission = HashMultimap.create();
@@ -66,7 +68,7 @@ public class ServerUserSession extends AbstractUserSession {
     this.userDto = userDto;
     this.dbClient = dbClient;
     this.resourceDao = dbClient.resourceDao();
-    this.userGroups = loadUserGroups();
+    this.groups = Suppliers.memoize(this::loadGroups);
   }
 
   public static ServerUserSession createForUser(DbClient dbClient, UserDto userDto) {
@@ -78,15 +80,12 @@ public class ServerUserSession extends AbstractUserSession {
     return new ServerUserSession(dbClient, null);
   }
 
-  private Set<String> loadUserGroups() {
+  private List<GroupDto> loadGroups() {
     if (this.userDto == null) {
-      return Collections.singleton(DefaultGroups.ANYONE);
+      return Collections.emptyList();
     }
     try (DbSession dbSession = dbClient.openSession(false)) {
-      return Stream.concat(
-        Stream.of(DefaultGroups.ANYONE),
-        dbClient.groupDao().selectByUserLogin(dbSession, userDto.getLogin()).stream().map(GroupDto::getName))
-        .collect(Collectors.toSet());
+      return dbClient.groupDao().selectByUserLogin(dbSession, userDto.getLogin());
     }
   }
 
@@ -108,9 +107,18 @@ public class ServerUserSession extends AbstractUserSession {
     return userDto == null ? null : userDto.getId().intValue();
   }
 
+  @Override
+  public Collection<GroupDto> getGroups() {
+    return groups.get();
+  }
+
   @Override
   public Set<String> getUserGroups() {
-    return userGroups;
+    Collection<GroupDto> groups = getGroups();
+    return Stream.concat(
+      Stream.of(DefaultGroups.ANYONE),
+      groups.stream().map(GroupDto::getName))
+      .collect(Collectors.toSet(groups.size() + 1));
   }
 
   @Override

diff --git a/server/sonar-server/src/main/java/org/sonar/server/user/ThreadLocalUserSession.java b/server/sonar-server/src/main/java/org/sonar/server/user/ThreadLocalUserSession.java
index 5c507d3ff1990a8ad73150d7749ed1b00dad7e57..c94ea11d004308813c33441200a7a5d274ee13c3 100644 (file)
--- a/server/sonar-server/src/main/java/org/sonar/server/user/ThreadLocalUserSession.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/user/ThreadLocalUserSession.java
@@ -23,6 +23,7 @@ import java.util.Collection;
 import java.util.List;
 import java.util.Set;
 import javax.annotation.CheckForNull;
+import org.sonar.db.user.GroupDto;
 import org.sonar.server.exceptions.UnauthorizedException;
 
 /**
@@ -70,6 +71,11 @@ public class ThreadLocalUserSession implements UserSession {
     return get().getUserId();
   }
 
+  @Override
+  public Collection<GroupDto> getGroups() {
+    return get().getGroups();
+  }
+
   @Override
   public Set<String> getUserGroups() {
     return get().getUserGroups();

diff --git a/server/sonar-server/src/main/java/org/sonar/server/user/UserSession.java b/server/sonar-server/src/main/java/org/sonar/server/user/UserSession.java
index a108fd10e0ad418f2190526c8422f71648e41d86..c3c6496145c639a37d17e8e1d3f486841d1a59c3 100644 (file)
--- a/server/sonar-server/src/main/java/org/sonar/server/user/UserSession.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/user/UserSession.java
@@ -23,6 +23,8 @@ import java.util.Collection;
 import java.util.List;
 import java.util.Set;
 import javax.annotation.CheckForNull;
+import org.sonar.api.security.DefaultGroups;
+import org.sonar.db.user.GroupDto;
 
 public interface UserSession {
   @CheckForNull
@@ -35,11 +37,25 @@ public interface UserSession {
   Integer getUserId();
 
   /**
-   * @deprecated does not support organizations as group names are not unique
+   * The groups that the logged-in user is member of. An empty
+   * collection is returned if user is anonymous.
+   */
+  Collection<GroupDto> getGroups();
+
+  /**
+   * The groups that the user is member of, always including
+   * the virtual group named {@link DefaultGroups#ANYONE}.
+   *
+   * @deprecated does not support organizations because group names 
+   * are not unique
+   * @see #getGroups()
    */
   @Deprecated
   Set<String> getUserGroups();
 
+  /**
+   * Whether the user is logged-in or anonymous.
+   */
   boolean isLoggedIn();
 
   boolean isRoot();

diff --git a/server/sonar-server/src/test/java/org/sonar/server/test/ws/ListActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/test/ws/ListActionTest.java
index ed629fdc5c0b1acd0182e33ce83dafd8d5c49818..1fa8636ec849a8d021beb97a836ce23650cb2d76 100644 (file)
--- a/server/sonar-server/src/test/java/org/sonar/server/test/ws/ListActionTest.java
+++ b/server/sonar-server/src/test/java/org/sonar/server/test/ws/ListActionTest.java
@@ -286,7 +286,7 @@ public class ListActionTest {
       .setFileUuid(testFile.uuid())
       .setTestData(asList(tests)));
     db.commit();
-    testIndexer.index(project.uuid());
+    testIndexer.index();
   }
 
   private static ListResponse call(TestRequest request) {

diff --git a/server/sonar-server/src/test/java/org/sonar/server/tester/AnonymousMockUserSession.java b/server/sonar-server/src/test/java/org/sonar/server/tester/AnonymousMockUserSession.java
index 6da49c33c0e50fc4e9aedd4deb5451f587f09e13..418f7877b92b247cbc9ec63aceffd076c5572915 100644 (file)
--- a/server/sonar-server/src/test/java/org/sonar/server/tester/AnonymousMockUserSession.java
+++ b/server/sonar-server/src/test/java/org/sonar/server/tester/AnonymousMockUserSession.java
@@ -19,6 +19,10 @@
  */
 package org.sonar.server.tester;
 
+import java.util.Collection;
+import java.util.Collections;
+import org.sonar.db.user.GroupDto;
+
 public class AnonymousMockUserSession extends AbstractMockUserSession<AnonymousMockUserSession> {
 
   public AnonymousMockUserSession() {
@@ -50,4 +54,8 @@ public class AnonymousMockUserSession extends AbstractMockUserSession<AnonymousM
     return false;
   }
 
+  @Override
+  public Collection<GroupDto> getGroups() {
+    return Collections.emptyList();
+  }
 }

diff --git a/server/sonar-server/src/test/java/org/sonar/server/tester/MockUserSession.java b/server/sonar-server/src/test/java/org/sonar/server/tester/MockUserSession.java
index 5439d9410652058f5824640d67fa993cd81d8cf6..ebf2acd78335cccd4535c7e1d89ea9a0815da25e 100644 (file)
--- a/server/sonar-server/src/test/java/org/sonar/server/tester/MockUserSession.java
+++ b/server/sonar-server/src/test/java/org/sonar/server/tester/MockUserSession.java
@@ -19,16 +19,22 @@
  */
 package org.sonar.server.tester;
 
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.List;
 import java.util.Objects;
+import org.sonar.db.user.GroupDto;
 import org.sonar.db.user.UserDto;
 
 import static com.google.common.base.Preconditions.checkArgument;
+import static java.util.Arrays.asList;
 
 public class MockUserSession extends AbstractMockUserSession<MockUserSession> {
   private final String login;
   private boolean root = false;
   private Integer userId;
   private String name;
+  private List<GroupDto> groups = new ArrayList<>();
 
   public MockUserSession(String login) {
     super(MockUserSession.class);
@@ -85,4 +91,13 @@ public class MockUserSession extends AbstractMockUserSession<MockUserSession> {
     return this;
   }
 
+  @Override
+  public Collection<GroupDto> getGroups() {
+    return groups;
+  }
+
+  public MockUserSession setGroups(GroupDto... groups) {
+    this.groups = asList(groups);
+    return this;
+  }
 }

diff --git a/server/sonar-server/src/test/java/org/sonar/server/tester/UserSessionRule.java b/server/sonar-server/src/test/java/org/sonar/server/tester/UserSessionRule.java
index 91192c42ffa25ee83e6fe803c768a9bf44fd69bf..b86e7ab2e21b901a8c531d4b6db4ac395eb7a271 100644 (file)
--- a/server/sonar-server/src/test/java/org/sonar/server/tester/UserSessionRule.java
+++ b/server/sonar-server/src/test/java/org/sonar/server/tester/UserSessionRule.java
@@ -29,6 +29,7 @@ import org.junit.rules.TestRule;
 import org.junit.runner.Description;
 import org.junit.runners.model.Statement;
 import org.sonar.db.organization.OrganizationDto;
+import org.sonar.db.user.GroupDto;
 import org.sonar.db.user.UserDto;
 import org.sonar.server.user.ThreadLocalUserSession;
 import org.sonar.server.user.UserSession;
@@ -278,6 +279,11 @@ public class UserSessionRule implements TestRule, UserSession {
     return currentUserSession.getUserId();
   }
 
+  @Override
+  public Collection<GroupDto> getGroups() {
+    return currentUserSession.getGroups();
+  }
+
   @Override
   public Set<String> getUserGroups() {
     return currentUserSession.getUserGroups();

diff --git a/server/sonar-server/src/test/java/org/sonar/server/user/ServerUserSessionTest.java b/server/sonar-server/src/test/java/org/sonar/server/user/ServerUserSessionTest.java
index df4243e055d0bf6f156e4c638b2a78fe7fa856b5..b2a0109ae7081a1f0e5875a9a48362210d05781d 100644 (file)
--- a/server/sonar-server/src/test/java/org/sonar/server/user/ServerUserSessionTest.java
+++ b/server/sonar-server/src/test/java/org/sonar/server/user/ServerUserSessionTest.java
@@ -32,6 +32,7 @@ import org.sonar.db.DbTester;
 import org.sonar.db.component.ComponentDto;
 import org.sonar.db.component.ComponentTesting;
 import org.sonar.db.organization.OrganizationDto;
+import org.sonar.db.user.GroupDto;
 import org.sonar.db.user.UserDto;
 import org.sonar.server.exceptions.ForbiddenException;
 
@@ -78,6 +79,40 @@ public class ServerUserSessionTest {
     db.users().insertUser(userDto);
   }
 
+  @Test
+  public void getGroups_is_empty_on_anonymous() {
+    assertThat(newAnonymousSession().getGroups()).isEmpty();
+  }
+
+  @Test
+  public void getGroups_is_empty_if_user_is_not_member_of_any_group() {
+    assertThat(newUserSession(userDto).getGroups()).isEmpty();
+  }
+
+  @Test
+  public void getGroups_returns_the_groups_of_logged_in_user() {
+    GroupDto group1 = db.users().insertGroup();
+    GroupDto group2 = db.users().insertGroup();
+    db.users().insertMember(group1, userDto);
+    db.users().insertMember(group2, userDto);
+
+    assertThat(newUserSession(userDto).getGroups()).extracting(GroupDto::getId).containsOnly(group1.getId(), group2.getId());
+  }
+
+  @Test
+  public void getGroups_keeps_groups_in_cache() {
+    GroupDto group1 = db.users().insertGroup();
+    GroupDto group2 = db.users().insertGroup();
+    db.users().insertMember(group1, userDto);
+
+    ServerUserSession session = newUserSession(userDto);
+    assertThat(session.getGroups()).extracting(GroupDto::getId).containsOnly(group1.getId());
+
+    // membership updated but not cache
+    db.users().insertMember(group2, userDto);
+    assertThat(session.getGroups()).extracting(GroupDto::getId).containsOnly(group1.getId());
+  }
+
   @Test
   public void isRoot_is_false_is_flag_root_is_false_on_UserDto() {
     assertThat(newUserSession(ROOT_USER_DTO).isRoot()).isTrue();

diff --git a/server/sonar-server/src/test/java/org/sonar/server/user/ThreadLocalUserSessionTest.java b/server/sonar-server/src/test/java/org/sonar/server/user/ThreadLocalUserSessionTest.java
index 38058382c472538649c5ff5e8c964c352c22f43c..2c8470aea3e002805715d7a5203bcc9aae6fbf33 100644 (file)
--- a/server/sonar-server/src/test/java/org/sonar/server/user/ThreadLocalUserSessionTest.java
+++ b/server/sonar-server/src/test/java/org/sonar/server/user/ThreadLocalUserSessionTest.java
@@ -24,8 +24,9 @@ import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.ExpectedException;
+import org.sonar.db.user.GroupDto;
+import org.sonar.db.user.GroupTesting;
 import org.sonar.server.exceptions.UnauthorizedException;
-import org.sonar.server.tester.AbstractMockUserSession;
 import org.sonar.server.tester.AnonymousMockUserSession;
 import org.sonar.server.tester.MockUserSession;
 
@@ -33,7 +34,7 @@ import static org.assertj.core.api.Assertions.assertThat;
 
 public class ThreadLocalUserSessionTest {
 
-  ThreadLocalUserSession threadLocalUserSession = new ThreadLocalUserSession();
+  private ThreadLocalUserSession threadLocalUserSession = new ThreadLocalUserSession();
 
   @Rule
   public ExpectedException thrown = ExpectedException.none();
@@ -52,7 +53,10 @@ public class ThreadLocalUserSessionTest {
 
   @Test
   public void get_session_for_user() {
-    AbstractMockUserSession expected = new MockUserSession("karadoc").setUserId(123);
+    GroupDto group = GroupTesting.newGroupDto();
+    MockUserSession expected = new MockUserSession("karadoc")
+      .setUserId(123)
+      .setGroups(group);
     threadLocalUserSession.set(expected);
 
     UserSession session = threadLocalUserSession.get();
@@ -60,6 +64,7 @@ public class ThreadLocalUserSessionTest {
     assertThat(threadLocalUserSession.getUserId()).isEqualTo(123);
     assertThat(threadLocalUserSession.getLogin()).isEqualTo("karadoc");
     assertThat(threadLocalUserSession.isLoggedIn()).isTrue();
+    assertThat(threadLocalUserSession.getGroups()).extracting(GroupDto::getId).containsOnly(group.getId());
   }
 
   @Test
@@ -72,6 +77,7 @@ public class ThreadLocalUserSessionTest {
     assertThat(threadLocalUserSession.getLogin()).isNull();
     assertThat(threadLocalUserSession.getUserId()).isNull();
     assertThat(threadLocalUserSession.isLoggedIn()).isFalse();
+    assertThat(threadLocalUserSession.getGroups()).isEmpty();
   }
 
   @Test