From e7441f8ceb0fd2e9d115402f68f9f8fd9b0e9a47 Mon Sep 17 00:00:00 2001 From: choksi2212 Date: Mon, 12 Oct 2026 18:08:18 +0530 Subject: [PATCH 1/7] feat(go): parse iggy:// connection strings into TCP client options Add ParseConnectionString to the Go SDK, so a caller can configure the TCP client from a connection string the way the Rust, Node and C# SDKs already do: opts, err := client.ParseConnectionString(iggy://iggy:iggy@localhost:8090) cli, err := client.NewIggyClient(client.WithTcp(opts...)) The parser accepts iggy:// and iggy+tcp:// with username:password or a personal access token before the @, bracketed IPv6 hosts, and the shared query-option grammar: tls, tls_domain and tls_ca_file are applied through the existing WithTLS options, while the reconnect, heartbeat and nodelay spellings are validated for cross-SDK compatibility and then ignored, because the Go client takes those from its own defaults. Other schemes, such as iggy+quic://, are rejected. Tests cover the valid shapes, both credential types, TLS (including a full connect-and-login over TLS against a fake server), the malformed strings each other SDK rejects, and that no error message leaks the password or token. Closes #4491 Signed-off-by: choksi2212 --- foreign/go/client/connection_string.go | 296 ++++++++++++ foreign/go/client/connection_string_test.go | 469 ++++++++++++++++++++ 2 files changed, 765 insertions(+) create mode 100644 foreign/go/client/connection_string.go create mode 100644 foreign/go/client/connection_string_test.go diff --git a/foreign/go/client/connection_string.go b/foreign/go/client/connection_string.go new file mode 100644 index 0000000000..030d1d8a42 --- /dev/null +++ b/foreign/go/client/connection_string.go @@ -0,0 +1,296 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package client + +import ( + "fmt" + "net" + "strconv" + "strings" + + "github.com/apache/iggy/foreign/go/client/tcp" + ierror "github.com/apache/iggy/foreign/go/errors" +) + +const ( + defaultProtocol = "iggy" + protocolPrefix = "iggy+" + tcpProtocol = "tcp" +) + +// ParseConnectionString parses an Iggy TCP connection string into the TCP +// client options it describes, so the result can be passed to WithTcp: +// +// client.NewIggyClient(client.WithTcp(opts...)) +// +// with opts from client.ParseConnectionString("iggy://iggy:iggy@localhost:8090"). +// +// The accepted shape is: +// +// iggy://[username:password@ | personal_access_token@]host:port[?options] +// iggy+tcp://[username:password@ | personal_access_token@]host:port[?options] +// +// The scheme iggy:// means TCP, the same as the explicit iggy+tcp://. +// Other iggy+ schemes such as iggy+quic:// are rejected, +// because the Go SDK implements the TCP transport only, which covers both +// plain TCP and TLS. Credentials are username:password or a single +// personal access token before the @; either way auto-login is configured, +// matching what the Rust and Node SDKs do with a connection string. The +// host may be a bracketed IPv6 literal, as in [::1]:8090. +// +// Query options follow the grammar the other SDKs share: +// +// tls=true|false enable TLS +// tls_domain= TLS server name indication (SNI) +// tls_ca_file= CA certificate file for verification +// reconnection_retries= +// reconnection_interval= +// reestablish_after= +// heartbeat_interval= +// nodelay=true|false +// +// The Go client takes its reconnect, heartbeat, and nodelay behavior from +// its own defaults, so those spellings are validated for compatibility and +// then ignored; a value the other SDKs would reject is still rejected here, +// so one connection string is either valid everywhere or nowhere. +func ParseConnectionString(connectionString string) ([]tcp.Option, error) { + protocolParts := strings.Split(connectionString, "://") + if len(protocolParts) != 2 { + return nil, ierror.InvalidConnectionString{} + } + + protocol, err := parseProtocol(protocolParts[0]) + if err != nil { + return nil, err + } + if protocol != tcpProtocol { + return nil, fmt.Errorf("%w: unsupported transport %q, Go SDK supports tcp only", + ierror.InvalidConnectionString{}, protocol) + } + + parts := strings.Split(protocolParts[1], "@") + if len(parts) != 2 { + return nil, ierror.InvalidConnectionString{} + } + + credentials, err := parseCredentials(parts[0]) + if err != nil { + return nil, err + } + + serverAndOptions := strings.Split(parts[1], "?") + if len(serverAndOptions) > 2 { + return nil, ierror.InvalidConnectionString{} + } + + var tlsConfig parsedTLS + if len(serverAndOptions) == 2 { + tlsConfig, err = parseConnectionOptions(serverAndOptions[1]) + if err != nil { + return nil, err + } + } + + serverAddress := serverAndOptions[0] + if serverAddress == "" { + return nil, ierror.InvalidConnectionString{} + } + host, portStr, err := net.SplitHostPort(serverAddress) + if err != nil || host == "" { + return nil, ierror.InvalidConnectionString{} + } + // Node and Rust accept any u16 port, 0 included; the dial fails later + // instead of the parse. + if _, err := strconv.ParseUint(portStr, 10, 16); err != nil { + return nil, ierror.InvalidNumberValue{} + } + + options := []tcp.Option{tcp.WithServerAddress(serverAddress)} + options = append(options, tcp.WithAutoLogin(credentials)) + if tlsConfig.enabled { + var tlsOpts []tcp.TLSOption + if tlsConfig.domain != "" { + tlsOpts = append(tlsOpts, tcp.WithTLSDomain(tlsConfig.domain)) + } + if tlsConfig.caFile != "" { + tlsOpts = append(tlsOpts, tcp.WithTLSCAFile(tlsConfig.caFile)) + } + options = append(options, tcp.WithTLS(tlsOpts...)) + } + return options, nil +} + +// parseProtocol extracts the transport from the scheme, which is either the +// bare default protocol or an explicit iggy+ spelling. +func parseProtocol(scheme string) (string, error) { + if scheme == defaultProtocol { + return tcpProtocol, nil + } + if !strings.HasPrefix(scheme, protocolPrefix) { + return "", ierror.InvalidConnectionString{} + } + return strings.TrimPrefix(scheme, protocolPrefix), nil +} + +// parseCredentials turns the userinfo before the @ into auto-login +// credentials: username:password or a single personal access token. +func parseCredentials(userinfo string) (tcp.Credentials, error) { + username, password, hasSeparator := strings.Cut(userinfo, ":") + if !hasSeparator { + if username == "" { + return tcp.Credentials{}, ierror.InvalidConnectionString{} + } + return tcp.NewPersonalAccessTokenCredentials(username), nil + } + if username == "" || password == "" { + return tcp.Credentials{}, ierror.InvalidConnectionString{} + } + return tcp.NewUsernamePasswordCredentials(username, password), nil +} + +type parsedTLS struct { + enabled bool + domain string + caFile string +} + +// parseConnectionOptions validates the shared connection-string grammar, +// keeping the tls options and ignoring the reconnect spellings the Go +// client does not take from a connection string. +func parseConnectionOptions(optionsString string) (parsedTLS, error) { + parsed := parsedTLS{} + if optionsString == "" { + return parsed, ierror.InvalidConnectionString{} + } + for _, option := range strings.Split(optionsString, "&") { + name, value, found := strings.Cut(option, "=") + if !found { + return parsed, ierror.InvalidConnectionString{} + } + switch name { + case "tls": + if value != "true" && value != "false" { + return parsed, ierror.InvalidConnectionString{} + } + parsed.enabled = value == "true" + case "tls_domain": + parsed.domain = value + case "tls_ca_file": + parsed.caFile = value + case "reconnection_retries": + if value != "unlimited" { + if _, err := strconv.ParseUint(value, 10, 32); err != nil { + return parsed, ierror.InvalidNumberValue{} + } + } + case "reconnection_interval", "reestablish_after", "heartbeat_interval": + if !isValidIggyDuration(value) { + return parsed, ierror.InvalidConnectionString{} + } + case "nodelay": + if value != "true" && value != "false" { + return parsed, ierror.InvalidConnectionString{} + } + default: + return parsed, ierror.InvalidConnectionString{} + } + } + return parsed, nil +} + +// isValidIggyDuration reports whether value is a duration the Rust SDK +// accepts for its connection-string options, which the Go SDK reuses for +// its grammar: the zero spellings, or a humantime-style duration such as +// 1s, 500ms, 1h 30m or 1h30m. +func isValidIggyDuration(value string) bool { + lower := strings.ToLower(value) + switch lower { + case "0", "unlimited", "disabled", "none": + return true + } + remainder := strings.TrimLeft(lower, " ") + for remainder != "" { + amount, rest, ok := splitLeadingNumber(remainder) + if !ok || amount <= 0 { + return false + } + unit, rest := cutDurationUnit(rest) + if unit == "" { + return false + } + // Parts may be separated by spaces, as in 1h 30m, or run + // together, as in 1h30m. + remainder = strings.TrimLeft(rest, " ") + } + return true +} + +// splitLeadingNumber consumes the leading decimal digits of s, with an +// optional fractional part as in 0.5s or 1.005s. A leading sign is +// rejected, because negative durations do not parse. The value is +// returned as a number, so a fraction-only amount such as 0.5 still +// counts as positive. +func splitLeadingNumber(s string) (float64, string, bool) { + digits := 0 + for digits < len(s) && s[digits] >= '0' && s[digits] <= '9' { + digits++ + } + // A fraction only counts when a digit follows the dot, so a bogus + // 1.s does not parse and 5. does not either. + if digits+2 <= len(s) && s[digits] == '.' && s[digits+1] >= '0' && s[digits+1] <= '9' { + digits++ // the dot + for digits < len(s) && s[digits] >= '0' && s[digits] <= '9' { + digits++ + } + } + if digits == 0 { + return 0, "", false + } + n, err := strconv.ParseFloat(s[:digits], 64) + if err != nil { + return 0, "", false + } + return n, s[digits:], true +} + +// durationUnits are the humantime units, longest spelling first so a +// prefix match cannot cut a longer unit short. +var durationUnits = []string{ + "milliseconds", "millisecond", "msec", "millis", "ms", + "microseconds", "microsecond", "usec", "us", + "nanoseconds", "nanosecond", "nsec", "ns", + "seconds", "second", "secs", "sec", "s", + "minutes", "minute", "mins", "min", "m", + "hours", "hour", "h", + "days", "day", "d", + "weeks", "week", "w", + "years", "year", "y", +} + +// cutDurationUnit consumes the leading duration unit of s, longest +// spelling first so a prefix match cannot cut a longer unit short. The +// unit may run straight into the next amount, as in 1h30m; a leftover +// with no leading unit, such as the x of 5x, still fails the caller. +func cutDurationUnit(s string) (unit string, rest string) { + for _, unit := range durationUnits { + if strings.HasPrefix(s, unit) { + return unit, s[len(unit):] + } + } + return "", s +} diff --git a/foreign/go/client/connection_string_test.go b/foreign/go/client/connection_string_test.go new file mode 100644 index 0000000000..5e89d535c8 --- /dev/null +++ b/foreign/go/client/connection_string_test.go @@ -0,0 +1,469 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package client + +import ( + "context" + "crypto/ecdsa" + "crypto/elliptic" + "crypto/rand" + "crypto/tls" + "crypto/x509" + "crypto/x509/pkix" + "encoding/binary" + "encoding/pem" + "io" + "math/big" + "net" + "os" + "path/filepath" + "sync" + "testing" + "time" + + "github.com/apache/iggy/foreign/go/client/tcp" + iggcon "github.com/apache/iggy/foreign/go/contracts" + "github.com/apache/iggy/foreign/go/internal/command" + "github.com/apache/iggy/foreign/go/internal/vsr" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// applyOptions runs options the way NewIggyClient does, and returns the +// resulting TCP client so a test can observe what the parser produced. +func applyOptions(t *testing.T, options []tcp.Option) *tcp.IggyTcpClient { + t.Helper() + return tcp.NewIggyTcpClient(nil, options...) +} + +func TestParseConnectionString_ParsesTheDefaultSchemeWithPasswordCredentials(t *testing.T) { + opts, err := ParseConnectionString("iggy://iggy:secret@127.0.0.1:8090") + require.NoError(t, err) + client := applyOptions(t, opts) + + assert.Equal(t, "127.0.0.1:8090", client.GetConnectionInfo().ServerAddress) +} + +func TestParseConnectionString_ParsesTheExplicitTcpSchemeWithAPersonalAccessToken(t *testing.T) { + opts, err := ParseConnectionString("iggy+tcp://iggypat-1234567890abcdef@localhost:8090") + require.NoError(t, err) + client := applyOptions(t, opts) + + assert.Equal(t, "localhost:8090", client.GetConnectionInfo().ServerAddress) +} + +func TestParseConnectionString_ParsesIPv6HostWithoutBrackets(t *testing.T) { + opts, err := ParseConnectionString("iggy://iggy:secret@[::1]:8090") + require.NoError(t, err) + client := applyOptions(t, opts) + + assert.Equal(t, "[::1]:8090", client.GetConnectionInfo().ServerAddress) +} + +// Frame offsets the fake server reads and writes. They mirror the +// unexported tables of the codec package, which this package cannot reach. +const ( + frameOffsetSize = 48 + frameOffsetCommand = 60 + frameOffsetClient = 128 + frameOffsetRequest = 168 + frameOffsetOperation = 176 + frameOffsetSession = 184 + frameOffsetReserved = 196 + + replyFrameOffsetRequest = 200 + replyFrameOffsetOperation = 208 + replyFrameOffsetStatus = 216 +) + +// wireRequest is one frame the fake server read off the wire. +type wireRequest struct { + header [vsr.HeaderSize]byte + payload []byte +} + +func (r wireRequest) operation() vsr.Operation { + return vsr.Operation(r.header[frameOffsetOperation]) +} + +func (r wireRequest) code() uint32 { + return binary.LittleEndian.Uint32(r.header[frameOffsetReserved:]) +} + +// readWireRequest reads one complete frame from conn. +func readWireRequest(conn net.Conn) (wireRequest, error) { + var read wireRequest + if _, err := io.ReadFull(conn, read.header[:]); err != nil { + return read, err + } + size := binary.LittleEndian.Uint32(read.header[frameOffsetSize:]) + if size > vsr.HeaderSize { + read.payload = make([]byte, int(size)-vsr.HeaderSize) + if _, err := io.ReadFull(conn, read.payload); err != nil { + return read, err + } + } + return read, nil +} + +// replyFrameFor builds a reply whose header carries the given operation. +func replyFrameFor(operation vsr.Operation, body []byte) []byte { + frame := make([]byte, vsr.HeaderSize+len(body)) + binary.LittleEndian.PutUint32(frame[frameOffsetSize:], uint32(len(frame))) + frame[frameOffsetCommand] = 8 + frame[replyFrameOffsetOperation] = byte(operation) + binary.LittleEndian.PutUint32(frame[replyFrameOffsetStatus:], 0) + copy(frame[vsr.HeaderSize:], body) + return frame +} + +// resultSection builds the committed result prefix of a metadata reply. +func resultSection(codes ...uint32) []byte { + section := binary.LittleEndian.AppendUint32(nil, uint32(len(codes))) + for index, code := range codes { + section = binary.LittleEndian.AppendUint32(section, uint32(index)) + section = binary.LittleEndian.AppendUint32(section, code) + } + return section +} + +// registerReply builds the reply of a successful sign-in. +func registerReply(userID uint32, session uint64) []byte { + body := binary.LittleEndian.AppendUint32(resultSection(), userID) + body = binary.LittleEndian.AppendUint64(body, session) + body = binary.LittleEndian.AppendUint32(body, vsr.ProtocolVersion) + body = append(body, byte(len("0.11.0-edge.1"))) + body = append(body, "0.11.0-edge.1"...) + return replyFrameFor(vsr.OperationRegister, body) +} + +// bindReply builds the reply of a successful session bind. +func bindReply(userID uint32, session uint64) []byte { + registered := registerReply(userID, session) + return replyFrameFor(vsr.OperationNonReplicated, registered[vsr.HeaderSize+len(resultSection()):]) +} + +// clusterMetadataReply builds a roster where the node at the listener +// address is the healthy leader. +func clusterMetadataReply(t *testing.T, address string) []byte { + t.Helper() + + metadata := iggcon.ClusterMetadata{Name: "test-cluster"} + host, port, err := net.SplitHostPort(address) + require.NoError(t, err) + tcpPort, err := net.LookupPort("tcp", port) + require.NoError(t, err) + metadata.Nodes = append(metadata.Nodes, iggcon.ClusterNode{ + Name: host, + IP: host, + Role: iggcon.RoleLeader, + Status: iggcon.Healthy, + Endpoints: iggcon.TransportEndpoints{Tcp: uint16(tcpPort)}, + }) + + body, err := metadata.MarshalBinary() + require.NoError(t, err) + return replyFrameFor(vsr.OperationNonReplicated, body) +} + +// fakeVSRServer answers frames the way a single-node Iggy server does, so a +// test can drive a full connect-and-login flow without a real server. +type fakeVSRServer struct { + mtx sync.Mutex + requests []wireRequest + listener net.Listener +} + +func listenFakeVSR(t *testing.T, wrap func(net.Conn) net.Conn) *fakeVSRServer { + t.Helper() + + listener, err := net.Listen("tcp", "127.0.0.1:0") + require.NoError(t, err) + t.Cleanup(func() { _ = listener.Close() }) + + server := &fakeVSRServer{listener: listener} + go func() { + for { + conn, err := listener.Accept() + if err != nil { + return + } + if wrap != nil { + conn = wrap(conn) + } + go func(conn net.Conn) { + defer func() { _ = conn.Close() }() + for { + read, err := readWireRequest(conn) + if err != nil { + return + } + server.mtx.Lock() + server.requests = append(server.requests, read) + server.mtx.Unlock() + + var answer []byte + switch { + case read.code() == uint32(command.BindSessionCode): + answer = bindReply(7, read.sessionID()) + case read.code() == uint32(command.GetClusterMetadataCode): + answer = clusterMetadataReply(t, server.listener.Addr().String()) + case read.operation() == vsr.OperationRegister: + answer = registerReply(7, 128) + default: + answer = replyFrameFor(vsr.OperationNonReplicated, nil) + } + // Echo the request id the way the server correlates replies. + if len(answer) >= vsr.HeaderSize && answer[frameOffsetCommand] == 8 { + copy(answer[replyFrameOffsetRequest:replyFrameOffsetRequest+8], + read.header[frameOffsetRequest:frameOffsetRequest+8]) + } + if _, err := conn.Write(answer); err != nil { + return + } + } + }(conn) + } + }() + return server +} + +func (s *fakeVSRServer) recorded() []wireRequest { + s.mtx.Lock() + defer s.mtx.Unlock() + return append([]wireRequest(nil), s.requests...) +} + +// sessionID reads the logical session the request carries. +func (r wireRequest) sessionID() uint64 { + return binary.LittleEndian.Uint64(r.header[frameOffsetSession:]) +} + +// TestParseConnectionString_ConnectsAndLogsInAgainstAServer drives the +// full connect-and-login flow from the issue acceptance criteria: opts +// from ParseConnectionString, wired into NewIggyClient via WithTcp, +// connecting and signing in against a running (fake) server. +func TestParseConnectionString_ConnectsAndLogsInAgainstAServer(t *testing.T) { + server := listenFakeVSR(t, nil) + address := server.listener.Addr().String() + + opts, err := ParseConnectionString("iggy://iggy:iggy@" + address) + require.NoError(t, err) + cli, err := NewIggyClient(WithTcp(opts...)) + require.NoError(t, err) + t.Cleanup(func() { _ = cli.Close() }) + + require.NoError(t, cli.Connect(context.Background())) + + recorded := server.recorded() + require.Len(t, recorded, 2) + // The roster read is auth-gated, so the sign-in comes first. + assert.Equal(t, vsr.OperationRegister, recorded[0].operation()) + assert.Contains(t, string(recorded[0].payload), "iggy") + assert.Equal(t, uint32(command.GetClusterMetadataCode), recorded[1].code()) +} + +// TestParseConnectionString_TLSEnabledByQueryString drives the same flow +// with tls=true and a TLS-only fake server: a client that ignored the +// option would send its register frame in plaintext and the TLS handshake +// would fail, so a successful connect-and-login proves the option landed. +func TestParseConnectionString_TLSEnabledByQueryString(t *testing.T) { + certificate, caPath := selfSignedCert(t) + + server := listenFakeVSR(t, func(conn net.Conn) net.Conn { + return tls.Server(conn, &tls.Config{Certificates: []tls.Certificate{certificate}}) + }) + address := server.listener.Addr().String() + host, port, err := net.SplitHostPort(address) + require.NoError(t, err) + + connectionString := "iggy://iggy:iggy@" + host + ":" + port + + "?tls=true&tls_domain=localhost&tls_ca_file=" + caPath + opts, err := ParseConnectionString(connectionString) + require.NoError(t, err) + cli, err := NewIggyClient(WithTcp(opts...)) + require.NoError(t, err) + t.Cleanup(func() { _ = cli.Close() }) + + require.NoError(t, cli.Connect(context.Background())) + + recorded := server.recorded() + require.Len(t, recorded, 2) + assert.Equal(t, vsr.OperationRegister, recorded[0].operation()) + assert.Equal(t, uint32(command.GetClusterMetadataCode), recorded[1].code()) +} + +// selfSignedCert mints a throwaway server certificate for the loopback +// listener, so the test carries no expiry or checkout-layout dependency. +func selfSignedCert(t *testing.T) (tls.Certificate, string) { + t.Helper() + + key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + require.NoError(t, err) + template := x509.Certificate{ + SerialNumber: big.NewInt(1), + Subject: pkix.Name{CommonName: "localhost"}, + NotBefore: time.Now().Add(-time.Hour), + NotAfter: time.Now().Add(time.Hour), + KeyUsage: x509.KeyUsageDigitalSignature | x509.KeyUsageCertSign, + ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth}, + BasicConstraintsValid: true, + IsCA: true, + DNSNames: []string{"localhost"}, + IPAddresses: []net.IP{net.IPv4(127, 0, 0, 1)}, + } + certDER, err := x509.CreateCertificate(rand.Reader, &template, &template, &key.PublicKey, key) + require.NoError(t, err) + keyDER, err := x509.MarshalECPrivateKey(key) + require.NoError(t, err) + + certPEM := pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: certDER}) + keyPEM := pem.EncodeToMemory(&pem.Block{Type: "EC PRIVATE KEY", Bytes: keyDER}) + certificate, err := tls.X509KeyPair(certPEM, keyPEM) + require.NoError(t, err) + + caPath := filepath.Join(t.TempDir(), "ca.pem") + require.NoError(t, os.WriteFile(caPath, certPEM, 0o600)) + return certificate, caPath +} + +// TestParseConnectionString_RejectsMalformedConnectionStrings pins every +// malformed shape the other SDKs reject, copied from their test tables. +func TestParseConnectionString_RejectsMalformedConnectionStrings(t *testing.T) { + for _, value := range []string{ + "", + "iggy", + "iggy://", + "iggy://:secret@localhost:8090", + "iggy://iggy:@localhost:8090", + "iggy://iggy:secret@localhost", + "iggy://iggy:secret@:8090", + "iggy://iggy:secret@localhost:port", + "iggy://iggy:secret@localhost:70000", + "iggy://iggy:secret@localhost:8090?unknown=value", + "iggy://iggy:secret@localhost:8090?tls=maybe", + "iggy://iggy:secret@localhost:8090?reconnection_retries=three", + "iggy://iggy:secret@localhost:8090?reconnection_retries=4294967296", + "iggy://iggy:secret@localhost:8090?reconnection_interval=5", + "iggy://iggy:secret@localhost:8090?reconnection_interval=garbage", + "iggy://iggy:secret@localhost:8090?reconnection_interval=-1s", + "iggy://iggy:secret@localhost:8090?nodelay=yes", + "iggy://iggy:secret@[::1:8090", + "iggy://iggy:secret@[]:8090", + "iggy://iggy:secret@[::1]x:8090", + "iggy://iggy:secret@2001:db8::1:8090", + "iggy://iggy:secret@host:8090:9090", + "iggy://iggy:secret@localhost:8090?", + "iggy://iggy:secret@localhost:8090?&", + "iggy://iggy:secret@localhost:8090?reestablish_after=garbage", + } { + t.Run(value, func(t *testing.T) { + opts, err := ParseConnectionString(value) + require.Error(t, err) + assert.Empty(t, opts) + }) + } +} + +// TestParseConnectionString_AcceptsPortZero pins the u16 port rule the +// other SDKs share: port 0 parses, because the port is only a u16 on the +// wire and the dial fails later instead of the parse. +func TestParseConnectionString_AcceptsPortZero(t *testing.T) { + opts, err := ParseConnectionString("iggy://iggy:secret@localhost:0") + require.NoError(t, err) + client := applyOptions(t, opts) + assert.Equal(t, "localhost:0", client.GetConnectionInfo().ServerAddress) +} + +// TestParseConnectionString_AcceptsFractionalAndMultiUnitDurations pins +// the duration grammar the other SDKs share: fractional seconds and +// sequences of units, all accepted in connection-string options. +func TestParseConnectionString_AcceptsFractionalAndMultiUnitDurations(t *testing.T) { + for _, options := range []string{ + "reconnection_interval=500ms", + "reconnection_interval=0.5s", + "reconnection_interval=1.005s", + "reconnection_interval=2m", + "reconnection_interval=1h 1m 1s", + "reconnection_interval=1h30m", + "reconnection_interval=5sec", + "reconnection_interval=5msec", + "heartbeat_interval=10s", + "reestablish_after=10s", + } { + t.Run(options, func(t *testing.T) { + opts, err := ParseConnectionString("iggy://iggy:secret@localhost:8090?" + options) + require.NoError(t, err) + assert.NotEmpty(t, opts) + }) + } +} + +// TestParseConnectionString_AcceptsUnlimitedRetries pins the u32 ceiling +// rule: unlimited is the keyword, and any u32 value parses. +func TestParseConnectionString_AcceptsUnlimitedRetries(t *testing.T) { + for _, options := range []string{ + "reconnection_retries=unlimited", + "reconnection_retries=3", + "reconnection_retries=4294967295", + } { + t.Run(options, func(t *testing.T) { + opts, err := ParseConnectionString("iggy://iggy:secret@localhost:8090?" + options) + require.NoError(t, err) + assert.NotEmpty(t, opts) + }) + } +} + +// TestParseConnectionString_RejectsUnsupportedTransports pins the scheme +// rule: the default scheme and the explicit tcp spelling parse, other +// iggy+ spellings are rejected with an error naming the +// transport. +func TestParseConnectionString_RejectsUnsupportedTransports(t *testing.T) { + for _, value := range []string{ + "iggy+quic://iggy:secret@localhost:8090", + "iggy+ws://iggy:secret@localhost:8090", + "http://iggy:secret@localhost:8090", + } { + opts, err := ParseConnectionString(value) + require.Error(t, err) + assert.Empty(t, opts) + } +} + +// TestParseConnectionString_ErrorsNeverLeakTheSecret checks that no error +// message contains the password or the token from the rejected string. +func TestParseConnectionString_ErrorsNeverLeakTheSecret(t *testing.T) { + secrets := []string{"hunter2", "iggypat-1234567890abcdef"} + for _, value := range []string{ + "iggy://iggy:hunter2@localhost", + "iggy+tcp://iggypat-1234567890abcdef@localhost", + "iggy://iggy:hunter2@localhost:8090?unknown=value", + "iggy://iggy:hunter2@localhost:8090?tls=maybe", + "iggy://iggy:hunter2@localhost:8090?reconnection_retries=three", + "iggy://iggy:hunter2@localhost:70000", + } { + opts, err := ParseConnectionString(value) + require.Error(t, err) + assert.Empty(t, opts) + for _, secret := range secrets { + assert.NotContains(t, err.Error(), secret, + "error message leaked a secret") + } + } +} From a9c4b1ae082edb06d1e2f1dfca67efb1d38ad682 Mon Sep 17 00:00:00 2001 From: choksi2212 Date: Mon, 12 Oct 2026 23:27:43 +0530 Subject: [PATCH 2/7] fix(go): address review on the connection-string parser Apply all nine review comments on #4497: - nodelay: the parser validated the spelling and dropped it, so ?nodelay=false still left Nagle disabled. The parsed value now reaches tcp.WithNoDelay, and the client default (Nagle off) is preserved when the option is absent. GetNoDelay exposes the setting, so a test can observe the option before a connection exists. - durations: reconnection_interval and heartbeat_interval are non-zero durations in the Rust SDK, so the zero spellings and zero-valued durations are now rejected for those two, while reestablish_after keeps accepting them, because Rust types it as a plain duration. - units: the duration table now matches the Node and C# SDKs, carrying hr, hrs, wk, wks and month and taking millis instead of milliseconds. - transport errors echo only an ASCII-letter transport name, so a malformed scheme with embedded credentials cannot leak them. - a bad port reports InvalidConnectionString like every other parse failure, so errors.Is covers the whole parse. - the IPv6 test is renamed for the bracketed host it passes. - the rejection table is attributed to the Node SDK, whose shapes it mirrors, not Rust. - the unused frameOffsetClient constant is deleted. - the frame offsets, fake server, reply builders and self-signed certificate helper move into one shared internal test-support package, internal/fakevsr, which both test binaries import; tcp_testing_test.go keeps only its TCP-specific fixtures. New tests pin each behavior change: nodelay landing on the client, the zero-spelling rejections per option, the extended duration units, and transport errors never echoing credentials. Signed-off-by: choksi2212 --- foreign/go/client/connection_string.go | 298 ++++++++++++---- foreign/go/client/connection_string_test.go | 310 ++++++---------- foreign/go/client/tcp/tcp_connect_test.go | 262 +++++++------- .../tcp/tcp_consumer_group_management_test.go | 34 +- foreign/go/client/tcp/tcp_core.go | 9 + foreign/go/client/tcp/tcp_core_encode_test.go | 5 +- .../go/client/tcp/tcp_group_polling_test.go | 136 +++---- .../tcp/tcp_session_credentials_test.go | 1 - .../client/tcp/tcp_stream_management_test.go | 46 +-- .../client/tcp/tcp_topic_management_test.go | 32 +- foreign/go/internal/fakevsr/fakevsr.go | 333 ++++++++++++++++++ 11 files changed, 941 insertions(+), 525 deletions(-) create mode 100644 foreign/go/internal/fakevsr/fakevsr.go diff --git a/foreign/go/client/connection_string.go b/foreign/go/client/connection_string.go index 030d1d8a42..99561caba3 100644 --- a/foreign/go/client/connection_string.go +++ b/foreign/go/client/connection_string.go @@ -19,6 +19,7 @@ package client import ( "fmt" + "math/big" "net" "strconv" "strings" @@ -59,15 +60,23 @@ const ( // tls_domain= TLS server name indication (SNI) // tls_ca_file= CA certificate file for verification // reconnection_retries= -// reconnection_interval= +// reconnection_interval= // reestablish_after= -// heartbeat_interval= +// heartbeat_interval= // nodelay=true|false // -// The Go client takes its reconnect, heartbeat, and nodelay behavior from -// its own defaults, so those spellings are validated for compatibility and -// then ignored; a value the other SDKs would reject is still rejected here, -// so one connection string is either valid everywhere or nowhere. +// Durations take the units the other SDKs share, from nanos to years, +// including the hr, hrs, wk, wks and month spellings, with optional +// fractions and parts run together or spaced, as in 1h30m or 1h 30m. +// The reconnection_interval and heartbeat_interval spellings must be +// non-zero, because the Rust SDK types them as non-zero durations; +// reestablish_after takes the zero spellings, which Rust accepts there. +// The Go client keeps its own reconnect and heartbeat pacing, so those +// three spellings are validated for compatibility and then ignored, and +// a value the other SDKs would reject is still rejected here, so one +// connection string is either valid everywhere or nowhere. The nodelay +// spelling is applied, because the Go client carries a matching +// TCP_NODELAY option. func ParseConnectionString(connectionString string) ([]tcp.Option, error) { protocolParts := strings.Split(connectionString, "://") if len(protocolParts) != 2 { @@ -79,6 +88,12 @@ func ParseConnectionString(connectionString string) ([]tcp.Option, error) { return nil, err } if protocol != tcpProtocol { + // Only a plain transport name is echoed, because a malformed + // scheme may carry credentials, as the C# SDK notes on the same + // check. + if !isASCIILetters(protocol) { + return nil, ierror.InvalidConnectionString{} + } return nil, fmt.Errorf("%w: unsupported transport %q, Go SDK supports tcp only", ierror.InvalidConnectionString{}, protocol) } @@ -99,6 +114,7 @@ func ParseConnectionString(connectionString string) ([]tcp.Option, error) { } var tlsConfig parsedTLS + tlsConfig.noDelay = true if len(serverAndOptions) == 2 { tlsConfig, err = parseConnectionOptions(serverAndOptions[1]) if err != nil { @@ -115,9 +131,10 @@ func ParseConnectionString(connectionString string) ([]tcp.Option, error) { return nil, ierror.InvalidConnectionString{} } // Node and Rust accept any u16 port, 0 included; the dial fails later - // instead of the parse. + // instead of the parse. A bad port reports the same error as every other + // parse failure, which is what the Rust SDK returns here. if _, err := strconv.ParseUint(portStr, 10, 16); err != nil { - return nil, ierror.InvalidNumberValue{} + return nil, ierror.InvalidConnectionString{} } options := []tcp.Option{tcp.WithServerAddress(serverAddress)} @@ -132,6 +149,7 @@ func ParseConnectionString(connectionString string) ([]tcp.Option, error) { } options = append(options, tcp.WithTLS(tlsOpts...)) } + options = append(options, tcp.WithNoDelay(tlsConfig.noDelay)) return options, nil } @@ -167,6 +185,7 @@ type parsedTLS struct { enabled bool domain string caFile string + noDelay bool } // parseConnectionOptions validates the shared connection-string grammar, @@ -198,14 +217,23 @@ func parseConnectionOptions(optionsString string) (parsedTLS, error) { return parsed, ierror.InvalidNumberValue{} } } - case "reconnection_interval", "reestablish_after", "heartbeat_interval": - if !isValidIggyDuration(value) { + case "reconnection_interval", "heartbeat_interval": + // Rust types both as non-zero durations, so the zero spellings + // and any zero-valued duration are rejected. + if nanos, err := parseIggyDuration(value); err != nil || nanos == 0 { + return parsed, ierror.InvalidConnectionString{} + } + case "reestablish_after": + // Rust takes this one as a plain duration, where the zero + // spellings parse. + if _, err := parseIggyDuration(value); err != nil { return parsed, ierror.InvalidConnectionString{} } case "nodelay": if value != "true" && value != "false" { return parsed, ierror.InvalidConnectionString{} } + parsed.noDelay = value == "true" default: return parsed, ierror.InvalidConnectionString{} } @@ -213,84 +241,220 @@ func parseConnectionOptions(optionsString string) (parsedTLS, error) { return parsed, nil } -// isValidIggyDuration reports whether value is a duration the Rust SDK -// accepts for its connection-string options, which the Go SDK reuses for -// its grammar: the zero spellings, or a humantime-style duration such as -// 1s, 500ms, 1h 30m or 1h30m. -func isValidIggyDuration(value string) bool { +// isASCIILetters reports whether s is non-empty and ASCII letters only, so +// an unsupported-transport error can echo it without ever echoing the rest +// of a malformed scheme, which may carry credentials. +func isASCIILetters(s string) bool { + if s == "" { + return false + } + for i := 0; i < len(s); i++ { + if (s[i] < 'a' || s[i] > 'z') && (s[i] < 'A' || s[i] > 'Z') { + return false + } + } + return true +} + +// parseIggyDuration parses a duration the Rust SDK accepts, returning the +// total nanoseconds. It mirrors IggyDuration::from_str: the value is +// lowercased, the zero spellings become a zero duration, and everything +// else goes through a humantime-style grammar of amounts and units, with +// optional fractions and parts run together as in 1h30m or spaced as in +// 1h 30m. Fractions must land on a whole nanosecond, as in the Rust and +// Node parsers, so 1.5us parses while 0.5ns does not. +func parseIggyDuration(value string) (uint64, error) { lower := strings.ToLower(value) switch lower { case "0", "unlimited", "disabled", "none": - return true + return 0, nil } - remainder := strings.TrimLeft(lower, " ") + remainder := strings.TrimLeft(lower, " ") + if remainder == "" { + return 0, errInvalidDuration(value) + } + + var seconds, nanos big.Int for remainder != "" { - amount, rest, ok := splitLeadingNumber(remainder) - if !ok || amount <= 0 { - return false + wholePart, fracPart, fracScale, rest, ok := cutLeadingAmount(remainder) + if !ok { + return 0, errInvalidDuration(value) } - unit, rest := cutDurationUnit(rest) - if unit == "" { - return false + remainder = rest + + index, rest, ok := cutDurationUnit(remainder) + if !ok { + return 0, errInvalidDuration(value) + } + remainder = strings.TrimLeft(rest, " ") + unit := durationUnits[index] + + whole := new(big.Int).SetUint64(wholePart) + seconds.Add(&seconds, whole.Mul(whole, new(big.Int).SetUint64(unit.seconds))) + whole = new(big.Int).SetUint64(wholePart) + nanos.Add(&nanos, whole.Mul(whole, new(big.Int).SetUint64(unit.nanos))) + + if fracScale != 0 { + // A fraction of a nanosecond always loses precision, as the + // Rust parser reports outright. + if unit.seconds == 0 && unit.nanos == 1 { + return 0, errInvalidDuration(value) + } + numerator := new(big.Int).SetUint64(fracPart) + denominator := new(big.Int).SetUint64(fracScale) + frac := new(big.Int) + switch { + case unit.nanos != 0: // usec and millis land in nanoseconds + frac.Mul(numerator, new(big.Int).SetUint64(unit.nanos)) + if new(big.Int).Mod(frac, denominator).Sign() != 0 { + return 0, errInvalidDuration(value) + } + nanos.Add(&nanos, frac.Div(frac, denominator)) + case unit.seconds == 1: // seconds land in nanoseconds + frac.Mul(numerator, big.NewInt(1_000_000_000)) + if new(big.Int).Mod(frac, denominator).Sign() != 0 { + return 0, errInvalidDuration(value) + } + nanos.Add(&nanos, frac.Div(frac, denominator)) + default: // minutes and above land in whole seconds + frac.Mul(numerator, new(big.Int).SetUint64(unit.seconds)) + if new(big.Int).Mod(frac, denominator).Sign() != 0 { + return 0, errInvalidDuration(value) + } + seconds.Add(&seconds, frac.Div(frac, denominator)) + } } - // Parts may be separated by spaces, as in 1h 30m, or run - // together, as in 1h30m. - remainder = strings.TrimLeft(rest, " ") } - return true + + // A carry out of the nanoseconds, as the Rust implementation does + // past a full second. + billion := big.NewInt(1_000_000_000) + if nanos.Cmp(billion) >= 0 { + seconds.Add(&seconds, new(big.Int).Div(&nanos, billion)) + nanos.Mod(&nanos, billion) + } + total := new(big.Int).Mul(&seconds, billion) + total.Add(total, &nanos) + if total.BitLen() > 64 { + return 0, errInvalidDuration(value) + } + return total.Uint64(), nil +} + +func errInvalidDuration(value string) error { + return fmt.Errorf("%w: invalid duration %q", ierror.InvalidConnectionString{}, value) } -// splitLeadingNumber consumes the leading decimal digits of s, with an -// optional fractional part as in 0.5s or 1.005s. A leading sign is -// rejected, because negative durations do not parse. The value is -// returned as a number, so a fraction-only amount such as 0.5 still -// counts as positive. -func splitLeadingNumber(s string) (float64, string, bool) { +// cutLeadingAmount consumes the leading amount of s: decimal digits with an +// optional fractional part, as in 5, 0.5 or 1.005. A leading sign is +// rejected, because negative durations do not parse, and so is a fraction +// without a digit after the dot. The fraction comes back as a numerator and +// a power-of-ten scale, so 1.005 is 1005 over 1000 and the caller can +// divide exactly the way the Rust parser does. +func cutLeadingAmount(s string) (whole uint64, fraction uint64, scale uint64, rest string, ok bool) { digits := 0 - for digits < len(s) && s[digits] >= '0' && s[digits] <= '9' { + for digits < len(s) && isDigitByte(s[digits]) { digits++ } - // A fraction only counts when a digit follows the dot, so a bogus - // 1.s does not parse and 5. does not either. - if digits+2 <= len(s) && s[digits] == '.' && s[digits+1] >= '0' && s[digits+1] <= '9' { - digits++ // the dot - for digits < len(s) && s[digits] >= '0' && s[digits] <= '9' { - digits++ - } - } if digits == 0 { - return 0, "", false + return 0, 0, 0, "", false } - n, err := strconv.ParseFloat(s[:digits], 64) + whole, err := strconv.ParseUint(s[:digits], 10, 64) if err != nil { - return 0, "", false + return 0, 0, 0, "", false } - return n, s[digits:], true + rest = s[digits:] + if len(rest) >= 2 && rest[0] == '.' && isDigitByte(rest[1]) { + fractionDigits := 1 + for fractionDigits < len(rest) && isDigitByte(rest[fractionDigits]) { + fractionDigits++ + } + text := rest[1:fractionDigits] + if len(text) > 19 { + // Beyond u64 precision, which a fraction could not carry + // exactly anyway. + return 0, 0, 0, "", false + } + fraction, err = strconv.ParseUint(text, 10, 64) + if err != nil { + return 0, 0, 0, "", false + } + scale = 1 + for range len(text) { + scale *= 10 + } + rest = rest[fractionDigits:] + } + return whole, fraction, scale, rest, true +} + +func isDigitByte(b byte) bool { + return b >= '0' && b <= '9' +} + +// durationUnit is one unit of the duration grammar, with its whole-second +// and sub-second nanosecond weights. A month is 30.44 days and a year is +// 365.25 days, the same weights the Rust and Node tables carry. +type durationUnit struct { + name string + seconds uint64 + nanos uint64 } -// durationUnits are the humantime units, longest spelling first so a -// prefix match cannot cut a longer unit short. -var durationUnits = []string{ - "milliseconds", "millisecond", "msec", "millis", "ms", - "microseconds", "microsecond", "usec", "us", - "nanoseconds", "nanosecond", "nsec", "ns", - "seconds", "second", "secs", "sec", "s", - "minutes", "minute", "mins", "min", "m", - "hours", "hour", "h", - "days", "day", "d", - "weeks", "week", "w", - "years", "year", "y", +// durationUnits are the humantime units, ordered longest spelling first so +// a prefix match cannot cut a longer unit short, which is also why the +// single-letter minute spelling m comes after month and millis. The table +// matches the one the Node and C# SDKs parse: it carries the hr, hrs, wk, +// wks and month spellings, and takes millis but not milliseconds. +var durationUnits = []durationUnit{ + {"seconds", 1, 0}, + {"minutes", 60, 0}, + {"second", 1, 0}, + {"minute", 60, 0}, + {"millis", 0, 1_000_000}, + {"months", 2_630_016, 0}, + {"month", 2_630_016, 0}, + {"nanos", 0, 1}, + {"hours", 3600, 0}, + {"weeks", 604_800, 0}, + {"years", 31_557_600, 0}, + {"nsec", 0, 1}, + {"usec", 0, 1_000}, + {"msec", 0, 1_000_000}, + {"secs", 1, 0}, + {"mins", 60, 0}, + {"hour", 3600, 0}, + {"week", 604_800, 0}, + {"year", 31_557_600, 0}, + {"days", 86_400, 0}, + {"sec", 1, 0}, + {"min", 60, 0}, + {"hrs", 3600, 0}, + {"day", 86_400, 0}, + {"wks", 604_800, 0}, + {"yrs", 31_557_600, 0}, + {"ns", 0, 1}, + {"us", 0, 1_000}, + {"ms", 0, 1_000_000}, + {"hr", 3600, 0}, + {"wk", 604_800, 0}, + {"yr", 31_557_600, 0}, + {"s", 1, 0}, + {"m", 60, 0}, + {"h", 3600, 0}, + {"d", 86_400, 0}, + {"w", 604_800, 0}, + {"y", 31_557_600, 0}, } -// cutDurationUnit consumes the leading duration unit of s, longest -// spelling first so a prefix match cannot cut a longer unit short. The -// unit may run straight into the next amount, as in 1h30m; a leftover -// with no leading unit, such as the x of 5x, still fails the caller. -func cutDurationUnit(s string) (unit string, rest string) { - for _, unit := range durationUnits { - if strings.HasPrefix(s, unit) { - return unit, s[len(unit):] +// cutDurationUnit consumes the leading duration unit of s. The unit may +// run straight into the next amount, as in 1h30m; a leftover with no +// leading unit, such as the x of 5x, still fails the caller. +func cutDurationUnit(s string) (int, string, bool) { + for i, unit := range durationUnits { + if strings.HasPrefix(s, unit.name) { + return i, s[len(unit.name):], true } } - return "", s + return 0, s, false } diff --git a/foreign/go/client/connection_string_test.go b/foreign/go/client/connection_string_test.go index 5e89d535c8..064b22d23c 100644 --- a/foreign/go/client/connection_string_test.go +++ b/foreign/go/client/connection_string_test.go @@ -19,26 +19,23 @@ package client import ( "context" + "crypto/tls" "crypto/ecdsa" "crypto/elliptic" "crypto/rand" - "crypto/tls" "crypto/x509" "crypto/x509/pkix" - "encoding/binary" "encoding/pem" - "io" "math/big" "net" "os" "path/filepath" - "sync" "testing" "time" "github.com/apache/iggy/foreign/go/client/tcp" - iggcon "github.com/apache/iggy/foreign/go/contracts" "github.com/apache/iggy/foreign/go/internal/command" + . "github.com/apache/iggy/foreign/go/internal/fakevsr" "github.com/apache/iggy/foreign/go/internal/vsr" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -67,7 +64,7 @@ func TestParseConnectionString_ParsesTheExplicitTcpSchemeWithAPersonalAccessToke assert.Equal(t, "localhost:8090", client.GetConnectionInfo().ServerAddress) } -func TestParseConnectionString_ParsesIPv6HostWithoutBrackets(t *testing.T) { +func TestParseConnectionString_ParsesBracketedIPv6Host(t *testing.T) { opts, err := ParseConnectionString("iggy://iggy:secret@[::1]:8090") require.NoError(t, err) client := applyOptions(t, opts) @@ -75,183 +72,112 @@ func TestParseConnectionString_ParsesIPv6HostWithoutBrackets(t *testing.T) { assert.Equal(t, "[::1]:8090", client.GetConnectionInfo().ServerAddress) } -// Frame offsets the fake server reads and writes. They mirror the -// unexported tables of the codec package, which this package cannot reach. -const ( - frameOffsetSize = 48 - frameOffsetCommand = 60 - frameOffsetClient = 128 - frameOffsetRequest = 168 - frameOffsetOperation = 176 - frameOffsetSession = 184 - frameOffsetReserved = 196 - - replyFrameOffsetRequest = 200 - replyFrameOffsetOperation = 208 - replyFrameOffsetStatus = 216 -) - -// wireRequest is one frame the fake server read off the wire. -type wireRequest struct { - header [vsr.HeaderSize]byte - payload []byte -} +// TestParseConnectionString_NoDelayOptionLandsOnTheClient pins the nodelay +// rule: the spelling validates and, unlike a dropped validation, the +// parsed value reaches the client, so ?nodelay=false really turns Nagle +// back on and the default stays with Nagle off. +func TestParseConnectionString_NoDelayOptionLandsOnTheClient(t *testing.T) { + withoutOption, err := ParseConnectionString("iggy://iggy:secret@localhost:8090") + require.NoError(t, err) + assert.True(t, applyOptions(t, withoutOption).GetNoDelay(), + "the default must keep Nagle disabled") -func (r wireRequest) operation() vsr.Operation { - return vsr.Operation(r.header[frameOffsetOperation]) -} + enabled, err := ParseConnectionString("iggy://iggy:secret@localhost:8090?nodelay=true") + require.NoError(t, err) + assert.True(t, applyOptions(t, enabled).GetNoDelay()) -func (r wireRequest) code() uint32 { - return binary.LittleEndian.Uint32(r.header[frameOffsetReserved:]) + disabled, err := ParseConnectionString("iggy://iggy:secret@localhost:8090?nodelay=false") + require.NoError(t, err) + assert.False(t, applyOptions(t, disabled).GetNoDelay(), + "?nodelay=false must reach the dial instead of being dropped") } -// readWireRequest reads one complete frame from conn. -func readWireRequest(conn net.Conn) (wireRequest, error) { - var read wireRequest - if _, err := io.ReadFull(conn, read.header[:]); err != nil { - return read, err - } - size := binary.LittleEndian.Uint32(read.header[frameOffsetSize:]) - if size > vsr.HeaderSize { - read.payload = make([]byte, int(size)-vsr.HeaderSize) - if _, err := io.ReadFull(conn, read.payload); err != nil { - return read, err +// TestParseConnectionString_NonZeroDurationOptionsRejectZeroSpellings +// pins the typing the Rust SDK gives each option: +// reconnection_interval and heartbeat_interval are non-zero durations, so +// the zero spellings and any zero-valued duration are refused, while +// reestablish_after is a plain duration where they parse. +func TestParseConnectionString_NonZeroDurationOptionsRejectZeroSpellings(t *testing.T) { + for _, option := range []string{"reconnection_interval", "heartbeat_interval"} { + for _, value := range []string{"0", "none", "disabled", "unlimited", "0s", "0ms"} { + invalid := "iggy://iggy:secret@localhost:8090?" + option + "=" + value + t.Run(invalid, func(t *testing.T) { + opts, err := ParseConnectionString(invalid) + require.Error(t, err, + "%s is a non-zero duration in the Rust SDK", option) + assert.Empty(t, opts) + }) } } - return read, nil -} - -// replyFrameFor builds a reply whose header carries the given operation. -func replyFrameFor(operation vsr.Operation, body []byte) []byte { - frame := make([]byte, vsr.HeaderSize+len(body)) - binary.LittleEndian.PutUint32(frame[frameOffsetSize:], uint32(len(frame))) - frame[frameOffsetCommand] = 8 - frame[replyFrameOffsetOperation] = byte(operation) - binary.LittleEndian.PutUint32(frame[replyFrameOffsetStatus:], 0) - copy(frame[vsr.HeaderSize:], body) - return frame -} - -// resultSection builds the committed result prefix of a metadata reply. -func resultSection(codes ...uint32) []byte { - section := binary.LittleEndian.AppendUint32(nil, uint32(len(codes))) - for index, code := range codes { - section = binary.LittleEndian.AppendUint32(section, uint32(index)) - section = binary.LittleEndian.AppendUint32(section, code) + for _, value := range []string{"0", "none", "disabled", "unlimited", "0s", "0ms"} { + valid := "iggy://iggy:secret@localhost:8090?reestablish_after=" + value + t.Run(valid, func(t *testing.T) { + opts, err := ParseConnectionString(valid) + require.NoError(t, err, + "reestablish_after is a plain duration in the Rust SDK") + assert.NotEmpty(t, opts) + }) } - return section -} - -// registerReply builds the reply of a successful sign-in. -func registerReply(userID uint32, session uint64) []byte { - body := binary.LittleEndian.AppendUint32(resultSection(), userID) - body = binary.LittleEndian.AppendUint64(body, session) - body = binary.LittleEndian.AppendUint32(body, vsr.ProtocolVersion) - body = append(body, byte(len("0.11.0-edge.1"))) - body = append(body, "0.11.0-edge.1"...) - return replyFrameFor(vsr.OperationRegister, body) } -// bindReply builds the reply of a successful session bind. -func bindReply(userID uint32, session uint64) []byte { - registered := registerReply(userID, session) - return replyFrameFor(vsr.OperationNonReplicated, registered[vsr.HeaderSize+len(resultSection()):]) +// TestParseConnectionString_AcceptsTheExtendedDurationUnits pins the +// spellings the Node and C# SDKs parse and the shared grammar carries: +// hr, hrs, wk, wks and month, plus the millis spelling they use instead +// of milliseconds. +func TestParseConnectionString_AcceptsTheExtendedDurationUnits(t *testing.T) { + for _, options := range []string{ + "reconnection_interval=1hr", + "reconnection_interval=2hrs", + "reconnection_interval=1wk", + "reconnection_interval=2wks", + "reconnection_interval=1month", + "reconnection_interval=2months", + "reconnection_interval=1hr 30m", + "reconnection_interval=250millis", + "heartbeat_interval=1hr", + "reestablish_after=1month", + } { + t.Run(options, func(t *testing.T) { + opts, err := ParseConnectionString("iggy://iggy:secret@localhost:8090?" + options) + require.NoError(t, err) + assert.NotEmpty(t, opts) + }) + } + // The long spelling stays outside the grammar, as it is in the Node + // and C# SDKs. + opts, err := ParseConnectionString("iggy://iggy:secret@localhost:8090?reconnection_interval=1milliseconds") + require.Error(t, err) + assert.Empty(t, opts) } -// clusterMetadataReply builds a roster where the node at the listener -// address is the healthy leader. -func clusterMetadataReply(t *testing.T, address string) []byte { - t.Helper() - - metadata := iggcon.ClusterMetadata{Name: "test-cluster"} - host, port, err := net.SplitHostPort(address) - require.NoError(t, err) - tcpPort, err := net.LookupPort("tcp", port) - require.NoError(t, err) - metadata.Nodes = append(metadata.Nodes, iggcon.ClusterNode{ - Name: host, - IP: host, - Role: iggcon.RoleLeader, - Status: iggcon.Healthy, - Endpoints: iggcon.TransportEndpoints{Tcp: uint16(tcpPort)}, +// TestParseConnectionString_RejectsUnsupportedTransports checks the scheme +// rule and pins the credential rule together: unsupported transports are +// rejected with an error naming the transport, and a malformed scheme +// never leaks the credentials embedded in it. +func TestParseConnectionString_RejectsUnsupportedTransportsAndNeverEchoesCredentials(t *testing.T) { + for _, value := range []string{ + "iggy+quic://iggy:secret@localhost:8090", + "iggy+ws://iggy:secret@localhost:8090", + "http://iggy:secret@localhost:8090", + } { + t.Run(value, func(t *testing.T) { + opts, err := ParseConnectionString(value) + require.Error(t, err) + assert.Empty(t, opts) + assert.NotContains(t, err.Error(), "secret", + "the transport error must not echo the connection string") + }) + } + // A scheme that embeds credentials after iggy+ must not surface them + // in the error, only the ASCII-letter transport name is echoed. + leaking := "iggy+user:secret@host:8090://host:8090" + t.Run(leaking, func(t *testing.T) { + opts, err := ParseConnectionString(leaking) + require.Error(t, err) + assert.Empty(t, opts) + assert.NotContains(t, err.Error(), "secret") + assert.NotContains(t, err.Error(), "user:secret@host") }) - - body, err := metadata.MarshalBinary() - require.NoError(t, err) - return replyFrameFor(vsr.OperationNonReplicated, body) -} - -// fakeVSRServer answers frames the way a single-node Iggy server does, so a -// test can drive a full connect-and-login flow without a real server. -type fakeVSRServer struct { - mtx sync.Mutex - requests []wireRequest - listener net.Listener -} - -func listenFakeVSR(t *testing.T, wrap func(net.Conn) net.Conn) *fakeVSRServer { - t.Helper() - - listener, err := net.Listen("tcp", "127.0.0.1:0") - require.NoError(t, err) - t.Cleanup(func() { _ = listener.Close() }) - - server := &fakeVSRServer{listener: listener} - go func() { - for { - conn, err := listener.Accept() - if err != nil { - return - } - if wrap != nil { - conn = wrap(conn) - } - go func(conn net.Conn) { - defer func() { _ = conn.Close() }() - for { - read, err := readWireRequest(conn) - if err != nil { - return - } - server.mtx.Lock() - server.requests = append(server.requests, read) - server.mtx.Unlock() - - var answer []byte - switch { - case read.code() == uint32(command.BindSessionCode): - answer = bindReply(7, read.sessionID()) - case read.code() == uint32(command.GetClusterMetadataCode): - answer = clusterMetadataReply(t, server.listener.Addr().String()) - case read.operation() == vsr.OperationRegister: - answer = registerReply(7, 128) - default: - answer = replyFrameFor(vsr.OperationNonReplicated, nil) - } - // Echo the request id the way the server correlates replies. - if len(answer) >= vsr.HeaderSize && answer[frameOffsetCommand] == 8 { - copy(answer[replyFrameOffsetRequest:replyFrameOffsetRequest+8], - read.header[frameOffsetRequest:frameOffsetRequest+8]) - } - if _, err := conn.Write(answer); err != nil { - return - } - } - }(conn) - } - }() - return server -} - -func (s *fakeVSRServer) recorded() []wireRequest { - s.mtx.Lock() - defer s.mtx.Unlock() - return append([]wireRequest(nil), s.requests...) -} - -// sessionID reads the logical session the request carries. -func (r wireRequest) sessionID() uint64 { - return binary.LittleEndian.Uint64(r.header[frameOffsetSession:]) } // TestParseConnectionString_ConnectsAndLogsInAgainstAServer drives the @@ -259,8 +185,8 @@ func (r wireRequest) sessionID() uint64 { // from ParseConnectionString, wired into NewIggyClient via WithTcp, // connecting and signing in against a running (fake) server. func TestParseConnectionString_ConnectsAndLogsInAgainstAServer(t *testing.T) { - server := listenFakeVSR(t, nil) - address := server.listener.Addr().String() + server := Listen(t, nil) + address := server.Listener().Addr().String() opts, err := ParseConnectionString("iggy://iggy:iggy@" + address) require.NoError(t, err) @@ -270,12 +196,12 @@ func TestParseConnectionString_ConnectsAndLogsInAgainstAServer(t *testing.T) { require.NoError(t, cli.Connect(context.Background())) - recorded := server.recorded() + recorded := server.Recorded() require.Len(t, recorded, 2) // The roster read is auth-gated, so the sign-in comes first. - assert.Equal(t, vsr.OperationRegister, recorded[0].operation()) - assert.Contains(t, string(recorded[0].payload), "iggy") - assert.Equal(t, uint32(command.GetClusterMetadataCode), recorded[1].code()) + assert.Equal(t, vsr.OperationRegister, recorded[0].Operation()) + assert.Contains(t, string(recorded[0].Payload), "iggy") + assert.Equal(t, uint32(command.GetClusterMetadataCode), recorded[1].Code()) } // TestParseConnectionString_TLSEnabledByQueryString drives the same flow @@ -285,10 +211,10 @@ func TestParseConnectionString_ConnectsAndLogsInAgainstAServer(t *testing.T) { func TestParseConnectionString_TLSEnabledByQueryString(t *testing.T) { certificate, caPath := selfSignedCert(t) - server := listenFakeVSR(t, func(conn net.Conn) net.Conn { + server := Listen(t, func(conn net.Conn) net.Conn { return tls.Server(conn, &tls.Config{Certificates: []tls.Certificate{certificate}}) }) - address := server.listener.Addr().String() + address := server.Listener().Addr().String() host, port, err := net.SplitHostPort(address) require.NoError(t, err) @@ -302,10 +228,10 @@ func TestParseConnectionString_TLSEnabledByQueryString(t *testing.T) { require.NoError(t, cli.Connect(context.Background())) - recorded := server.recorded() + recorded := server.Recorded() require.Len(t, recorded, 2) - assert.Equal(t, vsr.OperationRegister, recorded[0].operation()) - assert.Equal(t, uint32(command.GetClusterMetadataCode), recorded[1].code()) + assert.Equal(t, vsr.OperationRegister, recorded[0].Operation()) + assert.Equal(t, uint32(command.GetClusterMetadataCode), recorded[1].Code()) } // selfSignedCert mints a throwaway server certificate for the loopback @@ -343,7 +269,9 @@ func selfSignedCert(t *testing.T) (tls.Certificate, string) { } // TestParseConnectionString_RejectsMalformedConnectionStrings pins every -// malformed shape the other SDKs reject, copied from their test tables. +// malformed shape the Node SDK rejects, copied from its test table. The Rust +// SDK is laxer about some of these shapes, so the Node table is the shared +// floor. func TestParseConnectionString_RejectsMalformedConnectionStrings(t *testing.T) { for _, value := range []string{ "", @@ -430,22 +358,6 @@ func TestParseConnectionString_AcceptsUnlimitedRetries(t *testing.T) { } } -// TestParseConnectionString_RejectsUnsupportedTransports pins the scheme -// rule: the default scheme and the explicit tcp spelling parse, other -// iggy+ spellings are rejected with an error naming the -// transport. -func TestParseConnectionString_RejectsUnsupportedTransports(t *testing.T) { - for _, value := range []string{ - "iggy+quic://iggy:secret@localhost:8090", - "iggy+ws://iggy:secret@localhost:8090", - "http://iggy:secret@localhost:8090", - } { - opts, err := ParseConnectionString(value) - require.Error(t, err) - assert.Empty(t, opts) - } -} - // TestParseConnectionString_ErrorsNeverLeakTheSecret checks that no error // message contains the password or the token from the rejected string. func TestParseConnectionString_ErrorsNeverLeakTheSecret(t *testing.T) { diff --git a/foreign/go/client/tcp/tcp_connect_test.go b/foreign/go/client/tcp/tcp_connect_test.go index 7176cfcb1a..1201334c41 100644 --- a/foreign/go/client/tcp/tcp_connect_test.go +++ b/foreign/go/client/tcp/tcp_connect_test.go @@ -18,6 +18,7 @@ package tcp import ( + . "github.com/apache/iggy/foreign/go/internal/fakevsr" "context" "crypto/ecdsa" "crypto/elliptic" @@ -35,7 +36,6 @@ import ( "sync/atomic" "testing" "time" - iggcon "github.com/apache/iggy/foreign/go/contracts" ierror "github.com/apache/iggy/foreign/go/errors" "github.com/apache/iggy/foreign/go/internal/command" @@ -83,12 +83,12 @@ func selfSignedCert(t *testing.T) (tls.Certificate, string) { type testListener struct { listener net.Listener mtx sync.Mutex - requests []request + requests []Request accepted int } func listenVSR(t *testing.T, wrap func(net.Conn) net.Conn, - handler func(connection, index int, read request) []byte, + handler func(connection, index int, read Request) []byte, ) *testListener { t.Helper() @@ -115,7 +115,7 @@ func listenVSR(t *testing.T, wrap func(net.Conn) net.Conn, go func(conn net.Conn, connection int) { defer func() { _ = conn.Close() }() for index := 0; ; index++ { - read, err := readRequest(conn) + read, err := ReadRequest(conn) if err != nil { return } @@ -127,7 +127,7 @@ func listenVSR(t *testing.T, wrap func(net.Conn) net.Conn, if answer == nil { return } - echoReplyRequest(answer, read) + EchoReplyRequest(answer, read) if _, err := conn.Write(answer); err != nil { return } @@ -140,10 +140,10 @@ func listenVSR(t *testing.T, wrap func(net.Conn) net.Conn, func (s *testListener) address() string { return s.listener.Addr().String() } -func (s *testListener) recorded() []request { +func (s *testListener) recorded() []Request { s.mtx.Lock() defer s.mtx.Unlock() - return append([]request(nil), s.requests...) + return append([]Request(nil), s.requests...) } func (s *testListener) connections() int { @@ -166,18 +166,18 @@ func newDialingClient(t *testing.T, address string, options ...Option) *IggyTcpC // singleNodeHandler answers cluster metadata for a one-node roster and signs // in any register it sees, so tests can focus on the flow under test. -func singleNodeHandler(t *testing.T, address func() string) func(int, int, request) []byte { +func singleNodeHandler(t *testing.T, address func() string) func(int, int, Request) []byte { t.Helper() - return func(_, _ int, read request) []byte { + return func(_, _ int, read Request) []byte { switch { - case read.code() == uint32(command.BindSessionCode): - return bindReplyFrame(7, read.sessionID()) - case read.code() == uint32(command.GetClusterMetadataCode): - return clusterMetadataFrame(t, 0, address()) - case read.operation() == vsr.OperationRegister: - return registerReplyFrame(7, 128) + case read.Code() == uint32(command.BindSessionCode): + return BindReplyFrame(7, read.SessionID()) + case read.Code() == uint32(command.GetClusterMetadataCode): + return ClusterMetadataFrame(t, 0, address()) + case read.Operation() == vsr.OperationRegister: + return RegisterReplyFrame(7, 128) default: - return replyFrame(vsr.OperationNonReplicated, nil) + return ReplyFrame(vsr.OperationNonReplicated, nil) } } } @@ -192,15 +192,15 @@ func TestConnect_SignsInAndThenSettlesLeadership(t *testing.T) { recorded := server.recorded() require.Len(t, recorded, 2) - assert.Equal(t, vsr.OperationRegister, recorded[0].operation(), + assert.Equal(t, vsr.OperationRegister, recorded[0].Operation(), "the roster read is auth-gated, so the sign-in comes first") - assert.Zero(t, recorded[0].code(), "only a non-replicated frame carries the code") - assert.Zero(t, recorded[0].requestID(), "a register is always request zero") - assert.Zero(t, recorded[0].sessionID()) + assert.Zero(t, recorded[0].Code(), "only a non-replicated frame carries the code") + assert.Zero(t, recorded[0].RequestID(), "a register is always Request zero") + assert.Zero(t, recorded[0].SessionID()) - assert.Equal(t, uint32(command.GetClusterMetadataCode), recorded[1].code(), + assert.Equal(t, uint32(command.GetClusterMetadataCode), recorded[1].Code(), "leadership is settled after the sign-in") - assert.Equal(t, uint64(128), recorded[1].sessionID(), "the roster read is authenticated") + assert.Equal(t, uint64(128), recorded[1].SessionID(), "the roster read is authenticated") assert.True(t, client.session.Bound()) assert.Equal(t, uint64(128), client.session.SessionID()) @@ -217,9 +217,9 @@ func TestConnect_SignsInWithAPersonalAccessToken(t *testing.T) { recorded := server.recorded() require.Len(t, recorded, 2) - assert.Equal(t, vsr.OperationRegister, recorded[0].operation()) - assert.Contains(t, string(recorded[0].payload), "token-value") - assert.Contains(t, string(recorded[0].payload), vsr.SDKName) + assert.Equal(t, vsr.OperationRegister, recorded[0].Operation()) + assert.Contains(t, string(recorded[0].Payload), "token-value") + assert.Contains(t, string(recorded[0].Payload), vsr.SDKName) } func TestConnect_SendsNothingWithoutAutoLogin(t *testing.T) { @@ -241,16 +241,16 @@ func TestConnect_RedirectsToTheLeaderAfterSigningIn(t *testing.T) { // The follower completes the sign-in (the server forwards the register to // the primary) and only then reports the leader elsewhere. - follower := listenVSR(t, nil, func(_, _ int, read request) []byte { + follower := listenVSR(t, nil, func(_, _ int, read Request) []byte { switch { - case read.code() == uint32(command.BindSessionCode): - return bindReplyFrame(7, read.sessionID()) - case read.code() == uint32(command.GetClusterMetadataCode): - return clusterMetadataFrame(t, 1, "127.0.0.1:1", leader.address()) - case read.operation() == vsr.OperationRegister: - return registerReplyFrame(7, 512) + case read.Code() == uint32(command.BindSessionCode): + return BindReplyFrame(7, read.SessionID()) + case read.Code() == uint32(command.GetClusterMetadataCode): + return ClusterMetadataFrame(t, 1, "127.0.0.1:1", leader.address()) + case read.Operation() == vsr.OperationRegister: + return RegisterReplyFrame(7, 512) default: - return replyFrame(vsr.OperationNonReplicated, nil) + return ReplyFrame(vsr.OperationNonReplicated, nil) } }) @@ -260,14 +260,14 @@ func TestConnect_RedirectsToTheLeaderAfterSigningIn(t *testing.T) { onFollower := follower.recorded() require.Len(t, onFollower, 2, "the follower answered the sign-in and the roster") - assert.Equal(t, vsr.OperationRegister, onFollower[0].operation()) - assert.Equal(t, uint32(command.GetClusterMetadataCode), onFollower[1].code()) + assert.Equal(t, vsr.OperationRegister, onFollower[0].Operation()) + assert.Equal(t, uint32(command.GetClusterMetadataCode), onFollower[1].Code()) assert.Equal(t, leader.address(), client.currentServerAddress) onLeader := leader.recorded() require.Len(t, onLeader, 2, "the sign-in replay is followed by a leader re-check") - assert.Equal(t, uint32(command.BindSessionCode), onLeader[0].code()) - assert.Equal(t, uint32(command.GetClusterMetadataCode), onLeader[1].code()) + assert.Equal(t, uint32(command.BindSessionCode), onLeader[0].Code()) + assert.Equal(t, uint32(command.GetClusterMetadataCode), onLeader[1].Code()) assert.True(t, client.session.Bound()) assert.Equal(t, uint64(512), client.session.SessionID(), "the leader resumes the original session") @@ -278,30 +278,30 @@ func TestConnect_RechecksLeadershipAfterTheRedirectedSignIn(t *testing.T) { leader = listenVSR(t, nil, singleNodeHandler(t, func() string { return leader.address() })) var intermediate *testListener - intermediate = listenVSR(t, nil, func(_, _ int, read request) []byte { + intermediate = listenVSR(t, nil, func(_, _ int, read Request) []byte { switch { - case read.code() == uint32(command.BindSessionCode): - return bindReplyFrame(7, read.sessionID()) - case read.code() == uint32(command.GetClusterMetadataCode): - return clusterMetadataFrame(t, 1, intermediate.address(), leader.address()) - case read.operation() == vsr.OperationRegister: - return registerReplyFrame(7, 256) + case read.Code() == uint32(command.BindSessionCode): + return BindReplyFrame(7, read.SessionID()) + case read.Code() == uint32(command.GetClusterMetadataCode): + return ClusterMetadataFrame(t, 1, intermediate.address(), leader.address()) + case read.Operation() == vsr.OperationRegister: + return RegisterReplyFrame(7, 256) default: - return replyFrame(vsr.OperationNonReplicated, nil) + return ReplyFrame(vsr.OperationNonReplicated, nil) } }) var follower *testListener - follower = listenVSR(t, nil, func(_, _ int, read request) []byte { + follower = listenVSR(t, nil, func(_, _ int, read Request) []byte { switch { - case read.code() == uint32(command.BindSessionCode): - return bindReplyFrame(7, read.sessionID()) - case read.code() == uint32(command.GetClusterMetadataCode): - return clusterMetadataFrame(t, 1, follower.address(), intermediate.address()) - case read.operation() == vsr.OperationRegister: - return registerReplyFrame(7, 512) + case read.Code() == uint32(command.BindSessionCode): + return BindReplyFrame(7, read.SessionID()) + case read.Code() == uint32(command.GetClusterMetadataCode): + return ClusterMetadataFrame(t, 1, follower.address(), intermediate.address()) + case read.Operation() == vsr.OperationRegister: + return RegisterReplyFrame(7, 512) default: - return replyFrame(vsr.OperationNonReplicated, nil) + return ReplyFrame(vsr.OperationNonReplicated, nil) } }) @@ -329,19 +329,19 @@ func TestConnect_KeepsTheConnectionWhenTheRosterHasOneNode(t *testing.T) { func TestExchange_ResumesBeforeReplayingANonReplicatedRequest(t *testing.T) { var server *testListener - server = listenVSR(t, nil, func(connection, index int, read request) []byte { + server = listenVSR(t, nil, func(connection, index int, read Request) []byte { switch { - case read.code() == uint32(command.BindSessionCode): - return bindReplyFrame(7, read.sessionID()) - case read.code() == uint32(command.GetClusterMetadataCode): - return clusterMetadataFrame(t, 0, server.address()) - case read.operation() == vsr.OperationRegister: - return registerReplyFrame(7, uint64(128+connection)) + case read.Code() == uint32(command.BindSessionCode): + return BindReplyFrame(7, read.SessionID()) + case read.Code() == uint32(command.GetClusterMetadataCode): + return ClusterMetadataFrame(t, 0, server.address()) + case read.Operation() == vsr.OperationRegister: + return RegisterReplyFrame(7, uint64(128+connection)) case connection == 0 && index == 2: // Drop the first ping to force the reconnect path. return nil default: - return replyFrame(vsr.OperationNonReplicated, nil) + return ReplyFrame(vsr.OperationNonReplicated, nil) } }) @@ -355,32 +355,32 @@ func TestExchange_ResumesBeforeReplayingANonReplicatedRequest(t *testing.T) { recorded := server.recorded() require.Len(t, recorded, 6) - assert.Equal(t, uint32(command.PingCode), recorded[2].code(), "the dropped attempt") - assert.Equal(t, uint32(command.BindSessionCode), recorded[3].code(), - "the replay resumes before it repeats the request") - assert.Equal(t, uint32(command.GetClusterMetadataCode), recorded[4].code()) - assert.Equal(t, uint32(command.PingCode), recorded[5].code()) + assert.Equal(t, uint32(command.PingCode), recorded[2].Code(), "the dropped attempt") + assert.Equal(t, uint32(command.BindSessionCode), recorded[3].Code(), + "the replay resumes before it repeats the Request") + assert.Equal(t, uint32(command.GetClusterMetadataCode), recorded[4].Code()) + assert.Equal(t, uint32(command.PingCode), recorded[5].Code()) - assert.Equal(t, recorded[2].clientID(), recorded[5].clientID(), + assert.Equal(t, recorded[2].ClientID(), recorded[5].ClientID(), "a replacement connection preserves the client identity") assert.Equal(t, uint64(128), client.session.SessionID()) } func TestExchange_RefusesToReplayAReplicatedRequestWithAnUnknownOutcome(t *testing.T) { var server *testListener - server = listenVSR(t, nil, func(connection, index int, read request) []byte { + server = listenVSR(t, nil, func(connection, index int, read Request) []byte { switch { - case read.code() == uint32(command.BindSessionCode): - return bindReplyFrame(7, read.sessionID()) - case read.code() == uint32(command.GetClusterMetadataCode): - return clusterMetadataFrame(t, 0, server.address()) - case read.operation() == vsr.OperationRegister: - return registerReplyFrame(7, uint64(128+connection)) + case read.Code() == uint32(command.BindSessionCode): + return BindReplyFrame(7, read.SessionID()) + case read.Code() == uint32(command.GetClusterMetadataCode): + return ClusterMetadataFrame(t, 0, server.address()) + case read.Operation() == vsr.OperationRegister: + return RegisterReplyFrame(7, uint64(128+connection)) case connection == 0 && index == 2: // Drop the reply so the stream creation's outcome is unknown. return nil default: - return replyFrame(vsr.OperationCreateStream, resultSection()) + return ReplyFrame(vsr.OperationCreateStream, ResultSection()) } }) @@ -395,37 +395,37 @@ func TestExchange_RefusesToReplayAReplicatedRequestWithAnUnknownOutcome(t *testi assert.Equal(t, 1, server.connections(), "no reconnect was attempted") recorded := server.recorded() require.Len(t, recorded, 3) - assert.Equal(t, vsr.OperationCreateStream, recorded[2].operation(), - "the request reached the wire exactly once") + assert.Equal(t, vsr.OperationCreateStream, recorded[2].Operation(), + "the Request reached the wire exactly once") } func TestExchange_ReconnectsThroughAKnownFollowerAfterTheLeaderDies(t *testing.T) { var leaderFailed atomic.Bool var follower, leader *testListener - follower = listenVSR(t, nil, func(_, _ int, read request) []byte { + follower = listenVSR(t, nil, func(_, _ int, read Request) []byte { switch { - case read.code() == uint32(command.BindSessionCode): - return bindReplyFrame(7, read.sessionID()) - case read.code() == uint32(command.GetClusterMetadataCode): + case read.Code() == uint32(command.BindSessionCode): + return BindReplyFrame(7, read.SessionID()) + case read.Code() == uint32(command.GetClusterMetadataCode): leaderIndex := 1 if leaderFailed.Load() { leaderIndex = 0 } - return clusterMetadataFrame(t, leaderIndex, follower.address(), leader.address()) - case read.operation() == vsr.OperationRegister: - return registerReplyFrame(7, 256) + return ClusterMetadataFrame(t, leaderIndex, follower.address(), leader.address()) + case read.Operation() == vsr.OperationRegister: + return RegisterReplyFrame(7, 256) default: - return replyFrame(vsr.OperationNonReplicated, nil) + return ReplyFrame(vsr.OperationNonReplicated, nil) } }) - leader = listenVSR(t, nil, func(_, _ int, read request) []byte { + leader = listenVSR(t, nil, func(_, _ int, read Request) []byte { switch { - case read.code() == uint32(command.BindSessionCode): - return bindReplyFrame(7, read.sessionID()) - case read.code() == uint32(command.GetClusterMetadataCode): - return clusterMetadataFrame(t, 1, follower.address(), leader.address()) - case read.operation() == vsr.OperationRegister: - return registerReplyFrame(7, 128) + case read.Code() == uint32(command.BindSessionCode): + return BindReplyFrame(7, read.SessionID()) + case read.Code() == uint32(command.GetClusterMetadataCode): + return ClusterMetadataFrame(t, 1, follower.address(), leader.address()) + case read.Operation() == vsr.OperationRegister: + return RegisterReplyFrame(7, 128) default: leaderFailed.Store(true) _ = leader.listener.Close() @@ -444,22 +444,22 @@ func TestExchange_ReconnectsThroughAKnownFollowerAfterTheLeaderDies(t *testing.T assert.True(t, client.session.Bound()) recorded := follower.recorded() require.Len(t, recorded, 5) - assert.Equal(t, vsr.OperationRegister, recorded[0].operation(), + assert.Equal(t, vsr.OperationRegister, recorded[0].Operation(), "the dialed follower completed the initial sign-in") - assert.Equal(t, uint32(command.GetClusterMetadataCode), recorded[1].code()) - assert.Equal(t, uint32(command.BindSessionCode), recorded[2].code(), + assert.Equal(t, uint32(command.GetClusterMetadataCode), recorded[1].Code()) + assert.Equal(t, uint32(command.BindSessionCode), recorded[2].Code(), "the reconnect resumed the retained session on the surviving node") - assert.Equal(t, uint32(command.GetClusterMetadataCode), recorded[3].code()) - assert.Equal(t, uint32(command.PingCode), recorded[4].code()) + assert.Equal(t, uint32(command.GetClusterMetadataCode), recorded[3].Code()) + assert.Equal(t, uint32(command.PingCode), recorded[4].Code()) } func TestConnect_DropsTheConnectionWhenAutomaticSignInFails(t *testing.T) { var server *testListener - server = listenVSR(t, nil, func(_, _ int, read request) []byte { - if read.code() == uint32(command.GetClusterMetadataCode) { - return clusterMetadataFrame(t, 0, server.address()) + server = listenVSR(t, nil, func(_, _ int, read Request) []byte { + if read.Code() == uint32(command.GetClusterMetadataCode) { + return ClusterMetadataFrame(t, 0, server.address()) } - return evictionFrame(vsr.EvictionInvalidCredentials, 0, 0) + return EvictionFrame(vsr.EvictionInvalidCredentials, 0, 0) }) client := newDialingClient(t, server.address(), @@ -475,13 +475,13 @@ func TestExchange_DoesNotPreemptAReplayedSignIn(t *testing.T) { var server *testListener registers := 0 var mtx sync.Mutex - server = listenVSR(t, nil, func(connection, index int, read request) []byte { + server = listenVSR(t, nil, func(connection, index int, read Request) []byte { switch { - case read.code() == uint32(command.BindSessionCode): - return bindReplyFrame(7, read.sessionID()) - case read.code() == uint32(command.GetClusterMetadataCode): - return clusterMetadataFrame(t, 0, server.address()) - case read.operation() == vsr.OperationRegister: + case read.Code() == uint32(command.BindSessionCode): + return BindReplyFrame(7, read.SessionID()) + case read.Code() == uint32(command.GetClusterMetadataCode): + return ClusterMetadataFrame(t, 0, server.address()) + case read.Operation() == vsr.OperationRegister: mtx.Lock() registers++ count := registers @@ -490,9 +490,9 @@ func TestExchange_DoesNotPreemptAReplayedSignIn(t *testing.T) { if connection == 0 && count == 2 { return nil } - return registerReplyFrame(7, uint64(128+connection)) + return RegisterReplyFrame(7, uint64(128+connection)) default: - return replyFrame(vsr.OperationNonReplicated, nil) + return ReplyFrame(vsr.OperationNonReplicated, nil) } }) @@ -506,7 +506,7 @@ func TestExchange_DoesNotPreemptAReplayedSignIn(t *testing.T) { var signIns int for _, recorded := range server.recorded() { - if recorded.operation() == vsr.OperationRegister { + if recorded.Operation() == vsr.OperationRegister { signIns++ } } @@ -520,7 +520,7 @@ func TestExchange_DoesNotPreemptAReplayedSignIn(t *testing.T) { require.NoError(t, client.Connect(context.Background())) signIns = 0 for _, recorded := range server.recorded() { - if recorded.operation() == vsr.OperationRegister { + if recorded.Operation() == vsr.OperationRegister { signIns++ } } @@ -529,11 +529,11 @@ func TestExchange_DoesNotPreemptAReplayedSignIn(t *testing.T) { } func TestExchange_FailsFastWhenAutoLoginIsOff(t *testing.T) { - server := listenVSR(t, nil, func(_, index int, read request) []byte { + server := listenVSR(t, nil, func(_, index int, read Request) []byte { if index == 0 { return nil } - return replyFrame(vsr.OperationNonReplicated, nil) + return ReplyFrame(vsr.OperationNonReplicated, nil) }) client := newDialingClient(t, server.address()) @@ -548,18 +548,18 @@ func TestExchange_FailsFastWhenAutoLoginIsOff(t *testing.T) { func TestLogoutUser_ResetsTheSessionSoTheNextSignInIsANewIdentity(t *testing.T) { var server *testListener - server = listenVSR(t, nil, func(_, _ int, read request) []byte { + server = listenVSR(t, nil, func(_, _ int, read Request) []byte { switch { - case read.code() == uint32(command.BindSessionCode): - return bindReplyFrame(7, read.sessionID()) - case read.code() == uint32(command.GetClusterMetadataCode): - return clusterMetadataFrame(t, 0, server.address()) - case read.operation() == vsr.OperationRegister: - return registerReplyFrame(7, 128) - case read.operation() == vsr.OperationLogout: - return replyFrame(vsr.OperationLogout, nil) + case read.Code() == uint32(command.BindSessionCode): + return BindReplyFrame(7, read.SessionID()) + case read.Code() == uint32(command.GetClusterMetadataCode): + return ClusterMetadataFrame(t, 0, server.address()) + case read.Operation() == vsr.OperationRegister: + return RegisterReplyFrame(7, 128) + case read.Operation() == vsr.OperationLogout: + return ReplyFrame(vsr.OperationLogout, nil) default: - return replyFrame(vsr.OperationNonReplicated, nil) + return ReplyFrame(vsr.OperationNonReplicated, nil) } }) @@ -573,18 +573,18 @@ func TestLogoutUser_ResetsTheSessionSoTheNextSignInIsANewIdentity(t *testing.T) recorded := server.recorded() require.Len(t, recorded, 5) - assert.Equal(t, vsr.OperationLogout, recorded[2].operation(), + assert.Equal(t, vsr.OperationLogout, recorded[2].Operation(), "a re-login logs the live session out first") - assert.Equal(t, vsr.OperationRegister, recorded[3].operation()) - assert.NotEqual(t, first, recorded[3].clientID(), + assert.Equal(t, vsr.OperationRegister, recorded[3].Operation()) + assert.NotEqual(t, first, recorded[3].ClientID(), "the re-login presents a new client identity") assert.True(t, client.session.Bound()) } func TestHandleLeaderRedirection_StopsAtTheRedirectBudget(t *testing.T) { client, serverConn := newPipeClient(t) - serve(serverConn, func(_ int, _ request) []byte { - return clusterMetadataFrame(t, 0, "10.0.0.1:8090", "10.0.0.2:8090") + Serve(serverConn, func(_ int, _ Request) []byte { + return ClusterMetadataFrame(t, 0, "10.0.0.1:8090", "10.0.0.2:8090") }) for range iggcon.MaxLeaderRedirects { client.leaderRedirectionState.IncrementRedirect("10.0.0.1:8090") @@ -649,9 +649,9 @@ func TestConnect_ExchangesOverTLS(t *testing.T) { recorded := server.recorded() require.Len(t, recorded, 3) - assert.Equal(t, vsr.OperationRegister, recorded[0].operation()) - assert.Equal(t, uint32(command.GetClusterMetadataCode), recorded[1].code()) - assert.Equal(t, uint32(command.PingCode), recorded[2].code()) + assert.Equal(t, vsr.OperationRegister, recorded[0].Operation()) + assert.Equal(t, uint32(command.GetClusterMetadataCode), recorded[1].Code()) + assert.Equal(t, uint32(command.PingCode), recorded[2].Code()) assert.True(t, client.session.Bound()) } diff --git a/foreign/go/client/tcp/tcp_consumer_group_management_test.go b/foreign/go/client/tcp/tcp_consumer_group_management_test.go index 525bbfbd51..5201f889db 100644 --- a/foreign/go/client/tcp/tcp_consumer_group_management_test.go +++ b/foreign/go/client/tcp/tcp_consumer_group_management_test.go @@ -18,11 +18,11 @@ package tcp import ( + . "github.com/apache/iggy/foreign/go/internal/fakevsr" "context" "encoding/binary" "strings" "testing" - ierror "github.com/apache/iggy/foreign/go/errors" "github.com/apache/iggy/foreign/go/internal/vsr" "github.com/stretchr/testify/assert" @@ -44,8 +44,8 @@ func TestGetConsumerGroups_DecodesTheRoster(t *testing.T) { body := append( consumerGroupBody(1, 3, 2, "billing"), consumerGroupBody(2, 1, 0, "audit")...) - serve(serverConn, func(_ int, _ request) []byte { - return replyFrame(vsr.OperationNonReplicated, body) + Serve(serverConn, func(_ int, _ Request) []byte { + return ReplyFrame(vsr.OperationNonReplicated, body) }) groups, err := client.GetConsumerGroups(context.Background(), @@ -59,8 +59,8 @@ func TestGetConsumerGroups_DecodesTheRoster(t *testing.T) { func TestGetConsumerGroup_ReportsAMissingGroupAsATypedError(t *testing.T) { client, serverConn := newPipeClient(t) - serve(serverConn, func(_ int, _ request) []byte { - return replyFrame(vsr.OperationNonReplicated, nil) + Serve(serverConn, func(_ int, _ Request) []byte { + return ReplyFrame(vsr.OperationNonReplicated, nil) }) _, err := client.GetConsumerGroup(context.Background(), @@ -70,8 +70,8 @@ func TestGetConsumerGroup_ReportsAMissingGroupAsATypedError(t *testing.T) { func TestCreateConsumerGroup_RejectsAnInvalidNameWithoutWriting(t *testing.T) { client, serverConn := newPipeClient(t) - server := serve(serverConn, func(_ int, _ request) []byte { - return replyFrame(vsr.OperationCreateConsumerGroup, resultSection()) + server := Serve(serverConn, func(_ int, _ Request) []byte { + return ReplyFrame(vsr.OperationCreateConsumerGroup, ResultSection()) }) for _, name := range []string{"", strings.Repeat("x", MaxStringLength+1)} { @@ -79,14 +79,14 @@ func TestCreateConsumerGroup_RejectsAnInvalidNameWithoutWriting(t *testing.T) { numericIdentifier(t, 1), numericIdentifier(t, 2), name) assert.ErrorIs(t, err, ierror.ErrInvalidConsumerGroupName, "name %q", name) } - assert.Empty(t, server.recorded(), "an invalid name never reaches the wire") + assert.Empty(t, server.Recorded(), "an invalid name never reaches the wire") } func TestCreateConsumerGroup_DecodesTheCreatedGroup(t *testing.T) { client, serverConn := newPipeClient(t) - serve(serverConn, func(_ int, _ request) []byte { - return replyFrame(vsr.OperationCreateConsumerGroup, - append(resultSection(), consumerGroupBody(7, 3, 0, "billing")...)) + Serve(serverConn, func(_ int, _ Request) []byte { + return ReplyFrame(vsr.OperationCreateConsumerGroup, + append(ResultSection(), consumerGroupBody(7, 3, 0, "billing")...)) }) group, err := client.CreateConsumerGroup(context.Background(), @@ -98,8 +98,8 @@ func TestCreateConsumerGroup_DecodesTheCreatedGroup(t *testing.T) { func TestDeleteConsumerGroup_MarksTheGroupAsLeft(t *testing.T) { client, serverConn := newPipeClient(t) - serve(serverConn, func(_ int, _ request) []byte { - return replyFrame(vsr.OperationDeleteConsumerGroup, resultSection()) + Serve(serverConn, func(_ int, _ Request) []byte { + return ReplyFrame(vsr.OperationDeleteConsumerGroup, ResultSection()) }) streamId := numericIdentifier(t, 1) @@ -114,8 +114,8 @@ func TestDeleteConsumerGroup_MarksTheGroupAsLeft(t *testing.T) { func TestSyncConsumerGroup_ReportsANonMemberAsATypedError(t *testing.T) { client, serverConn := newPipeClient(t) - serve(serverConn, func(_ int, _ request) []byte { - return replyFrame(vsr.OperationNonReplicated, nil) + Serve(serverConn, func(_ int, _ Request) []byte { + return ReplyFrame(vsr.OperationNonReplicated, nil) }) assignment, err := client.SyncConsumerGroup(context.Background(), @@ -126,8 +126,8 @@ func TestSyncConsumerGroup_ReportsANonMemberAsATypedError(t *testing.T) { func TestSyncConsumerGroup_KeepsAMemberWithoutPartitionsNonNil(t *testing.T) { client, serverConn := newPipeClient(t) - serve(serverConn, func(_ int, _ request) []byte { - return replyFrame(vsr.OperationNonReplicated, assignmentBody(6)) + Serve(serverConn, func(_ int, _ Request) []byte { + return ReplyFrame(vsr.OperationNonReplicated, assignmentBody(6)) }) assignment, err := client.SyncConsumerGroup(context.Background(), diff --git a/foreign/go/client/tcp/tcp_core.go b/foreign/go/client/tcp/tcp_core.go index efb45f556a..22f6ceff2b 100644 --- a/foreign/go/client/tcp/tcp_core.go +++ b/foreign/go/client/tcp/tcp_core.go @@ -1113,6 +1113,15 @@ func (c *IggyTcpClient) GetConnectionInfo() *iggcon.ConnectionInfo { } } +// GetNoDelay reports whether TCP_NODELAY is set on new connections, which +// tests and callers can use to observe the parsed nodelay option before a +// connection exists to read it off. +func (c *IggyTcpClient) GetNoDelay() bool { + c.mtx.Lock() + defer c.mtx.Unlock() + return c.config.noDelay +} + // Connect establishes the TCP connection to the server. // // Single-flighted: one attempt dials, and every caller that arrives while it diff --git a/foreign/go/client/tcp/tcp_core_encode_test.go b/foreign/go/client/tcp/tcp_core_encode_test.go index 3ba3488301..8b9479bf0f 100644 --- a/foreign/go/client/tcp/tcp_core_encode_test.go +++ b/foreign/go/client/tcp/tcp_core_encode_test.go @@ -21,7 +21,6 @@ import ( "bytes" "errors" "testing" - iggcon "github.com/apache/iggy/foreign/go/contracts" "github.com/apache/iggy/foreign/go/internal/command" "github.com/apache/iggy/foreign/go/internal/vsr" @@ -73,7 +72,7 @@ func TestAppendCommandFrame_AppenderPathMatchesTheFallback(t *testing.T) { assert.Equal(t, want[vsr.HeaderSize:], got[vsr.HeaderSize:], "the fast path diverges from the MarshalBinary fallback") assert.Len(t, got, vsr.HeaderSize+len(body), - "the frame is the reserved prologue followed by the payload") + "the frame is the reserved prologue followed by the Payload") } func TestAppendCommandFrame_FallbackPropagatesTheMarshalError(t *testing.T) { @@ -100,7 +99,7 @@ func TestRequestBufPool_AcquireReleaseRoundTrip(t *testing.T) { require.NotNil(t, bp) require.NotNil(t, *bp) assert.GreaterOrEqual(t, cap(*bp), vsr.HeaderSize, - "a fresh buffer already holds the header prologue") + "a fresh buffer already holds the Header prologue") *bp = append(*bp, []byte("hello")...) releaseRequestBuf(bp) diff --git a/foreign/go/client/tcp/tcp_group_polling_test.go b/foreign/go/client/tcp/tcp_group_polling_test.go index a5fe7e11b2..2804fec899 100644 --- a/foreign/go/client/tcp/tcp_group_polling_test.go +++ b/foreign/go/client/tcp/tcp_group_polling_test.go @@ -18,11 +18,11 @@ package tcp import ( + . "github.com/apache/iggy/foreign/go/internal/fakevsr" "context" "encoding/binary" "testing" "time" - iggcon "github.com/apache/iggy/foreign/go/contracts" ierror "github.com/apache/iggy/foreign/go/errors" "github.com/apache/iggy/foreign/go/internal/command" @@ -50,13 +50,13 @@ func emptyBatchBody(partitionId uint32) []byte { return append(binary.LittleEndian.AppendUint32(body, 0), make([]byte, iggcon.PartitionContextSize)...) } -// polledPartition reads the explicit partition id of a recorded poll request +// polledPartition reads the explicit partition id of a recorded poll Request // that was built from numeric identifiers. -func polledPartition(t *testing.T, read request) uint32 { +func polledPartition(t *testing.T, read Request) uint32 { t.Helper() // [kind u8][consumer id 6][stream id 6][topic id 6][has_partition u8]. - require.Equal(t, byte(1), read.payload[19], "the group poll names an explicit partition") - return binary.LittleEndian.Uint32(read.payload[20:24]) + require.Equal(t, byte(1), read.Payload[19], "the group poll names an explicit partition") + return binary.LittleEndian.Uint32(read.Payload[20:24]) } // groupConsumer builds the group consumer and identifiers the tests poll with. @@ -86,12 +86,12 @@ func agedGroupEntries(client *IggyTcpClient) { func TestPollMessages_GroupPollRoundRobinsTheOwnedPartitions(t *testing.T) { client, serverConn := newPipeClient(t) - server := servePartitionOperations(t, serverConn, func(_ int, read request) []byte { - switch read.code() { + server := servePartitionOperations(t, serverConn, func(_ int, read Request) []byte { + switch read.Code() { case uint32(command.SyncGroupCode): - return replyFrame(vsr.OperationNonReplicated, assignmentBody(7, 1, 3)) + return ReplyFrame(vsr.OperationNonReplicated, assignmentBody(7, 1, 3)) default: - return replyFrame(vsr.OperationNonReplicated, emptyBatchBody(polledPartition(t, read))) + return ReplyFrame(vsr.OperationNonReplicated, emptyBatchBody(polledPartition(t, read))) } }) @@ -105,8 +105,8 @@ func TestPollMessages_GroupPollRoundRobinsTheOwnedPartitions(t *testing.T) { assert.Equal(t, []uint32{1, 3, 1}, partitions, "the cursor advances and wraps across polls") syncs := 0 - for _, read := range server.recorded() { - if read.code() == uint32(command.SyncGroupCode) { + for _, read := range server.Recorded() { + if read.Code() == uint32(command.SyncGroupCode) { syncs++ } } @@ -115,11 +115,11 @@ func TestPollMessages_GroupPollRoundRobinsTheOwnedPartitions(t *testing.T) { func TestPollMessages_GroupPollKeepsTheCursorAcrossASameGenerationRefresh(t *testing.T) { client, serverConn := newPipeClient(t) - servePartitionOperations(t, serverConn, func(_ int, read request) []byte { - if read.code() == uint32(command.SyncGroupCode) { - return replyFrame(vsr.OperationNonReplicated, assignmentBody(7, 1, 3)) + servePartitionOperations(t, serverConn, func(_ int, read Request) []byte { + if read.Code() == uint32(command.SyncGroupCode) { + return ReplyFrame(vsr.OperationNonReplicated, assignmentBody(7, 1, 3)) } - return replyFrame(vsr.OperationNonReplicated, emptyBatchBody(polledPartition(t, read))) + return ReplyFrame(vsr.OperationNonReplicated, emptyBatchBody(polledPartition(t, read))) }) first, err := pollOnce(t, client) @@ -139,11 +139,11 @@ func TestPollMessages_GroupPollKeepsTheCursorAcrossASameGenerationRefresh(t *tes func TestPollMessages_GroupPollRestartsTheCursorOnANewGeneration(t *testing.T) { client, serverConn := newPipeClient(t) generation := uint64(7) - servePartitionOperations(t, serverConn, func(_ int, read request) []byte { - if read.code() == uint32(command.SyncGroupCode) { - return replyFrame(vsr.OperationNonReplicated, assignmentBody(generation, 1, 3)) + servePartitionOperations(t, serverConn, func(_ int, read Request) []byte { + if read.Code() == uint32(command.SyncGroupCode) { + return ReplyFrame(vsr.OperationNonReplicated, assignmentBody(generation, 1, 3)) } - return replyFrame(vsr.OperationNonReplicated, emptyBatchBody(polledPartition(t, read))) + return ReplyFrame(vsr.OperationNonReplicated, emptyBatchBody(polledPartition(t, read))) }) first, err := pollOnce(t, client) @@ -162,18 +162,18 @@ func TestPollMessages_GroupPollRestartsTheCursorOnANewGeneration(t *testing.T) { func TestPollMessages_GroupPollJoinsTheGroupOnTheFirstPoll(t *testing.T) { client, serverConn := newPipeClient(t) joined := false - server := servePartitionOperations(t, serverConn, func(_ int, read request) []byte { + server := servePartitionOperations(t, serverConn, func(_ int, read Request) []byte { switch { - case read.code() == uint32(command.SyncGroupCode) && !joined: + case read.Code() == uint32(command.SyncGroupCode) && !joined: // An empty body means the client is not a member yet. - return replyFrame(vsr.OperationNonReplicated, nil) - case read.code() == uint32(command.SyncGroupCode): - return replyFrame(vsr.OperationNonReplicated, assignmentBody(1, 0)) - case read.operation() == vsr.OperationJoinConsumerGroup: + return ReplyFrame(vsr.OperationNonReplicated, nil) + case read.Code() == uint32(command.SyncGroupCode): + return ReplyFrame(vsr.OperationNonReplicated, assignmentBody(1, 0)) + case read.Operation() == vsr.OperationJoinConsumerGroup: joined = true - return replyFrame(vsr.OperationJoinConsumerGroup, resultSection()) + return ReplyFrame(vsr.OperationJoinConsumerGroup, ResultSection()) default: - return replyFrame(vsr.OperationNonReplicated, emptyBatchBody(polledPartition(t, read))) + return ReplyFrame(vsr.OperationNonReplicated, emptyBatchBody(polledPartition(t, read))) } }) @@ -182,8 +182,8 @@ func TestPollMessages_GroupPollJoinsTheGroupOnTheFirstPoll(t *testing.T) { assert.Equal(t, uint32(0), polled.PartitionId) operations := make([]vsr.Operation, 0) - for _, read := range server.recorded() { - operations = append(operations, read.operation()) + for _, read := range server.Recorded() { + operations = append(operations, read.Operation()) } assert.Contains(t, operations, vsr.OperationJoinConsumerGroup, "the first poll joins on the caller's behalf") @@ -191,14 +191,14 @@ func TestPollMessages_GroupPollJoinsTheGroupOnTheFirstPoll(t *testing.T) { func TestPollMessages_GroupPollDoesNotRejoinALeftGroup(t *testing.T) { client, serverConn := newPipeClient(t) - server := servePartitionOperations(t, serverConn, func(_ int, read request) []byte { + server := servePartitionOperations(t, serverConn, func(_ int, read Request) []byte { switch { - case read.code() == uint32(command.SyncGroupCode): - return replyFrame(vsr.OperationNonReplicated, nil) - case read.operation() == vsr.OperationLeaveConsumerGroup: - return replyFrame(vsr.OperationLeaveConsumerGroup, resultSection()) + case read.Code() == uint32(command.SyncGroupCode): + return ReplyFrame(vsr.OperationNonReplicated, nil) + case read.Operation() == vsr.OperationLeaveConsumerGroup: + return ReplyFrame(vsr.OperationLeaveConsumerGroup, ResultSection()) default: - return replyFrame(vsr.OperationJoinConsumerGroup, resultSection()) + return ReplyFrame(vsr.OperationJoinConsumerGroup, ResultSection()) } }) @@ -210,8 +210,8 @@ func TestPollMessages_GroupPollDoesNotRejoinALeftGroup(t *testing.T) { assert.ErrorIs(t, err, ierror.ErrConsumerGroupMemberNotFound, "an explicit leave stays left until an explicit join") - for _, read := range server.recorded() { - assert.NotEqual(t, vsr.OperationJoinConsumerGroup, read.operation(), + for _, read := range server.Recorded() { + assert.NotEqual(t, vsr.OperationJoinConsumerGroup, read.Operation(), "the poll must not undo the leave") } } @@ -219,20 +219,20 @@ func TestPollMessages_GroupPollDoesNotRejoinALeftGroup(t *testing.T) { func TestPollMessages_GroupPollRejoinsAfterAnExplicitJoin(t *testing.T) { client, serverConn := newPipeClient(t) member := false - servePartitionOperations(t, serverConn, func(_ int, read request) []byte { + servePartitionOperations(t, serverConn, func(_ int, read Request) []byte { switch { - case read.code() == uint32(command.SyncGroupCode) && member: - return replyFrame(vsr.OperationNonReplicated, assignmentBody(2, 4)) - case read.code() == uint32(command.SyncGroupCode): - return replyFrame(vsr.OperationNonReplicated, nil) - case read.operation() == vsr.OperationLeaveConsumerGroup: + case read.Code() == uint32(command.SyncGroupCode) && member: + return ReplyFrame(vsr.OperationNonReplicated, assignmentBody(2, 4)) + case read.Code() == uint32(command.SyncGroupCode): + return ReplyFrame(vsr.OperationNonReplicated, nil) + case read.Operation() == vsr.OperationLeaveConsumerGroup: member = false - return replyFrame(vsr.OperationLeaveConsumerGroup, resultSection()) - case read.operation() == vsr.OperationJoinConsumerGroup: + return ReplyFrame(vsr.OperationLeaveConsumerGroup, ResultSection()) + case read.Operation() == vsr.OperationJoinConsumerGroup: member = true - return replyFrame(vsr.OperationJoinConsumerGroup, resultSection()) + return ReplyFrame(vsr.OperationJoinConsumerGroup, ResultSection()) default: - return replyFrame(vsr.OperationNonReplicated, emptyBatchBody(polledPartition(t, read))) + return ReplyFrame(vsr.OperationNonReplicated, emptyBatchBody(polledPartition(t, read))) } }) @@ -249,11 +249,11 @@ func TestPollMessages_GroupPollRejoinsAfterAnExplicitJoin(t *testing.T) { func TestPollMessages_GroupPollReportsAnEmptyPollWhenTheMemberOwnsNothing(t *testing.T) { client, serverConn := newPipeClient(t) - servePartitionOperations(t, serverConn, func(_ int, read request) []byte { - if read.code() == uint32(command.SyncGroupCode) { - return replyFrame(vsr.OperationNonReplicated, assignmentBody(5)) + servePartitionOperations(t, serverConn, func(_ int, read Request) []byte { + if read.Code() == uint32(command.SyncGroupCode) { + return ReplyFrame(vsr.OperationNonReplicated, assignmentBody(5)) } - t.Errorf("unexpected request with code %d", read.code()) + t.Errorf("unexpected Request with code %d", read.Code()) return nil }) @@ -267,11 +267,11 @@ func TestPollMessages_GroupPollResyncsAnEmptyAssignmentOnTheNextPoll(t *testing. client, serverConn := newPipeClient(t) // The join committed, but no partition installed the member as owner yet. assignment := assignmentBody(1) - server := servePartitionOperations(t, serverConn, func(_ int, read request) []byte { - if read.code() == uint32(command.SyncGroupCode) { - return replyFrame(vsr.OperationNonReplicated, assignment) + server := servePartitionOperations(t, serverConn, func(_ int, read Request) []byte { + if read.Code() == uint32(command.SyncGroupCode) { + return ReplyFrame(vsr.OperationNonReplicated, assignment) } - return replyFrame(vsr.OperationNonReplicated, emptyBatchBody(polledPartition(t, read))) + return ReplyFrame(vsr.OperationNonReplicated, emptyBatchBody(polledPartition(t, read))) }) first, err := pollOnce(t, client) @@ -285,8 +285,8 @@ func TestPollMessages_GroupPollResyncsAnEmptyAssignmentOnTheNextPoll(t *testing. assert.Equal(t, uint32(7), second.PartitionId, "an empty assignment is not trusted for the refresh window") syncs := 0 - for _, read := range server.recorded() { - if read.code() == uint32(command.SyncGroupCode) { + for _, read := range server.Recorded() { + if read.Code() == uint32(command.SyncGroupCode) { syncs++ } } @@ -295,12 +295,12 @@ func TestPollMessages_GroupPollResyncsAnEmptyAssignmentOnTheNextPoll(t *testing. func TestPollMessages_GroupPollReportsAnEmptyPollWhenTheRebalanceOutlastsTheAttempts(t *testing.T) { client, serverConn := newPipeClient(t) - server := servePartitionOperations(t, serverConn, func(_ int, read request) []byte { - if read.code() == uint32(command.SyncGroupCode) { - return replyFrame(vsr.OperationNonReplicated, assignmentBody(9, 0)) + server := servePartitionOperations(t, serverConn, func(_ int, read Request) []byte { + if read.Code() == uint32(command.SyncGroupCode) { + return ReplyFrame(vsr.OperationNonReplicated, assignmentBody(9, 0)) } // The server marks a stale assignment with the resync sentinel. - return replyFrame(vsr.OperationNonReplicated, + return ReplyFrame(vsr.OperationNonReplicated, emptyBatchBody(iggcon.ResyncRequiredPartition)) }) @@ -310,8 +310,8 @@ func TestPollMessages_GroupPollReportsAnEmptyPollWhenTheRebalanceOutlastsTheAtte assert.Empty(t, polled.Messages) polls := 0 - for _, read := range server.recorded() { - if read.code() == uint32(command.PollMessagesCode) { + for _, read := range server.Recorded() { + if read.Code() == uint32(command.PollMessagesCode) { polls++ } } @@ -321,17 +321,17 @@ func TestPollMessages_GroupPollReportsAnEmptyPollWhenTheRebalanceOutlastsTheAtte func TestPollMessages_GroupPollResyncsAfterAFencedPoll(t *testing.T) { client, serverConn := newPipeClient(t) generation := uint64(1) - servePartitionOperations(t, serverConn, func(_ int, read request) []byte { + servePartitionOperations(t, serverConn, func(_ int, read Request) []byte { switch { - case read.code() == uint32(command.SyncGroupCode): - return replyFrame(vsr.OperationNonReplicated, assignmentBody(generation, 1)) + case read.Code() == uint32(command.SyncGroupCode): + return ReplyFrame(vsr.OperationNonReplicated, assignmentBody(generation, 1)) case generation == 1: // The member no longer owns the partition at this generation. generation = 2 - return statusReplyFrame(vsr.OperationNonReplicated, + return StatusReplyFrame(vsr.OperationNonReplicated, uint32(ierror.ConsumerGroupPartitionNotOwnedCode), nil) default: - return replyFrame(vsr.OperationNonReplicated, emptyBatchBody(polledPartition(t, read))) + return ReplyFrame(vsr.OperationNonReplicated, emptyBatchBody(polledPartition(t, read))) } }) diff --git a/foreign/go/client/tcp/tcp_session_credentials_test.go b/foreign/go/client/tcp/tcp_session_credentials_test.go index c219d4f6a3..c1d3cea203 100644 --- a/foreign/go/client/tcp/tcp_session_credentials_test.go +++ b/foreign/go/client/tcp/tcp_session_credentials_test.go @@ -19,7 +19,6 @@ package tcp import ( "testing" - "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) diff --git a/foreign/go/client/tcp/tcp_stream_management_test.go b/foreign/go/client/tcp/tcp_stream_management_test.go index f46033b89d..3a8d1dd793 100644 --- a/foreign/go/client/tcp/tcp_stream_management_test.go +++ b/foreign/go/client/tcp/tcp_stream_management_test.go @@ -18,11 +18,11 @@ package tcp import ( + . "github.com/apache/iggy/foreign/go/internal/fakevsr" "context" "encoding/binary" "strings" "testing" - ierror "github.com/apache/iggy/foreign/go/errors" "github.com/apache/iggy/foreign/go/internal/vsr" "github.com/stretchr/testify/assert" @@ -45,9 +45,9 @@ func streamDetailsBody(id uint32, name string) []byte { func TestCreateStream_DecodesTheCreatedStream(t *testing.T) { client, serverConn := newPipeClient(t) - serve(serverConn, func(_ int, _ request) []byte { - return replyFrame(vsr.OperationCreateStream, - append(resultSection(), streamDetailsBody(9, "orders")...)) + Serve(serverConn, func(_ int, _ Request) []byte { + return ReplyFrame(vsr.OperationCreateStream, + append(ResultSection(), streamDetailsBody(9, "orders")...)) }) stream, err := client.CreateStream(context.Background(), "orders") @@ -58,21 +58,21 @@ func TestCreateStream_DecodesTheCreatedStream(t *testing.T) { func TestCreateStream_RejectsAnInvalidNameWithoutWriting(t *testing.T) { client, serverConn := newPipeClient(t) - server := serve(serverConn, func(_ int, _ request) []byte { - return replyFrame(vsr.OperationCreateStream, resultSection()) + server := Serve(serverConn, func(_ int, _ Request) []byte { + return ReplyFrame(vsr.OperationCreateStream, ResultSection()) }) for _, name := range []string{"", strings.Repeat("x", MaxStringLength+1)} { _, err := client.CreateStream(context.Background(), name) assert.ErrorIs(t, err, ierror.ErrInvalidStreamName, "name %q", name) } - assert.Empty(t, server.recorded()) + assert.Empty(t, server.Recorded()) } func TestGetStream_ReportsAMissingStreamAsATypedError(t *testing.T) { client, serverConn := newPipeClient(t) - serve(serverConn, func(_ int, _ request) []byte { - return replyFrame(vsr.OperationNonReplicated, nil) + Serve(serverConn, func(_ int, _ Request) []byte { + return ReplyFrame(vsr.OperationNonReplicated, nil) }) _, err := client.GetStream(context.Background(), numericIdentifier(t, 42)) @@ -82,8 +82,8 @@ func TestGetStream_ReportsAMissingStreamAsATypedError(t *testing.T) { func TestGetStreams_DecodesTheRoster(t *testing.T) { client, serverConn := newPipeClient(t) body := append(streamDetailsBody(1, "orders"), streamDetailsBody(2, "billing")...) - serve(serverConn, func(_ int, _ request) []byte { - return replyFrame(vsr.OperationNonReplicated, body) + Serve(serverConn, func(_ int, _ Request) []byte { + return ReplyFrame(vsr.OperationNonReplicated, body) }) streams, err := client.GetStreams(context.Background()) @@ -95,21 +95,21 @@ func TestGetStreams_DecodesTheRoster(t *testing.T) { func TestUpdateStream_RejectsAnInvalidNameWithoutWriting(t *testing.T) { client, serverConn := newPipeClient(t) - server := serve(serverConn, func(_ int, _ request) []byte { - return replyFrame(vsr.OperationUpdateStream, resultSection()) + server := Serve(serverConn, func(_ int, _ Request) []byte { + return ReplyFrame(vsr.OperationUpdateStream, ResultSection()) }) for _, name := range []string{"", strings.Repeat("x", MaxStringLength+1)} { err := client.UpdateStream(context.Background(), numericIdentifier(t, 1), name) assert.ErrorIs(t, err, ierror.ErrInvalidStreamName, "name %q", name) } - assert.Empty(t, server.recorded()) + assert.Empty(t, server.Recorded()) } func TestUpdateStream_AcceptsACommittedRename(t *testing.T) { client, serverConn := newPipeClient(t) - serve(serverConn, func(_ int, _ request) []byte { - return replyFrame(vsr.OperationUpdateStream, resultSection()) + Serve(serverConn, func(_ int, _ Request) []byte { + return ReplyFrame(vsr.OperationUpdateStream, ResultSection()) }) assert.NoError(t, client.UpdateStream( @@ -118,14 +118,14 @@ func TestUpdateStream_AcceptsACommittedRename(t *testing.T) { func TestCreatePartitions_InvalidatesTheCachedPartitionCount(t *testing.T) { client, serverConn := newPipeClient(t) - serve(serverConn, func(_ int, read request) []byte { - switch read.operation() { + Serve(serverConn, func(_ int, read Request) []byte { + switch read.Operation() { case vsr.OperationCreatePartitions: - return replyFrame(vsr.OperationCreatePartitions, resultSection()) + return ReplyFrame(vsr.OperationCreatePartitions, ResultSection()) case vsr.OperationDeletePartitions: - return replyFrame(vsr.OperationDeletePartitions, resultSection()) + return ReplyFrame(vsr.OperationDeletePartitions, ResultSection()) default: - return replyFrame(vsr.OperationNonReplicated, nil) + return ReplyFrame(vsr.OperationNonReplicated, nil) } }) @@ -146,8 +146,8 @@ func TestCreatePartitions_InvalidatesTheCachedPartitionCount(t *testing.T) { func TestDeleteTopic_DropsTheTopicCacheEntry(t *testing.T) { client, serverConn := newPipeClient(t) - serve(serverConn, func(_ int, _ request) []byte { - return replyFrame(vsr.OperationDeleteTopic, resultSection()) + Serve(serverConn, func(_ int, _ Request) []byte { + return ReplyFrame(vsr.OperationDeleteTopic, ResultSection()) }) streamId := numericIdentifier(t, 1) diff --git a/foreign/go/client/tcp/tcp_topic_management_test.go b/foreign/go/client/tcp/tcp_topic_management_test.go index 93a531c234..6ace21481e 100644 --- a/foreign/go/client/tcp/tcp_topic_management_test.go +++ b/foreign/go/client/tcp/tcp_topic_management_test.go @@ -18,11 +18,11 @@ package tcp import ( + . "github.com/apache/iggy/foreign/go/internal/fakevsr" "context" "encoding/binary" "strings" "testing" - iggcon "github.com/apache/iggy/foreign/go/contracts" ierror "github.com/apache/iggy/foreign/go/errors" "github.com/apache/iggy/foreign/go/internal/vsr" @@ -32,9 +32,9 @@ import ( func TestCreateTopic_DecodesTheCreatedTopic(t *testing.T) { client, serverConn := newPipeClient(t) - serve(serverConn, func(_ int, _ request) []byte { - return replyFrame(vsr.OperationCreateTopic, - append(resultSection(), topicDetailsBody(t, 3)...)) + Serve(serverConn, func(_ int, _ Request) []byte { + return ReplyFrame(vsr.OperationCreateTopic, + append(ResultSection(), topicDetailsBody(t, 3)...)) }) topic, err := client.CreateTopic(context.Background(), @@ -46,8 +46,8 @@ func TestCreateTopic_DecodesTheCreatedTopic(t *testing.T) { func TestCreateTopic_RejectsInvalidArgumentsWithoutWriting(t *testing.T) { client, serverConn := newPipeClient(t) - server := serve(serverConn, func(_ int, _ request) []byte { - return replyFrame(vsr.OperationCreateTopic, resultSection()) + server := Serve(serverConn, func(_ int, _ Request) []byte { + return ReplyFrame(vsr.OperationCreateTopic, ResultSection()) }) streamId := numericIdentifier(t, 1) @@ -71,13 +71,13 @@ func TestCreateTopic_RejectsInvalidArgumentsWithoutWriting(t *testing.T) { assert.ErrorIs(t, err, test.want) }) } - assert.Empty(t, server.recorded()) + assert.Empty(t, server.Recorded()) } func TestUpdateTopic_RejectsInvalidArgumentsWithoutWriting(t *testing.T) { client, serverConn := newPipeClient(t) - server := serve(serverConn, func(_ int, _ request) []byte { - return replyFrame(vsr.OperationUpdateTopic, resultSection()) + server := Serve(serverConn, func(_ int, _ Request) []byte { + return ReplyFrame(vsr.OperationUpdateTopic, ResultSection()) }) streamId := numericIdentifier(t, 1) @@ -86,13 +86,13 @@ func TestUpdateTopic_RejectsInvalidArgumentsWithoutWriting(t *testing.T) { err := client.UpdateTopic(context.Background(), streamId, topicId, "", 0, iggcon.Duration(0), 0) assert.ErrorIs(t, err, ierror.ErrInvalidTopicName) - assert.Empty(t, server.recorded()) + assert.Empty(t, server.Recorded()) } func TestUpdateTopic_AcceptsACommittedUpdate(t *testing.T) { client, serverConn := newPipeClient(t) - serve(serverConn, func(_ int, _ request) []byte { - return replyFrame(vsr.OperationUpdateTopic, resultSection()) + Serve(serverConn, func(_ int, _ Request) []byte { + return ReplyFrame(vsr.OperationUpdateTopic, ResultSection()) }) assert.NoError(t, client.UpdateTopic(context.Background(), @@ -105,8 +105,8 @@ func TestGetTopics_DecodesTheRoster(t *testing.T) { body := binary.LittleEndian.AppendUint32(nil, 2) body = append(body, topicDetailsBody(t, 3)...) body = append(body, topicDetailsBody(t, 5)...) - serve(serverConn, func(_ int, _ request) []byte { - return replyFrame(vsr.OperationNonReplicated, body) + Serve(serverConn, func(_ int, _ Request) []byte { + return ReplyFrame(vsr.OperationNonReplicated, body) }) topics, err := client.GetTopics(context.Background(), numericIdentifier(t, 1)) @@ -118,8 +118,8 @@ func TestGetTopics_DecodesTheRoster(t *testing.T) { func TestGetTopic_ReportsAMissingTopicAsATypedError(t *testing.T) { client, serverConn := newPipeClient(t) - serve(serverConn, func(_ int, _ request) []byte { - return replyFrame(vsr.OperationNonReplicated, nil) + Serve(serverConn, func(_ int, _ Request) []byte { + return ReplyFrame(vsr.OperationNonReplicated, nil) }) _, err := client.GetTopic(context.Background(), diff --git a/foreign/go/internal/fakevsr/fakevsr.go b/foreign/go/internal/fakevsr/fakevsr.go new file mode 100644 index 0000000000..020cde48fe --- /dev/null +++ b/foreign/go/internal/fakevsr/fakevsr.go @@ -0,0 +1,333 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +// Package fakevsr provides the fake VSR server the Go SDK tests drive a +// full connect-and-login flow against, without a real server. Both the +// transport tests and the connection-string tests import it, so the wire +// offsets and the reply builders live in one place instead of drifting +// copies. It is an internal package, so it never reaches SDK users. +package fakevsr + +import ( + "encoding/binary" + "io" + "net" + "sync" + "testing" + + iggcon "github.com/apache/iggy/foreign/go/contracts" + "github.com/apache/iggy/foreign/go/internal/command" + "github.com/apache/iggy/foreign/go/internal/vsr" + "github.com/stretchr/testify/require" +) + +// Frame offsets the fake server reads and writes. They mirror the +// unexported tables of the codec package, which no test can reach. +const ( + FrameOffsetSize = 48 + FrameOffsetCommand = 60 + FrameOffsetClient = 128 + FrameOffsetRequest = 168 + FrameOffsetOperation = 176 + FrameOffsetSession = 184 + FrameOffsetReserved = 196 + + ReplyFrameOffsetRequest = 200 + ReplyFrameOffsetOperation = 208 + ReplyFrameOffsetStatus = 216 +) + +// Request is one frame the fake server read off the wire. +type Request struct { + Header [vsr.HeaderSize]byte + Payload []byte +} + +// Operation reads the request operation code. +func (r Request) Operation() vsr.Operation { + return vsr.Operation(r.Header[FrameOffsetOperation]) +} + +// Code reads the request command code. +func (r Request) Code() uint32 { + return binary.LittleEndian.Uint32(r.Header[FrameOffsetReserved:]) +} + +// RequestID reads the request correlation identifier. +func (r Request) RequestID() uint64 { + return binary.LittleEndian.Uint64(r.Header[FrameOffsetRequest:]) +} + +// SessionID reads the logical session the request carries. +func (r Request) SessionID() uint64 { + return binary.LittleEndian.Uint64(r.Header[FrameOffsetSession:]) +} + +// ReadRequest reads one complete frame from conn. +func ReadRequest(conn net.Conn) (Request, error) { + var read Request + if _, err := io.ReadFull(conn, read.Header[:]); err != nil { + return read, err + } + size := binary.LittleEndian.Uint32(read.Header[FrameOffsetSize:]) + if size > vsr.HeaderSize { + read.Payload = make([]byte, int(size)-vsr.HeaderSize) + if _, err := io.ReadFull(conn, read.Payload); err != nil { + return read, err + } + } + return read, nil +} + +// ReplyFrame builds a committed reply carrying body. +func ReplyFrame(operation vsr.Operation, body []byte) []byte { + return StatusReplyFrame(operation, 0, body) +} + +// StatusReplyFrame builds a reply whose header carries a pre-commit denial. +func StatusReplyFrame(operation vsr.Operation, status uint32, body []byte) []byte { + frame := make([]byte, vsr.HeaderSize+len(body)) + binary.LittleEndian.PutUint32(frame[FrameOffsetSize:], uint32(len(frame))) + frame[FrameOffsetCommand] = 8 + binary.LittleEndian.PutUint32(frame[ReplyFrameOffsetStatus:], status) + frame[ReplyFrameOffsetOperation] = byte(operation) + copy(frame[vsr.HeaderSize:], body) + return frame +} + +// EvictionFrame builds a header-only session-terminal frame. +func EvictionFrame(reason vsr.EvictionReason, version, versionMin uint32) []byte { + frame := make([]byte, vsr.HeaderSize) + binary.LittleEndian.PutUint32(frame[FrameOffsetSize:], vsr.HeaderSize) + frame[FrameOffsetCommand] = 13 + binary.LittleEndian.PutUint32(frame[144:], version) + binary.LittleEndian.PutUint32(frame[148:], versionMin) + frame[255] = byte(reason) + return frame +} + +// ResultSection builds the committed result prefix of a metadata reply. +func ResultSection(codes ...uint32) []byte { + section := binary.LittleEndian.AppendUint32(nil, uint32(len(codes))) + for index, code := range codes { + section = binary.LittleEndian.AppendUint32(section, uint32(index)) + section = binary.LittleEndian.AppendUint32(section, code) + } + return section +} + +// RegisterReplyFrame builds the reply of a successful sign-in. +func RegisterReplyFrame(userID uint32, session uint64) []byte { + body := binary.LittleEndian.AppendUint32(ResultSection(), userID) + body = binary.LittleEndian.AppendUint64(body, session) + body = binary.LittleEndian.AppendUint32(body, vsr.ProtocolVersion) + body = append(body, byte(len("0.11.0-edge.1"))) + body = append(body, "0.11.0-edge.1"...) + return ReplyFrame(vsr.OperationRegister, body) +} + +// BindReplyFrame builds the reply of a successful session bind. +func BindReplyFrame(userID uint32, session uint64) []byte { + registered := RegisterReplyFrame(userID, session) + return ReplyFrame(vsr.OperationNonReplicated, registered[vsr.HeaderSize+len(ResultSection()):]) +} + +// ClusterMetadataFrame builds a cluster roster where the node at +// leaderIndex is the healthy leader. +func ClusterMetadataFrame(t *testing.T, leaderIndex int, addresses ...string) []byte { + t.Helper() + + metadata := iggcon.ClusterMetadata{Name: "test-cluster"} + for index, address := range addresses { + host, port, err := net.SplitHostPort(address) + require.NoError(t, err) + tcpPort, err := net.LookupPort("tcp", port) + require.NoError(t, err) + + role := iggcon.RoleFollower + if index == leaderIndex { + role = iggcon.RoleLeader + } + metadata.Nodes = append(metadata.Nodes, iggcon.ClusterNode{ + Name: host, + IP: host, + Role: role, + Status: iggcon.Healthy, + Endpoints: iggcon.TransportEndpoints{Tcp: uint16(tcpPort)}, + }) + } + + body, err := metadata.MarshalBinary() + require.NoError(t, err) + return ReplyFrame(vsr.OperationNonReplicated, body) +} + +// EchoReplyRequest stamps the request id into a reply frame the way the +// server echoes it, so handlers do not repeat the correlation plumbing. An +// eviction frame passes through untouched. +func EchoReplyRequest(answer []byte, read Request) { + if len(answer) < vsr.HeaderSize || answer[FrameOffsetCommand] != 8 { + return + } + copy(answer[ReplyFrameOffsetRequest:ReplyFrameOffsetRequest+8], + read.Header[FrameOffsetRequest:FrameOffsetRequest+8]) +} + +// Server answers frames from a handler and records what it was asked. +type Server struct { + mtx sync.Mutex + requests []Request + listener net.Listener + done chan struct{} +} + +// Serve reads frames off conn until it closes, answering each with the +// frame the handler returns. A nil answer leaves the request unanswered. +func Serve(conn net.Conn, handler func(index int, read Request) []byte) *Server { + server := &Server{done: make(chan struct{})} + go func() { + defer close(server.done) + for index := 0; ; index++ { + read, err := ReadRequest(conn) + if err != nil { + return + } + server.mtx.Lock() + server.requests = append(server.requests, read) + server.mtx.Unlock() + + answer := handler(index, read) + if answer == nil { + continue + } + EchoReplyRequest(answer, read) + if _, err := conn.Write(answer); err != nil { + return + } + } + }() + return server +} + +// Listen answers frames the way a single-node Iggy server does, so a test +// can drive a full connect-and-login flow without a real server: the +// bind-session and cluster-metadata requests get their canned replies, a +// sign-in gets a successful register, and anything else gets an empty +// non-replicated reply. The wrap hook lets a test put TLS in front of the +// listener. +func Listen(t *testing.T, wrap func(net.Conn) net.Conn) *Server { + t.Helper() + + listener, err := net.Listen("tcp", "127.0.0.1:0") + require.NoError(t, err) + t.Cleanup(func() { _ = listener.Close() }) + + server := &Server{listener: listener, done: make(chan struct{})} + go func() { + defer close(server.done) + for { + conn, err := listener.Accept() + if err != nil { + return + } + if wrap != nil { + conn = wrap(conn) + } + go func(conn net.Conn) { + defer func() { _ = conn.Close() }() + for { + read, err := ReadRequest(conn) + if err != nil { + return + } + server.mtx.Lock() + server.requests = append(server.requests, read) + server.mtx.Unlock() + + var answer []byte + switch { + case read.Code() == uint32(command.BindSessionCode): + answer = BindReplyFrame(7, read.SessionID()) + case read.Code() == uint32(command.GetClusterMetadataCode): + answer = ClusterMetadataFrame(t, 0, server.listener.Addr().String()) + case read.Operation() == vsr.OperationRegister: + answer = RegisterReplyFrame(7, 128) + default: + answer = ReplyFrame(vsr.OperationNonReplicated, nil) + } + EchoReplyRequest(answer, read) + if _, err := conn.Write(answer); err != nil { + return + } + } + }(conn) + } + }() + return server +} + +// FrameOffsetContext is the incarnation-slot offset inside a request +// header, which a few transport tests read directly. +const FrameOffsetContext = 200 + +// PartitionID reads the resolved partition out of a recorded SendMessages +// payload: [metadata length u32][stream id][topic id][partitioning], where +// the identifiers and the partitioning are each +// [kind u8][length u8][value]. The frame carries no routing namespace, so +// the payload is the only place the client partitioning decision is +// observable. +func (r Request) PartitionID(t *testing.T) uint32 { + t.Helper() + + cursor := 4 + for range 2 { + require.Greater(t, len(r.Payload), cursor+1) + cursor += 2 + int(r.Payload[cursor+1]) + } + require.Greater(t, len(r.Payload), cursor+1) + require.Equal(t, byte(iggcon.PartitionIdKind), r.Payload[cursor], + "the client resolves every strategy to an explicit partition") + require.Equal(t, byte(4), r.Payload[cursor+1]) + require.GreaterOrEqual(t, len(r.Payload), cursor+6) + return binary.LittleEndian.Uint32(r.Payload[cursor+2:]) +} + +// ClientID reads the request header client identifier. +func (r Request) ClientID() vsr.ClientID { + return vsr.ClientID{ + Lo: binary.LittleEndian.Uint64(r.Header[FrameOffsetClient:]), + Hi: binary.LittleEndian.Uint64(r.Header[FrameOffsetClient+8:]), + } +} + +// Incarnation reads the request header incarnation. +func (r Request) Incarnation() uint64 { + return binary.LittleEndian.Uint64(r.Header[FrameOffsetContext:]) +} + +// Recorded returns the requests the server read off the wire. +func (s *Server) Recorded() []Request { + s.mtx.Lock() + defer s.mtx.Unlock() + return append([]Request(nil), s.requests...) +} + +// Listener returns the loopback listener, so a test can read the address +// the fake server bound to before the client connects. +func (s *Server) Listener() net.Listener { + return s.listener +} From 9271604df8bc3cef19f5e55bd99c16d6975fe513 Mon Sep 17 00:00:00 2001 From: Manas Choksi Date: Tue, 13 Oct 2026 03:15:37 +0530 Subject: [PATCH 3/7] style(go): gofmt the connection-string change set CI's Format Check on the LF checkout flagged every file my two commits touched: the import blocks were not in gofmt order (the dot-import of internal/fakevsr sat first instead of sorted in, and crypto/tls sorted after crypto/x509 in the new parser test). This applies exactly 'gofmt -w' to the committed content of the fourteen files touched by 04be090 and 57ce6bb - import ordering only, no logic or line-ending changes in the blobs. Verified: gofmt -l foreign/go is empty on every staged blob; go build ./... and go vet ./... clean; go test -race -count=1 ./client/... green (client 3.8s, tcp 29.3s). The internal/vsr protocol-parity tests are untouched by this change and fail identically on pristine 57ce6bb on a CRLF Windows checkout (their Rust-source regexes anchor on comma-EOL), so they are a checkout artifact here, not a regression. Signed-off-by: choksi2212 --- foreign/go/client/connection_string_test.go | 2 +- foreign/go/client/tcp/tcp_connect_test.go | 14 +++++++------- .../tcp/tcp_consumer_group_management_test.go | 6 +++--- foreign/go/client/tcp/tcp_core_encode_test.go | 2 +- foreign/go/client/tcp/tcp_group_polling_test.go | 6 +++--- .../go/client/tcp/tcp_session_credentials_test.go | 2 +- .../go/client/tcp/tcp_stream_management_test.go | 6 +++--- foreign/go/client/tcp/tcp_topic_management_test.go | 6 +++--- 8 files changed, 22 insertions(+), 22 deletions(-) diff --git a/foreign/go/client/connection_string_test.go b/foreign/go/client/connection_string_test.go index 064b22d23c..614692a423 100644 --- a/foreign/go/client/connection_string_test.go +++ b/foreign/go/client/connection_string_test.go @@ -19,10 +19,10 @@ package client import ( "context" - "crypto/tls" "crypto/ecdsa" "crypto/elliptic" "crypto/rand" + "crypto/tls" "crypto/x509" "crypto/x509/pkix" "encoding/pem" diff --git a/foreign/go/client/tcp/tcp_connect_test.go b/foreign/go/client/tcp/tcp_connect_test.go index 1201334c41..10d7397e9f 100644 --- a/foreign/go/client/tcp/tcp_connect_test.go +++ b/foreign/go/client/tcp/tcp_connect_test.go @@ -18,7 +18,6 @@ package tcp import ( - . "github.com/apache/iggy/foreign/go/internal/fakevsr" "context" "crypto/ecdsa" "crypto/elliptic" @@ -27,6 +26,13 @@ import ( "crypto/x509" "crypto/x509/pkix" "encoding/pem" + iggcon "github.com/apache/iggy/foreign/go/contracts" + ierror "github.com/apache/iggy/foreign/go/errors" + "github.com/apache/iggy/foreign/go/internal/command" + . "github.com/apache/iggy/foreign/go/internal/fakevsr" + "github.com/apache/iggy/foreign/go/internal/vsr" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "log/slog" "math/big" "net" @@ -36,12 +42,6 @@ import ( "sync/atomic" "testing" "time" - iggcon "github.com/apache/iggy/foreign/go/contracts" - ierror "github.com/apache/iggy/foreign/go/errors" - "github.com/apache/iggy/foreign/go/internal/command" - "github.com/apache/iggy/foreign/go/internal/vsr" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" ) // selfSignedCert mints a throwaway server certificate for the loopback diff --git a/foreign/go/client/tcp/tcp_consumer_group_management_test.go b/foreign/go/client/tcp/tcp_consumer_group_management_test.go index 5201f889db..435f2ed442 100644 --- a/foreign/go/client/tcp/tcp_consumer_group_management_test.go +++ b/foreign/go/client/tcp/tcp_consumer_group_management_test.go @@ -18,15 +18,15 @@ package tcp import ( - . "github.com/apache/iggy/foreign/go/internal/fakevsr" "context" "encoding/binary" - "strings" - "testing" ierror "github.com/apache/iggy/foreign/go/errors" + . "github.com/apache/iggy/foreign/go/internal/fakevsr" "github.com/apache/iggy/foreign/go/internal/vsr" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "strings" + "testing" ) // consumerGroupBody builds one wire consumer group: diff --git a/foreign/go/client/tcp/tcp_core_encode_test.go b/foreign/go/client/tcp/tcp_core_encode_test.go index 8b9479bf0f..97c6a25c3d 100644 --- a/foreign/go/client/tcp/tcp_core_encode_test.go +++ b/foreign/go/client/tcp/tcp_core_encode_test.go @@ -20,12 +20,12 @@ package tcp import ( "bytes" "errors" - "testing" iggcon "github.com/apache/iggy/foreign/go/contracts" "github.com/apache/iggy/foreign/go/internal/command" "github.com/apache/iggy/foreign/go/internal/vsr" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "testing" ) // marshalOnlyCmd implements command.Command without AppendBinary; it forces diff --git a/foreign/go/client/tcp/tcp_group_polling_test.go b/foreign/go/client/tcp/tcp_group_polling_test.go index 2804fec899..0bb99526ab 100644 --- a/foreign/go/client/tcp/tcp_group_polling_test.go +++ b/foreign/go/client/tcp/tcp_group_polling_test.go @@ -18,17 +18,17 @@ package tcp import ( - . "github.com/apache/iggy/foreign/go/internal/fakevsr" "context" "encoding/binary" - "testing" - "time" iggcon "github.com/apache/iggy/foreign/go/contracts" ierror "github.com/apache/iggy/foreign/go/errors" "github.com/apache/iggy/foreign/go/internal/command" + . "github.com/apache/iggy/foreign/go/internal/fakevsr" "github.com/apache/iggy/foreign/go/internal/vsr" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "testing" + "time" ) // assignmentBody builds the SyncConsumerGroup reply body: diff --git a/foreign/go/client/tcp/tcp_session_credentials_test.go b/foreign/go/client/tcp/tcp_session_credentials_test.go index c1d3cea203..c90b732173 100644 --- a/foreign/go/client/tcp/tcp_session_credentials_test.go +++ b/foreign/go/client/tcp/tcp_session_credentials_test.go @@ -18,9 +18,9 @@ package tcp import ( - "testing" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "testing" ) func newCredentialClient(autoLogin AutoLogin) *IggyTcpClient { diff --git a/foreign/go/client/tcp/tcp_stream_management_test.go b/foreign/go/client/tcp/tcp_stream_management_test.go index 3a8d1dd793..d3b18e5320 100644 --- a/foreign/go/client/tcp/tcp_stream_management_test.go +++ b/foreign/go/client/tcp/tcp_stream_management_test.go @@ -18,15 +18,15 @@ package tcp import ( - . "github.com/apache/iggy/foreign/go/internal/fakevsr" "context" "encoding/binary" - "strings" - "testing" ierror "github.com/apache/iggy/foreign/go/errors" + . "github.com/apache/iggy/foreign/go/internal/fakevsr" "github.com/apache/iggy/foreign/go/internal/vsr" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "strings" + "testing" ) // streamDetailsBody builds a stream reply with no topics: diff --git a/foreign/go/client/tcp/tcp_topic_management_test.go b/foreign/go/client/tcp/tcp_topic_management_test.go index 6ace21481e..64bd93e927 100644 --- a/foreign/go/client/tcp/tcp_topic_management_test.go +++ b/foreign/go/client/tcp/tcp_topic_management_test.go @@ -18,16 +18,16 @@ package tcp import ( - . "github.com/apache/iggy/foreign/go/internal/fakevsr" "context" "encoding/binary" - "strings" - "testing" iggcon "github.com/apache/iggy/foreign/go/contracts" ierror "github.com/apache/iggy/foreign/go/errors" + . "github.com/apache/iggy/foreign/go/internal/fakevsr" "github.com/apache/iggy/foreign/go/internal/vsr" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "strings" + "testing" ) func TestCreateTopic_DecodesTheCreatedTopic(t *testing.T) { From 12a00fc3492af6f5b125d08a5bd117bb0655b4d6 Mon Sep 17 00:00:00 2001 From: Manas Choksi Date: Tue, 13 Oct 2026 17:55:51 +0530 Subject: [PATCH 4/7] fix(go): review round 2 on the connection-string parser - keep TCP_NODELAY on when a query leaves nodelay out, matching the Go client default the other SDKs share - refuse reconnection_interval and heartbeat_interval below one microsecond, the resolution the Rust wire format carries - accept a space between an amount and its unit and the microsecond spelling, as humantime and the other SDK parsers do - drop a nanosecond carry the total already folded away - share the self-signed test certificate through internal/fakevsr Signed-off-by: Manas Choksi --- foreign/go/client/connection_string.go | 48 ++++++------ foreign/go/client/connection_string_test.go | 82 ++++++++------------- foreign/go/client/tcp/tcp_connect_test.go | 45 +---------- foreign/go/client/tcp/tcp_failover_test.go | 10 +-- foreign/go/internal/fakevsr/fakevsr.go | 46 ++++++++++++ 5 files changed, 109 insertions(+), 122 deletions(-) diff --git a/foreign/go/client/connection_string.go b/foreign/go/client/connection_string.go index 99561caba3..1b942179ea 100644 --- a/foreign/go/client/connection_string.go +++ b/foreign/go/client/connection_string.go @@ -66,17 +66,20 @@ const ( // nodelay=true|false // // Durations take the units the other SDKs share, from nanos to years, -// including the hr, hrs, wk, wks and month spellings, with optional -// fractions and parts run together or spaced, as in 1h30m or 1h 30m. +// including the hr, hrs, wk, wks, month and µs spellings, with optional +// fractions and parts run together or spaced, as in 1h30m, 1h 30m or 1 s. // The reconnection_interval and heartbeat_interval spellings must be -// non-zero, because the Rust SDK types them as non-zero durations; -// reestablish_after takes the zero spellings, which Rust accepts there. +// non-zero and no shorter than one microsecond, because the Rust SDK +// types them as non-zero durations whose wire format carries whole +// microseconds; reestablish_after takes the zero spellings, which Rust +// accepts there. // The Go client keeps its own reconnect and heartbeat pacing, so those // three spellings are validated for compatibility and then ignored, and // a value the other SDKs would reject is still rejected here, so one // connection string is either valid everywhere or nowhere. The nodelay // spelling is applied, because the Go client carries a matching -// TCP_NODELAY option. +// TCP_NODELAY option; it defaults to true, the Go client default, so +// only nodelay=false ever turns Nagle back on. func ParseConnectionString(connectionString string) ([]tcp.Option, error) { protocolParts := strings.Split(connectionString, "://") if len(protocolParts) != 2 { @@ -113,6 +116,9 @@ func ParseConnectionString(connectionString string) ([]tcp.Option, error) { return nil, ierror.InvalidConnectionString{} } + // The no-delay default lives here for the no-options path and in + // parseConnectionOptions for queries, so a query that leaves the + // spelling out keeps Nagle off. var tlsConfig parsedTLS tlsConfig.noDelay = true if len(serverAndOptions) == 2 { @@ -192,7 +198,7 @@ type parsedTLS struct { // keeping the tls options and ignoring the reconnect spellings the Go // client does not take from a connection string. func parseConnectionOptions(optionsString string) (parsedTLS, error) { - parsed := parsedTLS{} + parsed := parsedTLS{noDelay: true} if optionsString == "" { return parsed, ierror.InvalidConnectionString{} } @@ -218,9 +224,10 @@ func parseConnectionOptions(optionsString string) (parsedTLS, error) { } } case "reconnection_interval", "heartbeat_interval": - // Rust types both as non-zero durations, so the zero spellings - // and any zero-valued duration are rejected. - if nanos, err := parseIggyDuration(value); err != nil || nanos == 0 { + // Rust types both as NonZeroIggyDuration, which refuses the + // zero spellings and anything shorter than the whole + // microseconds the wire format carries. + if nanos, err := parseIggyDuration(value); err != nil || nanos < 1_000 { return parsed, ierror.InvalidConnectionString{} } case "reestablish_after": @@ -261,8 +268,10 @@ func isASCIILetters(s string) bool { // lowercased, the zero spellings become a zero duration, and everything // else goes through a humantime-style grammar of amounts and units, with // optional fractions and parts run together as in 1h30m or spaced as in -// 1h 30m. Fractions must land on a whole nanosecond, as in the Rust and -// Node parsers, so 1.5us parses while 0.5ns does not. +// 1h 30m. A space is allowed between an amount and its unit as in 1 s, +// and the µs spelling parses like us. Fractions must land on a whole +// nanosecond, as in the Rust and Node parsers, so 1.5us parses while +// 0.5ns does not. func parseIggyDuration(value string) (uint64, error) { lower := strings.ToLower(value) switch lower { @@ -280,7 +289,9 @@ func parseIggyDuration(value string) (uint64, error) { if !ok { return 0, errInvalidDuration(value) } - remainder = rest + // humantime skips whitespace between an amount and its unit, as + // in 1 s, the way spaces between parts are skipped below. + remainder = strings.TrimLeft(rest, " ") index, rest, ok := cutDurationUnit(remainder) if !ok { @@ -326,14 +337,7 @@ func parseIggyDuration(value string) (uint64, error) { } } - // A carry out of the nanoseconds, as the Rust implementation does - // past a full second. - billion := big.NewInt(1_000_000_000) - if nanos.Cmp(billion) >= 0 { - seconds.Add(&seconds, new(big.Int).Div(&nanos, billion)) - nanos.Mod(&nanos, billion) - } - total := new(big.Int).Mul(&seconds, billion) + total := new(big.Int).Mul(&seconds, big.NewInt(1_000_000_000)) total.Add(total, &nanos) if total.BitLen() > 64 { return 0, errInvalidDuration(value) @@ -405,7 +409,8 @@ type durationUnit struct { // a prefix match cannot cut a longer unit short, which is also why the // single-letter minute spelling m comes after month and millis. The table // matches the one the Node and C# SDKs parse: it carries the hr, hrs, wk, -// wks and month spellings, and takes millis but not milliseconds. +// wks and month spellings, the µs spelling beside us, and takes millis +// but not milliseconds. var durationUnits = []durationUnit{ {"seconds", 1, 0}, {"minutes", 60, 0}, @@ -420,6 +425,7 @@ var durationUnits = []durationUnit{ {"years", 31_557_600, 0}, {"nsec", 0, 1}, {"usec", 0, 1_000}, + {"µs", 0, 1_000}, {"msec", 0, 1_000_000}, {"secs", 1, 0}, {"mins", 60, 0}, diff --git a/foreign/go/client/connection_string_test.go b/foreign/go/client/connection_string_test.go index 614692a423..5a349fe5f8 100644 --- a/foreign/go/client/connection_string_test.go +++ b/foreign/go/client/connection_string_test.go @@ -19,19 +19,9 @@ package client import ( "context" - "crypto/ecdsa" - "crypto/elliptic" - "crypto/rand" "crypto/tls" - "crypto/x509" - "crypto/x509/pkix" - "encoding/pem" - "math/big" "net" - "os" - "path/filepath" "testing" - "time" "github.com/apache/iggy/foreign/go/client/tcp" "github.com/apache/iggy/foreign/go/internal/command" @@ -75,13 +65,19 @@ func TestParseConnectionString_ParsesBracketedIPv6Host(t *testing.T) { // TestParseConnectionString_NoDelayOptionLandsOnTheClient pins the nodelay // rule: the spelling validates and, unlike a dropped validation, the // parsed value reaches the client, so ?nodelay=false really turns Nagle -// back on and the default stays with Nagle off. +// back on, while a query that leaves nodelay out keeps the client +// default and stays with Nagle off. func TestParseConnectionString_NoDelayOptionLandsOnTheClient(t *testing.T) { withoutOption, err := ParseConnectionString("iggy://iggy:secret@localhost:8090") require.NoError(t, err) assert.True(t, applyOptions(t, withoutOption).GetNoDelay(), "the default must keep Nagle disabled") + unspecified, err := ParseConnectionString("iggy://iggy:secret@localhost:8090?tls=true") + require.NoError(t, err) + assert.True(t, applyOptions(t, unspecified).GetNoDelay(), + "a query without nodelay must keep Nagle disabled") + enabled, err := ParseConnectionString("iggy://iggy:secret@localhost:8090?nodelay=true") require.NoError(t, err) assert.True(t, applyOptions(t, enabled).GetNoDelay()) @@ -94,12 +90,13 @@ func TestParseConnectionString_NoDelayOptionLandsOnTheClient(t *testing.T) { // TestParseConnectionString_NonZeroDurationOptionsRejectZeroSpellings // pins the typing the Rust SDK gives each option: -// reconnection_interval and heartbeat_interval are non-zero durations, so -// the zero spellings and any zero-valued duration are refused, while -// reestablish_after is a plain duration where they parse. +// reconnection_interval and heartbeat_interval are non-zero durations +// whose wire format carries whole microseconds, so the zero spellings, +// zero-valued durations and anything below one microsecond are refused, +// while reestablish_after is a plain duration where they parse. func TestParseConnectionString_NonZeroDurationOptionsRejectZeroSpellings(t *testing.T) { for _, option := range []string{"reconnection_interval", "heartbeat_interval"} { - for _, value := range []string{"0", "none", "disabled", "unlimited", "0s", "0ms"} { + for _, value := range []string{"0", "none", "disabled", "unlimited", "0s", "0ms", "1ns", "999ns"} { invalid := "iggy://iggy:secret@localhost:8090?" + option + "=" + value t.Run(invalid, func(t *testing.T) { opts, err := ParseConnectionString(invalid) @@ -118,6 +115,17 @@ func TestParseConnectionString_NonZeroDurationOptionsRejectZeroSpellings(t *test assert.NotEmpty(t, opts) }) } + for _, option := range []string{"reconnection_interval", "heartbeat_interval"} { + for _, value := range []string{"1000ns", "1µs", "1us"} { + valid := "iggy://iggy:secret@localhost:8090?" + option + "=" + value + t.Run(valid, func(t *testing.T) { + opts, err := ParseConnectionString(valid) + require.NoError(t, err, + "%s must parse at the one-microsecond floor", option) + assert.NotEmpty(t, opts) + }) + } + } } // TestParseConnectionString_AcceptsTheExtendedDurationUnits pins the @@ -209,7 +217,7 @@ func TestParseConnectionString_ConnectsAndLogsInAgainstAServer(t *testing.T) { // option would send its register frame in plaintext and the TLS handshake // would fail, so a successful connect-and-login proves the option landed. func TestParseConnectionString_TLSEnabledByQueryString(t *testing.T) { - certificate, caPath := selfSignedCert(t) + certificate, caPath := SelfSignedCert(t) server := Listen(t, func(conn net.Conn) net.Conn { return tls.Server(conn, &tls.Config{Certificates: []tls.Certificate{certificate}}) @@ -234,40 +242,6 @@ func TestParseConnectionString_TLSEnabledByQueryString(t *testing.T) { assert.Equal(t, uint32(command.GetClusterMetadataCode), recorded[1].Code()) } -// selfSignedCert mints a throwaway server certificate for the loopback -// listener, so the test carries no expiry or checkout-layout dependency. -func selfSignedCert(t *testing.T) (tls.Certificate, string) { - t.Helper() - - key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) - require.NoError(t, err) - template := x509.Certificate{ - SerialNumber: big.NewInt(1), - Subject: pkix.Name{CommonName: "localhost"}, - NotBefore: time.Now().Add(-time.Hour), - NotAfter: time.Now().Add(time.Hour), - KeyUsage: x509.KeyUsageDigitalSignature | x509.KeyUsageCertSign, - ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth}, - BasicConstraintsValid: true, - IsCA: true, - DNSNames: []string{"localhost"}, - IPAddresses: []net.IP{net.IPv4(127, 0, 0, 1)}, - } - certDER, err := x509.CreateCertificate(rand.Reader, &template, &template, &key.PublicKey, key) - require.NoError(t, err) - keyDER, err := x509.MarshalECPrivateKey(key) - require.NoError(t, err) - - certPEM := pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: certDER}) - keyPEM := pem.EncodeToMemory(&pem.Block{Type: "EC PRIVATE KEY", Bytes: keyDER}) - certificate, err := tls.X509KeyPair(certPEM, keyPEM) - require.NoError(t, err) - - caPath := filepath.Join(t.TempDir(), "ca.pem") - require.NoError(t, os.WriteFile(caPath, certPEM, 0o600)) - return certificate, caPath -} - // TestParseConnectionString_RejectsMalformedConnectionStrings pins every // malformed shape the Node SDK rejects, copied from its test table. The Rust // SDK is laxer about some of these shapes, so the Node table is the shared @@ -319,8 +293,9 @@ func TestParseConnectionString_AcceptsPortZero(t *testing.T) { } // TestParseConnectionString_AcceptsFractionalAndMultiUnitDurations pins -// the duration grammar the other SDKs share: fractional seconds and -// sequences of units, all accepted in connection-string options. +// the duration grammar the other SDKs share: fractional seconds, +// sequences of units, a space between an amount and its unit, and the +// µs spelling, all accepted in connection-string options. func TestParseConnectionString_AcceptsFractionalAndMultiUnitDurations(t *testing.T) { for _, options := range []string{ "reconnection_interval=500ms", @@ -329,6 +304,9 @@ func TestParseConnectionString_AcceptsFractionalAndMultiUnitDurations(t *testing "reconnection_interval=2m", "reconnection_interval=1h 1m 1s", "reconnection_interval=1h30m", + "reconnection_interval=1µs", + "reestablish_after=1 s", + "reestablish_after=1µs", "reconnection_interval=5sec", "reconnection_interval=5msec", "heartbeat_interval=10s", diff --git a/foreign/go/client/tcp/tcp_connect_test.go b/foreign/go/client/tcp/tcp_connect_test.go index 10d7397e9f..dd26b517aa 100644 --- a/foreign/go/client/tcp/tcp_connect_test.go +++ b/foreign/go/client/tcp/tcp_connect_test.go @@ -19,13 +19,7 @@ package tcp import ( "context" - "crypto/ecdsa" - "crypto/elliptic" - "crypto/rand" "crypto/tls" - "crypto/x509" - "crypto/x509/pkix" - "encoding/pem" iggcon "github.com/apache/iggy/foreign/go/contracts" ierror "github.com/apache/iggy/foreign/go/errors" "github.com/apache/iggy/foreign/go/internal/command" @@ -34,50 +28,13 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "log/slog" - "math/big" "net" - "os" - "path/filepath" "sync" "sync/atomic" "testing" "time" ) -// selfSignedCert mints a throwaway server certificate for the loopback -// listener, so the test carries no expiry or checkout-layout dependency. -func selfSignedCert(t *testing.T) (tls.Certificate, string) { - t.Helper() - - key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) - require.NoError(t, err) - template := x509.Certificate{ - SerialNumber: big.NewInt(1), - Subject: pkix.Name{CommonName: "localhost"}, - NotBefore: time.Now().Add(-time.Hour), - NotAfter: time.Now().Add(time.Hour), - KeyUsage: x509.KeyUsageDigitalSignature | x509.KeyUsageCertSign, - ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth}, - BasicConstraintsValid: true, - IsCA: true, - DNSNames: []string{"localhost"}, - IPAddresses: []net.IP{net.IPv4(127, 0, 0, 1)}, - } - certDER, err := x509.CreateCertificate(rand.Reader, &template, &template, &key.PublicKey, key) - require.NoError(t, err) - keyDER, err := x509.MarshalECPrivateKey(key) - require.NoError(t, err) - - certPEM := pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: certDER}) - keyPEM := pem.EncodeToMemory(&pem.Block{Type: "EC PRIVATE KEY", Bytes: keyDER}) - certificate, err := tls.X509KeyPair(certPEM, keyPEM) - require.NoError(t, err) - - caPath := filepath.Join(t.TempDir(), "ca.pem") - require.NoError(t, os.WriteFile(caPath, certPEM, 0o600)) - return certificate, caPath -} - // testListener is a VSR server on a loopback port. The handler is called with // the zero-based index of the accepted connection and of the frame within it. type testListener struct { @@ -628,7 +585,7 @@ func TestConnect_UnwindsWhenEveryReadFailsInsideTheConnectFlow(t *testing.T) { } func TestConnect_ExchangesOverTLS(t *testing.T) { - certificate, caPath := selfSignedCert(t) + certificate, caPath := SelfSignedCert(t) var server *testListener server = listenVSR(t, diff --git a/foreign/go/client/tcp/tcp_failover_test.go b/foreign/go/client/tcp/tcp_failover_test.go index 1f00638da9..5144974c32 100644 --- a/foreign/go/client/tcp/tcp_failover_test.go +++ b/foreign/go/client/tcp/tcp_failover_test.go @@ -646,7 +646,7 @@ func TestFailover_DoesNotSettleOnAnEndpointThatFailedTheHandshake(t *testing.T) // cover fails the handshake -- which is every failover, once the addresses // differ. func TestFailover_UsesTheDialedEndpointAsTheServerName(t *testing.T) { - certificate, caPath := selfSignedCert(t) + certificate, caPath := SelfSignedCert(t) survivor := listenVSR(t, func(conn net.Conn) net.Conn { return tls.Server(conn, &tls.Config{Certificates: []tls.Certificate{certificate}}) @@ -671,7 +671,7 @@ func TestFailover_UsesTheDialedEndpointAsTheServerName(t *testing.T) { // every attempt, so it has to reach the caller instead of being redialed every // interval forever -- which is what the default unlimited retries did with it. func TestFailover_AConfigFaultEndsTheConnectInsteadOfRetryingForever(t *testing.T) { - certificate, _ := selfSignedCert(t) + certificate, _ := SelfSignedCert(t) server := listenVSR(t, func(conn net.Conn) net.Conn { return tls.Server(conn, &tls.Config{Certificates: []tls.Certificate{certificate}}) @@ -680,7 +680,7 @@ func TestFailover_AConfigFaultEndsTheConnectInsteadOfRetryingForever(t *testing. // A CA the server's certificate was not signed by: no retry makes that // certificate acceptable. - _, unrelatedCA := selfSignedCert(t) + _, unrelatedCA := SelfSignedCert(t) client := NewIggyTcpClient(slog.New(slog.DiscardHandler), WithServerAddress(server.address()), WithTLS(WithTLSCAFile(unrelatedCA), WithTLSValidateCertificate(true))) @@ -703,7 +703,7 @@ func TestFailover_AConfigFaultEndsTheConnectInsteadOfRetryingForever(t *testing. // one misconfigured node in the roster costs the client every endpoint behind // it, including the ones that are only down for a moment. func TestFailover_APlaintextEndpointDoesNotEndTheSweep(t *testing.T) { - certificate, _ := selfSignedCert(t) + certificate, _ := SelfSignedCert(t) var accepted atomic.Int32 var survivor *testListener survivor = listenVSR(t, @@ -882,7 +882,7 @@ func TestDisconnect_DoesNotCloseAConnectionItDidNotFailOn(t *testing.T) { // registerMtx, the transaction blocked on the owner -- and callers pass a // context with no deadline, so nothing breaks it. func TestConnect_DoesNotWaitOnAnAttemptThatSignsIn(t *testing.T) { - certificate, _ := selfSignedCert(t) + certificate, _ := SelfSignedCert(t) var survivor *testListener survivor = listenVSR(t, func(conn net.Conn) net.Conn { diff --git a/foreign/go/internal/fakevsr/fakevsr.go b/foreign/go/internal/fakevsr/fakevsr.go index 020cde48fe..10c2010e58 100644 --- a/foreign/go/internal/fakevsr/fakevsr.go +++ b/foreign/go/internal/fakevsr/fakevsr.go @@ -23,11 +23,22 @@ package fakevsr import ( + "crypto/ecdsa" + "crypto/elliptic" + "crypto/rand" + "crypto/tls" + "crypto/x509" + "crypto/x509/pkix" "encoding/binary" + "encoding/pem" "io" + "math/big" "net" + "os" + "path/filepath" "sync" "testing" + "time" iggcon "github.com/apache/iggy/foreign/go/contracts" "github.com/apache/iggy/foreign/go/internal/command" @@ -331,3 +342,38 @@ func (s *Server) Recorded() []Request { func (s *Server) Listener() net.Listener { return s.listener } + +// SelfSignedCert mints a throwaway server certificate for the loopback +// listener, so a TLS test carries no expiry or checkout-layout dependency. +// The connection-string and TCP suites share it. +func SelfSignedCert(t *testing.T) (tls.Certificate, string) { + t.Helper() + + key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + require.NoError(t, err) + template := x509.Certificate{ + SerialNumber: big.NewInt(1), + Subject: pkix.Name{CommonName: "localhost"}, + NotBefore: time.Now().Add(-time.Hour), + NotAfter: time.Now().Add(time.Hour), + KeyUsage: x509.KeyUsageDigitalSignature | x509.KeyUsageCertSign, + ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth}, + BasicConstraintsValid: true, + IsCA: true, + DNSNames: []string{"localhost"}, + IPAddresses: []net.IP{net.IPv4(127, 0, 0, 1)}, + } + certDER, err := x509.CreateCertificate(rand.Reader, &template, &template, &key.PublicKey, key) + require.NoError(t, err) + keyDER, err := x509.MarshalECPrivateKey(key) + require.NoError(t, err) + + certPEM := pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: certDER}) + keyPEM := pem.EncodeToMemory(&pem.Block{Type: "EC PRIVATE KEY", Bytes: keyDER}) + certificate, err := tls.X509KeyPair(certPEM, keyPEM) + require.NoError(t, err) + + caPath := filepath.Join(t.TempDir(), "ca.pem") + require.NoError(t, os.WriteFile(caPath, certPEM, 0o600)) + return certificate, caPath +} From c44ae1c103f44decaf4e33b7cf9daed3217e1647 Mon Sep 17 00:00:00 2001 From: choksi2212 Date: Wed, 14 Oct 2026 22:38:25 +0530 Subject: [PATCH 5/7] fix(go): rebase the connection-string suite onto the multi-leader rework The transport tests moved to their multi-leader request helpers upstream, so the rebase keeps every transport test on those helpers and drops the sweep that pointed them at fakevsr. What stays shared, as the review asked, is the certificate helper: fakevsr.SelfSignedCert now backs selfSignedCert, and the fake roster reply gains the ReplicaID the multi-leader client expects. Signed-off-by: choksi2212 --- foreign/go/client/tcp/tcp_connect_test.go | 285 +++++++++--------- .../tcp/tcp_consumer_group_management_test.go | 38 +-- foreign/go/client/tcp/tcp_core_encode_test.go | 7 +- foreign/go/client/tcp/tcp_failover_test.go | 10 +- .../go/client/tcp/tcp_group_polling_test.go | 140 ++++----- .../tcp/tcp_session_credentials_test.go | 3 +- .../client/tcp/tcp_stream_management_test.go | 50 +-- .../client/tcp/tcp_topic_management_test.go | 36 +-- foreign/go/internal/fakevsr/fakevsr.go | 95 +----- 9 files changed, 296 insertions(+), 368 deletions(-) diff --git a/foreign/go/client/tcp/tcp_connect_test.go b/foreign/go/client/tcp/tcp_connect_test.go index dd26b517aa..5ed35d8262 100644 --- a/foreign/go/client/tcp/tcp_connect_test.go +++ b/foreign/go/client/tcp/tcp_connect_test.go @@ -20,32 +20,41 @@ package tcp import ( "context" "crypto/tls" - iggcon "github.com/apache/iggy/foreign/go/contracts" - ierror "github.com/apache/iggy/foreign/go/errors" - "github.com/apache/iggy/foreign/go/internal/command" - . "github.com/apache/iggy/foreign/go/internal/fakevsr" - "github.com/apache/iggy/foreign/go/internal/vsr" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" "log/slog" "net" "sync" "sync/atomic" "testing" "time" + + iggcon "github.com/apache/iggy/foreign/go/contracts" + ierror "github.com/apache/iggy/foreign/go/errors" + "github.com/apache/iggy/foreign/go/internal/command" + "github.com/apache/iggy/foreign/go/internal/fakevsr" + "github.com/apache/iggy/foreign/go/internal/vsr" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) +// selfSignedCert mints the throwaway loopback certificate through the +// shared fakevsr helper, so the transport suite and the connection-string +// suite cannot drift apart. +func selfSignedCert(t *testing.T) (tls.Certificate, string) { + t.Helper() + return fakevsr.SelfSignedCert(t) +} + // testListener is a VSR server on a loopback port. The handler is called with // the zero-based index of the accepted connection and of the frame within it. type testListener struct { listener net.Listener mtx sync.Mutex - requests []Request + requests []request accepted int } func listenVSR(t *testing.T, wrap func(net.Conn) net.Conn, - handler func(connection, index int, read Request) []byte, + handler func(connection, index int, read request) []byte, ) *testListener { t.Helper() @@ -72,7 +81,7 @@ func listenVSR(t *testing.T, wrap func(net.Conn) net.Conn, go func(conn net.Conn, connection int) { defer func() { _ = conn.Close() }() for index := 0; ; index++ { - read, err := ReadRequest(conn) + read, err := readRequest(conn) if err != nil { return } @@ -84,7 +93,7 @@ func listenVSR(t *testing.T, wrap func(net.Conn) net.Conn, if answer == nil { return } - EchoReplyRequest(answer, read) + echoReplyRequest(answer, read) if _, err := conn.Write(answer); err != nil { return } @@ -97,10 +106,10 @@ func listenVSR(t *testing.T, wrap func(net.Conn) net.Conn, func (s *testListener) address() string { return s.listener.Addr().String() } -func (s *testListener) recorded() []Request { +func (s *testListener) recorded() []request { s.mtx.Lock() defer s.mtx.Unlock() - return append([]Request(nil), s.requests...) + return append([]request(nil), s.requests...) } func (s *testListener) connections() int { @@ -123,18 +132,18 @@ func newDialingClient(t *testing.T, address string, options ...Option) *IggyTcpC // singleNodeHandler answers cluster metadata for a one-node roster and signs // in any register it sees, so tests can focus on the flow under test. -func singleNodeHandler(t *testing.T, address func() string) func(int, int, Request) []byte { +func singleNodeHandler(t *testing.T, address func() string) func(int, int, request) []byte { t.Helper() - return func(_, _ int, read Request) []byte { + return func(_, _ int, read request) []byte { switch { - case read.Code() == uint32(command.BindSessionCode): - return BindReplyFrame(7, read.SessionID()) - case read.Code() == uint32(command.GetClusterMetadataCode): - return ClusterMetadataFrame(t, 0, address()) - case read.Operation() == vsr.OperationRegister: - return RegisterReplyFrame(7, 128) + case read.code() == uint32(command.BindSessionCode): + return bindReplyFrame(7, read.sessionID()) + case read.code() == uint32(command.GetClusterMetadataCode): + return clusterMetadataFrame(t, 0, address()) + case read.operation() == vsr.OperationRegister: + return registerReplyFrame(7, 128) default: - return ReplyFrame(vsr.OperationNonReplicated, nil) + return replyFrame(vsr.OperationNonReplicated, nil) } } } @@ -149,15 +158,15 @@ func TestConnect_SignsInAndThenSettlesLeadership(t *testing.T) { recorded := server.recorded() require.Len(t, recorded, 2) - assert.Equal(t, vsr.OperationRegister, recorded[0].Operation(), + assert.Equal(t, vsr.OperationRegister, recorded[0].operation(), "the roster read is auth-gated, so the sign-in comes first") - assert.Zero(t, recorded[0].Code(), "only a non-replicated frame carries the code") - assert.Zero(t, recorded[0].RequestID(), "a register is always Request zero") - assert.Zero(t, recorded[0].SessionID()) + assert.Zero(t, recorded[0].code(), "only a non-replicated frame carries the code") + assert.Zero(t, recorded[0].requestID(), "a register is always request zero") + assert.Zero(t, recorded[0].sessionID()) - assert.Equal(t, uint32(command.GetClusterMetadataCode), recorded[1].Code(), + assert.Equal(t, uint32(command.GetClusterMetadataCode), recorded[1].code(), "leadership is settled after the sign-in") - assert.Equal(t, uint64(128), recorded[1].SessionID(), "the roster read is authenticated") + assert.Equal(t, uint64(128), recorded[1].sessionID(), "the roster read is authenticated") assert.True(t, client.session.Bound()) assert.Equal(t, uint64(128), client.session.SessionID()) @@ -174,9 +183,9 @@ func TestConnect_SignsInWithAPersonalAccessToken(t *testing.T) { recorded := server.recorded() require.Len(t, recorded, 2) - assert.Equal(t, vsr.OperationRegister, recorded[0].Operation()) - assert.Contains(t, string(recorded[0].Payload), "token-value") - assert.Contains(t, string(recorded[0].Payload), vsr.SDKName) + assert.Equal(t, vsr.OperationRegister, recorded[0].operation()) + assert.Contains(t, string(recorded[0].payload), "token-value") + assert.Contains(t, string(recorded[0].payload), vsr.SDKName) } func TestConnect_SendsNothingWithoutAutoLogin(t *testing.T) { @@ -198,16 +207,16 @@ func TestConnect_RedirectsToTheLeaderAfterSigningIn(t *testing.T) { // The follower completes the sign-in (the server forwards the register to // the primary) and only then reports the leader elsewhere. - follower := listenVSR(t, nil, func(_, _ int, read Request) []byte { + follower := listenVSR(t, nil, func(_, _ int, read request) []byte { switch { - case read.Code() == uint32(command.BindSessionCode): - return BindReplyFrame(7, read.SessionID()) - case read.Code() == uint32(command.GetClusterMetadataCode): - return ClusterMetadataFrame(t, 1, "127.0.0.1:1", leader.address()) - case read.Operation() == vsr.OperationRegister: - return RegisterReplyFrame(7, 512) + case read.code() == uint32(command.BindSessionCode): + return bindReplyFrame(7, read.sessionID()) + case read.code() == uint32(command.GetClusterMetadataCode): + return clusterMetadataFrame(t, 1, "127.0.0.1:1", leader.address()) + case read.operation() == vsr.OperationRegister: + return registerReplyFrame(7, 512) default: - return ReplyFrame(vsr.OperationNonReplicated, nil) + return replyFrame(vsr.OperationNonReplicated, nil) } }) @@ -217,14 +226,14 @@ func TestConnect_RedirectsToTheLeaderAfterSigningIn(t *testing.T) { onFollower := follower.recorded() require.Len(t, onFollower, 2, "the follower answered the sign-in and the roster") - assert.Equal(t, vsr.OperationRegister, onFollower[0].Operation()) - assert.Equal(t, uint32(command.GetClusterMetadataCode), onFollower[1].Code()) + assert.Equal(t, vsr.OperationRegister, onFollower[0].operation()) + assert.Equal(t, uint32(command.GetClusterMetadataCode), onFollower[1].code()) assert.Equal(t, leader.address(), client.currentServerAddress) onLeader := leader.recorded() require.Len(t, onLeader, 2, "the sign-in replay is followed by a leader re-check") - assert.Equal(t, uint32(command.BindSessionCode), onLeader[0].Code()) - assert.Equal(t, uint32(command.GetClusterMetadataCode), onLeader[1].Code()) + assert.Equal(t, uint32(command.BindSessionCode), onLeader[0].code()) + assert.Equal(t, uint32(command.GetClusterMetadataCode), onLeader[1].code()) assert.True(t, client.session.Bound()) assert.Equal(t, uint64(512), client.session.SessionID(), "the leader resumes the original session") @@ -235,30 +244,30 @@ func TestConnect_RechecksLeadershipAfterTheRedirectedSignIn(t *testing.T) { leader = listenVSR(t, nil, singleNodeHandler(t, func() string { return leader.address() })) var intermediate *testListener - intermediate = listenVSR(t, nil, func(_, _ int, read Request) []byte { + intermediate = listenVSR(t, nil, func(_, _ int, read request) []byte { switch { - case read.Code() == uint32(command.BindSessionCode): - return BindReplyFrame(7, read.SessionID()) - case read.Code() == uint32(command.GetClusterMetadataCode): - return ClusterMetadataFrame(t, 1, intermediate.address(), leader.address()) - case read.Operation() == vsr.OperationRegister: - return RegisterReplyFrame(7, 256) + case read.code() == uint32(command.BindSessionCode): + return bindReplyFrame(7, read.sessionID()) + case read.code() == uint32(command.GetClusterMetadataCode): + return clusterMetadataFrame(t, 1, intermediate.address(), leader.address()) + case read.operation() == vsr.OperationRegister: + return registerReplyFrame(7, 256) default: - return ReplyFrame(vsr.OperationNonReplicated, nil) + return replyFrame(vsr.OperationNonReplicated, nil) } }) var follower *testListener - follower = listenVSR(t, nil, func(_, _ int, read Request) []byte { + follower = listenVSR(t, nil, func(_, _ int, read request) []byte { switch { - case read.Code() == uint32(command.BindSessionCode): - return BindReplyFrame(7, read.SessionID()) - case read.Code() == uint32(command.GetClusterMetadataCode): - return ClusterMetadataFrame(t, 1, follower.address(), intermediate.address()) - case read.Operation() == vsr.OperationRegister: - return RegisterReplyFrame(7, 512) + case read.code() == uint32(command.BindSessionCode): + return bindReplyFrame(7, read.sessionID()) + case read.code() == uint32(command.GetClusterMetadataCode): + return clusterMetadataFrame(t, 1, follower.address(), intermediate.address()) + case read.operation() == vsr.OperationRegister: + return registerReplyFrame(7, 512) default: - return ReplyFrame(vsr.OperationNonReplicated, nil) + return replyFrame(vsr.OperationNonReplicated, nil) } }) @@ -286,19 +295,19 @@ func TestConnect_KeepsTheConnectionWhenTheRosterHasOneNode(t *testing.T) { func TestExchange_ResumesBeforeReplayingANonReplicatedRequest(t *testing.T) { var server *testListener - server = listenVSR(t, nil, func(connection, index int, read Request) []byte { + server = listenVSR(t, nil, func(connection, index int, read request) []byte { switch { - case read.Code() == uint32(command.BindSessionCode): - return BindReplyFrame(7, read.SessionID()) - case read.Code() == uint32(command.GetClusterMetadataCode): - return ClusterMetadataFrame(t, 0, server.address()) - case read.Operation() == vsr.OperationRegister: - return RegisterReplyFrame(7, uint64(128+connection)) + case read.code() == uint32(command.BindSessionCode): + return bindReplyFrame(7, read.sessionID()) + case read.code() == uint32(command.GetClusterMetadataCode): + return clusterMetadataFrame(t, 0, server.address()) + case read.operation() == vsr.OperationRegister: + return registerReplyFrame(7, uint64(128+connection)) case connection == 0 && index == 2: // Drop the first ping to force the reconnect path. return nil default: - return ReplyFrame(vsr.OperationNonReplicated, nil) + return replyFrame(vsr.OperationNonReplicated, nil) } }) @@ -312,32 +321,32 @@ func TestExchange_ResumesBeforeReplayingANonReplicatedRequest(t *testing.T) { recorded := server.recorded() require.Len(t, recorded, 6) - assert.Equal(t, uint32(command.PingCode), recorded[2].Code(), "the dropped attempt") - assert.Equal(t, uint32(command.BindSessionCode), recorded[3].Code(), - "the replay resumes before it repeats the Request") - assert.Equal(t, uint32(command.GetClusterMetadataCode), recorded[4].Code()) - assert.Equal(t, uint32(command.PingCode), recorded[5].Code()) + assert.Equal(t, uint32(command.PingCode), recorded[2].code(), "the dropped attempt") + assert.Equal(t, uint32(command.BindSessionCode), recorded[3].code(), + "the replay resumes before it repeats the request") + assert.Equal(t, uint32(command.GetClusterMetadataCode), recorded[4].code()) + assert.Equal(t, uint32(command.PingCode), recorded[5].code()) - assert.Equal(t, recorded[2].ClientID(), recorded[5].ClientID(), + assert.Equal(t, recorded[2].clientID(), recorded[5].clientID(), "a replacement connection preserves the client identity") assert.Equal(t, uint64(128), client.session.SessionID()) } func TestExchange_RefusesToReplayAReplicatedRequestWithAnUnknownOutcome(t *testing.T) { var server *testListener - server = listenVSR(t, nil, func(connection, index int, read Request) []byte { + server = listenVSR(t, nil, func(connection, index int, read request) []byte { switch { - case read.Code() == uint32(command.BindSessionCode): - return BindReplyFrame(7, read.SessionID()) - case read.Code() == uint32(command.GetClusterMetadataCode): - return ClusterMetadataFrame(t, 0, server.address()) - case read.Operation() == vsr.OperationRegister: - return RegisterReplyFrame(7, uint64(128+connection)) + case read.code() == uint32(command.BindSessionCode): + return bindReplyFrame(7, read.sessionID()) + case read.code() == uint32(command.GetClusterMetadataCode): + return clusterMetadataFrame(t, 0, server.address()) + case read.operation() == vsr.OperationRegister: + return registerReplyFrame(7, uint64(128+connection)) case connection == 0 && index == 2: // Drop the reply so the stream creation's outcome is unknown. return nil default: - return ReplyFrame(vsr.OperationCreateStream, ResultSection()) + return replyFrame(vsr.OperationCreateStream, resultSection()) } }) @@ -352,37 +361,37 @@ func TestExchange_RefusesToReplayAReplicatedRequestWithAnUnknownOutcome(t *testi assert.Equal(t, 1, server.connections(), "no reconnect was attempted") recorded := server.recorded() require.Len(t, recorded, 3) - assert.Equal(t, vsr.OperationCreateStream, recorded[2].Operation(), - "the Request reached the wire exactly once") + assert.Equal(t, vsr.OperationCreateStream, recorded[2].operation(), + "the request reached the wire exactly once") } func TestExchange_ReconnectsThroughAKnownFollowerAfterTheLeaderDies(t *testing.T) { var leaderFailed atomic.Bool var follower, leader *testListener - follower = listenVSR(t, nil, func(_, _ int, read Request) []byte { + follower = listenVSR(t, nil, func(_, _ int, read request) []byte { switch { - case read.Code() == uint32(command.BindSessionCode): - return BindReplyFrame(7, read.SessionID()) - case read.Code() == uint32(command.GetClusterMetadataCode): + case read.code() == uint32(command.BindSessionCode): + return bindReplyFrame(7, read.sessionID()) + case read.code() == uint32(command.GetClusterMetadataCode): leaderIndex := 1 if leaderFailed.Load() { leaderIndex = 0 } - return ClusterMetadataFrame(t, leaderIndex, follower.address(), leader.address()) - case read.Operation() == vsr.OperationRegister: - return RegisterReplyFrame(7, 256) + return clusterMetadataFrame(t, leaderIndex, follower.address(), leader.address()) + case read.operation() == vsr.OperationRegister: + return registerReplyFrame(7, 256) default: - return ReplyFrame(vsr.OperationNonReplicated, nil) + return replyFrame(vsr.OperationNonReplicated, nil) } }) - leader = listenVSR(t, nil, func(_, _ int, read Request) []byte { + leader = listenVSR(t, nil, func(_, _ int, read request) []byte { switch { - case read.Code() == uint32(command.BindSessionCode): - return BindReplyFrame(7, read.SessionID()) - case read.Code() == uint32(command.GetClusterMetadataCode): - return ClusterMetadataFrame(t, 1, follower.address(), leader.address()) - case read.Operation() == vsr.OperationRegister: - return RegisterReplyFrame(7, 128) + case read.code() == uint32(command.BindSessionCode): + return bindReplyFrame(7, read.sessionID()) + case read.code() == uint32(command.GetClusterMetadataCode): + return clusterMetadataFrame(t, 1, follower.address(), leader.address()) + case read.operation() == vsr.OperationRegister: + return registerReplyFrame(7, 128) default: leaderFailed.Store(true) _ = leader.listener.Close() @@ -401,22 +410,22 @@ func TestExchange_ReconnectsThroughAKnownFollowerAfterTheLeaderDies(t *testing.T assert.True(t, client.session.Bound()) recorded := follower.recorded() require.Len(t, recorded, 5) - assert.Equal(t, vsr.OperationRegister, recorded[0].Operation(), + assert.Equal(t, vsr.OperationRegister, recorded[0].operation(), "the dialed follower completed the initial sign-in") - assert.Equal(t, uint32(command.GetClusterMetadataCode), recorded[1].Code()) - assert.Equal(t, uint32(command.BindSessionCode), recorded[2].Code(), + assert.Equal(t, uint32(command.GetClusterMetadataCode), recorded[1].code()) + assert.Equal(t, uint32(command.BindSessionCode), recorded[2].code(), "the reconnect resumed the retained session on the surviving node") - assert.Equal(t, uint32(command.GetClusterMetadataCode), recorded[3].Code()) - assert.Equal(t, uint32(command.PingCode), recorded[4].Code()) + assert.Equal(t, uint32(command.GetClusterMetadataCode), recorded[3].code()) + assert.Equal(t, uint32(command.PingCode), recorded[4].code()) } func TestConnect_DropsTheConnectionWhenAutomaticSignInFails(t *testing.T) { var server *testListener - server = listenVSR(t, nil, func(_, _ int, read Request) []byte { - if read.Code() == uint32(command.GetClusterMetadataCode) { - return ClusterMetadataFrame(t, 0, server.address()) + server = listenVSR(t, nil, func(_, _ int, read request) []byte { + if read.code() == uint32(command.GetClusterMetadataCode) { + return clusterMetadataFrame(t, 0, server.address()) } - return EvictionFrame(vsr.EvictionInvalidCredentials, 0, 0) + return evictionFrame(vsr.EvictionInvalidCredentials, 0, 0) }) client := newDialingClient(t, server.address(), @@ -432,13 +441,13 @@ func TestExchange_DoesNotPreemptAReplayedSignIn(t *testing.T) { var server *testListener registers := 0 var mtx sync.Mutex - server = listenVSR(t, nil, func(connection, index int, read Request) []byte { + server = listenVSR(t, nil, func(connection, index int, read request) []byte { switch { - case read.Code() == uint32(command.BindSessionCode): - return BindReplyFrame(7, read.SessionID()) - case read.Code() == uint32(command.GetClusterMetadataCode): - return ClusterMetadataFrame(t, 0, server.address()) - case read.Operation() == vsr.OperationRegister: + case read.code() == uint32(command.BindSessionCode): + return bindReplyFrame(7, read.sessionID()) + case read.code() == uint32(command.GetClusterMetadataCode): + return clusterMetadataFrame(t, 0, server.address()) + case read.operation() == vsr.OperationRegister: mtx.Lock() registers++ count := registers @@ -447,9 +456,9 @@ func TestExchange_DoesNotPreemptAReplayedSignIn(t *testing.T) { if connection == 0 && count == 2 { return nil } - return RegisterReplyFrame(7, uint64(128+connection)) + return registerReplyFrame(7, uint64(128+connection)) default: - return ReplyFrame(vsr.OperationNonReplicated, nil) + return replyFrame(vsr.OperationNonReplicated, nil) } }) @@ -463,7 +472,7 @@ func TestExchange_DoesNotPreemptAReplayedSignIn(t *testing.T) { var signIns int for _, recorded := range server.recorded() { - if recorded.Operation() == vsr.OperationRegister { + if recorded.operation() == vsr.OperationRegister { signIns++ } } @@ -477,7 +486,7 @@ func TestExchange_DoesNotPreemptAReplayedSignIn(t *testing.T) { require.NoError(t, client.Connect(context.Background())) signIns = 0 for _, recorded := range server.recorded() { - if recorded.Operation() == vsr.OperationRegister { + if recorded.operation() == vsr.OperationRegister { signIns++ } } @@ -486,11 +495,11 @@ func TestExchange_DoesNotPreemptAReplayedSignIn(t *testing.T) { } func TestExchange_FailsFastWhenAutoLoginIsOff(t *testing.T) { - server := listenVSR(t, nil, func(_, index int, read Request) []byte { + server := listenVSR(t, nil, func(_, index int, read request) []byte { if index == 0 { return nil } - return ReplyFrame(vsr.OperationNonReplicated, nil) + return replyFrame(vsr.OperationNonReplicated, nil) }) client := newDialingClient(t, server.address()) @@ -505,18 +514,18 @@ func TestExchange_FailsFastWhenAutoLoginIsOff(t *testing.T) { func TestLogoutUser_ResetsTheSessionSoTheNextSignInIsANewIdentity(t *testing.T) { var server *testListener - server = listenVSR(t, nil, func(_, _ int, read Request) []byte { + server = listenVSR(t, nil, func(_, _ int, read request) []byte { switch { - case read.Code() == uint32(command.BindSessionCode): - return BindReplyFrame(7, read.SessionID()) - case read.Code() == uint32(command.GetClusterMetadataCode): - return ClusterMetadataFrame(t, 0, server.address()) - case read.Operation() == vsr.OperationRegister: - return RegisterReplyFrame(7, 128) - case read.Operation() == vsr.OperationLogout: - return ReplyFrame(vsr.OperationLogout, nil) + case read.code() == uint32(command.BindSessionCode): + return bindReplyFrame(7, read.sessionID()) + case read.code() == uint32(command.GetClusterMetadataCode): + return clusterMetadataFrame(t, 0, server.address()) + case read.operation() == vsr.OperationRegister: + return registerReplyFrame(7, 128) + case read.operation() == vsr.OperationLogout: + return replyFrame(vsr.OperationLogout, nil) default: - return ReplyFrame(vsr.OperationNonReplicated, nil) + return replyFrame(vsr.OperationNonReplicated, nil) } }) @@ -530,18 +539,18 @@ func TestLogoutUser_ResetsTheSessionSoTheNextSignInIsANewIdentity(t *testing.T) recorded := server.recorded() require.Len(t, recorded, 5) - assert.Equal(t, vsr.OperationLogout, recorded[2].Operation(), + assert.Equal(t, vsr.OperationLogout, recorded[2].operation(), "a re-login logs the live session out first") - assert.Equal(t, vsr.OperationRegister, recorded[3].Operation()) - assert.NotEqual(t, first, recorded[3].ClientID(), + assert.Equal(t, vsr.OperationRegister, recorded[3].operation()) + assert.NotEqual(t, first, recorded[3].clientID(), "the re-login presents a new client identity") assert.True(t, client.session.Bound()) } func TestHandleLeaderRedirection_StopsAtTheRedirectBudget(t *testing.T) { client, serverConn := newPipeClient(t) - Serve(serverConn, func(_ int, _ Request) []byte { - return ClusterMetadataFrame(t, 0, "10.0.0.1:8090", "10.0.0.2:8090") + serve(serverConn, func(_ int, _ request) []byte { + return clusterMetadataFrame(t, 0, "10.0.0.1:8090", "10.0.0.2:8090") }) for range iggcon.MaxLeaderRedirects { client.leaderRedirectionState.IncrementRedirect("10.0.0.1:8090") @@ -585,7 +594,7 @@ func TestConnect_UnwindsWhenEveryReadFailsInsideTheConnectFlow(t *testing.T) { } func TestConnect_ExchangesOverTLS(t *testing.T) { - certificate, caPath := SelfSignedCert(t) + certificate, caPath := selfSignedCert(t) var server *testListener server = listenVSR(t, @@ -606,9 +615,9 @@ func TestConnect_ExchangesOverTLS(t *testing.T) { recorded := server.recorded() require.Len(t, recorded, 3) - assert.Equal(t, vsr.OperationRegister, recorded[0].Operation()) - assert.Equal(t, uint32(command.GetClusterMetadataCode), recorded[1].Code()) - assert.Equal(t, uint32(command.PingCode), recorded[2].Code()) + assert.Equal(t, vsr.OperationRegister, recorded[0].operation()) + assert.Equal(t, uint32(command.GetClusterMetadataCode), recorded[1].code()) + assert.Equal(t, uint32(command.PingCode), recorded[2].code()) assert.True(t, client.session.Bound()) } diff --git a/foreign/go/client/tcp/tcp_consumer_group_management_test.go b/foreign/go/client/tcp/tcp_consumer_group_management_test.go index 435f2ed442..525bbfbd51 100644 --- a/foreign/go/client/tcp/tcp_consumer_group_management_test.go +++ b/foreign/go/client/tcp/tcp_consumer_group_management_test.go @@ -20,13 +20,13 @@ package tcp import ( "context" "encoding/binary" + "strings" + "testing" + ierror "github.com/apache/iggy/foreign/go/errors" - . "github.com/apache/iggy/foreign/go/internal/fakevsr" "github.com/apache/iggy/foreign/go/internal/vsr" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "strings" - "testing" ) // consumerGroupBody builds one wire consumer group: @@ -44,8 +44,8 @@ func TestGetConsumerGroups_DecodesTheRoster(t *testing.T) { body := append( consumerGroupBody(1, 3, 2, "billing"), consumerGroupBody(2, 1, 0, "audit")...) - Serve(serverConn, func(_ int, _ Request) []byte { - return ReplyFrame(vsr.OperationNonReplicated, body) + serve(serverConn, func(_ int, _ request) []byte { + return replyFrame(vsr.OperationNonReplicated, body) }) groups, err := client.GetConsumerGroups(context.Background(), @@ -59,8 +59,8 @@ func TestGetConsumerGroups_DecodesTheRoster(t *testing.T) { func TestGetConsumerGroup_ReportsAMissingGroupAsATypedError(t *testing.T) { client, serverConn := newPipeClient(t) - Serve(serverConn, func(_ int, _ Request) []byte { - return ReplyFrame(vsr.OperationNonReplicated, nil) + serve(serverConn, func(_ int, _ request) []byte { + return replyFrame(vsr.OperationNonReplicated, nil) }) _, err := client.GetConsumerGroup(context.Background(), @@ -70,8 +70,8 @@ func TestGetConsumerGroup_ReportsAMissingGroupAsATypedError(t *testing.T) { func TestCreateConsumerGroup_RejectsAnInvalidNameWithoutWriting(t *testing.T) { client, serverConn := newPipeClient(t) - server := Serve(serverConn, func(_ int, _ Request) []byte { - return ReplyFrame(vsr.OperationCreateConsumerGroup, ResultSection()) + server := serve(serverConn, func(_ int, _ request) []byte { + return replyFrame(vsr.OperationCreateConsumerGroup, resultSection()) }) for _, name := range []string{"", strings.Repeat("x", MaxStringLength+1)} { @@ -79,14 +79,14 @@ func TestCreateConsumerGroup_RejectsAnInvalidNameWithoutWriting(t *testing.T) { numericIdentifier(t, 1), numericIdentifier(t, 2), name) assert.ErrorIs(t, err, ierror.ErrInvalidConsumerGroupName, "name %q", name) } - assert.Empty(t, server.Recorded(), "an invalid name never reaches the wire") + assert.Empty(t, server.recorded(), "an invalid name never reaches the wire") } func TestCreateConsumerGroup_DecodesTheCreatedGroup(t *testing.T) { client, serverConn := newPipeClient(t) - Serve(serverConn, func(_ int, _ Request) []byte { - return ReplyFrame(vsr.OperationCreateConsumerGroup, - append(ResultSection(), consumerGroupBody(7, 3, 0, "billing")...)) + serve(serverConn, func(_ int, _ request) []byte { + return replyFrame(vsr.OperationCreateConsumerGroup, + append(resultSection(), consumerGroupBody(7, 3, 0, "billing")...)) }) group, err := client.CreateConsumerGroup(context.Background(), @@ -98,8 +98,8 @@ func TestCreateConsumerGroup_DecodesTheCreatedGroup(t *testing.T) { func TestDeleteConsumerGroup_MarksTheGroupAsLeft(t *testing.T) { client, serverConn := newPipeClient(t) - Serve(serverConn, func(_ int, _ Request) []byte { - return ReplyFrame(vsr.OperationDeleteConsumerGroup, ResultSection()) + serve(serverConn, func(_ int, _ request) []byte { + return replyFrame(vsr.OperationDeleteConsumerGroup, resultSection()) }) streamId := numericIdentifier(t, 1) @@ -114,8 +114,8 @@ func TestDeleteConsumerGroup_MarksTheGroupAsLeft(t *testing.T) { func TestSyncConsumerGroup_ReportsANonMemberAsATypedError(t *testing.T) { client, serverConn := newPipeClient(t) - Serve(serverConn, func(_ int, _ Request) []byte { - return ReplyFrame(vsr.OperationNonReplicated, nil) + serve(serverConn, func(_ int, _ request) []byte { + return replyFrame(vsr.OperationNonReplicated, nil) }) assignment, err := client.SyncConsumerGroup(context.Background(), @@ -126,8 +126,8 @@ func TestSyncConsumerGroup_ReportsANonMemberAsATypedError(t *testing.T) { func TestSyncConsumerGroup_KeepsAMemberWithoutPartitionsNonNil(t *testing.T) { client, serverConn := newPipeClient(t) - Serve(serverConn, func(_ int, _ Request) []byte { - return ReplyFrame(vsr.OperationNonReplicated, assignmentBody(6)) + serve(serverConn, func(_ int, _ request) []byte { + return replyFrame(vsr.OperationNonReplicated, assignmentBody(6)) }) assignment, err := client.SyncConsumerGroup(context.Background(), diff --git a/foreign/go/client/tcp/tcp_core_encode_test.go b/foreign/go/client/tcp/tcp_core_encode_test.go index 97c6a25c3d..3ba3488301 100644 --- a/foreign/go/client/tcp/tcp_core_encode_test.go +++ b/foreign/go/client/tcp/tcp_core_encode_test.go @@ -20,12 +20,13 @@ package tcp import ( "bytes" "errors" + "testing" + iggcon "github.com/apache/iggy/foreign/go/contracts" "github.com/apache/iggy/foreign/go/internal/command" "github.com/apache/iggy/foreign/go/internal/vsr" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "testing" ) // marshalOnlyCmd implements command.Command without AppendBinary; it forces @@ -72,7 +73,7 @@ func TestAppendCommandFrame_AppenderPathMatchesTheFallback(t *testing.T) { assert.Equal(t, want[vsr.HeaderSize:], got[vsr.HeaderSize:], "the fast path diverges from the MarshalBinary fallback") assert.Len(t, got, vsr.HeaderSize+len(body), - "the frame is the reserved prologue followed by the Payload") + "the frame is the reserved prologue followed by the payload") } func TestAppendCommandFrame_FallbackPropagatesTheMarshalError(t *testing.T) { @@ -99,7 +100,7 @@ func TestRequestBufPool_AcquireReleaseRoundTrip(t *testing.T) { require.NotNil(t, bp) require.NotNil(t, *bp) assert.GreaterOrEqual(t, cap(*bp), vsr.HeaderSize, - "a fresh buffer already holds the Header prologue") + "a fresh buffer already holds the header prologue") *bp = append(*bp, []byte("hello")...) releaseRequestBuf(bp) diff --git a/foreign/go/client/tcp/tcp_failover_test.go b/foreign/go/client/tcp/tcp_failover_test.go index 5144974c32..1f00638da9 100644 --- a/foreign/go/client/tcp/tcp_failover_test.go +++ b/foreign/go/client/tcp/tcp_failover_test.go @@ -646,7 +646,7 @@ func TestFailover_DoesNotSettleOnAnEndpointThatFailedTheHandshake(t *testing.T) // cover fails the handshake -- which is every failover, once the addresses // differ. func TestFailover_UsesTheDialedEndpointAsTheServerName(t *testing.T) { - certificate, caPath := SelfSignedCert(t) + certificate, caPath := selfSignedCert(t) survivor := listenVSR(t, func(conn net.Conn) net.Conn { return tls.Server(conn, &tls.Config{Certificates: []tls.Certificate{certificate}}) @@ -671,7 +671,7 @@ func TestFailover_UsesTheDialedEndpointAsTheServerName(t *testing.T) { // every attempt, so it has to reach the caller instead of being redialed every // interval forever -- which is what the default unlimited retries did with it. func TestFailover_AConfigFaultEndsTheConnectInsteadOfRetryingForever(t *testing.T) { - certificate, _ := SelfSignedCert(t) + certificate, _ := selfSignedCert(t) server := listenVSR(t, func(conn net.Conn) net.Conn { return tls.Server(conn, &tls.Config{Certificates: []tls.Certificate{certificate}}) @@ -680,7 +680,7 @@ func TestFailover_AConfigFaultEndsTheConnectInsteadOfRetryingForever(t *testing. // A CA the server's certificate was not signed by: no retry makes that // certificate acceptable. - _, unrelatedCA := SelfSignedCert(t) + _, unrelatedCA := selfSignedCert(t) client := NewIggyTcpClient(slog.New(slog.DiscardHandler), WithServerAddress(server.address()), WithTLS(WithTLSCAFile(unrelatedCA), WithTLSValidateCertificate(true))) @@ -703,7 +703,7 @@ func TestFailover_AConfigFaultEndsTheConnectInsteadOfRetryingForever(t *testing. // one misconfigured node in the roster costs the client every endpoint behind // it, including the ones that are only down for a moment. func TestFailover_APlaintextEndpointDoesNotEndTheSweep(t *testing.T) { - certificate, _ := SelfSignedCert(t) + certificate, _ := selfSignedCert(t) var accepted atomic.Int32 var survivor *testListener survivor = listenVSR(t, @@ -882,7 +882,7 @@ func TestDisconnect_DoesNotCloseAConnectionItDidNotFailOn(t *testing.T) { // registerMtx, the transaction blocked on the owner -- and callers pass a // context with no deadline, so nothing breaks it. func TestConnect_DoesNotWaitOnAnAttemptThatSignsIn(t *testing.T) { - certificate, _ := SelfSignedCert(t) + certificate, _ := selfSignedCert(t) var survivor *testListener survivor = listenVSR(t, func(conn net.Conn) net.Conn { diff --git a/foreign/go/client/tcp/tcp_group_polling_test.go b/foreign/go/client/tcp/tcp_group_polling_test.go index 0bb99526ab..a5fe7e11b2 100644 --- a/foreign/go/client/tcp/tcp_group_polling_test.go +++ b/foreign/go/client/tcp/tcp_group_polling_test.go @@ -20,15 +20,15 @@ package tcp import ( "context" "encoding/binary" + "testing" + "time" + iggcon "github.com/apache/iggy/foreign/go/contracts" ierror "github.com/apache/iggy/foreign/go/errors" "github.com/apache/iggy/foreign/go/internal/command" - . "github.com/apache/iggy/foreign/go/internal/fakevsr" "github.com/apache/iggy/foreign/go/internal/vsr" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "testing" - "time" ) // assignmentBody builds the SyncConsumerGroup reply body: @@ -50,13 +50,13 @@ func emptyBatchBody(partitionId uint32) []byte { return append(binary.LittleEndian.AppendUint32(body, 0), make([]byte, iggcon.PartitionContextSize)...) } -// polledPartition reads the explicit partition id of a recorded poll Request +// polledPartition reads the explicit partition id of a recorded poll request // that was built from numeric identifiers. -func polledPartition(t *testing.T, read Request) uint32 { +func polledPartition(t *testing.T, read request) uint32 { t.Helper() // [kind u8][consumer id 6][stream id 6][topic id 6][has_partition u8]. - require.Equal(t, byte(1), read.Payload[19], "the group poll names an explicit partition") - return binary.LittleEndian.Uint32(read.Payload[20:24]) + require.Equal(t, byte(1), read.payload[19], "the group poll names an explicit partition") + return binary.LittleEndian.Uint32(read.payload[20:24]) } // groupConsumer builds the group consumer and identifiers the tests poll with. @@ -86,12 +86,12 @@ func agedGroupEntries(client *IggyTcpClient) { func TestPollMessages_GroupPollRoundRobinsTheOwnedPartitions(t *testing.T) { client, serverConn := newPipeClient(t) - server := servePartitionOperations(t, serverConn, func(_ int, read Request) []byte { - switch read.Code() { + server := servePartitionOperations(t, serverConn, func(_ int, read request) []byte { + switch read.code() { case uint32(command.SyncGroupCode): - return ReplyFrame(vsr.OperationNonReplicated, assignmentBody(7, 1, 3)) + return replyFrame(vsr.OperationNonReplicated, assignmentBody(7, 1, 3)) default: - return ReplyFrame(vsr.OperationNonReplicated, emptyBatchBody(polledPartition(t, read))) + return replyFrame(vsr.OperationNonReplicated, emptyBatchBody(polledPartition(t, read))) } }) @@ -105,8 +105,8 @@ func TestPollMessages_GroupPollRoundRobinsTheOwnedPartitions(t *testing.T) { assert.Equal(t, []uint32{1, 3, 1}, partitions, "the cursor advances and wraps across polls") syncs := 0 - for _, read := range server.Recorded() { - if read.Code() == uint32(command.SyncGroupCode) { + for _, read := range server.recorded() { + if read.code() == uint32(command.SyncGroupCode) { syncs++ } } @@ -115,11 +115,11 @@ func TestPollMessages_GroupPollRoundRobinsTheOwnedPartitions(t *testing.T) { func TestPollMessages_GroupPollKeepsTheCursorAcrossASameGenerationRefresh(t *testing.T) { client, serverConn := newPipeClient(t) - servePartitionOperations(t, serverConn, func(_ int, read Request) []byte { - if read.Code() == uint32(command.SyncGroupCode) { - return ReplyFrame(vsr.OperationNonReplicated, assignmentBody(7, 1, 3)) + servePartitionOperations(t, serverConn, func(_ int, read request) []byte { + if read.code() == uint32(command.SyncGroupCode) { + return replyFrame(vsr.OperationNonReplicated, assignmentBody(7, 1, 3)) } - return ReplyFrame(vsr.OperationNonReplicated, emptyBatchBody(polledPartition(t, read))) + return replyFrame(vsr.OperationNonReplicated, emptyBatchBody(polledPartition(t, read))) }) first, err := pollOnce(t, client) @@ -139,11 +139,11 @@ func TestPollMessages_GroupPollKeepsTheCursorAcrossASameGenerationRefresh(t *tes func TestPollMessages_GroupPollRestartsTheCursorOnANewGeneration(t *testing.T) { client, serverConn := newPipeClient(t) generation := uint64(7) - servePartitionOperations(t, serverConn, func(_ int, read Request) []byte { - if read.Code() == uint32(command.SyncGroupCode) { - return ReplyFrame(vsr.OperationNonReplicated, assignmentBody(generation, 1, 3)) + servePartitionOperations(t, serverConn, func(_ int, read request) []byte { + if read.code() == uint32(command.SyncGroupCode) { + return replyFrame(vsr.OperationNonReplicated, assignmentBody(generation, 1, 3)) } - return ReplyFrame(vsr.OperationNonReplicated, emptyBatchBody(polledPartition(t, read))) + return replyFrame(vsr.OperationNonReplicated, emptyBatchBody(polledPartition(t, read))) }) first, err := pollOnce(t, client) @@ -162,18 +162,18 @@ func TestPollMessages_GroupPollRestartsTheCursorOnANewGeneration(t *testing.T) { func TestPollMessages_GroupPollJoinsTheGroupOnTheFirstPoll(t *testing.T) { client, serverConn := newPipeClient(t) joined := false - server := servePartitionOperations(t, serverConn, func(_ int, read Request) []byte { + server := servePartitionOperations(t, serverConn, func(_ int, read request) []byte { switch { - case read.Code() == uint32(command.SyncGroupCode) && !joined: + case read.code() == uint32(command.SyncGroupCode) && !joined: // An empty body means the client is not a member yet. - return ReplyFrame(vsr.OperationNonReplicated, nil) - case read.Code() == uint32(command.SyncGroupCode): - return ReplyFrame(vsr.OperationNonReplicated, assignmentBody(1, 0)) - case read.Operation() == vsr.OperationJoinConsumerGroup: + return replyFrame(vsr.OperationNonReplicated, nil) + case read.code() == uint32(command.SyncGroupCode): + return replyFrame(vsr.OperationNonReplicated, assignmentBody(1, 0)) + case read.operation() == vsr.OperationJoinConsumerGroup: joined = true - return ReplyFrame(vsr.OperationJoinConsumerGroup, ResultSection()) + return replyFrame(vsr.OperationJoinConsumerGroup, resultSection()) default: - return ReplyFrame(vsr.OperationNonReplicated, emptyBatchBody(polledPartition(t, read))) + return replyFrame(vsr.OperationNonReplicated, emptyBatchBody(polledPartition(t, read))) } }) @@ -182,8 +182,8 @@ func TestPollMessages_GroupPollJoinsTheGroupOnTheFirstPoll(t *testing.T) { assert.Equal(t, uint32(0), polled.PartitionId) operations := make([]vsr.Operation, 0) - for _, read := range server.Recorded() { - operations = append(operations, read.Operation()) + for _, read := range server.recorded() { + operations = append(operations, read.operation()) } assert.Contains(t, operations, vsr.OperationJoinConsumerGroup, "the first poll joins on the caller's behalf") @@ -191,14 +191,14 @@ func TestPollMessages_GroupPollJoinsTheGroupOnTheFirstPoll(t *testing.T) { func TestPollMessages_GroupPollDoesNotRejoinALeftGroup(t *testing.T) { client, serverConn := newPipeClient(t) - server := servePartitionOperations(t, serverConn, func(_ int, read Request) []byte { + server := servePartitionOperations(t, serverConn, func(_ int, read request) []byte { switch { - case read.Code() == uint32(command.SyncGroupCode): - return ReplyFrame(vsr.OperationNonReplicated, nil) - case read.Operation() == vsr.OperationLeaveConsumerGroup: - return ReplyFrame(vsr.OperationLeaveConsumerGroup, ResultSection()) + case read.code() == uint32(command.SyncGroupCode): + return replyFrame(vsr.OperationNonReplicated, nil) + case read.operation() == vsr.OperationLeaveConsumerGroup: + return replyFrame(vsr.OperationLeaveConsumerGroup, resultSection()) default: - return ReplyFrame(vsr.OperationJoinConsumerGroup, ResultSection()) + return replyFrame(vsr.OperationJoinConsumerGroup, resultSection()) } }) @@ -210,8 +210,8 @@ func TestPollMessages_GroupPollDoesNotRejoinALeftGroup(t *testing.T) { assert.ErrorIs(t, err, ierror.ErrConsumerGroupMemberNotFound, "an explicit leave stays left until an explicit join") - for _, read := range server.Recorded() { - assert.NotEqual(t, vsr.OperationJoinConsumerGroup, read.Operation(), + for _, read := range server.recorded() { + assert.NotEqual(t, vsr.OperationJoinConsumerGroup, read.operation(), "the poll must not undo the leave") } } @@ -219,20 +219,20 @@ func TestPollMessages_GroupPollDoesNotRejoinALeftGroup(t *testing.T) { func TestPollMessages_GroupPollRejoinsAfterAnExplicitJoin(t *testing.T) { client, serverConn := newPipeClient(t) member := false - servePartitionOperations(t, serverConn, func(_ int, read Request) []byte { + servePartitionOperations(t, serverConn, func(_ int, read request) []byte { switch { - case read.Code() == uint32(command.SyncGroupCode) && member: - return ReplyFrame(vsr.OperationNonReplicated, assignmentBody(2, 4)) - case read.Code() == uint32(command.SyncGroupCode): - return ReplyFrame(vsr.OperationNonReplicated, nil) - case read.Operation() == vsr.OperationLeaveConsumerGroup: + case read.code() == uint32(command.SyncGroupCode) && member: + return replyFrame(vsr.OperationNonReplicated, assignmentBody(2, 4)) + case read.code() == uint32(command.SyncGroupCode): + return replyFrame(vsr.OperationNonReplicated, nil) + case read.operation() == vsr.OperationLeaveConsumerGroup: member = false - return ReplyFrame(vsr.OperationLeaveConsumerGroup, ResultSection()) - case read.Operation() == vsr.OperationJoinConsumerGroup: + return replyFrame(vsr.OperationLeaveConsumerGroup, resultSection()) + case read.operation() == vsr.OperationJoinConsumerGroup: member = true - return ReplyFrame(vsr.OperationJoinConsumerGroup, ResultSection()) + return replyFrame(vsr.OperationJoinConsumerGroup, resultSection()) default: - return ReplyFrame(vsr.OperationNonReplicated, emptyBatchBody(polledPartition(t, read))) + return replyFrame(vsr.OperationNonReplicated, emptyBatchBody(polledPartition(t, read))) } }) @@ -249,11 +249,11 @@ func TestPollMessages_GroupPollRejoinsAfterAnExplicitJoin(t *testing.T) { func TestPollMessages_GroupPollReportsAnEmptyPollWhenTheMemberOwnsNothing(t *testing.T) { client, serverConn := newPipeClient(t) - servePartitionOperations(t, serverConn, func(_ int, read Request) []byte { - if read.Code() == uint32(command.SyncGroupCode) { - return ReplyFrame(vsr.OperationNonReplicated, assignmentBody(5)) + servePartitionOperations(t, serverConn, func(_ int, read request) []byte { + if read.code() == uint32(command.SyncGroupCode) { + return replyFrame(vsr.OperationNonReplicated, assignmentBody(5)) } - t.Errorf("unexpected Request with code %d", read.Code()) + t.Errorf("unexpected request with code %d", read.code()) return nil }) @@ -267,11 +267,11 @@ func TestPollMessages_GroupPollResyncsAnEmptyAssignmentOnTheNextPoll(t *testing. client, serverConn := newPipeClient(t) // The join committed, but no partition installed the member as owner yet. assignment := assignmentBody(1) - server := servePartitionOperations(t, serverConn, func(_ int, read Request) []byte { - if read.Code() == uint32(command.SyncGroupCode) { - return ReplyFrame(vsr.OperationNonReplicated, assignment) + server := servePartitionOperations(t, serverConn, func(_ int, read request) []byte { + if read.code() == uint32(command.SyncGroupCode) { + return replyFrame(vsr.OperationNonReplicated, assignment) } - return ReplyFrame(vsr.OperationNonReplicated, emptyBatchBody(polledPartition(t, read))) + return replyFrame(vsr.OperationNonReplicated, emptyBatchBody(polledPartition(t, read))) }) first, err := pollOnce(t, client) @@ -285,8 +285,8 @@ func TestPollMessages_GroupPollResyncsAnEmptyAssignmentOnTheNextPoll(t *testing. assert.Equal(t, uint32(7), second.PartitionId, "an empty assignment is not trusted for the refresh window") syncs := 0 - for _, read := range server.Recorded() { - if read.Code() == uint32(command.SyncGroupCode) { + for _, read := range server.recorded() { + if read.code() == uint32(command.SyncGroupCode) { syncs++ } } @@ -295,12 +295,12 @@ func TestPollMessages_GroupPollResyncsAnEmptyAssignmentOnTheNextPoll(t *testing. func TestPollMessages_GroupPollReportsAnEmptyPollWhenTheRebalanceOutlastsTheAttempts(t *testing.T) { client, serverConn := newPipeClient(t) - server := servePartitionOperations(t, serverConn, func(_ int, read Request) []byte { - if read.Code() == uint32(command.SyncGroupCode) { - return ReplyFrame(vsr.OperationNonReplicated, assignmentBody(9, 0)) + server := servePartitionOperations(t, serverConn, func(_ int, read request) []byte { + if read.code() == uint32(command.SyncGroupCode) { + return replyFrame(vsr.OperationNonReplicated, assignmentBody(9, 0)) } // The server marks a stale assignment with the resync sentinel. - return ReplyFrame(vsr.OperationNonReplicated, + return replyFrame(vsr.OperationNonReplicated, emptyBatchBody(iggcon.ResyncRequiredPartition)) }) @@ -310,8 +310,8 @@ func TestPollMessages_GroupPollReportsAnEmptyPollWhenTheRebalanceOutlastsTheAtte assert.Empty(t, polled.Messages) polls := 0 - for _, read := range server.Recorded() { - if read.Code() == uint32(command.PollMessagesCode) { + for _, read := range server.recorded() { + if read.code() == uint32(command.PollMessagesCode) { polls++ } } @@ -321,17 +321,17 @@ func TestPollMessages_GroupPollReportsAnEmptyPollWhenTheRebalanceOutlastsTheAtte func TestPollMessages_GroupPollResyncsAfterAFencedPoll(t *testing.T) { client, serverConn := newPipeClient(t) generation := uint64(1) - servePartitionOperations(t, serverConn, func(_ int, read Request) []byte { + servePartitionOperations(t, serverConn, func(_ int, read request) []byte { switch { - case read.Code() == uint32(command.SyncGroupCode): - return ReplyFrame(vsr.OperationNonReplicated, assignmentBody(generation, 1)) + case read.code() == uint32(command.SyncGroupCode): + return replyFrame(vsr.OperationNonReplicated, assignmentBody(generation, 1)) case generation == 1: // The member no longer owns the partition at this generation. generation = 2 - return StatusReplyFrame(vsr.OperationNonReplicated, + return statusReplyFrame(vsr.OperationNonReplicated, uint32(ierror.ConsumerGroupPartitionNotOwnedCode), nil) default: - return ReplyFrame(vsr.OperationNonReplicated, emptyBatchBody(polledPartition(t, read))) + return replyFrame(vsr.OperationNonReplicated, emptyBatchBody(polledPartition(t, read))) } }) diff --git a/foreign/go/client/tcp/tcp_session_credentials_test.go b/foreign/go/client/tcp/tcp_session_credentials_test.go index c90b732173..c219d4f6a3 100644 --- a/foreign/go/client/tcp/tcp_session_credentials_test.go +++ b/foreign/go/client/tcp/tcp_session_credentials_test.go @@ -18,9 +18,10 @@ package tcp import ( + "testing" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "testing" ) func newCredentialClient(autoLogin AutoLogin) *IggyTcpClient { diff --git a/foreign/go/client/tcp/tcp_stream_management_test.go b/foreign/go/client/tcp/tcp_stream_management_test.go index d3b18e5320..f46033b89d 100644 --- a/foreign/go/client/tcp/tcp_stream_management_test.go +++ b/foreign/go/client/tcp/tcp_stream_management_test.go @@ -20,13 +20,13 @@ package tcp import ( "context" "encoding/binary" + "strings" + "testing" + ierror "github.com/apache/iggy/foreign/go/errors" - . "github.com/apache/iggy/foreign/go/internal/fakevsr" "github.com/apache/iggy/foreign/go/internal/vsr" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "strings" - "testing" ) // streamDetailsBody builds a stream reply with no topics: @@ -45,9 +45,9 @@ func streamDetailsBody(id uint32, name string) []byte { func TestCreateStream_DecodesTheCreatedStream(t *testing.T) { client, serverConn := newPipeClient(t) - Serve(serverConn, func(_ int, _ Request) []byte { - return ReplyFrame(vsr.OperationCreateStream, - append(ResultSection(), streamDetailsBody(9, "orders")...)) + serve(serverConn, func(_ int, _ request) []byte { + return replyFrame(vsr.OperationCreateStream, + append(resultSection(), streamDetailsBody(9, "orders")...)) }) stream, err := client.CreateStream(context.Background(), "orders") @@ -58,21 +58,21 @@ func TestCreateStream_DecodesTheCreatedStream(t *testing.T) { func TestCreateStream_RejectsAnInvalidNameWithoutWriting(t *testing.T) { client, serverConn := newPipeClient(t) - server := Serve(serverConn, func(_ int, _ Request) []byte { - return ReplyFrame(vsr.OperationCreateStream, ResultSection()) + server := serve(serverConn, func(_ int, _ request) []byte { + return replyFrame(vsr.OperationCreateStream, resultSection()) }) for _, name := range []string{"", strings.Repeat("x", MaxStringLength+1)} { _, err := client.CreateStream(context.Background(), name) assert.ErrorIs(t, err, ierror.ErrInvalidStreamName, "name %q", name) } - assert.Empty(t, server.Recorded()) + assert.Empty(t, server.recorded()) } func TestGetStream_ReportsAMissingStreamAsATypedError(t *testing.T) { client, serverConn := newPipeClient(t) - Serve(serverConn, func(_ int, _ Request) []byte { - return ReplyFrame(vsr.OperationNonReplicated, nil) + serve(serverConn, func(_ int, _ request) []byte { + return replyFrame(vsr.OperationNonReplicated, nil) }) _, err := client.GetStream(context.Background(), numericIdentifier(t, 42)) @@ -82,8 +82,8 @@ func TestGetStream_ReportsAMissingStreamAsATypedError(t *testing.T) { func TestGetStreams_DecodesTheRoster(t *testing.T) { client, serverConn := newPipeClient(t) body := append(streamDetailsBody(1, "orders"), streamDetailsBody(2, "billing")...) - Serve(serverConn, func(_ int, _ Request) []byte { - return ReplyFrame(vsr.OperationNonReplicated, body) + serve(serverConn, func(_ int, _ request) []byte { + return replyFrame(vsr.OperationNonReplicated, body) }) streams, err := client.GetStreams(context.Background()) @@ -95,21 +95,21 @@ func TestGetStreams_DecodesTheRoster(t *testing.T) { func TestUpdateStream_RejectsAnInvalidNameWithoutWriting(t *testing.T) { client, serverConn := newPipeClient(t) - server := Serve(serverConn, func(_ int, _ Request) []byte { - return ReplyFrame(vsr.OperationUpdateStream, ResultSection()) + server := serve(serverConn, func(_ int, _ request) []byte { + return replyFrame(vsr.OperationUpdateStream, resultSection()) }) for _, name := range []string{"", strings.Repeat("x", MaxStringLength+1)} { err := client.UpdateStream(context.Background(), numericIdentifier(t, 1), name) assert.ErrorIs(t, err, ierror.ErrInvalidStreamName, "name %q", name) } - assert.Empty(t, server.Recorded()) + assert.Empty(t, server.recorded()) } func TestUpdateStream_AcceptsACommittedRename(t *testing.T) { client, serverConn := newPipeClient(t) - Serve(serverConn, func(_ int, _ Request) []byte { - return ReplyFrame(vsr.OperationUpdateStream, ResultSection()) + serve(serverConn, func(_ int, _ request) []byte { + return replyFrame(vsr.OperationUpdateStream, resultSection()) }) assert.NoError(t, client.UpdateStream( @@ -118,14 +118,14 @@ func TestUpdateStream_AcceptsACommittedRename(t *testing.T) { func TestCreatePartitions_InvalidatesTheCachedPartitionCount(t *testing.T) { client, serverConn := newPipeClient(t) - Serve(serverConn, func(_ int, read Request) []byte { - switch read.Operation() { + serve(serverConn, func(_ int, read request) []byte { + switch read.operation() { case vsr.OperationCreatePartitions: - return ReplyFrame(vsr.OperationCreatePartitions, ResultSection()) + return replyFrame(vsr.OperationCreatePartitions, resultSection()) case vsr.OperationDeletePartitions: - return ReplyFrame(vsr.OperationDeletePartitions, ResultSection()) + return replyFrame(vsr.OperationDeletePartitions, resultSection()) default: - return ReplyFrame(vsr.OperationNonReplicated, nil) + return replyFrame(vsr.OperationNonReplicated, nil) } }) @@ -146,8 +146,8 @@ func TestCreatePartitions_InvalidatesTheCachedPartitionCount(t *testing.T) { func TestDeleteTopic_DropsTheTopicCacheEntry(t *testing.T) { client, serverConn := newPipeClient(t) - Serve(serverConn, func(_ int, _ Request) []byte { - return ReplyFrame(vsr.OperationDeleteTopic, ResultSection()) + serve(serverConn, func(_ int, _ request) []byte { + return replyFrame(vsr.OperationDeleteTopic, resultSection()) }) streamId := numericIdentifier(t, 1) diff --git a/foreign/go/client/tcp/tcp_topic_management_test.go b/foreign/go/client/tcp/tcp_topic_management_test.go index 64bd93e927..93a531c234 100644 --- a/foreign/go/client/tcp/tcp_topic_management_test.go +++ b/foreign/go/client/tcp/tcp_topic_management_test.go @@ -20,21 +20,21 @@ package tcp import ( "context" "encoding/binary" + "strings" + "testing" + iggcon "github.com/apache/iggy/foreign/go/contracts" ierror "github.com/apache/iggy/foreign/go/errors" - . "github.com/apache/iggy/foreign/go/internal/fakevsr" "github.com/apache/iggy/foreign/go/internal/vsr" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "strings" - "testing" ) func TestCreateTopic_DecodesTheCreatedTopic(t *testing.T) { client, serverConn := newPipeClient(t) - Serve(serverConn, func(_ int, _ Request) []byte { - return ReplyFrame(vsr.OperationCreateTopic, - append(ResultSection(), topicDetailsBody(t, 3)...)) + serve(serverConn, func(_ int, _ request) []byte { + return replyFrame(vsr.OperationCreateTopic, + append(resultSection(), topicDetailsBody(t, 3)...)) }) topic, err := client.CreateTopic(context.Background(), @@ -46,8 +46,8 @@ func TestCreateTopic_DecodesTheCreatedTopic(t *testing.T) { func TestCreateTopic_RejectsInvalidArgumentsWithoutWriting(t *testing.T) { client, serverConn := newPipeClient(t) - server := Serve(serverConn, func(_ int, _ Request) []byte { - return ReplyFrame(vsr.OperationCreateTopic, ResultSection()) + server := serve(serverConn, func(_ int, _ request) []byte { + return replyFrame(vsr.OperationCreateTopic, resultSection()) }) streamId := numericIdentifier(t, 1) @@ -71,13 +71,13 @@ func TestCreateTopic_RejectsInvalidArgumentsWithoutWriting(t *testing.T) { assert.ErrorIs(t, err, test.want) }) } - assert.Empty(t, server.Recorded()) + assert.Empty(t, server.recorded()) } func TestUpdateTopic_RejectsInvalidArgumentsWithoutWriting(t *testing.T) { client, serverConn := newPipeClient(t) - server := Serve(serverConn, func(_ int, _ Request) []byte { - return ReplyFrame(vsr.OperationUpdateTopic, ResultSection()) + server := serve(serverConn, func(_ int, _ request) []byte { + return replyFrame(vsr.OperationUpdateTopic, resultSection()) }) streamId := numericIdentifier(t, 1) @@ -86,13 +86,13 @@ func TestUpdateTopic_RejectsInvalidArgumentsWithoutWriting(t *testing.T) { err := client.UpdateTopic(context.Background(), streamId, topicId, "", 0, iggcon.Duration(0), 0) assert.ErrorIs(t, err, ierror.ErrInvalidTopicName) - assert.Empty(t, server.Recorded()) + assert.Empty(t, server.recorded()) } func TestUpdateTopic_AcceptsACommittedUpdate(t *testing.T) { client, serverConn := newPipeClient(t) - Serve(serverConn, func(_ int, _ Request) []byte { - return ReplyFrame(vsr.OperationUpdateTopic, ResultSection()) + serve(serverConn, func(_ int, _ request) []byte { + return replyFrame(vsr.OperationUpdateTopic, resultSection()) }) assert.NoError(t, client.UpdateTopic(context.Background(), @@ -105,8 +105,8 @@ func TestGetTopics_DecodesTheRoster(t *testing.T) { body := binary.LittleEndian.AppendUint32(nil, 2) body = append(body, topicDetailsBody(t, 3)...) body = append(body, topicDetailsBody(t, 5)...) - Serve(serverConn, func(_ int, _ Request) []byte { - return ReplyFrame(vsr.OperationNonReplicated, body) + serve(serverConn, func(_ int, _ request) []byte { + return replyFrame(vsr.OperationNonReplicated, body) }) topics, err := client.GetTopics(context.Background(), numericIdentifier(t, 1)) @@ -118,8 +118,8 @@ func TestGetTopics_DecodesTheRoster(t *testing.T) { func TestGetTopic_ReportsAMissingTopicAsATypedError(t *testing.T) { client, serverConn := newPipeClient(t) - Serve(serverConn, func(_ int, _ Request) []byte { - return ReplyFrame(vsr.OperationNonReplicated, nil) + serve(serverConn, func(_ int, _ request) []byte { + return replyFrame(vsr.OperationNonReplicated, nil) }) _, err := client.GetTopic(context.Background(), diff --git a/foreign/go/internal/fakevsr/fakevsr.go b/foreign/go/internal/fakevsr/fakevsr.go index 10c2010e58..45d6d517c7 100644 --- a/foreign/go/internal/fakevsr/fakevsr.go +++ b/foreign/go/internal/fakevsr/fakevsr.go @@ -15,11 +15,11 @@ // specific language governing permissions and limitations // under the License. -// Package fakevsr provides the fake VSR server the Go SDK tests drive a -// full connect-and-login flow against, without a real server. Both the -// transport tests and the connection-string tests import it, so the wire -// offsets and the reply builders live in one place instead of drifting -// copies. It is an internal package, so it never reaches SDK users. +// Package fakevsr provides the fake VSR server the connection-string +// tests drive a full connect-and-login flow against, without a real +// server, and the shared certificate helper the transport tests mint +// their throwaway TLS certificates with. It is an internal package, so +// it never reaches SDK users. package fakevsr import ( @@ -51,7 +51,6 @@ import ( const ( FrameOffsetSize = 48 FrameOffsetCommand = 60 - FrameOffsetClient = 128 FrameOffsetRequest = 168 FrameOffsetOperation = 176 FrameOffsetSession = 184 @@ -78,11 +77,6 @@ func (r Request) Code() uint32 { return binary.LittleEndian.Uint32(r.Header[FrameOffsetReserved:]) } -// RequestID reads the request correlation identifier. -func (r Request) RequestID() uint64 { - return binary.LittleEndian.Uint64(r.Header[FrameOffsetRequest:]) -} - // SessionID reads the logical session the request carries. func (r Request) SessionID() uint64 { return binary.LittleEndian.Uint64(r.Header[FrameOffsetSession:]) @@ -120,17 +114,6 @@ func StatusReplyFrame(operation vsr.Operation, status uint32, body []byte) []byt return frame } -// EvictionFrame builds a header-only session-terminal frame. -func EvictionFrame(reason vsr.EvictionReason, version, versionMin uint32) []byte { - frame := make([]byte, vsr.HeaderSize) - binary.LittleEndian.PutUint32(frame[FrameOffsetSize:], vsr.HeaderSize) - frame[FrameOffsetCommand] = 13 - binary.LittleEndian.PutUint32(frame[144:], version) - binary.LittleEndian.PutUint32(frame[148:], versionMin) - frame[255] = byte(reason) - return frame -} - // ResultSection builds the committed result prefix of a metadata reply. func ResultSection(codes ...uint32) []byte { section := binary.LittleEndian.AppendUint32(nil, uint32(len(codes))) @@ -174,6 +157,7 @@ func ClusterMetadataFrame(t *testing.T, leaderIndex int, addresses ...string) [] role = iggcon.RoleLeader } metadata.Nodes = append(metadata.Nodes, iggcon.ClusterNode{ + ReplicaID: uint8(index), Name: host, IP: host, Role: role, @@ -206,34 +190,6 @@ type Server struct { done chan struct{} } -// Serve reads frames off conn until it closes, answering each with the -// frame the handler returns. A nil answer leaves the request unanswered. -func Serve(conn net.Conn, handler func(index int, read Request) []byte) *Server { - server := &Server{done: make(chan struct{})} - go func() { - defer close(server.done) - for index := 0; ; index++ { - read, err := ReadRequest(conn) - if err != nil { - return - } - server.mtx.Lock() - server.requests = append(server.requests, read) - server.mtx.Unlock() - - answer := handler(index, read) - if answer == nil { - continue - } - EchoReplyRequest(answer, read) - if _, err := conn.Write(answer); err != nil { - return - } - } - }() - return server -} - // Listen answers frames the way a single-node Iggy server does, so a test // can drive a full connect-and-login flow without a real server: the // bind-session and cluster-metadata requests get their canned replies, a @@ -291,45 +247,6 @@ func Listen(t *testing.T, wrap func(net.Conn) net.Conn) *Server { return server } -// FrameOffsetContext is the incarnation-slot offset inside a request -// header, which a few transport tests read directly. -const FrameOffsetContext = 200 - -// PartitionID reads the resolved partition out of a recorded SendMessages -// payload: [metadata length u32][stream id][topic id][partitioning], where -// the identifiers and the partitioning are each -// [kind u8][length u8][value]. The frame carries no routing namespace, so -// the payload is the only place the client partitioning decision is -// observable. -func (r Request) PartitionID(t *testing.T) uint32 { - t.Helper() - - cursor := 4 - for range 2 { - require.Greater(t, len(r.Payload), cursor+1) - cursor += 2 + int(r.Payload[cursor+1]) - } - require.Greater(t, len(r.Payload), cursor+1) - require.Equal(t, byte(iggcon.PartitionIdKind), r.Payload[cursor], - "the client resolves every strategy to an explicit partition") - require.Equal(t, byte(4), r.Payload[cursor+1]) - require.GreaterOrEqual(t, len(r.Payload), cursor+6) - return binary.LittleEndian.Uint32(r.Payload[cursor+2:]) -} - -// ClientID reads the request header client identifier. -func (r Request) ClientID() vsr.ClientID { - return vsr.ClientID{ - Lo: binary.LittleEndian.Uint64(r.Header[FrameOffsetClient:]), - Hi: binary.LittleEndian.Uint64(r.Header[FrameOffsetClient+8:]), - } -} - -// Incarnation reads the request header incarnation. -func (r Request) Incarnation() uint64 { - return binary.LittleEndian.Uint64(r.Header[FrameOffsetContext:]) -} - // Recorded returns the requests the server read off the wire. func (s *Server) Recorded() []Request { s.mtx.Lock() From d82966ecd73e071dfdb234c9310fff48ba83ce4e Mon Sep 17 00:00:00 2001 From: choksi2212 Date: Tue, 20 Oct 2026 18:59:32 +0530 Subject: [PATCH 6/7] fix(go): apply heartbeat_interval and share the fake frame builders The parser validated heartbeat_interval but dropped it, so the client kept its five-second default no matter what the connection string asked for. The interval now reaches tcp.WithHeartbeatInterval after a checked int64 conversion, and a value wider than time.Duration fails the parse instead of wrapping. The tcp test helpers readRequest and clusterMetadataFrame now delegate to the shared fakevsr builders. Signed-off-by: choksi2212 --- foreign/go/client/connection_string.go | 37 ++++++++++++---- foreign/go/client/connection_string_test.go | 26 +++++++++++ foreign/go/client/tcp/tcp_core.go | 9 ++++ foreign/go/client/tcp/tcp_testing_test.go | 49 ++++----------------- 4 files changed, 71 insertions(+), 50 deletions(-) diff --git a/foreign/go/client/connection_string.go b/foreign/go/client/connection_string.go index 1b942179ea..0d728e6bf2 100644 --- a/foreign/go/client/connection_string.go +++ b/foreign/go/client/connection_string.go @@ -19,10 +19,12 @@ package client import ( "fmt" + "math" "math/big" "net" "strconv" "strings" + "time" "github.com/apache/iggy/foreign/go/client/tcp" ierror "github.com/apache/iggy/foreign/go/errors" @@ -73,13 +75,14 @@ const ( // types them as non-zero durations whose wire format carries whole // microseconds; reestablish_after takes the zero spellings, which Rust // accepts there. -// The Go client keeps its own reconnect and heartbeat pacing, so those -// three spellings are validated for compatibility and then ignored, and +// The Go client keeps its own reconnect pacing, so the reconnection +// spellings are validated for compatibility and then ignored, and // a value the other SDKs would reject is still rejected here, so one -// connection string is either valid everywhere or nowhere. The nodelay -// spelling is applied, because the Go client carries a matching -// TCP_NODELAY option; it defaults to true, the Go client default, so -// only nodelay=false ever turns Nagle back on. +// connection string is either valid everywhere or nowhere. The +// heartbeat_interval and nodelay spellings are applied, because the Go +// client carries matching options: heartbeat_interval defaults to the +// client five-second pacing, and nodelay defaults to true, the Go +// client default, so only nodelay=false ever turns Nagle back on. func ParseConnectionString(connectionString string) ([]tcp.Option, error) { protocolParts := strings.Split(connectionString, "://") if len(protocolParts) != 2 { @@ -156,6 +159,15 @@ func ParseConnectionString(connectionString string) ([]tcp.Option, error) { options = append(options, tcp.WithTLS(tlsOpts...)) } options = append(options, tcp.WithNoDelay(tlsConfig.noDelay)) + if tlsConfig.heartbeatIntervalNanos != 0 { + // time.Duration counts nanoseconds in an int64, so an interval the + // grammar accepts but Go cannot represent fails the parse, like + // every other spelling the connection string cannot honor. + if tlsConfig.heartbeatIntervalNanos > math.MaxInt64 { + return nil, ierror.InvalidConnectionString{} + } + options = append(options, tcp.WithHeartbeatInterval(time.Duration(tlsConfig.heartbeatIntervalNanos))) + } return options, nil } @@ -192,11 +204,14 @@ type parsedTLS struct { domain string caFile string noDelay bool + // heartbeatIntervalNanos carries the parsed heartbeat_interval, zero + // when the spelling is absent so the client keeps its own pacing. + heartbeatIntervalNanos uint64 } // parseConnectionOptions validates the shared connection-string grammar, -// keeping the tls options and ignoring the reconnect spellings the Go -// client does not take from a connection string. +// keeping the tls and heartbeat options and ignoring the reconnect +// spellings the Go client does not take from a connection string. func parseConnectionOptions(optionsString string) (parsedTLS, error) { parsed := parsedTLS{noDelay: true} if optionsString == "" { @@ -227,9 +242,13 @@ func parseConnectionOptions(optionsString string) (parsedTLS, error) { // Rust types both as NonZeroIggyDuration, which refuses the // zero spellings and anything shorter than the whole // microseconds the wire format carries. - if nanos, err := parseIggyDuration(value); err != nil || nanos < 1_000 { + nanos, err := parseIggyDuration(value) + if err != nil || nanos < 1_000 { return parsed, ierror.InvalidConnectionString{} } + if name == "heartbeat_interval" { + parsed.heartbeatIntervalNanos = nanos + } case "reestablish_after": // Rust takes this one as a plain duration, where the zero // spellings parse. diff --git a/foreign/go/client/connection_string_test.go b/foreign/go/client/connection_string_test.go index 5a349fe5f8..7a7d6b52c7 100644 --- a/foreign/go/client/connection_string_test.go +++ b/foreign/go/client/connection_string_test.go @@ -22,6 +22,7 @@ import ( "crypto/tls" "net" "testing" + "time" "github.com/apache/iggy/foreign/go/client/tcp" "github.com/apache/iggy/foreign/go/internal/command" @@ -88,6 +89,31 @@ func TestParseConnectionString_NoDelayOptionLandsOnTheClient(t *testing.T) { "?nodelay=false must reach the dial instead of being dropped") } +// TestParseConnectionString_HeartbeatIntervalLandsOnTheClient pins the +// parsed heartbeat_interval reaching the client instead of leaving the +// five-second default, and the representation check that refuses an +// interval wider than the client own duration type. +func TestParseConnectionString_HeartbeatIntervalLandsOnTheClient(t *testing.T) { + withoutOption, err := ParseConnectionString("iggy://iggy:secret@localhost:8090") + require.NoError(t, err) + assert.Equal(t, 5*time.Second, applyOptions(t, withoutOption).GetHeartbeatInterval(), + "a query without heartbeat_interval must keep the default pacing") + + oneSecond, err := ParseConnectionString("iggy://iggy:secret@localhost:8090?heartbeat_interval=1s") + require.NoError(t, err) + assert.Equal(t, time.Second, applyOptions(t, oneSecond).GetHeartbeatInterval(), + "?heartbeat_interval=1s must reach the client instead of being dropped") + + fractions, err := ParseConnectionString("iggy://iggy:secret@localhost:8090?heartbeat_interval=250ms") + require.NoError(t, err) + assert.Equal(t, 250*time.Millisecond, applyOptions(t, fractions).GetHeartbeatInterval()) + + tooLarge, err := ParseConnectionString("iggy://iggy:secret@localhost:8090?heartbeat_interval=1000y") + require.Error(t, err, + "an interval wider than time.Duration must fail the parse instead of wrapping") + assert.Empty(t, tooLarge) +} + // TestParseConnectionString_NonZeroDurationOptionsRejectZeroSpellings // pins the typing the Rust SDK gives each option: // reconnection_interval and heartbeat_interval are non-zero durations diff --git a/foreign/go/client/tcp/tcp_core.go b/foreign/go/client/tcp/tcp_core.go index 2fb5db853c..41f804fb18 100644 --- a/foreign/go/client/tcp/tcp_core.go +++ b/foreign/go/client/tcp/tcp_core.go @@ -459,6 +459,15 @@ func (c *IggyTcpClient) GetNoDelay() bool { return c.config.noDelay } +// GetHeartbeatInterval reports the pacing of the managed heartbeat task on +// ready replica links, which tests and callers can use to observe the +// parsed heartbeat_interval option before a connection exists. +func (c *IggyTcpClient) GetHeartbeatInterval() time.Duration { + c.mtx.Lock() + defer c.mtx.Unlock() + return c.config.heartbeatInterval +} + // ResolveOperation replays the retained bytes under their original identity. // A committed business rejection is returned unchanged and releases the lane. // For an operation that is no longer retained, it returns what a replay proved. diff --git a/foreign/go/client/tcp/tcp_testing_test.go b/foreign/go/client/tcp/tcp_testing_test.go index 9d69a5a02e..8d04cb47fd 100644 --- a/foreign/go/client/tcp/tcp_testing_test.go +++ b/foreign/go/client/tcp/tcp_testing_test.go @@ -20,7 +20,6 @@ package tcp import ( "context" "encoding/binary" - "io" "log/slog" "net" "sync" @@ -30,6 +29,7 @@ import ( iggcon "github.com/apache/iggy/foreign/go/contracts" ierror "github.com/apache/iggy/foreign/go/errors" "github.com/apache/iggy/foreign/go/internal/command" + "github.com/apache/iggy/foreign/go/internal/fakevsr" "github.com/apache/iggy/foreign/go/internal/vsr" "github.com/stretchr/testify/require" ) @@ -166,34 +166,9 @@ func bindReplyFrame(userID uint32, session uint64) []byte { } // clusterMetadataFrame builds a cluster roster where the node at leaderIndex -// is the healthy leader. +// is the healthy leader, through the shared fakevsr builder. func clusterMetadataFrame(t *testing.T, leaderIndex int, addresses ...string) []byte { - t.Helper() - - metadata := iggcon.ClusterMetadata{Name: "test-cluster"} - for index, address := range addresses { - host, port, err := net.SplitHostPort(address) - require.NoError(t, err) - tcpPort, err := net.LookupPort("tcp", port) - require.NoError(t, err) - - role := iggcon.RoleFollower - if index == leaderIndex { - role = iggcon.RoleLeader - } - metadata.Nodes = append(metadata.Nodes, iggcon.ClusterNode{ - ReplicaID: uint8(index), - Name: host, - IP: host, - Role: role, - Status: iggcon.Healthy, - Endpoints: iggcon.TransportEndpoints{Tcp: uint16(tcpPort)}, - }) - } - - body, err := metadata.MarshalBinary() - require.NoError(t, err) - return replyFrame(vsr.OperationNonReplicated, body) + return fakevsr.ClusterMetadataFrame(t, leaderIndex, addresses...) } // echoReplyRequest stamps the request id into a reply frame the way the @@ -241,20 +216,12 @@ func versionInfoEnd(t *testing.T, payload []byte) int { return offset } -// readRequest reads one complete frame from conn. +// readRequest reads one complete frame from conn through the shared +// fakevsr reader, adapting the result to the request type the assertions +// use. func readRequest(conn net.Conn) (request, error) { - var read request - if _, err := io.ReadFull(conn, read.header[:]); err != nil { - return read, err - } - size := binary.LittleEndian.Uint32(read.header[frameOffsetSize:]) - if size > vsr.HeaderSize { - read.payload = make([]byte, int(size)-vsr.HeaderSize) - if _, err := io.ReadFull(conn, read.payload); err != nil { - return read, err - } - } - return read, nil + read, err := fakevsr.ReadRequest(conn) + return request{header: read.Header, payload: read.Payload}, err } // fakeServer answers frames from a handler and records what it was asked. From c4604c6bf8935b468469b53b53ea85dc8a99a62d Mon Sep 17 00:00:00 2001 From: choksi2212 Date: Mon, 26 Oct 2026 12:31:01 +0530 Subject: [PATCH 7/7] fix(go): finish review round 3 - drop the dead done channels and the GetNoDelay lock Both fake-server helpers carried a done channel nothing ever read: the field, its allocation and its deferred close are gone from fakevsr.Server and from the tcp suite's fakeServer. GetNoDelay now reads the construction-frozen config directly, like the dial path does; GetHeartbeatInterval keeps its lock because a test rewrites config.heartbeatInterval after construction. Signed-off-by: choksi2212 --- foreign/go/client/tcp/tcp_core.go | 4 ++-- foreign/go/client/tcp/tcp_testing_test.go | 4 +--- foreign/go/internal/fakevsr/fakevsr.go | 4 +--- 3 files changed, 4 insertions(+), 8 deletions(-) diff --git a/foreign/go/client/tcp/tcp_core.go b/foreign/go/client/tcp/tcp_core.go index e0ddd7a60b..66cf87456e 100644 --- a/foreign/go/client/tcp/tcp_core.go +++ b/foreign/go/client/tcp/tcp_core.go @@ -513,8 +513,8 @@ func (c *IggyTcpClient) GetConnectionInfo() *iggcon.ConnectionInfo { // tests and callers can use to observe the parsed nodelay option before a // connection exists to read it off. func (c *IggyTcpClient) GetNoDelay() bool { - c.mtx.Lock() - defer c.mtx.Unlock() + // The client configuration does not change after construction, so this + // reads the field directly, like the dial path does. return c.config.noDelay } diff --git a/foreign/go/client/tcp/tcp_testing_test.go b/foreign/go/client/tcp/tcp_testing_test.go index e06099a0c4..2f3d3d3ee0 100644 --- a/foreign/go/client/tcp/tcp_testing_test.go +++ b/foreign/go/client/tcp/tcp_testing_test.go @@ -228,15 +228,13 @@ func readRequest(conn net.Conn) (request, error) { type fakeServer struct { mtx sync.Mutex requests []request - done chan struct{} } // serve reads frames off conn until it closes, answering each with the frame // the handler returns. A nil answer leaves the request unanswered. func serve(conn net.Conn, handler func(index int, read request) []byte) *fakeServer { - server := &fakeServer{done: make(chan struct{})} + server := &fakeServer{} go func() { - defer close(server.done) for index := 0; ; index++ { read, err := readRequest(conn) if err != nil { diff --git a/foreign/go/internal/fakevsr/fakevsr.go b/foreign/go/internal/fakevsr/fakevsr.go index 45d6d517c7..fd51447410 100644 --- a/foreign/go/internal/fakevsr/fakevsr.go +++ b/foreign/go/internal/fakevsr/fakevsr.go @@ -187,7 +187,6 @@ type Server struct { mtx sync.Mutex requests []Request listener net.Listener - done chan struct{} } // Listen answers frames the way a single-node Iggy server does, so a test @@ -203,9 +202,8 @@ func Listen(t *testing.T, wrap func(net.Conn) net.Conn) *Server { require.NoError(t, err) t.Cleanup(func() { _ = listener.Close() }) - server := &Server{listener: listener, done: make(chan struct{})} + server := &Server{listener: listener} go func() { - defer close(server.done) for { conn, err := listener.Accept() if err != nil {