Skip directly to content

Tight coupling (no, not like that)

on August 11th, 2010 at 2:00:44 PM

Drupal has a function called file_save_upload()

The docs say that it "Saves a file upload to a new location." Let's see what it really does...

45698_OVERSTUFFED_SANDWICH02.JPG

 

Reads in from global arrays

// Make sure there's an upload to process.
  if (empty($_FILES['files']['name'][$source])) {
    return NULL;
  }

Accesses global variables

global $user;

Validates input

if (isset($validators['file_validate_extensions'][0])) {
      // Build the list of non-munged extensions if the caller provided them.
      $extensions = $validators['file_validate_extensions'][0];
    }

Saves to the DB

if ($file = file_save($file)) {

Prints messages to the screen

if ($file->destination === FALSE) {
    drupal_set_message(t('The file %source could not be uploaded because a file by that name already exists in the destination %directory.', array('%source' => $source, '%directory' => $destination)), 'error');
    return FALSE;
  }

Writes to the disk

if (!move_uploaded_file($_FILES['files']['tmp_name'][$source], $file->uri)) {

Sets permissions on the disk

// Set the permissions on the new file.
  drupal_chmod($file->uri);

Could we shove any more stuff in here?

Some ideas:

  • Let's have it also send emails with the files as attachments
  • Image resizing
  • DropBox
  • Post to 4chan!
  • Archive all files in .lzh format
  • Submit mp3s to last.fm for scrobbling

Any more ideas? I want to make this function the bestest function ever. I want to make it bigger and better than Drupal.

This is obviously (mostly) a joke because the time to change this is probably long past.  Here's one issue though that takes out part of it (potentially): http://drupal.org/node/685818

Comments

Anonymous's picture

Is this working?

Anonymous's picture

Je me demande aussi si ça marche, en particulier dans certains secteurs spécifiques, comme un casino?

Anonymous's picture

Bonne question ? impressionnant la photo du sandwich sinon :) ca me rapelle des parties de folies de casino en ligne ce mois ci ...

Anonymous's picture

This is my first time i visit here. I found so many entertaining stuff in your blog, especially its discussion.
<a href="http://www.emanprinting.com/sticker-printing/Volcom-Stickers.php">Volcom Stickers</a>

Anonymous's picture

J'ai trouvé votre site sur google et j'ai lu plusieurs articles, ce serait intéressant de voir comment ça marche dans un casino.

Anonymous's picture

it saves all u need in one place

Darrel O&#039;Pry's picture

I think you're making it seem worse than it really is.

The reads in from Global Arrays and Accesses Global variables is all part of validation.

Saves to DB and Writes to Disk, including setting permissions, are a required grouping this part needs to be transactional in Drupal's File API. ie) One can't happen without the other. Without some sort of external transactional API or a full move to OO and exception based error handling it's difficult to further re-factor something like this.

Sounds like you're looking for some sort of File Factory...

static class BuildFile() : IFileFactory {
static public FileObject FromURI(String URI);
static public FileObject FromID(String ID);
static public FileObject FromPath(String Path);
}

I think this type of model would allow you to clean up some of the file save... But a lot of those parts are hard to clean out. With the drupal file API you're not going to easily be able to decouple the filesystem activity and db writes as they need to be transactional...

Jacob Singh's picture

Yeah, that's exactly what we need :)

Rob26's picture

Stumbled across this while searching for something completely different, but I'm glad I did, very interesting perspective.

Rob @ www.purelyhydroponic.com

Post new comment

Addthis