BE-636-api | .proto and related generated files for pools endpoint #575
Conversation
Adds relevant .proto and generated files for /pools endpoint.
deividaspetraitis
requested review from
cryptomatictrader,
PaddyMc and
p0mvn
WalkthroughThe changes introduce several new files and functionalities for handling HTTP requests and gRPC services related to pool data. Key features include the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
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
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Outside diff range and nitpick comments (11)
proto/sqs/query/v1beta1/sort.proto (2)
6-10: Enhance enum documentation and consider adding explicit default.While the enum is functionally correct, consider these improvements:
- Enhance documentation to explain the default behavior when no direction is specified
- Document that ASCENDING (0) is the default value per proto3 semantics
- Consider adding an explicit SORT_DIRECTION_UNSPECIFIED = 0 field for clarity
// SortDirection represents the direction for sorting. +// If unspecified, ASCENDING order is used as the default. enum SortDirection { - ASCENDING = 0; - DESCENDING = 1; + // SORT_DIRECTION_UNSPECIFIED represents an unspecified sort direction, + // resulting in default ASCENDING order + SORT_DIRECTION_UNSPECIFIED = 0; + // ASCENDING sorts in ascending order + ASCENDING = 1; + // DESCENDING sorts in descending order + DESCENDING = 2; }
12-19: Enhance SortField documentation with constraints and examples.The message structure is correct, but the documentation could be more helpful for API consumers:
- Document valid field names that can be used for sorting
- Add examples in comments
- Consider adding validation rules or allowed values annotation
-// SortField represents a field to sort by, including its direction. +// SortField represents a field to sort by, including its direction. +// Valid field names for pools endpoint include: "liquidity", "volume_24h", "fee", etc. +// Example usage: +// { +// "field": "liquidity", +// "direction": "DESCENDING" +// } message SortField { // field is the name of the field to sort by. + // Must be a valid field name from the available sorting options. string field = 1; // direction is a sorting direction: ASCENDING or DESCENDING. + // If unspecified, defaults to ASCENDING. SortDirection direction = 2; }pkg/api/v1beta1/sort.go (2)
9-11: Consider improving constant naming and documentation.While using a constant is good practice, consider:
- A more descriptive name like
QueryParamSortto better indicate its purpose as a query parameter- Adding a documentation comment explaining its usage
const ( - querySort = "sort" + // QueryParamSort is the query parameter key used for specifying sort criteria + QueryParamSort = "sort" )
18-19: Fix typo in documentation comment.There's a typo in "implements".
-// UnmarshalHTTPRequest imlpements RequestUnmarshaler interface. +// UnmarshalHTTPRequest implements RequestUnmarshaler interface.proto/sqs/pools/v1beta1/pools.proto (2)
9-15: Consider adding documentation for individual enum valuesWhile the enum itself is documented, adding descriptions for each incentive type would improve API clarity.
// IncentiveType represents the type of incentive for the pool. enum IncentiveType { - SUPERFLUID = 0; - OSMOSIS = 1; - BOOST = 2; - NONE = 3; + // SUPERFLUID represents pools eligible for superfluid staking + SUPERFLUID = 0; + // OSMOSIS represents pools with standard Osmosis incentives + OSMOSIS = 1; + // BOOST represents pools with boosted rewards + BOOST = 2; + // NONE represents pools without any incentives + NONE = 3; }
26-28: Consider using an enum for pool typesThe
typefield usesuint64which seems overly broad for pool types. Consider defining an enum to explicitly list valid pool types, improving API clarity and type safety.+// PoolType represents the different types of pools available +enum PoolType { + // Ensure to document each pool type + BALANCER = 0; + STABLESWAP = 1; + // Add other pool types as needed +} message GetPoolsRequestFilter { // ... - repeated uint64 type = 3; + repeated PoolType type = 3; // ... }pkg/api/v1beta1/pools/http.go (5)
31-58: Ensure Error Consistency inUnmarshalHTTPRequestConsider providing more context in the error messages returned from the
UnmarshalHTTPRequestmethod to aid in debugging. Currently, errors fromfilter.UnmarshalHTTPRequest,pagination.UnmarshalHTTPRequest, andsort.UnmarshalHTTPRequestare returned as-is, which might not indicate the source of the error.Apply this diff to wrap returned errors with additional context:
func (r *GetPoolsRequest) UnmarshalHTTPRequest(c echo.Context) error { if filter := new(GetPoolsRequestFilter); filter.IsPresent(c) { if err := filter.UnmarshalHTTPRequest(c); err != nil { - return err + return fmt.Errorf("failed to unmarshal filter: %w", err) } r.Filter = filter } if pagination := new(v1beta1.PaginationRequest); pagination.IsPresent(c) { if err := pagination.UnmarshalHTTPRequest(c); err != nil { - return err + return fmt.Errorf("failed to unmarshal pagination: %w", err) } r.Pagination = pagination } if sort := new(v1beta1.SortRequest); sort.IsPresent(c) { if err := sort.UnmarshalHTTPRequest(c); err != nil { - return err + return fmt.Errorf("failed to unmarshal sort: %w", err) } r.Sort = sort } return nil }
75-159: Refactor Deprecated and New Query Parameter ParsingThe
UnmarshalHTTPRequestmethod forGetPoolsRequestFiltercontains duplicated code for handling both deprecated and new query parameters. This can be refactored to improve maintainability and reduce potential errors.Propose refactoring by creating helper functions to handle the parsing logic for each parameter type. Here's how you might refactor the parsing of
PoolIdandPoolIdNotIn:func (r *GetPoolsRequestFilter) UnmarshalHTTPRequest(c echo.Context) error { var err error - // Deprecated: use filter[ID] - r.PoolId, err = number.ParseNumbers(c.QueryParam(queryIDs)) - if err != nil { - return err - } - - // Parse query filter by ID - id, err := number.ParseNumbers(c.QueryParam(queryFilterID)) - if err != nil { - return err - } - r.PoolId = append(r.PoolId, id...) + // Parse Pool IDs (including deprecated parameters) + if err := r.parsePoolIDs(c); err != nil { + return err + } // ... (similar changes for other parameters) return nil } +func (r *GetPoolsRequestFilter) parsePoolIDs(c echo.Context) error { + var err error + // Deprecated: use filter[id] + deprecatedIDs, err := number.ParseNumbers(c.QueryParam(queryIDs)) + if err != nil { + return err + } + newIDs, err := number.ParseNumbers(c.QueryParam(queryFilterID)) + if err != nil { + return err + } + r.PoolId = append(deprecatedIDs, newIDs...) + return nil +}Repeat this approach for other parameters like
MinLiquidityCap,WithMarketIncentives, etc.
152-154: Provide Specific Error Message for Long Search QueriesThe error message returned when the search query exceeds the maximum length is generic.
Consider providing a more specific error message that includes the maximum allowed length:
if len(p) > maxSearchQueryLength { - return fmt.Errorf("search query is too long") + return fmt.Errorf("search query exceeds maximum length of %d characters", maxSearchQueryLength) } r.Search = p
138-149: Simplify Parsing of Boolean Query ParametersCurrently, the code separately handles deprecated and new parameters for
WithMarketIncentives. This can be simplified.Refactor the code to unify the parsing logic:
// Parse query filter with market incentives -// Deprecated: use filter[with_market_incentives] -if p := c.QueryParam(queryWithMarketIncentives); p != "" { - r.WithMarketIncentives, err = http.ParseBooleanQueryParam(c, queryWithMarketIncentives) - if err != nil { - return err - } -} -if p := c.QueryParam(queryFilterWithMarketIncentives); p != "" { - r.WithMarketIncentives, err = http.ParseBooleanQueryParam(c, queryFilterWithMarketIncentives) +if p := c.QueryParam(queryFilterWithMarketIncentives); p != "" { + paramName := queryFilterWithMarketIncentives +} else { + // Deprecated: use filter[with_market_incentives] + paramName := queryWithMarketIncentives + p = c.QueryParam(paramName) +} +if p != "" { + r.WithMarketIncentives, err = http.ParseBooleanQueryParam(c, paramName) if err != nil { return err } }
78-159: Add Unit Tests for Query Parameter ParsingThe complexity of the
UnmarshalHTTPRequestmethod warrants thorough unit testing to ensure all query parameters are parsed correctly, including deprecated ones.Would you like assistance in creating unit tests for the
GetPoolsRequestFilterparsing logic?
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
⛔ Files ignored due to path filters (9)
pkg/api/v1beta1/meta.pb.gois excluded by!**/*.pb.gopkg/api/v1beta1/pools/http_test.gois excluded by!**/*_test.gopkg/api/v1beta1/pools/pools.pb.gois excluded by!**/*.pb.gopkg/api/v1beta1/pools/query.pb.gois excluded by!**/*.pb.gopkg/api/v1beta1/pools/query.pb.gw.gois excluded by!**/*.pb.gw.gopkg/api/v1beta1/pools/service.pb.gois excluded by!**/*.pb.gopkg/api/v1beta1/pools/service.pb.gw.gois excluded by!**/*.pb.gw.gopkg/api/v1beta1/sort.pb.gois excluded by!**/*.pb.gopkg/api/v1beta1/sort_test.gois excluded by!**/*_test.go
📒 Files selected for processing (6)
pkg/api/v1beta1/pools/http.go(1 hunks)pkg/api/v1beta1/sort.go(1 hunks)proto/sqs/pools/v1beta1/pools.proto(1 hunks)proto/sqs/pools/v1beta1/query.proto(1 hunks)proto/sqs/query/v1beta1/meta.proto(1 hunks)proto/sqs/query/v1beta1/sort.proto(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- proto/sqs/query/v1beta1/meta.proto
🧰 Additional context used
🪛 buf (1.47.2)
proto/sqs/pools/v1beta1/pools.proto
4-4: import "sqs/query/v1beta1/pagination.proto": file does not exist
(COMPILE)
proto/sqs/pools/v1beta1/query.proto
4-4: import "google/api/annotations.proto": file does not exist
(COMPILE)
🔇 Additional comments (11)
proto/sqs/query/v1beta1/sort.proto (2)
1-5: LGTM! Package declaration and setup follow best practices.
The file setup with proto3 syntax and package declarations are well-structured.
1-25: Verify integration with pools endpoint and generated code.
Since this is part of the pools endpoint implementation, let's verify:
- The integration with GetPoolsRequest
- The generated code usage in the handlers
✅ Verification successful
Integration with pools endpoint and generated code is properly implemented
The verification shows that:
- The
SortRequestis integrated withGetPoolsRequestinpkg/api/v1beta1/pools/pools.pb.go - Generated code includes proper marshaling/unmarshaling implementations
- HTTP request handling is implemented in
sort.gowith corresponding tests insort_test.go - The pools endpoint correctly uses the sorting functionality through
GetPoolsRequest.Sortfield
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check integration with pools proto and handler code
echo "Checking pools proto integration..."
rg -t proto "SortRequest" proto/sqs/pools
echo "Checking handler implementation..."
rg -t go "SortRequest" pkg/api
Length of output: 3062
proto/sqs/pools/v1beta1/query.proto (2)
15-15: LGTM!
The Go package option follows standard conventions and maintains consistency with the protobuf package name.
4-14: Ensure build environment includes required protobuf dependencies
The import google/api/annotations.proto is required for HTTP annotations but appears to be missing. This is a standard protobuf dependency that should be included in your build environment.
🧰 Tools
🪛 buf (1.47.2)
4-4: import "google/api/annotations.proto": file does not exist
(COMPILE)
pkg/api/v1beta1/sort.go (2)
1-7: LGTM! Clean and minimal imports.
The package name and imports are well-organized, using only necessary dependencies.
1-47: Verify Protocol Buffer integration.
Let's ensure the implementation aligns with the Protocol Buffer definitions for sorting.
✅ Verification successful
Protocol Buffer integration is correctly implemented
The implementation in pkg/api/v1beta1/sort.go perfectly aligns with the Protocol Buffer definitions:
- The
SortRequeststruct matches the proto message definition with itsfieldsfield - The
SortDirectionenum values (ASCENDING = 0, DESCENDING = 1) are correctly used - The
SortFieldimplementation matches the proto definition withfieldanddirectionfields - The sorting functionality is properly integrated into
GetPoolsRequestas field test mainnet for Grant #3
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify Protocol Buffer definitions match the implementation
# Check for sort-related message definitions
rg -A 10 "message.*Sort.*{" --type proto
# Check for SortDirection enum definition
rg -A 5 "enum.*SortDirection" --type proto
# Check for sort field usage in GetPoolsRequest
rg -A 10 "message.*GetPoolsRequest" --type proto
Length of output: 294
Script:
#!/bin/bash
# Let's try a different approach to find proto files and their content
# First, find proto files
fd "\.proto$"
# Then search for sort-related definitions in all files (without file type restriction)
rg -A 10 "message\s+Sort"
# Search for SortDirection enum
rg -A 5 "enum\s+SortDirection"
# Search for GetPoolsRequest message
rg -A 10 "message\s+GetPoolsRequest"
Length of output: 2911
proto/sqs/pools/v1beta1/pools.proto (2)
43-53: LGTM! Well-structured request message
The GetPoolsRequest message effectively combines filtering, pagination, and sorting capabilities as required by the PR objectives.
4-5: Verify the import paths for pagination and sort protos
The static analysis indicates that sqs/query/v1beta1/pagination.proto file is missing. Please ensure that all imported proto files exist in the correct paths.
✅ Verification successful
Import paths are correctly configured and files exist
The verification shows that both imported proto files exist in the correct paths:
sqs/query/v1beta1/pagination.protosqs/query/v1beta1/sort.proto
These files are properly imported in proto/sqs/pools/v1beta1/pools.proto and are also used consistently in other proto files within the codebase.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if the imported proto files exist
fd -e proto -x echo "Checking {}..." \; -x test -f {} \; \
"pagination.proto" "sort.proto"
Length of output: 188
Script:
#!/bin/bash
# Search for proto files in the repository
echo "Searching for proto files..."
fd -e proto
# Search for specific imports in proto files
echo -e "\nSearching for imports..."
rg "import.*pagination.proto|import.*sort.proto" -g "*.proto"
Length of output: 733
🧰 Tools
🪛 buf (1.47.2)
4-4: import "sqs/query/v1beta1/pagination.proto": file does not exist
(COMPILE)
pkg/api/v1beta1/pools/http.go (3)
138-149: Potential Overwriting of WithMarketIncentives
The code first assigns WithMarketIncentives using the deprecated parameter, then overwrites it if the new parameter is present.
Check if this behavior is intentional. If both deprecated and new parameters are provided, which one should take precedence?
Consider defining a clear precedence between deprecated and new parameters or handling conflicts explicitly.
105-116: Validate Incentive Type Parsing
The parsing of IncentiveType relies on IncentiveType_name map for validation. Ensure that this map is populated correctly at runtime to prevent invalid values from passing through.
Run the following script to verify the contents of IncentiveType_name:
If necessary, I can help refactor the validation to use a predefined set of acceptable values.
✅ Verification successful
IncentiveType validation is properly implemented
The validation is correctly implemented as the IncentiveType_name map is auto-generated from protocol buffers and contains all valid enum values: SUPERFLUID (0), OSMOSIS (1), BOOST (2), and NONE (3). The current validation approach using this map is a reliable way to ensure only valid enum values are accepted.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check that IncentiveType_name contains the expected values.
# Test: Search for the declaration of IncentiveType_name.
rg 'IncentiveType_name' -A 5
# Expected: The map should contain all valid IncentiveType keys and values.
Length of output: 1602
79-90:
Potential Duplicate Pool IDs
When parsing both deprecated (IDs) and new (filter[id]) query parameters, the code appends both results to r.PoolId. This could lead to duplicate Pool IDs in the filter.
Run the following script to check for duplicates in r.PoolId:
If duplicates are possible, consider modifying the code to remove duplicates:
r.PoolId = append(r.PoolId, id...)
+// Remove duplicate IDs
+uniquePoolIds := make(map[uint64]struct{})
+for _, id := range r.PoolId {
+ uniquePoolIds[id] = struct{}{}
+}
+r.PoolId = make([]uint64, 0, len(uniquePoolIds))
+for id := range uniquePoolIds {
+ r.PoolId = append(r.PoolId, id)
+}
|
Note that sonarcloud is complaining about low coverage rate becaues of generated files from |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (1)
pkg/api/v1beta1/sort.go (1)
28-28: Fix typo in documentation.There's a typo in the interface name.
-// UnmarshalHTTPRequest imlpements RequestUnmarshaler interface. +// UnmarshalHTTPRequest implements RequestUnmarshaler interface.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
⛔ Files ignored due to path filters (2)
pkg/api/v1beta1/sort.pb.gois excluded by!**/*.pb.gopkg/api/v1beta1/sort_test.gois excluded by!**/*_test.go
📒 Files selected for processing (2)
pkg/api/v1beta1/sort.go(1 hunks)proto/sqs/query/v1beta1/sort.proto(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- proto/sqs/query/v1beta1/sort.proto
🔇 Additional comments (2)
pkg/api/v1beta1/sort.go (2)
23-26: LGTM!
The method correctly checks for the presence of the sort parameter in the request.
41-59:
Add input validation and security measures.
The current implementation lacks important validation checks:
- No validation of field names could allow injection attacks
- Unbounded number of sort fields could lead to DoS
- No validation of empty field names after trimming prefix
Add these safeguards:
fields := strings.Split(sortParam, ",")
+
+ // Limit number of sort fields
+ if len(fields) > 5 {
+ return fmt.Errorf("too many sort fields (max 5)")
+ }
for _, field := range fields {
var direction SortDirection
if strings.HasPrefix(field, "-") {
direction = SortDirection_DESCENDING
field = strings.TrimPrefix(field, "-")
} else {
direction = SortDirection_ASCENDING
}
+ // Validate field name is not empty
+ if field == "" {
+ return fmt.Errorf("empty sort field")
+ }
+
+ // Validate field name against allowed list
+ if !isAllowedSortField(field) {
+ return fmt.Errorf("invalid sort field: %s", field)
+ }
+
r.Fields = append(r.Fields, &SortField{Add this helper function:
// isAllowedSortField validates if the given field name is in the allowlist
func isAllowedSortField(field string) bool {
// Add fields based on your pools endpoint requirements
allowedFields := map[string]bool{
"id": true,
"type": true,
"created_at": true,
// Add other allowed fields
}
return allowedFields[field]
}
pkg/api/v1beta1/sort.go
Outdated
| var ( | ||
| ErrSortFieldTooLong = fmt.Errorf("too many sort fields") | ||
| ) |
There was a problem hiding this comment.
Fix inconsistent error message.
The error message "too many sort fields" is misleading as ErrSortFieldTooLong is used when the sort parameter length exceeds MaxSortLength, not when there are too many fields.
- ErrSortFieldTooLong = fmt.Errorf("too many sort fields")
+ ErrSortFieldTooLong = fmt.Errorf("sort parameter exceeds maximum length of %d characters", MaxSortLength)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| var ( | |
| ErrSortFieldTooLong = fmt.Errorf("too many sort fields") | |
| ) | |
| var ( | |
| ErrSortFieldTooLong = fmt.Errorf("sort parameter exceeds maximum length of %d characters", MaxSortLength) | |
| ) |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (3)
pkg/api/v1beta1/sort.go (3)
4-4: Simplify fmt import.The explicit name for the fmt import is redundant as it matches the package name.
- fmt "fmt" + "fmt"
19-21: Add documentation for querySort constant.Add a comment explaining the purpose of the querySort constant, similar to MaxSortLength.
const ( + // querySort is the HTTP query parameter name used for sort operations. querySort = "sort" )
28-28: Fix typo in documentation.There's a typo in the interface name.
-// UnmarshalHTTPRequest imlpements RequestUnmarshaler interface. +// UnmarshalHTTPRequest implements RequestUnmarshaler interface.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
pkg/api/v1beta1/sort.go(1 hunks)
🔇 Additional comments (2)
pkg/api/v1beta1/sort.go (2)
23-26: LGTM!
The method is well documented and correctly implements the presence check for the sort parameter.
41-59:
Add field validation and limits.
The current implementation needs additional security measures:
- No validation of field names could allow injection attacks
- No limit on the number of sort fields could lead to performance issues
- Field names should be validated against an allowlist
Add these safeguards:
fields := strings.Split(sortParam, ",")
+
+ // Limit number of sort fields
+ if len(fields) > 5 {
+ return fmt.Errorf("too many sort fields (max 5)")
+ }
for _, field := range fields {
+ // Validate field name against allowlist
+ if !isAllowedSortField(field) {
+ return fmt.Errorf("invalid sort field: %s", field)
+ }
+
var direction SortDirectionAdditionally, consider adding:
- A constant for maximum number of sort fields
- An allowlist of valid sort fields
- Documentation of allowed fields in API docs
Would you like me to provide implementations for these components?
Likely invalid or redundant comment.
| package v1beta1 | ||
|
|
||
| import ( | ||
| fmt "fmt" | ||
| "strings" | ||
|
|
||
| "github.com/labstack/echo/v4" | ||
| ) | ||
|
|
||
| var ( | ||
| ErrSortFieldTooLong = fmt.Errorf("sort parameter exceeds maximum length of %d characters", MaxSortLength) | ||
| ) | ||
|
|
||
| const ( | ||
| // MaxSortLength is the maximum length of the sort query parameter. | ||
| MaxSortLength = 1000 | ||
| ) | ||
|
|
||
| const ( | ||
| querySort = "sort" | ||
| ) | ||
|
|
||
| // IsPresent checks if the sort request is present in the HTTP request. | ||
| func (r *SortRequest) IsPresent(c echo.Context) bool { | ||
| return c.QueryParam(querySort) != "" | ||
| } | ||
|
|
||
| // UnmarshalHTTPRequest imlpements RequestUnmarshaler interface. | ||
| func (r *SortRequest) UnmarshalHTTPRequest(c echo.Context) error { | ||
| // Retrieve the `sort` query parameter | ||
| sortParam := c.QueryParam(querySort) | ||
| if sortParam == "" { | ||
| return nil // No sort parameter provided, return early | ||
| } | ||
|
|
||
| // Prevent extremely long input | ||
| if len(sortParam) > MaxSortLength { | ||
| return ErrSortFieldTooLong | ||
| } | ||
|
|
||
| // Split the `sort` parameter by commas to get individual fields | ||
| fields := strings.Split(sortParam, ",") | ||
|
|
||
| // Parse each field and determine sort direction | ||
| for _, field := range fields { | ||
| var direction SortDirection | ||
| if strings.HasPrefix(field, "-") { | ||
| direction = SortDirection_DESCENDING | ||
| field = strings.TrimPrefix(field, "-") | ||
| } else { | ||
| direction = SortDirection_ASCENDING | ||
| } | ||
|
|
||
| // Append parsed field and direction to SortRequest's Fields | ||
| r.Fields = append(r.Fields, &SortField{ | ||
| Field: field, | ||
| Direction: direction, | ||
| }) | ||
| } | ||
|
|
||
| return nil | ||
| } |
There was a problem hiding this comment.
💡 Codebase verification
Based on the gathered information, I can now generate the final response:
Sort implementation needs to be connected to pools endpoint
The sort functionality is properly implemented in pkg/api/v1beta1/sort.go, but it's not yet integrated with the pools endpoint. Specifically:
- The proto definition in
proto/sqs/pools/v1beta1/pools.protoincludes the sort field inGetPoolsRequest - However, the HTTP handler in
pools/delivery/http/pools_handler.godoesn't process the sort parameter from the request
Required changes:
- Update
GetPoolshandler to parse and apply the sort parameter using theSortRequestimplementation - Pass the sort criteria to the pools usecase layer for proper sorting of results
🔗 Analysis chain
Verify integration with pools endpoint.
Let's verify that this sorting implementation is properly integrated with the pools endpoint and that the field names match the expected pool attributes.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check integration with pools endpoint and verify field names
# Search for pool-related sort field definitions
echo "Searching for pool sort field definitions..."
rg -l 'sort.*field.*pool' --type proto
# Search for usage of SortRequest in pools handler
echo "Checking usage in pools handler..."
rg -l 'SortRequest.*pools' --type go
Length of output: 287
Script:
#!/bin/bash
# Let's try a different approach to find the integration points
# Search for proto files that might contain pool-related definitions
echo "Looking for proto files..."
fd '\.proto$'
# Search for pool-related handlers or endpoints
echo "Searching for pool handlers..."
rg -A 5 '/pools' --type go
# Search for any usage of SortRequest
echo "Searching for SortRequest usage..."
rg -A 5 'SortRequest' --type go
# Look for pool-related model or type definitions
echo "Searching for pool type definitions..."
ast-grep --pattern 'type Pool struct {
$$$
}'
Length of output: 27792
Script:
#!/bin/bash
# Let's check the pools endpoint implementation and sort field usage
# Check the pools proto definition for sortable fields
echo "Checking pools proto definition..."
cat proto/sqs/pools/v1beta1/pools.proto
# Check the pools handler implementation for sort handling
echo "Checking pools handler implementation..."
cat pools/delivery/http/pools_handler.go
# Look for pool sorting implementation
echo "Looking for pool sorting implementation..."
rg -A 5 'func.*Sort.*Pool' --type go
Length of output: 14532
|
This PR represents a smaller, more focused part of bigger PR #553 implementing sorting, filtering, pagination and search functionality for /pools endpoint. For bigger implementation picture refer to mentioned PR and for more context please refer to BE-636.
PR adds relevant .proto and their generated files for /pools endpoint such as:
GetPoolsRequestused in handler to Unmarshal data from request.GetPoolsFilterused to Unmarshal passed filters via HTTP in handler and passing it down the chain for implementing actual filtering logicSortRequestused in pools endpoint handler implemeting Unmarshal of sort parameters from HTTP request and passed down the call chain for specifiying pools list sorting behaviour.Summary by CodeRabbit
Release Notes
New Features
Improvements
Documentation