|
I've finally undestood how pres.begin event is logged. When Sakai builds the page displayed to user, it includes an iframe with an invocation to presence tool (=servlet). This iframe is only displayed if display.users.present at sakai.properties is enabled. When this parameter is disabled, the invocation to the servlet is never done and, thus, pres.begin event is not registered. Presence tool servlet makes a call to PresenceService.setPresence which checks, for each user in the site, if pres.begin event has been stored at the ddbb and if don't, fires the event.
We've solved this issue by modifying SiteHandler under portal folder and invoking method PresenceService,setPresence from SiteHandler.doSite so that when the user visits the site a pres.begin event is logged. What you have to add between includeWorksite and portalIncludeButton at doSite method is the following: ... includeWorksite(rcontext, res, req, session, site, page, toolContextPath, "site"); //UPV - Generamos el evento de visita try{ org.sakaiproject.presence.cover.PresenceService.setPresence(siteId + "-presence"); }catch(Exception e){} //End - UPV portal.includeBottom(rcontext); ... We are going to spend today (and tomorrow, if needed) checking performance. I'll let you know. What do you think about this solution? I attach a patch for trunk version. I've tested it and found no performance problems.
I've added a new sakai property to configure if in an installation pres.begin events are needed to be logged or not. This new property is presence.events.log and is readed at SiteHandler. Trunk version r46803
Ian, is this one you could review and merge, if appropriate? Thanks.
I tested this feature on a local trunk instance and it is working properly. SiteStats was updated (trunk and 1.0.4 tagged) to look at this new property for enabling site visits (
Thanks. I'm happy the issue is fixed by this patch, but what strikes me is the introduction of an empty catch block. Either code can handle the exception, then the catch clause shouldn't been empty, or the code can not handle the exception, then there should not be a try/catch block at all.
You might get a harmless exception here if you don't have the presence impl in the build (e.g. cafe builds).
If you don't include event logging inside a try/catch block if, for any reason, an exception was launched client request would be broken because of the exception. That's why I included it inside the try/catch. But the problem was, what to do with the exception, then? Just keep processing client request. Maybe it should be inserted some message in the log. What do you think about it?
At least put a comment in for maintainability, this catch catches all exceptions which is very dangerous. See http://confluence.sakaiproject.org/confluence/display/SAKDEV/Best+Practices+for+High+Quality+Code#BestPracticesforHighQualityCode-Neverswallowexceptionsorleavecatchblocksempty
for sakai coding guidelines. Should have been closed earlier - I have verified this on local instances: "presence.events.log = true" enables logging of presence events as expected.
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||
 |
|
Bug
Closed
Major
Presence events are only logged if presence list is enabled
Our instructors who use stats like to know how often people are visiting the site, but we do not want display users present in the portal.
Right now, we're getting this functionality by:
1) Setting display.users.present=true
2) Setting "users = null;" before the presence tool iterates over users for display (currently on line 256 of presence-tool/tool/src/java/org/sakaiproject/presence/tool/PresenceTool.java)
3) Changing portal.css:
#presenceWrapper {
display:none;
}
It would be nice if there were a built-in way to achieve this.