Skip to content

Commit

Permalink
PB-796: remove CommonMiddlewareWithInternalRedirect.
Browse files Browse the repository at this point in the history
In #440 we added
CommonMiddlewareWithInternalRedirect to respond to URLs that don't end
with a trailing slash without user-visible redirection. This was primarily
done to handle a limitation of AWS API Gateway v1. We eventually decided to
use API Gateway v2 which does not have this limitation.

Hence, we remove this code that is not useful and update the tests accordingly.
This is basically a revert of #440
although we keep some of the test refactoring/improvements.
  • Loading branch information
adk-swisstopo committed Sep 24, 2024
1 parent b46e18d commit ce72ffb
Show file tree
Hide file tree
Showing 6 changed files with 38 additions and 104 deletions.
4 changes: 0 additions & 4 deletions app/config/admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,3 @@
class StacAdminSite(admin.AdminSite):
site_header = _('STAC API admin')
site_title = _('geoadmin STAC API')
# This is normally used to redirect URLs that are missing a trailing slash.
# We have our own special redirection code in middleware.internal_redirect
# for this and it doesn't play well with this feature. So we disable it.
final_catch_all_view = False
2 changes: 1 addition & 1 deletion app/config/settings_prod.py
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@
'middleware.cors.CORSHeadersMiddleware',
'whitenoise.middleware.WhiteNoiseMiddleware',
'django.contrib.sessions.middleware.SessionMiddleware',
'middleware.internal_redirect.CommonMiddlewareWithInternalRedirect',
'django.middleware.common.CommonMiddleware',
'django.middleware.csrf.CsrfViewMiddleware',
'django.contrib.auth.middleware.AuthenticationMiddleware',
'django.contrib.messages.middleware.MessageMiddleware',
Expand Down
51 changes: 0 additions & 51 deletions app/middleware/internal_redirect.py

This file was deleted.

53 changes: 21 additions & 32 deletions app/tests/test_admin_page.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@
import time
from io import BytesIO

from parameterized import parameterized

from django.conf import settings
from django.contrib.auth import get_user_model
from django.test import Client
Expand Down Expand Up @@ -50,13 +48,13 @@ def _setup(self, create_collection=False, create_item=False):
self.item = self._create_item(self.collection)[0]

def _create_collection(
self, name="test_collection", with_link=False, with_provider=False, extra=None
self, with_link=False, with_provider=False, extra=None
):
# Post data to create a new collection
# Note: the *-*_FORMS fields are necessary management form fields
# originating from the AdminInline and must be present
data = {
"name": name,
"name": "test_collection",
"license": "free",
"description": "some very important collection",
"published": "on",
Expand Down Expand Up @@ -321,26 +319,24 @@ def test_admin_page(self):
response = self.client.get("/api/stac/admin/login/?next=/api/stac/admin")
self.assertEqual(response.status_code, 200, "Admin page login not up.")

@parameterized.expand([
("/api/stac/admin/",),
("/api/stac/admin",),
])
def test_login(self, login_path):
def test_login(self):
# Make sure login of the test user works
self.client.login(username=self.username, password=self.password)
response = self.client.get(login_path)
response = self.client.get("/api/stac/admin/")
self.assertEqual(response.status_code, 200, msg="Admin page login failed")

@parameterized.expand([
("/api/stac/admin/",),
("/api/stac/admin",),
])
def test_login_failure(self, login_path):
def test_login_noslash(self):
self.client.login(username=self.username, password=self.password)
response = self.client.get("/api/stac/admin")
self.assertEqual(response.status_code, 301, msg="Admin page redirection failed")
self.assertEqual("/api/stac/admin/", response.url)

def test_login_failure(self):
# Make sure login with wrong password fails
self.client.login(username=self.username, password="wrongpassword")
response = self.client.get(login_path)
response = self.client.get("/api/stac/admin/")
self.assertEqual(response.status_code, 302)
self.assertEqual(f"/api/stac/admin/login/?next={login_path}", response.url)
self.assertEqual(f"/api/stac/admin/login/?next=/api/stac/admin/", response.url)


#--------------------------------------------------------------------------------------------------
Expand Down Expand Up @@ -424,16 +420,12 @@ def test_add_update_collection_with_provider(self):
msg="Admin page wrong provider.roles after update"
)

