SONAR-7545 functional error when updating a project key

- when updating a project key and a sub-component has a key longer than 400 characters
- when a component has a name longer than 2000 characters
- when a component has a key longer than 400 characters

diff --git a/server/sonar-server/src/main/java/org/sonar/server/component/NewComponent.java b/server/sonar-server/src/main/java/org/sonar/server/component/NewComponent.java
index ad380eeee3fc4fdba46e68d6025f9e267e37d265..ce0a0d60cf5dbc06d891dfe2c65d9ac5b33f55f9 100644 (file)
--- a/server/sonar-server/src/main/java/org/sonar/server/component/NewComponent.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/component/NewComponent.java
@@ -24,12 +24,12 @@ import javax.annotation.Nullable;
 import org.sonar.api.resources.Qualifiers;
 
 import static com.google.common.base.Preconditions.checkArgument;
+import static org.sonar.db.component.ComponentValidator.checkComponentKey;
+import static org.sonar.db.component.ComponentValidator.checkComponentName;
+import static org.sonar.db.component.ComponentValidator.checkComponentQualifier;
 
 public class NewComponent {
 
-  private static final int MAX_KEY_LENGHT = 400;
-  private static final int MAX_NAME_LENGTH = 2000;
-  private static final int MAX_QUALIFIER_LENGTH = 10;
   private String key;
   private String branch;
   private String qualifier;
@@ -63,20 +63,15 @@ public class NewComponent {
   }
 
   public NewComponent setQualifier(@Nullable String qualifier) {
-    if (qualifier != null) {
-      checkArgument(qualifier.length() <= MAX_QUALIFIER_LENGTH,
-        "Component qualifier length (%s) is longer than the maximum authorized (%s)", qualifier.length(), MAX_QUALIFIER_LENGTH);
-    }
-
-    this.qualifier = qualifier;
+    this.qualifier = qualifier == null ? null : checkComponentQualifier(qualifier);
     return this;
   }
 
   public static NewComponent create(String key, String name) {
-    checkArgument(key != null, "Key can't be null");
-    checkArgument(key.length() <= MAX_KEY_LENGHT, "Component key length (%s) is longer than the maximum authorized (%s)", key.length(), MAX_KEY_LENGHT);
-    checkArgument(name != null, "Name can't be null");
-    checkArgument(name.length() <= MAX_NAME_LENGTH, "Component name length (%s) is longer than the maximum authorized (%s)", name.length(), MAX_NAME_LENGTH);
+    checkArgument(key!=null, "Key can't be null");
+    checkComponentKey(key);
+    checkArgument(name!=null, "Name can't be null");
+    checkComponentName(name);
     return new NewComponent(key, name);
   }
 }

diff --git a/server/sonar-server/src/test/java/org/sonar/server/component/NewComponentTest.java b/server/sonar-server/src/test/java/org/sonar/server/component/NewComponentTest.java
index 35e01685c8cbc4419c3594f5af6edfdc042d6a77..4312778d1dd6fc9fc3cd6915e2bcf8b89eda89f3 100644 (file)
--- a/server/sonar-server/src/test/java/org/sonar/server/component/NewComponentTest.java
+++ b/server/sonar-server/src/test/java/org/sonar/server/component/NewComponentTest.java
@@ -23,7 +23,7 @@ import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.ExpectedException;
 
-import static org.apache.commons.lang.RandomStringUtils.randomAlphabetic;
+import static com.google.common.base.Strings.repeat;
 
 public class NewComponentTest {
   @Rule
@@ -42,7 +42,7 @@ public class NewComponentTest {
     expectedException.expect(IllegalArgumentException.class);
     expectedException.expectMessage("Component key length (401) is longer than the maximum authorized (400)");
 
-    NewComponent.create(randomAlphabetic(401), "name");
+    NewComponent.create(repeat("a", 400 + 1), "name");
   }
 
   @Test
@@ -58,7 +58,7 @@ public class NewComponentTest {
     expectedException.expect(IllegalArgumentException.class);
     expectedException.expectMessage("Component name length (2001) is longer than the maximum authorized (2000)");
 
-    NewComponent.create("key", randomAlphabetic(2001));
+    NewComponent.create("key", repeat("a", 2001));
   }
 
   @Test
@@ -66,6 +66,6 @@ public class NewComponentTest {
     expectedException.expect(IllegalArgumentException.class);
     expectedException.expectMessage("Component qualifier length (11) is longer than the maximum authorized (10)");
 
-    NewComponent.create("key", "name").setQualifier(randomAlphabetic(11));
+    NewComponent.create("key", "name").setQualifier(repeat("a", 10 + 1));
   }
 }

