#249 Fix MBS.validate_module_list to not remove modules with the same NSV but different context.
Merged 7 months ago by jkaluza. Opened 7 months ago by jkaluza.

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

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"),

  ]

  

  

@@ -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,

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.

I don't have much background, but the code change looks good. But please be aware of the failed OIDC auth tests in test_views.

Pull-Request has been merged by jkaluza

7 months ago