Navigator7 add-on: API of Vaadin 7 ?

Hope that with such a thread title, we are going to have reactions and feed the not-very-active (on the site) debate about what API change developers need to address that:

http://vaadin.com/web/joonas/wiki/-/wiki/Main/Vaadin_7_API

I’ve taken a week to extract what I know and have done in that area (with the accumulated knowlege given by your great support on this forum), into a reusable framework that is running in production on BlackBeltFactory.com. I’ve taken an extra day to document it, in order to have more reactions about what API developers need.


Navigator7 is born
:
http://vaadin.com/directory#addon/120

Let me already answer the following question:

I started from the Navigator plugin (1 class). You will notice that I’ve kept the examples of it to ease the usage comparison.
I’ve added all the reusable code of BlackBeltFactory.com in that area. I ended up with a mini framework of a dozen of classes. It’s much more code because I address more areas (as URI analysis and page templating. Navigator7 also fixes a technical weakness of UirFragmentUtiliy not addressed by the original Navigator). I’ve been also much more aggressive/intrusive in the API changes for the programmer.

If you have anything to say about what you expect in Vaadin 7 to ease your life in these areas, please post a reply.

  • page navigation,
  • tabbed browsing,
  • URI parameters handling,
  • page templating.


Vaadin 7 by Joonas

You agree with me that Vaadin 7 will be a major new version with lots of enhancement ?
I’m not OK with Joonas about “maintain 100% backwards compatibility “. I think that it’ll be a brake to solid fearless and audacious evolution of Vaadin if we maintain 100% backwards compatibility.
What is your opinion about that ?

Good question Ramzi.

Navigator7 is an odd-on on top of Vaadin 6. By definition it removes nothing from Vaadin 6 and backward compatibility is 100% (it is just not using the add-on). If it would be integrated in Vaadin 7, it could be an addition of classes without much impact on the existing ones.

