-
Notifications
You must be signed in to change notification settings - Fork 13
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
NHibernate.ISession still open when using signalr #28
Comments
Returning an IQueryable from within a "using session" doesn't work. What is the broader use case? |
It works, but as I said, the fact that because it's queryable, my example is not safe and shouldn't be done, I'm aware of that, but I don't see any other options. The broader use case is just an classique of SignalR. Would you like an example git repo if I'm not clear ? |
I don't see how it could work. The session is closed when the IQueryable is returned. |
Don't get stuck on the Users property, it's not used in my implementation, it's just that Identity requires its implemation. It only works because Users is never called, but my other methods are implemented like this : public override async Task<IdentityResult> CreateAsync(User user, CancellationToken cancellationToken = new CancellationToken()) {
cancellationToken.ThrowIfCancellationRequested();
ThrowIfDisposed();
if (user == null) {
throw new ArgumentNullException(nameof(user));
}
using(ISession session = _sessionFactory.OpenSession()) {
await session.SaveAsync(user, cancellationToken);
await FlushChangesAsync(session, cancellationToken);
}
return IdentityResult.Success;
}
public override async Task<IdentityResult> UpdateAsync(User user, CancellationToken cancellationToken = new CancellationToken()) {
cancellationToken.ThrowIfCancellationRequested();
ThrowIfDisposed();
if (user == null) {
throw new ArgumentNullException(nameof(user));
}
using(ISession session = _sessionFactory.OpenSession()) {
var exists = await session.Query<User> ().AnyAsync(
u => u.Id.Equals(user.Id),
cancellationToken
);
if (!exists) {
return IdentityResult.Failed(
new IdentityError {
Code = "UserNotExist",
Description = $ "User with id {user.Id} does not exists!"
}
);
}
user.ConcurrencyStamp = Guid.NewGuid().ToString("N");
await session.MergeAsync(user, cancellationToken);
await FlushChangesAsync(session, cancellationToken);
return IdentityResult.Success;
}
}
public override async Task<IdentityResult> DeleteAsync(User user, CancellationToken cancellationToken = new CancellationToken()) {
cancellationToken.ThrowIfCancellationRequested();
ThrowIfDisposed();
if (user == null) {
throw new ArgumentNullException(nameof(user));
}
using(ISession session = _sessionFactory.OpenSession()) {
await session.DeleteAsync(user, cancellationToken);
await FlushChangesAsync(session, cancellationToken);
return IdentityResult.Success;
}
}
public override async Task<User?> FindByIdAsync(string userId, CancellationToken cancellationToken = new CancellationToken()) {
cancellationToken.ThrowIfCancellationRequested();
ThrowIfDisposed();
var id = ConvertIdFromString(userId);
using(ISession session = _sessionFactory.OpenSession()) {
var user = await session.GetAsync<User> (id, cancellationToken);
return user;
}
} |
That looks like a perfectly viable solution |
It should work except if try to play with a user getting by FindByIdAsync function for example ... |
If you are using SignalR Hubs to send message, please follow the rules of hubs https://learn.microsoft.com/en-us/aspnet/core/signalr/hubs?view=aspnetcore-7.0#create-and-use-hubs Please Note Hubs are transient:
I think the safest useage should be like this: public class MessageHub : Hub {
public async Task SendMessage(string user, string message) {
var factory = this.Context.GetHttpContext().RequestServices.GetService<ISessionFactory>();
using var session = factory.OpenSession();
var users = await session.Query<Users>().ToListAsync();
await Clients.All.SendAsync("ReceiveMessage", user, users);
}
} Or try inject public class MessageHub : Hub {
private readonly IServiceProvider serviceProvider;
public MessageHub(IServiceProvider serviceProvider) {
this.serviceProvider = serviceProvider ?? throw new ArgumentNullException(nameof(serviceProvider));
}
public async Task SendMessage(string user, string message) {
var sessionFactory = serviceProvider.GetService<ISessionFactory>();
using var session = sessionFactory.OpenSession();
await Clients.All.SendAsync("ReceiveMessage", user, message);
}
} Not fully tested, just a suggestion; |
Thank you for your advice but i don't use any injection or session inside my hubs. Hubs are clean as you defined the transiant. The problem is just that: Hub is automatically connected with identity and the session injected into the UserStore constructor stay open for the entire life of the application. So Each Hubs results in an additional open session. |
By default If this is not suitable for SignalR, you can register them with another life time. And this is easy, just dont use the default extension method AddHibernateStores , register them with another lifetime which are suiteable for SignalR. PS: Any PR is welcome! |
I integrated SignalR into my app some time after using your ISession injection implementation with Identity.
The problem is that signalr is connected with identity and ISession's injected in the identity implementation stay open.
So each Hub from SignalR Keep in Context identity and injected sessions are never kill.
So one session stay open for each Hub.
I was thinking to inject ISessionFactory in identity and open the sessions in the implemented methods for kills Sessions at each end of action but I do not know if this method is suitable, Knowing that the implementation of Users in UserStore is IQueryable so this would not be very safe to do something like:
what do you think about this case ?
Thank for your times.
The text was updated successfully, but these errors were encountered: