Add BeforeConnect callback to configuration object

[ItalyPaleAle]

Description

This can be used to alter the connection options for each connection, right before it's established.

Our use case is that we want to connect to a MySQL server which supports authentication using a JWT (in the specific case, Azure Database for MySQL using tokens from Azure AD). The token is to be used in place of a password. Because the tokens have a limited lifetime, we cannot create a Config object with a hardcoded password, because connections in the pool can be created later in time and the token may be expired. By using the BeforeConnect callback, instead, we can ensure that the Config object used to open the connection has a fresh password every time.

This design is modeled after the "BeforeConnect" property offered by pgx.

Although our use case is for updating the password, there may be other uses for a BeforeConnect callback. The callback can also return an error to abort the connection.

We plan to use this for Dapr: dapr/components-contrib#2955

You can see how we implemented this for Postgres (using pgx) here: https://github.com/dapr/components-contrib/blob/566c7fd31adbd81740ba66995729a7eadeb3444a/internal/authentication/postgresql/metadata.go#L97-L109

Checklist

• Code compiles correctly
• Created tests which fail without the change (if possible)
• All tests passing
• Extended the README / documentation, if necessary
• Added myself / the copyright holder to the AUTHORS file

Summary by CodeRabbit

• New Features

  • Added Microsoft Corp. to the list of companies in the AUTHORS file.
  • Introduced a pre-connection configuration step, allowing custom logic before database connections are established.
  • Added a new field beforeConnect to the Config struct for invoking functions before connection establishment.

• Tests

  • Added a new test function TestBeforeConnect to verify database connection setup and pre-connection configurations.

[ItalyPaleAle]

@methane Would it be possible to have this PR considered please? Or please, let me know if there's some other process I should follow.

[methane]

wait. I'm very busy and sick.

[ItalyPaleAle]

Sorry to hear that, hope you get better soon 🙁 No worries, we're fine with waiting

[coderabbitai]

Walkthrough

The changes involve enhancing a database connection library to support pre-connection configuration adjustments. A new BeforeConnect callback function has been introduced, allowing custom logic to be executed before a connection is established. This functionality is tested with a new test case ensuring the connection is correctly modified before use. The changes affect the connection process, configuration structure, and testing suite to ensure the new feature works as intended.

Changes

File(s)	Change Summary
connector.go, dsn.go	Introduced BeforeConnect callback in Config and updated Connect method to handle pre-connection configuration changes. Added context package import.
driver_test.go	Added TestBeforeConnect function to test the new BeforeConnect feature and ensure proper database connection handling.

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share

• X
• Mastodon
• Reddit
• LinkedIn

---

Tips

Chat

There are 3 ways to chat with CodeRabbit:

• Review comments: Directly reply to a review comment made by CodeRabbit. Example:
  • I pushed a fix in commit <commit_id>.
  • Generate unit-tests for this file.
  • Open a follow-up GitHub issue for this discussion.
• Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
  • @coderabbitai generate unit tests for this file.
  • @coderabbitai modularize this function.
• PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
  • @coderabbitai generate interesting stats about this repository and render them as a table.
  • @coderabbitai show all the console.log statements in this repository.
  • @coderabbitai read src/utils.ts and generate unit tests.
  • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (invoked as PR comments)

• @coderabbitai pause to pause the reviews on a PR.
• @coderabbitai resume to resume the paused reviews.
• @coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
• @coderabbitai resolve resolve all the CodeRabbit review comments.
• @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (.coderabbit.yaml)

• You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
• The JSON schema for the configuration file is available here.
• If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/coderabbit-overrides.v2.json

CodeRabbit Discord Community

Join our Discord Community to get help, request features, and share feedback.

[coderabbitai]

Review Status

Actionable comments generated: 3

Configuration used: CodeRabbit UI

Commits

Files that changed from the base of the PR and between fc589cb and ef007e0.

Files selected for processing (4)

• AUTHORS (1 hunks)
• connector.go (1 hunks)
• driver_test.go (1 hunks)
• dsn.go (2 hunks)

Additional comments: 8

AUTHORS (1)

• 132-132: The addition of "Microsoft Corp." to the AUTHORS file is consistent with the PR's objective and follows the file's convention for listing organizations.

connector.go (2)

