fix: Property Collector updates should be triggered on the empty Filt…

…erSet

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

Signed-off-by: Brian Rak <[email protected]>

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 {

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