diff --git a/swh/web/ui/converters.py b/swh/web/ui/converters.py --- a/swh/web/ui/converters.py +++ b/swh/web/ui/converters.py @@ -158,17 +158,29 @@ Revision dictionary with the same keys as inputs, only: - sha1s are in hexadecimal strings (id, directory) - bytes are decoded in string (author_name, committer_name, - author_email, committer_email, message) + author_email, committer_email) - remaining keys are left as is """ - return from_swh(revision, - hashess=set(['id', 'directory', 'parents', 'children']), - bytess=set(['name', - 'fullname', - 'email', - 'message']), - dates={'date', 'committer_date'}) + revision = from_swh(revision, + hashess=set(['id', + 'directory', + 'parents', + 'children']), + bytess=set(['name', + 'fullname', + 'email']), + dates={'date', 'committer_date'}) + + if revision: + if 'message' in revision: + try: + revision['message'] = revision['message'].decode('utf-8') + except UnicodeDecodeError: + revision['message_decoding_failed'] = True + revision['message'] = None + + return revision def from_content(content): diff --git a/swh/web/ui/service.py b/swh/web/ui/service.py --- a/swh/web/ui/service.py +++ b/swh/web/ui/service.py @@ -215,9 +215,8 @@ ['sha1'], 'Only sha1_git is supported.') - res = backend.revision_get(sha1_git_bin) - res.pop('message', None) - return converters.from_revision(res) + revision = backend.revision_get(sha1_git_bin) + return converters.from_revision(revision) def lookup_revision_message(rev_sha1_git): @@ -247,7 +246,7 @@ raise NotFoundExc('No message for revision with sha1_git %s.' % rev_sha1_git) res = {'message': revision['message']} - return converters.from_revision(res) + return res def lookup_revision_by(origin_id, diff --git a/swh/web/ui/templates/revision-log.html b/swh/web/ui/templates/revision-log.html --- a/swh/web/ui/templates/revision-log.html +++ b/swh/web/ui/templates/revision-log.html @@ -73,8 +73,18 @@
Message
{{ revision['message'] }}
- {% endif %} - + {% elif revision['message_encoding_failed'] %} +
+
Message
+
Download raw message
+
+
Message
+
No message found.
+ + {% endif %} + {% for key in revision.keys() %} {% if key in ['type', 'synthetic'] and revision[key] is not none %}
diff --git a/swh/web/ui/templates/revision.html b/swh/web/ui/templates/revision.html --- a/swh/web/ui/templates/revision.html +++ b/swh/web/ui/templates/revision.html @@ -56,6 +56,16 @@
Message
{{ revision['message'] }}
+ {% elif revision['message_encoding_failed'] %} +
+
Message
+
Download raw message
+
+
Message
+
No message found.
+ {% endif %} {% for key in revision.keys() %} diff --git a/swh/web/ui/tests/test_converters.py b/swh/web/ui/tests/test_converters.py --- a/swh/web/ui/tests/test_converters.py +++ b/swh/web/ui/tests/test_converters.py @@ -326,6 +326,107 @@ self.assertEqual(actual_revision, expected_revision) @istest + def from_revision_invalid(self): + revision_input = { + 'id': hashutil.hex_to_hash( + '18d8be353ed3480476f032475e7c233eff7371d5'), + 'directory': hashutil.hex_to_hash( + '7834ef7e7c357ce2af928115c6c6a42b7e2a44e6'), + 'author': { + 'name': b'Software Heritage', + 'fullname': b'robot robot@softwareheritage.org', + 'email': b'robot@softwareheritage.org', + }, + 'committer': { + 'name': b'Software Heritage', + 'fullname': b'robot robot@softwareheritage.org', + 'email': b'robot@softwareheritage.org', + }, + 'message': b'invalid message \xff', + 'date': { + 'timestamp': datetime.datetime( + 2000, 1, 17, 11, 23, 54, + tzinfo=datetime.timezone.utc).timestamp(), + 'offset': 0, + 'negative_utc': False, + }, + 'committer_date': { + 'timestamp': datetime.datetime( + 2000, 1, 17, 11, 23, 54, + tzinfo=datetime.timezone.utc).timestamp(), + 'offset': 0, + 'negative_utc': False, + }, + 'synthetic': True, + 'type': 'tar', + 'parents': [ + hashutil.hex_to_hash( + '29d8be353ed3480476f032475e7c244eff7371d5'), + hashutil.hex_to_hash( + '30d8be353ed3480476f032475e7c244eff7371d5') + ], + 'children': [ + hashutil.hex_to_hash( + '123546353ed3480476f032475e7c244eff7371d5'), + ], + 'metadata': { + 'original_artifact': [{ + 'archive_type': 'tar', + 'name': 'webbase-5.7.0.tar.gz', + 'sha1': '147f73f369733d088b7a6fa9c4e0273dcd3c7ccd', + 'sha1_git': '6a15ea8b881069adedf11feceec35588f2cfe8f1', + 'sha256': '401d0df797110bea805d358b85bcc1ced29549d3d73f' + '309d36484e7edf7bb912', + + }] + }, + } + + expected_revision = { + 'id': '18d8be353ed3480476f032475e7c233eff7371d5', + 'directory': '7834ef7e7c357ce2af928115c6c6a42b7e2a44e6', + 'author': { + 'name': 'Software Heritage', + 'fullname': 'robot robot@softwareheritage.org', + 'email': 'robot@softwareheritage.org', + }, + 'committer': { + 'name': 'Software Heritage', + 'fullname': 'robot robot@softwareheritage.org', + 'email': 'robot@softwareheritage.org', + }, + 'message': None, + 'message_decoding_failed': True, + 'date': "2000-01-17T11:23:54+00:00", + 'committer_date': "2000-01-17T11:23:54+00:00", + 'children': [ + '123546353ed3480476f032475e7c244eff7371d5' + ], + 'parents': [ + '29d8be353ed3480476f032475e7c244eff7371d5', + '30d8be353ed3480476f032475e7c244eff7371d5' + ], + 'type': 'tar', + 'synthetic': True, + 'metadata': { + 'original_artifact': [{ + 'archive_type': 'tar', + 'name': 'webbase-5.7.0.tar.gz', + 'sha1': '147f73f369733d088b7a6fa9c4e0273dcd3c7ccd', + 'sha1_git': '6a15ea8b881069adedf11feceec35588f2cfe8f1', + 'sha256': '401d0df797110bea805d358b85bcc1ced29549d3d73f' + '309d36484e7edf7bb912' + }] + }, + } + + # when + actual_revision = converters.from_revision(revision_input) + + # then + self.assertEqual(actual_revision, expected_revision) + + @istest def from_content(self): content_input = { 'sha1': hashutil.hex_to_hash('5c6f0e2750f48fa0bd0c4cf5976ba0b9e0' diff --git a/swh/web/ui/tests/test_service.py b/swh/web/ui/tests/test_service.py --- a/swh/web/ui/tests/test_service.py +++ b/swh/web/ui/tests/test_service.py @@ -929,6 +929,76 @@ 'name': 'boule & bill', 'email': 'boule@bill.org', }, + 'message': 'elegant fix for bug 31415957', + 'date': "2000-01-17T11:23:54+00:00", + 'committer_date': "2000-01-17T11:23:54+00:00", + 'synthetic': False, + 'type': 'git', + 'parents': [], + 'metadata': [], + }) + + mock_backend.revision_get.assert_called_with( + hex_to_hash('18d8be353ed3480476f032475e7c233eff7371d5')) + + @patch('swh.web.ui.service.backend') + @istest + def lookup_revision_invalid_msg(self, mock_backend): + # given + stub_rev = { + 'id': hex_to_hash('123456'), + 'directory': hex_to_hash( + '7834ef7e7c357ce2af928115c6c6a42b7e2a44e6'), + 'author': { + 'name': b'bill & boule', + 'email': b'bill@boule.org', + }, + 'committer': { + 'name': b'boule & bill', + 'email': b'boule@bill.org', + }, + 'message': b'elegant fix for bug \xff', + 'date': { + 'timestamp': datetime.datetime( + 2000, 1, 17, 11, 23, 54, + tzinfo=datetime.timezone.utc, + ).timestamp(), + 'offset': 0, + 'negative_utc': False, + }, + 'committer_date': { + 'timestamp': datetime.datetime( + 2000, 1, 17, 11, 23, 54, + tzinfo=datetime.timezone.utc, + ).timestamp(), + 'offset': 0, + 'negative_utc': False, + }, + 'synthetic': False, + 'type': 'git', + 'parents': [], + 'metadata': [], + } + mock_backend.revision_get = MagicMock(return_value=stub_rev) + + # when + actual_revision = service.lookup_revision( + '18d8be353ed3480476f032475e7c233eff7371d5') + + # then + self.assertEqual(actual_revision, { + 'id': '123456', + 'directory': '7834ef7e7c357ce2af928115c6c6a42b7e2a44e6', + 'author': { + 'name': 'bill & boule', + 'email': 'bill@boule.org', + }, + 'committer': { + 'name': 'boule & bill', + 'email': 'boule@bill.org', + }, + 'message': None, + 'message_decoding_failed': True, 'date': "2000-01-17T11:23:54+00:00", 'committer_date': "2000-01-17T11:23:54+00:00", 'synthetic': False, @@ -984,7 +1054,7 @@ '18d8be353ed3480476f032475e7c233eff7371d5') # then - self.assertEquals(rv, {'message': 'elegant fix for bug 31415957'}) + self.assertEquals(rv, {'message': b'elegant fix for bug 31415957'}) mock_backend.revision_get.assert_called_with( hex_to_hash('18d8be353ed3480476f032475e7c233eff7371d5')) diff --git a/swh/web/ui/tests/test_utils.py b/swh/web/ui/tests/test_utils.py --- a/swh/web/ui/tests/test_utils.py +++ b/swh/web/ui/tests/test_utils.py @@ -570,3 +570,104 @@ call('api_revision_history', sha1_git_root='sha1_git_root', sha1_git='456')]) + + @patch('swh.web.ui.utils.flask') + @istest + def enrich_revision_with_no_message(self, mock_flask): + # given + def url_for_test(fn, **data): + print(fn, data) + if fn == 'api_revision': + return '/api/revision/' + data['sha1_git'] + '/' + elif fn == 'api_revision_log': + return '/api/revision/' + data['sha1_git'] + '/log/' + elif fn == 'api_revision_raw_message': + return '/api/revision/' + data['sha1_git'] + '/raw/' + else: + return '/api/revision/' + data['sha1_git_root'] + '/history/' + data['sha1_git'] + '/' # noqa + + mock_flask.url_for.side_effect = url_for_test + + # when + actual_revision = utils.enrich_revision({ + 'id': 'rev-id', + 'message': None, + 'parents': ['123'], + 'children': ['456'], + }, context='sha1_git_root') + + # then + self.assertEqual(actual_revision, { + 'id': 'rev-id', + 'url': '/api/revision/rev-id/', + 'message': None, + 'history_url': '/api/revision/rev-id/log/', + 'parents': ['123'], + 'parent_urls': ['/api/revision/sha1_git_root/history/123/'], + 'children': ['456'], + 'children_urls': ['/api/revision/sha1_git_root/history/456/'], + }) + + mock_flask.url_for.assert_has_calls( + [call('api_revision', + sha1_git='rev-id'), + call('api_revision_log', + sha1_git='rev-id'), + call('api_revision_history', + sha1_git_root='sha1_git_root', + sha1_git='123'), + call('api_revision_history', + sha1_git_root='sha1_git_root', + sha1_git='456')]) + + @patch('swh.web.ui.utils.flask') + @istest + def enrich_revision_with_invalid_message(self, mock_flask): + # given + def url_for_test(fn, **data): + print(fn, data) + if fn == 'api_revision': + return '/api/revision/' + data['sha1_git'] + '/' + elif fn == 'api_revision_log': + return '/api/revision/' + data['sha1_git'] + '/log/' + elif fn == 'api_revision_raw_message': + return '/api/revision/' + data['sha1_git'] + '/raw/' + else: + return '/api/revision/' + data['sha1_git_root'] + '/history/' + data['sha1_git'] + '/' # noqa + + mock_flask.url_for.side_effect = url_for_test + + # when + actual_revision = utils.enrich_revision({ + 'id': 'rev-id', + 'message': None, + 'message_decoding_failed': True, + 'parents': ['123'], + 'children': ['456'], + }, context='sha1_git_root') + + # then + self.assertEqual(actual_revision, { + 'id': 'rev-id', + 'url': '/api/revision/rev-id/', + 'message': None, + 'message_decoding_failed': True, + 'message_url': '/api/revision/rev-id/raw/', + 'history_url': '/api/revision/rev-id/log/', + 'parents': ['123'], + 'parent_urls': ['/api/revision/sha1_git_root/history/123/'], + 'children': ['456'], + 'children_urls': ['/api/revision/sha1_git_root/history/456/'], + }) + + mock_flask.url_for.assert_has_calls( + [call('api_revision', + sha1_git='rev-id'), + call('api_revision_log', + sha1_git='rev-id'), + call('api_revision_history', + sha1_git_root='sha1_git_root', + sha1_git='123'), + call('api_revision_history', + sha1_git_root='sha1_git_root', + sha1_git='456')]) diff --git a/swh/web/ui/tests/views/test_api.py b/swh/web/ui/tests/views/test_api.py --- a/swh/web/ui/tests/views/test_api.py +++ b/swh/web/ui/tests/views/test_api.py @@ -723,16 +723,12 @@ mock_service.lookup_revision_message.return_value = stub_revision # when - rv = self.app.get('/api/1/revision/' - '18d8be353ed3480476f032475e7c233eff7371d5/raw/') - + rv = self.app.get('/api/1/revision/18d8be353ed3480476f032475e7c2' + '33eff7371d5/raw/') # then self.assertEquals(rv.status_code, 200) - self.assertEquals(rv.mimetype, 'application/json') - - response_data = json.loads(rv.data.decode('utf-8')) - self.assertEquals(response_data, - {'message': 'synthetic revision message'}) + self.assertEquals(rv.mimetype, 'application/octet-stream') + self.assertEquals(rv.data, b'synthetic revision message') mock_service.lookup_revision_message.assert_called_once_with( '18d8be353ed3480476f032475e7c233eff7371d5') diff --git a/swh/web/ui/tests/views/test_browse.py b/swh/web/ui/tests/views/test_browse.py --- a/swh/web/ui/tests/views/test_browse.py +++ b/swh/web/ui/tests/views/test_browse.py @@ -839,6 +839,19 @@ @patch('swh.web.ui.views.browse.api') @istest + def browse_revision_raw_message(self, mock_api): + # given + sha1 = 'd770e558e21961ad6cfdf0ff7df0eb5d7d4f0754' + + # when + rv = self.client.get('/browse/revision/' + 'd770e558e21961ad6cfdf0ff7df0eb5d7d4f0754/raw/') + + self.assertRedirects( + rv, '/api/1/revision/%s/raw/' % sha1) + + @patch('swh.web.ui.views.browse.api') + @istest def browse_revision_log_ko_not_found(self, mock_api): # given mock_api.api_revision_log.side_effect = NotFoundExc('Not found!') diff --git a/swh/web/ui/utils.py b/swh/web/ui/utils.py --- a/swh/web/ui/utils.py +++ b/swh/web/ui/utils.py @@ -242,4 +242,11 @@ revision['children_urls'] = children + if 'message' in revision: + if 'message_decoding_failed' in revision: + if revision['message_decoding_failed']: + revision['message_url'] = flask.url_for( + 'api_revision_raw_message', + sha1_git=revision['id']) + return revision diff --git a/swh/web/ui/views/api.py b/swh/web/ui/views/api.py --- a/swh/web/ui/views/api.py +++ b/swh/web/ui/views/api.py @@ -512,8 +512,11 @@ GET /api/1/revision/baf18f9fc50a0b6fef50460a76c33b2ddc57486e/raw/ """ - - return service.lookup_revision_message(sha1_git) + raw = service.lookup_revision_message(sha1_git) + return Response(raw['message'], + headers={'Content-disposition': 'attachment;' + 'filename=rev_%s_raw' % sha1_git}, + mimetype='application/octet-stream') @app.route('/api/1/revision//directory/') diff --git a/swh/web/ui/views/browse.py b/swh/web/ui/views/browse.py --- a/swh/web/ui/views/browse.py +++ b/swh/web/ui/views/browse.py @@ -332,6 +332,14 @@ return render_template('revision.html', **env) +@app.route('/browse/revision//raw/') +def browse_revision_raw_message(sha1_git): + """Given a sha1_git, display the corresponding revision's raw message. + + """ + return redirect(url_for('api_revision_raw_message', sha1_git=sha1_git)) + + @app.route('/browse/revision//log/') @set_renderers(HTMLRenderer) def browse_revision_log(sha1_git):