Fixes service_category apis to respond with RFC3339 date/time Format (#…

…7408) * This fixes service_category apis to respond with RFC3339 date/time strings. Issue: #5911 * This fixes service_category apis to respond with RFC3339 date/time strings. * This fixes service_category apis to respond with RFC3339 date/time strings. This commit removed new struct TimeRFC3339 and uses time.Time instead * This commit removed new struct TimeRFC3339 and uses time.Time instead * #7413 Removing dependency of service catergory on generic cruder for V5 version * Corrected Error messages statement * Added Doc content for RFC 3339 Date/Time Format and updated service category docs. * Updated Http Get operations for Service Category V5 to handle If-Modified-Since request HTTP header * Added Unit Test Cases for V5 service_category functions * Updated comments for the V5 service category functions. * Updated CHANGELOG.md * Added Unit test for db_helpers.go * Addressed Code review from the PR. * Addressed Code review from the PR. * Addressed Code review from the PR. * Addressed Code review from the PR.
apache · Apr 19, 2023 · 664c2d5 · 664c2d5
1 parent 54e509b
commit 664c2d5
Show file tree

Hide file tree

Showing 12 changed files with 596 additions and 31 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -53,6 +53,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).
 
 ### Fixed
 - [#7441](https://github.com/apache/trafficcontrol/pull/7441) *Traffic Ops* Fixed the invalidation jobs endpoint to respect CDN locks.
+- [#7413](https://github.com/apache/trafficcontrol/issues/7413) *Traffic Ops* Fixes service_category apis to respond with RFC3339 date/time Format
 - [#7414](https://github.com/apache/trafficcontrol/pull/7414) * Traffic Portal* Fixed DSR difference for DS required capability.
 - [#7130](https://github.com/apache/trafficcontrol/issues/7130) *Traffic Ops* Fixes service_categories response to POST API.
 - [#7340](https://github.com/apache/trafficcontrol/pull/7340) *Traffic Router* Fixed TR logging for the `cqhv` field when absent.

diff --git a/docs/source/api/v5/service_categories.rst b/docs/source/api/v5/service_categories.rst
@@ -64,7 +64,7 @@ Request Structure
 Response Structure
 ------------------
 :name:        This :term:`Service Category`'s name
-:lastUpdated: The date and time at which this :term:`Service Category` was last modified, in :ref:`non-rfc-datetime`
+:lastUpdated: The date and time at which this :term:`Service Category` was last modified, in :rfc:`3339`
 
 .. code-block:: http
 	:caption: Response Example
@@ -78,13 +78,13 @@ Response Structure
 	Set-Cookie: mojolicious=...; Path=/; Expires=Mon, 18 Nov 2019 17:40:54 GMT; Max-Age=3600; HttpOnly
 	Whole-Content-Sha512: Yzr6TfhxgpZ3pbbrr4TRG4wC3PlnHDDzgs2igtz/1ppLSy2MzugqaGW4y5yzwzl5T3+7q6HWej7GQZt1XIVeZQ==
 	X-Server-Name: traffic_ops_golang/
-	Date: Wed, 11 Mar 2020 20:02:47 GMT
+	Date: Wed, 29 Mar 2023 15:56:34 GMT
 	Content-Length: 102
 
 	{
 		"response": [
 			{
-				"lastUpdated": "2020-03-04 15:46:20-07",
+				"lastUpdated": "2023-03-29T19:43:17.557642+05:30",
 				"name": "SERVICE_CATEGORY_NAME"
 			}
 		]
@@ -121,7 +121,7 @@ Request Structure
 Response Structure
 ------------------
 :name:        This :term:`Service Category`'s name
-:lastUpdated: The date and time at which this :term:`Service Category` was last modified, in :ref:`non-rfc-datetime`
+:lastUpdated: The date and time at which this :term:`Service Category` was last modified, in :rfc:`3339`
 
 .. code-block:: http
 	:caption: Response Example
@@ -135,7 +135,7 @@ Response Structure
 	Set-Cookie: mojolicious=...; Path=/; Expires=Mon, 18 Nov 2019 17:40:54 GMT; Max-Age=3600; HttpOnly
 	Whole-Content-Sha512: +pJm4c3O+JTaSXNt+LP+u240Ba/SsvSSDOQ4rDc6hcyZ0FIL+iY/WWrMHhpLulRGKGY88bM4YPCMaxGn3FZ9yQ==
 	X-Server-Name: traffic_ops_golang/
-	Date: Wed, 11 Mar 2020 20:12:20 GMT
+	Date: Wed, 29 Mar 2023 15:58:37 GMT
 	Content-Length: 154
 
 	{
@@ -146,7 +146,7 @@ Response Structure
 			}
 		],
 		"response": {
-			"lastUpdated": "2020-03-11 14:12:20-06",
+			"lastUpdated": "2023-03-29T21:28:37.884457+05:30",
 			"name": "SERVICE_CATEGORY_NAME"
 		}
 	}
diff --git a/docs/source/api/v5/service_categories_name.rst b/docs/source/api/v5/service_categories_name.rst
@@ -58,7 +58,7 @@ Request Structure
 Response Structure
 ------------------
 :name:        This :term:`Service Category`'s name
-:lastUpdated: The date and time at which this :term:`Service Category` was last modified, in :ref:`non-rfc-datetime`
+:lastUpdated: The date and time at which this :term:`Service Category` was last modified, in :rfc:`3339`
 
 .. code-block:: http
 	:caption: Response Example
@@ -72,7 +72,7 @@ Response Structure
 	Set-Cookie: mojolicious=...; Path=/; Expires=Mon, 18 Nov 2019 17:40:54 GMT; Max-Age=3600; HttpOnly
 	Whole-Content-Sha512: +pJm4c3O+JTaSXNt+LP+u240Ba/SsvSSDOQ4rDc6hcyZ0FIL+iY/WWrMHhpLulRGKGY88bM4YPCMaxGn3FZ9yQ==
 	X-Server-Name: traffic_ops_golang/
-	Date: Wed, 11 Mar 2020 20:12:20 GMT
+	Date: Wed, 29 Mar 2023 15:58:37 GMT
 	Content-Length: 189
 
 	{
@@ -83,7 +83,7 @@ Response Structure
 			}
 		],
 		"response": {
-			"lastUpdated": "2020-03-11 14:12:20-06",
+			"lastUpdated": "2023-03-29T21:28:37.884457+05:30",
 			"name": "New Name"
 		}
 	}
@@ -136,7 +136,7 @@ Response Structure
 	Set-Cookie: mojolicious=...; Path=/; Expires=Mon, 17 Aug 2020 16:13:31 GMT; Max-Age=3600; HttpOnly
 	Whole-Content-Sha512: yErJobzG9IA0khvqZQK+Yi7X4pFVvOqxn6PjrdzN5DnKVm/K8Kka3REul1XmKJnMXVRY8RayoEVGDm16mBFe4Q==
 	X-Server-Name: traffic_ops_golang/
-	Date: Mon, 17 Aug 2020 15:13:31 GMT
+	Date: Wed, 29 Mar 2023 15:58:37 GMT
 	Content-Length: 103
 
 	{

diff --git a/lib/go-tc/service_category.go b/lib/go-tc/service_category.go
@@ -19,6 +19,8 @@ package tc
  * under the License.
  */
 
+import "time"
+
 // ServiceCategoriesResponse is a list of Service Categories as a response.
 type ServiceCategoriesResponse struct {
 	Response []ServiceCategory `json:"response"`
@@ -37,3 +39,35 @@ type ServiceCategory struct {
 	LastUpdated TimeNoMod `json:"lastUpdated" db:"last_updated"`
 	Name        string    `json:"name" db:"name"`
 }
+
+// ServiceCategoriesResponseV50 is a list of Service Categories as a response.
+type ServiceCategoriesResponseV50 struct {
+	Response []ServiceCategoryV50 `json:"response"`
+	Alerts
+}
+
+// ServiceCategoryResponseV50 is a single Service Category response for Update and Create to
+// depict what changed.
+type ServiceCategoryResponseV50 struct {
+	Response ServiceCategoryV50 `json:"response"`
+	Alerts
+}
+
+// ServiceCategoryV50 holds the name and last updated time stamp.
+type ServiceCategoryV50 struct {
+	LastUpdated time.Time `json:"lastUpdated" db:"last_updated"`
+	Name        string    `json:"name" db:"name"`
+}
+
+// ServiceCategoriesResponseV5 is the type of a response from the service_categories
+// Traffic Ops endpoint.
+// It always points to the type for the latest minor version of ServiceCategoriesResponseV5x APIv5.
+type ServiceCategoriesResponseV5 = ServiceCategoriesResponseV50
+
+// ServiceCategoryResponseV5 is the type of a response from the service_categories
+// Traffic Ops endpoint.
+// It always points to the type for the latest minor version of ServiceCategoryResponseV5x APIv5.
+type ServiceCategoryResponseV5 = ServiceCategoryResponseV50
+
+// ServiceCategoryV5 always points to the type for the latest minor version of serviceCategoryV5x APIv5.
+type ServiceCategoryV5 = ServiceCategoryV50
diff --git a/traffic_ops/testing/api/v5/servicecategories_test.go b/traffic_ops/testing/api/v5/servicecategories_test.go
@@ -37,7 +37,7 @@ func TestServiceCategories(t *testing.T) {
 		currentTimeRFC := currentTime.Format(time.RFC1123)
 		tomorrow := currentTime.AddDate(0, 0, 1).Format(time.RFC1123)
 
-		methodTests := utils.TestCase[client.Session, client.RequestOptions, tc.ServiceCategory]{
+		methodTests := utils.TestCase[client.Session, client.RequestOptions, tc.ServiceCategoryV5]{
 			"GET": {
 				"NOT MODIFIED when NO CHANGES made": {
 					ClientSession: TOSession,
@@ -103,20 +103,20 @@ func TestServiceCategories(t *testing.T) {
 			"POST": {
 				"BAD REQUEST when ALREADY EXISTS": {
 					ClientSession: TOSession,
-					RequestBody:   tc.ServiceCategory{Name: "serviceCategory1"},
+					RequestBody:   tc.ServiceCategoryV5{Name: "serviceCategory1"},
 					Expectations:  utils.CkRequest(utils.HasError(), utils.HasStatus(http.StatusBadRequest)),
 				},
 				"BAD REQUEST when NAME FIELD is BLANK": {
 					ClientSession: TOSession,
-					RequestBody:   tc.ServiceCategory{Name: ""},
+					RequestBody:   tc.ServiceCategoryV5{Name: ""},
 					Expectations:  utils.CkRequest(utils.HasError(), utils.HasStatus(http.StatusBadRequest)),
 				},
 			},
 			"PUT": {
 				"OK when VALID request": {
 					ClientSession: TOSession,
 					RequestOpts:   client.RequestOptions{QueryParameters: url.Values{"name": {"barServiceCategory2"}}},
-					RequestBody:   tc.ServiceCategory{Name: "newName"},
+					RequestBody:   tc.ServiceCategoryV5{Name: "newName"},
 					Expectations: utils.CkRequest(utils.NoError(), utils.HasStatus(http.StatusOK),
 						validateServiceCategoriesUpdateCreateFields("newName", map[string]interface{}{"Name": "newName"})),
 				},
@@ -126,12 +126,12 @@ func TestServiceCategories(t *testing.T) {
 						QueryParameters: url.Values{"name": {"serviceCategory1"}},
 						Header:          http.Header{rfc.IfUnmodifiedSince: {currentTimeRFC}},
 					},
-					RequestBody:  tc.ServiceCategory{Name: "newName"},
+					RequestBody:  tc.ServiceCategoryV5{Name: "newName"},
 					Expectations: utils.CkRequest(utils.HasError(), utils.HasStatus(http.StatusPreconditionFailed)),
 				},
 				"PRECONDITION FAILED when updating with IFMATCH ETAG Header": {
 					ClientSession: TOSession,
-					RequestBody:   tc.ServiceCategory{Name: "newName"},
+					RequestBody:   tc.ServiceCategoryV5{Name: "newName"},
 					RequestOpts: client.RequestOptions{
 						QueryParameters: url.Values{"name": {"serviceCategory1"}},
 						Header:          http.Header{rfc.IfMatch: {rfc.ETag(currentTime)}},
@@ -196,7 +196,7 @@ func TestServiceCategories(t *testing.T) {
 func validateServiceCategoriesFields(expectedResp map[string]interface{}) utils.CkReqFunc {
 	return func(t *testing.T, _ toclientlib.ReqInf, resp interface{}, _ tc.Alerts, _ error) {
 		assert.RequireNotNil(t, resp, "Expected Service Categories response to not be nil.")
-		serviceCategoryResp := resp.([]tc.ServiceCategory)
+		serviceCategoryResp := resp.([]tc.ServiceCategoryV5)
 		for field, expected := range expectedResp {
 			for _, serviceCategory := range serviceCategoryResp {
 				switch field {
@@ -223,7 +223,7 @@ func validateServiceCategoriesUpdateCreateFields(name string, expectedResp map[s
 
 func validateServiceCategoriesPagination(paginationParam string) utils.CkReqFunc {
 	return func(t *testing.T, _ toclientlib.ReqInf, resp interface{}, _ tc.Alerts, _ error) {
-		paginationResp := resp.([]tc.ServiceCategory)
+		paginationResp := resp.([]tc.ServiceCategoryV5)
 
 		opts := client.NewRequestOptions()
 		opts.QueryParameters.Set("orderby", "id")
@@ -247,7 +247,7 @@ func validateServiceCategoriesSort() utils.CkReqFunc {
 	return func(t *testing.T, _ toclientlib.ReqInf, resp interface{}, alerts tc.Alerts, _ error) {
 		assert.RequireNotNil(t, resp, "Expected Service Categories response to not be nil.")
 		var serviceCategoryNames []string
-		serviceCategoryResp := resp.([]tc.ServiceCategory)
+		serviceCategoryResp := resp.([]tc.ServiceCategoryV5)
 		for _, serviceCategory := range serviceCategoryResp {
 			serviceCategoryNames = append(serviceCategoryNames, serviceCategory.Name)
 		}
@@ -258,7 +258,7 @@ func validateServiceCategoriesSort() utils.CkReqFunc {
 func validateServiceCategoriesDescSort() utils.CkReqFunc {
 	return func(t *testing.T, _ toclientlib.ReqInf, resp interface{}, alerts tc.Alerts, _ error) {
 		assert.RequireNotNil(t, resp, "Expected Service Categories response to not be nil.")
-		serviceCategoriesDescResp := resp.([]tc.ServiceCategory)
+		serviceCategoriesDescResp := resp.([]tc.ServiceCategoryV5)
 		var descSortedList []string
 		var ascSortedList []string
 		assert.RequireGreaterOrEqual(t, len(serviceCategoriesDescResp), 2, "Need at least 2 Service Categories in Traffic Ops to test desc sort, found: %d", len(serviceCategoriesDescResp))

diff --git a/traffic_ops/testing/api/v5/traffic_control_test.go b/traffic_ops/testing/api/v5/traffic_control_test.go
@@ -48,7 +48,7 @@ type TrafficControl struct {
 	Servers                                           []tc.ServerV4                           `json:"servers"`
 	ServerServerCapabilities                          []tc.ServerServerCapability             `json:"serverServerCapabilities"`
 	ServerCapabilities                                []tc.ServerCapabilityV41                `json:"serverCapabilities"`
-	ServiceCategories                                 []tc.ServiceCategory                    `json:"serviceCategories"`
+	ServiceCategories                                 []tc.ServiceCategoryV5                  `json:"serviceCategories"`
 	Statuses                                          []tc.StatusNullable                     `json:"statuses"`
 	StaticDNSEntries                                  []tc.StaticDNSEntry                     `json:"staticdnsentries"`
 	StatsSummaries                                    []tc.StatsSummary                       `json:"statsSummaries"`

diff --git a/traffic_ops/traffic_ops_golang/dbhelpers/db_helpers.go b/traffic_ops/traffic_ops_golang/dbhelpers/db_helpers.go
@@ -2187,3 +2187,18 @@ func GetSCInfo(tx *sql.Tx, name string) (bool, error) {
 	}
 	return true, nil
 }
+
+// ServiceCategoryExists confirms whether the service category exists, and an error (if one occurs).
+func ServiceCategoryExists(tx *sql.Tx, name string) (bool, error) {
+	var count int
+	if err := tx.QueryRow("SELECT count(name) FROM service_category AS sc WHERE sc.name=$1", name).Scan(&count); err != nil {
+		return false, fmt.Errorf("error getting service category info: %w", err)
+	}
+	if count == 0 {
+		return false, nil
+	}
+	if count != 1 {
+		return false, fmt.Errorf("getting service category info - expected row count: 1, actual: %d", count)
+	}
+	return true, nil
+}
diff --git a/traffic_ops/traffic_ops_golang/dbhelpers/db_helpers_test.go b/traffic_ops/traffic_ops_golang/dbhelpers/db_helpers_test.go
@@ -460,3 +460,54 @@ func TestGetSCInfo(t *testing.T) {
 		})
 	}
 }
+
+func TestServiceCategoryExists(t *testing.T) {
+	var testCases = []struct {
+		description   string
+		name          string
+		expectedError error
+		exists        bool
+	}{
+		{
+			description:   "Success: Get valid Service Category",
+			name:          "testServiceCategory1",
+			expectedError: nil,
+			exists:        true,
+		},
+		{
+			description:   "Failure: Service Category not in DB",
+			name:          "testServiceCategory2",
+			expectedError: sql.ErrNoRows,
+			exists:        false,
+		},
+	}
+	for _, testCase := range testCases {
+		t.Run(testCase.description, func(t *testing.T) {
+			mockDB, mock, err := sqlmock.New()
+			if err != nil {
+				t.Fatalf("an error '%s' was not expected when opening a stub database connection", err)
+			}
+			defer mockDB.Close()
+
+			db := sqlx.NewDb(mockDB, "sqlmock")
+			defer db.Close()
+
+			mock.ExpectBegin()
+			rows := sqlmock.NewRows([]string{"count"})
+			if testCase.exists {
+				rows = rows.AddRow(1)
+			}
+			mock.ExpectQuery("SELECT").WillReturnRows(rows)
+			mock.ExpectCommit()
+
+			scExists, err := ServiceCategoryExists(db.MustBegin().Tx, testCase.name)
+			if testCase.exists != scExists {
+				t.Errorf("Expected return exists: %t, actual %t", testCase.exists, scExists)
+			}
+
+			if !errors.Is(err, testCase.expectedError) {
+				t.Errorf("ServiceCategoryExists expected: %s, actual: %s", testCase.expectedError, err)
+			}
+		})
+	}
+}
diff --git a/traffic_ops/traffic_ops_golang/routing/routes.go b/traffic_ops/traffic_ops_golang/routing/routes.go
@@ -424,10 +424,10 @@ func Routes(d ServerData) ([]Route, http.Handler, error) {
 		{Version: api.Version{Major: 5, Minor: 0}, Method: http.MethodDelete, Path: `deliveryservices/{dsid}/regexes/{regexid}?$`, Handler: deliveryservicesregexes.Delete, RequiredPrivLevel: auth.PrivLevelOperations, RequiredPermissions: []string{"DELIVERY-SERVICE:UPDATE", "DELIVERY-SERVICE:READ", "TYPE:READ"}, Authenticated: Authenticated, Middlewares: nil, ID: 424673166331},
 
 		//ServiceCategories
-		{Version: api.Version{Major: 5, Minor: 0}, Method: http.MethodGet, Path: `service_categories/?$`, Handler: api.ReadHandler(&servicecategory.TOServiceCategory{}), RequiredPrivLevel: auth.PrivLevelReadOnly, RequiredPermissions: []string{"SERVICE-CATEGORY:READ"}, Authenticated: Authenticated, Middlewares: nil, ID: 40851815431},
-		{Version: api.Version{Major: 5, Minor: 0}, Method: http.MethodPut, Path: `service_categories/{name}/?$`, Handler: servicecategory.Update, RequiredPrivLevel: auth.PrivLevelOperations, RequiredPermissions: []string{"SERVICE-CATEGORY:UPDATE", "SERVICE-CATEGORY:READ"}, Authenticated: Authenticated, Middlewares: nil, ID: 4063691411},
-		{Version: api.Version{Major: 5, Minor: 0}, Method: http.MethodPost, Path: `service_categories/?$`, Handler: api.CreateHandler(&servicecategory.TOServiceCategory{}), RequiredPrivLevel: auth.PrivLevelOperations, RequiredPermissions: []string{"SERVICE-CATEGORY:CREATE", "SERVICE-CATEGORY:READ"}, Authenticated: Authenticated, Middlewares: nil, ID: 4537138011},
-		{Version: api.Version{Major: 5, Minor: 0}, Method: http.MethodDelete, Path: `service_categories/{name}$`, Handler: api.DeleteHandler(&servicecategory.TOServiceCategory{}), RequiredPrivLevel: auth.PrivLevelOperations, RequiredPermissions: []string{"SERVICE-CATEGORY:DELETE", "SERVICE-CATEGORY:READ"}, Authenticated: Authenticated, Middlewares: nil, ID: 43253822381},
+		{Version: api.Version{Major: 5, Minor: 0}, Method: http.MethodGet, Path: `service_categories/?$`, Handler: servicecategory.Get, RequiredPrivLevel: auth.PrivLevelReadOnly, RequiredPermissions: []string{"SERVICE-CATEGORY:READ"}, Authenticated: Authenticated, Middlewares: nil, ID: 40851815431},
+		{Version: api.Version{Major: 5, Minor: 0}, Method: http.MethodPut, Path: `service_categories/{name}/?$`, Handler: servicecategory.UpdateServiceCategory, RequiredPrivLevel: auth.PrivLevelOperations, RequiredPermissions: []string{"SERVICE-CATEGORY:UPDATE", "SERVICE-CATEGORY:READ"}, Authenticated: Authenticated, Middlewares: nil, ID: 4063691411},
+		{Version: api.Version{Major: 5, Minor: 0}, Method: http.MethodPost, Path: `service_categories/?$`, Handler: servicecategory.CreateServiceCategory, RequiredPrivLevel: auth.PrivLevelOperations, RequiredPermissions: []string{"SERVICE-CATEGORY:CREATE", "SERVICE-CATEGORY:READ"}, Authenticated: Authenticated, Middlewares: nil, ID: 4537138011},
+		{Version: api.Version{Major: 5, Minor: 0}, Method: http.MethodDelete, Path: `service_categories/{name}$`, Handler: servicecategory.DeleteServiceCategory, RequiredPrivLevel: auth.PrivLevelOperations, RequiredPermissions: []string{"SERVICE-CATEGORY:DELETE", "SERVICE-CATEGORY:READ"}, Authenticated: Authenticated, Middlewares: nil, ID: 43253822381},
 
 		//StaticDNSEntries
 		{Version: api.Version{Major: 5, Minor: 0}, Method: http.MethodGet, Path: `staticdnsentries/?$`, Handler: api.ReadHandler(&staticdnsentry.TOStaticDNSEntry{}), RequiredPrivLevel: auth.PrivLevelReadOnly, RequiredPermissions: []string{"STATIC-DN:READ", "CACHE-GROUP:READ", "DELIVERY-SERVICE:READ"}, Authenticated: Authenticated, Middlewares: nil, ID: 42893947731},