Completed
Pull Request — master (#2680)
by
unknown
22:28
created

MediaController   F

Complexity

Total Complexity 60

Size/Duplication

Total Lines 532
Duplicated Lines 0 %

Coupling/Cohesion

Components 1
Dependencies 14

Test Coverage

Coverage 0%

Importance

Changes 0
Metric Value
wmc 60
lcom 1
cbo 14
dl 0
loc 532
ccs 0
cts 345
cp 0
rs 3.6
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
A createModalAction() 0 22 3
A moveMedia() 0 22 3
A bulkMoveAction() 0 37 3
F bulkUploadSubmitAction() 0 135 28
A returnJsonError() 0 13 1
B dropAction() 0 43 6
A createAction() 0 4 1
B createAndRedirect() 0 74 6

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
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
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
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
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
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
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")
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
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
        // Get a file name
163
        if ($request->request->has('name')) {
164
            $fileName = $request->request->get('name');
165
        } elseif (0 !== $request->files->count()) {
166
            $fileName = $request->files->get('file')['name'];
167
        } else {
168
            $fileName = \uniqid('file_', false);
169
        }
170
        $filePath = $targetDir.DIRECTORY_SEPARATOR.$fileName;
171
172
        $chunk = 0;
173
        $chunks = 0;
174
        // Chunking might be enabled
175
        if ($request->request->has('chunk')) {
176
            $chunk = $request->request->getInt('chunk');
177
        }
178
        if ($request->request->has('chunks')) {
179
            $chunks = $request->request->getInt('chunks');
180
        }
181
182
        // Remove old temp files
183
        if ($cleanupTargetDir) {
184
            if (!\is_dir($targetDir) || !$dir = \opendir($targetDir)) {
185
                return $this->returnJsonError('100', 'Failed to open temp directory.');
186
            }
187
188
            while (($file = \readdir($dir)) !== false) {
189
                $tmpFilePath = $targetDir.DIRECTORY_SEPARATOR.$file;
190
191
                // If temp file is current file proceed to the next
192
                if ($tmpFilePath === "{$filePath}.part") {
193
                    continue;
194
                }
195
196
                // Remove temp file if it is older than the max age and is not the current file
197
                if (\preg_match('/\.part$/', $file) && (\filemtime($tmpFilePath) < \time() - $maxFileAge)) {
198
                    $success = @\unlink($tmpFilePath);
199
                    if ($success !== true) {
200
                        return $this->returnJsonError('106', 'Could not remove temp file: '.$filePath);
201
                    }
202
                }
203
            }
204
            \closedir($dir);
205
        }
206
207
        // Open temp file
208
        if (!$out = @\fopen("{$filePath}.part", $chunks ? 'ab' : 'wb')) {
209
            return $this->returnJsonError('102', 'Failed to open output stream.');
210
        }
211
212
        if (0 !== $request->files->count()) {
213
            $_file = $request->files->get('file');
214
            if ($_file->getError() > 0 || !\is_uploaded_file($_file->getRealPath())) {
215
                return $this->returnJsonError('103', 'Failed to move uploaded file.');
216
            }
217
218
            // Read binary input stream and append it to temp file
219
            if (!$input = @\fopen($_file->getRealPath(), 'rb')) {
220
                return $this->returnJsonError('101', 'Failed to open input stream.');
221
            }
222
        } else {
223
            if (!$input = @\fopen('php://input', 'rb')) {
224
                return $this->returnJsonError('101', 'Failed to open input stream.');
225
            }
226
        }
227
228
        while ($buff = \fread($input, 4096)) {
229
            \fwrite($out, $buff);
230
        }
231
232
        @\fclose($out);
0 ignored issues
show
Security Best Practice introduced by
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...
233
        @\fclose($input);
0 ignored issues
show
Security Best Practice introduced by
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...
234
235
        // Check if file has been uploaded
236
        if (!$chunks || $chunk === $chunks - 1) {
237
            // Strip the temp .part suffix off
238
            \rename("{$filePath}.part", $filePath);
239
        }
240
241
        $em = $this->getDoctrine()->getManager();
242
        /* @var Folder $folder */
243
        $folder = $em->getRepository(Folder::class)->getFolder($folderId);
0 ignored issues
show
Bug introduced by
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...
244
        $file = new File($filePath);
245
246
        if (!$this->get('kunstmaan_media.media_manager')->isExtensionAllowed($file)) {
247
            return $this->returnJsonError('104', 'This file type is not allowed on the server');
248
        }
249
250
        try {
251
            /* @var Media $media */
252
            $media = $this->get('kunstmaan_media.media_manager')->getHandler($file)->createNew($file);
253
            $media->setFolder($folder);
254
            $em->getRepository(Media::class)->save($media);
0 ignored issues
show
Bug introduced by
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...
255
        } catch (Exception $e) {
256
            return $this->returnJsonError('104', 'Failed performing save on media-manager');
257
        }
258
259
        $success = \unlink($filePath);
260
        if ($success !== true) {
261
            return $this->returnJsonError('105', 'Could not remove temp file: '.$filePath);
262
        }
263
264
        // Send headers making sure that the file is not cached (as it happens for example on iOS devices)
265
        $response = new JsonResponse(
266
            [
267
                'jsonrpc' => '2.0',
268
                'result' => '',
269
                'id' => 'id',
270
            ], JsonResponse::HTTP_OK, [
271
                'Expires' => 'Mon, 26 Jul 1997 05:00:00 GMT',
272
                'Last-Modified' => \gmdate('D, d M Y H:i:s').' GMT',
273
                'Cache-Control' => 'no-store, no-cache, must-revalidate, post-check=0, pre-check=0',
274
                'Pragma' => 'no-cache',
275
            ]
276
        );
277
278
        return $response;
279
    }
