From 9f51b6445f064439379af752372a3490a2fd5087 Mon Sep 17 00:00:00 2001 From: Ankit Agarwal <146331865+ankiaga@users.noreply.github.com> Date: Tue, 10 Oct 2023 16:25:06 +0530 Subject: [PATCH] fix: Noop in case there is no change in autocommit value for setAutocommit() method (#2662) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 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 --- README.md | 6 +- .../spanner/connection/ConnectionImpl.java | 3 + .../connection/ConnectionImplTest.java | 139 ++++++++++++++++++ 3 files changed, 145 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 169541eff0..2c5415c678 100644 --- 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" ``` @@ -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 index 566a93e76b..9135fc44a5 100644 --- 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/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/ConnectionImplTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/ConnectionImplTest.java index 7c5ba508e8..0b54b35225 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/ConnectionImplTest.java +++ b/google-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 =