Skip to content

Commit

Permalink
8028/refactor/add more typehints (#9793)
Browse files Browse the repository at this point in the history
* Add typehints in book_providers.py and app.py
* Add typehints in accounts/model.py
  • Loading branch information
yivgen authored Aug 23, 2024
1 parent bf4bc9d commit 4471420
Show file tree
Hide file tree
Showing 3 changed files with 41 additions and 23 deletions.
32 changes: 22 additions & 10 deletions openlibrary/accounts/model.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
import hmac
import random
import string
from typing import TYPE_CHECKING
from typing import TYPE_CHECKING, Any
import uuid
import logging
import requests
Expand Down Expand Up @@ -215,7 +215,7 @@ def unblock(self):
"""Unblocks this account."""
web.ctx.site.update_account(self.username, status="active")

def is_blocked(self):
def is_blocked(self) -> bool:
"""Tests if this account is blocked."""
return getattr(self, 'status', '') == "blocked"

Expand Down Expand Up @@ -308,11 +308,11 @@ def get_links(self):
type="account-link", name="username", value=self.username
)

def get_tags(self):
def get_tags(self) -> list[str]:
"""Returns list of tags that this user has."""
return self.get("tags", [])

def has_tag(self, tag):
def has_tag(self, tag: str) -> bool:
return tag in self.get_tags()

