Consolidate and fix item render params (#43) (#45)

* Consolidate and fix item render params
* Update changelog

CHANGELOG.md

@@ -13,6 +13,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Fixed
 
+
+## [2022.1.3]
+
+### Fixed
+
+- Fixes and tests for item render links [#43](https://github.com/microsoft/planetary-computer-apis/pull/43)
+
 ## [2022.1.2]
 
 ### Fixed

pccommon/render.py

@@ -29,17 +29,33 @@ class DefaultRenderConfig:
     requires_token: bool = False
     hidden: bool = False  # Hide from API
 
-    def get_assets_params(self) -> str:
-        if self.assets is None:
-            return ""
+    def get_full_render_qs(self, collection: str, item: Optional[str] = None) -> str:
+        """
+        Return the full render query string, including the
+        item, collection, render and assets parameters.
+        """
+        collection_part = f"collection={collection}" if collection else ""
+        item_part = f"&item={item}" if item else ""
+        asset_part = self.get_assets_params()
+        render_part = self.get_render_params()
+
+        return "".join([collection_part, item_part, asset_part, render_part])
 
-        if len(self.assets) == 1:
-            return f"&assets={self.assets[0]}"
+    def get_assets_params(self) -> str:
+        """
+        Convert listed assets to a query string format with multiple `asset` keys
+            None -> ""
+            [data1] -> "&asset=data1"
+            [data1, data2] -> "&asset=data1&asset=data2"
+        """
+        assets = self.assets or []
+        keys = ["&assets="] * len(assets)
+        params = ["".join(item) for item in zip(keys, assets)]
 
-        return "&assets=".join(self.assets)
+        return "".join(params)
 
     def get_render_params(self) -> str:
-        return get_param_str(self.render_params)
+        return f"&{get_param_str(self.render_params)}"
 
     @property
     def should_add_collection_links(self) -> bool:

pccommon/tests/test_render.py

@@ -0,0 +1,56 @@
+import unittest
+from urllib.parse import quote_plus
+
+from ..render import DefaultRenderConfig
+
+multi_assets = DefaultRenderConfig(
+    assets=["data1", "data2"],
+    render_params={"colormap_name": "terrain", "rescale": [-1000, 4000]},
+    minzoom=8,
+)
+
+single_asset = DefaultRenderConfig(
+    assets=["data1"],
+    render_params={"colormap_name": "terrain", "rescale": [-1000, 4000]},
+    minzoom=8,
+)
+
+no_assets = DefaultRenderConfig(
+    render_params={
+        "expression": ("asset1," "0.45*asset2," "asset3/asset1"),
+        "colormap_name": "terrain",
+        "rescale": [-1000, 4000],
+    },
+    minzoom=8,
+)
+
+
+class TestRenderParams(unittest.TestCase):
+    def test_multi_asset(self) -> None:
+        qs = multi_assets.get_full_render_qs("my_collection_id", "my_item_id")
+        self.assertEqual(
+            qs,
+            "collection=my_collection_id&item=my_item_id&assets=data1&assets=data2&colormap_name=terrain&rescale=-1000,4000",
+        )
+
+    def test_single_asset(self) -> None:
+        qs = single_asset.get_full_render_qs("my_collection_id", "my_item_id")
+        self.assertEqual(
+            qs,
+            "collection=my_collection_id&item=my_item_id&assets=data1&colormap_name=terrain&rescale=-1000,4000",
+        )
+
+    def test_no_asset(self) -> None:
+        qs = no_assets.get_full_render_qs("my_collection_id", "my_item_id")
+        encoded_params = quote_plus("asset1,0.45*asset2,asset3/asset1")
+        self.assertEqual(
+            qs,
+            f"collection=my_collection_id&item=my_item_id&expression={encoded_params}&colormap_name=terrain&rescale=-1000,4000",
+        )
+
+    def test_collection_only(self) -> None:
+        qs = single_asset.get_full_render_qs("my_collection_id")
+        self.assertEqual(
+            qs,
+            "collection=my_collection_id&assets=data1&colormap_name=terrain&rescale=-1000,4000",
+        )

pcstac/pcstac/tiles.py

@@ -51,17 +51,8 @@ def inject_item(self, item: Item) -> None:
         assets["rendered_preview"] = self._get_item_preview_asset(item_id)
 
     def _get_collection_tilejson_asset(self) -> Dict[str, Any]:
-        render_params = self.render_config.get_render_params()
-        render_params_part = f"&{render_params}" if render_params else ""
-        href = urljoin(
-            self.tiler_href,
-            (
-                "collection/tilejson.json?"
-                f"collection={self.collection_id}"
-                f"{self.render_config.get_assets_params()}"
-                f"{render_params_part}"
-            ),
-        )
+        qs = self.render_config.get_full_render_qs(self.collection_id)
+        href = urljoin(self.tiler_href, f"collection/tilejson.json?{qs}")
 
         return {
             "title": "Mosaic TileJSON with default rendering",
@@ -84,18 +75,8 @@ def _get_collection_map_link(self) -> Dict[str, Any]:
         }
 
     def _get_item_preview_asset(self, item_id: str) -> Dict[str, Any]:
-        render_params = self.render_config.get_render_params()
-        render_params_part = f"&{render_params}" if render_params else ""
-        href = urljoin(
-            self.tiler_href,
-            (
-                f"item/preview.png?"
-                f"collection={self.collection_id}"
-                f"&item={item_id}"
-                f"{self.render_config.get_assets_params()}"
-                f"{render_params_part}"
-            ),
-        )
+        qs = self.render_config.get_full_render_qs(self.collection_id, item_id)
+        href = urljoin(self.tiler_href, f"item/preview.png?{qs}")
 
         return {
             "title": "Rendered preview",
@@ -106,18 +87,8 @@ def _get_item_preview_asset(self, item_id: str) -> Dict[str, Any]:
         }
 
     def _get_item_tilejson_asset(self, item_id: str) -> Dict[str, Any]:
-        render_params = self.render_config.get_render_params()
-        render_params_part = f"&{render_params}" if render_params else ""
-        href = urljoin(
-            self.tiler_href,
-            (
-                "item/tilejson.json?"
-                f"collection={self.collection_id}"
-                f"&item={item_id}"
-                f"{self.render_config.get_assets_params()}"
-                f"{render_params_part}"
-            ),
-        )
+        qs = self.render_config.get_full_render_qs(self.collection_id, item_id)
+        href = urljoin(self.tiler_href, f"item/tilejson.json?{qs}")
 
         return {
             "title": "TileJSON with default rendering",

pctiler/pctiler/endpoints/item.py

@@ -7,7 +7,6 @@
 from titiler.core.factory import MultiBaseTilerFactory
 
 from pccommon.render import COLLECTION_RENDER_CONFIG
-from pccommon.utils import get_param_str
 from pctiler.colormaps import PCColorMapParams
 from pctiler.config import get_settings
 from pctiler.reader import ItemSTACReader
@@ -51,19 +50,9 @@ def map(
             content=f"No item map available for collection {collection}",
         )
 
-    tilejson_params = (
-        get_param_str(
-            {
-                "collection": collection,
-                "item": item,
-            }
-        )
-        + render_config.get_assets_params()
-        + f"&{render_config.get_render_params()}"
-    )
-
+    qs = render_config.get_full_render_qs(collection, item)
     tilejson_url = pc_tile_factory.url_for(request, "tilejson")
-    tilejson_url += f"?{tilejson_params}"
+    tilejson_url += f"?{qs}"
 
     item_url = urljoin(
         get_settings().stac_api_href,