Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

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

Thread safety using different Dialects? #155

Closed
scott-mac opened this issue Feb 24, 2022 · 6 comments
Closed

Thread safety using different Dialects? #155

scott-mac opened this issue Feb 24, 2022 · 6 comments

Comments

@scott-mac
Copy link

scott-mac commented Feb 24, 2022

This is more of a question than an issue, but hopefully you can confirm for me.

In past projects I have used SimpleCrud and it worked fine for me since I was always working against a single DB type (usually SQL Server). But recently I started working on some code for a client with multiple different databases types (MySQL, Oracle etc) and they need to sync data between them (imagine you have an Employee table in MySQL and you need to sync it to a duplicate Employee table in SQL Server). When I looked at how SimpleCrud handled the dialect for constructing the SQL i realized I was in trouble because he has that logic all defined in static variables so calling SetDialect is going to change it across all instances and cause issues

At first glance, FastCrud appeared to suffer the same issue as it uses OrmConfiguration with a static DefaultDialect but after digging some more it looks like that's a default behavior if you're using just a single db type and you've worked around that by having your CRUD methods take an optional EntityMapping where you can define the dialect for each call. Am I understanding this correctly?

Just trying to think of the best way to code this? It seems like creating a new mapping from scratch with the dialect I want every time I invoke a CRUD method would be adding unnecessary overhead to every call. Would it make sense to have a service that runs once on Startup to populate all these mappings for all db types into a singleton that can be injected anywhere I need it?

Thanks very much for your time, I'm liking the look of this package a lot, I think I'll be migrating over to it :)

@MoonStorm
Copy link
Owner

Thread safety should not be a problem in this case, but multi-db work is always gonna be challenging. All our operations do allow for an override of the entity registration (and that includes dialect as well) at the point of making the call, however we don't have a single test (automatic or manual) with a multi dialect scenario. So my suggestion is to try it out and see if it works for you. Bear in mind there is no reason to chose one library over another. You can even use both, as long as you get your work done.

@scott-mac
Copy link
Author

scott-mac commented Feb 24, 2022

Excellent, I think this will prove a good solution for me then, thank you.

One snag I hit in implementing this is that I use a lot of generics where the type is unknown until invoked at runtime. So imagine a repository pattern that looks like this (for brevity I'll just include a couple of the GET methods)

public interface IRepository<T> {
    T GetByPK(object pks) ;
    T Get(object whereClause) ;
}

And then to call them, you'd have something like this:

//single record
var result = repo.GetByPK(new {SomePK =123, AnotherPK=456});
//e.g. SELECT * FROM T WHERE SomePK=123 AND AnotherPK=456
//where 'T' is the runtime type which maps to the table in the db

//multiple records
var results = repo.Get(new {DeptId = 1, CategoryId=2, Name='FastCrud'});

That type T is getting passed all the way through in this scenario so I never explicitly define what it actually is at design time.

Any ideas on how I might tap into your framework to make this possible for the cases where I don't know the type so I'm not able to do this kind of thing?:

conn.Get(new ConcreteType(){ConcreteID=123});

@MoonStorm
Copy link
Owner

MoonStorm commented Feb 24, 2022

In your case you'd have T GetByPk(T objWithPrimaryKeys) or T[] Find(FormattableString whereClause). The idea is to pass on the generic type from your IRepository to FastCrud's or Dapper's methods, which are also generic.

I personally don't recommend a common generic repository pattern. Simply because of the variation in the database entities and the way you may be using them later in the business layer. Their primary keys might be different, might be composite, sometimes you need just the entity itself, sometimes you need them in association with other entities through the navigation properties etc. etc. So what I'm trying to suggest here is having standalone repositories, all having a form matching the operations you're gonna need on the entity they're supposed to work with. For example a UserRepository might have clear methods such as GetUserById, GetUserByEmailAddress, UpdatePassword, GetUserWithRights etc. That way you also encapsulate the logic used to query/update/delete them while also not allowing that to spill onto your other layers.

@scott-mac
Copy link
Author

I personally don't recommend a common generic repository pattern. Simply because of the variation in the database entities and the way you may be using them later in the business layer.

In general I agree, it is somewhat limited in it's use. But in my case it simply serves as a set of basic functions for my data layer, it then has hooks in that allow you to easily add on additional custom functionality just like you were saying. I've found a hybrid approach like this actually works really well.

in your case you'd have T GetByPk(T objWithPrimaryKeys) or T[] Find(FormattableString whereClause). The idea is to pass on the generic type from your IRepository to FastCrud's or Dapper's methods, which are also generic.

Yep, that is exactly how I am doing it for most of the methods, it was just some of these Gets that I have that I couldn't figure out how to mimic, sorry, I phrased my question poorly and got off into the weeds when I should have been looking at how your code works a little more ;)

But this:

var builder = OrmConfiguration.GetSqlBuilder<T>();

will allow me to do everything I need, that was the part I was missing, how to tap into the logic that does all the scaffolding for the generic type. I'm good to go now, thanks for your patience and time, I owe you a beer, cheers!

@scott-mac
Copy link
Author

EDIT: One thing though, what I really need is access to ConstructFullSingleSelectStatement which is part of the internal class GenericStatementSqlBuilder. Since all SQLBuilders inherit from that abstract base shouldn't this method (and the others of the same type) be defined as part of that ISqlBuilder interface so they can be invoked on whatever implementation builder at runtime? Thanks.

@MoonStorm
Copy link
Owner

Not really. The ISqlBuilder interface was created with the intent to indeed expose some of the functionality of the internal sql builders. But in the last version we haven't even documented that interface in the wiki since we're debating whether we should deprecate it completely and perhaps instead expand the format specifiers, which the library promotes heavily. As we kept using them, they kinda grew on us over time. People seem to like them and came up with ideas to expand them.

Repository owner locked and limited conversation to collaborators Mar 3, 2022
@MoonStorm MoonStorm converted this issue into discussion #160 Mar 3, 2022

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants