Repository: vmware/govmomi (go)

Issue:
[BUG] Property Collector Updates should be triggered on the empty FilterSet
**Describe the bug**
The `PropertyCollector` update closure passed to `WaitForUpdatesEx` is only triggered when a non-empty `FilterSet` occurs. This is an issue when side effects need to be triggered inside the update function such as notification mechanisms that an update was acknowledged.

It should be the choice of the update closure to determine whether to process the empty `FilterSet` or not.

**To Reproduce**
Steps to reproduce the behavior:

1. Create a container view with no child entities such as an empty vm or host folder.
2. Call wait for updates on the view.
3. Observe that the update closure passed to WFU was not triggered even when an iteration of the WFU loop was processed.

**Expected behavior**
The `onUpdatesFn` should be triggered even if the `FilterSet` is empty.

**Affected version**
Since 95aa2571dac212bfb067147a2bcf85b0eeb9afa6.

**Screenshots/Debug Output**
N/A

**Additional context**
N/A


Pull request description:
Property Collector updates should be triggered on the empty FilterSet
The PropertyCollector update closure passed to WaitForUpdatesEx is only triggered when a non-empty FilterSet occurs.
This is an issue when side effects need to be triggered inside the update function such as notification mechanisms that an update was acknowledged.

It should be the choice of the update closure to determine whether to process the empty FilterSet or not.

Closes #3631

## Description

Please include a summary of the change.

Closes: #(issue-number)

## How Has This Been Tested?

Please describe any manual tests done to verify your changes.

## Guidelines

Please read and follow the `CONTRIBUTION` [guidelines] of this project.

[guidelines]: https://github.com/vmware/govmomi/blob/main/CONTRIBUTING.md


Patch:
diff --git a/property/collector.go b/property/collector.go
index 9ee78d0d..244796df 100644
--- a/property/collector.go
+++ b/property/collector.go
@@ -264,9 +264,12 @@ func (p *Collector) RetrieveOne(ctx context.Context, obj types.ManagedObjectRefe
 
 // WaitForUpdatesEx waits for any of the specified properties of the specified
 // managed object to change. It calls the specified function for every update it
-// receives. If this function returns false, it continues waiting for
+// receives an update - including the empty filter set, which can occur if no
+// objects are eligible for updates.
+//
+// If this function returns false, it continues waiting for
 // subsequent updates. If this function returns true, it stops waiting and
-// returns.
+// returns upon receiving the first non-empty filter set.
 //
 // If the Context is canceled, a call to CancelWaitForUpdates() is made and its
 // error value is returned.
@@ -313,6 +316,12 @@ func (p *Collector) WaitForUpdatesEx(
 			opts.Truncated = *set.Truncated
 		}
 
+		if len(set.FilterSet) == 0 {
+			// Trigger callbacks in case callers need to be notified
+			// of the empty filter set.
+			_ = onUpdatesFn(make([]types.ObjectUpdate, 0))
+		}
+
 		for _, fs := range set.FilterSet {
 			if opts.PropagateMissing {
 				for i := range fs.ObjectSet {


Test patch:
diff --git a/property/collector_test.go b/property/collector_test.go
index 06ca8c26..90b3ec7b 100644
--- a/property/collector_test.go
+++ b/property/collector_test.go
@@ -18,6 +18,7 @@ package property_test
 
 import (
 	"context"
+	"fmt"
 	"testing"
 	"time"
 
@@ -110,6 +111,64 @@ func TestWaitForUpdatesEx(t *testing.T) {
 	}, model)
 }
 
+func TestWaitForUpdatesExEmptyUpdateSet(t *testing.T) {
+	model := simulator.VPX()
+	model.Datacenter = 1
+	model.Cluster = 0
+	model.Pool = 0
+	model.Machine = 0
+	model.Autostart = false
+
+	simulator.Test(func(ctx context.Context, c *vim25.Client) {
+		// Set up the finder and get a VM.
+		finder := find.NewFinder(c, true)
+		datacenter, err := finder.DefaultDatacenter(ctx)
+		if err != nil {
+			t.Fatalf("default datacenter not found: %s", err.Error())
+		}
+		finder.SetDatacenter(datacenter)
+		vmList, err := finder.VirtualMachineList(ctx, "*")
+		if len(vmList) != 0 {
+			t.Fatalf("vmList != 0")
+		}
+
+		pc, err := property.DefaultCollector(c).Create(ctx)
+		if err != nil {
+			t.Fatalf("failed to create new property collector: %s", err.Error())
+		}
+
+		// Start a goroutine to wait for updates on any VMs.
+		// Since there are no VMs in the filter set, we expect to
+		// receive an empty update set.
+		chanResult := make(chan error)
+		cancelCtx, cancel := context.WithCancel(ctx)
+		go func() {
+			defer close(chanResult)
+			_ = pc.WaitForUpdatesEx(
+				cancelCtx,
+				&property.WaitOptions{},
+				func(updates []types.ObjectUpdate) bool {
+					var err error
+					if len(updates) > 0 {
+						err = fmt.Errorf("unexpected update")
+					}
+					chanResult <- err
+					cancel()
+					return true
+				})
+		}()
+
+		select {
+		case <-ctx.Done():
+			t.Fatalf("timed out while waiting for updates")
+		case err := <-chanResult:
+			if err != nil {
+				t.Fatalf("error while waiting for updates: %s", err.Error())
+			}
+		}
+	}, model)
+}
+
 func TestRetrievePropertiesOneAtATime(t *testing.T) {
 	model := simulator.VPX()
 	model.Datacenter = 1