steid
Sign in

@jamesgill /

steid

steid/src/domain/pull_request.rs
Rust23.6 KBCode·Blame·History·Raw
1//! Pull requests — a numbered conversation about merging one branch into another.
2//!
3//! Deliberately the [issue](super::issue) shape with a merge attached, down to sharing
4//! [`IssueNumber`] and its per-repository sequence: `#7` names one thing on a
5//! repository whatever kind of thing it is, which is the property that makes a bare
6//! `#7` in a body a link somebody can follow without knowing what they are following.
7//! The two title-and-body rules are literally the same function, so a cap can never
8//! move for one and not the other.
9//!
10//! **Same repository only — no forks.** A pull request names two [`RefName`]s in one
11//! repository, and both are read live: nothing about where the source branch points is
12//! stored while the request is open, so the page follows the branch, a force-push
13//! included. What is stored is the outcome — [`merge_commit`](PullRequest::merge_commit)
14//! records what actually landed, which is the one fact the branches cannot answer later
15//! because either of them may be gone by then.
16
17use std::{fmt, str::FromStr, time::SystemTime};
18
19use super::{
20 DomainError, ObjectId, PullRequestCommentId, PullRequestId, RefName, RepoId, UserId,
21 issue::{IssueNumber, validate_content},
22};
23
24/// Where a pull request has got to.
25///
26/// Three states rather than the issue's two, and the third is the whole point: a closed
27/// pull request was abandoned, a merged one landed, and a tracker that could not tell
28/// them apart would say a branch was rejected when it shipped.
29///
30/// **Merged is terminal.** Closed reopens, merged does not: reopening would claim a
31/// commit that is already in the history can be un-landed by a button on a web page.
32#[derive(Debug, Clone, Copy, Default, PartialEq, Eq)]
33pub enum PullRequestState {
34 #[default]
35 Open,
36 Merged,
37 Closed,
38}
39
40impl PullRequestState {
41 pub fn as_str(self) -> &'static str {
42 match self {
43 Self::Open => "open",
44 Self::Merged => "merged",
45 Self::Closed => "closed",
46 }
47 }
48
49 pub fn is_open(self) -> bool {
50 self == Self::Open
51 }
52
53 pub fn is_merged(self) -> bool {
54 self == Self::Merged
55 }
56}
57
58impl fmt::Display for PullRequestState {
59 fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
60 f.write_str(self.as_str())
61 }
62}
63
64/// `Result`, never `Option`, for [`IssueState`](super::IssueState)'s reason: a state
65/// that silently defaulted to `Open` would reopen every request whose row failed to
66/// read — and here it would also un-merge one.
67impl FromStr for PullRequestState {
68 type Err = DomainError;
69
70 fn from_str(value: &str) -> Result<Self, Self::Err> {
71 match value {
72 "open" => Ok(Self::Open),
73 "merged" => Ok(Self::Merged),
74 "closed" => Ok(Self::Closed),
75 other => Err(DomainError::validation(
76 "state",
77 format!("unknown pull request state {other:?}"),
78 )),
79 }
80 }
81}
82
83/// A request to merge `source_branch` into `target_branch`.
84#[derive(Debug, Clone, PartialEq, Eq)]
85pub struct PullRequest {
86 pub id: PullRequestId,
87 pub repo_id: RepoId,
88 /// Drawn from the repository's one sequence, shared with issues.
89 pub number: IssueNumber,
90 pub title: String,
91 /// Markdown, rendered by the web layer. Stored exactly as written.
92 pub body: String,
93 pub state: PullRequestState,
94 pub author_id: UserId,
95 /// The branch being merged **from**. A name, not a commit: the request follows the
96 /// branch, so a push to it changes what the request proposes.
97 pub source_branch: RefName,
98 /// The branch being merged **into**.
99 pub target_branch: RefName,
100 pub created_at: SystemTime,
101 pub updated_at: SystemTime,
102 /// When it was closed without merging, or `None`. Cleared on reopen, like an
103 /// issue's — nothing reads a closure that was undone.
104 pub closed_at: Option<SystemTime>,
105 /// When it landed. Set once, with [`merge_commit`](Self::merge_commit), and never
106 /// cleared.
107 pub merged_at: Option<SystemTime>,
108 /// What landed on the target: the merge commit, the squash commit, or — for a
109 /// fast-forward — the source's own tip, since that is the commit the target now
110 /// points at. `None` while the request is open or closed.
111 pub merge_commit: Option<ObjectId>,
112}
113
114impl PullRequest {
115 /// Creates a pull request from user-supplied input.
116 ///
117 /// The number is supplied rather than allocated here, for
118 /// [`Issue::new`](super::Issue::new)'s reason: allocation has to be atomic against
119 /// storage, which the domain cannot see.
120 ///
121 /// The two branches are already-validated [`RefName`]s; what this adds is that they
122 /// must differ. A branch merged into itself is not a mistake worth a page — it is a
123 /// request that can never do anything.
124 #[allow(clippy::too_many_arguments)]
125 pub fn new(
126 id: PullRequestId,
127 repo_id: RepoId,
128 number: IssueNumber,
129 title: impl Into<String>,
130 body: impl Into<String>,
131 author_id: UserId,
132 source_branch: RefName,
133 target_branch: RefName,
134 now: SystemTime,
135 ) -> Result<Self, DomainError> {
136 let (title, body) = validate_content(title, body)?;
137
138 if source_branch == target_branch {
139 return Err(DomainError::validation(
140 "source branch",
141 "a branch cannot be merged into itself",
142 ));
143 }
144
145 Ok(Self {
146 id,
147 repo_id,
148 number,
149 title,
150 body,
151 state: PullRequestState::Open,
152 author_id,
153 source_branch,
154 target_branch,
155 created_at: now,
156 updated_at: now,
157 closed_at: None,
158 merged_at: None,
159 merge_commit: None,
160 })
161 }
162
163 /// Rebuilds a pull request from storage without revalidating it.
164 #[allow(clippy::too_many_arguments)]
165 pub fn from_trusted(
166 id: PullRequestId,
167 repo_id: RepoId,
168 number: IssueNumber,
169 title: impl Into<String>,
170 body: impl Into<String>,
171 state: PullRequestState,
172 author_id: UserId,
173 source_branch: RefName,
174 target_branch: RefName,
175 created_at: SystemTime,
176 updated_at: SystemTime,
177 closed_at: Option<SystemTime>,
178 merged_at: Option<SystemTime>,
179 merge_commit: Option<ObjectId>,
180 ) -> Self {
181 Self {
182 id,
183 repo_id,
184 number,
185 title: title.into(),
186 body: body.into(),
187 state,
188 author_id,
189 source_branch,
190 target_branch,
191 created_at,
192 updated_at,
193 closed_at,
194 merged_at,
195 merge_commit,
196 }
197 }
198
199 /// Replaces the title and body, validating them as a fresh submission would be.
200 ///
201 /// The branches are not editable. Changing either would make every comment on the
202 /// request an opinion about a different change.
203 pub fn edit(
204 &mut self,
205 title: impl Into<String>,
206 body: impl Into<String>,
207 now: SystemTime,
208 ) -> Result<(), DomainError> {
209 let (title, body) = validate_content(title, body)?;
210
211 self.title = title;
212 self.body = body;
213 self.updated_at = now;
214
215 Ok(())
216 }
217
218 /// Closes without merging, or does nothing if it is not open.
219 ///
220 /// Idempotent for the issue's reason, and a **merged request is left alone**: it is
221 /// already finished, and "closed" would be a worse description of what happened.
222 pub fn close(&mut self, now: SystemTime) {
223 if self.state != PullRequestState::Open {
224 return;
225 }
226
227 self.state = PullRequestState::Closed;
228 self.closed_at = Some(now);
229 self.updated_at = now;
230 }
231
232 /// Reopens a closed request, or does nothing if it is open or merged.
233 pub fn reopen(&mut self, now: SystemTime) {
234 if self.state != PullRequestState::Closed {
235 return;
236 }
237
238 self.state = PullRequestState::Open;
239 self.closed_at = None;
240 self.updated_at = now;
241 }
242
243 /// Records that it landed, naming the commit the target now points at.
244 ///
245 /// Refuses anything but an open request rather than being idempotent, which is the
246 /// one place this differs from [`close`](Self::close): closing twice is two clicks
247 /// on one button, whereas merging twice would be two commits on the target and the
248 /// second one silently forgotten. The caller has already written to the repository
249 /// by the time this is reached, so the refusal is the guard that stops a double
250 /// submit turning into two merges.
251 pub fn merge(&mut self, commit: ObjectId, now: SystemTime) -> Result<(), DomainError> {
252 if self.state != PullRequestState::Open {
253 return Err(DomainError::validation(
254 "state",
255 format!("a {} pull request cannot be merged", self.state),
256 ));
257 }
258
259 self.state = PullRequestState::Merged;
260 self.merge_commit = Some(commit);
261 self.merged_at = Some(now);
262 self.updated_at = now;
263
264 Ok(())
265 }
266
267 /// Marks the request as having been touched, without changing what it says.
268 pub fn touch(&mut self, now: SystemTime) {
269 self.updated_at = now;
270 }
271}
272
273/// A comment on a pull request.
274///
275/// Part of the [`PullRequest`] aggregate, exactly as an
276/// [`IssueComment`](super::IssueComment) is part of its issue.
277#[derive(Debug, Clone, PartialEq, Eq)]
278pub struct PullRequestComment {
279 pub id: PullRequestCommentId,
280 pub pull_request_id: PullRequestId,
281 pub author_id: UserId,
282 /// Markdown. Never empty: an empty comment is a misclick.
283 pub body: String,
284 pub created_at: SystemTime,
285}
286
287impl PullRequestComment {
288 pub fn new(
289 id: PullRequestCommentId,
290 pull_request_id: PullRequestId,
291 author_id: UserId,
292 body: impl Into<String>,
293 now: SystemTime,
294 ) -> Result<Self, DomainError> {
295 let body = body.into();
296 let body = body.trim();
297
298 if body.is_empty() {
299 return Err(DomainError::validation("comment", "must not be empty"));
300 }
301
302 if body.chars().count() > super::issue::MAX_BODY_LEN {
303 return Err(DomainError::validation(
304 "comment",
305 format!("must be at most {} characters", super::issue::MAX_BODY_LEN),
306 ));
307 }
308
309 Ok(Self {
310 id,
311 pull_request_id,
312 author_id,
313 body: body.to_owned(),
314 created_at: now,
315 })
316 }
317
318 /// Rebuilds a comment from storage without revalidating it.
319 pub fn from_trusted(
320 id: PullRequestCommentId,
321 pull_request_id: PullRequestId,
322 author_id: UserId,
323 body: impl Into<String>,
324 created_at: SystemTime,
325 ) -> Self {
326 Self {
327 id,
328 pull_request_id,
329 author_id,
330 body: body.into(),
331 created_at,
332 }
333 }
334}
335
336/// How a merge is performed.
337///
338/// Three, and no rebase-merge: rebasing rewrites every commit the author wrote and
339/// hands them back new ids, which is a thing to do on a branch you own rather than a
340/// button on somebody else's page.
341#[derive(Debug, Clone, Copy, Default, PartialEq, Eq)]
342pub enum MergeStrategy {
343 /// A commit with two parents — the target and the source. The default, because it
344 /// is the only one that keeps what actually happened.
345 #[default]
346 Merge,
347 /// Move the target to the source. Only offered when the target is an ancestor of
348 /// the source, where it is exact: no new object at all, just the ref moving.
349 FastForward,
350 /// One commit on the target carrying the whole change and no second parent.
351 Squash,
352}
353
354impl MergeStrategy {
355 pub fn as_str(self) -> &'static str {
356 match self {
357 Self::Merge => "merge",
358 Self::FastForward => "fast-forward",
359 Self::Squash => "squash",
360 }
361 }
362
363 /// What the button and the select call it.
364 pub fn label(self) -> &'static str {
365 match self {
366 Self::Merge => "Create a merge commit",
367 Self::FastForward => "Fast-forward",
368 Self::Squash => "Squash and merge",
369 }
370 }
371}
372
373impl fmt::Display for MergeStrategy {
374 fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
375 f.write_str(self.as_str())
376 }
377}
378
379/// `Result`, never `Option`: a strategy that silently defaulted would land a squash
380/// where somebody asked for a merge commit, and the history cannot be un-landed.
381impl FromStr for MergeStrategy {
382 type Err = DomainError;
383
384 fn from_str(value: &str) -> Result<Self, Self::Err> {
385 match value {
386 "merge" => Ok(Self::Merge),
387 "fast-forward" => Ok(Self::FastForward),
388 "squash" => Ok(Self::Squash),
389 other => Err(DomainError::validation(
390 "strategy",
391 format!("unknown merge strategy {other:?}"),
392 )),
393 }
394 }
395}
396
397/// The words that close an issue when the request carrying them is merged.
398///
399/// GitHub's set, minus the tenses nobody writes. Matched case-insensitively, so `Closes`,
400/// `closes` and `CLOSES` all count — this is prose somebody typed, not a keyword.
401const CLOSING_WORDS: [&str; 3] = ["closes", "fixes", "resolves"];
402
403/// The issues a merged pull request should close, in the order they are written and
404/// without repeats.
405///
406/// **Shape alone, and only this shape**: one of [`CLOSING_WORDS`], a space or two, then
407/// `#` and digits. A bare `#12` in a body is a cross-reference and stays one — turning
408/// every mention into a closure would shut issues somebody was merely pointing at.
409///
410/// Deliberately *not* markdown-aware, for the same reason the cross-reference scanner is
411/// not: this reads the source a person typed, and a closing keyword inside a code fence
412/// is a rare mistake next to the cost of parsing the document twice to find out. What it
413/// does refuse is a keyword that is part of a longer word — `precloses #4` is not a
414/// closure — and a number that is glued to something else, so `#12a` names nothing.
415pub fn closes(text: &str) -> Vec<IssueNumber> {
416 let mut found: Vec<IssueNumber> = Vec::new();
417 let lower = text.to_lowercase();
418
419 for (index, _) in lower.char_indices() {
420 // A keyword must start a word: anything alphanumeric before it means this is
421 // the middle of a longer one.
422 if index > 0
423 && lower[..index]
424 .chars()
425 .next_back()
426 .is_some_and(|previous| previous.is_alphanumeric() || previous == '_')
427 {
428 continue;
429 }
430
431 let rest = &lower[index..];
432
433 let Some(after) = CLOSING_WORDS
434 .iter()
435 .find_map(|word| rest.strip_prefix(word))
436 else {
437 continue;
438 };
439
440 // Whitespace between the word and the reference, and nothing else: `closes:#4`
441 // is not how anyone writes it, and accepting punctuation here would make
442 // `closes.#4` — the end of one sentence and the start of another — a closure.
443 let spaces = after.len() - after.trim_start_matches([' ', '\t']).len();
444
445 if spaces == 0 {
446 continue;
447 }
448
449 let Some(digits) = after[spaces..].strip_prefix('#') else {
450 continue;
451 };
452
453 let end = digits
454 .find(|c: char| !c.is_ascii_digit())
455 .unwrap_or(digits.len());
456
457 if end == 0 {
458 continue;
459 }
460
461 // Glued to a letter, so it names nothing: `#12a` is not issue 12.
462 if digits[end..]
463 .chars()
464 .next()
465 .is_some_and(|next| next.is_alphanumeric() || next == '_')
466 {
467 continue;
468 }
469
470 // `new` rather than `from_trusted`: `#0` is prose.
471 let Ok(number) = digits[..end]
472 .parse::<u64>()
473 .map_err(|_| ())
474 .and_then(|value| IssueNumber::new(value).map_err(|_| ()))
475 else {
476 continue;
477 };
478
479 if !found.contains(&number) {
480 found.push(number);
481 }
482 }
483
484 found
485}
486
487#[cfg(test)]
488mod tests {
489 use std::time::Duration;
490
491 use super::*;
492
493 fn at(seconds: u64) -> SystemTime {
494 SystemTime::UNIX_EPOCH + Duration::from_secs(seconds)
495 }
496
497 fn pull(source: &str, target: &str) -> Result<PullRequest, DomainError> {
498 PullRequest::new(
499 PullRequestId::generate(),
500 RepoId::generate(),
501 IssueNumber::from_trusted(7),
502 "Speed blame up",
503 "It walks history per line.",
504 UserId::generate(),
505 RefName::from_trusted(source),
506 RefName::from_trusted(target),
507 at(1_000),
508 )
509 }
510
511 fn oid() -> ObjectId {
512 ObjectId::from_trusted("a".repeat(40))
513 }
514
515 // --- state ----------------------------------------------------------------------
516
517 #[test]
518 fn state_round_trips_through_strings() {
519 for state in [
520 PullRequestState::Open,
521 PullRequestState::Merged,
522 PullRequestState::Closed,
523 ] {
524 assert_eq!(
525 state
526 .as_str()
527 .parse::<PullRequestState>()
528 .expect("round trip"),
529 state
530 );
531 }
532 }
533
534 #[test]
535 fn an_unknown_state_is_an_error_not_a_default() {
536 assert!("Open".parse::<PullRequestState>().is_err());
537 assert!("draft".parse::<PullRequestState>().is_err());
538 }
539
540 // --- creating -------------------------------------------------------------------
541
542 #[test]
543 fn a_new_request_is_open_and_unmerged() {
544 let pull = pull("blame-speed", "main").expect("valid");
545
546 assert_eq!(pull.state, PullRequestState::Open);
547 assert_eq!(pull.merge_commit, None);
548 assert_eq!(pull.merged_at, None);
549 assert_eq!(pull.closed_at, None);
550 assert_eq!(pull.created_at, at(1_000));
551 }
552
553 #[test]
554 fn a_branch_cannot_be_merged_into_itself() {
555 assert!(pull("main", "main").is_err());
556 }
557
558 #[test]
559 fn a_request_is_held_to_the_issue_title_rules() {
560 // The same function, so a cap cannot move for one and not the other.
561 assert!(
562 PullRequest::new(
563 PullRequestId::generate(),
564 RepoId::generate(),
565 IssueNumber::from_trusted(1),
566 " ",
567 "",
568 UserId::generate(),
569 RefName::from_trusted("spike"),
570 RefName::from_trusted("main"),
571 at(1_000),
572 )
573 .is_err()
574 );
575 }
576
577 // --- closing, reopening, merging --------------------------------------------------
578
579 #[test]
580 fn closing_stamps_the_time_and_reopening_clears_it() {
581 let mut pull = pull("spike", "main").expect("valid");
582
583 pull.close(at(2_000));
584 assert_eq!(pull.state, PullRequestState::Closed);
585 assert_eq!(pull.closed_at, Some(at(2_000)));
586
587 pull.reopen(at(3_000));
588 assert_eq!(pull.state, PullRequestState::Open);
589 assert_eq!(pull.closed_at, None);
590 assert_eq!(pull.updated_at, at(3_000));
591 }
592
593 #[test]
594 fn merging_records_what_landed_and_when() {
595 let mut pull = pull("spike", "main").expect("valid");
596
597 pull.merge(oid(), at(4_000)).expect("mergeable");
598
599 assert_eq!(pull.state, PullRequestState::Merged);
600 assert_eq!(pull.merge_commit, Some(oid()));
601 assert_eq!(pull.merged_at, Some(at(4_000)));
602 assert_eq!(pull.updated_at, at(4_000));
603 }
604
605 #[test]
606 fn merging_twice_is_refused_rather_than_ignored() {
607 // The guard against a double submit landing two commits on the target.
608 let mut pull = pull("spike", "main").expect("valid");
609
610 pull.merge(oid(), at(4_000)).expect("mergeable");
611
612 assert!(pull.merge(oid(), at(5_000)).is_err());
613 assert_eq!(pull.merged_at, Some(at(4_000)));
614 }
615
616 #[test]
617 fn a_merged_request_is_terminal() {
618 let mut pull = pull("spike", "main").expect("valid");
619 pull.merge(oid(), at(4_000)).expect("mergeable");
620
621 pull.close(at(5_000));
622 pull.reopen(at(6_000));
623
624 assert_eq!(pull.state, PullRequestState::Merged);
625 assert_eq!(pull.updated_at, at(4_000), "neither call touched it");
626 }
627
628 #[test]
629 fn a_closed_request_cannot_be_merged_without_being_reopened() {
630 let mut pull = pull("spike", "main").expect("valid");
631 pull.close(at(2_000));
632
633 assert!(pull.merge(oid(), at(3_000)).is_err());
634 }
635
636 // --- strategies -------------------------------------------------------------------
637
638 #[test]
639 fn a_strategy_round_trips_and_never_defaults() {
640 for strategy in [
641 MergeStrategy::Merge,
642 MergeStrategy::FastForward,
643 MergeStrategy::Squash,
644 ] {
645 assert_eq!(
646 strategy
647 .as_str()
648 .parse::<MergeStrategy>()
649 .expect("round trip"),
650 strategy
651 );
652 }
653
654 assert!("rebase".parse::<MergeStrategy>().is_err());
655 assert!("".parse::<MergeStrategy>().is_err());
656 }
657
658 // --- comments -----------------------------------------------------------------------
659
660 #[test]
661 fn a_comment_needs_a_body() {
662 let comment = |body: &str| {
663 PullRequestComment::new(
664 PullRequestCommentId::generate(),
665 PullRequestId::generate(),
666 UserId::generate(),
667 body,
668 at(1_000),
669 )
670 };
671
672 assert!(comment("").is_err());
673 assert!(comment(" \n ").is_err());
674 assert_eq!(comment(" ship it ").expect("valid").body, "ship it");
675 }
676
677 // --- closing references ------------------------------------------------------------
678
679 fn closed(text: &str) -> Vec<u64> {
680 closes(text).into_iter().map(IssueNumber::get).collect()
681 }
682
683 #[test]
684 fn each_closing_word_counts_whatever_its_case() {
685 assert_eq!(closed("Closes #4"), [4]);
686 assert_eq!(closed("fixes #4"), [4]);
687 assert_eq!(closed("RESOLVES #4"), [4]);
688 }
689
690 #[test]
691 fn a_bare_reference_is_a_mention_and_not_a_closure() {
692 // Turning every mention into a closure would shut issues somebody was pointing
693 // at for context.
694 assert!(closed("See #4 for the background").is_empty());
695 assert!(closed("#4").is_empty());
696 }
697
698 #[test]
699 fn several_are_found_in_order_and_repeats_are_one() {
700 assert_eq!(
701 closed("Closes #4 and fixes #9.\n\nAlso closes #4 again."),
702 [4, 9]
703 );
704 }
705
706 #[test]
707 fn a_keyword_inside_a_longer_word_is_not_one() {
708 assert!(closed("precloses #4").is_empty());
709 assert!(closed("unfixes #4").is_empty());
710 // ...but ordinary punctuation before it is fine.
711 assert_eq!(closed("(closes #4)"), [4]);
712 assert_eq!(closed("- closes #4"), [4]);
713 }
714
715 #[test]
716 fn the_reference_must_follow_the_word_with_only_spaces_between() {
717 assert_eq!(closed("closes #4"), [4]);
718 assert_eq!(closed("closes\t#4"), [4]);
719 // A full stop between them is two sentences, not a closure.
720 assert!(closed("closes. #4").is_empty());
721 assert!(closed("closes:#4").is_empty());
722 // A newline is a new line of prose; the reference has to be beside the word.
723 assert!(closed("closes\n#4").is_empty());
724 }
725
726 #[test]
727 fn a_number_that_names_nothing_is_not_a_closure() {
728 assert!(closed("closes #0").is_empty());
729 assert!(closed("closes #12a").is_empty());
730 assert!(closed("closes #").is_empty());
731 assert!(closed("closes issue 4").is_empty());
732 }
733}