r101413 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r101412‎ | r101413 | r101414 >
Date:21:40, 31 October 2011
Author:blobaugh
Status:deferred (Comments)
Tags:
Comment:
Filled in more architecting code for FileBackend
Modified paths:
  • /branches/FileBackend/phase3/includes/filerepo/AbstractFileStore.php (modified) (history)
  • /branches/FileBackend/phase3/includes/filerepo/FileBackend.php (modified) (history)

Diff [purge]

Index: branches/FileBackend/phase3/includes/filerepo/AbstractFileStore.php
@@ -1,42 +1,131 @@
22 <?php
33
44 /**
5 - * A particular data store may not support every operation. If it does not support
6 - * an operation that should return -1. It should not throw an exception as
7 - * the operation in question may be naively called from a loop that does not
8 - * check for support
 5+ * This class defines the methods as abstract that should be implemented in
 6+ * child classes if the target store supports the operation.
97 */
108 abstract class AbstractFileStore {
119
12 - abstract store( $params ) {
 10+ /**
 11+ * Array of operations that are supported by this data store
 12+ *
 13+ * This could potentially be populated via reflection at some point
 14+ *
 15+ * <code>
 16+ * $supportedOperations = array('store', 'copy', 'delete')
 17+ * </code>
 18+ *
 19+ * @var Array
 20+ */
 21+ protected $supportedOperations;
 22+
 23+ /**
 24+ * Setup the FileStore. The operations in this constructor are required for
 25+ * the object to operate properly. If any subclasses use a constructor they
 26+ * must call this parent constructor as well
 27+ */
 28+ public function __construct() {
 29+ // Make sure the getFileProps is in the supportedOperations array
 30+ // The support for this only exists in this parent class
 31+ $this->supportedOperations[] = 'getFileProps';
 32+ }
 33+
 34+ /**
 35+ * Returns a list of supported operations
 36+ *
 37+ * @return Array
 38+ */
 39+ public function getSupportedOperations() {
 40+ return $this->supportedOperations;
 41+ }
 42+
 43+ /**
 44+ * Use this function in subclasses to define the specific FileStore
 45+ * store functionality
 46+ *
 47+ * @param Array $params
 48+ */
 49+ abstract function store( $params ) {
1350 return -1;
1451 }
1552
16 - abstract relocate( $params ) {
 53+ /**
 54+ * Use this function in subclasses to define the specific FileStore
 55+ * relocate functionality
 56+ *
 57+ * @param Array $params
 58+ */
 59+ abstract function relocate( $params ) {
1760 return -1;
1861 }
1962
20 - abstract copy( $params ) {
 63+ /**
 64+ * Use this function in subclasses to define the specific FileStore
 65+ * copy functionality
 66+ *
 67+ * @param Array $params
 68+ */
 69+ abstract function copy( $params ) {
2170 return -1;
2271 }
2372
24 - abstract delete( $params ) {
 73+ /**
 74+ * Use this function in subclasses to define the specific FileStore
 75+ * delete functionality
 76+ *
 77+ * @param Array $params
 78+ */
 79+ abstract function delete( $params ) {
2580 return -1;
2681 }
2782
28 - abstract move( $params ) {
 83+ /**
 84+ * Use this function in subclasses to define the specific FileStore
 85+ * move functionality
 86+ *
 87+ * @param Array $params
 88+ */
 89+ abstract function move( $params ) {
2990 return -1;
3091 }
3192
32 - abstract concatinate( $params ) {
 93+ /**
 94+ * Use this function in subclasses to define the specific FileStore
 95+ * concatinate functionality
 96+ *
 97+ * @param Array $params
 98+ */
 99+ abstract function concatinate( $params ) {
33100 return -1;
34101 }
35102
36 - abstract fileExists( $params ) {
 103+ /**
 104+ * Use this function in subclasses to define the specific FileStore
 105+ * file exists functionality
 106+ *
 107+ * @param Array $params
 108+ */
 109+ abstract function fileExists( $params ) {
37110 return -1;
38111 }
39112
40 - abstract getLocalCopy( $params ) {
 113+ /**
 114+ * Use this function in subclasses to define the specific FileStore
 115+ * retreive functionality
 116+ *
 117+ * @param Array $params
 118+ */
 119+ abstract function getLocalCopy( $params ) {
41120 return -1;
42121 }
 122+
 123+ /**
 124+ * Get the properties of a file
 125+ *
 126+ * @param Array $params
 127+ * @return Array
 128+ */
 129+ public function getFileProps( $params ) {
 130+ throw new MWException(__METHOD__ . " has not yet been implemented in AbstractFileStore");
 131+ }
43132 } // end class
\ No newline at end of file
Index: branches/FileBackend/phase3/includes/filerepo/FileBackend.php
@@ -1,9 +1,8 @@
22 <?php
33
44 /**
5 - * Handles all operations on real files. The FileBacked is designed to be an
6 - * abstraction layer that allows developers to create a new class that extends
7 - * this class to use any arbitrary data store for media uploads. To manipulate
 5+ * Handles all operations on real files. The FileBackend should be the single
 6+ * point of interface to the filesystem in all core and extension code. To manipulate
87 * files contained within the current MediaWiki installation the dev/extension
98 * writer need only to interact with the doOps() method in this class
109 *
@@ -12,37 +11,48 @@
1312 * @todo Implement streamFile
1413 * @todo Implement enumFiles
1514 * @todo Design and implement smart file chunking uploads
16 - * @todo Fill in $this->validOperations via reflection into AbstractDataStore
17 - * @todo implement a method to ensure there is a primary store that registerDataStore can use
18 - * @todo Alter registerDataStore with the above to set incoming data store as primary if one does not exist
19 - * @todo Alter registerDataStore to alter the current primary to false if isPrimaryStore is set to true on incoming
20 - * @todo Register data store in global and existing in includes/filerepo/dataStores inside of constructor
2115 */
2216 abstract class FileBackend {
2317
24 - /*
25 - * This could potentially be populated via reflection. Look at the methods
26 - * available in the AbstratDataStore object
 18+ /**
 19+ * This should be a subclass of the AbstractFileStore and specific to a
 20+ * single data store
 21+ * @var AbstractFileStore
2722 */
28 - protected $validOperations = array('store'/* add a new file to data store */,
29 - 'relocate' /* used to relocate all files in store */,
30 - 'copy',
31 - 'delete',
32 - 'move',
33 - 'concatinate',
34 - 'fileExists',
35 - 'getFileProps',
36 - 'getLocalCopy'
37 - /* 'streamFile' not yet implemented , */
38 - /* 'enumFiles' not yet implemented*/
39 - );
 23+ private $registeredDataStore = array();
4024
41 - private $registeredDataStores = array();
42 -
4325 /**
44 - * Constructor, currently does nothing
 26+ * Constructor
 27+ * Loads the required data store object for interacting with any data store.
 28+ * The store is loaded via a config object that is loosely structured as
 29+ *
 30+ * <code>
 31+ * $conf = array( 'store' => 'LocalFileSystemDataStore' )
 32+ * </code>
 33+ *
 34+ * The only required array item is 'store' which specifies which data store
 35+ * object to load. Additional options may be in that array that are needed
 36+ * for various configuration details of the specific store
 37+ *
 38+ * @param Array $conf
4539 */
46 - public function __construct();
 40+ public function __construct($conf) {
 41+ if( !is_array( $conf ) ) {
 42+ // Ensure there is configuration data present
 43+ throw new MWException( __METHOD__ . ': no data store configuration available' );
 44+ } elseif( !isset( $conf['store'] ) ) {
 45+ // If a store is not specified use the LocalFileSystemDataStore
 46+ $conf['store'] = 'LocalFileSystemDataStore';
 47+ }
 48+ if( !file_exists( __DIR__ . "/dataStores/{$conf['store']}.php" ) ) {
 49+ // Ensure store object exists
 50+ throw new MWException( __METHOD__ . ": {$conf['store']} data store object does not exist" );
 51+ }
 52+
 53+ // Load the dataStore object
 54+ require_once "dataStore/{$conf['store']}.php";
 55+ $this->registerDataStore( new $conf['store']($conf) );
 56+ }
4757
4858
4959
@@ -53,7 +63,7 @@
5464 * correct file store module
5565 *
5666 * For more information about a specific operation see the abstract method
57 - * definitions comment block
 67+ * definitions in AbstractFileStore or the extended class
5868 *
5969 * Using $ops
6070 * $ops is an array of arrays. The first array holds a list of operations.
@@ -74,41 +84,34 @@
7585 throw new MWException(__METHOD__ . " not provided with an operations array");
7686 }
7787
 88+ $st = ''; // Status message string
7889 $statusObject = ''; // some sort of status object that can hold other status objects
7990
8091 foreach( $ops AS $i => $op ) {
81 - switch ( $op['operation'] ) {
82 - case 'move':
83 - $st = $this->commonCaller('move', $op);
84 - break;
85 - case 'delete':
86 - $st = $this->commonCaller('delete', $op);
87 - break;
88 - case 'copy':
89 - $st = $this->copy();
90 - break;
91 - case 'getFileProps':
92 - $tmpFile = $this->getLocalCopy();
93 - $st = $this->getFileProps( $tmpFile );
94 - default:
95 - $st = 'Unknown data store operation ' . $op['operation']);
 92+ if( in_array( $op['operation'], $this->registeredDataStore->getSupportedOperations() ) ) {
 93+ // Ensure that the operation is supported before attempting to do it
 94+ $st = $this->registeredDataStore->$op['operation']();
 95+ } else {
 96+ $st = "{$op['operation']} is not supported by the current data store";
9697 }
97 - $statusObject->append( $st );
 98+
 99+
 100+ $statusObject->append( $st );
98101 if ( $st && $reversed) {
99 - // oh noes! Something went wrong AGAIN.
100 - // pray to gods.
101 - return 'STATUS OBJECT';
102 - elseif ( $st ) {
103 - // oh crap, something went wrong. Try to unwind.
104 - return $this->doOps( $this->unwind( $ops, $i ), 1);
105 - }
106 - }
 102+ // oh noes! Something went wrong AGAIN.
 103+ // pray to gods.
 104+ return 'STATUS OBJECT';
 105+ } elseif ( $st ) {
 106+ // oh crap, something went wrong. Try to unwind.
 107+ return $this->doOps( $this->unwind( $ops, $i ), 1);
 108+ }
 109+ }
107110
108 - return 'STATUS OBJECT';
 111+ return 'STATUS OBJECT';
109112 }
110113
111 - /**
112 - * Unwinds an array of operations, attempting to reverse their action.
 114+ /**
 115+ * Unwinds an array of operations, attempting to reverse their action.
113116 * @param Array $ops - Array of arrays containing N operations to execute IN ORDER
114117 * @param Integer $i - index of first operation that failed.
115118 * @return Array
@@ -141,134 +144,15 @@
142145 }
143146
144147 /**
145 - *
146 - * @param <type> $dataStoreObject
147 - * @param <type> $isPrimaryStore
 148+ * Validates that the data store is infact of the proper type and then adds
 149+ * it to the registered data store property
 150+ *
 151+ * @param AbstractDataStore $dataStoreObject
148152 */
149 - public function registerDataStoreModule($dataStoreObject, $isPrimaryStore = false) {
 153+ public function registerDataStore($dataStoreObject) {
150154 if(is_subclassof($dataStoreObject, 'AbstractDataStore')) {
151 - $this->registeredDataStores[] = array($dataSourceObject, $isPrimaryStore);
 155+ $this->registeredDataStore[] = $dataSourceObject;
152156 }
153157 }
154 -
155 - /**
156 - * Ensure that there are data stores registered. If not then register the
157 - * local file system as the default data store
158 - *
159 - * @todo Create local data store object and set as default if no store exists
160 - */
161 - private function ensureRegisteredDataStore() {
162 - if(empty($this->registeredDataStores)) {
163 - // $this->registerDataStoreModule(new LocalFileSystemDataStore())
164 - }
165 - }
166 -
167 - /**
168 - * The commonCaller() method is used for operations that contain the same
169 - * code as many other options. It should be used as often as possible to
170 - * reduce code duplication
171 - * @param String $operation
172 - * @param Array $params
173 - * @return Status object of some sort
174 - */
175 - private function commonCaller($operation, $params) {
176 - $statusObject = ''; // new Status object of some sort
177 - foreach($this->registeredDataStores AS $store) {
178 - $statusObject->append($store->$operation($params));
179 - }
180 - return $statusObject;
181 - }
182 -
183 -
184 -// /**
185 -// * Store a file to a given destination.
186 -// *
187 -// * @param $srcPath String: source path or virtual URL
188 -// * @param $dstZone String: destination zone
189 -// * @param $dstRel String: destination relative path
190 -// * @param $flags Integer: bitwise combination of the following flags:
191 -// * self::DELETE_SOURCE Delete the source file after upload
192 -// * self::OVERWRITE Overwrite an existing destination file instead of failing
193 -// * self::OVERWRITE_SAME Overwrite the file if the destination exists and has the
194 -// * same contents as the source
195 -// * @return FileRepoStatus
196 -// */
197 -// abstract function store( $srcPath, $dstZone, $dstRel, $flags = 0 );
198 -//
199 -// /**
200 -// * Verify that a file exists in the file store
201 -// *
202 -// * @param String $file
203 -// * @param Integer $flags
204 -// * @return Boolean
205 -// */
206 -// abstract function fileExists( $file, $flags = 0 );
207 -//
208 -// /**
209 -// * Move a file to the deletion archive.
210 -// * If no valid deletion archive exists, this may either delete the file
211 -// * or throw an exception, depending on the preference of the repository
212 -// * @param $srcRel Mixed: relative path for the file to be deleted
213 -// * @param $archiveRel Mixed: relative path for the archive location.
214 -// * Relative to a private archive directory.
215 -// * @return FileRepoStatus object
216 -// */
217 -// abstract function delete( $srcRel, $archiveRel );
218 -//
219 -// /**
220 -// * Moves a file from one place to another in the file store
221 -// *
222 -// * @param String $srcRel - Path to source file
223 -// * @param String $destRel - Path to new destination
224 -// * @return Boolean
225 -// */
226 -// abstract function move( $srcRel, $destRel );
227 -//
228 -// abstract function copy ( $src, $dest );
229 -//
230 -//
231 -//
232 -//
233 -//
234 -//
235 -// /**
236 -// * Retreive a copy of a file from the file store and place it in the local
237 -// * $wgTmpDirectory. A new TempLocalFile object should be created and returned
238 -// * that contains the local path to the file
239 -// *
240 -// * Please overload this class inside your storage module
241 -// *
242 -// * @return TempLocalFile
243 -// */
244 -// abstract function getLocalCopy( $file );
245 -//
246 -// /**
247 -// * Return an associative array containing the file properties
248 -// *
249 -// * @param String $file
250 -// * @return Array
251 -// */
252 -// public function getFileProps( $file ) {
253 -// $file = $this->getLocalCopy( $file );
254 -// return File::getPropsFromPath( $file->getPath() );
255 -// }
256 -//
257 -// /**
258 -// * Returns an associative array containing a listing of the names of the
259 -// * thumbnail files associated with the provided image file
260 -// *
261 -// * Please overload this class inside your storage module
262 -// *
263 -// * @param String $file - Name of file to retreive thumbnail listing for
264 -// * @return Array
265 -// */
266 -// abstract function getThumbnailList( $file );
267 -//
268 -//
269 -//
270 -// public function addChunk() { throw new MWException( __METHOD__ . ' not yet implemented.' ); }
271 -// public function concatenateChunks() { throw new MWException( __METHOD__ . ' not yet implemented.' ); }
272 -
273 -
274158 } // end class
275159

Comments

#Comment by Aaron Schulz (talk | contribs)   00:22, 1 November 2011

What's with the 'concatinate' function? Tis spelled a bit funny :)

Status & tagging log