Repository: planetlabs/planet-client-python
Pull request #313 (2021-09-01 19:03:35)

Planet orders download default to not overwrite files
- Added option on whether or not to overwrite data in CLI
- Added overwriting logic in orders module
- Made the files download under a directory named after the order ID, consistent with our docs (https://developers.planet.com/docs/orders/delivery/)
- Updated downloading info echoed back to the user
- Updated an example to work with updated orders module


Patch:
diff --git a/examples/orders_create_and_download_multiple_orders.py b/examples/orders_create_and_download_multiple_orders.py
index e2d958f..50413f4 100644
--- a/examples/orders_create_and_download_multiple_orders.py
+++ b/examples/orders_create_and_download_multiple_orders.py
@@ -83,8 +83,8 @@ async def create_and_download(order_detail, directory, client):
     # download
     print(f'Downloading {oid} to {directory}.')
     filenames = await client.download_order(oid, directory, progress_bar=True)
-    print(f'Downloaded {oid}: '
-          f'{len(filenames)} files downloaded to {directory}.')
+    for f in filenames:
+        print(f)
 
 
 async def main():
diff --git a/planet/clients/orders.py b/planet/clients/orders.py
index 878d2e9..89aa872 100644
--- a/planet/clients/orders.py
+++ b/planet/clients/orders.py
@@ -30,6 +30,7 @@ STATS_PATH = 'stats/orders/v2/'
 ORDERS_PATH = 'orders/v2/'
 BULK_PATH = 'bulk/orders/v2/'
 
+# Order states https://developers.planet.com/docs/orders/ordering/#order-states
 ORDERS_STATES_COMPLETE = ['success', 'partial', 'cancelled', 'failed']
 ORDERS_STATES_IN_PROGRESS = ['queued', 'running']
 ORDERS_STATES = ORDERS_STATES_IN_PROGRESS + ORDERS_STATES_COMPLETE
@@ -278,7 +279,7 @@ class OrdersClient():
         location: str,
         filename: str = None,
         directory: str = None,
-        overwrite: bool = True,
+        overwrite: bool = False,
         progress_bar: bool = True
     ) -> str:
         """Download ordered asset.
@@ -310,7 +311,7 @@ class OrdersClient():
         self,
         order_id: str,
         directory: str = None,
-        overwrite: bool = True,
+        overwrite: bool = False,
         progress_bar: bool = False
     ) -> typing.List[str]:
         """Download all assets in an order.
diff --git a/planet/scripts/cli.py b/planet/scripts/cli.py
index 3065ed1..bb83343 100644
--- a/planet/scripts/cli.py
+++ b/planet/scripts/cli.py
@@ -200,7 +200,7 @@ async def cancel(ctx, order_id):
 @click.argument('order_id', type=click.UUID)
 @click.option('-q', '--quiet', is_flag=True, default=False,
               help=('Disable ANSI control output.'))
-@click.option('-o', '--overwrite', is_flag=True, default=True,
+@click.option('-o', '--overwrite', is_flag=True, default=False,
               help=('Overwrite files if they already exist.'))
 @click.option('--dest', default='.',
               help=('Directory to download files to.'),
@@ -208,6 +208,8 @@ async def cancel(ctx, order_id):
                               writable=True, file_okay=False))
 async def download(ctx, order_id, quiet, overwrite, dest):
     '''Download order by order ID.'''
+
+    # Download the user's order
     async with orders_client(ctx) as cl:
         await cl.poll(str(order_id), verbose=True)
         filenames = await cl.download_order(
@@ -215,7 +217,10 @@ async def download(ctx, order_id, quiet, overwrite, dest):
                 directory=dest,
                 overwrite=overwrite,
                 progress_bar=not quiet)
-    click.echo(f'Downloaded {len(filenames)} files.')
+
+    # Tell user all files that exist in order
+    for f in filenames:
+        click.echo(f)
 
 
 def split_id_list(ctx, param, value):


