Require manually calling Init() for timeperiod entries

Lazily initializing the timeperiod entries comes with problems regarding the
error handling and would also result in race conditions if multiple callers use
the object at the same time. This commit changes this so that Init() has to be
called explicitly first, allowing proper error handling and read-only use of
the object later.

Author: julianbrost

internal/recipient/rotations_test.go

@@ -5,6 +5,7 @@ import (
 	"github.com/icinga/icinga-go-library/types"
 	"github.com/icinga/icinga-notifications/internal/timeperiod"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 	"testing"
 	"time"
 )
@@ -36,8 +37,7 @@ func Test_rotationResolver_getCurrentRotations(t *testing.T) {
 		return t
 	}
 
-	var s rotationResolver
-	s.update([]*Rotation{
+	rotations := []*Rotation{
 		// Weekend rotation starting 2024, alternating between contacts contactWeekend2024a and contactWeekend2024b
 		{
 			ActualHandoff: types.UnixMilli(parse("2024-01-01")),
@@ -132,7 +132,18 @@ func Test_rotationResolver_getCurrentRotations(t *testing.T) {
 				},
 			},
 		},
-	})
+	}
+
+	for _, r := range rotations {
+		for _, m := range r.Members {
+			for _, e := range m.TimePeriodEntries {
+				require.NoError(t, e.Init())
+			}
+		}
+	}
+
+	var s rotationResolver
+	s.update(rotations)
 
 	for ts := parse("2023-01-01"); ts.Before(parse("2027-01-01")); ts = ts.Add(30 * time.Minute) {
 		got := s.getContactsAt(ts)

internal/timeperiod/timeperiod.go

@@ -6,7 +6,6 @@ import (
 	"github.com/pkg/errors"
 	"github.com/teambition/rrule-go"
 	"go.uber.org/zap/zapcore"
-	"log"
 	"time"
 )
 
@@ -129,10 +128,11 @@ func (e *Entry) Init() error {
 }
 
 // Contains returns whether a point in time t is covered by this entry.
+//
+// This function may only be called after a successful call to Init().
 func (e *Entry) Contains(t time.Time) bool {
-	err := e.Init()
-	if err != nil {
-		log.Printf("Can't initialize entry: %s", err)
+	if !e.initialized {
+		panic("timeperiod.Entry: called Contains() before Init()")
 	}
 
 	if t.Before(e.StartTime.Time()) {
@@ -156,10 +156,11 @@ func (e *Entry) Contains(t time.Time) bool {
 // NextTransition returns the next recurrence start or end of this entry relative to the given time inclusively.
 // This function returns also time.Time's zero value if there is no transition that starts/ends at/after the
 // specified time.
+//
+// This function may only be called after a successful call to Init().
 func (e *Entry) NextTransition(t time.Time) time.Time {
-	err := e.Init()
-	if err != nil {
-		log.Printf("Can't initialize entry: %s", err)
+	if !e.initialized {
+		panic("timeperiod.Entry: called NextTransition() before Init()")
 	}
 
 	if t.Before(e.StartTime.Time()) {

internal/timeperiod/timeperiod_test.go

@@ -6,6 +6,7 @@ import (
 	"github.com/icinga/icinga-go-library/types"
 	"github.com/icinga/icinga-notifications/internal/timeperiod"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 	"github.com/teambition/rrule-go"
 	"testing"
 	"time"
@@ -30,6 +31,8 @@ func TestEntry(t *testing.T) {
 			},
 		}
 
+		require.NoError(t, e.Init())
+
 		t.Run("TimeAtFirstRecurrenceStart", func(t *testing.T) {
 			assert.True(t, e.Contains(start))
 		})
@@ -85,6 +88,8 @@ func TestEntry(t *testing.T) {
 				RRule:     sql.NullString{String: "FREQ=DAILY", Valid: true},
 			}
 
+			require.NoError(t, e.Init())
+
 			assert.True(t, e.Contains(start))
 
 			tz := time.FixedZone("CET", 60*60)
@@ -108,6 +113,8 @@ func TestEntry(t *testing.T) {
 			RRule:     sql.NullString{String: "FREQ=DAILY", Valid: true},
 		}
 
+		require.NoError(t, e.Init())
+
 		t.Run("TimeAtFirstRecurrenceStart", func(t *testing.T) {
 			assert.Equal(t, end, e.NextTransition(start))
 		})
@@ -137,6 +144,7 @@ func TestEntry(t *testing.T) {
 				Timezone:  berlin,
 				RRule:     sql.NullString{String: "FREQ=DAILY", Valid: true},
 			}
+			require.NoError(t, e.Init())
 
 			assert.Equal(t, end, e.NextTransition(start), "next transition should match the first recurrence end")
 
@@ -168,6 +176,10 @@ func TestTimePeriodTransitions(t *testing.T) {
 			}},
 		}
 
+		for _, e := range p.Entries {
+			require.NoError(t, e.Init())
+		}
+
 		assert.Equal(t, end, p.NextTransition(start), "next transition should match the interval end")
 	})
 
@@ -194,6 +206,10 @@ func TestTimePeriodTransitions(t *testing.T) {
 			},
 		}
 
+		for _, e := range p.Entries {
+			require.NoError(t, e.Init())
+		}
+
 		assert.Equal(t, end, p.NextTransition(start), "next transition should match the interval end")
 
 		t.Run("TimeAfterFirstIntervalEnd", func(t *testing.T) {