test(protocol): Add unit tests for BytesReader.peek()

- Add missing tests for the `peek()` Method.html
- Make the usage of `ProtocolResponseWriter` more safer
by adding scope and state checks and updating affected
classes
- Add the `ResponseBody.writeTo(BufferedSink)` method

Author: Georgi Neykov

binapi-client/src/main/java/com/pcloud/networking/client/RealCall.java

@@ -1,11 +1,11 @@
 /*
- * Copyright (c) 2017 pCloud AG
+ * Copyright (c) 2020 pCloud AG
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
  * You may obtain a copy of the License at
  *
- *     http://www.apache.org/licenses/LICENSE-2.0
+ *      http://www.apache.org/licenses/LICENSE-2.0
  *
  * Unless required by applicable law or agreed to in writing, software
  * distributed under the License is distributed on an "AS IS" BASIS,
@@ -21,6 +21,7 @@
 import com.pcloud.networking.protocol.ProtocolRequestWriter;
 import com.pcloud.networking.protocol.ProtocolResponseReader;
 import com.pcloud.utils.IOUtils;
+import okio.BufferedSink;
 import okio.BufferedSource;
 import okio.ByteString;
 import okio.Okio;
@@ -244,6 +245,11 @@ public Endpoint endpoint() {
                 return endpoint;
             }
 
+            @Override
+            public void writeTo(BufferedSink sink) throws IOException {
+                source.readAll(sink);
+            }
+
             @Override
             public ResponseData data() throws IOException {
                 int scope = reader.currentScope();

binapi-client/src/main/java/com/pcloud/networking/client/RealMultiCall.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2017 pCloud AG
+ * Copyright (c) 2020 pCloud AG
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -23,6 +23,7 @@
 import com.pcloud.networking.protocol.ProtocolResponseReader;
 import com.pcloud.utils.IOUtils;
 import okio.Buffer;
+import okio.BufferedSink;
 import okio.BufferedSource;
 import okio.ByteString;
 import okio.Okio;
@@ -457,6 +458,11 @@ public Endpoint endpoint() {
             return endpoint;
         }
 
+        @Override
+        public void writeTo(BufferedSink sink) throws IOException {
+            source.readAll(sink);
+        }
+
         @Override
         public void close() {
             source.close();
@@ -508,6 +514,11 @@ public Endpoint endpoint() {
             return endpoint;
         }
 
+        @Override
+        public void writeTo(BufferedSink sink) throws IOException {
+            bufferedSource.readAll(sink);
+        }
+
         @Override
         public void close() throws IOException {
             source.close();

binapi-client/src/main/java/com/pcloud/networking/client/ResponseBody.java

@@ -18,6 +18,7 @@
 
 import com.pcloud.networking.protocol.ProtocolReader;
 import com.pcloud.networking.protocol.ValueReader;
+import okio.BufferedSink;
 import okio.ByteString;
 
 import java.io.Closeable;
@@ -97,6 +98,14 @@ public final byte[] valuesByteArray() throws IOException {
      */
     public abstract Endpoint endpoint();
 
