fix: Noop in case there is no change in autocommit value for setAutoc…

…ommit() method (#2662) * Fix: Noop in case there is no change in autocommit value for setAutocommit() method * 🦉 Updates from OwlBot post-processor See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md --------- Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
googleapis · Oct 10, 2023 · 9f51b64 · 9f51b64
1 parent efe95b0
commit 9f51b64
Show file tree

Hide file tree

Showing 3 changed files with 145 additions and 3 deletions.
diff --git a/README.md b/README.md
@@ -57,13 +57,13 @@ implementation 'com.google.cloud:google-cloud-spanner'
 If you are using Gradle without BOM, add this to your dependencies:
 
 ```Groovy
-implementation 'com.google.cloud:google-cloud-spanner:6.49.0'
+implementation 'com.google.cloud:google-cloud-spanner:6.50.0'
 ```
 
 If you are using SBT, add this to your dependencies:
 
 ```Scala
-libraryDependencies += "com.google.cloud" % "google-cloud-spanner" % "6.49.0"
+libraryDependencies += "com.google.cloud" % "google-cloud-spanner" % "6.50.0"
 ```
 <!-- {x-version-update-end} -->
 
@@ -432,7 +432,7 @@ Java is a registered trademark of Oracle and/or its affiliates.
 [kokoro-badge-link-5]: http://storage.googleapis.com/cloud-devrel-public/java/badges/java-spanner/java11.html
 [stability-image]: https://img.shields.io/badge/stability-stable-green
 [maven-version-image]: https://img.shields.io/maven-central/v/com.google.cloud/google-cloud-spanner.svg
-[maven-version-link]: https://central.sonatype.com/artifact/com.google.cloud/google-cloud-spanner/6.49.0
+[maven-version-link]: https://central.sonatype.com/artifact/com.google.cloud/google-cloud-spanner/6.50.0
 [authentication]: https://github.com/googleapis/google-cloud-java#authentication
 [auth-scopes]: https://developers.google.com/identity/protocols/oauth2/scopes
 [predefined-iam-roles]: https://cloud.google.com/iam/docs/understanding-roles#predefined_roles

diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ConnectionImpl.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ConnectionImpl.java
@@ -402,6 +402,9 @@ public boolean isClosed() {
   @Override
   public void setAutocommit(boolean autocommit) {
     ConnectionPreconditions.checkState(!isClosed(), CLOSED_ERROR_MSG);
+    if (isAutocommit() == autocommit) {
+      return;
+    }
     ConnectionPreconditions.checkState(!isBatchActive(), "Cannot set autocommit while in a batch");
     ConnectionPreconditions.checkState(
         !isTransactionStarted(), "Cannot set autocommit while a transaction is active");

diff --git a/...e-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/ConnectionImplTest.java b/...e-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/ConnectionImplTest.java
@@ -412,6 +412,145 @@ public void testExecuteSetAutocommitOff() {
     }
   }
 
+  @Test
+  public void testSetAutocommitToTrue_inAutoCommitAndNotInTransaction_noop() {
+    try (ConnectionImpl subject =
+        createConnection(
+            ConnectionOptions.newBuilder()
+                .setCredentials(NoCredentials.getInstance())
+                .setUri(URI)
+                .build())) {
+      assertThat(subject.isAutocommit(), is(true));
+
+      subject.setAutocommit(true);
+
+      assertTrue(subject.isAutocommit());
+    }
+  }
+
+  @Test
+  public void testSetAutocommitToTrue_inAutoCommitAndInTransaction_noop() {
+    try (ConnectionImpl subject =
+        createConnection(
+            ConnectionOptions.newBuilder()
+                .setCredentials(NoCredentials.getInstance())
+                .setUri(URI)
+                .build())) {
+      assertThat(subject.isAutocommit(), is(true));
+      subject.execute(Statement.of("begin transaction"));
+
+      subject.setAutocommit(true);
+
+      assertTrue(subject.isAutocommit());
+    }
+  }
+
+  @Test
+  public void testSetAutocommitToFalse_inAutoCommitAndNotInTransaction_autocommitModeChanged() {
+    try (ConnectionImpl subject =
+        createConnection(
+            ConnectionOptions.newBuilder()
+                .setCredentials(NoCredentials.getInstance())
+                .setUri(URI)
+                .build())) {
+      assertThat(subject.isAutocommit(), is(true));
+
+      subject.setAutocommit(false);
+
+      assertFalse(subject.isAutocommit());
+    }
+  }
+
+  @Test
+  public void testSetAutocommitToFalse_inAutoCommitAndInTransaction_throwsException() {
+    try (ConnectionImpl subject =
+        createConnection(
+            ConnectionOptions.newBuilder()
+                .setCredentials(NoCredentials.getInstance())
+                .setUri(URI)
+                .build())) {
+      assertThat(subject.isAutocommit(), is(true));
+      subject.execute(Statement.of("begin transaction"));
+
+      SpannerException exception =
+          assertThrows(SpannerException.class, () -> subject.setAutocommit(false));
+      assertEquals(ErrorCode.FAILED_PRECONDITION, exception.getErrorCode());
+      assertTrue(
+          exception
+              .getMessage()
+              .contains("Cannot set autocommit while in a temporary transaction"));
+    }
+  }
+
+  @Test
+  public void testSetAutocommitToFalse_notInAutoCommitAndTransactionNotStarted_noop() {
+    try (ConnectionImpl subject =
+        createConnection(
+            ConnectionOptions.newBuilder()
+                .setCredentials(NoCredentials.getInstance())
+                .setUri(URI + ";autocommit=false")
+                .build())) {
+      assertThat(subject.isAutocommit(), is(false));
+
+      subject.setAutocommit(false);
+
+      assertFalse(subject.isAutocommit());
+    }
+  }
+
+  @Test
+  public void testSetAutocommitToFalse_notInAutoCommitAndTransactionStarted_noop() {
+    try (ConnectionImpl subject =
+        createConnection(
+            ConnectionOptions.newBuilder()
+                .setCredentials(NoCredentials.getInstance())
+                .setUri(URI + ";autocommit=false")
+                .build())) {
+      assertThat(subject.isAutocommit(), is(false));
+      subject.executeQuery(Statement.of(SELECT));
+
+      subject.setAutocommit(false);
+
+      assertFalse(subject.isAutocommit());
+    }
+  }
+
+  @Test
+  public void
+      testSetAutocommitToTrue_notInAutoCommitAndTransactionNotStarted_autocommitModeChanged() {
+    try (ConnectionImpl subject =
+        createConnection(
+            ConnectionOptions.newBuilder()
+                .setCredentials(NoCredentials.getInstance())
+                .setUri(URI + ";autocommit=false")
+                .build())) {
+      assertThat(subject.isAutocommit(), is(false));
+
+      subject.setAutocommit(true);
+
+      assertTrue(subject.isAutocommit());
+    }
+  }
+
+  @Test
+  public void testSetAutocommitToTrue_notInAutoCommitAndTransactionStarted_throwsException() {
+    try (ConnectionImpl subject =
+        createConnection(
+            ConnectionOptions.newBuilder()
+                .setCredentials(NoCredentials.getInstance())
+                .setUri(URI + ";autocommit=false")
+                .build())) {
+      assertThat(subject.isAutocommit(), is(false));
+      subject.executeQuery(Statement.of(SELECT));
+
+      SpannerException exception =
+          assertThrows(SpannerException.class, () -> subject.setAutocommit(true));
+      assertEquals(ErrorCode.FAILED_PRECONDITION, exception.getErrorCode());
+      assertTrue(
+          exception.getMessage().contains("Cannot set autocommit while a transaction is active"));
+    }
+  }
+
   @Test
   public void testExecuteGetAutocommit() {
     try (ConnectionImpl subject =