Test patch:
diff --git a/tests/integration/test_orders_api.py b/tests/integration/test_orders_api.py
index f837e1a..fc52eaa 100644
--- a/tests/integration/test_orders_api.py
+++ b/tests/integration/test_orders_api.py
@@ -19,7 +19,6 @@ import math
 import os
 from pathlib import Path
 
-
 import httpx
 import pytest
 import respx
@@ -49,6 +48,45 @@ def oid2():
     return '5ece1dc0-ea81-11eb-837c-acde48001122'
 
 
+@pytest.fixture
+def downloaded_content():
+    return {'key': 'downloaded_file'}
+
+
+@pytest.fixture
+def original_content():
+    return {'key': 'original_file'}
+
+
+@pytest.fixture
+def create_download_mock(downloaded_content, order_description, oid):
+    '''
+    Mock an HTTP response for download.
+    '''
+
+    def f():
+        # Create mock HTTP response
+        dl_url = TEST_URL + 'download/1?token=IAmAToken'
+        order_description['_links']['results'] = [
+            {'location': dl_url},
+        ]
+
+        get_url = TEST_URL + 'orders/v2/' + oid
+        mock_resp = httpx.Response(HTTPStatus.OK, json=order_description)
+        respx.get(get_url).return_value = mock_resp
+
+        mock_resp = httpx.Response(
+            HTTPStatus.OK,
+            json=downloaded_content,
+            headers={
+                'Content-Type': 'application/json',
+                'Content-Disposition': 'attachment; filename="file.json"'
+            })
+        respx.get(dl_url).return_value = mock_resp
+
+    return f
+
+
 @respx.mock
 @pytest.mark.asyncio
 async def test_list_orders_basic(order_descriptions, session):
@@ -543,7 +581,13 @@ async def test_download_asset_img(tmpdir, open_test_img, session):
 
 @respx.mock
 @pytest.mark.asyncio
-async def test_download_order(tmpdir, order_description, oid, session):
+async def test_download_order_success(tmpdir, order_description, oid, session):
+    '''
+    Test if download_order() successfully downloads an order with two files,
+    given a singular order ID.
+    '''
+
+    # Mock an HTTP response for download
     dl_url1 = TEST_URL + 'download/1?token=IAmAToken'
     dl_url2 = TEST_URL + 'download/2?token=IAmAnotherToken'
     order_description['_links']['results'] = [
@@ -564,22 +608,100 @@ async def test_download_order(tmpdir, order_description, oid, session):
         })
     respx.get(dl_url1).return_value = mock_resp1
 
