max / makenotwork
4 files changed,
+517 insertions,
-4 deletions
| @@ -19,14 +19,17 @@ | |||
| 19 | 19 | //! - **Visibility is not stored and is not checked here.** These functions take | |
| 20 | 20 | //! a repo id and answer about that repository. Whether the person asking may | |
| 21 | 21 | //! see it is the caller's question, decided the same way it is for the | |
| 22 | - | //! repository's other pages. | |
| 22 | + | //! repository's other pages. The personal-annotation functions are the same | |
| 23 | + | //! rule in a sharper form: they scope by repository ownership, so the | |
| 24 | + | //! `user_id` they are handed must be the signed-in account's, and passing any | |
| 25 | + | //! other shows one person another person's private writing. | |
| 23 | 26 | ||
| 24 | 27 | use std::collections::HashMap; | |
| 25 | 28 | ||
| 26 | 29 | use chrono::{DateTime, Utc}; | |
| 27 | 30 | use sqlx::PgPool; | |
| 28 | 31 | ||
| 29 | - | use super::GitRepoId; | |
| 32 | + | use super::{GitRepoId, UserId}; | |
| 30 | 33 | use crate::error::Result; | |
| 31 | 34 | ||
| 32 | 35 | /// One note as the reindex hands it over. | |
| @@ -58,6 +61,26 @@ | |||
| 58 | 61 | pub updated_by: String, | |
| 59 | 62 | } | |
| 60 | 63 | ||
| 64 | + | /// One personal annotation: a note in the viewer's own annotation repository | |
| 65 | + | /// against an object that lives in somebody else's. | |
| 66 | + | /// | |
| 67 | + | /// `repo_id` and `repo_name` are the annotation repository's, not the annotated | |
| 68 | + | /// commit's. Which repository serves the target is not knowable from a row here | |
| 69 | + | /// and is not meant to be: a hash is global, and the question of who still | |
| 70 | + | /// carries that object is asked of the repositories the viewer can see, at the | |
| 71 | + | /// moment the page renders. | |
| 72 | + | #[derive(Debug, Clone, sqlx::FromRow)] | |
| 73 | + | pub struct PersonalAnnotation { | |
| 74 | + | pub repo_id: GitRepoId, | |
| 75 | + | pub repo_name: String, | |
| 76 | + | pub namespace: String, | |
| 77 | + | pub target_oid: String, | |
| 78 | + | pub blob_oid: String, | |
| 79 | + | pub content: String, | |
| 80 | + | pub updated_at: DateTime<Utc>, | |
| 81 | + | pub updated_by: String, | |
| 82 | + | } | |
| 83 | + | ||
| 61 | 84 | /// The tip a namespace was last indexed from, or `None` when it has never been | |
| 62 | 85 | /// indexed. | |
| 63 | 86 | /// | |
| @@ -385,3 +408,159 @@ | |||
| 385 | 408 | .await?; | |
| 386 | 409 | Ok(rows) | |
| 387 | 410 | } | |
| 411 | + | ||
| 412 | + | // --- Personal annotations --- | |
| 413 | + | // | |
| 414 | + | // The other direction from the rest of this file. Everything above answers | |
| 415 | + | // "what does this repository carry"; a commit page has to ask "does the person | |
| 416 | + | // reading have anything against this hash", which is a question about an | |
| 417 | + | // account rather than about a repository. | |
| 418 | + | // | |
| 419 | + | // It is a two-table question and stays one. No user id is denormalized onto | |
| 420 | + | // `git_notes`: `git_repos.user_id` already records who owns the repository a | |
| 421 | + | // note lives in, a copy would need a backfill the day a repository changes | |
| 422 | + | // hands, and an account owns one annotation repository, so the `git_repos` half | |
| 423 | + | // of the join is a single row reached through `idx_git_repos_one_annotation_repo`. | |
| 424 | + | // The `git_notes` half is `idx_git_notes_target (repo_id, target_oid)`, which | |
| 425 | + | // migration 197 already ships. | |
| 426 | + | ||
| 427 | + | /// Every annotation `user_id` has written against `target_oid`, across their | |
| 428 | + | /// annotation repositories. | |
| 429 | + | /// | |
| 430 | + | /// The commit page's query. The rows are a projection: the annotation repository | |
| 431 | + | /// holds the note and this table can be dropped and rebuilt from it, so a miss | |
| 432 | + | /// here means "the index has not caught up", never "there is no such note". | |
| 433 | + | /// | |
| 434 | + | /// `user_id` is the session's. This module checks nothing about who is asking. | |
| 435 | + | /// | |
| 436 | + | /// An annotation whose target no repository the viewer can see still serves is | |
| 437 | + | /// returned like any other. Nothing collects those: a commit can go away with | |
| 438 | + | /// the repository that carried it and the sentence somebody wrote about it is | |
| 439 | + | /// still theirs. | |
| 440 | + | #[tracing::instrument(skip_all)] | |
| 441 | + | pub async fn annotations_by_user_for_target( | |
| 442 | + | pool: &PgPool, | |
| 443 | + | user_id: UserId, | |
| 444 | + | target_oid: &str, | |
| 445 | + | ) -> Result<Vec<PersonalAnnotation>> { | |
| 446 | + | let rows = sqlx::query_as::<_, PersonalAnnotation>( | |
| 447 | + | "SELECT r.id AS repo_id, r.name AS repo_name, n.namespace, n.target_oid, | |
| 448 | + | n.blob_oid, n.content, n.updated_at, n.updated_by | |
| 449 | + | FROM git_notes n | |
| 450 | + | JOIN git_repos r ON r.id = n.repo_id | |
| 451 | + | WHERE r.user_id = $1 AND r.kind = 'annotations' AND n.target_oid = $2 | |
| 452 | + | ORDER BY r.name, n.namespace", | |
| 453 | + | ) | |
| 454 | + | .bind(user_id) | |
| 455 | + | .bind(target_oid) | |
| 456 | + | .fetch_all(pool) | |
| 457 | + | .await?; | |
| 458 | + | Ok(rows) | |
| 459 | + | } | |
| 460 | + | ||
| 461 | + | /// How many annotations `user_id` has against each of `targets`. | |
| 462 | + | /// | |
| 463 | + | /// The log page's badge: one query for a page of commits, keyed by the hex the | |
| 464 | + | /// caller passed in. Targets with no annotation are absent from the map rather | |
| 465 | + | /// than present with a zero. | |
| 466 | + | /// | |
| 467 | + | /// The same projection and the same trust: the counts are the index's, and | |
| 468 | + | /// `user_id` has to be the signed-in account's because nothing here checks it. | |
| 469 | + | #[tracing::instrument(skip_all)] | |
| 470 | + | pub async fn annotation_counts_for_user( | |
| 471 | + | pool: &PgPool, | |
| 472 | + | user_id: UserId, | |
| 473 | + | targets: &[String], | |
| 474 | + | ) -> Result<HashMap<String, i64>> { | |
| 475 | + | if targets.is_empty() { | |
| 476 | + | return Ok(HashMap::new()); | |
| 477 | + | } | |
| 478 | + | let rows = sqlx::query_as::<_, (String, i64)>( | |
| 479 | + | "SELECT n.target_oid, COUNT(*) | |
| 480 | + | FROM git_notes n | |
| 481 | + | JOIN git_repos r ON r.id = n.repo_id | |
| 482 | + | WHERE r.user_id = $1 AND r.kind = 'annotations' AND n.target_oid = ANY($2) | |
| 483 | + | GROUP BY n.target_oid", | |
| 484 | + | ) | |
| 485 | + | .bind(user_id) | |
| 486 | + | .bind(targets) | |
| 487 | + | .fetch_all(pool) | |
| 488 | + | .await?; | |
| 489 | + | Ok(rows.into_iter().collect()) | |
| 490 | + | } | |
| 491 | + | ||
| 492 | + | /// Everything `user_id` has ever annotated, newest first. | |
| 493 | + | /// | |
| 494 | + | /// Ordered by when the annotation was written rather than by the annotated | |
| 495 | + | /// commit's own date, for the reason the repository feed is: annotating a | |
| 496 | + | /// five-year-old commit is news. A full rebuild restamps `updated_at` from the | |
| 497 | + | /// notes ref's committer time, so a rebuilt timeline is the repository's, not | |
| 498 | + | /// the original keystrokes'. | |
| 499 | + | /// | |
| 500 | + | /// A projection, `user_id` is the session's, and orphans are included: an | |
| 501 | + | /// annotation whose target nothing serves any more is a row like any other. | |
| 502 | + | #[tracing::instrument(skip_all)] | |
| 503 | + | pub async fn user_annotations( | |
| 504 | + | pool: &PgPool, | |
| 505 | + | user_id: UserId, | |
| 506 | + | limit: i64, | |
| 507 | + | offset: i64, | |
| 508 | + | ) -> Result<Vec<PersonalAnnotation>> { | |
| 509 | + | let rows = sqlx::query_as::<_, PersonalAnnotation>( | |
| 510 | + | "SELECT r.id AS repo_id, r.name AS repo_name, n.namespace, n.target_oid, | |
| 511 | + | n.blob_oid, n.content, n.updated_at, n.updated_by | |
| 512 | + | FROM git_notes n | |
| 513 | + | JOIN git_repos r ON r.id = n.repo_id | |
| 514 | + | WHERE r.user_id = $1 AND r.kind = 'annotations' | |
| 515 | + | ORDER BY n.updated_at DESC, n.target_oid | |
| 516 | + | LIMIT $2 OFFSET $3", | |
| 517 | + | ) | |
| 518 | + | .bind(user_id) | |
| 519 | + | .bind(limit) | |
| 520 | + | .bind(offset) | |
| 521 | + | .fetch_all(pool) | |
| 522 | + | .await?; | |
| 523 | + | Ok(rows) | |
| 524 | + | } | |
| 525 | + | ||
| 526 | + | /// How many annotations `user_id` has, for the page count beside the rows. | |
| 527 | + | /// | |
| 528 | + | /// The index's count, which is the same projection the list is. `user_id` is | |
| 529 | + | /// the session's, and orphans are counted because they are shown. | |
| 530 | + | #[tracing::instrument(skip_all)] | |
| 531 | + | pub async fn count_user_annotations(pool: &PgPool, user_id: UserId) -> Result<i64> { | |
| 532 | + | let count = sqlx::query_scalar::<_, i64>( | |
| 533 | + | "SELECT COUNT(*) | |
| 534 | + | FROM git_notes n | |
| 535 | + | JOIN git_repos r ON r.id = n.repo_id | |
| 536 | + | WHERE r.user_id = $1 AND r.kind = 'annotations'", | |
| 537 | + | ) | |
| 538 | + | .bind(user_id) | |
| 539 | + | .fetch_one(pool) | |
| 540 | + | .await?; | |
| 541 | + | Ok(count) | |
| 542 | + | } | |
| 543 | + | ||
| 544 | + | /// Whether this account's annotations have ever been indexed. | |
| 545 | + | /// | |
| 546 | + | /// `is_indexed`'s twin, and needed for the same reason: an empty result cannot | |
| 547 | + | /// tell a cold index from an account that has annotated nothing. A caller that | |
| 548 | + | /// skips this reports "no annotations" to somebody whose annotations predate | |
| 549 | + | /// the index, and the answer to a cold index is to walk the annotation | |
| 550 | + | /// repository instead. | |
| 551 | + | /// | |
| 552 | + | /// `user_id` is the session's; this module checks nothing about who is asking. | |
| 553 | + | #[tracing::instrument(skip_all)] | |
| 554 | + | pub async fn user_annotation_index_is_warm(pool: &PgPool, user_id: UserId) -> Result<bool> { | |
| 555 | + | let found = sqlx::query_scalar::<_, bool>( | |
| 556 | + | "SELECT EXISTS ( | |
| 557 | + | SELECT 1 FROM git_notes_index_state s | |
| 558 | + | JOIN git_repos r ON r.id = s.repo_id | |
| 559 | + | WHERE r.user_id = $1 AND r.kind = 'annotations' | |
| 560 | + | )", | |
| 561 | + | ) | |
| 562 | + | .bind(user_id) | |
| 563 | + | .fetch_one(pool) | |
| 564 | + | .await?; | |
| 565 | + | Ok(found) | |
| 566 | + | } |
| @@ -573,6 +573,17 @@ | |||
| 573 | 573 | } | |
| 574 | 574 | } | |
| 575 | 575 | ||
| 576 | + | /// The config the app was built with. | |
| 577 | + | /// | |
| 578 | + | /// For the handful of library entry points a test drives directly rather | |
| 579 | + | /// than through the router: `notes_index::reindex_repo` and the other | |
| 580 | + | /// admin-side rebuilds take a `Config` because they open repositories off | |
| 581 | + | /// disk, and the path they open is the harness's temporary directory. | |
| 582 | + | #[allow(dead_code)] | |
| 583 | + | pub(crate) fn config(&self) -> &makenotwork::config::Config { | |
| 584 | + | &self.state.config | |
| 585 | + | } | |
| 586 | + | ||
| 576 | 587 | /// Run the orphaned-upload reaper once, synchronously. | |
| 577 | 588 | /// | |
| 578 | 589 | /// The scheduler drives this on a tick in production. Tests that want the |
| @@ -488,3 +488,298 @@ | |||
| 488 | 488 | assert_ne!(resp.status, 303, "an anonymous write must not succeed"); | |
| 489 | 489 | assert_eq!(note_in_repo(&tmp, "commits", &sha), None); | |
| 490 | 490 | } | |
| 491 | + | ||
| 492 | + | // --- Personal annotations, indexed by (user, target oid) --- | |
| 493 | + | // | |
| 494 | + | // The other direction from everything above: the note lives in the reader's own | |
| 495 | + | // annotation repository and targets a commit in somebody else's, so the commit | |
| 496 | + | // page's question is about an account rather than about a repository. These | |
| 497 | + | // assert the index answers it, and that the index is only ever a projection of | |
| 498 | + | // what the annotation repository holds. | |
| 499 | + | ||
| 500 | + | /// The account's annotation repository row: its id and its owner's id. | |
| 501 | + | async fn annotation_repo_row( | |
| 502 | + | h: &TestHarness, | |
| 503 | + | username: &str, | |
| 504 | + | ) -> (makenotwork::db::GitRepoId, makenotwork::db::UserId) { | |
| 505 | + | sqlx::query_as::<_, (makenotwork::db::GitRepoId, makenotwork::db::UserId)>( | |
| 506 | + | "SELECT r.id, r.user_id FROM git_repos r JOIN users u ON u.id = r.user_id | |
| 507 | + | WHERE u.username = $1 AND r.kind = 'annotations'", | |
| 508 | + | ) | |
| 509 | + | .bind(username) | |
| 510 | + | .fetch_one(&h.db) | |
| 511 | + | .await | |
| 512 | + | .expect("the annotation repository row exists") | |
| 513 | + | } | |
| 514 | + | ||
| 515 | + | async fn user_id(h: &TestHarness, username: &str) -> makenotwork::db::UserId { | |
| 516 | + | sqlx::query_scalar::<_, makenotwork::db::UserId>("SELECT id FROM users WHERE username = $1") | |
| 517 | + | .bind(username) | |
| 518 | + | .fetch_one(&h.db) | |
| 519 | + | .await | |
| 520 | + | .unwrap() | |
| 521 | + | } | |
| 522 | + | ||
| 523 | + | /// Write a note straight into a bare repository's `refs/notes/commits`, the way | |
| 524 | + | /// a push leaves it. `push_notes` is hard-wired to `testowner/testrepo`; this is | |
| 525 | + | /// the same thing against an arbitrary repository directory. | |
| 526 | + | fn push_notes_into(repo_dir: &std::path::Path, target: &str, body: &str) { | |
| 527 | + | use makenotwork::git::notes::{self, GixEngine, NoteObjects, NoteWrites, Oid, Signature}; | |
| 528 | + | ||
| 529 | + | let repo = gix::open(repo_dir).unwrap(); | |
| 530 | + | let engine = GixEngine::new(&repo); | |
| 531 | + | let who = Signature { | |
| 532 | + | name: "Annotator".into(), | |
| 533 | + | email: "annotator@users.makenot.work".into(), | |
| 534 | + | time: chrono::Utc::now(), | |
| 535 | + | }; | |
| 536 | + | let full_ref = "refs/notes/commits"; | |
| 537 | + | let existing = engine.resolve_ref(full_ref).unwrap(); | |
| 538 | + | let root = existing.map(|tip| engine.read_commit(tip).unwrap().tree); | |
| 539 | + | let blob = engine.write_blob(body.as_bytes()).unwrap(); | |
| 540 | + | let tree = notes::splice_note( | |
| 541 | + | &engine, | |
| 542 | + | root, | |
| 543 | + | Oid::from_hex(target.as_bytes()).unwrap(), | |
| 544 | + | Some(blob), | |
| 545 | + | ) | |
| 546 | + | .unwrap() | |
| 547 | + | .expect("the push changes something"); | |
| 548 | + | let commit = engine | |
| 549 | + | .write_commit(tree, existing.as_slice(), &who, &who, "notes: pushed\n") | |
| 550 | + | .unwrap(); | |
| 551 | + | engine.update_ref_cas(full_ref, existing, commit).unwrap(); | |
| 552 | + | } | |
| 553 | + | ||
| 554 | + | /// Sign up a second account and have it annotate `sha` in `testowner/testrepo`. | |
| 555 | + | /// Leaves that account logged in. | |
| 556 | + | async fn annotate_as_stranger(h: &mut TestHarness, sha: &str, body: &str) { | |
| 557 | + | h.signup("annotator", "annotator@example.com", "password123") | |
| 558 | + | .await; | |
| 559 | + | let resp = h | |
| 560 | + | .client | |
| 561 | + | .post_form( | |
| 562 | + | &format!("/git/testowner/testrepo/commit/{sha}/annotate"), | |
| 563 | + | &format!("content={}", urlencoding::encode(body)), | |
| 564 | + | ) | |
| 565 | + | .await; | |
| 566 | + | assert_eq!(resp.status, 303, "{}", resp.text); | |
| 567 | + | } | |
| 568 | + | ||
| 569 | + | #[tokio::test] | |
| 570 | + | async fn an_annotation_on_somebody_elses_commit_is_found_by_target_alone() { | |
| 571 | + | let tmp = tempfile::TempDir::new().unwrap(); | |
| 572 | + | let (mut h, sha) = setup(&tmp).await; | |
| 573 | + | annotate_as_stranger(&mut h, &sha, "this is the commit that broke it").await; | |
| 574 | + | ||
| 575 | + | let annotator = user_id(&h, "annotator").await; | |
| 576 | + | let found = makenotwork::db::git_notes::annotations_by_user_for_target(&h.db, annotator, &sha) | |
| 577 | + | .await | |
| 578 | + | .unwrap(); | |
| 579 | + | assert_eq!( | |
| 580 | + | found.len(), | |
| 581 | + | 1, | |
| 582 | + | "one annotation, keyed by the viewer and the hash" | |
| 583 | + | ); | |
| 584 | + | assert_eq!(found[0].content, "this is the commit that broke it\n"); | |
| 585 | + | assert_eq!(found[0].namespace, "commits"); | |
| 586 | + | assert_eq!(found[0].target_oid, sha); | |
| 587 | + | assert_eq!( | |
| 588 | + | found[0].repo_name, | |
| 589 | + | makenotwork::constants::ANNOTATION_REPO_NAME, | |
| 590 | + | "the row names the annotation repository, not the repository browsed" | |
| 591 | + | ); | |
| 592 | + | ||
| 593 | + | // The lookup is scoped by ownership and nothing else, so the person whose | |
| 594 | + | // commit it is sees nothing of it. | |
| 595 | + | let owner = user_id(&h, "testowner").await; | |
| 596 | + | let theirs = makenotwork::db::git_notes::annotations_by_user_for_target(&h.db, owner, &sha) | |
| 597 | + | .await | |
| 598 | + | .unwrap(); | |
| 599 | + | assert!( | |
| 600 | + | theirs.is_empty(), | |
| 601 | + | "somebody else's private annotation reached the commit's owner" | |
| 602 | + | ); | |
| 603 | + | ||
| 604 | + | // The count path the log page uses agrees with the list path. | |
| 605 | + | let counts = makenotwork::db::git_notes::annotation_counts_for_user( | |
| 606 | + | &h.db, | |
| 607 | + | annotator, | |
| 608 | + | std::slice::from_ref(&sha), | |
| 609 | + | ) | |
| 610 | + | .await | |
| 611 | + | .unwrap(); | |
| 612 | + | assert_eq!(counts.get(&sha), Some(&1)); | |
| 613 | + | assert!( | |
| 614 | + | makenotwork::db::git_notes::user_annotation_index_is_warm(&h.db, annotator) | |
| 615 | + | .await | |
| 616 | + | .unwrap(), | |
| 617 | + | "an annotation was written, so the index is warm and an empty read would mean empty" | |
| 618 | + | ); | |
| 619 | + | assert!( | |
| 620 | + | !makenotwork::db::git_notes::user_annotation_index_is_warm(&h.db, owner) | |
| 621 | + | .await | |
| 622 | + | .unwrap(), | |
| 623 | + | "an account with no annotation repository has a cold index, not an empty one" | |
| 624 | + | ); | |
| 625 | + | } | |
| 626 | + | ||
| 627 | + | #[tokio::test] | |
| 628 | + | async fn an_ordinary_repositorys_notes_never_answer_the_personal_lookup() { | |
| 629 | + | let tmp = tempfile::TempDir::new().unwrap(); | |
| 630 | + | let (mut h, sha) = setup(&tmp).await; | |
| 631 | + | h.login("testowner", "password123").await; | |
| 632 | + | let resp = h | |
| 633 | + | .client | |
| 634 | + | .post_form( | |
| 635 | + | &format!("/git/testowner/testrepo/commit/{sha}/notes"), | |
| 636 | + | "namespace=commits&content=a+repo+note", | |
| 637 | + | ) | |
| 638 | + | .await; | |
| 639 | + | assert_eq!(resp.status, 303, "{}", resp.text); | |
| 640 | + | ||
| 641 | + | // The note is indexed, against the source repository. | |
| 642 | + | let repo_id = sqlx::query_scalar::<_, makenotwork::db::GitRepoId>( | |
| 643 | + | "SELECT id FROM git_repos WHERE name = 'testrepo'", | |
| 644 | + | ) | |
| 645 | + | .fetch_one(&h.db) | |
| 646 | + | .await | |
| 647 | + | .unwrap(); | |
| 648 | + | let counts = | |
| 649 | + | makenotwork::db::git_notes::annotation_counts(&h.db, repo_id, std::slice::from_ref(&sha)) | |
| 650 | + | .await | |
| 651 | + | .unwrap(); | |
| 652 | + | assert_eq!(counts.get(&sha), Some(&1), "the repo note is in the index"); | |
| 653 | + | ||
| 654 | + | // A repo note is not a personal annotation. The predicate is the repository's | |
| 655 | + | // kind, so an owner's notes on their own repository never leak into it. | |
| 656 | + | let owner = user_id(&h, "testowner").await; | |
| 657 | + | let personal = makenotwork::db::git_notes::annotations_by_user_for_target(&h.db, owner, &sha) | |
| 658 | + | .await | |
| 659 | + | .unwrap(); | |
| 660 | + | assert!( | |
| 661 | + | personal.is_empty(), | |
| 662 | + | "a note in a source repository answered the personal lookup" | |
| 663 | + | ); | |
| 664 | + | assert_eq!( | |
| 665 | + | makenotwork::db::git_notes::count_user_annotations(&h.db, owner) | |
| 666 | + | .await | |
| 667 | + | .unwrap(), | |
| 668 | + | 0 | |
| 669 | + | ); | |
| 670 | + | } | |
| 671 | + | ||
| 672 | + | #[tokio::test] | |
| 673 | + | async fn an_annotation_whose_target_no_repository_serves_is_still_returned() { | |
| 674 | + | let tmp = tempfile::TempDir::new().unwrap(); | |
| 675 | + | let (mut h, sha) = setup(&tmp).await; | |
| 676 | + | annotate_as_stranger(&mut h, &sha, "still here").await; | |
| 677 | + | ||
| 678 | + | let (repo_id, annotator) = annotation_repo_row(&h, "annotator").await; | |
| 679 | + | let orphan = "0123456789abcdef0123456789abcdef01234567"; | |
| 680 | + | let repo_dir = tmp.path().join("annotator").join(format!( | |
| 681 | + | "{}.git", | |
| 682 | + | makenotwork::constants::ANNOTATION_REPO_NAME | |
| 683 | + | )); | |
| 684 | + | push_notes_into(&repo_dir, orphan, "a commit nobody serves any more\n"); | |
| 685 | + | makenotwork::routes::git::notes_index::reindex_repo( | |
| 686 | + | &h.db, | |
| 687 | + | h.config(), | |
| 688 | + | repo_id, | |
| 689 | + | "annotator", | |
| 690 | + | makenotwork::constants::ANNOTATION_REPO_NAME, | |
| 691 | + | ) | |
| 692 | + | .await | |
| 693 | + | .unwrap(); | |
| 694 | + | ||
| 695 | + | let found = | |
| 696 | + | makenotwork::db::git_notes::annotations_by_user_for_target(&h.db, annotator, orphan) | |
| 697 | + | .await | |
| 698 | + | .unwrap(); | |
| 699 | + | assert_eq!( | |
| 700 | + | found.len(), | |
| 701 | + | 1, | |
| 702 | + | "an annotation is not deleted because its target went away" | |
| 703 | + | ); | |
| 704 | + | assert_eq!(found[0].content, "a commit nobody serves any more\n"); | |
| 705 | + | assert_eq!( | |
| 706 | + | makenotwork::db::git_notes::count_user_annotations(&h.db, annotator) | |
| 707 | + | .await | |
| 708 | + | .unwrap(), | |
| 709 | + | 2, | |
| 710 | + | "orphans are listed with everything else, because nothing collects them" | |
| 711 | + | ); | |
| 712 | + | let all = makenotwork::db::git_notes::user_annotations(&h.db, annotator, 50, 0) | |
| 713 | + | .await | |
| 714 | + | .unwrap(); | |
| 715 | + | assert!(all.iter().any(|a| a.target_oid == orphan)); | |
| 716 | + | assert!(all.iter().any(|a| a.target_oid == sha)); | |
| 717 | + | } | |
| 718 | + | ||
| 719 | + | #[tokio::test] | |
| 720 | + | async fn the_personal_index_rebuilds_from_the_annotation_repository() { | |
| 721 | + | let tmp = tempfile::TempDir::new().unwrap(); | |
| 722 | + | let (mut h, sha) = setup(&tmp).await; | |
| 723 | + | annotate_as_stranger(&mut h, &sha, "the one on a live commit").await; | |
| 724 | + | ||
| 725 | + | let (repo_id, annotator) = annotation_repo_row(&h, "annotator").await; | |
| 726 | + | let orphan = "0123456789abcdef0123456789abcdef01234567"; | |
| 727 | + | let repo_dir = tmp.path().join("annotator").join(format!( | |
| 728 | + | "{}.git", | |
| 729 | + | makenotwork::constants::ANNOTATION_REPO_NAME | |
| 730 | + | )); | |
| 731 | + | push_notes_into(&repo_dir, orphan, "the one on a commit nobody serves\n"); | |
| 732 | + | let reindex = || async { | |
| 733 | + | makenotwork::routes::git::notes_index::reindex_repo( | |
| 734 | + | &h.db, | |
| 735 | + | h.config(), | |
| 736 | + | repo_id, | |
| 737 | + | "annotator", | |
| 738 | + | makenotwork::constants::ANNOTATION_REPO_NAME, | |
| 739 | + | ) | |
| 740 | + | .await | |
| 741 | + | .unwrap(); | |
| 742 | + | }; | |
| 743 | + | reindex().await; | |
| 744 | + | ||
| 745 | + | let mut before = makenotwork::db::git_notes::user_annotations(&h.db, annotator, 50, 0) | |
| 746 | + | .await | |
| 747 | + | .unwrap(); | |
| 748 | + | before.sort_by(|a, b| a.target_oid.cmp(&b.target_oid)); | |
| 749 | + | assert_eq!(before.len(), 2); | |
| 750 | + | ||
| 751 | + | // The load-bearing rule: Postgres holds nothing the repositories do not. | |
| 752 | + | sqlx::query("DELETE FROM git_notes") | |
| 753 | + | .execute(&h.db) | |
| 754 | + | .await | |
| 755 | + | .unwrap(); | |
| 756 | + | sqlx::query("DELETE FROM git_notes_index_state") | |
| 757 | + | .execute(&h.db) | |
| 758 | + | .await | |
| 759 | + | .unwrap(); | |
| 760 | + | assert!( | |
| 761 | + | !makenotwork::db::git_notes::user_annotation_index_is_warm(&h.db, annotator) | |
| 762 | + | .await | |
| 763 | + | .unwrap(), | |
| 764 | + | "a dropped index reads as cold, which is what stops an empty result being believed" | |
| 765 | + | ); | |
| 766 | + | ||
| 767 | + | reindex().await; | |
| 768 | + | ||
| 769 | + | let mut after = makenotwork::db::git_notes::user_annotations(&h.db, annotator, 50, 0) | |
| 770 | + | .await | |
| 771 | + | .unwrap(); | |
| 772 | + | after.sort_by(|a, b| a.target_oid.cmp(&b.target_oid)); | |
| 773 | + | assert_eq!(after.len(), 2, "the rebuild found both, orphan included"); | |
| 774 | + | for (was, now) in before.iter().zip(after.iter()) { | |
| 775 | + | assert_eq!(was.target_oid, now.target_oid); | |
| 776 | + | assert_eq!(was.blob_oid, now.blob_oid); | |
| 777 | + | assert_eq!(was.content, now.content); | |
| 778 | + | assert_eq!(was.namespace, now.namespace); | |
| 779 | + | } | |
| 780 | + | assert!( | |
| 781 | + | makenotwork::db::git_notes::user_annotation_index_is_warm(&h.db, annotator) | |
| 782 | + | .await | |
| 783 | + | .unwrap() | |
| 784 | + | ); | |
| 785 | + | } |
| @@ -87,6 +87,13 @@ | |||
| 87 | 87 | let previous = db::git_notes::indexed_tip(db_pool, repo_id, namespace).await?; | |
| 88 | 88 | let root = repos_root(config)?; | |
| 89 | 89 | ||
| 90 | + | // An annotation repository's notes target objects it does not contain, so | |
| 91 | + | // the target lookup every other repository does is a guaranteed miss here. | |
| 92 | + | // Reading the kind once beats resolving nothing once per note. | |
| 93 | + | let is_annotation = db::git_repos::get_repo_by_id(db_pool, repo_id) | |
| 94 | + | .await? | |
| 95 | + | .is_some_and(|repo| db::git_repos::is_annotation_repo(&repo)); | |
| 96 | + | ||
| 90 | 97 | let owned = ( | |
| 91 | 98 | owner.to_string(), | |
| 92 | 99 | repo_name.to_string(), | |
| @@ -94,7 +101,14 @@ | |||
| 94 | 101 | ); | |
| 95 | 102 | let outcome = tokio::task::spawn_blocking(move || { | |
| 96 | 103 | let (owner, repo_name, namespace) = owned; | |
| 97 | - | plan(&root, &owner, &repo_name, &namespace, previous.as_deref()) | |
| 104 | + | plan( | |
| 105 | + | &root, | |
| 106 | + | &owner, | |
| 107 | + | &repo_name, | |
| 108 | + | &namespace, | |
| 109 | + | previous.as_deref(), | |
| 110 | + | is_annotation, | |
| 111 | + | ) | |
| 98 | 112 | }) | |
| 99 | 113 | .await | |
| 100 | 114 | .map_err(|e| AppError::Internal(anyhow::anyhow!("notes reindex task failed: {e}")))??; | |
| @@ -154,12 +168,16 @@ | |||
| 154 | 168 | /// | |
| 155 | 169 | /// Blocking: this opens a repository and walks trees. Everything it returns is | |
| 156 | 170 | /// owned, so the async half touches no git object. | |
| 171 | + | /// | |
| 172 | + | /// `is_annotation` says the targets live somewhere else, which decides whether | |
| 173 | + | /// the target facts are worth looking for at all. | |
| 157 | 174 | fn plan( | |
| 158 | 175 | root: &std::path::Path, | |
| 159 | 176 | owner: &str, | |
| 160 | 177 | repo_name: &str, | |
| 161 | 178 | namespace: &str, | |
| 162 | 179 | previous: Option<&str>, | |
| 180 | + | is_annotation: bool, | |
| 163 | 181 | ) -> Result<Outcome> { | |
| 164 | 182 | let repo = crate::git::open_repo(root, owner, repo_name)?; | |
| 165 | 183 | let engine = GixEngine::new(&repo); | |
| @@ -193,7 +211,17 @@ | |||
| 193 | 211 | notes::NoteChange::Set { target, blob } => { | |
| 194 | 212 | buffer.clear(); | |
| 195 | 213 | engine.read_blob_into(blob, &mut buffer).map_err(to_app)?; | |
| 196 | - | let (target_is_commit, target_summary, target_time) = target_facts(&repo, target); | |
| 214 | + | // In an annotation repository the annotated object is by | |
| 215 | + | // construction not here, so `target_facts` would miss for every | |
| 216 | + | // row. Saying so outright keeps the columns honest: for these | |
| 217 | + | // rows `target_is_commit` is false and means nothing. It is not | |
| 218 | + | // an orphan marker. Whether a target is still served is decided | |
| 219 | + | // at render time by asking the repositories the viewer can see. | |
| 220 | + | let (target_is_commit, target_summary, target_time) = if is_annotation { | |
| 221 | + | (false, String::new(), None) | |
| 222 | + | } else { | |
| 223 | + | target_facts(&repo, target) | |
| 224 | + | }; | |
| 197 | 225 | upserts.push(db::git_notes::NoteUpsert { | |
| 198 | 226 | target_oid: target.to_hex(), | |
| 199 | 227 | blob_oid: blob.to_hex(), |