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 has to Path and Map #322

Merged
merged 2 commits into from
Jul 12, 2021
Merged
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
13 changes: 6 additions & 7 deletions contracts/cw1155-base/src/contract.rs
Original file line number Diff line number Diff line change
Expand Up @@ -233,9 +233,9 @@ pub fn execute_mint(
}

// insert if not exist
let key = TOKENS.key(&token_id);
if deps.storage.get(&key).is_none() {
key.save(deps.storage, &"".to_owned())?;
if !TOKENS.has(deps.storage, &token_id) {
Copy link
Contributor

Choose a reason for hiding this comment

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

👍🏼

Copy link
Member Author

Choose a reason for hiding this comment

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

When writing tests I found out the (untested) case would actually panic.
If you attempt to write b"" to the db.

(Which we added to avoid getting confusing results from the cosmos sdk storage layer). So I fixed that with real data as well.

Copy link
Contributor

Choose a reason for hiding this comment

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

So, the original code had that flaw too?

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, I guess you mean, as a key, not as a value.

// we must save some valid data here
TOKENS.save(deps.storage, &token_id, &String::new())?;
}

Ok(rsp)
Expand Down Expand Up @@ -329,10 +329,9 @@ pub fn execute_batch_mint(
event.add_attributes(&mut rsp);

// insert if not exist
let key = TOKENS.key(&token_id);
if deps.storage.get(&key).is_none() {
// insert an empty entry so token enumeration can find it
key.save(deps.storage, &"".to_owned())?;
if !TOKENS.has(deps.storage, &token_id) {
// we must save some valid data here
TOKENS.save(deps.storage, &token_id, &String::new())?;
}
}

Expand Down
33 changes: 33 additions & 0 deletions packages/storage-plus/src/map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,12 @@ where
self.key(k).may_load(store)
}

/// has returns true or false if any data is at this key, without parsing or interpreting the
/// contents.
pub fn has(&self, store: &dyn Storage, k: K) -> bool {
self.key(k).has(store)
}

/// Loads the data, perform the specified action, and store the result
/// in the database. This is shorthand for some common sequences, which may be useful.
///
Expand Down Expand Up @@ -199,6 +205,33 @@ mod test {
assert_eq!(None, john.may_load(&store).unwrap());
}

#[test]
fn existence() {
let mut store = MockStorage::new();

// set data in proper format
let data = Data {
name: "John".to_string(),
age: 32,
};
PEOPLE.save(&mut store, b"john", &data).unwrap();

// set and remove it
PEOPLE.save(&mut store, b"removed", &data).unwrap();
PEOPLE.remove(&mut store, b"removed");

// invalid, but non-empty data
store.set(&PEOPLE.key(b"random"), b"random-data");

// any data, including invalid or empty is returned as "has"
assert!(PEOPLE.has(&store, b"john"));
assert!(PEOPLE.has(&store, b"random"));

// if nothing was written, it is false
assert!(!PEOPLE.has(&store, b"never-writen"));
assert!(!PEOPLE.has(&store, b"removed"));
}

#[test]
fn composite_keys() {
let mut store = MockStorage::new();
Expand Down
6 changes: 6 additions & 0 deletions packages/storage-plus/src/path.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,12 @@ where
may_deserialize(&value)
}

/// has returns true or false if any data is at this key, without parsing or interpreting the
/// contents. It will returns true for an length-0 byte array (Some(b"")), if you somehow manage to set that.
pub fn has(&self, store: &dyn Storage) -> bool {
store.get(&self.storage_key).is_some()
}

/// Loads the data, perform the specified action, and store the result
/// in the database. This is shorthand for some common sequences, which may be useful.
///
Expand Down