diff --git a/ambry-protocol/src/main/java/com/github/ambry/protocol/FileCopyGetChunkRequest.java b/ambry-protocol/src/main/java/com/github/ambry/protocol/FileCopyGetChunkRequest.java index 10a94865c8..c9a75ffc8a 100644 --- a/ambry-protocol/src/main/java/com/github/ambry/protocol/FileCopyGetChunkRequest.java +++ b/ambry-protocol/src/main/java/com/github/ambry/protocol/FileCopyGetChunkRequest.java @@ -34,7 +34,7 @@ public FileCopyGetChunkRequest( short versionId, int correlationId, String clientId, PartitionId partitionId, String fileName, long startOffset, long sizeInBytes) { super(RequestOrResponseType.FileCopyGetChunkRequest, versionId, correlationId, clientId); if(partitionId == null || fileName.isEmpty() || startOffset < 0 || sizeInBytes < 0){ - throw new IllegalArgumentException("PartitionId, FileName, StartOffset and SizeInBytes cannot be null or negative"); + throw new IllegalArgumentException("PartitionId, FileName, StartOffset or SizeInBytes cannot be null or negative"); } this.partitionId = partitionId; this.fileName = fileName; diff --git a/ambry-protocol/src/main/java/com/github/ambry/protocol/FileCopyGetMetaDataRequest.java b/ambry-protocol/src/main/java/com/github/ambry/protocol/FileCopyGetMetaDataRequest.java index 4fd41bf9df..ee87c3b236 100644 --- a/ambry-protocol/src/main/java/com/github/ambry/protocol/FileCopyGetMetaDataRequest.java +++ b/ambry-protocol/src/main/java/com/github/ambry/protocol/FileCopyGetMetaDataRequest.java @@ -29,8 +29,11 @@ public class FileCopyGetMetaDataRequest extends RequestOrResponse{ public FileCopyGetMetaDataRequest(short versionId, int correlationId, String clientId, PartitionId partitionId, String hostName) { super(RequestOrResponseType.FileCopyGetMetaDataRequest, versionId, correlationId, clientId); - if (partitionId == null || hostName.isEmpty()) { - throw new IllegalArgumentException("Partition and Host Name cannot be null"); + if (partitionId == null) { + throw new IllegalArgumentException("Partition cannot be null"); + } + if (hostName.isEmpty()){ + throw new IllegalArgumentException("Host Name cannot be null"); } this.partitionId = partitionId; this.hostName = hostName; diff --git a/ambry-protocol/src/main/java/com/github/ambry/protocol/FileCopyGetMetaDataResponse.java b/ambry-protocol/src/main/java/com/github/ambry/protocol/FileCopyGetMetaDataResponse.java index 939df8bb04..d8ec80fdeb 100644 --- a/ambry-protocol/src/main/java/com/github/ambry/protocol/FileCopyGetMetaDataResponse.java +++ b/ambry-protocol/src/main/java/com/github/ambry/protocol/FileCopyGetMetaDataResponse.java @@ -38,7 +38,8 @@ public FileCopyGetMetaDataResponse(short versionId, int correlationId, String cl public static FileCopyGetMetaDataResponse readFrom(DataInputStream stream) throws IOException { RequestOrResponseType type = RequestOrResponseType.values()[stream.readShort()]; if (type != RequestOrResponseType.FileCopyGetMetaDataResponse) { - throw new IllegalArgumentException("The type of request response is not compatible"); + throw new IllegalArgumentException("The type of request response is not compatible. Expected : {}, Actual : {}" + + RequestOrResponseType.FileCopyGetMetaDataResponse + type); } short versionId = stream.readShort(); int correlationId = stream.readInt(); @@ -46,7 +47,8 @@ public static FileCopyGetMetaDataResponse readFrom(DataInputStream stream) throw ServerErrorCode errorCode = ServerErrorCode.values()[stream.readShort()]; if(errorCode != ServerErrorCode.No_Error) { - return new FileCopyGetMetaDataResponse(versionId, correlationId, clientId, -1, new ArrayList<>(), errorCode); + //Setting the number of logfiles to 0 as there are no logfiles to be read. + return new FileCopyGetMetaDataResponse(versionId, correlationId, clientId, 0, new ArrayList<>(), errorCode); } int numberOfLogfiles = stream.readInt(); diff --git a/ambry-protocol/src/main/java/com/github/ambry/protocol/FileInfo.java b/ambry-protocol/src/main/java/com/github/ambry/protocol/FileInfo.java index 8b4409c5cd..020a8348f4 100644 --- a/ambry-protocol/src/main/java/com/github/ambry/protocol/FileInfo.java +++ b/ambry-protocol/src/main/java/com/github/ambry/protocol/FileInfo.java @@ -19,6 +19,11 @@ import java.io.IOException; import java.nio.charset.Charset; + +/** + * Contains the fileName and fileSizeInBytes for a local partition. This is used + * by LogInfo as part of filecopy metadata request. + */ public class FileInfo { private String fileName; private long fileSizeInBytes; diff --git a/ambry-protocol/src/main/java/com/github/ambry/protocol/LogInfo.java b/ambry-protocol/src/main/java/com/github/ambry/protocol/LogInfo.java index 4ab4d2b9f7..fb8d131a9c 100644 --- a/ambry-protocol/src/main/java/com/github/ambry/protocol/LogInfo.java +++ b/ambry-protocol/src/main/java/com/github/ambry/protocol/LogInfo.java @@ -21,6 +21,11 @@ import java.util.ArrayList; import java.util.List; + +/** + * Contains the fileName, fileSizeInBytes, indexFiles and bloomFilters for a local partition. This is used + * by filecopy metadata request. + */ public class LogInfo { private String fileName; private long fileSizeInBytes; diff --git a/ambry-protocol/src/test/java/com/github/ambry/protocol/RequestResponseTest.java b/ambry-protocol/src/test/java/com/github/ambry/protocol/RequestResponseTest.java index 18824e4941..7bb4b1802e 100644 --- a/ambry-protocol/src/test/java/com/github/ambry/protocol/RequestResponseTest.java +++ b/ambry-protocol/src/test/java/com/github/ambry/protocol/RequestResponseTest.java @@ -65,6 +65,7 @@ import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.Parameterized; +import org.mockito.Mock; import static com.github.ambry.account.Account.*; import static com.github.ambry.account.Container.*; @@ -687,6 +688,22 @@ public void doFileCopyMetaDataRequestTest() throws IOException { Assert.assertEquals(fileMetadataRequestFromBytes.getPartitionId().toPathString(), "0"); request.release(); + + try{ + new FileCopyGetMetaDataRequest(requestVersionToUse, 111, "id1", + null, "host3"); + Assert.fail("Should have thrown exception"); + }catch (IllegalArgumentException e) { + //expected + } + + try{ + new FileCopyGetMetaDataRequest(requestVersionToUse, 111, "id1", + new MockPartitionId(), ""); + Assert.fail("Should have thrown exception"); + }catch (IllegalArgumentException e) { + //expected + } } @Test @@ -755,6 +772,16 @@ public void doFileCopyMetaDataResponseTest() throws IOException{ Assert.assertEquals(fileCopyProtocolMetaDataResponseTranformed.getLogInfos().get(1).getBloomFilters().get(0).getFileName(), "1_1_bloom"); Assert.assertEquals(fileCopyProtocolMetaDataResponseTranformed.getLogInfos().get(1).getBloomFilters().get(0).getFileSizeInBytes(), 1040); response.release(); + + + response = + new FileCopyGetMetaDataResponse(requestVersionToUse, 111, "id1", 2 , + logInfoList, ServerErrorCode.IO_Error); + DataInputStream requestStream1 = serAndPrepForRead(response, -1, false); + FileCopyGetMetaDataResponse fileCopyProtocolGetChunkResponse = + FileCopyGetMetaDataResponse.readFrom(requestStream1); + Assert.assertEquals(fileCopyProtocolGetChunkResponse.getError(), ServerErrorCode.IO_Error); + response.release(); } @Test @@ -773,6 +800,39 @@ public void doFileCopyChunkRequestTest() throws IOException{ Assert.assertEquals(request.getStartOffset(), 1000); Assert.assertEquals(request.getVersionId(), requestVersionToUse); request.release(); + + try{ + new FileCopyGetChunkRequest(requestVersionToUse, 111, "id1", + null, "file1", 1000, 0); + Assert.fail("Should have failed"); + } catch (IllegalArgumentException e){ + //expected + } + + try{ + new FileCopyGetChunkRequest(requestVersionToUse, 111, "id1", + new MockPartitionId(), "", 1000, 0); + Assert.fail("Should have failed"); + } catch (IllegalArgumentException e){ + //expected + } + + try{ + new FileCopyGetChunkRequest(requestVersionToUse, 111, "id1", + new MockPartitionId(), "file1", -1, 0); + Assert.fail("Should have failed"); + } catch (IllegalArgumentException e){ + //expected + } + + try{ + new FileCopyGetChunkRequest(requestVersionToUse, 111, "id1", + new MockPartitionId(), "file1", 1000, -1); + Assert.fail("Should have failed"); + } catch (IllegalArgumentException e){ + //expected + } + } private void doReplicaMetadataRequestTest(short responseVersionToUse, short requestVersionToUse,