Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

experiemntal: add StoreController to allowing extending interface #752

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

mfridman
Copy link
Collaborator

@mfridman mfridman commented Apr 21, 2024

This PR adds a *database.StoreController type to allow us to extend goose behavior without breaking the core Store interface.

We can document any optional methods by having a well-defined type like the *database.StoreController. It becomes much easier to reason about in the code than the type assertions and hidden interfaces. A good example of this in the wild is the ResponseController added to the net/http package in Go.

For a practical example see #461. The request here was to support a more graceful operation for checking whether the version table exists or not.

I've gone ahead and implemented the Postgres-specific version of the TableExists() method. And the nice thing is we can incrementally update the rest of the dialect implementations piecemeal.

But we always fallback to some less optimized version if ErrNotSupported is returned.

//
// If the Store does not implement a method, it will either return a [ErrNotSupported] error or fall
// back to the default behavior.
func NewStoreController(store Store) *StoreController {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The downside with this pattern is external implementations don't have a type assertion to lean on, and so it's really easy to believe a method is implemented, but in reality the method signature doesn't match. You lose that nice compile-time check.

Another pattern could be to have a second interface like StoreExtender that's a superset of Store.

type StoreExtender interface {
	Store
	// TableExists is an optional method that checks if the version table exists in the database. It is
	// recommended to implement this method if the database supports it, as it can be used to optimize
	// certain operations.
	TableExists(ctx context.Context, db DBTxConn) (bool, error)
}

There's pros/cons to both approaches, need to sleep on this a bit and chat with folks to get feedback.

@mfridman mfridman changed the title Add StoreController to allowing extending interface experiemntal: add StoreController to allowing extending interface Apr 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant