Skip to content

Commit 24be8a7

Browse files
author
Nitesh Kant
committed
Fixes #230 (FlatResponseOperator does not emit any item if there is no content.)
Also, fixed the test failure for previous PR
1 parent 447f3f3 commit 24be8a7

File tree

4 files changed

+133
-43
lines changed

4 files changed

+133
-43
lines changed

‎rx-netty-contexts/src/test/java/io/reactivex/netty/contexts/http/ContextPropagationTest.java

Lines changed: 23 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -28,9 +28,11 @@
2828
import io.reactivex.netty.contexts.RxContexts;
2929
import io.reactivex.netty.contexts.TestContext;
3030
import io.reactivex.netty.contexts.TestContextSerializer;
31+
import io.reactivex.netty.protocol.http.client.FlatResponseOperator;
3132
import io.reactivex.netty.protocol.http.client.HttpClient;
3233
import io.reactivex.netty.protocol.http.client.HttpClientRequest;
3334
import io.reactivex.netty.protocol.http.client.HttpClientResponse;
35+
import io.reactivex.netty.protocol.http.client.ResponseHolder;
3436
import io.reactivex.netty.protocol.http.server.HttpServer;
3537
import io.reactivex.netty.protocol.http.server.HttpServerBuilder;
3638
import io.reactivex.netty.protocol.http.server.HttpServerRequest;
@@ -41,18 +43,15 @@
4143
import org.junit.Before;
4244
import org.junit.Test;
4345
import rx.Observable;
44-
import rx.functions.Action0;
45-
import rx.functions.Action1;
4646
import rx.functions.Func1;
4747

48-
import java.util.ArrayList;
49-
import java.util.List;
5048
import java.util.concurrent.Callable;
51-
import java.util.concurrent.CountDownLatch;
49+
import java.util.concurrent.ExecutionException;
5250
import java.util.concurrent.ExecutorService;
5351
import java.util.concurrent.Executors;
5452
import java.util.concurrent.Future;
5553
import java.util.concurrent.TimeUnit;
54+
import java.util.concurrent.TimeoutException;
5655

5756
import static io.reactivex.netty.contexts.ThreadLocalRequestCorrelator.getCurrentContextContainer;
5857
import static io.reactivex.netty.contexts.ThreadLocalRequestCorrelator.getCurrentRequestId;
@@ -103,7 +102,7 @@ public String getContextValue(String key) {
103102
return Observable.error(e);
104103
}
105104
}
106-
}).enableWireLogging(LogLevel.DEBUG).build();
105+
}).enableWireLogging(LogLevel.ERROR).build();
107106
mockServer.start();
108107
}
109108

@@ -121,10 +120,10 @@ public void testEndToEnd() throws Exception {
121120
public Observable<HttpClientResponse<ByteBuf>> call(HttpClient<ByteBuf, ByteBuf> client) {
122121
return client.submit(HttpClientRequest.createGet("/"));
123122
}
124-
}).enableWireLogging(LogLevel.ERROR).build().start();
123+
}).enableWireLogging(LogLevel.DEBUG).build().start();
125124

126125
HttpClient<ByteBuf, ByteBuf> testClient = RxNetty.<ByteBuf, ByteBuf>newHttpClientBuilder("localhost", server.getServerPort())
127-
.enableWireLogging(LogLevel.DEBUG).build();
126+
.enableWireLogging(LogLevel.ERROR).build();
128127