• 66-87: The implementation of the BeforeConnect callback within the Connect method appears to be correct. It checks for the presence of the callback, clones the configuration, and invokes the callback with the cloned configuration. If an error is returned, it is correctly handled by aborting the connection process. The modified configuration is then used to establish the connection.

• 79-86: The cfg variable, which may have been modified by the BeforeConnect callback, is correctly used to initialize the mysqlConn struct. This ensures that any dynamic changes to the configuration are respected when establishing the connection.

driver_test.go (1)

• 1984-2016: The new test function TestBeforeConnect correctly tests the BeforeConnect callback functionality by setting up a connector with the callback, opening a database, and performing a query to ensure the connection is established correctly.

dsn.go (4)

• 13-13: The addition of the context package import is appropriate for the new BeforeConnect callback functionality.

• 56-56: The BeforeConnect field type is correctly defined as func(context.Context, *Config) error, aligning with the PR's objective to allow dynamic configuration before a connection is established.

• 38-56: The integration of the BeforeConnect field into the Config struct is done without altering the existing fields, maintaining the integrity of the struct.

• 56-56: The normalize method does not need to interact with the BeforeConnect field, as it does not have a default value or require validation.

[coveralls]

coverage: 82.055% (+0.1%) from 81.919%
when pulling 6a4e24e on ItalyPaleAle:add-beforeconnect
into 6964272 on go-sql-driver:master.

[methane]

@shogo82148 How do you think this new API?

[shogo82148]

LGTM

[methane]

Although I like this feature, I want to wait adding more fields to Config object.
Please wait for the discussion in #1548

[coderabbitai]

Review Status

Actionable comments generated: 1

Configuration used: CodeRabbit UI

Commits

Files that changed from the base of the PR and between ef007e0 and 6a4e24e.

Files selected for processing (4)

• AUTHORS (1 hunks)
• connector.go (1 hunks)
• driver_test.go (1 hunks)
• dsn.go (3 hunks)

Files skipped from review due to trivial changes (1)

• AUTHORS

Additional comments: 8

connector.go (1)

• 69-77: The implementation of the BeforeConnect callback within the Connect method correctly clones the configuration and checks for errors returned by the callback. However, ensure that the beforeConnect field in the Config struct is correctly exposed (e.g., as BeforeConnect) and accessible from this context, as direct field access is used here.

dsn.go (1)

• 117-123: The addition of the BeforeConnect function using the functional options pattern is well-implemented. It provides a flexible way to set the BeforeConnect callback in the Config struct.

driver_test.go (6)

• 2047-2079: The test TestBeforeConnect correctly sets up a scenario to test the BeforeConnect callback functionality. It ensures that the callback is invoked and modifies the configuration as expected. This test is well-structured and covers the intended functionality effectively.
• 2048-2048: It's good practice to check if the MySQL server is available before proceeding with the test. This prevents the test from failing due to external dependencies not being met.
• 2053-2053: Parsing the DSN (Data Source Name) to create a configuration object is a crucial step in setting up the test environment. This ensures that the test uses the correct database connection settings.
• 2058-2060: The use of the BeforeConnect callback to dynamically set the database name (DBName) in the configuration is a practical example of how to use this feature. It demonstrates the flexibility of modifying connection options immediately before a connection is established.
• 2063-2063: Creating a new connector using the modified configuration object is a key step in testing the BeforeConnect callback. This ensures that the connection will use the updated settings.
• 2071-2078: The test effectively verifies that the connection uses the database name set by the BeforeConnect callback. This is done by executing a query to select the current database and comparing the result with the expected database name.

[coderabbitai]

The beforeConnect field is added to the Config struct to support the BeforeConnect callback functionality. Ensure that this field is correctly exposed and accessible from other parts of the code, such as in connector.go, where it is directly accessed. Consider making it public or providing a getter method if necessary to maintain encapsulation.

[dolmen]

This seems to be an unnecessary solution to a true problem.

Instead the user should have created its own database/sql.Driver and database/sql.Connector that would build a *mysql.Config on the call to Connector's Connect and call mysql.NewConnector() and wrap its Connect.

See https://github.com/dolmen-go/mylogin-driver/blob/master/driver.go as an example of a database/sql.Driver that wraps github.com/go-sql-driver/mysql that implements a different DSN.