Skip to content

feat: Update Cloud SQL MySQL samples to include more connection methods#7054

Merged
lesv merged 25 commits into
mainfrom
cloudsql-v2-samples
May 2, 2022
Merged

feat: Update Cloud SQL MySQL samples to include more connection methods#7054
lesv merged 25 commits into
mainfrom
cloudsql-v2-samples

Conversation

@shubha-rajan
Copy link
Copy Markdown
Contributor

@shubha-rajan shubha-rajan commented Apr 22, 2022

Please review this PR to bring this sample into compliance with the standards in go/csql-v2-connectivity-samples. This PR only covers MySQL; I'll update the other two samples once this one is reviewed.

  • I have followed Sample Format Guide
  • pom.xml parent set to latest shared-configuration
  • Appropriate changes to README are included in PR
  • API's need to be enabled to test (tell us)
  • Environment Variables need to be set (ask us to set them)
  • Tests pass: mvn clean verify required
  • Lint passes: mvn -P lint checkstyle:check required
  • Static Analysis: mvn -P lint clean compile pmd:cpd-check spotbugs:check advisory only
  • Please merge this PR for me once it is approved.

@snippet-bot
Copy link
Copy Markdown

snippet-bot Bot commented Apr 22, 2022

You are about to delete the following sample browser pages.

You are about to delete the following frozen region tags.

Here is the summary of changes.

You are about to add 12 region tags.
You are about to delete 5 region tags.

This comment is generated by snippet-bot.
If you find problems with this result, please file an issue at:
https://github.com/googleapis/repo-automation-bots/issues.
To update this comment, add snippet-bot:force-run label or use the checkbox below:

  • Refresh this comment

@product-auto-label product-auto-label Bot added the samples Issues that are directly related to samples. label Apr 22, 2022
@shubha-rajan shubha-rajan changed the title (WIP) update Cloud SQL samples (WIP) feat: Update Cloud SQL MySQL samples to include more connection methods Apr 22, 2022
@shubha-rajan shubha-rajan marked this pull request as ready for review April 26, 2022 23:10
@shubha-rajan shubha-rajan requested a review from a team April 26, 2022 23:10
@shubha-rajan shubha-rajan requested review from a team and yoshi-approver as code owners April 26, 2022 23:10
@shubha-rajan shubha-rajan requested review from enocom and kurtisvg April 26, 2022 23:11
@shubha-rajan shubha-rajan changed the title (WIP) feat: Update Cloud SQL MySQL samples to include more connection methods feat: Update Cloud SQL MySQL samples to include more connection methods Apr 26, 2022
@lesv
Copy link
Copy Markdown
Contributor

lesv commented Apr 27, 2022

Are you going to add any tags? (SnippetBot is unhappy)

@shubha-rajan
Copy link
Copy Markdown
Contributor Author

Are you going to add any tags? (SnippetBot is unhappy)

Yes, I'll need to change the tags in our documentation right after merging this.

@enocom
Copy link
Copy Markdown
Member

enocom commented Apr 27, 2022

@shubha-rajan Let's pin the tags in the docs. Once we're done with all the samples, we'll make a pass and unpin everything.

Comment thread cloud-sql/mysql/servlet/.env.yaml Outdated
Comment thread cloud-sql/mysql/servlet/.env.yaml Outdated
Comment thread cloud-sql/mysql/servlet/README.md Outdated
Comment thread cloud-sql/mysql/servlet/README.md
@shubha-rajan shubha-rajan added the snippet-bot:force-run Force snippet-bot runs its logic label Apr 29, 2022
@snippet-bot snippet-bot Bot removed the snippet-bot:force-run Force snippet-bot runs its logic label Apr 29, 2022
Comment on lines +24 to +26
@SuppressFBWarnings(
value = "USBR_UNNECESSARY_STORE_BEFORE_RETURN",
justification = "Necessary for sample region tag.")
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we scrap these SuppressFBWarnings or suppress in the pom somehow?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They're already scrapped. I think you're looking at an outdated version?

