3.9 KiB
3.9 KiB
Decisions — Club Work Manager
Architectural choices and technical decisions made during implementation
Decision 1: Comma-Separated Clubs Format (2026-03-05)
Context
Keycloak sends clubs as comma-separated UUIDs instead of JSON dictionary, but the system expected dictionary format {"uuid": "role"}.
Decision: Accept Comma-Separated UUIDs, Lookup Roles from Database
Rationale:
- Cleaner claim format (less space in JWT)
- Single source of truth: Member table in database
- Decouples authorization from Keycloak configuration
- Simplifies Keycloak setup (no complex mapper needed)
Implementation:
- TenantValidationMiddleware: Split comma-separated string, validate requested tenant
- ClubRoleClaimsTransformation: Split clubs claim, lookup Member role from database
- Both use synchronous
.FirstOrDefault()for consistency and performance
Implications:
- Database must be accessible during authentication (fast query with indexed user + tenant)
- Role changes require database update (not JWT refresh)
- Members table is authoritative for role assignment
Decision 2: Synchronous Database Query in IClaimsTransformation (2026-03-05)
Context
Initially implemented async database query with FirstOrDefaultAsync() in IClaimsTransformation.
Decision: Use Synchronous .FirstOrDefault() Query
Rationale:
- IClaimsTransformation.TransformAsync() must return Task
- Hot reload (dotnet watch) fails when making method async (ENC0098)
- Synchronous query is acceptable: single database lookup, minimal blocking
- Avoids async/await complexity in authentication pipeline
Implications:
- Slightly less async but prevents hot reload issues
- Query performance is milliseconds (indexed on ExternalUserId + TenantId)
- Error handling via try/catch returns null role (fallback behavior)
Decision 3: Entity Framework Connection Interceptor Architecture (2026-03-05)
Context
Attempted to set PostgreSQL session variable (SET LOCAL app.current_tenant_id) in ConnectionOpeningAsync but failed with "Connection is not open" exception.
Decision: Move SQL Execution to ConnectionOpened Lifecycle Phase
Rationale:
ConnectionOpeningAsyncexecutes before actual connection establishmentConnectionOpenedis guaranteed to have an open connection- Synchronous execution is acceptable in ConnectionOpened callback
- Logging/validation can happen in ConnectionOpeningAsync without SQL
Implementation:
- ConnectionOpeningAsync: Only logs warnings about missing tenant
- ConnectionOpened: Executes SET LOCAL command synchronously
- Both methods check for tenant context, only opened executes SQL
Implications:
- Tenant isolation guaranteed at connection open time
- RLS (Row-Level Security) policies see correct tenant_id
- Error handling via try/catch with logging
- Synchronous operation in callback is expected pattern
Decision 4: actions/upload-artifact@v3 Over v4 for Gitea Compatibility (2026-03-06)
Context
Gitea CI pipeline needs artifact uploads for test results and build logs on failure. GitHub Actions has v3 and v4 of upload-artifact available.
Decision: Use actions/upload-artifact@v3
Rationale:
- v3: Stable across Gitea 1.18-1.22+ (verified by community reports)
- v4: Breaking changes in cache format (requires Gitea 1.21+)
- Project may deploy to various Gitea instances (internal/external)
- CI reliability > performance improvement (~30% upload speed gain in v4)
Tradeoffs Considered:
- v4 Performance: 30% faster uploads, better compression
- v3 Compatibility: Works on wider range of Gitea versions
- Decision: Prioritize compatibility for this infrastructure-critical workflow
Implications:
- Slightly slower artifact uploads (non-critical for failure-only uploads)
- If Gitea version known to be 1.21+, can upgrade to v4
- Document decision to prevent confusion during future reviews