Repository: vmware/govmomi
Pull request #3049 (2023-02-15 23:07:56)

Use string pointer for library item/library description field.
This commit changes library item description field from string to a string pointer. 
This can help distinguish if the item description is unset or set to an empty string when updating the library item.

BREAKING: change Item.Description type from string to *string in package library.

## Description

This commit changes library item description field from string to a string pointer. 
This can help distinguish if the item description is unset or set to an empty string 
when updating the library item.

## Type of change

Please mark options that are relevant:

- [ ] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [X] Breaking change (fix or feature that would cause existing functionality to
  not work as expected)
- [ ] This change requires a documentation update
- [ ] Build related change

## How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide
instructions so we can reproduce. If applicable, please also list any relevant
details for your test configuration.

- [ ] Test against a VC testbed
Sending a UpdateLibraryItem request with this change, and set the new description to "". 
The original item description was "virtualmachinepublishrequest.vmware.com: 6b0d06ad-8e18-4881-a806-2a7c8a7e1b4b",
after this UpdateLibraryItem request succeeded, the item description was set to "".
While before this change, it would be a no-op for the description.

- [ ] Test using govc 
Update Library
```
Create a library: 
➜  govmomi git:(issue-3048) ✗ govc library.create -d "description-1" cl-1
495f611a-cf74-40d6-a54a-8b239cf15406
➜  govmomi git:(issue-3048) ✗ ~/go/bin/govc library.info 495f611a-cf74-40d6-a54a-8b239cf15406
Name:                 cl-1
  ID:                 495f611a-cf74-40d6-a54a-8b239cf15406
  Path:               /cl-1
  Description:        description-1
  Version:            2
  Created:            Fri Feb 17 19:18:22 2023
  Security Policy ID
  StorageBackings:
    DatastoreID:      sharedVmfs-0
    Type:             DATASTORE

Update the library description with new string:
➜  govmomi git:(issue-3048) ✗ ~/go/bin/govc library.update -d "new description" 495f611a-cf74-40d6-a54a-8b239cf15406
➜  govmomi git:(issue-3048) ✗ ~/go/bin/govc library.info 495f611a-cf74-40d6-a54a-8b239cf15406
Name:                 cl-1
  ID:                 495f611a-cf74-40d6-a54a-8b239cf15406
  Path:               /cl-1
  Description:        new description
  Version:            3
  Created:            Fri Feb 17 19:18:22 2023
  Security Policy ID
  StorageBackings:
    DatastoreID:      sharedVmfs-0
    Type:             DATASTORE

Update the item name, description is not erased.
➜  govmomi git:(issue-3048) ✗ ~/go/bin/govc library.update -n "cl-1-new" 495f611a-cf74-40d6-a54a-8b239cf15406
➜  govmomi git:(issue-3048) ✗ ~/go/bin/govc library.info 495f611a-cf74-40d6-a54a-8b239cf15406
Name:                 cl-1-new
  ID:                 495f611a-cf74-40d6-a54a-8b239cf15406
  Path:               /cl-1-new
  Description:        new description
  Version:            4
  Created:            Fri Feb 17 19:18:22 2023
  Security Policy ID
  StorageBackings:
    DatastoreID:      sharedVmfs-0
    Type:             DATASTORE

Update the library description with empty string:
➜  govmomi git:(issue-3048) ✗ ~/go/bin/govc library.update -d ""
495f611a-cf74-40d6-a54a-8b239cf15406
➜  govmomi git:(issue-3048) ✗ ~/go/bin/govc library.info 495f611a-cf74-40d6-a54a-8b239cf15406
Name:                 cl-1-new
  ID:                 495f611a-cf74-40d6-a54a-8b239cf15406
  Path:               /cl-1-new
  Description:
  Version:            5
  Created:            Fri Feb 17 19:18:22 2023
  Security Policy ID
  StorageBackings:
    DatastoreID:      sharedVmfs-0
    Type:             DATASTORE
```
Update Library Item
```
➜  govmomi git:(issue-3048) ✗ ~/go/bin/govc library.update -d "item-2" /cl-1/image-2
➜  govmomi git:(issue-3048) ✗ ~/go/bin/govc library.info /cl-1/image-2
Name:                   image-2
  ID:                   0dc7df56-31e0-47dc-8b0a-6a33279ddccc
  Path:                 /cl-1/image-2
  Description:          item-2
  Type:                 ovf
  Size:                 2.0GB
  Created:              Wed Feb 15 23:05:15 2023
  Modified:             Fri Feb 17 19:29:54 2023
  Version:              3
  Security Compliance:  true
  Certificate Status:   INTERNAL

Update item name:
govmomi git:(issue-3048) ✗ ~/go/bin/govc library.update -n "image-2-new" /cl-1/image-2
➜  govmomi git:(issue-3048) ✗ ~/go/bin/govc library.info /cl-1/image-2-new
Name:                   image-2-new
  ID:                   0dc7df56-31e0-47dc-8b0a-6a33279ddccc
  Path:                 /cl-1/image-2-new
  Description:          item-2
  Type:                 ovf
  Size:                 2.0GB
  Created:              Wed Feb 15 23:05:15 2023
  Modified:             Fri Feb 17 19:31:44 2023
  Version:              4
  Security Compliance:  true
  Certificate Status:   INTERNAL

Update item description to empty string
➜  govmomi git:(issue-3048) ✗ ~/go/bin/govc library.update -d "" /cl-1/image-2-new
➜  govmomi git:(issue-3048) ✗ ~/go/bin/govc library.info /cl-1/image-2-new
Name:                   image-2-new
  ID:                   0dc7df56-31e0-47dc-8b0a-6a33279ddccc
  Path:                 /cl-1/image-2-new
  Description:
  Type:                 ovf
  Size:                 2.0GB
  Created:              Wed Feb 15 23:05:15 2023
  Modified:             Fri Feb 17 19:32:56 2023
  Version:              5
  Security Compliance:  true
  Certificate Status:   INTERNAL
```

