opendev/gerrit
Code Issues Proposed changes

Replace gerrit.disableReverseDnsLookup with gerrit.enableReverseDnsLookup

Browse Source 
Having a configuration option 'disable' with default value 'false'
results in a double-negative meaning which can be confusing.

Replace this option with enableReverseDnsLookup, with default 'true',
which effectively results in the same default behavior when no value
is configured.

Rename the DisableReverseDnsLookup and DisableReverseDnsLookupProvider
classes accordingly.

Change-Id: I36a8be9474f58a0d1457850bb5fae2fa7cc08ee8
This commit is contained in:
David Pursehouse 2018-11-30 11:12:19 +09:00
parent 0a1fc895d8
commit 46a2f6cf4c
9 changed files with 42 additions and 42 deletions

6
Documentation/config-gerrit.txt

@ -1998,11 +1998,11 @@ Only used when `gerrit.reportBugUrl` is set.
+
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.
+
Defaults to false.
Defaults to true.
[[gerrit.secureStoreClass]]gerrit.secureStoreClass::
+

8
java/com/google/gerrit/pgm/util/BatchProgramModule.java

@ -48,8 +48,8 @@ import com.google.gerrit.server.config.AdministrateServerGroups;
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 @@ public class BatchProgramModule extends FactoryModule {
.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));

36
java/com/google/gerrit/server/IdentifiedUser.java

@ -32,7 +32,7 @@ import com.google.gerrit.server.account.Realm;
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 @@ public class IdentifiedUser extends CurrentUser {
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 @@ public class IdentifiedUser extends CurrentUser {
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 @@ public class IdentifiedUser extends CurrentUser {
this.canonicalUrl = canonicalUrl;
this.accountCache = accountCache;
this.groupBackend = groupBackend;
this.disableReverseDnsLookup = disableReverseDnsLookup;
this.enableReverseDnsLookup = enableReverseDnsLookup;
}
public IdentifiedUser create(AccountState state) {
@ -95,7 +95,7 @@ public class IdentifiedUser extends CurrentUser {
canonicalUrl,
accountCache,
groupBackend,
disableReverseDnsLookup,
enableReverseDnsLookup,
Providers.of(null),
state,
null);
@ -118,7 +118,7 @@ public class IdentifiedUser extends CurrentUser {
canonicalUrl,
accountCache,
groupBackend,
disableReverseDnsLookup,
enableReverseDnsLookup,
Providers.of(remotePeer),
id,
caller);
@ -139,7 +139,7 @@ public class IdentifiedUser extends CurrentUser {
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 @@ public class IdentifiedUser extends CurrentUser {
@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 @@ public class IdentifiedUser extends CurrentUser {
this.canonicalUrl = canonicalUrl;
this.accountCache = accountCache;
this.groupBackend = groupBackend;
this.disableReverseDnsLookup = disableReverseDnsLookup;
this.enableReverseDnsLookup = enableReverseDnsLookup;
this.remotePeerProvider = remotePeerProvider;
}
@ -170,7 +170,7 @@ public class IdentifiedUser extends CurrentUser {
canonicalUrl,
accountCache,
groupBackend,
disableReverseDnsLookup,
enableReverseDnsLookup,
remotePeerProvider,
id,
null);
@ -184,7 +184,7 @@ public class IdentifiedUser extends CurrentUser {
canonicalUrl,
accountCache,
groupBackend,
disableReverseDnsLookup,
enableReverseDnsLookup,
remotePeerProvider,
id,
caller);
@ -201,7 +201,7 @@ public class IdentifiedUser extends CurrentUser {
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 @@ public class IdentifiedUser extends CurrentUser {
Provider<String> canonicalUrl,
AccountCache accountCache,
GroupBackend groupBackend,
Boolean disableReverseDnsLookup,
Boolean enableReverseDnsLookup,
@Nullable Provider<SocketAddress> remotePeerProvider,
AccountState state,
@Nullable CurrentUser realUser) {
@ -232,7 +232,7 @@ public class IdentifiedUser extends CurrentUser {
canonicalUrl,
accountCache,
groupBackend,
disableReverseDnsLookup,
enableReverseDnsLookup,
remotePeerProvider,
state.getAccount().getId(),
realUser);
@ -246,7 +246,7 @@ public class IdentifiedUser extends CurrentUser {
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 @@ public class IdentifiedUser extends CurrentUser {
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 @@ public class IdentifiedUser extends CurrentUser {
Providers.of(canonicalUrl.get()),
accountCache,
groupBackend,
disableReverseDnsLookup,
enableReverseDnsLookup,
remotePeer,
state,
realUser);
@ -554,7 +554,7 @@ public class IdentifiedUser extends CurrentUser {
}
private String getHost(InetAddress in) {
if (Boolean.FALSE.equals(disableReverseDnsLookup)) {
if (Boolean.TRUE.equals(enableReverseDnsLookup)) {
return in.getCanonicalHostName();
}
return in.getHostAddress();

2
java/com/google/gerrit/server/config/DisableReverseDnsLookup.java → java/com/google/gerrit/server/config/EnableReverseDnsLookup.java

@ -21,4 +21,4 @@ import java.lang.annotation.Retention;
@Retention(RUNTIME)
@BindingAnnotation
public @interface DisableReverseDnsLookup {}
public @interface EnableReverseDnsLookup {}

10
java/com/google/gerrit/server/config/DisableReverseDnsLookupProvider.java → java/com/google/gerrit/server/config/EnableReverseDnsLookupProvider.java

@ -18,16 +18,16 @@ import com.google.inject.Inject;
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", true);
}
@Override
public Boolean get() {
return disableReverseDnsLookup;
return enableReverseDnsLookup;
}
}

4
java/com/google/gerrit/server/config/GerritGlobalModule.java

@ -295,8 +295,8 @@ public class GerritGlobalModule extends FactoryModule {
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);

6
javatests/com/google/gerrit/acceptance/rest/account/EmailIT.java

@ -43,7 +43,7 @@ import com.google.gerrit.server.account.externalids.ExternalIds;
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 @@ import org.junit.Test;
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 @@ public class EmailIT extends AbstractDaemonTest {
realm,
anonymousCowardName,
canonicalUrl,
disableReverseDnsLookup,
enableReverseDnsLookup,
accountCache,
groupBackend);
return atrScope.set(atrScope.newContext(null, userFactory.create(admin.id)));

6
javatests/com/google/gerrit/server/IdentifiedUserTest.java

@ -25,7 +25,7 @@ import com.google.gerrit.server.account.Realm;
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 @@ public class IdentifiedUserTest extends GerritBaseTests {
@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)

6
javatests/com/google/gerrit/server/notedb/AbstractChangeNotesTest.java

@ -42,7 +42,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.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 @@ public abstract class AbstractChangeNotesTest extends GerritBaseTests {
.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);