Repository: falconry/falcon

Issue:
Fill in the "Query Strings" section of tutorial.rst
Right now it just says "coming soon".


Discussion:
bssyousefi: I have a question regarding a part of the code.
```
 _IMAGE_NAME_PATTERN = re.compile(
            '[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}\.[a-z]{2,4}$'
        )
```
which I used from previous parts of the docs.
the string used as the pattern will raise error in python. I guess we should use one of the following ways
```
 _IMAGE_NAME_PATTERN = re.compile(
            '[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}.[a-z]{2,4}$'
        )
 _IMAGE_NAME_PATTERN = re.compile(
            r'[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}\.[a-z]{2,4}$'
        )
```
However, I didn't try to fix it here and I thought that maybe I should first report it as an issue and then resolve it in another PR. Am I right or should I try the fix on this PR since it's a small change?
codecov[bot]: ## [Codecov](https://app.codecov.io/gh/falconry/falcon/pull/2239?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=falconry) Report
All modified and coverable lines are covered by tests :white_check_mark:
> Project coverage is 36.05%. Comparing base [(`89ccb98`)](https://app.codecov.io/gh/falconry/falcon/commit/89ccb9871f6bec7847048532f0ed20cd483b1f18?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=falconry) to head [(`3be386a`)](https://app.codecov.io/gh/falconry/falcon/commit/3be386a6f11668d2a1c2e1e331b5995d5c95cff1?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=falconry).
> Report is 5 commits behind head on master.

> :exclamation:  There is a different number of reports uploaded between BASE (89ccb98) and HEAD (3be386a). Click for more details.
> 
> <details><summary>HEAD has 1 upload less than BASE</summary>
>
>| Flag | BASE (89ccb98) | HEAD (3be386a) |
>|------|------|------|
>||2|1|
></details>

<details><summary>Additional details and impacted files</summary>


```diff
@@             Coverage Diff              @@
##            master    #2239       +/-   ##
============================================
- Coverage   100.00%   36.05%   -63.95%     
============================================
  Files           63       63               
  Lines         6889     6847       -42     
  Branches      1258     1258               
============================================
- Hits          6889     2469     -4420     
- Misses           0     4320     +4320     
- Partials         0       58       +58     
```

</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/falconry/falcon/pull/2239?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=falconry).   
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=falconry).

bssyousefi: > I have a question regarding a part of the code.
> 
> ```
>  _IMAGE_NAME_PATTERN = re.compile(
>             '[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}\.[a-z]{2,4}$'
>         )
> ```
> 
> which I used from previous parts of the docs. the string used as the pattern will raise error in python. I guess we should use one of the following ways
> 
> ```
>  _IMAGE_NAME_PATTERN = re.compile(
>             '[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}.[a-z]{2,4}$'
>         )
>  _IMAGE_NAME_PATTERN = re.compile(
>             r'[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}\.[a-z]{2,4}$'
>         )
> ```
> 
> However, I didn't try to fix it here and I thought that maybe I should first report it as an issue and then resolve it in another PR. Am I right or should I try the fix on this PR since it's a small change?

@vytas7 Would you please also help me on this?
bssyousefi: @vytas7 I'm very sorry for the very long delay. I mistakenly thought that all the tests were passed and I didn't got time to get back to the project since I was a little bit busy.
Thanks for you patience.
I will be more available and responsible from now on.
vytas7: No worries, @bssyousefi :slightly_smiling_face: 
We are now focusing on getting 4.0 out the door, so your PR is not urgent. But it will be nice to have it done when ready.
bssyousefi: I guess it's ready. I've updated the tests for the example `look`
bssyousefi: Hi,
Is there any issue blocking this PR?
vytas7: Hi @bssyousefi, and thanks for bringing this up. No, I don't think so, I just need to review it again.

Patch:
diff --git a/docs/user/tutorial.rst b/docs/user/tutorial.rst
index b4589af..9e483d3 100644
--- a/docs/user/tutorial.rst
+++ b/docs/user/tutorial.rst
@@ -1174,7 +1174,7 @@ Go ahead and edit your ``images.py`` file to look something like this:
 
         _CHUNK_SIZE_BYTES = 4096
         _IMAGE_NAME_PATTERN = re.compile(
-            '[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}\.[a-z]{2,4}$'
+            r'[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}\.[a-z]{2,4}$'
         )
 
         def __init__(self, storage_path, uuidgen=uuid.uuid4, fopen=io.open):
