MDL-42016 repository: Simplify API to sync external files
authorMarina Glancy <marina@moodle.com>
Fri, 13 Sep 2013 09:37:26 +0000 (19:37 +1000)
committerMarina Glancy <marina@moodle.com>
Sun, 6 Oct 2013 07:05:09 +0000 (18:05 +1100)
Too many functions, too different parameters, unnecessary DB queries.
All repositories developed for Moodle 2.3-2.5 will continue to work.

Also get rid of DB field files_reference.lifetime, it is not used by
anybody except repository itself.

lib/db/install.xml
lib/db/upgrade.php
lib/filestorage/file_storage.php
lib/filestorage/stored_file.php
lib/phpunit/classes/util.php
lib/setuplib.php
lib/upgrade.txt
repository/lib.php
repository/upgrade.txt
version.php

index f8f65cc..c99243a 100644 (file)
@@ -1,5 +1,5 @@
 <?xml version="1.0" encoding="UTF-8" ?>
-<XMLDB PATH="lib/db" VERSION="20130927" COMMENT="XMLDB file for core Moodle tables"
+<XMLDB PATH="lib/db" VERSION="20131004" COMMENT="XMLDB file for core Moodle tables"
     xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
     xsi:noNamespaceSchemaLocation="../../lib/xmldb/xmldb.xsd"
 >
         <FIELD NAME="id" TYPE="int" LENGTH="10" NOTNULL="true" SEQUENCE="true"/>
         <FIELD NAME="repositoryid" TYPE="int" LENGTH="10" NOTNULL="true" SEQUENCE="false"/>
         <FIELD NAME="lastsync" TYPE="int" LENGTH="10" NOTNULL="false" SEQUENCE="false" COMMENT="Last time the proxy file was synced with repository"/>
-        <FIELD NAME="lifetime" TYPE="int" LENGTH="10" NOTNULL="false" SEQUENCE="false" COMMENT="How often do we have to sync proxy file with repository"/>
         <FIELD NAME="reference" TYPE="text" NOTNULL="false" SEQUENCE="false" COMMENT="Identification of the external file. Repository plugins are interpreting it to locate the external file."/>
         <FIELD NAME="referencehash" TYPE="char" LENGTH="40" NOTNULL="true" SEQUENCE="false" COMMENT="Internal implementation detail, contains SHA1 hash of the reference field. Can be indexed and used for comparison. Not meant to be used by a non-core code."/>
       </FIELDS>
index 5409977..f912273 100644 (file)
@@ -2576,5 +2576,20 @@ function xmldb_main_upgrade($oldversion) {
         upgrade_main_savepoint(true, 2013092700.01);
     }
 
+    if ($oldversion < 2013100400.02) {
+
+        // Define field lifetime to be dropped from files_reference.
+        $table = new xmldb_table('files_reference');
+        $field = new xmldb_field('lifetime');
+
+        // Conditionally launch drop field lifetime.
+        if ($dbman->field_exists($table, $field)) {
+            $dbman->drop_field($table, $field);
+        }
+
+        // Main savepoint reached.
+        upgrade_main_savepoint(true, 2013100400.02);
+    }
+
     return true;
 }
index f05a70a..0ec9c2f 100644 (file)
@@ -1031,7 +1031,7 @@ class file_storage {
                 }
             }
 