I would be to say that it’s a layer on top of the existing API. It’s obvious to me now Navigator7 exists, but it was not clear to me a month ago. It’s like having (sorry for the ugly comparison :wink: Struts2 on top of Servlet. Struts2 does not break Servlet/JSP. Some need Vaadin Window management API as it is now. But it is probably too low level and the answer is probably to leave the current Window management API untouched (or nearly), and to add a web navigation layer.

What needs to change is the documentation in that case: you don’t teach the low level (existing) API anymore (Servlet) but directly the higher level one (Struts2) to the masses. The sampler application, for example, if a perfect candidate to benefit Navigator7.

I think that you misread my proposal. The quoted part should be “Make it easier to implement the Container interface, but maintain 100% backwards compatibility.”. The API I proposed for windowing in Vaadin 7 is far from being 100% compatible. See
http://vaadin.com/web/joonas/wiki/-/wiki/Main/Vaadin_7_API

Thank you Joonas for response. I understand now.

The word “Application” for the com.vaadin.Application class sounds counter intuitive to me.
When I explain to people that it’s in the HttpSession, everybody understand its scope.
But the word “Application” suggest the application level scope (or ServletContext, or Spring ApplicationContext,…), while there is one instance per browsing user.

I think that:

  • the word is misleading at that place
  • we need, in Vaadin, a real web application scoped object.

With Navigator7, I’ve stored a reference to the NavigatorConfig and to the UriAnalyzer in the com.vaadin.Application instance.
It’s a mistake. There should be one instance of these two classes for the whole web app, and any class of my application should have an easy access to them.

I’ll turn them into singletons in the meanwhile.

Hi John!

Just a comment to your thoughts about the Application scope:

I think too that it is a bit counter intuitive if you come from the world of web applications. People are used to that applications larger than just one session - more or less shared with all users.

I have always used the following approach when explaining this:

  1. Application class is a “template” for application.
  2. A new instance of an Application (sub-)class is created for every user.
  3. Good thing here is that all things you store (in fields) in the application instance are for that one user only.
  4. For web developers, an instance of an Application maps closely to HttpSession. There is ApplicationContext that is more precisely a session and that can map to instances of several applications.

Of course, all this makes more sense when considering desktop applications where there (typically) is no more than a single user per application.

I introduced a new class in the framework: WebApplication.
It’s a ServletContext scoped application. There is a name clash with the com.vaadin.Application.
That’s now Navigator7 v7.1

The more I think about it, the more I beleive that the “application” defined in the web.xml as init parameter for the vaadin servlet, should be ServletContext scoped (as my new WebApplication), and users should override it, instead of the HttpSession scoped com.vaadin.Application.
If we take the Navigation7 stuff apart, what is com.vaadin.Application instances used for?

  1. to hold references to Window objects. Is it ok to store that in the HttpSession? Probably.

  2. to define the algorithm for creating the main window. It’s probably no user-specific algo => It should probably moved in the ServletContext scoped application.

Now, with Navigator7, user application code (pages) don’t need the com.vaadin.Application (or their descendant). They need the main Window (to template it) and the UriAnalyzer (ServletContext scoped now).

As summary, I’d rename com.vaadin.Application to com.vaadin.UserContext, and make it less central in the design, with things moved to the ServletContext scoped WebApplication. People would extend WebApplication and tell the new class name in the web.xml (init param of Vaadin servlet).
I’ve not changed Navigator7 to that point, it’s not goal (at least not before an common agreement after discussion).

I just did it in fact. I’ve extended Vaadin’s ApplicationServlet. Release 7.15

Appreciate the work you done, did some testing earlier with navigator but you provide a more complete solution.

A suggestion, since it’s currently hard to secure pages with spring security since every url is one page. I would like to unregister pages once I a user logged out/logged in.

So some more methods to update/remove page sets methods on WebApplication would be useful ! Using this project in
http://cia.sourceforge.net/

Best regards Pether

Hi Pether,

What about adding some kind of filter/interceptor mechanism in the Navigator?

That way, you could put your code to decide if the navigation is done or not (or redirect to another error page). You could even annotate your page with some @Authorization(role=“manager”) annotation that your interceptor would check.

Someone would probably provide a reusable interceptor wrapping Spring security.

What do you think?

Use annotations in the service layer, so would be nice. Will have to solve it some way…

Some questions about the WebApplication class.

Why store “ThreadLocal currentServletContext” in thread local. if only one instance should be in the application. Otherwise i could use spring annotations for creation of the application impl without setting up the servlet context.

The use of “public static WebApplication getCurrent()” that creates some problem.

Pether

Because the (not wonderful) Servlet API gives you no way to get the curreent ServletContext out of the blue. You can get it from a Filter and from a Servlet. And we manipulate no filter nor servlet in our Vaadin application components, thus we don’t have access to the ServletContext.

I’ve found the VaadinServlet (NavigableApplicationServlet extends ApplicationServlet) a convenient entry point to catch the ServletContext and store it somewhere. Better ideas are welcome.
I cannot store the ServletContext into a WebApplication instance because I need the ServletContext to retreive WebApplication instances.

There is one ServletContext instance per web application => It’s not very clean to store it in a regular static variable.
With threadLocal, I remember the correct reference for each thread. If the Servlet API could tell me “for the current thread, this is the ServletContext” it would not be necessary. That whole notion of multiple web app / ServletContext in the same JVM is probably a mistake of the Servlet API. We have to live with it :wink:

I don’t understand what you mean. The WebApplication class is instantiated by the NavigableApplicationServlet. Do you bypass the NavigableApplicationServlet?

I don’t use the NavigableApplicationServlet, depend on springapplicationservlet for some functionality so created a new servlet.

Coded

http://cia.sourceforge.net/xref/com/hack23/cia/web/impl/ui/application/ApplicationServlet.java
, that works fine.

I always run the same application so not as flexible as http://code.google.com/p/navigator7/source/browse/trunk/Navigator7/src/org/vaadin/navigator7/NavigableApplicationServlet.java .

Pether

Ok, you don’t use Vaadin ApplicationServlet that my NavigableApplicationServlet extends, but the servlet of a Spring related add-on.

I’ve moved the my code from NavigableApplicationServlet to WebApplication. Now NavigableApplicationServlet is minimal, and you can easily adapt your servlet extending SpringApplicationServlet to initialize WebApplication correctly.
See NavigableApplicationServlet as an exemple and copy/paste the 3 lines of code:


WebApplication.init(servletConfig, getServletContext(), getClassLoader());
WebApplication.beforeService(request, response, getServletContext());
WebApplication.afterService(request, response, getServletContext());

I’m uploading v7.16 that includes these changes.
Good luck, and don’t hesitate to click 5 stars on the add-on if it’s useful for you.

http://vaadin.com/directory#addon/120

John.

Thanks for the update, changed my code and works excellent now.

Rated the plugin as well. Saved a lots of time and more advanced than the original navigator, so happy to give it 5 stars :slight_smile:

Pether

I’ve added an interceptor mechanism.
Here is the Interceptor interface and doc:


/** 
 * An interceptor is a stateless class that follows the interceptor pattern, as found in Filter and in AOP languages.
 * Interceptors are objects that dynamically intercept Page invocations. They provide the developer with the opportunity to define code that can be executed before and/or after the execution of an action. They also have the ability to prevent a page from being invoked. 
 * Interceptors provide developers a way to encapsulate common functionality in a re-usable form that can be applied to one or more Pages.
 * Interceptors must be stateless and not assume that a new instance will be created for each request or Page. 
 * Interceptors may do some processing before and/or after delegating the rest of the processing using PageInvocation.invoke().
 * 
 * Navigator7 Interceptor differs from Struts 2 Interceptors and Servlet Filters the following ways:
 * - There is one Navigator7 Interceptor invocation for each NavigationEvent (=> once when we move to the page and instanciate it), while Filters and Struts 2 Interceptors are called for every request (included when you click a button on a page).
 * - Navigator7 Interceptors are not configured into an external xml file, and you cannot define a set of pages for which they should be applied and others for which they should not be applied. Just program it inside the interceptor if you need to activate it for specific pages. This should probably change if we want to use reusable interceptors written by others only for specific pages, but it's not the case yet. Let's keep it simple for now. 
 *
 * To have an idea of what interceptors can be used for, see Struts 2 documentation (while some examples are meaningless in a Vaadin context): 
 * http://struts.apache.org/2.x/docs/interceptors.html
 * 
 * A typical usage would be security: the interceptor can check the logged-in user in the session, and can check annotations on your Page classes, and forward to an error page in case of mismatch.
 * 
 * Register your interceptor in your class extending WebApplication, by calling WebApplication.registerInterceptor().
 * 
 * 
 * @author John Rizzo - BlackBeltFactory.com
 *
 */
public interface Interceptor {
    public void intercept(PageInvocation pageInvocation);
}

In the initial Navigator, Joonas did implement a dialog box that asks the user to confirm before leaving some pages.
It was there (in a more generic form) in the Navigato7’s Navigator class. But I didn’t feel it was at the right place, it was too close form the heart.
I’ve transformed it into an interceptor and it’s much better now on the design point of view. It also gives you a non trivial example of Interceptor :wink:
The example application has been updated as well to show how to register interceptors.

It’s in version 7.2, available now.

Let’s continue this Vaadin-web-7 architecture discussion :wink:

Currently Interceptors are called before the page instantiation. Maybe the page wil never be instantiated.
It might be nice to have the page instance in interceptors because we may like an interceptor to do some automatic value injection in the page, for example.

Struts2 Interceptors can do that because they have access to the Action instance that is instantiated before the interceptors chain is called.
On the contrary our Vaadin Interceptor has no page instance, because:
1- it’s heavier to instantiate a Vaadin page than a light Struts2 Action, and we are not sure we’ll need the page at all (an interceptor may decide to stop the call chain).
2- Struts2 has the Action.execute() method to call at the end of the interceptor chain. We don’t have that in Vaadin. Our (non request scoped) “equivalent” is to instantiate the page (which is a VerticalLayout containing much stuff, or something similar).

A solution to have the page instance in the interceptor is to have our pages implementing a Vaadin interface. (I know, PageInterface is a bad name and it should be “Page”, it’s just avoid confusion in this post)

interface PageInterface {
    Component init(); // result placed in the AppLevelWindow.
}

The navigator would:
1- Instantiate our page (i.e. new HomePage implements PageInterface)
2- Call the interceptors chain (interceptor may inject values based on annotations for example)
3- Call page.init

The disadvantage of introducing PageInterface is that pages cannot be Vaadin Components anymore (instead they contain a main component, as CustomLayouts do). It’s more intrusive. I’m not fan of it.

To summarize, we have 3 options:
A. Pages are instantiated after the interceptor chain is called. Interceptors have no access to the page instance. It’s the implemented option.
B. Pages are instantiated before the insteceptor chain is called => Interceptors have access to page instance.
B1. and they are normal Vaadin component => much of the UI building is done in the constructor => it would not benefit (at constructor time) value injections done in the interceptor called afterwards => BAD OPTION.
B2. and they implement PageInterface, and kindly put the UI building code in the init method (can we really count on programmer discipline?)

My preference order is:
A. (but we cannot inject values in page instances)
B2.

Whatever you do in Vaadin 7, you probably want think about that aspect carefully.

To add naming consistency for web people, I’d rename TransactionListener to “RequestInterceptor” (if responsible to call the next RequestInterceptor) or “RequestListener” (if as now, has start/end methods).
The current interceptor should probably be renamed NavigationInterceptor (to make people understand it’s not called for every request :wink:

John.

In fixed the problem: the PageInvocation has now a lazy getPageInstance() method. If called by an interceptor who needs the instance, it creates it.

As simple as that :wink:

It enabled me to implement parameter injection:
http://vaadin.com/forum/-/message_boards/message/177821

On BlackBeltFactory (using Vaadin and Navigator7), when I come back to my browser and click a link in the menu, I often get this provocating Vaadin top red box “Session Expired”. Any Vaadin developper/user know what I mean. When you see that on a web site, you tell “ah, yet another Vaadin web site”. It’s sad that this redbox is associated to Vaadin brand, isn’t it?

Certainly, if I’m triggering some action within a page or a dialog box (as a button ClickListener invocation), and that the page, dialog box and button objects are gone with the expired session, then there is nothing else to do than to apologize to the user.

But, in the case that Vaadin is aware of page navigation (purpose of Navigator 7), it may detect that we want to move to another page. If we move to another page, not showing any error and silently renewing the session would be much more user friendly (as a default behavior, even if there is an option to change it).

Hope you will consider doing that if you integrate something like Navigator7 in Vaadin 7.