@@ -1304,10 +1304,216 @@ Inspecting the application now returns:
         ⇒ /images/{name} - Item:
            └── GET - on_get
 
-.. Query Strings
-.. -------------
+Query Strings
+-------------
+Now that we are able to get the images from the service, we need a way to get
+a list of available images. We have already set up this route. Before testing this
+route let's change its output format back to JSON to have a more
+terminal-friendly output. The top of file ``images.py`` should look like this:
+
+.. code:: python
+
+    import io
+    import os
+    import re
+    import uuid
+    import mimetypes
+
+    import falcon
+    import json
+
+
+    class Collection:
+
+        def __init__(self, image_store):
+            self._image_store = image_store
+
+        def on_get(self, req, resp):
+            # TODO: Modify this to return a list of href's based on
+            # what images are actually available.
+            doc = {
+                'images': [
+                    {
+                        'href': '/images/1eaf6ef1-7f2d-4ecc-a8d5-6e8adba7cc0e.png'
+                    }
+                ]
+            }
+
+            resp.text = json.dumps(doc, ensure_ascii=False)
+            resp.status = falcon.HTTP_200
+
+    def on_post(self, req, resp):
+        name = self._image_store.save(req.stream, req.content_type)
+        resp.status = falcon.HTTP_201
+        resp.location = '/images/' + name
+
+
+Now try the following:
+
+.. code:: bash
+
+    http localhost:8000/images
+
+In response you should get the following data that we statically have put in the code.
+
+.. code::
+
+    {
+        "images": [
+            {
+                "href": "/images/1eaf6ef1-7f2d-4ecc-a8d5-6e8adba7cc0e.png"
+            }
+        ]
+    }
+
+Let's go back to the ``on_get`` method and create a dynamic response. We can
+use query strings to set maximum image size and get the list of all images
+smaller than the specified value. We will use method ``get_param_as_int`` to
+set a default value of ``-1`` in case no ``maxsize`` query string was provided
+and also to enable a minimum value validation.
+
+.. code:: python
+
+    import io
+    import os
+    import re
+    import uuid
+    import mimetypes
+
+    import falcon
+    import json
+
+
+    class Collection:
+
+        def __init__(self, image_store):
+            self._image_store = image_store
+
+        def on_get(self, req, resp):
+            max_size = req.get_param_as_int("maxsize", min_value=1, default=-1)
+            images = self._image_store.list(max_size)
+            doc = {
+                'images': [
+                    {'href': '/images/' + image} for image in images
+                ]
+            }
+
+            resp.text = json.dumps(doc, ensure_ascii=False)
+            resp.status = falcon.HTTP_200
+
+        def on_post(self, req, resp):
+            name = self._image_store.save(req.stream, req.content_type)
+            resp.status = falcon.HTTP_201
+            resp.location = '/images/' + name
+
+
+    class Item:
+
+        def __init__(self, image_store):
+            self._image_store = image_store
+
+        def on_get(self, req, resp, name):
+            resp.content_type = mimetypes.guess_type(name)[0]
+            resp.stream, resp.content_length = self._image_store.open(name)
+
+
+    class ImageStore:
+
+        _CHUNK_SIZE_BYTES = 4096
+        _IMAGE_NAME_PATTERN = re.compile(
+            r'[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}\.[a-z]{2,4}$'
+        )
+
+        def __init__(self, storage_path, uuidgen=uuid.uuid4, fopen=io.open):
+            self._storage_path = storage_path
+            self._uuidgen = uuidgen
+            self._fopen = fopen
+
+        def save(self, image_stream, image_content_type):
+            ext = mimetypes.guess_extension(image_content_type)
+            name = '{uuid}{ext}'.format(uuid=self._uuidgen(), ext=ext)
+            image_path = os.path.join(self._storage_path, name)
+
+            with self._fopen(image_path, 'wb') as image_file:
+                while True:
+                    chunk = image_stream.read(self._CHUNK_SIZE_BYTES)
+                    if not chunk:
+                        break
+
+                    image_file.write(chunk)
+
+            return name
+
+        def open(self, name):
+            # Always validate untrusted input!
+            if not self._IMAGE_NAME_PATTERN.match(name):
+                raise IOError('File not found')
+
+            image_path = os.path.join(self._storage_path, name)
+            stream = self._fopen(image_path, 'rb')
+            content_length = os.path.getsize(image_path)
+
+            return stream, content_length
+
+        def list(self, max_size):
+            images = [
+                image for image in os.listdir(self._storage_path)
+                if self._IMAGE_NAME_PATTERN.match(image)
+                and (
+                    max_size == -1
+                    or os.path.getsize(os.path.join(self._storage_path, image)) <= max_size
+                )
+            ]
+            return images
+
+As you can see the method ``list`` has been added to ``ImageStore`` in order
+to return list of available images smaller than ``max_size`` unless it is not
+``-1``, in which case it will behave like there was no predicament of image size.
+Let's try to save some binary data as images in the service and then try to
+retrieve their list. Execute the following commands in order to simulate the
+creation of 3 files as images with different sizes. While these are not valid
+PNG files, they will work for this tutorial.
+
+.. code:: bash
+
+   echo "First Case" > pseudo-image-1.png
+   echo "Second Case" > pseudo-image-2.png
+   echo "3rd Case" > pseudo-image-3.png
+
+Now we need to store these files using ``POST`` request:
+
+.. code:: bash
+
+   http POST localhost:8000/images Content-Type:image/png < pseudo-image-1.png
+   http POST localhost:8000/images Content-Type:image/png < pseudo-image-2.png
+   http POST localhost:8000/images Content-Type:image/png < pseudo-image-3.png
+
+If we check the size of these files, we will see that they are 11, 12, 9 bytes
+respectively. Let's try to get the list of the images which are smaller or
+equal to 11 bytes.
+
+.. code:: bash
+
+   http localhost:8000/images?maxsize=11
+
+We expect to get a list of 2 files, which will be similar to the following:
+
+.. code::
+
+    {
+        "images": [
+            {
+                "href": "/images/7ba2ebc9-726f-46b0-9615-a69824f5089b.png"
+            },
+            {
+                "href": "/images/e4354a31-2161-4064-805c-3bc7c332e7e6.png"
+            }
+        ]
+    }
+
+You could also now validate the response with getting the image files using
+the ``href`` value in the response and compare them with the original files.
 
