diff --git a/docs/api-resources.md b/docs/api-resources.md index 11ab5735..1304905e 100644 --- a/docs/api-resources.md +++ b/docs/api-resources.md @@ -147,7 +147,7 @@ PUT /api/hyperfleet/v1/clusters/{cluster_id}/statuses ### List Clusters -**GET** `/api/hyperfleet/v1/clusters?page=1&pageSize=10` +**GET** `/api/hyperfleet/v1/clusters?page=1&size=10` **Response (200 OK):** @@ -603,13 +603,13 @@ Active ──(DELETE)──▶ Finalizing ──(adapters report Finalized=True) All list endpoints support pagination: ```text -GET /api/hyperfleet/v1/clusters?page=1&pageSize=10 +GET /api/hyperfleet/v1/clusters?page=1&size=10 ``` **Parameters:** - `page` - Page number (default: 1) -- `pageSize` - Items per page (default: 20) +- `size` - Items per page (default: 20) **Response:** @@ -686,13 +686,13 @@ All list endpoints accept the following query parameters: |------------|----------------|----------|---------------------|----------------------| | `search` | string | No | - | TSL query syntax | | `page` | integer (int32)| No | `1` | Must be >= 1 | -| `pageSize` | integer (int32)| No | `20` | Must be between 1 and 100 | -| `orderBy` | string | No | `created_time desc` | Field name(s) with optional direction (asc/desc) | +| `size` | integer (int32)| No | `20` | Must be between 1 and 100 | +| `order` | string | No | `created_time desc` | Field name(s) with optional direction (asc/desc) | **Ordering behavior**: -- Include direction in `orderBy`: `?orderBy=name desc` or `?orderBy=name asc,created_time desc` -- Fields without direction default to ascending: `?orderBy=name` → sorts by `name asc` -- Default ordering when `orderBy` is omitted: `created_time desc` +- Include direction in `order`: `?order=name desc` or `?order=name asc,created_time desc` +- Fields without direction default to ascending: `?order=name` → sorts by `name asc` +- Default ordering when `order` is omitted: `created_time desc` **Note**: Violating constraints returns a `400 Bad Request` response with [RFC 9457 Problem Details](https://datatracker.ietf.org/doc/html/rfc9457) format. @@ -753,7 +753,6 @@ Same naming rules as cluster, but with a shorter maximum length. - **AdapterConditionStatus** (used in adapter status reports): `True`, `False`, `Unknown` - **ResourceConditionStatus** (used in cluster/nodepool conditions): `True`, `False` -- **OrderDirection**: `asc`, `desc` ## Spec Validation diff --git a/go.mod b/go.mod index bf75edd9..b9c917f3 100755 --- a/go.mod +++ b/go.mod @@ -20,7 +20,7 @@ require ( github.com/mendsley/gojwk v0.0.0-20141217222730-4d5ec6e58103 github.com/oapi-codegen/runtime v1.4.2 github.com/onsi/gomega v1.42.0 - github.com/openshift-hyperfleet/hyperfleet-api-spec v1.0.23 + github.com/openshift-hyperfleet/hyperfleet-api-spec v1.0.24 github.com/prometheus/client_golang v1.16.0 github.com/prometheus/client_model v0.3.0 github.com/spf13/cobra v1.10.2 diff --git a/go.sum b/go.sum old mode 100755 new mode 100644 index d2eae4c6..67b32297 --- a/go.sum +++ b/go.sum @@ -201,8 +201,8 @@ github.com/opencontainers/go-digest v1.0.0 h1:apOUWs51W5PlhuyGyz9FCeeBIOUDA/6nW8 github.com/opencontainers/go-digest v1.0.0/go.mod h1:0JzlMkj0TRzQZfJkVvzbP0HBR3IKzErnv2BNG4W4MAM= github.com/opencontainers/image-spec v1.1.1 h1:y0fUlFfIZhPF1W537XOLg0/fcx6zcHCJwooC2xJA040= github.com/opencontainers/image-spec v1.1.1/go.mod h1:qpqAh3Dmcf36wStyyWU+kCeDgrGnAve2nCC8+7h8Q0M= -github.com/openshift-hyperfleet/hyperfleet-api-spec v1.0.23 h1:HyOWpJfEonjpLFnMmmhiqaAD3EJSVIvAuaA7y5fZ4kE= -github.com/openshift-hyperfleet/hyperfleet-api-spec v1.0.23/go.mod h1:KITzIAd8HcMpH5lXdHFjgk45dvL6XLpP3wwz8iK+KCI= +github.com/openshift-hyperfleet/hyperfleet-api-spec v1.0.24 h1:ACdI09b1TAqsTehVG0eNaAJhWfybMKt87nx/bZ1Dgwk= +github.com/openshift-hyperfleet/hyperfleet-api-spec v1.0.24/go.mod h1:KITzIAd8HcMpH5lXdHFjgk45dvL6XLpP3wwz8iK+KCI= github.com/pelletier/go-toml/v2 v2.2.4 h1:mye9XuhQ6gvn5h28+VilKrrPoQVanw5PMw/TB0t5Ec4= github.com/pelletier/go-toml/v2 v2.2.4/go.mod h1:2gIqNv+qfxSVS7cM2xJQKtLSTLUE9V8t9Stt+h56mCY= github.com/perimeterx/marshmallow v1.1.5 h1:a2LALqQ1BlHM8PZblsDdidgv1mWi1DgC2UmX50IvK2s= diff --git a/pkg/services/CLAUDE.md b/pkg/services/CLAUDE.md index 4d52c137..f8b29607 100644 --- a/pkg/services/CLAUDE.md +++ b/pkg/services/CLAUDE.md @@ -30,7 +30,7 @@ func NewClusterService(dao, adapterStatusDao, config) ClusterService ## GenericService `generic.go` provides `List()` with pagination, search, and ordering. -- `ListArguments` has Page, Size, Search, OrderBy, Fields, Preloads +- `ListArguments` has Page, Size, Search, Order, Fields, Preloads - Search validation: `SearchDisallowedFields` map blocks searching certain fields per resource type - Default ordering: `created_time desc` diff --git a/pkg/services/generic.go b/pkg/services/generic.go index 9e7ccfa5..8c048acb 100755 --- a/pkg/services/generic.go +++ b/pkg/services/generic.go @@ -141,8 +141,8 @@ func (s *sqlGenericService) buildPreload(listCtx *listContext, d *dao.GenericDao } func (s *sqlGenericService) buildOrderBy(listCtx *listContext, d *dao.GenericDao) (bool, *errors.ServiceError) { - if len(listCtx.args.OrderBy) != 0 { - orderByArgs, serviceErr := db.ArgsToOrderBy(listCtx.args.OrderBy, *listCtx.disallowedFields) + if len(listCtx.args.Order) != 0 { + orderByArgs, serviceErr := db.ArgsToOrderBy(listCtx.args.Order, *listCtx.disallowedFields) if serviceErr != nil { return false, serviceErr } diff --git a/pkg/services/types.go b/pkg/services/types.go index de6290c9..1f51ae74 100755 --- a/pkg/services/types.go +++ b/pkg/services/types.go @@ -13,7 +13,7 @@ import ( type ListArguments struct { Search string Preloads []string - OrderBy []string + Order []string Fields []string Page int Size int64 @@ -88,38 +88,27 @@ func NewListArguments(params url.Values) (*ListArguments, *errors.ServiceError) listArgs.Page = page } - // Validate size parameter (support both "size" legacy and "pageSize" OpenAPI spec) - var sizeParam string - var sizeValue string - if v := strings.Trim(params.Get("pageSize"), " "); v != "" { - sizeParam = "pageSize" - sizeValue = v - } else if v := strings.Trim(params.Get("size"), " "); v != "" { - sizeParam = "size" - sizeValue = v - } - - if sizeValue != "" { - size, err := strconv.ParseInt(sizeValue, 10, 64) + // Validate size parameter + if v := strings.Trim(params.Get("size"), " "); v != "" { + size, err := strconv.ParseInt(v, 10, 64) if err != nil { return nil, errors.New( errors.CodeValidationFormat, - "Invalid %s parameter: must be a positive integer", - sizeParam, + "Invalid size parameter: must be a positive integer", ) } if size < 1 { return nil, errors.New( errors.CodeValidationRange, - "Invalid %s parameter: %d is less than 1", - sizeParam, size, + "Invalid size parameter: %d is less than 1", + size, ) } if size > MaxPageSize { return nil, errors.New( errors.CodeValidationRange, - "Invalid %s parameter: %d exceeds maximum allowed value of %d", - sizeParam, size, MaxPageSize, + "Invalid size parameter: %d exceeds maximum allowed value of %d", + size, MaxPageSize, ) } listArgs.Size = size @@ -128,19 +117,17 @@ func NewListArguments(params url.Values) (*ListArguments, *errors.ServiceError) if v := strings.Trim(params.Get("search"), " "); v != "" { listArgs.Search = v } - if v := strings.Trim(params.Get("orderBy"), " "); v != "" { + if v := strings.Trim(params.Get("order"), " "); v != "" { rawFields := strings.Split(v, ",") - // Filter out empty tokens from malformed input like "name,,created_time" for _, field := range rawFields { if trimmed := strings.TrimSpace(field); trimmed != "" { - listArgs.OrderBy = append(listArgs.OrderBy, strings.Join(strings.Fields(trimmed), " ")) + listArgs.Order = append(listArgs.Order, strings.Join(strings.Fields(trimmed), " ")) } } } - // Set default sorting to created_time desc if orderBy not provided - if len(listArgs.OrderBy) == 0 { - listArgs.OrderBy = []string{"created_time desc"} + if len(listArgs.Order) == 0 { + listArgs.Order = []string{"created_time desc"} } // Parse fields parameter using shared logic diff --git a/pkg/services/types_test.go b/pkg/services/types_test.go index bd92f1ed..fb1192bf 100644 --- a/pkg/services/types_test.go +++ b/pkg/services/types_test.go @@ -8,58 +8,58 @@ import ( . "github.com/onsi/gomega" ) -func TestNewListArguments_OrderBy(t *testing.T) { +func TestNewListArguments_Order(t *testing.T) { RegisterTestingT(t) tests := []struct { - name string - queryParams url.Values - expectedOrderBy []string + name string + queryParams url.Values + expectedOrder []string }{ { - name: "no orderBy - should use default created_time desc", - queryParams: url.Values{}, - expectedOrderBy: []string{"created_time desc"}, + name: "no order - should use default created_time desc", + queryParams: url.Values{}, + expectedOrder: []string{"created_time desc"}, }, { - name: "orderBy with asc direction", - queryParams: url.Values{"orderBy": []string{"name asc"}}, - expectedOrderBy: []string{"name asc"}, + name: "order with asc direction", + queryParams: url.Values{"order": []string{"name asc"}}, + expectedOrder: []string{"name asc"}, }, { - name: "orderBy with desc direction", - queryParams: url.Values{"orderBy": []string{"name desc"}}, - expectedOrderBy: []string{"name desc"}, + name: "order with desc direction", + queryParams: url.Values{"order": []string{"name desc"}}, + expectedOrder: []string{"name desc"}, }, { - name: "orderBy without direction", - queryParams: url.Values{"orderBy": []string{"name"}}, - expectedOrderBy: []string{"name"}, + name: "order without direction", + queryParams: url.Values{"order": []string{"name"}}, + expectedOrder: []string{"name"}, }, { - name: "multiple orderBy fields", - queryParams: url.Values{"orderBy": []string{"name asc,created_time desc"}}, - expectedOrderBy: []string{"name asc", "created_time desc"}, + name: "multiple order fields", + queryParams: url.Values{"order": []string{"name asc,created_time desc"}}, + expectedOrder: []string{"name asc", "created_time desc"}, }, { - name: "orderBy with spaces should be trimmed", - queryParams: url.Values{"orderBy": []string{" name asc "}}, - expectedOrderBy: []string{"name asc"}, + name: "order with spaces should be trimmed", + queryParams: url.Values{"order": []string{" name asc "}}, + expectedOrder: []string{"name asc"}, }, { - name: "empty orderBy string - should use default", - queryParams: url.Values{"orderBy": []string{""}}, - expectedOrderBy: []string{"created_time desc"}, + name: "empty order string - should use default", + queryParams: url.Values{"order": []string{""}}, + expectedOrder: []string{"created_time desc"}, }, { - name: "orderBy with whitespace only - should use default", - queryParams: url.Values{"orderBy": []string{" "}}, - expectedOrderBy: []string{"created_time desc"}, + name: "order with whitespace only - should use default", + queryParams: url.Values{"order": []string{" "}}, + expectedOrder: []string{"created_time desc"}, }, { - name: "orderBy with empty tokens - should filter out", - queryParams: url.Values{"orderBy": []string{"name,,created_time"}}, - expectedOrderBy: []string{"name", "created_time"}, + name: "order with empty tokens - should filter out", + queryParams: url.Values{"order": []string{"name,,created_time"}}, + expectedOrder: []string{"name", "created_time"}, }, } @@ -67,9 +67,9 @@ func TestNewListArguments_OrderBy(t *testing.T) { t.Run(tt.name, func(t *testing.T) { RegisterTestingT(t) listArgs, err := NewListArguments(tt.queryParams) - Expect(err).To(BeNil(), "Should not return error for valid orderBy parameters") - Expect(listArgs.OrderBy).To(Equal(tt.expectedOrderBy), - "OrderBy mismatch for test case: %s", tt.name) + Expect(err).To(BeNil(), "Should not return error for valid order parameters") + Expect(listArgs.Order).To(Equal(tt.expectedOrder), + "Order mismatch for test case: %s", tt.name) }) } } @@ -83,10 +83,10 @@ func TestNewListArguments_DefaultValues(t *testing.T) { Expect(listArgs.Page).To(Equal(1), "Default page should be 1") Expect(listArgs.Size).To(Equal(int64(20)), "Default size should be 20") Expect(listArgs.Search).To(Equal(""), "Default search should be empty") - Expect(listArgs.OrderBy).To(Equal([]string{"created_time desc"}), "Default orderBy should be created_time desc") + Expect(listArgs.Order).To(Equal([]string{"created_time desc"}), "Default order should be created_time desc") } -func TestNewListArguments_PageSize(t *testing.T) { +func TestNewListArguments_Size(t *testing.T) { RegisterTestingT(t) tests := []struct { @@ -96,23 +96,17 @@ func TestNewListArguments_PageSize(t *testing.T) { expectedSize int64 }{ { - name: "custom page and pageSize", - queryParams: url.Values{"page": []string{"2"}, "pageSize": []string{"50"}}, + name: "custom page and size", + queryParams: url.Values{"page": []string{"2"}, "size": []string{"50"}}, expectedPage: 2, expectedSize: 50, }, { - name: "custom page and size (legacy)", + name: "custom page and size (different values)", queryParams: url.Values{"page": []string{"3"}, "size": []string{"25"}}, expectedPage: 3, expectedSize: 25, }, - { - name: "pageSize takes precedence over size", - queryParams: url.Values{"pageSize": []string{"30"}, "size": []string{"60"}}, - expectedPage: 1, - expectedSize: 30, - }, } for _, tt := range tests { @@ -278,36 +272,25 @@ func TestNewListArguments_Validation(t *testing.T) { errorCode: "HYPERFLEET-VAL-003", }, - // PageSize validation tests (OpenAPI spec parameter) - // PageSize boundary checks — VAL-004 (CodeValidationRange) for values outside valid range (1-100) - { - name: "negative pageSize returns error", - queryParams: url.Values{"pageSize": []string{"-1"}}, - expectError: true, - errorContains: "Invalid pageSize parameter", - errorCode: "HYPERFLEET-VAL-004", - }, + // Additional size edge cases { - name: "zero pageSize returns error", - queryParams: url.Values{"pageSize": []string{"0"}}, + name: "size with special characters returns error", + queryParams: url.Values{"size": []string{"