-
Notifications
You must be signed in to change notification settings - Fork 38
useragent: add FromSlice function
#1389
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 1 commit
c0aa19d
234c90f
ac02a7c
5a24a5b
e509f3f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,49 @@ | ||||||
| // Copyright (c) HashiCorp, Inc. | ||||||
| // SPDX-License-Identifier: MPL-2.0 | ||||||
|
|
||||||
| package useragent | ||||||
|
|
||||||
| import ( | ||||||
| "strings" | ||||||
|
|
||||||
| "github.com/hashicorp/aws-sdk-go-base/v2/internal/config" | ||||||
| "github.com/hashicorp/aws-sdk-go-base/v2/internal/slices" | ||||||
| ) | ||||||
|
|
||||||
| // FromSlice applies the conversion defined in [fromString] to all elements | ||||||
| // of a slice | ||||||
| // | ||||||
| // Slices of types which cannot assert to a string, empty string values, and string | ||||||
| // values which do not match the expected `{product}/{version} ({comment})` | ||||||
| // pattern (where version and comment are optional) return a zero value struct. | ||||||
| func FromSlice[T any](sl []T) config.UserAgentProducts { | ||||||
| return slices.ApplyToAll(sl, func(v T) config.UserAgentProduct { | ||||||
| if s, ok := any(v).(string); ok && s != "" { | ||||||
| return fromString(s) | ||||||
| } | ||||||
| return config.UserAgentProduct{} | ||||||
| }) | ||||||
| } | ||||||
|
|
||||||
| // fromString separates the provided string into the constituent parts | ||||||
| // expected by the UserAgentProduct struct | ||||||
| // | ||||||
| // Values which do not match the expected `{product}/{version} ({comment})` | ||||||
| // pattern, where version and comment are optional, return a zero value struct. | ||||||
| func fromString(s string) config.UserAgentProduct { | ||||||
| parts := strings.Split(s, "/") | ||||||
| switch len(parts) { | ||||||
| case 1: | ||||||
| return config.UserAgentProduct{Name: parts[0]} | ||||||
| case 2: //nolint: mnd | ||||||
| subparts := strings.Split(parts[1], "(") | ||||||
| if len(subparts) == 2 { //nolint: mnd | ||||||
|
||||||
| if len(subparts) == 2 { //nolint: mnd | |
| if len(subparts) == 2 && strings.HasSuffix(parts[1], ")") { //nolint: mnd |
probably not worth "fixing" but an edge case "my-product/v1.2.3 (not a comment" that would parse wrong
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A missing closing parenthesis would still parse correctly because strings.TrimSuffix(subparts[1], ")") just does nothing. For example, this test case passes.
{
"malformed comment closing",
"my-product/v1.2.3 (a comment",
config.UserAgentProduct{
Name: "my-product",
Version: "v1.2.3",
Comment: "a comment",
},
},--- PASS: Test_fromString (0.00s)
--- PASS: Test_fromString/malformed_comment_closing (0.00s)
There is weirdness if the opening parenthesis is missed because the version and comment will get mashed together into the Version struct field, but it winds up rendering the same in the eventual UA.
In general this conversion logic is just a shim until V2 of the AWSCC provider, where we'll deprecate the structured object user_agent in favor of a list of strings. At that time the shared components in this repo can also be simplified to just pass the strings through as-is without all this extra manipulation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll still go ahead and add some test cases for malformed comment patterns, just so we document the edge cases exists and what the expected handling is.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,116 @@ | ||
| // Copyright (c) HashiCorp, Inc. | ||
| // SPDX-License-Identifier: MPL-2.0 | ||
|
|
||
| package useragent | ||
|
|
||
| import ( | ||
| "reflect" | ||
| "testing" | ||
|
|
||
| "github.com/hashicorp/aws-sdk-go-base/v2/internal/config" | ||
| ) | ||
|
|
||
| func TestFromSlice(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| tests := []struct { | ||
| name string | ||
| s []any | ||
| want config.UserAgentProducts | ||
| }{ | ||
| { | ||
| "nil", | ||
| nil, | ||
| config.UserAgentProducts{}, | ||
| }, | ||
| { | ||
| "non-string element", | ||
| []any{false}, | ||
| config.UserAgentProducts{config.UserAgentProduct{}}, | ||
| }, | ||
| { | ||
| "valid string", | ||
| []any{"my-product/v1.2.3"}, | ||
| config.UserAgentProducts{ | ||
| config.UserAgentProduct{ | ||
| Name: "my-product", | ||
| Version: "v1.2.3", | ||
| }, | ||
| }, | ||
| }, | ||
| { | ||
| "valid and invalid string", | ||
| []any{"my-product/v1.2.3", "foo/bar/baz/qux"}, | ||
| config.UserAgentProducts{ | ||
| config.UserAgentProduct{ | ||
| Name: "my-product", | ||
| Version: "v1.2.3", | ||
| }, | ||
| config.UserAgentProduct{}, | ||
| }, | ||
| }, | ||
| } | ||
| for _, tt := range tests { | ||
| t.Run(tt.name, func(t *testing.T) { | ||
| t.Parallel() | ||
| got := FromSlice(tt.s) | ||
| if !reflect.DeepEqual(got, tt.want) { | ||
| t.Errorf("FromSlice() = %+v, want %+v", got, tt.want) | ||
| } | ||
| }) | ||
| } | ||
| } | ||
|
|
||
| func Test_fromString(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| tests := []struct { | ||
| name string | ||
| s string | ||
| want config.UserAgentProduct | ||
| }{ | ||
| { | ||
| "empty", | ||
| "", | ||
| config.UserAgentProduct{}, | ||
| }, | ||
| { | ||
| "name only", | ||
| "my-product", | ||
| config.UserAgentProduct{ | ||
| Name: "my-product", | ||
| }, | ||
| }, | ||
| { | ||
| "name and version", | ||
| "my-product/v1.2.3", | ||
| config.UserAgentProduct{ | ||
| Name: "my-product", | ||
| Version: "v1.2.3", | ||
| }, | ||
| }, | ||
| { | ||
| "name, version, and comment", | ||
| "my-product/v1.2.3 (a comment)", | ||
| config.UserAgentProduct{ | ||
| Name: "my-product", | ||
| Version: "v1.2.3", | ||
| Comment: "a comment", | ||
| }, | ||
| }, | ||
| { | ||
| "all the slash", | ||
| "foo/bar/baz/qux", | ||
| config.UserAgentProduct{}, | ||
| }, | ||
| } | ||
| for _, tt := range tests { | ||
| t.Run(tt.name, func(t *testing.T) { | ||
| t.Parallel() | ||
| got := fromString(tt.s) | ||
| if !reflect.DeepEqual(got, tt.want) { | ||
| t.Errorf("fromString() = %+v, want %+v", got, tt.want) | ||
| } | ||
| }) | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
doesn't really need the empty check since
fromStringchecks that?