fix(cloud-sql): postgres servlet reported a failed vote as success by rootkiller6788 · Pull Request #10382 · GoogleCloudPlatform/java-docs-samples

rootkiller6788 · 2026-11-12T08:15:56Z

The three Cloud SQL servlet samples (mysql, postgres, sqlserver) have a bit of a trap in doPost: when the vote insert throws SQLException, the catch block sets a 500 and writes the "Unable to successfully cast vote" message, but it never returns. Execution keeps going and hits the setStatus(200) plus the "Vote successfully cast" line at the end of the method. So the status gets overwritten to 200 and the body ends up saying both that it failed and that it succeeded - the vote never made it into the database but the page says it did.

I added the missing return in all three. It's inside the START_EXCLUDE block, so the published code snippets on cloud.google.com don't change.

For tests I didn't want anything that needs a real database, so each module gets a test that mocks a DataSource whose getConnection() throws SQLException, then checks doPost answers 500 and that the body does not contain "Vote successfully cast". I verified they actually catch the bug by removing the return again - they fail with expected: 500 but was: 200. With the fix in place all three pass (Tests run: 1, Failures: 0, Errors: 0).

The catch block writes a 500 and the error text, but without a return the code falls through to setStatus(200) and the "vote successfully cast" message, so a failed insert looked like a success to the user. Added the missing return. The line sits inside the START_EXCLUDE region, so the published snippet is unchanged.

Both have the identical fall-through after the error response.

…vlets Mocks a DataSource that throws SQLException so doPost takes the catch branch, and asserts the status is 500 and the body does not claim the vote was cast. Reverting the return makes these fail with 200.

gemini-code-assist

Code Review

This pull request fixes a bug in the IndexServlet implementations for MySQL, PostgreSQL, and SQL Server where a failed vote could be reported as a success due to a missing return statement in the error-handling block. It also introduces corresponding unit tests to verify this behavior. The review feedback suggests simplifying the newly added tests by removing unused imports and using Mockito's 'verify' method instead of manually tracking the HTTP status code with 'doAnswer' and 'AtomicInteger'.

gemini-code-assist · 2026-11-12T08:17:28Z

+import static org.mockito.ArgumentMatchers.anyInt;
+import static org.mockito.Mockito.doAnswer;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+import java.io.PrintWriter;
+import java.io.StringWriter;
+import java.sql.SQLException;
+import java.util.concurrent.atomic.AtomicInteger;


Remove unused imports (anyInt, doAnswer, AtomicInteger) and statically import verify to simplify the Mockito verification in the test.

Suggested change

import static org.mockito.ArgumentMatchers.anyInt;

import static org.mockito.Mockito.doAnswer;

import static org.mockito.Mockito.mock;

import static org.mockito.Mockito.when;

import java.io.PrintWriter;

import java.io.StringWriter;

import java.sql.SQLException;

import java.util.concurrent.atomic.AtomicInteger;

import static org.mockito.Mockito.mock;

import static org.mockito.Mockito.verify;

import static org.mockito.Mockito.when;

import java.io.PrintWriter;

import java.io.StringWriter;

import java.sql.SQLException;

gemini-code-assist · 2026-11-12T08:17:28Z

+    AtomicInteger status = new AtomicInteger();
+    HttpServletResponse response = mock(HttpServletResponse.class);
+    when(response.getWriter()).thenReturn(writer);
+    doAnswer(
+            invocation -> {
+              status.set(invocation.getArgument(0));
+              return null;
+            })
+        .when(response)
+        .setStatus(anyInt());
+
+    new IndexServlet().doPost(request, response);
+    writer.flush();
+
+    assertThat(status.get()).isEqualTo(500);
+    assertThat(body.toString()).contains("Unable to successfully cast vote");
+    assertThat(body.toString()).doesNotContain("Vote successfully cast");


Simplify the verification of the HTTP status code by using Mockito's verify instead of stubbing setStatus with doAnswer and tracking it with an AtomicInteger. This makes the test cleaner and more idiomatic.

HttpServletResponse response = mock(HttpServletResponse.class); when(response.getWriter()).thenReturn(writer); new IndexServlet().doPost(request, response); writer.flush(); verify(response).setStatus(500); assertThat(body.toString()).contains("Unable to successfully cast vote"); assertThat(body.toString()).doesNotContain("Vote successfully cast");

gemini-code-assist · 2026-11-12T08:17:28Z

+import static org.mockito.ArgumentMatchers.anyInt;
+import static org.mockito.Mockito.doAnswer;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+import java.io.PrintWriter;
+import java.io.StringWriter;
+import java.sql.SQLException;
+import java.util.concurrent.atomic.AtomicInteger;


Remove unused imports (anyInt, doAnswer, AtomicInteger) and statically import verify to simplify the Mockito verification in the test.

Suggested change

import static org.mockito.ArgumentMatchers.anyInt;

import static org.mockito.Mockito.doAnswer;

import static org.mockito.Mockito.mock;

import static org.mockito.Mockito.when;

import java.io.PrintWriter;

import java.io.StringWriter;

import java.sql.SQLException;

import java.util.concurrent.atomic.AtomicInteger;

import static org.mockito.Mockito.mock;

import static org.mockito.Mockito.verify;

import static org.mockito.Mockito.when;

