From d957f0b1c27bd881e552e270f54cddf7eff8b44d Mon Sep 17 00:00:00 2001 From: Fintan Halpenny Date: Wed, 14 Dec 2022 10:35:57 +0000 Subject: [PATCH 1/5] surf: fix path serialization for tree and blob The http-api code expects the 'path' serialized field to be the full path for trees, tree entries, and blobs, i.e. the path where it is located concatenated with the name of the entry. Signed-off-by: Fintan Halpenny X-Clacks-Overhead: GNU Terry Pratchett --- radicle-surf/src/source/commit.rs | 2 +- radicle-surf/src/source/object/blob.rs | 2 +- radicle-surf/src/source/object/tree.rs | 7 ++++--- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/radicle-surf/src/source/commit.rs b/radicle-surf/src/source/commit.rs index 927718a..048a70b 100644 --- a/radicle-surf/src/source/commit.rs +++ b/radicle-surf/src/source/commit.rs @@ -47,7 +47,7 @@ pub struct Commit { } /// Representation of a code commit. -#[derive(Clone)] +#[derive(Clone, Debug)] pub struct Header { /// Identifier of the commit in the form of a sha1 hash. Often referred to /// as oid or object id. diff --git a/radicle-surf/src/source/object/blob.rs b/radicle-surf/src/source/object/blob.rs index c3f95ad..281ef93 100644 --- a/radicle-surf/src/source/object/blob.rs +++ b/radicle-surf/src/source/object/blob.rs @@ -103,7 +103,7 @@ impl Serialize for Blob { state.serialize_field("content", &self.content)?; state.serialize_field("lastCommit", &self.commit)?; state.serialize_field("name", &self.file.name())?; - state.serialize_field("path", &self.file.location())?; + state.serialize_field("path", &self.file.path())?; state.end() } } diff --git a/radicle-surf/src/source/object/tree.rs b/radicle-surf/src/source/object/tree.rs index ef70379..40e5a4b 100644 --- a/radicle-surf/src/source/object/tree.rs +++ b/radicle-surf/src/source/object/tree.rs @@ -33,6 +33,7 @@ use crate::{ }; /// Result of a directory listing, carries other trees and blobs. +#[derive(Clone, Debug)] pub struct Tree { pub directory: Directory, pub commit: Option, @@ -95,7 +96,7 @@ impl Serialize for Tree { state.serialize_field("entries", &self.entries)?; state.serialize_field("lastCommit", &self.commit)?; state.serialize_field("name", &self.directory.name())?; - state.serialize_field("path", &self.directory.location())?; + state.serialize_field("path", &self.directory.path())?; state.end() } } @@ -126,14 +127,14 @@ impl Serialize for TreeEntry { { const FIELDS: usize = 4; let mut state = serializer.serialize_struct("TreeEntry", FIELDS)?; - state.serialize_field("path", &self.entry.location())?; + state.serialize_field("path", &self.entry.path())?; state.serialize_field("name", &self.entry.name())?; state.serialize_field("lastCommit", &None::)?; state.serialize_field( "kind", match self.entry { directory::Entry::File(_) => "blob", - directory::Entry::Directory(_) => "directory", + directory::Entry::Directory(_) => "tree", }, )?; state.end() From 95c31f2a8e9ec0eeb2b2ef8dac6d0dd8d216d571 Mon Sep 17 00:00:00 2001 From: Fintan Halpenny Date: Wed, 14 Dec 2022 13:37:34 +0000 Subject: [PATCH 2/5] surf: fix directory construction If a git Tree is a subtree then it no longer contains the path information while traversing it. The traversal will only carry the path with it if it does not skip directories, which in the case of `entries`, it does. To remedy this, the `path` method is used to pass the information to `from_entry` instead. Signed-off-by: Fintan Halpenny X-Clacks-Overhead: GNU Terry Pratchett --- radicle-surf/src/file_system/directory.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/radicle-surf/src/file_system/directory.rs b/radicle-surf/src/file_system/directory.rs index 41eea0b..2fb8278 100644 --- a/radicle-surf/src/file_system/directory.rs +++ b/radicle-surf/src/file_system/directory.rs @@ -352,10 +352,11 @@ impl Directory { let mut entries = BTreeMap::new(); let mut error = None; + let path = self.path(); // Walks only the first level of entries. - tree.walk(git2::TreeWalkMode::PreOrder, |path, entry| { - match Entry::from_entry(entry, Path::new(path).to_path_buf()) { + tree.walk(git2::TreeWalkMode::PreOrder, |_, entry| { + match Entry::from_entry(entry, path.clone()) { Ok(Some(entry)) => match entry { Entry::File(_) => { entries.insert(entry.name().clone(), entry); From 0d5b6c4c30dfebf343b5867ddb0907b8dfc62e2e Mon Sep 17 00:00:00 2001 From: Fintan Halpenny Date: Wed, 14 Dec 2022 13:40:53 +0000 Subject: [PATCH 3/5] surf: test serialization of tree Confirm that the serialization of `source::Tree` has the expected output. Signed-off-by: Fintan Halpenny X-Clacks-Overhead: GNU Terry Pratchett --- radicle-surf/t/src/lib.rs | 3 +++ radicle-surf/t/src/source.rs | 39 ++++++++++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+) create mode 100644 radicle-surf/t/src/source.rs diff --git a/radicle-surf/t/src/lib.rs b/radicle-surf/t/src/lib.rs index fc9023d..c46d568 100644 --- a/radicle-surf/t/src/lib.rs +++ b/radicle-surf/t/src/lib.rs @@ -6,3 +6,6 @@ mod git; #[cfg(test)] mod file_system; + +#[cfg(test)] +mod source; diff --git a/radicle-surf/t/src/source.rs b/radicle-surf/t/src/source.rs new file mode 100644 index 0000000..5f098ee --- /dev/null +++ b/radicle-surf/t/src/source.rs @@ -0,0 +1,39 @@ +use std::path::Path; + +use git_ref_format::refname; +use radicle_surf::{git::Repository, source}; +use serde_json::json; + +const GIT_PLATINUM: &str = "../data/git-platinum"; + +#[test] +fn tree_serialization() { + let repo = Repository::open(GIT_PLATINUM).unwrap(); + let tree = source::Tree::new( + &repo, + &refname!("refs/heads/master"), + Some(&Path::new("src")), + ) + .unwrap(); + + let expected = json!({ + "entries": [ + { + "kind": "blob", + "lastCommit": null, + "name": "Eval.hs", + "path": "src/Eval.hs" + }, + { + "kind": "blob", + "lastCommit": null, + "name": "memory.rs", + "path": "src/memory.rs" + } + ], + "lastCommit": null, + "name": "src", + "path": "src" + }); + assert_eq!(serde_json::to_value(tree).unwrap(), expected) +} From 21c0e68ab5e11089ba943a4fd267b5de7d3e9158 Mon Sep 17 00:00:00 2001 From: Fintan Halpenny Date: Thu, 15 Dec 2022 17:44:09 +0000 Subject: [PATCH 4/5] surf: testing an added file Test the serialization of an added file in a Diff. Signed-off-by: Fintan Halpenny X-Clacks-Overhead: GNU Terry Pratchett --- radicle-surf/t/src/git/diff.rs | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/radicle-surf/t/src/git/diff.rs b/radicle-surf/t/src/git/diff.rs index 9098f06..8de7555 100644 --- a/radicle-surf/t/src/git/diff.rs +++ b/radicle-surf/t/src/git/diff.rs @@ -9,7 +9,6 @@ use radicle_surf::{ EofNewLine, FileDiff, Hunk, - Hunks, Line, Modification, Modified, @@ -152,7 +151,14 @@ fn test_diff_serde() { let diff = Diff { added: vec![ Added { path: Path::new("LICENSE").to_path_buf(), - diff: FileDiff::Plain { hunks: Hunks::default() } + diff: FileDiff::Plain { + hunks: vec![Hunk { + header: Line::from(b"@@ -0,0 +1,1".to_vec()), + lines: vec![ + Addition { line: Line::from(b"MIT".to_vec()), line_no: 1 } + ] + }].into() + } }], deleted: vec![], moved: vec![ Moved { @@ -184,9 +190,17 @@ fn test_diff_serde() { let eof: Option = None; let json = serde_json::json!({ - "added": [{"path": "LICENSE", "diff": { + "added": [{ + "path": "LICENSE", + "diff": { "type": "plain", - "hunks": [] + "hunks": [{ + "header": "@@ -0,0 +1,1", + "lines": [{ + "line": "MIT", + "lineNo": 1, + }] + }] }, }], "deleted": [], From f4d2e386203e296921e457092707708b84d753fa Mon Sep 17 00:00:00 2001 From: Fintan Halpenny Date: Sun, 18 Dec 2022 15:04:15 +0000 Subject: [PATCH 5/5] radicle-surf: serialize type for any kind of line To help frontends with figuring out what kind of line is being talked about for added and deleted files, the serialization of `Addition` and `Deletion` always include a type. This mirrors the serialization of the `Modification` type, which is tagged because it's an enum. The serialization is hand-rolled since it is desirable to make sure that added files and deleted files are only ever composed of lines that are not modified. Signed-off-by: Fintan Halpenny X-Clacks-Overhead: GNU Terry Pratchett --- radicle-surf/src/diff.rs | 86 ++++++++++++++++++++++++++++++---- radicle-surf/t/src/git/diff.rs | 30 +++++++++++- 2 files changed, 104 insertions(+), 12 deletions(-) diff --git a/radicle-surf/src/diff.rs b/radicle-surf/src/diff.rs index 10ad9f9..802abc6 100644 --- a/radicle-surf/src/diff.rs +++ b/radicle-surf/src/diff.rs @@ -238,23 +238,15 @@ impl Serialize for Line { /// Either the modification of a single [`Line`], or just contextual /// information. -#[cfg_attr( - feature = "serde", - derive(Serialize), - serde(tag = "type", rename_all = "camelCase") -)] #[derive(Clone, Debug, PartialEq, Eq)] pub enum Modification { /// A lines is an addition in a file. - #[cfg_attr(feature = "serde", serde(rename_all = "camelCase"))] Addition(Addition), /// A lines is a deletion in a file. - #[cfg_attr(feature = "serde", serde(rename_all = "camelCase"))] Deletion(Deletion), /// A contextual line in a file, i.e. there were no changes to the line. - #[cfg_attr(feature = "serde", serde(rename_all = "camelCase"))] Context { line: Line, line_no_old: u32, @@ -262,22 +254,96 @@ pub enum Modification { }, } +#[cfg(feature = "serde")] +impl Serialize for Modification { + fn serialize(&self, serializer: S) -> Result + where + S: Serializer, + { + use serde::ser::SerializeMap as _; + + const NAME: &str = "Modification"; + const ADDITION: &str = "Addition"; + const DELETION: &str = "Deletion"; + const CONTEXT: &str = "Context"; + + match self { + Modification::Addition(addition) => { + let mut map = serializer.serialize_map(Some(3))?; + map.serialize_entry("line", &addition.line)?; + map.serialize_entry("lineNo", &addition.line_no)?; + map.serialize_entry("type", "addition")?; + map.end() + }, + Modification::Deletion(deletion) => { + let mut map = serializer.serialize_map(Some(3))?; + map.serialize_entry("line", &deletion.line)?; + map.serialize_entry("lineNo", &deletion.line_no)?; + map.serialize_entry("type", "deletion")?; + map.end() + }, + Modification::Context { + line, + line_no_old, + line_no_new, + } => { + let mut map = serializer.serialize_map(Some(4))?; + map.serialize_entry("line", line)?; + map.serialize_entry("lineNoOld", line_no_old)?; + map.serialize_entry("lineNoNew", line_no_new)?; + map.serialize_entry("type", "context")?; + map.end() + }, + } + } +} + /// A addition of a [`Line`] at the `line_no`. -#[cfg_attr(feature = "serde", derive(Serialize), serde(rename_all = "camelCase"))] #[derive(Clone, Debug, PartialEq, Eq)] pub struct Addition { pub line: Line, pub line_no: u32, } +#[cfg(feature = "serde")] +impl Serialize for Addition { + fn serialize(&self, serializer: S) -> Result + where + S: Serializer, + { + use serde::ser::SerializeStruct as _; + + let mut s = serializer.serialize_struct("Addition", 3)?; + s.serialize_field("line", &self.line)?; + s.serialize_field("lineNo", &self.line_no)?; + s.serialize_field("type", "addition")?; + s.end() + } +} + /// A deletion of a [`Line`] at the `line_no`. -#[cfg_attr(feature = "serde", derive(Serialize), serde(rename_all = "camelCase"))] #[derive(Clone, Debug, PartialEq, Eq)] pub struct Deletion { pub line: Line, pub line_no: u32, } +#[cfg(feature = "serde")] +impl Serialize for Deletion { + fn serialize(&self, serializer: S) -> Result + where + S: Serializer, + { + use serde::ser::SerializeStruct as _; + + let mut s = serializer.serialize_struct("Deletion", 3)?; + s.serialize_field("line", &self.line)?; + s.serialize_field("lineNo", &self.line_no)?; + s.serialize_field("type", "deletion")?; + s.end() + } +} + impl Modification { pub fn addition(line: impl Into, line_no: u32) -> Self { Self::Addition(Addition { diff --git a/radicle-surf/t/src/git/diff.rs b/radicle-surf/t/src/git/diff.rs index 8de7555..4efe938 100644 --- a/radicle-surf/t/src/git/diff.rs +++ b/radicle-surf/t/src/git/diff.rs @@ -5,6 +5,8 @@ use radicle_surf::{ diff::{ Added, Addition, + Deleted, + Deletion, Diff, EofNewLine, FileDiff, @@ -160,7 +162,17 @@ fn test_diff_serde() { }].into() } }], - deleted: vec![], + deleted: vec![ Deleted { + path: Path::new("DCO").to_path_buf(), + diff: FileDiff::Plain { + hunks: vec![Hunk { + header: Line::from(b"@@ -0,0 +1,1".to_vec()), + lines: vec![ + Deletion { line: Line::from(b"TODO".to_vec()), line_no: 1 } + ] + }].into() + } + }], moved: vec![ Moved { old_path: Path::new("CONTRIBUTING").to_path_buf(), new_path: Path::new("CONTRIBUTING.md").to_path_buf(), @@ -199,11 +211,25 @@ fn test_diff_serde() { "lines": [{ "line": "MIT", "lineNo": 1, + "type": "addition", + }] + }] + }, + }], + "deleted": [{ + "path": "DCO", + "diff": { + "type": "plain", + "hunks": [{ + "header": "@@ -0,0 +1,1", + "lines": [{ + "line": "TODO", + "lineNo": 1, + "type": "deletion", }] }] }, }], - "deleted": [], "moved": [{ "oldPath": "CONTRIBUTING", "newPath": "CONTRIBUTING.md" }], "copied": [], "modified": [{