identify.NewCacheDirectory, apidir.NewAPIDirectory, identity.NewMockDirectory return ptr values (#1318)

bnewbold · web-flow · commit a86f3ae1f8ec · 2026-02-03T15:53:05.000-08:00
Updates #1313 This PR addresses the first three items recorded in #1313. `identify.NewCacheDirectory`, `apidir.NewAPIDirectory`, and `identity.NewMockDirectory` now return a reference to their respective values. In all cases this is a quality of life improvement, to implement the `identify.Directory` and `identity.Resolver` interfaces, a pointer receiver is required, forcing the caller to store, then reference the temporary value. In the case of `identify.NewCacheDirectory` and `identity.NewMockDirectory`, returning by value was a correctness issue as those types contain fields which should not be copied.
diff --git a/atproto/atclient/password_auth_test.go b/atproto/atclient/password_auth_test.go
@@ -131,7 +131,7 @@ func TestPasswordAuth(t *testing.T) {
 
 	{
 		// simple GET requests, with token expire/retry
-		c, err := LoginWithPassword(ctx, &dir, syntax.Handle("user1.example.com").AtIdentifier(), "password1", "", nil)
+		c, err := LoginWithPassword(ctx, dir, syntax.Handle("user1.example.com").AtIdentifier(), "password1", "", nil)
 		require.NoError(err)
 		err = c.Get(ctx, syntax.NSID("com.example.get"), nil, nil)
 		assert.NoError(err)
@@ -166,7 +166,7 @@ func TestPasswordAuth(t *testing.T) {
 
 	{
 		// logout
-		c, err := LoginWithPassword(ctx, &dir, syntax.Handle("user1.example.com").AtIdentifier(), "password1", "", nil)
+		c, err := LoginWithPassword(ctx, dir, syntax.Handle("user1.example.com").AtIdentifier(), "password1", "", nil)
 		require.NoError(err)
 
 		passAuth, ok := c.Auth.(*PasswordAuth)
@@ -177,7 +177,7 @@ func TestPasswordAuth(t *testing.T) {
 
 	{
 		// simple POST request, with token expire/retry
-		c, err := LoginWithPassword(ctx, &dir, syntax.Handle("user1.example.com").AtIdentifier(), "password1", "", nil)
+		c, err := LoginWithPassword(ctx, dir, syntax.Handle("user1.example.com").AtIdentifier(), "password1", "", nil)
 		require.NoError(err)
 		body := map[string]any{
 			"a": 123,
@@ -192,7 +192,7 @@ func TestPasswordAuth(t *testing.T) {
 
 	{
 		// POST with bytes.Buffer body
-		c, err := LoginWithPassword(ctx, &dir, syntax.Handle("user1.example.com").AtIdentifier(), "password1", "", nil)
+		c, err := LoginWithPassword(ctx, dir, syntax.Handle("user1.example.com").AtIdentifier(), "password1", "", nil)
 		require.NoError(err)
 		body := bytes.NewBufferString("some text")
 		req := NewAPIRequest(MethodProcedure, syntax.NSID("com.example.expire"), body)
@@ -204,7 +204,7 @@ func TestPasswordAuth(t *testing.T) {
 
 	{
 		// POST with file on disk (can seek and retry)
-		c, err := LoginWithPassword(ctx, &dir, syntax.Handle("user1.example.com").AtIdentifier(), "password1", "", nil)
+		c, err := LoginWithPassword(ctx, dir, syntax.Handle("user1.example.com").AtIdentifier(), "password1", "", nil)
 		require.NoError(err)
 		f, err := os.Open("testdata/body.json")
 		require.NoError(err)
@@ -217,7 +217,7 @@ func TestPasswordAuth(t *testing.T) {
 
 	{
 		// POST with pipe reader (can *not* retry)
-		c, err := LoginWithPassword(ctx, &dir, syntax.Handle("user1.example.com").AtIdentifier(), "password1", "", nil)
+		c, err := LoginWithPassword(ctx, dir, syntax.Handle("user1.example.com").AtIdentifier(), "password1", "", nil)
 		require.NoError(err)
 		r1, w1 := io.Pipe()
 		go func() {
diff --git a/atproto/auth/http_test.go b/atproto/auth/http_test.go
@@ -96,7 +96,7 @@ func TestServiceAuthMiddleware(t *testing.T) {
 
 	v := ServiceAuthValidator{
 		Audience: aud,
-		Dir:      &dir,
+		Dir:      dir,
 	}
 
 	{
diff --git a/atproto/auth/jwt_test.go b/atproto/auth/jwt_test.go
@@ -110,7 +110,7 @@ func testSigningValidation(t *testing.T, priv atcrypto.PrivateKey) {
 
 	v := ServiceAuthValidator{
 		Audience: aud,
-		Dir:      &dir,
+		Dir:      dir,
 	}
 
 	t1, err := SignServiceAuth(iss, aud, time.Minute, nil, priv)
diff --git a/atproto/identity/apidir/apidir.go b/atproto/identity/apidir/apidir.go
@@ -40,8 +40,8 @@ type handleBody struct {
 	DID syntax.DID `json:"did"`
 }
 
-func NewAPIDirectory(host string) APIDirectory {
-	return APIDirectory{
+func NewAPIDirectory(host string) *APIDirectory {
+	return &APIDirectory{
 		Client: &http.Client{
 			Timeout: time.Second * 10,
 			Transport: &http.Transport{
diff --git a/atproto/identity/cache_directory.go b/atproto/identity/cache_directory.go
@@ -37,8 +37,8 @@ type identityEntry struct {
 var _ Directory = (*CacheDirectory)(nil)
 
 // Capacity of zero means unlimited size. Similarly, ttl of zero means unlimited duration.
-func NewCacheDirectory(inner Directory, capacity int, hitTTL, errTTL, invalidHandleTTL time.Duration) CacheDirectory {
-	return CacheDirectory{
+func NewCacheDirectory(inner Directory, capacity int, hitTTL, errTTL, invalidHandleTTL time.Duration) *CacheDirectory {
+	return &CacheDirectory{
 		ErrTTL:           errTTL,
 		InvalidHandleTTL: invalidHandleTTL,
 		Inner:            inner,
diff --git a/atproto/identity/directory.go b/atproto/identity/directory.go
@@ -85,6 +85,5 @@ func DefaultDirectory() Directory {
 		SkipDNSDomainSuffixes: []string{".bsky.social"},
 		UserAgent:             "indigo-identity/" + versioninfo.Short(),
 	}
-	cached := NewCacheDirectory(&base, 250_000, time.Hour*24, time.Minute*2, time.Minute*5)
-	return &cached
+	return NewCacheDirectory(&base, 250_000, time.Hour*24, time.Minute*2, time.Minute*5)
 }
diff --git a/atproto/identity/live_test.go b/atproto/identity/live_test.go
@@ -66,7 +66,7 @@ func TestCacheDirectory(t *testing.T) {
 	inner := BaseDirectory{}
 	d := NewCacheDirectory(&inner, 1000, time.Hour*1, time.Hour*1, time.Hour*1)
 	for i := 0; i < 3; i = i + 1 {
-		testDirectoryLive(t, &d)
+		testDirectoryLive(t, d)
 	}
 }
 
diff --git a/atproto/identity/mock_directory.go b/atproto/identity/mock_directory.go
@@ -19,8 +19,8 @@ type MockDirectory struct {
 var _ Directory = (*MockDirectory)(nil)
 var _ Resolver = (*MockDirectory)(nil)
 
-func NewMockDirectory() MockDirectory {
-	return MockDirectory{
+func NewMockDirectory() *MockDirectory {
+	return &MockDirectory{
 		handles:    make(map[syntax.Handle]syntax.DID),
 		identities: make(map[syntax.DID]Identity),
 	}
diff --git a/automod/capture/testing.go b/automod/capture/testing.go
@@ -42,7 +42,7 @@ func ProcessCaptureRules(eng *automod.Engine, capture AccountCapture) error {
 	handle := capture.AccountMeta.Identity.Handle.String()
 	dir := identity.NewMockDirectory()
 	dir.Insert(*capture.AccountMeta.Identity)
-	eng.Directory = &dir
+	eng.Directory = dir
 
 	// initial identity rules
 	identEvent := comatproto.SyncSubscribeRepos_Identity{
diff --git a/automod/engine/circuit_breaker_test.go b/automod/engine/circuit_breaker_test.go
@@ -29,7 +29,7 @@ func TestTakedownCircuitBreaker(t *testing.T) {
 	ctx := context.Background()
 	eng := EngineTestFixture()
 	dir := identity.NewMockDirectory()
-	eng.Directory = &dir
+	eng.Directory = dir
 	// note that this is a record-level action, not account-level
 	eng.Rules = RuleSet{
 		RecordRules: []RecordRuleFunc{
@@ -75,7 +75,7 @@ func TestReportCircuitBreaker(t *testing.T) {
 	ctx := context.Background()
 	eng := EngineTestFixture()
 	dir := identity.NewMockDirectory()
-	eng.Directory = &dir
+	eng.Directory = dir
 	eng.Rules = RuleSet{
 		RecordRules: []RecordRuleFunc{
 			alwaysReportRecordRule,
diff --git a/automod/engine/testing.go b/automod/engine/testing.go
@@ -60,7 +60,7 @@ func EngineTestFixture() Engine {
 	dir.Insert(id1)
 	eng := Engine{
 		Logger:    slog.Default(),
-		Directory: &dir,
+		Directory: dir,
 		Counters:  countstore.NewMemCountStore(),
 		Sets:      sets,
 		Flags:     flags,
diff --git a/cmd/bluepages/main.go b/cmd/bluepages/main.go
@@ -10,9 +10,11 @@ import (
 	"runtime"
 	"strings"
 
-	_ "github.com/joho/godotenv/autoload"
 	_ "net/http/pprof"
 
+	_ "github.com/joho/godotenv/autoload"
+
+	"github.com/bluesky-social/indigo/atproto/identity"
 	"github.com/bluesky-social/indigo/atproto/identity/apidir"
 	"github.com/bluesky-social/indigo/atproto/syntax"
 
@@ -184,7 +186,12 @@ func configLogger(cmd *cli.Command, writer io.Writer) *slog.Logger {
 	return logger
 }
 
-func configClient(cmd *cli.Command) apidir.APIDirectory {
+type directory interface {
+	identity.Directory
+	identity.Resolver
+}
+
+func configClient(cmd *cli.Command) directory {
 	return apidir.NewAPIDirectory(cmd.String("host"))
 }
 
diff --git a/cmd/hepa/main.go b/cmd/hepa/main.go
@@ -12,9 +12,10 @@ import (
 	"strings"
 	"time"
 
-	_ "github.com/joho/godotenv/autoload"
 	_ "net/http/pprof"
 
+	_ "github.com/joho/godotenv/autoload"
+
 	"github.com/bluesky-social/indigo/atproto/identity"
 	"github.com/bluesky-social/indigo/atproto/identity/redisdir"
 	"github.com/bluesky-social/indigo/atproto/syntax"
@@ -222,8 +223,7 @@ func configDirectory(cmd *cli.Command) (identity.Directory, error) {
 		}
 		dir = rdir
 	} else {
-		cdir := identity.NewCacheDirectory(&baseDir, 1_500_000, time.Hour*24, time.Minute*2, time.Minute*5)
-		dir = &cdir
+		dir = identity.NewCacheDirectory(&baseDir, 1_500_000, time.Hour*24, time.Minute*2, time.Minute*5)
 	}
 	return dir, nil
 }
diff --git a/cmd/palomar/main.go b/cmd/palomar/main.go
@@ -267,7 +267,7 @@ var runCmd = &cli.Command{
 			PostIndex:    cmd.String("es-post-index"),
 		}
 
-		srv, err := search.NewServer(escli, &dir, apiConfig)
+		srv, err := search.NewServer(escli, dir, apiConfig)
 		if err != nil {
 			return err
 		}
@@ -290,7 +290,7 @@ var runCmd = &cli.Command{
 				IndexingRateLimit:   cmd.Int("indexing-rate-limit"),
 			}
 
-			idx, err := search.NewIndexer(db, escli, &dir, indexerConfig)
+			idx, err := search.NewIndexer(db, escli, dir, indexerConfig)
 			if err != nil {
 				return fmt.Errorf("failed to set up indexer: %w", err)
 			}
diff --git a/cmd/relay/main.go b/cmd/relay/main.go
@@ -278,7 +278,7 @@ func runRelay(ctx context.Context, cmd *cli.Command) error {
 	evtman := eventmgr.NewEventManager(persister)
 
 	logger.Info("constructing relay service")
-	r, err := relay.NewRelay(db, evtman, &dir, relayConfig)
+	r, err := relay.NewRelay(db, evtman, dir, relayConfig)
 	if err != nil {
 		return err
 	}
diff --git a/cmd/relay/testing/runner.go b/cmd/relay/testing/runner.go
@@ -130,7 +130,7 @@ func RunScenario(ctx context.Context, s *Scenario) error {
 	hostPort := p.ListenRandom()
 	defer p.Shutdown()
 
-	sr := MustSimpleRelay(&dir, tmpd, s.Lenient)
+	sr := MustSimpleRelay(dir, tmpd, s.Lenient)
 
 	err = sr.Relay.SubscribeToHost(ctx, fmt.Sprintf("localhost:%d", hostPort), true, true)
 	if err != nil {
diff --git a/cmd/tap/tap.go b/cmd/tap/tap.go
@@ -70,7 +70,7 @@ func NewTap(config TapConfig) (*Tap, error) {
 
 	evtMngr := NewEventManager(logger, db, &config)
 
-	repoMngr := NewRepoManager(logger, db, evtMngr, &cdir)
+	repoMngr := NewRepoManager(logger, db, evtMngr, cdir)
 
 	resyncer := NewResyncer(logger, db, repoMngr, evtMngr, &config)
 
@@ -80,7 +80,7 @@ func NewTap(config TapConfig) (*Tap, error) {
 
 	outbox := NewOutbox(logger, evtMngr, &config)
 
-	server := NewTapServer(logger, db, outbox, &cdir, firehose, crawler, &config)
+	server := NewTapServer(logger, db, outbox, cdir, firehose, crawler, &config)
 
 	t := &Tap{
 		db:     db,
diff --git a/search/parse_query_test.go b/search/parse_query_test.go
@@ -22,35 +22,35 @@ func TestParseQuery(t *testing.T) {
 
 	var p PostSearchParams
 
-	p = ParsePostQuery(ctx, &dir, "", nil)
+	p = ParsePostQuery(ctx, dir, "", nil)
 	assert.Equal("*", p.Query)
 	assert.Empty(p.Filters())
 
 	q1 := "some +test \"with phrase\" -ok"
-	p = ParsePostQuery(ctx, &dir, q1, nil)
+	p = ParsePostQuery(ctx, dir, q1, nil)
 	assert.Equal(q1, p.Query)
 	assert.Empty(p.Filters())
 
 	q2 := "missing from:missing.example.com"
-	p = ParsePostQuery(ctx, &dir, q2, nil)
+	p = ParsePostQuery(ctx, dir, q2, nil)
 	assert.Equal("missing", p.Query)
 	assert.Empty(p.Filters())
 
 	q3 := "known from:known.example.com"
-	p = ParsePostQuery(ctx, &dir, q3, nil)
+	p = ParsePostQuery(ctx, dir, q3, nil)
 	assert.Equal("known", p.Query)
 	assert.NotNil(p.Author)
 	if p.Author != nil {
 		assert.Equal("did:plc:abc222", p.Author.String())
 	}
 
 	q4 := "from:known.example.com"
-	p = ParsePostQuery(ctx, &dir, q4, nil)
+	p = ParsePostQuery(ctx, dir, q4, nil)
 	assert.Equal("*", p.Query)
 	assert.Equal(1, len(p.Filters()))
 
 	q5 := `from:known.example.com "multi word phrase" coolio blorg`
-	p = ParsePostQuery(ctx, &dir, q5, nil)
+	p = ParsePostQuery(ctx, dir, q5, nil)
 	assert.Equal(`"multi word phrase" coolio blorg`, p.Query)
 	assert.NotNil(p.Author)
 	if p.Author != nil {
@@ -59,7 +59,7 @@ func TestParseQuery(t *testing.T) {
 	assert.Equal(1, len(p.Filters()))
 
 	q6 := `from:known.example.com #cool_tag some other stuff`
-	p = ParsePostQuery(ctx, &dir, q6, nil)
+	p = ParsePostQuery(ctx, dir, q6, nil)
 	assert.Equal(`some other stuff`, p.Query)
 	assert.NotNil(p.Author)
 	if p.Author != nil {
@@ -69,7 +69,7 @@ func TestParseQuery(t *testing.T) {
 	assert.Equal(2, len(p.Filters()))
 
 	q7 := "known from:@known.example.com"
-	p = ParsePostQuery(ctx, &dir, q7, nil)
+	p = ParsePostQuery(ctx, dir, q7, nil)
 	assert.Equal("known", p.Query)
 	assert.NotNil(p.Author)
 	if p.Author != nil {
@@ -78,7 +78,7 @@ func TestParseQuery(t *testing.T) {
 	assert.Equal(1, len(p.Filters()))
 
 	q8 := "known from:me"
-	p = ParsePostQuery(ctx, &dir, q8, &ident.DID)
+	p = ParsePostQuery(ctx, dir, q8, &ident.DID)
 	assert.Equal("known", p.Query)
 	assert.NotNil(p.Author)
 	if p.Author != nil {
@@ -87,7 +87,7 @@ func TestParseQuery(t *testing.T) {
 	assert.Equal(1, len(p.Filters()))
 
 	q9 := "did:plc:abc222"
-	p = ParsePostQuery(ctx, &dir, q9, nil)
+	p = ParsePostQuery(ctx, dir, q9, nil)
 	assert.Equal("*", p.Query)
 	assert.Equal(1, len(p.Filters()))
 	if p.Author != nil {

Original file line number	Diff line number	Diff line change
`@@ -96,7 +96,7 @@ func TestServiceAuthMiddleware(t *testing.T) {`
`96`	`96`
`97`	`97`	`v := ServiceAuthValidator{`
`98`	`98`	`Audience: aud,`
`99`		`- Dir: &dir,`
	`99`	`+ Dir: dir,`
`100`	`100`	`}`
`101`	`101`
`102`	`102`	`{`
Original file line number	Diff line number	Diff line change
`@@ -110,7 +110,7 @@ func testSigningValidation(t *testing.T, priv atcrypto.PrivateKey) {`
`110`	`110`
`111`	`111`	`v := ServiceAuthValidator{`
`112`	`112`	`Audience: aud,`
`113`		`- Dir: &dir,`
	`113`	`+ Dir: dir,`
`114`	`114`	`}`
`115`	`115`
`116`	`116`	`t1, err := SignServiceAuth(iss, aud, time.Minute, nil, priv)`
Original file line number	Diff line number	Diff line change
`@@ -85,6 +85,5 @@ func DefaultDirectory() Directory {`
`85`	`85`	`SkipDNSDomainSuffixes: []string{".bsky.social"},`
`86`	`86`	`UserAgent: "indigo-identity/" + versioninfo.Short(),`
`87`	`87`	`}`
`88`		`- cached := NewCacheDirectory(&base, 250_000, time.Hour24, time.Minute2, time.Minute*5)`
`89`		`- return &cached`
	`88`	`+ return NewCacheDirectory(&base, 250_000, time.Hour24, time.Minute2, time.Minute*5)`
`90`	`89`	`}`
Original file line number	Diff line number	Diff line change
`@@ -66,7 +66,7 @@ func TestCacheDirectory(t *testing.T) {`
`66`	`66`	`inner := BaseDirectory{}`
`67`	`67`	`d := NewCacheDirectory(&inner, 1000, time.Hour1, time.Hour1, time.Hour*1)`
`68`	`68`	`for i := 0; i < 3; i = i + 1 {`
`69`		`- testDirectoryLive(t, &d)`
	`69`	`+ testDirectoryLive(t, d)`
`70`	`70`	`}`
`71`	`71`	`}`
`72`	`72`