UploadField documentation

345 views
Skip to first unread message

Martimiz

unread,
Oct 16, 2012, 1:29:29 PM10/16/12
to silverst...@googlegroups.com
Hi all - and especially Zauberfish :-)

I've been experimenting with the UploadField in 3.0.2 (nice!) and I couldn't find any official documentation yet. Maybe I just somehow missed it? So I did some documenting for myself, and thought I might just as well... Anyway, here it is:

https://github.com/Martimiz/sapphire/blob/3.0-DocUploadField/docs/en/reference/uploadfield.md

I stuck to documenting the use within in the CMS, as I feel it isn't quite supporting frontend use yet (or is it?). So unless someone else has already done the work, would you care to take a look and see what I'm missing?

Martine


Sam Minnée

unread,
Oct 16, 2012, 4:24:07 PM10/16/12
to silverst...@googlegroups.com, silverst...@googlegroups.com
This is awesome! Is there a pull request yet?

Zauberfisch

unread,
Oct 16, 2012, 4:57:31 PM10/16/12
to silverst...@googlegroups.com
Wonderful

docs where certainly missing, and I am to be blamed for it, thank you very much!
And those look pretty good, I just found 1 little error and added max file size to the docs
pull request sent to you (Martimiz) https://github.com/Martimiz/sapphire/pull/1

After that, I think you are ready to pull request to silverstripe

good work, thanks again!

Martimiz

unread,
Oct 17, 2012, 5:18:42 AM10/17/12
to silverst...@googlegroups.com
Thanks guys,

I've merged zauberfish's pull request, and sent one.

I've one question popping up just now, although its not an UploadField issue as such: Max filesize works on large files, as in bytes, but not on large files in pixels that are extremely compressed! Example: cad files. Once uploaded GD will (try to) load them in memory and that's when things can get really bad. I know it doesn't happen very often, but when it does...

A while back I created a 'hack' for the people at duurza.am on version 2.4 to take care of that. This impacts Upload and file syncing. Would it be an idea to do something similar for 3.0 - but then as a permanent fixture - or would you consider this too minor an issue?

Martine

wolfv

unread,
Oct 17, 2012, 11:25:13 AM10/17/12
to silverst...@googlegroups.com
Very nice, actually, the missing documentation of the need of $many_many instead of $has_many was one of the reasons that led me to believe that silverstripe 3.0.2 is still unstable and not ready for production.... $has_many breaks everything regarding the uploadfield.

IMO there should have been at least a little hint somewhere ... and this doc looks great :)

Regards,

Wolf

Martimiz

unread,
Oct 17, 2012, 12:10:43 PM10/17/12
to silverst...@googlegroups.com

Thanks :-)


$has_many breaks everything regarding the uploadfield.


Actually I found out just now that it does work! And I think it should be added to the docs, if someone can replay and/or confirm?

Point 1 is: you need an Image class that has a $has_one relation to the page (or object) that displays the field. This means that you cannot just use the Image class like that, since it lacks that relation. You could probably add it using a decorator, or just use a class GalleryImage extends Image, like this:

class GalleryItem extends Image {

    static $has_one = array(
        'GalleryPage' => 'GalleryPage'
    );  
}


The second thing is you need to tell the UploadField to use that relation, since it doesn't seem to be able to autodetect it. So this is what I have next:

class GalleryPage extends Page {

    static $has_many = array(
        'GalleryImages' => 'GalleryItem'
    );

    function getCMSFields() {

        $fields = parent::getCMSFields();

        $fields->addFieldToTab(
            'Root.Upload', 
            $uploadField = new UploadField(
                $name = 'GalleryImages',
                $title = 'Please upload some images'
            )  
        );
        $uploadField->setFolderName('whisky');

        return $fields;        
    }  
}
class GalleryPage_Controller extends Page_Controller {}


This now works in version 3.0.2 dd 16-10-2012

One thing that makes experimenting with this class a bit difficult is that it doesn't handle errors very well - a simple reloading of the CMS isn't always enough to get rid of javascript instabilities. What I often do is select settings, select pages, then reload the cms and go back to the page :-) So don't give up too soon...

Martine

Martimiz

unread,
Oct 17, 2012, 12:20:14 PM10/17/12
to silverst...@googlegroups.com
Sorry - the error persistance again made me think the relation wasn't autodetected, which it was, after several reloads. So it seems you really only need to have the Image class $has_one in place... And as I inadvertantly left out the trird parameter anyway, the code above is exactly what I have working now :-)

Martine

