Skip to content

Commit a95e472

Browse files
committed
Add generic Do[T] function for compile-time pointer enforcement
Add opensearch.Do[T]() that enforces pointer response types at compile time, preventing a class of bugs where non-pointer values silently fail JSON unmarshaling at runtime. Mark Client.Do() as deprecated in favor of the generic alternative. Ref: opensearch-project#809 Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com>
1 parent 39c2a2f commit a95e472

70 files changed

Lines changed: 346 additions & 241 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

CHANGELOG.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
77
### Added
88

99
- Add `primary_terms_map` and `split_shards_metadata` fields to ClusterState index metadata for OpenSearch >=3.6.0 compatibility
10+
- Add generic `opensearch.Do[T]()` function for compile-time pointer enforcement on response types, preventing a class of bugs where non-pointer values are silently passed to `Client.Do()` and fail at runtime during JSON unmarshaling
1011
- Add `InsecureSkipVerify` config option to disable TLS certificate verification without constructing a custom `http.Transport`, preserving `DefaultTransport` connection pooling, HTTP/2, and timeout defaults ([#786](https://github.com/opensearch-project/opensearch-go/issues/786))
1112
- Add `DisableResponseBuffering` config option to skip eager `io.ReadAll` buffering of response bodies in `Perform()`, reducing per-request allocations and TTFB for proxy and streaming use cases ([#786](https://github.com/opensearch-project/opensearch-go/issues/786))
1213
- Add per-attempt `RequestTimeout` to bound individual HTTP round-trips, preventing indefinite hangs on stalled connections ([#786](https://github.com/opensearch-project/opensearch-go/issues/786))
@@ -148,6 +149,8 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
148149

149150
### Deprecated
150151

152+
- Mark `Client.Do()` with a `Deprecated` doc annotation in favor of `opensearch.Do[T]()` for compile-time pointer safety; `Client.Do()` remains fully functional and will not be removed, but `staticcheck` SA1019 will nudge cross-package callers toward the safer generic alternative
153+
151154
### Removed
152155

153156
### Fixed

USER_GUIDE.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -517,6 +517,7 @@ All `OPENSEARCH_GO_*` environment variables are evaluated once at client initial
517517
- [Advanced Index Actions](guides/advanced_index_actions.md)
518518
- [Index Templates](guides/index_template.md)
519519
- [Data Streams](guides/data_streams.md)
520+
- [Making Raw JSON REST Requests](guides/json.md)
520521
- [Request Routing](guides/routing.md)
521522
- [Cluster Health Checking](guides/cluster_health_checking.md)
522523
- [Node Discovery and Role Management](guides/node_discovery_and_roles.md)

error_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -318,11 +318,11 @@ func TestError(t *testing.T) {
318318

319319
// Parse the error
320320
err := opensearch.ParseError(resp)
321-
require.NotNil(t, err)
321+
require.Error(t, err)
322322

323323
// Verify the body is still readable after ParseError
324324
body, readErr := io.ReadAll(resp.Body)
325-
require.Nil(t, readErr, "body should be readable after ParseError")
325+
require.NoError(t, readErr, "body should be readable after ParseError")
326326
require.NotEmpty(t, body, "body should not be empty after ParseError")
327327

328328
// Verify the body content matches the original

guides/json.md

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
- [Making Raw JSON REST Requests](#making-raw-json-rest-requests)
22
- [Setup](#setup)
3+
- [Using Do for Typed Responses](#using-do-for-typed-responses)
34
- [GET](#get)
45
- [PUT](#put)
56
- [POST](#post)
@@ -17,12 +18,14 @@ Let's create a client instance:
1718
package main
1819

1920
import (
21+
"context"
2022
"fmt"
2123
"io"
2224
"net/http"
2325
"os"
2426
"strings"
2527

28+
"github.com/opensearch-project/opensearch-go/v4"
2629
"github.com/opensearch-project/opensearch-go/v4/opensearchapi"
2730
)
2831

@@ -40,6 +43,63 @@ func example() error {
4043
}
4144
```
4245
46+
## Using Do for Typed Responses
47+
48+
When you need to call an API that `opensearchapi` doesn't cover — plugin endpoints, newly released server APIs, or internal custom endpoints — use `opensearch.Do()` to execute a request and automatically unmarshal the JSON response into a struct.
49+
50+
The `Client.Do()` method accepts `any` for its response parameter, which means passing a non-pointer compiles but fails at runtime during JSON unmarshaling. The generic `opensearch.Do[T]()` function catches this mistake at compile time. `Client.Do()` is marked with a `Deprecated` doc annotation to steer callers toward the safer alternative — it remains fully functional and will not be removed, but `staticcheck` SA1019 will flag cross-package usage as a nudge.
51+
52+
First, define a request type that satisfies `opensearch.Request`:
53+
54+
```go
55+
// customReq wraps opensearch.BuildRequest to satisfy the opensearch.Request interface.
56+
type customReq struct {
57+
method string
58+
path string
59+
body io.Reader
60+
}
61+
62+
func (r customReq) GetRequest() (*http.Request, error) {
63+
return opensearch.BuildRequest(r.method, r.path, r.body, nil, nil)
64+
}
65+
```
66+
67+
Then use `opensearch.Do` to call the endpoint with a typed response:
68+
69+
```go
70+
type PluginStatusResp struct {
71+
Status string `json:"status"`
72+
Version string `json:"version"`
73+
}
74+
75+
ctx := context.Background()
76+
77+
// Preferred: opensearch.Do[T] enforces *T at compile time.
78+
var pluginStatus PluginStatusResp
79+
req := customReq{method: http.MethodGet, path: "/_plugins/my_plugin/status"}
80+
resp, err := opensearch.Do(ctx, client.Client, req, &pluginStatus)
81+
if err != nil {
82+
return err
83+
}
84+
fmt.Printf("plugin status: %s (v%s), http: %d\n", pluginStatus.Status, pluginStatus.Version, resp.StatusCode)
85+
```
86+
87+
If you pass a non-pointer value to `opensearch.Do`, the compiler rejects it:
88+
89+
```go
90+
// Compile error: cannot use pluginStatus (variable of type PluginStatusResp)
91+
// as *PluginStatusResp value in argument to opensearch.Do
92+
resp, err := opensearch.Do(ctx, client.Client, req, pluginStatus)
93+
```
94+
95+
The three levels of the client API, from lowest to highest:
96+
97+
| Level | Function | Response handling | When to use |
98+
| ----- | ------------------------------------------------------------------ | --------------------------------------------------------- | ---------------------------------------------------------- |
99+
| Low | `client.Perform(req)` | Raw `*http.Response`; caller reads and closes body | Proxying, streaming, full control needed |
100+
| Mid | `opensearch.Do(ctx, client, req, &resp)` | Automatic JSON unmarshal with compile-time pointer safety | Plugin APIs, unsupported endpoints, custom `Request` types |
101+
| High | `client.Search(ctx, req)` / `client.Indices.Create(ctx, req)` etc. | Fully typed request and response | Standard OpenSearch APIs |
102+
43103
## GET
44104
45105
The following example returns the server version information via `GET /`.

opensearch.go

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -381,7 +381,12 @@ func (c *Client) Perform(req *http.Request) (*http.Response, error) {
381381
return c.Transport.Perform(req)
382382
}
383383

384-
// Do gets and performs the request. It also tries to parse the response into the dataPointer
384+
// Do gets and performs the request. It also tries to parse the response into the dataPointer.
385+
//
386+
// Deprecated: Use [Do] instead, which enforces that dataPointer is a pointer at compile time.
387+
// Client.Do accepts any, so passing a non-pointer compiles but fails at runtime during JSON
388+
// unmarshaling. The method remains fully functional and will not be removed; this annotation
389+
// exists to steer callers toward the safer generic alternative.
385390
func (c *Client) Do(ctx context.Context, req Request, dataPointer any) (*Response, error) {
386391
httpReq, err := req.GetRequest()
387392
if err != nil {
@@ -420,6 +425,12 @@ func (c *Client) Do(ctx context.Context, req Request, dataPointer any) (*Respons
420425
return response, nil
421426
}
422427

428+
// Do is a generic version of [Client.Do] that enforces dataPointer as a pointer at compile time.
429+
// It delegates to [Client.Do] after the type system has guaranteed *T.
430+
func Do[T any](ctx context.Context, c *Client, req Request, dataPointer *T) (*Response, error) {
431+
return c.Do(ctx, req, dataPointer)
432+
}
433+
423434
// Metrics returns the client metrics.
424435
func (c *Client) Metrics() (opensearchtransport.Metrics, error) {
425436
if mt, ok := c.Transport.(opensearchtransport.Measurable); ok {

opensearch_internal_test.go

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -218,6 +218,35 @@ func TestClientInterfe(t *testing.T) {
218218
assert.NotNil(t, resp)
219219
})
220220

221+
t.Run("Generic Do()", func(t *testing.T) {
222+
c, err := NewClient(Config{Transport: mockhttp.NewRoundTripFunc(t, defaultRoundTripFunc)})
223+
require.NoError(t, err)
224+
225+
type versionInfo struct {
226+
Number string `json:"number"`
227+
Distribution string `json:"distribution"`
228+
}
229+
type rootResp struct {
230+
Version versionInfo `json:"version"`
231+
}
232+
233+
var got rootResp
234+
resp, err := Do(context.TODO(), c, testReq{Path: "/"}, &got)
235+
require.NoError(t, err)
236+
require.NotNil(t, resp)
237+
require.Equal(t, "1.0.0", got.Version.Number)
238+
require.Equal(t, "opensearch", got.Version.Distribution)
239+
})
240+
241+
t.Run("Generic Do() nil pointer", func(t *testing.T) {
242+
c, err := NewClient(Config{Transport: mockhttp.NewRoundTripFunc(t, defaultRoundTripFunc)})
243+
require.NoError(t, err)
244+
245+
resp, err := Do[struct{}](context.TODO(), c, testReq{}, nil)
246+
require.NoError(t, err)
247+
require.NotNil(t, resp)
248+
})
249+
221250
t.Run("Do() GetRequest error", func(t *testing.T) {
222251
c, err := NewClient(Config{Transport: mockhttp.NewRoundTripFunc(t, defaultRoundTripFunc)})
223252
require.NoError(t, err)

opensearchapi/api_aliases.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ func (c Client) Aliases(ctx context.Context, req AliasesReq) (*AliasesResp, erro
2020
data AliasesResp
2121
err error
2222
)
23-
if data.response, err = c.do(ctx, req, &data); err != nil {
23+
if data.response, err = do(ctx, &c, req, &data); err != nil {
2424
return &data, err
2525
}
2626

opensearchapi/api_bulk.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ func (c Client) Bulk(ctx context.Context, req BulkReq) (*BulkResp, error) {
2020
data BulkResp
2121
err error
2222
)
23-
if data.response, err = c.do(ctx, req, &data); err != nil {
23+
if data.response, err = do(ctx, &c, req, &data); err != nil {
2424
return &data, err
2525
}
2626

opensearchapi/api_cat.go

Lines changed: 20 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ func (c catClient) Aliases(ctx context.Context, req *CatAliasesReq) (*CatAliases
2424
data CatAliasesResp
2525
err error
2626
)
27-
if data.response, err = c.apiClient.do(ctx, req, &data.Aliases); err != nil {
27+
if data.response, err = do(ctx, c.apiClient, req, &data.Aliases); err != nil {
2828
return &data, err
2929
}
3030

@@ -41,7 +41,7 @@ func (c catClient) Allocation(ctx context.Context, req *CatAllocationReq) (*CatA
4141
data CatAllocationsResp
4242
err error
4343
)
44-
if data.response, err = c.apiClient.do(ctx, req, &data.Allocations); err != nil {
44+
if data.response, err = do(ctx, c.apiClient, req, &data.Allocations); err != nil {
4545
return &data, err
4646
}
4747

@@ -58,7 +58,7 @@ func (c catClient) ClusterManager(ctx context.Context, req *CatClusterManagerReq
5858
data CatClusterManagersResp
5959
err error
6060
)
61-
if data.response, err = c.apiClient.do(ctx, req, &data.ClusterManagers); err != nil {
61+
if data.response, err = do(ctx, c.apiClient, req, &data.ClusterManagers); err != nil {
6262
return &data, err
6363
}
6464

@@ -75,7 +75,7 @@ func (c catClient) Count(ctx context.Context, req *CatCountReq) (*CatCountsResp,
7575
data CatCountsResp
7676
err error
7777
)
78-
if data.response, err = c.apiClient.do(ctx, req, &data.Counts); err != nil {
78+
if data.response, err = do(ctx, c.apiClient, req, &data.Counts); err != nil {
7979
return &data, err
8080
}
8181

@@ -92,7 +92,7 @@ func (c catClient) FieldData(ctx context.Context, req *CatFieldDataReq) (*CatFie
9292
data CatFieldDataResp
9393
err error
9494
)
95-
if data.response, err = c.apiClient.do(ctx, req, &data.FieldData); err != nil {
95+
if data.response, err = do(ctx, c.apiClient, req, &data.FieldData); err != nil {
9696
return &data, err
9797
}
9898

@@ -109,7 +109,7 @@ func (c catClient) Health(ctx context.Context, req *CatHealthReq) (*CatHealthRes
109109
data CatHealthResp
110110
err error
111111
)
112-
if data.response, err = c.apiClient.do(ctx, req, &data.Health); err != nil {
112+
if data.response, err = do(ctx, c.apiClient, req, &data.Health); err != nil {
113113
return &data, err
114114
}
115115

@@ -126,7 +126,7 @@ func (c catClient) Indices(ctx context.Context, req *CatIndicesReq) (*CatIndices
126126
data CatIndicesResp
127127
err error
128128
)
129-
if data.response, err = c.apiClient.do(ctx, req, &data.Indices); err != nil {
129+
if data.response, err = do(ctx, c.apiClient, req, &data.Indices); err != nil {
130130
return &data, err
131131
}
132132

@@ -143,7 +143,7 @@ func (c catClient) Master(ctx context.Context, req *CatMasterReq) (*CatMasterRes
143143
data CatMasterResp
144144
err error
145145
)
146-
if data.response, err = c.apiClient.do(ctx, req, &data.Master); err != nil {
146+
if data.response, err = do(ctx, c.apiClient, req, &data.Master); err != nil {
147147
return &data, err
148148
}
149149

@@ -160,7 +160,7 @@ func (c catClient) NodeAttrs(ctx context.Context, req *CatNodeAttrsReq) (*CatNod
160160
data CatNodeAttrsResp
161161
err error
162162
)
163-
if data.response, err = c.apiClient.do(ctx, req, &data.NodeAttrs); err != nil {
163+
if data.response, err = do(ctx, c.apiClient, req, &data.NodeAttrs); err != nil {
164164
return &data, err
165165
}
166166

@@ -177,7 +177,7 @@ func (c catClient) Nodes(ctx context.Context, req *CatNodesReq) (*CatNodesResp,
177177
data CatNodesResp
178178
err error
179179
)
180-
if data.response, err = c.apiClient.do(ctx, req, &data.Nodes); err != nil {
180+
if data.response, err = do(ctx, c.apiClient, req, &data.Nodes); err != nil {
181181
return &data, err
182182
}
183183

@@ -194,7 +194,7 @@ func (c catClient) PendingTasks(ctx context.Context, req *CatPendingTasksReq) (*
194194
data CatPendingTasksResp
195195
err error
196196
)
197-
if data.response, err = c.apiClient.do(ctx, req, &data.PendingTasks); err != nil {
197+
if data.response, err = do(ctx, c.apiClient, req, &data.PendingTasks); err != nil {
198198
return &data, err
199199
}
200200

@@ -211,7 +211,7 @@ func (c catClient) Plugins(ctx context.Context, req *CatPluginsReq) (*CatPlugins
211211
data CatPluginsResp
212212
err error
213213
)
214-
if data.response, err = c.apiClient.do(ctx, req, &data.Plugins); err != nil {
214+
if data.response, err = do(ctx, c.apiClient, req, &data.Plugins); err != nil {
215215
return &data, err
216216
}
217217

@@ -228,7 +228,7 @@ func (c catClient) Recovery(ctx context.Context, req *CatRecoveryReq) (*CatRecov
228228
data CatRecoveryResp
229229
err error
230230
)
231-
if data.response, err = c.apiClient.do(ctx, req, &data.Recovery); err != nil {
231+
if data.response, err = do(ctx, c.apiClient, req, &data.Recovery); err != nil {
232232
return &data, err
233233
}
234234

@@ -245,7 +245,7 @@ func (c catClient) Repositories(ctx context.Context, req *CatRepositoriesReq) (*
245245
data CatRepositoriesResp
246246
err error
247247
)
248-
if data.response, err = c.apiClient.do(ctx, req, &data.Repositories); err != nil {
248+
if data.response, err = do(ctx, c.apiClient, req, &data.Repositories); err != nil {
249249
return &data, err
250250
}
251251

@@ -262,7 +262,7 @@ func (c catClient) Segments(ctx context.Context, req *CatSegmentsReq) (*CatSegme
262262
data CatSegmentsResp
263263
err error
264264
)
265-
if data.response, err = c.apiClient.do(ctx, req, &data.Segments); err != nil {
265+
if data.response, err = do(ctx, c.apiClient, req, &data.Segments); err != nil {
266266
return &data, err
267267
}
268268

@@ -279,7 +279,7 @@ func (c catClient) Shards(ctx context.Context, req *CatShardsReq) (*CatShardsRes
279279
data CatShardsResp
280280
err error
281281
)
282-
if data.response, err = c.apiClient.do(ctx, req, &data.Shards); err != nil {
282+
if data.response, err = do(ctx, c.apiClient, req, &data.Shards); err != nil {
283283
return &data, err
284284
}
285285

@@ -292,7 +292,7 @@ func (c catClient) Snapshots(ctx context.Context, req CatSnapshotsReq) (*CatSnap
292292
data CatSnapshotsResp
293293
err error
294294
)
295-
if data.response, err = c.apiClient.do(ctx, req, &data.Snapshots); err != nil {
295+
if data.response, err = do(ctx, c.apiClient, req, &data.Snapshots); err != nil {
296296
return &data, err
297297
}
298298

@@ -309,7 +309,7 @@ func (c catClient) Tasks(ctx context.Context, req *CatTasksReq) (*CatTasksResp,
309309
data CatTasksResp
310310
err error
311311
)
312-
if data.response, err = c.apiClient.do(ctx, req, &data.Tasks); err != nil {
312+
if data.response, err = do(ctx, c.apiClient, req, &data.Tasks); err != nil {
313313
return &data, err
314314
}
315315

@@ -326,7 +326,7 @@ func (c catClient) Templates(ctx context.Context, req *CatTemplatesReq) (*CatTem
326326
data CatTemplatesResp
327327
err error
328328
)
329-
if data.response, err = c.apiClient.do(ctx, req, &data.Templates); err != nil {
329+
if data.response, err = do(ctx, c.apiClient, req, &data.Templates); err != nil {
330330
return &data, err
331331
}
332332

@@ -343,7 +343,7 @@ func (c catClient) ThreadPool(ctx context.Context, req *CatThreadPoolReq) (*CatT
343343
data CatThreadPoolResp
344344
err error
345345
)
346-
if data.response, err = c.apiClient.do(ctx, req, &data.ThreadPool); err != nil {
346+
if data.response, err = do(ctx, c.apiClient, req, &data.ThreadPool); err != nil {
347347
return &data, err
348348
}
349349

0 commit comments

Comments
 (0)