Skip to content

Commit

Permalink
fix: nil pointer bug introduced by commit 319e6a0 (issue #911) (#930)
Browse files Browse the repository at this point in the history
  • Loading branch information
favonia authored Sep 13, 2024
1 parent dbde79b commit 3ea0eb9
Show file tree
Hide file tree
Showing 9 changed files with 170 additions and 19 deletions.
4 changes: 2 additions & 2 deletions internal/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ func Default() *Config {
WAFListDescription: "",
DetectionTimeout: time.Second * 5, //nolint:mnd
UpdateTimeout: time.Second * 30, //nolint:mnd
Monitor: nil,
Notifier: nil,
Monitor: monitor.NewComposed(),
Notifier: notifier.NewComposed(),
}
}
12 changes: 10 additions & 2 deletions internal/config/config_print.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,15 +85,23 @@ func (c *Config) Print(ppfmt pp.PP) {
item("Record/list updating:", "%v", c.UpdateTimeout)

if c.Monitor != nil {
section("Monitors:")
count := 0
for name, params := range c.Monitor.Describe {
count++
if count == 1 {
section("Monitors:")
}
item(name+":", "%s", params)
}
}

if c.Notifier != nil {
section("Notification services (via shoutrrr):")
count := 0
for name, params := range c.Notifier.Describe {
count++
if count == 1 {
section("Notification services (via shoutrrr):")
}
item(name+":", "%s", params)
}
}
Expand Down
12 changes: 12 additions & 0 deletions internal/config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,3 +13,15 @@ func TestDefaultConfigNotNil(t *testing.T) {

require.NotNil(t, config.Default())
}

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

require.NotNil(t, config.Default().Monitor)
}

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

require.NotNil(t, config.Default().Notifier)
}
2 changes: 1 addition & 1 deletion internal/config/env_notifier.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,6 @@ func ReadAndAppendShoutrrrURL(ppfmt pp.PP, key string, field *notifier.Notifier)
}

