You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@struts.apache.org by bu...@apache.org on 2006/02/11 06:30:38 UTC
DO NOT REPLY [Bug 38613] New: - Multipart Command Implementation
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG�
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
<http://issues.apache.org/bugzilla/show_bug.cgi?id=38613>.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND�
INSERTED IN THE BUG DATABASE.
http://issues.apache.org/bugzilla/show_bug.cgi?id=38613
Summary: Multipart Command Implementation
Product: Struts
Version: 1.3.0
Platform: Other
OS/Version: other
Status: NEW
Severity: enhancement
Priority: P2
Component: Action
AssignedTo: dev@struts.apache.org
ReportedBy: niallp@apache.org
I'm proposing that the multipart processing be separated out into a set of
Commands in a separate chain:
<!-- ========== Mulitpart Processing chain ========== -->
<chain name="process-multipart">
<!-- Check if the request is a multipart request -->
<command className="o.a.s.c.c.servlet.CheckIfMultipartRequest"/>
<!-- Create the multipart handler -->
<command className="o.a.s.c.c.CreateMultipartHandler"/>
<!-- Wrap the multipart request -->
<command className="o.a.s.c.c.servlet.WrapMultipartRequest"/>
<!-- See if the Struts Multipart Handler can process the request -->
<command className="o.a.s.c.c.servlet.StrutsMultipartHandler"/>
</chain>
The main "process-action" chain would be modified to use a LookupCommand that
ignores the result of this new "process-multipart" chain. This will allow
multiple multipart handlers to be added to the chain, with the one that
actually handles the request returning "true" to stop processing of
the "process-multipart" chain (but not "process-action" chain).
This proposal has a number of benefits:
1) All mutlipart handling is now in Commands which can easily be configured
2) Removed request "wrapping" code from ComposableRequestProcessor - no need to
sub-class ComposableRequestProcessor to modify the "wrapping".
3) RequestUtils's static populate() method is redundant - all that code is now
in commands - making custom multipart (and form population) processing much
easier.
4) Multiple multipart handlers can be easily configured by simply adding
additional commands to the multipart chain.
--
Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org
DO NOT REPLY [Bug 38613] - Multipart Command Implementation
Posted by bu...@apache.org.
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG�
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
<http://issues.apache.org/bugzilla/show_bug.cgi?id=38613>.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND�
INSERTED IN THE BUG DATABASE.
http://issues.apache.org/bugzilla/show_bug.cgi?id=38613
------- Additional Comments From husted@apache.org 2006-02-11 14:19 -------
I haven't worked with MultipartHanders, so I can't speak to the code, but it
does sound fine in principle.
The only thing is that I was about to make the changes described by #38374
(Cancel) this morning, which involves some of the commands affected by this patch.
If we are going to do this, then let's do it immediately, so that we can also do
#38374, and ship it.
I have time set aside on Monday to like to roll 1.3.0, but after that I'll be
heads-down on other things for several weeks. So, it's now or its 1.3.1.
--
Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org
DO NOT REPLY [Bug 38613] - Multipart Command Implementation
Posted by bu...@apache.org.
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG�
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
<http://issues.apache.org/bugzilla/show_bug.cgi?id=38613>.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND�
INSERTED IN THE BUG DATABASE.
http://issues.apache.org/bugzilla/show_bug.cgi?id=38613
------- Additional Comments From niallp@apache.org 2006-02-11 06:37 -------
Created an attachment (id=17649)
--> (http://issues.apache.org/bugzilla/attachment.cgi?id=17649&action=view)
New CheckIfMultipartRequest Command
The new CheckIfMultipartRequest command returns false if its a multipart
request (to continue processing) or true if its not (to stop further
processing).
--
Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org
DO NOT REPLY [Bug 38613] - Multipart Command Implementation
Posted by bu...@apache.org.
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG�
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
<http://issues.apache.org/bugzilla/show_bug.cgi?id=38613>.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND�
INSERTED IN THE BUG DATABASE.
http://issues.apache.org/bugzilla/show_bug.cgi?id=38613
------- Additional Comments From niallp@apache.org 2006-02-11 06:46 -------
Created an attachment (id=17653)
--> (http://issues.apache.org/bugzilla/attachment.cgi?id=17653&action=view)
New StrutsMultipartHandler Command
StrutsMultipartHandler processes the mutlipart request if the handler is a
o.a.s.u.MultipartRequestHandler and returns true to stop processing - if its
not an o.a.s.u.MultipartRequestHandler it return false to carry on in the
"multipart" chain.
--
Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org
DO NOT REPLY [Bug 38613] - Multipart Command Implementation
Posted by bu...@apache.org.
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG�
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
<http://issues.apache.org/bugzilla/show_bug.cgi?id=38613>.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND�
INSERTED IN THE BUG DATABASE.
http://issues.apache.org/bugzilla/show_bug.cgi?id=38613
------- Additional Comments From niallp@apache.org 2006-02-12 07:39 -------
(In reply to comment #11)
> I like the idea of turning the multipart handling into a chain. However, I
> have some concerns about the implementation. For example, I'm not thrilled
> about ActionContextBase having to know about multipart handlers.
It doesn't have to - it could just be in the ServletActionContext - or shoved
in the request (or wrapped request). Its just being used to pass the handler
from the Command that creates it to the following Command(s) that use it.
> I'm also concerned about preserving the existing context when the request is
> wrapped;
Can't think of a reason why, but it sounds like a good plan. Rather than re-
intializing the underlying ServletWebContext - it could be left alone and
the "wrapped" request stored in the ServletActionContext.
> I'm afraid I haven't had enough time to dig into the code yet to see it that
> all works properly. Finally, this makes it so easy to customise multipart
> handling that I'd much prefer to expose a cleaned up interface rather than
> the very clunky one we've been living with since pre-1.0 days. On the plus
> side, this has the potential to re-introduce the ability to disable multipart
> handling completely, which is something we had in early Struts versions that
> got lost somewhere along the way.
My motivation is to disentangle the multipart processing from the form
population and factor it out of the static RequestUtils method into a set of
classes that can be re-used. Once its out of RequestUtils then I think the
struts MultipartRequestHandler could be deprecated and a native Commons
FileUpload Command implemented - or any other flavour of file upload processing.
> Really, multipart support should be moving to a filter now, rather than being
> so intertwined with the request processing.
IMO there are pros and cons to both approaches and offering both flavours seems
like a good choice. If its in the standard chain config, its one less thing for
users to have to configure and with a Filter you can only put it before the
request processing starts. IMO it makes more sense to only do the multipart
processing after the user has been authorized to execute the action. But mainly
I think its about user preference - I'm sure there would be die-hard advocates
of both :-)
> I actually have the implementation for this as part of FileUpload sitting on
> my disk, but I wasn't happy enough with it to put it in FileUpload 1.1. Of
> course, Struts will still have to recognise the wrapped request, with the
> file items in it, but it will certainly reduce the amount of custom code in
> Struts.
Thanks for the feedback, I'll re-work some of this and post a modified version.
--
Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org
DO NOT REPLY [Bug 38613] - Multipart Command Implementation
Posted by bu...@apache.org.
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG�
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
<http://issues.apache.org/bugzilla/show_bug.cgi?id=38613>.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND�
INSERTED IN THE BUG DATABASE.
http://issues.apache.org/bugzilla/show_bug.cgi?id=38613
------- Additional Comments From niallp@apache.org 2006-02-11 06:44 -------
Created an attachment (id=17652)
--> (http://issues.apache.org/bugzilla/attachment.cgi?id=17652&action=view)
New AbstractMultipartHandler Command
AbstractMultipartHandler is a base Command for multipart handling Command
implementations. It returns true if the Command can handle the multipart
request (and stop subsequent Commands processing) or false if it can't
--
Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org
DO NOT REPLY [Bug 38613] - Multipart Command Implementation
Posted by bu...@apache.org.
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG�
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
<http://issues.apache.org/bugzilla/show_bug.cgi?id=38613>.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND�
INSERTED IN THE BUG DATABASE.
http://issues.apache.org/bugzilla/show_bug.cgi?id=38613
------- Additional Comments From niallp@apache.org 2006-02-11 06:39 -------
Created an attachment (id=17650)
--> (http://issues.apache.org/bugzilla/attachment.cgi?id=17650&action=view)
New CreateMultipartHandler Command
The CreateMultipartHandler command creates the handler for the mutlipart
request and stores it in the Context. If it can't be created it returns true to
stop further processing, otherwise false to carry on.
--
Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org
DO NOT REPLY [Bug 38613] - Multipart Command Implementation
Posted by bu...@apache.org.
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG�
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
<http://issues.apache.org/bugzilla/show_bug.cgi?id=38613>.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND�
INSERTED IN THE BUG DATABASE.
http://issues.apache.org/bugzilla/show_bug.cgi?id=38613
------- Additional Comments From niallp@apache.org 2006-02-11 06:41 -------
Created an attachment (id=17651)
--> (http://issues.apache.org/bugzilla/attachment.cgi?id=17651&action=view)
New WrapMultipartRequest Command
The WrapMultipartRequest command calls the ActionContext's
wrapMulitpartRequest() method (a new method) to wrap the request.
--
Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org
DO NOT REPLY [Bug 38613] - Multipart Command Implementation
Posted by bu...@apache.org.
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG�
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
<http://issues.apache.org/bugzilla/show_bug.cgi?id=38613>.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND�
INSERTED IN THE BUG DATABASE.
http://issues.apache.org/bugzilla/show_bug.cgi?id=38613
------- Additional Comments From niallp@apache.org 2006-02-11 15:47 -------
Ahh I didn't realize 1.3 was so imminent and I was hoping to give more time for
a heads up on this. While I'm keen to see this go in for 1.3, I also don't want
1.3 to slip because of it.
How about if you go ahead and do Bug 38374 and I'll leave this until tommorrow
evening (GMT) - from a quick look it should be straight forward to merge the
changes for Bug 38374 into my code pretty easily.
Does that sound reasonable or would you prefer commit now and ask for
forgiveness later?
--
Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org
DO NOT REPLY [Bug 38613] - Multipart Command Implementation
Posted by bu...@apache.org.
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG�
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
<http://issues.apache.org/bugzilla/show_bug.cgi?id=38613>.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND�
INSERTED IN THE BUG DATABASE.
http://issues.apache.org/bugzilla/show_bug.cgi?id=38613
------- Additional Comments From martinc@apache.org 2006-02-12 06:15 -------
I like the idea of turning the multipart handling into a chain. However, I have
some concerns about the implementation. For example, I'm not thrilled about
ActionContextBase having to know about multipart handlers. I'm also concerned
about preserving the existing context when the request is wrapped; I'm afraid I
haven't had enough time to dig into the code yet to see it that all works
properly. Finally, this makes it so easy to customise multipart handling that
I'd much prefer to expose a cleaned up interface rather than the very clunky one
we've been living with since pre-1.0 days. On the plus side, this has the
potential to re-introduce the ability to disable multipart handling completely,
which is something we had in early Struts versions that got lost somewhere along
the way.
Really, multipart support should be moving to a filter now, rather than being so
intertwined with the request processing. I actually have the implementation for
this as part of FileUpload sitting on my disk, but I wasn't happy enough with it
to put it in FileUpload 1.1. Of course, Struts will still have to recognise the
wrapped request, with the file items in it, but it will certainly reduce the
amount of custom code in Struts.
--
Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org
DO NOT REPLY [Bug 38613] - Multipart Command Implementation
Posted by bu...@apache.org.
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG�
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
<http://issues.apache.org/bugzilla/show_bug.cgi?id=38613>.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND�
INSERTED IN THE BUG DATABASE.
http://issues.apache.org/bugzilla/show_bug.cgi?id=38613
------- Additional Comments From niallp@apache.org 2006-02-11 06:34 -------
Created an attachment (id=17648)
--> (http://issues.apache.org/bugzilla/attachment.cgi?id=17648&action=view)
Modifications to existing struts classes
Changes to the following:
- chain-config.xml
- AbstractPopulateActionForm (Command)
- PopulateActionForm (Command)
- ValidateActionForm (Command)
- ComposableRequestProcessor
- Constants
- ActionContext
- ActionContextBase
- ServletActionContext
- Globals
--
Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org
DO NOT REPLY [Bug 38613] - Multipart Command Implementation
Posted by bu...@apache.org.
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG�
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
<http://issues.apache.org/bugzilla/show_bug.cgi?id=38613>.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND�
INSERTED IN THE BUG DATABASE.
http://issues.apache.org/bugzilla/show_bug.cgi?id=38613
------- Additional Comments From niallp@apache.org 2006-02-11 06:51 -------
Created an attachment (id=17654)
--> (http://issues.apache.org/bugzilla/attachment.cgi?id=17654&action=view)
LookupIgnoreResult Command
This LookupIgnoreResult commands extends the Commons Chain LookupCommand, but
it always returns "false" from the execute() method. This is so that a
sub-chain can execute and one of the Commands return "true" to stop the chain
executing, without affecting the main chain it was called from.
The Commons Chain Lookup Command currently has this functionality (it has an
"ignoreExecuteResult" property) - but it hasn't been released yet. After the
next Commons Chain release, this Command can be removed.
--
Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org
DO NOT REPLY [Bug 38613] - Multipart Command Implementation
Posted by bu...@apache.org.
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG�
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
<http://issues.apache.org/bugzilla/show_bug.cgi?id=38613>.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND�
INSERTED IN THE BUG DATABASE.
http://issues.apache.org/bugzilla/show_bug.cgi?id=38613
------- Additional Comments From niallp@apache.org 2006-02-11 07:37 -------
Bug 30210 is resolved by this enhancement as the multipart processing now takes
place before the ActionForm's reset() method is called.
Bug 31922 is resolved by this enhancement as everything about multipart
handling would be plugable.
Bug 33247 is resolved by this enhancement.
I believe other multipart issues will also be easier for people to resolve,
since in general this makes plugging in custom multipart handling much easier.
--
Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org