Skip to content

Commit

Permalink
fix: add nio entry to user-agent (#774)
Browse files Browse the repository at this point in the history
Introduce StorageOptionsUtil which provides utilities and default instance access for StorageOptions which include our new user-agent entry.

To all tests which called com.google.cloud.storage.contrib.nio.CloudStorageFileSystemProvider.setStorageOptions in their @before, there is now a corresponding @after which sets the storageOptions back to defaults
  • Loading branch information
BenWhitehead authored Dec 21, 2021
1 parent 63e38eb commit 2d30f78
Show file tree
Hide file tree
Showing 14 changed files with 304 additions and 31 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ public final class CloudStorageFileSystemProvider extends FileSystemProvider {
private final @Nullable String userProject;

// used only when we create a new instance of CloudStorageFileSystemProvider.
private static StorageOptions futureStorageOptions;
private static StorageOptions futureStorageOptions = StorageOptionsUtil.getDefaultInstance();

private static class LazyPathIterator extends AbstractIterator<Path> {
private final Iterator<Blob> blobIterator;
Expand Down Expand Up @@ -198,20 +198,18 @@ public CloudStorageFileSystemProvider() {
*/
CloudStorageFileSystemProvider(
@Nullable String userProject, @Nullable StorageOptions gcsStorageOptions) {
this.storageOptions = gcsStorageOptions;
this.storageOptions =
gcsStorageOptions != null
? StorageOptionsUtil.mergeOptionsWithUserAgent(gcsStorageOptions)
: StorageOptionsUtil.getDefaultInstance();
this.userProject = userProject;
}

// Initialize this.storage, once. This may throw an exception if default authentication
// credentials are not available (hence not doing it in the ctor).
private void initStorage() {
if (this.storage != null) {
return;
}
if (storageOptions == null) {
this.storage = StorageOptions.getDefaultInstance().getService();
} else {
this.storage = storageOptions.getService();
if (this.storage == null) {
doInitStorage();
}
}

Expand Down Expand Up @@ -1037,4 +1035,9 @@ private IOException asIoException(StorageException oops) {
}
return new IOException(oops.getMessage(), oops);
}

@VisibleForTesting
void doInitStorage() {
this.storage = storageOptions.getService();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,119 @@
/*
* Copyright 2021 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 com.google.api.gax.rpc.FixedHeaderProvider;
import com.google.api.gax.rpc.HeaderProvider;
import com.google.cloud.storage.StorageOptions;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.collect.ImmutableMap;
import java.io.IOException;
import java.io.InputStream;
import java.util.Collections;
import java.util.HashMap;
import java.util.Map;
import java.util.Properties;

final class StorageOptionsUtil {
static final String USER_AGENT_ENTRY_NAME = "gcloud-java-nio";
static final String USER_AGENT_ENTRY_VERSION = getVersion();
private static final String USER_AGENT_ENTRY =
String.format("%s/%s", USER_AGENT_ENTRY_NAME, USER_AGENT_ENTRY_VERSION);
private static final FixedHeaderProvider DEFAULT_HEADER_PROVIDER =
FixedHeaderProvider.create("user-agent", USER_AGENT_ENTRY);

private static final StorageOptions DEFAULT_STORAGE_OPTIONS_INSTANCE =
StorageOptions.newBuilder().setHeaderProvider(DEFAULT_HEADER_PROVIDER).build();
private static final FixedHeaderProvider EMTPY_HEADER_PROVIDER =
FixedHeaderProvider.create(Collections.emptyMap());

private StorageOptionsUtil() {}

static StorageOptions getDefaultInstance() {
return DEFAULT_STORAGE_OPTIONS_INSTANCE;
}

static StorageOptions mergeOptionsWithUserAgent(StorageOptions providedStorageOptions) {
if (providedStorageOptions == DEFAULT_STORAGE_OPTIONS_INSTANCE) {
return providedStorageOptions;
}

String userAgent = providedStorageOptions.getUserAgent();
if (userAgent == null) {
return nullSafeSet(providedStorageOptions, DEFAULT_HEADER_PROVIDER);
} else {
if (!userAgent.contains(USER_AGENT_ENTRY_NAME)) {
HeaderProvider providedHeaderProvider = getHeaderProvider(providedStorageOptions);
Map<String, String> newHeaders = new HashMap<>(providedHeaderProvider.getHeaders());
newHeaders.put("user-agent", String.format("%s %s", userAgent, USER_AGENT_ENTRY));
FixedHeaderProvider headerProvider =
FixedHeaderProvider.create(ImmutableMap.copyOf(newHeaders));
return nullSafeSet(providedStorageOptions, headerProvider);
} else {
return providedStorageOptions;
}
}
}

/**
* Due to some complex interactions between init and mocking, it's possible that the builder
* instance returned from {@link StorageOptions#toBuilder()} can be null. This utility method will
* attempt to create the builder and set the new header provider. If however the builder instance
* is null, the orignal options will be returned without setting the header provider.
*
* <p>Since this method is only every called by us trying to add our user-agent entry to the
* headers this makes our attempt effectively a no-op, which is much better than failing customer
* code.
*/
private static StorageOptions nullSafeSet(
StorageOptions storageOptions, HeaderProvider headerProvider) {
StorageOptions.Builder builder = storageOptions.toBuilder();
if (builder == null) {
return storageOptions;
} else {
return builder.setHeaderProvider(headerProvider).build();
}
}

/** Resolve the version of google-cloud-nio for inclusion in request meta-data */
private static String getVersion() {
// attempt to read the library's version from a properties file generated during the build
// this value should be read and cached for later use
String version = "";
try (InputStream inputStream =
CloudStorageFileSystemProvider.class.getResourceAsStream(
"/META-INF/maven/com.google.cloud/google-cloud-nio/pom.properties")) {
if (inputStream != null) {
final Properties properties = new Properties();
properties.load(inputStream);
version = properties.getProperty("version");
}
} catch (IOException e) {
// ignore
}
return version;
}

/**
* {@link com.google.cloud.ServiceOptions} does not specify a getter for the headerProvider, so
* instead merge with an empty provider.
*/
@VisibleForTesting
static HeaderProvider getHeaderProvider(StorageOptions options) {
return options.getMergedHeaderProvider(EMTPY_HEADER_PROVIDER);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import java.nio.file.Path;
import java.nio.file.Paths;
import java.nio.file.attribute.FileTime;
import org.junit.After;
import org.junit.Assert;
import org.junit.Before;
import org.junit.Rule;
Expand All @@ -52,6 +53,11 @@ public void before() {
path = Paths.get(URI.create("gs://red/water"));
}

@After
public void after() {
CloudStorageFileSystemProvider.setStorageOptions(StorageOptionsUtil.getDefaultInstance());
}

@Test
public void testReadAttributes() throws IOException {
Files.write(path, HAPPY, CloudStorageOptions.withCacheControl("potato"));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
import org.junit.After;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
Expand All @@ -53,6 +54,11 @@ public void before() {
dir = Paths.get(URI.create("gs://bucket/randompath/"));
}

@After
public void after() {
CloudStorageFileSystemProvider.setStorageOptions(StorageOptionsUtil.getDefaultInstance());
}

@Test
public void testCacheControl() throws IOException {
Files.write(path, HAPPY, CloudStorageOptions.withCacheControl("potato"));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import static java.nio.file.StandardOpenOption.CREATE_NEW;
import static java.nio.file.StandardOpenOption.TRUNCATE_EXISTING;
import static java.nio.file.StandardOpenOption.WRITE;
import static org.junit.Assert.assertTrue;

import com.google.cloud.storage.contrib.nio.testing.LocalStorageHelper;
import com.google.cloud.testing.junit4.MultipleAttemptsRule;
Expand All @@ -52,6 +53,7 @@
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import org.junit.After;
import org.junit.Assert;
import org.junit.Before;
import org.junit.Rule;
Expand Down Expand Up @@ -99,6 +101,11 @@ public void before() {
CloudStorageFileSystemProvider.setStorageOptions(LocalStorageHelper.getOptions());
}

@After
public void after() {
CloudStorageFileSystemProvider.setStorageOptions(StorageOptionsUtil.getDefaultInstance());
}

@Test
public void testSize() throws Exception {
Path path = Paths.get(URI.create("gs://bucket/wat"));
Expand Down Expand Up @@ -795,6 +802,21 @@ public void testFromSpace() throws Exception {
assertThat(path4.toString()).isEqualTo("/with/a%20percent");
}

@Test
public void testVersion_matchesAcceptablePatterns() {
String acceptableVersionPattern = "|(?:\\d+\\.\\d+\\.\\d+(?:-.*?)?(?:-SNAPSHOT)?)";
String version = StorageOptionsUtil.USER_AGENT_ENTRY_VERSION;
assertTrue(
String.format("the loaded version '%s' did not match the acceptable pattern", version),
version.matches(acceptableVersionPattern));
}

@Test
public void getUserAgentStartsWithCorrectToken() {
assertThat(String.format("gcloud-java-nio/%s", StorageOptionsUtil.USER_AGENT_ENTRY_VERSION))
.startsWith("gcloud-java-nio/");
}

private static CloudStorageConfiguration permitEmptyPathComponents(boolean value) {
return CloudStorageConfiguration.builder().permitEmptyPathComponents(value).build();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@
import java.nio.file.attribute.BasicFileAttributes;
import java.util.ArrayList;
import java.util.List;
import org.junit.After;
import org.junit.Assert;
import org.junit.Before;
import org.junit.Rule;
Expand Down Expand Up @@ -73,6 +74,11 @@ public void before() {
CloudStorageFileSystemProvider.setStorageOptions(LocalStorageHelper.getOptions());
}

@After
public void after() {
CloudStorageFileSystemProvider.setStorageOptions(StorageOptionsUtil.getDefaultInstance());
}

@Test
public void checkDefaultOptions() throws IOException {
// 1. We get the normal default if we don't do anything special.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import com.google.cloud.testing.junit4.MultipleAttemptsRule;
import com.google.common.collect.Lists;
import java.nio.file.Files;
import org.junit.After;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
Expand Down Expand Up @@ -57,6 +58,11 @@ public void before() {
CloudStorageFileSystemProvider.setStorageOptions(mockOptions);
}

@After
public void after() {
CloudStorageFileSystemProvider.setStorageOptions(StorageOptionsUtil.getDefaultInstance());
}

@Test
public void testIsDirectoryNoUserProject() {
CloudStorageFileSystem fs =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,56 +16,41 @@

package com.google.cloud.storage.contrib.nio;

import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;

import com.google.cloud.storage.Storage;
import com.google.cloud.storage.StorageOptions;
import com.google.cloud.testing.junit4.MultipleAttemptsRule;
import java.net.URI;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;
import org.mockito.Spy;
import org.mockito.junit.MockitoJUnitRunner;

/** Unit tests for {@link CloudStorageFileSystemProvider} late initialization. */
@RunWith(JUnit4.class)
@RunWith(MockitoJUnitRunner.class)
public class CloudStorageLateInitializationTest {
@Rule public final MultipleAttemptsRule multipleAttemptsRule = new MultipleAttemptsRule(3);

private StorageOptions mockOptions;

@Before
public void before() {
mockOptions = mock(StorageOptions.class);
Storage mockStorage = mock(Storage.class);
when(mockOptions.getService()).thenReturn(mockStorage);
CloudStorageFileSystemProvider.setStorageOptions(mockOptions);
}
@Spy private final CloudStorageFileSystemProvider provider = new CloudStorageFileSystemProvider();

@Test
public void ctorDoesNotCreateStorage() {
new CloudStorageFileSystemProvider();
verify(mockOptions, never()).getService();
verify(provider, never()).doInitStorage();
}

@Test
public void getPathCreatesStorageOnce() {
CloudStorageFileSystemProvider provider = new CloudStorageFileSystemProvider();
provider.getPath(URI.create("gs://bucket1/wat"));
provider.getPath(URI.create("gs://bucket2/wat"));
verify(mockOptions, times(1)).getService();
verify(provider, times(1)).doInitStorage();
}

@Test
public void getFileSystemCreatesStorageOnce() {
CloudStorageFileSystemProvider provider = new CloudStorageFileSystemProvider();
provider.getFileSystem(URI.create("gs://bucket1"));
provider.getFileSystem(URI.create("gs://bucket2"));
verify(mockOptions, times(1)).getService();
verify(provider, times(1)).doInitStorage();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
import org.junit.After;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
Expand All @@ -44,6 +45,11 @@ public void before() {
CloudStorageFileSystemProvider.setStorageOptions(LocalStorageHelper.getOptions());
}

@After
public void after() {
CloudStorageFileSystemProvider.setStorageOptions(StorageOptionsUtil.getDefaultInstance());
}

@Test
public void testWithoutCaching() throws IOException {
Path path = Paths.get(URI.create("gs://bucket/path"));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import java.nio.file.FileSystems;
import java.nio.file.Path;
import java.nio.file.ProviderMismatchException;
import org.junit.After;
import org.junit.Assert;
import org.junit.Before;
import org.junit.Rule;
Expand All @@ -47,6 +48,11 @@ public void before() {
CloudStorageFileSystemProvider.setStorageOptions(LocalStorageHelper.getOptions());
}

@After
public void after() {
CloudStorageFileSystemProvider.setStorageOptions(StorageOptionsUtil.getDefaultInstance());
}

@Test
public void testCreate_neverRemoveExtraSlashes() throws IOException {
try (CloudStorageFileSystem fs = CloudStorageFileSystem.forBucket("doodle")) {
Expand Down
Loading

0 comments on commit 2d30f78

Please sign in to comment.