Some things that could be addresssed before Action 2.0.0. (If we don't address them, we'll be stuck supporting them throughout all eternity or until Struts 3.0.0, whichever comes first. ;))

We have a small number of existing WebWork users compared to the number of Struts users we'll (hopefully) eventually have. This is a new framework (if only in name) and a major release. This is our one chance to break compatibility. We must get it right now, because we will *not* be able to fix most of these problems later (the cost of breaking existing Struts users will almost always outweigh the value of the fix).

We do not need to expose Struts users to XWork; they do not care. At the very least we should build a thin abstraction layer to isolate users from XWork. XWork should be an implementation detail not part of the SAF API. We can make most of the following changes in SAF's abstraction layer and avoid breaking existing XWork users.

Patrick's issues

My concerns aren't as detailed as Bob's, but I would like to see these general themes promoted by the framework:

  1. Use a lot more conventions with easy overrides.
  2. Don't dismiss XML entirely - annotations are nice but currently can't be HotSwapped (due to a bug in the JDK). For any configuration, we should read in the following order: XML, annotations, convention.

    • [jcarreira] Shouldn't annotations be the default, and XML be the override?
    • [crazybob] I think that's what he means. Speaking of annotations, I've yet to see a method for representing result mappings using annotations that I actually like (due to limitations of annotations). If we can't come up with something decent, I'd just assume stick with XML; we shouldn't use annotations for the sake of using annotations. I personally don't find the xwork.xml as annoying as XML in other places. If we do simple things like defaulting the action name to the simple name of the action class, it will be even more pleasant. I definitely think we should use annotations for things like validation.
    • [frankz] I for one have zero problem with annotations being an option, even being the default, but do keep in mind that not everyone sees annotations as really being that great of an idea. I acknowledge it might the minority view now, but I for one see it as configuration information scattered throughout the code base, rather than in one known location (read: XML config file), so speaking for myself, I am not entirely sold on annotations being superior to XML config files (assuming the config files aren't overly complex that is!)
    • [phil] I'd like to be able to reconfigure my application without the need for recompilation. If annotations support that (or if we're using an xdoclet/generator approach), then I'm all for it. Otherwise, keep the xwork.xml file - it's clean, simple and to the point.
    • [tm_jee] I agree with Phil, I still like configuring through configuration without recompilation.
  3. Fail fast with detailed error messages, ideally ones that show you what you did wrong and what you should to.
  4. Address the confusing issue of the validation/workflow lifecycle and different methods (this is mentioned in more detail above, but it is something that is problematic). Right now we sort of hack it by making the "input" method a special case in webwork-default.xml.
    • [jcarreira] +1 : Carlos at G**gle had some good ideas for this... basically stuff like if your action method is foo() then you'd have prepareFoo() and validateFoo(), but then I added that the prepare() and validate() methods should be the defaults that we call for all action methods.
    • [crazybob] Interesting idea. Might be overkill (i.e. at that point, the user should probably create another action class).
    • [hani] No magic method names. If you want to do that, use annotations so you have a good chance of IDE support dealing with it. For example @Validate/@Prepare etc, with parameters to indicate what request you'd like it involved for, in the case where you need multiples of them
    • [jcarreira] I think RoR has shown that convention over configuration is liked by lots of people... these should be straightforward to figure out without annotations.
      • [crazybob] Actually, Ruby doesn't have annotations, but Rails leans heavily on the :foo syntax to emulate them.

  5. Don't encourage lots of interceptor stacks. Ideally the normal user should never need to deal with them. It is better to have a larger stack that has optional features that could be turned on through annotations or marker interfaces than to encourage users to build their own stacks.
    • [jcarreira] I think we should have some pre-defined ones for standard things: view vs. CRUD vs. "action" -> do somthing that's not CRUD. We should then use annotations to make it where you can declaratively associate a particular action method with a "stereotype" which is mapped to an interceptor stack, etc.

    • [crazybob] "C[R]UD" isn't a good name because "view" == "[R]etrieve". Let's call it "update" for the moment. What will the difference be between "update" and "action"?
    • [plightbo] I really don't think having a "update" and "action" stack is a good idea. Let's make a single stack and then have the stack behave differently depending on the context in which the action was executed. There are several options (GET vs POST, method names, annotations, etc).
    • [jcarreira] I really think this is a bad idea and will lead to great suffering... We put in interceptors to allow people to customize the processing of a request, why move away from that now? I think if we do some thinking we can think of the common use-cases that cover 80+% of the action cases, and make it very easy for an action developer to tag this action as using one of that small number of stereotypical uses. The action will then pick up the configuration set up for that stereotype (which could include things like defaulting the result pages to go to, etc.)

