Skip to content

Commit

Permalink
Address the review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
marcobebway committed Jan 26, 2024
1 parent df439d7 commit 3195dcb
Show file tree
Hide file tree
Showing 6 changed files with 38 additions and 49 deletions.
4 changes: 2 additions & 2 deletions internal/connection/nats/connection.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,15 +49,15 @@ func (c *connection) isConnected() bool {
return c.conn.IsConnected()
}

func (c *connection) RegisterReconnectHandlerIfNotRegistered(handler natsio.ConnHandler) {
func (c *connection) RegisterReconnectHandler(handler natsio.ConnHandler) {
if c.conn == nil || c.reconnectHandlerRegistered {
return
}
c.conn.SetReconnectHandler(handler)
c.reconnectHandlerRegistered = true
}

func (c *connection) RegisterDisconnectErrHandlerIfNotRegistered(handler natsio.ConnErrHandler) {
func (c *connection) RegisterDisconnectErrHandler(handler natsio.ConnErrHandler) {
if c.conn == nil || c.disconnectErrHandlerRegistered {
return
}
Expand Down
8 changes: 4 additions & 4 deletions internal/connection/nats/interface.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,9 @@ type Interface interface {
// Disconnect disconnects the NATS connection.
Disconnect()

// RegisterReconnectHandlerIfNotRegistered registers a ReconnectHandler only if it was not registered before.
RegisterReconnectHandlerIfNotRegistered(natsio.ConnHandler)
// RegisterReconnectHandler registers a ReconnectHandler only if it was not registered before.
RegisterReconnectHandler(natsio.ConnHandler)

// RegisterDisconnectErrHandlerIfNotRegistered registers a DisconnectErrHandler only if it was not registered before.
RegisterDisconnectErrHandlerIfNotRegistered(natsio.ConnErrHandler)
// RegisterDisconnectErrHandler registers a DisconnectErrHandler only if it was not registered before.
RegisterDisconnectErrHandler(natsio.ConnErrHandler)
}
40 changes: 20 additions & 20 deletions internal/connection/nats/mocks/connection.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

23 changes: 6 additions & 17 deletions internal/controller/operator/eventing/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ import (
"context"
"errors"
"fmt"
"os"
"strings"

natsio "github.com/nats-io/nats.go"
Expand Down Expand Up @@ -76,11 +75,10 @@ const (
)

var (
ErrSubscriptionExists = errors.New(SubscriptionExistsErrMessage)
ErrUnsupportedBackedType = errors.New("backend type not supported")
ErrNatsModuleMissing = errors.New("NATS module has to be installed")
ErrAPIGatewayModuleMissing = errors.New("API-Gateway module is needed for EventMesh backend. APIRules CRD is not installed")
ErrNilNATSConnectionBuilder = errors.New("connection builder for NATS backend is nil")
ErrSubscriptionExists = errors.New(SubscriptionExistsErrMessage)
ErrUnsupportedBackedType = errors.New("backend type not supported")
ErrNatsModuleMissing = errors.New("NATS module has to be installed")
ErrAPIGatewayModuleMissing = errors.New("API-Gateway module is needed for EventMesh backend. APIRules CRD is not installed")
)

// Reconciler reconciles an Eventing object
Expand Down Expand Up @@ -575,11 +573,6 @@ func (r *Reconciler) reconcileNATSBackend(ctx context.Context,
}

if connErr := r.connectToNATS(eventingCR); connErr != nil {
if errors.Is(connErr, ErrNilNATSConnectionBuilder) {
r.namedLogger().Error(connErr)
os.Exit(1)
}

if errors.Is(connErr, natsconnectionerrors.ErrCannotConnect) {
return kctrl.Result{}, reconcile.TerminalError(
r.syncStatusWithNATSErr(ctx, eventingCR, connErr, log),
Expand Down Expand Up @@ -608,10 +601,6 @@ func (r *Reconciler) reconcileNATSBackend(ctx context.Context,
// connectToNATS connects to NATS and returns an error if it failed.
// It also registers handlers for reconnection and disconnection.
func (r *Reconciler) connectToNATS(eventingCR *operatorv1alpha1.Eventing) error {
if r.natsConnectionBuilder == nil {
return ErrNilNATSConnectionBuilder
}

if r.natsConnection == nil {
r.natsConnection = r.natsConnectionBuilder.Build()
}
Expand All @@ -622,13 +611,13 @@ func (r *Reconciler) connectToNATS(eventingCR *operatorv1alpha1.Eventing) error

// At this point, it is safe to register handlers
// because the internal nats connection is initialized.
r.natsConnection.RegisterReconnectHandlerIfNotRegistered(
r.natsConnection.RegisterReconnectHandler(
func(_ *natsio.Conn) {
r.namedLogger().Debug("Handle NATS reconnection")
r.genericEvents <- event.GenericEvent{Object: eventingCR}
},
)
r.natsConnection.RegisterDisconnectErrHandlerIfNotRegistered(
r.natsConnection.RegisterDisconnectErrHandler(
func(_ *natsio.Conn, _ error) {
r.namedLogger().Debug("Handle NATS disconnection")
r.genericEvents <- event.GenericEvent{Object: eventingCR}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,8 @@ func Test_NATSConnection(t *testing.T) {
conn := &natsconnectionmocks.Connection{}
conn.On("Connect").Return(nil)
conn.On("IsConnected").Return(true)
conn.On("RegisterReconnectHandlerIfNotRegistered", mock.Anything).Return()
conn.On("RegisterDisconnectErrHandlerIfNotRegistered", mock.Anything).Return()
conn.On("RegisterReconnectHandler", mock.Anything).Return()
conn.On("RegisterDisconnectErrHandler", mock.Anything).Return()
return conn
},
wantMatches: gomega.And(
Expand All @@ -50,8 +50,8 @@ func Test_NATSConnection(t *testing.T) {
conn := &natsconnectionmocks.Connection{}
conn.On("Connect").Return(natsconnectionerrors.ErrCannotConnect)
conn.On("IsConnected").Return(false)
conn.On("RegisterReconnectHandlerIfNotRegistered", mock.Anything).Return()
conn.On("RegisterDisconnectErrHandlerIfNotRegistered", mock.Anything).Return()
conn.On("RegisterReconnectHandler", mock.Anything).Return()
conn.On("RegisterDisconnectErrHandler", mock.Anything).Return()
return conn
},
wantMatches: gomega.And(
Expand Down
4 changes: 2 additions & 2 deletions test/utils/integration/integration.go
Original file line number Diff line number Diff line change
Expand Up @@ -199,8 +199,8 @@ func NewTestEnvironment(config TestEnvironmentConfig, connMock *natsconnectionmo
connMock.On("Connect").Return(nil)
connMock.On("IsConnected").Return(true)
connMock.On("Disconnect").Return()
connMock.On("RegisterReconnectHandlerIfNotRegistered", mock.Anything).Return()
connMock.On("RegisterDisconnectErrHandlerIfNotRegistered", mock.Anything).Return()
connMock.On("RegisterReconnectHandler", mock.Anything).Return()
connMock.On("RegisterDisconnectErrHandler", mock.Anything).Return()
}

// create a new watcher
Expand Down

0 comments on commit 3195dcb

Please sign in to comment.