## Checklist:

- [X] My code follows the `CONTRIBUTION`
  [guidelines](https://github.com/vmware/govmomi/blob/main/CONTRIBUTING.md) of
  this project
- [X] I have commented my code, particularly in hard-to-understand areas
- [X] I have made corresponding changes to the documentation
- [X] I have added tests that prove my fix is effective or that my feature works
- [X] New and existing unit tests pass locally with my changes
- [X] Any dependent changes have been merged


Patch:
diff --git a/govc/USAGE.md b/govc/USAGE.md
index 9bcecbf0..58759d86 100644
--- a/govc/USAGE.md
+++ b/govc/USAGE.md
@@ -3228,7 +3228,7 @@ Examples:
   govc library.create -pub library_name
 
 Options:
-  -d=                    Description of library
+  -d=<nil>               Description of library
   -ds=                   Datastore [GOVC_DATASTORE]
   -policy=               Security Policy ID
   -pub=<nil>             Publish library
@@ -3580,7 +3580,7 @@ Examples:
   govc library.update -d "new item description" -n "new-item-name" my-library/my-item
 
 Options:
-  -d=                    Library or item description
+  -d=<nil>               Library or item description
   -n=                    Library or item name
 ```
 
diff --git a/govc/flags/optional_string.go b/govc/flags/optional_string.go
new file mode 100644
index 00000000..0460785d
--- /dev/null
+++ b/govc/flags/optional_string.go
@@ -0,0 +1,50 @@
+/*
+Copyright (c) 2023-2023 VMware, Inc. All Rights Reserved.
+
+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 flags
+
+import (
+	"flag"
+)
+
+type optionalString struct {
+	val **string
+}
+
+func (s *optionalString) Set(input string) error {
+	*s.val = &input
+	return nil
+}
+
+func (s *optionalString) Get() interface{} {
+	if *s.val == nil {
+		return nil
+	}
+	return **s.val
+}
+
+func (s *optionalString) String() string {
+	if s.val == nil || *s.val == nil {
+		return "<nil>"
+	}
+	return **s.val
+}
+
+// NewOptionalString returns a flag.Value implementation where there is no default value.
+// This avoids sending a default value over the wire as using flag.StringVar() would.
+func NewOptionalString(v **string) flag.Value {
+	return &optionalString{v}
+}
diff --git a/govc/library/create.go b/govc/library/create.go
index 2d6dc195..c1e8f8a7 100644
--- a/govc/library/create.go
+++ b/govc/library/create.go
@@ -44,7 +44,7 @@ func (cmd *create) Register(ctx context.Context, f *flag.FlagSet) {
 	cmd.sub.AutomaticSyncEnabled = new(bool)
 	cmd.sub.OnDemand = new(bool)
 
-	f.StringVar(&cmd.library.Description, "d", "", "Description of library")
+	f.Var(flags.NewOptionalString(&cmd.library.Description), "d", "Description of library")
 	f.StringVar(&cmd.sub.SubscriptionURL, "sub", "", "Subscribe to library URL")
 	f.StringVar(&cmd.sub.UserName, "sub-username", "", "Subscription username")
 	f.StringVar(&cmd.sub.Password, "sub-password", "", "Subscription password")
diff --git a/govc/library/info.go b/govc/library/info.go
index 1f59eaf6..11e25d8c 100644
--- a/govc/library/info.go
+++ b/govc/library/info.go
@@ -168,7 +168,9 @@ func (r infoResultsWriter) writeLibrary(
 	fmt.Fprintf(w, "Name:\t%s\n", v.Name)
 	fmt.Fprintf(w, "  ID:\t%s\n", v.ID)
 	fmt.Fprintf(w, "  Path:\t%s\n", res.GetPath())
-	fmt.Fprintf(w, "  Description:\t%s\n", v.Description)
+	if v.Description != nil {
+		fmt.Fprintf(w, "  Description:\t%s\n", *v.Description)
+	}
 	fmt.Fprintf(w, "  Version:\t%s\n", v.Version)
 	fmt.Fprintf(w, "  Created:\t%s\n", v.CreationTime.Format(time.ANSIC))
 	fmt.Fprintf(w, "  Security Policy ID\t%s\n", v.SecurityPolicyID)
@@ -216,7 +218,9 @@ func (r infoResultsWriter) writeItem(
 	fmt.Fprintf(w, "Name:\t%s\n", v.Name)
 	fmt.Fprintf(w, "  ID:\t%s\n", v.ID)
 	fmt.Fprintf(w, "  Path:\t%s\n", res.GetPath())
-	fmt.Fprintf(w, "  Description:\t%s\n", v.Description)
+	if v.Description != nil {
+		fmt.Fprintf(w, "  Description:\t%s\n", *v.Description)
+	}
 	fmt.Fprintf(w, "  Type:\t%s\n", v.Type)
 	fmt.Fprintf(w, "  Size:\t%s\n", units.ByteSize(v.Size))
 	fmt.Fprintf(w, "  Created:\t%s\n", v.CreationTime.Format(time.ANSIC))
diff --git a/govc/library/update.go b/govc/library/update.go
index 570bfaf6..f0e832dc 100644
--- a/govc/library/update.go
+++ b/govc/library/update.go
@@ -1,5 +1,5 @@
 /*
-Copyright (c) 2021 VMware, Inc. All Rights Reserved.
+Copyright (c) 2021-2023 VMware, Inc. All Rights Reserved.
 
 Licensed under the Apache License, Version 2.0 (the "License");
 you may not use this file except in compliance with the License.
@@ -29,7 +29,8 @@ import (
 type update struct {
 	*flags.ClientFlag
 
-	name, desc string
+	name string
+	desc *string
 }
 
 func init() {
@@ -41,7 +42,7 @@ func (cmd *update) Register(ctx context.Context, f *flag.FlagSet) {
 	cmd.ClientFlag.Register(ctx, f)
 
 	f.StringVar(&cmd.name, "n", "", "Library or item name")
-	f.StringVar(&cmd.desc, "d", "", "Library or item description")
+	f.Var(flags.NewOptionalString(&cmd.desc), "d", "Library or item description")
 }
 
 func (cmd *update) Usage() string {
@@ -76,17 +77,21 @@ func (cmd *update) Run(ctx context.Context, f *flag.FlagSet) error {
 	switch t := res.GetResult().(type) {
 	case library.Library:
 		lib := &library.Library{
-			ID:          t.ID,
-			Name:        cmd.name,
-			Description: cmd.desc,
+			ID:   t.ID,
+			Name: cmd.name,
+		}
+		if cmd.desc != nil {
+			lib.Description = cmd.desc
 		}
 		t.Patch(lib)
 		return m.UpdateLibrary(ctx, &t)
 	case library.Item:
 		item := &library.Item{
-			ID:          t.ID,
-			Name:        cmd.name,
-			Description: cmd.desc,
+			ID:   t.ID,
+			Name: cmd.name,
+		}
+		if cmd.desc != nil {
+			item.Description = cmd.desc
 		}
 		t.Patch(item)
 		return m.UpdateLibraryItem(ctx, item)
diff --git a/vapi/library/library.go b/vapi/library/library.go
index e5e5d089..62ad873b 100644
--- a/vapi/library/library.go
+++ b/vapi/library/library.go
@@ -1,5 +1,5 @@
 /*
-Copyright (c) 2018 VMware, Inc. All Rights Reserved.
+Copyright (c) 2018-2023 VMware, Inc. All Rights Reserved.
 
 Licensed under the Apache License, Version 2.0 (the "License");
 you may not use this file except in compliance with the License.
@@ -37,7 +37,7 @@ type StorageBackings struct {
 // Library  provides methods to create, read, update, delete, and enumerate libraries.
 type Library struct {
 	CreationTime          *time.Time        `json:"creation_time,omitempty"`
-	Description           string            `json:"description,omitempty"`
+	Description           *string           `json:"description,omitempty"`
 	ID                    string            `json:"id,omitempty"`
 	LastModifiedTime      *time.Time        `json:"last_modified_time,omitempty"`
 	LastSyncTime          *time.Time        `json:"last_sync_time,omitempty"`
@@ -120,7 +120,7 @@ func (l *Library) Patch(src *Library) {
 	if src.Name != "" {
 		l.Name = src.Name
 	}
-	if src.Description != "" {
+	if src.Description != nil {
 		l.Description = src.Description
 	}
 	if src.Version != "" {
diff --git a/vapi/library/library_item.go b/vapi/library/library_item.go
index adc2101a..e143da5d 100644
--- a/vapi/library/library_item.go
+++ b/vapi/library/library_item.go
@@ -36,7 +36,7 @@ type Item struct {
 	Cached           bool       `json:"cached,omitempty"`
 	ContentVersion   string     `json:"content_version,omitempty"`
 	CreationTime     *time.Time `json:"creation_time,omitempty"`
-	Description      string     `json:"description,omitempty"`
+	Description      *string    `json:"description,omitempty"`
 	ID               string     `json:"id,omitempty"`
 	LastModifiedTime *time.Time `json:"last_modified_time,omitempty"`
 	LastSyncTime     *time.Time `json:"last_sync_time,omitempty"`
@@ -63,7 +63,7 @@ func (i *Item) Patch(src *Item) {
 	if src.Name != "" {
 		i.Name = src.Name
 	}
-	if src.Description != "" {
+	if src.Description != nil {
 		i.Description = src.Description
 	}
 	if src.Type != "" {
@@ -82,12 +82,17 @@ func (c *Manager) CreateLibraryItem(ctx context.Context, item Item) (string, err
 		LibraryID   string `json:"library_id,omitempty"`
 		Type        string `json:"type"`
 	}
+
+	description := ""
+	if item.Description != nil {
+		description = *item.Description
+	}
 	spec := struct {
 		Item createItemSpec `json:"create_spec"`
 	}{
 		Item: createItemSpec{
 			Name:        item.Name,
-			Description: item.Description,
+			Description: description,
 			LibraryID:   item.LibraryID,
 			Type:        item.Type,
 		},
diff --git a/vapi/simulator/simulator.go b/vapi/simulator/simulator.go
index c43494f2..3ef5233e 100644
--- a/vapi/simulator/simulator.go
+++ b/vapi/simulator/simulator.go
@@ -1865,7 +1865,7 @@ func (s *handler) libraryItemOVF(w http.ResponseWriter, r *http.Request) {
 				ID:               id,
 				LibraryID:        l.Library.ID,
 				Name:             req.Spec.Name,
-				Description:      req.Spec.Description,
+				Description:      &req.Spec.Description,
 				Type:             library.ItemTypeOVF,
 				CreationTime:     types.NewTime(time.Now()),
 				LastModifiedTime: types.NewTime(time.Now()),


Test patch:
diff --git a/govc/flags/optional_string_test.go b/govc/flags/optional_string_test.go
new file mode 100644
index 00000000..42c99d15
--- /dev/null
+++ b/govc/flags/optional_string_test.go
@@ -0,0 +1,57 @@
+/*
+Copyright (c) 2023-2023 VMware, Inc. All Rights Reserved.
+
+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 flags
+
+import (
+	"flag"
+	"testing"
+)
+
+func TestOptionalString(t *testing.T) {
+	fs := flag.NewFlagSet("", flag.ContinueOnError)
+	var val *string
+
+	fs.Var(NewOptionalString(&val), "ostring", "optional string")
+
+	s := fs.Lookup("ostring")
+
+	if s.DefValue != "<nil>" {
+		t.Fail()
+	}
+
+	if s.Value.String() != "<nil>" {
+		t.Fail()
+	}
+
+	if s.Value.(flag.Getter).Get() != nil {
+		t.Fail()
+	}
+
+	s.Value.Set("test")
+
+	if s.Value.String() != "test" {
+		t.Fail()
+	}
+
+	if s.Value.(flag.Getter).Get() != "test" {
+		t.Fail()
+	}
+
+	if val == nil || *val != "test" {
+		t.Fail()
+	}
+}