Jakob Kristoferitsch

unread,
Oct 17, 2012, 12:33:21 PM10/17/12
to silverst...@googlegroups.com
Martimiz wrote
> Max filesize works on large files, as in bytes, but not on large
> files in pixels that are extremely compressed! Example: cad files.
> Once uploaded GD will (try to) load them in memory and that's when
> things can get really bad. I know it doesn't happen very often, but
> when it does...

Hi Martine,
I once created a workaround for this problem, maybe not the cleanest
one, but it works. If gd comes across a huge file, it will run out of
memory and PHP will stop with a fatal error every time it tries to
resize that file.
My assumption was, that there is no way to predict at what file size
that will happen (because of different compression levels) and I don't
know of any other information that could prevent running out of memory,
so there is no way to prevent the first crash

Therefore, I just tried to prevent any further crash after that first
one. The way I tried to do this is explained in ticket
<http://open.silverstripe.org/ticket/5284>

In summary: Create a dummy file before resizing, delete it afterwards.
If the file is present, we assume that we crashed the last time that we
tried to read the file, so we skip resizing. Furthermore, we take care
that a dummy file is deleted if the image is deleted.

It was never included because it lacks tests, but I don't really know
any test cases that make sense. If someone can up with a reasonable test
case and there is an interest in this, I could reimplement that patch
for SS 3.0 and create a pull request.

Yours
Jakob

Martine Bloem

unread,
Oct 17, 2012, 1:01:21 PM10/17/12
to silverst...@googlegroups.com
Hi jacob,
Nice knowing that I wasn't the only one to tackle this :)

What I did was, as i remember it, based on a GD function that lets you find out the pixelsize. I did the memorytests, and memory never spiked, so i guess it doesn't need to expand the file to get that information. Checking this before any resizing gets done would prevent silverstripe from accepting these files - writing and/or syncing.

When a user tried to upload a file by ftp, it was also refused - and in this case even renamed, so that the admin could still trace these files, but silverstripe would never acknowledge them.

Something like that too might be an option..?
> --
> You received this message because you are subscribed to the Google Groups "SilverStripe Core Development" group.
> To post to this group, send email to silverst...@googlegroups.com.
> To unsubscribe from this group, send email to silverstripe-d...@googlegroups.com.
> For more options, visit this group at http://groups.google.com/group/silverstripe-dev?hl=en.
>

Jakob Kristoferitsch

unread,
Oct 17, 2012, 1:41:22 PM10/17/12
to silverst...@googlegroups.com
After I wrote the mail I also checked that, and it seems that
getimagesize only reads the header of the images to calculate the image
size, so it should not need to read the whole image. Taking
width*height*4(rgba) should give a reasonable estimate of the image size
in memory. If that is larger than the available memory (memory_limit -
memory_get_usage()), we could reject the image.

In short it should work that way. I'll give it a try during the weekend.

Yours
Jakob

Sam Minnée

unread,
Oct 17, 2012, 8:43:12 PM10/17/12
to silverst...@googlegroups.com
On 18/10/2012, at 5:33 AM, Jakob Kristoferitsch wrote:

Martimiz wrote
> Max filesize works on large files, as in bytes, but not on large
> files in pixels that are extremely compressed! Example: cad files.
> Once uploaded GD will (try to) load them in memory and that's when
> things can get really bad. I know it doesn't happen very often, but
> when it does...

Hi Martine,
I once created a workaround for this problem, maybe not the cleanest one, but it works. If gd comes across a huge file, it will run out of memory and PHP will stop with a fatal error every time it tries to resize that file.
My assumption was, that there is no way to predict at what file size that will happen (because of different compression levels) and I don't know of any other information that could prevent running out of memory, so there is no way to prevent the first crash

Has anyone tested whether http://nz.php.net/manual/en/function.getimagesize.php crashes on very large images?  My suspicion/hope is that it just reads the header of the file.  If that's the case, we could add some code that checked the image size with getimagesize before loading.

If that's the case, I would suggest adding the ability to set maxWidth, maxHeight or maxPixels (just W x H) for either site-wide, or for specific upload fields.

As a default, perhaps we set max pixels to 10,000,000?

Martine Bloem

unread,
Oct 18, 2012, 6:47:25 AM10/18/12
to silverst...@googlegroups.com
I tested it on 6000 x 6000 px images, I recall, and it never crashed, where it definitely did without the check. I had a maxwidth/maxheight, but a maxpixels would be a better choice I think. I could adapt my code for 3.0 if you wish, so it can be tested.Ii've got it documented anyway..

