From 1ebe762ea6f306158bcef02ccc4a68711a9865ed Mon Sep 17 00:00:00 2001
From: Antoine Lambert <antoine.lambert@inria.fr>
Date: Thu, 12 Sep 2019 23:43:35 +0200
Subject: [PATCH] phabricator/lister: Do not override max_index when
 bootstrapping

Turns out all newly listed repositories were filtered out because of that.
Consequently, no entries in the listers database and no scheduler loading
tasks were created when listing a Phabricator instance.

Closes T1999
---
 swh/lister/core/tests/test_lister.py        |  9 +++++--
 swh/lister/phabricator/lister.py            |  3 +--
 swh/lister/phabricator/tests/test_lister.py | 30 +++++++++++++++++++--
 3 files changed, 36 insertions(+), 6 deletions(-)

diff --git a/swh/lister/core/tests/test_lister.py b/swh/lister/core/tests/test_lister.py
index dec68c6..b1cda8f 100644
--- a/swh/lister/core/tests/test_lister.py
+++ b/swh/lister/core/tests/test_lister.py
@@ -163,8 +163,7 @@ class HttpListerTester(HttpListerTesterBase, abc.ABC):
         if m and (len(m.groups()) > 0):
             return self.convert_type(m.group(1))
 
-    @requests_mock.Mocker()
-    def test_fetch_multiple_pages_yesdb(self, http_mocker):
+    def create_fl_with_db(self, http_mocker):
         http_mocker.get(self.test_re, text=self.mock_response)
         db = init_db()
 
@@ -174,10 +173,16 @@ class HttpListerTester(HttpListerTesterBase, abc.ABC):
                 'args': {'db': db.url()}
                 }
             })
+        fl.db = db
         self.init_db(db, fl.MODEL)
 
         self.disable_scheduler(fl)
+        return fl
 
+    @requests_mock.Mocker()
+    def test_fetch_multiple_pages_yesdb(self, http_mocker):
+
+        fl = self.create_fl_with_db(http_mocker)
         fl.run(min_bound=self.first_index)
 
         self.assertEqual(fl.db_last_index(), self.last_index)
diff --git a/swh/lister/phabricator/lister.py b/swh/lister/phabricator/lister.py
index 7f60c14..ebf5fcf 100644
--- a/swh/lister/phabricator/lister.py
+++ b/swh/lister/phabricator/lister.py
@@ -119,11 +119,10 @@ class PhabricatorLister(IndexingHttpLister):
         params = '&order=oldest&limit=1'
         response = self.safely_issue_request(params)
         models_list = self.transport_response_simplified(response)
-        self.max_index = models_list[0]['indexable']
         models_list = self.filter_before_inject(models_list)
         injected = self.inject_repo_data_into_db(models_list)
         self.schedule_missing_tasks(models_list, injected)
-        return self.max_index
+        return models_list[0]['indexable']
 
 
 def get_repo_url(attachments):
diff --git a/swh/lister/phabricator/tests/test_lister.py b/swh/lister/phabricator/tests/test_lister.py
index f52e560..bd2297a 100644
--- a/swh/lister/phabricator/tests/test_lister.py
+++ b/swh/lister/phabricator/tests/test_lister.py
@@ -5,6 +5,9 @@
 import re
 import json
 import unittest
+
+import requests_mock
+
 from swh.lister.core.tests.test_lister import HttpListerTester
 from swh.lister.phabricator.lister import PhabricatorLister
 from swh.lister.phabricator.lister import get_repo_url
@@ -12,17 +15,29 @@ from swh.lister.phabricator.lister import get_repo_url
 
 class PhabricatorListerTester(HttpListerTester, unittest.TestCase):
     Lister = PhabricatorLister
-    test_re = re.compile(r'\&after=([^?&]+)')
+    # first request will have the after parameter empty
+    test_re = re.compile(r'\&after=([^?&]*)')
     lister_subdir = 'phabricator'
     good_api_response_file = 'api_response.json'
     good_api_response_undefined_protocol = 'api_response_undefined_'\
                                            'protocol.json'
     bad_api_response_file = 'api_empty_response.json'
-    first_index = 1
+    # first_index must be retrieved through a bootstrap process for Phabricator
+    first_index = None
     last_index = 12
     entries_per_page = 10
+
     convert_type = int
 
+    def request_index(self, request):
+        """(Override) This is needed to emulate the listing bootstrap
+        when no min_bound is provided to run
+        """
+        m = self.test_re.search(request.path_url)
+        idx = m.group(1)
+        if idx == str(self.last_index):
+            return int(idx)
+
     def get_fl(self, override_config=None):
         """(Override) Retrieve an instance of fake lister (fl).
 
@@ -58,3 +73,14 @@ class PhabricatorListerTester(HttpListerTester, unittest.TestCase):
         self.assertEqual(
                 'https://svn.blender.org/svnroot/bf-blender/',
                 get_repo_url(repo['attachments']['uris']['uris']))
+
+    @requests_mock.Mocker()
+    def test_full_listing(self, http_mocker):
+        fl = self.create_fl_with_db(http_mocker)
+
+        fl.run()
+
+        self.assertEqual(fl.db_last_index(), self.last_index)
+        ingested_repos = list(fl.db_query_range(self.first_index,
+                                                self.last_index))
+        self.assertEqual(len(ingested_repos), self.entries_per_page)