From 959945f3504e50f769d189fc1328684bad5cbe0e Mon Sep 17 00:00:00 2001 From: Harshit Mehrotra Date: Tue, 13 Sep 2022 01:18:53 +0000 Subject: [PATCH 1/9] Add database role to SpannerConfig for role-based access control. --- .../sdk/io/gcp/spanner/SpannerAccessor.java | 4 ++ .../sdk/io/gcp/spanner/SpannerConfig.java | 16 +++++++ .../io/gcp/spanner/SpannerAccessorTest.java | 42 +++++++++++++++++++ 3 files changed, 62 insertions(+) diff --git a/sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/spanner/SpannerAccessor.java b/sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/spanner/SpannerAccessor.java index e84324a81455..0b8ededaf7ef 100644 --- a/sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/spanner/SpannerAccessor.java +++ b/sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/spanner/SpannerAccessor.java @@ -185,6 +185,10 @@ private static SpannerAccessor createAndConnect(SpannerConfig spannerConfig) { } String userAgentString = USER_AGENT_PREFIX + "/" + ReleaseInfo.getReleaseInfo().getVersion(); builder.setHeaderProvider(FixedHeaderProvider.create("user-agent", userAgentString)); + ValueProvider databaseRole = spannerConfig.getDatabaseRole(); + if (databaseRole != null && !databaseRole.get().isEmpty()){ + builder.setDatabaseRole(databaseRole.get()); + } SpannerOptions options = builder.build(); Spanner spanner = options.getService(); diff --git a/sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/spanner/SpannerConfig.java b/sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/spanner/SpannerConfig.java index 2d3c15143c9a..2c2ca805b06b 100644 --- a/sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/spanner/SpannerConfig.java +++ b/sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/spanner/SpannerConfig.java @@ -74,6 +74,8 @@ public abstract class SpannerConfig implements Serializable { public abstract @Nullable ValueProvider getRpcPriority(); + public abstract @Nullable ValueProvider getDatabaseRole(); + @VisibleForTesting abstract @Nullable ServiceFactory getServiceFactory(); @@ -146,6 +148,8 @@ abstract Builder setExecuteStreamingSqlRetrySettings( abstract Builder setRpcPriority(ValueProvider rpcPriority); + abstract Builder setDatabaseRole(ValueProvider databaseRole); + public abstract SpannerConfig build(); } @@ -261,4 +265,16 @@ public SpannerConfig withRpcPriority(ValueProvider rpcPriority) { Preconditions.checkNotNull(rpcPriority.get()); return toBuilder().setRpcPriority(rpcPriority).build(); } + + /** Specifies the Cloud Spanner database role. */ + public SpannerConfig withDatabaseRole(ValueProvider databaseRole) { + Preconditions.checkNotNull(databaseRole); + Preconditions.checkNotNull(databaseRole.get()); + return toBuilder().setDatabaseRole(databaseRole).build(); + } + + /** Specifies the Cloud Spanner database role. */ + public SpannerConfig withDatabaseRole(String databaseRole) { + return withDatabaseRole(ValueProvider.StaticValueProvider.of(databaseRole)); + } } diff --git a/sdks/java/io/google-cloud-platform/src/test/java/org/apache/beam/sdk/io/gcp/spanner/SpannerAccessorTest.java b/sdks/java/io/google-cloud-platform/src/test/java/org/apache/beam/sdk/io/gcp/spanner/SpannerAccessorTest.java index 8ce5d681b84d..df38d22f5c13 100644 --- a/sdks/java/io/google-cloud-platform/src/test/java/org/apache/beam/sdk/io/gcp/spanner/SpannerAccessorTest.java +++ b/sdks/java/io/google-cloud-platform/src/test/java/org/apache/beam/sdk/io/gcp/spanner/SpannerAccessorTest.java @@ -99,4 +99,46 @@ public void testRefCountedSpannerAccessorDifferentDbsOnlyOnce() { .getDatabaseClient(eq(DatabaseId.of("project", "test2", "test2"))); verify(serviceFactory.mockSpanner(), times(2)).close(); } + + @Test + public void testCreateWithValidDatabaseRole() { + SpannerConfig config1 = + SpannerConfig.create() + .toBuilder() + .setServiceFactory(serviceFactory) + .setProjectId(StaticValueProvider.of("project")) + .setInstanceId(StaticValueProvider.of("test1")) + .setDatabaseId(StaticValueProvider.of("test1")) + .setDatabaseRole(StaticValueProvider.of("test-role")) + .build(); + + SpannerAccessor acc1 = SpannerAccessor.getOrCreate(config1); + acc1.close(); + + // getDatabaseClient and close() only called once. + verify(serviceFactory.mockSpanner(), times(1)) + .getDatabaseClient(DatabaseId.of("project", "test1", "test1")); + verify(serviceFactory.mockSpanner(), times(1)).close(); + } + + @Test + public void testCreateWithEmptyDatabaseRole() { + SpannerConfig config1 = + SpannerConfig.create() + .toBuilder() + .setServiceFactory(serviceFactory) + .setProjectId(StaticValueProvider.of("project")) + .setInstanceId(StaticValueProvider.of("test1")) + .setDatabaseId(StaticValueProvider.of("test1")) + .setDatabaseRole(StaticValueProvider.of("")) + .build(); + + SpannerAccessor acc1 = SpannerAccessor.getOrCreate(config1); + acc1.close(); + + // getDatabaseClient and close() only called once. + verify(serviceFactory.mockSpanner(), times(1)) + .getDatabaseClient(DatabaseId.of("project", "test1", "test1")); + verify(serviceFactory.mockSpanner(), times(1)).close(); + } } From 3dc560334080e01cb5d46cfef76b837d7c81caa6 Mon Sep 17 00:00:00 2001 From: Harshit Mehrotra Date: Tue, 13 Sep 2022 01:18:53 +0000 Subject: [PATCH 2/9] Add database role to SpannerConfig for role-based access control. --- .../sdk/io/gcp/spanner/SpannerAccessor.java | 4 ++ .../sdk/io/gcp/spanner/SpannerConfig.java | 16 +++++++ .../io/gcp/spanner/SpannerAccessorTest.java | 42 +++++++++++++++++++ 3 files changed, 62 insertions(+) diff --git a/sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/spanner/SpannerAccessor.java b/sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/spanner/SpannerAccessor.java index e84324a81455..0b8ededaf7ef 100644 --- a/sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/spanner/SpannerAccessor.java +++ b/sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/spanner/SpannerAccessor.java @@ -185,6 +185,10 @@ private static SpannerAccessor createAndConnect(SpannerConfig spannerConfig) { } String userAgentString = USER_AGENT_PREFIX + "/" + ReleaseInfo.getReleaseInfo().getVersion(); builder.setHeaderProvider(FixedHeaderProvider.create("user-agent", userAgentString)); + ValueProvider databaseRole = spannerConfig.getDatabaseRole(); + if (databaseRole != null && !databaseRole.get().isEmpty()){ + builder.setDatabaseRole(databaseRole.get()); + } SpannerOptions options = builder.build(); Spanner spanner = options.getService(); diff --git a/sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/spanner/SpannerConfig.java b/sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/spanner/SpannerConfig.java index 05c8c8926d9c..d38187febf4e 100644 --- a/sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/spanner/SpannerConfig.java +++ b/sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/spanner/SpannerConfig.java @@ -73,6 +73,8 @@ public abstract class SpannerConfig implements Serializable { public abstract @Nullable ValueProvider getRpcPriority(); + public abstract @Nullable ValueProvider getDatabaseRole(); + @VisibleForTesting abstract @Nullable ServiceFactory getServiceFactory(); @@ -145,6 +147,8 @@ abstract Builder setExecuteStreamingSqlRetrySettings( abstract Builder setRpcPriority(ValueProvider rpcPriority); + abstract Builder setDatabaseRole(ValueProvider databaseRole); + public abstract SpannerConfig build(); } @@ -256,4 +260,16 @@ public SpannerConfig withRpcPriority(ValueProvider rpcPriority) { checkNotNull(rpcPriority, "withRpcPriority(rpcPriority) called with null input."); return toBuilder().setRpcPriority(rpcPriority).build(); } + + /** Specifies the Cloud Spanner database role. */ + public SpannerConfig withDatabaseRole(ValueProvider databaseRole) { + Preconditions.checkNotNull(databaseRole); + Preconditions.checkNotNull(databaseRole.get()); + return toBuilder().setDatabaseRole(databaseRole).build(); + } + + /** Specifies the Cloud Spanner database role. */ + public SpannerConfig withDatabaseRole(String databaseRole) { + return withDatabaseRole(ValueProvider.StaticValueProvider.of(databaseRole)); + } } diff --git a/sdks/java/io/google-cloud-platform/src/test/java/org/apache/beam/sdk/io/gcp/spanner/SpannerAccessorTest.java b/sdks/java/io/google-cloud-platform/src/test/java/org/apache/beam/sdk/io/gcp/spanner/SpannerAccessorTest.java index 8ce5d681b84d..df38d22f5c13 100644 --- a/sdks/java/io/google-cloud-platform/src/test/java/org/apache/beam/sdk/io/gcp/spanner/SpannerAccessorTest.java +++ b/sdks/java/io/google-cloud-platform/src/test/java/org/apache/beam/sdk/io/gcp/spanner/SpannerAccessorTest.java @@ -99,4 +99,46 @@ public void testRefCountedSpannerAccessorDifferentDbsOnlyOnce() { .getDatabaseClient(eq(DatabaseId.of("project", "test2", "test2"))); verify(serviceFactory.mockSpanner(), times(2)).close(); } + + @Test + public void testCreateWithValidDatabaseRole() { + SpannerConfig config1 = + SpannerConfig.create() + .toBuilder() + .setServiceFactory(serviceFactory) + .setProjectId(StaticValueProvider.of("project")) + .setInstanceId(StaticValueProvider.of("test1")) + .setDatabaseId(StaticValueProvider.of("test1")) + .setDatabaseRole(StaticValueProvider.of("test-role")) + .build(); + + SpannerAccessor acc1 = SpannerAccessor.getOrCreate(config1); + acc1.close(); + + // getDatabaseClient and close() only called once. + verify(serviceFactory.mockSpanner(), times(1)) + .getDatabaseClient(DatabaseId.of("project", "test1", "test1")); + verify(serviceFactory.mockSpanner(), times(1)).close(); + } + + @Test + public void testCreateWithEmptyDatabaseRole() { + SpannerConfig config1 = + SpannerConfig.create() + .toBuilder() + .setServiceFactory(serviceFactory) + .setProjectId(StaticValueProvider.of("project")) + .setInstanceId(StaticValueProvider.of("test1")) + .setDatabaseId(StaticValueProvider.of("test1")) + .setDatabaseRole(StaticValueProvider.of("")) + .build(); + + SpannerAccessor acc1 = SpannerAccessor.getOrCreate(config1); + acc1.close(); + + // getDatabaseClient and close() only called once. + verify(serviceFactory.mockSpanner(), times(1)) + .getDatabaseClient(DatabaseId.of("project", "test1", "test1")); + verify(serviceFactory.mockSpanner(), times(1)).close(); + } } From 94db78d039eb86924fec801f1f6f04fee09037a8 Mon Sep 17 00:00:00 2001 From: Harshit Mehrotra Date: Tue, 27 Sep 2022 02:18:06 +0000 Subject: [PATCH 3/9] Do not use databaseRole.get() on ValueProvider during pipeline creation --- .../java/org/apache/beam/sdk/io/gcp/spanner/SpannerConfig.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/spanner/SpannerConfig.java b/sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/spanner/SpannerConfig.java index d38187febf4e..c40cdf97c969 100644 --- a/sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/spanner/SpannerConfig.java +++ b/sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/spanner/SpannerConfig.java @@ -263,8 +263,7 @@ public SpannerConfig withRpcPriority(ValueProvider rpcPriority) { /** Specifies the Cloud Spanner database role. */ public SpannerConfig withDatabaseRole(ValueProvider databaseRole) { - Preconditions.checkNotNull(databaseRole); - Preconditions.checkNotNull(databaseRole.get()); + checkNotNull(databaseRole, "withDatabaseRole(databaseRole) called with null input."); return toBuilder().setDatabaseRole(databaseRole).build(); } From 9c0e8461f039ff2e868a2fd6d0c83b9066e1267b Mon Sep 17 00:00:00 2001 From: Harshit Mehrotra Date: Tue, 27 Sep 2022 05:05:04 +0000 Subject: [PATCH 4/9] Run spotless to fix formatting issues --- .../org/apache/beam/sdk/io/gcp/spanner/SpannerAccessor.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/spanner/SpannerAccessor.java b/sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/spanner/SpannerAccessor.java index 0b8ededaf7ef..01de03a5f516 100644 --- a/sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/spanner/SpannerAccessor.java +++ b/sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/spanner/SpannerAccessor.java @@ -186,7 +186,7 @@ private static SpannerAccessor createAndConnect(SpannerConfig spannerConfig) { String userAgentString = USER_AGENT_PREFIX + "/" + ReleaseInfo.getReleaseInfo().getVersion(); builder.setHeaderProvider(FixedHeaderProvider.create("user-agent", userAgentString)); ValueProvider databaseRole = spannerConfig.getDatabaseRole(); - if (databaseRole != null && !databaseRole.get().isEmpty()){ + if (databaseRole != null && !databaseRole.get().isEmpty()) { builder.setDatabaseRole(databaseRole.get()); } SpannerOptions options = builder.build(); From b9474e46e4d72dc7c4ae7de094cb5d648b480d84 Mon Sep 17 00:00:00 2001 From: Harshit Mehrotra Date: Tue, 13 Sep 2022 01:18:53 +0000 Subject: [PATCH 5/9] Add database role to SpannerConfig for role-based access control. --- .../sdk/io/gcp/spanner/SpannerAccessor.java | 4 ++ .../sdk/io/gcp/spanner/SpannerConfig.java | 16 +++++++ .../io/gcp/spanner/SpannerAccessorTest.java | 42 +++++++++++++++++++ 3 files changed, 62 insertions(+) diff --git a/sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/spanner/SpannerAccessor.java b/sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/spanner/SpannerAccessor.java index e84324a81455..0b8ededaf7ef 100644 --- a/sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/spanner/SpannerAccessor.java +++ b/sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/spanner/SpannerAccessor.java @@ -185,6 +185,10 @@ private static SpannerAccessor createAndConnect(SpannerConfig spannerConfig) { } String userAgentString = USER_AGENT_PREFIX + "/" + ReleaseInfo.getReleaseInfo().getVersion(); builder.setHeaderProvider(FixedHeaderProvider.create("user-agent", userAgentString)); + ValueProvider databaseRole = spannerConfig.getDatabaseRole(); + if (databaseRole != null && !databaseRole.get().isEmpty()){ + builder.setDatabaseRole(databaseRole.get()); + } SpannerOptions options = builder.build(); Spanner spanner = options.getService(); diff --git a/sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/spanner/SpannerConfig.java b/sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/spanner/SpannerConfig.java index 05c8c8926d9c..d38187febf4e 100644 --- a/sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/spanner/SpannerConfig.java +++ b/sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/spanner/SpannerConfig.java @@ -73,6 +73,8 @@ public abstract class SpannerConfig implements Serializable { public abstract @Nullable ValueProvider getRpcPriority(); + public abstract @Nullable ValueProvider getDatabaseRole(); + @VisibleForTesting abstract @Nullable ServiceFactory getServiceFactory(); @@ -145,6 +147,8 @@ abstract Builder setExecuteStreamingSqlRetrySettings( abstract Builder setRpcPriority(ValueProvider rpcPriority); + abstract Builder setDatabaseRole(ValueProvider databaseRole); + public abstract SpannerConfig build(); } @@ -256,4 +260,16 @@ public SpannerConfig withRpcPriority(ValueProvider rpcPriority) { checkNotNull(rpcPriority, "withRpcPriority(rpcPriority) called with null input."); return toBuilder().setRpcPriority(rpcPriority).build(); } + + /** Specifies the Cloud Spanner database role. */ + public SpannerConfig withDatabaseRole(ValueProvider databaseRole) { + Preconditions.checkNotNull(databaseRole); + Preconditions.checkNotNull(databaseRole.get()); + return toBuilder().setDatabaseRole(databaseRole).build(); + } + + /** Specifies the Cloud Spanner database role. */ + public SpannerConfig withDatabaseRole(String databaseRole) { + return withDatabaseRole(ValueProvider.StaticValueProvider.of(databaseRole)); + } } diff --git a/sdks/java/io/google-cloud-platform/src/test/java/org/apache/beam/sdk/io/gcp/spanner/SpannerAccessorTest.java b/sdks/java/io/google-cloud-platform/src/test/java/org/apache/beam/sdk/io/gcp/spanner/SpannerAccessorTest.java index 8ce5d681b84d..df38d22f5c13 100644 --- a/sdks/java/io/google-cloud-platform/src/test/java/org/apache/beam/sdk/io/gcp/spanner/SpannerAccessorTest.java +++ b/sdks/java/io/google-cloud-platform/src/test/java/org/apache/beam/sdk/io/gcp/spanner/SpannerAccessorTest.java @@ -99,4 +99,46 @@ public void testRefCountedSpannerAccessorDifferentDbsOnlyOnce() { .getDatabaseClient(eq(DatabaseId.of("project", "test2", "test2"))); verify(serviceFactory.mockSpanner(), times(2)).close(); } + + @Test + public void testCreateWithValidDatabaseRole() { + SpannerConfig config1 = + SpannerConfig.create() + .toBuilder() + .setServiceFactory(serviceFactory) + .setProjectId(StaticValueProvider.of("project")) + .setInstanceId(StaticValueProvider.of("test1")) + .setDatabaseId(StaticValueProvider.of("test1")) + .setDatabaseRole(StaticValueProvider.of("test-role")) + .build(); + + SpannerAccessor acc1 = SpannerAccessor.getOrCreate(config1); + acc1.close(); + + // getDatabaseClient and close() only called once. + verify(serviceFactory.mockSpanner(), times(1)) + .getDatabaseClient(DatabaseId.of("project", "test1", "test1")); + verify(serviceFactory.mockSpanner(), times(1)).close(); + } + + @Test + public void testCreateWithEmptyDatabaseRole() { + SpannerConfig config1 = + SpannerConfig.create() + .toBuilder() + .setServiceFactory(serviceFactory) + .setProjectId(StaticValueProvider.of("project")) + .setInstanceId(StaticValueProvider.of("test1")) + .setDatabaseId(StaticValueProvider.of("test1")) + .setDatabaseRole(StaticValueProvider.of("")) + .build(); + + SpannerAccessor acc1 = SpannerAccessor.getOrCreate(config1); + acc1.close(); + + // getDatabaseClient and close() only called once. + verify(serviceFactory.mockSpanner(), times(1)) + .getDatabaseClient(DatabaseId.of("project", "test1", "test1")); + verify(serviceFactory.mockSpanner(), times(1)).close(); + } } From 2610e690e9dd480dcdafd46e8945f55f862099b7 Mon Sep 17 00:00:00 2001 From: Harshit Mehrotra Date: Tue, 27 Sep 2022 02:18:06 +0000 Subject: [PATCH 6/9] Do not use databaseRole.get() on ValueProvider during pipeline creation --- .../java/org/apache/beam/sdk/io/gcp/spanner/SpannerConfig.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/spanner/SpannerConfig.java b/sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/spanner/SpannerConfig.java index d38187febf4e..c40cdf97c969 100644 --- a/sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/spanner/SpannerConfig.java +++ b/sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/spanner/SpannerConfig.java @@ -263,8 +263,7 @@ public SpannerConfig withRpcPriority(ValueProvider rpcPriority) { /** Specifies the Cloud Spanner database role. */ public SpannerConfig withDatabaseRole(ValueProvider databaseRole) { - Preconditions.checkNotNull(databaseRole); - Preconditions.checkNotNull(databaseRole.get()); + checkNotNull(databaseRole, "withDatabaseRole(databaseRole) called with null input."); return toBuilder().setDatabaseRole(databaseRole).build(); } From 38f23be301d59711fc30ab99a63577b4bea0f4be Mon Sep 17 00:00:00 2001 From: Harshit Mehrotra Date: Tue, 13 Sep 2022 01:18:53 +0000 Subject: [PATCH 7/9] Add database role to SpannerConfig for role-based access control. --- .../beam/sdk/io/gcp/spanner/SpannerAccessor.java | 4 ++-- .../beam/sdk/io/gcp/spanner/SpannerConfig.java | 12 +++--------- 2 files changed, 5 insertions(+), 11 deletions(-) diff --git a/sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/spanner/SpannerAccessor.java b/sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/spanner/SpannerAccessor.java index 0b8ededaf7ef..b41c79acc8f3 100644 --- a/sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/spanner/SpannerAccessor.java +++ b/sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/spanner/SpannerAccessor.java @@ -185,9 +185,9 @@ private static SpannerAccessor createAndConnect(SpannerConfig spannerConfig) { } String userAgentString = USER_AGENT_PREFIX + "/" + ReleaseInfo.getReleaseInfo().getVersion(); builder.setHeaderProvider(FixedHeaderProvider.create("user-agent", userAgentString)); - ValueProvider databaseRole = spannerConfig.getDatabaseRole(); + String databaseRole = spannerConfig.getDatabaseRole(); if (databaseRole != null && !databaseRole.get().isEmpty()){ - builder.setDatabaseRole(databaseRole.get()); + builder.setDatabaseRole(databaseRole); } SpannerOptions options = builder.build(); diff --git a/sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/spanner/SpannerConfig.java b/sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/spanner/SpannerConfig.java index c40cdf97c969..c10af8429ece 100644 --- a/sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/spanner/SpannerConfig.java +++ b/sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/spanner/SpannerConfig.java @@ -73,7 +73,7 @@ public abstract class SpannerConfig implements Serializable { public abstract @Nullable ValueProvider getRpcPriority(); - public abstract @Nullable ValueProvider getDatabaseRole(); + public abstract @Nullable String getDatabaseRole(); @VisibleForTesting abstract @Nullable ServiceFactory getServiceFactory(); @@ -147,7 +147,7 @@ abstract Builder setExecuteStreamingSqlRetrySettings( abstract Builder setRpcPriority(ValueProvider rpcPriority); - abstract Builder setDatabaseRole(ValueProvider databaseRole); + abstract Builder setDatabaseRole(String databaseRole); public abstract SpannerConfig build(); } @@ -261,14 +261,8 @@ public SpannerConfig withRpcPriority(ValueProvider rpcPriority) { return toBuilder().setRpcPriority(rpcPriority).build(); } - /** Specifies the Cloud Spanner database role. */ - public SpannerConfig withDatabaseRole(ValueProvider databaseRole) { - checkNotNull(databaseRole, "withDatabaseRole(databaseRole) called with null input."); - return toBuilder().setDatabaseRole(databaseRole).build(); - } - /** Specifies the Cloud Spanner database role. */ public SpannerConfig withDatabaseRole(String databaseRole) { - return withDatabaseRole(ValueProvider.StaticValueProvider.of(databaseRole)); + return toBuilder().setDatabaseRole(databaseRole).build(); } } From 3e1b2737c8b1683a532022a807fd8b3a465ddd60 Mon Sep 17 00:00:00 2001 From: Harshit Mehrotra Date: Tue, 11 Oct 2022 00:47:43 +0000 Subject: [PATCH 8/9] Remove ValueProvider for database roles. --- .../org/apache/beam/sdk/io/gcp/spanner/SpannerAccessor.java | 2 +- .../apache/beam/sdk/io/gcp/spanner/SpannerAccessorTest.java | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/spanner/SpannerAccessor.java b/sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/spanner/SpannerAccessor.java index b41c79acc8f3..548571c47b6b 100644 --- a/sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/spanner/SpannerAccessor.java +++ b/sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/spanner/SpannerAccessor.java @@ -186,7 +186,7 @@ private static SpannerAccessor createAndConnect(SpannerConfig spannerConfig) { String userAgentString = USER_AGENT_PREFIX + "/" + ReleaseInfo.getReleaseInfo().getVersion(); builder.setHeaderProvider(FixedHeaderProvider.create("user-agent", userAgentString)); String databaseRole = spannerConfig.getDatabaseRole(); - if (databaseRole != null && !databaseRole.get().isEmpty()){ + if (databaseRole != null && !databaseRole.isEmpty()){ builder.setDatabaseRole(databaseRole); } SpannerOptions options = builder.build(); diff --git a/sdks/java/io/google-cloud-platform/src/test/java/org/apache/beam/sdk/io/gcp/spanner/SpannerAccessorTest.java b/sdks/java/io/google-cloud-platform/src/test/java/org/apache/beam/sdk/io/gcp/spanner/SpannerAccessorTest.java index df38d22f5c13..ef9f59a2d6e2 100644 --- a/sdks/java/io/google-cloud-platform/src/test/java/org/apache/beam/sdk/io/gcp/spanner/SpannerAccessorTest.java +++ b/sdks/java/io/google-cloud-platform/src/test/java/org/apache/beam/sdk/io/gcp/spanner/SpannerAccessorTest.java @@ -109,7 +109,7 @@ public void testCreateWithValidDatabaseRole() { .setProjectId(StaticValueProvider.of("project")) .setInstanceId(StaticValueProvider.of("test1")) .setDatabaseId(StaticValueProvider.of("test1")) - .setDatabaseRole(StaticValueProvider.of("test-role")) + .setDatabaseRole("test-role") .build(); SpannerAccessor acc1 = SpannerAccessor.getOrCreate(config1); @@ -130,7 +130,7 @@ public void testCreateWithEmptyDatabaseRole() { .setProjectId(StaticValueProvider.of("project")) .setInstanceId(StaticValueProvider.of("test1")) .setDatabaseId(StaticValueProvider.of("test1")) - .setDatabaseRole(StaticValueProvider.of("")) + .setDatabaseRole("") .build(); SpannerAccessor acc1 = SpannerAccessor.getOrCreate(config1); From 6f8960516bb051ef7fda9d0a8337cf79e0f585d0 Mon Sep 17 00:00:00 2001 From: Harshit Mehrotra Date: Tue, 11 Oct 2022 01:56:23 +0000 Subject: [PATCH 9/9] Fix spotless error. --- .../org/apache/beam/sdk/io/gcp/spanner/SpannerAccessor.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/spanner/SpannerAccessor.java b/sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/spanner/SpannerAccessor.java index 548571c47b6b..2a95791f6c80 100644 --- a/sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/spanner/SpannerAccessor.java +++ b/sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/spanner/SpannerAccessor.java @@ -186,7 +186,7 @@ private static SpannerAccessor createAndConnect(SpannerConfig spannerConfig) { String userAgentString = USER_AGENT_PREFIX + "/" + ReleaseInfo.getReleaseInfo().getVersion(); builder.setHeaderProvider(FixedHeaderProvider.create("user-agent", userAgentString)); String databaseRole = spannerConfig.getDatabaseRole(); - if (databaseRole != null && !databaseRole.isEmpty()){ + if (databaseRole != null && !databaseRole.isEmpty()) { builder.setDatabaseRole(databaseRole); } SpannerOptions options = builder.build();