Skip to content

Commit b3c3f12

Browse files
dmjbdmjb
authored
Remove factory for RegistryDataProvider (#285)
The code previously had a factory for the RegistryDataProvider interface which had stubbed out logic for returning an implementation of this type backed by a database. In practice, this interface was not useful when we added the DB, and so only the file-based implementation is needed. This PR: 1) Gets rid of the factory. The application wireup code calls the factory method of the file-based implementation directly when wiring up a file-backed instance of the server. 2) Moves the file-based implementation and the interface under the inmemory package to make it clear that it is only relevant for the file-based storage logic. In future, we may refactor the code to remove this interface entirely.
1 parent 0149b0b commit b3c3f12

File tree

12 files changed

+24
-280
lines changed

12 files changed

+24
-280
lines changed

‎internal/app/builder.go‎

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ type registryAppConfig struct {
5757
storageManager sources.StorageManager
5858
statusPersistence status.StatusPersistence
5959
syncManager pkgsync.Manager
60-
registryProvider service.RegistryDataProvider
60+
registryProvider inmemory.RegistryDataProvider
6161

6262
// HTTP server options
6363
address string
@@ -260,7 +260,7 @@ func WithSyncManager(sm pkgsync.Manager) RegistryAppOptions {
260260
}
261261

262262
// WithRegistryProvider allows injecting a custom registry provider (for testing)
263-
func WithRegistryProvider(provider service.RegistryDataProvider) RegistryAppOptions {
263+
func WithRegistryProvider(provider inmemory.RegistryDataProvider) RegistryAppOptions {
264264
return func(cfg *registryAppConfig) error {
265265
cfg.registryProvider = provider
266266
return nil
@@ -358,13 +358,7 @@ func buildServiceComponents(
358358
case config.StorageTypeFile:
359359
// Build registry provider (reads from synced data via StorageManager)
360360
if b.registryProvider == nil {
361-
// StorageManager was already built in buildSyncComponents
362-
factory := service.NewRegistryProviderFactory(b.storageManager)
363-
provider, err := factory.CreateProvider(b.config)
364-
if err != nil {
365-
return nil, fmt.Errorf("failed to create registry provider: %w", err)
366-
}
367-
b.registryProvider = provider
361+
b.registryProvider = inmemory.NewFileRegistryDataProvider(b.storageManager, b.config)
368362
slog.Info("Created registry data provider using storage manager")
369363
}
370364

‎internal/app/builder_test.go‎

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,9 @@ import (
1515
"github.com/stacklok/toolhive-registry-server/internal/config"
1616
"github.com/stacklok/toolhive-registry-server/internal/kubernetes"
1717
"github.com/stacklok/toolhive-registry-server/internal/service"
18-
"github.com/stacklok/toolhive-registry-server/internal/service/mocks"
18+
"github.com/stacklok/toolhive-registry-server/internal/service/inmemory"
19+
"github.com/stacklok/toolhive-registry-server/internal/service/inmemory/mocks"
20+
mocksvc "github.com/stacklok/toolhive-registry-server/internal/service/mocks"
1921
"github.com/stacklok/toolhive-registry-server/internal/sources"
2022
"github.com/stacklok/toolhive-registry-server/internal/status"
2123
pkgsync "github.com/stacklok/toolhive-registry-server/internal/sync"
@@ -313,7 +315,7 @@ func TestWithRegistryProvider(t *testing.T) {
313315
t.Parallel()
314316
cfg := &registryAppConfig{}
315317
// Use nil registry provider for testing - we're just verifying the field is set
316-
var testRegistryProvider service.RegistryDataProvider
318+
var testRegistryProvider inmemory.RegistryDataProvider
317319

318320
opt := WithRegistryProvider(testRegistryProvider)
319321
err := opt(cfg)
@@ -329,7 +331,7 @@ func TestBuildHTTPServer(t *testing.T) {
329331
tests := []struct {
330332
name string
331333
config *registryAppConfig
332-
setupMock func(*mocks.MockRegistryService)
334+
setupMock func(*mocksvc.MockRegistryService)
333335
wantAddr string
334336
wantReadTO time.Duration
335337
wantWriteTO time.Duration
@@ -346,7 +348,7 @@ func TestBuildHTTPServer(t *testing.T) {
346348
writeTimeout: 15 * time.Second,
347349
idleTimeout: 60 * time.Second,
348350
},
349-
setupMock: func(_ *mocks.MockRegistryService) {},
351+
setupMock: func(_ *mocksvc.MockRegistryService) {},
350352
wantAddr: ":8080",
351353
wantReadTO: 10 * time.Second,
352354
wantWriteTO: 15 * time.Second,
@@ -365,7 +367,7 @@ func TestBuildHTTPServer(t *testing.T) {
365367
writeTimeout: 10 * time.Second,
366368
idleTimeout: 30 * time.Second,
367369
},
368-
setupMock: func(_ *mocks.MockRegistryService) {},
370+
setupMock: func(_ *mocksvc.MockRegistryService) {},
369371
wantAddr: ":9090",
370372
wantReadTO: 5 * time.Second,
371373
wantWriteTO: 10 * time.Second,
@@ -382,7 +384,7 @@ func TestBuildHTTPServer(t *testing.T) {
382384
writeTimeout: 30 * time.Second,
383385
idleTimeout: 120 * time.Second,
384386
},
385-
setupMock: func(_ *mocks.MockRegistryService) {},
387+
setupMock: func(_ *mocksvc.MockRegistryService) {},
386388
wantAddr: "127.0.0.1:3000",
387389
wantReadTO: 20 * time.Second,
388390
wantWriteTO: 30 * time.Second,
@@ -397,7 +399,7 @@ func TestBuildHTTPServer(t *testing.T) {
397399
ctrl := gomock.NewController(t)
398400
defer ctrl.Finish()
399401

400-
mockSvc := mocks.NewMockRegistryService(ctrl)
402+
mockSvc := mocksvc.NewMockRegistryService(ctrl)
401403
tt.setupMock(mockSvc)
402404

403405
// Set auth middleware in config for tests
@@ -441,7 +443,7 @@ func TestBuildServiceComponents(t *testing.T) {
441443
*testing.T,
442444
service.RegistryService,
443445
*registryAppConfig,
444-
service.RegistryDataProvider,
446+
inmemory.RegistryDataProvider,
445447
)
446448
}{
447449
{
@@ -463,7 +465,7 @@ func TestBuildServiceComponents(t *testing.T) {
463465
t *testing.T,
464466
_ service.RegistryService,
465467
config *registryAppConfig,
466-
originalProvider service.RegistryDataProvider,
468+
originalProvider inmemory.RegistryDataProvider,
467469
) {
468470
assert.NotNil(
469471
t,
@@ -501,7 +503,7 @@ func TestBuildServiceComponents(t *testing.T) {
501503
t *testing.T,
502504
_ service.RegistryService,
503505
config *registryAppConfig,
504-
originalProvider service.RegistryDataProvider,
506+
originalProvider inmemory.RegistryDataProvider,
505507
) {
506508
assert.Equal(
507509
t,
@@ -534,7 +536,7 @@ func TestBuildServiceComponents(t *testing.T) {
534536
t *testing.T,
535537
_ service.RegistryService,
536538
config *registryAppConfig,
537-
originalProvider service.RegistryDataProvider,
539+
originalProvider inmemory.RegistryDataProvider,
538540
) {
539541
assert.Equal(
540542
t,

internal/service/file_provider.go renamed to internal/service/inmemory/file_provider.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
1-
// Package service provides the business logic for the MCP registry API
2-
package service
1+
// Package inmemory provides the business logic for the MCP registry API
2+
package inmemory
33

44
import (
55
"context"

internal/service/file_provider_test.go renamed to internal/service/inmemory/file_provider_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
package service
1+
package inmemory
22

33
import (
44
"context"

‎internal/service/inmemory/impl.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ import (
2222
// regSvc implements the RegistryService interface
2323
type regSvc struct {
2424
mu sync.RWMutex // Protects registryData, lastFetch
25-
registryProvider service.RegistryDataProvider
25+
registryProvider RegistryDataProvider
2626
config *config.Config // Config for registry validation
2727

2828
// Map of registry name -> registry data
@@ -58,7 +58,7 @@ func WithConfig(cfg *config.Config) Option {
5858
// deploymentProvider can be nil if deployed servers functionality is not needed.
5959
func New(
6060
ctx context.Context,
61-
registryProvider service.RegistryDataProvider,
61+
registryProvider RegistryDataProvider,
6262
opts ...Option,
6363
) (service.RegistryService, error) {
6464
if registryProvider == nil {

‎internal/service/inmemory/impl_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import (
1313

1414
"github.com/stacklok/toolhive-registry-server/internal/config"
1515
"github.com/stacklok/toolhive-registry-server/internal/service"
16-
"github.com/stacklok/toolhive-registry-server/internal/service/mocks"
16+
"github.com/stacklok/toolhive-registry-server/internal/service/inmemory/mocks"
1717
)
1818

1919
func TestValidateManagedRegistry(t *testing.T) {

internal/service/mocks/mock_provider.go renamed to internal/service/inmemory/mocks/mock_provider.go

File renamed without changes.

internal/service/provider.go renamed to internal/service/inmemory/provider.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
1-
// Package service provides the business logic for the MCP registry API
2-
package service
1+
package inmemory
32

43
import (
54
"context"

‎internal/service/inmemory/service_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ import (
1616
"github.com/stacklok/toolhive-registry-server/internal/registry"
1717
"github.com/stacklok/toolhive-registry-server/internal/service"
1818
"github.com/stacklok/toolhive-registry-server/internal/service/inmemory"
19-
"github.com/stacklok/toolhive-registry-server/internal/service/mocks"
19+
"github.com/stacklok/toolhive-registry-server/internal/service/inmemory/mocks"
2020
)
2121

2222
// testManagedConfig creates a config with a managed registry for testing write operations

‎internal/service/mocks/mock_provider_factory.go‎

Lines changed: 0 additions & 57 deletions
This file was deleted.

0 commit comments

Comments
 (0)