Combine files from multiple pulp fields in RpmUnit [RHELDST-34141]

Update RmpUnit's `files` field to contain files from both
`files.file` and `repodata.filelists` pulp fields, ensuring
all files will be listed there.
This is to avoid reporting false positive file depsolving
warnings in ubi-manifest, because there are cases in which
the `files.file` field is missing, resulting in false positive
reports.

Author: drepelov

src/pubtools/pulplib/_impl/model/common.py

@@ -174,7 +174,15 @@ def _data_to_init_args(cls, data):
 
         for field in fields:
             pulp_field = field.metadata.get(PULP2_FIELD)
-            if pulp_field:
+            if not pulp_field:
+                continue
+            if pulp_field == "_combines":
+                # Field combines multiple pulp_fields, so we need to
+                # pass whole Pulp data to converter.
+                converter = get_converter(field)
+                value = converter(data)
+                out[field.name] = value
+            else:
                 value = lookup(data, pulp_field, absent)
                 if value is not absent:
                     converter = get_converter(field, value)

src/pubtools/pulplib/_impl/model/convert.py

@@ -1,13 +1,15 @@
 import datetime
+import xml.etree.ElementTree as ET
 import functools
 
 from frozenlist2 import frozenlist
 from frozendict.core import frozendict  # pylint: disable=no-name-in-module
 
+from pubtools.pulplib._impl.util import lookup
 from .attr import PULP2_PY_CONVERTER
 
 
-def get_converter(field, value):
+def get_converter(field, value=None):
     """Given an attrs target field and an input value from Pulp,
     return a converter function which should be used to convert the Pulp value
     into a Python representation."""
@@ -73,6 +75,32 @@ def write_timestamp(value):
     return value.strftime("%Y-%m-%dT%H:%M:%SZ")
 
 
+def combine_files_converter(data):
+    """Combine files from files.file and repodata.filelists"""
+    absent = object()
+    files = set()
+
+    from_files = lookup(data, "files.file", absent)
+    if from_files is not absent:
+        files.update(from_files)
+
+    filelist = lookup(data, "repodata.filelists", absent)
+    if filelist is not absent:
+        try:
+            parsed_filelist = ET.fromstring(filelist)
+            from_filelists = {
+                file.text
+                for file in parsed_filelist.findall("file")
+                if file.get("type") != "dir"
+            }
+        except ET.ParseError:
+            from_filelists = set()
+
+        files.update(from_filelists)
+
+    return frozenlist_or_none_converter(sorted(files) if files else None)
+
+
 def frozenlist_or_none_converter(obj, map_fn=(lambda x: x)):
     if obj is not None:
         return frozenlist(map_fn(obj))

src/pubtools/pulplib/_impl/model/unit/rpm.py

@@ -6,6 +6,7 @@
 from ..attr import pulp_attrib
 from ..common import PulpObject, schemaless_init
 from ..convert import (
+    combine_files_converter,
     frozenlist_or_none_converter,
     frozenlist_or_none_sorted_converter,
     timestamp_converter,
@@ -240,8 +241,8 @@ class RpmUnit(Unit):
     files = pulp_attrib(
         default=None,
         type=list,
-        converter=frozenlist_or_none_converter,
-        pulp_field="files.file",
+        pulp_field="_combines",  # combines multiple pulp_fields
+        pulp_py_converter=combine_files_converter,
         validator=optional_list_of(str),
     )
     """

tests/unit/test_rpm_unit.py

@@ -19,10 +19,19 @@ def test_user_metadata_fields():
                 "cdn_path": "/some/path/to/my.rpm",
                 "cdn_published": "2021-04-01T01:08:26",
             },
+            "repodata": {
+                "filelists": """<package arch='x86_64' name='bash' pkgid='123'>\n
+                    <version epoch='0' rel='1' ver='4.0' />\n
+                    <file>/usr/bin/bash</file>\n
+                    <file>/usr/bin/some/script.sh</file>\n
+                    <file>/usr/bin/some/script-provided.py</file>\n
+                    <file type='dir'>/usr/bin/some</file>\n
+                    </package>"""
+            },
             "files": {
                 "file": [
                     "/usr/bin/some/script.sh",
-                    "/usr/bin/another/script/provided.py",
+                    "/usr/bin/some/script-provided.py",
                 ],
             },
         }
@@ -38,7 +47,52 @@ def test_user_metadata_fields():
         content_type_id="rpm",
         cdn_path="/some/path/to/my.rpm",
         cdn_published=datetime.datetime(2021, 4, 1, 1, 8, 26),
-        files=["/usr/bin/some/script.sh", "/usr/bin/another/script/provided.py"],
+        files=[
+            "/usr/bin/bash",
+            "/usr/bin/some/script-provided.py",
+            "/usr/bin/some/script.sh",
+        ],
+    )
+
+
+def test_user_metadata_fields_malformed_xml():
+    """Malformed repodata.filelists is ignored"""
+
+    loaded = RpmUnit.from_data(
+        {
+            "_content_type_id": "rpm",
+            "name": "bash",
+            "filename": "bash-x86_64.rpm",
+            "version": "4.0",
+            "release": "1",
+            "arch": "x86_64",
+            "repodata": {
+                "filelists": """<package arch='x86_64' name='bash' pkgid='123'>\n
+                    <version epoch='0' rel='1' ver='4.0' />\n
+                    <file>/usr/bin/bash
+                    <file type='dir'>/usr/bin/some</file>\n
+                    </package>"""
+            },
+            "files": {
+                "file": [
+                    "/usr/bin/some/script.sh",
+                ],
+            },
+        }
+    )
+
+    # Malformed repodata.filelists is ignored and only the files from files.file
+    # field are present
+    assert loaded == RpmUnit(
+        name="bash",
+        filename="bash-x86_64.rpm",
+        version="4.0",
+        release="1",
+        arch="x86_64",
+        content_type_id="rpm",
+        files=[
+            "/usr/bin/some/script.sh",
+        ],
     )