From a743ff565474ad1f97d8296879af84e7a56a032a Mon Sep 17 00:00:00 2001 From: Marcos Gaeta <31750075+mgaeta@users.noreply.github.com> Date: Tue, 20 Aug 2024 10:43:08 -0700 Subject: [PATCH] ref: Helpers (#204) * fix docstrings * remove pkg/helpers/helpers.go * Add test helpers * refactors based on coderabbit suggestions --- pkg/annotations/annotations.go | 2 +- pkg/field/fields.go | 2 +- pkg/field/validation.go | 2 +- pkg/helpers/helpers.go | 164 +++---------------------------- pkg/helpers/helpers_test.go | 73 -------------- pkg/ratelimit/grpc.go | 2 +- pkg/ratelimit/http.go | 127 ++++++++++++++++++++++++ pkg/ratelimit/http_test.go | 39 ++++++++ pkg/test/integration.go | 58 +++++++++++ pkg/test/pagination.go | 91 +++++++++++++++++ pkg/test/ratelimit.go | 58 +++++++++++ pkg/types/resource/parse.go | 18 ++++ pkg/types/resource/parse_test.go | 54 ++++++++++ pkg/types/resource/user_trait.go | 1 + pkg/uhttp/contenttype.go | 25 +++++ pkg/uhttp/contenttype_test.go | 28 ++++++ pkg/uhttp/wrapper.go | 14 +-- 17 files changed, 521 insertions(+), 237 deletions(-) delete mode 100644 pkg/helpers/helpers_test.go create mode 100644 pkg/ratelimit/http.go create mode 100644 pkg/ratelimit/http_test.go create mode 100644 pkg/test/integration.go create mode 100644 pkg/test/pagination.go create mode 100644 pkg/test/ratelimit.go create mode 100644 pkg/types/resource/parse.go create mode 100644 pkg/types/resource/parse_test.go create mode 100644 pkg/uhttp/contenttype.go create mode 100644 pkg/uhttp/contenttype_test.go diff --git a/pkg/annotations/annotations.go b/pkg/annotations/annotations.go index 19eda948..e1284bd1 100644 --- a/pkg/annotations/annotations.go +++ b/pkg/annotations/annotations.go @@ -10,7 +10,7 @@ import ( type Annotations []*anypb.Any -// Convenience function to create annotations. +// New - Convenience function to create annotations. func New(msgs ...proto.Message) Annotations { annos := Annotations{} for _, msg := range msgs { diff --git a/pkg/field/fields.go b/pkg/field/fields.go index 4413407e..006d6e38 100644 --- a/pkg/field/fields.go +++ b/pkg/field/fields.go @@ -51,7 +51,7 @@ func (s SchemaField) String() (string, error) { return value, nil } -// StringSlice retuns the default value as a string array. +// StringSlice returns the default value as a string array. func (s SchemaField) StringSlice() ([]string, error) { value, ok := s.DefaultValue.([]string) if !ok { diff --git a/pkg/field/validation.go b/pkg/field/validation.go index 05812b57..576d880d 100644 --- a/pkg/field/validation.go +++ b/pkg/field/validation.go @@ -33,7 +33,7 @@ func (e *ErrConfigurationMissingFields) Push(err error) { // - repeated fields (by name) are defined // - if sets of fields are mutually exclusive and required // together at the same time -// - if fields depedent on themselves +// - if fields depend on themselves func Validate(c Configuration, v *viper.Viper) error { present := make(map[string]int) missingFieldsError := &ErrConfigurationMissingFields{} diff --git a/pkg/helpers/helpers.go b/pkg/helpers/helpers.go index b4da5ed8..a6758f05 100644 --- a/pkg/helpers/helpers.go +++ b/pkg/helpers/helpers.go @@ -2,171 +2,29 @@ package helpers import ( "net/http" - "strconv" - "strings" - "time" v2 "github.com/conductorone/baton-sdk/pb/c1/connector/v2" - "google.golang.org/protobuf/types/known/timestamppb" + "github.com/conductorone/baton-sdk/pkg/ratelimit" + "github.com/conductorone/baton-sdk/pkg/types/resource" + "github.com/conductorone/baton-sdk/pkg/uhttp" ) +// Deprecated: see resource.SplitFullName. func SplitFullName(name string) (string, string) { - names := strings.SplitN(name, " ", 2) - var firstName, lastName string - - switch len(names) { - case 1: - firstName = names[0] - case 2: - firstName = names[0] - lastName = names[1] - } - - return firstName, lastName -} - -var limitHeaders = []string{ - "X-Ratelimit-Limit", - "Ratelimit-Limit", - "X-RateLimit-Requests-Limit", // Linear uses a non-standard header -} - -var remainingHeaders = []string{ - "X-Ratelimit-Remaining", - "Ratelimit-Remaining", - "X-RateLimit-Requests-Remaining", // Linear uses a non-standard header -} - -var resetAtHeaders = []string{ - "X-Ratelimit-Reset", - "Ratelimit-Reset", - "X-RateLimit-Requests-Reset", // Linear uses a non-standard header - "Retry-After", // Often returned with 429 -} - -const thirtyYears = 60 * 60 * 24 * 365 * (2000 - 1970) - -// Many APIs don't follow standards and return incorrect datetimes. This function tries to handle those cases. -func parseTime(timeStr string) (time.Time, error) { - var t time.Time - res, err := strconv.ParseInt(timeStr, 10, 64) - if err != nil { - t, err = time.Parse(time.RFC850, timeStr) - if err != nil { - // Datetimes should be RFC850 but some APIs return RFC3339 - t, err = time.Parse(time.RFC3339, timeStr) - } - return t, err - } - - // Times are supposed to be in seconds, but some APIs return milliseconds - if res > thirtyYears*1000 { - res /= 1000 - } - - // Times are supposed to be offsets, but some return absolute seconds since 1970. - if res > thirtyYears { - // If more than 30 years, it's probably an absolute timestamp - t = time.Unix(res, 0) - } else { - // Otherwise, it's a relative timestamp - t = time.Now().Add(time.Second * time.Duration(res)) - } - - return t, nil + return resource.SplitFullName(name) } +// Deprecated: see ratelimit.ExtractRateLimitData. func ExtractRateLimitData(statusCode int, header *http.Header) (*v2.RateLimitDescription, error) { - if header == nil { - return nil, nil - } - - var rlstatus v2.RateLimitDescription_Status - - var limit int64 - var err error - for _, limitHeader := range limitHeaders { - limitStr := header.Get(limitHeader) - if limitStr != "" { - limit, err = strconv.ParseInt(limitStr, 10, 64) - if err != nil { - return nil, err - } - break - } - } - - var remaining int64 - for _, remainingHeader := range remainingHeaders { - remainingStr := header.Get(remainingHeader) - if remainingStr != "" { - remaining, err = strconv.ParseInt(remainingStr, 10, 64) - if err != nil { - return nil, err - } - break - } - } - if remaining > 0 { - rlstatus = v2.RateLimitDescription_STATUS_OK - } - - var resetAt time.Time - for _, resetAtHeader := range resetAtHeaders { - resetAtStr := header.Get(resetAtHeader) - if resetAtStr != "" { - resetAt, err = parseTime(resetAtStr) - if err != nil { - return nil, err - } - break - } - } - - if statusCode == http.StatusTooManyRequests { - rlstatus = v2.RateLimitDescription_STATUS_OVERLIMIT - remaining = 0 - } - - // If we didn't get any rate limit headers and status code is 429, return some sane defaults - if remaining == 0 && resetAt.IsZero() && rlstatus == v2.RateLimitDescription_STATUS_OVERLIMIT { - limit = 1 - resetAt = time.Now().Add(time.Second * 60) - } - - return &v2.RateLimitDescription{ - Status: rlstatus, - Limit: limit, - Remaining: remaining, - ResetAt: timestamppb.New(resetAt), - }, nil + return ratelimit.ExtractRateLimitData(statusCode, header) } +// Deprecated: see contenttype.IsJSONContentType. func IsJSONContentType(contentType string) bool { - if !strings.HasPrefix(contentType, "application") { - return false - } - - if !strings.Contains(contentType, "json") { - return false - } - - return true -} - -var xmlContentTypes []string = []string{ - "text/xml", - "application/xml", + return uhttp.IsJSONContentType(contentType) } +// Deprecated: see contenttype.IsXMLContentType. func IsXMLContentType(contentType string) bool { - // there are some janky APIs out there - normalizedContentType := strings.TrimSpace(strings.ToLower(contentType)) - - for _, xmlContentType := range xmlContentTypes { - if strings.HasPrefix(normalizedContentType, xmlContentType) { - return true - } - } - return false + return uhttp.IsXMLContentType(contentType) } diff --git a/pkg/helpers/helpers_test.go b/pkg/helpers/helpers_test.go deleted file mode 100644 index 004784db..00000000 --- a/pkg/helpers/helpers_test.go +++ /dev/null @@ -1,73 +0,0 @@ -package helpers - -import ( - "net/http" - "testing" - "time" - - "github.com/stretchr/testify/require" -) - -func TestHelpers_SplitFullName(t *testing.T) { - firstName, lastName := SplitFullName("Prince") - require.Equal(t, "Prince", firstName) - require.Equal(t, "", lastName) - - firstName, lastName = SplitFullName("John Smith") - require.Equal(t, "John", firstName) - require.Equal(t, "Smith", lastName) - - firstName, lastName = SplitFullName("John Jacob Jingleheimer Schmidt") - require.Equal(t, "John", firstName) - require.Equal(t, "Jacob Jingleheimer Schmidt", lastName) -} - -func TestHelpers_ExtractRateLimitData(t *testing.T) { - n := time.Now() - - resp := &http.Response{ - StatusCode: http.StatusOK, - Header: map[string][]string{ - "X-Ratelimit-Limit": {"100"}, - "X-Ratelimit-Remaining": {"50"}, - "X-Ratelimit-Reset": {"30"}, - }, - } - - rl, err := ExtractRateLimitData(resp.StatusCode, &resp.Header) - require.NoError(t, err) - require.Equal(t, int64(100), rl.Limit) - require.Equal(t, int64(50), rl.Remaining) - require.Equal(t, n.Add(time.Second*30).Unix(), rl.ResetAt.AsTime().Unix()) - - resp = &http.Response{ - StatusCode: http.StatusTooManyRequests, - Header: map[string][]string{}, - } - - rl, err = ExtractRateLimitData(resp.StatusCode, &resp.Header) - require.NoError(t, err) - require.Equal(t, int64(1), rl.Limit) - require.Equal(t, int64(0), rl.Remaining) - require.Equal(t, n.Add(time.Second*60).Unix(), rl.ResetAt.AsTime().Unix()) -} - -func TestHelpers_IsJSONContentType_Success(t *testing.T) { - resp := &http.Response{ - Header: map[string][]string{ - "Content-Type": {"application/vdn+json"}, - }, - } - h := resp.Header.Get("Content-Type") - require.True(t, IsJSONContentType(h)) -} - -func TestHelpers_IsJSONContentType_Failure(t *testing.T) { - resp := &http.Response{ - Header: map[string][]string{ - "Content-Type": {"application/xml"}, - }, - } - h := resp.Header.Get("Content-Type") - require.False(t, IsJSONContentType(h)) -} diff --git a/pkg/ratelimit/grpc.go b/pkg/ratelimit/grpc.go index 5f58598b..0483b960 100644 --- a/pkg/ratelimit/grpc.go +++ b/pkg/ratelimit/grpc.go @@ -83,7 +83,7 @@ func getRatelimitDescriptors(ctx context.Context, method string, in interface{}, return ret } -// UnaryServerInterceptor returns a new unary server interceptors that adds zap.Logger to the context. +// UnaryInterceptor returns a new unary server interceptors that adds zap.Logger to the context. func UnaryInterceptor(now func() time.Time, descriptors ...*ratelimitV1.RateLimitDescriptors_Entry) grpc.UnaryClientInterceptor { return func(ctx context.Context, method string, req, reply interface{}, cc *grpc.ClientConn, invoker grpc.UnaryInvoker, opts ...grpc.CallOption) error { // If this is a call to the rate limit service, skip it diff --git a/pkg/ratelimit/http.go b/pkg/ratelimit/http.go new file mode 100644 index 00000000..9db8c5b2 --- /dev/null +++ b/pkg/ratelimit/http.go @@ -0,0 +1,127 @@ +package ratelimit + +import ( + "net/http" + "strconv" + "time" + + v2 "github.com/conductorone/baton-sdk/pb/c1/connector/v2" + "google.golang.org/protobuf/types/known/timestamppb" +) + +var limitHeaders = []string{ + "X-Ratelimit-Limit", + "Ratelimit-Limit", + "X-RateLimit-Requests-Limit", // Linear uses a non-standard header +} + +var remainingHeaders = []string{ + "X-Ratelimit-Remaining", + "Ratelimit-Remaining", + "X-RateLimit-Requests-Remaining", // Linear uses a non-standard header +} + +var resetAtHeaders = []string{ + "X-Ratelimit-Reset", + "Ratelimit-Reset", + "X-RateLimit-Requests-Reset", // Linear uses a non-standard header + "Retry-After", // Often returned with 429 +} + +const thirtyYears = 60 * 60 * 24 * 365 * (2000 - 1970) + +// Many APIs don't follow standards and return incorrect datetimes. This function tries to handle those cases. +func parseTime(timeStr string) (time.Time, error) { + var t time.Time + res, err := strconv.ParseInt(timeStr, 10, 64) + if err != nil { + t, err = time.Parse(time.RFC850, timeStr) + if err != nil { + // Datetimes should be RFC850 but some APIs return RFC3339 + t, err = time.Parse(time.RFC3339, timeStr) + } + return t, err + } + + // Times are supposed to be in seconds, but some APIs return milliseconds + if res > thirtyYears*1000 { + res /= 1000 + } + + // Times are supposed to be offsets, but some return absolute seconds since 1970. + if res > thirtyYears { + // If more than 30 years, it's probably an absolute timestamp + t = time.Unix(res, 0) + } else { + // Otherwise, it's a relative timestamp + t = time.Now().Add(time.Second * time.Duration(res)) + } + + return t, nil +} + +func ExtractRateLimitData(statusCode int, header *http.Header) (*v2.RateLimitDescription, error) { + if header == nil { + return nil, nil + } + + var rlstatus v2.RateLimitDescription_Status + + var limit int64 + var err error + for _, limitHeader := range limitHeaders { + limitStr := header.Get(limitHeader) + if limitStr != "" { + limit, err = strconv.ParseInt(limitStr, 10, 64) + if err != nil { + return nil, err + } + break + } + } + + var remaining int64 + for _, remainingHeader := range remainingHeaders { + remainingStr := header.Get(remainingHeader) + if remainingStr != "" { + remaining, err = strconv.ParseInt(remainingStr, 10, 64) + if err != nil { + return nil, err + } + break + } + } + if remaining > 0 { + rlstatus = v2.RateLimitDescription_STATUS_OK + } + + var resetAt time.Time + for _, resetAtHeader := range resetAtHeaders { + resetAtStr := header.Get(resetAtHeader) + if resetAtStr != "" { + resetAt, err = parseTime(resetAtStr) + if err != nil { + return nil, err + } + break + } + } + + if statusCode == http.StatusTooManyRequests { + rlstatus = v2.RateLimitDescription_STATUS_OVERLIMIT + remaining = 0 + } + + // If we didn't get any rate limit headers and status code is 429, return some sane defaults + if remaining == 0 && resetAt.IsZero() && rlstatus == v2.RateLimitDescription_STATUS_OVERLIMIT { + limit = 1 + resetAt = time.Now().Add(time.Second * 60) + } + + return &v2.RateLimitDescription{ + Status: rlstatus, + Limit: limit, + Remaining: remaining, + ResetAt: timestamppb.New(resetAt), + }, nil +} diff --git a/pkg/ratelimit/http_test.go b/pkg/ratelimit/http_test.go new file mode 100644 index 00000000..b5d20351 --- /dev/null +++ b/pkg/ratelimit/http_test.go @@ -0,0 +1,39 @@ +package ratelimit + +import ( + "net/http" + "testing" + "time" + + "github.com/stretchr/testify/require" +) + +func TestHelpers_ExtractRateLimitData(t *testing.T) { + n := time.Now() + + resp := &http.Response{ + StatusCode: http.StatusOK, + Header: map[string][]string{ + "X-Ratelimit-Limit": {"100"}, + "X-Ratelimit-Remaining": {"50"}, + "X-Ratelimit-Reset": {"30"}, + }, + } + + rl, err := ExtractRateLimitData(resp.StatusCode, &resp.Header) + require.NoError(t, err) + require.Equal(t, int64(100), rl.Limit) + require.Equal(t, int64(50), rl.Remaining) + require.Equal(t, n.Add(time.Second*30).Unix(), rl.ResetAt.AsTime().Unix()) + + resp = &http.Response{ + StatusCode: http.StatusTooManyRequests, + Header: map[string][]string{}, + } + + rl, err = ExtractRateLimitData(resp.StatusCode, &resp.Header) + require.NoError(t, err) + require.Equal(t, int64(1), rl.Limit) + require.Equal(t, int64(0), rl.Remaining) + require.Equal(t, n.Add(time.Second*60).Unix(), rl.ResetAt.AsTime().Unix()) +} diff --git a/pkg/test/integration.go b/pkg/test/integration.go new file mode 100644 index 00000000..b64877d1 --- /dev/null +++ b/pkg/test/integration.go @@ -0,0 +1,58 @@ +package test + +import ( + "context" + "testing" + + v2 "github.com/conductorone/baton-sdk/pb/c1/connector/v2" + "github.com/conductorone/baton-sdk/pkg/connectorbuilder" + "github.com/stretchr/testify/require" +) + +type builder interface { + connectorbuilder.ResourceProvisioner + connectorbuilder.ResourceSyncer +} + +func assertGrants(ctx context.Context, + t *testing.T, + c builder, + resource *v2.Resource, +) []*v2.Grant { + grants, annotations, err := ExhaustGrantPagination(ctx, c, resource) + require.Nil(t, err) + AssertNoRatelimitAnnotations(t, annotations) + return grants +} + +// GrantsIntegrationTest - create a grant and then revoke it. Any connector that +// implements provisioning should be able to successfully run this test. +func GrantsIntegrationTest( + ctx context.Context, + t *testing.T, + c builder, + principal *v2.Resource, + entitlement *v2.Entitlement, +) { + grant := v2.Grant{ + Entitlement: entitlement, + Principal: principal, + } + + grantsBefore := assertGrants(ctx, t, c, entitlement.Resource) + grantCount := len(grantsBefore) + + annotations, err := c.Grant(ctx, principal, entitlement) + require.Nil(t, err) + AssertNoRatelimitAnnotations(t, annotations) + + grantsDuring := assertGrants(ctx, t, c, entitlement.Resource) + require.Len(t, grantsDuring, grantCount+1) + + annotations, err = c.Revoke(ctx, &grant) + require.Nil(t, err) + AssertNoRatelimitAnnotations(t, annotations) + + grantsAfter := assertGrants(ctx, t, c, entitlement.Resource) + require.Len(t, grantsAfter, grantCount) +} diff --git a/pkg/test/pagination.go b/pkg/test/pagination.go new file mode 100644 index 00000000..fb4ae92d --- /dev/null +++ b/pkg/test/pagination.go @@ -0,0 +1,91 @@ +package test + +import ( + "context" + "fmt" + + v2 "github.com/conductorone/baton-sdk/pb/c1/connector/v2" + "github.com/conductorone/baton-sdk/pkg/annotations" + "github.com/conductorone/baton-sdk/pkg/connectorbuilder" + "github.com/conductorone/baton-sdk/pkg/pagination" +) + +// exhaustPagination - some integration tests don't care about how pagination +// works and just want to get the full list of resources. +func exhaustPagination[T any, R any]( + ctx context.Context, + resource *R, + f func( + ctx context.Context, + resource *R, + pToken *pagination.Token, + ) ( + []*T, + string, + annotations.Annotations, + error, + ), +) ( + []*T, + annotations.Annotations, + error, +) { + var outputAnnotations annotations.Annotations + pToken := pagination.Token{} + objects := make([]*T, 0) + for { + nextObjects, nextToken, listAnnotations, err := f(ctx, resource, &pToken) + if err != nil { + return nil, nil, err + } + objects = append(objects, nextObjects...) + + if IsRatelimited(listAnnotations) { + return nil, + listAnnotations, + fmt.Errorf("request was ratelimited, expected not to be ratelimited") + } + if nextToken == "" { + // Just return the final set of annotations. + outputAnnotations = listAnnotations + break + } + pToken.Token = nextToken + } + return objects, outputAnnotations, nil +} + +func ExhaustResourcePagination( + ctx context.Context, + c connectorbuilder.ResourceSyncer, +) ( + []*v2.Resource, + annotations.Annotations, + error, +) { + return exhaustPagination[v2.Resource, v2.ResourceId](ctx, nil, c.List) +} + +func ExhaustEntitlementPagination( + ctx context.Context, + c connectorbuilder.ResourceSyncer, + resource *v2.Resource, +) ( + []*v2.Entitlement, + annotations.Annotations, + error, +) { + return exhaustPagination[v2.Entitlement, v2.Resource](ctx, resource, c.Entitlements) +} + +func ExhaustGrantPagination( + ctx context.Context, + c connectorbuilder.ResourceSyncer, + resource *v2.Resource, +) ( + []*v2.Grant, + annotations.Annotations, + error, +) { + return exhaustPagination[v2.Grant, v2.Resource](ctx, resource, c.Grants) +} diff --git a/pkg/test/ratelimit.go b/pkg/test/ratelimit.go new file mode 100644 index 00000000..9065aa76 --- /dev/null +++ b/pkg/test/ratelimit.go @@ -0,0 +1,58 @@ +package test + +import ( + "slices" + "testing" + + v2 "github.com/conductorone/baton-sdk/pb/c1/connector/v2" + "github.com/conductorone/baton-sdk/pkg/annotations" + "google.golang.org/protobuf/types/known/anypb" +) + +func isRatelimitingAnnotation(annotation *anypb.Any) bool { + var ratelimitDescription v2.RateLimitDescription + err := annotation.UnmarshalTo(&ratelimitDescription) + if err != nil { + // Some other kind of annotation. + return false + } + return slices.Contains( + []v2.RateLimitDescription_Status{ + v2.RateLimitDescription_STATUS_ERROR, + v2.RateLimitDescription_STATUS_OVERLIMIT, + }, + ratelimitDescription.Status, + ) +} + +func IsRatelimited(actualAnnotations annotations.Annotations) bool { + for _, annotation := range actualAnnotations { + if isRatelimitingAnnotation(annotation) { + return true + } + } + return false +} + +func AssertWasRatelimited( + t *testing.T, + actualAnnotations annotations.Annotations, +) { + if !IsRatelimited(actualAnnotations) { + t.Fatal("request was _not_ ratelimited, expected to be ratelimited") + } +} + +// AssertNoRatelimitAnnotations - the annotations that ResourceSyncers return +// can contain "informational" ratelimit annotations that let the caller know +// how much quota they have left and that their request was _not_ rejected. This +// helper just asserts that there are no "your request was ratelimited" +// annotations. +func AssertNoRatelimitAnnotations( + t *testing.T, + actualAnnotations annotations.Annotations, +) { + if IsRatelimited(actualAnnotations) { + t.Fatal("request was ratelimited, expected not to be ratelimited") + } +} diff --git a/pkg/types/resource/parse.go b/pkg/types/resource/parse.go new file mode 100644 index 00000000..f5e41ab1 --- /dev/null +++ b/pkg/types/resource/parse.go @@ -0,0 +1,18 @@ +package resource + +import "strings" + +// SplitFullName looks for the first ` `, everything to the left is first name, +// everything to the right is last name. +func SplitFullName(name string) (string, string) { + names := strings.SplitN(name, " ", 2) + var firstName, lastName string + if len(names) > 0 { + firstName = names[0] + } + if len(names) > 1 { + lastName = names[1] + } + + return firstName, lastName +} diff --git a/pkg/types/resource/parse_test.go b/pkg/types/resource/parse_test.go new file mode 100644 index 00000000..7432c280 --- /dev/null +++ b/pkg/types/resource/parse_test.go @@ -0,0 +1,54 @@ +package resource + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +func TestHelpers_SplitFullName(t *testing.T) { + tests := []struct { + name string + input string + expected [2]string + }{ + { + "Single name", + "Prince", + [2]string{"Prince", ""}, + }, + { + "First and last name", + "John Smith", + [2]string{"John", "Smith"}, + }, + { + "Multiple names", + "John Jacob Jingleheimer Schmidt", + [2]string{"John", "Jacob Jingleheimer Schmidt"}, + }, + { + "Empty string", + "", + [2]string{"", ""}, + }, + { + "Multiple spaces", + "John Smith", + [2]string{"John", " Smith"}, + }, + { + "Starts with Space", + " John Smith", + [2]string{"", "John Smith"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + firstName, lastName := SplitFullName(tt.input) + require.Equal(t, tt.expected[0], firstName) + require.Equal(t, tt.expected[1], lastName) + }) + } +} diff --git a/pkg/types/resource/user_trait.go b/pkg/types/resource/user_trait.go index 7c104801..aefcdc7c 100644 --- a/pkg/types/resource/user_trait.go +++ b/pkg/types/resource/user_trait.go @@ -45,6 +45,7 @@ func WithEmail(email string, primary bool) UserTraitOption { return nil } } + func WithUserLogin(login string, aliases ...string) UserTraitOption { return func(ut *v2.UserTrait) error { if login == "" { diff --git a/pkg/uhttp/contenttype.go b/pkg/uhttp/contenttype.go new file mode 100644 index 00000000..cbad9818 --- /dev/null +++ b/pkg/uhttp/contenttype.go @@ -0,0 +1,25 @@ +package uhttp + +import "strings" + +var xmlContentTypes []string = []string{ + "text/xml", + "application/xml", +} + +func IsJSONContentType(contentType string) bool { + return strings.HasPrefix(contentType, "application") && + strings.Contains(contentType, "json") +} + +func IsXMLContentType(contentType string) bool { + // there are some janky APIs out there + normalizedContentType := strings.TrimSpace(strings.ToLower(contentType)) + + for _, xmlContentType := range xmlContentTypes { + if strings.HasPrefix(normalizedContentType, xmlContentType) { + return true + } + } + return false +} diff --git a/pkg/uhttp/contenttype_test.go b/pkg/uhttp/contenttype_test.go new file mode 100644 index 00000000..9b1f913e --- /dev/null +++ b/pkg/uhttp/contenttype_test.go @@ -0,0 +1,28 @@ +package uhttp + +import ( + "net/http" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestHelpers_IsJSONContentType_Success(t *testing.T) { + resp := &http.Response{ + Header: map[string][]string{ + "Content-Type": {"application/vdn+json"}, + }, + } + h := resp.Header.Get("Content-Type") + require.True(t, IsJSONContentType(h)) +} + +func TestHelpers_IsJSONContentType_Failure(t *testing.T) { + resp := &http.Response{ + Header: map[string][]string{ + "Content-Type": {"application/xml"}, + }, + } + h := resp.Header.Get("Content-Type") + require.False(t, IsJSONContentType(h)) +} diff --git a/pkg/uhttp/wrapper.go b/pkg/uhttp/wrapper.go index 2ae024ba..49a50249 100644 --- a/pkg/uhttp/wrapper.go +++ b/pkg/uhttp/wrapper.go @@ -14,7 +14,7 @@ import ( "strconv" v2 "github.com/conductorone/baton-sdk/pb/c1/connector/v2" - "github.com/conductorone/baton-sdk/pkg/helpers" + "github.com/conductorone/baton-sdk/pkg/ratelimit" "github.com/grpc-ecosystem/go-grpc-middleware/logging/zap/ctxzap" "go.uber.org/zap" "google.golang.org/grpc/codes" @@ -115,7 +115,7 @@ func NewBaseHttpClientWithContext(ctx context.Context, httpClient *http.Client) // status code 204 No Content), then pass a `nil` to `response`. func WithJSONResponse(response interface{}) DoOption { return func(resp *WrapperResponse) error { - if !helpers.IsJSONContentType(resp.Header.Get(ContentType)) { + if !IsJSONContentType(resp.Header.Get(ContentType)) { return fmt.Errorf("unexpected content type for json response: %s", resp.Header.Get(ContentType)) } if response == nil && len(resp.Body) == 0 { @@ -139,7 +139,7 @@ func WithErrorResponse(resource ErrorResponse) DoOption { return nil } - if !helpers.IsJSONContentType(resp.Header.Get(ContentType)) { + if !IsJSONContentType(resp.Header.Get(ContentType)) { return fmt.Errorf("%v", string(resp.Body)) } @@ -157,7 +157,7 @@ func WithErrorResponse(resource ErrorResponse) DoOption { func WithRatelimitData(resource *v2.RateLimitDescription) DoOption { return func(resp *WrapperResponse) error { - rl, err := helpers.ExtractRateLimitData(resp.StatusCode, &resp.Header) + rl, err := ratelimit.ExtractRateLimitData(resp.StatusCode, &resp.Header) if err != nil { return err } @@ -173,7 +173,7 @@ func WithRatelimitData(resource *v2.RateLimitDescription) DoOption { func WithXMLResponse(response interface{}) DoOption { return func(resp *WrapperResponse) error { - if !helpers.IsXMLContentType(resp.Header.Get(ContentType)) { + if !IsXMLContentType(resp.Header.Get(ContentType)) { return fmt.Errorf("unexpected content type for xml response: %s", resp.Header.Get(ContentType)) } if response == nil && len(resp.Body) == 0 { @@ -189,10 +189,10 @@ func WithXMLResponse(response interface{}) DoOption { func WithResponse(response interface{}) DoOption { return func(resp *WrapperResponse) error { - if helpers.IsJSONContentType(resp.Header.Get(ContentType)) { + if IsJSONContentType(resp.Header.Get(ContentType)) { return WithJSONResponse(response)(resp) } - if helpers.IsXMLContentType(resp.Header.Get(ContentType)) { + if IsXMLContentType(resp.Header.Get(ContentType)) { return WithXMLResponse(response)(resp) }