launchpad-reviewers team mailing list archive

[Maximiliano Bertacchini <maximiliano.bertacchini@xxxxxxxxxxxxx>]

Maximiliano Bertacchini has proposed merging lp:~maxiberta/launchpad/git-get-blob-unexport-api into lp:launchpad.

Commit message:
Unexport GitRepository.getBlob(); return a binary blob instead of dict.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~maxiberta/launchpad/git-get-blob-unexport-api/+merge/293637

Unexport GitRepository.getBlob(); return a binary blob instead of dict.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~maxiberta/launchpad/git-get-blob-unexport-api into lp:launchpad.

=== modified file 'lib/lp/code/interfaces/githosting.py'
--- lib/lp/code/interfaces/githosting.py	2016-04-29 22:06:11 +0000
+++ lib/lp/code/interfaces/githosting.py	2016-05-03 15:27:15 +0000
@@ -96,5 +96,5 @@
         :param filename: Relative path of a file in the repository.
         :param rev: An optional revision. Defaults to 'HEAD'.
         :param logger: An optional logger.
-        :return: A dict with keys 'data' and 'size'.
+        :return: A binary string with the blob content.
         """

=== modified file 'lib/lp/code/interfaces/gitrepository.py'
--- lib/lp/code/interfaces/gitrepository.py	2016-04-29 15:17:20 +0000
+++ lib/lp/code/interfaces/gitrepository.py	2016-05-03 15:27:15 +0000
@@ -533,20 +533,12 @@
         :param logger: An optional logger.
         """
 
-    @operation_parameters(
-        filename=TextLine(
-            title=_("Relative path of file in the repository."),
-            required=True),
-        rev=TextLine(title=_("An optional revision. Defaults to 'HEAD'.")),
-        )
-    @export_read_operation()
-    @operation_for_version("devel")
     def getBlob(filename, rev=None):
         """Get a blob by file name from this repository.
 
         :param filename: Relative path of a file in the repository.
         :param rev: An optional revision. Defaults to 'HEAD'.
-        :return: A dict with keys 'data' and 'size'.
+        :return: A binary string with the blob content.
         """
 
 

=== modified file 'lib/lp/code/model/githosting.py'
--- lib/lp/code/model/githosting.py	2016-04-29 22:06:11 +0000
+++ lib/lp/code/model/githosting.py	2016-05-03 15:27:15 +0000
@@ -155,7 +155,8 @@
                 logger.info(
                     "Fetching file %s from repository %s" % (filename, path))
             url = "/repo/%s/blob/%s" % (path, quote(filename))
-            return self._get(url, params={"rev": rev})
+            response = self._get(url, params={"rev": rev})
+            return response["data"].decode("base64")
         except Exception as e:
             raise GitRepositoryScanFault(
                 "Failed to get file from Git repository: %s" % unicode(e))

=== modified file 'lib/lp/code/model/tests/test_githosting.py'
--- lib/lp/code/model/tests/test_githosting.py	2016-04-29 22:06:11 +0000
+++ lib/lp/code/model/tests/test_githosting.py	2016-05-03 15:27:15 +0000
@@ -185,20 +185,22 @@
                 self.client.delete, "123")
 
     def test_getBlob(self):
-        blob = b''.join(chr(i) for i in range(256)).encode("base64")
-        content = {"data": blob, "size": len(blob)}
+        blob = b''.join(chr(i) for i in range(256))
+        encoded_blob = blob.encode("base64")
+        content = {"data": encoded_blob, "size": len(encoded_blob)}
         with self.mockRequests(content=json.dumps(content)):
             response = self.client.getBlob("123", "dir/path/file/name")
-        self.assertEqual(content, response)
+        self.assertEqual(blob, response)
         self.assertRequest(
             "repo/123/blob/dir/path/file/name", method="GET")
 
     def test_getBlob_revision(self):
-        blob = b''.join(chr(i) for i in range(256)).encode("base64")
-        content = {"data": blob, "size": len(blob)}
+        blob = b''.join(chr(i) for i in range(256))
+        encoded_blob = blob.encode("base64")
+        content = {"data": encoded_blob, "size": len(encoded_blob)}
         with self.mockRequests(content=json.dumps(content)):
             response = self.client.getBlob("123", "dir/path/file/name", "dev")
-        self.assertEqual(content, response)
+        self.assertEqual(blob, response)
         self.assertRequest(
             "repo/123/blob/dir/path/file/name?rev=dev", method="GET")
 
@@ -210,7 +212,10 @@
                 self.client.getBlob, "123", "dir/path/file/name")
 
     def test_getBlob_url_quoting(self):
-        with self.mockRequests():
+        blob = b''.join(chr(i) for i in range(256))
+        encoded_blob = blob.encode("base64")
+        content = {"data": encoded_blob, "size": len(encoded_blob)}
+        with self.mockRequests(content=json.dumps(content)):
             self.client.getBlob("123", "dir/+file name?.txt", "+rev/ no?")
         self.assertRequest(
             "repo/123/blob/dir/%2Bfile%20name%3F.txt?rev=%2Brev%2F+no%3F",

=== modified file 'lib/lp/code/model/tests/test_gitrepository.py'
--- lib/lp/code/model/tests/test_gitrepository.py	2016-04-29 22:06:11 +0000
+++ lib/lp/code/model/tests/test_gitrepository.py	2016-05-03 15:27:15 +0000
@@ -2041,26 +2041,18 @@
     def test_getBlob_with_default_rev(self):
         repository = self.factory.makeGitRepository()
         hosting_client = FakeMethod()
-        expected_result = {
-            'data': u'Some text'.encode('base64'),
-            'size': len(u'Some Text'),
-            }
-        hosting_client.getBlob = FakeMethod(result=expected_result)
+        hosting_client.getBlob = FakeMethod(result='Some text')
         self.useFixture(ZopeUtilityFixture(hosting_client, IGitHostingClient))
         ret = repository.getBlob('src/README.txt')
-        self.assertEqual(expected_result, ret)
+        self.assertEqual('Some text', ret)
 
     def test_getBlob_with_rev(self):
         repository = self.factory.makeGitRepository()
         hosting_client = FakeMethod()
-        expected_result = {
-            'data': u'Some text'.encode('base64'),
-            'size': len(u'Some Text'),
-            }
-        hosting_client.getBlob = FakeMethod(result=expected_result)
+        hosting_client.getBlob = FakeMethod(result='Some text')
         self.useFixture(ZopeUtilityFixture(hosting_client, IGitHostingClient))
         ret = repository.getBlob('src/README.txt', 'some-rev')
-        self.assertEqual(expected_result, ret)
+        self.assertEqual('Some text', ret)
 
 
 class TestGitRepositorySet(TestCaseWithFactory):