diff --git a/commafeed-server/src/main/java/com/commafeed/backend/dao/UnitOfWork.java b/commafeed-server/src/main/java/com/commafeed/backend/dao/UnitOfWork.java index 90d6d8b9..82bc62f0 100644 --- a/commafeed-server/src/main/java/com/commafeed/backend/dao/UnitOfWork.java +++ b/commafeed-server/src/main/java/com/commafeed/backend/dao/UnitOfWork.java @@ -15,39 +15,40 @@ public class UnitOfWork { } public static T call(SessionFactory sessionFactory, SessionRunnerReturningValue sessionRunner) { - final Session session = sessionFactory.openSession(); - if (ManagedSessionContext.hasBind(sessionFactory)) { - throw new IllegalStateException("Already in a unit of work!"); - } T t = null; - try { - ManagedSessionContext.bind(session); - session.beginTransaction(); + + boolean sessionAlreadyBound = ManagedSessionContext.hasBind(sessionFactory); + try (Session session = sessionFactory.openSession()) { + if (!sessionAlreadyBound) { + ManagedSessionContext.bind(session); + } + + Transaction tx = session.beginTransaction(); try { t = sessionRunner.runInSession(); - commitTransaction(session); + commitTransaction(tx); } catch (Exception e) { - rollbackTransaction(session); - UnitOfWork. rethrow(e); + rollbackTransaction(tx); + UnitOfWork.rethrow(e); } } finally { - session.close(); - ManagedSessionContext.unbind(sessionFactory); + if (!sessionAlreadyBound) { + ManagedSessionContext.unbind(sessionFactory); + } } + return t; } - private static void rollbackTransaction(Session session) { - final Transaction txn = session.getTransaction(); - if (txn != null && txn.isActive()) { - txn.rollback(); + private static void rollbackTransaction(Transaction tx) { + if (tx != null && tx.isActive()) { + tx.rollback(); } } - private static void commitTransaction(Session session) { - final Transaction txn = session.getTransaction(); - if (txn != null && txn.isActive()) { - txn.commit(); + private static void commitTransaction(Transaction tx) { + if (tx != null && tx.isActive()) { + tx.commit(); } } diff --git a/commafeed-server/src/main/java/com/commafeed/backend/service/internal/PostLoginActivities.java b/commafeed-server/src/main/java/com/commafeed/backend/service/internal/PostLoginActivities.java index 62cb08ff..b4b91547 100644 --- a/commafeed-server/src/main/java/com/commafeed/backend/service/internal/PostLoginActivities.java +++ b/commafeed-server/src/main/java/com/commafeed/backend/service/internal/PostLoginActivities.java @@ -6,8 +6,10 @@ import javax.inject.Inject; import javax.inject.Singleton; import org.apache.commons.lang3.time.DateUtils; +import org.hibernate.SessionFactory; import com.commafeed.CommaFeedConfiguration; +import com.commafeed.backend.dao.UnitOfWork; import com.commafeed.backend.dao.UserDAO; import com.commafeed.backend.model.User; import com.commafeed.backend.service.FeedSubscriptionService; @@ -20,6 +22,7 @@ public class PostLoginActivities { private final UserDAO userDAO; private final FeedSubscriptionService feedSubscriptionService; + private final SessionFactory sessionFactory; private final CommaFeedConfiguration config; public void executeFor(User user) { @@ -27,19 +30,26 @@ public class PostLoginActivities { Date now = new Date(); boolean saveUser = false; + // only update lastLogin field every hour in order to not // invalidate the cache every time someone logs in if (lastLogin == null || lastLogin.before(DateUtils.addHours(now, -1))) { user.setLastLogin(now); saveUser = true; } - if (config.getApplicationSettings().getHeavyLoad() && user.shouldRefreshFeedsAt(now)) { + + if (Boolean.TRUE.equals(config.getApplicationSettings().getHeavyLoad()) && user.shouldRefreshFeedsAt(now)) { feedSubscriptionService.refreshAll(user); user.setLastFullRefresh(now); saveUser = true; } + if (saveUser) { - userDAO.saveOrUpdate(user); + // Post login activites are susceptible to run for any webservice call. + // We update the user in a new transaction to update the user immediately. + // If we didn't and the webservice call takes time, subsequent webservice calls would have to wait for the first call to + // finish even if they didn't use the same database tables, because they updated the user too. + UnitOfWork.run(sessionFactory, () -> userDAO.saveOrUpdate(user)); } }