core: make BaseWriteChannel surprise less (#2900)

pongad · web-flow · commit 21e7f4ccaf70 · 2018-02-15T09:00:42.000+11:00
Fixes #2895. The chunk size should round up, not down. Fixes #2896 by removing flush from capture. It is too surprising that capture might fail.
diff --git a/google-cloud-core/src/main/java/com/google/cloud/BaseWriteChannel.java b/google-cloud-core/src/main/java/com/google/cloud/BaseWriteChannel.java
@@ -104,8 +104,9 @@ protected int getChunkSize() {
 
   @Override
   public final void setChunkSize(int chunkSize) {
-    chunkSize = (chunkSize / getMinChunkSize()) * getMinChunkSize();
-    this.chunkSize = Math.max(getMinChunkSize(), chunkSize);
+    int minSize = getMinChunkSize();
+
+    this.chunkSize = Math.max(minSize, (chunkSize + minSize - 1) / minSize * minSize);
   }
 
   @InternalApi("This class should only be extended within google-cloud-java")
@@ -173,7 +174,6 @@ public final void close() throws IOException {
   public RestorableState<WriteChannel> capture() {
     byte[] bufferToSave = null;
     if (isOpen) {
-      flush();
       bufferToSave = Arrays.copyOf(buffer, limit);
     }
     return stateBuilder()
diff --git a/google-cloud-core/src/main/java/com/google/cloud/WriteChannel.java b/google-cloud-core/src/main/java/com/google/cloud/WriteChannel.java
@@ -23,13 +23,11 @@
  * A channel for writing data to Google Cloud services.
  *
  * <p>Implementations of this class may further buffer data internally to reduce remote calls.
- * Written data will only be visible after calling {@link #close()}. This interface implements
+ * Written data might not be visible until calling {@link #close()}. This interface implements
  * {@link Restorable} to allow saving the writer's state to continue writing afterwards.
- * </p>
  */
 public interface WriteChannel extends WritableByteChannel, Closeable, Restorable<WriteChannel> {
 
-
   /**
    * Sets the minimum size that will be written by a single RPC.
    * Written data will be buffered and only flushed upon reaching this size or closing the channel.
diff --git a/google-cloud-core/src/test/java/com/google/cloud/BaseWriteChannelTest.java b/google-cloud-core/src/test/java/com/google/cloud/BaseWriteChannelTest.java
@@ -16,25 +16,24 @@
 
 package com.google.cloud;
 
+import static com.google.common.truth.Truth.assertThat;
 import static junit.framework.TestCase.assertFalse;
 import static junit.framework.TestCase.assertTrue;
 import static org.junit.Assert.assertArrayEquals;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNull;
 
 import com.google.cloud.spi.ServiceRpcFactory;
-
-import org.junit.Before;
-import org.junit.Rule;
-import org.junit.Test;
-import org.junit.rules.ExpectedException;
-
 import java.io.IOException;
 import java.io.Serializable;
 import java.nio.ByteBuffer;
 import java.nio.channels.ClosedChannelException;
 import java.util.Arrays;
 import java.util.Random;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.ExpectedException;
 
 public class BaseWriteChannelTest {
 
@@ -111,11 +110,16 @@ public void testValidateOpen() throws IOException {
   @Test
   public void testChunkSize() {
     channel.setChunkSize(42);
-    assertEquals(MIN_CHUNK_SIZE, channel.getChunkSize());
+    assertThat(channel.getChunkSize() >= MIN_CHUNK_SIZE).isTrue();
+    assertThat(channel.getChunkSize() % MIN_CHUNK_SIZE).isEqualTo(0);
+
     channel.setChunkSize(2 * MIN_CHUNK_SIZE);
-    assertEquals(2 * MIN_CHUNK_SIZE, channel.getChunkSize());
-    channel.setChunkSize(512 * 1025);
-    assertEquals(2 * MIN_CHUNK_SIZE, channel.getChunkSize());
+    assertThat(channel.getChunkSize() >= MIN_CHUNK_SIZE).isTrue();
+    assertThat(channel.getChunkSize() % MIN_CHUNK_SIZE).isEqualTo(0);
+
+    channel.setChunkSize(2 * MIN_CHUNK_SIZE + 1);
+    assertThat(channel.getChunkSize() >= MIN_CHUNK_SIZE).isTrue();
+    assertThat(channel.getChunkSize() % MIN_CHUNK_SIZE).isEqualTo(0);
   }
 
   @Test
