From accc2e979a86e54bdb749c5332f0a497d0aff4b0 Mon Sep 17 00:00:00 2001 From: Trent Smith Date: Fri, 3 Dec 2021 15:04:20 -0800 Subject: [PATCH 1/5] fix: verify caching headers for dataset-metadata --- server/tests/unit/common/apis/test_api_v2.py | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/server/tests/unit/common/apis/test_api_v2.py b/server/tests/unit/common/apis/test_api_v2.py index bc9865894..1787d040c 100644 --- a/server/tests/unit/common/apis/test_api_v2.py +++ b/server/tests/unit/common/apis/test_api_v2.py @@ -745,6 +745,7 @@ def setUpClass(cls): app__flask_secret_key="testing", app__debug=True, data_locator__s3__region_name="us-east-1", + app__generate_cache_control_headers=True, ) cls.meta_response_body = { "collection_id": "4f098ff4-4a12-446b-a841-91ba3d8e3fa6", @@ -758,6 +759,13 @@ def setUpClass(cls): cls.app.testing = True cls.client = cls.app.test_client() + def verify_response(self, result): + self.assertEqual(result.status_code, HTTPStatus.OK) + self.assertEqual(result.headers["Content-Type"], "application/json") + self.assertTrue(result.cache_control.public) + self.assertTrue(result.cache_control.no_store) + self.assertEqual(result.cache_control.max_age, 0) + @patch("server.data_common.dataset_metadata.request_dataset_metadata_from_data_portal") @patch("server.data_common.dataset_metadata.requests.get") def test_dataset_metadata_api_called_for_public_collection(self, mock_get, mock_dp): @@ -790,8 +798,7 @@ def test_dataset_metadata_api_called_for_public_collection(self, mock_get, mock_ url = f"{self.TEST_URL_BASE}{endpoint}" result = self.client.get(url) - self.assertEqual(result.status_code, HTTPStatus.OK) - self.assertEqual(result.headers["Content-Type"], "application/json") + self.verify_response(result) self.assertEqual(mock_get.call_count, 1) @@ -843,8 +850,7 @@ def test_dataset_metadata_api_called_for_private_collection(self, mock_get, mock url = f"{self.TEST_URL_BASE}{endpoint}" result = self.client.get(url) - self.assertEqual(result.status_code, HTTPStatus.OK) - self.assertEqual(result.headers["Content-Type"], "application/json") + self.verify_response(result) self.assertEqual(mock_get.call_count, 1) From d55a19be4b237375f180cb267f0e4f3b90aca777 Mon Sep 17 00:00:00 2001 From: Trent Smith Date: Wed, 8 Dec 2021 12:32:16 -0800 Subject: [PATCH 2/5] fix: add backend E2E tests case for dataset-metadata endpoint --- server/Makefile | 7 +++++++ server/tests/e2e/__init__.py | 0 server/tests/e2e/test_api_cache.py | 25 +++++++++++++++++++++++++ 3 files changed, 32 insertions(+) create mode 100644 server/tests/e2e/__init__.py create mode 100644 server/tests/e2e/test_api_cache.py diff --git a/server/Makefile b/server/Makefile index b04ed7428..2226f27e6 100644 --- a/server/Makefile +++ b/server/Makefile @@ -25,3 +25,10 @@ test-annotations-performance: .PHONY: test-annotations-scale test-annotations-scale: locust -f tests/performance/scale_test_annotations.py --headless -u 30 -r 10 --host https://api.cellxgene.dev.single-cell.czi.technology/cellxgene/e/ --run-time 5m 2>&1 | tee locust_dev_stats.txt + +.PHONY: e2e +e2e: + python -m unittest discover \ + --start-directory tests/e2e \ + --top-level-directory .. \ + --verbose; test_result=$$?; \ diff --git a/server/tests/e2e/__init__.py b/server/tests/e2e/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/server/tests/e2e/test_api_cache.py b/server/tests/e2e/test_api_cache.py new file mode 100644 index 000000000..3e131ae7f --- /dev/null +++ b/server/tests/e2e/test_api_cache.py @@ -0,0 +1,25 @@ +import os +import unittest + +import requests + + +@unittest.skipIf(os.getenv("CXG_API_BASE") is None, "CXG_API_BASE not defined") +class TestAPICache(unittest.TestCase): + def setUp(self): + self.apiUrlBase = os.environ["CXG_API_BASE"] + + def test_dataset_metadata(self): + response = requests.get("/".join([self.apiUrlBase, "dp/v1/collections"])) + response.raise_for_status() + collection_id = response.json()['collections'][0]['id'] + response = requests.get("/".join([self.apiUrlBase, f"dp/v1/collections/{collection_id}"])) + response.raise_for_status() + dataset_id = response.json()['datasets'][0]['id'] + response = requests.head('/'.join([self.apiUrlBase, 'cellxgene/e', f"{dataset_id}.cxg", 'api/v0.3/dataset-metadata'])) + response.raise_for_status() + cache_control = response.headers['cache-control'] + self.assertIn('no-store', cache_control) + self.assertIn('max-age=0', cache_control) + self.assertIn('public', cache_control) + From 3391191b576c23e08505bc1e9b47ae724087768b Mon Sep 17 00:00:00 2001 From: Trent Smith Date: Wed, 8 Dec 2021 12:35:19 -0800 Subject: [PATCH 3/5] fix spacing --- server/Makefile | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/server/Makefile b/server/Makefile index 2226f27e6..20a67e79e 100644 --- a/server/Makefile +++ b/server/Makefile @@ -28,7 +28,7 @@ test-annotations-scale: .PHONY: e2e e2e: - python -m unittest discover \ - --start-directory tests/e2e \ - --top-level-directory .. \ - --verbose; test_result=$$?; \ + python -m unittest discover \ + --start-directory tests/e2e \ + --top-level-directory .. \ + --verbose; test_result=$$?; \ From 36e240a6bc0e0d93aa9391db32f8e4ae88718037 Mon Sep 17 00:00:00 2001 From: Trent Smith Date: Thu, 9 Dec 2021 14:18:59 -0800 Subject: [PATCH 4/5] Adding cache tests for all explorer endpoints --- server/requirements-dev.txt | 1 + server/tests/e2e/test_api_cache.py | 91 ++++++++++++++++++++++++++---- 2 files changed, 80 insertions(+), 12 deletions(-) diff --git a/server/requirements-dev.txt b/server/requirements-dev.txt index 12f505994..aa765005a 100644 --- a/server/requirements-dev.txt +++ b/server/requirements-dev.txt @@ -5,6 +5,7 @@ codecov>=2.0.15 parameterized>=0.7.0 psycopg2-binary>=2.8.5 pytest>=3.6.3 +pytest-subtests>=0.5.0 python-jose>=3.2.0 twine>=1.12.1 -r requirements.txt diff --git a/server/tests/e2e/test_api_cache.py b/server/tests/e2e/test_api_cache.py index 3e131ae7f..630bec034 100644 --- a/server/tests/e2e/test_api_cache.py +++ b/server/tests/e2e/test_api_cache.py @@ -1,25 +1,92 @@ +import json import os import unittest +from urllib.parse import quote import requests @unittest.skipIf(os.getenv("CXG_API_BASE") is None, "CXG_API_BASE not defined") class TestAPICache(unittest.TestCase): - def setUp(self): - self.apiUrlBase = os.environ["CXG_API_BASE"] + """These tests must run against a deployed API backed by AWS cloudfront.""" - def test_dataset_metadata(self): - response = requests.get("/".join([self.apiUrlBase, "dp/v1/collections"])) + def setUp(self): + self.api_url_base = os.environ["CXG_API_BASE"].rstrip("/") + response = requests.get("/".join([self.api_url_base, "dp/v1/collections"])) response.raise_for_status() - collection_id = response.json()['collections'][0]['id'] - response = requests.get("/".join([self.apiUrlBase, f"dp/v1/collections/{collection_id}"])) + collection_id = response.json()["collections"][0]["id"] + response = requests.get("/".join([self.api_url_base, f"dp/v1/collections/{collection_id}"])) response.raise_for_status() - dataset_id = response.json()['datasets'][0]['id'] - response = requests.head('/'.join([self.apiUrlBase, 'cellxgene/e', f"{dataset_id}.cxg", 'api/v0.3/dataset-metadata'])) + self.dataset_id = response.json()["datasets"][0]["id"] + self.explorer_api_url_base = "/".join([self.api_url_base, "cellxgene", "e", f"{self.dataset_id}.cxg"]) + self.cloudfront_miss = "Miss from cloudfront" + self.cloudfront_hit = "Hit from cloudfront" + + @staticmethod + def generate_error_msg(response, message): + return json.dumps( + {"msg": message, "x-cache": response.headers["x-cache"], "x-amz-cf-id": response.headers["x-amz-cf-id"]} + ) + + def test_uncached_endpoints(self): + """Verify the cache headers are properly set for uncached endpoints""" + error_message = "Miss Expected" + endpoints = ["api/v0.3/dataset-metadata", "api/v0.3/s3_uri"] + + def verify_cache_control(_response): + _response.raise_for_status() + cache_control = _response.headers["cache-control"] + self.assertIn("no-store", cache_control) + self.assertIn("max-age=0", cache_control) + self.assertIn("public", cache_control) + + for endpoint in endpoints: + with self.subTest(endpoint): + # Test + url = "/".join([self.explorer_api_url_base, endpoint]) + response = requests.head(url) + + # Verify + verify_cache_control(response) + self.assertEqual( + response.headers["x-cache"], + self.cloudfront_miss, + msg=self.generate_error_msg(response, error_message), + ) + + response = requests.head(url) + verify_cache_control(response) + self.assertEqual( + response.headers["x-cache"], + self.cloudfront_miss, + msg=self.generate_error_msg(response, error_message), + ) + + def test_cached_endpoints(self): + """Verify cloudfront caching headers are properly set for cached endpoints""" + + response = requests.get("/".join([self.explorer_api_url_base, "api/v0.3/s3_uri"])) response.raise_for_status() - cache_control = response.headers['cache-control'] - self.assertIn('no-store', cache_control) - self.assertIn('max-age=0', cache_control) - self.assertIn('public', cache_control) + body = response.json() + url_base = "/".join( + [self.api_url_base, "cellxgene", "s3_uri", quote(quote(body, safe=""), safe=""), "api/v0.3"] + ) + endpoints = ["config", "schema", "colors", "genesets", "layout/obs", "annotations/obs", "annotations/var"] + + for endpoint in endpoints: + with self.subTest(endpoint): + # Test + url = "/".join([url_base, endpoint]) + response = requests.head(url) + "https://api.cellxgene.dev.single-cell.czi.technology/cellxgene/e/276f87b0-5ed9-42bb-b751-d6cf85500a99.cxg" + # Verify + response.raise_for_status() + self.assertIn(response.headers["x-cache"], [self.cloudfront_miss, self.cloudfront_hit]) + response = requests.head(url) + response.raise_for_status() + self.assertEqual( + response.headers["x-cache"], + self.cloudfront_hit, + msg=self.generate_error_msg(response, "Hit Expected"), + ) From 8ca2de240021f9b08e15d6d7eaaa222e81901c46 Mon Sep 17 00:00:00 2001 From: Trent Smith Date: Thu, 9 Dec 2021 14:32:08 -0800 Subject: [PATCH 5/5] add comments --- server/tests/e2e/test_api_cache.py | 4 +++- server/tests/unit/common/apis/test_api_v2.py | 3 +-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/server/tests/e2e/test_api_cache.py b/server/tests/e2e/test_api_cache.py index 630bec034..feecd2f4e 100644 --- a/server/tests/e2e/test_api_cache.py +++ b/server/tests/e2e/test_api_cache.py @@ -54,6 +54,7 @@ def verify_cache_control(_response): msg=self.generate_error_msg(response, error_message), ) + # Call the request twice to make sure the cache wasn't cold. response = requests.head(url) verify_cache_control(response) self.assertEqual( @@ -78,11 +79,12 @@ def test_cached_endpoints(self): # Test url = "/".join([url_base, endpoint]) response = requests.head(url) - "https://api.cellxgene.dev.single-cell.czi.technology/cellxgene/e/276f87b0-5ed9-42bb-b751-d6cf85500a99.cxg" + # Verify response.raise_for_status() self.assertIn(response.headers["x-cache"], [self.cloudfront_miss, self.cloudfront_hit]) + # Call the request twice to make sure the cache wasn't cold. response = requests.head(url) response.raise_for_status() self.assertEqual( diff --git a/server/tests/unit/common/apis/test_api_v2.py b/server/tests/unit/common/apis/test_api_v2.py index 1787d040c..07922714c 100644 --- a/server/tests/unit/common/apis/test_api_v2.py +++ b/server/tests/unit/common/apis/test_api_v2.py @@ -762,10 +762,9 @@ def setUpClass(cls): def verify_response(self, result): self.assertEqual(result.status_code, HTTPStatus.OK) self.assertEqual(result.headers["Content-Type"], "application/json") - self.assertTrue(result.cache_control.public) self.assertTrue(result.cache_control.no_store) self.assertEqual(result.cache_control.max_age, 0) - + @patch("server.data_common.dataset_metadata.request_dataset_metadata_from_data_portal") @patch("server.data_common.dataset_metadata.requests.get") def test_dataset_metadata_api_called_for_public_collection(self, mock_get, mock_dp):