Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

james/min interval between presence detection #2005

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion ee/localserver/presence-detection-middleware_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ func TestPresenceDetectionHandler(t *testing.T) {
mockPresenceDetector := mocks.NewPresenceDetector(t)

if tt.expectDetectPresenceCall {
mockPresenceDetector.On("DetectPresence", mock.AnythingOfType("string"), mock.AnythingOfType("Duration")).Return(tt.durationSinceLastDetection, tt.presenceDetectionError)
mockPresenceDetector.On("DetectPresence", mock.AnythingOfType("string"), mock.AnythingOfType("Duration")).Return(tt.durationSinceLastDetection, tt.presenceDetectionError).Once()
}

server := &localServer{
Expand Down
1 change: 0 additions & 1 deletion ee/localserver/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -461,7 +461,6 @@ func (ls *localServer) presenceDetectionHandler(next http.Handler) http.Handler
}

durationSinceLastDetection, err := ls.presenceDetector.DetectPresence(reason, detectionIntervalDuration)

if err != nil {
ls.slogger.Log(r.Context(), slog.LevelInfo,
"presence_detection",
Expand Down
28 changes: 22 additions & 6 deletions ee/presencedetection/presencedetection.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,18 @@ import (
)

const (
DetectionFailedDurationValue = -1 * time.Second
DetectionTimeout = 1 * time.Minute
DetectionFailedDurationValue = -1 * time.Second
DetectionTimeout = 1 * time.Minute
DefaultMinDetectionAttmemptInterval = 3 * time.Second
)

type PresenceDetector struct {
lastDetection time.Time
mutext sync.Mutex
mutex sync.Mutex
// detector is an interface to allow for mocking in tests
detector detectorIface
detector detectorIface
lastDetectionAttempt time.Time
minDetectionAttemptInterval time.Duration
}

// just exists for testing purposes
Expand All @@ -32,8 +35,8 @@ func (d *detector) Detect(reason string) (bool, error) {
// DetectPresence checks if the user is present by detecting the presence of a user.
// It returns the duration since the last detection.
func (pd *PresenceDetector) DetectPresence(reason string, detectionInterval time.Duration) (time.Duration, error) {
pd.mutext.Lock()
defer pd.mutext.Unlock()
pd.mutex.Lock()
defer pd.mutex.Unlock()

if pd.detector == nil {
pd.detector = &detector{}
Expand All @@ -44,7 +47,20 @@ func (pd *PresenceDetector) DetectPresence(reason string, detectionInterval time
return time.Since(pd.lastDetection), nil
}

minDetetionInterval := pd.minDetectionAttemptInterval
if minDetetionInterval == 0 {
minDetetionInterval = DefaultMinDetectionAttmemptInterval
}

// if the user fails or cancels the presence detection, we want to wait a bit before trying again
// so that if there are several queued up requests, we don't prompt the user multiple times in a row
// if they keep hitting cancel
if time.Since(pd.lastDetectionAttempt) < minDetetionInterval {
return time.Since(pd.lastDetection), nil
}

success, err := pd.detector.Detect(reason)
pd.lastDetectionAttempt = time.Now()
if err != nil {
// if we got an error, we behave as if there have been no successful detections in the past
return DetectionFailedDurationValue, fmt.Errorf("detecting presence: %w", err)
Expand Down
44 changes: 44 additions & 0 deletions ee/presencedetection/presencedetection_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"github.com/kolide/launcher/ee/presencedetection/mocks"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/mock"
"github.com/stretchr/testify/require"
)

func TestPresenceDetector_DetectPresence(t *testing.T) {
Expand Down Expand Up @@ -106,3 +107,46 @@ func TestPresenceDetector_DetectPresence(t *testing.T) {
})
}
}

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

d := mocks.NewDetectorIface(t)
d.On("Detect", mock.AnythingOfType("string")).Return(true, nil).Once()

pd := &PresenceDetector{
detector: d,
minDetectionAttemptInterval: 500 * time.Millisecond,
}

// first detection, should prompt user for new detection
interval, err := pd.DetectPresence("this is a test", 0)
assert.NoError(t, err)
require.Equal(t, time.Duration(0), interval,
"interval should be 0 since detection just happened",
)

d = mocks.NewDetectorIface(t)
pd.detector = d

// second detection, should NOT prompt user for new detection
// since within min interval
interval, err = pd.DetectPresence("this is a test", 0)
assert.NoError(t, err)
require.Greater(t, interval, time.Duration(0),
"interval should be greater than 0 since some time passed since last detection",
)

// should not have been called since we are within the minDetectionAttemptInterval
d.AssertNotCalled(t, "Detect", mock.Anything)

time.Sleep(1 * time.Second)

// third detection, should prompt user for new detection since minDetectionAttemptInterval has passed
d.On("Detect", mock.AnythingOfType("string")).Return(true, nil).Once()
interval, err = pd.DetectPresence("this is a test", 0)
assert.NoError(t, err)
require.Equal(t, time.Duration(0), interval,
"interval should be 0 since detection just happened",
)
}
Loading