diff --git a/.github/workflows/python-app.yml b/.github/workflows/python-app.yml index 7a5196e..8aa9dee 100644 --- a/.github/workflows/python-app.yml +++ b/.github/workflows/python-app.yml @@ -17,12 +17,16 @@ jobs: runs-on: ubuntu-latest + strategy: + matrix: + python-version: ["3.8", "3.9", "3.10", "3.11", "3.12"] + steps: - uses: actions/checkout@v3 - - name: Set up Python 3.8 + - name: Set up Python ${{ matrix.python-version }} uses: actions/setup-python@v3 with: - python-version: "3.8" + python-version: ${{ matrix.python-version }} - name: Install app & dependencies run: | python -m pip install --upgrade pip diff --git a/README.md b/README.md index 245bed9..4f002fd 100644 --- a/README.md +++ b/README.md @@ -11,6 +11,10 @@ Installeer via: pip install mijnbib +Of, om een ugrade af te dwingen: + + pip install --upgrade mijnbib + ## Gebruik Bijvoorbeeld, het opvragen van je ontleende items kan als volgt (na installatie): diff --git a/changelog.md b/changelog.md index 4040182..dbf980f 100644 --- a/changelog.md +++ b/changelog.md @@ -2,6 +2,11 @@ new: new feature / impr: improvement / fix: bug fix +## v0.3.0 - 2023-12-27 + +- new: rename base exception to MijnbibError +- impr: general internal improvements + ## v0.2.0 - 2023-12-20 - new: Add extend_by_ids method diff --git a/pyproject.toml b/pyproject.toml index 3742ce5..43e0393 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "setuptools.build_meta" [project] name = "mijnbib" -version = "0.2.0" +version = "0.3.0" description = "Python API voor de website mijn.bibliotheek.be" readme = "README.md" authors = [{ name = "Ward Van Heddeghem", email = "wardvh@fastmail.fm" }] @@ -32,3 +32,10 @@ python_files = ["tests.py", "test_*.py", "*_tests.py"] [tool.isort] profile = "black" + +[tool.ruff.lint] +extend-select = [ + "W", # warning + "B", # flake8-bugbear + # "SIM", # flake8-simplify, for simplified code +] diff --git a/src/mijnbib/__init__.py b/src/mijnbib/__init__.py index 6ac22ef..10f2e3d 100644 --- a/src/mijnbib/__init__.py +++ b/src/mijnbib/__init__.py @@ -2,13 +2,14 @@ # So user can do e.g. # from mijnbib import MijnBibliotheek, Loan -from .mijnbibliotheek import MijnBibliotheek -from .models import Account, Loan, Reservation -from .plugin_errors import ( +from .errors import ( AuthenticationError, CanNotConnectError, ExtendLoanError, IncompatibleSourceError, ItemAccessError, - PluginError, + MijnbibError, + TemporarySiteError, ) +from .mijnbibliotheek import MijnBibliotheek +from .models import Account, Loan, Reservation diff --git a/src/mijnbib/__main__.py b/src/mijnbib/__main__.py index 693c1da..12807df 100644 --- a/src/mijnbib/__main__.py +++ b/src/mijnbib/__main__.py @@ -4,11 +4,25 @@ import pprint as pp import sys -from mijnbib import MijnBibliotheek +from mijnbib import AuthenticationError, MijnBibliotheek CONFIG_FILE = "mijnbib.ini" +def _do_login(args: argparse.Namespace): + print("Trying to log in ...") + + print(f"City: : {args.city}") + print(f"Username : {args.username}") + + mb = MijnBibliotheek(args.username, args.password, args.city) + try: + mb.login() + except AuthenticationError as e: + print(str(e)) + print(f"Logged in: {mb._logged_in}") + + def _do_all(args: argparse.Namespace): print("Retrieving all information ...") @@ -98,6 +112,10 @@ def main(): "reservations", parents=[common_parser], help="retrieve reservations for account id" ) parser_all.set_defaults(func=_do_reservations) + parser_all = subparsers.add_parser( + "login", parents=[common_parser], help="just log in, and report if success or not" + ) + parser_all.set_defaults(func=_do_login) # Add values from ini file as default values config = configparser.ConfigParser() diff --git a/src/mijnbib/plugin_errors.py b/src/mijnbib/errors.py similarity index 73% rename from src/mijnbib/plugin_errors.py rename to src/mijnbib/errors.py index 2845364..100b10a 100644 --- a/src/mijnbib/plugin_errors.py +++ b/src/mijnbib/errors.py @@ -1,15 +1,15 @@ -class PluginError(Exception): +class MijnbibError(Exception): """Base exception.""" # *** client-side errors *** -class AuthenticationError(PluginError): +class AuthenticationError(MijnbibError): """Raised when authentication has failed.""" -class ItemAccessError(PluginError): +class ItemAccessError(MijnbibError): """Raised when an item (loan, reservation) could not be accessed. This is likely a client-side error, but in rare cases might have a @@ -17,14 +17,14 @@ class ItemAccessError(PluginError): """ -class InvalidExtendLoanURL(PluginError): +class InvalidExtendLoanURL(MijnbibError): """Raised when the extending loan(s) url is not considered valid.""" # *** server-side errors *** -class CanNotConnectError(PluginError): +class CanNotConnectError(MijnbibError): """Raised when a url can not be reached. Args: @@ -37,7 +37,7 @@ def __init__(self, msg: str, url: str): self.url = url -class IncompatibleSourceError(PluginError): +class IncompatibleSourceError(MijnbibError): """Raised for any general errors in parsing the source. Args: @@ -50,5 +50,9 @@ def __init__(self, msg, html_body: str): self.html_body = html_body -class ExtendLoanError(PluginError): +class ExtendLoanError(MijnbibError): """Raised when extending loan(s) failed for unclear reasons.""" + + +class TemporarySiteError(MijnbibError): + """Raised when the site reports a temporary error.""" diff --git a/src/mijnbib/mijnbibliotheek.py b/src/mijnbib/mijnbibliotheek.py index b0a2933..f79dc68 100644 --- a/src/mijnbib/mijnbibliotheek.py +++ b/src/mijnbib/mijnbibliotheek.py @@ -10,25 +10,25 @@ import logging import urllib.error -import urllib.parse from dataclasses import asdict import mechanize -from mijnbib.models import Account, Loan, Reservation -from mijnbib.parsers import ( - AccountsListPageParser, - ExtendResponsePageParser, - LoansListPageParser, - ReservationsPageParser, -) -from mijnbib.plugin_errors import ( +from mijnbib.errors import ( AuthenticationError, CanNotConnectError, ExtendLoanError, IncompatibleSourceError, InvalidExtendLoanURL, ItemAccessError, + TemporarySiteError, +) +from mijnbib.models import Account, Loan, Reservation +from mijnbib.parsers import ( + AccountsListPageParser, + ExtendResponsePageParser, + LoansListPageParser, + ReservationsPageParser, ) _log = logging.getLogger(__name__) @@ -75,6 +75,7 @@ def get_loans(self, account_id: str) -> list[Loan]: AuthenticationError IncompatibleSourceError ItemAccessError: something went wrong fetching loans + TemporarySiteError """ if not self._logged_in: self.login() @@ -83,6 +84,8 @@ def get_loans(self, account_id: str) -> list[Loan]: html_string = self._open_account_loans_page(url) try: loans = LoansListPageParser(html_string, self.BASE_URL, account_id).parse() + except TemporarySiteError as e: + raise e except Exception as e: raise IncompatibleSourceError( f"Problem scraping loans ({str(e)})", html_body="" @@ -188,7 +191,7 @@ def extend_loans(self, extend_url: str, execute: bool = False) -> tuple[bool, di InvalidExtendLoanURL ExtendLoanError """ - # TODO: would make more sense to return loan list (since final page is loan page) + # NOTE: would make more sense to return loan list (since final page is loan page) # Perhaps retrieving those loans again, and check extendability would also be good idea. if not self._logged_in: self.login() @@ -198,14 +201,16 @@ def extend_loans(self, extend_url: str, execute: bool = False) -> tuple[bool, di response = self._br.open(extend_url) # pylint: disable=assignment-from-none except mechanize.HTTPError as e: if e.code == 500: - raise InvalidExtendLoanURL(f"Probably invalid extend loan URL: {extend_url}") + raise InvalidExtendLoanURL( + f"Probably invalid extend loan URL: {extend_url}" + ) from e else: raise e try: self._br.select_form(id="my-library-extend-loan-form") - except mechanize.FormNotFoundError: - raise IncompatibleSourceError("Can not find extend loan form", html_body="") + except mechanize.FormNotFoundError as e: + raise IncompatibleSourceError("Can not find extend loan form", html_body="") from e if not execute: _log.warning("SIMULATING extending the loan. Will stop now.") @@ -219,12 +224,12 @@ def extend_loans(self, extend_url: str, execute: bool = False) -> tuple[bool, di # (e.g. nonexisting id, ids that belong to different library accounts) # However, if multiple id's, some of them *might* have been extended, # even if 500 response - raise ExtendLoanError(f"Could not extend loans using url: {extend_url}") + raise ExtendLoanError(f"Could not extend loans using url: {extend_url}") from e else: raise e # disclaimer: not sure if other codes are realistic - success = True if response.code == 200 else False + success = response.code == 200 if success: _log.debug("Looks like extending the loan(s) was successful") @@ -281,16 +286,16 @@ def _log_in(self, url): self._br["email"] = self._username self._br["password"] = self._pwd response = self._br.submit() # pylint: disable=assignment-from-none - except mechanize.FormNotFoundError: + except mechanize.FormNotFoundError as e: raise IncompatibleSourceError( "Can not find login form", html_body=html_string_start_page - ) + ) from e except urllib.error.URLError as e: # We specifically catch this because site periodically (maintenance?) # throws a 500, 502 or 504 raise CanNotConnectError( f"Error while trying to log in at: {url} ({str(e)})", url - ) + ) from e return response def _validate_logged_in(self, response): @@ -316,7 +321,7 @@ def _open_account_loans_page(self, acc_url: str) -> str: if e.code == 500: # duh, server crashes on incorrect or nonexisting ID in the link raise ItemAccessError( - f"Loans url can not be opened. Likely incorrect or " + "Loans url can not be opened. Likely incorrect or " f"nonexisting account ID in the url '{acc_url}'" ) from e raise ItemAccessError( diff --git a/src/mijnbib/parsers.py b/src/mijnbib/parsers.py index f7b081b..9036154 100644 --- a/src/mijnbib/parsers.py +++ b/src/mijnbib/parsers.py @@ -8,6 +8,7 @@ from bs4 import BeautifulSoup +from mijnbib.errors import TemporarySiteError from mijnbib.models import Account, Loan, Reservation _log = logging.getLogger(__name__) @@ -110,10 +111,12 @@ def parse(self) -> list[Loan]: ) # Sometimes, this error is present if soup.find(string=re.compile(error_msg)) is not None: - # TODO: probably better to thrown an exception instead - _log.warning( + raise TemporarySiteError( f"Loans or reservations can not be retrieved. Site reports: {error_msg}" ) + # _log.warning( + # f"Loans or reservations can not be retrieved. Site reports: {error_msg}" + # ) return loans # Unfortunately, the branch names are interwoven siblings of the loans, @@ -123,7 +126,6 @@ def parse(self) -> list[Loan]: for child in children: if child.name == "h2": # we expect this to be the first child branch_name = child.get_text().strip() - # TODO: check if this resolves to the same https://github.com/myTselection/bibliotheek_be/blob/fec95c3481f78d98062c1117627da652ec8d032d/custom_components/bibliotheek_be/utils.py#L306 elif child.name == "div": # loan div # we convert child soup object to string, so called function # can be used also easily for unit tests @@ -137,9 +139,6 @@ def parse(self) -> list[Loan]: def _get_loan_info_from_div(self, loan_div_html: str, branch: str) -> Loan: """Return loan from html loan_div blob""" - self._base_url - self._acc_id - loan_div = BeautifulSoup(loan_div_html, "html.parser") loan = {} @@ -292,7 +291,17 @@ def parse(self) -> list[Account]: "div", class_="my-library-user-library-account-list__account" ) for acc_div in acc_divs: - # TODO: get details from json object, see https://github.com/myTselection/bibliotheek_be/blob/fec95c3481f78d98062c1117627da652ec8d032d/custom_components/bibliotheek_be/utils.py#L145C53-L145C75 + # Some details also available in this json blob; perhaps useful for later + # (credits to see https://github.com/myTselection/bibliotheek_be/blob/fec95c3481f78d98062c1117627da652ec8d032d/custom_components/bibliotheek_be/utils.py#L145C53-L145C75) + # {'id', 'libraryName', 'userName', 'email', 'alertEmailSync', 'barcode'} + # try: + # details = acc_div.find(attrs={":default-active-account": True}).get( + # ":default-active-account" + # ) + # details = json.loads(details) + # except (AttributeError, json.JSONDecodeError): + # details = {} + # Get id from acc_id = acc_div.a["href"].strip().split("/")[3] @@ -384,7 +393,6 @@ def _parse_item_count_from_li(acc_div, class_: str) -> int | None: class ReservationsPageParser(Parser): def __init__(self, html: str): self._html = html - # self._base_url = base_url def parse(self) -> list[Reservation]: """Return list of holds @@ -532,7 +540,7 @@ def parse(self) -> list[Reservation]: class ExtendResponsePageParser(Parser): def __init__(self, html: str): - self.html = html + self._html = html def parse(self) -> dict: """For dict structure, see the called method""" @@ -551,7 +559,7 @@ def find_between(s: str, start: str, end: str): return s[s.find(start) + len(start) : s.rfind(end)] # find relevant snippet - soup = BeautifulSoup(self.html, "html.parser") + soup = BeautifulSoup(self._html, "html.parser") script_txt = soup.find( "script", string=re.compile("(Statusbericht|Foutmelding)") ).get_text() diff --git a/tests/save_testref.py b/tests/save_testref.py index e64f6ba..a250f99 100644 --- a/tests/save_testref.py +++ b/tests/save_testref.py @@ -1,9 +1,10 @@ """This script allows to create a reference response set for mijnbibliotheek. -When ran, it create reference files, which can be used in the mijnbibliotheek -tests as expected data. When the files do not exist, the idea is that the +When ran, it create reference files, which can be used in the mijnbibliotheek +tests as expected data. When the files do not exist, the idea is that the relevant tests will be skipped. """ + import configparser import pickle import sys diff --git a/tests/test_errors.py b/tests/test_errors.py index 41f14df..0074db7 100644 --- a/tests/test_errors.py +++ b/tests/test_errors.py @@ -1,4 +1,4 @@ -from mijnbib.plugin_errors import CanNotConnectError, IncompatibleSourceError +from mijnbib.errors import CanNotConnectError, IncompatibleSourceError def test_incompatiblesourceerror(): diff --git a/tests/test_mijnbib.py b/tests/test_mijnbib.py index 2022b02..3e972f1 100644 --- a/tests/test_mijnbib.py +++ b/tests/test_mijnbib.py @@ -14,8 +14,9 @@ def test_mijnbib_available_imports(): "CanNotConnectError", "ExtendLoanError", "IncompatibleSourceError", - "PluginError", - "plugin_errors", + "MijnbibError", + "TemporarySiteError", + "errors", "mijnbibliotheek", "parsers", "models", diff --git a/tests/test_mijnbibliotheek.py b/tests/test_mijnbibliotheek.py new file mode 100644 index 0000000..02d2ef8 --- /dev/null +++ b/tests/test_mijnbibliotheek.py @@ -0,0 +1,53 @@ +import io +from typing import BinaryIO + +import pytest + +from mijnbib import MijnBibliotheek +from mijnbib.errors import AuthenticationError + + +class FakeMechanizeBrowser: + def __init__(self, form_response: str) -> None: + self._form_response = form_response.encode("utf8") + + def __setitem__(self, key, value): + pass + + def open(self, url) -> BinaryIO: + return io.BytesIO(b"some response html") + + def select_form(self, *args, **kwargs): + pass + + def submit(self, *args, **kwargs) -> BinaryIO: + return io.BytesIO(self._form_response) + + +def test_login_ok(): + mb = MijnBibliotheek("user", "pwd", "city") + mb._br = FakeMechanizeBrowser(form_response="Profiel") # type: ignore + mb.login() + + assert mb._logged_in + + +def test_login_fails(): + mb = MijnBibliotheek("user", "pwd", "city") + mb._br = FakeMechanizeBrowser(form_response="whatever") # type: ignore + + with pytest.raises(AuthenticationError, match=r".*Login not accepted.*"): + mb.login() + assert mb._logged_in is False + + +def test_login_fails_because_of_privacy(): + mb = MijnBibliotheek("user", "pwd", "city") + mb._br = FakeMechanizeBrowser(form_response="privacyverklaring is gewijzigd") # type: ignore + + with pytest.raises( + AuthenticationError, + match=r".*Login not accepted \(likely need to accept privacy statement again\).*", + ): + mb.login() + assert mb._logged_in is False