+    /**
+     * Write the body content to a {@linkplain BufferedSink}
+     *
+     * @param sink the receiver of the data
+     * @throws IOException on failed IO operations
+     */
+    public abstract void writeTo(BufferedSink sink) throws IOException;
+
     @Override
     public String toString() {
         return String.format("(%s)->[Response]: %d bytes", endpoint(), contentLength());

binapi-client/src/test/java/com/pcloud/networking/client/RealApiChannelTest.java

@@ -113,13 +113,17 @@ public void writer_Writes_To_Connection_Sink() throws Exception {
 
     private ByteString mockResponse() throws IOException {
         return new ResponseBytesWriter()
+                .beginObject()
                 .writeValue("result", 5000)
+                .endObject()
                 .bytes();
     }
 
     private ByteString mockDataResponse() throws IOException {
         return new ResponseBytesWriter()
+                .beginObject()
                 .setData(ByteString.encodeUtf8("Some data"))
+                .endObject()
                 .bytes();
     }
 
@@ -212,7 +216,7 @@ public void Reader_endResponse_Throws_ClosedChannelException_On_Closed_Instance(
     public void Reader_beginArray_Throws_ClosedChannelException_On_Closed_Instance() throws Exception {
         ApiChannel apiChannel = createChannelInstance();
         connection.readBuffer().write(mockResponse());
-                apiChannel.reader().beginResponse();
+        apiChannel.reader().beginResponse();
         apiChannel.close();
         expectException(apiChannel.reader(), ClosedChannelException.class).beginArray();
     }
@@ -221,14 +225,18 @@ public void Reader_beginArray_Throws_ClosedChannelException_On_Closed_Instance()
     public void Reader_endArray_Throws_ClosedChannelException_On_Closed_Instance() throws Exception {
         ApiChannel apiChannel = createChannelInstance();
         connection.readBuffer().write(new ResponseBytesWriter()
+                .beginObject()
+                .writeKey("array")
                 .beginArray()
                 .write("1")
                 .endArray()
+                .endObject()
                 .bytes());
 
         ProtocolResponseReader reader = apiChannel.reader();
         reader.beginResponse();
         reader.beginObject();
+        reader.readString();
         reader.beginArray();
         apiChannel.close();
         expectException(reader, ClosedChannelException.class).endArray();

binapi-client/src/test/java/com/pcloud/networking/client/RealMultiCallTest.java

@@ -17,34 +17,20 @@
 package com.pcloud.networking.client;
 
 import com.pcloud.networking.protocol.ResponseBytesWriter;
+import okio.ByteString;
 import org.assertj.core.api.ThrowableAssert;
-import org.junit.AfterClass;
-import org.junit.Before;
-import org.junit.BeforeClass;
-import org.junit.Rule;
-import org.junit.Test;
+import org.junit.*;
 import org.junit.rules.ExpectedException;
 import org.mockito.ArgumentCaptor;
 import org.mockito.invocation.InvocationOnMock;
 import org.mockito.stubbing.Answer;
 
 import java.io.IOException;
 import java.nio.ByteBuffer;
-import java.util.ArrayList;
-import java.util.Arrays;
-import java.util.Collection;
-import java.util.Collections;
-import java.util.List;
-import java.util.Map;
-import java.util.concurrent.ExecutorService;
-import java.util.concurrent.SynchronousQueue;
-import java.util.concurrent.ThreadFactory;
-import java.util.concurrent.ThreadPoolExecutor;
-import java.util.concurrent.TimeUnit;
-import java.util.concurrent.TimeoutException;
+import java.util.*;
+import java.util.concurrent.*;
 
 import static org.assertj.core.api.Assertions.assertThatThrownBy;
-import static org.hamcrest.CoreMatchers.hasItem;
 import static org.junit.Assert.*;
 import static org.mockito.Mockito.*;
 
@@ -270,7 +256,7 @@ public Object answer(InvocationOnMock invocation) throws Throwable {
         verify(connection).close();
     }
 
-@Test
+    @Test
     public void test_Interactor_WritesRequests() throws IOException {
 
         Request request1 = Request.create()
@@ -292,26 +278,26 @@ public void test_Interactor_WritesRequests() throws IOException {
                         .build())
                 .build();
 
-    Request expectedRequest1 = Request.create()
-            .methodName("someApiMethod")
-            .body(TestRequestBody.create()
-                    .setValue("id", 0)
-                    .setValue("arg1", "value1")
-                    .setValue("arg2", "value2")
-                    .setValue("arg3", "value3")
-                    .setValue("arg4", "value4")
-                    .build())
-            .build();
-
-    Request expectedRequest2 = request1.newRequest()
-            .body(TestRequestBody.create()
-                    .setValue("id", 1)
-                    .setValue("arg5", "value5")
-                    .setValue("arg6", "value6")
-                    .setValue("arg7", "value7")
-                    .setValue("arg8", "value8")
-                    .build())
-            .build();
+        Request expectedRequest1 = Request.create()
+                .methodName("someApiMethod")
+                .body(TestRequestBody.create()
+                        .setValue("id", 0)
+                        .setValue("arg1", "value1")
+                        .setValue("arg2", "value2")
+                        .setValue("arg3", "value3")
+                        .setValue("arg4", "value4")
+                        .build())
+                .build();
+
+        Request expectedRequest2 = request1.newRequest()
+                .body(TestRequestBody.create()
+                        .setValue("id", 1)
+                        .setValue("arg5", "value5")
+                        .setValue("arg6", "value6")
+                        .setValue("arg7", "value7")
+                        .setValue("arg8", "value8")
+                        .build())
+                .build();
 
         Connection connection = spy(new DummyConnection());
         retrofitConnectionProvider(connection);
@@ -365,24 +351,30 @@ public void interactor_ReadsResponses() throws IOException {
 
         List<ResponseBytesWriter> expectedResponses = Arrays.asList(
                 new ResponseBytesWriter()
+                        .beginObject()
                         .writeValue("id", 0)
-                        .writeValue("result", 0),
+                        .writeValue("result", 0)
+                        .endObject(),
                 new ResponseBytesWriter()
+                        .beginObject()
                         .writeValue("id", 1)
                         .writeValue("result", 5000)
-                        .writeValue("error", "Something went ka-boom."),
+                        .writeValue("error", "Something went ka-boom.")
+                        .endObject(),
                 new ResponseBytesWriter()
+                        .beginObject()
                         .writeValue("id", 2)
                         .writeValue("result", 2000)
-                        .writeValue("error", "Token went ka-boom."));
+                        .writeValue("error", "Token went ka-boom.")
+                        .endObject());
         Connection connection = spy(DummyConnection.withResponses(expectedResponses));
         retrofitConnectionProvider(connection);
 
         MultiCall call = createMultiCall(request1, request1, request1);
         Interactor interactor = call.start();
         interactor.submitRequests(Integer.MAX_VALUE);
 
-        while (interactor.hasNextResponse()){
+        while (interactor.hasNextResponse()) {
             assertContainsResponse(expectedResponses, interactor.nextResponse());
         }
 
@@ -394,14 +386,18 @@ public void interactor_ReadsResponses() throws IOException {
     }
 
     private static void assertContainsResponse(Collection<ResponseBytesWriter> responses, Response response) throws IOException {
-        Map<String,?> responseValues = response.responseBody().toValues();
-
-        Collection<Map<String,?>> expectedValues = new ArrayList<>(responses.size());
-        for (ResponseBytesWriter bytes : responses){
-            expectedValues.add(bytes.toValues());
+        ByteString responseBytes = new okio.Buffer()
+                .writeIntLe((int) response.responseBody().contentLength())
+                .write(response.responseBody().valuesBytes())
+                .snapshot();
+        for (ResponseBytesWriter writer : responses) {
+            ByteString expected = writer.bytes();
+            if (expected.equals(responseBytes)) {
+                return;
+            }
         }
 
-        assertThat(expectedValues, hasItem(responseValues));
+        throw new AssertionError("Response does not match any of the expected responses.");
     }
 
     private byte[] getMockByteDataResponse(int numberOfRequests) {