From d2248617ebe8c574936f6dc7d24a76579e638fd0 Mon Sep 17 00:00:00 2001
From: Chenxiong Qi <cqi@redhat.com>
Date: Mar 24 2020 01:57:13 +0000
Subject: Refactor find latest image by its build NV


Signed-off-by: Chenxiong Qi <cqi@redhat.com>

---

diff --git a/freshmaker/lightblue.py b/freshmaker/lightblue.py
index d7e5efb..aa039ef 100644
--- a/freshmaker/lightblue.py
+++ b/freshmaker/lightblue.py
@@ -147,7 +147,11 @@ class ContainerImage(dict):
         return image
 
     def __hash__(self):
-        return hash((self['brew']['build']))
+        return hash((self.nvr))
+
+    @property
+    def nvr(self):
+        return self['brew']['build']
 
     def log_error(self, err):
         """
@@ -157,7 +161,7 @@ class ContainerImage(dict):
         """
         prefix = ""
         if 'brew' in self and 'build' in self['brew']:
-            prefix = self['brew']['build'] + ": "
+            prefix = self.nvr + ": "
         log.error("%s%s", prefix, err)
         if 'error' not in self or not self['error']:
             self['error'] = str(err)
@@ -191,8 +195,7 @@ class ContainerImage(dict):
         dockerfile = [file for file in self['parsed_data']['files']
                       if file['filename'] == 'Dockerfile']
         if not dockerfile:
-            log.warning('Image %s does not contain a Dockerfile.',
-                        self['brew']['build'])
+            log.warning('Image %s does not contain a Dockerfile.', self.nvr)
             return None
         return dockerfile[0]
 
@@ -885,7 +888,7 @@ class LightBlue(object):
         ret = []
         for image in images:
             if not content_sets & set(image["content_sets"]):
