]> source.dussan.org Git - sonarqube.git/commitdiff
SONAR-9422 Bad error message when creating a project with '%' in key
authorJulien Lancelot <julien.lancelot@sonarsource.com>
Wed, 14 Jun 2017 08:08:58 +0000 (10:08 +0200)
committerJulien Lancelot <julien.lancelot@sonarsource.com>
Wed, 14 Jun 2017 12:43:10 +0000 (14:43 +0200)
server/sonar-server/src/main/java/org/sonar/server/component/ComponentUpdater.java
server/sonar-server/src/test/java/org/sonar/server/component/ComponentUpdaterTest.java
server/sonar-server/src/test/java/org/sonar/server/project/ws/CreateActionTest.java

index 632c646a8885e274e02e388216a45afb5e1f89c1..759033e7f204957c5379baceb38d55f5e7f517fa 100644 (file)
@@ -82,7 +82,7 @@ public class ComponentUpdater {
     checkBranchFormat(newComponent.qualifier(), newComponent.branch());
     String keyWithBranch = ComponentKeys.createKey(newComponent.key(), newComponent.branch());
     checkRequest(!dbClient.componentDao().selectByKey(session, keyWithBranch).isPresent(),
-      formatMessage("Could not create %s, key already exists: %s", newComponent.qualifier(), keyWithBranch));
+      "Could not create %s, key already exists: %s", getQualifierToDisplay(newComponent.qualifier()), keyWithBranch);
 
     String uuid = Uuids.create();
     ComponentDto component = new ComponentDto()
@@ -126,18 +126,18 @@ public class ComponentUpdater {
     }
   }
 
-  private void checkKeyFormat(String qualifier, String kee) {
-    checkRequest(isValidModuleKey(kee), formatMessage("Malformed key for %s: %s. Allowed characters are alphanumeric, '-', '_', '.' and ':', with at least one non-digit.",
-      qualifier, kee));
+  private void checkKeyFormat(String qualifier, String key) {
+    checkRequest(isValidModuleKey(key),
+      "Malformed key for %s: %s. Allowed characters are alphanumeric, '-', '_', '.' and ':', with at least one non-digit.", getQualifierToDisplay(qualifier), key);
   }
 
   private void checkBranchFormat(String qualifier, @Nullable String branch) {
     checkRequest(branch == null || ComponentKeys.isValidBranch(branch),
-      formatMessage("Malformed branch for %s: %s. Allowed characters are alphanumeric, '-', '_', '.' and '/', with at least one non-digit.", qualifier, branch));
+      "Malformed branch for %s: %s. Allowed characters are alphanumeric, '-', '_', '.' and '/', with at least one non-digit.", getQualifierToDisplay(qualifier), branch);
   }
 
