From 46d19c4676efe5201c5de790bcb963bfc93a95c7 Mon Sep 17 00:00:00 2001 From: Alexey Terentyev Date: Thu, 21 Jun 2018 12:09:46 +0300 Subject: [PATCH 1/3] Fix topics addition (Another solution) (#4031) (#4258) * Added topics validation, fixed repo topics duplication (#4031) Signed-off-by: Alexey Terentyev * Added tests Signed-off-by: Alexey Terentyev * Fixed fmt Signed-off-by: Alexey Terentyev * Added comments to exported functions Signed-off-by: Alexey Terentyev * Deleted RemoveDuplicateTopics function Signed-off-by: Alexey Terentyev * Fixed messages Signed-off-by: Alexey Terentyev * Added migration Signed-off-by: Alexey Terentyev * fmt migration file Signed-off-by: Alexey Terentyev * fixed lint Signed-off-by: Alexey Terentyev * Added Copyright Signed-off-by: Alexey Terentyev * Added query solution for duplicates Signed-off-by: Alexey Terentyev * Fixed migration query Signed-off-by: Alexey Terentyev * Changed RegExp. Fixed migration Signed-off-by: Alexey Terentyev * fmt migration file Signed-off-by: Alexey Terentyev * Fixed test for changed regexp Signed-off-by: Alexey Terentyev * Removed validation log messages Signed-off-by: Alexey Terentyev * Renamed migration file Signed-off-by: Alexey Terentyev * Renamed validate function Signed-off-by: Alexey Terentyev --- models/migrations/migrations.go | 2 + models/migrations/v68.go | 160 ++++++++++++++++++++++++++++++++ models/topic.go | 15 +++ models/topic_test.go | 13 +++ options/locale/locale_en-US.ini | 2 + public/js/index.js | 4 +- routers/repo/topic.go | 35 ++++++- routers/routes/routes.go | 2 +- 8 files changed, 229 insertions(+), 4 deletions(-) create mode 100644 models/migrations/v68.go diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go index 2537e5712..7732e1709 100644 --- a/models/migrations/migrations.go +++ b/models/migrations/migrations.go @@ -188,6 +188,8 @@ var migrations = []Migration{ NewMigration("add login source id column for public_key table", addLoginSourceIDToPublicKeyTable), // v67 -> v68 NewMigration("remove stale watches", removeStaleWatches), + // v68 -> V69 + NewMigration("Reformat and remove incorrect topics", reformatAndRemoveIncorrectTopics), } // Migrate database to current version diff --git a/models/migrations/v68.go b/models/migrations/v68.go new file mode 100644 index 000000000..d6a0d04c5 --- /dev/null +++ b/models/migrations/v68.go @@ -0,0 +1,160 @@ +// Copyright 2018 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 migrations + +import ( + "strings" + + "code.gitea.io/gitea/models" + "code.gitea.io/gitea/modules/log" + + "github.com/go-xorm/xorm" +) + +func reformatAndRemoveIncorrectTopics(x *xorm.Engine) (err error) { + log.Info("This migration could take up to minutes, please be patient.") + type Topic struct { + ID int64 + Name string `xorm:"unique"` + } + + sess := x.NewSession() + defer sess.Close() + + const batchSize = 100 + touchedRepo := make(map[int64]struct{}) + topics := make([]*Topic, 0, batchSize) + delTopicIDs := make([]int64, 0, batchSize) + ids := make([]int64, 0, 30) + + if err := sess.Begin(); err != nil { + return err + } + log.Info("Validating existed topics...") + for start := 0; ; start += batchSize { + topics = topics[:0] + if err := sess.Asc("id").Limit(batchSize, start).Find(&topics); err != nil { + return err + } + if len(topics) == 0 { + break + } + for _, topic := range topics { + if models.ValidateTopic(topic.Name) { + continue + } + topic.Name = strings.Replace(strings.TrimSpace(strings.ToLower(topic.Name)), " ", "-", -1) + + if err := sess.Table("repo_topic").Cols("repo_id"). + Where("topic_id = ?", topic.ID).Find(&ids); err != nil { + return err + } + for _, id := range ids { + touchedRepo[id] = struct{}{} + } + + if models.ValidateTopic(topic.Name) { + log.Info("Updating topic: id = %v, name = %v", topic.ID, topic.Name) + if _, err := sess.Table("topic").ID(topic.ID). + Update(&Topic{Name: topic.Name}); err != nil { + return err + } + } else { + delTopicIDs = append(delTopicIDs, topic.ID) + } + } + } + + log.Info("Deleting incorrect topics...") + for start := 0; ; start += batchSize { + if (start + batchSize) < len(delTopicIDs) { + ids = delTopicIDs[start:(start + batchSize)] + } else { + ids = delTopicIDs[start:] + } + + log.Info("Deleting 'repo_topic' rows for topics with ids = %v", ids) + if _, err := sess.In("topic_id", ids).Delete(&models.RepoTopic{}); err != nil { + return err + } + + log.Info("Deleting topics with id = %v", ids) + if _, err := sess.In("id", ids).Delete(&Topic{}); err != nil { + return err + } + + if len(ids) < batchSize { + break + } + } + + repoTopics := make([]*models.RepoTopic, 0, batchSize) + delRepoTopics := make([]*models.RepoTopic, 0, batchSize) + tmpRepoTopics := make([]*models.RepoTopic, 0, 30) + + log.Info("Checking the number of topics in the repositories...") + for start := 0; ; start += batchSize { + repoTopics = repoTopics[:0] + if err := sess.Cols("repo_id").Asc("repo_id").Limit(batchSize, start). + GroupBy("repo_id").Having("COUNT(*) > 25").Find(&repoTopics); err != nil { + return err + } + if len(repoTopics) == 0 { + break + } + + log.Info("Number of repositories with more than 25 topics: %v", len(repoTopics)) + for _, repoTopic := range repoTopics { + touchedRepo[repoTopic.RepoID] = struct{}{} + + tmpRepoTopics = tmpRepoTopics[:0] + if err := sess.Where("repo_id = ?", repoTopic.RepoID).Find(&tmpRepoTopics); err != nil { + return err + } + + log.Info("Repository with id = %v has %v topics", repoTopic.RepoID, len(tmpRepoTopics)) + + for i := len(tmpRepoTopics) - 1; i > 24; i-- { + delRepoTopics = append(delRepoTopics, tmpRepoTopics[i]) + } + } + } + + log.Info("Deleting superfluous topics for repositories (more than 25 topics)...") + for _, repoTopic := range delRepoTopics { + log.Info("Deleting 'repo_topic' rows for 'repository' with id = %v. Topic id = %v", + repoTopic.RepoID, repoTopic.TopicID) + + if _, err := sess.Where("repo_id = ? AND topic_id = ?", repoTopic.RepoID, + repoTopic.TopicID).Delete(&models.RepoTopic{}); err != nil { + return err + } + if _, err := sess.Exec( + "UPDATE topic SET repo_count = (SELECT repo_count FROM topic WHERE id = ?) - 1 WHERE id = ?", + repoTopic.TopicID, repoTopic.TopicID); err != nil { + return err + } + } + + topicNames := make([]string, 0, 30) + log.Info("Updating repositories 'topics' fields...") + for repoID := range touchedRepo { + if err := sess.Table("topic").Cols("name"). + Join("INNER", "repo_topic", "topic.id = repo_topic.topic_id"). + Where("repo_topic.repo_id = ?", repoID).Find(&topicNames); err != nil { + return err + } + log.Info("Updating 'topics' field for repository with id = %v", repoID) + if _, err := sess.ID(repoID).Cols("topics"). + Update(&models.Repository{Topics: topicNames}); err != nil { + return err + } + } + if err := sess.Commit(); err != nil { + return err + } + + return nil +} diff --git a/models/topic.go b/models/topic.go index 3b1737f8a..247aac5ff 100644 --- a/models/topic.go +++ b/models/topic.go @@ -6,6 +6,7 @@ package models import ( "fmt" + "regexp" "strings" "code.gitea.io/gitea/modules/util" @@ -20,6 +21,8 @@ func init() { ) } +var topicPattern = regexp.MustCompile(`^[a-z0-9][a-z0-9-]*$`) + // Topic represents a topic of repositories type Topic struct { ID int64 @@ -51,6 +54,11 @@ func (err ErrTopicNotExist) Error() string { return fmt.Sprintf("topic is not exist [name: %s]", err.Name) } +// ValidateTopic checks topics by length and match pattern rules +func ValidateTopic(topic string) bool { + return len(topic) <= 35 && topicPattern.MatchString(topic) +} + // GetTopicByName retrieves topic by name func GetTopicByName(name string) (*Topic, error) { var topic Topic @@ -182,6 +190,13 @@ func SaveTopics(repoID int64, topicNames ...string) error { } } + topicNames = topicNames[:0] + if err := sess.Table("topic").Cols("name"). + Join("INNER", "repo_topic", "topic.id = repo_topic.topic_id"). + Where("repo_topic.repo_id = ?", repoID).Find(&topicNames); err != nil { + return err + } + if _, err := sess.ID(repoID).Cols("topics").Update(&Repository{ Topics: topicNames, }); err != nil { diff --git a/models/topic_test.go b/models/topic_test.go index 472f4e52d..ef374e557 100644 --- a/models/topic_test.go +++ b/models/topic_test.go @@ -55,3 +55,16 @@ func TestAddTopic(t *testing.T) { assert.NoError(t, err) assert.EqualValues(t, 2, len(topics)) } + +func TestTopicValidator(t *testing.T) { + assert.True(t, ValidateTopic("12345")) + assert.True(t, ValidateTopic("2-test")) + assert.True(t, ValidateTopic("test-3")) + assert.True(t, ValidateTopic("first")) + assert.True(t, ValidateTopic("second-test-topic")) + assert.True(t, ValidateTopic("third-project-topic-with-max-length")) + + assert.False(t, ValidateTopic("$fourth-test,topic")) + assert.False(t, ValidateTopic("-fifth-test-topic")) + assert.False(t, ValidateTopic("sixth-go-project-topic-with-excess-length")) +} diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index 8cf6111c6..21ae775e4 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -1167,6 +1167,8 @@ branch.protected_deletion_failed = Branch '%s' is protected. It cannot be delete topic.manage_topics = Manage Topics topic.done = Done +topic.count_prompt = You can't select more than 25 topics +topic.format_prompt = Topics must start with a letter or number, can include hyphens(-) and must be no more than 35 characters long [org] org_name_holder = Organization Name diff --git a/public/js/index.js b/public/js/index.js index e98a3fe6d..823dd8766 100644 --- a/public/js/index.js +++ b/public/js/index.js @@ -2336,8 +2336,10 @@ function initTopicbar() { }).done(function() { editDiv.hide(); viewDiv.show(); + }).fail(function(xhr) { + alert(xhr.responseJSON.message) }) - }) + }); $('#topic_edit .dropdown').dropdown({ allowAdditions: true, diff --git a/routers/repo/topic.go b/routers/repo/topic.go index 2a43d53ff..63fcf793f 100644 --- a/routers/repo/topic.go +++ b/routers/repo/topic.go @@ -12,8 +12,8 @@ import ( "code.gitea.io/gitea/modules/log" ) -// TopicPost response for creating repository -func TopicPost(ctx *context.Context) { +// TopicsPost response for creating repository +func TopicsPost(ctx *context.Context) { if ctx.User == nil { ctx.JSON(403, map[string]interface{}{ "message": "Only owners could change the topics.", @@ -27,6 +27,37 @@ func TopicPost(ctx *context.Context) { topics = strings.Split(topicsStr, ",") } + invalidTopics := make([]string, 0) + i := 0 + for _, topic := range topics { + topic = strings.TrimSpace(strings.ToLower(topic)) + // ignore empty string + if len(topic) > 0 { + topics[i] = topic + i++ + } + if !models.ValidateTopic(topic) { + invalidTopics = append(invalidTopics, topic) + } + } + topics = topics[:i] + + if len(topics) > 25 { + ctx.JSON(422, map[string]interface{}{ + "invalidTopics": topics[:0], + "message": ctx.Tr("repo.topic.count_prompt"), + }) + return + } + + if len(invalidTopics) > 0 { + ctx.JSON(422, map[string]interface{}{ + "invalidTopics": invalidTopics, + "message": ctx.Tr("repo.topic.format_prompt"), + }) + return + } + err := models.SaveTopics(ctx.Repo.Repository.ID, topics...) if err != nil { log.Error(2, "SaveTopics failed: %v", err) diff --git a/routers/routes/routes.go b/routers/routes/routes.go index 250b98507..1eefbf1b6 100644 --- a/routers/routes/routes.go +++ b/routers/routes/routes.go @@ -626,7 +626,7 @@ func RegisterRoutes(m *macaron.Macaron) { }, context.RepoAssignment(), context.UnitTypes(), context.LoadRepoUnits(), context.CheckUnit(models.UnitTypeReleases)) m.Group("/:username/:reponame", func() { - m.Post("/topics", repo.TopicPost) + m.Post("/topics", repo.TopicsPost) }, context.RepoAssignment(), reqRepoAdmin) m.Group("/:username/:reponame", func() { From 0b3ea428477b9da33f40252e79aafe85b09526f3 Mon Sep 17 00:00:00 2001 From: David Schneiderbauer Date: Thu, 21 Jun 2018 18:00:13 +0200 Subject: [PATCH 2/3] hide issues from org private repos w/o team assignment (#4034) --- integrations/api_repo_test.go | 6 +- models/access_test.go | 16 ++- models/fixtures/repository.yml | 15 ++- models/fixtures/team.yml | 8 +- models/fixtures/team_repo.yml | 10 +- models/fixtures/team_unit.yml | 209 ++++++++++++++++++++++++++++++++ models/fixtures/user.yml | 2 +- models/migrations/migrations.go | 2 + models/migrations/v38.go | 9 +- models/migrations/v69.go | 80 ++++++++++++ models/models.go | 1 + models/notification.go | 10 ++ models/org.go | 17 ++- models/org_team.go | 86 +++++++++++-- models/org_test.go | 12 +- models/repo.go | 18 +-- models/repo_list_test.go | 6 +- models/repo_watch.go | 17 +++ models/user.go | 38 ++++-- models/user_test.go | 22 ++++ routers/org/teams.go | 21 +++- routers/user/home.go | 6 +- routers/user/home_test.go | 4 +- templates/org/team/new.tmpl | 2 +- 24 files changed, 545 insertions(+), 72 deletions(-) create mode 100644 models/fixtures/team_unit.yml create mode 100644 models/migrations/v69.go diff --git a/integrations/api_repo_test.go b/integrations/api_repo_test.go index b766dd584..12429c88a 100644 --- a/integrations/api_repo_test.go +++ b/integrations/api_repo_test.go @@ -67,9 +67,9 @@ func TestAPISearchRepo(t *testing.T) { expectedResults }{ {name: "RepositoriesMax50", requestURL: "/api/v1/repos/search?limit=50", expectedResults: expectedResults{ - nil: {count: 15}, - user: {count: 15}, - user2: {count: 15}}, + nil: {count: 16}, + user: {count: 16}, + user2: {count: 16}}, }, {name: "RepositoriesMax10", requestURL: "/api/v1/repos/search?limit=10", expectedResults: expectedResults{ nil: {count: 10}, diff --git a/models/access_test.go b/models/access_test.go index 59575acb7..46d6f723e 100644 --- a/models/access_test.go +++ b/models/access_test.go @@ -22,8 +22,12 @@ func TestAccessLevel(t *testing.T) { user1 := AssertExistsAndLoadBean(t, &User{ID: 2}).(*User) user2 := AssertExistsAndLoadBean(t, &User{ID: 5}).(*User) - repo1 := AssertExistsAndLoadBean(t, &Repository{OwnerID: 2, IsPrivate: false}).(*Repository) - repo2 := AssertExistsAndLoadBean(t, &Repository{OwnerID: 3, IsPrivate: true}).(*Repository) + // A public repository owned by User 2 + repo1 := AssertExistsAndLoadBean(t, &Repository{ID: 1}).(*Repository) + assert.False(t, repo1.IsPrivate) + // A private repository owned by Org 3 + repo2 := AssertExistsAndLoadBean(t, &Repository{ID: 3}).(*Repository) + assert.True(t, repo2.IsPrivate) level, err := AccessLevel(user1.ID, repo1) assert.NoError(t, err) @@ -47,8 +51,12 @@ func TestHasAccess(t *testing.T) { user1 := AssertExistsAndLoadBean(t, &User{ID: 2}).(*User) user2 := AssertExistsAndLoadBean(t, &User{ID: 5}).(*User) - repo1 := AssertExistsAndLoadBean(t, &Repository{OwnerID: 2, IsPrivate: false}).(*Repository) - repo2 := AssertExistsAndLoadBean(t, &Repository{OwnerID: 3, IsPrivate: true}).(*Repository) + // A public repository owned by User 2 + repo1 := AssertExistsAndLoadBean(t, &Repository{ID: 1}).(*Repository) + assert.False(t, repo1.IsPrivate) + // A private repository owned by Org 3 + repo2 := AssertExistsAndLoadBean(t, &Repository{ID: 3}).(*Repository) + assert.True(t, repo2.IsPrivate) for _, accessMode := range accessModes { has, err := HasAccess(user1.ID, repo1, accessMode) diff --git a/models/fixtures/repository.yml b/models/fixtures/repository.yml index c7d73fe17..3238b65ea 100644 --- a/models/fixtures/repository.yml +++ b/models/fixtures/repository.yml @@ -351,7 +351,7 @@ is_mirror: true num_forks: 1 is_fork: false - + - id: 29 fork_id: 27 @@ -365,7 +365,7 @@ num_closed_pulls: 0 is_mirror: false is_fork: true - + - id: 30 fork_id: 28 @@ -389,3 +389,14 @@ num_forks: 0 num_issues: 0 is_mirror: false + +- + id: 32 + owner_id: 3 + lower_name: repo21 + name: repo21 + is_private: false + num_stars: 0 + num_forks: 0 + num_issues: 0 + is_mirror: false diff --git a/models/fixtures/team.yml b/models/fixtures/team.yml index 1d242cb5b..4b4a1d798 100644 --- a/models/fixtures/team.yml +++ b/models/fixtures/team.yml @@ -4,9 +4,8 @@ lower_name: owners name: Owners authorize: 4 # owner - num_repos: 2 + num_repos: 3 num_members: 1 - unit_types: '[1,2,3,4,5,6,7]' - id: 2 @@ -16,7 +15,6 @@ authorize: 2 # write num_repos: 1 num_members: 2 - unit_types: '[1,2,3,4,5,6,7]' - id: 3 @@ -26,7 +24,6 @@ authorize: 4 # owner num_repos: 0 num_members: 1 - unit_types: '[1,2,3,4,5,6,7]' - id: 4 @@ -36,7 +33,6 @@ authorize: 4 # owner num_repos: 0 num_members: 1 - unit_types: '[1,2,3,4,5,6,7]' - id: 5 @@ -46,7 +42,6 @@ authorize: 4 # owner num_repos: 2 num_members: 2 - unit_types: '[1,2,3,4,5,6,7]' - id: 6 @@ -56,4 +51,3 @@ authorize: 4 # owner num_repos: 2 num_members: 1 - unit_types: '[1,2,3,4,5,6,7]' \ No newline at end of file diff --git a/models/fixtures/team_repo.yml b/models/fixtures/team_repo.yml index 9e6d74553..b324e0941 100644 --- a/models/fixtures/team_repo.yml +++ b/models/fixtures/team_repo.yml @@ -33,9 +33,15 @@ org_id: 19 team_id: 6 repo_id: 27 - + - id: 7 org_id: 19 team_id: 6 - repo_id: 28 \ No newline at end of file + repo_id: 28 + +- + id: 8 + org_id: 3 + team_id: 1 + repo_id: 32 diff --git a/models/fixtures/team_unit.yml b/models/fixtures/team_unit.yml new file mode 100644 index 000000000..ad5466a5c --- /dev/null +++ b/models/fixtures/team_unit.yml @@ -0,0 +1,209 @@ +- + id: 1 + team_id: 1 + type: 1 + +- + id: 2 + team_id: 1 + type: 2 + +- + id: 3 + team_id: 1 + type: 3 + +- + id: 4 + team_id: 1 + type: 4 + +- + id: 5 + team_id: 1 + type: 5 + +- + id: 6 + team_id: 1 + type: 6 + +- + id: 7 + team_id: 1 + type: 7 + +- + id: 8 + team_id: 2 + type: 1 + +- + id: 9 + team_id: 2 + type: 2 + +- + id: 10 + team_id: 2 + type: 3 + +- + id: 11 + team_id: 2 + type: 4 + +- + id: 12 + team_id: 2 + type: 5 + +- + id: 13 + team_id: 2 + type: 6 + +- + id: 14 + team_id: 2 + type: 7 + +- + id: 15 + team_id: 3 + type: 1 + +- + id: 16 + team_id: 3 + type: 2 + +- + id: 17 + team_id: 3 + type: 3 + +- + id: 18 + team_id: 3 + type: 4 + +- + id: 19 + team_id: 3 + type: 5 + +- + id: 20 + team_id: 3 + type: 6 + +- + id: 21 + team_id: 3 + type: 7 + +- + id: 22 + team_id: 4 + type: 1 + +- + id: 23 + team_id: 4 + type: 2 + +- + id: 24 + team_id: 4 + type: 3 + +- + id: 25 + team_id: 4 + type: 4 + +- + id: 26 + team_id: 4 + type: 5 + +- + id: 27 + team_id: 4 + type: 6 + +- + id: 28 + team_id: 4 + type: 7 + +- + id: 29 + team_id: 5 + type: 1 + +- + id: 30 + team_id: 5 + type: 2 + +- + id: 31 + team_id: 5 + type: 3 + +- + id: 32 + team_id: 5 + type: 4 + +- + id: 33 + team_id: 5 + type: 5 + +- + id: 34 + team_id: 5 + type: 6 + +- + id: 35 + team_id: 5 + type: 7 + +- + id: 36 + team_id: 6 + type: 1 + +- + id: 37 + team_id: 6 + type: 2 + +- + id: 38 + team_id: 6 + type: 3 + +- + id: 39 + team_id: 6 + type: 4 + +- + id: 40 + team_id: 6 + type: 5 + +- + id: 41 + team_id: 6 + type: 6 + +- + id: 42 + team_id: 6 + type: 7 diff --git a/models/fixtures/user.yml b/models/fixtures/user.yml index 7ad48f7fb..a2e3b88d7 100644 --- a/models/fixtures/user.yml +++ b/models/fixtures/user.yml @@ -45,7 +45,7 @@ is_admin: false avatar: avatar3 avatar_email: user3@example.com - num_repos: 2 + num_repos: 3 num_members: 2 num_teams: 2 diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go index 7732e1709..cc262d810 100644 --- a/models/migrations/migrations.go +++ b/models/migrations/migrations.go @@ -190,6 +190,8 @@ var migrations = []Migration{ NewMigration("remove stale watches", removeStaleWatches), // v68 -> V69 NewMigration("Reformat and remove incorrect topics", reformatAndRemoveIncorrectTopics), + // v69 -> v70 + NewMigration("move team units to team_unit table", moveTeamUnitsToTeamUnitTable), } // Migrate database to current version diff --git a/models/migrations/v38.go b/models/migrations/v38.go index 6f66484b0..eb90f9fbf 100644 --- a/models/migrations/v38.go +++ b/models/migrations/v38.go @@ -25,10 +25,15 @@ func removeCommitsUnitType(x *xorm.Engine) (err error) { Created time.Time `xorm:"-"` } + type Team struct { + ID int64 + UnitTypes []int `xorm:"json"` + } + // Update team unit types const batchSize = 100 for start := 0; ; start += batchSize { - teams := make([]*models.Team, 0, batchSize) + teams := make([]*Team, 0, batchSize) if err := x.Limit(batchSize, start).Find(&teams); err != nil { return err } @@ -36,7 +41,7 @@ func removeCommitsUnitType(x *xorm.Engine) (err error) { break } for _, team := range teams { - ut := make([]models.UnitType, 0, len(team.UnitTypes)) + ut := make([]int, 0, len(team.UnitTypes)) for _, u := range team.UnitTypes { if u < V16UnitTypeCommits { ut = append(ut, u) diff --git a/models/migrations/v69.go b/models/migrations/v69.go new file mode 100644 index 000000000..8d964688a --- /dev/null +++ b/models/migrations/v69.go @@ -0,0 +1,80 @@ +// Copyright 2018 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 migrations + +import ( + "fmt" + + "github.com/go-xorm/xorm" +) + +func moveTeamUnitsToTeamUnitTable(x *xorm.Engine) error { + // Team see models/team.go + type Team struct { + ID int64 + OrgID int64 + UnitTypes []int `xorm:"json"` + } + + // TeamUnit see models/org_team.go + type TeamUnit struct { + ID int64 `xorm:"pk autoincr"` + OrgID int64 `xorm:"INDEX"` + TeamID int64 `xorm:"UNIQUE(s)"` + Type int `xorm:"UNIQUE(s)"` + } + + if err := x.Sync2(new(TeamUnit)); err != nil { + return fmt.Errorf("Sync2: %v", err) + } + + sess := x.NewSession() + defer sess.Close() + + if err := sess.Begin(); err != nil { + return err + } + + // Update team unit types + const batchSize = 100 + for start := 0; ; start += batchSize { + teams := make([]*Team, 0, batchSize) + if err := x.Limit(batchSize, start).Find(&teams); err != nil { + return err + } + if len(teams) == 0 { + break + } + + for _, team := range teams { + var unitTypes []int + if len(team.UnitTypes) == 0 { + unitTypes = allUnitTypes + } else { + unitTypes = team.UnitTypes + } + + // insert units for team + var units = make([]TeamUnit, 0, len(unitTypes)) + for _, tp := range unitTypes { + units = append(units, TeamUnit{ + OrgID: team.OrgID, + TeamID: team.ID, + Type: tp, + }) + } + + if _, err := sess.Insert(&units); err != nil { + return fmt.Errorf("Insert team units: %v", err) + } + + } + } + + if err := dropTableColumns(sess, "team", "unit_types"); err != nil { + return err + } + return sess.Commit() +} diff --git a/models/models.go b/models/models.go index 5743f1862..aaf1370fd 100644 --- a/models/models.go +++ b/models/models.go @@ -122,6 +122,7 @@ func init() { new(Reaction), new(IssueAssignees), new(U2FRegistration), + new(TeamUnit), ) gonicNames := []string{"SSL", "UID"} diff --git a/models/notification.go b/models/notification.go index c8376a857..8c36c0c5c 100644 --- a/models/notification.go +++ b/models/notification.go @@ -119,7 +119,17 @@ func createOrUpdateIssueNotifications(e Engine, issue *Issue, notificationAuthor } } + issue.loadRepo(e) + for _, watch := range watches { + issue.Repo.Units = nil + if issue.IsPull && !issue.Repo.CheckUnitUser(watch.UserID, false, UnitTypePullRequests) { + continue + } + if !issue.IsPull && !issue.Repo.CheckUnitUser(watch.UserID, false, UnitTypeIssues) { + continue + } + if err := notifyUser(watch.UserID); err != nil { return err } diff --git a/models/org.go b/models/org.go index ed0d58306..23f6c58bf 100644 --- a/models/org.go +++ b/models/org.go @@ -154,12 +154,26 @@ func CreateOrganization(org, owner *User) (err error) { Name: ownerTeamName, Authorize: AccessModeOwner, NumMembers: 1, - UnitTypes: allRepUnitTypes, } if _, err = sess.Insert(t); err != nil { return fmt.Errorf("insert owner team: %v", err) } + // insert units for team + var units = make([]TeamUnit, 0, len(allRepUnitTypes)) + for _, tp := range allRepUnitTypes { + units = append(units, TeamUnit{ + OrgID: org.ID, + TeamID: t.ID, + Type: tp, + }) + } + + if _, err = sess.Insert(&units); err != nil { + sess.Rollback() + return err + } + if _, err = sess.Insert(&TeamUser{ UID: owner.ID, OrgID: org.ID, @@ -238,6 +252,7 @@ func deleteOrg(e *xorm.Session, u *User) error { &Team{OrgID: u.ID}, &OrgUser{OrgID: u.ID}, &TeamUser{OrgID: u.ID}, + &TeamUnit{OrgID: u.ID}, ); err != nil { return fmt.Errorf("deleteBeans: %v", err) } diff --git a/models/org_team.go b/models/org_team.go index 5ea6e76cd..3b37f936f 100644 --- a/models/org_team.go +++ b/models/org_team.go @@ -1,3 +1,4 @@ +// Copyright 2018 The Gitea Authors. All rights reserved. // Copyright 2016 The Gogs Authors. All rights reserved. // Use of this source code is governed by a MIT-style // license that can be found in the LICENSE file. @@ -10,7 +11,6 @@ import ( "strings" "code.gitea.io/gitea/modules/log" - "github.com/go-xorm/xorm" ) @@ -28,15 +28,16 @@ type Team struct { Members []*User `xorm:"-"` NumRepos int NumMembers int - UnitTypes []UnitType `xorm:"json"` + Units []*TeamUnit `xorm:"-"` } -// GetUnitTypes returns unit types the team owned, empty means all the unit types -func (t *Team) GetUnitTypes() []UnitType { - if len(t.UnitTypes) == 0 { - return allRepUnitTypes +func (t *Team) getUnits(e Engine) (err error) { + if t.Units != nil { + return nil } - return t.UnitTypes + + t.Units, err = getUnitsByTeamID(e, t.ID) + return err } // HasWriteAccess returns true if team has at least write level access mode. @@ -214,11 +215,12 @@ func (t *Team) RemoveRepository(repoID int64) error { // UnitEnabled returns if the team has the given unit type enabled func (t *Team) UnitEnabled(tp UnitType) bool { - if len(t.UnitTypes) == 0 { - return true + if err := t.getUnits(x); err != nil { + log.Warn("Error loading repository (ID: %d) units: %s", t.ID, err.Error()) } - for _, u := range t.UnitTypes { - if u == tp { + + for _, unit := range t.Units { + if unit.Type == tp { return true } } @@ -275,6 +277,17 @@ func NewTeam(t *Team) (err error) { return err } + // insert units for team + if len(t.Units) > 0 { + for _, unit := range t.Units { + unit.TeamID = t.ID + } + if _, err = sess.Insert(&t.Units); err != nil { + sess.Rollback() + return err + } + } + // Update organization number of teams. if _, err = sess.Exec("UPDATE `user` SET num_teams=num_teams+1 WHERE id = ?", t.OrgID); err != nil { sess.Rollback() @@ -424,6 +437,13 @@ func DeleteTeam(t *Team) error { return err } + // Delete team-unit. + if _, err := sess. + Where("team_id=?", t.ID). + Delete(new(TeamUnit)); err != nil { + return err + } + // Delete team. if _, err := sess.ID(t.ID).Delete(new(Team)); err != nil { return err @@ -695,3 +715,47 @@ func GetTeamsWithAccessToRepo(orgID, repoID int64, mode AccessMode) ([]*Team, er And("team_repo.repo_id = ?", repoID). Find(&teams) } + +// ___________ ____ ___ .__ __ +// \__ ___/___ _____ _____ | | \____ |__|/ |_ +// | |_/ __ \\__ \ / \| | / \| \ __\ +// | |\ ___/ / __ \| Y Y \ | / | \ || | +// |____| \___ >____ /__|_| /______/|___| /__||__| +// \/ \/ \/ \/ + +// TeamUnit describes all units of a repository +type TeamUnit struct { + ID int64 `xorm:"pk autoincr"` + OrgID int64 `xorm:"INDEX"` + TeamID int64 `xorm:"UNIQUE(s)"` + Type UnitType `xorm:"UNIQUE(s)"` +} + +// Unit returns Unit +func (t *TeamUnit) Unit() Unit { + return Units[t.Type] +} + +func getUnitsByTeamID(e Engine, teamID int64) (units []*TeamUnit, err error) { + return units, e.Where("team_id = ?", teamID).Find(&units) +} + +// UpdateTeamUnits updates a teams's units +func UpdateTeamUnits(team *Team, units []TeamUnit) (err error) { + sess := x.NewSession() + defer sess.Close() + if err = sess.Begin(); err != nil { + return err + } + + if _, err = sess.Where("team_id = ?", team.ID).Delete(new(TeamUnit)); err != nil { + return err + } + + if _, err = sess.Insert(units); err != nil { + sess.Rollback() + return err + } + + return sess.Commit() +} diff --git a/models/org_test.go b/models/org_test.go index 42ab4a2a4..c54e7a93b 100644 --- a/models/org_test.go +++ b/models/org_test.go @@ -489,8 +489,8 @@ func TestAccessibleReposEnv_CountRepos(t *testing.T) { assert.NoError(t, err) assert.EqualValues(t, expectedCount, count) } - testSuccess(2, 2) - testSuccess(4, 1) + testSuccess(2, 3) + testSuccess(4, 2) } func TestAccessibleReposEnv_RepoIDs(t *testing.T) { @@ -503,8 +503,8 @@ func TestAccessibleReposEnv_RepoIDs(t *testing.T) { assert.NoError(t, err) assert.Equal(t, expectedRepoIDs, repoIDs) } - testSuccess(2, 1, 100, []int64{3, 5}) - testSuccess(4, 0, 100, []int64{3}) + testSuccess(2, 1, 100, []int64{3, 5, 32}) + testSuccess(4, 0, 100, []int64{3, 32}) } func TestAccessibleReposEnv_Repos(t *testing.T) { @@ -522,8 +522,8 @@ func TestAccessibleReposEnv_Repos(t *testing.T) { } assert.Equal(t, expectedRepos, repos) } - testSuccess(2, []int64{3, 5}) - testSuccess(4, []int64{3}) + testSuccess(2, []int64{3, 5, 32}) + testSuccess(4, []int64{3, 32}) } func TestAccessibleReposEnv_MirrorRepos(t *testing.T) { diff --git a/models/repo.go b/models/repo.go index 7f2be502a..d1cc290c4 100644 --- a/models/repo.go +++ b/models/repo.go @@ -365,22 +365,14 @@ func (repo *Repository) getUnitsByUserID(e Engine, userID int64, isAdmin bool) ( return err } - var allTypes = make(map[UnitType]struct{}, len(allRepUnitTypes)) - for _, team := range teams { - // Administrators can not be limited - if team.Authorize >= AccessModeAdmin { - return nil - } - for _, unitType := range team.UnitTypes { - allTypes[unitType] = struct{}{} - } - } - // unique var newRepoUnits = make([]*RepoUnit, 0, len(repo.Units)) for _, u := range repo.Units { - if _, ok := allTypes[u.Type]; ok { - newRepoUnits = append(newRepoUnits, u) + for _, team := range teams { + if team.UnitEnabled(u.Type) { + newRepoUnits = append(newRepoUnits, u) + break + } } } diff --git a/models/repo_list_test.go b/models/repo_list_test.go index 3bccb1aeb..164bc19bf 100644 --- a/models/repo_list_test.go +++ b/models/repo_list_test.go @@ -147,10 +147,10 @@ func TestSearchRepositoryByName(t *testing.T) { count: 14}, {name: "AllPublic/PublicRepositoriesOfUserIncludingCollaborative", opts: &SearchRepoOptions{Page: 1, PageSize: 10, OwnerID: 15, AllPublic: true}, - count: 15}, + count: 16}, {name: "AllPublic/PublicAndPrivateRepositoriesOfUserIncludingCollaborative", opts: &SearchRepoOptions{Page: 1, PageSize: 10, OwnerID: 15, Private: true, AllPublic: true}, - count: 19}, + count: 20}, {name: "AllPublic/PublicAndPrivateRepositoriesOfUserIncludingCollaborativeByName", opts: &SearchRepoOptions{Keyword: "test", Page: 1, PageSize: 10, OwnerID: 15, Private: true, AllPublic: true}, count: 13}, @@ -159,7 +159,7 @@ func TestSearchRepositoryByName(t *testing.T) { count: 11}, {name: "AllPublic/PublicRepositoriesOfOrganization", opts: &SearchRepoOptions{Page: 1, PageSize: 10, OwnerID: 17, AllPublic: true, Collaborate: util.OptionalBoolFalse}, - count: 15}, + count: 16}, } for _, testCase := range testCases { diff --git a/models/repo_watch.go b/models/repo_watch.go index fb89a55a1..8019027c1 100644 --- a/models/repo_watch.go +++ b/models/repo_watch.go @@ -109,6 +109,23 @@ func notifyWatchers(e Engine, act *Action) error { act.ID = 0 act.UserID = watches[i].UserID + act.Repo.Units = nil + + switch act.OpType { + case ActionCommitRepo, ActionPushTag, ActionDeleteTag, ActionDeleteBranch: + if !act.Repo.CheckUnitUser(act.UserID, false, UnitTypeCode) { + continue + } + case ActionCreateIssue, ActionCommentIssue, ActionCloseIssue, ActionReopenIssue: + if !act.Repo.CheckUnitUser(act.UserID, false, UnitTypeIssues) { + continue + } + case ActionCreatePullRequest, ActionMergePullRequest, ActionClosePullRequest, ActionReopenPullRequest: + if !act.Repo.CheckUnitUser(act.UserID, false, UnitTypePullRequests) { + continue + } + } + if _, err = e.InsertOne(act); err != nil { return fmt.Errorf("insert new action: %v", err) } diff --git a/models/user.go b/models/user.go index 1497eef44..653e99426 100644 --- a/models/user.go +++ b/models/user.go @@ -546,28 +546,46 @@ func (u *User) GetRepositories(page, pageSize int) (err error) { return err } -// GetRepositoryIDs returns repositories IDs where user owned -func (u *User) GetRepositoryIDs() ([]int64, error) { +// GetRepositoryIDs returns repositories IDs where user owned and has unittypes +func (u *User) GetRepositoryIDs(units ...UnitType) ([]int64, error) { var ids []int64 - return ids, x.Table("repository").Cols("id").Where("owner_id = ?", u.ID).Find(&ids) + + sess := x.Table("repository").Cols("repository.id") + + if len(units) > 0 { + sess = sess.Join("INNER", "repo_unit", "repository.id = repo_unit.repo_id") + sess = sess.In("repo_unit.type", units) + } + + return ids, sess.Where("owner_id = ?", u.ID).Find(&ids) } -// GetOrgRepositoryIDs returns repositories IDs where user's team owned -func (u *User) GetOrgRepositoryIDs() ([]int64, error) { +// GetOrgRepositoryIDs returns repositories IDs where user's team owned and has unittypes +func (u *User) GetOrgRepositoryIDs(units ...UnitType) ([]int64, error) { var ids []int64 - return ids, x.Table("repository"). + + sess := x.Table("repository"). Cols("repository.id"). - Join("INNER", "team_user", "repository.owner_id = team_user.org_id AND team_user.uid = ?", u.ID). + Join("INNER", "team_user", "repository.owner_id = team_user.org_id"). + Join("INNER", "team_repo", "repository.is_private != ? OR (team_user.team_id = team_repo.team_id AND repository.id = team_repo.repo_id)", true) + + if len(units) > 0 { + sess = sess.Join("INNER", "team_unit", "team_unit.team_id = team_user.team_id") + sess = sess.In("team_unit.type", units) + } + + return ids, sess. + Where("team_user.uid = ?", u.ID). GroupBy("repository.id").Find(&ids) } // GetAccessRepoIDs returns all repositories IDs where user's or user is a team member organizations -func (u *User) GetAccessRepoIDs() ([]int64, error) { - ids, err := u.GetRepositoryIDs() +func (u *User) GetAccessRepoIDs(units ...UnitType) ([]int64, error) { + ids, err := u.GetRepositoryIDs(units...) if err != nil { return nil, err } - ids2, err := u.GetOrgRepositoryIDs() + ids2, err := u.GetOrgRepositoryIDs(units...) if err != nil { return nil, err } diff --git a/models/user_test.go b/models/user_test.go index 4fd0bc0fa..20de1a64b 100644 --- a/models/user_test.go +++ b/models/user_test.go @@ -159,3 +159,25 @@ func BenchmarkHashPassword(b *testing.B) { u.HashPassword(pass) } } + +func TestGetOrgRepositoryIDs(t *testing.T) { + assert.NoError(t, PrepareTestDatabase()) + user2 := AssertExistsAndLoadBean(t, &User{ID: 2}).(*User) + user4 := AssertExistsAndLoadBean(t, &User{ID: 4}).(*User) + user5 := AssertExistsAndLoadBean(t, &User{ID: 5}).(*User) + + accessibleRepos, err := user2.GetOrgRepositoryIDs() + assert.NoError(t, err) + // User 2's team has access to private repos 3, 5, repo 32 is a public repo of the organization + assert.Equal(t, []int64{3, 5, 32}, accessibleRepos) + + accessibleRepos, err = user4.GetOrgRepositoryIDs() + assert.NoError(t, err) + // User 4's team has access to private repo 3, repo 32 is a public repo of the organization + assert.Equal(t, []int64{3, 32}, accessibleRepos) + + accessibleRepos, err = user5.GetOrgRepositoryIDs() + assert.NoError(t, err) + // User 5's team has no access to any repo + assert.Len(t, accessibleRepos, 0) +} diff --git a/routers/org/teams.go b/routers/org/teams.go index d894c8661..87bfb8596 100644 --- a/routers/org/teams.go +++ b/routers/org/teams.go @@ -182,7 +182,14 @@ func NewTeamPost(ctx *context.Context, form auth.CreateTeamForm) { Authorize: models.ParseAccessMode(form.Permission), } if t.Authorize < models.AccessModeAdmin { - t.UnitTypes = form.Units + var units = make([]*models.TeamUnit, 0, len(form.Units)) + for _, tp := range form.Units { + units = append(units, &models.TeamUnit{ + OrgID: ctx.Org.Organization.ID, + Type: tp, + }) + } + t.Units = units } ctx.Data["Team"] = t @@ -264,9 +271,17 @@ func EditTeamPost(ctx *context.Context, form auth.CreateTeamForm) { } t.Description = form.Description if t.Authorize < models.AccessModeAdmin { - t.UnitTypes = form.Units + var units = make([]models.TeamUnit, 0, len(form.Units)) + for _, tp := range form.Units { + units = append(units, models.TeamUnit{ + OrgID: t.OrgID, + TeamID: t.ID, + Type: tp, + }) + } + models.UpdateTeamUnits(t, units) } else { - t.UnitTypes = nil + models.UpdateTeamUnits(t, nil) } if ctx.HasError() { diff --git a/routers/user/home.go b/routers/user/home.go index 2a193bbde..0c84b2498 100644 --- a/routers/user/home.go +++ b/routers/user/home.go @@ -203,7 +203,11 @@ func Issues(ctx *context.Context) { return } } else { - userRepoIDs, err = ctxUser.GetAccessRepoIDs() + unitType := models.UnitTypeIssues + if isPullList { + unitType = models.UnitTypePullRequests + } + userRepoIDs, err = ctxUser.GetAccessRepoIDs(unitType) if err != nil { ctx.ServerError("ctxUser.GetAccessRepoIDs", err) return diff --git a/routers/user/home_test.go b/routers/user/home_test.go index a9b146b76..8a3d9b9f5 100644 --- a/routers/user/home_test.go +++ b/routers/user/home_test.go @@ -26,8 +26,8 @@ func TestIssues(t *testing.T) { Issues(ctx) assert.EqualValues(t, http.StatusOK, ctx.Resp.Status()) - assert.EqualValues(t, map[int64]int64{1: 1, 2: 1}, ctx.Data["Counts"]) + assert.EqualValues(t, map[int64]int64{1: 1}, ctx.Data["Counts"]) assert.EqualValues(t, true, ctx.Data["IsShowClosed"]) assert.Len(t, ctx.Data["Issues"], 1) - assert.Len(t, ctx.Data["Repos"], 2) + assert.Len(t, ctx.Data["Repos"], 1) } diff --git a/templates/org/team/new.tmpl b/templates/org/team/new.tmpl index ec1a3dd72..12cdd697c 100644 --- a/templates/org/team/new.tmpl +++ b/templates/org/team/new.tmpl @@ -57,7 +57,7 @@ {{range $t, $unit := $.Units}}
- + {{$.i18n.Tr $unit.DescKey}}
From e2721b619098b887e6e27675450f3a4663716f59 Mon Sep 17 00:00:00 2001 From: techknowlogick Date: Thu, 21 Jun 2018 17:12:56 -0400 Subject: [PATCH 3/3] Remove call to update certs (#4296) --- docker/etc/s6/gitea/setup | 2 -- 1 file changed, 2 deletions(-) diff --git a/docker/etc/s6/gitea/setup b/docker/etc/s6/gitea/setup index 500cca584..03758ed81 100755 --- a/docker/etc/s6/gitea/setup +++ b/docker/etc/s6/gitea/setup @@ -1,7 +1,5 @@ #!/bin/bash -/usr/sbin/update-ca-certificates - if [ ! -d /data/git/.ssh ]; then mkdir -p /data/git/.ssh chmod 700 /data/git/.ssh