[GitHub] [trafficcontrol] srijeet0406 commented on a diff in pull request #7079: Assign multiple servers to a capability
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
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
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
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
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
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
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