You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@trafficcontrol.apache.org by GitBox <gi...@apache.org> on 2022/11/22 01:20:34 UTC
[GitHub] [trafficcontrol] rimashah25 opened a new pull request, #7213: Added description field to server capabilities
rimashah25 opened a new pull request, #7213:
URL: https://github.com/apache/trafficcontrol/pull/7213
<!--
Thank you for contributing! Please be sure to read our contribution guidelines: https://github.com/apache/trafficcontrol/blob/master/CONTRIBUTING.md
If this closes or relates to an existing issue, please reference it using one of the following:
Closes: #ISSUE
Related: #ISSUE
If this PR fixes a security vulnerability, DO NOT submit! Instead, contact
the Apache Traffic Control Security Team at security@trafficcontrol.apache.org and follow the
guidelines at https://apache.org/security regarding vulnerability disclosure.
-->
Closed: #https://github.com/apache/trafficcontrol/issues/6234
<!-- **^ Add meaningful description above** --><hr/>
## Which Traffic Control components are affected by this PR?
<!-- Please delete all components from this list that are NOT affected by this PR.
Feel free to add the name of a tool or script that is affected but not on the list.
-->
- Documentation
- Traffic Ops
- Traffic Portal
## What is the best way to verify this PR?
<!-- Please include here ALL the steps necessary to test your PR.
If your PR has tests (and most should), provide the steps needed to run the tests.
If not, please provide step-by-step instructions to test the PR manually and explain why your PR does not need tests. -->
Use TP and curl cmds to perform all CRUD operations for server capability.
## If this is a bugfix, which Traffic Control versions contained the bug?
<!-- Delete this section if the PR is not a bugfix, or if the bug is only in the master branch.
Examples:
- 5.1.2
- 5.1.3 (RC1)
-->
## PR submission checklist
- [x] This PR has tests <!-- If not, please delete this text and explain why this PR does not need tests. -->
- [x] This PR has documentation <!-- If not, please delete this text and explain why this PR does not need documentation. -->
- [x] This PR has a CHANGELOG.md entry <!-- A fix for a bug from an ATC release, an improvement, or a new feature should have a changelog entry. -->
- [x] This PR **DOES NOT FIX A SERIOUS SECURITY VULNERABILITY** (see [the Apache Software Foundation's security guidelines](https://apache.org/security) for details)
<!--
Licensed to the Apache Software Foundation (ASF) under one
or more contributor license agreements. See the NOTICE file
distributed with this work for additional information
regarding copyright ownership. The ASF licenses this file
to you 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.
-->
--
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-unsubscribe@trafficcontrol.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] rimashah25 commented on a diff in pull request #7213: Added description field to server capabilities
Posted by GitBox <gi...@apache.org>.
rimashah25 commented on code in PR #7213:
URL: https://github.com/apache/trafficcontrol/pull/7213#discussion_r1051035999
##########
traffic_ops/traffic_ops_golang/dbhelpers/db_helpers_test.go:
##########
@@ -408,3 +408,55 @@ func TestGetCDNIDFromName(t *testing.T) {
}
}
+
+func TestGetSCInfo(t *testing.T) {
+ var testCases = []struct {
+ description string
+ name string
+ expectedError error
+ exists bool
+ }{
+ //{
+ // description: "Success: Get valid SC",
+ // name: "hdd",
+ // expectedError: nil,
+ // exists: true,
+ //},
+ {
+ description: "Failure: SC not in DB",
+ name: "disk",
+ expectedError: errors.New("error getting server capability info: sql: no rows in result set"),
+ exists: false,
+ },
+ }
+ for _, testCase := range testCases {
+ t.Run(testCase.description, func(t *testing.T) {
+ t.Log("Starting test scenario: ", testCase.description)
+ 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()
+
+ rows := sqlmock.NewRows([]string{"name"})
+ mock.ExpectBegin()
+ if testCase.exists {
+ rows = rows.AddRow(testCase.name)
+ }
+ mock.ExpectQuery("SELECT").WillReturnRows(rows)
+ mock.ExpectCommit()
+
+ scExists, err := GetSCInfo(db.MustBegin().Tx, testCase.name)
+ if testCase.exists != scExists {
+ t.Errorf("Expected return exists: %t, actual %t", testCase.exists, scExists)
+ }
+
+ if err == testCase.expectedError {
+ t.Errorf("getSCInfo expected: %s, actual: %s", testCase.expectedError, err)
+ }
+ })
+ }
+}
Review Comment:
Thanks, I needed to make a couple more changes along with your suggestion.
--
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-unsubscribe@trafficcontrol.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] github-code-scanning[bot] commented on a diff in pull request #7213: Added description field to server capabilities
Posted by GitBox <gi...@apache.org>.
github-code-scanning[bot] commented on code in PR #7213:
URL: https://github.com/apache/trafficcontrol/pull/7213#discussion_r1047658449
##########
traffic_ops/traffic_ops_golang/servercapability/servercapability.go:
##########
@@ -151,6 +153,203 @@
return nil, nil, http.StatusOK
}
+func GetServerCapability(w http.ResponseWriter, r *http.Request) {
+ var sc tc.ServerCapabilityV4
+ inf, userErr, sysErr, errCode := api.NewInfo(r, nil, nil)
+ tx := inf.Tx
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+ defer inf.Close()
+
+ // Query Parameters to Database Query column mappings
+ queryParamsToQueryCols := map[string]dbhelpers.WhereColumnInfo{
+ "name": {Column: "sc.name", Checker: nil},
+ }
+ if _, ok := inf.Params["orderby"]; !ok {
+ inf.Params["orderby"] = "name"
+ }
+ where, orderBy, pagination, queryValues, errs := dbhelpers.BuildWhereAndOrderByAndPagination(inf.Params, queryParamsToQueryCols)
+ if len(errs) > 0 {
+ api.HandleErr(w, r, tx.Tx, http.StatusBadRequest, util.JoinErrs(errs), nil)
+ return
+ }
+
+ selectQuery := "SELECT name, description, last_updated FROM server_capability sc"
+ query := selectQuery + where + orderBy + pagination
+ rows, err := tx.NamedQuery(query, queryValues)
+ if err != nil {
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, nil, fmt.Errorf("server capability read: error getting server capability(ies): %w", err))
+ return
+ }
+ defer log.Close(rows, "unable to close DB connection")
+
+ scList := []tc.ServerCapabilityV4{}
+ for rows.Next() {
+ if err = rows.Scan(&sc.Name, &sc.Description, &sc.LastUpdated); err != nil {
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, nil, fmt.Errorf("error getting server capability(ies): %w", err))
+ return
+ }
+ scList = append(scList, sc)
+ }
+
+ api.WriteResp(w, r, scList)
+ return
+}
+
+func UpdateServerCapability(w http.ResponseWriter, r *http.Request) {
+ inf, userErr, sysErr, errCode := api.NewInfo(r, nil, nil)
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+ defer inf.Close()
+
+ tx := inf.Tx.Tx
+ sc, readValErr := readAndValidateJsonStruct(r)
+ if readValErr != nil {
+ api.HandleErr(w, r, tx, http.StatusBadRequest, readValErr, nil)
+ return
+ }
+
+ requestedName := inf.Params["name"]
+ // check if the entity was already updated
+ userErr, sysErr, errCode = api.CheckIfUnModifiedByName(r.Header, inf.Tx, requestedName, "server_capability")
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, tx, errCode, userErr, sysErr)
+ return
+ }
+
+ //update name and description of a capability
+ query := `UPDATE server_capability sc SET
+ name = $1,
+ description = $2
+ WHERE sc.name = $3
+ RETURNING sc.name, sc.description, sc.last_updated`
+
+ err := tx.QueryRow(query, sc.Name, sc.Description, requestedName).Scan(&sc.Name, &sc.Description, &sc.LastUpdated)
+ if err != nil && err == sql.ErrNoRows {
+ api.HandleErr(w, r, tx, http.StatusBadRequest, err, nil)
+ return
+ }
+ alerts := tc.CreateAlerts(tc.SuccessLevel, "server capability was updated")
+ api.WriteAlertsObj(w, r, http.StatusOK, alerts, sc)
+ return
+}
+
+func CreateServerCapability(w http.ResponseWriter, r *http.Request) {
+ inf, userErr, sysErr, errCode := api.NewInfo(r, nil, nil)
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+ defer inf.Close()
+ tx := inf.Tx.Tx
+
+ sc, readValErr := readAndValidateJsonStruct(r)
+ if readValErr != nil {
+ api.HandleErr(w, r, tx, http.StatusBadRequest, readValErr, nil)
+ return
+ }
+
+ // check if capability already exists
+ var count int
+ err := tx.QueryRow("SELECT count(*) from server_capability where name = $1", sc.Name).Scan(&count)
+ if err != nil {
+ api.HandleErr(w, r, tx, http.StatusInternalServerError, nil, fmt.Errorf("checking if server capability with name %s exists", sc.Name))
+ return
+ }
+ if count == 1 {
+ api.HandleErr(w, r, tx, http.StatusBadRequest, fmt.Errorf("capability with name: %s already exists", sc.Name), nil)
+ return
+ }
+
+ // create server capability
+ query := `INSERT INTO server_capability (name, description) VALUES ($1, $2) RETURNING last_updated`
+ err = tx.QueryRow(query, sc.Name, sc.Description).Scan(&sc.LastUpdated)
+ if err != nil && err == sql.ErrNoRows {
+ api.HandleErr(w, r, tx, http.StatusInternalServerError, nil, err)
+ return
+ }
+ alerts := tc.CreateAlerts(tc.SuccessLevel, "server capability was created")
+ api.WriteAlertsObj(w, r, http.StatusOK, alerts, sc)
+ return
+}
+
+func DeleteServerCapability(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)
+ return
+ }
+ defer inf.Close()
+
+ name := inf.Params["name"]
+ exists, err := dbhelpers.GetSCInfo(inf.Tx, name)
+ fmt.Println(exists, name)
Review Comment:
## Log entries created from user input
This log entry depends on a [user-provided value](1).
[Show more details](https://github.com/apache/trafficcontrol/security/code-scanning/251)
--
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-unsubscribe@trafficcontrol.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] rimashah25 commented on a diff in pull request #7213: Added description field to server capabilities
Posted by GitBox <gi...@apache.org>.
rimashah25 commented on code in PR #7213:
URL: https://github.com/apache/trafficcontrol/pull/7213#discussion_r1042731170
##########
traffic_portal/test/integration/Data/servercapabilities.ts:
##########
@@ -47,67 +48,75 @@ export const serverCapabilities = {
],
check: [
{
- description: "check CSV link from Server Capabilities page",
+ caseDescription: "check CSV link from Server Capabilities page",
Name: "Export as CSV"
}
],
add: [
{
- description: "can create a server capability",
+ caseDescription: "can create a server capability",
name: "TP_SC",
+ description: "Server Capability",
Review Comment:
Oops.. I forgot to add it.. thanks for catching it.
--
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-unsubscribe@trafficcontrol.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] codecov[bot] commented on pull request #7213: Added description field to server capabilities
Posted by GitBox <gi...@apache.org>.
codecov[bot] commented on PR #7213:
URL: https://github.com/apache/trafficcontrol/pull/7213#issuecomment-1322885454
# [Codecov](https://codecov.io/gh/apache/trafficcontrol/pull/7213?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#7213](https://codecov.io/gh/apache/trafficcontrol/pull/7213?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (5ba540a) into [master](https://codecov.io/gh/apache/trafficcontrol/commit/14d7f253edaa3fe4069eadbba9e513639d991721?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (14d7f25) will **decrease** coverage by `3.74%`.
> The diff coverage is `n/a`.
```diff
@@ Coverage Diff @@
## master #7213 +/- ##
==========================================
- Coverage 69.71% 65.97% -3.75%
==========================================
Files 167 111 -56
Lines 5647 4100 -1547
==========================================
- Hits 3937 2705 -1232
+ Misses 1461 1226 -235
+ Partials 249 169 -80
```
| Flag | Coverage Δ | |
|---|---|---|
| traffic_ops_integration | `65.97% <ø> (-3.75%)` | :arrow_down: |
| v3 | `57.68% <ø> (ø)` | |
| v4 | `79.65% <ø> (ø)` | |
| v5 | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/trafficcontrol/pull/7213?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [traffic\_ops/v5-client/ping.go](https://codecov.io/gh/apache/trafficcontrol/pull/7213/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-dHJhZmZpY19vcHMvdjUtY2xpZW50L3BpbmcuZ28=) | | |
| [...5-client/deliveryservices\_required\_capabilities.go](https://codecov.io/gh/apache/trafficcontrol/pull/7213/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-dHJhZmZpY19vcHMvdjUtY2xpZW50L2RlbGl2ZXJ5c2VydmljZXNfcmVxdWlyZWRfY2FwYWJpbGl0aWVzLmdv) | | |
| [traffic\_ops/v5-client/servercheckextensions.go](https://codecov.io/gh/apache/trafficcontrol/pull/7213/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-dHJhZmZpY19vcHMvdjUtY2xpZW50L3NlcnZlcmNoZWNrZXh0ZW5zaW9ucy5nbw==) | | |
| [traffic\_ops/v5-client/federation\_resolver.go](https://codecov.io/gh/apache/trafficcontrol/pull/7213/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-dHJhZmZpY19vcHMvdjUtY2xpZW50L2ZlZGVyYXRpb25fcmVzb2x2ZXIuZ28=) | | |
| [traffic\_ops/v5-client/division.go](https://codecov.io/gh/apache/trafficcontrol/pull/7213/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-dHJhZmZpY19vcHMvdjUtY2xpZW50L2RpdmlzaW9uLmdv) | | |
| [traffic\_ops/v5-client/endpoints.go](https://codecov.io/gh/apache/trafficcontrol/pull/7213/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-dHJhZmZpY19vcHMvdjUtY2xpZW50L2VuZHBvaW50cy5nbw==) | | |
| [...ic\_ops/v5-client/federation\_federation\_resolver.go](https://codecov.io/gh/apache/trafficcontrol/pull/7213/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-dHJhZmZpY19vcHMvdjUtY2xpZW50L2ZlZGVyYXRpb25fZmVkZXJhdGlvbl9yZXNvbHZlci5nbw==) | | |
| [traffic\_ops/v5-client/acme.go](https://codecov.io/gh/apache/trafficcontrol/pull/7213/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-dHJhZmZpY19vcHMvdjUtY2xpZW50L2FjbWUuZ28=) | | |
| [traffic\_ops/v5-client/crconfig.go](https://codecov.io/gh/apache/trafficcontrol/pull/7213/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-dHJhZmZpY19vcHMvdjUtY2xpZW50L2NyY29uZmlnLmdv) | | |
| [traffic\_ops/v5-client/user.go](https://codecov.io/gh/apache/trafficcontrol/pull/7213/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-dHJhZmZpY19vcHMvdjUtY2xpZW50L3VzZXIuZ28=) | | |
| ... and [46 more](https://codecov.io/gh/apache/trafficcontrol/pull/7213/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
--
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-unsubscribe@trafficcontrol.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] rimashah25 commented on a diff in pull request #7213: Added description field to server capabilities
Posted by GitBox <gi...@apache.org>.
rimashah25 commented on code in PR #7213:
URL: https://github.com/apache/trafficcontrol/pull/7213#discussion_r1042620775
##########
traffic_portal/app/src/common/modules/table/serverCapabilities/table.serverCapabilities.tpl.html:
##########
@@ -34,11 +34,13 @@
<thead>
<tr>
<th>Name</th>
+ <th>Description</th>
</tr>
</thead>
<tbody>
<tr ng-click="editServerCapability(sc.name)" ng-repeat="sc in ::serverCapabilities" context-menu="contextMenuItems">
<td name="name" data-search="^{{::sc.name}}$">{{::sc.name}}</td>
+ <td name="name" data-search="^{{::sc.description}}$">{{::sc.description}}</td>
Review Comment:
Above comment is a refactor and better off in a new PR. Lets create an issue.
--
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-unsubscribe@trafficcontrol.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] rimashah25 commented on a diff in pull request #7213: Added description field to server capabilities
Posted by GitBox <gi...@apache.org>.
rimashah25 commented on code in PR #7213:
URL: https://github.com/apache/trafficcontrol/pull/7213#discussion_r1051064103
##########
traffic_ops/traffic_ops_golang/servercapability/servercapability.go:
##########
@@ -151,6 +153,213 @@ func (v *TOServerCapability) Update(h http.Header) (error, error, int) {
return nil, nil, http.StatusOK
}
+func GetServerCapability(w http.ResponseWriter, r *http.Request) {
+ var sc tc.ServerCapabilityV4
+ inf, userErr, sysErr, errCode := api.NewInfo(r, nil, nil)
+ tx := inf.Tx
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+ defer inf.Close()
+
+ // Query Parameters to Database Query column mappings
+ queryParamsToQueryCols := map[string]dbhelpers.WhereColumnInfo{
+ "name": {Column: "sc.name", Checker: nil},
+ }
+ if _, ok := inf.Params["orderby"]; !ok {
+ inf.Params["orderby"] = "name"
+ }
+ where, orderBy, pagination, queryValues, errs := dbhelpers.BuildWhereAndOrderByAndPagination(inf.Params, queryParamsToQueryCols)
+ if len(errs) > 0 {
+ api.HandleErr(w, r, tx.Tx, http.StatusBadRequest, util.JoinErrs(errs), nil)
+ return
+ }
+
+ selectQuery := "SELECT name, description, last_updated FROM server_capability sc"
+ query := selectQuery + where + orderBy + pagination
+ rows, err := tx.NamedQuery(query, queryValues)
+ if err != nil {
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, nil, fmt.Errorf("server capability read: error getting server capability(ies): %w", err))
+ return
+ }
+ defer log.Close(rows, "unable to close DB connection")
+
+ scList := []tc.ServerCapabilityV4{}
+ for rows.Next() {
+ if err = rows.Scan(&sc.Name, &sc.Description, &sc.LastUpdated); err != nil {
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, nil, fmt.Errorf("error getting server capability(ies): %w", err))
+ return
+ }
+ scList = append(scList, sc)
+ }
+
+ api.WriteResp(w, r, scList)
+ return
+}
+
+func UpdateServerCapability(w http.ResponseWriter, r *http.Request) {
+ inf, userErr, sysErr, errCode := api.NewInfo(r, nil, nil)
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+ defer inf.Close()
+
+ tx := inf.Tx.Tx
+ sc, readValErr := readAndValidateJsonStruct(r)
+ if readValErr != nil {
+ api.HandleErr(w, r, tx, http.StatusBadRequest, readValErr, nil)
+ return
+ }
+
+ requestedName := inf.Params["name"]
+ // check if the entity was already updated
+ userErr, sysErr, errCode = api.CheckIfUnModifiedByName(r.Header, inf.Tx, requestedName, "server_capability")
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, tx, errCode, userErr, sysErr)
+ return
+ }
+
+ //update name and description of a capability
+ query := `UPDATE server_capability sc SET
+ name = $1,
+ description = $2
+ WHERE sc.name = $3
+ RETURNING sc.name, sc.description, sc.last_updated`
+
+ err := tx.QueryRow(query, sc.Name, sc.Description, requestedName).Scan(&sc.Name, &sc.Description, &sc.LastUpdated)
+ if err != nil {
+ if err == sql.ErrNoRows {
+ api.HandleErr(w, r, tx, http.StatusBadRequest, err, nil)
+ return
+ }
Review Comment:
Based on offline conversation, this check is here to stay
--
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-unsubscribe@trafficcontrol.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] ocket8888 commented on a diff in pull request #7213: Added description field to server capabilities
Posted by GitBox <gi...@apache.org>.
ocket8888 commented on code in PR #7213:
URL: https://github.com/apache/trafficcontrol/pull/7213#discussion_r1029909191
##########
docs/source/api/v3/server_capabilities.rst:
##########
@@ -70,6 +71,7 @@ Response Structure
"response": [
{
"name": "RAM",
+ "description": "ram server capability",
Review Comment:
We can't add properties to released API versions; i.e. 3.0, 3.1, and 4.0
##########
lib/go-tc/server_capabilities.go:
##########
@@ -28,6 +28,7 @@ type ServerCapabilitiesResponse struct {
// ServerCapability contains information about a given ServerCapability in Traffic Ops.
type ServerCapability struct {
Name string `json:"name" db:"name"`
+ Description string `json:"description" db:"description"`
Review Comment:
This change should result in a new Server Capability structure instead of modifying the old one. Using this structure in old API versions will result in them having a `description` property, which they can't have.
##########
traffic_portal/app/src/common/modules/form/serverCapability/form.serverCapability.tpl.html:
##########
@@ -41,6 +41,13 @@
<span ng-show="hasError(serverCapabilityForm.name)" class="form-control-feedback"><i class="fa fa-times"></i></span>
</div>
</div>
+ <div class="form-group" ng-class="{'has-error': hasError(serverCapabilityForm.description), 'has-feedback': hasError(serverCapabilityForm.description)}">
Review Comment:
I don't think it's possible for this field to have errors or feedback
##########
traffic_ops/testing/api/v3/tc-fixtures.json:
##########
@@ -4822,22 +4822,28 @@
],
"serverCapabilities": [
{
- "name": "foo"
+ "name": "foo",
+ "description": "server capability foo"
Review Comment:
This won't fail tests, so I feel like I should call it out so it's not accidentally overlooked - extraneous fields shouldn't be present in the testing data.
##########
traffic_ops/app/db/migrations/2022112116304800_add_description_in_sc_table.up.sql:
##########
@@ -0,0 +1,18 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with this
+ * work for additional information regarding copyright ownership. The ASF
+ * licenses this file to you 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.
+ */
+
+ALTER TABLE public.server_capability ADD COLUMN IF NOT EXISTS description text DEFAULT NULL;
Review Comment:
The structure that models this uses `string` which cannot store a `NULL` driver value. If a server capability exists with a null description, attempting to read it into a pointer to a `string` will fail and return an error. Instead, this should actually be `NOT NULL` and have a default value of `''`.
##########
docs/source/api/v4/server_capabilities.rst:
##########
@@ -50,6 +50,7 @@ Request Structure
Response Structure
------------------
:name: The name of this :term:`Server Capability`
+:description: The description of this :term:`Server Capability`
Review Comment:
This should have a `..versionadded::` directive specifying that this was added in 4.1 - because it can't go into 4.0
##########
traffic_portal/app/src/common/modules/form/serverCapability/form.serverCapability.tpl.html:
##########
@@ -41,6 +41,13 @@
<span ng-show="hasError(serverCapabilityForm.name)" class="form-control-feedback"><i class="fa fa-times"></i></span>
</div>
</div>
+ <div class="form-group" ng-class="{'has-error': hasError(serverCapabilityForm.description), 'has-feedback': hasError(serverCapabilityForm.description)}">
+ <label class="control-label col-md-2 col-sm-2 col-xs-12" >Description *</label>
Review Comment:
labels should have a `for` attribute that links them to the thing they label
##########
traffic_portal/app/src/common/modules/form/serverCapability/form.serverCapability.tpl.html:
##########
@@ -41,6 +41,13 @@
<span ng-show="hasError(serverCapabilityForm.name)" class="form-control-feedback"><i class="fa fa-times"></i></span>
</div>
</div>
+ <div class="form-group" ng-class="{'has-error': hasError(serverCapabilityForm.description), 'has-feedback': hasError(serverCapabilityForm.description)}">
+ <label class="control-label col-md-2 col-sm-2 col-xs-12" >Description *</label>
+ <div class="col-md-10 col-sm-10 col-xs-12">
+ <textarea id="description" name="description" rows="3" cols="17" class="form-control" ng-model="serverCapability.description" maxlength="256" autofocus></textarea>
+ <span ng-show="hasError(serverCapabilityForm.description)" class="form-control-feedback"><i class="fa fa-times"></i></span>
Review Comment:
I don't think it's possible for the textarea to have an error. Its only constraint is maxlength, and that means that user agents won't allow the user to type more than that, so it'll never violate that.
##########
traffic_portal/app/src/common/modules/table/serverCapabilities/table.serverCapabilities.tpl.html:
##########
@@ -34,11 +34,13 @@
<thead>
<tr>
<th>Name</th>
+ <th>Description</th>
</tr>
</thead>
<tbody>
<tr ng-click="editServerCapability(sc.name)" ng-repeat="sc in ::serverCapabilities" context-menu="contextMenuItems">
<td name="name" data-search="^{{::sc.name}}$">{{::sc.name}}</td>
+ <td name="name" data-search="^{{::sc.description}}$">{{::sc.description}}</td>
Review Comment:
this is a small enough change that you probably don't need to bother with it, but for what it's worth we should convert that table to use the common grid controller at some point.
##########
traffic_portal/app/src/common/modules/form/serverCapability/form.serverCapability.tpl.html:
##########
@@ -41,6 +41,13 @@
<span ng-show="hasError(serverCapabilityForm.name)" class="form-control-feedback"><i class="fa fa-times"></i></span>
</div>
</div>
+ <div class="form-group" ng-class="{'has-error': hasError(serverCapabilityForm.description), 'has-feedback': hasError(serverCapabilityForm.description)}">
+ <label class="control-label col-md-2 col-sm-2 col-xs-12" >Description *</label>
+ <div class="col-md-10 col-sm-10 col-xs-12">
+ <textarea id="description" name="description" rows="3" cols="17" class="form-control" ng-model="serverCapability.description" maxlength="256" autofocus></textarea>
Review Comment:
This should not have the `autofocus` attribute; the name input already does and appears before it in the form.
##########
traffic_portal/test/integration/Data/servercapabilities.ts:
##########
@@ -47,67 +48,75 @@ export const serverCapabilities = {
],
check: [
{
- description: "check CSV link from Server Capabilities page",
+ caseDescription: "check CSV link from Server Capabilities page",
Name: "Export as CSV"
}
],
add: [
{
- description: "can create a server capability",
+ caseDescription: "can create a server capability",
name: "TP_SC",
+ description: "Server Capability",
Review Comment:
These descriptions aren't being used in the spec. The add cases should be setting the descriptions from these values.
--
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-unsubscribe@trafficcontrol.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] rimashah25 commented on a diff in pull request #7213: Added description field to server capabilities
Posted by GitBox <gi...@apache.org>.
rimashah25 commented on code in PR #7213:
URL: https://github.com/apache/trafficcontrol/pull/7213#discussion_r1039910430
##########
traffic_portal/app/src/common/modules/form/serverCapability/form.serverCapability.tpl.html:
##########
@@ -41,6 +41,13 @@
<span ng-show="hasError(serverCapabilityForm.name)" class="form-control-feedback"><i class="fa fa-times"></i></span>
</div>
</div>
+ <div class="form-group" ng-class="{'has-error': hasError(serverCapabilityForm.description), 'has-feedback': hasError(serverCapabilityForm.description)}">
Review Comment:
ok
--
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-unsubscribe@trafficcontrol.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] ocket8888 commented on a diff in pull request #7213: Added description field to server capabilities
Posted by GitBox <gi...@apache.org>.
ocket8888 commented on code in PR #7213:
URL: https://github.com/apache/trafficcontrol/pull/7213#discussion_r1050958389
##########
traffic_ops/traffic_ops_golang/servercapability/servercapability.go:
##########
@@ -151,6 +153,213 @@ func (v *TOServerCapability) Update(h http.Header) (error, error, int) {
return nil, nil, http.StatusOK
}
+func GetServerCapability(w http.ResponseWriter, r *http.Request) {
+ var sc tc.ServerCapabilityV4
+ inf, userErr, sysErr, errCode := api.NewInfo(r, nil, nil)
+ tx := inf.Tx
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+ defer inf.Close()
+
+ // Query Parameters to Database Query column mappings
+ queryParamsToQueryCols := map[string]dbhelpers.WhereColumnInfo{
+ "name": {Column: "sc.name", Checker: nil},
+ }
+ if _, ok := inf.Params["orderby"]; !ok {
+ inf.Params["orderby"] = "name"
+ }
+ where, orderBy, pagination, queryValues, errs := dbhelpers.BuildWhereAndOrderByAndPagination(inf.Params, queryParamsToQueryCols)
+ if len(errs) > 0 {
+ api.HandleErr(w, r, tx.Tx, http.StatusBadRequest, util.JoinErrs(errs), nil)
+ return
+ }
+
+ selectQuery := "SELECT name, description, last_updated FROM server_capability sc"
+ query := selectQuery + where + orderBy + pagination
+ rows, err := tx.NamedQuery(query, queryValues)
+ if err != nil {
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, nil, fmt.Errorf("server capability read: error getting server capability(ies): %w", err))
+ return
+ }
+ defer log.Close(rows, "unable to close DB connection")
+
+ scList := []tc.ServerCapabilityV4{}
+ for rows.Next() {
+ if err = rows.Scan(&sc.Name, &sc.Description, &sc.LastUpdated); err != nil {
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, nil, fmt.Errorf("error getting server capability(ies): %w", err))
+ return
+ }
+ scList = append(scList, sc)
+ }
+
+ api.WriteResp(w, r, scList)
+ return
+}
+
+func UpdateServerCapability(w http.ResponseWriter, r *http.Request) {
+ inf, userErr, sysErr, errCode := api.NewInfo(r, nil, nil)
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+ defer inf.Close()
+
+ tx := inf.Tx.Tx
+ sc, readValErr := readAndValidateJsonStruct(r)
+ if readValErr != nil {
+ api.HandleErr(w, r, tx, http.StatusBadRequest, readValErr, nil)
+ return
+ }
+
+ requestedName := inf.Params["name"]
+ // check if the entity was already updated
+ userErr, sysErr, errCode = api.CheckIfUnModifiedByName(r.Header, inf.Tx, requestedName, "server_capability")
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, tx, errCode, userErr, sysErr)
+ return
+ }
+
+ //update name and description of a capability
+ query := `UPDATE server_capability sc SET
+ name = $1,
+ description = $2
+ WHERE sc.name = $3
+ RETURNING sc.name, sc.description, sc.last_updated`
+
+ err := tx.QueryRow(query, sc.Name, sc.Description, requestedName).Scan(&sc.Name, &sc.Description, &sc.LastUpdated)
+ if err != nil {
+ if err == sql.ErrNoRows {
+ api.HandleErr(w, r, tx, http.StatusBadRequest, err, nil)
+ return
+ }
+ usrErr, sysErr, code := api.ParseDBError(err)
+ api.HandleErr(w, r, tx, code, usrErr, sysErr)
+ return
+ }
+ alerts := tc.CreateAlerts(tc.SuccessLevel, "server capability was updated")
+ api.WriteAlertsObj(w, r, http.StatusOK, alerts, sc)
+ return
+}
+
+func CreateServerCapability(w http.ResponseWriter, r *http.Request) {
+ inf, userErr, sysErr, errCode := api.NewInfo(r, nil, nil)
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+ defer inf.Close()
+ tx := inf.Tx.Tx
+
+ sc, readValErr := readAndValidateJsonStruct(r)
+ if readValErr != nil {
+ api.HandleErr(w, r, tx, http.StatusBadRequest, readValErr, nil)
+ return
+ }
+
+ // check if capability already exists
+ var count int
+ err := tx.QueryRow("SELECT count(*) from server_capability where name = $1", sc.Name).Scan(&count)
+ if err != nil {
+ api.HandleErr(w, r, tx, http.StatusInternalServerError, nil, fmt.Errorf("error: %w, when checking if server capability with name %s exists", err, sc.Name))
+ return
+ }
+ if count == 1 {
+ api.HandleErr(w, r, tx, http.StatusBadRequest, fmt.Errorf("server_capability name '%s' already exists.", sc.Name), nil)
Review Comment:
error strings shouldn't end with punctuation
##########
traffic_ops/traffic_ops_golang/servercapability/servercapability.go:
##########
@@ -151,6 +153,213 @@ func (v *TOServerCapability) Update(h http.Header) (error, error, int) {
return nil, nil, http.StatusOK
}
+func GetServerCapability(w http.ResponseWriter, r *http.Request) {
+ var sc tc.ServerCapabilityV4
+ inf, userErr, sysErr, errCode := api.NewInfo(r, nil, nil)
+ tx := inf.Tx
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+ defer inf.Close()
+
+ // Query Parameters to Database Query column mappings
+ queryParamsToQueryCols := map[string]dbhelpers.WhereColumnInfo{
+ "name": {Column: "sc.name", Checker: nil},
+ }
+ if _, ok := inf.Params["orderby"]; !ok {
+ inf.Params["orderby"] = "name"
+ }
+ where, orderBy, pagination, queryValues, errs := dbhelpers.BuildWhereAndOrderByAndPagination(inf.Params, queryParamsToQueryCols)
+ if len(errs) > 0 {
+ api.HandleErr(w, r, tx.Tx, http.StatusBadRequest, util.JoinErrs(errs), nil)
+ return
+ }
+
+ selectQuery := "SELECT name, description, last_updated FROM server_capability sc"
+ query := selectQuery + where + orderBy + pagination
+ rows, err := tx.NamedQuery(query, queryValues)
+ if err != nil {
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, nil, fmt.Errorf("server capability read: error getting server capability(ies): %w", err))
+ return
+ }
+ defer log.Close(rows, "unable to close DB connection")
+
+ scList := []tc.ServerCapabilityV4{}
+ for rows.Next() {
+ if err = rows.Scan(&sc.Name, &sc.Description, &sc.LastUpdated); err != nil {
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, nil, fmt.Errorf("error getting server capability(ies): %w", err))
+ return
+ }
+ scList = append(scList, sc)
+ }
+
+ api.WriteResp(w, r, scList)
+ return
+}
+
+func UpdateServerCapability(w http.ResponseWriter, r *http.Request) {
+ inf, userErr, sysErr, errCode := api.NewInfo(r, nil, nil)
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+ defer inf.Close()
+
+ tx := inf.Tx.Tx
+ sc, readValErr := readAndValidateJsonStruct(r)
+ if readValErr != nil {
+ api.HandleErr(w, r, tx, http.StatusBadRequest, readValErr, nil)
+ return
+ }
+
+ requestedName := inf.Params["name"]
+ // check if the entity was already updated
+ userErr, sysErr, errCode = api.CheckIfUnModifiedByName(r.Header, inf.Tx, requestedName, "server_capability")
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, tx, errCode, userErr, sysErr)
+ return
+ }
+
+ //update name and description of a capability
+ query := `UPDATE server_capability sc SET
+ name = $1,
+ description = $2
+ WHERE sc.name = $3
+ RETURNING sc.name, sc.description, sc.last_updated`
+
+ err := tx.QueryRow(query, sc.Name, sc.Description, requestedName).Scan(&sc.Name, &sc.Description, &sc.LastUpdated)
+ if err != nil {
+ if err == sql.ErrNoRows {
+ api.HandleErr(w, r, tx, http.StatusBadRequest, err, nil)
+ return
+ }
+ usrErr, sysErr, code := api.ParseDBError(err)
+ api.HandleErr(w, r, tx, code, usrErr, sysErr)
+ return
+ }
+ alerts := tc.CreateAlerts(tc.SuccessLevel, "server capability was updated")
+ api.WriteAlertsObj(w, r, http.StatusOK, alerts, sc)
+ return
+}
+
+func CreateServerCapability(w http.ResponseWriter, r *http.Request) {
+ inf, userErr, sysErr, errCode := api.NewInfo(r, nil, nil)
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+ defer inf.Close()
+ tx := inf.Tx.Tx
+
+ sc, readValErr := readAndValidateJsonStruct(r)
+ if readValErr != nil {
+ api.HandleErr(w, r, tx, http.StatusBadRequest, readValErr, nil)
+ return
+ }
+
+ // check if capability already exists
+ var count int
+ err := tx.QueryRow("SELECT count(*) from server_capability where name = $1", sc.Name).Scan(&count)
+ if err != nil {
+ api.HandleErr(w, r, tx, http.StatusInternalServerError, nil, fmt.Errorf("error: %w, when checking if server capability with name %s exists", err, sc.Name))
+ return
+ }
+ if count == 1 {
+ api.HandleErr(w, r, tx, http.StatusBadRequest, fmt.Errorf("server_capability name '%s' already exists.", sc.Name), nil)
+ return
+ }
+
+ // create server capability
+ query := `INSERT INTO server_capability (name, description) VALUES ($1, $2) RETURNING last_updated`
+ err = tx.QueryRow(query, sc.Name, sc.Description).Scan(&sc.LastUpdated)
+ if err != nil {
+ if err == sql.ErrNoRows {
+ api.HandleErr(w, r, tx, http.StatusBadRequest, err, nil)
+ return
+ }
+ usrErr, sysErr, code := api.ParseDBError(err)
+ api.HandleErr(w, r, tx, code, usrErr, sysErr)
+ return
+ }
+ alerts := tc.CreateAlerts(tc.SuccessLevel, "server capability was created.")
+ w.Header().Set("Location", fmt.Sprintf("/api/%d.%d/server_capabilities$?name=%s", inf.Version.Major, inf.Version.Minor, sc.Name))
+ api.WriteAlertsObj(w, r, http.StatusCreated, alerts, sc)
+ return
+}
+
+func DeleteServerCapability(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)
+ return
+ }
+ defer inf.Close()
+
+ name := inf.Params["name"]
+ exists, err := dbhelpers.GetSCInfo(tx, name)
+ if err != nil {
+ api.HandleErr(w, r, tx, http.StatusInternalServerError, nil, err)
+ return
+ }
+ if !exists {
+ if name != "" {
+ api.HandleErr(w, r, tx, http.StatusNotFound, fmt.Errorf("no server capability exists by name: %s", name), nil)
+ return
+ } else {
+ api.HandleErr(w, r, tx, http.StatusBadRequest, fmt.Errorf("no server capability exists for empty name: %s", name), nil)
+ return
+ }
+ }
+
+ assignedServer := 0
+ if err := inf.Tx.Get(&assignedServer, "SELECT count(server) FROM server_server_capability ssc WHERE ssc.server_capability=$1", name); err != nil {
+ api.HandleErr(w, r, tx, http.StatusInternalServerError, nil, fmt.Errorf("server capability delete, counting assigned servers: %w", err))
+ return
+ } else if assignedServer != 0 {
+ api.HandleErr(w, r, tx, http.StatusBadRequest, fmt.Errorf("can not delete a server capability with %d assigned servers", assignedServer), nil)
+ return
+ }
+
+ res, err := tx.Exec("DELETE FROM server_capability AS sc WHERE sc.name=$1", name)
+ if err != nil {
+ api.HandleErr(w, r, tx, http.StatusInternalServerError, nil, err)
+ return
+ }
+ rowsAffected, err := res.RowsAffected()
+ if err != nil {
+ api.HandleErr(w, r, tx, http.StatusInternalServerError, nil, fmt.Errorf("determining rows affected for delete server capability: %w", err))
+ return
+ }
+ if rowsAffected == 0 {
+ api.HandleErr(w, r, tx, http.StatusBadRequest, fmt.Errorf("no rows deleted for server capability"), nil)
Review Comment:
This seems like an error that's internal to the server to me.
##########
traffic_ops/traffic_ops_golang/dbhelpers/db_helpers_test.go:
##########
@@ -408,3 +408,55 @@ func TestGetCDNIDFromName(t *testing.T) {
}
}
+
+func TestGetSCInfo(t *testing.T) {
+ var testCases = []struct {
+ description string
+ name string
+ expectedError error
+ exists bool
+ }{
+ //{
+ // description: "Success: Get valid SC",
+ // name: "hdd",
+ // expectedError: nil,
+ // exists: true,
+ //},
+ {
+ description: "Failure: SC not in DB",
+ name: "disk",
+ expectedError: errors.New("error getting server capability info: sql: no rows in result set"),
+ exists: false,
+ },
+ }
+ for _, testCase := range testCases {
+ t.Run(testCase.description, func(t *testing.T) {
+ t.Log("Starting test scenario: ", testCase.description)
Review Comment:
You shouldn't need to log this, since the results for the `t.Run` function will include the description in the name, and everything will be under that.
##########
traffic_ops/traffic_ops_golang/servercapability/servercapability.go:
##########
@@ -151,6 +153,213 @@ func (v *TOServerCapability) Update(h http.Header) (error, error, int) {
return nil, nil, http.StatusOK
}
+func GetServerCapability(w http.ResponseWriter, r *http.Request) {
+ var sc tc.ServerCapabilityV4
+ inf, userErr, sysErr, errCode := api.NewInfo(r, nil, nil)
+ tx := inf.Tx
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+ defer inf.Close()
+
+ // Query Parameters to Database Query column mappings
+ queryParamsToQueryCols := map[string]dbhelpers.WhereColumnInfo{
+ "name": {Column: "sc.name", Checker: nil},
+ }
+ if _, ok := inf.Params["orderby"]; !ok {
+ inf.Params["orderby"] = "name"
+ }
+ where, orderBy, pagination, queryValues, errs := dbhelpers.BuildWhereAndOrderByAndPagination(inf.Params, queryParamsToQueryCols)
+ if len(errs) > 0 {
+ api.HandleErr(w, r, tx.Tx, http.StatusBadRequest, util.JoinErrs(errs), nil)
+ return
+ }
+
+ selectQuery := "SELECT name, description, last_updated FROM server_capability sc"
+ query := selectQuery + where + orderBy + pagination
+ rows, err := tx.NamedQuery(query, queryValues)
+ if err != nil {
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, nil, fmt.Errorf("server capability read: error getting server capability(ies): %w", err))
+ return
+ }
+ defer log.Close(rows, "unable to close DB connection")
+
+ scList := []tc.ServerCapabilityV4{}
+ for rows.Next() {
+ if err = rows.Scan(&sc.Name, &sc.Description, &sc.LastUpdated); err != nil {
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, nil, fmt.Errorf("error getting server capability(ies): %w", err))
+ return
+ }
+ scList = append(scList, sc)
+ }
+
+ api.WriteResp(w, r, scList)
+ return
+}
+
+func UpdateServerCapability(w http.ResponseWriter, r *http.Request) {
+ inf, userErr, sysErr, errCode := api.NewInfo(r, nil, nil)
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+ defer inf.Close()
+
+ tx := inf.Tx.Tx
+ sc, readValErr := readAndValidateJsonStruct(r)
+ if readValErr != nil {
+ api.HandleErr(w, r, tx, http.StatusBadRequest, readValErr, nil)
+ return
+ }
+
+ requestedName := inf.Params["name"]
+ // check if the entity was already updated
+ userErr, sysErr, errCode = api.CheckIfUnModifiedByName(r.Header, inf.Tx, requestedName, "server_capability")
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, tx, errCode, userErr, sysErr)
+ return
+ }
+
+ //update name and description of a capability
+ query := `UPDATE server_capability sc SET
+ name = $1,
+ description = $2
+ WHERE sc.name = $3
+ RETURNING sc.name, sc.description, sc.last_updated`
+
+ err := tx.QueryRow(query, sc.Name, sc.Description, requestedName).Scan(&sc.Name, &sc.Description, &sc.LastUpdated)
+ if err != nil {
+ if err == sql.ErrNoRows {
+ api.HandleErr(w, r, tx, http.StatusBadRequest, err, nil)
+ return
+ }
+ usrErr, sysErr, code := api.ParseDBError(err)
+ api.HandleErr(w, r, tx, code, usrErr, sysErr)
+ return
+ }
+ alerts := tc.CreateAlerts(tc.SuccessLevel, "server capability was updated")
+ api.WriteAlertsObj(w, r, http.StatusOK, alerts, sc)
+ return
+}
+
+func CreateServerCapability(w http.ResponseWriter, r *http.Request) {
+ inf, userErr, sysErr, errCode := api.NewInfo(r, nil, nil)
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+ defer inf.Close()
+ tx := inf.Tx.Tx
+
+ sc, readValErr := readAndValidateJsonStruct(r)
+ if readValErr != nil {
+ api.HandleErr(w, r, tx, http.StatusBadRequest, readValErr, nil)
+ return
+ }
+
+ // check if capability already exists
+ var count int
+ err := tx.QueryRow("SELECT count(*) from server_capability where name = $1", sc.Name).Scan(&count)
+ if err != nil {
+ api.HandleErr(w, r, tx, http.StatusInternalServerError, nil, fmt.Errorf("error: %w, when checking if server capability with name %s exists", err, sc.Name))
+ return
+ }
+ if count == 1 {
+ api.HandleErr(w, r, tx, http.StatusBadRequest, fmt.Errorf("server_capability name '%s' already exists.", sc.Name), nil)
+ return
+ }
+
+ // create server capability
+ query := `INSERT INTO server_capability (name, description) VALUES ($1, $2) RETURNING last_updated`
+ err = tx.QueryRow(query, sc.Name, sc.Description).Scan(&sc.LastUpdated)
+ if err != nil {
+ if err == sql.ErrNoRows {
+ api.HandleErr(w, r, tx, http.StatusBadRequest, err, nil)
+ return
+ }
Review Comment:
If an `INSERT` affects zero rows without violating a constraint, that's not a bad request; it means that there's something very wrong with the database.
##########
traffic_ops/traffic_ops_golang/dbhelpers/db_helpers_test.go:
##########
@@ -408,3 +408,55 @@ func TestGetCDNIDFromName(t *testing.T) {
}
}
+
+func TestGetSCInfo(t *testing.T) {
+ var testCases = []struct {
+ description string
+ name string
+ expectedError error
+ exists bool
+ }{
+ //{
+ // description: "Success: Get valid SC",
+ // name: "hdd",
+ // expectedError: nil,
+ // exists: true,
+ //},
+ {
+ description: "Failure: SC not in DB",
+ name: "disk",
+ expectedError: errors.New("error getting server capability info: sql: no rows in result set"),
+ exists: false,
+ },
+ }
+ for _, testCase := range testCases {
+ t.Run(testCase.description, func(t *testing.T) {
+ t.Log("Starting test scenario: ", testCase.description)
+ 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()
+
+ rows := sqlmock.NewRows([]string{"name"})
+ mock.ExpectBegin()
+ if testCase.exists {
+ rows = rows.AddRow(testCase.name)
+ }
+ mock.ExpectQuery("SELECT").WillReturnRows(rows)
+ mock.ExpectCommit()
+
+ scExists, err := GetSCInfo(db.MustBegin().Tx, testCase.name)
+ if testCase.exists != scExists {
+ t.Errorf("Expected return exists: %t, actual %t", testCase.exists, scExists)
+ }
+
+ if err == testCase.expectedError {
+ t.Errorf("getSCInfo expected: %s, actual: %s", testCase.expectedError, err)
+ }
+ })
+ }
+}
Review Comment:
So, it seems you've commented out the positive test, and I think I know why. I was puzzled how the negative test would be passing since two `error`s aren't equal just because they have the same message, but then I realized: your negative test passing is a false positive, because your test fails *if the error you get is equal to the error you expect*, which is the opposite of what expectation implies. Line 457 should be doing the opposite of what it's doing, but that alone won't pass, because you also need the correct error specified on line 428 - I believe you're looking for `sql.ErrNoRows`, and that comparison should be done with `errors.Is`.
##########
traffic_ops/traffic_ops_golang/servercapability/servercapability.go:
##########
@@ -151,6 +153,213 @@ func (v *TOServerCapability) Update(h http.Header) (error, error, int) {
return nil, nil, http.StatusOK
}
+func GetServerCapability(w http.ResponseWriter, r *http.Request) {
+ var sc tc.ServerCapabilityV4
+ inf, userErr, sysErr, errCode := api.NewInfo(r, nil, nil)
+ tx := inf.Tx
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+ defer inf.Close()
+
+ // Query Parameters to Database Query column mappings
+ queryParamsToQueryCols := map[string]dbhelpers.WhereColumnInfo{
+ "name": {Column: "sc.name", Checker: nil},
+ }
+ if _, ok := inf.Params["orderby"]; !ok {
+ inf.Params["orderby"] = "name"
+ }
+ where, orderBy, pagination, queryValues, errs := dbhelpers.BuildWhereAndOrderByAndPagination(inf.Params, queryParamsToQueryCols)
+ if len(errs) > 0 {
+ api.HandleErr(w, r, tx.Tx, http.StatusBadRequest, util.JoinErrs(errs), nil)
+ return
+ }
+
+ selectQuery := "SELECT name, description, last_updated FROM server_capability sc"
+ query := selectQuery + where + orderBy + pagination
+ rows, err := tx.NamedQuery(query, queryValues)
+ if err != nil {
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, nil, fmt.Errorf("server capability read: error getting server capability(ies): %w", err))
+ return
+ }
+ defer log.Close(rows, "unable to close DB connection")
+
+ scList := []tc.ServerCapabilityV4{}
+ for rows.Next() {
+ if err = rows.Scan(&sc.Name, &sc.Description, &sc.LastUpdated); err != nil {
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, nil, fmt.Errorf("error getting server capability(ies): %w", err))
+ return
+ }
+ scList = append(scList, sc)
+ }
+
+ api.WriteResp(w, r, scList)
+ return
+}
+
+func UpdateServerCapability(w http.ResponseWriter, r *http.Request) {
+ inf, userErr, sysErr, errCode := api.NewInfo(r, nil, nil)
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+ defer inf.Close()
+
+ tx := inf.Tx.Tx
+ sc, readValErr := readAndValidateJsonStruct(r)
+ if readValErr != nil {
+ api.HandleErr(w, r, tx, http.StatusBadRequest, readValErr, nil)
+ return
+ }
+
+ requestedName := inf.Params["name"]
+ // check if the entity was already updated
+ userErr, sysErr, errCode = api.CheckIfUnModifiedByName(r.Header, inf.Tx, requestedName, "server_capability")
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, tx, errCode, userErr, sysErr)
+ return
+ }
+
+ //update name and description of a capability
+ query := `UPDATE server_capability sc SET
+ name = $1,
+ description = $2
+ WHERE sc.name = $3
+ RETURNING sc.name, sc.description, sc.last_updated`
+
+ err := tx.QueryRow(query, sc.Name, sc.Description, requestedName).Scan(&sc.Name, &sc.Description, &sc.LastUpdated)
+ if err != nil {
+ if err == sql.ErrNoRows {
+ api.HandleErr(w, r, tx, http.StatusBadRequest, err, nil)
+ return
+ }
Review Comment:
You don't need to bother checking this. The above call to `api.CheckIfUnModifiedByName` already covers the case that the SC doesn't exist.
##########
traffic_ops/traffic_ops_golang/servercapability/servercapability.go:
##########
@@ -151,6 +153,213 @@ func (v *TOServerCapability) Update(h http.Header) (error, error, int) {
return nil, nil, http.StatusOK
}
+func GetServerCapability(w http.ResponseWriter, r *http.Request) {
+ var sc tc.ServerCapabilityV4
+ inf, userErr, sysErr, errCode := api.NewInfo(r, nil, nil)
+ tx := inf.Tx
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+ defer inf.Close()
+
+ // Query Parameters to Database Query column mappings
+ queryParamsToQueryCols := map[string]dbhelpers.WhereColumnInfo{
+ "name": {Column: "sc.name", Checker: nil},
+ }
+ if _, ok := inf.Params["orderby"]; !ok {
+ inf.Params["orderby"] = "name"
+ }
+ where, orderBy, pagination, queryValues, errs := dbhelpers.BuildWhereAndOrderByAndPagination(inf.Params, queryParamsToQueryCols)
+ if len(errs) > 0 {
+ api.HandleErr(w, r, tx.Tx, http.StatusBadRequest, util.JoinErrs(errs), nil)
+ return
+ }
+
+ selectQuery := "SELECT name, description, last_updated FROM server_capability sc"
+ query := selectQuery + where + orderBy + pagination
+ rows, err := tx.NamedQuery(query, queryValues)
+ if err != nil {
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, nil, fmt.Errorf("server capability read: error getting server capability(ies): %w", err))
+ return
+ }
+ defer log.Close(rows, "unable to close DB connection")
+
+ scList := []tc.ServerCapabilityV4{}
+ for rows.Next() {
+ if err = rows.Scan(&sc.Name, &sc.Description, &sc.LastUpdated); err != nil {
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, nil, fmt.Errorf("error getting server capability(ies): %w", err))
+ return
+ }
+ scList = append(scList, sc)
+ }
+
+ api.WriteResp(w, r, scList)
+ return
+}
+
+func UpdateServerCapability(w http.ResponseWriter, r *http.Request) {
+ inf, userErr, sysErr, errCode := api.NewInfo(r, nil, nil)
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+ defer inf.Close()
+
+ tx := inf.Tx.Tx
+ sc, readValErr := readAndValidateJsonStruct(r)
+ if readValErr != nil {
+ api.HandleErr(w, r, tx, http.StatusBadRequest, readValErr, nil)
+ return
+ }
+
+ requestedName := inf.Params["name"]
+ // check if the entity was already updated
+ userErr, sysErr, errCode = api.CheckIfUnModifiedByName(r.Header, inf.Tx, requestedName, "server_capability")
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, tx, errCode, userErr, sysErr)
+ return
+ }
+
+ //update name and description of a capability
+ query := `UPDATE server_capability sc SET
+ name = $1,
+ description = $2
+ WHERE sc.name = $3
+ RETURNING sc.name, sc.description, sc.last_updated`
+
+ err := tx.QueryRow(query, sc.Name, sc.Description, requestedName).Scan(&sc.Name, &sc.Description, &sc.LastUpdated)
+ if err != nil {
+ if err == sql.ErrNoRows {
+ api.HandleErr(w, r, tx, http.StatusBadRequest, err, nil)
+ return
+ }
+ usrErr, sysErr, code := api.ParseDBError(err)
+ api.HandleErr(w, r, tx, code, usrErr, sysErr)
+ return
+ }
+ alerts := tc.CreateAlerts(tc.SuccessLevel, "server capability was updated")
+ api.WriteAlertsObj(w, r, http.StatusOK, alerts, sc)
+ return
+}
+
+func CreateServerCapability(w http.ResponseWriter, r *http.Request) {
+ inf, userErr, sysErr, errCode := api.NewInfo(r, nil, nil)
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+ defer inf.Close()
+ tx := inf.Tx.Tx
+
+ sc, readValErr := readAndValidateJsonStruct(r)
+ if readValErr != nil {
+ api.HandleErr(w, r, tx, http.StatusBadRequest, readValErr, nil)
+ return
+ }
+
+ // check if capability already exists
+ var count int
+ err := tx.QueryRow("SELECT count(*) from server_capability where name = $1", sc.Name).Scan(&count)
+ if err != nil {
+ api.HandleErr(w, r, tx, http.StatusInternalServerError, nil, fmt.Errorf("error: %w, when checking if server capability with name %s exists", err, sc.Name))
+ return
+ }
+ if count == 1 {
+ api.HandleErr(w, r, tx, http.StatusBadRequest, fmt.Errorf("server_capability name '%s' already exists.", sc.Name), nil)
+ return
+ }
+
+ // create server capability
+ query := `INSERT INTO server_capability (name, description) VALUES ($1, $2) RETURNING last_updated`
+ err = tx.QueryRow(query, sc.Name, sc.Description).Scan(&sc.LastUpdated)
+ if err != nil {
+ if err == sql.ErrNoRows {
+ api.HandleErr(w, r, tx, http.StatusBadRequest, err, nil)
+ return
+ }
+ usrErr, sysErr, code := api.ParseDBError(err)
+ api.HandleErr(w, r, tx, code, usrErr, sysErr)
+ return
+ }
+ alerts := tc.CreateAlerts(tc.SuccessLevel, "server capability was created.")
+ w.Header().Set("Location", fmt.Sprintf("/api/%d.%d/server_capabilities$?name=%s", inf.Version.Major, inf.Version.Minor, sc.Name))
Review Comment:
The `$` in here makes this incorrect; I suspect you copy/pasted from the handler's route definition?
--
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-unsubscribe@trafficcontrol.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] ocket8888 commented on a diff in pull request #7213: Added description field to server capabilities
Posted by GitBox <gi...@apache.org>.
ocket8888 commented on code in PR #7213:
URL: https://github.com/apache/trafficcontrol/pull/7213#discussion_r1047331960
##########
traffic_ops/traffic_ops_golang/servercapability/servercapability.go:
##########
@@ -151,6 +153,147 @@ func (v *TOServerCapability) Update(h http.Header) (error, error, int) {
return nil, nil, http.StatusOK
}
+func GetServerCapability(w http.ResponseWriter, r *http.Request) {
+ var sc tc.ServerCapabilityV4
+ inf, userErr, sysErr, errCode := api.NewInfo(r, nil, nil)
+ tx := inf.Tx
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+ defer inf.Close()
+
+ // Query Parameters to Database Query column mappings
+ queryParamsToQueryCols := map[string]dbhelpers.WhereColumnInfo{
+ "name": {Column: "sc.name", Checker: nil},
+ }
+ if _, ok := inf.Params["orderby"]; !ok {
+ inf.Params["orderby"] = "name"
+ }
+ where, orderBy, pagination, queryValues, errs := dbhelpers.BuildWhereAndOrderByAndPagination(inf.Params, queryParamsToQueryCols)
+ if len(errs) > 0 {
+ api.HandleErr(w, r, tx.Tx, http.StatusBadRequest, util.JoinErrs(errs), nil)
+ return
+ }
+
+ const selectQuery = "SELECT name, description, last_updated FROM server_capability sc"
+ query := selectQuery + where + orderBy + pagination
+ rows, err := tx.Query(query, queryValues)
+ if err != nil {
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, nil, fmt.Errorf("server capability read: error getting server capability(ies): %w", err))
+ }
+ defer log.Close(rows, "unable to close DB connection")
+
+ scList := []tc.ServerCapabilityV4{}
+ for rows.Next() {
+ if err = rows.Scan(&sc.Name, &sc.Description, &sc.LastUpdated); err != nil {
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, nil, fmt.Errorf("error getting server capability(ies): %w", err))
+ }
+ scList = append(scList, sc)
+ }
+
+ api.WriteResp(w, r, scList)
+ return
Review Comment:
According to codecov, this final `return` is not hit by the tests. That plus the invalid JSON error in the client tests leads me to believe that a scan error is occurring that hits line 190, probably multiple times which may or may not be causing a panic. It needs a `return` statement so that execution stops when an error is handled.
--
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-unsubscribe@trafficcontrol.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] ocket8888 commented on a diff in pull request #7213: Added description field to server capabilities
Posted by GitBox <gi...@apache.org>.
ocket8888 commented on code in PR #7213:
URL: https://github.com/apache/trafficcontrol/pull/7213#discussion_r1049951099
##########
traffic_ops/traffic_ops_golang/dbhelpers/db_helpers.go:
##########
@@ -2184,3 +2184,18 @@ func GetProfileIDDesc(tx *sql.Tx, name string) (id int, desc string) {
}
return
}
+
+// GetSCInfo confirms whether the server capability exists, and an error (if one occurs).
+func GetSCInfo(tx *sqlx.Tx, name string) (bool, error) {
+ var count int
+ if err := tx.QueryRow("SELECT count(name) FROM server_capability AS sc WHERE sc.name=$1", name).Scan(&count); err != nil {
+ return false, fmt.Errorf("getting server capability info: %v", err)
Review Comment:
formatting a source error should use `%w` to wrap it
##########
traffic_ops/traffic_ops_golang/servercapability/servercapability.go:
##########
@@ -151,6 +153,202 @@ func (v *TOServerCapability) Update(h http.Header) (error, error, int) {
return nil, nil, http.StatusOK
}
+func GetServerCapability(w http.ResponseWriter, r *http.Request) {
+ var sc tc.ServerCapabilityV4
+ inf, userErr, sysErr, errCode := api.NewInfo(r, nil, nil)
+ tx := inf.Tx
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+ defer inf.Close()
+
+ // Query Parameters to Database Query column mappings
+ queryParamsToQueryCols := map[string]dbhelpers.WhereColumnInfo{
+ "name": {Column: "sc.name", Checker: nil},
+ }
+ if _, ok := inf.Params["orderby"]; !ok {
+ inf.Params["orderby"] = "name"
+ }
+ where, orderBy, pagination, queryValues, errs := dbhelpers.BuildWhereAndOrderByAndPagination(inf.Params, queryParamsToQueryCols)
+ if len(errs) > 0 {
+ api.HandleErr(w, r, tx.Tx, http.StatusBadRequest, util.JoinErrs(errs), nil)
+ return
+ }
+
+ selectQuery := "SELECT name, description, last_updated FROM server_capability sc"
+ query := selectQuery + where + orderBy + pagination
+ rows, err := tx.NamedQuery(query, queryValues)
+ if err != nil {
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, nil, fmt.Errorf("server capability read: error getting server capability(ies): %w", err))
+ return
+ }
+ defer log.Close(rows, "unable to close DB connection")
+
+ scList := []tc.ServerCapabilityV4{}
+ for rows.Next() {
+ if err = rows.Scan(&sc.Name, &sc.Description, &sc.LastUpdated); err != nil {
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, nil, fmt.Errorf("error getting server capability(ies): %w", err))
+ return
+ }
+ scList = append(scList, sc)
+ }
+
+ api.WriteResp(w, r, scList)
+ return
+}
+
+func UpdateServerCapability(w http.ResponseWriter, r *http.Request) {
+ inf, userErr, sysErr, errCode := api.NewInfo(r, nil, nil)
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+ defer inf.Close()
+
+ tx := inf.Tx.Tx
+ sc, readValErr := readAndValidateJsonStruct(r)
+ if readValErr != nil {
+ api.HandleErr(w, r, tx, http.StatusBadRequest, readValErr, nil)
+ return
+ }
+
+ requestedName := inf.Params["name"]
+ // check if the entity was already updated
+ userErr, sysErr, errCode = api.CheckIfUnModifiedByName(r.Header, inf.Tx, requestedName, "server_capability")
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, tx, errCode, userErr, sysErr)
+ return
+ }
+
+ //update name and description of a capability
+ query := `UPDATE server_capability sc SET
+ name = $1,
+ description = $2
+ WHERE sc.name = $3
+ RETURNING sc.name, sc.description, sc.last_updated`
+
+ err := tx.QueryRow(query, sc.Name, sc.Description, requestedName).Scan(&sc.Name, &sc.Description, &sc.LastUpdated)
+ if err != nil && err == sql.ErrNoRows {
+ api.HandleErr(w, r, tx, http.StatusBadRequest, err, nil)
+ return
+ }
Review Comment:
This ignores all errors that aren't exactly `sql.ErrNoRows`. Ithink you should just check if the error is nil and use `api.ParseDBError` instead.
##########
traffic_ops/traffic_ops_golang/servercapability/servercapability.go:
##########
@@ -151,6 +153,202 @@ func (v *TOServerCapability) Update(h http.Header) (error, error, int) {
return nil, nil, http.StatusOK
}
+func GetServerCapability(w http.ResponseWriter, r *http.Request) {
+ var sc tc.ServerCapabilityV4
+ inf, userErr, sysErr, errCode := api.NewInfo(r, nil, nil)
+ tx := inf.Tx
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+ defer inf.Close()
+
+ // Query Parameters to Database Query column mappings
+ queryParamsToQueryCols := map[string]dbhelpers.WhereColumnInfo{
+ "name": {Column: "sc.name", Checker: nil},
+ }
+ if _, ok := inf.Params["orderby"]; !ok {
+ inf.Params["orderby"] = "name"
+ }
+ where, orderBy, pagination, queryValues, errs := dbhelpers.BuildWhereAndOrderByAndPagination(inf.Params, queryParamsToQueryCols)
+ if len(errs) > 0 {
+ api.HandleErr(w, r, tx.Tx, http.StatusBadRequest, util.JoinErrs(errs), nil)
+ return
+ }
+
+ selectQuery := "SELECT name, description, last_updated FROM server_capability sc"
+ query := selectQuery + where + orderBy + pagination
+ rows, err := tx.NamedQuery(query, queryValues)
+ if err != nil {
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, nil, fmt.Errorf("server capability read: error getting server capability(ies): %w", err))
+ return
+ }
+ defer log.Close(rows, "unable to close DB connection")
+
+ scList := []tc.ServerCapabilityV4{}
+ for rows.Next() {
+ if err = rows.Scan(&sc.Name, &sc.Description, &sc.LastUpdated); err != nil {
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, nil, fmt.Errorf("error getting server capability(ies): %w", err))
+ return
+ }
+ scList = append(scList, sc)
+ }
+
+ api.WriteResp(w, r, scList)
+ return
+}
+
+func UpdateServerCapability(w http.ResponseWriter, r *http.Request) {
+ inf, userErr, sysErr, errCode := api.NewInfo(r, nil, nil)
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+ defer inf.Close()
+
+ tx := inf.Tx.Tx
+ sc, readValErr := readAndValidateJsonStruct(r)
+ if readValErr != nil {
+ api.HandleErr(w, r, tx, http.StatusBadRequest, readValErr, nil)
+ return
+ }
+
+ requestedName := inf.Params["name"]
+ // check if the entity was already updated
+ userErr, sysErr, errCode = api.CheckIfUnModifiedByName(r.Header, inf.Tx, requestedName, "server_capability")
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, tx, errCode, userErr, sysErr)
+ return
+ }
+
+ //update name and description of a capability
+ query := `UPDATE server_capability sc SET
+ name = $1,
+ description = $2
+ WHERE sc.name = $3
+ RETURNING sc.name, sc.description, sc.last_updated`
+
+ err := tx.QueryRow(query, sc.Name, sc.Description, requestedName).Scan(&sc.Name, &sc.Description, &sc.LastUpdated)
+ if err != nil && err == sql.ErrNoRows {
+ api.HandleErr(w, r, tx, http.StatusBadRequest, err, nil)
+ return
+ }
+ alerts := tc.CreateAlerts(tc.SuccessLevel, "server capability was updated")
+ api.WriteAlertsObj(w, r, http.StatusOK, alerts, sc)
+ return
+}
+
+func CreateServerCapability(w http.ResponseWriter, r *http.Request) {
+ inf, userErr, sysErr, errCode := api.NewInfo(r, nil, nil)
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+ defer inf.Close()
+ tx := inf.Tx.Tx
+
+ sc, readValErr := readAndValidateJsonStruct(r)
+ if readValErr != nil {
+ api.HandleErr(w, r, tx, http.StatusBadRequest, readValErr, nil)
+ return
+ }
+
+ // check if capability already exists
+ var count int
+ err := tx.QueryRow("SELECT count(*) from server_capability where name = $1", sc.Name).Scan(&count)
+ if err != nil {
+ api.HandleErr(w, r, tx, http.StatusInternalServerError, nil, fmt.Errorf("checking if server capability with name %s exists", sc.Name))
Review Comment:
I think you meant to include the `err` in this `fmt.Errorf` call but forgot to
##########
traffic_ops/traffic_ops_golang/servercapability/servercapability.go:
##########
@@ -151,6 +153,202 @@ func (v *TOServerCapability) Update(h http.Header) (error, error, int) {
return nil, nil, http.StatusOK
}
+func GetServerCapability(w http.ResponseWriter, r *http.Request) {
+ var sc tc.ServerCapabilityV4
+ inf, userErr, sysErr, errCode := api.NewInfo(r, nil, nil)
+ tx := inf.Tx
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+ defer inf.Close()
+
+ // Query Parameters to Database Query column mappings
+ queryParamsToQueryCols := map[string]dbhelpers.WhereColumnInfo{
+ "name": {Column: "sc.name", Checker: nil},
+ }
+ if _, ok := inf.Params["orderby"]; !ok {
+ inf.Params["orderby"] = "name"
+ }
+ where, orderBy, pagination, queryValues, errs := dbhelpers.BuildWhereAndOrderByAndPagination(inf.Params, queryParamsToQueryCols)
+ if len(errs) > 0 {
+ api.HandleErr(w, r, tx.Tx, http.StatusBadRequest, util.JoinErrs(errs), nil)
+ return
+ }
+
+ selectQuery := "SELECT name, description, last_updated FROM server_capability sc"
+ query := selectQuery + where + orderBy + pagination
+ rows, err := tx.NamedQuery(query, queryValues)
+ if err != nil {
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, nil, fmt.Errorf("server capability read: error getting server capability(ies): %w", err))
+ return
+ }
+ defer log.Close(rows, "unable to close DB connection")
+
+ scList := []tc.ServerCapabilityV4{}
+ for rows.Next() {
+ if err = rows.Scan(&sc.Name, &sc.Description, &sc.LastUpdated); err != nil {
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, nil, fmt.Errorf("error getting server capability(ies): %w", err))
+ return
+ }
+ scList = append(scList, sc)
+ }
+
+ api.WriteResp(w, r, scList)
+ return
+}
+
+func UpdateServerCapability(w http.ResponseWriter, r *http.Request) {
+ inf, userErr, sysErr, errCode := api.NewInfo(r, nil, nil)
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+ defer inf.Close()
+
+ tx := inf.Tx.Tx
+ sc, readValErr := readAndValidateJsonStruct(r)
+ if readValErr != nil {
+ api.HandleErr(w, r, tx, http.StatusBadRequest, readValErr, nil)
+ return
+ }
+
+ requestedName := inf.Params["name"]
+ // check if the entity was already updated
+ userErr, sysErr, errCode = api.CheckIfUnModifiedByName(r.Header, inf.Tx, requestedName, "server_capability")
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, tx, errCode, userErr, sysErr)
+ return
+ }
+
+ //update name and description of a capability
+ query := `UPDATE server_capability sc SET
+ name = $1,
+ description = $2
+ WHERE sc.name = $3
+ RETURNING sc.name, sc.description, sc.last_updated`
+
+ err := tx.QueryRow(query, sc.Name, sc.Description, requestedName).Scan(&sc.Name, &sc.Description, &sc.LastUpdated)
+ if err != nil && err == sql.ErrNoRows {
+ api.HandleErr(w, r, tx, http.StatusBadRequest, err, nil)
+ return
+ }
+ alerts := tc.CreateAlerts(tc.SuccessLevel, "server capability was updated")
+ api.WriteAlertsObj(w, r, http.StatusOK, alerts, sc)
+ return
+}
+
+func CreateServerCapability(w http.ResponseWriter, r *http.Request) {
+ inf, userErr, sysErr, errCode := api.NewInfo(r, nil, nil)
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+ defer inf.Close()
+ tx := inf.Tx.Tx
+
+ sc, readValErr := readAndValidateJsonStruct(r)
+ if readValErr != nil {
+ api.HandleErr(w, r, tx, http.StatusBadRequest, readValErr, nil)
+ return
+ }
+
+ // check if capability already exists
+ var count int
+ err := tx.QueryRow("SELECT count(*) from server_capability where name = $1", sc.Name).Scan(&count)
+ if err != nil {
+ api.HandleErr(w, r, tx, http.StatusInternalServerError, nil, fmt.Errorf("checking if server capability with name %s exists", sc.Name))
+ return
+ }
+ if count == 1 {
+ api.HandleErr(w, r, tx, http.StatusBadRequest, fmt.Errorf("server_capability name '%s' already exists.", sc.Name), nil)
+ return
+ }
+
+ // create server capability
+ query := `INSERT INTO server_capability (name, description) VALUES ($1, $2) RETURNING last_updated`
+ err = tx.QueryRow(query, sc.Name, sc.Description).Scan(&sc.LastUpdated)
+ if err != nil && err == sql.ErrNoRows {
+ api.HandleErr(w, r, tx, http.StatusInternalServerError, nil, err)
+ return
+ }
+ alerts := tc.CreateAlerts(tc.SuccessLevel, "server capability was created.")
+ api.WriteAlertsObj(w, r, http.StatusOK, alerts, sc)
Review Comment:
The response for a successful creation should be `201 Created` with a `Location` header set to a URL (absolute or relative but relative is way simpler) to which a GET request can be sent to retrieve a representation of the created object.
##########
traffic_ops/traffic_ops_golang/servercapability/servercapability.go:
##########
@@ -151,6 +153,202 @@ func (v *TOServerCapability) Update(h http.Header) (error, error, int) {
return nil, nil, http.StatusOK
}
+func GetServerCapability(w http.ResponseWriter, r *http.Request) {
+ var sc tc.ServerCapabilityV4
+ inf, userErr, sysErr, errCode := api.NewInfo(r, nil, nil)
+ tx := inf.Tx
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+ defer inf.Close()
+
+ // Query Parameters to Database Query column mappings
+ queryParamsToQueryCols := map[string]dbhelpers.WhereColumnInfo{
+ "name": {Column: "sc.name", Checker: nil},
+ }
+ if _, ok := inf.Params["orderby"]; !ok {
+ inf.Params["orderby"] = "name"
+ }
+ where, orderBy, pagination, queryValues, errs := dbhelpers.BuildWhereAndOrderByAndPagination(inf.Params, queryParamsToQueryCols)
+ if len(errs) > 0 {
+ api.HandleErr(w, r, tx.Tx, http.StatusBadRequest, util.JoinErrs(errs), nil)
+ return
+ }
+
+ selectQuery := "SELECT name, description, last_updated FROM server_capability sc"
+ query := selectQuery + where + orderBy + pagination
+ rows, err := tx.NamedQuery(query, queryValues)
+ if err != nil {
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, nil, fmt.Errorf("server capability read: error getting server capability(ies): %w", err))
+ return
+ }
+ defer log.Close(rows, "unable to close DB connection")
+
+ scList := []tc.ServerCapabilityV4{}
+ for rows.Next() {
+ if err = rows.Scan(&sc.Name, &sc.Description, &sc.LastUpdated); err != nil {
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, nil, fmt.Errorf("error getting server capability(ies): %w", err))
+ return
+ }
+ scList = append(scList, sc)
+ }
+
+ api.WriteResp(w, r, scList)
+ return
+}
+
+func UpdateServerCapability(w http.ResponseWriter, r *http.Request) {
+ inf, userErr, sysErr, errCode := api.NewInfo(r, nil, nil)
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+ defer inf.Close()
+
+ tx := inf.Tx.Tx
+ sc, readValErr := readAndValidateJsonStruct(r)
+ if readValErr != nil {
+ api.HandleErr(w, r, tx, http.StatusBadRequest, readValErr, nil)
+ return
+ }
+
+ requestedName := inf.Params["name"]
+ // check if the entity was already updated
+ userErr, sysErr, errCode = api.CheckIfUnModifiedByName(r.Header, inf.Tx, requestedName, "server_capability")
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, tx, errCode, userErr, sysErr)
+ return
+ }
+
+ //update name and description of a capability
+ query := `UPDATE server_capability sc SET
+ name = $1,
+ description = $2
+ WHERE sc.name = $3
+ RETURNING sc.name, sc.description, sc.last_updated`
+
+ err := tx.QueryRow(query, sc.Name, sc.Description, requestedName).Scan(&sc.Name, &sc.Description, &sc.LastUpdated)
+ if err != nil && err == sql.ErrNoRows {
+ api.HandleErr(w, r, tx, http.StatusBadRequest, err, nil)
+ return
+ }
+ alerts := tc.CreateAlerts(tc.SuccessLevel, "server capability was updated")
+ api.WriteAlertsObj(w, r, http.StatusOK, alerts, sc)
+ return
+}
+
+func CreateServerCapability(w http.ResponseWriter, r *http.Request) {
+ inf, userErr, sysErr, errCode := api.NewInfo(r, nil, nil)
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+ defer inf.Close()
+ tx := inf.Tx.Tx
+
+ sc, readValErr := readAndValidateJsonStruct(r)
+ if readValErr != nil {
+ api.HandleErr(w, r, tx, http.StatusBadRequest, readValErr, nil)
+ return
+ }
+
+ // check if capability already exists
+ var count int
+ err := tx.QueryRow("SELECT count(*) from server_capability where name = $1", sc.Name).Scan(&count)
+ if err != nil {
+ api.HandleErr(w, r, tx, http.StatusInternalServerError, nil, fmt.Errorf("checking if server capability with name %s exists", sc.Name))
+ return
+ }
+ if count == 1 {
+ api.HandleErr(w, r, tx, http.StatusBadRequest, fmt.Errorf("server_capability name '%s' already exists.", sc.Name), nil)
+ return
+ }
+
+ // create server capability
+ query := `INSERT INTO server_capability (name, description) VALUES ($1, $2) RETURNING last_updated`
+ err = tx.QueryRow(query, sc.Name, sc.Description).Scan(&sc.LastUpdated)
+ if err != nil && err == sql.ErrNoRows {
+ api.HandleErr(w, r, tx, http.StatusInternalServerError, nil, err)
+ return
+ }
+ alerts := tc.CreateAlerts(tc.SuccessLevel, "server capability was created.")
+ api.WriteAlertsObj(w, r, http.StatusOK, alerts, sc)
+ return
+}
+
+func DeleteServerCapability(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)
+ return
+ }
+ defer inf.Close()
+
+ name := inf.Params["name"]
+ exists, err := dbhelpers.GetSCInfo(inf.Tx, name)
+ if err != nil {
+ api.HandleErr(w, r, tx, http.StatusInternalServerError, nil, err)
+ return
+ }
+ if !exists {
+ if name != "" {
+ api.HandleErr(w, r, tx, http.StatusNotFound, fmt.Errorf("no server capability exists by name: %s", name), nil)
+ return
+ } else {
+ api.HandleErr(w, r, tx, http.StatusBadRequest, fmt.Errorf("no server capability exists for empty name: %s", name), nil)
+ return
+ }
+ }
+
+ assignedServer := 0
+ if err := inf.Tx.Get(&assignedServer, "SELECT count(server) FROM server_server_capability ssc WHERE ssc.server_capability=$1", name); err != nil {
+ api.HandleErr(w, r, tx, http.StatusInternalServerError, nil, fmt.Errorf("server capability delete, counting assigned servers: %w", err))
+ return
+ } else if assignedServer != 0 {
+ api.HandleErr(w, r, tx, http.StatusBadRequest, fmt.Errorf("can not delete a server capability with %d assigned servers", assignedServer), nil)
+ return
+ }
+
+ res, err := tx.Exec("DELETE FROM server_capability AS sc WHERE sc.name=$1", name)
+ if err != nil {
+ api.HandleErr(w, r, tx, http.StatusInternalServerError, nil, err)
+ return
+ }
+ rowsAffected, err := res.RowsAffected()
+ if err != nil {
+ api.HandleErr(w, r, tx, http.StatusInternalServerError, nil, fmt.Errorf("determining rows affected for delete server capability: %w", err))
+ return
+ }
+ if rowsAffected == 0 {
+ api.HandleErr(w, r, tx, http.StatusBadRequest, fmt.Errorf("no rows deleted for server capability"), nil)
+ return
+ }
+
+ api.WriteResp(w, r, "Successfully deleted server capability "+name)
Review Comment:
This seems like it should be a success-level alert instead of a `response`
##########
traffic_ops/traffic_ops_golang/servercapability/servercapability.go:
##########
@@ -151,6 +153,202 @@ func (v *TOServerCapability) Update(h http.Header) (error, error, int) {
return nil, nil, http.StatusOK
}
+func GetServerCapability(w http.ResponseWriter, r *http.Request) {
+ var sc tc.ServerCapabilityV4
+ inf, userErr, sysErr, errCode := api.NewInfo(r, nil, nil)
+ tx := inf.Tx
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+ defer inf.Close()
+
+ // Query Parameters to Database Query column mappings
+ queryParamsToQueryCols := map[string]dbhelpers.WhereColumnInfo{
+ "name": {Column: "sc.name", Checker: nil},
+ }
+ if _, ok := inf.Params["orderby"]; !ok {
+ inf.Params["orderby"] = "name"
+ }
+ where, orderBy, pagination, queryValues, errs := dbhelpers.BuildWhereAndOrderByAndPagination(inf.Params, queryParamsToQueryCols)
+ if len(errs) > 0 {
+ api.HandleErr(w, r, tx.Tx, http.StatusBadRequest, util.JoinErrs(errs), nil)
+ return
+ }
+
+ selectQuery := "SELECT name, description, last_updated FROM server_capability sc"
+ query := selectQuery + where + orderBy + pagination
+ rows, err := tx.NamedQuery(query, queryValues)
+ if err != nil {
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, nil, fmt.Errorf("server capability read: error getting server capability(ies): %w", err))
+ return
+ }
+ defer log.Close(rows, "unable to close DB connection")
+
+ scList := []tc.ServerCapabilityV4{}
+ for rows.Next() {
+ if err = rows.Scan(&sc.Name, &sc.Description, &sc.LastUpdated); err != nil {
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, nil, fmt.Errorf("error getting server capability(ies): %w", err))
+ return
+ }
+ scList = append(scList, sc)
+ }
+
+ api.WriteResp(w, r, scList)
+ return
+}
+
+func UpdateServerCapability(w http.ResponseWriter, r *http.Request) {
+ inf, userErr, sysErr, errCode := api.NewInfo(r, nil, nil)
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+ defer inf.Close()
+
+ tx := inf.Tx.Tx
+ sc, readValErr := readAndValidateJsonStruct(r)
+ if readValErr != nil {
+ api.HandleErr(w, r, tx, http.StatusBadRequest, readValErr, nil)
+ return
+ }
+
+ requestedName := inf.Params["name"]
+ // check if the entity was already updated
+ userErr, sysErr, errCode = api.CheckIfUnModifiedByName(r.Header, inf.Tx, requestedName, "server_capability")
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, tx, errCode, userErr, sysErr)
+ return
+ }
+
+ //update name and description of a capability
+ query := `UPDATE server_capability sc SET
+ name = $1,
+ description = $2
+ WHERE sc.name = $3
+ RETURNING sc.name, sc.description, sc.last_updated`
+
+ err := tx.QueryRow(query, sc.Name, sc.Description, requestedName).Scan(&sc.Name, &sc.Description, &sc.LastUpdated)
+ if err != nil && err == sql.ErrNoRows {
+ api.HandleErr(w, r, tx, http.StatusBadRequest, err, nil)
+ return
+ }
+ alerts := tc.CreateAlerts(tc.SuccessLevel, "server capability was updated")
+ api.WriteAlertsObj(w, r, http.StatusOK, alerts, sc)
+ return
+}
+
+func CreateServerCapability(w http.ResponseWriter, r *http.Request) {
+ inf, userErr, sysErr, errCode := api.NewInfo(r, nil, nil)
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+ defer inf.Close()
+ tx := inf.Tx.Tx
+
+ sc, readValErr := readAndValidateJsonStruct(r)
+ if readValErr != nil {
+ api.HandleErr(w, r, tx, http.StatusBadRequest, readValErr, nil)
+ return
+ }
+
+ // check if capability already exists
+ var count int
+ err := tx.QueryRow("SELECT count(*) from server_capability where name = $1", sc.Name).Scan(&count)
+ if err != nil {
+ api.HandleErr(w, r, tx, http.StatusInternalServerError, nil, fmt.Errorf("checking if server capability with name %s exists", sc.Name))
+ return
+ }
+ if count == 1 {
+ api.HandleErr(w, r, tx, http.StatusBadRequest, fmt.Errorf("server_capability name '%s' already exists.", sc.Name), nil)
+ return
+ }
+
+ // create server capability
+ query := `INSERT INTO server_capability (name, description) VALUES ($1, $2) RETURNING last_updated`
+ err = tx.QueryRow(query, sc.Name, sc.Description).Scan(&sc.LastUpdated)
+ if err != nil && err == sql.ErrNoRows {
+ api.HandleErr(w, r, tx, http.StatusInternalServerError, nil, err)
+ return
+ }
Review Comment:
same as above RE: ignoring database errors/using `api.ParseDBError`
--
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-unsubscribe@trafficcontrol.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] rimashah25 commented on a diff in pull request #7213: Added description field to server capabilities
Posted by GitBox <gi...@apache.org>.
rimashah25 commented on code in PR #7213:
URL: https://github.com/apache/trafficcontrol/pull/7213#discussion_r1051021229
##########
traffic_ops/traffic_ops_golang/servercapability/servercapability.go:
##########
@@ -151,6 +153,213 @@ func (v *TOServerCapability) Update(h http.Header) (error, error, int) {
return nil, nil, http.StatusOK
}
+func GetServerCapability(w http.ResponseWriter, r *http.Request) {
+ var sc tc.ServerCapabilityV4
+ inf, userErr, sysErr, errCode := api.NewInfo(r, nil, nil)
+ tx := inf.Tx
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+ defer inf.Close()
+
+ // Query Parameters to Database Query column mappings
+ queryParamsToQueryCols := map[string]dbhelpers.WhereColumnInfo{
+ "name": {Column: "sc.name", Checker: nil},
+ }
+ if _, ok := inf.Params["orderby"]; !ok {
+ inf.Params["orderby"] = "name"
+ }
+ where, orderBy, pagination, queryValues, errs := dbhelpers.BuildWhereAndOrderByAndPagination(inf.Params, queryParamsToQueryCols)
+ if len(errs) > 0 {
+ api.HandleErr(w, r, tx.Tx, http.StatusBadRequest, util.JoinErrs(errs), nil)
+ return
+ }
+
+ selectQuery := "SELECT name, description, last_updated FROM server_capability sc"
+ query := selectQuery + where + orderBy + pagination
+ rows, err := tx.NamedQuery(query, queryValues)
+ if err != nil {
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, nil, fmt.Errorf("server capability read: error getting server capability(ies): %w", err))
+ return
+ }
+ defer log.Close(rows, "unable to close DB connection")
+
+ scList := []tc.ServerCapabilityV4{}
+ for rows.Next() {
+ if err = rows.Scan(&sc.Name, &sc.Description, &sc.LastUpdated); err != nil {
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, nil, fmt.Errorf("error getting server capability(ies): %w", err))
+ return
+ }
+ scList = append(scList, sc)
+ }
+
+ api.WriteResp(w, r, scList)
+ return
+}
+
+func UpdateServerCapability(w http.ResponseWriter, r *http.Request) {
+ inf, userErr, sysErr, errCode := api.NewInfo(r, nil, nil)
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+ defer inf.Close()
+
+ tx := inf.Tx.Tx
+ sc, readValErr := readAndValidateJsonStruct(r)
+ if readValErr != nil {
+ api.HandleErr(w, r, tx, http.StatusBadRequest, readValErr, nil)
+ return
+ }
+
+ requestedName := inf.Params["name"]
+ // check if the entity was already updated
+ userErr, sysErr, errCode = api.CheckIfUnModifiedByName(r.Header, inf.Tx, requestedName, "server_capability")
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, tx, errCode, userErr, sysErr)
+ return
+ }
+
+ //update name and description of a capability
+ query := `UPDATE server_capability sc SET
+ name = $1,
+ description = $2
+ WHERE sc.name = $3
+ RETURNING sc.name, sc.description, sc.last_updated`
+
+ err := tx.QueryRow(query, sc.Name, sc.Description, requestedName).Scan(&sc.Name, &sc.Description, &sc.LastUpdated)
+ if err != nil {
+ if err == sql.ErrNoRows {
+ api.HandleErr(w, r, tx, http.StatusBadRequest, err, nil)
+ return
+ }
Review Comment:
I need this because integration tests are failing for this case: `BAD REQUEST when NAME DOESNT EXIST`
--
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-unsubscribe@trafficcontrol.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] rimashah25 commented on a diff in pull request #7213: Added description field to server capabilities
Posted by GitBox <gi...@apache.org>.
rimashah25 commented on code in PR #7213:
URL: https://github.com/apache/trafficcontrol/pull/7213#discussion_r1051122255
##########
traffic_ops/traffic_ops_golang/servercapability/servercapability.go:
##########
@@ -151,6 +153,213 @@ func (v *TOServerCapability) Update(h http.Header) (error, error, int) {
return nil, nil, http.StatusOK
}
+func GetServerCapability(w http.ResponseWriter, r *http.Request) {
+ var sc tc.ServerCapabilityV4
+ inf, userErr, sysErr, errCode := api.NewInfo(r, nil, nil)
+ tx := inf.Tx
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+ defer inf.Close()
+
+ // Query Parameters to Database Query column mappings
+ queryParamsToQueryCols := map[string]dbhelpers.WhereColumnInfo{
+ "name": {Column: "sc.name", Checker: nil},
+ }
+ if _, ok := inf.Params["orderby"]; !ok {
+ inf.Params["orderby"] = "name"
+ }
+ where, orderBy, pagination, queryValues, errs := dbhelpers.BuildWhereAndOrderByAndPagination(inf.Params, queryParamsToQueryCols)
+ if len(errs) > 0 {
+ api.HandleErr(w, r, tx.Tx, http.StatusBadRequest, util.JoinErrs(errs), nil)
+ return
+ }
+
+ selectQuery := "SELECT name, description, last_updated FROM server_capability sc"
+ query := selectQuery + where + orderBy + pagination
+ rows, err := tx.NamedQuery(query, queryValues)
+ if err != nil {
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, nil, fmt.Errorf("server capability read: error getting server capability(ies): %w", err))
+ return
+ }
+ defer log.Close(rows, "unable to close DB connection")
+
+ scList := []tc.ServerCapabilityV4{}
+ for rows.Next() {
+ if err = rows.Scan(&sc.Name, &sc.Description, &sc.LastUpdated); err != nil {
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, nil, fmt.Errorf("error getting server capability(ies): %w", err))
+ return
+ }
+ scList = append(scList, sc)
+ }
+
+ api.WriteResp(w, r, scList)
+ return
+}
+
+func UpdateServerCapability(w http.ResponseWriter, r *http.Request) {
+ inf, userErr, sysErr, errCode := api.NewInfo(r, nil, nil)
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+ defer inf.Close()
+
+ tx := inf.Tx.Tx
+ sc, readValErr := readAndValidateJsonStruct(r)
+ if readValErr != nil {
+ api.HandleErr(w, r, tx, http.StatusBadRequest, readValErr, nil)
+ return
+ }
+
+ requestedName := inf.Params["name"]
+ // check if the entity was already updated
+ userErr, sysErr, errCode = api.CheckIfUnModifiedByName(r.Header, inf.Tx, requestedName, "server_capability")
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, tx, errCode, userErr, sysErr)
+ return
+ }
+
+ //update name and description of a capability
+ query := `UPDATE server_capability sc SET
+ name = $1,
+ description = $2
+ WHERE sc.name = $3
+ RETURNING sc.name, sc.description, sc.last_updated`
+
+ err := tx.QueryRow(query, sc.Name, sc.Description, requestedName).Scan(&sc.Name, &sc.Description, &sc.LastUpdated)
+ if err != nil {
+ if err == sql.ErrNoRows {
+ api.HandleErr(w, r, tx, http.StatusBadRequest, err, nil)
+ return
+ }
+ usrErr, sysErr, code := api.ParseDBError(err)
+ api.HandleErr(w, r, tx, code, usrErr, sysErr)
+ return
+ }
+ alerts := tc.CreateAlerts(tc.SuccessLevel, "server capability was updated")
+ api.WriteAlertsObj(w, r, http.StatusOK, alerts, sc)
+ return
+}
+
+func CreateServerCapability(w http.ResponseWriter, r *http.Request) {
+ inf, userErr, sysErr, errCode := api.NewInfo(r, nil, nil)
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+ defer inf.Close()
+ tx := inf.Tx.Tx
+
+ sc, readValErr := readAndValidateJsonStruct(r)
+ if readValErr != nil {
+ api.HandleErr(w, r, tx, http.StatusBadRequest, readValErr, nil)
+ return
+ }
+
+ // check if capability already exists
+ var count int
+ err := tx.QueryRow("SELECT count(*) from server_capability where name = $1", sc.Name).Scan(&count)
+ if err != nil {
+ api.HandleErr(w, r, tx, http.StatusInternalServerError, nil, fmt.Errorf("error: %w, when checking if server capability with name %s exists", err, sc.Name))
+ return
+ }
+ if count == 1 {
+ api.HandleErr(w, r, tx, http.StatusBadRequest, fmt.Errorf("server_capability name '%s' already exists.", sc.Name), nil)
Review Comment:
I did... apparently `validaionMessage` in TP tests was expecting it.. and now it is failing.
--
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-unsubscribe@trafficcontrol.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] rimashah25 commented on a diff in pull request #7213: Added description field to server capabilities
Posted by GitBox <gi...@apache.org>.
rimashah25 commented on code in PR #7213:
URL: https://github.com/apache/trafficcontrol/pull/7213#discussion_r1039992971
##########
traffic_ops/testing/api/v3/tc-fixtures.json:
##########
@@ -4822,22 +4822,28 @@
],
"serverCapabilities": [
{
- "name": "foo"
+ "name": "foo",
+ "description": "server capability foo"
Review Comment:
i realized since this field is part of 4.1, v3 doesn't need it.
--
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-unsubscribe@trafficcontrol.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] ocket8888 commented on a diff in pull request #7213: Added description field to server capabilities
Posted by GitBox <gi...@apache.org>.
ocket8888 commented on code in PR #7213:
URL: https://github.com/apache/trafficcontrol/pull/7213#discussion_r1043785590
##########
traffic_ops/v4-client/servercapability.go:
##########
@@ -27,26 +27,26 @@ import (
const apiServerCapabilities = "/server_capabilities"
// CreateServerCapability creates the given Server Capability.
-func (to *Session) CreateServerCapability(sc tc.ServerCapability, opts RequestOptions) (tc.ServerCapabilityDetailResponse, toclientlib.ReqInf, error) {
- var scResp tc.ServerCapabilityDetailResponse
+func (to *Session) CreateServerCapability(sc tc.ServerCapabilityV41, opts RequestOptions) (tc.ServerCapabilityDetailResponseV41, toclientlib.ReqInf, error) {
+ var scResp tc.ServerCapabilityDetailResponseV41
Review Comment:
So unfortunately, we can't do this. This is a breaking call signature change, because if I have
```go
var resp tc.ServerCapabilityDetailResponse
resp, _, _ = to.CreateServerCapability(tc.ServerCapability{Name: "anything"})
```
My code no longer compiles for this API version. You have to add a separate client method with the updated struct, deprecate the old one - and be sure to mention that the call signature will change in v5 rather than the new method name be used, because what you have in the v5 client is just fine.
--
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-unsubscribe@trafficcontrol.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] github-code-scanning[bot] commented on a diff in pull request #7213: Added description field to server capabilities
Posted by GitBox <gi...@apache.org>.
github-code-scanning[bot] commented on code in PR #7213:
URL: https://github.com/apache/trafficcontrol/pull/7213#discussion_r1042617899
##########
traffic_ops/traffic_ops_golang/servercapability/servercapability.go:
##########
@@ -151,6 +153,125 @@
return nil, nil, http.StatusOK
}
+func GetServerCapability(w http.ResponseWriter, r *http.Request) {
+ var sc tc.ServerCapabilityV41
+ inf, userErr, sysErr, errCode := api.NewInfo(r, nil, nil)
+ tx := inf.Tx
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+ defer inf.Close()
+
+ // Query Parameters to Database Query column mappings
+ queryParamsToQueryCols := map[string]dbhelpers.WhereColumnInfo{
+ "name": dbhelpers.WhereColumnInfo{Column: "sc.name"},
+ }
+ where, orderBy, pagination, queryValues, errs := dbhelpers.BuildWhereAndOrderByAndPagination(inf.Params, queryParamsToQueryCols)
+ if len(errs) > 0 {
+ api.HandleErr(w, r, tx.Tx, http.StatusBadRequest, util.JoinErrs(errs), nil)
+ return
+ }
+
+ query := "SELECT name, description, last_updated FROM server_capability sc"
+ query += where + orderBy + pagination
+ rows, err := tx.NamedQuery(query, queryValues)
Review Comment:
## Database query built from user-controlled sources
This query depends on a [user-provided value](1).
[Show more details](https://github.com/apache/trafficcontrol/security/code-scanning/249)
--
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-unsubscribe@trafficcontrol.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] rimashah25 commented on a diff in pull request #7213: Added description field to server capabilities
Posted by GitBox <gi...@apache.org>.
rimashah25 commented on code in PR #7213:
URL: https://github.com/apache/trafficcontrol/pull/7213#discussion_r1050008729
##########
traffic_ops/traffic_ops_golang/servercapability/servercapability.go:
##########
@@ -151,6 +153,202 @@ func (v *TOServerCapability) Update(h http.Header) (error, error, int) {
return nil, nil, http.StatusOK
}
+func GetServerCapability(w http.ResponseWriter, r *http.Request) {
+ var sc tc.ServerCapabilityV4
+ inf, userErr, sysErr, errCode := api.NewInfo(r, nil, nil)
+ tx := inf.Tx
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+ defer inf.Close()
+
+ // Query Parameters to Database Query column mappings
+ queryParamsToQueryCols := map[string]dbhelpers.WhereColumnInfo{
+ "name": {Column: "sc.name", Checker: nil},
+ }
+ if _, ok := inf.Params["orderby"]; !ok {
+ inf.Params["orderby"] = "name"
+ }
+ where, orderBy, pagination, queryValues, errs := dbhelpers.BuildWhereAndOrderByAndPagination(inf.Params, queryParamsToQueryCols)
+ if len(errs) > 0 {
+ api.HandleErr(w, r, tx.Tx, http.StatusBadRequest, util.JoinErrs(errs), nil)
+ return
+ }
+
+ selectQuery := "SELECT name, description, last_updated FROM server_capability sc"
+ query := selectQuery + where + orderBy + pagination
+ rows, err := tx.NamedQuery(query, queryValues)
+ if err != nil {
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, nil, fmt.Errorf("server capability read: error getting server capability(ies): %w", err))
+ return
+ }
+ defer log.Close(rows, "unable to close DB connection")
+
+ scList := []tc.ServerCapabilityV4{}
+ for rows.Next() {
+ if err = rows.Scan(&sc.Name, &sc.Description, &sc.LastUpdated); err != nil {
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, nil, fmt.Errorf("error getting server capability(ies): %w", err))
+ return
+ }
+ scList = append(scList, sc)
+ }
+
+ api.WriteResp(w, r, scList)
+ return
+}
+
+func UpdateServerCapability(w http.ResponseWriter, r *http.Request) {
+ inf, userErr, sysErr, errCode := api.NewInfo(r, nil, nil)
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+ defer inf.Close()
+
+ tx := inf.Tx.Tx
+ sc, readValErr := readAndValidateJsonStruct(r)
+ if readValErr != nil {
+ api.HandleErr(w, r, tx, http.StatusBadRequest, readValErr, nil)
+ return
+ }
+
+ requestedName := inf.Params["name"]
+ // check if the entity was already updated
+ userErr, sysErr, errCode = api.CheckIfUnModifiedByName(r.Header, inf.Tx, requestedName, "server_capability")
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, tx, errCode, userErr, sysErr)
+ return
+ }
+
+ //update name and description of a capability
+ query := `UPDATE server_capability sc SET
+ name = $1,
+ description = $2
+ WHERE sc.name = $3
+ RETURNING sc.name, sc.description, sc.last_updated`
+
+ err := tx.QueryRow(query, sc.Name, sc.Description, requestedName).Scan(&sc.Name, &sc.Description, &sc.LastUpdated)
+ if err != nil && err == sql.ErrNoRows {
+ api.HandleErr(w, r, tx, http.StatusBadRequest, err, nil)
+ return
+ }
+ alerts := tc.CreateAlerts(tc.SuccessLevel, "server capability was updated")
+ api.WriteAlertsObj(w, r, http.StatusOK, alerts, sc)
+ return
+}
+
+func CreateServerCapability(w http.ResponseWriter, r *http.Request) {
+ inf, userErr, sysErr, errCode := api.NewInfo(r, nil, nil)
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+ defer inf.Close()
+ tx := inf.Tx.Tx
+
+ sc, readValErr := readAndValidateJsonStruct(r)
+ if readValErr != nil {
+ api.HandleErr(w, r, tx, http.StatusBadRequest, readValErr, nil)
+ return
+ }
+
+ // check if capability already exists
+ var count int
+ err := tx.QueryRow("SELECT count(*) from server_capability where name = $1", sc.Name).Scan(&count)
+ if err != nil {
+ api.HandleErr(w, r, tx, http.StatusInternalServerError, nil, fmt.Errorf("checking if server capability with name %s exists", sc.Name))
Review Comment:
yes. :)
--
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-unsubscribe@trafficcontrol.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] rimashah25 commented on a diff in pull request #7213: Added description field to server capabilities
Posted by GitBox <gi...@apache.org>.
rimashah25 commented on code in PR #7213:
URL: https://github.com/apache/trafficcontrol/pull/7213#discussion_r1051029701
##########
traffic_ops/traffic_ops_golang/servercapability/servercapability.go:
##########
@@ -151,6 +153,213 @@ func (v *TOServerCapability) Update(h http.Header) (error, error, int) {
return nil, nil, http.StatusOK
}
+func GetServerCapability(w http.ResponseWriter, r *http.Request) {
+ var sc tc.ServerCapabilityV4
+ inf, userErr, sysErr, errCode := api.NewInfo(r, nil, nil)
+ tx := inf.Tx
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+ defer inf.Close()
+
+ // Query Parameters to Database Query column mappings
+ queryParamsToQueryCols := map[string]dbhelpers.WhereColumnInfo{
+ "name": {Column: "sc.name", Checker: nil},
+ }
+ if _, ok := inf.Params["orderby"]; !ok {
+ inf.Params["orderby"] = "name"
+ }
+ where, orderBy, pagination, queryValues, errs := dbhelpers.BuildWhereAndOrderByAndPagination(inf.Params, queryParamsToQueryCols)
+ if len(errs) > 0 {
+ api.HandleErr(w, r, tx.Tx, http.StatusBadRequest, util.JoinErrs(errs), nil)
+ return
+ }
+
+ selectQuery := "SELECT name, description, last_updated FROM server_capability sc"
+ query := selectQuery + where + orderBy + pagination
+ rows, err := tx.NamedQuery(query, queryValues)
+ if err != nil {
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, nil, fmt.Errorf("server capability read: error getting server capability(ies): %w", err))
+ return
+ }
+ defer log.Close(rows, "unable to close DB connection")
+
+ scList := []tc.ServerCapabilityV4{}
+ for rows.Next() {
+ if err = rows.Scan(&sc.Name, &sc.Description, &sc.LastUpdated); err != nil {
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, nil, fmt.Errorf("error getting server capability(ies): %w", err))
+ return
+ }
+ scList = append(scList, sc)
+ }
+
+ api.WriteResp(w, r, scList)
+ return
+}
+
+func UpdateServerCapability(w http.ResponseWriter, r *http.Request) {
+ inf, userErr, sysErr, errCode := api.NewInfo(r, nil, nil)
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+ defer inf.Close()
+
+ tx := inf.Tx.Tx
+ sc, readValErr := readAndValidateJsonStruct(r)
+ if readValErr != nil {
+ api.HandleErr(w, r, tx, http.StatusBadRequest, readValErr, nil)
+ return
+ }
+
+ requestedName := inf.Params["name"]
+ // check if the entity was already updated
+ userErr, sysErr, errCode = api.CheckIfUnModifiedByName(r.Header, inf.Tx, requestedName, "server_capability")
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, tx, errCode, userErr, sysErr)
+ return
+ }
+
+ //update name and description of a capability
+ query := `UPDATE server_capability sc SET
+ name = $1,
+ description = $2
+ WHERE sc.name = $3
+ RETURNING sc.name, sc.description, sc.last_updated`
+
+ err := tx.QueryRow(query, sc.Name, sc.Description, requestedName).Scan(&sc.Name, &sc.Description, &sc.LastUpdated)
+ if err != nil {
+ if err == sql.ErrNoRows {
+ api.HandleErr(w, r, tx, http.StatusBadRequest, err, nil)
+ return
+ }
+ usrErr, sysErr, code := api.ParseDBError(err)
+ api.HandleErr(w, r, tx, code, usrErr, sysErr)
+ return
+ }
+ alerts := tc.CreateAlerts(tc.SuccessLevel, "server capability was updated")
+ api.WriteAlertsObj(w, r, http.StatusOK, alerts, sc)
+ return
+}
+
+func CreateServerCapability(w http.ResponseWriter, r *http.Request) {
+ inf, userErr, sysErr, errCode := api.NewInfo(r, nil, nil)
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+ defer inf.Close()
+ tx := inf.Tx.Tx
+
+ sc, readValErr := readAndValidateJsonStruct(r)
+ if readValErr != nil {
+ api.HandleErr(w, r, tx, http.StatusBadRequest, readValErr, nil)
+ return
+ }
+
+ // check if capability already exists
+ var count int
+ err := tx.QueryRow("SELECT count(*) from server_capability where name = $1", sc.Name).Scan(&count)
+ if err != nil {
+ api.HandleErr(w, r, tx, http.StatusInternalServerError, nil, fmt.Errorf("error: %w, when checking if server capability with name %s exists", err, sc.Name))
+ return
+ }
+ if count == 1 {
+ api.HandleErr(w, r, tx, http.StatusBadRequest, fmt.Errorf("server_capability name '%s' already exists.", sc.Name), nil)
+ return
+ }
+
+ // create server capability
+ query := `INSERT INTO server_capability (name, description) VALUES ($1, $2) RETURNING last_updated`
+ err = tx.QueryRow(query, sc.Name, sc.Description).Scan(&sc.LastUpdated)
+ if err != nil {
+ if err == sql.ErrNoRows {
+ api.HandleErr(w, r, tx, http.StatusBadRequest, err, nil)
+ return
+ }
+ usrErr, sysErr, code := api.ParseDBError(err)
+ api.HandleErr(w, r, tx, code, usrErr, sysErr)
+ return
+ }
+ alerts := tc.CreateAlerts(tc.SuccessLevel, "server capability was created.")
+ w.Header().Set("Location", fmt.Sprintf("/api/%d.%d/server_capabilities$?name=%s", inf.Version.Major, inf.Version.Minor, sc.Name))
Review Comment:
yes, copy-paste error
--
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-unsubscribe@trafficcontrol.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] ocket8888 commented on a diff in pull request #7213: Added description field to server capabilities
Posted by GitBox <gi...@apache.org>.
ocket8888 commented on code in PR #7213:
URL: https://github.com/apache/trafficcontrol/pull/7213#discussion_r1051157021
##########
traffic_ops/traffic_ops_golang/servercapability/servercapability.go:
##########
@@ -151,6 +153,213 @@ func (v *TOServerCapability) Update(h http.Header) (error, error, int) {
return nil, nil, http.StatusOK
}
+func GetServerCapability(w http.ResponseWriter, r *http.Request) {
+ var sc tc.ServerCapabilityV4
+ inf, userErr, sysErr, errCode := api.NewInfo(r, nil, nil)
+ tx := inf.Tx
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+ defer inf.Close()
+
+ // Query Parameters to Database Query column mappings
+ queryParamsToQueryCols := map[string]dbhelpers.WhereColumnInfo{
+ "name": {Column: "sc.name", Checker: nil},
+ }
+ if _, ok := inf.Params["orderby"]; !ok {
+ inf.Params["orderby"] = "name"
+ }
+ where, orderBy, pagination, queryValues, errs := dbhelpers.BuildWhereAndOrderByAndPagination(inf.Params, queryParamsToQueryCols)
+ if len(errs) > 0 {
+ api.HandleErr(w, r, tx.Tx, http.StatusBadRequest, util.JoinErrs(errs), nil)
+ return
+ }
+
+ selectQuery := "SELECT name, description, last_updated FROM server_capability sc"
+ query := selectQuery + where + orderBy + pagination
+ rows, err := tx.NamedQuery(query, queryValues)
+ if err != nil {
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, nil, fmt.Errorf("server capability read: error getting server capability(ies): %w", err))
+ return
+ }
+ defer log.Close(rows, "unable to close DB connection")
+
+ scList := []tc.ServerCapabilityV4{}
+ for rows.Next() {
+ if err = rows.Scan(&sc.Name, &sc.Description, &sc.LastUpdated); err != nil {
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, nil, fmt.Errorf("error getting server capability(ies): %w", err))
+ return
+ }
+ scList = append(scList, sc)
+ }
+
+ api.WriteResp(w, r, scList)
+ return
+}
+
+func UpdateServerCapability(w http.ResponseWriter, r *http.Request) {
+ inf, userErr, sysErr, errCode := api.NewInfo(r, nil, nil)
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+ defer inf.Close()
+
+ tx := inf.Tx.Tx
+ sc, readValErr := readAndValidateJsonStruct(r)
+ if readValErr != nil {
+ api.HandleErr(w, r, tx, http.StatusBadRequest, readValErr, nil)
+ return
+ }
+
+ requestedName := inf.Params["name"]
+ // check if the entity was already updated
+ userErr, sysErr, errCode = api.CheckIfUnModifiedByName(r.Header, inf.Tx, requestedName, "server_capability")
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, tx, errCode, userErr, sysErr)
+ return
+ }
+
+ //update name and description of a capability
+ query := `UPDATE server_capability sc SET
+ name = $1,
+ description = $2
+ WHERE sc.name = $3
+ RETURNING sc.name, sc.description, sc.last_updated`
+
+ err := tx.QueryRow(query, sc.Name, sc.Description, requestedName).Scan(&sc.Name, &sc.Description, &sc.LastUpdated)
+ if err != nil {
+ if err == sql.ErrNoRows {
+ api.HandleErr(w, r, tx, http.StatusBadRequest, err, nil)
+ return
+ }
+ usrErr, sysErr, code := api.ParseDBError(err)
+ api.HandleErr(w, r, tx, code, usrErr, sysErr)
+ return
+ }
+ alerts := tc.CreateAlerts(tc.SuccessLevel, "server capability was updated")
+ api.WriteAlertsObj(w, r, http.StatusOK, alerts, sc)
+ return
+}
+
+func CreateServerCapability(w http.ResponseWriter, r *http.Request) {
+ inf, userErr, sysErr, errCode := api.NewInfo(r, nil, nil)
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+ defer inf.Close()
+ tx := inf.Tx.Tx
+
+ sc, readValErr := readAndValidateJsonStruct(r)
+ if readValErr != nil {
+ api.HandleErr(w, r, tx, http.StatusBadRequest, readValErr, nil)
+ return
+ }
+
+ // check if capability already exists
+ var count int
+ err := tx.QueryRow("SELECT count(*) from server_capability where name = $1", sc.Name).Scan(&count)
+ if err != nil {
+ api.HandleErr(w, r, tx, http.StatusInternalServerError, nil, fmt.Errorf("error: %w, when checking if server capability with name %s exists", err, sc.Name))
+ return
+ }
+ if count == 1 {
+ api.HandleErr(w, r, tx, http.StatusBadRequest, fmt.Errorf("server_capability name '%s' already exists.", sc.Name), nil)
Review Comment:
They aren't supposed to be doing that, but that's a systemic problem with those tests
--
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-unsubscribe@trafficcontrol.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] github-code-scanning[bot] commented on a diff in pull request #7213: Added description field to server capabilities
Posted by GitBox <gi...@apache.org>.
github-code-scanning[bot] commented on code in PR #7213:
URL: https://github.com/apache/trafficcontrol/pull/7213#discussion_r1042892773
##########
traffic_ops/traffic_ops_golang/servercapability/servercapability.go:
##########
@@ -151,6 +153,144 @@
return nil, nil, http.StatusOK
}
+func GetServerCapability(w http.ResponseWriter, r *http.Request) {
+ var sc tc.ServerCapabilityV41
+ inf, userErr, sysErr, errCode := api.NewInfo(r, nil, nil)
+ tx := inf.Tx
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+ defer inf.Close()
+
+ // Query Parameters to Database Query column mappings
+ queryParamsToQueryCols := map[string]dbhelpers.WhereColumnInfo{
+ "name": dbhelpers.WhereColumnInfo{Column: "sc.name"},
+ }
+ where, orderBy, pagination, queryValues, errs := dbhelpers.BuildWhereAndOrderByAndPagination(inf.Params, queryParamsToQueryCols)
+ if len(errs) > 0 {
+ api.HandleErr(w, r, tx.Tx, http.StatusBadRequest, util.JoinErrs(errs), nil)
+ return
+ }
+
+ selectQuery := "SELECT name, description, last_updated FROM server_capability sc"
+ query := selectQuery + where + orderBy + pagination
+ rows, err := tx.Query(query, queryValues)
Review Comment:
## Database query built from user-controlled sources
This query depends on a [user-provided value](1).
[Show more details](https://github.com/apache/trafficcontrol/security/code-scanning/250)
--
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-unsubscribe@trafficcontrol.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] ocket8888 commented on a diff in pull request #7213: Added description field to server capabilities
Posted by GitBox <gi...@apache.org>.
ocket8888 commented on code in PR #7213:
URL: https://github.com/apache/trafficcontrol/pull/7213#discussion_r1049958365
##########
traffic_ops/traffic_ops_golang/servercapability/servercapability.go:
##########
@@ -151,6 +153,202 @@ func (v *TOServerCapability) Update(h http.Header) (error, error, int) {
return nil, nil, http.StatusOK
}
+func GetServerCapability(w http.ResponseWriter, r *http.Request) {
+ var sc tc.ServerCapabilityV4
+ inf, userErr, sysErr, errCode := api.NewInfo(r, nil, nil)
+ tx := inf.Tx
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+ defer inf.Close()
+
+ // Query Parameters to Database Query column mappings
+ queryParamsToQueryCols := map[string]dbhelpers.WhereColumnInfo{
+ "name": {Column: "sc.name", Checker: nil},
+ }
+ if _, ok := inf.Params["orderby"]; !ok {
+ inf.Params["orderby"] = "name"
+ }
+ where, orderBy, pagination, queryValues, errs := dbhelpers.BuildWhereAndOrderByAndPagination(inf.Params, queryParamsToQueryCols)
+ if len(errs) > 0 {
+ api.HandleErr(w, r, tx.Tx, http.StatusBadRequest, util.JoinErrs(errs), nil)
+ return
+ }
+
+ selectQuery := "SELECT name, description, last_updated FROM server_capability sc"
+ query := selectQuery + where + orderBy + pagination
+ rows, err := tx.NamedQuery(query, queryValues)
+ if err != nil {
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, nil, fmt.Errorf("server capability read: error getting server capability(ies): %w", err))
+ return
+ }
+ defer log.Close(rows, "unable to close DB connection")
+
+ scList := []tc.ServerCapabilityV4{}
+ for rows.Next() {
+ if err = rows.Scan(&sc.Name, &sc.Description, &sc.LastUpdated); err != nil {
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, nil, fmt.Errorf("error getting server capability(ies): %w", err))
+ return
+ }
+ scList = append(scList, sc)
+ }
+
+ api.WriteResp(w, r, scList)
+ return
+}
+
+func UpdateServerCapability(w http.ResponseWriter, r *http.Request) {
+ inf, userErr, sysErr, errCode := api.NewInfo(r, nil, nil)
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+ defer inf.Close()
+
+ tx := inf.Tx.Tx
+ sc, readValErr := readAndValidateJsonStruct(r)
+ if readValErr != nil {
+ api.HandleErr(w, r, tx, http.StatusBadRequest, readValErr, nil)
+ return
+ }
+
+ requestedName := inf.Params["name"]
+ // check if the entity was already updated
+ userErr, sysErr, errCode = api.CheckIfUnModifiedByName(r.Header, inf.Tx, requestedName, "server_capability")
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, tx, errCode, userErr, sysErr)
+ return
+ }
+
+ //update name and description of a capability
+ query := `UPDATE server_capability sc SET
+ name = $1,
+ description = $2
+ WHERE sc.name = $3
+ RETURNING sc.name, sc.description, sc.last_updated`
+
+ err := tx.QueryRow(query, sc.Name, sc.Description, requestedName).Scan(&sc.Name, &sc.Description, &sc.LastUpdated)
+ if err != nil && err == sql.ErrNoRows {
+ api.HandleErr(w, r, tx, http.StatusBadRequest, err, nil)
+ return
+ }
Review Comment:
This ignores all errors that aren't exactly `sql.ErrNoRows`. I think you should just check if the error is nil and use `api.ParseDBError` instead.
--
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-unsubscribe@trafficcontrol.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] rimashah25 commented on a diff in pull request #7213: Added description field to server capabilities
Posted by GitBox <gi...@apache.org>.
rimashah25 commented on code in PR #7213:
URL: https://github.com/apache/trafficcontrol/pull/7213#discussion_r1051122255
##########
traffic_ops/traffic_ops_golang/servercapability/servercapability.go:
##########
@@ -151,6 +153,213 @@ func (v *TOServerCapability) Update(h http.Header) (error, error, int) {
return nil, nil, http.StatusOK
}
+func GetServerCapability(w http.ResponseWriter, r *http.Request) {
+ var sc tc.ServerCapabilityV4
+ inf, userErr, sysErr, errCode := api.NewInfo(r, nil, nil)
+ tx := inf.Tx
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+ defer inf.Close()
+
+ // Query Parameters to Database Query column mappings
+ queryParamsToQueryCols := map[string]dbhelpers.WhereColumnInfo{
+ "name": {Column: "sc.name", Checker: nil},
+ }
+ if _, ok := inf.Params["orderby"]; !ok {
+ inf.Params["orderby"] = "name"
+ }
+ where, orderBy, pagination, queryValues, errs := dbhelpers.BuildWhereAndOrderByAndPagination(inf.Params, queryParamsToQueryCols)
+ if len(errs) > 0 {
+ api.HandleErr(w, r, tx.Tx, http.StatusBadRequest, util.JoinErrs(errs), nil)
+ return
+ }
+
+ selectQuery := "SELECT name, description, last_updated FROM server_capability sc"
+ query := selectQuery + where + orderBy + pagination
+ rows, err := tx.NamedQuery(query, queryValues)
+ if err != nil {
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, nil, fmt.Errorf("server capability read: error getting server capability(ies): %w", err))
+ return
+ }
+ defer log.Close(rows, "unable to close DB connection")
+
+ scList := []tc.ServerCapabilityV4{}
+ for rows.Next() {
+ if err = rows.Scan(&sc.Name, &sc.Description, &sc.LastUpdated); err != nil {
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, nil, fmt.Errorf("error getting server capability(ies): %w", err))
+ return
+ }
+ scList = append(scList, sc)
+ }
+
+ api.WriteResp(w, r, scList)
+ return
+}
+
+func UpdateServerCapability(w http.ResponseWriter, r *http.Request) {
+ inf, userErr, sysErr, errCode := api.NewInfo(r, nil, nil)
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+ defer inf.Close()
+
+ tx := inf.Tx.Tx
+ sc, readValErr := readAndValidateJsonStruct(r)
+ if readValErr != nil {
+ api.HandleErr(w, r, tx, http.StatusBadRequest, readValErr, nil)
+ return
+ }
+
+ requestedName := inf.Params["name"]
+ // check if the entity was already updated
+ userErr, sysErr, errCode = api.CheckIfUnModifiedByName(r.Header, inf.Tx, requestedName, "server_capability")
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, tx, errCode, userErr, sysErr)
+ return
+ }
+
+ //update name and description of a capability
+ query := `UPDATE server_capability sc SET
+ name = $1,
+ description = $2
+ WHERE sc.name = $3
+ RETURNING sc.name, sc.description, sc.last_updated`
+
+ err := tx.QueryRow(query, sc.Name, sc.Description, requestedName).Scan(&sc.Name, &sc.Description, &sc.LastUpdated)
+ if err != nil {
+ if err == sql.ErrNoRows {
+ api.HandleErr(w, r, tx, http.StatusBadRequest, err, nil)
+ return
+ }
+ usrErr, sysErr, code := api.ParseDBError(err)
+ api.HandleErr(w, r, tx, code, usrErr, sysErr)
+ return
+ }
+ alerts := tc.CreateAlerts(tc.SuccessLevel, "server capability was updated")
+ api.WriteAlertsObj(w, r, http.StatusOK, alerts, sc)
+ return
+}
+
+func CreateServerCapability(w http.ResponseWriter, r *http.Request) {
+ inf, userErr, sysErr, errCode := api.NewInfo(r, nil, nil)
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+ defer inf.Close()
+ tx := inf.Tx.Tx
+
+ sc, readValErr := readAndValidateJsonStruct(r)
+ if readValErr != nil {
+ api.HandleErr(w, r, tx, http.StatusBadRequest, readValErr, nil)
+ return
+ }
+
+ // check if capability already exists
+ var count int
+ err := tx.QueryRow("SELECT count(*) from server_capability where name = $1", sc.Name).Scan(&count)
+ if err != nil {
+ api.HandleErr(w, r, tx, http.StatusInternalServerError, nil, fmt.Errorf("error: %w, when checking if server capability with name %s exists", err, sc.Name))
+ return
+ }
+ if count == 1 {
+ api.HandleErr(w, r, tx, http.StatusBadRequest, fmt.Errorf("server_capability name '%s' already exists.", sc.Name), nil)
Review Comment:
@ocket8888 I did... apparently `validaionMessage` in TP tests was expecting it.. and now it is failing.
--
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-unsubscribe@trafficcontrol.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] ocket8888 merged pull request #7213: Added description field to server capabilities
Posted by GitBox <gi...@apache.org>.
ocket8888 merged PR #7213:
URL: https://github.com/apache/trafficcontrol/pull/7213
--
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-unsubscribe@trafficcontrol.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org