AmazonS3::delete() - Code Metrics - Inspection of "Allow access to underlying service in adapters" - partnermarketing/PartnermarketingFileSystemBundle - Measure and Improve Code Quality continuously with Scrutinizer

Completed

Pull Request — master (#24)

by Zac

created 2017-01-28 18:25 UTC

AmazonS3::delete() A

↳ Parent: AmazonS3

Complexity

Conditions	1
Paths	1

Size

Total Lines	11
Code Lines	6

Duplication

Lines	0
Ratio	0 %

Code Coverage

Tests	0
CRAP Score	2

Importance

Changes

Metric	Value
dl	0
loc	11
ccs	0
cts	6
cp	0
rs	9.4285
c	0
b	0
f	0
cc	1
eloc	6
nc	1
nop	1
crap	2

<?php

namespace Partnermarketing\FileSystemBundle\Adapter;

use Aws\S3\S3Client as AmazonClient;
use Guzzle\Http\EntityBody;
use Guzzle\Http\Mimetypes;

/**
 * Amazon specific file system adapter
 */
class AmazonS3 implements AdapterInterface
{
    private $service;
    private $bucket;
    private $localTmpDir;
    private $options;
    private $haveEnsuredBucketExists = false;

    /**
     * Constructor for AmazonS3 adapter
     *
     * @param \Aws\S3\S3Client $service
     * @param $bucket
     * @param string           $acl
     * @param array            $options
     */
    public function __construct(AmazonClient $service, $bucket, $acl = 'public-read', $localTmpDir, $options = array())
    {
        $this->service = $service;
        $this->bucket  = $bucket;
        $this->localTmpDir = $localTmpDir;
        $this->options = array_replace_recursive(
            array('create' => false, 'region' => 'eu-west-1', 'ACL' => $acl),
            $options
        );
    }

    /**
     * {@inheritDoc}
     */
    public function read($path)
    {
        // In this method only, $path can be absolute, for reading files from
        // outside of the file system directory, e.g. uploads, fixtures.
        if (file_exists($path)) {
            return file_get_contents($path);
        }

        list($path, $bucket) = $this->pathOrUrlToPath($path);

        $this->ensureBucketExists();

        $response = $this->service->getObject(array(
            'Bucket' => $bucket,
            'Key' => $path
        ));

        $response['Body']->rewind();

        return (string) $response['Body'];
    }

    /**
     * {@inheritDoc}
     */
    public function write($path, $source)
    {
        list($path, $bucket) = $this->pathOrUrlToPath($path);
<?php

function returnThreeValues() {
    return array('a', 'b', 'c');
}

list($a, $b, $c) = returnThreeValues();

print $a . " - " . $c;

        return $this->writeContent($path, EntityBody::factory(fopen($source, 'r')));
    }

    /**
     * {@inheritDoc}
     */
    public function writeContent($path, $content)
    {
        list($path, $bucket) = $this->pathOrUrlToPath($path);

        $this->ensureBucketExists();

        $response = $this->service->putObject(array(
$myVar = 'Value';
$higher = false;

if (rand(1, 6) > 3) {
    $higher = true;
} else {
    $higher = false;
}
            'Bucket' => $bucket,
            'Key' => $path,
            'Body' => $content,
            'ACL' => $this->options['ACL'],
            'ContentType' => Mimetypes::getInstance()->fromFilename($path)
        ));

        $this->service->waitUntilObjectExists(array(
            'Bucket' => $bucket,
            'Key'    => $path
        ));

        return $path;
    }

    /**
     * {@inheritDoc}
     */
    public function rename($sourcePath, $targetPath)
    {
        list($sourcePath, $sourceBucket) = $this->pathOrUrlToPath($sourcePath);
        list($targetPath, $targetBucket) = $this->pathOrUrlToPath($targetPath);

        $this->ensureBucketExists();

        $this->service->copyObject(array(
            'Bucket' => $targetBucket,
            'Key' => $targetPath,
            'CopySource' => urlencode($sourceBucket.'/'.$sourcePath),
            'ACL' => $this->options['ACL']
        ));

        $this->delete($sourcePath);

        return true;
    }

    /**
     * {@inheritDoc}
     */
    public function delete($path)
    {
        list($path, $bucket) = $this->pathOrUrlToPath($path);

        $this->ensureBucketExists();

        return $this->service->deleteObject(array(
class Author {
    private $name;

    public function __construct($name) {
        $this->name = $name;
    }

    public function getName() {
        return $this->name;
    }
}

abstract class Post {
    public function getAuthor() {
        return 'Johannes';
    }
}

class BlogPost extends Post {
    public function getAuthor() {
        return new Author('Johannes');
    }
}

class ForumPost extends Post { /* ... */ }

function my_function(Post $post) {
    echo strtoupper($post->getAuthor());
}
            'Bucket' => $bucket,
            'Key' => $path
        ));
    }

    /**
     * {@inheritDoc}
     */
    public function getFiles($directory = "")
    {
        list($directory, $bucket) = $this->pathOrUrlToPath($directory);

        $this->ensureBucketExists();

        $list = $this->service->getIterator('ListObjects', array(
            'Bucket' => $bucket, 'Prefix' => $directory
        ));

        $files = [];
        foreach ($list as $object) {
            $files[] = $object['Key'];
        }

        sort($files);

        return $files;
    }

    /**
     * {@inheritDoc}
     */
    public function copyFiles($sourceDir, $targetDir)
    {
        list($sourceDir, $sourceBucket) = $this->pathOrUrlToPath($sourceDir);
        list($targetDir, $targetBucket) = $this->pathOrUrlToPath($targetDir);

        /*
         * Add '/' character to the directories if necessary
         */
        $sourceDir = $sourceDir . (substr($sourceDir, -1) == '/' ? '' : '/');
        $targetDir = $targetDir . (substr($targetDir, -1) == '/' ? '' : '/');

        $this->ensureBucketExists();

        $files = $this->getFiles($sourceDir);

        $batch = array();
        for ($i = 0; $i < count($files); $i++) {
for ($i=0; $i<count($array); $i++) { // calls count() on each iteration
}

// Better
for ($i=0, $c=count($array); $i<$c; $i++) { // calls count() just once
}
            $targetFile = str_replace($sourceDir, "", $files[$i]);
            $batch[] =  $this->service->getCommand('CopyObject', array(
                'Bucket'     => $targetBucket,
                'Key'        => "{$targetDir}{$targetFile}",
                'CopySource' => "{$sourceBucket}/{$files[$i]}",
            ));
        }

        try {
            $this->service->execute($batch);
function acceptsInteger($int) { }

$x = '123'; // string "123"

// Instead of
acceptsInteger($x);

// we recommend to use
acceptsInteger((integer) $x);
        } catch (\Exception $e) {
            throw new \RuntimeException(sprintf('Failed to copy files from %s to %s.', $sourceDir, $targetDir));
        }

        return true;
    }

    /**
     * {@inheritDoc}
     */
    public function exists($path)
    {
        list($path, $bucket) = $this->pathOrUrlToPath($path);

        return $this->service->doesObjectExist($bucket, $path);
    }

    /**
     * {@inheritDoc}
     */
    public function isDirectory($path)
    {
        list($path, $bucket) = $this->pathOrUrlToPath($path);
<?php

function returnThreeValues() {
    return array('a', 'b', 'c');
}

list($a, $b, $c) = returnThreeValues();

print $a . " - " . $c;

        if ($this->exists($path.'/')) {
            return true;
        }

        return false;
    }

    /**
     * Calls the bucket exist method, and returns its result.
     *
     * @return boolean
     */
    public function checkBucket()
    {
        return $this->ensureBucketExists();
    }

    /**
    * Ensures the specified bucket exists. If is does not, and create is true, it will try to create it.
    *
    * @return boolean
    */
    private function ensureBucketExists()
    {
        if ($this->haveEnsuredBucketExists) {
            return true;
        }

        if ($this->service->doesBucketExist($this->bucket)) {
            $this->haveEnsuredBucketExists = true;

            return true;
        }

        if (!$this->options['create']) {
            throw new \RuntimeException(sprintf(
                'The configured bucket "%s" does not exist.',
                $this->bucket
            ));
        }

        $response = $this->service->createBucket(
            array('Bucket' => $this->bucket, 'LocationConstraint' => $this->options['region'])
        );

        $this->service->waitUntilBucketExists(array('Bucket' => $this->bucket));

        if (!$response['Location']) {
            throw new \RuntimeException(sprintf(
                'Failed to create the configured bucket "%s".',
                $this->bucket
            ));
        }

        $this->haveEnsuredBucketExists = true;

        return true;
    }

    /**
     * {@inheritDoc}
     */
    public function getURL($path)
    {
        list($path, $bucket) = $this->pathOrUrlToPath($path);

        return $this->service->getObjectUrl($bucket, $path);
    }

    /**
     * {@inheritDoc}
     */
    public function getFileSize($path)
    {
        list($path, $bucket) = $this->pathOrUrlToPath($path);

        return (int) $this->service->headObject([
            'Bucket' => $bucket,
            'Key' => $path,
        ])->get('ContentLength');
    }

    /**
     * {@inheritDoc}
     */
    public function copyToLocalTemporaryFile($path)

    {
        $content = $this->read($path);
        $extension = pathinfo($path, PATHINFO_EXTENSION);
        $target = tempnam($this->localTmpDir, null) . '.' . $extension;

        file_put_contents($target, $content);

        return $target;
    }
    
    /**
     * {@inheritDoc}
     */
    public function getService() {
        return $this->service;
    }

    /**
     * Returns an s3 location in normalised format, plus parses the bucket name
     * from the URL if a URL is used.
     *
     * @param  string $input
     * @return array  Contains two values:
     *                Index 0: the path to the file in S3.
     *                Index 1: the S3 bucket name.
     */
    private function pathOrUrlToPath($input)
    {
        $bucket = $this->bucket;

        if (empty($input)) {
            return ['', $bucket];
        }
        if (strpos($input, 'http://') === 0 || strpos($input, 'https://') === 0) {
            $path = parse_url($input, PHP_URL_PATH);

            /**
             *  Try to detect the bucket name from the hostname
             */
            $host = parse_url($input, PHP_URL_HOST);
            if (preg_match('/.s3[-\.a-z0-9]*\.amazonaws\.com$/', $host)) {
                $bucket = substr($host, 0, strpos($host, '.s3'));
            } else {
                $bucket = $host;
            }
        } else {
            $path = $input;
        }

        return [ltrim($path, '/'), $bucket];
    }
}


1			<?php
2
3			namespace Partnermarketing\FileSystemBundle\Adapter;
4
5			use Aws\S3\S3Client as AmazonClient;
6			use Guzzle\Http\EntityBody;
7			use Guzzle\Http\Mimetypes;
8
9			/**
10			* Amazon specific file system adapter
11			*/
12			class AmazonS3 implements AdapterInterface
13			{
14			private $service;
15			private $bucket;
16			private $localTmpDir;
17			private $options;
18			private $haveEnsuredBucketExists = false;
19
20			/**
21			* Constructor for AmazonS3 adapter
22			*
23			* @param \Aws\S3\S3Client $service
24			* @param $bucket
25			* @param string $acl
26			* @param array $options
27			*/
28	3		public function __construct(AmazonClient $service, $bucket, $acl = 'public-read', $localTmpDir, $options = array())
29			{
30	3		$this->service = $service;
31	3		$this->bucket = $bucket;
32	3		$this->localTmpDir = $localTmpDir;
33	3		$this->options = array_replace_recursive(
34	3		array('create' => false, 'region' => 'eu-west-1', 'ACL' => $acl),
35			$options
36	3		);
37	3		}
38
39			/**
40			* {@inheritDoc}
41			*/
42			public function read($path)
43			{
44			// In this method only, $path can be absolute, for reading files from
45			// outside of the file system directory, e.g. uploads, fixtures.
46			if (file_exists($path)) {
47			return file_get_contents($path);
48			}
49
50			list($path, $bucket) = $this->pathOrUrlToPath($path);
51
52			$this->ensureBucketExists();
53
54			$response = $this->service->getObject(array(
55			'Bucket' => $bucket,
56			'Key' => $path
57			));
58
59			$response['Body']->rewind();
60
61			return (string) $response['Body'];
62			}
63
64			/**
65			* {@inheritDoc}
66			*/
67			public function write($path, $source)
68			{
69			list($path, $bucket) = $this->pathOrUrlToPath($path);
			0 ignored issues – show Unused Code introduced 2017-01-19 17:16 UTC by Report Bug Copy Issue Report The assignment to `$bucket` is unused. Consider omitting it like so `list($first,,$third)`. This checks looks for assignemnts to variables using the `list(...)` function, where not all assigned variables are subsequently used. Consider the following code example. <?php function returnThreeValues() { return array('a', 'b', 'c'); } list($a, $b, $c) = returnThreeValues(); print $a . " - " . $c; Only the variables `$a` and `$c` are used. There was no need to assign `$b`. Instead, the list call could have been. list($a,, $c) = returnThreeValues(); Loading history...
70
71			return $this->writeContent($path, EntityBody::factory(fopen($source, 'r')));
72			}
73
74			/**
75			* {@inheritDoc}
76			*/
77			public function writeContent($path, $content)
78			{
79			list($path, $bucket) = $this->pathOrUrlToPath($path);
80
81			$this->ensureBucketExists();
82
83			$response = $this->service->putObject(array(
			0 ignored issues – show Unused Code introduced 2016-01-14 10:36 UTC by Report Bug Copy Issue Report `$response` is not used, you could remove the assignment. This check looks for variable assignements that are either overwritten by other assignments or where the variable is not used subsequently. $myVar = 'Value'; $higher = false; if (rand(1, 6) > 3) { $higher = true; } else { $higher = false; } Both the `$myVar` assignment in line 1 and the `$higher` assignment in line 2 are dead. The first because `$myVar` is never used and the second because `$higher` is always overwritten for every possible time line. Loading history...
84			'Bucket' => $bucket,
85			'Key' => $path,
86			'Body' => $content,
87			'ACL' => $this->options['ACL'],
88			'ContentType' => Mimetypes::getInstance()->fromFilename($path)
89			));
90
91			$this->service->waitUntilObjectExists(array(
92			'Bucket' => $bucket,
93			'Key' => $path
94			));
95
96			return $path;
97			}
98
99			/**
100			* {@inheritDoc}
101			*/
102			public function rename($sourcePath, $targetPath)
103			{
104			list($sourcePath, $sourceBucket) = $this->pathOrUrlToPath($sourcePath);
105			list($targetPath, $targetBucket) = $this->pathOrUrlToPath($targetPath);
106
107			$this->ensureBucketExists();
108
109			$this->service->copyObject(array(
110			'Bucket' => $targetBucket,
111			'Key' => $targetPath,
112			'CopySource' => urlencode($sourceBucket.'/'.$sourcePath),
113			'ACL' => $this->options['ACL']
114			));
115
116			$this->delete($sourcePath);
117
118			return true;
119			}
120
121			/**
122			* {@inheritDoc}
123			*/
124			public function delete($path)
125			{
126			list($path, $bucket) = $this->pathOrUrlToPath($path);
127
128			$this->ensureBucketExists();
129
130			return $this->service->deleteObject(array(
			0 ignored issues – show Bug Best Practice introduced 2017-01-19 18:28 UTC by Report Bug Copy Issue Report The return type of `return $this->service->d...cket, 'Key' => $path));` (`Aws\Result`) is incompatible with the return type declared by the interface `Partnermarketing\FileSys...dapterInterface::delete` of type `boolean`. If you return a value from a function or method, it should be a sub-type of the type that is given by the parent type f.e. an interface, or abstract method. This is more formally defined by the Lizkov substitution principle, and guarantees that classes that depend on the parent type can use any instance of a child type interchangably. This principle also belongs to the SOLID principles for object oriented design. Let’s take a look at an example: class Author { private $name; public function __construct($name) { $this->name = $name; } public function getName() { return $this->name; } } abstract class Post { public function getAuthor() { return 'Johannes'; } } class BlogPost extends Post { public function getAuthor() { return new Author('Johannes'); } } class ForumPost extends Post { /* ... */ } function my_function(Post $post) { echo strtoupper($post->getAuthor()); } Our function `my_function` expects a `Post` object, and outputs the author of the post. The base class `Post` returns a simple string and outputting a simple string will work just fine. However, the child class `BlogPost` which is a sub-type of `Post` instead decided to return an `object`, and is therefore violating the SOLID principles. If a `BlogPost` were passed to `my_function`, PHP would not complain, but ultimately fail when executing the `strtoupper` call in its body. Loading history...
131			'Bucket' => $bucket,
132			'Key' => $path
133			));
134			}
135
136			/**
137			* {@inheritDoc}
138			*/
139			public function getFiles($directory = "")
140			{
141			list($directory, $bucket) = $this->pathOrUrlToPath($directory);
142
143			$this->ensureBucketExists();
144
145			$list = $this->service->getIterator('ListObjects', array(
146			'Bucket' => $bucket, 'Prefix' => $directory
147			));
148
149			$files = [];
150			foreach ($list as $object) {
151			$files[] = $object['Key'];
152			}
153
154			sort($files);
155
156			return $files;
157			}
158
159			/**
160			* {@inheritDoc}
161			*/
162			public function copyFiles($sourceDir, $targetDir)
163			{
164			list($sourceDir, $sourceBucket) = $this->pathOrUrlToPath($sourceDir);
165			list($targetDir, $targetBucket) = $this->pathOrUrlToPath($targetDir);
166
167			/*
168			* Add '/' character to the directories if necessary
169			*/
170			$sourceDir = $sourceDir . (substr($sourceDir, -1) == '/' ? '' : '/');
171			$targetDir = $targetDir . (substr($targetDir, -1) == '/' ? '' : '/');
172
173			$this->ensureBucketExists();
174
175			$files = $this->getFiles($sourceDir);
176
177			$batch = array();
178			for ($i = 0; $i < count($files); $i++) {
			0 ignored issues – show Performance Best Practice introduced 2016-01-14 10:36 UTC by Report Bug Copy Issue Report It seems like you are calling the size function `count()` as part of the test condition. You might want to compute the size beforehand, and not on each iteration. If the size of the collection does not change during the iteration, it is generally a good practice to compute it beforehand, and not on each iteration: for ($i=0; $i<count($array); $i++) { // calls count() on each iteration } // Better for ($i=0, $c=count($array); $i<$c; $i++) { // calls count() just once } Loading history...
179			$targetFile = str_replace($sourceDir, "", $files[$i]);
180			$batch[] = $this->service->getCommand('CopyObject', array(
181			'Bucket' => $targetBucket,
182			'Key' => "{$targetDir}{$targetFile}",
183			'CopySource' => "{$sourceBucket}/{$files[$i]}",
184			));
185			}
186
187			try {
188			$this->service->execute($batch);
			0 ignored issues – show Documentation introduced 2017-01-19 18:28 UTC by Report Bug Copy Issue Report `$batch` is of type `array`, but the function expects a `object<Aws\CommandInterface>`. It seems like the type of the argument is not accepted by the function/method which you are calling. In some cases, in particular if PHP’s automatic type-juggling kicks in this might be fine. In other cases, however this might be a bug. We suggest to add an explicit type cast like in the following example: function acceptsInteger($int) { } $x = '123'; // string "123" // Instead of acceptsInteger($x); // we recommend to use acceptsInteger((integer) $x); Loading history...
189			} catch (\Exception $e) {
190			throw new \RuntimeException(sprintf('Failed to copy files from %s to %s.', $sourceDir, $targetDir));
191			}
192
193			return true;
194			}
195
196			/**
197			* {@inheritDoc}
198			*/
199			public function exists($path)
200			{
201			list($path, $bucket) = $this->pathOrUrlToPath($path);
202
203			return $this->service->doesObjectExist($bucket, $path);
204			}
205
206			/**
207			* {@inheritDoc}
208			*/
209			public function isDirectory($path)
210			{
211			list($path, $bucket) = $this->pathOrUrlToPath($path);
			0 ignored issues – show Unused Code introduced 2017-01-19 17:16 UTC by Report Bug Copy Issue Report The assignment to `$bucket` is unused. Consider omitting it like so `list($first,,$third)`. This checks looks for assignemnts to variables using the `list(...)` function, where not all assigned variables are subsequently used. Consider the following code example. <?php function returnThreeValues() { return array('a', 'b', 'c'); } list($a, $b, $c) = returnThreeValues(); print $a . " - " . $c; Only the variables `$a` and `$c` are used. There was no need to assign `$b`. Instead, the list call could have been. list($a,, $c) = returnThreeValues(); Loading history...
212
213			if ($this->exists($path.'/')) {
214			return true;
215			}
216
217			return false;
218			}
219
220			/**
221			* Calls the bucket exist method, and returns its result.
222			*
223			* @return boolean
224			*/
225			public function checkBucket()
226			{
227			return $this->ensureBucketExists();
228			}
229
230			/**
231			* Ensures the specified bucket exists. If is does not, and create is true, it will try to create it.
232			*
233			* @return boolean
234			*/
235			private function ensureBucketExists()
236			{
237			if ($this->haveEnsuredBucketExists) {
238			return true;
239			}
240
241			if ($this->service->doesBucketExist($this->bucket)) {
242			$this->haveEnsuredBucketExists = true;
243
244			return true;
245			}
246
247			if (!$this->options['create']) {
248			throw new \RuntimeException(sprintf(
249			'The configured bucket "%s" does not exist.',
250			$this->bucket
251			));
252			}
253
254			$response = $this->service->createBucket(
255			array('Bucket' => $this->bucket, 'LocationConstraint' => $this->options['region'])
256			);
257
258			$this->service->waitUntilBucketExists(array('Bucket' => $this->bucket));
259
260			if (!$response['Location']) {
261			throw new \RuntimeException(sprintf(
262			'Failed to create the configured bucket "%s".',
263			$this->bucket
264			));
265			}
266
267			$this->haveEnsuredBucketExists = true;
268
269			return true;
270			}
271
272			/**
273			* {@inheritDoc}
274			*/
275			public function getURL($path)
276			{
277			list($path, $bucket) = $this->pathOrUrlToPath($path);
278
279			return $this->service->getObjectUrl($bucket, $path);
280			}
281
282			/**
283			* {@inheritDoc}
284			*/
285			public function getFileSize($path)
286			{
287			list($path, $bucket) = $this->pathOrUrlToPath($path);
288
289			return (int) $this->service->headObject([
290			'Bucket' => $bucket,
291			'Key' => $path,
292			])->get('ContentLength');
293			}
294
295			/**
296			* {@inheritDoc}
297			*/
298		View Code Duplication	public function copyToLocalTemporaryFile($path)
			0 ignored issues – show Duplication introduced 2016-01-14 10:36 UTC by Report Bug Copy Issue Report This method seems to be duplicated in your project. Duplicated code is one of the most pungent code smells. If you need to duplicate the same code in three or more different places, we strongly encourage you to look into extracting the code into a single class or operation. You can also find more detailed suggestions in the “Code” section of your repository. Loading history...
299			{
300			$content = $this->read($path);
301			$extension = pathinfo($path, PATHINFO_EXTENSION);
302			$target = tempnam($this->localTmpDir, null) . '.' . $extension;
303
304			file_put_contents($target, $content);
305
306			return $target;
307			}
308
309			/**
310			* {@inheritDoc}
311			*/
312			public function getService() {
313			return $this->service;
314			}
315
316			/**
317			* Returns an s3 location in normalised format, plus parses the bucket name
318			* from the URL if a URL is used.
319			*
320			* @param string $input
321			* @return array Contains two values:
322			* Index 0: the path to the file in S3.
323			* Index 1: the S3 bucket name.
324			*/
325			private function pathOrUrlToPath($input)
326			{
327			$bucket = $this->bucket;
328
329			if (empty($input)) {
330			return ['', $bucket];
331			}
332			if (strpos($input, 'http://') === 0 \|\| strpos($input, 'https://') === 0) {
333			$path = parse_url($input, PHP_URL_PATH);
334
335			/**
336			* Try to detect the bucket name from the hostname
337			*/
338			$host = parse_url($input, PHP_URL_HOST);
339			if (preg_match('/.s3[-\.a-z0-9]*\.amazonaws\.com$/', $host)) {
340			$bucket = substr($host, 0, strpos($host, '.s3'));
341			} else {
342			$bucket = $host;
343			}
344			} else {
345			$path = $input;
346			}
347
348			return [ltrim($path, '/'), $bucket];
349			}
350			}
351

partnermarketing / PartnermarketingFileSystemBundle

Pull Request — master (#24)

AmazonS3::delete() A

Complexity

Size

Duplication

Code Coverage

Importance

Duplication Side-by-Side

Filter issues like