-    mock_resp1 = httpx.Response(
+    mock_resp2 = httpx.Response(
         HTTPStatus.OK,
         json={'key2': 'value2'},
         headers={
             'Content-Type': 'application/json',
             'Content-Disposition': 'attachment; filename="m2.json"'
         })
-    respx.get(dl_url2).return_value = mock_resp1
+    respx.get(dl_url2).return_value = mock_resp2
 
     cl = OrdersClient(session, base_url=TEST_URL)
     filenames = await cl.download_order(oid, directory=str(tmpdir))
 
+    # Check there are as many files as expected, given in order_description
     assert len(filenames) == 2
 
-    assert json.loads(open(filenames[0]).read()) == {'key': 'value'}
+    # Check that the downloaded files have the correct filename and contents
+    assert json.load(open(filenames[0])) == {'key': 'value'}
     assert Path(filenames[0]).name == 'm1.json'
-
-    assert json.loads(open(filenames[1]).read()) == {'key2': 'value2'}
+    assert json.load(open(filenames[1])) == {'key2': 'value2'}
     assert Path(filenames[1]).name == 'm2.json'
+
+
+@respx.mock
+@pytest.mark.asyncio
+async def test_download_order_overwrite_true_preexisting_data(
+        tmpdir, oid, session, create_download_mock, original_content,
+        downloaded_content):
+    '''
+    Test if download_order() overwrites pre-existing data with
+    overwrite flag set to True.
+    '''
+
+    # Save JSON to a temporary file
+    with open(Path(tmpdir, 'file.json'), "a") as out_file:
+        json.dump(original_content, out_file)
+
+    create_download_mock()
+    cl = OrdersClient(session, base_url=TEST_URL)
+    _ = await cl.download_order(oid, directory=str(tmpdir), overwrite=True)
+
+    # Check that the data downloaded has overwritten the original data
+    assert json.load(open(Path(tmpdir, 'file.json'))) == downloaded_content
+
+
+@respx.mock
+@pytest.mark.asyncio
+async def test_download_order_overwrite_false_preexisting_data(
+        tmpdir, oid, session, create_download_mock, original_content):
+    '''
+    Test if download_order() does not overwrite pre-existing
+    data with overwrite flag set to False.
+    '''
+
+    # Save JSON to a temporary file
+    with open(Path(tmpdir, 'file.json'), "a") as out_file:
+        json.dump(original_content, out_file)
+
+    create_download_mock()
+    cl = OrdersClient(session, base_url=TEST_URL)
+    _ = await cl.download_order(oid, directory=str(tmpdir), overwrite=False)
+
+    # Check that the original data has not been overwritten
+    assert json.load(open(Path(tmpdir, 'file.json'))) == original_content
+
+
+@respx.mock
+@pytest.mark.asyncio
+async def test_download_order_overwrite_true_nonexisting_data(
+        tmpdir, oid, session, create_download_mock, downloaded_content):
+    '''
+    Test if download_order() downloads data with overwrite flag
+    set to true without pre-existing data.
+    '''
+
+    create_download_mock()
+    cl = OrdersClient(session, base_url=TEST_URL)
+    _ = await cl.download_order(oid, directory=str(tmpdir), overwrite=True)
+
+    # Check that the was data downloaded and has the correct contents
+    assert json.load(open(Path(tmpdir, 'file.json'))) == downloaded_content
+
+
+@respx.mock
+@pytest.mark.asyncio
+async def test_download_order_overwrite_false_nonexisting_data(
+        tmpdir, oid, session, create_download_mock, downloaded_content):
+    '''
+    Test if download_order() downloads data with overwrite flag
+    set to false without pre-existing data.
+    '''
+
+    create_download_mock()
+    cl = OrdersClient(session, base_url=TEST_URL)
+    _ = await cl.download_order(oid, directory=str(tmpdir), overwrite=False)
+
+    # Check that the was data downloaded and has the correct contents
+    assert json.load(open(Path(tmpdir, 'file.json'))) == downloaded_content
diff --git a/tests/unit/test_cli.py b/tests/unit/test_cli.py
index 4ae039d..a941afc 100644
--- a/tests/unit/test_cli.py
+++ b/tests/unit/test_cli.py
@@ -134,8 +134,10 @@ def test_cli_orders_cancel(runner, monkeypatch, order_description, oid):
 
 
 def test_cli_orders_download(runner, monkeypatch, oid):
+    all_test_files = ['file1.json', 'file2.zip', 'file3.tiff', 'file4.jpg']
+
     async def do(*arg, **kwarg):
-        return ['file1']
+        return all_test_files
     monkeypatch.setattr(planet.scripts.cli.OrdersClient, 'download_order', do)
 
     async def poll(*arg, **kwarg):
@@ -145,8 +147,12 @@ def test_cli_orders_download(runner, monkeypatch, oid):
     result = runner.invoke(
         cli, ['orders', 'download', oid])
     assert not result.exception
-    # assert "file1" in result.output
-    assert 'Downloaded 1 files.\n' == result.output
+
+    # Check the output is as expected (list all files downloaded line-by-line)
+    # Add a new line character (\n) for each test filename
+    all_test_files_newline = '\n'.join(all_test_files) + '\n'
+    # Expect output to look like 'file1\nfile2\n...fileN\n'
+    assert all_test_files_newline == result.output
 
 
 class AsyncMock(Mock):