Moodle
  1. Moodle
  2. MDL-35376

Shortcut/alias not working for Private file repo

    Details

    • Testing Instructions:
      Hide
      1. Login as teacher
      2. Go to "Manage private files"
      3. Upload text file called test.txt and click save changes
      4. Go to a course and add a file resource using the uploaded text file as a shortcut/alias
      5. Login as student (in another browser for convenience) and try to download/view file from course
      6. Make sure this is a correct file
      7. Work again as teacher
      8. Edit text file and then go to "Manage private files" again
      9. Upload edited text file and choose "Overwrite" when the prompt appears after file upload
      10. Then click save changes
      11. Verify error is not thrown
      12. As a student try to download/view file from course
      13. Make sure this is a correct (new) file
      Show
      Login as teacher Go to "Manage private files" Upload text file called test.txt and click save changes Go to a course and add a file resource using the uploaded text file as a shortcut/alias Login as student (in another browser for convenience) and try to download/view file from course Make sure this is a correct file Work again as teacher Edit text file and then go to "Manage private files" again Upload edited text file and choose "Overwrite" when the prompt appears after file upload Then click save changes Verify error is not thrown As a student try to download/view file from course Make sure this is a correct (new) file
    • Affected Branches:
      MOODLE_23_STABLE
    • Fixed Branches:
      MOODLE_23_STABLE
    • Pull from Repository:
    • Pull Master Branch:
    • Rank:
      44052

      Description

      Steps to reproduce problem:

      1. Go to QA site: http://qa.moodle.net/
      2. Login as teacher
      3. Go to "Manage private files"
      4. Upload text file called test.txt and click save changes
      5. Go to a course and add a file resource using the uploaded text file as a shortcut/alias
      6. Edit text file and then go to "Manage private files" again
      7. Upload edited text file and choose "Overwrite" when the prompt appears after file upload
      8. Then click save changes
      9. Following error message appears:
      Coding error detected, it must be fixed by a programmer: PHP catchable fatal error
      
      Debug info: Argument 1 passed to file_storage::update_references_to_storedfile() must be an instance of stored_file, null given, called in /html/lib/filelib.php on line 835 and defined
      Error code: codingerror
      Stack trace:
      line 397 of /lib/setuplib.php: coding_exception thrown
      line 1823 of /lib/filestorage/file_storage.php: call to default_error_handler()
      line 835 of /lib/filelib.php: call to file_storage->update_references_to_storedfile()
      line 291 of /lib/filelib.php: call to file_save_draft_area_files()
      line 64 of /user/files.php: call to file_postupdate_standard_filemanager()
      Output buffer: <br /> <b>Notice</b>: Undefined variable: this in <b>/html/lib/filelib.php</b> on line <b>835</b><br />
      
      1. Also, another error message that appears is that in the function update_references_to_storedfile in lib/filestorage/file_storage is missing the reference to a global $DB object.

        Issue Links

          Activity

          Rex Lorenzo created issue -
          Rex Lorenzo made changes -
          Field Original Value New Value
          Link This issue is a regression caused by MDL-34290 [ MDL-34290 ]
          Rex Lorenzo made changes -
          Summary Unable to overwrite files in Private file repo Shortcut/alias not working for Private file repo
          Description Steps to reproduce problem:

          # Go to QA site: http://qa.moodle.net/
          # Login as teacher
          # Go to "Manage private files"
          # Upload text file called test.txt and click save changes
          # Edit text file and then go to "Manage private files" again
          # Upload edited text file and choose "Overwrite" when the prompt appears after file upload
          # Then click save changes
          # Following error message appears:

          {code}
          Coding error detected, it must be fixed by a programmer: PHP catchable fatal error

          Debug info: Argument 1 passed to file_storage::update_references_to_storedfile() must be an instance of stored_file, null given, called in /html/lib/filelib.php on line 835 and defined
          Error code: codingerror
          Stack trace:
          line 397 of /lib/setuplib.php: coding_exception thrown
          line 1823 of /lib/filestorage/file_storage.php: call to default_error_handler()
          line 835 of /lib/filelib.php: call to file_storage->update_references_to_storedfile()
          line 291 of /lib/filelib.php: call to file_save_draft_area_files()
          line 64 of /user/files.php: call to file_postupdate_standard_filemanager()
          Output buffer: <br /> <b>Notice</b>: Undefined variable: this in <b>/html/lib/filelib.php</b> on line <b>835</b><br />
          {code}
          Steps to reproduce problem:

          # Go to QA site: http://qa.moodle.net/
          # Login as teacher
          # Go to "Manage private files"
          # Upload text file called test.txt and click save changes
          # Go to a course and add a file resource using the uploaded text file as a shortcut/alias
          # Edit text file and then go to "Manage private files" again
          # Upload edited text file and choose "Overwrite" when the prompt appears after file upload
          # Then click save changes
          # Following error message appears:

          {code}
          Coding error detected, it must be fixed by a programmer: PHP catchable fatal error

          Debug info: Argument 1 passed to file_storage::update_references_to_storedfile() must be an instance of stored_file, null given, called in /html/lib/filelib.php on line 835 and defined
          Error code: codingerror
          Stack trace:
          line 397 of /lib/setuplib.php: coding_exception thrown
          line 1823 of /lib/filestorage/file_storage.php: call to default_error_handler()
          line 835 of /lib/filelib.php: call to file_storage->update_references_to_storedfile()
          line 291 of /lib/filelib.php: call to file_save_draft_area_files()
          line 64 of /user/files.php: call to file_postupdate_standard_filemanager()
          Output buffer: <br /> <b>Notice</b>: Undefined variable: this in <b>/html/lib/filelib.php</b> on line <b>835</b><br />
          {code}
          # Also, another error message that appears is that in the function update_references_to_storedfile in lib/filestorage/file_storage is missing the reference to a global $DB object.
          Hide
          Rex Lorenzo added a comment -

          Using the latest Moodle 2.3.2 version I am trying to get the shortcut/alias feature to work for private files and it isn't working for me.

          For the bug messages that I posted I tried to fix them by changing:

          $fs->update_references_to_storedfile($this);
          

          To:

          $fs->update_references_to_storedfile($newfile);
          

          Then that resulted in another error message about update_references_to_storedfile not having a reference to a global $DB object. I added that to the function and now I get no more errors.

          However, a weird thing is that when I shortcut/alias the text file as an embedded resource for a course, my file changes weren't showing up when I displayed the resource. However, when I switched the file resource to "force download", I got the latest copy of the file. Then I switched it back to embedded and now I see the latest version. Maybe it was a caching issue?

          Show
          Rex Lorenzo added a comment - Using the latest Moodle 2.3.2 version I am trying to get the shortcut/alias feature to work for private files and it isn't working for me. For the bug messages that I posted I tried to fix them by changing: $fs->update_references_to_storedfile($ this ); To: $fs->update_references_to_storedfile($newfile); Then that resulted in another error message about update_references_to_storedfile not having a reference to a global $DB object. I added that to the function and now I get no more errors. However, a weird thing is that when I shortcut/alias the text file as an embedded resource for a course, my file changes weren't showing up when I displayed the resource. However, when I switched the file resource to "force download", I got the latest copy of the file. Then I switched it back to embedded and now I see the latest version. Maybe it was a caching issue?
          Rex Lorenzo made changes -
          Hide
          Rex Lorenzo added a comment -

          I changed

          $fs->update_references_to_storedfile($this);
          

          to

          $fs->update_references_to_storedfile($oldfile);
          

          Since it seems to be more correct.

          Show
          Rex Lorenzo added a comment - I changed $fs->update_references_to_storedfile($ this ); to $fs->update_references_to_storedfile($oldfile); Since it seems to be more correct.
          Rex Lorenzo made changes -
          Status Open [ 1 ] Peer review in progress [ 10013 ]
          Peer reviewer marina
          Rex Lorenzo made changes -
          Link This issue will help resolve MDL-35361 [ MDL-35361 ]
          Hide
          Marina Glancy added a comment -

          looks good for me, thanks for fixing this

          Show
          Marina Glancy added a comment - looks good for me, thanks for fixing this
          Marina Glancy made changes -
          Status Peer review in progress [ 10013 ] Development in progress [ 3 ]
          Marina Glancy made changes -
          Status Development in progress [ 3 ] Waiting for integration review [ 10010 ]
          Marina Glancy made changes -
          Hide
          kiswap added a comment -

          $newfile or $oldfile is better? what's the difference in results?

          Show
          kiswap added a comment - $newfile or $oldfile is better? what's the difference in results?
          Hide
          Rex Lorenzo added a comment -

          I thought $oldfile was better because in the code $oldfile is being updated with the updated values in $newfile. So it seems like $oldfile is the storedfile that is already on the system that needs to be updated.

          Also, in our further testing for shortcuts/aliasing Dropbox doesn't seem to be updating its file references even with this patch. Going to open another bug ticket.

          Show
          Rex Lorenzo added a comment - I thought $oldfile was better because in the code $oldfile is being updated with the updated values in $newfile. So it seems like $oldfile is the storedfile that is already on the system that needs to be updated. Also, in our further testing for shortcuts/aliasing Dropbox doesn't seem to be updating its file references even with this patch. Going to open another bug ticket.
          Hide
          Rex Lorenzo added a comment -

          Okay, so Dropbox does work, but the lifetime before it syncs is set to be 1 day! Is there a config option to change how often files are sync'd? Why so long?

          Show
          Rex Lorenzo added a comment - Okay, so Dropbox does work, but the lifetime before it syncs is set to be 1 day! Is there a config option to change how often files are sync'd? Why so long?
          Hide
          Rex Lorenzo added a comment -

          More findings. For the Dropbox repository there is a new config setting called "Cache limit" and it is set to 1 MiB by default. It seems that if a file is less than this cache limit, then the file is just served from the local cache, rather than getting an updated copy from Dropbox. Is that correct? The online documentation doesn't mention anything about this behavior, but my testing does seem to reflect that.

          For example, if I have a text file over 1 MiB, then the changes are displayed instantly. But for a small one line text file the changes don't appear unless I invalidate the lastsync timestamp.

          I would be happy to update the Moodle documentation, but I have no clue what is suppose to happen.

          Show
          Rex Lorenzo added a comment - More findings. For the Dropbox repository there is a new config setting called "Cache limit" and it is set to 1 MiB by default. It seems that if a file is less than this cache limit, then the file is just served from the local cache, rather than getting an updated copy from Dropbox. Is that correct? The online documentation doesn't mention anything about this behavior, but my testing does seem to reflect that. For example, if I have a text file over 1 MiB, then the changes are displayed instantly. But for a small one line text file the changes don't appear unless I invalidate the lastsync timestamp. I would be happy to update the Moodle documentation, but I have no clue what is suppose to happen.
          Hide
          Marina Glancy added a comment -

          Hi,
          $oldfile or $newfile makes no difference. I'd prefer $oldfile for the very odd case where the function update_references_to_storedfile gets changed over the time and analyzes the file id.

          Rex,
          you are completely right about Dropbox. I will try to find time to update docs. It was code of another person, I just had to re-write many things but I did not change the constant. Feel free to create an issue to convert it to the setting.
          At the moment you can change the setting (Site Administration->Plugins->Repositories->Dropbox->Cache limit) to 1 byte and files will not be cached at all.

          Show
          Marina Glancy added a comment - Hi, $oldfile or $newfile makes no difference. I'd prefer $oldfile for the very odd case where the function update_references_to_storedfile gets changed over the time and analyzes the file id. Rex, you are completely right about Dropbox. I will try to find time to update docs. It was code of another person, I just had to re-write many things but I did not change the constant. Feel free to create an issue to convert it to the setting. At the moment you can change the setting (Site Administration->Plugins->Repositories->Dropbox->Cache limit) to 1 byte and files will not be cached at all.
          Michael de Raadt made changes -
          Fix Version/s STABLE backlog [ 10463 ]
          Labels triaged
          Affects Version/s 2.4 [ 12255 ]
          Hide
          Eloy Lafuente (stronk7) added a comment -

          The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

          TIA and ciao

          Show
          Eloy Lafuente (stronk7) added a comment - The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week. TIA and ciao
          Eloy Lafuente (stronk7) made changes -
          Currently in integration Yes [ 10041 ]
          Dan Poltawski made changes -
          Status Waiting for integration review [ 10010 ] Integration review in progress [ 10004 ]
          Integrator poltawski
          Dan Poltawski made changes -
          Testing Instructions # Login as teacher
          # Go to "Manage private files"
          # Upload text file called test.txt and click save changes
          # Go to a course and add a file resource using the uploaded text file as a shortcut/alias
          # Edit text file and then go to "Manage private files" again
          # Upload edited text file and choose "Overwrite" when the prompt appears after file upload
          # Then click save changes
          # Verify error is not thrown
          Hide
          Dan Poltawski added a comment -

          I've copied the replication instructions to testing instructions. Please modify them if that doesn't make sense.

          Show
          Dan Poltawski added a comment - I've copied the replication instructions to testing instructions. Please modify them if that doesn't make sense.
          Hide
          Dan Poltawski added a comment -

          Integrated to master and 2.3, thanks.

          Show
          Dan Poltawski added a comment - Integrated to master and 2.3, thanks.
          Dan Poltawski made changes -
          Status Integration review in progress [ 10004 ] Waiting for testing [ 10005 ]
          Fix Version/s 2.3.3 [ 12373 ]
          Fix Version/s STABLE backlog [ 10463 ]
          Marina Glancy made changes -
          Testing Instructions # Login as teacher
          # Go to "Manage private files"
          # Upload text file called test.txt and click save changes
          # Go to a course and add a file resource using the uploaded text file as a shortcut/alias
          # Edit text file and then go to "Manage private files" again
          # Upload edited text file and choose "Overwrite" when the prompt appears after file upload
          # Then click save changes
          # Verify error is not thrown
          # Login as teacher
          # Go to "Manage private files"
          # Upload text file called test.txt and click save changes
          # Go to a course and add a file resource using the uploaded text file as a shortcut/alias
          # Login as student (in another browser for convenience) and try to download/view file from course
          # Make sure this is a correct file
          # Work again as teacher
          # Edit text file and then go to "Manage private files" again
          # Upload edited text file and choose "Overwrite" when the prompt appears after file upload
          # Then click save changes
          # Verify error is not thrown
          # As a student try to download/view file from course
          # Make sure this is a correct (new) file
          Tim Barker made changes -
          Tester rwijaya
          Rossiani Wijaya made changes -
          Status Waiting for testing [ 10005 ] Testing in progress [ 10011 ]
          Hide
          Rossiani Wijaya added a comment -

          This is working great.

          Thank for fixing.

          Test passed.

          Show
          Rossiani Wijaya added a comment - This is working great. Thank for fixing. Test passed.
          Rossiani Wijaya made changes -
          Status Testing in progress [ 10011 ] Tested [ 10006 ]
          Hide
          Dan Poltawski added a comment -

          Congratulations, you've done it!

          Thanks, this change is now in the latest weekly release!

          Join the crowds of people tomorrow from 8am and download this Moodle release from your local apple store!

          Show
          Dan Poltawski added a comment - Congratulations, you've done it! Thanks, this change is now in the latest weekly release! Join the crowds of people tomorrow from 8am and download this Moodle release from your local apple store!
          Dan Poltawski made changes -
          Status Tested [ 10006 ] Closed [ 6 ]
          Resolution Fixed [ 1 ]
          Currently in integration Yes [ 10041 ]
          Integration date 20/Sep/12
          Michael de Raadt made changes -
          Link This issue is duplicated by MDL-36133 [ MDL-36133 ]
          Michael de Raadt made changes -
          Link This issue is duplicated by MDL-36166 [ MDL-36166 ]
          Mary Cooch made changes -
          Labels triaged docs_required triaged
          Hide
          Ashley Holman added a comment -

          Shouldn't it pass $newfile instead of $oldfile? The update_references_to_storedfile is supposed to "Updates all files that are referencing this file with the new contenthash and filesize". It calls:

          $this->update_references($record->id, $now, $lifetime, $storedfile->get_contenthash(), $storedfile->get_filesize(), 0);

          So shouldn't it use the new size and content hash, otherwise the update isn't changing anything?

          Show
          Ashley Holman added a comment - Shouldn't it pass $newfile instead of $oldfile? The update_references_to_storedfile is supposed to "Updates all files that are referencing this file with the new contenthash and filesize". It calls: $this->update_references($record->id, $now, $lifetime, $storedfile->get_contenthash(), $storedfile->get_filesize(), 0); So shouldn't it use the new size and content hash, otherwise the update isn't changing anything?
          Hide
          Marina Glancy added a comment -

          Ashley, all fields in $oldfile are already updated by that time. See my comment above

          Show
          Marina Glancy added a comment - Ashley, all fields in $oldfile are already updated by that time. See my comment above
          Mary Cooch made changes -
          Labels docs_required triaged triaged
          Hide
          Mary Cooch added a comment -

          I've removed the docs_required label as the ability to make a shortcut was already documented -but if I have got the aspect of documentation wrong please add it back and I'll take another look.

          Show
          Mary Cooch added a comment - I've removed the docs_required label as the ability to make a shortcut was already documented -but if I have got the aspect of documentation wrong please add it back and I'll take another look.

            People

            • Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: