]> source.dussan.org Git - gitea.git/commitdiff
Correctly set the organization num repos (#11339)
authorzeripath <art27@cantab.net>
Mon, 11 May 2020 22:04:08 +0000 (23:04 +0100)
committerGitHub <noreply@github.com>
Mon, 11 May 2020 22:04:08 +0000 (01:04 +0300)
* Correctly set the organization num repos

Correctly set the organization num repos to the number of
accessible repos for the user

Fix #11194

Signed-off-by: Andrew Thornton <art27@cantab.net>
* as per @lunny

Signed-off-by: Andrew Thornton <art27@cantab.net>
* attempt to fix mssql

Signed-off-by: Andrew Thornton <art27@cantab.net>
* Update models/user.go

* Explicit columns

Signed-off-by: Andrew Thornton <art27@cantab.net>
* Add test and fix 0 counted orgs

Signed-off-by: Andrew Thornton <art27@cantab.net>
* remove orgname from api

Signed-off-by: Andrew Thornton <art27@cantab.net>
integrations/api_helper_for_declarative_test.go
integrations/org_count_test.go [new file with mode: 0644]
models/user.go
routers/api/v1/api.go

index cae7691c4b96171be7ab7b09bf7b2c91b417a6b6..ec7f1d74969056167dc7faedf63f6e1e2ba13849 100644 (file)
@@ -266,3 +266,86 @@ func doAPICreateFile(ctx APITestContext, treepath string, options *api.CreateFil
                }
        }
 }
