Skip to content
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.

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

Http Class Name Cleanup #18911

Merged
merged 8 commits into from
Feb 3, 2021
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
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
284 changes: 148 additions & 136 deletions eng/code-quality-reports/src/main/resources/spotbugs/spotbugs-exclude.xml

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
import com.azure.core.http.HttpRequest;
import com.azure.core.http.HttpResponse;
import com.azure.core.http.ProxyOptions;
import com.azure.core.http.netty.implementation.NettyAsyncHttpResponse;
import com.azure.core.http.netty.implementation.NettyAsyncHttpBufferedResponse;
import com.azure.core.util.Context;
import com.azure.core.util.FluxUtil;
import io.netty.buffer.ByteBuf;
Expand All @@ -27,6 +29,8 @@
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.function.BiFunction;

import static com.azure.core.http.netty.implementation.Utility.closeConnection;

/**
* This class provides a Netty-based implementation for the {@link HttpClient} interface. Creating an instance of this
* class can be achieved by using the {@link NettyAsyncHttpClientBuilder} class, which offers Netty-specific API for
Expand All @@ -42,13 +46,6 @@ class NettyAsyncHttpClient implements HttpClient {

final reactor.netty.http.client.HttpClient nettyClient;

/**
* Creates default NettyAsyncHttpClient.
*/
NettyAsyncHttpClient() {
this(reactor.netty.http.client.HttpClient.create(), false);
}

