BKTCLT-31 [squash] address review comments

- Use references for options (also modified ListObjectVersions) - Use url.URL object to create the full URL (also modified ListObjectVersions and CreateBucket) - replace allocation with new() with a local var (also modified ListObjectVersions)
scality · Oct 22, 2024 · d2f8752 · d2f8752
1 parent 2b13d95
commit d2f8752
Show file tree

Hide file tree

Showing 3 changed files with 55 additions and 53 deletions.
diff --git a/go/createbucket.go b/go/createbucket.go
@@ -52,7 +52,9 @@ func (client *BucketClient) CreateBucket(ctx context.Context,
 	if parsedOpts.sessionId > 0 {
 		query.Set("raftsession", strconv.Itoa(parsedOpts.sessionId))
 	}
-	resource += "?" + query.Encode()
+	u, _ := url.Parse(resource)
+	u.RawQuery = query.Encode()
+	resource = u.String()
 	requestOptions := []RequestOption{
 		RequestBodyOption(bucketAttributes),
 		RequestBodyContentTypeOption("application/json"),

diff --git a/go/listbasic.go b/go/listbasic.go
@@ -13,31 +13,31 @@ type ListBasicOption func(*listBasicOptionSet) error
 // ListBasicGTOption only lists keys greater than the given argument
 func ListBasicGTOption(gt string) ListBasicOption {
 	return func(opts *listBasicOptionSet) error {
-		opts.gt = gt
+		opts.gt = &gt
 		return nil
 	}
 }
 
 // ListBasicGTEOption only lists keys greater or equal to the given argument
 func ListBasicGTEOption(gte string) ListBasicOption {
 	return func(opts *listBasicOptionSet) error {
-		opts.gte = gte
+		opts.gte = &gte
 		return nil
 	}
 }
 
 // ListBasicLTOption only lists keys less than the given argument
 func ListBasicLTOption(lt string) ListBasicOption {
 	return func(opts *listBasicOptionSet) error {
-		opts.lt = lt
+		opts.lt = &lt
 		return nil
 	}
 }
 
 // ListBasicLTEOption only lists keys less or equal to the given argument
 func ListBasicLTEOption(lte string) ListBasicOption {
 	return func(opts *listBasicOptionSet) error {
-		opts.lte = lte
+		opts.lte = &lte
 		return nil
 	}
 }
@@ -48,7 +48,7 @@ func ListBasicMaxKeysOption(maxKeys int) ListBasicOption {
 		if maxKeys < 0 || maxKeys > 10000 {
 			return fmt.Errorf("maxKeys=%d is out of the valid range [0, 10000]", maxKeys)
 		}
-		opts.maxKeys = maxKeys
+		opts.maxKeys = &maxKeys
 		return nil
 	}
 }
@@ -83,19 +83,17 @@ type ListBasicEntry struct {
 type ListBasicResponse []ListBasicEntry
 
 type listBasicOptionSet struct {
-	gt       string
-	gte      string
-	lt       string
-	lte      string
-	maxKeys  int
+	gt       *string
+	gte      *string
+	lt       *string
+	lte      *string
+	maxKeys  *int
 	noKeys   bool
 	noValues bool
 }
 
 func parseListBasicOptions(opts []ListBasicOption) (listBasicOptionSet, error) {
-	parsedOpts := listBasicOptionSet{
-		maxKeys: -1,
-	}
+	parsedOpts := listBasicOptionSet{}
 	for _, opt := range opts {
 		err := opt(&parsedOpts)
 		if err != nil {
@@ -117,37 +115,39 @@ func (client *BucketClient) ListBasic(ctx context.Context,
 			"ListBasic", "GET", client.Endpoint, resource, 0, "", err,
 		}
 	}
-	if options.gt != "" {
-		query.Set("gt", options.gt)
+	if options.gt != nil {
+		query.Set("gt", *options.gt)
 	}
-	if options.gte != "" {
-		query.Set("gte", options.gte)
+	if options.gte != nil {
+		query.Set("gte", *options.gte)
 	}
-	if options.lt != "" {
-		query.Set("lt", options.lt)
+	if options.lt != nil {
+		query.Set("lt", *options.lt)
 	}
-	if options.lte != "" {
-		query.Set("lte", options.lte)
+	if options.lte != nil {
+		query.Set("lte", *options.lte)
 	}
-	if options.maxKeys != -1 {
-		query.Set("maxKeys", strconv.Itoa(options.maxKeys))
+	if options.maxKeys != nil {
+		query.Set("maxKeys", strconv.Itoa(*options.maxKeys))
 	}
 	if options.noKeys {
 		query.Set("keys", "false")
 	}
 	if options.noValues {
 		query.Set("values", "false")
 	}
-	resource += "?" + query.Encode()
+	u, _ := url.Parse(resource)
+	u.RawQuery = query.Encode()
+	resource = u.String()
 	responseBody, err := client.Request(ctx, "ListBasic", "GET", resource)
 	if err != nil {
 		return nil, err
 	}
-	var parsedResponse = new(ListBasicResponse)
-	jsonErr := json.Unmarshal(responseBody, parsedResponse)
+	var parsedResponse ListBasicResponse
+	jsonErr := json.Unmarshal(responseBody, &parsedResponse)
 	if jsonErr != nil {
 		return nil, ErrorMalformedResponse("ListBasic", "GET",
 			client.Endpoint, resource, jsonErr)
 	}
-	return parsedResponse, nil
+	return &parsedResponse, nil
 }
diff --git a/go/listobjectversions.go b/go/listobjectversions.go
@@ -12,8 +12,8 @@ type ListObjectVersionsOption func(*listObjectVersionsOptionSet) error
 
 func ListObjectVersionsMarkerOption(keyMarker string, versionIdMarker string) ListObjectVersionsOption {
 	return func(opts *listObjectVersionsOptionSet) error {
-		opts.keyMarker = keyMarker
-		opts.versionIdMarker = versionIdMarker
+		opts.keyMarker = &keyMarker
+		opts.versionIdMarker = &versionIdMarker
 		return nil
 	}
 }
@@ -23,7 +23,7 @@ func ListObjectVersionsMaxKeysOption(maxKeys int) ListObjectVersionsOption {
 		if maxKeys < 0 || maxKeys > 10000 {
 			return fmt.Errorf("maxKeys=%d is out of the valid range [0, 10000]", maxKeys)
 		}
-		opts.maxKeys = maxKeys
+		opts.maxKeys = &maxKeys
 		return nil
 	}
 }
@@ -38,8 +38,8 @@ func ListObjectVersionsMaxKeysOption(maxKeys int) ListObjectVersionsOption {
 // client.
 func ListObjectVersionsLastMarkerOption(lastKeyMarker string, lastVersionIdMarker string) ListObjectVersionsOption {
 	return func(opts *listObjectVersionsOptionSet) error {
-		opts.lastKeyMarker = lastKeyMarker
-		opts.lastVersionIdMarker = lastVersionIdMarker
+		opts.lastKeyMarker = &lastKeyMarker
+		opts.lastVersionIdMarker = &lastVersionIdMarker
 		return nil
 	}
 }
@@ -59,17 +59,15 @@ type ListObjectVersionsResponse struct {
 }
 
 type listObjectVersionsOptionSet struct {
-	keyMarker           string
-	versionIdMarker     string
-	maxKeys             int
-	lastKeyMarker       string
-	lastVersionIdMarker string
+	keyMarker           *string
+	versionIdMarker     *string
+	maxKeys             *int
+	lastKeyMarker       *string
+	lastVersionIdMarker *string
 }
 
 func parseListObjectVersionsOptions(opts []ListObjectVersionsOption) (listObjectVersionsOptionSet, error) {
-	parsedOpts := listObjectVersionsOptionSet{
-		maxKeys: -1,
-	}
+	parsedOpts := listObjectVersionsOptionSet{}
 	for _, opt := range opts {
 		err := opt(&parsedOpts)
 		if err != nil {
@@ -91,27 +89,29 @@ func (client *BucketClient) ListObjectVersions(ctx context.Context,
 			"ListObjectVersions", "GET", client.Endpoint, resource, 0, "", err,
 		}
 	}
-	if options.keyMarker != "" {
-		query.Set("keyMarker", options.keyMarker)
-		query.Set("versionIdMarker", options.versionIdMarker)
+	if options.keyMarker != nil {
+		query.Set("keyMarker", *options.keyMarker)
+		query.Set("versionIdMarker", *options.versionIdMarker)
 	}
-	if options.maxKeys != -1 {
-		query.Set("maxKeys", strconv.Itoa(options.maxKeys))
+	if options.maxKeys != nil {
+		query.Set("maxKeys", strconv.Itoa(*options.maxKeys))
 	}
-	resource += "?" + query.Encode()
+	u, _ := url.Parse(resource)
+	u.RawQuery = query.Encode()
+	resource = u.String()
 	responseBody, err := client.Request(ctx, "ListObjectVersions", "GET", resource)
 	if err != nil {
 		return nil, err
 	}
-	var parsedResponse = new(ListObjectVersionsResponse)
-	jsonErr := json.Unmarshal(responseBody, parsedResponse)
+	var parsedResponse ListObjectVersionsResponse
+	jsonErr := json.Unmarshal(responseBody, &parsedResponse)
 	if jsonErr != nil {
 		return nil, ErrorMalformedResponse("ListObjectVersions", "GET",
 			client.Endpoint, resource, jsonErr)
 	}
-	if options.lastKeyMarker != "" {
-		truncateListObjectVersionsResponse(parsedResponse,
-			options.lastKeyMarker, options.lastVersionIdMarker)
+	if options.lastKeyMarker != nil {
+		truncateListObjectVersionsResponse(&parsedResponse,
+			*options.lastKeyMarker, *options.lastVersionIdMarker)
 	}
-	return parsedResponse, nil
+	return &parsedResponse, nil
 }