diff --git a/.gitignore b/.gitignore index cceb77e39312436f45b5cf84e92d57ceba164413..21c4bfbf68742f96058f4bf9daaa843a50c790ac 100644 --- a/.gitignore +++ b/.gitignore @@ -12,3 +12,4 @@ version.txt /test.json /swh/test db.sqlite3 +/.noseids diff --git a/Makefile.local b/Makefile.local index 0a9c1368265b83f74c48d6c67fc763a2d199d4b6..a0cda2c368f7efb2c4875625b0d9fb904d57b144 100644 --- a/Makefile.local +++ b/Makefile.local @@ -27,4 +27,4 @@ run: gunicorn3 -b 127.0.0.1:5006 swh.deposit.wsgi test: - cd swh && python3 -m manage test + ./swh/manage.py test diff --git a/PKG-INFO b/PKG-INFO index b9fd7e41d64a7f7402b8a36b9865a61a35cbe405..428e3e4b6112cf4af6313035209014f0edd20489 100644 --- a/PKG-INFO +++ b/PKG-INFO @@ -1,6 +1,6 @@ Metadata-Version: 2.1 Name: swh.deposit -Version: 0.0.20 +Version: 0.0.21 Summary: Software Heritage Deposit Server Home-page: https://forge.softwareheritage.org/source/swh-deposit/ Author: Software Heritage developers diff --git a/README-injection.md b/README-injection.md index 84b40d08d0ff36479067b396c7cc4077db736af1..bbc39cf5f99d351aeea72ffd6158567d5c1f32ea 100644 --- a/README-injection.md +++ b/README-injection.md @@ -11,9 +11,11 @@ The injection of the deposit will use the deposit's associated data: - the metadata - the archive(s) -We will use the `synthetic` revision notion. To that revision will be -associated the metadata, thus those will be included in the hash -computation. +We will use the `synthetic` revision notion. + +To that revision will be associated the metadata. Those will be +included in the hash computation, thus resulting in a unique +identifier. ### Injection mapping diff --git a/debian/control b/debian/control index 856c25d808f975ef54fc0a716e7ac9675ff3fbea..e1080f704d94c25325c2eae4bcfa2b72cbe6ab43 100644 --- a/debian/control +++ b/debian/control @@ -7,6 +7,7 @@ Build-Depends: debhelper (>= 9), python3-setuptools, python3-all, python3-nose, + python3-django-nose, python3-vcversioner, python3-swh.core (>= 0.0.14~), python3-swh.loader.core (>= 0.0.19~), diff --git a/swh.deposit.egg-info/PKG-INFO b/swh.deposit.egg-info/PKG-INFO index b9fd7e41d64a7f7402b8a36b9865a61a35cbe405..428e3e4b6112cf4af6313035209014f0edd20489 100644 --- a/swh.deposit.egg-info/PKG-INFO +++ b/swh.deposit.egg-info/PKG-INFO @@ -1,6 +1,6 @@ Metadata-Version: 2.1 Name: swh.deposit -Version: 0.0.20 +Version: 0.0.21 Summary: Software Heritage Deposit Server Home-page: https://forge.softwareheritage.org/source/swh-deposit/ Author: Software Heritage developers diff --git a/swh.deposit.egg-info/SOURCES.txt b/swh.deposit.egg-info/SOURCES.txt index 7d2a79ac362cefcfd37d39169ea690211d302a58..dcb34ecd79c80fd613640fe7e5a4df66a4932a26 100644 --- a/swh.deposit.egg-info/SOURCES.txt +++ b/swh.deposit.egg-info/SOURCES.txt @@ -78,6 +78,7 @@ swh/deposit/injection/tasks.py swh/deposit/migrations/0001_initial.py swh/deposit/migrations/0002_depositrequest_archive.py swh/deposit/migrations/0003_temporaryarchive.py +swh/deposit/migrations/0004_delete_temporaryarchive.py swh/deposit/migrations/__init__.py swh/deposit/service/__init__.py swh/deposit/service/clean_temporary_directory.py @@ -103,7 +104,8 @@ swh/deposit/tests/api/test_deposit_atom.py swh/deposit/tests/api/test_deposit_binary.py swh/deposit/tests/api/test_deposit_delete.py swh/deposit/tests/api/test_deposit_multipart.py -swh/deposit/tests/api/test_deposit_read.py +swh/deposit/tests/api/test_deposit_read_archive.py +swh/deposit/tests/api/test_deposit_read_metadata.py swh/deposit/tests/api/test_deposit_status.py swh/deposit/tests/api/test_deposit_update.py swh/deposit/tests/api/test_deposit_update_status.py diff --git a/swh/deposit/api/private/deposit_read.py b/swh/deposit/api/private/deposit_read.py index 348abb1d3dbdab4d2c4d41c4ab4de607ac6414d7..a6703fa1532411ab388d0735061902742dbee3cf 100644 --- a/swh/deposit/api/private/deposit_read.py +++ b/swh/deposit/api/private/deposit_read.py @@ -8,16 +8,18 @@ import os import shutil import tempfile +from contextlib import contextmanager from rest_framework import status from swh.loader.tar import tarball from swh.model import hashutil, identifiers from ..common import SWHGetDepositAPI, SWHPrivateAPIView -from ...models import Deposit, DepositRequest, TemporaryArchive +from ...models import Deposit, DepositRequest from ...models import previous_revision_id +@contextmanager def aggregate_tarballs(extraction_dir, archive_paths): """Aggregate multiple tarballs into one and returns this new archive's path. @@ -33,7 +35,7 @@ def aggregate_tarballs(extraction_dir, archive_paths): if len(archive_paths) > 1: # need to rebuild one archive # from multiple ones os.makedirs(extraction_dir, 0o755, exist_ok=True) - dir_path = tempfile.mkdtemp(prefix='swh.deposit.scheduler-', + dir_path = tempfile.mkdtemp(prefix='swh.deposit-', dir=extraction_dir) # root folder to build an aggregated tarball aggregated_tarball_rootdir = os.path.join(dir_path, 'aggregate') @@ -49,32 +51,16 @@ def aggregate_tarballs(extraction_dir, archive_paths): nature='zip', dirpath_or_files=aggregated_tarball_rootdir) - # clean up temporary uncompressed tarball's on-disk content + # can already clean up temporary directory shutil.rmtree(aggregated_tarball_rootdir) - # need to cleanup the temporary tarball when we are done - directory_to_cleanup = dir_path - else: # only 1 archive, no need to do fancy actions (and no cleanup step) - temp_tarpath = archive_paths[0] - directory_to_cleanup = None - - return directory_to_cleanup, temp_tarpath - - -def stream_content(tarpath): - """Stream a tarpath's content. - Args: - tarpath (path): Path to a tarball - - Raises: - ValueError if the tarpath targets something nonexistent - - """ - if not os.path.exists(tarpath): - raise ValueError('Development error: %s should exist' % tarpath) + try: + yield from open(temp_tarpath, 'rb') + finally: + shutil.rmtree(dir_path) - with open(tarpath, 'rb') as f: - yield from f + else: # only 1 archive, no need to do fancy actions (and no cleanup step) + yield from open(archive_paths[0], 'rb') class SWHDepositReadArchives(SWHGetDepositAPI, SWHPrivateAPIView): @@ -108,23 +94,6 @@ class SWHDepositReadArchives(SWHGetDepositAPI, SWHPrivateAPIView): for deposit_request in deposit_requests: yield deposit_request.archive.path - def cleanup(self, directory_to_cleanup): - """Reference the temporary directory holding the archive to be cleaned - up. This actually does not clean up but add a reference for - a directory to be cleaned up if it exists. - - Args: - directory_to_cleanup (str/None): A reference to a - directory to be cleaned up - - """ - if directory_to_cleanup: - # Add a temporary directory to be cleaned up in the db model - # Another service is in charge of actually cleaning up - if os.path.exists(directory_to_cleanup): - tmp_archive = TemporaryArchive(path=directory_to_cleanup) - tmp_archive.save() - def process_get(self, req, collection_name, deposit_id): """Build a unique tarball from the multiple received and stream that content to the client. @@ -139,12 +108,9 @@ class SWHDepositReadArchives(SWHGetDepositAPI, SWHPrivateAPIView): """ archive_paths = list(self.retrieve_archives(deposit_id)) - directory_to_cleanup, temp_tarpath = aggregate_tarballs( - self.extraction_dir, archive_paths) - stream = stream_content(temp_tarpath) - self.cleanup(directory_to_cleanup) - - return status.HTTP_200_OK, stream, 'application/octet-stream' + with aggregate_tarballs(self.extraction_dir, + archive_paths) as stream: + return status.HTTP_200_OK, stream, 'application/octet-stream' class SWHDepositReadMetadata(SWHGetDepositAPI, SWHPrivateAPIView): diff --git a/swh/deposit/migrations/0004_delete_temporaryarchive.py b/swh/deposit/migrations/0004_delete_temporaryarchive.py new file mode 100644 index 0000000000000000000000000000000000000000..6fa9d84bc7f3f413f96320352cc1aac31015d4bf --- /dev/null +++ b/swh/deposit/migrations/0004_delete_temporaryarchive.py @@ -0,0 +1,18 @@ +# -*- coding: utf-8 -*- +# Generated by Django 1.10.7 on 2017-10-18 09:03 +from __future__ import unicode_literals + +from django.db import migrations + + +class Migration(migrations.Migration): + + dependencies = [ + ('deposit', '0003_temporaryarchive'), + ] + + operations = [ + migrations.DeleteModel( + name='TemporaryArchive', + ), + ] diff --git a/swh/deposit/models.py b/swh/deposit/models.py index a454f649bd8fb273c40389920e03c9d0b2f8e193..2f9e26e543105250967f0b5f0a2d71e8c3e42cfd 100644 --- a/swh/deposit/models.py +++ b/swh/deposit/models.py @@ -220,22 +220,3 @@ class DepositCollection(models.Model): def __str__(self): return str({'id': self.id, 'name': self.name}) - - -class TemporaryArchive(models.Model): - """Temporary archive path to remove - - """ - id = models.BigAutoField(primary_key=True) - path = models.TextField() - date = models.DateTimeField(auto_now_add=True) - - class Meta: - db_table = 'deposit_temporary_archive' - - def __str__(self): - return str({ - 'id': self.id, - 'date': self.date, - 'path': self.path, - }) diff --git a/swh/deposit/settings/testing.py b/swh/deposit/settings/testing.py index 1c2a197d4bfc053e8eae7c48b56b6f3cf09aee82..9f6799c9bd8876633288ca710460bae34b822bb0 100644 --- a/swh/deposit/settings/testing.py +++ b/swh/deposit/settings/testing.py @@ -5,7 +5,13 @@ from .common import * # noqa from .development import * # noqa +from .development import INSTALLED_APPS +# django-nose setup + +INSTALLED_APPS += ['django_nose'] + +TEST_RUNNER = 'django_nose.NoseTestSuiteRunner' # https://docs.djangoproject.com/en/1.10/ref/settings/#logging LOGGING = { diff --git a/swh/deposit/tests/__init__.py b/swh/deposit/tests/__init__.py index 94d9602b37d89160374ba45a67b16d81882c3de5..6a4f3fc9ace376afdb6d9fd0089313370f01a4c8 100644 --- a/swh/deposit/tests/__init__.py +++ b/swh/deposit/tests/__init__.py @@ -9,12 +9,7 @@ from swh.deposit.config import SWHDefaultConfig # noqa TEST_CONFIG = { 'max_upload_size': 209715200, - 'authentication': { - 'activated': 'true', - 'white-list': { - 'GET': ['/'], - }, - }, + 'extraction_dir': '/tmp/swh-deposit/test/extraction-dir', } diff --git a/swh/deposit/tests/api/test_common.py b/swh/deposit/tests/api/test_common.py index 6cda47a4175b804854d9952609c56c83b5dc62a4..6e4465202455e6b51ebcfae721a37e1c254175e8 100644 --- a/swh/deposit/tests/api/test_common.py +++ b/swh/deposit/tests/api/test_common.py @@ -5,36 +5,38 @@ from django.core.urlresolvers import reverse +from nose.tools import istest from rest_framework import status from rest_framework.test import APITestCase from ..common import BasicTestCase, WithAuthTestCase -def assert_test_home_is_ok(testcase): - url = reverse('home') - response = testcase.client.get(url) - testcase.assertEqual(response.status_code, status.HTTP_200_OK) - testcase.assertEqual(response.content, b'SWH Deposit API') - - class IndexNoAuthCase(APITestCase, BasicTestCase): """Access to main entry point is ok without authentication """ - def test_get_home_is_ok(self): + @istest + def get_home_is_ok(self): """Without authentication, endpoint refuses access with 401 response """ - assert_test_home_is_ok(self) + url = reverse('home') + response = self.client.get(url) + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(response.content, b'SWH Deposit API') class IndexWithAuthCase(WithAuthTestCase, APITestCase, BasicTestCase): """Access to main entry point is ok with authentication as well """ - def test_get_home_is_ok_2(self): + @istest + def get_home_is_ok_2(self): """Without authentication, endpoint refuses access with 401 response """ - assert_test_home_is_ok(self) + url = reverse('home') + response = self.client.get(url) + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(response.content, b'SWH Deposit API') diff --git a/swh/deposit/tests/api/test_deposit.py b/swh/deposit/tests/api/test_deposit.py index d5898b85c7af8d5a9c19329461e0b41783017df9..835c4c19fdcbdbe9f477752877dacc601c5a40b9 100644 --- a/swh/deposit/tests/api/test_deposit.py +++ b/swh/deposit/tests/api/test_deposit.py @@ -4,6 +4,7 @@ # See top-level LICENSE file for more information from django.core.urlresolvers import reverse +from nose.tools import istest from rest_framework import status from rest_framework.test import APITestCase @@ -27,13 +28,18 @@ class DepositFailuresTest(APITestCase, WithAuthTestCase, BasicTestCase): _user.collections = [_collection2.id] self.collection2 = _collection2 - def test_access_to_another_user_collection_is_forbidden(self): + @istest + def access_to_another_user_collection_is_forbidden(self): + """Access to another user collection should return a 403 + + """ url = reverse(COL_IRI, args=[self.collection2.name]) response = self.client.post(url) self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) - def test_delete_on_col_iri_not_supported(self): + @istest + def delete_on_col_iri_not_supported(self): """Delete on col iri should return a 405 response """ diff --git a/swh/deposit/tests/api/test_deposit_atom.py b/swh/deposit/tests/api/test_deposit_atom.py index 2525d37c492bd70cd64367213d628393b6277d45..895c6765817ca0b81c97d807c622829a200767ab 100644 --- a/swh/deposit/tests/api/test_deposit_atom.py +++ b/swh/deposit/tests/api/test_deposit_atom.py @@ -5,6 +5,7 @@ from django.core.urlresolvers import reverse from io import BytesIO +from nose.tools import istest from rest_framework import status from rest_framework.test import APITestCase @@ -92,21 +93,33 @@ and other stuff</description> self.atom_entry_data_badly_formatted = b"""<?xml version="1.0"?> <entry xmlns="http://www.w3.org/2005/Atom"</entry>""" - def test_post_deposit_atom_empty_body_request(self): + @istest + def post_deposit_atom_empty_body_request(self): + """Posting empty body request should return a 400 response + + """ response = self.client.post( reverse(COL_IRI, args=[self.collection.name]), content_type='application/atom+xml;type=entry', data=self.atom_entry_data_empty_body) self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) - def test_post_deposit_atom_badly_formatted_is_a_bad_request(self): + @istest + def post_deposit_atom_badly_formatted_is_a_bad_request(self): + """Posting a badly formatted atom should return a 400 response + + """ response = self.client.post( reverse(COL_IRI, args=[self.collection.name]), content_type='application/atom+xml;type=entry', data=self.atom_entry_data_badly_formatted) self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) - def test_post_deposit_atom_without_slug_header_is_bad_request(self): + @istest + def post_deposit_atom_without_slug_header_is_bad_request(self): + """Posting an atom entry without a slug header should return a 400 + + """ url = reverse(COL_IRI, args=[self.collection.name]) # when @@ -121,7 +134,11 @@ and other stuff</description> self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) - def test_post_deposit_atom_unknown_collection(self): + @istest + def post_deposit_atom_unknown_collection(self): + """Posting an atom entry to an unknown collection should return a 404 + + """ response = self.client.post( reverse(COL_IRI, args=['unknown-one']), content_type='application/atom+xml;type=entry', @@ -129,8 +146,9 @@ and other stuff</description> HTTP_SLUG='something') self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) - def test_post_deposit_atom_entry_initial(self): - """One deposit upload as atom entry + @istest + def post_deposit_atom_entry_initial(self): + """Posting an initial atom entry should return 201 with deposit receipt """ # given @@ -167,8 +185,9 @@ and other stuff</description> self.assertIsNotNone(deposit_request.metadata) self.assertFalse(bool(deposit_request.archive)) + @istest def test_post_deposit_atom_entry_tei(self): - """One deposit upload as atom entry + """Posting initial atom entry as TEI should return 201 with receipt """ # given @@ -204,8 +223,11 @@ and other stuff</description> self.assertIsNotNone(deposit_request.metadata) self.assertFalse(bool(deposit_request.archive)) - def test_post_deposit_atom_entry_multiple_steps(self): - """Test one deposit upload.""" + @istest + def post_deposit_atom_entry_multiple_steps(self): + """After initial deposit, updating a deposit should return a 201 + + """ # given external_id = 'urn:uuid:2225c695-cfb8-4ebb-aaaa-80da344efa6a' diff --git a/swh/deposit/tests/api/test_deposit_binary.py b/swh/deposit/tests/api/test_deposit_binary.py index 1894c75bee6a139e55fc5efe40dc203853da3d0a..1adf4f7e573b44e5d33c9c71770425a7024adb5f 100644 --- a/swh/deposit/tests/api/test_deposit_binary.py +++ b/swh/deposit/tests/api/test_deposit_binary.py @@ -8,6 +8,8 @@ import hashlib from django.core.files.uploadedfile import InMemoryUploadedFile from django.core.urlresolvers import reverse from io import BytesIO +from nose.tools import istest + from rest_framework import status from rest_framework.test import APITestCase @@ -21,7 +23,8 @@ class DepositNoAuthCase(APITestCase, BasicTestCase): """Deposit access are protected with basic authentication. """ - def test_post_will_fail_with_401(self): + @istest + def post_will_fail_with_401(self): """Without authentication, endpoint refuses access with 401 response """ @@ -150,7 +153,11 @@ and other stuff</description> </entry>""" - def test_post_deposit_binary_without_slug_header_is_bad_request(self): + @istest + def post_deposit_binary_without_slug_header_is_bad_request(self): + """Posting a binary deposit without slug header should return 400 + + """ url = reverse(COL_IRI, args=[self.collection.name]) data_text = b'some content' md5sum = hashlib.md5(data_text).hexdigest() @@ -171,8 +178,9 @@ and other stuff</description> self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) - def test_post_deposit_binary_upload_final_and_status_check(self): - """Binary upload should be accepted + @istest + def post_deposit_binary_upload_final_and_status_check(self): + """Binary upload with correct headers should return 201 with receipt """ # given @@ -227,8 +235,9 @@ and other stuff</description> self.assertEqual(response._headers['location'], ('Location', edit_se_iri)) - def test_post_deposit_binary_upload_only_supports_zip(self): - """Binary upload only supports application/zip (for now)... + @istest + def post_deposit_binary_upload_only_supports_zip(self): + """Binary upload without content_type application/zip should return 415 """ # given @@ -258,9 +267,10 @@ and other stuff</description> with self.assertRaises(Deposit.DoesNotExist): Deposit.objects.get(external_id=external_id) - def test_post_deposit_binary_fails_if_unsupported_packaging_header( + @istest + def post_deposit_binary_fails_if_unsupported_packaging_header( self): - """Binary upload must have content_disposition header provided... + """Bin deposit without supported content_disposition header returns 400 """ # given @@ -288,9 +298,10 @@ and other stuff</description> with self.assertRaises(Deposit.DoesNotExist): Deposit.objects.get(external_id=external_id) - def test_post_deposit_binary_upload_fail_if_no_content_disposition_header( + @istest + def post_deposit_binary_upload_fail_if_no_content_disposition_header( self): - """Binary upload must have content_disposition header provided... + """Binary upload without content_disposition header should return 400 """ # given @@ -318,8 +329,9 @@ and other stuff</description> with self.assertRaises(Deposit.DoesNotExist): Deposit.objects.get(external_id=external_id) - def test_post_deposit_mediation_not_supported(self): - """Binary upload only supports application/zip (for now)... + @istest + def post_deposit_mediation_not_supported(self): + """Binary upload with mediation should return a 412 response """ # given @@ -353,7 +365,8 @@ and other stuff</description> # FIXME: Test this scenario (need a way to override the default # size limit in test scenario) - # def test_post_deposit_binary_upload_fail_if_upload_size_limit_exceeded( + # @istest + # def post_deposit_binary_upload_fail_if_upload_size_limit_exceeded( # self): # """Binary upload must not exceed the limit set up... @@ -384,8 +397,9 @@ and other stuff</description> # with self.assertRaises(Deposit.DoesNotExist): # Deposit.objects.get(external_id=external_id) - def test_post_deposit_2_post_2_different_deposits(self): - """Making 2 post requests result in 2 different deposit + @istest + def post_deposit_2_post_2_different_deposits(self): + """2 posting deposits should return 2 different 201 with receipt """ url = reverse(COL_IRI, args=[self.collection.name]) @@ -445,9 +459,9 @@ and other stuff</description> self.assertEqual(len(deposits), 2) self.assertEqual(list(deposits), [deposit, deposit2]) - def test_post_deposit_binary_and_post_to_add_another_archive(self): - """One post to post a binary deposit, One other post to update the - first deposit. + @istest + def post_deposit_binary_and_post_to_add_another_archive(self): + """Updating a deposit should return a 201 with receipt """ # given @@ -535,9 +549,9 @@ and other stuff</description> deposits = Deposit.objects.all() self.assertEqual(len(deposits), 1) - def test_post_deposit_then_post_or_put_is_refused_when_status_ready(self): - """When a deposit is complete, updating/adding new data to it is - forbidden. + @istest + def post_deposit_then_post_or_put_is_refused_when_status_ready(self): + """Updating a deposit with status 'ready' should return a 400 """ url = reverse(COL_IRI, args=[self.collection.name]) diff --git a/swh/deposit/tests/api/test_deposit_delete.py b/swh/deposit/tests/api/test_deposit_delete.py index ea33f7f2b8e8f2dd6ce744b5e6d17fb096596fcc..ea3caa90435bb58a728a2244d5a4ebe9f15a8326 100644 --- a/swh/deposit/tests/api/test_deposit_delete.py +++ b/swh/deposit/tests/api/test_deposit_delete.py @@ -4,6 +4,8 @@ # See top-level LICENSE file for more information from django.core.urlresolvers import reverse +from nose.tools import istest + from rest_framework import status from rest_framework.test import APITestCase @@ -15,7 +17,8 @@ from ..common import BasicTestCase, WithAuthTestCase, CommonCreationRoutine class DepositDeleteTest(APITestCase, WithAuthTestCase, BasicTestCase, CommonCreationRoutine): - def test_delete_archive_on_partial_deposit_works(self): + @istest + def delete_archive_on_partial_deposit_works(self): """Removing partial deposit's archive should return a 204 response """ @@ -46,7 +49,8 @@ class DepositDeleteTest(APITestCase, WithAuthTestCase, BasicTestCase, self.assertEquals(len(requests), 1) self.assertEquals(requests[0].type.name, 'metadata') - def test_delete_archive_on_undefined_deposit_fails(self): + @istest + def delete_archive_on_undefined_deposit_fails(self): """Delete undefined deposit returns a 404 response """ @@ -56,7 +60,8 @@ class DepositDeleteTest(APITestCase, WithAuthTestCase, BasicTestCase, # then self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) - def test_delete_archive_on_non_partial_deposit_fails(self): + @istest + def delete_archive_on_non_partial_deposit_fails(self): """Delete !partial status deposit should return a 400 response""" deposit_id = self.create_deposit_ready() deposit = Deposit.objects.get(pk=deposit_id) @@ -70,7 +75,8 @@ class DepositDeleteTest(APITestCase, WithAuthTestCase, BasicTestCase, deposit = Deposit.objects.get(pk=deposit_id) self.assertIsNotNone(deposit) - def test_delete_partial_deposit_works(self): + @istest + def delete_partial_deposit_works(self): """Delete deposit should return a 204 response """ @@ -90,7 +96,8 @@ class DepositDeleteTest(APITestCase, WithAuthTestCase, BasicTestCase, deposits = list(Deposit.objects.filter(pk=deposit_id)) self.assertEquals(deposits, []) - def test_delete_on_edit_se_iri_cannot_delete_non_partial_deposit(self): + @istest + def delete_on_edit_se_iri_cannot_delete_non_partial_deposit(self): """Delete !partial deposit should return a 400 response """ diff --git a/swh/deposit/tests/api/test_deposit_multipart.py b/swh/deposit/tests/api/test_deposit_multipart.py index f00f2afbc6d7e5e2271f5e3644c19335e57923f5..59f26f94bde585de84f3cb0bf9dcb989226d0f0d 100644 --- a/swh/deposit/tests/api/test_deposit_multipart.py +++ b/swh/deposit/tests/api/test_deposit_multipart.py @@ -6,6 +6,7 @@ from django.core.files.uploadedfile import InMemoryUploadedFile from django.core.urlresolvers import reverse from io import BytesIO +from nose.tools import istest from rest_framework import status from rest_framework.test import APITestCase @@ -60,7 +61,8 @@ class DepositMultipartTestCase(APITestCase, WithAuthTestCase, BasicTestCase): <dcterms:type>Type</dcterms:type> </entry>""" - def test_post_deposit_multipart_without_slug_header_is_bad_request(self): + @istest + def post_deposit_multipart_without_slug_header_is_bad_request(self): # given url = reverse(COL_IRI, args=[self.collection.name]) data_atom_entry = self.data_atom_entry_ok @@ -97,7 +99,8 @@ class DepositMultipartTestCase(APITestCase, WithAuthTestCase, BasicTestCase): self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) - def test_post_deposit_multipart(self): + @istest + def post_deposit_multipart(self): """one multipart deposit should be accepted """ @@ -164,7 +167,8 @@ class DepositMultipartTestCase(APITestCase, WithAuthTestCase, BasicTestCase): '{http://www.w3.org/2005/Atom}id'], 'urn:uuid:1225c695-cfb8-4ebb-aaaa-80da344efa6a') - def test_post_deposit_multipart_put_to_replace_metadata(self): + @istest + def post_deposit_multipart_put_to_replace_metadata(self): """One multipart deposit followed by a metadata update should be accepted @@ -263,7 +267,8 @@ class DepositMultipartTestCase(APITestCase, WithAuthTestCase, BasicTestCase): # FAILURE scenarios - def test_post_deposit_multipart_only_archive_and_atom_entry(self): + @istest + def post_deposit_multipart_only_archive_and_atom_entry(self): """Multipart deposit only accepts one archive and one atom+xml""" # given url = reverse(COL_IRI, args=[self.collection.name]) diff --git a/swh/deposit/tests/api/test_deposit_read_archive.py b/swh/deposit/tests/api/test_deposit_read_archive.py new file mode 100644 index 0000000000000000000000000000000000000000..2cd56144833ac79b3e1096e47c696b1fe9ab481d --- /dev/null +++ b/swh/deposit/tests/api/test_deposit_read_archive.py @@ -0,0 +1,173 @@ +# Copyright (C) 2017 The Software Heritage developers +# See the AUTHORS file at the top-level directory of this distribution +# License: GNU General Public License version 3, or any later version +# See top-level LICENSE file for more information + +import hashlib +import os +import shutil +import tempfile + +from django.core.urlresolvers import reverse +from nose.tools import istest +from nose.plugins.attrib import attr +from rest_framework import status +from rest_framework.test import APITestCase + +from swh.loader.tar import tarball +from swh.deposit.config import PRIVATE_GET_RAW_CONTENT +from swh.deposit.tests import TEST_CONFIG + +from ..common import BasicTestCase, WithAuthTestCase, CommonCreationRoutine + + +def _create_arborescence_zip(root_path, archive_name, filename, content): + root_path = '/tmp/swh-deposit/test/build-zip/' + os.makedirs(root_path, exist_ok=True) + archive_path_dir = tempfile.mkdtemp(dir=root_path) + + dir_path = os.path.join(archive_path_dir, archive_name) + os.mkdir(dir_path) + + filepath = os.path.join(dir_path, filename) + with open(filepath, 'wb') as f: + f.write(content) + + zip_path = dir_path + '.zip' + zip_path = tarball.compress(zip_path, 'zip', dir_path) + + with open(zip_path, 'rb') as f: + sha1 = hashlib.sha1() + for chunk in f: + sha1.update(chunk) + + return archive_path_dir, zip_path, sha1.hexdigest() + + +@attr('fs') +class DepositReadArchivesTest(APITestCase, WithAuthTestCase, BasicTestCase, + CommonCreationRoutine): + + def setUp(self): + super().setUp() + + root_path = '/tmp/swh-deposit/test/build-zip/' + os.makedirs(root_path, exist_ok=True) + + archive_path_dir, zip_path, zip_sha1sum = _create_arborescence_zip( + root_path, 'archive1', 'file1', b'some content in file') + + self.archive_path = zip_path + self.archive_path_sha1sum = zip_sha1sum + self.archive_path_dir = archive_path_dir + + archive_path_dir2, zip_path2, zip_sha1sum2 = _create_arborescence_zip( + root_path, 'archive2', 'file2', b'some other content in file') + + self.archive_path2 = zip_path2 + self.archive_path_sha1sum2 = zip_sha1sum2 + self.archive_path_dir2 = archive_path_dir2 + + self.workdir = tempfile.mkdtemp(dir=root_path) + self.root_path = root_path + + def tearDown(self): + shutil.rmtree(self.root_path) + + @istest + def access_to_existing_deposit_with_one_archive(self): + """Access to deposit should stream a 200 response with its raw content + + """ + deposit_id = self.create_simple_binary_deposit( + archive_path=self.archive_path) + + url = reverse(PRIVATE_GET_RAW_CONTENT, + args=[self.collection.name, deposit_id]) + + r = self.client.get(url) + + self.assertEquals(r.status_code, status.HTTP_200_OK) + self.assertEquals(r._headers['content-type'][1], + 'application/octet-stream') + + data = r.content + actual_sha1 = hashlib.sha1(data).hexdigest() + self.assertEquals(actual_sha1, self.archive_path_sha1sum) + + # this does not touch the extraction dir so this should stay empty + self.assertEquals(os.listdir(TEST_CONFIG['extraction_dir']), []) + + def _check_tarball_consistency(self, actual_sha1): + tarball.uncompress(self.archive_path, self.workdir) + self.assertEquals(os.listdir(self.workdir), ['file1']) + tarball.uncompress(self.archive_path2, self.workdir) + self.assertEquals(os.listdir(self.workdir), ['file1', 'file2']) + + new_path = self.workdir + '.zip' + tarball.compress(new_path, 'zip', self.workdir) + with open(new_path, 'rb') as f: + h = hashlib.sha1(f.read()).hexdigest() + + self.assertEqual(actual_sha1, h) + self.assertNotEqual(actual_sha1, self.archive_path_sha1sum) + self.assertNotEqual(actual_sha1, self.archive_path_sha1sum2) + + @istest + def access_to_existing_deposit_with_multiple_archives(self): + """Access to deposit should stream a 200 response with its raw contents + + """ + deposit_id = self.create_complex_binary_deposit( + archive_path=self.archive_path, + archive_path2=self.archive_path2) + + url = reverse(PRIVATE_GET_RAW_CONTENT, + args=[self.collection.name, deposit_id]) + + r = self.client.get(url) + + self.assertEquals(r.status_code, status.HTTP_200_OK) + self.assertEquals(r._headers['content-type'][1], + 'application/octet-stream') + data = r.content + actual_sha1 = hashlib.sha1(data).hexdigest() + self._check_tarball_consistency(actual_sha1) + + # this touches the extraction directory but should clean up + # after itself + self.assertEquals(os.listdir(TEST_CONFIG['extraction_dir']), []) + + +class DepositReadArchivesFailureTest(APITestCase, WithAuthTestCase, + BasicTestCase, CommonCreationRoutine): + @istest + def access_to_nonexisting_deposit_returns_404_response(self): + """Read unknown collection should return a 404 response + + """ + unknown_id = '999' + url = reverse(PRIVATE_GET_RAW_CONTENT, + args=[self.collection.name, unknown_id]) + + response = self.client.get(url) + self.assertEqual(response.status_code, + status.HTTP_404_NOT_FOUND) + self.assertIn('Deposit with id %s does not exist' % unknown_id, + response.content.decode('utf-8')) + + @istest + def access_to_nonexisting_collection_returns_404_response(self): + """Read unknown deposit should return a 404 response + + """ + collection_name = 'non-existing' + deposit_id = self.create_deposit_partial() + url = reverse(PRIVATE_GET_RAW_CONTENT, + args=[collection_name, deposit_id]) + + response = self.client.get(url) + self.assertEqual(response.status_code, + status.HTTP_404_NOT_FOUND) + self.assertIn('Unknown collection name %s' % collection_name, + response.content.decode('utf-8')) diff --git a/swh/deposit/tests/api/test_deposit_read.py b/swh/deposit/tests/api/test_deposit_read_metadata.py similarity index 71% rename from swh/deposit/tests/api/test_deposit_read.py rename to swh/deposit/tests/api/test_deposit_read_metadata.py index c203fda448385d671ecd5e5a502caa1d02f642e7..c1b79a2ee2af48a3971379f64b15610d8f1d57b1 100644 --- a/swh/deposit/tests/api/test_deposit_read.py +++ b/swh/deposit/tests/api/test_deposit_read_metadata.py @@ -6,6 +6,7 @@ import json from django.core.urlresolvers import reverse +from nose.tools import istest from rest_framework import status from rest_framework.test import APITestCase @@ -19,7 +20,8 @@ class DepositReadMetadataTest(APITestCase, WithAuthTestCase, BasicTestCase, """Deposit access to read metadata information on deposit. """ - def test_access_to_an_existing_deposit_returns_metadata(self): + @istest + def access_to_an_existing_deposit_returns_metadata(self): deposit_id = self.create_deposit_partial() url = reverse(PRIVATE_GET_DEPOSIT_METADATA, @@ -59,8 +61,9 @@ class DepositReadMetadataTest(APITestCase, WithAuthTestCase, BasicTestCase, self.assertEquals(data, expected_meta) - def test_access_to_unexisting_collection_returns_404_response(self): - """Read unknown deposit should return a 404 response + @istest + def access_to_nonexisting_deposit_returns_404_response(self): + """Read unknown collection should return a 404 response """ unknown_id = '999' @@ -70,3 +73,21 @@ class DepositReadMetadataTest(APITestCase, WithAuthTestCase, BasicTestCase, response = self.client.get(url) self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) + self.assertIn('Deposit with id %s does not exist' % unknown_id, + response.content.decode('utf-8')) + + @istest + def access_to_nonexisting_collection_returns_404_response(self): + """Read unknown deposit should return a 404 response + + """ + collection_name = 'non-existing' + deposit_id = self.create_deposit_partial() + url = reverse(PRIVATE_GET_DEPOSIT_METADATA, + args=[collection_name, deposit_id]) + + response = self.client.get(url) + self.assertEqual(response.status_code, + status.HTTP_404_NOT_FOUND) + self.assertIn('Unknown collection name %s' % collection_name, + response.content.decode('utf-8'),) diff --git a/swh/deposit/tests/api/test_deposit_status.py b/swh/deposit/tests/api/test_deposit_status.py index 76baf4eb9f8558423120df150c183aad614fc9b9..48262625fe80317f5806124cd9e054647e48aedf 100644 --- a/swh/deposit/tests/api/test_deposit_status.py +++ b/swh/deposit/tests/api/test_deposit_status.py @@ -7,6 +7,7 @@ import hashlib from django.core.urlresolvers import reverse from io import BytesIO +from nose.tools import istest from rest_framework import status from rest_framework.test import APITestCase @@ -21,7 +22,8 @@ class DepositStatusTestCase(APITestCase, WithAuthTestCase, BasicTestCase): """Status on deposit """ - def test_post_deposit_with_status_check(self): + @istest + def post_deposit_with_status_check(self): """Binary upload should be accepted """ @@ -66,7 +68,8 @@ class DepositStatusTestCase(APITestCase, WithAuthTestCase, BasicTestCase): self.assertEqual(r['{http://www.w3.org/2005/Atom}detail'], 'deposit is fully received and ready for injection') - def test_status_on_unknown_deposit(self): + @istest + def status_on_unknown_deposit(self): """Asking for the status of unknown deposit returns 404 response""" status_url = reverse(STATE_IRI, args=[self.collection.name, 999]) status_response = self.client.get(status_url) diff --git a/swh/deposit/tests/api/test_deposit_update.py b/swh/deposit/tests/api/test_deposit_update.py index a939d76a7a643402ef50484b71a6477b144076c2..c9e23469738b783ca4f1bf07755af862ad1bb27d 100644 --- a/swh/deposit/tests/api/test_deposit_update.py +++ b/swh/deposit/tests/api/test_deposit_update.py @@ -6,6 +6,7 @@ import hashlib from django.core.urlresolvers import reverse +from nose.tools import istest from rest_framework import status from rest_framework.test import APITestCase @@ -27,7 +28,8 @@ class DepositReplaceExistingDataTest(APITestCase, WithAuthTestCase, <foobar>bar</foobar> </entry>""" - def test_replace_archive_to_deposit_is_possible(self): + @istest + def replace_archive_to_deposit_is_possible(self): """Replace all archive with another one should return a 204 response """ @@ -78,7 +80,8 @@ class DepositReplaceExistingDataTest(APITestCase, WithAuthTestCase, deposit=deposit, type=self.deposit_request_types['metadata'])) self.assertEquals(len(requests), 1) - def test_replace_metadata_to_deposit_is_possible(self): + @istest + def replace_metadata_to_deposit_is_possible(self): """Replace all metadata with another one should return a 204 response """ @@ -137,7 +140,8 @@ class DepositUpdateDepositWithNewDataTest( <foobar>bar</foobar> </entry>""" - def test_add_archive_to_deposit_is_possible(self): + @istest + def add_archive_to_deposit_is_possible(self): """Add another archive to a deposit return a 201 response """ @@ -191,7 +195,8 @@ class DepositUpdateDepositWithNewDataTest( deposit=deposit, type=self.deposit_request_types['metadata'])) self.assertEquals(len(requests), 1) - def test_add_metadata_to_deposit_is_possible(self): + @istest + def add_metadata_to_deposit_is_possible(self): """Replace all metadata with another one should return a 204 response """ @@ -245,7 +250,8 @@ class DepositUpdateFailuresTest(APITestCase, WithAuthTestCase, BasicTestCase, """Failure scenario about add/replace (post/put) query on deposit. """ - def test_add_metadata_to_unknown_collection(self): + @istest + def add_metadata_to_unknown_collection(self): """Replacing metadata to unknown deposit should return a 404 response """ @@ -257,7 +263,8 @@ class DepositUpdateFailuresTest(APITestCase, WithAuthTestCase, BasicTestCase, data=self.atom_entry_data0) self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) - def test_add_metadata_to_unknown_deposit(self): + @istest + def add_metadata_to_unknown_deposit(self): """Replacing metadata to unknown deposit should return a 404 response """ @@ -269,7 +276,8 @@ class DepositUpdateFailuresTest(APITestCase, WithAuthTestCase, BasicTestCase, data=self.atom_entry_data0) self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) - def test_replace_metadata_to_unknown_deposit(self): + @istest + def replace_metadata_to_unknown_deposit(self): """Adding metadata to unknown deposit should return a 404 response """ @@ -281,7 +289,8 @@ class DepositUpdateFailuresTest(APITestCase, WithAuthTestCase, BasicTestCase, data=self.atom_entry_data0) self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) - def test_add_archive_to_unknown_deposit(self): + @istest + def add_archive_to_unknown_deposit(self): """Adding metadata to unknown deposit should return a 404 response """ @@ -293,7 +302,8 @@ class DepositUpdateFailuresTest(APITestCase, WithAuthTestCase, BasicTestCase, data=self.atom_entry_data0) self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) - def test_replace_archive_to_unknown_deposit(self): + @istest + def replace_archive_to_unknown_deposit(self): """Replacing archive to unknown deposit should return a 404 response """ @@ -305,7 +315,8 @@ class DepositUpdateFailuresTest(APITestCase, WithAuthTestCase, BasicTestCase, data=self.atom_entry_data0) self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) - def test_post_metadata_to_em_iri_failure(self): + @istest + def post_metadata_to_em_iri_failure(self): """Add archive with wrong content type should return a 400 response """ @@ -318,7 +329,8 @@ class DepositUpdateFailuresTest(APITestCase, WithAuthTestCase, BasicTestCase, data=self.atom_entry_data0) self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) - def test_put_metadata_to_em_iri_failure(self): + @istest + def put_metadata_to_em_iri_failure(self): """Update archive with wrong content type should return 400 response """ diff --git a/swh/deposit/tests/api/test_deposit_update_status.py b/swh/deposit/tests/api/test_deposit_update_status.py index 046851bfa0a3db8383bef3dd6fc591d70b0b4d08..6980b8d626c4bc0b49bceb55085883b927459e81 100644 --- a/swh/deposit/tests/api/test_deposit_update_status.py +++ b/swh/deposit/tests/api/test_deposit_update_status.py @@ -6,6 +6,7 @@ import json from django.core.urlresolvers import reverse +from nose.tools import istest from rest_framework import status from rest_framework.test import APITestCase @@ -27,7 +28,8 @@ class UpdateDepositStatusTest(APITestCase, BasicTestCase): self.deposit = Deposit.objects.get(pk=deposit.id) assert self.deposit.status == 'ready' - def test_update_deposit_status(self): + @istest + def update_deposit_status(self): """Existing status for update should return a 204 response """ @@ -47,7 +49,8 @@ class UpdateDepositStatusTest(APITestCase, BasicTestCase): deposit = Deposit.objects.get(pk=self.deposit.id) self.assertEquals(deposit.status, _status) - def test_update_deposit_with_success_ingestion_and_swh_id(self): + @istest + def update_deposit_with_success_ingestion_and_swh_id(self): """Existing status for update should return a 204 response """ @@ -71,7 +74,8 @@ class UpdateDepositStatusTest(APITestCase, BasicTestCase): self.assertEquals(deposit.status, expected_status) self.assertEquals(deposit.swh_id, expected_id) - def test_update_deposit_status_will_fail_with_unknown_status(self): + @istest + def update_deposit_status_will_fail_with_unknown_status(self): """Unknown status for update should return a 400 response """ @@ -85,7 +89,8 @@ class UpdateDepositStatusTest(APITestCase, BasicTestCase): self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) - def test_update_deposit_status_will_fail_with_no_status_key(self): + @istest + def update_deposit_status_will_fail_with_no_status_key(self): """No status provided for update should return a 400 response """ @@ -99,7 +104,8 @@ class UpdateDepositStatusTest(APITestCase, BasicTestCase): self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) - def test_update_deposit_status_success_without_swh_id_fail(self): + @istest + def update_deposit_status_success_without_swh_id_fail(self): """Providing 'success' status without swh_id should return a 400 """ diff --git a/swh/deposit/tests/api/test_service_document.py b/swh/deposit/tests/api/test_service_document.py index 2ace44a7502a9d9ec6615af3d455cd907902449c..dfb7f7448bcb04996306595739dd9f3fd90ab870 100644 --- a/swh/deposit/tests/api/test_service_document.py +++ b/swh/deposit/tests/api/test_service_document.py @@ -4,6 +4,7 @@ # See top-level LICENSE file for more information from django.core.urlresolvers import reverse +from nose.tools import istest from rest_framework import status from rest_framework.test import APITestCase @@ -15,7 +16,8 @@ class ServiceDocumentNoAuthCase(APITestCase, BasicTestCase): """Service document endpoints are protected with basic authentication. """ - def test_service_document_no_authentication_fails(self): + @istest + def service_document_no_authentication_fails(self): """Without authentication, service document endpoint is unauthorized""" url = reverse(SD_IRI) @@ -25,7 +27,8 @@ class ServiceDocumentNoAuthCase(APITestCase, BasicTestCase): class ServiceDocumentCase(APITestCase, WithAuthTestCase, BasicTestCase): - def test_service_document(self): + @istest + def service_document(self): """With authentication, service document list user's collection """ diff --git a/swh/deposit/tests/common.py b/swh/deposit/tests/common.py index fa5faf3655b7be80563da5234f2d5a38379bdc00..f1d6456af44ee1526f0cc252ed837e7b00ad0347 100644 --- a/swh/deposit/tests/common.py +++ b/swh/deposit/tests/common.py @@ -107,12 +107,78 @@ class CommonCreationRoutine(TestCase): """ response = self.client.post( - reverse(COL_IRI, args=[self.username]), + reverse(COL_IRI, args=[self.collection.name]), content_type='application/atom+xml;type=entry', data=self.atom_entry_data0, HTTP_SLUG='external-id', HTTP_IN_PROGRESS='true') + assert response.status_code == status.HTTP_201_CREATED + response_content = parse_xml(BytesIO(response.content)) + deposit_id = response_content[ + '{http://www.w3.org/2005/Atom}deposit_id'] + return deposit_id + + def _init_data_from(self, archive_path, default_data): + if not archive_path: + data = default_data + else: + with open(archive_path, 'rb') as f: + data = f.read() + + md5sum = hashlib.md5(data).hexdigest() + return data, md5sum + + def create_simple_binary_deposit(self, status_partial=False, + archive_path=None): + + data, md5sum = self._init_data_from( + archive_path, b'some simulation data to pass as binary package') + + response = self.client.post( + reverse(COL_IRI, args=[self.collection.name]), + content_type='application/zip', + data=data, + HTTP_MD5SUM=md5sum, + HTTP_SLUG='external-id', + HTTP_IN_PROGRESS=status_partial, + HTTP_CONTENT_DISPOSITION='attachment; filename=filename0.zip') + + # then + if response.status_code != status.HTTP_201_CREATED: + print(response.status_code, response.content) + + # then + assert response.status_code == status.HTTP_201_CREATED + response_content = parse_xml(BytesIO(response.content)) + deposit_id = response_content[ + '{http://www.w3.org/2005/Atom}deposit_id'] + return deposit_id + + def create_complex_binary_deposit(self, status_partial=False, + archive_path=None, + archive_path2=None): + + deposit_id = self.create_simple_binary_deposit( + status_partial=True, + archive_path=archive_path) + + # Update the deposit to add another archive + # and update its status to 'ready' + data, md5sum = self._init_data_from( + archive_path2, b'some other data to pass as binary package') + + # Add a second archive to the deposit + # update its status to 'ready' + response = self.client.post( + reverse(EM_IRI, args=[self.collection.name, deposit_id]), + content_type='application/zip', + data=data, + HTTP_MD5SUM=md5sum, + HTTP_SLUG='external-id', + HTTP_IN_PROGRESS=status_partial, + HTTP_CONTENT_DISPOSITION='attachment; filename=filename1.zip') + # then assert response.status_code == status.HTTP_201_CREATED response_content = parse_xml(BytesIO(response.content)) @@ -134,7 +200,7 @@ class CommonCreationRoutine(TestCase): # when response = self.client.post( - reverse(EM_IRI, args=[self.username, deposit_id]), + reverse(EM_IRI, args=[self.collection.name, deposit_id]), content_type='application/zip', # as zip data=data_text, # + headers diff --git a/version.txt b/version.txt index b5da016c58b10cf3460f82c68bbdd5fb5e40603d..c4b2f804debab4a28e07635a23c9c90205e2ead1 100644 --- a/version.txt +++ b/version.txt @@ -1 +1 @@ -v0.0.20-0-g0df83b9 \ No newline at end of file +v0.0.21-0-g4f4b192 \ No newline at end of file