From 00ad20dfcdd7bbffa6df631057fcc0a8ff99b752 Mon Sep 17 00:00:00 2001
From: Jan Kaluza <jkaluza@redhat.com>
Date: Wed, 28 Aug 2019 08:20:32 +0200
Subject: [PATCH] Reuse the latest module build found.

Before this commit, the base modules used in the `get_reusable_module` have
not been sorted and therefore when `get_reusable_module` tried to find out
the reusable module built against some base module, it could find a module
built against some old version of base module.

This could lead to situation when MBS tried to reuse components from quite
old module despite the fact that newer module build existed.

This commit fixes this by sorting the base modules by stream_version,
so MBS always tries to get the reusable module built against the latest
base module.
---
 module_build_service/utils/reuse.py |  4 ++
 tests/test_utils/test_utils.py      | 64 +++++++++++++++++++++++++++++
 2 files changed, 68 insertions(+)

diff --git a/module_build_service/utils/reuse.py b/module_build_service/utils/reuse.py
index b51cf266..7f37da52 100644
--- a/module_build_service/utils/reuse.py
+++ b/module_build_service/utils/reuse.py
@@ -94,6 +94,10 @@ def get_reusable_module(db_session, module):
     previous_module_build = None
 
     base_mmds = get_base_module_mmds(db_session, mmd)["ready"]
+    # Sort the base_mmds based on the stream version, higher version first.
+    base_mmds.sort(
+        key=lambda mmd: models.ModuleBuild.get_stream_version(mmd.get_stream_name(), False),
+        reverse=True)
     for base_mmd in base_mmds:
         mbs_xmd = mmd.get_xmd()["mbs"]
         if base_mmd.get_module_name() not in mbs_xmd["buildrequires"]:
diff --git a/tests/test_utils/test_utils.py b/tests/test_utils/test_utils.py
index 63cd02f2..2f5166bc 100644
--- a/tests/test_utils/test_utils.py
+++ b/tests/test_utils/test_utils.py
@@ -26,6 +26,7 @@ from shutil import copyfile, rmtree
 from datetime import datetime
 from werkzeug.datastructures import FileStorage
 from mock import patch
+from sqlalchemy.orm.session import make_transient
 from module_build_service.utils.general import load_mmd_file, mmd_to_str
 import module_build_service.utils
 import module_build_service.scm
@@ -1614,3 +1615,66 @@ class TestUtilsModuleReuse:
         assert build_module.reused_module
         assert reusable_module.id == build_module.reused_module_id
         assert reusable_module.id == reused_module.id
+
+    @patch(
+        "module_build_service.config.Config.allow_only_compatible_base_modules",
+        new_callable=mock.PropertyMock, return_value=False
+    )
+    def test_get_reusable_module_use_latest_build(self, cfg, db_session):
+        """
+        Test that the `get_reusable_module` tries to reuse the latest module in case when
+        multiple modules can be reused.
+        """
+        # Set "fedora" virtual stream to platform:f28.
+        platform_f28 = db_session.query(models.ModuleBuild).filter_by(name="platform").one()
+        mmd = platform_f28.mmd()
+        xmd = mmd.get_xmd()
+        xmd["mbs"]["virtual_streams"] = ["fedora"]
+        mmd.set_xmd(xmd)
+        platform_f28.modulemd = mmd_to_str(mmd)
+        platform_f28.update_virtual_streams(db_session, ["fedora"])
+
+        # Create platform:f29 with "fedora" virtual stream.
+        mmd = load_mmd(read_staged_data("platform"))
+        mmd = mmd.copy("platform", "f29")
+        xmd = mmd.get_xmd()
+        xmd["mbs"]["virtual_streams"] = ["fedora"]
+        mmd.set_xmd(xmd)
+        platform_f29 = module_build_service.utils.import_mmd(db_session, mmd)[0]
+
+        # Create another copy of `testmodule:master` which should be reused, because its
+        # stream version will be higher than the previous one. Also set its buildrequires
+        # to platform:f29.
+        latest_module = db_session.query(models.ModuleBuild).filter_by(
+            name="testmodule").filter_by(state=models.BUILD_STATES["ready"]).one()
+        # This is used to clone the ModuleBuild SQLAlchemy object without recreating it from
+        # scratch.
+        db_session.expunge(latest_module)
+        make_transient(latest_module)
+
+        # Change the platform:f28 buildrequirement to platform:f29 and recompute the build_context.
+        mmd = latest_module.mmd()
+        xmd = mmd.get_xmd()
+        xmd["mbs"]["buildrequires"]["platform"]["stream"] = "f29"
+        mmd.set_xmd(xmd)
+        latest_module.modulemd = mmd_to_str(mmd)
+        latest_module.build_context = module_build_service.models.ModuleBuild.contexts_from_mmd(
+            latest_module.modulemd
+        ).build_context
+        latest_module.buildrequires = [platform_f29]
+
+        # Set the `id` to None, so new one is generated by SQLAlchemy.
+        latest_module.id = None
+        db_session.add(latest_module)
+        db_session.commit()
+
+        module = db_session.query(models.ModuleBuild)\
+                           .filter_by(name="testmodule")\
+                           .filter_by(state=models.BUILD_STATES["build"])\
+                           .one()
+        db_session.commit()
+
+        reusable_module = module_build_service.utils.get_reusable_module(
+            db_session, module)
+
+        assert reusable_module.id == latest_module.id