[trafficcontrol] branch master updated: Fix Internal server error in update cachegroups by already exist cg name (#6064)

[oc...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

ocket8888 pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/trafficcontrol.git


The following commit(s) were added to refs/heads/master by this push:
     new d0a15d5  Fix Internal server error in update cachegroups by already exist cg name (#6064)
d0a15d5 is described below

commit d0a15d58bcd51b28af5b986b3255d5799a15341d
Author: dmohan001c <de...@comcast.com>
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
---
 .../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
 }