You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@trafficcontrol.apache.org by "gbkannan89 (via GitHub)" <gi...@apache.org> on 2023/08/18 08:35:43 UTC
[GitHub] [trafficcontrol] gbkannan89 opened a new pull request, #7733: To origin rfc3339
gbkannan89 opened a new pull request, #7733:
URL: https://github.com/apache/trafficcontrol/pull/7733
Related: https://github.com/apache/trafficcontrol/issues/5911
<hr/>
## Which Traffic Control components are affected by this PR?
- Documentation
- Traffic Ops
## What is the best way to verify this PR?
Curl all Origin v5 REST calls (GET, CREATE, UPDATE, DELETE) and ensure you are getting RFC3339 date-time ( eg: lastUpdated : "2023-05-25T15:59:33.7096-06:00" )
## If this is a bugfix, which Traffic Control versions contained the bug?
- 7.0.x
## 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)
--
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] gbkannan89 commented on a diff in pull request #7733: To origin rfc3339
Posted by "gbkannan89 (via GitHub)" <gi...@apache.org>.
gbkannan89 commented on code in PR #7733:
URL: https://github.com/apache/trafficcontrol/pull/7733#discussion_r1307304091
##########
traffic_ops/traffic_ops_golang/origin/origins.go:
##########
@@ -487,3 +488,277 @@ func deleteQuery() string {
WHERE id=:id`
return query
}
+
+// Get is the handler for GET requests to Origins of APIv5.
+func Get(w http.ResponseWriter, r *http.Request) {
+
+ var useIMS bool
+
+ inf, sysErr, userErr, errCode := api.NewInfo(r, nil, nil)
+ if sysErr != nil || userErr != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+ defer inf.Close()
+
+ origins, userErr, sysErr, errCode, _ := getOrigins(w.Header(), inf.Params, inf.Tx, inf.User, useIMS)
+ var returnable []tc.OriginV5
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+
+ for _, origin := range origins {
+ returnable = append(returnable, origin.ToOriginV5())
+ }
+
+ // Assign an empty array if no origin is found instead of null.
+ if len(returnable) == 0 {
+ returnable = []tc.OriginV5{}
+ }
Review Comment:
removed this line of code as you don't need to do anything here if there are no origins, the response will be an empty array by default
--
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 #7733: To origin rfc3339
Posted by "ocket8888 (via GitHub)" <gi...@apache.org>.
ocket8888 commented on code in PR #7733:
URL: https://github.com/apache/trafficcontrol/pull/7733#discussion_r1312108021
##########
traffic_ops/traffic_ops_golang/origin/origins.go:
##########
@@ -38,6 +38,7 @@ import (
"github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/dbhelpers"
"github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/tenant"
+ "github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/util/ims"
Review Comment:
this line belongs grouped with the rest of the project-internal imports
##########
traffic_ops/traffic_ops_golang/origin/origins.go:
##########
@@ -487,3 +488,294 @@ func deleteQuery() string {
WHERE id=:id`
return query
}
+
+// Get is the handler for GET requests to Origins of APIv5.
+func Get(w http.ResponseWriter, r *http.Request) {
+
+ var useIMS bool
+
+ inf, sysErr, userErr, errCode := api.NewInfo(r, nil, nil)
+ if sysErr != nil || userErr != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+ defer inf.Close()
+
+ origins, userErr, sysErr, errCode, _ := getOrigins(w.Header(), inf.Params, inf.Tx, inf.User, useIMS)
+
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+
+ returnable := make([]tc.OriginV5, len(origins))
+ for i, origin := range origins {
+ returnable[i] = origin.ToOriginV5()
+ }
+
+ api.WriteResp(w, r, returnable)
+ return
+}
+
+// Create Origin with the passed data for APIv5.
+func Create(w http.ResponseWriter, r *http.Request) {
+ inf, sysError, userError, errorCode := api.NewInfo(r, nil, nil)
+ tx := inf.Tx
+ if sysError != nil || userError != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errorCode, userError, sysError)
+ return
+ }
+ defer inf.Close()
+
+ org, errorCode, readValErr := readAndValidateJsonStruct(r, tx)
+ if readValErr != nil {
+ api.HandleErr(w, r, tx.Tx, errorCode, readValErr, nil)
+ return
+ }
+
+ userErr, sysErr, errCode := checkTenancy(&org.TenantID, &org.DeliveryServiceID, tx, inf.User)
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+
+ _, cdnName, _, err := dbhelpers.GetDSNameAndCDNFromID(inf.Tx.Tx, org.DeliveryServiceID)
+ if err != nil {
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, nil, fmt.Errorf("database error: unable to retrieve delivery service name and cdn: %w", err))
+ return
+ }
+ userErr, sysErr, errCode = dbhelpers.CheckIfCurrentUserCanModifyCDN(tx.Tx, string(cdnName), inf.User.UserName)
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+
+ resultRows, err := tx.NamedQuery(insertQuery(), org)
+ if err != nil {
+ usrErr, sysErr, code := api.ParseDBError(err)
+ api.HandleErr(w, r, tx.Tx, code, usrErr, sysErr)
+ return
+ }
+ defer resultRows.Close()
+
+ rowsAffected := 0
+ for resultRows.Next() {
+ rowsAffected++
+ if err := resultRows.Scan(&org.ID, &org.LastUpdated); err != nil {
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, fmt.Errorf("origin create: scanning: %w", err), nil)
+ return
+ }
+ }
+
+ if rowsAffected == 0 {
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, fmt.Errorf("origin create: no rows inserted"), nil)
+ return
+ } else if rowsAffected > 1 {
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, fmt.Errorf("origin create: multiple rows returned"), nil)
+ return
+ }
+
+ alerts := tc.CreateAlerts(tc.SuccessLevel, "origin was created.")
+ w.Header().Set(rfc.Location, fmt.Sprintf("/api/%s/origins?id=%d", inf.Version, org.ID))
+ api.WriteAlertsObj(w, r, http.StatusCreated, alerts, org)
+}
+
+// Update a Origin for APIv5.
+func Update(w http.ResponseWriter, r *http.Request) {
+ inf, sysError, userError, errorCode := api.NewInfo(r, []string{"id"}, []string{"id"})
+ tx := inf.Tx
+ if sysError != nil || userError != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errorCode, userError, sysError)
+ return
+ }
+ defer inf.Close()
+
+ requestedOriginId := inf.IntParams["id"]
+
+ origin, errorCode, readValErr := readAndValidateJsonStruct(r, tx)
+ if readValErr != nil {
+ api.HandleErr(w, r, tx.Tx, errorCode, readValErr, nil)
+ return
+ }
+
+ userErr, sysErr, errCode := checkTenancy(&origin.TenantID, &origin.DeliveryServiceID, tx, inf.User)
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+
+ isPrimary := false
+ ds := 0
+ var existingLastUpdated time.Time
+
+ q := `SELECT is_primary, deliveryservice, last_updated FROM origin WHERE id = $1`
+ errLookup := tx.QueryRow(q, requestedOriginId).Scan(&isPrimary, &ds, &existingLastUpdated)
+ if errLookup != nil {
+ if errors.Is(errLookup, sql.ErrNoRows) {
+ api.HandleErr(w, r, tx.Tx, http.StatusNotFound, fmt.Errorf("no origin exists by id: %d", requestedOriginId), nil)
+ return
+ }
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, nil, fmt.Errorf("database error: %w, when checking if origin with id %d exists", errLookup, requestedOriginId))
+ return
+ }
+ // check if the entity was already updated
+ userErr, sysErr, errCode = api.CheckIfUnModified(r.Header, inf.Tx, requestedOriginId, "origin")
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+
+ if isPrimary && origin.DeliveryServiceID != ds {
+ api.HandleErr(w, r, tx.Tx, http.StatusBadRequest, fmt.Errorf("cannot update the delivery service of a primary origin"), nil)
+ return
+ }
+
+ _, cdnName, _, err := dbhelpers.GetDSNameAndCDNFromID(tx.Tx, origin.DeliveryServiceID)
+ if err != nil {
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, err, nil)
+ return
+ }
+ userErr, sysErr, errCode = dbhelpers.CheckIfCurrentUserCanModifyCDN(tx.Tx, string(cdnName), inf.User.UserName)
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+
+ query := `UPDATE origin SET
+ cachegroup=$1,
+ coordinate=$2,
+ deliveryservice=$3,
+ fqdn=$4,
+ ip6_address=$5,
+ ip_address=$6,
+ name=$7,
+ port=$8,
+ profile=$9,
+ protocol=$10,
+ tenant=$11
+ WHERE id=$12 RETURNING cachegroup, coordinate, deliveryservice, fqdn, ip6_address, ip_address, port, protocol, tenant`
+ errUpdate := tx.QueryRow(query, origin.CachegroupID, origin.CoordinateID, origin.DeliveryServiceID,
+ origin.FQDN, origin.IP6Address, origin.IPAddress, origin.Name,
+ origin.Port, origin.ProfileID, origin.Protocol, origin.TenantID, requestedOriginId).Scan(&origin.Cachegroup, &origin.Coordinate, &origin.DeliveryServiceID,
+ &origin.FQDN, &origin.IP6Address, &origin.IPAddress,
+ &origin.Port, &origin.Protocol, &origin.TenantID)
Review Comment:
It looks like you've fixed the scanning of Tenant IDs into the `Tenant` field, but Cache Group and Coordinate IDs are still being scanned into their respective name fields. Also, this does fix the tenant thing, but it's kind of redundant because you're only scanning in the values that are already set, and thus don't need to be re-scanned. It's not doing the whole struct just for consistency, either. It doesn't matter, just pointing out that you need not waste time when fixing the others.
##########
traffic_ops/traffic_ops_golang/origin/origins.go:
##########
@@ -487,3 +488,294 @@ func deleteQuery() string {
WHERE id=:id`
return query
}
+
+// Get is the handler for GET requests to Origins of APIv5.
+func Get(w http.ResponseWriter, r *http.Request) {
+
+ var useIMS bool
+
+ inf, sysErr, userErr, errCode := api.NewInfo(r, nil, nil)
+ if sysErr != nil || userErr != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+ defer inf.Close()
+
+ origins, userErr, sysErr, errCode, _ := getOrigins(w.Header(), inf.Params, inf.Tx, inf.User, useIMS)
+
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+
+ returnable := make([]tc.OriginV5, len(origins))
+ for i, origin := range origins {
+ returnable[i] = origin.ToOriginV5()
+ }
+
+ api.WriteResp(w, r, returnable)
+ return
+}
+
+// Create Origin with the passed data for APIv5.
+func Create(w http.ResponseWriter, r *http.Request) {
+ inf, sysError, userError, errorCode := api.NewInfo(r, nil, nil)
+ tx := inf.Tx
+ if sysError != nil || userError != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errorCode, userError, sysError)
+ return
+ }
+ defer inf.Close()
+
+ org, errorCode, readValErr := readAndValidateJsonStruct(r, tx)
+ if readValErr != nil {
+ api.HandleErr(w, r, tx.Tx, errorCode, readValErr, nil)
+ return
+ }
+
+ userErr, sysErr, errCode := checkTenancy(&org.TenantID, &org.DeliveryServiceID, tx, inf.User)
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+
+ _, cdnName, _, err := dbhelpers.GetDSNameAndCDNFromID(inf.Tx.Tx, org.DeliveryServiceID)
+ if err != nil {
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, nil, fmt.Errorf("database error: unable to retrieve delivery service name and cdn: %w", err))
+ return
+ }
+ userErr, sysErr, errCode = dbhelpers.CheckIfCurrentUserCanModifyCDN(tx.Tx, string(cdnName), inf.User.UserName)
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+
+ resultRows, err := tx.NamedQuery(insertQuery(), org)
+ if err != nil {
+ usrErr, sysErr, code := api.ParseDBError(err)
+ api.HandleErr(w, r, tx.Tx, code, usrErr, sysErr)
+ return
+ }
+ defer resultRows.Close()
+
+ rowsAffected := 0
+ for resultRows.Next() {
+ rowsAffected++
+ if err := resultRows.Scan(&org.ID, &org.LastUpdated); err != nil {
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, fmt.Errorf("origin create: scanning: %w", err), nil)
+ return
+ }
+ }
+
+ if rowsAffected == 0 {
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, fmt.Errorf("origin create: no rows inserted"), nil)
+ return
+ } else if rowsAffected > 1 {
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, fmt.Errorf("origin create: multiple rows returned"), nil)
+ return
+ }
+
+ alerts := tc.CreateAlerts(tc.SuccessLevel, "origin was created.")
+ w.Header().Set(rfc.Location, fmt.Sprintf("/api/%s/origins?id=%d", inf.Version, org.ID))
+ api.WriteAlertsObj(w, r, http.StatusCreated, alerts, org)
+}
+
+// Update a Origin for APIv5.
+func Update(w http.ResponseWriter, r *http.Request) {
+ inf, sysError, userError, errorCode := api.NewInfo(r, []string{"id"}, []string{"id"})
+ tx := inf.Tx
+ if sysError != nil || userError != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errorCode, userError, sysError)
+ return
+ }
+ defer inf.Close()
+
+ requestedOriginId := inf.IntParams["id"]
+
+ origin, errorCode, readValErr := readAndValidateJsonStruct(r, tx)
+ if readValErr != nil {
+ api.HandleErr(w, r, tx.Tx, errorCode, readValErr, nil)
+ return
+ }
+
+ userErr, sysErr, errCode := checkTenancy(&origin.TenantID, &origin.DeliveryServiceID, tx, inf.User)
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+
+ isPrimary := false
+ ds := 0
+ var existingLastUpdated time.Time
+
+ q := `SELECT is_primary, deliveryservice, last_updated FROM origin WHERE id = $1`
+ errLookup := tx.QueryRow(q, requestedOriginId).Scan(&isPrimary, &ds, &existingLastUpdated)
+ if errLookup != nil {
+ if errors.Is(errLookup, sql.ErrNoRows) {
+ api.HandleErr(w, r, tx.Tx, http.StatusNotFound, fmt.Errorf("no origin exists by id: %d", requestedOriginId), nil)
+ return
+ }
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, nil, fmt.Errorf("database error: %w, when checking if origin with id %d exists", errLookup, requestedOriginId))
+ return
+ }
+ // check if the entity was already updated
+ userErr, sysErr, errCode = api.CheckIfUnModified(r.Header, inf.Tx, requestedOriginId, "origin")
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+
+ if isPrimary && origin.DeliveryServiceID != ds {
+ api.HandleErr(w, r, tx.Tx, http.StatusBadRequest, fmt.Errorf("cannot update the delivery service of a primary origin"), nil)
+ return
+ }
+
+ _, cdnName, _, err := dbhelpers.GetDSNameAndCDNFromID(tx.Tx, origin.DeliveryServiceID)
+ if err != nil {
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, err, nil)
+ return
+ }
+ userErr, sysErr, errCode = dbhelpers.CheckIfCurrentUserCanModifyCDN(tx.Tx, string(cdnName), inf.User.UserName)
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+
+ query := `UPDATE origin SET
+ cachegroup=$1,
+ coordinate=$2,
+ deliveryservice=$3,
+ fqdn=$4,
+ ip6_address=$5,
+ ip_address=$6,
+ name=$7,
+ port=$8,
+ profile=$9,
+ protocol=$10,
+ tenant=$11
+ WHERE id=$12 RETURNING cachegroup, coordinate, deliveryservice, fqdn, ip6_address, ip_address, port, protocol, tenant`
+ errUpdate := tx.QueryRow(query, origin.CachegroupID, origin.CoordinateID, origin.DeliveryServiceID,
+ origin.FQDN, origin.IP6Address, origin.IPAddress, origin.Name,
+ origin.Port, origin.ProfileID, origin.Protocol, origin.TenantID, requestedOriginId).Scan(&origin.Cachegroup, &origin.Coordinate, &origin.DeliveryServiceID,
+ &origin.FQDN, &origin.IP6Address, &origin.IPAddress,
+ &origin.Port, &origin.Protocol, &origin.TenantID)
Review Comment:
This isn't scanning `lastUpdated`, so it's always being reported as the zero value: `"0001-01-01T00:00:00Z"`
--
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 #7733: To origin rfc3339
Posted by "ocket8888 (via GitHub)" <gi...@apache.org>.
ocket8888 merged PR #7733:
URL: https://github.com/apache/trafficcontrol/pull/7733
--
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 #7733: To origin rfc3339
Posted by "ocket8888 (via GitHub)" <gi...@apache.org>.
ocket8888 commented on code in PR #7733:
URL: https://github.com/apache/trafficcontrol/pull/7733#discussion_r1308130343
##########
traffic_ops/testing/api/v5/origins_test.go:
##########
@@ -162,113 +162,113 @@ func TestOrigins(t *testing.T) {
"POST": {
"BAD REQUEST when ALREADY EXISTS": {
ClientSession: TOSession,
- RequestBody: tc.Origin{
- Name: util.Ptr("origin1"),
+ RequestBody: tc.OriginV5{
+ Name: *util.Ptr("origin1"),
Cachegroup: util.Ptr("originCachegroup"),
Coordinate: util.Ptr("coordinate1"),
- DeliveryService: util.Ptr("ds1"),
- FQDN: util.Ptr("origin1.example.com"),
+ DeliveryService: *util.Ptr("ds1"),
+ FQDN: *util.Ptr("origin1.example.com"),
IPAddress: util.Ptr("1.2.3.4"),
IP6Address: util.Ptr("dead:beef:cafe::42"),
Port: util.Ptr(1234),
Profile: util.Ptr("ATS_EDGE_TIER_CACHE"),
- Protocol: util.Ptr("http"),
- TenantID: util.Ptr(GetTenantID(t, "tenant1")()),
- IsPrimary: util.Ptr(true),
+ Protocol: *util.Ptr("http"),
+ TenantID: *util.Ptr(GetTenantID(t, "tenant1")()),
+ IsPrimary: *util.Ptr(true),
},
Expectations: utils.CkRequest(utils.HasError(), utils.HasStatus(http.StatusBadRequest)),
},
"FORBIDDEN when CHILD TENANT CREATES ORIGIN OUTSIDE TENANCY": {
ClientSession: tenant4UserSession,
- RequestBody: tc.Origin{
- Name: util.Ptr("originTenancyTest"),
+ RequestBody: tc.OriginV5{
+ Name: *util.Ptr("originTenancyTest"),
Cachegroup: util.Ptr("originCachegroup"),
- DeliveryServiceID: util.Ptr(GetDeliveryServiceId(t, "ds1")()),
- FQDN: util.Ptr("origintenancy.example.com"),
- Protocol: util.Ptr("http"),
- TenantID: util.Ptr(GetTenantID(t, "tenant3")()),
+ DeliveryServiceID: *util.Ptr(GetDeliveryServiceId(t, "ds1")()),
+ FQDN: *util.Ptr("origintenancy.example.com"),
+ Protocol: *util.Ptr("http"),
+ TenantID: *util.Ptr(GetTenantID(t, "tenant3")()),
Review Comment:
dereferecing and `util.Ptr` are inverse operations. Doing them both is redundant; just get rid of `util.Ptr`
##########
traffic_ops/traffic_ops_golang/origin/origins.go:
##########
@@ -487,3 +488,338 @@ func deleteQuery() string {
WHERE id=:id`
return query
}
+
+// Get is the handler for GET requests to Origins of APIv5.
+func Get(w http.ResponseWriter, r *http.Request) {
+
+ var useIMS bool
+
+ inf, sysErr, userErr, errCode := api.NewInfo(r, nil, nil)
+ if sysErr != nil || userErr != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+ defer inf.Close()
+
+ origins, userErr, sysErr, errCode, _ := getOrigins(w.Header(), inf.Params, inf.Tx, inf.User, useIMS)
+ var returnable []tc.OriginV5
Review Comment:
If zero origins match the filtering parameters sent by the client, this will cause it to return a `null` `response` property, when the proper representation for an empty list is the empty array, i.e. '[]'.
> You don't need to do anything here if there are no origins, the response will be an empty array by default
This is not correct. [The default - or "zero" - value of any slice is `nil` because slices are reference values](https://go.dev/play/p/bFHr2D1Upic).
##########
traffic_ops/v5-client/origin.go:
##########
@@ -85,24 +85,24 @@ func (to *Session) originIDs(origin *tc.Origin) error {
origin.CoordinateID = coordinates.Response[0].ID
}
- if origin.TenantID == nil && origin.Tenant != nil {
- opts.QueryParameters.Set("name", *origin.Tenant)
+ if &origin.TenantID == nil && &origin.Tenant != nil {
Review Comment:
same as above
##########
traffic_ops/v5-client/origin.go:
##########
@@ -48,17 +48,17 @@ func (to *Session) originIDs(origin *tc.Origin) error {
origin.CachegroupID = p.Response[0].ID
}
- if origin.DeliveryServiceID == nil && origin.DeliveryService != nil {
- opts.QueryParameters.Set("xmlId", *origin.DeliveryService)
+ if &origin.DeliveryServiceID == nil && &origin.DeliveryService != nil {
Review Comment:
`&v` for any variable `v` will never be `nil`, so this condition will never pass.
##########
traffic_ops/testing/api/v5/origins_test.go:
##########
@@ -630,12 +630,14 @@ func GetOriginID(t *testing.T, name string) func() int {
assert.RequireNoError(t, err, "Get Origins Request failed with error:", err)
assert.RequireEqual(t, 1, len(origins.Response), "Expected response object length 1, but got %d", len(origins.Response))
assert.RequireNotNil(t, origins.Response[0].ID, "Expected ID to not be nil.")
- return *origins.Response[0].ID
+ return origins.Response[0].ID
}
}
func CreateTestOrigins(t *testing.T) {
for _, origin := range testData.Origins {
+ origin.TenantID = GetTenantID(t, "root")()
+ origin.DeliveryServiceID = GetDeliveryServiceId(t, origin.DeliveryService)()
Review Comment:
Do you know why you had to add this? It's because the changes you made to the `originIDs` function in the client aren't working, because you're looking for `nil` where it's impossible for `nil` to occur.
--
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 #7733: To origin rfc3339
Posted by "ocket8888 (via GitHub)" <gi...@apache.org>.
ocket8888 commented on code in PR #7733:
URL: https://github.com/apache/trafficcontrol/pull/7733#discussion_r1309188621
##########
lib/go-tc/origins.go:
##########
@@ -54,3 +56,75 @@ type Origin struct {
Tenant *string `json:"tenant" db:"tenant"`
TenantID *int `json:"tenantId" db:"tenant_id"`
}
+
+// OriginsResponseV5 is an alias for the latest minor version of the major version 5.
+type OriginsResponseV5 = OriginsResponseV50
+
+// OriginsResponseV50 is a list of Origins as a response for APIv5.
+type OriginsResponseV50 struct {
+ Response []OriginV5 `json:"response"`
+ Alerts
+}
+
+// OriginDetailResponseV5 is an alias for the latest minor version of the major version 5.
+type OriginDetailResponseV5 = OriginDetailResponseV50
+
+// OriginDetailResponseV50 is the JSON object returned for a single origin in APIv5.
+type OriginDetailResponseV50 struct {
+ Response OriginV5 `json:"response"`
+ Alerts
+}
+
+// OriginV5 is an alias for the latest minor version of the major version 5.
+type OriginV5 = OriginV50
+
+// OriginV50 contains information relating to an Origin, in the latest minor version APIv50.
+type OriginV50 struct {
+ Cachegroup *string `json:"cachegroup" db:"cachegroup"`
+ CachegroupID *int `json:"cachegroupId" db:"cachegroup_id"`
+ Coordinate *string `json:"coordinate" db:"coordinate"`
+ CoordinateID *int `json:"coordinateId" db:"coordinate_id"`
+ DeliveryService string `json:"deliveryService" db:"deliveryservice"`
+ DeliveryServiceID int `json:"deliveryServiceId" db:"deliveryservice_id"`
+ FQDN string `json:"fqdn" db:"fqdn"`
+ ID int `json:"id" db:"id"`
+ IP6Address *string `json:"ip6Address" db:"ip6_address"`
+ IPAddress *string `json:"ipAddress" db:"ip_address"`
+ IsPrimary bool `json:"isPrimary" db:"is_primary"`
+ LastUpdated time.Time `json:"lastUpdated" db:"last_updated"`
+ Name string `json:"name" db:"name"`
+ Port *int `json:"port" db:"port"`
+ Profile *string `json:"profile" db:"profile"`
+ ProfileID *int `json:"profileId" db:"profile_id"`
+ Protocol string `json:"protocol" db:"protocol"`
+ Tenant string `json:"tenant" db:"tenant"`
+ TenantID int `json:"tenantId" db:"tenant_id"`
+}
+
+// ToOriginV5 upgrades from Origin to APIv5
Review Comment:
GoDoc comment should end with punctuation.
##########
traffic_ops/testing/api/v5/origins_test.go:
##########
@@ -630,12 +633,14 @@ func GetOriginID(t *testing.T, name string) func() int {
assert.RequireNoError(t, err, "Get Origins Request failed with error:", err)
assert.RequireEqual(t, 1, len(origins.Response), "Expected response object length 1, but got %d", len(origins.Response))
assert.RequireNotNil(t, origins.Response[0].ID, "Expected ID to not be nil.")
- return *origins.Response[0].ID
+ return origins.Response[0].ID
}
}
func CreateTestOrigins(t *testing.T) {
for _, origin := range testData.Origins {
+ origin.TenantID = GetTenantID(t, origin.Tenant)()
+ origin.DeliveryServiceID = GetDeliveryServiceId(t, origin.DeliveryService)()
Review Comment:
with the changes you have made to the client, this should no longer be necessary
--
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] gbkannan89 commented on a diff in pull request #7733: To origin rfc3339
Posted by "gbkannan89 (via GitHub)" <gi...@apache.org>.
gbkannan89 commented on code in PR #7733:
URL: https://github.com/apache/trafficcontrol/pull/7733#discussion_r1307303610
##########
traffic_ops/traffic_ops_golang/origin/origins.go:
##########
@@ -487,3 +488,217 @@ func deleteQuery() string {
WHERE id=:id`
return query
}
+
+// Get is the handler for GET requests to Origins of APIv5
+func Get(w http.ResponseWriter, r *http.Request) {
+
+ var useIMS bool
+
+ inf, sysErr, userErr, errCode := api.NewInfo(r, nil, nil)
+ if sysErr != nil || userErr != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+ defer inf.Close()
+
+ origins, userErr, sysErr, errCode, maxTime := getOrigins(w.Header(), inf.Params, inf.Tx, inf.User, useIMS)
+ var returnable []tc.OriginV5
Review Comment:
You don't need to do anything here if there are no origins, the response will be an empty array by default
--
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] gbkannan89 commented on a diff in pull request #7733: To origin rfc3339
Posted by "gbkannan89 (via GitHub)" <gi...@apache.org>.
gbkannan89 commented on code in PR #7733:
URL: https://github.com/apache/trafficcontrol/pull/7733#discussion_r1307318177
##########
traffic_ops/traffic_ops_golang/origin/origins.go:
##########
@@ -487,3 +488,277 @@ func deleteQuery() string {
WHERE id=:id`
return query
}
+
+// Get is the handler for GET requests to Origins of APIv5.
+func Get(w http.ResponseWriter, r *http.Request) {
+
+ var useIMS bool
+
+ inf, sysErr, userErr, errCode := api.NewInfo(r, nil, nil)
+ if sysErr != nil || userErr != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+ defer inf.Close()
+
+ origins, userErr, sysErr, errCode, _ := getOrigins(w.Header(), inf.Params, inf.Tx, inf.User, useIMS)
+ var returnable []tc.OriginV5
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+
+ for _, origin := range origins {
+ returnable = append(returnable, origin.ToOriginV5())
+ }
+
+ // Assign an empty array if no origin is found instead of null.
+ if len(returnable) == 0 {
+ returnable = []tc.OriginV5{}
+ }
+
+ api.WriteResp(w, r, returnable)
+ return
+}
+
+// Create Origin with the passed data for APIv5.
+func Create(w http.ResponseWriter, r *http.Request) {
+ inf, sysError, userError, errorCode := api.NewInfo(r, nil, nil)
+ tx := inf.Tx
+ if sysError != nil || userError != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errorCode, userError, sysError)
+ return
+ }
+ defer inf.Close()
+ org, readValErr := readAndValidateJsonStruct(r)
+ if readValErr != nil {
+ api.HandleErr(w, r, tx.Tx, http.StatusBadRequest, readValErr, nil)
+ return
+ }
+
+ userErr, sysErr, errCode := checkTenancy(&org.TenantID, &org.DeliveryServiceID, tx, inf.User)
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+
+ _, cdnName, _, err := dbhelpers.GetDSNameAndCDNFromID(inf.Tx.Tx, org.DeliveryServiceID)
+ if err != nil {
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, nil, fmt.Errorf("database error: unable to retrieve delivery service name and cdn: %w", err))
+ return
+ }
+ userErr, sysErr, errCode = dbhelpers.CheckIfCurrentUserCanModifyCDN(tx.Tx, string(cdnName), inf.User.UserName)
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+
+ resultRows, err := tx.NamedQuery(insertQuery(), org)
+ if err != nil {
+ usrErr, sysErr, code := api.ParseDBError(err)
+ api.HandleErr(w, r, tx.Tx, code, usrErr, sysErr)
+ return
+ }
+ defer resultRows.Close()
+
+ rowsAffected := 0
+ for resultRows.Next() {
+ rowsAffected++
+ if err := resultRows.Scan(&org.ID, &org.LastUpdated); err != nil {
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, fmt.Errorf("origin create: scanning: "+err.Error()), nil)
+ return
+ }
+ }
+ if rowsAffected == 0 {
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, fmt.Errorf("origin create: scanning: %w", err), nil)
+ return
+ } else if rowsAffected > 1 {
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, fmt.Errorf("origin create: multiple rows returned"), nil)
+ return
+ }
+
+ alerts := tc.CreateAlerts(tc.SuccessLevel, "origin was created.")
+ w.Header().Set("Location", fmt.Sprintf("/api/%d.%d/origins?id=%d", inf.Version.Major, inf.Version.Minor, org.ID))
+ api.WriteAlertsObj(w, r, http.StatusCreated, alerts, org)
+}
+
+// Update a Origin for APIv5.
+func Update(w http.ResponseWriter, r *http.Request) {
+ inf, sysError, userError, errorCode := api.NewInfo(r, []string{"id"}, []string{"id"})
+ tx := inf.Tx
+ if sysError != nil || userError != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errorCode, userError, sysError)
+ return
+ }
+ defer inf.Close()
+
+ origin, readValErr := readAndValidateJsonStruct(r)
+ if readValErr != nil {
+ api.HandleErr(w, r, tx.Tx, http.StatusBadRequest, readValErr, nil)
+ return
+ }
+ userErr, sysErr, errCode := checkTenancy(&origin.TenantID, &origin.DeliveryServiceID, tx, inf.User)
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+
+ isPrimary := false
+ ds := 0
+ var existingLastUpdated time.Time
+
+ requestedOriginId := inf.IntParams["id"]
+
+ q := `SELECT is_primary, deliveryservice, last_updated FROM origin WHERE id = $1`
+ errLookup := tx.QueryRow(q, requestedOriginId).Scan(&isPrimary, &ds, &existingLastUpdated)
+ if errLookup != nil {
+ if errors.Is(errLookup, sql.ErrNoRows) {
+ api.HandleErr(w, r, tx.Tx, http.StatusNotFound, fmt.Errorf("origin not found"), nil)
+ return
+ }
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, nil, fmt.Errorf("database error: %w, when checking if origin with id %d exists", errLookup, requestedOriginId))
+ return
+ }
+ // check if the entity was already updated
+ userErr, sysErr, errCode = api.CheckIfUnModified(r.Header, inf.Tx, requestedOriginId, "origin")
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+
+ if isPrimary && origin.DeliveryServiceID != ds {
+ api.HandleErr(w, r, tx.Tx, http.StatusBadRequest, fmt.Errorf("cannot update the delivery service of a primary origin"), nil)
+ return
+ }
+
+ _, cdnName, _, err := dbhelpers.GetDSNameAndCDNFromID(tx.Tx, origin.DeliveryServiceID)
+ if err != nil {
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, err, nil)
+ return
+ }
+ userErr, sysErr, errCode = dbhelpers.CheckIfCurrentUserCanModifyCDN(tx.Tx, string(cdnName), inf.User.UserName)
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+
+ var lastUpdated time.Time
+ query := `UPDATE origin SET
+ cachegroup=$1,
+ coordinate=$2,
+ deliveryservice=$3,
+ fqdn=$4,
+ ip6_address=$5,
+ ip_address=$6,
+ name=$7,
+ port=$8,
+ profile=$9,
+ protocol=$10,
+ tenant=$11
+ WHERE id=$12 RETURNING last_updated`
+ errUpdate := tx.QueryRow(query, origin.CachegroupID, origin.CoordinateID, origin.DeliveryServiceID,
+ origin.FQDN, origin.IP6Address, origin.IPAddress, origin.Name,
+ origin.Port, origin.ProfileID, origin.Protocol, origin.TenantID, requestedOriginId).Scan(&lastUpdated)
+ if errUpdate != nil {
+ if errors.Is(errUpdate, sql.ErrNoRows) {
+ api.HandleErr(w, r, tx.Tx, http.StatusBadRequest, fmt.Errorf("origin: %d not found", requestedOriginId), nil)
+ return
+ }
+ usrErr, sysErr, code := api.ParseDBError(errUpdate)
+ api.HandleErr(w, r, tx.Tx, code, usrErr, sysErr)
+ return
+ }
+
+ origin.LastUpdated = lastUpdated
+ alerts := tc.CreateAlerts(tc.SuccessLevel, "origin was updated.")
+ api.WriteAlertsObj(w, r, http.StatusOK, alerts, origin)
+ return
+}
+
+// Delete an Origin for APIv5.
+func Delete(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()
+
+ id := inf.Params["id"]
+ errsValid := tovalidate.ToErrors(validation.Errors{
+ "id": validation.Validate(id, validation.NilOrNotEmpty),
+ })
+
+ if len(errsValid) > 0 {
+ api.HandleErr(w, r, tx, http.StatusBadRequest, fmt.Errorf("missing key: id"), nil)
+ return
+ }
Review Comment:
i did because if id is not given in delete it is now throughing internal server error instead thought of showing an error msg. still i should have got id as IntParam.
i can see inf: &{Params:map[id:10] IntParams:map[]
not getting in intParams so converting into int from string.
--
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 #7733: To origin rfc3339
Posted by "ocket8888 (via GitHub)" <gi...@apache.org>.
ocket8888 commented on code in PR #7733:
URL: https://github.com/apache/trafficcontrol/pull/7733#discussion_r1306064852
##########
traffic_ops/traffic_ops_golang/origin/origins.go:
##########
@@ -487,3 +488,277 @@ func deleteQuery() string {
WHERE id=:id`
return query
}
+
+// Get is the handler for GET requests to Origins of APIv5.
+func Get(w http.ResponseWriter, r *http.Request) {
+
+ var useIMS bool
+
+ inf, sysErr, userErr, errCode := api.NewInfo(r, nil, nil)
+ if sysErr != nil || userErr != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+ defer inf.Close()
+
+ origins, userErr, sysErr, errCode, _ := getOrigins(w.Header(), inf.Params, inf.Tx, inf.User, useIMS)
+ var returnable []tc.OriginV5
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+
+ for _, origin := range origins {
+ returnable = append(returnable, origin.ToOriginV5())
+ }
+
+ // Assign an empty array if no origin is found instead of null.
+ if len(returnable) == 0 {
+ returnable = []tc.OriginV5{}
+ }
+
+ api.WriteResp(w, r, returnable)
+ return
+}
+
+// Create Origin with the passed data for APIv5.
+func Create(w http.ResponseWriter, r *http.Request) {
+ inf, sysError, userError, errorCode := api.NewInfo(r, nil, nil)
+ tx := inf.Tx
+ if sysError != nil || userError != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errorCode, userError, sysError)
+ return
+ }
+ defer inf.Close()
+ org, readValErr := readAndValidateJsonStruct(r)
+ if readValErr != nil {
+ api.HandleErr(w, r, tx.Tx, http.StatusBadRequest, readValErr, nil)
+ return
+ }
+
+ userErr, sysErr, errCode := checkTenancy(&org.TenantID, &org.DeliveryServiceID, tx, inf.User)
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+
+ _, cdnName, _, err := dbhelpers.GetDSNameAndCDNFromID(inf.Tx.Tx, org.DeliveryServiceID)
+ if err != nil {
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, nil, fmt.Errorf("database error: unable to retrieve delivery service name and cdn: %w", err))
+ return
+ }
+ userErr, sysErr, errCode = dbhelpers.CheckIfCurrentUserCanModifyCDN(tx.Tx, string(cdnName), inf.User.UserName)
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+
+ resultRows, err := tx.NamedQuery(insertQuery(), org)
+ if err != nil {
+ usrErr, sysErr, code := api.ParseDBError(err)
+ api.HandleErr(w, r, tx.Tx, code, usrErr, sysErr)
+ return
+ }
+ defer resultRows.Close()
+
+ rowsAffected := 0
+ for resultRows.Next() {
+ rowsAffected++
+ if err := resultRows.Scan(&org.ID, &org.LastUpdated); err != nil {
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, fmt.Errorf("origin create: scanning: "+err.Error()), nil)
Review Comment:
constructing an error from an error should use a wrapping construction
##########
traffic_ops/traffic_ops_golang/origin/origins.go:
##########
@@ -487,3 +488,277 @@ func deleteQuery() string {
WHERE id=:id`
return query
}
+
+// Get is the handler for GET requests to Origins of APIv5.
+func Get(w http.ResponseWriter, r *http.Request) {
+
+ var useIMS bool
+
+ inf, sysErr, userErr, errCode := api.NewInfo(r, nil, nil)
+ if sysErr != nil || userErr != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+ defer inf.Close()
+
+ origins, userErr, sysErr, errCode, _ := getOrigins(w.Header(), inf.Params, inf.Tx, inf.User, useIMS)
+ var returnable []tc.OriginV5
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+
+ for _, origin := range origins {
+ returnable = append(returnable, origin.ToOriginV5())
+ }
+
+ // Assign an empty array if no origin is found instead of null.
+ if len(returnable) == 0 {
+ returnable = []tc.OriginV5{}
+ }
Review Comment:
You don't need to do this if instead you just never set it to an incorrect value; the response can be initialized directly as a zero-length slice immediately.
##########
traffic_ops/traffic_ops_golang/origin/origins.go:
##########
@@ -487,3 +488,277 @@ func deleteQuery() string {
WHERE id=:id`
return query
}
+
+// Get is the handler for GET requests to Origins of APIv5.
+func Get(w http.ResponseWriter, r *http.Request) {
+
+ var useIMS bool
+
+ inf, sysErr, userErr, errCode := api.NewInfo(r, nil, nil)
+ if sysErr != nil || userErr != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+ defer inf.Close()
+
+ origins, userErr, sysErr, errCode, _ := getOrigins(w.Header(), inf.Params, inf.Tx, inf.User, useIMS)
+ var returnable []tc.OriginV5
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+
+ for _, origin := range origins {
+ returnable = append(returnable, origin.ToOriginV5())
+ }
+
+ // Assign an empty array if no origin is found instead of null.
+ if len(returnable) == 0 {
+ returnable = []tc.OriginV5{}
+ }
+
+ api.WriteResp(w, r, returnable)
+ return
+}
+
+// Create Origin with the passed data for APIv5.
+func Create(w http.ResponseWriter, r *http.Request) {
+ inf, sysError, userError, errorCode := api.NewInfo(r, nil, nil)
+ tx := inf.Tx
+ if sysError != nil || userError != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errorCode, userError, sysError)
+ return
+ }
+ defer inf.Close()
+ org, readValErr := readAndValidateJsonStruct(r)
+ if readValErr != nil {
+ api.HandleErr(w, r, tx.Tx, http.StatusBadRequest, readValErr, nil)
+ return
+ }
+
+ userErr, sysErr, errCode := checkTenancy(&org.TenantID, &org.DeliveryServiceID, tx, inf.User)
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+
+ _, cdnName, _, err := dbhelpers.GetDSNameAndCDNFromID(inf.Tx.Tx, org.DeliveryServiceID)
+ if err != nil {
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, nil, fmt.Errorf("database error: unable to retrieve delivery service name and cdn: %w", err))
+ return
+ }
+ userErr, sysErr, errCode = dbhelpers.CheckIfCurrentUserCanModifyCDN(tx.Tx, string(cdnName), inf.User.UserName)
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+
+ resultRows, err := tx.NamedQuery(insertQuery(), org)
+ if err != nil {
+ usrErr, sysErr, code := api.ParseDBError(err)
+ api.HandleErr(w, r, tx.Tx, code, usrErr, sysErr)
+ return
+ }
+ defer resultRows.Close()
+
+ rowsAffected := 0
+ for resultRows.Next() {
+ rowsAffected++
+ if err := resultRows.Scan(&org.ID, &org.LastUpdated); err != nil {
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, fmt.Errorf("origin create: scanning: "+err.Error()), nil)
+ return
+ }
+ }
+ if rowsAffected == 0 {
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, fmt.Errorf("origin create: scanning: %w", err), nil)
+ return
+ } else if rowsAffected > 1 {
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, fmt.Errorf("origin create: multiple rows returned"), nil)
+ return
+ }
+
+ alerts := tc.CreateAlerts(tc.SuccessLevel, "origin was created.")
+ w.Header().Set("Location", fmt.Sprintf("/api/%d.%d/origins?id=%d", inf.Version.Major, inf.Version.Minor, org.ID))
Review Comment:
Instead of using a string literal to name headers, use [constants available in the `github.com/apache/trafficcontrol/lib/go-rfc` package](https://pkg.go.dev/github.com/apache/trafficcontrol/lib/go-rfc#pkg-constants).
Also, just FYI, the `api.Version` struct implements `fmt.Stringer` in a way that formats it exactly the way you're formatting it by hand.
##########
traffic_ops/traffic_ops_golang/origin/origins.go:
##########
@@ -487,3 +488,277 @@ func deleteQuery() string {
WHERE id=:id`
return query
}
+
+// Get is the handler for GET requests to Origins of APIv5.
+func Get(w http.ResponseWriter, r *http.Request) {
+
+ var useIMS bool
+
+ inf, sysErr, userErr, errCode := api.NewInfo(r, nil, nil)
+ if sysErr != nil || userErr != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+ defer inf.Close()
+
+ origins, userErr, sysErr, errCode, _ := getOrigins(w.Header(), inf.Params, inf.Tx, inf.User, useIMS)
+ var returnable []tc.OriginV5
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+
+ for _, origin := range origins {
+ returnable = append(returnable, origin.ToOriginV5())
+ }
+
+ // Assign an empty array if no origin is found instead of null.
+ if len(returnable) == 0 {
+ returnable = []tc.OriginV5{}
+ }
+
+ api.WriteResp(w, r, returnable)
+ return
+}
+
+// Create Origin with the passed data for APIv5.
+func Create(w http.ResponseWriter, r *http.Request) {
+ inf, sysError, userError, errorCode := api.NewInfo(r, nil, nil)
+ tx := inf.Tx
+ if sysError != nil || userError != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errorCode, userError, sysError)
+ return
+ }
+ defer inf.Close()
+ org, readValErr := readAndValidateJsonStruct(r)
+ if readValErr != nil {
+ api.HandleErr(w, r, tx.Tx, http.StatusBadRequest, readValErr, nil)
+ return
+ }
+
+ userErr, sysErr, errCode := checkTenancy(&org.TenantID, &org.DeliveryServiceID, tx, inf.User)
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+
+ _, cdnName, _, err := dbhelpers.GetDSNameAndCDNFromID(inf.Tx.Tx, org.DeliveryServiceID)
+ if err != nil {
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, nil, fmt.Errorf("database error: unable to retrieve delivery service name and cdn: %w", err))
+ return
+ }
+ userErr, sysErr, errCode = dbhelpers.CheckIfCurrentUserCanModifyCDN(tx.Tx, string(cdnName), inf.User.UserName)
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+
+ resultRows, err := tx.NamedQuery(insertQuery(), org)
+ if err != nil {
+ usrErr, sysErr, code := api.ParseDBError(err)
+ api.HandleErr(w, r, tx.Tx, code, usrErr, sysErr)
+ return
+ }
+ defer resultRows.Close()
+
+ rowsAffected := 0
+ for resultRows.Next() {
+ rowsAffected++
+ if err := resultRows.Scan(&org.ID, &org.LastUpdated); err != nil {
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, fmt.Errorf("origin create: scanning: "+err.Error()), nil)
+ return
+ }
+ }
+ if rowsAffected == 0 {
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, fmt.Errorf("origin create: scanning: %w", err), nil)
+ return
+ } else if rowsAffected > 1 {
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, fmt.Errorf("origin create: multiple rows returned"), nil)
+ return
+ }
+
+ alerts := tc.CreateAlerts(tc.SuccessLevel, "origin was created.")
+ w.Header().Set("Location", fmt.Sprintf("/api/%d.%d/origins?id=%d", inf.Version.Major, inf.Version.Minor, org.ID))
+ api.WriteAlertsObj(w, r, http.StatusCreated, alerts, org)
+}
+
+// Update a Origin for APIv5.
+func Update(w http.ResponseWriter, r *http.Request) {
+ inf, sysError, userError, errorCode := api.NewInfo(r, []string{"id"}, []string{"id"})
+ tx := inf.Tx
+ if sysError != nil || userError != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errorCode, userError, sysError)
+ return
+ }
+ defer inf.Close()
+
+ origin, readValErr := readAndValidateJsonStruct(r)
+ if readValErr != nil {
+ api.HandleErr(w, r, tx.Tx, http.StatusBadRequest, readValErr, nil)
+ return
+ }
+ userErr, sysErr, errCode := checkTenancy(&origin.TenantID, &origin.DeliveryServiceID, tx, inf.User)
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+
+ isPrimary := false
+ ds := 0
+ var existingLastUpdated time.Time
+
+ requestedOriginId := inf.IntParams["id"]
+
+ q := `SELECT is_primary, deliveryservice, last_updated FROM origin WHERE id = $1`
+ errLookup := tx.QueryRow(q, requestedOriginId).Scan(&isPrimary, &ds, &existingLastUpdated)
+ if errLookup != nil {
+ if errors.Is(errLookup, sql.ErrNoRows) {
+ api.HandleErr(w, r, tx.Tx, http.StatusNotFound, fmt.Errorf("origin not found"), nil)
+ return
+ }
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, nil, fmt.Errorf("database error: %w, when checking if origin with id %d exists", errLookup, requestedOriginId))
+ return
+ }
+ // check if the entity was already updated
+ userErr, sysErr, errCode = api.CheckIfUnModified(r.Header, inf.Tx, requestedOriginId, "origin")
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+
+ if isPrimary && origin.DeliveryServiceID != ds {
+ api.HandleErr(w, r, tx.Tx, http.StatusBadRequest, fmt.Errorf("cannot update the delivery service of a primary origin"), nil)
+ return
+ }
+
+ _, cdnName, _, err := dbhelpers.GetDSNameAndCDNFromID(tx.Tx, origin.DeliveryServiceID)
+ if err != nil {
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, err, nil)
+ return
+ }
+ userErr, sysErr, errCode = dbhelpers.CheckIfCurrentUserCanModifyCDN(tx.Tx, string(cdnName), inf.User.UserName)
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+
+ var lastUpdated time.Time
+ query := `UPDATE origin SET
+ cachegroup=$1,
+ coordinate=$2,
+ deliveryservice=$3,
+ fqdn=$4,
+ ip6_address=$5,
+ ip_address=$6,
+ name=$7,
+ port=$8,
+ profile=$9,
+ protocol=$10,
+ tenant=$11
+ WHERE id=$12 RETURNING last_updated`
+ errUpdate := tx.QueryRow(query, origin.CachegroupID, origin.CoordinateID, origin.DeliveryServiceID,
+ origin.FQDN, origin.IP6Address, origin.IPAddress, origin.Name,
+ origin.Port, origin.ProfileID, origin.Protocol, origin.TenantID, requestedOriginId).Scan(&lastUpdated)
+ if errUpdate != nil {
+ if errors.Is(errUpdate, sql.ErrNoRows) {
+ api.HandleErr(w, r, tx.Tx, http.StatusBadRequest, fmt.Errorf("origin: %d not found", requestedOriginId), nil)
Review Comment:
If something isn't found, we should use `http.StatusNotFound` as the response status code
##########
traffic_ops/traffic_ops_golang/origin/origins.go:
##########
@@ -487,3 +488,277 @@ func deleteQuery() string {
WHERE id=:id`
return query
}
+
+// Get is the handler for GET requests to Origins of APIv5.
+func Get(w http.ResponseWriter, r *http.Request) {
+
+ var useIMS bool
+
+ inf, sysErr, userErr, errCode := api.NewInfo(r, nil, nil)
+ if sysErr != nil || userErr != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+ defer inf.Close()
+
+ origins, userErr, sysErr, errCode, _ := getOrigins(w.Header(), inf.Params, inf.Tx, inf.User, useIMS)
+ var returnable []tc.OriginV5
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+
+ for _, origin := range origins {
+ returnable = append(returnable, origin.ToOriginV5())
+ }
+
+ // Assign an empty array if no origin is found instead of null.
+ if len(returnable) == 0 {
+ returnable = []tc.OriginV5{}
+ }
+
+ api.WriteResp(w, r, returnable)
+ return
+}
+
+// Create Origin with the passed data for APIv5.
+func Create(w http.ResponseWriter, r *http.Request) {
+ inf, sysError, userError, errorCode := api.NewInfo(r, nil, nil)
+ tx := inf.Tx
+ if sysError != nil || userError != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errorCode, userError, sysError)
+ return
+ }
+ defer inf.Close()
+ org, readValErr := readAndValidateJsonStruct(r)
+ if readValErr != nil {
+ api.HandleErr(w, r, tx.Tx, http.StatusBadRequest, readValErr, nil)
+ return
+ }
+
+ userErr, sysErr, errCode := checkTenancy(&org.TenantID, &org.DeliveryServiceID, tx, inf.User)
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+
+ _, cdnName, _, err := dbhelpers.GetDSNameAndCDNFromID(inf.Tx.Tx, org.DeliveryServiceID)
+ if err != nil {
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, nil, fmt.Errorf("database error: unable to retrieve delivery service name and cdn: %w", err))
+ return
+ }
+ userErr, sysErr, errCode = dbhelpers.CheckIfCurrentUserCanModifyCDN(tx.Tx, string(cdnName), inf.User.UserName)
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+
+ resultRows, err := tx.NamedQuery(insertQuery(), org)
+ if err != nil {
+ usrErr, sysErr, code := api.ParseDBError(err)
+ api.HandleErr(w, r, tx.Tx, code, usrErr, sysErr)
+ return
+ }
+ defer resultRows.Close()
+
+ rowsAffected := 0
+ for resultRows.Next() {
+ rowsAffected++
+ if err := resultRows.Scan(&org.ID, &org.LastUpdated); err != nil {
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, fmt.Errorf("origin create: scanning: "+err.Error()), nil)
+ return
+ }
+ }
+ if rowsAffected == 0 {
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, fmt.Errorf("origin create: scanning: %w", err), nil)
+ return
+ } else if rowsAffected > 1 {
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, fmt.Errorf("origin create: multiple rows returned"), nil)
+ return
+ }
+
+ alerts := tc.CreateAlerts(tc.SuccessLevel, "origin was created.")
+ w.Header().Set("Location", fmt.Sprintf("/api/%d.%d/origins?id=%d", inf.Version.Major, inf.Version.Minor, org.ID))
+ api.WriteAlertsObj(w, r, http.StatusCreated, alerts, org)
+}
+
+// Update a Origin for APIv5.
+func Update(w http.ResponseWriter, r *http.Request) {
+ inf, sysError, userError, errorCode := api.NewInfo(r, []string{"id"}, []string{"id"})
+ tx := inf.Tx
+ if sysError != nil || userError != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errorCode, userError, sysError)
+ return
+ }
+ defer inf.Close()
+
+ origin, readValErr := readAndValidateJsonStruct(r)
+ if readValErr != nil {
+ api.HandleErr(w, r, tx.Tx, http.StatusBadRequest, readValErr, nil)
+ return
+ }
+ userErr, sysErr, errCode := checkTenancy(&origin.TenantID, &origin.DeliveryServiceID, tx, inf.User)
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+
+ isPrimary := false
+ ds := 0
+ var existingLastUpdated time.Time
+
+ requestedOriginId := inf.IntParams["id"]
+
+ q := `SELECT is_primary, deliveryservice, last_updated FROM origin WHERE id = $1`
+ errLookup := tx.QueryRow(q, requestedOriginId).Scan(&isPrimary, &ds, &existingLastUpdated)
+ if errLookup != nil {
+ if errors.Is(errLookup, sql.ErrNoRows) {
+ api.HandleErr(w, r, tx.Tx, http.StatusNotFound, fmt.Errorf("origin not found"), nil)
+ return
+ }
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, nil, fmt.Errorf("database error: %w, when checking if origin with id %d exists", errLookup, requestedOriginId))
+ return
+ }
+ // check if the entity was already updated
+ userErr, sysErr, errCode = api.CheckIfUnModified(r.Header, inf.Tx, requestedOriginId, "origin")
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+
+ if isPrimary && origin.DeliveryServiceID != ds {
+ api.HandleErr(w, r, tx.Tx, http.StatusBadRequest, fmt.Errorf("cannot update the delivery service of a primary origin"), nil)
+ return
+ }
+
+ _, cdnName, _, err := dbhelpers.GetDSNameAndCDNFromID(tx.Tx, origin.DeliveryServiceID)
+ if err != nil {
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, err, nil)
+ return
+ }
+ userErr, sysErr, errCode = dbhelpers.CheckIfCurrentUserCanModifyCDN(tx.Tx, string(cdnName), inf.User.UserName)
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+
+ var lastUpdated time.Time
+ query := `UPDATE origin SET
+ cachegroup=$1,
+ coordinate=$2,
+ deliveryservice=$3,
+ fqdn=$4,
+ ip6_address=$5,
+ ip_address=$6,
+ name=$7,
+ port=$8,
+ profile=$9,
+ protocol=$10,
+ tenant=$11
+ WHERE id=$12 RETURNING last_updated`
+ errUpdate := tx.QueryRow(query, origin.CachegroupID, origin.CoordinateID, origin.DeliveryServiceID,
+ origin.FQDN, origin.IP6Address, origin.IPAddress, origin.Name,
+ origin.Port, origin.ProfileID, origin.Protocol, origin.TenantID, requestedOriginId).Scan(&lastUpdated)
+ if errUpdate != nil {
+ if errors.Is(errUpdate, sql.ErrNoRows) {
+ api.HandleErr(w, r, tx.Tx, http.StatusBadRequest, fmt.Errorf("origin: %d not found", requestedOriginId), nil)
+ return
+ }
+ usrErr, sysErr, code := api.ParseDBError(errUpdate)
+ api.HandleErr(w, r, tx.Tx, code, usrErr, sysErr)
+ return
+ }
+
+ origin.LastUpdated = lastUpdated
+ alerts := tc.CreateAlerts(tc.SuccessLevel, "origin was updated.")
+ api.WriteAlertsObj(w, r, http.StatusOK, alerts, origin)
+ return
+}
+
+// Delete an Origin for APIv5.
+func Delete(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()
+
+ id := inf.Params["id"]
+ errsValid := tovalidate.ToErrors(validation.Errors{
+ "id": validation.Validate(id, validation.NilOrNotEmpty),
+ })
+
+ if len(errsValid) > 0 {
+ api.HandleErr(w, r, tx, http.StatusBadRequest, fmt.Errorf("missing key: id"), nil)
+ return
+ }
Review Comment:
You can avoid doing all of these by simply specifying `id` as an `IntParam` in your call to `NewInfo`.
##########
traffic_ops/traffic_ops_golang/origin/origins.go:
##########
@@ -487,3 +488,277 @@ func deleteQuery() string {
WHERE id=:id`
return query
}
+
+// Get is the handler for GET requests to Origins of APIv5.
+func Get(w http.ResponseWriter, r *http.Request) {
+
+ var useIMS bool
+
+ inf, sysErr, userErr, errCode := api.NewInfo(r, nil, nil)
+ if sysErr != nil || userErr != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+ defer inf.Close()
+
+ origins, userErr, sysErr, errCode, _ := getOrigins(w.Header(), inf.Params, inf.Tx, inf.User, useIMS)
+ var returnable []tc.OriginV5
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+
+ for _, origin := range origins {
+ returnable = append(returnable, origin.ToOriginV5())
+ }
+
+ // Assign an empty array if no origin is found instead of null.
+ if len(returnable) == 0 {
+ returnable = []tc.OriginV5{}
+ }
+
+ api.WriteResp(w, r, returnable)
+ return
+}
+
+// Create Origin with the passed data for APIv5.
+func Create(w http.ResponseWriter, r *http.Request) {
+ inf, sysError, userError, errorCode := api.NewInfo(r, nil, nil)
+ tx := inf.Tx
+ if sysError != nil || userError != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errorCode, userError, sysError)
+ return
+ }
+ defer inf.Close()
+ org, readValErr := readAndValidateJsonStruct(r)
+ if readValErr != nil {
+ api.HandleErr(w, r, tx.Tx, http.StatusBadRequest, readValErr, nil)
+ return
+ }
+
+ userErr, sysErr, errCode := checkTenancy(&org.TenantID, &org.DeliveryServiceID, tx, inf.User)
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+
+ _, cdnName, _, err := dbhelpers.GetDSNameAndCDNFromID(inf.Tx.Tx, org.DeliveryServiceID)
+ if err != nil {
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, nil, fmt.Errorf("database error: unable to retrieve delivery service name and cdn: %w", err))
+ return
+ }
+ userErr, sysErr, errCode = dbhelpers.CheckIfCurrentUserCanModifyCDN(tx.Tx, string(cdnName), inf.User.UserName)
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+
+ resultRows, err := tx.NamedQuery(insertQuery(), org)
+ if err != nil {
+ usrErr, sysErr, code := api.ParseDBError(err)
+ api.HandleErr(w, r, tx.Tx, code, usrErr, sysErr)
+ return
+ }
+ defer resultRows.Close()
+
+ rowsAffected := 0
+ for resultRows.Next() {
+ rowsAffected++
+ if err := resultRows.Scan(&org.ID, &org.LastUpdated); err != nil {
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, fmt.Errorf("origin create: scanning: "+err.Error()), nil)
+ return
+ }
+ }
+ if rowsAffected == 0 {
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, fmt.Errorf("origin create: scanning: %w", err), nil)
+ return
+ } else if rowsAffected > 1 {
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, fmt.Errorf("origin create: multiple rows returned"), nil)
+ return
+ }
+
+ alerts := tc.CreateAlerts(tc.SuccessLevel, "origin was created.")
+ w.Header().Set("Location", fmt.Sprintf("/api/%d.%d/origins?id=%d", inf.Version.Major, inf.Version.Minor, org.ID))
+ api.WriteAlertsObj(w, r, http.StatusCreated, alerts, org)
+}
+
+// Update a Origin for APIv5.
+func Update(w http.ResponseWriter, r *http.Request) {
+ inf, sysError, userError, errorCode := api.NewInfo(r, []string{"id"}, []string{"id"})
+ tx := inf.Tx
+ if sysError != nil || userError != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errorCode, userError, sysError)
+ return
+ }
+ defer inf.Close()
+
+ origin, readValErr := readAndValidateJsonStruct(r)
+ if readValErr != nil {
+ api.HandleErr(w, r, tx.Tx, http.StatusBadRequest, readValErr, nil)
+ return
+ }
+ userErr, sysErr, errCode := checkTenancy(&origin.TenantID, &origin.DeliveryServiceID, tx, inf.User)
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+
+ isPrimary := false
+ ds := 0
+ var existingLastUpdated time.Time
+
+ requestedOriginId := inf.IntParams["id"]
+
+ q := `SELECT is_primary, deliveryservice, last_updated FROM origin WHERE id = $1`
+ errLookup := tx.QueryRow(q, requestedOriginId).Scan(&isPrimary, &ds, &existingLastUpdated)
+ if errLookup != nil {
+ if errors.Is(errLookup, sql.ErrNoRows) {
+ api.HandleErr(w, r, tx.Tx, http.StatusNotFound, fmt.Errorf("origin not found"), nil)
+ return
+ }
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, nil, fmt.Errorf("database error: %w, when checking if origin with id %d exists", errLookup, requestedOriginId))
+ return
+ }
+ // check if the entity was already updated
+ userErr, sysErr, errCode = api.CheckIfUnModified(r.Header, inf.Tx, requestedOriginId, "origin")
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+
+ if isPrimary && origin.DeliveryServiceID != ds {
+ api.HandleErr(w, r, tx.Tx, http.StatusBadRequest, fmt.Errorf("cannot update the delivery service of a primary origin"), nil)
+ return
+ }
+
+ _, cdnName, _, err := dbhelpers.GetDSNameAndCDNFromID(tx.Tx, origin.DeliveryServiceID)
+ if err != nil {
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, err, nil)
+ return
+ }
+ userErr, sysErr, errCode = dbhelpers.CheckIfCurrentUserCanModifyCDN(tx.Tx, string(cdnName), inf.User.UserName)
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+
+ var lastUpdated time.Time
+ query := `UPDATE origin SET
+ cachegroup=$1,
+ coordinate=$2,
+ deliveryservice=$3,
+ fqdn=$4,
+ ip6_address=$5,
+ ip_address=$6,
+ name=$7,
+ port=$8,
+ profile=$9,
+ protocol=$10,
+ tenant=$11
+ WHERE id=$12 RETURNING last_updated`
+ errUpdate := tx.QueryRow(query, origin.CachegroupID, origin.CoordinateID, origin.DeliveryServiceID,
+ origin.FQDN, origin.IP6Address, origin.IPAddress, origin.Name,
+ origin.Port, origin.ProfileID, origin.Protocol, origin.TenantID, requestedOriginId).Scan(&lastUpdated)
+ if errUpdate != nil {
+ if errors.Is(errUpdate, sql.ErrNoRows) {
+ api.HandleErr(w, r, tx.Tx, http.StatusBadRequest, fmt.Errorf("origin: %d not found", requestedOriginId), nil)
+ return
+ }
+ usrErr, sysErr, code := api.ParseDBError(errUpdate)
+ api.HandleErr(w, r, tx.Tx, code, usrErr, sysErr)
+ return
+ }
+
+ origin.LastUpdated = lastUpdated
+ alerts := tc.CreateAlerts(tc.SuccessLevel, "origin was updated.")
+ api.WriteAlertsObj(w, r, http.StatusOK, alerts, origin)
+ return
+}
+
+// Delete an Origin for APIv5.
+func Delete(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()
+
+ id := inf.Params["id"]
+ errsValid := tovalidate.ToErrors(validation.Errors{
+ "id": validation.Validate(id, validation.NilOrNotEmpty),
+ })
+
+ if len(errsValid) > 0 {
+ api.HandleErr(w, r, tx, http.StatusBadRequest, fmt.Errorf("missing key: id"), nil)
+ return
+ }
+
+ isPrimary := false
+ if err := tx.QueryRow(`SELECT is_primary FROM origin WHERE id = $1`, id).Scan(&isPrimary); err != nil {
+ if err == sql.ErrNoRows {
Review Comment:
checking for error identity should use `errors.Is`
##########
traffic_ops/traffic_ops_golang/origin/origins.go:
##########
@@ -487,3 +488,277 @@ func deleteQuery() string {
WHERE id=:id`
return query
}
+
+// Get is the handler for GET requests to Origins of APIv5.
+func Get(w http.ResponseWriter, r *http.Request) {
+
+ var useIMS bool
+
+ inf, sysErr, userErr, errCode := api.NewInfo(r, nil, nil)
+ if sysErr != nil || userErr != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+ defer inf.Close()
+
+ origins, userErr, sysErr, errCode, _ := getOrigins(w.Header(), inf.Params, inf.Tx, inf.User, useIMS)
+ var returnable []tc.OriginV5
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+
+ for _, origin := range origins {
+ returnable = append(returnable, origin.ToOriginV5())
+ }
+
+ // Assign an empty array if no origin is found instead of null.
+ if len(returnable) == 0 {
+ returnable = []tc.OriginV5{}
+ }
+
+ api.WriteResp(w, r, returnable)
+ return
+}
+
+// Create Origin with the passed data for APIv5.
+func Create(w http.ResponseWriter, r *http.Request) {
+ inf, sysError, userError, errorCode := api.NewInfo(r, nil, nil)
+ tx := inf.Tx
+ if sysError != nil || userError != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errorCode, userError, sysError)
+ return
+ }
+ defer inf.Close()
+ org, readValErr := readAndValidateJsonStruct(r)
+ if readValErr != nil {
+ api.HandleErr(w, r, tx.Tx, http.StatusBadRequest, readValErr, nil)
+ return
+ }
+
+ userErr, sysErr, errCode := checkTenancy(&org.TenantID, &org.DeliveryServiceID, tx, inf.User)
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+
+ _, cdnName, _, err := dbhelpers.GetDSNameAndCDNFromID(inf.Tx.Tx, org.DeliveryServiceID)
+ if err != nil {
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, nil, fmt.Errorf("database error: unable to retrieve delivery service name and cdn: %w", err))
+ return
+ }
+ userErr, sysErr, errCode = dbhelpers.CheckIfCurrentUserCanModifyCDN(tx.Tx, string(cdnName), inf.User.UserName)
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+
+ resultRows, err := tx.NamedQuery(insertQuery(), org)
+ if err != nil {
+ usrErr, sysErr, code := api.ParseDBError(err)
+ api.HandleErr(w, r, tx.Tx, code, usrErr, sysErr)
+ return
+ }
+ defer resultRows.Close()
+
+ rowsAffected := 0
+ for resultRows.Next() {
+ rowsAffected++
+ if err := resultRows.Scan(&org.ID, &org.LastUpdated); err != nil {
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, fmt.Errorf("origin create: scanning: "+err.Error()), nil)
+ return
+ }
+ }
+ if rowsAffected == 0 {
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, fmt.Errorf("origin create: scanning: %w", err), nil)
Review Comment:
The error you're wrapping here will always be `nil` because if it wasn't execution wouldn't reach this point.
--
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 #7733: To origin rfc3339
Posted by "ocket8888 (via GitHub)" <gi...@apache.org>.
ocket8888 commented on code in PR #7733:
URL: https://github.com/apache/trafficcontrol/pull/7733#discussion_r1311911252
##########
traffic_ops/traffic_ops_golang/origin/origins.go:
##########
@@ -487,3 +487,294 @@ func deleteQuery() string {
WHERE id=:id`
return query
}
+
+// Get is the handler for GET requests to Origins of APIv5.
+func Get(w http.ResponseWriter, r *http.Request) {
+
+ var useIMS bool
+
+ inf, sysErr, userErr, errCode := api.NewInfo(r, nil, nil)
+ if sysErr != nil || userErr != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+ defer inf.Close()
+
+ origins, userErr, sysErr, errCode, _ := getOrigins(w.Header(), inf.Params, inf.Tx, inf.User, useIMS)
+
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+
+ returnable := make([]tc.OriginV5, len(origins))
+ for i, origin := range origins {
+ returnable[i] = origin.ToOriginV5()
+ }
+
+ api.WriteResp(w, r, returnable)
+ return
+}
+
+// Create Origin with the passed data for APIv5.
+func Create(w http.ResponseWriter, r *http.Request) {
+ inf, sysError, userError, errorCode := api.NewInfo(r, nil, nil)
+ tx := inf.Tx
+ if sysError != nil || userError != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errorCode, userError, sysError)
+ return
+ }
+ defer inf.Close()
+
+ org, errorCode, readValErr := readAndValidateJsonStruct(r, tx)
+ if readValErr != nil {
+ api.HandleErr(w, r, tx.Tx, errorCode, readValErr, nil)
+ return
+ }
+
+ userErr, sysErr, errCode := checkTenancy(&org.TenantID, &org.DeliveryServiceID, tx, inf.User)
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+
+ _, cdnName, _, err := dbhelpers.GetDSNameAndCDNFromID(inf.Tx.Tx, org.DeliveryServiceID)
+ if err != nil {
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, nil, fmt.Errorf("database error: unable to retrieve delivery service name and cdn: %w", err))
+ return
+ }
+ userErr, sysErr, errCode = dbhelpers.CheckIfCurrentUserCanModifyCDN(tx.Tx, string(cdnName), inf.User.UserName)
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+
+ resultRows, err := tx.NamedQuery(insertQuery(), org)
+ if err != nil {
+ usrErr, sysErr, code := api.ParseDBError(err)
+ api.HandleErr(w, r, tx.Tx, code, usrErr, sysErr)
+ return
+ }
+ defer resultRows.Close()
+
+ rowsAffected := 0
+ for resultRows.Next() {
+ rowsAffected++
+ if err := resultRows.Scan(&org.ID, &org.LastUpdated); err != nil {
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, fmt.Errorf("origin create: scanning: %w", err), nil)
+ return
+ }
+ }
+
+ if rowsAffected == 0 {
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, fmt.Errorf("origin create: no rows inserted"), nil)
+ return
+ } else if rowsAffected > 1 {
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, fmt.Errorf("origin create: multiple rows returned"), nil)
+ return
+ }
+
+ alerts := tc.CreateAlerts(tc.SuccessLevel, "origin was created.")
+ w.Header().Set(rfc.Location, fmt.Sprintf("/api/%s/origins?id=%d", inf.Version, org.ID))
+ api.WriteAlertsObj(w, r, http.StatusCreated, alerts, org)
+}
+
+// Update a Origin for APIv5.
+func Update(w http.ResponseWriter, r *http.Request) {
+ inf, sysError, userError, errorCode := api.NewInfo(r, []string{"id"}, []string{"id"})
+ tx := inf.Tx
+ if sysError != nil || userError != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errorCode, userError, sysError)
+ return
+ }
+ defer inf.Close()
+
+ requestedOriginId := inf.IntParams["id"]
+
+ origin, errorCode, readValErr := readAndValidateJsonStruct(r, tx)
+ if readValErr != nil {
+ api.HandleErr(w, r, tx.Tx, errorCode, readValErr, nil)
+ return
+ }
+
+ userErr, sysErr, errCode := checkTenancy(&origin.TenantID, &origin.DeliveryServiceID, tx, inf.User)
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+
+ isPrimary := false
+ ds := 0
+ var existingLastUpdated time.Time
+
+ q := `SELECT is_primary, deliveryservice, last_updated FROM origin WHERE id = $1`
+ errLookup := tx.QueryRow(q, requestedOriginId).Scan(&isPrimary, &ds, &existingLastUpdated)
+ if errLookup != nil {
+ if errors.Is(errLookup, sql.ErrNoRows) {
+ api.HandleErr(w, r, tx.Tx, http.StatusNotFound, fmt.Errorf("no origin exists by id: %d", requestedOriginId), nil)
+ return
+ }
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, nil, fmt.Errorf("database error: %w, when checking if origin with id %d exists", errLookup, requestedOriginId))
+ return
+ }
+ // check if the entity was already updated
+ userErr, sysErr, errCode = api.CheckIfUnModified(r.Header, inf.Tx, requestedOriginId, "origin")
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+
+ if isPrimary && origin.DeliveryServiceID != ds {
+ api.HandleErr(w, r, tx.Tx, http.StatusBadRequest, fmt.Errorf("cannot update the delivery service of a primary origin"), nil)
+ return
+ }
+
+ _, cdnName, _, err := dbhelpers.GetDSNameAndCDNFromID(tx.Tx, origin.DeliveryServiceID)
+ if err != nil {
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, err, nil)
+ return
+ }
+ userErr, sysErr, errCode = dbhelpers.CheckIfCurrentUserCanModifyCDN(tx.Tx, string(cdnName), inf.User.UserName)
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+
+ query := `UPDATE origin SET
+ cachegroup=$1,
+ coordinate=$2,
+ deliveryservice=$3,
+ fqdn=$4,
+ ip6_address=$5,
+ ip_address=$6,
+ name=$7,
+ port=$8,
+ profile=$9,
+ protocol=$10,
+ tenant=$11
+ WHERE id=$12 RETURNING cachegroup, coordinate, deliveryservice, fqdn, ip6_address, ip_address, port, protocol, tenant`
Review Comment:
IMO, these things would be easier to see if each argument was on its own line, making it easier to match returned fields up with the struct fields into which they're being scanned.
--
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 #7733: To origin rfc3339
Posted by "ocket8888 (via GitHub)" <gi...@apache.org>.
ocket8888 commented on code in PR #7733:
URL: https://github.com/apache/trafficcontrol/pull/7733#discussion_r1311909834
##########
traffic_ops/traffic_ops_golang/origin/origins.go:
##########
@@ -487,3 +487,294 @@ func deleteQuery() string {
WHERE id=:id`
return query
}
+
+// Get is the handler for GET requests to Origins of APIv5.
+func Get(w http.ResponseWriter, r *http.Request) {
+
+ var useIMS bool
+
+ inf, sysErr, userErr, errCode := api.NewInfo(r, nil, nil)
+ if sysErr != nil || userErr != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+ defer inf.Close()
+
+ origins, userErr, sysErr, errCode, _ := getOrigins(w.Header(), inf.Params, inf.Tx, inf.User, useIMS)
+
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+
+ returnable := make([]tc.OriginV5, len(origins))
+ for i, origin := range origins {
+ returnable[i] = origin.ToOriginV5()
+ }
+
+ api.WriteResp(w, r, returnable)
+ return
+}
+
+// Create Origin with the passed data for APIv5.
+func Create(w http.ResponseWriter, r *http.Request) {
+ inf, sysError, userError, errorCode := api.NewInfo(r, nil, nil)
+ tx := inf.Tx
+ if sysError != nil || userError != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errorCode, userError, sysError)
+ return
+ }
+ defer inf.Close()
+
+ org, errorCode, readValErr := readAndValidateJsonStruct(r, tx)
+ if readValErr != nil {
+ api.HandleErr(w, r, tx.Tx, errorCode, readValErr, nil)
+ return
+ }
+
+ userErr, sysErr, errCode := checkTenancy(&org.TenantID, &org.DeliveryServiceID, tx, inf.User)
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+
+ _, cdnName, _, err := dbhelpers.GetDSNameAndCDNFromID(inf.Tx.Tx, org.DeliveryServiceID)
+ if err != nil {
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, nil, fmt.Errorf("database error: unable to retrieve delivery service name and cdn: %w", err))
+ return
+ }
+ userErr, sysErr, errCode = dbhelpers.CheckIfCurrentUserCanModifyCDN(tx.Tx, string(cdnName), inf.User.UserName)
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+
+ resultRows, err := tx.NamedQuery(insertQuery(), org)
+ if err != nil {
+ usrErr, sysErr, code := api.ParseDBError(err)
+ api.HandleErr(w, r, tx.Tx, code, usrErr, sysErr)
+ return
+ }
+ defer resultRows.Close()
+
+ rowsAffected := 0
+ for resultRows.Next() {
+ rowsAffected++
+ if err := resultRows.Scan(&org.ID, &org.LastUpdated); err != nil {
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, fmt.Errorf("origin create: scanning: %w", err), nil)
+ return
+ }
+ }
+
+ if rowsAffected == 0 {
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, fmt.Errorf("origin create: no rows inserted"), nil)
+ return
+ } else if rowsAffected > 1 {
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, fmt.Errorf("origin create: multiple rows returned"), nil)
+ return
+ }
+
+ alerts := tc.CreateAlerts(tc.SuccessLevel, "origin was created.")
+ w.Header().Set(rfc.Location, fmt.Sprintf("/api/%s/origins?id=%d", inf.Version, org.ID))
+ api.WriteAlertsObj(w, r, http.StatusCreated, alerts, org)
+}
+
+// Update a Origin for APIv5.
+func Update(w http.ResponseWriter, r *http.Request) {
+ inf, sysError, userError, errorCode := api.NewInfo(r, []string{"id"}, []string{"id"})
+ tx := inf.Tx
+ if sysError != nil || userError != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errorCode, userError, sysError)
+ return
+ }
+ defer inf.Close()
+
+ requestedOriginId := inf.IntParams["id"]
+
+ origin, errorCode, readValErr := readAndValidateJsonStruct(r, tx)
+ if readValErr != nil {
+ api.HandleErr(w, r, tx.Tx, errorCode, readValErr, nil)
+ return
+ }
+
+ userErr, sysErr, errCode := checkTenancy(&origin.TenantID, &origin.DeliveryServiceID, tx, inf.User)
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+
+ isPrimary := false
+ ds := 0
+ var existingLastUpdated time.Time
+
+ q := `SELECT is_primary, deliveryservice, last_updated FROM origin WHERE id = $1`
+ errLookup := tx.QueryRow(q, requestedOriginId).Scan(&isPrimary, &ds, &existingLastUpdated)
+ if errLookup != nil {
+ if errors.Is(errLookup, sql.ErrNoRows) {
+ api.HandleErr(w, r, tx.Tx, http.StatusNotFound, fmt.Errorf("no origin exists by id: %d", requestedOriginId), nil)
+ return
+ }
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, nil, fmt.Errorf("database error: %w, when checking if origin with id %d exists", errLookup, requestedOriginId))
+ return
+ }
+ // check if the entity was already updated
+ userErr, sysErr, errCode = api.CheckIfUnModified(r.Header, inf.Tx, requestedOriginId, "origin")
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+
+ if isPrimary && origin.DeliveryServiceID != ds {
+ api.HandleErr(w, r, tx.Tx, http.StatusBadRequest, fmt.Errorf("cannot update the delivery service of a primary origin"), nil)
+ return
+ }
+
+ _, cdnName, _, err := dbhelpers.GetDSNameAndCDNFromID(tx.Tx, origin.DeliveryServiceID)
+ if err != nil {
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, err, nil)
+ return
+ }
+ userErr, sysErr, errCode = dbhelpers.CheckIfCurrentUserCanModifyCDN(tx.Tx, string(cdnName), inf.User.UserName)
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+
+ query := `UPDATE origin SET
+ cachegroup=$1,
+ coordinate=$2,
+ deliveryservice=$3,
+ fqdn=$4,
+ ip6_address=$5,
+ ip_address=$6,
+ name=$7,
+ port=$8,
+ profile=$9,
+ protocol=$10,
+ tenant=$11
+ WHERE id=$12 RETURNING cachegroup, coordinate, deliveryservice, fqdn, ip6_address, ip_address, port, protocol, tenant`
Review Comment:
That actually also has the same problem with everything else that's given as an ID - you're returning the ID and scanning it into the corresponding name field, so `cachegroup`, `coordinate`, `deliveryService`, and `profile` are being set incorrectly as well.
--
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] gbkannan89 commented on a diff in pull request #7733: To origin rfc3339
Posted by "gbkannan89 (via GitHub)" <gi...@apache.org>.
gbkannan89 commented on code in PR #7733:
URL: https://github.com/apache/trafficcontrol/pull/7733#discussion_r1309115171
##########
traffic_ops/testing/api/v5/origins_test.go:
##########
@@ -630,12 +630,14 @@ func GetOriginID(t *testing.T, name string) func() int {
assert.RequireNoError(t, err, "Get Origins Request failed with error:", err)
assert.RequireEqual(t, 1, len(origins.Response), "Expected response object length 1, but got %d", len(origins.Response))
assert.RequireNotNil(t, origins.Response[0].ID, "Expected ID to not be nil.")
- return *origins.Response[0].ID
+ return origins.Response[0].ID
}
}
func CreateTestOrigins(t *testing.T) {
for _, origin := range testData.Origins {
+ origin.TenantID = GetTenantID(t, "root")()
+ origin.DeliveryServiceID = GetDeliveryServiceId(t, origin.DeliveryService)()
Review Comment:
here i have added gettenantID and getdeliveryserviceID as testdata is not having tenant id and dsID so with the name am trying to add the missing data. to testdata
--
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 #7733: To origin rfc3339
Posted by "ocket8888 (via GitHub)" <gi...@apache.org>.
ocket8888 commented on code in PR #7733:
URL: https://github.com/apache/trafficcontrol/pull/7733#discussion_r1303462079
##########
docs/source/api/v5/origins.rst:
##########
@@ -89,7 +89,7 @@ Response Structure
:ip6Address: The IPv6 address of the :term:`Origin`
:ipAddress: The IPv4 address of the :term:`Origin`
:isPrimary: A boolean value which, when ``true`` specifies this :term:`Origin` as the 'primary' :term:`Origin` served by ``deliveryService``
-:lastUpdated: The date and time at which this :term:`Origin` was last modified
+:lastUpdated: The date and time at which this :term:`Origin` was last modified in :rfc:`3339` Format
Review Comment:
"Format" should not be capitalized
##########
lib/go-tc/origins.go:
##########
@@ -54,3 +56,74 @@ type Origin struct {
Tenant *string `json:"tenant" db:"tenant"`
TenantID *int `json:"tenantId" db:"tenant_id"`
}
+
+// OriginsResponseV5 is an alias for the latest minor version of the major version 5.
+type OriginsResponseV5 = OriginsResponseV50
+
+// OriginsResponseV50 is a list of Origins as a response for APIv5.
+type OriginsResponseV50 struct {
+ Response []OriginV5 `json:"response"`
+ Alerts
+}
+
+// OriginDetailResponseV5 is an alias for the latest minor version of the major version 5.
+type OriginDetailResponseV5 = OriginDetailResponseV50
+
+// OriginDetailResponseV50 is the JSON object returned for a single origin in APIv5.
+type OriginDetailResponseV50 struct {
+ Response OriginV5 `json:"response"`
+ Alerts
+}
+
+// OriginV5 is an alias for the latest minor version of the major version 5.
+type OriginV5 = OriginV50
+
+// OriginV50 contains information relating to an Origin, in the latest minor version APIv50.
+type OriginV50 struct {
+ Cachegroup *string `json:"cachegroup" db:"cachegroup"`
+ CachegroupID *int `json:"cachegroupId" db:"cachegroup_id"`
+ Coordinate *string `json:"coordinate" db:"coordinate"`
+ CoordinateID *int `json:"coordinateId" db:"coordinate_id"`
+ DeliveryService *string `json:"deliveryService" db:"deliveryservice"`
+ DeliveryServiceID *int `json:"deliveryServiceId" db:"deliveryservice_id"`
+ FQDN *string `json:"fqdn" db:"fqdn"`
+ ID *int `json:"id" db:"id"`
+ IP6Address *string `json:"ip6Address" db:"ip6_address"`
+ IPAddress *string `json:"ipAddress" db:"ip_address"`
+ IsPrimary *bool `json:"isPrimary" db:"is_primary"`
+ LastUpdated *time.Time `json:"lastUpdated" db:"last_updated"`
+ Name *string `json:"name" db:"name"`
+ Port *int `json:"port" db:"port"`
+ Profile *string `json:"profile" db:"profile"`
+ ProfileID *int `json:"profileId" db:"profile_id"`
+ Protocol *string `json:"protocol" db:"protocol"`
+ Tenant *string `json:"tenant" db:"tenant"`
+ TenantID *int `json:"tenantId" db:"tenant_id"`
Review Comment:
These values are never allowed to be `nil`, so they shouldn't be reference types
##########
lib/go-tc/origins.go:
##########
@@ -54,3 +56,74 @@ type Origin struct {
Tenant *string `json:"tenant" db:"tenant"`
TenantID *int `json:"tenantId" db:"tenant_id"`
}
+
+// OriginsResponseV5 is an alias for the latest minor version of the major version 5.
+type OriginsResponseV5 = OriginsResponseV50
+
+// OriginsResponseV50 is a list of Origins as a response for APIv5.
+type OriginsResponseV50 struct {
+ Response []OriginV5 `json:"response"`
+ Alerts
+}
+
+// OriginDetailResponseV5 is an alias for the latest minor version of the major version 5.
+type OriginDetailResponseV5 = OriginDetailResponseV50
+
+// OriginDetailResponseV50 is the JSON object returned for a single origin in APIv5.
+type OriginDetailResponseV50 struct {
+ Response OriginV5 `json:"response"`
+ Alerts
+}
+
+// OriginV5 is an alias for the latest minor version of the major version 5.
+type OriginV5 = OriginV50
+
+// OriginV50 contains information relating to an Origin, in the latest minor version APIv50.
+type OriginV50 struct {
+ Cachegroup *string `json:"cachegroup" db:"cachegroup"`
+ CachegroupID *int `json:"cachegroupId" db:"cachegroup_id"`
+ Coordinate *string `json:"coordinate" db:"coordinate"`
+ CoordinateID *int `json:"coordinateId" db:"coordinate_id"`
+ DeliveryService *string `json:"deliveryService" db:"deliveryservice"`
+ DeliveryServiceID *int `json:"deliveryServiceId" db:"deliveryservice_id"`
+ FQDN *string `json:"fqdn" db:"fqdn"`
+ ID *int `json:"id" db:"id"`
Review Comment:
These values are never allowed to be `nil`, so they shouldn't be reference types
##########
traffic_ops/traffic_ops_golang/origin/origins.go:
##########
@@ -487,3 +488,217 @@ func deleteQuery() string {
WHERE id=:id`
return query
}
+
+// Get is the handler for GET requests to Origins of APIv5
Review Comment:
GoDoc comment should end with punctuation.
##########
traffic_ops/traffic_ops_golang/origin/origins.go:
##########
@@ -487,3 +488,217 @@ func deleteQuery() string {
WHERE id=:id`
return query
}
+
+// Get is the handler for GET requests to Origins of APIv5
+func Get(w http.ResponseWriter, r *http.Request) {
+
+ var useIMS bool
+
+ inf, sysErr, userErr, errCode := api.NewInfo(r, nil, nil)
+ if sysErr != nil || userErr != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+ defer inf.Close()
+
+ origins, userErr, sysErr, errCode, maxTime := getOrigins(w.Header(), inf.Params, inf.Tx, inf.User, useIMS)
+ var returnable []tc.OriginV5
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+
+ for _, origin := range origins {
+ returnable = append(returnable, origin.ToOriginV5())
+ }
+ _ = maxTime // consider removing maxTime if it's not used in getOrigins in future
+
+ api.WriteResp(w, r, returnable)
+ return
+}
+
+// Create Origin with the passed data for APIv5.
+func Create(w http.ResponseWriter, r *http.Request) {
+ inf, sysError, userError, errorCode := api.NewInfo(r, nil, nil)
+ tx := inf.Tx
+ if sysError != nil || userError != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errorCode, userError, sysError)
+ return
+ }
+ defer inf.Close()
+ org, readValErr := readAndValidateJsonStruct(r)
+ if readValErr != nil {
+ api.HandleErr(w, r, tx.Tx, http.StatusBadRequest, readValErr, nil)
+ return
+ }
+
+ userErr, sysErr, errCode := checkTenancy(org.TenantID, org.DeliveryServiceID, tx, inf.User)
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+
+ _, cdnName, _, err := dbhelpers.GetDSNameAndCDNFromID(inf.Tx.Tx, *org.DeliveryServiceID)
+ if err != nil {
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, fmt.Errorf("Error checking the database for delivery service name and cdn, whether it existed, and any error"), nil)
+ return
+ }
+ userErr, sysErr, errCode = dbhelpers.CheckIfCurrentUserCanModifyCDN(tx.Tx, string(cdnName), inf.User.UserName)
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+
+ resultRows, err := tx.NamedQuery(insertQuery(), org)
+ if err != nil {
+ usrErr, sysErr, code := api.ParseDBError(err)
+ api.HandleErr(w, r, tx.Tx, code, usrErr, sysErr)
+ return
+ }
+ defer resultRows.Close()
+
+ rowsAffected := 0
+ for resultRows.Next() {
+ rowsAffected++
+ if err := resultRows.Scan(&org.ID, &org.LastUpdated); err != nil {
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, fmt.Errorf("origin create: scanning: "+err.Error()), nil)
Review Comment:
constructing an error from another error should use a wrapping construction
##########
traffic_ops/traffic_ops_golang/origin/origins.go:
##########
@@ -487,3 +488,217 @@ func deleteQuery() string {
WHERE id=:id`
return query
}
+
+// Get is the handler for GET requests to Origins of APIv5
+func Get(w http.ResponseWriter, r *http.Request) {
+
+ var useIMS bool
+
+ inf, sysErr, userErr, errCode := api.NewInfo(r, nil, nil)
+ if sysErr != nil || userErr != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+ defer inf.Close()
+
+ origins, userErr, sysErr, errCode, maxTime := getOrigins(w.Header(), inf.Params, inf.Tx, inf.User, useIMS)
+ var returnable []tc.OriginV5
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+
+ for _, origin := range origins {
+ returnable = append(returnable, origin.ToOriginV5())
+ }
+ _ = maxTime // consider removing maxTime if it's not used in getOrigins in future
+
+ api.WriteResp(w, r, returnable)
+ return
+}
+
+// Create Origin with the passed data for APIv5.
+func Create(w http.ResponseWriter, r *http.Request) {
+ inf, sysError, userError, errorCode := api.NewInfo(r, nil, nil)
+ tx := inf.Tx
+ if sysError != nil || userError != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errorCode, userError, sysError)
+ return
+ }
+ defer inf.Close()
+ org, readValErr := readAndValidateJsonStruct(r)
+ if readValErr != nil {
+ api.HandleErr(w, r, tx.Tx, http.StatusBadRequest, readValErr, nil)
+ return
+ }
+
+ userErr, sysErr, errCode := checkTenancy(org.TenantID, org.DeliveryServiceID, tx, inf.User)
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+
+ _, cdnName, _, err := dbhelpers.GetDSNameAndCDNFromID(inf.Tx.Tx, *org.DeliveryServiceID)
+ if err != nil {
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, fmt.Errorf("Error checking the database for delivery service name and cdn, whether it existed, and any error"), nil)
+ return
+ }
+ userErr, sysErr, errCode = dbhelpers.CheckIfCurrentUserCanModifyCDN(tx.Tx, string(cdnName), inf.User.UserName)
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+
+ resultRows, err := tx.NamedQuery(insertQuery(), org)
+ if err != nil {
+ usrErr, sysErr, code := api.ParseDBError(err)
+ api.HandleErr(w, r, tx.Tx, code, usrErr, sysErr)
+ return
+ }
+ defer resultRows.Close()
+
+ rowsAffected := 0
+ for resultRows.Next() {
+ rowsAffected++
+ if err := resultRows.Scan(&org.ID, &org.LastUpdated); err != nil {
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, fmt.Errorf("origin create: scanning: "+err.Error()), nil)
+ return
+ }
+ }
+ if rowsAffected == 0 {
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, fmt.Errorf("origin create: no rows returned"), nil)
+ return
+ } else if rowsAffected > 1 {
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, fmt.Errorf("origin create: multiple rows returned"), nil)
+ return
+ }
+
+ alerts := tc.CreateAlerts(tc.SuccessLevel, "origin was created.")
+ api.WriteAlertsObj(w, r, http.StatusCreated, alerts, org)
+}
+
+// Update a Origin for APIv5.
+func Update(w http.ResponseWriter, r *http.Request) {
+ inf, sysError, userError, errorCode := api.NewInfo(r, []string{"id"}, []string{"id"})
+ tx := inf.Tx
+ if sysError != nil || userError != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errorCode, userError, sysError)
+ return
+ }
+ defer inf.Close()
+
+ origin, readValErr := readAndValidateJsonStruct(r)
+ if readValErr != nil {
+ api.HandleErr(w, r, tx.Tx, http.StatusBadRequest, readValErr, nil)
+ return
+ }
+ userErr, sysErr, errCode := checkTenancy(origin.TenantID, origin.DeliveryServiceID, tx, inf.User)
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+
+ isPrimary := false
+ ds := 0
+ var existingLastUpdated time.Time
+
+ requestedOriginId := inf.IntParams["id"]
+
+ q := `SELECT is_primary, deliveryservice, last_updated FROM origin WHERE id = $1`
+ if err := tx.QueryRow(q, requestedOriginId).Scan(&isPrimary, &ds, &existingLastUpdated); err != nil {
+ if err == sql.ErrNoRows {
Review Comment:
Error identity checking should use [`errors.Is`](pkg.go.dev/errors#Is)
##########
traffic_ops/traffic_ops_golang/origin/origins.go:
##########
@@ -487,3 +488,217 @@ func deleteQuery() string {
WHERE id=:id`
return query
}
+
+// Get is the handler for GET requests to Origins of APIv5
+func Get(w http.ResponseWriter, r *http.Request) {
+
+ var useIMS bool
+
+ inf, sysErr, userErr, errCode := api.NewInfo(r, nil, nil)
+ if sysErr != nil || userErr != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+ defer inf.Close()
+
+ origins, userErr, sysErr, errCode, maxTime := getOrigins(w.Header(), inf.Params, inf.Tx, inf.User, useIMS)
+ var returnable []tc.OriginV5
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+
+ for _, origin := range origins {
+ returnable = append(returnable, origin.ToOriginV5())
+ }
+ _ = maxTime // consider removing maxTime if it's not used in getOrigins in future
+
+ api.WriteResp(w, r, returnable)
+ return
+}
+
+// Create Origin with the passed data for APIv5.
+func Create(w http.ResponseWriter, r *http.Request) {
+ inf, sysError, userError, errorCode := api.NewInfo(r, nil, nil)
+ tx := inf.Tx
+ if sysError != nil || userError != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errorCode, userError, sysError)
+ return
+ }
+ defer inf.Close()
+ org, readValErr := readAndValidateJsonStruct(r)
+ if readValErr != nil {
+ api.HandleErr(w, r, tx.Tx, http.StatusBadRequest, readValErr, nil)
+ return
+ }
+
+ userErr, sysErr, errCode := checkTenancy(org.TenantID, org.DeliveryServiceID, tx, inf.User)
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+
+ _, cdnName, _, err := dbhelpers.GetDSNameAndCDNFromID(inf.Tx.Tx, *org.DeliveryServiceID)
+ if err != nil {
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, fmt.Errorf("Error checking the database for delivery service name and cdn, whether it existed, and any error"), nil)
+ return
+ }
+ userErr, sysErr, errCode = dbhelpers.CheckIfCurrentUserCanModifyCDN(tx.Tx, string(cdnName), inf.User.UserName)
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+
+ resultRows, err := tx.NamedQuery(insertQuery(), org)
+ if err != nil {
+ usrErr, sysErr, code := api.ParseDBError(err)
+ api.HandleErr(w, r, tx.Tx, code, usrErr, sysErr)
+ return
+ }
+ defer resultRows.Close()
+
+ rowsAffected := 0
+ for resultRows.Next() {
+ rowsAffected++
+ if err := resultRows.Scan(&org.ID, &org.LastUpdated); err != nil {
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, fmt.Errorf("origin create: scanning: "+err.Error()), nil)
+ return
+ }
+ }
+ if rowsAffected == 0 {
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, fmt.Errorf("origin create: no rows returned"), nil)
+ return
+ } else if rowsAffected > 1 {
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, fmt.Errorf("origin create: multiple rows returned"), nil)
+ return
+ }
+
+ alerts := tc.CreateAlerts(tc.SuccessLevel, "origin was created.")
+ api.WriteAlertsObj(w, r, http.StatusCreated, alerts, org)
+}
+
+// Update a Origin for APIv5.
+func Update(w http.ResponseWriter, r *http.Request) {
+ inf, sysError, userError, errorCode := api.NewInfo(r, []string{"id"}, []string{"id"})
+ tx := inf.Tx
+ if sysError != nil || userError != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errorCode, userError, sysError)
+ return
+ }
+ defer inf.Close()
+
+ origin, readValErr := readAndValidateJsonStruct(r)
+ if readValErr != nil {
+ api.HandleErr(w, r, tx.Tx, http.StatusBadRequest, readValErr, nil)
+ return
+ }
+ userErr, sysErr, errCode := checkTenancy(origin.TenantID, origin.DeliveryServiceID, tx, inf.User)
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+
+ isPrimary := false
+ ds := 0
+ var existingLastUpdated time.Time
+
+ requestedOriginId := inf.IntParams["id"]
+
+ q := `SELECT is_primary, deliveryservice, last_updated FROM origin WHERE id = $1`
+ if err := tx.QueryRow(q, requestedOriginId).Scan(&isPrimary, &ds, &existingLastUpdated); err != nil {
+ if err == sql.ErrNoRows {
+ api.HandleErr(w, r, tx.Tx, http.StatusNotFound, fmt.Errorf("origin not found"), nil)
+ return
+ }
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, nil, fmt.Errorf("error: %s, when checking if origin with id %s exists", err.Error(), requestedOriginId))
Review Comment:
Constructing an error from an error should use a wrapping construction.
##########
traffic_ops/traffic_ops_golang/origin/origins.go:
##########
@@ -487,3 +488,217 @@ func deleteQuery() string {
WHERE id=:id`
return query
}
+
+// Get is the handler for GET requests to Origins of APIv5
+func Get(w http.ResponseWriter, r *http.Request) {
+
+ var useIMS bool
+
+ inf, sysErr, userErr, errCode := api.NewInfo(r, nil, nil)
+ if sysErr != nil || userErr != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+ defer inf.Close()
+
+ origins, userErr, sysErr, errCode, maxTime := getOrigins(w.Header(), inf.Params, inf.Tx, inf.User, useIMS)
+ var returnable []tc.OriginV5
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+
+ for _, origin := range origins {
+ returnable = append(returnable, origin.ToOriginV5())
+ }
+ _ = maxTime // consider removing maxTime if it's not used in getOrigins in future
Review Comment:
can we just do that right now?
##########
traffic_ops/traffic_ops_golang/origin/origins.go:
##########
@@ -487,3 +488,217 @@ func deleteQuery() string {
WHERE id=:id`
return query
}
+
+// Get is the handler for GET requests to Origins of APIv5
+func Get(w http.ResponseWriter, r *http.Request) {
+
+ var useIMS bool
+
+ inf, sysErr, userErr, errCode := api.NewInfo(r, nil, nil)
+ if sysErr != nil || userErr != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+ defer inf.Close()
+
+ origins, userErr, sysErr, errCode, maxTime := getOrigins(w.Header(), inf.Params, inf.Tx, inf.User, useIMS)
+ var returnable []tc.OriginV5
Review Comment:
If zero origins match the filtering parameters sent by the client, this will cause it to return a `null` `response` property, when the proper representation for an empty list is the empty array, i.e. `[]`.
##########
lib/go-tc/origins.go:
##########
@@ -54,3 +56,74 @@ type Origin struct {
Tenant *string `json:"tenant" db:"tenant"`
TenantID *int `json:"tenantId" db:"tenant_id"`
}
+
+// OriginsResponseV5 is an alias for the latest minor version of the major version 5.
+type OriginsResponseV5 = OriginsResponseV50
+
+// OriginsResponseV50 is a list of Origins as a response for APIv5.
+type OriginsResponseV50 struct {
+ Response []OriginV5 `json:"response"`
+ Alerts
+}
+
+// OriginDetailResponseV5 is an alias for the latest minor version of the major version 5.
+type OriginDetailResponseV5 = OriginDetailResponseV50
+
+// OriginDetailResponseV50 is the JSON object returned for a single origin in APIv5.
+type OriginDetailResponseV50 struct {
+ Response OriginV5 `json:"response"`
+ Alerts
+}
+
+// OriginV5 is an alias for the latest minor version of the major version 5.
+type OriginV5 = OriginV50
+
+// OriginV50 contains information relating to an Origin, in the latest minor version APIv50.
+type OriginV50 struct {
+ Cachegroup *string `json:"cachegroup" db:"cachegroup"`
+ CachegroupID *int `json:"cachegroupId" db:"cachegroup_id"`
+ Coordinate *string `json:"coordinate" db:"coordinate"`
+ CoordinateID *int `json:"coordinateId" db:"coordinate_id"`
+ DeliveryService *string `json:"deliveryService" db:"deliveryservice"`
+ DeliveryServiceID *int `json:"deliveryServiceId" db:"deliveryservice_id"`
+ FQDN *string `json:"fqdn" db:"fqdn"`
+ ID *int `json:"id" db:"id"`
+ IP6Address *string `json:"ip6Address" db:"ip6_address"`
+ IPAddress *string `json:"ipAddress" db:"ip_address"`
+ IsPrimary *bool `json:"isPrimary" db:"is_primary"`
+ LastUpdated *time.Time `json:"lastUpdated" db:"last_updated"`
+ Name *string `json:"name" db:"name"`
Review Comment:
These values are never allowed to be `nil`, so they shouldn't be reference types
##########
traffic_ops/traffic_ops_golang/origin/origins.go:
##########
@@ -487,3 +488,217 @@ func deleteQuery() string {
WHERE id=:id`
return query
}
+
+// Get is the handler for GET requests to Origins of APIv5
+func Get(w http.ResponseWriter, r *http.Request) {
+
+ var useIMS bool
+
+ inf, sysErr, userErr, errCode := api.NewInfo(r, nil, nil)
+ if sysErr != nil || userErr != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+ defer inf.Close()
+
+ origins, userErr, sysErr, errCode, maxTime := getOrigins(w.Header(), inf.Params, inf.Tx, inf.User, useIMS)
+ var returnable []tc.OriginV5
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+
+ for _, origin := range origins {
+ returnable = append(returnable, origin.ToOriginV5())
+ }
+ _ = maxTime // consider removing maxTime if it's not used in getOrigins in future
+
+ api.WriteResp(w, r, returnable)
+ return
+}
+
+// Create Origin with the passed data for APIv5.
+func Create(w http.ResponseWriter, r *http.Request) {
+ inf, sysError, userError, errorCode := api.NewInfo(r, nil, nil)
+ tx := inf.Tx
+ if sysError != nil || userError != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errorCode, userError, sysError)
+ return
+ }
+ defer inf.Close()
+ org, readValErr := readAndValidateJsonStruct(r)
+ if readValErr != nil {
+ api.HandleErr(w, r, tx.Tx, http.StatusBadRequest, readValErr, nil)
+ return
+ }
+
+ userErr, sysErr, errCode := checkTenancy(org.TenantID, org.DeliveryServiceID, tx, inf.User)
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+
+ _, cdnName, _, err := dbhelpers.GetDSNameAndCDNFromID(inf.Tx.Tx, *org.DeliveryServiceID)
+ if err != nil {
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, fmt.Errorf("Error checking the database for delivery service name and cdn, whether it existed, and any error"), nil)
Review Comment:
error messages should not begin with a capitalized word as though they were the beginning of a sentence, and they really shouldn't start with the word "error" at all, since that's implied by the fact that it's literally an `error`. Also, this message tells me nothing about what actually happened; all of that information is carried by `err`, which you aren't using.
##########
traffic_ops/traffic_ops_golang/origin/origins.go:
##########
@@ -487,3 +488,217 @@ func deleteQuery() string {
WHERE id=:id`
return query
}
+
+// Get is the handler for GET requests to Origins of APIv5
+func Get(w http.ResponseWriter, r *http.Request) {
+
+ var useIMS bool
+
+ inf, sysErr, userErr, errCode := api.NewInfo(r, nil, nil)
+ if sysErr != nil || userErr != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+ defer inf.Close()
+
+ origins, userErr, sysErr, errCode, maxTime := getOrigins(w.Header(), inf.Params, inf.Tx, inf.User, useIMS)
+ var returnable []tc.OriginV5
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+
+ for _, origin := range origins {
+ returnable = append(returnable, origin.ToOriginV5())
+ }
+ _ = maxTime // consider removing maxTime if it's not used in getOrigins in future
+
+ api.WriteResp(w, r, returnable)
+ return
+}
+
+// Create Origin with the passed data for APIv5.
+func Create(w http.ResponseWriter, r *http.Request) {
+ inf, sysError, userError, errorCode := api.NewInfo(r, nil, nil)
+ tx := inf.Tx
+ if sysError != nil || userError != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errorCode, userError, sysError)
+ return
+ }
+ defer inf.Close()
+ org, readValErr := readAndValidateJsonStruct(r)
+ if readValErr != nil {
+ api.HandleErr(w, r, tx.Tx, http.StatusBadRequest, readValErr, nil)
+ return
+ }
+
+ userErr, sysErr, errCode := checkTenancy(org.TenantID, org.DeliveryServiceID, tx, inf.User)
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+
+ _, cdnName, _, err := dbhelpers.GetDSNameAndCDNFromID(inf.Tx.Tx, *org.DeliveryServiceID)
+ if err != nil {
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, fmt.Errorf("Error checking the database for delivery service name and cdn, whether it existed, and any error"), nil)
+ return
+ }
+ userErr, sysErr, errCode = dbhelpers.CheckIfCurrentUserCanModifyCDN(tx.Tx, string(cdnName), inf.User.UserName)
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+
+ resultRows, err := tx.NamedQuery(insertQuery(), org)
+ if err != nil {
+ usrErr, sysErr, code := api.ParseDBError(err)
+ api.HandleErr(w, r, tx.Tx, code, usrErr, sysErr)
+ return
+ }
+ defer resultRows.Close()
+
+ rowsAffected := 0
+ for resultRows.Next() {
+ rowsAffected++
+ if err := resultRows.Scan(&org.ID, &org.LastUpdated); err != nil {
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, fmt.Errorf("origin create: scanning: "+err.Error()), nil)
+ return
+ }
+ }
+ if rowsAffected == 0 {
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, fmt.Errorf("origin create: no rows returned"), nil)
+ return
+ } else if rowsAffected > 1 {
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, fmt.Errorf("origin create: multiple rows returned"), nil)
+ return
+ }
+
+ alerts := tc.CreateAlerts(tc.SuccessLevel, "origin was created.")
+ api.WriteAlertsObj(w, r, http.StatusCreated, alerts, org)
Review Comment:
A `201 Created` response MUST be accompanied by a `Location` header which gives the URL to which the client may send a GET request to receive a representation of the created object.
--
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 #7733: To origin rfc3339
Posted by "ocket8888 (via GitHub)" <gi...@apache.org>.
ocket8888 commented on code in PR #7733:
URL: https://github.com/apache/trafficcontrol/pull/7733#discussion_r1311902118
##########
traffic_ops/traffic_ops_golang/origin/origins.go:
##########
@@ -487,3 +487,294 @@ func deleteQuery() string {
WHERE id=:id`
return query
}
+
+// Get is the handler for GET requests to Origins of APIv5.
+func Get(w http.ResponseWriter, r *http.Request) {
+
+ var useIMS bool
+
+ inf, sysErr, userErr, errCode := api.NewInfo(r, nil, nil)
+ if sysErr != nil || userErr != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+ defer inf.Close()
+
+ origins, userErr, sysErr, errCode, _ := getOrigins(w.Header(), inf.Params, inf.Tx, inf.User, useIMS)
+
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+
+ returnable := make([]tc.OriginV5, len(origins))
+ for i, origin := range origins {
+ returnable[i] = origin.ToOriginV5()
+ }
+
+ api.WriteResp(w, r, returnable)
+ return
+}
+
+// Create Origin with the passed data for APIv5.
+func Create(w http.ResponseWriter, r *http.Request) {
+ inf, sysError, userError, errorCode := api.NewInfo(r, nil, nil)
+ tx := inf.Tx
+ if sysError != nil || userError != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errorCode, userError, sysError)
+ return
+ }
+ defer inf.Close()
+
+ org, errorCode, readValErr := readAndValidateJsonStruct(r, tx)
+ if readValErr != nil {
+ api.HandleErr(w, r, tx.Tx, errorCode, readValErr, nil)
+ return
+ }
+
+ userErr, sysErr, errCode := checkTenancy(&org.TenantID, &org.DeliveryServiceID, tx, inf.User)
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+
+ _, cdnName, _, err := dbhelpers.GetDSNameAndCDNFromID(inf.Tx.Tx, org.DeliveryServiceID)
+ if err != nil {
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, nil, fmt.Errorf("database error: unable to retrieve delivery service name and cdn: %w", err))
+ return
+ }
+ userErr, sysErr, errCode = dbhelpers.CheckIfCurrentUserCanModifyCDN(tx.Tx, string(cdnName), inf.User.UserName)
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+
+ resultRows, err := tx.NamedQuery(insertQuery(), org)
+ if err != nil {
+ usrErr, sysErr, code := api.ParseDBError(err)
+ api.HandleErr(w, r, tx.Tx, code, usrErr, sysErr)
+ return
+ }
+ defer resultRows.Close()
+
+ rowsAffected := 0
+ for resultRows.Next() {
+ rowsAffected++
+ if err := resultRows.Scan(&org.ID, &org.LastUpdated); err != nil {
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, fmt.Errorf("origin create: scanning: %w", err), nil)
+ return
+ }
+ }
+
+ if rowsAffected == 0 {
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, fmt.Errorf("origin create: no rows inserted"), nil)
+ return
+ } else if rowsAffected > 1 {
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, fmt.Errorf("origin create: multiple rows returned"), nil)
+ return
+ }
+
+ alerts := tc.CreateAlerts(tc.SuccessLevel, "origin was created.")
+ w.Header().Set(rfc.Location, fmt.Sprintf("/api/%s/origins?id=%d", inf.Version, org.ID))
+ api.WriteAlertsObj(w, r, http.StatusCreated, alerts, org)
+}
+
+// Update a Origin for APIv5.
+func Update(w http.ResponseWriter, r *http.Request) {
+ inf, sysError, userError, errorCode := api.NewInfo(r, []string{"id"}, []string{"id"})
+ tx := inf.Tx
+ if sysError != nil || userError != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errorCode, userError, sysError)
+ return
+ }
+ defer inf.Close()
+
+ requestedOriginId := inf.IntParams["id"]
+
+ origin, errorCode, readValErr := readAndValidateJsonStruct(r, tx)
+ if readValErr != nil {
+ api.HandleErr(w, r, tx.Tx, errorCode, readValErr, nil)
+ return
+ }
+
+ userErr, sysErr, errCode := checkTenancy(&origin.TenantID, &origin.DeliveryServiceID, tx, inf.User)
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+
+ isPrimary := false
+ ds := 0
+ var existingLastUpdated time.Time
+
+ q := `SELECT is_primary, deliveryservice, last_updated FROM origin WHERE id = $1`
+ errLookup := tx.QueryRow(q, requestedOriginId).Scan(&isPrimary, &ds, &existingLastUpdated)
+ if errLookup != nil {
+ if errors.Is(errLookup, sql.ErrNoRows) {
+ api.HandleErr(w, r, tx.Tx, http.StatusNotFound, fmt.Errorf("no origin exists by id: %d", requestedOriginId), nil)
+ return
+ }
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, nil, fmt.Errorf("database error: %w, when checking if origin with id %d exists", errLookup, requestedOriginId))
+ return
+ }
+ // check if the entity was already updated
+ userErr, sysErr, errCode = api.CheckIfUnModified(r.Header, inf.Tx, requestedOriginId, "origin")
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+
+ if isPrimary && origin.DeliveryServiceID != ds {
+ api.HandleErr(w, r, tx.Tx, http.StatusBadRequest, fmt.Errorf("cannot update the delivery service of a primary origin"), nil)
+ return
+ }
+
+ _, cdnName, _, err := dbhelpers.GetDSNameAndCDNFromID(tx.Tx, origin.DeliveryServiceID)
+ if err != nil {
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, err, nil)
+ return
+ }
+ userErr, sysErr, errCode = dbhelpers.CheckIfCurrentUserCanModifyCDN(tx.Tx, string(cdnName), inf.User.UserName)
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+
+ query := `UPDATE origin SET
+ cachegroup=$1,
+ coordinate=$2,
+ deliveryservice=$3,
+ fqdn=$4,
+ ip6_address=$5,
+ ip_address=$6,
+ name=$7,
+ port=$8,
+ profile=$9,
+ protocol=$10,
+ tenant=$11
+ WHERE id=$12 RETURNING cachegroup, coordinate, deliveryservice, fqdn, ip6_address, ip_address, port, protocol, tenant`
Review Comment:
you're returning `tenant` and then scanning that into `origin.Tenant` - but that's actually the Tenant's _**ID**_. So you wind up with:
```
"tenant": "2",
"tenantId": 2
```
(that Tenant's Name is _not_ "2").
##########
traffic_ops/traffic_ops_golang/origin/origins.go:
##########
@@ -21,23 +21,23 @@ package origin
import (
"database/sql"
+ "encoding/json"
"errors"
"fmt"
"net/http"
"strconv"
"time"
- "github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/util/ims"
-
"github.com/apache/trafficcontrol/lib/go-log"
+ "github.com/apache/trafficcontrol/lib/go-rfc"
"github.com/apache/trafficcontrol/lib/go-tc"
"github.com/apache/trafficcontrol/lib/go-tc/tovalidate"
"github.com/apache/trafficcontrol/lib/go-util"
"github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/api"
"github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/auth"
"github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/dbhelpers"
"github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/tenant"
-
+ "github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/util/ims"
Review Comment:
there should be a blank line between the project's internal imports and 3<sup>rd</sup> party library imports
##########
traffic_ops/traffic_ops_golang/origin/origins.go:
##########
@@ -487,3 +487,294 @@ func deleteQuery() string {
WHERE id=:id`
return query
}
+
+// Get is the handler for GET requests to Origins of APIv5.
+func Get(w http.ResponseWriter, r *http.Request) {
+
+ var useIMS bool
+
+ inf, sysErr, userErr, errCode := api.NewInfo(r, nil, nil)
+ if sysErr != nil || userErr != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+ defer inf.Close()
+
+ origins, userErr, sysErr, errCode, _ := getOrigins(w.Header(), inf.Params, inf.Tx, inf.User, useIMS)
+
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+
+ returnable := make([]tc.OriginV5, len(origins))
+ for i, origin := range origins {
+ returnable[i] = origin.ToOriginV5()
+ }
+
+ api.WriteResp(w, r, returnable)
+ return
+}
+
+// Create Origin with the passed data for APIv5.
+func Create(w http.ResponseWriter, r *http.Request) {
+ inf, sysError, userError, errorCode := api.NewInfo(r, nil, nil)
+ tx := inf.Tx
+ if sysError != nil || userError != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errorCode, userError, sysError)
+ return
+ }
+ defer inf.Close()
+
+ org, errorCode, readValErr := readAndValidateJsonStruct(r, tx)
+ if readValErr != nil {
+ api.HandleErr(w, r, tx.Tx, errorCode, readValErr, nil)
+ return
+ }
+
+ userErr, sysErr, errCode := checkTenancy(&org.TenantID, &org.DeliveryServiceID, tx, inf.User)
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+
+ _, cdnName, _, err := dbhelpers.GetDSNameAndCDNFromID(inf.Tx.Tx, org.DeliveryServiceID)
+ if err != nil {
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, nil, fmt.Errorf("database error: unable to retrieve delivery service name and cdn: %w", err))
+ return
+ }
+ userErr, sysErr, errCode = dbhelpers.CheckIfCurrentUserCanModifyCDN(tx.Tx, string(cdnName), inf.User.UserName)
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+
+ resultRows, err := tx.NamedQuery(insertQuery(), org)
+ if err != nil {
+ usrErr, sysErr, code := api.ParseDBError(err)
+ api.HandleErr(w, r, tx.Tx, code, usrErr, sysErr)
+ return
+ }
+ defer resultRows.Close()
+
+ rowsAffected := 0
+ for resultRows.Next() {
+ rowsAffected++
+ if err := resultRows.Scan(&org.ID, &org.LastUpdated); err != nil {
Review Comment:
You're not setting the bound names according to their IDs, so it's not populating `cachegroup`, `coordinate`, `deliveryService`, `profile`, or `tenant`. It reports those as whatever their "zero" values are: `null` for pointers, `""` for `string`s.
##########
traffic_ops/traffic_ops_golang/origin/origins.go:
##########
@@ -487,3 +487,294 @@ func deleteQuery() string {
WHERE id=:id`
return query
}
+
+// Get is the handler for GET requests to Origins of APIv5.
+func Get(w http.ResponseWriter, r *http.Request) {
+
+ var useIMS bool
+
+ inf, sysErr, userErr, errCode := api.NewInfo(r, nil, nil)
+ if sysErr != nil || userErr != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+ defer inf.Close()
+
+ origins, userErr, sysErr, errCode, _ := getOrigins(w.Header(), inf.Params, inf.Tx, inf.User, useIMS)
+
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+
+ returnable := make([]tc.OriginV5, len(origins))
+ for i, origin := range origins {
+ returnable[i] = origin.ToOriginV5()
+ }
+
+ api.WriteResp(w, r, returnable)
+ return
+}
+
+// Create Origin with the passed data for APIv5.
+func Create(w http.ResponseWriter, r *http.Request) {
+ inf, sysError, userError, errorCode := api.NewInfo(r, nil, nil)
+ tx := inf.Tx
+ if sysError != nil || userError != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errorCode, userError, sysError)
+ return
+ }
+ defer inf.Close()
+
+ org, errorCode, readValErr := readAndValidateJsonStruct(r, tx)
+ if readValErr != nil {
+ api.HandleErr(w, r, tx.Tx, errorCode, readValErr, nil)
+ return
+ }
+
+ userErr, sysErr, errCode := checkTenancy(&org.TenantID, &org.DeliveryServiceID, tx, inf.User)
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+
+ _, cdnName, _, err := dbhelpers.GetDSNameAndCDNFromID(inf.Tx.Tx, org.DeliveryServiceID)
+ if err != nil {
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, nil, fmt.Errorf("database error: unable to retrieve delivery service name and cdn: %w", err))
+ return
+ }
+ userErr, sysErr, errCode = dbhelpers.CheckIfCurrentUserCanModifyCDN(tx.Tx, string(cdnName), inf.User.UserName)
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+
+ resultRows, err := tx.NamedQuery(insertQuery(), org)
+ if err != nil {
+ usrErr, sysErr, code := api.ParseDBError(err)
+ api.HandleErr(w, r, tx.Tx, code, usrErr, sysErr)
+ return
+ }
+ defer resultRows.Close()
+
+ rowsAffected := 0
+ for resultRows.Next() {
+ rowsAffected++
+ if err := resultRows.Scan(&org.ID, &org.LastUpdated); err != nil {
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, fmt.Errorf("origin create: scanning: %w", err), nil)
+ return
+ }
+ }
+
+ if rowsAffected == 0 {
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, fmt.Errorf("origin create: no rows inserted"), nil)
+ return
+ } else if rowsAffected > 1 {
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, fmt.Errorf("origin create: multiple rows returned"), nil)
+ return
+ }
+
+ alerts := tc.CreateAlerts(tc.SuccessLevel, "origin was created.")
+ w.Header().Set(rfc.Location, fmt.Sprintf("/api/%s/origins?id=%d", inf.Version, org.ID))
+ api.WriteAlertsObj(w, r, http.StatusCreated, alerts, org)
+}
+
+// Update a Origin for APIv5.
+func Update(w http.ResponseWriter, r *http.Request) {
+ inf, sysError, userError, errorCode := api.NewInfo(r, []string{"id"}, []string{"id"})
+ tx := inf.Tx
+ if sysError != nil || userError != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errorCode, userError, sysError)
+ return
+ }
+ defer inf.Close()
+
+ requestedOriginId := inf.IntParams["id"]
+
+ origin, errorCode, readValErr := readAndValidateJsonStruct(r, tx)
+ if readValErr != nil {
+ api.HandleErr(w, r, tx.Tx, errorCode, readValErr, nil)
+ return
+ }
+
+ userErr, sysErr, errCode := checkTenancy(&origin.TenantID, &origin.DeliveryServiceID, tx, inf.User)
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+
+ isPrimary := false
+ ds := 0
+ var existingLastUpdated time.Time
+
+ q := `SELECT is_primary, deliveryservice, last_updated FROM origin WHERE id = $1`
+ errLookup := tx.QueryRow(q, requestedOriginId).Scan(&isPrimary, &ds, &existingLastUpdated)
+ if errLookup != nil {
+ if errors.Is(errLookup, sql.ErrNoRows) {
+ api.HandleErr(w, r, tx.Tx, http.StatusNotFound, fmt.Errorf("no origin exists by id: %d", requestedOriginId), nil)
+ return
+ }
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, nil, fmt.Errorf("database error: %w, when checking if origin with id %d exists", errLookup, requestedOriginId))
+ return
+ }
+ // check if the entity was already updated
+ userErr, sysErr, errCode = api.CheckIfUnModified(r.Header, inf.Tx, requestedOriginId, "origin")
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+
+ if isPrimary && origin.DeliveryServiceID != ds {
+ api.HandleErr(w, r, tx.Tx, http.StatusBadRequest, fmt.Errorf("cannot update the delivery service of a primary origin"), nil)
+ return
+ }
+
+ _, cdnName, _, err := dbhelpers.GetDSNameAndCDNFromID(tx.Tx, origin.DeliveryServiceID)
+ if err != nil {
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, err, nil)
+ return
+ }
+ userErr, sysErr, errCode = dbhelpers.CheckIfCurrentUserCanModifyCDN(tx.Tx, string(cdnName), inf.User.UserName)
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+
+ query := `UPDATE origin SET
+ cachegroup=$1,
+ coordinate=$2,
+ deliveryservice=$3,
+ fqdn=$4,
+ ip6_address=$5,
+ ip_address=$6,
+ name=$7,
+ port=$8,
+ profile=$9,
+ protocol=$10,
+ tenant=$11
+ WHERE id=$12 RETURNING cachegroup, coordinate, deliveryservice, fqdn, ip6_address, ip_address, port, protocol, tenant`
+ errUpdate := tx.QueryRow(query, origin.CachegroupID, origin.CoordinateID, origin.DeliveryServiceID,
+ origin.FQDN, origin.IP6Address, origin.IPAddress, origin.Name,
+ origin.Port, origin.ProfileID, origin.Protocol, origin.TenantID, requestedOriginId).Scan(&origin.Cachegroup, &origin.Coordinate, &origin.DeliveryService,
+ &origin.FQDN, &origin.IP6Address, &origin.IPAddress,
+ &origin.Port, &origin.Protocol, &origin.Tenant)
+ if errUpdate != nil {
+ if errors.Is(errUpdate, sql.ErrNoRows) {
+ api.HandleErr(w, r, tx.Tx, http.StatusNotFound, fmt.Errorf("origin: %d not found", requestedOriginId), nil)
+ return
+ }
+ usrErr, sysErr, code := api.ParseDBError(errUpdate)
+ api.HandleErr(w, r, tx.Tx, code, usrErr, sysErr)
+ return
+ }
+
+ origin.ID = requestedOriginId
+ alerts := tc.CreateAlerts(tc.SuccessLevel, "origin was updated.")
+ api.WriteAlertsObj(w, r, http.StatusOK, alerts, origin)
+ return
+}
+
+// Delete an Origin for APIv5.
+func Delete(w http.ResponseWriter, r *http.Request) {
+ inf, userErr, sysErr, errCode := api.NewInfo(r, []string{"id"}, []string{"id"})
+ tx := inf.Tx.Tx
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, tx, errCode, userErr, sysErr)
+ return
+ }
+ defer inf.Close()
+
+ id := inf.IntParams["id"]
+
+ var origin tc.OriginV5
+ if err := tx.QueryRow(`SELECT is_primary, deliveryservice, tenant FROM origin WHERE id = $1`, id).Scan(&origin.IsPrimary, &origin.DeliveryServiceID, &origin.TenantID); err != nil {
+ if errors.Is(err, sql.ErrNoRows) {
+ api.HandleErr(w, r, tx, http.StatusNotFound, fmt.Errorf("no origin exists by id: %d", id), nil)
+ return
+ }
+ api.HandleErr(w, r, tx, http.StatusInternalServerError, fmt.Errorf("origin delete: is_primary scanning: %w", err), nil)
+ return
+ }
+
+ if origin.IsPrimary {
+ api.HandleErr(w, r, tx, http.StatusBadRequest, fmt.Errorf("cannot delete a primary origin"), nil)
+ return
+ }
+
+ if &origin.DeliveryServiceID != nil {
+ _, cdnName, _, err := dbhelpers.GetDSNameAndCDNFromID(tx, origin.DeliveryServiceID)
+ if err != nil {
+ api.HandleErr(w, r, tx, http.StatusInternalServerError, err, nil)
+ return
+ }
+
+ userErr, sysErr, errCode := dbhelpers.CheckIfCurrentUserCanModifyCDN(tx, string(cdnName), inf.User.UserName)
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, tx, errCode, userErr, sysErr)
+ return
+ }
+ }
+
+ userErr, sysErr, errCode = checkTenancy(&origin.TenantID, &origin.DeliveryServiceID, inf.Tx, inf.User)
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, tx, errCode, userErr, sysErr)
+ return
+ }
+
+ res, err := tx.Exec("DELETE FROM origin WHERE id=$1", id)
+ 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("origin delete: getting rows affected: %w", err))
+ return
+ }
+ if rowsAffected == 0 {
+ api.HandleErr(w, r, tx, http.StatusInternalServerError, fmt.Errorf("no rows deleted for origin"), nil)
+ return
+ }
+ if rowsAffected != 1 {
+ api.HandleErr(w, r, tx, http.StatusInternalServerError, fmt.Errorf("origin delete: multiple rows affected"), nil)
+ return
+ }
+
+ alerts := tc.CreateAlerts(tc.SuccessLevel, "origin was deleted.")
+ api.WriteAlerts(w, r, http.StatusOK, alerts)
+ return
+}
+
+// readAndValidateJsonStruct reads json body and validates json fields.
+func readAndValidateJsonStruct(r *http.Request, tx *sqlx.Tx) (tc.OriginV5, int, error) {
+ var origin tc.OriginV5
+ if err := json.NewDecoder(r.Body).Decode(&origin); err != nil {
+ userErr := fmt.Errorf("error decoding POST request body into OriginV5 struct %w", err)
+ return origin, http.StatusBadRequest, userErr
+ }
+
+ noSpaces := validation.NewStringRule(tovalidate.NoSpaces, "cannot contain spaces")
+ validProtocol := validation.NewStringRule(tovalidate.IsOneOfStringICase("http", "https"), "must be http or https")
+ portErr := "must be a valid integer between 1 and 65535"
+
+ // validate JSON body
+ errs := tovalidate.ToErrors(validation.Errors{
+ "cachegroupId": validation.Validate(origin.CachegroupID, validation.Min(1)),
+ "coordinateId": validation.Validate(origin.CoordinateID, validation.Min(1)),
+ "deliveryServiceId": validation.Validate(origin.DeliveryServiceID, validation.NotNil),
+ "fqdn": validation.Validate(origin.FQDN, validation.Required, is.DNSName),
+ "ip6Address": validation.Validate(origin.IP6Address, validation.NilOrNotEmpty, is.IPv6),
+ "ipAddress": validation.Validate(origin.IPAddress, validation.NilOrNotEmpty, is.IPv4),
+ "name": validation.Validate(origin.Name, validation.Required, noSpaces),
+ "port": validation.Validate(origin.Port, validation.NilOrNotEmpty.Error(portErr), validation.Min(1).Error(portErr), validation.Max(65535).Error(portErr)),
+ "profileId": validation.Validate(origin.ProfileID, validation.Min(1)),
+ "protocol": validation.Validate(origin.Protocol, validation.Required, validProtocol),
+ "tenantId": validation.Validate(origin.TenantID, validation.Min(1)),
Review Comment:
There are some problems with this validation. `validation.Min` considers empty values to be valid, and empty values are determined for `int` types using reflection to determine if the value is zero. What that means is that `"tenantId": 0` is considered to be passing a filter of `validation.Min(1)`, even though its value is less than one. So when I don't provide a Tenant, this endpoint tells me that I'm not authorized on the non-existent Tenant, instead of telling me `tenantId` is required. You'll need to combine that validation with `validation.Required`.
Also, the `deliveryServiceId` is not actually being validated at all - you're giving it the `validation.NotNil` rule, but as a non-pointer property it'll never be `nil` even if I don't provide it. So you need to make it required 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] gbkannan89 commented on a diff in pull request #7733: To origin rfc3339
Posted by "gbkannan89 (via GitHub)" <gi...@apache.org>.
gbkannan89 commented on code in PR #7733:
URL: https://github.com/apache/trafficcontrol/pull/7733#discussion_r1311942139
##########
traffic_ops/traffic_ops_golang/origin/origins.go:
##########
@@ -487,3 +487,294 @@ func deleteQuery() string {
WHERE id=:id`
return query
}
+
+// Get is the handler for GET requests to Origins of APIv5.
+func Get(w http.ResponseWriter, r *http.Request) {
+
+ var useIMS bool
+
+ inf, sysErr, userErr, errCode := api.NewInfo(r, nil, nil)
+ if sysErr != nil || userErr != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+ defer inf.Close()
+
+ origins, userErr, sysErr, errCode, _ := getOrigins(w.Header(), inf.Params, inf.Tx, inf.User, useIMS)
+
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+
+ returnable := make([]tc.OriginV5, len(origins))
+ for i, origin := range origins {
+ returnable[i] = origin.ToOriginV5()
+ }
+
+ api.WriteResp(w, r, returnable)
+ return
+}
+
+// Create Origin with the passed data for APIv5.
+func Create(w http.ResponseWriter, r *http.Request) {
+ inf, sysError, userError, errorCode := api.NewInfo(r, nil, nil)
+ tx := inf.Tx
+ if sysError != nil || userError != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errorCode, userError, sysError)
+ return
+ }
+ defer inf.Close()
+
+ org, errorCode, readValErr := readAndValidateJsonStruct(r, tx)
+ if readValErr != nil {
+ api.HandleErr(w, r, tx.Tx, errorCode, readValErr, nil)
+ return
+ }
+
+ userErr, sysErr, errCode := checkTenancy(&org.TenantID, &org.DeliveryServiceID, tx, inf.User)
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+
+ _, cdnName, _, err := dbhelpers.GetDSNameAndCDNFromID(inf.Tx.Tx, org.DeliveryServiceID)
+ if err != nil {
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, nil, fmt.Errorf("database error: unable to retrieve delivery service name and cdn: %w", err))
+ return
+ }
+ userErr, sysErr, errCode = dbhelpers.CheckIfCurrentUserCanModifyCDN(tx.Tx, string(cdnName), inf.User.UserName)
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+
+ resultRows, err := tx.NamedQuery(insertQuery(), org)
+ if err != nil {
+ usrErr, sysErr, code := api.ParseDBError(err)
+ api.HandleErr(w, r, tx.Tx, code, usrErr, sysErr)
+ return
+ }
+ defer resultRows.Close()
+
+ rowsAffected := 0
+ for resultRows.Next() {
+ rowsAffected++
+ if err := resultRows.Scan(&org.ID, &org.LastUpdated); err != nil {
Review Comment:
yes, but that is what we have mentioned in document also.
`{ "alerts": [
{
"text": "origin was created.",
"level": "success"
}
],
"response": {
"cachegroup": null,
"cachegroupId": null,
"coordinate": null,
"coordinateId": null,
"deliveryService": null,
"deliveryServiceId": 2,
"fqdn": "example.com",
"id": 2,
"ip6Address": null,
"ipAddress": null,
"isPrimary": null,
"lastUpdated": "2018-12-11 15:14:27+00",
"name": "example",
"port": 80,
"profile": null,
"profileId": null,
"protocol": "http",
"tenant": null,
"tenantId": 1
}}`
--
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 #7733: To origin rfc3339
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #7733:
URL: https://github.com/apache/trafficcontrol/pull/7733#issuecomment-1684230029
## [Codecov](https://app.codecov.io/gh/apache/trafficcontrol/pull/7733?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
> Merging [#7733](https://app.codecov.io/gh/apache/trafficcontrol/pull/7733?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (3f5fb3c) into [master](https://app.codecov.io/gh/apache/trafficcontrol/commit/13184ded20372ed77d7593126361aca9fc518356?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (13184de) will **decrease** coverage by `6.66%`.
> The diff coverage is `0.00%`.
```diff
@@ Coverage Diff @@
## master #7733 +/- ##
============================================
- Coverage 44.20% 37.54% -6.66%
Complexity 98 98
============================================
Files 274 420 +146
Lines 13780 33753 +19973
Branches 90 90
============================================
+ Hits 6091 12674 +6583
- Misses 7197 19907 +12710
- Partials 492 1172 +680
```
| [Flag](https://app.codecov.io/gh/apache/trafficcontrol/pull/7733/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [golib_unit](https://app.codecov.io/gh/apache/trafficcontrol/pull/7733/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `52.52% <0.00%> (?)` | |
| [grove_unit](https://app.codecov.io/gh/apache/trafficcontrol/pull/7733/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `12.02% <ø> (?)` | |
| [t3c_unit](https://app.codecov.io/gh/apache/trafficcontrol/pull/7733/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `5.99% <ø> (?)` | |
| [traffic_monitor_unit](https://app.codecov.io/gh/apache/trafficcontrol/pull/7733/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `26.33% <ø> (ø)` | |
| [traffic_stats_unit](https://app.codecov.io/gh/apache/trafficcontrol/pull/7733/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `10.76% <ø> (?)` | |
| [unit_tests](https://app.codecov.io/gh/apache/trafficcontrol/pull/7733/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `31.49% <0.00%> (+5.15%)` | :arrow_up: |
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=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Files Changed](https://app.codecov.io/gh/apache/trafficcontrol/pull/7733?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [lib/go-tc/origins.go](https://app.codecov.io/gh/apache/trafficcontrol/pull/7733?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-bGliL2dvLXRjL29yaWdpbnMuZ28=) | `0.00% <0.00%> (ø)` | |
| [traffic\_ops/v5-client/origin.go](https://app.codecov.io/gh/apache/trafficcontrol/pull/7733?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-dHJhZmZpY19vcHMvdjUtY2xpZW50L29yaWdpbi5nbw==) | `56.17% <ø> (-3.20%)` | :arrow_down: |
... and [147 files with indirect coverage changes](https://app.codecov.io/gh/apache/trafficcontrol/pull/7733/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
: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=apache)
--
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] gbkannan89 commented on a diff in pull request #7733: To origin rfc3339
Posted by "gbkannan89 (via GitHub)" <gi...@apache.org>.
gbkannan89 commented on code in PR #7733:
URL: https://github.com/apache/trafficcontrol/pull/7733#discussion_r1307318177
##########
traffic_ops/traffic_ops_golang/origin/origins.go:
##########
@@ -487,3 +488,277 @@ func deleteQuery() string {
WHERE id=:id`
return query
}
+
+// Get is the handler for GET requests to Origins of APIv5.
+func Get(w http.ResponseWriter, r *http.Request) {
+
+ var useIMS bool
+
+ inf, sysErr, userErr, errCode := api.NewInfo(r, nil, nil)
+ if sysErr != nil || userErr != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+ defer inf.Close()
+
+ origins, userErr, sysErr, errCode, _ := getOrigins(w.Header(), inf.Params, inf.Tx, inf.User, useIMS)
+ var returnable []tc.OriginV5
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+
+ for _, origin := range origins {
+ returnable = append(returnable, origin.ToOriginV5())
+ }
+
+ // Assign an empty array if no origin is found instead of null.
+ if len(returnable) == 0 {
+ returnable = []tc.OriginV5{}
+ }
+
+ api.WriteResp(w, r, returnable)
+ return
+}
+
+// Create Origin with the passed data for APIv5.
+func Create(w http.ResponseWriter, r *http.Request) {
+ inf, sysError, userError, errorCode := api.NewInfo(r, nil, nil)
+ tx := inf.Tx
+ if sysError != nil || userError != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errorCode, userError, sysError)
+ return
+ }
+ defer inf.Close()
+ org, readValErr := readAndValidateJsonStruct(r)
+ if readValErr != nil {
+ api.HandleErr(w, r, tx.Tx, http.StatusBadRequest, readValErr, nil)
+ return
+ }
+
+ userErr, sysErr, errCode := checkTenancy(&org.TenantID, &org.DeliveryServiceID, tx, inf.User)
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+
+ _, cdnName, _, err := dbhelpers.GetDSNameAndCDNFromID(inf.Tx.Tx, org.DeliveryServiceID)
+ if err != nil {
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, nil, fmt.Errorf("database error: unable to retrieve delivery service name and cdn: %w", err))
+ return
+ }
+ userErr, sysErr, errCode = dbhelpers.CheckIfCurrentUserCanModifyCDN(tx.Tx, string(cdnName), inf.User.UserName)
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+
+ resultRows, err := tx.NamedQuery(insertQuery(), org)
+ if err != nil {
+ usrErr, sysErr, code := api.ParseDBError(err)
+ api.HandleErr(w, r, tx.Tx, code, usrErr, sysErr)
+ return
+ }
+ defer resultRows.Close()
+
+ rowsAffected := 0
+ for resultRows.Next() {
+ rowsAffected++
+ if err := resultRows.Scan(&org.ID, &org.LastUpdated); err != nil {
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, fmt.Errorf("origin create: scanning: "+err.Error()), nil)
+ return
+ }
+ }
+ if rowsAffected == 0 {
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, fmt.Errorf("origin create: scanning: %w", err), nil)
+ return
+ } else if rowsAffected > 1 {
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, fmt.Errorf("origin create: multiple rows returned"), nil)
+ return
+ }
+
+ alerts := tc.CreateAlerts(tc.SuccessLevel, "origin was created.")
+ w.Header().Set("Location", fmt.Sprintf("/api/%d.%d/origins?id=%d", inf.Version.Major, inf.Version.Minor, org.ID))
+ api.WriteAlertsObj(w, r, http.StatusCreated, alerts, org)
+}
+
+// Update a Origin for APIv5.
+func Update(w http.ResponseWriter, r *http.Request) {
+ inf, sysError, userError, errorCode := api.NewInfo(r, []string{"id"}, []string{"id"})
+ tx := inf.Tx
+ if sysError != nil || userError != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errorCode, userError, sysError)
+ return
+ }
+ defer inf.Close()
+
+ origin, readValErr := readAndValidateJsonStruct(r)
+ if readValErr != nil {
+ api.HandleErr(w, r, tx.Tx, http.StatusBadRequest, readValErr, nil)
+ return
+ }
+ userErr, sysErr, errCode := checkTenancy(&origin.TenantID, &origin.DeliveryServiceID, tx, inf.User)
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+
+ isPrimary := false
+ ds := 0
+ var existingLastUpdated time.Time
+
+ requestedOriginId := inf.IntParams["id"]
+
+ q := `SELECT is_primary, deliveryservice, last_updated FROM origin WHERE id = $1`
+ errLookup := tx.QueryRow(q, requestedOriginId).Scan(&isPrimary, &ds, &existingLastUpdated)
+ if errLookup != nil {
+ if errors.Is(errLookup, sql.ErrNoRows) {
+ api.HandleErr(w, r, tx.Tx, http.StatusNotFound, fmt.Errorf("origin not found"), nil)
+ return
+ }
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, nil, fmt.Errorf("database error: %w, when checking if origin with id %d exists", errLookup, requestedOriginId))
+ return
+ }
+ // check if the entity was already updated
+ userErr, sysErr, errCode = api.CheckIfUnModified(r.Header, inf.Tx, requestedOriginId, "origin")
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+
+ if isPrimary && origin.DeliveryServiceID != ds {
+ api.HandleErr(w, r, tx.Tx, http.StatusBadRequest, fmt.Errorf("cannot update the delivery service of a primary origin"), nil)
+ return
+ }
+
+ _, cdnName, _, err := dbhelpers.GetDSNameAndCDNFromID(tx.Tx, origin.DeliveryServiceID)
+ if err != nil {
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, err, nil)
+ return
+ }
+ userErr, sysErr, errCode = dbhelpers.CheckIfCurrentUserCanModifyCDN(tx.Tx, string(cdnName), inf.User.UserName)
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+
+ var lastUpdated time.Time
+ query := `UPDATE origin SET
+ cachegroup=$1,
+ coordinate=$2,
+ deliveryservice=$3,
+ fqdn=$4,
+ ip6_address=$5,
+ ip_address=$6,
+ name=$7,
+ port=$8,
+ profile=$9,
+ protocol=$10,
+ tenant=$11
+ WHERE id=$12 RETURNING last_updated`
+ errUpdate := tx.QueryRow(query, origin.CachegroupID, origin.CoordinateID, origin.DeliveryServiceID,
+ origin.FQDN, origin.IP6Address, origin.IPAddress, origin.Name,
+ origin.Port, origin.ProfileID, origin.Protocol, origin.TenantID, requestedOriginId).Scan(&lastUpdated)
+ if errUpdate != nil {
+ if errors.Is(errUpdate, sql.ErrNoRows) {
+ api.HandleErr(w, r, tx.Tx, http.StatusBadRequest, fmt.Errorf("origin: %d not found", requestedOriginId), nil)
+ return
+ }
+ usrErr, sysErr, code := api.ParseDBError(errUpdate)
+ api.HandleErr(w, r, tx.Tx, code, usrErr, sysErr)
+ return
+ }
+
+ origin.LastUpdated = lastUpdated
+ alerts := tc.CreateAlerts(tc.SuccessLevel, "origin was updated.")
+ api.WriteAlertsObj(w, r, http.StatusOK, alerts, origin)
+ return
+}
+
+// Delete an Origin for APIv5.
+func Delete(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()
+
+ id := inf.Params["id"]
+ errsValid := tovalidate.ToErrors(validation.Errors{
+ "id": validation.Validate(id, validation.NilOrNotEmpty),
+ })
+
+ if len(errsValid) > 0 {
+ api.HandleErr(w, r, tx, http.StatusBadRequest, fmt.Errorf("missing key: id"), nil)
+ return
+ }
Review Comment:
i did because if id is not given in delete it is now throughing internal server error instead thought of showing an error msg. still i should have got id as IntParam
--
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 #7733: To origin rfc3339
Posted by "ocket8888 (via GitHub)" <gi...@apache.org>.
ocket8888 commented on code in PR #7733:
URL: https://github.com/apache/trafficcontrol/pull/7733#discussion_r1311792768
##########
traffic_ops/testing/api/v5/origins_test.go:
##########
@@ -277,17 +277,18 @@ func TestOrigins(t *testing.T) {
"OK when VALID request": {
EndpointID: GetOriginID(t, "origin2"),
ClientSession: TOSession,
- RequestBody: tc.Origin{
- Name: util.Ptr("origin2"),
- Cachegroup: util.Ptr("multiOriginCachegroup"),
- Coordinate: util.Ptr("coordinate2"),
- DeliveryService: util.Ptr("ds3"),
- FQDN: util.Ptr("originupdated.example.com"),
- IPAddress: util.Ptr("1.2.3.4"),
- IP6Address: util.Ptr("0000::1111"),
- Port: util.Ptr(1234),
- Protocol: util.Ptr("http"),
- TenantID: util.Ptr(GetTenantID(t, "tenant2")()),
+ RequestBody: tc.OriginV5{
+ Name: "origin2",
+ Cachegroup: util.Ptr("multiOriginCachegroup"),
+ Coordinate: util.Ptr("coordinate2"),
+ DeliveryService: "ds3",
+ DeliveryServiceID: GetDeliveryServiceId(t, "ds3")(),
Review Comment:
You shouldn't need to add this line. It shouldn't need to manually fetch the TenantID, either, but it was already doing that before you touched this file, so that's not your problem.
##########
traffic_ops/testing/api/v5/origins_test.go:
##########
@@ -646,18 +649,20 @@ func DeleteTestOrigins(t *testing.T) {
assert.NoError(t, err, "Cannot get Origins : %v - alerts: %+v", err, origins.Alerts)
for _, origin := range origins.Response {
+ origin.TenantID = GetTenantID(t, "root")()
+ origin.DeliveryServiceID = GetDeliveryServiceId(t, origin.DeliveryService)()
Review Comment:
This should not be necessary because of the changes you made in the client
--
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 #7733: To origin rfc3339
Posted by "ocket8888 (via GitHub)" <gi...@apache.org>.
ocket8888 commented on code in PR #7733:
URL: https://github.com/apache/trafficcontrol/pull/7733#discussion_r1309298495
##########
traffic_ops/traffic_ops_golang/origin/origins.go:
##########
@@ -487,3 +488,295 @@ func deleteQuery() string {
WHERE id=:id`
return query
}
+
+// Get is the handler for GET requests to Origins of APIv5.
+func Get(w http.ResponseWriter, r *http.Request) {
+
+ var useIMS bool
+
+ inf, sysErr, userErr, errCode := api.NewInfo(r, nil, nil)
+ if sysErr != nil || userErr != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+ defer inf.Close()
+
+ origins, userErr, sysErr, errCode, _ := getOrigins(w.Header(), inf.Params, inf.Tx, inf.User, useIMS)
+
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+
+ returnable := make([]tc.OriginV5, 0) // Initialize as an empty slice
+
+ for _, origin := range origins {
+ returnable = append(returnable, origin.ToOriginV5())
+ }
Review Comment:
this works fine, but fwiw you can avoid reallocations because you know the size ahead of time like:
```go
returnable := make([]tc.OriginV5, len(origins))
for i, origin := range origins {
returnable[i]=origin.ToOriginV5()
}
```
--
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 #7733: To origin rfc3339
Posted by "ocket8888 (via GitHub)" <gi...@apache.org>.
ocket8888 commented on code in PR #7733:
URL: https://github.com/apache/trafficcontrol/pull/7733#discussion_r1309300956
##########
traffic_ops/traffic_ops_golang/origin/origins.go:
##########
@@ -487,3 +488,295 @@ func deleteQuery() string {
WHERE id=:id`
return query
}
+
+// Get is the handler for GET requests to Origins of APIv5.
+func Get(w http.ResponseWriter, r *http.Request) {
+
+ var useIMS bool
+
+ inf, sysErr, userErr, errCode := api.NewInfo(r, nil, nil)
+ if sysErr != nil || userErr != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+ defer inf.Close()
+
+ origins, userErr, sysErr, errCode, _ := getOrigins(w.Header(), inf.Params, inf.Tx, inf.User, useIMS)
+
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+
+ returnable := make([]tc.OriginV5, 0) // Initialize as an empty slice
+
+ for _, origin := range origins {
+ returnable = append(returnable, origin.ToOriginV5())
+ }
+
+ api.WriteResp(w, r, returnable)
+ return
+}
+
+// Create Origin with the passed data for APIv5.
+func Create(w http.ResponseWriter, r *http.Request) {
+ inf, sysError, userError, errorCode := api.NewInfo(r, nil, nil)
+ tx := inf.Tx
+ if sysError != nil || userError != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errorCode, userError, sysError)
+ return
+ }
+ defer inf.Close()
+
+ org, errorCode, readValErr := readAndValidateJsonStruct(r, tx)
+ if readValErr != nil {
+ api.HandleErr(w, r, tx.Tx, errorCode, readValErr, nil)
+ return
+ }
+
+ userErr, sysErr, errCode := checkTenancy(&org.TenantID, &org.DeliveryServiceID, tx, inf.User)
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+
+ _, cdnName, _, err := dbhelpers.GetDSNameAndCDNFromID(inf.Tx.Tx, org.DeliveryServiceID)
+ if err != nil {
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, nil, fmt.Errorf("database error: unable to retrieve delivery service name and cdn: %w", err))
+ return
+ }
+ userErr, sysErr, errCode = dbhelpers.CheckIfCurrentUserCanModifyCDN(tx.Tx, string(cdnName), inf.User.UserName)
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+
+ resultRows, err := tx.NamedQuery(insertQuery(), org)
+ if err != nil {
+ usrErr, sysErr, code := api.ParseDBError(err)
+ api.HandleErr(w, r, tx.Tx, code, usrErr, sysErr)
+ return
+ }
+ defer resultRows.Close()
+
+ rowsAffected := 0
+ for resultRows.Next() {
+ rowsAffected++
+ if err := resultRows.Scan(&org.ID, &org.LastUpdated); err != nil {
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, fmt.Errorf("origin create: scanning: %w", err), nil)
+ return
+ }
+ }
+
+ if rowsAffected == 0 {
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, fmt.Errorf("origin create: no rows inserted"), nil)
+ return
+ } else if rowsAffected > 1 {
+ api.HandleErr(w, r, tx.Tx, http.StatusInternalServerError, fmt.Errorf("origin create: multiple rows returned"), nil)
+ return
+ }
+
+ alerts := tc.CreateAlerts(tc.SuccessLevel, "origin was created.")
+ w.Header().Set("Location", fmt.Sprintf("/api/%s/origins?id=%d", inf.Version, org.ID))
Review Comment:
instead of a string literal, please use the [`github.com/apache/trafficcontrol/lib/go-rfc` package's constant](https://pkg.go.dev/github.com/apache/trafficcontrol/lib/go-rfc#pkg-constants) for HTTP header names.
--
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