Difference between VaadinSession.getCurrent() and VaadinService.getCurrentR

Hi everybody,

I need some advise about the safeness of what I’m doing. Besides reading the docs and
this related

couple of forum posts
, I’m testing it empirically, to an extent that hasn’t caused any problems yet.

Some background info on how I’ve come to this point:

My webapp uses user authentication. That is, the user has to log in in order to use it. By imposition, each one is not in a “users” table, but is a database user. When a user tries to authenticate in my app, a connection to the database is attempted with those credentials, and the successfulness of this operations is what determines if the user is logged in the app. Each app/database users will get different sets of data at the database level, which is going to determine the operations he’s allowed to do.

At the beginning, I was initalizing my database class as an instance object and was passing it via dependency injection to the classes that needed it. After a short time it became obvious that this wasn’t the best approach, bot for efficiency and for code cleanless.

When I tried to switch to a static connection connection pool which could be “pooled”, making connections available via the typical static getConnectionFromPool() (and also the typical returnConnToPool() after ending the database operations, and so on), I soon realized that this approach would not work for me because once the static pool object was initialized it was shared between all the instances of the app being accessed by different users. Replacing it with a newly created pool with different credentials would affect also each current user. In practice, a refresh (done by a background thread) in, let’s say, a table displaying some listing, would change the set of records being displayed for all users to the set of information available for the permissions of the last user that logged in to the app (that is, into the database).

After considering some solutions, I determined the easiest and quickest solution would be to change my database class to, instead of a single pool, use a set or collection of connections pools, one for each authenticated user. It could be the beast also in terms of efficiency, as it is allowed that the same user can use several instances of the app in several computers at once. Finally, I ended having a static HashMap of connections pools. I’d just need to change my code in the “getConnectionFromPool()” method (and so on) to access the HashMap and return the proper connection pool for the user and then grabbing a connection from it, as usual.
I was already storing the username in the session and using it for several operations, so my db pool HashMap is indexed by name, and my aim was to access it each time I needed a db operation by grabbing it from the session.

This was working fine until I realized some table/grid which refreshes periodically was throwing exceptions. I could track it down to the refresher thread. The code I was using everywhere for getting the username I stored in the session was:

VaadinRequest request = VaadinService.getCurrentRequest(); Then, once I had the request, I could easily try a .getWrappedSession(), and finally easily get the username attribute.

But when running this code from the background thread, VaadinService.getCurrentRequest() returns null, which is consistent with what it’s stated, for instance, in
the javadoc for this method

I remembered some other way of getting the session, so I attempted simply to use it in case VaadinService.getCurrentRequest() was returning null:


and after that, “again”, a .getSession(), to finally retrieve the username attribute from it.

Apparently, this is running fine. This is what the javadoc says about the getCurrent() method:

What does mean “at a point when the current session is defined”? Can I assume that if this method is returning a value (not null), it’s returning the “proper” session? I’ve never got a null from it, but my fear is that the returned session is not the correct one for that background thread. I’ve done extensively testing and don’t notice any “mix of information” in tables being refreshed with several different users, each one with different sets of results, logged in the app at once in several different browser instances… appart from the FIRST testing I did, in which I SEEMED to notice some “leaking” between users, although I’m not sure and haven’t been able to reproduce it.

Is this a safe approach? My refreshing code is inside the run() method inside a TimerTask() which is turned on at the beginning of the app. It looks like this:

countDowner = new Timer(); countDowner.schedule(new TimerTask(){ @Override public void run() { UI.getCurrent().access(new Runnable() { @Override public void run() { getANewResultSetAndPaintItInTable(); } }); } }, new Date(), 60000); As you see, the aim of this is to refresh an info table each minute.

In the end: is this guaranteed to work as expected?

Does enveloping getANewResultSetAndPaintItInTable() in a

UI.getCurrent().access(new Runnable() { @Override public void run() { // call to method here } }); block should provide any extra robustness / safeness? (I tried it already but didn’t notice any difference).

Do you have any other comments/suggestions about my approach?

I will not recommend to use UI directly in Timer process. Instead I would store UI reference in the class where you start Timer process, i.e. UI ui = UI.getCurrent(); And in timer process use ui.access(new Runnable() { … }); You cannot assume UI being threadsafe. Your Table refresh needs also @Push in order to work. Also note, that you need to stop the process somewhere. Threads are not cleaned up by Vaadin automatically. You could for example add service destroy listener and do stopping of the thread there. Otherwise you will have memory leak.

Hi Tatu and thanks a lot for your reply,

I still have doubts on how to accomplish your suggestions the proper way.

First of all: in order to apply your suggestions, the way I’ve used to pass the current UI reference to my TimerTask is the usual one which extends it, and in my extended constructor I add an UI parameter which sets a class UI field for it. Then in my overriden run() method I do this passedUI.access(new Runnable( // whatever ){}).

Second one: I’m using pushes (with @Push) successfully to the moment. Although I’m not sure if my refresher method for my data Table takes advantage of it as it “manually” “repaints” it by replacing all rows contents in case they have changed (and deleting rows in case they’ve become empty and also adding new ones in case they are needed). I understand that pushes work, for instance, for refreshing a ProgressBar (that I also use here - for visualizing the time left until my next Table refresh), but it’s not clear to me if it makes any difference when I “repaint” manually a Table the way I explained.

Third: I already was using a mechanism to stop my thread(s). What I was using was a DetachListener in the View that uses the thread that in its detach() method fires a call to .cancel() and .purge() methods of the thread. Is this a proper way?

Fourth: I don’t seem to notice any difference after putting the call to my refresher method inside my extended TimerTask with an explicit UI passed as a parameter. Is there a way to feasibly check if the thread is working “unsafely”? I mean… Is there a way to force a “corrupted” state, to see if it it’s effectively working fine or it’s just that I had the luck to not experience it?

Finally, I’ll go back to my initial message and reading again VaadinSession.getCurrent() javadoc it says that

My thread was being started inside a View. Do I have to understand from the previous quote that this thread is being started at a point when the current session is defined? Could this explain that in this case VaadinSession.getCurrent() was “working” and not VaadinService.getCurrentRequest() (the javadocs say, in this case, that “The current request can not be used in e.g. background threads”) and, in the end, that it doesn’t make a difference by explicitly passing an UI in this case?

Sorry for the added confusion…