@parameterized.expand([
("/api/stac/admin/stac_api/collection/add", "test_collection_noslash"),
("/api/stac/admin/stac_api/collection/add/", "test_collection_slash"),
])
def test_add_collection_with_provider_empty_description(self, post_path, collection_name):
def test_add_collection_with_provider_empty_description(self):
# Login the user first
self.client.login(username=self.username, password=self.password)

data = {
"name": collection_name,
"name": "test_collection",
"license": "free",
"description": "some very important collection",
"links-TOTAL_FORMS": "0",
Expand All @@ -447,7 +439,7 @@ def test_add_collection_with_provider_empty_description(self, post_path, collect
"assets-TOTAL_FORMS": "0",
"assets-INITIAL_FORMS": "0"
}
response = self.client.post(post_path, data)
response = self.client.post("/api/stac/admin/stac_api/collection/add/", data)

# Status code for successful creation is 302, since in the admin UI
# you're redirected to the list view after successful creation
Expand All @@ -468,16 +460,11 @@ def test_add_collection_with_provider_empty_description(self, post_path, collect
provider.description, None, msg="Admin page wrong provider.description on creation"
)

@parameterized.expand([
("/api/stac/admin/stac_api/collection/{id}/change", "test_collection_noslash"),
("/api/stac/admin/stac_api/collection/{id}/change/", "test_collection_slash"),
])
def test_add_update_collection_empty_provider_description(self, post_path, collection_name):
def test_add_update_collection_empty_provider_description(self):
# Login the user first
self.client.login(username=self.username, password=self.password)

collection, data, link, provider = self._create_collection(name=collection_name,
with_provider=True)
collection, data, link, provider = self._create_collection(with_provider=True)

self.assertEqual(
provider.description,
Expand All @@ -490,7 +477,9 @@ def test_add_update_collection_empty_provider_description(self, post_path, colle
data["providers-0-id"] = provider.id
data["providers-0-collection"] = collection.id
data["providers-0-description"] = ""
response = self.client.post(post_path.format(id=collection.id), data)
response = self.client.post(
f"/api/stac/admin/stac_api/collection/{collection.id}/change/", data
)

# Status code for successful creation is 302, since in the admin UI
# you're redirected to the list view after successful creation
Expand Down
16 changes: 8 additions & 8 deletions app/tests/tests_09/test_landing_page.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
from parameterized import parameterized

from django.test import Client

from tests.tests_09.base_test import STAC_BASE_V
Expand All @@ -12,12 +10,8 @@ class IndexTestCase(StacBaseTestCase):
def setUp(self):
self.client = Client()

@parameterized.expand([
(f"/{STAC_BASE_V}/",),
(f"/{STAC_BASE_V}",),
])
def test_landing_page(self, path):
response = self.client.get(path)
def test_landing_page(self):
response = self.client.get(f"/{STAC_BASE_V}/")
self.assertEqual(response.status_code, 200)
self.assertCors(response)
self.assertEqual(response['Content-Type'], 'application/json')
Expand All @@ -36,3 +30,9 @@ def test_landing_page(self, path):
set(),
msg="missing required attribute in the answer['links'] array"
)

def test_landing_page_redirect(self):
response = self.client.get(f"/{STAC_BASE_V}")
self.assertEqual(response.status_code, 301)
self.assertLocationHeader(f"/{STAC_BASE_V}/", response)
self.assertCors(response)
16 changes: 8 additions & 8 deletions app/tests/tests_10/test_landing_page.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
from parameterized import parameterized

from django.test import Client

from tests.tests_10.base_test import STAC_BASE_V
Expand All @@ -12,12 +10,8 @@ class IndexTestCase(StacBaseTestCase):
def setUp(self):
self.client = Client()

@parameterized.expand([
(f"/{STAC_BASE_V}/",),
(f"/{STAC_BASE_V}",),
])
def test_landing_page(self, path):
response = self.client.get(path)
def test_landing_page(self):
response = self.client.get(f"/{STAC_BASE_V}/")
self.assertEqual(response.status_code, 200)
self.assertCors(response)
self.assertEqual(response['Content-Type'], 'application/json')
Expand All @@ -36,3 +30,9 @@ def test_landing_page(self, path):
set(),
msg="missing required attribute in the answer['links'] array"
)

def test_landing_page_redirect(self):
response = self.client.get(f"/{STAC_BASE_V}")
self.assertEqual(response.status_code, 301)
self.assertLocationHeader(f"/{STAC_BASE_V}/", response)
self.assertCors(response)

0 comments on commit ce72ffb

Please sign in to comment.