Browse Source

Allow users to figure out why github hook sync failed on their own

Jordi Boggiano 6 years ago
parent
commit
a393d16108

+ 19 - 0
app/Resources/FOSUserBundle/views/Profile/show.html.twig

@@ -19,6 +19,25 @@
         <hr>
     {%- endif %}
 
+    {% if githubSync is not null and githubSync.createdAt > '-2 days'|date %}
+        <h3 class="font-normal">GitHub Hook Sync</h3>
+        {% if githubSync.completedAt is null %}
+            <p>Still running... <a href="{{ path('fos_user_profile_show') }}">Refresh</a></p>
+        {% else %}
+            <p>Completed at {{ githubSync.completedAt|date('Y-m-d H:i:s') }} UTC, <a href="{{ path('user_github_sync') }}">retry hook sync</a>.</p>
+            {% if githubSync.result.results is defined %}
+                <h4>{{ githubSync.result.results.hooks_setup }} hooks setup/updated</h4>
+                <h4>{{ githubSync.result.results.hooks_ok_unchanged }} hooks already setup and left unchanged</h4>
+                {% if githubSync.result.results.hooks_failed|length > 0 %}
+                    <h4>Hooks setup failures</h4>
+                    {% for fail in githubSync.result.results.hooks_failed %}
+                        <p><a href="{{ path('view_package', {name: fail.package}) }}">{{ fail.package }}</a> {{ fail.reason }}</p>
+                    {% endfor %}
+                {% endif %}
+            {% endif %}
+        {% endif %}
+    {% endif %}
+
     {% embed "PackagistWebBundle:Web:list.html.twig" with {noLayout: 'true', showAutoUpdateWarning: true} %}
         {% block content_title %}
             <h3 class="font-normal">{{ 'packages.yours'|trans }}</h3>

+ 1 - 1
app/config/routing.yml

@@ -8,7 +8,7 @@ fos_user_profile:
 
 fos_user_profile_show:
     path: /profile/
-    defaults: { _controller: PackagistWebBundle:User:myProfile }
+    defaults: { _controller: PackagistWebBundle:User:viewProfile }
     methods: [GET]
 
 fos_user_register:

+ 4 - 1
src/Packagist/WebBundle/Controller/UserController.php

@@ -16,6 +16,7 @@ use Doctrine\ORM\NoResultException;
 use FOS\UserBundle\Model\UserInterface;
 use Packagist\WebBundle\Entity\Package;
 use Packagist\WebBundle\Entity\User;
+use Packagist\WebBundle\Entity\Job;
 use Packagist\WebBundle\Model\RedisAdapter;
 use Pagerfanta\Adapter\DoctrineORMAdapter;
 use Pagerfanta\Pagerfanta;
@@ -137,7 +138,7 @@ class UserController extends Controller
      * @param Request $req
      * @return Response
      */
-    public function myProfileAction(Request $req)
+    public function viewProfileAction(Request $req)
     {
         $user = $this->container->get('security.token_storage')->getToken()->getUser();
         if (!is_object($user) || !$user instanceof UserInterface) {
@@ -145,6 +146,7 @@ class UserController extends Controller
         }
 
         $packages = $this->getUserPackages($req, $user);
+        $lastGithubSync = $this->getDoctrine()->getRepository(Job::class)->getLastGitHubSyncJob($user->getId());
 
         return $this->container->get('templating')->renderResponse(
             'FOSUserBundle:Profile:show.html.twig',
@@ -152,6 +154,7 @@ class UserController extends Controller
                 'packages' => $packages,
                 'meta' => $this->getPackagesMetadata($packages),
                 'user' => $user,
+                'githubSync' => $lastGithubSync,
             )
         );
     }

+ 12 - 0
src/Packagist/WebBundle/Entity/JobRepository.php

@@ -28,6 +28,18 @@ class JobRepository extends EntityRepository
         ]);
     }
 