-  private String formatMessage(String message, String qualifier, String key) {
-    return String.format(message, i18n.message(Locale.getDefault(), "qualifier." + qualifier, "Project"), key);
+  private String getQualifierToDisplay(String qualifier) {
+    return i18n.message(Locale.getDefault(), "qualifier." + qualifier, "Project");
   }
 
   private void index(ComponentDto project) {
index afacf907f686e1e208a65da29fce1fcad1573290..fae662acb0a7693a78b9676a7c4a0a492196f4df 100644 (file)
@@ -61,9 +61,9 @@ public class ComponentUpdaterTest {
   private PermissionTemplateService permissionTemplateService = mock(PermissionTemplateService.class);
 
   private ComponentUpdater underTest = new ComponentUpdater(db.getDbClient(), i18n, system2,
-      permissionTemplateService,
-      new FavoriteUpdater(db.getDbClient()),
-      projectIndexer);
+    permissionTemplateService,
+    new FavoriteUpdater(db.getDbClient()),
+    projectIndexer);
 
   @Test
   public void should_persist_and_index_when_creating_project() throws Exception {
@@ -125,13 +125,13 @@ public class ComponentUpdaterTest {
   @Test
   public void create_project_with_branch() throws Exception {
     ComponentDto project = underTest.create(db.getSession(),
-        NewComponent.newComponentBuilder()
-            .setKey(DEFAULT_PROJECT_KEY)
-            .setName(DEFAULT_PROJECT_NAME)
-            .setBranch("origin/master")
-            .setOrganizationUuid(db.getDefaultOrganization().getUuid())
-            .build(),
-        null);
+      NewComponent.newComponentBuilder()
+        .setKey(DEFAULT_PROJECT_KEY)
+        .setName(DEFAULT_PROJECT_NAME)
+        .setBranch("origin/master")
+        .setOrganizationUuid(db.getDefaultOrganization().getUuid())
+        .build(),
+      null);
 
     assertThat(project.getKey()).isEqualTo("project-key:origin/master");
   }
@@ -140,10 +140,10 @@ public class ComponentUpdaterTest {
   public void should_apply_default_permission_template() throws Exception {
     int userId = 42;
     NewComponent project = NewComponent.newComponentBuilder()
-        .setKey(DEFAULT_PROJECT_KEY)
-        .setName(DEFAULT_PROJECT_NAME)
-        .setOrganizationUuid(db.getDefaultOrganization().getUuid())
-        .build();
+      .setKey(DEFAULT_PROJECT_KEY)
+      .setName(DEFAULT_PROJECT_NAME)
+      .setOrganizationUuid(db.getDefaultOrganization().getUuid())
+      .build();
     ComponentDto dto = underTest.create(db.getSession(), project, userId);
 
     verify(permissionTemplateService).applyDefault(db.getSession(), dto.getOrganizationUuid(), dto, userId);
@@ -153,16 +153,16 @@ public class ComponentUpdaterTest {
   public void should_add_project_to_user_favorites_if_project_creator_is_defined_in_permission_template() throws Exception {
     UserDto userDto = db.users().insertUser();
     NewComponent project = NewComponent.newComponentBuilder()
-        .setKey(DEFAULT_PROJECT_KEY)
-        .setName(DEFAULT_PROJECT_NAME)
-        .setOrganizationUuid(db.getDefaultOrganization().getUuid())
-        .build();
+      .setKey(DEFAULT_PROJECT_KEY)
+      .setName(DEFAULT_PROJECT_NAME)
+      .setOrganizationUuid(db.getDefaultOrganization().getUuid())
+      .build();
     when(permissionTemplateService.hasDefaultTemplateWithPermissionOnProjectCreator(eq(db.getSession()), eq(project.getOrganizationUuid()), any(ComponentDto.class)))
-        .thenReturn(true);
+      .thenReturn(true);
 
     ComponentDto dto = underTest.create(db.getSession(),
-        project,
-        userDto.getId());
+      project,
+      userDto.getId());
 
     assertThat(db.favorites().hasFavorite(dto, userDto.getId())).isTrue();
   }
@@ -170,12 +170,12 @@ public class ComponentUpdaterTest {
   @Test
   public void does_not_add_project_to_favorite_when_anonymously_created() throws Exception {
     ComponentDto project = underTest.create(db.getSession(),
-        NewComponent.newComponentBuilder()
-            .setKey(DEFAULT_PROJECT_KEY)
-            .setName(DEFAULT_PROJECT_NAME)
-            .setOrganizationUuid(db.getDefaultOrganization().getUuid())
-            .build(),
-        null);
+      NewComponent.newComponentBuilder()
+        .setKey(DEFAULT_PROJECT_KEY)
+        .setName(DEFAULT_PROJECT_NAME)
+        .setOrganizationUuid(db.getDefaultOrganization().getUuid())
+        .build(),
+      null);
 
     assertThat(db.favorites().hasNoFavorite(project)).isTrue();
   }
@@ -183,12 +183,12 @@ public class ComponentUpdaterTest {
   @Test
   public void does_not_add_project_to_favorite_when_project_has_no_permission_on_template() throws Exception {
     ComponentDto project = underTest.create(db.getSession(),
-        NewComponent.newComponentBuilder()
-            .setKey(DEFAULT_PROJECT_KEY)
-            .setName(DEFAULT_PROJECT_NAME)
-            .setOrganizationUuid(db.getDefaultOrganization().getUuid())
-            .build(),
-        null);
+      NewComponent.newComponentBuilder()
+        .setKey(DEFAULT_PROJECT_KEY)
+        .setName(DEFAULT_PROJECT_NAME)
+        .setOrganizationUuid(db.getDefaultOrganization().getUuid())
+        .build(),
+      null);
 
     assertThat(db.favorites().hasNoFavorite(project)).isTrue();
   }
@@ -201,12 +201,12 @@ public class ComponentUpdaterTest {
     expectedException.expectMessage("Could not create Project, key already exists: " + existing.key());
 
     underTest.create(db.getSession(),
-        NewComponent.newComponentBuilder()
-            .setKey(existing.key())
-            .setName(DEFAULT_PROJECT_NAME)
-            .setOrganizationUuid(existing.getOrganizationUuid())
-            .build(),
-        null);
+      NewComponent.newComponentBuilder()
+        .setKey(existing.key())
+        .setName(DEFAULT_PROJECT_NAME)
+        .setOrganizationUuid(existing.getOrganizationUuid())
+        .build(),
+      null);
   }
 
   @Test
@@ -217,12 +217,12 @@ public class ComponentUpdaterTest {
     expectedException.expectMessage("Could not create Project, key already exists: " + existing.key());
 
     underTest.create(db.getSession(),
-        NewComponent.newComponentBuilder()
-            .setKey(existing.key())
-            .setName(DEFAULT_PROJECT_NAME)
-            .setOrganizationUuid(existing.getOrganizationUuid())
-            .build(),
-        null);
+      NewComponent.newComponentBuilder()
+        .setKey(existing.key())
+        .setName(DEFAULT_PROJECT_NAME)
+        .setOrganizationUuid(existing.getOrganizationUuid())
+        .build(),
+      null);
   }
 
   @Test
@@ -231,12 +231,26 @@ public class ComponentUpdaterTest {
     expectedException.expectMessage("Malformed key for Project: 1234");
 
     underTest.create(db.getSession(),
-        NewComponent.newComponentBuilder()
-            .setKey("1234")
-            .setName(DEFAULT_PROJECT_NAME)
-            .setOrganizationUuid(db.getDefaultOrganization().getUuid())
-            .build(),
-        null);
+      NewComponent.newComponentBuilder()
+        .setKey("1234")
+        .setName(DEFAULT_PROJECT_NAME)
+        .setOrganizationUuid(db.getDefaultOrganization().getUuid())
+        .build(),
+      null);
+  }
+
+  @Test
+  public void properly_fail_when_key_contains_percent_character() {
+    expectedException.expect(BadRequestException.class);
+    expectedException.expectMessage("Malformed key for Project: project%Key");
+
+    underTest.create(db.getSession(),
+      NewComponent.newComponentBuilder()
+        .setKey("project%Key")
+        .setName(DEFAULT_PROJECT_NAME)
+        .setOrganizationUuid(db.getDefaultOrganization().getUuid())
+        .build(),
+      null);
   }
 
   @Test
@@ -245,23 +259,23 @@ public class ComponentUpdaterTest {
     expectedException.expectMessage("Malformed branch for Project: origin?branch. Allowed characters are alphanumeric, '-', '_', '.' and '/', with at least one non-digit.");
 
     underTest.create(db.getSession(),
-        NewComponent.newComponentBuilder()
-            .setKey(DEFAULT_PROJECT_KEY)
-            .setName(DEFAULT_PROJECT_NAME)
-            .setBranch("origin?branch")
-            .setOrganizationUuid(db.getDefaultOrganization().getUuid())
-            .build(),
-        null);
+      NewComponent.newComponentBuilder()
+        .setKey(DEFAULT_PROJECT_KEY)
+        .setName(DEFAULT_PROJECT_NAME)
+        .setBranch("origin?branch")
+        .setOrganizationUuid(db.getDefaultOrganization().getUuid())
+        .build(),
+      null);
   }
 
   @Test
   public void persist_and_index_when_creating_view() {
     NewComponent view = NewComponent.newComponentBuilder()
-        .setKey("view-key")
-        .setName("view-name")
-        .setQualifier(VIEW)
-        .setOrganizationUuid(db.getDefaultOrganization().getUuid())
-        .build();
+      .setKey("view-key")
+      .setName("view-name")
+      .setQualifier(VIEW)
+      .setOrganizationUuid(db.getDefaultOrganization().getUuid())
+      .build();
 
     ComponentDto returned = underTest.create(db.getSession(), view, null);
 
index 44b6226c6a8df8a7abc342730a396a1fce242427..ed742f09207f5db2b8a10e9a4ac015821fe2e5f4 100644 (file)
@@ -24,37 +24,35 @@ import org.assertj.core.api.AssertionsForClassTypes;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.ExpectedException;
-import org.mockito.ArgumentCaptor;
-import org.mockito.Mockito;
 import org.sonar.api.server.ws.WebService;
 import org.sonar.api.utils.System2;
-import org.sonar.db.DbSession;
 import org.sonar.db.DbTester;
 import org.sonar.db.component.ComponentDto;
 import org.sonar.db.organization.OrganizationDto;
 import org.sonar.server.component.ComponentUpdater;
-import org.sonar.server.component.NewComponent;
+import org.sonar.server.es.ProjectIndexer;
 import org.sonar.server.exceptions.BadRequestException;
 import org.sonar.server.exceptions.ForbiddenException;
+import org.sonar.server.favorite.FavoriteUpdater;
+import org.sonar.server.i18n.I18nRule;
 import org.sonar.server.organization.BillingValidations;
 import org.sonar.server.organization.BillingValidations.BillingValidationsException;
 import org.sonar.server.organization.BillingValidationsProxy;
 import org.sonar.server.organization.DefaultOrganizationProvider;
 import org.sonar.server.organization.TestDefaultOrganizationProvider;
+import org.sonar.server.permission.PermissionTemplateService;
 import org.sonar.server.tester.UserSessionRule;
 import org.sonar.server.ws.TestRequest;
 import org.sonar.server.ws.WsActionTester;
 import org.sonarqube.ws.WsProjects.CreateWsResponse;
+import org.sonarqube.ws.WsProjects.CreateWsResponse.Project;
 import org.sonarqube.ws.client.project.CreateRequest;
 
 import static org.assertj.core.api.AssertionsForInterfaceTypes.assertThat;
 import static org.mockito.Matchers.any;
-import static org.mockito.Matchers.anyInt;
 import static org.mockito.Matchers.eq;
 import static org.mockito.Mockito.doThrow;
 import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.verify;
-import static org.mockito.Mockito.when;
 import static org.sonar.core.util.Protobuf.setNullable;
 import static org.sonar.db.permission.OrganizationPermission.PROVISION_PROJECTS;
 import static org.sonar.server.project.Visibility.PRIVATE;
@@ -79,46 +77,50 @@ public class CreateActionTest {
   public DbTester db = DbTester.create(system2);
   @Rule
   public UserSessionRule userSession = UserSessionRule.standalone();
+  @Rule
+  public I18nRule i18n = new I18nRule().put("qualifier.TRK", "Project");
 
   private DefaultOrganizationProvider defaultOrganizationProvider = TestDefaultOrganizationProvider.from(db);
-  private ComponentUpdater componentUpdater = mock(ComponentUpdater.class, Mockito.RETURNS_MOCKS);
   private BillingValidationsProxy billingValidations = mock(BillingValidationsProxy.class);
 
   private WsActionTester ws = new WsActionTester(
     new CreateAction(
       new ProjectsWsSupport(db.getDbClient(), billingValidations),
       db.getDbClient(), userSession,
-      componentUpdater,
+      new ComponentUpdater(db.getDbClient(), i18n, system2, mock(PermissionTemplateService.class), new FavoriteUpdater(db.getDbClient()),
+        mock(ProjectIndexer.class)),
       defaultOrganizationProvider));
 
   @Test
   public void create_project() throws Exception {
     userSession.addPermission(PROVISION_PROJECTS, db.getDefaultOrganization());
-    expectSuccessfulCallToComponentUpdater();
 
     CreateWsResponse response = call(CreateRequest.builder()
       .setKey(DEFAULT_PROJECT_KEY)
       .setName(DEFAULT_PROJECT_NAME)
       .build());
 
-    assertThat(response.getProject().getKey()).isEqualTo(DEFAULT_PROJECT_KEY);
-    assertThat(response.getProject().getName()).isEqualTo(DEFAULT_PROJECT_NAME);
-    assertThat(response.getProject().getQualifier()).isEqualTo("TRK");
+    assertThat(response.getProject())
+      .extracting(Project::getKey, Project::getName, Project::getQualifier, Project::getVisibility)
+      .containsOnly(DEFAULT_PROJECT_KEY, DEFAULT_PROJECT_NAME, "TRK", "public");
+    assertThat(db.getDbClient().componentDao().selectByKey(db.getSession(), DEFAULT_PROJECT_KEY).get())
+      .extracting(ComponentDto::getKey, ComponentDto::name, ComponentDto::qualifier, ComponentDto::scope, ComponentDto::isPrivate)
+      .containsOnly(DEFAULT_PROJECT_KEY, DEFAULT_PROJECT_NAME, "TRK", "PRJ", false);
   }
 
   @Test
   public void create_project_with_branch() throws Exception {
     userSession.addPermission(PROVISION_PROJECTS, db.getDefaultOrganization());
 
-    call(CreateRequest.builder()
+    CreateWsResponse response = call(CreateRequest.builder()
       .setKey(DEFAULT_PROJECT_KEY)
       .setName(DEFAULT_PROJECT_NAME)
       .setBranch("origin/master")
       .build());
 
-    NewComponent called = verifyCallToComponentUpdater();
-    assertThat(called.key()).isEqualTo(DEFAULT_PROJECT_KEY);
-    assertThat(called.branch()).isEqualTo("origin/master");
+    assertThat(response.getProject())
+      .extracting(Project::getKey, Project::getName, Project::getQualifier, Project::getVisibility)
+      .containsOnly(DEFAULT_PROJECT_KEY + ":origin/master", DEFAULT_PROJECT_NAME, "TRK", "public");
   }
 
   @Test
@@ -126,23 +128,22 @@ public class CreateActionTest {
     OrganizationDto organization = db.organizations().insert();
     userSession.addPermission(PROVISION_PROJECTS, organization);
 
-    ws.newRequest()
+    CreateWsResponse response = ws.newRequest()
       .setMethod(POST.name())
       .setParam("organization", organization.getKey())
       .setParam("key", DEFAULT_PROJECT_KEY)
       .setParam(PARAM_NAME, DEFAULT_PROJECT_NAME)
-      .execute();
+      .executeProtobuf(CreateWsResponse.class);
 
-    NewComponent called = verifyCallToComponentUpdater();
-    assertThat(called.key()).isEqualTo(DEFAULT_PROJECT_KEY);
-    assertThat(called.branch()).isNull();
+    assertThat(response.getProject())
+      .extracting(Project::getKey, Project::getName, Project::getQualifier, Project::getVisibility)
+      .containsOnly(DEFAULT_PROJECT_KEY, DEFAULT_PROJECT_NAME, "TRK", "public");
   }
 
   @Test
   public void apply_project_visibility_public() {
     OrganizationDto organization = db.organizations().insert();
     userSession.addPermission(PROVISION_PROJECTS, organization);
-    expectSuccessfulCallToComponentUpdater();
 
     CreateWsResponse result = ws.newRequest()
       .setParam("key", DEFAULT_PROJECT_KEY)
@@ -158,7 +159,6 @@ public class CreateActionTest {
   public void apply_project_visibility_private() {
     OrganizationDto organization = db.organizations().insert();
     userSession.addPermission(PROVISION_PROJECTS, organization);
-    expectSuccessfulCallToComponentUpdater();
 
     CreateWsResponse result = ws.newRequest()
       .setParam("key", DEFAULT_PROJECT_KEY)
@@ -175,7 +175,6 @@ public class CreateActionTest {
     OrganizationDto organization = db.organizations().insert();
     db.organizations().setNewProjectPrivate(organization, false);
     userSession.addPermission(PROVISION_PROJECTS, organization);
-    expectSuccessfulCallToComponentUpdater();
 
     CreateWsResponse result = ws.newRequest()
       .setParam("key", DEFAULT_PROJECT_KEY)
@@ -191,7 +190,6 @@ public class CreateActionTest {
     OrganizationDto organization = db.organizations().insert();
     db.organizations().setNewProjectPrivate(organization, true);
     userSession.addPermission(PROVISION_PROJECTS, organization);
-    expectSuccessfulCallToComponentUpdater();
 
     CreateWsResponse result = ws.newRequest()
       .setParam("key", DEFAULT_PROJECT_KEY)
@@ -206,7 +204,6 @@ public class CreateActionTest {
   public void does_not_fail_to_create_public_projects_when_organization_is_not_allowed_to_use_private_projects() {
     OrganizationDto organization = db.organizations().insert();
     userSession.addPermission(PROVISION_PROJECTS, organization);
-    expectSuccessfulCallToComponentUpdater();
     doThrow(new BillingValidationsException("This organization cannot use project private")).when(billingValidations)
       .checkCanUpdateProjectVisibility(any(BillingValidations.Organization.class), eq(true));
 
@@ -224,7 +221,6 @@ public class CreateActionTest {
   public void fail_to_create_private_projects_when_organization_is_not_allowed_to_use_private_projects() {
     OrganizationDto organization = db.organizations().insert();
     userSession.addPermission(PROVISION_PROJECTS, organization);
-    expectSuccessfulCallToComponentUpdater();
     doThrow(new BillingValidationsException("This organization cannot use project private")).when(billingValidations)
       .checkCanUpdateProjectVisibility(any(BillingValidations.Organization.class), eq(true));
 
@@ -242,7 +238,7 @@ public class CreateActionTest {
   @Test
   public void fail_when_project_already_exists() throws Exception {
     OrganizationDto organization = db.organizations().insert();
-    when(componentUpdater.create(any(DbSession.class), any(NewComponent.class), anyInt())).thenThrow(BadRequestException.create("already exists"));
+    db.components().insertPublicProject(project -> project.setKey(DEFAULT_PROJECT_KEY));
     userSession.addPermission(PROVISION_PROJECTS, organization);
 
     expectedException.expect(BadRequestException.class);
@@ -254,6 +250,19 @@ public class CreateActionTest {
       .build());
   }
 
+  @Test
+  public void properly_fail_when_project_key_contains_percent_character() {
+    userSession.addPermission(PROVISION_PROJECTS, db.getDefaultOrganization());
+
+    expectedException.expect(BadRequestException.class);
+    expectedException.expectMessage("Malformed key for Project: project%Key. Allowed characters are alphanumeric, '-', '_', '.' and ':', with at least one non-digit.");
+
+    call(CreateRequest.builder()
+      .setKey("project%Key")
+      .setName(DEFAULT_PROJECT_NAME)
+      .build());
+  }
+
   @Test
   public void fail_when_missing_project_parameter() throws Exception {
     expectedException.expect(IllegalArgumentException.class);
@@ -280,7 +289,6 @@ public class CreateActionTest {
   @Test
   public void test_example() {
     userSession.addPermission(PROVISION_PROJECTS, db.getDefaultOrganization());
-    expectSuccessfulCallToComponentUpdater();
 
     String result = ws.newRequest()
       .setParam("key", DEFAULT_PROJECT_KEY)
@@ -304,8 +312,7 @@ public class CreateActionTest {
       PARAM_ORGANIZATION,
       PARAM_NAME,
       PARAM_PROJECT,
-      PARAM_BRANCH
-    );
+      PARAM_BRANCH);
 
     WebService.Param organization = definition.param(PARAM_ORGANIZATION);
     Assertions.assertThat(organization.description()).isEqualTo("The key of the organization");
@@ -331,16 +338,4 @@ public class CreateActionTest {
     return httpRequest.executeProtobuf(CreateWsResponse.class);
   }
 
-  private NewComponent verifyCallToComponentUpdater() {
-    ArgumentCaptor<NewComponent> argument = ArgumentCaptor.forClass(NewComponent.class);
-    verify(componentUpdater).create(any(DbSession.class), argument.capture(), anyInt());
-    return argument.getValue();
-  }
-
-  private void expectSuccessfulCallToComponentUpdater() {
-    when(componentUpdater.create(any(DbSession.class), any(NewComponent.class), anyInt())).thenAnswer(invocation -> {
-      NewComponent newC = invocation.getArgumentAt(1, NewComponent.class);
-      return new ComponentDto().setKey(newC.key()).setQualifier(newC.qualifier()).setName(newC.name()).setPrivate(newC.isPrivate());
-    });
-  }
 }