Merge branch 'stable-2.14' into stable-2.15

* stable-2.14:
  AbstractElasticIndex: fix index name of delete request and add coverage
  Bazel: Remove unused commons-lang3
  DropWizardMetricMaker: Move sanitizeMetricName to MetricMaker base class

Change-Id: I5e03191d329c6893ba9823fd20676aeefe0fb653
This commit is contained in:
David Pursehouse 2018-06-08 10:33:15 +09:00
commit b989f3d421
9 changed files with 40 additions and 30 deletions

View File

@ -324,12 +324,6 @@ maven_jar(
sha1 = "0ce1edb914c94ebc388f086c6827e8bdeec71ac2",
)
maven_jar(
name = "commons_lang3",
artifact = "org.apache.commons:commons-lang3:3.3.2",
sha1 = "90a3822c38ec8c996e84c16a3477ef632cbc87a3",
)
maven_jar(
name = "commons_dbcp",
artifact = "commons-dbcp:commons-dbcp:1.4",

View File

@ -157,7 +157,7 @@ abstract class AbstractElasticIndex<K, V> implements Index<K, V> {
protected abstract String getId(V v);
protected String delete(String type, K id) {
return new DeleteRequest(id.toString(), indexNameRaw, type).toString();
return new DeleteRequest(id.toString(), indexName, type).toString();
}
protected void addNamedElement(String name, JsonObject element, JsonArray array) {

View File

@ -18,7 +18,6 @@ EXPORTS = [
"//gerrit-server:prolog-common",
"//lib/commons:dbcp",
"//lib/commons:lang",
"//lib/commons:lang3",
"//lib/dropwizard:dropwizard-core",
"//lib/guice:guice",
"//lib/guice:guice-assistedinject",

View File

@ -153,4 +153,14 @@ public abstract class MetricMaker {
}
public abstract RegistrationHandle newTrigger(Set<CallbackMetric<?>> metrics, Runnable trigger);
/**
* Sanitize the given metric name.
*
* @param name the name to sanitize.
* @return sanitized version of the name.
*/
public String sanitizeMetricName(String name) {
return name;
}
}

View File

@ -344,17 +344,18 @@ public class DropWizardMetricMaker extends MetricMaker {
METRIC_NAME_PATTERN.pattern());
}
public static String sanitizeMetricName(String input) {
if (METRIC_NAME_PATTERN.matcher(input).matches()) {
return input;
@Override
public String sanitizeMetricName(String name) {
if (METRIC_NAME_PATTERN.matcher(name).matches()) {
return name;
}
String first = input.substring(0, 1).replaceFirst("[^\\w-]", "_");
if (input.length() == 1) {
String first = name.substring(0, 1).replaceFirst("[^\\w-]", "_");
if (name.length() == 1) {
return first;
}
String result = first + input.substring(1).replaceAll("/[/]+", "/").replaceAll("[^\\w-/]", "_");
String result = first + name.substring(1).replaceAll("/[/]+", "/").replaceAll("[^\\w-/]", "_");
if (result.endsWith("/")) {
result = result.substring(0, result.length() - 1);

View File

@ -14,8 +14,6 @@
package com.google.gerrit.server.git;
import static com.google.gerrit.metrics.dropwizard.DropWizardMetricMaker.sanitizeMetricName;
import com.google.common.base.CaseFormat;
import com.google.common.base.Supplier;
import com.google.gerrit.extensions.events.LifecycleListener;
@ -347,7 +345,7 @@ public class WorkQueue {
CaseFormat.UPPER_CAMEL.to(
CaseFormat.LOWER_UNDERSCORE,
queueName.replaceFirst("SSH", "Ssh").replaceAll("-", ""));
return sanitizeMetricName(String.format("queue/%s/%s", name, metricName));
return metrics.sanitizeMetricName(String.format("queue/%s/%s", name, metricName));
}
@Override

View File

@ -15,28 +15,30 @@
package com.google.gerrit.metrics.dropwizard;
import static com.google.common.truth.Truth.assertThat;
import static com.google.gerrit.metrics.dropwizard.DropWizardMetricMaker.sanitizeMetricName;
import org.junit.Test;
public class DropWizardMetricMakerTest {
DropWizardMetricMaker metrics =
new DropWizardMetricMaker(null /* MetricRegistry unused in tests */);
@Test
public void shouldSanitizeUnwantedChars() throws Exception {
assertThat(sanitizeMetricName("very+confusing$long#metric@net/name^1"))
assertThat(metrics.sanitizeMetricName("very+confusing$long#metric@net/name^1"))
.isEqualTo("very_confusing_long_metric_net/name_1");
assertThat(sanitizeMetricName("/metric/submetric")).isEqualTo("_metric/submetric");
assertThat(metrics.sanitizeMetricName("/metric/submetric")).isEqualTo("_metric/submetric");
}
@Test
public void shouldReduceConsecutiveSlashesToOne() throws Exception {
assertThat(sanitizeMetricName("/metric//submetric1///submetric2/submetric3"))
assertThat(metrics.sanitizeMetricName("/metric//submetric1///submetric2/submetric3"))
.isEqualTo("_metric/submetric1/submetric2/submetric3");
}
@Test
public void shouldNotFinishWithSlash() throws Exception {
assertThat(sanitizeMetricName("metric/")).isEqualTo("metric");
assertThat(sanitizeMetricName("metric//")).isEqualTo("metric");
assertThat(sanitizeMetricName("metric/submetric/")).isEqualTo("metric/submetric");
assertThat(metrics.sanitizeMetricName("metric/")).isEqualTo("metric");
assertThat(metrics.sanitizeMetricName("metric//")).isEqualTo("metric");
assertThat(metrics.sanitizeMetricName("metric/submetric/")).isEqualTo("metric/submetric");
}
}

View File

@ -2601,6 +2601,18 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
assertQuery("owner: \"" + nameEmail + "\"\\");
}
@Test
public void byDeletedChange() throws Exception {
TestRepository<Repo> repo = createProject("repo");
Change change = insert(repo, newChange(repo));
String query = "change:" + change.getId();
assertQuery(query, change);
gApi.changes().id(change.getChangeId()).delete();
assertQuery(query);
}
protected ChangeInserter newChange(TestRepository<Repo> repo) throws Exception {
return newChange(repo, null, null, null, null, false);
}

View File

@ -28,12 +28,6 @@ java_library(
exports = ["@commons_lang//jar"],
)
java_library(
name = "lang3",
data = ["//lib:LICENSE-Apache2.0"],
exports = ["@commons_lang3//jar"],
)
java_library(
name = "net",
data = ["//lib:LICENSE-Apache2.0"],