Clean up job/folder path handling

Avoid repeatedly serializing and splitting the root path component of
jobs being added to the list to be processed and returned.

Attempting to split a string path using 'job' introduces a bug if a job
ever happens to also use the name 'job' as part of its name. This could
be fixed by using root.split('/job/') but it still seems hackish.

Maintain the root path to the folder in the hierarchy as a list of path
components, add to it when descending and store the result of joining
it with '/job/' as a separate variable to avoid needing to re-parse.

Change-Id: I074ad74ad935c50716141a79822ca84999734c3f

jenkins/__init__.py

@@ -1025,25 +1025,23 @@ class Jenkins(object):
         """
         jobs_list = []
 
-        jobs = [(0, "", self.get_info(query=JOBS_QUERY)['jobs'])]
+        jobs = [(0, [], self.get_info(query=JOBS_QUERY)['jobs'])]
         for lvl, root, lvl_jobs in jobs:
             if not isinstance(lvl_jobs, list):
                 lvl_jobs = [lvl_jobs]
             for job in lvl_jobs:
+                path = root + [job[u'name']]
                 # insert fullname info if it doesn't exist to
                 # allow callers to easily reference unambiguously
                 if u'fullname' not in job:
-                    job[u'fullname'] = '/'.join(
+                    job[u'fullname'] = '/'.join(path)
-                        [p for p in root.split('/')
-                         if p and p != 'job'] +
-                        [job[u'name']])
                 jobs_list.append(job)
                 if 'jobs' in job:  # folder
                     if folder_depth is None or lvl < folder_depth:
-                        path = '/job/'.join((root, job[u'name']))
+                        url_path = ''.join(['/job/' + p for p in path])
                         jobs.append(
                             (lvl + 1, path,
-                             self.get_info(path,
+                             self.get_info(url_path,
                                            query=JOBS_QUERY)['jobs']))
         return jobs_list
 

tests/jobs/base.py

@@ -49,6 +49,11 @@ class JenkinsGetJobsTestBase(JenkinsJobsTestBase):
         ]
     )
 
+    jobs_in_folder_named_job = [
+        [{'name': 'job', 'jobs': None}],  # actually a folder :-)
+        [{'name': 'my_job'}]  # "job" folder jobs
+    ]
+
 
 def build_jobs_list_responses(jobs_list, server_url):
     responses = []

tests/jobs/test_getall.py

@@ -106,3 +106,23 @@ class JenkinsGetAllJobsTest(JenkinsGetJobsTestBase):
         ]
         self.assertEqual(expected_request_urls,
                          self.got_request_urls(jenkins_mock))
+
+    @patch.object(jenkins.Jenkins, 'jenkins_open')
+    def test_folder_named_job(self, jenkins_mock):
+        response = build_jobs_list_responses(
+            self.jobs_in_folder_named_job, 'http://example.com/')
+        jenkins_mock.side_effect = iter(response)
+
+        jobs_info = self.j.get_all_jobs()
+
+        expected_fullnames = [u"job", u"job/my_job"]
+        self.assertEqual(len(expected_fullnames), len(jobs_info))
+        got_fullnames = [job[u"fullname"] for job in jobs_info]
+        self.assertEqual(expected_fullnames, got_fullnames)
+
+        expected_request_urls = [
+            self.make_url('api/json'),
+            self.make_url('job/job/api/json'),
+        ]
+        self.assertEqual(expected_request_urls,
+                         self.got_request_urls(jenkins_mock))