Skip to content

Commit e023ccc

Browse files
GuillaumeGomezGuillaumeGomez
committed
Code improvements and cleanup:
 * Cleanup dependencies (remove unneeded funty and reverted systemstat to 0.1.4) * Improve repository_name test (replace macro with a function and added other tests) * Added comments for better code readability * Replaced unneeded loop with a `if let` * Rename newly created DB indexes
1 parent 8fa4772 commit e023ccc

File tree

8 files changed

+94
-52
lines changed

8 files changed

+94
-52
lines changed

‎Cargo.lock

Lines changed: 0 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎Cargo.toml

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ comrak = { version = "0.9.1", default-features = false }
4141
toml = "0.5"
4242
schemamama = "0.3"
4343
schemamama_postgres = "0.3"
44-
systemstat = "0.1.7"
44+
systemstat = "0.1.4"
4545
prometheus = { version = "0.10.0", default-features = false }
4646
rustwide = "0.12"
4747
mime_guess = "2"
@@ -58,7 +58,6 @@ dashmap = "3.11.10"
5858
string_cache = "0.8.0"
5959
postgres-types = { version = "0.2", features = ["derive"] }
6060
getrandom = "0.2.1"
61-
funty = "=1.1.0"
6261

6362
# Async
6463
tokio = { version = "1.0", features = ["rt-multi-thread"] }