+
+func doAPICreateOrganization(ctx APITestContext, options *api.CreateOrgOption, callback ...func(*testing.T, api.Organization)) func(t *testing.T) {
+       return func(t *testing.T) {
+               url := fmt.Sprintf("/api/v1/orgs?token=%s", ctx.Token)
+
+               req := NewRequestWithJSON(t, "POST", url, &options)
+               if ctx.ExpectedCode != 0 {
+                       ctx.Session.MakeRequest(t, req, ctx.ExpectedCode)
+                       return
+               }
+               resp := ctx.Session.MakeRequest(t, req, http.StatusCreated)
+
+               var contents api.Organization
+               DecodeJSON(t, resp, &contents)
+               if len(callback) > 0 {
+                       callback[0](t, contents)
+               }
+       }
+}
+
+func doAPICreateOrganizationRepository(ctx APITestContext, orgName string, options *api.CreateRepoOption, callback ...func(*testing.T, api.Repository)) func(t *testing.T) {
+       return func(t *testing.T) {
+               url := fmt.Sprintf("/api/v1/orgs/%s/repos?token=%s", orgName, ctx.Token)
+
+               req := NewRequestWithJSON(t, "POST", url, &options)
+               if ctx.ExpectedCode != 0 {
+                       ctx.Session.MakeRequest(t, req, ctx.ExpectedCode)
+                       return
+               }
+               resp := ctx.Session.MakeRequest(t, req, http.StatusCreated)
+
+               var contents api.Repository
+               DecodeJSON(t, resp, &contents)
+               if len(callback) > 0 {
+                       callback[0](t, contents)
+               }
+       }
+}
+
+func doAPICreateOrganizationTeam(ctx APITestContext, orgName string, options *api.CreateTeamOption, callback ...func(*testing.T, api.Team)) func(t *testing.T) {
+       return func(t *testing.T) {
+               url := fmt.Sprintf("/api/v1/orgs/%s/teams?token=%s", orgName, ctx.Token)
+
+               req := NewRequestWithJSON(t, "POST", url, &options)
+               if ctx.ExpectedCode != 0 {
+                       ctx.Session.MakeRequest(t, req, ctx.ExpectedCode)
+                       return
+               }
+               resp := ctx.Session.MakeRequest(t, req, http.StatusCreated)
+
+               var contents api.Team
+               DecodeJSON(t, resp, &contents)
+               if len(callback) > 0 {
+                       callback[0](t, contents)
+               }
+       }
+}
+
+func doAPIAddUserToOrganizationTeam(ctx APITestContext, teamID int64, username string) func(t *testing.T) {
+       return func(t *testing.T) {
+               url := fmt.Sprintf("/api/v1/teams/%d/members/%s?token=%s", teamID, username, ctx.Token)
+
+               req := NewRequest(t, "PUT", url)
+               if ctx.ExpectedCode != 0 {
+                       ctx.Session.MakeRequest(t, req, ctx.ExpectedCode)
+                       return
+               }
+               ctx.Session.MakeRequest(t, req, http.StatusNoContent)
+       }
+}
+
+func doAPIAddRepoToOrganizationTeam(ctx APITestContext, teamID int64, orgName, repoName string) func(t *testing.T) {
+       return func(t *testing.T) {
+               url := fmt.Sprintf("/api/v1/teams/%d/repos/%s/%s?token=%s", teamID, orgName, repoName, ctx.Token)
+
+               req := NewRequest(t, "PUT", url)
+               if ctx.ExpectedCode != 0 {
+                       ctx.Session.MakeRequest(t, req, ctx.ExpectedCode)
+                       return
+               }
+               ctx.Session.MakeRequest(t, req, http.StatusNoContent)
+       }
+}
diff --git a/integrations/org_count_test.go b/integrations/org_count_test.go
new file mode 100644 (file)
index 0000000..755ee3c
--- /dev/null
@@ -0,0 +1,140 @@
+// Copyright 2020 The Gitea Authors. All rights reserved.
+// Use of this source code is governed by a MIT-style
+// license that can be found in the LICENSE file.
+
+package integrations
+
+import (
+       "net/url"
+       "strings"
+       "testing"
+
+       "code.gitea.io/gitea/models"
+       api "code.gitea.io/gitea/modules/structs"
+       "github.com/stretchr/testify/assert"
+)
+
+func TestOrgCounts(t *testing.T) {
+       onGiteaRun(t, testOrgCounts)
+}
+
+func testOrgCounts(t *testing.T, u *url.URL) {
+       orgOwner := "user2"
+       orgName := "testOrg"
+       orgCollaborator := "user4"
+       ctx := NewAPITestContext(t, orgOwner, "repo1")
+
+       var ownerCountRepos map[string]int
+       var collabCountRepos map[string]int
+
+       t.Run("GetTheOwnersNumRepos", doCheckOrgCounts(orgOwner, map[string]int{},
+               false,
+               func(_ *testing.T, calcOrgCounts map[string]int) {
+                       ownerCountRepos = calcOrgCounts
+               },
+       ))
+       t.Run("GetTheCollaboratorsNumRepos", doCheckOrgCounts(orgCollaborator, map[string]int{},
+               false,
+               func(_ *testing.T, calcOrgCounts map[string]int) {
+                       collabCountRepos = calcOrgCounts
+               },
+       ))
+
+       t.Run("CreatePublicTestOrganization", doAPICreateOrganization(ctx, &api.CreateOrgOption{
+               UserName:   orgName,
+               Visibility: "public",
+       }))
+
+       // Following the creation of the organization, the orgName must appear in the counts with 0 repos
+       ownerCountRepos[orgName] = 0
+
+       t.Run("AssertNumRepos0ForTestOrg", doCheckOrgCounts(orgOwner, ownerCountRepos, true))
+
+       // the collaborator is not a collaborator yet
+       t.Run("AssertNoTestOrgReposForCollaborator", doCheckOrgCounts(orgCollaborator, collabCountRepos, true))
+
+       t.Run("CreateOrganizationPrivateRepo", doAPICreateOrganizationRepository(ctx, orgName, &api.CreateRepoOption{
+               Name:     "privateTestRepo",
+               AutoInit: true,
+               Private:  true,
+       }))
+
+       ownerCountRepos[orgName] = 1
+       t.Run("AssertNumRepos1ForTestOrg", doCheckOrgCounts(orgOwner, ownerCountRepos, true))
+
+       t.Run("AssertNoTestOrgReposForCollaborator", doCheckOrgCounts(orgCollaborator, collabCountRepos, true))
+
+       var testTeam api.Team
+
+       t.Run("CreateTeamForPublicTestOrganization", doAPICreateOrganizationTeam(ctx, orgName, &api.CreateTeamOption{
+               Name:             "test",
+               Permission:       "read",
+               Units:            []string{"repo.code", "repo.issues", "repo.wiki", "repo.pulls", "repo.releases"},
+               CanCreateOrgRepo: true,
+       }, func(_ *testing.T, team api.Team) {
+               testTeam = team
+       }))
+
+       t.Run("AssertNoTestOrgReposForCollaborator", doCheckOrgCounts(orgCollaborator, collabCountRepos, true))
+
+       t.Run("AddCollboratorToTeam", doAPIAddUserToOrganizationTeam(ctx, testTeam.ID, orgCollaborator))
+
+       collabCountRepos[orgName] = 0
+       t.Run("AssertNumRepos0ForTestOrgForCollaborator", doCheckOrgCounts(orgOwner, ownerCountRepos, true))
+
+       // Now create a Public Repo
+       t.Run("CreateOrganizationPublicRepo", doAPICreateOrganizationRepository(ctx, orgName, &api.CreateRepoOption{
+               Name:     "publicTestRepo",
+               AutoInit: true,
+       }))
+
+       ownerCountRepos[orgName] = 2
+       t.Run("AssertNumRepos2ForTestOrg", doCheckOrgCounts(orgOwner, ownerCountRepos, true))
+       collabCountRepos[orgName] = 1
+       t.Run("AssertNumRepos1ForTestOrgForCollaborator", doCheckOrgCounts(orgOwner, ownerCountRepos, true))
+
+       // Now add the testTeam to the privateRepo
+       t.Run("AddTestTeamToPrivateRepo", doAPIAddRepoToOrganizationTeam(ctx, testTeam.ID, orgName, "privateTestRepo"))
+
+       t.Run("AssertNumRepos2ForTestOrg", doCheckOrgCounts(orgOwner, ownerCountRepos, true))
+       collabCountRepos[orgName] = 2
+       t.Run("AssertNumRepos2ForTestOrgForCollaborator", doCheckOrgCounts(orgOwner, ownerCountRepos, true))
+}
+
+func doCheckOrgCounts(username string, orgCounts map[string]int, strict bool, callback ...func(*testing.T, map[string]int)) func(t *testing.T) {
+       canonicalCounts := make(map[string]int, len(orgCounts))
+
+       for key, value := range orgCounts {
+               newKey := strings.TrimSpace(strings.ToLower(key))
+               canonicalCounts[newKey] = value
+       }
+
+       return func(t *testing.T) {
+               user := models.AssertExistsAndLoadBean(t, &models.User{
+                       Name: username,
+               }).(*models.User)
+
+               user.GetOrganizations(&models.SearchOrganizationsOptions{All: true})
+
+               calcOrgCounts := map[string]int{}
+
+               for _, org := range user.Orgs {
+                       calcOrgCounts[org.LowerName] = org.NumRepos
+                       count, ok := canonicalCounts[org.LowerName]
+                       if ok {
+                               assert.True(t, count == org.NumRepos, "Number of Repos in %s is %d when we expected %d", org.Name, org.NumRepos, count)
+                       } else {
+                               assert.False(t, strict, "Did not expect to see %s with count %d", org.Name, org.NumRepos)
+                       }
+               }
+
+               for key, value := range orgCounts {
+                       _, seen := calcOrgCounts[strings.TrimSpace(strings.ToLower(key))]
+                       assert.True(t, seen, "Expected to see %s with %d but did not", key, value)
+               }
+
+               if len(callback) > 0 {
+                       callback[0](t, calcOrgCounts)
+               }
+       }
+}
index a89eb144cedc4cdd1f1be29c6be62251ecf1a0c6..01cce2506d23e5678d87943ca2bc772d3ad4e3e9 100644 (file)
@@ -712,18 +712,52 @@ func (u *User) GetOwnedOrganizations() (err error) {
 
 // GetOrganizations returns paginated organizations that user belongs to.
 func (u *User) GetOrganizations(opts *SearchOrganizationsOptions) error {
-       ous, err := GetOrgUsersByUserID(u.ID, opts)
+       sess := x.NewSession()
+       defer sess.Close()
+
+       schema, err := x.TableInfo(new(User))
        if err != nil {
                return err
        }
+       groupByCols := &strings.Builder{}
+       for _, col := range schema.Columns() {
+               fmt.Fprintf(groupByCols, "`%s`.%s,", schema.Name, col.Name)
+       }
+       groupByStr := groupByCols.String()
+       groupByStr = groupByStr[0 : len(groupByStr)-1]
 
-       u.Orgs = make([]*User, len(ous))
-       for i, ou := range ous {
-               u.Orgs[i], err = GetUserByID(ou.OrgID)
-               if err != nil {
-                       return err
-               }
+       sess.Select("`user`.*, count(repo_id) as org_count").
+               Table("user").
+               Join("INNER", "org_user", "`org_user`.org_id=`user`.id").
+               Join("LEFT", builder.
+                       Select("id as repo_id, owner_id as repo_owner_id").
+                       From("repository").
+                       Where(accessibleRepositoryCondition(u)), "`repository`.repo_owner_id = `org_user`.org_id").
+               And("`org_user`.uid=?", u.ID).
+               GroupBy(groupByStr)
+       if opts.PageSize != 0 {
+               sess = opts.setSessionPagination(sess)
        }
+       type OrgCount struct {
+               User     `xorm:"extends"`
+               OrgCount int
+       }
+       orgCounts := make([]*OrgCount, 0, 10)
+
+       if err := sess.
+               Asc("`user`.name").
+               Find(&orgCounts); err != nil {
+               return err
+       }
+
+       orgs := make([]*User, len(orgCounts))
+       for i, orgCount := range orgCounts {
+               orgCount.User.NumRepos = orgCount.OrgCount
+               orgs[i] = &orgCount.User
+       }
+
+       u.Orgs = orgs
+
        return nil
 }
 
index 6973e1df53186fd9d876df31d7708277cf955326..0d62b751cc89881146874d8ad17b3f5c1c4e6fac 100644 (file)
@@ -383,7 +383,7 @@ func orgAssignment(args ...bool) macaron.Handler {
 
                var err error
                if assignOrg {
-                       ctx.Org.Organization, err = models.GetOrgByName(ctx.Params(":orgname"))
+                       ctx.Org.Organization, err = models.GetOrgByName(ctx.Params(":org"))
                        if err != nil {
                                if models.IsErrOrgNotExist(err) {
                                        ctx.NotFound()
@@ -857,7 +857,7 @@ func RegisterRoutes(m *macaron.Macaron) {
                m.Get("/users/:username/orgs", org.ListUserOrgs)
                m.Post("/orgs", reqToken(), bind(api.CreateOrgOption{}), org.Create)
                m.Get("/orgs", org.GetAll)
-               m.Group("/orgs/:orgname", func() {
+               m.Group("/orgs/:org", func() {
                        m.Combo("").Get(org.Get).
                                Patch(reqToken(), reqOrgOwnership(), bind(api.EditOrgOption{}), org.Edit).
                                Delete(reqToken(), reqOrgOwnership(), org.Delete)
@@ -907,7 +907,7 @@ func RegisterRoutes(m *macaron.Macaron) {
                        })
                        m.Group("/repos", func() {
                                m.Get("", org.GetTeamRepos)
-                               m.Combo("/:orgname/:reponame").
+                               m.Combo("/:org/:reponame").
                                        Put(org.AddTeamRepository).
                                        Delete(org.RemoveTeamRepository)
                        })