Filemanager does not honor the maximum files per directory setting.

I found the cause of the bug. Here is a patch that fixes it.

It would take some scripting or something fancy. If you aren't handy at that, I could probably help you out. It might take a day or two to get it all straight.

Can someone explain to me how the patch works. It seems that the if-statement in question "if ($directory->directory > $file->directory || $directory->filecount < variable_get('filemanager_max_file_count', '2000')) " will allow you to bypass the maximum files per directory setting if you meet the criteria ($directory->directory > $files->directory). For the life of me, I have no idea why you would want this.

Frankly, I am not sure what the original code intended (I am confused by the comment) but the patch is at least comparing integer to integer rather than object to integer. Though, now that I look closer, I think that it is a silly comparison.

I may be missing something, but why isn't that line more like

if ($directory->filecount < variable_get('filemanager_max_file_count', '2000')) {
...

I don't really understand what the directory comparison is all about. As long as the directory has an empty slot, the file should be allowed to be added to it.

here is an updated patch.

My friend just figured out the comment (the author's intentions) and why the directory comparison is needed.

So if there is directory 0,1,2 and 4, the directory comparison will make sure that directory 3 is created and used instead. It's also meant to keep the $file->directory and $directory->directory in sync...
If in an extreme example,
a) there is directory 0,1,2 and 100
b) directory 0,1 and 2 are full

Iterating through the loop:
loop 1: (full)
$directory->directory = 0 $file->directory = 0
loop 2: (full)
$directory->directory = 1 $file->directory = 1
loop 3: (full)
$directory->directory = 2 $file->directory = 2
loop 4:
$directory->directory = 100 $file->directory = 3

It's not harmful, but still a bit weird. Though this example also shows that even without the directory comparison, the directory 3 is created and used. So maybe the directory comparison is not needed after all.

argh.. you can't edit a comment?

I take it back. The directory comparison is still needed.

In the case where:
a) Directory 0 does not exist
b) Directory 1 exists and is full
c) Directory 2 exists and is not full

Iterating through the loop
Loop 1: (full)
$directory->directory=1 $file->directory=0
Loop 2: success because /2 is not full
$directory->directory=2 $file->directory=1

and so the file will be uploaded to directory 1 which violates the maximum files per directory setting. So keeping the two variables in sync is quite important.

With the directory comparison, what will happen is that the term ($directory->directory > $file->directory) is TRUE in Loop 1, so the file will be uploaded to directory 0.

The code is quite subtle.. and that's troubling.. so many weird cases to consider. I especially don't like the assumption that the list of directories has to be ordered in ascending order. Someone might just come by and change that query to be non-ascending, and it will cause problems to the while loop.

cowdinosaur, when you come accross stuff that's got wierd assumptions, add comments to describe them. you'll save others the trouble and if someone does later change the query it'll make the debugging easier.

I have commented the loop. Hopefully it has more clarity :) The patch incorporates vhmauery's patch too.

I took a look at the patch and even with the additional comemnts, I had to come re-read this thread before it made sense to me. I've re-written the comment to try to clarify it. Please take a look and let me know if it's understandable.

This has been open a while and the only recent changes have been in the comments.

I'm going to commit the latest patch to both HEAD and 4.6, with appologies to ccourtne.

Hi,

I'm using Filemanager CVS with Acidfree 4.7

I've recently noticed the "0" directory only has one 5MB video,
and for the "1" directory it has 600 files (47MB).

The main problem is that each directory should have 200 files,
which I set it for in the my filemanager settings:

Maximum files per directory: 200
Working size limit: 10MB
Maximum working age: 1440
Maximum size limit: 200MB
General - Max size (Mb): -1
Acidfree - Max size (Mb): -1

I thought maybe cron needed to run, but that wasnt the problem.
Filemanager is installed properly according to the Acidfree self-test checklist

1. Filemanager module enabled
2. Latest version of Filemanager module installed
3. filemanager-private.patch applied
4. Filemanager private path set and permissions OK
5. Filemanager public path set and permissions OK
6. Filemanager database table created
7. Acidfree class definition files found for album, photo, video
8. Drupal 'Temporary directory' path set and permissions OK
9. Acidfree database tables created
10. gd image toolkit installed and properly configured

Thanks for any help

// $Id: filemanager.module,v 1.15 2006/04/20 07:20:07 chx Exp $

I've compared the patches in this thread to the filemanager.module I'm using,
and its exactly the same it seems

/**
 * this must be called while the lock is held
 * @param $file
 */
function _filemanager_find_directory(&$file) {
    // Find a directory that is not already full and does not contain our files
    $file->directory = 0;
    $directories = db_query("SELECT directory, count(1) filecount FROM {file} WHERE private = '%s' GROUP BY directory ORDER BY directory ASC", $file->private);

    // this while loop requires the $directories array to be ordered in ascending order
    while ($directory = db_fetch_object($directories)) {
      // The idea here is to find a directory where the filename doesn't exist
      // and we haven't hit the maximum file limit. The directories are named
      // numerically and the first part of the test makes sure that the they're
      // filled in sequenceially. $file->directory is incremented by 1 each time
      // but $directory->directory comes from the database. If
      // $directory->directory > $file->directory, then $file->directory doesn't
      // exist and would be a safe place to save the file.
      if ($directory->directory > $file->directory || $directory->filecount < variable_get('filemanager_max_file_count', '2000')) {
        // If the directory is ok now lets make sure we don't already have this
        // filename in the directory (checking both working and active).
        if (!file_exists(filemanager_create_path($file, FALSE)) && !file_exists(filemanager_create_path($file, TRUE))) {
          break;
        }
      }
      $file->directory++;
    }
    return $file;
}

Thanks

Should '2000' be '200' or changed to the value I want it to be??

if ($directory->directory > $file->directory || $directory->filecount < variable_get('filemanager_max_file_count', '2000')) {
        // If the directory is ok now lets make sure we don't already have this
        // filename in the directory (checking both working and active).

Thanks

hmmm.... that still doesn't explain why the "0" directory only has 1 file.

the maximum file size limit is working.

hmm

could it be my version of Acidfree?

/* $Id: acidfree.module,v 1.62.2.31 2006/07/12 19:17:20 vhmauery Exp $ */

Hi andre75,

Thanks for the suggestions

Increase the maximum working age or simply forget about it. 

I know its not a big issue, since the max file size limit is working, but I'm just curious why its not working as stated.

Is it working properly for you?
If so, which version of Drupal are you using?

Thanks

4.6 eh?
damn that explains it.

thanks