rawlinp commented on a change in pull request #5974:
URL: https://github.com/apache/trafficcontrol/pull/5974#discussion_r665805796
##########
File path: traffic_ops/traffic_ops_golang/dbhelpers/db_helpers.go
##########
@@ -242,52 +289,59 @@ func CheckIfCurrentUserCanModifyCDNWithID(tx *sql.Tx,
cdnID int64, user string)
} else if !ok {
return errors.New("CDN not found"), nil, http.StatusNotFound
}
- query := `SELECT username, soft FROM cdn_lock WHERE cdn=$1`
+ return CheckIfCurrentUserCanModifyCDN(tx, string(cdnName), user)
+}
+
+// CheckIfCurrentUserCanModifyCachegroup checks if the current user has the
lock on the cdns that are associated with the provided cachegroup ID.
+// This will succeed if no other user has a hard lock on any of the CDNs that
relate to the cachegroup in question.
+func CheckIfCurrentUserCanModifyCachegroup(tx *sql.Tx, cachegroupID int, user
string) (error, error, int) {
+ query := `SELECT username, cdn, soft FROM cdn_lock WHERE cdn IN (SELECT
name FROM cdn WHERE id IN (SELECT cdn_id FROM server WHERE cachegroup = ($1)))`
var userName string
+ var cdn string
var soft bool
- rows, err := tx.Query(query, string(cdnName))
+ rows, err := tx.Query(query, cachegroupID)
if err != nil {
if errors.Is(err, sql.ErrNoRows) {
return nil, nil, http.StatusOK
}
- return nil, errors.New("querying cdn_lock for user " + user + "
and cdn " + string(cdnName) + ": " + err.Error()),
http.StatusInternalServerError
+ return nil, errors.New("querying cdn_lock for user " + user + "
and cachegroup ID " + strconv.Itoa(cachegroupID) + ": " + err.Error()),
http.StatusInternalServerError
}
defer rows.Close()
for rows.Next() {
- err = rows.Scan(&userName, &soft)
+ err = rows.Scan(&userName, &cdn, &soft)
if err != nil {
- return nil, errors.New("scanning cdn_lock for user " +
user + " and cdn " + string(cdnName) + ": " + err.Error()),
http.StatusInternalServerError
+ return nil, errors.New("scanning cdn_lock for user " +
user + " and cachegroup ID " + strconv.Itoa(cachegroupID) + ": " +
err.Error()), http.StatusInternalServerError
}
}
if userName != "" && user != userName && !soft {
Review comment:
Is this check supposed to be in the loop above?
##########
File path: traffic_ops/traffic_ops_golang/dbhelpers/db_helpers.go
##########
@@ -242,52 +289,59 @@ func CheckIfCurrentUserCanModifyCDNWithID(tx *sql.Tx,
cdnID int64, user string)
} else if !ok {
return errors.New("CDN not found"), nil, http.StatusNotFound
}
- query := `SELECT username, soft FROM cdn_lock WHERE cdn=$1`
+ return CheckIfCurrentUserCanModifyCDN(tx, string(cdnName), user)
+}
+
+// CheckIfCurrentUserCanModifyCachegroup checks if the current user has the
lock on the cdns that are associated with the provided cachegroup ID.
+// This will succeed if no other user has a hard lock on any of the CDNs that
relate to the cachegroup in question.
+func CheckIfCurrentUserCanModifyCachegroup(tx *sql.Tx, cachegroupID int, user
string) (error, error, int) {
+ query := `SELECT username, cdn, soft FROM cdn_lock WHERE cdn IN (SELECT
name FROM cdn WHERE id IN (SELECT cdn_id FROM server WHERE cachegroup = ($1)))`
var userName string
+ var cdn string
var soft bool
- rows, err := tx.Query(query, string(cdnName))
+ rows, err := tx.Query(query, cachegroupID)
if err != nil {
if errors.Is(err, sql.ErrNoRows) {
return nil, nil, http.StatusOK
}
- return nil, errors.New("querying cdn_lock for user " + user + "
and cdn " + string(cdnName) + ": " + err.Error()),
http.StatusInternalServerError
+ return nil, errors.New("querying cdn_lock for user " + user + "
and cachegroup ID " + strconv.Itoa(cachegroupID) + ": " + err.Error()),
http.StatusInternalServerError
}
defer rows.Close()
for rows.Next() {
- err = rows.Scan(&userName, &soft)
+ err = rows.Scan(&userName, &cdn, &soft)
if err != nil {
- return nil, errors.New("scanning cdn_lock for user " +
user + " and cdn " + string(cdnName) + ": " + err.Error()),
http.StatusInternalServerError
+ return nil, errors.New("scanning cdn_lock for user " +
user + " and cachegroup ID " + strconv.Itoa(cachegroupID) + ": " +
err.Error()), http.StatusInternalServerError
}
}
if userName != "" && user != userName && !soft {
- return errors.New("user " + userName + " currently has a hard
lock on cdn " + string(cdnName)), nil, http.StatusForbidden
+ return errors.New("user " + userName + " currently has a hard
lock on cdn " + cdn), nil, http.StatusForbidden
}
return nil, nil, http.StatusOK
}
-// CheckIfCurrentUserCanModifyCachegroup checks if the current user has the
lock on the cdns that are associated with the provided cachegroup ID.
-// This will succeed if no other user has a hard lock on any of the CDNs that
relate to the cachegroup in question.
-func CheckIfCurrentUserCanModifyCachegroup(tx *sql.Tx, cachegroupID int, user
string) (error, error, int) {
- query := `SELECT username, cdn, soft FROM cdn_lock WHERE cdn IN (SELECT
name FROM cdn WHERE id IN (SELECT cdn_id FROM server WHERE cachegroup = ($1)))`
+// CheckIfCurrentUserCanModifyCachegroups checks if the current user has the
lock on the cdns that are associated with the provided cachegroup IDs.
+// This will succeed if no other user has a hard lock on any of the CDNs that
relate to the cachegroups in question.
+func CheckIfCurrentUserCanModifyCachegroups(tx *sql.Tx, cachegroupID []int,
user string) (error, error, int) {
Review comment:
nit: `cachegroupID` -> `cachegroupIDs` since it's a slice
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
