set uploaded unit href to unit filename in repo

When uploading a unit, the href in the generated xml's location tag is
based on the content unit file path, which is an autogenerated unique
value. This sets the href back to the unit filename, which matches the
published repo layout and allows uploaded units to again be downloaded
from the published repo.

fixes #1556
https://pulp.plan.io/issues/1556

plugins/pulp_rpm/plugins/importers/yum/upload.py

@@ -375,9 +375,10 @@ def _handle_package(repo, type_id, unit_key, metadata, file_path, conduit, confi
     except TypeError:
         raise ModelInstantiationError()
 
-    # Extract the repodata snippets
+    # Extract/adjust the repodata snippets
     unit.repodata = rpm_parse.get_package_xml(file_path, sumtype=unit.checksumtype)
     _update_provides_requires(unit)
+    _update_location(unit)
 
     # check if the unit has duplicate nevra
     purge.remove_unit_duplicate_nevra(unit, repo)
@@ -388,26 +389,39 @@ def _handle_package(repo, type_id, unit_key, metadata, file_path, conduit, confi
     repo_controller.associate_single_unit(repo, unit)
 
 
-def _update_provides_requires(unit):
+def _fake_xml_element(repodata_snippet):
     """
-    Determines the provides and requires fields based on the RPM's XML snippet and updates
-    the model instance.
+    Wrap a snippet of xml in a fake element so it can coerced to an ElementTree Element
 
-    :param unit: the unit being added to Pulp; the metadata attribute must already have
-                 a key called 'repodata'
-    :type  unit: subclass of pulp.server.db.model.ContentUnit
+    :param repodata_snippet: Snippet of XML to be turn into an ElementTree Element
+    :type repodata_snippet: str
+
+    :return: Parsed ElementTree Element containing the parsed repodata snippet
+    :rtype: xml.etree.ElementTree.Element
     """
     try:
         # make a guess at the encoding
         codec = 'UTF-8'
-        unit.repodata['primary'].encode(codec)
+        repodata_snippet.encode(codec)
     except UnicodeEncodeError:
         # best second guess we have, and it will never fail due to the nature
         # of the encoding.
         codec = 'ISO-8859-1'
-        unit.repodata['primary'].encode(codec)
-    fake_xml = FAKE_XML % {'encoding': codec, 'xml': unit.repodata['primary']}
-    fake_element = ET.fromstring(fake_xml.encode(codec))
+    fake_xml = FAKE_XML % {'encoding': codec, 'xml': repodata_snippet}
+    # s/fromstring/phone_home/
+    return ET.fromstring(fake_xml.encode(codec))
+
+
+def _update_provides_requires(unit):
+    """
+    Determines the provides and requires fields based on the RPM's XML snippet and updates
+    the model instance.
+
+    :param unit: the unit being added to Pulp; the metadata attribute must already have
+                 a key called 'repodata'
+    :type  unit: subclass of pulp.server.db.model.ContentUnit
+    """
+    fake_element = _fake_xml_element(unit.repodata['primary'])
     utils.strip_ns(fake_element)
     primary_element = fake_element.find('package')
     format_element = primary_element.find('format')
@@ -419,6 +433,35 @@ def _update_provides_requires(unit):
                         requires_element.findall('entry')) if requires_element else []
 
 
+def _update_location(unit):
+    """
+    Fix a unit's repodata primary xml
+
+    When uploading a unit, the href in the generated xml's location tag is
+    based on the content unit file path, which is an autogenerated unique
+    value. This sets the href back to the unit filename, which matches the
+    published repo layout and allows uploaded units to again be downloaded
+    from the published repo.
+
+    :param unit: the unit being added to Pulp; the metadata attribute must already have
+                 a key called 'repodata'
+    :type  unit: subclass of pulp.server.db.model.ContentUnit
+    """
+    fake_element = _fake_xml_element(unit.repodata['primary'])
+    primary_element = fake_element.find('package')
+    location_element = primary_element.find('location')
+    location_element.set('href', unit.filename)
+    new_location = ET.tostring(location_element).strip()
+    lines = list(unit.repodata['primary'].splitlines())
+    # rather than deal with the xml namespaces, just
+    # replace the old location with the new one in-place
+    for i, line in enumerate(lines):
+        index = line.find('<location')
+        if index != -1:
+            lines[i] = line[:index] + new_location
+    unit.repodata['primary'] = '\n'.join(lines)
+
+
 def _extract_rpm_data(type_id, rpm_filename):
     """
     Extract a dict of information for a given RPM or SRPM.

plugins/test/unit/plugins/importers/yum/test_upload.py

@@ -675,3 +675,37 @@ def test__generate_rpm_data_sanitizes_checksum_type(self):
         self.assertEqual(metadata['license'], 'GPLv2')
         self.assertEqual(metadata['relativepath'], 'walrus-5.21-1.noarch.rpm')
         self.assertEqual(metadata['vendor'], None)
+
+
+class TestMangleRepodataPrimaryXML(unittest.TestCase):
+    # a snippet from repodata primary xml for a package
+    # this snippet has been truncated to only provide the tags needed to test
+    PRIMARY_XML = '''
+    <package type="rpm">
+      <location href="fixme" />
+      <format>
+        <rpm:provides>
+          <rpm:entry name="shark" flags="EQ" epoch="0" ver="0.1" rel="1"/>
+        </rpm:provides>
+        <rpm:requires>
+          <rpm:entry name="shark" flags="EQ" epoch="0" ver="0.1" rel="1"/>
+          <rpm:entry name="walrus" flags="EQ" epoch="0" ver="5.21" rel="1"/>
+        </rpm:requires>
+      </format>
+    </package>
+    '''
+
+    def setUp(self):
+        self.unit = models.RPM()
+        self.unit.repodata['primary'] = self.PRIMARY_XML
+        self.unit.filename = 'fixed-filename.rpm'
+
+    def test_update_provides_requires(self):
+        upload._update_provides_requires(self.unit)
+        self.assertEqual(len(self.unit.provides), 1)
+        self.assertEqual(len(self.unit.requires), 2)
+
+    def test_update_location(self):
+        upload._update_location(self.unit)
+        self.assertTrue('fixme' not in self.unit.repodata['primary'])
+        self.assertTrue(self.unit.filename in self.unit.repodata['primary'])