Refactor property info handling, allow keys different from Id property #158
Closed
nseguin42 wants to merge 8 commits intoDapperLib:mainfrom
nseguin42:main
Closed
Refactor property info handling, allow keys different from Id property #158nseguin42 wants to merge 8 commits intoDapperLib:mainfrom nseguin42:main
nseguin42 wants to merge 8 commits intoDapperLib:mainfrom
nseguin42:main
Conversation
- Only treat a property named 'Id' as a key if there is no Key or ExplicitKey property. (fixes #140) The new behavior allows manually specifying a key (or explicit key) that is different from the 'Id' property. Any usage that would previously have succeeded (not thrown an exception) is unaffected. - Introduce PropertyInfoWrapper to hold all the relevant information about a type's properties. - Move logic about selecting a key into the PropertyInfoWrapper so it can be cached as well. - Use a single ConcurrentDictionary of PropertyInfoWrappers instead of one for each type of property. Thread-safety is preserved because all mutation happens inside a thread-safe Lazy<T>. - Add `PropertyNamedId` to distinguish this property from those explicitly annotated with a `KeyAttribute`.
…ropertiesCache Quick and dirty fix. Let's see if the tests pass.
This brings the behavior closer to current while preserving the new GetSingleKey<T> behavior. Should fix more tests.
Woops x3
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
I've moved this PR to a draft until I have the chance to investigate the test failures. In hindsight, it might be worth just changing GetSingleKey() in a minimal way.
Behavior changes:
GetSingleKey<T>(), only return the property named 'Id' as a fallback if there is noKeyorExplicitKeyproperty. (fixes Why a hard reference to the column "id" #140)Note: The new behavior allows manually specifying a key (or explicit key) that is different from the 'Id' property. Any usage that would previously have succeeded (not thrown an exception) should be unaffected.
Refactors/implementation details:
Introduce
PropertyInfoWrapperto hold all the relevant information about a type's properties. This object is only mutated within a thread-safeLazy<T>, so we shouldn't have any issues even if multiple threads grab the same instance from a ConcurrentDictionary.Move logic about selecting a key into the
PropertyInfoWrapperand cache the result ofGetSingleKey<T>().Add
PropertyNamedIdto distinguish properties named 'Id' from properties explicitly annotated with aKeyAttribute.Add an optional parameter to
KeyPropertiesCacheto specify whether you want to include theIdproperty or not, so that the existing code didn't need to be modified as much.