Addresses github comments

This commit is contained in:
riccardom
2026-06-10 16:00:00 +02:00
parent 98e304044c
commit aac109f47f
3 changed files with 15 additions and 43 deletions

View File

@@ -4,24 +4,16 @@ import (
"context"
"reflect"
"sort"
"testing"
"time"
log "github.com/sirupsen/logrus"
)
// defaultReloadInterval is the production cadence at which the desktop daemon
// DefaultReloadInterval is the production cadence at which the desktop daemon
// re-reads the OS-native MDM policy. Picked to balance responsiveness against
// registry/plist I/O overhead. Mobile builds use OS-side notifications
// instead and bypass this ticker entirely. Unexported on purpose: callers do
// not pass it — NewTicker owns the default (see reloadInterval).
const defaultReloadInterval = 1 * time.Minute
// testReloadInterval is the cadence used under `go test` (detected via
// testing.Testing()) so the reload path is exercised in seconds rather than
// minutes. It has no effect on production builds, where testing.Testing()
// always returns false.
const testReloadInterval = 1 * time.Second
// instead, hence anticipating the ticker mechanism entirely.
const DefaultReloadInterval = 1 * time.Minute
// policyLoader is the indirection through which the ticker reads the
// OS-native policy, both for the initial observation and on every tick.
@@ -39,29 +31,17 @@ type Ticker struct {
prev *Policy
}
// reloadInterval returns the production cadence, or the accelerated test
// cadence when running under `go test` (detected via testing.Testing()).
// Centralising the choice here keeps the prod/test split in one place
// and out of the ticker's call sites.
func reloadInterval() time.Duration {
if testing.Testing() {
return testReloadInterval
}
return defaultReloadInterval
}
// NewTicker constructs a Ticker that re-reads the OS-native policy
// every reloadInterval() and invokes onChange on any diff. The
// cadence is owned by reloadInterval (production default, accelerated
// under `go test`); callers do not supply it. onChange may be nil for
// a log-only ticker. The initial snapshot is populated by calling
// policyLoader at construction time so the first tick only fires
// every reloadInterval and invokes onChange on any diff.
// onChange may be nil for a log-only ticker.
// The initial snapshot is populated by calling policyLoader at
// construction time so the first tick only fires
// onChange when the policy actually changed since boot — without
// this baseline the first tick would report every currently-managed
// key as "added" and trigger a spurious engine restart.
func NewTicker(onChange func(prev, curr *Policy)) *Ticker {
func NewTicker(reloadInterval time.Duration, onChange func(prev, curr *Policy)) *Ticker {
return &Ticker{
interval: reloadInterval(),
interval: reloadInterval,
onChange: onChange,
prev: policyLoader(),
}

View File

@@ -10,6 +10,9 @@ import (
"github.com/stretchr/testify/require"
)
// testReloadInterval for speeding up the ticker cadence under `go test`
const testReloadInterval = 1 * time.Second
// withPolicyLoader overrides the package-level policyLoader for the duration
// of the test so the ticker observes a scripted policy instead of the real
// OS-native store. The original loader is restored on cleanup.
@@ -20,17 +23,6 @@ func withPolicyLoader(t *testing.T, fn func() *Policy) {
t.Cleanup(func() { policyLoader = prev })
}
func TestTicker_UsesTestCadenceUnderGoTest(t *testing.T) {
withPolicyLoader(t, func() *Policy { return NewPolicy(nil) })
// Under `go test`, testing.Testing() is true so reloadInterval() returns
// the accelerated 1s cadence instead of the minute-long production
// default — this is what makes the reload path observable without a real
// wall-clock wait.
assert.Equal(t, testReloadInterval, reloadInterval())
assert.Equal(t, testReloadInterval, NewTicker(nil).interval)
}
func TestTicker_FiresOnChangeWithDelta(t *testing.T) {
var mu sync.Mutex
current := NewPolicy(nil) // initial observation: empty (no enforcement)
@@ -42,7 +34,7 @@ func TestTicker_FiresOnChangeWithDelta(t *testing.T) {
type change struct{ prev, curr *Policy }
changes := make(chan change, 1)
tk := NewTicker(func(prev, curr *Policy) {
tk := NewTicker(testReloadInterval, func(prev, curr *Policy) {
select {
case changes <- change{prev, curr}:
default:
@@ -78,7 +70,7 @@ func TestTicker_NoCallbackWhenPolicyUnchanged(t *testing.T) {
})
fired := make(chan struct{}, 1)
tk := NewTicker(func(_, _ *Policy) {
tk := NewTicker(testReloadInterval, func(_, _ *Policy) {
select {
case fired <- struct{}{}:
default:

View File

@@ -174,7 +174,7 @@ func (s *Server) Start() error {
// applies the freshly-read MDM policy as the last layer) and brings
// the engine back with the new values.
if s.mdmTicker == nil {
s.mdmTicker = mdm.NewTicker(s.onMDMPolicyChange)
s.mdmTicker = mdm.NewTicker(mdm.DefaultReloadInterval, s.onMDMPolicyChange)
go s.mdmTicker.Run(s.rootCtx)
}