Skip to content

ProxyOptions change #11656

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
kushagraThapar merged 3 commits into from
Jun 2, 2020
Merged

ProxyOptions change #11656

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b373edb
ProxyOptions change
Jun 2, 2020
99e39d9
resolve comments
Jun 2, 2020
3b9a065
fix
Jun 2, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion eng/code-quality-reports/src/main/resources/spotbugs/spotbugs-exclude.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1836,7 +1836,7 @@
<Bug pattern="UPM_UNCALLED_PRIVATE_METHOD"/>
</Match>

<!-- Serializing a null value should return null to allow client to handle null cases
<!-- Serializing a null value should return null to allow client to handle null cases
consistently instead of injecting null payload behavior from the payload. -->
<Match>
<Class name="com.azure.data.schemaregistry.avro.SchemaRegistryAvroSerializer"/>
Expand Down
27 changes: 18 additions & 9 deletions sdk/cosmos/azure-cosmos/src/main/java/com/azure/cosmos/GatewayConnectionConfig.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@

package com.azure.cosmos;

import java.net.InetSocketAddress;
import com.azure.core.http.ProxyOptions;

import java.time.Duration;

/**
Expand All @@ -18,7 +19,7 @@ public final class GatewayConnectionConfig {
private Duration requestTimeout;
private int maxConnectionPoolSize;
private Duration idleConnectionTimeout;
private InetSocketAddress inetSocketProxyAddress;
private ProxyOptions proxy;

/**
* Constructor.
Expand Down Expand Up @@ -103,12 +104,12 @@ public GatewayConnectionConfig setIdleConnectionTimeout(Duration idleConnectionT
}

/**
* Gets the InetSocketAddress of proxy server.
* Gets the proxy options which contain the InetSocketAddress of proxy server.
*
* @return the value of proxyHost.
* @return the proxy options.
*/
public InetSocketAddress getProxy() {
return this.inetSocketProxyAddress;
public ProxyOptions getProxy() {
return this.proxy;
}

/**
Expand All @@ -119,18 +120,26 @@ public InetSocketAddress getProxy() {
* @return the {@link GatewayConnectionConfig}.
*/

public GatewayConnectionConfig setProxy(InetSocketAddress proxy) {
this.inetSocketProxyAddress = proxy;
public GatewayConnectionConfig setProxy(ProxyOptions proxy) {
if (proxy.getType() != ProxyOptions.Type.HTTP) {
throw new IllegalArgumentException("Only http proxy type is supported.");
Copy link
Contributor

@moderakh moderakh Jun 2, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as we don't support password with proxy, shouldn't we throw if password is set?

Copy link
Member

@kushagraThapar kushagraThapar Jun 2, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure. may be we should, may be we should not, it will just get ignored.

But we definitely should add more documentation to setProxy() API that we only support HTTP proxy as of now - without any username and password.

I see, setProxy() docs are incorrect and should be updated.

@xinlian12 - please create another PR for this - with updated docs - "that we only support HTTP proxy type with just the routing address. Username and password are not supported"

Copy link
Member Author

@xinlian12 xinlian12 Jun 2, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks~ have created another PR for the docs update:
chttps://github.com//pull/11672

}

this.proxy = proxy;
return this;
}

@Override
public String toString() {
String proxyType = proxy != null ? proxy.getType().toString() : null;
String proxyAddress = proxy != null ? proxy.getAddress().toString() : null;

return "GatewayConnectionConfig{" +
"requestTimeout=" + requestTimeout +
", maxConnectionPoolSize=" + maxConnectionPoolSize +
", idleConnectionTimeout=" + idleConnectionTimeout +
", inetSocketProxyAddress=" + inetSocketProxyAddress +
", proxyType=" + proxyType +
", inetSocketProxyAddress=" + proxyAddress +
'}';
}
}
21 changes: 11 additions & 10 deletions sdk/cosmos/azure-cosmos/src/main/java/com/azure/cosmos/implementation/ConnectionPolicy.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,12 @@

package com.azure.cosmos.implementation;

import com.azure.core.http.ProxyOptions;
import com.azure.cosmos.ConnectionMode;
import com.azure.cosmos.DirectConnectionConfig;
import com.azure.cosmos.GatewayConnectionConfig;
import com.azure.cosmos.ThrottlingRetryOptions;

import java.net.InetSocketAddress;
import java.time.Duration;
import java.util.Collections;
import java.util.List;
Expand Down Expand Up @@ -37,7 +37,7 @@ public final class ConnectionPolicy {
private int maxConnectionPoolSize = DEFAULT_MAX_POOL_SIZE;
private Duration requestTimeout = DEFAULT_REQUEST_TIMEOUT;
private Duration idleConnectionTimeout = DEFAULT_IDLE_CONNECTION_TIMEOUT;
private InetSocketAddress inetSocketProxyAddress;
private ProxyOptions proxy;

// Direct connection config properties
private Duration connectionTimeout;
Expand All @@ -54,7 +54,7 @@ public ConnectionPolicy(GatewayConnectionConfig gatewayConnectionConfig) {
this.idleConnectionTimeout = gatewayConnectionConfig.getIdleConnectionTimeout();
this.maxConnectionPoolSize = gatewayConnectionConfig.getMaxConnectionPoolSize();
this.requestTimeout = gatewayConnectionConfig.getRequestTimeout();
this.inetSocketProxyAddress = gatewayConnectionConfig.getProxy();
this.proxy = gatewayConnectionConfig.getProxy();
}

public ConnectionPolicy(DirectConnectionConfig directConnectionConfig) {
Expand Down Expand Up @@ -350,12 +350,12 @@ public ConnectionPolicy setPreferredRegions(List<String> preferredRegions) {
}

/**
* Gets the InetSocketAddress of proxy server.
* Gets the proxy options which contain the InetSocketAddress of proxy server.
*
* @return the value of proxyHost.
* @return the proxy options.
*/
public InetSocketAddress getProxy() {
return this.inetSocketProxyAddress;
public ProxyOptions getProxy() {
return this.proxy;
}

/**
Expand All @@ -366,8 +366,8 @@ public InetSocketAddress getProxy() {
* @return the ConnectionPolicy.
*/

public ConnectionPolicy setProxy(InetSocketAddress proxy) {
this.inetSocketProxyAddress = proxy;
public ConnectionPolicy setProxy(ProxyOptions proxy) {
this.proxy = proxy;
return this;
}

Expand Down Expand Up @@ -473,7 +473,8 @@ public String toString() {
", endpointDiscoveryEnabled=" + endpointDiscoveryEnabled +
", preferredRegions=" + preferredRegions +
", multipleWriteRegionsEnabled=" + multipleWriteRegionsEnabled +
", inetSocketProxyAddress=" + inetSocketProxyAddress +
", proxyType=" + proxy.getType() +
", inetSocketProxyAddress=" + proxy.getAddress() +
", readRequestsFallbackEnabled=" + readRequestsFallbackEnabled +
", connectionTimeout=" + connectionTimeout +
", idleChannelTimeout=" + idleChannelTimeout +
Expand Down
2 changes: 1 addition & 1 deletion ...smos/azure-cosmos/src/main/java/com/azure/cosmos/implementation/RxDocumentClientImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -359,7 +359,7 @@ private HttpClient httpClient() {
HttpClientConfig httpClientConfig = new HttpClientConfig(this.configs)
.withMaxIdleConnectionTimeout(this.connectionPolicy.getIdleConnectionTimeout())
.withPoolSize(this.connectionPolicy.getMaxConnectionPoolSize())
.withHttpProxy(this.connectionPolicy.getProxy())
.withProxy(this.connectionPolicy.getProxy())
.withRequestTimeout(this.connectionPolicy.getRequestTimeout());

if (connectionSharingAcrossClientsEnabled) {
Expand Down
8 changes: 4 additions & 4 deletions ...mos/azure-cosmos/src/main/java/com/azure/cosmos/implementation/http/HttpClientConfig.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,9 @@

package com.azure.cosmos.implementation.http;

import com.azure.core.http.ProxyOptions;
import com.azure.cosmos.implementation.Configs;

import java.net.InetSocketAddress;
import java.time.Duration;

/**
Expand All @@ -18,7 +18,7 @@ public class HttpClientConfig {
private Integer maxPoolSize;
private Duration maxIdleConnectionTimeout;
private Duration requestTimeout;
private InetSocketAddress proxy;
private ProxyOptions proxy;
private boolean connectionKeepAlive = true;

public HttpClientConfig(Configs configs) {
Expand All @@ -30,7 +30,7 @@ public HttpClientConfig withPoolSize(int maxPoolSize) {
return this;
}

public HttpClientConfig withHttpProxy(InetSocketAddress proxy) {
public HttpClientConfig withProxy(ProxyOptions proxy) {
this.proxy = proxy;
return this;
}
Expand Down Expand Up @@ -66,7 +66,7 @@ public Duration getRequestTimeout() {
return requestTimeout;
}

public InetSocketAddress getProxy() {
public ProxyOptions getProxy() {
return proxy;
}

Expand Down
2 changes: 1 addition & 1 deletion ...s/azure-cosmos/src/main/java/com/azure/cosmos/implementation/http/ReactorNettyClient.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ private void configureChannelPipelineHandlers() {

if (this.httpClientConfig.getProxy() != null) {
tcpClient =
tcpClient.proxy(typeSpec -> typeSpec.type(ProxyProvider.Proxy.HTTP).address(this.httpClientConfig.getProxy()));
tcpClient.proxy(typeSpec -> typeSpec.type(ProxyProvider.Proxy.HTTP).address(this.httpClientConfig.getProxy().getAddress()));
}
tcpClient =
tcpClient.secure(sslContextSpec -> sslContextSpec.sslContext(configs.getSslContext()));
Expand Down
2 changes: 1 addition & 1 deletion sdk/cosmos/azure-cosmos/src/test/java/com/azure/cosmos/models/CosmosPartitionKeyTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ private void createContainerWithoutPk() throws URISyntaxException, IOException {
HttpClientConfig httpClientConfig = new HttpClientConfig(new Configs())
.withMaxIdleConnectionTimeout(connectionPolicy.getIdleConnectionTimeout())
.withPoolSize(connectionPolicy.getMaxConnectionPoolSize())
.withHttpProxy(connectionPolicy.getProxy())
.withProxy(connectionPolicy.getProxy())
.withRequestTimeout(connectionPolicy.getRequestTimeout());

HttpClient httpClient = HttpClient.createFixed(httpClientConfig);
Expand Down
23 changes: 18 additions & 5 deletions sdk/cosmos/azure-cosmos/src/test/java/com/azure/cosmos/rx/ProxyHostTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,17 @@
// Licensed under the MIT License.
package com.azure.cosmos.rx;

import com.azure.cosmos.GatewayConnectionConfig;
import com.azure.core.http.ProxyOptions;
import com.azure.cosmos.ConsistencyLevel;
import com.azure.cosmos.CosmosAsyncClient;
import com.azure.cosmos.CosmosAsyncContainer;
import com.azure.cosmos.CosmosAsyncDatabase;
import com.azure.cosmos.models.CosmosItemResponse;
import com.azure.cosmos.CosmosClientBuilder;
import com.azure.cosmos.GatewayConnectionConfig;
import com.azure.cosmos.implementation.CosmosItemProperties;
import com.azure.cosmos.models.CosmosItemRequestOptions;
import com.azure.cosmos.implementation.TestConfigurations;
import com.azure.cosmos.models.CosmosItemRequestOptions;
import com.azure.cosmos.models.CosmosItemResponse;
import com.azure.cosmos.rx.proxy.HttpProxyServer;
import org.apache.logging.log4j.Level;
import org.testng.annotations.AfterClass;
Expand Down Expand Up @@ -70,7 +71,7 @@ public void createDocumentWithValidHttpProxy() throws Exception {
CosmosAsyncClient clientWithRightProxy = null;
try {
GatewayConnectionConfig gatewayConnectionConfig = new GatewayConnectionConfig();
gatewayConnectionConfig.setProxy(new InetSocketAddress(PROXY_HOST, PROXY_PORT));
gatewayConnectionConfig.setProxy(new ProxyOptions(ProxyOptions.Type.HTTP, new InetSocketAddress(PROXY_HOST, PROXY_PORT)));
clientWithRightProxy = new CosmosClientBuilder().endpoint(TestConfigurations.HOST)
.key(TestConfigurations.MASTER_KEY)
.gatewayMode(gatewayConnectionConfig)
Expand Down Expand Up @@ -106,7 +107,7 @@ public void createDocumentWithValidHttpProxyWithNettyWireLogging() throws Except
"ProxyStringAppender", consoleWriter);

GatewayConnectionConfig gatewayConnectionConfig = new GatewayConnectionConfig();
gatewayConnectionConfig.setProxy(new InetSocketAddress(PROXY_HOST, PROXY_PORT));
gatewayConnectionConfig.setProxy(new ProxyOptions(ProxyOptions.Type.HTTP, new InetSocketAddress(PROXY_HOST, PROXY_PORT)));
clientWithRightProxy = new CosmosClientBuilder().endpoint(TestConfigurations.HOST)
.key(TestConfigurations.MASTER_KEY)
.gatewayMode(gatewayConnectionConfig)
Expand Down Expand Up @@ -155,4 +156,16 @@ private CosmosItemProperties getDocumentDefinition() {
, uuid, uuid));
return doc;
}

/**
* This test will try to create gateway connection policy via non http proxy.
*
*/
@Test(groups = { "simple" }, timeOut = TIMEOUT,
expectedExceptions = IllegalArgumentException.class,
expectedExceptionsMessageRegExp = "Only http proxy type is supported.")
public void createWithNonHttpProxy() {
GatewayConnectionConfig gatewayConnectionConfig = new GatewayConnectionConfig();
gatewayConnectionConfig.setProxy(new ProxyOptions(ProxyOptions.Type.SOCKS4, new InetSocketAddress(PROXY_HOST, PROXY_PORT)));
}
}