Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-34290

Bugs with synchronisation of files references and Dropbox improvement

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Blocker
    • Resolution: Fixed
    • Affects Version/s: 2.3, 2.4
    • Fix Version/s: 2.3.2
    • Component/s: Files API, Repositories
    • Labels:
      None
    • Testing Instructions:
      Hide

      Note: Patches are 100% the same so, it should be enough to test this under 23_STABLE only.

      Prerequisites:

      1. Make sure that all files and images you work with in this test you have not ever used on this moodle installation, which means they were never cached in filepool.
      2. Enable as much repositories as possible. In the tests below I will call ExtRefRepo the repositories Dropbox, Equella, Box.net, Filesystem and Private Files.
      3. Add in each ExtRefRepo repository several image and non-image files. All files must be different. There should be very big-sized non-image files in each repository.
      4. Create a course and enroll two users there as teachers
      5. Do some code hacks to make your testing easier:
      • search for all implementations of get_reference_file_lifetime() in repository/ folder and subfolders and replace it with 5 minutes. Do it BEFORE you start picking files otherwise it wouldn't be applied
      • in repository/lib.php change the value of const GETFILE_TIMEOUT to something smaller. This is the timeout for downloading the file when it is picked as a copy into moodle and when it is retrieved for user download

      Dropbox upgrade test. Check that references are converted.

      Before you upgrade master to this version!

      1. hack the code do decrease get_reference_file_lifetime()
      2. Create a folder resource (folder1) and add at least 3 files from Dropbox repository by reference [file1, file2, file3]
      3. Create folder2, Logout from dropbox, login again using 'logout' and 'login' links IN FILEPICKER. This will change your access_key and access_secret
      4. add files to folder2 from Dropbox by reference [file1, file2, file4]
      5. You may check in your DB now that entries in table files_reference contain the access_key and access_secret. There are two pairs of different entries for instances of references to file1 and to file2 because they have different access_key/access_secret.
      6. Remove your hack and upgrade to this version. Do not view the folders created above yet
      7. At the same time remove file3 from your Dropbox
      8. Create another folder (folder3) and add by reference files you added before upgrade [file1, file4].
      9. Check in {files_reference}

        that there is ONLY ONE entry for [file4] and it does not contain access_key and access_secret any more. There are two entries for [file1] now, one with access info and one without.

      10. View the folder1. If sync lifetime has expired, the synchronisation must be performed (no visible action though)
      11. Check in files_reference that there is one reference to file1 (without access info), two references to file2 (one with and one without access info). The reference to file3 still contains access information.
      12. Make sure that you can download file1 and file2 and can not download file3.
      13. View the folder2
      14. Check in files_reference that there is now only one reference for each file and they don't contain access information (with the exception of removed file3)
      15. Login as another user (teacher) and make sure you can download all files (except file3). Edit folders, click on each file and check that original is displayed and 'missing' string for file3.

      Test 1. This test checks that initial synchronisation has a reasonable timeout, files are not downloaded into moodle if not necessary, images are downloaded and thumbnails are generated. Also this test checks that files that are already aliases can be picked up by the same or another user (was a bug in Dropbox repository there).

      1. As User1 create a folder1 and add files as aliases from each ExtRefRepo - one image and one non-image from each.
      2. Make sure that after inserting image files are displayed in filemanager with thumbnails. It might happen that if image file is too big it will not be downloaded to moodle due to timeout and thumbnail will not be displayed.
      3. Make sure that when you insert large files the filemanager does not freeze but when you click on file you can see it's size (this means file was synchronised)
      4. Save the folder and make sure you can download all files
      5. Login as User2 from another browser where you did not login to any of ExtRefRepo repositories and make sure you can download all files.
      6. Edit this folder and click on each file to make sure you can see the original properly and size is displayed.
      7. Create folder2 and pick all files one by one from folder1 as aliases
      8. Make sure all files are picked correctly and fast, thumbnails are displayed for images and size is displayed in file info for all files. All files have the same original as the files in folder1.
      9. Save the folder and make sure you can download all files
      10. Still as User2 create folder2 and pick all files from folder1 as copies. It may take long time for big files, you may even experience timeout exception.
      11. Save the folder and make sure you can download all files

      Test 2. Making sure that synchronisation works if the external file was modified or removed. This test may be done with the folders created in Test 1.

      1. After finishing test 1 do not access any pages on moodle site for 5 minutes to make sure the previous cache expired.
      2. Go to ExtRefRepo and substitute one image and one non-image file with other files with the same name.
      3. Open folder1 and make sure the new thumbnails are displayed on view folder page
      4. Edit folder1 and also make sure that thumbnails are new, click on each file to make sure that the size is displayed correctly for each file.
      5. Edit folder2 and make sure that all files are also synchronised (this did not work before - the copies of the references were not synchronised)
      6. Do not view any pages for another 5 minutes, meanwhile remove source files from ExtRefRepo repositories
      7. View/edit folder1. Thumbnails will probably stay there but when you click on file (in edit mode) the "original" should tell that source is lost. You wouldn't be able to download any of those files (sending from cache is not implemented in the scope of this issue).
      8. Do not view any pages for another 5 minutes, meanwhile restore the files in ExtRefRepo repositories with the same names (may be different images though).
      9. View/edit folder1. Thumbnails must reflect new images. File information should display the correct original and size. This will probably not work for Box.net because the files there have unique IDs and when you create file with the same name as previously deleted it might have another ID.

      Test 3. Make sure that picking a file works from every repository. I changed argument for repository::get_file() to $reference instead of $source to fix issue with picking an alias. $source and $reference are the same for the most repositories but still worth checking. Besides I changed get_file to use timeout, so worth just checking that nothing is broken.

      1. Create a folder and pick one file from each repository as copy (including ExtRefRepo and local repositories)
      2. Make sure you can see the correct filesize when clicking on each file in filemanager.
      3. Save the folder and make sure you can view folder and download each file.
      4. Disable JS, create folder and pick files from each repository (only copies will work in non-JS mode).
      5. With JS disabled pick files from Server/Private files that are references to the files in ExtRefRepo
      6. Save and make sure you can download all files
      Show
      Note: Patches are 100% the same so, it should be enough to test this under 23_STABLE only. Prerequisites: Make sure that all files and images you work with in this test you have not ever used on this moodle installation, which means they were never cached in filepool. Enable as much repositories as possible. In the tests below I will call ExtRefRepo the repositories Dropbox, Equella, Box.net, Filesystem and Private Files . Add in each ExtRefRepo repository several image and non-image files. All files must be different. There should be very big-sized non-image files in each repository. Create a course and enroll two users there as teachers Do some code hacks to make your testing easier: search for all implementations of get_reference_file_lifetime() in repository/ folder and subfolders and replace it with 5 minutes. Do it BEFORE you start picking files otherwise it wouldn't be applied in repository/lib.php change the value of const GETFILE_TIMEOUT to something smaller. This is the timeout for downloading the file when it is picked as a copy into moodle and when it is retrieved for user download Dropbox upgrade test. Check that references are converted. Before you upgrade master to this version! hack the code do decrease get_reference_file_lifetime() Create a folder resource (folder1) and add at least 3 files from Dropbox repository by reference [file1, file2, file3] Create folder2, Logout from dropbox, login again using 'logout' and 'login' links IN FILEPICKER. This will change your access_key and access_secret add files to folder2 from Dropbox by reference [file1, file2, file4] You may check in your DB now that entries in table files_reference contain the access_key and access_secret. There are two pairs of different entries for instances of references to file1 and to file2 because they have different access_key/access_secret. Remove your hack and upgrade to this version . Do not view the folders created above yet At the same time remove file3 from your Dropbox Create another folder (folder3) and add by reference files you added before upgrade [file1, file4] . Check in {files_reference} that there is ONLY ONE entry for [file4] and it does not contain access_key and access_secret any more. There are two entries for [file1] now, one with access info and one without. View the folder1. If sync lifetime has expired, the synchronisation must be performed (no visible action though) Check in files_reference that there is one reference to file1 (without access info), two references to file2 (one with and one without access info). The reference to file3 still contains access information. Make sure that you can download file1 and file2 and can not download file3. View the folder2 Check in files_reference that there is now only one reference for each file and they don't contain access information (with the exception of removed file3) Login as another user (teacher) and make sure you can download all files (except file3). Edit folders, click on each file and check that original is displayed and 'missing' string for file3. Test 1. This test checks that initial synchronisation has a reasonable timeout, files are not downloaded into moodle if not necessary, images are downloaded and thumbnails are generated. Also this test checks that files that are already aliases can be picked up by the same or another user (was a bug in Dropbox repository there). As User1 create a folder1 and add files as aliases from each ExtRefRepo - one image and one non-image from each. Make sure that after inserting image files are displayed in filemanager with thumbnails. It might happen that if image file is too big it will not be downloaded to moodle due to timeout and thumbnail will not be displayed. Make sure that when you insert large files the filemanager does not freeze but when you click on file you can see it's size (this means file was synchronised) Save the folder and make sure you can download all files Login as User2 from another browser where you did not login to any of ExtRefRepo repositories and make sure you can download all files. Edit this folder and click on each file to make sure you can see the original properly and size is displayed. Create folder2 and pick all files one by one from folder1 as aliases Make sure all files are picked correctly and fast, thumbnails are displayed for images and size is displayed in file info for all files. All files have the same original as the files in folder1. Save the folder and make sure you can download all files Still as User2 create folder2 and pick all files from folder1 as copies. It may take long time for big files, you may even experience timeout exception. Save the folder and make sure you can download all files Test 2. Making sure that synchronisation works if the external file was modified or removed. This test may be done with the folders created in Test 1. After finishing test 1 do not access any pages on moodle site for 5 minutes to make sure the previous cache expired. Go to ExtRefRepo and substitute one image and one non-image file with other files with the same name. Open folder1 and make sure the new thumbnails are displayed on view folder page Edit folder1 and also make sure that thumbnails are new, click on each file to make sure that the size is displayed correctly for each file. Edit folder2 and make sure that all files are also synchronised (this did not work before - the copies of the references were not synchronised) Do not view any pages for another 5 minutes, meanwhile remove source files from ExtRefRepo repositories View/edit folder1. Thumbnails will probably stay there but when you click on file (in edit mode) the "original" should tell that source is lost. You wouldn't be able to download any of those files (sending from cache is not implemented in the scope of this issue). Do not view any pages for another 5 minutes, meanwhile restore the files in ExtRefRepo repositories with the same names (may be different images though). View/edit folder1. Thumbnails must reflect new images. File information should display the correct original and size. This will probably not work for Box.net because the files there have unique IDs and when you create file with the same name as previously deleted it might have another ID. Test 3. Make sure that picking a file works from every repository. I changed argument for repository::get_file() to $reference instead of $source to fix issue with picking an alias. $source and $reference are the same for the most repositories but still worth checking. Besides I changed get_file to use timeout, so worth just checking that nothing is broken. Create a folder and pick one file from each repository as copy (including ExtRefRepo and local repositories) Make sure you can see the correct filesize when clicking on each file in filemanager. Save the folder and make sure you can view folder and download each file. Disable JS, create folder and pick files from each repository (only copies will work in non-JS mode). With JS disabled pick files from Server/Private files that are references to the files in ExtRefRepo Save and make sure you can download all files
    • Affected Branches:
      MOODLE_23_STABLE, MOODLE_24_STABLE
    • Fixed Branches:
      MOODLE_23_STABLE
    • Pull Master Branch:
      wip-MDL-34290-master

      Description

      In the end of the day this issue grew quite large:

      • Synchronisation for aliases shouldfast be very . We don't need to download the file in the most cases. We try to download images though because we need them in our filepool to generate thumbnails.
      • There should be a timeout less than php script exection time for requests used in get_file/send_file
      • In order to add timeout classes curl, oauth_helper, dropbox have been changed
      • Get rid of cache_file and caching in curl, it is uncontrollable.
      • When one file referencing an external file is synchronised, all other aliases to the same external file should also be synchronised (at the moment they remain frozen). Also attempt to auto synchronise newly created references
      • Functions copy_to_area and get_file should receive as argument not the 'source' returned by repository listing but $reference = repository::get_file_reference($source). Because 'source' is not stored for aliases. In the most cases they are the same except Dropbox.
        Dropbox aliases simply did not work if they were picked by another user
      • added new function to repository API to import the referenced file into the moodle filepool because files synchronisation does not guarantee this any more.

      Dropbox repository improvements (except listed above):

      • All references to external files must be created using 'shares' functionality of Dropbox API. The unique permanent link is created by dropbox for each file and we store this URL.
      • existing links are converted to new style on the next synchronisation
      • there is a parameter for dropbox repository that tells what is the maximum size of the files to cache in our filepool (default 1M). Files smaller than this limit are retrieved during synchronisation and in cron. For bigger files we just synchronise the file size.
      • this branch also contains commit for MDL-34665 - displaying thumbnails and file size for browsing dropbox repository
      • Original for Dropbox files now contains information about user
      • Added a 'manage' link for Dropbox

      Also small improvements:

      • Updated some phpdocs, especially in repository class
      • In Equella repository I count number of failed attempts to access the server and after the 3rd do not synchronise files within this request any more.
      • Re-wrote function repository::prepare_file to use the standard functions
      • In Filesystem repository lowered the sync lifetime (no requests required here, we can do it almost realtime) and added missing function for displaying 'Original'

      initial description:

      At the moment functions such as stored_file::get_filesize() for external aliases invoke the file synchronisation which results in executing code in repository plugin before displaying the filearea contents. Errors in 3rd party code may significantly affect the core functionality

      It looked fine at the beginning but resulted with the following situation:
      http://tracker.moodle.org/browse/MDL-33700?focusedCommentId=168124#comment-168124
      in this case the external repository website is down and synchronisation function waits forever and the whole php script dies because of timeout. In this particular case Equella plugin developers can set timeout in CURL request but we need to prevent such situations in repository API.

      After discussion we decided that still the external repository functions should be called but all CURL requests must have a small timeout.

      Besides if filesize can be retrieved without downloading the file we should just retrieve filesize.

      cache_file class is excessive

        Gliffy Diagrams

          Attachments

            Issue Links

              Activity

                People

                • Votes:
                  1 Vote for this issue
                  Watchers:
                  7 Start watching this issue

                  Dates

                  • Created:
                    Updated:
                    Resolved:
                    Fix Release Date:
                    10/Sep/12