Return empty array instead of null when permissions are omitted (#7521)

* Return empty array instead of null when permissions are omitted

* Add CHANGELOG entry

* Add roles unit tests for empty permissions

* Separate 3rd-party packages

CHANGELOG.md

@@ -37,6 +37,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).
 - [#7388](https://github.com/apache/trafficcontrol/pull/7388) *TC go Client* Adds sslkey_expiration methodology in v4 and v5 clients
 
 ### Changed
+- [#7521](https://github.com/apache/trafficcontrol/pull/7521) *Traffic Ops* Returns empty array instead of null when no permissions are given for roles endpoint using POST or PUT request.
 - [#7369](https://github.com/apache/trafficcontrol/pull/7369) *Traffic Portal* Adds better labels to routing methods widget on the TP dashboard.
 - [#7369](https://github.com/apache/trafficcontrol/pull/7369) *Traffic Portal* Simplifies DS button bar by moving DS changes / DSRs under More menu and renaming to 'View Change Requests'.
 - [#7224](https://github.com/apache/trafficcontrol/pull/7224) *Traffic Ops* Required Capabilities are now a part of the `DeliveryService` structure.

traffic_ops/traffic_ops_golang/role/roles.go

@@ -418,6 +418,10 @@ func Update(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 	alerts := tc.CreateAlerts(tc.SuccessLevel, "role was updated.")
+	// to return empty array instead of null
+	if roleV4.Permissions == nil {
+		roleV4.Permissions = []string{}
+	}
 	var roleResponse interface{}
 	roleResponse = tc.RoleV4{
 		Name:        roleV4.Name,
@@ -576,6 +580,10 @@ func Create(w http.ResponseWriter, r *http.Request) {
 		}
 	}
 	alerts := tc.CreateAlerts(tc.SuccessLevel, "role was created.")
+	// to return empty array instead of null
+	if roleCapabilities == nil {
+		roleCapabilities = []string{}
+	}
 	var roleResponse interface{}
 	capabilities := roleCapabilities
 	roleResponse = tc.RoleV4{

traffic_ops/traffic_ops_golang/role/roles_test.go

@@ -20,15 +20,27 @@ package role
  */
 
 import (
+	"context"
+	"encoding/json"
 	"errors"
+	"net/http"
+	"net/http/httptest"
 	"reflect"
 	"strings"
 	"testing"
+	"time"
 
 	"github.com/apache/trafficcontrol/lib/go-tc"
 	"github.com/apache/trafficcontrol/lib/go-util"
 	"github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/api"
+	"github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/auth"
+	"github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/config"
 	"github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/test"
+	"github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/trafficvault"
+	"github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/trafficvault/backends/disabled"
+
+	"github.com/jmoiron/sqlx"
+	"gopkg.in/DATA-DOG/go-sqlmock.v1"
 )
 
 func stringAddr(s string) *string {
@@ -116,3 +128,113 @@ func TestValidate(t *testing.T) {
 	}
 
 }
+
+func TestCreateWithEmptyPermissions(t *testing.T) {
+	mockDB, mock, err := sqlmock.New()
+	if err != nil {
+		t.Fatalf("an error '%s' was not expected when opening a stub database connection", err)
+	}
+	defer mockDB.Close()
+
+	db := sqlx.NewDb(mockDB, "sqlmock")
+	defer db.Close()
+
+	w := httptest.NewRecorder()
+	r, err := http.NewRequest("", "", strings.NewReader(`{"name":"role", "description":"description"}`))
+	if err != nil {
+		t.Error("Error creating new request")
+	}
+
+	addRequestContext(r, db)
+
+	columns := []string{"id", "last_updated"}
+
+	mock.ExpectBegin()
+	mock.ExpectQuery("INSERT").WithArgs("role", "description", auth.PrivLevelAdmin).WillReturnRows(sqlmock.NewRows(columns).AddRow(1, time.Now()))
+	mock.ExpectCommit()
+
+	Create(w, r)
+
+	if w.Code != http.StatusCreated {
+		t.Errorf("Got status code %d but want 201", w.Code)
+	}
+
+	resp := struct {
+		tc.Alerts
+		Response tc.RoleV4 `json:"response"`
+	}{}
+
+	if err := json.Unmarshal(w.Body.Bytes(), &resp); err != nil {
+		t.Fatal("Error decoding response")
+	}
+
+	if resp.Response.Permissions == nil {
+		t.Error("Permissions should be empty not nil")
+	}
+}
+
+func TestUpdateWithEmptyPermissions(t *testing.T) {
+	mockDB, mock, err := sqlmock.New()
+	if err != nil {
+		t.Fatalf("an error '%s' was not expected when opening a stub database connection", err)
+	}
+	defer mockDB.Close()
+
+	db := sqlx.NewDb(mockDB, "sqlmock")
+	defer db.Close()
+
+	w := httptest.NewRecorder()
+	r, err := http.NewRequest("", "", strings.NewReader(`{"name":"new_role", "description":"new_description"}`))
+	if err != nil {
+		t.Error("Error creating new request")
+	}
+
+	addRequestContext(r, db)
+
+	r.URL.RawQuery = "name=role"
+
+	mock.ExpectBegin()
+
+	mock.ExpectQuery("SELECT").WillReturnRows(sqlmock.NewRows([]string{"id"}).AddRow(1))
+	mock.ExpectQuery("(?i)SELECT").WillReturnRows(sqlmock.NewRows([]string{"last_updated"}).AddRow(time.Now()))
+	mock.ExpectQuery("SET").WithArgs("new_role", "new_description", "role").WillReturnRows(sqlmock.NewRows([]string{"last_updated"}).AddRow(time.Now()))
+
+	mock.ExpectExec("DELETE").WithArgs("new_role").WillReturnResult(sqlmock.NewResult(1, 0))
+	mock.ExpectExec("INSERT").WithArgs(1, nil).WillReturnResult(sqlmock.NewResult(1, 0))
+
+	mock.ExpectCommit()
+
+	Update(w, r)
+
+	if w.Code != http.StatusOK {
+		t.Errorf("Got status code %d but want 200", w.Code)
+	}
+
+	resp := struct {
+		tc.Alerts
+		Response tc.RoleV4 `json:"response"`
+	}{}
+
+	if err := json.Unmarshal(w.Body.Bytes(), &resp); err != nil {
+		t.Fatal("Error decoding response")
+	}
+
+	if resp.Response.Permissions == nil {
+		t.Fatal("Permissions should be empty not nil")
+	}
+}
+
+func addRequestContext(r *http.Request, db *sqlx.DB) {
+	cfg := config.Config{ConfigTrafficOpsGolang: config.ConfigTrafficOpsGolang{DBQueryTimeoutSeconds: 20}, UseIMS: true}
+	ctx := r.Context()
+	ctx = context.WithValue(ctx, auth.CurrentUserKey,
+		auth.CurrentUser{UserName: "username", ID: 1, PrivLevel: auth.PrivLevelAdmin})
+	ctx = context.WithValue(ctx, api.PathParamsKey, map[string]string{"id": "1"})
+	ctx = context.WithValue(ctx, api.DBContextKey, db)
+	ctx = context.WithValue(ctx, api.ConfigContextKey, &cfg)
+	ctx = context.WithValue(ctx, api.ReqIDContextKey, uint64(0))
+	var tv trafficvault.TrafficVault = &disabled.Disabled{}
+	ctx = context.WithValue(ctx, api.TrafficVaultContextKey, tv)
+
+	*r = *r.WithContext(ctx)
+}