Skip to content

chore: remove dbmem #18803

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jul 9, 2025
+143 −15,000
Merged

chore: remove dbmem #18803

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
abbd3b9
remove dbmem
hugodutka Jul 8, 2025
b14e99c
make gen
hugodutka Jul 8, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 0 additions & 39 deletions .claude/docs/DATABASE.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,31 +58,6 @@ If adding fields to auditable types:
- `ActionSecret`: Field contains sensitive data
3. Run `make gen` to verify no audit errors

## In-Memory Database (dbmem) Updates

### Critical Requirements

When adding new fields to database structs:

- **CRITICAL**: Update `coderd/database/dbmem/dbmem.go` in-memory implementations
- The `Insert*` functions must include ALL new fields, not just basic ones
- Common issue: Tests pass with real database but fail with in-memory database due to missing field mappings
- Always verify in-memory database functions match the real database schema after migrations

### Example Pattern

```go
// In dbmem.go - ensure ALL fields are included
code := database.OAuth2ProviderAppCode{
ID: arg.ID,
CreatedAt: arg.CreatedAt,
// ... existing fields ...
ResourceUri: arg.ResourceUri, // New field
CodeChallenge: arg.CodeChallenge, // New field
CodeChallengeMethod: arg.CodeChallengeMethod, // New field
}
```

## Database Architecture

