From d9a9a338ae67b5ecedad1a3575fc9f6554d1b99a Mon Sep 17 00:00:00 2001 From: Tinyblargon <76069640+Tinyblargon@users.noreply.github.com> Date: Wed, 6 Dec 2023 10:03:10 +0000 Subject: [PATCH 1/2] refactor: change to standard stringer interface Use standard stringer interface instead of `toString()` --- main.go | 6 ++-- proxmox/client.go | 2 +- proxmox/config_group.go | 6 ++-- proxmox/config_user.go | 28 +++++++++---------- proxmox/config_user_test.go | 4 +-- .../Users/user_sub_tests/user_sub_tests.go | 10 +++---- 6 files changed, 28 insertions(+), 28 deletions(-) diff --git a/main.go b/main.go index 7b2e8aa..0c457b2 100644 --- a/main.go +++ b/main.go @@ -445,7 +445,7 @@ func main() { User: userId, }.UpdateUserPassword(c) failError(err) - fmt.Printf("Password of User %s updated\n", userId.ToString()) + fmt.Printf("Password of User %s updated\n", userId.String()) case "setUser": var password proxmox.UserPassword @@ -457,7 +457,7 @@ func main() { password = proxmox.UserPassword(flag.Args()[2]) } failError(config.SetUser(userId, password, c)) - log.Printf("User %s has been configured\n", userId.ToString()) + log.Printf("User %s has been configured\n", userId.String()) case "deleteUser": if len(flag.Args()) < 2 { @@ -468,7 +468,7 @@ func main() { failError(err) err = proxmox.ConfigUser{User: userId}.DeleteUser(c) failError(err) - fmt.Printf("User %s removed\n", userId.ToString()) + fmt.Printf("User %s removed\n", userId.String()) //ACME Account case "getAcmeAccountList": diff --git a/proxmox/client.go b/proxmox/client.go index c00e5a6..fc607ca 100644 --- a/proxmox/client.go +++ b/proxmox/client.go @@ -1510,7 +1510,7 @@ func (c *Client) GetUserPermissions(id UserID, path string) (permissions []strin if !existence { return nil, fmt.Errorf("cannot get user (%s) permissions, the user does not exist", id) } - permlist, err := c.getItemList("/access/permissions?userid=" + id.ToString() + "&path=" + path) + permlist, err := c.getItemList("/access/permissions?userid=" + id.String() + "&path=" + path) failError(err) data := permlist["data"].(map[string]interface{}) for pth, prm := range data { diff --git a/proxmox/config_group.go b/proxmox/config_group.go index 3e418f7..b6d742a 100644 --- a/proxmox/config_group.go +++ b/proxmox/config_group.go @@ -261,7 +261,7 @@ func (group GroupName) removeAllUsersFromGroupExcept(allUsers []interface{}, mem } var userInMembers bool for _, ee := range *members { - if params["userid"] == ee.ToString() { + if params["userid"] == ee.String() { userInMembers = true break } @@ -324,7 +324,7 @@ func (group GroupName) usersToAddToGroup(allUsers []interface{}, members *[]User continue } for ii, ee := range *members { - if params["userid"] == ee.ToString() { + if params["userid"] == ee.String() { var groups []GroupName if _, isSet := params["groups"]; isSet { groups = GroupName("").csvToArray(params["groups"].(string)) @@ -350,7 +350,7 @@ func (group GroupName) usersToRemoveFromGroup(allUsers []interface{}, members *[ continue } for ii, ee := range *members { - if params["userid"] == ee.ToString() { + if params["userid"] == ee.String() { var groups []GroupName if _, isSet := params["groups"]; isSet { groups = GroupName("").csvToArray(params["groups"].(string)) diff --git a/proxmox/config_user.go b/proxmox/config_user.go index 22f3221..4ea3157 100644 --- a/proxmox/config_user.go +++ b/proxmox/config_user.go @@ -45,10 +45,10 @@ func (config ConfigUser) DeleteUser(client *Client) (err error) { return } if !existence { - return fmt.Errorf("user (%s) could not be deleted, the user does not exist", config.User.ToString()) + return fmt.Errorf("user (%s) could not be deleted, the user does not exist", config.User.String()) } // Proxmox silently fails a user delete if the users does not exist - return client.delete("/access/users/" + config.User.ToString()) + return client.delete("/access/users/" + config.User.String()) } // Maps the struct to the API values proxmox understands @@ -65,7 +65,7 @@ func (config ConfigUser) mapToApiValues(create bool) (params map[string]interfac } if create { params["password"] = config.Password - params["userid"] = config.User.ToString() + params["userid"] = config.User.String() } return } @@ -150,7 +150,7 @@ func (config *ConfigUser) SetUser(userId UserID, password UserPassword, client * func (config *ConfigUser) UpdateUser(client *Client) (err error) { params := config.mapToApiValues(false) - err = client.put(params, "/access/users/"+config.User.ToString()) + err = client.put(params, "/access/users/"+config.User.String()) if err != nil { params, _ := json.Marshal(¶ms) return fmt.Errorf("error updating User: %v, (params: %v)", err, string(params)) @@ -167,7 +167,7 @@ func (config ConfigUser) UpdateUserPassword(client *Client) (err error) { return err } return client.put(map[string]interface{}{ - "userid": config.User.ToString(), + "userid": config.User.String(), "password": config.Password, }, "/access/password") } @@ -219,7 +219,7 @@ func (config ConfigUser) CreateApiToken(client *Client, token ApiToken) (value s "comment": token.Comment, "expire": token.Expire, "privsep": token.Privsep, - }, "/access/users/"+config.User.ToString()+"/token/"+token.TokenId) + }, "/access/users/"+config.User.String()+"/token/"+token.TokenId) if err != nil { return } @@ -234,12 +234,12 @@ func (config ConfigUser) UpdateApiToken(client *Client, token ApiToken) (err err "comment": token.Comment, "expire": token.Expire, "privsep": token.Privsep, - }, "/access/users/"+config.User.ToString()+"/token/"+token.TokenId) + }, "/access/users/"+config.User.String()+"/token/"+token.TokenId) return } func (config ConfigUser) ListApiTokens(client *Client) (tokens *[]ApiToken, err error) { - status, err := client.getItemListInterfaceArray("/access/users/" + config.User.ToString() + "/token") + status, err := client.getItemListInterfaceArray("/access/users/" + config.User.String() + "/token") if err != nil { return } @@ -248,7 +248,7 @@ func (config ConfigUser) ListApiTokens(client *Client) (tokens *[]ApiToken, err } func (config ConfigUser) DeleteApiToken(client *Client, token ApiToken) (err error) { - err = client.delete("/access/users/" + config.User.ToString() + "/token/" + token.TokenId) + err = client.delete("/access/users/" + config.User.String() + "/token/" + token.TokenId) return } @@ -333,7 +333,7 @@ func (UserID) mapToStruct(userId string) UserID { // Converts the userID to "username@realm" // Returns an empty string when either the Name or Realm is empty -func (id UserID) ToString() string { +func (id UserID) String() string { if id.Name == "" || id.Realm == "" { return "" } @@ -369,9 +369,9 @@ func CheckUserExistence(userId UserID, client *Client) (existence bool, err erro } // This should be the case where you have an API Token with privilege separation but no permissions attached if len(list) == 0 { - return false, fmt.Errorf("user %s has valid credentials but cannot retrieve user list, check privilege separation of api token", userId.ToString()) + return false, fmt.Errorf("user %s has valid credentials but cannot retrieve user list, check privilege separation of api token", userId.String()) } - existence = ItemInKeyOfArray(list, "userid", userId.ToString()) + existence = ItemInKeyOfArray(list, "userid", userId.String()) return } @@ -403,7 +403,7 @@ func listUsersFull(client *Client) ([]interface{}, error) { } func NewConfigUserFromApi(userId UserID, client *Client) (*ConfigUser, error) { - userConfig, err := client.getItemConfigMapStringInterface("/access/users/"+userId.ToString(), "user", "CONFIG") + userConfig, err := client.getItemConfigMapStringInterface("/access/users/"+userId.String(), "user", "CONFIG") if err != nil { return nil, err } @@ -451,5 +451,5 @@ func NewUserIDs(userIds string) (*[]UserID, error) { // URL for updating users func updateUser(user UserID, params map[string]interface{}, client *Client) error { - return client.put(params, "/access/users/"+user.ToString()) + return client.put(params, "/access/users/"+user.String()) } diff --git a/proxmox/config_user_test.go b/proxmox/config_user_test.go index 4be5a99..4e43ecf 100644 --- a/proxmox/config_user_test.go +++ b/proxmox/config_user_test.go @@ -482,7 +482,7 @@ func Test_UserID_mapToStruct(t *testing.T) { } // TODO improve test when a validation function for the UserID exists -func Test_UserID_ToString(t *testing.T) { +func Test_UserID_String(t *testing.T) { testData := []struct { input UserID Output string @@ -499,7 +499,7 @@ func Test_UserID_ToString(t *testing.T) { {input: UserID{}}, } for _, e := range testData { - require.Equal(t, e.Output, e.input.ToString()) + require.Equal(t, e.Output, e.input.String()) } } diff --git a/test/cli/Users/user_sub_tests/user_sub_tests.go b/test/cli/Users/user_sub_tests/user_sub_tests.go index b0c8f53..936cf4c 100644 --- a/test/cli/Users/user_sub_tests/user_sub_tests.go +++ b/test/cli/Users/user_sub_tests/user_sub_tests.go @@ -11,8 +11,8 @@ import ( func Cleanup(t *testing.T, user proxmox.UserID) { Test := cliTest.Test{ ReqErr: true, - ErrContains: user.ToString(), - Args: []string{"-i", "delete", "user", user.ToString()}, + ErrContains: user.String(), + Args: []string{"-i", "delete", "user", user.String()}, } Test.StandardTest(t) } @@ -20,15 +20,15 @@ func Cleanup(t *testing.T, user proxmox.UserID) { // Default DELETE test for User func Delete(t *testing.T, user proxmox.UserID) { Test := cliTest.Test{ - Contains: []string{user.ToString()}, - Args: []string{"-i", "delete", "user", user.ToString()}, + Contains: []string{user.String()}, + Args: []string{"-i", "delete", "user", user.String()}, } Test.StandardTest(t) } // Default SET test for User func Set(t *testing.T, user proxmox.ConfigUser) { - userID := user.User.ToString() + userID := user.User.String() user.User = proxmox.UserID{} Test := cliTest.Test{ InputJson: user, From 6ae2dd4b4cb85cd1e6da5004b3a331aebf55a9d8 Mon Sep 17 00:00:00 2001 From: Tinyblargon <76069640+Tinyblargon@users.noreply.github.com> Date: Wed, 6 Dec 2023 10:03:40 +0000 Subject: [PATCH 2/2] docs: Add section on standardized interfaces --- docs/style-guide/sdk.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/docs/style-guide/sdk.md b/docs/style-guide/sdk.md index 963bc2e..760be70 100644 --- a/docs/style-guide/sdk.md +++ b/docs/style-guide/sdk.md @@ -25,6 +25,7 @@ When this document refers to the SDK, it references the code in the [/proxmox](. - [Safe function](#safe-function) - [Unsafe function](#unsafe-function) - [Public and Private](#public-and-private) + - [Standardized Interfaces](#standardized-interfaces) - [Pure and Impure Functions](#pure-and-impure-functions) - [Pure Functions](#pure-functions) - [Impure Functions](#impure-functions) @@ -329,6 +330,12 @@ 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. +## Standardized Interfaces + +Standardized interfaces are key in software development for consistent functionality exposure. They enhance code understanding, usage, and system interoperability. + +If the Go standard library has an interface for existing functionality, use it. It's designed with best practices, well-documented, and tested. Ignoring it can lead to complexity and compatibility issues. + ## Pure and Impure Functions In software development, functions are categorized into two main types: [pure functions](https://en.wikipedia.org/wiki/Pure_function#Pure_functions) and [impure functions](https://en.wikipedia.org/wiki/Pure_function#Impure_functions).