diff --git a/module_build_service/models.py b/module_build_service/models.py index 8fa6a80a..1fe0326d 100644 --- a/module_build_service/models.py +++ b/module_build_service/models.py @@ -358,21 +358,33 @@ class ModuleBuild(MBSBase): # the minimal `stream_version` is 280000. min_stream_version = (stream_version // 10000) * 10000 - # Prepare the subquery to find out all unique name:stream records. - subq = session.query( - func.max(sqlalchemy.cast(ModuleBuild.version, db.BigInteger)).label("maxversion") - ).filter_by(name=name, state=BUILD_STATES["ready"]).filter( - stream_version <= stream_version).filter( - stream_version >= min_stream_version).subquery('t2') + query = session.query(ModuleBuild)\ + .filter(ModuleBuild.name == name)\ + .filter(ModuleBuild.state == BUILD_STATES["ready"])\ + .filter(ModuleBuild.stream_version <= stream_version)\ + .filter(ModuleBuild.stream_version >= min_stream_version)\ + .order_by(ModuleBuild.version.desc()) + builds = query.all() - # Use the subquery to actually return all the columns for its results. - query = session.query(ModuleBuild).join( - subq, and_( - ModuleBuild.name == name, - ModuleBuild.stream_version <= stream_version, - ModuleBuild.stream_version >= min_stream_version, - sqlalchemy.cast(ModuleBuild.version, db.BigInteger) == subq.c.maxversion)) - return query.all() + # In case there are multiple versions of single name:stream build, we want to return + # the latest version only. The `builds` are ordered by "version" desc, so we + # can just get the first (greatest) version of name:stream. + # TODO: Is there a way how to do that nicely in the SQL query itself? + seen = {} # {"n:s": v, ...} + ret = [] + for build in builds: + ns = "%s:%s" % (build.name, build.stream) + if ns in seen and seen[ns] != build.version: + # Skip the builds if we already handled this nsv before. + continue + elif ns in seen and seen[ns] == build.version: + # Different context of the NSV + ret.append(build) + continue + + seen[ns] = build.version + ret.append(build) + return ret @staticmethod def get_build_by_koji_tag(session, tag): diff --git a/module_build_service/utils/mse.py b/module_build_service/utils/mse.py index caa86555..0a75db1f 100644 --- a/module_build_service/utils/mse.py +++ b/module_build_service/utils/mse.py @@ -218,11 +218,52 @@ def _get_base_module_mmds(mmd): if ns in seen: continue + # Get the MMD for particular buildrequired name:stream to find out the virtual + # streams according to which we can find the compatible streams later. + # The returned MMDs are all the module builds for name:stream in the highest + # version. Given the base module does not depend on other modules, it can appear + # only in single context and therefore the `mmds` should always contain just + # zero or one module build. + mmds = resolver.get_module_modulemds(name, stream) + if not mmds: + continue + stream_mmd = mmds[0] + + # In case there are no virtual_streams in the buildrequired name:stream, + # it is clear that there are no compatible streams, so return just this + # `stream_mmd`. + xmd = stream_mmd.get_xmd() + if "mbs" not in xmd.keys() or "virtual_streams" not in xmd["mbs"].keys(): + seen.add(ns) + ret.append(stream_mmd) + continue + + # Get the list of compatible virtual streams. + virtual_streams = xmd["mbs"]["virtual_streams"] + 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()]) + xmd = mmd_.get_xmd() + if "mbs" not in xmd.keys() or "virtual_streams" not in xmd["mbs"].keys(): + # This module does not contain any virtual stream, so it cannot + # be compatible with our buildrequired module. + continue + + # Check if some virtual stream from buildrequired module exists in + # this module. That would mean the modules are compatible. + mmd_virtual_streams = xmd["mbs"]["virtual_streams"] + for virtual_stream in virtual_streams: + if virtual_stream in mmd_virtual_streams: + break + else: + # No stream from `virtual_streams` exist in `mmd_virtual_streams`, so this + # module is not compatible with our buildrequired module. + continue + 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 diff --git a/tests/__init__.py b/tests/__init__.py index 8352a2e9..e9f43042 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -126,7 +126,14 @@ def init_data(data_size=10, contexts=False, multiple_stream_versions=False, scra for stream in ["f28.0.0", "f29.0.0", "f29.1.0", "f29.2.0"]: mmd.set_name("platform") mmd.set_stream(stream) + + # Set the virtual_streams based on "fXY" to mark the platform streams + # with the same major stream_version compatible. + xmd = glib.from_variant_dict(mmd.get_xmd()) + xmd['mbs']['virtual_streams'] = [stream[:3]] + mmd.set_xmd(glib.dict_values(xmd)) import_mmd(db.session, mmd) + # Just to possibly confuse tests by adding another base module. mmd.set_name("bootstrap") mmd.set_stream(stream) @@ -724,7 +731,7 @@ def reuse_shared_userspace_init_data(): def make_module(nsvc, requires_list=None, build_requires_list=None, base_module=None, - filtered_rpms=None, xmd=None, store_to_db=True): + filtered_rpms=None, xmd=None, store_to_db=True, virtual_streams=None): """ Creates new models.ModuleBuild defined by `nsvc` string with requires and buildrequires set according to ``requires_list`` and ``build_requires_list``. @@ -742,6 +749,7 @@ def make_module(nsvc, requires_list=None, build_requires_list=None, base_module= default key/value pairs are added if not present. :param bool store_to_db: whether to store created module metadata to the database. + :param list virtual_streams: List of virtual streams provided by this module. :return: New Module Build if set to store module metadata to database, otherwise the module metadata is returned. :rtype: ModuleBuild or Modulemd.Module @@ -795,6 +803,9 @@ def make_module(nsvc, requires_list=None, build_requires_list=None, base_module= if 'mse' not in xmd_mbs: xmd_mbs['mse'] = 'true' + if virtual_streams: + xmd_mbs['virtual_streams'] = virtual_streams + mmd.set_xmd(glib.dict_values(xmd)) if not store_to_db: diff --git a/tests/test_models/test_models.py b/tests/test_models/test_models.py index 2b1c9863..e98d19e2 100644 --- a/tests/test_models/test_models.py +++ b/tests/test_models/test_models.py @@ -24,7 +24,7 @@ import os from module_build_service.utils import to_text_type from tests.test_models import init_data, module_build_from_modulemd -from tests import (init_data as init_data_contexts, clean_database) +from tests import (init_data as init_data_contexts, clean_database, make_module) from module_build_service import conf, Modulemd from module_build_service.models import ComponentBuild, ModuleBuild, make_session @@ -142,3 +142,25 @@ class TestModelsGetStreamsContexts: builds = set(["%s:%s:%s:%s" % (build.name, build.stream, str(build.version), build.context) for build in builds]) assert builds == set(['platform:f29.0.0:3:00000000', 'platform:f29.1.0:3:00000000']) + + def test_get_last_builds_in_stream_version_lte_different_versions(self): + """ + Tests that get_last_builds_in_stream_version_lte works in case the + name:stream_ver modules have different versions. + """ + clean_database(False) + make_module("platform:f29.1.0:10:old_version", {}, {}, virtual_streams=["f29"]) + make_module("platform:f29.1.0:15:c11.another", {}, {}, virtual_streams=["f29"]) + make_module("platform:f29.1.0:15:c11", {}, {}, virtual_streams=["f29"]) + make_module("platform:f29.2.0:0:old_version", {}, {}, virtual_streams=["f29"]) + make_module("platform:f29.2.0:1:c11", {}, {}, virtual_streams=["f29"]) + make_module("platform:f29.3.0:15:old_version", {}, {}, virtual_streams=["f29"]) + make_module("platform:f29.3.0:20:c11", {}, {}, virtual_streams=["f29"]) + + with make_session(conf) as session: + builds = ModuleBuild.get_last_builds_in_stream_version_lte( + session, "platform", 290200) + builds = set(["%s:%s:%s:%s" % (build.name, build.stream, str(build.version), + build.context) for build in builds]) + assert builds == set(['platform:f29.1.0:15:c11', 'platform:f29.1.0:15:c11.another', + 'platform:f29.2.0:1:c11']) diff --git a/tests/test_utils/test_utils_mse.py b/tests/test_utils/test_utils_mse.py index e82ccd40..49bc78e5 100644 --- a/tests/test_utils/test_utils_mse.py +++ b/tests/test_utils/test_utils_mse.py @@ -337,9 +337,9 @@ class TestUtilsModuleStreamExpansion: and lorem:1 modules which require base:f29 module requiring platform:f29 module :). """ - f290000 = make_module("platform:f29.0.0:0:c11", {}, {}) - f290100 = make_module("platform:f29.1.0:0:c11", {}, {}) - f290200 = make_module("platform:f29.2.0:0:c11", {}, {}) + f290000 = make_module("platform:f29.0.0:0:c11", {}, {}, virtual_streams=["f29"]) + f290100 = make_module("platform:f29.1.0:0:c11", {}, {}, virtual_streams=["f29"]) + f290200 = make_module("platform:f29.2.0:0:c11", {}, {}, virtual_streams=["f29"]) make_module("gtk:1:0:c2", {"platform": ["f29"]}, {}, f290000) make_module("gtk:1:1:c2", {"platform": ["f29"]}, {}, f290100) make_module("gtk:1:2:c2", {"platform": ["f29"]}, {}, f290100) @@ -362,7 +362,7 @@ class TestUtilsModuleStreamExpansion: 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']) + brs['platform'].set(['f29.1.0', 'f29.2.0']) deps[0].set_buildrequires(brs) mmd.set_dependencies(deps) @@ -375,3 +375,31 @@ class TestUtilsModuleStreamExpansion: for mmd_ in mmds: actual.add('{}:{}'.format(mmd_.get_name(), mmd_.get_stream())) assert actual == expected + + @pytest.mark.parametrize('virtual_streams', (None, ["f29"], ["lp29"])) + def test__get_base_module_mmds_virtual_streams(self, virtual_streams): + """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(['f29.2.0']) + deps[0].set_buildrequires(brs) + mmd.set_dependencies(deps) + + make_module("platform:lp29.1.1:12:c11", {}, {}, virtual_streams=virtual_streams) + + mmds = module_build_service.utils.mse._get_base_module_mmds(mmd) + if virtual_streams == ["f29"]: + expected = set(['platform:f29.0.0', 'platform:f29.1.0', 'platform:f29.2.0', + 'platform:lp29.1.1']) + else: + 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 diff --git a/tests/test_views/test_views.py b/tests/test_views/test_views.py index db5b9739..8ec579a0 100644 --- a/tests/test_views/test_views.py +++ b/tests/test_views/test_views.py @@ -1289,6 +1289,9 @@ class TestViews: @patch('module_build_service.auth.get_user', return_value=user) @patch('module_build_service.scm.SCM') def test_submit_build_invalid_basemodule_stream(self, mocked_scm, mocked_get_user): + # By default tests do not provide platform:f28.0.0, but just platform:f28. + # Therefore we want to enable multiple_stream_versions. + init_data(2, multiple_stream_versions=True) FakeSCM(mocked_scm, 'testmodule', 'testmodule.yaml', '620ec77321b2ea7b0d67d82992dda3e1d67055b4') @@ -1302,12 +1305,12 @@ class TestViews: rv = self.client.post('/module-build-service/1/module-builds/', data=json.dumps(data)) result = json.loads(rv.data) assert result == { - 'error': 'Bad Request', - 'status': 400, - 'message': ('No dependency combination was satisfied. Please verify the ' - 'buildrequires in your modulemd have previously been built.') + 'error': 'Unprocessable Entity', + 'status': 422, + 'message': ('None of the base module (platform) streams in the buildrequires ' + 'section could be found') } - assert rv.status_code == 400 + assert rv.status_code == 422 @patch('module_build_service.auth.get_user', return_value=user) @patch('module_build_service.scm.SCM')