diff --git a/.golangci.yaml b/.golangci.yaml index c86b20d5f..d40f1217b 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -1,6 +1,7 @@ -issues: - exclude-rules: - # Exclude some staticcheck messages - - linters: - - staticcheck - text: "SA1008:" +linters-settings: + staticcheck: + checks: + - all + # Exclude some staticcheck messages + - '-SA1008' # "content-length" is not canonical, avoid OBS headers warnings + - '-SA1019' # deprecations, used to avoid Extract... deprecation warnings diff --git a/internal/extract/doc.go b/internal/extract/doc.go new file mode 100644 index 000000000..c5f020bfa --- /dev/null +++ b/internal/extract/doc.go @@ -0,0 +1,3 @@ +// Package extract contains functions for extracting JSON results into given structure or slice pointers. +// Those are wrappers over `json.Marshall` and `json.Unmarshall` functions with additional validation built it +package extract diff --git a/internal/extract/json.go b/internal/extract/json.go new file mode 100644 index 000000000..db822f46f --- /dev/null +++ b/internal/extract/json.go @@ -0,0 +1,153 @@ +package extract + +import ( + "bytes" + "encoding/json" + "fmt" + "io" + "reflect" +) + +func intoPtr(body, to interface{}, label string) error { + if label == "" { + return Into(body, &to) + } + + var m map[string]interface{} + err := Into(body, &m) + if err != nil { + return err + } + + b, err := JsonMarshal(m[label]) + if err != nil { + return err + } + + toValue := reflect.ValueOf(to) + if toValue.Kind() == reflect.Ptr { + toValue = toValue.Elem() + } + + switch toValue.Kind() { + case reflect.Slice: + typeOfV := toValue.Type().Elem() + if typeOfV.Kind() == reflect.Struct { + if typeOfV.NumField() > 0 && typeOfV.Field(0).Anonymous { + newSlice := reflect.MakeSlice(reflect.SliceOf(typeOfV), 0, 0) + + for _, v := range m[label].([]interface{}) { + // For each iteration of the slice, we create a new struct. + // This is to work around a bug where elements of a slice + // are reused and not overwritten when the same copy of the + // struct is used: + // + // https://github.com/golang/go/issues/21092 + // https://github.com/golang/go/issues/24155 + // https://play.golang.org/p/NHo3ywlPZli + newType := reflect.New(typeOfV).Elem() + + b, err := JsonMarshal(v) + if err != nil { + return err + } + + // This is needed for structs with an UnmarshalJSON method. + // Technically this is just unmarshalling the response into + // a struct that is never used, but it's good enough to + // trigger the UnmarshalJSON method. + for i := 0; i < newType.NumField(); i++ { + s := newType.Field(i).Addr().Interface() + + // Unmarshal is used rather than NewDecoder to also work + // around the above-mentioned bug. + err = json.Unmarshal(b, s) + if err != nil { + continue + } + } + + newSlice = reflect.Append(newSlice, newType) + } + + // "to" should now be properly modeled to receive the + // JSON response body and unmarshal into all the correct + // fields of the struct or composed extension struct + // at the end of this method. + toValue.Set(newSlice) + } + } + case reflect.Struct: + typeOfV := toValue.Type() + if typeOfV.NumField() > 0 && typeOfV.Field(0).Anonymous { + for i := 0; i < toValue.NumField(); i++ { + toField := toValue.Field(i) + if toField.Kind() == reflect.Struct { + s := toField.Addr().Interface() + err = json.NewDecoder(bytes.NewReader(b)).Decode(s) + if err != nil { + return err + } + } + } + } + } + + err = json.Unmarshal(b, &to) + return err +} + +func JsonMarshal(t interface{}) ([]byte, error) { + buffer := &bytes.Buffer{} + enc := json.NewEncoder(buffer) + enc.SetEscapeHTML(false) + err := enc.Encode(t) + return buffer.Bytes(), err +} + +func Into(body interface{}, to interface{}) error { + if reader, ok := body.(io.Reader); ok { + if readCloser, ok := reader.(io.Closer); ok { + defer readCloser.Close() + } + return json.NewDecoder(reader).Decode(to) + } + + b, err := JsonMarshal(body) + if err != nil { + return err + } + err = json.Unmarshal(b, to) + + return err +} + +// IntoStructPtr will unmarshal the given body into the provided +// interface{} (to). +func IntoStructPtr(body, to interface{}, label string) error { + t := reflect.TypeOf(to) + if k := t.Kind(); k != reflect.Ptr { + return fmt.Errorf("expected pointer, got %v", k) + } + switch t.Elem().Kind() { + case reflect.Struct: + return intoPtr(body, to, label) + default: + return fmt.Errorf("expected pointer to struct, got: %v", t) + } +} + +// IntoSlicePtr will unmarshal the provided body into the provided +// interface{} (to). +func IntoSlicePtr(body, to interface{}, label string) error { + t := reflect.TypeOf(to) + if k := t.Kind(); k != reflect.Ptr { + return fmt.Errorf("expected pointer, got %v", k) + } + switch t.Elem().Kind() { + case reflect.Slice: + return intoPtr(body, to, label) + default: + return fmt.Errorf("expected pointer to slice, got: %v", t) + } +} diff --git a/internal/extract/json_test.go b/internal/extract/json_test.go new file mode 100644 index 000000000..04c933b0b --- /dev/null +++ b/internal/extract/json_test.go @@ -0,0 +1,274 @@ +package extract + +import ( + "bytes" + "fmt" + "io" + "math/rand" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// randomString duplicates here to avoid cyclic imports +// TODO: this function should be moved to some other package later +func randomString(prefix string, n int) string { + const alphanum = "0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz" + var bytes = make([]byte, n) + _, _ = rand.Read(bytes) + for i, b := range bytes { + bytes[i] = alphanum[b%byte(len(alphanum))] + } + return prefix + string(bytes) +} + +func TestInto(t *testing.T) { + key := "data_key" + value := randomString("v-", 20) + + expected := map[string]string{key: value} + + cases := map[string]interface{}{ + "map": map[string]string{key: value}, + "struct": struct { + DataKey string `json:"data_key"` + }{value}, + "struct with other fields": struct { + DataKey string `json:"data_key"` + DataKey2 string `json:"-"` + }{value, "difgljdfgn"}, + "io.Reader": bytes.NewReader([]byte(fmt.Sprintf(`{ "data_key": "%s"}`, value))), + } + + for name, source := range cases { + source := source // avoid issues with parallel tests + expectedValue := expected[key] + + t.Run(name, func(t *testing.T) { + t.Parallel() + + actual := make(map[string]string) + err := Into(source, &actual) + + assert.NoError(t, err) // not exiting after one fail + assert.EqualValues(t, expectedValue, actual[key]) + }) + } +} + +type TestDataType struct { + DataKey string `json:"data_key"` +} + +type TestDataType2 struct { + TestDataType + + SecondDataField string `json:"second_data_field"` +} + +func readerFromString(src string) io.Reader { + return bytes.NewReader([]byte(src)) +} + +func TestIntoStructPtr(t *testing.T) { + t.Run("ok", func(t *testing.T) { + t.Parallel() + + actual := new(TestDataType) + value := randomString("v-", 20) + + data := fmt.Sprintf(` + { + "data_key": "%s" + } + `, value) + + err := IntoStructPtr(readerFromString(data), actual, "") + require.NoError(t, err) + require.Equal(t, value, actual.DataKey) + }) + + t.Run("with label", func(t *testing.T) { + t.Parallel() + + actual := new(TestDataType) + value := randomString("v-", 20) + + data := fmt.Sprintf(` + { + "internal": { + "data_key": "%s" + } + } + `, value) + + err := IntoStructPtr(readerFromString(data), actual, "internal") + require.NoError(t, err) + require.Equal(t, value, actual.DataKey) + }) + + t.Run("with label and embed", func(t *testing.T) { + t.Parallel() + + actual := new(TestDataType2) + value := randomString("v-", 20) + + data := fmt.Sprintf(` + { + "internal": { + "data_key": "%s", + "second_data_field": "%[1]s-2" + } + } + `, value) + + err := IntoStructPtr(readerFromString(data), actual, "internal") + require.NoError(t, err) + require.Equal(t, value, actual.DataKey) + require.Equal(t, value+"-2", actual.SecondDataField) + }) + + t.Run("with label (err)", func(t *testing.T) { + t.Parallel() + + actual := new(TestDataType) + value := randomString("v-", 20) + + data := fmt.Sprintf(` + { + "internal": { + "data_key": "%s" + } + } + `, value) + + err := IntoStructPtr(readerFromString(data), actual, "") + require.NoError(t, err) + require.Equal(t, "", actual.DataKey) + }) + + t.Run("non pointer", func(t *testing.T) { + t.Parallel() + + actual := TestDataType{} + value := randomString("v-", 20) + + data := fmt.Sprintf(` + { + "data_key": "%s" + } + `, value) + + err := IntoStructPtr(readerFromString(data), actual, "") + require.EqualError(t, err, "expected pointer, got struct") + }) + + t.Run("non struct", func(t *testing.T) { + t.Parallel() + + actual := make(map[string]interface{}) + value := randomString("v-", 20) + + data := fmt.Sprintf(` + { + "data_key": "%s" + } + `, value) + + err := IntoStructPtr(readerFromString(data), &actual, "") + require.EqualError(t, err, "expected pointer to struct, got: *map[string]interface {}") + }) +} + +func TestIntoSlicePtr(t *testing.T) { + t.Run("ok", func(t *testing.T) { + t.Parallel() + + actual := make([]TestDataType, 0) + value := randomString("v-", 20) + + data := fmt.Sprintf(` + [{ + "data_key": "%s" + }] + `, value) + + err := IntoSlicePtr(readerFromString(data), &actual, "") + require.NoError(t, err) + require.Len(t, actual, 1) + require.Equal(t, value, actual[0].DataKey) + }) + + t.Run("with label", func(t *testing.T) { + t.Parallel() + + actual := make([]TestDataType, 0) + value := randomString("v-", 20) + + data := fmt.Sprintf(` + { + "data": [{ "data_key": "%s" }] + } + `, value) + + err := IntoSlicePtr(readerFromString(data), &actual, "data") + require.NoError(t, err) + require.Len(t, actual, 1) + require.Equal(t, value, actual[0].DataKey) + }) + + t.Run("with label and embed", func(t *testing.T) { + t.Parallel() + + actual := make([]TestDataType2, 0) + value := randomString("v-", 20) + + data := fmt.Sprintf(` + { + "internal": [{ + "data_key": "%s", + "second_data_field": "%[1]s" + }] + } + `, value) + + err := IntoSlicePtr(readerFromString(data), &actual, "internal") + require.NoError(t, err) + require.Len(t, actual, 1) + require.Equal(t, value, actual[0].DataKey) + require.Equal(t, value, actual[0].SecondDataField) + }) + + t.Run("not pointer", func(t *testing.T) { + t.Parallel() + + actual := make([]TestDataType, 0) + value := randomString("v-", 20) + + data := fmt.Sprintf(` + [{ + "data_key": "%s" + }] + `, value) + + err := IntoSlicePtr(readerFromString(data), actual, "") + require.EqualError(t, err, "expected pointer, got slice") + }) + + t.Run("not slice", func(t *testing.T) { + t.Parallel() + + actual := new(TestDataType) + value := randomString("v-", 20) + + data := fmt.Sprintf(` + { + "data_key": "%s" + } + `, value) + + err := IntoSlicePtr(readerFromString(data), actual, "") + require.EqualError(t, err, "expected pointer to slice, got: *extract.TestDataType") + }) +} diff --git a/provider_client.go b/provider_client.go index cc98f3e8d..74938d92a 100644 --- a/provider_client.go +++ b/provider_client.go @@ -9,6 +9,8 @@ import ( "strings" "sync" "time" + + "github.com/opentelekomcloud/gophertelekomcloud/internal/extract" ) // DefaultUserAgent is the default User-Agent string set in the request header. @@ -174,14 +176,6 @@ type RequestOpts struct { var applicationJSON = "application/json" -func jsonMarshal(t interface{}) ([]byte, error) { - buffer := &bytes.Buffer{} - enc := json.NewEncoder(buffer) - enc.SetEscapeHTML(false) - err := enc.Encode(t) - return buffer.Bytes(), err -} - // Request performs an HTTP request using the ProviderClient's current HTTPClient. An authentication // header will automatically be provided. func (client *ProviderClient) Request(method, url string, options *RequestOpts) (*http.Response, error) { @@ -195,7 +189,7 @@ func (client *ProviderClient) Request(method, url string, options *RequestOpts) panic("Please provide only one of JSONBody or RawBody to golangsdk.Request().") } - rendered, err := jsonMarshal(options.JSONBody) + rendered, err := extract.JsonMarshal(options.JSONBody) if err != nil { return nil, err } diff --git a/results.go b/results.go index 7433d4c4c..e0c8fcb4d 100644 --- a/results.go +++ b/results.go @@ -4,11 +4,11 @@ import ( "bytes" "encoding/json" "fmt" - "io" "net/http" - "reflect" "strconv" "time" + + "github.com/opentelekomcloud/gophertelekomcloud/internal/extract" ) /* @@ -24,6 +24,8 @@ Generally, each Result type will have an Extract method that can be used to further interpret the result's payload in a specific context. Extensions or providers can then provide additional extraction functions to pull out provider- or extension-specific information as well. + +Deprecated: use functions from internal/extract package instead */ type Result struct { // Body is the payload of the HTTP response from the server. In most cases, @@ -52,114 +54,14 @@ type JsonRDSInstanceField struct { // ExtractInto allows users to provide an object into which `Extract` will extract // the `Result.Body`. This would be useful for OpenStack providers that have // different fields in the response object than OpenStack proper. +// +// Deprecated: use extract.Into function instead func (r Result) ExtractInto(to interface{}) error { if r.Err != nil { return r.Err } - if reader, ok := r.Body.(io.Reader); ok { - if readCloser, ok := reader.(io.Closer); ok { - defer readCloser.Close() - } - return json.NewDecoder(reader).Decode(to) - } - - b, err := jsonMarshal(r.Body) - if err != nil { - return err - } - err = json.Unmarshal(b, to) - - return err -} - -func (r Result) extractIntoPtr(to interface{}, label string) error { - if label == "" { - return r.ExtractInto(&to) - } - - var m map[string]interface{} - err := r.ExtractInto(&m) - if err != nil { - return err - } - - b, err := jsonMarshal(m[label]) - if err != nil { - return err - } - - toValue := reflect.ValueOf(to) - if toValue.Kind() == reflect.Ptr { - toValue = toValue.Elem() - } - - switch toValue.Kind() { - case reflect.Slice: - typeOfV := toValue.Type().Elem() - if typeOfV.Kind() == reflect.Struct { - if typeOfV.NumField() > 0 && typeOfV.Field(0).Anonymous { - newSlice := reflect.MakeSlice(reflect.SliceOf(typeOfV), 0, 0) - - for _, v := range m[label].([]interface{}) { - // For each iteration of the slice, we create a new struct. - // This is to work around a bug where elements of a slice - // are reused and not overwritten when the same copy of the - // struct is used: - // - // https://github.com/golang/go/issues/21092 - // https://github.com/golang/go/issues/24155 - // https://play.golang.org/p/NHo3ywlPZli - newType := reflect.New(typeOfV).Elem() - - b, err := jsonMarshal(v) - if err != nil { - return err - } - - // This is needed for structs with an UnmarshalJSON method. - // Technically this is just unmarshalling the response into - // a struct that is never used, but it's good enough to - // trigger the UnmarshalJSON method. - for i := 0; i < newType.NumField(); i++ { - s := newType.Field(i).Addr().Interface() - - // Unmarshal is used rather than NewDecoder to also work - // around the above-mentioned bug. - err = json.Unmarshal(b, s) - if err != nil { - return err - } - } - - newSlice = reflect.Append(newSlice, newType) - } - - // "to" should now be properly modeled to receive the - // JSON response body and unmarshal into all the correct - // fields of the struct or composed extension struct - // at the end of this method. - toValue.Set(newSlice) - } - } - case reflect.Struct: - typeOfV := toValue.Type() - if typeOfV.NumField() > 0 && typeOfV.Field(0).Anonymous { - for i := 0; i < toValue.NumField(); i++ { - toField := toValue.Field(i) - if toField.Kind() == reflect.Struct { - s := toField.Addr().Interface() - err = json.NewDecoder(bytes.NewReader(b)).Decode(s) - if err != nil { - return err - } - } - } - } - } - - err = json.Unmarshal(b, &to) - return err + return extract.Into(r.Body, to) } // ExtractIntoStructPtr will unmarshal the Result (r) into the provided @@ -171,21 +73,14 @@ func (r Result) extractIntoPtr(to interface{}, label string) error { // // If provided, `label` will be filtered out of the response // body prior to `r` being unmarshalled into `to`. +// +// Deprecated: use extract.IntoStructPtr function instead func (r Result) ExtractIntoStructPtr(to interface{}, label string) error { if r.Err != nil { return r.Err } - t := reflect.TypeOf(to) - if k := t.Kind(); k != reflect.Ptr { - return fmt.Errorf("Expected pointer, got %v", k) - } - switch t.Elem().Kind() { - case reflect.Struct: - return r.extractIntoPtr(to, label) - default: - return fmt.Errorf("Expected pointer to struct, got: %v", t) - } + return extract.IntoStructPtr(r.Body, to, label) } // ExtractIntoSlicePtr will unmarshal the Result (r) into the provided @@ -197,21 +92,22 @@ func (r Result) ExtractIntoStructPtr(to interface{}, label string) error { // // If provided, `label` will be filtered out of the response // body prior to `r` being unmarshalled into `to`. +// +// Deprecated: use extract.IntoSlicePtr function instead func (r Result) ExtractIntoSlicePtr(to interface{}, label string) error { if r.Err != nil { return r.Err } - t := reflect.TypeOf(to) - if k := t.Kind(); k != reflect.Ptr { - return fmt.Errorf("Expected pointer, got %v", k) - } - switch t.Elem().Kind() { - case reflect.Slice: - return r.extractIntoPtr(to, label) - default: - return fmt.Errorf("Expected pointer to slice, got: %v", t) + return extract.IntoSlicePtr(r.Body, to, label) +} + +func PrettyPrintJSON(body interface{}) string { + pretty, err := json.MarshalIndent(body, "", " ") + if err != nil { + panic(err.Error()) } + return string(pretty) } // PrettyPrintJSON creates a string containing the full response body as @@ -219,11 +115,7 @@ func (r Result) ExtractIntoSlicePtr(to interface{}, label string) error { // debugging extraction bugs. If you include its output in an issue related to // a buggy extraction function, we will all love you forever. func (r Result) PrettyPrintJSON() string { - pretty, err := json.MarshalIndent(r.Body, "", " ") - if err != nil { - panic(err.Error()) - } - return string(pretty) + return PrettyPrintJSON(r.Body) } // ErrResult is an internal type to be used by individual resource packages, but @@ -235,6 +127,8 @@ func (r Result) PrettyPrintJSON() string { // will be nil; otherwise it will be stocked with a relevant error. Use the // ExtractErr method // to cleanly pull it out. +// +// Deprecated: use plain err return instead type ErrResult struct { Result } @@ -296,7 +190,7 @@ func (r HeaderResult) ExtractInto(to interface{}) error { } } - b, err := jsonMarshal(tmpHeaderMap) + b, err := extract.JsonMarshal(tmpHeaderMap) if err != nil { return err }