Mailing-List: contact dev-help@struts.apache.org; run by ezmlm
Precedence: bulk
Reply-To: "Struts Developers List" <dev@struts.apache.org>
Content-Type: text/plain; charset="us-ascii"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
From: Apache Wiki <wikidiffs@apache.org>
To: dev@struts.apache.org
Date: Wed, 19 Apr 2006 19:27:33 -0000
Message-ID: <20060419192733.27767.73419@ajax.apache.org>
Subject: [Struts Wiki] Update of "RoughSpots" by plightbo

Dear Wiki user,

You have subscribed to a wiki page or wiki category on "Struts Wiki" for change notification.

The following page has been changed by plightbo:
http://wiki.apache.org/struts/RoughSpots

------------------------------------------------------------------------------
  }
  }}}
  
+     * [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).
+ 
    1. 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.
  
    1. 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.
  
    1. 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.
+ 
    1. 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...
  
    1. 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).
  
    1. 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.
  
    1. 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 {
@@ -47, +57 @@

  
      * [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.
  
    1. 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. 
  
    1. 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.
  
    1. Should `ActionInvocation.getResult()` recurse over chain results? Maybe we should have two methods? `getResult()` and `getFinalResult()`. Is there a good use case for this?
  
@@ -77, +91 @@

  
    1. 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).
+ 
    1. 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...    
@@ -93, +109 @@

  
    1. 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.
+ 
    1. 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.
  
    1. 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.
  
    1. 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.
+ 
+ == 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.
+   1. 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.
+   1. Fail fast with detailed error messages, ideally ones that show you what you did wrong and what you should to.
+   1. 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.
+   1. 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.
  
  == Nice to haves ==
  
@@ -119, +151 @@

  
      * [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.
  
  == 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 [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.
+   * [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).
  

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org