LNK-3288: Combined swagger spec for all services in BFF #582
Conversation
… include their operations in the BFF's spec. TODO: Still need to include schemas from proxy'd services.
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThe pull request introduces a new Changes
Assessment against linked issues
Sequence DiagramsequenceDiagram
participant BFF as BFF Swagger Generator
participant ServiceRegistry as Service Registry
participant ServiceA as Service A
participant ServiceB as Service B
BFF->>ServiceRegistry: Retrieve service URLs
ServiceRegistry-->>BFF: Return service URLs
BFF->>ServiceA: Fetch Swagger Specification
ServiceA-->>BFF: Return Swagger Spec
BFF->>ServiceB: Fetch Swagger Specification
ServiceB-->>BFF: Return Swagger Spec
BFF->>BFF: Merge Service Specifications
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
DotNet/LinkAdmin.BFF/Application/Swagger/ServiceSpecAppender.cs (1)
16-26: Investigate error handling for swagger specification retrieval
If the swagger JSON is malformed, the current method might throw. Consider adding more robust error handling (beyond caching) to handle parse failures gracefully.DotNet/LinkAdmin.BFF/Program.cs (1)
16-16: Unused import?
The import "System.Reflection.Metadata" might be unused. Consider removing it if it's not required.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
DotNet/LinkAdmin.BFF/Application/Swagger/ServiceSpecAppender.cs(1 hunks)DotNet/LinkAdmin.BFF/LinkAdmin.BFF.csproj(1 hunks)DotNet/LinkAdmin.BFF/Program.cs(3 hunks)DotNet/Shared/Application/Models/Configs/ServiceRegistry.cs(1 hunks)
🔇 Additional comments (4)
DotNet/LinkAdmin.BFF/Application/Swagger/ServiceSpecAppender.cs (1)
9-9: Review the use of primary constructor feature
The class declaration uses a primary constructor-like syntax. While this is valid from C# 12 onwards, please ensure your project’s language version is set to support this feature if you’re targeting older frameworks or compilers.
✅ Verification successful
Primary constructor usage is appropriate
The project targets .NET 8.0 (<TargetFramework>net8.0</TargetFramework>), which includes C# 12 by default. Primary constructors are fully supported in this environment, so the syntax used in the code is appropriate and safe to use.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that the project is using C# version 12 or later to support the primary constructor feature.
grep -A 5 "<LangVersion>" DotNet/LinkAdmin.BFF/LinkAdmin.BFF.csproj
Length of output: 69
Script:
#!/bin/bash
# Let's first check if the file exists and see its content
cat DotNet/LinkAdmin.BFF/LinkAdmin.BFF.csproj
Length of output: 2747
DotNet/Shared/Application/Models/Configs/ServiceRegistry.cs (1)
19-19: Audit potential usage of ValidationServiceUrl
While adding this property provides a new configuration point, ensure that validation service calls, if implemented, handle potential null or empty strings and provide proper fallback or exception handling.
✅ Verification successful
Let me check the implementation of the service spec appender to understand how the ValidationServiceUrl is used.
Let me try a different approach to check the implementation using ripgrep.
Let me check the complete implementation of the Apply method where ValidationServiceUrl is used.
Based on the evidence gathered, I can now provide the final response:
ValidationServiceUrl is properly handled with null checks
The ValidationServiceUrl is only used in ServiceSpecAppender where it's properly guarded with a null check using string.IsNullOrEmpty() before usage, following the same pattern as other service URLs in the class. The property's null-forgiving operator (null!) is appropriate as it's a required configuration value that should be set after initialization.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify references to ValidationServiceUrl in the codebase to ensure proper usage.
rg -A 3 'ValidationServiceUrl'
Length of output: 1041
Script:
#!/bin/bash
# Get the implementation of ServiceSpecAppender
ast-grep --pattern 'class ServiceSpecAppender {
$$$
}'
Length of output: 58
Script:
#!/bin/bash
# Get the implementation details of ServiceSpecAppender
rg -A 20 "class ServiceSpecAppender"
Length of output: 2144
Script:
#!/bin/bash
# Get the implementation of Apply method in ServiceSpecAppender
rg -A 10 "public void Apply" DotNet/LinkAdmin.BFF/Application/Swagger/ServiceSpecAppender.cs
Length of output: 679
DotNet/LinkAdmin.BFF/Program.cs (2)
29-29: Ensure correct namespace usage
The newly added namespace reference "LantanaGroup.Link.LinkAdmin.BFF.Application.Swagger" is valid for referencing the document filter. This looks good, so ensure no circular dependencies.
308-308: Excellent addition of ServiceSpecAppender to the Swagger configuration
Registering the custom document filter ensures that proxied services’ Swagger specs are appended to the final document, fulfilling the PR objective.
DotNet/LinkAdmin.BFF/Application/Swagger/ServiceSpecAppender.cs
Outdated
Show resolved
Hide resolved
DotNet/LinkAdmin.BFF/Application/Swagger/ServiceSpecAppender.cs
Outdated
Show resolved
Hide resolved
DotNet/LinkAdmin.BFF/Application/Swagger/ServiceSpecAppender.cs
Outdated
Show resolved
Hide resolved
# Conflicts: # DotNet/Admin.BFF/Program.cs
… other services. Updating BFF service spec appender to correct which services are java vs. DotNet
* Moving details about submission folder to the functionality section * Providing information about how YARP configuration works in service spec
# Conflicts: # docker-compose.yml
Updating Microsoft.OpenApi.Readers package to newer version.
seanmcilvenna
changed the title
🛠️ Description of Changes
Modifications to BFF to gather swagger specs for proxy'd services and include their operations in the BFF's spec.
🧪 Testing Performed
Please describe the testing that was performed on the changes included in this PR.
📓 Documentation Updated
Please update any relevant sections in the project documentation that were impacted by the changes in the PR.