Differences between revisions 74 and 75
Revision 74 as of 2006-06-09 19:27:04
Size: 74530
Editor: TedHusted
Comment: Formatting
Revision 75 as of 2009-09-20 23:11:58
Size: 74550
Editor: localhost
Comment: converted to 1.6 markup
Deletions are marked like this. Additions are marked like this.
Line 197: Line 197:
    * [Gabe] Discussion here: [http://forums.opensymphony.com/thread.jspa?messageID=32084] Basically, I think we can come up with a better way to accomplish this than requiring setter methods to be used for business logic and depending on parameter ordering.     * [Gabe] Discussion here: [[http://forums.opensymphony.com/thread.jspa?messageID=32084]] Basically, I think we can come up with a better way to accomplish this than requiring setter methods to be used for business logic and depending on parameter ordering.
Line 199: Line 199:
    * [Gabe] I've created an XWork JIRA for a solution to the same use case here. [http://jira.opensymphony.com/browse/XW-387] I'd be happy to contribute the code.     * [Gabe] I've created an XWork JIRA for a solution to the same use case here. [[http://jira.opensymphony.com/browse/XW-387]] I'd be happy to contribute the code.
Line 294: Line 294:
     * [Gabe] +1 I've created an XWork issue related: [http://jira.opensymphony.com/browse/XW-388]      * [Gabe] +1 I've created an XWork issue related: [[http://jira.opensymphony.com/browse/XW-388]]
Line 311: Line 311:
  1. 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].   1. 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]].
Line 323: Line 323:
  1. 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.   1. 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.
