I'm using multiple upload.

If I choose to delete the original video after conversion, in _flashvideo_perform_postop() function the following code is executed

Code part1

 1        if (flashvideo_variable_get($node_type, 'delete', 0) && get_file_ext($oldfile->filepath) != 'flv') {
 2          // Delete the original file from the file system.
 3          file_delete($oldfile->filepath);

 4          // Delete the original file from the files table.
 5          db_query("DELETE FROM {files} WHERE fid = %d", $oldfile->fid);

 6          // Delete the original file from the appropriate table.
 7          if (module_exists('filefield') && flashvideo_variable_get(NULL, 'flashvideo_filefield', 0)) { // Using FileField
 8            // For single-upload file fields
 9            if (db_column_exists($cck_table, $cck_original_video_field .'_fid')) {
10              db_query("UPDATE {". $cck_table ."} SET ". $cck_original_video_field ."_fid = NULL, ". $cck_original_video_field ."_list = NULL WHERE nid = %d", $oldfile->nid);
11            }
12            else { // For multi-upload file fields
13              db_query("DELETE FROM {content_". $cck_original_video_field ."} WHERE ".$cck_original_video_field ."_fid = %d", $oldfile->fid);
14            }
15          }
16          else { // They're using the core Upload module.
17            db_query("DELETE FROM {upload} WHERE fid = %d", $oldfile->fid);
18          }

19          // Delete the file from the flashvideo table.
20          db_query("DELETE FROM {flashvideo} WHERE fid=%d", $oldfile->fid);

21          // Allow other modules do do some delete operations.
22          module_invoke_all('flashvideo_delete_file', $oldfile, $node_type);
23        }

The line n°13 delete each entry from the cck original field.

When I delete the node, the hook flashvideo_nodeapi is executed.
Code part 2