Gabe's Issues

  1. Simpler XML Configuration of actions. Ted mentioned adding wildcard support for action names something like name="view*" where the wildcard can then be used elsewhere as a variable. Another idea is allowing one action configuration to extend another or having default action configuration that other action configurations can use.
  2. Add the possibility of setting to the OgnlValueStack rather than pushing so we can get rid of using the context for user app variables and reserve it for framework variables. The user then wouldn't need to know anything about the context, just the stack. Also, this allows us to get rid of the '#' sign completely in expressions. Similarly remove the push tag to simplify the API. More detail here: http://jira.opensymphony.com/browse/XW-329 and here: https://issues.apache.org/struts/browse/WW-1133.

    • [plightbo] I still don't know about this. My biggest concern is being able to do a fairly standard pattern of pushing an object on the stack (User) and then including a common snippet such as user-details.jspf. Without the stack and the ability to push, we might loose the loose coupling the value stack provides.
    • [Gabe] Just wanted to make it important to clarify that I am not suggesting that we remove the ability to push on the stack, just remove the push tag to do it within JSPs. Thus, in your Interceptor etc you would still be able to push. Also, allowing the ability to set on the stack is a seperate issue. It would be powerful to be able to add dynamic names to the stack rather than only be able to use static method names to an object pushed to the stack.
    • [jcarreira] -1 to removing the push tag. It's very useful to push an object onto the stack and include a template to render it. +1 to making it easier to bind values to names that are easily accessible to the stack.
  3. Hope I know what I'm talking about with this one: Provide a way that request parameters can be used as a form element value in case of error. If you submit a form with a text field that requires a numeric value but you enter a non numeric value and errors are returned, you lose the value entered when the type conversion happens.
    • [plightbo] We support this already (the value stack has an "overrides" map which gets set when type conversion errors occur). If this isn't the case, it's simply a bug :)

  4. Remove OGNL Map attributes (and List/Set to be consistent) such as size, isEmpty, iterator. These can be accessed by size(), empty, and iterator() respectively and the way it works now you can never have myMap['size'] because it will just get the size not the value of the map with key 'size'.
    • [plightbo] +1, all I'd ask is that we try to make it feel as much like JSTL as possible. Not sure what that means, but just something to keep in mind :)

    • [Gabe] I agree. OGNL is very much like JSTL already. I wonder if we can even add the few JSTL features OGNL doesn't have to OGNL and then position OGNL as an EL that "extends" JSTL.
    • [jcarreira] +1 to making OGNL a superset of JSTL, unless it involves trying to be "bug-compatible".
  5. Allow indexable parameters similar to how it works in struts (with indexed="true") but being able to take advantage of XWork's advanced type conversion features. See: https://issues.apache.org/struts/browse/WW-1189. This is unfortunately not trivial at all.

  6. Get rid of the use of static constant variables that are used in the key in the stack and accessed all over the place like XWorkNullHandler.CREATE_NULL_OBJECTS etc. I've started to do that with the OgnlContextState class, but it's not complete and I'm not sure if that's the best way to do it.

  7. Specify and simplify Interceptor scope. Currently, you have an Interceptor that calls actionInvocation.invoke() and then returns a different result than actionInvocation.invoke() returns, the actionInvocation.invoke() result will be used anyway. This is confusing and muddies the meaning of the Interceptor API, which IMHO should simply wrap the action not the action all the way through to the end of the result. The reason it's set up the way it is, as I understand it, is so that Interceptors can clean up resources like connections after the result is returned. However, I wonder if we can implement a request based object that can take care of such resources and destroy them at the end of the request rather than using Interceptors in this way.
    • [crazybob] That was really surprising and confusing to me at first, too. I thought it would have been more intuitive for the result to run after all the interceptors returned. I'm not sure whether we should change it or not. I like the idea of interceptors being able to clean up after results a lot more than I like the idea of an interceptor being able to return a different result.
    • [Gabe] It is an advantage for Interceptors to be able to clean up at the end of a request, but it isn't great how they do that either. Take for example an action chain. If you have a create connection Interceptor surrounding each of the chained actions, you will open two connections, which besides being wasteful could cause problems with other resource types. I wonder if we can create some sort of request scoped ResourceManager class that can allow Interceptors to create resources or access them if they exist and specify how they should be destroyed at the end of the request. Thus in the connection case, the Interceptor could check if the resource manager had one and if not create it and add it to the resource manager for other objects to use. (Another option of course is an inversion of control approach)

    • [jcarreira] Interceptors can still change the result... Implement PreResultListener and in your callback, change the resultCode and voila! The result executed will be changed. The PreResultListener interface lets you register your interceptor to get a callback after the action and before the result is executed. Oh, and on the ConnectionInterceptor -> It's just like AOP. You have to check if it's been done already and know not to create a new one or close it on the way out. I do this all the time in AOP interceptors, so why should this be different? Personally, I'd rather use the same connection across all of the actions in a chain than clean it up after each one and use a new one per action. For request scoped resources, take a look at Spring's scoped components. I'm using them at work and they work pretty well (a few issues I'm working through with them notwithstanding).

    • [Gabe] The question is not about functionality but about simplicity of implementation. Can we, for example, allow the return from the intercept method to change what is executed regardless of whether it is returned before or after actionInvocation.invoke() and get rid of PreResultListener? IMHO that would positively simplify things. I'm not too particular about whether destroy() is called before or after the result is executed to clean up resources.