-.. *Coming soon...*
 
 Introducing Hooks
 -----------------
diff --git a/examples/look/look/app.py b/examples/look/look/app.py
index abce808..7b591ce 100644
--- a/examples/look/look/app.py
+++ b/examples/look/look/app.py
@@ -2,14 +2,15 @@ import os
 
 import falcon
 
+from .images import Collection
 from .images import ImageStore
-from .images import Resource
+from .images import Item
 
 
 def create_app(image_store):
-    image_resource = Resource(image_store)
     app = falcon.App()
-    app.add_route('/images', image_resource)
+    app.add_route('/images', Collection(image_store))
+    app.add_route('/images/{name}', Item(image_store))
     return app
 
 
diff --git a/examples/look/look/images.py b/examples/look/look/images.py
index 31466d9..8f2332f 100644
--- a/examples/look/look/images.py
+++ b/examples/look/look/images.py
@@ -1,28 +1,23 @@
 import io
+import json
 import mimetypes
 import os
+import re
 import uuid
 
-import msgpack
-
 import falcon
 
 
-class Resource:
+class Collection:
     def __init__(self, image_store):
         self._image_store = image_store
 
     def on_get(self, req, resp):
-        doc = {
-            'images': [
-                {
-                    'href': '/images/1eaf6ef1-7f2d-4ecc-a8d5-6e8adba7cc0e.png',
-                },
-            ],
-        }
-
-        resp.data = msgpack.packb(doc, use_bin_type=True)
-        resp.content_type = 'application/msgpack'
+        max_size = req.get_param_as_int('maxsize', min_value=1, default=-1)
+        images = self._image_store.list(max_size)
+        doc = {'images': [{'href': '/images/' + image} for image in images]}
+
+        resp.text = json.dumps(doc, ensure_ascii=False)
         resp.status = falcon.HTTP_200
 
     def on_post(self, req, resp):
@@ -31,8 +26,20 @@ class Resource:
         resp.location = '/images/' + name
 
 
+class Item:
+    def __init__(self, image_store):
+        self._image_store = image_store
+
+    def on_get(self, req, resp, name):
+        resp.content_type = mimetypes.guess_type(name)[0]
+        resp.stream, resp.content_length = self._image_store.open(name)
+
+
 class ImageStore:
     _CHUNK_SIZE_BYTES = 4096
+    _IMAGE_NAME_PATTERN = re.compile(
+        r'[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}\.[a-z]{2,4}$'
+    )
 
     # Note the use of dependency injection for standard library
     # methods. We'll use these later to avoid monkey-patching.
