PR#249: Fix MBS.validate_module_list to not remove modules with the same NSV but different context. - odcs

odcs

#249 Fix MBS.validate_module_list to not remove modules with the same NSV but different context.

Merged 5 years ago by jkaluza. Opened 5 years ago by jkaluza.

jkaluza/odcs module-multiple-contexts into master

Fix MBS.validate_module_list to not remove modules with the same NSV but different context.

Jan Kaluza • 5 years ago

6b29fe9

server/odcs/server/mbs.py

file modified

+17 -9

		`@@ -21,6 +21,7 @@`
		`#`

		`import requests`
		`+ from collections import defaultdict`

		`import odcs.server.utils`
		`from odcs.server import log`
		`@@ -129,30 +130,37 @@`
		`new_modules = []`
		`# Temporary dict with "name:stream" as key and list of module dicts`
		`# as value.`
		`- module_map = {}`
		`+ module_map = defaultdict(list)`

		`for module in modules:`
		`key = "%s:%s" % (module['name'], module['stream'])`

		- # If this module is not in `new_modules` yet, add it there and
		`- # continue to next module.`
		`- if key not in module_map:`
		`- module_map[key] = [module]`
		`+ # In case this is the first module with this name:stream,`
		`+ # just add it to new_modules.`
		`+ old_modules = module_map[key]`
		`+ if not old_modules:`
		`+ module_map[key].append(module)`
		`new_modules.append(module)`
		`continue`

		`# Check if there is already this module in new_modules, but in`
		`# different version. If so, raise an exception.`
		`- old_modules = module_map[key]`
		`- if (module['version'] != old_modules[0]['version']):`
		`+ if module['version'] != old_modules[0]['version']:`
		`raise ModuleLookupError(`
		`"%s:%s:%s:%s conflicts with %s:%s:%s:%s" % (`
		`module['name'], module["stream"], module["version"],`
		`module["context"], old_modules[0]['name'],`
		`old_modules[0]["stream"], old_modules[0]["version"],`
		`old_modules[0]["context"]))`
		`- else:`
		`- module_map[key].append(module)`
		`+`
		`+ # Check if there is already this module in new_modules in the very`
		`+ # same context - do not add it there, because it would be duplicate.`
		`+ if module['context'] in [m["context"] for m in old_modules]:`
		`+ continue`
		`+`
		`+ # Add it to new_modules/module_map.`
		`+ module_map[key].append(module)`
		`+ new_modules.append(module)`

		`if expand:`
		`added_module_list = new_modules`

server/tests/mbs.py

file modified

+13 -3

		`@@ -33,12 +33,14 @@`
		`from gi.repository import Modulemd`


		`- def make_module(name, stream, version, requires={}, mdversion=1):`
		`+ def make_module(name, stream, version, requires={}, mdversion=1,`
		`+ context=None):`
		`mmd = Modulemd.Module()`
		`mmd.set_mdversion(mdversion)`
		`mmd.set_name(name)`
		`mmd.set_stream(stream)`
		`mmd.set_version(version)`
		`+ mmd.set_context(context or '00000000')`
		`mmd.set_summary("foo")`
		`mmd.set_description("foo")`
		`licenses = Modulemd.SimpleSet()`
		`@@ -57,7 +59,7 @@`
		`'name': name,`
		`'stream': stream,`
		`'version': str(version),`
		`- 'context': '00000000',`
		`+ 'context': context or '00000000',`
		`'modulemd': mmd.dumps()`
		`}`

		`@@ -104,7 +106,15 @@`

		`# test_composerthread.py`
		`make_module('testmodule', 'master', 20170515074418, {}, 2),`
		`- make_module('testmodule', 'master', 20170515074419, {}, 2)`
		`+ make_module('testmodule', 'master', 20170515074419, {}, 2),`
		`+`
		`+ # multiple contexts`
		`+ make_module('parent', 'master', 1, {}, 2, context="a"),`
		`+ make_module('parent', 'master', 1, {}, 2, context="b"),`
		`+ make_module('testcontexts', 'master', 1, {"parent": "master"},`
		`+ 2, context="a"),`
		`+ make_module('testcontexts', 'master', 1, {"parent": "master"},`
		`+ 2, context="b"),`
		`]`

server/tests/test_backend.py

file modified

+35

		`@@ -78,6 +78,41 @@`
		`"moduleD:f26:20170806000000:00000000"]))`

		`@mock_mbs()`
		`+ def test_resolve_compose_module_multiple_contexts_no_deps(self):`
		`+ c = Compose.create(`
		`+ db.session, "me", PungiSourceType.MODULE,`
		`+ "testcontexts:master:1",`
		`+ COMPOSE_RESULTS["repository"], 3600,`
		`+ flags=COMPOSE_FLAGS["no_deps"])`
		`+ db.session.commit()`
		`+`
		`+ resolve_compose(c)`
		`+ db.session.commit()`
		`+`
		`+ c = db.session.query(Compose).filter(Compose.id == 1).one()`
		`+ self.assertEqual(c.source,`
		`+ " ".join(["testcontexts:master:1:a",`
		`+ "testcontexts:master:1:b"]))`
		`+`
		`+ @mock_mbs()`
		`+ def test_resolve_compose_module_multiple_contexts_deps(self):`
		`+ c = Compose.create(`
		`+ db.session, "me", PungiSourceType.MODULE,`
		`+ "testcontexts:master:1",`
		`+ COMPOSE_RESULTS["repository"], 3600)`
		`+ db.session.commit()`
		`+`
		`+ resolve_compose(c)`
		`+ db.session.commit()`
		`+`
		`+ c = db.session.query(Compose).filter(Compose.id == 1).one()`
		`+ self.assertEqual(c.source,`
		`+ " ".join(["parent:master:1:a",`
		`+ "parent:master:1:b",`
		`+ "testcontexts:master:1:a",`
		`+ "testcontexts:master:1:b"]))`
		`+`
		`+ @mock_mbs()`
		`def test_resolve_compose_module_no_deps(self):`
		`c = Compose.create(`
		`db.session, "me", PungiSourceType.MODULE,`

jkaluza commented 5 years ago

There was missing new_modules.append(module) line in this method,
but in this commit I also reformatted the method a bit to use
defaultdict and also fix detection of duplicate modules in a input
list.