Database implementation cleanup (#335)
## Motivation and Context The database logic was getting a bit unwieldy and fragile - especially because business logic that should really have lived at the registry service level was being pushed into the persistence level. This PR tightens the interface, simplifies the code there, as well as rips out a bunch of mocks etc. we don't need. ## How Has This Been Tested? Locally everything works ## Breaking Changes The list endpoint was surprisingly always filtering for isLatest only, which seems like a bug? I have removed this filter.
This commit is contained in:
parent
07d353ba92
commit
9d9a02c57f
3 changed files with 669 additions and 1280 deletions
|
|
@ -1,176 +0,0 @@
|
|||
package service
|
||||
|
||||
import (
|
||||
"context"
|
||||
"time"
|
||||
|
||||
"github.com/google/uuid"
|
||||
"github.com/modelcontextprotocol/registry/internal/database"
|
||||
"github.com/modelcontextprotocol/registry/internal/validators"
|
||||
apiv0 "github.com/modelcontextprotocol/registry/pkg/api/v0"
|
||||
"github.com/modelcontextprotocol/registry/pkg/model"
|
||||
)
|
||||
|
||||
// fakeRegistryService implements RegistryService interface with an in-memory database
|
||||
type fakeRegistryService struct {
|
||||
db *database.MemoryDB
|
||||
}
|
||||
|
||||
// NewFakeRegistryService creates a new fake registry service with pre-populated data
|
||||
func NewFakeRegistryService() RegistryService {
|
||||
// Sample registry entries with updated model structure using ServerJSON
|
||||
serverDetails := []*apiv0.ServerJSON{
|
||||
{
|
||||
Name: "bluegreen/mcp-server",
|
||||
Description: "A dummy MCP registry for testing",
|
||||
Repository: model.Repository{
|
||||
URL: "https://github.com/example/mcp-1",
|
||||
Source: "github",
|
||||
ID: "example/mcp-1",
|
||||
},
|
||||
VersionDetail: model.VersionDetail{
|
||||
Version: "1.0.0",
|
||||
},
|
||||
},
|
||||
{
|
||||
Name: "orangepurple/mcp-server",
|
||||
Description: "Another dummy MCP registry for testing",
|
||||
Repository: model.Repository{
|
||||
URL: "https://github.com/example/mcp-2",
|
||||
Source: "github",
|
||||
ID: "example/mcp-2",
|
||||
},
|
||||
VersionDetail: model.VersionDetail{
|
||||
Version: "0.9.0",
|
||||
},
|
||||
},
|
||||
{
|
||||
Name: "greenyellow/mcp-server",
|
||||
Description: "Yet another dummy MCP registry for testing",
|
||||
Repository: model.Repository{
|
||||
URL: "https://github.com/example/mcp-3",
|
||||
Source: "github",
|
||||
ID: "example/mcp-3",
|
||||
},
|
||||
VersionDetail: model.VersionDetail{
|
||||
Version: "0.9.5",
|
||||
},
|
||||
},
|
||||
}
|
||||
|
||||
// Create a new in-memory database using registry metadata IDs
|
||||
serverDetailMap := make(map[string]*apiv0.ServerJSON)
|
||||
for _, entry := range serverDetails {
|
||||
registryID := uuid.New().String() // Generate registry metadata ID
|
||||
serverDetailMap[registryID] = entry
|
||||
}
|
||||
memDB := database.NewMemoryDB(serverDetailMap)
|
||||
return &fakeRegistryService{
|
||||
db: memDB,
|
||||
}
|
||||
}
|
||||
|
||||
// List retrieves servers with extension wrapper format
|
||||
func (s *fakeRegistryService) List(cursor string, limit int) ([]apiv0.ServerJSON, string, error) {
|
||||
// Create a timeout context for the database operation
|
||||
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
|
||||
defer cancel()
|
||||
|
||||
// Use the database's List method to get ServerRecord entries
|
||||
serverRecords, nextCursor, err := s.db.List(ctx, nil, cursor, limit)
|
||||
if err != nil {
|
||||
return nil, "", err
|
||||
}
|
||||
|
||||
// Return ServerRecords directly (they're now the same as ServerResponse)
|
||||
result := make([]apiv0.ServerJSON, len(serverRecords))
|
||||
for i, record := range serverRecords {
|
||||
result[i] = *record
|
||||
}
|
||||
|
||||
return result, nextCursor, nil
|
||||
}
|
||||
|
||||
// GetByID retrieves a specific server by its registry metadata ID in extension wrapper format
|
||||
func (s *fakeRegistryService) GetByID(id string) (*apiv0.ServerJSON, error) {
|
||||
// Create a timeout context for the database operation
|
||||
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
|
||||
defer cancel()
|
||||
|
||||
// Use the database's GetByID method to retrieve the server record
|
||||
serverRecord, err := s.db.GetByID(ctx, id)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
// Return ServerRecord directly (it's now the same as ServerResponse)
|
||||
return serverRecord, nil
|
||||
}
|
||||
|
||||
// Publish publishes a server with separated extensions
|
||||
func (s *fakeRegistryService) Publish(req apiv0.ServerJSON) (*apiv0.ServerJSON, error) {
|
||||
// Create a timeout context for the database operation
|
||||
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
|
||||
defer cancel()
|
||||
|
||||
// Validate the request
|
||||
if err := validators.ValidatePublishRequest(req); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
// Extract publisher extensions from _meta.publisher
|
||||
publisherExtensions := make(map[string]interface{})
|
||||
if req.Meta != nil && req.Meta.Publisher != nil {
|
||||
publisherExtensions = req.Meta.Publisher
|
||||
}
|
||||
|
||||
// Create registry metadata for fake service (always marks as latest)
|
||||
now := time.Now()
|
||||
registryMetadata := apiv0.RegistryExtensions{
|
||||
ID: uuid.New().String(),
|
||||
PublishedAt: now,
|
||||
UpdatedAt: now,
|
||||
IsLatest: true,
|
||||
ReleaseDate: now.Format(time.RFC3339),
|
||||
}
|
||||
|
||||
// Publish to database
|
||||
serverRecord, err := s.db.Publish(ctx, req, publisherExtensions, registryMetadata)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
// Return ServerRecord directly (it's now the same as ServerResponse)
|
||||
return serverRecord, nil
|
||||
}
|
||||
|
||||
// EditServer updates an existing server with new details (admin operation)
|
||||
func (s *fakeRegistryService) EditServer(id string, req apiv0.ServerJSON) (*apiv0.ServerJSON, error) {
|
||||
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
|
||||
defer cancel()
|
||||
|
||||
// Validate the request
|
||||
if err := validators.ValidatePublishRequest(req); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
// Extract publisher extensions from _meta.publisher
|
||||
publisherExtensions := make(map[string]interface{})
|
||||
if req.Meta != nil && req.Meta.Publisher != nil {
|
||||
publisherExtensions = req.Meta.Publisher
|
||||
}
|
||||
|
||||
// Update server in database
|
||||
serverRecord, err := s.db.UpdateServer(ctx, id, req, publisherExtensions)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
// Return ServerRecord directly (it's now the same as ServerResponse)
|
||||
return serverRecord, nil
|
||||
}
|
||||
|
||||
// Close closes the in-memory database connection
|
||||
func (s *fakeRegistryService) Close() error {
|
||||
return s.db.Close()
|
||||
}
|
||||
|
|
@ -37,7 +37,7 @@ func (s *registryServiceImpl) List(cursor string, limit int) ([]apiv0.ServerJSON
|
|||
limit = 30
|
||||
}
|
||||
|
||||
// Use the database's List method with pagination
|
||||
// Use the database's ListServers method with pagination
|
||||
serverRecords, nextCursor, err := s.db.List(ctx, nil, cursor, limit)
|
||||
if err != nil {
|
||||
return nil, "", err
|
||||
|
|
@ -58,7 +58,6 @@ func (s *registryServiceImpl) GetByID(id string) (*apiv0.ServerJSON, error) {
|
|||
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
|
||||
defer cancel()
|
||||
|
||||
// Use the database's GetByID method to retrieve the server record
|
||||
serverRecord, err := s.db.GetByID(ctx, id)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
|
|
@ -87,7 +86,8 @@ func (s *registryServiceImpl) Publish(req apiv0.ServerJSON) (*apiv0.ServerJSON,
|
|||
return nil, err
|
||||
}
|
||||
|
||||
existingServerVersions, _, err := s.db.List(ctx, map[string]any{"name": serverJSON.Name}, "", maxServerVersionsPerServer)
|
||||
filter := &database.ServerFilter{Name: &serverJSON.Name}
|
||||
existingServerVersions, _, err := s.db.List(ctx, filter, "", maxServerVersionsPerServer)
|
||||
if err != nil && !errors.Is(err, database.ErrNotFound) {
|
||||
return nil, err
|
||||
}
|
||||
|
|
@ -121,8 +121,16 @@ func (s *registryServiceImpl) Publish(req apiv0.ServerJSON) (*apiv0.ServerJSON,
|
|||
) > 0
|
||||
}
|
||||
|
||||
// Create registry metadata with service-determined values
|
||||
registryMetadata := apiv0.RegistryExtensions{
|
||||
// Create complete server with metadata
|
||||
server := serverJSON // Copy the input
|
||||
|
||||
// Initialize meta if not present
|
||||
if server.Meta == nil {
|
||||
server.Meta = &apiv0.ServerMeta{}
|
||||
}
|
||||
|
||||
// Set registry metadata
|
||||
server.Meta.IOModelContextProtocolRegistry = &apiv0.RegistryExtensions{
|
||||
ID: uuid.New().String(),
|
||||
PublishedAt: publishTime,
|
||||
UpdatedAt: publishTime,
|
||||
|
|
@ -130,14 +138,8 @@ func (s *registryServiceImpl) Publish(req apiv0.ServerJSON) (*apiv0.ServerJSON,
|
|||
ReleaseDate: publishTime.Format(time.RFC3339),
|
||||
}
|
||||
|
||||
// Extract publisher extensions from _meta.publisher
|
||||
publisherExtensions := make(map[string]interface{})
|
||||
if serverJSON.Meta != nil && serverJSON.Meta.Publisher != nil {
|
||||
publisherExtensions = serverJSON.Meta.Publisher
|
||||
}
|
||||
|
||||
// Publish to database with the registry metadata (still using old format internally)
|
||||
serverRecord, err := s.db.Publish(ctx, serverJSON, publisherExtensions, registryMetadata)
|
||||
// Create server in database
|
||||
serverRecord, err := s.db.CreateServer(ctx, &server)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
|
@ -149,7 +151,10 @@ func (s *registryServiceImpl) Publish(req apiv0.ServerJSON) (*apiv0.ServerJSON,
|
|||
existingLatestID = existingLatest.Meta.IOModelContextProtocolRegistry.ID
|
||||
}
|
||||
if existingLatestID != "" {
|
||||
if err := s.db.UpdateLatestFlag(ctx, existingLatestID, false); err != nil {
|
||||
// Update the existing server to set is_latest = false
|
||||
existingLatest.Meta.IOModelContextProtocolRegistry.IsLatest = false
|
||||
existingLatest.Meta.IOModelContextProtocolRegistry.UpdatedAt = time.Now()
|
||||
if _, err := s.db.UpdateServer(ctx, existingLatestID, existingLatest); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
}
|
||||
|
|
@ -164,9 +169,7 @@ func (s *registryServiceImpl) validateNoDuplicateRemoteURLs(ctx context.Context,
|
|||
// Check each remote URL in the new server for conflicts
|
||||
for _, remote := range serverDetail.Remotes {
|
||||
// Use filter to find servers with this remote URL
|
||||
filter := map[string]any{
|
||||
"remote_url": remote.URL,
|
||||
}
|
||||
filter := &database.ServerFilter{RemoteURL: &remote.URL}
|
||||
|
||||
conflictingServers, _, err := s.db.List(ctx, filter, "", 1000)
|
||||
if err != nil {
|
||||
|
|
@ -187,8 +190,8 @@ func (s *registryServiceImpl) validateNoDuplicateRemoteURLs(ctx context.Context,
|
|||
// getCurrentLatestVersion finds the current latest version from existing server versions
|
||||
func (s *registryServiceImpl) getCurrentLatestVersion(existingServerVersions []*apiv0.ServerJSON) *apiv0.ServerJSON {
|
||||
for _, server := range existingServerVersions {
|
||||
if server.Meta != nil && server.Meta.IOModelContextProtocolRegistry != nil &&
|
||||
server.Meta.IOModelContextProtocolRegistry.IsLatest {
|
||||
if server.Meta != nil && server.Meta.IOModelContextProtocolRegistry != nil &&
|
||||
server.Meta.IOModelContextProtocolRegistry.IsLatest {
|
||||
return server
|
||||
}
|
||||
}
|
||||
|
|
@ -212,14 +215,8 @@ func (s *registryServiceImpl) EditServer(id string, req apiv0.ServerJSON) (*apiv
|
|||
return nil, err
|
||||
}
|
||||
|
||||
// Extract publisher extensions from _meta.publisher
|
||||
publisherExtensions := make(map[string]interface{})
|
||||
if serverJSON.Meta != nil && serverJSON.Meta.Publisher != nil {
|
||||
publisherExtensions = serverJSON.Meta.Publisher
|
||||
}
|
||||
|
||||
// Update server in database
|
||||
serverRecord, err := s.db.UpdateServer(ctx, id, serverJSON, publisherExtensions)
|
||||
serverRecord, err := s.db.UpdateServer(ctx, id, &serverJSON)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
|
@ -227,4 +224,3 @@ func (s *registryServiceImpl) EditServer(id string, req apiv0.ServerJSON) (*apiv
|
|||
// Return the server record directly
|
||||
return serverRecord, nil
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -37,9 +37,16 @@ func TestValidateNoDuplicateRemoteURLs(t *testing.T) {
|
|||
},
|
||||
}
|
||||
|
||||
memDB := database.NewMemoryDB(existingServers)
|
||||
memDB := database.NewMemoryDB()
|
||||
service := NewRegistryServiceWithDB(memDB)
|
||||
|
||||
for _, server := range existingServers {
|
||||
_, err := service.Publish(*server)
|
||||
if err != nil {
|
||||
t.Fatalf("failed to publish server: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
tests := []struct {
|
||||
name string
|
||||
serverDetail apiv0.ServerJSON
|
||||
|
|
|
|||
Loading…
Reference in a new issue