Comment on lines +70 to +72
// See the link below for more information on how to configure SSL Certificates for use with
// MySQL Connector/J
//
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

    // See the _link below_ for more information on how to configure SSL Certificates for use with
    // MySQL Connector/J
    // 👀

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added the link

Comment thread cloud-sql/mysql/servlet/src/main/java/com/example/cloudsql/TemplateData.java Outdated
Comment on lines +46 to +71
private void createTable(DataSource pool) throws SQLException {
// Safely attempt to create the table schema.
try (Connection conn = pool.getConnection()) {
String stmt =
"CREATE TABLE IF NOT EXISTS votes ( "
+ "vote_id SERIAL NOT NULL, time_cast timestamp NOT NULL, candidate CHAR(6) NOT NULL,"
+ " PRIMARY KEY (vote_id) );";
try (PreparedStatement createTableStatement = conn.prepareStatement(stmt);) {
createTableStatement.execute();
}
}
}

// Used to validate user input. All user provided data should be validated and sanitized before
// being used something like a SQL query. Returns null if invalid.
@Nullable
private String validateTeam(String input) {
if (input != null) {
input = input.toUpperCase(Locale.ENGLISH);
// Must be either "TABS" or "SPACES"
if (!"TABS".equals(input) && !"SPACES".equals(input)) {
return null;
}
}
return input;
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any way to abstract some of this out so it's shared between the function and the servlet?

shubha-rajan and others added 4 commits April 29, 2022 10:05
…nectorConnectionPoolFactory.java

Co-authored-by: Kurtis Van Gent <31518063+kurtisvg@users.noreply.github.com>
…plateData.java

Co-authored-by: Kurtis Van Gent <31518063+kurtisvg@users.noreply.github.com>
@shubha-rajan shubha-rajan requested a review from kurtisvg April 29, 2022 18:08
private static final String DB_NAME = System.getenv("DB_NAME");

public static DataSource createConnectionPool() {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: no newline?

Comment on lines +59 to +63
// Unix sockets are not natively supported in Java, so it is necessary to use the Cloud SQL JDBC
// Socket Factory to connect.
// Note: For Java users, the Cloud SQL JDBC Socket Factory can provide authenticated connections
// which is usually preferable to using the Cloud SQL Proxy with Unix sockets.
// See https://github.com/GoogleCloudPlatform/cloud-sql-jdbc-socket-factory for details.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Should we s/SocketFactory/Java connector here?

import java.util.Locale;
import javax.annotation.Nullable;

public class InputValidator {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: You could merge InputValidator and DatabaseSetup into a utils class instead of each having it's own file if you wanted


// [END cloud_sql_mysql_servlet_connect_connector]
// Unix sockets are not natively supported in Java, so it is necessary to use the Cloud SQL
// Java Connector to connect.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need another sentence here that explains what enabling the Unix socket does. Something like, "When setting INSTANCE_UNIX_SOCKET, the connector will do X, Y, and Z," where X, Y, and Z are the things it does differently than just using the connector without a Unix socket.

@shubha-rajan shubha-rajan added automerge Merge the pull request once unit tests and other checks pass. and removed automerge Merge the pull request once unit tests and other checks pass. labels May 2, 2022
@shubha-rajan shubha-rajan added the automerge Merge the pull request once unit tests and other checks pass. label May 2, 2022
@gcf-merge-on-green
Copy link
Copy Markdown
Contributor

Merge-on-green attempted to merge your PR for 6 hours, but it was not mergeable because either one of your required status checks failed, one of your required reviews was not approved, or there is a do not merge label. Learn more about your required status checks here: https://help.github.com/en/github/administering-a-repository/enabling-required-status-checks. You can remove and reapply the label to re-run the bot.

@gcf-merge-on-green gcf-merge-on-green Bot removed the automerge Merge the pull request once unit tests and other checks pass. label May 2, 2022
@lesv lesv merged commit f63f258 into main May 2, 2022
@lesv lesv deleted the cloudsql-v2-samples branch May 2, 2022 23:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

samples Issues that are directly related to samples.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants