Improved Handling of URLFor Parameters (#199)

* Add values argument to URLFor and AbsoluteURLFor Add values argument to URLFOR and AbsoluteURLFor to separate between arguments passed to flask.url_for and arguments passed to marshmallow.fields.Field.__init__ with checks to ensure backward compatibility. * ADD DOCS FOR Add values argument to URLFor and AbsoluteURLFor Update the documentation to reflect the changes to URLFOR and AbsoluteURLFor * ADD TESTS FOR: Add values argument to URLFor and AbsoluteURLFor Update the tests to reflect the changes to URLFor and AbsoluteURLFor. Previous tests are kepts to test backward compatiability * Update docs and changelog; fix watch-docs * Update changelog * Reduce delay * Format Co-authored-by: Steven Loria <[email protected]>
marshmallow-code · Sep 28, 2020 · 21057fb · 21057fb
1 parent 9ce0f02
commit 21057fb
Show file tree

Hide file tree

Showing 7 changed files with 92 additions and 18 deletions.
diff --git a/CHANGELOG.rst b/CHANGELOG.rst
@@ -1,6 +1,41 @@
 Changelog
 =========
 
+0.14.0 (unreleased)
+*******************
+
+* Add ``values`` argument to ``URLFor`` and ``AbsoluteURLFor`` for passing values to ``flask.url_for``.
+ This prevents unrelated parameters from getting passed (:issue:`52`, :issue:`67`).
+ Thanks :user:`AlrasheedA` for the PR.
+
+Deprecation:
+
+* Passing params to ``flask.url_for`` via ``URLFor``'s and ``AbsoluteURLFor``'s constructor
+  params is deprecated. Pass ``values`` instead.
+
+.. code-block:: python
+
+    # flask-marshmallow<0.14.0
+
+
+    class UserSchema(ma.Schema):
+        _links = ma.Hyperlinks(
+            {
+                "self": ma.URLFor("user_detail", id="<id>"),
+            }
+        )
+
+
+    # flask-marshmallow>=0.14.0
+
+
+    class UserSchema(ma.Schema):
+        _links = ma.Hyperlinks(
+            {
+                "self": ma.URLFor("user_detail", values=dict(id="<id>")),
+            }
+        )
+
 0.13.0 (2020-06-07)
 *******************
 
@@ -20,8 +55,8 @@ Other changes:
 
 .. warning::
   It is highly recommended that you use the newer ``ma.SQLAlchemySchema`` and ``ma.SQLAlchemyAutoSchema``  classes
-  instead of ``ModelSchema`` and ``TableSchema``. See the release notes for `marshmallow-sqlalchemy 0.22.0 <https://marshmallow-sqlalchemy.readthedocs.io/en/latest/changelog.html>`_ 
-  for instructions on how to migrate. 
+  instead of ``ModelSchema`` and ``TableSchema``. See the release notes for `marshmallow-sqlalchemy 0.22.0 <https://marshmallow-sqlalchemy.readthedocs.io/en/latest/changelog.html>`_
+  for instructions on how to migrate.
 
 If you need to use ``ModelSchema`` and ``TableSchema`` for the time being, you'll need to import these directly from ``marshmallow_sqlalchemy``.
 

diff --git a/README.rst b/README.rst
@@ -51,7 +51,10 @@ Define your output format with marshmallow.
 
         # Smart hyperlinking
         _links = ma.Hyperlinks(
-            {"self": ma.URLFor("user_detail", id="<id>"), "collection": ma.URLFor("users")}
+            {
+                "self": ma.URLFor("user_detail", values=dict(id="<id>")),
+                "collection": ma.URLFor("users"),
+            }
         )
 
 

diff --git a/docs/index.rst b/docs/index.rst
@@ -55,7 +55,10 @@ Define your output format with marshmallow.
 
         # Smart hyperlinking
         _links = ma.Hyperlinks(
-            {"self": ma.URLFor("user_detail", id="<id>"), "collection": ma.URLFor("users")}
+            {
+                "self": ma.URLFor("user_detail", values=dict(id="<id>")),
+                "collection": ma.URLFor("users"),
+            }
         )
 
 

diff --git a/src/flask_marshmallow/__init__.py b/src/flask_marshmallow/__init__.py
@@ -71,7 +71,7 @@ class Meta:
             author = ma.Nested(AuthorSchema)
 
             links = ma.Hyperlinks({
-                'self': ma.URLFor('book_detail', id='<id>'),
+                'self': ma.URLFor('book_detail', values=dict(id='<id>')),
                 'collection': ma.URLFor('book_list')
             })
 

diff --git a/src/flask_marshmallow/fields.py b/src/flask_marshmallow/fields.py
@@ -64,32 +64,34 @@ def _get_value_for_key(obj, key, default):
 class URLFor(fields.Field):
     """Field that outputs the URL for an endpoint. Acts identically to
     Flask's ``url_for`` function, except that arguments can be pulled from the
-    object to be serialized.
+    object to be serialized, and ``**values`` should be passed to the ``values``
+    parameter.
 
     Usage: ::
 
-        url = URLFor('author_get', id='<id>')
-        https_url = URLFor('author_get', id='<id>', _scheme='https', _external=True)
+        url = URLFor('author_get', values=dict(id='<id>'))
+        https_url = URLFor('author_get', values=dict(id='<id>', _scheme='https', _external=True))
 
     :param str endpoint: Flask endpoint name.
-    :param kwargs: Same keyword arguments as Flask's url_for, except string
+    :param dict values: Same keyword arguments as Flask's url_for, except string
         arguments enclosed in `< >` will be interpreted as attributes to pull
         from the object.
+    :param kwargs: keyword arguments to pass to marshmallow field (e.g. ``required``).
     """
 
     _CHECK_ATTRIBUTE = False
 
-    def __init__(self, endpoint, **kwargs):
+    def __init__(self, endpoint, values=None, **kwargs):
         self.endpoint = endpoint
-        self.params = kwargs
+        self.values = values or kwargs  # kwargs for backward compatibility
         fields.Field.__init__(self, **kwargs)
 
     def _serialize(self, value, key, obj):
         """Output the URL for the endpoint, given the kwargs passed to
         ``__init__``.
         """
         param_values = {}
-        for name, attr_tpl in self.params.items():
+        for name, attr_tpl in self.values.items():
             attr_name = _tpl(str(attr_tpl))
             if attr_name:
                 attribute_value = _get_value(obj, attr_name, default=missing)
@@ -113,9 +115,12 @@ def _serialize(self, value, key, obj):
 class AbsoluteURLFor(URLFor):
     """Field that outputs the absolute URL for an endpoint."""
 
-    def __init__(self, endpoint, **kwargs):
-        kwargs["_external"] = True
-        URLFor.__init__(self, endpoint=endpoint, **kwargs)
+    def __init__(self, endpoint, values=None, **kwargs):
+        if values:  # for backward compatibility
+            values["_external"] = True
+        else:
+            kwargs["_external"] = True
+        URLFor.__init__(self, endpoint=endpoint, values=values, **kwargs)
 
 
 AbsoluteUrlFor = AbsoluteURLFor
@@ -149,15 +154,15 @@ class Hyperlinks(fields.Field):
     Example: ::
 
         _links = Hyperlinks({
-            'self': URLFor('author', id='<id>'),
+            'self': URLFor('author', values=dict(id='<id>')),
             'collection': URLFor('author_list'),
         })
 
     `URLFor` objects can be nested within the dictionary. ::
 
         _links = Hyperlinks({
             'self': {
-                'href': URLFor('book', id='<id>'),
+                'href': URLFor('book', values=dict(id='<id>')),
                 'title': 'book detail'
             }
         })

diff --git a/tests/test_fields.py b/tests/test_fields.py
@@ -19,6 +19,10 @@ def test_url_field(ma, mockauthor):
     result = field.serialize("url", mockauthor)
     assert result == url_for("author", id=mockauthor.id)
 
+    field = ma.URLFor("author", values=dict(id="<id>"))
+    result = field.serialize("url", mockauthor)
+    assert result == url_for("author", id=mockauthor.id)
+
     mockauthor.id = 0
     result = field.serialize("url", mockauthor)
     assert result == url_for("author", id=0)
@@ -32,12 +36,23 @@ def test_url_field_with_invalid_attribute(ma, mockauthor):
     with pytest.raises(AttributeError, match=expected_msg):
         field.serialize("url", mockauthor)
 
+    field = ma.URLFor("author", values=dict(id="<not-an-attr>"))
+    expected_msg = "{!r} is not a valid attribute of {!r}".format(
+        "not-an-attr", mockauthor
+    )
+    with pytest.raises(AttributeError, match=expected_msg):
+        field.serialize("url", mockauthor)
+
 
 def test_url_field_handles_nested_attribute(ma, mockbook, mockauthor):
     field = ma.URLFor("author", id="<author.id>")
     result = field.serialize("url", mockbook)
     assert result == url_for("author", id=mockauthor.id)
 
+    field = ma.URLFor("author", values=dict(id="<author.id>"))
+    result = field.serialize("url", mockbook)
+    assert result == url_for("author", id=mockauthor.id)
+
 
 def test_url_field_handles_none_attribute(ma, mockbook, mockauthor):
     mockbook.author = None
@@ -50,13 +65,26 @@ def test_url_field_handles_none_attribute(ma, mockbook, mockauthor):
     result = field.serialize("url", mockbook)
     assert result is None
 
+    field = ma.URLFor("author", values=dict(id="<author>"))
+    result = field.serialize("url", mockbook)
+    assert result is None
+
+    field = ma.URLFor("author", values=dict(id="<author.id>"))
+    result = field.serialize("url", mockbook)
+    assert result is None
+
 
 def test_url_field_deserialization(ma):
     field = ma.URLFor("author", id="<not-an-attr>", allow_none=True)
     # noop
     assert field.deserialize("foo") == "foo"
     assert field.deserialize(None) is None
 
+    field = ma.URLFor("author", values=dict(id="<not-an-attr>"), allow_none=True)
+    # noop
+    assert field.deserialize("foo") == "foo"
+    assert field.deserialize(None) is None
+
 
 def test_invalid_endpoint_raises_build_error(ma, mockauthor):
     field = ma.URLFor("badendpoint")

diff --git a/tox.ini b/tox.ini
@@ -32,7 +32,7 @@ commands = sphinx-build docs/ docs/_build {posargs}
 deps =
   sphinx-autobuild
 extras = docs
-commands = sphinx-autobuild --open-browser docs/ docs/_build {posargs} -z src/flask_marshmallow -s 2
+commands = sphinx-autobuild --open-browser docs/ docs/_build {posargs} --watch src/flask_marshmallow --delay 2
 
 [testenv:watch-readme]
 deps = restview