A community based topic aggregation platform built on atproto

fix(blocking): address PR review feedback - performance and error handling

Fixes four issues identified in PR review:

**BUG 1 - Performance: Remove redundant database query**
- Removed duplicate GetByDID call in BlockCommunity service method
- ResolveCommunityIdentifier already verifies community exists
- Reduces block operations from 2 DB queries to 1

**BUG 2 - Performance: Move regex compilation to package level**
- Moved DID validation regex to package-level variable in block.go
- Prevents recompiling regex on every block/unblock request
- Eliminates unnecessary CPU overhead on hot path

**BUG 3 - DRY: Remove duplicated extractRKeyFromURI**
- Removed duplicate implementations in service.go and tests
- Now uses shared utils.ExtractRKeyFromURI function
- Single source of truth for AT-URI parsing logic

**P1 - Critical: Fix duplicate block race condition**
- Added ErrBlockAlreadyExists error type
- Returns 409 Conflict instead of 500 when PDS has block but AppView hasn't indexed yet
- Handles normal race in eventually-consistent flow gracefully
- Prevents double-click scenarios from appearing as server failures

All tests passing (33.2s runtime, 100% pass rate).

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>

Changed files
+26 -35
internal
api
handlers
community
core
communities
tests
integration
+5 -2
internal/api/handlers/community/block.go
···
"strings"
)
+
// Package-level compiled regex for DID validation (compiled once at startup)
+
var (
+
didRegex = regexp.MustCompile(`^did:(plc|web):[a-zA-Z0-9._:%-]+$`)
+
)
+
// BlockHandler handles community blocking operations
type BlockHandler struct {
service communities.Service
···
}
// Validate DID format with regex: did:method:identifier
-
didRegex := regexp.MustCompile(`^did:(plc|web):[a-zA-Z0-9._:%-]+$`)
if !didRegex.MatchString(req.Community) {
writeError(w, http.StatusBadRequest, "InvalidRequest", "invalid DID format")
return
···
}
// Validate DID format with regex: did:method:identifier
-
didRegex := regexp.MustCompile(`^did:(plc|web):[a-zA-Z0-9._:%-]+$`)
if !didRegex.MatchString(req.Community) {
writeError(w, http.StatusBadRequest, "InvalidRequest", "invalid DID format")
return
+5 -1
internal/core/communities/errors.go
···
// ErrBlockNotFound is returned when block doesn't exist
ErrBlockNotFound = errors.New("block not found")
+
// ErrBlockAlreadyExists is returned when user has already blocked the community
+
ErrBlockAlreadyExists = errors.New("community already blocked")
+
// ErrMembershipNotFound is returned when membership doesn't exist
ErrMembershipNotFound = errors.New("membership not found")
···
func IsConflict(err error) bool {
return errors.Is(err, ErrCommunityAlreadyExists) ||
errors.Is(err, ErrHandleTaken) ||
-
errors.Is(err, ErrSubscriptionAlreadyExists)
+
errors.Is(err, ErrSubscriptionAlreadyExists) ||
+
errors.Is(err, ErrBlockAlreadyExists)
}
// IsValidationError checks if error is a validation error
+9 -18
internal/core/communities/service.go
···
package communities
import (
+
"Coves/internal/atproto/utils"
"bytes"
"context"
"encoding/json"
···
}
// Extract rkey from record URI (at://did/collection/rkey)
-
rkey := extractRKeyFromURI(subscription.RecordURI)
+
rkey := utils.ExtractRKeyFromURI(subscription.RecordURI)
if rkey == "" {
return fmt.Errorf("invalid subscription record URI")
}
···
return nil, NewValidationError("userAccessToken", "required")
}
-
// Resolve community identifier
+
// Resolve community identifier (also verifies community exists)
communityDID, err := s.ResolveCommunityIdentifier(ctx, communityIdentifier)
if err != nil {
return nil, err
}
-
// Verify community exists
-
_, err = s.repo.GetByDID(ctx, communityDID)
-
if err != nil {
-
return nil, err
-
}
-
// Build block record
// CRITICAL: Collection is social.coves.community.block (RECORD TYPE)
// This record will be created in the USER's repository: at://user_did/social.coves.community.block/{tid}
···
// Fetch and return existing block from our indexed view
existingBlock, getErr := s.repo.GetBlock(ctx, userDID, communityDID)
if getErr == nil {
+
// Block exists in our index - return it
return existingBlock, nil
}
-
// If we can't find it in our index, return the original PDS error
+
// Race condition: PDS has the block but Jetstream hasn't indexed it yet
+
// Return typed conflict error so handler can return 409 instead of 500
+
// This is normal in eventually-consistent systems
+
return nil, ErrBlockAlreadyExists
}
return nil, fmt.Errorf("failed to create block on PDS: %w", err)
}
···
}
// Extract rkey from record URI (at://did/collection/rkey)
-
rkey := extractRKeyFromURI(block.RecordURI)
+
rkey := utils.ExtractRKeyFromURI(block.RecordURI)
if rkey == "" {
return fmt.Errorf("invalid block record URI")
}
···
// Helper functions
-
func extractRKeyFromURI(uri string) string {
-
// at://did/collection/rkey -> rkey
-
parts := strings.Split(uri, "/")
-
if len(parts) >= 4 {
-
return parts[len(parts)-1]
-
}
-
return ""
-
}
+7 -14
tests/integration/community_e2e_test.go
···
"Coves/internal/api/routes"
"Coves/internal/atproto/identity"
"Coves/internal/atproto/jetstream"
+
"Coves/internal/atproto/utils"
"Coves/internal/core/communities"
"Coves/internal/core/users"
"Coves/internal/db/postgres"
···
t.Logf("\n📡 V2: Querying PDS for record in community's repository...")
collection := "social.coves.community.profile"
-
rkey := extractRKeyFromURI(community.RecordURI)
+
rkey := utils.ExtractRKeyFromURI(community.RecordURI)
// V2: Query community's repository (not instance repository!)
getRecordURL := fmt.Sprintf("%s/xrpc/com.atproto.repo.getRecord?repo=%s&collection=%s&rkey=%s",
···
// NOTE: Using synthetic event for speed. Real Jetstream WebSocket testing
// happens in "Part 2: Real Jetstream Firehose Consumption" above.
t.Logf("🔄 Simulating Jetstream consumer indexing...")
-
rkey := extractRKeyFromURI(createResp.URI)
+
rkey := utils.ExtractRKeyFromURI(createResp.URI)
// V2: Event comes from community's DID (community owns the repo)
event := jetstream.JetstreamEvent{
Did: createResp.DID,
···
pdsURL = "http://localhost:3001"
}
-
rkey := extractRKeyFromURI(subscribeResp.URI)
+
rkey := utils.ExtractRKeyFromURI(subscribeResp.URI)
// CRITICAL: Use correct collection name (record type, not XRPC endpoint)
collection := "social.coves.community.subscription"
···
}
// Index the subscription in AppView (simulate firehose event)
-
rkey := extractRKeyFromURI(subscription.RecordURI)
+
rkey := utils.ExtractRKeyFromURI(subscription.RecordURI)
subEvent := jetstream.JetstreamEvent{
Did: instanceDID,
TimeUS: time.Now().UnixMicro(),
···
// Simulate Jetstream consumer picking up the update event
t.Logf("🔄 Simulating Jetstream consumer indexing update...")
-
rkey := extractRKeyFromURI(updateResp.URI)
+
rkey := utils.ExtractRKeyFromURI(updateResp.URI)
// Fetch updated record from PDS
pdsURL := os.Getenv("PDS_URL")
···
// Fetch from PDS to get full record
// V2: Record lives in community's own repository (at://community.DID/...)
collection := "social.coves.community.profile"
-
rkey := extractRKeyFromURI(community.RecordURI)
+
rkey := utils.ExtractRKeyFromURI(community.RecordURI)
pdsResp, pdsErr := http.Get(fmt.Sprintf("%s/xrpc/com.atproto.repo.getRecord?repo=%s&collection=%s&rkey=%s",
pdsURL, community.DID, collection, rkey))
···
return community
-
func extractRKeyFromURI(uri string) string {
-
// at://did/collection/rkey -> rkey
-
parts := strings.Split(uri, "/")
-
if len(parts) >= 4 {
-
return parts[len(parts)-1]
-
}
-
return ""
-
}
// authenticateWithPDS authenticates with the PDS and returns access token and DID
func authenticateWithPDS(pdsURL, handle, password string) (string, string, error) {