bitmovin / bitmovin-api-sdk-go

Go API SDK which enables you to seamlessly integrate the Bitmovin API into your projects
MIT License
4 stars 5 forks source link

sdk calls internal POST with incorrect signature in a number of cases #1

Closed zsiec closed 5 years ago

zsiec commented 5 years ago

I noticed this when using this SDK to cancel a job, the call to:

 api.Encoding.Encodings.Stop(id)

returned the error:

there are still some placeholders left in the URL. Please make sure to pass the correct values to this function to replace all placeholders. url=/encoding/encodings/{encoding_id}/stop

After some debugging, I noticed .Stop() calls ApiClient.Post() with the following params:

https://github.com/bitmovin/bitmovin-api-sdk-go/blob/5a601b88994c6ca30e001cb758e45862d05a19be/encoding/encoding_encodings_api.go#L155-L157

while the signature for .Post() is:

https://github.com/bitmovin/bitmovin-api-sdk-go/blob/5a601b88994c6ca30e001cb758e45862d05a19be/common/client.go#L176-L178

this raises no type error, of course, as the second and third parameters are of type interface{}, but the function is incorrectly sending the reqParams variadic into the responseModel param and the response model into the requestModel.

A quick search points at other places (not exhaustive) that may be similarly effected:

https://github.com/bitmovin/bitmovin-api-sdk-go/blob/5a601b88994c6ca30e001cb758e45862d05a19be/encoding/encoding_encodings_live_api.go#L53-L55

https://github.com/bitmovin/bitmovin-api-sdk-go/blob/5a601b88994c6ca30e001cb758e45862d05a19be/notifications/notifications_api.go#L94-L96

https://github.com/bitmovin/bitmovin-api-sdk-go/blob/5a601b88994c6ca30e001cb758e45862d05a19be/encoding/encoding_manifests_hls_api.go#L90-L92

This issue is quickly reproducible with the following test:

func TestStopError(t *testing.T) {
    api, err := bitmovin.NewBitmovinApi(func(apiClient *common.ApiClient) {
        apiClient.ApiKey = "123"
        apiClient.BaseUrl = "https://api.bitmovin.com/v1/"
    })
    if err != nil {
        t.Fatal(err)
    }

    _, err = api.Encoding.Encodings.Stop("some_id")
    if err != nil {
        t.Fatal(err)
    }
}

which returns:

=== RUN   TestStopError
--- FAIL: TestStopError (0.00s)
    stop_test.go:20: there are still some placeholders left in the URL. Please make sure to pass the correct values to this function to replace all placeholders. url=/encoding/encodings/{encoding_id}/stop
FAIL
zsiec commented 5 years ago

I was able to unblock myself by forking and using this workaround:

ack -l "(api.apiClient.Post\(\".*\",\s)(\&[a-zA-Z0-9_-]*,\s*\sreqParams)" | xargs perl -pi -E 's/(api.apiClient.Post\(\".*\",\s)(\&[a-zA-Z0-9_-]*,\s*\sreqParams)/$1struct{}{}, $2/g'

done here: https://github.com/cbsinteractive/bitmovin-api-sdk-go/commit/ee02befeafb5f306c15c197978bc73d17e531270

This is only a workaround and may not be the desired fix.

jonathanhperry commented 5 years ago

@zsiec, this should be fixed now with the latest commit. I ran your single Stop test case and it worked. This should also address other API calls where it's a POST call that does not require a specific body payload.

zsiec commented 5 years ago

thanks for the update @jonathanhperry!