‎crates/metadata/lib.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,6 @@ pub const DEFAULT_TARGETS: &[&str] = &[
7171
#[non_exhaustive]
7272
pub enum MetadataError {
7373
/// The error returned when the manifest could not be read.
74-
#[allow(clippy::upper_case_acronyms)]
7574
#[error("failed to read manifest from disk")]
7675
IO(#[from] io::Error),
7776
/// The error returned when the manifest could not be parsed.

‎src/db/migrate.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -681,8 +681,8 @@ pub fn migrate(version: Option<Version>, conn: &mut Client) -> CratesfyiResult<(
681681
DROP INDEX releases_github_repo_idx;
682682
DROP INDEX github_repos_stars_idx;
683683
684-
CREATE INDEX releases_github_repo_idx ON releases(repository_id);
685-
CREATE INDEX github_repos_stars_idx ON repositories(stars DESC);
684+
CREATE INDEX releases_repo_idx ON releases(repository_id);
685+
CREATE INDEX repos_stars_idx ON repositories(stars DESC);
686686
687687
ALTER TABLE releases
688688
DROP COLUMN github_repo;
@@ -720,8 +720,8 @@ pub fn migrate(version: Option<Version>, conn: &mut Client) -> CratesfyiResult<(
720720
WHERE
721721
repository_id IS NOT NULL;
722722
723-
DROP INDEX releases_github_repo_idx;
724-
DROP INDEX github_repos_stars_idx;
723+
DROP INDEX releases_repo_idx;
724+
DROP INDEX repos_stars_idx;
725725
726726
CREATE INDEX releases_github_repo_idx ON releases (github_repo);
727727
CREATE INDEX github_repos_stars_idx ON github_repos(stars DESC);

‎src/repositories/github.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ impl GitHub {
7575
}
7676

7777
impl RepositoryForge for GitHub {
78-
fn host(&self) -> &str {
78+
fn host(&self) -> &'static str {
7979
"github.com"
8080
}
8181

‎src/repositories/gitlab.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ impl GitLab {
6969
}
7070

7171
impl RepositoryForge for GitLab {
72-
fn host(&self) -> &str {
72+
fn host(&self) -> &'static str {
7373
self.host
7474
}
7575

‎src/repositories/updater.rs

Lines changed: 83 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ use std::fmt;
1313
pub trait RepositoryForge {
1414
/// Result used both as the `host` column in the DB and to match repository URLs during
1515
/// backfill.
16-
fn host(&self) -> &str;
16+
fn host(&self) -> &'static str;
1717

1818
/// FontAwesome icon used in the front-end.
1919
fn icon(&self) -> &'static str;
@@ -56,11 +56,11 @@ pub struct RepositoryStatsUpdater {
5656

5757
impl fmt::Debug for RepositoryStatsUpdater {
5858
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
59-
write!(
60-
f,
61-
"RepositoryStatsUpdater {{ {:?} }}",
62-
self.updaters.iter().map(|u| u.host()).collect::<Vec<_>>()
63-
)
59+
write!(f, "RepositoryStatsUpdater {{ updaters: ")?;
60+
f.debug_list()
61+
.entries(self.updaters.iter().map(|u| u.host()))
62+
.finish()?;
63+
write!(f, " }}")
6464
}
6565
}
6666

@@ -104,14 +104,21 @@ impl RepositoryStatsUpdater {
104104
)? {
105105
return Ok(Some(row.get("id")));
106106
}
107-
for updater in self.updaters.iter().filter(|u| u.host() == name.host) {
107+
if let Some(updater) = self.updaters.iter().find(|u| u.host() == name.host) {
108108
let res = match updater.fetch_repository(&name) {
109109
Ok(Some(repo)) => self.store_repository(conn, updater.host(), repo),
110+
Ok(None) => {
111+
warn!(
112+
"failed to fetch repository `{}` on host `{}`",
113+
url,
114+
updater.host()
115+
);
116+
return Ok(None);
117+
}
110118
Err(err) => {
111119
warn!("failed to collect `{}` stats: {}", updater.host(), err);
112120
return Ok(None);
113121
}
114-
_ => continue,
115122
};
116123
return match res {
117124
Ok(repo_id) => Ok(Some(repo_id)),
@@ -148,6 +155,8 @@ impl RepositoryStatsUpdater {
148155
);
149156
continue;
150157
}
158+
// FIXME: The collect can be avoided if we use Itertools::chunks:
159+
// https://docs.rs/itertools/0.10.0/itertools/trait.Itertools.html#method.chunks.
151160
for chunk in needs_update.chunks(updater.chunk_size()) {
152161
let res = updater.fetch_repositories(chunk)?;
153162
for node in res.missing {
@@ -301,45 +310,77 @@ mod test {
301310

302311
#[test]
303312
fn test_repository_name() {
304-
macro_rules! assert_name {
305-
($url:expr => ($owner:expr, $repo:expr, $host:expr)) => {
306-
assert_eq!(
307-
repository_name($url),
308-
Some(RepositoryName {
309-
owner: $owner,
310-
repo: $repo,
311-
host: $host,
312-
})
313-
);
314-
};
315-
($url:expr => None) => {
316-
assert_eq!(repository_name($url), None);
317-
};
313+
fn assert_name(url: &str, data: Option<(&str, &str, &str)>) {
314+
assert_eq!(
315+
repository_name(url),
316+
data.map(|(owner, repo, host)| RepositoryName { owner, repo, host }),
317+
);
318318
}
319319

320320
// gitlab checks
321-
assert_name!("https://gitlab.com/onur/cratesfyi" => ("onur", "cratesfyi", "gitlab.com"));
322-
assert_name!("http://gitlab.com/onur/cratesfyi" => ("onur", "cratesfyi", "gitlab.com"));
323-
assert_name!("https://gitlab.com/onur/cratesfyi.git" => ("onur", "cratesfyi", "gitlab.com"));
324-
assert_name!("https://gitlab.com/docopt/docopt.rs" => ("docopt", "docopt.rs", "gitlab.com"));
325-
assert_name!("https://gitlab.com/onur23cmD_M_R_L_/crates_fy-i" => (
326-
"onur23cmD_M_R_L_", "crates_fy-i", "gitlab.com"
327-
));
328-
assert_name!("https://gitlab.freedesktop.org/test/test" => (
329-
"test", "test", "gitlab.freedesktop.org"
330-
));
331-
assert_name!("https://gitlab.com/artgam3s/public-libraries/rust/rpa" => (
332-
"artgam3s/public-libraries/rust", "rpa", "gitlab.com"
333-
));
321+
assert_name(
322+
"https://gitlab.com/onur/cratesfyi",
323+
Some(("onur", "cratesfyi", "gitlab.com")),
324+
);
325+
assert_name(
326+
"http://gitlab.com/onur/cratesfyi",
327+
Some(("onur", "cratesfyi", "gitlab.com")),
328+
);
329+
assert_name(
330+
"https://gitlab.com/onur/cratesfyi.git",
331+
Some(("onur", "cratesfyi", "gitlab.com")),
332+
);
333+
assert_name(
334+
"https://gitlab.com/docopt/docopt.rs",
335+
Some(("docopt", "docopt.rs", "gitlab.com")),
336+
);
337+
assert_name(
338+
"https://gitlab.com/onur23cmD_M_R_L_/crates_fy-i",
339+
Some(("onur23cmD_M_R_L_", "crates_fy-i", "gitlab.com")),
340+
);
341+
assert_name(
342+
"https://gitlab.freedesktop.org/test1/test2",
343+
Some(("test1", "test2", "gitlab.freedesktop.org")),
344+
);
345+
assert_name(
346+
"https://gitlab.com/artgam3s/public-libraries/rust/rpa",
347+
Some(("artgam3s/public-libraries/rust", "rpa", "gitlab.com")),
348+
);
349+
350+
assert_name("https://gitlab.com/moi/", None);
351+
assert_name("https://gitlab.com/moi", None);
352+
assert_name("https://gitlab.com", None);
353+
assert_name("https://gitlab.com/", None);
334354

335355
// github checks
336-
assert_name!("https://github.com/onur/cratesfyi" => ("onur", "cratesfyi", "github.com"));
337-
assert_name!("http://github.com/onur/cratesfyi" => ("onur", "cratesfyi", "github.com"));
338-
assert_name!("https://github.com/onur/cratesfyi.git" => ("onur", "cratesfyi", "github.com"));
339-
assert_name!("https://github.com/docopt/docopt.rs" => ("docopt", "docopt.rs", "github.com"));
340-
assert_name!("https://github.com/onur23cmD_M_R_L_/crates_fy-i" => (
341-
"onur23cmD_M_R_L_", "crates_fy-i", "github.com"
342-
));
356+
assert_name(
357+
"https://github.com/onur/cratesfyi",
358+
Some(("onur", "cratesfyi", "github.com")),
359+
);
360+
assert_name(
361+
"http://github.com/onur/cratesfyi",
362+
Some(("onur", "cratesfyi", "github.com")),
363+
);
364+
assert_name(
365+
"https://github.com/onur/cratesfyi.git",
366+
Some(("onur", "cratesfyi", "github.com")),
367+
);
368+
assert_name(
369+
"https://github.com/docopt/docopt.rs",
370+
Some(("docopt", "docopt.rs", "github.com")),
371+
);
372+
assert_name(
373+
"https://github.com/onur23cmD_M_R_L_/crates_fy-i",
374+
Some(("onur23cmD_M_R_L_", "crates_fy-i", "github.com")),
375+
);
376+
377+
assert_name("https://github.com/moi/", None);
378+
assert_name("https://github.com/moi", None);
379+
assert_name("https://github.com", None);
380+
assert_name("https://github.com/", None);
381+
382+
// Unknown host
383+
assert_name("https://git.sr.ht/~ireas/merge-rs", None);
343384
}
344385

345386
#[test]

‎src/utils/daemon.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,10 @@ pub fn start_daemon(context: &dyn Context, enable_registry_watcher: bool) -> Res
7272
})
7373
.unwrap();
7474

75+
// This call will still skip github repositories updates and continue if no token is provided
76+
// (gitlab doesn't require to have a token). The only time this can return an error is when
77+
// creating a pool or if config fails, which shouldn't happen here because this is run right at
78+
// startup.
7579
let updater = context.repository_stats_updater()?;
7680
cron(
7781
"repositories stats updater",

0 commit comments

Comments
 (0)