Updates on comments from @stof

src/Packagist/WebBundle/Command/GenerateTokensCommand.php

@@ -13,12 +13,8 @@
 namespace Packagist\WebBundle\Command;
 
 use Symfony\Bundle\FrameworkBundle\Command\ContainerAwareCommand;
-use Symfony\Bridge\Doctrine\RegistryInterface;
 use Symfony\Component\Console\Input\InputInterface;
-use Symfony\Component\Console\Input\InputOption;
 use Symfony\Component\Console\Output\OutputInterface;
-use Symfony\Component\HttpKernel\KernelInterface;
-use Symfony\Component\Finder\Finder;
 
 /**
  * @author Jordi Boggiano <j.boggiano@seld.be>

src/Packagist/WebBundle/Controller/ApiController.php

@@ -30,7 +30,6 @@ use Sensio\Bundle\FrameworkExtraBundle\Configuration\Method;
  */
 class ApiController extends Controller
 {
-
     protected $supportedLinkTypes = array(
         'require'   => 'RequireLink',
         'conflict'  => 'ConflictLink',
@@ -42,7 +41,7 @@ class ApiController extends Controller
 
     /**
      * @Template()
-     * @Route("/packages.json", name="packages")
+     * @Route("/packages.json", name="packages", defaults={"_format" = "json"})
      */
     public function packagesAction()
     {
@@ -55,42 +54,47 @@ class ApiController extends Controller
             $data[$package->getName()] = $package->toArray();
         }
 
-        $response = new Response(json_encode($data), 200, array('Content-Type' => 'application/json'));
+        $response = new Response(json_encode($data), 200);
         $response->setSharedMaxAge(60);
         return $response;
     }
+
     /**
-     * @Route("/api/github.json", name="github_postreceive")
+     * @Route("/api/github", name="github_postreceive", defaults={"_format" = "json"})
      * @Method({"POST"})
      */
     public function githubPostReceive(Request $request)
     {
-        $responseHeaders = array('Content-Type' => 'application/json');
         $payload = json_decode($request->request->get('payload'), true);
         if (!$payload or !isset($payload['repository']['url'])) {
-            return new Response(json_encode(array('status' => 'error', 'message' => 'Missing or invalid payload',)), 406, $responseHeaders);
+            return new Response(json_encode(array('status' => 'error', 'message' => 'Missing or invalid payload',)), 406);
         }
+
         $username = $request->query->get('username');
         $apiToken = $request->query->get('apiToken');
+
         $doctrine = $this->get('doctrine');
         $user = $doctrine
             ->getRepository('Packagist\WebBundle\Entity\User')
             ->findOneBy(array('username' => $username, 'apiToken' => $apiToken));
+
         if (!$user) {
-            return new Response(json_encode(array('status' => 'error', 'message' => 'Invalid credentials',)), 403, $responseHeaders);
+            return new Response(json_encode(array('status' => 'error', 'message' => 'Invalid credentials',)), 403);
         }
+
         if (! preg_match('~(github.com/[\w_\-\.]+/[\w_\-\.]+)$~', $payload['repository']['url'], $matches)) {
-            return new Response(json_encode(array('status' => 'error', 'message' => 'Could not parse payload repository URL',)), 406, $responseHeaders);
+            return new Response(json_encode(array('status' => 'error', 'message' => 'Could not parse payload repository URL',)), 406);
         }
+
         $payloadRepositoryChunk = $matches[1];
+
         foreach ($user->getPackages() as $package) {
-            if (strpos($package->getRepository(), $payloadRepositoryChunk) !== false) {
+            if (false !== strpos($package->getRepository(), $payloadRepositoryChunk)) {
 
                 //
                 // We found the package that was referenced.
                 //
-                
-                $force = true;
+
                 $start = new \DateTime();
 
                 $repository = new VcsRepository(array('url' => $package->getRepository()));
@@ -100,17 +104,6 @@ class ApiController extends Controller
                     return version_compare($a->getVersion(), $b->getVersion());
                 });
             
-                // clear existing versions to force a clean reloading if --force is enabled
-                if ($force) {
-                    $versionRepo = $doctrine->getRepository('PackagistWebBundle:Version');
-                    foreach ($package->getVersions() as $version) {
-                        $versionRepo->remove($version);
-                    }
-            
-                    $doctrine->getEntityManager()->flush();
-                    $doctrine->getEntityManager()->refresh($package);
-                }
-            
                 foreach ($versions as $version) {
                     $this->updateInformation($doctrine, $package, $version);
                     $doctrine->getEntityManager()->flush();
@@ -127,10 +120,10 @@ class ApiController extends Controller
                 $package->setCrawledAt(new \DateTime);
                 $doctrine->getEntityManager()->flush();
 
-                return new Response('{ "status": "success" }', 202, $responseHeaders);
+                return new Response('{ "status": "success" }', 202);
             }
         }
-        return new Response(json_encode(array('status' => 'error', 'message' => 'Could not find a package that matches this request (does user maintain the package?)',)), 404, $responseHeaders);
+        return new Response(json_encode(array('status' => 'error', 'message' => 'Could not find a package that matches this request (does user maintain the package?)',)), 404);
     }
 
     private function updateInformation(RegistryInterface $doctrine, $package, PackageInterface $data)

src/Packagist/WebBundle/Entity/User.php

@@ -45,7 +45,7 @@ class User extends BaseUser
     private $createdAt;
 
     /**
-     * @ORM\Column(type="string")
+     * @ORM\Column(type="string", length=20, nullable=true)
      * @var string
      */
     private $apiToken;
@@ -162,6 +162,6 @@ class User extends BaseUser
      */
     protected function generateApiToken()
     {
-        return base_convert(sha1(uniqid(mt_rand(), true)), 16, 36);
+        return substr($this->generateToken(), 0, 20);
     }
 }

src/Packagist/WebBundle/Tests/Controller/ApiControllerTest.php

@@ -20,20 +20,11 @@ class ApiControllerTest extends WebTestCase
     {
         $client = self::createClient();
         
-        $client->request('GET', '/api/github.json');
+        $client->request('GET', '/api/github');
         $this->assertEquals(405, $client->getResponse()->getStatusCode(), 'GET method should not be allowed for GitHub Post-Receive URL');
 
-        $doctrine = $client->getContainer()->get('doctrine');
-        $em = $doctrine->getEntityManager();
-        $userRepo = $doctrine->getRepository('PackagistWebBundle:User');
-        $testUser = new User();
-        $testUser->setUsername('ApiControllerTest');
         $payload = json_encode(array('repository' => array('url' => 'git://github.com/composer/composer',)));
-
-        $client->request('POST', '/api/github.json?username='.$testUser->getUsername().'&apiToken=BAD'.$testUser->getApiToken(), array('payload' => $payload,));
-        $this->assertEquals(403, $client->getResponse()->getStatusCode(), 'POST method should return 403 "Forbidden" if invalid API Token is sent');
-
-        $client->request('POST', '/api/github.json?username=BAD'.$testUser->getUsername().'&apiToken='.$testUser->getApiToken(), array('payload' => $payload,));
-        $this->assertEquals(403, $client->getResponse()->getStatusCode(), 'POST method should return 403 "Forbidden" if invalid API Token is sent');
+        $client->request('POST', '/api/github?username=INVALID_USER&apiToken=INVALID_TOKEN', array('payload' => $payload,));
+        $this->assertEquals(403, $client->getResponse()->getStatusCode(), 'POST method should return 403 "Forbidden" if invalid username and API Token are sent');
     }
 }