Completed
Push — master ( 947afa...ae5e03 )
by Jeroen
26s queued 14s
created

MediaController   D

Complexity

Total Complexity 58

Size/Duplication

Total Lines 509
Duplicated Lines 0 %

Coupling/Cohesion

Components 1
Dependencies 15

Test Coverage

Coverage 0%

Importance

Changes 0
Metric Value
wmc 58
lcom 1
cbo 15
dl 0
loc 509
ccs 0
cts 326
cp 0
rs 4.5599
c 0
b 0
f 0

11 Methods

Rating   Name   Duplication   Size   Complexity  
A showAction() 0 44 4
A deleteAction() 0 32 4
A bulkUploadAction() 0 9 1
F bulkUploadSubmitAction() 0 136 28
A returnJsonError() 0 13 1
A dropAction() 0 33 5
A createAction() 0 4 1
B createAndRedirect() 0 60 5
A createModalAction() 0 22 3
A moveMedia() 0 22 3
A bulkMoveAction() 0 37 3

How to fix   Complexity   

Complex Class

Complex classes like MediaController often do a lot of different things. To break such a class down, we need to identify a cohesive component within that class. A common approach to find such a component is to look for fields/methods that share the same prefixes, or suffixes. You can also have a look at the cohesion graph to spot any un-connected, or weakly-connected components.

Once you have determined the fields that belong together, you can apply the Extract Class refactoring. If the component makes sense as a sub-class, Extract Subclass is also a candidate, and is often faster.

While breaking up the class, it is a good idea to analyze how other classes use MediaController, and based on these observations, apply Extract Interface, too.

1
<?php
2
3
namespace Kunstmaan\MediaBundle\Controller;
4
5
use Exception;
6
use Kunstmaan\AdminBundle\FlashMessages\FlashTypes;
7
use Kunstmaan\MediaBundle\Entity\Folder;
8
use Kunstmaan\MediaBundle\Entity\Media;
9
use Kunstmaan\MediaBundle\Form\BulkMoveMediaType;
10
use Kunstmaan\MediaBundle\Helper\MediaManager;
11
use Symfony\Component\Routing\Annotation\Route;
12
use Sensio\Bundle\FrameworkExtraBundle\Configuration\Template;
13
use Symfony\Bundle\FrameworkBundle\Controller\Controller;
14
use Symfony\Component\HttpFoundation\File\File;
15
use Symfony\Component\HttpFoundation\JsonResponse;
16
use Symfony\Component\HttpFoundation\RedirectResponse;
17
use Symfony\Component\HttpFoundation\Request;
18
use Symfony\Component\HttpFoundation\Response;
19
20
/**
21
 * MediaController
22
 */
23
class MediaController extends Controller
0 ignored issues
show
Deprecated Code introduced by Kristof Van Cauwenbergh
The class Symfony\Bundle\Framework...e\Controller\Controller has been deprecated with message: since Symfony 4.2, use "Symfony\Bundle\FrameworkBundle\Controller\AbstractController" instead.

This class, trait or interface has been deprecated. The supplier of the file has supplied an explanatory message.

The explanatory message should give you some clue as to whether and when the type will be removed from the class and what other constant to use instead.

