Moodle
  1. Moodle
  2. MDL-34290

Bugs with synchronisation of files references and Dropbox improvement

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Blocker 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
    • Rank:
      42656

      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

        Issue Links

          Activity

          Marina Glancy created issue -
          Marina Glancy made changes -
          Field Original Value New Value
          Link This issue has been marked as being related by MDL-33700 [ MDL-33700 ]
          Hide
          Martin Dougiamas added a comment -

          Don't want to forget this.

          Show
          Martin Dougiamas added a comment - Don't want to forget this.
          Martin Dougiamas made changes -
          Fix Version/s DEV backlog [ 10464 ]
          Priority Minor [ 4 ] Critical [ 2 ]
          Assignee moodle.com [ moodle.com ] Marina Glancy [ marina ]
          Marina Glancy made changes -
          Marina Glancy made changes -
          Summary Retrieving the list of files in filearea should not call repository plugins functions Retrieving the list of files in filearea should not hang the php script
          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.
          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
          Hide
          Marina Glancy added a comment -

          When working on this issue I took a closer look at synchronisation and found some weak issues in it. Fixing on the way:

          1. If there are several files in moodle referencing to the same source, there is only one entry in

          {files_reference}. When one of those files is synchronised, the record in {files_reference}

          is updated and only one corresponding record in

          {files}

          . All other files referencing to the same source do not update their filesize/contenthash. At the same time record in

          {files_reference}

          now says that it is synchronised and it results with other records not being updated ever.

          2. files_reference.lifetime is populated only once when the first reference is created. It is never changed later even if repository plugin developer decides to change it ( function repository::get_reference_file_lifetime() )

          Show
          Marina Glancy added a comment - When working on this issue I took a closer look at synchronisation and found some weak issues in it. Fixing on the way: 1. If there are several files in moodle referencing to the same source, there is only one entry in {files_reference}. When one of those files is synchronised, the record in {files_reference} is updated and only one corresponding record in {files} . All other files referencing to the same source do not update their filesize/contenthash. At the same time record in {files_reference} now says that it is synchronised and it results with other records not being updated ever. 2. files_reference.lifetime is populated only once when the first reference is created. It is never changed later even if repository plugin developer decides to change it ( function repository::get_reference_file_lifetime() )
          Marina Glancy made changes -
          Link This issue has been marked as being related by MDL-33416 [ MDL-33416 ]
          Marina Glancy made changes -
          Testing Instructions 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. Must be enabled Dropbox, Equella, Box.net and Filesystem (ExtRefRepo)
          # 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 only)

          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 (except ExtRefRepo because you already tested it. Also local repositories were not changed)
          # 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.
          Marina Glancy made changes -
          Testing Instructions 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. Must be enabled Dropbox, Equella, Box.net and Filesystem (ExtRefRepo)
          # 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 only)

          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 (except ExtRefRepo because you already tested it. Also local repositories were not changed)
          # 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.
          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 only)

          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 (except ExtRefRepo because you already tested it. Also local repositories were not changed)
          # 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.
          Marina Glancy made changes -
          Testing Instructions 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 only)

          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 (except ExtRefRepo because you already tested it. Also local repositories were not changed)
          # 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.
          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 only)

          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
          Hide
          Marina Glancy added a comment -

          The following issues are being fixed:

          1. Synchronisation for aliases should be very fast. 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.
          2. There should be a timeout less than php script exection time for requests used in get_file/send_file
          3. In order to add timeout classes curl, oauth_helper, dropbox have been changed
          4. Get rid of cache_file and caching in curl, it is uncontrollable. Commented out cron caching for dropbox. There is no caching of external files at the moment (should be another issue for that).
          5. 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)
          6. 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.
          7. Dropbox aliases simply did not work if they were picked by another user

          Also small improvements:

          1. Original for Dropbox files now contains information about user
          2. Added a 'manage' link for Dropbox
          3. Updated some phpdocs, especially in repository class
          4. 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.
          5. Re-wrote function repository::prepare_file to use the standard functions
          6. In Filesystem repository lowered the sync lifetime (no requests required here, we can do it almost realtime) and added missing function for displaying 'Original'
          Show
          Marina Glancy added a comment - The following issues are being fixed: Synchronisation for aliases should be very fast. 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. Commented out cron caching for dropbox. There is no caching of external files at the moment (should be another issue for that). 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) 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 Also small improvements: Original for Dropbox files now contains information about user Added a 'manage' link for Dropbox 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'
          Hide
          David Mudrak added a comment -

          Hi Marina. Thanks for your work on this. Quick notes after looking at the patch.

          • By 'following issues are being fixed', do you mean you still working on them? For example there is 'Get rid of cache_file'. Does it mean you plan to do so or that the current patch at wip-MDL-34290-master already implements it? I can still some references to the cache_file class so I'm just wondering.
          • At https://github.com/marinaglancy/moodle/compare/master...wip-MDL-34290-master#L11R129 your new phpDocs says "If we received the connection timeout more than 3 times in a row, we don't attemt to connect to the server any more during this session.". I am probably missing something but it seems to me that the counter is valid for the given request only, not the whole user session (I can't see that being stored in teh $_SESSION). From that point of view, using the sesskey as the array key seems irrelevant. Also, the phpdocs for @param int $errno is not clear.
          Show
          David Mudrak added a comment - Hi Marina. Thanks for your work on this. Quick notes after looking at the patch. By 'following issues are being fixed', do you mean you still working on them? For example there is 'Get rid of cache_file'. Does it mean you plan to do so or that the current patch at wip- MDL-34290 -master already implements it? I can still some references to the cache_file class so I'm just wondering. At https://github.com/marinaglancy/moodle/compare/master...wip-MDL-34290-master#L11R129 your new phpDocs says "If we received the connection timeout more than 3 times in a row, we don't attemt to connect to the server any more during this session.". I am probably missing something but it seems to me that the counter is valid for the given request only, not the whole user session (I can't see that being stored in teh $_SESSION). From that point of view, using the sesskey as the array key seems irrelevant. Also, the phpdocs for @param int $errno is not clear.
          Marina Glancy made changes -
          Link This issue will help resolve MDL-33989 [ MDL-33989 ]
          Hide
          Marina Glancy added a comment -
          • David, by "are being fixed" I meant that I fix them as well in this issue.
          • I changed phpdoc, session->request
          Show
          Marina Glancy added a comment - David, by "are being fixed" I meant that I fix them as well in this issue. I changed phpdoc, session->request
          Hide
          Marina Glancy added a comment -

          David, I split my changes into different commits. It is ready for your review.
          I also want to try to add timeout to S3, webdav and googledocs requests but it's a wishlist

          Show
          Marina Glancy added a comment - David, I split my changes into different commits. It is ready for your review. I also want to try to add timeout to S3, webdav and googledocs requests but it's a wishlist
          Marina Glancy made changes -
          Link This issue will help resolve MDL-34665 [ MDL-34665 ]
          Dan Poltawski made changes -
          Status Open [ 1 ] Peer review in progress [ 10013 ]
          Peer reviewer poltawski
          Hide
          Dan Poltawski added a comment -

          Not full comments, but some anyway:

          1. download_one should would be nicer named download_file()?
          2. I think it'd be better to throw an exception rather than return two 'true' results with * @return bool|string true on success or error string on failure
          3. Similarly shoudln't that function throw an error if filepath/file missing??
          4. Commit message on: https://github.com/marinaglancy/moodle/commit/525e11696733480e1d720249b0cfae463d3a2c2c
            "now" -> "know", and it'd be good if you could add linebreaks there so it wraps nicely
          5. Why do we need to check both hash and filesize in file_save_draft_area_files? Surely its impossible for the filesize to change without the hash changing?
            if ($oldfile->get_contenthash() != $newfile->get_contenthash() || $oldfile->get_filesize() != $newfile->get_filesize()) {
            
          6. Doing the cast here is not necessary, update_record will do it for you $DB->update_record('files_reference', (object)$data);
          Show
          Dan Poltawski added a comment - Not full comments, but some anyway: download_one should would be nicer named download_file()? I think it'd be better to throw an exception rather than return two 'true' results with * @return bool|string true on success or error string on failure Similarly shoudln't that function throw an error if filepath/file missing?? Commit message on: https://github.com/marinaglancy/moodle/commit/525e11696733480e1d720249b0cfae463d3a2c2c "now" -> "know", and it'd be good if you could add linebreaks there so it wraps nicely Why do we need to check both hash and filesize in file_save_draft_area_files? Surely its impossible for the filesize to change without the hash changing? if ($oldfile->get_contenthash() != $newfile->get_contenthash() || $oldfile->get_filesize() != $newfile->get_filesize()) { Doing the cast here is not necessary, update_record will do it for you $DB->update_record('files_reference', (object)$data);
          Hide
          Marina Glancy added a comment -

          Hi Dan,

          1. download_one() is called so because the already existing function download() is for downloading multiple files
          2. I just made it the same as get(), request(), head() do it in the same class
          3. curl class does not throw exceptions. I already made a lot of changes to the API that is supposed to be working fine but apparently does not. I don't want to change more than needed
          4. fixed
          5. because if we don't download a file during synchronisation, the contenthash equals to sha1('') and it is not changed when file is changed externally. This was the very idea of this issue - not to download referenced file when not needed
          6. phpdocs to update_record() says that it is stdClass. Although I can see that all existing db drivers cast it to array
          Show
          Marina Glancy added a comment - Hi Dan, download_one() is called so because the already existing function download() is for downloading multiple files I just made it the same as get(), request(), head() do it in the same class curl class does not throw exceptions. I already made a lot of changes to the API that is supposed to be working fine but apparently does not. I don't want to change more than needed fixed because if we don't download a file during synchronisation, the contenthash equals to sha1('') and it is not changed when file is changed externally. This was the very idea of this issue - not to download referenced file when not needed phpdocs to update_record() says that it is stdClass. Although I can see that all existing db drivers cast it to array
          Marina Glancy made changes -
          Testing Instructions 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 only)

          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
          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 and cached files were transferred to the new location.

          *Before you upgrade master to this version!*
          # hack the code do decrease get_reference_file_lifetime()
          # As user 1: Create a folder resource and add several files from Dropbox repository by reference
          # As user 2: Create a folder resource, add files from your Dropbox repository by referece (this should be different Dropbox repository than in previous item)
          # As user 2 logout from Dropbox using 'logout' link in the filepicker AND logout from dropbox in your browser
          # As user 2 login to Dropbox (this action will create new access_key/access_secret pair)
          # You may check in your DB now that entries in table {files_reference} contain the access_key and access_secret
          # Remove your hack and *upgrade to this version*. Do not view the folders created above yet
          # As user 1: Create another folder and add by reference one of the files you added to folder 1.
          # Check in {files_reference} that there is ONLY ONE entry for this file and it does not contain access_key and access_secret any more
          # View the folder created by user 1. If sync lifetime has expired, the synchronisation must be performed (no visible action though)
          # Check in {files_reference} that all entries for user 1 dropbox files now contain url and do not contain access_key and access_secret
          # View the folder created by user 2. You should be able to download the files (they were cached)
          # Edit the folder created by user 2. If you click on file it will say that original is missing.
          # the entires in {files_reference} for those files are not changed. access_key and access_secret do not work any more but there is nothing to substitute them with.

          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
          Hide
          Martin Dougiamas added a comment -

          Thanks Dan and David for looking at this. I hope we can land it soon.

          Show
          Martin Dougiamas added a comment - Thanks Dan and David for looking at this. I hope we can land it soon.
          Marina Glancy made changes -
          Link This issue has been marked as being related by MDL-34668 [ MDL-34668 ]
          Marina Glancy made changes -
          Testing Instructions 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 and cached files were transferred to the new location.

          *Before you upgrade master to this version!*
          # hack the code do decrease get_reference_file_lifetime()
          # As user 1: Create a folder resource and add several files from Dropbox repository by reference
          # As user 2: Create a folder resource, add files from your Dropbox repository by referece (this should be different Dropbox repository than in previous item)
          # As user 2 logout from Dropbox using 'logout' link in the filepicker AND logout from dropbox in your browser
          # As user 2 login to Dropbox (this action will create new access_key/access_secret pair)
          # You may check in your DB now that entries in table {files_reference} contain the access_key and access_secret
          # Remove your hack and *upgrade to this version*. Do not view the folders created above yet
          # As user 1: Create another folder and add by reference one of the files you added to folder 1.
          # Check in {files_reference} that there is ONLY ONE entry for this file and it does not contain access_key and access_secret any more
          # View the folder created by user 1. If sync lifetime has expired, the synchronisation must be performed (no visible action though)
          # Check in {files_reference} that all entries for user 1 dropbox files now contain url and do not contain access_key and access_secret
          # View the folder created by user 2. You should be able to download the files (they were cached)
          # Edit the folder created by user 2. If you click on file it will say that original is missing.
          # the entires in {files_reference} for those files are not changed. access_key and access_secret do not work any more but there is nothing to substitute them with.

          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
          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
          Marina Glancy made changes -
          Testing Instructions 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
          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
          Hide
          Marina Glancy added a comment -

          The status is already 'peer review in progress' so I can't change it. This issue is waiting for peer review and integration.
          The function download_one is still not renamed can do it if necessary

          Show
          Marina Glancy added a comment - The status is already 'peer review in progress' so I can't change it. This issue is waiting for peer review and integration. The function download_one is still not renamed can do it if necessary
          Marina Glancy made changes -
          Summary Retrieving the list of files in filearea should not hang the php script Bugs with synchronisation of files references and Dropbox improvement
          Marina Glancy made changes -
          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
          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
          Dan Poltawski made changes -
          Status Peer review in progress [ 10013 ] Waiting for integration review [ 10010 ]
          Hide
          Dan Poltawski added a comment -

          Putting into this weeks integration state as this has been waiting on me.

          Show
          Dan Poltawski added a comment - Putting into this weeks integration state as this has been waiting on me.
          Dan Poltawski made changes -
          Currently in integration Yes [ 10041 ]
          Hide
          Dan Poltawski added a comment -

          This should be declared PARAM_INT, rather than doing custom validation (I think you can even add a numeric client side mform rule if necessary).
          $mform->addElement('text', 'dropbox_cachelimit', get_string('cachelimit', 'repository_dropbox'), array('size' => '40'));

          
          
          Show
          Dan Poltawski added a comment - This should be declared PARAM_INT, rather than doing custom validation (I think you can even add a numeric client side mform rule if necessary). $mform->addElement('text', 'dropbox_cachelimit', get_string('cachelimit', 'repository_dropbox'), array('size' => '40'));
          Hide
          Marina Glancy added a comment -

          Dan, I tried adding PARAM_INT but there is no effect. is_validated() is never called for this form. I am not going to fix this form because if I fix ALL things like that it would be easier to re-write everything

          Show
          Marina Glancy added a comment - Dan, I tried adding PARAM_INT but there is no effect. is_validated() is never called for this form. I am not going to fix this form because if I fix ALL things like that it would be easier to re-write everything
          Hide
          Dan Poltawski added a comment -

          I've not heard for is_validated() before.

          What about:

          @@ -463,26 +463,12 @@ class repository_dropbox extends repository {
                   $mform->addElement('static', null, '',  $str_getkey);
           
                   $mform->addElement('text', 'dropbox_cachelimit', get_string('cachelimit', 'repository_dropbox'), array('size' => '40'));
          +        $mform->addRule('dropbox_cachelimit', null, 'numeric', null, 'client');
          +        $mform->setType('dropbox_cachelimit', PARAM_INT);
                   $mform->addElement('static', 'dropbox_cachelimit_info', '',  get_string('cachelimit_info', 'repository_dropbox'));
               }
           
               /**
          -     * Validate Admin Settings Moodle form
          -     *
          -     * @param moodleform $mform Moodle form (passed by reference)
          -     * @param array $data array of ("fieldname"=>value) of submitted data
          -     * @param array $errors array of ("fieldname"=>errormessage) of errors
          -     * @return array array of errors
          -     */
          -    public static function type_form_validation($mform, $data, $errors) {
          -        if (!empty($data['dropbox_cachelimit']) && (!is_number($data['dropbox_cachelimit']) ||
          -                (int)$data['dropbox_cachelimit']<0)) {
          -            $errors['dropbox_cachelimit'] = get_string('error_cachelimit', 'repository_dropbox');
          -        }
          -        return $errors;
          -    }
          -
          -    /**
                * Option names of dropbox plugin
                *
                * @return array
          
          Show
          Dan Poltawski added a comment - I've not heard for is_validated() before. What about: @@ -463,26 +463,12 @@ class repository_dropbox extends repository { $mform->addElement(' static ', null , '', $str_getkey); $mform->addElement('text', 'dropbox_cachelimit', get_string('cachelimit', 'repository_dropbox'), array('size' => '40')); + $mform->addRule('dropbox_cachelimit', null , 'numeric', null , 'client'); + $mform->setType('dropbox_cachelimit', PARAM_INT); $mform->addElement(' static ', 'dropbox_cachelimit_info', '', get_string('cachelimit_info', 'repository_dropbox')); } /** - * Validate Admin Settings Moodle form - * - * @param moodleform $mform Moodle form (passed by reference) - * @param array $data array of ( "fieldname" =>value) of submitted data - * @param array $errors array of ( "fieldname" =>errormessage) of errors - * @ return array array of errors - */ - public static function type_form_validation($mform, $data, $errors) { - if (!empty($data['dropbox_cachelimit']) && (!is_number($data['dropbox_cachelimit']) || - ( int )$data['dropbox_cachelimit']<0)) { - $errors['dropbox_cachelimit'] = get_string('error_cachelimit', 'repository_dropbox'); - } - return $errors; - } - - /** * Option names of dropbox plugin * * @ return array
          Hide
          Marina Glancy added a comment -

          It's even worse with PARAM_INT. The form automatically casts the value to int and does not give any warning. For example, if somebody says "1M", it will cast it to 1 (byte), no error/warning is displayed, form disappears and user will never know that the value was entered incorrectly.

          Show
          Marina Glancy added a comment - It's even worse with PARAM_INT. The form automatically casts the value to int and does not give any warning. For example, if somebody says "1M", it will cast it to 1 (byte), no error/warning is displayed, form disappears and user will never know that the value was entered incorrectly.
          Hide
          Dan Poltawski added a comment -

          The client rule sorts that out - have you tried the code I gave above, it works.

          Show
          Dan Poltawski added a comment - The client rule sorts that out - have you tried the code I gave above, it works.
          Hide
          Marina Glancy added a comment - - edited

          What you suggested Dan works if javascript is enabled. If you want I can change this, np.

          And if you are really sure this is the biggest problem with this patch then that's great

          Show
          Marina Glancy added a comment - - edited What you suggested Dan works if javascript is enabled. If you want I can change this, np. And if you are really sure this is the biggest problem with this patch then that's great
          Hide
          Dan Poltawski added a comment -

          No, i'm not sure its the biggest problem with the patch, but its something I spotted and we have moodle tools to deal with it.

          Show
          Dan Poltawski added a comment - No, i'm not sure its the biggest problem with the patch, but its something I spotted and we have moodle tools to deal with it.
          Hide
          Marina Glancy added a comment -

          BTW, https://github.com/moodle/moodle/blob/master/lib/formslib.php#L480
          this is the function moodleform::is_validated()

          Show
          Marina Glancy added a comment - BTW, https://github.com/moodle/moodle/blob/master/lib/formslib.php#L480 this is the function moodleform::is_validated()
          Hide
          Marina Glancy added a comment -

          Dan, I added commit upon your suggested patch. Although I don't completely agree with it.

          Show
          Marina Glancy added a comment - Dan, I added commit upon your suggested patch. Although I don't completely agree with it.
          Martin Dougiamas made changes -
          Link This issue has been marked as being related by MDL-34185 [ MDL-34185 ]
          Hide
          Eloy Lafuente (stronk7) added a comment -

          I suppose this is for 2.3 stable too, correct? As far as they are bugfixes...

          Show
          Eloy Lafuente (stronk7) added a comment - I suppose this is for 2.3 stable too, correct? As far as they are bugfixes...
          Hide
          Eloy Lafuente (stronk7) added a comment -

          The integration of this issue has been delayed to next week because the integration period is over (Monday, Tuesday) and testing must happen on Wednesday.

          This change to a more rigid timeframe on each integration/testing cycle aims to produce a better and clear separation and organization of tasks for everybody.

          This is a bulk-automated message, so if you want to blame somebody/thing/where, don't do it here (use git instead) :-D :-P

          Apologizes for the inconvenient, this will be integrated next week. Thanks for your collaboration & ciao

          Show
          Eloy Lafuente (stronk7) added a comment - The integration of this issue has been delayed to next week because the integration period is over (Monday, Tuesday) and testing must happen on Wednesday. This change to a more rigid timeframe on each integration/testing cycle aims to produce a better and clear separation and organization of tasks for everybody. This is a bulk-automated message, so if you want to blame somebody/thing/where, don't do it here (use git instead) :-D :-P Apologizes for the inconvenient, this will be integrated next week. Thanks for your collaboration & ciao
          Eloy Lafuente (stronk7) made changes -
          Currently in integration Yes [ 10041 ]
          Dan Poltawski made changes -
          Currently in integration Yes [ 10041 ]
          Hide
          Eloy Lafuente (stronk7) added a comment -

          I suppose this is for 2.3 stable too, correct? As far as they are bugfixes...

          Show
          Eloy Lafuente (stronk7) added a comment - I suppose this is for 2.3 stable too, correct? As far as they are bugfixes...
          Hide
          Eloy Lafuente (stronk7) added a comment -

          The integration of this issue has been delayed to next week because the integration period is over (Monday, Tuesday) and testing must happen on Wednesday.

          This change to a more rigid timeframe on each integration/testing cycle aims to produce a better and clear separation and organization of tasks for everybody.

          This is a bulk-automated message, so if you want to blame somebody/thing/where, don't do it here (use git instead) :-D :-P

          Apologizes for the inconvenient, this will be integrated next week. Thanks for your collaboration & ciao

          Show
          Eloy Lafuente (stronk7) added a comment - The integration of this issue has been delayed to next week because the integration period is over (Monday, Tuesday) and testing must happen on Wednesday. This change to a more rigid timeframe on each integration/testing cycle aims to produce a better and clear separation and organization of tasks for everybody. This is a bulk-automated message, so if you want to blame somebody/thing/where, don't do it here (use git instead) :-D :-P Apologizes for the inconvenient, this will be integrated next week. Thanks for your collaboration & ciao
          Eloy Lafuente (stronk7) made changes -
          Currently in integration Yes [ 10041 ]
          Martin Dougiamas made changes -
          Priority Critical [ 2 ] Blocker [ 1 ]
          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
          Aparup Banerjee made changes -
          Currently in integration Yes [ 10041 ]
          Hide
          Marina Glancy added a comment -

          TO INTEGRATORS: yes, it's for 2.3 as well. I'm waiting for someone's review to create a 2.3 branch

          Show
          Marina Glancy added a comment - TO INTEGRATORS: yes, it's for 2.3 as well. I'm waiting for someone's review to create a 2.3 branch
          Marina Glancy made changes -
          Link This issue will help resolve MDL-33695 [ MDL-33695 ]
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Yay, at last! Now I can begin examining this (eh, 100% joking, lol).

          Show
          Eloy Lafuente (stronk7) added a comment - Yay, at last! Now I can begin examining this (eh, 100% joking, lol).
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Hi, for reference, here it is one list of (mainly questions) while I go commit by commit reviewing this:

          1) 54730b73e1: doesn't replace_content_with() need to also update some other information of the file, like timemodified. Or, in other words, if the content/size has not changed... has any sense to be modifying the timemodified?

          2) 54730b73e1: update_references_to_storedfile(). It's missing one $rs->close().

          3) 54730b73e1: update_references() it's used both for references to stored files and to external stuff correct? What's the rationale about cleaning all those parameters? Aren't all them private stuff not needing any cleaning? Also, why don't we update (again) timemodified and friends (is that perhaps, part of the API and the corresponding get_timemodified() method is being routed in some way to its source (stored or external)?

          4) 54730b73e1: set_synchronized() it's not clear to me about if performing that sort of "transversal" update (aka automatically updating all "sisters" pointing to same reference) is oki or that should be achieved in another way, controlled by their common reference. But surely there are good reasons for that, just they are out from my knowledge.

          5) a6223a0a99: perhaps it would be better to call it 'curl_options' instead of 'http_options'. Not critical at all, and phpdocs state it clearly but...

          6) 80c232f2d6: Are SYNCFILE_TIMEOUT and SYNCIMAGE_TIMEOUT realistic enough. I can imagine > 1 second to perform any required authentication/authorization and then ask for file size. Or > 3 seconds downloading one image.

          The rest looks really promising. I got a bit lost with the file_cache deletion and with the "semi-deprecation" of those referencelastxxx fields, but I've nothing to object to it.

          I really find it too-much "intricate" with interdependencies happening in all directions, but I'm sure (hope) that all them are crystal clear in your BIG head. They are clearly beyond my understanding, specially trying to "digest" everything together in a couple of hours.

          I've not found anything obvious/critical causing this to be halted and, as far as testing seems complete enough, ... I'd be happy to integrate this both to master and 23_STABLE.

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Hi, for reference, here it is one list of (mainly questions) while I go commit by commit reviewing this: 1) 54730b73e1: doesn't replace_content_with() need to also update some other information of the file, like timemodified. Or, in other words, if the content/size has not changed... has any sense to be modifying the timemodified? 2) 54730b73e1: update_references_to_storedfile(). It's missing one $rs->close(). 3) 54730b73e1: update_references() it's used both for references to stored files and to external stuff correct? What's the rationale about cleaning all those parameters? Aren't all them private stuff not needing any cleaning? Also, why don't we update (again) timemodified and friends (is that perhaps, part of the API and the corresponding get_timemodified() method is being routed in some way to its source (stored or external)? 4) 54730b73e1: set_synchronized() it's not clear to me about if performing that sort of "transversal" update (aka automatically updating all "sisters" pointing to same reference) is oki or that should be achieved in another way, controlled by their common reference. But surely there are good reasons for that, just they are out from my knowledge. 5) a6223a0a99: perhaps it would be better to call it 'curl_options' instead of 'http_options'. Not critical at all, and phpdocs state it clearly but... 6) 80c232f2d6: Are SYNCFILE_TIMEOUT and SYNCIMAGE_TIMEOUT realistic enough. I can imagine > 1 second to perform any required authentication/authorization and then ask for file size. Or > 3 seconds downloading one image. The rest looks really promising. I got a bit lost with the file_cache deletion and with the "semi-deprecation" of those referencelastxxx fields, but I've nothing to object to it. I really find it too-much "intricate" with interdependencies happening in all directions, but I'm sure (hope) that all them are crystal clear in your BIG head. They are clearly beyond my understanding, specially trying to "digest" everything together in a couple of hours. I've not found anything obvious/critical causing this to be halted and, as far as testing seems complete enough, ... I'd be happy to integrate this both to master and 23_STABLE. Ciao
          Eloy Lafuente (stronk7) made changes -
          Status Waiting for integration review [ 10010 ] Integration review in progress [ 10004 ]
          Integrator stronk7
          Hide
          Marina Glancy added a comment -

          I added $rs->close() and also check for contributed repositories that they don't accidentally return non-existing contenthash.
          Other Eloy comments we discussed in chat. There will be another issue for making the repository::GETFILE_TIMEOUT more flexible.

          Added a branch for 2.3

          Show
          Marina Glancy added a comment - I added $rs->close() and also check for contributed repositories that they don't accidentally return non-existing contenthash. Other Eloy comments we discussed in chat. There will be another issue for making the repository::GETFILE_TIMEOUT more flexible. Added a branch for 2.3
          Marina Glancy made changes -
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Integrated (23 & master), thanks!

          Show
          Eloy Lafuente (stronk7) added a comment - Integrated (23 & master), thanks!
          Eloy Lafuente (stronk7) made changes -
          Status Integration review in progress [ 10004 ] Waiting for testing [ 10005 ]
          Affects Version/s 2.4 [ 12255 ]
          Fix Version/s 2.3.2 [ 12353 ]
          Fix Version/s DEV backlog [ 10464 ]
          Eloy Lafuente (stronk7) made changes -
          Testing Instructions 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
          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
          Hide
          Frédéric Massart added a comment -

          Tested everything went OK. Just happened that the some of the links were not merged, but I checked with Marina and this is not an issue, it was just a missing test step which should mention to hack the get_reference_file_lifetime() after upgrading the instance.

          Show
          Frédéric Massart added a comment - Tested everything went OK. Just happened that the some of the links were not merged, but I checked with Marina and this is not an issue, it was just a missing test step which should mention to hack the get_reference_file_lifetime() after upgrading the instance.
          David Monllaó made changes -
          Link This issue testing discovered MDL-35139 [ MDL-35139 ]
          Hide
          David Monllaó added a comment -

          I've created new http://tracker.moodle.org/browse/MDL-35139, discovered while testing

          Show
          David Monllaó added a comment - I've created new http://tracker.moodle.org/browse/MDL-35139 , discovered while testing
          Tim Barker made changes -
          Tester rajeshtaneja
          Rajesh Taneja made changes -
          Status Waiting for testing [ 10005 ] Testing in progress [ 10011 ]
          Hide
          Rajesh Taneja added a comment -

          Sorry Marina,

          I can't get my head around Test 2.

          Issue 1: (dropbox)
          New image thumbnail is not visible.
          Steps to reproduce:

          1. Add alias image from dropbox
          2. Change actual image on dropbox, with same name
          3. After 5+ min refresh page and image thumbnail is changed to default image icon. It's not showing new image thumnail.

          Issue 2: (Box.net)
          box.net alias is always showing "Sync error" icon (dragon icon). I checked on David's machine as well and it's same. Although, changing image on box.net also change icon to default image icon.

          David has also found few things. His reviews are here: http://tracker.moodle.org/secure/ViewSession.jspa?testSessionId=10441

          FYI: (Equella)
          It's all broken and hence not tested. As discussed with you, it's another issue so ignored.

          Failing it for now. Not sure, if it's a major concern at this point. Please feel free to pass if you feel otherwise.

          Show
          Rajesh Taneja added a comment - Sorry Marina, I can't get my head around Test 2. Issue 1: (dropbox) New image thumbnail is not visible. Steps to reproduce: Add alias image from dropbox Change actual image on dropbox, with same name After 5+ min refresh page and image thumbnail is changed to default image icon. It's not showing new image thumnail. Issue 2: (Box.net) box.net alias is always showing "Sync error" icon (dragon icon). I checked on David's machine as well and it's same. Although, changing image on box.net also change icon to default image icon. David has also found few things. His reviews are here: http://tracker.moodle.org/secure/ViewSession.jspa?testSessionId=10441 FYI: (Equella) It's all broken and hence not tested. As discussed with you, it's another issue so ignored. Failing it for now. Not sure, if it's a major concern at this point. Please feel free to pass if you feel otherwise.
          Rajesh Taneja made changes -
          Status Testing in progress [ 10011 ] Problem during testing [ 10007 ]
          Hide
          Dongsheng Cai added a comment -

          Hi Rajesh

          Can you please write more details about the EQUELLA plugin? I will investigate and fix it in another issue.

          Show
          Dongsheng Cai added a comment - Hi Rajesh Can you please write more details about the EQUELLA plugin? I will investigate and fix it in another issue.
          Hide
          Marina Glancy added a comment -

          Dongsheng, there has already been issue MDL-33809. Basically it is about that Equella showing the "select" link for the file collection which is actually an HTML page with links and it is not compartible with our repositories API.
          But during testing yesterday we noticed that Equella (the content loaded in <object>) does not allow any more to select individual files. Therefore it is useless for filepicker.

          Show
          Marina Glancy added a comment - Dongsheng, there has already been issue MDL-33809 . Basically it is about that Equella showing the "select" link for the file collection which is actually an HTML page with links and it is not compartible with our repositories API. But during testing yesterday we noticed that Equella (the content loaded in <object>) does not allow any more to select individual files. Therefore it is useless for filepicker.
          Hide
          Dongsheng Cai added a comment -

          Thanks Marina.

          Show
          Dongsheng Cai added a comment - Thanks Marina.
          Michael de Raadt made changes -
          Status Problem during testing [ 10007 ] Integration review in progress [ 10004 ]
          Michael de Raadt made changes -
          Status Integration review in progress [ 10004 ] Waiting for integration review [ 10010 ]
          Michael de Raadt made changes -
          Status Waiting for integration review [ 10010 ] Integration review in progress [ 10004 ]
          Michael de Raadt made changes -
          Status Integration review in progress [ 10004 ] Waiting for testing [ 10005 ]
          Hide
          Michael de Raadt added a comment - - edited

          I just reset testing on this issue on the request of Raj.

          Show
          Michael de Raadt added a comment - - edited I just reset testing on this issue on the request of Raj.
          Rajesh Taneja made changes -
          Status Waiting for testing [ 10005 ] Testing in progress [ 10011 ]
          Hide
          Rajesh Taneja added a comment -

          As discussed with Marina,

          Passing this issue and she will open another issue to look at Test 2 problems.

          Show
          Rajesh Taneja added a comment - As discussed with Marina, Passing this issue and she will open another issue to look at Test 2 problems.
          Rajesh Taneja made changes -
          Status Testing in progress [ 10011 ] Tested [ 10006 ]
          Hide
          Eloy Lafuente (stronk7) added a comment -

          I'm so proud...of you, many thanks!

          http://youtu.be/n64CdfDRnZY

          Closing as fixed, ciao

          Show
          Eloy Lafuente (stronk7) added a comment - I'm so proud...of you, many thanks! http://youtu.be/n64CdfDRnZY Closing as fixed, ciao
          Eloy Lafuente (stronk7) made changes -
          Status Tested [ 10006 ] Closed [ 6 ]
          Resolution Fixed [ 1 ]
          Currently in integration Yes [ 10041 ]
          Integration date 31/Aug/12
          Rex Lorenzo made changes -
          Link This issue caused a regression MDL-35376 [ MDL-35376 ]
          Dongsheng Cai made changes -
          Link This issue has a non-specific relationship to MDL-35729 [ MDL-35729 ]
          Michael de Raadt made changes -
          Link This issue caused a regression MDL-38091 [ MDL-38091 ]

            People

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

              Dates

              • Created:
                Updated:
                Resolved: