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 committed Dec 21, 2021
1 parent 63e38eb commit 2d30f78
Show file tree
Hide file tree
Showing 14 changed files with 304 additions and 31 deletions.
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();
}
}
@@ -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);
}
}
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
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
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
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
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
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();
}
}
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
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

0 comments on commit 2d30f78

Please sign in to comment.