Use PermissionUpdate instead of PermissionServer to remove a permission

6 files changed, 44 insertions, 34 deletions

diff --git a/server/sonar-server/src/main/java/org/sonar/server/permission/PermissionService.java b/server/sonar-server/src/main/java/org/sonar/server/permission/PermissionService.java
index 867a553dc9a..179aea5d758 100644
--- a/server/sonar-server/src/main/java/org/sonar/server/permission/PermissionService.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/permission/PermissionService.java
@@ -118,9 +118,19 @@ public class PermissionService {
    * To be used only by jruby webapp
    */
   public void removePermission(Map<String, Object> params) {
-    removePermission(PermissionChange.buildFromParams(params));
+    PermissionChange change = PermissionChange.buildFromParams(params);
+    DbSession session = dbClient.openSession(false);
+    try {
+      applyChange(Operation.REMOVE, change, session);
+    } finally {
+      session.close();
+    }
   }
 
+  /**
+   * @deprecated since 5.2. Use PermissionUpdater.removePermission
+   */
+  @Deprecated
   public void removePermission(PermissionChange change) {
     DbSession session = dbClient.openSession(false);
     try {
diff --git a/server/sonar-server/src/main/java/org/sonar/server/permission/ws/RemoveGroupAction.java b/server/sonar-server/src/main/java/org/sonar/server/permission/ws/RemoveGroupAction.java
index 737a4104efd..e5f0bf42e6e 100644
--- a/server/sonar-server/src/main/java/org/sonar/server/permission/ws/RemoveGroupAction.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/permission/ws/RemoveGroupAction.java
@@ -26,7 +26,7 @@ import org.sonar.api.server.ws.WebService;
 import org.sonar.core.permission.GlobalPermissions;
 import org.sonar.db.DbClient;
 import org.sonar.server.permission.PermissionChange;
-import org.sonar.server.permission.PermissionService;
+import org.sonar.server.permission.PermissionUpdater;
 
 import static org.sonar.server.permission.ws.PermissionWsCommons.searchName;
 
@@ -37,11 +37,11 @@ public class RemoveGroupAction implements PermissionsWsAction {
   public static final String PARAM_GROUP_NAME = "groupName";
   public static final String PARAM_GROUP_ID = "groupId";
 
-  private final PermissionService permissionService;
+  private final PermissionUpdater permissionUpdater;
   private final DbClient dbClient;
 
-  public RemoveGroupAction(PermissionService permissionService, DbClient dbClient) {
-    this.permissionService = permissionService;
+  public RemoveGroupAction(PermissionUpdater permissionUpdater, DbClient dbClient) {
+    this.permissionUpdater = permissionUpdater;
     this.dbClient = dbClient;
   }
 
@@ -77,11 +77,11 @@ public class RemoveGroupAction implements PermissionsWsAction {
 
     String groupName = searchName(dbClient, groupNameParam, groupId);
 
-    permissionService.removePermission(
+    permissionUpdater.removePermission(
       new PermissionChange()
         .setPermission(permission)
         .setGroup(groupName)
-      );
+    );
 
     response.noContent();
   }
diff --git a/server/sonar-server/src/main/java/org/sonar/server/permission/ws/RemoveUserAction.java b/server/sonar-server/src/main/java/org/sonar/server/permission/ws/RemoveUserAction.java
index 708fecc0c95..519a523ed55 100644
--- a/server/sonar-server/src/main/java/org/sonar/server/permission/ws/RemoveUserAction.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/permission/ws/RemoveUserAction.java
@@ -25,7 +25,7 @@ import org.sonar.api.server.ws.Response;
 import org.sonar.api.server.ws.WebService;
 import org.sonar.core.permission.GlobalPermissions;
 import org.sonar.server.permission.PermissionChange;
-import org.sonar.server.permission.PermissionService;
+import org.sonar.server.permission.PermissionUpdater;
 
 public class RemoveUserAction implements PermissionsWsAction {
 
@@ -33,10 +33,10 @@ public class RemoveUserAction implements PermissionsWsAction {
   public static final String PARAM_PERMISSION = "permission";
   public static final String PARAM_USER_LOGIN = "login";
 
-  private final PermissionService permissionService;
+  private final PermissionUpdater permissionUpdater;
 
-  public RemoveUserAction(PermissionService permissionService) {
-    this.permissionService = permissionService;
+  public RemoveUserAction(PermissionUpdater permissionUpdater) {
+    this.permissionUpdater = permissionUpdater;
   }
 
   @Override
@@ -62,11 +62,11 @@ public class RemoveUserAction implements PermissionsWsAction {
   public void handle(Request request, Response response) throws Exception {
     String permission = request.mandatoryParam(PARAM_PERMISSION);
     String userLogin = request.mandatoryParam(PARAM_USER_LOGIN);
-    permissionService.removePermission(
+    permissionUpdater.removePermission(
       new PermissionChange()
         .setPermission(permission)
         .setUser(userLogin)
-      );
+    );
 
     response.noContent();
   }
diff --git a/server/sonar-server/src/test/java/org/sonar/server/permission/PermissionServiceMediumTest.java b/server/sonar-server/src/test/java/org/sonar/server/permission/PermissionServiceMediumTest.java
index c84d6eb113a..320366fe78d 100644
--- a/server/sonar-server/src/test/java/org/sonar/server/permission/PermissionServiceMediumTest.java
+++ b/server/sonar-server/src/test/java/org/sonar/server/permission/PermissionServiceMediumTest.java
@@ -58,7 +58,7 @@ public class PermissionServiceMediumTest {
 
   DbClient db;
   DbSession session;
-  PermissionService service;
+  PermissionService underTest;
 
   ComponentDto project;
 
@@ -67,7 +67,7 @@ public class PermissionServiceMediumTest {
     tester.clearDbAndIndexes();
     db = tester.get(DbClient.class);
     session = db.openSession(false);
-    service = tester.get(PermissionService.class);
+    underTest = tester.get(PermissionService.class);
 
     project = ComponentTesting.newProjectDto();
     db.componentDao().insert(session, project);
@@ -90,7 +90,7 @@ public class PermissionServiceMediumTest {
     assertThat(countIssueAuthorizationDocs()).isZero();
 
     // add permission
-    service.addPermission(params(user.getLogin(), null, project.key(), UserRole.USER));
+    underTest.addPermission(params(user.getLogin(), null, project.key(), UserRole.USER));
     session.commit();
 
     // Check db
@@ -111,9 +111,9 @@ public class PermissionServiceMediumTest {
     db.userDao().insert(session, user2);
     session.commit();
 
-    service.addPermission(params(user1.getLogin(), null, project.key(), UserRole.USER));
-    service.addPermission(params(user2.getLogin(), null, project.key(), UserRole.USER));
-    service.removePermission(params(user1.getLogin(), null, project.key(), UserRole.USER));
+    underTest.addPermission(params(user1.getLogin(), null, project.key(), UserRole.USER));
+    underTest.addPermission(params(user2.getLogin(), null, project.key(), UserRole.USER));
+    underTest.removePermission(params(user1.getLogin(), null, project.key(), UserRole.USER));
     session.commit();
 
     // Check in db
@@ -132,8 +132,8 @@ public class PermissionServiceMediumTest {
     db.userDao().insert(session, user);
     session.commit();
 
-    service.addPermission(params(user.getLogin(), null, project.key(), UserRole.USER));
-    service.removePermission(params(user.getLogin(), null, project.key(), UserRole.USER));
+    underTest.addPermission(params(user.getLogin(), null, project.key(), UserRole.USER));
+    underTest.removePermission(params(user.getLogin(), null, project.key(), UserRole.USER));
     session.commit();
 
     // Check in db
@@ -162,7 +162,7 @@ public class PermissionServiceMediumTest {
 
     // add permission
     PermissionChange change = new PermissionChange().setPermission(UserRole.USER).setGroup(group.getName()).setComponentKey(project.key());
-    service.addPermission(change);
+    underTest.addPermission(change);
     session.commit();
 
     // Check db
@@ -172,7 +172,7 @@ public class PermissionServiceMediumTest {
     assertThat(countIssueAuthorizationDocs()).isEqualTo(1);
 
     // remove permission
-    service.removePermission(change);
+    underTest.removePermission(change);
     session.commit();
     assertThat(tester.get(RoleDao.class).selectGroupPermissions(session, group.getName(), project.getId())).hasSize(0);
 
diff --git a/server/sonar-server/src/test/java/org/sonar/server/permission/ws/RemoveGroupActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/permission/ws/RemoveGroupActionTest.java
index bac3a2c2a08..52d0b7847ce 100644
--- a/server/sonar-server/src/test/java/org/sonar/server/permission/ws/RemoveGroupActionTest.java
+++ b/server/sonar-server/src/test/java/org/sonar/server/permission/ws/RemoveGroupActionTest.java
@@ -31,8 +31,8 @@ import org.sonar.db.user.GroupDto;
 import org.sonar.server.exceptions.BadRequestException;
 import org.sonar.server.exceptions.NotFoundException;
 import org.sonar.server.exceptions.ServerException;
-import org.sonar.server.permission.PermissionService;
 import org.sonar.server.permission.PermissionChange;
+import org.sonar.server.permission.PermissionUpdater;
 import org.sonar.server.tester.UserSessionRule;
 import org.sonar.server.ws.WsTester;
 
@@ -49,13 +49,13 @@ public class RemoveGroupActionTest {
   public DbTester db = DbTester.create(System2.INSTANCE);
   @Rule
   public ExpectedException expectedException = ExpectedException.none();
-  private PermissionService permissionService;
+  private PermissionUpdater permissionUpdater;
 
   @Before
   public void setUp() {
-    permissionService = mock(PermissionService.class);
+    permissionUpdater = mock(PermissionUpdater.class);
     ws = new WsTester(new PermissionsWs(
-      new RemoveGroupAction(permissionService, db.getDbClient())));
+      new RemoveGroupAction(permissionUpdater, db.getDbClient())));
     userSession.login("admin").setGlobalPermissions(SYSTEM_ADMIN);
   }
 
@@ -67,7 +67,7 @@ public class RemoveGroupActionTest {
       .execute();
 
     ArgumentCaptor<PermissionChange> permissionChangeCaptor = ArgumentCaptor.forClass(PermissionChange.class);
-    verify(permissionService).removePermission(permissionChangeCaptor.capture());
+    verify(permissionUpdater).removePermission(permissionChangeCaptor.capture());
     PermissionChange permissionChange = permissionChangeCaptor.getValue();
     assertThat(permissionChange.group()).isEqualTo("sonar-administrators");
     assertThat(permissionChange.permission()).isEqualTo(SYSTEM_ADMIN);
@@ -85,7 +85,7 @@ public class RemoveGroupActionTest {
       .execute();
 
     ArgumentCaptor<PermissionChange> permissionChangeCaptor = ArgumentCaptor.forClass(PermissionChange.class);
-    verify(permissionService).removePermission(permissionChangeCaptor.capture());
+    verify(permissionUpdater).removePermission(permissionChangeCaptor.capture());
     PermissionChange permissionChange = permissionChangeCaptor.getValue();
     assertThat(permissionChange.group()).isEqualTo("sonar-administrators");
   }
diff --git a/server/sonar-server/src/test/java/org/sonar/server/permission/ws/RemoveUserActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/permission/ws/RemoveUserActionTest.java
index ee7661dbfbd..2081b627f4a 100644
--- a/server/sonar-server/src/test/java/org/sonar/server/permission/ws/RemoveUserActionTest.java
+++ b/server/sonar-server/src/test/java/org/sonar/server/permission/ws/RemoveUserActionTest.java
@@ -28,8 +28,8 @@ import org.mockito.ArgumentCaptor;
 import org.sonar.api.utils.System2;
 import org.sonar.db.DbTester;
 import org.sonar.server.exceptions.ServerException;
-import org.sonar.server.permission.PermissionService;
 import org.sonar.server.permission.PermissionChange;
+import org.sonar.server.permission.PermissionUpdater;
 import org.sonar.server.tester.UserSessionRule;
 import org.sonar.server.ws.WsTester;
 
@@ -46,13 +46,13 @@ public class RemoveUserActionTest {
   public DbTester db = DbTester.create(System2.INSTANCE);
   @Rule
   public ExpectedException expectedException = ExpectedException.none();
-  private PermissionService permissionService;
+  private PermissionUpdater permissionUpdater;
 
   @Before
   public void setUp() {
-    permissionService = mock(PermissionService.class);
+    permissionUpdater = mock(PermissionUpdater.class);
     ws = new WsTester(new PermissionsWs(
-      new RemoveUserAction(permissionService)));
+      new RemoveUserAction(permissionUpdater)));
     userSession.login("admin").setGlobalPermissions(SYSTEM_ADMIN);
   }
 
@@ -64,7 +64,7 @@ public class RemoveUserActionTest {
       .execute();
 
     ArgumentCaptor<PermissionChange> permissionChangeCaptor = ArgumentCaptor.forClass(PermissionChange.class);
-    verify(permissionService).removePermission(permissionChangeCaptor.capture());
+    verify(permissionUpdater).removePermission(permissionChangeCaptor.capture());
     PermissionChange permissionChange = permissionChangeCaptor.getValue();
     assertThat(permissionChange.user()).isEqualTo("ray.bradbury");
     assertThat(permissionChange.permission()).isEqualTo(SYSTEM_ADMIN);