129128
String reqId = "testE2E";
130129
sendTestRequest(testClient, reqId);
@@ -184,7 +183,8 @@ public void testWithPooledConnections() throws Exception {
184183
RxContexts.<ByteBuf, ByteBuf>newHttpClientBuilder("localhost", mockServer.getServerPort(),
185184
REQUEST_ID_HEADER_NAME,
186185
RxContexts.DEFAULT_CORRELATOR)
187-
.withMaxConnections(1).withIdleConnectionsTimeoutMillis(100000).build();
186+
.withMaxConnections(1).enableWireLogging(LogLevel.ERROR)
187+
.withIdleConnectionsTimeoutMillis(100000).build();
188188
ContextsContainer container = new ContextsContainerImpl(new MapBackedKeySupplier());
189189
container.addContext(CTX_1_NAME, CTX_1_VAL);
190190
container.addContext(CTX_2_NAME, CTX_2_VAL, new TestContextSerializer());
@@ -203,7 +203,8 @@ public void testNoStateLeakOnThreadReuse() throws Exception {
203203
RxContexts.<ByteBuf, ByteBuf>newHttpClientBuilder("localhost", mockServer.getServerPort(),
204204
REQUEST_ID_HEADER_NAME,
205205
RxContexts.DEFAULT_CORRELATOR)
206-
.withMaxConnections(1).withIdleConnectionsTimeoutMillis(100000).build();
206+
.withMaxConnections(1).enableWireLogging(LogLevel.ERROR)
207+
.withIdleConnectionsTimeoutMillis(100000).build();
207208

208209
ContextsContainer container = new ContextsContainerImpl(new MapBackedKeySupplier());
209210
container.addContext(CTX_1_NAME, CTX_1_VAL);
@@ -259,7 +260,7 @@ public Observable<Void> call(HttpClientResponse<ByteBuf> response) {
259260

260261
private static void invokeMockServer(HttpClient<ByteBuf, ByteBuf> testClient, final String requestId,
261262
boolean finishServerProcessing)
262-
throws MockBackendRequestFailedException, InterruptedException {
263+
throws MockBackendRequestFailedException, InterruptedException, TimeoutException, ExecutionException {
263264
try {
264265
sendTestRequest(testClient, requestId);
265266
} finally {
@@ -277,40 +278,24 @@ private static void invokeMockServer(HttpClient<ByteBuf, ByteBuf> testClient, fi
277278
}
278279

279280
private static void sendTestRequest(HttpClient<ByteBuf, ByteBuf> testClient, final String requestId)
280-
throws MockBackendRequestFailedException, InterruptedException {
281+
throws MockBackendRequestFailedException, InterruptedException, TimeoutException, ExecutionException {
281282
System.err.println("Sending test request to mock server, with request id: " + requestId);
282283
RxContexts.DEFAULT_CORRELATOR.dumpThreadState(System.err);
283-
final CountDownLatch finishLatch = new CountDownLatch(1);
284-
final List<HttpClientResponse<ByteBuf>> responseHolder = new ArrayList<HttpClientResponse<ByteBuf>>();
285-
testClient.submit(HttpClientRequest.createGet("").withHeader(REQUEST_ID_HEADER_NAME, requestId))
286-
.finallyDo(new Action0() {
287-
@Override
288-
public void call() {
289-
finishLatch.countDown();
290-
}
291-
})
292-
.subscribe(new Action1<HttpClientResponse<ByteBuf>>() {
293-
@Override
294-
public void call(HttpClientResponse<ByteBuf> response) {
295-
responseHolder.add(response);
296-
}
297-
});
298-
299-
finishLatch.await(1, TimeUnit.MINUTES);
300-
if (responseHolder.isEmpty()) {
301-
throw new AssertionError("Response not received.");
302-
}
303284

304-
System.err.println("Received response from mock server, with request id: " + requestId
305-
+ ", status: " + responseHolder.get(0).getStatus());
285+
ResponseHolder<ByteBuf> responseHolder =
286+
testClient.submit(HttpClientRequest.createGet("").withHeader(REQUEST_ID_HEADER_NAME, requestId))
287+
.lift(FlatResponseOperator.<ByteBuf>flatResponse())
288+
.toBlocking().toFuture().get(1, TimeUnit.MINUTES);
306289

307-
HttpClientResponse<ByteBuf> response = responseHolder.get(0);
290+
System.err.println("Received response from mock server, with request id: " + requestId
291+
+ ", status: " + responseHolder.getResponse().getStatus());
308292

309-
if (response.getStatus().code() != HttpResponseStatus.OK.code()) {
310-
throw new MockBackendRequestFailedException("Test request failed. Status: " + response.getStatus().code());
293+
if (responseHolder.getResponse().getStatus().code() != HttpResponseStatus.OK.code()) {
294+
throw new MockBackendRequestFailedException("Test request failed. Status: "
295+
+ responseHolder.getResponse().getStatus().code());
311296
}
312297

313-
String requestIdGot = response.getHeaders().get(REQUEST_ID_HEADER_NAME);
298+
String requestIdGot = responseHolder.getResponse().getHeaders().get(REQUEST_ID_HEADER_NAME);
314299

315300
if (!requestId.equals(requestId)) {
316301
throw new MockBackendRequestFailedException("Request Id not sent from mock server. Expected: "

‎rx-netty/src/main/java/io/reactivex/netty/protocol/http/client/FlatResponseOperator.java

Lines changed: 27 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@
1818

1919
import rx.Observable;
2020
import rx.Subscriber;
21-
import rx.functions.Func1;
2221

2322
/**
2423
* An operator to be used for a source of {@link HttpClientResponse} containing aggregated responses i.e. which does not
@@ -52,12 +51,35 @@ public void onError(Throwable e) {
5251
public void onNext(final HttpClientResponse<T> response) {
5352
response.getContent()
5453
.take(1)
55-
.map(new Func1<T, ResponseHolder<T>>() {
54+
.lift(new Observable.Operator<ResponseHolder<T>, T>() {
5655
@Override
57-
public ResponseHolder<T> call(T t) {
58-
return new ResponseHolder<T>(response, t);
56+
public Subscriber<? super T> call(final Subscriber<? super ResponseHolder<T>> child) {
57+
return new Subscriber<T>() {
58+
59+
private boolean hasContent;
60+
61+
@Override
62+
public void onCompleted() {
63+
if (!hasContent) {
64+
child.onNext(new ResponseHolder<T>(response));
65+
}
66+
child.onCompleted();
67+
}
68+
69+
@Override
70+
public void onError(Throwable e) {
71+
child.onError(e);
72+
}
73+
74+
@Override
75+
public void onNext(T next) {
76+
hasContent = true;
77+
child.onNext(new ResponseHolder<T>(response, next));
78+
}
79+
};
5980
}
60-
}).subscribe(child);
81+
})
82+
.subscribe(child);
6183
}
6284
};
6385
}

‎rx-netty/src/main/java/io/reactivex/netty/protocol/http/client/ResponseHolder.java

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,11 +29,26 @@ public ResponseHolder(HttpClientResponse<T> response, T content) {
2929
this.content = content;
3030
}
3131

32+
public ResponseHolder(HttpClientResponse<T> response) {
33+
this.response = response;
34+
content = null;
35+
}
36+
3237
public HttpClientResponse<T> getResponse() {
3338
return response;
3439
}
3540

41+
/**
42+
* Returns the content, if any. Use {@link #hasContent()} to check if there is a content in this holder.
43+
*
44+
* @return The content, if any, {@code null} otherwise.
45+
* Use {@link #hasContent()} to check if there is a content in this holder.
46+
*/
3647
public T getContent() {
3748
return content;
3849
}
50+
51+
public boolean hasContent() {
52+
return null != content;
53+
}
3954
}

‎rx-netty/src/test/java/io/reactivex/netty/protocol/http/client/FlatResponseOperatorTest.java

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
/*
2+
* Copyright 2014 Netflix, Inc.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package io.reactivex.netty.protocol.http.client;
17+
18+
import io.netty.buffer.ByteBuf;
19+
import io.netty.buffer.Unpooled;
20+
import io.netty.handler.codec.http.DefaultHttpResponse;
21+
import io.netty.handler.codec.http.HttpResponseStatus;
22+
import io.netty.handler.codec.http.HttpVersion;
23+
import io.reactivex.netty.protocol.http.UnicastContentSubject;
24+
import org.junit.Assert;
25+
import org.junit.Test;
26+
import rx.Observable;
27+
28+
import java.util.concurrent.TimeUnit;
29+
30+
/**
31+
* @author Nitesh Kant
32+
*/
33+
public class FlatResponseOperatorTest {
34+
35+
@Test
36+
public void testContent() throws Exception {
37+
DefaultHttpResponse nettyResponse = new DefaultHttpResponse(HttpVersion.HTTP_1_1,
38+
HttpResponseStatus.NO_CONTENT);
39+
UnicastContentSubject<ByteBuf> contentSubject = UnicastContentSubject.createWithoutNoSubscriptionTimeout();
40+
contentSubject.onNext(Unpooled.buffer());
41+
contentSubject.onCompleted();
42+
43+
ResponseHolder<ByteBuf> holder = Observable.just(new HttpClientResponse<ByteBuf>(nettyResponse, contentSubject))
44+
.lift(FlatResponseOperator.<ByteBuf>flatResponse())
45+
.toBlocking().toFuture().get(1, TimeUnit.MINUTES);
46+
47+
Assert.assertEquals("Unexpected http response status", HttpResponseStatus.NO_CONTENT,
48+
holder.getResponse().getStatus());
49+
Assert.assertTrue("Response holder does not have content.", holder.hasContent());
50+
51+
}
52+
53+
@Test
54+
public void testNoContent() throws Exception {
55+
DefaultHttpResponse nettyResponse = new DefaultHttpResponse(HttpVersion.HTTP_1_1,
56+
HttpResponseStatus.NO_CONTENT);
57+
UnicastContentSubject<ByteBuf> contentSubject = UnicastContentSubject.createWithoutNoSubscriptionTimeout();
58+
contentSubject.onCompleted();
59+
60+
ResponseHolder<ByteBuf> holder = Observable.just(new HttpClientResponse<ByteBuf>(nettyResponse, contentSubject))
61+
.lift(FlatResponseOperator.<ByteBuf>flatResponse())
62+
.toBlocking().toFuture().get(1, TimeUnit.MINUTES);
63+
Assert.assertEquals("Unexpected http response status", HttpResponseStatus.NO_CONTENT,
64+
holder.getResponse().getStatus());
65+
Assert.assertFalse("Response holder has content.", holder.hasContent());
66+
67+
}
68+
}

0 commit comments

Comments
 (0)