[Html-widget] RFC Handling uploads, WAS uploads in the results object?

[Carl Franks]

On 15/11/06, John Napiorkowski <jjn1056 at yahoo.com> wrote:
> --- Carl Franks <fireartist at gmail.com> wrote:
>
> > On 14/11/06, John Napiorkowski <jjn1056 at yahoo.com>
> > wrote:
> > >
> > > Okay, sorry to post so many brain dumps about this
> > > upload issue but now that I've spent some time
> > looking
> > > at the code for this and for the way uploads are
> > > handled in Catalyst and CGI.pm I have some
> > thoughts
> > > and would really like to get some opinions.
> > >
> > > First of all I don't think having the uploads
> > method
> > > or attribute should be an Apache::Uploads object.
> > > Right now there is nothing in the code enforcing
> > that,
> > > and although the upload reference is passed to
> > > contraints, filters and elements, nothing is being
> > > done with that so changing this won't break
> > anything,
> > > as far as I know.
> >
> > Agreed.
> >
> > > The documentation specifics that:
> > >
> > > my $w = HTML::Widget->new('widget');
> > >
> > > $w->uploads($arrayref)
> > >
> > > where the arrayref should be of Apache Upload
> > objects.
> > >  However my guess is that most people are using
> > this
> > > primarily in catalyst or for CGI development, and
> > > between the three methods for accessing uploads
> > > (Catalyst->request->uploads, Apache::Uploads and
> > > CGI.pm) there are significant differences.
> > >
> > > As far as I can see there are only two things to
> > do,
> > > either make a least common denominator structure
> > to
> > > pass to uploads or to create a new interface
> > objects
> > > that you'd need to build based on the upload
> > interface
> > > of your choice.
> > >
> > > For solution one that structure would probably
> > have to
> > > a hashref something like:
> > >
> > > {
> > >   uploadfieldname1 => {
> > >
> > >     fh => <filehandle>,
> > >     size => xxxxd,
> > >   },
> > >   uploadfieldname2 => {
> > >
> > >     fh => <filehandle>,
> > >     size => xxxxd,
> > >   },
> > > }
> > >
> > > Since there isn't much in common between the
> > standard
> > > ways I've found of accessing uploads.
> > >
> > > For the second method, create an interface, we
> > could
> > > do it to be the 'most common denominator' method,
> > > which would be a catalyst compatible hashref of
> > > objects.  Then we'd have parse plugins to convert
> > from
> > > CGI.pm, Apache::Upload and Catalyst to that common
> > > interface.
> >
> > Having $w->uploads() compatible with
> > $catalyst->request->uploads()
> > should be the top priority.
> >
> > Maybe for compatibility with CGI.pm, we could add a
> > $w->cgi_query()
> > which would use $q->upload() and $q->uploadInfo() to
> > fake up a
> > structure to give to $w->uploads().
> > Anyway, if you're not using CGI.pm yourself, don't
> > worry about that now.
> >
> > > Then it wouldn't be so hard to make the filters
> > and
> > > constraints work.
>
> I'm starting to wonder if separating the uploads from
> the regular params is the right thing to do.  As i
> test I did the following (in catalyst):
>
> my @keys = values %{$c->request->uploads};
>
> foreach my $field (@keys)
> {
>    $c->request->parameters->{$field} =
>       $c->request->upload($field);
> }
>
> And then passed this on to 'process'.  The effect of
> this was that I was able to write filters and
> callbacks in the usual way, except instead of a scalar
> value in $element->value you get a reference to that
> catalyst request upload object.  From there you could
> call methods like slurp to get the content, although I
> think it would not be hard to add overload to
> stringify the object for compatibility.

In the past I was considering a cat plugin which edited
$c->request->parameters, but was told it was very bad practice to
change anything on the Request.
Doing this would likely make a lot of people refuse to use H-W at all
- for this reason alone.

> I think this way we could get the length constraints
> and so forth working pretty well.
>
> I think this could be the right direction.  Someday if
> we want to support other form systems, such as xforms,
> that might submit their data in the post, we will have
> to more away from the idea of seperating uploads from
> form params anyway.
>
> I know it's a bigger change, we could leave it for
> another time if people are not comfortable.  If so one
> thing I'd really like to change is to have uploads
> expect a hashref ala Cataly over the arrayref that
> currently documented.  As you said, Catalyst
> compatibility is most important right now so I don't
> think that's an issue.

Yup, it doesn't work anyway - so just change the docs to how we want it to work.

> >
> > If you already have an svn account on
> > dev.catalystframework.org - just
> > ask an admin for write access to the HTML-Widget
> > directory.
> > Otherwise a patch (preferably with tests) against
> > svn would ge great.
>
> I don't currently have svn to that server, so I'll
> send a patch for now.  Thanks for your thoughts.

Cool.
Carl