Merge lp:~maxiberta/launchpad/git-get-blob-unexport-api into lp:launchpad

[Maximiliano Bertacchini]

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

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

[Colin Watson (cjwatson)]

review: Needs Fixing

[Colin Watson (cjwatson)]

Looks better now, thanks!

review: Approve

=== 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 18:43:05 +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 18:43:05 +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 18:43:05 +0000
@@ -155,7 +155,13 @@
                 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})
+            blob = response["data"].decode("base64")
+            if len(blob) != response["size"]:
+                raise GitRepositoryScanFault(
+                    "Unexpected size (%s vs %s)" % (
+                        len(blob), response["size"]))
+            return blob
         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 18:43:05 +0000
@@ -185,20 +185,20 @@
                 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))
+        content = {"data": blob.encode("base64"), "size": len(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))
+        content = {"data": blob.encode("base64"), "size": len(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,8 +210,42 @@
                 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))
+        content = {"data": blob.encode("base64"), "size": len(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",
             method="GET")
+
+    def test_getBlob_no_data(self):
+        with self.mockRequests(content=json.dumps({"size": 1})):
+            self.assertRaisesWithContent(
+                GitRepositoryScanFault,
+                "Failed to get file from Git repository: 'data'",
+                self.client.getBlob, "123", "dir/path/file/name")
+
+    def test_getBlob_no_size(self):
+        with self.mockRequests(content=json.dumps({"data": "data"})):
+            self.assertRaisesWithContent(
+                GitRepositoryScanFault,
+                "Failed to get file from Git repository: 'size'",
+                self.client.getBlob, "123", "dir/path/file/name")
+
+    def test_getBlob_bad_encoding(self):
+        content = {"data": "x", "size": 1}
+        with self.mockRequests(content=json.dumps(content)):
+            self.assertRaisesWithContent(
+                GitRepositoryScanFault,
+                "Failed to get file from Git repository: Incorrect padding",
+                self.client.getBlob, "123", "dir/path/file/name")
+
+    def test_getBlob_wrong_size(self):
+        blob = b''.join(chr(i) for i in range(256))
+        content = {"data": blob.encode("base64"), "size": 0}
+        with self.mockRequests(content=json.dumps(content)):
+            self.assertRaisesWithContent(
+                GitRepositoryScanFault,
+                "Failed to get file from Git repository: Unexpected size"
+                " (256 vs 0)",
+                self.client.getBlob, "123", "dir/path/file/name")
 
=== 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 18:43:05 +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):