r108467 MediaWiki - Code Review archive

Repository:	MediaWiki
Revision:	< r108466‎ \| r108467 \| r108468 >
Date:	19:51, 9 January 2012
Author:	mglaser
Status:	deferred
Tags:
Comment:	now works against current phase3 better README.txt removed default config (should be put in LocalSettings)
Modified paths:	/trunk/extensions/WindowsAzureStorage/README.txt (modified) (history) /trunk/extensions/WindowsAzureStorage/WindowsAzureStorage.php (modified) (history) /trunk/extensions/WindowsAzureStorage/includes/filerepo/backend/WindowsAzureFileBackend.php (modified) (history)

Diff [purge]

Index: trunk/extensions/WindowsAzureStorage/includes/filerepo/backend/WindowsAzureFileBackend.php
—	—	@@ -23,50 +23,13 @@
24	24	* @ingroup FileBackend
25	25	*/
26	26	class WindowsAzureFileBackend extends FileBackend {
27		-
28		~~- function doStore( array $p ) {~~
29		~~- return $this->doStoreInternal( $p );~~
30		~~- }~~
31		-
32		~~- function doCopy( array $p ) {~~
33		~~- return $this->doCopyInternal( $p );~~
34		~~- }~~
35		-
36		~~- function doDelete( array $p ) {~~
37		~~- return $this->doDeleteInternal( $p );~~
38		~~- }~~
39		-
40		~~- function doConcatenate( array $p ) {~~
41		~~- return $this->dodoConcatenateInternal( $p );~~
42		~~- }~~
43		-
44		~~- function doCreate( array $p ) {~~
45		~~- return $this->doCreateInternal( $p );~~
46		~~- }~~
47		-
48		- /**
49		~~- * @see FileBackend::move()~~
50		~~- */~~
51		~~- protected function doMove( array $params ) {~~
52		~~- // Copy source to dest~~
53		~~- // TODO: Remove backend. I assume, this function does not need to be overridden.~~
54		~~- $status = $this->backend->copy( $params );~~
55		~~- if ( !$status->isOK() ) {~~
56		~~- return $status;~~
57		~~- }~~
58		~~- // Delete source (only fails due to races or medium going down)~~
59		~~- // TODO: Remoce backend~~
60		~~- $status->merge( $this->backend->delete( array( 'src' => $params['src'] ) ) );~~
61		~~- $status->setResult( true, $status->value ); // ignore delete() errors~~
62		~~- return $status;~~
63		~~- }~~
64		-
	27	+
65	28	/** @var Microsoft_WindowsAzure_Storage_Blob */
66	29	protected $storageClient = null;
67	30
68	31	/** @var Array Map of container names to Azure container names */
69	32	protected $containerPaths = array();
70		-
	33	+
71	34	/**
72	35	* @see FileBackend::__construct()
73	36	* Additional $config params include:
—	—	@@ -74,7 +37,7 @@
75	38	* azureAccount : Windows Azure user used by MediaWiki
76	39	* azureKey : Authentication key for the above user (used to get sessions)
77	40	* //azureContainer : Identifier of the container. (Optional. If not provided wikiId will be used as container name)
78		~~- * containerPaths : Map of container names to Azure container names~~
	41	+ * containerPaths : Map of container names to Azure container names
79	42	*/
80	43	public function __construct( array $config ) {
81	44	parent::__construct( $config );
—	—	@@ -96,7 +59,13 @@
97	60	if ( strlen( urlencode( $relStoragePath ) ) > 1024 ) {
98	61	return null;
99	62	}
	63	+
	64	+ // Get absolute path given the container base dir
	65	+ if ( isset( $this->containerPaths[$container] ) ) {
	66	+ return $this->containerPaths[$container] . "/{$relStoragePath}";
	67	+ }
100	68	// TODO: Should storagepath not be urlencoded?
	69	+ // TODO: return null
101	70	return $relStoragePath;
102	71	}
103	72
—	—	@@ -105,16 +74,14 @@
106	75	*/
107	76	function doStoreInternal( array $params ) {
108	77	$status = Status::newGood();
109		~~- // TODO: Use more telling names~~
110		~~- list( $c, $dir ) = $this->resolveStoragePath( $params['dst'] );~~
	78	+ list( $dstCont, $dstRel ) = $this->resolveStoragePath( $params['dst'] );
111	79	try {
112		~~- $result = $this->storageClient->putBlob( $c, $dir, $params['src']);~~
	80	+ $result = $this->storageClient->putBlob( $dstCont, $dstRel, $params['src']);
113	81	}
114	82	catch ( Exception $e ) {
115	83	// TODO: Read exception. Are there different ones?
116	84	$status->fatal( 'backend-fail-put' );
117	85	}
118		~~- //error_log( __METHOD__.'::putBlob - result: '.print_r( $result, true ) );~~
119	86	return $status;
120	87	}
121	88
—	—	@@ -125,6 +92,7 @@
126	93	$status = Status::newGood();
127	94	list( $srcContainer, $srcDir ) = $this->resolveStoragePath( $params['src'] );
128	95	list( $dstContainer, $dstDir ) = $this->resolveStoragePath( $params['dst'] );
	96	+
129	97	// TODO: check for null
130	98	try {
131	99	$result = $this->storageClient->copyBlob( $srcContainer, $srcDir, $dstContainer, $dstDir);
—	—	@@ -132,7 +100,6 @@
133	101	catch ( Exception $e ) {
134	102	$status->fatal( 'backend-fail-copy', $e->getMessage() );
135	103	}
136		~~- //error_log( __METHOD__.'::copyBlob - result: '.print_r( $result, true ) );~~
137	104	return $status;
138	105	}
139	106
—	—	@@ -149,12 +116,10 @@
150	117	}
151	118
152	119	// (a) Check the source container
153		~~- try { //TODO: Unnecessary --> remove~~
	120	+ try { //TODO: Unnecessary --> remove (or better, check in resolveStoragePath?)
154	121	$container = $this->storageClient->getContainer( $srcCont );
155	122	}
156	123	catch ( Exception $e ) {
157		~~- // TODO: remove error_log~~
158		~~- error_log( __METHOD__.':'.__LINE__.' '.$e->getMessage() );~~
159	124	$status->fatal( 'backend-fail-internal' );
160	125	return $status;
161	126	}
—	—	@@ -164,7 +129,6 @@
165	130	$this->storageClient->deleteBlob( $srcCont, $srcRel );
166	131	}
167	132	catch ( Exception $e ) {
168		~~- error_log( __METHOD__.':'.__LINE__.' '.$e->getMessage() );~~
169	133	$status->fatal( 'backend-fail-internal' );
170	134	}
171	135
—	—	@@ -193,10 +157,9 @@
194	158	// (b) Actually create the object
195	159	try {
196	160	// TODO: how do I know the container exists? Should we call prepare?
197		~~- $this->storageClient->putBlobData( $dstCont, $dstRel, $params['content'] );~~
	161	+ $this->storageClient->putBlobData( $dstCont, $dstRel, $params['content'] );
198	162	}
199	163	catch ( Exception $e ) {
200		~~- error_log( __METHOD__.':'.__LINE__.' '.$e->getMessage() );~~
201	164	$status->fatal( 'backend-fail-internal' );
202	165	}
203	166
—	—	@@ -206,7 +169,8 @@
207	170	/**
208	171	* @see FileBackend::prepare()
209	172	*/
210		~~- function prepare( array $params ) {~~
	173	+
	174	+ function doPrepareInternal( $container, $dir, array $params ) {
211	175	$status = Status::newGood();
212	176
213	177	list( $c, $dir ) = $this->resolveStoragePath( $params['dir'] );
—	—	@@ -216,9 +180,10 @@
217	181	}
218	182	try {
219	183	$this->storageClient->createContainerIfNotExists( $c );
220		~~- $this->storageClient->setContainerAcl( $c, Microsoft_WindowsAzure_Storage_Blob::ACL_PUBLIC );//TODO: Really set public?~~
	184	+ // TODO: must this be set anytime prepare is called?
	185	+ $this->storageClient->setContainerAcl( $c, Microsoft_WindowsAzure_Storage_Blob::ACL_PUBLIC );
221	186
222		~~- //TODO: check if readable and writeable~~
	187	+ // TODO: check if readable and writeable
223	188	//$container = $this->storageClient->getContainer( $c );
224	189	//$status->fatal( 'directoryreadonlyerror', $params['dir'] );
225	190	//$status->fatal( 'directorynotreadableerror', $params['dir'] );
—	—	@@ -230,6 +195,7 @@
231	196	return $status;
232	197	}
233	198
	199	+
234	200	/**
235	201	* @see FileBackend::resolveContainerName()
236	202	*/
—	—	@@ -237,7 +203,7 @@
238	204	//Azure container naming conventions; http://msdn.microsoft.com/en-us/library/dd135715.aspx
239	205	$container = strtolower($container);
240	206	$container = preg_replace( '#[^a-z0-9\-]#', '', $container );
241		~~- // TODO: -test und test- geht auch nicht~~
	207	+ // TODO: "-test" and "test-" are invalid, too
242	208	$container = preg_replace( '#-{2,}#', '-', $container );
243	209
244	210	return $container;
—	—	@@ -246,27 +212,35 @@
247	213	/**
248	214	* @see FileBackend::secure()
249	215	*/
	216	+ /*
	217	+ // TODO: check if we need to override doSecure
250	218	function secure( array $params ) {
251	219	$status = Status::newGood();
252		~~- // @TODO: restrict container from $this->swiftProxyUser~~
253	220	return $status; // badgers? We don't need no steenking badgers!
254	221	}
	222	+ */
255	223
256	224	/**
257	225	* @see FileBackend::fileExists()
258	226	*/
259		~~- function fileExists( array $params ) {~~
260		~~- list( $c, $dir ) = $this->resolveStoragePath( $params['src'] );~~
261		~~- // TODO: null? Telling names~~
262		~~- $exists = $this->storageClient->blobExists( $c, $dir );~~
263		~~- //error_log( __METHOD__.'::blobExists - result: '.$exists );~~
	227	+ function doFileExists( array $params ) {die();
	228	+ // TODO: Merely renamed this functino from fileExists. Check if more is neccessarey
	229	+ // TODO: Off topic here: Prepare function might call some internal function which can be overridden.
	230	+ list( $srcCont, $srcRel ) = $this->resolveStoragePath( $params['src'] );
	231	+ if ( $srcRel === null ) {
	232	+ return false; // invalid storage path
	233	+ }
	234	+
	235	+ $exists = $this->storageClient->blobExists( $srcCont, $srcRel );
264	236	return $exists;
265	237	}
266	238
267	239	/**
268	240	* @see FileBackend::getFileTimestamp()
269	241	*/
270		~~- function getFileTimestamp( array $params ) {~~
	242	+ // TODO: merely renamed from getFileTimestamp. Check if there are any probs.
	243	+ // TODO: Is this method still used?
	244	+ function doGetFileTimestamp( array $params ) {
271	245	list( $srcCont, $srcRel ) = $this->resolveStoragePath( $params['src'] );
272	246	if ( $srcRel === null ) {
273	247	return false; // invalid storage path
—	—	@@ -276,9 +250,9 @@
277	251	try {
278	252	//TODO Maybe use getBlobData()?
279	253	$blob = $this->storageClient->getBlobInstance( $srcCont, $srcRel );
280		~~- $timestamp = wfTimestamp( TS_MW, $blob->lastmodified ); //TODO: Timezone?~~
281		~~- } catch ( Exception $e ) { // some other exception?~~
282		~~- error_log( __METHOD__.':'.__LINE__.' '.$e->getMessage() );~~
	254	+ $timestamp = wfTimestamp( TS_MW, $blob->LastModified ); //TODO: Timezone?
	255	+ } catch ( Exception $e ) {
	256	+ // TODO: Log it? What about different types of exceptions?
283	257	}
284	258	return $timestamp;
285	259	}
—	—	@@ -286,22 +260,24 @@
287	261	/**
288	262	* @see FileBackend::getFileList()
289	263	*/
290		~~- function getFileList( array $params ) {~~
	264	+ function getFileListInternal( $container, $dir, array $params ) {
	265	+ // TODO: merely renamed from getFileList. Check implications (i assume, the list thing is no longer needed.
	266	+ //list( $c, $dir ) = $this->resolveStoragePath( $params['dir'] );
291	267	$files = array();
292		~~- list( $c, $dir ) = $this->resolveStoragePath( $params['dir'] );~~
293	268	try {
294	269	if ( $dir === null ) {
295		~~- $blobs = $this->storageClient->listBlobs($c);~~
	270	+ $blobs = $this->storageClient->listBlobs($container);
296	271	}
297	272	else {
298		~~- $blobs = $this->storageClient->listBlobs( $c, $dir );//TODO:Check if $dir really is a startsequence of the blob name~~
	273	+ //TODO: Check if $dir really is a startsequence of the blob name
	274	+ $blobs = $this->storageClient->listBlobs($container, $dir);
299	275	}
	276	+
300	277	foreach( $blobs as $blob ) {
301	278	$files[] = $blob->name;
302	279	}
303	280	}
304	281	catch( Exception $e ) {
305		~~- error_log( __METHOD__.':'.__LINE__.' '.$e->getMessage() );~~
306	282	return null;
307	283	}
308	284
—	—	@@ -332,10 +308,37 @@
333	309	$this->storageClient->getBlob( $srcCont, $srcRel, $tmpPath );
334	310	}
335	311	catch ( Exception $e ) {
336		~~- error_log( __METHOD__.':'.__LINE__.' '.$e->getMessage() );~~
337	312	$tmpFile = null;
338	313	}
339	314
340	315	return $tmpFile;
341	316	}
342		-}
	317	+
	318	+ /**
	319	+ * @see FileBackend::doFileExists()
	320	+ */
	321	+ protected function doGetFileStat( array $params ) {
	322	+ // TODO: Refactor
	323	+ list( $srcCont, $srcRel ) = $this->resolveStoragePathReal( $params['src'] );
	324	+ if ( $srcRel === null ) {
	325	+ return false; // invalid storage path
	326	+ }
	327	+
	328	+ $timestamp= false;
	329	+ $size = false;
	330	+ // TODO: need additional checks??
	331	+ try {
	332	+ // TODO: Maybe use getBlobData()?
	333	+ $blob = $this->storageClient->getBlobInstance( $srcCont, $srcRel );
	334	+ $timestamp = wfTimestamp( TS_MW, $blob->LastModified ); //TODO: Timezone?
	335	+ $size = $blob->Size;
	336	+ return array(
	337	+ 'mtime' => $timestamp,
	338	+ 'size' => $size
	339	+ );
	340	+ } catch ( Exception $e ) {
	341	+ // TODO: Log it? What about different types of exceptions?
	342	+ }
	343	+ return false;
	344	+ }
	345	+}
\ No newline at end of file
Index: trunk/extensions/WindowsAzureStorage/README.txt
—	—	@@ -1 +1,80 @@
2		~~-This extension contains the Microsoft "Windows Azure SDK for PHP v4.1.0" (http://phpazure.codeplex.com/) by REALDOLMEN.~~
\ No newline at end of file
	2	+==Prerequisites==
	3	+You will need to have all classes from the "PHPAzure - Windows Azure SDK for
	4	+PHP" by REALDOLMEN available.
	5	+
	6	+Download it at http://phpazure.codeplex.com/
	7	+
	8	+You can use the WindowsAzureSDK extension for MediaWiki to register the SDK with
	9	+your environment.
	10	+
	11	+
	12	+==Installation==
	13	+Copy the WindowsAzureStorage extension to your <mediawiki>/extensions directory
	14	+and add the following line to your LocalSettings.php:
	15	+
	16	+include_once( "$IP/../extensions/WindowsAzureStorage/WindowsAzureStorage.php" );
	17	+
	18	+
	19	+==Configuration==
	20	+Add an entry for the Azure File Backend to the $wgFileBackends array in your
	21	+LocalSettings.php. Also configure your repo (i.e. the LocalFileRepo) to use the
	22	+Azure File Backend.
	23	+
	24	+The following configuration is suitable for a development environment running
	25	+Microsoft Windows Azure Service Emulators from the Azure SDK.
	26	+
	27	+$wgFileBackends[] = array(
	28	+ 'name' => 'azure-backend',
	29	+ 'class' => 'WindowsAzureFileBackend',
	30	+ 'lockManager' => 'nullLockManager',
	31	+ 'azureHost' => 'http://127.0.0.1:10000',
	32	+ 'azureAccount' => 'devstoreaccount1',
	33	+ 'azureKey' => 'Eby8vdM02xNOcqFlqUwJPLlmEtlCDXJ1OUzFT50uSRZ6IFsuFq2UVErCz4I6tq/K1SZFPTOtr/KBHBeksoGMGw==',
	34	+
	35	+ //IMPORTANT: Mind the container naming conventions! http://msdn.microsoft.com/en-us/library/dd135715.aspx
	36	+ 'containerPaths' => array(
	37	+ 'media-public' => 'media-public',
	38	+ 'media-thumb' => 'media-thumb',
	39	+ 'media-deleted' => 'media-deleted',
	40	+ 'media-temp' => 'media-temp',
	41	+
	42	+ )
	43	+);
	44	+
	45	+$wgLocalFileRepo = array (
	46	+ 'class' => 'LocalRepo',
	47	+ 'name' => 'local',
	48	+ 'scriptDirUrl' => '/php/mediawiki-filebackend-azure',
	49	+ 'scriptExtension' => '.php',
	50	+ 'url' => $wgScriptPath.'/img_auth.php', // It is important to set this to img_auth. Basically, there is no alternative.
	51	+ 'hashLevels' => 2,
	52	+ 'thumbScriptUrl' => false,
	53	+ 'transformVia404' => false,
	54	+ 'deletedHashLevels' => 3,
	55	+ 'backend' => 'azure-backend',
	56	+ 'zones' =>
	57	+ array (
	58	+ 'public' =>
	59	+ array (
	60	+ 'container' => 'local-public',
	61	+ 'directory' => '',
	62	+ ),
	63	+ 'thumb' =>
	64	+ array(
	65	+ 'container' => 'local-public',
	66	+ 'directory' => 'thumb',
	67	+ ),
	68	+ 'deleted' =>
	69	+ array (
	70	+ 'container' => 'local-public',
	71	+ 'directory' => 'deleted',
	72	+ ),
	73	+ 'temp' =>
	74	+ array(
	75	+ 'container' => 'local-public',
	76	+ 'directory' => 'temp',
	77	+ )
	78	+ )
	79	+);
	80	+
	81	+$wgImgAuthPublicTest = false;
\ No newline at end of file
Index: trunk/extensions/WindowsAzureStorage/WindowsAzureStorage.php
—	—	@@ -1,7 +1,7 @@
2	2	<?php
3	3	/*
4		~~- (c) Hallo Welt! Medienwerkstatt GmbH, 2011 GPL~~
5		-
	4	+ (c) Hallo Welt! Medienwerkstatt GmbH together with Microsoft Corp., 2011 GPL
	5	+
6	6	This program is free software; you can redistribute it and/or modify
7	7	it under the terms of the GNU General Public License as published by
8	8	the Free Software Foundation; either version 2 of the License, or
—	—	@@ -26,7 +26,7 @@
27	27	$wgExtensionCredits['other'][] = array(
28	28	'path' => __FILE__,
29	29	'name' => 'WindowsAzureStorage',
30		~~- 'author' => array( 'Hallo Welt! Medienwerkstatt GmbH' ),~~
	30	+ 'author' => array( 'Hallo Welt! Medienwerkstatt GmbH', 'Markus Glaser', 'Robert Vogel' ),
31	31	'url' => 'http://www.hallowelt.biz',
32	32	'version' => '1.0.0',
33	33	'descriptionmsg' => 'windowsazurestorage-desc',
—	—	@@ -35,25 +35,4 @@
36	36	$dir = dirname(__FILE__) . '/';
37	37	$wgExtensionMessagesFiles['WindowsAzureStorage'] = $dir . 'WindowsAzureStorage.i18n.php';
38	38
39		~~-$wgAutoloadClasses['WindowsAzureFileBackend'] = $dir . 'includes/filerepo/backend/WindowsAzureFileBackend.php';~~
40		-
41		~~-/* Those are just development values. You may override them or specify your own backend definition in LocalSettings.php */~~
42		~~-$wgFileBackends[] = array(~~
43		~~- 'name' => 'azure-backend',~~
44		~~- 'class' => 'WindowsAzureFileBackend',~~
45		~~- //'wikiId' => 'some_unique_ID',~~
46		~~- 'lockManager' => 'nullLockManager',~~
47		~~- 'azureHost' => 'http://127.0.0.1:10000',~~
48		~~- 'azureAccount' => 'devstoreaccount1',~~
49		~~- 'azureKey' => 'Eby8vdM02xNOcqFlqUwJPLlmEtlCDXJ1OUzFT50uSRZ6IFsuFq2UVErCz4I6tq/K1SZFPTOtr/KBHBeksoGMGw==',~~
50		~~- //'azureContainer' => 'developcontainer',~~
51		-
52		~~- //IMPORTANT: Mind the container naming conventions! http://msdn.microsoft.com/en-us/library/dd135715.aspx~~
53		~~- 'containerPaths' => array(~~
54		~~- 'media-public' => 'media-public',~~
55		~~- 'media-thumb' => 'media-thumb',~~
56		~~- 'media-deleted' => 'media-deleted',~~
57		~~- 'media-temp' => 'media-temp',~~
58		-
59		~~- )~~
60		~~-);~~
	39	+$wgAutoloadClasses['WindowsAzureFileBackend'] = $dir . 'includes/filerepo/backend/WindowsAzureFileBackend.php';
\ No newline at end of file

Status & tagging log

01:28, 13 June 2012 Krinkle (talk | contribs) changed the tags for r108467 [removed: nodeploy]
22:37, 13 January 2012 Reedy (talk | contribs) changed the status of r108467 [removed: new added: deferred]
20:05, 9 January 2012 RobLa-WMF (talk | contribs) changed the tags for r108467 [added: nodeploy]