Repository: vmware/govmomi
Pull request #3256 (2023-10-10 02:09:32)

vcsim: fix ModifyListView to return unresolved references
api: add ListView.Modify method, returns unresolved references



Patch:
diff --git a/go.mod b/go.mod
index 3fb12670..028de512 100644
--- a/go.mod
+++ b/go.mod
@@ -5,12 +5,11 @@ go 1.19
 require (
 	github.com/a8m/tree v0.0.0-20210115125333-10a5fd5b637d
 	github.com/dougm/pretty v0.0.0-20171025230240-2ee9d7453c02
-	github.com/google/go-cmp v0.6.0
+	github.com/google/go-cmp v0.5.9
 	github.com/google/uuid v1.3.1
 	github.com/rasky/go-xdr v0.0.0-20170217172119-4930550ba2e2
 	github.com/stretchr/testify v1.8.4
 	github.com/vmware/vmw-guestinfo v0.0.0-20170707015358-25eff159a728
-	github.com/xlab/treeprint v1.2.0
 )
 
 require (
@@ -18,6 +17,7 @@ require (
 	github.com/kr/pretty v0.3.0 // indirect
 	github.com/kr/text v0.2.0 // indirect
 	github.com/pmezard/go-difflib v1.0.0 // indirect
+	github.com/xlab/treeprint v1.2.0 // indirect
 	gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127 // indirect
 	gopkg.in/yaml.v3 v3.0.1 // indirect
 )
diff --git a/go.sum b/go.sum
index 470da59c..3c8012c0 100644
--- a/go.sum
+++ b/go.sum
@@ -6,8 +6,8 @@ github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c
 github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
 github.com/dougm/pretty v0.0.0-20171025230240-2ee9d7453c02 h1:tR3jsKPiO/mb6ntzk/dJlHZtm37CPfVp1C9KIo534+4=
 github.com/dougm/pretty v0.0.0-20171025230240-2ee9d7453c02/go.mod h1:7NQ3kWOx2cZOSjtcveTa5nqupVr2s6/83sG+rTlI7uA=
-github.com/google/go-cmp v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI=
-github.com/google/go-cmp v0.6.0/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY=
+github.com/google/go-cmp v0.5.9 h1:O2Tfq5qg4qc4AmwVlvv0oLiVAGB7enBSJ2x2DqQFi38=
+github.com/google/go-cmp v0.5.9/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY=
 github.com/google/uuid v1.3.1 h1:KjJaJ9iWZ3jOFZIf1Lqf4laDRCasjl0BCmnEGxkdLb4=
 github.com/google/uuid v1.3.1/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo=
 github.com/kr/pretty v0.1.0/go.mod h1:dAy3ld7l9f0ibDNOQOHHMYYIIbhfbHSm3C4ZsoJORNo=
diff --git a/simulator/view_manager.go b/simulator/view_manager.go
index 842ff53d..94e1d526 100644
--- a/simulator/view_manager.go
+++ b/simulator/view_manager.go
@@ -227,8 +227,8 @@ func (m *ViewManager) CreateListView(ctx *Context, req *types.CreateListView) so
 	body := new(methods.CreateListViewBody)
 	list := new(ListView)
 
-	if err := list.add(ctx, req.Obj); err != nil {
-		body.Fault_ = Fault("", err)
+	if refs := list.add(ctx, req.Obj); len(refs) != 0 {
+		body.Fault_ = Fault("", &types.ManagedObjectNotFound{Obj: refs[0]})
 		return body
 	}
 
@@ -249,15 +249,16 @@ func (v *ListView) update(ctx *Context) {
 	ctx.Map.Update(v, []types.PropertyChange{{Name: "view", Val: v.View}})
 }
 
-func (v *ListView) add(ctx *Context, refs []types.ManagedObjectReference) *types.ManagedObjectNotFound {
+func (v *ListView) add(ctx *Context, refs []types.ManagedObjectReference) []types.ManagedObjectReference {
+	var unresolved []types.ManagedObjectReference
 	for _, ref := range refs {
 		obj := ctx.Session.Get(ref)
 		if obj == nil {
-			return &types.ManagedObjectNotFound{Obj: ref}
+			unresolved = append(unresolved, ref)
 		}
 		v.View = append(v.View, ref)
 	}
-	return nil
+	return unresolved
 }
 
 func (v *ListView) DestroyView(ctx *Context, c *types.DestroyView) soap.HasFault {
@@ -266,19 +267,19 @@ func (v *ListView) DestroyView(ctx *Context, c *types.DestroyView) soap.HasFault
 }
 
 func (v *ListView) ModifyListView(ctx *Context, req *types.ModifyListView) soap.HasFault {
-	body := new(methods.ModifyListViewBody)
+	body := &methods.ModifyListViewBody{
+		Res: new(types.ModifyListViewResponse),
+	}
+
+	body.Res.Returnval = v.add(ctx, req.Add)
 
 	for _, ref := range req.Remove {
 		RemoveReference(&v.View, ref)
+		if ctx.Map.Get(ref) == nil {
+			body.Res.Returnval = append(body.Res.Returnval, ref)
+		}
 	}
 
-	if err := v.add(ctx, req.Add); err != nil {
-		body.Fault_ = Fault("", err)
-		return body
-	}
-
-	body.Res = new(types.ModifyListViewResponse)
-
 	if len(req.Remove) != 0 || len(req.Add) != 0 {
 		v.update(ctx)
 	}
@@ -287,16 +288,13 @@ func (v *ListView) ModifyListView(ctx *Context, req *types.ModifyListView) soap.
 }
 
 func (v *ListView) ResetListView(ctx *Context, req *types.ResetListView) soap.HasFault {
-	body := new(methods.ResetListViewBody)
+	body := &methods.ResetListViewBody{
+		Res: new(types.ResetListViewResponse),
+	}
 
 	v.View = nil
 
-	if err := v.add(ctx, req.Obj); err != nil {
-		body.Fault_ = Fault("", err)
-		return body
-	}
-
-	body.Res = new(types.ResetListViewResponse)
+	body.Res.Returnval = v.add(ctx, req.Obj)
 
 	v.update(ctx)
 
diff --git a/view/list_view.go b/view/list_view.go
index e8cfb504..4958941c 100644
--- a/view/list_view.go
+++ b/view/list_view.go
@@ -34,29 +34,41 @@ func NewListView(c *vim25.Client, ref types.ManagedObjectReference) *ListView {
 	}
 }
 
-func (v ListView) Add(ctx context.Context, refs []types.ManagedObjectReference) error {
+func (v ListView) Add(ctx context.Context, refs []types.ManagedObjectReference) ([]types.ManagedObjectReference, error) {
 	req := types.ModifyListView{
 		This: v.Reference(),
 		Add:  refs,
 	}
-	_, err := methods.ModifyListView(ctx, v.Client(), &req)
-	return err
+	res, err := methods.ModifyListView(ctx, v.Client(), &req)
+	if err != nil {
+		return nil, err
+	}
+
+	return res.Returnval, nil
 }
 
-func (v ListView) Remove(ctx context.Context, refs []types.ManagedObjectReference) error {
+func (v ListView) Remove(ctx context.Context, refs []types.ManagedObjectReference) ([]types.ManagedObjectReference, error) {
 	req := types.ModifyListView{
 		This:   v.Reference(),
 		Remove: refs,
 	}
-	_, err := methods.ModifyListView(ctx, v.Client(), &req)
-	return err
+	res, err := methods.ModifyListView(ctx, v.Client(), &req)
+	if err != nil {
+		return nil, err
+	}
+
+	return res.Returnval, nil
 }
 
-func (v ListView) Reset(ctx context.Context, refs []types.ManagedObjectReference) error {
+func (v ListView) Reset(ctx context.Context, refs []types.ManagedObjectReference) ([]types.ManagedObjectReference, error) {
 	req := types.ResetListView{
 		This: v.Reference(),
 		Obj:  refs,
 	}
-	_, err := methods.ResetListView(ctx, v.Client(), &req)
-	return err
+	res, err := methods.ResetListView(ctx, v.Client(), &req)
+	if err != nil {
+		return nil, err
+	}
+
+	return res.Returnval, nil
 }
diff --git a/view/task_view.go b/view/task_view.go
index 68f62f8d..4e1beef3 100644
--- a/view/task_view.go
+++ b/view/task_view.go
@@ -73,7 +73,7 @@ func (v TaskView) Collect(ctx context.Context, f func([]types.TaskInfo)) error {
 					tasks = change.Val.(types.ArrayOfManagedObjectReference).ManagedObjectReference
 					if len(tasks) != 0 {
 						reset = func() {
-							_ = v.Reset(ctx, tasks)
+							_, _ = v.Reset(ctx, tasks)
 
 							// Remember any tasks we've reported as complete already,
 							// to avoid reporting multiple times when Reset is triggered.
@@ -113,7 +113,7 @@ func (v TaskView) Collect(ctx context.Context, f func([]types.TaskInfo)) error {
 		if reset != nil {
 			reset()
 		} else if len(prune) != 0 {
-			_ = v.Remove(ctx, prune)
+			_, _ = v.Remove(ctx, prune)
 		}
 
 		if len(tasks) != 0 && len(infos) == 0 {


Test patch:
diff --git a/simulator/race_test.go b/simulator/race_test.go
index 0daabca1..2f4bc318 100644
--- a/simulator/race_test.go
+++ b/simulator/race_test.go
@@ -187,7 +187,7 @@ func TestRace(t *testing.T) {
 					if terr != nil {
 						t.Error(terr)
 					}
-					terr = lv.Add(ctx, []types.ManagedObjectReference{r.Result.(types.ManagedObjectReference)})
+					_, terr = lv.Add(ctx, []types.ManagedObjectReference{r.Result.(types.ManagedObjectReference)})
 					if terr != nil {
 						t.Error(terr)
 					}
@@ -221,7 +221,7 @@ func TestRace(t *testing.T) {
 						time.AfterFunc(100*time.Millisecond, func() {
 							defer wg.Done()
 
-							_ = lv.Remove(ctx, []types.ManagedObjectReference{vm.Reference()})
+							_, _ = lv.Remove(ctx, []types.ManagedObjectReference{vm.Reference()})
 							task, _ := vm.PowerOff(ctx)
 							_ = task.Wait(ctx)
 						})
diff --git a/simulator/view_manager_test.go b/simulator/view_manager_test.go
index 36ed522d..8d3d68ba 100644
--- a/simulator/view_manager_test.go
+++ b/simulator/view_manager_test.go
@@ -18,6 +18,7 @@ package simulator
 
 import (
 	"context"
+	"log"
 	"sync"
 	"testing"
 
@@ -198,3 +199,28 @@ func TestViewManager_CreateContainerView(t *testing.T) {
 		t.Errorf("Failed to run simulation: %s", err.Error())
 	}
 }
+
+func TestListViewModify(t *testing.T) {
+	Test(func(ctx context.Context, c *vim25.Client) {
+		list, err := view.NewManager(c).CreateListView(ctx, nil)
+		if err != nil {
+			log.Fatal(err)
+		}
+
+		defer list.Destroy(ctx)
+
+		add := []types.ManagedObjectReference{
+			c.ServiceContent.RootFolder,
+			{Type: "Folder", Value: "invalid"},
+		}
+
+		refs, err := list.Add(ctx, add)
+		if err != nil {
+			log.Fatal(err)
+		}
+
+		if len(refs) != 1 {
+			t.Errorf("unresolved refs=%s", refs)
+		}
+	})
+}
diff --git a/view/example_test.go b/view/example_test.go
index 529c9b3e..18353831 100644
--- a/view/example_test.go
+++ b/view/example_test.go
@@ -211,7 +211,7 @@ func ExampleListView_tasks() {
 
 						switch info.State {
 						case types.TaskInfoStateSuccess, types.TaskInfoStateError:
-							_ = list.Remove(ctx, []types.ManagedObjectReference{update.Obj})
+							_, _ = list.Remove(ctx, []types.ManagedObjectReference{update.Obj})
 							result[info.State]++
 							n--
 							wg.Done()
@@ -228,7 +228,7 @@ func ExampleListView_tasks() {
 			if err != nil {
 				return err
 			}
-			err = list.Add(ctx, []types.ManagedObjectReference{task.Reference()})
+			_, err = list.Add(ctx, []types.ManagedObjectReference{task.Reference()})
 			if err != nil {
 				return err
 			}