Re: [OS-webwork] Configuration questions
Jason Carreira wrote: I started looking at doing this and ran into some snags. For instance, if the code calling the Proxy wants to get at the Action, how does it do that? The ActionInvocation won't even have been created yet, if the Proxy hasn't been executed, and will the Action make sense in a context outside of the execute method of the ActionProxy? Well, that's a damn good question :-) One could argue that an invocation should be created immediately upon creation of actionproxy, and when it's invoked then the reference to it is lost so that any subsequent calls to get the action (from the wrapped invocation) would get a new invocation/action. Perhaps not a perfect solution, but it would give us all the right semantics (I think). After the execute on the ActionProxy, should the ActionInvocation still be available? All in all, I was more sure of this design before I started doing it :-) After the invocation the the invocation should be thrown away (I think). As you said, it's really only once you start doing it that all these problems emerge. Keep doing it and we'll take the issues as we go along. /Rickard --- This sf.net email is sponsored by:ThinkGeek Welcome to geek heaven. http://thinkgeek.com/sf ___ Opensymphony-webwork mailing list [EMAIL PROTECTED] https://lists.sourceforge.net/lists/listinfo/opensymphony-webwork
[OS-webwork] Configuration questions
Hi! I'm looking through the Configuration stuff in XWork, and have the following comments/questions: * What does it mean for a package to be abstract? * The ConfigurationProviderFactory is not a factory, so it's a bad name. Also, what is it's purpose? It seems that the current code only allows for one ConfigurationProvider. Considering the need to build apps from subapps, which may use different config styles, that is a bad idea. * This method in ConfigurationManager is wrong: Interceptor getInterceptor(String clazz) It assumes that there is only one instance of each interceptor class. This does not account for the case where one instance is used with many names (compare with servlets), and configured differently in each case. It has to change to: Interceptor getInterceptor(String namespace, String name) * the getAll* methods in the PackageContext's should not be there. The contexts may understand their own stuff, and that they happen to have a parent, but they should not be responsible for putting it all together. That's the task of the ConfigurationManager IMHO. I'm not even sure a PC should have a direct link to it's parent. Maybe the name is enough, and the CM should make the connection. * I can't remember if we resolved the issue of whether to use a strict parent/child relationship between PC's, or whether we should instead use a many2many depends-on relationship. I.e. with parent/child a package can only have one parent which it can use for accessing interceptors and interceptor packages, and for referring to other actions. With a depends-on relationship, where a package can depen on many other packages it is a much more open scenario, where a PC could (for example) depend on both the XWork default package, some company-specific ourstuff package, and then some ourappdefault package. This is useful for being able to reuse stuff to the maximum. Otherwise one would have to make sure that ourappdefault has ourstuff as parent which in turn has default as parent. And once you get to the point where you want to reuse two packages that you can't control you're in big doodoo. The classical multiple inheritance problem. Methinks the depends-on style is more useful, as outlined above. The rules for resolving names would still be deterministic though. * Overall, I have trouble to (in JBoss-speak) read the story. I can't see how things fit together, and how these classes are supposed to be used. There doesn't seem to be a coherent vision of what the goal of the configuration stuff is. I'm seeing lots of code, but no idea of why it's there. This is subjective, of course, but it's the feeling I get reading the code. I'm having a hard time relating it to any other configuration packages I've seen too, which will probably also make it harder for people (in general) to understand how it's supposed to work. Some issues not related to configuration: * It seems ActionInvocation's are called directly by code. A better way to do this is to create an ActionProxy which upon execute() creates an invocation and invoke it. This ensure that invocations are not reused. In general an invocation is not a good thing to deal with directly since it's conceptually a side-effect of something else. Hiding it behind an ActionProxy would fix this. * The ActionInvocation seems to have a lot of stuff that is not really related to the invocation at all. The Action and result are invocation stuff, i.e. local (temporally) variables. The rest are more or less fixed and should go into the ActionProxy, and the invocation should then have a reference to the proxy. This removes a lot of code for copying parameters into the invocation as well (see the constructor). Similarly the code for init'ing the action should go into the ActionProxy. The invocation is just an invocation, nothing more. That's what I could find so far. Any comments are appreciated, especially from Jason. All in all, what I want to see is a refactoring that makes it easier to read the story, and to easily see what the different players are and how the play together. Currently, as described, it is rather confusing. regards, Rickard ps. I really don't like the code style that is being used (and it's rather different from WW), but maybe that's just me. --- This sf.net email is sponsored by:ThinkGeek Welcome to geek heaven. http://thinkgeek.com/sf ___ Opensymphony-webwork mailing list [EMAIL PROTECTED] https://lists.sourceforge.net/lists/listinfo/opensymphony-webwork
Re: [OS-webwork] Configuration questions
Jason Carreira wrote: * This method in ConfigurationManager is wrong: Interceptor getInterceptor(String clazz) It assumes that there is only one instance of each interceptor class. This does not account for the case where one instance is used with many names (compare with servlets), and configured differently in each case. It has to change to: Interceptor getInterceptor(String namespace, String name) Patrick and I have talked about parameterized Interceptors but we were unable to come up with an example yet, so we didn't implement it (YAGNI in action). I agree that if we decide to have parameterized Interceptors, this is necessary. Ok, well I guess this argument is the same as parameterized commands. I have a couple of interceptors in my own AOP application that are indeed parameterized, so at least in that context there's a need, and I can make a similar example for this case (this, interestingly enough, touches upon the parameterization issues of commands too that I mentioned in my last post). * the getAll* methods in the PackageContext's should not be there. The contexts may understand their own stuff, and that they happen to have a parent, but they should not be responsible for putting it all together. That's the task of the ConfigurationManager IMHO. I'm not even sure a PC should have a direct link to it's parent. Maybe the name is enough, and the CM should make the connection. Well, maybe. This just made a lot of things easier. I'll have to look in the code again for more specific examples. Just move them to the CM. Some issues not related to configuration: * It seems ActionInvocation's are called directly by code. A better way to do this is to create an ActionProxy which upon execute() creates an invocation and invoke it. This ensure that invocations are not reused. In general an invocation is not a good thing to deal with directly since it's conceptually a side-effect of something else. Hiding it behind an ActionProxy would fix this. Sounds like a good idea, but how do we prevent people from using ActionInvocations directly? Would we make them package local and put the ActionProxy in there as well? I'm not sure why people would try to use it directly, but using package protected constructors would work. In what way? Well, mainly the good 'ol curly braces thing (and I have thus far never seen a good reason why one would put 'em on the same line), and also how portions of the class is delineated. I prefer the template used in WW, which had more clearly defined sections. /Rickard --- This sf.net email is sponsored by:ThinkGeek Welcome to geek heaven. http://thinkgeek.com/sf ___ Opensymphony-webwork mailing list [EMAIL PROTECTED] https://lists.sourceforge.net/lists/listinfo/opensymphony-webwork
RE: [OS-webwork] Configuration questions
-Original Message- From: Rickard Öberg [mailto:[EMAIL PROTECTED] Ok, well I guess this argument is the same as parameterized commands. I have a couple of interceptors in my own AOP application that are indeed parameterized, so at least in that context there's a need, and I can make a similar example for this case (this, interestingly enough, touches upon the parameterization issues of commands too that I mentioned in my last post). Ok, can you implement or describe an interceptor that would need parameterization so we can put it in and use it as a test case? Well, mainly the good 'ol curly braces thing (and I have thus far never seen a good reason why one would put 'em on the same line), and also how portions of the class is delineated. I prefer the template used in WW, which had more clearly defined sections. The curly braces are a religious battle that no-one can ever win :-) The way the class is delineated is a function of the Jalopy template Patrick put in You can try to convince him to change it :-) Jason --- This sf.net email is sponsored by:ThinkGeek Welcome to geek heaven. http://thinkgeek.com/sf ___ Opensymphony-webwork mailing list [EMAIL PROTECTED] https://lists.sourceforge.net/lists/listinfo/opensymphony-webwork
