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-unsubscr...@trafficcontrol.apache.org

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

Reply via email to