Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support Multiple Key Types for Base Entity #172

Closed
iammukeshm opened this issue Nov 18, 2021 · 8 comments
Closed

Support Multiple Key Types for Base Entity #172

iammukeshm opened this issue Nov 18, 2021 · 8 comments
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@iammukeshm
Copy link
Member

Guid is a secure and reliable data type. But there can be instances where performance can be a bit down due to lot of Guids in DBs.
To combat this, we can make the BaseEntity to BaseEntity where T can be simple ints as well.

Someone can take this up and raise a PR as well.

Things to note.
Ensure new migrations are built.
Test every other scenario as well.

@iammukeshm iammukeshm added enhancement New feature or request help wanted Extra attention is needed labels Nov 18, 2021
@iammukeshm iammukeshm added this to the Release 2.0 milestone Nov 18, 2021
@AdelSS04
Copy link

AdelSS04 commented Dec 5, 2021

i think the main problem here is at Repository level :)

@pashaie
Copy link
Contributor

pashaie commented Dec 5, 2021

I've tested PR and there is no apparent problem in code behavior.
Please check it out

@pashaie
Copy link
Contributor

pashaie commented Dec 12, 2021

I've done some work to support generic Key type in this project, but imho the result wasn't good.
the repo methods needs to be changed to accept TKey and client code must provide TKey type in everywhere because C# Method type inference ignores generic constraints on the method type parameters
Other solution that I was considering was to provide multiple RepositoryAsync for example RepositoryIntAsync or RepositoryGuidAsync , but that would be massive code duplicate and also bypassing UOW

So I'm wondering if anyone has a better idea?
btw we can also consider ULID instead of GUID for primary keys:

Universally Unique Lexicographically Sortable Identifier

A GUID/UUID can be suboptimal for many use-cases because:

  • It isn't the most character efficient way of encoding 128 bits
  • It provides no other information than randomness

A ULID however:

  • Is compatible with UUID/GUID's
  • 1.21e+24 unique ULIDs per millisecond (1,208,925,819,614,629,174,706,176 to be exact)
  • Lexicographically sortable
  • Canonically encoded as a 26 character string, as opposed to the 36 character UUID
  • Uses Crockford's base32 for better efficiency and readability (5 bits per character)
  • Case insensitive
  • No special characters (URL safe)

@iammukeshm
Copy link
Member Author

https://github.com/blazorhero/CleanArchitecture/blob/master/src/Domain/Entities/Catalog/Brand.cs

We have developed more or less the same requirement for BlazorHero as well. Can you please check it.

@pashaie
Copy link
Contributor

pashaie commented Dec 12, 2021

@iammukeshm thanks.
the problem arise in RepositoryAsync:

  • equality checks like e.Id == entityId will cause Operator '==' cannot be applied to operands of type 'TKey' and 'TKey' because of TKey is not constrained. we can constrain TKey with where TKey : struct but string and/or guid are not struct and int is not class. the solution that I'm considering is to provide overloaded method for type int and guid. but that's tricky.
  • methods like GetByIdAsync<T> needs to be changed to GetByIdAsync<T, TKey> and client code needs to supply each of this types. for example code like GetByIdAsync(product) needs to be changed to GetByIdAsync<Product, Guid>(product)

@imnikitaokunev
Copy link

  • equality checks like e.Id == entityId will cause Operator '==' cannot be applied to operands of type 'TKey' and 'TKey'
    Can we use Equals() method instead of == operator?

@pashaie
Copy link
Contributor

pashaie commented Dec 12, 2021

@iammukeshm I'm not quite sure. because predicate will be translated to sql, I suspect that Equals can't be translated.
can you check it out?

pashaie added a commit to pashaie/dotnet-webapi-boilerplate that referenced this issue Dec 12, 2021
@pashaie
Copy link
Contributor

pashaie commented Dec 12, 2021

@imnikitaokunev Equals() works, tnx!

pashaie added a commit to pashaie/dotnet-webapi-boilerplate that referenced this issue Dec 12, 2021
pashaie added a commit to pashaie/dotnet-webapi-boilerplate that referenced this issue Dec 28, 2021
pashaie added a commit to pashaie/dotnet-webapi-boilerplate that referenced this issue Jan 5, 2022
pashaie added a commit to pashaie/dotnet-webapi-boilerplate that referenced this issue Jan 6, 2022
pashaie added a commit to pashaie/dotnet-webapi-boilerplate that referenced this issue Jan 8, 2022
# Conflicts:
#	src/Core/Application/Application.csproj
#	src/Infrastructure/Catalog/BrandGeneratorJob.cs
pashaie added a commit to pashaie/dotnet-webapi-boilerplate that referenced this issue Jan 9, 2022
# Conflicts:
#	src/Core/Application/Application.csproj
#	src/Core/Application/Common/Persistence/IRepositoryAsync.cs
#	src/Core/Domain/Domain.csproj
#	src/Infrastructure/Infrastructure.csproj
#	src/Infrastructure/Persistence/RepositoryAsync.cs
iammukeshm added a commit that referenced this issue Jan 9, 2022
#172 - Support Multiple Key Types for Base Entity
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

4 participants