Martine
--

Simon J Welsh

unread,
Oct 18, 2012, 6:56:41 AM10/18/12
to silverst...@googlegroups.com
<?php
var_dump($start = memory_get_usage());
$size = getimagesize('/Users/simon/Downloads/20121016_190253.jpg');
var_dump($size, $end = memory_get_usage());
var_dump($end - $start);

This showed a difference of about 2 KB between $end and $start. The image is around 3.2 MB.
---
Simon Welsh
Admin of http://simon.geek.nz/

Martimiz

unread,
Oct 18, 2012, 9:11:24 AM10/18/12
to silverst...@googlegroups.com

Please have a look at this:

https://github.com/Martimiz/sapphire/commit/b6c29366f00489ff349df32bb72358a4e467b605

I tested it with a 100.000.000 pixel png (10.000x10.000px) - 172 KB on disk) and found no memory issues. I set the max-resolution to 400.000 but that of course is arbitrary. A simular approach is needed for file synchronization, if this solution is acceptable to you



Op donderdag 18 oktober 2012 12:56:50 UTC+2 schreef Simon het volgende:

Uncle Cheese

unread,
Oct 18, 2012, 11:37:16 AM10/18/12
to silverst...@googlegroups.com
Using an UploadField to manage a $has_many relation is a bad idea, IMO, because it precludes you from using existing files. If one UploadField selects a file that is already in use on another UploadField, you've now broken the previous relationship without even knowing it.

There's no benefit that I can see in using $has_many over $many_many when managing files. The latter is much more scalable and forgiving.

djd4ws0n

unread,
Oct 18, 2012, 12:07:59 PM10/18/12
to silverst...@googlegroups.com
After seeing this thread, it made me realize that this was exactly the documentation I was looking for with a recent project.

However, it lead me to a use case in which UploadField breaks the CMS.

Imagine the example;

I have a DataObject, called Catgeory. A Category can have many images, and images can belong to many categories. I added an UploadField to the Category dataobject, and it worked fine. However, the problem stems from the fact that I plan to add quite a lot of images to a category. Since UploadField isn't paginated, it loads in all of the associated items, causing the CMS to whitescreen.

I came up with multiple solutions;

* Defer the listing to a GridField instance, where we can make use of the pagination aspects.
* Hide the related images from the UploadField field.
* Limit the UploadField field's display, to only the last 10~20 items that were uploaded to it.

I'd like to try and implement one of the solutions, so I'm looking for any potential pitfalls I may hit on the way.

Cheers,

Steve.

Uncle Cheese

unread,
Oct 18, 2012, 12:16:32 PM10/18/12
to silverst...@googlegroups.com
I'm not sure this is the intended use of UploadField. This type of scenario is another reminder of the need for a some kind of gallery UI in SS3 the way that FileDOM and ImageDOM worked in SS2. The GridField bulk upload component is a start, but we really need GF to do a much better job handling image-containing DataObjects.

Sam Minnée

unread,
Oct 18, 2012, 3:51:58 PM10/18/12
to silverst...@googlegroups.com, silverst...@googlegroups.com
Agreed. I suggest that we include a note in the docs advising people to use has_one or many_many.

Martine Bloem

unread,
Oct 18, 2012, 4:27:19 PM10/18/12
to silverst...@googlegroups.com
I feel that just a warning is not really enough...$has_many is a very legitimate relation for Image subclasses and UploadField halfway works for it. So I feel you can't really buy that off by saying 'hey, you should have studied the docs better'...

So maybe
have uploadfield display a message when it detects any has_one, and not work at all
Or
Make a distinction between valid (have a has_one counterpart) and non-valid has_many's.
Valids:
- the 'existing files' button is disabled or
- a new record is created for 'existing files' if a different parent is detected
Non-valids just get the message

Sam Minnée

unread,
Oct 18, 2012, 4:35:33 PM10/18/12
to silverst...@googlegroups.com

On 19/10/2012, at 9:27 AM, Martine Bloem wrote:

> I feel that just a warning is not really enough...$has_many is a very legitimate relation for Image subclasses and UploadField halfway works for it. So I feel you can't really buy that off by saying 'hey, you should have studied the docs better'...

Agreed, but people reading the documentation should know that it's generally a bad idea. This is the kind of area where if someone came to me and said they wanted to have a has_many relationship, I would be trying to talk them out of it, and getting them to use many_many.

