From 6ba838b1da5fdd77748f08953711fb554cbf80e1 Mon Sep 17 00:00:00 2001 From: Duarte Meneses Date: Wed, 16 Sep 2020 11:38:28 -0500 Subject: [PATCH] SONAR-13450 Deprecate 'id' parameter in `api/user_groups/update` --- ...ProjectAnalysisTaskContainerPopulator.java | 4 +- .../DefaultOrganizationLoader.java | 45 ---------- .../organization/package-info.java | 23 ----- .../DefaultOrganizationLoaderTest.java | 48 ---------- .../DefaultOrganizationCache.java | 33 ------- .../DefaultOrganizationProviderImpl.java | 37 ++------ .../DefaultOrganizationProviderImplTest.java | 83 ----------------- .../NoopDefaultOrganizationCache.java | 37 -------- .../server/usergroups/ws/UpdateAction.java | 44 +++++++--- .../usergroups/ws/UpdateActionTest.java | 88 +++++++++++++++---- .../platformlevel/PlatformLevelSafeMode.java | 5 +- .../platform/web/UserSessionFilter.java | 18 ++-- .../platform/web/UserSessionFilterTest.java | 84 ++---------------- 13 files changed, 125 insertions(+), 424 deletions(-) delete mode 100644 server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/organization/DefaultOrganizationLoader.java delete mode 100644 server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/organization/package-info.java delete mode 100644 server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/organization/DefaultOrganizationLoaderTest.java delete mode 100644 server/sonar-server-common/src/main/java/org/sonar/server/organization/DefaultOrganizationCache.java delete mode 100644 server/sonar-webserver-auth/src/main/java/org/sonar/server/organization/NoopDefaultOrganizationCache.java diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/container/ProjectAnalysisTaskContainerPopulator.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/container/ProjectAnalysisTaskContainerPopulator.java index 7cd9fc307c3..73c3b4db8f3 100644 --- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/container/ProjectAnalysisTaskContainerPopulator.java +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/container/ProjectAnalysisTaskContainerPopulator.java @@ -60,7 +60,6 @@ import org.sonar.ce.task.projectanalysis.issue.DefaultAssignee; import org.sonar.ce.task.projectanalysis.issue.EffortAggregator; import org.sonar.ce.task.projectanalysis.issue.IntegrateIssuesVisitor; import org.sonar.ce.task.projectanalysis.issue.IssueAssigner; -import org.sonar.ce.task.projectanalysis.issue.ProtoIssueCache; import org.sonar.ce.task.projectanalysis.issue.IssueCounter; import org.sonar.ce.task.projectanalysis.issue.IssueCreationDateCalculator; import org.sonar.ce.task.projectanalysis.issue.IssueLifecycle; @@ -70,6 +69,7 @@ import org.sonar.ce.task.projectanalysis.issue.IssuesRepositoryVisitor; import org.sonar.ce.task.projectanalysis.issue.LoadComponentUuidsHavingOpenIssuesVisitor; import org.sonar.ce.task.projectanalysis.issue.MovedIssueVisitor; import org.sonar.ce.task.projectanalysis.issue.NewEffortAggregator; +import org.sonar.ce.task.projectanalysis.issue.ProtoIssueCache; import org.sonar.ce.task.projectanalysis.issue.PullRequestTrackerExecution; import org.sonar.ce.task.projectanalysis.issue.ReferenceBranchTrackerExecution; import org.sonar.ce.task.projectanalysis.issue.RemoveProcessedComponentsVisitor; @@ -99,7 +99,6 @@ import org.sonar.ce.task.projectanalysis.measure.MeasureRepositoryImpl; import org.sonar.ce.task.projectanalysis.measure.MeasureToMeasureDto; import org.sonar.ce.task.projectanalysis.metric.MetricModule; import org.sonar.ce.task.projectanalysis.notification.NotificationFactory; -import org.sonar.ce.task.projectanalysis.organization.DefaultOrganizationLoader; import org.sonar.ce.task.projectanalysis.period.NewCodePeriodResolver; import org.sonar.ce.task.projectanalysis.period.PeriodHolderImpl; import org.sonar.ce.task.projectanalysis.qualitygate.EvaluationResultTextConverterImpl; @@ -155,7 +154,6 @@ public final class ProjectAnalysisTaskContainerPopulator implements ContainerPop public void populateContainer(TaskContainer container) { ComputationSteps steps = new ReportComputationSteps(container); container.add(SettingsLoader.class); - container.add(DefaultOrganizationLoader.class); container.add(task); container.add(steps); container.addSingletons(componentClasses()); diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/organization/DefaultOrganizationLoader.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/organization/DefaultOrganizationLoader.java deleted file mode 100644 index 71962f9174a..00000000000 --- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/organization/DefaultOrganizationLoader.java +++ /dev/null @@ -1,45 +0,0 @@ -/* - * SonarQube - * Copyright (C) 2009-2020 SonarSource SA - * mailto:info AT sonarsource DOT com - * - * This program is free software; you can redistribute it and/or - * modify it under the terms of the GNU Lesser General Public - * License as published by the Free Software Foundation; either - * version 3 of the License, or (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - * Lesser General Public License for more details. - * - * You should have received a copy of the GNU Lesser General Public License - * along with this program; if not, write to the Free Software Foundation, - * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. - */ -package org.sonar.ce.task.projectanalysis.organization; - -import org.picocontainer.Startable; -import org.sonar.api.ce.ComputeEngineSide; -import org.sonar.ce.task.container.EagerStart; -import org.sonar.server.organization.DefaultOrganizationCache; - -@EagerStart -@ComputeEngineSide -public class DefaultOrganizationLoader implements Startable { - private final DefaultOrganizationCache defaultOrganizationCache; - - public DefaultOrganizationLoader(DefaultOrganizationCache defaultOrganizationCache) { - this.defaultOrganizationCache = defaultOrganizationCache; - } - - @Override - public void start() { - defaultOrganizationCache.load(); - } - - @Override - public void stop() { - defaultOrganizationCache.unload(); - } -} diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/organization/package-info.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/organization/package-info.java deleted file mode 100644 index ca7f2753dfb..00000000000 --- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/organization/package-info.java +++ /dev/null @@ -1,23 +0,0 @@ -/* - * SonarQube - * Copyright (C) 2009-2020 SonarSource SA - * mailto:info AT sonarsource DOT com - * - * This program is free software; you can redistribute it and/or - * modify it under the terms of the GNU Lesser General Public - * License as published by the Free Software Foundation; either - * version 3 of the License, or (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - * Lesser General Public License for more details. - * - * You should have received a copy of the GNU Lesser General Public License - * along with this program; if not, write to the Free Software Foundation, - * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. - */ -@ParametersAreNonnullByDefault -package org.sonar.ce.task.projectanalysis.organization; - -import javax.annotation.ParametersAreNonnullByDefault; diff --git a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/organization/DefaultOrganizationLoaderTest.java b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/organization/DefaultOrganizationLoaderTest.java deleted file mode 100644 index 652574929e3..00000000000 --- a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/organization/DefaultOrganizationLoaderTest.java +++ /dev/null @@ -1,48 +0,0 @@ -/* - * SonarQube - * Copyright (C) 2009-2020 SonarSource SA - * mailto:info AT sonarsource DOT com - * - * This program is free software; you can redistribute it and/or - * modify it under the terms of the GNU Lesser General Public - * License as published by the Free Software Foundation; either - * version 3 of the License, or (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - * Lesser General Public License for more details. - * - * You should have received a copy of the GNU Lesser General Public License - * along with this program; if not, write to the Free Software Foundation, - * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. - */ -package org.sonar.ce.task.projectanalysis.organization; - -import org.junit.Test; -import org.sonar.server.organization.DefaultOrganizationCache; - -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.verifyNoMoreInteractions; - -public class DefaultOrganizationLoaderTest { - private DefaultOrganizationCache defaultOrganizationCache = mock(DefaultOrganizationCache.class); - private DefaultOrganizationLoader underTest = new DefaultOrganizationLoader(defaultOrganizationCache); - - @Test - public void start_calls_cache_load_method() { - underTest.start(); - - verify(defaultOrganizationCache).load(); - verifyNoMoreInteractions(defaultOrganizationCache); - } - - @Test - public void stop_calls_cache_unload_method() { - underTest.stop(); - - verify(defaultOrganizationCache).unload(); - verifyNoMoreInteractions(defaultOrganizationCache); - } -} diff --git a/server/sonar-server-common/src/main/java/org/sonar/server/organization/DefaultOrganizationCache.java b/server/sonar-server-common/src/main/java/org/sonar/server/organization/DefaultOrganizationCache.java deleted file mode 100644 index 53c7476341c..00000000000 --- a/server/sonar-server-common/src/main/java/org/sonar/server/organization/DefaultOrganizationCache.java +++ /dev/null @@ -1,33 +0,0 @@ -/* - * SonarQube - * Copyright (C) 2009-2020 SonarSource SA - * mailto:info AT sonarsource DOT com - * - * This program is free software; you can redistribute it and/or - * modify it under the terms of the GNU Lesser General Public - * License as published by the Free Software Foundation; either - * version 3 of the License, or (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - * Lesser General Public License for more details. - * - * You should have received a copy of the GNU Lesser General Public License - * along with this program; if not, write to the Free Software Foundation, - * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. - */ -package org.sonar.server.organization; - -public interface DefaultOrganizationCache { - - /** - * Loads {@link DefaultOrganization} in cache. - */ - void load(); - - /** - * Unloads {@link DefaultOrganization} from cache. - */ - void unload(); -} diff --git a/server/sonar-server-common/src/main/java/org/sonar/server/organization/DefaultOrganizationProviderImpl.java b/server/sonar-server-common/src/main/java/org/sonar/server/organization/DefaultOrganizationProviderImpl.java index 448d352993a..3b958ef838b 100644 --- a/server/sonar-server-common/src/main/java/org/sonar/server/organization/DefaultOrganizationProviderImpl.java +++ b/server/sonar-server-common/src/main/java/org/sonar/server/organization/DefaultOrganizationProviderImpl.java @@ -20,8 +20,6 @@ package org.sonar.server.organization; import java.util.Optional; -import java.util.function.Supplier; -import javax.annotation.CheckForNull; import org.sonar.db.DbClient; import org.sonar.db.DbSession; import org.sonar.db.organization.OrganizationDto; @@ -29,10 +27,9 @@ import org.sonar.server.property.InternalProperties; import static com.google.common.base.Preconditions.checkState; -public class DefaultOrganizationProviderImpl implements DefaultOrganizationProvider, DefaultOrganizationCache { - private static final ThreadLocal CACHE = new ThreadLocal<>(); - +public class DefaultOrganizationProviderImpl implements DefaultOrganizationProvider { private final DbClient dbClient; + private DefaultOrganization cache; public DefaultOrganizationProviderImpl(DbClient dbClient) { this.dbClient = dbClient; @@ -40,12 +37,11 @@ public class DefaultOrganizationProviderImpl implements DefaultOrganizationProvi @Override public DefaultOrganization get() { - Cache cache = CACHE.get(); - if (cache != null) { - return cache.get(() -> getDefaultOrganization(dbClient)); + if (cache == null) { + cache = getDefaultOrganization(dbClient); } - return getDefaultOrganization(dbClient); + return cache; } private static DefaultOrganization getDefaultOrganization(DbClient dbClient) { @@ -67,27 +63,4 @@ public class DefaultOrganizationProviderImpl implements DefaultOrganizationProvi .setUpdatedAt(organizationDto.getUpdatedAt()) .build(); } - - @Override - public void load() { - CACHE.set(new Cache()); - } - - @Override - public void unload() { - CACHE.remove(); - } - - private static final class Cache { - @CheckForNull - private DefaultOrganization defaultOrganization; - - public DefaultOrganization get(Supplier supplier) { - if (defaultOrganization == null) { - defaultOrganization = supplier.get(); - } - return defaultOrganization; - } - - } } diff --git a/server/sonar-server-common/src/test/java/org/sonar/server/organization/DefaultOrganizationProviderImplTest.java b/server/sonar-server-common/src/test/java/org/sonar/server/organization/DefaultOrganizationProviderImplTest.java index 3c5d2b79c03..a7034d31356 100644 --- a/server/sonar-server-common/src/test/java/org/sonar/server/organization/DefaultOrganizationProviderImplTest.java +++ b/server/sonar-server-common/src/test/java/org/sonar/server/organization/DefaultOrganizationProviderImplTest.java @@ -19,7 +19,6 @@ */ package org.sonar.server.organization; -import org.junit.After; import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; @@ -54,11 +53,6 @@ public class DefaultOrganizationProviderImplTest { private DefaultOrganizationProviderImpl underTest = new DefaultOrganizationProviderImpl(dbClient); - @After - public void tearDown() { - underTest.unload(); - } - @Test public void get_fails_with_ISE_if_default_organization_internal_property_does_not_exist() { expectISENoDefaultOrganizationUuid(); @@ -106,83 +100,6 @@ public class DefaultOrganizationProviderImplTest { assertThat(defaultOrganization.getUpdatedAt()).isEqualTo(DATE_1); } - @Test - public void get_returns_new_DefaultOrganization_with_each_call_when_cache_is_not_loaded() { - insertOrganization(ORGANIZATION_DTO_1, DATE_1); - dbClient.internalPropertiesDao().save(dbSession, DEFAULT_ORGANIZATION, ORGANIZATION_DTO_1.getUuid()); - dbSession.commit(); - - assertThat(underTest.get()).isNotSameAs(underTest.get()); - } - - @Test - public void unload_does_not_fail_if_load_has_not_been_called() { - underTest.unload(); - } - - @Test - public void load_resets_thread_local_when_called_twice() { - insertOrganization(ORGANIZATION_DTO_1, DATE_1); - dbClient.internalPropertiesDao().save(dbSession, DEFAULT_ORGANIZATION, ORGANIZATION_DTO_1.getUuid()); - dbSession.commit(); - - underTest.load(); - DefaultOrganization org1 = underTest.get(); - - underTest.load(); - DefaultOrganization org2 = underTest.get(); - assertThat(org1).isNotSameAs(org2); - } - - @Test - public void load_and_unload_cache_DefaultOrganization_object_by_thread() throws InterruptedException { - insertOrganization(ORGANIZATION_DTO_1, DATE_1); - dbClient.internalPropertiesDao().save(dbSession, DEFAULT_ORGANIZATION, ORGANIZATION_DTO_1.getUuid()); - dbSession.commit(); - - try { - underTest.load(); - - DefaultOrganization cachedForThread1 = underTest.get(); - assertThat(cachedForThread1).isSameAs(underTest.get()); - - Thread otherThread = new Thread(() -> { - try { - underTest.load(); - - assertThat(underTest.get()) - .isNotSameAs(cachedForThread1) - .isSameAs(underTest.get()); - } finally { - underTest.unload(); - } - }); - otherThread.start(); - otherThread.join(); - } finally { - underTest.unload(); - } - } - - @Test - public void get_returns_new_instance_for_each_call_once_unload_has_been_called() { - insertOrganization(ORGANIZATION_DTO_1, DATE_1); - dbClient.internalPropertiesDao().save(dbSession, DEFAULT_ORGANIZATION, ORGANIZATION_DTO_1.getUuid()); - dbSession.commit(); - - try { - underTest.load(); - DefaultOrganization cached = underTest.get(); - assertThat(cached).isSameAs(underTest.get()); - - underTest.unload(); - assertThat(underTest.get()).isNotSameAs(underTest.get()).isNotSameAs(cached); - } finally { - // fail safe - underTest.unload(); - } - } - private void insertOrganization(OrganizationDto dto, long createdAt) { when(system2.now()).thenReturn(createdAt); dbClient.organizationDao().insert(dbSession, dto, false); diff --git a/server/sonar-webserver-auth/src/main/java/org/sonar/server/organization/NoopDefaultOrganizationCache.java b/server/sonar-webserver-auth/src/main/java/org/sonar/server/organization/NoopDefaultOrganizationCache.java deleted file mode 100644 index 103e6326bee..00000000000 --- a/server/sonar-webserver-auth/src/main/java/org/sonar/server/organization/NoopDefaultOrganizationCache.java +++ /dev/null @@ -1,37 +0,0 @@ -/* - * SonarQube - * Copyright (C) 2009-2020 SonarSource SA - * mailto:info AT sonarsource DOT com - * - * This program is free software; you can redistribute it and/or - * modify it under the terms of the GNU Lesser General Public - * License as published by the Free Software Foundation; either - * version 3 of the License, or (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - * Lesser General Public License for more details. - * - * You should have received a copy of the GNU Lesser General Public License - * along with this program; if not, write to the Free Software Foundation, - * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. - */ -package org.sonar.server.organization; - -/** - * This implementation of {@link DefaultOrganizationCache} has no effect. It is used when SQ is running in safe mode - * (ie. when we are expecting a DB upgrade and we can't assume the tables storing default organization information are - * available). - */ -public class NoopDefaultOrganizationCache implements DefaultOrganizationCache { - @Override - public void load() { - // do nothing - } - - @Override - public void unload() { - // do nothing - } -} diff --git a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/usergroups/ws/UpdateAction.java b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/usergroups/ws/UpdateAction.java index 6fc48fcf1b3..609020eb44b 100644 --- a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/usergroups/ws/UpdateAction.java +++ b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/usergroups/ws/UpdateAction.java @@ -31,6 +31,7 @@ import org.sonar.db.DbSession; import org.sonar.db.organization.OrganizationDto; import org.sonar.db.user.GroupDto; import org.sonar.db.user.UserMembershipQuery; +import org.sonar.server.organization.DefaultOrganizationProvider; import org.sonar.server.user.UserSession; import org.sonarqube.ws.UserGroups; @@ -40,6 +41,7 @@ import static org.sonar.core.util.Uuids.UUID_EXAMPLE_01; import static org.sonar.db.permission.OrganizationPermission.ADMINISTER; import static org.sonar.server.exceptions.NotFoundException.checkFound; import static org.sonar.server.exceptions.NotFoundException.checkFoundWithOptional; +import static org.sonar.server.qualitygate.ws.QualityGatesWsParameters.PARAM_CURRENT_NAME; import static org.sonar.server.usergroups.ws.GroupWsSupport.DESCRIPTION_MAX_LENGTH; import static org.sonar.server.usergroups.ws.GroupWsSupport.PARAM_GROUP_DESCRIPTION; import static org.sonar.server.usergroups.ws.GroupWsSupport.PARAM_GROUP_ID; @@ -52,11 +54,13 @@ public class UpdateAction implements UserGroupsWsAction { private final DbClient dbClient; private final UserSession userSession; private final GroupWsSupport support; + private final DefaultOrganizationProvider defaultOrganizationProvider; - public UpdateAction(DbClient dbClient, UserSession userSession, GroupWsSupport support) { + public UpdateAction(DbClient dbClient, UserSession userSession, GroupWsSupport support, DefaultOrganizationProvider defaultOrganizationProvider) { this.dbClient = dbClient; this.userSession = userSession; this.support = support; + this.defaultOrganizationProvider = defaultOrganizationProvider; } @Override @@ -69,13 +73,19 @@ public class UpdateAction implements UserGroupsWsAction { .setResponseExample(getClass().getResource("update.example.json")) .setSince("5.2") .setChangelog( + new Change("8.5", "Parameter 'id' deprecated in favor of 'currentName'"), new Change("8.4", "Parameter 'id' format changes from integer to string"), new Change("6.4", "The default group is no longer editable")); action.createParam(PARAM_GROUP_ID) - .setDescription("Identifier of the group.") + .setDescription("Identifier of the group. Use '" + PARAM_CURRENT_NAME + "' instead.") .setExampleValue(UUID_EXAMPLE_01) - .setRequired(true); + .setDeprecatedSince("8.5"); + + action.createParam(PARAM_CURRENT_NAME) + .setDescription("Name of the group to be updated. Mandatory unless '" + PARAM_GROUP_ID + "' is used.") + .setExampleValue(UUID_EXAMPLE_01) + .setSince("8.5"); action.createParam(PARAM_GROUP_NAME) .setMaximumLength(GROUP_NAME_MAX_LENGTH) @@ -93,12 +103,26 @@ public class UpdateAction implements UserGroupsWsAction { @Override public void handle(Request request, Response response) throws Exception { try (DbSession dbSession = dbClient.openSession(false)) { - String groupUuid = request.mandatoryParam(PARAM_GROUP_ID); - GroupDto group = dbClient.groupDao().selectByUuid(dbSession, groupUuid); - checkFound(group, "Could not find a user group with id '%s'.", groupUuid); - Optional org = dbClient.organizationDao().selectByUuid(dbSession, group.getOrganizationUuid()); - checkFoundWithOptional(org, "Could not find organization with id '%s'.", group.getOrganizationUuid()); - userSession.checkPermission(ADMINISTER, org.get()); + String groupUuid = request.param(PARAM_GROUP_ID); + String currentName = request.param(PARAM_CURRENT_NAME); + + if ((groupUuid == null) == (currentName == null)) { + throw new IllegalArgumentException(format("Need to specify one and only one of '%s' or '%s'", PARAM_GROUP_ID, PARAM_CURRENT_NAME)); + } + + GroupDto group; + if (groupUuid != null) { + group = dbClient.groupDao().selectByUuid(dbSession, groupUuid); + checkFound(group, "Could not find a user group with id '%s'.", groupUuid); + } else { + group = dbClient.groupDao().selectByName(dbSession, defaultOrganizationProvider.get().getUuid(), currentName).orElse(null); + checkFound(group, "Could not find a user group with name '%s'.", currentName); + } + + Optional orgOpt = dbClient.organizationDao().selectByUuid(dbSession, group.getOrganizationUuid()); + checkFoundWithOptional(orgOpt, "Could not find organization with id '%s'.", group.getOrganizationUuid()); + + userSession.checkPermission(ADMINISTER, orgOpt.get()); support.checkGroupIsNotDefault(dbSession, group); boolean changed = false; @@ -121,7 +145,7 @@ public class UpdateAction implements UserGroupsWsAction { dbSession.commit(); } - writeResponse(dbSession, request, response, org.get(), group); + writeResponse(dbSession, request, response, orgOpt.get(), group); } } diff --git a/server/sonar-webserver-webapi/src/test/java/org/sonar/server/usergroups/ws/UpdateActionTest.java b/server/sonar-webserver-webapi/src/test/java/org/sonar/server/usergroups/ws/UpdateActionTest.java index ef6d51481b9..e10ee418c16 100644 --- a/server/sonar-webserver-webapi/src/test/java/org/sonar/server/usergroups/ws/UpdateActionTest.java +++ b/server/sonar-webserver-webapi/src/test/java/org/sonar/server/usergroups/ws/UpdateActionTest.java @@ -39,7 +39,6 @@ import org.sonar.server.ws.TestRequest; import org.sonar.server.ws.WsActionTester; import static org.assertj.core.api.Assertions.assertThat; -import static org.assertj.core.api.Assertions.tuple; import static org.sonar.db.permission.OrganizationPermission.ADMINISTER; import static org.sonar.test.JsonAssert.assertJson; @@ -54,7 +53,8 @@ public class UpdateActionTest { private TestDefaultOrganizationProvider defaultOrganizationProvider = TestDefaultOrganizationProvider.from(db); private WsActionTester ws = new WsActionTester( - new UpdateAction(db.getDbClient(), userSession, new GroupWsSupport(db.getDbClient(), defaultOrganizationProvider, new DefaultGroupFinder(db.getDbClient())))); + new UpdateAction(db.getDbClient(), userSession, new GroupWsSupport(db.getDbClient(), defaultOrganizationProvider, new DefaultGroupFinder(db.getDbClient())), + defaultOrganizationProvider)); @Test public void verify_definition() { @@ -63,9 +63,7 @@ public class UpdateActionTest { assertThat(wsDef.isInternal()).isEqualTo(false); assertThat(wsDef.since()).isEqualTo("5.2"); assertThat(wsDef.isPost()).isEqualTo(true); - assertThat(wsDef.changelog()).extracting(Change::getVersion, Change::getDescription).containsOnly( - tuple("8.4", "Parameter 'id' format changes from integer to string"), - tuple("6.4", "The default group is no longer editable")); + assertThat(wsDef.changelog()).extracting(Change::getVersion, Change::getDescription).isNotEmpty(); } @Test @@ -78,7 +76,7 @@ public class UpdateActionTest { loginAsAdminOnDefaultOrganization(); String result = newRequest() - .setParam("id", group.getUuid().toString()) + .setParam("id", group.getUuid()) .setParam("name", "new-name") .setParam("description", "New Description") .execute().getInput(); @@ -99,7 +97,27 @@ public class UpdateActionTest { loginAsAdminOnDefaultOrganization(); String result = newRequest() - .setParam("id", group.getUuid().toString()) + .setParam("id", group.getUuid()) + .setParam("name", "new-name") + .execute().getInput(); + + assertJson(result).isSimilarTo("{" + + " \"group\": {" + + " \"name\": \"new-name\"," + + " \"description\": \"" + group.getDescription() + "\"," + + " \"membersCount\": 0" + + " }" + + "}"); + } + + @Test + public void update_only_name_by_name() { + insertDefaultGroupOnDefaultOrganization(); + GroupDto group = db.users().insertGroup(); + loginAsAdminOnDefaultOrganization(); + + String result = newRequest() + .setParam("currentName", group.getName()) .setParam("name", "new-name") .execute().getInput(); @@ -119,7 +137,7 @@ public class UpdateActionTest { loginAsAdminOnDefaultOrganization(); String result = newRequest() - .setParam("id", group.getUuid().toString()) + .setParam("id", group.getUuid()) .setParam("description", "New Description") .execute().getInput(); @@ -139,7 +157,7 @@ public class UpdateActionTest { loginAsAdminOnDefaultOrganization(); String result = newRequest() - .setParam("id", group.getUuid().toString()) + .setParam("id", group.getUuid()) .setParam("name", "new-name") .execute().getInput(); @@ -162,7 +180,7 @@ public class UpdateActionTest { expectedException.expect(ForbiddenException.class); newRequest() - .setParam("id", group.getUuid().toString()) + .setParam("id", group.getUuid()) .setParam("name", "some-product-bu") .setParam("description", "Business Unit for Some Awesome Product") .execute(); @@ -180,7 +198,7 @@ public class UpdateActionTest { expectedException.expect(ForbiddenException.class); newRequest() - .setParam("id", group.getUuid().toString()) + .setParam("id", group.getUuid()) .setParam("name", "some-product-bu") .setParam("description", "Business Unit for Some Awesome Product") .execute(); @@ -196,11 +214,37 @@ public class UpdateActionTest { expectedException.expectMessage("Group name cannot be empty"); newRequest() - .setParam("id", group.getUuid().toString()) + .setParam("id", group.getUuid()) .setParam("name", "") .execute(); } + + @Test + public void fail_if_no_id_and_no_currentname_are_provided() { + insertDefaultGroupOnDefaultOrganization(); + + expectedException.expect(IllegalArgumentException.class); + expectedException.expectMessage("Need to specify one and only one of 'id' or 'currentName'"); + + newRequest() + .setParam("name", "newname") + .execute(); + } + + @Test + public void fail_if_both_id_and_currentname_are_provided() { + insertDefaultGroupOnDefaultOrganization(); + + expectedException.expect(IllegalArgumentException.class); + expectedException.expectMessage("Need to specify one and only one of 'id' or 'currentName'"); + + newRequest() + .setParam("id", "id") + .setParam("currentName", "name") + .execute(); + } + @Test public void fail_if_new_name_is_anyone() { insertDefaultGroupOnDefaultOrganization(); @@ -211,7 +255,7 @@ public class UpdateActionTest { expectedException.expectMessage("Anyone group cannot be used"); newRequest() - .setParam("id", group.getUuid().toString()) + .setParam("id", group.getUuid()) .setParam("name", "AnYoNe") .execute(); } @@ -229,7 +273,7 @@ public class UpdateActionTest { expectedException.expectMessage("Group 'new-name' already exists"); newRequest() - .setParam("id", groupToBeRenamed.getUuid().toString()) + .setParam("id", groupToBeRenamed.getUuid()) .setParam("name", newName) .execute(); } @@ -246,6 +290,18 @@ public class UpdateActionTest { .execute(); } + @Test + public void fail_if_unknown_group_name() { + loginAsAdminOnDefaultOrganization(); + + expectedException.expect(NotFoundException.class); + expectedException.expectMessage("Could not find a user group with name '42'."); + + newRequest() + .setParam("currentName", "42") + .execute(); + } + @Test public void fail_to_update_default_group_name() { GroupDto group = db.users().insertDefaultGroup(db.getDefaultOrganization(), "default"); @@ -255,7 +311,7 @@ public class UpdateActionTest { expectedException.expectMessage("Default group 'default' cannot be used to perform this action"); newRequest() - .setParam("id", group.getUuid().toString()) + .setParam("id", group.getUuid()) .setParam("name", "new name") .execute(); } @@ -269,7 +325,7 @@ public class UpdateActionTest { expectedException.expectMessage("Default group 'default' cannot be used to perform this action"); newRequest() - .setParam("id", group.getUuid().toString()) + .setParam("id", group.getUuid()) .setParam("description", "new description") .execute(); } diff --git a/server/sonar-webserver/src/main/java/org/sonar/server/platform/platformlevel/PlatformLevelSafeMode.java b/server/sonar-webserver/src/main/java/org/sonar/server/platform/platformlevel/PlatformLevelSafeMode.java index 15c2bb9a161..830da6ef24a 100644 --- a/server/sonar-webserver/src/main/java/org/sonar/server/platform/platformlevel/PlatformLevelSafeMode.java +++ b/server/sonar-webserver/src/main/java/org/sonar/server/platform/platformlevel/PlatformLevelSafeMode.java @@ -20,7 +20,6 @@ package org.sonar.server.platform.platformlevel; import org.sonar.server.authentication.SafeModeUserSession; -import org.sonar.server.organization.NoopDefaultOrganizationCache; import org.sonar.server.platform.ServerImpl; import org.sonar.server.platform.db.migration.AutoDbMigration; import org.sonar.server.platform.db.migration.DatabaseMigrationImpl; @@ -58,9 +57,7 @@ public class PlatformLevelSafeMode extends PlatformLevel { // WS engine SafeModeUserSession.class, WebServiceEngine.class, - WebServiceFilter.class, - - NoopDefaultOrganizationCache.class); + WebServiceFilter.class); addIfStartupLeader( DatabaseMigrationImpl.class, MigrationEngineModule.class, diff --git a/server/sonar-webserver/src/main/java/org/sonar/server/platform/web/UserSessionFilter.java b/server/sonar-webserver/src/main/java/org/sonar/server/platform/web/UserSessionFilter.java index 12a95caf60a..aa87a67963a 100644 --- a/server/sonar-webserver/src/main/java/org/sonar/server/platform/web/UserSessionFilter.java +++ b/server/sonar-webserver/src/main/java/org/sonar/server/platform/web/UserSessionFilter.java @@ -30,11 +30,10 @@ import javax.servlet.ServletRequest; import javax.servlet.ServletResponse; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; -import org.sonar.db.DBSessions; import org.sonar.api.utils.log.Logger; import org.sonar.api.utils.log.Loggers; +import org.sonar.db.DBSessions; import org.sonar.server.authentication.UserSessionInitializer; -import org.sonar.server.organization.DefaultOrganizationCache; import org.sonar.server.platform.Platform; import org.sonar.server.platform.PlatformImpl; import org.sonar.server.setting.ThreadLocalSettings; @@ -47,8 +46,7 @@ public class UserSessionFilter implements Filter { this.platform = PlatformImpl.getInstance(); } - @VisibleForTesting - UserSessionFilter(Platform platform) { + @VisibleForTesting UserSessionFilter(Platform platform) { this.platform = platform; } @@ -59,22 +57,16 @@ public class UserSessionFilter implements Filter { DBSessions dbSessions = platform.getContainer().getComponentByType(DBSessions.class); ThreadLocalSettings settings = platform.getContainer().getComponentByType(ThreadLocalSettings.class); - DefaultOrganizationCache defaultOrganizationCache = platform.getContainer().getComponentByType(DefaultOrganizationCache.class); UserSessionInitializer userSessionInitializer = platform.getContainer().getComponentByType(UserSessionInitializer.class); LOG.trace("{} serves {}", Thread.currentThread(), request.getRequestURI()); dbSessions.enableCaching(); try { - defaultOrganizationCache.load(); + settings.load(); try { - settings.load(); - try { - doFilter(request, response, chain, userSessionInitializer); - } finally { - settings.unload(); - } + doFilter(request, response, chain, userSessionInitializer); } finally { - defaultOrganizationCache.unload(); + settings.unload(); } } finally { dbSessions.disableCaching(); diff --git a/server/sonar-webserver/src/test/java/org/sonar/server/platform/web/UserSessionFilterTest.java b/server/sonar-webserver/src/test/java/org/sonar/server/platform/web/UserSessionFilterTest.java index dbdcc256e3a..aca65747ca3 100644 --- a/server/sonar-webserver/src/test/java/org/sonar/server/platform/web/UserSessionFilterTest.java +++ b/server/sonar-webserver/src/test/java/org/sonar/server/platform/web/UserSessionFilterTest.java @@ -31,7 +31,6 @@ import org.mockito.InOrder; import org.sonar.core.platform.ComponentContainer; import org.sonar.db.DBSessions; import org.sonar.server.authentication.UserSessionInitializer; -import org.sonar.server.organization.DefaultOrganizationCache; import org.sonar.server.platform.Platform; import org.sonar.server.setting.ThreadLocalSettings; @@ -42,7 +41,7 @@ import static org.mockito.Mockito.inOrder; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.verifyZeroInteractions; +import static org.mockito.Mockito.verifyNoInteractions; import static org.mockito.Mockito.when; public class UserSessionFilterTest { @@ -55,12 +54,11 @@ public class UserSessionFilterTest { private FilterChain chain = mock(FilterChain.class); private DBSessions dbSessions = mock(DBSessions.class); private ThreadLocalSettings settings = mock(ThreadLocalSettings.class); - private DefaultOrganizationCache defaultOrganizationCache = mock(DefaultOrganizationCache.class); private UserSessionFilter underTest = new UserSessionFilter(platform); @Before public void setUp() { - container.add(dbSessions, settings, defaultOrganizationCache); + container.add(dbSessions, settings); when(platform.getContainer()).thenReturn(container); } @@ -89,7 +87,7 @@ public class UserSessionFilterTest { underTest.doFilter(request, response, chain); verify(chain).doFilter(request, response); - verifyZeroInteractions(userSessionInitializer); + verifyNoInteractions(userSessionInitializer); } @Test @@ -138,22 +136,6 @@ public class UserSessionFilterTest { } } - @Test - public void doFilter_unloads_Settings_even_if_DefaultOrganizationCache_unload_fails() throws Exception { - RuntimeException thrown = new RuntimeException("Faking DefaultOrganizationCache.unload failing"); - doThrow(thrown) - .when(defaultOrganizationCache) - .unload(); - - try { - underTest.doFilter(request, response, chain); - fail("A RuntimeException should have been thrown"); - } catch (RuntimeException e) { - assertThat(e).isSameAs(thrown); - verify(settings).unload(); - } - } - @Test public void doFilter_unloads_Settings_even_if_UserSessionInitializer_removeUserSession_fails() throws Exception { RuntimeException thrown = mockUserSessionInitializerRemoveUserSessionFailing(); @@ -167,58 +149,6 @@ public class UserSessionFilterTest { } } - @Test - public void doFilter_loads_and_unloads_DefaultOrganizationCache() throws Exception { - underTest.doFilter(request, response, chain); - - InOrder inOrder = inOrder(defaultOrganizationCache); - inOrder.verify(defaultOrganizationCache).load(); - inOrder.verify(defaultOrganizationCache).unload(); - inOrder.verifyNoMoreInteractions(); - } - - @Test - public void doFilter_unloads_DefaultOrganizationCache_even_if_chain_throws_exception() throws Exception { - RuntimeException thrown = mockChainDoFilterError(); - - try { - underTest.doFilter(request, response, chain); - fail("A RuntimeException should have been thrown"); - } catch (RuntimeException e) { - assertThat(e).isSameAs(thrown); - verify(defaultOrganizationCache).unload(); - } - } - - @Test - public void doFilter_unloads_DefaultOrganizationCache_even_if_Settings_unload_fails() throws Exception { - RuntimeException thrown = new RuntimeException("Faking Settings.unload failing"); - doThrow(thrown) - .when(settings) - .unload(); - - try { - underTest.doFilter(request, response, chain); - fail("A RuntimeException should have been thrown"); - } catch (RuntimeException e) { - assertThat(e).isSameAs(thrown); - verify(defaultOrganizationCache).unload(); - } - } - - @Test - public void doFilter_unloads_DefaultOrganizationCache_even_if_UserSessionInitializer_removeUserSession_fails() throws Exception { - RuntimeException thrown = mockUserSessionInitializerRemoveUserSessionFailing(); - - try { - underTest.doFilter(request, response, chain); - fail("A RuntimeException should have been thrown"); - } catch (RuntimeException e) { - assertThat(e).isSameAs(thrown); - verify(defaultOrganizationCache).unload(); - } - } - @Test public void just_for_fun_and_coverage() { UserSessionFilter filter = new UserSessionFilter(); @@ -236,16 +166,16 @@ public class UserSessionFilterTest { container.add(userSessionInitializer); RuntimeException thrown = new RuntimeException("Faking UserSessionInitializer.removeUserSession failing"); doThrow(thrown) - .when(userSessionInitializer) - .removeUserSession(); + .when(userSessionInitializer) + .removeUserSession(); return thrown; } private RuntimeException mockChainDoFilterError() throws IOException, ServletException { RuntimeException thrown = new RuntimeException("Faking chain.doFilter failing"); doThrow(thrown) - .when(chain) - .doFilter(request, response); + .when(chain) + .doFilter(request, response); return thrown; } } -- 2.39.5