Browse Source

Clean the code of controllers

- avoid using deprecation logger methods on failure
- remove dead code (unused Template annotation, unused argument...)
- fix inline phpdoc helping IDE, to actually help them
Christophe Coevoet 9 years ago
parent
commit
6660d23d6c

+ 3 - 4
src/Packagist/WebBundle/Controller/ApiController.php

@@ -21,10 +21,8 @@ use Composer\Repository\InvalidRepositoryException;
 use Composer\Repository\VcsRepository;
 use Packagist\WebBundle\Entity\Package;
 use Packagist\WebBundle\Entity\User;
-use Packagist\WebBundle\Package\Updater;
 use Sensio\Bundle\FrameworkExtraBundle\Configuration\Method;
 use Sensio\Bundle\FrameworkExtraBundle\Configuration\Route;
-use Sensio\Bundle\FrameworkExtraBundle\Configuration\Template;
 use Symfony\Component\Console\Output\OutputInterface;
 use Symfony\Component\HttpFoundation\JsonResponse;
 use Symfony\Component\HttpFoundation\Request;
@@ -39,7 +37,7 @@ class ApiController extends Controller
      * @Route("/packages.json", name="packages", defaults={"_format" = "json"})
      * @Method({"GET"})
      */
-    public function packagesAction(Request $req)
+    public function packagesAction()
     {
         // fallback if any of the dumped files exist
         $rootJson = $this->container->getParameter('kernel.root_dir').'/../web/packages_root.json';
@@ -75,6 +73,7 @@ class ApiController extends Controller
         $package->setRepository($url);
         $errors = $this->get('validator')->validate($package);
         if (count($errors) > 0) {
+            $errorArray = array();
             foreach ($errors as $error) {
                 $errorArray[$error->getPropertyPath()] =  $error->getMessage();
             }
@@ -85,7 +84,7 @@ class ApiController extends Controller
             $em->persist($package);
             $em->flush();
         } catch (\Exception $e) {
-            $this->get('logger')->crit($e->getMessage(), array('exception', $e));
+            $this->get('logger')->critical($e->getMessage(), array('exception', $e));
             return new JsonResponse(array('status' => 'error', 'message' => 'Error saving package'), 500);
         }
 

+ 4 - 4
src/Packagist/WebBundle/Controller/FeedController.php

@@ -51,7 +51,7 @@ class FeedController extends Controller
      */
     public function packagesAction(Request $req)
     {
-        /** @var $repo \Packagist\WebBundle\Entity\VersionRepository */
+        /** @var $repo \Packagist\WebBundle\Entity\PackageRepository */
         $repo = $this->getDoctrine()->getRepository('PackagistWebBundle:Package');
         $packages = $this->getLimitedResults(
             $repo->getQueryBuilderForNewestPackages()
@@ -78,7 +78,7 @@ class FeedController extends Controller
      */
     public function releasesAction(Request $req)
     {
-        /** @var $repo \Packagist\WebBundle\Entity\PackageRepository */
+        /** @var $repo \Packagist\WebBundle\Entity\VersionRepository */
         $repo = $this->getDoctrine()->getRepository('PackagistWebBundle:Version');
         $packages = $this->getLimitedResults(
             $repo->getQueryBuilderForLatestVersionWithPackage()
@@ -105,7 +105,7 @@ class FeedController extends Controller
      */
     public function vendorAction(Request $req, $vendor)
     {
-        /** @var $repo \Packagist\WebBundle\Entity\PackageRepository */
+        /** @var $repo \Packagist\WebBundle\Entity\VersionRepository */
         $repo = $this->getDoctrine()->getRepository('PackagistWebBundle:Version');
         $packages = $this->getLimitedResults(
             $repo->getQueryBuilderForLatestVersionWithPackage($vendor)
@@ -132,7 +132,7 @@ class FeedController extends Controller
      */
     public function packageAction(Request $req, $package)
     {
-        /** @var $repo \Packagist\WebBundle\Entity\PackageRepository */
+        /** @var $repo \Packagist\WebBundle\Entity\VersionRepository */
         $repo = $this->getDoctrine()->getRepository('PackagistWebBundle:Version');
         $packages = $this->getLimitedResults(
             $repo->getQueryBuilderForLatestVersionWithPackage(null, $package)

+ 7 - 14
src/Packagist/WebBundle/Controller/PackageController.php

@@ -36,7 +36,6 @@ use Sensio\Bundle\FrameworkExtraBundle\Configuration\Template;
 use Pagerfanta\Adapter\FixedAdapter;
 use Pagerfanta\Pagerfanta;
 use Packagist\WebBundle\Package\Updater;
-use Zend\Json\Json;
 
 class PackageController extends Controller
 {
@@ -121,7 +120,7 @@ class PackageController extends Controller
 
                 return new RedirectResponse($this->generateUrl('view_package', array('name' => $package->getName())));
             } catch (\Exception $e) {
-                $this->get('logger')->crit($e->getMessage(), array('exception', $e));
+                $this->get('logger')->critical($e->getMessage(), array('exception', $e));
                 $this->get('session')->getFlashBag()->set('error', $package->getName().' could not be saved.');
             }
         }
@@ -243,7 +242,6 @@ class PackageController extends Controller
     }
 
     /**
-     * @Template()
      * @Route(
      *     "/providers/{name}",
      *     name="view_providers",
@@ -252,7 +250,7 @@ class PackageController extends Controller
      * )
      * @Method({"GET"})
      */
-    public function viewProvidersAction(Request $req, $name)
+    public function viewProvidersAction($name)
     {
         /** @var PackageRepository $repo */
         $repo = $this->getDoctrine()->getRepository('PackagistWebBundle:Package');
@@ -394,7 +392,6 @@ class PackageController extends Controller
     }
 
     /**
-     * @Template()
      * @Route(
      *     "/packages/{name}/downloads.{_format}",
      *     name="package_downloads_full",
@@ -450,7 +447,6 @@ class PackageController extends Controller
     }
 
     /**
-     * @Template()
      * @Route(
      *     "/versions/{versionId}.{_format}",
      *     name="view_version",
@@ -458,7 +454,7 @@ class PackageController extends Controller
      * )
      * @Method({"GET"})
      */
-    public function viewPackageVersionAction(Request $req, $versionId)
+    public function viewPackageVersionAction($versionId)
     {
         /** @var VersionRepository $repo  */
         $repo = $this->getDoctrine()->getRepository('PackagistWebBundle:Version');
@@ -472,7 +468,6 @@ class PackageController extends Controller
     }
 
     /**
-     * @Template()
      * @Route(
      *     "/versions/{versionId}/delete",
      *     name="delete_version",
@@ -493,7 +488,7 @@ class PackageController extends Controller
             throw new AccessDeniedException;
         }
 
-        if (!$this->get('form.csrf_provider')->isCsrfTokenValid('delete_version', $req->request->get('_token'))) {
+        if (!$this->isCsrfTokenValid('delete_version', $req->request->get('_token'))) {
             throw new AccessDeniedException;
         }
 
@@ -505,7 +500,6 @@ class PackageController extends Controller
     }
 
     /**
-     * @Template()
      * @Route("/packages/{name}", name="update_package", requirements={"name"="[A-Za-z0-9_.-]+/[A-Za-z0-9_.-]+"}, defaults={"_format" = "json"})
      * @Method({"PUT"})
      */
@@ -588,7 +582,6 @@ class PackageController extends Controller
     }
 
     /**
-     * @Template()
      * @Route("/packages/{name}", name="delete_package", requirements={"name"="[A-Za-z0-9_.-]+/[A-Za-z0-9_.-]+"})
      * @Method({"DELETE"})
      */
@@ -691,7 +684,7 @@ class PackageController extends Controller
                 }
                 $this->get('session')->getFlashBag()->set('error', 'The user could not be found.');
             } catch (\Exception $e) {
-                $this->get('logger')->crit($e->getMessage(), array('exception', $e));
+                $this->get('logger')->critical($e->getMessage(), array('exception', $e));
                 $this->get('session')->getFlashBag()->set('error', 'The maintainer could not be added.');
             }
         }
@@ -746,7 +739,7 @@ class PackageController extends Controller
                 }
                 $this->get('session')->getFlashBag()->set('error', 'The user could not be found.');
             } catch (\Exception $e) {
-                $this->get('logger')->crit($e->getMessage(), array('exception', $e));
+                $this->get('logger')->critical($e->getMessage(), array('exception', $e));
                 $this->get('session')->getFlashBag()->set('error', 'The maintainer could not be removed.');
             }
         }
@@ -1014,7 +1007,7 @@ class PackageController extends Controller
         return $this->overallStatsAction($req, $package, $version);
     }
 
-    private function createAddMaintainerForm($package)
+    private function createAddMaintainerForm(Package $package)
     {
         if (!$user = $this->getUser()) {
             return;