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(&mspc); 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: <error>`
##########
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(&mspc); 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(&correctType); 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(&multipleServersPerCapability))
+ 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 Comment:
Can `multipleServersPerCapability` be an `int` array to begin with?
##########
docs/source/api/v4/multiple_servers_per_capability.rst:
##########
@@ -0,0 +1,83 @@
+..
+..
+.. 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_per_capability:
+
+***********************************
+``multiple_servers_per_capability``
+***********************************
+
+.. versionadded:: 4.1
+
+``PUT``
+========
+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:
You'll need to define the term Servers in the glossary. Look at Server
Capabilities as an example. That should get rid of all the other `Servers` doc
errors.
##########
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(&mspc); 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(&correctType); err != nil {
Review Comment:
This does a DB query for every server in the list, and that might not be
very performant. I think we should get an array of results back and then go
through that array to make sure every server is of the right type. Or even do
an `&&` on the individual results, and then check the final resultant value to
be `true` or `false`, but I'm not a 100% sure how to do that.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