diff --git a/sonar-db/src/main/java/org/sonar/db/component/ComponentDto.java b/sonar-db/src/main/java/org/sonar/db/component/ComponentDto.java
index 3f18a8f07b247b55f822d6f30e59b32c482f9200..d1ba2e6b605f217398609c7415d8285fa0d5362a 100644 (file)
--- a/sonar-db/src/main/java/org/sonar/db/component/ComponentDto.java
+++ b/sonar-db/src/main/java/org/sonar/db/component/ComponentDto.java
@@ -26,6 +26,9 @@ import org.apache.commons.lang.builder.ToStringBuilder;
 import org.sonar.api.component.Component;
 import org.sonar.api.resources.Scopes;
 
+import static org.sonar.db.component.ComponentValidator.checkComponentKey;
+import static org.sonar.db.component.ComponentValidator.checkComponentName;
+
 public class ComponentDto implements Component {
 
   public static final String MODULE_UUID_PATH_SEP = ".";
@@ -160,7 +163,7 @@ public class ComponentDto implements Component {
   }
 
   public ComponentDto setName(String name) {
-    this.name = name;
+    this.name = checkComponentName(name);
     return this;
   }
 
@@ -258,7 +261,7 @@ public class ComponentDto implements Component {
   }
 
   public ComponentDto setKey(String key) {
-    this.kee = key;
+    this.kee = checkComponentKey(key);
     return this;
   }
 

diff --git a/sonar-db/src/main/java/org/sonar/db/component/ComponentValidator.java b/sonar-db/src/main/java/org/sonar/db/component/ComponentValidator.java
new file mode 100644 (file)
index 0000000..eb628cd
--- /dev/null
+++ b/sonar-db/src/main/java/org/sonar/db/component/ComponentValidator.java
@@ -0,0 +1,55 @@
+/*
+ * SonarQube
+ * Copyright (C) 2009-2016 SonarSource SA
+ * mailto:contact 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.db.component;
+
+import static com.google.common.base.Preconditions.checkArgument;
+
+public class ComponentValidator {
+  private static final int MAX_NAME_LENGTH = 2000;
+  private static final int MAX_KEY_LENGTH = 400;
+  private static final int MAX_QUALIFIER_LENGTH = 10;
+
+  private ComponentValidator() {
+    // prevent instantiation
+  }
+
+  public static String checkComponentName(String name) {
+    checkArgument(name.length() <= MAX_NAME_LENGTH, "Component name length (%s) is longer than the maximum authorized (%s). '%s' was provided.",
+      name.length(), MAX_NAME_LENGTH, name);
+    return name;
+  }
+
+  public static String checkComponentKey(String key) {
+    checkArgument(isComponentKeyValid(key), "Component key length (%s) is longer than the maximum authorized (%s). '%s' was provided.",
+      key.length(), MAX_KEY_LENGTH, key);
+    return key;
+  }
+
+  public static boolean isComponentKeyValid(String key) {
+    return key.length() <= MAX_KEY_LENGTH;
+  }
+
+  public static String checkComponentQualifier(String qualifier) {
+    checkArgument(qualifier.length() <= MAX_QUALIFIER_LENGTH, "Component qualifier length (%s) is longer than the maximum authorized (%s). '%s' was provided.",
+      qualifier.length(), MAX_QUALIFIER_LENGTH, qualifier);
+    return qualifier;
+  }
+}

diff --git a/sonar-db/src/main/java/org/sonar/db/component/ResourceDto.java b/sonar-db/src/main/java/org/sonar/db/component/ResourceDto.java
index 22b14fd2facd613a4cff6927dca5fa5be9913409..844803142958df9d3ca9093d8c06139aac4b4891 100644 (file)
--- a/sonar-db/src/main/java/org/sonar/db/component/ResourceDto.java
+++ b/sonar-db/src/main/java/org/sonar/db/component/ResourceDto.java
@@ -21,6 +21,9 @@ package org.sonar.db.component;
 
 import java.util.Date;
 
+import static org.sonar.db.component.ComponentValidator.checkComponentKey;
+import static org.sonar.db.component.ComponentValidator.checkComponentName;
+
 public class ResourceDto {
 
   private Long id;
@@ -94,7 +97,7 @@ public class ResourceDto {
   }
 
   public ResourceDto setName(String name) {
-    this.name = name;
+    this.name = checkComponentName(name);
     return this;
   }
 
@@ -103,7 +106,7 @@ public class ResourceDto {
   }
 
   public ResourceDto setKey(String s) {
-    this.key = s;
+    this.key = checkComponentKey(s);
     return this;
   }
 

diff --git a/sonar-db/src/main/java/org/sonar/db/component/ResourceKeyUpdaterDao.java b/sonar-db/src/main/java/org/sonar/db/component/ResourceKeyUpdaterDao.java
index 7572ddee73ca06c4d8483eff7311a65571bd5e0f..a5b4f7db04dc8bd0c98293165649ae4fdeb768cc 100644 (file)
--- a/sonar-db/src/main/java/org/sonar/db/component/ResourceKeyUpdaterDao.java
+++ b/sonar-db/src/main/java/org/sonar/db/component/ResourceKeyUpdaterDao.java
@@ -123,11 +123,13 @@ public class ResourceKeyUpdaterDao implements Dao {
 
   private static void runBatchUpdateForAllResources(Collection<ResourceDto> resources, String oldKey, String newKey, ResourceKeyUpdaterMapper mapper) {
     for (ResourceDto resource : resources) {
-      String resourceKey = resource.getKey();
-      resource.setKey(newKey + resourceKey.substring(oldKey.length(), resourceKey.length()));
-      String resourceDeprecatedKey = resource.getDeprecatedKey();
-      if (StringUtils.isNotBlank(resourceDeprecatedKey)) {
-        resource.setDeprecatedKey(newKey + resourceDeprecatedKey.substring(oldKey.length(), resourceDeprecatedKey.length()));
+      String oldResourceKey = resource.getKey();
+      String newResourceKey = newKey + oldResourceKey.substring(oldKey.length(), oldResourceKey.length());
+      resource.setKey(newResourceKey);
+      String oldResourceDeprecatedKey = resource.getDeprecatedKey();
+      if (StringUtils.isNotBlank(oldResourceDeprecatedKey)) {
+        String newResourceDeprecatedKey = newKey + oldResourceDeprecatedKey.substring(oldKey.length(), oldResourceDeprecatedKey.length());
+        resource.setDeprecatedKey(newResourceDeprecatedKey);
       }
       mapper.update(resource);
     }

diff --git a/sonar-db/src/test/java/org/sonar/db/component/ComponentValidatorTest.java b/sonar-db/src/test/java/org/sonar/db/component/ComponentValidatorTest.java
new file mode 100644 (file)
index 0000000..8e15e27
--- /dev/null
+++ b/sonar-db/src/test/java/org/sonar/db/component/ComponentValidatorTest.java
@@ -0,0 +1,87 @@
+/*
+ * SonarQube
+ * Copyright (C) 2009-2016 SonarSource SA
+ * mailto:contact 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.db.component;
+
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.ExpectedException;
+
+import static com.google.common.base.Strings.repeat;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.sonar.test.TestUtils.hasOnlyPrivateConstructors;
+
+public class ComponentValidatorTest {
+  @Rule
+  public ExpectedException expectedException = ExpectedException.none();
+
+  @Test
+  public void check_name() {
+    String name = repeat("a", 2000);
+
+    assertThat(ComponentValidator.checkComponentName(name)).isEqualTo(name);
+  }
+
+  @Test
+  public void fail_when_name_longer_than_2000_characters() {
+    expectedException.expect(IllegalArgumentException.class);
+    expectedException.expectMessage("Component name length");
+
+    ComponentValidator.checkComponentName(repeat("a", 2000 + 1));
+  }
+
+  @Test
+  public void check_key() {
+    String key = repeat("a", 400);
+
+    assertThat(ComponentValidator.isComponentKeyValid(key)).isTrue();
+    assertThat(ComponentValidator.checkComponentKey(key)).isEqualTo(key);
+  }
+
+  @Test
+  public void fail_when_key_longer_than_400_characters() {
+    expectedException.expect(IllegalArgumentException.class);
+    expectedException.expectMessage("Component key length");
+    String key = repeat("a", 400 + 1);
+
+    assertThat(ComponentValidator.isComponentKeyValid(key)).isFalse();
+    ComponentValidator.checkComponentKey(key);
+  }
+
+  @Test
+  public void check_qualifier() {
+    String qualifier = repeat("a", 10);
+
+    assertThat(ComponentValidator.checkComponentQualifier(qualifier)).isEqualTo(qualifier);
+  }
+
+  @Test
+  public void fail_when_qualifier_is_longer_than_10_characters() {
+    expectedException.expect(IllegalArgumentException.class);
+    expectedException.expectMessage("Component qualifier length");
+
+    ComponentValidator.checkComponentQualifier(repeat("a", 10 + 1));
+  }
+
+  @Test
+  public void private_constructor() {
+    assertThat(hasOnlyPrivateConstructors(ComponentValidator.class)).isTrue();
+  }
+}

diff --git a/sonar-db/src/test/java/org/sonar/db/component/ResourceKeyUpdaterDaoTest.java b/sonar-db/src/test/java/org/sonar/db/component/ResourceKeyUpdaterDaoTest.java
index c71fd65f1b4d1104d4ba2b7d4e0918da2b0adf8b..c44ee35e9e46c1a4f1ec6e8df7372e750aee3910 100644 (file)
--- a/sonar-db/src/test/java/org/sonar/db/component/ResourceKeyUpdaterDaoTest.java
+++ b/sonar-db/src/test/java/org/sonar/db/component/ResourceKeyUpdaterDaoTest.java
@@ -19,6 +19,7 @@
  */
 package org.sonar.db.component;
 
+import com.google.common.base.Strings;
 import java.util.Map;
 import org.junit.Rule;
 import org.junit.Test;
@@ -27,7 +28,8 @@ import org.sonar.api.utils.System2;
 import org.sonar.db.DbTester;
 
 import static org.assertj.core.api.Assertions.assertThat;
-
+import static org.sonar.db.component.ComponentTesting.newFileDto;
+import static org.sonar.db.component.ComponentTesting.newProjectDto;
 
 public class ResourceKeyUpdaterDaoTest {
 
@@ -35,71 +37,84 @@ public class ResourceKeyUpdaterDaoTest {
   public ExpectedException thrown = ExpectedException.none();
 
   @Rule
-  public DbTester dbTester = DbTester.create(System2.INSTANCE);
+  public DbTester db = DbTester.create(System2.INSTANCE);
+  ComponentDbTester componentDb = new ComponentDbTester(db);
 
-  ResourceKeyUpdaterDao dao = dbTester.getDbClient().resourceKeyUpdaterDao();
+  ResourceKeyUpdaterDao underTest = db.getDbClient().resourceKeyUpdaterDao();
 
   @Test
   public void shouldUpdateKey() {
-    dbTester.prepareDbUnit(getClass(), "shared.xml");
+    db.prepareDbUnit(getClass(), "shared.xml");
 
-    dao.updateKey(2, "struts:core");
+    underTest.updateKey(2, "struts:core");
 
-    dbTester.assertDbUnit(getClass(), "shouldUpdateKey-result.xml", "projects");
+    db.assertDbUnit(getClass(), "shouldUpdateKey-result.xml", "projects");
   }
 
   @Test
   public void shouldNotUpdateKey() {
-    dbTester.prepareDbUnit(getClass(), "shared.xml");
+    db.prepareDbUnit(getClass(), "shared.xml");
 
     thrown.expect(IllegalStateException.class);
     thrown.expectMessage("Impossible to update key: a resource with \"org.struts:struts-ui\" key already exists.");
 
-    dao.updateKey(2, "org.struts:struts-ui");
+    underTest.updateKey(2, "org.struts:struts-ui");
   }
 
   @Test
   public void shouldBulkUpdateKey() {
-    dbTester.prepareDbUnit(getClass(), "shared.xml");
+    db.prepareDbUnit(getClass(), "shared.xml");
 
-    dao.bulkUpdateKey(1, "org.struts", "org.apache.struts");
+    underTest.bulkUpdateKey(1, "org.struts", "org.apache.struts");
 
-    dbTester.assertDbUnit(getClass(), "shouldBulkUpdateKey-result.xml", "projects");
+    db.assertDbUnit(getClass(), "shouldBulkUpdateKey-result.xml", "projects");
   }
 
   @Test
   public void shouldBulkUpdateKeyOnOnlyOneSubmodule() {
-    dbTester.prepareDbUnit(getClass(), "shared.xml");
+    db.prepareDbUnit(getClass(), "shared.xml");
 
-    dao.bulkUpdateKey(1, "struts-ui", "struts-web");
+    underTest.bulkUpdateKey(1, "struts-ui", "struts-web");
 
-    dbTester.assertDbUnit(getClass(), "shouldBulkUpdateKeyOnOnlyOneSubmodule-result.xml", "projects");
+    db.assertDbUnit(getClass(), "shouldBulkUpdateKeyOnOnlyOneSubmodule-result.xml", "projects");
   }
 
   @Test
   public void shouldFailBulkUpdateKeyIfKeyAlreadyExist() {
-    dbTester.prepareDbUnit(getClass(), "shared.xml");
+    db.prepareDbUnit(getClass(), "shared.xml");
 
     thrown.expect(IllegalStateException.class);
     thrown.expectMessage("Impossible to update key: a resource with \"foo:struts-core\" key already exists.");
 
-    dao.bulkUpdateKey(1, "org.struts", "foo");
+    underTest.bulkUpdateKey(1, "org.struts", "foo");
   }
 
   @Test
   public void shouldNotUpdateAllSubmodules() {
-    dbTester.prepareDbUnit(getClass(), "shouldNotUpdateAllSubmodules.xml");
+    db.prepareDbUnit(getClass(), "shouldNotUpdateAllSubmodules.xml");
+
+    underTest.bulkUpdateKey(1, "org.struts", "org.apache.struts");
 
-    dao.bulkUpdateKey(1, "org.struts", "org.apache.struts");
+    db.assertDbUnit(getClass(), "shouldNotUpdateAllSubmodules-result.xml", "projects");
+  }
 
-    dbTester.assertDbUnit(getClass(), "shouldNotUpdateAllSubmodules-result.xml", "projects");
+  @Test
+  public void fail_with_functional_exception_when_sub_component_key_is_longer_than_authorized() {
+    ComponentDto project = newProjectDto("project-uuid").setKey("old-project-key");
+    componentDb.insertComponent(project);
+    componentDb.insertComponent(newFileDto(project).setKey("old-project-key:file"));
+    String newLongProjectKey = Strings.repeat("a", 400);
+    thrown.expect(IllegalArgumentException.class);
+    thrown.expectMessage("Component key length (405) is longer than the maximum authorized (400). '" + newLongProjectKey + ":file' was provided.");
+
+    underTest.updateKey(project.getId(), newLongProjectKey);
   }
 
   @Test
   public void shouldCheckModuleKeysBeforeRenaming() {
-    dbTester.prepareDbUnit(getClass(), "shared.xml");
+    db.prepareDbUnit(getClass(), "shared.xml");
 
-    Map<String, String> checkResults = dao.checkModuleKeysBeforeRenaming(1, "org.struts", "foo");
+    Map<String, String> checkResults = underTest.checkModuleKeysBeforeRenaming(1, "org.struts", "foo");
     assertThat(checkResults.size()).isEqualTo(3);
     assertThat(checkResults.get("org.struts:struts")).isEqualTo("foo:struts");
     assertThat(checkResults.get("org.struts:struts-core")).isEqualTo("#duplicate_key#");