Indexation of project permission should only be done on the first project analysis

6 files changed, 70 insertions, 43 deletions

diff --git a/server/sonar-server/src/main/java/org/sonar/server/computation/SynchronizeProjectPermissionsStep.java b/server/sonar-server/src/main/java/org/sonar/server/computation/SynchronizeProjectPermissionsStep.java
index 2a50b59fcb0..e7f1cd91522 100644
--- a/server/sonar-server/src/main/java/org/sonar/server/computation/SynchronizeProjectPermissionsStep.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/computation/SynchronizeProjectPermissionsStep.java
@@ -20,20 +20,22 @@
 
 package org.sonar.server.computation;
 
+import com.google.common.collect.ImmutableMap;
 import org.sonar.core.computation.db.AnalysisReportDto;
 import org.sonar.core.persistence.DbSession;
+import org.sonar.server.db.DbClient;
+import org.sonar.server.issue.db.IssueAuthorizationDao;
 import org.sonar.server.issue.index.IssueAuthorizationIndex;
-import org.sonar.server.permission.InternalPermissionService;
 import org.sonar.server.search.IndexClient;
 
 public class SynchronizeProjectPermissionsStep implements ComputationStep {
 
+  private final DbClient dbClient;
   private final IndexClient index;
-  private final InternalPermissionService permissionService;
 
-  public SynchronizeProjectPermissionsStep(IndexClient index, InternalPermissionService permissionService) {
+  public SynchronizeProjectPermissionsStep(DbClient dbClient, IndexClient index) {
+    this.dbClient = dbClient;
     this.index = index;
-    this.permissionService = permissionService;
   }
 
   @Override
@@ -47,8 +49,10 @@ public class SynchronizeProjectPermissionsStep implements ComputationStep {
   }
 
   private void synchronizeProjectPermissionsIfNotFound(DbSession session, AnalysisReportDto report) {
-    if (index.get(IssueAuthorizationIndex.class).getNullableByKey(report.getProjectKey()) == null) {
-      permissionService.synchronizePermissions(session, report.getProject().uuid());
+    String projectUuid = report.getProject().uuid();
+    if (index.get(IssueAuthorizationIndex.class).getNullableByKey(projectUuid) == null) {
+      dbClient.issueAuthorizationDao().synchronizeAfter(session, null,
+        ImmutableMap.of(IssueAuthorizationDao.PROJECT_UUID, projectUuid));
       session.commit();
     }
   }
diff --git a/server/sonar-server/src/main/java/org/sonar/server/permission/InternalPermissionService.java b/server/sonar-server/src/main/java/org/sonar/server/permission/InternalPermissionService.java
index 378cc130935..7cbddf2ae3e 100644
--- a/server/sonar-server/src/main/java/org/sonar/server/permission/InternalPermissionService.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/permission/InternalPermissionService.java
@@ -96,11 +96,21 @@ public class InternalPermissionService implements ServerComponent {
   }
 
   public void addPermission(final Map<String, Object> params) {
-    changePermission(ADD, params);
+    DbSession session = dbClient.openSession(false);
+    try {
+      changePermission(ADD, params, session);
+    } finally {
+      session.close();
+    }
   }
 
   public void removePermission(Map<String, Object> params) {
-    changePermission(REMOVE, params);
+    DbSession session = dbClient.openSession(false);
+    try {
+      changePermission(REMOVE, params, session);
+    } finally {
+      session.close();
+    }
   }
 
   public void applyDefaultPermissionTemplate(final String componentKey) {
@@ -125,7 +135,7 @@ public class InternalPermissionService implements ServerComponent {
 
   public void applyDefaultPermissionTemplate(DbSession session, ComponentDto component) {
     permissionFacade.grantDefaultRoles(session, component.getId(), component.qualifier());
-    synchronizePermissions(session, component.key());
+    synchronizeProjectPermissions(session, component.key());
   }
 
   public void applyPermissionTemplate(Map<String, Object> params) {
@@ -149,7 +159,7 @@ public class InternalPermissionService implements ServerComponent {
       for (String componentKey : query.getSelectedComponents()) {
         ComponentDto component = dbClient.componentDao().getByKey(session, componentKey);
         permissionFacade.applyPermissionTemplate(session, query.getTemplateKey(), component.getId());
-        synchronizePermissions(session, component.uuid());
+        synchronizeProjectPermissions(session, component.uuid());
       }
       session.commit();
     } finally {
@@ -157,33 +167,27 @@ public class InternalPermissionService implements ServerComponent {
     }
   }
 
-  private void changePermission(String permissionChange, Map<String, Object> params) {
+  private void changePermission(String permissionChange, Map<String, Object> params, DbSession session) {
     UserSession.get().checkLoggedIn();
     PermissionChangeQuery permissionChangeQuery = PermissionChangeQuery.buildFromParams(params);
     permissionChangeQuery.validate();
-    applyPermissionChange(permissionChange, permissionChangeQuery);
+    applyPermissionChange(permissionChange, permissionChangeQuery, session);
   }
 
-  private void applyPermissionChange(String operation, PermissionChangeQuery permissionChangeQuery) {
-    DbSession session = dbClient.openSession(false);
+  private void applyPermissionChange(String operation, PermissionChangeQuery permissionChangeQuery, DbSession session) {
     boolean changed;
-    try {
-      if (permissionChangeQuery.targetsUser()) {
-        changed = applyUserPermissionChange(session, operation, permissionChangeQuery);
-      } else {
-        changed = applyGroupPermissionChange(session, operation, permissionChangeQuery);
-      }
-      if (changed) {
-        String project = permissionChangeQuery.component();
-        if (project != null) {
-          synchronizePermissions(session, dbClient.componentDao().getByKey(session, project).uuid());
-        }
-        session.commit();
+    if (permissionChangeQuery.targetsUser()) {
+      changed = applyUserPermissionChange(session, operation, permissionChangeQuery);
+    } else {
+      changed = applyGroupPermissionChange(session, operation, permissionChangeQuery);
+    }
+    if (changed) {
+      String project = permissionChangeQuery.component();
+      if (project != null) {
+        synchronizeProjectPermissions(session, dbClient.componentDao().getByKey(session, project).uuid());
       }
-    } finally {
-      session.close();
+      session.commit();
     }
-
   }
 
   private boolean applyGroupPermissionChange(DbSession session, String operation, PermissionChangeQuery permissionChangeQuery) {
@@ -275,7 +279,7 @@ public class InternalPermissionService implements ServerComponent {
     }
   }
 
-  public void synchronizePermissions(DbSession session, String projectUuid) {
+  public void synchronizeProjectPermissions(DbSession session, String projectUuid) {
     dbClient.issueAuthorizationDao().synchronizeAfter(session,
       index.get(IssueAuthorizationIndex.class).getLastSynchronization(),
       ImmutableMap.of(IssueAuthorizationDao.PROJECT_UUID, projectUuid));
diff --git a/server/sonar-server/src/main/java/org/sonar/server/permission/PermissionChangeQuery.java b/server/sonar-server/src/main/java/org/sonar/server/permission/PermissionChangeQuery.java
index aebb0418c85..db42661f0ad 100644
--- a/server/sonar-server/src/main/java/org/sonar/server/permission/PermissionChangeQuery.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/permission/PermissionChangeQuery.java
@@ -33,10 +33,10 @@ import java.util.Map;
 
 public class PermissionChangeQuery {
 
-  private static final String USER_KEY = "user";
-  private static final String GROUP_KEY = "group";
-  private static final String PERMISSION_KEY = "permission";
-  private static final String COMPONENT_KEY = "component";
+  static final String USER_KEY = "user";
+  static final String GROUP_KEY = "group";
+  static final String PERMISSION_KEY = "permission";
+  static final String COMPONENT_KEY = "component";
 
   private final String user;
   private final String group;
diff --git a/server/sonar-server/src/test/java/org/sonar/server/computation/SynchronizeProjectPermissionsStepMediumTest.java b/server/sonar-server/src/test/java/org/sonar/server/computation/SynchronizeProjectPermissionsStepMediumTest.java
index c248b93e786..4a669ef5c3a 100644
--- a/server/sonar-server/src/test/java/org/sonar/server/computation/SynchronizeProjectPermissionsStepMediumTest.java
+++ b/server/sonar-server/src/test/java/org/sonar/server/computation/SynchronizeProjectPermissionsStepMediumTest.java
@@ -20,6 +20,7 @@
 
 package org.sonar.server.computation;
 
+import com.google.common.collect.ImmutableMap;
 import org.junit.After;
 import org.junit.Before;
 import org.junit.ClassRule;
@@ -35,6 +36,7 @@ import org.sonar.core.persistence.MyBatis;
 import org.sonar.core.user.UserDto;
 import org.sonar.server.component.ComponentTesting;
 import org.sonar.server.db.DbClient;
+import org.sonar.server.issue.db.IssueAuthorizationDao;
 import org.sonar.server.issue.index.IssueAuthorizationDoc;
 import org.sonar.server.issue.index.IssueAuthorizationIndex;
 import org.sonar.server.tester.ServerTester;
@@ -96,6 +98,27 @@ public class SynchronizeProjectPermissionsStepMediumTest {
 
     IssueAuthorizationDoc issueAuthorizationIndex = tester.get(IssueAuthorizationIndex.class).getNullableByKey(project.uuid());
     assertThat(issueAuthorizationIndex).isNotNull();
+    assertThat(issueAuthorizationIndex.groups()).containsExactly(DefaultGroups.ANYONE);
+  }
+
+  @Test
+  public void not_add_project_issue_permission_if_already_existing() throws Exception {
+    ComponentDto project = insertPermissionsForProject(DEFAULT_PROJECT_KEY);
+    // Synchronisation on project is already done
+    db.issueAuthorizationDao().synchronizeAfter(session, null, ImmutableMap.of(IssueAuthorizationDao.PROJECT_UUID, project.uuid()));
+
+    // To check that permission will not be synchronized again, add a new permission on the project in db, this permission should not be in the index
+    tester.get(PermissionFacade.class).insertGroupPermission(project.getId(), DefaultGroups.USERS, UserRole.USER, session);
+
+    queue.add(DEFAULT_PROJECT_KEY, 123L);
+    List<AnalysisReportDto> reports = queue.findByProjectKey(DEFAULT_PROJECT_KEY);
+    getAndSetProjectStep.execute(session, reports.get(0));
+
+    sut.execute(session, reports.get(0));
+
+    IssueAuthorizationDoc issueAuthorizationIndex = tester.get(IssueAuthorizationIndex.class).getNullableByKey(project.uuid());
+    assertThat(issueAuthorizationIndex).isNotNull();
+    assertThat(issueAuthorizationIndex.groups()).containsExactly(DefaultGroups.ANYONE);
   }
 
   private ComponentDto insertPermissionsForProject(String projectKey) {
@@ -103,10 +126,7 @@ public class SynchronizeProjectPermissionsStepMediumTest {
     db.componentDao().insert(session, project);
 
     tester.get(PermissionFacade.class).insertGroupPermission(project.getId(), DefaultGroups.ANYONE, UserRole.USER, session);
-    userSession.addProjectPermissions(UserRole.USER, project.key());
-
     session.commit();
-
     return project;
   }
 }
diff --git a/server/sonar-server/src/test/java/org/sonar/server/issue/db/IssueAuthorizationDaoTest.java b/server/sonar-server/src/test/java/org/sonar/server/issue/db/IssueAuthorizationDaoTest.java
index 1779efa134c..0d96d7d4ee8 100644
--- a/server/sonar-server/src/test/java/org/sonar/server/issue/db/IssueAuthorizationDaoTest.java
+++ b/server/sonar-server/src/test/java/org/sonar/server/issue/db/IssueAuthorizationDaoTest.java
@@ -29,8 +29,6 @@ import org.sonar.api.utils.System2;
 import org.sonar.core.persistence.AbstractDaoTestCase;
 import org.sonar.core.persistence.DbSession;
 
-import java.util.Date;
-
 import static org.fest.assertions.Assertions.assertThat;
 
 public class IssueAuthorizationDaoTest extends AbstractDaoTestCase {
@@ -60,7 +58,7 @@ public class IssueAuthorizationDaoTest extends AbstractDaoTestCase {
 
     assertThat(session.getActionCount()).isEqualTo(0);
 
-    dao.synchronizeAfter(session, new Date(0));
+    dao.synchronizeAfter(session);
     // SynchronizeAfter adds an implicit action (refresh) after execution of synchronization
     assertThat(session.getActionCount()).isEqualTo(2);
   }
@@ -93,7 +91,7 @@ public class IssueAuthorizationDaoTest extends AbstractDaoTestCase {
 
     assertThat(session.getActionCount()).isEqualTo(0);
 
-    dao.synchronizeAfter(session, new Date(0));
+    dao.synchronizeAfter(session);
     // SynchronizeAfter adds an implicit action (refresh) after execution of synchronization
     assertThat(session.getActionCount()).isEqualTo(1);
   }
diff --git a/server/sonar-server/src/test/java/org/sonar/server/issue/index/IssueAuthorizationIndexMediumTest.java b/server/sonar-server/src/test/java/org/sonar/server/issue/index/IssueAuthorizationIndexMediumTest.java
index 7a1740da1f9..10b3167fdf3 100644
--- a/server/sonar-server/src/test/java/org/sonar/server/issue/index/IssueAuthorizationIndexMediumTest.java
+++ b/server/sonar-server/src/test/java/org/sonar/server/issue/index/IssueAuthorizationIndexMediumTest.java
@@ -168,14 +168,15 @@ public class IssueAuthorizationIndexMediumTest {
     db.userDao().insert(session, john);
 
     // Insert one permission
+
     tester.get(PermissionFacade.class).insertGroupPermission(project.getId(), "devs", UserRole.USER, session);
-    db.issueAuthorizationDao().synchronizeAfter(session, new Date(0), ImmutableMap.of(IssueAuthorizationDao.PROJECT_UUID, project.uuid()));
+    db.issueAuthorizationDao().synchronizeAfter(session, null, ImmutableMap.of(IssueAuthorizationDao.PROJECT_UUID, project.uuid()));
     session.commit();
     assertThat(index.getByKey(project.uuid())).isNotNull();
 
     // Delete the permission
     tester.get(PermissionFacade.class).deleteGroupPermission(project.getId(), "devs", UserRole.USER, session);
-    db.issueAuthorizationDao().synchronizeAfter(session, new Date(0), ImmutableMap.of(IssueAuthorizationDao.PROJECT_UUID, project.uuid()));
+    db.issueAuthorizationDao().synchronizeAfter(session, null, ImmutableMap.of(IssueAuthorizationDao.PROJECT_UUID, project.uuid()));
     session.commit();
     assertThat(index.getNullableByKey(project.uuid())).isNull();
   }