Details

    • Type: New Feature
    • Status: Closed
    • Priority: Critical
    • Resolution: Fixed
    • Affects Version/s: 1.9
    • Fix Version/s: 2.0
    • Component/s: Files API
    • Labels:
      None
    • Affected Branches:
      MOODLE_19_STABLE
    • Fixed Branches:
      MOODLE_20_STABLE

      Description

      This issue contains all the subtasks associated with the new file API

      http://docs.moodle.org/en/Development:File_API

        Gliffy Diagrams

        1. fileapi38.patch
          204 kB
          Petr Skoda
        2. fileapi40.patch
          204 kB
          Petr Skoda
        3. fileapi41.patch
          206 kB
          Petr Skoda
        4. fileapi46.patch
          263 kB
          Petr Skoda
        5. new_picker.bmml
          7 kB
          Petr Skoda
        1. new_picker.png
          59 kB

          Issue Links

          1.
          HTML parser Sub-task Closed moodle.com
           
          2.
          Rewrite question export to use the File API Sub-task Closed Dongsheng Cai
           
          3.
          Some DB implementation things Sub-task Closed Petr Skoda
           
          4.
          File storage conversion Assignment Sub-task Closed Petr Skoda
           
          5.
          File storage conversion Blog Sub-task Closed Petr Skoda
           
          6.
          Stored file support in formslib - file upload rewrite Sub-task Closed Petr Skoda
           
          7.
          Limited access to backup files Sub-task Closed Petr Skoda
           
          8.
          general unzipping support Sub-task Closed Petr Skoda
           
          9.
          general zipping support Sub-task Closed Petr Skoda
           
          10.
          File storage conversion Forum Sub-task Closed Petr Skoda
           
          11.
          File storage conversion Glossary Sub-task Closed Petr Skoda
           
          12.
          File storage conversion SCORM Sub-task Closed Petr Skoda
           
          13.
          File storage conversion Resource Sub-task Closed Petr Skoda
           
          14.
          File storage conversion Feedback Sub-task Closed Andreas Grabs
           
          15.
          File storage conversion Label Sub-task Closed Petr Skoda
           
          16.
          File storage conversion Lesson Sub-task Closed moodle.com
           
          17.
          File storage conversion Quiz and Questions Sub-task Closed Tim Hunt
           
          18.
          File storage conversion Wiki Sub-task Closed Dongsheng Cai
           
          19.
          File storage conversion Database Sub-task Closed Sam Hemelryk
           
          20.
          allow deleting of all files attached to context Sub-task Closed Petr Skoda
           
          21.
          rafactor modedit inrastructure to allow access to context from xx_add_instance() module methods Sub-task Closed Petr Skoda
           
          22.
          pass mform to xxx_add_instance() and xxx_update_instance() Sub-task Closed Petr Skoda
           
          23.
          Implement file storage cleanup in cron Sub-task Closed Petr Skoda
           
          24.
          support for module file browsing Sub-task Closed Petr Skoda
           
          25.
          file_rewrite_urls undefined in setup Sub-task Closed Petr Skoda
           
          26.
          user profile embedded files browsing and serving support Sub-task Closed Petr Skoda
           
          27.
          Document the File API on Moodle Docs Sub-task Closed Petr Skoda
           
          28.
          Formslib element: filemanager (non-JS) Sub-task Closed Dongsheng Cai
           
          29.
          Formslib element: filemanager (JS) Sub-task Closed Dongsheng Cai
           
          30.
          Formslib element: htmleditor Sub-task Closed Dongsheng Cai
           
          31.
          Formslib element: file Sub-task Closed Petr Skoda
           
          32.
          Formslib element: filepicker (non-JS) Sub-task Closed Dongsheng Cai
           
          33.
          Formslib element: filepicker (JS) Sub-task Closed Dongsheng Cai
           
          34.
          Formslib element: editor Sub-task Closed Petr Skoda
           
          35.
          Formslib element: url Sub-task Closed Dongsheng Cai
           
          36.
          When a backup completes, send users to the backup file area Sub-task Closed Tim Hunt
           
          37.
          File API code is incomprehensible without comments Sub-task Closed Petr Skoda
           
          38.
          file storage methods to move files out from storage Sub-task Closed Petr Skoda
           
          39.
          Suggested file API function name improvements Sub-task Closed Tim Hunt
           
          40.
          implement embedded and attachment file storage limits in file api and related code Sub-task Closed Dongsheng Cai
           
          41.
          reimplement maintenance mode Sub-task Closed Petr Skoda
           
          42.
          improve embedding of texteditors Sub-task Closed Petr Skoda
           
          43.
          File storage conversion of HTML block Sub-task Closed Dongsheng Cai
           
          44.
          File storage conversion of core Sub-task Closed Sam Hemelryk
           
          45.
          Clean up files associated with a context when the context is deleted Sub-task Closed Sam Hemelryk
           
          46.
          Every file should have a license type Sub-task Closed Dongsheng Cai
           
          47.
          How should we support course-wide or site-wide files created by modules? Sub-task Closed Petr Skoda
           
          48.
          add course->legacyfiles which disables old course files in new courses Sub-task Closed Petr Skoda
           
          49.
          the filemanager element MUST support the main file selection natively in ajax UI Sub-task Closed Dongsheng Cai
           
          50.
          Convert all "import" pages to use filepicker Sub-task Closed Dongsheng Cai
           
          51.
          Implement core renderer to display a read-only filearea Sub-task Closed Petr Skoda
           
          52.
          When displaying a file tree in browse mode, ignore empty folders Sub-task Closed Dongsheng Cai
           
          53.
          Implement stickiness on file picker for repository selection Sub-task Closed Dongsheng Cai
           
          54.
          convert user icons to file storage pool Sub-task Closed Petr Skoda
           
          55.
          convert group icons to file storage pool Sub-task Closed Petr Skoda
           
          56.
          Implement expiry of draft files Sub-task Closed Petr Skoda
           

            Activity

            Hide
            skodak Petr Skoda added a comment -

            sending draft file storage implementation - it is still missing methods and interface definitions...

            Show
            skodak Petr Skoda added a comment - sending draft file storage implementation - it is still missing methods and interface definitions...
            Hide
            skodak Petr Skoda added a comment -

            sending latest patch - the only part that fully works if blog attachments

            Show
            skodak Petr Skoda added a comment - sending latest patch - the only part that fully works if blog attachments
            Hide
            skodak Petr Skoda added a comment -

            user image and group image uploading fixed

            Show
            skodak Petr Skoda added a comment - user image and group image uploading fixed
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            Comments about patch 28:

            1) Once set, it's applied to all the items?

            + if ($item->isLink())

            { + // do not delete symbolic links + $delete = false; + }

            2) If anything goes wrong in migration of files... cannot copy something or so... what happens? Exception (so savepoint is no set?) Should we also provide some feedback of the process? We need the upgrade heartbeat here, I think.

            + if ($result && $oldversion < 2008071803)

            { + /// move all course, backup and other files to new filepool based storage + upgrade_migrate_files(); + /// Main savepoint reached + upgrade_main_savepoint($result, 2008071803); + }

            3) Do we need to copy files from temp to moodledata this way? Cannot the file be moved or copied directly?

            + while (!feof($temp))

            { + $data = fread($temp, 65536); + fwrite($file, $data); + }

            4) Lines like this (in core) assume we are using local file storage. Could we call that, simply, get_file_storage() and then, effectively, the local one (filepool). But that would allow to get others if someday we see new ones. i.e. in general I always would use file_storage, send_file... and so on (without the "local" part) because that could be overwritten by other storages. We should, simply be using "storages".

            + $fs = get_local_file_storage();

            Same with functions like: file_browser-> build_local_file_children(), that "local" naming.... -1 for it.

            5) I haven't looked too much about formslib internals. Really I need to learn a bit before understanding what you've changed there.

            6) One question I think we haven't commented... user & group icons... are they going to be in new filepool or will continue in /user dirs?

            7) I'd define the algorithm to calculate path-hashes within one method somewhere, instead of having it here and there:

            + $pathnamehash = sha1($contextid.$filearea.$itemid.$filepath.'.');
            ....
            + $newrecord->pathnamehash = sha1($newrecord->contextid.$newrecord->filearea.$newrecord->itemid.$newrecord->filepath.$newrecord->filename);
            ....

            8) Repeating 4), this name, abusing of the "local" token... we need to make one glossary about how to name everything.

            + public function create_file_from_localfile($file_record, $fid) {

            9) Not critical now, but it has been difficult to me to guess what are all those file_info_XXXX classes. Need documenting.

            10) Cool stuff! Congrats!

            Going to test it now! B-)

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - Comments about patch 28: 1) Once set, it's applied to all the items? + if ($item->isLink()) { + // do not delete symbolic links + $delete = false; + } 2) If anything goes wrong in migration of files... cannot copy something or so... what happens? Exception (so savepoint is no set?) Should we also provide some feedback of the process? We need the upgrade heartbeat here, I think. + if ($result && $oldversion < 2008071803) { + /// move all course, backup and other files to new filepool based storage + upgrade_migrate_files(); + /// Main savepoint reached + upgrade_main_savepoint($result, 2008071803); + } 3) Do we need to copy files from temp to moodledata this way? Cannot the file be moved or copied directly? + while (!feof($temp)) { + $data = fread($temp, 65536); + fwrite($file, $data); + } 4) Lines like this (in core) assume we are using local file storage. Could we call that, simply, get_file_storage() and then, effectively, the local one (filepool). But that would allow to get others if someday we see new ones. i.e. in general I always would use file_storage, send_file... and so on (without the "local" part) because that could be overwritten by other storages. We should, simply be using "storages". + $fs = get_local_file_storage(); Same with functions like: file_browser-> build_local_file_children(), that "local" naming.... -1 for it. 5) I haven't looked too much about formslib internals. Really I need to learn a bit before understanding what you've changed there. 6) One question I think we haven't commented... user & group icons... are they going to be in new filepool or will continue in /user dirs? 7) I'd define the algorithm to calculate path-hashes within one method somewhere, instead of having it here and there: + $pathnamehash = sha1($contextid.$filearea.$itemid.$filepath.'.'); .... + $newrecord->pathnamehash = sha1($newrecord->contextid.$newrecord->filearea.$newrecord->itemid.$newrecord->filepath.$newrecord->filename); .... 8) Repeating 4), this name, abusing of the "local" token... we need to make one glossary about how to name everything. + public function create_file_from_localfile($file_record, $fid) { 9) Not critical now, but it has been difficult to me to guess what are all those file_info_XXXX classes. Need documenting. 10) Cool stuff! Congrats! Going to test it now! B-)
            Hide
            skodak Petr Skoda added a comment -

            sendig patch nothing new, just merged after latest messaging commit

            Show
            skodak Petr Skoda added a comment - sendig patch nothing new, just merged after latest messaging commit
            Hide
            skodak Petr Skoda added a comment -

            1/ hmmm, that is a bug
            2/ if anything goes wrong the file is not deleted, but the upgrade continues - I agree there should be some feedback
            3/ if we move we can not call that function repeatedly - that is why I copied it
            4/ I like the "local" because it means "not repository" - this could be easily changed any time, my -1
            6/ user and group icons can be in old location or we could move them to pool - not sure, fortunately this is a separate problem
            7/ good idea, I thought about this too, but was lazy
            8/ I still like "local"
            9/ sure
            10/ thanks!

            Show
            skodak Petr Skoda added a comment - 1/ hmmm, that is a bug 2/ if anything goes wrong the file is not deleted, but the upgrade continues - I agree there should be some feedback 3/ if we move we can not call that function repeatedly - that is why I copied it 4/ I like the "local" because it means "not repository" - this could be easily changed any time, my -1 6/ user and group icons can be in old location or we could move them to pool - not sure, fortunately this is a separate problem 7/ good idea, I thought about this too, but was lazy 8/ I still like "local" 9/ sure 10/ thanks!
            Hide
            dougiamas Martin Dougiamas added a comment - - edited

            OK, some random thoughts as I"m going through.

            M1) Cool, it works!

            M2) Some naming/text things ...

            a) Can "filepool" just be "files"? Or to keep it unique and easy to discuss "filedata" (we should avoid new jargon for non-english admins).

            b) In file->fileareas there is "coursefiles" ... should that not probably just be "course" to be consistent?

            c) The file->fileareas are a single word ... won't there be collisions when two plugins have the same name? eg a mod and block ... how about we use the path of the module like mod/fred and blocks/fred as a convention for everybody to follow? and if mod/fred wants multiple fileareas then mod/fred/blah and mod/fred/foo

            d) "Please do not edit, delete, move or change in any way files or subdirectories here!!" --> "This directory contains the content of uploaded files and is controlled by Moodle code. Do not manually move, change or rename any of the files and subdirectories here."

            M3) userid is always null in my table even for new files I add ... shouldn't we record who uploaded the file there? (I know it's "owned" by the module)

            more coming...

            Show
            dougiamas Martin Dougiamas added a comment - - edited OK, some random thoughts as I"m going through. M1) Cool, it works! M2) Some naming/text things ... a) Can "filepool" just be "files"? Or to keep it unique and easy to discuss "filedata" (we should avoid new jargon for non-english admins). b) In file->fileareas there is "coursefiles" ... should that not probably just be "course" to be consistent? c) The file->fileareas are a single word ... won't there be collisions when two plugins have the same name? eg a mod and block ... how about we use the path of the module like mod/fred and blocks/fred as a convention for everybody to follow? and if mod/fred wants multiple fileareas then mod/fred/blah and mod/fred/foo d) "Please do not edit, delete, move or change in any way files or subdirectories here!!" --> "This directory contains the content of uploaded files and is controlled by Moodle code. Do not manually move, change or rename any of the files and subdirectories here." M3) userid is always null in my table even for new files I add ... shouldn't we record who uploaded the file there? (I know it's "owned" by the module) more coming...
            Hide
            dougiamas Martin Dougiamas added a comment -

            M4) Tiny thing in blog/lib.php: this line is not used ... $fb = get_file_browser();

            M5) I agree with Eloy above about removing the "local" names ... by definition everything in the File API is "local" (not and external repository).

            M6) The name get_file_browser() threw me off for a while ... I guessed from the name that it was something that allowed the user to browse files because that word is usually a user activity. So I guessed wrongly that it was something to do with the user browsing files. It's not clear at all at first what the difference is to get_local_file_storage(). Is this like the difference between dmllib and ddlib? Are there clearer names we can come up with?

            M7) I think we should define $CFG->filedir in lib/setup.php rather than use $CFG->dataroot/filepool ... so that admins with big complex servers can put it on another drive etc. it's just more flexible and consistent with libdir etc

            Show
            dougiamas Martin Dougiamas added a comment - M4) Tiny thing in blog/lib.php: this line is not used ... $fb = get_file_browser(); M5) I agree with Eloy above about removing the "local" names ... by definition everything in the File API is "local" (not and external repository). M6) The name get_file_browser() threw me off for a while ... I guessed from the name that it was something that allowed the user to browse files because that word is usually a user activity. So I guessed wrongly that it was something to do with the user browsing files. It's not clear at all at first what the difference is to get_local_file_storage(). Is this like the difference between dmllib and ddlib? Are there clearer names we can come up with? M7) I think we should define $CFG->filedir in lib/setup.php rather than use $CFG->dataroot/filepool ... so that admins with big complex servers can put it on another drive etc. it's just more flexible and consistent with libdir etc
            Hide
            skodak Petr Skoda added a comment -

            M2)

            a) I personally do not care much about the naming scheme, anything consistent is ok for me; I did not spent much time on this

            b) I would vote for "content" instead of "course" or "coursefiles"

            c) file->fileareas is not a problem, we have contextid there -> context level -> we know exact module or block

            f) sure

            M3) during upgrade we do not know the original author, later we know him only sometimes, I did not concentrate on this, going to fix it

            M4) sure

            M5) oki

            M6)

            • file_storage is a low level implementation of file storage, each module may access only own files through this API, it must not access anything outside it's own scope
            • file_browser is an abstraction of all files that are available - they may or may not be stored in file_storage; the primary use will be in file manager UI and image selector/manager in html editor

            M7/ sure

            Show
            skodak Petr Skoda added a comment - M2) a) I personally do not care much about the naming scheme, anything consistent is ok for me; I did not spent much time on this b) I would vote for "content" instead of "course" or "coursefiles" c) file->fileareas is not a problem, we have contextid there -> context level -> we know exact module or block f) sure M3) during upgrade we do not know the original author, later we know him only sometimes, I did not concentrate on this, going to fix it M4) sure M5) oki M6) file_storage is a low level implementation of file storage, each module may access only own files through this API, it must not access anything outside it's own scope file_browser is an abstraction of all files that are available - they may or may not be stored in file_storage; the primary use will be in file manager UI and image selector/manager in html editor M7/ sure
            Hide
            dougiamas Martin Dougiamas added a comment -

            OK cool.

            Just one little bone left for this dog. the filearea names are for self-documentation for humans in table and URLs (I realise the contextid is there and is sufficient for code to work it out). This is just about a coding guide to suggest standard format names for developers of modules to choose.

            I agree with the argument from the chat we just had that full pathnames are too much, so I think we have three things to choose from:

            Choice 1) http://mymoodlesite.edu/pluginfile.php/787/content/1/sample.jpg

            Choice 2) http://mymoodlesite.edu/pluginfile.php/787/assignment_content/1/sample.jpg

            Choice 3) http://mymoodlesite.edu/pluginfile.php/787/assignmentcontent/1/sample.jpg

            My +1 for Choice 3.

            Show
            dougiamas Martin Dougiamas added a comment - OK cool. Just one little bone left for this dog. the filearea names are for self-documentation for humans in table and URLs (I realise the contextid is there and is sufficient for code to work it out). This is just about a coding guide to suggest standard format names for developers of modules to choose. I agree with the argument from the chat we just had that full pathnames are too much, so I think we have three things to choose from: Choice 1) http://mymoodlesite.edu/pluginfile.php/787/content/1/sample.jpg Choice 2) http://mymoodlesite.edu/pluginfile.php/787/assignment_content/1/sample.jpg Choice 3) http://mymoodlesite.edu/pluginfile.php/787/assignmentcontent/1/sample.jpg My +1 for Choice 3.
            Hide
            skodak Petr Skoda added a comment -

            my +1 for Choice 1

            Show
            skodak Petr Skoda added a comment - my +1 for Choice 1
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            My +1 for Choice 2 (better readability)

            hehe :-D

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - My +1 for Choice 2 (better readability) hehe :-D
            Hide
            dougiamas Martin Dougiamas added a comment -

            Penny agrees with me so #3 wins! Next bikeshed please!

            Show
            dougiamas Martin Dougiamas added a comment - Penny agrees with me so #3 wins! Next bikeshed please!
            Hide
            skodak Petr Skoda added a comment -

            hmm, "_" is allowed character in blocks, just name of module may not be enough because names may collide with block
            ok, I give up it is easy to change this anyway if the collective mind changes
            luckily any module author is free to use whatever he/she wants

            Show
            skodak Petr Skoda added a comment - hmm, "_" is allowed character in blocks, just name of module may not be enough because names may collide with block ok, I give up it is easy to change this anyway if the collective mind changes luckily any module author is free to use whatever he/she wants
            Hide
            skodak Petr Skoda added a comment -

            latest version, applies to current HEAD

            Show
            skodak Petr Skoda added a comment - latest version, applies to current HEAD
            Hide
            scyrma Mathieu Petit-Clair added a comment -

            On a brand new checkout, with files used a bit everywhere (some files used in content, some as attachments, some multiple times, etc.), in blog, assignements, forums, summaries...

            Upgrade fails and outputs following messages:

            (mysqli): SELECT * FROM mdl_context WHERE contextlevel = 50 AND instanceid = '1' LIMIT 2
            Warning: sha1() expects at most 2 parameters, 5 given in /home/mathieu/workspace/head_fresh/lib/file/file_storage.php on line 43
            ...
            (mysqli): SELECT 'x' FROM mdl_files WHERE pathnamehash IS NULL LIMIT 1
            Warning: Missing argument 2 for file_storage::get_pathname_hash(), called in /home/mathieu/workspace/head_fresh/lib/file/file_storage.php on line 390 and defined in /home/mathieu/workspace/head_fresh/lib/file/file_storage.php on line 42
            Warning: Missing argument 3 for file_storage::get_pathname_hash(), called in /home/mathieu/workspace/head_fresh/lib/file/file_storage.php on line 390 and defined in /home/mathieu/workspace/head_fresh/lib/file/file_storage.php on line 42
            Warning: Missing argument 4 for file_storage::get_pathname_hash(), called in /home/mathieu/workspace/head_fresh/lib/file/file_storage.php on line 390 and defined in /home/mathieu/workspace/head_fresh/lib/file/file_storage.php on line 42
            Warning: Missing argument 5 for file_storage::get_pathname_hash(), called in /home/mathieu/workspace/head_fresh/lib/file/file_storage.php on line 390 and defined in /home/mathieu/workspace/head_fresh/lib/file/file_storage.php on line 42
            Notice: Undefined variable: filearea in /home/mathieu/workspace/head_fresh/lib/file/file_storage.php on line 43
            Notice: Undefined variable: itemid in /home/mathieu/workspace/head_fresh/lib/file/file_storage.php on line 43
            Notice: Undefined variable: filepath in /home/mathieu/workspace/head_fresh/lib/file/file_storage.php on line 43
            Notice: Undefined variable: filename in /home/mathieu/workspace/head_fresh/lib/file/file_storage.php on line 43
            Warning: sha1() expects at most 2 parameters, 5 given in /home/mathieu/workspace/head_fresh/lib/file/file_storage.php on line 43
            ...
            Query: INSERT INTO mdl_files (contextid,filearea,itemid,filepath,filename,timecreated,timemodified,mimetype,userid,filesize,contenthash,pathnamehash) VALUES('2','course_content',0,'/','prof_vieux.jpeg',1217302754,1217302754,'image/jpeg',NULL,2479,'f6b5dbc5a7a6ba41ce809a9f234c849befac9af0',NULL) failed. Column 'pathnamehash' cannot be null
            ....
            (mysqli): INSERT INTO mdl_files_cleanup (contenthash) VALUES('f6b5dbc5a7a6ba41ce809a9f234c849befac9af0')

            Can not create file "2/course_content/0///prof_vieux.jpeg"

            1. line 402 of lib/file/file_storage.php: stored_file_creation_exception thrown
            2. line 161 of lib/db/upgradelib.php: call to file_storage->create_file_from_pathname()
            3. line 70 of lib/db/upgradelib.php: call to upgrade_migrate_files_course()
            4. line 487 of lib/db/upgrade.php: call to upgrade_migrate_files()
            5. line 326 of admin/index.php: call to xmldb_main_upgrade()

            The file 'prof_vieux.jpeg' is in the site's files, and used multiple times in different summaries.

            Show
            scyrma Mathieu Petit-Clair added a comment - On a brand new checkout, with files used a bit everywhere (some files used in content, some as attachments, some multiple times, etc.), in blog, assignements, forums, summaries... Upgrade fails and outputs following messages: (mysqli): SELECT * FROM mdl_context WHERE contextlevel = 50 AND instanceid = '1' LIMIT 2 Warning: sha1() expects at most 2 parameters, 5 given in /home/mathieu/workspace/head_fresh/lib/file/file_storage.php on line 43 ... (mysqli): SELECT 'x' FROM mdl_files WHERE pathnamehash IS NULL LIMIT 1 Warning: Missing argument 2 for file_storage::get_pathname_hash(), called in /home/mathieu/workspace/head_fresh/lib/file/file_storage.php on line 390 and defined in /home/mathieu/workspace/head_fresh/lib/file/file_storage.php on line 42 Warning: Missing argument 3 for file_storage::get_pathname_hash(), called in /home/mathieu/workspace/head_fresh/lib/file/file_storage.php on line 390 and defined in /home/mathieu/workspace/head_fresh/lib/file/file_storage.php on line 42 Warning: Missing argument 4 for file_storage::get_pathname_hash(), called in /home/mathieu/workspace/head_fresh/lib/file/file_storage.php on line 390 and defined in /home/mathieu/workspace/head_fresh/lib/file/file_storage.php on line 42 Warning: Missing argument 5 for file_storage::get_pathname_hash(), called in /home/mathieu/workspace/head_fresh/lib/file/file_storage.php on line 390 and defined in /home/mathieu/workspace/head_fresh/lib/file/file_storage.php on line 42 Notice: Undefined variable: filearea in /home/mathieu/workspace/head_fresh/lib/file/file_storage.php on line 43 Notice: Undefined variable: itemid in /home/mathieu/workspace/head_fresh/lib/file/file_storage.php on line 43 Notice: Undefined variable: filepath in /home/mathieu/workspace/head_fresh/lib/file/file_storage.php on line 43 Notice: Undefined variable: filename in /home/mathieu/workspace/head_fresh/lib/file/file_storage.php on line 43 Warning: sha1() expects at most 2 parameters, 5 given in /home/mathieu/workspace/head_fresh/lib/file/file_storage.php on line 43 ... Query: INSERT INTO mdl_files (contextid,filearea,itemid,filepath,filename,timecreated,timemodified,mimetype,userid,filesize,contenthash,pathnamehash) VALUES('2','course_content',0,'/','prof_vieux.jpeg',1217302754,1217302754,'image/jpeg',NULL,2479,'f6b5dbc5a7a6ba41ce809a9f234c849befac9af0',NULL) failed. Column 'pathnamehash' cannot be null .... (mysqli): INSERT INTO mdl_files_cleanup (contenthash) VALUES('f6b5dbc5a7a6ba41ce809a9f234c849befac9af0') Can not create file "2/course_content/0///prof_vieux.jpeg" line 402 of lib/file/file_storage.php: stored_file_creation_exception thrown line 161 of lib/db/upgradelib.php: call to file_storage->create_file_from_pathname() line 70 of lib/db/upgradelib.php: call to upgrade_migrate_files_course() line 487 of lib/db/upgrade.php: call to upgrade_migrate_files() line 326 of admin/index.php: call to xmldb_main_upgrade() The file 'prof_vieux.jpeg' is in the site's files, and used multiple times in different summaries.
            Hide
            skodak Petr Skoda added a comment -

            fixed wrong refactoring, should work again

            Show
            skodak Petr Skoda added a comment - fixed wrong refactoring, should work again
            Hide
            scyrma Mathieu Petit-Clair added a comment -

            Hi Petr, on the same installation (reverted to pre-previous update database), most things (forum post, blogs post - both in text and attachements - summary text for course and forum) work nicely now after the upgrade.

            The only problem I see now (though it might just be because you haven't attacked this yet) is with assignments. In both "advanced uploading of files" and "upload a single file" types, uploaded files are not available anymore. I get a "filenotfound" error.

            More testing on the way...

            Show
            scyrma Mathieu Petit-Clair added a comment - Hi Petr, on the same installation (reverted to pre-previous update database), most things (forum post, blogs post - both in text and attachements - summary text for course and forum) work nicely now after the upgrade. The only problem I see now (though it might just be because you haven't attacked this yet) is with assignments. In both "advanced uploading of files" and "upload a single file" types, uploaded files are not available anymore. I get a "filenotfound" error. More testing on the way...
            Hide
            skodak Petr Skoda added a comment - - edited

            Modules (including assignment) are not converted yet

            Show
            skodak Petr Skoda added a comment - - edited Modules (including assignment) are not converted yet
            Hide
            skodak Petr Skoda added a comment -

            sending latest patch, some bugfixing and converted assignment mod

            Show
            skodak Petr Skoda added a comment - sending latest patch, some bugfixing and converted assignment mod
            Hide
            skodak Petr Skoda added a comment -

            patch in cvs

            my current plan is:
            1/ improve upgrade code (concurrent upgrade prevention, interruptible upgrades, etc.)
            2/ review PARAM_ handling
            3/ review clean_param() and filename handling in general
            4/ basic zipping/unzipping support
            5/ continue with file storage conversion

            Show
            skodak Petr Skoda added a comment - patch in cvs my current plan is: 1/ improve upgrade code (concurrent upgrade prevention, interruptible upgrades, etc.) 2/ review PARAM_ handling 3/ review clean_param() and filename handling in general 4/ basic zipping/unzipping support 5/ continue with file storage conversion
            Hide
            dongsheng Dongsheng Cai added a comment -

            Petr, I have several questions to ask you.

            1. Repository file picker as part of a HTML editor
            In this case, I need to return the url of the file in filepool, but I could find a function to return url. Should I build url by myself?

            2. Repository file picker as part of a Moodleform with Javascript
            What is file reference id exactly? The unique ID of each entry in mdl_files table?

            Show
            dongsheng Dongsheng Cai added a comment - Petr, I have several questions to ask you. 1. Repository file picker as part of a HTML editor In this case, I need to return the url of the file in filepool, but I could find a function to return url. Should I build url by myself? 2. Repository file picker as part of a Moodleform with Javascript What is file reference id exactly? The unique ID of each entry in mdl_files table?
            Hide
            dongsheng Dongsheng Cai added a comment -

            Hi, Petr, how to get file_record id from file_info class?

            I added 'id'=>$this->lf->get_id() in file_info->get_params() function, is that OK?

            Show
            dongsheng Dongsheng Cai added a comment - Hi, Petr, how to get file_record id from file_info class? I added 'id'=>$this->lf->get_id() in file_info->get_params() function, is that OK?
            Hide
            skodak Petr Skoda added a comment -

            hmm, the only problem is that other infos may not return it, maybe we should return 'id'=>null there

            Show
            skodak Petr Skoda added a comment - hmm, the only problem is that other infos may not return it, maybe we should return 'id'=>null there
            Hide
            skodak Petr Skoda added a comment -

            alternative would be to not use file id, but instead use draft area item id.

            Show
            skodak Petr Skoda added a comment - alternative would be to not use file id, but instead use draft area item id.
            Hide
            skodak Petr Skoda added a comment -

            the more I think about this the less I like the passing of file id around because in the end we will have to validate that user may access it (== is located in his draft file area).

            Show
            skodak Petr Skoda added a comment - the more I think about this the less I like the passing of file id around because in the end we will have to validate that user may access it (== is located in his draft file area).
            Hide
            dongsheng Dongsheng Cai added a comment -

            Can you have a look at this: http://cvs.moodle.org/moodle/repository/lib.php?r1=1.21&r2=1.22
            It seems I need to assign itemid value before I call create_file_from_pathname
            This id will be used in moodleform.

            Show
            dongsheng Dongsheng Cai added a comment - Can you have a look at this: http://cvs.moodle.org/moodle/repository/lib.php?r1=1.21&r2=1.22 It seems I need to assign itemid value before I call create_file_from_pathname This id will be used in moodleform.
            Show
            ralfh Ralf Hilgenstock added a comment - Add a tooltip to http://moodle.de/file.php/XX/filename.pdf?forcedownload=1
            Hide
            mattgibson Matt Gibson added a comment -

            Not sure if this fits better on a sub task, but I've been making courses with images in the labels and html pages and I've found that the links don't get maintained when I backup the course and restore it again as a fresh copy. There really needs to be a way for backup/restore to work with the file system to make sure that the necessary files are included within the backup and are that links are updated on unzipping.

            Show
            mattgibson Matt Gibson added a comment - Not sure if this fits better on a sub task, but I've been making courses with images in the labels and html pages and I've found that the links don't get maintained when I backup the course and restore it again as a fresh copy. There really needs to be a way for backup/restore to work with the file system to make sure that the necessary files are included within the backup and are that links are updated on unzipping.
            Hide
            dougiamas Martin Dougiamas added a comment -

            Assigning to Petr to take over managing this project.

            Items need to be

            • completed with anything not in this list
            • re-sorted into order
            • assigned to new people if necessary

            Thanks!

            Show
            dougiamas Martin Dougiamas added a comment - Assigning to Petr to take over managing this project. Items need to be completed with anything not in this list re-sorted into order assigned to new people if necessary Thanks!
            Hide
            nadavkav Nadav Kavalerchik added a comment -

            i am not sure if i am in the right place, so please excuse me if i am not.

            how about adding Meta data to files ? (like IMS meta data : http://www.imsproject.org/metadata/)
            and allowing to search it, when looking for files in the course or the entire site.

            maybe have a new field that holds the meta data schema type
            and another field to hold xml data itself.
            and s popup form that parses the schema and enables the user
            to fill in the schema's fields and save them in the DB with the file.

            Show
            nadavkav Nadav Kavalerchik added a comment - i am not sure if i am in the right place, so please excuse me if i am not. how about adding Meta data to files ? (like IMS meta data : http://www.imsproject.org/metadata/ ) and allowing to search it, when looking for files in the course or the entire site. maybe have a new field that holds the meta data schema type and another field to hold xml data itself. and s popup form that parses the schema and enables the user to fill in the schema's fields and save them in the DB with the file.
            Hide
            skodak Petr Skoda added a comment -

            Added UI Mockup: <new_picker>

            Show
            skodak Petr Skoda added a comment - Added UI Mockup: <new_picker>
            Hide
            dougiamas Martin Dougiamas added a comment -

            Just a note, plans for file management in and out of editor : http://docs.moodle.org/en/Development:Editor_file_management

            Show
            dougiamas Martin Dougiamas added a comment - Just a note, plans for file management in and out of editor : http://docs.moodle.org/en/Development:Editor_file_management
            Hide
            danielneis Daniel Neis added a comment -

            Hello,

            this file API is related to the current moodle/files/index.php ? Maybe it is a good chance to start using HTTP Etags instead of the current cache control that is done in send_file function to correct bugs like MDL-20042 .

            Show
            danielneis Daniel Neis added a comment - Hello, this file API is related to the current moodle/files/index.php ? Maybe it is a good chance to start using HTTP Etags instead of the current cache control that is done in send_file function to correct bugs like MDL-20042 .
            Hide
            dougiamas Martin Dougiamas added a comment -

            I'm dropping the priority slightly on the meta bug because essentially files stuff is all working and the fact this is still open should not block 2.0 now. Individual subtasks can be blockers if necessary.

            Show
            dougiamas Martin Dougiamas added a comment - I'm dropping the priority slightly on the meta bug because essentially files stuff is all working and the fact this is still open should not block 2.0 now. Individual subtasks can be blockers if necessary.
            Hide
            skodak Petr Skoda added a comment -

            All subtasks are finished. Thanks everybody.

            Petr

            Show
            skodak Petr Skoda added a comment - All subtasks are finished. Thanks everybody. Petr

              People

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

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Fix Release Date:
                  24/Nov/10