use shouldRefreshService instead of createNewService

Author: olavloite

google-cloud-clients/google-cloud-core/src/main/java/com/google/cloud/ServiceOptions.java

@@ -494,20 +494,18 @@ static String getServiceAccountProjectId(String credentialsPath) {
    */
   @SuppressWarnings("unchecked")
   public ServiceT getService() {
-    if (service == null) {
+    if (shouldRefreshService(service)) {
       service = serviceFactory.create((OptionsT) this);
     }
     return service;
   }
 
   /**
-   * Creates a new service object and returns it. Subsequent calls to {@link #getService()} will
-   * return this instance.
+   * @param cachedService The currently cached service object
+   * @return true if the currently cached service object should be refreshed.
    */
-  @SuppressWarnings("unchecked")
-  protected ServiceT createNewService() {
-    service = serviceFactory.create((OptionsT) this);
-    return service;
+  protected boolean shouldRefreshService(ServiceT cachedService) {
+    return cachedService == null;
   }
 
   /**
@@ -516,20 +514,18 @@ protected ServiceT createNewService() {
    */
   @SuppressWarnings("unchecked")
   public ServiceRpc getRpc() {
-    if (rpc == null) {
+    if (shouldRefreshRpc(rpc)) {
       rpc = serviceRpcFactory.create((OptionsT) this);
     }
     return rpc;
   }
 
   /**
-   * Creates a new rpc object and returns it. Subsequent calls to {@link #getRpc()} will return this
-   * instance.
+   * @param cachedService The currently cached service object
+   * @return true if the currently cached service object should be refreshed.
    */
-  @SuppressWarnings("unchecked")
-  protected ServiceRpc createNewRpc() {
-    rpc = serviceRpcFactory.create((OptionsT) this);
-    return rpc;
+  protected boolean shouldRefreshRpc(ServiceRpc cachedRpc) {
+    return cachedRpc == null;
   }
 
   /**

google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java

@@ -295,7 +295,7 @@ public Set<String> getScopes() {
   }
 
   protected SpannerRpc getSpannerRpcV1() {
-    return (SpannerRpc) getRpc();
+    return getRpc();
   }
 
   /**
@@ -306,11 +306,13 @@ protected SpannerRpc getSpannerRpcV1() {
    */
   @Override
   public Spanner getService() {
-    Spanner spanner = super.getService();
-    if (spanner.isClosed()) {
-      spanner = createNewService();
-    }
-    return spanner;
+    // Method is only overridden in order to supply additional documentation.
+    return super.getService();
+  }
+
+  @Override
+  protected boolean shouldRefreshService(Spanner cachedService) {
+    return cachedService == null || cachedService.isClosed();
   }
 
   /**
@@ -320,12 +322,14 @@ public Spanner getService() {
    * ServiceRpc} instance.
    */
   @Override
-  public ServiceRpc getRpc() {
-    SpannerRpc rpc = (SpannerRpc) super.getRpc();
-    if (rpc.isClosed()) {
-      rpc = (SpannerRpc) createNewRpc();
-    }
-    return rpc;
+  public SpannerRpc getRpc() {
+    // Method is only overridden in order to supply additional documentation.
+    return (SpannerRpc) super.getRpc();
+  }
+
+  @Override
+  protected boolean shouldRefreshRpc(ServiceRpc cachedRpc) {
+    return cachedRpc == null || ((SpannerRpc) cachedRpc).isClosed();
   }
 
   @SuppressWarnings("unchecked")

google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerOptionsTest.java

@@ -19,6 +19,7 @@
 import static com.google.common.truth.Truth.assertThat;
 import static org.hamcrest.CoreMatchers.is;
 import static org.junit.Assert.assertThat;
+
 import com.google.cloud.NoCredentials;
 import com.google.cloud.TransportOptions;
 import java.util.HashMap;
@@ -87,10 +88,11 @@ public void testNullSessionLabels() {
 
   @Test
   public void testDoNotCacheClosedSpannerInstance() {
-    SpannerOptions options = SpannerOptions.newBuilder()
-        .setProjectId("[PROJECT]")
-        .setCredentials(NoCredentials.getInstance())
-        .build();
+    SpannerOptions options =
+        SpannerOptions.newBuilder()
+            .setProjectId("[PROJECT]")
+            .setCredentials(NoCredentials.getInstance())
+            .build();
     // Getting a service twice should give the same instance.
     Spanner service1 = options.getService();
     Spanner service2 = options.getService();