From 850dc9b1d324d0e0becd7758f79c33608cbb54c9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Sat, 6 Jan 2024 18:02:18 +0100 Subject: [PATCH 1/5] Simplify storage of repositories It was only used for sequential iteration. --- src/data.rs | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/data.rs b/src/data.rs index 4162a0ba0..da9e31693 100644 --- a/src/data.rs +++ b/src/data.rs @@ -10,7 +10,7 @@ pub(crate) struct Data { people: HashMap, teams: HashMap, archived_teams: Vec, - repos: HashMap<(String, String), Repo>, + repos: Vec, config: Config, } @@ -20,7 +20,7 @@ impl Data { people: HashMap::new(), teams: HashMap::new(), archived_teams: Vec::new(), - repos: HashMap::new(), + repos: Vec::new(), config: load_file(Path::new("config.toml"))?, }; @@ -41,8 +41,7 @@ impl Data { ) } - this.repos - .insert((repo.org.clone(), repo.name.clone()), repo); + this.repos.push(repo); Ok(()) })?; @@ -154,7 +153,7 @@ impl Data { } pub(crate) fn repos(&self) -> impl Iterator { - self.repos.values() + self.repos.iter() } pub(crate) fn archived_teams(&self) -> impl Iterator { From ef81a3468d9bd741743bc217d17c21f475addf07 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Sat, 6 Jan 2024 18:09:21 +0100 Subject: [PATCH 2/5] Store archived flag for repos Archived repositories are loaded from `repos/archive`. --- rust_team_data/src/v1.rs | 1 + src/data.rs | 30 +++++++++++++++++-- src/static_api.rs | 9 +++++- tests/static-api/_expected/v1/repos.json | 26 +++++++++++++++- .../_expected/v1/repos/archived_repo.json | 23 ++++++++++++++ .../_expected/v1/repos/some_repo.json | 3 +- .../repos/archive/test-org/archived_repo.toml | 11 +++++++ 7 files changed, 98 insertions(+), 5 deletions(-) create mode 100644 tests/static-api/_expected/v1/repos/archived_repo.json create mode 100644 tests/static-api/repos/archive/test-org/archived_repo.toml diff --git a/rust_team_data/src/v1.rs b/rust_team_data/src/v1.rs index 9ed2fa5b7..3228e6c71 100644 --- a/rust_team_data/src/v1.rs +++ b/rust_team_data/src/v1.rs @@ -165,6 +165,7 @@ pub struct Repo { pub teams: Vec, pub members: Vec, pub branch_protections: Vec, + pub archived: bool, } #[derive(Debug, Clone, Deserialize, Serialize, PartialEq)] diff --git a/src/data.rs b/src/data.rs index da9e31693..ebc7b3112 100644 --- a/src/data.rs +++ b/src/data.rs @@ -11,6 +11,7 @@ pub(crate) struct Data { teams: HashMap, archived_teams: Vec, repos: Vec, + archived_repos: Vec, config: Config, } @@ -21,11 +22,12 @@ impl Data { teams: HashMap::new(), archived_teams: Vec::new(), repos: Vec::new(), + archived_repos: Vec::new(), config: load_file(Path::new("config.toml"))?, }; - data.load_dir("repos", true, |this, org, repo: Repo, path: &Path| { - if repo.org != org { + fn validate_repo(org: &str, repo: &Repo, path: &Path) -> anyhow::Result<()> { + if &repo.org != org { bail!( "repo '{}' is located in the '{}' org directory but its org is '{}'", repo.name, @@ -40,11 +42,31 @@ impl Data { path.file_name().unwrap().to_str().unwrap() ) } + Ok(()) + } + + data.load_dir("repos", true, |this, org, repo: Repo, path: &Path| { + if org == "archive" { + return Ok(()); + } + validate_repo(org, &repo, path)?; this.repos.push(repo); Ok(()) })?; + if Path::new("repos/archive").is_dir() { + data.load_dir( + "repos/archive", + true, + |this, org, repo: Repo, path: &Path| { + validate_repo(org, &repo, path)?; + this.archived_repos.push(repo); + Ok(()) + }, + )?; + } + data.load_dir("people", false, |this, _dir, person: Person, _path| { person.validate()?; this.people.insert(person.github().to_string(), person); @@ -156,6 +178,10 @@ impl Data { self.repos.iter() } + pub(crate) fn archived_repos(&self) -> impl Iterator { + self.archived_repos.iter() + } + pub(crate) fn archived_teams(&self) -> impl Iterator { self.archived_teams.iter() } diff --git a/src/static_api.rs b/src/static_api.rs index 929b01329..e35cd53af 100644 --- a/src/static_api.rs +++ b/src/static_api.rs @@ -36,7 +36,13 @@ impl<'a> Generator<'a> { fn generate_repos(&self) -> Result<(), Error> { let mut repos: IndexMap> = IndexMap::new(); - for r in self.data.repos() { + let repo_iter = self + .data + .repos() + .map(|repo| (repo, false)) + .chain(self.data.archived_repos().map(|repo| (repo, true))); + + for (r, archived) in repo_iter { let branch_protections: Vec<_> = r .branch_protections .iter() @@ -98,6 +104,7 @@ impl<'a> Generator<'a> { }) .collect(), branch_protections, + archived, }; self.add(&format!("v1/repos/{}.json", r.name), &repo)?; diff --git a/tests/static-api/_expected/v1/repos.json b/tests/static-api/_expected/v1/repos.json index abc1ddbd4..6405a24c6 100644 --- a/tests/static-api/_expected/v1/repos.json +++ b/tests/static-api/_expected/v1/repos.json @@ -1,5 +1,28 @@ { "test-org": [ + { + "org": "test-org", + "name": "archived_repo", + "description": "An archived repo!", + "bots": [], + "teams": [ + { + "name": "foo", + "permission": "admin" + } + ], + "members": [], + "branch_protections": [ + { + "pattern": "master", + "ci_checks": [ + "CI" + ], + "dismiss_stale_review": false + } + ], + "archived": true + }, { "org": "test-org", "name": "some_repo", @@ -24,7 +47,8 @@ "foo" ] } - ] + ], + "archived": false } ] } \ No newline at end of file diff --git a/tests/static-api/_expected/v1/repos/archived_repo.json b/tests/static-api/_expected/v1/repos/archived_repo.json new file mode 100644 index 000000000..2fab13ea9 --- /dev/null +++ b/tests/static-api/_expected/v1/repos/archived_repo.json @@ -0,0 +1,23 @@ +{ + "org": "test-org", + "name": "archived_repo", + "description": "An archived repo!", + "bots": [], + "teams": [ + { + "name": "foo", + "permission": "admin" + } + ], + "members": [], + "branch_protections": [ + { + "pattern": "master", + "ci_checks": [ + "CI" + ], + "dismiss_stale_review": false + } + ], + "archived": true +} \ No newline at end of file diff --git a/tests/static-api/_expected/v1/repos/some_repo.json b/tests/static-api/_expected/v1/repos/some_repo.json index 500e01509..81be10b9e 100644 --- a/tests/static-api/_expected/v1/repos/some_repo.json +++ b/tests/static-api/_expected/v1/repos/some_repo.json @@ -22,5 +22,6 @@ "foo" ] } - ] + ], + "archived": false } \ No newline at end of file diff --git a/tests/static-api/repos/archive/test-org/archived_repo.toml b/tests/static-api/repos/archive/test-org/archived_repo.toml new file mode 100644 index 000000000..c56f03b4e --- /dev/null +++ b/tests/static-api/repos/archive/test-org/archived_repo.toml @@ -0,0 +1,11 @@ +org = "test-org" +name = "archived_repo" +description = "An archived repo!" +bots = [] + +[access.teams] +foo = "admin" + +[[branch-protections]] +pattern = "master" +ci-checks = ["CI"] From 4ba6668106453e550b681fc9c729b81900181593 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Sat, 6 Jan 2024 18:25:03 +0100 Subject: [PATCH 3/5] Add check for duplicated repositories It could now be a problem because we store repositories under the `archive` directory. It could also produce an error before though, if someone set the same org/name combination for two repositories in two different files. --- src/data.rs | 6 +++++- src/validate.rs | 10 ++++++++-- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/src/data.rs b/src/data.rs index ebc7b3112..4c3fbe24f 100644 --- a/src/data.rs +++ b/src/data.rs @@ -27,7 +27,7 @@ impl Data { }; fn validate_repo(org: &str, repo: &Repo, path: &Path) -> anyhow::Result<()> { - if &repo.org != org { + if repo.org != org { bail!( "repo '{}' is located in the '{}' org directory but its org is '{}'", repo.name, @@ -182,6 +182,10 @@ impl Data { self.archived_repos.iter() } + pub(crate) fn all_repos(&self) -> impl Iterator { + self.repos().chain(self.archived_repos()) + } + pub(crate) fn archived_teams(&self) -> impl Iterator { self.archived_teams.iter() } diff --git a/src/validate.rs b/src/validate.rs index 3a7cc9f7b..b17f3b26d 100644 --- a/src/validate.rs +++ b/src/validate.rs @@ -729,11 +729,17 @@ fn validate_zulip_group_extra_people(data: &Data, errors: &mut Vec) { }); } -/// Ensure repos reference valid teams +/// Ensure repos reference valid teams and that they are unique fn validate_repos(data: &Data, errors: &mut Vec) { let allowed_orgs = data.config().allowed_github_orgs(); let github_teams = data.github_teams(); - wrapper(data.repos(), errors, |repo, _| { + let mut repo_map = HashSet::new(); + + wrapper(data.all_repos(), errors, |repo, _| { + if !repo_map.insert(format!("{}/{}", repo.org, repo.name)) { + bail!("The repo {}/{} is duplicated", repo.org, repo.name); + } + if !allowed_orgs.contains(&repo.org) { bail!( "The repo '{}' is in an invalid org '{}'", From d3672e9168e39b36dac468ec099c14ad18443f6c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Sat, 6 Jan 2024 18:25:39 +0100 Subject: [PATCH 4/5] Use `all_repos` in validation --- src/validate.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/validate.rs b/src/validate.rs index b17f3b26d..14f701519 100644 --- a/src/validate.rs +++ b/src/validate.rs @@ -316,7 +316,7 @@ fn validate_inactive_members(data: &Data, errors: &mut Vec) { let all_members = data.people().map(|p| p.github()).collect::>(); // All the individual contributors to any Rust controlled repos let all_ics = data - .repos() + .all_repos() .flat_map(|r| r.access.individuals.keys()) .map(|n| n.as_str()) .collect::>(); From f43aaa1c4c2ea465093e7fa9e1518493f6f96c58 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Mon, 12 Feb 2024 16:42:33 +0100 Subject: [PATCH 5/5] Fix tests --- tests/static-api/_expected/v1/repos.json | 4 +++- tests/static-api/_expected/v1/repos/archived_repo.json | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/tests/static-api/_expected/v1/repos.json b/tests/static-api/_expected/v1/repos.json index 6405a24c6..b618dde46 100644 --- a/tests/static-api/_expected/v1/repos.json +++ b/tests/static-api/_expected/v1/repos.json @@ -18,7 +18,9 @@ "ci_checks": [ "CI" ], - "dismiss_stale_review": false + "dismiss_stale_review": false, + "required_approvals": 1, + "allowed_merge_teams": [] } ], "archived": true diff --git a/tests/static-api/_expected/v1/repos/archived_repo.json b/tests/static-api/_expected/v1/repos/archived_repo.json index 2fab13ea9..71be59fc8 100644 --- a/tests/static-api/_expected/v1/repos/archived_repo.json +++ b/tests/static-api/_expected/v1/repos/archived_repo.json @@ -16,7 +16,9 @@ "ci_checks": [ "CI" ], - "dismiss_stale_review": false + "dismiss_stale_review": false, + "required_approvals": 1, + "allowed_merge_teams": [] } ], "archived": true