Fix unecessary re-download of repo when not using metalink

https://bugzilla.redhat.com/show_bug.cgi?id=1373992

 * Passes existing tests

 * Tested `dnf --refresh check-update` now avoids re-downloading
   repos specified using baseurl (e.g. for mirror on local network).

 * Includes new test cases.

   The test for a new repomd.xml just uses an empty file...
   there doesn't seem much point writing a whole fake one.

dnf/repo.py

@@ -726,12 +726,8 @@ def _try_cache(self):
         self._reset_metadata_expired()
         return True
 
-    def _try_revive(self):
+    def _try_revive_by_metalink(self):
         """Use metalink to check whether our metadata are still current."""
-        if not self.metadata:
-            return False
-        if not self.metalink:
-            return False
         repomd_fn = self.metadata._repo_dct['repomd']
         with dnf.util.tmpdir() as tmpdir, open(repomd_fn) as repomd:
             handle = self._handle_new_remote(tmpdir)
@@ -758,9 +754,35 @@ def _try_revive(self):
                     logger.debug("reviving: failed for '%s', mismatched %s sum.",
                                  self.id, algo)
                     return False
-        logger.debug("reviving: '%s' can be revived.", self.id)
+        logger.debug("reviving: '%s' can be revived - metalink checksums match.", self.id)
+        return True
+
+    def _try_revive_by_repomd(self):
+        """Use repomd to check whether our metadata are still current."""
+        repomd_fn = self.metadata._repo_dct['repomd']
+        with dnf.util.tmpdir() as tmpdir, open(repomd_fn) as repomd:
+            handle = self._handle_new_remote(tmpdir)
+            handle.yumdlist = librepo.YUM_REPOMDONLY
+            result = handle._perform()
+            fresh_repomd_fn = result.rpmmd_repo['repomd']
+            with open(fresh_repomd_fn) as fresh_repomd:
+                if repomd.read() != fresh_repomd.read():
+                    logger.debug("reviving: failed for '%s', mismatched repomd.",
+                             self.id)
+                    return False
+        logger.debug("reviving: '%s' can be revived - repomd matches.", self.id)
         return True
 
+    def _try_revive(self):
+        """Use metalink to check whether our metadata are still current."""
+        if not self.metadata:
+            return False
+
+        if self.metalink:
+            return self._try_revive_by_metalink()
+        else:
+            return self._try_revive_by_repomd()
+
     def _configure_from_options(self, opts):
         if getattr(opts, 'cacheonly', None):
             self._md_only_cached = True

tests/test_repo.py

@@ -412,6 +412,46 @@ def test_reviving_404(self, new_remote_m):
         with mock.patch('dnf.repo.Repo._cachedir', REPOS + "/rpm"):
             self.assertRaises(dnf.exceptions.RepoError, self.repo.load)
 
+    @mock.patch.object(dnf.repo.Metadata, '_reset_age')
+    @mock.patch('dnf.repo.Repo._handle_new_remote')
+    def test_reviving_baseurl(self, new_remote_m, reset_age_m):
+        self.repo._md_expire_cache()
+        self.repo.baseurl = 'http://meh'
+        remote_handle_m = new_remote_m()
+        remote_handle_m._perform().rpmmd_repo = \
+            { 'repomd': REPOS + '/rpm/repodata/repomd.xml'}
+        with mock.patch('dnf.repo.Repo._cachedir', REPOS + "/rpm"):
+            self.assertTrue(self.repo.load())
+        self.assertTrue(remote_handle_m.fetchmirrors)
+        self.assertFalse(self.repo._expired)
+        reset_age_m.assert_called_with()
+
+    @mock.patch.object(dnf.repo.Metadata, '_reset_age')
+    @mock.patch('dnf.repo.Repo._handle_new_remote')
+    def test_reviving_baseurl_mismatched(self, new_remote_m, _):
+        self.repo._md_expire_cache()
+        self.repo.baseurl = 'http://meh'
+        remote_handle_m = new_remote_m()
+        remote_handle_m._perform().rpmmd_repo = \
+            { 'repomd': '/dev/null'}
+        # can not do the entire load() here, it would run on after try_revive()
+        # failed.
+        with mock.patch('dnf.repo.Repo._cachedir', REPOS + "/rpm"):
+            self.repo._try_cache()
+            self.assertFalse(self.repo._try_revive())
+
+    @mock.patch('dnf.repo.Repo._handle_new_remote')
+    def test_reviving_baseurl_404(self, new_remote_m):
+        url = 'http://meh'
+        self.repo._md_expire_cache()
+        self.repo.baseurl = url
+        lr_exc = librepo.LibrepoException(
+            librepo.LRE_CURL, 'Error HTTP/FTP status code: 404', 'Curl error.')
+        exc = dnf.repo._DetailedLibrepoError(lr_exc, url)
+        new_remote_m()._perform = mock.Mock(side_effect=exc)
+        with mock.patch('dnf.repo.Repo._cachedir', REPOS + "/rpm"):
+            self.assertRaises(dnf.exceptions.RepoError, self.repo.load)
+
 
 class DownloadPayloadsTest(RepoTestMixin, support.TestCase):