@@ -55,3 +62,26 @@ class ImageStore:
                 image_file.write(chunk)
 
         return name
+
+    def open(self, name):
+        # Always validate untrusted input!
+        if not self._IMAGE_NAME_PATTERN.match(name):
+            raise IOError('File not found')
+
+        image_path = os.path.join(self._storage_path, name)
+        stream = self._fopen(image_path, 'rb')
+        content_length = os.path.getsize(image_path)
+
+        return stream, content_length
+
+    def list(self, max_size):
+        images = [
+            image
+            for image in os.listdir(self._storage_path)
+            if self._IMAGE_NAME_PATTERN.match(image)
+            and (
+                max_size == -1
+                or os.path.getsize(os.path.join(self._storage_path, image)) <= max_size
+            )
+        ]
+        return images


Test patch:
diff --git a/examples/look/tests/test_app.py b/examples/look/tests/test_app.py
index c6db645..419fe83 100644
--- a/examples/look/tests/test_app.py
+++ b/examples/look/tests/test_app.py
@@ -1,10 +1,12 @@
 import io
+import os
+from unittest import TestCase
 from unittest.mock import call
 from unittest.mock import MagicMock
 from unittest.mock import mock_open
+import uuid
 from wsgiref.validate import InputWrapper
 
-import msgpack
 import pytest
 
 import falcon
@@ -25,19 +27,17 @@ def client(mock_store):
     return testing.TestClient(api)
 
 
-def test_list_images(client):
-    doc = {
-        'images': [
-            {
-                'href': '/images/1eaf6ef1-7f2d-4ecc-a8d5-6e8adba7cc0e.png',
-            },
-        ],
-    }
+def test_list_images(client, mock_store):
+    images = ['first-file', 'second-file', 'third-file']
+    image_docs = [{'href': '/images/' + image} for image in images]
+
+    mock_store.list.return_value = images
 
     response = client.simulate_get('/images')
-    result_doc = msgpack.unpackb(response.content, raw=False)
 
-    assert result_doc == doc
+    result = response.json
+
+    assert result['images'] == image_docs
     assert response.status == falcon.HTTP_OK
 
 
@@ -64,7 +64,7 @@ def test_post_image(client, mock_store):
     assert saver_call[0][1] == image_content_type
 
 
-def test_saving_image(monkeypatch):
+def test_saving_image():
     # This still has some mocks, but they are more localized and do not
     # have to be monkey-patched into standard library modules (always a
     # risky business).
@@ -84,3 +84,55 @@ def test_saving_image(monkeypatch):
 
     assert store.save(fake_request_stream, 'image/png') == fake_uuid + '.png'
     assert call().write(fake_image_bytes) in mock_file_open.mock_calls
+
+
+def test_get_image(client, mock_store):
+    file_bytes = b'fake-image-bytes'
+
+    mock_store.open.return_value = ((file_bytes,), 17)
+
+    response = client.simulate_get('/images/filename.png')
+
+    assert response.status == falcon.HTTP_OK
+    assert response.content == file_bytes
+
+
+def test_opening_image():
+    file_name = f'{uuid.uuid4()}.png'
+    storage_path = '.'
+    file_path = f'{storage_path}/{file_name}'
+    fake_image_bytes = b'fake-image-bytes'
+    with open(file_path, 'wb') as image_file:
+        file_length = image_file.write(fake_image_bytes)
+
+    store = look.images.ImageStore(storage_path)
+
+    file_reader, content_length = store.open(file_name)
+    assert content_length == file_length
+    assert file_reader.read() == fake_image_bytes
+    os.remove(file_path)
+
+    with TestCase().assertRaises(IOError):
+        store.open('wrong_file_name_format')
+
+
+def test_listing_images():
+    file_names = [f'{uuid.uuid4()}.png' for _ in range(2)]
+    storage_path = '.'
+    file_paths = [f'{storage_path}/{name}' for name in file_names]
+    fake_images_bytes = [
+        b'fake-image-bytes',  # 17
+        b'fake-image-bytes-with-more-length',  # 34
+    ]
+    for i in range(2):
+        with open(file_paths[i], 'wb') as image_file:
+            image_file.write(fake_images_bytes[i])
+
+    store = look.images.ImageStore(storage_path)
+    assert store.list(10) == []
+    assert store.list(20) == [file_names[0]]
+    assert len(store.list(40)) == 2
+    assert sorted(store.list(40)) == sorted(file_names)
+
+    for file_path in file_paths:
+        os.remove(file_path)