diff --git a/pkg/docker/dockerimage/topobjects.go b/pkg/docker/dockerimage/topobjects.go index 445c8416f..44d6f3610 100644 --- a/pkg/docker/dockerimage/topobjects.go +++ b/pkg/docker/dockerimage/topobjects.go @@ -4,8 +4,11 @@ import ( "container/heap" ) +// TopObjects is a slice of ObjectMetadata pointers that implements the heap.Interface +// for maintaining a priority queue of objects, typically sorted by size. type TopObjects []*ObjectMetadata +// NewTopObjects creates and returns a new, empty TopObjects slice with a specified capacity. func NewTopObjects(n int) TopObjects { if n < 1 { n = 1 @@ -14,34 +17,51 @@ func NewTopObjects(n int) TopObjects { return make(TopObjects, 0, n) } +// Len returns the number of elements in the TopObjects slice. func (to TopObjects) Len() int { return len(to) } +// Less compares two elements in the slice for sorting. +// Nil elements are considered smaller than non-nil elements. +// Two nil elements are considered equal. func (to TopObjects) Less(i, j int) bool { - if to[i] == nil && to[j] != nil { - return true + // Handle cases where either element is nil + if to[i] == nil && to[j] == nil { + return false // Equal, so not less than } - if to[i] != nil && to[j] == nil { - return false + if to[i] == nil { + return true // nil is considered smaller than non-nil } - if to[i] == nil && to[j] == nil { - return false + if to[j] == nil { + return false // Non-nil is considered larger than nil } - + // Both elements are non-nil, compare their sizes return to[i].Size < to[j].Size } +// Swap swaps the elements with indexes i and j. +// It performs bounds checking to prevent panics. func (to TopObjects) Swap(i, j int) { + if i < 0 || i >= len(to) || j < 0 || j >= len(to) { + return // Out of bounds, no-op + } to[i], to[j] = to[j], to[i] } +// Push adds an element to the heap. It handles nil values safely and only adds +// valid *ObjectMetadata pointers to the slice. func (to *TopObjects) Push(x interface{}) { - item := x.(*ObjectMetadata) - if item == nil { + if x == nil { + return + } + item, ok := x.(*ObjectMetadata) + if !ok || item == nil { return } *to = append(*to, item) } +// Pop removes and returns the smallest element from the heap. +// The result is the element that would be returned by Pop() from the heap package. func (to *TopObjects) Pop() interface{} { old := *to n := len(old) @@ -51,14 +71,24 @@ func (to *TopObjects) Pop() interface{} { return item } +// List returns a sorted slice of ObjectMetadata, with the largest elements first. +// It creates a copy of the underlying data to preserve the original heap. +// Returns nil if the receiver is nil. func (to TopObjects) List() []*ObjectMetadata { - list := []*ObjectMetadata{} - for len(to) > 0 { - item := heap.Pop(&to).(*ObjectMetadata) - if item == nil { - continue + if to == nil { + return nil + } + + tmp := make(TopObjects, len(to)) + copy(tmp, to) + heap.Init(&tmp) + + list := make([]*ObjectMetadata, 0, len(to)) + for tmp.Len() > 0 { + item := heap.Pop(&tmp).(*ObjectMetadata) + if item != nil { + list = append([]*ObjectMetadata{item}, list...) // prepend to maintain order } - list = append([]*ObjectMetadata{item}, list...) } return list diff --git a/pkg/docker/dockerimage/topobjects_test.go b/pkg/docker/dockerimage/topobjects_test.go new file mode 100644 index 000000000..0424753f1 --- /dev/null +++ b/pkg/docker/dockerimage/topobjects_test.go @@ -0,0 +1,94 @@ +package dockerimage + +import ( + "container/heap" + "testing" +) + +func TestTopObjects_List_NilSafety(t *testing.T) { + tests := []struct { + name string + setup func(t *testing.T) TopObjects + wantLen int + }{ + { + name: "nil TopObjects", + setup: func(t *testing.T) TopObjects { + return nil + }, + wantLen: 0, + }, + { + name: "empty TopObjects", + setup: func(t *testing.T) TopObjects { + return NewTopObjects(0) + }, + wantLen: 0, + }, + { + name: "TopObjects with nil elements", + setup: func(t *testing.T) TopObjects { + to := NewTopObjects(3) + heap.Push(&to, &ObjectMetadata{Name: "file1", Size: 100}) + // Test both interface{} nil and typed nil + heap.Push(&to, nil) + heap.Push(&to, (*ObjectMetadata)(nil)) + heap.Push(&to, &ObjectMetadata{Name: "file2", Size: 200}) + return to + }, + wantLen: 2, // Should only contain non-nil elements + }, + { + name: "TopObjects with multiple elements", + setup: func(t *testing.T) TopObjects { + to := NewTopObjects(3) + heap.Push(&to, &ObjectMetadata{Name: "file1", Size: 100}) + heap.Push(&to, &ObjectMetadata{Name: "file2", Size: 200}) + heap.Push(&to, &ObjectMetadata{Name: "file3", Size: 50}) + return to + }, + wantLen: 3, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + to := tt.setup(t) + result := to.List() + + if len(result) != tt.wantLen { + t.Errorf("List() returned %d items, want %d", len(result), tt.wantLen) + } + + // Verify the order is correct (descending by size) + for i := 1; i < len(result); i++ { + if result[i-1] == nil || result[i] == nil { + t.Fatal("List() contains nil elements") + } + if result[i-1].Size < result[i].Size { + t.Errorf("List() is not sorted in descending order: %d < %d at index %d", result[i-1].Size, result[i].Size, i) + } + } + }) + } +} + +func TestTopObjects_List_ModificationSafety(t *testing.T) { + // Test that the original TopObjects is not modified by List() + to := NewTopObjects(3) + heap.Push(&to, &ObjectMetadata{Name: "file1", Size: 100}) + heap.Push(&to, &ObjectMetadata{Name: "file2", Size: 200}) + + originalLen := to.Len() + result := to.List() + + // Modify the result slice + if len(result) > 0 { + result[0] = &ObjectMetadata{Name: "modified", Size: 999} + } + + // The original TopObjects should not be affected + if to.Len() != originalLen { + t.Errorf("Original TopObjects length changed from %d to %d", originalLen, to.Len()) + } +}