import java.io.PrintWriter;

import java.io.StringWriter;

import java.sql.SQLException;

gemini-code-assist · 2026-11-12T08:17:28Z

+    AtomicInteger status = new AtomicInteger();
+    HttpServletResponse response = mock(HttpServletResponse.class);
+    when(response.getWriter()).thenReturn(writer);
+    doAnswer(
+            invocation -> {
+              status.set(invocation.getArgument(0));
+              return null;
+            })
+        .when(response)
+        .setStatus(anyInt());
+
+    new IndexServlet().doPost(request, response);
+    writer.flush();
+
+    assertThat(status.get()).isEqualTo(500);
+    assertThat(body.toString()).contains("Unable to successfully cast vote");
+    assertThat(body.toString()).doesNotContain("Vote successfully cast");


Simplify the verification of the HTTP status code by using Mockito's verify instead of stubbing setStatus with doAnswer and tracking it with an AtomicInteger. This makes the test cleaner and more idiomatic.

HttpServletResponse response = mock(HttpServletResponse.class); when(response.getWriter()).thenReturn(writer); new IndexServlet().doPost(request, response); writer.flush(); verify(response).setStatus(500); assertThat(body.toString()).contains("Unable to successfully cast vote"); assertThat(body.toString()).doesNotContain("Vote successfully cast");

gemini-code-assist · 2026-11-12T08:17:28Z

+import static org.mockito.ArgumentMatchers.anyInt;
+import static org.mockito.Mockito.doAnswer;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+import java.io.PrintWriter;
+import java.io.StringWriter;
+import java.sql.SQLException;
+import java.util.concurrent.atomic.AtomicInteger;


Remove unused imports (anyInt, doAnswer, AtomicInteger) and statically import verify to simplify the Mockito verification in the test.

Suggested change

import static org.mockito.ArgumentMatchers.anyInt;

import static org.mockito.Mockito.doAnswer;

import static org.mockito.Mockito.mock;

import static org.mockito.Mockito.when;

import java.io.PrintWriter;

import java.io.StringWriter;

import java.sql.SQLException;

import java.util.concurrent.atomic.AtomicInteger;

import static org.mockito.Mockito.mock;

import static org.mockito.Mockito.verify;

import static org.mockito.Mockito.when;

import java.io.PrintWriter;

import java.io.StringWriter;

import java.sql.SQLException;

gemini-code-assist · 2026-11-12T08:17:28Z

+    AtomicInteger status = new AtomicInteger();
+    HttpServletResponse response = mock(HttpServletResponse.class);
+    when(response.getWriter()).thenReturn(writer);
+    doAnswer(
+            invocation -> {
+              status.set(invocation.getArgument(0));
+              return null;
+            })
+        .when(response)
+        .setStatus(anyInt());
+
+    new IndexServlet().doPost(request, response);
+    writer.flush();
+
+    assertThat(status.get()).isEqualTo(500);
+    assertThat(body.toString()).contains("Unable to successfully cast vote");
+    assertThat(body.toString()).doesNotContain("Vote successfully cast");


Simplify the verification of the HTTP status code by using Mockito's verify instead of stubbing setStatus with doAnswer and tracking it with an AtomicInteger. This makes the test cleaner and more idiomatic.

HttpServletResponse response = mock(HttpServletResponse.class); when(response.getWriter()).thenReturn(writer); new IndexServlet().doPost(request, response); writer.flush(); verify(response).setStatus(500); assertThat(body.toString()).contains("Unable to successfully cast vote"); assertThat(body.toString()).doesNotContain("Vote successfully cast");

rootkiller6788 added 3 commits September 22, 2026 23:18

fix(cloud-sql): same missing return in the mysql and sqlserver servlets

02fce1e

Both have the identical fall-through after the error response.

test(cloud-sql): cover the failed-vote response path in the three ser…

445978c

…vlets Mocks a DataSource that throws SQLException so doPost takes the catch branch, and asserts the status is 500 and the body does not claim the vote was cast. Reverting the return makes these fail with 200.

product-auto-label Bot added api: cloudsql Issues related to the Cloud SQL for MySQL API. samples Issues that are directly related to samples. labels Nov 12, 2026

gemini-code-assist Bot reviewed Nov 12, 2026

View reviewed changes

Navigation Menu

Repository navigation

fix(cloud-sql): postgres servlet reported a failed vote as success - #10382

fix(cloud-sql): postgres servlet reported a failed vote as success#10382
rootkiller6788 wants to merge 3 commits into
GoogleCloudPlatform:mainfrom
rootkiller6788:fix-cloud-sql-vote-error-response

rootkiller6788 commented Nov 12, 2026

Uh oh!

gemini-code-assist Bot left a comment

Uh oh!

gemini-code-assist Bot Nov 12, 2026

Uh oh!

gemini-code-assist Bot Nov 12, 2026

Uh oh!

gemini-code-assist Bot Nov 12, 2026

Uh oh!

gemini-code-assist Bot Nov 12, 2026

Uh oh!

gemini-code-assist Bot Nov 12, 2026

Uh oh!

gemini-code-assist Bot Nov 12, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

rootkiller6788 commented Nov 12, 2026

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot Nov 12, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Nov 12, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Nov 12, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Nov 12, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Nov 12, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Nov 12, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant