Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 1 addition & 5 deletions .secrets.baseline
Original file line number Diff line number Diff line change
Expand Up @@ -66,10 +66,6 @@
{
"path": "detect_secrets.filters.allowlist.is_line_allowlisted"
},
{
"path": "detect_secrets.filters.common.is_baseline_file",
"filename": ".secrets.baseline"
},
{
"path": "detect_secrets.filters.common.is_ignored_due_to_verification_policies",
"min_level": 2
Expand Down Expand Up @@ -105,5 +101,5 @@
}
],
"results": {},
"generated_at": "2025-06-12T11:22:54Z"
"generated_at": "2025-06-25T17:16:15Z"
}
1 change: 1 addition & 0 deletions changes/20250625181408.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
:bug: Fix response unmarshalling issue when API has unknown fields
62 changes: 44 additions & 18 deletions utils/api/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,29 +9,34 @@ package api
import (
"context"
"fmt"
_http "net/http"
"net/http"
"reflect"
"strings"

"github.com/perimeterx/marshmallow"

"github.com/ARM-software/embedded-development-services-client-utils/utils/errors"
"github.com/ARM-software/golang-utils/utils/commonerrors"
"github.com/ARM-software/golang-utils/utils/parallelisation"
"github.com/ARM-software/golang-utils/utils/reflection"
"github.com/ARM-software/golang-utils/utils/safeio"
)

const requiredFieldError = "no value given for required property"

