From c6962d4d1851a18bc9406dc55c7c66e53e9748a3 Mon Sep 17 00:00:00 2001 From: nielm Date: Wed, 4 May 2022 10:32:07 +0200 Subject: [PATCH] [BEAM-14405] Fix NPE when ProjectID is not specified in a template execution PR #16547 added handling for null ProjectIDs on command line execution, but did not handle the possibility of null ProjectIDs in template execution. PR #17094 added nullness checks in MonitoringInfoMetricNames, which then triggered NPEs if the ProjectID was not specified during a template execution. --- .../sdk/io/gcp/spanner/BatchSpannerRead.java | 2 ++ .../beam/sdk/io/gcp/spanner/SpannerIO.java | 2 ++ .../sdk/io/gcp/spanner/SpannerIOReadTest.java | 10 ++++++++++ .../sdk/io/gcp/spanner/SpannerIOWriteTest.java | 16 ++++++++++++++++ 4 files changed, 30 insertions(+) diff --git a/sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/spanner/BatchSpannerRead.java b/sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/spanner/BatchSpannerRead.java index 810f7ce8aaae..2244598905b2 100644 --- a/sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/spanner/BatchSpannerRead.java +++ b/sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/spanner/BatchSpannerRead.java @@ -195,6 +195,8 @@ public void setup() throws Exception { spannerAccessor = SpannerAccessor.getOrCreate(config); projectId = this.config.getProjectId() == null + || this.config.getProjectId().get() == null + || this.config.getProjectId().get().isEmpty() ? SpannerOptions.getDefaultProjectId() : this.config.getProjectId().get(); } diff --git a/sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/spanner/SpannerIO.java b/sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/spanner/SpannerIO.java index 2b76bb0db5b6..dee1c8020a75 100644 --- a/sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/spanner/SpannerIO.java +++ b/sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/spanner/SpannerIO.java @@ -1975,6 +1975,8 @@ public void setup() { projectId = this.spannerConfig.getProjectId() == null + || this.spannerConfig.getProjectId().get() == null + || this.spannerConfig.getProjectId().get().isEmpty() ? SpannerOptions.getDefaultProjectId() : this.spannerConfig.getProjectId().get(); } diff --git a/sdks/java/io/google-cloud-platform/src/test/java/org/apache/beam/sdk/io/gcp/spanner/SpannerIOReadTest.java b/sdks/java/io/google-cloud-platform/src/test/java/org/apache/beam/sdk/io/gcp/spanner/SpannerIOReadTest.java index 48f7fed7feee..d6af52232ba3 100644 --- a/sdks/java/io/google-cloud-platform/src/test/java/org/apache/beam/sdk/io/gcp/spanner/SpannerIOReadTest.java +++ b/sdks/java/io/google-cloud-platform/src/test/java/org/apache/beam/sdk/io/gcp/spanner/SpannerIOReadTest.java @@ -208,6 +208,16 @@ public void runReadTestWithDefaultProject() throws Exception { .withServiceFactory(serviceFactory)); } + @Test + public void runReadTestWithNullProject() throws Exception { + runReadTest( + SpannerConfig.create() + .withProjectId((String) null) + .withInstanceId("123") + .withDatabaseId("aaa") + .withServiceFactory(serviceFactory)); + } + private void runReadTest(SpannerConfig spannerConfig) throws Exception { Timestamp timestamp = Timestamp.ofTimeMicroseconds(12345); TimestampBound timestampBound = TimestampBound.ofReadTimestamp(timestamp); diff --git a/sdks/java/io/google-cloud-platform/src/test/java/org/apache/beam/sdk/io/gcp/spanner/SpannerIOWriteTest.java b/sdks/java/io/google-cloud-platform/src/test/java/org/apache/beam/sdk/io/gcp/spanner/SpannerIOWriteTest.java index 0bb7c66e382f..32509d2eb872 100644 --- a/sdks/java/io/google-cloud-platform/src/test/java/org/apache/beam/sdk/io/gcp/spanner/SpannerIOWriteTest.java +++ b/sdks/java/io/google-cloud-platform/src/test/java/org/apache/beam/sdk/io/gcp/spanner/SpannerIOWriteTest.java @@ -323,6 +323,22 @@ public void singleMutationPipelineNoProjectId() throws Exception { verifyBatches(batch(m(2L))); } + @Test + public void singleMutationPipelineNullProjectId() throws Exception { + Mutation mutation = m(2L); + PCollection mutations = pipeline.apply(Create.of(mutation)); + + mutations.apply( + SpannerIO.write() + .withProjectId((String) null) + .withInstanceId("test-instance") + .withDatabaseId("test-database") + .withServiceFactory(serviceFactory)); + pipeline.run(); + + verifyBatches(batch(m(2L))); + } + @Test public void singleMutationGroupPipeline() throws Exception { PCollection mutations =