diff --git a/CLAUDE.md b/CLAUDE.md --- a/CLAUDE.md +++ b/CLAUDE.md @@ -1,82 +1,112 @@ -# CLAUDE-BUILD.md -Project: Coves Builder You are a distinguished developer actively building Coves, a forum-like atProto social media platform. Your goal is to ship working features quickly while maintaining quality and security. +Project: Coves PR Reviewer +You are a distinguished senior architect conducting a thorough code review for Coves, a forum-like atProto social media platform. -## Builder Mindset +## Review Mindset +- Be constructive but thorough - catch issues before they reach production +- Question assumptions and look for edge cases +- Prioritize security, performance, and maintainability concerns +- Suggest alternatives when identifying problems -- Ship working code today, refactor tomorrow -- Security is built-in, not bolted-on -- Test-driven: write the test, then make it pass -- When stuck, check Context7 for patterns and examples -- ASK QUESTIONS if you need context surrounding the product DONT ASSUME -#### Human & LLM Readability Guidelines: -- Descriptive Naming: Use full words over abbreviations (e.g., CommunityGovernance not CommGov) +## Special Attention Areas for Coves +- **atProto Integration**: Verify proper use of indigo packages +- **atProto architecture**: Ensure architecture follows atProto recommendations +- **Federation**: Check for proper DID resolution and identity verification +- **PostgreSQL**: Verify migrations are reversible and indexes are appropriate -## atProto Essentials for Coves +## Review Checklist -### Architecture -- **PDS is Self-Contained**: Uses internal SQLite + CAR files (in Docker volume) -- **PostgreSQL for AppView Only**: One database for Coves AppView indexing -- **Don't Touch PDS Internals**: PDS manages its own storage, we just read from firehose -- **Data Flow**: Client → PDS → Firehose → AppView → PostgreSQL +### 1. Architecture Compliance +**MUST VERIFY:** +- [ ] NO SQL queries in handlers (automatic rejection if found) +- [ ] Proper layer separation: Handler → Service → Repository → Database +- [ ] Services use repository interfaces, not concrete implementations +- [ ] Dependencies injected via constructors, not globals +- [ ] No database packages imported in handlers -### Always Consider: -- [ ] **Identity**: Every action needs DID verification -- [ ] **Record Types**: Define custom lexicons (e.g., `social.coves.post`, `social.coves.community`) -- [ ] **Is it federated-friendly?** (Can other PDSs interact with it?) -- [ ] **Does the Lexicon make sense?** (Would it work for other forums?) -- [ ] **AppView only indexes**: We don't write to CAR files, only read from firehose - -## Security-First Building - -### Every Feature MUST: - -- [ ] **Validate all inputs** at the handler level -- [ ] **Use parameterized queries** (never string concatenation) -- [ ] **Check authorization** before any operation -- [ ] **Limit resource access** (pagination, rate limits) -- [ ] **Log security events** (failed auth, invalid inputs) -- [ ] **Never log sensitive data** (passwords, tokens, PII) +### 2. Security Review +**CHECK FOR:** +- SQL injection vulnerabilities (even with prepared statements, verify) +- Proper input validation and sanitization +- Authentication/authorization checks on all protected endpoints +- No sensitive data in logs or error messages +- Rate limiting on public endpoints +- CSRF protection where applicable +- Proper atProto identity verification -### Red Flags to Avoid: +### 3. Error Handling Audit +**VERIFY:** +- All errors are handled, not ignored +- Error wrapping provides context: `fmt.Errorf("service: %w", err)` +- Domain errors defined in core/errors/ +- HTTP status codes correctly map to error types +- No internal error details exposed to API consumers +- Nil pointer checks before dereferencing -- `fmt.Sprintf` in SQL queries → Use parameterized queries -- Missing `context.Context` → Need it for timeouts/cancellation -- No input validation → Add it immediately -- Error messages with internal details → Wrap errors properly -- Unbounded queries → Add limits/pagination +### 4. Performance Considerations +**LOOK FOR:** +- N+1 query problems +- Missing database indexes for frequently queried fields +- Unnecessary database round trips +- Large unbounded queries without pagination +- Memory leaks in goroutines +- Proper connection pool usage +- Efficient atProto federation calls -### "How should I structure this?" +### 5. Testing Coverage +**REQUIRE:** +- Unit tests for all new service methods +- Integration tests for new API endpoints +- Edge case coverage (empty inputs, max values, special characters) +- Error path testing +- Mock verification in unit tests +- No flaky tests (check for time dependencies, random values) -1. One domain, one package -2. Interfaces for testability -3. Services coordinate repos -4. Handlers only handle XRPC +### 6. Code Quality +**ASSESS:** +- Naming follows conventions (full words, not abbreviations) +- Functions do one thing well +- No code duplication (DRY principle) +- Consistent error handling patterns +- Proper use of Go idioms +- No commented-out code -## Pre-Production Advantages +### 7. Breaking Changes +**IDENTIFY:** +- API contract changes +- Database schema modifications affecting existing data +- Changes to core interfaces +- Modified error codes or response formats -Since we're pre-production: +### 8. Documentation +**ENSURE:** +- API endpoints have example requests/responses +- Complex business logic is explained +- Database migrations include rollback scripts +- README updated if setup process changes +- Swagger/OpenAPI specs updated if applicable -- **Break things**: Delete and rebuild rather than complex migrations -- **Experiment**: Try approaches, keep what works -- **Simplify**: Remove unused code aggressively -- **But never compromise security basics** +## Review Process -## Success Metrics +1. **First Pass - Automatic Rejections** + - SQL in handlers + - Missing tests + - Security vulnerabilities + - Broken layer separation -Your code is ready when: +2. **Second Pass - Deep Dive** + - Business logic correctness + - Edge case handling + - Performance implications + - Code maintainability -- [ ] Tests pass (including security tests) -- [ ] Follows atProto patterns -- [ ] Handles errors gracefully -- [ ] Works end-to-end with auth +3. **Third Pass - Suggestions** + - Better patterns or approaches + - Refactoring opportunities + - Future considerations -## Quick Checks Before Committing +Then provide detailed feedback organized by: 1. 🚨 **Critical Issues** (must fix) 2. ⚠️ **Important Issues** (should fix) 3. 💡 **Suggestions** (consider for improvement) 4. ✅ **Good Practices Observed** (reinforce positive patterns) -1. **Will it work?** (Integration test proves it) -2. **Is it secure?** (Auth, validation, parameterized queries) -3. **Is it simple?** (Could you explain to a junior?) -4. **Is it complete?** (Test, implementation, documentation) -Remember: We're building a working product. Perfect is the enemy of shipped. +Remember: The goal is to ship quality code quickly. Perfection is not required, but safety and maintainability are non-negotiable. diff --git a/TESTING_SUMMARY.md b/TESTING_SUMMARY.md --- a/TESTING_SUMMARY.md +++ b/TESTING_SUMMARY.md @@ -81,10 +81,10 @@ ### 1. Unit Tests Test individual components in isolation. -**Example**: [internal/core/users/service_test.go](internal/core/users/service_test.go) +**Note**: Unit tests will be added as needed. Currently focusing on integration tests. ```bash -# Run unit tests for a specific package +# Run unit tests for a specific package (when available) go test -v ./internal/core/users/... ``` @@ -346,7 +346,7 @@ - ✅ Test data lives in `tests/lexicon-test-data/` - ✅ One test file per feature/endpoint ### Test Data -- ✅ Use `@example.com` emails for test users (auto-cleaned by setupTestDB) +- ✅ Use `.test` handles for test users (e.g., `alice.test`) (auto-cleaned by setupTestDB) - ✅ Clean up data in tests (or rely on setupTestDB cleanup) - ✅ Don't rely on specific test execution order - ✅ Each test should be independent diff --git a/cmd/server/main.go b/cmd/server/main.go --- a/cmd/server/main.go +++ b/cmd/server/main.go @@ -6,21 +6,31 @@ "fmt" "log" "net/http" "os" + "time" "github.com/go-chi/chi/v5" - "github.com/go-chi/chi/v5/middleware" + chiMiddleware "github.com/go-chi/chi/v5/middleware" _ "github.com/lib/pq" "github.com/pressly/goose/v3" + "Coves/internal/api/middleware" "Coves/internal/api/routes" "Coves/internal/core/users" postgresRepo "Coves/internal/db/postgres" ) func main() { + // Database configuration (AppView database) dbURL := os.Getenv("DATABASE_URL") if dbURL == "" { - dbURL = "postgres://postgres:password@localhost:5432/coves?sslmode=disable" + // Use dev database from .env.dev + dbURL = "postgres://dev_user:dev_password@localhost:5433/coves_dev?sslmode=disable" + } + + // PDS URL configuration + pdsURL := os.Getenv("PDS_URL") + if pdsURL == "" { + pdsURL = "http://localhost:3001" // Local dev PDS } db, err := sql.Open("postgres", dbURL) @@ -33,6 +43,9 @@ if err := db.Ping(); err != nil { log.Fatal("Failed to ping database:", err) } + log.Println("Connected to AppView database") + + // Run migrations if err := goose.SetDialect("postgres"); err != nil { log.Fatal("Failed to set goose dialect:", err) } @@ -41,29 +54,36 @@ if err := goose.Up(db, "internal/db/migrations"); err != nil { log.Fatal("Failed to run migrations:", err) } + log.Println("Migrations completed successfully") + r := chi.NewRouter() - r.Use(middleware.Logger) - r.Use(middleware.Recoverer) - r.Use(middleware.RequestID) + r.Use(chiMiddleware.Logger) + r.Use(chiMiddleware.Recoverer) + r.Use(chiMiddleware.RequestID) - // Initialize repositories + // Rate limiting: 100 requests per minute per IP + rateLimiter := middleware.NewRateLimiter(100, 1*time.Minute) + r.Use(rateLimiter.Middleware) + + // Initialize repositories and services userRepo := postgresRepo.NewUserRepository(db) - userService := users.NewUserService(userRepo) + userService := users.NewUserService(userRepo, pdsURL) - // Mount routes - r.Mount("/api/users", routes.UserRoutes(userService)) + // Mount XRPC routes + r.Mount("/xrpc/social.coves.actor", routes.UserRoutes(userService)) r.Get("/health", func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusOK) w.Write([]byte("OK")) }) - port := os.Getenv("PORT") + port := os.Getenv("APPVIEW_PORT") if port == "" { - port = "8080" + port = "8081" // Match .env.dev default } - fmt.Printf("Server starting on port %s\n", port) + fmt.Printf("Coves AppView starting on port %s\n", port) + fmt.Printf("PDS URL: %s\n", pdsURL) log.Fatal(http.ListenAndServe(":"+port, r)) } diff --git a/internal/api/middleware/ratelimit.go b/internal/api/middleware/ratelimit.go new file mode 100644 --- /dev/null +++ b/internal/api/middleware/ratelimit.go @@ -0,0 +1,122 @@ +package middleware + +import ( + "net/http" + "sync" + "time" +) + +// RateLimiter implements a simple in-memory rate limiter +// For production, consider using Redis or a distributed rate limiter +type RateLimiter struct { + mu sync.Mutex + clients map[string]*clientLimit + requests int // Max requests per window + window time.Duration // Time window +} + +type clientLimit struct { + count int + resetTime time.Time +} + +// NewRateLimiter creates a new rate limiter +// requests: maximum number of requests allowed per window +// window: time window duration (e.g., 1 minute) +func NewRateLimiter(requests int, window time.Duration) *RateLimiter { + rl := &RateLimiter{ + clients: make(map[string]*clientLimit), + requests: requests, + window: window, + } + + // Cleanup old entries every window duration + go rl.cleanup() + + return rl +} + +// Middleware returns a rate limiting middleware +func (rl *RateLimiter) Middleware(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + // Use IP address as client identifier + // In production, consider using authenticated user ID if available + clientID := getClientIP(r) + + if !rl.allow(clientID) { + http.Error(w, "Rate limit exceeded. Please try again later.", http.StatusTooManyRequests) + return + } + + next.ServeHTTP(w, r) + }) +} + +// allow checks if a client is allowed to make a request +func (rl *RateLimiter) allow(clientID string) bool { + rl.mu.Lock() + defer rl.mu.Unlock() + + now := time.Now() + + // Get or create client limit + client, exists := rl.clients[clientID] + if !exists { + rl.clients[clientID] = &clientLimit{ + count: 1, + resetTime: now.Add(rl.window), + } + return true + } + + // Check if window has expired + if now.After(client.resetTime) { + client.count = 1 + client.resetTime = now.Add(rl.window) + return true + } + + // Check if under limit + if client.count < rl.requests { + client.count++ + return true + } + + // Rate limit exceeded + return false +} + +// cleanup removes expired client entries periodically +func (rl *RateLimiter) cleanup() { + ticker := time.NewTicker(rl.window) + defer ticker.Stop() + + for range ticker.C { + rl.mu.Lock() + now := time.Now() + for clientID, client := range rl.clients { + if now.After(client.resetTime) { + delete(rl.clients, clientID) + } + } + rl.mu.Unlock() + } +} + +// getClientIP extracts the client IP from the request +func getClientIP(r *http.Request) string { + // Check X-Forwarded-For header (if behind proxy) + forwarded := r.Header.Get("X-Forwarded-For") + if forwarded != "" { + return forwarded + } + + // Check X-Real-IP header + realIP := r.Header.Get("X-Real-IP") + if realIP != "" { + return realIP + } + + // Fall back to RemoteAddr + return r.RemoteAddr +} diff --git a/internal/api/routes/user.go b/internal/api/routes/user.go --- a/internal/api/routes/user.go +++ b/internal/api/routes/user.go @@ -1,20 +1,76 @@ package routes import ( + "encoding/json" + "net/http" + "time" + "Coves/internal/core/users" "github.com/go-chi/chi/v5" - "net/http" ) -// UserRoutes returns user-related routes +// UserHandler handles user-related XRPC endpoints +type UserHandler struct { + userService users.UserService +} + +// NewUserHandler creates a new user handler +func NewUserHandler(userService users.UserService) *UserHandler { + return &UserHandler{ + userService: userService, + } +} + +// UserRoutes returns user-related XRPC routes +// Implements social.coves.actor.* lexicon endpoints func UserRoutes(service users.UserService) chi.Router { + h := NewUserHandler(service) r := chi.NewRouter() - - // TODO: Implement user handlers - r.Get("/", func(w http.ResponseWriter, r *http.Request) { - w.WriteHeader(http.StatusOK) - w.Write([]byte("User routes not yet implemented")) - }) - + + // social.coves.actor.getProfile - query endpoint + r.Get("/profile", h.GetProfile) + return r +} + +// GetProfile handles social.coves.actor.getProfile +// Query endpoint that retrieves a user profile by DID or handle +func (h *UserHandler) GetProfile(w http.ResponseWriter, r *http.Request) { + ctx := r.Context() + + // Get actor parameter (DID or handle) + actor := r.URL.Query().Get("actor") + if actor == "" { + http.Error(w, "actor parameter is required", http.StatusBadRequest) + return + } + + var user *users.User + var err error + + // Determine if actor is a DID or handle + // DIDs start with "did:", handles don't + if len(actor) > 4 && actor[:4] == "did:" { + user, err = h.userService.GetUserByDID(ctx, actor) + } else { + user, err = h.userService.GetUserByHandle(ctx, actor) + } + + if err != nil { + http.Error(w, "user not found", http.StatusNotFound) + return + } + + // Minimal profile response (matching lexicon structure) + response := map[string]interface{}{ + "did": user.DID, + "profile": map[string]interface{}{ + "handle": user.Handle, + "createdAt": user.CreatedAt.Format(time.RFC3339), + }, + } + + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusOK) + json.NewEncoder(w).Encode(response) } \ No newline at end of file diff --git a/internal/core/users/interfaces.go b/internal/core/users/interfaces.go --- a/internal/core/users/interfaces.go +++ b/internal/core/users/interfaces.go @@ -1,10 +1,18 @@ package users -type UserServiceInterface interface { - CreateUser(req CreateUserRequest) (*User, error) - GetUserByID(id int) (*User, error) - GetUserByEmail(email string) (*User, error) - GetUserByUsername(username string) (*User, error) - UpdateUser(id int, req UpdateUserRequest) (*User, error) - DeleteUser(id int) error +import "context" + +// UserRepository defines the interface for user data persistence +type UserRepository interface { + Create(ctx context.Context, user *User) (*User, error) + GetByDID(ctx context.Context, did string) (*User, error) + GetByHandle(ctx context.Context, handle string) (*User, error) +} + +// UserService defines the interface for user business logic +type UserService interface { + CreateUser(ctx context.Context, req CreateUserRequest) (*User, error) + GetUserByDID(ctx context.Context, did string) (*User, error) + GetUserByHandle(ctx context.Context, handle string) (*User, error) + ResolveHandleToDID(ctx context.Context, handle string) (string, error) } \ No newline at end of file diff --git a/internal/core/users/repository.go b/internal/core/users/repository.go deleted file mode 100644 --- a/internal/core/users/repository.go +++ /dev/null @@ -1,10 +0,0 @@ -package users - -type UserRepository interface { - Create(user *User) (*User, error) - GetByID(id int) (*User, error) - GetByEmail(email string) (*User, error) - GetByUsername(username string) (*User, error) - Update(user *User) (*User, error) - Delete(id int) error -} \ No newline at end of file diff --git a/internal/core/users/service.go b/internal/core/users/service.go --- a/internal/core/users/service.go +++ b/internal/core/users/service.go @@ -1,164 +1,117 @@ package users import ( + "context" "fmt" + "regexp" "strings" ) -type UserService struct { +// atProto handle validation regex +// Handles must: start/end with alphanumeric, contain only alphanumeric + hyphens, no consecutive hyphens +var handleRegex = regexp.MustCompile(`^[a-zA-Z0-9]([a-zA-Z0-9-]*[a-zA-Z0-9])?(\.[a-zA-Z0-9]([a-zA-Z0-9-]*[a-zA-Z0-9])?)*$`) + +type userService struct { userRepo UserRepository + pdsURL string // TODO: Support federated PDS - different users may have different PDS hosts } -func NewUserService(userRepo UserRepository) *UserService { - return &UserService{ +// NewUserService creates a new user service +func NewUserService(userRepo UserRepository, pdsURL string) UserService { + return &userService{ userRepo: userRepo, + pdsURL: pdsURL, } } -func (s *UserService) CreateUser(req CreateUserRequest) (*User, error) { +// CreateUser creates a new user in the AppView database +func (s *userService) CreateUser(ctx context.Context, req CreateUserRequest) (*User, error) { if err := s.validateCreateRequest(req); err != nil { return nil, err } - - req.Email = strings.TrimSpace(strings.ToLower(req.Email)) - req.Username = strings.TrimSpace(req.Username) - - existingUser, _ := s.userRepo.GetByEmail(req.Email) - if existingUser != nil { - return nil, fmt.Errorf("service: email already exists") - } - - existingUser, _ = s.userRepo.GetByUsername(req.Username) - if existingUser != nil { - return nil, fmt.Errorf("service: username already exists") - } - + + // Normalize handle + req.Handle = strings.TrimSpace(strings.ToLower(req.Handle)) + req.DID = strings.TrimSpace(req.DID) + user := &User{ - Email: req.Email, - Username: req.Username, + DID: req.DID, + Handle: req.Handle, } - - return s.userRepo.Create(user) + + // Repository will handle duplicate constraint errors + return s.userRepo.Create(ctx, user) } -func (s *UserService) GetUserByID(id int) (*User, error) { - if id <= 0 { - return nil, fmt.Errorf("service: invalid user ID") +// GetUserByDID retrieves a user by their DID +func (s *userService) GetUserByDID(ctx context.Context, did string) (*User, error) { + if strings.TrimSpace(did) == "" { + return nil, fmt.Errorf("DID is required") } - - user, err := s.userRepo.GetByID(id) - if err != nil { - if strings.Contains(err.Error(), "not found") { - return nil, fmt.Errorf("service: user not found") - } - return nil, fmt.Errorf("service: %w", err) - } - - return user, nil + + return s.userRepo.GetByDID(ctx, did) } -func (s *UserService) GetUserByEmail(email string) (*User, error) { - email = strings.TrimSpace(strings.ToLower(email)) - if email == "" { - return nil, fmt.Errorf("service: email is required") +// GetUserByHandle retrieves a user by their handle +func (s *userService) GetUserByHandle(ctx context.Context, handle string) (*User, error) { + handle = strings.TrimSpace(strings.ToLower(handle)) + if handle == "" { + return nil, fmt.Errorf("handle is required") } - - user, err := s.userRepo.GetByEmail(email) - if err != nil { - if strings.Contains(err.Error(), "not found") { - return nil, fmt.Errorf("service: user not found") - } - return nil, fmt.Errorf("service: %w", err) - } - - return user, nil + + return s.userRepo.GetByHandle(ctx, handle) } -func (s *UserService) GetUserByUsername(username string) (*User, error) { - username = strings.TrimSpace(username) - if username == "" { - return nil, fmt.Errorf("service: username is required") +// ResolveHandleToDID resolves a handle to a DID +// This is critical for login: users enter their handle, we resolve to DID +// TODO: Implement actual DNS/HTTPS resolution via atProto +func (s *userService) ResolveHandleToDID(ctx context.Context, handle string) (string, error) { + handle = strings.TrimSpace(strings.ToLower(handle)) + if handle == "" { + return "", fmt.Errorf("handle is required") } - - user, err := s.userRepo.GetByUsername(username) + + // For now, check if user exists in our AppView database + // Later: implement DNS TXT record lookup or HTTPS .well-known/atproto-did + user, err := s.userRepo.GetByHandle(ctx, handle) if err != nil { - if strings.Contains(err.Error(), "not found") { - return nil, fmt.Errorf("service: user not found") - } - return nil, fmt.Errorf("service: %w", err) + return "", fmt.Errorf("failed to resolve handle %s: %w", handle, err) } - - return user, nil -} -func (s *UserService) UpdateUser(id int, req UpdateUserRequest) (*User, error) { - user, err := s.GetUserByID(id) - if err != nil { - return nil, err - } - - if req.Email != "" { - req.Email = strings.TrimSpace(strings.ToLower(req.Email)) - if req.Email != user.Email { - existingUser, _ := s.userRepo.GetByEmail(req.Email) - if existingUser != nil && existingUser.ID != id { - return nil, fmt.Errorf("service: email already exists") - } - } - user.Email = req.Email - } - - if req.Username != "" { - req.Username = strings.TrimSpace(req.Username) - if req.Username != user.Username { - existingUser, _ := s.userRepo.GetByUsername(req.Username) - if existingUser != nil && existingUser.ID != id { - return nil, fmt.Errorf("service: username already exists") - } - } - user.Username = req.Username - } - - return s.userRepo.Update(user) + return user.DID, nil } -func (s *UserService) DeleteUser(id int) error { - if id <= 0 { - return fmt.Errorf("service: invalid user ID") +func (s *userService) validateCreateRequest(req CreateUserRequest) error { + if strings.TrimSpace(req.DID) == "" { + return fmt.Errorf("DID is required") } - - err := s.userRepo.Delete(id) - if err != nil { - if strings.Contains(err.Error(), "not found") { - return fmt.Errorf("service: user not found") - } - return fmt.Errorf("service: %w", err) + + if strings.TrimSpace(req.Handle) == "" { + return fmt.Errorf("handle is required") } - - return nil -} -func (s *UserService) validateCreateRequest(req CreateUserRequest) error { - if strings.TrimSpace(req.Email) == "" { - return fmt.Errorf("service: email is required") + // DID format validation + if !strings.HasPrefix(req.DID, "did:") { + return fmt.Errorf("invalid DID format: must start with 'did:'") } - - if strings.TrimSpace(req.Username) == "" { - return fmt.Errorf("service: username is required") + + // atProto handle validation + handle := strings.TrimSpace(strings.ToLower(req.Handle)) + + // Length validation (1-253 characters per atProto spec) + if len(handle) < 1 || len(handle) > 253 { + return fmt.Errorf("handle must be between 1 and 253 characters") } - - if !strings.Contains(req.Email, "@") { - return fmt.Errorf("service: invalid email format") + + // Regex validation: alphanumeric + hyphens + dots, no consecutive hyphens + if !handleRegex.MatchString(handle) { + return fmt.Errorf("invalid handle format: must contain only alphanumeric characters, hyphens, and dots; must start and end with alphanumeric; no consecutive hyphens") } - - if len(req.Username) < 3 { - return fmt.Errorf("service: username must be at least 3 characters") + + // Check for consecutive hyphens (not allowed in atProto) + if strings.Contains(handle, "--") { + return fmt.Errorf("invalid handle format: consecutive hyphens not allowed") } - - return nil -} -type UpdateUserRequest struct { - Email string `json:"email,omitempty"` - Username string `json:"username,omitempty"` + return nil } \ No newline at end of file diff --git a/internal/core/users/service_test.go b/internal/core/users/service_test.go deleted file mode 100644 --- a/internal/core/users/service_test.go +++ /dev/null @@ -1,272 +0,0 @@ -package users_test - -import ( - "fmt" - "testing" - "time" - - "Coves/internal/core/users" -) - -type mockUserRepository struct { - users map[int]*users.User - nextID int - shouldFail bool -} - -func newMockUserRepository() *mockUserRepository { - return &mockUserRepository{ - users: make(map[int]*users.User), - nextID: 1, - } -} - -func (m *mockUserRepository) Create(user *users.User) (*users.User, error) { - if m.shouldFail { - return nil, fmt.Errorf("mock: database error") - } - - user.ID = m.nextID - m.nextID++ - user.CreatedAt = time.Now() - user.UpdatedAt = time.Now() - - m.users[user.ID] = user - return user, nil -} - -func (m *mockUserRepository) GetByID(id int) (*users.User, error) { - if m.shouldFail { - return nil, fmt.Errorf("mock: database error") - } - - user, exists := m.users[id] - if !exists { - return nil, fmt.Errorf("repository: user not found") - } - return user, nil -} - -func (m *mockUserRepository) GetByEmail(email string) (*users.User, error) { - if m.shouldFail { - return nil, fmt.Errorf("mock: database error") - } - - for _, user := range m.users { - if user.Email == email { - return user, nil - } - } - return nil, fmt.Errorf("repository: user not found") -} - -func (m *mockUserRepository) GetByUsername(username string) (*users.User, error) { - if m.shouldFail { - return nil, fmt.Errorf("mock: database error") - } - - for _, user := range m.users { - if user.Username == username { - return user, nil - } - } - return nil, fmt.Errorf("repository: user not found") -} - -func (m *mockUserRepository) Update(user *users.User) (*users.User, error) { - if m.shouldFail { - return nil, fmt.Errorf("mock: database error") - } - - if _, exists := m.users[user.ID]; !exists { - return nil, fmt.Errorf("repository: user not found") - } - - user.UpdatedAt = time.Now() - m.users[user.ID] = user - return user, nil -} - -func (m *mockUserRepository) Delete(id int) error { - if m.shouldFail { - return fmt.Errorf("mock: database error") - } - - if _, exists := m.users[id]; !exists { - return fmt.Errorf("repository: user not found") - } - - delete(m.users, id) - return nil -} - -func TestCreateUser(t *testing.T) { - repo := newMockUserRepository() - service := users.NewUserService(repo) - - tests := []struct { - name string - req users.CreateUserRequest - wantErr bool - errMsg string - }{ - { - name: "valid user", - req: users.CreateUserRequest{ - Email: "test@example.com", - Username: "testuser", - }, - wantErr: false, - }, - { - name: "empty email", - req: users.CreateUserRequest{ - Email: "", - Username: "testuser", - }, - wantErr: true, - errMsg: "email is required", - }, - { - name: "empty username", - req: users.CreateUserRequest{ - Email: "test@example.com", - Username: "", - }, - wantErr: true, - errMsg: "username is required", - }, - { - name: "invalid email format", - req: users.CreateUserRequest{ - Email: "invalidemail", - Username: "testuser", - }, - wantErr: true, - errMsg: "invalid email format", - }, - { - name: "short username", - req: users.CreateUserRequest{ - Email: "test@example.com", - Username: "ab", - }, - wantErr: true, - errMsg: "username must be at least 3 characters", - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - user, err := service.CreateUser(tt.req) - - if tt.wantErr { - if err == nil { - t.Errorf("expected error but got none") - } else if tt.errMsg != "" && err.Error() != "service: "+tt.errMsg { - t.Errorf("expected error message '%s' but got '%s'", tt.errMsg, err.Error()) - } - } else { - if err != nil { - t.Errorf("unexpected error: %v", err) - } - if user == nil { - t.Errorf("expected user but got nil") - } - } - }) - } -} - -func TestCreateUserDuplicates(t *testing.T) { - repo := newMockUserRepository() - service := users.NewUserService(repo) - - req := users.CreateUserRequest{ - Email: "test@example.com", - Username: "testuser", - } - - _, err := service.CreateUser(req) - if err != nil { - t.Fatalf("unexpected error creating first user: %v", err) - } - - _, err = service.CreateUser(req) - if err == nil { - t.Errorf("expected error for duplicate email but got none") - } else if err.Error() != "service: email already exists" { - t.Errorf("unexpected error message: %v", err) - } - - req2 := users.CreateUserRequest{ - Email: "different@example.com", - Username: "testuser", - } - - _, err = service.CreateUser(req2) - if err == nil { - t.Errorf("expected error for duplicate username but got none") - } else if err.Error() != "service: username already exists" { - t.Errorf("unexpected error message: %v", err) - } -} - -func TestGetUserByID(t *testing.T) { - repo := newMockUserRepository() - service := users.NewUserService(repo) - - createdUser, err := service.CreateUser(users.CreateUserRequest{ - Email: "test@example.com", - Username: "testuser", - }) - if err != nil { - t.Fatalf("failed to create user: %v", err) - } - - tests := []struct { - name string - id int - wantErr bool - errMsg string - }{ - { - name: "valid ID", - id: createdUser.ID, - wantErr: false, - }, - { - name: "invalid ID", - id: 0, - wantErr: true, - errMsg: "invalid user ID", - }, - { - name: "non-existent ID", - id: 999, - wantErr: true, - errMsg: "user not found", - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - user, err := service.GetUserByID(tt.id) - - if tt.wantErr { - if err == nil { - t.Errorf("expected error but got none") - } else if tt.errMsg != "" && err.Error() != "service: "+tt.errMsg { - t.Errorf("expected error message '%s' but got '%s'", tt.errMsg, err.Error()) - } - } else { - if err != nil { - t.Errorf("unexpected error: %v", err) - } - if user == nil { - t.Errorf("expected user but got nil") - } - } - }) - } -} \ No newline at end of file diff --git a/internal/core/users/user.go b/internal/core/users/user.go --- a/internal/core/users/user.go +++ b/internal/core/users/user.go @@ -4,15 +4,18 @@ import ( "time" ) +// User represents an atProto user tracked in the Coves AppView +// This is NOT the user's repository - that lives in the PDS +// This table only tracks metadata for efficient AppView queries type User struct { - ID int `json:"id" db:"id"` - Email string `json:"email" db:"email"` - Username string `json:"username" db:"username"` - CreatedAt time.Time `json:"created_at" db:"created_at"` - UpdatedAt time.Time `json:"updated_at" db:"updated_at"` + DID string `json:"did" db:"did"` // atProto DID (e.g., did:plc:xyz123) + Handle string `json:"handle" db:"handle"` // Human-readable handle (e.g., alice.coves.dev) + CreatedAt time.Time `json:"createdAt" db:"created_at"` + UpdatedAt time.Time `json:"updatedAt" db:"updated_at"` } +// CreateUserRequest represents the input for creating a new user type CreateUserRequest struct { - Email string `json:"email"` - Username string `json:"username"` + DID string `json:"did"` + Handle string `json:"handle"` } diff --git a/internal/db/migrations/001_create_users_table.sql b/internal/db/migrations/001_create_users_table.sql --- a/internal/db/migrations/001_create_users_table.sql +++ b/internal/db/migrations/001_create_users_table.sql @@ -1,14 +1,15 @@ -- +goose Up +-- Create main users table for Coves (all users are atProto users) CREATE TABLE users ( - id SERIAL PRIMARY KEY, - email VARCHAR(255) UNIQUE NOT NULL, - username VARCHAR(50) UNIQUE NOT NULL, + did TEXT PRIMARY KEY, + handle TEXT UNIQUE NOT NULL, created_at TIMESTAMP WITH TIME ZONE DEFAULT CURRENT_TIMESTAMP, updated_at TIMESTAMP WITH TIME ZONE DEFAULT CURRENT_TIMESTAMP ); -CREATE INDEX idx_users_email ON users(email); -CREATE INDEX idx_users_username ON users(username); +-- Indexes for efficient lookups +CREATE INDEX idx_users_handle ON users(handle); +CREATE INDEX idx_users_created_at ON users(created_at); -- +goose Down -DROP TABLE users; \ No newline at end of file +DROP TABLE users; diff --git a/internal/db/migrations/005_add_user_maps_indices.sql b/internal/db/migrations/005_add_user_maps_indices.sql deleted file mode 100644 --- a/internal/db/migrations/005_add_user_maps_indices.sql +++ /dev/null @@ -1,32 +0,0 @@ --- +goose Up --- +goose StatementBegin - --- Note: The user_maps table is created by GORM's AutoMigrate in the carstore package --- Only add indices if the table exists -DO $$ -BEGIN - -- Check if user_maps table exists - IF EXISTS (SELECT 1 FROM information_schema.tables WHERE table_name = 'user_maps') THEN - -- Check if column exists before creating index - IF EXISTS (SELECT 1 FROM information_schema.columns WHERE table_name = 'user_maps' AND column_name = 'did') THEN - -- Explicit column name specified in GORM tag - CREATE INDEX IF NOT EXISTS idx_user_maps_did ON user_maps(did); - END IF; - - -- Add index on created_at if column exists - IF EXISTS (SELECT 1 FROM information_schema.columns WHERE table_name = 'user_maps' AND column_name = 'created_at') THEN - CREATE INDEX IF NOT EXISTS idx_user_maps_created_at ON user_maps(created_at); - END IF; - END IF; -END $$; - --- +goose StatementEnd - --- +goose Down --- +goose StatementBegin - --- Remove indices if they exist -DROP INDEX IF EXISTS idx_user_maps_did; -DROP INDEX IF EXISTS idx_user_maps_created_at; - --- +goose StatementEnd \ No newline at end of file diff --git a/internal/db/postgres/user_repo.go b/internal/db/postgres/user_repo.go --- a/internal/db/postgres/user_repo.go +++ b/internal/db/postgres/user_repo.go @@ -1,123 +1,81 @@ package postgres import ( + "context" "database/sql" "fmt" + "strings" "Coves/internal/core/users" ) -type PostgresUserRepo struct { +type postgresUserRepo struct { db *sql.DB } +// NewUserRepository creates a new PostgreSQL user repository func NewUserRepository(db *sql.DB) users.UserRepository { - return &PostgresUserRepo{db: db} + return &postgresUserRepo{db: db} } -func (r *PostgresUserRepo) Create(user *users.User) (*users.User, error) { +// Create inserts a new user into the users table +func (r *postgresUserRepo) Create(ctx context.Context, user *users.User) (*users.User, error) { query := ` - INSERT INTO users (email, username) - VALUES ($1, $2) - RETURNING id, email, username, created_at, updated_at` + INSERT INTO users (did, handle) + VALUES ($1, $2) + RETURNING did, handle, created_at, updated_at` - err := r.db.QueryRow(query, user.Email, user.Username). - Scan(&user.ID, &user.Email, &user.Username, &user.CreatedAt, &user.UpdatedAt) + err := r.db.QueryRowContext(ctx, query, user.DID, user.Handle). + Scan(&user.DID, &user.Handle, &user.CreatedAt, &user.UpdatedAt) if err != nil { - return nil, fmt.Errorf("repository: failed to create user: %w", err) - } - - return user, nil -} - -func (r *PostgresUserRepo) GetByID(id int) (*users.User, error) { - user := &users.User{} - query := `SELECT id, email, username, created_at, updated_at FROM users WHERE id = $1` - - err := r.db.QueryRow(query, id). - Scan(&user.ID, &user.Email, &user.Username, &user.CreatedAt, &user.UpdatedAt) - - if err == sql.ErrNoRows { - return nil, fmt.Errorf("repository: user not found") - } - if err != nil { - return nil, fmt.Errorf("repository: failed to get user: %w", err) + // Check for unique constraint violations + if strings.Contains(err.Error(), "duplicate key") { + if strings.Contains(err.Error(), "users_pkey") { + return nil, fmt.Errorf("user with DID already exists") + } + if strings.Contains(err.Error(), "users_handle_key") { + return nil, fmt.Errorf("handle already taken") + } + } + return nil, fmt.Errorf("failed to create user: %w", err) } return user, nil } -func (r *PostgresUserRepo) GetByEmail(email string) (*users.User, error) { +// GetByDID retrieves a user by their DID +func (r *postgresUserRepo) GetByDID(ctx context.Context, did string) (*users.User, error) { user := &users.User{} - query := `SELECT id, email, username, created_at, updated_at FROM users WHERE email = $1` + query := `SELECT did, handle, created_at, updated_at FROM users WHERE did = $1` - err := r.db.QueryRow(query, email). - Scan(&user.ID, &user.Email, &user.Username, &user.CreatedAt, &user.UpdatedAt) + err := r.db.QueryRowContext(ctx, query, did). + Scan(&user.DID, &user.Handle, &user.CreatedAt, &user.UpdatedAt) if err == sql.ErrNoRows { - return nil, fmt.Errorf("repository: user not found") + return nil, fmt.Errorf("user not found") } if err != nil { - return nil, fmt.Errorf("repository: failed to get user by email: %w", err) + return nil, fmt.Errorf("failed to get user by DID: %w", err) } return user, nil } -func (r *PostgresUserRepo) GetByUsername(username string) (*users.User, error) { +// GetByHandle retrieves a user by their handle +func (r *postgresUserRepo) GetByHandle(ctx context.Context, handle string) (*users.User, error) { user := &users.User{} - query := `SELECT id, email, username, created_at, updated_at FROM users WHERE username = $1` + query := `SELECT did, handle, created_at, updated_at FROM users WHERE handle = $1` - err := r.db.QueryRow(query, username). - Scan(&user.ID, &user.Email, &user.Username, &user.CreatedAt, &user.UpdatedAt) + err := r.db.QueryRowContext(ctx, query, handle). + Scan(&user.DID, &user.Handle, &user.CreatedAt, &user.UpdatedAt) if err == sql.ErrNoRows { - return nil, fmt.Errorf("repository: user not found") + return nil, fmt.Errorf("user not found") } if err != nil { - return nil, fmt.Errorf("repository: failed to get user by username: %w", err) + return nil, fmt.Errorf("failed to get user by handle: %w", err) } return user, nil } - -func (r *PostgresUserRepo) Update(user *users.User) (*users.User, error) { - query := ` - UPDATE users - SET email = $2, username = $3, updated_at = CURRENT_TIMESTAMP - WHERE id = $1 - RETURNING id, email, username, created_at, updated_at` - - err := r.db.QueryRow(query, user.ID, user.Email, user.Username). - Scan(&user.ID, &user.Email, &user.Username, &user.CreatedAt, &user.UpdatedAt) - - if err == sql.ErrNoRows { - return nil, fmt.Errorf("repository: user not found") - } - if err != nil { - return nil, fmt.Errorf("repository: failed to update user: %w", err) - } - - return user, nil -} - -func (r *PostgresUserRepo) Delete(id int) error { - query := `DELETE FROM users WHERE id = $1` - - result, err := r.db.Exec(query, id) - if err != nil { - return fmt.Errorf("repository: failed to delete user: %w", err) - } - - rowsAffected, err := result.RowsAffected() - if err != nil { - return fmt.Errorf("repository: failed to get rows affected: %w", err) - } - - if rowsAffected == 0 { - return fmt.Errorf("repository: user not found") - } - - return nil -} diff --git a/tests/integration/integration_test.go b/tests/integration/integration_test.go --- a/tests/integration/integration_test.go +++ b/tests/integration/integration_test.go @@ -1,15 +1,14 @@ package integration import ( - "Coves/internal/core/users" - "Coves/internal/db/postgres" - "bytes" + "context" "database/sql" "encoding/json" "fmt" "net/http" "net/http/httptest" "os" + "strings" "testing" "github.com/go-chi/chi/v5" @@ -17,11 +16,12 @@ _ "github.com/lib/pq" "github.com/pressly/goose/v3" "Coves/internal/api/routes" + "Coves/internal/core/users" + "Coves/internal/db/postgres" ) func setupTestDB(t *testing.T) *sql.DB { // Build connection string from environment variables (set by .env.dev) - // These are loaded by the Makefile when running tests testUser := os.Getenv("POSTGRES_TEST_USER") testPassword := os.Getenv("POSTGRES_TEST_PASSWORD") testPort := os.Getenv("POSTGRES_TEST_PORT") @@ -62,7 +62,7 @@ t.Fatalf("Failed to run migrations: %v", err) } // Clean up any existing test data - _, err = db.Exec("DELETE FROM users WHERE email LIKE '%@example.com'") + _, err = db.Exec("DELETE FROM users WHERE handle LIKE '%.test'") if err != nil { t.Logf("Warning: Failed to clean up test data: %v", err) } @@ -70,45 +70,314 @@ return db } -func TestCreateUser(t *testing.T) { +func TestUserCreationAndRetrieval(t *testing.T) { + db := setupTestDB(t) + defer db.Close() + + // Wire up dependencies + userRepo := postgres.NewUserRepository(db) + userService := users.NewUserService(userRepo, "http://localhost:3001") + + ctx := context.Background() + + // Test 1: Create a user + t.Run("Create User", func(t *testing.T) { + req := users.CreateUserRequest{ + DID: "did:plc:test123456", + Handle: "alice.test", + } + + user, err := userService.CreateUser(ctx, req) + if err != nil { + t.Fatalf("Failed to create user: %v", err) + } + + if user.DID != req.DID { + t.Errorf("Expected DID %s, got %s", req.DID, user.DID) + } + + if user.Handle != req.Handle { + t.Errorf("Expected handle %s, got %s", req.Handle, user.Handle) + } + + if user.CreatedAt.IsZero() { + t.Error("CreatedAt should not be zero") + } + }) + + // Test 2: Retrieve user by DID + t.Run("Get User By DID", func(t *testing.T) { + user, err := userService.GetUserByDID(ctx, "did:plc:test123456") + if err != nil { + t.Fatalf("Failed to get user by DID: %v", err) + } + + if user.Handle != "alice.test" { + t.Errorf("Expected handle alice.test, got %s", user.Handle) + } + }) + + // Test 3: Retrieve user by handle + t.Run("Get User By Handle", func(t *testing.T) { + user, err := userService.GetUserByHandle(ctx, "alice.test") + if err != nil { + t.Fatalf("Failed to get user by handle: %v", err) + } + + if user.DID != "did:plc:test123456" { + t.Errorf("Expected DID did:plc:test123456, got %s", user.DID) + } + }) + + // Test 4: Resolve handle to DID + t.Run("Resolve Handle to DID", func(t *testing.T) { + did, err := userService.ResolveHandleToDID(ctx, "alice.test") + if err != nil { + t.Fatalf("Failed to resolve handle: %v", err) + } + + if did != "did:plc:test123456" { + t.Errorf("Expected DID did:plc:test123456, got %s", did) + } + }) +} + +func TestGetProfileEndpoint(t *testing.T) { db := setupTestDB(t) defer db.Close() - // Wire up dependencies according to architecture + // Wire up dependencies userRepo := postgres.NewUserRepository(db) - userService := users.NewUserService(userRepo) + userService := users.NewUserService(userRepo, "http://localhost:3001") + + // Create test user directly in service + ctx := context.Background() + _, err := userService.CreateUser(ctx, users.CreateUserRequest{ + DID: "did:plc:endpoint123", + Handle: "bob.test", + }) + if err != nil { + t.Fatalf("Failed to create test user: %v", err) + } + // Set up HTTP router r := chi.NewRouter() - r.Mount("/api/users", routes.UserRoutes(userService)) + r.Mount("/xrpc/social.coves.actor", routes.UserRoutes(userService)) + + // Test 1: Get profile by DID + t.Run("Get Profile By DID", func(t *testing.T) { + req := httptest.NewRequest("GET", "/xrpc/social.coves.actor/profile?actor=did:plc:endpoint123", nil) + w := httptest.NewRecorder() + r.ServeHTTP(w, req) + + if w.Code != http.StatusOK { + t.Errorf("Expected status %d, got %d. Response: %s", http.StatusOK, w.Code, w.Body.String()) + return + } + + var response map[string]interface{} + if err := json.NewDecoder(w.Body).Decode(&response); err != nil { + t.Fatalf("Failed to decode response: %v", err) + } + + if response["did"] != "did:plc:endpoint123" { + t.Errorf("Expected DID did:plc:endpoint123, got %v", response["did"]) + } + }) + + // Test 2: Get profile by handle + t.Run("Get Profile By Handle", func(t *testing.T) { + req := httptest.NewRequest("GET", "/xrpc/social.coves.actor/profile?actor=bob.test", nil) + w := httptest.NewRecorder() + r.ServeHTTP(w, req) + + if w.Code != http.StatusOK { + t.Errorf("Expected status %d, got %d. Response: %s", http.StatusOK, w.Code, w.Body.String()) + return + } + + var response map[string]interface{} + if err := json.NewDecoder(w.Body).Decode(&response); err != nil { + t.Fatalf("Failed to decode response: %v", err) + } + + profile := response["profile"].(map[string]interface{}) + if profile["handle"] != "bob.test" { + t.Errorf("Expected handle bob.test, got %v", profile["handle"]) + } + }) + + // Test 3: Missing actor parameter + t.Run("Missing Actor Parameter", func(t *testing.T) { + req := httptest.NewRequest("GET", "/xrpc/social.coves.actor/profile", nil) + w := httptest.NewRecorder() + r.ServeHTTP(w, req) + + if w.Code != http.StatusBadRequest { + t.Errorf("Expected status %d, got %d", http.StatusBadRequest, w.Code) + } + }) + + // Test 4: User not found + t.Run("User Not Found", func(t *testing.T) { + req := httptest.NewRequest("GET", "/xrpc/social.coves.actor/profile?actor=nonexistent.test", nil) + w := httptest.NewRecorder() + r.ServeHTTP(w, req) + + if w.Code != http.StatusNotFound { + t.Errorf("Expected status %d, got %d", http.StatusNotFound, w.Code) + } + }) +} + +// TestDuplicateCreation tests that duplicate DID/handle creation fails properly +func TestDuplicateCreation(t *testing.T) { + db := setupTestDB(t) + defer db.Close() + + userRepo := postgres.NewUserRepository(db) + userService := users.NewUserService(userRepo, "http://localhost:3001") + ctx := context.Background() - user := users.CreateUserRequest{ - Email: "test@example.com", - Username: "testuser", + // Create first user + _, err := userService.CreateUser(ctx, users.CreateUserRequest{ + DID: "did:plc:duplicate123", + Handle: "duplicate.test", + }) + if err != nil { + t.Fatalf("Failed to create first user: %v", err) } - body, _ := json.Marshal(user) - req := httptest.NewRequest("POST", "/api/users", bytes.NewBuffer(body)) - req.Header.Set("Content-Type", "application/json") + // Test duplicate DID + t.Run("Duplicate DID", func(t *testing.T) { + _, err := userService.CreateUser(ctx, users.CreateUserRequest{ + DID: "did:plc:duplicate123", + Handle: "different.test", + }) - w := httptest.NewRecorder() - r.ServeHTTP(w, req) + if err == nil { + t.Error("Expected error for duplicate DID, got nil") + } - if w.Code != http.StatusCreated { - t.Errorf("Expected status %d, got %d. Response: %s", http.StatusCreated, w.Code, w.Body.String()) - return - } + if !strings.Contains(err.Error(), "DID already exists") { + t.Errorf("Expected 'DID already exists' error, got: %v", err) + } + }) - var createdUser users.User - if err := json.NewDecoder(w.Body).Decode(&createdUser); err != nil { - t.Fatalf("Failed to decode response: %v", err) - } + // Test duplicate handle + t.Run("Duplicate Handle", func(t *testing.T) { + _, err := userService.CreateUser(ctx, users.CreateUserRequest{ + DID: "did:plc:different456", + Handle: "duplicate.test", + }) + + if err == nil { + t.Error("Expected error for duplicate handle, got nil") + } + + if !strings.Contains(err.Error(), "handle already taken") { + t.Errorf("Expected 'handle already taken' error, got: %v", err) + } + }) +} + +// TestHandleValidation tests atProto handle validation rules +func TestHandleValidation(t *testing.T) { + db := setupTestDB(t) + defer db.Close() - if createdUser.Email != user.Email { - t.Errorf("Expected email %s, got %s", user.Email, createdUser.Email) + userRepo := postgres.NewUserRepository(db) + userService := users.NewUserService(userRepo, "http://localhost:3001") + ctx := context.Background() + + testCases := []struct { + name string + did string + handle string + shouldError bool + errorMsg string + }{ + { + name: "Valid handle with hyphen", + did: "did:plc:valid1", + handle: "alice-bob.test", + shouldError: false, + }, + { + name: "Valid handle with dots", + did: "did:plc:valid2", + handle: "alice.bob.test", + shouldError: false, + }, + { + name: "Invalid: consecutive hyphens", + did: "did:plc:invalid1", + handle: "alice--bob.test", + shouldError: true, + errorMsg: "consecutive hyphens", + }, + { + name: "Invalid: starts with hyphen", + did: "did:plc:invalid2", + handle: "-alice.test", + shouldError: true, + errorMsg: "invalid handle format", + }, + { + name: "Invalid: ends with hyphen", + did: "did:plc:invalid3", + handle: "alice-.test", + shouldError: true, + errorMsg: "invalid handle format", + }, + { + name: "Invalid: special characters", + did: "did:plc:invalid4", + handle: "alice!bob.test", + shouldError: true, + errorMsg: "invalid handle format", + }, + { + name: "Invalid: spaces", + did: "did:plc:invalid5", + handle: "alice bob.test", + shouldError: true, + errorMsg: "invalid handle format", + }, + { + name: "Invalid: too long", + did: "did:plc:invalid6", + handle: strings.Repeat("a", 254) + ".test", + shouldError: true, + errorMsg: "must be between 1 and 253 characters", + }, + { + name: "Invalid: missing DID prefix", + did: "plc:invalid7", + handle: "valid.test", + shouldError: true, + errorMsg: "must start with 'did:'", + }, } - if createdUser.Username != user.Username { - t.Errorf("Expected username %s, got %s", user.Username, createdUser.Username) + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + _, err := userService.CreateUser(ctx, users.CreateUserRequest{ + DID: tc.did, + Handle: tc.handle, + }) + + if tc.shouldError { + if err == nil { + t.Errorf("Expected error, got nil") + } else if !strings.Contains(err.Error(), tc.errorMsg) { + t.Errorf("Expected error containing '%s', got: %v", tc.errorMsg, err) + } + } else { + if err != nil { + t.Errorf("Expected no error, got: %v", err) + } + } + }) } } -