Merge branch 'foolproof_file_redirection_branch' into 'nightly'

Foolproof file redirection

This solves an issue when our internal /attachment/{file_id} links are shortened with an remote shorteners (which caused the /attachment/{file_id} links to be saved to the File table and a thumbnail of a thumbnail being generated)

See merge request !98

classes/File.php

@@ -106,6 +106,19 @@ class File extends Managed_DataObject
             // We don't have the file's URL since before, so let's continue.
         }
 
+        // if the given url is an local attachment url and the id already exists, don't
+        // save a new file record. This should never happen, but let's make it foolproof
+        $attachment_path = common_path('attachment/');
+        if(strpos($given_url,$attachment_path) == 0) {
+            $possible_file_id = substr($given_url,strlen($attachment_path));
+            if(is_numeric($possible_file_id)) {
+                $file = File::getKV('id',$possible_file_id);
+                if($file instanceof File) {
+                    return $file;
+                }
+            }
+        }
+
         $file = new File;
         $file->url = $given_url;
         if (!empty($redir_data['protected'])) $file->protected = $redir_data['protected'];

classes/File_redirection.php

@@ -172,43 +172,70 @@ class File_redirection extends Managed_DataObject
 
         try {
             $r = File_redirection::getByUrl($in_url);
-            if($r instanceof File_redirection) {
+
-				try {
             $f = File::getKV('id',$r->file_id);
+
+            if($file instanceof File) {
                 $r->file = $f;
                 $r->redir_url = $f->url;            
-				} catch (NoResultException $e) {
+            } else {
                 // Invalid entry, delete and run again
-					common_log(LOG_ERR, "Could not find File with id=".$r->file_id." referenced in File_redirection, deleting File redirection entry and creating new File and File_redirection entries.");					
+                common_log(LOG_ERR, "Could not find File with id=".$r->file_id." referenced in File_redirection, deleting File redirection entry and and trying again...");                 
                 $r->delete();
                 return self::where($in_url);            
             }
+            
+            // File_redirecion and File record found, return both
             return $r;
-            }
+            
         } catch (NoResultException $e) {
+            // File_redirecion record not found, but this might be a direct link to a file
             try {
                 $f = File::getByUrl($in_url);
                 $redir->file_id = $f->id;
                 $redir->file = $f;
                 return $redir;
             } catch (NoResultException $e) {                    
-                // Oh well, let's keep going
+                // nope, this was not a direct link to a file either, let's keep going
             }
         }
 
         if ($discover) {    
+            // try to follow redirects and get the final url        
             $redir_info = File_redirection::lookupWhere($in_url);
             if(is_string($redir_info)) {
                 $redir_info = array('url' => $redir_info);
             }
             
-			// Save the file if we don't have it already
+            // the last url in the redirection chain can actually be a redirect!
-			$redir->file = File::saveNew($redir_info,$redir_info['url']);
+            // this is the case with local /attachment/{file_id} links
+            // in that case we have the file id already
+            try {
+                $r = File_redirection::getByUrl($redir_info['url']);
                 
-			// If this is a redirection, save it
+                $f = File::getKV('id',$r->file_id);
-			// (if it hasn't been saved yet by some other process while we we
+                
-			// were running lookupWhere())			
+                if($f instanceof File) {
-            if($redir_info['url'] != $in_url) {
+                    $redir->file = $f;
+                    $redir->redir_url = $f->url;                
+                } else {
+                    // Invalid entry in File_redirection, delete and run again
+                    common_log(LOG_ERR, "Could not find File with id=".$r->file_id." referenced in File_redirection, deleting File_redirection entry and trying again...");                 
+                    $r->delete();
+                    return self::where($in_url);                
+                }
+            } catch (NoResultException $e) {
+                // save the file now when we know that we don't have it in File_redirection
+                try {
+                    $redir->file = File::saveNew($redir_info,$redir_info['url']);
+                } catch (ServerException $e) {
+                    common_log(LOG_ERR, $e);
+                }   
+            }
+             
+            // If this is a redirection and we have a file to redirect to, save it
+            // (if it doesn't exist in File_redirection already)            
+            if($redir->file instanceof File && $redir_info['url'] != $in_url) {
                 try {
                     $file_redir = File_redirection::getByUrl($in_url);
                 } catch (NoResultException $e) {