feat(server): add skip_code_list_fetch option to CityGML SpatialID Attributes API #392
Conversation
WalkthroughThe pull request introduces a new parameter Changes
Sequence DiagramsequenceDiagram
participant Client
participant SpatialIDHandler
participant URLReader
participant CodeResolver
Client->>SpatialIDHandler: Request with skipCodeListFetch
SpatialIDHandler->>URLReader: Create with skipCodeListFetch flag
alt skipCodeListFetch is true
URLReader-->>CodeResolver: Skip resolver creation
else skipCodeListFetch is false
URLReader->>CodeResolver: Create resolver normally
end
Possibly related PRs
Suggested reviewers
Poem
Finishing Touches
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: 1
🧹 Nitpick comments (1)
server/citygml/echo.go (1)
139-139: Consider adding documentation for the new parameter.It would be helpful to add a comment explaining the purpose and behavior of the
skip_code_list_fetchparameter for future maintainers.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
server/citygml/echo.go(2 hunks)server/citygml/spatialid_attributes.go(2 hunks)
🧰 Additional context used
🪛 GitHub Actions: ci
server/citygml/echo.go
[error] 189-189: unknown field 'url' in struct literal of type urlReader
🔇 Additional comments (3)
server/citygml/spatialid_attributes.go (2)
25-26: LGTM! Field addition looks good.The new
skipCodeListFetchfield is properly added to theurlReaderstruct with appropriate type and positioning.
70-72: LGTM! Conditional resolver logic is correct.The implementation correctly bypasses code list fetching when
skipCodeListFetchis true.server/citygml/echo.go (1)
139-139: LGTM! Query parameter handling looks good.The implementation correctly extracts the
skip_code_list_fetchparameter from the query string.
server/citygml/echo.go
Outdated
| @@ -185,7 +186,7 @@ func spatialIDAttributesHandler(dc *dataCatalogAPI) echo.HandlerFunc { | |||
| rs := make([]Reader, 0, len(urls)) | |||
| etagCache := make(map[string]string) | |||
| for _, u := range urls { | |||
| rs = append(rs, &urlReader{URL: u, client: httpClient, etagCache: etagCache}) | |||
| rs = append(rs, &urlReader{url: u, client: httpClient, etagCache: etagCache, skipCodeListFetch: skipCodeListFetch}) | |||
There was a problem hiding this comment.
Fix field name in struct initialization.
There's a field name mismatch in the struct initialization. The field name should be URL (uppercase) instead of url (lowercase) as defined in the urlReader struct.
Apply this fix:
- rs = append(rs, &urlReader{url: u, client: httpClient, etagCache: etagCache, skipCodeListFetch: skipCodeListFetch})
+ rs = append(rs, &urlReader{URL: u, client: httpClient, etagCache: etagCache, skipCodeListFetch: skipCodeListFetch})Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 GitHub Actions: ci
[error] 189-189: unknown field 'url' in struct literal of type urlReader
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
server/citygml/echo.go (2)
139-139: Consider stricter validation for skip_code_list_fetch parameter.The current implementation (
!= "") accepts any non-empty value as true. Consider using explicit boolean values for better clarity and validation.-skipCodeListFetch := c.QueryParam("skip_code_list_fetch") != "" +skipCodeListFetch := strings.EqualFold(c.QueryParam("skip_code_list_fetch"), "true")
139-139: Document the skip_code_list_fetch parameter.The new parameter lacks documentation explaining its purpose, usage, and implications. Consider:
- Adding comments explaining when to use this parameter
- Documenting the expected values and behavior
- Updating API documentation if it exists
Also applies to: 189-189
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
server/citygml/echo.go(2 hunks)
🔇 Additional comments (1)
server/citygml/echo.go (1)
189-189: LGTM! Proper initialization of urlReader with skipCodeListFetch parameter.The implementation correctly initializes the urlReader with all required fields, including the new skipCodeListFetch parameter.
Summary by CodeRabbit
New Features
skipCodeListFetchto control code list fetching behaviorImprovements