/**
* Creates NettyAsyncHttpClient with provided http client.
*
Expand Down Expand Up @@ -148,25 +145,12 @@ private static BiFunction<HttpClientResponse, Connection, Publisher<HttpResponse
.doFinally(ignored -> closeConnection(reactorNettyConnection));

return FluxUtil.collectBytesInByteBufferStream(body)
.map(bytes -> new BufferedReactorNettyHttpResponse(reactorNettyResponse, restRequest, bytes));
.map(bytes -> new NettyAsyncHttpBufferedResponse(reactorNettyResponse, restRequest, bytes));

} else {
return Mono.just(new ReactorNettyHttpResponse(reactorNettyResponse, reactorNettyConnection, restRequest,
return Mono.just(new NettyAsyncHttpResponse(reactorNettyResponse, reactorNettyConnection, restRequest,
disableBufferCopy));
}
};
}

static ByteBuffer deepCopyBuffer(ByteBuf byteBuf) {
ByteBuffer buffer = ByteBuffer.allocate(byteBuf.readableBytes());
byteBuf.readBytes(buffer);
buffer.rewind();
return buffer;
}

static void closeConnection(Connection reactorNettyConnection) {
if (!reactorNettyConnection.isDisposed()) {
reactorNettyConnection.channel().eventLoop().execute(reactorNettyConnection::dispose);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -383,7 +383,7 @@ private static void removeReadTimeoutHandler(Connection connection) {
static long getTimeoutMillis(Duration timeout) {
// Timeout is null, use the 60 second default.
if (timeout == null) {
return TimeUnit.SECONDS.toMillis(60);
return DEFAULT_TIMEOUT;
}

// Timeout is less than or equal to zero, return no timeout.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
/**
* An {@link HttpClientProvider} that provides an implementation of HttpClient based on Netty.
*/
public final class ReactorNettyClientProvider implements HttpClientProvider {
public final class NettyAsyncHttpClientProvider implements HttpClientProvider {
alzimmermsft marked this conversation as resolved.
Show resolved Hide resolved

@Override
public HttpClient createInstance() {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

package com.azure.core.http.netty;
package com.azure.core.http.netty.implementation;

import com.azure.core.http.HttpRequest;
import com.azure.core.util.CoreUtils;
Expand All @@ -15,10 +15,10 @@
/**
* A Reactor Netty response where the response body has been buffered into memory.
*/
final class BufferedReactorNettyHttpResponse extends ReactorNettyHttpResponseBase {
public final class NettyAsyncHttpBufferedResponse extends NettyAsyncHttpResponseBase {
private final byte[] body;

BufferedReactorNettyHttpResponse(HttpClientResponse httpClientResponse, HttpRequest httpRequest, byte[] body) {
public NettyAsyncHttpBufferedResponse(HttpClientResponse httpClientResponse, HttpRequest httpRequest, byte[] body) {
super(httpClientResponse, httpRequest);
this.body = body;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

package com.azure.core.http.netty;
package com.azure.core.http.netty.implementation;

import com.azure.core.http.HttpRequest;
import com.azure.core.util.CoreUtils;
Expand All @@ -14,17 +14,17 @@
import java.nio.ByteBuffer;
import java.nio.charset.Charset;

import static com.azure.core.http.netty.NettyAsyncHttpClient.closeConnection;
import static com.azure.core.http.netty.NettyAsyncHttpClient.deepCopyBuffer;
import static com.azure.core.http.netty.implementation.Utility.closeConnection;
import static com.azure.core.http.netty.implementation.Utility.deepCopyBuffer;

/**
* Default HTTP response for Reactor Netty.
*/
final class ReactorNettyHttpResponse extends ReactorNettyHttpResponseBase {
public final class NettyAsyncHttpResponse extends NettyAsyncHttpResponseBase {
private final Connection reactorNettyConnection;
private final boolean disableBufferCopy;

ReactorNettyHttpResponse(HttpClientResponse reactorNettyResponse, Connection reactorNettyConnection,
public NettyAsyncHttpResponse(HttpClientResponse reactorNettyResponse, Connection reactorNettyConnection,
HttpRequest httpRequest, boolean disableBufferCopy) {
super(reactorNettyResponse, httpRequest);
this.reactorNettyConnection = reactorNettyConnection;
Expand Down Expand Up @@ -62,7 +62,7 @@ private ByteBufFlux bodyIntern() {
}

// used for testing only
Connection internConnection() {
public Connection internConnection() {
return reactorNettyConnection;
}
}
Original file line number Diff line number Diff line change
@@ -1,26 +1,25 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

package com.azure.core.http.netty;
package com.azure.core.http.netty.implementation;

import com.azure.core.http.HttpHeaders;
import com.azure.core.http.HttpRequest;
import com.azure.core.http.HttpResponse;
import com.azure.core.http.netty.implementation.NettyToAzureCoreHttpHeadersWrapper;
import reactor.netty.http.client.HttpClientResponse;

/**
* Base response class for Reactor Netty with implementations for response metadata.
*/
abstract class ReactorNettyHttpResponseBase extends HttpResponse {
public abstract class NettyAsyncHttpResponseBase extends HttpResponse {
private final HttpClientResponse reactorNettyResponse;

// We use a wrapper for the Netty-returned headers, so we are not forced to pay up-front the cost of converting
// from Netty HttpHeaders to azure-core HttpHeaders. Instead, by wrapping it, there is no cost to pay as we map
// the Netty HttpHeaders API into the azure-core HttpHeaders API.
private NettyToAzureCoreHttpHeadersWrapper headers;

ReactorNettyHttpResponseBase(HttpClientResponse reactorNettyResponse, HttpRequest httpRequest) {
NettyAsyncHttpResponseBase(HttpClientResponse reactorNettyResponse, HttpRequest httpRequest) {
super(httpRequest);
this.reactorNettyResponse = reactorNettyResponse;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

package com.azure.core.http.netty.implementation;

import io.netty.buffer.ByteBuf;
import reactor.netty.Connection;

import java.nio.ByteBuffer;

/**
* Helper class containing utility methods.
*/
public final class Utility {
Copy link
Member

Choose a reason for hiding this comment

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

Not a huge fan of how many Utility classes we have - can this be made into, e.g. NettyUtility or similar

/**
* Deep copies the passed {@link ByteBuf} into a {@link ByteBuffer}.
* <p>
* Using this method ensures that data returned by the network is resilient against Reactor Netty releasing the
* passed {@link ByteBuf} once the {@code doOnNext} operator fires.
*
* @param byteBuf The Netty {@link ByteBuf} to deep copy.
* @return A newly allocated {@link ByteBuffer} containing the copied bytes.
*/
public static ByteBuffer deepCopyBuffer(ByteBuf byteBuf) {
ByteBuffer buffer = ByteBuffer.allocate(byteBuf.readableBytes());
byteBuf.readBytes(buffer);
buffer.rewind();
return buffer;
}

/**
* Closes a connection if it hasn't been disposed.
*
* @param reactorNettyConnection The connection to close.
*/
public static void closeConnection(Connection reactorNettyConnection) {
if (!reactorNettyConnection.isDisposed()) {
reactorNettyConnection.channel().eventLoop().execute(reactorNettyConnection::dispose);
}
}

private Utility() {
}
}
4 changes: 2 additions & 2 deletions sdk/core/azure-core-http-netty/src/main/java/module-info.java
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

import com.azure.core.http.netty.ReactorNettyClientProvider;
import com.azure.core.http.netty.NettyAsyncHttpClientProvider;

module com.azure.http.netty {
requires transitive com.azure.core;
Expand All @@ -18,7 +18,7 @@
exports com.azure.core.http.netty;

provides com.azure.core.http.HttpClientProvider
with ReactorNettyClientProvider;
with NettyAsyncHttpClientProvider;

uses com.azure.core.http.HttpClientProvider;
}
Original file line number Diff line number Diff line change
@@ -1 +1 @@
com.azure.core.http.netty.ReactorNettyClientProvider
com.azure.core.http.netty.NettyAsyncHttpClientProvider
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
/**
* Reactor Netty {@link HttpClientTests}.
*/
public class ReactorNettyHttpClientTests extends HttpClientTests {
public class NettyAsyncHttpClientHttpClientTests extends HttpClientTests {
private static WireMockServer server;

@BeforeAll
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,13 @@
import com.github.tomakehurst.wiremock.http.Request;
import com.github.tomakehurst.wiremock.http.Response;

import static com.azure.core.http.netty.ReactorNettyClientTests.EXPECTED_HEADER;
import static com.azure.core.http.netty.ReactorNettyClientTests.NO_DOUBLE_UA_PATH;
import static com.azure.core.http.netty.NettyAsyncHttpClientTests.EXPECTED_HEADER;
import static com.azure.core.http.netty.NettyAsyncHttpClientTests.NO_DOUBLE_UA_PATH;

/**
* Mock response transformer used to test {@link NettyAsyncHttpClient}.
*/
public final class ReactorNettyClientResponseTransformer extends ResponseTransformer {
public final class NettyAsyncHttpClientResponseTransformer extends ResponseTransformer {
public static final String NAME = "reactor-netty-client-response-transformer";
public static final String NULL_REPLACEMENT = "null";

Expand Down Expand Up @@ -51,13 +51,13 @@ public boolean applyGlobally() {
}

private static Response httpHeadersResponseHandler(Request request, Response response) {
String responseTestHeaderValue = request.containsHeader(ReactorNettyClientTests.TEST_HEADER)
? request.getHeaders().getHeader(ReactorNettyClientTests.TEST_HEADER).firstValue()
String responseTestHeaderValue = request.containsHeader(NettyAsyncHttpClientTests.TEST_HEADER)
? request.getHeaders().getHeader(NettyAsyncHttpClientTests.TEST_HEADER).firstValue()
: NULL_REPLACEMENT;

return new Response.Builder()
.status(response.getStatus())
.headers(new HttpHeaders(new HttpHeader(ReactorNettyClientTests.TEST_HEADER, responseTestHeaderValue)))
.headers(new HttpHeaders(new HttpHeader(NettyAsyncHttpClientTests.TEST_HEADER, responseTestHeaderValue)))
.build();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
import org.junit.jupiter.api.AfterAll;
import org.junit.jupiter.api.BeforeAll;

public class RestProxyWithNettyTests extends RestProxyTests {
public class NettyAsyncHttpClientRestProxyTests extends RestProxyTests {
private static WireMockServer server;

@BeforeAll
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
import java.net.InetSocketAddress;

@Disabled("Should only be run manually when a local proxy server (e.g. Fiddler) is running")
public class RestProxyWithHttpProxyNettyTests extends RestProxyTests {
public class NettyAsyncHttpClientRestProxyWithHttpProxyTests extends RestProxyTests {
private static WireMockServer server;

@BeforeAll
Expand Down
Loading