Skip to content

Commit f8edd64

Browse files
committed
Use ospath builders instead of fmt.Sprintf for URL construction in tests
Replace all fmt.Sprintf URL path construction with typed ospath builders (IndicesCreatePath, IndicesDeletePath, SearchPath, SearchShardsPath, IndexPath, ClusterHealthPath, ClusterStatePath) for compile-time safety and proper percent-encoding of path segments. Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com>
1 parent 8d29e8d commit f8edd64

2 files changed

Lines changed: 42 additions & 16 deletions

File tree

opensearchtransport/policy_doc_router_internal_test.go

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@ import (
1515
"time"
1616

1717
"github.com/stretchr/testify/require"
18+
19+
ospath "github.com/opensearch-project/opensearch-go/v4/internal/path"
1820
)
1921

2022
func TestExtractDocumentFromPath(t *testing.T) {
@@ -334,8 +336,9 @@ func TestDocRouterConsistency(t *testing.T) {
334336
// to different nodes (by pigeonhole with 5 nodes).
335337
selectedNodes := make(map[string]struct{})
336338
for i := range 50 {
337-
path := fmt.Sprintf("/orders/_doc/doc-%d", i)
338-
req, err := http.NewRequest(http.MethodGet, path, nil)
339+
docID := fmt.Sprintf("doc-%d", i)
340+
p2, _ := ospath.IndexPath{Index: "orders", ID: docID}.Build()
341+
req, err := http.NewRequest(http.MethodGet, p2, nil)
339342
require.NoError(t, err)
340343

341344
hop, evalErr := p.Eval(context.Background(), req)

opensearchtransport/shard_routing_integration_test.go

Lines changed: 37 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import (
2222

2323
"github.com/stretchr/testify/require"
2424

25+
ospath "github.com/opensearch-project/opensearch-go/v4/internal/path"
2526
"github.com/opensearch-project/opensearch-go/v4/opensearchtransport/testutil"
2627
)
2728

@@ -66,9 +67,10 @@ func TestMurmur3ShardRouting_Integration(t *testing.T) {
6667
// Retry index creation — transient HTTP 500 can occur while the cluster
6768
// is still settling after heavy discovery/warmup activity.
6869
var createResp *http.Response
70+
createPath, _ := ospath.IndicesCreatePath{Index: index}.Build()
6971
testutil.RequireMinConns(t, ctx, 1, transport.DiscoverNodes, func() bool {
7072
createReq, reqErr := http.NewRequestWithContext(ctx, http.MethodPut,
71-
fmt.Sprintf("/%s", index),
73+
createPath,
7274
bytes.NewReader([]byte(createBody)))
7375
if reqErr != nil {
7476
return false
@@ -90,8 +92,9 @@ func TestMurmur3ShardRouting_Integration(t *testing.T) {
9092
createResp.Body.Close()
9193

9294
t.Cleanup(func() {
95+
delPath, _ := ospath.IndicesDeletePath{Index: []string{index}}.Build()
9396
delReq, _ := http.NewRequestWithContext(context.Background(), http.MethodDelete,
94-
fmt.Sprintf("/%s", index), nil)
97+
delPath, nil)
9598
resp, _ := transport.Perform(delReq)
9699
if resp != nil {
97100
resp.Body.Close()
@@ -163,9 +166,12 @@ func TestMurmur3ShardRouting_Integration(t *testing.T) {
163166
func querySearchShardsForRouting(t *testing.T, transport *Client, ctx context.Context, index, routing string) int {
164167
t.Helper()
165168

166-
req, err := http.NewRequestWithContext(ctx, http.MethodPost,
167-
fmt.Sprintf("/%s/_search_shards?routing=%s", index, url.QueryEscape(routing)),
168-
nil)
169+
p, _ := ospath.SearchShardsPath{Index: []string{index}}.Build()
170+
endpoint := url.URL{
171+
Path: p,
172+
RawQuery: url.Values{"routing": {routing}}.Encode(),
173+
}
174+
req, err := http.NewRequestWithContext(ctx, http.MethodPost, endpoint.String(), nil)
169175
require.NoError(t, err)
170176

171177
resp, err := transport.Perform(req)
@@ -206,8 +212,8 @@ func querySearchShardsForRouting(t *testing.T, transport *Client, ctx context.Co
206212
func indexDoc(t *testing.T, transport *Client, ctx context.Context, index, docID, body string) {
207213
t.Helper()
208214

209-
req, err := http.NewRequestWithContext(ctx, http.MethodPut,
210-
fmt.Sprintf("/%s/_doc/%s", index, url.PathEscape(docID)),
215+
p, _ := ospath.IndexPath{Index: index, ID: docID}.Build()
216+
req, err := http.NewRequestWithContext(ctx, http.MethodPut, p,
211217
bytes.NewReader([]byte(body)))
212218
require.NoError(t, err)
213219
req.Header.Set("Content-Type", "application/json")
@@ -300,9 +306,10 @@ func TestShardExactRouting_FullPipeline_Integration(t *testing.T) {
300306
// Retry index creation — transient HTTP 500 can occur while the cluster
301307
// is still settling after heavy discovery/warmup activity.
302308
var createResp *http.Response
309+
createPath, _ := ospath.IndicesCreatePath{Index: index}.Build()
303310
testutil.RequireMinConns(t, ctx, 1, transport.DiscoverNodes, func() bool {
304311
createReq, reqErr := http.NewRequestWithContext(ctx, http.MethodPut,
305-
fmt.Sprintf("/%s", index),
312+
createPath,
306313
bytes.NewReader([]byte(createBody)))
307314
if reqErr != nil {
308315
return false
@@ -324,8 +331,9 @@ func TestShardExactRouting_FullPipeline_Integration(t *testing.T) {
324331
createResp.Body.Close()
325332

326333
t.Cleanup(func() {
334+
delPath, _ := ospath.IndicesDeletePath{Index: []string{index}}.Build()
327335
delReq, _ := http.NewRequestWithContext(context.Background(), http.MethodDelete,
328-
fmt.Sprintf("/%s", index), nil)
336+
delPath, nil)
329337
resp, _ := transport.Perform(delReq)
330338
if resp != nil {
331339
resp.Body.Close()
@@ -345,8 +353,9 @@ func TestShardExactRouting_FullPipeline_Integration(t *testing.T) {
345353
// The router cache creates index slots lazily (on first request).
346354
// We need the slot to exist before DiscoverNodes so that
347355
// fetchAndUpdateShardPlacement can populate its shardMap.
356+
warmPath, _ := ospath.SearchPath{Index: []string{index}}.Build()
348357
warmReq, err := http.NewRequestWithContext(ctx, http.MethodPost,
349-
fmt.Sprintf("/%s/_search", index),
358+
warmPath,
350359
bytes.NewReader([]byte(`{"query":{"match_all":{}},"size":0}`)))
351360
require.NoError(t, err)
352361
warmReq.Header.Set("Content-Type", "application/json")
@@ -414,8 +423,13 @@ func TestShardExactRouting_FullPipeline_Integration(t *testing.T) {
414423
// cleared by a discovery race), not a timing issue.
415424
obs.reset()
416425

426+
searchPath, _ := ospath.SearchPath{Index: []string{index}}.Build()
427+
searchEndpoint := url.URL{
428+
Path: searchPath,
429+
RawQuery: url.Values{"routing": {routing}}.Encode(),
430+
}
417431
searchReq, err := http.NewRequestWithContext(ctx, http.MethodPost,
418-
fmt.Sprintf("/%s/_search?routing=%s", index, url.QueryEscape(routing)),
432+
searchEndpoint.String(),
419433
bytes.NewReader([]byte(`{"query":{"match_all":{}}}`)))
420434
require.NoError(t, err)
421435
searchReq.Header.Set("Content-Type", "application/json")
@@ -495,9 +509,12 @@ func querySearchShardsWithNodes( //nolint:nonamedreturns // named returns docume
495509
) (shardNum int, nodeNames map[string]struct{}) {
496510
t.Helper()
497511

498-
req, err := http.NewRequestWithContext(ctx, http.MethodPost,
499-
fmt.Sprintf("/%s/_search_shards?routing=%s", index, url.QueryEscape(routing)),
500-
nil)
512+
p, _ := ospath.SearchShardsPath{Index: []string{index}}.Build()
513+
endpoint := url.URL{
514+
Path: p,
515+
RawQuery: url.Values{"routing": {routing}}.Encode(),
516+
}
517+
req, err := http.NewRequestWithContext(ctx, http.MethodPost, endpoint.String(), nil)
501518
require.NoError(t, err)
502519

503520
resp, err := transport.Perform(req)
@@ -550,6 +567,12 @@ func querySearchShardsWithNodes( //nolint:nonamedreturns // named returns docume
550567
func fetchRoutingNumShardsForTest(t *testing.T, transport *Client, ctx context.Context, index string) int {
551568
t.Helper()
552569

570+
p, _ := ospath.ClusterStatePath{Metric: []string{"metadata"}, Index: []string{index}}.Build()
571+
endpoint := url.URL{
572+
Path: p,
573+
RawQuery: url.Values{"filter_path": {"metadata.indices.*.routing_num_shards"}}.Encode(),
574+
}
575+
553576
var routingNumShards int
554577
testutil.RequireMinConns(t, ctx, 1, transport.DiscoverNodes, func() bool {
555578
req, err := http.NewRequestWithContext(ctx, http.MethodGet, endpoint.String(), nil)

0 commit comments

Comments
 (0)