Merge pull request #61 from j0k3r/same-tag

Fix case insensitive error for some tag
j0k3r · Mar 15, 2017 · 740f34f · 740f34f
2 parents 5a7d668 + 0c72935
commit 740f34f
Show file tree

Hide file tree

Showing 7 changed files with 198 additions and 39 deletions.
diff --git a/app/config/config.yml b/app/config/config.yml
@@ -105,7 +105,6 @@ swarrot:
             processor: banditore.consumer.sync_versions
             extras:
                 poll_interval: 500000
-                requeue_on_error: false
             middleware_stack:
                 - configurator: swarrot.processor.new_relic
                   extras:

diff --git a/composer.lock b/composer.lock
diff --git a/src/AppBundle/Consumer/SyncStarredRepos.php b/src/AppBundle/Consumer/SyncStarredRepos.php
@@ -91,6 +91,11 @@ private function doSyncRepo(User $user)
         $starredRepos = $this->client->api('user')->starred($user->getUsername(), $page, $perPage);
         $em = $this->doctrine->getManager();
 
+        // in case of the manager is closed following a previous exception
+        if (!$em->isOpen()) {
+            $em = $this->doctrine->resetManager();
+        }
+
         do {
             $this->logger->info('    sync ' . count($starredRepos) . ' starred repos', [
                 'user' => $user->getUsername(),

diff --git a/src/AppBundle/Consumer/SyncVersions.php b/src/AppBundle/Consumer/SyncVersions.php
@@ -64,13 +64,8 @@ public function process(Message $message, array $options)
 
         $this->logger->notice('[' . $this->getRateLimits($this->client, $this->logger) . '] Check <info>' . $repo->getFullName() . '</info> … ');
 
-        try {
-            $nbVersions = $this->doSyncVersions($repo);
-        } catch (\Exception $e) {
-            $this->logger->error('Error while syncing repo', ['exception' => get_class($e), 'message' => $e->getMessage(), 'repo' => $repo->getFullName()]);
-
-            return;
-        }
+        // this shouldn't be catched so the worker will die when an exception is thrown
+        $nbVersions = $this->doSyncVersions($repo);
 
         // notify pubsubhubbub for that repo
         if ($nbVersions > 0) {
@@ -90,6 +85,11 @@ private function doSyncVersions(Repo $repo)
         $newVersion = 0;
         $em = $this->doctrine->getManager();
 
+        // in case of the manager is closed following a previous exception
+        if (!$em->isOpen()) {
+            $em = $this->doctrine->resetManager();
+        }
+
         list($username, $repoName) = explode('/', $repo->getFullName());
 
         // this is a simple call to retrieve at least one tag from the selected repo
@@ -128,6 +128,16 @@ private function doSyncVersions(Repo $repo)
                 continue;
             }
 
+            // check for scheduled version to be persisted later
+            // in rare case where the tag name is almost equal, like "v1.1.0" & "V1.1.0" in might avoid error
+            foreach ($em->getUnitOfWork()->getScheduledEntityInsertions() as $entity) {
+                if ($entity instanceof Version && strtolower($entity->getTagName()) === strtolower($tag['name'])) {
+                    $this->logger->info($tag['name'] . ' skipped because it seems to be already scheduled');
+
+                    continue 2;
+                }
+            }
+
             // is there an associated release?
             $newRelease = [
                 'tag_name' => $tag['name'],

diff --git a/src/AppBundle/DataFixtures/ORM/LoadVersionData.php b/src/AppBundle/DataFixtures/ORM/LoadVersionData.php
@@ -19,11 +19,22 @@ public function load(ObjectManager $manager)
             'message' => 'YAY',
             'published_at' => (date('Y') + 1) . '-10-15T07:49:21Z',
         ]);
-
         $manager->persist($version1);
+
+        $version2 = new Version($this->getReference('repo2'));
+        $version2->hydrateFromGithub([
+            'tag_name' => '1.0.21',
+            'name' => 'First release',
+            'prerelease' => false,
+            'message' => 'YAY 555',
+            'published_at' => (date('Y') + 1) . '-06-15T07:49:21Z',
+        ]);
+
+        $manager->persist($version2);
         $manager->flush();
 
         $this->addReference('version1', $version1);
+        $this->addReference('version2', $version2);
     }
 
     /**

diff --git a/tests/AppBundle/Consumer/SyncStarredReposTest.php b/tests/AppBundle/Consumer/SyncStarredReposTest.php
@@ -66,6 +66,9 @@ public function testProcessSuccessfulMessage()
         $em = $this->getMockBuilder('Doctrine\ORM\EntityManager')
             ->disableOriginalConstructor()
             ->getMock();
+        $em->expects($this->once())
+            ->method('isOpen')
+            ->willReturn(true);
 
         $doctrine = $this->getMockBuilder('Doctrine\Bundle\DoctrineBundle\Registry')
             ->disableOriginalConstructor()
@@ -164,6 +167,9 @@ public function testProcessUnexpectedError()
         $em = $this->getMockBuilder('Doctrine\ORM\EntityManager')
             ->disableOriginalConstructor()
             ->getMock();
+        $em->expects($this->once())
+            ->method('isOpen')
+            ->willReturn(true);
 
         $doctrine = $this->getMockBuilder('Doctrine\Bundle\DoctrineBundle\Registry')
             ->disableOriginalConstructor()
@@ -262,13 +268,19 @@ public function testProcessSuccessfulMessageNoStarToRemove()
         $em = $this->getMockBuilder('Doctrine\ORM\EntityManager')
             ->disableOriginalConstructor()
             ->getMock();
+        $em->expects($this->once())
+            ->method('isOpen')
+            ->willReturn(false); // simulate a closing manager
 
         $doctrine = $this->getMockBuilder('Doctrine\Bundle\DoctrineBundle\Registry')
             ->disableOriginalConstructor()
             ->getMock();
         $doctrine->expects($this->once())
             ->method('getManager')
             ->willReturn($em);
+        $doctrine->expects($this->once())
+            ->method('resetManager')
+            ->willReturn($em);
 
         $user = new User();
         $user->setId(123);