Review ManagedEntry

[strawgate]

It looks like ManagedEntry has turned into a bit of slop. Let's review how each store and wrapper leverages ManagedEntry and come up with a clean-up plan

[claude]

Claude finished @strawgate's task —— View job

---

ManagedEntry Investigation Complete ✓

tl;dr

ManagedEntry has become inconsistent across stores and wrappers. Recent "native storage" features (PR #149, #164, #134) introduced multiple serialization approaches with differing parameter names and conventions. Key problems: inconsistent serialization methods, duplicated helper functions, mixed responsibility between stores and ManagedEntry itself, and unclear documentation about when to stringify values.

---

Recommendation

Consolidate serialization logic into ManagedEntry and establish clear, consistent patterns across all stores.

The primary issue is that ManagedEntry's serialization responsibilities have leaked out into individual stores, creating duplication and inconsistency. Each store has re-implemented serialization logic with different parameter names (stringified_value vs stringify_value vs native_storage), making the codebase harder to maintain.

Specific actions:

1. Add standardized serialization methods to ManagedEntry class
2. Deprecate store-specific helper functions in favor of ManagedEntry methods
3. Standardize parameter naming across all stores (native_storage as the canonical name)
4. Create clear documentation about when to use each serialization mode
5. Consider extracting serialization strategy into a separate class/module

---

Findings

1. Inconsistent Serialization Patterns

Different stores use different approaches to serialize ManagedEntry:

MongoDB Store (managed_entry.py:48, mongodb/store.py:36-69)

• Uses ManagedEntry.from_dict(stringified_value=True) for deserialization
• Uses managed_entry.to_dict(stringify_value=True) for serialization
• Has helper functions document_to_managed_entry() and managed_entry_to_document()
• Parameter name: stringified_value (input) vs stringify_value (output) - inconsistent!

Elasticsearch Store (elasticsearch/store.py:87-132)

• Uses native_storage parameter (boolean, defaults to True)
• Stores in two fields: value.flattened (native) or value.string (JSON)
• Has helper functions managed_entry_to_document() and source_to_managed_entry()
• Uses managed_entry.value_as_dict and managed_entry.value_as_json properties
• Reads from both fields for backward compatibility

Redis Store (redis/store.py:23-51)

• Has dedicated helper functions managed_entry_to_json() and json_to_managed_entry()
• Always serializes to JSON (no native mode)
• Uses to_json(include_metadata=True, include_expiration=True, include_creation=True)

Memory Store (memory/store.py:24-40)

• Uses intermediate MemoryCacheEntry class that wraps JSON string
• Always serializes via to_json() / from_json()
• No helper functions, uses class methods on MemoryCacheEntry

2. Wrapper Misuse of ManagedEntry

Two wrappers instantiate ManagedEntry directly for size calculation, which violates separation of concerns:

CompressionWrapper (compression/wrapper.py:59):

item_size: int = len(ManagedEntry(value=value).to_json())

LimitSizeWrapper (limit_size/wrapper.py:68):

item_size: int = len(ManagedEntry(value=value).to_json())

Problem: These wrappers create temporary ManagedEntry objects just to calculate serialized size. This is inefficient and creates unnecessary coupling between wrappers and the internal serialization format.

3. Inconsistent Parameter Naming

The serialization parameter names vary across the codebase:

Location	Parameter Name	Type	Purpose
ManagedEntry.to_dict()	stringify_value	bool	Whether to stringify the value field
ManagedEntry.from_dict()	stringified_value	bool	Whether value field is stringified
MongoDB Store	stringified_value	bool	Indicates stringified storage mode
Elasticsearch Store	native_storage	bool	Whether to use native ES storage
DuckDB Store (PR #134)	native_storage	bool	Whether to use native JSON column

Problem: stringify_value vs stringified_value are inconsistent (verb vs past participle), and the newer native_storage parameter inverts the boolean logic (True = native, False = stringify).

4. Mixed Responsibilities

ManagedEntry has unclear boundaries for what it should vs shouldn't do:

What ManagedEntry does:

• Store value, TTL metadata, timestamps
• Serialize to/from JSON with various options
• Provide value_as_json and value_as_dict properties
• Handle TTL calculations and expiration checks

What store-specific helpers do:

• Convert ManagedEntry to store-specific document format (MongoDB, Elasticsearch)
• Decide which storage mode to use (native vs stringified)
• Handle backward compatibility when reading

Problem: The line between "ManagedEntry serialization" and "store-specific document transformation" is blurry. Some stores (Redis, Memory) use ManagedEntry methods directly, while others (MongoDB, Elasticsearch) wrap them in helper functions.

5. Recent "Native Storage" Pattern Creates Duplication

Three recent PRs (#149 Elasticsearch, #164 MongoDB, #134 DuckDB) all implemented "native storage" modes that:

• Add a native_storage boolean parameter to store constructors
• Use two-field approach (one for native, one for stringified)
• Implement similar serialization/deserialization logic
• All have slight variations in implementation details

This pattern is being copy-pasted across stores rather than being abstracted into shared utilities.

---

Detailed Action Plan

Phase 1: Standardize ManagedEntry API

Goal: Make ManagedEntry the single source of truth for serialization

1. Add new serialization modes to ManagedEntry:

   class SerializationMode(Enum):
       FULL_JSON = "full_json"  # Current default: metadata + JSON value
       NATIVE_DICT = "native_dict"  # For stores with native dict support
       COMPACT_JSON = "compact_json"  # Minimal JSON, no metadata

2. Add mode-aware serialization methods:

   def to_store_format(self, mode: SerializationMode) -> dict[str, Any]:
       """Convert to store-specific format based on mode."""
       
   @classmethod
   def from_store_format(cls, data: dict[str, Any], mode: SerializationMode) -> Self:
       """Reconstruct from store-specific format."""

3. Deprecate inconsistent parameters:

   • Mark stringify_value and stringified_value as deprecated
   • Add deprecation warnings to guide users to new API
   • Keep old API working for backward compatibility

Phase 2: Refactor Stores to Use Standardized API

Goal: Eliminate store-specific helper functions

1. MongoDB: Replace managed_entry_to_document() and document_to_managed_entry() with ManagedEntry.to_store_format(NATIVE_DICT) and ManagedEntry.from_store_format()

2. Elasticsearch: Replace helper functions with ManagedEntry methods, keeping the two-field approach for backward compatibility

3. Redis: Simplify to use ManagedEntry.to_store_format(FULL_JSON)

4. Memory: Keep MemoryCacheEntry but have it use standardized ManagedEntry methods

Phase 3: Fix Wrapper Misuse

Goal: Eliminate direct ManagedEntry instantiation in wrappers

1. Add utility function for size calculation:

   def estimate_serialized_size(value: Mapping[str, Any]) -> int:
       """Estimate size of value when serialized, without creating ManagedEntry."""
       return len(dump_to_json(dict(value)))

2. Update CompressionWrapper and LimitSizeWrapper to use the utility function instead of instantiating ManagedEntry

Phase 4: Documentation and Testing

Goal: Make usage patterns clear and prevent future drift

1. Document serialization modes in AGENTS.md and docstrings:

   • When to use FULL_JSON (distributed stores without native dict support: Redis, Memcached, Valkey)
   • When to use NATIVE_DICT (document databases with query support: MongoDB, Elasticsearch, DuckDB)
   • When to use COMPACT_JSON (if ever needed for size optimization)

2. Add integration tests that verify:

   • All stores can round-trip ManagedEntry correctly
   • Backward compatibility with old serialization formats
   • Migration paths from old to new formats

3. Create migration guide for users with existing data in MongoDB/Elasticsearch who need to handle the format changes

Phase 5: Consider Serialization Strategy Pattern

Goal: Make it easy to add new storage modes without modifying ManagedEntry

1. Extract serialization logic into strategy classes:

   class SerializationStrategy(ABC):
       @abstractmethod
       def serialize(self, entry: ManagedEntry) -> dict[str, Any]: ...
       
       @abstractmethod
       def deserialize(self, data: dict[str, Any]) -> ManagedEntry: ...
   
   class FullJsonStrategy(SerializationStrategy): ...
   class NativeDictStrategy(SerializationStrategy): ...

2. Update ManagedEntry to accept strategy in serialization methods

3. Update stores to specify their strategy in constructor

---

Related Items

Related Issues and Pull Requests

Repository	Item	Relevance
strawgate/py-key-value	PR #149 - Native storage for Elasticsearch	Introduced native_storage parameter and two-field approach (merged)
strawgate/py-key-value	PR #164 - Native storage for MongoDB	Copied Elasticsearch pattern with slight variations (open)
strawgate/py-key-value	PR #134 - DuckDB store	Third implementation of native storage pattern (open)
strawgate/py-key-value	Issue #90 - Review MongoDB	Originally identified MongoDB storage format concerns
strawgate/py-key-value	Issue #87 - Elasticsearch native storage	Requested flattened field support in Elasticsearch
strawgate/py-key-value	Issue #159 - MongoDB native storage	Requested same pattern as Elasticsearch for MongoDB
strawgate/py-key-value	PR #104 - Native document storage (closed)	Earlier attempt at native storage, closed due to refactor conflicts

Related Files

File	Relevance	Key Lines
key_value/shared/utils/managed_entry.py	Core ManagedEntry implementation	14-153
key_value/aio/stores/base.py	BaseStore that all stores inherit from	52-489
key_value/aio/stores/mongodb/store.py	MongoDB with stringified_value parameter	36-69
key_value/aio/stores/elasticsearch/store.py	Elasticsearch with native_storage parameter	87-132
key_value/aio/stores/redis/store.py	Redis with dedicated helper functions	23-51
key_value/aio/stores/memory/store.py	Memory with MemoryCacheEntry wrapper	24-40
key_value/aio/wrappers/compression/wrapper.py	Wrapper that misuses ManagedEntry for size calc	59
key_value/aio/wrappers/limit_size/wrapper.py	Wrapper that misuses ManagedEntry for size calc	68

---

Problems Encountered During Investigation

None - all investigation tools and resources were accessible and provided comprehensive information.

---

[strawgate]

@claude each store needs the data in a slightly different format, would it make sense to have managed entry and then some stores will take their own managed entry classes that inherit from managed entry?

I'm thinking about Elasticsearch, mongo, memory, etc

[claude]

Claude finished @strawgate's task —— View job

---

ManagedEntry Design Analysis

• Read repository guidelines
• Review current ManagedEntry implementation
• Analyze store-specific requirements
• Evaluate inheritance vs alternative patterns
• Provide design recommendation

---

My Recommendation: Composition over Inheritance

I would not recommend store-specific ManagedEntry subclasses. Instead, I suggest using composition with serialization adapters or a strategy pattern. Here's why:

Problems with Inheritance Approach

1. ManagedEntry is a data class, not behavior - It's a simple container for value, ttl, expires_at, and created_at. Subclassing it would mean duplicating this data structure for each store, which violates DRY principles.

2. The differences aren't in the entry itself - Looking at the stores, they all work with the same ManagedEntry data. The differences are in how they serialize/deserialize that data for storage:

   • MongoDB: Stringifies the value field (stringify_value=True)
   • Elasticsearch: Can use native flattened field OR stringified value
   • Redis: Always serializes to full JSON string
   • Memory: Uses an intermediate MemoryCacheEntry wrapper (which is actually composition!)

3. Store logic would leak into ManagedEntry - If MongoDB had MongoManagedEntry and Elasticsearch had ElasticsearchManagedEntry, you'd need to:

   • Import store-specific classes throughout the codebase
   • Handle type conversions when moving between stores
   • Duplicate TTL calculation, expiration logic, and other core behavior

4. Breaks the Liskov Substitution Principle - Stores should be interchangeable. If each store requires its own ManagedEntry subclass, you can't easily swap stores or wrap them.

Better Alternative: Serialization Adapters

The real variation is in serialization, not the entry itself. I recommend one of these approaches:

Option 1: Store-Specific Serialization Functions (Current Pattern)

This is actually what you already have, and it's pretty clean:

# MongoDB
def managed_entry_to_document(key: str, managed_entry: ManagedEntry) -> dict[str, Any]:
    return {
        "key": key,
        **managed_entry.to_dict(stringify_value=True),
    }

# Elasticsearch  
def managed_entry_to_document(collection: str, key: str, managed_entry: ManagedEntry, *, native_storage: bool) -> dict[str, Any]:
    # Custom logic for two-field approach
    ...

# Redis
def managed_entry_to_json(managed_entry: ManagedEntry) -> str:
    return managed_entry.to_json(include_metadata=True, ...)

Pros:

• Simple and explicit
• Each store owns its serialization logic
• No new abstractions

Cons:

• Duplication across stores (you've already identified this)
• Inconsistent parameter naming

Option 2: Serialization Strategy Classes (Recommended)

Create a formal adapter/strategy pattern:

@dataclass
class SerializationAdapter(ABC):
    """Adapter for converting ManagedEntry to/from store-specific format."""
    
    @abstractmethod
    def to_storage(self, key: str, entry: ManagedEntry, collection: str | None = None) -> dict[str, Any] | str:
        """Convert ManagedEntry to store-specific format."""
        pass
    
    @abstractmethod
    def from_storage(self, data: dict[str, Any] | str) -> ManagedEntry:
        """Convert store-specific format back to ManagedEntry."""
        pass


@dataclass
class MongoDBAdapter(SerializationAdapter):
    def to_storage(self, key: str, entry: ManagedEntry, collection: str | None = None) -> dict[str, Any]:
        return {
            "key": key,
            **entry.to_dict(stringify_value=True, include_metadata=True),
        }
    
    def from_storage(self, data: dict[str, Any]) -> ManagedEntry:
        return ManagedEntry.from_dict(data, stringified_value=True)


@dataclass
class ElasticsearchAdapter(SerializationAdapter):
    native_storage: bool = True
    
    def to_storage(self, key: str, entry: ManagedEntry, collection: str | None = None) -> dict[str, Any]:
        document = {"collection": collection, "key": key, "value": {}}
        if self.native_storage:
            document["value"]["flattened"] = entry.value_as_dict
        else:
            document["value"]["string"] = entry.value_as_json
        # ... metadata fields
        return document
    
    def from_storage(self, data: dict[str, Any]) -> ManagedEntry:
        # Read from either flattened or string field
        ...


@dataclass  
class RedisAdapter(SerializationAdapter):
    def to_storage(self, key: str, entry: ManagedEntry, collection: str | None = None) -> str:
        return entry.to_json(include_metadata=True, include_expiration=True, include_creation=True)
    
    def from_storage(self, data: str) -> ManagedEntry:
        return ManagedEntry.from_json(data, includes_metadata=True)

Then each store holds an adapter:

class MongoDBStore(BaseStore):
    _adapter: SerializationAdapter
    
    def __init__(self, ...):
        self._adapter = MongoDBAdapter()
        ...
    
    async def _put_managed_entry(self, *, key: str, collection: str, managed_entry: ManagedEntry) -> None:
        document = self._adapter.to_storage(key=key, entry=managed_entry)
        await self._collection.update_one(...)

Pros:

• Clear separation of concerns
• Testable in isolation
• Store logic stays in store, serialization logic in adapters
• Easy to add new stores or change serialization
• Adapters can be configured (like native_storage flag)

Cons:

• Adds abstraction layer
• Slightly more code

Option 3: Enhance ManagedEntry with Modes (Middle Ground)

Add serialization modes directly to ManagedEntry but keep stores generic:

class SerializationMode(Enum):
    FULL_JSON = "full_json"        # Redis, Memcached
    NATIVE_DICT = "native_dict"    # MongoDB, Elasticsearch (native mode)
    STRINGIFIED_DICT = "stringified_dict"  # MongoDB, Elasticsearch (compat mode)

class ManagedEntry:
    ...
    
    def to_storage(self, mode: SerializationMode, **extra_fields) -> dict[str, Any] | str:
        """Serialize for storage in specified mode."""
        if mode == SerializationMode.FULL_JSON:
            return self.to_json(include_metadata=True, ...)
        elif mode == SerializationMode.NATIVE_DICT:
            return {"value": self.value_as_dict, ...}
        elif mode == SerializationMode.STRINGIFIED_DICT:
            return {"value": self.value_as_json, ...}
    
    @classmethod
    def from_storage(cls, data: dict | str, mode: SerializationMode) -> Self:
        """Deserialize from storage format."""
        ...

Pros:

• Centralizes serialization in ManagedEntry
• No extra classes
• Explicit modes

Cons:

• ManagedEntry becomes more complex
• Doesn't handle store-specific fields well (like Elasticsearch's two-field approach)

Real-World Example: Memory Store

Interestingly, your MemoryStore already uses composition correctly! It has MemoryCacheEntry that wraps the JSON string:

@dataclass
class MemoryCacheEntry:
    json_str: str
    expires_at: datetime | None
    
    @classmethod
    def from_managed_entry(cls, managed_entry: ManagedEntry) -> Self: ...
    
    def to_managed_entry(self) -> ManagedEntry: ...

This is great! It's composition, not inheritance. MemoryCacheEntry is a store-specific container that adapts ManagedEntry for the memory store's needs (caching with TLRU).

My Specific Recommendation

Go with Option 2 (Serialization Strategy Classes) for these reasons:

1. It matches the existing pattern in MemoryStore (composition, not inheritance)
2. It solves the inconsistency problems you identified (parameter naming, duplication)
3. It keeps ManagedEntry simple and focused on its core responsibility
4. It makes testing easier (test adapters separately from stores)
5. It's more maintainable as you add new stores

Implementation Path

1. Create key_value/shared/utils/serialization.py with base SerializationAdapter class
2. Move existing helper functions to adapter classes in each store's directory
3. Update stores to use adapters instead of direct ManagedEntry methods
4. Gradually standardize on consistent parameter names via the adapter interface

This preserves backward compatibility while cleaning up the architecture.

---

Summary

Don't subclass ManagedEntry. The variation isn't in the entry's data or behavior—it's in how stores serialize that data. Use composition (adapters/strategies) instead of inheritance. This keeps ManagedEntry simple, stores interchangeable, and serialization logic modular.

[strawgate]

@claude make a PR please

[claude]

Claude finished @strawgate's task —— View job • claude/issue-165-20251029-0155 • Create PR ➔

---

ManagedEntry Cleanup Complete ✅

Successfully refactored ManagedEntry serialization using the adapter pattern to consolidate and standardize serialization logic across all stores.

---

Summary of Changes

Created Serialization Adapter Pattern:

• Added SerializationAdapter base class in key-value-shared/src/key_value/shared/utils/serialization.py
• Implemented three adapter types:
  • FullJsonAdapter: For stores without native dict support (Redis, Valkey, Memcached)
  • StringifiedDictAdapter: For MongoDB with stringified value fields
  • ElasticsearchAdapter: For Elasticsearch with configurable native/stringified storage

Updated Stores:

• MongoDB: Now uses StringifiedDictAdapter, removed helper functions
• Elasticsearch: Now uses ElasticsearchAdapter, removed helper functions, maintains backward compatibility with both flattened and string fields
• Redis: Now uses FullJsonAdapter, removed helper functions

Fixed Wrapper Issues:

• Added estimate_serialized_size() utility function to managed_entry.py
• Updated CompressionWrapper and LimitSizeWrapper to use utility instead of creating unnecessary ManagedEntry instances
• Eliminates inefficient temporary object creation for size calculations

Updated Tests:

• Modified test files to use new adapter-based approach
• All serialization round-trip tests updated to use adapters directly

Generated Sync Library:

• Rebuilt synchronous library from async code
• All changes propagated to sync equivalents

---

Benefits

✅ Consistent Interface: All stores now use the same serialization pattern
✅ Separation of Concerns: Serialization logic isolated in adapters
✅ Better Maintainability: Easier to modify or add new serialization strategies
✅ Improved Performance: Wrappers no longer create unnecessary ManagedEntry objects
✅ Enhanced Testability: Adapters can be tested independently

---

Create Pull Request

---

| Branch: claude/issue-165-20251029-0155