Support generic value types (SparKV<V>)

[uzyn]

Summary

Makes SparKV generic over its value type instead of being hardcoded to String, addressing the "Support generic data types" item in the README TODO.

• SparKV<V = String> and KvEntry<V = String> — value type is now generic, defaulting to String so the bare type name and string-centric usage still work. Keys remain String.
• New ValueSize trait (fn value_size(&self) -> usize) with built-in impls for String, &str, Vec<u8>, &[u8], and all scalar types. max_item_size enforcement now works for any value type; custom types add a one-line impl.
• Config.max_item_size is now Option<usize> (default Some(500_000)); set it to None to disable per-entry size enforcement.
• Trait bounds placed on impls, not the struct: set/set_with_ttl require V: ValueSize, get requires V: Clone, delete moves the value out (no Clone needed).

Migrating from 0.1 to 0.2

This is a breaking release (0.2.0), but the migration is small and mechanical. The store behaves exactly as before; only value passing and max_item_size config change. The SparKV and KvEntry type names still resolve to their String forms, so type annotations and struct fields are unaffected. A migration guide is also included in the README.

1. set / set_with_ttl take the value by value — pass an owned value instead of &str:

// 0.1
kv.set("key", "value");
// 0.2
kv.set("key", "value".to_string()); // or "value".into()

get and delete still return Option<String> for the default String store, so read sites do not change.

2. Config.max_item_size is now Option<usize> — wrap the limit in Some, or use None to disable enforcement:

// 0.1
config.max_item_size = 500_000;
// 0.2
config.max_item_size = Some(500_000); // or None to disable

3. Custom value types need a ValueSize impl — only relevant if you switch away from String. Built-in types (integers, Vec<u8>, &[u8], etc.) already implement it.

No other call sites change.

Breaking changes (0.2.0)

• set(key, value: V) takes the value by value.
• get/delete return Option<V> (still Option<String> for the default).
• get requires V: Clone; set requires V: ValueSize.
• Config.max_item_size changed from usize to Option<usize>.

Testing

• cargo test — 24 passed (19 existing adapted + 5 new: generic int store, generic size enforcement, unbounded-when-None, and two ValueSize unit tests).
• cargo clippy --all-targets -- -D warnings — clean (also cleaned up a few pre-existing test-style lints).
• cargo fmt --check — clean.

[uzyn]

Review: Support generic value types (SparKV)

Reviewed at commit 6e67752. I built, tested, linted, and independently compiled the README examples. Verdict up front: Ready to merge — no blockers. A handful of non-blocking polish items are listed below for tracking.

Changes Overview

Makes the store generic over its value type: SparKV<V = String> and KvEntry<V = String>, with String as the default so the bare type name and existing string usage keep working. Introduces a ValueSize trait (impls for String/&str/str, Vec<u8>/&[u8]/[u8], and all scalars) so max_item_size enforcement works for any value type, and changes Config.max_item_size from usize to Option<usize> (None disables per-entry size enforcement). Trait bounds are placed on impls rather than the struct: set/set_with_ttl need V: ValueSize, get needs V: Clone, delete needs neither (moves the value out). README gets a generic-types section and a 0.1→0.2 migration guide; version bumped to 0.2.0.

Verification

• cargo test — 24 passed, 0 failed.
• cargo clippy --all-targets -- -D warnings — clean.
• cargo fmt --check — clean.
• Compiled the README quick-start, i64, Session, and a Vec<u8> example as a real binary — all compile and run.

Scope Alignment

The implementation matches the stated intent precisely, and the README TODO item it claims to close was removed. One thing the "Breaking changes (0.2.0)" list omits:

• Undocumented breaking change: KvEntry lost its PartialEq/Eq derives (src/kventry.rs:1). It went from #[derive(Debug, Clone, PartialEq, Eq)] to #[derive(Debug, Clone)]. KvEntry is a public re-export (pub use kventry::KvEntry), so any downstream code comparing KvEntry values with == will stop compiling. Dropping the derive is a reasonable call (deriving it generically would force V: PartialEq), but it should be listed in the breaking-changes section. Non-blocker.

Potential Bugs

No correctness bugs in the changed code. I specifically checked that size-enforcement semantics are preserved for the default String store: the old path compared value.len(), and ValueSize for String returns self.len(), so behavior is identical. The Option<usize> gate (src/lib.rs:139) correctly skips enforcement only when None.

One pre-existing issue, called out as context only (not introduced or touched by this PR, so not a blocker here): clear_expired does self.data.get(&exp_item.key).unwrap() (src/lib.rs:69). If a key is deleted manually, its stale ExpEntry remains in the heap (delete intentionally doesn't remove from the heap), and if that entry later reaches the top while expired, the unwrap() panics because the key is gone from data. Worth a follow-up issue, but it predates this PR and is out of scope for this review.

Security Issues

None. In-memory library, no input parsing, no unsafe, no new dependencies.

Test Coverage

The new tests are meaningful: generic i64 store round-trip + delete, size enforcement on a generic type, unbounded behavior when max_item_size = None, and ValueSize unit tests. Gaps (all non-blocking):

• The "delete needs no Clone" design claim is untested. This is a headline feature of the bound design, but every value type exercised in tests is Clone. A test storing a move-only type (a struct that impls ValueSize but not Clone), then delete-ing it, would actually prove delete works without Clone and guard the bound from regressing. I confirmed manually that it compiles, but nothing in the suite locks it in.
• README examples aren't doctests, so cargo test reports Doc-tests sparkv ... 0 tests. The Session and Vec<u8> examples compile today (I checked), but they're unguarded against future drift. Converting the key snippets to doctests would keep them honest.
• No store-level test for a byte-buffer value (Vec<u8>) — only the ValueSize unit test covers byte sizing. Minor.

Code Quality

• impl ValueSize for str and impl ValueSize for [u8] (src/value_size.rs:12, :30) are effectively unreachable through the store: V must be Sized to live in KvEntry<V>, so SparKV<str> / SparKV<[u8]> can never exist. They compile and could serve generic callers of the trait directly, but within this crate they're dead. Harmless — flagging only so it's a conscious choice.
• set requires V: ValueSize even when max_item_size = None. Disabling enforcement doesn't lift the trait requirement, so a type with no meaningful size still needs a stub value_size impl. The README documents "return 0 if you don't care," so this is acceptable, just an API-ergonomics note.
• char reports size_of::<char>() = 4 regardless of its UTF-8 byte length. Consistent with the "logical size" framing but mildly surprising next to String/str using byte length. Trivial.

Summary and Recommended Actions

Overall verdict: Ready to merge. Clean implementation, faithful to the stated scope, all checks green, examples verified.

Blockers: none.

Non-blockers (worth addressing, not blocking):

1. Document the KvEntry PartialEq/Eq derive removal in the breaking-changes list.
2. Add a test that stores and deletes a non-Clone value type to lock in the "delete needs no Clone" design.

Nice-to-haves (track for later):

1. Convert key README snippets to doctests so they're compiled by cargo test.
2. Add a store-level test for a Vec<u8> value type.
3. Decide whether the unsized str/[u8] ValueSize impls are worth keeping.
4. (Pre-existing, separate issue) clear_expired can panic via unwrap() after a manual delete leaves a stale heap entry.

Recommended merge commit message:

Support generic value types (SparKV<V>)

Make the value type generic, defaulting to String so existing usage keeps
working. Add a ValueSize trait (built-in impls for strings, byte buffers, and
scalars) so max_item_size enforcement works for any value type, and make
max_item_size an Option so the limit can be disabled. Trait bounds live on
impls: set requires V: ValueSize, get requires V: Clone, delete needs neither.
Update README with a generic-types section and a 0.1->0.2 migration guide.

Breaking change: bump to 0.2.0.