Skip to content

Commit

Permalink
feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
James-Pickett committed Sep 27, 2024
1 parent dcaa01e commit 98c8c51
Show file tree
Hide file tree
Showing 4 changed files with 20 additions and 22 deletions.
1 change: 1 addition & 0 deletions ee/desktop/user/client/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ func (c *client) DetectPresence(reason string, interval time.Duration) (time.Dur
encodedReason := url.QueryEscape(reason)
encodedInterval := url.QueryEscape(interval.String())

// default time out of 30s is set in New()
resp, requestErr := c.base.Get(fmt.Sprintf("http://unix/detect_presence?reason=%s&interval=%s", encodedReason, encodedInterval))
if requestErr != nil {
return presencedetection.DetectionFailedDurationValue, fmt.Errorf("getting presence: %w", requestErr)
Expand Down
30 changes: 13 additions & 17 deletions ee/presencedetection/presencedetection.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ import (
const DetectionFailedDurationValue = -1 * time.Second

type PresenceDetector struct {
lastDetectionUTC time.Time
mutext sync.Mutex
lastDetection time.Time
mutext sync.Mutex
// detector is an interface to allow for mocking in tests
detector detectorIface
}
Expand All @@ -36,27 +36,23 @@ func (pd *PresenceDetector) DetectPresence(reason string, detectionInterval time
pd.detector = &detector{}
}

hadHadSuccessfulDetection := pd.lastDetectionUTC != time.Time{}

// Check if the last detection was within the detection interval
if hadHadSuccessfulDetection && time.Since(pd.lastDetectionUTC) < detectionInterval {
return time.Since(pd.lastDetectionUTC), nil
if (pd.lastDetection != time.Time{}) && time.Since(pd.lastDetection) < detectionInterval {
return time.Since(pd.lastDetection), nil
}

success, err := pd.detector.Detect(reason)

switch {
case err != nil && hadHadSuccessfulDetection:
return time.Since(pd.lastDetectionUTC), fmt.Errorf("detecting presence: %w", err)

case err != nil: // error without initial successful detection
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)
}

case success:
pd.lastDetectionUTC = time.Now().UTC()
if success {
pd.lastDetection = time.Now().UTC()
return 0, nil

default: // failed detection without error, maybe not possible?
return time.Since(pd.lastDetectionUTC), fmt.Errorf("detection failed without error")
}

// if we got here it means we failed without an error
// this "should" never happen, but here for completeness
return DetectionFailedDurationValue, fmt.Errorf("detection failed without OS error")
}
6 changes: 3 additions & 3 deletions ee/presencedetection/presencedetection_darwin_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,14 @@ import (
"github.com/stretchr/testify/require"
)

const testPresenceEnvVar = "launcher_test_presence"
const testPresenceEnvVar = "LAUNCHER_TEST_PRESENCE"

// Since there is no way to test user presence in a CI / automated fashion,
// these test are expected to be run manually via cmd line when needed.

// To test this run
//
// launcher_test_presence=true go test ./ee/presencedetection/ -run Test_detectSuccess
// LAUNCHER_TEST_PRESENCE=true go test ./ee/presencedetection/ -run Test_detectSuccess
//
// then successfully auth with the pop up
func Test_detectSuccess(t *testing.T) {
Expand All @@ -32,7 +32,7 @@ func Test_detectSuccess(t *testing.T) {

// To test this run
//
// launcher_test_presence=true go test ./ee/presencedetection/ -run Test_detectCancel
// LAUNCHER_TEST_PRESENCE=true go test ./ee/presencedetection/ -run Test_detectCancel
//
// then cancel the biometric auth that pops up
func Test_detectCancel(t *testing.T) {
Expand Down
5 changes: 3 additions & 2 deletions ee/presencedetection/presencedetection_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ func TestPresenceDetector_DetectPresence(t *testing.T) {
expectError: true,
},
{
// this should never happen, but it is here for completeness
name: "detection failed without OS error",
interval: 0,
detector: func(t *testing.T) detectorIface {
Expand All @@ -89,8 +90,8 @@ func TestPresenceDetector_DetectPresence(t *testing.T) {
t.Parallel()

pd := &PresenceDetector{
detector: tt.detector(t),
lastDetectionUTC: tt.initialLastDetectionUTC,
detector: tt.detector(t),
lastDetection: tt.initialLastDetectionUTC,
}

timeSinceLastDetection, err := pd.DetectPresence("this is a test", tt.interval)
Expand Down

0 comments on commit 98c8c51

Please sign in to comment.