-            if ($key == 'referencefileid' or $key == 'referencelastsync' or $key == 'referencelifetime') {
+            if ($key == 'referencefileid' or $key == 'referencelastsync') {
                 $value = clean_param($value, PARAM_INT);
             }
 
@@ -2102,10 +2102,7 @@ class file_storage {
 
         $now = time();
         foreach ($rs as $record) {
-            require_once($CFG->dirroot.'/repository/lib.php');
-            $repo = repository::get_instance($record->repositoryid);
-            $lifetime = $repo->get_reference_file_lifetime($reference);
-            $this->update_references($record->id, $now, $lifetime,
+            $this->update_references($record->id, $now, null,
                     $storedfile->get_contenthash(), $storedfile->get_filesize(), 0);
         }
         $rs->close();
@@ -2238,8 +2235,7 @@ class file_storage {
 
         $referencefields = array('repositoryid' => 'repositoryid',
             'reference' => 'reference',
-            'lastsync' => 'referencelastsync',
-            'lifetime' => 'referencelifetime');
+            'lastsync' => 'referencelastsync');
 
         // id is specifically named to prevent overlaping between the two tables.
         $fields = array();
@@ -2259,10 +2255,12 @@ class file_storage {
      * Returns the id of the record in {files_reference} that matches the passed repositoryid and reference
      *
      * If the record already exists, its id is returned. If there is no such record yet,
-     * new one is created (using the lastsync and lifetime provided, too) and its id is returned.
+     * new one is created (using the lastsync provided, too) and its id is returned.
      *
      * @param int $repositoryid
      * @param string $reference
+     * @param int $lastsync
+     * @param int $lifetime argument not used any more
      * @return int
      */
     private function get_or_create_referencefileid($repositoryid, $reference, $lastsync = null, $lifetime = null) {
@@ -2281,8 +2279,7 @@ class file_storage {
                 'repositoryid'  => $repositoryid,
                 'reference'     => $reference,
                 'referencehash' => sha1($reference),
-                'lastsync'      => $lastsync,
-                'lifetime'      => $lifetime));
+                'lastsync'      => $lastsync));
         } catch (dml_exception $e) {
             // if inserting the new record failed, chances are that the race condition has just
             // occured and the unique index did not allow to create the second record with the same
@@ -2316,12 +2313,11 @@ class file_storage {
      *
      * This function is called after synchronisation of an external file and updates the
      * contenthash, filesize and status of all files that reference this external file
-     * as well as time last synchronised and sync lifetime (how long we don't need to call
-     * synchronisation for this reference).
+     * as well as time last synchronised.
      *
      * @param int $referencefileid
      * @param int $lastsync
-     * @param int $lifetime
+     * @param int $lifetime argument not used any more, liefetime is returned by repository
      * @param string $contenthash
      * @param int $filesize
      * @param int $status 0 if ok or 666 if source is missing
@@ -2330,20 +2326,17 @@ class file_storage {
         global $DB;
         $referencefileid = clean_param($referencefileid, PARAM_INT);
         $lastsync = clean_param($lastsync, PARAM_INT);
-        $lifetime = clean_param($lifetime, PARAM_INT);
         validate_param($contenthash, PARAM_TEXT, NULL_NOT_ALLOWED);
         $filesize = clean_param($filesize, PARAM_INT);
         $status = clean_param($status, PARAM_INT);
         $params = array('contenthash' => $contenthash,
                     'filesize' => $filesize,
                     'status' => $status,
-                    'referencefileid' => $referencefileid,
-                    'lastsync' => $lastsync,
-                    'lifetime' => $lifetime);
+                    'referencefileid' => $referencefileid);
         $DB->execute('UPDATE {files} SET contenthash = :contenthash, filesize = :filesize,
             status = :status
             WHERE referencefileid = :referencefileid', $params);
-        $data = array('id' => $referencefileid, 'lastsync' => $lastsync, 'lifetime' => $lifetime);
+        $data = array('id' => $referencefileid, 'lastsync' => $lastsync);
         $DB->update_record('files_reference', (object)$data);
     }
 }
index de04fbb..b5f5b33 100644 (file)
@@ -73,7 +73,7 @@ class stored_file {
             $this->repository = null;
         }
         // make sure all reference fields exist in file_record even when it is not a reference
-        foreach (array('referencelastsync', 'referencelifetime', 'referencefileid', 'reference', 'repositoryid') as $key) {
+        foreach (array('referencelastsync', 'referencefileid', 'reference', 'repositoryid') as $key) {
             if (empty($this->file_record->$key)) {
                 $this->file_record->$key = null;
             }
@@ -296,7 +296,6 @@ class stored_file {
         $this->file_record->reference = null;
         $this->file_record->referencefileid = null;
         $this->file_record->referencelastsync = null;
-        $this->file_record->referencelifetime = null;
     }
 
     /**
@@ -620,10 +619,8 @@ class stored_file {
      * Updates contenthash and filesize
      */
     public function sync_external_file() {
-        global $CFG;
-        if (!empty($this->file_record->referencefileid)) {
-            require_once($CFG->dirroot.'/repository/lib.php');
-            repository::sync_external_file($this);
+        if (!empty($this->repository)) {
+            $this->repository->sync_reference($this);
         }
     }
 
@@ -902,11 +899,26 @@ class stored_file {
     }
 
     /**
-     * Get reference last sync time
+     * Get reference life time (in seconds) after which sync is required
+     *
+     * This data is no longer stored in DB or returned by repository. Each
+     * repository should decide by itself when to synchronise the references.
+     *
+     * @deprecated since 2.6
+     * @see repository::sync_reference()
      * @return int
      */
     public function get_referencelifetime() {
-        return $this->file_record->referencelifetime;
+        debugging('Function stored_file::get_referencelifetime() is deprecated.', DEBUG_DEVELOPER);
+        if ($this->repository) {
+            if (method_exists($this->repository, 'get_reference_file_lifetime')) {
+                return $this->repository->get_reference_file_lifetime($this->get_reference());
+            } else {
+                return 24 * 60 * 60;
+            }
+        } else {
+            return 0;
+        }
     }
     /**
      * Returns file reference
@@ -932,31 +944,28 @@ class stored_file {
      * We update contenthash, filesize and status in files table if changed
      * and we always update lastsync in files_reference table
      *
-     * @param string $contenthash
-     * @param int $filesize
-     * @param int $status
-     * @param int $lifetime the life time of this synchronisation results
+     * @param null|string $contenthash if set to null contenthash is not changed
+     * @param int $filesize new size of the file
+     * @param int $status new status of the file (0 means OK, 666 - source missing)
      */
-    public function set_synchronized($contenthash, $filesize, $status = 0, $lifetime = null) {
-        global $DB;
+    public function set_synchronized($contenthash, $filesize, $status = 0) {
         if (!$this->is_external_file()) {
             return;
         }
         $now = time();
+        if ($contenthash === null) {
+            $contenthash = $this->file_record->contenthash;
+        }
         if ($contenthash != $this->file_record->contenthash) {
             $oldcontenthash = $this->file_record->contenthash;
         }
-        if ($lifetime === null) {
-            $lifetime = $this->file_record->referencelifetime;
-        }
         // this will update all entries in {files} that have the same filereference id
-        $this->fs->update_references($this->file_record->referencefileid, $now, $lifetime, $contenthash, $filesize, $status);
+        $this->fs->update_references($this->file_record->referencefileid, $now, null, $contenthash, $filesize, $status);
         // we don't need to call update() for this object, just set the values of changed fields
         $this->file_record->contenthash = $contenthash;
         $this->file_record->filesize = $filesize;
         $this->file_record->status = $status;
         $this->file_record->referencelastsync = $now;
-        $this->file_record->referencelifetime = $lifetime;
         if (isset($oldcontenthash)) {
             $this->fs->deleted_file_cleanup($oldcontenthash);
         }
@@ -964,11 +973,9 @@ class stored_file {
 
     /**
      * Sets the error status for a file that could not be synchronised
-     *
-     * @param int $lifetime the life time of this synchronisation results
      */
-    public function set_missingsource($lifetime = null) {
-        $this->set_synchronized($this->get_contenthash(), $this->get_filesize(), 666, $lifetime);
+    public function set_missingsource() {
+        $this->set_synchronized($this->file_record->contenthash, $this->file_record->filesize, 666);
     }
 
     /**
index 7461fb4..7c6fd2f 100644 (file)
@@ -201,9 +201,6 @@ class phpunit_util extends testing_util {
         events_get_handlers('reset');
         core_text::reset_caches();
         get_message_processors(false, true);
-        if (class_exists('repository')) {
-            repository::reset_caches();
-        }
         filter_manager::reset_caches();
         //TODO MDL-25290: add more resets here and probably refactor them to new core function
 
index fcbad37..2e79285 100644 (file)
@@ -1161,7 +1161,7 @@ function disable_output_buffering() {
  */
 function redirect_if_major_upgrade_required() {
     global $CFG;
-    $lastmajordbchanges = 2013091000.03;
+    $lastmajordbchanges = 2013100400.02;
     if (empty($CFG->version) or (float)$CFG->version < $lastmajordbchanges or
             during_initial_install() or !empty($CFG->adminsetuppending)) {
         try {
index acb06eb..0f9ee60 100644 (file)
@@ -103,6 +103,13 @@ Navigation:
 Files and repositories:
     * stored_file::replace_content_with()   -> stored_file::replace_file_with()
     * stored_file::set_filesize()           -> stored_file::replace_file_with()
+    * stored_file::get_referencelifetime()  -> (no replacement)
+    * repository::sync_external_file()      -> see repository::sync_reference()
+    * repository::get_file_by_reference()   -> repository::sync_reference()
+    * repository::
+          get_reference_file_lifetime()     -> (no replacement)
+    * repository::sync_individual_file()    -> (no replacement)
+    * repository::reset_caches()            -> (no replacement)
 
 Calendar:
     * add_event()                           -> calendar_event::create()
index 43ef70d..6a7669f 100644 (file)
@@ -559,6 +559,35 @@ abstract class repository implements cacheable_object {
         $this->super_called = true;
     }
 
+    /**
+     * Magic method for non-existing (usually deprecated) class methods.
+     *
+     * @param string $name
+     * @param array $arguments
+     * @return mixed
+     * @throws coding_exception
+     */
+    public function __call($name, $arguments) {
+        if ($name === 'sync_individual_file') {
+            // Method repository::sync_individual_file() was deprecated in Moodle 2.6.
+            // See repository::sync_reference().
+            debugging('Function repository::sync_individual_file() is deprecated.', DEBUG_DEVELOPER);
+            return true;
+        } else if ($name === 'get_file_by_reference') {
+            // Method repository::get_file_by_reference() was deprecated in Moodle 2.6.
+            // See repository::sync_reference().
+            debugging('Function repository::get_file_by_reference() is deprecated.', DEBUG_DEVELOPER);
+            return null;
+        } else if ($name === 'get_reference_file_lifetime') {
+            // Method repository::get_file_by_reference() was deprecated in Moodle 2.6.
+            // See repository::sync_reference().
+            debugging('Function repository::get_reference_file_lifetime() is deprecated.', DEBUG_DEVELOPER);
+            return 24 * 60 * 60;
+        } else {
+            throw new coding_exception('Tried to call unknown method '.get_class($this).'::'.$name);
+        }
+    }
+
     /**
      * Get repository instance using repository id
      *
@@ -1276,27 +1305,6 @@ abstract class repository implements cacheable_object {
         }
     }
 
-    /**
-     * Return reference file life time
-     *
-     * @param string $ref
-     * @return int
-     */
-    public function get_reference_file_lifetime($ref) {
-        // One day
-        return 60 * 60 * 24;
-    }
-
-    /**
-     * Decide whether or not the file should be synced
-     *
-     * @param stored_file $storedfile
-     * @return bool
-     */
-    public function sync_individual_file(stored_file $storedfile) {
-        return true;
-    }
-
     /**
      * Return human readable reference information
      *
@@ -1345,50 +1353,6 @@ abstract class repository implements cacheable_object {
     public function cache_file_by_reference($reference, $storedfile) {
     }
 
-    /**
-     * Returns information about file in this repository by reference
-     *
-     * This function must be implemented for repositories supporting FILE_REFERENCE, it is called
-     * for existing aliases when the lifetime of the previous syncronisation has expired.
-     *
-     * Returns null if file not found or is not readable or timeout occured during request.
-     * Note that this function may be run for EACH file that needs to be synchronised at the
-     * moment. If anything is being downloaded or requested from external sources there
-     * should be a small timeout. The synchronisation is performed to update the size of
-     * the file and/or to update image and re-generated image preview. There is nothing
-     * fatal if syncronisation fails but it is fatal if syncronisation takes too long
-     * and hangs the script generating a page.
-     *
-     * If get_file_by_reference() returns filesize just the record in {files} table is being updated.
-     * If filepath, handle or content are returned - the file is also stored in moodle filepool
-     * (recommended for images to generate the thumbnails). For non-image files it is not
-     * recommended to download them to moodle during syncronisation since it may take
-     * unnecessary long time.
-     *
-     * @param stdClass $reference record from DB table {files_reference}
-     * @return stdClass|null contains one of the following:
-     *   - 'filesize' and optionally 'contenthash'
-     *   - 'filepath'
-     *   - 'handle'
-     *   - 'content'
-     */
-    public function get_file_by_reference($reference) {
-        if ($this->has_moodle_files() && isset($reference->reference)) {
-            $fs = get_file_storage();
-            $params = file_storage::unpack_reference($reference->reference, true);
-            if (!is_array($params) || !($storedfile = $fs->get_file($params['contextid'],
-                    $params['component'], $params['filearea'], $params['itemid'], $params['filepath'],
-                    $params['filename']))) {
-                return null;
-            }
-            return (object)array(
-                'contenthash' => $storedfile->get_contenthash(),
-                'filesize'    => $storedfile->get_filesize()
-            );
-        }
-        return null;
-    }
-
     /**
      * Return the source information
      *
@@ -1771,7 +1735,7 @@ abstract class repository implements cacheable_object {
 
     /**
      * Downloads the file from external repository and saves it in moodle filepool.
-     * This function is different from {@link repository::sync_external_file()} because it has
+     * This function is different from {@link repository::sync_reference()} because it has
      * bigger request timeout and always downloads the content.
      *
      * This function is invoked when we try to unlink the file from the source and convert
@@ -1809,10 +1773,7 @@ abstract class repository implements cacheable_object {
                 // content for the file that was not actually downloaded
                 $contentexists = false;
             }
-            $now = time();
-            if ($file->get_referencelastsync() + $file->get_referencelifetime() >= $now &&
-                        !$file->get_status() &&
-                        $contentexists) {
+            if (!$file->get_status() && $contentexists) {
                 // we already have the content in moodle filepool and it was synchronised recently.
                 // Repositories may overwrite it if they want to force synchronisation anyway!
                 return;
@@ -1823,8 +1784,7 @@ abstract class repository implements cacheable_object {
                     if (isset($fileinfo['path'])) {
                         list($contenthash, $filesize, $newfile) = $fs->add_file_to_pool($fileinfo['path']);
                         // set this file and other similar aliases synchronised
-                        $lifetime = $this->get_reference_file_lifetime($file->get_reference());
-                        $file->set_synchronized($contenthash, $filesize, 0, $lifetime);
+                        $file->set_synchronized($contenthash, $filesize);
                     } else {
                         throw new moodle_exception('errorwhiledownload', 'repository', '', '');
                     }
@@ -2724,67 +2684,150 @@ abstract class repository implements cacheable_object {
     }
 
     /**
-     * Called from phpunit between tests, resets whatever was cached
+     * Method deprecated, cache is handled by MUC now.
+     * @deprecated since 2.6
      */
     public static function reset_caches() {
-        self::sync_external_file(null, true);
+        debugging('Function repository::reset_caches() is deprecated.', DEBUG_DEVELOPER);
     }
 
     /**
-     * Performs synchronisation of reference to an external file if the previous one has expired.
-     *
-     * @param stored_file $file
-     * @param bool $resetsynchistory whether to reset all history of sync (used by phpunit)
-     * @return bool success
+     * Method deprecated
+     * @deprecated since 2.6
+     * @see repository::sync_reference()
      */
     public static function sync_external_file($file, $resetsynchistory = false) {
-        global $DB;
-        // TODO MDL-25290 static should be replaced with MUC code.
-        static $synchronized = array();
-        if ($resetsynchistory) {
-            $synchronized = array();
+        debugging('Function repository::sync_external_file() is deprecated.',
+                DEBUG_DEVELOPER);
+        if ($resetsynchistory || !$file || !$file->get_repository_id() ||
+                !($repository = self::get_repository_by_id($file->get_repository_id(), SYSCONTEXTID))) {
+            return false;
         }
+        return $repository->sync_reference($file);
+    }
 
-        $fs = get_file_storage();
-
-        if (!$file || !$file->get_referencefileid()) {
+    /**
+     * Performs synchronisation of an external file if the previous one has expired.
+     *
+     * This function must be implemented for external repositories supporting
+     * FILE_REFERENCE, it is called for existing aliases when their filesize,
+     * contenthash or timemodified are requested. It is not called for internal
+     * repositories (see {@link repository::has_moodle_files()}), references to
+     * internal files are updated immediately when source is modified.
+     *
+     * Referenced files may optionally keep their content in Moodle filepool (for
+     * thumbnail generation or to be able to serve cached copy). In this
+     * case both contenthash and filesize need to be synchronized. Otherwise repositories
+     * should use contenthash of empty file and correct filesize in bytes.
+     *
+     * Note that this function may be run for EACH file that needs to be synchronised at the
+     * moment. If anything is being downloaded or requested from external sources there
+     * should be a small timeout. The synchronisation is performed to update the size of
+     * the file and/or to update image and re-generated image preview. There is nothing
+     * fatal if syncronisation fails but it is fatal if syncronisation takes too long
+     * and hangs the script generating a page.
+     *
+     * Note: If you wish to call $file->get_filesize(), $file->get_contenthash() or
+     * $file->get_timemodified() make sure that recursion does not happen.
+     *
+     * Called from {@link stored_file::sync_external_file()}
+     *
+     * @uses stored_file::set_missingsource()
+     * @uses stored_file::set_synchronized()
+     * @param stored_file $file
+     * @return bool false when file does not need synchronisation, true if it was synchronised
+     */
+    public function sync_reference(stored_file $file) {
+        if ($file->get_repository_id() != $this->id) {
+            // This should not really happen because the function can be called from stored_file only.
             return false;
         }
-        if (array_key_exists($file->get_id(), $synchronized)) {
-            return $synchronized[$file->get_id()];
+
+        if ($this->has_moodle_files()) {
+            // References to local files need to be synchronised only once.
+            // Later they will be synchronised automatically when the source is changed.
+            if ($file->get_referencelastsync()) {
+                return false;
+            }
+            $fs = get_file_storage();
+            $params = file_storage::unpack_reference($file->get_reference(), true);
+            if (!is_array($params) || !($storedfile = $fs->get_file($params['contextid'],
+                    $params['component'], $params['filearea'], $params['itemid'], $params['filepath'],
+                    $params['filename']))) {
+                $file->set_missingsource();
+            } else {
+                $file->set_synchronized($storedfile->get_contenthash(), $storedfile->get_filesize());
+            }
+            return true;
         }
 
-        // remember that we already cached in current request to prevent from querying again
-        $synchronized[$file->get_id()] = false;
+        // Backward compatibility (Moodle 2.3-2.5) implementation that calls
+        // methods repository::get_reference_file_lifetime(), repository::sync_individual_file()
+        // and repository::get_file_by_reference(). These methods are removed from the
+        // base repository class but may still be implemented by the child classes.
 
-        if (!$reference = $DB->get_record('files_reference', array('id'=>$file->get_referencefileid()))) {
+        // THIS IS NOT A GOOD EXAMPLE of implementation. For good examples see the overwriting methods.
+
+        if (!method_exists($this, 'get_file_by_reference')) {
+            // Function get_file_by_reference() is not implemented. No synchronisation.
             return false;
         }
 
-        if (!empty($reference->lastsync) and ($reference->lastsync + $reference->lifetime > time())) {
-            $synchronized[$file->get_id()] = true;
-            return true;
+        // Check if the previous sync result is still valid.
+        if (method_exists($this, 'get_reference_file_lifetime')) {
+            $lifetime = $this->get_reference_file_lifetime($file->get_reference());
+        } else {
+            // Default value that was hardcoded in Moodle 2.3 - 2.5.
+            $lifetime =  60 * 60 * 24;
         }
-
-        if (!$repository = self::get_repository_by_id($reference->repositoryid, SYSCONTEXTID)) {
+        if (($lastsynced = $file->get_referencelastsync()) && $lastsynced + $lifetime >= time()) {
             return false;
         }
 
-        if (!$repository->sync_individual_file($file)) {
-            return false;
+        $cache = cache::make('core', 'repositories');
+        if (($lastsyncresult = $cache->get('sync:'.$file->get_referencefileid())) !== false) {
+            if ($lastsyncresult === true) {
+                // We are in the process of synchronizing this reference.
+                // Avoid recursion when calling $file->get_filesize() and $file->get_contenthash().
+                return false;
+            } else {
+                // We have synchronised the same reference inside this request already.
+                // It looks like the object $file was created before the synchronisation and contains old data.
+                if (!empty($lastsyncresult['missing'])) {
+                    $file->set_missingsource();
+                } else {
+                    $cache->set('sync:'.$file->get_referencefileid(), true);
+                    if ($file->get_contenthash() != $lastsyncresult['contenthash'] ||
+                            $file->get_filesize() != $lastsyncresult['filesize']) {
+                        $file->set_synchronized($lastsyncresult['contenthash'], $lastsyncresult['filesize']);
+                    }
+                    $cache->set('sync:'.$file->get_referencefileid(), $lastsyncresult);
+                }
+                return true;
+            }
         }
 
-        $lifetime = $repository->get_reference_file_lifetime($reference);
-        $fileinfo = $repository->get_file_by_reference($reference);
-        if ($fileinfo === null) {
-            // does not exist any more - set status to missing
-            $file->set_missingsource($lifetime);
-            $synchronized[$file->get_id()] = true;
-            return true;
+        // Weird function sync_individual_file() that was present in API in 2.3 - 2.5, default value was true.
+        if (method_exists($this, 'sync_individual_file') && !$this->sync_individual_file($file)) {
+            return false;
         }
 
+        // Set 'true' into the cache to indicate that file is in the process of synchronisation.
+        $cache->set('sync:'.$file->get_referencefileid(), true);
+
+        // Create object with the structure that repository::get_file_by_reference() expects.
+        $reference = new stdClass();
+        $reference->id = $file->get_referencefileid();
+        $reference->reference = $file->get_reference();
+        $reference->referencehash = sha1($file->get_reference());
+        $reference->lastsync = $file->get_referencelastsync();
+        $reference->lifetime = $lifetime;
+
+        $fileinfo = $this->get_file_by_reference($reference);
+
         $contenthash = null;
         $filesize = null;
+        $fs = get_file_storage();
         if (!empty($fileinfo->filesize)) {
             // filesize returned
             if (!empty($fileinfo->contenthash) && $fs->content_exists($fileinfo->contenthash)) {
@@ -2796,8 +2839,7 @@ abstract class repository implements cacheable_object {
                 $contenthash = $file->get_contenthash();
             } else {
                 // we can't save empty contenthash so generate contenthash from empty string
-                $fs->add_string_to_pool('');
-                $contenthash = sha1('');
+                list($contenthash, $unused1, $unused2) = $fs->add_string_to_pool('');
             }
             $filesize = $fileinfo->filesize;
         } else if (!empty($fileinfo->filepath)) {
@@ -2807,22 +2849,25 @@ abstract class repository implements cacheable_object {
             // File handle returned
             $contents = '';
             while (!feof($fileinfo->handle)) {
-                $contents .= fread($handle, 8192);
+                $contents .= fread($fileinfo->handle, 8192);
             }
             fclose($fileinfo->handle);
-            list($contenthash, $filesize, $newfile) = $fs->add_string_to_pool($content);
+            list($contenthash, $filesize, $newfile) = $fs->add_string_to_pool($contents);
         } else if (isset($fileinfo->content)) {
             // File content returned
             list($contenthash, $filesize, $newfile) = $fs->add_string_to_pool($fileinfo->content);
         }
 
         if (!isset($contenthash) or !isset($filesize)) {
-            return false;
+            $file->set_missingsource(null);
+            $cache->set('sync:'.$file->get_referencefileid(), array('missing' => true));
+        } else {
+            // update files table
+            $file->set_synchronized($contenthash, $filesize);
+            $cache->set('sync:'.$file->get_referencefileid(),
+                    array('contenthash' => $contenthash, 'filesize' => $filesize));
         }
 
-        // update files table
-        $file->set_synchronized($contenthash, $filesize, 0, $lifetime);
-        $synchronized[$file->get_id()] = true;
         return true;
     }
 
index 6d021bb..1c6c25e 100644 (file)
@@ -7,11 +7,21 @@ http://docs.moodle.org/dev/Repository_API
 
 * get_option() now always return null when the first parameter ($config) is not empty, and
   no value was found for this $config. Previously this could sometimes return an empty array().
+
 * The function repository_attach_id() was removed, it was never used and was not useful.
+
 * New functions send_relative_file() and supports_relative_file() to allow sending relative linked
   files - see filesystem repository for example.
-* DB fields files.referencelifetime and files.referencelastsync are deleted.
-  Their values are stored only in files_reference.lastsync and files_reference.lifetime.
+
+* DB fields files.referencelifetime, files.referencelastsync and files_reference.lifetime
+  are deleted. The last synchronization time is stored only in files_reference.lastsync
+  and lifetime is not stored in DB any more, each repository must decide for itself
+  when to synchronize the file in function repository::sync_reference().
+
+* The following methods in class repository are deprecated: sync_external_file(),
+  get_file_by_reference(), get_reference_file_lifetime(), sync_individual_file() and
+  reset_caches(). Instead there is one method repository::sync_reference() - this simplifies
+  the repositories API and reduces the number of DB queries.
 
 === 2.5 ===
 
index 7f9a43a..b7de119 100644 (file)
@@ -29,7 +29,7 @@
 
 defined('MOODLE_INTERNAL') || die();
 
-$version  = 2013100400.00;              // YYYYMMDD      = weekly release date of this DEV branch.
+$version  = 2013100400.02;              // YYYYMMDD      = weekly release date of this DEV branch.
                                         //         RR    = release increments - 00 in DEV branches.
                                         //           .XX = incremental changes.