// Append the new monitor to the existing list
*field = s
*field = notifier.NewComposed(*field, s)
return true
}
10 changes: 8 additions & 2 deletions internal/config/env_notifier_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,10 @@ func TestReadAndAppendShoutrrrURL(t *testing.T) {
nil,
func(t *testing.T, n not) {
t.Helper()
s, ok := n.(notifier.Shoutrrr)
ns, ok := n.(notifier.Composed)
require.True(t, ok)
require.Len(t, ns, 1)
s, ok := ns[0].(notifier.Shoutrrr)
require.True(t, ok)
require.Equal(t, []string{"Generic"}, s.ServiceDescriptions)
},
Expand All @@ -71,7 +74,10 @@ func TestReadAndAppendShoutrrrURL(t *testing.T) {
nil,
func(t *testing.T, n not) {
t.Helper()
s, ok := n.(notifier.Shoutrrr)
ns, ok := n.(notifier.Composed)
require.True(t, ok)
require.Len(t, ns, 1)
s, ok := ns[0].(notifier.Shoutrrr)
require.True(t, ok)
require.Equal(t, []string{"Generic", "Pushover"}, s.ServiceDescriptions)
},
Expand Down
21 changes: 11 additions & 10 deletions internal/monitor/composite.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,28 +6,29 @@ import (
"github.com/favonia/cloudflare-ddns/internal/pp"
)

type monitors []BasicMonitor
// Composed represents the composite of multiple monitors.
type Composed []BasicMonitor

var _ Monitor = monitors{}
var _ Monitor = Composed{}

// NewComposed creates a new composed monitor.
func NewComposed(mons ...BasicMonitor) monitors {
func NewComposed(mons ...BasicMonitor) Composed {
ms := make([]BasicMonitor, 0, len(mons))
for _, m := range mons {
if m == nil {
continue
}
if list, composed := m.(monitors); composed {
if list, composed := m.(Composed); composed {
ms = append(ms, list...)
} else {
ms = append(ms, m)
}
}
return monitors(ms)
return Composed(ms)
}

// Describe calls [Monitor.Describe] for each monitor in the group with the callback.
func (ms monitors) Describe(yield func(name string, params string) bool) {
func (ms Composed) Describe(yield func(name string, params string) bool) {
for _, m := range ms {
for name, params := range m.Describe {
if !yield(name, params) {
Expand All @@ -38,7 +39,7 @@ func (ms monitors) Describe(yield func(name string, params string) bool) {
}

// Ping calls [Monitor.Ping] for each monitor in the group.
func (ms monitors) Ping(ctx context.Context, ppfmt pp.PP, message Message) bool {
func (ms Composed) Ping(ctx context.Context, ppfmt pp.PP, message Message) bool {
ok := true
for _, m := range ms {
ok = ok && m.Ping(ctx, ppfmt, message)
Expand All @@ -47,7 +48,7 @@ func (ms monitors) Ping(ctx context.Context, ppfmt pp.PP, message Message) bool
}

// Start calls [Monitor.Start] for each monitor in ms.
func (ms monitors) Start(ctx context.Context, ppfmt pp.PP, message string) bool {
func (ms Composed) Start(ctx context.Context, ppfmt pp.PP, message string) bool {
ok := true
for _, m := range ms {
if em, extended := m.(Monitor); extended {
Expand All @@ -58,7 +59,7 @@ func (ms monitors) Start(ctx context.Context, ppfmt pp.PP, message string) bool
}

// Exit calls [Monitor.Exit] for each monitor in ms.
func (ms monitors) Exit(ctx context.Context, ppfmt pp.PP, message string) bool {
func (ms Composed) Exit(ctx context.Context, ppfmt pp.PP, message string) bool {
ok := true
for _, m := range ms {
if em, extended := m.(Monitor); extended {
Expand All @@ -69,7 +70,7 @@ func (ms monitors) Exit(ctx context.Context, ppfmt pp.PP, message string) bool {
}

// Log calls [Monitor.Log] for each monitor in the group.
func (ms monitors) Log(ctx context.Context, ppfmt pp.PP, msg Message) bool {
func (ms Composed) Log(ctx context.Context, ppfmt pp.PP, msg Message) bool {
ok := true
for _, m := range ms {
if em, extended := m.(Monitor); extended {
Expand Down
4 changes: 2 additions & 2 deletions internal/monitor/composite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ func TestComposedDescribe(t *testing.T) {

mockCtrl := gomock.NewController(t)

ms1 := make([]monitor.BasicMonitor, 0, 5)
ms1 := make([]monitor.BasicMonitor, 0, 3)
for range 3 {
m := mocks.NewMockMonitor(mockCtrl)
m.EXPECT().Describe(gomock.Any()).DoAndReturn(
Expand All @@ -27,7 +27,7 @@ func TestComposedDescribe(t *testing.T) {
)
ms1 = append(ms1, m)
}
ms2 := make([]monitor.BasicMonitor, 0, 5)
ms2 := make([]monitor.BasicMonitor, 0, 2)
for range 2 {
m := mocks.NewMockMonitor(mockCtrl)
ms2 = append(ms2, m)
Expand Down
48 changes: 48 additions & 0 deletions internal/notifier/composite.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
package notifier

import (
"context"

"github.com/favonia/cloudflare-ddns/internal/pp"
)

// Composed represents the composite of multiple notifiers.
type Composed []Notifier

var _ Notifier = Composed{}

// NewComposed creates a new composed notifier.
func NewComposed(nots ...Notifier) Composed {
ns := make([]Notifier, 0, len(nots))
for _, n := range nots {
if n == nil {
continue
}
if list, composed := n.(Composed); composed {
ns = append(ns, list...)
} else {
ns = append(ns, n)
}
}
return Composed(ns)
}

// Describe calls [Notifier.Describe] for each notifier in the group with the callback.
func (ns Composed) Describe(yield func(name string, params string) bool) {
for _, n := range ns {
for name, params := range n.Describe {
if !yield(name, params) {
return
}
}
}
}

// Send calls [Notifier.Send] for each notifier in the group.
func (ns Composed) Send(ctx context.Context, ppfmt pp.PP, msg Message) bool {
ok := true
for _, n := range ns {
ok = ok && n.Send(ctx, ppfmt, msg)
}
return ok
}
76 changes: 76 additions & 0 deletions internal/notifier/composite_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
package notifier_test

import (
"context"
"testing"

"github.com/stretchr/testify/require"
"go.uber.org/mock/gomock"

"github.com/favonia/cloudflare-ddns/internal/mocks"
"github.com/favonia/cloudflare-ddns/internal/notifier"
)

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

mockCtrl := gomock.NewController(t)

ns1 := make([]notifier.Notifier, 0, 3)
for range 3 {
n := mocks.NewMockNotifier(mockCtrl)
n.EXPECT().Describe(gomock.Any()).DoAndReturn(
func(yield func(string, string) bool) {
yield("name", "params")
},
)
ns1 = append(ns1, n)
}
ns2 := make([]notifier.Notifier, 0, 2)
for range 2 {
n := mocks.NewMockNotifier(mockCtrl)
ns2 = append(ns2, n)
}
c := notifier.NewComposed(notifier.NewComposed(ns1...), notifier.NewComposed(ns2...))

count := 0
for range c.Describe {
count++
if count >= 3 {
break
}
}
require.Equal(t, 3, count)
}

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

for name, tc := range map[string]struct {
ss []string
}{
"nil": {nil},
"empty": {[]string{}},
"one": {[]string{"hi"}},
"two": {[]string{"hi", "hey"}},
} {
t.Run(name, func(t *testing.T) {
t.Parallel()

ns := make([]notifier.Notifier, 0, 5)
mockCtrl := gomock.NewController(t)
mockPP := mocks.NewMockPP(mockCtrl)

msg := notifier.Message(tc.ss)

for range 5 {
n := mocks.NewMockNotifier(mockCtrl)
n.EXPECT().Send(context.Background(), mockPP, msg).Return(true)
ns = append(ns, n)
}

ok := notifier.NewComposed(ns...).Send(context.Background(), mockPP, msg)
require.True(t, ok)
})
}
}

0 comments on commit 3ea0eb9

Please sign in to comment.