Line 339: Line 339:
    * [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 [http://www.superinterface.com/wizard/signupWizard.do 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] 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 [[http://www.superinterface.com/wizard/signupWizard.do|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.
Line 459: Line 459:
  * [MJ] With JDK 1.5 as a requirement for SAF2-based projects, users may be inclined to take a look at [http://stripes.mc4j.org/confluence/display/stripes/Home Stripes] first. It is compact, it features event-dispatching, built-in validation and conversion, Action and !ActionForm being one entity, and it allows to forgo XML config files by using annotations. The last feature alone is worth sack o'gold. If SAF2 is going to require JDK 1.5, it should allow XML-free configuration, at least for simple use cases.   * [MJ] With JDK 1.5 as a requirement for SAF2-based projects, users may be inclined to take a look at [[http://stripes.mc4j.org/confluence/display/stripes/Home|Stripes]] first. It is compact, it features event-dispatching, built-in validation and conversion, Action and !ActionForm being one entity, and it allows to forgo XML config files by using annotations. The last feature alone is worth sack o'gold. If SAF2 is going to require JDK 1.5, it should allow XML-free configuration, at least for simple use cases.
Line 461: Line 461:
  * [mrdon] I'd like to use Java 5, but use [http://retroweaver.sourceforge.net/ Retroweaver] to continue to support 1.4. If we keep XML in addition to annotations, that should be very doable.   * [mrdon] I'd like to use Java 5, but use [[http://retroweaver.sourceforge.net/|Retroweaver]] to continue to support 1.4. If we keep XML in addition to annotations, that should be very doable.
Line 473: Line 473:
  * [niallp] +1 for JDK 1.5 (btw I did a [http://mail-archives.apache.org/mod_mbox/struts-user/200601.mbox/%3c55afdc850601260220x198d3b77j7a5dde3d3eeab370@mail.gmail.com%3e POLL on what jdk people used] in Jan). Although it will prevent some users upgrading, maybe for others it will be part of the ''tipping point'' to encourage them to move to 1.5. Also what will the JDK landscape look +1 year from now - I believe most users take their time before upgrading, so thats probably a realistic timescale for the vast majority to consider SAF2.   * [niallp] +1 for JDK 1.5 (btw I did a [[http://mail-archives.apache.org/mod_mbox/struts-user/200601.mbox/%3c55afdc850601260220x198d3b77j7a5dde3d3eeab370@mail.gmail.com%3e|POLL on what jdk people used]] in Jan). Although it will prevent some users upgrading, maybe for others it will be part of the ''tipping point'' to encourage them to move to 1.5. Also what will the JDK landscape look +1 year from now - I believe most users take their time before upgrading, so thats probably a realistic timescale for the vast majority to consider SAF2.

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. ;))

  • [Ted] Action 3.0 doesn't have to be far away. It could be phase two, and ship as soon as we want to write it.

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).

  • [Ted] Hopefully, there will always be new major releases and new chances to break compatability. I would also hope any change we make will pay for itself by increasing future usability. Since each team will absorb the cost of their own changes, these types of costs will always scale. The relative cost for ten teams is not more than the relative cost for ten thousand teams, since each team pays their own way. Likewise, each team will receive a payback in future productivity, and so each team will receive the net gain. Of coure, there can be an economy in clustering changes, so certain changes can be made together, so sooner can be better. I doubt that there will be a stampede of people migrating the day we roll 2.0.0. My expectation would be that it would be at leaset six months or a year after the initial release of SAF2 before a majority of teams start to migrate, giving us time for additional, incremental releases. But, I am on board for whatever set of changes we can make and have ready to roll by August.

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.

  • [Ted] To an extent this is true. OTOH, XWork might make a very nice business facade for a lot of applications.
    1. Looking up a ResultConfig should be a one-liner. Right now we have to:

      ActionConfig config = invocation.getProxy().getConfig();
      Map results = config.getResults();
      ResultConfig resultConfig;
      synchronized (config) {
        resultConfig = (ResultConfig) results.get(resultCode);
      }
      • [plightbo] The overall RuntimeConfiguration/ConfigurationManager/ConfigurationProvider/Configuration stuff is very confusing. As long as we abstract it away, I don't care if it stays or goes. One thing I would note is that the APIs should try to pass around a fully populated ActionConfig where possible (currently some APIs simply take a String namespace and String actionName - leaving out the method name).

    2. We don't really need the Action interface anymore. Should we get rid of it? It has constant fields for result names. Should we move these to a class named ResultNames and encourage users to static import them as needed?

      • [jcarreira] I'm not sure about this... The Action interface is kind of just a marker interface, but at least it gives us SOMETHING to point users to
      • [crazybob] I'll buy that. We do need to move the constants out and encourage users to use static import (Effective Java Item 17).
      • [plightbo] Related to this, I would encourage us to try to find a solution (using Bob's mix-in suggestion below, or possibly just taking advantage of the value stack) that would make ActionSupport much simpler. This would encourage using POJOs more.

      • [mrdon] Regardless whether we remove Action or not, I like the idea of moving the result constants out.
      • [tfenne] I think removing Action would be a bad idea. If you're going to try and get away from requiring XML configuration or listing out all your Actions somewhere, you'll probably want to have someway of discovering your Actions. An interface is the easiest option - an annotation would do too, but would feel a little stranger I think.
      • [craigmcc] You can actually have this one both ways, as we do in Shale -- define an Action interface for
        • the JDK 1.4 users, and provide an optional JavaSE 5 layer that lets you define the appropriate callbacks with annotations instead. For a detailed example, take a look at how Shale's "Tiger Extensions" package

          lets you get the functionality of a ViewController interface without having to say "implements ViewController" in your action class.

      • [crazybob] +1 for keeping the Action interface--it enforces that execute() is the default method.

      • [rainerh] +1 for keeping the Action interface and +1 for moving the result constants out.

    3. Only put classes in root package that most users need to know about. For example, most don't need to know about Default* or ObjectFactory.

      • [plightbo] +1 on this - sounds like Bob has a good handle on what it takes to make a nice API. I'll defer to him on this.
      • [mrdon] +1
      • [craigmcc] In Shale, I adopted the convention of package "org.apache.shale.foo" for public APIs, and
        • "org.apache.shale.foo.impl" for things like default implemetnation classes. However, there is an additional subtlety here to take into account -- there is a difference between APIs that application developers should depend on versus those extending the framework should be allowed to extend. The latter category of folks can be presumed to be smaller, as well as more willing to deal with occastional API breakages.
      • [crazybob] +1 for "impl" package naming convention. I think we should focus on the application developer API for the initial release and then introduce a more featureful extension API in a later release.
      • [rainerh] +1 for "impl" packages as well
    4. Only make classes/members public if we're willing to guarantee future compatibility. Everything else should be package-private or excluded from the Javadocs.
      • [plightbo] + 1 again.
      • [mrdon] This I don't agree with. From a framework developer, I understand the logic, but from a user, it is arrogant. I think we should allow people to extend Struts in ways we haven't imagined, and restricting access like this says, "We know more than you and will force you to do it our way."
      • [crazybob] I don't think we should make everything final or anything, but I do think we should differentiate between the published API and the implementation through package organization, excluding stuff from the Javadocs, etc. Users can know that we won't change the published API out from under them, but that if they depend on implementation classes, we may break them (reluctantly). It's the polite thing to do, and it frees us up to refactor and improve our implementation.
      • [jcarreira] +1 published public vs. private APIs are a good thing, and a contract with the user of what can change.
      • [craigmcc] See comments on the previous bullet related to APIs that application developers use versus those
        • who are extending the framework might use.
    5. Remove destroy() and init() from Interceptor. They don't make much sense until the interceptor lifecycle is specified (see next item). I've never needed them, yet it's a pain to implement empty methods every time I implement an interceptor. Users can use the constructor/finalizer or we can create additional lifecycle interfaces.

      • [plightbo] I don't really care. This ties more in to the next item...
      • [tm_jee] I'd prefer to keep them as a separate optional implementable interface as Bob advised, such that Interceptor interface implementor are not required to implement them, and for the minority who needs them, its there.
    6. Specify Interceptor lifecycle. Right now if we apply an interceptor to a single action, we get a new instance every time. If we define an interceptor in a stack, the same instance gets reused.

      • [jcarreira] A new instance per action configuration, right? Not per-invocation...
      • [crazybob] Last I tested it was per invocation (I remember because it surprised me). This is actually a non-issue. We'll create a custom ConfigurationProvider for Struts which won't have this problem.

      • [plightbo] Agreed, by abstracting most configuration out, we can control the lifecycle. I think the lifecycle should be either once per interceptor or once per invocation. Currently the implementation is too cumbersome (once per unique action configuration).
    7. Get rid of AroundInterceptor. Having before() and after() methods doesn't make things simpler. It reduces flexibility. We can't return a different result. You can't handle exceptions cleanly. The actual interceptor class doesn't appear in the stack trace (we see AroundInterceptor over and over).

      • [jcarreira] The idea was that people would forget to do invocation.invoke() and be confused... Easier for users just to implement a before() method when that's all they need. I agree on the stack traces though.
      • [crazybob] It's kind of hard to forget to call invocation.invoke(); you have to return something. ;) Interceptors are already an "expert" feature anyway.

      • [plightbo] Big +1.
      • [mrdon] +1 as well
      • [rainerh] +1 from me as well
    8. Try to get rid of thread locals: ActionContext and ServletActionContext. At least make them package-private. Sometimes interceptors need access to the servlet API. In this case, they should implement a servlet-aware interceptor interface. For example:

      class MyInterceptor implements HttpInterceptor {
        public String intercept(HttpInvocation i) {
          HttpServletRequest request = i.getRequest();
          ...
        }
      }
      • [jcarreira] These 2 are orthogonal... Getting rid of ThreadLocals is problematic. I think we'd end up breaking 90% of old WebWork apps if we did, and it's still not clear that everything could be covered if we did... I like the idea though, and Patrick and I really wanted to do this out of the gate, but backwards compatibility with WebWork 1.x at a macro-level made us think otherwise...

      • [crazybob] Interceptors need access to the servlet API. They shouldn't have to call a ThreadLocal if we can avoid it and they shouldn't need to cast. We shouldn't worry about breaking old WebWork apps (see new opening paragraphs). Let's get it right the first time around because we will not be able to fix it later.

      • [plightbo] By running our interceptors and other objects through the same factories and lifecycle managers that the action classes go through, this should be a non issue.
      • [mrdon] This I'd like to see. I've found myself using these objects so often in wierd little places, I'd be loath to remove them unless we could prove 100% that their information can be retrieved elsewhere.
      • [jcarreira] +1 to Patrick's point... we may need to introduce some more advanced *Aware interfaces, though, to give people access to the guts if they really want it.
      • [frankz] ActionContext being ThreadLocal was one of the first "cool" things I noticed about WW. I'd hate to see that change. The only thing I can think of that would make me agree to change that is that I think we may find developers using it in "inappropriate" ways, i.e., an Action calls a business delegate and the delegate uses ActionContext. My bet is most people would agree it should be a "best practice" to not do that. Still, it's cool that you can!

      • [craigmcc] JSF likes the thread local approach to a per-request context object (FacesContext in this case)

        • too. One of the really nice benefits is simplifying parameter signatures to methods that *might* need access to the context state, but not necessarily. The counter-example, of course, is the Struts signature for Action.perform(). Two more comments separated out because they address separate issues.
      • [craigmcc] Regarding Frank's concern about potential "inappropriate" changes ... if you design an API that
        • passes a particular interface in to a method, and then expects that method to pass the same interface along, you are in exactly the same boat. The Servlet API considers this a feature rather than a bug, because a Filter can actually wrap the incoming request and response objects to provide extra functionality -- this at least wouldn't be possible in the case of a thread-local state object.
          • [crazybob] Good point (Though it is possible with a thread local API. Your code can set the thread local to your wrapped object and then restore the original object in a finally block. Not ideal.)
      • [craigmcc] "Interceptors need access to the servlet API" -- doesn't anyone care about making it possible
        • to write applications that work for portlets too? I would hope that we all work towards a world where direct access to the underlying servlet API objects is considered an anti-pattern.
          • [crazybob] Another good point.
    9. Is ValidationAware a good name? Perhaps Errors or ErrorList would be a better name.

      • [plightbo] Eh, it's not a big deal to me. Errors wouldn't be accurate either though, since there are also generic messages that you can add.
      • [rainerh] What about ValidationMessages?

    10. Merge ActionContext and ActionProxy into ActionInvocation (at least from the users' perspective). Better specify what happens during chaining/action tags.

      • [jcarreira] It is well specified... There are some things that the ActionProxy / ActionInvocation let you do that a merged one doesn't... for instance easily knowing when you're done :-)

      • [crazybob] Does "specified" == "documented"? Can you elaborate on "easily knowing when you're done" and why we can't address that use case with one interface? We should expose the user to one interface: Invocation. We can have as many objects as we like when it comes to the internal implementation.

      • [plightbo] Big +1. I would add the ValueStack to this list as well. Let's have one object for each invocation, and also make "sub invocations" (chaining or ww:action calls) a first class citizen through child/parent invocation relationships.

      • [jcarreira] Let's have an interactive design session on this... I'm not sure it's as easy as you think to merge them all into one (although behind one interface should be doable).
      • [tm_jee] Sorry, I don't get this bit. :-P We could just expose ActionInvocation. ActionProxy could be obtained from ActionInvocation. Or maybe just expose ActionProxy's method in ActionInvocation, but actually its just delegating to ActionProxy. Did I miss something?

    11. Should ActionInvocation.getResult() recurse over chain results? Maybe we should have two methods? getResult() and getFinalResult(). Is there a good use case for this?

      • [jcarreira] See the TokenSessionInterceptor and the stuff it does to re-render the same result if you post the form more than once. That was the reason for the complexity in finding the result to execute. It's a nice feature, but I agree it makes the code harder to read.

      • [crazybob] We should move this logic to TokenSessionInterceptor until we need it somewhere else. TokenSessionInterceptor can access the functionality using package-private access if need be so we don't have to expose it through the published API.

    12. ActionInvocation.invokeActionOnly(). Does this need to be public? Sounds dangerous.

      • [jcarreira] Not sure... This may be part of the same TokenSession stuff... can't remember exactly.

      • [crazybob] See above.
    13. Eliminate non-private fields. Protected fields in ActionConfig for example.

      • [jcarreira] We don't want to allow for extension?
      • [crazybob] Extension through interfaces and methods? Yes. Public/protected fields? Absolutely not!
      • [mrdon] I dunno, are you planning to make protected getters/setters for every field? I've found protected fields to be invaluable when extending frameworks (again, subscribing to the believe we should let the user do what they want and not artifically restrict them). I do wish you could declare the fields _only_ available to subclasses and not to the whole package.
      • [crazybob] Absolutely, methods instead of fields. Methods enable us to hook access if need be, change the underlying implementation, manage thread safety, protect our own code against unexpected conditions/states, etc. Fields are OK within a package, but when it comes to public APIs, better safe than sorry a year from now.
      • [jcarreira] Oops, I was translating "fields" -> "properties" and thinking of getters and setters.

      • [tm_jee] I am kindof thinking in the same line as Don.
    14. Rename ActionInvocation to Invocation or Request. Shorter is better.

      • [jcarreira] Most users don't see these... Let's not change names on a whim, since it will be more work for the power users who already use them.
      • [crazybob] We can make the change in our abstraction layer and not impact existing XWork users. This is our one chance to get this stuff right.
    15. Is TextProvider a good name? The JDK refers to these as "messages" everywhere.

      • [plightbo] The name doesn't bother me, but the implementation is far too complex. I would propose we cut it down from all the crazy package/interface/model-driven stuff we have now to a simple i18n RB loading approach that uses 0 or more global bundles as well as bundles associated with the _view_ (NOT the action, since that never made much sense).
      • [mrdon] I'd like to see this re-evaluated as well. For one, I want to make it easier to provide an impl that gets messages from a Struts bundle, which really isn't possible now.
      • [jcarreira] +1 to simplifying. We started with lots of separate resource bundles for per-action texts, but we're moving to one resource bundle per module. It's too much hassle.
      • [tm_jee] I like the feature WebWork has that allows resource bundle to be overriden at different levels through inheritance hierarchy, package hierarchy and a global level. If possible, please keep them. :-)

    16. Come up with a clean way to separate "view" actions from "update" actions. For example, we might have view() and update() methods in the same action class. Right now XWork has MethodFilterInterceptor, but that's not a very clean solution. Do we want validation or the DefaultWorkflowInterceptor to run for the view() method? One solution is separate interceptor stacks, but it would be nice if there were some first class support for this. We could flag action invocations as "view" or "update" (using an enum). We could automatically choose a mode based on whether the request is an HTTP GET or POST. Or we could set the mode based on an annotation on the action method. Or some other way...

      • [jcarreira] This is where I think the power of annotations can be great for us... If we had some common annotations like @view, @edit, etc. then we could just let users map configurations to those stereotypes (to use an UML-ism) and reduce configuration quite a bit. Maybe if we just had a generic @Action annotation the stereotype could be a String parameter so we don't limit them to the ones we pre-define...
        • [crazybob] I'd prefer to avoid arbitrary strings when possible. Use the annotation class itself as the "stereotype" identifier. Apply a @Stereotype annotation to these annotation classes to mark them as such.
      • [MJ] Using GET for render and POST for submit works well unless you want to trigger event with a link. Also, these links might help: DataEntryForm, EventActionDispatcher

        • [crazybob] Triggering an event should still be a POST (though the framework should make it easy). From the HTTP spec.: "GET and HEAD methods SHOULD NOT have the significance of taking an action other than retrieval."
        • [jcarreira] I think it's great that you want to take the HTTP spec at its word... most users don't care though.
        • [crazybob] I'm not arguing semantics. There are real security implications to using GET when you should use POST, not to mention products like Google Web Accelerator will reak havok on your site. As framework developers, we should make using POST as easy as GET for users. To not help users do the right thing in this situation would be irresponsible, and not in the letter of the law sense.
        • [frankz] Perhaps a new attribute on the <action> mapping? type="view" or type="update"? This would really make the mapping of a specific various type, not the underlying Action, which might be better because the type is abstracted from the actual implementation and the Action class itself can house both types of functions (i.e., something like a DispatchAction), but the framework can know to treat them differently as appropriate. I'm not one of those "no XML config!" folks, I actually still prefer it to annotations, but having an analogous annotation would be a good idea too.

      • [crazybob] In addition to "view" and "update", we might want "chained view" and "chained update". Can anyone else think of more "stereotypes?" Can you think of a better name than stereotype? Strategy?
    17. On the OGNL value stack #request refers to request attributes and #parameters refers to the parameters. We could rename these #request for request parameters and #requestAttributes for request attributes.

      • [jcarreira] That one always has been confusing...
      • [mrdon] Or #requestScope keeping in line with JSTL/JSP
      • [rainerh] +1 for JSTL/JSP like naming
      • [tm_jee] +1 for JSTL compatible naming as well.
    18. Warnings and failing silently is not the best practice. For example, if we can't find a method in a given action class, blow up ASAP (at load time). We shouldn't be fuzzy and do stuff like try to find a method named doXxx() when we can't find a method named xxx() (WebWork had backward compatibility restrictions, but we don't need to perpetuate them).

      • [plightbo] +1, everything should be much more proactive about failing fast and failing with a nice error message.
      • [mrdon] Agreed, and we have done a lot of work lately on this issue.
      • [rainerh] Better error reporting for developers is a very important issue. Latest additions to xwork will help here but could be improved a lot.
      • [tm_jee] +1
    19. Add better support for file uploads.
      • [jcarreira] Anything specific? We're using them at work and they work well... Maybe we could pull out the file upload progress bar with DWR thing that we've got here...
      • [crazybob] We have an UploadedFile value object which has properties for the File object, the file name in the form, the content type string, and the name specified by the user. An interceptor passes that object to a setter on our action and then deletes the file at the end of the request.

      • [plightbo] +1 for a compound object that holds the data, file name, and content type. The file shouldn't be represented by java.io.File, but some other wrapper object. That way people can implement in-memory options if they choose to.
      • [tm_jee] +1. I like the idea of Compound File Upload object as well.
    20. Don't eat/wrap exceptions. Throw them through to the container. Don't eat exceptions that occur in getters.
      • [plightbo] We're in serious need of a cleanup here.
      • [mrdon] Again, changes have been made but much more to be done
    21. Modify ParametersInterceptor to sort parameter names by depth (using bucket sort) and then map them in that order (shallowest first), so we can create objects and then map fields to those objects in the same action invocation without hacks like applying the ParametersInterceptor twice or chaining.

      • [jcarreira] I'm not sure that's useful... We discussed it at some length on the mailing list and it wasn't clear. mapping the param interceptor twice isn't for that problem, though, it's for model-driven actions.
      • [crazybob] I'm not sure what you discussed, but it's *very* useful, and there should be no reason not to do it. Say for example my form has a 'userId' and fields to set on the user 'user.name', 'user.address'. With the sorting, 'userId' gets set first at which point we load a User object. Then the other parameters get mapped to that User object. Without the sorting, there's no guarantee on the ordering. You have to load the user in one action and then chain to another. This is a common use case; might as well make it simple.

      • [Gabe] Discussion here: http://forums.opensymphony.com/thread.jspa?messageID=32084 Basically, I think we can come up with a better way to accomplish this than requiring setter methods to be used for business logic and depending on parameter ordering.

      • [plightbo] As Gabe said, we already discussed this. And the last post on the subject was that we should do it. We still should.
      • [Gabe] I've created an XWork JIRA for a solution to the same use case here. http://jira.opensymphony.com/browse/XW-387 I'd be happy to contribute the code.

      • [tm_jee] What about a new parameter interceptor that does this functionality and users could switch between these interceptor according to their needs instead of reimplementing current parameter interceptor. Or have a switch in the current parameter interceptor to allow it to switch strategy of populating parameters.
    22. Allow paths in action names. For example <action name="reports/myreport">.

      • [jcarreira] Why do you want this? Isn't this part of the namespace of the action?
    23. Enable Java package import in configuration so you don't have to repeat the same package name over and over (like WW1 does).
      • [jcarreira] +1 if it can be made sane... It can get confusing. It also makes tool support worse (i.e. IDEA can find it as a fully qualified class name)
      • [crazybob] Regarding tool support, I like writing less code, and this doesn't preclude using fully qualified class names if you still want to. Hopefully this new exposure will bring us real tool support and this won't be an issue.
    24. The ajax support is pitiful. Have a look at how stripes does it. Ajax for validation is trivial and not that impressive, but people are going to want to do real ajax work, and webwork does absolutely nothing to help in that regard. I'd like to for example be able to easily invoke actions and get at some kind of result to display, and have webwork provide at least some of the wiring
      • [jcarreira] Well, that's a relatively simple usecase, and I think it IS supported... at least we use it at work?
      • [frankz] I would ask what "real AJAX work" means, because that would really determine what path makes sense.
    25. The default theme for the ui tags should be simple. The current stuff is too dumb to get right on the first go, which gives an awful impression. It's NOT intuitive to write:

      <table>
      <ww:textfield label="Enter blah" />
      </table>
      • [jcarreira] That depends on whether you're using the form tag or not, but agreed... the XHTML theme should be cleaned up and made the default.
      • [tm_jee] XHTML theme is currently the default, if not mistaken.
    26. File upload should support progress notification. Have a look at webwork-multipart on java.net, it's based on the pell parser but has a progress API.
      • [jcarreira] We've implemented this at work with WebWork fileupload + DWR + a class that looks at the file as it's uploading to see how big it is on disk

      • [frankz] Just an opinion, but this seems to me too specific a use case for a framework to encapsulate. I think having an example showing how to do it, perhaps what jcarreira has done at work, would be great, but I for one wouldn't like the framework offering this out of the box... I think it is possible for a framework to be able to do TOO much!
      • [tm_jee] I think this is pretty use case specific as well, but a progress monitor ui component would be nice.
      • [jcarreira] If we agree that supporting file uploads out of the box is important, then this is just a nice feature for that support to let the user know how much of the file has been uploaded, etc.
    27. Better error checking for UI tags. The freemarker error message, while great for freemarker users, look like gibberish. People should not be forced to learn freemarker. So in such cases, the tags themselves should check the parameters and report back sane messages, even if that check is duplicated in the templates
      • [phil] This is, depending on what you want, fairly easy to really complicated. You can easily register a new FreemarkerExceptionHandler in components.template.FreemarkerTemplateEngine to limit the stacktraces. But it will still be gibberish if you don't know what happens (eg. " Error on line 43, column 13 in template/simple/select.ftl - stack.findString(parameters.listValue) is undefined. It cannot be assigned to itemValue]" -> that hardly tells you you specified a non-existant method for listValue in your select box). Solution: we should do more checking in the components instead before rendering.

    28. Defaults should be JSP all the way. People know it and like it, despite all the limitations. Allow for other view technologies, but don't force people to learn stuff they don't want to. Learning ww is enough of a pain as it is
      • [tm_jee] Hmm... are you suggesting that we should support JSP template as well? WebWork currently does support JSP view, just theat there's no theme/templates written in JSP due to JSP not being able to be packaged and displayed from a jar file in the classpath.

      • [plightbo] Agreed - examples, default results, etc should be JSP. However, we need to keep UI tags in a template language so they can be used in all view technologies. Right now that is FreeMarker, though if we can find a template language that is more like JSP (say, without the scripplets), I would like to switch to that.

    29. Get rid of the validation framework. it's stupid and pointless, validate methods are good enough.
      • [jcarreira] -1 I take offense at this... It's neither stupid NOR pointless, and we use it extensively. It's the best validation framework I've seen out there, and NO, validate methods are NOT enough. For instance, we define reusable validations for our domain models and use them for both the web front end as well as web services and batch imports.
      • [tmjee] -1 If possible please don't do so. I use it and like it. I guess (for me at least), sometimes for simple validation it is nice to be able to just describe it (using xml or annotation). Plus the validation could be tied to DWR for ajax support. Being able to write custom validator is really cool. Please reconsider this. :-)

      • [frankz] -1 as well. If you had said the validation framework could stand to be enhanced and expanded on a bit, I'd agree, but I definitely think it should be there, not pointless or stupid at all. Declarative validation is a fantastic approach, especially with validator being a separate Commons component. For instance, we are working on a project at work that is going to use Validator and the CoR implementation in JWP as the basis for a rules engine... I put together a proof of concept showing how we could use the exact same validations in the web front-end via AJAX calls as in the Web Service interface for other systems to call on. Being able to write those validations in XML without having to write actual code was a great thing.
      • [crazybob] I think sharing validations between AJAX and Java more than justifies the framework. I would like to see us replace most if not all of the XML with annotations.
      • [rainerh] -1 on removing the validation framework. This is one of the best, if not the best validation framework I've seen so far.
    30. Ditch xwork as a separate project, nobody uses it or cares about it
      • [jcarreira] You're kidding, right? We've discussed this already....
      • [tmjee] -1 If possible, I'd like to keep xwork, not that I used it apart from WebWork but, I don't know, it's just good to have it there.

      • [rainerh] -1 as well
      • [Gabe] -1 I believe XWork should be moved over to Apache and more importantly, the final decision on whether to do so should be made now rather than later. However, I don't believe it should be merged with the former webwork.
      • [phil] -1
    31. Add java5 support to ognl. It's silly that it still doesn't handle enums (that I know of).
      • [jcarreira] +1 this is biting us right now
      • [crazybob] What needs to be done here? We wrote a type converter for enums. Is there more to it?
      • [rainerh] +1 as well
      • [tm_jee] +1
      • [phil] +1
      • [plightbo] +1 - we'll likely need to make new releases of OGNL to do this. That means it would be a good opportunity to also fix up other problems (Gabe probably knows the most about the limitations/problems here).
      • [Gabe] +1 Hopefully, this would only be a modification in our PropertyAccessors to include enums. Here is one place where we will want to figure out a way that we can add these things but remain 1.4 compatible.

    32. Clean up documentation. Focus on quality not quantity.
      • [jcarreira] Didn't you read the book? ;-)

      • [tm_jee] +1 What do you think about the reference docs, we put a lot of effort in it. Of course there's still lots of room for improvement. We'll continue to do our best. :-)

      • [plightbo] The reference docs are great, but we failed terribly on the tutorials and non-reference docs.
    33. Do we want to keep ModelDriven?

      • [Gabe] Absolutely YES! ModelDriven allows us to build forms and populate the model without a prefix. It's simple. It also allows for security interceptors to zero in on one method for ModelDriven actions to determine what to secure.

      • [plightbo] I think we should give some serious thought to this. Look around the internal implementation of WebWork/XWork as well as the number of questions that come up on the mailing lists. It's a very confusing thing, especially when interacting with setting or getting values of form fields.
      • [jcarreira] yes, it makes the internal implementation more complex, but it makes things easier for the user of the framework in a lot of cases. If people are confused by it, they can choose not to use it.
    34. Do we want ValidationAware (or its equivalent) to take message keys or actual messages. It takes the actual messages in WW2. ActionMessages in Struts takes keys. I'm a fan of keys; we would no longer need TextProvider. Pat suggested we take keys, and in the event that we don't find a message for the given key, pass the key along as the message. I think I'd rather fail fast.

      • [mrdon] Keys are fine, as long as you can do parameter replacement easily enough later. Not all apps need L18N, so I'm kinda against the fail fast. Perhaps in devMode, we add a clear warning?
      • [Ted] Having lived with keys-only for all these years, I like the idea of being able to use either literals or keys.
    35. Craig McC mentioned that we might want to use this in a portlet. Does this mean I should completely abstract us from HttpServletRequest/PortletRequest?

      • [mrdon] +1, at least in some form. This was the goal of the generic ActionContext, I believe. Cocoon has been struggling with the same issue, and they are leaning towards implementing the HttpServletRequest, et al with a portlet impl to solve this problem. They used to have this generic "Environment" api, but they are in the process of giving that up, I believe, favoring this servlet api approach. I wonder if we shouldn't find out more about their results and adopt them.

      • [plightbo] +1. we should keep this in mind when designing the API.
      • [jcarreira] -1 to using the HttpServletRequest... Why do we need it? We've got a nice abstraction with the Maps for parameters, session, and application scopes, I think. Why tie to the servlet API and make a phony one to proxy to Portlets?

    36. Add "action" and "method" attributes to the submit button tag so users don't have to know about the "action:Name" and "method:name" conventions.
      • [plightbo] We already do this :)

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">

  • <result name="input">xmlAddPerson.jsp</result>

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

  • [tm_jee] Sounds cool. It would probably be an overkill to extend this idea into the interceptor as well, wouldn't it.
  • [molitor] Agree, falls on the short side of the 80/20 rule
  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.

  • [Ted] The use case seems to be a POJO Action method. But, if the method (AddPerson) is truly POJO, then what value can it add? If it doesn't know the framework exists, then it can't utiize the runtime data. If the class does know the framework exists, why not have a helper method return SUCCESS. If there is a use case for calling a framework-aware "void" method, then if the "action" returns void, the framework could seek the default result (the one with no name) and obviate the annotation.

  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?

  • [jcarreira] We've been using JDK 1.5 on Tomcat 5+ for over a year... Everything we write and wire together is using generics and annotations.
  • [crazybob] +1 for JDK 1.5 since it came out. I have a lot of code I could contribute which depends on the new concurrency libraries, etc.
  • [MJ] With JDK 1.5 as a requirement for SAF2-based projects, users may be inclined to take a look at Stripes first. It is compact, it features event-dispatching, built-in validation and conversion, Action and ActionForm being one entity, and it allows to forgo XML config files by using annotations. The last feature alone is worth sack o'gold. If SAF2 is going to require JDK 1.5, it should allow XML-free configuration, at least for simple use cases.

  • [plightbo] I use JDK 1.5 as well. I think we really should think about doing this. Non-1.5 options exist (WebWork 2.2 and Struts 1.3), and we're really at a close point where it is no longer unreasonable to require 1.5 (1.6 will have been out for almost 6 months by the time we hit a final release).

  • [mrdon] I'd like to use Java 5, but use Retroweaver to continue to support 1.4. If we keep XML in addition to annotations, that should be very doable.

    • [crazybob] Sounds good. We'll also need to handle JDK-1.5 only APIs (concurrency for example).
  • [Gabe] I am required to use 1.4 at work. To me the question of whether to require 1.5 comes down to whether the same shops that are stuck using 1.4 are also not going to let people use Struts 2.0, because it is too bleeding edge anyway. In that case it doesn't make sense to allow 1.4, because the only people who would be using it would also have access to 1.5 anyway. I don't know if that is the case though.
  • [martinc] The big issue with the JDK version is app servers. This comes in two parts. First is whether all of the major app server vendors have products available that support the desired SDK version. I believe we're OK in that regard with JDK 1.5. The bigger issue is customer acceptance. Enterprise customers, especially, tend to standardise on their app server, and they are not quick to upgrade. Unless the application vendor has a great deal of influence over the customer's infrastructure, the vendor has to live with whatever app server version is in use at the customer site. It is rare, then, that the application vendor can dictate the JDK version. On the other hand, the customer usually couldn't care less what version of Struts the application was built with.
  • [tfenne] I think you *have* to support JDK 1.5, and it should be the default. If it's not too hard to provide 1.4 compatibility great, but I think all examples, defaults etc. should leverage 1.5. Generics allow you to do much more for the user without asking for configuration information. If a user wants to use JDK 1.5 enums, it should work, etc. etc. If it's extra work on the user's part to make 1.5 features work, simplicity goes out the window.
  • [frankz] I think this is one of those things to be really careful about the perception people may form. If Action1 is going to continue to develop and be supported, even if to a lesser degree, then forcing 1.5 for Action2 is probably fine. However, I know at my company, we are stuck on 1.4, and won't be changing for a long time. I also know that we are not unique in this regard. If we can't move to Action2. so long as Action1 is still around and being supported, that's fine. But if we can't move to Action2 and it even seems like Action1 isn't getting enough attention, that wouldn't look so good to us. Ultimately, if both can be supported, I think that is still the best answer. I definitely think the points made about moving to 1.5 are totally valid, but I think that may lock out a lot of people who might otherwise use Action2, so if that can be avoided, so much the better.

  • [crazybob] Someone made the point earlier on that if a company is hesitant to switch to JDK 1.5, they'll probably be hesitant to adopt SAF2, too. With a little time, 1.4 will become irrelevant. I'm fine with supporting 1.4, but 1.5 should be the priority, and we shouldn't let 1.4 support negatively impact design decisions.
  • [gdinwiddie] WRT "if a company is hesitant to switch to JDK 1.5, they'll probably be hesitant to adopt SAF2, too," I don't think that's necessarily true. In companies where I've worked, the choice of what libraries are used for app development are often made by the developers of that app, but choice of app server (which dictates JDK version), is generally made at a higher level, and often by a slow-moving committee.
  • [crazybob] You're right. That's actually been my experience, too. I'm fine with supporting 1.4 (though I'd prefer not to have to write the code as I haven't used it for some time).
  • [rainerh] +1 for Java 5 support
  • [tm_jee] +1
  • [molitor] +1 for JDK 1.5+
  • [niallp] +1 for JDK 1.5 (btw I did a POLL on what jdk people used in Jan). Although it will prevent some users upgrading, maybe for others it will be part of the tipping point to encourage them to move to 1.5. Also what will the JDK landscape look +1 year from now - I believe most users take their time before upgrading, so thats probably a realistic timescale for the vast majority to consider SAF2.

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