Merge pull request #430 from elandau/misc

config: Misc cleanup

Author: elandau

ribbon-core/src/main/java/com/netflix/client/RequestSpecificRetryHandler.java

@@ -35,10 +35,10 @@ public RequestSpecificRetryHandler(boolean okToRetryOnConnectErrors, boolean okT
         this.okToRetryOnAllErrors = okToRetryOnAllErrors;
         this.fallback = baseRetryHandler;
         if (requestConfig != null) {
-            Optional.ofNullable(requestConfig.get(CommonClientConfigKey.MaxAutoRetries)).ifPresent(
+            requestConfig.getIfSet(CommonClientConfigKey.MaxAutoRetries).ifPresent(
                     value -> retrySameServer = value
             );
-            Optional.ofNullable(requestConfig.get(CommonClientConfigKey.MaxAutoRetriesNextServer)).ifPresent(
+            requestConfig.getIfSet(CommonClientConfigKey.MaxAutoRetriesNextServer).ifPresent(
                     value -> retryNextServer = value
             );
         }

ribbon-core/src/main/java/com/netflix/client/config/ReloadableClientConfig.java

@@ -456,10 +456,10 @@ public long getRefreshCount() {
     private String generateToString() {
         return "ClientConfig:" + internalProperties.entrySet().stream()
                     .map(t -> {
-                        if (t.getKey().key().endsWith("Password")) {
+                        if (t.getKey().key().endsWith("Password") && t.getValue().isPresent()) {
                             return t.getKey() + ":***";
                         }
-                        return t.getKey() + ":" + t.getValue();
+                        return t.getKey() + ":" + t.getValue().orElse(null);
                     })
                 .collect(Collectors.joining(", "));
     }

ribbon-eureka/src/main/java/com/netflix/niws/loadbalancer/DiscoveryEnabledNIWSServerList.java

@@ -117,7 +117,7 @@ public void initWithNiwsConfig(IClientConfig clientConfig) {
         shouldUseIpAddr = clientConfig.getOrDefault(CommonClientConfigKey.UseIPAddrForServer);
 
         // override client configuration and use client-defined port
-        if (clientConfig.get(CommonClientConfigKey.ForceClientPortConfiguration, false)){
+        if (clientConfig.get(CommonClientConfigKey.ForceClientPortConfiguration, false)) {
             if (isSecure) {
                 final Integer port = clientConfig.get(CommonClientConfigKey.SecurePort);
                 if (port != null) {

ribbon-eureka/src/test/java/com/netflix/niws/loadbalancer/DiscoveryEnabledLoadBalancerSupportsPortOverrideTest.java

@@ -181,7 +181,6 @@ public void testSecureVipPortCanBeOverriden() throws Exception{
         ConfigurationManager.getConfigInstance().setProperty("DiscoveryEnabled.testSecureVipPortCanBeOverriden.ribbon.SecurePort", "6002");
         ConfigurationManager.getConfigInstance().setProperty("DiscoveryEnabled.testSecureVipPortCanBeOverriden.ribbon.TargetRegion", "region");
         ConfigurationManager.getConfigInstance().setProperty("DiscoveryEnabled.testSecureVipPortCanBeOverriden.ribbon.NIWSServerListClassName", DiscoveryEnabledNIWSServerList.class.getName());
-
         ConfigurationManager.getConfigInstance().setProperty("DiscoveryEnabled.testSecureVipPortCanBeOverriden.ribbon.ForceClientPortConfiguration", "true");
 
         DiscoveryEnabledNIWSServerList deList = new DiscoveryEnabledNIWSServerList();

ribbon-httpclient/src/main/java/com/netflix/niws/client/http/RestClient.java

@@ -147,19 +147,19 @@ public void initWithNiwsConfig(IClientConfig clientConfig) {
         super.initWithNiwsConfig(clientConfig);
         this.ncc = clientConfig;
         this.restClientName = ncc.getClientName();
-        this.isSecure = getBooleanFromConfig(ncc, CommonClientConfigKey.IsSecure, this.isSecure);
-        this.isHostnameValidationRequired = getBooleanFromConfig(ncc, CommonClientConfigKey.IsHostnameValidationRequired, this.isHostnameValidationRequired);
-        this.isClientAuthRequired = getBooleanFromConfig(ncc, CommonClientConfigKey.IsClientAuthRequired, this.isClientAuthRequired);
-        this.bFollowRedirects = getBooleanFromConfig(ncc, CommonClientConfigKey.FollowRedirects, true);
-        this.ignoreUserToken = getBooleanFromConfig(ncc, CommonClientConfigKey.IgnoreUserTokenInConnectionPoolForSecureClient, this.ignoreUserToken);
+        this.isSecure = ncc.get(CommonClientConfigKey.IsSecure, this.isSecure);
+        this.isHostnameValidationRequired = ncc.get(CommonClientConfigKey.IsHostnameValidationRequired, this.isHostnameValidationRequired);
+        this.isClientAuthRequired = ncc.get(CommonClientConfigKey.IsClientAuthRequired, this.isClientAuthRequired);
+        this.bFollowRedirects = ncc.get(CommonClientConfigKey.FollowRedirects, true);
+        this.ignoreUserToken = ncc.get(CommonClientConfigKey.IgnoreUserTokenInConnectionPoolForSecureClient, this.ignoreUserToken);
 
         this.config = new DefaultApacheHttpClient4Config();
         this.config.getProperties().put(
                 ApacheHttpClient4Config.PROPERTY_CONNECT_TIMEOUT,
-                ncc.getOrDefault(CommonClientConfigKey.ConnectTimeout));
+                ncc.get(CommonClientConfigKey.ConnectTimeout));
         this.config.getProperties().put(
                 ApacheHttpClient4Config.PROPERTY_READ_TIMEOUT,
-                ncc.getOrDefault(CommonClientConfigKey.ReadTimeout));
+                ncc.get(CommonClientConfigKey.ReadTimeout));
 
         this.restClient = apacheHttpClientSpecificInitialization();
         this.setRetryHandler(new HttpClientLoadBalancerErrorHandler(ncc));
@@ -470,13 +470,6 @@ public HttpResponse execute(HttpRequest task, IClientConfig requestConfig) throw
                 task.getHeaders(), task.getQueryParams(), config, task.getEntity());
     }
 
-
-    private boolean getBooleanFromConfig(IClientConfig overriddenClientConfig, IClientConfigKey<Boolean> key, boolean defaultValue) {
-        return Optional.ofNullable(overriddenClientConfig)
-                .map(config -> config.get(key))
-                .orElse(defaultValue);
-    }
-
     @Override
     protected int getDefaultPortFromScheme(String scheme) {
         int port = super.getDefaultPortFromScheme(scheme);
@@ -489,7 +482,7 @@ protected int getDefaultPortFromScheme(String scheme) {
 
     @Override
     protected Pair<String, Integer> deriveSchemeAndPortFromPartialUri(URI uri) {
-        boolean isSecure = getBooleanFromConfig(ncc, CommonClientConfigKey.IsSecure, this.isSecure);
+        boolean isSecure = ncc.get(CommonClientConfigKey.IsSecure, this.isSecure);
         String scheme = uri.getScheme();
         if (scheme != null) {
             isSecure = 	scheme.equalsIgnoreCase("https");
@@ -507,7 +500,7 @@ protected Pair<String, Integer> deriveSchemeAndPortFromPartialUri(URI uri) {
                 scheme = "http";
             }
         }
-        return new Pair<String, Integer>(scheme, port);
+        return new Pair<>(scheme, port);
     }
 
     private HttpResponse execute(HttpRequest.Verb verb, URI uri,
@@ -516,7 +509,7 @@ private HttpResponse execute(HttpRequest.Verb verb, URI uri,
         HttpClientResponse thisResponse = null;
 
         final boolean bbFollowRedirects = Optional.ofNullable(overriddenClientConfig)
-                .map(config -> config.get(CommonClientConfigKey.FollowRedirects, bFollowRedirects))
+                .flatMap(config -> config.getIfSet(CommonClientConfigKey.FollowRedirects))
                 .orElse(bFollowRedirects);
 
         restClient.setFollowRedirects(bbFollowRedirects);

ribbon-transport/src/main/java/com/netflix/ribbon/transport/netty/http/LoadBalancingHttpClient.java

@@ -456,7 +456,7 @@ private Observable<HttpClientResponse<O>> submitToServerInURI(
         }
         int port = uri.getPort();
         if (port < 0) {
-            if (Optional.ofNullable(clientConfig.get(IClientConfigKey.Keys.IsSecure)).orElse(false)) {
+            if (clientConfig.get(IClientConfigKey.Keys.IsSecure, false)) {
                 port = 443;
             } else {
                 port = 80;