Merge "Add acceptance test for plugin URL bindings"
diff --git a/Documentation/cmd-stream-events.txt b/Documentation/cmd-stream-events.txt
index 08b661d..8f24a47 100644
--- a/Documentation/cmd-stream-events.txt
+++ b/Documentation/cmd-stream-events.txt
@@ -280,6 +280,8 @@
change:: link:json.html#change[change attribute]
+patchSet:: link:json.html#patchSet[patchSet attribute]
+
changer:: link:json.html#account[account attribute]
eventCreatedOn:: Time in seconds since the UNIX epoch when this event was
@@ -294,6 +296,8 @@
change:: link:json.html#change[change attribute]
+patchSet:: link:json.html#patchSet[patchSet attribute]
+
changer:: link:json.html#account[account attribute]
eventCreatedOn:: Time in seconds since the UNIX epoch when this event was
diff --git a/Documentation/config-gerrit.txt b/Documentation/config-gerrit.txt
index 4880706..8479a8e 100644
--- a/Documentation/config-gerrit.txt
+++ b/Documentation/config-gerrit.txt
@@ -1967,6 +1967,23 @@
installModule = com.example.abc.OurSpecialSauceModule
----
+[[gerrit.listProjectsFromIndex]]gerrit.listProjectsFromIndex::
++
+Enable rendering of project list from the secondary index instead
+of purely relying on the in-memory cache.
++
+By default false.
++
+[NOTE]
+The in-memory cache (set to false) rendering provides an **unlimited list** as a result
+of the list project API, causing the full list of projects to be
+returned as a result of the link:rest-api-projects.html[/projects/] REST API
+or the link:cmd-ls-projects.html[gerrit ls-projects] SSH command.
+When the rendering from the secondary index (set to true),
+the **list is limited** by the global capability
+link:access-control.html#capability_queryLimit[queryLimit]
+which is defaulted to 500 entries.
+
[[gerrit.primaryWeblinkName]]gerrit.primaryWeblinkName::
+
Name of the link:dev-plugins.html#links-to-external-tools[Weblink] that should
@@ -1998,11 +2015,16 @@
+
Defaults to "Report Bug".
-[[gerrit.disableReverseDnsLookup]]gerrit.disableReverseDnsLookup::
+[[gerrit.enableReverseDnsLookup]]gerrit.enableReverseDnsLookup::
+
-Disables reverse DNS lookup during computing ref log entry for identified user.
+Enable reverse DNS lookup during computing ref log entry for identified user,
+to record the actual hostname of the user's host in the ref log.
+
-Defaults to false.
+Enabling reverse DNS lookup can cause performance issues on git push when
+the reverse DNS lookup is slow.
++
+Defaults to false, reverse DNS lookup is disabled. The user's IP address
+will be recorded in the ref log rather than their hostname.
[[gerrit.secureStoreClass]]gerrit.secureStoreClass::
+
diff --git a/java/com/google/gerrit/acceptance/GerritServer.java b/java/com/google/gerrit/acceptance/GerritServer.java
index 7571184..3a49f46 100644
--- a/java/com/google/gerrit/acceptance/GerritServer.java
+++ b/java/com/google/gerrit/acceptance/GerritServer.java
@@ -113,7 +113,8 @@
null, // @GerritConfig is only valid on methods.
null, // @GerritConfigs is only valid on methods.
null, // @GlobalPluginConfig is only valid on methods.
- null); // @GlobalPluginConfigs is only valid on methods.
+ null, // @GlobalPluginConfigs is only valid on methods.
+ getLogLevelThresholdAnnotation(testDesc));
}
public static Description forTestMethod(
@@ -135,7 +136,8 @@
testDesc.getAnnotation(GerritConfig.class),
testDesc.getAnnotation(GerritConfigs.class),
testDesc.getAnnotation(GlobalPluginConfig.class),
- testDesc.getAnnotation(GlobalPluginConfigs.class));
+ testDesc.getAnnotation(GlobalPluginConfigs.class),
+ getLogLevelThresholdAnnotation(testDesc));
}
private static boolean has(Class<? extends Annotation> annotation, Class<?> clazz) {
@@ -147,6 +149,14 @@
return false;
}
+ private static Level getLogLevelThresholdAnnotation(org.junit.runner.Description testDesc) {
+ LogThreshold logLevelThreshold = testDesc.getTestClass().getAnnotation(LogThreshold.class);
+ if (logLevelThreshold == null) {
+ return Level.DEBUG;
+ }
+ return Level.toLevel(logLevelThreshold.level());
+ }
+
abstract org.junit.runner.Description testDescription();
@Nullable
@@ -178,6 +188,8 @@
@Nullable
abstract GlobalPluginConfigs pluginConfigs();
+ abstract Level logLevelThreshold();
+
private void checkValidAnnotations() {
if (configs() != null && config() != null) {
throw new IllegalStateException("Use either @GerritConfigs or @GerritConfig not both");
@@ -364,7 +376,7 @@
throws Exception {
checkArgument(site != null, "site is required (even for in-memory server");
desc.checkValidAnnotations();
- configureLogging();
+ configureLogging(desc.logLevelThreshold());
CyclicBarrier serverStarted = new CyclicBarrier(2);
Daemon daemon =
new Daemon(
@@ -468,7 +480,7 @@
return new GerritServer(desc, site, createTestInjector(daemon), daemon, daemonService);
}
- private static void configureLogging() {
+ private static void configureLogging(Level threshold) {
LogManager.resetConfiguration();
PatternLayout layout = new PatternLayout();
@@ -477,7 +489,7 @@
ConsoleAppender dst = new ConsoleAppender();
dst.setLayout(layout);
dst.setTarget("System.err");
- dst.setThreshold(Level.DEBUG);
+ dst.setThreshold(threshold);
dst.activateOptions();
Logger root = LogManager.getRootLogger();
diff --git a/java/com/google/gerrit/server/config/DisableReverseDnsLookup.java b/java/com/google/gerrit/acceptance/LogThreshold.java
similarity index 66%
copy from java/com/google/gerrit/server/config/DisableReverseDnsLookup.java
copy to java/com/google/gerrit/acceptance/LogThreshold.java
index 336edeb..da1fcc5 100644
--- a/java/com/google/gerrit/server/config/DisableReverseDnsLookup.java
+++ b/java/com/google/gerrit/acceptance/LogThreshold.java
@@ -1,4 +1,4 @@
-// Copyright (C) 2014 The Android Open Source Project
+// Copyright (C) 2019 The Android Open Source Project
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
@@ -11,14 +11,17 @@
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
+package com.google.gerrit.acceptance;
-package com.google.gerrit.server.config;
-
+import static java.lang.annotation.ElementType.METHOD;
+import static java.lang.annotation.ElementType.TYPE;
import static java.lang.annotation.RetentionPolicy.RUNTIME;
-import com.google.inject.BindingAnnotation;
import java.lang.annotation.Retention;
+import java.lang.annotation.Target;
+@Target({TYPE, METHOD})
@Retention(RUNTIME)
-@BindingAnnotation
-public @interface DisableReverseDnsLookup {}
+public @interface LogThreshold {
+ String level() default "DEBUG";
+}
diff --git a/java/com/google/gerrit/gpg/server/GpgKeys.java b/java/com/google/gerrit/gpg/server/GpgKeys.java
index 3f090a1..e555f07 100644
--- a/java/com/google/gerrit/gpg/server/GpgKeys.java
+++ b/java/com/google/gerrit/gpg/server/GpgKeys.java
@@ -219,13 +219,14 @@
Iterator<String> userIds = key.getUserIDs();
info.userIds = ImmutableList.copyOf(userIds);
- try (ByteArrayOutputStream out = new ByteArrayOutputStream(4096);
- ArmoredOutputStream aout = new ArmoredOutputStream(out)) {
- // This is not exactly the key stored in the store, but is equivalent. In
- // particular, it will have a Bouncy Castle version string. The armored
- // stream reader in PublicKeyStore doesn't give us an easy way to extract
- // the original ASCII armor.
- key.encode(aout);
+ try (ByteArrayOutputStream out = new ByteArrayOutputStream(4096)) {
+ try (ArmoredOutputStream aout = new ArmoredOutputStream(out)) {
+ // This is not exactly the key stored in the store, but is equivalent. In
+ // particular, it will have a Bouncy Castle version string. The armored
+ // stream reader in PublicKeyStore doesn't give us an easy way to extract
+ // the original ASCII armor.
+ key.encode(aout);
+ }
info.key = new String(out.toByteArray(), UTF_8);
}
}
diff --git a/java/com/google/gerrit/pgm/util/BatchProgramModule.java b/java/com/google/gerrit/pgm/util/BatchProgramModule.java
index b0c1c25..2087104 100644
--- a/java/com/google/gerrit/pgm/util/BatchProgramModule.java
+++ b/java/com/google/gerrit/pgm/util/BatchProgramModule.java
@@ -48,8 +48,8 @@
import com.google.gerrit.server.config.CanonicalWebUrl;
import com.google.gerrit.server.config.CanonicalWebUrlProvider;
import com.google.gerrit.server.config.DefaultUrlFormatter;
-import com.google.gerrit.server.config.DisableReverseDnsLookup;
-import com.google.gerrit.server.config.DisableReverseDnsLookupProvider;
+import com.google.gerrit.server.config.EnableReverseDnsLookup;
+import com.google.gerrit.server.config.EnableReverseDnsLookupProvider;
import com.google.gerrit.server.config.GitReceivePackGroups;
import com.google.gerrit.server.config.GitUploadPackGroups;
import com.google.gerrit.server.config.SysExecutorModule;
@@ -120,8 +120,8 @@
.annotatedWith(CanonicalWebUrl.class)
.toProvider(CanonicalWebUrlProvider.class);
bind(Boolean.class)
- .annotatedWith(DisableReverseDnsLookup.class)
- .toProvider(DisableReverseDnsLookupProvider.class)
+ .annotatedWith(EnableReverseDnsLookup.class)
+ .toProvider(EnableReverseDnsLookupProvider.class)
.in(SINGLETON);
bind(Realm.class).to(FakeRealm.class);
bind(IdentifiedUser.class).toProvider(Providers.of(null));
diff --git a/java/com/google/gerrit/server/IdentifiedUser.java b/java/com/google/gerrit/server/IdentifiedUser.java
index e5e0cad..e65f562 100644
--- a/java/com/google/gerrit/server/IdentifiedUser.java
+++ b/java/com/google/gerrit/server/IdentifiedUser.java
@@ -32,7 +32,7 @@
import com.google.gerrit.server.config.AnonymousCowardName;
import com.google.gerrit.server.config.AuthConfig;
import com.google.gerrit.server.config.CanonicalWebUrl;
-import com.google.gerrit.server.config.DisableReverseDnsLookup;
+import com.google.gerrit.server.config.EnableReverseDnsLookup;
import com.google.gerrit.server.group.SystemGroupBackend;
import com.google.inject.Inject;
import com.google.inject.OutOfScopeException;
@@ -67,7 +67,7 @@
private final Provider<String> canonicalUrl;
private final AccountCache accountCache;
private final GroupBackend groupBackend;
- private final Boolean disableReverseDnsLookup;
+ private final Boolean enableReverseDnsLookup;
@Inject
public GenericFactory(
@@ -75,7 +75,7 @@
Realm realm,
@AnonymousCowardName String anonymousCowardName,
@CanonicalWebUrl Provider<String> canonicalUrl,
- @DisableReverseDnsLookup Boolean disableReverseDnsLookup,
+ @EnableReverseDnsLookup Boolean enableReverseDnsLookup,
AccountCache accountCache,
GroupBackend groupBackend) {
this.authConfig = authConfig;
@@ -84,7 +84,7 @@
this.canonicalUrl = canonicalUrl;
this.accountCache = accountCache;
this.groupBackend = groupBackend;
- this.disableReverseDnsLookup = disableReverseDnsLookup;
+ this.enableReverseDnsLookup = enableReverseDnsLookup;
}
public IdentifiedUser create(AccountState state) {
@@ -95,7 +95,7 @@
canonicalUrl,
accountCache,
groupBackend,
- disableReverseDnsLookup,
+ enableReverseDnsLookup,
Providers.of(null),
state,
null);
@@ -118,7 +118,7 @@
canonicalUrl,
accountCache,
groupBackend,
- disableReverseDnsLookup,
+ enableReverseDnsLookup,
Providers.of(remotePeer),
id,
caller);
@@ -139,7 +139,7 @@
private final Provider<String> canonicalUrl;
private final AccountCache accountCache;
private final GroupBackend groupBackend;
- private final Boolean disableReverseDnsLookup;
+ private final Boolean enableReverseDnsLookup;
private final Provider<SocketAddress> remotePeerProvider;
@Inject
@@ -150,7 +150,7 @@
@CanonicalWebUrl Provider<String> canonicalUrl,
AccountCache accountCache,
GroupBackend groupBackend,
- @DisableReverseDnsLookup Boolean disableReverseDnsLookup,
+ @EnableReverseDnsLookup Boolean enableReverseDnsLookup,
@RemotePeer Provider<SocketAddress> remotePeerProvider) {
this.authConfig = authConfig;
this.realm = realm;
@@ -158,7 +158,7 @@
this.canonicalUrl = canonicalUrl;
this.accountCache = accountCache;
this.groupBackend = groupBackend;
- this.disableReverseDnsLookup = disableReverseDnsLookup;
+ this.enableReverseDnsLookup = enableReverseDnsLookup;
this.remotePeerProvider = remotePeerProvider;
}
@@ -170,7 +170,7 @@
canonicalUrl,
accountCache,
groupBackend,
- disableReverseDnsLookup,
+ enableReverseDnsLookup,
remotePeerProvider,
id,
null);
@@ -184,7 +184,7 @@
canonicalUrl,
accountCache,
groupBackend,
- disableReverseDnsLookup,
+ enableReverseDnsLookup,
remotePeerProvider,
id,
caller);
@@ -201,7 +201,7 @@
private final Realm realm;
private final GroupBackend groupBackend;
private final String anonymousCowardName;
- private final Boolean disableReverseDnsLookup;
+ private final Boolean enableReverseDnsLookup;
private final Set<String> validEmails = Sets.newTreeSet(String.CASE_INSENSITIVE_ORDER);
private final CurrentUser realUser; // Must be final since cached properties depend on it.
@@ -221,7 +221,7 @@
Provider<String> canonicalUrl,
AccountCache accountCache,
GroupBackend groupBackend,
- Boolean disableReverseDnsLookup,
+ Boolean enableReverseDnsLookup,
@Nullable Provider<SocketAddress> remotePeerProvider,
AccountState state,
@Nullable CurrentUser realUser) {
@@ -232,7 +232,7 @@
canonicalUrl,
accountCache,
groupBackend,
- disableReverseDnsLookup,
+ enableReverseDnsLookup,
remotePeerProvider,
state.getAccount().getId(),
realUser);
@@ -246,7 +246,7 @@
Provider<String> canonicalUrl,
AccountCache accountCache,
GroupBackend groupBackend,
- Boolean disableReverseDnsLookup,
+ Boolean enableReverseDnsLookup,
@Nullable Provider<SocketAddress> remotePeerProvider,
Account.Id id,
@Nullable CurrentUser realUser) {
@@ -256,7 +256,7 @@
this.authConfig = authConfig;
this.realm = realm;
this.anonymousCowardName = anonymousCowardName;
- this.disableReverseDnsLookup = disableReverseDnsLookup;
+ this.enableReverseDnsLookup = enableReverseDnsLookup;
this.remotePeerProvider = remotePeerProvider;
this.accountId = id;
this.realUser = realUser != null ? realUser : this;
@@ -523,7 +523,7 @@
Providers.of(canonicalUrl.get()),
accountCache,
groupBackend,
- disableReverseDnsLookup,
+ enableReverseDnsLookup,
remotePeer,
state,
realUser);
@@ -554,7 +554,7 @@
}
private String getHost(InetAddress in) {
- if (Boolean.FALSE.equals(disableReverseDnsLookup)) {
+ if (Boolean.TRUE.equals(enableReverseDnsLookup)) {
return in.getCanonicalHostName();
}
return in.getHostAddress();
diff --git a/java/com/google/gerrit/server/change/SetPrivateOp.java b/java/com/google/gerrit/server/change/SetPrivateOp.java
index 41e7e31..2b00a40 100644
--- a/java/com/google/gerrit/server/change/SetPrivateOp.java
+++ b/java/com/google/gerrit/server/change/SetPrivateOp.java
@@ -45,7 +45,7 @@
}
public interface Factory {
- SetPrivateOp create(ChangeMessagesUtil cmUtil, boolean isPrivate, @Nullable Input input);
+ SetPrivateOp create(boolean isPrivate, @Nullable Input input);
}
private final PrivateStateChanged privateStateChanged;
@@ -56,12 +56,13 @@
private Change change;
private PatchSet ps;
+ private boolean isNoOp;
@Inject
SetPrivateOp(
PrivateStateChanged privateStateChanged,
PatchSetUtil psUtil,
- @Assisted ChangeMessagesUtil cmUtil,
+ ChangeMessagesUtil cmUtil,
@Assisted boolean isPrivate,
@Assisted @Nullable Input input) {
this.privateStateChanged = privateStateChanged;
@@ -75,6 +76,12 @@
public boolean updateChange(ChangeContext ctx)
throws ResourceConflictException, OrmException, BadRequestException {
change = ctx.getChange();
+ if (ctx.getChange().isPrivate() == isPrivate) {
+ // No-op
+ isNoOp = true;
+ return false;
+ }
+
if (isPrivate && !change.isNew()) {
throw new BadRequestException("cannot set a non-open change to private");
}
@@ -90,7 +97,9 @@
@Override
public void postUpdate(Context ctx) {
- privateStateChanged.fire(change, ps, ctx.getAccount(), ctx.getWhen());
+ if (!isNoOp) {
+ privateStateChanged.fire(change, ps, ctx.getAccount(), ctx.getWhen());
+ }
}
private void addMessage(ChangeContext ctx, ChangeUpdate update) {
diff --git a/java/com/google/gerrit/server/config/DisableReverseDnsLookup.java b/java/com/google/gerrit/server/config/EnableReverseDnsLookup.java
similarity index 94%
rename from java/com/google/gerrit/server/config/DisableReverseDnsLookup.java
rename to java/com/google/gerrit/server/config/EnableReverseDnsLookup.java
index 336edeb..ec57338 100644
--- a/java/com/google/gerrit/server/config/DisableReverseDnsLookup.java
+++ b/java/com/google/gerrit/server/config/EnableReverseDnsLookup.java
@@ -21,4 +21,4 @@
@Retention(RUNTIME)
@BindingAnnotation
-public @interface DisableReverseDnsLookup {}
+public @interface EnableReverseDnsLookup {}
diff --git a/java/com/google/gerrit/server/config/DisableReverseDnsLookupProvider.java b/java/com/google/gerrit/server/config/EnableReverseDnsLookupProvider.java
similarity index 71%
rename from java/com/google/gerrit/server/config/DisableReverseDnsLookupProvider.java
rename to java/com/google/gerrit/server/config/EnableReverseDnsLookupProvider.java
index 87d6bac2..71086a9 100644
--- a/java/com/google/gerrit/server/config/DisableReverseDnsLookupProvider.java
+++ b/java/com/google/gerrit/server/config/EnableReverseDnsLookupProvider.java
@@ -18,16 +18,16 @@
import com.google.inject.Provider;
import org.eclipse.jgit.lib.Config;
-public class DisableReverseDnsLookupProvider implements Provider<Boolean> {
- private final boolean disableReverseDnsLookup;
+public class EnableReverseDnsLookupProvider implements Provider<Boolean> {
+ private final Boolean enableReverseDnsLookup;
@Inject
- DisableReverseDnsLookupProvider(@GerritServerConfig Config config) {
- disableReverseDnsLookup = config.getBoolean("gerrit", null, "disableReverseDnsLookup", false);
+ EnableReverseDnsLookupProvider(@GerritServerConfig Config config) {
+ enableReverseDnsLookup = config.getBoolean("gerrit", null, "enableReverseDnsLookup", false);
}
@Override
public Boolean get() {
- return disableReverseDnsLookup;
+ return enableReverseDnsLookup;
}
}
diff --git a/java/com/google/gerrit/server/config/GerritGlobalModule.java b/java/com/google/gerrit/server/config/GerritGlobalModule.java
index 9650ac2..4158346 100644
--- a/java/com/google/gerrit/server/config/GerritGlobalModule.java
+++ b/java/com/google/gerrit/server/config/GerritGlobalModule.java
@@ -295,8 +295,8 @@
bind(SoyTofu.class).annotatedWith(MailTemplates.class).toProvider(MailSoyTofuProvider.class);
bind(FromAddressGenerator.class).toProvider(FromAddressGeneratorProvider.class).in(SINGLETON);
bind(Boolean.class)
- .annotatedWith(DisableReverseDnsLookup.class)
- .toProvider(DisableReverseDnsLookupProvider.class)
+ .annotatedWith(EnableReverseDnsLookup.class)
+ .toProvider(EnableReverseDnsLookupProvider.class)
.in(SINGLETON);
bind(PatchSetInfoFactory.class);
diff --git a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
index 8413b6d..79dd5b5 100644
--- a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
+++ b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
@@ -96,6 +96,7 @@
import com.google.gerrit.server.change.ChangeInserter;
import com.google.gerrit.server.change.NotifyResolver;
import com.google.gerrit.server.change.SetHashtagsOp;
+import com.google.gerrit.server.change.SetPrivateOp;
import com.google.gerrit.server.config.AllProjectsName;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.config.PluginConfig;
@@ -322,6 +323,7 @@
private final SubmoduleOp.Factory subOpFactory;
private final TagCache tagCache;
private final ProjectConfig.Factory projectConfigFactory;
+ private final SetPrivateOp.Factory setPrivateOpFactory;
// Assisted injected fields.
private final AllRefsWatcher allRefsWatcher;
@@ -396,6 +398,7 @@
SetHashtagsOp.Factory hashtagsFactory,
SubmoduleOp.Factory subOpFactory,
TagCache tagCache,
+ SetPrivateOp.Factory setPrivateOpFactory,
@Assisted ProjectState projectState,
@Assisted IdentifiedUser user,
@Assisted ReceivePack rp,
@@ -436,6 +439,7 @@
this.subOpFactory = subOpFactory;
this.tagCache = tagCache;
this.projectConfigFactory = projectConfigFactory;
+ this.setPrivateOpFactory = setPrivateOpFactory;
// Assisted injected fields.
this.allRefsWatcher = allRefsWatcher;
@@ -3172,6 +3176,7 @@
Optional<ChangeNotes> notes = getChangeNotes(psId.getParentKey());
if (notes.isPresent() && notes.get().getChange().getDest().equals(branch)) {
existingPatchSets++;
+ bu.addOp(notes.get().getChangeId(), setPrivateOpFactory.create(false, null));
bu.addOp(
psId.getParentKey(),
mergedByPushOpFactory.create(requestScopePropagator, psId, refName));
@@ -3204,6 +3209,7 @@
continue;
}
req.addOps(bu, null);
+ bu.addOp(id, setPrivateOpFactory.create(false, null));
bu.addOp(
id,
mergedByPushOpFactory
diff --git a/java/com/google/gerrit/server/git/validators/CommitValidators.java b/java/com/google/gerrit/server/git/validators/CommitValidators.java
index 0dd075c..36561d8 100644
--- a/java/com/google/gerrit/server/git/validators/CommitValidators.java
+++ b/java/com/google/gerrit/server/git/validators/CommitValidators.java
@@ -28,6 +28,7 @@
import com.google.gerrit.common.FooterConstants;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.extensions.api.config.ConsistencyCheckInfo.ConsistencyProblemInfo;
+import com.google.gerrit.extensions.registration.DynamicItem;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.BooleanProjectConfig;
@@ -88,7 +89,7 @@
@Singleton
public static class Factory {
private final PersonIdent gerritIdent;
- private final UrlFormatter urlFormatter;
+ private final DynamicItem<UrlFormatter> urlFormatter;
private final PluginSetContext<CommitValidationListener> pluginValidators;
private final GitRepositoryManager repoManager;
private final AllUsersName allUsers;
@@ -102,7 +103,7 @@
@Inject
Factory(
@GerritPersonIdent PersonIdent gerritIdent,
- UrlFormatter urlFormatter,
+ DynamicItem<UrlFormatter> urlFormatter,
@GerritServerConfig Config cfg,
PluginSetContext<CommitValidationListener> pluginValidators,
GitRepositoryManager repoManager,
@@ -142,11 +143,16 @@
new UploadMergesPermissionValidator(perm),
new ProjectStateValidationListener(projectState),
new AmendedGerritMergeCommitValidationListener(perm, gerritIdent),
- new AuthorUploaderValidator(user, perm, urlFormatter),
- new CommitterUploaderValidator(user, perm, urlFormatter),
+ new AuthorUploaderValidator(user, perm, urlFormatter.get()),
+ new CommitterUploaderValidator(user, perm, urlFormatter.get()),
new SignedOffByValidator(user, perm, projectState),
new ChangeIdValidator(
- projectState, user, urlFormatter, installCommitMsgHookCommand, sshInfo, change),
+ projectState,
+ user,
+ urlFormatter.get(),
+ installCommitMsgHookCommand,
+ sshInfo,
+ change),
new ConfigValidator(projectConfigFactory, branch, user, rw, allUsers, allProjects),
new BannedCommitsValidator(rejectCommits),
new PluginCommitValidationListener(pluginValidators),
@@ -170,10 +176,15 @@
new UploadMergesPermissionValidator(perm),
new ProjectStateValidationListener(projectState),
new AmendedGerritMergeCommitValidationListener(perm, gerritIdent),
- new AuthorUploaderValidator(user, perm, urlFormatter),
+ new AuthorUploaderValidator(user, perm, urlFormatter.get()),
new SignedOffByValidator(user, perm, projectCache.checkedGet(branch.getParentKey())),
new ChangeIdValidator(
- projectState, user, urlFormatter, installCommitMsgHookCommand, sshInfo, change),
+ projectState,
+ user,
+ urlFormatter.get(),
+ installCommitMsgHookCommand,
+ sshInfo,
+ change),
new ConfigValidator(projectConfigFactory, branch, user, rw, allUsers, allProjects),
new PluginCommitValidationListener(pluginValidators),
new ExternalIdUpdateListener(allUsers, externalIdsConsistencyChecker),
@@ -202,8 +213,8 @@
ImmutableList.of(
new UploadMergesPermissionValidator(perm),
new ProjectStateValidationListener(projectCache.checkedGet(branch.getParentKey())),
- new AuthorUploaderValidator(user, perm, urlFormatter),
- new CommitterUploaderValidator(user, perm, urlFormatter)));
+ new AuthorUploaderValidator(user, perm, urlFormatter.get()),
+ new CommitterUploaderValidator(user, perm, urlFormatter.get())));
}
}
diff --git a/java/com/google/gerrit/server/restapi/change/DeletePrivate.java b/java/com/google/gerrit/server/restapi/change/DeletePrivate.java
index 64ffb59..8601e68 100644
--- a/java/com/google/gerrit/server/restapi/change/DeletePrivate.java
+++ b/java/com/google/gerrit/server/restapi/change/DeletePrivate.java
@@ -22,7 +22,6 @@
import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.RestApiException;
-import com.google.gerrit.server.ChangeMessagesUtil;
import com.google.gerrit.server.change.ChangeResource;
import com.google.gerrit.server.change.SetPrivateOp;
import com.google.gerrit.server.permissions.GlobalPermission;
@@ -38,18 +37,15 @@
@Singleton
public class DeletePrivate
extends RetryingRestModifyView<ChangeResource, SetPrivateOp.Input, Response<String>> {
- private final ChangeMessagesUtil cmUtil;
private final PermissionBackend permissionBackend;
private final SetPrivateOp.Factory setPrivateOpFactory;
@Inject
DeletePrivate(
RetryHelper retryHelper,
- ChangeMessagesUtil cmUtil,
PermissionBackend permissionBackend,
SetPrivateOp.Factory setPrivateOpFactory) {
super(retryHelper);
- this.cmUtil = cmUtil;
this.permissionBackend = permissionBackend;
this.setPrivateOpFactory = setPrivateOpFactory;
}
@@ -66,7 +62,7 @@
throw new ResourceConflictException("change is not private");
}
- SetPrivateOp op = setPrivateOpFactory.create(cmUtil, false, input);
+ SetPrivateOp op = setPrivateOpFactory.create(false, input);
try (BatchUpdate u =
updateFactory.create(rsrc.getProject(), rsrc.getUser(), TimeUtil.nowTs())) {
u.addOp(rsrc.getId(), op).execute();
diff --git a/java/com/google/gerrit/server/restapi/change/DeletePrivateByPost.java b/java/com/google/gerrit/server/restapi/change/DeletePrivateByPost.java
index cfd9e16..c86d0ca 100644
--- a/java/com/google/gerrit/server/restapi/change/DeletePrivateByPost.java
+++ b/java/com/google/gerrit/server/restapi/change/DeletePrivateByPost.java
@@ -17,7 +17,6 @@
import static com.google.gerrit.extensions.conditions.BooleanCondition.and;
import com.google.gerrit.extensions.webui.UiAction;
-import com.google.gerrit.server.ChangeMessagesUtil;
import com.google.gerrit.server.change.ChangeResource;
import com.google.gerrit.server.change.SetPrivateOp;
import com.google.gerrit.server.permissions.PermissionBackend;
@@ -30,10 +29,9 @@
@Inject
DeletePrivateByPost(
RetryHelper retryHelper,
- ChangeMessagesUtil cmUtil,
PermissionBackend permissionBackend,
SetPrivateOp.Factory setPrivateOpFactory) {
- super(retryHelper, cmUtil, permissionBackend, setPrivateOpFactory);
+ super(retryHelper, permissionBackend, setPrivateOpFactory);
}
@Override
diff --git a/java/com/google/gerrit/server/restapi/change/PostPrivate.java b/java/com/google/gerrit/server/restapi/change/PostPrivate.java
index bdfdd21..5aa2ecc 100644
--- a/java/com/google/gerrit/server/restapi/change/PostPrivate.java
+++ b/java/com/google/gerrit/server/restapi/change/PostPrivate.java
@@ -24,7 +24,6 @@
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.webui.UiAction;
import com.google.gerrit.reviewdb.client.Change;
-import com.google.gerrit.server.ChangeMessagesUtil;
import com.google.gerrit.server.change.ChangeResource;
import com.google.gerrit.server.change.SetPrivateOp;
import com.google.gerrit.server.config.GerritServerConfig;
@@ -43,7 +42,6 @@
public class PostPrivate
extends RetryingRestModifyView<ChangeResource, SetPrivateOp.Input, Response<String>>
implements UiAction<ChangeResource> {
- private final ChangeMessagesUtil cmUtil;
private final PermissionBackend permissionBackend;
private final SetPrivateOp.Factory setPrivateOpFactory;
private final boolean disablePrivateChanges;
@@ -51,12 +49,10 @@
@Inject
PostPrivate(
RetryHelper retryHelper,
- ChangeMessagesUtil cmUtil,
PermissionBackend permissionBackend,
SetPrivateOp.Factory setPrivateOpFactory,
@GerritServerConfig Config config) {
super(retryHelper);
- this.cmUtil = cmUtil;
this.permissionBackend = permissionBackend;
this.setPrivateOpFactory = setPrivateOpFactory;
this.disablePrivateChanges = config.getBoolean("change", null, "disablePrivateChanges", false);
@@ -78,7 +74,7 @@
return Response.ok("");
}
- SetPrivateOp op = setPrivateOpFactory.create(cmUtil, true, input);
+ SetPrivateOp op = setPrivateOpFactory.create(true, input);
try (BatchUpdate u =
updateFactory.create(rsrc.getProject(), rsrc.getUser(), TimeUtil.nowTs())) {
u.addOp(rsrc.getId(), op).execute();
diff --git a/java/com/google/gerrit/server/restapi/project/ListProjects.java b/java/com/google/gerrit/server/restapi/project/ListProjects.java
index 4bf1230..4c55ff5 100644
--- a/java/com/google/gerrit/server/restapi/project/ListProjects.java
+++ b/java/com/google/gerrit/server/restapi/project/ListProjects.java
@@ -44,6 +44,7 @@
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.WebLinks;
import com.google.gerrit.server.account.GroupControl;
+import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.group.GroupResolver;
import com.google.gerrit.server.ioutil.RegexListSearcher;
@@ -82,6 +83,7 @@
import java.util.stream.Stream;
import java.util.stream.StreamSupport;
import org.eclipse.jgit.errors.RepositoryNotFoundException;
+import org.eclipse.jgit.lib.Config;
import org.eclipse.jgit.lib.Constants;
import org.eclipse.jgit.lib.Ref;
import org.eclipse.jgit.lib.Repository;
@@ -253,6 +255,7 @@
private String matchRegex;
private AccountGroup.UUID groupUuid;
private final Provider<QueryProjects> queryProjectsProvider;
+ private final boolean listProjectsFromIndex;
@Inject
protected ListProjects(
@@ -264,7 +267,8 @@
PermissionBackend permissionBackend,
ProjectNode.Factory projectNodeFactory,
WebLinks webLinks,
- Provider<QueryProjects> queryProjectsProvider) {
+ Provider<QueryProjects> queryProjectsProvider,
+ @GerritServerConfig Config config) {
this.currentUser = currentUser;
this.projectCache = projectCache;
this.groupResolver = groupResolver;
@@ -274,6 +278,7 @@
this.projectNodeFactory = projectNodeFactory;
this.webLinks = webLinks;
this.queryProjectsProvider = queryProjectsProvider;
+ this.listProjectsFromIndex = config.getBoolean("gerrit", "listProjectsFromIndex", false);
}
public List<String> getShowBranch() {
@@ -322,7 +327,8 @@
}
private Optional<String> expressAsProjectsQuery() {
- return !all
+ return listProjectsFromIndex
+ && !all
&& state != HIDDEN
&& isNullOrEmpty(matchPrefix)
&& isNullOrEmpty(matchRegex)
diff --git a/java/com/google/gerrit/server/submit/SubmitStrategy.java b/java/com/google/gerrit/server/submit/SubmitStrategy.java
index 37858d5..dc221f8 100644
--- a/java/com/google/gerrit/server/submit/SubmitStrategy.java
+++ b/java/com/google/gerrit/server/submit/SubmitStrategy.java
@@ -30,6 +30,7 @@
import com.google.gerrit.server.account.AccountCache;
import com.google.gerrit.server.change.LabelNormalizer;
import com.google.gerrit.server.change.RebaseChangeOp;
+import com.google.gerrit.server.change.SetPrivateOp;
import com.google.gerrit.server.change.TestSubmitInput;
import com.google.gerrit.server.extensions.events.ChangeMerged;
import com.google.gerrit.server.git.CodeReviewCommit;
@@ -111,6 +112,7 @@
final TagCache tagCache;
final Provider<InternalChangeQuery> queryProvider;
final ProjectConfig.Factory projectConfigFactory;
+ final SetPrivateOp.Factory setPrivateOpFactory;
final Branch.NameKey destBranch;
final CodeReviewRevWalk rw;
@@ -149,6 +151,7 @@
TagCache tagCache,
Provider<InternalChangeQuery> queryProvider,
ProjectConfig.Factory projectConfigFactory,
+ SetPrivateOp.Factory setPrivateOpFactory,
@Assisted Branch.NameKey destBranch,
@Assisted CommitStatus commitStatus,
@Assisted CodeReviewRevWalk rw,
@@ -176,6 +179,7 @@
this.rebaseFactory = rebaseFactory;
this.tagCache = tagCache;
this.queryProvider = queryProvider;
+ this.setPrivateOpFactory = setPrivateOpFactory;
this.serverIdent = serverIdent;
this.destBranch = destBranch;
@@ -244,12 +248,14 @@
Collections.reverse(difference);
for (CodeReviewCommit c : difference) {
Change.Id id = c.change().getId();
+ bu.addOp(id, args.setPrivateOpFactory.create(false, null));
bu.addOp(id, new ImplicitIntegrateOp(args, c));
maybeAddTestHelperOp(bu, id);
}
// Then ops for explicitly merged changes
for (SubmitStrategyOp op : ops) {
+ bu.addOp(op.getId(), args.setPrivateOpFactory.create(false, null));
bu.addOp(op.getId(), op);
maybeAddTestHelperOp(bu, op.getId());
}
diff --git a/javatests/com/google/gerrit/acceptance/api/accounts/AccountIT.java b/javatests/com/google/gerrit/acceptance/api/accounts/AccountIT.java
index 06f7dbd..d723a0f 100644
--- a/javatests/com/google/gerrit/acceptance/api/accounts/AccountIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/accounts/AccountIT.java
@@ -2850,7 +2850,9 @@
.isEqualTo(Fingerprint.toString(expected.getPublicKey().getFingerprint()));
List<String> userIds = ImmutableList.copyOf(expected.getPublicKey().getUserIDs());
assertThat(actual.userIds).named(id).containsExactlyElementsIn(userIds);
- assertThat(actual.key).named(id).startsWith("-----BEGIN PGP PUBLIC KEY BLOCK-----\n");
+ String key = actual.key;
+ assertThat(key).named(id).startsWith("-----BEGIN PGP PUBLIC KEY BLOCK-----\n");
+ assertThat(key).named(id).endsWith("-----END PGP PUBLIC KEY BLOCK-----\n");
assertThat(actual.status).isEqualTo(GpgKeyInfo.Status.TRUSTED);
assertThat(actual.problems).isEmpty();
}
diff --git a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
index c4e74ee..88f090f 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
@@ -271,165 +271,6 @@
}
@Test
- public void setPrivateByOwner() throws Exception {
- TestRepository<InMemoryRepository> userRepo = cloneProject(project, user);
- PushOneCommit.Result result =
- pushFactory.create(user.getIdent(), userRepo).to("refs/for/master");
-
- requestScopeOperations.setApiUser(user.getId());
- String changeId = result.getChangeId();
- assertThat(gApi.changes().id(changeId).get().isPrivate).isNull();
-
- gApi.changes().id(changeId).setPrivate(true, null);
- ChangeInfo info = gApi.changes().id(changeId).get();
- assertThat(info.isPrivate).isTrue();
- assertThat(Iterables.getLast(info.messages).message).isEqualTo("Set private");
- assertThat(Iterables.getLast(info.messages).tag).contains(ChangeMessagesUtil.TAG_SET_PRIVATE);
-
- gApi.changes().id(changeId).setPrivate(false, null);
- info = gApi.changes().id(changeId).get();
- assertThat(info.isPrivate).isNull();
- assertThat(Iterables.getLast(info.messages).message).isEqualTo("Unset private");
- assertThat(Iterables.getLast(info.messages).tag).contains(ChangeMessagesUtil.TAG_UNSET_PRIVATE);
-
- String msg = "This is a security fix that must not be public.";
- gApi.changes().id(changeId).setPrivate(true, msg);
- info = gApi.changes().id(changeId).get();
- assertThat(info.isPrivate).isTrue();
- assertThat(Iterables.getLast(info.messages).message).isEqualTo("Set private\n\n" + msg);
- assertThat(Iterables.getLast(info.messages).tag).contains(ChangeMessagesUtil.TAG_SET_PRIVATE);
-
- msg = "After this security fix has been released we can make it public now.";
- gApi.changes().id(changeId).setPrivate(false, msg);
- info = gApi.changes().id(changeId).get();
- assertThat(info.isPrivate).isNull();
- assertThat(Iterables.getLast(info.messages).message).isEqualTo("Unset private\n\n" + msg);
- assertThat(Iterables.getLast(info.messages).tag).contains(ChangeMessagesUtil.TAG_UNSET_PRIVATE);
- }
-
- @Test
- public void setMergedChangePrivate() throws Exception {
- PushOneCommit.Result result = createChange();
- approve(result.getChangeId());
- merge(result);
-
- String changeId = result.getChangeId();
- assertThat(gApi.changes().id(changeId).get().isPrivate).isNull();
-
- exception.expect(BadRequestException.class);
- exception.expectMessage("cannot set a non-open change to private");
- gApi.changes().id(changeId).setPrivate(true);
- }
-
- @Test
- public void administratorCanSetUserChangePrivate() throws Exception {
- TestRepository<InMemoryRepository> userRepo = cloneProject(project, user);
- PushOneCommit.Result result =
- pushFactory.create(user.getIdent(), userRepo).to("refs/for/master");
-
- String changeId = result.getChangeId();
- assertThat(gApi.changes().id(changeId).get().isPrivate).isNull();
-
- gApi.changes().id(changeId).setPrivate(true, null);
- requestScopeOperations.setApiUser(user.getId());
- ChangeInfo info = gApi.changes().id(changeId).get();
- assertThat(info.isPrivate).isTrue();
- }
-
- @Test
- public void cannotSetOtherUsersChangePrivate() throws Exception {
- PushOneCommit.Result result = createChange();
- requestScopeOperations.setApiUser(user.getId());
- exception.expect(AuthException.class);
- exception.expectMessage("not allowed to mark private");
- gApi.changes().id(result.getChangeId()).setPrivate(true, null);
- }
-
- @Test
- public void accessPrivate() throws Exception {
- TestRepository<InMemoryRepository> userRepo = cloneProject(project, user);
- PushOneCommit.Result result =
- pushFactory.create(user.getIdent(), userRepo).to("refs/for/master");
-
- requestScopeOperations.setApiUser(user.getId());
- gApi.changes().id(result.getChangeId()).setPrivate(true, null);
- // Owner can always access its private changes.
- assertThat(gApi.changes().id(result.getChangeId()).get().isPrivate).isTrue();
-
- // Add admin as a reviewer.
- gApi.changes().id(result.getChangeId()).addReviewer(admin.getId().toString());
-
- // This change should be visible for admin as a reviewer.
- requestScopeOperations.setApiUser(admin.getId());
- assertThat(gApi.changes().id(result.getChangeId()).get().isPrivate).isTrue();
-
- // Remove admin from reviewers.
- gApi.changes().id(result.getChangeId()).reviewer(admin.getId().toString()).remove();
-
- // This change should not be visible for admin anymore.
- exception.expect(ResourceNotFoundException.class);
- exception.expectMessage("Not found: " + result.getChangeId());
- gApi.changes().id(result.getChangeId());
- }
-
- @Test
- public void privateChangeOfOtherUserCanBeAccessedWithPermission() throws Exception {
- PushOneCommit.Result result = createChange();
- gApi.changes().id(result.getChangeId()).setPrivate(true, null);
-
- allow("refs/*", Permission.VIEW_PRIVATE_CHANGES, REGISTERED_USERS);
- requestScopeOperations.setApiUser(user.getId());
- assertThat(gApi.changes().id(result.getChangeId()).get().isPrivate).isTrue();
- }
-
- @Test
- public void administratorCanUnmarkPrivateAfterMerging() throws Exception {
- PushOneCommit.Result result = createChange();
- String changeId = result.getChangeId();
- gApi.changes().id(changeId).setPrivate(true, null);
- assertThat(gApi.changes().id(changeId).get().isPrivate).isTrue();
- merge(result);
- gApi.changes().id(changeId).setPrivate(false, null);
- assertThat(gApi.changes().id(changeId).get().isPrivate).isNull();
- }
-
- @Test
- public void ownerCannotMarkPrivateAfterMerging() throws Exception {
- TestRepository<InMemoryRepository> userRepo = cloneProject(project, user);
- PushOneCommit.Result result =
- pushFactory.create(user.getIdent(), userRepo).to("refs/for/master");
-
- String changeId = result.getChangeId();
- assertThat(gApi.changes().id(changeId).get().isPrivate).isNull();
-
- merge(result);
-
- requestScopeOperations.setApiUser(user.getId());
- exception.expect(AuthException.class);
- exception.expectMessage("not allowed to mark private");
- gApi.changes().id(changeId).setPrivate(true, null);
- }
-
- @Test
- public void ownerCanUnmarkPrivateAfterMerging() throws Exception {
- TestRepository<InMemoryRepository> userRepo = cloneProject(project, user);
- PushOneCommit.Result result =
- pushFactory.create(user.getIdent(), userRepo).to("refs/for/master");
-
- String changeId = result.getChangeId();
- assertThat(gApi.changes().id(changeId).get().isPrivate).isNull();
- gApi.changes().id(changeId).addReviewer(admin.getId().toString());
- gApi.changes().id(changeId).setPrivate(true, null);
- assertThat(gApi.changes().id(changeId).get().isPrivate).isTrue();
-
- merge(result);
-
- requestScopeOperations.setApiUser(user.getId());
- gApi.changes().id(changeId).setPrivate(false, null);
- assertThat(gApi.changes().id(changeId).get().isPrivate).isNull();
- }
-
- @Test
public void setWorkInProgressNotAllowedWithoutPermission() throws Exception {
PushOneCommit.Result rwip = createChange();
String changeId = rwip.getChangeId();
diff --git a/javatests/com/google/gerrit/acceptance/api/change/PrivateChangeIT.java b/javatests/com/google/gerrit/acceptance/api/change/PrivateChangeIT.java
new file mode 100644
index 0000000..7ccf9a9c
--- /dev/null
+++ b/javatests/com/google/gerrit/acceptance/api/change/PrivateChangeIT.java
@@ -0,0 +1,248 @@
+// Copyright (C) 2019 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.acceptance.api.change;
+
+import static com.google.common.truth.Truth.assertThat;
+import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
+
+import com.google.common.collect.Iterables;
+import com.google.gerrit.acceptance.AbstractDaemonTest;
+import com.google.gerrit.acceptance.PushOneCommit;
+import com.google.gerrit.acceptance.testsuite.request.RequestScopeOperations;
+import com.google.gerrit.common.data.Permission;
+import com.google.gerrit.extensions.client.ChangeStatus;
+import com.google.gerrit.extensions.common.ChangeInfo;
+import com.google.gerrit.extensions.restapi.AuthException;
+import com.google.gerrit.extensions.restapi.BadRequestException;
+import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
+import com.google.gerrit.reviewdb.client.Change;
+import com.google.gerrit.server.ChangeMessagesUtil;
+import com.google.gerrit.server.notedb.ChangeUpdate;
+import com.google.gerrit.server.update.BatchUpdate;
+import com.google.gerrit.server.update.BatchUpdateOp;
+import com.google.gerrit.server.update.ChangeContext;
+import com.google.gerrit.server.util.time.TimeUtil;
+import com.google.inject.Inject;
+import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository;
+import org.eclipse.jgit.junit.TestRepository;
+import org.junit.Test;
+
+public class PrivateChangeIT extends AbstractDaemonTest {
+
+ @Inject private RequestScopeOperations requestScopeOperations;
+
+ @Test
+ public void setPrivateByOwner() throws Exception {
+ TestRepository<InMemoryRepository> userRepo = cloneProject(project, user);
+ PushOneCommit.Result result =
+ pushFactory.create(user.getIdent(), userRepo).to("refs/for/master");
+
+ requestScopeOperations.setApiUser(user.getId());
+ String changeId = result.getChangeId();
+ assertThat(gApi.changes().id(changeId).get().isPrivate).isNull();
+
+ gApi.changes().id(changeId).setPrivate(true, null);
+ ChangeInfo info = gApi.changes().id(changeId).get();
+ assertThat(info.isPrivate).isTrue();
+ assertThat(Iterables.getLast(info.messages).message).isEqualTo("Set private");
+ assertThat(Iterables.getLast(info.messages).tag).contains(ChangeMessagesUtil.TAG_SET_PRIVATE);
+
+ gApi.changes().id(changeId).setPrivate(false, null);
+ info = gApi.changes().id(changeId).get();
+ assertThat(info.isPrivate).isNull();
+ assertThat(Iterables.getLast(info.messages).message).isEqualTo("Unset private");
+ assertThat(Iterables.getLast(info.messages).tag).contains(ChangeMessagesUtil.TAG_UNSET_PRIVATE);
+
+ String msg = "This is a security fix that must not be public.";
+ gApi.changes().id(changeId).setPrivate(true, msg);
+ info = gApi.changes().id(changeId).get();
+ assertThat(info.isPrivate).isTrue();
+ assertThat(Iterables.getLast(info.messages).message).isEqualTo("Set private\n\n" + msg);
+ assertThat(Iterables.getLast(info.messages).tag).contains(ChangeMessagesUtil.TAG_SET_PRIVATE);
+
+ msg = "After this security fix has been released we can make it public now.";
+ gApi.changes().id(changeId).setPrivate(false, msg);
+ info = gApi.changes().id(changeId).get();
+ assertThat(info.isPrivate).isNull();
+ assertThat(Iterables.getLast(info.messages).message).isEqualTo("Unset private\n\n" + msg);
+ assertThat(Iterables.getLast(info.messages).tag).contains(ChangeMessagesUtil.TAG_UNSET_PRIVATE);
+ }
+
+ @Test
+ public void setMergedChangePrivate() throws Exception {
+ PushOneCommit.Result result = createChange();
+ approve(result.getChangeId());
+ merge(result);
+
+ String changeId = result.getChangeId();
+ assertThat(gApi.changes().id(changeId).get().isPrivate).isNull();
+
+ exception.expect(BadRequestException.class);
+ exception.expectMessage("cannot set a non-open change to private");
+ gApi.changes().id(changeId).setPrivate(true);
+ }
+
+ @Test
+ public void administratorCanSetUserChangePrivate() throws Exception {
+ TestRepository<InMemoryRepository> userRepo = cloneProject(project, user);
+ PushOneCommit.Result result =
+ pushFactory.create(user.getIdent(), userRepo).to("refs/for/master");
+
+ String changeId = result.getChangeId();
+ assertThat(gApi.changes().id(changeId).get().isPrivate).isNull();
+
+ gApi.changes().id(changeId).setPrivate(true, null);
+ requestScopeOperations.setApiUser(user.getId());
+ ChangeInfo info = gApi.changes().id(changeId).get();
+ assertThat(info.isPrivate).isTrue();
+ }
+
+ @Test
+ public void cannotSetOtherUsersChangePrivate() throws Exception {
+ PushOneCommit.Result result = createChange();
+ requestScopeOperations.setApiUser(user.getId());
+ exception.expect(AuthException.class);
+ exception.expectMessage("not allowed to mark private");
+ gApi.changes().id(result.getChangeId()).setPrivate(true, null);
+ }
+
+ @Test
+ public void accessPrivate() throws Exception {
+ TestRepository<InMemoryRepository> userRepo = cloneProject(project, user);
+ PushOneCommit.Result result =
+ pushFactory.create(user.getIdent(), userRepo).to("refs/for/master");
+
+ requestScopeOperations.setApiUser(user.getId());
+ gApi.changes().id(result.getChangeId()).setPrivate(true, null);
+ // Owner can always access its private changes.
+ assertThat(gApi.changes().id(result.getChangeId()).get().isPrivate).isTrue();
+
+ // Add admin as a reviewer.
+ gApi.changes().id(result.getChangeId()).addReviewer(admin.getId().toString());
+
+ // This change should be visible for admin as a reviewer.
+ requestScopeOperations.setApiUser(admin.getId());
+ assertThat(gApi.changes().id(result.getChangeId()).get().isPrivate).isTrue();
+
+ // Remove admin from reviewers.
+ gApi.changes().id(result.getChangeId()).reviewer(admin.getId().toString()).remove();
+
+ // This change should not be visible for admin anymore.
+ exception.expect(ResourceNotFoundException.class);
+ exception.expectMessage("Not found: " + result.getChangeId());
+ gApi.changes().id(result.getChangeId());
+ }
+
+ @Test
+ public void privateChangeOfOtherUserCanBeAccessedWithPermission() throws Exception {
+ PushOneCommit.Result result = createChange();
+ gApi.changes().id(result.getChangeId()).setPrivate(true, null);
+
+ allow("refs/*", Permission.VIEW_PRIVATE_CHANGES, REGISTERED_USERS);
+ requestScopeOperations.setApiUser(user.getId());
+ assertThat(gApi.changes().id(result.getChangeId()).get().isPrivate).isTrue();
+ }
+
+ @Test
+ public void administratorCanUnmarkPrivateAfterMerging() throws Exception {
+ PushOneCommit.Result result = createChange();
+ String changeId = result.getChangeId();
+ merge(result);
+ markMergedChangePrivate(new Change.Id(gApi.changes().id(changeId).get()._number));
+
+ gApi.changes().id(changeId).setPrivate(false, null);
+ assertThat(gApi.changes().id(changeId).get().isPrivate).isNull();
+ }
+
+ @Test
+ public void ownerCannotMarkPrivateAfterMerging() throws Exception {
+ TestRepository<InMemoryRepository> userRepo = cloneProject(project, user);
+ PushOneCommit.Result result =
+ pushFactory.create(user.getIdent(), userRepo).to("refs/for/master");
+
+ String changeId = result.getChangeId();
+ assertThat(gApi.changes().id(changeId).get().isPrivate).isNull();
+
+ merge(result);
+
+ requestScopeOperations.setApiUser(user.getId());
+ exception.expect(AuthException.class);
+ exception.expectMessage("not allowed to mark private");
+ gApi.changes().id(changeId).setPrivate(true, null);
+ }
+
+ @Test
+ public void mergingPrivateChangePublishesIt() throws Exception {
+ PushOneCommit.Result result = createChange();
+ gApi.changes().id(result.getChangeId()).setPrivate(true);
+ assertThat(gApi.changes().id(result.getChangeId()).get().isPrivate).isTrue();
+
+ approve(result.getChangeId());
+ merge(result);
+
+ assertThat(gApi.changes().id(result.getChangeId()).get().status).isEqualTo(ChangeStatus.MERGED);
+ assertThat(gApi.changes().id(result.getChangeId()).get().isPrivate).isNull();
+ }
+
+ @Test
+ public void ownerCanUnmarkPrivateAfterMerging() throws Exception {
+ TestRepository<InMemoryRepository> userRepo = cloneProject(project, user);
+ PushOneCommit.Result result =
+ pushFactory.create(user.getIdent(), userRepo).to("refs/for/master");
+
+ String changeId = result.getChangeId();
+ gApi.changes().id(changeId).addReviewer(admin.getId().toString());
+ merge(result);
+ markMergedChangePrivate(new Change.Id(gApi.changes().id(changeId).get()._number));
+
+ requestScopeOperations.setApiUser(user.getId());
+ gApi.changes().id(changeId).setPrivate(false, null);
+ assertThat(gApi.changes().id(changeId).get().isPrivate).isNull();
+ }
+
+ @Test
+ public void mergingPrivateChangeThroughGitPublishesIt() throws Exception {
+ PushOneCommit.Result r = createChange();
+ gApi.changes().id(r.getChangeId()).setPrivate(true);
+
+ PushOneCommit push = pushFactory.create(admin.getIdent(), testRepo);
+ PushOneCommit.Result result = push.to("refs/heads/master");
+ result.assertOkStatus();
+
+ assertThat(gApi.changes().id(r.getChangeId()).get().isPrivate).isNull();
+ }
+
+ private void markMergedChangePrivate(Change.Id changeId) throws Exception {
+ try (BatchUpdate u =
+ batchUpdateFactory.create(
+ project, identifiedUserFactory.create(admin.id), TimeUtil.nowTs())) {
+ u.addOp(
+ changeId,
+ new BatchUpdateOp() {
+ @Override
+ public boolean updateChange(ChangeContext ctx) {
+ ctx.getChange().setPrivate(true);
+ ChangeUpdate update = ctx.getUpdate(ctx.getChange().currentPatchSetId());
+ ctx.getChange().setPrivate(true);
+ ctx.getChange().setLastUpdatedOn(ctx.getWhen());
+ update.setPrivate(true);
+ return true;
+ }
+ })
+ .execute();
+ }
+ assertThat(gApi.changes().id(changeId.get()).get().isPrivate).isTrue();
+ }
+}
diff --git a/javatests/com/google/gerrit/acceptance/rest/account/EmailIT.java b/javatests/com/google/gerrit/acceptance/rest/account/EmailIT.java
index 9749d67..0bdb4e4 100644
--- a/javatests/com/google/gerrit/acceptance/rest/account/EmailIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/account/EmailIT.java
@@ -43,7 +43,7 @@
import com.google.gerrit.server.config.AnonymousCowardName;
import com.google.gerrit.server.config.AuthConfig;
import com.google.gerrit.server.config.CanonicalWebUrl;
-import com.google.gerrit.server.config.DisableReverseDnsLookup;
+import com.google.gerrit.server.config.EnableReverseDnsLookup;
import com.google.gson.reflect.TypeToken;
import com.google.inject.Inject;
import com.google.inject.Provider;
@@ -55,7 +55,7 @@
public class EmailIT extends AbstractDaemonTest {
@Inject private @AnonymousCowardName String anonymousCowardName;
@Inject private @CanonicalWebUrl Provider<String> canonicalUrl;
- @Inject private @DisableReverseDnsLookup Boolean disableReverseDnsLookup;
+ @Inject private @EnableReverseDnsLookup boolean enableReverseDnsLookup;
@Inject private @ServerInitiated Provider<AccountsUpdate> accountsUpdateProvider;
@Inject private AuthConfig authConfig;
@Inject private EmailExpander emailExpander;
@@ -275,7 +275,7 @@
realm,
anonymousCowardName,
canonicalUrl,
- disableReverseDnsLookup,
+ enableReverseDnsLookup,
accountCache,
groupBackend);
return atrScope.set(atrScope.newContext(null, userFactory.create(admin.id)));
diff --git a/javatests/com/google/gerrit/acceptance/rest/project/ListProjectsIT.java b/javatests/com/google/gerrit/acceptance/rest/project/ListProjectsIT.java
index a30e2b9..f2c369e 100644
--- a/javatests/com/google/gerrit/acceptance/rest/project/ListProjectsIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/project/ListProjectsIT.java
@@ -23,6 +23,7 @@
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables;
import com.google.gerrit.acceptance.AbstractDaemonTest;
+import com.google.gerrit.acceptance.GerritConfig;
import com.google.gerrit.acceptance.NoHttpd;
import com.google.gerrit.acceptance.Sandboxed;
import com.google.gerrit.acceptance.TestProjectInput;
@@ -121,6 +122,21 @@
}
@Test
+ @GerritConfig(name = "gerrit.listProjectsFromIndex", value = "true")
+ public void listProjectsFromIndexShouldBeLimitedTo500() throws Exception {
+ int numTestProjects = 501;
+ assertThat(createProjects("foo", numTestProjects)).hasSize(numTestProjects);
+ assertThat(gApi.projects().list().get()).hasSize(500);
+ }
+
+ @Test
+ public void listProjectsShouldNotBeLimitedByDefault() throws Exception {
+ int numTestProjects = 501;
+ assertThat(createProjects("foo", numTestProjects)).hasSize(numTestProjects);
+ assertThat(gApi.projects().list().get().size()).isAtLeast(numTestProjects);
+ }
+
+ @Test
public void listProjectsToOutputStream() throws Exception {
int numInitialProjects = gApi.projects().list().get().size();
int numTestProjects = 5;
diff --git a/javatests/com/google/gerrit/server/IdentifiedUserTest.java b/javatests/com/google/gerrit/server/IdentifiedUserTest.java
index a8daac3..485de49 100644
--- a/javatests/com/google/gerrit/server/IdentifiedUserTest.java
+++ b/javatests/com/google/gerrit/server/IdentifiedUserTest.java
@@ -25,7 +25,7 @@
import com.google.gerrit.server.config.AnonymousCowardName;
import com.google.gerrit.server.config.AnonymousCowardNameProvider;
import com.google.gerrit.server.config.CanonicalWebUrl;
-import com.google.gerrit.server.config.DisableReverseDnsLookup;
+import com.google.gerrit.server.config.EnableReverseDnsLookup;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.group.SystemGroupBackend;
import com.google.gerrit.server.util.time.TimeUtil;
@@ -81,8 +81,8 @@
@Override
protected void configure() {
bind(Boolean.class)
- .annotatedWith(DisableReverseDnsLookup.class)
- .toInstance(Boolean.FALSE);
+ .annotatedWith(EnableReverseDnsLookup.class)
+ .toInstance(Boolean.TRUE);
bind(Config.class).annotatedWith(GerritServerConfig.class).toInstance(config);
bind(String.class)
.annotatedWith(AnonymousCowardName.class)
diff --git a/javatests/com/google/gerrit/server/notedb/AbstractChangeNotesTest.java b/javatests/com/google/gerrit/server/notedb/AbstractChangeNotesTest.java
index be523d8..712449e 100644
--- a/javatests/com/google/gerrit/server/notedb/AbstractChangeNotesTest.java
+++ b/javatests/com/google/gerrit/server/notedb/AbstractChangeNotesTest.java
@@ -42,7 +42,7 @@
import com.google.gerrit.server.config.AnonymousCowardNameProvider;
import com.google.gerrit.server.config.CanonicalWebUrl;
import com.google.gerrit.server.config.DefaultUrlFormatter;
-import com.google.gerrit.server.config.DisableReverseDnsLookup;
+import com.google.gerrit.server.config.EnableReverseDnsLookup;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.config.GerritServerId;
import com.google.gerrit.server.extensions.events.GitReferenceUpdated;
@@ -147,8 +147,8 @@
.annotatedWith(CanonicalWebUrl.class)
.toInstance("http://localhost:8080/");
bind(Boolean.class)
- .annotatedWith(DisableReverseDnsLookup.class)
- .toInstance(Boolean.FALSE);
+ .annotatedWith(EnableReverseDnsLookup.class)
+ .toInstance(Boolean.TRUE);
bind(Realm.class).to(FakeRealm.class);
bind(GroupBackend.class).to(SystemGroupBackend.class).in(SINGLETON);
bind(AccountCache.class).toInstance(accountCache);
diff --git a/plugins/hooks b/plugins/hooks
index 9d0ad9a..84a8a13 160000
--- a/plugins/hooks
+++ b/plugins/hooks
@@ -1 +1 @@
-Subproject commit 9d0ad9ae5667b7da5bb3e7e8066d2dbff446d70b
+Subproject commit 84a8a1376c5ba1275a5a2938fca573419895b431
diff --git a/polygerrit-ui/app/elements/admin/gr-group-members/gr-group-members.js b/polygerrit-ui/app/elements/admin/gr-group-members/gr-group-members.js
index c698617..2e36a38 100644
--- a/polygerrit-ui/app/elements/admin/gr-group-members/gr-group-members.js
+++ b/polygerrit-ui/app/elements/admin/gr-group-members/gr-group-members.js
@@ -199,7 +199,7 @@
_handleSavingIncludedGroups() {
return this.$.restAPI.saveIncludedGroup(this._groupName,
- this._includedGroupSearchId, err => {
+ this._includedGroupSearchId.replace(/\+/g, ' '), err => {
if (err.status === 404) {
this.dispatchEvent(new CustomEvent('show-alert', {
detail: {message: SAVING_ERROR_TEXT},
@@ -223,7 +223,7 @@
},
_handleDeleteIncludedGroup(e) {
- const id = decodeURIComponent(e.model.get('item.id'));
+ const id = decodeURIComponent(e.model.get('item.id')).replace(/\+/g, ' ');
const name = e.model.get('item.name');
const item = name || id;
if (!item) { return ''; }
diff --git a/polygerrit-ui/app/elements/admin/gr-permission/gr-permission.js b/polygerrit-ui/app/elements/admin/gr-permission/gr-permission.js
index afe5a86..c45ffd5 100644
--- a/polygerrit-ui/app/elements/admin/gr-permission/gr-permission.js
+++ b/polygerrit-ui/app/elements/admin/gr-permission/gr-permission.js
@@ -249,7 +249,7 @@
_handleAddRuleItem(e) {
// The group id is encoded, but have to decode in order for the access
// API to work as expected.
- const groupId = decodeURIComponent(e.detail.value.id);
+ const groupId = decodeURIComponent(e.detail.value.id).replace(/\+/g, ' ');
this.set(['permission', 'value', 'rules', groupId], {});
// Purposely don't recompute sorted array so that the newly added rule
diff --git a/polygerrit-ui/app/elements/admin/gr-permission/gr-permission_test.html b/polygerrit-ui/app/elements/admin/gr-permission/gr-permission_test.html
index a6381d1..5f1f9b5 100644
--- a/polygerrit-ui/app/elements/admin/gr-permission/gr-permission_test.html
+++ b/polygerrit-ui/app/elements/admin/gr-permission/gr-permission_test.html
@@ -310,11 +310,11 @@
element.name = 'Priority';
element.section = 'refs/*';
element.groups = {};
- element.$.groupAutocomplete.text = 'new group name';
+ element.$.groupAutocomplete.text = 'ldap/tests tests';
const e = {
detail: {
value: {
- id: 'newUserGroupId',
+ id: 'ldap:CN=test+test',
},
},
};
@@ -323,11 +323,11 @@
assert.equal(Object.keys(element._groupsWithRules).length, 2);
element._handleAddRuleItem(e);
flushAsynchronousOperations();
- assert.deepEqual(element.groups, {newUserGroupId: {
- name: 'new group name'}});
+ assert.deepEqual(element.groups, {'ldap:CN=test test': {
+ name: 'ldap/tests tests'}});
assert.equal(element._rules.length, 3);
assert.equal(Object.keys(element._groupsWithRules).length, 3);
- assert.deepEqual(element.permission.value.rules['newUserGroupId'],
+ assert.deepEqual(element.permission.value.rules['ldap:CN=test test'],
{action: 'ALLOW', min: -2, max: 2, added: true});
// New rule should be removed if cancel from editing.
element.editing = false;
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html
index fbecb30..fc0cab0 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html
@@ -200,6 +200,7 @@
}
.collapseToggleContainer {
display: flex;
+ margin-bottom: 8px;
}
#relatedChangesToggle {
display: none;
diff --git a/polygerrit-ui/app/elements/change/gr-confirm-submit-dialog/gr-confirm-submit-dialog.html b/polygerrit-ui/app/elements/change/gr-confirm-submit-dialog/gr-confirm-submit-dialog.html
index 346bdd0..1036b7f 100644
--- a/polygerrit-ui/app/elements/change/gr-confirm-submit-dialog/gr-confirm-submit-dialog.html
+++ b/polygerrit-ui/app/elements/change/gr-confirm-submit-dialog/gr-confirm-submit-dialog.html
@@ -49,9 +49,10 @@
</div>
<div class="main" slot="main">
<gr-endpoint-decorator name="confirm-submit-change">
- <p>
- Ready to submit “<strong>[[change.subject]]</strong>”?
- </p>
+ <p>Ready to submit “<strong>[[change.subject]]</strong>”?</p>
+ <template is="dom-if" if="[[change.is_private]]">
+ <p><strong>Heads Up!</strong> Submitting this private change will also make it public.</p>
+ </template>
<gr-endpoint-param name="change" value="[[change]]"></gr-endpoint-param>
<gr-endpoint-param name="action" value="[[action]]"></gr-endpoint-param>
</gr-endpoint-decorator>
diff --git a/polygerrit-ui/app/elements/change/gr-confirm-submit-dialog/gr-confirm-submit-dialog.js b/polygerrit-ui/app/elements/change/gr-confirm-submit-dialog/gr-confirm-submit-dialog.js
index bda79a1..cefa045 100644
--- a/polygerrit-ui/app/elements/change/gr-confirm-submit-dialog/gr-confirm-submit-dialog.js
+++ b/polygerrit-ui/app/elements/change/gr-confirm-submit-dialog/gr-confirm-submit-dialog.js
@@ -35,6 +35,7 @@
properties: {
/**
* @type {{
+ * is_private: boolean,
* subject: string,
* }}
*/
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js
index 1be2cb1..d5fb878 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js
@@ -92,6 +92,16 @@
return content;
};
+ const COMMIT_MSG_PATH = '/COMMIT_MSG';
+ /**
+ * 72 is the inofficial length standard for git commit messages.
+ * Derived from the fact that git log/show appends 4 ws in the beginning of
+ * each line when displaying commit messages. To center the commit message
+ * in an 80 char terminal a 4 ws border is added to the rightmost side:
+ * 4 + 72 + 4
+ */
+ const COMMIT_MSG_LINE_LENGTH = 72;
+
Polymer({
is: 'gr-diff',
@@ -640,17 +650,19 @@
this._blame = null;
+ const lineLength = this.path === COMMIT_MSG_PATH ?
+ COMMIT_MSG_LINE_LENGTH : prefs.line_length;
const stylesToUpdate = {};
if (prefs.line_wrapping) {
this._diffTableClass = 'full-width';
if (this.viewMode === 'SIDE_BY_SIDE') {
stylesToUpdate['--content-width'] = 'none';
- stylesToUpdate['--line-limit'] = prefs.line_length + 'ch';
+ stylesToUpdate['--line-limit'] = lineLength + 'ch';
}
} else {
this._diffTableClass = '';
- stylesToUpdate['--content-width'] = prefs.line_length + 'ch';
+ stylesToUpdate['--content-width'] = lineLength + 'ch';
}
if (prefs.font_size) {