Tim's Issues

I'm new around here, so be nice ;) I probably have a lot less WW experience than most, so I apologize in advance if I'm flat out wrong about some of the things here.

  1. How does WW help the user with state management? As far as I can tell, if I want to keep a 'user' object around I have to interact with the map returned by ActionContext.getSession(). Actions should in general have a type-safe and transparent way to do this, e.g. by subclassing ActionContext and providing getUser()/setUser() which store the user in session. This allows for re-working of the storage strategy (e.g. write a cookie and lookup the user each time) without affecting actions.

    • [crazybob] I prefer an injection-based approach. You can use the ScopeInterceptor to pull an object off the session and pass it to your action. Or you can use Spring to inject session-scoped objects into your action (though I would avoid Spring personally).

    • [jcarreira] I can attest that the Spring scoped components work well with WebWork. It's what we use at work for maintaining session or request state.

    • [plightbo] Let's not dismiss Tim's comments too quickly. While we might not implement a solution exactly like he suggests, his point is valid that handling state management in WebWork has always been a very week area. The ScopeInterceptor isn't a great option either, especially considering one of my other issues specifically asks that we avoid having to create custom interceptor stacks for various actions.

    • [jcarreira] I'm open to hearing good ideas for state management. Especially short-lived wizard type state. Long lived session state (things like your user profile) work really well as Spring session-scoped components, but state which is just used for the next 4 or 5 pages isn't such a good fit.
    • [MJ] I think talking in terms of pages does not really get us further from ancient SAF1 practices. A web resource can have one view (page) or ten. A wizard can have ten pages defined, but it can use only 3 or 5 of them depending on current state and transitions. It is better to think in terms of web resources, and when it goes out of scope. A wizard is a distinct web resource, who cares how many pages it has. I have built a wizard engine a year ago, and it proved its viability and robustness. No XML, by the way. Check the sample. Try to break it with Reload, Back and Forward buttons. The wizard code hasn't been changed for almost a year, which shows the maturity. I can provide the code and the docs if there is interest. I know that Tim as well as Jason dislike session-scoped data. I hope you guys change your point of view.

    • [MJ] In regards to wizards and Redirect-After-Post pattern, does Webwork have something like FlashScope? FlashScope works for only one redirect call, so there may be something longer than FlashScope, but shorter than session. At best, it would be great if a stateful component like a wizard, could be assigned to its own FlashScope+ .

  2. In tandem with the previous point, since Actions are already stateful, it'd be nice to have the ActionContext injected into the Action. One benefit is when a newbie developer needs it, the linkage is obvious (they don't have to a priori know about the ActionContext, they're being handed in it on a platter). If the developer can subclass ActionContext, it would also encourage them to implement a base action which accepts the context inject and leveraging the fact that JDK 1.5 allows co-variant returns, also write a getContext() method that returns the down-casted type; they wouldn't have to do ((MyActionContext) ActionContext.getContext()).getUser() for example, just getContext().getUser().

    • [frankz] This might well address the issue of ActionContext being ThreadLocal. If it was injected, it wouldn't need to be ThreadLocal to get the same basic effect, and maybe more importantly, it wouldn't automatically be available to helper classes as it is as a ThreadLocal. That would address my concern about "inappropriate" usage of ActionContext.

    • [jcarreira] I think this is a bad idea, in general. Actions should specify the exact things they need and have them supplied, not just ask for the "world" (the ActionContext is the world the action lives in).

    • [mrdon] While I agree more specific is generally better, I like the idea of the user being able to subclass ActionContext for their particular application. Tapestry has the Visit object (I think that's the name) I've always liked.

    • [plightbo] Tim's suggestion of allowing actions to get the ActionContext without referencing the thread local is a good one. This is in line with other requests to reduce the number of situations where users have to deal with ThreadLocals.

    • [tm_jee] I think its ok, if we allow action to implements say ActionContextAware and have an interceptor "inject" the ActionContext into it.

  3. HTML analog tags should stick to HTML attributes. I don't mean they shouldn't have more functionality, but the attributes should be identical where possible, and they shouldn't do things like render a label and an input. Keeping them more like regular HTML tags makes them easier to ramp up on, and more non-developer friendly
    • [MJ] I see the following options when it comes to tags. (1) Use plain HTML + implicit scoped variables like "actionName", "actionAddress", etc. to create dynamic values; this looks pretty compact with JSP 2.0. (2) Use 1:1 relation between WW tags and HTML tags. (3) Use 1:M relation between WW tags and HTML tags, like to create data entry form or a table. (4) Use non-HTML-looking tags + more abstract attributes + "media" attribute, thus creating something like JSF renderer for different media. Choosing between (1) and (2) I prefer the first one.
    • [jcarreira] I'd encourage people to give the ww: tags a spin... they're really much more powerful than the JSTL or previous struts tags and you don't need so many tags to do things. On being closer to HTML attributes, do you have some examples?
    • [mrdon] +1 for aligning attributes with HTML attributes
    • [tm_jee] -1. If I'm not mistaken I think WW's tag are doing stuff in a broader perspective, typically in a theme perspective and I hope we could still keep it that way, if possible. A textfield for example, merely says its a textfield, how it behaves is entirely up to the particular theme. Normally, one would extends from a parent theme, and overide those components' template that they want alteration accordingly or introduce other templates that could co-exists with their counterparts to skin the component.
    • [rainerh] Big -1... The ww tags are no dynamic replacement for simple HTML tags, but a complete set of UI components. With these themeable components small and large webapps can be easily build. All (important) HTML attributes are supported, not for the simple tags, but for the complete component.
  4. Actions should return concrete objects, not symbolic results. Symbolic results might have been optimal when you had one event/method per action and the outcomes were always whole-page views, but they get in the way now. When you want to return anything that requires more than the symbol, you have to do some less than intuitive things to make the Action and the Result cooperate. I'd prefer to see a concrete Result get returned from Action methods, which would allows developers to do more powerful things more easily. There are a bunch of ways to make it backward compatible too. You could return 'new SymbolicResult("success")' and have the SymbolicResult do the lookup stuff (You could even redefine the String constants to be SymbolicResults). You could alternatively use a static class to do Results.find(SUCCESS). Or you could even allow method to continue to return String or Result, and if String wrap it in a SymbolicResult.

    • [frankz] +1. This is one area where I personally think Struts had it right and we've seen frameworks getting it "wrong" subsequently. ActionForward I believe is the right concept, even if the realization might not be optimal. I think the difference between return "ok"; and return new ActionResult("ok"); is pretty minimal, but the later opens up a lot of possibilities being a true object that can have behaviors and properties and such.

    • [crazybob] There's no reason we can't support both String and Result return types for action methods. I think we should encourage String though. Can you provide some example use cases for when String isn't enough?

    • [frankz] A few that come to mind... the ability to choose between redirect and forward based on decisions made in code (you could argue that you can have two <result>'s defined, one with type redirect and one with type forward, but then that starts to clutter the config file I think)... the ability to add parameters to a forwarded result (i.e., if you wanted to forward to another action, how can you add parameters just by returning a String? Well, unless the processor expects and can deal with a query string appended I support)... the ability to add an anchor name to the response... the opportunity to include additional objects in the response (i.e., what if you had some sort of ViewSupport object that the presentation can make use of... it might be more intuitive to add it to a Result than to directly add it as a request attribute, and that also makes for better abstraction from the servlet API). I'm sure there's more, and I'm sure there are ways to accomplish most of these things without returning an object, but I think the clarity of the resultant code is greater by returning a Result object, and it opens up possibilities I can't think of (i.e., functionality encapsulated in that object independant of tbe interceptor stack).

    • [mrdon] For further reading: http://forums.opensymphony.com/thread.jspa?threadID=23621&tstart=45

    • [crazybob] I'm not sold on this. 1) The result pulls values from the value stack, not the request attributes. 2) I ran into a case recently where I thought I wanted to return a real object at first: I wanted the action to dynamically select a redirect URL. I ended up doing this:

        <result name="..." type="redirect">${url}</result>

      3) Enabling users to return either a real object or a string is going to impact the interceptor API, too. :( 4) Wait a second... you can do what you want right now! Create a new result type which looks up a Result object on the value stack and delegates to it. Then map a global result to the new type. For example, you could return CUSTOM, and the the result type would execute the Result returned from getCustomResult() on your action. ;)

    • [jcarreira] -1 I've never, ever seen anything that needed this, and it makes the API requirements on the action classes more tightly coupled. Result configurations and Result instances can pull values from the value stack (including but not limited to action properties) so you can easily set up any of the values you want to give the result as properties of your action and have them pulled.
    • [mrdon] I'm still in favor of this idea, because it allows a more code-centric style of programming, without giving up any backwards compatibility. For example, consider an app that uses wildcards in its xwork.xml to define common patterns, however each submit action is going to be redirecting to somewhere else. Without adding a new action in the config, you want to just return a redirect result. Yes, you can do that now by putting values into the stack then pulling them out in the definition, but I think it is cleaner to be more explicit:

       Result result = ActionRedirectResult("bar", "foo");
       return result;

      rather than

       ActionContext.push("actionName", "foo");
       ActionContext.push("actionNamespace", "bar");
       return "redirect";
       ...
       <result type="redirect-action">
         <param name="actionName">${actionName}</param>
         <param name="namespace">/${actionNamespace}</param>
       </result>
      My point is this feature enables a new style of development, one closer to RoR and, IMO, easier for the newbie, without affecting anyone else.
    • [crazybob] Assuming you have a getResult() method and you've defined a global result, your second example could look like this:

       result = ActionRedirectResult("bar", "foo");
       return CUSTOM;

      A Ruby on Rails analog would look like this:

        public void execute() {
          ...
          if (foo) {
            redirectToAction("foo", "bar");
          }
          else {
            forwardToAction("tee");
          }
        }
      If the user doesn't call a result method, we would use an intelligent default. You could implement this using an interceptor and an action support class. However, I'm with Jason: I've never needed this and I like the seperation.
    • [plightbo] Bob: your suggestion doesn't seem very "user friendly". I think we should really think long and hard about doing this. I'll be the first to admit that much of the reason I never thought about doing it in WebWork was purely to be "anti Struts" (sorry!). I've grown up a bit since then. The downsides I can see to this are: 1) more confusing if we offer both options, 2) views are tightly coupled to the action with no easy way to override, 3) support for both could cause problems with overriding actions, and 4) tool support, such as graphing out page flows, would be difficult or impossible for actions that use this technique. However, there are good positives for this, as already mentioned. Let's give this some real consideration.

    • [jcarreira] Rather than make the framework deal with this, let's make an Action superclass or interceptor, etc which can handle this. How about an interface with one method:
    • [tm_jee] I'd rather prefer to introduce an interceptor as well.

