From ec543ead33dd796ac16c81dd837354fa2818f64c Mon Sep 17 00:00:00 2001 From: Olav Loite Date: Thu, 21 Mar 2019 08:21:14 +0100 Subject: [PATCH 01/11] add CloudStorageFileChannel --- .../contrib/nio/CloudStorageFileChannel.java | 157 +++++++++++ .../nio/CloudStorageFileSystemProvider.java | 256 ++++++++---------- .../nio/CloudStorageFileChannelTest.java | 207 ++++++++++++++ ...IntegrationTestWithAbortedInjectorEnv.java | 35 +++ 4 files changed, 515 insertions(+), 140 deletions(-) create mode 100644 google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageFileChannel.java create mode 100644 google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/test/java/com/google/cloud/storage/contrib/nio/CloudStorageFileChannelTest.java create mode 100644 google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/IntegrationTestWithAbortedInjectorEnv.java diff --git a/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageFileChannel.java b/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageFileChannel.java new file mode 100644 index 000000000000..2f17affd6470 --- /dev/null +++ b/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageFileChannel.java @@ -0,0 +1,157 @@ +/* + * Copyright 2019 Google LLC + * + * 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 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.google.cloud.storage.contrib.nio; + +import java.io.IOException; +import java.nio.ByteBuffer; +import java.nio.MappedByteBuffer; +import java.nio.channels.FileChannel; +import java.nio.channels.FileLock; +import java.nio.channels.ReadableByteChannel; +import java.nio.channels.SeekableByteChannel; +import java.nio.channels.WritableByteChannel; +import com.google.common.base.Preconditions; + +class CloudStorageFileChannel extends FileChannel { + private long position = 0L; + private final SeekableByteChannel writeChannel; + + CloudStorageFileChannel(SeekableByteChannel writeChannel) { + this.writeChannel = writeChannel; + } + + @Override + public int read(ByteBuffer dst) throws IOException { + int res = writeChannel.read(dst); + position += res; + return res; + } + + @Override + public long read(ByteBuffer[] dsts, int offset, int length) throws IOException { + long res = 0L; + for (int i = offset; i < offset + length; i++) { + res += writeChannel.read(dsts[i]); + } + position += res; + return res; + } + + @Override + public int write(ByteBuffer src) throws IOException { + int res = writeChannel.write(src); + position += res; + return res; + } + + @Override + public long write(ByteBuffer[] srcs, int offset, int length) throws IOException { + long res = 0L; + for (int i = offset; i < offset + length; i++) { + res += writeChannel.write(srcs[i]); + } + position += res; + return res; + } + + @Override + public long position() throws IOException { + return position; + } + + @Override + public FileChannel position(long newPosition) throws IOException { + this.position = newPosition; + writeChannel.position(newPosition); + return this; + } + + @Override + public long size() throws IOException { + return writeChannel.size(); + } + + @Override + public FileChannel truncate(long size) throws IOException { + writeChannel.truncate(size); + return this; + } + + @Override + public void force(boolean metaData) throws IOException { + throw new UnsupportedOperationException(); + } + + @Override + public long transferTo(long position, long count, WritableByteChannel target) throws IOException { + Preconditions.checkArgument(count <= Integer.MAX_VALUE, + "Transfering more than Integer.MAX_VALUE bytes is not supported"); + ByteBuffer buffer = ByteBuffer.allocate((int) count); + long res = read(buffer, position); + buffer.position(0); + target.write(buffer); + return res; + } + + @Override + public long transferFrom(ReadableByteChannel src, long position, long count) throws IOException { + Preconditions.checkArgument(count <= Integer.MAX_VALUE, + "Transfering more than Integer.MAX_VALUE bytes is not supported"); + ByteBuffer buffer = ByteBuffer.allocate((int) count); + long res = src.read(buffer); + write(buffer); + return res; + } + + @Override + public int read(ByteBuffer dst, long position) throws IOException { + long originalPosition = position(); + position(position); + int res = writeChannel.read(dst); + position(originalPosition); + return res; + } + + @Override + public int write(ByteBuffer src, long position) throws IOException { + long originalPosition = position(); + position(position); + int res = writeChannel.write(src); + position(originalPosition); + return res; + } + + @Override + public MappedByteBuffer map(MapMode mode, long position, long size) throws IOException { + throw new UnsupportedOperationException(); + } + + @Override + public FileLock lock(long position, long size, boolean shared) throws IOException { + throw new UnsupportedOperationException(); + } + + @Override + public FileLock tryLock(long position, long size, boolean shared) throws IOException { + throw new UnsupportedOperationException(); + } + + @Override + protected void implCloseChannel() throws IOException { + writeChannel.close(); + } + +} diff --git a/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageFileSystemProvider.java b/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageFileSystemProvider.java index 4f22c9aeb89e..11f92ddffe4f 100644 --- a/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageFileSystemProvider.java +++ b/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageFileSystemProvider.java @@ -19,29 +19,11 @@ import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkNotNull; import static com.google.common.base.Strings.isNullOrEmpty; - -import com.google.api.gax.paging.Page; -import com.google.auto.service.AutoService; -import com.google.cloud.storage.Acl; -import com.google.cloud.storage.Blob; -import com.google.cloud.storage.BlobId; -import com.google.cloud.storage.BlobInfo; -import com.google.cloud.storage.Bucket; -import com.google.cloud.storage.CopyWriter; -import com.google.cloud.storage.Storage; -import com.google.cloud.storage.Storage.BlobGetOption; -import com.google.cloud.storage.Storage.BlobSourceOption; -import com.google.cloud.storage.StorageException; -import com.google.cloud.storage.StorageOptions; -import com.google.common.annotations.VisibleForTesting; -import com.google.common.base.MoreObjects; -import com.google.common.collect.AbstractIterator; -import com.google.common.net.UrlEscapers; -import com.google.common.primitives.Ints; import java.io.BufferedInputStream; import java.io.IOException; import java.io.InputStream; import java.net.URI; +import java.nio.channels.FileChannel; import java.nio.channels.SeekableByteChannel; import java.nio.file.AccessMode; import java.nio.file.AtomicMoveNotSupportedException; @@ -74,14 +56,33 @@ import javax.annotation.Nullable; import javax.annotation.concurrent.ThreadSafe; import javax.inject.Singleton; +import com.google.api.gax.paging.Page; +import com.google.auto.service.AutoService; +import com.google.cloud.storage.Acl; +import com.google.cloud.storage.Blob; +import com.google.cloud.storage.BlobId; +import com.google.cloud.storage.BlobInfo; +import com.google.cloud.storage.Bucket; +import com.google.cloud.storage.CopyWriter; +import com.google.cloud.storage.Storage; +import com.google.cloud.storage.Storage.BlobGetOption; +import com.google.cloud.storage.Storage.BlobSourceOption; +import com.google.cloud.storage.StorageException; +import com.google.cloud.storage.StorageOptions; +import com.google.common.annotations.VisibleForTesting; +import com.google.common.base.MoreObjects; +import com.google.common.collect.AbstractIterator; +import com.google.common.net.UrlEscapers; +import com.google.common.primitives.Ints; /** * Google Cloud Storage {@link FileSystemProvider} implementation. * - *

Note: This class should never be used directly. This class is instantiated by the - * service loader and called through a standardized API, e.g. {@link java.nio.file.Files}. However - * the javadocs in this class serve as useful documentation for the behavior of the Google Cloud - * Storage NIO library. + *

+ * Note: This class should never be used directly. This class is instantiated by the service + * loader and called through a standardized API, e.g. {@link java.nio.file.Files}. However the + * javadocs in this class serve as useful documentation for the behavior of the Google Cloud Storage + * NIO library. */ @Singleton @ThreadSafe @@ -104,12 +105,8 @@ private static class LazyPathIterator extends AbstractIterator { // whether to make the paths absolute before returning them. private final boolean absolutePaths; - LazyPathIterator( - CloudStorageFileSystem fileSystem, - String prefix, - Iterator blobIterator, - Filter filter, - boolean absolutePaths) { + LazyPathIterator(CloudStorageFileSystem fileSystem, String prefix, Iterator blobIterator, + Filter filter, boolean absolutePaths) { this.prefix = prefix; this.blobIterator = blobIterator; this.filter = filter; @@ -150,17 +147,20 @@ public static void setStorageOptions(@Nullable StorageOptions newStorageOptions) * Changes the default configuration for every filesystem object created from here on, including * via SPI. If null then future filesystem objects will have the factory default configuration. * - *

If options are specified later then they override the defaults. Methods that take a whole + *

+ * If options are specified later then they override the defaults. Methods that take a whole * CloudStorageConfiguration (eg. CloudStorageFileSystem.forBucket) will completely override the * defaults. Methods that take individual options (eg. * CloudStorageFileSystemProvier.newFileSystem) will override only these options; the rest will be * taken from the defaults specified here. * - *

This is meant to be done only once, at the beginning of some main program, in order to force + *

+ * This is meant to be done only once, at the beginning of some main program, in order to force * all libraries to use some settings we like. * - *

Libraries should never call this. If you're a library then, instead, create your own - * filesystem object with the right configuration and pass it along. + *

+ * Libraries should never call this. If you're a library then, instead, create your own filesystem + * object with the right configuration and pass it along. * * @param newDefault new default CloudStorageConfiguration */ @@ -176,8 +176,7 @@ public static void setDefaultCloudStorageConfiguration( * @see CloudStorageFileSystem#forBucket(String) */ public CloudStorageFileSystemProvider() { - this( - CloudStorageFileSystem.getDefaultCloudStorageConfiguration().userProject(), + this(CloudStorageFileSystem.getDefaultCloudStorageConfiguration().userProject(), futureStorageOptions); } @@ -192,8 +191,8 @@ public CloudStorageFileSystemProvider() { * Internal constructor, fully configurable. Note that null options means to use the system * defaults (NOT the user-provided ones). */ - CloudStorageFileSystemProvider( - @Nullable String userProject, @Nullable StorageOptions gcsStorageOptions) { + CloudStorageFileSystemProvider(@Nullable String userProject, + @Nullable StorageOptions gcsStorageOptions) { this.storageOptions = gcsStorageOptions; this.userProject = userProject; } @@ -228,45 +227,33 @@ public CloudStorageFileSystem getFileSystem(URI uri) { * include a path component (that will be ignored). * * @param uri bucket and current working directory, e.g. {@code gs://bucket} - * @param env map of configuration options, whose keys correspond to the method names of {@link - * CloudStorageConfiguration.Builder}. However you are not allowed to set the working - * directory, as that should be provided in the {@code uri} + * @param env map of configuration options, whose keys correspond to the method names of + * {@link CloudStorageConfiguration.Builder}. However you are not allowed to set the + * working directory, as that should be provided in the {@code uri} * @throws IllegalArgumentException if {@code uri} specifies a port, user, query, or fragment, or - * if scheme is not {@value CloudStorageFileSystem#URI_SCHEME} + * if scheme is not {@value CloudStorageFileSystem#URI_SCHEME} */ @Override public CloudStorageFileSystem newFileSystem(URI uri, Map env) { + checkArgument(uri.getScheme().equalsIgnoreCase(CloudStorageFileSystem.URI_SCHEME), + "Cloud Storage URIs must have '%s' scheme: %s", CloudStorageFileSystem.URI_SCHEME, uri); + checkArgument(!isNullOrEmpty(uri.getHost()), "%s:// URIs must have a host: %s", + CloudStorageFileSystem.URI_SCHEME, uri); checkArgument( - uri.getScheme().equalsIgnoreCase(CloudStorageFileSystem.URI_SCHEME), - "Cloud Storage URIs must have '%s' scheme: %s", - CloudStorageFileSystem.URI_SCHEME, - uri); - checkArgument( - !isNullOrEmpty(uri.getHost()), - "%s:// URIs must have a host: %s", - CloudStorageFileSystem.URI_SCHEME, - uri); - checkArgument( - uri.getPort() == -1 - && isNullOrEmpty(uri.getQuery()) - && isNullOrEmpty(uri.getFragment()) + uri.getPort() == -1 && isNullOrEmpty(uri.getQuery()) && isNullOrEmpty(uri.getFragment()) && isNullOrEmpty(uri.getUserInfo()), - "GCS FileSystem URIs mustn't have: port, userinfo, query, or fragment: %s", - uri); + "GCS FileSystem URIs mustn't have: port, userinfo, query, or fragment: %s", uri); CloudStorageUtil.checkBucket(uri.getHost()); initStorage(); - return new CloudStorageFileSystem( - this, - uri.getHost(), - CloudStorageConfiguration.fromMap( - CloudStorageFileSystem.getDefaultCloudStorageConfiguration(), env)); + return new CloudStorageFileSystem(this, uri.getHost(), CloudStorageConfiguration + .fromMap(CloudStorageFileSystem.getDefaultCloudStorageConfiguration(), env)); } @Override public CloudStoragePath getPath(URI uri) { initStorage(); - return CloudStoragePath.getPath( - getFileSystem(CloudStorageUtil.stripPathFromUri(uri)), uri.getPath()); + return CloudStoragePath.getPath(getFileSystem(CloudStorageUtil.stripPathFromUri(uri)), + uri.getPath()); } /** Convenience method: replaces spaces with "%20", builds a URI, and calls getPath(uri). */ @@ -281,14 +268,14 @@ public CloudStoragePath getPath(String uriInStringForm) { * * @param path: the path to the file to open or create * @param options: options specifying how the file is opened, e.g. StandardOpenOption.WRITE or - * BlobSourceOption.userProject + * BlobSourceOption.userProject * @param attrs: (not supported, values will be ignored) * @return * @throws IOException */ @Override - public SeekableByteChannel newByteChannel( - Path path, Set options, FileAttribute... attrs) throws IOException { + public SeekableByteChannel newByteChannel(Path path, Set options, + FileAttribute... attrs) throws IOException { checkNotNull(path); initStorage(); CloudStorageUtil.checkNotNullArray(attrs); @@ -300,6 +287,17 @@ public SeekableByteChannel newByteChannel( } } + @Override + public FileChannel newFileChannel(Path path, Set options, + FileAttribute... attrs) throws IOException { + checkNotNull(path); + initStorage(); + CloudStorageUtil.checkNotNullArray(attrs); + CloudStorageWriteChannel writeChannel = + (CloudStorageWriteChannel) newWriteChannel(path, options); + return new CloudStorageFileChannel(writeChannel); + } + private SeekableByteChannel newReadChannel(Path path, Set options) throws IOException { initStorage(); @@ -339,13 +337,8 @@ private SeekableByteChannel newReadChannel(Path path, Set if (cloudPath.seemsLikeADirectoryAndUsePseudoDirectories(null)) { throw new CloudStoragePseudoDirectoryException(cloudPath); } - return CloudStorageReadChannel.create( - storage, - cloudPath.getBlobId(), - 0, - maxChannelReopens, - cloudPath.getFileSystem().config(), - userProject, + return CloudStorageReadChannel.create(storage, cloudPath.getBlobId(), 0, maxChannelReopens, + cloudPath.getFileSystem().config(), userProject, blobSourceOptions.toArray(new BlobSourceOption[blobSourceOptions.size()])); } @@ -418,10 +411,8 @@ private SeekableByteChannel newWriteChannel(Path path, Set } try { - return new CloudStorageWriteChannel( - storage.writer( - infoBuilder.build(), - writeOptions.toArray(new Storage.BlobWriteOption[writeOptions.size()]))); + return new CloudStorageWriteChannel(storage.writer(infoBuilder.build(), + writeOptions.toArray(new Storage.BlobWriteOption[writeOptions.size()]))); } catch (StorageException oops) { throw asIoException(oops); } @@ -466,8 +457,8 @@ public boolean deleteIfExists(Path path) throws IOException { if (isNullOrEmpty(userProject)) { return storage.delete(cloudPath.getBlobId()); } else { - return storage.delete( - cloudPath.getBlobId(), Storage.BlobSourceOption.userProject(userProject)); + return storage.delete(cloudPath.getBlobId(), + Storage.BlobSourceOption.userProject(userProject)); } } catch (StorageException exs) { // Will rethrow a StorageException if all retries/reopens are exhausted @@ -491,9 +482,7 @@ public void move(Path source, Path target, CopyOption... options) throws IOExcep initStorage(); for (CopyOption option : options) { if (option == StandardCopyOption.ATOMIC_MOVE) { - throw new AtomicMoveNotSupportedException( - source.toString(), - target.toString(), + throw new AtomicMoveNotSupportedException(source.toString(), target.toString(), "Google Cloud Storage does not support atomic move operations."); } } @@ -542,8 +531,8 @@ public void copy(Path source, Path target, CopyOption... options) throws IOExcep tgtInfoBuilder.setContentEncoding(((OptionContentEncoding) option).contentEncoding()); overrideContentEncoding = true; } else if (option instanceof OptionContentDisposition) { - tgtInfoBuilder.setContentDisposition( - ((OptionContentDisposition) option).contentDisposition()); + tgtInfoBuilder + .setContentDisposition(((OptionContentDisposition) option).contentDisposition()); overrideContentDisposition = true; } else { throw new UnsupportedOperationException(option.toString()); @@ -555,12 +544,9 @@ public void copy(Path source, Path target, CopyOption... options) throws IOExcep CloudStoragePath fromPath = CloudStorageUtil.checkPath(source); - blockSize = - blockSize != -1 - ? blockSize - : Ints.max( - fromPath.getFileSystem().config().blockSize(), - toPath.getFileSystem().config().blockSize()); + blockSize = blockSize != -1 ? blockSize + : Ints.max(fromPath.getFileSystem().config().blockSize(), + toPath.getFileSystem().config().blockSize()); // TODO: actually use blockSize if (fromPath.seemsLikeADirectory() && toPath.seemsLikeADirectory()) { @@ -626,13 +612,11 @@ public void copy(Path source, Path target, CopyOption... options) throws IOExcep copyReqBuilder.setTarget(tgtInfo, Storage.BlobTargetOption.doesNotExist()); } if (!isNullOrEmpty(fromPath.getFileSystem().config().userProject())) { - copyReqBuilder = - copyReqBuilder.setSourceOptions( - BlobSourceOption.userProject(fromPath.getFileSystem().config().userProject())); + copyReqBuilder = copyReqBuilder.setSourceOptions( + BlobSourceOption.userProject(fromPath.getFileSystem().config().userProject())); } else if (!isNullOrEmpty(toPath.getFileSystem().config().userProject())) { - copyReqBuilder = - copyReqBuilder.setSourceOptions( - BlobSourceOption.userProject(toPath.getFileSystem().config().userProject())); + copyReqBuilder = copyReqBuilder.setSourceOptions( + BlobSourceOption.userProject(toPath.getFileSystem().config().userProject())); } CopyWriter copyWriter = storage.copy(copyReqBuilder.build()); copyWriter.getResult(); @@ -683,16 +667,12 @@ public void checkAccess(Path path, AccessMode... modes) throws IOException { try { boolean nullId; if (isNullOrEmpty(userProject)) { - nullId = - storage.get(cloudPath.getBlobId(), Storage.BlobGetOption.fields(Storage.BlobField.ID)) - == null; + nullId = storage.get(cloudPath.getBlobId(), + Storage.BlobGetOption.fields(Storage.BlobField.ID)) == null; } else { nullId = - storage.get( - cloudPath.getBlobId(), - Storage.BlobGetOption.fields(Storage.BlobField.ID), - Storage.BlobGetOption.userProject(userProject)) - == null; + storage.get(cloudPath.getBlobId(), Storage.BlobGetOption.fields(Storage.BlobField.ID), + Storage.BlobGetOption.userProject(userProject)) == null; } if (nullId) { if (cloudPath.seemsLikeADirectoryAndUsePseudoDirectories(storage)) { @@ -720,8 +700,8 @@ public void checkAccess(Path path, AccessMode... modes) throws IOException { } @Override - public A readAttributes( - Path path, Class type, LinkOption... options) throws IOException { + public A readAttributes(Path path, Class type, + LinkOption... options) throws IOException { checkNotNull(type); CloudStorageUtil.checkNotNullArray(options); if (type != CloudStorageFileAttributes.class && type != BasicFileAttributes.class) { @@ -788,14 +768,14 @@ public A readAttributes( @Override public Map readAttributes(Path path, String attributes, LinkOption... options) { // TODO(#811): Java 7 NIO defines at least eleven string attributes we'd want to support - // (eg. BasicFileAttributeView and PosixFileAttributeView), so rather than a partial - // implementation we rely on the other overload for now. + // (eg. BasicFileAttributeView and PosixFileAttributeView), so rather than a partial + // implementation we rely on the other overload for now. throw new UnsupportedOperationException(); } @Override - public V getFileAttributeView( - Path path, Class type, LinkOption... options) { + public V getFileAttributeView(Path path, Class type, + LinkOption... options) { checkNotNull(type); CloudStorageUtil.checkNotNullArray(options); if (type != CloudStorageFileAttributeView.class && type != BasicFileAttributeView.class) { @@ -815,8 +795,8 @@ public void createDirectory(Path dir, FileAttribute... attrs) { } @Override - public DirectoryStream newDirectoryStream( - final Path dir, final Filter filter) { + public DirectoryStream newDirectoryStream(final Path dir, + final Filter filter) { final CloudStoragePath cloudPath = CloudStorageUtil.checkPath(dir); checkNotNull(filter); initStorage(); @@ -835,27 +815,19 @@ public DirectoryStream newDirectoryStream( final String prefix = prePrefix; Page dirList; if (isNullOrEmpty(userProject)) { - dirList = - storage.list( - cloudPath.bucket(), - Storage.BlobListOption.prefix(prefix), - Storage.BlobListOption.currentDirectory(), - Storage.BlobListOption.fields()); + dirList = storage.list(cloudPath.bucket(), Storage.BlobListOption.prefix(prefix), + Storage.BlobListOption.currentDirectory(), Storage.BlobListOption.fields()); } else { - dirList = - storage.list( - cloudPath.bucket(), - Storage.BlobListOption.prefix(prefix), - Storage.BlobListOption.currentDirectory(), - Storage.BlobListOption.fields(), - Storage.BlobListOption.userProject(userProject)); + dirList = storage.list(cloudPath.bucket(), Storage.BlobListOption.prefix(prefix), + Storage.BlobListOption.currentDirectory(), Storage.BlobListOption.fields(), + Storage.BlobListOption.userProject(userProject)); } final Iterator blobIterator = dirList.iterateAll().iterator(); return new DirectoryStream() { @Override public Iterator iterator() { - return new LazyPathIterator( - cloudPath.getFileSystem(), prefix, blobIterator, filter, dir.isAbsolute()); + return new LazyPathIterator(cloudPath.getFileSystem(), prefix, blobIterator, filter, + dir.isAbsolute()); } @Override @@ -887,9 +859,8 @@ public FileStore getFileStore(Path path) { @Override public boolean equals(Object other) { - return this == other - || other instanceof CloudStorageFileSystemProvider - && Objects.equals(storage, ((CloudStorageFileSystemProvider) other).storage); + return this == other || other instanceof CloudStorageFileSystemProvider + && Objects.equals(storage, ((CloudStorageFileSystemProvider) other).storage); } @Override @@ -925,7 +896,8 @@ public boolean requesterPays(String bucketName) { * Returns a NEW CloudStorageFileSystemProvider identical to this one, but with userProject * removed. * - *

Perhaps you want to call this is you realize you'll be working on a bucket that is not + *

+ * Perhaps you want to call this is you realize you'll be working on a bucket that is not * requester-pays. */ public CloudStorageFileSystemProvider withNoUserProject() { @@ -941,17 +913,21 @@ public String getProject() { /** * Lists the project's buckets. But use the one in CloudStorageFileSystem. * - *

Example of listing buckets, specifying the page size and a name prefix. + *

+ * Example of listing buckets, specifying the page size and a name prefix. * - * 

{@code
-   * String prefix = "bucket_";
-   * Page buckets = provider.listBuckets(BucketListOption.prefix(prefix));
-   * Iterator bucketIterator = buckets.iterateAll();
-   * while (bucketIterator.hasNext()) {
-   *   Bucket bucket = bucketIterator.next();
-   *   // do something with the bucket
+   * 
+   * {
+   *   @code
+   *   String prefix = "bucket_";
+   *   Page buckets = provider.listBuckets(BucketListOption.prefix(prefix));
+   *   Iterator bucketIterator = buckets.iterateAll();
+   *   while (bucketIterator.hasNext()) {
+   *     Bucket bucket = bucketIterator.next();
+   *     // do something with the bucket
+   *   }
    * }
-   * }

+   *
* * @throws StorageException upon failure */ diff --git a/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/test/java/com/google/cloud/storage/contrib/nio/CloudStorageFileChannelTest.java b/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/test/java/com/google/cloud/storage/contrib/nio/CloudStorageFileChannelTest.java new file mode 100644 index 000000000000..c73e688711bd --- /dev/null +++ b/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/test/java/com/google/cloud/storage/contrib/nio/CloudStorageFileChannelTest.java @@ -0,0 +1,207 @@ +/* + * Copyright 2019 Google LLC + * + * 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 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.google.cloud.storage.contrib.nio; + +import static org.hamcrest.CoreMatchers.equalTo; +import static org.hamcrest.CoreMatchers.is; +import static org.junit.Assert.assertThat; +import java.io.IOException; +import java.nio.ByteBuffer; +import java.nio.channels.FileChannel; +import java.nio.channels.SeekableByteChannel; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.ExpectedException; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +@RunWith(JUnit4.class) +public class CloudStorageFileChannelTest { + private static final class SeekableByteChannelImpl implements SeekableByteChannel { + private boolean open = true; + private ByteBuffer data; + + private SeekableByteChannelImpl(ByteBuffer data) { + this.data = data; + } + + @Override + public boolean isOpen() { + return open; + } + + @Override + public void close() throws IOException { + open = false; + } + + @Override + public int read(ByteBuffer dst) throws IOException { + byte[] tmp = new byte[dst.remaining()]; + data.get(tmp); + dst.put(tmp); + return tmp.length; + } + + @Override + public int write(ByteBuffer src) throws IOException { + int res = src.remaining(); + if (data.position() + res > data.capacity()) { + ByteBuffer newData = ByteBuffer.allocate(data.capacity() + res); + int currentPos = data.position(); + data.position(0); + newData.put(data); + data = newData; + data.position(currentPos); + } + data.put(src); + return res; + } + + @Override + public long position() throws IOException { + return data.position(); + } + + @Override + public SeekableByteChannel position(long newPosition) throws IOException { + if (newPosition >= data.capacity()) { + ByteBuffer newData = ByteBuffer.allocate((int) newPosition); + newData.put(data); + data = newData; + } + data.position((int) newPosition); + return this; + } + + @Override + public long size() throws IOException { + return data.capacity(); + } + + @Override + public SeekableByteChannel truncate(long size) throws IOException { + if (size < data.capacity()) { + int position = data.position(); + ByteBuffer newData = ByteBuffer.allocate((int) size); + newData.put(data.array(), 0, (int) size); + data = newData; + data.position(position > (int) size ? (int) size : position); + } + return this; + } + } + + @Rule + public final ExpectedException thrown = ExpectedException.none(); + + private CloudStorageFileChannel fileChannel; + private SeekableByteChannel writeChannel; + + @Before + public void before() throws IOException { + ByteBuffer data = ByteBuffer.wrap(new byte[] {1, 2, 3}); + writeChannel = new SeekableByteChannelImpl(data); + fileChannel = new CloudStorageFileChannel(writeChannel); + } + + @Test + public void testRead() throws IOException { + ByteBuffer buffer = ByteBuffer.allocate(1); + assertThat(fileChannel.position(), is(equalTo(0L))); + assertThat(fileChannel.read(buffer), is(equalTo(1))); + assertThat(fileChannel.position(), is(equalTo(1L))); + assertThat(buffer.get(0), is(equalTo((byte) 1))); + } + + @Test + public void testReadArray() throws IOException { + ByteBuffer[] buffers = + new ByteBuffer[] {ByteBuffer.allocate(1), ByteBuffer.allocate(1), ByteBuffer.allocate(1)}; + assertThat(fileChannel.position(), is(equalTo(0L))); + assertThat(fileChannel.read(buffers), is(equalTo(3L))); + assertThat(fileChannel.position(), is(equalTo(3L))); + assertThat(buffers[0].get(0), is(equalTo((byte) 1))); + assertThat(buffers[1].get(0), is(equalTo((byte) 2))); + assertThat(buffers[2].get(0), is(equalTo((byte) 3))); + } + + @Test + public void testWrite() throws IOException { + ByteBuffer buffer = ByteBuffer.allocate(1); + buffer.put((byte) 100).position(0); + assertThat(fileChannel.position(), is(equalTo(0L))); + assertThat(fileChannel.write(buffer), is(equalTo(1))); + assertThat(fileChannel.position(), is(equalTo(1L))); + + ByteBuffer read = ByteBuffer.allocate(1); + fileChannel.position(0); + fileChannel.read(read); + assertThat(read.get(0), is(equalTo((byte) 100))); + } + + @Test + public void testWriteArray() throws IOException { + ByteBuffer[] buffers = + new ByteBuffer[] {ByteBuffer.allocate(1), ByteBuffer.allocate(1), ByteBuffer.allocate(1)}; + buffers[0].put((byte) 10).position(0); + buffers[1].put((byte) 20).position(0); + buffers[2].put((byte) 30).position(0); + assertThat(fileChannel.position(), is(equalTo(0L))); + assertThat(fileChannel.write(buffers), is(equalTo(3L))); + assertThat(fileChannel.position(), is(equalTo(3L))); + + ByteBuffer[] read = + new ByteBuffer[] {ByteBuffer.allocate(1), ByteBuffer.allocate(1), ByteBuffer.allocate(1)}; + fileChannel.position(0); + fileChannel.read(read); + assertThat(read[0].get(0), is(equalTo((byte) 10))); + assertThat(read[1].get(0), is(equalTo((byte) 20))); + assertThat(read[2].get(0), is(equalTo((byte) 30))); + } + + @Test + public void testPosition() throws IOException { + assertThat(fileChannel.position(), is(equalTo(0L))); + assertThat(fileChannel.position(1L), is(equalTo((FileChannel) fileChannel))); + assertThat(fileChannel.position(), is(equalTo(1L))); + assertThat(fileChannel.position(0L), is(equalTo((FileChannel) fileChannel))); + assertThat(fileChannel.position(), is(equalTo(0L))); + assertThat(fileChannel.position(100L), is(equalTo((FileChannel) fileChannel))); + assertThat(fileChannel.position(), is(equalTo(100L))); + } + + @Test + public void testSizeAndTruncate() throws IOException { + assertThat(fileChannel.size(), is(equalTo(3L))); + fileChannel.truncate(1L); + assertThat(fileChannel.size(), is(equalTo(1L))); + fileChannel.truncate(10L); + assertThat(fileChannel.size(), is(equalTo(1L))); + assertThat(fileChannel.position(), is(equalTo(0L))); + } + + @Test + public void testTransferTo() throws IOException { + SeekableByteChannelImpl target = new SeekableByteChannelImpl(ByteBuffer.allocate(100)); + assertThat(fileChannel.transferTo(0L, 3L, target), is(equalTo(3L))); + assertThat(target.position(), is(equalTo(3L))); + ByteBuffer dst = ByteBuffer.allocate(3); + target.read(dst); + } + +} diff --git a/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/IntegrationTestWithAbortedInjectorEnv.java b/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/IntegrationTestWithAbortedInjectorEnv.java new file mode 100644 index 000000000000..78e171fcb44a --- /dev/null +++ b/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/IntegrationTestWithAbortedInjectorEnv.java @@ -0,0 +1,35 @@ +package com.google.cloud.spanner; + +import com.google.cloud.spanner.SpannerOptions.SpannerTestOptions; + +public class IntegrationTestWithAbortedInjectorEnv extends IntegrationTestEnv { + private final AbortedTransactionInjectorImpl injector = new AbortedTransactionInjectorImpl(); + + public AbortedTransactionInjectorImpl getAbortedTransactionInjector() { + return injector; + } + + @Override + protected SpannerTestOptions createSpannerTestOptions(SpannerTestOptions testOptions) { + return testOptions.toBuilder().setAbortedTransactionInjector(injector).build(); + } + + /** Injects simulated aborted transaction. */ + public static final class AbortedTransactionInjectorImpl implements AbortedTransactionInjector { + private boolean injectAbortOnce = false; + + @Override + public boolean shouldAbort() { + if (injectAbortOnce) { + injectAbortOnce = false; + return true; + } + return false; + } + + @Override + public void injectAbortOnce() { + this.injectAbortOnce = true; + } + } +} From c46e001b7fe5f04fd9385fd33c0667663f8a44ab Mon Sep 17 00:00:00 2001 From: Olav Loite Date: Fri, 22 Mar 2019 07:33:50 +0100 Subject: [PATCH 02/11] #4702 add support for newFileChannel(...) --- .../nio/CloudStorageFileSystemProvider.java | 37 ++- ....java => CloudStorageReadFileChannel.java} | 84 +++---- .../nio/CloudStorageWriteFileChannel.java | 183 ++++++++++++++ ...a => CloudStorageReadFileChannelTest.java} | 121 ++++----- .../nio/CloudStorageWriteFileChannelTest.java | 203 +++++++++++++++ .../storage/contrib/nio/it/ITGcsNio.java | 237 ++++++++++++++++++ 6 files changed, 745 insertions(+), 120 deletions(-) rename google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/{CloudStorageFileChannel.java => CloudStorageReadFileChannel.java} (67%) create mode 100644 google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageWriteFileChannel.java rename google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/test/java/com/google/cloud/storage/contrib/nio/{CloudStorageFileChannelTest.java => CloudStorageReadFileChannelTest.java} (61%) create mode 100644 google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/test/java/com/google/cloud/storage/contrib/nio/CloudStorageWriteFileChannelTest.java diff --git a/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageFileSystemProvider.java b/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageFileSystemProvider.java index 11f92ddffe4f..78f54ef78a7e 100644 --- a/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageFileSystemProvider.java +++ b/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageFileSystemProvider.java @@ -287,15 +287,46 @@ public SeekableByteChannel newByteChannel(Path path, Set o } } + /** + * Open a file for reading OR writing. The {@link FileChannel} that is returned will only allow + * reads or writes depending on the {@link OpenOption}s that are specified. If any of the + * following have been specified, the {@link FileChannel} will be write-only: + * {@link StandardOpenOption#CREATE} + *
    + *
  • {@link StandardOpenOption#CREATE}
    • + *
  • {@link StandardOpenOption#CREATE_NEW}
    • + *
  • {@link StandardOpenOption#WRITE}
    • + *
  • {@link StandardOpenOption#TRUNCATE_EXISTING}
    • + *
+ * In all other cases the {@link FileChannel} will be read-only. + * + * @param path The path to the file to open or create + * @param options The options specifying how the file should be opened, and whether the + * {@link FileChannel} should be read-only or write-only. + * @param attrs (not supported, the values will be ignored) + * @throws IOException + */ @Override public FileChannel newFileChannel(Path path, Set options, FileAttribute... attrs) throws IOException { checkNotNull(path); initStorage(); CloudStorageUtil.checkNotNullArray(attrs); - CloudStorageWriteChannel writeChannel = - (CloudStorageWriteChannel) newWriteChannel(path, options); - return new CloudStorageFileChannel(writeChannel); + if (options.contains(StandardOpenOption.CREATE_NEW)) { + Files.createFile(path, attrs); + } else if (options.contains(StandardOpenOption.CREATE) && !Files.exists(path)) { + Files.createFile(path, attrs); + } + if (options.contains(StandardOpenOption.WRITE) || options.contains(StandardOpenOption.CREATE) + || options.contains(StandardOpenOption.CREATE_NEW) + || options.contains(StandardOpenOption.TRUNCATE_EXISTING)) { + CloudStorageWriteChannel writeChannel = + (CloudStorageWriteChannel) newWriteChannel(path, options); + return new CloudStorageWriteFileChannel(writeChannel); + } else { + CloudStorageReadChannel readChannel = (CloudStorageReadChannel) newReadChannel(path, options); + return new CloudStorageReadFileChannel(readChannel); + } } private SeekableByteChannel newReadChannel(Path path, Set options) diff --git a/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageFileChannel.java b/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageReadFileChannel.java similarity index 67% rename from google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageFileChannel.java rename to google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageReadFileChannel.java index 2f17affd6470..45bc43fc498d 100644 --- a/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageFileChannel.java +++ b/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageReadFileChannel.java @@ -1,5 +1,5 @@ /* - * Copyright 2019 Google LLC + * Copyright 2016 Google LLC * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -15,6 +15,7 @@ */ package com.google.cloud.storage.contrib.nio; +import com.google.common.base.Preconditions; import java.io.IOException; import java.nio.ByteBuffer; import java.nio.MappedByteBuffer; @@ -23,71 +24,59 @@ import java.nio.channels.ReadableByteChannel; import java.nio.channels.SeekableByteChannel; import java.nio.channels.WritableByteChannel; -import com.google.common.base.Preconditions; -class CloudStorageFileChannel extends FileChannel { - private long position = 0L; - private final SeekableByteChannel writeChannel; +class CloudStorageReadFileChannel extends FileChannel { + private static final String READ_ONLY = "This FileChannel is read-only"; + private final SeekableByteChannel readChannel; - CloudStorageFileChannel(SeekableByteChannel writeChannel) { - this.writeChannel = writeChannel; + CloudStorageReadFileChannel(SeekableByteChannel readChannel) { + Preconditions.checkNotNull(readChannel); + this.readChannel = readChannel; } @Override public int read(ByteBuffer dst) throws IOException { - int res = writeChannel.read(dst); - position += res; - return res; + return readChannel.read(dst); } @Override public long read(ByteBuffer[] dsts, int offset, int length) throws IOException { long res = 0L; for (int i = offset; i < offset + length; i++) { - res += writeChannel.read(dsts[i]); + res += readChannel.read(dsts[i]); } - position += res; return res; } @Override public int write(ByteBuffer src) throws IOException { - int res = writeChannel.write(src); - position += res; - return res; + throw new UnsupportedOperationException(READ_ONLY); } @Override public long write(ByteBuffer[] srcs, int offset, int length) throws IOException { - long res = 0L; - for (int i = offset; i < offset + length; i++) { - res += writeChannel.write(srcs[i]); - } - position += res; - return res; + throw new UnsupportedOperationException(READ_ONLY); } @Override public long position() throws IOException { - return position; + return readChannel.position(); } @Override public FileChannel position(long newPosition) throws IOException { - this.position = newPosition; - writeChannel.position(newPosition); + readChannel.position(newPosition); return this; } @Override public long size() throws IOException { - return writeChannel.size(); + return readChannel.size(); } @Override public FileChannel truncate(long size) throws IOException { - writeChannel.truncate(size); - return this; + throw new UnsupportedOperationException(READ_ONLY); } @Override @@ -97,41 +86,43 @@ public void force(boolean metaData) throws IOException { @Override public long transferTo(long position, long count, WritableByteChannel target) throws IOException { - Preconditions.checkArgument(count <= Integer.MAX_VALUE, - "Transfering more than Integer.MAX_VALUE bytes is not supported"); - ByteBuffer buffer = ByteBuffer.allocate((int) count); - long res = read(buffer, position); - buffer.position(0); - target.write(buffer); + long originalPosition = position(); + position(position); + int blockSize = (int) Math.min(count, 0xfffffL); + long res = 0L; + int bytesRead = 0; + ByteBuffer buffer = ByteBuffer.allocate(blockSize); + while (res < count && bytesRead >= 0) { + buffer.position(0); + bytesRead = read(buffer); + if (bytesRead > 0) { + buffer.position(0); + buffer.limit(bytesRead); + target.write(buffer); + res += bytesRead; + } + } + position(originalPosition); return res; } @Override public long transferFrom(ReadableByteChannel src, long position, long count) throws IOException { - Preconditions.checkArgument(count <= Integer.MAX_VALUE, - "Transfering more than Integer.MAX_VALUE bytes is not supported"); - ByteBuffer buffer = ByteBuffer.allocate((int) count); - long res = src.read(buffer); - write(buffer); - return res; + throw new UnsupportedOperationException(READ_ONLY); } @Override public int read(ByteBuffer dst, long position) throws IOException { long originalPosition = position(); position(position); - int res = writeChannel.read(dst); + int res = readChannel.read(dst); position(originalPosition); return res; } @Override public int write(ByteBuffer src, long position) throws IOException { - long originalPosition = position(); - position(position); - int res = writeChannel.write(src); - position(originalPosition); - return res; + throw new UnsupportedOperationException(READ_ONLY); } @Override @@ -151,7 +142,6 @@ public FileLock tryLock(long position, long size, boolean shared) throws IOExcep @Override protected void implCloseChannel() throws IOException { - writeChannel.close(); + readChannel.close(); } - } diff --git a/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageWriteFileChannel.java b/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageWriteFileChannel.java new file mode 100644 index 000000000000..6a3406e380a7 --- /dev/null +++ b/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageWriteFileChannel.java @@ -0,0 +1,183 @@ +/* + * Copyright 2016 Google LLC + * + * 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 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.google.cloud.storage.contrib.nio; + +import java.io.IOException; +import java.nio.ByteBuffer; +import java.nio.MappedByteBuffer; +import java.nio.channels.FileChannel; +import java.nio.channels.FileLock; +import java.nio.channels.ReadableByteChannel; +import java.nio.channels.SeekableByteChannel; +import java.nio.channels.WritableByteChannel; + +class CloudStorageWriteFileChannel extends FileChannel { + private static final String WRITE_ONLY = "This FileChannel is write-only"; + private SeekableByteChannel writeChannel; + private boolean valid = true; + + CloudStorageWriteFileChannel(SeekableByteChannel writeChannel) { + this.writeChannel = writeChannel; + } + + private void checkValid() throws IOException { + if (!valid) { + // These methods are only supported to be called once, because the underlying channel does not + // support changing the position. + throw new IOException( + "This FileChannel is no longer valid. " + + "A Cloud Storage FileChannel is invalidated after calling one of " + + "the methods FileChannel#write(ByteBuffer, long) or " + + "FileChannel#transferFrom(ReadableByteChannel, long, long)"); + } + if (!writeChannel.isOpen()) { + throw new IOException("This FileChannel is closed"); + } + } + + @Override + public int read(ByteBuffer dst) throws IOException { + throw new UnsupportedOperationException(WRITE_ONLY); + } + + @Override + public long read(ByteBuffer[] dsts, int offset, int length) throws IOException { + throw new UnsupportedOperationException(WRITE_ONLY); + } + + @Override + public int write(ByteBuffer src) throws IOException { + checkValid(); + int res = writeChannel.write(src); + return res; + } + + @Override + public long write(ByteBuffer[] srcs, int offset, int length) throws IOException { + checkValid(); + long res = 0L; + for (int i = offset; i < offset + length; i++) { + res += writeChannel.write(srcs[i]); + } + return res; + } + + @Override + public long position() throws IOException { + checkValid(); + return writeChannel.position(); + } + + @Override + public FileChannel position(long newPosition) throws IOException { + checkValid(); + if (newPosition != position()) { + writeChannel.position(newPosition); + } + return this; + } + + @Override + public long size() throws IOException { + checkValid(); + return writeChannel.size(); + } + + @Override + public FileChannel truncate(long size) throws IOException { + checkValid(); + writeChannel.truncate(size); + return this; + } + + @Override + public void force(boolean metaData) throws IOException { + throw new UnsupportedOperationException(); + } + + @Override + public long transferTo(long position, long count, WritableByteChannel target) throws IOException { + throw new UnsupportedOperationException(WRITE_ONLY); + } + + @Override + public long transferFrom(ReadableByteChannel src, long position, long count) throws IOException { + checkValid(); + if (position != position()) { + throw new UnsupportedOperationException( + "This FileChannel only supports transferFrom at the current position"); + } + int blockSize = (int) Math.min(count, 0xfffffL); + long res = 0L; + int bytesRead = 0; + ByteBuffer buffer = ByteBuffer.allocate(blockSize); + while (res < count && bytesRead >= 0) { + buffer.position(0); + bytesRead = src.read(buffer); + if (bytesRead > 0) { + buffer.position(0); + buffer.limit(bytesRead); + write(buffer); + res += bytesRead; + } + } + // The channel is no longer valid as the position has been updated, and there is no way of + // resetting it, but this way we at least support the write-at-position and transferFrom methods + // being called once. + this.valid = false; + return res; + } + + @Override + public int read(ByteBuffer dst, long position) throws IOException { + throw new UnsupportedOperationException(WRITE_ONLY); + } + + @Override + public int write(ByteBuffer src, long position) throws IOException { + checkValid(); + if (position != position()) { + throw new UnsupportedOperationException( + "This FileChannel only supports write at the current position"); + } + int res = writeChannel.write(src); + // The channel is no longer valid as the position has been updated, and there is no way of + // resetting it, but this way we at least support the write-at-position and transferFrom methods + // being called once. + this.valid = false; + return res; + } + + @Override + public MappedByteBuffer map(MapMode mode, long position, long size) throws IOException { + throw new UnsupportedOperationException(); + } + + @Override + public FileLock lock(long position, long size, boolean shared) throws IOException { + throw new UnsupportedOperationException(); + } + + @Override + public FileLock tryLock(long position, long size, boolean shared) throws IOException { + throw new UnsupportedOperationException(); + } + + @Override + protected void implCloseChannel() throws IOException { + writeChannel.close(); + } +} diff --git a/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/test/java/com/google/cloud/storage/contrib/nio/CloudStorageFileChannelTest.java b/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/test/java/com/google/cloud/storage/contrib/nio/CloudStorageReadFileChannelTest.java similarity index 61% rename from google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/test/java/com/google/cloud/storage/contrib/nio/CloudStorageFileChannelTest.java rename to google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/test/java/com/google/cloud/storage/contrib/nio/CloudStorageReadFileChannelTest.java index c73e688711bd..c3474a3a1c06 100644 --- a/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/test/java/com/google/cloud/storage/contrib/nio/CloudStorageFileChannelTest.java +++ b/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/test/java/com/google/cloud/storage/contrib/nio/CloudStorageReadFileChannelTest.java @@ -18,6 +18,7 @@ import static org.hamcrest.CoreMatchers.equalTo; import static org.hamcrest.CoreMatchers.is; import static org.junit.Assert.assertThat; + import java.io.IOException; import java.nio.ByteBuffer; import java.nio.channels.FileChannel; @@ -30,7 +31,7 @@ import org.junit.runners.JUnit4; @RunWith(JUnit4.class) -public class CloudStorageFileChannelTest { +public class CloudStorageReadFileChannelTest { private static final class SeekableByteChannelImpl implements SeekableByteChannel { private boolean open = true; private ByteBuffer data; @@ -51,22 +52,21 @@ public void close() throws IOException { @Override public int read(ByteBuffer dst) throws IOException { - byte[] tmp = new byte[dst.remaining()]; - data.get(tmp); - dst.put(tmp); - return tmp.length; + byte[] tmp = new byte[Math.min(dst.remaining(), data.remaining())]; + if (tmp.length == 0) { + return -1; + } else { + data.get(tmp); + dst.put(tmp); + return tmp.length; + } } @Override public int write(ByteBuffer src) throws IOException { int res = src.remaining(); - if (data.position() + res > data.capacity()) { - ByteBuffer newData = ByteBuffer.allocate(data.capacity() + res); - int currentPos = data.position(); - data.position(0); - newData.put(data); - data = newData; - data.position(currentPos); + if (data.position() + res > data.limit()) { + data.limit(data.limit() + res); } data.put(src); return res; @@ -79,10 +79,8 @@ public long position() throws IOException { @Override public SeekableByteChannel position(long newPosition) throws IOException { - if (newPosition >= data.capacity()) { - ByteBuffer newData = ByteBuffer.allocate((int) newPosition); - newData.put(data); - data = newData; + if (newPosition >= data.limit()) { + data.limit((int) newPosition); } data.position((int) newPosition); return this; @@ -90,33 +88,35 @@ public SeekableByteChannel position(long newPosition) throws IOException { @Override public long size() throws IOException { - return data.capacity(); + return data.limit(); } @Override public SeekableByteChannel truncate(long size) throws IOException { - if (size < data.capacity()) { - int position = data.position(); - ByteBuffer newData = ByteBuffer.allocate((int) size); - newData.put(data.array(), 0, (int) size); - data = newData; - data.position(position > (int) size ? (int) size : position); + if (size < data.limit()) { + if (data.position() >= size) { + data.position((int) size - 1); + } + data.limit((int) size); } return this; } } - @Rule - public final ExpectedException thrown = ExpectedException.none(); + @Rule public final ExpectedException thrown = ExpectedException.none(); - private CloudStorageFileChannel fileChannel; - private SeekableByteChannel writeChannel; + private CloudStorageReadFileChannel fileChannel; + private SeekableByteChannel readChannel; + private ByteBuffer data; @Before public void before() throws IOException { - ByteBuffer data = ByteBuffer.wrap(new byte[] {1, 2, 3}); - writeChannel = new SeekableByteChannelImpl(data); - fileChannel = new CloudStorageFileChannel(writeChannel); + data = ByteBuffer.allocate(5000); + data.limit(3); + data.put(new byte[] {1, 2, 3}); + data.position(0); + readChannel = new SeekableByteChannelImpl(data); + fileChannel = new CloudStorageReadFileChannel(readChannel); } @Test @@ -140,40 +140,6 @@ public void testReadArray() throws IOException { assertThat(buffers[2].get(0), is(equalTo((byte) 3))); } - @Test - public void testWrite() throws IOException { - ByteBuffer buffer = ByteBuffer.allocate(1); - buffer.put((byte) 100).position(0); - assertThat(fileChannel.position(), is(equalTo(0L))); - assertThat(fileChannel.write(buffer), is(equalTo(1))); - assertThat(fileChannel.position(), is(equalTo(1L))); - - ByteBuffer read = ByteBuffer.allocate(1); - fileChannel.position(0); - fileChannel.read(read); - assertThat(read.get(0), is(equalTo((byte) 100))); - } - - @Test - public void testWriteArray() throws IOException { - ByteBuffer[] buffers = - new ByteBuffer[] {ByteBuffer.allocate(1), ByteBuffer.allocate(1), ByteBuffer.allocate(1)}; - buffers[0].put((byte) 10).position(0); - buffers[1].put((byte) 20).position(0); - buffers[2].put((byte) 30).position(0); - assertThat(fileChannel.position(), is(equalTo(0L))); - assertThat(fileChannel.write(buffers), is(equalTo(3L))); - assertThat(fileChannel.position(), is(equalTo(3L))); - - ByteBuffer[] read = - new ByteBuffer[] {ByteBuffer.allocate(1), ByteBuffer.allocate(1), ByteBuffer.allocate(1)}; - fileChannel.position(0); - fileChannel.read(read); - assertThat(read[0].get(0), is(equalTo((byte) 10))); - assertThat(read[1].get(0), is(equalTo((byte) 20))); - assertThat(read[2].get(0), is(equalTo((byte) 30))); - } - @Test public void testPosition() throws IOException { assertThat(fileChannel.position(), is(equalTo(0L))); @@ -186,13 +152,8 @@ public void testPosition() throws IOException { } @Test - public void testSizeAndTruncate() throws IOException { + public void testSize() throws IOException { assertThat(fileChannel.size(), is(equalTo(3L))); - fileChannel.truncate(1L); - assertThat(fileChannel.size(), is(equalTo(1L))); - fileChannel.truncate(10L); - assertThat(fileChannel.size(), is(equalTo(1L))); - assertThat(fileChannel.position(), is(equalTo(0L))); } @Test @@ -201,7 +162,27 @@ public void testTransferTo() throws IOException { assertThat(fileChannel.transferTo(0L, 3L, target), is(equalTo(3L))); assertThat(target.position(), is(equalTo(3L))); ByteBuffer dst = ByteBuffer.allocate(3); + target.position(0L); target.read(dst); + assertThat(dst.get(0), is(equalTo((byte) 1))); + assertThat(dst.get(1), is(equalTo((byte) 2))); + assertThat(dst.get(2), is(equalTo((byte) 3))); } + @Test + public void testReadOnPosition() throws IOException { + ByteBuffer buffer = ByteBuffer.allocate(1); + assertThat(fileChannel.position(), is(equalTo(0L))); + assertThat(fileChannel.read(buffer, 1L), is(equalTo(1))); + assertThat(fileChannel.position(), is(equalTo(0L))); + assertThat(buffer.get(0), is(equalTo((byte) 2))); + } + + @Test + public void testReadBeyondEnd() throws IOException { + fileChannel.position(3L); + assertThat(fileChannel.read(ByteBuffer.allocate(1)), is(equalTo(-1))); + fileChannel.position(2L); + assertThat(fileChannel.read(ByteBuffer.allocate(2)), is(equalTo(1))); + } } diff --git a/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/test/java/com/google/cloud/storage/contrib/nio/CloudStorageWriteFileChannelTest.java b/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/test/java/com/google/cloud/storage/contrib/nio/CloudStorageWriteFileChannelTest.java new file mode 100644 index 000000000000..136ec54e0e99 --- /dev/null +++ b/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/test/java/com/google/cloud/storage/contrib/nio/CloudStorageWriteFileChannelTest.java @@ -0,0 +1,203 @@ +/* + * Copyright 2019 Google LLC + * + * 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 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.google.cloud.storage.contrib.nio; + +import static org.hamcrest.CoreMatchers.equalTo; +import static org.hamcrest.CoreMatchers.is; +import static org.junit.Assert.assertThat; + +import java.io.IOException; +import java.nio.ByteBuffer; +import java.nio.channels.FileChannel; +import java.nio.channels.SeekableByteChannel; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.ExpectedException; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +@RunWith(JUnit4.class) +public class CloudStorageWriteFileChannelTest { + private static final class SeekableByteChannelImpl implements SeekableByteChannel { + private boolean open = true; + private ByteBuffer data; + + private SeekableByteChannelImpl(ByteBuffer data) { + this.data = data; + } + + @Override + public boolean isOpen() { + return open; + } + + @Override + public void close() throws IOException { + open = false; + } + + @Override + public int read(ByteBuffer dst) throws IOException { + byte[] tmp = new byte[Math.min(dst.remaining(), data.remaining())]; + if (tmp.length == 0) { + return -1; + } else { + data.get(tmp); + dst.put(tmp); + return tmp.length; + } + } + + @Override + public int write(ByteBuffer src) throws IOException { + int res = src.remaining(); + if (data.position() + res > data.limit()) { + data.limit(data.limit() + res); + } + data.put(src); + return res; + } + + @Override + public long position() throws IOException { + return data.position(); + } + + @Override + public SeekableByteChannel position(long newPosition) throws IOException { + if (newPosition >= data.limit()) { + data.limit((int) newPosition); + } + data.position((int) newPosition); + return this; + } + + @Override + public long size() throws IOException { + return data.limit(); + } + + @Override + public SeekableByteChannel truncate(long size) throws IOException { + if (size < data.limit()) { + if (data.position() >= size) { + data.position((int) size - 1); + } + data.limit((int) size); + } + return this; + } + } + + @Rule public final ExpectedException thrown = ExpectedException.none(); + + private CloudStorageWriteFileChannel fileChannel; + private SeekableByteChannel writeChannel; + private ByteBuffer data; + + @Before + public void before() throws IOException { + data = ByteBuffer.allocate(5000); + data.limit(3); + data.put(new byte[] {1, 2, 3}); + data.position(0); + writeChannel = new SeekableByteChannelImpl(data); + fileChannel = new CloudStorageWriteFileChannel(writeChannel); + } + + @Test + public void testWrite() throws IOException { + ByteBuffer buffer = ByteBuffer.allocate(1); + buffer.put((byte) 100).position(0); + assertThat(fileChannel.position(), is(equalTo(0L))); + assertThat(fileChannel.write(buffer), is(equalTo(1))); + assertThat(fileChannel.position(), is(equalTo(1L))); + assertThat(data.get(0), is(equalTo((byte) 100))); + } + + @Test + public void testWriteArray() throws IOException { + ByteBuffer[] buffers = + new ByteBuffer[] {ByteBuffer.allocate(1), ByteBuffer.allocate(1), ByteBuffer.allocate(1)}; + buffers[0].put((byte) 10).position(0); + buffers[1].put((byte) 20).position(0); + buffers[2].put((byte) 30).position(0); + assertThat(fileChannel.position(), is(equalTo(0L))); + assertThat(fileChannel.write(buffers), is(equalTo(3L))); + assertThat(fileChannel.position(), is(equalTo(3L))); + + assertThat(data.get(0), is(equalTo((byte) 10))); + assertThat(data.get(1), is(equalTo((byte) 20))); + assertThat(data.get(2), is(equalTo((byte) 30))); + } + + @Test + public void testPosition() throws IOException { + assertThat(fileChannel.position(), is(equalTo(0L))); + assertThat(fileChannel.position(1L), is(equalTo((FileChannel) fileChannel))); + assertThat(fileChannel.position(), is(equalTo(1L))); + assertThat(fileChannel.position(0L), is(equalTo((FileChannel) fileChannel))); + assertThat(fileChannel.position(), is(equalTo(0L))); + assertThat(fileChannel.position(100L), is(equalTo((FileChannel) fileChannel))); + assertThat(fileChannel.position(), is(equalTo(100L))); + } + + @Test + public void testSizeAndTruncate() throws IOException { + assertThat(fileChannel.size(), is(equalTo(3L))); + fileChannel.truncate(1L); + assertThat(fileChannel.size(), is(equalTo(1L))); + fileChannel.truncate(10L); + assertThat(fileChannel.size(), is(equalTo(1L))); + assertThat(fileChannel.position(), is(equalTo(0L))); + } + + @Test + public void testTransferFrom() throws IOException { + SeekableByteChannelImpl src = new SeekableByteChannelImpl(ByteBuffer.allocate(100)); + src.write(ByteBuffer.wrap(new byte[] {10, 20, 30})); + src.position(0L); + fileChannel.position(0L); + assertThat(fileChannel.transferFrom(src, 0L, 3L), is(equalTo(3L))); + assertThat(src.position(), is(equalTo(3L))); + + assertThat(data.get(0), is(equalTo((byte) 10))); + assertThat(data.get(1), is(equalTo((byte) 20))); + assertThat(data.get(2), is(equalTo((byte) 30))); + } + + @Test + public void testWriteOnPosition() throws IOException { + ByteBuffer buffer = ByteBuffer.allocate(1); + buffer.put((byte) 100).position(0); + assertThat(fileChannel.position(), is(equalTo(0L))); + assertThat(fileChannel.write(buffer, 0), is(equalTo(1))); + assertThat(data.get(0), is(equalTo((byte) 100))); + } + + @Test + public void testWriteBeyondEnd() throws IOException { + fileChannel.position(3L); + ByteBuffer src = ByteBuffer.wrap(new byte[] {10, 20, 30}); + assertThat(fileChannel.write(src), is(equalTo(3))); + assertThat(fileChannel.position(), is(equalTo(6L))); + fileChannel.position(3L); + assertThat(data.get(3), is(equalTo((byte) 10))); + assertThat(data.get(4), is(equalTo((byte) 20))); + assertThat(data.get(5), is(equalTo((byte) 30))); + } +} diff --git a/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/test/java/com/google/cloud/storage/contrib/nio/it/ITGcsNio.java b/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/test/java/com/google/cloud/storage/contrib/nio/it/ITGcsNio.java index e85febfa31c8..6f3293eb1cbd 100644 --- a/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/test/java/com/google/cloud/storage/contrib/nio/it/ITGcsNio.java +++ b/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/test/java/com/google/cloud/storage/contrib/nio/it/ITGcsNio.java @@ -31,15 +31,18 @@ import com.google.cloud.storage.StorageOptions; import com.google.cloud.storage.contrib.nio.CloudStorageConfiguration; import com.google.cloud.storage.contrib.nio.CloudStorageFileSystem; +import com.google.cloud.storage.contrib.nio.CloudStorageFileSystemProvider; import com.google.cloud.storage.contrib.nio.CloudStoragePath; import com.google.cloud.storage.testing.RemoteStorageHelper; import com.google.common.collect.ImmutableList; +import com.google.common.collect.Sets; import java.io.ByteArrayOutputStream; import java.io.EOFException; import java.io.IOException; import java.io.OutputStreamWriter; import java.io.PrintWriter; import java.nio.ByteBuffer; +import java.nio.channels.FileChannel; import java.nio.channels.ReadableByteChannel; import java.nio.channels.SeekableByteChannel; import java.nio.file.FileSystem; @@ -733,6 +736,240 @@ public void testFakeDirectories() throws IOException { } } + @Test + public void testFileChannelRead() throws IOException { + CloudStorageFileSystem testBucket = getTestBucket(); + Path path = testBucket.getPath(SML_FILE); + CloudStorageFileSystemProvider provider = new CloudStorageFileSystemProvider(); + FileChannel chan = provider.newFileChannel(path, Sets.newHashSet(StandardOpenOption.READ)); + long size = Files.size(path); + assertThat(chan.size()).isEqualTo(size); + ByteBuffer buf = ByteBuffer.allocate(SML_SIZE); + int read = 0; + while (chan.isOpen()) { + int rc = chan.read(buf); + assertThat(chan.size()).isEqualTo(size); + if (rc < 0) { + // EOF + break; + } + assertThat(rc).isGreaterThan(0); + read += rc; + assertThat(chan.position()).isEqualTo(read); + } + chan.close(); + assertThat(read).isEqualTo(size); + byte[] expected = new byte[SML_SIZE]; + new Random(SML_SIZE).nextBytes(expected); + assertThat(Arrays.equals(buf.array(), expected)).isTrue(); + } + + @Test + public void testFileChannelCreate() throws IOException { + CloudStorageFileSystem testBucket = getTestBucket(); + Path path = testBucket.getPath(PREFIX + randomSuffix()); + assertThat(Files.exists(path)).isFalse(); + CloudStorageFileSystemProvider provider = new CloudStorageFileSystemProvider(); + try { + FileChannel channel = + provider.newFileChannel(path, Sets.newHashSet(StandardOpenOption.CREATE)); + channel.close(); + assertThat(Files.exists(path)).isTrue(); + long size = Files.size(path); + assertThat(size).isEqualTo(0); + } finally { + Files.deleteIfExists(path); + } + } + + @Test + public void testFileChannelWrite() throws IOException { + CloudStorageFileSystem testBucket = getTestBucket(); + Path path = testBucket.getPath(PREFIX + randomSuffix()); + assertThat(Files.exists(path)).isFalse(); + CloudStorageFileSystemProvider provider = new CloudStorageFileSystemProvider(); + try { + FileChannel channel = + provider.newFileChannel( + path, Sets.newHashSet(StandardOpenOption.CREATE, StandardOpenOption.WRITE)); + ByteBuffer src = ByteBuffer.allocate(5000); + int written = 0; + for (String s : FILE_CONTENTS) { + byte[] bytes = (s + "\n").getBytes(UTF_8); + written += bytes.length; + src.put(bytes); + } + src.limit(written); + src.position(0); + channel.write(src); + channel.close(); + assertThat(Files.exists(path)).isTrue(); + + ByteArrayOutputStream wantBytes = new ByteArrayOutputStream(); + PrintWriter writer = new PrintWriter(new OutputStreamWriter(wantBytes, UTF_8)); + for (String content : FILE_CONTENTS) { + writer.println(content); + } + writer.close(); + SeekableByteChannel chan = Files.newByteChannel(path, StandardOpenOption.READ); + byte[] gotBytes = new byte[(int) chan.size()]; + readFully(chan, gotBytes); + assertThat(gotBytes).isEqualTo(wantBytes.toByteArray()); + } finally { + Files.deleteIfExists(path); + } + } + + @Test + public void testFileChannelWriteOnClose() throws IOException { + CloudStorageFileSystem testBucket = getTestBucket(); + Path path = testBucket.getPath(PREFIX + randomSuffix()); + assertThat(Files.exists(path)).isFalse(); + CloudStorageFileSystemProvider provider = new CloudStorageFileSystemProvider(); + try { + long expectedSize = 0; + try (FileChannel chan = + provider.newFileChannel(path, Sets.newHashSet(StandardOpenOption.WRITE))) { + for (String s : FILE_CONTENTS) { + byte[] sBytes = s.getBytes(UTF_8); + expectedSize += sBytes.length * 9999; + for (int i = 0; i < 9999; i++) { + chan.write(ByteBuffer.wrap(sBytes)); + } + } + try { + Files.size(path); + // we shouldn't make it to this line. Not using thrown.expect because + // I still want to run a few lines after the exception. + Assert.fail("Files.size should have thrown an exception"); + } catch (NoSuchFileException nsf) { + // that's what we wanted, we're good. + } + } + // channel now closed, the file should be there and with the new contents. + assertThat(Files.exists(path)).isTrue(); + assertThat(Files.size(path)).isEqualTo(expectedSize); + } finally { + Files.deleteIfExists(path); + } + } + + @Test + public void testFileChannelSeek() throws IOException { + CloudStorageFileSystem testBucket = getTestBucket(); + Path path = testBucket.getPath(BIG_FILE); + CloudStorageFileSystemProvider provider = new CloudStorageFileSystemProvider(); + int size = BIG_SIZE; + byte[] contents = randomContents(size); + byte[] sample = new byte[100]; + byte[] wanted; + byte[] wanted2; + FileChannel chan = provider.newFileChannel(path, Sets.newHashSet(StandardOpenOption.READ)); + assertThat(chan.size()).isEqualTo(size); + + // check seek + int dest = size / 2; + chan.position(dest); + readFully(chan, sample); + wanted = Arrays.copyOfRange(contents, dest, dest + 100); + assertThat(wanted).isEqualTo(sample); + // now go back and check the beginning + // (we do 2 locations because 0 is sometimes a special case). + chan.position(0); + readFully(chan, sample); + wanted2 = Arrays.copyOf(contents, 100); + assertThat(wanted2).isEqualTo(sample); + // if the two spots in the file have the same contents, then this isn't a good file for this + // test. + assertThat(wanted).isNotEqualTo(wanted2); + } + + @Test + public void testFileChannelTransferFrom() throws IOException { + CloudStorageFileSystem testBucket = getTestBucket(); + Path path = testBucket.getPath(SML_FILE); + Path destPath = testBucket.getPath(PREFIX + randomSuffix()); + assertThat(Files.exists(destPath)).isFalse(); + CloudStorageFileSystemProvider provider = new CloudStorageFileSystemProvider(); + try { + try (FileChannel source = + provider.newFileChannel(path, Sets.newHashSet(StandardOpenOption.READ)); + FileChannel dest = + provider.newFileChannel( + destPath, Sets.newHashSet(StandardOpenOption.CREATE, StandardOpenOption.WRITE))) { + dest.transferFrom(source, 0L, SML_SIZE); + } + + FileChannel source = + provider.newFileChannel(destPath, Sets.newHashSet(StandardOpenOption.READ)); + long size = Files.size(destPath); + assertThat(source.size()).isEqualTo(size); + ByteBuffer buf = ByteBuffer.allocate(SML_SIZE); + int read = 0; + while (source.isOpen()) { + int rc = source.read(buf); + assertThat(source.size()).isEqualTo(size); + if (rc < 0) { + // EOF + break; + } + assertThat(rc).isGreaterThan(0); + read += rc; + assertThat(source.position()).isEqualTo(read); + } + source.close(); + assertThat(read).isEqualTo(size); + byte[] expected = new byte[SML_SIZE]; + new Random(SML_SIZE).nextBytes(expected); + assertThat(Arrays.equals(buf.array(), expected)).isTrue(); + } finally { + Files.deleteIfExists(destPath); + } + } + + @Test + public void testFileChannelTransferTo() throws IOException { + CloudStorageFileSystem testBucket = getTestBucket(); + Path path = testBucket.getPath(SML_FILE); + Path destPath = testBucket.getPath(PREFIX + randomSuffix()); + assertThat(Files.exists(destPath)).isFalse(); + CloudStorageFileSystemProvider provider = new CloudStorageFileSystemProvider(); + try { + try (FileChannel source = + provider.newFileChannel(path, Sets.newHashSet(StandardOpenOption.READ)); + FileChannel dest = + provider.newFileChannel( + destPath, Sets.newHashSet(StandardOpenOption.CREATE, StandardOpenOption.WRITE))) { + source.transferTo(0L, SML_SIZE, dest); + } + + FileChannel source = + provider.newFileChannel(destPath, Sets.newHashSet(StandardOpenOption.READ)); + long size = Files.size(destPath); + assertThat(source.size()).isEqualTo(size); + ByteBuffer buf = ByteBuffer.allocate(SML_SIZE); + int read = 0; + while (source.isOpen()) { + int rc = source.read(buf); + assertThat(source.size()).isEqualTo(size); + if (rc < 0) { + // EOF + break; + } + assertThat(rc).isGreaterThan(0); + read += rc; + assertThat(source.position()).isEqualTo(read); + } + source.close(); + assertThat(read).isEqualTo(size); + byte[] expected = new byte[SML_SIZE]; + new Random(SML_SIZE).nextBytes(expected); + assertThat(Arrays.equals(buf.array(), expected)).isTrue(); + } finally { + Files.deleteIfExists(destPath); + } + } + /** * Delete the given directory and all of its contents if non-empty. * From 831dbf756b20143a9110f975fc006af81c8d2943 Mon Sep 17 00:00:00 2001 From: Olav Loite Date: Fri, 22 Mar 2019 08:15:01 +0100 Subject: [PATCH 03/11] removed erroneously committed file --- ...IntegrationTestWithAbortedInjectorEnv.java | 35 ------------------- 1 file changed, 35 deletions(-) delete mode 100644 google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/IntegrationTestWithAbortedInjectorEnv.java diff --git a/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/IntegrationTestWithAbortedInjectorEnv.java b/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/IntegrationTestWithAbortedInjectorEnv.java deleted file mode 100644 index 78e171fcb44a..000000000000 --- a/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/IntegrationTestWithAbortedInjectorEnv.java +++ /dev/null @@ -1,35 +0,0 @@ -package com.google.cloud.spanner; - -import com.google.cloud.spanner.SpannerOptions.SpannerTestOptions; - -public class IntegrationTestWithAbortedInjectorEnv extends IntegrationTestEnv { - private final AbortedTransactionInjectorImpl injector = new AbortedTransactionInjectorImpl(); - - public AbortedTransactionInjectorImpl getAbortedTransactionInjector() { - return injector; - } - - @Override - protected SpannerTestOptions createSpannerTestOptions(SpannerTestOptions testOptions) { - return testOptions.toBuilder().setAbortedTransactionInjector(injector).build(); - } - - /** Injects simulated aborted transaction. */ - public static final class AbortedTransactionInjectorImpl implements AbortedTransactionInjector { - private boolean injectAbortOnce = false; - - @Override - public boolean shouldAbort() { - if (injectAbortOnce) { - injectAbortOnce = false; - return true; - } - return false; - } - - @Override - public void injectAbortOnce() { - this.injectAbortOnce = true; - } - } -} From 7f38d08e248852bf161c1ce8624e5159daced8ad Mon Sep 17 00:00:00 2001 From: Olav Loite Date: Fri, 22 Mar 2019 19:49:24 +0100 Subject: [PATCH 04/11] fixed formatting --- .../nio/CloudStorageFileSystemProvider.java | 246 ++++++++++-------- 1 file changed, 144 insertions(+), 102 deletions(-) diff --git a/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageFileSystemProvider.java b/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageFileSystemProvider.java index 78f54ef78a7e..4a94804ef0ea 100644 --- a/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageFileSystemProvider.java +++ b/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageFileSystemProvider.java @@ -19,6 +19,25 @@ import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkNotNull; import static com.google.common.base.Strings.isNullOrEmpty; + +import com.google.api.gax.paging.Page; +import com.google.auto.service.AutoService; +import com.google.cloud.storage.Acl; +import com.google.cloud.storage.Blob; +import com.google.cloud.storage.BlobId; +import com.google.cloud.storage.BlobInfo; +import com.google.cloud.storage.Bucket; +import com.google.cloud.storage.CopyWriter; +import com.google.cloud.storage.Storage; +import com.google.cloud.storage.Storage.BlobGetOption; +import com.google.cloud.storage.Storage.BlobSourceOption; +import com.google.cloud.storage.StorageException; +import com.google.cloud.storage.StorageOptions; +import com.google.common.annotations.VisibleForTesting; +import com.google.common.base.MoreObjects; +import com.google.common.collect.AbstractIterator; +import com.google.common.net.UrlEscapers; +import com.google.common.primitives.Ints; import java.io.BufferedInputStream; import java.io.IOException; import java.io.InputStream; @@ -56,33 +75,14 @@ import javax.annotation.Nullable; import javax.annotation.concurrent.ThreadSafe; import javax.inject.Singleton; -import com.google.api.gax.paging.Page; -import com.google.auto.service.AutoService; -import com.google.cloud.storage.Acl; -import com.google.cloud.storage.Blob; -import com.google.cloud.storage.BlobId; -import com.google.cloud.storage.BlobInfo; -import com.google.cloud.storage.Bucket; -import com.google.cloud.storage.CopyWriter; -import com.google.cloud.storage.Storage; -import com.google.cloud.storage.Storage.BlobGetOption; -import com.google.cloud.storage.Storage.BlobSourceOption; -import com.google.cloud.storage.StorageException; -import com.google.cloud.storage.StorageOptions; -import com.google.common.annotations.VisibleForTesting; -import com.google.common.base.MoreObjects; -import com.google.common.collect.AbstractIterator; -import com.google.common.net.UrlEscapers; -import com.google.common.primitives.Ints; /** * Google Cloud Storage {@link FileSystemProvider} implementation. * - *

- * Note: This class should never be used directly. This class is instantiated by the service - * loader and called through a standardized API, e.g. {@link java.nio.file.Files}. However the - * javadocs in this class serve as useful documentation for the behavior of the Google Cloud Storage - * NIO library. + *

Note: This class should never be used directly. This class is instantiated by the + * service loader and called through a standardized API, e.g. {@link java.nio.file.Files}. However + * the javadocs in this class serve as useful documentation for the behavior of the Google Cloud + * Storage NIO library. */ @Singleton @ThreadSafe @@ -105,8 +105,12 @@ private static class LazyPathIterator extends AbstractIterator { // whether to make the paths absolute before returning them. private final boolean absolutePaths; - LazyPathIterator(CloudStorageFileSystem fileSystem, String prefix, Iterator blobIterator, - Filter filter, boolean absolutePaths) { + LazyPathIterator( + CloudStorageFileSystem fileSystem, + String prefix, + Iterator blobIterator, + Filter filter, + boolean absolutePaths) { this.prefix = prefix; this.blobIterator = blobIterator; this.filter = filter; @@ -147,20 +151,17 @@ public static void setStorageOptions(@Nullable StorageOptions newStorageOptions) * Changes the default configuration for every filesystem object created from here on, including * via SPI. If null then future filesystem objects will have the factory default configuration. * - *

- * If options are specified later then they override the defaults. Methods that take a whole + *

If options are specified later then they override the defaults. Methods that take a whole * CloudStorageConfiguration (eg. CloudStorageFileSystem.forBucket) will completely override the * defaults. Methods that take individual options (eg. * CloudStorageFileSystemProvier.newFileSystem) will override only these options; the rest will be * taken from the defaults specified here. * - *

- * This is meant to be done only once, at the beginning of some main program, in order to force + *

This is meant to be done only once, at the beginning of some main program, in order to force * all libraries to use some settings we like. * - *

- * Libraries should never call this. If you're a library then, instead, create your own filesystem - * object with the right configuration and pass it along. + *

Libraries should never call this. If you're a library then, instead, create your own + * filesystem object with the right configuration and pass it along. * * @param newDefault new default CloudStorageConfiguration */ @@ -176,7 +177,8 @@ public static void setDefaultCloudStorageConfiguration( * @see CloudStorageFileSystem#forBucket(String) */ public CloudStorageFileSystemProvider() { - this(CloudStorageFileSystem.getDefaultCloudStorageConfiguration().userProject(), + this( + CloudStorageFileSystem.getDefaultCloudStorageConfiguration().userProject(), futureStorageOptions); } @@ -191,8 +193,8 @@ public CloudStorageFileSystemProvider() { * Internal constructor, fully configurable. Note that null options means to use the system * defaults (NOT the user-provided ones). */ - CloudStorageFileSystemProvider(@Nullable String userProject, - @Nullable StorageOptions gcsStorageOptions) { + CloudStorageFileSystemProvider( + @Nullable String userProject, @Nullable StorageOptions gcsStorageOptions) { this.storageOptions = gcsStorageOptions; this.userProject = userProject; } @@ -227,33 +229,45 @@ public CloudStorageFileSystem getFileSystem(URI uri) { * include a path component (that will be ignored). * * @param uri bucket and current working directory, e.g. {@code gs://bucket} - * @param env map of configuration options, whose keys correspond to the method names of - * {@link CloudStorageConfiguration.Builder}. However you are not allowed to set the - * working directory, as that should be provided in the {@code uri} + * @param env map of configuration options, whose keys correspond to the method names of {@link + * CloudStorageConfiguration.Builder}. However you are not allowed to set the working + * directory, as that should be provided in the {@code uri} * @throws IllegalArgumentException if {@code uri} specifies a port, user, query, or fragment, or - * if scheme is not {@value CloudStorageFileSystem#URI_SCHEME} + * if scheme is not {@value CloudStorageFileSystem#URI_SCHEME} */ @Override public CloudStorageFileSystem newFileSystem(URI uri, Map env) { - checkArgument(uri.getScheme().equalsIgnoreCase(CloudStorageFileSystem.URI_SCHEME), - "Cloud Storage URIs must have '%s' scheme: %s", CloudStorageFileSystem.URI_SCHEME, uri); - checkArgument(!isNullOrEmpty(uri.getHost()), "%s:// URIs must have a host: %s", - CloudStorageFileSystem.URI_SCHEME, uri); checkArgument( - uri.getPort() == -1 && isNullOrEmpty(uri.getQuery()) && isNullOrEmpty(uri.getFragment()) + uri.getScheme().equalsIgnoreCase(CloudStorageFileSystem.URI_SCHEME), + "Cloud Storage URIs must have '%s' scheme: %s", + CloudStorageFileSystem.URI_SCHEME, + uri); + checkArgument( + !isNullOrEmpty(uri.getHost()), + "%s:// URIs must have a host: %s", + CloudStorageFileSystem.URI_SCHEME, + uri); + checkArgument( + uri.getPort() == -1 + && isNullOrEmpty(uri.getQuery()) + && isNullOrEmpty(uri.getFragment()) && isNullOrEmpty(uri.getUserInfo()), - "GCS FileSystem URIs mustn't have: port, userinfo, query, or fragment: %s", uri); + "GCS FileSystem URIs mustn't have: port, userinfo, query, or fragment: %s", + uri); CloudStorageUtil.checkBucket(uri.getHost()); initStorage(); - return new CloudStorageFileSystem(this, uri.getHost(), CloudStorageConfiguration - .fromMap(CloudStorageFileSystem.getDefaultCloudStorageConfiguration(), env)); + return new CloudStorageFileSystem( + this, + uri.getHost(), + CloudStorageConfiguration.fromMap( + CloudStorageFileSystem.getDefaultCloudStorageConfiguration(), env)); } @Override public CloudStoragePath getPath(URI uri) { initStorage(); - return CloudStoragePath.getPath(getFileSystem(CloudStorageUtil.stripPathFromUri(uri)), - uri.getPath()); + return CloudStoragePath.getPath( + getFileSystem(CloudStorageUtil.stripPathFromUri(uri)), uri.getPath()); } /** Convenience method: replaces spaces with "%20", builds a URI, and calls getPath(uri). */ @@ -268,14 +282,14 @@ public CloudStoragePath getPath(String uriInStringForm) { * * @param path: the path to the file to open or create * @param options: options specifying how the file is opened, e.g. StandardOpenOption.WRITE or - * BlobSourceOption.userProject + * BlobSourceOption.userProject * @param attrs: (not supported, values will be ignored) * @return * @throws IOException */ @Override - public SeekableByteChannel newByteChannel(Path path, Set options, - FileAttribute... attrs) throws IOException { + public SeekableByteChannel newByteChannel( + Path path, Set options, FileAttribute... attrs) throws IOException { checkNotNull(path); initStorage(); CloudStorageUtil.checkNotNullArray(attrs); @@ -290,25 +304,27 @@ public SeekableByteChannel newByteChannel(Path path, Set o /** * Open a file for reading OR writing. The {@link FileChannel} that is returned will only allow * reads or writes depending on the {@link OpenOption}s that are specified. If any of the - * following have been specified, the {@link FileChannel} will be write-only: - * {@link StandardOpenOption#CREATE} + * following have been specified, the {@link FileChannel} will be write-only: {@link + * StandardOpenOption#CREATE} + * *

    - *
  • {@link StandardOpenOption#CREATE}
    • - *
  • {@link StandardOpenOption#CREATE_NEW}
    • - *
  • {@link StandardOpenOption#WRITE}
    • - *
  • {@link StandardOpenOption#TRUNCATE_EXISTING}
    • + *
  • {@link StandardOpenOption#CREATE} + *
  • {@link StandardOpenOption#CREATE_NEW} + *
  • {@link StandardOpenOption#WRITE} + *
  • {@link StandardOpenOption#TRUNCATE_EXISTING} *
+ * * In all other cases the {@link FileChannel} will be read-only. * * @param path The path to the file to open or create - * @param options The options specifying how the file should be opened, and whether the - * {@link FileChannel} should be read-only or write-only. + * @param options The options specifying how the file should be opened, and whether the {@link + * FileChannel} should be read-only or write-only. * @param attrs (not supported, the values will be ignored) * @throws IOException */ @Override - public FileChannel newFileChannel(Path path, Set options, - FileAttribute... attrs) throws IOException { + public FileChannel newFileChannel( + Path path, Set options, FileAttribute... attrs) throws IOException { checkNotNull(path); initStorage(); CloudStorageUtil.checkNotNullArray(attrs); @@ -317,7 +333,8 @@ public FileChannel newFileChannel(Path path, Set options, } else if (options.contains(StandardOpenOption.CREATE) && !Files.exists(path)) { Files.createFile(path, attrs); } - if (options.contains(StandardOpenOption.WRITE) || options.contains(StandardOpenOption.CREATE) + if (options.contains(StandardOpenOption.WRITE) + || options.contains(StandardOpenOption.CREATE) || options.contains(StandardOpenOption.CREATE_NEW) || options.contains(StandardOpenOption.TRUNCATE_EXISTING)) { CloudStorageWriteChannel writeChannel = @@ -368,8 +385,13 @@ private SeekableByteChannel newReadChannel(Path path, Set if (cloudPath.seemsLikeADirectoryAndUsePseudoDirectories(null)) { throw new CloudStoragePseudoDirectoryException(cloudPath); } - return CloudStorageReadChannel.create(storage, cloudPath.getBlobId(), 0, maxChannelReopens, - cloudPath.getFileSystem().config(), userProject, + return CloudStorageReadChannel.create( + storage, + cloudPath.getBlobId(), + 0, + maxChannelReopens, + cloudPath.getFileSystem().config(), + userProject, blobSourceOptions.toArray(new BlobSourceOption[blobSourceOptions.size()])); } @@ -442,8 +464,10 @@ private SeekableByteChannel newWriteChannel(Path path, Set } try { - return new CloudStorageWriteChannel(storage.writer(infoBuilder.build(), - writeOptions.toArray(new Storage.BlobWriteOption[writeOptions.size()]))); + return new CloudStorageWriteChannel( + storage.writer( + infoBuilder.build(), + writeOptions.toArray(new Storage.BlobWriteOption[writeOptions.size()]))); } catch (StorageException oops) { throw asIoException(oops); } @@ -488,8 +512,8 @@ public boolean deleteIfExists(Path path) throws IOException { if (isNullOrEmpty(userProject)) { return storage.delete(cloudPath.getBlobId()); } else { - return storage.delete(cloudPath.getBlobId(), - Storage.BlobSourceOption.userProject(userProject)); + return storage.delete( + cloudPath.getBlobId(), Storage.BlobSourceOption.userProject(userProject)); } } catch (StorageException exs) { // Will rethrow a StorageException if all retries/reopens are exhausted @@ -513,7 +537,9 @@ public void move(Path source, Path target, CopyOption... options) throws IOExcep initStorage(); for (CopyOption option : options) { if (option == StandardCopyOption.ATOMIC_MOVE) { - throw new AtomicMoveNotSupportedException(source.toString(), target.toString(), + throw new AtomicMoveNotSupportedException( + source.toString(), + target.toString(), "Google Cloud Storage does not support atomic move operations."); } } @@ -562,8 +588,8 @@ public void copy(Path source, Path target, CopyOption... options) throws IOExcep tgtInfoBuilder.setContentEncoding(((OptionContentEncoding) option).contentEncoding()); overrideContentEncoding = true; } else if (option instanceof OptionContentDisposition) { - tgtInfoBuilder - .setContentDisposition(((OptionContentDisposition) option).contentDisposition()); + tgtInfoBuilder.setContentDisposition( + ((OptionContentDisposition) option).contentDisposition()); overrideContentDisposition = true; } else { throw new UnsupportedOperationException(option.toString()); @@ -575,9 +601,12 @@ public void copy(Path source, Path target, CopyOption... options) throws IOExcep CloudStoragePath fromPath = CloudStorageUtil.checkPath(source); - blockSize = blockSize != -1 ? blockSize - : Ints.max(fromPath.getFileSystem().config().blockSize(), - toPath.getFileSystem().config().blockSize()); + blockSize = + blockSize != -1 + ? blockSize + : Ints.max( + fromPath.getFileSystem().config().blockSize(), + toPath.getFileSystem().config().blockSize()); // TODO: actually use blockSize if (fromPath.seemsLikeADirectory() && toPath.seemsLikeADirectory()) { @@ -643,11 +672,13 @@ public void copy(Path source, Path target, CopyOption... options) throws IOExcep copyReqBuilder.setTarget(tgtInfo, Storage.BlobTargetOption.doesNotExist()); } if (!isNullOrEmpty(fromPath.getFileSystem().config().userProject())) { - copyReqBuilder = copyReqBuilder.setSourceOptions( - BlobSourceOption.userProject(fromPath.getFileSystem().config().userProject())); + copyReqBuilder = + copyReqBuilder.setSourceOptions( + BlobSourceOption.userProject(fromPath.getFileSystem().config().userProject())); } else if (!isNullOrEmpty(toPath.getFileSystem().config().userProject())) { - copyReqBuilder = copyReqBuilder.setSourceOptions( - BlobSourceOption.userProject(toPath.getFileSystem().config().userProject())); + copyReqBuilder = + copyReqBuilder.setSourceOptions( + BlobSourceOption.userProject(toPath.getFileSystem().config().userProject())); } CopyWriter copyWriter = storage.copy(copyReqBuilder.build()); copyWriter.getResult(); @@ -698,12 +729,16 @@ public void checkAccess(Path path, AccessMode... modes) throws IOException { try { boolean nullId; if (isNullOrEmpty(userProject)) { - nullId = storage.get(cloudPath.getBlobId(), - Storage.BlobGetOption.fields(Storage.BlobField.ID)) == null; + nullId = + storage.get(cloudPath.getBlobId(), Storage.BlobGetOption.fields(Storage.BlobField.ID)) + == null; } else { nullId = - storage.get(cloudPath.getBlobId(), Storage.BlobGetOption.fields(Storage.BlobField.ID), - Storage.BlobGetOption.userProject(userProject)) == null; + storage.get( + cloudPath.getBlobId(), + Storage.BlobGetOption.fields(Storage.BlobField.ID), + Storage.BlobGetOption.userProject(userProject)) + == null; } if (nullId) { if (cloudPath.seemsLikeADirectoryAndUsePseudoDirectories(storage)) { @@ -731,8 +766,8 @@ public void checkAccess(Path path, AccessMode... modes) throws IOException { } @Override - public A readAttributes(Path path, Class type, - LinkOption... options) throws IOException { + public A readAttributes( + Path path, Class type, LinkOption... options) throws IOException { checkNotNull(type); CloudStorageUtil.checkNotNullArray(options); if (type != CloudStorageFileAttributes.class && type != BasicFileAttributes.class) { @@ -805,8 +840,8 @@ public Map readAttributes(Path path, String attributes, LinkOpti } @Override - public V getFileAttributeView(Path path, Class type, - LinkOption... options) { + public V getFileAttributeView( + Path path, Class type, LinkOption... options) { checkNotNull(type); CloudStorageUtil.checkNotNullArray(options); if (type != CloudStorageFileAttributeView.class && type != BasicFileAttributeView.class) { @@ -826,8 +861,8 @@ public void createDirectory(Path dir, FileAttribute... attrs) { } @Override - public DirectoryStream newDirectoryStream(final Path dir, - final Filter filter) { + public DirectoryStream newDirectoryStream( + final Path dir, final Filter filter) { final CloudStoragePath cloudPath = CloudStorageUtil.checkPath(dir); checkNotNull(filter); initStorage(); @@ -846,19 +881,27 @@ public DirectoryStream newDirectoryStream(final Path dir, final String prefix = prePrefix; Page dirList; if (isNullOrEmpty(userProject)) { - dirList = storage.list(cloudPath.bucket(), Storage.BlobListOption.prefix(prefix), - Storage.BlobListOption.currentDirectory(), Storage.BlobListOption.fields()); + dirList = + storage.list( + cloudPath.bucket(), + Storage.BlobListOption.prefix(prefix), + Storage.BlobListOption.currentDirectory(), + Storage.BlobListOption.fields()); } else { - dirList = storage.list(cloudPath.bucket(), Storage.BlobListOption.prefix(prefix), - Storage.BlobListOption.currentDirectory(), Storage.BlobListOption.fields(), - Storage.BlobListOption.userProject(userProject)); + dirList = + storage.list( + cloudPath.bucket(), + Storage.BlobListOption.prefix(prefix), + Storage.BlobListOption.currentDirectory(), + Storage.BlobListOption.fields(), + Storage.BlobListOption.userProject(userProject)); } final Iterator blobIterator = dirList.iterateAll().iterator(); return new DirectoryStream() { @Override public Iterator iterator() { - return new LazyPathIterator(cloudPath.getFileSystem(), prefix, blobIterator, filter, - dir.isAbsolute()); + return new LazyPathIterator( + cloudPath.getFileSystem(), prefix, blobIterator, filter, dir.isAbsolute()); } @Override @@ -890,8 +933,9 @@ public FileStore getFileStore(Path path) { @Override public boolean equals(Object other) { - return this == other || other instanceof CloudStorageFileSystemProvider - && Objects.equals(storage, ((CloudStorageFileSystemProvider) other).storage); + return this == other + || other instanceof CloudStorageFileSystemProvider + && Objects.equals(storage, ((CloudStorageFileSystemProvider) other).storage); } @Override @@ -927,8 +971,7 @@ public boolean requesterPays(String bucketName) { * Returns a NEW CloudStorageFileSystemProvider identical to this one, but with userProject * removed. * - *

- * Perhaps you want to call this is you realize you'll be working on a bucket that is not + *

Perhaps you want to call this is you realize you'll be working on a bucket that is not * requester-pays. */ public CloudStorageFileSystemProvider withNoUserProject() { @@ -944,8 +987,7 @@ public String getProject() { /** * Lists the project's buckets. But use the one in CloudStorageFileSystem. * - *

- * Example of listing buckets, specifying the page size and a name prefix. + *

Example of listing buckets, specifying the page size and a name prefix. * * 

    * {

From e87fbc96890d4061c67e385474c8bbe863c7089c Mon Sep 17 00:00:00 2001
From: Olav Loite 
Date: Sat, 23 Mar 2019 06:37:21 +0100
Subject: [PATCH 05/11] added synchronization to file channel classes

---
 .../nio/CloudStorageReadFileChannel.java      |  46 ++++---
 .../nio/CloudStorageWriteFileChannel.java     | 116 ++++++++++--------
 2 files changed, 92 insertions(+), 70 deletions(-)

diff --git a/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageReadFileChannel.java b/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageReadFileChannel.java
index 45bc43fc498d..ac71cd152584 100644
--- a/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageReadFileChannel.java
+++ b/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageReadFileChannel.java
@@ -42,8 +42,10 @@ public int read(ByteBuffer dst) throws IOException {
   @Override
   public long read(ByteBuffer[] dsts, int offset, int length) throws IOException {
     long res = 0L;
-    for (int i = offset; i < offset + length; i++) {
-      res += readChannel.read(dsts[i]);
+    synchronized (this) {
+      for (int i = offset; i < offset + length; i++) {
+        res += readChannel.read(dsts[i]);
+      }
     }
     return res;
   }
@@ -86,23 +88,25 @@ public void force(boolean metaData) throws IOException {
 
   @Override
   public long transferTo(long position, long count, WritableByteChannel target) throws IOException {
-    long originalPosition = position();
-    position(position);
-    int blockSize = (int) Math.min(count, 0xfffffL);
     long res = 0L;
-    int bytesRead = 0;
-    ByteBuffer buffer = ByteBuffer.allocate(blockSize);
-    while (res < count && bytesRead >= 0) {
-      buffer.position(0);
-      bytesRead = read(buffer);
-      if (bytesRead > 0) {
+    synchronized (this) {
+      long originalPosition = position();
+      position(position);
+      int blockSize = (int) Math.min(count, 0xfffffL);
+      int bytesRead = 0;
+      ByteBuffer buffer = ByteBuffer.allocate(blockSize);
+      while (res < count && bytesRead >= 0) {
         buffer.position(0);
-        buffer.limit(bytesRead);
-        target.write(buffer);
-        res += bytesRead;
+        bytesRead = read(buffer);
+        if (bytesRead > 0) {
+          buffer.position(0);
+          buffer.limit(bytesRead);
+          target.write(buffer);
+          res += bytesRead;
+        }
       }
+      position(originalPosition);
     }
-    position(originalPosition);
     return res;
   }
 
@@ -113,11 +117,13 @@ public long transferFrom(ReadableByteChannel src, long position, long count) thr
 
   @Override
   public int read(ByteBuffer dst, long position) throws IOException {
-    long originalPosition = position();
-    position(position);
-    int res = readChannel.read(dst);
-    position(originalPosition);
-    return res;
+    synchronized (this) {
+      long originalPosition = position();
+      position(position);
+      int res = readChannel.read(dst);
+      position(originalPosition);
+      return res;
+    }
   }
 
   @Override
diff --git a/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageWriteFileChannel.java b/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageWriteFileChannel.java
index 6a3406e380a7..935fa7de61bc 100644
--- a/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageWriteFileChannel.java
+++ b/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageWriteFileChannel.java
@@ -60,47 +60,58 @@ public long read(ByteBuffer[] dsts, int offset, int length) throws IOException {
 
   @Override
   public int write(ByteBuffer src) throws IOException {
-    checkValid();
-    int res = writeChannel.write(src);
-    return res;
+    synchronized (this) {
+      checkValid();
+      return writeChannel.write(src);
+    }
   }
 
   @Override
   public long write(ByteBuffer[] srcs, int offset, int length) throws IOException {
-    checkValid();
-    long res = 0L;
-    for (int i = offset; i < offset + length; i++) {
-      res += writeChannel.write(srcs[i]);
+    synchronized (this) {
+      checkValid();
+      long res = 0L;
+      for (int i = offset; i < offset + length; i++) {
+        res += writeChannel.write(srcs[i]);
+      }
+      return res;
     }
-    return res;
   }
 
   @Override
   public long position() throws IOException {
-    checkValid();
-    return writeChannel.position();
+    synchronized (this) {
+      checkValid();
+      return writeChannel.position();
+    }
   }
 
   @Override
   public FileChannel position(long newPosition) throws IOException {
-    checkValid();
-    if (newPosition != position()) {
-      writeChannel.position(newPosition);
+    synchronized (this) {
+      checkValid();
+      if (newPosition != position()) {
+        writeChannel.position(newPosition);
+      }
+      return this;
     }
-    return this;
   }
 
   @Override
   public long size() throws IOException {
-    checkValid();
-    return writeChannel.size();
+    synchronized (this) {
+      checkValid();
+      return writeChannel.size();
+    }
   }
 
   @Override
   public FileChannel truncate(long size) throws IOException {
-    checkValid();
-    writeChannel.truncate(size);
-    return this;
+    synchronized (this) {
+      checkValid();
+      writeChannel.truncate(size);
+      return this;
+    }
   }
 
   @Override
@@ -115,30 +126,32 @@ public long transferTo(long position, long count, WritableByteChannel target) th
 
   @Override
   public long transferFrom(ReadableByteChannel src, long position, long count) throws IOException {
-    checkValid();
-    if (position != position()) {
-      throw new UnsupportedOperationException(
-          "This FileChannel only supports transferFrom at the current position");
-    }
-    int blockSize = (int) Math.min(count, 0xfffffL);
-    long res = 0L;
-    int bytesRead = 0;
-    ByteBuffer buffer = ByteBuffer.allocate(blockSize);
-    while (res < count && bytesRead >= 0) {
-      buffer.position(0);
-      bytesRead = src.read(buffer);
-      if (bytesRead > 0) {
+    synchronized (this) {
+      checkValid();
+      if (position != position()) {
+        throw new UnsupportedOperationException(
+            "This FileChannel only supports transferFrom at the current position");
+      }
+      int blockSize = (int) Math.min(count, 0xfffffL);
+      long res = 0L;
+      int bytesRead = 0;
+      ByteBuffer buffer = ByteBuffer.allocate(blockSize);
+      while (res < count && bytesRead >= 0) {
         buffer.position(0);
-        buffer.limit(bytesRead);
-        write(buffer);
-        res += bytesRead;
+        bytesRead = src.read(buffer);
+        if (bytesRead > 0) {
+          buffer.position(0);
+          buffer.limit(bytesRead);
+          write(buffer);
+          res += bytesRead;
+        }
       }
+      // The channel is no longer valid as the position has been updated, and there is no way of
+      // resetting it, but this way we at least support the write-at-position and transferFrom
+      // methods being called once.
+      this.valid = false;
+      return res;
     }
-    // The channel is no longer valid as the position has been updated, and there is no way of
-    // resetting it, but this way we at least support the write-at-position and transferFrom methods
-    // being called once.
-    this.valid = false;
-    return res;
   }
 
   @Override
@@ -148,17 +161,20 @@ public int read(ByteBuffer dst, long position) throws IOException {
 
   @Override
   public int write(ByteBuffer src, long position) throws IOException {
-    checkValid();
-    if (position != position()) {
-      throw new UnsupportedOperationException(
-          "This FileChannel only supports write at the current position");
+    synchronized (this) {
+      checkValid();
+      if (position != position()) {
+        throw new UnsupportedOperationException(
+            "This FileChannel only supports write at the current position");
+      }
+      int res = writeChannel.write(src);
+      // The channel is no longer valid as the position has been updated, and there is no way of
+      // resetting it, but this way we at least support the write-at-position and transferFrom
+      // methods
+      // being called once.
+      this.valid = false;
+      return res;
     }
-    int res = writeChannel.write(src);
-    // The channel is no longer valid as the position has been updated, and there is no way of
-    // resetting it, but this way we at least support the write-at-position and transferFrom methods
-    // being called once.
-    this.valid = false;
-    return res;
   }
 
   @Override

From b08147da16681e8831c2f6a7b14b5d7a61de2fb3 Mon Sep 17 00:00:00 2001
From: Olav Loite 
Date: Thu, 28 Mar 2019 10:00:48 +0100
Subject: [PATCH 06/11] changed copyright years and removed unnecessary
 formatting changes

---
 .../nio/CloudStorageFileSystemProvider.java   | 23 ++++++++-----------
 .../nio/CloudStorageReadFileChannel.java      |  2 +-
 .../nio/CloudStorageWriteFileChannel.java     |  2 +-
 3 files changed, 12 insertions(+), 15 deletions(-)

diff --git a/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageFileSystemProvider.java b/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageFileSystemProvider.java
index 4a94804ef0ea..f3b626e67bd5 100644
--- a/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageFileSystemProvider.java
+++ b/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageFileSystemProvider.java
@@ -834,8 +834,8 @@ public  A readAttributes(
   @Override
   public Map readAttributes(Path path, String attributes, LinkOption... options) {
     // TODO(#811): Java 7 NIO defines at least eleven string attributes we'd want to support
-    // (eg. BasicFileAttributeView and PosixFileAttributeView), so rather than a partial
-    // implementation we rely on the other overload for now.
+    //             (eg. BasicFileAttributeView and PosixFileAttributeView), so rather than a partial
+    //             implementation we rely on the other overload for now.
     throw new UnsupportedOperationException();
   }
 
@@ -989,18 +989,15 @@ public String getProject() {
    *
    * 
Example of listing buckets, specifying the page size and a name prefix.
    *
-   * 
-   * {
-   *   @code
-   *   String prefix = "bucket_";
-   *   Page buckets = provider.listBuckets(BucketListOption.prefix(prefix));
-   *   Iterator bucketIterator = buckets.iterateAll();
-   *   while (bucketIterator.hasNext()) {
-   *     Bucket bucket = bucketIterator.next();
-   *     // do something with the bucket
-   *   }
+   * 
{@code
+   * String prefix = "bucket_";
+   * Page buckets = provider.listBuckets(BucketListOption.prefix(prefix));
+   * Iterator bucketIterator = buckets.iterateAll();
+   * while (bucketIterator.hasNext()) {
+   *   Bucket bucket = bucketIterator.next();
+   *   // do something with the bucket
    * }
-   * 

+   * }

    *
    * @throws StorageException upon failure
    */
diff --git a/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageReadFileChannel.java b/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageReadFileChannel.java
index ac71cd152584..f7ec7ae7fa2b 100644
--- a/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageReadFileChannel.java
+++ b/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageReadFileChannel.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2016 Google LLC
+ * Copyright 2019 Google LLC
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
diff --git a/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageWriteFileChannel.java b/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageWriteFileChannel.java
index 935fa7de61bc..5dc5f48adb12 100644
--- a/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageWriteFileChannel.java
+++ b/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageWriteFileChannel.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2016 Google LLC
+ * Copyright 2019 Google LLC
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.

From 693f852d06c9f740c22eea2a2be8d1b38cb8fe8a Mon Sep 17 00:00:00 2001
From: Olav Loite 
Date: Mon, 8 Apr 2019 11:05:30 +0200
Subject: [PATCH 07/11] removed unnecessary cast

---
 .../contrib/nio/CloudStorageFileSystemProvider.java        | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageFileSystemProvider.java b/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageFileSystemProvider.java
index f3b626e67bd5..45d6b6608502 100644
--- a/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageFileSystemProvider.java
+++ b/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageFileSystemProvider.java
@@ -337,12 +337,9 @@ public FileChannel newFileChannel(
         || options.contains(StandardOpenOption.CREATE)
         || options.contains(StandardOpenOption.CREATE_NEW)
         || options.contains(StandardOpenOption.TRUNCATE_EXISTING)) {
-      CloudStorageWriteChannel writeChannel =
-          (CloudStorageWriteChannel) newWriteChannel(path, options);
-      return new CloudStorageWriteFileChannel(writeChannel);
+      return new CloudStorageWriteFileChannel(newWriteChannel(path, options));
     } else {
-      CloudStorageReadChannel readChannel = (CloudStorageReadChannel) newReadChannel(path, options);
-      return new CloudStorageReadFileChannel(readChannel);
+      return new CloudStorageReadFileChannel(newReadChannel(path, options));
     }
   }
 

From 0561c8276bcb53f02aeaf63648b733618cf0bf5e Mon Sep 17 00:00:00 2001
From: Olav Loite 
Date: Wed, 10 Apr 2019 16:52:32 +0200
Subject: [PATCH 08/11] make methods synchronized + clearify parameter

---
 .../nio/CloudStorageReadFileChannel.java      | 52 ++++++++-----------
 1 file changed, 23 insertions(+), 29 deletions(-)

diff --git a/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageReadFileChannel.java b/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageReadFileChannel.java
index f7ec7ae7fa2b..c5f0704b3c7a 100644
--- a/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageReadFileChannel.java
+++ b/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageReadFileChannel.java
@@ -40,12 +40,10 @@ public int read(ByteBuffer dst) throws IOException {
   }
 
   @Override
-  public long read(ByteBuffer[] dsts, int offset, int length) throws IOException {
+  public synchronized long read(ByteBuffer[] dsts, int offset, int length) throws IOException {
     long res = 0L;
-    synchronized (this) {
-      for (int i = offset; i < offset + length; i++) {
-        res += readChannel.read(dsts[i]);
-      }
+    for (int i = offset; i < offset + length; i++) {
+      res += readChannel.read(dsts[i]);
     }
     return res;
   }
@@ -87,26 +85,24 @@ public void force(boolean metaData) throws IOException {
   }
 
   @Override
-  public long transferTo(long position, long count, WritableByteChannel target) throws IOException {
+  public synchronized long transferTo(long transferFromPosition, long count, WritableByteChannel target) throws IOException {
     long res = 0L;
-    synchronized (this) {
-      long originalPosition = position();
-      position(position);
-      int blockSize = (int) Math.min(count, 0xfffffL);
-      int bytesRead = 0;
-      ByteBuffer buffer = ByteBuffer.allocate(blockSize);
-      while (res < count && bytesRead >= 0) {
+    long originalPosition = position();
+    position(transferFromPosition);
+    int blockSize = (int) Math.min(count, 0xfffffL);
+    int bytesRead = 0;
+    ByteBuffer buffer = ByteBuffer.allocate(blockSize);
+    while (res < count && bytesRead >= 0) {
+      buffer.position(0);
+      bytesRead = read(buffer);
+      if (bytesRead > 0) {
         buffer.position(0);
-        bytesRead = read(buffer);
-        if (bytesRead > 0) {
-          buffer.position(0);
-          buffer.limit(bytesRead);
-          target.write(buffer);
-          res += bytesRead;
-        }
+        buffer.limit(bytesRead);
+        target.write(buffer);
+        res += bytesRead;
       }
-      position(originalPosition);
     }
+    position(originalPosition);
     return res;
   }
 
@@ -116,14 +112,12 @@ public long transferFrom(ReadableByteChannel src, long position, long count) thr
   }
 
   @Override
-  public int read(ByteBuffer dst, long position) throws IOException {
-    synchronized (this) {
-      long originalPosition = position();
-      position(position);
-      int res = readChannel.read(dst);
-      position(originalPosition);
-      return res;
-    }
+  public synchronized int read(ByteBuffer dst, long readFromPosition) throws IOException {
+    long originalPosition = position();
+    position(readFromPosition);
+    int res = readChannel.read(dst);
+    position(originalPosition);
+    return res;
   }
 
   @Override

From cf7307090c8b4ccce5a2749959b35d83177e22fd Mon Sep 17 00:00:00 2001
From: Olav Loite 
Date: Wed, 10 Apr 2019 16:53:11 +0200
Subject: [PATCH 09/11] make methods synchronized + remove redundant validation

---
 .../nio/CloudStorageWriteFileChannel.java     | 128 ++++++++----------
 1 file changed, 54 insertions(+), 74 deletions(-)

diff --git a/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageWriteFileChannel.java b/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageWriteFileChannel.java
index 5dc5f48adb12..65823655a401 100644
--- a/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageWriteFileChannel.java
+++ b/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageWriteFileChannel.java
@@ -59,59 +59,46 @@ public long read(ByteBuffer[] dsts, int offset, int length) throws IOException {
   }
 
   @Override
-  public int write(ByteBuffer src) throws IOException {
-    synchronized (this) {
-      checkValid();
-      return writeChannel.write(src);
-    }
+  public synchronized int write(ByteBuffer src) throws IOException {
+    checkValid();
+    return writeChannel.write(src);
   }
 
   @Override
-  public long write(ByteBuffer[] srcs, int offset, int length) throws IOException {
-    synchronized (this) {
-      checkValid();
-      long res = 0L;
-      for (int i = offset; i < offset + length; i++) {
-        res += writeChannel.write(srcs[i]);
-      }
-      return res;
+  public synchronized long write(ByteBuffer[] srcs, int offset, int length) throws IOException {
+    checkValid();
+    long res = 0L;
+    for (int i = offset; i < offset + length; i++) {
+      res += writeChannel.write(srcs[i]);
     }
+    return res;
   }
 
   @Override
-  public long position() throws IOException {
-    synchronized (this) {
-      checkValid();
-      return writeChannel.position();
-    }
+  public synchronized long position() throws IOException {
+    checkValid();
+    return writeChannel.position();
   }
 
   @Override
-  public FileChannel position(long newPosition) throws IOException {
-    synchronized (this) {
-      checkValid();
-      if (newPosition != position()) {
-        writeChannel.position(newPosition);
-      }
-      return this;
+  public synchronized FileChannel position(long newPosition) throws IOException {
+    if (newPosition != position()) {
+      writeChannel.position(newPosition);
     }
+    return this;
   }
 
   @Override
-  public long size() throws IOException {
-    synchronized (this) {
-      checkValid();
-      return writeChannel.size();
-    }
+  public synchronized long size() throws IOException {
+    checkValid();
+    return writeChannel.size();
   }
 
   @Override
-  public FileChannel truncate(long size) throws IOException {
-    synchronized (this) {
-      checkValid();
-      writeChannel.truncate(size);
-      return this;
-    }
+  public synchronized FileChannel truncate(long size) throws IOException {
+    checkValid();
+    writeChannel.truncate(size);
+    return this;
   }
 
   @Override
@@ -125,33 +112,30 @@ public long transferTo(long position, long count, WritableByteChannel target) th
   }
 
   @Override
-  public long transferFrom(ReadableByteChannel src, long position, long count) throws IOException {
-    synchronized (this) {
-      checkValid();
-      if (position != position()) {
-        throw new UnsupportedOperationException(
-            "This FileChannel only supports transferFrom at the current position");
-      }
-      int blockSize = (int) Math.min(count, 0xfffffL);
-      long res = 0L;
-      int bytesRead = 0;
-      ByteBuffer buffer = ByteBuffer.allocate(blockSize);
-      while (res < count && bytesRead >= 0) {
+  public synchronized long transferFrom(ReadableByteChannel src, long position, long count) throws IOException {
+    if (position != position()) {
+      throw new UnsupportedOperationException(
+          "This FileChannel only supports transferFrom at the current position");
+    }
+    int blockSize = (int) Math.min(count, 0xfffffL);
+    long res = 0L;
+    int bytesRead = 0;
+    ByteBuffer buffer = ByteBuffer.allocate(blockSize);
+    while (res < count && bytesRead >= 0) {
+      buffer.position(0);
+      bytesRead = src.read(buffer);
+      if (bytesRead > 0) {
         buffer.position(0);
-        bytesRead = src.read(buffer);
-        if (bytesRead > 0) {
-          buffer.position(0);
-          buffer.limit(bytesRead);
-          write(buffer);
-          res += bytesRead;
-        }
+        buffer.limit(bytesRead);
+        write(buffer);
+        res += bytesRead;
       }
-      // The channel is no longer valid as the position has been updated, and there is no way of
-      // resetting it, but this way we at least support the write-at-position and transferFrom
-      // methods being called once.
-      this.valid = false;
-      return res;
     }
+    // The channel is no longer valid as the position has been updated, and there is no way of
+    // resetting it, but this way we at least support the write-at-position and transferFrom
+    // methods being called once.
+    this.valid = false;
+    return res;
   }
 
   @Override
@@ -160,21 +144,17 @@ public int read(ByteBuffer dst, long position) throws IOException {
   }
 
   @Override
-  public int write(ByteBuffer src, long position) throws IOException {
-    synchronized (this) {
-      checkValid();
-      if (position != position()) {
-        throw new UnsupportedOperationException(
-            "This FileChannel only supports write at the current position");
-      }
-      int res = writeChannel.write(src);
-      // The channel is no longer valid as the position has been updated, and there is no way of
-      // resetting it, but this way we at least support the write-at-position and transferFrom
-      // methods
-      // being called once.
-      this.valid = false;
-      return res;
+  public synchronized int write(ByteBuffer src, long position) throws IOException {
+    if (position != position()) {
+      throw new UnsupportedOperationException(
+          "This FileChannel only supports write at the current position");
     }
+    int res = writeChannel.write(src);
+    // The channel is no longer valid as the position has been updated, and there is no way of
+    // resetting it, but this way we at least support the write-at-position and transferFrom
+    // methods being called once.
+    this.valid = false;
+    return res;
   }
 
   @Override

From 8d1a382bc0af79d1b3c4aaa0c8970917c49f67cb Mon Sep 17 00:00:00 2001
From: Olav Loite 
Date: Wed, 10 Apr 2019 16:54:33 +0200
Subject: [PATCH 10/11] fixed formatting

---
 .../cloud/storage/contrib/nio/CloudStorageReadFileChannel.java | 3 ++-
 .../storage/contrib/nio/CloudStorageWriteFileChannel.java      | 3 ++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageReadFileChannel.java b/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageReadFileChannel.java
index c5f0704b3c7a..25a5d4a691cc 100644
--- a/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageReadFileChannel.java
+++ b/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageReadFileChannel.java
@@ -85,7 +85,8 @@ public void force(boolean metaData) throws IOException {
   }
 
   @Override
-  public synchronized long transferTo(long transferFromPosition, long count, WritableByteChannel target) throws IOException {
+  public synchronized long transferTo(
+      long transferFromPosition, long count, WritableByteChannel target) throws IOException {
     long res = 0L;
     long originalPosition = position();
     position(transferFromPosition);
diff --git a/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageWriteFileChannel.java b/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageWriteFileChannel.java
index 65823655a401..3e930f7cd2a8 100644
--- a/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageWriteFileChannel.java
+++ b/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageWriteFileChannel.java
@@ -112,7 +112,8 @@ public long transferTo(long position, long count, WritableByteChannel target) th
   }
 
   @Override
-  public synchronized long transferFrom(ReadableByteChannel src, long position, long count) throws IOException {
+  public synchronized long transferFrom(ReadableByteChannel src, long position, long count)
+      throws IOException {
     if (position != position()) {
       throw new UnsupportedOperationException(
           "This FileChannel only supports transferFrom at the current position");

From 5c2c732ab6146eb8f6c71919c950c0d72b420d52 Mon Sep 17 00:00:00 2001
From: Olav Loite 
Date: Fri, 12 Apr 2019 20:16:32 +0200
Subject: [PATCH 11/11] put position(originalPosition) in finally-block

---
 .../nio/CloudStorageReadFileChannel.java      | 40 +++++++++++--------
 1 file changed, 23 insertions(+), 17 deletions(-)

diff --git a/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageReadFileChannel.java b/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageReadFileChannel.java
index 25a5d4a691cc..db89365b939d 100644
--- a/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageReadFileChannel.java
+++ b/google-cloud-clients/google-cloud-contrib/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageReadFileChannel.java
@@ -89,22 +89,25 @@ public synchronized long transferTo(
       long transferFromPosition, long count, WritableByteChannel target) throws IOException {
     long res = 0L;
     long originalPosition = position();
-    position(transferFromPosition);
-    int blockSize = (int) Math.min(count, 0xfffffL);
-    int bytesRead = 0;
-    ByteBuffer buffer = ByteBuffer.allocate(blockSize);
-    while (res < count && bytesRead >= 0) {
-      buffer.position(0);
-      bytesRead = read(buffer);
-      if (bytesRead > 0) {
+    try {
+      position(transferFromPosition);
+      int blockSize = (int) Math.min(count, 0xfffffL);
+      int bytesRead = 0;
+      ByteBuffer buffer = ByteBuffer.allocate(blockSize);
+      while (res < count && bytesRead >= 0) {
         buffer.position(0);
-        buffer.limit(bytesRead);
-        target.write(buffer);
-        res += bytesRead;
+        bytesRead = read(buffer);
+        if (bytesRead > 0) {
+          buffer.position(0);
+          buffer.limit(bytesRead);
+          target.write(buffer);
+          res += bytesRead;
+        }
       }
+      return res;
+    } finally {
+      position(originalPosition);
     }
-    position(originalPosition);
-    return res;
   }
 
   @Override
@@ -115,10 +118,13 @@ public long transferFrom(ReadableByteChannel src, long position, long count) thr
   @Override
   public synchronized int read(ByteBuffer dst, long readFromPosition) throws IOException {
     long originalPosition = position();
-    position(readFromPosition);
-    int res = readChannel.read(dst);
-    position(originalPosition);
-    return res;
+    try {
+      position(readFromPosition);
+      int res = readChannel.read(dst);
+      return res;
+    } finally {
+      position(originalPosition);
+    }
   }
 
   @Override