|
| 1 | +# Design Document: Pre-Release Fixes |
| 2 | + |
| 3 | +## Overview |
| 4 | + |
| 5 | +This design addresses critical issues and improvements identified during pre-release review of the Oproto Lambda OpenAPI source generator. The changes span the source generator, MSBuild task, package configuration, and documentation. |
| 6 | + |
| 7 | +## Architecture |
| 8 | + |
| 9 | +The library consists of three main components: |
| 10 | + |
| 11 | +``` |
| 12 | +┌─────────────────────────────────────────────────────────────────┐ |
| 13 | +│ Consumer Project Build │ |
| 14 | +├─────────────────────────────────────────────────────────────────┤ |
| 15 | +│ 1. Compilation Phase │ |
| 16 | +│ ┌──────────────────────────────────────────┐ │ |
| 17 | +│ │ Roslyn Compiler + Source Generator │ │ |
| 18 | +│ │ - Analyzes Lambda function classes │ │ |
| 19 | +│ │ - Generates OpenApiOutput.g.cs │ │ |
| 20 | +│ │ - Embeds JSON as assembly attribute │ │ |
| 21 | +│ └──────────────────────────────────────────┘ │ |
| 22 | +│ │ │ |
| 23 | +│ ▼ │ |
| 24 | +│ 2. Post-Build Phase (MSBuild Task) │ |
| 25 | +│ ┌──────────────────────────────────────────┐ │ |
| 26 | +│ │ ExtractOpenApiSpecTask │ │ |
| 27 | +│ │ - Strategy 1: Parse .g.cs file (AOT) │ │ |
| 28 | +│ │ - Strategy 2: Reflection (non-AOT) │ │ |
| 29 | +│ │ - Writes openapi.json │ │ |
| 30 | +│ └──────────────────────────────────────────┘ │ |
| 31 | +└─────────────────────────────────────────────────────────────────┘ |
| 32 | +``` |
| 33 | + |
| 34 | +## Components and Interfaces |
| 35 | + |
| 36 | +### 1. Source Generator Changes |
| 37 | + |
| 38 | +#### Task<T> Unwrapping |
| 39 | + |
| 40 | +Add a method to unwrap async return types: |
| 41 | + |
| 42 | +```csharp |
| 43 | +// OpenApiSpecGenerator.cs |
| 44 | +private ITypeSymbol UnwrapAsyncType(ITypeSymbol typeSymbol) |
| 45 | +{ |
| 46 | + if (typeSymbol is INamedTypeSymbol namedType && namedType.IsGenericType) |
| 47 | + { |
| 48 | + var typeName = namedType.ConstructedFrom.ToDisplayString(); |
| 49 | + if (typeName == "System.Threading.Tasks.Task<T>" || |
| 50 | + typeName == "System.Threading.Tasks.ValueTask<T>") |
| 51 | + { |
| 52 | + return namedType.TypeArguments[0]; |
| 53 | + } |
| 54 | + } |
| 55 | + |
| 56 | + // Check for non-generic Task/ValueTask |
| 57 | + var displayName = typeSymbol.ToDisplayString(); |
| 58 | + if (displayName == "System.Threading.Tasks.Task" || |
| 59 | + displayName == "System.Threading.Tasks.ValueTask") |
| 60 | + { |
| 61 | + return null; // Indicates void/no content |
| 62 | + } |
| 63 | + |
| 64 | + return typeSymbol; |
| 65 | +} |
| 66 | +``` |
| 67 | + |
| 68 | +#### Debug Statement Cleanup |
| 69 | + |
| 70 | +Remove all `Console.WriteLine` calls. Wrap meaningful `Debug.WriteLine` calls: |
| 71 | + |
| 72 | +```csharp |
| 73 | +#if DEBUG |
| 74 | + Debug.WriteLine($"Processing class: {classSymbol.Name}"); |
| 75 | +#endif |
| 76 | +``` |
| 77 | + |
| 78 | +#### HTTP Method Completeness |
| 79 | + |
| 80 | +Update `GenerateOpenApiDocument` to handle all HTTP methods: |
| 81 | + |
| 82 | +```csharp |
| 83 | +switch (endpoint.HttpMethod.ToUpperInvariant()) |
| 84 | +{ |
| 85 | + case "GET": path.Operations[OperationType.Get] = operation; break; |
| 86 | + case "POST": path.Operations[OperationType.Post] = operation; break; |
| 87 | + case "PUT": path.Operations[OperationType.Put] = operation; break; |
| 88 | + case "DELETE": path.Operations[OperationType.Delete] = operation; break; |
| 89 | + case "PATCH": path.Operations[OperationType.Patch] = operation; break; |
| 90 | + case "HEAD": path.Operations[OperationType.Head] = operation; break; |
| 91 | + case "OPTIONS": path.Operations[OperationType.Options] = operation; break; |
| 92 | +} |
| 93 | +``` |
| 94 | + |
| 95 | +### 2. Security Scheme Attributes |
| 96 | + |
| 97 | +New attributes for configurable security: |
| 98 | + |
| 99 | +```csharp |
| 100 | +// OpenApiSecuritySchemeAttribute.cs |
| 101 | +[AttributeUsage(AttributeTargets.Assembly, AllowMultiple = true)] |
| 102 | +public class OpenApiSecuritySchemeAttribute : Attribute |
| 103 | +{ |
| 104 | + public string SchemeId { get; } |
| 105 | + public SecuritySchemeType Type { get; set; } |
| 106 | + |
| 107 | + // For API Key |
| 108 | + public string ApiKeyName { get; set; } |
| 109 | + public ApiKeyLocation ApiKeyLocation { get; set; } |
| 110 | + |
| 111 | + // For OAuth2 |
| 112 | + public string AuthorizationUrl { get; set; } |
| 113 | + public string TokenUrl { get; set; } |
| 114 | + public string Scopes { get; set; } // Comma-separated "scope1:desc1,scope2:desc2" |
| 115 | + |
| 116 | + public OpenApiSecuritySchemeAttribute(string schemeId) => SchemeId = schemeId; |
| 117 | +} |
| 118 | + |
| 119 | +public enum SecuritySchemeType { ApiKey, OAuth2, OpenIdConnect, Http } |
| 120 | +public enum ApiKeyLocation { Header, Query, Cookie } |
| 121 | +``` |
| 122 | + |
| 123 | +Usage: |
| 124 | +```csharp |
| 125 | +[assembly: OpenApiSecurityScheme("apiKey", |
| 126 | + Type = SecuritySchemeType.ApiKey, |
| 127 | + ApiKeyName = "x-api-key", |
| 128 | + ApiKeyLocation = ApiKeyLocation.Header)] |
| 129 | + |
| 130 | +[assembly: OpenApiSecurityScheme("oauth2", |
| 131 | + Type = SecuritySchemeType.OAuth2, |
| 132 | + AuthorizationUrl = "https://auth.example.com/authorize", |
| 133 | + TokenUrl = "https://auth.example.com/token", |
| 134 | + Scopes = "read:Read access,write:Write access")] |
| 135 | +``` |
| 136 | + |
| 137 | +### 3. MSBuild Task AOT Support |
| 138 | + |
| 139 | +Update `ExtractOpenApiSpecTask` with dual extraction strategy: |
| 140 | + |
| 141 | +```csharp |
| 142 | +public override bool Execute() |
| 143 | +{ |
| 144 | + // Strategy 1: Try parsing generated source file (AOT-compatible) |
| 145 | + var generatedFilePath = FindGeneratedSourceFile(); |
| 146 | + if (generatedFilePath != null && File.Exists(generatedFilePath)) |
| 147 | + { |
| 148 | + var json = ExtractJsonFromSourceFile(generatedFilePath); |
| 149 | + if (json != null) |
| 150 | + { |
| 151 | + WriteOpenApiSpec(json); |
| 152 | + return true; |
| 153 | + } |
| 154 | + } |
| 155 | + |
| 156 | + // Strategy 2: Fall back to reflection (non-AOT) |
| 157 | + try |
| 158 | + { |
| 159 | + var json = ExtractJsonViaReflection(); |
| 160 | + WriteOpenApiSpec(json); |
| 161 | + return true; |
| 162 | + } |
| 163 | + catch (Exception ex) |
| 164 | + { |
| 165 | + Log.LogError($"Failed to extract OpenAPI spec. For AOT builds, enable " + |
| 166 | + "EmitCompilerGeneratedFiles in your project. Error: {ex.Message}"); |
| 167 | + return false; |
| 168 | + } |
| 169 | +} |
| 170 | + |
| 171 | +private string FindGeneratedSourceFile() |
| 172 | +{ |
| 173 | + // Look in standard generated files location |
| 174 | + var searchPaths = new[] |
| 175 | + { |
| 176 | + Path.Combine(Path.GetDirectoryName(AssemblyPath), "..", "obj", |
| 177 | + "GeneratedFiles", "Oproto.Lambda.OpenApi.SourceGenerator", |
| 178 | + "Oproto.Lambda.OpenApi.SourceGenerator.OpenApiSpecGenerator", |
| 179 | + "OpenApiOutput.g.cs"), |
| 180 | + // Also check if path is provided via property |
| 181 | + }; |
| 182 | + return searchPaths.FirstOrDefault(File.Exists); |
| 183 | +} |
| 184 | + |
| 185 | +private string ExtractJsonFromSourceFile(string filePath) |
| 186 | +{ |
| 187 | + var content = File.ReadAllText(filePath); |
| 188 | + // Parse: [assembly: OpenApiOutput(@"...", "openapi.json")] |
| 189 | + var match = Regex.Match(content, |
| 190 | + @"\[assembly:\s*OpenApiOutput\s*\(\s*@""(.+?)""\s*,", |
| 191 | + RegexOptions.Singleline); |
| 192 | + if (match.Success) |
| 193 | + { |
| 194 | + return match.Groups[1].Value.Replace("\"\"", "\""); |
| 195 | + } |
| 196 | + return null; |
| 197 | +} |
| 198 | +``` |
| 199 | + |
| 200 | +### 4. Package Configuration Changes |
| 201 | + |
| 202 | +#### Version Alignment (SourceGenerator.csproj) |
| 203 | + |
| 204 | +Remove hardcoded version: |
| 205 | +```xml |
| 206 | +<!-- Remove this line --> |
| 207 | +<Version>1.0.0</Version> |
| 208 | +``` |
| 209 | + |
| 210 | +#### Dependency Updates |
| 211 | + |
| 212 | +```xml |
| 213 | +<!-- Oproto.Lambda.OpenApi.Build.csproj --> |
| 214 | +<PackageReference Include="Microsoft.Build.Utilities.Core" Version="17.12.6" /> |
| 215 | + |
| 216 | +<!-- Oproto.Lambda.OpenApi.SourceGenerator.csproj --> |
| 217 | +<PackageReference Include="System.Text.Json" Version="8.0.5" /> |
| 218 | +``` |
| 219 | + |
| 220 | +### 5. Dead Code Removal |
| 221 | + |
| 222 | +Remove the following unused code: |
| 223 | + |
| 224 | +| Item | Location | Action | |
| 225 | +|------|----------|--------| |
| 226 | +| `GenerateOpenApiSpecAttribute` | Attributes/ | Remove (not used by generator) | |
| 227 | +| `GetLambdaClassInfo` method | OpenApiSpecGenerator.cs | Remove | |
| 228 | +| `GetApiInfo` method | OpenApiSpecGenerator.cs | Remove | |
| 229 | +| `ResponseTypeInfo` class | SourceGenerator/ | Remove (not populated) | |
| 230 | +| `ExampleInfo` class | SourceGenerator/ | Remove (not populated) | |
| 231 | +| `ResponseTypes` property | EndpointInfo.cs | Remove | |
| 232 | +| `Examples` property | DocumentationInfo.cs | Remove | |
| 233 | + |
| 234 | +## Data Models |
| 235 | + |
| 236 | +### Security Scheme Configuration |
| 237 | + |
| 238 | +```csharp |
| 239 | +internal class SecuritySchemeConfig |
| 240 | +{ |
| 241 | + public string SchemeId { get; set; } |
| 242 | + public SecuritySchemeType Type { get; set; } |
| 243 | + public string ApiKeyName { get; set; } |
| 244 | + public ApiKeyLocation? ApiKeyLocation { get; set; } |
| 245 | + public string AuthorizationUrl { get; set; } |
| 246 | + public string TokenUrl { get; set; } |
| 247 | + public Dictionary<string, string> Scopes { get; set; } |
| 248 | +} |
| 249 | +``` |
| 250 | + |
| 251 | +## Correctness Properties |
| 252 | + |
| 253 | +*A property is a characteristic or behavior that should hold true across all valid executions of a system-essentially, a formal statement about what the system should do. Properties serve as the bridge between human-readable specifications and machine-verifiable correctness guarantees.* |
| 254 | + |
| 255 | +### Property 1: Task<T> Unwrapping Preserves Inner Type |
| 256 | + |
| 257 | +*For any* Lambda function with return type `Task<T>` or `ValueTask<T>`, the generated OpenAPI response schema SHALL be equivalent to the schema that would be generated for return type `T` directly. |
| 258 | + |
| 259 | +**Validates: Requirements 1.1, 1.3, 1.4** |
| 260 | + |
| 261 | +### Property 2: Security Scheme Attribute Round-Trip |
| 262 | + |
| 263 | +*For any* valid `OpenApiSecuritySchemeAttribute` configuration, the generated OpenAPI security scheme definition SHALL contain all specified properties with their exact values. |
| 264 | + |
| 265 | +**Validates: Requirements 4.1, 4.3, 4.4, 4.5** |
| 266 | + |
| 267 | +### Property 3: Generated Source File JSON Extraction |
| 268 | + |
| 269 | +*For any* valid `OpenApiOutput.g.cs` file generated by the source generator, parsing the file to extract the JSON string SHALL produce output identical to the JSON embedded in the assembly attribute. |
| 270 | + |
| 271 | +**Validates: Requirements 11.2, 11.4** |
| 272 | + |
| 273 | +### Property 4: HTTP Method Mapping Completeness |
| 274 | + |
| 275 | +*For any* Lambda function with a valid HTTP method attribute (GET, POST, PUT, DELETE, PATCH, HEAD, OPTIONS), the generated OpenAPI specification SHALL include an operation under the corresponding lowercase method key. |
| 276 | + |
| 277 | +**Validates: Requirements 6.1, 6.2, 6.3** |
| 278 | + |
| 279 | +## Error Handling |
| 280 | + |
| 281 | +| Scenario | Behavior | |
| 282 | +|----------|----------| |
| 283 | +| Task without generic argument | Generate response with no content schema | |
| 284 | +| Unknown HTTP method | Log warning, skip endpoint | |
| 285 | +| Invalid security scheme config | Log error, skip scheme | |
| 286 | +| Generated file not found (AOT) | Fall back to reflection | |
| 287 | +| Reflection fails (AOT) | Log error with EmitCompilerGeneratedFiles instructions | |
| 288 | +| Malformed generated file | Log error, fail extraction | |
| 289 | + |
| 290 | +## Testing Strategy |
| 291 | + |
| 292 | +### Property-Based Testing |
| 293 | + |
| 294 | +Use **FsCheck** (or **Hedgehog** for C#) for property-based tests: |
| 295 | + |
| 296 | +1. **Task<T> Unwrapping Property Test** |
| 297 | + - Generate random type structures |
| 298 | + - Wrap in Task<T>/ValueTask<T> |
| 299 | + - Verify schema equivalence |
| 300 | + |
| 301 | +2. **Security Scheme Round-Trip Property Test** |
| 302 | + - Generate random valid security configurations |
| 303 | + - Verify all properties appear in output |
| 304 | + |
| 305 | +3. **JSON Extraction Property Test** |
| 306 | + - Generate valid OpenAPI JSON strings |
| 307 | + - Create mock .g.cs file content |
| 308 | + - Verify extraction produces identical JSON |
| 309 | + |
| 310 | +### Unit Tests |
| 311 | + |
| 312 | +| Test | Description | |
| 313 | +|------|-------------| |
| 314 | +| `Task_String_UnwrapsToString` | Task<string> produces string schema | |
| 315 | +| `Task_ComplexType_UnwrapsToComplexType` | Task<Order> produces Order schema | |
| 316 | +| `Task_NonGeneric_ProducesNoContent` | Task produces empty response | |
| 317 | +| `ValueTask_UnwrapsCorrectly` | ValueTask<T> behaves like Task<T> | |
| 318 | +| `Patch_Method_GeneratesCorrectOperation` | PATCH produces "patch" key | |
| 319 | +| `Head_Method_GeneratesCorrectOperation` | HEAD produces "head" key | |
| 320 | +| `Options_Method_GeneratesCorrectOperation` | OPTIONS produces "options" key | |
| 321 | +| `NoSecurityAttributes_NoSecuritySchemes` | Empty security when no attributes | |
| 322 | +| `ApiKeyAttribute_GeneratesApiKeyScheme` | API key config maps correctly | |
| 323 | +| `OAuth2Attribute_GeneratesOAuth2Scheme` | OAuth2 config maps correctly | |
| 324 | +| `ExtractFromGeneratedFile_MatchesReflection` | Both extraction methods produce same result | |
| 325 | +| `ExtractFromGeneratedFile_HandlesEscapedQuotes` | Escaped quotes in JSON handled | |
| 326 | +| `NoGeneratedFile_FallsBackToReflection` | Fallback behavior works | |
| 327 | + |
| 328 | +### Integration Tests |
| 329 | + |
| 330 | +| Test | Description | |
| 331 | +|------|-------------| |
| 332 | +| `FullBuild_WithEmitGeneratedFiles_ExtractsSpec` | End-to-end with AOT-compatible path | |
| 333 | +| `FullBuild_WithoutEmitGeneratedFiles_ExtractsSpec` | End-to-end with reflection path | |
0 commit comments