Add Entity<TModel>(repository, string tableName) constructor overload - #25
Merged
Merged
Conversation
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
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.
Summary
Adds a further-simplified constructor overload on
Entity<TModel>(and the baseEntity<TModel, TFilter, TOptions>/Entity<T, TOptions>/Entity<TModel, TUpdate, TFilter, TOptions>constructor chain) that accepts a plainstring tableName, e.g.:This is equivalent to today's:
Internally the new overload just delegates to the existing
(repository, Table<TModel>)constructor, constructing theTable<TModel>from the given name so callers don't need to know aboutTable<T>for the common case. The existing(repository, Table<TModel>)constructor is untouched and remains available for advanced cases (custom schema, quoting, reusing an existingTable<T>, etc.).Docs (
docs/USAGE.md) were updated to show the simpleststring-based form first, noting theTable<T>overload as the more advanced option, per the reasoning from the linked review discussion.Changes
src/Simpleverse.Repository.Db/Entity/Entity.cs: addedEntity(DbRepository repository, string tableName)overloads onEntity<TModel, TUpdate, TFilter, TOptions>,Entity<TModel, TFilter, TOptions>,Entity<T, TOptions>, andEntity<T>, each delegating to the existingTable<TModel>-based constructor.src/Simpleverse.Repository.Db.Test/SqlServer/Entity/EntityTest.cs: added a newIdentityEntityByTableNametest helper class built with the new string-based constructor, plus a test (Constructor_WhenGivenTableNameString_CreatesEquivalentTableToTableConstructorOverload) asserting it produces an equivalentTable<T>/table name to the existingTable<T>-based constructor.docs/USAGE.md: reordered the "Defining an Entity" quick-start section to lead with the new string-based constructor, with theTable<T>overload described afterwards as the more advanced form.Test plan
dotnet build src/Simpleverse.Repository.Db/Simpleverse.Repository.Db.csproj– succeeds, 0 warnings/errors.dotnet build src/Simpleverse.Repository.Db.Test/Simpleverse.Repository.Db.Test.csproj– succeeds (only pre-existing xUnit analyzer warnings, unrelated to this change).DatabaseTestFixture/SqlRepositorypattern used by all other tests inEntityTest.cs, and only exercises the pure C# constructor delegation (no actual DB round-trip), so it should behave identically to the other entity-construction tests in this file once run against a real SQL Server, as CI presumably does.Risks / things to double check
ProjectedEntity<...>constructors — they take anIEntity<...>instance rather than aTable<T>/table name, so the same simplification doesn't apply there. Worth confirming that's the intended scope per the issue.stringtoTable<T>elsewhere in the codebase (none found in review, but worth a second look).TableNameequivalence (via a small publicTableNameproperty added to the two test helper entity classes) since running the DB-backed test suite wasn't possible in this sandbox; a maintainer with access to CI/a SQL Server should confirm the new test passes end-to-end.Closes #24