From 4c06f0f1e69074f21ed8df3d1e1008f9ccd8fcce Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Jul 24 2018 12:10:18 +0000 Subject: Add tests checking the behavior when uploading two files and fix two bugs The two bugs were: - Ensure we run update the checksums only once, even if we upload two files, otherwise, the first file uploaded ends up twice in the checksums file. - Ensure we do not write twice the header line when uploading two files to a project that did not have a checksums file before. Signed-off-by: Pierre-Yves Chibon --- diff --git a/pagure/lib/tasks.py b/pagure/lib/tasks.py index e152711..c3cea08 100644 --- a/pagure/lib/tasks.py +++ b/pagure/lib/tasks.py @@ -814,6 +814,7 @@ def update_checksums_file(self, session, folder, filenames): with open(sha_file, 'a') as stream: if new_file: stream.write('# Generated and updated by pagure\n') + new_file = False for algo in sorted(algos): stream.write('%s (%s) = %s\n' % ( algo.upper(), filename, algos[algo].hexdigest())) diff --git a/pagure/ui/repo.py b/pagure/ui/repo.py index a0d8f00..200a7e5 100644 --- a/pagure/ui/repo.py +++ b/pagure/ui/repo.py @@ -1070,6 +1070,7 @@ def new_release(repo, username=None, namespace=None): if form.validate_on_submit(): filenames = [] + error = False for filestream in flask.request.files.getlist('filestream'): filename = werkzeug.secure_filename(filestream.filename) filenames.append(filename) @@ -1086,18 +1087,21 @@ def new_release(repo, username=None, namespace=None): filestream.save(dest) flask.flash('File "%s" uploaded' % filename) - - task = pagure.lib.tasks.update_checksums_file.delay( - folder=folder, filenames=filenames) - _log.info( - 'Updating checksums for %s of project %s in task: %s' % ( - filenames, repo.fullname, task.id)) except pagure.exceptions.PagureException as err: _log.debug(err) flask.flash(str(err), 'error') + error = True except Exception as err: # pragma: no cover _log.exception(err) flask.flash('Upload failed', 'error') + error = True + + if not error: + task = pagure.lib.tasks.update_checksums_file.delay( + folder=folder, filenames=filenames) + _log.info( + 'Updating checksums for %s of project %s in task: %s' % ( + filenames, repo.fullname, task.id)) return flask.redirect(flask.url_for( 'ui_ns.view_tags', repo=repo.name, username=username, diff --git a/tests/test_pagure_flask_ui_repo.py b/tests/test_pagure_flask_ui_repo.py index a22d916..2a23947 100644 --- a/tests/test_pagure_flask_ui_repo.py +++ b/tests/test_pagure_flask_ui_repo.py @@ -4744,6 +4744,81 @@ index 0000000..fb7093d 'been uploaded', output_text) self.assertIn('This project has not been tagged.', output_text) + def test_new_release_two_files(self): + """ Test the new_release endpoint when uploading two files. """ + tests.create_projects(self.session) + repo = tests.create_projects_git(os.path.join(self.path, 'repos')) + + user = tests.FakeUser(username='pingou') + with tests.user_set(self.app.application, user): + img = os.path.join( + os.path.abspath(os.path.dirname(__file__)), 'placebo.png') + img2 = os.path.join( + os.path.abspath(os.path.dirname(__file__)), 'pagure.png') + + csrf_token = self.get_csrf() + + upload_dir = os.path.join(self.path, 'releases') + self.assertEqual(os.listdir(upload_dir), []) + + # Upload successful + with open(img, mode='rb') as stream: + with open(img2, mode='rb') as stream2: + data = { + 'filestream': [stream, stream2], + 'csrf_token': csrf_token + } + output = self.app.post( + '/test/upload/', data=data, follow_redirects=True) + self.assertEqual(output.status_code, 200) + output_text = output.get_data(as_text=True) + self.assertIn( + ' File', output_text) + self.assertIn('pagure.png" uploaded\n', output_text) + # self.assertTrue(0) + + self.assertEqual(os.listdir(upload_dir), ['test']) + folder = os.path.join(upload_dir, 'test') + checksum_file = os.path.join(folder, 'CHECKSUMS') + + # Wait for the worker to create the checksums file + cnt = 0 + while not os.path.exists(checksum_file): + cnt += 1 + if cnt == 40: + raise ValueError( + 'The worker did not create the checksums file ' + 'in a timely manner') + time.sleep(0.5) + + self.assertEqual(len(os.listdir(folder)), 3) + + self.assertTrue(os.path.exists(checksum_file)) + + # Check the content of the checksums file + with open(checksum_file) as stream: + data = stream.readlines() + self.assertEqual(len(data), 5) + self.assertEqual(data[0], '# Generated and updated by pagure\n') + self.assertTrue(data[1].startswith('SHA256 (')) + self.assertTrue(data[1].endswith( + 'tests_placebo.png) = 8a06845923010b27bfd8e7e75acff' + '7badc40d1021b4994e01f5e11ca40bc3abe\n')) + self.assertTrue(data[2].startswith('SHA512 (')) + self.assertTrue(data[2].endswith( + 'tests_placebo.png) = 65a4458df0acb29dc3c5ad4a3620e' + '98841d1fcf3f8df358f5348fdeddd1a86706491ac6e416768e' + '9f218aae8147d6ac524a59d3eb91fb925fdcb5c489e55ccbb\n')) + self.assertTrue(data[3].startswith('SHA256 (')) + self.assertTrue(data[3].endswith( + 'tests_pagure.png) = 6498a2de405546200b6144da56fc25' + 'd0a3976ae688dbfccaca609c8b4480523e\n')) + self.assertTrue(data[4].startswith('SHA512 (')) + self.assertTrue(data[4].endswith( + 'tests_pagure.png) = 15458775e5d73cd74de7da7224597f6' + '7f8b23d62d3affb8abba4f5db74d33235642a0f744de2265cca7' + 'd2b5866782c45e1fdeb32dd2822ae33e97995d4879afd\n')) + @patch('pagure.decorators.admin_session_timedout') def test_add_token_all_tokens(self, ast): """ Test the add_token endpoint. """