Skip to content

Conversation

@irateswami
Copy link

Refactored the local key store system to remove reliance on reflection when instantiating keys from Turnkey private key data. Introduced a generic TurnkeyKeyFactory interface and concrete Factory implementations for both API keys and encryption keys. This refactor improves type safety, testability, and future extensibility of key instantiation logic.

Key improvements:
- Removed reflection-based logic from KeyFactory in favor of a type-safe factory pattern
- Added concrete Factory types in apikey and encryptionkey packages
- Updated store/local to use NewAPIKeyStore and NewEncryptionKeyStore constructors
- Preserved backwards compatibility via DeprecatedKeyFactory
- Improved test readability and safety by switching to factory-based store creation

…n when instantiating keys from Turnkey private key data. Introduced a generic TurnkeyKeyFactory interface and concrete Factory implementations for both API keys and encryption keys.

Key improvements:
- Removed reflection-based logic from KeyFactory in favor of a type-safe factory pattern
- Added concrete Factory types in apikey and encryptionkey packages
- Updated store/local to use NewAPIKeyStore and NewEncryptionKeyStore constructors
- Preserved backwards compatibility via DeprecatedKeyFactory
- Improved test readability and safety by switching to factory-based store creation

This refactor improves type safety, testability, and future extensibility of key instantiation logic.
Copy link
Contributor

@r-n-o r-n-o left a comment

Choose a reason for hiding this comment

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

Looks really good, thank you for the PR @irateswami!

Comment on lines +38 to +40
// DeprecatedKeyFactory is the old reflection-based factory for backward compatibility.
// Deprecated: Use NewKeyFactory with concrete factory implementations instead.
type DeprecatedKeyFactory[T common.IKey[M], M common.IMetadata] struct{}
Copy link
Contributor

Choose a reason for hiding this comment

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

💟

@r-n-o
Copy link
Contributor

r-n-o commented Aug 28, 2025

Unfortunately you'll have to push a new commit since we enforce signatures on commits:

image

Once that's done and CI/checks are happy I'll merge!

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.

2 participants