Incidentally, creating a subclass of Image rather than applying and extension to Image or File to add the necessary has_one counterpart relation is another thing that I would try and talk them out of, probably using our experiences with Member subclasses as an example.

> So maybe
> have uploadfield display a message when it detects any has_one, and not work at all

I assume you mean when it *doesn't* detect any has_one?

> Or
> Make a distinction between valid (have a has_one counterpart) and non-valid has_many's.
> Valids:
> - the 'existing files' button is disabled or
> - a new record is created for 'existing files' if a different parent is detected
> Non-valids just get the message

I'd say that attempting to create an invalid data model should throw an exception. It's a bug, like a misspelled classname or a missing semicolon, and SilverStripe will be doing the developer if it explains this loudly and clearly.
This should probably be at the DataObject level rather than the UploadField.

At the UploadField level, it should throw an exception if the relation in question doesn't point to a subclass of File.

Sam Minnée

unread,
Oct 18, 2012, 4:38:03 PM10/18/12
to silverst...@googlegroups.com
Yeah, agreed.

There was talk of refactoring the <table> generating bit of GridField into a GridFieldTableView component, and letting developers replace that with a GridFieldGalleryView.  It kind of abstracts GridField even further, but it seems like a better option than creating a separate GalleryField.

One challenge would be building it in such a way that still allowed for the creation of a "GridFieldViewSelector" component, that let you flip between grid and table views.

Jakob Kristoferitsch

unread,
Oct 19, 2012, 2:22:09 AM10/19/12
to silverst...@googlegroups.com
Hi Martine,
I don't know if the upload field is the right place to do this check. I
think that it should be done in filesystem/GD.php, after line 29:

list($width, $height, $type, $attr) = getimagesize($filename);

I also think that it should not be a hard limit, but take the available
memory into account. One way of doing that would be to check

memory_get_usage()*width*height*4<value of memory_limit

Two problems with that approach:
1.) Checking in GD decouples the source of the problem (the file upload)
from the error, so maybe it's better to do the check in the upload field
(as you suggested).
2.) Some shared hosters have a hard memory limit that is significantly
lower than the memory_limit. An example for this is 1&1 who have
memory_limit set to 128MB but PHP runs out of memory after 30MB. I know
that running SilverStripe on a shared host is not supported, but that's
what I (and I assume many others) use, so I think those edge cases
should be taken into account (if possible).

Yours
Jakob
Am 18.10.2012 15:11, schrieb Martimiz:
>
> Please have a look at this:
>
> https://github.com/Martimiz/sapphire/commit/b6c29366f00489ff349df32bb72358a4e467b605
>
> I tested it with a 100.000.000 pixel png (10.000x10.000px) - 172 KB on
> disk) and found no memory issues. I set the max-resolution to 400.000
> but that of course is arbitrary. A simular approach is needed for file
> synchronization, if this solution is acceptable to you
>
>
>
> Op donderdag 18 oktober 2012 12:56:50 UTC+2 schreef Simon het volgende:
>
> <?php
> var_dump($start = memory_get_usage());
> $size = getimagesize('/Users/simon/Downloads/20121016_190253.jpg');
> var_dump($size, $end = memory_get_usage());
> var_dump($end - $start);
>
> This showed a difference of about 2 KB between $end and $start. The
> image is around 3.2 MB.
>
> On 18/10/2012, at 11:47 PM, Martine Bloem <mart...@gmail.com

Martimiz

unread,
Oct 19, 2012, 8:46:44 AM10/19/12
to silverst...@googlegroups.com
@sam, @unclecheese

>> ... people reading the documentation should know that it's generally a bad idea.

You've convinced me too now :-( I should probably have listened, but for some reason I just really wanted it to work. Although the UploadField does recognize valid has_many's, there is still too much going wrong, so maybe it should rather just throw an error and be done with it for now. Not very hard to accomplish, that :-)

As for subclassing vs decorating - it looks like UploadField is handling both well. The only thing is that if you subclass and then use 'From file' you can only see files already belonging to the subclass. Although technically correct I suppose, I can see how that could be confusing.

If you wish me to, I'll rewrite the doc to use a decorator for the many_many instead. I'll include a warning against has_many anyway.  

@jacob

> I don't know if the upload field is the right place to do this check. I
> think that it should be done in filesystem/GD.php, after line 29:

I'm not checking the size in the UploadField, but in the Upload class. :-) I think preventing an image from being uploaded would be a task for the class that manages uploads, although the resolution check functionality itself should probably be part of GD, as you suggest. Something like:

