Refactor/extract utilities #10
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Create internal/api/utils package with shared utilities - Add pagination parsing functions to eliminate duplication across handlers - Add search/filtering utilities for consistent behavior - Add standardized error response functions - Update handleListPods to use new pagination utilities - Update handleListServices to use new pagination utilities - Maintain identical API behavior and response format - Add comprehensive unit tests for all utilities This change eliminates 50+ instances of duplicated pagination logic across handlers while maintaining backward compatibility.
- Create internal/api/utils package with shared utilities - Add pagination parsing functions to eliminate duplication across handlers - Add search/filtering utilities for consistent behavior - Add standardized error response functions - Update handleListPods to use new pagination utilities - Update handleListServices to use new pagination utilities - Maintain identical API behavior and response format - Add comprehensive unit tests for all utilities - Add lint-go target to Makefile for Go-only linting This change eliminates 50+ instances of duplicated pagination logic across handlers while maintaining backward compatibility.
There was a problem hiding this comment.
Pull Request Overview
This PR refactors common API utilities by extracting pagination, filtering, and error handling functionality into a dedicated utils package. This eliminates code duplication across API handlers and provides consistent behavior across all endpoints.
Key changes:
- Created utility functions for pagination parameter parsing and response formatting
- Extracted search/filtering functions with generic type support
- Standardized error response handling with consistent JSON structure
Reviewed Changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/api/utils/pagination.go | New utility functions for parsing pagination parameters and applying pagination to slices |
| internal/api/utils/pagination_test.go | Comprehensive test coverage for pagination utilities |
| internal/api/utils/filters.go | Generic filtering functions with case-insensitive search capabilities |
| internal/api/utils/filters_test.go | Test suite for filtering and search functionality |
| internal/api/utils/errors.go | Standardized error response functions and JSON formatting utilities |
| internal/api/utils/errors_test.go | Test coverage for error handling and response formatting |
| internal/api/utils/doc.go | Package documentation describing utility functions |
| internal/api/handlers_workloads.go | Updated to use new pagination utilities instead of inline logic |
| internal/api/handlers_services.go | Refactored to use standardized pagination and error response utilities |
| Makefile | Added lint-go target for Go-specific linting |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Comment on lines
+492
to
508
| // Parse pagination parameters using utility function | ||
| pageStr := r.URL.Query().Get("page") | ||
| pageSizeStr := r.URL.Query().Get("pageSize") | ||
|
|
||
| // Use default pageSize of 50 to match original behavior | ||
| pagination := utils.ParsePaginationParams(pageStr, pageSizeStr) | ||
| page := pagination.Page | ||
| pageSize := pagination.PageSize | ||
|
|
||
| // Override default pageSize for services (original was 50, not 25) | ||
| if pageSizeStr == "" { | ||
| pageSize = 50 | ||
| } | ||
|
|
||
| pageSize := 50 | ||
| if pageSizeStr := r.URL.Query().Get("pageSize"); pageSizeStr != "" { | ||
| if ps, err := strconv.Atoi(pageSizeStr); err == nil && ps > 0 && ps <= 100 { | ||
| pageSize = ps | ||
| } | ||
| // Apply max pageSize limit as in original | ||
| if pageSize > 100 { | ||
| pageSize = 100 | ||
| } |
There was a problem hiding this comment.
The pageSize override logic after parsing defeats the purpose of using the utility function. Consider creating a specialized function like ParsePaginationParamsWithDefaults(pageStr, pageSizeStr, defaultPage, defaultPageSize, maxPageSize) or pass custom defaults to the existing function to maintain consistency.
Copilot uses AI. Check for mistakes.
Comment on lines
+53
to
+58
| func ParsePaginationParams(pageStr, pageSizeStr string) PaginationParams { | ||
| // Default values based on analysis of existing handlers | ||
| const defaultPage = 1 | ||
| const defaultPageSize = 25 | ||
| const maxPageSize = 100 | ||
|
|
There was a problem hiding this comment.
The hardcoded default values in this function reduce flexibility. Consider making these configurable parameters to allow different handlers to use different defaults while still benefiting from the common parsing logic.
Suggested change
| func ParsePaginationParams(pageStr, pageSizeStr string) PaginationParams { | |
| // Default values based on analysis of existing handlers | |
| const defaultPage = 1 | |
| const defaultPageSize = 25 | |
| const maxPageSize = 100 | |
| func ParsePaginationParams(pageStr, pageSizeStr string, defaultPage, defaultPageSize, maxPageSize int) PaginationParams { |
Copilot uses AI. Check for mistakes.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.