SONAR-13001 apply review feedback

diff --git a/server/sonar-db-dao/src/main/java/org/sonar/db/alm/pat/AlmPatDao.java b/server/sonar-db-dao/src/main/java/org/sonar/db/alm/pat/AlmPatDao.java
index 44447b942a01fff910b1841aba973f0c49181db1..7f0164253ae8970efad9ce9bb90d56b7e5bff155 100644 (file)
--- a/server/sonar-db-dao/src/main/java/org/sonar/db/alm/pat/AlmPatDao.java
+++ b/server/sonar-db-dao/src/main/java/org/sonar/db/alm/pat/AlmPatDao.java
@@ -19,13 +19,13 @@
  */
 package org.sonar.db.alm.pat;
 
-import java.util.List;
 import java.util.Optional;
 import org.sonar.api.utils.System2;
 import org.sonar.core.util.UuidFactory;
 import org.sonar.db.Dao;
 import org.sonar.db.DbSession;
 import org.sonar.db.alm.setting.AlmSettingDto;
+import org.sonar.db.user.UserDto;
 
 public class AlmPatDao implements Dao {
 
@@ -45,12 +45,8 @@ public class AlmPatDao implements Dao {
     return Optional.ofNullable(getMapper(dbSession).selectByUuid(uuid));
   }
 
-  public Optional<AlmPatDto> selectByAlmSetting(DbSession dbSession, String userUuid, AlmSettingDto almSettingDto) {
-    return Optional.ofNullable(getMapper(dbSession).selectByAlmSetting(userUuid, almSettingDto.getUuid()));
-  }
-
-  public List<AlmPatDto> selectAll(DbSession dbSession) {
-    return getMapper(dbSession).selectAll();
+  public Optional<AlmPatDto> selectByUserAndAlmSetting(DbSession dbSession, String userUuid, AlmSettingDto almSettingDto) {
+    return Optional.ofNullable(getMapper(dbSession).selectByUserAndAlmSetting(userUuid, almSettingDto.getUuid()));
   }
 
   public void insert(DbSession dbSession, AlmPatDto almPatDto) {
@@ -72,4 +68,14 @@ public class AlmPatDao implements Dao {
     getMapper(dbSession).deleteByUuid(almPatDto.getUuid());
   }
 
+  public void deleteByUser(DbSession dbSession, UserDto user) {
+    getMapper(dbSession).deleteByUser(user.getUuid());
+  }
+
+  public void deleteByAlmSetting(DbSession dbSession, AlmSettingDto almSetting) {
+    getMapper(dbSession).deleteByAlmSetting(almSetting.getUuid());
+  }
+
+
+
 }

diff --git a/server/sonar-db-dao/src/main/java/org/sonar/db/alm/pat/AlmPatMapper.java b/server/sonar-db-dao/src/main/java/org/sonar/db/alm/pat/AlmPatMapper.java
index eaf84b038e160ac96c1c2a7f684cface29f27dc0..e4e5b6318b105fa423956e4769222dea36122b9b 100644 (file)
--- a/server/sonar-db-dao/src/main/java/org/sonar/db/alm/pat/AlmPatMapper.java
+++ b/server/sonar-db-dao/src/main/java/org/sonar/db/alm/pat/AlmPatMapper.java
@@ -19,7 +19,6 @@
  */
 package org.sonar.db.alm.pat;
 
-import java.util.List;
 import javax.annotation.CheckForNull;
 import org.apache.ibatis.annotations.Param;
 
@@ -29,9 +28,7 @@ public interface AlmPatMapper {
   AlmPatDto selectByUuid(@Param("uuid") String uuid);
 
   @CheckForNull
-  AlmPatDto selectByAlmSetting(@Param("userUuid") String userUuid, @Param("almSettingUuid") String almSettingUuid);
-
-  List<AlmPatDto> selectAll();
+  AlmPatDto selectByUserAndAlmSetting(@Param("userUuid") String userUuid, @Param("almSettingUuid") String almSettingUuid);
 
   void insert(@Param("dto") AlmPatDto almPatDto, @Param("uuid") String uuid, @Param("now") long now);
 
@@ -39,4 +36,7 @@ public interface AlmPatMapper {
 
   void deleteByUuid(@Param("uuid") String uuid);
 
+  void deleteByUser(@Param("userUuid") String userUuid);
+
+  void deleteByAlmSetting(@Param("almSettingUuid") String almSettingUuid);
 }

diff --git a/server/sonar-db-dao/src/main/resources/org/sonar/db/alm/pat/AlmPatMapper.xml b/server/sonar-db-dao/src/main/resources/org/sonar/db/alm/pat/AlmPatMapper.xml
index 248c2c03ef0f75d6cca8f46a179de934c32329ec..68f1fb44b893911bd4479a2c5a2f08aba98114a0 100644 (file)
--- a/server/sonar-db-dao/src/main/resources/org/sonar/db/alm/pat/AlmPatMapper.xml
+++ b/server/sonar-db-dao/src/main/resources/org/sonar/db/alm/pat/AlmPatMapper.xml
@@ -20,7 +20,7 @@
       a.uuid = #{uuid, jdbcType=VARCHAR}
   </select>
 
-  <select id="selectByAlmSetting" parameterType="string" resultType="org.sonar.db.alm.pat.AlmPatDto">
+  <select id="selectByUserAndAlmSetting" parameterType="string" resultType="org.sonar.db.alm.pat.AlmPatDto">
     select <include refid="sqlColumns"/>
     from
     alm_pats a
@@ -30,12 +30,6 @@
       a.user_uuid = #{userUuid, jdbcType=VARCHAR}
   </select>
 
-  <select id="selectAll" resultType="org.sonar.db.alm.pat.AlmPatDto">
-    select <include refid="sqlColumns"/>
-    from alm_pats a
-  </select>
-
-
   <insert id="insert" parameterType="Map" useGeneratedKeys="false">
     INSERT INTO alm_pats
     (
@@ -71,5 +65,13 @@
     DELETE FROM alm_pats WHERE uuid = #{uuid, jdbcType=VARCHAR}
   </delete>
 
+  <delete id="deleteByUser" parameterType="String">
+    DELETE FROM alm_pats WHERE user_uuid = #{userUuid, jdbcType=VARCHAR}
+  </delete>
+
+  <delete id="deleteByAlmSetting" parameterType="String">
+    DELETE FROM alm_pats WHERE alm_setting_uuid = #{almSettingUuid, jdbcType=VARCHAR}
+  </delete>
+
 
 </mapper>

diff --git a/server/sonar-db-dao/src/schema/schema-sq.ddl b/server/sonar-db-dao/src/schema/schema-sq.ddl
index 15d5484b25e64bea3e6c408686083abfc4cd22b7..eb922de20313341612f1ab6aff3806d61080d7e7 100644 (file)
--- a/server/sonar-db-dao/src/schema/schema-sq.ddl
+++ b/server/sonar-db-dao/src/schema/schema-sq.ddl
@@ -53,7 +53,7 @@ CREATE INDEX "ALM_APP_INSTALLS_EXTERNAL_ID" ON "ALM_APP_INSTALLS"("USER_EXTERNAL
 CREATE TABLE "ALM_PATS"(
     "UUID" VARCHAR(40) NOT NULL,
     "PAT" VARCHAR(2000) NOT NULL,
-    "USER_UUID" VARCHAR(40) NOT NULL,
+    "USER_UUID" VARCHAR(256) NOT NULL,
     "ALM_SETTING_UUID" VARCHAR(40) NOT NULL,
     "UPDATED_AT" BIGINT NOT NULL,
     "CREATED_AT" BIGINT NOT NULL

diff --git a/server/sonar-db-dao/src/test/java/org/sonar/db/alm/pat/ALMPatDaoTest.java b/server/sonar-db-dao/src/test/java/org/sonar/db/alm/pat/ALMPatDaoTest.java
index d30ed17f5ed57c71b3dd3eb6854b3a917a901686..ee0870424048b831af59c116f7cc2925a9524639 100644 (file)
--- a/server/sonar-db-dao/src/test/java/org/sonar/db/alm/pat/ALMPatDaoTest.java
+++ b/server/sonar-db-dao/src/test/java/org/sonar/db/alm/pat/ALMPatDaoTest.java
@@ -19,8 +19,6 @@
  */
 package org.sonar.db.alm.pat;
 
-import java.util.List;
-import org.assertj.core.api.Assertions;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.ExpectedException;
@@ -28,10 +26,9 @@ import org.sonar.api.utils.System2;
 import org.sonar.core.util.UuidFactory;
 import org.sonar.db.DbSession;
 import org.sonar.db.DbTester;
-import org.sonar.db.alm.pat.AlmPatDao;
-import org.sonar.db.alm.pat.AlmPatDto;
 import org.sonar.db.alm.setting.AlmSettingDao;
 import org.sonar.db.alm.setting.AlmSettingDto;
+import org.sonar.db.user.UserDto;
 
 import static org.apache.commons.lang.RandomStringUtils.randomAlphanumeric;
 import static org.assertj.core.api.Assertions.assertThat;
@@ -39,7 +36,6 @@ import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 import static org.sonar.db.alm.integration.pat.AlmPatsTesting.newAlmPatDto;
 import static org.sonar.db.almsettings.AlmSettingsTesting.newGithubAlmSettingDto;
-import static org.sonar.db.user.UserTesting.newUserDto;
 
 public class ALMPatDaoTest {
 
@@ -89,27 +85,14 @@ public class ALMPatDaoTest {
     almPatDto.setUserUuid(userUuid);
     underTest.insert(dbSession, almPatDto);
 
-    assertThat(underTest.selectByAlmSetting(dbSession, userUuid, almSetting).get())
+    assertThat(underTest.selectByUserAndAlmSetting(dbSession, userUuid, almSetting).get())
       .extracting(AlmPatDto::getUuid, AlmPatDto::getPersonalAccessToken,
         AlmPatDto::getUserUuid, AlmPatDto::getAlmSettingUuid,
         AlmPatDto::getCreatedAt, AlmPatDto::getUpdatedAt)
       .containsExactly(A_UUID, almPatDto.getPersonalAccessToken(),
         userUuid, almSetting.getUuid(), NOW, NOW);
 
-    assertThat(underTest.selectByAlmSetting(dbSession, randomAlphanumeric(40), newGithubAlmSettingDto())).isNotPresent();
-  }
-
-  @Test
-  public void selectAll() {
-    when(uuidFactory.create()).thenReturn(A_UUID);
-    when(system2.now()).thenReturn(NOW);
-    underTest.insert(dbSession, newAlmPatDto());
-    when(uuidFactory.create()).thenReturn(A_UUID + "bis");
-    underTest.insert(dbSession, newAlmPatDto());
-
-    List<AlmPatDto> almPats = underTest.selectAll(dbSession);
-
-    Assertions.assertThat(almPats).size().isEqualTo(2);
+    assertThat(underTest.selectByUserAndAlmSetting(dbSession, randomAlphanumeric(40), newGithubAlmSettingDto())).isNotPresent();
   }
 
   @Test
@@ -146,7 +129,34 @@ public class ALMPatDaoTest {
     underTest.delete(dbSession, almPat);
 
     assertThat(underTest.selectByUuid(dbSession, almPat.getUuid()).isPresent()).isFalse();
+  }
+
+  @Test
+  public void deleteByUser() {
+    when(uuidFactory.create()).thenReturn(A_UUID);
+    when(system2.now()).thenReturn(NOW);
+    UserDto userDto = db.users().insertUser();
+    AlmPatDto almPat = newAlmPatDto();
+    almPat.setUserUuid(userDto.getUuid());
+    underTest.insert(dbSession, almPat);
+
+    underTest.deleteByUser(dbSession, userDto);
+
+    assertThat(underTest.selectByUuid(dbSession, almPat.getUuid()).isPresent()).isFalse();
+  }
 
+  @Test
+  public void deleteByAlmSetting() {
+    when(uuidFactory.create()).thenReturn(A_UUID);
+    when(system2.now()).thenReturn(NOW);
+    AlmSettingDto almSettingDto = db.almSettings().insertBitbucketAlmSetting();
+    AlmPatDto almPat = newAlmPatDto();
+    almPat.setAlmSettingUuid(almSettingDto.getUuid());
+    underTest.insert(dbSession, almPat);
+
+    underTest.deleteByAlmSetting(dbSession, almSettingDto);
+
+    assertThat(underTest.selectByUuid(dbSession, almPat.getUuid()).isPresent()).isFalse();
   }
 
 }

diff --git a/server/sonar-db-migration/src/test/java/org/sonar/server/platform/db/migration/version/v82/CreateAlmPATsTableTest.java b/server/sonar-db-migration/src/test/java/org/sonar/server/platform/db/migration/version/v82/CreateAlmPATsTableTest.java
index bdffd01bb1d0dfbddc53a9b5c16f7b266d97ce64..464f86a19fa03383a950a24b9b9ff446ae47b14c 100644 (file)
--- a/server/sonar-db-migration/src/test/java/org/sonar/server/platform/db/migration/version/v82/CreateAlmPATsTableTest.java
+++ b/server/sonar-db-migration/src/test/java/org/sonar/server/platform/db/migration/version/v82/CreateAlmPATsTableTest.java
@@ -51,7 +51,7 @@ public class CreateAlmPATsTableTest {
 
     dbTester.assertColumnDefinition(TABLE_NAME, "uuid", VARCHAR, 40, false);
     dbTester.assertColumnDefinition(TABLE_NAME, "pat", VARCHAR, 2000, false);
-    dbTester.assertColumnDefinition(TABLE_NAME, "user_uuid", VARCHAR, 40, false);
+    dbTester.assertColumnDefinition(TABLE_NAME, "user_uuid", VARCHAR, 256, false);
     dbTester.assertColumnDefinition(TABLE_NAME, "alm_setting_uuid", VARCHAR, 40, false);
     dbTester.assertColumnDefinition(TABLE_NAME, "updated_at", BIGINT, 20, false);
     dbTester.assertColumnDefinition(TABLE_NAME, "created_at", BIGINT, 20, false);

diff --git a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/user/ws/DeactivateAction.java b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/user/ws/DeactivateAction.java
index e0231623046fdeae1b5d006bf399ff0a3bb34a24..2c0254899b141b60e919844fc9ec16f4419b807f 100644 (file)
--- a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/user/ws/DeactivateAction.java
+++ b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/user/ws/DeactivateAction.java
@@ -119,10 +119,11 @@ public class DeactivateAction implements UsersWsAction {
       dbClient.qProfileEditUsersDao().deleteByUser(dbSession, user);
       dbClient.organizationMemberDao().deleteByUserId(dbSession, userId);
       dbClient.userPropertiesDao().deleteByUser(dbSession, user);
+      dbClient.almPatDao().deleteByUser(dbSession, user);
       deactivateUser(dbSession, user);
       userIndexer.commitAndIndex(dbSession, user);
 
-      LOGGER.info("Deactivate user: {}; by admin: {}", login, userSession.isSystemAdministrator());
+      LOGGER.debug("Deactivate user: {}; by admin: {}", login, userSession.isSystemAdministrator());
     }
 
     writeResponse(response, login);

diff --git a/server/sonar-webserver-webapi/src/test/java/org/sonar/server/user/ws/DeactivateActionTest.java b/server/sonar-webserver-webapi/src/test/java/org/sonar/server/user/ws/DeactivateActionTest.java
index b190790464f07f4d791c76d1e3ef7e76ab2bd283..04a29d08ae3fdd03fe8e03c880d8c13ee8ef4d19 100644 (file)
--- a/server/sonar-webserver-webapi/src/test/java/org/sonar/server/user/ws/DeactivateActionTest.java
+++ b/server/sonar-webserver-webapi/src/test/java/org/sonar/server/user/ws/DeactivateActionTest.java
@@ -30,6 +30,7 @@ import org.sonar.api.utils.System2;
 import org.sonar.db.DbClient;
 import org.sonar.db.DbSession;
 import org.sonar.db.DbTester;
+import org.sonar.db.alm.setting.AlmSettingDto;
 import org.sonar.db.component.ComponentDto;
 import org.sonar.db.organization.OrganizationDto;
 import org.sonar.db.permission.template.PermissionTemplateDto;
@@ -243,6 +244,23 @@ public class DeactivateActionTest {
     assertThat(db.getDbClient().userPropertiesDao().selectByUser(dbSession, anotherUser)).hasSize(1);
   }
 
+  @Test
+  public void deactivate_user_deletes_his_alm_pat() {
+    logInAsSystemAdministrator();
+
+    AlmSettingDto almSettingDto = db.almSettings().insertBitbucketAlmSetting();
+
+    UserDto user = db.users().insertUser();
+    db.almPats().insert(p -> p.setUserUuid(user.getUuid()), p -> p.setAlmSettingUuid(almSettingDto.getUuid()));
+    UserDto anotherUser = db.users().insertUser();
+    db.almPats().insert(p -> p.setUserUuid(anotherUser.getUuid()), p -> p.setAlmSettingUuid(almSettingDto.getUuid()));
+
+    deactivate(user.getLogin());
+
+    assertThat(db.getDbClient().almPatDao().selectByUserAndAlmSetting(dbSession, user.getUuid(), almSettingDto)).isEmpty();
+    assertThat(db.getDbClient().almPatDao().selectByUserAndAlmSetting(dbSession, anotherUser.getUuid(), almSettingDto)).isNotNull();
+  }
+
   @Test
   public void user_can_deactivate_itself_on_sonarcloud() {
     WsActionTester customWs = newSonarCloudWs();

diff --git a/sonar-ws/src/main/java/org/sonarqube/ws/client/almintegration/AlmIntegrationService.java b/sonar-ws/src/main/java/org/sonarqube/ws/client/almintegration/AlmIntegrationService.java
index 9a159675c55986d75f11677d46968a9b2eb97b29..e68e67138ac47d36fb8a699838f24cfeec38afa2 100644 (file)
--- a/sonar-ws/src/main/java/org/sonarqube/ws/client/almintegration/AlmIntegrationService.java
+++ b/sonar-ws/src/main/java/org/sonarqube/ws/client/almintegration/AlmIntegrationService.java
@@ -61,6 +61,4 @@ public class AlmIntegrationService extends BaseService {
         .setMediaType(MediaTypes.JSON)).content();
   }
 
-  //xx add projectList and repoList
-
 }