code-health: allow to use Call17 as default Call

An incompatible change was introduced in Tarantool 1.7 which was
released in 2016. This change is about a new binary protocol
command for CALL, which no more restricts a function to returning
an array of tuples and allows returning an arbitrary MsgPack/JSON
result, including scalars, nil and void (nothing).

We should be in-line with tarantool here and provide `Call17`
as just `Call`. For backward compatibility in current `go-tarantool`
version build tag `go_tarantool_call_17` have been defined.
This tag used to change the default `Call` behavior
from `Call16` to `Call17`.

Closes #125

Author: AnaNek

.github/workflows/testing.yml

@@ -60,6 +60,9 @@ jobs:
       - name: Run tests
         run: make test
 
+      - name: Run tests with call_17
+        run: make test TAGS="go_tarantool_call_17"
+
       - name: Run tests, collect code coverage data and send to Coveralls
         if: ${{ matrix.coveralls }}
         env:
@@ -134,6 +137,13 @@ jobs:
         env:
           TEST_TNT_SSL: ${{matrix.ssl}}
 
+      - name: Run tests with call_17
+        run: |
+          source tarantool-enterprise/env.sh
+          make test TAGS="go_tarantool_call_17"
+        env:
+          TEST_TNT_SSL: ${{matrix.ssl}}
+
       - name: Run tests, collect code coverage data and send to Coveralls
         if: ${{ matrix.coveralls }}
         env:

CHANGELOG.md

