Details

    • Testing Instructions:
      Hide
      1. Create a Moodle 1.9 site and ensure debugging is turned on.
      2. Visit a course and choose to add a file resource.
      3. Create a file that has a file name with the length of 255 characters (including the extension).
      4. Upgrade to 2.2 and ensure the resource module upgrades successfully.
      5. Browse to the course and click to view it.
      Show
      Create a Moodle 1.9 site and ensure debugging is turned on. Visit a course and choose to add a file resource. Create a file that has a file name with the length of 255 characters (including the extension). Upgrade to 2.2 and ensure the resource module upgrades successfully. Browse to the course and click to view it.
    • Affected Branches:
      MOODLE_22_STABLE
    • Fixed Branches:
      MOODLE_22_STABLE

      Description

      This has been previously reported in MDL-32851 and incorrectly marked as "not a bug". The issue is quite simple:
      during upgrade of resources, the value from mdl_resource_old.reference is copied into mdl_resource.mainfile but with additional character - "/" (slash) at the beginning. Both fields are varchar(255) so if mdl_resource_old.reference has length of 255, the total length of the string for mdl_resource.mainfile will be 256 - hence the error.
      During an upgrade to the latest version the field mdl_resource.mainfile is dropped - so you won't see it in that table once upgrade is successfully finished.

        Gliffy Diagrams

          Attachments

            Issue Links

              Activity

              Hide
              tmuras Tomasz Muras added a comment -

              Another backtrace for developer's convenience:
              Debug info: Data too long for column 'mainfile' at row 1
              UPDATE mdl_resource SET tobemigrated = ?,mainfile = ?,filterfiles = ?,legacyfiles = ?,display = ?,displayoptions = ? WHERE id=?
              [array (
              0 => 0,
              1 => '/This 81-page handbook from Pierce provides protocols and technical and product information to help maximize results for protein purification procedures. The handbook provides background, helpful hints and troubleshooting advice for covalent coupling of af',
              2 => '1',
              3 => 0,
              4 => 6,
              5 => 'a:4:

              {s:12:"printheading";i:0;s:10:"printintro";i:1;s:10:"popupwidth";s:3:"620";s:11:"popupheight";s:3:"450";}

              ',
              6 => '35676',
              )]
              Stack trace:

              line 397 of /lib/dml/moodle_database.php: dml_write_exception thrown
              line 999 of /lib/dml/mysqli_native_moodle_database.php: call to moodle_database->query_end()
              line 1031 of /lib/dml/mysqli_native_moodle_database.php: call to mysqli_native_moodle_database->update_record_raw()
              line 173 of /mod/resource/db/upgradelib.php: call to mysqli_native_moodle_database->update_record()
              line 181 of /mod/resource/db/upgrade.php: call to resource_20_migrate()
              line 540 of /lib/upgradelib.php: call to xmldb_resource_upgrade()
              line 271 of /lib/upgradelib.php: call to upgrade_plugins_modules()
              line 1437 of /lib/upgradelib.php: call to upgrade_plugins()
              line 269 of /admin/index.php: call to upgrade_noncore()

              Show
              tmuras Tomasz Muras added a comment - Another backtrace for developer's convenience: Debug info: Data too long for column 'mainfile' at row 1 UPDATE mdl_resource SET tobemigrated = ?,mainfile = ?,filterfiles = ?,legacyfiles = ?,display = ?,displayoptions = ? WHERE id=? [array ( 0 => 0, 1 => '/This 81-page handbook from Pierce provides protocols and technical and product information to help maximize results for protein purification procedures. The handbook provides background, helpful hints and troubleshooting advice for covalent coupling of af', 2 => '1', 3 => 0, 4 => 6, 5 => 'a:4: {s:12:"printheading";i:0;s:10:"printintro";i:1;s:10:"popupwidth";s:3:"620";s:11:"popupheight";s:3:"450";} ', 6 => '35676', )] Stack trace: line 397 of /lib/dml/moodle_database.php: dml_write_exception thrown line 999 of /lib/dml/mysqli_native_moodle_database.php: call to moodle_database->query_end() line 1031 of /lib/dml/mysqli_native_moodle_database.php: call to mysqli_native_moodle_database->update_record_raw() line 173 of /mod/resource/db/upgradelib.php: call to mysqli_native_moodle_database->update_record() line 181 of /mod/resource/db/upgrade.php: call to resource_20_migrate() line 540 of /lib/upgradelib.php: call to xmldb_resource_upgrade() line 271 of /lib/upgradelib.php: call to upgrade_plugins_modules() line 1437 of /lib/upgradelib.php: call to upgrade_plugins() line 269 of /admin/index.php: call to upgrade_noncore()
              Hide
              salvetore Michael de Raadt added a comment -

              Hi, Tomasz.

              The bug was closed at one stage, but it was reopened and work was done on the issue.

              I assume that you have found another way to trigger the same problem not covered by the original solution?

              Show
              salvetore Michael de Raadt added a comment - Hi, Tomasz. The bug was closed at one stage, but it was reopened and work was done on the issue. I assume that you have found another way to trigger the same problem not covered by the original solution?
              Hide
              salvetore Michael de Raadt added a comment -

              That last comment was not meant to be a question, really, but I guess I was waiting for you to respond.

              Show
              salvetore Michael de Raadt added a comment - That last comment was not meant to be a question, really, but I guess I was waiting for you to respond.
              Hide
              tmuras Tomasz Muras added a comment -

              Hi Michael,

              I don't think the issue reported has ever been fixed, here is a code from mod/resource/db/upgradelib.php and latest 2.2.5+ (Build: 20120914) that may cause it (didn't test it recently/again):

                          // current course files
                          // cleanup old path first
                          $path = '/'.trim(trim($path), '/');
                          //...
                          $resource->mainfile     = $path;
               

              Show
              tmuras Tomasz Muras added a comment - Hi Michael, I don't think the issue reported has ever been fixed, here is a code from mod/resource/db/upgradelib.php and latest 2.2.5+ (Build: 20120914) that may cause it (didn't test it recently/again): // current course files // cleanup old path first $path = '/'.trim(trim($path), '/'); //... $resource->mainfile = $path;  
              Hide
              skodak Petr Skoda added a comment -

              We might consider increasing the database field size, since 2.4 we support 1333 char fields, it works in older version too. The only problem is indexing, because MySQL has max limit around 333, but it should be a problem in case of files or resource tables.

              Show
              skodak Petr Skoda added a comment - We might consider increasing the database field size, since 2.4 we support 1333 char fields, it works in older version too. The only problem is indexing, because MySQL has max limit around 333, but it should be a problem in case of files or resource tables.
              Hide
              tmuras Tomasz Muras added a comment -

              Increasing the field size is a very simple fix, why not. To fix this particular issue we need to increase by just 1 character!

              Show
              tmuras Tomasz Muras added a comment - Increasing the field size is a very simple fix, why not. To fix this particular issue we need to increase by just 1 character!
              Hide
              markn Mark Nelson added a comment -

              I had this marked as 'Development in progress' a while ago but have only now managed to get to it, looking into it now.

              Show
              markn Mark Nelson added a comment - I had this marked as 'Development in progress' a while ago but have only now managed to get to it, looking into it now.
              Hide
              markn Mark Nelson added a comment -

              Ok, I am able to replicate this issue right now and will be creating a patch for it shortly.

              Show
              markn Mark Nelson added a comment - Ok, I am able to replicate this issue right now and will be creating a patch for it shortly.
              Hide
              markn Mark Nelson added a comment -

              Ok, so basically if a file resource was added in 1.9 with the maximum 255 characters in length and then an upgrade to 2.x is performed you will receive the error mentioned above. The upgrade process creates a temporary column called 'mainfile' with the length of 255 and saves the filename here, the upgrade then performs some operations and then drops this column. The solution is simply (as Tomasz stated) to increase the length to 256.

              Show
              markn Mark Nelson added a comment - Ok, so basically if a file resource was added in 1.9 with the maximum 255 characters in length and then an upgrade to 2.x is performed you will receive the error mentioned above. The upgrade process creates a temporary column called 'mainfile' with the length of 255 and saves the filename here, the upgrade then performs some operations and then drops this column. The solution is simply (as Tomasz stated) to increase the length to 256.
              Hide
              skodak Petr Skoda added a comment -

              +1

              Show
              skodak Petr Skoda added a comment - +1
              Hide
              skodak Petr Skoda added a comment - - edited

              grrr, sorry for the noise, I just wanted to add +1 from peer review

              Show
              skodak Petr Skoda added a comment - - edited grrr, sorry for the noise, I just wanted to add +1 from peer review
              Hide
              stronk7 Eloy Lafuente (stronk7) added a comment -

              Integrated, thanks!

              Show
              stronk7 Eloy Lafuente (stronk7) added a comment - Integrated, thanks!
              Hide
              fred Frédéric Massart added a comment -

              Test passed. Thanks!

              Show
              fred Frédéric Massart added a comment - Test passed. Thanks!
              Hide
              stronk7 Eloy Lafuente (stronk7) added a comment -

              Surely you will be happy to know that your code is now part of Moodle upstream. Thanks, thanks!

              Closing as fixed, ciao

              Show
              stronk7 Eloy Lafuente (stronk7) added a comment - Surely you will be happy to know that your code is now part of Moodle upstream. Thanks, thanks! Closing as fixed, ciao

                People

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

                  Dates

                  • Created:
                    Updated:
                    Resolved:
                    Fix Release Date:
                    11/Mar/13