From ada520b7a03174684ceb4ee92f523b04e2683b66 Mon Sep 17 00:00:00 2001 From: Dan Hemberger Date: Wed, 29 Aug 2018 08:29:35 -0700 Subject: [PATCH 1/2] stateful_browser.py: rename submit_selected to submit The obvious method to use when trying to submit a form is the `submit` method. Unless you are following the documentation explicitly, you may overlook the `submit_selected` method (which is really the only correct method to use when submitting a form within a StatefulBrowser instance). This is demonstrated by #230, where the user got an unexpected error because they used `submit` instead of `submit_selected`. To adhere to the principle of least astonishment, make `submit` a synonym for `submit_selected` (with `submit` being the primary name and `submit_selected` being an alias for backwards compatibility). This is a breaking change for anyone calling the base class `submit` from a StatefulBrowser instance. It is arguable that even if the call is made correctly, it is the "wrong" way to submit, so we should make it more difficult to "do the wrong thing". You can still access the base class `Browser.submit` from an instance of `StatefulBrowser` if you need to by using the `super` builtin, e.g. br = mechanicalsoup.StatefulBrowser() super(StatefulBrowser, br).submit(...) --- README.rst | 2 +- docs/ChangeLog.rst | 9 +++++++++ docs/tutorial.rst | 2 +- examples/example.py | 2 +- examples/expl_duck_duck_go.py | 2 +- examples/expl_google.py | 2 +- examples/expl_httpbin.py | 2 +- mechanicalsoup/form.py | 2 +- mechanicalsoup/stateful_browser.py | 11 ++++++++--- 9 files changed, 24 insertions(+), 10 deletions(-) diff --git a/README.rst b/README.rst index 2c1abcbb..214590e0 100644 --- a/README.rst +++ b/README.rst @@ -80,7 +80,7 @@ a DuckDuckGo search: # Fill-in the search form browser.select_form('#search_form_homepage') browser["q"] = "MechanicalSoup" - browser.submit_selected() + browser.submit() # Display the results for link in browser.get_current_page().select('a.result__a'): diff --git a/docs/ChangeLog.rst b/docs/ChangeLog.rst index 4dbdc55b..bb685bd3 100644 --- a/docs/ChangeLog.rst +++ b/docs/ChangeLog.rst @@ -5,6 +5,15 @@ Release Notes Version 1.0 (in development) ============================ +Main changes: +------------- + +* **Breaking Change:** The method ``StatefulBrowser.submit_selected`` has been + renamed to :func:`StatefulBrowser.submit`. The original name remains usable + for backwards compatibility. This is a breaking change _only_ if you use the + :func:`Browser.submit` method from a ``StatefulBrowser`` instance (this is + not typical), since it is now overridden by :func:`StatefulBrowser.submit`. + Bug fixes --------- diff --git a/docs/tutorial.rst b/docs/tutorial.rst index 58233edf..2e6ad22f 100644 --- a/docs/tutorial.rst +++ b/docs/tutorial.rst @@ -166,7 +166,7 @@ used to list the fields):: Assuming we're satisfied with the content of the form, we can submit it (i.e. simulate a click on the submit button):: - >>> response = browser.submit_selected() + >>> response = browser.submit() The response is not an HTML page, so the browser doesn't parse it to a BeautifulSoup object, but we can still see the text it contains:: diff --git a/examples/example.py b/examples/example.py index 0e6d2331..a057ad3e 100644 --- a/examples/example.py +++ b/examples/example.py @@ -24,7 +24,7 @@ browser.select_form('#login form') browser["login"] = args.username browser["password"] = args.password -resp = browser.submit_selected() +resp = browser.submit() # Uncomment to launch a web browser on the current page: # browser.launch_browser() diff --git a/examples/expl_duck_duck_go.py b/examples/expl_duck_duck_go.py index 800fd5d2..74ca53ee 100644 --- a/examples/expl_duck_duck_go.py +++ b/examples/expl_duck_duck_go.py @@ -10,7 +10,7 @@ # Fill-in the search form browser.select_form('#search_form_homepage') browser["q"] = "MechanicalSoup" -browser.submit_selected() +browser.submit() # Display the results for link in browser.get_current_page().select('a.result__a'): diff --git a/examples/expl_google.py b/examples/expl_google.py index 3dd99d3d..2a433990 100644 --- a/examples/expl_google.py +++ b/examples/expl_google.py @@ -10,7 +10,7 @@ browser["q"] = "MechanicalSoup" # Note: the button name is btnK in the content served to actual # browsers, but btnG for bots. -browser.submit_selected(btnName="btnG") +browser.submit(btnName="btnG") # Display links for link in browser.links(): diff --git a/examples/expl_httpbin.py b/examples/expl_httpbin.py index 7aa2cad5..9eb454ad 100644 --- a/examples/expl_httpbin.py +++ b/examples/expl_httpbin.py @@ -23,5 +23,5 @@ # Uncomment to display a summary of the filled-in form # browser.get_current_form().print_summary() -response = browser.submit_selected() +response = browser.submit() print(response.text) diff --git a/mechanicalsoup/form.py b/mechanicalsoup/form.py index f99c033c..3491fd0c 100644 --- a/mechanicalsoup/form.py +++ b/mechanicalsoup/form.py @@ -306,7 +306,7 @@ def choose_submit(self, submit): browser.open(url) form = browser.select_form() form.choose_submit('form_name_attr') - browser.submit_selected() + browser.submit() """ # Since choose_submit is destructive, it doesn't make sense to call # this method twice unless no submit is specified. diff --git a/mechanicalsoup/stateful_browser.py b/mechanicalsoup/stateful_browser.py index 0abbb996..745cef69 100644 --- a/mechanicalsoup/stateful_browser.py +++ b/mechanicalsoup/stateful_browser.py @@ -60,6 +60,10 @@ def __init__(self, *args, **kwargs): self.__verbose = 0 self.__state = _BrowserState() + # Aliases for backwards compatibility + # (Included specifically in __init__ to suppress them in Sphinx docs) + self.submit_selected = self.submit + def set_debug(self, debug): """Set the debug mode (off by default). @@ -207,7 +211,7 @@ def select_form(self, selector="form", nr=0): return self.get_current_form() - def submit_selected(self, btnName=None, *args, **kwargs): + def submit(self, btnName=None, *args, **kwargs): """Submit the form that was selected with :func:`select_form`. :return: Forwarded from :func:`Browser.submit`. @@ -225,8 +229,9 @@ def submit_selected(self, btnName=None, *args, **kwargs): else: kwargs['headers'] = {'Referer': referer} - resp = self.submit(self.__state.form, url=self.__state.url, - *args, **kwargs) + resp = super(StatefulBrowser, self).submit(self.__state.form, + url=self.__state.url, + *args, **kwargs) self.__state = _BrowserState(page=resp.soup, url=resp.url, request=resp.request) return resp From f681e7ba1df5515572f7d8bb351a8dca79ee66da Mon Sep 17 00:00:00 2001 From: Dan Hemberger Date: Wed, 29 Aug 2018 10:33:23 -0700 Subject: [PATCH 2/2] stateful_browser.py: improve submit backwards compatibility In the (likely) most common scenario of an intended call to the `Browser.submit` method from a StatefulBrowser instance, add a backwards-compatible passthrough to `Browser.submit` in the new `StatefulBrowser.submit` method while also issuing a deprecation warning. The way this works is that it checks to see if `btnName` is an instance of the `Form` class, in which case it is unambiguously intended to be a call to `Browser.submit`. This should fix the majority of cases where users may have been affected by the breaking change of overriding the `submit` method in StatefulBrowser. We cannot fix the remaining case, which is when `Browser.submit` was passed a bs4.element.Tag with name attribute "form". The reason we cannot create a similar passthrough for this is that there may be a legitimate `btnName` that is a bs4.element.Tag with name="form". This is unlikely (since submit elements typically would not be named "form"), but is technically possible. --- mechanicalsoup/stateful_browser.py | 14 ++++++++++++++ tests/test_stateful_browser.py | 15 +++++++++++++++ 2 files changed, 29 insertions(+) diff --git a/mechanicalsoup/stateful_browser.py b/mechanicalsoup/stateful_browser.py index 745cef69..6b250d71 100644 --- a/mechanicalsoup/stateful_browser.py +++ b/mechanicalsoup/stateful_browser.py @@ -7,6 +7,7 @@ import sys import re import bs4 +import warnings class _BrowserState: @@ -220,6 +221,19 @@ def submit(self, btnName=None, *args, **kwargs): to :func:`Form.choose_submit` on the current form to choose between them. All other arguments are forwarded to :func:`Browser.submit`. """ + # Temporarily allow calling the old inherited Browser.submit directly + # in case we can detect with certainty that the call is an old style. + # Note: Browser.submit also accepts a bs4.element.Tag with name="form", + # but we cannot assume this is an old-style call since there could be + # a submit button with name="form". + if isinstance(btnName, Form): + warnings.warn("This usage of StatefulBrowser.submit is deprecated." + " Please see the documentation for this function to " + "upgrade to the new interface.", + DeprecationWarning) + return super(StatefulBrowser, self).submit(btnName, *args, + **kwargs) + self.get_current_form().choose_submit(btnName) referer = self.get_url() diff --git a/tests/test_stateful_browser.py b/tests/test_stateful_browser.py index 291bec2a..4a6e855a 100644 --- a/tests/test_stateful_browser.py +++ b/tests/test_stateful_browser.py @@ -621,5 +621,20 @@ def test_refresh_error(): browser.refresh() +def test_deprecated_submit(recwarn): + """Check that calling StatefulBrowser.submit forwards to the base class + with a deprecation warning when a deprecated call is detected.""" + expected_post = [('diff', 'Review Changes'), + ('text', 'All I know is my gut says maybe')] + browser, url = setup_mock_browser(expected_post=expected_post) + browser.open(url) + form = browser.select_form('#choose-submit-form') + form.choose_submit(expected_post[0][0]) + form[expected_post[1][0]] = expected_post[1][1] + res = browser.submit(form, browser.get_url()) + assert issubclass(recwarn.pop().category, DeprecationWarning) + assert(res.status_code == 200 and res.text == 'Success!') + + if __name__ == '__main__': pytest.main(sys.argv)