Skip to content

Commit aa6ad98

Browse files
CopilotJoannaaKL
andcommitted
Implement RepoAccessCache as a singleton pattern
Co-authored-by: JoannaaKL <67866556+JoannaaKL@users.noreply.github.com>
1 parent 7fb4152 commit aa6ad98

3 files changed

Lines changed: 103 additions & 5 deletions

File tree

internal/ghmcp/server.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ func NewMCPServer(cfg MCPServerConfig) (*server.MCPServer, error) {
9090
}
9191
var repoAccessCache *lockdown.RepoAccessCache
9292
if cfg.LockdownMode {
93-
repoAccessCache = lockdown.NewRepoAccessCache(gqlClient, repoAccessOpts...)
93+
repoAccessCache = lockdown.GetInstance(gqlClient, repoAccessOpts...)
9494
}
9595

9696
// When a client send an initialize request, update the user agent to include the client info.

pkg/lockdown/lockdown.go

Lines changed: 47 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"log/slog"
77
"strings"
88
"sync"
9+
"sync/atomic"
910
"time"
1011

1112
"github.com/muesli/cache2go"
@@ -29,6 +30,13 @@ type repoAccessCacheEntry struct {
2930

3031
const defaultRepoAccessTTL = 5 * time.Minute
3132

33+
var (
34+
instance *RepoAccessCache
35+
instanceOnce sync.Once
36+
instanceMu sync.RWMutex
37+
cacheIDCounter atomic.Uint64
38+
)
39+
3240
// RepoAccessOption configures RepoAccessCache at construction time.
3341
type RepoAccessOption func(*RepoAccessCache)
3442

@@ -47,11 +55,46 @@ func WithLogger(logger *slog.Logger) RepoAccessOption {
4755
}
4856
}
4957

50-
// NewRepoAccessCache returns a cache bound to the provided GitHub GraphQL
51-
// client. The cache is safe for concurrent use.
58+
// GetInstance returns the singleton instance of RepoAccessCache.
59+
// It initializes the instance on first call with the provided client and options.
60+
// Subsequent calls ignore the client and options parameters and return the existing instance.
61+
// This is the preferred way to access the cache in production code.
62+
func GetInstance(client *githubv4.Client, opts ...RepoAccessOption) *RepoAccessCache {
63+
instanceOnce.Do(func() {
64+
instance = newRepoAccessCache(client, opts...)
65+
})
66+
return instance
67+
}
68+
69+
// ResetInstance clears the singleton instance. This is primarily for testing purposes.
70+
// It flushes the cache and allows re-initialization with different parameters.
71+
// Note: This should not be called while the instance is in use.
72+
func ResetInstance() {
73+
instanceMu.Lock()
74+
defer instanceMu.Unlock()
75+
if instance != nil {
76+
instance.cache.Flush()
77+
}
78+
instance = nil
79+
instanceOnce = sync.Once{}
80+
}
81+
82+
// NewRepoAccessCache returns a cache bound to the provided GitHub GraphQL client.
83+
// The cache is safe for concurrent use.
84+
//
85+
// For production code, consider using GetInstance() to ensure singleton behavior and
86+
// consistent configuration across the application. NewRepoAccessCache is appropriate
87+
// for testing scenarios where independent cache instances are needed.
5288
func NewRepoAccessCache(client *githubv4.Client, opts ...RepoAccessOption) *RepoAccessCache {
53-
// Use a unique cache name for each instance to avoid sharing state between tests
54-
cacheName := "repo-access-cache"
89+
return newRepoAccessCache(client, opts...)
90+
}
91+
92+
// newRepoAccessCache creates a new cache instance. This is a private helper function
93+
// used by GetInstance.
94+
func newRepoAccessCache(client *githubv4.Client, opts ...RepoAccessOption) *RepoAccessCache {
95+
// Use a unique cache name for each instance to avoid sharing state between instances
96+
cacheID := cacheIDCounter.Add(1)
97+
cacheName := fmt.Sprintf("repo-access-cache-%d", cacheID)
5598
c := &RepoAccessCache{
5699
client: client,
57100
cache: cache2go.Cache(cacheName),

pkg/lockdown/lockdown_test.go

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -145,3 +145,58 @@ func TestRepoAccessCacheSetTTLReschedulesExistingEntry(t *testing.T) {
145145
requireAccess(ctx, t, cache)
146146
require.EqualValues(t, 2, transport.CallCount())
147147
}
148+
149+
func TestGetInstanceReturnsSingleton(t *testing.T) {
150+
// Reset any existing singleton
151+
ResetInstance()
152+
defer ResetInstance() // Clean up after test
153+
154+
gqlClient := githubv4.NewClient(nil)
155+
156+
// Get instance twice, should return the same instance
157+
instance1 := GetInstance(gqlClient)
158+
instance2 := GetInstance(gqlClient)
159+
160+
// Verify they're the same instance (same pointer)
161+
require.Same(t, instance1, instance2, "GetInstance should return the same singleton instance")
162+
163+
// Verify subsequent calls with different options are ignored
164+
instance3 := GetInstance(gqlClient, WithTTL(1*time.Second))
165+
require.Same(t, instance1, instance3, "GetInstance should ignore options on subsequent calls")
166+
require.Equal(t, defaultRepoAccessTTL, instance3.ttl, "TTL should remain unchanged after first initialization")
167+
}
168+
169+
func TestResetInstanceClearsSingleton(t *testing.T) {
170+
// Reset any existing singleton
171+
ResetInstance()
172+
defer ResetInstance() // Clean up after test
173+
174+
gqlClient := githubv4.NewClient(nil)
175+
176+
// Get first instance with default TTL
177+
instance1 := GetInstance(gqlClient)
178+
require.Equal(t, defaultRepoAccessTTL, instance1.ttl)
179+
180+
// Reset the singleton
181+
ResetInstance()
182+
183+
// Get new instance with custom TTL
184+
customTTL := 10 * time.Second
185+
instance2 := GetInstance(gqlClient, WithTTL(customTTL))
186+
require.NotSame(t, instance1, instance2, "After reset, GetInstance should return a new instance")
187+
require.Equal(t, customTTL, instance2.ttl, "New instance should have the custom TTL")
188+
}
189+
190+
func TestNewRepoAccessCacheCreatesIndependentInstances(t *testing.T) {
191+
t.Parallel()
192+
193+
gqlClient := githubv4.NewClient(nil)
194+
195+
// NewRepoAccessCache should create independent instances
196+
cache1 := NewRepoAccessCache(gqlClient, WithTTL(1*time.Second))
197+
cache2 := NewRepoAccessCache(gqlClient, WithTTL(2*time.Second))
198+
199+
require.NotSame(t, cache1, cache2, "NewRepoAccessCache should create different instances")
200+
require.Equal(t, 1*time.Second, cache1.ttl)
201+
require.Equal(t, 2*time.Second, cache2.ttl)
202+
}

0 commit comments

Comments
 (0)