+    public function getLastGitHubSyncJob(int $userId)
+    {
+        return $this->createQueryBuilder('j')
+            ->where('j.packageId = :userId')
+            ->andWhere('j.type = :type')
+            ->orderBy('j.createdAt', 'DESC')
+            ->getQuery()
+            ->setMaxResults(1)
+            ->setParameters(['userId' => $userId, 'type' => 'githubuser:migrate'])
+            ->getOneOrNullResult();
+    }
+
     public function getScheduledJobIds(): \Generator
     {
         $conn = $this->getEntityManager()->getConnection();

+ 26 - 11
src/Packagist/WebBundle/Service/GitHubUserMigrationWorker.php

@@ -45,9 +45,17 @@ class GitHubUserMigrationWorker
         }
 
         try {
-            $hookChanges = 0;
+            $results = ['hooks_setup' => 0, 'hooks_failed' => [], 'hooks_ok_unchanged' => 0];
             foreach ($packageRepository->getGitHubPackagesByMaintainer($id) as $package) {
-                $hookChanges += $this->setupWebHook($user->getGithubToken(), $package);
+                $result = $this->setupWebHook($user->getGithubToken(), $package);
+                if (is_string($result)) {
+                    $results['hooks_failed'][] = ['package' => $package->getName(), 'reason' => $result];
+                } elseif ($result === true) {
+                    $results['hooks_setup']++;
+                } elseif ($result === false) {
+                    $results['hooks_ok_unchanged']++;
+                }
+                // null result means not processed as not a github-like URL
             }
         } catch (\GuzzleHttp\Exception\ServerException $e) {
             return [
@@ -66,19 +74,20 @@ class GitHubUserMigrationWorker
         return [
             'status' => Job::STATUS_COMPLETED,
             'message' => 'Hooks updated for user '.$user->getUsername(),
-            'hookChanges' => $hookChanges,
+            'results' => $results,
         ];
     }
 
-    public function setupWebHook(string $token, Package $package): int
+    public function setupWebHook(string $token, Package $package)
     {
         if (!preg_match('#^(?:(?:https?|git)://([^/]+)/|git@([^:]+):)(?P<owner>[^/]+)/(?P<repo>.+?)(?:\.git|/)?$#', $package->getRepository(), $match)) {
-            return 0;
+            return;
         }
 
         $this->logger->debug('Updating hooks for package '.$package->getName());
 
         $repoKey = $match['owner'].'/'.$match['repo'];
+        $changed = false;
 
         try {
             $hooks = $this->getHooks($token, $repoKey);
@@ -106,32 +115,37 @@ class GitHubUserMigrationWorker
                 if ($hook['updated_at'] < '2018-09-04T13:00:00' || $hook['events'] != $hookData['events'] || $configWithoutSecret != $expectedConfigWithoutSecret || !$hook['active']) {
                     $this->logger->debug('Updating hook '.$hook['id']);
                     $this->request($token, 'PATCH', 'repos/'.$repoKey.'/hooks/'.$hook['id'], $hookData);
-                    $hasValidHook = true;
+                    $changed = true;
                 }
+
+                $hasValidHook = true;
                 unset($currentHooks[$index]);
             }
 
             foreach (array_merge(array_values($currentHooks), $legacyHooks) as $hook) {
                 $this->logger->debug('Deleting hook '.$hook['id'], ['hook' => $hook]);
                 $this->request($token, 'DELETE', 'repos/'.$repoKey.'/hooks/'.$hook['id']);
+                $changed = true;
             }
 
             if (!$hasValidHook) {
                 $this->logger->debug('Creating hook');
                 $this->request($token, 'POST', 'repos/'.$repoKey.'/hooks', $hookData);
+                $changed = true;
             }
         } catch (\GuzzleHttp\Exception\ClientException $e) {
-            // repo not found probably means the user does not have admin access to it on github
             if ($msg = $this->isAcceptableException($e)) {
                 $this->logger->debug($msg);
 
-                return 0;
+                return $msg;
             }
 
+            $this->logger->error('Rejected GitHub hook request', ['response' => (string) $e->getResponse()->getBody()]);
+
             throw $e;
         }
 
-        return 1;
+        return $changed;
     }
 
     public function deleteWebHook(string $token, Package $package): bool
@@ -213,6 +227,7 @@ class GitHubUserMigrationWorker
                 'url' => self::HOOK_URL,
                 'content_type' => 'json',
                 'secret' => $this->webhookSecret,
+                'insecure_ssl' => 0,
             ],
             'events' => [
                 'push',
@@ -225,11 +240,11 @@ class GitHubUserMigrationWorker
     {
         // repo not found probably means the user does not have admin access to it on github
         if ($e->getCode() === 404) {
-            return 'User has no access, skipping';
+            return 'GitHub user has no admin access to repository';
         }
 
         if ($e->getCode() === 403 && strpos($e->getMessage(), 'Repository was archived so is read-only') !== false) {
-            return 'Repository was archived';
+            return 'Repository is archived and read-only';
         }
 
         return false;

+ 1 - 1
src/Packagist/WebBundle/Service/Scheduler.php

@@ -58,7 +58,7 @@ class Scheduler
 
     public function scheduleUserScopeMigration(int $userId, string $oldScope, string $newScope): Job
     {
-        return $this->createJob('githubuser:migrate', ['id' => $userId, 'old_scope' => $oldScope, 'new_scope' => $newScope]);
+        return $this->createJob('githubuser:migrate', ['id' => $userId, 'old_scope' => $oldScope, 'new_scope' => $newScope], $userId);
     }
 
     private function getPendingUpdateJob(int $packageId, $updateEqualRefs = false, $deleteBefore = false)