Merge pull request #979 from RS-PYTHON/fix/rspy542-invalid-service-desc-link

fix-rspy542-invalid-service-desc-link

Author: alinbutu

services/catalog/rs_server_catalog/user_catalog.py

@@ -95,6 +95,7 @@
     "coordinates": [[[-180, -90], [180, -90], [180, 90], [-180, 90], [-180, -90]]],
 }
 DEFAULT_BBOX = (-180.0, -90.0, 180.0, 90.0)
+QUERYABLES = "/queryables"
 # pylint: disable=too-many-lines
 logger = Logging.default(__name__)
 
@@ -961,7 +962,7 @@ async def manage_get_response(  # pylint: disable=too-many-locals, too-many-bran
             )
             # I don't know why but the STAC browser doesn't send authentication for the queryables endpoint.
             # So allow this endpoint without authentication in this specific case.
-            and not (common_settings.request_from_stacbrowser(request) and request.url.path.endswith("/queryables"))
+            and not (common_settings.request_from_stacbrowser(request) and request.url.path.endswith(QUERYABLES))
         ):
             detail = {"error": "Unauthorized access."}
             return JSONResponse(content=detail, status_code=HTTP_401_UNAUTHORIZED)
@@ -1344,6 +1345,25 @@ async def dispatch(
             response_content = json.loads(b"".join(body).decode())  # type:ignore
             self.clear_catalog_bucket(response_content)
 
+            # GET: '/catalog/queryables' when no collections in the catalog
+            if (
+                request.method == "GET"
+                and request.scope["path"] == QUERYABLES
+                and not self.request_ids["collection_ids"]
+                and response_content["code"] == "NotFoundError"
+            ):
+                # Return empty list of properties and additionalProperties set to true on /catalog/queryables
+                # when there are no collections in catalog.
+                empty_catalog_queryables_response = {
+                    "$id": f"{request.base_url}queryables",
+                    "type": "object",
+                    "title": "STAC Queryables.",
+                    "$schema": "https://json-schema.org/draft-07/schema#",
+                    "properties": {},
+                    "additionalProperties": True,
+                }
+                return JSONResponse(status_code=HTTP_200_OK, content=empty_catalog_queryables_response)
+
             # Return a regular JSON response instead of StreamingResponse because the body cannot be read again.
             return JSONResponse(status_code=response.status_code, content=response_content)
 
@@ -1356,7 +1376,7 @@ async def dispatch(
             response = await self.manage_download_response(request, response)
 
         elif request.method == "GET" and (
-            self.request_ids["owner_id"] or request.scope["path"] in ["/", "/collections", "/queryables"]
+            self.request_ids["owner_id"] or request.scope["path"] in ["/", "/collections", QUERYABLES]
         ):
             # URL: GET: '/catalog/collections/{USER}:{COLLECTION}'
             # URL: GET: '/catalog/'

services/catalog/rs_server_catalog/user_handler.py

@@ -34,6 +34,7 @@
 COLLECTIONS_SEARCH_REGEX = r"/catalog/collections/((?P<owner_id>.+):)?(?P<collection_id>.+)/search"
 BULK_ITEMS_REGEX = r"/catalog/collections/((?P<owner_id>.+):)?(?P<collection_id>.+)/bulk_items"
 CATALOG_COLLECTION = "/catalog/collections"
+CATALOG_PREFIX = "/catalog"
 
 # Regexp for search endpoints
 CATALOG_SEARCH = "/catalog/search"
@@ -83,19 +84,19 @@ def reroute_url(  # type: ignore # pylint: disable=too-many-branches,too-many-st
     # Catch one endpoint of the following list
     regexp_list = [
         "/",
+        "/catalog",
         "/catalog/",
         "/catalog/search",
         "/catalog/queryables",
-        "catalog/api",
+        "/catalog/api",
         "/catalog/api.html",
         "/catalog/docs/oauth2-redirect",
-        "/catalog/queryables",
         "/catalog/conformance",
     ]
     is_in_regexp_list = False
     for pattern in regexp_list:
         if re.fullmatch(pattern, path):
-            path = path.replace("/catalog", "") if path != "/" else path
+            path = path.replace(CATALOG_PREFIX, "") or "/"
             is_in_regexp_list = True
             break
     if is_in_regexp_list:
@@ -134,7 +135,7 @@ def reroute_url(  # type: ignore # pylint: disable=too-many-branches,too-many-st
         ids_dict["owner_id"] = get_user(groups["owner_id"], ids_dict["user_login"])
         ids_dict["collection_ids"].append(f"{ids_dict['owner_id']}_{groups['collection_id']}")
         # This endpoint will be redirected to the "/search" endpoint later in the code
-        path = path.replace("/catalog", "")
+        path = path.replace(CATALOG_PREFIX, "")
 
     # Catch all other endpoints.
     elif match := re.match(CATALOG_OWNER_ID_STAC_ENDPOINT_REGEX, path):

services/catalog/tests/conftest.py

@@ -113,6 +113,20 @@ def client_fixture(start_database):  # pylint: disable=missing-function-docstrin
         yield client
 
 
+@pytest.mark.integration
+@pytest.fixture(scope="session", name="client_with_empty_catalog")
+def client_empty_catalog_fixture(start_database):  # pylint: disable=missing-function-docstring, unused-argument
+    """Client with an empty catalog (no collections added)."""
+    with TestClient(app, follow_redirects=False) as client:
+        # Ensure the catalog is empty by deleting all collections
+        response = client.get("/catalog/collections")
+        if response.status_code == 200:
+            for collection in response.json().get("collections", []):
+                collection_id = collection["id"].replace("_", ":", 1)
+                client.delete(f"/catalog/collections/{collection_id}")
+        yield client  # Does NOT trigger setup_database!
+
+
 @dataclass
 class Collection:
     """A collection for test purpose."""
@@ -490,6 +504,7 @@ def setup_database(
         feature_titi_S2_L1_0 (_type_): a feature from the collection S2_L1 with the
         user id titi.
     """
+
     add_collection(client, toto_s1_l1)
     add_collection(client, toto_s2_l3)
     add_collection(client, titi_s2_l1)

services/catalog/tests/test_endpoints.py

@@ -1725,3 +1725,21 @@ def test_catalog_catalogs_owner_id_is_disabled(client):
 
     response = client.get("/catalog/catalogs/toto")
     assert response.status_code == fastapi.status.HTTP_400_BAD_REQUEST
+
+
+def test_queryables_with_empty_catalog(client_with_empty_catalog):
+    """
+    Test Queryables feature endpoint when catalog has no collections in it
+    """
+    response_empty = client_with_empty_catalog.get("/catalog/queryables")
+
+    assert response_empty.status_code == fastapi.status.HTTP_200_OK
+    expected_response = {
+        "$id": f"{client_with_empty_catalog.base_url}/queryables",
+        "type": "object",
+        "title": "STAC Queryables.",
+        "$schema": "https://json-schema.org/draft-07/schema#",
+        "properties": {},
+        "additionalProperties": True,
+    }
+    assert response_empty.json() == expected_response  # JSON Content Check

services/catalog/tests/test_user_handler.py

@@ -120,8 +120,8 @@ def test_get_user():
     assert get_user(None, None) == getpass.getuser()
 
 
-class TestRemovePrefix:  # pylint: disable=missing-function-docstring
-    """This Class contains unit tests for the function remove_user_prefix."""
+class TestRerouteURL:  # pylint: disable=missing-function-docstring
+    """This Class contains unit tests for the function reroute_url."""
 
     def test_root_url(self, request_ids):
         request = Request(
@@ -220,6 +220,43 @@ def test_reroute_oauth2(self, request_ids):
         reroute_url(request, request_ids)
         assert request.scope["path"] == "/docs/oauth2-redirect"
 
+    @pytest.mark.parametrize(
+        "path, expected",
+        [
+            ("/catalog/", "/"),
+            ("/catalog", "/"),
+        ],
+    )
+    def test_reroute_catalog(self, request_ids, path, expected):
+        """Test that reroute_url modifies "/catalog" and "/catalog/" to the root URL ("/")."""
+        request = Request(
+            scope={
+                "type": "http",
+                "method": "GET",
+                "path": path,
+                "query_string": "",
+                "user": "",
+                "headers": {},
+            },
+        )
+        reroute_url(request, request_ids)
+        assert request.scope["path"] == expected
+
+    def test_reroute_catalog_api(self, request_ids):
+        """Test that reroute_url modifies "/catalog/api" to the "/api"."""
+        request = Request(
+            scope={
+                "type": "http",
+                "method": "GET",
+                "path": "/catalog/api",
+                "query_string": "",
+                "user": "",
+                "headers": {},
+            },
+        )
+        reroute_url(request, request_ids)
+        assert request.scope["path"] == "/api"
+
     def test_reroute_queryables(self, request_ids):
         request = Request(
             scope={
@@ -234,6 +271,28 @@ def test_reroute_queryables(self, request_ids):
         reroute_url(request, request_ids)
         assert request.scope["path"] == "/queryables"
 
+    @pytest.mark.parametrize(
+        "path, expected",
+        [
+            ("/whatever-test/health", "/health"),
+            ("/health", "/health"),
+        ],
+    )
+    def test_reroute_health(self, request_ids, path, expected):
+        """Test that reroute_url catch health endpoints "/health"."""
+        request = Request(
+            scope={
+                "type": "http",
+                "method": "GET",
+                "path": path,
+                "query_string": "",
+                "user": "",
+                "headers": {},
+            },
+        )
+        reroute_url(request, request_ids)
+        assert request.scope["path"] == expected
+
     def test_search_collection(self, request_ids):
         request = Request(
             scope={