-                log.info(f"Will not rebuild {image['brew']['build']} because its content_sets "
+                log.info(f"Will not rebuild {image.nvr} because its content_sets "
                          "({image['content_sets']}) are not related to the requested content_sets"
                          " ({content_sets})")
             else:
@@ -1406,6 +1409,7 @@ class LightBlue(object):
             the given image - can be used for comparisons if needed
         :rtype: list
         """
+
         repos = self.find_all_container_repositories(published, release_categories)
         if not repos:
             return []
@@ -1422,30 +1426,26 @@ class LightBlue(object):
             # architecture, it is only interested in NVR, so group the images
             # by the same image['brew']['build'] and include just first one in the
             # image list.
-            sorted_images = sorted_by_nvr(
-                images, get_nvr=lambda image: image['brew']['build'], reverse=True)
+            sorted_images = sorted_by_nvr(images, reverse=True)
             images = []
-            for k, v in groupby(sorted_images, key=lambda x: x['brew']['build']):
+            for k, v in groupby(sorted_images, key=lambda item: item.nvr):
                 images.append(next(v))
 
         # In case we query for unpublished images, we need to return just
         # the latest NVR for given name-version, otherwise images would
         # contain all the versions which ever containing the srpm_name.
         if not published:
-            # Sort images by brew build NVR descending
-            sorted_images = sorted_by_nvr(
-                images, get_nvr=lambda image: image['brew']['build'], reverse=True)
 
-            # Iterate over all the images and only keep the very first one
-            # with the given name-version - this is the latest one.
-            images = []
-            seen_name_versions = []
-            for image in sorted_images:
-                parsed_build = koji.parse_NVR(image["brew"]["build"])
-                nv = "%s-%s" % (parsed_build["name"], parsed_build["version"])
-                if nv not in seen_name_versions:
-                    images.append(image)
-                    seen_name_versions.append(nv)
+            def _name_version_key(item):
+                nvr = koji.parse_NVR(item.nvr)
+                return f"{nvr['name']}-{nvr['version']}"
+
+            images = [
+                next(grouped_images) for _, grouped_images in groupby(
+                    sorted_by_nvr(images, reverse=True),
+                    key=_name_version_key
+                )
+            ]
 
         # Filter out images based on the filter_fnc.
         if filter_fnc:
@@ -1726,8 +1726,7 @@ class LightBlue(object):
                     # This `srpm_name` is not in image.
                     continue
 
-                unpublished = self.find_unpublished_image_for_build(
-                    image['brew']['build'])
+                unpublished = self.find_unpublished_image_for_build(image.nvr)
                 if not unpublished:
                     image.log_error(
                         "Cannot find unpublished version of image, Lightblue "
diff --git a/freshmaker/utils.py b/freshmaker/utils.py
index 6a658cf..52036d7 100644
--- a/freshmaker/utils.py
+++ b/freshmaker/utils.py
@@ -59,6 +59,9 @@ def sorted_by_nvr(lst, get_nvr=None, reverse=False):
         if get_nvr:
             nvr1 = get_nvr(item1)
             nvr2 = get_nvr(item2)
+        elif hasattr(item1, 'nvr') and hasattr(item2, 'nvr'):
+            nvr1 = item1.nvr
+            nvr2 = item2.nvr
         else:
             nvr1 = item1
             nvr2 = item2
diff --git a/tests/test_lightblue.py b/tests/test_lightblue.py
index f071a55..5886fa6 100644
--- a/tests/test_lightblue.py
+++ b/tests/test_lightblue.py
@@ -1446,6 +1446,111 @@ class TestQueryEntityFromLightBlue(helpers.FreshmakerTestCase):
     @patch('freshmaker.kojiservice.KojiService.get_build')
     @patch('freshmaker.kojiservice.KojiService.get_task_request')
     @patch('os.path.exists')
+    def test_images_with_content_set_packages_unpublished(
+            self, exists, koji_task_request, koji_get_build, cont_images, cont_repos
+    ):
+        exists.return_value = True
+        cont_repos.return_value = self.fake_repositories_with_content_sets
+
+        # "filtered_x-1-23" image will be filtered by filter_fnc.
+        cont_images.return_value = self.fake_container_images + [
+            ContainerImage.create(
+                {
+                    "content_sets": ["dummy-content-set-1"],
+                    "brew": {"build": "filtered_x-1-23"},
+                    "repositories": [
+                        {
+                            "repository": "product/repo1",
+                            "published": True,
+                            "tags": [{"name": "latest"}]
+                        }
+                    ]
+                }
+            )]
+        # Include the images for second time to ensure that they will be
+        # returned only once. This can happen when the image is multiarch.
+        cont_images.return_value += self.fake_container_images
+
+        koji_task_request.side_effect = self.fake_koji_task_requests
+        koji_get_build.side_effect = self.fake_koji_builds
+
+        lb = LightBlue(server_url=self.fake_server_url,
+                       cert=self.fake_cert_file,
+                       private_key=self.fake_private_key)
+
+        ret = lb.find_images_with_packages_from_content_set(
+            set(["openssl-1.2.3-3"]), ["dummy-content-set-1"],
+            filter_fnc=self._filter_fnc,
+            published=False
+        )
+
+        # Only the first image should be returned, because the first one
+        # is in repository "product1/repo1", but we have asked for images
+        # in repository "product/repo1".
+        self.assertEqual(1, len(ret))
+        self.assertEqual(ret,
+                         [
+                             {
+                                 "latest_released": True,
+                                 "generate_pulp_repos": True,
+                                 "repository": "rpms/repo-2",
+                                 "commit": "commit_hash2",
+                                 "target": "target2",
+                                 "git_branch": "mybranch",
+                                 "error": None,
+                                 "arches": None,
+                                 "odcs_compose_ids": None,
+                                 "parent_build_id": None,
+                                 "parent_image_builds": None,
+                                 "multi_arch_rpm_manifest": {},
+                                 "published": True,
+                                 "brew": {
+                                     "completion_date": u"20170421T04:27:51.000-0400",
+                                     "build": "package-name-2-4-12.10",
+                                     "package": "package-name-2"
+                                 },
+                                 'content_sets': ["dummy-content-set-1"],
+                                 'content_sets_source': 'lightblue_container_image',
+                                 'directly_affected': True,
+                                 "release_categories": ["Generally Available"],
+                                 'repositories': [
+                                     {'repository': 'product2/repo2', 'published': True,
+                                      'tags': [{"name": "latest"}]}
+                                 ],
+                                 'parsed_data': {
+                                     'files': [
+                                         {
+                                             'key': 'buildfile',
+                                             'content_url': 'http://git.repo.com/cgit/rpms/repo-2/plain/Dockerfile?id=commit_hash2',
+                                             'filename': 'Dockerfile'
+                                         },
+                                         {
+                                             'key': 'bogusfile',
+                                             'content_url': 'bogus_test_url',
+                                             'filename': 'bogus.file'
+                                         }
+                                     ]
+                                 },
+                                 'rpm_manifest': [{
+                                     'rpms': [
+                                         {
+                                             "srpm_name": "openssl",
+                                             "srpm_nevra": "openssl-1:1.2.3-1.src"
+                                         },
+                                         {
+                                             "srpm_name": "tespackage2",
+                                             "srpm_nevra": "testpackage2-10:1.2.3-1.src"
+                                         }
+                                     ]
+                                 }]
+                             },
+                         ])
+
+    @patch('freshmaker.lightblue.LightBlue.find_container_repositories')
+    @patch('freshmaker.lightblue.LightBlue.find_container_images')
+    @patch('freshmaker.kojiservice.KojiService.get_build')
+    @patch('freshmaker.kojiservice.KojiService.get_task_request')
+    @patch('os.path.exists')
     def test_images_with_content_set_packages_beta(
             self, exists, koji_task_request, koji_get_build, cont_images, cont_repos):
         exists.return_value = True

