Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add export_keys optional param to native_db macro #246

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion native_db_macro/src/model_attributes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,14 @@ use quote::ToTokens;
use std::collections::HashSet;
use syn::meta::ParseNestedMeta;
use syn::parse::Result;
use syn::Field;
use syn::{Field, LitBool};

#[derive(Clone)]
pub(crate) struct ModelAttributes {
pub(crate) struct_name: StructName,
pub(crate) primary_key: Option<KeyDefinition<()>>,
pub(crate) secondary_keys: HashSet<KeyDefinition<KeyOptions>>,
pub(crate) do_export_keys: Option<LitBool>,
}

impl ModelAttributes {
Expand Down Expand Up @@ -73,6 +74,8 @@ impl ModelAttributes {
}

self.secondary_keys.insert(key);
} else if meta.path.is_ident("export_keys") {
self.do_export_keys = Some(meta.value()?.parse()?);
} else {
panic!(
"Unknown attribute: {}",
Expand Down
15 changes: 15 additions & 0 deletions native_db_macro/src/model_native_db.rs
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,21 @@ impl ModelNativeDB {
Ident::new(&format!("{}Key", struct_name), Span::call_site().into())
}

pub(crate) fn keys_enum_visibility(&self) -> proc_macro2::TokenStream {
let do_export = match &self.attrs.do_export_keys {
Some(do_export_keys) => do_export_keys.value,
None => false,
};

let visibility = if do_export {
""
} else {
"(crate)"
};

format!("pub{}", visibility).parse().unwrap()
}

pub(crate) fn secondary_keys_enum(&self) -> Vec<proc_macro2::TokenStream> {
self.attrs
.secondary_keys
Expand Down
4 changes: 3 additions & 1 deletion native_db_macro/src/native_db.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ pub fn native_db(args: TokenStream, input: TokenStream) -> TokenStream {
struct_name: struct_name.clone(),
primary_key: None,
secondary_keys: Default::default(),
do_export_keys: None,
};
let model_attributes_parser = syn::meta::parser(|meta| attrs.parse(meta));
parse_macro_input!(args with model_attributes_parser);
Expand All @@ -33,6 +34,7 @@ pub fn native_db(args: TokenStream, input: TokenStream) -> TokenStream {
let native_db_gks = model_native_db.native_db_secondary_key();
let native_db_model = model_native_db.native_db_model();

let keys_enum_visibility = model_native_db.keys_enum_visibility();
let keys_enum_name = model_native_db.keys_enum_name();
let keys_enum = model_native_db.secondary_keys_enum();
let keys_enum_database_key = model_native_db.keys_enum_database_key();
Expand All @@ -57,7 +59,7 @@ pub fn native_db(args: TokenStream, input: TokenStream) -> TokenStream {
}

#[allow(non_camel_case_types)]
pub(crate) enum #keys_enum_name {
#keys_enum_visibility enum #keys_enum_name {
#(#keys_enum),*
}

Expand Down
7 changes: 6 additions & 1 deletion src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -391,6 +391,11 @@ doc_comment! {
include_str!("../README.md")
}

/// Macro which link [`native_model`](https://crates.io/crates/native_model) to the Native DB. See [`Builder.define`](struct.Builder.html#method.define) for more information.
/// Macro which link [`native_model`](https://crates.io/crates/native_model) to the Native DB. See
/// [`Builder.define`](struct.Builder.html#method.define) for more information.
///
/// Optional parameter `export_keys` controls whether the model keys enum is visible outside of the
/// crate, default value is false with visibility limited to `pub(crate)`. Use
/// `#[native_db(export_keys = true)]` to make enum visible outside of crate (ie. `pub`).
Comment on lines +394 to +399
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you think about moving the documentation of export_keys into the documentation of define in order to have all the model creation documentation in one place?

pub use native_db_macro::*;
pub use serialization::*;
27 changes: 27 additions & 0 deletions tests/macro_def/export_keys_attribute.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
use native_db::*;
use native_model::{native_model, Model};
use serde::{Deserialize, Serialize};

// TODO somehow test visibility of keys enum from a sibling/child crate?
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have left a TODO in tests/macro_def/export_keys_attribute.rs:5 to note that this functionality can't be fully tested without somehow testing from an external crate. I could just remove that TODO if you're happy with how it is, not sure what your thoughts on the matter are.

IMHO, it is preferable for this feature to be tested by an external library to avoid regressions. For now, it's fine, but I would prefer that you leave the TODO.


/// Test struct to ensure `#[native_db(export_keys = true)]` compiles successfully
#[derive(Serialize, Deserialize, Eq, PartialEq, Debug)]
#[native_model(id = 1, version = 1)]
#[native_db(export_keys = true)]
struct Item {
#[primary_key]
id: u32,
#[secondary_key]
name: String,
}

#[test]
fn test_insert_my_item() {
let item = Item {
id: 1,
name: "test".to_string(),
};

let key = item.native_db_primary_key();
assert_eq!(key, 1_u32.to_key());
Comment on lines +20 to +26
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest simply testing whether the database works with a get on the primary keys and a get on the secondary keys. This ensures that there are no regressions with the addition of the new export_keys parameter.

Suggested change
let item = Item {
id: 1,
name: "test".to_string(),
};
let key = item.native_db_primary_key();
assert_eq!(key, 1_u32.to_key());
let mut models: Models = Models::new();
models.define::<Item>().unwrap();
let db = Builder::new()
.create_in_memory(&models)
.unwrap();
let rw = db.rw_transaction().unwrap();
rw.insert(Item { id: 1, name: "test".to_string() }).unwrap();
rw.commit().unwrap();
// Get primary key
let r = db.r_transaction().unwrap();
let result_item: Item = r.get().primary(1u32).unwrap().unwrap();
assert_eq!(result_item.id, 1);
assert_eq!(result_item.name, "test");
// Get secondary key
let r = db.r_transaction().unwrap();
let result_item: Vec<Item> = r.scan().secondary(ItemKey::name).unwrap().all().unwrap().try_collect().unwrap();
assert_eq!(result_item.len(), 1);
assert_eq!(result_item[0].id, 1);
assert_eq!(result_item[0].name, "test");

}
1 change: 1 addition & 0 deletions tests/macro_def/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,4 @@ mod primary_key_attribute;
mod secondary_key;
mod secondary_key_attribute;
mod secondary_key_mix;
mod export_keys_attribute;
Loading