Fix delete nonexist oauth application 500 and prevent deadlock (#15384)
* Fix delete nonexist oauth application 500 * Fix test * Close the session Signed-off-by: Andrew Thornton <art27@cantab.net> * Update integrations/api_oauth2_apps_test.go * Fix more missed sess.Close * Remove unnecessary blank line Co-authored-by: Andrew Thornton <art27@cantab.net> Co-authored-by: 6543 <6543@obermui.de>
This commit is contained in:
		
							parent
							
								
									c680eb2cc7
								
							
						
					
					
						commit
						1fc1d60517
					
				
					 4 changed files with 13 additions and 2 deletions
				
			
		|  | @ -92,6 +92,10 @@ func testAPIDeleteOAuth2Application(t *testing.T) { | ||||||
| 	session.MakeRequest(t, req, http.StatusNoContent) | 	session.MakeRequest(t, req, http.StatusNoContent) | ||||||
| 
 | 
 | ||||||
| 	models.AssertNotExistsBean(t, &models.OAuth2Application{UID: oldApp.UID, Name: oldApp.Name}) | 	models.AssertNotExistsBean(t, &models.OAuth2Application{UID: oldApp.UID, Name: oldApp.Name}) | ||||||
|  | 
 | ||||||
|  | 	// Delete again will return not found
 | ||||||
|  | 	req = NewRequest(t, "DELETE", urlStr) | ||||||
|  | 	session.MakeRequest(t, req, http.StatusNotFound) | ||||||
| } | } | ||||||
| 
 | 
 | ||||||
| func testAPIGetOAuth2Application(t *testing.T) { | func testAPIGetOAuth2Application(t *testing.T) { | ||||||
|  |  | ||||||
|  | @ -39,6 +39,7 @@ func InsertMilestones(ms ...*Milestone) (err error) { | ||||||
| // InsertIssues insert issues to database
 | // InsertIssues insert issues to database
 | ||||||
| func InsertIssues(issues ...*Issue) error { | func InsertIssues(issues ...*Issue) error { | ||||||
| 	sess := x.NewSession() | 	sess := x.NewSession() | ||||||
|  | 	defer sess.Close() | ||||||
| 	if err := sess.Begin(); err != nil { | 	if err := sess.Begin(); err != nil { | ||||||
| 		return err | 		return err | ||||||
| 	} | 	} | ||||||
|  | @ -194,6 +195,7 @@ func InsertPullRequests(prs ...*PullRequest) error { | ||||||
| // InsertReleases migrates release
 | // InsertReleases migrates release
 | ||||||
| func InsertReleases(rels ...*Release) error { | func InsertReleases(rels ...*Release) error { | ||||||
| 	sess := x.NewSession() | 	sess := x.NewSession() | ||||||
|  | 	defer sess.Close() | ||||||
| 	if err := sess.Begin(); err != nil { | 	if err := sess.Begin(); err != nil { | ||||||
| 		return err | 		return err | ||||||
| 	} | 	} | ||||||
|  |  | ||||||
|  | @ -235,7 +235,7 @@ func deleteOAuth2Application(sess *xorm.Session, id, userid int64) error { | ||||||
| 	if deleted, err := sess.Delete(&OAuth2Application{ID: id, UID: userid}); err != nil { | 	if deleted, err := sess.Delete(&OAuth2Application{ID: id, UID: userid}); err != nil { | ||||||
| 		return err | 		return err | ||||||
| 	} else if deleted == 0 { | 	} else if deleted == 0 { | ||||||
| 		return fmt.Errorf("cannot find oauth2 application") | 		return ErrOAuthApplicationNotFound{ID: id} | ||||||
| 	} | 	} | ||||||
| 	codes := make([]*OAuth2AuthorizationCode, 0) | 	codes := make([]*OAuth2AuthorizationCode, 0) | ||||||
| 	// delete correlating auth codes
 | 	// delete correlating auth codes
 | ||||||
|  | @ -261,6 +261,7 @@ func deleteOAuth2Application(sess *xorm.Session, id, userid int64) error { | ||||||
| // DeleteOAuth2Application deletes the application with the given id and the grants and auth codes related to it. It checks if the userid was the creator of the app.
 | // DeleteOAuth2Application deletes the application with the given id and the grants and auth codes related to it. It checks if the userid was the creator of the app.
 | ||||||
| func DeleteOAuth2Application(id, userid int64) error { | func DeleteOAuth2Application(id, userid int64) error { | ||||||
| 	sess := x.NewSession() | 	sess := x.NewSession() | ||||||
|  | 	defer sess.Close() | ||||||
| 	if err := sess.Begin(); err != nil { | 	if err := sess.Begin(); err != nil { | ||||||
| 		return err | 		return err | ||||||
| 	} | 	} | ||||||
|  |  | ||||||
|  | @ -274,7 +274,11 @@ func DeleteOauth2Application(ctx *context.APIContext) { | ||||||
| 	//     "$ref": "#/responses/empty"
 | 	//     "$ref": "#/responses/empty"
 | ||||||
| 	appID := ctx.ParamsInt64(":id") | 	appID := ctx.ParamsInt64(":id") | ||||||
| 	if err := models.DeleteOAuth2Application(appID, ctx.User.ID); err != nil { | 	if err := models.DeleteOAuth2Application(appID, ctx.User.ID); err != nil { | ||||||
|  | 		if models.IsErrOAuthApplicationNotFound(err) { | ||||||
|  | 			ctx.NotFound() | ||||||
|  | 		} else { | ||||||
| 			ctx.Error(http.StatusInternalServerError, "DeleteOauth2ApplicationByID", err) | 			ctx.Error(http.StatusInternalServerError, "DeleteOauth2ApplicationByID", err) | ||||||
|  | 		} | ||||||
| 		return | 		return | ||||||
| 	} | 	} | ||||||
| 
 | 
 | ||||||
|  |  | ||||||
		Loading…
	
		Reference in a new issue