Skip to content
Open
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
25 changes: 19 additions & 6 deletions fbconn/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -675,6 +675,19 @@ func makeConnectTimeoutDialFunc(timeout time.Duration) DialFunc {
return d.DialContext
}

var (
// ErrReadOnlyConnection is returned when a read-write connection is required but the connection is read-only.
ErrReadOnlyConnection = errors.New("read only connection")
// ErrReadWriteConnection is returned when a read-only connection is required but the connection is read-write.
ErrReadWriteConnection = errors.New("connection is not read only")
// ErrPrimary is returned when a replica connection is required but the connection is primary.
ErrPrimary = errors.New("not replica")
// ErrReplica is returned when a primary connection is required but the connection is replica.
ErrReplica = errors.New("not primary")
Comment on lines +684 to +686

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Seems more consistent to use ErrPrimaryConnection and ErrReplicaConnection here. This follows public naming pattern and matches ErrReadOnlyConnection and ErrReadWriteConnection above.

// ErrReplicationNotConfigured is returned when replication not configured on host.
ErrReplicationNotConfigured = errors.New("replication is not configured")
)

// ValidateConnectTargetSessionAttrsReadWrite is a ValidateConnectFunc that implements libpq compatible
// target_session_attrs=read-write.
func ValidateConnectTargetSessionAttrsReadWrite(ctx context.Context, fbConn *FbConn) error {
Expand All @@ -684,7 +697,7 @@ func ValidateConnectTargetSessionAttrsReadWrite(ctx context.Context, fbConn *FbC
}

if res.Rows[0][0][3] == 1 {
return errors.New("read only connection")
return ErrReadOnlyConnection
}

return nil
Expand All @@ -699,7 +712,7 @@ func ValidateConnectTargetSessionAttrsReadOnly(ctx context.Context, fbConn *FbCo
}

if res.Rows[0][0][3] != 1 {
return errors.New("connection is not read only")
return ErrReadWriteConnection
}

return nil
Expand All @@ -717,7 +730,7 @@ func checkSessionAttrIsPrimary(ctx context.Context, fbConn *FbConn) (bool, error
return false, nil
}

return false, errors.New("replication is not configured")
return false, ErrReplicationNotConfigured
}

// ValidateConnectTargetSessionAttrsReplica is a [ValidateConnectFunc] that implements
Expand All @@ -730,7 +743,7 @@ func ValidateConnectTargetSessionAttrsReplica(ctx context.Context, fbConn *FbCon
}

if isPrimary {
return errors.New("not replica")
return ErrPrimary
}

return nil
Expand All @@ -746,7 +759,7 @@ func ValidateConnectTargetSessionAttrsPrimary(ctx context.Context, fbConn *FbCon
}

if !isPrimary {
return errors.New("not primary")
return ErrReplica
}

return nil
Expand All @@ -762,7 +775,7 @@ func ValidateConnectTargetSessionAttrsPreferReplica(ctx context.Context, fbConn
}

if isPrimary {
return &NotPreferredError{err: errors.New("not replica")}
return &NotPreferredError{err: ErrPrimary}
}

return nil
Expand Down
47 changes: 47 additions & 0 deletions fbconn/errors_internal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,11 @@ package fbconn
import (
"context"
"errors"
"os"
"testing"
"time"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

Expand All @@ -21,3 +24,47 @@ func TestSyncConnClosedConnErrorIsErrConnClosed(t *testing.T) {
require.Error(t, err)
require.ErrorIs(t, err, ErrConnClosed)
}

func TestConnectWithValidateConnectSessionAttributeErrors(t *testing.T) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm not sure what this test is intended to verify. It opens a real connection, but the callback returns each sentinel directly, so none of the actual target_session_attrs validators are exercised. The read-only/read-write paths are already covered in fbconn_test.go, while the message and NotPreferredError checks can be unit tests without a database.

tests := []struct {
name string
err error
msg string
}{
{"read-only connection", ErrReadOnlyConnection, "read only connection"},
{"read-write connection", ErrReadWriteConnection, "connection is not read only"},
{"primary", ErrPrimary, "not replica"},
{"replica", ErrReplica, "not primary"},
{"replication not configured", ErrReplicationNotConfigured, "replication is not configured"},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
ctx, cancel := context.WithTimeout(context.Background(), 120*time.Second)
defer cancel()

config, err := ParseConfig(os.Getenv("FBX_TEST_DATABASE"))
require.NoError(t, err)
config.ValidateConnect = func(context.Context, *FbConn) error {
return tt.err
}

conn, err := ConnectConfig(ctx, config)
if !assert.NotNil(t, err) {
_ = conn.Close(ctx)
}
require.ErrorIs(t, err, tt.err)
var connectErr *ConnectError
require.ErrorAs(t, err, &connectErr)
require.ErrorContains(t, err, "ValidateConnect failed: "+tt.msg)
require.EqualError(t, tt.err, tt.msg)
})
}

t.Run("prefer-replica", func(t *testing.T) {
err := &NotPreferredError{err: ErrPrimary}
require.ErrorIs(t, err, ErrPrimary)
require.NotErrorIs(t, err, ErrReplica)
require.EqualError(t, err, "standby server not found: not replica")
})
}
20 changes: 20 additions & 0 deletions fbconn/fbconn_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -420,6 +420,26 @@ func TestConnectWithValidateConnectTargetSessionAttrsReadWrite(t *testing.T) {
if !assert.NotNil(t, err) {
_ = conn.Close(ctx)
}
require.ErrorIs(t, err, fbconn.ErrReadOnlyConnection)
}

func TestConnectWithValidateConnectTargetSessionAttrsReadOnly(t *testing.T) {
t.Parallel()

ctx, cancel := context.WithTimeout(context.Background(), 120*time.Second)
defer cancel()

config, err := fbconn.ParseConfig(os.Getenv("FBX_TEST_DATABASE"))
require.NoError(t, err)

config.ValidateConnect = fbconn.ValidateConnectTargetSessionAttrsReadOnly
config.DefaultTransactionReadOnly = false

conn, err := fbconn.ConnectConfig(ctx, config)
if !assert.NotNil(t, err) {
_ = conn.Close(ctx)
}
require.ErrorIs(t, err, fbconn.ErrReadWriteConnection)
}

func TestConnectWithAfterConnect(t *testing.T) {
Expand Down