diff --git a/.gitignore b/.gitignore index ce2a48b2..ad5f83d4 100644 --- a/.gitignore +++ b/.gitignore @@ -14,6 +14,7 @@ build/ coverage coverage.data .coverage +htmlcov/ # junk .DS_Store diff --git a/Containerfile b/Containerfile index c27ed6aa..ea443227 100644 --- a/Containerfile +++ b/Containerfile @@ -23,6 +23,7 @@ RUN pip install --no-cache-dir -r /tmp/requirements.txt # Copy the application code to the container COPY src /app +COPY pytest.ini /app/pytest.ini WORKDIR /app # Run collectstatic to gather static files diff --git a/Makefile b/Makefile index 5a712284..253562bc 100644 --- a/Makefile +++ b/Makefile @@ -1,4 +1,4 @@ -.PHONY: test-env test test-clean build gcp-push gcp-restart gcp-deploy +.PHONY: test-env test test-coverage test-coverage-html test-clean build gcp-push gcp-restart gcp-deploy # Build the container image build: @@ -32,7 +32,15 @@ test-env: # Run tests in a clean container environment test: test-env test-clean - podman-compose run --rm test + podman-compose run --rm test pytest + +# Run tests with coverage report +test-coverage: test-env test-clean + podman-compose run --rm test pytest --cov=sa_api_v2 --cov-report=term-missing + +# Run tests with HTML coverage report (outputs to htmlcov/) +test-coverage-html: test-env test-clean + podman-compose run --rm test pytest --cov=sa_api_v2 --cov-report=html # Just clean up containers test-clean: diff --git a/compose.yml b/compose.yml index eb72d625..59d64628 100644 --- a/compose.yml +++ b/compose.yml @@ -13,8 +13,8 @@ services: environment: - REDIS_URL=redis://redis:6379/0 depends_on: - db: {"condition": "service_healthy"} - redis: {"condition": "service_healthy"} + db: { "condition": "service_healthy" } + redis: { "condition": "service_healthy" } restart: "no" web: @@ -27,9 +27,9 @@ services: ports: - "8000:8000" depends_on: - db: {"condition": "service_healthy"} - redis: {"condition": "service_healthy"} - init: {"condition": "service_completed_successfully"} + db: { "condition": "service_healthy" } + redis: { "condition": "service_healthy" } + init: { "condition": "service_completed_successfully" } worker: build: @@ -41,22 +41,24 @@ services: C_FORCE_ROOT: "true" REDIS_URL: "redis://redis:6379/0" depends_on: - db: {"condition": "service_healthy"} - redis: {"condition": "service_healthy"} - init: {"condition": "service_completed_successfully"} + db: { "condition": "service_healthy" } + redis: { "condition": "service_healthy" } + init: { "condition": "service_completed_successfully" } test: - build: - context: . - dockerfile: Containerfile - command: python3 manage.py test . + image: shareabouts-api + command: pytest env_file: .env environment: - REDIS_URL=redis://redis:6379/0 + volumes: + - ./htmlcov:/app/htmlcov + - ./src:/app + - ./pytest.ini:/app/pytest.ini depends_on: - db: {"condition": "service_healthy"} - redis: {"condition": "service_healthy"} - init: {"condition": "service_completed_successfully"} + db: { "condition": "service_healthy" } + redis: { "condition": "service_healthy" } + init: { "condition": "service_completed_successfully" } db: image: postgis/postgis:15-3.3 @@ -67,7 +69,7 @@ services: ports: - "15432:5432" healthcheck: - test: [ "CMD-SHELL","psql -U postgres -d shareabouts -c \"SELECT ST_Buffer( ST_SetSRID('POINT(0 0)'::GEOMETRY, 4326), 1) AS geom ;\""] + test: [ "CMD-SHELL", "psql -U postgres -d shareabouts -c \"SELECT ST_Buffer( ST_SetSRID('POINT(0 0)'::GEOMETRY, 4326), 1) AS geom ;\"" ] interval: 10s timeout: 5s retries: 5 @@ -80,4 +82,4 @@ services: test: [ "CMD", "redis-cli", "ping" ] interval: 10s timeout: 5s - retries: 5 \ No newline at end of file + retries: 5 diff --git a/pytest.ini b/pytest.ini new file mode 100644 index 00000000..b81679ec --- /dev/null +++ b/pytest.ini @@ -0,0 +1,4 @@ +[pytest] +DJANGO_SETTINGS_MODULE = project.settings +python_files = tests.py test_*.py *_tests.py +addopts = --reuse-db --nomigrations diff --git a/requirements.txt b/requirements.txt index 87cb938d..d3547c21 100644 --- a/requirements.txt +++ b/requirements.txt @@ -45,6 +45,10 @@ prompt-toolkit==3.0.43 psycopg==3.1.17 psycopg-binary==3.1.17 pycparser==2.21 +pytest==7.4.4 +pytest-cov==4.1.0 +pytest-django==4.7.0 +pytest-xdist==3.5.0 PyJWT==2.8.0 python-dateutil==2.8.2 python-social-auth==0.2.21 diff --git a/src/project/settings.py b/src/project/settings.py index f5f528de..7b2fd707 100644 --- a/src/project/settings.py +++ b/src/project/settings.py @@ -66,7 +66,6 @@ LANGUAGE_CODE = 'en-us' USE_I18N = True -USE_L10N = True ############################################################################### # @@ -106,7 +105,8 @@ }, ] -ATTACHMENT_STORAGE = 'django.core.files.storage.FileSystemStorage' +# Use the new div-based form rendering to avoid RemovedInDjango50Warning +FORM_RENDERER = 'django.forms.renderers.DjangoDivFormRenderer' ############################################################################### # @@ -433,16 +433,27 @@ def custom_show_toolbar(request): # We support S3, GCS, and local filesystem. # Precedence: GCS > S3 > Local -DEFAULT_FILE_STORAGE = 'django.core.files.storage.FileSystemStorage' -STATICFILES_STORAGE = 'django.contrib.staticfiles.storage.StaticFilesStorage' -ATTACHMENT_STORAGE = DEFAULT_FILE_STORAGE +STORAGES = { + 'default': { + 'BACKEND': 'django.core.files.storage.FileSystemStorage', + }, + # Static files (CSS, JavaScript, Images) + # https://docs.djangoproject.com/en/1.6/howto/static-files/ + 'staticfiles': { + 'BACKEND': 'django.contrib.staticfiles.storage.StaticFilesStorage', + }, + 'attachments': { + 'BACKEND': 'django.core.files.storage.FileSystemStorage', + }, +} +ATTACHMENT_STORAGE = STORAGES['attachments']['BACKEND'] if 'GS_BUCKET_NAME' in environ: # Google Cloud Storage GS_BUCKET_NAME = environ['GS_BUCKET_NAME'] GS_PROJECT_ID = environ.get('GS_PROJECT_ID') - DEFAULT_FILE_STORAGE = "storages.backends.gcloud.GoogleCloudStorage" + STORAGES['default']['BACKEND'] = "storages.backends.gcloud.GoogleCloudStorage" GS_DEFAULT_ACL = "publicRead" @@ -456,7 +467,7 @@ def custom_show_toolbar(request): MEDIA_URL = f"https://storage.googleapis.com/{GS_BUCKET_NAME}/media/" # Attachments - ATTACHMENT_STORAGE = DEFAULT_FILE_STORAGE + ATTACHMENT_STORAGE = STORAGES['attachments']['BACKEND'] = STORAGES['default']['BACKEND'] elif all([key in environ for key in ('SHAREABOUTS_AWS_KEY', 'SHAREABOUTS_AWS_SECRET', @@ -468,8 +479,8 @@ def custom_show_toolbar(request): AWS_QUERYSTRING_AUTH = False AWS_PRELOAD_METADATA = True - DEFAULT_FILE_STORAGE = 'storages.backends.s3boto3.S3Boto3Storage' - ATTACHMENT_STORAGE = DEFAULT_FILE_STORAGE + STORAGES['default']['BACKEND'] = 'storages.backends.s3boto3.S3Boto3Storage' + ATTACHMENT_STORAGE = STORAGES['attachments']['BACKEND'] = STORAGES['default']['BACKEND'] if 'SHAREABOUTS_TWITTER_KEY' in environ and 'SHAREABOUTS_TWITTER_SECRET' in environ: diff --git a/src/pytest.ini b/src/pytest.ini new file mode 100755 index 00000000..e69de29b diff --git a/src/sa_api_v2/admin.py b/src/sa_api_v2/admin.py index ecf2f2b1..4312c98d 100644 --- a/src/sa_api_v2/admin.py +++ b/src/sa_api_v2/admin.py @@ -82,7 +82,7 @@ def render(self, name, value, attrs=None, renderer=None): return super(PrettyAceWidget, self).render(name, value, attrs=attrs, renderer=renderer) -BaseGeoAdmin = admin.OSMGeoAdmin if settings.USE_GEODB else admin.ModelAdmin +BaseGeoAdmin = admin.GISModelAdmin if settings.USE_GEODB else admin.ModelAdmin class SubmittedThingAdmin(BaseGeoAdmin): diff --git a/src/sa_api_v2/models/core.py b/src/sa_api_v2/models/core.py index 1e787ad8..6d05d4c1 100644 --- a/src/sa_api_v2/models/core.py +++ b/src/sa_api_v2/models/core.py @@ -7,7 +7,7 @@ else: from django.db import models -from django.core.files.storage import get_storage_class +from django.core.files.storage import storages from django.utils.timezone import now from .. import cache from .. import utils @@ -18,6 +18,10 @@ from PIL import Image, UnidentifiedImageError +# Create a simple AttachmentStorage factory +AttachmentStorage = lambda *a, **kw : storages.create_storage(storages.backends['attachments'], *a, **kw) + + class TimeStampedModel (models.Model): created_datetime = models.DateTimeField(default=now, blank=True, db_index=True) updated_datetime = models.DateTimeField(auto_now=True, db_index=True) @@ -294,9 +298,6 @@ def timestamp_filename(attachment, filename): return ''.join(['attachments/', utils.base62_time(), '-', filename]) -AttachmentStorage = get_storage_class(settings.ATTACHMENT_STORAGE) - - class Attachment (CacheClearingModel, TimeStampedModel): """ A file attached to a submitted thing. diff --git a/src/sa_api_v2/renderers.py b/src/sa_api_v2/renderers.py index ed50802c..06e08442 100644 --- a/src/sa_api_v2/renderers.py +++ b/src/sa_api_v2/renderers.py @@ -51,7 +51,7 @@ class GeoJSONRenderer(JSONRenderer): """ media_type = 'application/json' - format = 'json' + format = 'geojson' geometry_field = 'geometry' id_field = 'id' diff --git a/src/sa_api_v2/tests/test_bulk_data_views.py b/src/sa_api_v2/tests/test_bulk_data_views.py new file mode 100644 index 00000000..79846d3a --- /dev/null +++ b/src/sa_api_v2/tests/test_bulk_data_views.py @@ -0,0 +1,279 @@ +import json +from django.test import TestCase +from django.test.client import RequestFactory +from django.urls import reverse +from django.core.cache import cache as django_cache +from unittest import mock + +from ..models import User, DataSet, Place, DataSnapshotRequest, DataSnapshot +from ..cache import cache_buffer +from ..apikey.models import ApiKey +from ..views import DataSnapshotRequestListView, DataSnapshotInstanceView + + +class TestDataSnapshotRequestListView(TestCase): + def setUp(self): + self.owner = User.objects.create_user(username='owner', password='123', email='owner@example.com') + self.dataset = DataSet.objects.create(slug='test-dataset', owner=self.owner) + self.apikey = ApiKey.objects.create(key='abc123', dataset=self.dataset) + + self.factory = RequestFactory() + self.request_kwargs = { + 'owner_username': self.owner.username, + 'dataset_slug': self.dataset.slug, + 'submission_set_name': 'places' + } + self.path = reverse('dataset-snapshot-list', kwargs=self.request_kwargs) + self.view = DataSnapshotRequestListView.as_view() + + cache_buffer.reset() + django_cache.clear() + + def tearDown(self): + User.objects.all().delete() + DataSet.objects.all().delete() + DataSnapshotRequest.objects.all().delete() + ApiKey.objects.all().delete() + + cache_buffer.reset() + django_cache.clear() + + def test_GET_empty_list(self): + """GET should return empty list when no snapshots exist.""" + request = self.factory.get(self.path) + request.user = self.owner + response = self.view(request, **self.request_kwargs) + + self.assertEqual(response.status_code, 200) + self.assertEqual(response.data, []) + + def test_GET_with_existing_snapshots(self): + """GET should return list of existing snapshots.""" + # Create a snapshot request + snapshot = DataSnapshotRequest.objects.create( + dataset=self.dataset, + submission_set='places', + status='success', + guid='test-guid-123' + ) + + request = self.factory.get(self.path) + request.user = self.owner + response = self.view(request, **self.request_kwargs) + + self.assertEqual(response.status_code, 200) + self.assertEqual(len(response.data), 1) + self.assertEqual(response.data[0]['status'], 'success') + + @mock.patch('sa_api_v2.views.bulk_data_views.store_bulk_data') + def test_POST_creates_snapshot_request(self, mock_store): + """POST should create a new snapshot request.""" + # Mock the celery task + mock_task = mock.MagicMock() + mock_task.id = 'mock-task-id' + mock_store.apply_async.return_value = mock_task + + request = self.factory.post(self.path) + request.user = self.owner + response = self.view(request, **self.request_kwargs) + + self.assertEqual(response.status_code, 202) + self.assertEqual(response.data['status'], 'pending') + self.assertIn('url', response.data) + + # Verify the celery task was called + mock_store.apply_async.assert_called_once() + + @mock.patch('sa_api_v2.views.bulk_data_views.store_bulk_data') + def test_POST_returns_existing_pending_request(self, mock_store): + """POST should return existing pending request instead of creating duplicate.""" + # Create an existing pending request + existing = DataSnapshotRequest.objects.create( + dataset=self.dataset, + submission_set='places', + status='pending', + guid='existing-guid' + ) + + request = self.factory.post(self.path) + request.user = self.owner + response = self.view(request, **self.request_kwargs) + + self.assertEqual(response.status_code, 202) + # Should not have called the task since we're returning existing + mock_store.apply_async.assert_not_called() + + @mock.patch('sa_api_v2.views.bulk_data_views.store_bulk_data') + def test_GET_with_new_param_creates_request(self, mock_store): + """GET with ?new parameter should create a new snapshot request.""" + mock_task = mock.MagicMock() + mock_task.id = 'mock-task-id' + mock_store.apply_async.return_value = mock_task + + request = self.factory.get(self.path + '?new=true') + request.user = self.owner + response = self.view(request, **self.request_kwargs) + + self.assertEqual(response.status_code, 202) + mock_store.apply_async.assert_called_once() + + +class TestDataSnapshotInstanceView(TestCase): + def setUp(self): + self.owner = User.objects.create_user(username='owner', password='123', email='owner@example.com') + self.dataset = DataSet.objects.create(slug='test-dataset', owner=self.owner) + self.apikey = ApiKey.objects.create(key='abc123', dataset=self.dataset) + + # Create a fulfilled snapshot request + self.snapshot_request = DataSnapshotRequest.objects.create( + dataset=self.dataset, + submission_set='places', + status='success', + guid='test-guid-123' + ) + self.snapshot = DataSnapshot.objects.create( + request=self.snapshot_request, + json='{"type": "FeatureCollection", "features": []}', + csv='id,name\n1,Test' + ) + + self.factory = RequestFactory() + self.request_kwargs = { + 'owner_username': self.owner.username, + 'dataset_slug': self.dataset.slug, + 'submission_set_name': 'places', + 'data_guid': 'test-guid-123' + } + self.path = reverse('dataset-snapshot-instance', kwargs=self.request_kwargs) + self.view = DataSnapshotInstanceView.as_view() + + cache_buffer.reset() + django_cache.clear() + + def tearDown(self): + User.objects.all().delete() + DataSet.objects.all().delete() + DataSnapshotRequest.objects.all().delete() + DataSnapshot.objects.all().delete() + ApiKey.objects.all().delete() + + cache_buffer.reset() + django_cache.clear() + + def test_GET_snapshot_json(self): + """GET should return snapshot data as JSON.""" + request = self.factory.get(self.path) + request.user = self.owner + response = self.view(request, **self.request_kwargs) + + self.assertEqual(response.status_code, 200) + self.assertEqual(response['Content-Type'], 'application/json') + + def test_GET_snapshot_csv(self): + """GET with format=csv should return CSV data.""" + request_kwargs = self.request_kwargs.copy() + request_kwargs['format'] = 'csv' + # Append format manually since reverse with optional group is tricky + path = f"{self.path}.csv" + + request = self.factory.get(path) + request.user = self.owner + response = self.view(request, **request_kwargs) + + self.assertEqual(response.status_code, 200) + self.assertEqual(response['Content-Type'], 'text/csv') + + def test_GET_snapshot_geojson(self): + """GET with format=geojson should return GeoJSON data.""" + request_kwargs = self.request_kwargs.copy() + request_kwargs['format'] = 'geojson' + + # Append format manually since reverse with optional group is tricky + path = f"{self.path}.geojson" + + request = self.factory.get(path) + request.user = self.owner + response = self.view(request, **request_kwargs) + + self.assertEqual(response.status_code, 200) + self.assertEqual(response['Content-Type'], 'application/json') + + def test_GET_invalid_format(self): + """GET with invalid format should return 404.""" + request_kwargs = self.request_kwargs.copy() + request_kwargs['format'] = 'invalid' + + # Append format manually + path = f"{self.path}.invalid" + + request = self.factory.get(path) + request.user = self.owner + response = self.view(request, **request_kwargs) + + self.assertEqual(response.status_code, 404) + + def test_GET_nonexistent_snapshot(self): + """GET for non-existent snapshot should return 404.""" + request_kwargs = self.request_kwargs.copy() + request_kwargs['data_guid'] = 'nonexistent-guid' + request = self.factory.get(self.path) + request.user = self.owner + response = self.view(request, **request_kwargs) + + self.assertEqual(response.status_code, 404) + + def test_GET_pending_snapshot(self): + """GET for pending snapshot should return 503.""" + # Create a pending request without fulfillment + pending_request = DataSnapshotRequest.objects.create( + dataset=self.dataset, + submission_set='places', + status='pending', + guid='pending-guid' + ) + + request_kwargs = self.request_kwargs.copy() + request_kwargs['data_guid'] = 'pending-guid' + request = self.factory.get(self.path) + request.user = self.owner + response = self.view(request, **request_kwargs) + + self.assertEqual(response.status_code, 503) + + def test_GET_failed_snapshot(self): + """GET for failed snapshot should return 404.""" + failed_request = DataSnapshotRequest.objects.create( + dataset=self.dataset, + submission_set='places', + status='failure', + guid='failed-guid' + ) + + request_kwargs = self.request_kwargs.copy() + request_kwargs['data_guid'] = 'failed-guid' + request = self.factory.get(self.path) + request.user = self.owner + response = self.view(request, **request_kwargs) + + self.assertEqual(response.status_code, 404) + + def test_DELETE_snapshot(self): + """DELETE should remove the snapshot.""" + from ..apikey.auth import KEY_HEADER + request = self.factory.delete(self.path) + request.META[KEY_HEADER] = self.apikey.key + response = self.view(request, **self.request_kwargs) + + self.assertEqual(response.status_code, 204) + self.assertFalse(DataSnapshotRequest.objects.filter(guid='test-guid-123').exists()) + + def test_DELETE_nonexistent_snapshot(self): + """DELETE for non-existent snapshot should return 404.""" + from ..apikey.auth import KEY_HEADER + request_kwargs = self.request_kwargs.copy() + request_kwargs['data_guid'] = 'nonexistent-guid' + request = self.factory.delete(self.path) + request.META[KEY_HEADER] = self.apikey.key + response = self.view(request, **request_kwargs) + + self.assertEqual(response.status_code, 404) diff --git a/src/sa_api_v2/tests/test_spatial_views.py b/src/sa_api_v2/tests/test_spatial_views.py new file mode 100644 index 00000000..8649759b --- /dev/null +++ b/src/sa_api_v2/tests/test_spatial_views.py @@ -0,0 +1,126 @@ +from django.test import TestCase +from django.test.client import RequestFactory +from django.urls import reverse +from django.conf import settings +from sa_api_v2.models import User, DataSet, Place +from sa_api_v2.views import PlaceListView +import json + +class APITestMixin(object): + def assertStatusCode(self, response, *expected): + self.assertIn(response.status_code, expected, + 'Status code not in %s response: (%s) %s' % + (expected, response.status_code, response.rendered_content)) + +class TestSpatialViews(APITestMixin, TestCase): + def setUp(self): + self.owner = User.objects.create_user(username='aaron', password='123', email='abc@example.com') + self.dataset = DataSet.objects.create(slug='ds', owner=self.owner) + self.factory = RequestFactory() + + # Create base path for place list + self.request_kwargs = { + 'owner_username': self.owner.username, + 'dataset_slug': self.dataset.slug + } + self.path = reverse('place-list', kwargs=self.request_kwargs) + self.view = PlaceListView.as_view() + + # Create some places at specific coordinates + # (0,0), (10,0), (20,0), (30,0) + self.p1 = Place.objects.create(dataset=self.dataset, geometry='POINT(0 0)', data=json.dumps({'name': 1})) + self.p2 = Place.objects.create(dataset=self.dataset, geometry='POINT(10 0)', data=json.dumps({'name': 2})) + self.p3 = Place.objects.create(dataset=self.dataset, geometry='POINT(20 0)', data=json.dumps({'name': 3})) + self.p4 = Place.objects.create(dataset=self.dataset, geometry='POINT(30 0)', data=json.dumps({'name': 4})) + + def test_GET_near_valid(self): + """GET with valid 'near' param should sort by distance.""" + # Request near (0, 0) + request = self.factory.get(self.path + '?near=0,0') + response = self.view(request, **self.request_kwargs) + + self.assertStatusCode(response, 200) + data = json.loads(response.rendered_content) + + # Should be ordered 1, 2, 3, 4 + self.assertEqual(len(data['features']), 4) + names = [f['properties']['name'] for f in data['features']] + self.assertEqual(names, [1, 2, 3, 4]) + + # Request near (30, 0). to_geom expects "lat, lng". + # So we pass "0, 30" (Lat 0, Lon 30). + request = self.factory.get(self.path + '?near=0,30') + response = self.view(request, **self.request_kwargs) + + self.assertStatusCode(response, 200) + data = json.loads(response.rendered_content) + + # Should be ordered 4, 3, 2, 1 + names = [f['properties']['name'] for f in data['features']] + self.assertEqual(names, [4, 3, 2, 1]) + + def test_GET_near_invalid(self): + """GET with invalid 'near' param should return 400.""" + # Invalid format (not enough coords) + request = self.factory.get(self.path + '?near=0') + response = self.view(request, **self.request_kwargs) + self.assertStatusCode(response, 400) + + # Invalid format (non-numeric) + request = self.factory.get(self.path + '?near=foo,bar') + response = self.view(request, **self.request_kwargs) + self.assertStatusCode(response, 400) + + def test_GET_bbox_valid(self): + """GET with valid 'bbox' param should filter places.""" + # Approx bounding box around (9, -1) to (21, 1) -> Should catch 2 (10,0) and 3 (20,0) + # Bbox format: min_lon,min_lat,max_lon,max_lat + request = self.factory.get(self.path + '?bounds=9,-1,21,1') + response = self.view(request, **self.request_kwargs) + + self.assertStatusCode(response, 200) + data = json.loads(response.rendered_content) + + self.assertEqual(len(data['features']), 2) + names = sorted([f['properties']['name'] for f in data['features']]) + self.assertEqual(names, [2, 3]) + + def test_GET_bbox_invalid(self): + """GET with invalid 'bounds' param should return 400.""" + # Not enough coords + request = self.factory.get(self.path + '?bounds=0,0,10') + response = self.view(request, **self.request_kwargs) + self.assertStatusCode(response, 400) + + # Non-numeric + request = self.factory.get(self.path + '?bounds=a,b,c,d') + response = self.view(request, **self.request_kwargs) + self.assertStatusCode(response, 400) + + def test_GET_distance_valid(self): + """GET with 'distance' param should filter results by distance from 'near'.""" + # Near (0,0), distance 1500km. + # Places at 0 degrees (0km) and 10 degrees (~1111km). + # Place at 20 degrees (~2222km) should be excluded. + request = self.factory.get(self.path + '?near=0,0&distance_lt=1500km') + response = self.view(request, **self.request_kwargs) + + self.assertStatusCode(response, 200) + data = json.loads(response.rendered_content) + + names = sorted([f['properties']['name'] for f in data['features']]) + # Should include 1 (0,0) and 2 (10,0) + self.assertEqual(names, [1, 2]) + + def test_GET_distance_missing_near(self): + """GET with 'distance' but missing 'near' should return 400.""" + request = self.factory.get(self.path + '?distance_lt=10') + response = self.view(request, **self.request_kwargs) + # The view raises QueryError, which usually maps to 400. + self.assertStatusCode(response, 400) + + def test_GET_distance_invalid(self): + """GET with invalid 'distance' param should return 400.""" + request = self.factory.get(self.path + '?near=0,0&distance_lt=invalid') + response = self.view(request, **self.request_kwargs) + self.assertStatusCode(response, 400) diff --git a/src/sa_api_v2/utils.py b/src/sa_api_v2/utils.py index 2a9e74b6..a902aa3b 100644 --- a/src/sa_api_v2/utils.py +++ b/src/sa_api_v2/utils.py @@ -4,6 +4,10 @@ if settings.USE_GEODB: from django.contrib.gis.geos import GEOSGeometry, Point from django.contrib.gis.measure import D + from django.contrib.gis.geos.error import GEOSException +else: + class GEOSException(Exception): pass + from functools import wraps try: @@ -43,7 +47,7 @@ def to_geom(string): """ try: geom = GEOSGeometry(string) - except ValueError: + except (ValueError, GEOSException): try: lat, lng = [float(coord.strip()) for coord in string.split(',')] except ValueError: diff --git a/src/sa_api_v2/views/base_views.py b/src/sa_api_v2/views/base_views.py index 3fcdbbe3..85a83feb 100644 --- a/src/sa_api_v2/views/base_views.py +++ b/src/sa_api_v2/views/base_views.py @@ -534,7 +534,12 @@ def get_queryset(self): if len(bounds) != 4: raise QueryError(detail='Invalid parameter for "%s": %r' % (BBOX_PARAM, self.request.GET[BBOX_PARAM])) - boundingbox = Polygon.from_bbox(bounds) + try: + boundingbox = Polygon.from_bbox(bounds) + boundingbox.srid = 4326 + except ValueError: + raise QueryError(detail='Invalid parameter for "%s": %r' % (BBOX_PARAM, self.request.GET[BBOX_PARAM])) + queryset = queryset.filter(geometry__within=boundingbox) return queryset diff --git a/src/sa_api_v2/views/bulk_data_views.py b/src/sa_api_v2/views/bulk_data_views.py index 1cd0bfdf..965e3470 100644 --- a/src/sa_api_v2/views/bulk_data_views.py +++ b/src/sa_api_v2/views/bulk_data_views.py @@ -103,7 +103,7 @@ def get_characteristic_params(self, request, owner_username, dataset_slug, submi Get the parameters that should identify all snapshots formed off of this query. """ - params = request.GET if request.method.upper() == 'GET' else request.DATA + params = request.GET if request.method.upper() == 'GET' else request.data return { 'dataset': self.get_dataset(), 'submission_set': submission_set_name, @@ -147,6 +147,10 @@ def get(self, request, owner_username, dataset_slug, submission_set_name): for datarequest in datarequests], status=200) +from .. import renderers +from rest_framework.renderers import JSONRenderer +from rest_framework_csv.renderers import CSVRenderer + class DataSnapshotInstanceView (DataSnapshotMixin, OwnedResourceMixin, views.APIView): """ @@ -165,8 +169,10 @@ class DataSnapshotInstanceView (DataSnapshotMixin, OwnedResourceMixin, views.API ------------------------------------------------------------ """ submission_set_name_kwarg = 'submission_set_name' + renderer_classes = (JSONRenderer, renderers.JSONPRenderer, renderers.GeoJSONPRenderer, renderers.GeoJSONRenderer, CSVRenderer) def get(self, request, owner_username, dataset_slug, submission_set_name, data_guid, format=None): + print(f"DEBUG: View called. GUID: {data_guid}, Format: {format}") # Get the snapshot request model try: datarequest = DataSnapshotRequest.objects.get(guid=data_guid)