function flashvideo_nodeapi(&$node, $op, $teaser) {

 1  // Only if the node type is enabled.
 2  if (flashvideo_variable_get($node->type, 'enable', 0)) {
 3    switch ($op) {
......
......
 4      case 'delete':
 5      case 'delete revision':

 6        db_query("DELETE FROM {flashvideo} WHERE nid = %d", $node->nid);

 7        if (module_exists('filefield') && flashvideo_variable_get(NULL, 'flashvideo_filefield', 0)) {
 8          // Names of the different video fields? Let's find out.
 9          $cck_original_video_field = flashvideo_variable_get($node->type, 'cck_original_video_field', '');
10          $cck_finished_video_field = flashvideo_variable_get($node->type, 'cck_finished_video_field', '');
11          $cck_finished_thumbnail_field = flashvideo_variable_get($node->type, 'cck_finished_thumbnail_field', '');
12          $cck_table = 'content_type_'. $node->type;

13          // Grab all of the files associated with the node.
14          $video_files = array();
15          foreach ($node->$cck_original_video_field as $file) {
16            $video_files[] = $file;
17          }

$node->$cck_original_video_field on line 15 IS NULL because we deleted each record after a successfull conversion.

My fix is to add line 14-18

14          if(!empty($node->$cck_original_video_field)) {
15            foreach ($node->$cck_original_video_field as $file) {
16              $video_files[] = $file;
17            }
18          }

that is better to write line 14 in this way:

14          if(!flashvideo_variable_get($node->type, 'delete', 0)) {// If we wish to delete the original video 

because if you uploaded an flv (Line 1 of Code part 1 of this issue) the $node->$cck_original_video_field is not empty because the original video is the converted video at the same time

CommentFileSizeAuthor
#5 flashvideo.patch1.34 KBtagliavinid

Comments

attheshow’s picture

Status: Active » Postponed (maintainer needs more info)

I just tried setting my content type to delete original videos after conversion and didn't receive any errors when I deleted my multi-video test node.

tagliavinid’s picture

Priority: Minor » Normal

You're right, sorry for the second time.

This error appears because I fixed another (I suppose) bug.

Now I try to explain everything.

After you save a node and everything is ok, the following functions is called.

1 function _flashvideo_perform_postop($oldfile, $newfile, $node_type, $create_thumbnail = FALSE) {
2
3   $timestamp = time();
4
5   if ((_flashvideo_get_filetype($oldfile->filepath) == 'flv') && !$create_thumbnail) {
6     db_query("UPDATE {files} SET filepath='%s', filename='%s', filesize=%d WHERE fid=%d", $newfile->filepath, $newfile->filename, $newfile->filesize, $newfile->fid);
7
8     // Insert this video into the flashvideo table if it is not already in there.
9     if (!db_result(db_query("SELECT COUNT(*) FROM {flashvideo} WHERE oid=%d AND fid=%d AND nid=%d", $newfile->fid, $newfile->fid, $newfile->nid))) {
10       // Insert this video into the flashvideo table.
11       db_query("INSERT INTO {flashvideo} (fid, nid, oid, status, video_index, width, height, flags) VALUES (%d, %d, %d, %d, %d, %d, %d, 0)", $newfile->fid, $newfile->nid, $oldfile->fid, FLASHVIDEO_STATUS_CONVERTED, $params->video_index, $params->width, $params->height);
12       db_query("INSERT INTO {ffmpeg_data} (fid, created, input_file, output_file, status) VALUES (%d, %d, '%s', '%s', %d)", $oldfile->fid, $timestamp, $oldfile->filepath, $newfile->filepath, 1);
13     }
14   }
15   else {
16     // Create a temporary file for the new file.
17     db_query("INSERT INTO {files} (uid, filename, filepath, filemime, filesize, status, timestamp) VALUES (%d, '%s', '%s', '%s', %d, %d, %d)", $newfile->uid, $newfile->filename, $newfile->filepath, $newfile->filemime, $newfile->filesize, FILE_STATUS_TEMPORARY, time());
18
19     // Get the new File ID
20     $newfile->fid = db_last_insert_id('files', 'fid');
21
22     // Look to make sure that the File ID doesn't already exists in the files table.
23     $found = db_result(db_query("SELECT COUNT(*) FROM {files} WHERE filepath='%s' AND status=%d", $newfile->filepath, FILE_STATUS_PERMANENT));
24
25     // If the file isn't already in the files table...
26     if ($found == 0) {
27
28       // Grab the width, height, and video_index from the original video to copy over to the new video, then insert the new video in the flashvideo table.
29       $params = db_fetch_object(db_query("SELECT video_index, width, height FROM {flashvideo} WHERE fid = %d", $oldfile->fid));
30
31       // Create the appropriate database entry for the newly converted video file.
32       if (module_exists('filefield') && flashvideo_variable_get(NULL, 'flashvideo_filefield', 0)) { // Using FileField
33         // Names of the different video fields? Let's find out.
34         $cck_original_video_field = flashvideo_variable_get($node_type, 'cck_original_video_field', '');
35         $cck_finished_video_field = flashvideo_variable_get($node_type, 'cck_finished_video_field', '');
36         $cck_finished_thumbnail_field = flashvideo_variable_get($node_type, 'cck_finished_thumbnail_field', '');
37         $cck_table = 'content_type_'. $node_type;
38         if ($create_thumbnail) { // This is for the finished thumbnail.
39           // For single-upload file fields
40           if (db_column_exists($cck_table, $cck_original_video_field .'_fid')) {
41             db_query("UPDATE {". $cck_table ."} SET ". $cck_finished_thumbnail_field ."_fid = %d, ". $cck_finished_thumbnail_field ."_list = 0 WHERE nid = %d", $newfile->fid, $newfile->nid);
42           }
43           else { // For multi-upload file fields
44             // Get rid of the initial NULL entry.
45             db_query("DELETE FROM {content_". $cck_finished_thumbnail_field ."} WHERE nid = %d AND ". $cck_finished_thumbnail_field ."_fid IS NULL", $newfile->nid);
46             db_query("INSERT INTO {content_". $cck_finished_thumbnail_field ."} (vid, nid, delta, ". $cck_finished_thumbnail_field ."_fid, ". $cck_finished_thumbnail_field ."_list) VALUES (%d, %d, %d, %d, 0)", $newfile->nid, $newfile->nid, $params->video_index, $newfile->fid);
47           }
48         }
49         else { // This is for the finished video.
50           // For single-upload file fields
51           if (db_column_exists($cck_table, $cck_original_video_field .'_fid')) {
52             db_query("UPDATE {". $cck_table ."} SET ". $cck_finished_video_field ."_fid = %d, ". $cck_finished_video_field ."_list = 0 WHERE nid = %d", $newfile->fid, $newfile->nid);
53           }
54           else { // For multi-upload file fields
55             // Get rid of the initial NULL entry.
56             db_query("DELETE FROM {content_". $cck_finished_video_field ."} WHERE nid = %d AND ". $cck_finished_video_field ."_fid IS NULL", $newfile->nid);
57             db_query("INSERT INTO {content_". $cck_finished_video_field ."} (vid, nid, delta, ". $cck_finished_video_field ."_fid, ". $cck_finished_video_field ."_list) VALUES (%d, %d, %d, %d, 0)", $newfile->nid, $newfile->nid, $params->video_index, $newfile->fid);
58           }
59         }
60
61       }
62       else { // They're using the core Upload module.
63         // Figure out the vid for this node.
64         $sql = "SELECT vid FROM {node} WHERE nid = %d";
65         $newfile->vid = db_result(db_query($sql, $newfile->nid));
66         // Add the video into the upload table.
67         db_query("INSERT INTO {upload} (fid, nid, vid, description, list) VALUES (%d, %d, %d, '%s', 0)", $newfile->fid, $newfile->nid, $newfile->vid, $newfile->description);
68       }
69
70       // Insert this video into the flashvideo table.
71       db_query("INSERT INTO {flashvideo} (fid, nid, oid, status, video_index, width, height, flags) VALUES (%d, %d, %d, %d, %d, %d, %d, 0)", $newfile->fid, $newfile->nid, $oldfile->fid, FLASHVIDEO_STATUS_CONVERTED, $params->video_index, $params->width, $params->height);
72
73       // If we are not creating a thumbnail...
74       if (!$create_thumbnail) {
75         // If they wish to delete the original video...
76         if (flashvideo_variable_get($node_type, 'delete', 0) && get_file_ext($oldfile->filepath) != 'flv') {
77           // Delete the original file from the file system.
78           file_delete($oldfile->filepath);
79
80           // Delete the original file from the files table.
81           db_query("DELETE FROM {files} WHERE fid = %d", $oldfile->fid);
82
83           // Delete the original file from the appropriate table.
84           if (module_exists('filefield') && flashvideo_variable_get(NULL, 'flashvideo_filefield', 0)) { // Using FileField
85             // For single-upload file fields
86             if (db_column_exists($cck_table, $cck_original_video_field .'_fid')) {
87               db_query("UPDATE {". $cck_table ."} SET ". $cck_original_video_field ."_fid = NULL, ". $cck_original_video_field ."_list = NULL WHERE nid = %d", $oldfile->nid);
88             }
89             else { // For multi-upload file fields
90               db_query("DELETE FROM {content_". $cck_original_video_field ."} WHERE nid = %d", $node->nid);
91             }
92           }
93           else { // They're using the core Upload module.
94             db_query("DELETE FROM {upload} WHERE fid = %d", $oldfile->fid);
95           }
96
97           // Delete the file from the flashvideo table.
98           db_query("DELETE FROM {flashvideo} WHERE fid=%d", $oldfile->fid);
99
100           // Allow other modules do do some delete operations.
101           module_invoke_all('flashvideo_delete_file', $oldfile, $node_type);
102         } 

Take a look at line 76

76         if (flashvideo_variable_get($node_type, 'delete', 0) && get_file_ext($oldfile->filepath) != 'flv') {

If this line returns true we go to line 90 (for multi-upload file fields)

......
84           if (module_exists('filefield') && flashvideo_variable_get(NULL, 'flashvideo_filefield', 0)) { // Using FileField
85             // For single-upload file fields
86             if (db_column_exists($cck_table, $cck_original_video_field .'_fid')) {
87               db_query("UPDATE {". $cck_table ."} SET ". $cck_original_video_field ."_fid = NULL, ". $cck_original_video_field ."_list = NULL WHERE nid = %d", $oldfile->nid);
88             }
89             else { // For multi-upload file fields
90               db_query("DELETE FROM {content_". $cck_original_video_field ."} WHERE nid = %d", $node->nid);
91             }
....

on line 90 $node->nid IS NULL because we pass as argument to this functions (_flashvideo_perform_postop) only $node->type (line 1, function declarations) and $node object doesnt' exists.

But even if we pass $node instead of $node->type, this make no sense, because in this way we delete all original video record in {content_". $cck_original_video_field ."} and not only that one we finished to process (if I have for example 3 videos in my node with multiple field upload).

For this reason when conversion is finished {content_". $cck_original_video_field ."} table is not empty, but contains my original videos record.

When I delete the node there are filefield hook that delete records inside the above table (e.g. filefield_field_delete hook).

When I found this bug I fixed line 90 in this way

90     db_query("DELETE FROM {content_". $cck_original_video_field ."} WHERE ".$cck_original_video_field ."_fid = %d", $oldfile->fid);

This fix deleted orginal video record after a successful conversion.

And after all this we go back to the origin of my issue.

function flashvideo_nodeapi(&$node, $op, $teaser) {

1  // Only if the node type is enabled.
2  if (flashvideo_variable_get($node->type, 'enable', 0)) {
3    switch ($op) {
......
......
4      case 'delete':
5      case 'delete revision':

6        db_query("DELETE FROM {flashvideo} WHERE nid = %d", $node->nid);

7        if (module_exists('filefield') && flashvideo_variable_get(NULL, 'flashvideo_filefield', 0)) {
8          // Names of the different video fields? Let's find out.
9          $cck_original_video_field = flashvideo_variable_get($node->type, 'cck_original_video_field', '');
10          $cck_finished_video_field = flashvideo_variable_get($node->type, 'cck_finished_video_field', '');
11          $cck_finished_thumbnail_field = flashvideo_variable_get($node->type, 'cck_finished_thumbnail_field', '');
12          $cck_table = 'content_type_'. $node->type;

13          // Grab all of the files associated with the node.
14          $video_files = array();
15          foreach ($node->$cck_original_video_field as $file) {
16            $video_files[] = $file;
17          }

With your original code when I delete a node on line 15 $node->$cck_original_video_field I have an array with 3 NULL entry, one for each video (0=> NULL, 1=>NULL, 2=>NULL).

and going on the code doesn't produce error.
For example line 6 and line 8 may return simply false
In my case {ffmpeg_data} table grow up even if I delete node.

1           if (count($video_files)) {
2             $counties = count($video_files);
3             // Iterate through each file and delete it.
4             foreach ($video_files as $file) {
5               $file = (object)$file;
6               db_query("DELETE FROM {ffmpeg_data} WHERE fid = %d", $file->fid);
7               // Delete that file from the files and CCK tables as well as from the file system.
8               db_query("DELETE FROM {files} WHERE fid = %d", $file->fid);
9               // For single-upload file fields
10               if (db_column_exists($cck_table, $cck_original_video_field .'_fid')) {
11                 db_query("UPDATE {". $cck_table ."} SET ". $cck_original_video_field ."_fid = NULL, ". $cck_original_video_field ."_list = NULL, ". $cck_finished_video_field ."_fid = NULL, ". $cck_finished_video_field ."_list = NULL, ". $cck_finished_thumbnail_field ."_fid = NULL, ". $cck_finished_thumbnail_field ."_list = NULL WHERE nid = %d", $node->nid);
12               }
13               else { // For multi-upload file fields
14                 db_query("DELETE FROM {content_". $cck_original_video_field ."} WHERE nid = %d", $node->nid);
15                 db_query("DELETE FROM {content_". $cck_finished_video_field ."} WHERE nid = %d", $node->nid);
16                 db_query("DELETE FROM {content_". $cck_finished_thumbnail_field ."} WHERE nid = %d", $node->nid);
17               }
18               file_delete($file->filepath);
19               module_invoke_all('flashvideo_delete_file', $file, $node->type);
20             }
21           } 

With my fix for line 90, the problem on this code (line 15 flashvideo_nodeapi in this issue) is that $node->$cck_original_video_field is not an array, but is a NULL value.

13          // Grab all of the files associated with the node.
14          $video_files = array();
15          foreach ($node->$cck_original_video_field as $file) {
16            $video_files[] = $file;
17          }

So finally we are at the initial reason for which I wrote this fix.

14        if(!empty($node->$cck_original_video_field)) {
15          foreach ($node->$cck_original_video_field as $file) {

So, if you think my fix for line 90 (_flashvideo_perform_postop) is correct, you need a fix for line 15 of flashvideo_nodeapi too.

Hope this helps

tagliavinid’s picture

Status: Postponed (maintainer needs more info) » Active
attheshow’s picture

Version: 6.x-1.5-rc1 » 6.x-1.5-rc2
Status: Active » Postponed (maintainer needs more info)

Any chance you can post a patch instead of such long strings of code? That would cover your suggested code changes in much more efficient manner. http://drupal.org/patch/create

tagliavinid’s picture

StatusFileSize
new1.34 KB

Sorry, I'm new to drupal and his community, so I need more time to understand how to help people in the best way.

In this issue I reported all this code to help you to understand what I mean even if I changed only 3 lines.

Let's go on.

here is the patch.

The explanation why flashvideo needs this patch (in my opinion) is in this issue.

tagliavinid’s picture

Status: Postponed (maintainer needs more info) » Needs review
attheshow’s picture

Assigned: Unassigned » attheshow
Status: Needs review » Fixed

Just fixed these error messages in last commit. Made sure that the item was an array before running foreach().

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.