Merge lp:~twom/loggerhead/add-download-compatibility-api into lp:loggerhead

[Tom Wardill]

Add compatibility download API

There is a breaking change in the download URL structure to use /download/revid/filename. This restores the ability to use /download/revid/fileid/filename, but checks the newer structure first.

[lp:~twom/loggerhead/add-download-compatibility-api]

505. By Tom Wardill on 2020-07-08

Remove old code

506. By Tom Wardill on 2020-07-08

No need to check contents here

[Colin Watson (cjwatson)]

We may not be able to support this indefinitely, since I don't know how long the file ID APIs will continue to exist in Breezy, but we can cross that bridge when we come to it.

review: Approve

[lp:~twom/loggerhead/add-download-compatibility-api]

507. By Tom Wardill on 2020-07-08

Add encode for py3 compat

508. By Tom Wardill on 2020-07-08

Encode the correct argument

[Colin Watson (cjwatson)]

review: Approve

=== modified file 'loggerhead/controllers/download_ui.py'
--- loggerhead/controllers/download_ui.py	2020-06-04 19:21:17 +0000
+++ loggerhead/controllers/download_ui.py	2020-07-08 17:12:29 +0000
@@ -23,6 +23,7 @@
 
 from breezy.errors import (
     NoSuchFile,
+    NoSuchId,
     NoSuchRevision,
     )
 from breezy import osutils, urlutils
@@ -60,7 +61,12 @@
         try:
             path, filename, content = h.get_file(args[1], revid)
         except (NoSuchFile, NoSuchRevision):
-            raise httpexceptions.HTTPNotFound()
+            # Compatibility API for /download/rev_id/file_id/<filename>
+            try:
+                path, filename, content = h.get_file_by_fileid(
+                    args[1].encode('UTF-8'), revid)
+            except (NoSuchId, NoSuchRevision):
+                raise httpexceptions.HTTPNotFound()
         mime_type, encoding = mimetypes.guess_type(filename)
         if mime_type is None:
             mime_type = 'application/octet-stream'

=== modified file 'loggerhead/history.py'
--- loggerhead/history.py	2020-06-04 19:21:17 +0000
+++ loggerhead/history.py	2020-07-08 17:12:29 +0000
@@ -217,7 +217,7 @@
         finally:
             rev_info_memory_cache_lock.release()
 
-# Used to store locks that prevent multiple threads from building a 
+# Used to store locks that prevent multiple threads from building a
 # revision graph for the same branch at the same time, because that can
 # cause severe performance issues that are so bad that the system seems
 # to hang.
@@ -786,6 +786,20 @@
         return (display_path, breezy.osutils.basename(path),
                 rev_tree.get_file_text(path))
 
+    def get_file_by_fileid(self, fileid, revid):
+        """Returns (path, filename, file contents)"""
+        if not isinstance(fileid, bytes):
+            raise TypeError(fileid)
+        if not isinstance(revid, bytes):
+            raise TypeError(revid)
+        rev_tree = self._branch.repository.revision_tree(revid)
+        path = rev_tree.id2path(fileid)
+        display_path = path
+        if not display_path.startswith('/'):
+            path = '/' + path
+        return (display_path, breezy.osutils.basename(path),
+                rev_tree.get_file_text(path))
+
     def file_changes_for_revision_ids(self, old_revid, new_revid):
         """
         Return a nested data structure containing the changes in a delta::

=== modified file 'loggerhead/tests/test_controllers.py'
--- loggerhead/tests/test_controllers.py	2020-07-07 10:35:12 +0000
+++ loggerhead/tests/test_controllers.py	2020-07-08 17:12:29 +0000
@@ -394,6 +394,15 @@
             response,
             MatchesDownloadHeaders('myfilename', 'application/octet-stream'))
 
+    def test_download_with_revid(self):
+        app = self.setUpLoggerhead()
+        response = app.get('/download/1/myfilename-id/myfilename')
+        self.assertEqual(
+            b'some\nmultiline\ndata\nwith<htmlspecialchars\n', response.body)
+        self.assertThat(
+            response,
+            MatchesDownloadHeaders('myfilename', 'application/octet-stream'))
+
     def test_download_bad_revision(self):
         app = self.setUpLoggerhead()
         e = self.assertRaises(