Repository: falconry/falcon
Pull request #1802 (2020-12-11 17:56:35)

refactor(Response): rename add_link to append_link
# Summary of Changes

Rename add_link to append_link for accuracy and consistency.

# Related Issues

#1801

# Pull Request Checklist

This is just a reminder about the most common mistakes.  Please make sure that you tick all *appropriate* boxes.  But please read our [contribution guide](../CONTRIBUTING.md) at least once; it will save you a few review cycles!

If an item doesn't apply to your pull request, **check it anyway** to make it apparent that there's nothing to do.

- [x] Applied changes to both WSGI and ASGI code paths and interfaces (where applicable).
- [x] Added **tests** for changed code.
- [x] Prefixed code comments with GitHub nick and an appropriate prefix.
- [x] Coding style is consistent with the rest of the framework.
- [x] Updated **documentation** for changed code.
    - [ ] Added docstrings for any new classes, functions, or modules.
    - [ ] Updated docstrings for any modifications to existing code.
    - [ ] Updated both WSGI and ASGI docs (where applicable).
    - [ ] Added references to new classes, functions, or modules to the relevant RST file under `docs/`.
    - [ ] Updated all relevant supporting documentation files under `docs/`.
    - [ ] A copyright notice is included at the top of any new modules (using your own name or the name of your organization).
    - [ ] Changed/added classes/methods/functions have appropriate `versionadded`, `versionchanged`, or `deprecated` [directives](http://www.sphinx-doc.org/en/stable/usage/restructuredtext/directives.html?highlight=versionadded#directive-versionadded).
- [x] Changes (and possible deprecations) have [towncrier](https://towncrier.readthedocs.io/en/actual-freaking-docs/index.html) news fragments under `docs/_newsfragments/`, with the file name format `{issue_number}.{fragment_type}.rst`. (Run `towncrier --draft` to ensure it renders correctly.)

If you have *any* questions to *any* of the points above, just **submit and ask**! This checklist is here to *help* you, not to deter you from contributing!

*PR template inspired by the attrs project.*


codecov[bot]: # [Codecov](https://codecov.io/gh/falconry/falcon/pull/1802?src=pr&el=h1) Report
> Merging [#1802](https://codecov.io/gh/falconry/falcon/pull/1802?src=pr&el=desc) (466de1a) into [master](https://codecov.io/gh/falconry/falcon/commit/3c2c872b2108a068e755c579f2a329f58a8aa266?el=desc) (3c2c872) will **decrease** coverage by `38.10%`.
> The diff coverage is `n/a`.

[![Impacted file tree graph](https://codecov.io/gh/falconry/falcon/pull/1802/graphs/tree.svg?width=650&height=150&src=pr&token=zOtkfNQ3h6)](https://codecov.io/gh/falconry/falcon/pull/1802?src=pr&el=tree)

```diff
@@             Coverage Diff              @@
##            master    #1802       +/-   ##
============================================
- Coverage   100.00%   61.89%   -38.11%     
============================================
  Files           54       54               
  Lines         5076     5076               
  Branches       808      808               
============================================
- Hits          5076     3142     -1934     
- Misses           0     1924     +1924     
- Partials         0       10       +10     
```


| [Impacted Files](https://codecov.io/gh/falconry/falcon/pull/1802?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [falcon/asgi/ws.py](https://codecov.io/gh/falconry/falcon/pull/1802/diff?src=pr&el=tree#diff-ZmFsY29uL2FzZ2kvd3MucHk=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [falcon/asgi/app.py](https://codecov.io/gh/falconry/falcon/pull/1802/diff?src=pr&el=tree#diff-ZmFsY29uL2FzZ2kvYXBwLnB5) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [falcon/asgi/reader.py](https://codecov.io/gh/falconry/falcon/pull/1802/diff?src=pr&el=tree#diff-ZmFsY29uL2FzZ2kvcmVhZGVyLnB5) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [falcon/asgi/stream.py](https://codecov.io/gh/falconry/falcon/pull/1802/diff?src=pr&el=tree#diff-ZmFsY29uL2FzZ2kvc3RyZWFtLnB5) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [falcon/asgi/request.py](https://codecov.io/gh/falconry/falcon/pull/1802/diff?src=pr&el=tree#diff-ZmFsY29uL2FzZ2kvcmVxdWVzdC5weQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [falcon/asgi/response.py](https://codecov.io/gh/falconry/falcon/pull/1802/diff?src=pr&el=tree#diff-ZmFsY29uL2FzZ2kvcmVzcG9uc2UucHk=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [falcon/asgi/multipart.py](https://codecov.io/gh/falconry/falcon/pull/1802/diff?src=pr&el=tree#diff-ZmFsY29uL2FzZ2kvbXVsdGlwYXJ0LnB5) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [falcon/asgi/structures.py](https://codecov.io/gh/falconry/falcon/pull/1802/diff?src=pr&el=tree#diff-ZmFsY29uL2FzZ2kvc3RydWN0dXJlcy5weQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [falcon/asgi/\_request\_helpers.py](https://codecov.io/gh/falconry/falcon/pull/1802/diff?src=pr&el=tree#diff-ZmFsY29uL2FzZ2kvX3JlcXVlc3RfaGVscGVycy5weQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [falcon/testing/helpers.py](https://codecov.io/gh/falconry/falcon/pull/1802/diff?src=pr&el=tree#diff-ZmFsY29uL3Rlc3RpbmcvaGVscGVycy5weQ==) | `36.57% <0.00%> (-63.43%)` | :arrow_down: |
| ... and [25 more](https://codecov.io/gh/falconry/falcon/pull/1802/diff?src=pr&el=tree-more) | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/falconry/falcon/pull/1802?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/falconry/falcon/pull/1802?src=pr&el=footer). Last update [3c2c872...466de1a](https://codecov.io/gh/falconry/falcon/pull/1802?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

kgriffs: > Shouldn't we emit a deprecation warning upon referencing the deprecated alias?

I did think about it, but didn't want to have to add another layer of indirection. Perhaps we can add the warning in 3.1 once people have started migrating.

Patch:
diff --git a/docs/_newsfragments/1801.misc.rst b/docs/_newsfragments/1801.misc.rst
new file mode 100644
index 0000000..2d47273
--- /dev/null
+++ b/docs/_newsfragments/1801.misc.rst
@@ -0,0 +1,3 @@
+The `add_link()` method of the :class:`falcon.Request` class was renamed to
+:meth:`falcon.Request.append_link`. The old name is still available as a
+deprecated alias.
diff --git a/docs/api/request_and_response_asgi.rst b/docs/api/request_and_response_asgi.rst
index 8b155cc..8d7d154 100644
--- a/docs/api/request_and_response_asgi.rst
+++ b/docs/api/request_and_response_asgi.rst
@@ -45,7 +45,7 @@ Response
 .. autoclass:: falcon.asgi.Response
     :members:
     :inherited-members:
-    :exclude-members: context_type
+    :exclude-members: context_type, add_link
 
 .. autoclass:: falcon.asgi.SSEvent
     :members:
diff --git a/docs/api/request_and_response_wsgi.rst b/docs/api/request_and_response_wsgi.rst
index 8c03fee..ccca0d0 100644
--- a/docs/api/request_and_response_wsgi.rst
+++ b/docs/api/request_and_response_wsgi.rst
@@ -45,4 +45,4 @@ Response
 
 .. autoclass:: falcon.Response
     :members:
-    :exclude-members: context_type
+    :exclude-members: context_type, add_link
diff --git a/falcon/response.py b/falcon/response.py
index 3583fb0..a26f456 100644
--- a/falcon/response.py
+++ b/falcon/response.py
@@ -751,9 +751,9 @@ class Response:
 
             _headers[name] = value
 
-    def add_link(self, target, rel, title=None, title_star=None,
-                 anchor=None, hreflang=None, type_hint=None, crossorigin=None):
-        """Add a link header to the response.
+    def append_link(self, target, rel, title=None, title_star=None,
+                    anchor=None, hreflang=None, type_hint=None, crossorigin=None):
+        """Append a link header to the response.
 
         (See also: RFC 5988, Section 1)
 
@@ -877,6 +877,9 @@ class Response:
         else:
             _headers['link'] = value
 
+    # NOTE(kgriffs): Alias deprecated as of 3.0
+    add_link = append_link
+
     cache_control = header_property(
         'Cache-Control',
         """Set the Cache-Control header.


Test patch:
diff --git a/tests/test_headers.py b/tests/test_headers.py
index 581685d..d60d958 100644
--- a/tests/test_headers.py
+++ b/tests/test_headers.py
@@ -158,14 +158,16 @@ class LinkHeaderResource:
     def __init__(self):
         self._links = []
 
-    def add_link(self, *args, **kwargs):
+    def append_link(self, *args, **kwargs):
         self._links.append((args, kwargs))
 
     def on_get(self, req, resp):
         resp.body = '{}'
 
+        append_link = None
         for args, kwargs in self._links:
-            resp.add_link(*args, **kwargs)
+            append_link = resp.append_link if append_link is resp.add_link else resp.add_link
+            append_link(*args, **kwargs)
 
 
 class AppendHeaderResource:
@@ -666,15 +668,15 @@ class TestHeaders:
         resource = XmlResource(content_type)
         self._check_header(client, resource, 'Content-Type', content_type)
 
-    def test_add_link_single(self, client):
+    def test_append_link_single(self, client):
         expected_value = '</things/2842>; rel=next'
 
         resource = LinkHeaderResource()
-        resource.add_link('/things/2842', 'next')
+        resource.append_link('/things/2842', 'next')
 
         self._check_link_header(client, resource, expected_value)
 
-    def test_add_link_multiple(self, client):
+    def test_append_link_multiple(self, client):
         expected_value = (
             '</things/2842>; rel=next, ' +
             '<http://%C3%A7runchy/bacon>; rel=contents, ' +
@@ -686,81 +688,81 @@ class TestHeaders:
         uri = 'ab\u00e7'
 
         resource = LinkHeaderResource()
-        resource.add_link('/things/2842', 'next')
-        resource.add_link('http://\u00e7runchy/bacon', 'contents')
-        resource.add_link(uri, 'http://example.com/ext-type')
-        resource.add_link(uri, 'http://example.com/\u00e7runchy')
-        resource.add_link(uri, 'https://example.com/too-\u00e7runchy')
-        resource.add_link('/alt-thing',
-                          'alternate http://example.com/\u00e7runchy')
+        resource.append_link('/things/2842', 'next')
+        resource.append_link('http://\u00e7runchy/bacon', 'contents')
+        resource.append_link(uri, 'http://example.com/ext-type')
+        resource.append_link(uri, 'http://example.com/\u00e7runchy')
+        resource.append_link(uri, 'https://example.com/too-\u00e7runchy')
+        resource.append_link('/alt-thing',
+                             'alternate http://example.com/\u00e7runchy')
 
         self._check_link_header(client, resource, expected_value)
 
-    def test_add_link_with_title(self, client):
+    def test_append_link_with_title(self, client):
         expected_value = ('</related/thing>; rel=item; '
                           'title="A related thing"')
 
         resource = LinkHeaderResource()
-        resource.add_link('/related/thing', 'item',
-                          title='A related thing')
+        resource.append_link('/related/thing', 'item',
+                             title='A related thing')
 
         self._check_link_header(client, resource, expected_value)
 
-    def test_add_link_with_title_star(self, client):
+    def test_append_link_with_title_star(self, client):
         expected_value = ('</related/thing>; rel=item; '
                           "title*=UTF-8''A%20related%20thing, "
                           '</%C3%A7runchy/thing>; rel=item; '
                           "title*=UTF-8'en'A%20%C3%A7runchy%20thing")
 
         resource = LinkHeaderResource()
-        resource.add_link('/related/thing', 'item',
-                          title_star=('', 'A related thing'))
+        resource.append_link('/related/thing', 'item',
+                             title_star=('', 'A related thing'))
 
-        resource.add_link('/\u00e7runchy/thing', 'item',
-                          title_star=('en', 'A \u00e7runchy thing'))
+        resource.append_link('/\u00e7runchy/thing', 'item',
+                             title_star=('en', 'A \u00e7runchy thing'))
 
         self._check_link_header(client, resource, expected_value)
 
-    def test_add_link_with_anchor(self, client):
+    def test_append_link_with_anchor(self, client):
         expected_value = ('</related/thing>; rel=item; '
                           'anchor="/some%20thing/or-other"')
 
         resource = LinkHeaderResource()
-        resource.add_link('/related/thing', 'item',
-                          anchor='/some thing/or-other')
+        resource.append_link('/related/thing', 'item',
+                             anchor='/some thing/or-other')
 
         self._check_link_header(client, resource, expected_value)
 
-    def test_add_link_with_hreflang(self, client):
+    def test_append_link_with_hreflang(self, client):
         expected_value = ('</related/thing>; rel=about; '
                           'hreflang=en')
 
         resource = LinkHeaderResource()
-        resource.add_link('/related/thing', 'about', hreflang='en')
+        resource.append_link('/related/thing', 'about', hreflang='en')
 
         self._check_link_header(client, resource, expected_value)
 
-    def test_add_link_with_hreflang_multi(self, client):
+    def test_append_link_with_hreflang_multi(self, client):
         expected_value = ('</related/thing>; rel=about; '
                           'hreflang=en-GB; hreflang=de')
 
         resource = LinkHeaderResource()
-        resource.add_link('/related/thing', 'about',
-                          hreflang=('en-GB', 'de'))
+        resource.append_link('/related/thing', 'about',
+                             hreflang=('en-GB', 'de'))
 
         self._check_link_header(client, resource, expected_value)
 
-    def test_add_link_with_type_hint(self, client):
+    def test_append_link_with_type_hint(self, client):
         expected_value = ('</related/thing>; rel=alternate; '
                           'type="video/mp4; codecs=avc1.640028"')
 
         resource = LinkHeaderResource()
-        resource.add_link('/related/thing', 'alternate',
-                          type_hint='video/mp4; codecs=avc1.640028')
+        resource.append_link('/related/thing', 'alternate',
+                             type_hint='video/mp4; codecs=avc1.640028')
 
         self._check_link_header(client, resource, expected_value)
 
-    def test_add_link_complex(self, client):
+    def test_append_link_complex(self, client):
         expected_value = ('</related/thing>; rel=alternate; '
                           'title="A related thing"; '
                           "title*=UTF-8'en'A%20%C3%A7runchy%20thing; "
@@ -768,11 +770,11 @@ class TestHeaders:
                           'hreflang=en-GB; hreflang=de')
 
         resource = LinkHeaderResource()
-        resource.add_link('/related/thing', 'alternate',
-                          title='A related thing',
-                          hreflang=('en-GB', 'de'),
-                          type_hint='application/json',
-                          title_star=('en', 'A \u00e7runchy thing'))
+        resource.append_link('/related/thing', 'alternate',
+                             title='A related thing',
+                             hreflang=('en-GB', 'de'),
+                             type_hint='application/json',
+                             title_star=('en', 'A \u00e7runchy thing'))
 
         self._check_link_header(client, resource, expected_value)
 
@@ -790,10 +792,10 @@ class TestHeaders:
             '</related/thing>; rel=alternate; crossorigin="use-credentials"',
         ),
     ])
-    def test_add_link_crossorigin(self, client, crossorigin, expected_value):
+    def test_append_link_crossorigin(self, client, crossorigin, expected_value):
         resource = LinkHeaderResource()
-        resource.add_link('/related/thing', 'alternate',
-                          crossorigin=crossorigin)
+        resource.append_link('/related/thing', 'alternate',
+                             crossorigin=crossorigin)
 
         self._check_link_header(client, resource, expected_value)
 
@@ -805,11 +807,11 @@ class TestHeaders:
         'Strict',
         'deny',
     ])
-    def test_add_link_invalid_crossorigin_value(self, crossorigin):
+    def test_append_link_invalid_crossorigin_value(self, crossorigin):
         resp = falcon.Response()
 
         with pytest.raises(ValueError):
-            resp.add_link('/related/resource', 'next', crossorigin=crossorigin)
+            resp.append_link('/related/resource', 'next', crossorigin=crossorigin)
 
     def test_content_length_options(self, client):
         result = client.simulate_options()