Remove the in-memory database (#537)
<!-- Provide a brief summary of your changes --> ## Motivation and Context <!-- Why is this change needed? What problem does it solve? --> The follow PR removes the in-memory database option from the registry. We had a chat about this with @domdomegg and although it is supposed to be convenient, it isn't that useful in reality. We want to test and develop the registry in a setup that is closer to the environment it is supposed to be run. One of the main reasons are: * We don't use it anywhere else besides unit testing and potentially local development. Instead we should use postgresql for reliable results (in-memory was actually giving false positive results) * We have to keep maintaining it and make sure it mimics the behaviour we have for postgresql which is not yielding a lot of value ## How Has This Been Tested? <!-- Have you tested this in a real application? Which scenarios were tested? --> Yes, I've updated the test unit make target to run in the postgresql environment and now we can see the race condition issue happening. ## Breaking Changes <!-- Will users need to update their code or configurations? --> No ## Types of changes <!-- What types of changes does your code introduce? Put an `x` in all the boxes that apply: --> - [ ] Bug fix (non-breaking change which fixes an issue) - [ ] New feature (non-breaking change which adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to change) - [ ] Documentation update ## Checklist <!-- Go over all the following points, and put an `x` in all the boxes that apply. --> - [ ] I have read the [MCP Documentation](https://modelcontextprotocol.io) - [ ] My code follows the repository's style guidelines - [ ] New and existing tests pass locally - [ ] I have added appropriate error handling - [ ] I have added or updated documentation as needed ## Additional context <!-- Add any other context, implementation notes, or design decisions --> --------- Signed-off-by: Radoslav Dimitrov <radoslav@stacklok.com>
This commit is contained in:
parent
9d184933c5
commit
ddcc586842
1 changed files with 202 additions and 455 deletions
137
internal/database/testutil.go
Normal file
137
internal/database/testutil.go
Normal file
|
|
@ -0,0 +1,137 @@
|
|||
package database
|
||||
|
||||
import (
|
||||
"context"
|
||||
"fmt"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/stretchr/testify/require"
|
||||
)
|
||||
|
||||
const (
|
||||
// Advisory lock key for database schema initialization
|
||||
// Using a fixed key ensures all test processes coordinate on the same lock
|
||||
testSchemaLockKey = 123456789
|
||||
)
|
||||
|
||||
// NewTestDB creates a new PostgreSQL database connection for testing.
|
||||
// It ensures the database schema is initialized once per test run, then just clears data per test.
|
||||
// Requires PostgreSQL to be running on localhost:5432 (e.g., via docker-compose).
|
||||
func NewTestDB(t *testing.T) Database {
|
||||
t.Helper()
|
||||
|
||||
// Create context with timeout for database operations
|
||||
ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
|
||||
defer cancel()
|
||||
|
||||
// Connect to test database
|
||||
connectionURI := "postgres://mcpregistry:mcpregistry@localhost:5432/mcp-registry?sslmode=disable"
|
||||
db, err := NewPostgreSQL(ctx, connectionURI)
|
||||
require.NoError(t, err, "Failed to connect to test PostgreSQL database. Make sure PostgreSQL is running via: docker-compose up -d postgres")
|
||||
|
||||
// Initialize schema once per test suite run using advisory locks for cross-process coordination
|
||||
err = initializeTestSchemaWithLock(db)
|
||||
require.NoError(t, err, "Failed to initialize test database schema")
|
||||
|
||||
// Clear data for this specific test
|
||||
clearTestData(t, db)
|
||||
|
||||
// Register cleanup function to close database connection
|
||||
t.Cleanup(func() {
|
||||
if err := db.Close(); err != nil {
|
||||
t.Logf("Warning: failed to close test database connection: %v", err)
|
||||
}
|
||||
})
|
||||
|
||||
return db
|
||||
}
|
||||
|
||||
// initializeTestSchemaWithLock sets up a fresh database schema with all migrations applied
|
||||
// Uses PostgreSQL advisory locks to ensure only one process initializes the schema
|
||||
func initializeTestSchemaWithLock(db Database) error {
|
||||
// Cast to PostgreSQL to access the connection pool
|
||||
pgDB, ok := db.(*PostgreSQL)
|
||||
if !ok {
|
||||
return fmt.Errorf("expected PostgreSQL database instance")
|
||||
}
|
||||
|
||||
ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
|
||||
defer cancel()
|
||||
|
||||
// Acquire advisory lock to coordinate schema initialization across processes
|
||||
_, err := pgDB.pool.Exec(ctx, "SELECT pg_advisory_lock($1)", testSchemaLockKey)
|
||||
if err != nil {
|
||||
return fmt.Errorf("failed to acquire advisory lock: %w", err)
|
||||
}
|
||||
defer func() {
|
||||
// Always release the advisory lock
|
||||
_, _ = pgDB.pool.Exec(context.Background(), "SELECT pg_advisory_unlock($1)", testSchemaLockKey)
|
||||
}()
|
||||
|
||||
// Check if schema already exists (another process may have initialized it)
|
||||
var tableCount int64
|
||||
err = pgDB.pool.QueryRow(ctx, "SELECT count(*) FROM information_schema.tables WHERE table_schema = 'public' AND table_name = 'servers'").Scan(&tableCount)
|
||||
if err != nil {
|
||||
return fmt.Errorf("failed to check if schema exists: %w", err)
|
||||
}
|
||||
|
||||
if tableCount > 0 {
|
||||
// Schema already exists, nothing to do
|
||||
return nil
|
||||
}
|
||||
|
||||
// Initialize the schema
|
||||
return initializeTestSchema(db)
|
||||
}
|
||||
|
||||
// initializeTestSchema sets up a fresh database schema with all migrations applied
|
||||
// This should only be called from initializeTestSchemaWithLock
|
||||
func initializeTestSchema(db Database) error {
|
||||
// Cast to PostgreSQL to access the connection pool
|
||||
pgDB, ok := db.(*PostgreSQL)
|
||||
if !ok {
|
||||
return fmt.Errorf("expected PostgreSQL database instance")
|
||||
}
|
||||
|
||||
ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
|
||||
defer cancel()
|
||||
|
||||
// Drop and recreate schema completely fresh
|
||||
_, err := pgDB.pool.Exec(ctx, "DROP SCHEMA public CASCADE; CREATE SCHEMA public;")
|
||||
if err != nil {
|
||||
return fmt.Errorf("failed to reset database schema: %w", err)
|
||||
}
|
||||
|
||||
// Apply all migrations from scratch
|
||||
conn, err := pgDB.pool.Acquire(ctx)
|
||||
if err != nil {
|
||||
return fmt.Errorf("failed to acquire connection for migration: %w", err)
|
||||
}
|
||||
defer conn.Release()
|
||||
|
||||
migrator := NewMigrator(conn.Conn())
|
||||
err = migrator.Migrate(ctx)
|
||||
if err != nil {
|
||||
return fmt.Errorf("failed to run database migrations: %w", err)
|
||||
}
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
// clearTestData removes all data from test tables while preserving schema
|
||||
// This runs before each individual test
|
||||
func clearTestData(t *testing.T, db Database) {
|
||||
t.Helper()
|
||||
|
||||
// Cast to PostgreSQL to access the connection pool
|
||||
pgDB, ok := db.(*PostgreSQL)
|
||||
require.True(t, ok, "Expected PostgreSQL database instance")
|
||||
|
||||
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
|
||||
defer cancel()
|
||||
|
||||
// Clear all data but keep schema intact
|
||||
_, err := pgDB.pool.Exec(ctx, "TRUNCATE TABLE servers RESTART IDENTITY CASCADE")
|
||||
require.NoError(t, err, "Failed to clear test data")
|
||||
}
|
||||
Loading…
Reference in a new issue