Loading history...
24
{
25
    /**
26
     * @param Request $request
27
     * @param int     $mediaId
28
     *
29
     * @Route("/{mediaId}", requirements={"mediaId" = "\d+"}, name="KunstmaanMediaBundle_media_show")
30
     *
31
     * @return Response
32
     */
33
    public function showAction(Request $request, $mediaId)
34
    {
35
        $em = $this->getDoctrine()->getManager();
36
37
        /* @var Media $media */
38
        $media = $em->getRepository(Media::class)->getMedia($mediaId);
0 ignored issues
show
Bug introduced by Jeroen Thora
It seems like you code against a concrete implementation and not the interface Doctrine\Persistence\ObjectRepository as the method getMedia() does only exist in the following implementations of said interface: Kunstmaan\MediaBundle\Repository\MediaRepository.

Let’s take a look at an example:

interface User
{
    /** @return string */
    public function getPassword();
}

class MyUser implements User
{
    public function getPassword()
    {
        // return something
    }

    public function getDisplayName()
    {
        // return some name.
    }
}

class AuthSystem
{
    public function authenticate(User $user)
    {
        $this->logger->info(sprintf('Authenticating %s.', $user->getDisplayName()));
        // do something.
    }
}

In the above example, the authenticate() method works fine as long as you just pass instances of MyUser. However, if you now also want to pass a different implementation of User which does not have a getDisplayName() method, the code will break.

Available Fixes

  1. Change the type-hint for the parameter:

    class AuthSystem
    {
        public function authenticate(MyUser $user) { /* ... */ }
    }
    
  2. Add an additional type-check:

    class AuthSystem
    {
        public function authenticate(User $user)
        {
            if ($user instanceof MyUser) {
                $this->logger->info(/** ... */);
            }
    
            // or alternatively
            if ( ! $user instanceof MyUser) {
                throw new \LogicException(
                    '$user must be an instance of MyUser, '
                   .'other instances are not supported.'
                );
            }
    
        }
    }
    
Note: PHP Analyzer uses reverse abstract interpretation to narrow down the types inside the if block in such a case.
  1. Add the method to the interface:

    interface User
    {
        /** @return string */
        public function getPassword();
    
        /** @return string */
        public function getDisplayName();
    }
    
Loading history...
39
        $folder = $media->getFolder();
40
41
        /* @var MediaManager $mediaManager */
42
        $mediaManager = $this->get('kunstmaan_media.media_manager');
43
        $handler = $mediaManager->getHandler($media);
44
        $helper = $handler->getFormHelper($media);
45
46
        $form = $this->createForm($handler->getFormType(), $helper, $handler->getFormTypeOptions());
47
48
        if ($request->isMethod('POST')) {
49
            $form->handleRequest($request);
50
            if ($form->isSubmitted() && $form->isValid()) {
51
                $media = $helper->getMedia();
52
                $em->getRepository(Media::class)->save($media);
0 ignored issues
show
Bug introduced by Jeroen Thora
It seems like you code against a concrete implementation and not the interface Doctrine\Persistence\ObjectRepository as the method save() does only exist in the following implementations of said interface: Kunstmaan\MediaBundle\Repository\FolderRepository, Kunstmaan\MediaBundle\Repository\MediaRepository.

Let’s take a look at an example:

interface User
{
    /** @return string */
    public function getPassword();
}

class MyUser implements User
{
    public function getPassword()
    {
        // return something
    }

    public function getDisplayName()
    {
        // return some name.
    }
}

class AuthSystem
{
    public function authenticate(User $user)
    {
        $this->logger->info(sprintf('Authenticating %s.', $user->getDisplayName()));
        // do something.
    }
}

In the above example, the authenticate() method works fine as long as you just pass instances of MyUser. However, if you now also want to pass a different implementation of User which does not have a getDisplayName() method, the code will break.

Available Fixes

  1. Change the type-hint for the parameter:

    class AuthSystem
    {
        public function authenticate(MyUser $user) { /* ... */ }
    }
    
  2. Add an additional type-check:

    class AuthSystem
    {
        public function authenticate(User $user)
        {
            if ($user instanceof MyUser) {
                $this->logger->info(/** ... */);
            }
    
            // or alternatively
            if ( ! $user instanceof MyUser) {
                throw new \LogicException(
                    '$user must be an instance of MyUser, '
                   .'other instances are not supported.'
                );
            }
    
        }
    }
    
Note: PHP Analyzer uses reverse abstract interpretation to narrow down the types inside the if block in such a case.
  1. Add the method to the interface:

    interface User
    {
        /** @return string */
        public function getPassword();
    
        /** @return string */
        public function getDisplayName();
    }
    
Loading history...
53
54
                return new RedirectResponse(
55
                    $this->generateUrl(
56
                        'KunstmaanMediaBundle_media_show',
57
                        ['mediaId' => $media->getId()]
58
                    )
59
                );
60
            }
61
        }
62
        $showTemplate = $mediaManager->getHandler($media)->getShowTemplate($media);
63
64
        return $this->render(
65
            $showTemplate,
66
            [
67
                'handler' => $handler,
68
                'foldermanager' => $this->get('kunstmaan_media.folder_manager'),
69
                'mediamanager' => $this->get('kunstmaan_media.media_manager'),
70
                'editform' => $form->createView(),
71
                'media' => $media,
72
                'helper' => $helper,
73
                'folder' => $folder,
74
            ]
75
        );
76
    }
77
78
    /**
79
     * @param Request $request
80
     * @param int     $mediaId
81
     *
82
     * @Route("/delete/{mediaId}", requirements={"mediaId" = "\d+"}, name="KunstmaanMediaBundle_media_delete")
83
     *
84
     * @return RedirectResponse
85
     */
86
    public function deleteAction(Request $request, $mediaId)
87
    {
88
        $em = $this->getDoctrine()->getManager();
89
90
        /* @var Media $media */
91
        $media = $em->getRepository(Media::class)->getMedia($mediaId);
0 ignored issues
show
Bug introduced by Jeroen Thora
It seems like you code against a concrete implementation and not the interface Doctrine\Persistence\ObjectRepository as the method getMedia() does only exist in the following implementations of said interface: Kunstmaan\MediaBundle\Repository\MediaRepository.

Let’s take a look at an example:

interface User
{
    /** @return string */
    public function getPassword();
}

class MyUser implements User
{
    public function getPassword()
    {
        // return something
    }

    public function getDisplayName()
    {
        // return some name.
    }
}

class AuthSystem
{
    public function authenticate(User $user)
    {
        $this->logger->info(sprintf('Authenticating %s.', $user->getDisplayName()));
        // do something.
    }
}

In the above example, the authenticate() method works fine as long as you just pass instances of MyUser. However, if you now also want to pass a different implementation of User which does not have a getDisplayName() method, the code will break.

Available Fixes

  1. Change the type-hint for the parameter:

    class AuthSystem
    {
        public function authenticate(MyUser $user) { /* ... */ }
    }
    
  2. Add an additional type-check:

    class AuthSystem
    {
        public function authenticate(User $user)
        {
            if ($user instanceof MyUser) {
                $this->logger->info(/** ... */);
            }
    
            // or alternatively
            if ( ! $user instanceof MyUser) {
                throw new \LogicException(
                    '$user must be an instance of MyUser, '
                   .'other instances are not supported.'
                );
            }
    
        }
    }
    
Note: PHP Analyzer uses reverse abstract interpretation to narrow down the types inside the if block in such a case.
  1. Add the method to the interface:

    interface User
    {
        /** @return string */
        public function getPassword();
    
        /** @return string */
        public function getDisplayName();
    }
    
Loading history...
92
        $medianame = $media->getName();
93
        $folder = $media->getFolder();
94
95
        $em->getRepository(Media::class)->delete($media);
0 ignored issues
show
Bug introduced by Jeroen Thora
It seems like you code against a concrete implementation and not the interface Doctrine\Persistence\ObjectRepository as the method delete() does only exist in the following implementations of said interface: Kunstmaan\MediaBundle\Repository\FolderRepository, Kunstmaan\MediaBundle\Repository\MediaRepository.

Let’s take a look at an example:

interface User
{
    /** @return string */
    public function getPassword();
}

class MyUser implements User
{
    public function getPassword()
    {
        // return something
    }

    public function getDisplayName()
    {
        // return some name.
    }
}

class AuthSystem
{
    public function authenticate(User $user)
    {
        $this->logger->info(sprintf('Authenticating %s.', $user->getDisplayName()));
        // do something.
    }
}

In the above example, the authenticate() method works fine as long as you just pass instances of MyUser. However, if you now also want to pass a different implementation of User which does not have a getDisplayName() method, the code will break.

Available Fixes

  1. Change the type-hint for the parameter:

    class AuthSystem
    {
        public function authenticate(MyUser $user) { /* ... */ }
    }
    
  2. Add an additional type-check:

    class AuthSystem
    {
        public function authenticate(User $user)
        {
            if ($user instanceof MyUser) {
                $this->logger->info(/** ... */);
            }
    
            // or alternatively
            if ( ! $user instanceof MyUser) {
                throw new \LogicException(
                    '$user must be an instance of MyUser, '
                   .'other instances are not supported.'
                );
            }
    
        }
    }
    
Note: PHP Analyzer uses reverse abstract interpretation to narrow down the types inside the if block in such a case.
  1. Add the method to the interface:

    interface User
    {
        /** @return string */
        public function getPassword();
    
        /** @return string */
        public function getDisplayName();
    }
    
Loading history...
96
97
        $this->addFlash(
98
            FlashTypes::SUCCESS,
99
            $this->get('translator')->trans(
100
                'kuma_admin.media.flash.deleted_success.%medianame%',
101
                [
102
                    '%medianame%' => $medianame,
103
                ]
104
            )
105
        );
106
107
        // If the redirect url is passed via the url we use it
108
        $redirectUrl = $request->query->get('redirectUrl');
109
        if (empty($redirectUrl) || (\strpos($redirectUrl, $request->getSchemeAndHttpHost()) !== 0 && strncmp($redirectUrl, '/', 1) !== 0)) {
110
            $redirectUrl = $this->generateUrl(
111
                'KunstmaanMediaBundle_folder_show',
112
                ['folderId' => $folder->getId()]
113
            );
114
        }
115
116
        return new RedirectResponse($redirectUrl);
117
    }
118
119
    /**
120
     * @param int $folderId
121
     *
122
     * @Route("bulkupload/{folderId}", requirements={"folderId" = "\d+"}, name="KunstmaanMediaBundle_media_bulk_upload")
123
     * @Template("@KunstmaanMedia/Media/bulkUpload.html.twig")
124
     *
125
     * @return array|RedirectResponse
126
     */
127
    public function bulkUploadAction($folderId)
128
    {
129
        $em = $this->getDoctrine()->getManager();
130
131
        /* @var Folder $folder */
132
        $folder = $em->getRepository(Folder::class)->getFolder($folderId);
0 ignored issues
show
Bug introduced by Jeroen Thora
It seems like you code against a concrete implementation and not the interface Doctrine\Persistence\ObjectRepository as the method getFolder() does only exist in the following implementations of said interface: Kunstmaan\MediaBundle\Repository\FolderRepository.

Let’s take a look at an example:

interface User
{
    /** @return string */
    public function getPassword();
}

class MyUser implements User
{
    public function getPassword()
    {
        // return something
    }

    public function getDisplayName()
    {
        // return some name.
    }
}

class AuthSystem
{
    public function authenticate(User $user)
    {
        $this->logger->info(sprintf('Authenticating %s.', $user->getDisplayName()));
        // do something.
    }
}

In the above example, the authenticate() method works fine as long as you just pass instances of MyUser. However, if you now also want to pass a different implementation of User which does not have a getDisplayName() method, the code will break.

Available Fixes

  1. Change the type-hint for the parameter:

    class AuthSystem
    {
        public function authenticate(MyUser $user) { /* ... */ }
    }
    
  2. Add an additional type-check:

    class AuthSystem
    {
        public function authenticate(User $user)
        {
            if ($user instanceof MyUser) {
                $this->logger->info(/** ... */);
            }
    
            // or alternatively
            if ( ! $user instanceof MyUser) {
                throw new \LogicException(
                    '$user must be an instance of MyUser, '
                   .'other instances are not supported.'
                );
            }
    
        }
    }
    
Note: PHP Analyzer uses reverse abstract interpretation to narrow down the types inside the if block in such a case.
  1. Add the method to the interface:

    interface User
    {
        /** @return string */
        public function getPassword();
    
        /** @return string */
        public function getDisplayName();
    }
    
Loading history...
133
134
        return ['folder' => $folder];
135
    }
136
137
    /**
138
     * @param Request $request
139
     * @param int     $folderId
140
     *
141
     * @Route("bulkuploadsubmit/{folderId}", requirements={"folderId" = "\d+"}, name="KunstmaanMediaBundle_media_bulk_upload_submit", methods={"POST"})
142
     *
143
     * @return JsonResponse
144
     */
145
    public function bulkUploadSubmitAction(Request $request, $folderId)
146
    {
147
        // Settings
148
        if (\ini_get('upload_tmp_dir')) {
149
            $tempDir = \ini_get('upload_tmp_dir');
150
        } else {
151
            $tempDir = \sys_get_temp_dir();
152
        }
153
        $targetDir = \rtrim($tempDir, '/').DIRECTORY_SEPARATOR.'plupload';
154
        $cleanupTargetDir = true; // Remove old files
155
        $maxFileAge = 5 * 60 * 60; // Temp file age in seconds
156
157
        // Create target dir
158
        if (!\file_exists($targetDir)) {
159
            @\mkdir($targetDir);
0 ignored issues
show
Security Best Practice introduced by Derek Stephen McLean
It seems like you do not handle an error condition here. This can introduce security issues, and is generally not recommended.

If you suppress an error, we recommend checking for the error condition explicitly:

// For example instead of
@mkdir($dir);

// Better use
if (@mkdir($dir) === false) {
    throw new \RuntimeException('The directory '.$dir.' could not be created.');
}
Loading history...
160
        }
161
162
        $submittedToken = $request->headers->get('x-upload-token');
163
        if (!$this->isCsrfTokenValid('bulk-upload-media', $submittedToken)) {
0 ignored issues
show
Bug introduced by Kevin Jossart
It seems like $submittedToken defined by $request->headers->get('x-upload-token') on line 162 can also be of type array; however, Symfony\Bundle\Framework...ait::isCsrfTokenValid() does only seem to accept string|null, maybe add an additional type check?

If a method or function can return multiple different values and unless you are sure that you only can receive a single value in this context, we recommend to add an additional type check:

/**
 * @return array|string
 */
function returnsDifferentValues($x) {
    if ($x) {
        return 'foo';
    }

    return array();
}

$x = returnsDifferentValues($y);
if (is_array($x)) {
    // $x is an array.
}

If this a common case that PHP Analyzer should handle natively, please let us know by opening an issue.

Loading history...
164
            return $this->returnJsonError('105', 'Could not verify token');
165
        }
166
167
        // Get a file name
168
        if ($request->request->has('name')) {
169
            $fileName = $request->request->get('name');
170
        } elseif (0 !== $request->files->count()) {
171
            $fileName = $request->files->get('file')['name'];
172
        } else {
173
            $fileName = \uniqid('file_', false);
174
        }
175
        $filePath = $targetDir.DIRECTORY_SEPARATOR.$fileName;
176
177
        $chunk = 0;
178
        $chunks = 0;
179
        // Chunking might be enabled
180
        if ($request->request->has('chunk')) {
181
            $chunk = $request->request->getInt('chunk');
182
        }
183
        if ($request->request->has('chunks')) {
184
            $chunks = $request->request->getInt('chunks');
185
        }
186
187
        // Remove old temp files
188
        if ($cleanupTargetDir) {
189
            if (!\is_dir($targetDir) || !$dir = \opendir($targetDir)) {
190
                return $this->returnJsonError('100', 'Failed to open temp directory.');
191
            }
192
193
            while (($file = \readdir($dir)) !== false) {
194
                $tmpFilePath = $targetDir.DIRECTORY_SEPARATOR.$file;
195
196
                // If temp file is current file proceed to the next
197
                if ($tmpFilePath === "{$filePath}.part") {
198
                    continue;
199
                }
200
201
                // Remove temp file if it is older than the max age and is not the current file
202
                if (\preg_match('/\.part$/', $file) && (\filemtime($tmpFilePath) < \time() - $maxFileAge)) {
203
                    $success = @\unlink($tmpFilePath);
204
                    if ($success !== true) {
205
                        return $this->returnJsonError('106', 'Could not remove temp file: '.$filePath);
206
                    }
207
                }
208
            }
209
            \closedir($dir);
210
        }
211
212
        // Open temp file
213
        if (!$out = @\fopen("{$filePath}.part", $chunks ? 'ab' : 'wb')) {
214
            return $this->returnJsonError('102', 'Failed to open output stream.');
215
        }
216
217
        if (0 !== $request->files->count()) {
218
            $_file = $request->files->get('file');
219
            if ($_file->getError() > 0 || !\is_uploaded_file($_file->getRealPath())) {
220
                return $this->returnJsonError('103', 'Failed to move uploaded file.');
221
            }
222
223
            // Read binary input stream and append it to temp file
224
            if (!$input = @\fopen($_file->getRealPath(), 'rb')) {
225
                return $this->returnJsonError('101', 'Failed to open input stream.');
226
            }
227
        } else {
228
            if (!$input = @\fopen('php://input', 'rb')) {
229
                return $this->returnJsonError('101', 'Failed to open input stream.');
230
            }
231
        }
232
233
        while ($buff = \fread($input, 4096)) {
234
            \fwrite($out, $buff);
235
        }
236
237
        @\fclose($out);
0 ignored issues
show
Security Best Practice introduced by Roderik van der Veer
It seems like you do not handle an error condition here. This can introduce security issues, and is generally not recommended.

If you suppress an error, we recommend checking for the error condition explicitly:

// For example instead of
@mkdir($dir);

// Better use
if (@mkdir($dir) === false) {
    throw new \RuntimeException('The directory '.$dir.' could not be created.');
}
Loading history...
238
        @\fclose($input);
0 ignored issues
show
Security Best Practice introduced by whitewhidow
It seems like you do not handle an error condition here. This can introduce security issues, and is generally not recommended.

If you suppress an error, we recommend checking for the error condition explicitly:

// For example instead of
@mkdir($dir);

// Better use
if (@mkdir($dir) === false) {
    throw new \RuntimeException('The directory '.$dir.' could not be created.');
}
Loading history...
239
240
        // Check if file has been uploaded
241
        if (!$chunks || $chunk === $chunks - 1) {
242
            // Strip the temp .part suffix off
243
            \rename("{$filePath}.part", $filePath);
244
        }
245
246
        $em = $this->getDoctrine()->getManager();
247
        /* @var Folder $folder */
248
        $folder = $em->getRepository(Folder::class)->getFolder($folderId);
0 ignored issues
show
Bug introduced by Jeroen Thora
It seems like you code against a concrete implementation and not the interface Doctrine\Persistence\ObjectRepository as the method getFolder() does only exist in the following implementations of said interface: Kunstmaan\MediaBundle\Repository\FolderRepository.

Let’s take a look at an example:

interface User
{
    /** @return string */
    public function getPassword();
}

class MyUser implements User
{
    public function getPassword()
    {
        // return something
    }

    public function getDisplayName()
    {
        // return some name.
    }
}

class AuthSystem
{
    public function authenticate(User $user)
    {
        $this->logger->info(sprintf('Authenticating %s.', $user->getDisplayName()));
        // do something.
    }
}

In the above example, the authenticate() method works fine as long as you just pass instances of MyUser. However, if you now also want to pass a different implementation of User which does not have a getDisplayName() method, the code will break.

Available Fixes

  1. Change the type-hint for the parameter:

    class AuthSystem
    {
        public function authenticate(MyUser $user) { /* ... */ }
    }
    
  2. Add an additional type-check:

    class AuthSystem
    {
        public function authenticate(User $user)
        {
            if ($user instanceof MyUser) {
                $this->logger->info(/** ... */);
            }
    
            // or alternatively
            if ( ! $user instanceof MyUser) {
                throw new \LogicException(
                    '$user must be an instance of MyUser, '
                   .'other instances are not supported.'
                );
            }
    
        }
    }
    
Note: PHP Analyzer uses reverse abstract interpretation to narrow down the types inside the if block in such a case.
  1. Add the method to the interface:

    interface User
    {
        /** @return string */
        public function getPassword();
    
        /** @return string */
        public function getDisplayName();
    }
    
Loading history...
249
        $file = new File($filePath);
250
251
        try {
252
            /* @var Media $media */
253
            $media = $this->get('kunstmaan_media.media_manager')->getHandler($file)->createNew($file);
254
            $media->setFolder($folder);
255
            $em->getRepository(Media::class)->save($media);
0 ignored issues
show
Bug introduced by Kajetan
It seems like you code against a concrete implementation and not the interface Doctrine\Persistence\ObjectRepository as the method save() does only exist in the following implementations of said interface: Kunstmaan\MediaBundle\Repository\FolderRepository, Kunstmaan\MediaBundle\Repository\MediaRepository.

Let’s take a look at an example:

interface User
{
    /** @return string */
    public function getPassword();
}

class MyUser implements User
{
    public function getPassword()
    {
        // return something
    }

    public function getDisplayName()
    {
        // return some name.
    }
}

class AuthSystem
{
    public function authenticate(User $user)
    {
        $this->logger->info(sprintf('Authenticating %s.', $user->getDisplayName()));
        // do something.
    }
}

In the above example, the authenticate() method works fine as long as you just pass instances of MyUser. However, if you now also want to pass a different implementation of User which does not have a getDisplayName() method, the code will break.

Available Fixes

  1. Change the type-hint for the parameter:

    class AuthSystem
    {
        public function authenticate(MyUser $user) { /* ... */ }
    }
    
  2. Add an additional type-check:

    class AuthSystem
    {
        public function authenticate(User $user)
        {
            if ($user instanceof MyUser) {
                $this->logger->info(/** ... */);
            }
    
            // or alternatively
            if ( ! $user instanceof MyUser) {
                throw new \LogicException(
                    '$user must be an instance of MyUser, '
                   .'other instances are not supported.'
                );
            }
    
        }
    }
    
Note: PHP Analyzer uses reverse abstract interpretation to narrow down the types inside the if block in such a case.
  1. Add the method to the interface:

    interface User
    {
        /** @return string */
        public function getPassword();
    
        /** @return string */
        public function getDisplayName();
    }
    
Loading history...
256
        } catch (Exception $e) {
257
            return $this->returnJsonError('104', 'Failed performing save on media-manager');
258
        }
259
260
        $success = \unlink($filePath);
261
        if ($success !== true) {
262
            return $this->returnJsonError('105', 'Could not remove temp file: '.$filePath);
263
        }
264
265
        // Send headers making sure that the file is not cached (as it happens for example on iOS devices)
266
        $response = new JsonResponse(
267
            [
268
                'jsonrpc' => '2.0',
269
                'result' => '',
270
                'id' => 'id',
271
            ], JsonResponse::HTTP_OK, [
272
                'Expires' => 'Mon, 26 Jul 1997 05:00:00 GMT',
273
                'Last-Modified' => \gmdate('D, d M Y H:i:s').' GMT',
274
                'Cache-Control' => 'no-store, no-cache, must-revalidate, post-check=0, pre-check=0',
275
                'Pragma' => 'no-cache',
276
            ]
277
        );
278
279
        return $response;
280
    }
281
282
    private function returnJsonError($code, $message)
283
    {
284
        return new JsonResponse(
285
            [
286
                'jsonrpc' => '2.0',
287
                'error ' => [
288
                    'code' => $code,
289
                    'message' => $message,
290
                ],
291
                'id' => 'id',
292
            ]
293
        );
294
    }
295
296
    /**
297
     * @param Request $request
298
     * @param int     $folderId
299
     *
300
     * @Route("drop/{folderId}", requirements={"folderId" = "\d+"}, name="KunstmaanMediaBundle_media_drop_upload", methods={"GET", "POST"})
301
     *
302
     * @return JsonResponse
303
     */
304
    public function dropAction(Request $request, $folderId)
305
    {
306
        $em = $this->getDoctrine()->getManager();
307
308
        /* @var Folder $folder */
309
        $folder = $em->getRepository(Folder::class)->getFolder($folderId);
0 ignored issues
show
Bug introduced by Jeroen Thora
It seems like you code against a concrete implementation and not the interface Doctrine\Persistence\ObjectRepository as the method getFolder() does only exist in the following implementations of said interface: Kunstmaan\MediaBundle\Repository\FolderRepository.

Let’s take a look at an example:

interface User
{
    /** @return string */
    public function getPassword();
}

class MyUser implements User
{
    public function getPassword()
    {
        // return something
    }

    public function getDisplayName()
    {
        // return some name.
    }
}

class AuthSystem
{
    public function authenticate(User $user)
    {
        $this->logger->info(sprintf('Authenticating %s.', $user->getDisplayName()));
        // do something.
    }
}

In the above example, the authenticate() method works fine as long as you just pass instances of MyUser. However, if you now also want to pass a different implementation of User which does not have a getDisplayName() method, the code will break.

Available Fixes

  1. Change the type-hint for the parameter:

    class AuthSystem
    {
        public function authenticate(MyUser $user) { /* ... */ }
    }
    
  2. Add an additional type-check:

    class AuthSystem
    {
        public function authenticate(User $user)
        {
            if ($user instanceof MyUser) {
                $this->logger->info(/** ... */);
            }
    
            // or alternatively
            if ( ! $user instanceof MyUser) {
                throw new \LogicException(
                    '$user must be an instance of MyUser, '
                   .'other instances are not supported.'
                );
            }
    
        }
    }
    
Note: PHP Analyzer uses reverse abstract interpretation to narrow down the types inside the if block in such a case.
  1. Add the method to the interface:

    interface User
    {
        /** @return string */
        public function getPassword();
    
        /** @return string */
        public function getDisplayName();
    }
    
Loading history...
310
311
        $drop = null;
0 ignored issues
show
Unused Code introduced by Kris Pypen
$drop 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...
312
313
        if ($request->files->has('files') && $request->files->get('files')['error'] === 0) {
314
            $drop = $request->files->get('files');
315
        } else {
316
            if ($request->files->get('file')) {
317
                $drop = $request->files->get('file');
318
            } else {
319
                $drop = $request->get('text');
320
            }
321
        }
322
        $media = $this->get('kunstmaan_media.media_manager')->createNew($drop);
323
        if ($media) {
324
            $media->setFolder($folder);
325
            $em->getRepository(Media::class)->save($media);
0 ignored issues
show
Bug introduced by Jeroen Thora
It seems like you code against a concrete implementation and not the interface Doctrine\Persistence\ObjectRepository as the method save() does only exist in the following implementations of said interface: Kunstmaan\MediaBundle\Repository\FolderRepository, Kunstmaan\MediaBundle\Repository\MediaRepository.

Let’s take a look at an example:

interface User
{
    /** @return string */
    public function getPassword();
}

class MyUser implements User
{
    public function getPassword()
    {
        // return something
    }

    public function getDisplayName()
    {
        // return some name.
    }
}

class AuthSystem
{
    public function authenticate(User $user)
    {
        $this->logger->info(sprintf('Authenticating %s.', $user->getDisplayName()));
        // do something.
    }
}

In the above example, the authenticate() method works fine as long as you just pass instances of MyUser. However, if you now also want to pass a different implementation of User which does not have a getDisplayName() method, the code will break.

Available Fixes

  1. Change the type-hint for the parameter:

    class AuthSystem
    {
        public function authenticate(MyUser $user) { /* ... */ }
    }
    
  2. Add an additional type-check:

    class AuthSystem
    {
        public function authenticate(User $user)
        {
            if ($user instanceof MyUser) {
                $this->logger->info(/** ... */);
            }
    
            // or alternatively
            if ( ! $user instanceof MyUser) {
                throw new \LogicException(
                    '$user must be an instance of MyUser, '
                   .'other instances are not supported.'
                );
            }
    
        }
    }
    
Note: PHP Analyzer uses reverse abstract interpretation to narrow down the types inside the if block in such a case.
  1. Add the method to the interface:

    interface User
    {
        /** @return string */
        public function getPassword();
    
        /** @return string */
        public function getDisplayName();
    }
    
Loading history...
326
327
            return new JsonResponse(['status' => $this->get('translator')->trans('kuma_admin.media.flash.drop_success')]);
328
        }
329
330
        $request->getSession()->getFlashBag()->add(
331
            FlashTypes::DANGER,
332
            $this->get('translator')->trans('kuma_admin.media.flash.drop_unrecognized')
333
        );
334
335
        return new JsonResponse(['status' => $this->get('translator')->trans('kuma_admin.media.flash.drop_unrecognized')]);
336
    }
337
338
    /**
339
     * @param Request $request
340
     * @param int     $folderId The folder id
341
     * @param string  $type     The type
342
     *
343
     * @Route("create/{folderId}/{type}", requirements={"folderId" = "\d+", "type" = ".+"}, name="KunstmaanMediaBundle_media_create", methods={"GET", "POST"})
344
     * @Template("@KunstmaanMedia/Media/create.html.twig")
345
     *
346
     * @return array|RedirectResponse
0 ignored issues
show
Documentation introduced by Wim Vandersmissen
Consider making the return type a bit more specific; maybe use RedirectResponse|array<s...t\Form\FormView|Folder>.

This check looks for the generic type array as a return type and suggests a more specific type. This type is inferred from the actual code.

Loading history...
347
     */
348
    public function createAction(Request $request, $folderId, $type)
349
    {
350
        return $this->createAndRedirect($request, $folderId, $type, 'KunstmaanMediaBundle_folder_show');
351
    }
352
353
    /**
354
     * @param Request $request
355
     * @param int     $folderId    The folder Id
356
     * @param string  $type        The type
357
     * @param string  $redirectUrl The url where we want to redirect to on success
358
     * @param array   $extraParams The extra parameters that will be passed wen redirecting
359
     *
360
     * @return array|RedirectResponse
0 ignored issues
show
Documentation introduced by Wim Vandersmissen
Consider making the return type a bit more specific; maybe use RedirectResponse|array<s...t\Form\FormView|Folder>.

This check looks for the generic type array as a return type and suggests a more specific type. This type is inferred from the actual code.

Loading history...
361
     */
362
    private function createAndRedirect(Request $request, $folderId, $type, $redirectUrl, $extraParams = [], $isInModal = false)
363
    {
364
        $em = $this->getDoctrine()->getManager();
365
366
        /* @var Folder $folder */
367
        $folder = $em->getRepository(Folder::class)->getFolder($folderId);
0 ignored issues
show
Bug introduced by Jeroen Thora
It seems like you code against a concrete implementation and not the interface Doctrine\Persistence\ObjectRepository as the method getFolder() does only exist in the following implementations of said interface: Kunstmaan\MediaBundle\Repository\FolderRepository.

Let’s take a look at an example:

interface User
{
    /** @return string */
    public function getPassword();
}

class MyUser implements User
{
    public function getPassword()
    {
        // return something
    }

    public function getDisplayName()
    {
        // return some name.
    }
}

class AuthSystem
{
    public function authenticate(User $user)
    {
        $this->logger->info(sprintf('Authenticating %s.', $user->getDisplayName()));
        // do something.
    }
}

In the above example, the authenticate() method works fine as long as you just pass instances of MyUser. However, if you now also want to pass a different implementation of User which does not have a getDisplayName() method, the code will break.

Available Fixes

  1. Change the type-hint for the parameter:

    class AuthSystem
    {
        public function authenticate(MyUser $user) { /* ... */ }
    }
    
  2. Add an additional type-check:

    class AuthSystem
    {
        public function authenticate(User $user)
        {
            if ($user instanceof MyUser) {
                $this->logger->info(/** ... */);
            }
    
            // or alternatively
            if ( ! $user instanceof MyUser) {
                throw new \LogicException(
                    '$user must be an instance of MyUser, '
                   .'other instances are not supported.'
                );
            }
    
        }
    }
    
Note: PHP Analyzer uses reverse abstract interpretation to narrow down the types inside the if block in such a case.
  1. Add the method to the interface:

    interface User
    {
        /** @return string */
        public function getPassword();
    
        /** @return string */
        public function getDisplayName();
    }
    
Loading history...
368
369
        /* @var MediaManager $mediaManager */
370
        $mediaManager = $this->get('kunstmaan_media.media_manager');
371
        $handler = $mediaManager->getHandlerForType($type);
372
        $media = new Media();
373
        $helper = $handler->getFormHelper($media);
374
375
        $form = $this->createForm($handler->getFormType(), $helper, $handler->getFormTypeOptions());
376
377
        if ($request->isMethod('POST')) {
378
            $params = ['folderId' => $folder->getId()];
379
            $params = \array_merge($params, $extraParams);
380
381
            $form->handleRequest($request);
382
383
            if ($form->isSubmitted() && $form->isValid()) {
384
                $media = $helper->getMedia();
385
                $media->setFolder($folder);
386
                $em->getRepository(Media::class)->save($media);
0 ignored issues
show
Bug introduced by Jeroen Thora
It seems like you code against a concrete implementation and not the interface Doctrine\Persistence\ObjectRepository as the method save() does only exist in the following implementations of said interface: Kunstmaan\MediaBundle\Repository\FolderRepository, Kunstmaan\MediaBundle\Repository\MediaRepository.

Let’s take a look at an example:

interface User
{
    /** @return string */
    public function getPassword();
}

class MyUser implements User
{
    public function getPassword()
    {
        // return something
    }

    public function getDisplayName()
    {
        // return some name.
    }
}

class AuthSystem
{
    public function authenticate(User $user)
    {
        $this->logger->info(sprintf('Authenticating %s.', $user->getDisplayName()));
        // do something.
    }
}

In the above example, the authenticate() method works fine as long as you just pass instances of MyUser. However, if you now also want to pass a different implementation of User which does not have a getDisplayName() method, the code will break.

Available Fixes

  1. Change the type-hint for the parameter:

    class AuthSystem
    {
        public function authenticate(MyUser $user) { /* ... */ }
    }
    
  2. Add an additional type-check:

    class AuthSystem
    {
        public function authenticate(User $user)
        {
            if ($user instanceof MyUser) {
                $this->logger->info(/** ... */);
            }
    
            // or alternatively
            if ( ! $user instanceof MyUser) {
                throw new \LogicException(
                    '$user must be an instance of MyUser, '
                   .'other instances are not supported.'
                );
            }
    
        }
    }
    
Note: PHP Analyzer uses reverse abstract interpretation to narrow down the types inside the if block in such a case.
  1. Add the method to the interface:

    interface User
    {
        /** @return string */
        public function getPassword();
    
        /** @return string */
        public function getDisplayName();
    }
    
Loading history...
387
388
                $this->addFlash(
389
                    FlashTypes::SUCCESS,
390
                    $this->get('translator')->trans(
391
                        'media.flash.created',
392
                        [
393
                            '%medianame%' => $media->getName(),
394
                        ]
395
                    )
396
                );
397
398
                return new RedirectResponse($this->generateUrl($redirectUrl, $params));
399
            }
400
401
            if ($isInModal) {
402
                $this->addFlash(
403
                    FlashTypes::DANGER,
404
                    $this->get('translator')->trans(
405
                        'media.flash.not_created',
406
                        [
407
                            '%mediaerrors%' => $form->getErrors(true, true),
408
                        ]
409
                    )
410
                );
411
412
                return new RedirectResponse($this->generateUrl($redirectUrl, $params));
413
            }
414
        }
415
416
        return [
417
            'type' => $type,
418
            'form' => $form->createView(),
419
            'folder' => $folder,
420
        ];
421
    }
422
423
    /**
424
     * @param Request $request
425
     * @param int     $folderId The folder id
426
     * @param string  $type     The type
427
     *
428
     * @Route("create/modal/{folderId}/{type}", requirements={"folderId" = "\d+", "type" = ".+"}, name="KunstmaanMediaBundle_media_modal_create", methods={"POST"})
429
     *
430
     * @return array|RedirectResponse
0 ignored issues
show
Documentation introduced by Kris Pypen
Consider making the return type a bit more specific; maybe use RedirectResponse|array<s...t\Form\FormView|Folder>.

This check looks for the generic type array as a return type and suggests a more specific type. This type is inferred from the actual code.

Loading history...
431
     */
432
    public function createModalAction(Request $request, $folderId, $type)
433
    {
434
        $cKEditorFuncNum = $request->get('CKEditorFuncNum');
435
        $linkChooser = $request->get('linkChooser');
436
437
        $extraParams = [];
438
        if (!empty($cKEditorFuncNum)) {
439
            $extraParams['CKEditorFuncNum'] = $cKEditorFuncNum;
440
        }
441
        if (!empty($linkChooser)) {
442
            $extraParams['linkChooser'] = $linkChooser;
443
        }
444
445
        return $this->createAndRedirect(
446
            $request,
447
            $folderId,
448
            $type,
449
            'KunstmaanMediaBundle_chooser_show_folder',
450
            $extraParams,
451
            true
452
        );
453
    }
454
455
    /**
456
     * @param Request $request
457
     *
458
     * @Route("move/", name="KunstmaanMediaBundle_media_move", methods={"POST"})
459
     *
460
     * @return string
0 ignored issues
show
Documentation introduced by Sebastien Roch
Should the return type not be JsonResponse?

This check compares the return type specified in the @return annotation of a function or method doc comment with the types returned by the function and raises an issue if they mismatch.

Loading history...
461
     */
462
    public function moveMedia(Request $request)
463
    {
464
        @trigger_error(sprintf('The "%s" controller action is deprecated in KunstmaanMediaBundle 5.1 and will be removed in KunstmaanMediaBundle 6.0.', __METHOD__), E_USER_DEPRECATED);
0 ignored issues
show
Security Best Practice introduced by Jeroen Thora
It seems like you do not handle an error condition here. This can introduce security issues, and is generally not recommended.

If you suppress an error, we recommend checking for the error condition explicitly:

// For example instead of
@mkdir($dir);

// Better use
if (@mkdir($dir) === false) {
    throw new \RuntimeException('The directory '.$dir.' could not be created.');
}
Loading history...
465
466
        $mediaId = $request->request->get('mediaId');
467
        $folderId = $request->request->get('folderId');
468
469
        if (empty($mediaId) || empty($folderId)) {
470
            return new JsonResponse(['error' => ['title' => 'Missing media id or folder id']], 400);
471
        }
472
473
        $em = $this->getDoctrine()->getManager();
474
        $mediaRepo = $em->getRepository(Media::class);
475
476
        $media = $mediaRepo->getMedia($mediaId);
477
        $folder = $em->getRepository(Folder::class)->getFolder($folderId);
0 ignored issues
show
Bug introduced by Jeroen Thora
It seems like you code against a concrete implementation and not the interface Doctrine\Persistence\ObjectRepository as the method getFolder() does only exist in the following implementations of said interface: Kunstmaan\MediaBundle\Repository\FolderRepository.

Let’s take a look at an example:

interface User
{
    /** @return string */
    public function getPassword();
}

class MyUser implements User
{
    public function getPassword()
    {
        // return something
    }

    public function getDisplayName()
    {
        // return some name.
    }
}

class AuthSystem
{
    public function authenticate(User $user)
    {
        $this->logger->info(sprintf('Authenticating %s.', $user->getDisplayName()));
        // do something.
    }
}

In the above example, the authenticate() method works fine as long as you just pass instances of MyUser. However, if you now also want to pass a different implementation of User which does not have a getDisplayName() method, the code will break.

Available Fixes

  1. Change the type-hint for the parameter:

    class AuthSystem
    {
        public function authenticate(MyUser $user) { /* ... */ }
    }
    
  2. Add an additional type-check:

    class AuthSystem
    {
        public function authenticate(User $user)
        {
            if ($user instanceof MyUser) {
                $this->logger->info(/** ... */);
            }
    
            // or alternatively
            if ( ! $user instanceof MyUser) {
                throw new \LogicException(
                    '$user must be an instance of MyUser, '
                   .'other instances are not supported.'
                );
            }
    
        }
    }
    
Note: PHP Analyzer uses reverse abstract interpretation to narrow down the types inside the if block in such a case.
  1. Add the method to the interface:

    interface User
    {
        /** @return string */
        public function getPassword();
    
        /** @return string */
        public function getDisplayName();
    }
    
Loading history...
478
479
        $media->setFolder($folder);
480
        $mediaRepo->save($media);
481
482
        return new JsonResponse();
483
    }
484
485
    /**
486
     * @Route("/bulk-move", name="KunstmaanMediaBundle_media_bulk_move")
487
     *
488
     * @param Request $request
489
     *
490
     * @return JsonResponse|Response
491
     *
492
     * @throws \Doctrine\DBAL\DBALException
493
     */
494
    public function bulkMoveAction(Request $request)
495
    {
496
        $em = $this->getDoctrine()->getManager();
497
        $mediaRepo = $em->getRepository(Media::class);
498
        $form = $this->createForm(BulkMoveMediaType::class);
499
500
        $form->handleRequest($request);
501
502
        if ($form->isSubmitted() && $form->isValid()) {
503
            /** @var Folder $folder */
504
            $folder = $form->getData()['folder'];
505
            $mediaIds = explode(',', $form->getData()['media']);
506
507
            $mediaRepo->createQueryBuilder('m')
508
                ->update()
509
                ->set('m.folder', $folder->getId())
510
                ->where('m.id in (:mediaIds)')
511
                ->setParameter('mediaIds', $mediaIds)
512
                ->getQuery()
513
                ->execute();
514
515
            $this->addFlash(FlashTypes::SUCCESS, $this->get('translator')->trans('media.folder.bulk_move.success.text'));
516
517
            return new JsonResponse(
518
                [
519
                    'Success' => 'The media is moved',
520
                ]
521
            );
522
        }
523
524
        return $this->render(
525
            '@KunstmaanMedia/Folder/bulk-move-modal_form.html.twig',
526
            [
527
                'form' => $form->createView(),
528
            ]
529
        );
530
    }
531
}
532