From 1633e5591775061e2b20f85e9ef3d7d0fecc6feb Mon Sep 17 00:00:00 2001
From: mprahl <mprahl@redhat.com>
Date: Oct 26 2018 18:39:54 +0000
Subject: Remove duplicate modulemds returned in utils.mse._get_base_module_mmds


---

diff --git a/module_build_service/utils/mse.py b/module_build_service/utils/mse.py
index a443472..65eed77 100644
--- a/module_build_service/utils/mse.py
+++ b/module_build_service/utils/mse.py
@@ -207,12 +207,31 @@ def _get_base_module_mmds(mmd):
             if name not in buildrequires.keys():
                 continue
 
-            for stream in buildrequires[name].get():
+            # Since the query below uses stream_version_lte, we can sort the streams by stream
+            # version in descending order to not perform unnecessary queries. Otherwise, if the
+            # streams are f29.1.0 and f29.2.0, then two queries will occur, causing f29.1.0 to be
+            # returned twice. Only one query for that scenario is necessary.
+            sorted_desc_streams = sorted(
+                buildrequires[name].get(), reverse=True, key=models.ModuleBuild.get_stream_version)
+            for stream in sorted_desc_streams:
                 ns = ":".join([name, stream])
                 if ns in seen:
                     continue
+
+                mmds = resolver.get_module_modulemds(name, stream, stream_version_lte=True)
+                ret_chunk = []
+                # Add the returned mmds to the `seen` set to avoid querying those individually if
+                # they are part of the buildrequire streams for this base module
+                for mmd_ in mmds:
+                    mmd_ns = ":".join([mmd_.get_name(), mmd_.get_stream()])
+                    # An extra precaution to ensure there are no duplicates in the event the sorting
+                    # above wasn't flawless
+                    if mmd_ns not in seen:
+                        ret_chunk.append(mmd_)
+                        seen.add(mmd_ns)
+                ret += ret_chunk
+                # Just in case it was queried for but MBS didn't find it
                 seen.add(ns)
-                ret += resolver.get_module_modulemds(name, stream, stream_version_lte=True)
             break
     return ret
 
diff --git a/tests/test_utils/test_utils_mse.py b/tests/test_utils/test_utils_mse.py
index 7b7b46b..e82ccd4 100644
--- a/tests/test_utils/test_utils_mse.py
+++ b/tests/test_utils/test_utils_mse.py
@@ -18,12 +18,14 @@
 # OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
 # SOFTWARE.
 
+import os
+
 import pytest
 
 import module_build_service.utils
 from module_build_service import glib
 from module_build_service.errors import StreamAmbigous
-from tests import (db, clean_database, make_module)
+from tests import db, clean_database, make_module, init_data, base_dir
 
 
 class TestUtilsModuleStreamExpansion:
@@ -352,3 +354,24 @@ class TestUtilsModuleStreamExpansion:
         self._generate_default_modules_modules_multiple_stream_versions()
         nsvcs = self._get_mmds_required_by_module_recursively(module_build)
         assert set(nsvcs) == set(expected)
+
+    def test__get_base_module_mmds(self):
+        """Ensure the correct results are returned without duplicates."""
+        init_data(data_size=1, multiple_stream_versions=True)
+        mmd = module_build_service.utils.load_mmd(
+            os.path.join(base_dir, 'staged_data', 'testmodule_v2.yaml'), True)
+        deps = mmd.get_dependencies()
+        brs = deps[0].get_buildrequires()
+        brs['platform'].set(['platform:f29.1.0', 'platform:f29.2.0'])
+        deps[0].set_buildrequires(brs)
+        mmd.set_dependencies(deps)
+
+        mmds = module_build_service.utils.mse._get_base_module_mmds(mmd)
+        expected = set(['platform:f29.0.0', 'platform:f29.1.0', 'platform:f29.2.0'])
+        # Verify no duplicates were returned before doing set operations
+        assert len(mmds) == len(expected)
+        # Verify the expected ones were returned
+        actual = set()
+        for mmd_ in mmds:
+            actual.add('{}:{}'.format(mmd_.get_name(), mmd_.get_stream()))
+        assert actual == expected