enhance(api/scm)!: return affected repos in API response and do not a…

…ssume 404 on GetRepo error (#1015)

* enhance(api/scm): return affected repos in API response and do not assume 404 on error

* add new responses to API spec

---------

Co-authored-by: Kelly Merrick <[email protected]>

api/repo/create.go

@@ -97,7 +97,7 @@ func CreateRepo(c *gin.Context) {
 	}).Infof("creating new repo %s", input.GetFullName())
 
 	// get repo information from the source
-	r, err := scm.FromContext(c).GetRepo(ctx, u, input)
+	r, _, err := scm.FromContext(c).GetRepo(ctx, u, input)
 	if err != nil {
 		retErr := fmt.Errorf("unable to retrieve repo info for %s from source: %w", r.GetFullName(), err)
 

api/scm/sync.go

@@ -41,7 +41,17 @@ import (
 //   '200':
 //     description: Successfully synchronized repo
 //     schema:
-//     type: string
+//       "$ref": "#/definitions/Repo"
+//   '204':
+//     description: Successful request resulting in no change
+//   '301':
+//     description: Repo has moved permanently
+//     schema:
+//       "$ref": "#/definitions/Error"
+//   '403':
+//     description: User has been forbidden access to repository
+//     schema:
+//       "$ref": "#/definitions/Error"
 //   '500':
 //     description: Unable to synchronize repo
 //     schema:
@@ -71,25 +81,33 @@ func SyncRepo(c *gin.Context) {
 	logger.Infof("syncing repo %s", r.GetFullName())
 
 	// retrieve repo from source code manager service
-	_, err := scm.FromContext(c).GetRepo(ctx, u, r)
+	_, respCode, err := scm.FromContext(c).GetRepo(ctx, u, r)
 
 	// if there is an error retrieving repo, we know it is deleted: set to inactive
 	if err != nil {
-		// set repo to inactive - do not delete
-		r.SetActive(false)
+		if respCode == http.StatusNotFound {
+			// set repo to inactive - do not delete
+			r.SetActive(false)
 
-		// update repo in database
-		_, err := database.FromContext(c).UpdateRepo(ctx, r)
-		if err != nil {
-			retErr := fmt.Errorf("unable to update repo for org %s: %w", o, err)
+			// update repo in database
+			r, err = database.FromContext(c).UpdateRepo(ctx, r)
+			if err != nil {
+				retErr := fmt.Errorf("unable to update repo for org %s: %w", o, err)
 
-			util.HandleError(c, http.StatusInternalServerError, retErr)
+				util.HandleError(c, http.StatusInternalServerError, retErr)
+
+				return
+			}
+
+			// exit with success as hook sync will be unnecessary
+			c.JSON(http.StatusOK, r)
 
 			return
 		}
 
-		// exit with success as hook sync will be unnecessary
-		c.JSON(http.StatusOK, fmt.Sprintf("repo %s synced", r.GetFullName()))
+		retErr := fmt.Errorf("error while retrieving repo %s from %s: %w", r.GetFullName(), scm.FromContext(c).Driver(), err)
+
+		util.HandleError(c, respCode, retErr)
 
 		return
 	}
@@ -130,7 +148,7 @@ func SyncRepo(c *gin.Context) {
 			if !webhookExists {
 				r.SetActive(false)
 
-				_, err := database.FromContext(c).UpdateRepo(ctx, r)
+				r, err = database.FromContext(c).UpdateRepo(ctx, r)
 				if err != nil {
 					retErr := fmt.Errorf("unable to update repo for org %s: %w", o, err)
 
@@ -139,7 +157,7 @@ func SyncRepo(c *gin.Context) {
 					return
 				}
 
-				c.JSON(http.StatusOK, fmt.Sprintf("webhook not found, repo %s deactivated", r.GetFullName()))
+				c.JSON(http.StatusOK, r)
 
 				return
 			}
@@ -152,5 +170,5 @@ func SyncRepo(c *gin.Context) {
 		}
 	}
 
-	c.JSON(http.StatusOK, fmt.Sprintf("repo %s synced", r.GetFullName()))
+	c.Status(http.StatusNoContent)
 }

api/scm/sync_org.go

@@ -35,7 +35,19 @@ import (
 //   '200':
 //     description: Successfully synchronized repos
 //     schema:
-//       type: string
+//       type: array
+//       items:
+//         "$ref": "#/definitions/Repo"
+//   '204':
+//     description: Successful request resulting in no change
+//   '301':
+//     description: One repo in the org has moved permanently
+//     schema:
+//       "$ref": "#/definitions/Error"
+//   '403':
+//     description: User has been forbidden access to at least one repository in org
+//     schema:
+//       "$ref": "#/definitions/Error"
 //   '500':
 //     description: Unable to synchronize org repositories
 //     schema:
@@ -105,18 +117,28 @@ func SyncReposForOrg(c *gin.Context) {
 		page++
 	}
 
+	var results []*library.Repo
+
 	// iterate through captured repos and check if they are in GitHub
 	for _, repo := range repos {
-		_, err := scm.FromContext(c).GetRepo(ctx, u, repo)
+		_, respCode, err := scm.FromContext(c).GetRepo(ctx, u, repo)
 		// if repo cannot be captured from GitHub, set to inactive in database
 		if err != nil {
-			repo.SetActive(false)
+			if respCode == http.StatusNotFound {
+				_, err := database.FromContext(c).UpdateRepo(ctx, repo)
+				if err != nil {
+					retErr := fmt.Errorf("unable to update repo for org %s: %w", o, err)
 
-			_, err := database.FromContext(c).UpdateRepo(ctx, repo)
-			if err != nil {
-				retErr := fmt.Errorf("unable to update repo for org %s: %w", o, err)
+					util.HandleError(c, http.StatusInternalServerError, retErr)
 
-				util.HandleError(c, http.StatusInternalServerError, retErr)
+					return
+				}
+
+				results = append(results, repo)
+			} else {
+				retErr := fmt.Errorf("error while retrieving repo %s from %s: %w", repo.GetFullName(), scm.FromContext(c).Driver(), err)
+
+				util.HandleError(c, respCode, retErr)
 
 				return
 			}
@@ -152,6 +174,8 @@ func SyncReposForOrg(c *gin.Context) {
 						return
 					}
 
+					results = append(results, repo)
+
 					continue
 				}
 
@@ -164,5 +188,12 @@ func SyncReposForOrg(c *gin.Context) {
 		}
 	}
 
-	c.JSON(http.StatusOK, fmt.Sprintf("org %s repos synced", o))
+	// if no repo was changed, return no content status
+	if len(results) == 0 {
+		c.Status(http.StatusNoContent)
+
+		return
+	}
+
+	c.JSON(http.StatusOK, results)
 }

scm/github/repo.go

@@ -367,7 +367,7 @@ func (c *client) Status(ctx context.Context, u *library.User, b *library.Build,
 }
 
 // GetRepo gets repo information from Github.
-func (c *client) GetRepo(ctx context.Context, u *library.User, r *library.Repo) (*library.Repo, error) {
+func (c *client) GetRepo(ctx context.Context, u *library.User, r *library.Repo) (*library.Repo, int, error) {
 	c.Logger.WithFields(logrus.Fields{
 		"org":  r.GetOrg(),
 		"repo": r.GetName(),
@@ -378,12 +378,12 @@ func (c *client) GetRepo(ctx context.Context, u *library.User, r *library.Repo)
 	client := c.newClientToken(u.GetToken())
 
 	// send an API call to get the repo info
-	repo, _, err := client.Repositories.Get(ctx, r.GetOrg(), r.GetName())
+	repo, resp, err := client.Repositories.Get(ctx, r.GetOrg(), r.GetName())
 	if err != nil {
-		return nil, err
+		return nil, resp.StatusCode, err
 	}
 
-	return toLibraryRepo(*repo), nil
+	return toLibraryRepo(*repo), resp.StatusCode, nil
 }
 
 // GetOrgAndRepoName returns the name of the org and the repository in the SCM.
@@ -557,6 +557,7 @@ func (c *client) GetBranch(ctx context.Context, u *library.User, r *library.Repo
 
 	maxRedirects := 3
 	data, _, err := client.Repositories.GetBranch(ctx, r.GetOrg(), r.GetName(), branch, maxRedirects)
+
 	if err != nil {
 		return "", "", err
 	}

scm/github/repo_test.go

@@ -1106,9 +1106,9 @@ func TestGithub_GetRepo(t *testing.T) {
 	client, _ := NewTest(s.URL)
 
 	// run test
-	got, err := client.GetRepo(context.TODO(), u, r)
+	got, code, err := client.GetRepo(context.TODO(), u, r)
 
-	if resp.Code != http.StatusOK {
+	if code != http.StatusOK {
 		t.Errorf("GetRepo returned %v, want %v", resp.Code, http.StatusOK)
 	}
 
@@ -1149,11 +1149,15 @@ func TestGithub_GetRepo_Fail(t *testing.T) {
 	client, _ := NewTest(s.URL)
 
 	// run test
-	_, err := client.GetRepo(context.TODO(), u, r)
+	_, code, err := client.GetRepo(context.TODO(), u, r)
 
 	if err == nil {
 		t.Error("GetRepo should return error")
 	}
+
+	if code != http.StatusNotFound {
+		t.Errorf("GetRepo should have returned %d status, got %d", http.StatusNotFound, code)
+	}
 }
 
 func TestGithub_GetOrgAndRepoName(t *testing.T) {

scm/service.go

@@ -118,7 +118,7 @@ type Service interface {
 	GetPullRequest(context.Context, *library.User, *library.Repo, int) (string, string, string, string, error)
 	// GetRepo defines a function that retrieves
 	// details for a repo.
-	GetRepo(context.Context, *library.User, *library.Repo) (*library.Repo, error)
+	GetRepo(context.Context, *library.User, *library.Repo) (*library.Repo, int, error)
 	// GetOrgAndRepoName defines a function that retrieves
 	// the name of the org and repo in the SCM.
 	GetOrgAndRepoName(context.Context, *library.User, string, string) (string, string, error)