Result getResult();

and an interceptor which can get the result from the action (in its callback method from PreResultListener) and set this result as the one to be executed. Making this one of the stereotypes for action configuration should make it very easy to get set up.

I think all the reasons you listed and more are the reasons we've always avoided this, and I don't think they're any less relevant today.

Nice to haves

  1. Inheritance is not a good way to reuse code between actions. One work around is to use the strategy pattern to swap in different implementations of interfaces like ValidationAware. It would be nice if the framework had built-in support for mixins using cglib or Dynaop. For example, instead of extending a class that implements ValidationAware, SAF could extend an action class at runtime and implement the ValidationAware methods by delegating them to another object (a mixin):

    abstract class MyAction implements Validateable, ValidationAware {
    
      public void validate() {
        addActionError(...);
      }
    } 
    We could specify mixin implementation classes by convention (defaults), in the configuration file, or using annotations. This could also be a simpler alternative to action chaining in many cases.
    • [jcarreira] You had me until the abstract class bit... Does it have to be abstract? Also, this limits testability in not-ok-ways...
    • [crazybob] It only has to be abstract if you want your action to be able to call methods on the mixin without casting. If it doesn't need to call those methods, there's no need for your action to explicitly implement that interface. You could also say ((ValidationAware) this).addActionError(). I personally don't mind making the action abstract. In IntelliJ, you just make a mock class that extends your action and it will automatically generate stubs for the methods.

    • [plightbo] As mentioned earlier, you might be able to do this by using the value stack. For example, you could stick in a single "ValidationSupport" object at the bottom of the stack and then all classes would add error messages there. By adding a simple *Aware interface, actions could get a handle to that for adding error messages.

    • [jcarreira] Very nice idea... +1 ... We already do something like that. There's always a default TextProvider pushed onto the bottom of the stack for getting default texts... makes localized texts work with Sitemesh, for instance, when you're not hitting an action. Of course, that will be a problem when we're not using ThreadLocals.

    • [tm_jee] What about keeping ActionSupport but instead have an AbstractActionSupport which ActionSupport extends off which consisting of getter/setters for the plugable strategy (like setValidatable(...) setValidationAware(...) setTextProvider(...) etc). There will be a default for those if none is set. We could then wired them in through Spring since Sprinc is now the recommended IOC container.

  2. [molitor]Extends support on actions in xml (Configuration Inheritance). Occasionally I want to utilize an existing action but only change one parameter. It would be nice to do something as simple as.{{{<action name="addPerson" class="com.opensymphony.webwork.showcase.person.AddPerson" >

    • <interceptor-ref name="validationStack"/> <param name="dataSource">people</param> <result>showPerson.jsp</result> <result name="input">addPerson.jsp</result> <result name="error">errorOnAddPerson.jsp</result> <result type="redirect">login.action</result>

</action>

<action name="xmlAddPerson" extends="addPerson">

</action>}}}I'd prefer using this approach instead of having the xmlAction chain to another.

  1. [molitor] Modular Ant Build, I've started working on this using the build files that Jason supplied (which were forwarded on to me). Allows the project to build in whole or in part with seperate build files that extend a set of common/parent build files. Should parallel the maven build.
    • [tm_jee] +1 Jason's Ant Build is impressive, I'll need some time to get familiar with Maven and would like to keep an alternate optiona available. :-)

  2. [molitor] Ability to use values on the stack to override/define configuration options. My patch for the StreamResult is just one application of this, there are many others.

  3. [molitor] Ability to invoke an action without an execute method. If I annotate a class with @assume_success (or something like @assumed result=SUCCESS) I shouldn't need an execute method. In XML this could be achieved something like this.{{{<action name="addPerson" class="com.opensymphony.webwork.showcase.person.AddPerson" assumeResult="success">

    • <result>showPerson.jsp</result> <result name="input">addPerson.jsp</result>

</action>}}}The ValidationInterceptor should still cause an input result to occur but if we make it through to invoking the action and there is no execute method we return the value of the configured assumed result.

  1. When an action returns LOGIN, save the request in the session (even POSTs) and enable the user to re-execute it after login.

Ted's Issues

  1. Combine configuration DTDs so that Actions, Messages, and Validators can be defined as a single file or in a set of homogenous files. ("One config to map them all.")
  2. Change the default for the <form> tag to "POST" (rather than "GET").

What JDK Version?

RoughSpots (last edited 2009-09-20 23:11:58 by localhost)