in Upload:
if (!GD::check_max_resolution($tmpFile['tmp_name'], $maxResolution)) {
    $this->errors[] = ... 
    return false;
}
In GD:
static function check_max_resolution($fileName, $maxResolution) {
    ....
}
























}

Martine


Op vrijdag 19 oktober 2012 08:22:03 UTC+2 schreef Jakob Kristoferitsch het volgende:

djd4ws0n

unread,
Oct 19, 2012, 9:55:14 AM10/19/12
to silverst...@googlegroups.com
In light of this, I think I'll return to the drawing board to see if I can't come up with a better solution to the problem than trying bend something against its will as it were.

Cheers,
Steve.

wolfv

unread,
Oct 20, 2012, 3:30:52 AM10/20/12
to silverst...@googlegroups.com
Hi thanks for playing with this -- cool that it works as well. But as Uncle Cheese say's now that I've seen how it *should* be done (and $many_many really does make more sense somehow) I guess I'll stick with it.

It's great that you made docs for this, and hope other developer will get to know these »best practices« earlier :)

Jakob Kristoferitsch

unread,
Oct 21, 2012, 4:27:38 PM10/21/12
to silverst...@googlegroups.com
martine wrote:
> *In GD:*
> static function check_max_resolution($fileName, $maxResolution) {

Hi Martine,
I created a branch and implemented something close to your idea in GD,
see <https://github.com/jakr/sapphire/tree/trac5284>.

The method image_fits_available_memory gives a rough estimate if the
image will fit in the memory (based on memory_limit, memory usage and
image size - width*height*4). This can fail for several reasons:
1. Some Hosters have a hard php memory limit that is lower than that set
in memory_limit. This is mainly the case for low end products of shared
hosters (my experience is from 1&1), which is arguably not a supported
configuration. But since it's a problem for me I would like to fix it :-).
2. Apparently some image formats take more space than that estimate
suggests - code examples I found (e.g. on php.net) include an image
format dependent fudge factor.

For these two reasons, I have implemented the lock file mechanism I
described earlier as a fallback. If GD crashes while resizing an image,
that image will be ignored in the future.

I am not set on that implementation, so if you (or other developers)
think that this additional step is unnecessary or have a better idea
feel free to exclude that part.

Yours
Jakob

Am 19.10.2012 14:46, schrieb Martimiz:
> @sam, @unclecheese
>
> >> ... people reading the documentation should know that it's generally
> a bad idea.
>
> You've convinced me too now :-( I should probably have listened, but for
> some reason I just really wanted it to work. Although the UploadField
> does recognize valid has_many's, there is still too much going wrong, so
> maybe it should rather just throw an error and be done with it for now.
> Not very hard to accomplish, that :-)
>
> As for subclassing vs decorating - it looks like UploadField is handling
> both well. The only thing is that if you subclass and then use 'From
> file' you can only see files already belonging to the subclass. Although
> technically correct I suppose, I can see how that could be confusing.
>
> If you wish me to, I'll rewrite the doc to use a decorator for the
> many_many instead. I'll include a warning against has_many anyway.
>
> @jacob
> > I don't know if the upload field is the right place to do this check. I
> > think that it should be done in filesystem/GD.php, after line 29:
>
> I'm not checking the size in the UploadField, but in the Upload class.
> :-) I think preventing an image from being uploaded would be a task for
> the class that manages uploads, although the resolution check
> functionality itself should probably be part of GD, as you suggest.
> Something like:
>
> *in Upload:*
> if (!GD::check_max_resolution($tmpFile['tmp_name'], $maxResolution)) {
>
> $this->errors[] = ...
>
> return false;
> }
> *In GD:*

Martimiz

unread,
Oct 22, 2012, 6:37:44 AM10/22/12
to silverst...@googlegroups.com
Documentation:
I've added a warning not to use has_many relations, and I've replaced the example using subclassed Images by one using a DataExtension.

Revised version here:
https://github.com/Martimiz/sapphire/blob/3.0-DocUploadField/docs/en/reference/uploadfield.md

Pull request:
https://github.com/silverstripe/sapphire/pull/885

Martine

Martimiz

unread,
Oct 22, 2012, 7:47:21 AM10/22/12
to silverst...@googlegroups.com
Hi all,

The large images issue got very mixed up with the UploadFile documentation, my fault for adding it into this thread in the first place! So I took the liberty of setting up a new thread here, hope tyou're allright with that:

Block high compression/large dimension images, that crash the system on resize 

Thanks, Martine
Reply all
Reply to author
Forward
0 new messages