[GitHub] [trafficcontrol] srijeet0406 commented on a diff in pull request #7079: Assign multiple servers to a capability

2022-10-28 Thread GitBox


srijeet0406 commented on code in PR #7079:
URL: https://github.com/apache/trafficcontrol/pull/7079#discussion_r1008285232


##
traffic_ops/traffic_ops_golang/server/servers_server_capability.go:
##
@@ -453,77 +443,185 @@ func AssignMultipleServerCapabilities(w 
http.ResponseWriter, r *http.Request) {
}
defer inf.Close()
 
-   var msc tc.MultipleServerCapabilities
-   if err := json.NewDecoder(r.Body).Decode(); err != nil {
-   api.HandleErr(w, r, tx, http.StatusBadRequest, err, nil)
+   var mssc tc.MultipleServersCapabilities
+   if err := json.NewDecoder(r.Body).Decode(); err != nil {
+   api.HandleErr(w, r, tx, http.StatusBadRequest, 
fmt.Errorf("error decoding POST request body into MultipleServersCapabilities 
struct %w", err), nil)
return
}
 
-   // Check existence prior to checking type
-   _, exists, err := dbhelpers.GetServerNameFromID(tx, int64(msc.ServerID))
-   if err != nil {
-   api.HandleErr(w, r, tx, http.StatusInternalServerError, nil, 
err)
-   }
-   if !exists {
-   userErr := fmt.Errorf("server %d does not exist", msc.ServerID)
-   api.HandleErr(w, r, tx, http.StatusNotFound, userErr, nil)
-   return
+   if len(mssc.ServerIDs) >= 1 {
+   errCode, userErr, sysErr = checkExistingServer(tx, 
mssc.ServerIDs, inf.User.UserName)
+   if userErr != nil || sysErr != nil {
+   api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+   return
+   }
}
 
// Ensure type is correct
-   correctType := true
-   if err := tx.QueryRow(scCheckServerTypeQuery(), 
msc.ServerID).Scan(); err != nil {
-   api.HandleErr(w, r, tx, http.StatusInternalServerError, nil, 
fmt.Errorf("checking server type: %w", err))
+   errCode, userErr, sysErr = checkServerType(tx, mssc.ServerIDs)
+   if userErr != nil || sysErr != nil {
+   api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
return
}
-   if !correctType {
-   userErr := fmt.Errorf("server %d has an incorrect server type. 
Server capabilities can only be assigned to EDGE or MID servers", msc.ServerID)
-   api.HandleErr(w, r, tx, http.StatusBadRequest, userErr, nil)
-   return
+
+   // Insert rows in DB
+   sid := make([]int64, len(mssc.ServerCapabilities))
+   scs := make([]string, len(mssc.ServerIDs))
+   if len(mssc.ServerIDs) == 1 {
+   if len(mssc.ServerCapabilities) >= 1 {
+   for i := range mssc.ServerCapabilities {
+   sid[i] = mssc.ServerIDs[0]
+   }
+   scs = mssc.ServerCapabilities
+   }
+   } else if len(mssc.ServerCapabilities) == 1 {
+   if len(mssc.ServerIDs) >= 1 {
+   for i := range mssc.ServerIDs {
+   scs[i] = mssc.ServerCapabilities[0]
+   }
+   sid = mssc.ServerIDs
+   }
+   } else {
+   scs = mssc.ServerCapabilities
+   sid = mssc.ServerIDs
}
 
-   cdnName, err := dbhelpers.GetCDNNameFromServerID(tx, 
int64(msc.ServerID))
+   msscQuery := `INSERT INTO server_server_capability
+   select "server_capability", "server"
+   FROM UNNEST($1::text[], $2::int[]) AS 
tmp("server_capability", "server")`
+   _, err := tx.Query(msscQuery, pq.Array(scs), pq.Array(sid))
if err != nil {
-   api.HandleErr(w, r, tx, http.StatusInternalServerError, nil, 
err)
+   useErr, sysErr, statusCode := api.ParseDBError(err)
+   api.HandleErr(w, r, tx, statusCode, useErr, sysErr)
return
}
 
-   userErr, sysErr, errCode = dbhelpers.CheckIfCurrentUserCanModifyCDN(tx, 
string(cdnName), inf.User.UserName)
+   var alerts tc.Alerts
+   if mssc.PageType == "sc" {
+   alerts = tc.CreateAlerts(tc.SuccessLevel, "Assign Server(s) to 
a capability")
+   } else {
+   alerts = tc.CreateAlerts(tc.SuccessLevel, "Assign Server 
Capability(ies) to a server")
+   }
+   api.WriteAlertsObj(w, r, http.StatusOK, alerts, mssc)
+   return
+}
+
+// DeleteMultipleServersCapabilities deletes multiple servers to a capability 
or multiple server capabilities to a server
+func DeleteMultipleServersCapabilities(w http.ResponseWriter, r *http.Request) 
{
+   inf, userErr, sysErr, errCode := api.NewInfo(r, nil, nil)
+   tx := inf.Tx.Tx
if userErr != nil || sysErr != nil {
-   api.HandleErr(w, r, tx, errCode, userErr, sysErr)
+   api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
return
}
+   defer inf.Close()
 
-   

[GitHub] [trafficcontrol] srijeet0406 commented on a diff in pull request #7079: Assign multiple servers to a capability

2022-10-27 Thread GitBox


srijeet0406 commented on code in PR #7079:
URL: https://github.com/apache/trafficcontrol/pull/7079#discussion_r1007454984


##
traffic_ops/traffic_ops_golang/server/servers_server_capability.go:
##
@@ -453,77 +443,185 @@ func AssignMultipleServerCapabilities(w 
http.ResponseWriter, r *http.Request) {
}
defer inf.Close()
 
-   var msc tc.MultipleServerCapabilities
-   if err := json.NewDecoder(r.Body).Decode(); err != nil {
-   api.HandleErr(w, r, tx, http.StatusBadRequest, err, nil)
+   var mssc tc.MultipleServersCapabilities
+   if err := json.NewDecoder(r.Body).Decode(); err != nil {
+   api.HandleErr(w, r, tx, http.StatusBadRequest, 
fmt.Errorf("error decoding POST request body into MultipleServersCapabilities 
struct %w", err), nil)
return
}
 
-   // Check existence prior to checking type
-   _, exists, err := dbhelpers.GetServerNameFromID(tx, int64(msc.ServerID))
-   if err != nil {
-   api.HandleErr(w, r, tx, http.StatusInternalServerError, nil, 
err)
-   }
-   if !exists {
-   userErr := fmt.Errorf("server %d does not exist", msc.ServerID)
-   api.HandleErr(w, r, tx, http.StatusNotFound, userErr, nil)
-   return
+   if len(mssc.ServerIDs) >= 1 {
+   errCode, userErr, sysErr = checkExistingServer(tx, 
mssc.ServerIDs, inf.User.UserName)
+   if userErr != nil || sysErr != nil {
+   api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+   return
+   }
}
 
// Ensure type is correct
-   correctType := true
-   if err := tx.QueryRow(scCheckServerTypeQuery(), 
msc.ServerID).Scan(); err != nil {
-   api.HandleErr(w, r, tx, http.StatusInternalServerError, nil, 
fmt.Errorf("checking server type: %w", err))
+   errCode, userErr, sysErr = checkServerType(tx, mssc.ServerIDs)
+   if userErr != nil || sysErr != nil {
+   api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
return
}
-   if !correctType {
-   userErr := fmt.Errorf("server %d has an incorrect server type. 
Server capabilities can only be assigned to EDGE or MID servers", msc.ServerID)
-   api.HandleErr(w, r, tx, http.StatusBadRequest, userErr, nil)
-   return
+
+   // Insert rows in DB
+   sid := make([]int64, len(mssc.ServerCapabilities))
+   scs := make([]string, len(mssc.ServerIDs))
+   if len(mssc.ServerIDs) == 1 {

Review Comment:
   I thought the whole point of adding the pagetype was so that we didn't have 
to do length checks anymore?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscr...@trafficcontrol.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



[GitHub] [trafficcontrol] srijeet0406 commented on a diff in pull request #7079: Assign multiple servers to a capability

2022-10-27 Thread GitBox


srijeet0406 commented on code in PR #7079:
URL: https://github.com/apache/trafficcontrol/pull/7079#discussion_r1007454708


##
traffic_ops/traffic_ops_golang/server/servers_server_capability.go:
##
@@ -453,77 +443,185 @@ func AssignMultipleServerCapabilities(w 
http.ResponseWriter, r *http.Request) {
}
defer inf.Close()
 
-   var msc tc.MultipleServerCapabilities
-   if err := json.NewDecoder(r.Body).Decode(); err != nil {
-   api.HandleErr(w, r, tx, http.StatusBadRequest, err, nil)
+   var mssc tc.MultipleServersCapabilities
+   if err := json.NewDecoder(r.Body).Decode(); err != nil {
+   api.HandleErr(w, r, tx, http.StatusBadRequest, 
fmt.Errorf("error decoding POST request body into MultipleServersCapabilities 
struct %w", err), nil)
return
}
 
-   // Check existence prior to checking type
-   _, exists, err := dbhelpers.GetServerNameFromID(tx, int64(msc.ServerID))
-   if err != nil {
-   api.HandleErr(w, r, tx, http.StatusInternalServerError, nil, 
err)
-   }
-   if !exists {
-   userErr := fmt.Errorf("server %d does not exist", msc.ServerID)
-   api.HandleErr(w, r, tx, http.StatusNotFound, userErr, nil)
-   return
+   if len(mssc.ServerIDs) >= 1 {
+   errCode, userErr, sysErr = checkExistingServer(tx, 
mssc.ServerIDs, inf.User.UserName)
+   if userErr != nil || sysErr != nil {
+   api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+   return
+   }
}
 
// Ensure type is correct
-   correctType := true
-   if err := tx.QueryRow(scCheckServerTypeQuery(), 
msc.ServerID).Scan(); err != nil {
-   api.HandleErr(w, r, tx, http.StatusInternalServerError, nil, 
fmt.Errorf("checking server type: %w", err))
+   errCode, userErr, sysErr = checkServerType(tx, mssc.ServerIDs)
+   if userErr != nil || sysErr != nil {
+   api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
return
}
-   if !correctType {
-   userErr := fmt.Errorf("server %d has an incorrect server type. 
Server capabilities can only be assigned to EDGE or MID servers", msc.ServerID)
-   api.HandleErr(w, r, tx, http.StatusBadRequest, userErr, nil)
-   return
+
+   // Insert rows in DB
+   sid := make([]int64, len(mssc.ServerCapabilities))
+   scs := make([]string, len(mssc.ServerIDs))
+   if len(mssc.ServerIDs) == 1 {
+   if len(mssc.ServerCapabilities) >= 1 {
+   for i := range mssc.ServerCapabilities {
+   sid[i] = mssc.ServerIDs[0]
+   }
+   scs = mssc.ServerCapabilities
+   }
+   } else if len(mssc.ServerCapabilities) == 1 {
+   if len(mssc.ServerIDs) >= 1 {
+   for i := range mssc.ServerIDs {
+   scs[i] = mssc.ServerCapabilities[0]
+   }
+   sid = mssc.ServerIDs
+   }
+   } else {
+   scs = mssc.ServerCapabilities
+   sid = mssc.ServerIDs
}
 
-   cdnName, err := dbhelpers.GetCDNNameFromServerID(tx, 
int64(msc.ServerID))
+   msscQuery := `INSERT INTO server_server_capability
+   select "server_capability", "server"
+   FROM UNNEST($1::text[], $2::int[]) AS 
tmp("server_capability", "server")`
+   _, err := tx.Query(msscQuery, pq.Array(scs), pq.Array(sid))
if err != nil {
-   api.HandleErr(w, r, tx, http.StatusInternalServerError, nil, 
err)
+   useErr, sysErr, statusCode := api.ParseDBError(err)
+   api.HandleErr(w, r, tx, statusCode, useErr, sysErr)
return
}
 
-   userErr, sysErr, errCode = dbhelpers.CheckIfCurrentUserCanModifyCDN(tx, 
string(cdnName), inf.User.UserName)
+   var alerts tc.Alerts
+   if mssc.PageType == "sc" {
+   alerts = tc.CreateAlerts(tc.SuccessLevel, "Assign Server(s) to 
a capability")
+   } else {
+   alerts = tc.CreateAlerts(tc.SuccessLevel, "Assign Server 
Capability(ies) to a server")
+   }
+   api.WriteAlertsObj(w, r, http.StatusOK, alerts, mssc)
+   return
+}
+
+// DeleteMultipleServersCapabilities deletes multiple servers to a capability 
or multiple server capabilities to a server
+func DeleteMultipleServersCapabilities(w http.ResponseWriter, r *http.Request) 
{
+   inf, userErr, sysErr, errCode := api.NewInfo(r, nil, nil)
+   tx := inf.Tx.Tx
if userErr != nil || sysErr != nil {
-   api.HandleErr(w, r, tx, errCode, userErr, sysErr)
+   api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
return
}
+   defer inf.Close()
 
-   

[GitHub] [trafficcontrol] srijeet0406 commented on a diff in pull request #7079: Assign multiple servers to a capability

2022-10-24 Thread GitBox


srijeet0406 commented on code in PR #7079:
URL: https://github.com/apache/trafficcontrol/pull/7079#discussion_r1003829984


##
traffic_ops/traffic_ops_golang/server/servers_server_capability.go:
##
@@ -453,77 +443,185 @@ func AssignMultipleServerCapabilities(w 
http.ResponseWriter, r *http.Request) {
}
defer inf.Close()
 
-   var msc tc.MultipleServerCapabilities
-   if err := json.NewDecoder(r.Body).Decode(); err != nil {
-   api.HandleErr(w, r, tx, http.StatusBadRequest, err, nil)
+   var mssc tc.MultipleServersCapabilities
+   if err := json.NewDecoder(r.Body).Decode(); err != nil {
+   api.HandleErr(w, r, tx, http.StatusBadRequest, 
fmt.Errorf("error decoding POST request body into MultipleServersCapabilities 
struct %w", err), nil)
return
}
 
-   // Check existence prior to checking type
-   _, exists, err := dbhelpers.GetServerNameFromID(tx, int64(msc.ServerID))
-   if err != nil {
-   api.HandleErr(w, r, tx, http.StatusInternalServerError, nil, 
err)
-   }
-   if !exists {
-   userErr := fmt.Errorf("server %d does not exist", msc.ServerID)
-   api.HandleErr(w, r, tx, http.StatusNotFound, userErr, nil)
-   return
+   if len(mssc.ServerIDs) >= 1 {
+   errCode, userErr, sysErr = checkExistingServer(tx, 
mssc.ServerIDs, inf.User.UserName)
+   if userErr != nil || sysErr != nil {
+   api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+   return
+   }
}
 
// Ensure type is correct
-   correctType := true
-   if err := tx.QueryRow(scCheckServerTypeQuery(), 
msc.ServerID).Scan(); err != nil {
-   api.HandleErr(w, r, tx, http.StatusInternalServerError, nil, 
fmt.Errorf("checking server type: %w", err))
+   errCode, userErr, sysErr = checkServerType(tx, mssc.ServerIDs)
+   if userErr != nil || sysErr != nil {
+   api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
return
}
-   if !correctType {
-   userErr := fmt.Errorf("server %d has an incorrect server type. 
Server capabilities can only be assigned to EDGE or MID servers", msc.ServerID)
-   api.HandleErr(w, r, tx, http.StatusBadRequest, userErr, nil)
-   return
+
+   // Insert rows in DB
+   sid := make([]int64, len(mssc.ServerCapabilities))
+   scs := make([]string, len(mssc.ServerIDs))
+   if len(mssc.ServerIDs) == 1 {

Review Comment:
   Do you need the length checks if you make use of the `PageType` attribute?



##
traffic_ops/traffic_ops_golang/server/servers_server_capability.go:
##
@@ -453,77 +443,185 @@ func AssignMultipleServerCapabilities(w 
http.ResponseWriter, r *http.Request) {
}
defer inf.Close()
 
-   var msc tc.MultipleServerCapabilities
-   if err := json.NewDecoder(r.Body).Decode(); err != nil {
-   api.HandleErr(w, r, tx, http.StatusBadRequest, err, nil)
+   var mssc tc.MultipleServersCapabilities
+   if err := json.NewDecoder(r.Body).Decode(); err != nil {
+   api.HandleErr(w, r, tx, http.StatusBadRequest, 
fmt.Errorf("error decoding POST request body into MultipleServersCapabilities 
struct %w", err), nil)
return
}
 
-   // Check existence prior to checking type
-   _, exists, err := dbhelpers.GetServerNameFromID(tx, int64(msc.ServerID))
-   if err != nil {
-   api.HandleErr(w, r, tx, http.StatusInternalServerError, nil, 
err)
-   }
-   if !exists {
-   userErr := fmt.Errorf("server %d does not exist", msc.ServerID)
-   api.HandleErr(w, r, tx, http.StatusNotFound, userErr, nil)
-   return
+   if len(mssc.ServerIDs) >= 1 {
+   errCode, userErr, sysErr = checkExistingServer(tx, 
mssc.ServerIDs, inf.User.UserName)
+   if userErr != nil || sysErr != nil {
+   api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+   return
+   }
}
 
// Ensure type is correct
-   correctType := true
-   if err := tx.QueryRow(scCheckServerTypeQuery(), 
msc.ServerID).Scan(); err != nil {
-   api.HandleErr(w, r, tx, http.StatusInternalServerError, nil, 
fmt.Errorf("checking server type: %w", err))
+   errCode, userErr, sysErr = checkServerType(tx, mssc.ServerIDs)
+   if userErr != nil || sysErr != nil {
+   api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
return
}
-   if !correctType {
-   userErr := fmt.Errorf("server %d has an incorrect server type. 
Server capabilities can only be assigned to EDGE or MID servers", msc.ServerID)
-   api.HandleErr(w, r, tx, http.StatusBadRequest, userErr, nil)
-   return

[GitHub] [trafficcontrol] srijeet0406 commented on a diff in pull request #7079: Assign multiple servers to a capability

2022-10-10 Thread GitBox


srijeet0406 commented on code in PR #7079:
URL: https://github.com/apache/trafficcontrol/pull/7079#discussion_r991409061


##
traffic_ops/traffic_ops_golang/server/servers_server_capability.go:
##
@@ -453,77 +454,166 @@ func AssignMultipleServerCapabilities(w 
http.ResponseWriter, r *http.Request) {
}
defer inf.Close()
 
-   var msc tc.MultipleServerCapabilities
-   if err := json.NewDecoder(r.Body).Decode(); err != nil {
-   api.HandleErr(w, r, tx, http.StatusBadRequest, err, nil)
+   var mssc tc.MultipleServersCapabilities
+   if err := json.NewDecoder(r.Body).Decode(); err != nil {
+   api.HandleErr(w, r, tx, http.StatusBadRequest, 
fmt.Errorf("error decoding POST request body into MultipleServersCapabilities 
struct %w", err), nil)
return
}
 
-   // Check existence prior to checking type
-   _, exists, err := dbhelpers.GetServerNameFromID(tx, int64(msc.ServerID))
-   if err != nil {
-   api.HandleErr(w, r, tx, http.StatusInternalServerError, nil, 
err)
-   }
-   if !exists {
-   userErr := fmt.Errorf("server %d does not exist", msc.ServerID)
-   api.HandleErr(w, r, tx, http.StatusNotFound, userErr, nil)
-   return
+   if len(mssc.ServerIDs) == 1 {
+   errCode, userErr, sysErr = checkExistingServer(tx, 
mssc.ServerIDs[0], inf.User.UserName)
+   if userErr != nil || sysErr != nil {
+   api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+   return
+   }
}
 
-   // Ensure type is correct
-   correctType := true
-   if err := tx.QueryRow(scCheckServerTypeQuery(), 
msc.ServerID).Scan(); err != nil {
+   //Check if the server type is MID and/or EDGE
+   var servArray []int64
+   queryType := `SELECT array_agg(s.id) 
+   FROM server s
+   JOIN type t ON s.type = t.id
+   WHERE s.id = any ($1)
+   AND t.use_in_table = 'server'
+   AND (t.name LIKE 'MID%' OR t.name LIKE 'EDGE%')`
+   if err := tx.QueryRow(queryType, 
pq.Array(mssc.ServerIDs)).Scan(pq.Array()); err != nil {
api.HandleErr(w, r, tx, http.StatusInternalServerError, nil, 
fmt.Errorf("checking server type: %w", err))
return
}
-   if !correctType {
-   userErr := fmt.Errorf("server %d has an incorrect server type. 
Server capabilities can only be assigned to EDGE or MID servers", msc.ServerID)
-   api.HandleErr(w, r, tx, http.StatusBadRequest, userErr, nil)
-   return
+   cmp := make(map[int64]bool)
+   for _, item := range servArray {
+   cmp[item] = true
+   }
+   for _, sid := range mssc.ServerIDs {
+   if _, ok := cmp[sid]; !ok {
+   userErr := fmt.Errorf("server id: %d has an incorrect 
server type. Server capability can only be assigned to EDGE or MID servers", 
sid)
+   api.HandleErr(w, r, tx, http.StatusBadRequest, userErr, 
nil)
+   return
+   }
}
 
-   cdnName, err := dbhelpers.GetCDNNameFromServerID(tx, 
int64(msc.ServerID))
+   // Insert rows in DB
+   sid := make([]int64, len(mssc.ServerCapabilities))
+   scs := make([]string, len(mssc.ServerIDs))
+   if len(mssc.ServerIDs) == 1 {
+   if len(mssc.ServerCapabilities) >= 1 {
+   for i := range mssc.ServerCapabilities {
+   sid[i] = mssc.ServerIDs[0]
+   }
+   scs = mssc.ServerCapabilities
+   }
+   } else if len(mssc.ServerCapabilities) == 1 {
+   if len(mssc.ServerIDs) >= 1 {
+   for i := range mssc.ServerIDs {
+   scs[i] = mssc.ServerCapabilities[0]
+   }
+   sid = mssc.ServerIDs
+   }
+   } else {
+   scs = mssc.ServerCapabilities
+   sid = mssc.ServerIDs
+   }
+
+   msscQuery := `INSERT INTO server_server_capability
+   select "server_capability", "server"
+   FROM UNNEST($1::text[], $2::int[]) AS 
tmp("server_capability", "server")`
+   _, err := tx.Query(msscQuery, pq.Array(scs), pq.Array(sid))
if err != nil {
-   api.HandleErr(w, r, tx, http.StatusInternalServerError, nil, 
err)
+   useErr, sysErr, statusCode := api.ParseDBError(err)
+   api.HandleErr(w, r, tx, statusCode, useErr, sysErr)
return
}
 
-   userErr, sysErr, errCode = dbhelpers.CheckIfCurrentUserCanModifyCDN(tx, 
string(cdnName), inf.User.UserName)
+   var alerts tc.Alerts
+   if len(mssc.ServerCapabilities) == 1 && len(mssc.ServerIDs) == 1 {

Review Comment:
   But that's what I mean 

[GitHub] [trafficcontrol] srijeet0406 commented on a diff in pull request #7079: Assign multiple servers to a capability

2022-10-07 Thread GitBox


srijeet0406 commented on code in PR #7079:
URL: https://github.com/apache/trafficcontrol/pull/7079#discussion_r990344181


##
docs/source/api/v4/multiple_servers_capabilities.rst:
##
@@ -0,0 +1,190 @@
+..
+..
+.. 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.
+..
+
+.. _to-api-v4-multiple_servers_capabilities:
+
+*
+``multiple_servers_capabilities``
+*
+
+.. versionadded:: 4.1
+
+``POST``
+
+Associates a list of :term:`Server Capability` to a server. The API call 
replaces all the server capabilities assigned to a server with the ones 
specified in the serverCapabilities field.
+And also Associates a list of :term:`Servers` to a server capability. The API 
call replaces all the servers assigned to a server capability with the ones 
specified in the servers field.

Review Comment:
   Instead of repitition, could we reframe this sentence as:
   Associates a list of server capabilities to a server, and vice versa. This 
API call replaces all the ..., and vice versa. 



##
docs/source/api/v4/multiple_servers_capabilities.rst:
##
@@ -0,0 +1,190 @@
+..
+..
+.. 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.
+..
+
+.. _to-api-v4-multiple_servers_capabilities:
+
+*
+``multiple_servers_capabilities``
+*
+
+.. versionadded:: 4.1
+
+``POST``
+
+Associates a list of :term:`Server Capability` to a server. The API call 
replaces all the server capabilities assigned to a server with the ones 
specified in the serverCapabilities field.
+And also Associates a list of :term:`Servers` to a server capability. The API 
call replaces all the servers assigned to a server capability with the ones 
specified in the servers field.
+
+:Auth. Required: Yes
+:Roles Required: "admin" or "operations"
+:Permissions Required: SERVER:READ, SERVER:CREATE, SERVER-CAPABILITY:READ, 
SERVER-CAPABILITY:CREATE
+:Response Type:  Object
+
+Request Structure
+-
+:serverIds:  List of :term:`Server` ids ((integral, unique identifier) 
associated with a :term:`Server Capability`
+:serverCapabilities: List of :term:`Server Capability`'s name to associate 
with a :term:`Server` id
+
+
+.. code-block:: http
+   :caption: Request Example1
+
+   POST /api/4.1/multiple_servers_capabilities/ HTTP/1.1
+   Host: trafficops.infra.ciab.test
+   User-Agent: curl/7.47.0
+   Accept: */*
+   Cookie: mojolicious=...
+   Content-Length: 84
+   Content-Type: application/json
+
+   {
+   "serverIds": [1],
+   "serverCapabilities": ["test", "disk"]
+   }
+
+.. code-block:: http
+   :caption: Request Example2
+
+   POST /api/4.1/multiple_servers_capabilities/ HTTP/1.1
+   Host: trafficops.infra.ciab.test
+   User-Agent: curl/7.47.0
+   Accept: */*
+   Cookie: mojolicious=...
+   Content-Length: 84
+   Content-Type: application/json
+
+   {
+   "serverIds": [2, 3]
+   "serverCapabilities": ["eas"],
+   }
+
+Response Structure
+--
+:serverId:   List of :term:`Server` ids ((integral, unique identifier) 
associated with a server capability.
+:serverCapabilities: List of :term:`Server Capability`'s name to be associated 
with a :term:`Server` id.

Review Comment:
   same as above re: `names`



##
docs/source/api/v4/multiple_servers_capabilities.rst:
##
@@ -0,0 +1,190 @@
+..
+..
+.. 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, 

[GitHub] [trafficcontrol] srijeet0406 commented on a diff in pull request #7079: Assign multiple servers to a capability

2022-09-27 Thread GitBox


srijeet0406 commented on code in PR #7079:
URL: https://github.com/apache/trafficcontrol/pull/7079#discussion_r981477784


##
traffic_ops/traffic_ops_golang/server/servers_server_capability.go:
##
@@ -527,3 +527,70 @@ func AssignMultipleServerCapabilities(w 
http.ResponseWriter, r *http.Request) {
api.WriteAlertsObj(w, r, http.StatusOK, alerts, msc)
return
 }
+
+// AssignMultipleServersToCapability helps assign multiple servers to a given 
capability.
+func AssignMultipleServersToCapability(w http.ResponseWriter, r *http.Request) 
{
+   inf, userErr, sysErr, errCode := api.NewInfo(r, nil, nil)
+   tx := inf.Tx.Tx
+   if userErr != nil || sysErr != nil {
+   api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+   return
+   }
+   defer inf.Close()
+
+   var mspc tc.MultipleServersToCapability
+   if err := json.NewDecoder(r.Body).Decode(); err != nil {
+   api.HandleErr(w, r, tx, http.StatusBadRequest, err, nil)

Review Comment:
   It'll help us debug if we add some context into the error, alongwith the 
actual error. Something like `error decoding request body into 
multipleServersToCapability object: `



##
traffic_ops/testing/api/v4/server_server_capabilities_test.go:
##
@@ -223,7 +233,14 @@ func TestServerServerCapabilities(t *testing.T) {
})
case "PUT":
t.Run(name, func(t *testing.T) {
-   alerts, reqInf, err := 
testCase.ClientSession.AssignMultipleServerCapability(msc, 
testCase.RequestOpts, serverId)
+   var alerts tc.Alerts

Review Comment:
   Could we also add some `GET` call that checks whether or not the capability 
was actually added to the servers?



##
traffic_ops/traffic_ops_golang/server/servers_server_capability.go:
##
@@ -527,3 +527,70 @@ func AssignMultipleServerCapabilities(w 
http.ResponseWriter, r *http.Request) {
api.WriteAlertsObj(w, r, http.StatusOK, alerts, msc)
return
 }
+
+// AssignMultipleServersToCapability helps assign multiple servers to a given 
capability.
+func AssignMultipleServersToCapability(w http.ResponseWriter, r *http.Request) 
{
+   inf, userErr, sysErr, errCode := api.NewInfo(r, nil, nil)
+   tx := inf.Tx.Tx
+   if userErr != nil || sysErr != nil {
+   api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+   return
+   }
+   defer inf.Close()
+
+   var mspc tc.MultipleServersToCapability
+   if err := json.NewDecoder(r.Body).Decode(); err != nil {
+   api.HandleErr(w, r, tx, http.StatusBadRequest, err, nil)
+   return
+   }
+
+   //loop through server list to check if the type is MID and/or EDGE
+   for _, sid := range mspc.ServersIDs {
+   correctType := true
+   if err := tx.QueryRow(scCheckServerTypeQuery(), 
sid).Scan(); err != nil {
+   api.HandleErr(w, r, tx, http.StatusInternalServerError, 
nil, fmt.Errorf("checking server type: %w", err))
+   return
+   }
+   if !correctType {
+   userErr := fmt.Errorf("server %d has an incorrect 
server type. Server capability can only be assigned to EDGE or MID servers", 
sid)
+   api.HandleErr(w, r, tx, http.StatusBadRequest, userErr, 
nil)
+   return
+   }
+   }
+
+   multipleServersPerCapability := make([]string, 0, len(mspc.ServersIDs))
+
+   //Delete existing rows from server_server_capability for a given server 
capability
+   _, err := tx.Exec("DELETE FROM server_server_capability ssc WHERE 
ssc.server_capability=$1", mspc.ServerCapability)
+   if err != nil {
+   useErr, sysErr, statusCode := api.ParseDBError(err)
+   api.HandleErr(w, r, tx, statusCode, useErr, sysErr)
+   return
+   }
+
+   mspcQuery := `WITH inserted AS (
+   INSERT INTO server_server_capability
+   SELECT $2, "server" 
+   FROM UNNEST($1::int[]) AS tmp("server")
+   RETURNING server
+   )
+   SELECT ARRAY_AGG(server)
+   FROM (
+   SELECT server
+   FROM inserted
+   ) AS returned(server)`
+
+   err = tx.QueryRow(mspcQuery, pq.Array(mspc.ServersIDs), 
mspc.ServerCapability).Scan(pq.Array())
+   if err != nil {
+   useErr, sysErr, statusCode := api.ParseDBError(err)
+   api.HandleErr(w, r, tx, statusCode, useErr, sysErr)
+   return
+   }
+   for i, val := range multipleServersPerCapability {
+   mspc.ServersIDs[i], _ = strconv.Atoi(val)

Review