### Core Components
Expand Down Expand Up @@ -116,7 +91,6 @@ roles, err := db.GetAuthorizationUserRoles(dbauthz.AsSystemRestricted(ctx), user

1. **Nullable field errors**: Use `sql.Null*` types consistently
2. **Missing audit entries**: Update `enterprise/audit/table.go`
3. **dbmem inconsistencies**: Ensure in-memory implementations match schema

### Query Issues

Expand All @@ -139,19 +113,6 @@ func TestDatabaseFunction(t *testing.T) {
}
```

### In-Memory Testing

```go
func TestInMemoryDatabase(t *testing.T) {
db := dbmem.New()

// Test with in-memory database
result, err := db.GetSomething(ctx, param)
require.NoError(t, err)
require.Equal(t, expected, result)
}
```

## Best Practices

### Schema Design
Expand Down
15 changes: 7 additions & 8 deletions .claude/docs/OAUTH2.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,14 +112,13 @@ Always run the full test suite after OAuth2 changes:
## Common OAuth2 Issues

1. **OAuth2 endpoints returning wrong error format** - Ensure OAuth2 endpoints return RFC 6749 compliant errors
2. **OAuth2 tests failing but scripts working** - Check in-memory database implementations in `dbmem.go`
3. **Resource indicator validation failing** - Ensure database stores and retrieves resource parameters correctly
4. **PKCE tests failing** - Verify both authorization code storage and token exchange handle PKCE fields
5. **RFC compliance failures** - Verify against actual RFC specifications, not assumptions
6. **Authorization context errors in public endpoints** - Use `dbauthz.AsSystemRestricted(ctx)` pattern
7. **Default value mismatches** - Ensure database migrations match application code defaults
8. **Bearer token authentication issues** - Check token extraction precedence and format validation
9. **URI validation failures** - Support both standard schemes and custom schemes per protocol requirements
2. **Resource indicator validation failing** - Ensure database stores and retrieves resource parameters correctly
3. **PKCE tests failing** - Verify both authorization code storage and token exchange handle PKCE fields
4. **RFC compliance failures** - Verify against actual RFC specifications, not assumptions
5. **Authorization context errors in public endpoints** - Use `dbauthz.AsSystemRestricted(ctx)` pattern
6. **Default value mismatches** - Ensure database migrations match application code defaults
7. **Bearer token authentication issues** - Check token extraction precedence and format validation
8. **URI validation failures** - Support both standard schemes and custom schemes per protocol requirements

## Authorization Context Patterns

Expand Down
35 changes: 4 additions & 31 deletions .claude/docs/TESTING.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,31 +39,6 @@
2. **Verify information disclosure protections**
3. **Test token security and proper invalidation**

## Database Testing

### In-Memory Database Testing

When adding new database fields:

- **CRITICAL**: Update `coderd/database/dbmem/dbmem.go` in-memory implementations
- The `Insert*` functions must include ALL new fields, not just basic ones
- Common issue: Tests pass with real database but fail with in-memory database due to missing field mappings
- Always verify in-memory database functions match the real database schema after migrations

Example pattern:

```go
// In dbmem.go - ensure ALL fields are included
code := database.OAuth2ProviderAppCode{
ID: arg.ID,
CreatedAt: arg.CreatedAt,
// ... existing fields ...
ResourceUri: arg.ResourceUri, // New field
CodeChallenge: arg.CodeChallenge, // New field
CodeChallengeMethod: arg.CodeChallengeMethod, // New field
}
```

## Test Organization

### Test File Structure
Expand Down Expand Up @@ -107,15 +82,13 @@ coderd/

### Database-Related

1. **Tests passing locally but failing in CI** - Check if `dbmem` implementation needs updating
2. **SQL type errors** - Use `sql.Null*` types for nullable fields
3. **Race conditions in tests** - Use unique identifiers instead of hardcoded names
1. **SQL type errors** - Use `sql.Null*` types for nullable fields
2. **Race conditions in tests** - Use unique identifiers instead of hardcoded names

### OAuth2 Testing

1. **OAuth2 tests failing but scripts working** - Check in-memory database implementations in `dbmem.go`
2. **PKCE tests failing** - Verify both authorization code storage and token exchange handle PKCE fields
3. **Resource indicator validation failing** - Ensure database stores and retrieves resource parameters correctly
1. **PKCE tests failing** - Verify both authorization code storage and token exchange handle PKCE fields
2. **Resource indicator validation failing** - Ensure database stores and retrieves resource parameters correctly

### General Issues

Expand Down
33 changes: 12 additions & 21 deletions .claude/docs/TROUBLESHOOTING.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,59 +21,50 @@
}
```

3. **Tests passing locally but failing in CI**
- **Solution**: Check if `dbmem` implementation needs updating
- Update `coderd/database/dbmem/dbmem.go` for Insert/Update methods
- Missing fields in dbmem can cause tests to fail even if main implementation is correct

### Testing Issues

4. **"package should be X_test"**
3. **"package should be X_test"**
- **Solution**: Use `package_test` naming for test files
- Example: `identityprovider_test` for black-box testing

5. **Race conditions in tests**
4. **Race conditions in tests**
- **Solution**: Use unique identifiers instead of hardcoded names
- Example: `fmt.Sprintf("test-client-%s-%d", t.Name(), time.Now().UnixNano())`
- Never use hardcoded names in concurrent tests

6. **Missing newlines**
5. **Missing newlines**
- **Solution**: Ensure files end with newline character
- Most editors can be configured to add this automatically

### OAuth2 Issues

7. **OAuth2 endpoints returning wrong error format**
6. **OAuth2 endpoints returning wrong error format**
- **Solution**: Ensure OAuth2 endpoints return RFC 6749 compliant errors
- Use standard error codes: `invalid_client`, `invalid_grant`, `invalid_request`
- Format: `{"error": "code", "error_description": "details"}`

8. **OAuth2 tests failing but scripts working**
- **Solution**: Check in-memory database implementations in `dbmem.go`
- Ensure all OAuth2 fields are properly copied in Insert/Update methods

9. **Resource indicator validation failing**
7. **Resource indicator validation failing**
- **Solution**: Ensure database stores and retrieves resource parameters correctly
- Check both authorization code storage and token exchange handling

10. **PKCE tests failing**
8. **PKCE tests failing**
- **Solution**: Verify both authorization code storage and token exchange handle PKCE fields
- Check `CodeChallenge` and `CodeChallengeMethod` field handling

### RFC Compliance Issues

11. **RFC compliance failures**
9. **RFC compliance failures**
- **Solution**: Verify against actual RFC specifications, not assumptions
- Use WebFetch tool to get current RFC content for compliance verification
- Read the actual RFC specifications before implementation

12. **Default value mismatches**
10. **Default value mismatches**
- **Solution**: Ensure database migrations match application code defaults
- Example: RFC 7591 specifies `client_secret_basic` as default, not `client_secret_post`

### Authorization Issues

13. **Authorization context errors in public endpoints**
11. **Authorization context errors in public endpoints**
- **Solution**: Use `dbauthz.AsSystemRestricted(ctx)` pattern
- Example:

Expand All @@ -84,17 +75,17 @@

### Authentication Issues

14. **Bearer token authentication issues**
12. **Bearer token authentication issues**
- **Solution**: Check token extraction precedence and format validation
- Ensure proper RFC 6750 Bearer Token Support implementation

15. **URI validation failures**
13. **URI validation failures**
- **Solution**: Support both standard schemes and custom schemes per protocol requirements
- Native OAuth2 apps may use custom schemes

### General Development Issues

16. **Log message formatting errors**
14. **Log message formatting errors**
- **Solution**: Use lowercase, descriptive messages without special characters
- Follow Go logging conventions

Expand Down
10 changes: 2 additions & 8 deletions .claude/docs/WORKFLOWS.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,11 +81,6 @@
- Add each new field with appropriate action (ActionTrack, ActionIgnore, ActionSecret)
- Run `make gen` to verify no audit errors

6. **In-memory database (dbmem) updates**:
- When adding new fields to database structs, ensure `dbmem` implementation copies all fields
- Check `coderd/database/dbmem/dbmem.go` for Insert/Update methods
- Missing fields in dbmem can cause tests to fail even if main implementation is correct

### Database Generation Process

1. Modify SQL files in `coderd/database/queries/`
Expand Down Expand Up @@ -164,9 +159,8 @@

1. **Development server won't start** - Use `./scripts/develop.sh` instead of manual commands
2. **Database migration errors** - Check migration file format and use helper scripts
3. **Test failures after database changes** - Update `dbmem` implementations
4. **Audit table errors** - Update `enterprise/audit/table.go` with new fields
5. **OAuth2 compliance issues** - Ensure RFC-compliant error responses
3. **Audit table errors** - Update `enterprise/audit/table.go` with new fields
4. **OAuth2 compliance issues** - Ensure RFC-compliant error responses

### Debug Commands

Expand Down
Loading
Loading