280
281
    private function returnJsonError($code, $message)
282
    {
283
        return new JsonResponse(
284
            [
285
                'jsonrpc' => '2.0',
286
                'error ' => [
287
                    'code' => $code,
288
                    'message' => $message,
289
                ],
290
                'id' => 'id',
291
            ]
292
        );
293
    }
294
295
    /**
296
     * @param Request $request
297
     * @param int     $folderId
298
     *
299
     * @Route("drop/{folderId}", requirements={"folderId" = "\d+"}, name="KunstmaanMediaBundle_media_drop_upload", methods={"GET", "POST"})
300
     *
301
     * @return JsonResponse
302
     */
303
    public function dropAction(Request $request, $folderId)
304
    {
305
        $em = $this->getDoctrine()->getManager();
306
307
        /* @var Folder $folder */
308
        $folder = $em->getRepository(Folder::class)->getFolder($folderId);
0 ignored issues
show
Bug introduced by
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...
309
310
        $drop = null;
0 ignored issues
show
Unused Code introduced by
$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...
311
312
        if ($request->files->has('files') && $request->files->get('files')['error'] === 0) {
313
            $drop = $request->files->get('files');
314
        } else {
315
            if ($request->files->get('file')) {
316
                $drop = $request->files->get('file');
317
            } else {
318
                $drop = $request->get('text');
319
            }
320
        }
321
        $mediaManager = $this->get('kunstmaan_media.media_manager');
322
        $media = $mediaManager->createNew($drop);
323
        if ($media) {
324
            $media->setFolder($folder);
325
            $em->getRepository(Media::class)->save($media);
0 ignored issues
show
Bug introduced by
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
        if (!$mediaManager->isExtensionAllowed($drop)) {
331
            $request->getSession()->getFlashBag()->add(
332
                FlashTypes::DANGER,
333
                $this->get('translator')->trans('media.flash.not_valid')
334
            );
335
336
            return new JsonResponse(['status' => $this->get('translator')->trans('kuma_admin.media.flash.drop_unrecognized')]);
337
        }
338
339
        $request->getSession()->getFlashBag()->add(
340
            FlashTypes::DANGER,
341
            $this->get('translator')->trans('kuma_admin.media.flash.drop_unrecognized')
342
        );
343
344
        return new JsonResponse(['status' => $this->get('translator')->trans('media.flash.not_valid')]);
345
    }
346
347
    /**
348
     * @param Request $request
349
     * @param int     $folderId The folder id
350
     * @param string  $type     The type
351
     *
352
     * @Route("create/{folderId}/{type}", requirements={"folderId" = "\d+", "type" = ".+"}, name="KunstmaanMediaBundle_media_create", methods={"GET", "POST"})
353
     * @Template("@KunstmaanMedia/Media/create.html.twig")
354
     *
355
     * @return array|RedirectResponse
0 ignored issues
show
Documentation introduced by
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...
356
     */
357
    public function createAction(Request $request, $folderId, $type)
358
    {
359
        return $this->createAndRedirect($request, $folderId, $type, 'KunstmaanMediaBundle_folder_show');
360
    }
361
362
    /**
363
     * @param Request $request
364
     * @param int     $folderId    The folder Id
365
     * @param string  $type        The type
366
     * @param string  $redirectUrl The url where we want to redirect to on success
367
     * @param array   $extraParams The extra parameters that will be passed wen redirecting
368
     *
369
     * @return array|RedirectResponse
0 ignored issues
show
Documentation introduced by
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...
370
     */
371
    private function createAndRedirect(Request $request, $folderId, $type, $redirectUrl, $extraParams = [], $isInModal = false)
372
    {
373
        $em = $this->getDoctrine()->getManager();
374
375
        /* @var Folder $folder */
376
        $folder = $em->getRepository(Folder::class)->getFolder($folderId);
0 ignored issues
show
Bug introduced by
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...
377
378
        /* @var MediaManager $mediaManager */
379
        $mediaManager = $this->get('kunstmaan_media.media_manager');
380
        $handler = $mediaManager->getHandlerForType($type);
381
        $media = new Media();
382
        $helper = $handler->getFormHelper($media);
383
384
        $form = $this->createForm($handler->getFormType(), $helper, $handler->getFormTypeOptions());
385
386
        if ($request->isMethod('POST')) {
387
            $params = ['folderId' => $folder->getId()];
388
            $params = \array_merge($params, $extraParams);
389
390
            $form->handleRequest($request);
391
392
            if ($form->isSubmitted() && $form->isValid()) {
393
                if ($mediaManager->isExtensionAllowed($helper->getFile())) {
394
                    $media = $helper->getMedia();
395
                    $media->setFolder($folder);
396
                    $em->getRepository(Media::class)->save($media);
0 ignored issues
show
Bug introduced by
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...
397
398
                    $this->addFlash(
399
                        FlashTypes::SUCCESS,
400
                        $this->get('translator')->trans(
401
                            'media.flash.created',
402
                            [
403
                                '%medianame%' => $media->getName(),
404
                            ]
405
                        )
406
                    );
407
408
                    return new RedirectResponse($this->generateUrl($redirectUrl, $params));
409
                }
410
411
                $this->addFlash(
412
                    FlashTypes::DANGER,
413
                    $this->get('translator')->trans(
414
                        'media.flash.not_valid',
415
                        [
416
                            '%medianame%' => $media->getName(),
417
                        ]
418
                    )
419
                );
420
421
                return new RedirectResponse($this->generateUrl($redirectUrl, $params));
422
            }
423
424
            if ($isInModal) {
425
                $this->addFlash(
426
                    FlashTypes::DANGER,
427
                    $this->get('translator')->trans(
428
                        'media.flash.not_created',
429
                        [
430
                            '%mediaerrors%' => $form->getErrors(true, true),
431
                        ]
432
                    )
433
                );
434
435
                return new RedirectResponse($this->generateUrl($redirectUrl, $params));
436
            }
437
        }
438
439
        return [
440
            'type' => $type,
441
            'form' => $form->createView(),
442
            'folder' => $folder,
443
        ];
444
    }
445
446
    /**
447
     * @param Request $request
448
     * @param int     $folderId The folder id
449
     * @param string  $type     The type
450
     *
451
     * @Route("create/modal/{folderId}/{type}", requirements={"folderId" = "\d+", "type" = ".+"}, name="KunstmaanMediaBundle_media_modal_create", methods={"POST"})
452
     *
453
     * @return array|RedirectResponse
0 ignored issues
show
Documentation introduced by
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...
454
     */
455
    public function createModalAction(Request $request, $folderId, $type)
456
    {
457
        $cKEditorFuncNum = $request->get('CKEditorFuncNum');
458
        $linkChooser = $request->get('linkChooser');
459
460
        $extraParams = [];
461
        if (!empty($cKEditorFuncNum)) {
462
            $extraParams['CKEditorFuncNum'] = $cKEditorFuncNum;
463
        }
464
        if (!empty($linkChooser)) {
465
            $extraParams['linkChooser'] = $linkChooser;
466
        }
467
468
        return $this->createAndRedirect(
469
            $request,
470
            $folderId,
471
            $type,
472
            'KunstmaanMediaBundle_chooser_show_folder',
473
            $extraParams,
474
            true
475
        );
476
    }
477
478
    /**
479
     * @param Request $request
480
     *
481
     * @Route("move/", name="KunstmaanMediaBundle_media_move", methods={"POST"})
482
     *
483
     * @return string
0 ignored issues
show
Documentation introduced by
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...
484
     */
485
    public function moveMedia(Request $request)
486
    {
487
        @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
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...
488
489
        $mediaId = $request->request->get('mediaId');
490
        $folderId = $request->request->get('folderId');
491
492
        if (empty($mediaId) || empty($folderId)) {
493
            return new JsonResponse(['error' => ['title' => 'Missing media id or folder id']], 400);
494
        }
495
496
        $em = $this->getDoctrine()->getManager();
497
        $mediaRepo = $em->getRepository(Media::class);
498
499
        $media = $mediaRepo->getMedia($mediaId);
500
        $folder = $em->getRepository(Folder::class)->getFolder($folderId);
0 ignored issues
show
Bug introduced by
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...
501
502
        $media->setFolder($folder);
503
        $mediaRepo->save($media);
504
505
        return new JsonResponse();
506
    }
507
508
    /**
509
     * @Route("/bulk-move", name="KunstmaanMediaBundle_media_bulk_move")
510
     *
511
     * @param Request $request
512
     *
513
     * @return JsonResponse|Response
514
     *
515
     * @throws \Doctrine\DBAL\DBALException
516
     */
517
    public function bulkMoveAction(Request $request)
518
    {
519
        $em = $this->getDoctrine()->getManager();
520
        $mediaRepo = $em->getRepository(Media::class);
521
        $form = $this->createForm(BulkMoveMediaType::class);
522
523
        $form->handleRequest($request);
524
525
        if ($form->isSubmitted() && $form->isValid()) {
526
            /** @var Folder $folder */
527
            $folder = $form->getData()['folder'];
528
            $mediaIds = explode(',', $form->getData()['media']);
529
530
            $mediaRepo->createQueryBuilder('m')
531
                ->update()
532
                ->set('m.folder', $folder->getId())
533
                ->where('m.id in (:mediaIds)')
534
                ->setParameter('mediaIds', $mediaIds)
535
                ->getQuery()
536
                ->execute();
537
538
            $this->addFlash(FlashTypes::SUCCESS, $this->get('translator')->trans('media.folder.bulk_move.success.text'));
539
540
            return new JsonResponse(
541
                [
542
                    'Success' => 'The media is moved',
543
                ]
544
            );
545
        }
546
547
        return $this->render(
548
            '@KunstmaanMedia/Folder/bulk-move-modal_form.html.twig',
549
            [
550
                'form' => $form->createView(),
551
            ]
552
        );
553
    }
554
}
555