Skip to content

Commit c55277b

Browse files
author
Sophie Guo
committed
Address comments
1 parent 43ca55d commit c55277b

File tree

3 files changed

+37
-26
lines changed

3 files changed

+37
-26
lines changed

ambry-commons/src/main/java/com/github/ambry/commons/RetainingAsyncWritableChannel.java

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -170,6 +170,26 @@ public ByteBuf consumeContentAsByteBuf() {
170170
}
171171
}
172172

173+
/**
174+
* Consume the collected content buffer and return its contents as a byte array.
175+
*
176+
* @return a byte array containing all data received up to this point.
177+
* @throws IllegalStateException if the content has already been consumed or the channel was closed.
178+
*/
179+
public byte[] consumeContentAsBytes() {
180+
ByteBuf buf = null;
181+
try {
182+
buf = consumeContentAsByteBuf();
183+
byte[] data = new byte[buf.readableBytes()];
184+
buf.readBytes(data);
185+
return data;
186+
} finally {
187+
if (buf != null) {
188+
buf.release();
189+
}
190+
}
191+
}
192+
173193
/**
174194
* @return an {@link InputStream} that contains the data from the chunks received up to this point in time.
175195
*/

ambry-frontend/src/main/java/com/github/ambry/frontend/s3/S3BatchDeleteHandler.java

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -187,19 +187,12 @@ private Callback<Long> parseRequestBodyAndDeleteCallback(RetainingAsyncWritableC
187187
*/
188188
public S3MessagePayload.S3BatchDeleteObjects deserializeRequest(RetainingAsyncWritableChannel channel)
189189
throws RestServiceException {
190-
ByteBuf byteBuffer = null;
191190
try {
192-
byteBuffer = channel.consumeContentAsByteBuf();
193-
byte[] byteArray = new byte[byteBuffer.readableBytes()];
194-
byteBuffer.readBytes(byteArray);
191+
byte[] byteArray = channel.consumeContentAsBytes();
195192
return new XmlMapper().readValue(byteArray, S3MessagePayload.S3BatchDeleteObjects.class);
196193
} catch (Exception e) {
197194
logger.trace("s3batchdelete failed to deserialize request");
198195
throw new RestServiceException("failed to deserialize", e, RestServiceErrorCode.BadRequest);
199-
} finally {
200-
if (byteBuffer != null) {
201-
byteBuffer.release();
202-
}
203196
}
204197
}
205198
}

ambry-frontend/src/test/java/com/github/ambry/frontend/S3BatchDeleteHandlerTest.java

Lines changed: 16 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -144,12 +144,9 @@ public void deleteObjectTest() throws Exception {
144144
}
145145

146146
@Test
147-
public void testBufferReleaseInDeserializeRequest() throws Exception {
148-
// Arrange
149-
S3BatchDeleteHandler handler = new S3BatchDeleteHandler(null, null);
150-
RetainingAsyncWritableChannel channel = mock(RetainingAsyncWritableChannel.class);
151-
ByteBuf mockByteBuf = mock(ByteBuf.class);
152-
147+
public void testConsumeContentAsBytesReleasesBuffer() {
148+
// Arrange: Create a ByteBuf with some data
149+
ByteBuf testBuf = Unpooled.buffer();
153150
String validXml = "<?xml version=\"1.0\" encoding=\"UTF-8\"?>" +
154151
"<Delete xmlns=\"http://s3.amazonaws.com/doc/2006-03-01\">" +
155152
"<Object>" +
@@ -165,21 +162,22 @@ public void testBufferReleaseInDeserializeRequest() throws Exception {
165162
"<Key>key-success-2</Key>" +
166163
"</Object>" +
167164
"</Delete>";
168-
169165
byte[] xmlBytes = validXml.getBytes();
166+
testBuf.writeBytes(xmlBytes);
167+
// Wrap the consumeContentAsByteBuf method to return the test buffer
168+
RetainingAsyncWritableChannel channel = new RetainingAsyncWritableChannel() {
169+
@Override
170+
public ByteBuf consumeContentAsByteBuf() {
171+
return testBuf;
172+
}
173+
};
170174

171-
when(channel.consumeContentAsByteBuf()).thenReturn(mockByteBuf);
172-
when(mockByteBuf.readableBytes()).thenReturn(xmlBytes.length);
173-
doAnswer(invocation -> {
174-
byte[] buffer = invocation.getArgument(0);
175-
System.arraycopy(xmlBytes, 0, buffer, 0, xmlBytes.length);
176-
return null;
177-
}).when(mockByteBuf).readBytes(Mockito.any(byte[].class));
178-
179-
S3MessagePayload.S3BatchDeleteObjects result = handler.deserializeRequest(channel);
175+
// Act: Call the method
176+
byte[] result = channel.consumeContentAsBytes();
180177

181-
assertNotNull(result);
182-
verify(mockByteBuf, times(1)).release(); // Verify that release() was called
178+
// Assert: Verify the buffer was released and the data matches
179+
assertArrayEquals(xmlBytes, result);
180+
assertEquals("Buffer should be released", 0, testBuf.refCnt());
183181
}
184182

185183
@Test

0 commit comments

Comments
 (0)