diff --git a/reviewboard/hostingsvcs/beanstalk.py b/reviewboard/hostingsvcs/beanstalk.py index b4617c5dec62527f008a3f73ee74d92dc2f4f2ee..a5f81dbf3b8891a686716b960c8a077cc30221a4 100644 --- a/reviewboard/hostingsvcs/beanstalk.py +++ b/reviewboard/hostingsvcs/beanstalk.py @@ -1,3 +1,4 @@ +import os from urllib import quote from urllib2 import HTTPError, URLError @@ -89,13 +90,9 @@ class Beanstalk(HostingService): If using Git, this will expect a base commit ID to be provided. """ - revision = self._normalize_revision(repository, path, revision, - base_commit_id) - try: - node_data = self._api_get_node(repository, path, revision, - contents=True) - return node_data['contents'] + return self._api_get_node(repository, path, revision, + base_commit_id, contents=True) except (HTTPError, URLError): raise FileNotFoundError(path, revision) @@ -108,47 +105,40 @@ class Beanstalk(HostingService): If using Git, this will expect a base commit ID to be provided. """ try: - revision = self._normalize_revision(repository, path, revision, - base_commit_id) - - self._api_get_node(repository, path, revision) + self._api_get_node(repository, path, revision, base_commit_id) return True except (HTTPError, URLError, FileNotFoundError): return False - def _normalize_revision(self, repository, path, revision, base_commit_id): - if base_commit_id: - revision = base_commit_id - elif repository.tool.name == 'Git': - raise FileNotFoundError( - path, - revision, - detail='The necessary revision information needed to find ' - 'this file was not provided. Use RBTools 0.5.2 or ' - 'newer.') - - return revision - def _api_get_repository(self, account_domain, repository_name): url = self._build_api_url(account_domain, 'repositories/%s.json' % repository_name) return self._api_get(url) - def _api_get_node(self, repository, path, revision, contents=False): - if contents: - contents_str = '1' + def _api_get_node(self, repository, path, revision, base_commit_id, + contents=False): + # Unless we're fetching raw content, we optimistically want to + # grab the metadata for the file. That's going to be a lot smaller + # than the file contents in most cases. However, we can only do that + # with a base_commit_id. If we don't have that, we fall back on + # fetching the full file contents. + raw_content = (contents or not base_commit_id) + + if raw_content: + url_path = ('blob?id=%s&name=%s' + % (quote(revision), quote(os.path.basename(path)))) else: - contents_str = '0' + url_path = ('node.json?path=%s&revision=%s&contents=0' + % (quote(path), quote(base_commit_id))) url = self._build_api_url( self._get_repository_account_domain(repository), - 'repositories/%s/node.json?path=%s&revision=%s&contents=%s' - % (repository.extra_data['beanstalk_repo_name'], - quote(path), quote(revision), contents_str)) + 'repositories/%s/%s' + % (repository.extra_data['beanstalk_repo_name'], url_path)) - return self._api_get(url) + return self._api_get(url, raw_content=True) def _build_api_url(self, account_domain, url): return 'https://%s.beanstalkapp.com/api/%s' % (account_domain, url) @@ -156,13 +146,17 @@ class Beanstalk(HostingService): def _get_repository_account_domain(self, repository): return repository.extra_data['beanstalk_account_domain'] - def _api_get(self, url): + def _api_get(self, url, raw_content=False): try: data, headers = self._http_get( url, username=self.account.username, password=decrypt_password(self.account.data['password'])) - return simplejson.loads(data) + + if raw_content: + return data + else: + return simplejson.loads(data) except HTTPError, e: data = e.read() diff --git a/reviewboard/hostingsvcs/tests.py b/reviewboard/hostingsvcs/tests.py index f8b503072232e62c3f0915d3fb7be41b2bf04d58..66b12bf869b6ba96784d5b82735501f3c1a9bd17 100644 --- a/reviewboard/hostingsvcs/tests.py +++ b/reviewboard/hostingsvcs/tests.py @@ -155,7 +155,7 @@ class BeanstalkTests(ServiceTests): tool_name='Subversion', revision='123', base_commit_id='456', - expected_revision='456') + expected_revision='123') def test_get_file_with_svn_and_revision(self): """Testing Beanstalk get_file with Subversion and revision""" @@ -171,13 +171,11 @@ class BeanstalkTests(ServiceTests): tool_name='Git', revision='123', base_commit_id='456', - expected_revision='456') + expected_revision='123') def test_get_file_with_git_and_revision(self): - """Testing Beanstalk get_file with Git and revision raises error""" - self.assertRaises( - FileNotFoundError, - self._test_get_file, + """Testing Beanstalk get_file with Git and revision""" + self._test_get_file( tool_name='Git', revision='123', base_commit_id=None, @@ -211,14 +209,13 @@ class BeanstalkTests(ServiceTests): expected_found=True) def test_get_file_exists_with_git_and_revision(self): - """Testing Beanstalk get_file_exists with Git and revision raises error""" + """Testing Beanstalk get_file_exists with Git and revision""" self._test_get_file_exists( tool_name='Git', revision='123', base_commit_id=None, - expected_revision='456', - expected_found=False, - expected_http_called=False) + expected_revision='123', + expected_found=True) def _test_get_file(self, tool_name, revision, base_commit_id, expected_revision): @@ -226,9 +223,9 @@ class BeanstalkTests(ServiceTests): self.assertEqual( url, 'https://mydomain.beanstalkapp.com/api/repositories/' - 'myrepo/node.json?path=/path&revision=%s&contents=1' + 'myrepo/blob?id=%s&name=path' % expected_revision) - return '{"contents": "My data"}', {} + return 'My data', {} account = self._get_hosting_account() service = account.service @@ -252,11 +249,16 @@ class BeanstalkTests(ServiceTests): expected_revision, expected_found, expected_http_called=True): def _http_get(service, url, *args, **kwargs): - self.assertEqual( - url, - 'https://mydomain.beanstalkapp.com/api/repositories/' - 'myrepo/node.json?path=/path&revision=%s&contents=0' - % (expected_revision)) + expected_url = ('https://mydomain.beanstalkapp.com/api/' + 'repositories/myrepo/') + + if base_commit_id: + expected_url += ('node.json?path=/path&revision=%s&contents=0' + % expected_revision) + else: + expected_url += 'blob?id=%s&name=path' % expected_revision + + self.assertEqual(url, expected_url) if expected_found: return '{}', {} @@ -278,7 +280,7 @@ class BeanstalkTests(ServiceTests): result = service.get_file_exists(repository, '/path', revision, base_commit_id) - self.assertEqual(service._http_get.called, expected_http_called) + self.assertTrue(service._http_get.called) self.assertEqual(result, expected_found) @@ -452,8 +454,7 @@ class BitbucketTests(ServiceTests): self.assertEqual(result, 'My data') def _test_get_file_exists(self, tool_name, revision, base_commit_id, - expected_revision, expected_found, - expected_http_called=True): + expected_revision, expected_found): def _http_get(service, url, *args, **kwargs): self.assertEqual( url,