This is an automated email from the ASF dual-hosted git repository. zrhoffman pushed a commit to branch 6.0.x in repository https://gitbox.apache.org/repos/asf/trafficcontrol.git
commit 8567ae401e4e128440d878579e1207f0d98e33ca Author: dmohan001c <[email protected]> AuthorDate: Thu Sep 2 19:35:53 2021 +0530 Fix Internal server error in update cachegroups by already exist cg name (#6064) * changed error status code for update cachegroup * updated the error status code * updated error message and provided correct status code * added validation for existence of cachegroup * updated query to check existence of cachegroup name * updated constanst error message * updated constanst error message * updated query to return number of rows * added new validation to compare errors * replace the tc.DBError * returning errors (cherry picked from commit d0a15d58bcd51b28af5b986b3255d5799a15341d) --- .../traffic_ops_golang/cachegroup/cachegroups.go | 46 ++++++++++++++++++---- 1 file changed, 39 insertions(+), 7 deletions(-) diff --git a/traffic_ops/traffic_ops_golang/cachegroup/cachegroups.go b/traffic_ops/traffic_ops_golang/cachegroup/cachegroups.go index 04272729..b6c794e 100644 --- a/traffic_ops/traffic_ops_golang/cachegroup/cachegroups.go +++ b/traffic_ops/traffic_ops_golang/cachegroup/cachegroups.go @@ -37,7 +37,7 @@ import ( "github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/api" "github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/dbhelpers" - "github.com/go-ozzo/ozzo-validation" + validation "github.com/go-ozzo/ozzo-validation" "github.com/jmoiron/sqlx" "github.com/lib/pq" ) @@ -414,16 +414,42 @@ func (cg *TOCacheGroup) createCoordinate() (*int, error) { return coordinateID, nil } +const numberOfDuplicatesQuery = ` +SELECT COUNT(*) +FROM public.coordinate +WHERE id NOT IN ( + SELECT coordinate + FROM public.cachegroup + WHERE id = $1 +) +AND name = $2` + +type userError string + +func (e userError) Error() string { + return string(e) +} + +const duplicateExist userError = "cachegroup name already exists, please choose a different name" + func (cg *TOCacheGroup) updateCoordinate() error { if cg.Latitude != nil && cg.Longitude != nil { + var count uint + if err := cg.ReqInfo.Tx.Tx.QueryRow(numberOfDuplicatesQuery, *cg.ID, tc.CachegroupCoordinateNamePrefix+*cg.Name).Scan(&count); err != nil { + return fmt.Errorf("getting coordinate for Cache Group '%s': %w", *cg.Name, err) + } + if count > 0 { + return duplicateExist + } q := `UPDATE coordinate SET name = $1, latitude = $2, longitude = $3 WHERE id = (SELECT coordinate FROM cachegroup WHERE id = $4)` result, err := cg.ReqInfo.Tx.Tx.Exec(q, tc.CachegroupCoordinateNamePrefix+*cg.Name, *cg.Latitude, *cg.Longitude, *cg.ID) + if err != nil { - return fmt.Errorf("updating coordinate for cachegroup '%s': %s", *cg.Name, err.Error()) + return fmt.Errorf("updating coordinate for cachegroup '%s': %w", *cg.Name, err) } rowsAffected, err := result.RowsAffected() if err != nil { - return fmt.Errorf("updating coordinate for cachegroup '%s', getting rows affected: %s", *cg.Name, err.Error()) + return fmt.Errorf("updating coordinate for cachegroup '%s', getting rows affected: %w", *cg.Name, err) } if rowsAffected == 0 { return fmt.Errorf("updating coordinate for cachegroup '%s', zero rows affected", *cg.Name) @@ -663,7 +689,7 @@ func (cg *TOCacheGroup) handleCoordinateUpdate() (*int, error, error, int) { return nil, fmt.Errorf("no cachegroup with id %d found", *cg.ID), nil, http.StatusNotFound } if err != nil { - return nil, nil, tc.DBError, http.StatusInternalServerError + return nil, nil, err, http.StatusInternalServerError } // If partial coordinate information is given or the coordinate information is wholly @@ -680,25 +706,31 @@ func (cg *TOCacheGroup) handleCoordinateUpdate() (*int, error, error, int) { // if cg.Latitude == nil || cg.Longitude == nil { if err = cg.deleteCoordinate(*coordinateID); err != nil { - return nil, nil, tc.DBError, http.StatusInternalServerError + return nil, nil, err, http.StatusInternalServerError } cg.Latitude = nil cg.Longitude = nil return nil, nil, nil, http.StatusOK } - if err = cg.updateCoordinate(); err != nil { - return nil, nil, tc.DBError, http.StatusInternalServerError + err = cg.updateCoordinate() + if err != nil { + if errors.Is(err, duplicateExist) { + return nil, err, err, http.StatusBadRequest + } + return nil, err, err, http.StatusInternalServerError } return coordinateID, nil, nil, http.StatusOK } func (cg *TOCacheGroup) getCoordinateID() (*int, error) { q := `SELECT coordinate FROM cachegroup WHERE id = $1` + var coordinateID *int if err := cg.ReqInfo.Tx.Tx.QueryRow(q, *cg.ID).Scan(&coordinateID); err != nil { return nil, err } + return coordinateID, nil }
