Skip to content

fix: Model.from_storage should handle missing optional fields#419

Merged
ngjunsiang merged 1 commit intoweeklyfrom
bugfix/model-from-storage-optional-fields
Apr 7, 2026
Merged

fix: Model.from_storage should handle missing optional fields#419
ngjunsiang merged 1 commit intoweeklyfrom
bugfix/model-from-storage-optional-fields

Conversation

@nycomp
Copy link
Copy Markdown
Contributor

@nycomp nycomp commented Apr 7, 2026

Summary

Fixes #418 - The Model.from_storage() method was raising KeyError when loading storage records that were missing optional fields (fields with default values).

Problem

When campus OAuth tokens (which don't have refresh_token) were stored in MongoDB and then retrieved, the OAuthToken.from_storage() method would raise KeyError: 'refresh_token' because:

  1. MongoDB stores sparse documents (missing keys for null/undefined values)
  2. Model.from_storage() used direct dictionary access record[field.name]
  3. Even though refresh_token: str | None = None has a default, the code didn't use it

Solution

Modified Model.from_storage() to handle missing fields gracefully:

  • Returns value from record if key exists
  • Falls back to field.default for fields with default values
  • Falls back to field.default_factory() for fields with factory defaults
  • Raises helpful KeyError only for truly required fields (no defaults)

Tests

Added tests/unit/model/test_base.py with comprehensive coverage:

  • from_storage with all fields present
  • from_storage with missing optional field (uses None)
  • from_storage with missing default field (uses default value)
  • from_storage with missing default_factory field (calls factory)
  • from_storage with missing required field (raises KeyError with helpful message)
  • Roundtrip serialization tests
  • Sparse record handling (simulates MongoDB documents)

Test Results

All tests pass:

  • sanity checks: PASS
  • type checks: PASS
  • unit tests: PASS (141 tests)
  • integration tests: PASS (30 tests)

🤖 Generated with Claude Code

Fixes #418

The Model.from_storage() method was raising KeyError when loading
storage records that were missing optional fields (fields with default
values), even though those fields were correctly defined with defaults.

This was causing OAuth token loading to fail for campus tokens, which
don't have a refresh_token field. When stored in MongoDB, the sparse
document doesn't include the refresh_token key, and from_storage()
would raise KeyError when trying to access it.

Changes:
- Modified Model.from_storage() to use record.get() pattern with
  fallback to field defaults for missing keys
- Added get_value() helper function that:
  1. Returns record value if key exists
  2. Falls back to field.default if available
  3. Falls back to field.default_factory() if available
  4. Raises KeyError with helpful message for truly required fields

Tests:
- Added test_base.py with comprehensive tests for Model serialization:
  - from_storage with all fields present
  - from_storage with missing optional field (uses None)
  - from_storage with missing default field (uses default value)
  - from_storage with missing default_factory field (calls factory)
  - from_storage with missing required field (raises KeyError)
  - Roundtrip serialization tests
  - Sparse record handling (simulates MongoDB documents)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@ngjunsiang ngjunsiang linked an issue Apr 7, 2026 that may be closed by this pull request
@ngjunsiang ngjunsiang merged commit d27eb16 into weekly Apr 7, 2026
4 checks passed
@ngjunsiang ngjunsiang deleted the bugfix/model-from-storage-optional-fields branch April 7, 2026 08:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug: Model.from_storage raises KeyError for missing optional fields

2 participants