ISSUE: 42 and 30, implement headers to catalog v1 and get MimeType

[rustchain64]

Headers include metadata formatted as CBOR data, and
Endpoints provided with a JSON decoded format.

Closes #42 #30

[cryptoquick]

This is a good start, but there's definitely a lot of room for improvement.

[rustchain64]

Thanks for the encouragement and input

…

On Mon, Sep 4, 2023 at 19:23 Hunter Beast ***@***.***> wrote: ***@***.**** requested changes on this pull request. This is a good start, but there's definitely a lot of room for improvement. ------------------------------ In Cargo.toml <#52 (comment)> : > @@ -13,22 +13,34 @@ rayon = ["blake3/rayon"] [dependencies] anyhow = "1.0.69" -axum = "0.6.9" +#axum = "0.6.9" +axum = { version = "0.6.9", features = ["headers"] } Why was this added? I'm not sure this needs to be added. ------------------------------ In Cargo.toml <#52 (comment)> : > axum-macros = "0.3.4" bao = "0.12.1" blake3 = "1.3.3" bytes = "1.4.0" -carbonado = "0.3.0-rc.8" + +#carbonado = "0.3.0-rc.8" + +carbonado = { path = "/Users/wesleyparr/Projects/carbonado/carbonado" } Not good ------------------------------ In Cargo.toml <#52 (comment)> : > chrono = "0.4.23" clap = { version = "4.1.8", features = ["derive"] } directories = "5.0.1" flexi_logger = { version = "0.25.2", features = ["colors"] } flexi_syslog = "0.5.2" +futures = "0.3.28" We shouldn't need this except for very specific cases, try to find a way to work around it ------------------------------ In Cargo.toml <#52 (comment)> : > futures-util = "0.3.28" hex = "0.4.3" +hyper = "0.14" This is nonsense, do not add this, we already have axum ------------------------------ In Cargo.toml <#52 (comment)> : > # human_bytes = "0.4.1" # infer = "0.13.0" +infer = { version = "0.15.0", default-features = false } Why did you turn off default features? The ones provided seem pretty useful. ------------------------------ In Cargo.toml <#52 (comment)> : > # human_bytes = "0.4.1" # infer = "0.13.0" +infer = { version = "0.15.0", default-features = false } +lazy_static = "1.4.0" We actually use once_cell, we don't use lazy_static. I'm also skeptical this is needed to implement this feature. ------------------------------ In src/backend/fs.rs <#52 (comment)> : > @@ -68,6 +88,8 @@ pub async fn write_file<'a>( .try_collect() .await?; + let mime_type_value = shared_mime_type.lock().await; This is a terrible approach. Please refactor to not use locks and instead just get the mimetype before segmentation. This might require doing that in the http server stream loop. ------------------------------ In src/frontend/http.rs <#52 (comment)> : > + + let convert = &bytes as &[u8]; + trace!("Show me the convert: {:?}", convert); Remove this ------------------------------ In src/frontend/http.rs <#52 (comment)> : > @@ -81,6 +86,7 @@ async fn post_file_named( } let hash = write_file_handler(&pk, body, Some(name)).await?; + //let buf = [0xFF, 0xD8, 0xFF, 0xAA]; Clean this up please ------------------------------ In src/frontend/http.rs <#52 (comment)> : > @@ -57,6 +61,7 @@ async fn write_file_handler(pk: &str, body: BodyStream, name: Option<String>) -> #[axum_macros::debug_handler] async fn post_file( Path(pk): Path<String>, + Clean this up, please ------------------------------ In src/header.rs <#52 (comment)> : > + pub fn len() -> usize { + 12 + 33 + 32 + 64 + 1 + 1 + 4 + 4 + 8 + 1 + } + + /// Creates a new Catalog Header struct using the provided parameters. + #[allow(clippy::too_many_arguments)] + pub fn new(metadata: Vec<u8>) -> Result<Self, Error> { + //type Error = CarbonadoError; + let cbor_data = serde_cbor::to_vec(&metadata)?; + println!("cbor data; {:?}", cbor_data); + + let cbor_len = cbor_data.len() as u8; + + println!("cbor length ; {:?}", cbor_len); + + let magic_no = carbonado::constants::MAGICNO; Cat files will use their own magic number, remember what I said: Use this for magic no: https://github.com/diba-io/carbonado/blob/518188593fc2a86bf2fa687f9165b83904c0d64f/src/constants.rs#L5 Except use: CARBONCAT00 instead of CARBONADO00 ------------------------------ In src/header.rs <#52 (comment)> : > + // } + + //metadata = cbor_data; + Ok(CatHeader { + cbor_len, + metadata: if metadata.iter().any(|b| b != &0) { + Some(metadata) + } else { + None + }, + }) + } + + /// Creates a header to be prepended to files. + pub fn try_to_vec(&self) -> Result<Vec<u8>, CarbonadoError> { + let mut cbore_bytes = self.cbor_len.to_le_bytes().to_vec(); // 2 bytes This variable is misspelled ------------------------------ In tests/file.rs <#52 (comment)> : > - let file_stream = stream::iter(file_bytes.clone()) - .chunks(1024 * 1024) - .map(|chunk| Ok(Bytes::from(chunk))) - .boxed(); - - // first file to write - let (_sk, pk) = generate_keypair(&mut thread_rng()); - - // info!("Write Delete:: Writing file if not exists in order to test delete"); - let file_did_write = write_file(&Secp256k1PubKey(pk), file_stream, None) - .await - .is_ok(); - - if file_did_write { - info!( - "Write File Group One to Delete File as blake3_hash:: {} ", - file_did_write.to_string() - ); - } This whole test is removed! Please put it back. ------------------------------ In tests/file.rs <#52 (comment)> : > - if blake3_hash { - info!( - "Write File in Group Two to Delete File as blake3_hash:: {} ", - blake3_hash.to_string() - ); Put these back, please ------------------------------ In src/structs.rs <#52 (comment)> : > + pub str_name: String, + pub str_date: String, + pub str_mime_type: String, Let's remove the str prefix: ⬇️ Suggested change - pub str_name: String, - pub str_date: String, - pub str_mime_type: String, + pub name: String, + pub date: String, + pub mime_type: String, — Reply to this email directly, view it on GitHub <#52 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ATWG7QUHINWGMHRSTRQWOMTXYZ5JNANCNFSM6AAAAAA4KVMQS4> . You are receiving this because you authored the thread.Message ID: ***@***.***>

[rustchain64]

WARNING; adding header to the file, breaks the test.
File no longer matches origonal hash

202 ) trace!("Write file header, should be added");
203 ) // file.write_all(&cat_header_bytes)?;

[rustchain64]

In "pass mime_type to tuple works"

We have verified that the mime_type value passes the mime_type to the tuple and is to be parsed from the segment, within "fs.rs"

[cryptoquick]

Really close! These changes should help get this on the right track.

[cryptoquick]

Please clean up debug statements and commented code

[cryptoquick]

Looking really good. LGTM.