[BEAM-13665] Make SpannerIO projectID optional again

Fixes regression introduced by PR #15493 which inadvertently caused an NPE when the projectID was not specified for a Spanner read. Adds unit test for reading both with and without projectID
apache · Jan 18, 2022 · 9dea794 · 9dea794
1 parent b3e9c7d
commit 9dea794
Show file tree

Hide file tree

Showing 2 changed files with 20 additions and 8 deletions.
diff --git a/...gle-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/spanner/BatchSpannerRead.java b/...gle-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/spanner/BatchSpannerRead.java
@@ -172,9 +172,9 @@ public void teardown() throws Exception {
     public void processElement(ProcessContext c) throws Exception {
       ServiceCallMetric serviceCallMetric =
           createServiceCallMetric(
-              this.config.getProjectId().toString(),
-              this.config.getDatabaseId().toString(),
-              this.config.getInstanceId().toString());
+              this.config.getProjectId() == null ? null : this.config.getProjectId().get(),
+              this.config.getDatabaseId().get(),
+              this.config.getInstanceId().get());
       Transaction tx = c.sideInput(txView);
 
       BatchReadOnlyTransaction batchTx =
@@ -193,15 +193,16 @@ public void processElement(ProcessContext c) throws Exception {
     }
 
     private ServiceCallMetric createServiceCallMetric(
-        String projectId, String databaseId, String tableId) {
+        @Nullable String projectId, String databaseId, String tableId) {
       HashMap<String, String> baseLabels = new HashMap<>();
       baseLabels.put(MonitoringInfoConstants.Labels.PTRANSFORM, "");
       baseLabels.put(MonitoringInfoConstants.Labels.SERVICE, "Spanner");
       baseLabels.put(MonitoringInfoConstants.Labels.METHOD, "Read");
       baseLabels.put(
           MonitoringInfoConstants.Labels.RESOURCE,
           GcpResourceIdentifiers.spannerTable(projectId, databaseId, tableId));
-      baseLabels.put(MonitoringInfoConstants.Labels.SPANNER_PROJECT_ID, projectId);
+      baseLabels.put(
+          MonitoringInfoConstants.Labels.SPANNER_PROJECT_ID, (projectId == null ? "" : projectId));
       baseLabels.put(MonitoringInfoConstants.Labels.SPANNER_DATABASE_ID, databaseId);
       baseLabels.put(MonitoringInfoConstants.Labels.SPANNER_INSTANCE_ID, tableId);
       ServiceCallMetric serviceCallMetric =

diff --git a/...le-cloud-platform/src/test/java/org/apache/beam/sdk/io/gcp/spanner/SpannerIOReadTest.java b/...le-cloud-platform/src/test/java/org/apache/beam/sdk/io/gcp/spanner/SpannerIOReadTest.java
@@ -192,12 +192,23 @@ private SpannerConfig getSpannerConfig() {
   }
 
   @Test
-  public void runRead() throws Exception {
+  public void runReadTestWithProjectId() throws Exception {
+    runReadTest(getSpannerConfig());
+  }
+
+  @Test
+  public void runReadTestWithDefaultProject() throws Exception {
+    runReadTest(
+        SpannerConfig.create()
+            .withInstanceId("123")
+            .withDatabaseId("aaa")
+            .withServiceFactory(serviceFactory));
+  }
+
+  private void runReadTest(SpannerConfig spannerConfig) throws Exception {
     Timestamp timestamp = Timestamp.ofTimeMicroseconds(12345);
     TimestampBound timestampBound = TimestampBound.ofReadTimestamp(timestamp);
 
-    SpannerConfig spannerConfig = getSpannerConfig();
-
     PCollection<Struct> one =
         pipeline.apply(
             "read q",