@@ -15,6 +15,9 @@ Versioning](http://semver.org/spec/v2.0.0.html) except to the first release.
 
 ### Changed
 
+- Add `Call16` method, support build tag `go_tarantool_call_17`
+  to choose behavior for `Call` method (#125)
+
 ### Fixed
 
 ## [1.6.0] - 2022-06-01

Makefile

@@ -13,6 +13,7 @@ BENCH_REFERENCE_REPO := ${BENCH_PATH}/go-tarantool
 BENCH_OPTIONS := -bench=. -run=^Benchmark -benchmem -benchtime=${DURATION} -count=${COUNT}
 GO_TARANTOOL_URL := https://github.com/tarantool/go-tarantool
 GO_TARANTOOL_DIR := ${PROJECT_DIR}/${BENCH_PATH}/go-tarantool
+TAGS :=
 
 .PHONY: clean
 clean:
@@ -33,7 +34,7 @@ golangci-lint:
 
 .PHONY: test
 test:
-	go test ./... -v -p 1
+	go test -tags "$(TAGS)" ./... -v -p 1
 
 .PHONY: testdata
 testdata:
@@ -43,38 +44,38 @@ testdata:
 test-connection-pool:
 	@echo "Running tests in connection_pool package"
 	go clean -testcache
-	go test ./connection_pool/ -v -p 1
+	go test -tags "$(TAGS)" ./connection_pool/ -v -p 1
 
 .PHONY: test-multi
 test-multi:
 	@echo "Running tests in multiconnection package"
 	go clean -testcache
-	go test ./multi/ -v -p 1
+	go test -tags "$(TAGS)" ./multi/ -v -p 1
 
 .PHONY: test-queue
 test-queue:
 	@echo "Running tests in queue package"
 	cd ./queue/ && tarantool -e "require('queue')"
 	go clean -testcache
-	go test ./queue/ -v -p 1
+	go test -tags "$(TAGS)" ./queue/ -v -p 1
 
 .PHONY: test-uuid
 test-uuid:
 	@echo "Running tests in UUID package"
 	go clean -testcache
-	go test ./uuid/ -v -p 1
+	go test -tags "$(TAGS)" ./uuid/ -v -p 1
 
 .PHONY: test-main
 test-main:
 	@echo "Running tests in main package"
 	go clean -testcache
-	go test . -v -p 1
+	go test -tags "$(TAGS)" . -v -p 1
 
 .PHONY: coverage
 coverage:
 	go clean -testcache
 	go get golang.org/x/tools/cmd/cover
-	go test ./... -v -p 1 -covermode=atomic -coverprofile=$(COVERAGE_FILE) -coverpkg=./...
+	go test -tags "$(TAGS)" ./... -v -p 1 -covermode=atomic -coverprofile=$(COVERAGE_FILE) -coverpkg=./...
 	go tool cover -func=$(COVERAGE_FILE)
 
 .PHONY: coveralls
@@ -94,7 +95,7 @@ ${BENCH_PATH} bench-deps:
 .PHONY: bench
 ${BENCH_FILE} bench: ${BENCH_PATH}
 	@echo "Running benchmark tests from the current branch"
-	go test ${TEST_PATH} ${BENCH_OPTIONS} 2>&1 \
+	go test -tags "$(TAGS)" ${TEST_PATH} ${BENCH_OPTIONS} 2>&1 \
 		| tee ${BENCH_FILE}
 	benchstat ${BENCH_FILE}
 
@@ -104,7 +105,7 @@ ${GO_TARANTOOL_DIR}:
 
 ${REFERENCE_FILE}: ${GO_TARANTOOL_DIR}
 	@echo "Running benchmark tests from master for using results in bench-diff target"
-	cd ${GO_TARANTOOL_DIR} && git pull && go test ./... ${BENCH_OPTIONS} 2>&1 \
+	cd ${GO_TARANTOOL_DIR} && git pull && go test ./... -tags "$(TAGS)" ${BENCH_OPTIONS} 2>&1 \
 		| tee ${REFERENCE_FILE}
 
 bench-diff: ${BENCH_FILES}

README.md

@@ -54,10 +54,20 @@ of any Go program.
 
 ### Build tags
 
-To disable SSL support and linking with OpenSSL, you can use the tag:
-```
-go_tarantool_ssl_disable
-```
+We define multiple [build tags](https://pkg.go.dev/go/build#hdr-Build_Constraints).
+
+This allows us to introduce new features without losing backward compatibility.
+
+1. To disable SSL support and linking with OpenSSL, you can use the tag:
+   ```
+   go_tarantool_ssl_disable
+   ```
+2. to change the default `Call` behavior from `Call16` to `Call17`, you can use the build
+   tag:
+   ```
+   go_tarantool_call_17
+   ```
+   **Note:** In future releases, `Call17` may be used as default `Call` behavior.
 
 ## Documentation
 

call_16_test.go

@@ -0,0 +1,27 @@
+//go:build !go_tarantool_call_17
+// +build !go_tarantool_call_17
+
+package tarantool_test
+
+import (
+	"testing"
+
+	. "github.com/tarantool/go-tarantool"
+)
+
+func TestConnection_Call(t *testing.T) {
+	var resp *Response
+	var err error
+
+	conn := connect(t, server, opts)
+	defer conn.Close()
+
+	// Call16
+	resp, err = conn.Call("simple_incr", []interface{}{1})
+	if err != nil {
+		t.Errorf("Failed to use Call")
+	}
+	if resp.Data[0].([]interface{})[0].(uint64) != 2 {
+		t.Errorf("result is not {{1}} : %v", resp.Data)
+	}
+}

call_17_test.go

@@ -0,0 +1,27 @@
+//go:build go_tarantool_call_17
+// +build go_tarantool_call_17
+
+package tarantool_test
+
+import (
+	"testing"
+
+	. "github.com/tarantool/go-tarantool"
+)
+
+func TestConnection_Call(t *testing.T) {
+	var resp *Response
+	var err error
+
+	conn := connect(t, server, opts)
+	defer conn.Close()
+
+	// Call17
+	resp, err = conn.Call17("simple_incr", []interface{}{1})
+	if err != nil {
+		t.Errorf("Failed to use Call")
+	}
+	if resp.Data[0].(uint64) != 2 {
+		t.Errorf("result is not {{1}} : %v", resp.Data)
+	}
+}

connection.go

@@ -116,9 +116,9 @@ func (d defaultLogger) Report(event ConnLogKind, conn *Connection, v ...interfac
 //
 // ATTENTION: result argument for *Typed methods should deserialize from
 // msgpack array, cause Tarantool always returns result as an array.
-// For all space related methods and Call* (but not Call17*) methods Tarantool
+// For all space related methods and Call16* (but not Call17*) methods Tarantool
 // always returns array of array (array of tuples for space related methods).
-// For Eval* and Call17* Tarantool always returns array, but does not forces
+// For Eval* and Call* Tarantool always returns array, but does not forces
 // array of arrays.
 type Connection struct {
 	addr  string

connection_pool/call_16_test.go

@@ -0,0 +1,69 @@
+//go:build !go_tarantool_call_17
+// +build !go_tarantool_call_17
+
+package connection_pool_test
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+	"github.com/tarantool/go-tarantool/connection_pool"
+	"github.com/tarantool/go-tarantool/test_helpers"
+)
+
+func TestCall(t *testing.T) {
+	roles := []bool{false, true, false, false, true}
+
+	err := test_helpers.SetClusterRO(servers, connOpts, roles)
+	require.Nilf(t, err, "fail to set roles for cluster")
+
+	connPool, err := connection_pool.Connect(servers, connOpts)
+	require.Nilf(t, err, "failed to connect")
+	require.NotNilf(t, connPool, "conn is nil after Connect")
+
+	defer connPool.Close()
+
+	// PreferRO
+	resp, err := connPool.Call("box.info", []interface{}{}, connection_pool.PreferRO)
+	require.Nilf(t, err, "failed to Call")
+	require.NotNilf(t, resp, "response is nil after Call")
+	require.GreaterOrEqualf(t, len(resp.Data), 1, "response.Data is empty after Call")
+
+	val := resp.Data[0].([]interface{})[0].(map[interface{}]interface{})["ro"]
+	ro, ok := val.(bool)
+	require.Truef(t, ok, "expected `true` with mode `PreferRO`")
+	require.Truef(t, ro, "expected `true` with mode `PreferRO`")
+
+	// PreferRW
+	resp, err = connPool.Call("box.info", []interface{}{}, connection_pool.PreferRW)
+	require.Nilf(t, err, "failed to Call")
+	require.NotNilf(t, resp, "response is nil after Call")
+	require.GreaterOrEqualf(t, len(resp.Data), 1, "response.Data is empty after Call")
+
+	val = resp.Data[0].([]interface{})[0].(map[interface{}]interface{})["ro"]
+	ro, ok = val.(bool)
+	require.Truef(t, ok, "expected `false` with mode `PreferRW`")
+	require.Falsef(t, ro, "expected `false` with mode `PreferRW`")
+
+	// RO
+	resp, err = connPool.Call("box.info", []interface{}{}, connection_pool.RO)
+	require.Nilf(t, err, "failed to Call")
+	require.NotNilf(t, resp, "response is nil after Call")
+	require.GreaterOrEqualf(t, len(resp.Data), 1, "response.Data is empty after Call")
+
+	val = resp.Data[0].([]interface{})[0].(map[interface{}]interface{})["ro"]
+	ro, ok = val.(bool)
+	require.Truef(t, ok, "expected `true` with mode `RO`")
+	require.Truef(t, ro, "expected `true` with mode `RO`")
+
+	// RW
+	resp, err = connPool.Call("box.info", []interface{}{}, connection_pool.RW)
+	require.Nilf(t, err, "failed to Call")
+	require.NotNilf(t, resp, "response is nil after Call")
+	require.GreaterOrEqualf(t, len(resp.Data), 1, "response.Data is empty after Call")
+
+	val = resp.Data[0].([]interface{})[0].(map[interface{}]interface{})["ro"]
+	ro, ok = val.(bool)
+	require.Truef(t, ok, "expected `false` with mode `RW`")
+	require.Falsef(t, ro, "expected `false` with mode `RW`")
+}

connection_pool/call_17_test.go

@@ -0,0 +1,69 @@
+//go:build go_tarantool_call_17
+// +build go_tarantool_call_17
+
+package connection_pool_test
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+	"github.com/tarantool/go-tarantool/connection_pool"
+	"github.com/tarantool/go-tarantool/test_helpers"
+)
+
+func TestCall(t *testing.T) {
+	roles := []bool{false, true, false, false, true}
+
+	err := test_helpers.SetClusterRO(servers, connOpts, roles)
+	require.Nilf(t, err, "fail to set roles for cluster")
+
+	connPool, err := connection_pool.Connect(servers, connOpts)
+	require.Nilf(t, err, "failed to connect")
+	require.NotNilf(t, connPool, "conn is nil after Connect")
+
+	defer connPool.Close()
+
+	// PreferRO
+	resp, err := connPool.Call("box.info", []interface{}{}, connection_pool.PreferRO)
+	require.Nilf(t, err, "failed to Call")
+	require.NotNilf(t, resp, "response is nil after Call")
+	require.GreaterOrEqualf(t, len(resp.Data), 1, "response.Data is empty after Call")
+
+	val := resp.Data[0].(map[interface{}]interface{})["ro"]
+	ro, ok := val.(bool)
+	require.Truef(t, ok, "expected `true` with mode `PreferRO`")
+	require.Truef(t, ro, "expected `true` with mode `PreferRO`")
+
+	// PreferRW
+	resp, err = connPool.Call("box.info", []interface{}{}, connection_pool.PreferRW)
+	require.Nilf(t, err, "failed to Call")
+	require.NotNilf(t, resp, "response is nil after Call")
+	require.GreaterOrEqualf(t, len(resp.Data), 1, "response.Data is empty after Call")
+
+	val = resp.Data[0].(map[interface{}]interface{})["ro"]
+	ro, ok = val.(bool)
+	require.Truef(t, ok, "expected `false` with mode `PreferRW`")
+	require.Falsef(t, ro, "expected `false` with mode `PreferRW`")
+
+	// RO
+	resp, err = connPool.Call("box.info", []interface{}{}, connection_pool.RO)
+	require.Nilf(t, err, "failed to Call")
+	require.NotNilf(t, resp, "response is nil after Call")
+	require.GreaterOrEqualf(t, len(resp.Data), 1, "response.Data is empty after Call")
+
+	val = resp.Data[0].(map[interface{}]interface{})["ro"]
+	ro, ok = val.(bool)
+	require.Truef(t, ok, "expected `true` with mode `RO`")
+	require.Truef(t, ro, "expected `true` with mode `RO`")
+
+	// RW
+	resp, err = connPool.Call("box.info", []interface{}{}, connection_pool.RW)
+	require.Nilf(t, err, "failed to Call")
+	require.NotNilf(t, resp, "response is nil after Call")
+	require.GreaterOrEqualf(t, len(resp.Data), 1, "response.Data is empty after Call")
+
+	val = resp.Data[0].(map[interface{}]interface{})["ro"]
+	ro, ok = val.(bool)
+	require.Truef(t, ok, "expected `false` with mode `RW`")
+	require.Falsef(t, ro, "expected `false` with mode `RW`")
+}