// IsCallSuccessful determines whether an API response is successful or not
func IsCallSuccessful(r *_http.Response) bool {
func IsCallSuccessful(r *http.Response) bool {
if r == nil {
return false
}
return r.StatusCode >= _http.StatusOK && r.StatusCode < _http.StatusMultipleChoices
return r.StatusCode >= http.StatusOK && r.StatusCode < http.StatusMultipleChoices
}

// CheckAPICallSuccess verifies whether an API response is successful or not and if not, populates an error with all the information needed.
// errorContext corresponds to the description of what led to the error if error there is e.g. `Failed adding a user`.
// resp corresponds to the HTTP response from a certain endpoint. The body of such response is not closed by this function.
// apiErr corresponds to the error which may be returned by the HTTP client when calling the endpoint.
func CheckAPICallSuccess(ctx context.Context, errorContext string, resp *_http.Response, apiErr error) (err error) {
func CheckAPICallSuccess(ctx context.Context, errorContext string, resp *http.Response, apiErr error) (err error) {
err = parallelisation.DetermineContextError(ctx)
if err != nil {
return
Expand Down Expand Up @@ -63,38 +68,63 @@ func CheckAPICallSuccess(ctx context.Context, errorContext string, resp *_http.R
// CallAndCheckSuccess is a wrapper for making an API call and then checking success with `CheckAPICallSuccess`
// errorContext corresponds to the description of what led to the error if error there is e.g. `Failed adding a user`.
// apiCallFunc corresponds to a generic function that will be called to make the API call
func CallAndCheckSuccess[T any](ctx context.Context, errorContext string, apiCallFunc func(ctx context.Context) (*T, *_http.Response, error)) (result *T, err error) {
func CallAndCheckSuccess[T any](ctx context.Context, errorContext string, apiCallFunc func(ctx context.Context) (*T, *http.Response, error)) (result *T, err error) {
if err = parallelisation.DetermineContextError(ctx); err != nil {
return
}

result, resp, apiErr := apiCallFunc(ctx)
if resp != nil && resp.Body != nil {
_ = resp.Body.Close()
defer func() {
if resp != nil && resp.Body != nil {
_ = resp.Body.Close()
}
}()
}

if err = CheckAPICallSuccess(ctx, errorContext, resp, apiErr); err != nil {
err = checkResponse(ctx, apiErr, resp, result, errorContext)
return
}

func checkResponse(ctx context.Context, apiErr error, resp *http.Response, result any, errorContext string) (err error) {
err = CheckAPICallSuccess(ctx, errorContext, resp, apiErr)
if err != nil {
return
}

if apiErr != nil {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if this is going to be its own function then you should check the statuscode too, before it was after something that did that so we knew it would be successful but here we don't and it could cause confusion if someone uses the function elsewhere

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is a private function though

err = commonerrors.WrapError(commonerrors.ErrMarshalling, apiErr, "API call was successful but an error occurred during response marshalling")
return
if commonerrors.CorrespondTo(apiErr, requiredFieldError) {
return
}
if resp == nil || resp.Body == nil {
return
}
// At this point, the marshalling problem may be due to the present of unknown fields in the response due to an API extension.
// See https://github.com/OpenAPITools/openapi-generator/issues/21446
var respB []byte
respB, err = safeio.ReadAll(ctx, resp.Body)
if err != nil {
return
}
_, err = marshmallow.Unmarshal(respB, result, marshmallow.WithSkipPopulateStruct(false), marshmallow.WithExcludeKnownFieldsFromMap(true))
if err != nil {
err = commonerrors.WrapError(commonerrors.ErrMarshalling, err, "API call was successful but an error occurred during response marshalling")
return
}
}

if result != nil && reflection.IsEmpty(result) {
if reflection.IsEmpty(result) {
err = commonerrors.New(commonerrors.ErrMarshalling, "unmarshalled response is empty")
return
}

return
}

// GenericCallAndCheckSuccess is similar to CallAndCheckSuccess but for function returning interfaces rather than concrete types.
// T must be an interface.
// errorContext corresponds to the description of what led to the error if error there is e.g. `Failed adding a user`.
// apiCallFunc corresponds to a generic function that will be called to make the API call
func GenericCallAndCheckSuccess[T any](ctx context.Context, errorContext string, apiCallFunc func(ctx context.Context) (T, *_http.Response, error)) (result T, err error) {
func GenericCallAndCheckSuccess[T any](ctx context.Context, errorContext string, apiCallFunc func(ctx context.Context) (T, *http.Response, error)) (result T, err error) {
if err = parallelisation.DetermineContextError(ctx); err != nil {
return
}
Expand All @@ -104,7 +134,8 @@ func GenericCallAndCheckSuccess[T any](ctx context.Context, errorContext string,
_ = resp.Body.Close()
}

if err = CheckAPICallSuccess(ctx, errorContext, resp, apiErr); err != nil {
err = checkResponse(ctx, apiErr, resp, result, errorContext)
if err != nil {
return
}

Expand All @@ -113,10 +144,5 @@ func GenericCallAndCheckSuccess[T any](ctx context.Context, errorContext string,
return
}

if reflection.IsEmpty(result) {
err = commonerrors.New(commonerrors.ErrMarshalling, "unmarshalled response is empty")
return
}

return
}
76 changes: 70 additions & 6 deletions utils/api/api_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,16 +8,20 @@ package api
import (
"bytes"
"context"
"encoding/json"
"errors"
"io"
_http "net/http"
"testing"

"github.com/go-faker/faker/v4"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"github.com/ARM-software/embedded-development-services-client/client"
"github.com/ARM-software/golang-utils/utils/commonerrors"
"github.com/ARM-software/golang-utils/utils/commonerrors/errortest"
"github.com/ARM-software/golang-utils/utils/field"
)

func TestIsAPICallSuccessful(t *testing.T) {
Expand Down Expand Up @@ -104,6 +108,66 @@ func TestCallAndCheckSuccess(t *testing.T) {
assert.Equal(t, actualErr.Error(), expectedErr)
})

t.Run("api call successful, marshalling failed due to missing required field in response", func(t *testing.T) {
expectedErrorMessage := client.ErrorResponse{
Fields: []client.FieldObject{{
FieldName: faker.Name(),
FieldPath: field.ToOptionalString(faker.Name()),
Message: faker.Sentence(),
}},
HttpStatusCode: 200,
Message: faker.Sentence(),
RequestId: faker.UUIDDigit(),
}
response, err := expectedErrorMessage.ToMap()
require.NoError(t, err)
delete(response, "message")

reducedResponse, err := json.Marshal(response)
require.NoError(t, err)

errorResponse := client.ErrorResponse{}
errM := errorResponse.UnmarshalJSON(reducedResponse)
require.Error(t, errM)

parentCtx := context.Background()
_, err = CallAndCheckSuccess[client.ErrorResponse](parentCtx, "test",
func(ctx context.Context) (*client.ErrorResponse, *_http.Response, error) {
return &errorResponse, &_http.Response{StatusCode: 200, Body: io.NopCloser(bytes.NewReader(reducedResponse))}, errM
})
require.Error(t, err)
errortest.AssertError(t, err, commonerrors.ErrMarshalling)
})

t.Run("api call successful, strict marshalling failed but recovery", func(t *testing.T) {
expectedErrorMessage := client.ErrorResponse{
Fields: []client.FieldObject{{
FieldName: faker.Name(),
FieldPath: field.ToOptionalString(faker.Name()),
Message: faker.Sentence(),
}},
HttpStatusCode: 200,
Message: faker.Sentence(),
RequestId: faker.UUIDDigit(),
}
response, err := expectedErrorMessage.ToMap()
require.NoError(t, err)
response[faker.Word()] = faker.Name()
response[faker.Word()] = faker.Sentence()
response[faker.Word()] = faker.Paragraph()
response[faker.Word()] = faker.UUIDDigit()
extendedResponse, err := json.Marshal(response)
require.NoError(t, err)
errMessage := "no error"
parentCtx := context.Background()
result, err := CallAndCheckSuccess[client.ErrorResponse](parentCtx, errMessage,
func(ctx context.Context) (*client.ErrorResponse, *_http.Response, error) {
return &client.ErrorResponse{}, &_http.Response{StatusCode: 200, Body: io.NopCloser(bytes.NewReader(extendedResponse))}, errors.New(errMessage)
})
require.NoError(t, err)
assert.Equal(t, expectedErrorMessage, *result)
})

t.Run("api call successful, empty response", func(t *testing.T) {
errMessage := "no error"
parentCtx := context.Background()
Expand Down Expand Up @@ -152,7 +216,7 @@ func TestGenericCallAndCheckSuccess(t *testing.T) {
assert.Equal(t, actualErr.Error(), expectedErr)
})

t.Run("no context error, api call successful", func(t *testing.T) {
t.Run("api call successful but error marshalling", func(t *testing.T) {
errMessage := "no error"
parentCtx := context.Background()
_, err := GenericCallAndCheckSuccess(parentCtx, errMessage,
Expand All @@ -164,7 +228,8 @@ func TestGenericCallAndCheckSuccess(t *testing.T) {
}
return &tmp, &_http.Response{StatusCode: 200}, errors.New(errMessage)
})
assert.NoError(t, err)
require.Error(t, err)
errortest.AssertError(t, err, commonerrors.ErrMarshalling)
})

t.Run("api call successful, empty response", func(t *testing.T) {
Expand All @@ -178,11 +243,10 @@ func TestGenericCallAndCheckSuccess(t *testing.T) {
})

t.Run("api call successful, incorrect response", func(t *testing.T) {
errMessage := "response error"
parentCtx := context.Background()
_, err := GenericCallAndCheckSuccess(parentCtx, errMessage,
func(ctx context.Context) (struct{}, *_http.Response, error) {
return struct{}{}, &_http.Response{StatusCode: 200}, errors.New(errMessage)
_, err := GenericCallAndCheckSuccess(parentCtx, "response error",
func(ctx context.Context) (struct{ Blah string }, *_http.Response, error) {
return struct{ Blah string }{Blah: "fsadsfs"}, &_http.Response{StatusCode: 200}, nil
})
errortest.AssertError(t, err, commonerrors.ErrConflict)
})
Expand Down
3 changes: 3 additions & 0 deletions utils/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ require (
github.com/ARM-software/golang-utils/utils v1.101.0
github.com/go-faker/faker/v4 v4.6.1
github.com/go-logr/logr v1.4.3
github.com/perimeterx/marshmallow v1.1.5
github.com/stretchr/testify v1.10.0
go.uber.org/atomic v1.11.0
go.uber.org/goleak v1.3.0
Expand Down Expand Up @@ -42,7 +43,9 @@ require (
github.com/hashicorp/go-retryablehttp v0.7.7 // indirect
github.com/iamacarpet/go-win64api v0.0.0-20240507095429-873e84e85847 // indirect
github.com/joho/godotenv v1.5.1 // indirect
github.com/josharian/intern v1.0.0 // indirect
github.com/lufia/plan9stats v0.0.0-20240909124753-873cd0166683 // indirect
github.com/mailru/easyjson v0.7.7 // indirect
github.com/mattn/go-colorable v0.1.13 // indirect
github.com/mattn/go-isatty v0.0.20 // indirect
github.com/mitchellh/go-homedir v1.1.0 // indirect
Expand Down
10 changes: 10 additions & 0 deletions utils/go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,8 @@ github.com/go-ole/go-ole v1.3.0 h1:Dt6ye7+vXGIKZ7Xtk4s6/xVdGDQynvom7xCFEdWr6uE=
github.com/go-ole/go-ole v1.3.0/go.mod h1:5LS6F96DhAwUc7C+1HLexzMXY1xGRSryjyPPKW6zv78=
github.com/go-ozzo/ozzo-validation/v4 v4.3.0 h1:byhDUpfEwjsVQb1vBunvIjh2BHQ9ead57VkAEY4V+Es=
github.com/go-ozzo/ozzo-validation/v4 v4.3.0/go.mod h1:2NKgrcHl3z6cJs+3Oo940FPRiTzuqKbvfrL2RxCj6Ew=
github.com/go-test/deep v1.0.8 h1:TDsG77qcSprGbC6vTN8OuXp5g+J+b5Pcguhf7Zt61VM=
github.com/go-test/deep v1.0.8/go.mod h1:5C2ZWiW0ErCdrYzpqxLbTX7MG14M9iiw8DgHncVwcsE=
github.com/go-viper/mapstructure/v2 v2.2.1 h1:ZAaOCxANMuZx5RCeg0mBdEZk7DZasvvZIxtHqx8aGss=
github.com/go-viper/mapstructure/v2 v2.2.1/go.mod h1:oJDH3BJKyqBA2TXFhDsKDGDTlndYOZ6rGS0BRZIxGhM=
github.com/godbus/dbus v4.1.0+incompatible/go.mod h1:/YcGZj5zSblfDWMMoOzV4fas9FZnQYTkDnsGvmh2Grw=
Expand Down Expand Up @@ -109,12 +111,16 @@ github.com/iamacarpet/go-win64api v0.0.0-20240507095429-873e84e85847 h1:cRHZFGwI
github.com/iamacarpet/go-win64api v0.0.0-20240507095429-873e84e85847/go.mod h1:B7zFQPAznj+ujXel5X+LUoK3LgY6VboCdVYHZNn7gpg=
github.com/joho/godotenv v1.5.1 h1:7eLL/+HRGLY0ldzfGMeQkb7vMd0as4CfYvUVzLqw0N0=
github.com/joho/godotenv v1.5.1/go.mod h1:f4LDr5Voq0i2e/R5DDNOoa2zzDfwtkZa6DnEwAbqwq4=
github.com/josharian/intern v1.0.0 h1:vlS4z54oSdjm0bgjRigI+G1HpF+tI+9rE5LLzOg8HmY=
github.com/josharian/intern v1.0.0/go.mod h1:5DoeVV0s6jJacbCEi61lwdGj/aVlrQvzHFFd8Hwg//Y=
github.com/kr/pretty v0.3.1 h1:flRD4NNwYAUpkphVc1HcthR4KEIFJ65n8Mw5qdRn3LE=
github.com/kr/pretty v0.3.1/go.mod h1:hoEshYVHaxMs3cyo3Yncou5ZscifuDolrwPKZanG3xk=
github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY=
github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE=
github.com/lufia/plan9stats v0.0.0-20240909124753-873cd0166683 h1:7UMa6KCCMjZEMDtTVdcGu0B1GmmC7QJKiCCjyTAWQy0=
github.com/lufia/plan9stats v0.0.0-20240909124753-873cd0166683/go.mod h1:ilwx/Dta8jXAgpFYFvSWEMwxmbWXyiUHkd5FwyKhb5k=
github.com/mailru/easyjson v0.7.7 h1:UGYAvKxe3sBsEDzO8ZeWOSlIQfWFlxbzLZe7hwFURr0=
github.com/mailru/easyjson v0.7.7/go.mod h1:xzfreul335JAWq5oZzymOObrkdz5UnU4kGfJJLY9Nlc=
github.com/mattn/go-colorable v0.1.9/go.mod h1:u6P/XSegPjTcexA+o6vUJrdnUu04hMope9wVRipJSqc=
github.com/mattn/go-colorable v0.1.12/go.mod h1:u5H1YNBxpqRaxsYJYSkiCWKzEfiAb1Gb520KVy5xxl4=
github.com/mattn/go-colorable v0.1.13 h1:fFA4WZxdEF4tXPZVKMLwD8oUnCTTo08duU7wxecdEvA=
Expand All @@ -136,6 +142,8 @@ github.com/onsi/gomega v1.7.1/go.mod h1:XdKZgCCFLUoM/7CFJVPcG8C1xQ1AJ0vpAezJrB7J
github.com/onsi/gomega v1.10.2/go.mod h1:iN09h71vgCQne3DLsj+A5owkum+a2tYe+TOCB1ybHNo=
github.com/pelletier/go-toml/v2 v2.2.3 h1:YmeHyLY8mFWbdkNWwpr+qIL2bEqT0o95WSdkNHvL12M=
github.com/pelletier/go-toml/v2 v2.2.3/go.mod h1:MfCQTFTvCcUyyvvwm1+G6H/jORL20Xlb6rzQu9GuUkc=
github.com/perimeterx/marshmallow v1.1.5 h1:a2LALqQ1BlHM8PZblsDdidgv1mWi1DgC2UmX50IvK2s=
github.com/perimeterx/marshmallow v1.1.5/go.mod h1:dsXbUu8CRzfYP5a87xpp0xq9S3u0Vchtcl8we9tYaXw=
github.com/petermattis/goid v0.0.0-20240813172612-4fcff4a6cae7/go.mod h1:pxMtw7cyUw6B2bRH0ZBANSPg+AoSud1I1iyJHI69jH4=
github.com/petermattis/goid v0.0.0-20250319124200-ccd6737f222a h1:S+AGcmAESQ0pXCUNnRH7V+bOUIgkSX5qVt2cNKCrm0Q=
github.com/petermattis/goid v0.0.0-20250319124200-ccd6737f222a/go.mod h1:pxMtw7cyUw6B2bRH0ZBANSPg+AoSud1I1iyJHI69jH4=
Expand Down Expand Up @@ -190,6 +198,8 @@ github.com/tklauser/go-sysconf v0.3.14 h1:g5vzr9iPFFz24v2KZXs/pvpvh8/V9Fw6vQK5ZZ
github.com/tklauser/go-sysconf v0.3.14/go.mod h1:1ym4lWMLUOhuBOPGtRcJm7tEGX4SCYNEEEtghGG/8uY=
github.com/tklauser/numcpus v0.9.0 h1:lmyCHtANi8aRUgkckBgoDk1nHCux3n2cgkJLXdQGPDo=
github.com/tklauser/numcpus v0.9.0/go.mod h1:SN6Nq1O3VychhC1npsWostA+oW+VOQTxZrS604NSRyI=
github.com/ugorji/go/codec v1.2.7 h1:YPXUKf7fYbp/y8xloBqZOw2qaVggbfwMlI8WM3wZUJ0=
github.com/ugorji/go/codec v1.2.7/go.mod h1:WGN1fab3R1fzQlVQTkfxVtIBhWDRqOviHU95kRgeqEY=
github.com/yusufpapurcu/wmi v1.2.4 h1:zFUKzehAFReQwLys1b/iSMl+JQGSCSjtVqQn9bBrPo0=
github.com/yusufpapurcu/wmi v1.2.4/go.mod h1:SBZ9tNy3G9/m5Oi98Zks0QjeHVDvuK0qfxQmPyzfmi0=
github.com/zailic/slogr v0.0.2-alpha h1:ZZ+96+AOnk4L9JoPkZ6aGbGXnn90/53A9zm9JcjYSYc=
Expand Down
Loading