def add_tag(self, tag):
Expand Down Expand Up @@ -483,7 +483,14 @@ def create(
return ol_account

@classmethod
def get(cls, link=None, email=None, username=None, key=None, test=False):
def get(
cls,
link: str | None = None,
email: str | None = None,
username: str | None = None,
key: str | None = None,
test: bool = False,
) -> 'OpenLibraryAccount | None':
"""Utility method retrieve an openlibrary account by its email,
username or archive.org itemname (i.e. link)
"""
Expand All @@ -503,7 +510,9 @@ def get_by_key(cls, key, test=False):
return cls.get_by_username(username)

@classmethod
def get_by_username(cls, username, test=False):
def get_by_username(
cls, username: str, test: bool = False
) -> 'OpenLibraryAccount | None':
"""Retrieves and OpenLibraryAccount by username if it exists or"""
match = web.ctx.site.store.values(
type="account", name="username", value=username, limit=1
Expand All @@ -522,7 +531,7 @@ def get_by_username(cls, username, test=False):
return None

@classmethod
def get_by_link(cls, link, test=False):
def get_by_link(cls, link: str, test: bool = False) -> 'OpenLibraryAccount | None':
"""
:rtype: OpenLibraryAccount or None
"""
Expand All @@ -532,7 +541,9 @@ def get_by_link(cls, link, test=False):
return cls(ol_accounts[0]) if ol_accounts else None

@classmethod
def get_by_email(cls, email, test=False):
def get_by_email(
cls, email: str, test: bool = False
) -> 'OpenLibraryAccount | None':
"""the email stored in account doc is case-sensitive.
The lowercase of email is used in the account-email document.
querying that first and taking the username from there to make
Expand Down Expand Up @@ -932,6 +943,7 @@ def audit_accounts(


@public
def get_internet_archive_id(key):
def get_internet_archive_id(key: str) -> str | None:
username = key.split('/')[-1]
return OpenLibraryAccount.get(username=username).itemname
ol_account = OpenLibraryAccount.get(username=username)
return ol_account.itemname if ol_account else None
3 changes: 2 additions & 1 deletion openlibrary/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
from infogami.utils import app as _app
from infogami.utils.view import render, public
from infogami.utils.macro import macro
from web.template import TemplateResult


class view(_app.page):
Expand Down Expand Up @@ -63,7 +64,7 @@ class work_identifiers(delegate.view):

@macro
@public
def render_template(name, *a, **kw):
def render_template(name: str, *a, **kw) -> TemplateResult:
if "." in name:
name = name.rsplit(".", 1)[0]
return render[name](*a, **kw)
29 changes: 17 additions & 12 deletions openlibrary/book_providers.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

import web
from web import uniq
from web.template import TemplateResult

from openlibrary.app import render_template
from openlibrary.plugins.upstream.models import Edition
Expand Down Expand Up @@ -147,7 +148,7 @@ class AbstractBookProvider(Generic[TProviderMetadata]):
"""
identifier_key: str | None

def get_olids(self, identifier):
def get_olids(self, identifier: str) -> list[str]:
return web.ctx.site.things(
{"type": "/type/edition", self.db_selector: identifier}
)
Expand All @@ -157,7 +158,7 @@ def editions_query(self):
return {f"{self.db_selector}~": "*"}

@property
def db_selector(self):
def db_selector(self) -> str:
return f"identifiers.{self.identifier_key}"

@property
Expand Down Expand Up @@ -190,14 +191,16 @@ def get_template_path(self, typ: Literal['read_button', 'download_options']) ->

def render_read_button(
self, ed_or_solr: Edition | dict, analytics_attr: Callable[[str], str]
):
) -> TemplateResult:
return render_template(
self.get_template_path('read_button'),
self.get_best_identifier(ed_or_solr),
analytics_attr,
)

def render_download_options(self, edition: Edition, extra_args: list | None = None):
def render_download_options(
self, edition: Edition, extra_args: list | None = None
) -> TemplateResult:
return render_template(
self.get_template_path('download_options'),
self.get_best_identifier(edition),
Expand Down Expand Up @@ -234,11 +237,11 @@ class InternetArchiveProvider(AbstractBookProvider[IALiteMetadata]):
identifier_key = 'ocaid'

@property
def db_selector(self):
def db_selector(self) -> str:
return self.identifier_key

@property
def solr_key(self):
def solr_key(self) -> str:
return "ia"

def get_identifiers(self, ed_or_solr: Edition | dict) -> list[str]:
Expand All @@ -258,7 +261,9 @@ def get_identifiers(self, ed_or_solr: Edition | dict) -> list[str]:
def is_own_ocaid(self, ocaid: str) -> bool:
return True

def render_download_options(self, edition: Edition, extra_args: list | None = None):
def render_download_options(
self, edition: Edition, extra_args: list | None = None
) -> TemplateResult | str:
if edition.is_access_restricted():
return ''

Expand Down Expand Up @@ -430,11 +435,11 @@ class DirectProvider(AbstractBookProvider):
identifier_key = None

@property
def db_selector(self):
def db_selector(self) -> str:
return "providers.url"

@property
def solr_key(self):
def solr_key(self) -> None:
# TODO: Not implemented yet
return None

Expand Down Expand Up @@ -469,7 +474,7 @@ def get_identifiers(self, ed_or_solr: Edition | dict) -> list[str]:

def render_read_button(
self, ed_or_solr: Edition | dict, analytics_attr: Callable[[str], str]
):
) -> TemplateResult | str:
acq_sorted = sorted(
(
p
Expand Down Expand Up @@ -595,7 +600,7 @@ def get_book_provider_by_name(short_name: str) -> AbstractBookProvider | None:
prefer_ia_provider_order = uniq([ia_provider, *PROVIDER_ORDER])


def get_provider_order(prefer_ia=False) -> list[AbstractBookProvider]:
def get_provider_order(prefer_ia: bool = False) -> list[AbstractBookProvider]:
default_order = prefer_ia_provider_order if prefer_ia else PROVIDER_ORDER

provider_order = default_order
Expand Down Expand Up @@ -679,7 +684,7 @@ def get_best_edition(
return best if best else (None, None)


def get_solr_keys():
def get_solr_keys() -> list[str]:
return [p.solr_key for p in PROVIDER_ORDER if p.solr_key]


Expand Down

0 comments on commit 4471420

Please sign in to comment.