Skip to content

Commit 1667bd4

Browse files
committed
Lazy load multipart ByteBuf, close AsyncHttpClient#1030
1 parent f6087a4 commit 1667bd4

File tree

5 files changed

+92
-64
lines changed

5 files changed

+92
-64
lines changed

client/src/main/java/org/asynchttpclient/request/body/multipart/part/ByteArrayMultipartPart.java

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,8 @@
2323

2424
public class ByteArrayMultipartPart extends MultipartPart<ByteArrayPart> {
2525

26-
private final ByteBuf contentBuffer;
26+
// lazy
27+
private ByteBuf contentBuffer;
2728

2829
public ByteArrayMultipartPart(ByteArrayPart part, byte[] boundary) {
2930
super(part, boundary);
@@ -37,14 +38,20 @@ protected long getContentLength() {
3738

3839
@Override
3940
protected long transferContentTo(ByteBuf target) throws IOException {
40-
return transfer(contentBuffer, target, MultipartState.POST_CONTENT);
41+
return transfer(lazyLoadContentBuffer(), target, MultipartState.POST_CONTENT);
4142
}
42-
43+
4344
@Override
4445
protected long transferContentTo(WritableByteChannel target) throws IOException {
45-
return transfer(contentBuffer, target, MultipartState.POST_CONTENT);
46+
return transfer(lazyLoadContentBuffer(), target, MultipartState.POST_CONTENT);
47+
}
48+
49+
private ByteBuf lazyLoadContentBuffer() {
50+
if (contentBuffer == null)
51+
contentBuffer = Unpooled.wrappedBuffer(part.getBytes());
52+
return contentBuffer;
4653
}
47-
54+
4855
@Override
4956
public void close() {
5057
super.close();

client/src/main/java/org/asynchttpclient/request/body/multipart/part/MessageEndMultipartPart.java

Lines changed: 27 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -25,33 +25,50 @@
2525

2626
public class MessageEndMultipartPart extends MultipartPart<FileLikePart> {
2727

28-
private final ByteBuf buffer;
28+
// lazy
29+
private ByteBuf contentBuffer;
2930

3031
public MessageEndMultipartPart(byte[] boundary) {
3132
super(null, boundary);
32-
buffer = ByteBufAllocator.DEFAULT.buffer((int) length());
33-
buffer.writeBytes(EXTRA_BYTES).writeBytes(boundary).writeBytes(EXTRA_BYTES).writeBytes(CRLF_BYTES);
3433
state = MultipartState.PRE_CONTENT;
3534
}
3635

3736
@Override
3837
public long transferTo(ByteBuf target) throws IOException {
39-
return transfer(buffer, target, MultipartState.DONE);
38+
return transfer(lazyLoadContentBuffer(), target, MultipartState.DONE);
4039
}
4140

4241
@Override
4342
public long transferTo(WritableByteChannel target) throws IOException {
4443
slowTarget = false;
45-
return transfer(buffer, target, MultipartState.DONE);
44+
return transfer(lazyLoadContentBuffer(), target, MultipartState.DONE);
45+
}
46+
47+
private ByteBuf lazyLoadContentBuffer() {
48+
if (contentBuffer == null) {
49+
contentBuffer = ByteBufAllocator.DEFAULT.buffer((int) getContentLength());
50+
contentBuffer.writeBytes(EXTRA_BYTES).writeBytes(boundary).writeBytes(EXTRA_BYTES).writeBytes(CRLF_BYTES);
51+
}
52+
return contentBuffer;
53+
}
54+
55+
@Override
56+
protected int computePreContentLength() {
57+
return 0;
4658
}
4759

4860
@Override
49-
protected ByteBuf computePreContentBytes() {
61+
protected ByteBuf computePreContentBytes(int preContentLength) {
5062
return Unpooled.EMPTY_BUFFER;
5163
}
5264

5365
@Override
54-
protected ByteBuf computePostContentBytes() {
66+
protected int computePostContentLength() {
67+
return 0;
68+
}
69+
70+
@Override
71+
protected ByteBuf computePostContentBytes(int postContentLength) {
5572
return Unpooled.EMPTY_BUFFER;
5673
}
5774

@@ -72,6 +89,8 @@ protected long transferContentTo(WritableByteChannel target) throws IOException
7289

7390
@Override
7491
public void close() {
75-
buffer.release();
92+
super.close();
93+
if (contentBuffer != null)
94+
contentBuffer.release();
7695
}
7796
}

client/src/main/java/org/asynchttpclient/request/body/multipart/part/MultipartPart.java

Lines changed: 39 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -90,23 +90,25 @@ public abstract class MultipartPart<T extends FileLikePart> implements Closeable
9090
protected final T part;
9191
protected final byte[] boundary;
9292

93-
private final long length;
94-
private ByteBuf preContentBuffer;
95-
private ByteBuf postContentBuffer;
93+
private final int preContentLength;
94+
private final int postContentLength;
9695
protected MultipartState state;
9796
protected boolean slowTarget;
9897

98+
// lazy
99+
private ByteBuf preContentBuffer;
100+
private ByteBuf postContentBuffer;
101+
99102
public MultipartPart(T part, byte[] boundary) {
100103
this.part = part;
101104
this.boundary = boundary;
102-
preContentBuffer = computePreContentBytes();
103-
postContentBuffer = computePostContentBytes();
104-
length = preContentBuffer.readableBytes() + postContentBuffer.readableBytes() + getContentLength();
105+
preContentLength = computePreContentLength();
106+
postContentLength = computePostContentLength();
105107
state = MultipartState.PRE_CONTENT;
106108
}
107109

108110
public long length() {
109-
return length;
111+
return preContentLength + postContentLength + getContentLength();
110112
}
111113

112114
public MultipartState getState() {
@@ -124,13 +126,13 @@ public long transferTo(ByteBuf target) throws IOException {
124126
return 0L;
125127

126128
case PRE_CONTENT:
127-
return transfer(preContentBuffer, target, MultipartState.CONTENT);
129+
return transfer(lazyLoadPreContentBuffer(), target, MultipartState.CONTENT);
128130

129131
case CONTENT:
130132
return transferContentTo(target);
131133

132134
case POST_CONTENT:
133-
return transfer(postContentBuffer, target, MultipartState.DONE);
135+
return transfer(lazyLoadPostContentBuffer(), target, MultipartState.DONE);
134136

135137
default:
136138
throw new IllegalStateException("Unknown state " + state);
@@ -145,23 +147,37 @@ public long transferTo(WritableByteChannel target) throws IOException {
145147
return 0L;
146148

147149
case PRE_CONTENT:
148-
return transfer(preContentBuffer, target, MultipartState.CONTENT);
150+
return transfer(lazyLoadPreContentBuffer(), target, MultipartState.CONTENT);
149151

150152
case CONTENT:
151153
return transferContentTo(target);
152154

153155
case POST_CONTENT:
154-
return transfer(postContentBuffer, target, MultipartState.DONE);
156+
return transfer(lazyLoadPostContentBuffer(), target, MultipartState.DONE);
155157

156158
default:
157159
throw new IllegalStateException("Unknown state " + state);
158160
}
159161
}
160162

163+
private ByteBuf lazyLoadPreContentBuffer() {
164+
if (preContentBuffer == null)
165+
preContentBuffer = computePreContentBytes(preContentLength);
166+
return preContentBuffer;
167+
}
168+
169+
private ByteBuf lazyLoadPostContentBuffer() {
170+
if (postContentBuffer == null)
171+
postContentBuffer = computePostContentBytes(postContentLength);
172+
return postContentBuffer;
173+
}
174+
161175
@Override
162176
public void close() {
163-
preContentBuffer.release();
164-
postContentBuffer.release();
177+
if (preContentBuffer != null)
178+
preContentBuffer.release();
179+
if (postContentBuffer != null)
180+
postContentBuffer.release();
165181
}
166182

167183
protected abstract long getContentLength();
@@ -212,29 +228,27 @@ protected long transfer(ByteBuf source, WritableByteChannel target, MultipartSta
212228
return transferred;
213229
}
214230

215-
protected ByteBuf computePreContentBytes() {
216-
217-
// compute length
231+
protected int computePreContentLength() {
218232
CounterPartVisitor counterVisitor = new CounterPartVisitor();
219233
visitPreContent(counterVisitor);
220-
long length = counterVisitor.getCount();
234+
return counterVisitor.getCount();
235+
}
221236

222-
// compute bytes
223-
ByteBuf buffer = ByteBufAllocator.DEFAULT.buffer((int) length);
237+
protected ByteBuf computePreContentBytes(int preContentLength) {
238+
ByteBuf buffer = ByteBufAllocator.DEFAULT.buffer(preContentLength);
224239
ByteBufVisitor bytesVisitor = new ByteBufVisitor(buffer);
225240
visitPreContent(bytesVisitor);
226241
return buffer;
227242
}
228243

229-
protected ByteBuf computePostContentBytes() {
230-
231-
// compute length
244+
protected int computePostContentLength() {
232245
CounterPartVisitor counterVisitor = new CounterPartVisitor();
233246
visitPostContent(counterVisitor);
234-
long length = counterVisitor.getCount();
247+
return counterVisitor.getCount();
248+
}
235249

236-
// compute bytes
237-
ByteBuf buffer = ByteBufAllocator.DEFAULT.buffer((int) length);
250+
protected ByteBuf computePostContentBytes(int postContentLength) {
251+
ByteBuf buffer = ByteBufAllocator.DEFAULT.buffer(postContentLength);
238252
ByteBufVisitor bytesVisitor = new ByteBufVisitor(buffer);
239253
visitPostContent(bytesVisitor);
240254
return buffer;

client/src/main/java/org/asynchttpclient/request/body/multipart/part/PartVisitor.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ public interface PartVisitor {
2424

2525
class CounterPartVisitor implements PartVisitor {
2626

27-
private long count = 0L;
27+
private int count = 0;
2828

2929
@Override
3030
public void withBytes(byte[] bytes) {
@@ -36,7 +36,7 @@ public void withByte(byte b) {
3636
count++;
3737
}
3838

39-
public long getCount() {
39+
public int getCount() {
4040
return count;
4141
}
4242
}

client/src/test/java/org/asynchttpclient/request/body/multipart/MultipartBodyTest.java

Lines changed: 12 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
package org.asynchttpclient.request.body.multipart;
1414

1515
import static java.nio.charset.StandardCharsets.UTF_8;
16+
import static org.testng.Assert.*;
1617
import io.netty.buffer.ByteBuf;
1718
import io.netty.buffer.Unpooled;
1819
import io.netty.handler.codec.http.HttpHeaders;
@@ -24,19 +25,19 @@
2425
import java.util.ArrayList;
2526
import java.util.List;
2627

27-
import org.asynchttpclient.request.body.Body;
28+
import org.apache.commons.io.IOUtils;
2829
import org.asynchttpclient.request.body.Body.BodyState;
29-
import org.testng.Assert;
3030
import org.testng.annotations.Test;
3131

3232
public class MultipartBodyTest {
3333

3434
@Test(groups = "standalone")
35-
public void testBasics() throws IOException {
35+
public void testBasics() throws Exception {
3636
final List<Part> parts = new ArrayList<>();
3737

3838
// add a file
3939
final File testFile = getTestfile();
40+
System.err.println(testFile.length());
4041
parts.add(new FilePart("filePart", testFile));
4142

4243
// add a byte array
@@ -48,38 +49,25 @@ public void testBasics() throws IOException {
4849
compareContentLength(parts);
4950
}
5051

51-
private static File getTestfile() {
52+
private static File getTestfile() throws URISyntaxException {
5253
final ClassLoader cl = MultipartBodyTest.class.getClassLoader();
5354
final URL url = cl.getResource("textfile.txt");
54-
Assert.assertNotNull(url);
55-
File file = null;
56-
try {
57-
file = new File(url.toURI());
58-
} catch (URISyntaxException use) {
59-
Assert.fail("uri syntax error");
60-
}
61-
return file;
55+
assertNotNull(url);
56+
return new File(url.toURI());
6257
}
6358

6459
private static void compareContentLength(final List<Part> parts) throws IOException {
65-
Assert.assertNotNull(parts);
60+
assertNotNull(parts);
6661
// get expected values
67-
final Body multipartBody = MultipartUtils.newMultipartBody(parts, HttpHeaders.EMPTY_HEADERS);
62+
final MultipartBody multipartBody = MultipartUtils.newMultipartBody(parts, HttpHeaders.EMPTY_HEADERS);
6863
final long expectedContentLength = multipartBody.getContentLength();
6964
try {
7065
final ByteBuf buffer = Unpooled.buffer(8192);
71-
boolean last = false;
72-
while (!last) {
73-
if (multipartBody.transferTo(buffer) == BodyState.STOP) {
74-
last = true;
75-
}
66+
while (multipartBody.transferTo(buffer) != BodyState.STOP) {
7667
}
77-
Assert.assertEquals(buffer.readableBytes(), expectedContentLength);
68+
assertEquals(buffer.readableBytes(), expectedContentLength);
7869
} finally {
79-
try {
80-
multipartBody.close();
81-
} catch (IOException ignore) {
82-
}
70+
IOUtils.closeQuietly(multipartBody);
8371
}
8472
}
8573
}

0 commit comments

Comments
 (0)