utils: make Median generic, overflow-safe, and add tests (#4553)

* utils: make Median generic, overflow-safe, and add comprehensive * tests: fix staticcheck unused variable warning in changenotifier_test.go * tests: switch from assert to require for consistency with existing tests * trigger ci rerun
2026-09-03 15:06:21 +00:00 · 2026-08-28 10:42:14 +05:30
parent c362e61d3c
commit c88fd2b4d7
4 changed files with 288 additions and 9 deletions
@@ -0,0 +1,122 @@
+// Copyright 2026 LiveKit, Inc.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package utils
+
+import (
+	"sync"
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestChangeNotifier(t *testing.T) {
+	t.Run("Observer management", func(t *testing.T) {
+		notifier := NewChangeNotifier()
+		require.False(t, notifier.HasObservers())
+
+		called := false
+		notifier.AddObserver("test-key", func() {
+			called = true
+		})
+		require.True(t, notifier.HasObservers())
+
+		notifier.RemoveObserver("test-key")
+		require.False(t, notifier.HasObservers())
+		require.False(t, called)
+	})
+
+	t.Run("Notification triggers callbacks asynchronously", func(t *testing.T) {
+		notifier := NewChangeNotifier()
+		var wg sync.WaitGroup
+		wg.Add(2)
+
+		var mu sync.Mutex
+		callCounts := make(map[string]int)
+
+		notifier.AddObserver("obs1", func() {
+			mu.Lock()
+			callCounts["obs1"]++
+			mu.Unlock()
+			wg.Done()
+		})
+
+		notifier.AddObserver("obs2", func() {
+			mu.Lock()
+			callCounts["obs2"]++
+			mu.Unlock()
+			wg.Done()
+		})
+
+		notifier.NotifyChanged()
+
+		// Wait for async execution of observers
+		done := make(chan struct{})
+		go func() {
+			wg.Wait()
+			close(done)
+		}()
+
+		select {
+		case <-done:
+			// Success
+		case <-time.After(1 * time.Second):
+			t.Fatal("Timeout waiting for change notification callbacks")
+		}
+
+		mu.Lock()
+		require.Equal(t, 1, callCounts["obs1"])
+		require.Equal(t, 1, callCounts["obs2"])
+		mu.Unlock()
+	})
+}
+
+func TestChangeNotifierManager(t *testing.T) {
+	t.Run("Get and Create Notifiers", func(t *testing.T) {
+		manager := NewChangeNotifierManager()
+		require.Nil(t, manager.GetNotifier("non-existent"))
+
+		notifier := manager.GetOrCreateNotifier("room1")
+		require.NotNil(t, notifier)
+
+		retrieved := manager.GetNotifier("room1")
+		require.Equal(t, notifier, retrieved)
+
+		// GetOrCreate should return the existing one
+		again := manager.GetOrCreateNotifier("room1")
+		require.Equal(t, notifier, again)
+	})
+
+	t.Run("Remove Notifiers with HasObservers check", func(t *testing.T) {
+		manager := NewChangeNotifierManager()
+		_ = manager.GetOrCreateNotifier("room1")
+
+		// Case 1: notifier has no observers, should be removed
+		manager.RemoveNotifier("room1", false)
+		require.Nil(t, manager.GetNotifier("room1"))
+
+		// Re-create and add an observer
+		notifier := manager.GetOrCreateNotifier("room1")
+		notifier.AddObserver("observer", func() {})
+
+		// Case 2: notifier has observer, RemoveNotifier(..., false) should not remove it
+		manager.RemoveNotifier("room1", false)
+		require.NotNil(t, manager.GetNotifier("room1"))
+
+		// Case 3: notifier has observer, RemoveNotifier(..., true) (force) should remove it
+		manager.RemoveNotifier("room1", true)
+		require.Nil(t, manager.GetNotifier("room1"))
+	})
+}
@@ -14,24 +14,45 @@

 package utils

-import "slices"
+import (
+	"cmp"
+	"slices"
+)

-// Median gets median value for an array
-func Median[T float32](input []T) T {
+// OrderedNumber defines a constraint for numeric types that can be ordered and divided.
+type OrderedNumber interface {
+	cmp.Ordered
+	~int | ~int8 | ~int16 | ~int32 | ~int64 |
+		~uint | ~uint8 | ~uint16 | ~uint32 | ~uint64 |
+		~uintptr | ~float32 | ~float64
+}
+
+// Median gets the median value for a slice without modifying the original slice.
+//
+// Note:
+// 1. For integer types, if the slice has an even length, the division (/ 2)
+//    is performed using integer division, which truncates the result towards zero.
+// 2. Uses an overflow-safe formula left + (right-left)/2 to support narrow integer types.
+func Median[T OrderedNumber](input []T) T {
 	num := len(input)
 	switch num {
 	case 0:
-		return 0
+		var zero T
+		return zero
 	case 1:
 		return input[0]
 	}
-	slices.Sort(input)
+
+	// Clone the slice to avoid mutating the caller's slice
+	sortedInput := slices.Clone(input)
+	slices.Sort(sortedInput)
+
 	if num%2 != 0 {
-		return input[num/2]
+		return sortedInput[num/2]
 	}
-	left := input[num/2-1]
-	right := input[num/2]
-	return (left + right) / 2
+	left := sortedInput[num/2-1]
+	right := sortedInput[num/2]
+	return left + (right-left)/T(2)
 }

 func Signum[T int | int8 | int16 | int32 | int64 | float32 | float64](val T) int {
@@ -0,0 +1,89 @@
+// Copyright 2026 LiveKit, Inc.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package utils
+
+import (
+	"slices"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestMedian(t *testing.T) {
+	t.Run("Empty slice", func(t *testing.T) {
+		require.Equal(t, float32(0), Median([]float32{}))
+		require.Equal(t, int(0), Median([]int{}))
+	})
+
+	t.Run("Single element", func(t *testing.T) {
+		require.Equal(t, float32(42), Median([]float32{42}))
+		require.Equal(t, int(42), Median([]int{42}))
+	})
+
+	t.Run("Odd length float32", func(t *testing.T) {
+		input := []float32{3.0, 1.0, 2.0}
+		require.Equal(t, float32(2.0), Median(input))
+	})
+
+	t.Run("Even length float32 - exact average", func(t *testing.T) {
+		input := []float32{1.0, 2.0, 3.0, 4.0}
+		require.Equal(t, float32(2.5), Median(input))
+	})
+
+	t.Run("Even length int - integer truncation", func(t *testing.T) {
+		input := []int{1, 2}
+		// (1 + 2) / 2 = 1.5 -> truncates to 1
+		require.Equal(t, int(1), Median(input))
+
+		inputOddAverage := []int{1, 3}
+		// (1 + 3) / 2 = 2
+		require.Equal(t, int(2), Median(inputOddAverage))
+	})
+
+	t.Run("Int8 overflow prevention", func(t *testing.T) {
+		// Without overflow protection: 120 + 126 = 246 (overflows int8 to -10) -> -10 / 2 = -5
+		// With overflow protection: 120 + (126-120)/2 = 123
+		input := []int8{120, 126}
+		require.Equal(t, int8(123), Median(input))
+	})
+
+	t.Run("Uint8 overflow prevention", func(t *testing.T) {
+		input := []uint8{250, 254}
+		require.Equal(t, uint8(252), Median(input))
+	})
+
+	t.Run("Immutability test - caller slice is not sorted/mutated", func(t *testing.T) {
+		original := []int{3, 1, 4, 2}
+		input := slices.Clone(original)
+
+		median := Median(input)
+		require.Equal(t, int(2), median)
+		require.Equal(t, original, input, "Input slice must not be modified by Median")
+	})
+}
+
+func TestSignum(t *testing.T) {
+	t.Run("Integer values", func(t *testing.T) {
+		require.Equal(t, -1, Signum(-42))
+		require.Equal(t, 0, Signum(0))
+		require.Equal(t, 1, Signum(42))
+	})
+
+	t.Run("Floating point values", func(t *testing.T) {
+		require.Equal(t, -1, Signum(float32(-0.01)))
+		require.Equal(t, 0, Signum(float32(0.0)))
+		require.Equal(t, 1, Signum(float32(0.01)))
+	})
+}
@@ -0,0 +1,47 @@
+// Copyright 2026 LiveKit, Inc.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package utils
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestDedupeSlice(t *testing.T) {
+	t.Run("Empty slice", func(t *testing.T) {
+		var input []int
+		result := DedupeSlice(input)
+		require.Empty(t, result)
+	})
+
+	t.Run("Single element", func(t *testing.T) {
+		input := []string{"hello"}
+		result := DedupeSlice(input)
+		require.Equal(t, []string{"hello"}, result)
+	})
+
+	t.Run("Unsorted slice with duplicates", func(t *testing.T) {
+		input := []int{4, 2, 4, 1, 3, 2}
+		result := DedupeSlice(input)
+		require.Equal(t, []int{1, 2, 3, 4}, result)
+	})
+
+	t.Run("Already sorted and unique", func(t *testing.T) {
+		input := []string{"apple", "banana", "cherry"}
+		result := DedupeSlice(input)
+		require.Equal(t, []string{"apple", "banana", "cherry"}, result)
+	})
+}