Optimize

docs/style-guide/sdk.md

@@ -330,6 +330,22 @@ Public functions should follow this guide as much as possible, to realize a homo
 It is important to note that every public function becomes a critical point of reliance for some software.
 As such, it is imperative to uphold backward compatibility when making changes. See [Versioning](#versioning) for more information.
 
+## Types
+
+In the SDK, we use public and private types. This section will discuss the naming conventions for these types.
+
+### Public Types
+
+For now we don't have any specific naming conventions for public types. The only thing that is important that it conveys it's purpose, scope and usage.
+
+### Private Types
+
+For private types we use the following naming conventions:
+
+#### Enumerations
+
+For an internal enum type, the name must be suffixed with `Enum`.
+
 ## Standardized Interfaces
 
 Standardized interfaces are key in software development for consistent functionality exposure. They enhance code understanding, usage, and system interoperability.
@@ -414,11 +430,11 @@ func Test_UserID_Validate(t *testing.T) {
         input UserID
         output error
     }{
-        {name: "Valid ID",
+        {name: `Valid ID`,
             input: UserID(1),
             output: nil,
         },
-        {name: "Invalid ID",
+        {name: `Invalid ID`,
             input: UserID(0),
             output: errors.New(UserID_Error_Invalid),
         },

proxmox/config_group.go

@@ -16,7 +16,7 @@ type ConfigGroup struct {
 }
 
 // Creates the specified group
-func (config *ConfigGroup) Create(client *Client) error {
+func (config ConfigGroup) Create(client *Client) error {
 	config.Validate(true)
 	params := config.mapToApiValues(true)
 	err := client.Post(params, "/access/groups")
@@ -31,7 +31,7 @@ func (config *ConfigGroup) Create(client *Client) error {
 }
 
 // Maps the struct to the API values proxmox understands
-func (config *ConfigGroup) mapToApiValues(create bool) (params map[string]interface{}) {
+func (config ConfigGroup) mapToApiValues(create bool) (params map[string]interface{}) {
 	params = map[string]interface{}{
 		"comment": config.Comment,
 	}
@@ -54,20 +54,8 @@ func (config ConfigGroup) mapToStruct(params map[string]interface{}) *ConfigGrou
 	return &config
 }
 
-// Custom error for when the *ConfigGroup is nil
-func (config *ConfigGroup) nilCheck() error {
-	if config == nil {
-		return errors.New("pointer for (ConfigGroup) is nil")
-	}
-	return nil
-}
-
 // Created or updates the specified group
-func (config *ConfigGroup) Set(client *Client) (err error) {
-	err = config.nilCheck()
-	if err != nil {
-		return
-	}
+func (config ConfigGroup) Set(client *Client) (err error) {
 	existence, err := config.Name.CheckExistence(client)
 	if err != nil {
 		return
@@ -79,7 +67,7 @@ func (config *ConfigGroup) Set(client *Client) (err error) {
 }
 
 // Updates the specified group
-func (config *ConfigGroup) Update(client *Client) error {
+func (config ConfigGroup) Update(client *Client) error {
 	config.Validate(false)
 	params := config.mapToApiValues(true)
 	err := client.Put(params, "/access/groups/"+string(config.Name))
@@ -94,14 +82,10 @@ func (config *ConfigGroup) Update(client *Client) error {
 }
 
 // Validates all items and sub items of the ConfigGroup
-func (config *ConfigGroup) Validate(create bool) (err error) {
-	err = config.nilCheck()
-	if err != nil {
+func (config ConfigGroup) Validate(create bool) (err error) {
+	if err = config.Name.Validate(); err != nil {
 		return
 	}
-	if create {
-		err = config.Name.Validate()
-	}
 	if config.Members != nil {
 		for _, e := range *config.Members {
 			err = e.Validate()
@@ -116,6 +100,12 @@ func (config *ConfigGroup) Validate(create bool) (err error) {
 // GroupName may only contain the following characters: abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890-_
 type GroupName string
 
+const (
+	GroupName_Error_Invalid   string = "variable of type (GroupName) may only contain the following characters: -_abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890"
+	GroupName_Error_Empty     string = "variable of type (GroupName) may not be empty"
+	GroupName_Error_MaxLength string = "variable of type (GroupName) may not be more tha 1000 characters long"
+)
+
 // Add users to the specified group
 func (group GroupName) AddUsersToGroup(members *[]UserID, client *Client) error {
 	users, err := listUsersFull(client)
@@ -368,17 +358,17 @@ func (group GroupName) usersToRemoveFromGroup(allUsers []interface{}, members *[
 // Check if a groupname is valid.
 func (group GroupName) Validate() error {
 	if group == "" {
-		return errors.New("variable of type (GroupName) may not be empty")
+		return errors.New(GroupName_Error_Empty)
 	}
 	// proxmox does not seem to enforce any limit on the length of a group name. When going over thousands of charters the ui kinda breaks.
 	if len([]rune(group)) > 1000 {
-		return errors.New("variable of type (GroupName) may not be more tha 1000 characters long")
+		return errors.New(GroupName_Error_MaxLength)
 	}
 	regex, _ := regexp.Compile(`^([a-z]|[A-Z]|[0-9]|_|-)*$`)
-	if regex.Match([]byte(group)) {
-		return nil
+	if !regex.Match([]byte(group)) {
+		return errors.New(GroupName_Error_Invalid)
 	}
-	return errors.New("")
+	return nil
 }
 
 // Returns a list of all existing groups

proxmox/config_group_test.go

@@ -1,6 +1,7 @@
 package proxmox
 
 import (
+	"errors"
 	"testing"
 
 	"github.com/Telmate/proxmox-api-go/test/data/test_data_group"
@@ -114,72 +115,49 @@ func Test_ConfigGroup_mapToStruct(t *testing.T) {
 	}
 }
 
-func Test_ConfigGroup_nilCheck(t *testing.T) {
-	testData := []struct {
-		input *ConfigGroup
-		err   bool
-	}{
-		{input: &ConfigGroup{}},
-		{err: true},
-	}
-	for _, e := range testData {
-		if e.err {
-			require.Error(t, e.input.nilCheck())
-		} else {
-			require.NoError(t, e.input.nilCheck())
-		}
-	}
-}
-
 // TODO improve when Name and Realm have their own types
 func Test_ConfigGroup_Validate(t *testing.T) {
 	validGroupName := GroupName("groupName")
 	False := 0
 	TrueAndFalse := 1
 	True := 2
 	testData := []struct {
-		input  *ConfigGroup
-		err    bool
+		input    ConfigGroup
+		hasError bool
+		err      error
+
 		create int
 	}{
 		// GroupName
 		{
-			err:    true,
-			create: TrueAndFalse,
-		},
+			input:  ConfigGroup{},
+			err:    errors.New(GroupName_Error_Empty),
+			create: TrueAndFalse},
 		{
-			input:  &ConfigGroup{},
-			err:    true,
-			create: True,
-		},
-		{input: &ConfigGroup{}},
+			input:  ConfigGroup{Name: GroupName(test_data_group.GroupName_Max_Legal())},
+			create: TrueAndFalse},
 		{
-			input:  &ConfigGroup{Name: GroupName(test_data_group.GroupName_Max_Legal())},
-			create: True,
-		},
-		{
-			input:  &ConfigGroup{Name: GroupName(test_data_group.GroupName_Max_Illegal())},
-			err:    true,
-			create: True,
-		},
+			input:  ConfigGroup{Name: GroupName(test_data_group.GroupName_Max_Illegal())},
+			err:    errors.New(GroupName_Error_MaxLength),
+			create: TrueAndFalse},
 		// GroupMembers
 		{
-			input: &ConfigGroup{
+			input: ConfigGroup{
 				Name: validGroupName,
 				Members: &[]UserID{
 					{Name: "user1"},
 				}},
-			err:    true,
-			create: TrueAndFalse,
+			hasError: true,
+			create:   TrueAndFalse,
 		},
 		{
-			input: &ConfigGroup{
+			input: ConfigGroup{
 				Name:    validGroupName,
 				Members: &[]UserID{{Name: "user1", Realm: "pam"}}},
 			create: TrueAndFalse,
 		},
 		{
-			input: &ConfigGroup{
+			input: ConfigGroup{
 				Name: validGroupName,
 				Members: &[]UserID{
 					{Name: "user1", Realm: "pam"},
@@ -191,14 +169,18 @@ func Test_ConfigGroup_Validate(t *testing.T) {
 	}
 	for _, e := range testData {
 		if e.create < True {
-			if e.err {
+			if e.err != nil {
+				require.Equal(t, e.err, e.input.Validate(false))
+			} else if e.hasError {
 				require.Error(t, e.input.Validate(false))
 			} else {
 				require.NoError(t, e.input.Validate(false))
 			}
 		}
 		if e.create > False {
-			if e.err {
+			if e.err != nil {
+				require.Equal(t, e.err, e.input.Validate(false))
+			} else if e.hasError {
 				require.Error(t, e.input.Validate(true))
 			} else {
 				require.NoError(t, e.input.Validate(true))

proxmox/config_qemu.go

@@ -17,13 +17,11 @@ import (
 // Currently ZFS local, LVM, Ceph RBD, CephFS, Directory and virtio-scsi-pci are considered.
 // Other formats are not verified, but could be added if they're needed.
 // const rxStorageTypes = `(zfspool|lvm|rbd|cephfs|dir|virtio-scsi-pci)`
-const machineModels = `(pc|q35|pc-i440fx)`
 
 type (
 	QemuDevices     map[int]map[string]interface{}
 	QemuDevice      map[string]interface{}
 	QemuDeviceParam []string
-	IpconfigMap     map[int]interface{}
 )
 
 // ConfigQemu - Proxmox API QEMU options
@@ -1046,15 +1044,6 @@ func FormatDiskParam(disk QemuDevice) string {
 	return strings.Join(diskConfParam, ",")
 }
 
-// Given a QemuDevice (representing a usb), return a param string to give to ProxMox
-func FormatUsbParam(usb QemuDevice) string {
-	usbConfParam := QemuDeviceParam{}
-
-	usbConfParam = usbConfParam.createDeviceParam(usb, []string{})
-
-	return strings.Join(usbConfParam, ",")
-}
-
 // Create RNG parameter.
 func (c ConfigQemu) CreateQemuRngParams(params map[string]interface{}) {
 	rngParam := QemuDeviceParam{}
@@ -1098,24 +1087,6 @@ func (c ConfigQemu) CreateQemuEfiParams(params map[string]interface{}) {
 	}
 }
 
-// Create parameters for each disk.
-func (c ConfigQemu) CreateQemuDisksParams(params map[string]interface{}, cloned bool) {
-	// For new style with multi disk device.
-	for diskID, diskConfMap := range c.QemuDisks {
-		// skip the first disk for clones (may not always be right, but a template probably has at least 1 disk)
-		if diskID == 0 && cloned {
-			continue
-		}
-
-		// Device name.
-		deviceType := diskConfMap["type"].(string)
-		qemuDiskName := deviceType + strconv.Itoa(diskID)
-
-		// Add back to Qemu prams.
-		params[qemuDiskName] = FormatDiskParam(diskConfMap)
-	}
-}
-
 // Create parameters for each PCI Device
 func (c ConfigQemu) CreateQemuPCIsParams(params map[string]interface{}) {
 	// For new style with multi pci device.
@@ -1134,20 +1105,6 @@ func (c ConfigQemu) CreateQemuPCIsParams(params map[string]interface{}) {
 	}
 }
 
-// Create parameters for serial interface
-func (c ConfigQemu) CreateQemuMachineParam(
-	params map[string]interface{},
-) error {
-	if c.Machine == "" {
-		return nil
-	}
-	if matched, _ := regexp.MatchString(machineModels, c.Machine); matched {
-		params["machine"] = c.Machine
-		return nil
-	}
-	return fmt.Errorf("unsupported machine type, fall back to default")
-}
-
 func (p QemuDeviceParam) createDeviceParam(
 	deviceConfMap QemuDevice,
 	ignoredKeys []string,

proxmox/config_qemu_test.go

@@ -8472,7 +8472,7 @@ func Test_ConfigQemu_Validate(t *testing.T) {
 							QemuUsbID0: QemuUSB{Mapping: &QemuUsbMapping{}}}}),
 						current: &ConfigQemu{USBs: QemuUSBs{
 							QemuUsbID0: QemuUSB{Port: &QemuUsbPort{}}}},
-						err: errors.New(QemuUSB_Error_MappedID)},
+						err: errors.New(QemuUSB_Error_MappingID)},
 					{name: `errors.New(QemuUSB_Error_PortID)`,
 						input: baseConfig(ConfigQemu{USBs: QemuUSBs{
 							QemuUsbID0: QemuUSB{Port: &QemuUsbPort{}}}}),