From 109ca497d46b0f8a8563bb029799d99e285b065e Mon Sep 17 00:00:00 2001 From: Han Xu Date: Wed, 18 Jan 2023 16:42:06 -0800 Subject: [PATCH] surf: improvements to Blob Improve on the previous iteration of `Blob`. `BlobVec` is simply a `Vec` underneath, so it is technically not needed and can be replaced by using `Vec` directly. Notice that BlobRef can implement AsRef and Deref for `[u8]`. This leads to being able to use `T: AsRef<[u8]>` for `Blob` methods and the `Serialize` implementation. Apply these two facts and remove any of the remaining code that is no longer needed. Signed-off-by: Fintan Halpenny --- radicle-surf/src/blob.rs | 144 ++++++++++++----------------------- radicle-surf/t/src/source.rs | 9 +-- 2 files changed, 50 insertions(+), 103 deletions(-) diff --git a/radicle-surf/src/blob.rs b/radicle-surf/src/blob.rs index 1bdd2ed..2a9ba83 100644 --- a/radicle-surf/src/blob.rs +++ b/radicle-surf/src/blob.rs @@ -18,6 +18,8 @@ //! Represents git object type 'blob', i.e. actual file contents. //! See git [doc](https://git-scm.com/book/en/v2/Git-Internals-Git-Objects) for more details. +use std::ops::Deref; + use radicle_git_ext::Oid; #[cfg(feature = "serde")] use serde::{ @@ -29,7 +31,8 @@ use crate::Commit; /// Represents a git blob object. /// -/// The type parameter `T` could be [`BlobRef`] or [`BlobVec`]. +/// The type parameter `T` can be fulfilled by [`BlobRef`] or a +/// [`Vec`] of bytes. pub struct Blob { id: Oid, is_binary: bool, @@ -51,13 +54,23 @@ impl Blob { &self.commit } - pub fn content(&self) -> &T { - &self.content + pub fn content(&self) -> &[u8] + where + T: AsRef<[u8]>, + { + self.content.as_ref() + } + + pub fn size(&self) -> usize + where + T: AsRef<[u8]>, + { + self.content.as_ref().len() } } impl<'a> Blob> { - /// Returns the [`Blob`] wrapping around an underlying `git2::Blob`. + /// Returns the [`Blob`] wrapping around an underlying [`git2::Blob`]. pub(crate) fn new(id: Oid, git2_blob: git2::Blob<'a>, commit: Commit) -> Self { let is_binary = git2_blob.is_binary(); let content = BlobRef { inner: git2_blob }; @@ -70,28 +83,13 @@ impl<'a> Blob> { } /// Converts into a `Blob` with owned content bytes. - pub fn to_owned(&self) -> Blob { - Blob::::new( - self.id, - self.content.as_bytes().to_vec(), - self.commit.clone(), - self.is_binary, - ) - } -} - -#[cfg(feature = "serde")] -impl<'a> Serialize for Blob> { - fn serialize(&self, serializer: S) -> Result - where - S: Serializer, - { - const FIELDS: usize = 5; - let mut state = serializer.serialize_struct("Blob", FIELDS)?; - state.serialize_field("binary", &self.is_binary())?; - state.serialize_field("content", &self.content)?; - state.serialize_field("lastCommit", &self.commit)?; - state.end() + pub fn to_owned(&self) -> Blob> { + Blob { + id: self.id, + content: self.content.to_vec(), + commit: self.commit.clone(), + is_binary: self.is_binary, + } } } @@ -100,91 +98,43 @@ pub struct BlobRef<'a> { inner: git2::Blob<'a>, } -impl<'a> BlobRef<'a> { - /// Returns the size of the blob content. - pub fn size(&self) -> usize { - self.inner.size() +impl AsRef<[u8]> for BlobRef<'_> { + fn as_ref(&self) -> &[u8] { + self.inner.content() } +} - /// Returns the content as bytes. - pub fn as_bytes(&self) -> &[u8] { +impl Deref for BlobRef<'_> { + type Target = [u8]; + + fn deref(&self) -> &Self::Target { self.inner.content() } } #[cfg(feature = "serde")] -impl<'a> Serialize for BlobRef<'a> { +impl Serialize for Blob +where + T: AsRef<[u8]>, +{ fn serialize(&self, serializer: S) -> Result where S: Serializer, { - serialize_bytes(self.as_bytes(), serializer) - } -} - -/// Represents a blob with owned content bytes. -pub struct BlobVec { - inner: Vec, -} - -impl BlobVec { - pub fn size(&self) -> usize { - self.inner.len() - } - - pub fn as_bytes(&self) -> &[u8] { - self.inner.as_ref() - } -} - -impl Blob { - pub(crate) fn new(id: Oid, bytes: Vec, commit: Commit, is_binary: bool) -> Self { - let content = BlobVec { inner: bytes }; - Self { - id, - is_binary, - content, - commit, - } - } -} - -#[cfg(feature = "serde")] -impl Serialize for BlobVec { - fn serialize(&self, serializer: S) -> Result - where - S: Serializer, - { - serialize_bytes(self.as_bytes(), serializer) - } -} - -#[cfg(feature = "serde")] -impl Serialize for Blob { - fn serialize(&self, serializer: S) -> Result - where - S: Serializer, - { - const FIELDS: usize = 5; + const FIELDS: usize = 4; let mut state = serializer.serialize_struct("Blob", FIELDS)?; + state.serialize_field("id", &self.id)?; state.serialize_field("binary", &self.is_binary())?; - state.serialize_field("content", &self.content)?; + + let bytes = self.content.as_ref(); + match std::str::from_utf8(bytes) { + Ok(s) => state.serialize_field("content", s)?, + Err(_) => { + let encoded = base64::encode(bytes); + state.serialize_field("content", &encoded)? + }, + }; state.serialize_field("lastCommit", &self.commit)?; state.end() } } - -/// Common serialization for a `Blob`'s content bytes. -#[cfg(feature = "serde")] -fn serialize_bytes(bytes: &[u8], serializer: S) -> Result -where - S: Serializer, -{ - match std::str::from_utf8(bytes) { - Ok(s) => serializer.serialize_str(s), - Err(_) => { - let encoded = base64::encode(bytes); - serializer.serialize_str(&encoded) - }, - } -} diff --git a/radicle-surf/t/src/source.rs b/radicle-surf/t/src/source.rs index e634c73..7ebe888 100644 --- a/radicle-surf/t/src/source.rs +++ b/radicle-surf/t/src/source.rs @@ -159,15 +159,12 @@ fn repo_blob() { // Verify the blob content size matches with the file size of "memory.rs" let content = blob.content(); - assert_eq!(content.size(), 6253); - - // Verify as_bytes. - assert_eq!(content.as_bytes().len(), content.size()); + assert_eq!(blob.size(), 6253); // Verify to_owned(). let blob_owned = blob.to_owned(); - assert_eq!(blob_owned.content().size(), 6253); - assert_eq!(blob_owned.content().as_bytes(), content.as_bytes()); + assert_eq!(blob_owned.size(), 6253); + assert_eq!(blob.content(), blob_owned.content()); // Verify JSON output is the same. let json_ref = json!({ "content": content }).to_string();