Repository: planetlabs/planet-client-python

Issue:
Implement CLI command: planet data search-run
As specified in https://github.com/planetlabs/planet-client-python/blob/main/design-docs/CLI-Data.md#search-run

Discussion:

Patch:
diff --git a/planet/cli/data.py b/planet/cli/data.py
index 1f3e139..a232ede 100644
--- a/planet/cli/data.py
+++ b/planet/cli/data.py
@@ -17,8 +17,12 @@ from contextlib import asynccontextmanager
 
 import click
 
-from planet import data_filter, DataClient
-from planet.clients.data import (SEARCH_SORT,
+from planet import data_filter, DataClient, exceptions
+from planet.clients.data import (LIST_SEARCH_TYPE,
+                                 LIST_SEARCH_TYPE_DEFAULT,
+                                 LIST_SORT_ORDER,
+                                 LIST_SORT_DEFAULT,
+                                 SEARCH_SORT,
                                  SEARCH_SORT_DEFAULT,
                                  STATS_INTERVAL)
 from planet.specs import (get_item_types,
@@ -74,6 +78,15 @@ def check_item_types(ctx, param, item_types) -> Optional[List[dict]]:
         raise click.BadParameter(str(e))
 
 
+def check_search_id(ctx, param, search_id) -> str:
+    '''Ensure search id is a valix hex string'''
+    try:
+        _ = DataClient._check_search_id(search_id)
+    except exceptions.ClientError as e:
+        raise click.BadParameter(str(e))
+    return search_id
+
+
 def date_range_to_filter(ctx, param, values) -> Optional[List[dict]]:
 
     def _func(obj):
@@ -324,6 +337,62 @@ async def search_create(ctx, name, item_types, filter, daily_email, pretty):
         echo_json(items, pretty)
 
 
+@data.command()
+@click.pass_context
+@translate_exceptions
+@coro
+@click.option('--sort',
+              type=click.Choice(LIST_SORT_ORDER),
+              default=LIST_SORT_DEFAULT,
+              show_default=True,
+              help='Field and direction to order results by.')
+@click.option('--search-type',
+              type=click.Choice(LIST_SEARCH_TYPE),
+              default=LIST_SEARCH_TYPE_DEFAULT,
+              show_default=True,
+              help='Search type filter.')
+@limit
+@pretty
+async def search_list(ctx, sort, search_type, limit, pretty):
+    """List saved searches.
+
+    This function outputs a full JSON description of the saved searches,
+    optionally pretty-printed.
+    """
+    async with data_client(ctx) as cl:
+        async for item in cl.list_searches(sort=sort,
+                                           search_type=search_type,
+                                           limit=limit):
+            echo_json(item, pretty)
+
+
+@data.command()
+@click.pass_context
+@translate_exceptions
+@coro
+@click.argument('search_id', callback=check_search_id)
+@click.option('--sort',
+              type=click.Choice(SEARCH_SORT),
+              default=SEARCH_SORT_DEFAULT,
+              show_default=True,
+              help='Field and direction to order results by.')
+@limit
+@pretty
+async def search_run(ctx, search_id, sort, limit, pretty):
+    """Execute a saved structured item search.
+
+    This function outputs a series of GeoJSON descriptions, one for each of the
+    returned items, optionally pretty-printed.
+    """
+    async with data_client(ctx) as cl:
+        async for item in cl.run_search(search_id, sort=sort, limit=limit):
+            echo_json(item, pretty)
+
+
+# TODO: search-update
+# TODO: search-delete
+
+
 @data.command(epilog=valid_item_string)
 @click.pass_context
 @translate_exceptions
diff --git a/planet/clients/data.py b/planet/clients/data.py
index 4e0f52c..82c22c2 100644
--- a/planet/clients/data.py
+++ b/planet/clients/data.py
@@ -18,6 +18,7 @@ import logging
 from pathlib import Path
 import time
 from typing import Any, AsyncIterator, Callable, Dict, List, Optional
+import uuid
 
 from ..data_filter import empty_filter
 from .. import exceptions
@@ -30,8 +31,13 @@ SEARCHES_PATH = '/searches'
 STATS_PATH = '/stats'
 
 # TODO: get these values from the spec directly gh-619
+# NOTE: these values are mached against a lower-case value so must also be
+# lower-case
 LIST_SORT_ORDER = ('created desc', 'created asc')
 LIST_SEARCH_TYPE = ('any', 'saved', 'quick')
+LIST_SORT_DEFAULT = 'created desc'
+LIST_SEARCH_TYPE_DEFAULT = 'any'
+
 SEARCH_SORT = ('published desc',
                'published asc',
                'acquired desc',
@@ -88,6 +94,15 @@ class DataClient:
         if self._base_url.endswith('/'):
             self._base_url = self._base_url[:-1]
 
+    @staticmethod
+    def _check_search_id(sid):
+        """Raises planet.exceptions.ClientError if sid is not a valid UUID"""
+        try:
+            uuid.UUID(hex=sid)
+        except (ValueError, AttributeError):
+            msg = f'Search id ({sid}) is not a valid UUID hexadecimal string.'
+            raise exceptions.ClientError(msg)
+
     def _searches_url(self):
         return f'{self._base_url}{SEARCHES_PATH}'
 
@@ -234,8 +249,8 @@ class DataClient:
         return response.json()
 
     async def list_searches(self,
-                            sort: str = 'created desc',
-                            search_type: str = 'any',
+                            sort: str = LIST_SORT_DEFAULT,
+                            search_type: str = LIST_SEARCH_TYPE_DEFAULT,
                             limit: int = 100) -> AsyncIterator[dict]:
         """Iterate through list of searches available to the user.
 
@@ -246,7 +261,7 @@ class DataClient:
 
         Parameters:
             sort: Field and direction to order results by.
-            search_type: Search type filter.
+            search_type: Filter to specified search type.
             limit: Maximum number of results to return. When set to 0, no
                 maximum is applied.
 
@@ -258,19 +273,26 @@ class DataClient:
             planet.exceptions.ClientError: If sort or search_type are not
                 valid.
         """
+        params = {}
+
         sort = sort.lower()
         if sort not in LIST_SORT_ORDER:
             raise exceptions.ClientError(
                 f'{sort} must be one of {LIST_SORT_ORDER}')
+        elif sort != LIST_SORT_DEFAULT:
+            params['_sort'] = sort
 
         search_type = search_type.lower()
         if search_type not in LIST_SEARCH_TYPE:
             raise exceptions.ClientError(
                 f'{search_type} must be one of {LIST_SEARCH_TYPE}')
+        elif search_type != LIST_SEARCH_TYPE_DEFAULT:
+            params['search_type'] = search_type
 
-        url = f'{self._searches_url()}'
-
-        response = await self._session.request(method='GET', url=url)
+        url = self._searches_url()
+        response = await self._session.request(method='GET',
+                                               url=url,
+                                               params=params)
         async for s in Searches(response, self._session.request, limit=limit):
             yield s
 
@@ -306,6 +328,7 @@ class DataClient:
 
     async def run_search(self,
                          search_id: str,
+                         sort: Optional[str] = None,
                          limit: int = 100) -> AsyncIterator[dict]:
         """Iterate over results from a saved search.
 
@@ -316,6 +339,8 @@ class DataClient:
 
         Parameters:
             search_id: Stored search identifier.
+            sort: Field and direction to order results by. Valid options are
+            given in SEARCH_SORT.
             limit: Maximum number of results to return. When set to 0, no
                 maximum is applied.
 
@@ -324,10 +349,22 @@ class DataClient:
 
         Raises:
             planet.exceptions.APIError: On API error.
+            planet.exceptions.ClientError: If search_id or sort is not valid.
         """
+        self._check_search_id(search_id)
         url = f'{self._searches_url()}/{search_id}/results'
 
-        response = await self._session.request(method='GET', url=url)
+        params = {}
+        if sort and sort != SEARCH_SORT_DEFAULT:
+            sort = sort.lower()
+            if sort not in SEARCH_SORT:
+                raise exceptions.ClientError(
+                    f'{sort} must be one of {SEARCH_SORT}')
+            params['_sort'] = sort
+
+        response = await self._session.request(method='GET',
+                                               url=url,
+                                               params=params)
         async for i in Items(response, self._session.request, limit=limit):
             yield i
 


Test patch:
diff --git a/tests/integration/test_data_api.py b/tests/integration/test_data_api.py
index 3856e4d..736d751 100644
--- a/tests/integration/test_data_api.py
+++ b/tests/integration/test_data_api.py
@@ -25,11 +25,16 @@ import pytest
 import respx
 
 from planet import exceptions, DataClient, data_filter
+from planet.clients.data import (LIST_SORT_DEFAULT,
+                                 LIST_SEARCH_TYPE_DEFAULT,
+                                 SEARCH_SORT_DEFAULT)
 
-TEST_URL = 'http://www.MockNotRealURL.com/api/path'
+TEST_URL = 'http://www.mocknotrealurl.com/api/path'
 TEST_SEARCHES_URL = f'{TEST_URL}/searches'
 TEST_STATS_URL = f'{TEST_URL}/stats'
 
+VALID_SEARCH_ID = '286469f0b27c476e96c3c4e561f59664'
+
 LOGGER = logging.getLogger(__name__)
 
 
@@ -325,7 +330,6 @@ async def test_get_search_id_doesnt_exist(search_id, session):
 @respx.mock
 @pytest.mark.asyncio
 async def test_update_search_basic(search_filter, session):
-    sid = 'search_id'
 
     page_response = {
         "__daily_email_enabled": False,
@@ -334,16 +338,19 @@ async def test_update_search_basic(search_filter, session):
         },
         "created": "2019-08-24T14:15:22Z",
         "filter": search_filter,
-        "id": sid,
+        "id": VALID_SEARCH_ID,
         "last_executed": "2019-08-24T14:15:22Z",
         "name": "test",
         "updated": "2019-08-24T14:15:22Z"
     }
     mock_resp = httpx.Response(HTTPStatus.OK, json=page_response)
-    respx.put(f'{TEST_SEARCHES_URL}/{sid}').return_value = mock_resp
+    respx.put(
+        f'{TEST_SEARCHES_URL}/{VALID_SEARCH_ID}').return_value = mock_resp
 
     cl = DataClient(session, base_url=TEST_URL)
-    search = await cl.update_search(sid, 'test', ['PSScene'], search_filter)
+    search = await cl.update_search(VALID_SEARCH_ID,
+                                    'test', ['PSScene'],
+                                    search_filter)
 
     # check that request is correct
     expected_request = {
@@ -378,6 +385,41 @@ async def test_list_searches_success(limit,
     assert route.called
 
 
+@respx.mock
+@pytest.mark.asyncio
+@pytest.mark.parametrize("sort, rel_url",
+                         [(LIST_SORT_DEFAULT, ''),
+                          ('created asc', '?_sort=created+asc')])
+async def test_list_searches_sort(sort, rel_url, search_result, session):
+    page1_response = {"_links": {}, "searches": [search_result] * 4}
+    route = respx.get(f'{TEST_SEARCHES_URL}{rel_url}')
+    route.return_value = httpx.Response(200, json=page1_response)
+
+    cl = DataClient(session, base_url=TEST_URL)
+    _ = [s async for s in cl.list_searches(sort=sort)]
+
+    assert route.called
+
+
+@respx.mock
+@pytest.mark.asyncio
+@pytest.mark.parametrize("search_type, rel_url",
+                         [(LIST_SEARCH_TYPE_DEFAULT, ''),
+                          ('saved', '?search_type=saved')])
+async def test_list_searches_searchtype(search_type,
+                                        rel_url,
+                                        search_result,
+                                        session):
+    page1_response = {"_links": {}, "searches": [search_result] * 4}
+    route = respx.get(f'{TEST_SEARCHES_URL}{rel_url}')
+    route.return_value = httpx.Response(200, json=page1_response)
+
+    cl = DataClient(session, base_url=TEST_URL)
+    _ = [s async for s in cl.list_searches(search_type=search_type)]
+
+    assert route.called
+
+
 @respx.mock
 @pytest.mark.asyncio
 @pytest.mark.parametrize(
@@ -405,24 +447,69 @@ async def test_list_searches_args_do_not_match(sort,
                          [(204, does_not_raise()),
                           (404, pytest.raises(exceptions.APIError))])
 async def test_delete_search(retcode, expectation, session):
-    sid = 'search_id'
     mock_resp = httpx.Response(retcode)
-    route = respx.delete(f'{TEST_SEARCHES_URL}/{sid}')
+    route = respx.delete(f'{TEST_SEARCHES_URL}/{VALID_SEARCH_ID}')
     route.return_value = mock_resp
     cl = DataClient(session, base_url=TEST_URL)
 
     with expectation:
-        await cl.delete_search(sid)
+        await cl.delete_search(VALID_SEARCH_ID)
 
     assert route.called
 
 
 @respx.mock
 @pytest.mark.asyncio
-async def test_run_search_success(item_descriptions, session):
-    sid = 'search_id'
-    route = respx.get(f'{TEST_SEARCHES_URL}/{sid}/results')
+@pytest.mark.parametrize("search_id, valid", [(VALID_SEARCH_ID, True),
+                                              ('invalid', False)])
+@pytest.mark.parametrize("limit, expected_count", [(None, 3), (2, 2)])
+async def test_run_search_basic(item_descriptions,
+                                session,
+                                search_id,
+                                valid,
+                                limit,
+                                expected_count):
+    """Ensure run_search is successful and handles search_id and limit"""
+    next_page_url = f'{TEST_URL}/blob/?page_marker=IAmATest'
+    item1, item2, item3 = item_descriptions
+    page1_response = {
+        "_links": {
+            "_next": next_page_url
+        }, "features": [item1, item2]
+    }
+
+    route = respx.get(f'{TEST_SEARCHES_URL}/{search_id}/results')
+    route.return_value = httpx.Response(204, json=page1_response)
+
+    page2_response = {"_links": {"_self": next_page_url}, "features": [item3]}
+    mock_resp2 = httpx.Response(HTTPStatus.OK, json=page2_response)
+    respx.get(next_page_url).return_value = mock_resp2
+
+    cl = DataClient(session, base_url=TEST_URL)
 
+    if valid:
+        items_list = [i async for i in cl.run_search(search_id, limit=limit)]
+
+        assert route.called
+
+        # check that all of the items were returned unchanged
+        assert items_list == item_descriptions[:expected_count]
+    else:
+        with pytest.raises(exceptions.ClientError):
+            [i async for i in cl.run_search(search_id)]
+
+
+@respx.mock
+@pytest.mark.asyncio
+@pytest.mark.parametrize("sort, rel_url, valid",
+                         [(SEARCH_SORT_DEFAULT, '', True),
+                          ('acquired asc', '?_sort=acquired+asc', True),
+                          ('invalid', '', False)])
+async def test_run_search_sort(item_descriptions,
+                               session,
+                               sort,
+                               rel_url,
+                               valid):
     next_page_url = f'{TEST_URL}/blob/?page_marker=IAmATest'
     item1, item2, item3 = item_descriptions
     page1_response = {
@@ -431,6 +518,8 @@ async def test_run_search_success(item_descriptions, session):
         }, "features": [item1, item2]
     }
 
+    route = respx.get(
+        f'{TEST_SEARCHES_URL}/{VALID_SEARCH_ID}/results{rel_url}')
     route.return_value = httpx.Response(204, json=page1_response)
 
     page2_response = {"_links": {"_self": next_page_url}, "features": [item3]}
@@ -438,24 +527,24 @@ async def test_run_search_success(item_descriptions, session):
     respx.get(next_page_url).return_value = mock_resp2
 
     cl = DataClient(session, base_url=TEST_URL)
-    items_list = [i async for i in cl.run_search(sid)]
 
-    assert route.called
+    expectation = pytest.raises(
+        exceptions.ClientError) if not valid else does_not_raise()
 
-    # check that all of the items were returned unchanged
-    assert items_list == item_descriptions
+    with expectation:
+        [s async for s in cl.run_search(VALID_SEARCH_ID, sort=sort)]
+        assert route.called
 
 
 @respx.mock
 @pytest.mark.asyncio
 async def test_run_search_doesnotexist(session):
-    sid = 'search_id'
-    route = respx.get(f'{TEST_SEARCHES_URL}/{sid}/results')
+    route = respx.get(f'{TEST_SEARCHES_URL}/{VALID_SEARCH_ID}/results')
     route.return_value = httpx.Response(404)
 
     cl = DataClient(session, base_url=TEST_URL)
     with pytest.raises(exceptions.APIError):
-        [i async for i in cl.run_search(sid)]
+        [i async for i in cl.run_search(VALID_SEARCH_ID)]
 
     assert route.called
 
diff --git a/tests/integration/test_data_cli.py b/tests/integration/test_data_cli.py
index 54cdb78..c8f37b8 100644
--- a/tests/integration/test_data_cli.py
+++ b/tests/integration/test_data_cli.py
@@ -23,6 +23,9 @@ from click.testing import CliRunner
 import pytest
 
 from planet.cli import cli
+from planet.clients.data import (LIST_SEARCH_TYPE_DEFAULT,
+                                 LIST_SORT_DEFAULT,
+                                 SEARCH_SORT_DEFAULT)
 from planet.specs import get_item_types
 
 LOGGER = logging.getLogger(__name__)
@@ -32,18 +35,31 @@ TEST_QUICKSEARCH_URL = f'{TEST_URL}/quick-search'
 TEST_SEARCHES_URL = f'{TEST_URL}/searches'
 TEST_STATS_URL = f'{TEST_URL}/stats'
 
+VALID_SEARCH_ID = '286469f0b27c476e96c3c4e561f59664'
+
 
 @pytest.fixture
 def invoke():
 
-    def _invoke(extra_args, runner=None):
+    def _invoke(extra_args, runner=None, **kwargs):
         runner = runner or CliRunner()
         args = ['data'] + extra_args
-        return runner.invoke(cli.main, args=args)
+        return runner.invoke(cli.main, args=args, **kwargs)
 
     return _invoke
 
 
+@pytest.fixture
+def item_descriptions(get_test_file_json):
+    item_ids = [
+        '20220125_075509_67_1061',
+        '20220125_075511_17_1061',
+        '20220125_075650_17_1061'
+    ]
+    items = [get_test_file_json(f'data_item_{id}.json') for id in item_ids]
+    return items
+
+
 def test_data_command_registered(invoke):
     """planet-data command prints help and usage message."""
     runner = CliRunner()
@@ -523,7 +539,7 @@ def test_data_search_create_filter_success(invoke, item_types):
 
 
 @respx.mock
-def test_search_create_daily_email(invoke, search_result):
+def test_data_search_create_daily_email(invoke, search_result):
     mock_resp = httpx.Response(HTTPStatus.OK, json=search_result)
     respx.post(TEST_SEARCHES_URL).return_value = mock_resp
 
@@ -561,6 +577,127 @@ def test_search_create_daily_email(invoke, search_result):
     assert json.loads(result.output) == search_result
 
 
+@respx.mock
+@pytest.mark.asyncio
+@pytest.mark.parametrize("limit, expected_list_length", [(0, 4), (3, 3)])
+def test_data_search_list_basic(invoke,
+                                search_result,
+                                limit,
+                                expected_list_length):
+    """Ensure planet data search-list runs successfully and respects limit."""
+    page1_response = {"_links": {}, "searches": [search_result] * 4}
+    route = respx.get(TEST_SEARCHES_URL)
+    route.return_value = httpx.Response(200, json=page1_response)
+
+    result = invoke(['search-list', f'--limit={limit}'])
+    assert result.exit_code == 0
+    assert len(result.output.strip().split('\n')) == expected_list_length
+
+
+@respx.mock
+@pytest.mark.asyncio
+@pytest.mark.parametrize("sort, rel_url, valid",
+                         [(LIST_SORT_DEFAULT, '', True),
+                          ('created asc', '?_sort=created+asc', True),
+                          ('notvalid', '', False)])
+def test_data_search_list_sort(invoke, search_result, sort, rel_url, valid):
+    """Ensure planet data search-list handles sort."""
+    page1_response = {"_links": {}, "searches": [search_result] * 4}
+    route = respx.get(f'{TEST_SEARCHES_URL}{rel_url}')
+    route.return_value = httpx.Response(200, json=page1_response)
+
+    result = invoke(['search-list', f'--sort={sort}'])
+    expected_code = 0 if valid else 2
+    assert result.exit_code == expected_code
+
+
+@respx.mock
+@pytest.mark.asyncio
+@pytest.mark.parametrize("search_type, rel_url, valid",
+                         [(LIST_SEARCH_TYPE_DEFAULT, '', True),
+                          ('saved', '?search_type=saved', True),
+                          ('notvalid', '', False)])
+def test_data_search_list_searchtype(invoke,
+                                     search_result,
+                                     search_type,
+                                     rel_url,
+                                     valid):
+    """Ensure planet data search-list handles search-type."""
+    page1_response = {"_links": {}, "searches": [search_result] * 4}
+    route = respx.get(f'{TEST_SEARCHES_URL}{rel_url}')
+    route.return_value = httpx.Response(200, json=page1_response)
+
+    result = invoke(['search-list', f'--search-type={search_type}'])
+    expected_code = 0 if valid else 2
+    assert result.exit_code == expected_code
+
+
+@respx.mock
+@pytest.mark.asyncio
+@pytest.mark.parametrize("search_id, valid", [(VALID_SEARCH_ID, True),
+                                              ('invalid', False)])
+@pytest.mark.parametrize("limit, expected_count", [(0, 3), (2, 2)])
+def test_data_search_run_basic(invoke,
+                               item_descriptions,
+                               search_id,
+                               valid,
+                               limit,
+                               expected_count):
+    """Ensure planet data search-run runs successfully and handles search id
+    and limit."""
+    next_page_url = f'{TEST_URL}/blob/?page_marker=IAmATest'
+    item1, item2, item3 = item_descriptions
+    page1_response = {
+        "_links": {
+            "_next": next_page_url
+        }, "features": [item1, item2]
+    }
+
+    route = respx.get(f'{TEST_SEARCHES_URL}/{search_id}/results')
+    route.return_value = httpx.Response(204, json=page1_response)
+
+    page2_response = {"_links": {"_self": next_page_url}, "features": [item3]}
+    mock_resp2 = httpx.Response(HTTPStatus.OK, json=page2_response)
+    respx.get(next_page_url).return_value = mock_resp2
+
+    result = invoke(['search-run', f'--limit={limit}', search_id])
+
+    if valid:
+        assert result.exit_code == 0
+        assert len(result.output.strip().split('\n')) == expected_count
+    else:
+        assert result.exit_code == 2
+
+
+@respx.mock
+@pytest.mark.asyncio
+@pytest.mark.parametrize("sort, rel_url, valid",
+                         [(SEARCH_SORT_DEFAULT, '', True),
+                          ('acquired asc', '?_sort=acquired+asc', True),
+                          ('invalid', '', False)])
+def test_data_search_run_sort(invoke, item_descriptions, sort, rel_url, valid):
+    """Ensure planet data search-run handles sort."""
+    next_page_url = f'{TEST_URL}/blob/?page_marker=IAmATest'
+    item1, item2, item3 = item_descriptions
+    page1_response = {
+        "_links": {
+            "_next": next_page_url
+        }, "features": [item1, item2]
+    }
+
+    route = respx.get(
+        f'{TEST_SEARCHES_URL}/{VALID_SEARCH_ID}/results{rel_url}')
+    route.return_value = httpx.Response(204, json=page1_response)
+
+    page2_response = {"_links": {"_self": next_page_url}, "features": [item3]}
+    mock_resp2 = httpx.Response(HTTPStatus.OK, json=page2_response)
+    respx.get(next_page_url).return_value = mock_resp2
+
+    result = invoke(['search-run', f'--sort={sort}', VALID_SEARCH_ID])
+    expected_code = 0 if valid else 2
+    assert result.exit_code == expected_code
+
+
 @respx.mock
 @pytest.mark.asyncio
 @pytest.mark.parametrize("filter", ['{1:1}', '{"foo"}'])