diff --git a/Documentation/dev-bazel.txt b/Documentation/dev-bazel.txt index 9b43a290cf..103ae0ac95 100644 --- a/Documentation/dev-bazel.txt +++ b/Documentation/dev-bazel.txt @@ -376,6 +376,21 @@ If Docker is not available, the Elasticsearch tests will be skipped. Note that Bazel currently does not show link:https://github.com/bazelbuild/bazel/issues/3476[the skipped tests,role=external,window=_blank]. +[[debug]] +=== Index Query Tests + +The `DEBUG` log level can optionally be enabled for the index query tests. That log level applies to +both Elasticsearch and Lucene tests. + +In Eclipse, set `-Ddebug=true` as a VM argument under the Run Configuration's `Arguments` tab. + +With `bazel`, here is an example for the Lucene `account` test: + +---- +bazel test --jvmopt='-Ddebug=true' \ +javatests/com/google/gerrit/server/query/account:lucene_query_test +---- + == Dependencies Dependency JARs are normally downloaded as needed, but you can diff --git a/Documentation/dev-e2e-tests.txt b/Documentation/dev-e2e-tests.txt index 628a0ec9cc..9a62f01cf3 100644 --- a/Documentation/dev-e2e-tests.txt +++ b/Documentation/dev-e2e-tests.txt @@ -106,12 +106,14 @@ More complex scenarios can be further developed, under the `com.google.gerrit.sc Valid commands are: +* `clone` * `fetch` * `pull` * `push` -* `clone` -The example above assumes that the `loadtest-repo` project exists in the Gerrit under test. +The example above assumes that the `loadtest-repo` project exists in the Gerrit under test. The +`HTTP Credentials` or password obtained from test user's `Settings` (in Gerrit) may be required, in +`src/test/resources/application.conf`, depending on the above commands used. == How to run tests diff --git a/java/com/google/gerrit/server/permissions/PermissionCollection.java b/java/com/google/gerrit/server/permissions/PermissionCollection.java index b23b5a9811..1f0370b74f 100644 --- a/java/com/google/gerrit/server/permissions/PermissionCollection.java +++ b/java/com/google/gerrit/server/permissions/PermissionCollection.java @@ -15,6 +15,7 @@ package com.google.gerrit.server.permissions; import static com.google.gerrit.common.data.PermissionRule.Action.BLOCK; +import static com.google.gerrit.server.project.RefPattern.containsParameters; import static com.google.gerrit.server.project.RefPattern.isRE; import static java.util.stream.Collectors.mapping; import static java.util.stream.Collectors.toList; @@ -131,7 +132,9 @@ public class PermissionCollection { Iterable matcherList, String ref, CurrentUser user) { try (Timer0.Context ignored = filterLatency.start()) { if (isRE(ref)) { - ref = RefPattern.shortestExample(ref); + if (!containsParameters(ref)) { + ref = RefPattern.shortestExample(ref); + } } else if (ref.endsWith("/*")) { ref = ref.substring(0, ref.length() - 1); } diff --git a/java/com/google/gerrit/server/project/RefPattern.java b/java/com/google/gerrit/server/project/RefPattern.java index 0e916fb959..c52914b667 100644 --- a/java/com/google/gerrit/server/project/RefPattern.java +++ b/java/com/google/gerrit/server/project/RefPattern.java @@ -18,9 +18,12 @@ import com.google.common.base.Throwables; import com.google.common.cache.CacheBuilder; import com.google.common.cache.CacheLoader; import com.google.common.cache.LoadingCache; +import com.google.common.collect.ImmutableMap; import com.google.gerrit.common.data.AccessSection; +import com.google.gerrit.common.data.ParameterizedString; import com.google.gerrit.exceptions.InvalidNameException; import dk.brics.automaton.RegExp; +import java.util.Map; import java.util.concurrent.ExecutionException; import java.util.regex.Pattern; import java.util.regex.PatternSyntaxException; @@ -69,11 +72,21 @@ public class RefPattern { return refPattern.startsWith(AccessSection.REGEX_PREFIX); } + public static boolean containsParameters(String refPattern) { + return refPattern.contains("${"); + } + public static RegExp toRegExp(String refPattern) { if (isRE(refPattern)) { refPattern = refPattern.substring(1); } - return new RegExp(refPattern, RegExp.NONE); + ParameterizedString template = new ParameterizedString(refPattern); + String replacement = "_PLACEHOLDER_"; + Map params = + ImmutableMap.of( + RefPattern.USERID_SHARDED, replacement, + RefPattern.USERNAME, replacement); + return new RegExp(template.replace(params), RegExp.NONE); } public static void validate(String refPattern) throws InvalidNameException { diff --git a/java/com/google/gerrit/server/project/RefPatternMatcher.java b/java/com/google/gerrit/server/project/RefPatternMatcher.java index f00e98ed35..b9076b3db6 100644 --- a/java/com/google/gerrit/server/project/RefPatternMatcher.java +++ b/java/com/google/gerrit/server/project/RefPatternMatcher.java @@ -15,6 +15,7 @@ package com.google.gerrit.server.project; import static com.google.common.collect.ImmutableSet.toImmutableSet; +import static com.google.gerrit.server.project.RefPattern.containsParameters; import static com.google.gerrit.server.project.RefPattern.isRE; import com.google.common.collect.ImmutableMap; @@ -32,7 +33,7 @@ import java.util.stream.Stream; public abstract class RefPatternMatcher { public static RefPatternMatcher getMatcher(String pattern) { - if (pattern.contains("${")) { + if (containsParameters(pattern)) { return new ExpandParameters(pattern); } else if (isRE(pattern)) { return new Regexp(pattern); @@ -80,7 +81,7 @@ public abstract class RefPatternMatcher { @Override public boolean match(String ref, CurrentUser user) { - return pattern.matcher(ref).matches(); + return pattern.matcher(ref).matches() || (isRE(ref) && pattern.pattern().equals(ref)); } } @@ -112,7 +113,11 @@ public abstract class RefPatternMatcher { @Override public boolean match(String ref, CurrentUser user) { - if (!ref.startsWith(prefix)) { + if (isRE(ref)) { + if (!ref.substring(1).startsWith(prefix)) { + return false; + } + } else if (!ref.startsWith(prefix)) { return false; } @@ -142,6 +147,9 @@ public abstract class RefPatternMatcher { } public boolean matchPrefix(String ref) { + if (isRE(ref)) { + return ref.substring(1).startsWith(prefix); + } return ref.startsWith(prefix); } diff --git a/javatests/com/google/gerrit/server/permissions/RefControlTest.java b/javatests/com/google/gerrit/server/permissions/RefControlTest.java index 08b008d534..43a3f10111 100644 --- a/javatests/com/google/gerrit/server/permissions/RefControlTest.java +++ b/javatests/com/google/gerrit/server/permissions/RefControlTest.java @@ -476,6 +476,21 @@ public class RefControlTest { assertCanUpload(u); } + @Test + public void usernamePatternRegExpCanUploadToAnyRef() throws Exception { + projectOperations + .project(localKey) + .forUpdate() + .add( + allow(PUSH) + .ref("^refs/heads/users/${username}/(public|private)/.+") + .group(REGISTERED_USERS)) + .update(); + ProjectControl u = user(localKey, "a-registered-user"); + assertCanUpload(u); + assertCanUpdate("refs/heads/users/a-registered-user/private/a", u); + } + @Test public void usernamePatternNonRegex() throws Exception { projectOperations @@ -500,6 +515,8 @@ public class RefControlTest { ProjectControl u = user(localKey, "d.v", DEVS); ProjectControl d = user(localKey, "dev", DEVS); + assertCanAccess(u); + assertCanAccess(d); assertCannotRead("refs/sb/dev/heads/foobar", u); assertCanRead("refs/sb/dev/heads/foobar", d); } @@ -1127,6 +1144,7 @@ public class RefControlTest { RefPattern.validate("^refs/heads/*"); RefPattern.validate("^refs/tags/[0-9a-zA-Z-_.]+"); RefPattern.validate("refs/heads/review/${username}/*"); + RefPattern.validate("^refs/heads/review/${username}/.+"); } @Test diff --git a/javatests/com/google/gerrit/server/query/account/AbstractQueryAccountsTest.java b/javatests/com/google/gerrit/server/query/account/AbstractQueryAccountsTest.java index e7f0812c24..31d20480c5 100644 --- a/javatests/com/google/gerrit/server/query/account/AbstractQueryAccountsTest.java +++ b/javatests/com/google/gerrit/server/query/account/AbstractQueryAccountsTest.java @@ -91,10 +91,14 @@ import java.util.HashMap; import java.util.Iterator; import java.util.List; import java.util.Optional; +import org.apache.log4j.Level; +import org.apache.log4j.LogManager; import org.eclipse.jgit.lib.PersonIdent; import org.eclipse.jgit.lib.Repository; import org.junit.After; +import org.junit.AfterClass; import org.junit.Before; +import org.junit.BeforeClass; import org.junit.Ignore; import org.junit.Rule; import org.junit.Test; @@ -146,6 +150,20 @@ public abstract class AbstractQueryAccountsTest extends GerritServerTests { protected abstract Injector createInjector(); + @BeforeClass + public static void setLogLevel() { + if (Boolean.getBoolean("debug")) { + LogManager.getRootLogger().setLevel(Level.DEBUG); + } + } + + @AfterClass + public static void resetLogLevel() { + if (Boolean.getBoolean("debug")) { + LogManager.getRootLogger().setLevel(Level.INFO); + } + } + @Before public void setUpInjector() throws Exception { lifecycle = new LifecycleManager(); diff --git a/javatests/com/google/gerrit/server/query/account/BUILD b/javatests/com/google/gerrit/server/query/account/BUILD index 5c910a04f3..da37f26cfb 100644 --- a/javatests/com/google/gerrit/server/query/account/BUILD +++ b/javatests/com/google/gerrit/server/query/account/BUILD @@ -23,8 +23,10 @@ java_library( "//lib:guava", "//lib:jgit", "//lib/guice", + "//lib/log:log4j", "//lib/truth", "//lib/truth:truth-java8-extension", + "//resources:log4j-config", ], ) diff --git a/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java b/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java index 38e4ca426b..71bdd69510 100644 --- a/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java +++ b/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java @@ -132,6 +132,8 @@ import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.concurrent.TimeUnit; +import org.apache.log4j.Level; +import org.apache.log4j.LogManager; import org.eclipse.jgit.junit.TestRepository; import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.ObjectInserter; @@ -143,7 +145,9 @@ import org.eclipse.jgit.revwalk.RevCommit; import org.eclipse.jgit.revwalk.RevWalk; import org.eclipse.jgit.util.SystemReader; import org.junit.After; +import org.junit.AfterClass; import org.junit.Before; +import org.junit.BeforeClass; import org.junit.Ignore; import org.junit.Test; @@ -206,6 +210,20 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests { protected abstract Injector createInjector(); + @BeforeClass + public static void setLogLevel() { + if (Boolean.getBoolean("debug")) { + LogManager.getRootLogger().setLevel(Level.DEBUG); + } + } + + @AfterClass + public static void resetLogLevel() { + if (Boolean.getBoolean("debug")) { + LogManager.getRootLogger().setLevel(Level.INFO); + } + } + @Before public void setUpInjector() throws Exception { lifecycle = new LifecycleManager(); diff --git a/javatests/com/google/gerrit/server/query/change/BUILD b/javatests/com/google/gerrit/server/query/change/BUILD index e5b51e7bce..520e65a71c 100644 --- a/javatests/com/google/gerrit/server/query/change/BUILD +++ b/javatests/com/google/gerrit/server/query/change/BUILD @@ -34,7 +34,9 @@ java_library( "//lib:jgit", "//lib:jgit-junit", "//lib/guice", + "//lib/log:log4j", "//lib/truth", + "//resources:log4j-config", ], ) diff --git a/javatests/com/google/gerrit/server/query/group/AbstractQueryGroupsTest.java b/javatests/com/google/gerrit/server/query/group/AbstractQueryGroupsTest.java index d80eac0c47..9b01f8d15e 100644 --- a/javatests/com/google/gerrit/server/query/group/AbstractQueryGroupsTest.java +++ b/javatests/com/google/gerrit/server/query/group/AbstractQueryGroupsTest.java @@ -68,8 +68,12 @@ import java.util.Iterator; import java.util.List; import java.util.Locale; import java.util.Optional; +import org.apache.log4j.Level; +import org.apache.log4j.LogManager; import org.junit.After; +import org.junit.AfterClass; import org.junit.Before; +import org.junit.BeforeClass; import org.junit.Ignore; import org.junit.Rule; import org.junit.Test; @@ -115,6 +119,20 @@ public abstract class AbstractQueryGroupsTest extends GerritServerTests { protected abstract Injector createInjector(); + @BeforeClass + public static void setLogLevel() { + if (Boolean.getBoolean("debug")) { + LogManager.getRootLogger().setLevel(Level.DEBUG); + } + } + + @AfterClass + public static void resetLogLevel() { + if (Boolean.getBoolean("debug")) { + LogManager.getRootLogger().setLevel(Level.INFO); + } + } + @Before public void setUpInjector() throws Exception { lifecycle = new LifecycleManager(); diff --git a/javatests/com/google/gerrit/server/query/group/BUILD b/javatests/com/google/gerrit/server/query/group/BUILD index e14350f093..8f6fd6dc0d 100644 --- a/javatests/com/google/gerrit/server/query/group/BUILD +++ b/javatests/com/google/gerrit/server/query/group/BUILD @@ -20,8 +20,10 @@ java_library( "//lib:guava", "//lib:jgit", "//lib/guice", + "//lib/log:log4j", "//lib/truth", "//lib/truth:truth-java8-extension", + "//resources:log4j-config", ], ) diff --git a/javatests/com/google/gerrit/server/query/project/AbstractQueryProjectsTest.java b/javatests/com/google/gerrit/server/query/project/AbstractQueryProjectsTest.java index dfd7928bf0..e142e0c7bf 100644 --- a/javatests/com/google/gerrit/server/query/project/AbstractQueryProjectsTest.java +++ b/javatests/com/google/gerrit/server/query/project/AbstractQueryProjectsTest.java @@ -68,8 +68,12 @@ import java.util.Iterator; import java.util.List; import java.util.Locale; import java.util.Set; +import org.apache.log4j.Level; +import org.apache.log4j.LogManager; import org.junit.After; +import org.junit.AfterClass; import org.junit.Before; +import org.junit.BeforeClass; import org.junit.Ignore; import org.junit.Rule; import org.junit.Test; @@ -111,6 +115,20 @@ public abstract class AbstractQueryProjectsTest extends GerritServerTests { protected abstract Injector createInjector(); + @BeforeClass + public static void setLogLevel() { + if (Boolean.getBoolean("debug")) { + LogManager.getRootLogger().setLevel(Level.DEBUG); + } + } + + @AfterClass + public static void resetLogLevel() { + if (Boolean.getBoolean("debug")) { + LogManager.getRootLogger().setLevel(Level.INFO); + } + } + @Before public void setUpInjector() throws Exception { lifecycle = new LifecycleManager(); diff --git a/javatests/com/google/gerrit/server/query/project/BUILD b/javatests/com/google/gerrit/server/query/project/BUILD index 984d82470f..996be2ff20 100644 --- a/javatests/com/google/gerrit/server/query/project/BUILD +++ b/javatests/com/google/gerrit/server/query/project/BUILD @@ -21,7 +21,9 @@ java_library( "//lib:guava", "//lib:jgit", "//lib/guice", + "//lib/log:log4j", "//lib/truth", + "//resources:log4j-config", ], )