You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@trafficcontrol.apache.org by GitBox <gi...@apache.org> on 2020/01/31 19:47:54 UTC

[GitHub] [trafficcontrol] ocket8888 opened a new pull request #4365: Add missing deprecation notices to /user/current/jobs

ocket8888 opened a new pull request #4365: Add missing deprecation notices to /user/current/jobs
URL: https://github.com/apache/trafficcontrol/pull/4365
 
 
   ## What does this PR (Pull Request) do?
   - [x] This PR is not related to any Issue
   
   The `/user/current/jobs` endpoint has been marked as deprecated in the documentation, and the "happy paths" had deprecation notices. However, in the event of an error no deprecation notice was output. This PR includes changes that ensure the notice is always output, and adds them to the Perl handlers as well - which used to never output the notice (this is important because the route is perl-bypass-able, so it's possible an API consumer never sees the notice depending on TO server configuration).
   
   Finally, adds a "deprecated" entry for the endpoint in the CHANGELOG and updates API examples in the docs to show the new deprecation notices.
   
   ## Which Traffic Control components are affected by this PR?
   - Documentation
   - Traffic Ops
   
   ## What is the best way to verify this PR?
   Run the existing unit and client/api integration tests, build and read the documentation, and make requests to `/user/current/jobs` and ensure the deprecation notice is output.
   
   ## The following criteria are ALL met by this PR
   - [x] Tests are unnecessary
   - [x] This PR includes documentation
   - [x] This PR includes an update to CHANGELOG.md
   - [x] This PR includes any and all required license headers
   - [x] This PR does not include a database migration
   - [x] This PR **DOES NOT FIX A SERIOUS SECURITY VULNERABILITY**

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [trafficcontrol] ocket8888 commented on a change in pull request #4365: Add missing deprecation notices to /user/current/jobs

Posted by GitBox <gi...@apache.org>.
ocket8888 commented on a change in pull request #4365: Add missing deprecation notices to /user/current/jobs
URL: https://github.com/apache/trafficcontrol/pull/4365#discussion_r373704514
 
 

 ##########
 File path: traffic_ops/traffic_ops_golang/invalidationjobs/userinvalidationjobs.go
 ##########
 @@ -106,52 +114,53 @@ func CreateUserJob(w http.ResponseWriter, r *http.Request) {
 		&result.StartTime)
 	if err != nil {
 		userErr, sysErr, code := api.ParseDBError(err)
-		api.HandleErr(w, r, inf.Tx.Tx, code, userErr, sysErr)
+		userErr = api.LogErr(r, code, userErr, sysErr)
+		if err := inf.Tx.Tx.Rollback(); err != nil && err != sql.ErrTxDone {
+			log.Errorln("rolling back transaction: " + err.Error())
+		}
+		alerts.AddNewAlert(tc.ErrorLevel, userErr.Error())
+		api.WriteAlerts(w, r, code, alerts)
 		return
 	}
 
 	if err := setRevalFlags(*job.DSID, inf.Tx.Tx); err != nil {
-		api.HandleErr(w, r, inf.Tx.Tx, http.StatusInternalServerError, nil, fmt.Errorf("setting reval flags: %v", err))
-		return
-	}
-
-	respObj := apiResponse{
-		[]tc.Alert{
-			tc.Alert{
-				Level: tc.SuccessLevel.String(),
-				Text:  "Invalidation Job creation was successful.",
-			},
-			api.DeprecationWarning("POST /jobs"),
-		},
-		result,
-	}
-	resp, err := json.Marshal(respObj)
-	if err != nil {
-		api.HandleErr(w, r, inf.Tx.Tx, http.StatusInternalServerError, nil, fmt.Errorf("Marshaling JSON: %v", err))
+		errCode = http.StatusInternalServerError
+		alerts.AddNewAlert(tc.ErrorLevel, api.LogErr(r, errCode, nil, fmt.Errorf("setting reval flags: %v", err)).Error())
+		if err := inf.Tx.Tx.Rollback(); err != nil && err != sql.ErrTxDone {
+			log.Errorln("rolling back transaction: " + err.Error())
+		}
+		api.WriteAlerts(w, r, errCode, alerts)
 		return
 	}
 
-	w.Header().Set(rfc.ContentType, rfc.ApplicationJSON)
+	alerts.AddNewAlert(tc.SuccessLevel, "Invalidation Job creation was successful")
 	w.Header().Set(http.CanonicalHeaderKey("location"), inf.Config.URL.Scheme+"://"+r.Host+"/api/1.4/jobs?id="+strconv.FormatUint(uint64(*result.ID), 10))
-	w.WriteHeader(http.StatusOK)
-	w.Write(append(resp, '\n'))
-
+	api.WriteAlertsObj(w, r, http.StatusOK, alerts, result)
 	api.CreateChangeLogRawTx(api.ApiChange, api.Created+"content invalidation job: #"+strconv.FormatUint(*result.ID, 10), inf.User, inf.Tx.Tx)
 }
 
 // Gets all jobs that were created by the requesting user, and returns them in
 // in a special format encoded in the tc.UserInvalidationJob structure
 func GetUserJobs(w http.ResponseWriter, r *http.Request) {
+	alerts := tc.CreateAlerts(tc.WarnLevel, "This endpoint is deprecated, please use the 'userId' or 'createdBy' query parameters of a GET request to /jobs instead")
+
 	inf, userErr, sysErr, errCode := api.NewInfo(r, nil, nil)
 	if userErr != nil || sysErr != nil {
-		api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+		userErr = api.LogErr(r, errCode, userErr, sysErr)
+		alerts.AddNewAlert(tc.ErrorLevel, userErr.Error())
+		api.WriteAlerts(w, r, errCode, alerts)
 		return
 	}
 	defer inf.Close()
 
 	rows, err := inf.Tx.Query(userReadQuery, inf.User.ID)
 	if err != nil {
-		api.HandleErr(w, r, inf.Tx.Tx, http.StatusInternalServerError, nil, fmt.Errorf("Fetching user jobs: %v", err))
+		userErr = api.LogErr(r, http.StatusInternalServerError, nil, fmt.Errorf("Fetching user jobs: %v", err))
+		alerts.AddNewAlert(tc.ErrorLevel, userErr.Error())
+		// if err := inf.Tx.Tx.Rollback(); err != nil && err != sql.ErrTxDone {
 
 Review comment:
   nah. It's gone.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [trafficcontrol] mhoppa commented on a change in pull request #4365: Add missing deprecation notices to /user/current/jobs

Posted by GitBox <gi...@apache.org>.
mhoppa commented on a change in pull request #4365: Add missing deprecation notices to /user/current/jobs
URL: https://github.com/apache/trafficcontrol/pull/4365#discussion_r373700298
 
 

 ##########
 File path: traffic_ops/traffic_ops_golang/invalidationjobs/userinvalidationjobs.go
 ##########
 @@ -106,52 +114,53 @@ func CreateUserJob(w http.ResponseWriter, r *http.Request) {
 		&result.StartTime)
 	if err != nil {
 		userErr, sysErr, code := api.ParseDBError(err)
-		api.HandleErr(w, r, inf.Tx.Tx, code, userErr, sysErr)
+		userErr = api.LogErr(r, code, userErr, sysErr)
+		if err := inf.Tx.Tx.Rollback(); err != nil && err != sql.ErrTxDone {
+			log.Errorln("rolling back transaction: " + err.Error())
+		}
+		alerts.AddNewAlert(tc.ErrorLevel, userErr.Error())
+		api.WriteAlerts(w, r, code, alerts)
 		return
 	}
 
 	if err := setRevalFlags(*job.DSID, inf.Tx.Tx); err != nil {
-		api.HandleErr(w, r, inf.Tx.Tx, http.StatusInternalServerError, nil, fmt.Errorf("setting reval flags: %v", err))
-		return
-	}
-
-	respObj := apiResponse{
-		[]tc.Alert{
-			tc.Alert{
-				Level: tc.SuccessLevel.String(),
-				Text:  "Invalidation Job creation was successful.",
-			},
-			api.DeprecationWarning("POST /jobs"),
-		},
-		result,
-	}
-	resp, err := json.Marshal(respObj)
-	if err != nil {
-		api.HandleErr(w, r, inf.Tx.Tx, http.StatusInternalServerError, nil, fmt.Errorf("Marshaling JSON: %v", err))
+		errCode = http.StatusInternalServerError
+		alerts.AddNewAlert(tc.ErrorLevel, api.LogErr(r, errCode, nil, fmt.Errorf("setting reval flags: %v", err)).Error())
+		if err := inf.Tx.Tx.Rollback(); err != nil && err != sql.ErrTxDone {
+			log.Errorln("rolling back transaction: " + err.Error())
+		}
+		api.WriteAlerts(w, r, errCode, alerts)
 		return
 	}
 
-	w.Header().Set(rfc.ContentType, rfc.ApplicationJSON)
+	alerts.AddNewAlert(tc.SuccessLevel, "Invalidation Job creation was successful")
 	w.Header().Set(http.CanonicalHeaderKey("location"), inf.Config.URL.Scheme+"://"+r.Host+"/api/1.4/jobs?id="+strconv.FormatUint(uint64(*result.ID), 10))
-	w.WriteHeader(http.StatusOK)
-	w.Write(append(resp, '\n'))
-
+	api.WriteAlertsObj(w, r, http.StatusOK, alerts, result)
 	api.CreateChangeLogRawTx(api.ApiChange, api.Created+"content invalidation job: #"+strconv.FormatUint(*result.ID, 10), inf.User, inf.Tx.Tx)
 }
 
 // Gets all jobs that were created by the requesting user, and returns them in
 // in a special format encoded in the tc.UserInvalidationJob structure
 func GetUserJobs(w http.ResponseWriter, r *http.Request) {
+	alerts := tc.CreateAlerts(tc.WarnLevel, "This endpoint is deprecated, please use the 'userId' or 'createdBy' query parameters of a GET request to /jobs instead")
+
 	inf, userErr, sysErr, errCode := api.NewInfo(r, nil, nil)
 	if userErr != nil || sysErr != nil {
-		api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+		userErr = api.LogErr(r, errCode, userErr, sysErr)
+		alerts.AddNewAlert(tc.ErrorLevel, userErr.Error())
+		api.WriteAlerts(w, r, errCode, alerts)
 		return
 	}
 	defer inf.Close()
 
 	rows, err := inf.Tx.Query(userReadQuery, inf.User.ID)
 	if err != nil {
-		api.HandleErr(w, r, inf.Tx.Tx, http.StatusInternalServerError, nil, fmt.Errorf("Fetching user jobs: %v", err))
+		userErr = api.LogErr(r, http.StatusInternalServerError, nil, fmt.Errorf("Fetching user jobs: %v", err))
+		alerts.AddNewAlert(tc.ErrorLevel, userErr.Error())
+		// if err := inf.Tx.Tx.Rollback(); err != nil && err != sql.ErrTxDone {
 
 Review comment:
   still here?

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [trafficcontrol] mitchell852 merged pull request #4365: Add missing deprecation notices to /user/current/jobs

Posted by GitBox <gi...@apache.org>.
mitchell852 merged pull request #4365: Add missing deprecation notices to /user/current/jobs
URL: https://github.com/apache/trafficcontrol/pull/4365
 
 
   

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [trafficcontrol] mhoppa commented on a change in pull request #4365: Add missing deprecation notices to /user/current/jobs

Posted by GitBox <gi...@apache.org>.
mhoppa commented on a change in pull request #4365: Add missing deprecation notices to /user/current/jobs
URL: https://github.com/apache/trafficcontrol/pull/4365#discussion_r373671744
 
 

 ##########
 File path: traffic_ops/traffic_ops_golang/invalidationjobs/userinvalidationjobs.go
 ##########
 @@ -106,52 +114,53 @@ func CreateUserJob(w http.ResponseWriter, r *http.Request) {
 		&result.StartTime)
 	if err != nil {
 		userErr, sysErr, code := api.ParseDBError(err)
-		api.HandleErr(w, r, inf.Tx.Tx, code, userErr, sysErr)
+		userErr = api.LogErr(r, code, userErr, sysErr)
+		if err := inf.Tx.Tx.Rollback(); err != nil && err != sql.ErrTxDone {
+			log.Errorln("rolling back transaction: " + err.Error())
+		}
+		alerts.AddNewAlert(tc.ErrorLevel, userErr.Error())
+		api.WriteAlerts(w, r, code, alerts)
 		return
 	}
 
 	if err := setRevalFlags(*job.DSID, inf.Tx.Tx); err != nil {
-		api.HandleErr(w, r, inf.Tx.Tx, http.StatusInternalServerError, nil, fmt.Errorf("setting reval flags: %v", err))
-		return
-	}
-
-	respObj := apiResponse{
-		[]tc.Alert{
-			tc.Alert{
-				Level: tc.SuccessLevel.String(),
-				Text:  "Invalidation Job creation was successful.",
-			},
-			api.DeprecationWarning("POST /jobs"),
-		},
-		result,
-	}
-	resp, err := json.Marshal(respObj)
-	if err != nil {
-		api.HandleErr(w, r, inf.Tx.Tx, http.StatusInternalServerError, nil, fmt.Errorf("Marshaling JSON: %v", err))
+		errCode = http.StatusInternalServerError
+		alerts.AddNewAlert(tc.ErrorLevel, api.LogErr(r, errCode, nil, fmt.Errorf("setting reval flags: %v", err)).Error())
+		if err := inf.Tx.Tx.Rollback(); err != nil && err != sql.ErrTxDone {
+			log.Errorln("rolling back transaction: " + err.Error())
+		}
+		api.WriteAlerts(w, r, errCode, alerts)
 		return
 	}
 
-	w.Header().Set(rfc.ContentType, rfc.ApplicationJSON)
+	alerts.AddNewAlert(tc.SuccessLevel, "Invalidation Job creation was successful")
 	w.Header().Set(http.CanonicalHeaderKey("location"), inf.Config.URL.Scheme+"://"+r.Host+"/api/1.4/jobs?id="+strconv.FormatUint(uint64(*result.ID), 10))
-	w.WriteHeader(http.StatusOK)
-	w.Write(append(resp, '\n'))
-
+	api.WriteAlertsObj(w, r, http.StatusOK, alerts, result)
 	api.CreateChangeLogRawTx(api.ApiChange, api.Created+"content invalidation job: #"+strconv.FormatUint(*result.ID, 10), inf.User, inf.Tx.Tx)
 }
 
 // Gets all jobs that were created by the requesting user, and returns them in
 // in a special format encoded in the tc.UserInvalidationJob structure
 func GetUserJobs(w http.ResponseWriter, r *http.Request) {
+	alerts := tc.CreateAlerts(tc.WarnLevel, "This endpoint is deprecated, please use the 'userId' or 'createdBy' query parameters of a GET request to /jobs instead")
+
 	inf, userErr, sysErr, errCode := api.NewInfo(r, nil, nil)
 	if userErr != nil || sysErr != nil {
-		api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+		userErr = api.LogErr(r, errCode, userErr, sysErr)
+		alerts.AddNewAlert(tc.ErrorLevel, userErr.Error())
+		api.WriteAlerts(w, r, errCode, alerts)
 		return
 	}
 	defer inf.Close()
 
 	rows, err := inf.Tx.Query(userReadQuery, inf.User.ID)
 	if err != nil {
-		api.HandleErr(w, r, inf.Tx.Tx, http.StatusInternalServerError, nil, fmt.Errorf("Fetching user jobs: %v", err))
+		userErr = api.LogErr(r, http.StatusInternalServerError, nil, fmt.Errorf("Fetching user jobs: %v", err))
+		alerts.AddNewAlert(tc.ErrorLevel, userErr.Error())
+		// if err := inf.Tx.Tx.Rollback(); err != nil && err != sql.ErrTxDone {
 
 Review comment:
   remove commented out code please 

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services