From 968bea1a72f1310e76b671e7e633db1e846a99ff Mon Sep 17 00:00:00 2001 From: Julien Lancelot Date: Thu, 14 Feb 2019 16:15:31 +0100 Subject: [PATCH] Rename ownerId to organizationAlmId and almId to alm in AlmAppInstallDto --- .../org/sonar/db/alm/AlmAppInstallDao.java | 29 ++--- .../org/sonar/db/alm/AlmAppInstallDto.java | 33 +++--- .../org/sonar/db/alm/AlmAppInstallMapper.java | 12 +- .../org/sonar/db/alm/AlmAppInstallMapper.xml | 26 ++--- .../sonar/db/alm/AlmAppInstallDaoTest.java | 104 +++++++++--------- .../java/org/sonar/db/alm/AlmDbTester.java | 10 +- .../db/alm/OrganizationAlmBindingDaoTest.java | 8 +- .../db/organization/OrganizationDaoTest.java | 3 +- ...serRegistrarImplOrgMembershipSyncTest.java | 22 ++-- .../organization/MemberUpdaterTest.java | 20 ++-- 10 files changed, 131 insertions(+), 136 deletions(-) diff --git a/server/sonar-db-dao/src/main/java/org/sonar/db/alm/AlmAppInstallDao.java b/server/sonar-db-dao/src/main/java/org/sonar/db/alm/AlmAppInstallDao.java index 932beca42be..7f43a45d2f6 100644 --- a/server/sonar-db-dao/src/main/java/org/sonar/db/alm/AlmAppInstallDao.java +++ b/server/sonar-db-dao/src/main/java/org/sonar/db/alm/AlmAppInstallDao.java @@ -50,12 +50,12 @@ public class AlmAppInstallDao implements Dao { return Optional.ofNullable(mapper.selectByUuid(uuid)); } - public Optional selectByOwnerId(DbSession dbSession, ALM alm, String ownerId) { + public Optional selectByOrganizationAlmId(DbSession dbSession, ALM alm, String organizationAlmId) { checkAlm(alm); - checkOwnerId(ownerId); + checkOrganizationAlmId(organizationAlmId); AlmAppInstallMapper mapper = getMapper(dbSession); - return Optional.ofNullable(mapper.selectByOwnerId(alm.getId(), ownerId)); + return Optional.ofNullable(mapper.selectByOrganizationAlmId(alm.getId(), organizationAlmId)); } public Optional selectByInstallationId(DbSession dbSession, ALM alm, String installationId) { @@ -73,38 +73,33 @@ public class AlmAppInstallDao implements Dao { return getMapper(dbSession).selectUnboundByUserExternalId(userExternalId); } - /** - * @param alm Unique identifier of the ALM, like 'bitbucketcloud' or 'github', can't be null - * @param ownerId ALM specific identifier of the owner of the app, like team or user uuid for Bitbucket Cloud or organization id for Github, can't be null - * @param installId ALM specific identifier of the app installation, can't be null - */ - public void insertOrUpdate(DbSession dbSession, ALM alm, String ownerId, @Nullable Boolean isOwnerUser, String installId, @Nullable String userExternalId) { + public void insertOrUpdate(DbSession dbSession, ALM alm, String organizationAlmId, @Nullable Boolean isOwnerUser, String installId, @Nullable String userExternalId) { checkAlm(alm); - checkOwnerId(ownerId); + checkOrganizationAlmId(organizationAlmId); checkArgument(isNotEmpty(installId), "installId can't be null nor empty"); AlmAppInstallMapper mapper = getMapper(dbSession); long now = system2.now(); - if (mapper.update(alm.getId(), ownerId, isOwnerUser, installId, userExternalId, now) == 0) { - mapper.insert(uuidFactory.create(), alm.getId(), ownerId, isOwnerUser, installId, userExternalId, now); + if (mapper.update(alm.getId(), organizationAlmId, isOwnerUser, installId, userExternalId, now) == 0) { + mapper.insert(uuidFactory.create(), alm.getId(), organizationAlmId, isOwnerUser, installId, userExternalId, now); } } - public void delete(DbSession dbSession, ALM alm, String ownerId) { + public void delete(DbSession dbSession, ALM alm, String organizationAlmId) { checkAlm(alm); - checkOwnerId(ownerId); + checkOrganizationAlmId(organizationAlmId); AlmAppInstallMapper mapper = getMapper(dbSession); - mapper.delete(alm.getId(), ownerId); + mapper.delete(alm.getId(), organizationAlmId); } private static void checkAlm(@Nullable ALM alm) { requireNonNull(alm, "alm can't be null"); } - private static void checkOwnerId(@Nullable String ownerId) { - checkArgument(isNotEmpty(ownerId), "ownerId can't be null nor empty"); + private static void checkOrganizationAlmId(@Nullable String organizationAlmId) { + checkArgument(isNotEmpty(organizationAlmId), "organizationAlmId can't be null nor empty"); } private static AlmAppInstallMapper getMapper(DbSession dbSession) { diff --git a/server/sonar-db-dao/src/main/java/org/sonar/db/alm/AlmAppInstallDto.java b/server/sonar-db-dao/src/main/java/org/sonar/db/alm/AlmAppInstallDto.java index 2b11de83d67..c0d811555c1 100644 --- a/server/sonar-db-dao/src/main/java/org/sonar/db/alm/AlmAppInstallDto.java +++ b/server/sonar-db-dao/src/main/java/org/sonar/db/alm/AlmAppInstallDto.java @@ -29,15 +29,20 @@ public class AlmAppInstallDto { */ private String uuid; /** - * alm_id, can't be null + * Identifier of the ALM, like 'bitbucketcloud' or 'github', can't be null. + * Note that the db column is named alm_id. + * + * @see ALM for the list of available values */ - private String almId; + private String rawAlm; /** - * Owner id, can't be null + * ALM specific identifier of the organization, like team or user uuid for Bitbucket Cloud or organization id for Github, can't be null. + * Note that the column is badly named owner_id, in the first place it was only possible to install personal organizations. + * The column name has been kept to prevent doing a db migration. */ - private String ownerId; + private String organizationAlmId; /** - * Installation id, can't be null + * ALM specific identifier of the app installation, can't be null */ private String installId; /** @@ -61,25 +66,21 @@ public class AlmAppInstallDto { return this; } - public String getAlmId() { - return almId; - } - public ALM getAlm() { - return ALM.fromId(almId); + return ALM.fromId(rawAlm); } - public AlmAppInstallDto setAlmId(String almId) { - this.almId = almId; + public AlmAppInstallDto setAlm(ALM alm) { + this.rawAlm = alm.getId(); return this; } - public String getOwnerId() { - return ownerId; + public String getOrganizationAlmId() { + return organizationAlmId; } - public AlmAppInstallDto setOwnerId(String ownerId) { - this.ownerId = ownerId; + public AlmAppInstallDto setOrganizationAlmId(String organizationAlmId) { + this.organizationAlmId = organizationAlmId; return this; } diff --git a/server/sonar-db-dao/src/main/java/org/sonar/db/alm/AlmAppInstallMapper.java b/server/sonar-db-dao/src/main/java/org/sonar/db/alm/AlmAppInstallMapper.java index fe7c55e2d56..9d5e6254522 100644 --- a/server/sonar-db-dao/src/main/java/org/sonar/db/alm/AlmAppInstallMapper.java +++ b/server/sonar-db-dao/src/main/java/org/sonar/db/alm/AlmAppInstallMapper.java @@ -27,24 +27,24 @@ import org.apache.ibatis.annotations.Param; public interface AlmAppInstallMapper { @CheckForNull - AlmAppInstallDto selectByOwnerId(@Param("almId") String almId, @Param("ownerId") String ownerId); + AlmAppInstallDto selectByOrganizationAlmId(@Param("alm") String alm, @Param("organizationAlmId") String organizationAlmId); @CheckForNull - AlmAppInstallDto selectByInstallationId(@Param("almId") String almId, @Param("installId") String installId); + AlmAppInstallDto selectByInstallationId(@Param("alm") String alm, @Param("installId") String installId); @CheckForNull AlmAppInstallDto selectByUuid(@Param("uuid") String uuid); @CheckForNull - AlmAppInstallDto selectByOrganizationUuid(@Param("almId") String almId, @Param("organizationUuid") String organizationUuid); + AlmAppInstallDto selectByOrganizationUuid(@Param("alm") String alm, @Param("organizationUuid") String organizationUuid); List selectUnboundByUserExternalId(@Param("userExternalId") String userExternalId); - void insert(@Param("uuid") String uuid, @Param("almId") String almId, @Param("ownerId") String ownerId, + void insert(@Param("uuid") String uuid, @Param("alm") String alm, @Param("organizationAlmId") String organizationAlmId, @Nullable @Param("isOwnerUser") Boolean isOwnerUser, @Param("installId") String installId, @Nullable @Param("userExternalId") String userExternalId, @Param("now") long now); - int update(@Param("almId") String almId, @Param("ownerId") String ownerId, + int update(@Param("alm") String alm, @Param("organizationAlmId") String organizationAlmId, @Nullable @Param("isOwnerUser") Boolean isOwnerUser, @Param("installId") String installId, @Nullable @Param("userExternalId") String userExternalId, @Param("now") long now); - void delete(@Param("almId") String almId, @Param("ownerId") String ownerId); + void delete(@Param("alm") String alm, @Param("organizationAlmId") String organizationAlmId); } diff --git a/server/sonar-db-dao/src/main/resources/org/sonar/db/alm/AlmAppInstallMapper.xml b/server/sonar-db-dao/src/main/resources/org/sonar/db/alm/AlmAppInstallMapper.xml index b29b7ec7268..7f4718b904b 100644 --- a/server/sonar-db-dao/src/main/resources/org/sonar/db/alm/AlmAppInstallMapper.xml +++ b/server/sonar-db-dao/src/main/resources/org/sonar/db/alm/AlmAppInstallMapper.xml @@ -5,8 +5,8 @@ aai.uuid, - aai.alm_id as almId, - aai.owner_id as ownerId, + aai.alm_id as rawAlm, + aai.owner_id as organizationAlmId, aai.is_owner_user as isOwnerUser, aai.install_id as installId, aai.user_external_id as userExternalId, @@ -14,13 +14,13 @@ aai.updated_at as updatedAt - select from alm_app_installs aai where - alm_id = #{almId, jdbcType=VARCHAR} - and owner_id = #{ownerId, jdbcType=VARCHAR} + alm_id = #{alm, jdbcType=VARCHAR} + and owner_id = #{organizationAlmId, jdbcType=VARCHAR} @@ -46,7 +46,7 @@ alm_app_installs aai inner join organization_alm_bindings oab on oab.alm_app_install_uuid = aai.uuid where - oab.alm_id = #{almId, jdbcType=VARCHAR} + oab.alm_id = #{alm, jdbcType=VARCHAR} and oab.organization_uuid = #{organizationUuid, jdbcType=VARCHAR} @@ -74,8 +74,8 @@ ) VALUES ( #{uuid, jdbcType=VARCHAR}, - #{almId, jdbcType=VARCHAR}, - #{ownerId, jdbcType=VARCHAR}, + #{alm, jdbcType=VARCHAR}, + #{organizationAlmId, jdbcType=VARCHAR}, #{isOwnerUser, jdbcType=BOOLEAN}, #{installId, jdbcType=VARCHAR}, #{userExternalId, jdbcType=VARCHAR}, @@ -91,15 +91,15 @@ user_external_id = #{userExternalId, jdbcType=VARCHAR}, updated_at = #{now, jdbcType=BIGINT} where - alm_id = #{almId, jdbcType=VARCHAR} - and owner_id = #{ownerId, jdbcType=VARCHAR} + alm_id = #{alm, jdbcType=VARCHAR} + and owner_id = #{organizationAlmId, jdbcType=VARCHAR} delete from alm_app_installs where - alm_id = #{almId, jdbcType=VARCHAR} - and owner_id = #{ownerId, jdbcType=VARCHAR} + alm_id = #{alm, jdbcType=VARCHAR} + and owner_id = #{organizationAlmId, jdbcType=VARCHAR} diff --git a/server/sonar-db-dao/src/test/java/org/sonar/db/alm/AlmAppInstallDaoTest.java b/server/sonar-db-dao/src/test/java/org/sonar/db/alm/AlmAppInstallDaoTest.java index babb422fd3a..3f59bcee22d 100644 --- a/server/sonar-db-dao/src/test/java/org/sonar/db/alm/AlmAppInstallDaoTest.java +++ b/server/sonar-db-dao/src/test/java/org/sonar/db/alm/AlmAppInstallDaoTest.java @@ -45,8 +45,8 @@ public class AlmAppInstallDaoTest { private static final String A_UUID = "abcde1234"; private static final String A_UUID_2 = "xyz789"; private static final String EMPTY_STRING = ""; - private static final String A_OWNER = "my_org_id"; - private static final String ANOTHER_OWNER = "another_org"; + private static final String AN_ORGANIZATION_ALM_ID = "my_org_id"; + private static final String ANOTHER_ORGANIZATION_ALM_ID = "another_org"; private static final long DATE = 1_600_000_000_000L; private static final long DATE_LATER = 1_700_000_000_000L; private static final String AN_INSTALL = "some install id"; @@ -68,62 +68,62 @@ public class AlmAppInstallDaoTest { when(uuidFactory.create()).thenReturn(A_UUID); when(system2.now()).thenReturn(DATE); String userUuid = Uuids.createFast(); - underTest.insertOrUpdate(dbSession, GITHUB, A_OWNER, true, AN_INSTALL, userUuid); + underTest.insertOrUpdate(dbSession, GITHUB, AN_ORGANIZATION_ALM_ID, true, AN_INSTALL, userUuid); assertThat(underTest.selectByUuid(dbSession, A_UUID).get()) - .extracting(AlmAppInstallDto::getUuid, AlmAppInstallDto::getAlm, AlmAppInstallDto::getInstallId, AlmAppInstallDto::getOwnerId, AlmAppInstallDto::getUserExternalId, + .extracting(AlmAppInstallDto::getUuid, AlmAppInstallDto::getAlm, AlmAppInstallDto::getInstallId, AlmAppInstallDto::getOrganizationAlmId, AlmAppInstallDto::getUserExternalId, AlmAppInstallDto::getCreatedAt, AlmAppInstallDto::getUpdatedAt) - .contains(A_UUID, GITHUB, A_OWNER, AN_INSTALL, userUuid, DATE, DATE); + .contains(A_UUID, GITHUB, AN_ORGANIZATION_ALM_ID, AN_INSTALL, userUuid, DATE, DATE); assertThat(underTest.selectByUuid(dbSession, "foo")).isNotPresent(); } @Test - public void selectByOwnerId() { + public void selectByOrganizationAlmId() { when(uuidFactory.create()).thenReturn(A_UUID); when(system2.now()).thenReturn(DATE); - underTest.insertOrUpdate(dbSession, GITHUB, A_OWNER, true, AN_INSTALL, null); + underTest.insertOrUpdate(dbSession, GITHUB, AN_ORGANIZATION_ALM_ID, true, AN_INSTALL, null); - assertThat(underTest.selectByOwnerId(dbSession, GITHUB, A_OWNER).get()) - .extracting(AlmAppInstallDto::getUuid, AlmAppInstallDto::getAlm, AlmAppInstallDto::getInstallId, AlmAppInstallDto::getOwnerId, + assertThat(underTest.selectByOrganizationAlmId(dbSession, GITHUB, AN_ORGANIZATION_ALM_ID).get()) + .extracting(AlmAppInstallDto::getUuid, AlmAppInstallDto::getAlm, AlmAppInstallDto::getInstallId, AlmAppInstallDto::getOrganizationAlmId, AlmAppInstallDto::getCreatedAt, AlmAppInstallDto::getUpdatedAt) - .contains(A_UUID, GITHUB, A_OWNER, AN_INSTALL, DATE, DATE); + .contains(A_UUID, GITHUB, AN_ORGANIZATION_ALM_ID, AN_INSTALL, DATE, DATE); - assertThat(underTest.selectByOwnerId(dbSession, BITBUCKETCLOUD, A_OWNER)).isNotPresent(); - assertThat(underTest.selectByOwnerId(dbSession, GITHUB, "Unknown owner")).isNotPresent(); + assertThat(underTest.selectByOrganizationAlmId(dbSession, BITBUCKETCLOUD, AN_ORGANIZATION_ALM_ID)).isNotPresent(); + assertThat(underTest.selectByOrganizationAlmId(dbSession, GITHUB, "Unknown owner")).isNotPresent(); } @Test public void selectByOwner_throws_NPE_when_alm_is_null() { expectAlmNPE(); - underTest.selectByOwnerId(dbSession, null, A_OWNER); + underTest.selectByOrganizationAlmId(dbSession, null, AN_ORGANIZATION_ALM_ID); } @Test - public void selectByOwner_throws_IAE_when_owner_id_is_null() { - expectOwnerIdNullOrEmptyIAE(); + public void selectByOwner_throws_IAE_when_organization_alm_id_is_null() { + expectOrganizationAlmIdNullOrEmptyIAE(); - underTest.selectByOwnerId(dbSession, GITHUB, null); + underTest.selectByOrganizationAlmId(dbSession, GITHUB, null); } @Test - public void selectByOwner_throws_IAE_when_owner_id_is_empty() { - expectOwnerIdNullOrEmptyIAE(); + public void selectByOwner_throws_IAE_when_organization_alm_id_is_empty() { + expectOrganizationAlmIdNullOrEmptyIAE(); - underTest.selectByOwnerId(dbSession, GITHUB, EMPTY_STRING); + underTest.selectByOrganizationAlmId(dbSession, GITHUB, EMPTY_STRING); } @Test public void selectByInstallationId() { when(uuidFactory.create()).thenReturn(A_UUID); when(system2.now()).thenReturn(DATE); - underTest.insertOrUpdate(dbSession, GITHUB, A_OWNER, true, AN_INSTALL, Uuids.createFast()); + underTest.insertOrUpdate(dbSession, GITHUB, AN_ORGANIZATION_ALM_ID, true, AN_INSTALL, Uuids.createFast()); assertThat(underTest.selectByInstallationId(dbSession, GITHUB, AN_INSTALL).get()) - .extracting(AlmAppInstallDto::getUuid, AlmAppInstallDto::getAlm, AlmAppInstallDto::getInstallId, AlmAppInstallDto::getOwnerId, + .extracting(AlmAppInstallDto::getUuid, AlmAppInstallDto::getAlm, AlmAppInstallDto::getInstallId, AlmAppInstallDto::getOrganizationAlmId, AlmAppInstallDto::getCreatedAt, AlmAppInstallDto::getUpdatedAt) - .contains(A_UUID, GITHUB, A_OWNER, AN_INSTALL, DATE, DATE); + .contains(A_UUID, GITHUB, AN_ORGANIZATION_ALM_ID, AN_INSTALL, DATE, DATE); assertThat(underTest.selectByInstallationId(dbSession, GITHUB, "unknown installation")).isEmpty(); assertThat(underTest.selectByInstallationId(dbSession, BITBUCKETCLOUD, AN_INSTALL)).isEmpty(); @@ -154,7 +154,7 @@ public class AlmAppInstallDaoTest { OrganizationDto organization = db.organizations().insert(); db.getDbClient().almAppInstallDao().insertOrUpdate(db.getSession(), ALM.GITHUB, "the-owner", false, "123456", null); // could be improved, insertOrUpdate should return the DTO with its uuid - Optional install = db.getDbClient().almAppInstallDao().selectByOwnerId(db.getSession(), ALM.GITHUB, "the-owner"); + Optional install = db.getDbClient().almAppInstallDao().selectByOrganizationAlmId(db.getSession(), ALM.GITHUB, "the-owner"); db.getDbClient().organizationAlmBindingDao().insert(db.getSession(), organization, install.get(), "xxx", "xxx", true); db.commit(); @@ -167,19 +167,19 @@ public class AlmAppInstallDaoTest { public void insert_throws_NPE_if_alm_is_null() { expectAlmNPE(); - underTest.insertOrUpdate(dbSession, null, A_OWNER, true, AN_INSTALL, null); + underTest.insertOrUpdate(dbSession, null, AN_ORGANIZATION_ALM_ID, true, AN_INSTALL, null); } @Test - public void insert_throws_IAE_if_owner_id_is_null() { - expectOwnerIdNullOrEmptyIAE(); + public void insert_throws_IAE_if_organization_alm_id_is_null() { + expectOrganizationAlmIdNullOrEmptyIAE(); underTest.insertOrUpdate(dbSession, GITHUB, null, true, AN_INSTALL, null); } @Test - public void insert_throws_IAE_if_owner_id_is_empty() { - expectOwnerIdNullOrEmptyIAE(); + public void insert_throws_IAE_if_organization_alm_id_is_empty() { + expectOrganizationAlmIdNullOrEmptyIAE(); underTest.insertOrUpdate(dbSession, GITHUB, EMPTY_STRING, true, AN_INSTALL, null); } @@ -188,14 +188,14 @@ public class AlmAppInstallDaoTest { public void insert_throws_IAE_if_install_id_is_null() { expectInstallIdNullOrEmptyIAE(); - underTest.insertOrUpdate(dbSession, GITHUB, A_OWNER, true, null, null); + underTest.insertOrUpdate(dbSession, GITHUB, AN_ORGANIZATION_ALM_ID, true, null, null); } @Test public void insert_throws_IAE_if_install_id_is_empty() { expectInstallIdNullOrEmptyIAE(); - underTest.insertOrUpdate(dbSession, GITHUB, A_OWNER, true, EMPTY_STRING, null); + underTest.insertOrUpdate(dbSession, GITHUB, AN_ORGANIZATION_ALM_ID, true, EMPTY_STRING, null); } @Test @@ -203,9 +203,9 @@ public class AlmAppInstallDaoTest { when(uuidFactory.create()).thenReturn(A_UUID); when(system2.now()).thenReturn(DATE); String userUuid = Uuids.createFast(); - underTest.insertOrUpdate(dbSession, GITHUB, A_OWNER, true, AN_INSTALL, userUuid); + underTest.insertOrUpdate(dbSession, GITHUB, AN_ORGANIZATION_ALM_ID, true, AN_INSTALL, userUuid); - assertThatAlmAppInstall(GITHUB, A_OWNER) + assertThatAlmAppInstall(GITHUB, AN_ORGANIZATION_ALM_ID) .hasInstallId(AN_INSTALL) .hasUserExternalId(userUuid) .hasCreatedAt(DATE) @@ -216,18 +216,18 @@ public class AlmAppInstallDaoTest { public void delete() { when(uuidFactory.create()).thenReturn(A_UUID); when(system2.now()).thenReturn(DATE); - underTest.insertOrUpdate(dbSession, GITHUB, A_OWNER, true, AN_INSTALL, null); + underTest.insertOrUpdate(dbSession, GITHUB, AN_ORGANIZATION_ALM_ID, true, AN_INSTALL, null); - underTest.delete(dbSession, GITHUB, A_OWNER); + underTest.delete(dbSession, GITHUB, AN_ORGANIZATION_ALM_ID); - assertThatAlmAppInstall(GITHUB, A_OWNER).doesNotExist(); + assertThatAlmAppInstall(GITHUB, AN_ORGANIZATION_ALM_ID).doesNotExist(); } @Test public void delete_does_not_fail() { - assertThatAlmAppInstall(GITHUB, A_OWNER).doesNotExist(); + assertThatAlmAppInstall(GITHUB, AN_ORGANIZATION_ALM_ID).doesNotExist(); - underTest.delete(dbSession, GITHUB, A_OWNER); + underTest.delete(dbSession, GITHUB, AN_ORGANIZATION_ALM_ID); } @Test @@ -235,13 +235,13 @@ public class AlmAppInstallDaoTest { when(uuidFactory.create()).thenReturn(A_UUID); when(system2.now()).thenReturn(DATE); String userExternalId1 = randomAlphanumeric(10); - underTest.insertOrUpdate(dbSession, GITHUB, A_OWNER, true, AN_INSTALL, userExternalId1); + underTest.insertOrUpdate(dbSession, GITHUB, AN_ORGANIZATION_ALM_ID, true, AN_INSTALL, userExternalId1); when(system2.now()).thenReturn(DATE_LATER); String userExternalId2 = randomAlphanumeric(10); - underTest.insertOrUpdate(dbSession, GITHUB, A_OWNER, true, OTHER_INSTALL, userExternalId2); + underTest.insertOrUpdate(dbSession, GITHUB, AN_ORGANIZATION_ALM_ID, true, OTHER_INSTALL, userExternalId2); - assertThatAlmAppInstall(GITHUB, A_OWNER) + assertThatAlmAppInstall(GITHUB, AN_ORGANIZATION_ALM_ID) .hasInstallId(OTHER_INSTALL) .hasUserExternalId(userExternalId2) .hasCreatedAt(DATE) @@ -256,17 +256,17 @@ public class AlmAppInstallDaoTest { .thenReturn(A_UUID_2); String userExternalId1 = randomAlphanumeric(10); String userExternalId2 = randomAlphanumeric(10); - underTest.insertOrUpdate(dbSession, GITHUB, A_OWNER, true, AN_INSTALL, userExternalId1); - underTest.insertOrUpdate(dbSession, GITHUB, ANOTHER_OWNER, false, OTHER_INSTALL, userExternalId2); + underTest.insertOrUpdate(dbSession, GITHUB, AN_ORGANIZATION_ALM_ID, true, AN_INSTALL, userExternalId1); + underTest.insertOrUpdate(dbSession, GITHUB, ANOTHER_ORGANIZATION_ALM_ID, false, OTHER_INSTALL, userExternalId2); - assertThatAlmAppInstall(GITHUB, A_OWNER) + assertThatAlmAppInstall(GITHUB, AN_ORGANIZATION_ALM_ID) .hasInstallId(AN_INSTALL) .hasOwnerUser(true) .hasUserExternalId(userExternalId1) .hasCreatedAt(DATE) .hasUpdatedAt(DATE); - assertThatAlmAppInstall(GITHUB, ANOTHER_OWNER) + assertThatAlmAppInstall(GITHUB, ANOTHER_ORGANIZATION_ALM_ID) .hasInstallId(OTHER_INSTALL) .hasOwnerUser(false) .hasUserExternalId(userExternalId2) @@ -279,9 +279,9 @@ public class AlmAppInstallDaoTest { expectedException.expectMessage("alm can't be null"); } - private void expectOwnerIdNullOrEmptyIAE() { + private void expectOrganizationAlmIdNullOrEmptyIAE() { expectedException.expect(IllegalArgumentException.class); - expectedException.expectMessage("ownerId can't be null nor empty"); + expectedException.expectMessage("organizationAlmId can't be null nor empty"); } private void expectInstallIdNullOrEmptyIAE() { @@ -289,18 +289,18 @@ public class AlmAppInstallDaoTest { expectedException.expectMessage("installId can't be null nor empty"); } - private AlmAppInstallAssert assertThatAlmAppInstall(ALM alm, String ownerId) { - return new AlmAppInstallAssert(db, dbSession, alm, ownerId); + private AlmAppInstallAssert assertThatAlmAppInstall(ALM alm, String organizationAlmId) { + return new AlmAppInstallAssert(db, dbSession, alm, organizationAlmId); } private static class AlmAppInstallAssert extends AbstractAssert { - private AlmAppInstallAssert(DbTester dbTester, DbSession dbSession, ALM alm, String ownerId) { - super(asAlmAppInstall(dbTester, dbSession, alm, ownerId), AlmAppInstallAssert.class); + private AlmAppInstallAssert(DbTester dbTester, DbSession dbSession, ALM alm, String organizationAlmId) { + super(asAlmAppInstall(dbTester, dbSession, alm, organizationAlmId), AlmAppInstallAssert.class); } - private static AlmAppInstallDto asAlmAppInstall(DbTester db, DbSession dbSession, ALM alm, String ownerId) { - Optional almAppInstall = db.getDbClient().almAppInstallDao().selectByOwnerId(dbSession, alm, ownerId); + private static AlmAppInstallDto asAlmAppInstall(DbTester db, DbSession dbSession, ALM alm, String organizationAlmId) { + Optional almAppInstall = db.getDbClient().almAppInstallDao().selectByOrganizationAlmId(dbSession, alm, organizationAlmId); return almAppInstall.orElse(null); } diff --git a/server/sonar-db-dao/src/test/java/org/sonar/db/alm/AlmDbTester.java b/server/sonar-db-dao/src/test/java/org/sonar/db/alm/AlmDbTester.java index a8163994f26..579f505a602 100644 --- a/server/sonar-db-dao/src/test/java/org/sonar/db/alm/AlmDbTester.java +++ b/server/sonar-db-dao/src/test/java/org/sonar/db/alm/AlmDbTester.java @@ -21,6 +21,7 @@ package org.sonar.db.alm; import java.util.Arrays; import java.util.function.Consumer; +import org.apache.commons.lang.math.RandomUtils; import org.sonar.db.DbTester; import org.sonar.db.component.ComponentDto; import org.sonar.db.organization.OrganizationDto; @@ -28,7 +29,6 @@ import org.sonar.db.user.UserDto; import static org.apache.commons.lang.RandomStringUtils.randomAlphabetic; import static org.apache.commons.lang.RandomStringUtils.randomAlphanumeric; -import static org.apache.commons.lang.RandomStringUtils.randomNumeric; import static org.sonar.db.alm.ALM.GITHUB; public class AlmDbTester { @@ -56,14 +56,14 @@ public class AlmDbTester { @SafeVarargs public final AlmAppInstallDto insertAlmAppInstall(Consumer... dtoPopulators) { AlmAppInstallDto dto = new AlmAppInstallDto() - .setAlmId(GITHUB.getId()) + .setAlm(GITHUB) .setInstallId(randomAlphanumeric(10)) - .setOwnerId(randomNumeric(10)) + .setOrganizationAlmId(Integer.toString(RandomUtils.nextInt())) .setIsOwnerUser(false) .setUserExternalId(randomAlphanumeric(10)); Arrays.stream(dtoPopulators).forEach(dtoPopulator -> dtoPopulator.accept(dto)); - db.getDbClient().almAppInstallDao().insertOrUpdate(db.getSession(), dto.getAlm(), dto.getOwnerId(), dto.isOwnerUser(), dto.getInstallId(), dto.getUserExternalId()); + db.getDbClient().almAppInstallDao().insertOrUpdate(db.getSession(), dto.getAlm(), dto.getOrganizationAlmId(), dto.isOwnerUser(), dto.getInstallId(), dto.getUserExternalId()); db.commit(); - return db.getDbClient().almAppInstallDao().selectByOwnerId(db.getSession(), dto.getAlm(), dto.getOwnerId()).get(); + return db.getDbClient().almAppInstallDao().selectByOrganizationAlmId(db.getSession(), dto.getAlm(), dto.getOrganizationAlmId()).get(); } } diff --git a/server/sonar-db-dao/src/test/java/org/sonar/db/alm/OrganizationAlmBindingDaoTest.java b/server/sonar-db-dao/src/test/java/org/sonar/db/alm/OrganizationAlmBindingDaoTest.java index 8f165bda4d1..a390b8327e1 100644 --- a/server/sonar-db-dao/src/test/java/org/sonar/db/alm/OrganizationAlmBindingDaoTest.java +++ b/server/sonar-db-dao/src/test/java/org/sonar/db/alm/OrganizationAlmBindingDaoTest.java @@ -150,15 +150,15 @@ public class OrganizationAlmBindingDaoTest { @Test public void selectByOrganizationAlmIds() { - AlmAppInstallDto gitHubInstall1 = db.alm().insertAlmAppInstall(a -> a.setAlmId(GITHUB.getId())); + AlmAppInstallDto gitHubInstall1 = db.alm().insertAlmAppInstall(a -> a.setAlm(GITHUB)); OrganizationAlmBindingDto organizationAlmBinding1 = db.alm().insertOrganizationAlmBinding(db.organizations().insert(), gitHubInstall1, true); - AlmAppInstallDto gitHubInstall2 = db.alm().insertAlmAppInstall(a -> a.setAlmId(GITHUB.getId())); + AlmAppInstallDto gitHubInstall2 = db.alm().insertAlmAppInstall(a -> a.setAlm(GITHUB)); OrganizationAlmBindingDto organizationAlmBinding2 = db.alm().insertOrganizationAlmBinding(db.organizations().insert(), gitHubInstall2, true); - AlmAppInstallDto bitBucketInstall = db.alm().insertAlmAppInstall(a -> a.setAlmId(BITBUCKETCLOUD.getId())); + AlmAppInstallDto bitBucketInstall = db.alm().insertAlmAppInstall(a -> a.setAlm(BITBUCKETCLOUD)); OrganizationAlmBindingDto organizationAlmBinding3 = db.alm().insertOrganizationAlmBinding(db.organizations().insert(), bitBucketInstall, true); List result = underTest.selectByOrganizationAlmIds(db.getSession(), GITHUB, - asList(gitHubInstall1.getOwnerId(), gitHubInstall2.getOwnerId(), bitBucketInstall.getOwnerId(), "unknown")); + asList(gitHubInstall1.getOrganizationAlmId(), gitHubInstall2.getOrganizationAlmId(), bitBucketInstall.getOrganizationAlmId(), "unknown")); assertThat(result).extracting(OrganizationAlmBindingDto::getUuid) .containsExactlyInAnyOrder(organizationAlmBinding1.getUuid(), organizationAlmBinding2.getUuid()); diff --git a/server/sonar-db-dao/src/test/java/org/sonar/db/organization/OrganizationDaoTest.java b/server/sonar-db-dao/src/test/java/org/sonar/db/organization/OrganizationDaoTest.java index 37f67e56c3d..a00c755e213 100644 --- a/server/sonar-db-dao/src/test/java/org/sonar/db/organization/OrganizationDaoTest.java +++ b/server/sonar-db-dao/src/test/java/org/sonar/db/organization/OrganizationDaoTest.java @@ -66,7 +66,6 @@ import static org.assertj.core.api.Assertions.tuple; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; import static org.sonar.db.Pagination.forPage; -import static org.sonar.db.alm.ALM.BITBUCKETCLOUD; import static org.sonar.db.alm.ALM.GITHUB; import static org.sonar.db.organization.OrganizationDto.Subscription.FREE; import static org.sonar.db.organization.OrganizationDto.Subscription.PAID; @@ -1121,7 +1120,7 @@ public class OrganizationDaoTest { private void insertOrgAlmBinding(OrganizationDto organization, ALM alm, String organizationAlmId) { dbClient.almAppInstallDao().insertOrUpdate(dbSession, alm, organizationAlmId, true, randomAlphabetic(8), randomAlphabetic(8)); - Optional almAppInstallDto = dbClient.almAppInstallDao().selectByOwnerId(dbSession, GITHUB, organizationAlmId); + Optional almAppInstallDto = dbClient.almAppInstallDao().selectByOrganizationAlmId(dbSession, GITHUB, organizationAlmId); dbClient.organizationAlmBindingDao().insert(dbSession, organization, almAppInstallDto.get(), "http://github.com/myteam", random(8),true); } diff --git a/server/sonar-server/src/test/java/org/sonar/server/authentication/UserRegistrarImplOrgMembershipSyncTest.java b/server/sonar-server/src/test/java/org/sonar/server/authentication/UserRegistrarImplOrgMembershipSyncTest.java index 5a940b3225d..cfb39c95859 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/authentication/UserRegistrarImplOrgMembershipSyncTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/authentication/UserRegistrarImplOrgMembershipSyncTest.java @@ -123,7 +123,7 @@ public class UserRegistrarImplOrgMembershipSyncTest { organizationFlags.setEnabled(true); OrganizationDto organization = db.organizations().insert(); db.users().insertDefaultGroup(organization, "Members"); - AlmAppInstallDto gitHubInstall = db.alm().insertAlmAppInstall(a -> a.setAlmId(GITHUB.getId())); + AlmAppInstallDto gitHubInstall = db.alm().insertAlmAppInstall(a -> a.setAlm(GITHUB)); db.alm().insertOrganizationAlmBinding(organization, gitHubInstall, true); underTest.register(UserRegistration.builder() @@ -132,7 +132,7 @@ public class UserRegistrarImplOrgMembershipSyncTest { .setSource(Source.realm(BASIC, GITHUB_PROVIDER.getName())) .setExistingEmailStrategy(ExistingEmailStrategy.ALLOW) .setUpdateLoginStrategy(UpdateLoginStrategy.ALLOW) - .setOrganizationAlmIds(ImmutableSet.of(gitHubInstall.getOwnerId())) + .setOrganizationAlmIds(ImmutableSet.of(gitHubInstall.getOrganizationAlmId())) .build()); UserDto user = db.users().selectUserByLogin(USER_LOGIN).get(); @@ -144,7 +144,7 @@ public class UserRegistrarImplOrgMembershipSyncTest { organizationFlags.setEnabled(true); OrganizationDto organization = db.organizations().insert(); db.users().insertDefaultGroup(organization, "Members"); - AlmAppInstallDto gitHubInstall = db.alm().insertAlmAppInstall(a -> a.setAlmId(GITHUB.getId())); + AlmAppInstallDto gitHubInstall = db.alm().insertAlmAppInstall(a -> a.setAlm(GITHUB)); db.alm().insertOrganizationAlmBinding(organization, gitHubInstall, true); underTest.register(UserRegistration.builder() @@ -165,7 +165,7 @@ public class UserRegistrarImplOrgMembershipSyncTest { organizationFlags.setEnabled(true); OrganizationDto organization = db.organizations().insert(); db.users().insertDefaultGroup(organization, "Members"); - AlmAppInstallDto gitHubInstall = db.alm().insertAlmAppInstall(a -> a.setAlmId(BITBUCKETCLOUD.getId())); + AlmAppInstallDto gitHubInstall = db.alm().insertAlmAppInstall(a -> a.setAlm(BITBUCKETCLOUD)); db.alm().insertOrganizationAlmBinding(organization, gitHubInstall, true); underTest.register(UserRegistration.builder() @@ -174,7 +174,7 @@ public class UserRegistrarImplOrgMembershipSyncTest { .setSource(Source.realm(BASIC, BITBUCKET_PROVIDER.getName())) .setExistingEmailStrategy(ExistingEmailStrategy.ALLOW) .setUpdateLoginStrategy(UpdateLoginStrategy.ALLOW) - .setOrganizationAlmIds(ImmutableSet.of(gitHubInstall.getOwnerId())) + .setOrganizationAlmIds(ImmutableSet.of(gitHubInstall.getOrganizationAlmId())) .build()); UserDto user = db.users().selectUserByLogin(USER_LOGIN).get(); @@ -186,7 +186,7 @@ public class UserRegistrarImplOrgMembershipSyncTest { organizationFlags.setEnabled(true); OrganizationDto organization = db.organizations().insert(); db.users().insertDefaultGroup(organization, "Members"); - AlmAppInstallDto almAppInstall = db.alm().insertAlmAppInstall(a -> a.setAlmId(GITHUB.getId())); + AlmAppInstallDto almAppInstall = db.alm().insertAlmAppInstall(a -> a.setAlm(GITHUB)); db.alm().insertOrganizationAlmBinding(organization, almAppInstall, true); TestIdentityProvider identityProvider = new TestIdentityProvider() .setKey("unknown") @@ -200,7 +200,7 @@ public class UserRegistrarImplOrgMembershipSyncTest { .setSource(Source.realm(BASIC, identityProvider.getName())) .setExistingEmailStrategy(ExistingEmailStrategy.ALLOW) .setUpdateLoginStrategy(UpdateLoginStrategy.ALLOW) - .setOrganizationAlmIds(ImmutableSet.of(almAppInstall.getOwnerId())) + .setOrganizationAlmIds(ImmutableSet.of(almAppInstall.getOrganizationAlmId())) .build()); UserDto user = db.users().selectUserByLogin(USER_LOGIN).get(); @@ -212,7 +212,7 @@ public class UserRegistrarImplOrgMembershipSyncTest { organizationFlags.setEnabled(true); OrganizationDto organization = db.organizations().insert(); db.users().insertDefaultGroup(organization, "Members"); - AlmAppInstallDto gitHubInstall = db.alm().insertAlmAppInstall(a -> a.setAlmId(GITHUB.getId())); + AlmAppInstallDto gitHubInstall = db.alm().insertAlmAppInstall(a -> a.setAlm(GITHUB)); db.alm().insertOrganizationAlmBinding(organization, gitHubInstall, true); UserDto user = db.users().insertUser(u -> u .setLogin("Old login") @@ -225,7 +225,7 @@ public class UserRegistrarImplOrgMembershipSyncTest { .setSource(Source.local(BASIC)) .setExistingEmailStrategy(ExistingEmailStrategy.FORBID) .setUpdateLoginStrategy(UpdateLoginStrategy.ALLOW) - .setOrganizationAlmIds(ImmutableSet.of(gitHubInstall.getOwnerId())) + .setOrganizationAlmIds(ImmutableSet.of(gitHubInstall.getOrganizationAlmId())) .build()); db.organizations().assertUserIsNotMemberOfOrganization(organization, user); @@ -236,7 +236,7 @@ public class UserRegistrarImplOrgMembershipSyncTest { organizationFlags.setEnabled(true); OrganizationDto organization = db.organizations().insert(); db.users().insertDefaultGroup(organization, "Members"); - AlmAppInstallDto gitHubInstall = db.alm().insertAlmAppInstall(a -> a.setAlmId(GITHUB.getId())); + AlmAppInstallDto gitHubInstall = db.alm().insertAlmAppInstall(a -> a.setAlm(GITHUB)); db.alm().insertOrganizationAlmBinding(organization, gitHubInstall, true); UserDto user = db.users().insertDisabledUser(u -> u.setLogin(USER_LOGIN)); @@ -246,7 +246,7 @@ public class UserRegistrarImplOrgMembershipSyncTest { .setSource(Source.local(BASIC)) .setExistingEmailStrategy(ExistingEmailStrategy.FORBID) .setUpdateLoginStrategy(UpdateLoginStrategy.ALLOW) - .setOrganizationAlmIds(ImmutableSet.of(gitHubInstall.getOwnerId())) + .setOrganizationAlmIds(ImmutableSet.of(gitHubInstall.getOrganizationAlmId())) .build()); db.organizations().assertUserIsMemberOfOrganization(organization, user); diff --git a/server/sonar-server/src/test/java/org/sonar/server/organization/MemberUpdaterTest.java b/server/sonar-server/src/test/java/org/sonar/server/organization/MemberUpdaterTest.java index b9f11ab1047..157981454a4 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/organization/MemberUpdaterTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/organization/MemberUpdaterTest.java @@ -404,15 +404,15 @@ public class MemberUpdaterTest { public void synchronize_user_organization_membership() { OrganizationDto organization1 = db.organizations().insert(); GroupDto org1defaultGroup = db.users().insertDefaultGroup(organization1, "Members"); - AlmAppInstallDto gitHubInstall1 = db.alm().insertAlmAppInstall(a -> a.setAlmId(GITHUB.getId())); + AlmAppInstallDto gitHubInstall1 = db.alm().insertAlmAppInstall(a -> a.setAlm(GITHUB)); db.alm().insertOrganizationAlmBinding(organization1, gitHubInstall1, true); OrganizationDto organization2 = db.organizations().insert(); db.users().insertDefaultGroup(organization2, "Members"); - AlmAppInstallDto gitHubInstall2 = db.alm().insertAlmAppInstall(a -> a.setAlmId(GITHUB.getId())); + AlmAppInstallDto gitHubInstall2 = db.alm().insertAlmAppInstall(a -> a.setAlm(GITHUB)); db.alm().insertOrganizationAlmBinding(organization2, gitHubInstall2, true); OrganizationDto organization3 = db.organizations().insert(); GroupDto org3defaultGroup = db.users().insertDefaultGroup(organization3, "Members"); - AlmAppInstallDto gitHubInstall3 = db.alm().insertAlmAppInstall(a -> a.setAlmId(GITHUB.getId())); + AlmAppInstallDto gitHubInstall3 = db.alm().insertAlmAppInstall(a -> a.setAlm(GITHUB)); db.alm().insertOrganizationAlmBinding(organization3, gitHubInstall3, true); // User is member of organization1 and organization3, but organization3 membership will be removed and organization2 membership will be // added @@ -422,7 +422,7 @@ public class MemberUpdaterTest { db.organizations().addMember(organization3, user); db.users().insertMember(org3defaultGroup, user); - underTest.synchronizeUserOrganizationMembership(db.getSession(), user, GITHUB, ImmutableSet.of(gitHubInstall1.getOwnerId(), gitHubInstall2.getOwnerId())); + underTest.synchronizeUserOrganizationMembership(db.getSession(), user, GITHUB, ImmutableSet.of(gitHubInstall1.getOrganizationAlmId(), gitHubInstall2.getOrganizationAlmId())); db.organizations().assertUserIsMemberOfOrganization(organization1, user); db.organizations().assertUserIsMemberOfOrganization(organization2, user); @@ -433,11 +433,11 @@ public class MemberUpdaterTest { public void synchronize_user_organization_membership_does_not_update_es_index() { OrganizationDto organization = db.organizations().insert(); db.users().insertDefaultGroup(organization, "Members"); - AlmAppInstallDto gitHubInstall = db.alm().insertAlmAppInstall(a -> a.setAlmId(GITHUB.getId())); + AlmAppInstallDto gitHubInstall = db.alm().insertAlmAppInstall(a -> a.setAlm(GITHUB)); db.alm().insertOrganizationAlmBinding(organization, gitHubInstall, true); UserDto user = db.users().insertUser(); - underTest.synchronizeUserOrganizationMembership(db.getSession(), user, GITHUB, ImmutableSet.of(gitHubInstall.getOwnerId())); + underTest.synchronizeUserOrganizationMembership(db.getSession(), user, GITHUB, ImmutableSet.of(gitHubInstall.getOrganizationAlmId())); assertThat(userIndex.search(UserQuery.builder().build(), new SearchOptions()).getDocs()).isEmpty(); } @@ -446,7 +446,7 @@ public class MemberUpdaterTest { public void synchronize_user_organization_membership_ignores_organization_alm_ids_match_no_existing_organizations() { OrganizationDto organization = db.organizations().insert(); db.users().insertDefaultGroup(organization, "Members"); - AlmAppInstallDto gitHubInstall = db.alm().insertAlmAppInstall(a -> a.setAlmId(GITHUB.getId())); + AlmAppInstallDto gitHubInstall = db.alm().insertAlmAppInstall(a -> a.setAlm(GITHUB)); db.alm().insertOrganizationAlmBinding(organization, gitHubInstall, true); UserDto user = db.users().insertUser(); @@ -460,11 +460,11 @@ public class MemberUpdaterTest { public void synchronize_user_organization_membership_ignores_organization_with_member_sync_disabled() { OrganizationDto organization = db.organizations().insert(); db.users().insertDefaultGroup(organization, "Members"); - AlmAppInstallDto gitHubInstall = db.alm().insertAlmAppInstall(a -> a.setAlmId(GITHUB.getId())); + AlmAppInstallDto gitHubInstall = db.alm().insertAlmAppInstall(a -> a.setAlm(GITHUB)); db.alm().insertOrganizationAlmBinding(organization, gitHubInstall, false); UserDto user = db.users().insertUser(); - underTest.synchronizeUserOrganizationMembership(db.getSession(), user, GITHUB, ImmutableSet.of(gitHubInstall.getOwnerId())); + underTest.synchronizeUserOrganizationMembership(db.getSession(), user, GITHUB, ImmutableSet.of(gitHubInstall.getOrganizationAlmId())); db.organizations().assertUserIsNotMemberOfOrganization(organization, user); } @@ -473,7 +473,7 @@ public class MemberUpdaterTest { public void synchronize_user_organization_membership_does_not_remove_existing_membership_on_organization_with_member_sync_disabled() { OrganizationDto organization = db.organizations().insert(); GroupDto org1defaultGroup = db.users().insertDefaultGroup(organization, "Members"); - AlmAppInstallDto gitHubInstall = db.alm().insertAlmAppInstall(a -> a.setAlmId(GITHUB.getId())); + AlmAppInstallDto gitHubInstall = db.alm().insertAlmAppInstall(a -> a.setAlm(GITHUB)); db.alm().insertOrganizationAlmBinding(organization, gitHubInstall, false); UserDto user = db.users().insertUser(); db.users().insertMember(org1defaultGroup, user); -- 2.39.5