From c2d04047f7c169d24c7b220eccdb22843a0d3668 Mon Sep 17 00:00:00 2001 From: srosse <none@none> Date: Mon, 8 Oct 2012 16:14:43 +0200 Subject: [PATCH] OO-373: made the notifications manager robust under concurrent access, write more unit tests to tests the queries on the database and test it under concurrent load --- .../assessment/AssessmentMainController.java | 20 - .../course/nodes/den/DENRunController.java | 2 +- .../calendar/ui/WeeklyCalendarController.java | 4 +- .../InfoSubscriptionManagerImpl.java | 2 +- .../modules/bc/commands/CmdMoveCopy.java | 2 +- .../modules/bc/commands/CmdUpload.java | 2 +- .../core/util/mail/manager/MailManager.java | 6 +- .../notifications/NotificationsHandler.java | 2 +- .../notifications/NotificationsManager.java | 9 +- .../core/util/servlets/VFSDirContext.java | 2 +- .../assessment/AssessmentMainController.java | 19 - .../AssessmentNotificationsHandler.java | 6 +- .../course/nodes/ta/DropboxController.java | 2 +- .../manager/BusinessGroupServiceImpl.java | 5 +- .../dialog/DialogElementsController.java | 10 +- .../org/olat/modules/fo/ForumController.java | 6 +- .../olat/modules/wiki/WikiMainController.java | 4 +- .../NotificationsManagerImpl.java | 450 ++++++------ .../NotificationsPortletRunController.java | 2 +- .../org/olat/notifications/PublisherImpl.java | 21 +- .../olat/notifications/SubscriberImpl.java | 22 +- .../UsersSubscriptionManagerImpl.java | 2 +- .../NotificationsManagerTest.java | 646 ++++++++++++++---- .../org/olat/restapi/NotificationsTest.java | 10 +- src/test/java/org/olat/test/OlatTestCase.java | 6 + 25 files changed, 810 insertions(+), 452 deletions(-) diff --git a/src/main/java/de/bps/course/assessment/AssessmentMainController.java b/src/main/java/de/bps/course/assessment/AssessmentMainController.java index 8b935909c86..f6994837a16 100644 --- a/src/main/java/de/bps/course/assessment/AssessmentMainController.java +++ b/src/main/java/de/bps/course/assessment/AssessmentMainController.java @@ -685,26 +685,6 @@ AssessmentMainController(UserRequest ureq, WindowControl wControl, StackedContro columLayoutCtr.setCol2(toolC.getInitialComponent()); } } - - - /** - * Notify subscribers when test are passed or attemps count change - * EXPERIMENTAL!!!!! - */ - /*private void doNotifyAssessmentEvent(AssessmentChangedEvent ace) { - String assessmentChangeType = ace.getCommand(); - // notify only comment has been changed - if (assessmentChangeType == AssessmentChangedEvent.TYPE_PASSED_CHANGED - || assessmentChangeType == AssessmentChangedEvent.TYPE_ATTEMPTS_CHANGED) - { - // if notification is enabled -> notify the publisher about news - if (subsContext != null) - { - NotificationsManagerImpl.getInstance().markPublisherNews(subsContext, ace.getIdentity()); - } - } - }*/ - /** * Updates the local user course environment cache if the given event is for an identity diff --git a/src/main/java/de/bps/course/nodes/den/DENRunController.java b/src/main/java/de/bps/course/nodes/den/DENRunController.java index 2bb8ba6174a..3445eaa63cb 100644 --- a/src/main/java/de/bps/course/nodes/den/DENRunController.java +++ b/src/main/java/de/bps/course/nodes/den/DENRunController.java @@ -208,7 +208,7 @@ public class DENRunController extends BasicController implements GenericEventLis runDENTable.setTableDataModel(runTableData); fireEvent(ureq, Event.DONE_EVENT); // inform subscription context about changes - NotificationsManager.getInstance().markPublisherNews(subsContext, ureq.getIdentity()); + NotificationsManager.getInstance().markPublisherNews(subsContext, ureq.getIdentity(), true); // </OPAL-122> } } else if(authorOptions == source) { diff --git a/src/main/java/org/olat/commons/calendar/ui/WeeklyCalendarController.java b/src/main/java/org/olat/commons/calendar/ui/WeeklyCalendarController.java index 568ec9787bf..c95d36cb5d3 100644 --- a/src/main/java/org/olat/commons/calendar/ui/WeeklyCalendarController.java +++ b/src/main/java/org/olat/commons/calendar/ui/WeeklyCalendarController.java @@ -523,13 +523,13 @@ public class WeeklyCalendarController extends BasicController implements Activat if (weeklyCalendar.isDirty()) { if (subsContext != null) { // group or course calendar -> prepared subscription context is the right one - NotificationsManager.getInstance().markPublisherNews(subsContext, ureq.getIdentity()); + NotificationsManager.getInstance().markPublisherNews(subsContext, ureq.getIdentity(), true); } else if(caller.equals(CALLER_HOME) && affectedCal != null) { // one can add/edit/remove dates of group and course calendars from the home calendar view -> choose right subscription context for( KalendarRenderWrapper calWrapper : calendarWrappers) { if(affectedCal == calWrapper.getKalendar()) { SubscriptionContext tmpSubsContext = calendarNotificationsManager.getSubscriptionContext(calWrapper); - NotificationsManager.getInstance().markPublisherNews(tmpSubsContext, ureq.getIdentity()); + NotificationsManager.getInstance().markPublisherNews(tmpSubsContext, ureq.getIdentity(), true); } } } diff --git a/src/main/java/org/olat/commons/info/notification/InfoSubscriptionManagerImpl.java b/src/main/java/org/olat/commons/info/notification/InfoSubscriptionManagerImpl.java index 30d76bf5138..b0e81e5a791 100644 --- a/src/main/java/org/olat/commons/info/notification/InfoSubscriptionManagerImpl.java +++ b/src/main/java/org/olat/commons/info/notification/InfoSubscriptionManagerImpl.java @@ -120,6 +120,6 @@ public class InfoSubscriptionManagerImpl extends InfoSubscriptionManager { @Override public void markPublisherNews(OLATResourceable resource, String subPath) { SubscriptionContext context = getInfoSubscriptionContext(resource, subPath); - notificationsManager.markPublisherNews(context, null); + notificationsManager.markPublisherNews(context, null, true); } } diff --git a/src/main/java/org/olat/core/commons/modules/bc/commands/CmdMoveCopy.java b/src/main/java/org/olat/core/commons/modules/bc/commands/CmdMoveCopy.java index 9cb50d08e83..a1a01f41634 100644 --- a/src/main/java/org/olat/core/commons/modules/bc/commands/CmdMoveCopy.java +++ b/src/main/java/org/olat/core/commons/modules/bc/commands/CmdMoveCopy.java @@ -181,7 +181,7 @@ public class CmdMoveCopy extends DefaultController implements FolderCommand { if (secCallback != null) { SubscriptionContext subsContext = secCallback.getSubscriptionContext(); if (subsContext != null) { - NotificationsManager.getInstance().markPublisherNews(subsContext, ureq.getIdentity()); + NotificationsManager.getInstance().markPublisherNews(subsContext, ureq.getIdentity(), true); } } fireEvent(ureq, new FolderEvent(move ? FolderEvent.MOVE_EVENT : FolderEvent.COPY_EVENT, fileSelection.renderAsHtml())); diff --git a/src/main/java/org/olat/core/commons/modules/bc/commands/CmdUpload.java b/src/main/java/org/olat/core/commons/modules/bc/commands/CmdUpload.java index 5217eb994e6..acf3e5d9bef 100644 --- a/src/main/java/org/olat/core/commons/modules/bc/commands/CmdUpload.java +++ b/src/main/java/org/olat/core/commons/modules/bc/commands/CmdUpload.java @@ -229,7 +229,7 @@ public class CmdUpload extends BasicController implements FolderCommand { if (secCallback != null) { SubscriptionContext subsContext = secCallback.getSubscriptionContext(); if (subsContext != null) { - NotificationsManager.getInstance().markPublisherNews(subsContext, ureq.getIdentity()); + NotificationsManager.getInstance().markPublisherNews(subsContext, ureq.getIdentity(), true); } } // Notify everybody diff --git a/src/main/java/org/olat/core/util/mail/manager/MailManager.java b/src/main/java/org/olat/core/util/mail/manager/MailManager.java index a364141a555..a86147b3a72 100644 --- a/src/main/java/org/olat/core/util/mail/manager/MailManager.java +++ b/src/main/java/org/olat/core/util/mail/manager/MailManager.java @@ -692,11 +692,7 @@ public class MailManager extends BasicManager { } SubscriptionContext subContext = getSubscriptionContext(); - Publisher publisher = NotificationsManager.getInstance().getPublisher(subContext); - if(publisher != null && publisher.getKey() != null) { - publisher.setLatestNewsDate(new Date()); - notificationsManager.updatePublisher(publisher); - } + notificationsManager.markPublisherNews(subContext, null, false); return mail; } catch (AddressException e) { logError("Cannot send e-mail: ", e); diff --git a/src/main/java/org/olat/core/util/notifications/NotificationsHandler.java b/src/main/java/org/olat/core/util/notifications/NotificationsHandler.java index 05dce4298c9..1bb3ea396c6 100644 --- a/src/main/java/org/olat/core/util/notifications/NotificationsHandler.java +++ b/src/main/java/org/olat/core/util/notifications/NotificationsHandler.java @@ -32,7 +32,7 @@ import java.util.Locale; /** * Description:<br> - * NotificationsHandler is stateless and represents a starting point for all subscription activities for a given type, see NotificationsManagerImpl.java + * NotificationsHandler is stateless and represents a starting point for all subscription activities for a given type, see NotificationsManager.java * * <P> * Initial Date: 25.10.2004 <br> diff --git a/src/main/java/org/olat/core/util/notifications/NotificationsManager.java b/src/main/java/org/olat/core/util/notifications/NotificationsManager.java index ac8431d5976..34035299518 100644 --- a/src/main/java/org/olat/core/util/notifications/NotificationsManager.java +++ b/src/main/java/org/olat/core/util/notifications/NotificationsManager.java @@ -145,11 +145,6 @@ public abstract class NotificationsManager extends BasicManager { */ public abstract List<Identity> getSubscriberIdentities(Publisher publisher); - /** - * @param publisher - */ - public abstract void updatePublisher(Publisher publisher); - /** * sets the latest visited date of the subscription to 'now' .assumes the * identity is already subscribed to the publisher @@ -166,7 +161,7 @@ public abstract class NotificationsManager extends BasicManager { * @param subscriptionContext * @param ignoreNewsFor */ - public abstract void markPublisherNews(SubscriptionContext subscriptionContext, Identity ignoreNewsFor); + public abstract void markPublisherNews(SubscriptionContext subscriptionContext, Identity ignoreNewsFor, boolean sendEvent); public abstract void registerAsListener(GenericEventListener gel, Identity ident); @@ -226,7 +221,7 @@ public abstract class NotificationsManager extends BasicManager { * @param subscriptionContext * @return true when the subscriptionContext refers to the publisher p */ - public abstract boolean matches(Publisher p, SubscriptionContext subscriptionContext); + //public abstract boolean matches(Publisher p, SubscriptionContext subscriptionContext); /** * @param subscriber diff --git a/src/main/java/org/olat/core/util/servlets/VFSDirContext.java b/src/main/java/org/olat/core/util/servlets/VFSDirContext.java index c777850edf8..979078041d4 100644 --- a/src/main/java/org/olat/core/util/servlets/VFSDirContext.java +++ b/src/main/java/org/olat/core/util/servlets/VFSDirContext.java @@ -583,7 +583,7 @@ public class VFSDirContext extends BaseDirContext { VFSSecurityCallback callback = folder.getLocalSecurityCallback(); if(callback != null && callback.getSubscriptionContext() != null) { SubscriptionContext subContext = callback.getSubscriptionContext(); - NotificationsManager.getInstance().markPublisherNews(subContext, null); + NotificationsManager.getInstance().markPublisherNews(subContext, null, true); } if(childLeaf instanceof MetaTagged) { diff --git a/src/main/java/org/olat/course/assessment/AssessmentMainController.java b/src/main/java/org/olat/course/assessment/AssessmentMainController.java index 62c8b8882fb..338ef9ed1da 100644 --- a/src/main/java/org/olat/course/assessment/AssessmentMainController.java +++ b/src/main/java/org/olat/course/assessment/AssessmentMainController.java @@ -493,25 +493,6 @@ AssessmentMainController(UserRequest ureq, WindowControl wControl, StackedContro } } - /** - * Notify subscribers when test are passed or attemps count change - * EXPERIMENTAL!!!!! - */ - /*private void doNotifyAssessmentEvent(AssessmentChangedEvent ace) { - String assessmentChangeType = ace.getCommand(); - // notify only comment has been changed - if (assessmentChangeType == AssessmentChangedEvent.TYPE_PASSED_CHANGED - || assessmentChangeType == AssessmentChangedEvent.TYPE_ATTEMPTS_CHANGED) - { - // if notification is enabled -> notify the publisher about news - if (subsContext != null) - { - NotificationsManagerImpl.getInstance().markPublisherNews(subsContext, ace.getIdentity()); - } - } - }*/ - - /** * Updates the local user course environment cache if the given event is for an identity * cached in the local cache. Also updates the user list table model if the identity from diff --git a/src/main/java/org/olat/course/assessment/AssessmentNotificationsHandler.java b/src/main/java/org/olat/course/assessment/AssessmentNotificationsHandler.java index 57983f5cdf4..f4c756e2275 100644 --- a/src/main/java/org/olat/course/assessment/AssessmentNotificationsHandler.java +++ b/src/main/java/org/olat/course/assessment/AssessmentNotificationsHandler.java @@ -176,7 +176,7 @@ public class AssessmentNotificationsHandler implements NotificationsHandler { } /** - * Signal the <code>NotificationsManagerImpl</code> about assessment news + * Signal the <code>NotificationsManager</code> about assessment news * available for a course.<br> * <br> * <b>PRE CONDITIONS</b> @@ -194,7 +194,7 @@ public class AssessmentNotificationsHandler implements NotificationsHandler { } /** - * Signal the <code>NotificationsManagerImpl</code> about assessment news + * Signal the <code>NotificationsManager</code> about assessment news * available on a course.<br> * <br> * <b>PRE CONDITIONS</b> @@ -208,7 +208,7 @@ public class AssessmentNotificationsHandler implements NotificationsHandler { private void markPublisherNews(Identity ident, ICourse course) { SubscriptionContext subsContext = getAssessmentSubscriptionContext(course); if (subsContext != null) { - NotificationsManager.getInstance().markPublisherNews(subsContext, ident); + NotificationsManager.getInstance().markPublisherNews(subsContext, ident, true); } } diff --git a/src/main/java/org/olat/course/nodes/ta/DropboxController.java b/src/main/java/org/olat/course/nodes/ta/DropboxController.java index c3b7729e38f..87499c56265 100644 --- a/src/main/java/org/olat/course/nodes/ta/DropboxController.java +++ b/src/main/java/org/olat/course/nodes/ta/DropboxController.java @@ -315,7 +315,7 @@ public class DropboxController extends BasicController { subsContext = DropboxFileUploadNotificationHandler.getSubscriptionContext(userCourseEnv, node); // inform subscription manager about new element if (subsContext != null) { - NotificationsManager.getInstance().markPublisherNews(subsContext, ureq.getIdentity()); + NotificationsManager.getInstance().markPublisherNews(subsContext, ureq.getIdentity(), true); } // configuration is already translated, don't use showInfo(i18nKey)! //FIXME:FG:6.2: fix problem in info message, not here diff --git a/src/main/java/org/olat/group/manager/BusinessGroupServiceImpl.java b/src/main/java/org/olat/group/manager/BusinessGroupServiceImpl.java index d32fef5cc65..fd299c6c41c 100644 --- a/src/main/java/org/olat/group/manager/BusinessGroupServiceImpl.java +++ b/src/main/java/org/olat/group/manager/BusinessGroupServiceImpl.java @@ -95,7 +95,6 @@ import org.olat.group.ui.edit.BusinessGroupModifiedEvent; import org.olat.instantMessaging.IMConfigSync; import org.olat.instantMessaging.InstantMessagingModule; import org.olat.instantMessaging.syncservice.SyncUserListTask; -import org.olat.notifications.NotificationsManagerImpl; import org.olat.properties.Property; import org.olat.repository.RepositoryEntry; import org.olat.repository.RepositoryEntryShort; @@ -137,6 +136,8 @@ public class BusinessGroupServiceImpl implements BusinessGroupService, UserDataD @Autowired private UserDeletionManager userDeletionManager; @Autowired + private NotificationsManager notificationsManager; + @Autowired private ACService acService; @Autowired private DB dbInstance; @@ -762,7 +763,7 @@ public class BusinessGroupServiceImpl implements BusinessGroupService, UserDataD // delete the publisher attached to this group (e.g. the forum and folder // publisher) - NotificationsManagerImpl.getInstance().deletePublishersOf(group); + notificationsManager.deletePublishersOf(group); // delete potential jabber group roster if (InstantMessagingModule.isEnabled()) { diff --git a/src/main/java/org/olat/modules/dialog/DialogElementsController.java b/src/main/java/org/olat/modules/dialog/DialogElementsController.java index d26ba9a889e..37acc51d88c 100644 --- a/src/main/java/org/olat/modules/dialog/DialogElementsController.java +++ b/src/main/java/org/olat/modules/dialog/DialogElementsController.java @@ -187,11 +187,11 @@ public class DialogElementsController extends BasicController { removeAsListenerAndDispose(tableCtr); tableCtr = new TableController(tableConf, ureq, getWindowControl(), getTranslator()); DialogPropertyElements elements = dialogElmsMgr.findDialogElements(coursePropMgr, courseNode); - List list = new ArrayList(); + List<DialogElement> list = new ArrayList<DialogElement>(); tableModel = new DialogElementsTableModel(getTranslator(), callback, courseNode.getModuleConfiguration()); if (elements != null) list = elements.getDialogPropertyElements(); - for (Iterator iter = list.iterator(); iter.hasNext();) { - DialogElement element = (DialogElement) iter.next(); + for (Iterator<DialogElement> iter = list.iterator(); iter.hasNext();) { + DialogElement element = iter.next(); Integer msgCount = forumMgr.countMessagesByForumID(element.getForumKey()); element.setMessagesCount(msgCount); element.setNewMessages(new Integer(msgCount.intValue() @@ -287,7 +287,7 @@ public class DialogElementsController extends BasicController { // inform subscription manager about new element if (subsContext != null) { - NotificationsManager.getInstance().markPublisherNews(subsContext, ureq.getIdentity()); + NotificationsManager.getInstance().markPublisherNews(subsContext, ureq.getIdentity(), true); } //everything when well so save the property dialogElmsMgr.addDialogElement(coursePropMgr, courseNode, element); @@ -330,7 +330,7 @@ public class DialogElementsController extends BasicController { // inform subscription manager about new element if (subsContext != null) { - NotificationsManager.getInstance().markPublisherNews(subsContext, ureq.getIdentity()); + NotificationsManager.getInstance().markPublisherNews(subsContext, ureq.getIdentity(), true); } //everything when well so save the property dialogElmsMgr.addDialogElement(coursePropMgr, courseNode, element); diff --git a/src/main/java/org/olat/modules/fo/ForumController.java b/src/main/java/org/olat/modules/fo/ForumController.java index ef6278d5b17..d81cb747e76 100644 --- a/src/main/java/org/olat/modules/fo/ForumController.java +++ b/src/main/java/org/olat/modules/fo/ForumController.java @@ -589,7 +589,7 @@ public class ForumController extends BasicController implements GenericEventList fm.updateMessage(currentMsg, changeLastModifiedDate, null); // if notification is enabled -> notify the publisher about news if (subsContext != null) { - NotificationsManager.getInstance().markPublisherNews(subsContext, ureq.getIdentity()); + NotificationsManager.getInstance().markPublisherNews(subsContext, ureq.getIdentity(), true); } // do logging @@ -611,7 +611,7 @@ public class ForumController extends BasicController implements GenericEventList DBFactory.getInstance().intermediateCommit(); // if notification is enabled -> notify the publisher about news if (subsContext != null) { - NotificationsManager.getInstance().markPublisherNews(subsContext, ureq.getIdentity()); + NotificationsManager.getInstance().markPublisherNews(subsContext, ureq.getIdentity(), true); } currentMsg = m; markRead(m, ureq.getIdentity()); @@ -631,7 +631,7 @@ public class ForumController extends BasicController implements GenericEventList fm.addTopMessage(ureq.getIdentity(), forum, m); // if notification is enabled -> notify the publisher about news if (subsContext != null) { - NotificationsManager.getInstance().markPublisherNews(subsContext, ureq.getIdentity()); + NotificationsManager.getInstance().markPublisherNews(subsContext, ureq.getIdentity(), true); } currentMsg = m; markRead(m, ureq.getIdentity()); diff --git a/src/main/java/org/olat/modules/wiki/WikiMainController.java b/src/main/java/org/olat/modules/wiki/WikiMainController.java index c9c3766bf08..ff60729f9f3 100644 --- a/src/main/java/org/olat/modules/wiki/WikiMainController.java +++ b/src/main/java/org/olat/modules/wiki/WikiMainController.java @@ -174,8 +174,6 @@ public class WikiMainController extends BasicController implements CloneableCont this.ores = ores; this.securityCallback = securityCallback; this.subsContext = securityCallback.getSubscriptionContext(); - - if (securityCallback == null) throw new AssertException("WikiSecurityCallback is null! You must provide an security callback!", null); this.ident = ureq.getIdentity(); WikiPage page = null; Wiki wiki = getWiki(); @@ -847,7 +845,7 @@ public class WikiMainController extends BasicController implements CloneableCont if (page.getPageName().equals(WikiPage.WIKI_MENU_PAGE)) wikiMenuComp.setWikiContent(page.getContent()); WikiManager.getInstance().saveWikiPage(ores, page, true, wiki); // inform subscription context about changes - NotificationsManager.getInstance().markPublisherNews(subsContext, ureq.getIdentity()); + NotificationsManager.getInstance().markPublisherNews(subsContext, ureq.getIdentity(), true); updatePageContext(ureq, page); } diff --git a/src/main/java/org/olat/notifications/NotificationsManagerImpl.java b/src/main/java/org/olat/notifications/NotificationsManagerImpl.java index 3ccb3521b0a..4683b08ac4d 100644 --- a/src/main/java/org/olat/notifications/NotificationsManagerImpl.java +++ b/src/main/java/org/olat/notifications/NotificationsManagerImpl.java @@ -38,15 +38,19 @@ import java.util.Locale; import java.util.Map; import java.util.Set; +import javax.persistence.EntityManager; +import javax.persistence.LockModeType; +import javax.persistence.TypedQuery; + import org.apache.velocity.VelocityContext; import org.hibernate.FlushMode; import org.olat.ControllerFactory; import org.olat.admin.user.delete.service.UserDeletionManager; import org.olat.core.CoreBeanTypes; import org.olat.core.CoreSpringFactory; -import org.olat.core.commons.persistence.DB; import org.olat.core.commons.persistence.DBFactory; import org.olat.core.commons.persistence.DBQuery; +import org.olat.core.commons.persistence.PersistenceHelper; import org.olat.core.gui.translator.Translator; import org.olat.core.id.Identity; import org.olat.core.id.OLATResourceable; @@ -141,7 +145,7 @@ public class NotificationsManagerImpl extends NotificationsManager implements Us * @param subscriptionContext the context of the object we subscribe to * @return a subscriber with a db key */ - private Subscriber doCreateAndPersistSubscriber(Publisher persistedPublisher, Identity listener) { + protected Subscriber doCreateAndPersistSubscriber(Publisher persistedPublisher, Identity listener) { SubscriberImpl si = new SubscriberImpl(persistedPublisher, listener); si.setLastModified(new Date()); si.setLatestEmailed(new Date()); @@ -176,14 +180,12 @@ public class NotificationsManagerImpl extends NotificationsManager implements Us if(types != null && !types.isEmpty()) { sb.append(" and publisher.type in (:types)"); } - DBQuery query = DBFactory.getInstance().createQuery(sb.toString()); - query.setEntity("anIdentity", identity); + TypedQuery<Subscriber> query = DBFactory.getInstance().getCurrentEntityManager().createQuery(sb.toString(), Subscriber.class); + query.setParameter("anIdentity", identity); if(types != null && !types.isEmpty()) { - query.setParameterList("types", types); + query.setParameter("types", types); } - @SuppressWarnings("unchecked") - List<Subscriber> res = query.list(); - return res; + return query.getResultList(); } /** @@ -191,48 +193,47 @@ public class NotificationsManagerImpl extends NotificationsManager implements Us * @return a list of all subscribers which belong to the identity and which * publishers are valid */ - @SuppressWarnings("unchecked") + @Override public List<Subscriber> getValidSubscribers(Identity identity) { - //pub.getState() == PUB_STATE_OK; - DB db = DBFactory.getInstance(); - String q = "select sub from org.olat.notifications.SubscriberImpl sub" + " inner join fetch sub.publisher as pub" - + " where sub.identity = :anIdentity" + " and pub.state = :aState"; - DBQuery query = db.createQuery(q); - query.setEntity("anIdentity", identity); - query.setLong("aState", PUB_STATE_OK); - List<Subscriber> res = query.list(); - return res; + StringBuilder q = new StringBuilder(); + q.append("select sub from ").append(SubscriberImpl.class.getName()).append(" sub ") + .append(" inner join fetch sub.publisher as pub ") + .append(" where sub.identity.key=:anIdentityKey and pub.state=").append(PUB_STATE_OK); + + return DBFactory.getInstance().getCurrentEntityManager() + .createQuery(q.toString(), Subscriber.class) + .setParameter("anIdentityKey", identity.getKey()) + .getResultList(); } /** * @see org.olat.core.util.notifications.NotificationsManager#getValidSubscribersOf(org.olat.core.util.notifications.Publisher) */ - @SuppressWarnings("unchecked") + @Override public List<Subscriber> getValidSubscribersOf(Publisher publisher) { - DB db = DBFactory.getInstance(); - String q = "select sub from org.olat.notifications.SubscriberImpl sub inner join fetch sub.identity" - + " where sub.publisher = :publisher" - + " and sub.publisher.state = "+PUB_STATE_OK; - DBQuery query = db.createQuery(q); - query.setEntity("publisher", publisher); - List<Subscriber> res = query.list(); - return res; + StringBuilder q = new StringBuilder(); + q.append("select sub from ").append(SubscriberImpl.class.getName()).append(" sub ") + .append(" inner join fetch sub.identity") + .append(" where sub.publisher = :publisher and sub.publisher.state=").append(PUB_STATE_OK); + + return DBFactory.getInstance().getCurrentEntityManager() + .createQuery(q.toString(), Subscriber.class) + .setParameter("publisher", publisher) + .getResultList(); } - /** * @return a list of subscribers ordered by the name of the identity of the * subscription */ - @SuppressWarnings("unchecked") - private List<Subscriber> getAllValidSubscribers() { - DB db = DBFactory.getInstance(); - String q = "select sub from org.olat.notifications.SubscriberImpl sub" + " inner join fetch sub.publisher as pub" - + " where pub.state = :aState" + " order by sub.identity.name"; - DBQuery query = db.createQuery(q); - query.setLong("aState", PUB_STATE_OK); - List<Subscriber> res = query.list(); - return res; + protected List<Subscriber> getAllValidSubscribers() { + StringBuilder q = new StringBuilder(); + q.append("select sub from ").append(SubscriberImpl.class.getName()).append(" sub") + .append(" inner join fetch sub.publisher as pub") + .append(" where pub.state=").append(PUB_STATE_OK).append(" order by sub.identity.name"); + return DBFactory.getInstance().getCurrentEntityManager() + .createQuery(q.toString(), Subscriber.class) + .getResultList(); } @Override @@ -242,13 +243,12 @@ public class NotificationsManagerImpl extends NotificationsManager implements Us .append(" inner join fetch sub.publisher as pub") .append(" where sub.identity=:identity and pub.type=:type and pub.state=:aState"); - DBQuery query = DBFactory.getInstance().createQuery(sb.toString()); - query.setLong("aState", PUB_STATE_OK); - query.setString("type", publisherType); - query.setEntity("identity", identity); - - @SuppressWarnings("unchecked") - List<Subscriber> subscribers = query.list(); + List<Subscriber> subscribers = DBFactory.getInstance().getCurrentEntityManager() + .createQuery(sb.toString(), Subscriber.class) + .setParameter("aState", PUB_STATE_OK) + .setParameter("type", publisherType) + .setParameter("identity", identity) + .getResultList(); if(subscribers.isEmpty()) { return Collections.emptyList(); } @@ -452,21 +452,36 @@ public class NotificationsManagerImpl extends NotificationsManager implements Us return userInterval; } - + @Override public boolean sendMailToUserAndUpdateSubscriber(Identity curIdent, List<SubscriptionItem> items, Translator translator, List<Subscriber> subscribersToUpdate) { boolean sentOk = sendEmail(curIdent, translator.translate("rss.title", new String[] { NotificationHelper.getFormatedName(curIdent) }), items); // save latest email sent date for the subscription just emailed // do this only if the mail was successfully sent if (sentOk) { - for (Iterator<Subscriber> it_subs = subscribersToUpdate.iterator(); it_subs.hasNext();) { - Subscriber subscriber = it_subs.next(); - subscriber.setLatestEmailed(new Date()); - updateSubscriber(subscriber); - } + updateSubscriberLatestEmail(subscribersToUpdate); } return sentOk; } + protected void updateSubscriberLatestEmail(List<Subscriber> subscribersToUpdate) { + StringBuilder q = new StringBuilder(); + q.append("select sub from ").append(SubscriberImpl.class.getName()).append(" sub ") + .append(" inner join fetch sub.publisher where sub.key in (:aKey)"); + + EntityManager em = DBFactory.getInstance().getCurrentEntityManager(); + List<Long> keys = PersistenceHelper.toKeys(subscribersToUpdate); + List<Subscriber> subscribers = em.createQuery(q.toString(), Subscriber.class) + .setParameter("aKey", keys) + .setLockMode(LockModeType.PESSIMISTIC_WRITE) + .getResultList(); + + for (Subscriber subscriber :subscribers) { + subscriber.setLastModified(new Date()); + subscriber.setLatestEmailed(new Date()); + em.merge(subscriber); + } + } + private boolean sendEmail(Identity to, String title, List<SubscriptionItem> subItems) { StringBuilder plaintext = new StringBuilder(); for (Iterator<SubscriptionItem> it_subs = subItems.iterator(); it_subs.hasNext();) { @@ -514,16 +529,19 @@ public class NotificationsManagerImpl extends NotificationsManager implements Us * @param key * @return the subscriber with this key or null if not found */ - @SuppressWarnings("unchecked") + @Override public Subscriber getSubscriber(Long key) { - DB db = DBFactory.getInstance(); - String q = "select sub from org.olat.notifications.SubscriberImpl sub" + " inner join fetch sub.publisher " + " where sub.key = :aKey"; - DBQuery query = db.createQuery(q); - query.setLong("aKey", key.longValue()); - List<Subscriber> res = query.list(); - int cnt = res.size(); - if (cnt == 0) return null; - if (cnt > 1) throw new AssertException("more than one subscriber for key " + key); + StringBuilder q = new StringBuilder(); + q.append("select sub from ").append(SubscriberImpl.class.getName()).append(" as sub") + .append(" inner join fetch sub.publisher ") + .append(" where sub.key=:aKey"); + + List<Subscriber> res = DBFactory.getInstance().getCurrentEntityManager() + .createQuery(q.toString(), Subscriber.class) + .setParameter("aKey", key.longValue()) + .getResultList(); + if (res.isEmpty()) return null; + if (res.size() > 1) throw new AssertException("more than one subscriber for key " + key); return res.get(0); } @@ -544,25 +562,19 @@ public class NotificationsManagerImpl extends NotificationsManager implements Us final OLATResourceable ores = OresHelper.createOLATResourceableInstance(scontext.getResName() + "_" + scontext.getSubidentifier(),scontext.getResId()); //o_clusterOK by:cg //fxdiff VCRP-16:prevent nested doInSync - Publisher pub = getPublisher(scontext.getResName(), scontext.getResId(), scontext.getSubidentifier()); + Publisher pub = getPublisher(scontext); if(pub != null) { return pub; } Publisher publisher = CoordinatorManager.getInstance().getCoordinator().getSyncer().doInSync(ores, new SyncerCallback<Publisher>(){ public Publisher execute() { - Publisher p = getPublisher(scontext.getResName(), scontext.getResId(), scontext.getSubidentifier()); + Publisher p = getPublisher(scontext); // if not found, create it if (p == null) { p = createAndPersistPublisher(scontext.getResName(), scontext.getResId(), scontext.getSubidentifier(), pdata.getType(), pdata .getData(), pdata.getBusinessPath()); } - if (p.getData() == null || !p.getData().startsWith("[")) { - //update silently the publisher - if(pdata.getData() != null) { - //updatePublisher(p, pdata.getData()); - } - } return p; } }); @@ -573,35 +585,43 @@ public class NotificationsManagerImpl extends NotificationsManager implements Us * @param subsContext * @return the publisher belonging to the given context or null */ + @Override public Publisher getPublisher(SubscriptionContext subsContext) { - return getPublisher(subsContext.getResName(), subsContext.getResId(), subsContext.getSubidentifier()); + return getPublisher(subsContext.getResName(), subsContext.getResId(), subsContext.getSubidentifier(), false); + } + + private Publisher getPublisherForUpdate(SubscriptionContext subsContext) { + return getPublisher(subsContext.getResName(), subsContext.getResId(), subsContext.getSubidentifier(), true); } + @Override public List<Publisher> getAllPublisher() { - DB db = DBFactory.getInstance(); String q = "select pub from org.olat.notifications.PublisherImpl pub"; - DBQuery query = db.createQuery(q); - return query.list(); + return DBFactory.getInstance().getCurrentEntityManager().createQuery(q, Publisher.class) + .getResultList(); } - + /** * return the publisher for the given composite primary key ores + * subidentifier. */ - @SuppressWarnings("unchecked") - private Publisher getPublisher(String resName, Long resId, String subidentifier) { - DB db = DBFactory.getInstance(); - String q = "select pub from org.olat.notifications.PublisherImpl pub" + " where pub.resName = :resName" + " and pub.resId = :resId" - + " and pub.subidentifier = :subidentifier"; - DBQuery query = db.createQuery(q); - query.setString("resName", resName); - query.setLong("resId", resId.longValue()); - query.setString("subidentifier", subidentifier); - List<Publisher> res = query.list(); - if (res.size() == 0) return null; + private Publisher getPublisher(String resName, Long resId, String subidentifier, boolean forUpdate) { + StringBuilder q = new StringBuilder(); + q.append("select pub from ").append(PublisherImpl.class.getName()).append(" pub ") + .append(" where pub.resName=:resName and pub.resId = :resId and pub.subidentifier = :subidentifier"); + + TypedQuery<Publisher> query = DBFactory.getInstance().getCurrentEntityManager() + .createQuery(q.toString(), Publisher.class) + .setParameter("resName", resName) + .setParameter("resId", resId.longValue()) + .setParameter("subidentifier", subidentifier); + if(forUpdate) { + query.setLockMode(LockModeType.PESSIMISTIC_WRITE); + } + List<Publisher> res = query.getResultList(); + if (res.isEmpty()) return null; if (res.size() != 1) throw new AssertException("only one subscriber per person and publisher!!"); - Publisher p = res.get(0); - return p; + return res.get(0); } /** @@ -609,15 +629,13 @@ public class NotificationsManagerImpl extends NotificationsManager implements Us * @param resId * @return a list of publishers belonging to the resource */ - @SuppressWarnings("unchecked") private List<Publisher> getPublishers(String resName, Long resId) { - DB db = DBFactory.getInstance(); String q = "select pub from org.olat.notifications.PublisherImpl pub" + " where pub.resName = :resName" + " and pub.resId = :resId"; - DBQuery query = db.createQuery(q); - query.setString("resName", resName); - query.setLong("resId", resId.longValue()); - List<Publisher> res = query.list(); - return res; + return DBFactory.getInstance().getCurrentEntityManager() + .createQuery(q, Publisher.class) + .setParameter("resName", resName) + .setParameter("resId", resId.longValue()) + .getResultList(); } /** @@ -651,14 +669,36 @@ public class NotificationsManagerImpl extends NotificationsManager implements Us * @return a Subscriber object belonging to the identity and listening to the * given publisher */ - @SuppressWarnings("unchecked") + @Override public Subscriber getSubscriber(Identity identity, Publisher publisher) { - String q = "select sub from org.olat.notifications.SubscriberImpl sub where sub.publisher = :publisher" - + " and sub.identity = :identity"; - DBQuery query = DBFactory.getInstance().createQuery(q); - query.setEntity("publisher", publisher); - query.setEntity("identity", identity); - List<Subscriber> res = query.list(); + StringBuilder q = new StringBuilder(); + q.append("select sub from ").append(SubscriberImpl.class.getName()).append(" as sub ") + .append(" where sub.publisher.key=:publisherKey and sub.identity.key=:identityKey"); + + List<Subscriber> res = DBFactory.getInstance().getCurrentEntityManager() + .createQuery(q.toString(), Subscriber.class) + .setParameter("publisherKey", publisher.getKey()) + .setParameter("identityKey", identity.getKey()) + .getResultList(); + + if (res.size() == 0) return null; + if (res.size() != 1) throw new AssertException("only one subscriber per person and publisher!!"); + Subscriber s = res.get(0); + return s; + } + + private Subscriber getSubscriberForUpdate(Identity identity, Publisher publisher) { + StringBuilder q = new StringBuilder(); + q.append("select sub from ").append(SubscriberImpl.class.getName()).append(" as sub ") + .append(" where sub.publisher.key=:publisherKey and sub.identity.key=:identityKey"); + + List<Subscriber> res = DBFactory.getInstance().getCurrentEntityManager() + .createQuery(q.toString(), Subscriber.class) + .setParameter("publisherKey", publisher.getKey()) + .setParameter("identityKey", identity.getKey()) + .setLockMode(LockModeType.PESSIMISTIC_WRITE) + .getResultList(); + if (res.size() == 0) return null; if (res.size() != 1) throw new AssertException("only one subscriber per person and publisher!!"); Subscriber s = res.get(0); @@ -671,13 +711,12 @@ public class NotificationsManagerImpl extends NotificationsManager implements Us @Override public List<Subscriber> getSubscribers(Publisher publisher) { String q = "select sub from org.olat.notifications.SubscriberImpl sub where sub.publisher = :publisher"; - DBQuery query = DBFactory.getInstance().createQuery(q); - query.setEntity("publisher", publisher); - List<Subscriber> res = query.list(); - return res; + return DBFactory.getInstance().getCurrentEntityManager() + .createQuery(q, Subscriber.class) + .setParameter("publisher", publisher) + .getResultList(); } - /** * * @see org.olat.core.util.notifications.NotificationsManager#getSubscriberIdentities(org.olat.core.util.notifications.Publisher) @@ -685,10 +724,10 @@ public class NotificationsManagerImpl extends NotificationsManager implements Us @Override public List<Identity> getSubscriberIdentities(Publisher publisher) { String q = "select sub.identity from org.olat.notifications.SubscriberImpl sub where sub.publisher = :publisher"; - DBQuery query = DBFactory.getInstance().createQuery(q); - query.setEntity("publisher", publisher); - List<Identity> res = query.list(); - return res; + return DBFactory.getInstance().getCurrentEntityManager() + .createQuery(q, Identity.class) + .setParameter("publisher", publisher) + .getResultList(); } /** @@ -717,30 +756,7 @@ public class NotificationsManagerImpl extends NotificationsManager implements Us * @param subscriber */ private void deleteSubscriber(Subscriber subscriber) { - DB db = DBFactory.getInstance(); - db.deleteObject(subscriber); - } - - /** - * @param subscriber - */ - private void updateSubscriber(Subscriber subscriber) { - subscriber.setLastModified(new Date()); - DBFactory.getInstance().updateObject(subscriber); - } - - /** - * @param publisher - */ - public void updatePublisher(Publisher publisher) { - DBFactory.getInstance().updateObject(publisher); - } - - /** - * @param publisher - */ - public void deletePublisher(Publisher publisher) { - DBFactory.getInstance().deleteObject(publisher); + DBFactory.getInstance().deleteObject(subscriber); } /** @@ -750,14 +766,21 @@ public class NotificationsManagerImpl extends NotificationsManager implements Us * @param identity * @param subsContext */ + @Override public void markSubscriberRead(Identity identity, SubscriptionContext subsContext) { - Publisher p = getPublisher(subsContext.getResName(), subsContext.getResId(), subsContext.getSubidentifier()); + Publisher p = getPublisherForUpdate(subsContext); if (p == null) throw new AssertException("cannot markRead for identity " + identity.getName() + ", since the publisher for the given subscriptionContext does not exist: subscontext = " + subsContext); - Subscriber sub = getSubscriber(identity, p); - if (sub == null) throw new AssertException("cannot markRead, since identity " + identity.getName() - + " is not subscribed to subscontext " + subsContext); - updateSubscriber(sub); + + markSubscriberRead(identity, p); + } + + private void markSubscriberRead(Identity identity, Publisher p) { + Subscriber sub = getSubscriberForUpdate(identity, p); + if(sub != null) { + sub.setLastModified(new Date()); + DBFactory.getInstance().getCurrentEntityManager().merge(sub); + } } /** @@ -765,15 +788,23 @@ public class NotificationsManagerImpl extends NotificationsManager implements Us * @param subscriptionContext * @param publisherData */ + @Override public void subscribe(Identity identity, SubscriptionContext subscriptionContext, PublisherData publisherData) { - // no need to sync, since an identity only has one gui thread / one mouse - Publisher p = findOrCreatePublisher(subscriptionContext, publisherData); - Subscriber s = getSubscriber(identity, p); + //need to sync as opt-in is sometimes implemented + Publisher toUpdate = getPublisherForUpdate(subscriptionContext); + if(toUpdate == null) { + //create the publisher + findOrCreatePublisher(subscriptionContext, publisherData); + //lock the publisher + toUpdate = getPublisherForUpdate(subscriptionContext); + } + + Subscriber s = getSubscriber(identity, toUpdate); if (s == null) { // no subscriber -> create. // s.latestReadDate >= p.latestNewsDate == no news for subscriber when no // news after subscription time - doCreateAndPersistSubscriber(p, identity); + doCreateAndPersistSubscriber(toUpdate, identity); } } @@ -784,62 +815,39 @@ public class NotificationsManagerImpl extends NotificationsManager implements Us * @param subscriptionContext * @param ignoreNewsFor */ - public void markPublisherNews(final SubscriptionContext subscriptionContext, Identity ignoreNewsFor) { + @Override + public void markPublisherNews(final SubscriptionContext subscriptionContext, Identity ignoreNewsFor, boolean sendEvents) { // to make sure: ignore if no subscriptionContext if (subscriptionContext == null) return; - final Date now = new Date(); - - // two threads with both having a publisher they want to update - //o_clusterOK by:cg - final OLATResourceable ores = OresHelper.createOLATResourceableInstance(subscriptionContext.getResName() + "_" + subscriptionContext.getSubidentifier(),subscriptionContext.getResId()); - Publisher publisher = CoordinatorManager.getInstance().getCoordinator().getSyncer().doInSync(ores, new SyncerCallback<Publisher>(){ - public Publisher execute() { - Publisher p = getPublisher(subscriptionContext); - // if no publisher yet, ignore - //TODO: check if that can be null - if (p == null) return null; - - // force a reload from db loadObject(..., true) by evicting it from - // hibernates session - // cache to catch up on a different thread having commited the update of - // this object - - // not needed, since getPublisher()... always loads from db??? - //p = (Publisher) DB.getInstance().loadObject(p, true); - p.setLatestNewsDate(now); - updatePublisher(p); - return p; - } - }); - if (publisher == null) {//TODO: check if that can be null - return; + Publisher toUpdate = getPublisher(subscriptionContext.getResName(), subscriptionContext.getResId(), subscriptionContext.getSubidentifier(), true); + if(toUpdate == null) { + return; } - + toUpdate.setLatestNewsDate(new Date()); + Publisher publisher = DBFactory.getInstance().getCurrentEntityManager().merge(toUpdate); + // no need to sync, since there is only one gui thread at a time from one // user if (ignoreNewsFor != null) { - Subscriber sub = getSubscriber(ignoreNewsFor, publisher); - if (sub != null) { // mark as read if subscribed - updateSubscriber(sub); - } + markSubscriberRead(ignoreNewsFor, publisher); } - - // channel-notify all interested listeners (e.g. the pnotificationsportletruncontroller) - // 1. find all subscribers which can be affected - List<Subscriber> subscribers = getValidSubscribersOf(publisher); - - Set<Long> subsKeys = new HashSet<Long>(); - // 2. collect all keys of the affected subscribers - for (Iterator<Subscriber> it_subs = subscribers.iterator(); it_subs.hasNext();) { - Subscriber su = it_subs.next(); - subsKeys.add(su.getKey()); + if(sendEvents) { + // channel-notify all interested listeners (e.g. the pnotificationsportletruncontroller) + // 1. find all subscribers which can be affected + List<Subscriber> subscribers = getValidSubscribersOf(publisher); + + Set<Long> subsKeys = new HashSet<Long>(); + // 2. collect all keys of the affected subscribers + for (Iterator<Subscriber> it_subs = subscribers.iterator(); it_subs.hasNext();) { + Subscriber su = it_subs.next(); + subsKeys.add(su.getKey()); + } + // fire the event + MultiUserEvent mue = EventFactory.createAffectedEvent(subsKeys); + CoordinatorManager.getInstance().getCoordinator().getEventBus().fireEventToListenersOf(mue, oresMyself); } - // fire the event - MultiUserEvent mue = EventFactory.createAffectedEvent(subsKeys); - CoordinatorManager.getInstance().getCoordinator().getEventBus().fireEventToListenersOf(mue, oresMyself); - } /** @@ -855,7 +863,6 @@ public class NotificationsManagerImpl extends NotificationsManager implements Us public void deregisterAsListener(GenericEventListener gel) { CoordinatorManager.getInstance().getCoordinator().getEventBus().deregisterFor(gel, oresMyself); } - /** * @param identity @@ -863,7 +870,7 @@ public class NotificationsManagerImpl extends NotificationsManager implements Us */ public void unsubscribe(Identity identity, SubscriptionContext subscriptionContext) { // no need to sync, since an identity only has one gui thread / one mouse - Publisher p = getPublisher(subscriptionContext); + Publisher p = getPublisherForUpdate(subscriptionContext); // if no publisher yet. //TODO: check race condition: can p be null at all? if (p == null) return; @@ -879,6 +886,7 @@ public class NotificationsManagerImpl extends NotificationsManager implements Us * * @see org.olat.core.util.notifications.NotificationsManager#unsubscribe(org.olat.core.util.notifications.Subscriber) */ + @Override public void unsubscribe(Subscriber s) { Subscriber foundSub = getSubscriber(s.getKey()); if (foundSub != null) { @@ -893,24 +901,25 @@ public class NotificationsManagerImpl extends NotificationsManager implements Us * @param subscriptionContext * @return true if this user is subscribed */ - @SuppressWarnings("unchecked") + @Override public boolean isSubscribed(Identity identity, SubscriptionContext subscriptionContext) { - DB db = DBFactory.getInstance(); - String q = "select count(sub) from org.olat.notifications.SubscriberImpl sub inner join sub.publisher as pub " - + " where sub.identity = :anIdentity and pub.resName = :resName and pub.resId = :resId" - + " and pub.subidentifier = :subidentifier group by sub"; - DBQuery query = db.createQuery(q); - query.setEntity("anIdentity", identity); - query.setString("resName", subscriptionContext.getResName()); - query.setLong("resId", subscriptionContext.getResId().longValue()); - query.setString("subidentifier", subscriptionContext.getSubidentifier()); - List res = query.list(); - // must return one result or null - if (res.isEmpty()) return false; - long cnt = ( (Long)res.get(0) ).longValue(); + StringBuilder q = new StringBuilder(); + q.append("select count(sub) from ").append(SubscriberImpl.class.getName()).append(" as sub ") + .append(" inner join sub.publisher as pub ") + .append(" where sub.identity.key=:anIdentityKey and pub.resName=:resName and pub.resId=:resId") + .append(" and pub.subidentifier=:subidentifier"); + + Number count = DBFactory.getInstance().getCurrentEntityManager() + .createQuery(q.toString(), Number.class) + .setParameter("anIdentityKey", identity.getKey()) + .setParameter("resName", subscriptionContext.getResName()) + .setParameter("resId", subscriptionContext.getResId().longValue()) + .setParameter("subidentifier", subscriptionContext.getSubidentifier()) + .getSingleResult(); + + long cnt = count.longValue(); if (cnt == 0) return false; - else if (cnt == 1) return true; - else throw new AssertException("more than once subscribed!" + identity + ", " + subscriptionContext); + else return true; } /** @@ -919,18 +928,18 @@ public class NotificationsManagerImpl extends NotificationsManager implements Us * @param scontext the subscriptioncontext */ public void delete(SubscriptionContext scontext) { - Publisher p = getPublisher(scontext.getResName(), scontext.getResId(), scontext.getSubidentifier()); + Publisher p = getPublisher(scontext); // if none found, no one has subscribed yet and therefore no publisher has // been generated lazily. // -> nothing to do if (p == null) return; //first delete all subscribers List<Subscriber> subscribers = getValidSubscribersOf(p); - for (Object susbscriberObj : subscribers) { - deleteSubscriber((Subscriber)susbscriberObj); + for (Subscriber subscriber : subscribers) { + deleteSubscriber(subscriber); } // else: - deletePublisher(p); + DBFactory.getInstance().deleteObject(p); } /** @@ -938,9 +947,13 @@ public class NotificationsManagerImpl extends NotificationsManager implements Us * * @param publisher the publisher to delete */ + @Override public void deactivate(Publisher publisher) { - publisher.setState(PUB_STATE_NOT_OK); - updatePublisher(publisher); + EntityManager em = DBFactory.getInstance().getCurrentEntityManager(); + + PublisherImpl toDeactivate = em.find(PublisherImpl.class, publisher.getKey(), LockModeType.PESSIMISTIC_WRITE); + toDeactivate.setState(PUB_STATE_NOT_OK); + em.merge(toDeactivate); } /** @@ -952,21 +965,6 @@ public class NotificationsManagerImpl extends NotificationsManager implements Us return pub.getState() == PUB_STATE_OK; } - /** - * no match if: a) not the same publisher b) a deleted publisher - * - * @param p - * @param subscriptionContext - * @return true when the subscriptionContext refers to the publisher p - */ - public boolean matches(Publisher p, SubscriptionContext subscriptionContext) { - // if the publisher has been deleted in the meantime, return no match - if (!isPublisherValid(p)) return false; - boolean ok = (p.getResName().equals(subscriptionContext.getResName()) && p.getResId().equals(subscriptionContext.getResId()) && p - .getSubidentifier().equals(subscriptionContext.getSubidentifier())); - return ok; - } - /** * @param subscriber * @param locale diff --git a/src/main/java/org/olat/notifications/NotificationsPortletRunController.java b/src/main/java/org/olat/notifications/NotificationsPortletRunController.java index b7fa0a563b6..62829f0b33e 100644 --- a/src/main/java/org/olat/notifications/NotificationsPortletRunController.java +++ b/src/main/java/org/olat/notifications/NotificationsPortletRunController.java @@ -247,7 +247,7 @@ public class NotificationsPortletRunController extends AbstractPortletRunControl } public void event(Event event) { - // check if our tablemodel -is- affected (see NotificationsManagerImpl where the event is fired), + // check if our tablemodel -is- affected (see NotificationsManager where the event is fired), // (if we are subscriber of the publisher which data has changed) if (event instanceof PersistsEvent) { PersistsEvent pe = (PersistsEvent) event; diff --git a/src/main/java/org/olat/notifications/PublisherImpl.java b/src/main/java/org/olat/notifications/PublisherImpl.java index 196140a2659..e64bae0a9ef 100644 --- a/src/main/java/org/olat/notifications/PublisherImpl.java +++ b/src/main/java/org/olat/notifications/PublisherImpl.java @@ -39,6 +39,9 @@ import org.olat.core.util.notifications.Publisher; * @author Felix Jost */ public class PublisherImpl extends PersistentObject implements Publisher { + + private static final long serialVersionUID = -7684628889607509977L; + private String type; // e.g. Forum private String resName; // e.g. CourseModule private Long resId; // e.g. 2343284327 @@ -181,5 +184,21 @@ public class PublisherImpl extends PersistentObject implements Publisher { public void setBusinessPath(String businessPath) { this.businessPath = businessPath; } -} + @Override + public int hashCode() { + return getKey() == null ? 54212 : getKey().hashCode(); + } + + @Override + public boolean equals(Object obj) { + if(this == obj) { + return true; + } + if(obj instanceof PublisherImpl) { + PublisherImpl p = (PublisherImpl)obj; + return getKey() != null && getKey().equals(p.getKey()); + } + return false; + } +} \ No newline at end of file diff --git a/src/main/java/org/olat/notifications/SubscriberImpl.java b/src/main/java/org/olat/notifications/SubscriberImpl.java index 6f1cce5ba0c..e0516b10772 100644 --- a/src/main/java/org/olat/notifications/SubscriberImpl.java +++ b/src/main/java/org/olat/notifications/SubscriberImpl.java @@ -41,6 +41,8 @@ import org.olat.core.util.notifications.Subscriber; * @author Felix Jost */ public class SubscriberImpl extends PersistentObject implements Subscriber { + private static final long serialVersionUID = 6165097156137862263L; + // reference to the subscribed publisher private Publisher publisher; @@ -125,6 +127,20 @@ public class SubscriberImpl extends PersistentObject implements Subscriber { public void setLastModified(Date date) { this.lastModified = date; } - -} - + @Override + public int hashCode() { + return getKey() == null ? 813184 : getKey().hashCode(); + } + + @Override + public boolean equals(Object obj) { + if(this == obj) { + return true; + } + if(obj instanceof SubscriberImpl) { + SubscriberImpl s = (SubscriberImpl)obj; + return getKey() != null && getKey().equals(s.getKey()); + } + return false; + } +} \ No newline at end of file diff --git a/src/main/java/org/olat/user/notification/UsersSubscriptionManagerImpl.java b/src/main/java/org/olat/user/notification/UsersSubscriptionManagerImpl.java index f38b077daaf..6a40cd27695 100644 --- a/src/main/java/org/olat/user/notification/UsersSubscriptionManagerImpl.java +++ b/src/main/java/org/olat/user/notification/UsersSubscriptionManagerImpl.java @@ -138,7 +138,7 @@ public class UsersSubscriptionManagerImpl extends UsersSubscriptionManager imple */ public void markPublisherNews() { SubscriptionContext context = getNewUsersSubscriptionContext(); - NotificationsManager.getInstance().markPublisherNews(context, null); + NotificationsManager.getInstance().markPublisherNews(context, null, true); } /** diff --git a/src/test/java/org/olat/notifications/NotificationsManagerTest.java b/src/test/java/org/olat/notifications/NotificationsManagerTest.java index 8dea1f333c0..521fafdea1c 100644 --- a/src/test/java/org/olat/notifications/NotificationsManagerTest.java +++ b/src/test/java/org/olat/notifications/NotificationsManagerTest.java @@ -31,34 +31,30 @@ import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; -import static org.junit.Assert.fail; import java.util.ArrayList; import java.util.Collections; import java.util.List; import java.util.UUID; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; + +import junit.framework.Assert; -import org.junit.After; -import org.junit.Before; import org.junit.Test; import org.olat.core.commons.persistence.DB; -import org.olat.core.commons.persistence.DBFactory; import org.olat.core.id.Identity; +import org.olat.core.logging.DBRuntimeException; import org.olat.core.logging.OLog; import org.olat.core.logging.Tracing; import org.olat.core.util.notifications.NotificationsManager; import org.olat.core.util.notifications.Publisher; import org.olat.core.util.notifications.PublisherData; +import org.olat.core.util.notifications.Subscriber; import org.olat.core.util.notifications.SubscriptionContext; -import org.olat.test.JMSCodePointServerJunitHelper; +import org.olat.core.util.notifications.SubscriptionInfo; import org.olat.test.JunitTestHelper; import org.olat.test.OlatTestCase; -import org.olat.testutils.codepoints.client.BreakpointStateException; -import org.olat.testutils.codepoints.client.CodepointClient; -import org.olat.testutils.codepoints.client.CodepointClientFactory; -import org.olat.testutils.codepoints.client.CodepointRef; -import org.olat.testutils.codepoints.client.CommunicationException; -import org.olat.testutils.codepoints.client.TemporaryPausedThread; import org.springframework.beans.factory.annotation.Autowired; /** @@ -70,61 +66,316 @@ import org.springframework.beans.factory.annotation.Autowired; * */ public class NotificationsManagerTest extends OlatTestCase { - private static OLog log = Tracing.createLoggerFor(NotificationsManagerTest.class); - private static final String CODEPOINT_SERVER_ID = "NotificationsManagerTest"; - - private static Identity identity1, identity2, identity3; @Autowired private NotificationsManager notificationManager; @Autowired private DB dbInstance; + + @Test + public void testCreatePublisher() { + String identifier = UUID.randomUUID().toString().replace("-", ""); + SubscriptionContext context = new SubscriptionContext("PS", new Long(123), identifier); + PublisherData publisherData = new PublisherData("testPublisherSubscriber", "e.g. forumdata=keyofforum", null); + + Publisher publisher = notificationManager.getOrCreatePublisher(context, publisherData); + dbInstance.commitAndCloseSession(); + //check values + Assert.assertNotNull(publisher); + Assert.assertNotNull(publisher.getKey()); + Assert.assertNotNull(publisher.getCreationDate()); + Assert.assertNotNull(publisher.getLatestNewsDate()); + Assert.assertEquals("PS", publisher.getResName()); + Assert.assertEquals(new Long(123), publisher.getResId()); + Assert.assertEquals(identifier, publisher.getSubidentifier()); + + //check if exists + Publisher reloadedPublisher = notificationManager.getPublisher(context); + Assert.assertNotNull(reloadedPublisher); + Assert.assertEquals(publisher, reloadedPublisher); + } + @Test + public void testAllPublishers() { + String identifier = UUID.randomUUID().toString().replace("-", ""); + SubscriptionContext context = new SubscriptionContext("All", new Long(123), identifier); + PublisherData publisherData = new PublisherData("testAllPublishers", "e.g. forumdata=keyofforum", null); + + Publisher publisher = notificationManager.getOrCreatePublisher(context, publisherData); + dbInstance.commitAndCloseSession(); + Assert.assertNotNull(publisher); + List<Publisher> publishers = notificationManager.getAllPublisher(); + Assert.assertNotNull(publishers); + Assert.assertTrue(publishers.contains(publisher)); + } + + @Test + public void testSubscribe() { + Identity id = JunitTestHelper.createAndPersistIdentityAsUser("subs-" + UUID.randomUUID().toString()); + //create a publisher + String identifier = UUID.randomUUID().toString().replace("-", ""); + SubscriptionContext context = new SubscriptionContext("All", new Long(123), identifier); + PublisherData publisherData = new PublisherData("testAllPublishers", "e.g. forumdata=keyofforum", null); + Publisher publisher = notificationManager.getOrCreatePublisher(context, publisherData); + dbInstance.commitAndCloseSession(); + Assert.assertNotNull(publisher); + + //subscribe + notificationManager.subscribe(id, context, publisherData); + dbInstance.commitAndCloseSession(); + + //check + boolean subscribed = notificationManager.isSubscribed(id, context); + Assert.assertTrue(subscribed); + dbInstance.commitAndCloseSession(); + + //double check + Subscriber subscriber = notificationManager.getSubscriber(id, publisher); + Assert.assertNotNull(subscriber); + Assert.assertEquals(publisher, subscriber.getPublisher()); + dbInstance.commitAndCloseSession(); + + //triple check + Subscriber reloadedSubscriber = notificationManager.getSubscriber(subscriber.getKey()); + Assert.assertNotNull(reloadedSubscriber); + Assert.assertEquals(subscriber, reloadedSubscriber); + } + + @Test + public void testUnsubscribe_v1() { + Identity id = JunitTestHelper.createAndPersistIdentityAsUser("unsubs-" + UUID.randomUUID().toString()); + //create a publisher + String identifier = UUID.randomUUID().toString().replace("-", ""); + SubscriptionContext context = new SubscriptionContext("All", new Long(123), identifier); + PublisherData publisherData = new PublisherData("testUnsubscribe", "e.g. forumdata=keyofforum", null); + Publisher publisher = notificationManager.getOrCreatePublisher(context, publisherData); + dbInstance.commitAndCloseSession(); + Assert.assertNotNull(publisher); + + //subscribe + notificationManager.subscribe(id, context, publisherData); + dbInstance.commitAndCloseSession(); + + //check + Subscriber subscriber = notificationManager.getSubscriber(id, publisher); + Assert.assertNotNull(subscriber); + + //unsubscribe + notificationManager.unsubscribe(subscriber); + dbInstance.commitAndCloseSession(); + + //check + boolean subscribed = notificationManager.isSubscribed(id, context); + Assert.assertFalse(subscribed); + } + + @Test + public void testUnsubscribe_v2() { + Identity id = JunitTestHelper.createAndPersistIdentityAsUser("unsubs-" + UUID.randomUUID().toString()); + //create a publisher + String identifier = UUID.randomUUID().toString().replace("-", ""); + SubscriptionContext context = new SubscriptionContext("All", new Long(123), identifier); + PublisherData publisherData = new PublisherData("testUnsubscribe", "e.g. forumdata=keyofforum", null); + Publisher publisher = notificationManager.getOrCreatePublisher(context, publisherData); + dbInstance.commitAndCloseSession(); + Assert.assertNotNull(publisher); + + //subscribe + notificationManager.subscribe(id, context, publisherData); + dbInstance.commitAndCloseSession(); + + //check + Subscriber subscriber = notificationManager.getSubscriber(id, publisher); + Assert.assertNotNull(subscriber); + + //unsubscribe + notificationManager.unsubscribe(id, context); + dbInstance.commitAndCloseSession(); + + //check + boolean subscribed = notificationManager.isSubscribed(id, context); + Assert.assertFalse(subscribed); + } + + @Test + public void testValidSubscribers() { + Identity id1 = JunitTestHelper.createAndPersistIdentityAsUser("valid1-" + UUID.randomUUID().toString()); + Identity id2 = JunitTestHelper.createAndPersistIdentityAsUser("valid1-" + UUID.randomUUID().toString()); + //create a publisher + String identifier = UUID.randomUUID().toString().replace("-", ""); + SubscriptionContext context = new SubscriptionContext("Valid", new Long(123), identifier); + PublisherData publisherData = new PublisherData("testValidSubscribers", "e.g. forumdata=keyofforum", null); + Publisher publisher = notificationManager.getOrCreatePublisher(context, publisherData); + dbInstance.commitAndCloseSession(); + Assert.assertNotNull(publisher); + + //add subscribers + notificationManager.subscribe(id1, context, publisherData); + notificationManager.subscribe(id2, context, publisherData); + dbInstance.commitAndCloseSession(); + + //get valid subscribers + List<Subscriber> subscribers = notificationManager.getValidSubscribers(id1); + Assert.assertNotNull(subscribers); + Assert.assertEquals(1, subscribers.size()); + Assert.assertEquals(publisher, subscribers.get(0).getPublisher()); + Assert.assertEquals(id1, subscribers.get(0).getIdentity()); + } + + @Test + public void testValidSubscribersOf() { + Identity id1 = JunitTestHelper.createAndPersistIdentityAsUser("valid1b-" + UUID.randomUUID().toString()); + Identity id2 = JunitTestHelper.createAndPersistIdentityAsUser("valid1b-" + UUID.randomUUID().toString()); + //create a publisher + String identifier = UUID.randomUUID().toString().replace("-", ""); + SubscriptionContext context = new SubscriptionContext("Validb", new Long(123), identifier); + PublisherData publisherData = new PublisherData("testValidSubscribers", "e.g. forumdata=keyofforum", null); + Publisher publisher = notificationManager.getOrCreatePublisher(context, publisherData); + dbInstance.commitAndCloseSession(); + Assert.assertNotNull(publisher); + + //add subscribers + notificationManager.subscribe(id1, context, publisherData); + notificationManager.subscribe(id2, context, publisherData); + dbInstance.commitAndCloseSession(); + + //get all subscribers of the publisher + List<Subscriber> subscribers = notificationManager.getValidSubscribersOf(publisher); + Assert.assertNotNull(subscribers); + Assert.assertEquals(2, subscribers.size()); + Assert.assertEquals(publisher, subscribers.get(0).getPublisher()); + Assert.assertEquals(publisher, subscribers.get(1).getPublisher()); + } + + @Test + public void testGetAllValidSubscribers() { + Identity id = JunitTestHelper.createAndPersistIdentityAsUser("valid1b-" + UUID.randomUUID().toString()); + //create a publisher + String identifier = UUID.randomUUID().toString().replace("-", ""); + SubscriptionContext context = new SubscriptionContext("AllSubs", new Long(130), identifier); + PublisherData publisherData = new PublisherData("testGetAllValidSubscribers", "e.g. forumdata=keyofforum", null); + Publisher publisher = notificationManager.getOrCreatePublisher(context, publisherData); + dbInstance.commitAndCloseSession(); + Assert.assertNotNull(publisher); + //add subscriber + notificationManager.subscribe(id, context, publisherData); + dbInstance.commitAndCloseSession(); + + //get all subscribers + List<Subscriber> allSubscribers = ((NotificationsManagerImpl)notificationManager).getAllValidSubscribers(); + Assert.assertNotNull(allSubscribers); + Assert.assertFalse(allSubscribers.isEmpty()); + + //get current subscriber + Subscriber thisSubscriber = notificationManager.getSubscriber(id, publisher); + Assert.assertNotNull(thisSubscriber); + Assert.assertTrue(allSubscribers.contains(thisSubscriber)); + } + + @Test + public void testGetSubscriberIdentities() { + Identity id1 = JunitTestHelper.createAndPersistIdentityAsUser("valid1b-" + UUID.randomUUID().toString()); + Identity id2 = JunitTestHelper.createAndPersistIdentityAsUser("valid1b-" + UUID.randomUUID().toString()); + //create a publisher + String identifier = UUID.randomUUID().toString().replace("-", ""); + SubscriptionContext context = new SubscriptionContext("Subscribers", new Long(123), identifier); + PublisherData publisherData = new PublisherData("testGetSubscriberIdentities", "e.g. forumdata=keyofforum", null); + Publisher publisher = notificationManager.getOrCreatePublisher(context, publisherData); + dbInstance.commitAndCloseSession(); + //add subscribers + notificationManager.subscribe(id1, context, publisherData); + notificationManager.subscribe(id2, context, publisherData); + dbInstance.commitAndCloseSession(); + + //get identities + List<Identity> identities = notificationManager.getSubscriberIdentities(publisher); + Assert.assertNotNull(identities); + Assert.assertEquals(2, identities.size()); + Assert.assertTrue(identities.contains(id1)); + Assert.assertTrue(identities.contains(id2)); + } + + @Test + public void testGetSubscribersByTypes() { + Identity id = JunitTestHelper.createAndPersistIdentityAsUser("type1-" + UUID.randomUUID().toString()); + //create a first publisher + String identifier = UUID.randomUUID().toString().replace("-", ""); + SubscriptionContext context1 = new SubscriptionContext("Subscribers", new Long(123), identifier); + PublisherData publisherData1 = new PublisherData("testGetSubscribersByType1", "e.g. forumdata=keyofforum", null); + Publisher publisher1 = notificationManager.getOrCreatePublisher(context1, publisherData1); + dbInstance.commitAndCloseSession(); + + String identifier2 = UUID.randomUUID().toString().replace("-", ""); + SubscriptionContext context2 = new SubscriptionContext("Subscribers", new Long(123), identifier2); + PublisherData publisherData2 = new PublisherData("testGetSubscribersByType2", "e.g. forumdata=keyofforum", null); + Publisher publisher2 = notificationManager.getOrCreatePublisher(context2, publisherData2); + dbInstance.commitAndCloseSession(); + + //add subscribers + notificationManager.subscribe(id, context1, publisherData1); + notificationManager.subscribe(id, context2, publisherData2); + dbInstance.commitAndCloseSession(); + + //get subscribers without types + List<Subscriber> emptySubscribers = notificationManager.getSubscribers(id, null); + Assert.assertNotNull(emptySubscribers); + Assert.assertEquals(2, emptySubscribers.size()); - /** - * @see junit.framework.TestCase#setUp() - */ - @Before - public void setup() { - // identity with null User should be ok for test case - identity1 = JunitTestHelper.createAndPersistIdentityAsUser("fi1-" + UUID.randomUUID().toString()); - identity2 = JunitTestHelper.createAndPersistIdentityAsUser("fi2-" + UUID.randomUUID().toString()); - identity3 = JunitTestHelper.createAndPersistIdentityAsUser("fi3-" + UUID.randomUUID().toString()); - dbInstance.commit(); + //get subscribers with 1 type + List<String> types = Collections.singletonList(publisher1.getType()); + List<Subscriber> typedSubscribers = notificationManager.getSubscribers(id, types); + Assert.assertNotNull(typedSubscribers); + Assert.assertEquals(1, typedSubscribers.size()); + + //get subscribers with 2 types + List<String> allTypes = new ArrayList<String>(2); + allTypes.add(publisher1.getType()); + allTypes.add(publisher2.getType()); + List<Subscriber> allSubscribers = notificationManager.getSubscribers(id, allTypes); + Assert.assertNotNull(allSubscribers); + Assert.assertEquals(2, allSubscribers.size()); } - /** - * @see junit.framework.TestCase#tearDown() - */ - @After public void tearDown() { - try { - DBFactory.getInstance().closeSession(); - } catch (Exception e) { - log.error("tearDown failed: ", e); - } + //markPublisherNews + + @Test + public void testGetSubscriptionInfos() { + Identity id = JunitTestHelper.createAndPersistIdentityAsUser("fi1-" + UUID.randomUUID().toString()); + SubscriptionContext context = new SubscriptionContext("Course", new Long(789521), UUID.randomUUID().toString()); + PublisherData publisherData = new PublisherData("Forum", "e.g. forumdata=keyofforum", null); + Publisher publisher = notificationManager.getOrCreatePublisher(context, publisherData); + dbInstance.commitAndCloseSession(); + notificationManager.subscribe(id, context, publisherData); + dbInstance.commitAndCloseSession(); + + //get infos + List<SubscriptionInfo> infos = notificationManager.getSubscriptionInfos(id, publisher.getType()); + Assert.assertNotNull(infos); } @Test public void testSubscriptions() { - SubscriptionContext sc = new SubscriptionContext("Course", new Long(123), "676"); + Identity id1 = JunitTestHelper.createAndPersistIdentityAsUser("fi1-" + UUID.randomUUID().toString()); + Identity id2 = JunitTestHelper.createAndPersistIdentityAsUser("fi2-" + UUID.randomUUID().toString()); + Identity id3 = JunitTestHelper.createAndPersistIdentityAsUser("fi3-" + UUID.randomUUID().toString()); + + SubscriptionContext sc = new SubscriptionContext("Course", new Long(123), UUID.randomUUID().toString()); PublisherData pd = new PublisherData("Forum", "e.g. forumdata=keyofforum", null); - SubscriptionContext sc2 = new SubscriptionContext("Course2", new Long(123), "6762"); + SubscriptionContext sc2 = new SubscriptionContext("Course2", new Long(123), UUID.randomUUID().toString()); PublisherData pd2 = new PublisherData("Forum", "e.g. forumdata=keyofforum2", null); - //Publisher p = nm.getPublisher(sc); - //assertNull(p); - DBFactory.getInstance().closeSession(); - - notificationManager.subscribe(identity1, sc, pd); - notificationManager.subscribe(identity3, sc, pd); - notificationManager.subscribe(identity2, sc2, pd2); - notificationManager.subscribe(identity1, sc2, pd2); + dbInstance.closeSession(); + + notificationManager.subscribe(id1, sc, pd); + notificationManager.subscribe(id3, sc, pd); + notificationManager.subscribe(id2, sc2, pd2); + notificationManager.subscribe(id1, sc2, pd2); - DBFactory.getInstance().closeSession(); + dbInstance.closeSession(); Publisher p = notificationManager.getPublisher(sc); assertNotNull(p); @@ -133,135 +384,252 @@ public class NotificationsManagerTest extends OlatTestCase { assertEquals(p.getResId(), sc.getResId()); assertEquals(p.getSubidentifier(), sc.getSubidentifier()); - boolean isSub = notificationManager.isSubscribed(identity1, sc); + boolean isSub = notificationManager.isSubscribed(id1, sc); assertTrue("subscribed::", isSub); notificationManager.notifyAllSubscribersByEmail(); - DBFactory.getInstance().closeSession(); - notificationManager.unsubscribe(identity1, sc); - DBFactory.getInstance().closeSession(); + dbInstance.closeSession(); + notificationManager.unsubscribe(id1, sc); + dbInstance.closeSession(); - boolean isStillSub = notificationManager.isSubscribed(identity1, sc); + boolean isStillSub = notificationManager.isSubscribed(id1, sc); assertFalse("subscribed::", isStillSub); notificationManager.delete(sc); + dbInstance.commitAndCloseSession(); Publisher p2 = notificationManager.getPublisher(sc); assertNull("publisher marked deleted should not be found", p2); } + @Test(expected=DBRuntimeException.class) + public void testDuplicateSubscribers() throws Exception { + try { + PublisherData pd = new PublisherData("CreateSubscriber@2x", "e.g. forumdata=keyofforum", null); + SubscriptionContext sc = new SubscriptionContext("Course", new Long(1238778567), UUID.randomUUID().toString().replace("-", "")); + Identity id = JunitTestHelper.createAndPersistIdentityAsUser("fci@2x-" + UUID.randomUUID().toString()); + Publisher publisher = notificationManager.getOrCreatePublisher(sc, pd); + dbInstance.commit(); + + ((NotificationsManagerImpl)notificationManager).doCreateAndPersistSubscriber(publisher, id); + dbInstance.commit(); + + ((NotificationsManagerImpl)notificationManager).doCreateAndPersistSubscriber(publisher, id); + dbInstance.commit(); + } catch (Exception e) { + dbInstance.rollback(); + throw e; + } + } + /** - * Test synchronized 'findOrCreatePublisher' triggered by method 'subscribe'. - * Start 2 threads which call 'subscribe' with same SubscriptionContext. - * Breakpoint at doInSync, second thread must wait until thread 1 has released the breakpoint. + * Test creation of concurrent subscriber */ @Test - public void testConcurrentFindOrCreatePublisher() { + public void testConcurrentCreateSubscriberWithOneIdentity() { + final int NUM_OF_THREADS = 100; - JMSCodePointServerJunitHelper.startServer(CODEPOINT_SERVER_ID); + PublisherData pd = new PublisherData("CreateSubscriber", "e.g. forumdata=keyofforum", null); + SubscriptionContext sc = new SubscriptionContext("Course", new Long(1238778566), UUID.randomUUID().toString().replace("-", "")); + Identity id = JunitTestHelper.createAndPersistIdentityAsUser("fci-" + UUID.randomUUID().toString()); - final SubscriptionContext sc = new SubscriptionContext("Course", new Long(1238778565), UUID.randomUUID().toString().replace("-", "")); - final PublisherData pd = new PublisherData("Forum", "e.g. forumdata=keyofforum", null ); - - final List<Exception> exceptionHolder = Collections.synchronizedList(new ArrayList<Exception>(1)); - final List<Boolean> statusList = Collections.synchronizedList(new ArrayList<Boolean>(1)); - - // enable breakpoint - - CodepointClient codepointClient = null; - CodepointRef codepointRef = null; + final CountDownLatch finishCount = new CountDownLatch(NUM_OF_THREADS); + List<Exception> exceptionHolder = Collections.synchronizedList(new ArrayList<Exception>(1)); + List<Boolean> statusList = Collections.synchronizedList(new ArrayList<Boolean>(1)); + List<SubscribeThread> threads = new ArrayList<SubscribeThread>(); + for(int i=0; i<NUM_OF_THREADS; i++) { + SubscribeThread thread = new SubscribeThread(sc, pd, id, exceptionHolder, statusList, finishCount); + threads.add(thread); + } + + for(SubscribeThread thread:threads) { + thread.start(); + } + + // sleep until threads should have terminated/excepted try { - codepointClient = CodepointClientFactory.createCodepointClient("vm://localhost?broker.persistent=false", CODEPOINT_SERVER_ID); - codepointRef = codepointClient.getCodepoint("org.olat.commons.coordinate.cluster.ClusterSyncer.doInSync-in-sync.org.olat.notifications.NotificationsManagerImpl.findOrCreatePublisher"); - codepointRef.enableBreakpoint(); - } catch (Exception e) { + finishCount.await(10, TimeUnit.SECONDS); + } catch (InterruptedException e) { log.error("", e); - fail("Could not initialize CodepointClient"); } - // thread 1 - new Thread(new Runnable() { - public void run() { - try { - NotificationsManager.getInstance().subscribe(identity1, sc, pd); - DBFactory.getInstance().closeSession(); - statusList.add(Boolean.TRUE); - System.out.println("testConcurrentFindOrCreatePublisher thread1 finished"); - } catch (Exception ex) { - exceptionHolder.add(ex);// no exception should happen - } - }}).start(); - - // thread 2 - new Thread(new Runnable() { - public void run() { - try { - sleep(1000); - NotificationsManager.getInstance().subscribe(identity2, sc, pd); - DBFactory.getInstance().closeSession(); - statusList.add(Boolean.TRUE); - System.out.println("testConcurrentFindOrCreatePublisher thread2 finished"); - } catch (Exception ex) { - exceptionHolder.add(ex);// no exception should happen - } - }}).start(); - - System.out.println("Thread point 3"); - sleep(2000); - System.out.println("Thread point 4"); - // check thread 2 should not finished - assertEquals("Thread already finished => synchronization did not work",0,statusList.size()); - try { - // to see all registered code-points: comment-in next 2 lines - // List<CodepointRef> codepointList = codepointClient.listAllCodepoints(); - // System.out.println("codepointList=" + codepointList); - System.out.println("testConcurrentFindOrCreatePublisher start waiting for breakpoint reached"); - TemporaryPausedThread[] threads = codepointRef.waitForBreakpointReached(1000); - assertTrue("Did not reach breakpoint", threads.length > 0); - System.out.println("threads[0].getCodepointRef()=" + threads[0].getCodepointRef()); - codepointRef.disableBreakpoint(true); - System.out.println("testConcurrentFindOrCreatePublisher breakpoint reached => continue"); - } catch (BreakpointStateException e) { - e.printStackTrace(); - fail("Codepoints: BreakpointStateException=" + e.getMessage()); - } catch (CommunicationException e) { - e.printStackTrace(); - fail("Codepoints: CommunicationException=" + e.getMessage()); + for(Exception e:exceptionHolder) { + log.error("Excpetion during concurrent subscription: ", e); } + + assertTrue("It throws an exception in test", exceptionHolder.isEmpty()); + assertEquals("Thread(s) did not finish", NUM_OF_THREADS, statusList.size()); + assertTrue("Subscriber does not exists", NotificationsManager.getInstance().isSubscribed(id, sc)); + } - // sleep until t1 and t2 should have terminated/excepted - int loopCount = 0; - while ( (statusList.size()<2) && exceptionHolder.isEmpty() && (loopCount<5)) { - sleep(1000); - loopCount++; + /** + * Test creation of concurrent subscriber + */ + @Test + public void testConcurrentSubscriberOperationsWithOneIdentity() { + final int NUM_OF_THREADS = 100; + + PublisherData pd = new PublisherData("MPSubscriber", "e.g. forumdata=keyofforum", null); + SubscriptionContext sc = new SubscriptionContext("MPSubscriber", new Long(1238778566), UUID.randomUUID().toString().replace("-", "")); + Identity id = JunitTestHelper.createAndPersistIdentityAsUser("fci-" + UUID.randomUUID().toString()); + + final CountDownLatch finishCount = new CountDownLatch(NUM_OF_THREADS); + List<Exception> exceptionHolder = Collections.synchronizedList(new ArrayList<Exception>(1)); + List<Boolean> statusList = Collections.synchronizedList(new ArrayList<Boolean>(1)); + List<MPSubscriberThread> threads = new ArrayList<MPSubscriberThread>(); + for(int i=0; i<NUM_OF_THREADS; i++) { + MPSubscriberThread thread = new MPSubscriberThread(sc, pd, id, exceptionHolder, statusList, finishCount); + threads.add(thread); } - assertTrue("Threads did not finish in 5sec", loopCount<5); - // if not -> they are in deadlock and the db did not detect it - for (Exception exception : exceptionHolder) { - log.error("exception: "+exception.getMessage(), exception); + + for(MPSubscriberThread thread:threads) { + thread.start(); + } + + // sleep until threads should have terminated/excepted + try { + finishCount.await(10, TimeUnit.SECONDS); + } catch (InterruptedException e) { + log.error("", e); + Assert.fail(); + } + + for(Exception e:exceptionHolder) { + log.error("Excpetion during concurrent subscription: ", e); } - assertEquals("It throws an exception in test", 0, exceptionHolder.size()); - assertEquals("Thread(s) did not finish",2, statusList.size()); - assertTrue("Subscriber does not exists for identity=" + identity1, NotificationsManager.getInstance().isSubscribed(identity1, sc)); - assertTrue("Subscriber does not exists for identity=" + identity2, NotificationsManager.getInstance().isSubscribed(identity2, sc)); - codepointClient.close(); - System.out.println("testConcurrentFindOrCreatePublisher finish successful"); + assertTrue("It throws an exception in test", exceptionHolder.isEmpty()); + assertEquals("Thread(s) did not finish", NUM_OF_THREADS, statusList.size()); + assertTrue("Subscriber does not exists", NotificationsManager.getInstance().isSubscribed(id, sc)); + } + + private class MPSubscriberThread extends Thread { + private final SubscriptionContext sc; + private final PublisherData pd; + private final Identity id; + + private final List<Exception> exceptionHolder; + private final List<Boolean> statusList; + private final CountDownLatch countDown; + + public MPSubscriberThread(SubscriptionContext sc, PublisherData pd, Identity id, + List<Exception> exceptionHolder, List<Boolean> statusList, CountDownLatch countDown) { + this.sc = sc; + this.pd = pd; + this.id = id; + this.exceptionHolder = exceptionHolder; + this.statusList = statusList; + this.countDown = countDown; + } - JMSCodePointServerJunitHelper.stopServer(); + @Override + public void run() { + try { + Thread.sleep(10); + for(int i=5; i-->0; ) { + //subscribe + notificationManager.subscribe(id, sc, pd); + + //mark as read + notificationManager.markSubscriberRead(id, sc); + + //update email date + Publisher publisher = notificationManager.getPublisher(sc); + Subscriber subscriber = notificationManager.getSubscriber(id, publisher); + List<Subscriber> subscribersToUpdate = Collections.singletonList(subscriber); + ((NotificationsManagerImpl)notificationManager).updateSubscriberLatestEmail(subscribersToUpdate); + + dbInstance.closeSession(); + } + statusList.add(Boolean.TRUE); + } catch (Exception ex) { + exceptionHolder.add(ex);// no exception should happen + } finally { + countDown.countDown(); + } + } } /** - * - * @param milis the duration in miliseconds to sleep + * Test synchronized 'findOrCreatePublisher' triggered by method 'subscribe'. + * Start 10 threads which call 'subscribe' with same SubscriptionContext. */ - private void sleep(int milis) { + @Test + public void testConcurrentFindOrCreatePublisher() { + final int NUM_OF_THREADS = 10; + + PublisherData pd = new PublisherData("Forum", "e.g. forumdata=keyofforum", null ); + SubscriptionContext sc = new SubscriptionContext("Course", new Long(1238778565), UUID.randomUUID().toString().replace("-", "")); + + final CountDownLatch finishCount = new CountDownLatch(NUM_OF_THREADS); + List<Exception> exceptionHolder = Collections.synchronizedList(new ArrayList<Exception>(1)); + List<Boolean> statusList = Collections.synchronizedList(new ArrayList<Boolean>(1)); + List<SubscribeThread> threads = new ArrayList<SubscribeThread>(); + for(int i=0; i<NUM_OF_THREADS; i++) { + Identity id = JunitTestHelper.createAndPersistIdentityAsUser("fci-" + i + "-" + UUID.randomUUID().toString()); + SubscribeThread thread = new SubscribeThread(sc, pd, id, exceptionHolder, statusList, finishCount); + threads.add(thread); + } + + for(SubscribeThread thread:threads) { + thread.start(); + } + + // sleep until threads should have terminated/excepted try { - Thread.sleep(milis); + finishCount.await(10, TimeUnit.SECONDS); } catch (InterruptedException e) { - e.printStackTrace(); + log.error("", e); + Assert.fail(); + } + + assertTrue("It throws an exception in test", exceptionHolder.isEmpty()); + assertEquals("Thread(s) did not finish", NUM_OF_THREADS, statusList.size()); + + for(SubscribeThread thread:threads) { + assertTrue("Subscriber does not exists", NotificationsManager.getInstance().isSubscribed(thread.getIdentity(), sc)); } } + + private class SubscribeThread extends Thread { + + private final SubscriptionContext sc; + private final PublisherData pd; + private final Identity id; + private final List<Exception> exceptionHolder; + private final List<Boolean> statusList; + private final CountDownLatch countDown; + + public SubscribeThread(SubscriptionContext sc, PublisherData pd, Identity id, + List<Exception> exceptionHolder, List<Boolean> statusList, CountDownLatch countDown) { + this.sc = sc; + this.pd = pd; + this.id = id; + this.exceptionHolder = exceptionHolder; + this.statusList = statusList; + this.countDown = countDown; + } + + public Identity getIdentity() { + return id; + } + + @Override + public void run() { + try { + Thread.sleep(10); + for(int i=5; i-->0; ) { + notificationManager.subscribe(id, sc, pd); + dbInstance.closeSession(); + } + statusList.add(Boolean.TRUE); + } catch (Exception ex) { + exceptionHolder.add(ex);// no exception should happen + } finally { + countDown.countDown(); + } + } + } } diff --git a/src/test/java/org/olat/restapi/NotificationsTest.java b/src/test/java/org/olat/restapi/NotificationsTest.java index f20ab7ee75c..07979d823e2 100644 --- a/src/test/java/org/olat/restapi/NotificationsTest.java +++ b/src/test/java/org/olat/restapi/NotificationsTest.java @@ -140,7 +140,7 @@ public class NotificationsTest extends OlatJerseyTestCase { if(!notificationManager.isSubscribed(userAndForumSubscriberId, forumSubContext)) { notificationManager.subscribe(userAndForumSubscriberId, forumSubContext, forumPdata); } - notificationManager.markPublisherNews(forumSubContext, userSubscriberId); + notificationManager.markPublisherNews(forumSubContext, userSubscriberId, true); //generate one notification String randomLogin = UUID.randomUUID().toString().replace("-", ""); @@ -285,7 +285,7 @@ public class NotificationsTest extends OlatJerseyTestCase { new PublisherData(OresHelper.calculateTypeName(Forum.class), forum.getKey().toString(), businessPath); notificationManager.subscribe(id, forumSubContext, forumPdata); Message message = createMessage(id, forum); - notificationManager.markPublisherNews(forumSubContext, null); + notificationManager.markPublisherNews(forumSubContext, null, true); dbInstance.commitAndCloseSession(); //get the notification @@ -328,7 +328,7 @@ public class NotificationsTest extends OlatJerseyTestCase { String filename = addFile(folder); //mark as published - notificationManager.markPublisherNews(folderSubContext, null); + notificationManager.markPublisherNews(folderSubContext, null, true); dbInstance.commitAndCloseSession(); //get the notification @@ -376,7 +376,7 @@ public class NotificationsTest extends OlatJerseyTestCase { new PublisherData(OresHelper.calculateTypeName(Forum.class), forum.getKey().toString(), businessPath); notificationManager.subscribe(id, forumSubContext, forumPdata); Message message = createMessage(id, forum); - notificationManager.markPublisherNews(forumSubContext, null); + notificationManager.markPublisherNews(forumSubContext, null, true); dbInstance.commitAndCloseSession(); //get the notification @@ -425,7 +425,7 @@ public class NotificationsTest extends OlatJerseyTestCase { PublisherData folderPdata = new PublisherData("FolderModule", relPath, businessPath); notificationManager.subscribe(id, folderSubContext, folderPdata); String filename = addFile(folder); - notificationManager.markPublisherNews(folderSubContext, null); + notificationManager.markPublisherNews(folderSubContext, null, true); dbInstance.commitAndCloseSession(); //get the notification diff --git a/src/test/java/org/olat/test/OlatTestCase.java b/src/test/java/org/olat/test/OlatTestCase.java index c10e76c7244..1d271558423 100644 --- a/src/test/java/org/olat/test/OlatTestCase.java +++ b/src/test/java/org/olat/test/OlatTestCase.java @@ -129,6 +129,12 @@ public abstract class OlatTestCase extends AbstractJUnit4SpringContextTests { DBFactory.getInstance().commitAndCloseSession(); } catch (Exception e) { e.printStackTrace(); + + try { + DBFactory.getInstance().rollbackAndCloseSession(); + } catch (Exception e1) { + e1.printStackTrace(); + } } } -- GitLab