[trafficcontrol] branch master updated: Show correct CDN ID in Changelog (#5366)

[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 da25a2d  Show correct CDN ID in Changelog (#5366)
da25a2d is described below

commit da25a2d047507de765ba4e665ee025d0122d1097
Author: Taylor Clayton Frey <ta...@gmail.com>
AuthorDate: Thu Dec 10 08:25:18 2020 -0700

    Show correct CDN ID in Changelog (#5366)
    
    * Show correct CDN ID in Changelog for snapshots (#5195)
    
    Currently, when a CDN snapshot has taken place a log entry with the
    CDN ID is written to the Changelog. The handler is still responsible
    for maintaining backwards compatibility with v1 API parameters.
    
    As such, the log entry is currently only logging the CDN ID should
    the API parameters match v1 legacy parameters.
    
    Going forward the CDN ID shown in the changelog will match either
    the v1 legacy "id" parameter or the newer "cdnID" parameter.
    
    * Include bug fix information in CHANGELOG
    
    * Ensure ID value is validated in tests
    
    * Make ID value check specific integer in test
    
    * Add positive test to ensure ID result is returned
    
    Co-authored-by: Taylor Frey <ta...@comcast.com>
---
 CHANGELOG.md                                               |  1 +
 traffic_ops/traffic_ops_golang/crconfig/handler.go         |  6 +++---
 .../traffic_ops_golang/dbhelpers/db_helpers_test.go        | 14 +++++++++-----
 3 files changed, 13 insertions(+), 8 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 0802677..e365cbc 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -11,6 +11,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).
 - Fixed Astats csv issue where it could crash if caches dont return proc data
 
 ### Fixed
+- [#5195](https://github.com/apache/trafficcontrol/issues/5195) - Correctly show CDN ID in Changelog during Snap
 - [#5294](https://github.com/apache/trafficcontrol/issues/5294) - TP ag grid tables now properly persist column filters 
     on page refresh.
 - [#5295](https://github.com/apache/trafficcontrol/issues/5295) - TP types/servers table now clears all filters instead
diff --git a/traffic_ops/traffic_ops_golang/crconfig/handler.go b/traffic_ops/traffic_ops_golang/crconfig/handler.go
index 21da66e..c37dae4 100644
--- a/traffic_ops/traffic_ops_golang/crconfig/handler.go
+++ b/traffic_ops/traffic_ops_golang/crconfig/handler.go
@@ -175,9 +175,9 @@ func snapshotHandler(w http.ResponseWriter, r *http.Request, deprecated bool) {
 		return
 	}
 
+	id := -1
 	cdn, ok := inf.Params["cdn"]
 	if !ok {
-		var id int
 		if deprecated {
 			id, ok = inf.IntParams["id"]
 			if !ok {
@@ -203,7 +203,7 @@ func snapshotHandler(w http.ResponseWriter, r *http.Request, deprecated bool) {
 		}
 		cdn = name
 	} else {
-		_, ok, err := dbhelpers.GetCDNIDFromName(inf.Tx.Tx, tc.CDNName(cdn))
+		id, ok, err = dbhelpers.GetCDNIDFromName(inf.Tx.Tx, tc.CDNName(cdn))
 		if err != nil {
 			api.HandleErrOptionalDeprecation(w, r, inf.Tx.Tx, http.StatusInternalServerError, nil, errors.New("Error getting CDN ID from name: "+err.Error()), deprecated, &alt)
 			return
@@ -235,7 +235,7 @@ func snapshotHandler(w http.ResponseWriter, r *http.Request, deprecated bool) {
 		return
 	}
 
-	api.CreateChangeLogRawTx(api.ApiChange, "CDN: "+cdn+", ID: "+strconv.Itoa(inf.IntParams["id"])+", ACTION: Snapshot of CRConfig and Monitor", inf.User, inf.Tx.Tx)
+	api.CreateChangeLogRawTx(api.ApiChange, "CDN: "+cdn+", ID: "+strconv.Itoa(id)+", ACTION: Snapshot of CRConfig and Monitor", inf.User, inf.Tx.Tx)
 	if deprecated {
 		api.WriteAlertsObj(w, r, http.StatusOK, api.CreateDeprecationAlerts(&alt), "SUCCESS")
 		return
diff --git a/traffic_ops/traffic_ops_golang/dbhelpers/db_helpers_test.go b/traffic_ops/traffic_ops_golang/dbhelpers/db_helpers_test.go
index da012b6..0c82baa 100644
--- a/traffic_ops/traffic_ops_golang/dbhelpers/db_helpers_test.go
+++ b/traffic_ops/traffic_ops_golang/dbhelpers/db_helpers_test.go
@@ -372,23 +372,27 @@ func TestGetCDNIDFromName(t *testing.T) {
 				"id",
 			})
 			mock.ExpectBegin()
+			expectedIDValue := 3
 			if testCase.storageError != nil {
 				mock.ExpectQuery("SELECT").WillReturnError(testCase.storageError)
 			} else {
 				if testCase.found {
-					rows = rows.AddRow(1)
+					rows = rows.AddRow(expectedIDValue)
 				}
 				mock.ExpectQuery("SELECT").WillReturnRows(rows)
 			}
 			mock.ExpectCommit()
-			_, exists, err := GetCDNIDFromName(db.MustBegin().Tx, "testCdn")
-			if testCase.storageError != nil && err == nil {
+			id, exists, err := GetCDNIDFromName(db.MustBegin().Tx, "testCdn")
+			if testCase.storageError != nil && err == nil && id == expectedIDValue {
 				t.Errorf("Storage error expected: received no storage error")
 			}
-			if testCase.storageError == nil && err != nil {
+			if testCase.storageError == nil && err != nil && id == expectedIDValue {
 				t.Errorf("Storage error not expected: received storage error")
 			}
-			if testCase.found != exists {
+			if exists && testCase.storageError == nil && err == nil && id != expectedIDValue {
+				t.Errorf("Expected ID %d, but got %d", expectedIDValue, id)
+			}
+			if testCase.found != exists && id == 0 {
 				t.Errorf("Expected return exists: %t, actual %t", testCase.found, exists)
 			}
 		})