Repository: hazelcast/hazelcast-go-client
Pull request #968 (2023-08-18 08:12:58)

Discovery strategy priority
Changed the discovery strategy priority so if a discovery strategy is provided, it overrides Cloud discovery strategy.


Patch:
diff --git a/internal/addr.go b/internal/addr.go
index 6ca5293..592d5df 100644
--- a/internal/addr.go
+++ b/internal/addr.go
@@ -21,31 +21,43 @@ import (
 	"net"
 	"strconv"
 	"strings"
+
+	"github.com/hazelcast/hazelcast-go-client/hzerrors"
+	ihzerrors "github.com/hazelcast/hazelcast-go-client/internal/hzerrors"
 )
 
 const defaultHost = "127.0.0.1"
 
 func ParseAddr(addr string) (string, int, error) {
-	// TODO: refactor this function, it can be written in a better way.
-	if addr == "" || strings.TrimSpace(addr) == "" {
+	addr = strings.TrimSpace(addr)
+	if addr == "" {
 		return defaultHost, 0, nil
 	}
-	if !strings.Contains(addr, ":") {
+	host, port, err := splitHostPort(addr)
+	if err != nil {
+		return "", 0, ihzerrors.NewClientError("", err, hzerrors.ErrInvalidAddress)
+	}
+	if port < 0 || port > 65535 {
+		return "", 0, ihzerrors.NewClientError("port not in valid range", err, hzerrors.ErrInvalidAddress)
+	}
+	return host, port, nil
+}
+
+func splitHostPort(addr string) (host string, port int, err error) {
+	idx := strings.LastIndex(addr, ":")
+	if idx == -1 {
 		return addr, 0, nil
 	}
-	host, port, err := net.SplitHostPort(addr)
+	port, err = strconv.Atoi(addr[idx+1:])
 	if err != nil {
-		return "", 0, err
+		return "", 0, fmt.Errorf("invalid port")
 	}
-	portInt, err := strconv.Atoi(port)
+	host, _, err = net.SplitHostPort(addr)
 	if err != nil {
 		return "", 0, err
 	}
-	if host == "" || strings.TrimSpace(host) == "" {
+	if host == "" {
 		host = defaultHost
 	}
-	if portInt < 0 { // port number should be more than 0
-		return "", 0, fmt.Errorf("invalid port number: '%d'", portInt)
-	}
-	return host, portInt, nil
+	return host, port, nil
 }
diff --git a/internal/client/client.go b/internal/client/client.go
index a303235..7aaf822 100644
--- a/internal/client/client.go
+++ b/internal/client/client.go
@@ -337,14 +337,14 @@ func (c *Client) addDiscoveryDestroyer() {
 }
 
 func addrProviderTranslator(config *cluster.Config, logger ilogger.LogAdaptor) (icluster.AddressProvider, icluster.AddressTranslator) {
-	if config.Cloud.Enabled {
-		dc := cloud.NewDiscoveryClient(&config.Cloud, logger)
-		return cloud.NewAddressProvider(dc), cloud.NewAddressTranslator(dc)
-	}
 	if config.Discovery.Strategy != nil {
 		a := icluster.NewDiscoveryStrategyAdapter(config.Discovery, logger)
 		return a, a
 	}
+	if config.Cloud.Enabled {
+		dc := cloud.NewDiscoveryClient(&config.Cloud, logger)
+		return cloud.NewAddressProvider(dc), cloud.NewAddressTranslator(dc)
+	}
 	pr := icluster.NewDefaultAddressProvider(&config.Network)
 	if config.Discovery.UsePublicIP {
 		return pr, icluster.NewDefaultPublicAddressTranslator()
diff --git a/internal/cluster/address_provider.go b/internal/cluster/address_provider.go
index 97d5a33..412584f 100644
--- a/internal/cluster/address_provider.go
+++ b/internal/cluster/address_provider.go
@@ -19,6 +19,8 @@ package cluster
 import (
 	"context"
 	"fmt"
+	"net"
+	"strconv"
 
 	pubcluster "github.com/hazelcast/hazelcast-go-client/cluster"
 	"github.com/hazelcast/hazelcast-go-client/internal"
@@ -37,7 +39,7 @@ func ParseAddress(addr string) (pubcluster.Address, error) {
 	if err != nil {
 		return "", fmt.Errorf("parsing address: %w", err)
 	}
-	return pubcluster.Address(fmt.Sprintf("%s:%d", host, port)), nil
+	return pubcluster.Address(net.JoinHostPort(host, strconv.Itoa(port))), nil
 }
 
 func NewDefaultAddressProvider(networkConfig *pubcluster.NetworkConfig) *DefaultAddressProvider {


Test patch:
diff --git a/internal/addr_test.go b/internal/addr_test.go
index 1e4164c..359c9a4 100644
--- a/internal/addr_test.go
+++ b/internal/addr_test.go
@@ -17,11 +17,13 @@
 package internal_test
 
 import (
+	"errors"
 	"fmt"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
 
+	"github.com/hazelcast/hazelcast-go-client/hzerrors"
 	"github.com/hazelcast/hazelcast-go-client/internal"
 )
 
@@ -38,13 +40,22 @@ func TestParseAddr(t *testing.T) {
 	}{
 		{Addr: "foo:1234", Target: parseAddrTarget{Host: "foo", Port: 1234, Err: nil}},
 		{Addr: "foo", Target: parseAddrTarget{Host: "foo", Port: 0, Err: nil}},
+		{Addr: "foo:", Target: parseAddrTarget{Host: "", Port: 0, Err: hzerrors.ErrInvalidAddress}},
 		{Addr: ":1234", Target: parseAddrTarget{Host: "127.0.0.1", Port: 1234, Err: nil}},
 		{Addr: "", Target: parseAddrTarget{Host: "127.0.0.1", Port: 0, Err: nil}},
+		{Addr: "169.254.172.39", Target: parseAddrTarget{Host: "169.254.172.39", Port: 0, Err: nil}},
+		{Addr: "169.254.172.39:5703", Target: parseAddrTarget{Host: "169.254.172.39", Port: 5703, Err: nil}},
+		{Addr: "169.254.172.39:", Target: parseAddrTarget{Host: "", Port: 0, Err: hzerrors.ErrInvalidAddress}},
+		{Addr: "[fe80::43:ecff:fec9:1683]:5705", Target: parseAddrTarget{Host: "fe80::43:ecff:fec9:1683", Port: 5705, Err: nil}},
+		{Addr: "[fe80::43:ecff:fec9:1683]:", Target: parseAddrTarget{Host: "", Port: 0, Err: hzerrors.ErrInvalidAddress}},
+		{Addr: "fe80::43:ecff:fec9:1683", Target: parseAddrTarget{Host: "", Port: 0, Err: hzerrors.ErrInvalidAddress}},
+		{Addr: "[fe80::43:ecff:fec9:1683", Target: parseAddrTarget{Host: "", Port: 0, Err: hzerrors.ErrInvalidAddress}},
+		{Addr: "fe80::43:ecff:fec9:1683]:5678", Target: parseAddrTarget{Host: "", Port: 0, Err: hzerrors.ErrInvalidAddress}},
 	}
 	for _, tc := range testCases {
 		t.Run(fmt.Sprintf("addr: %s", tc.Addr), func(t *testing.T) {
 			host, port, err := internal.ParseAddr(tc.Addr)
-			assert.Equal(t, tc.Target.Err, err)
+			assert.True(t, errors.Is(err, tc.Target.Err))
 			assert.Equal(t, tc.Target.Host, host)
 			assert.Equal(t, tc.Target.Port, port)
 		})