diff --git a/alembic/versions/20230719_b3749bac3e55_migrate_library_settings.py b/alembic/versions/20230719_b3749bac3e55_migrate_library_settings.py new file mode 100644 index 0000000000..09638fbeb6 --- /dev/null +++ b/alembic/versions/20230719_b3749bac3e55_migrate_library_settings.py @@ -0,0 +1,57 @@ +"""Migrate library settings + +Revision ID: b3749bac3e55 +Revises: 3d380776c1bf +Create Date: 2023-07-19 16:13:14.831349+00:00 + +""" +import sqlalchemy as sa +from sqlalchemy.dialects import postgresql + +from alembic import op +from core.configuration.library import LibrarySettings +from core.migration.migrate_external_integration import _validate_and_load_settings +from core.model import json_serializer + +# revision identifiers, used by Alembic. +revision = "b3749bac3e55" +down_revision = "3d380776c1bf" +branch_labels = None +depends_on = None + + +def upgrade() -> None: + op.add_column( + "libraries", + sa.Column( + "settings_dict", postgresql.JSONB(astext_type=sa.Text()), nullable=True + ), + ) + + connection = op.get_bind() + libraries = connection.execute("select id, short_name from libraries") + for library in libraries: + configuration_settings = connection.execute( + "select key, value from configurationsettings " + "where library_id = (%s) and external_integration_id IS NULL", + (library.id,), + ) + settings_dict = {} + for key, value in configuration_settings: + if key in ["announcements", "logo", "key-pair"]: + continue + if not value: + continue + settings_dict[key] = value + + settings = _validate_and_load_settings(LibrarySettings, settings_dict) + connection.execute( + "update libraries set settings_dict = (%s) where id = (%s)", + (json_serializer(settings.dict()), library.id), + ) + + op.alter_column("libraries", "settings_dict", nullable=False) + + +def downgrade() -> None: + op.drop_column("libraries", "settings_dict") diff --git a/api/admin/controller/library_settings.py b/api/admin/controller/library_settings.py index 9cf08ad0f0..176561c15a 100644 --- a/api/admin/controller/library_settings.py +++ b/api/admin/controller/library_settings.py @@ -1,75 +1,81 @@ +from __future__ import annotations + import base64 import json import uuid from io import BytesIO -from typing import Optional +from typing import Optional, Tuple import flask -import wcag_contrast_ratio from flask import Response from flask_babel import lazy_gettext as _ -from PIL import Image +from PIL import Image, UnidentifiedImageError from PIL.Image import Resampling from werkzeug.datastructures import FileStorage -from api.admin.announcement_list_validator import AnnouncementListValidator -from api.admin.geographic_validator import GeographicValidator from api.admin.problem_details import * -from api.config import Configuration from api.lanes import create_default_lanes -from core.model import ConfigurationSetting, Library, create, get_one +from core.configuration.library import LibrarySettings +from core.model import ( + Library, + Representation, + create, + get_one, + json_serializer, + site_configuration_has_changed, +) from core.model.announcements import SETTING_NAME as ANNOUNCEMENT_SETTING_NAME from core.model.announcements import Announcement from core.model.library import LibraryLogo -from core.util import LanguageCodes from core.util.problem_detail import ProblemDetail, ProblemError -from . import SettingsController +from ...config import Configuration +from ..announcement_list_validator import AnnouncementListValidator +from ..form_data import ProcessFormData +from . import AdminCirculationManagerController -class LibrarySettingsController(SettingsController): - def process_libraries(self): - if flask.request.method == "GET": - return self.process_get() - else: - return self.process_post() +class LibrarySettingsController(AdminCirculationManagerController): + def process_libraries(self) -> Response | ProblemDetail: + try: + if flask.request.method == "GET": + return self.process_get() + elif flask.request.method == "POST": + return self.process_post() + else: + return INCOMPLETE_CONFIGURATION + except ProblemError as e: + self._db.rollback() + return e.problem_detail - def process_get(self): - response = [] + def process_get(self) -> Response: + libraries_response = [] libraries = self._db.query(Library).order_by(Library.name).all() - ConfigurationSetting.cache_warm(self._db) for library in libraries: # Only include libraries this admin has librarian access to. - if not flask.request.admin or not flask.request.admin.is_librarian(library): + if not flask.request.admin or not flask.request.admin.is_librarian(library): # type: ignore[attr-defined] continue - settings = dict() - for setting in Configuration.LIBRARY_SETTINGS: - if setting.get("type") == "announcements": - db_announcements = ( - self._db.execute(Announcement.library_announcements(library)) - .scalars() - .all() - ) - announcements = [x.to_data().as_dict() for x in db_announcements] - if announcements: - value = json.dumps(announcements) - else: - value = None - elif setting.get("type") == "list": - value = ConfigurationSetting.for_library( - setting.get("key"), library - ).json_value - if value and setting.get("format") == "geographic": - value = self.get_extra_geographic_information(value) - else: - value = self.current_value(setting, library) - - if value: - settings[setting.get("key")] = value - - response += [ + settings = library.settings_dict + + # TODO: It would be nice to make this more sane in the future, but right now the admin interface + # is expecting the "announcements" field to be a JSON string, within the JSON document we send, + # so it ends up being double-encoded. This is a quick fix to make it work without modifying the + # admin interface. + db_announcements = ( + self._db.execute(Announcement.library_announcements(library)) + .scalars() + .all() + ) + announcements = [x.to_data().as_dict() for x in db_announcements] + if announcements: + settings["announcements"] = json.dumps(announcements) + + if library.logo: + settings["logo"] = library.logo.data_url + + libraries_response += [ dict( uuid=library.uuid, name=library.name, @@ -77,64 +83,75 @@ def process_get(self): settings=settings, ) ] - return dict(libraries=response, settings=Configuration.LIBRARY_SETTINGS) + return Response( + json_serializer( + { + "libraries": libraries_response, + "settings": LibrarySettings.configuration_form(self._db), + } + ), + status=200, + mimetype="application/json", + ) - def process_post(self, validators_by_type=None): + def process_post(self) -> Response: is_new = False - if validators_by_type is None: - validators_by_type = dict() - validators_by_type["geographic"] = GeographicValidator() + form_data = flask.request.form - library_uuid = flask.request.form.get("uuid") - library = self.get_library_from_uuid(library_uuid) - if isinstance(library, ProblemDetail): - return library + library_uuid = form_data.get("uuid") + if library_uuid: + library = self.get_library_from_uuid(library_uuid) + else: + library = None - short_name = flask.request.form.get("short_name") - short_name_not_unique = self.check_short_name_unique(library, short_name) - if short_name_not_unique: - return short_name_not_unique + name = form_data.get("name", "").strip() + if name == "": + raise ProblemError( + problem_detail=INCOMPLETE_CONFIGURATION.detailed( + "Required field 'Name' is missing." + ) + ) - error = self.validate_form_fields() - if error: - return error + short_name = form_data.get("short_name", "").strip() + if short_name == "": + raise ProblemError( + problem_detail=INCOMPLETE_CONFIGURATION.detailed( + "Required field 'Short name' is missing." + ) + ) + + self.check_short_name_unique(library, short_name) + validated_settings = ProcessFormData.get_settings(LibrarySettings, form_data) if not library: # Everyone can modify an existing library, but only a system admin can create a new one. self.require_system_admin() - (library, is_new) = self.create_library(short_name, library_uuid) + library, is_new = self.create_library(short_name) - name = flask.request.form.get("name") - if name: - library.name = name - if short_name: - library.short_name = short_name - - configuration_settings = self.library_configuration_settings( - library, validators_by_type - ) - if isinstance(configuration_settings, ProblemDetail): - return configuration_settings + library.name = name + library.short_name = short_name + library.settings_dict = validated_settings.dict() - self.scale_and_store_logo(library, flask.request.files.get(Configuration.LOGO)) + # Validate and scale logo + self.scale_and_store_logo(library, flask.request.files.get("logo")) - try: - if ANNOUNCEMENT_SETTING_NAME in flask.request.form: - validated_announcements = ( - AnnouncementListValidator().validate_announcements( - flask.request.form.get(ANNOUNCEMENT_SETTING_NAME) - ) - ) - existing_announcements = ( - self._db.execute(Announcement.library_announcements(library)) - .scalars() - .all() - ) - Announcement.sync( - self._db, existing_announcements, validated_announcements, library + if ANNOUNCEMENT_SETTING_NAME in flask.request.form: + validated_announcements = ( + AnnouncementListValidator().validate_announcements( + flask.request.form[ANNOUNCEMENT_SETTING_NAME] ) - except ProblemError as e: - return e.problem_detail + ) + existing_announcements = ( + self._db.execute(Announcement.library_announcements(library)) + .scalars() + .all() + ) + Announcement.sync( + self._db, existing_announcements, validated_announcements, library + ) + + # Trigger a site configuration change + site_configuration_has_changed(self._db) if is_new: # Now that the configuration settings are in place, create @@ -144,7 +161,7 @@ def process_post(self, validators_by_type=None): else: return Response(str(library.uuid), 200) - def create_library(self, short_name, library_uuid): + def create_library(self, short_name: str) -> Tuple[Library, bool]: self.require_system_admin() public_key, private_key = Library.generate_keypair() library, is_new = create( @@ -157,283 +174,41 @@ def create_library(self, short_name, library_uuid): ) return library, is_new - def process_delete(self, library_uuid): + def process_delete(self, library_uuid: str) -> Response: self.require_system_admin() library = self.get_library_from_uuid(library_uuid) self._db.delete(library) return Response(str(_("Deleted")), 200) - # Validation methods: - - def validate_form_fields(self): - settings = Configuration.LIBRARY_SETTINGS - validations = [ - self.check_for_missing_fields, - self.check_web_color_contrast, - self.check_header_links, - self.validate_formats, - ] - for validation in validations: - result = validation(settings) - if isinstance(result, ProblemDetail): - return result - - def check_for_missing_fields(self, settings): - if not flask.request.form.get("short_name"): - return MISSING_LIBRARY_SHORT_NAME - - error = self.check_for_missing_settings(settings) - if error: - return error - - def check_for_missing_settings(self, settings): - email_label = None - website_label = None - email_or_website = None - for s in Configuration.LIBRARY_SETTINGS: - key = s.get("key") - if s.get("required") and not s.get("default"): - if not flask.request.form.get(key): - return INCOMPLETE_CONFIGURATION.detailed( - _( - "The configuration is missing a required setting: %(setting)s", - setting=s.get("label"), - ) - ) - # Either email or website is present - if key == Configuration.HELP_EMAIL: - email_or_website = email_or_website or flask.request.form.get(key) - email_label = s.get("label") - elif key == Configuration.HELP_WEB: - email_or_website = email_or_website or flask.request.form.get(key) - website_label = s.get("label") - - if not email_or_website: - return INCOMPLETE_CONFIGURATION.detailed( - _( - "The configuration is missing a required setting: %(setting)s", - setting=f"{email_label} or {website_label}", - ) - ) - - def check_web_color_contrast(self, settings): - """ - Verify that the web primary and secondary color both contrast - well on white, as these colors will serve as button backgrounds with - white test, as well as text color on white backgrounds. - """ - primary = flask.request.form.get( - Configuration.WEB_PRIMARY_COLOR, Configuration.DEFAULT_WEB_PRIMARY_COLOR - ) - secondary = flask.request.form.get( - Configuration.WEB_SECONDARY_COLOR, Configuration.DEFAULT_WEB_SECONDARY_COLOR - ) - - def hex_to_rgb(hex): - hex = hex.lstrip("#") - return tuple(int(hex[i : i + 2], 16) / 255.0 for i in (0, 2, 4)) - - primary_passes = wcag_contrast_ratio.passes_AA( - wcag_contrast_ratio.rgb(hex_to_rgb(primary), hex_to_rgb("#ffffff")) - ) - secondary_passes = wcag_contrast_ratio.passes_AA( - wcag_contrast_ratio.rgb(hex_to_rgb(secondary), hex_to_rgb("#ffffff")) + def get_library_from_uuid(self, library_uuid: str) -> Library: + # Library UUID is required when editing an existing library + # from the admin interface, and isn't present for new libraries. + library = get_one( + self._db, + Library, + uuid=library_uuid, ) - if not (primary_passes and secondary_passes): - primary_check_url = ( - "https://contrast-ratio.com/#%23" - + secondary[1:] - + "-on-%23" - + "#ffffff"[1:] - ) - secondary_check_url = ( - "https://contrast-ratio.com/#%23" - + secondary[1:] - + "-on-%23" - + "#ffffff"[1:] - ) - return INVALID_CONFIGURATION_OPTION.detailed( - _( - "The web primary and secondary colors don't have enough contrast to pass the WCAG 2.0 AA guidelines and will be difficult for some patrons to read. Check contrast for primary here and secondary here.", - primary_check_url=primary_check_url, - secondary_check_url=secondary_check_url, - ) - ) - - def check_header_links(self, settings): - """Verify that header links and labels are the same length.""" - header_links = flask.request.form.getlist(Configuration.WEB_HEADER_LINKS) - header_labels = flask.request.form.getlist(Configuration.WEB_HEADER_LABELS) - if len(header_links) != len(header_labels): - return INVALID_CONFIGURATION_OPTION.detailed( - _( - "There must be the same number of web header links and web header labels." - ) - ) - - def get_library_from_uuid(self, library_uuid): - if library_uuid: - # Library UUID is required when editing an existing library - # from the admin interface, and isn't present for new libraries. - library = get_one( - self._db, - Library, - uuid=library_uuid, - ) - if library: - return library - else: - return LIBRARY_NOT_FOUND.detailed( + if library: + return library + else: + raise ProblemError( + problem_detail=LIBRARY_NOT_FOUND.detailed( _("The specified library uuid does not exist.") ) + ) - def check_short_name_unique(self, library, short_name): - if not library or short_name != library.short_name: + def check_short_name_unique( + self, library: Optional[Library], short_name: Optional[str] + ) -> None: + if not library or (short_name and short_name != library.short_name): # If you're adding a new short_name, either by editing an # existing library or creating a new library, it must be unique. library_with_short_name = get_one(self._db, Library, short_name=short_name) if library_with_short_name: - return LIBRARY_SHORT_NAME_ALREADY_IN_USE - - # Configuration settings: - - def get_extra_geographic_information(self, value): - validator = GeographicValidator() - - for country in value: - zips = [x for x in value[country] if validator.is_zip(x, country)] - other = [x for x in value[country] if not x in zips] - zips_with_extra_info = [] - for zip in zips: - info = validator.look_up_zip(zip, country, formatted=True) - zips_with_extra_info.append(info) - value[country] = zips_with_extra_info + other - - return value - - def library_configuration_settings( - self, library, validators_by_format, settings=None - ): - """Validate and update a library's configuration settings based on incoming new - values. - - :param library: A Library - :param validators_by_format: A dictionary mapping the 'format' field from a setting - configuration to a corresponding validator object. - :param settings: A list of setting configurations to use in tests instead of - Configuration.LIBRARY_SETTINGS - """ - settings = settings or Configuration.LIBRARY_SETTINGS - for setting in settings: - if setting.get("key") == ANNOUNCEMENT_SETTING_NAME: - # Announcement is a special case -- it's not a library setting - # but stored in its own table in the database. - continue - - # Validate the incoming value. - validator = None - if "format" in setting: - validator = validators_by_format.get(setting["format"]) - elif "type" in setting: - validator = validators_by_format.get(setting["type"]) - validated_value = self._validate_setting(library, setting, validator) - - if isinstance(validated_value, ProblemDetail): - # Validation failed -- return a ProblemDetail. - return validated_value - - # Validation succeeded -- set the new value. - ConfigurationSetting.for_library( - setting["key"], library - ).value = self._format_validated_value(validated_value, validator) - - def _validate_setting(self, library, setting, validator=None): - """Validate the incoming value for a single library setting. - - :param library: A Library - :param setting: Configuration data for one of the library's settings. - :param validator: A validation object for data of this type. - """ - # TODO: there are some opportunities for improvement here: - # * There's no standard interface for validators. - # * We can handle settings that are lists of certain types (language codes, - # geographic areas), but not settings that are a single value of that type - # (_one_ language code or geographic area). Sometimes there's even an implication - # that a certain data type ('geographic') _must_ mean a list. - # * A list value is returned as a JSON-encoded string. It - # would be better to keep that as a list for longer in case - # controller code needs to look at it. - format = setting.get("format") - type = setting.get("type") - - # In some cases, if there is no incoming value we can use a - # default value or the current value. - # - # When the configuration item is a list, we can't do this - # because an empty list may be a valid value. - current_value = self.current_value(setting, library) - default_value = setting.get("default") or current_value - - if format == "geographic": - value = self.list_setting(setting) - value = validator.validate_geographic_areas(value, self._db) - elif type == "announcements": - value = self.list_setting(setting, json_objects=True) - value = validator.validate_announcements(value) - elif type == "list": - value = self.list_setting(setting) - if format == "language-code": - value = json.dumps( - [ - LanguageCodes.string_to_alpha_3(language) - for language in json.loads(value) - ] - ) - else: - value = self.scalar_setting(setting) - # An empty "" value or 0 value is valid, hence check for None - value = default_value if value is None else value - return value - - def scalar_setting(self, setting): - """Retrieve the single value of the given setting from the current HTTP request.""" - return flask.request.form.get(setting["key"]) - - def list_setting(self, setting, json_objects=False): - """Retrieve the list of values for the given setting from the current HTTP request. - - :param json_objects: If this is True, the incoming settings are JSON-encoded objects and not - regular strings. - - :return: A JSON-encoded string encoding the list of values set for the given setting in the - current request. - """ - if setting.get("options"): - # Restrict to the values in 'options'. - value = [] - for option in setting.get("options"): - if setting["key"] + "_" + option["key"] in flask.request.form: - value += [option["key"]] - else: - # Allow any entered values. - value = [] - if setting.get("type") == "list": - inputs = flask.request.form.getlist(setting.get("key")) - else: - inputs = flask.request.form.get(setting.get("key")) - - if json_objects and inputs: - inputs = json.loads(inputs) - if inputs: - for i in inputs: - if not isinstance(i, list): - i = [i] - value.extend(i) - - return json.dumps([_f for _f in value if _f]) + raise ProblemError(problem_detail=LIBRARY_SHORT_NAME_ALREADY_IN_USE) @staticmethod - def _process_image(image: Image.Image, _format="PNG") -> bytes: + def _process_image(image: Image.Image, _format: str = "PNG") -> bytes: """Convert PIL image to RGBA if necessary and return it as base64 encoded bytes. """ @@ -450,12 +225,32 @@ def scale_and_store_logo( cls, library: Library, image_file: Optional[FileStorage], - max_dimension=Configuration.LOGO_MAX_DIMENSION, + max_dimension: int = Configuration.LOGO_MAX_DIMENSION, ) -> None: if not image_file: return None - image = Image.open(image_file.stream) + allowed_types = [ + Representation.JPEG_MEDIA_TYPE, + Representation.PNG_MEDIA_TYPE, + Representation.GIF_MEDIA_TYPE, + ] + image_type = image_file.headers.get("Content-Type") + if image_type not in allowed_types: + raise ProblemError( + INVALID_CONFIGURATION_OPTION.detailed( + f"Image upload must be in GIF, PNG, or JPG format. (Upload was {image_type}.)" + ) + ) + + try: + image = Image.open(image_file.stream) + except UnidentifiedImageError: + raise ProblemError( + INVALID_CONFIGURATION_OPTION.detailed( + f"Unable to open uploaded image, please try again or upload a different image." + ) + ) width, height = image.size if width > max_dimension or height > max_dimension: image.thumbnail((max_dimension, max_dimension), Resampling.LANCZOS) @@ -465,15 +260,3 @@ def scale_and_store_logo( library.logo.content = image_data else: library.logo = LibraryLogo(content=image_data) - - def current_value(self, setting, library): - """Retrieve the current value of the given setting from the database.""" - return ConfigurationSetting.for_library(setting["key"], library).value - - @classmethod - def _format_validated_value(cls, value, validator=None): - """Convert a validated value to a string that can be stored in ConfigurationSetting.value""" - if not validator: - # Assume the value is already a string. - return value - return validator.format_as_string(value) diff --git a/api/admin/dashboard_stats.py b/api/admin/dashboard_stats.py index 3f9ffeae4f..f5a9973c74 100644 --- a/api/admin/dashboard_stats.py +++ b/api/admin/dashboard_stats.py @@ -198,7 +198,11 @@ def stats(self, admin: Admin) -> StatisticsResponse: patron_statistics=patron_stats_by_library[lib.short_name], inventory_summary=inventory_by_library[lib.short_name], collection_ids=sorted( - [c.id for c in authorized_collections_by_library[lib.short_name]] + [ + c.id + for c in authorized_collections_by_library[lib.short_name] + if c.id is not None + ] ), ) for lib in authorized_libraries diff --git a/api/admin/geographic_validator.py b/api/admin/geographic_validator.py deleted file mode 100644 index 501f4d4d90..0000000000 --- a/api/admin/geographic_validator.py +++ /dev/null @@ -1,204 +0,0 @@ -import json -import os -import re -import urllib.error -import urllib.parse -import urllib.request - -import uszipcode -from flask_babel import lazy_gettext as _ -from pypostalcode import PostalCodeDatabase - -from api.admin.exceptions import * -from api.admin.validator import Validator -from api.problem_details import * -from api.registration.registry import RemoteRegistry -from core.model import ExternalIntegration -from core.util.http import HTTP -from core.util.problem_detail import ProblemDetail - - -class GeographicValidator(Validator): - @staticmethod - def get_us_search(): - # Use a known path for the uszipcode db_file_dir that already contains the DB that the - # library would otherwise download. This is done because the host for this file can - # be flaky. There is an issue for this in the underlying library here: - # https://github.com/MacHu-GWU/uszipcode-project/issues/40 - db_file_path = os.path.join( - os.path.dirname(__file__), "..", "..", "data", "uszipcode" - ) - return uszipcode.SearchEngine(simple_zipcode=True, db_file_dir=db_file_path) - - def validate_geographic_areas(self, values, db): - # Note: the validator does not recognize data from US territories other than Puerto Rico. - - us_search = self.get_us_search() - ca_search = PostalCodeDatabase() - CA_PROVINCES = { - "AB": "Alberta", - "BC": "British Columbia", - "MB": "Manitoba", - "NB": "New Brunswick", - "NL": "Newfoundland and Labrador", - "NT": "Northwest Territories", - "NS": "Nova Scotia", - "NU": "Nunavut", - "ON": "Ontario", - "PE": "Prince Edward Island", - "QC": "Quebec", - "SK": "Saskatchewan", - "YT": "Yukon Territories", - } - - locations = {"US": [], "CA": []} - - for value in json.loads(values): - flagged = False - if value == "everywhere": - locations["US"].append(value) - elif len(value) and isinstance(value, str): - if len(value) == 2: - # Is it a US state or Canadian province abbreviation? - if value in CA_PROVINCES: - locations["CA"].append(CA_PROVINCES[value]) - elif len(us_search.query(state=value)): - locations["US"].append(value) - else: - return UNKNOWN_LOCATION.detailed( - _( - '"%(value)s" is not a valid U.S. state or Canadian province abbreviation.', - value=value, - ) - ) - elif value in list(CA_PROVINCES.values()): - locations["CA"].append(value) - elif self.is_zip(value, "CA"): - # Is it a Canadian zipcode? - try: - info = self.look_up_zip(value, "CA") - formatted = f"{info.city}, {info.province}" - # In some cases--mainly involving very small towns--even if the zip code is valid, - # the registry won't recognize the name of the place to which it corresponds. - registry_response = self.find_location_through_registry( - formatted, db - ) - if registry_response: - locations["CA"].append(formatted) - else: - return UNKNOWN_LOCATION.detailed( - _( - 'Unable to locate "%(value)s" (%(formatted)s). Try entering the name of a larger area.', - value=value, - formatted=formatted, - ) - ) - except: - return UNKNOWN_LOCATION.detailed( - _( - '"%(value)s" is not a valid Canadian zipcode.', - value=value, - ) - ) - elif len(value.split(", ")) == 2: - # Is it in the format "[city], [state abbreviation]" or "[county], [state abbreviation]"? - city_or_county, state = value.split(", ") - if us_search.by_city_and_state(city_or_county, state): - locations["US"].append(value) - elif len( - [ - x - for x in us_search.query(state=state, returns=None) - if x.county == city_or_county - ] - ): - locations["US"].append(value) - else: - # Flag this as needing to be checked with the registry - flagged = True - elif self.is_zip(value, "US"): - # Is it a US zipcode? - info = self.look_up_zip(value, "US") - if not info: - return UNKNOWN_LOCATION.detailed( - _('"%(value)s" is not a valid U.S. zipcode.', value=value) - ) - locations["US"].append(value) - else: - flagged = True - - if flagged: - registry_response = self.find_location_through_registry(value, db) - if registry_response and isinstance( - registry_response, ProblemDetail - ): - return registry_response - elif registry_response: - locations[registry_response].append(value) - else: - return UNKNOWN_LOCATION.detailed( - _('Unable to locate "%(value)s".', value=value) - ) - return locations - - def is_zip(self, value, country): - if country == "US": - return len(value) == 5 and value.isdigit() - elif country == "CA": - return len(value) == 3 and bool(re.search("^[A-Za-z]\\d[A-Za-z]", value)) - - def look_up_zip(self, zip, country, formatted=False): - if country == "US": - info = self.get_us_search().by_zipcode(zip) - if formatted: - info = self.format_place(zip, info.major_city, info.state) - elif country == "CA": - info = PostalCodeDatabase()[zip] - if formatted: - info = self.format_place(zip, info.city, info.province) - return info - - def format_place(self, zip, city, state_or_province): - details = f"{city}, {state_or_province}" - return {zip: details} - - def find_location_through_registry(self, value, db): - for nation in ["US", "CA"]: - service_area_object = urllib.parse.quote(f'{{"{nation}": "{value}"}}') - registry_check = self.ask_registry(service_area_object, db) - if registry_check and isinstance(registry_check, ProblemDetail): - return registry_check - elif registry_check: - # If the registry has established that this is a US location, don't bother also trying to find it in Canada - return nation - - def ask_registry(self, service_area_object, db, do_get=HTTP.debuggable_get): - # If the circulation manager doesn't know about this location, check whether the Library Registry does. - result = None - for registry in RemoteRegistry.for_protocol_and_goal( - db, - ExternalIntegration.OPDS_REGISTRATION, - ExternalIntegration.DISCOVERY_GOAL, - ): - base_url = registry.integration.url + "/coverage?coverage=" - - response = do_get(base_url + service_area_object) - if not response.status_code == 200: - result = REMOTE_INTEGRATION_FAILED.detailed( - _( - "Unable to contact the registry at %(url)s.", - url=registry.integration.url, - ) - ) - - if hasattr(response, "content"): - content = json.loads(response.content) - found_place = not (content.get("unknown") or content.get("ambiguous")) - if found_place: - return True - - return result - - def format_as_string(self, value): - """Format the output of validate_geographic_areas for storage in ConfigurationSetting.value.""" - return json.dumps(value) diff --git a/api/admin/problem_details.py b/api/admin/problem_details.py index f010bd2ece..aa8d508272 100644 --- a/api/admin/problem_details.py +++ b/api/admin/problem_details.py @@ -138,13 +138,6 @@ detail=_("The submitted image is invalid."), ) -MISSING_LIBRARY_SHORT_NAME = pd( - "http://librarysimplified.org/terms/problem/missing-library-short-name", - status_code=400, - title=_("Missing library short name"), - detail=_("You must set a short name for the library."), -) - LIBRARY_SHORT_NAME_ALREADY_IN_USE = pd( "http://librarysimplified.org/terms/problem/library-short-name-already-in-use", status_code=400, diff --git a/api/admin/validator.py b/api/admin/validator.py index 4a7dbf9105..8751f09839 100644 --- a/api/admin/validator.py +++ b/api/admin/validator.py @@ -3,8 +3,6 @@ from flask_babel import lazy_gettext as _ from api.admin.exceptions import * -from core.model import Representation -from core.util import LanguageCodes class Validator: @@ -13,8 +11,6 @@ def validate(self, settings, content): self.validate_email, self.validate_url, self.validate_number, - self.validate_language_code, - self.validate_image, ] for validator in validators: @@ -139,51 +135,6 @@ def _number_error(self, field, number): ) ) - def validate_language_code(self, settings, content): - # Find the fields that should contain language codes and are not blank. - language_inputs = self._extract_inputs( - settings, "language-code", content.get("form"), is_list=True - ) - for language in language_inputs: - if not self._is_language(language): - return UNKNOWN_LANGUAGE.detailed( - _('"%(language)s" is not a valid language code.', language=language) - ) - - def _is_language(self, language): - # Check that the input string is in the list of recognized language codes. - return LanguageCodes.string_to_alpha_3(language) - - def validate_image(self, settings, content): - # Find the fields that contain image uploads and are not blank. - files = content.get("files") - if files: - image_inputs = self._extract_inputs( - settings, "image", files, key="type", should_zip=True - ) - - for setting, image in image_inputs: - invalid_format = self._image_format_error(image) - if invalid_format: - return INVALID_CONFIGURATION_OPTION.detailed( - _( - "Upload for %(setting)s must be in GIF, PNG, or JPG format. (Upload was %(format)s.)", - setting=setting.get("label"), - format=invalid_format, - ) - ) - - def _image_format_error(self, image_file): - # Check that the uploaded image is in an acceptable format. - allowed_types = [ - Representation.JPEG_MEDIA_TYPE, - Representation.PNG_MEDIA_TYPE, - Representation.GIF_MEDIA_TYPE, - ] - image_type = image_file.headers.get("Content-Type") - if not image_type in allowed_types: - return image_type - def _list_of_values(self, fields, form): result = [] for field in fields: diff --git a/api/adobe_vendor_id.py b/api/adobe_vendor_id.py index d81986d43f..fd9723c272 100644 --- a/api/adobe_vendor_id.py +++ b/api/adobe_vendor_id.py @@ -24,7 +24,7 @@ ) from core.util.datetime_helpers import datetime_utc, utc_now -from .config import CannotLoadConfiguration, Configuration +from .config import CannotLoadConfiguration if sys.version_info >= (3, 11): from typing import Self @@ -156,9 +156,7 @@ def from_config( else: return None - library_uri = ConfigurationSetting.for_library( - Configuration.WEBSITE_URL, library - ).value + library_uri = library.settings.website vendor_id = integration.setting(cls.VENDOR_ID_KEY).value library_short_name = ConfigurationSetting.for_library_and_externalintegration( diff --git a/api/authenticator.py b/api/authenticator.py index ba93839741..d693f9da52 100644 --- a/api/authenticator.py +++ b/api/authenticator.py @@ -4,7 +4,7 @@ import logging import sys from abc import ABC -from typing import Dict, Iterable, List, Literal, Optional, Tuple, Type +from typing import Dict, Iterable, List, Optional, Tuple, Type import flask import jwt @@ -20,7 +20,6 @@ from api.authentication.basic import BasicAuthenticationProvider from api.authentication.basic_token import BasicTokenAuthenticationProvider from api.custom_patron_catalog import CustomPatronCatalog -from api.opds import LibraryAnnotator from core.analytics import Analytics from core.integration.goals import Goals from core.integration.registry import IntegrationRegistry @@ -559,25 +558,74 @@ def create_authentication_document(self) -> str: """Create the Authentication For OPDS document to be used when a request comes in with no authentication. """ - links = [] - library = self.library - - # Add the same links that we would show in an OPDS feed, plus - # some extra like 'registration' that are specific to Authentication - # For OPDS. - for rel in ( - LibraryAnnotator.CONFIGURATION_LINKS - + Configuration.AUTHENTICATION_FOR_OPDS_LINKS - ): - value = ConfigurationSetting.for_library(rel, library).value - if not value: - continue - link = dict(rel=rel, href=value) - if any(value.startswith(x) for x in ("http:", "https:")): - # We assume that HTTP URLs lead to HTML, but we don't - # assume anything about other URL schemes. - link["type"] = "text/html" - links.append(link) + links: List[Dict[str, Optional[str]]] = [] + if self.library is None: + raise ValueError("No library specified!") + + # Add the same links that we would show in an OPDS feed. + if self.library.settings.terms_of_service: + links.append( + dict( + rel="terms-of-service", + href=self.library.settings.terms_of_service, + type="text/html", + ) + ) + + if self.library.settings.privacy_policy: + links.append( + dict( + rel="privacy-policy", + href=self.library.settings.privacy_policy, + type="text/html", + ) + ) + + if self.library.settings.copyright: + links.append( + dict( + rel="copyright", + href=self.library.settings.copyright, + type="text/html", + ) + ) + + if self.library.settings.about: + links.append( + dict( + rel="about", + href=self.library.settings.about, + type="text/html", + ) + ) + + if self.library.settings.license: + links.append( + dict( + rel="license", + href=self.library.settings.license, + type="text/html", + ) + ) + + # Plus some extra like 'registration' that are specific to Authentication For OPDS. + if self.library.settings.registration_url: + links.append( + dict( + rel="register", + href=self.library.settings.registration_url, + type="text/html", + ) + ) + + if self.library.settings.patron_password_reset: + links.append( + dict( + rel="http://librarysimplified.org/terms/rel/patron-password-reset", + href=self.library.settings.patron_password_reset, + type="text/html", + ) + ) # Add a rel="start" link pointing to the root OPDS feed. index_url = url_for( @@ -610,35 +658,35 @@ def create_authentication_document(self) -> str: # If there is a Designated Agent email address, add it as a # link. - designated_agent_uri = Configuration.copyright_designated_agent_uri(library) + designated_agent_uri = Configuration.copyright_designated_agent_uri( + self.library + ) if designated_agent_uri: links.append( dict( - rel=Configuration.COPYRIGHT_DESIGNATED_AGENT_REL, + rel="http://librarysimplified.org/rel/designated-agent/copyright", href=designated_agent_uri, ) ) # Add a rel="help" link for every type of URL scheme that # leads to library-specific help. - for type, uri in Configuration.help_uris(library): + for type, uri in Configuration.help_uris(self.library): links.append(dict(rel="help", href=uri, type=type)) # Add a link to the web page of the library itself. - library_uri = ConfigurationSetting.for_library( - Configuration.WEBSITE_URL, library - ).value + library_uri = self.library.settings.website if library_uri: links.append(dict(rel="alternate", type="text/html", href=library_uri)) # Add the library's logo, if it has one. - if library and library.logo: - links.append(dict(rel="logo", type="image/png", href=library.logo.data_url)) + if self.library and self.library.logo: + links.append( + dict(rel="logo", type="image/png", href=self.library.logo.data_url) + ) # Add the library's custom CSS file, if it has one. - css_file = ConfigurationSetting.for_library( - Configuration.WEB_CSS_FILE, library - ).value + css_file = self.library.settings.web_css_file if css_file: links.append(dict(rel="stylesheet", type="text/css", href=css_file)) @@ -652,19 +700,13 @@ def create_authentication_document(self) -> str: ).to_dict(self._db) # Add the library's mobile color scheme, if it has one. - description = ConfigurationSetting.for_library( - Configuration.COLOR_SCHEME, library - ).value - if description: - doc["color_scheme"] = description + color_scheme = self.library.settings.color_scheme + if color_scheme: + doc["color_scheme"] = color_scheme # Add the library's web colors, if it has any. - primary = ConfigurationSetting.for_library( - Configuration.WEB_PRIMARY_COLOR, library - ).value - secondary = ConfigurationSetting.for_library( - Configuration.WEB_SECONDARY_COLOR, library - ).value + primary = self.library.settings.web_primary_color + secondary = self.library.settings.web_secondary_color if primary or secondary: doc["web_color_scheme"] = dict( primary=primary, @@ -675,20 +717,10 @@ def create_authentication_document(self) -> str: # Add the description of the library as the OPDS feed's # service_description. - description = ConfigurationSetting.for_library( - Configuration.LIBRARY_DESCRIPTION, library - ).value + description = self.library.settings.library_description if description: doc["service_description"] = description - # Add the library's focus area and service area, if either is - # specified. - focus_area, service_area = self._geographic_areas(library) - if focus_area: - doc["focus_area"] = focus_area - if service_area: - doc["service_area"] = service_area - # Add the library's public key. if self.library and self.library.public_key: doc["public_key"] = dict(type="RSA", value=self.library.public_key) @@ -706,7 +738,7 @@ def create_authentication_document(self) -> str: # offer. enabled: List[str] = [] disabled: List[str] = [] - if library and library.allow_holds: + if self.library and self.library.settings.allow_holds: bucket = enabled else: bucket = disabled @@ -714,9 +746,9 @@ def create_authentication_document(self) -> str: doc["features"] = dict(enabled=enabled, disabled=disabled) # Add any active announcements for the library. - if library: + if self.library: doc["announcements"] = Announcement.authentication_document_announcements( - library + self.library ) # Finally, give the active annotator a chance to modify the document. @@ -724,63 +756,12 @@ def create_authentication_document(self) -> str: if self.authentication_document_annotator: doc = ( self.authentication_document_annotator.annotate_authentication_document( - library, doc, url_for + self.library, doc, url_for ) ) return json.dumps(doc) - @classmethod - def _geographic_areas( - cls, library: Optional[Library] - ) -> Tuple[ - Literal["everywhere"] | List[str] | None, - Literal["everywhere"] | List[str] | None, - ]: - """Determine the library's focus area and service area. - - :param library: A Library - :return: A 2-tuple (focus_area, service_area) - """ - if not library: - return None, None - - focus_area = cls._geographic_area(Configuration.LIBRARY_FOCUS_AREA, library) - service_area = cls._geographic_area(Configuration.LIBRARY_SERVICE_AREA, library) - - # If only one value is provided, both values are considered to - # be the same. - if focus_area and not service_area: - service_area = focus_area - if service_area and not focus_area: - focus_area = service_area - return focus_area, service_area - - @classmethod - def _geographic_area( - cls, key: str, library: Library - ) -> Literal["everywhere"] | List[str] | None: - """Extract a geographic area from a ConfigurationSetting - for the given `library`. - - See https://github.com/NYPL-Simplified/Simplified/wiki/Authentication-For-OPDS-Extensions#service_area and #focus_area - """ - setting = ConfigurationSetting.for_library(key, library).value - if not setting: - return setting - if setting == "everywhere": - # This literal string may be served as is. - return setting - try: - # If we can load the setting as JSON, it is either a list - # of place names or a GeoJSON object. - setting = json.loads(setting) - except (ValueError, TypeError) as e: - # The most common outcome -- treat the value as a single place - # name by turning it into a list. - setting = [setting] - return setting - def create_authentication_headers(self) -> Headers: """Create the HTTP headers to return with the OPDS authentication document.""" diff --git a/api/circulation.py b/api/circulation.py index 5b9f78691f..2c9c303944 100644 --- a/api/circulation.py +++ b/api/circulation.py @@ -26,7 +26,6 @@ from core.model import ( CirculationEvent, Collection, - ConfigurationSetting, DeliveryMechanism, ExternalIntegration, ExternalIntegrationLink, @@ -44,7 +43,6 @@ from core.util.datetime_helpers import utc_now from .circulation_exceptions import * -from .config import Configuration from .util.patron import PatronUtility @@ -606,17 +604,19 @@ def internal_format(self, delivery_mechanism): return internal_format @classmethod - def default_notification_email_address(self, library_or_patron, pin): + def default_notification_email_address( + self, library_or_patron: Library | Patron, pin: str + ) -> str: """What email address should be used to notify this library's patrons of changes? :param library_or_patron: A Library or a Patron. """ if isinstance(library_or_patron, Patron): - library_or_patron = library_or_patron.library - return ConfigurationSetting.for_library( - Configuration.DEFAULT_NOTIFICATION_EMAIL_ADDRESS, library_or_patron - ).value + library = library_or_patron.library + else: + library = library_or_patron + return library.settings.default_notification_email_address @classmethod def _library_authenticator(self, library): @@ -1369,7 +1369,7 @@ def enforce_limits(self, patron, pool): # This patron can neither take out a loan or place a hold. # Raise PatronLoanLimitReached for the most understandable # error message. - raise PatronLoanLimitReached(library=patron.library) + raise PatronLoanLimitReached(limit=patron.library.settings.loan_limit) # At this point it's important that we get up-to-date # availability information about this LicensePool, to reduce @@ -1380,11 +1380,11 @@ def enforce_limits(self, patron, pool): currently_available = pool.licenses_available > 0 if currently_available and at_loan_limit: - raise PatronLoanLimitReached(library=patron.library) + raise PatronLoanLimitReached(limit=patron.library.settings.loan_limit) if not currently_available and at_hold_limit: - raise PatronHoldLimitReached(library=patron.library) + raise PatronHoldLimitReached(limit=patron.library.settings.hold_limit) - def patron_at_loan_limit(self, patron): + def patron_at_loan_limit(self, patron: Patron) -> bool: """Is the given patron at their loan limit? This doesn't belong in Patron because the loan limit is not core functionality. @@ -1392,8 +1392,8 @@ def patron_at_loan_limit(self, patron): :param patron: A Patron. """ - loan_limit = patron.library.setting(Configuration.LOAN_LIMIT).int_value - if loan_limit is None: + loan_limit = patron.library.settings.loan_limit + if not loan_limit: return False # Open-access loans, and loans of indefinite duration, don't count towards the loan limit @@ -1403,7 +1403,7 @@ def patron_at_loan_limit(self, patron): for loan in patron.loans if loan.license_pool and loan.license_pool.open_access == False and loan.end ] - return loan_limit and len(non_open_access_loans_with_end_date) >= loan_limit + return len(non_open_access_loans_with_end_date) >= loan_limit def patron_at_hold_limit(self, patron): """Is the given patron at their hold limit? @@ -1413,10 +1413,10 @@ def patron_at_hold_limit(self, patron): :param patron: A Patron. """ - hold_limit = patron.library.setting(Configuration.HOLD_LIMIT).int_value - if hold_limit is None: + hold_limit = patron.library.settings.hold_limit + if not hold_limit: return False - return hold_limit and len(patron.holds) >= hold_limit + return len(patron.holds) >= hold_limit def can_fulfill_without_loan(self, patron, pool, lpdm): """Can we deliver the given book in the given format to the given diff --git a/api/circulation_exceptions.py b/api/circulation_exceptions.py index 2ac9c00296..8b04aa6cc5 100644 --- a/api/circulation_exceptions.py +++ b/api/circulation_exceptions.py @@ -2,7 +2,6 @@ from flask_babel import lazy_gettext as _ -from api.config import Configuration from core.config import IntegrationException from core.problem_details import INTEGRATION_ERROR, INTERNAL_SERVER_ERROR from core.util.problem_detail import ProblemDetail @@ -140,25 +139,17 @@ class LimitReached(CirculationException): This exception cannot be used on its own. It must be subclassed and the following constants defined: * `BASE_DOC`: A ProblemDetail, used as the basis for conversion of this exception into a problem detail document. - * `SETTING_NAME`: Then name of the library-specific ConfigurationSetting whose numeric - value is the limit that cannot be exceeded. * `MESSAGE_WITH_LIMIT` A string containing the interpolation value "%(limit)s", which offers a more specific explanation of the limit exceeded. """ status_code = 403 BASE_DOC: Optional[ProblemDetail] = None - SETTING_NAME: Optional[str] = None MESSAGE_WITH_LIMIT = None - def __init__(self, message=None, debug_info=None, library=None, limit=None): + def __init__(self, message=None, debug_info=None, limit=None): super().__init__(message=message, debug_info=debug_info) - if library: - self.limit = library.setting(self.SETTING_NAME).int_value - elif limit: - self.limit = limit - else: - self.limit = None + self.limit = limit if limit else None def as_problem_detail_document(self, debug=False): """Return a suitable problem detail document.""" @@ -172,7 +163,6 @@ def as_problem_detail_document(self, debug=False): class PatronLoanLimitReached(CannotLoan, LimitReached): BASE_DOC = LOAN_LIMIT_REACHED MESSAGE_WITH_LIMIT = SPECIFIC_LOAN_LIMIT_MESSAGE - SETTING_NAME = Configuration.LOAN_LIMIT class CannotReturn(CirculationException): @@ -186,7 +176,6 @@ class CannotHold(CirculationException): class PatronHoldLimitReached(CannotHold, LimitReached): BASE_DOC = HOLD_LIMIT_REACHED MESSAGE_WITH_LIMIT = SPECIFIC_HOLD_LIMIT_MESSAGE - SETTING_NAME = Configuration.HOLD_LIMIT class CannotReleaseHold(CirculationException): diff --git a/api/config.py b/api/config.py index 4986ec7fcd..b77a568329 100644 --- a/api/config.py +++ b/api/config.py @@ -1,16 +1,17 @@ -import json +from typing import Iterable, List, Optional, Tuple from Crypto.Cipher import PKCS1_OAEP from Crypto.Cipher.PKCS1_OAEP import PKCS1OAEP_Cipher from Crypto.PublicKey import RSA from flask_babel import lazy_gettext as _ +from money import Money from core.config import CannotLoadConfiguration # noqa: autoflake from core.config import IntegrationException # noqa: autoflake from core.config import Configuration as CoreConfiguration -from core.model import ConfigurationSetting, announcements +from core.configuration.library import LibrarySettings +from core.model import Library from core.model.announcements import SETTING_NAME as ANNOUNCEMENT_SETTING_NAME -from core.model.constants import LinkRelations from core.util import MoneyUtility @@ -47,103 +48,13 @@ class Configuration(CoreConfiguration): "Terms of Service for presenting content through the Palace client applications" ) - # A short description of the library, used in its Authentication - # for OPDS document. - LIBRARY_DESCRIPTION = "library_description" - - # The name of the per-library setting that sets the maximum amount - # of fines a patron can have before losing lending privileges. - MAX_OUTSTANDING_FINES = "max_outstanding_fines" - - # The name of the per-library settings that set the maximum amounts - # of books a patron can have on loan or on hold at once. - # (Note: depending on distributor settings, a patron may be able - # to exceed the limits by checking out books directly from a distributor's - # app. They may also get a limit exceeded error before they reach these - # limits if a distributor has a smaller limit.) - LOAN_LIMIT = "loan_limit" - HOLD_LIMIT = "hold_limit" - - # The name of the per-library setting that sets the default email - # address to use when notifying patrons of changes. - DEFAULT_NOTIFICATION_EMAIL_ADDRESS = "default_notification_email_address" - STANDARD_NOREPLY_EMAIL_ADDRESS = "noreply@thepalaceproject.org" - - # The name of the per-library setting that sets the email address - # of the Designated Agent for copyright complaints - COPYRIGHT_DESIGNATED_AGENT_EMAIL = "copyright_designated_agent_email_address" - - # This is the link relation used to indicate - COPYRIGHT_DESIGNATED_AGENT_REL = ( - "http://librarysimplified.org/rel/designated-agent/copyright" - ) - - # The name of the per-library setting that sets the contact address - # for problems with the library configuration itself. - CONFIGURATION_CONTACT_EMAIL = "configuration_contact_email_address" - # Name of the site-wide ConfigurationSetting containing the secret # used to sign bearer tokens. BEARER_TOKEN_SIGNING_SECRET = "bearer_token_signing_secret" - # Names of per-library ConfigurationSettings that control - # how detailed the lane configuration gets for various languages. - LARGE_COLLECTION_LANGUAGES = "large_collections" - SMALL_COLLECTION_LANGUAGES = "small_collections" - TINY_COLLECTION_LANGUAGES = "tiny_collections" - - LANGUAGE_DESCRIPTION = _( - 'Each value can be either the full name of a language or an ISO-639-2 language code.' - ) - - HIDDEN_CONTENT_TYPES = "hidden_content_types" - - # The color scheme for native mobile applications to use for this library. - COLOR_SCHEME = "color_scheme" - DEFAULT_COLOR_SCHEME = "blue" - - # The color options for web applications to use for this library. - WEB_PRIMARY_COLOR = "web-primary-color" - WEB_SECONDARY_COLOR = "web-secondary-color" - DEFAULT_WEB_PRIMARY_COLOR = "#377F8B" - DEFAULT_WEB_SECONDARY_COLOR = "#D53F34" - - # A link to a CSS file for customizing the catalog display in web applications. - WEB_CSS_FILE = "web-css-file" - - # Header links and labels for web applications to display for this library. - # TODO: It's very awkward to have these as separate settings, and separate - # lists of inputs in the UI. - WEB_HEADER_LINKS = "web-header-links" - WEB_HEADER_LABELS = "web-header-labels" - - # The library-wide logo setting. - LOGO = "logo" # Maximum height and width for the saved logo image LOGO_MAX_DIMENSION = 135 - # Settings for geographic areas associated with the library. - LIBRARY_FOCUS_AREA = "focus_area" - LIBRARY_SERVICE_AREA = "service_area" - AREA_INPUT_INSTRUCTIONS = _( - """
    Accepted formats: -
  1. US zipcode or Canadian FSA
    2. -
  2. Two-letter US state abbreviation
    3. -
  3. City, US state abbreviatione.g. "Boston, MA"
    4. -
  4. County, US state abbreviatione.g. "Litchfield County, CT"
    5. -
  5. Canadian province name or two-letter abbreviation
    6. -
  6. City, Canadian province name/abbreviatione.g. "Stratford, Ontario"/"Stratford, ON"
    7. -
""" - ) - - # Names of the library-wide link settings. - TERMS_OF_SERVICE = "terms-of-service" - PRIVACY_POLICY = "privacy-policy" - COPYRIGHT = "copyright" - ABOUT = "about" - LICENSE = "license" - REGISTER = "register" - # A library with this many titles in a given language will be given # a large, detailed lane configuration for that language. LARGE_COLLECTION_CUTOFF = 10000 @@ -153,17 +64,6 @@ class Configuration(CoreConfiguration): # A library with fewer titles than that will be given a single # lane containing all books in that language. - # These are link relations that are valid in Authentication for - # OPDS documents but are not registered with IANA. - AUTHENTICATION_FOR_OPDS_LINKS = ["register", LinkRelations.PATRON_PASSWORD_RESET] - - # We support three different ways of integrating help processes. - # All three of these will be sent out as links with rel='help' - HELP_EMAIL = "help-email" - HELP_WEB = "help-web" - HELP_URI = "help-uri" - HELP_LINKS = [HELP_EMAIL, HELP_WEB, HELP_URI] - # Features of an OPDS client which a library may want to enable or # disable. RESERVATIONS_FEATURE = "https://librarysimplified.org/rel/policy/reservations" @@ -222,343 +122,6 @@ class Configuration(CoreConfiguration): }, ] - # The "level" property determines which admins will be able to modify the setting. Level 1 settings can be modified by anyone. - # Level 2 settings can be modified only by library managers and system admins (i.e. not by librarians). Level 3 settings can be changed only by system admins. - # If no level is specified, the setting will be treated as Level 1 by default. - - LIBRARY_SETTINGS = CoreConfiguration.LIBRARY_SETTINGS + [ - { - "key": LIBRARY_DESCRIPTION, - "label": _("A short description of this library"), - "description": _( - "This will be shown to people who aren't sure they've chosen the right library." - ), - "category": "Basic Information", - "level": CoreConfiguration.SYS_ADMIN_ONLY, - }, - { - "key": announcements.SETTING_NAME, - "label": _("Scheduled announcements"), - "description": _( - "Announcements will be displayed to authenticated patrons." - ), - "category": "Announcements", - "type": "announcements", - "level": CoreConfiguration.ALL_ACCESS, - }, - { - "key": HELP_EMAIL, - "label": _("Patron support email address"), - "description": _( - "An email address a patron can use if they need help, e.g. 'simplyehelp@yourlibrary.org'." - ), - "category": "Basic Information", - "format": "email", - "level": CoreConfiguration.SYS_ADMIN_ONLY, - }, - { - "key": HELP_WEB, - "label": _("Patron support web site"), - "description": _( - "A URL for patrons to get help. Either this field or patron support email address must be provided." - ), - "format": "url", - "category": "Basic Information", - "level": CoreConfiguration.ALL_ACCESS, - }, - { - "key": HELP_URI, - "label": _("Patron support custom integration URI"), - "description": _( - "A custom help integration like Helpstack, e.g. 'helpstack:nypl.desk.com'." - ), - "category": "Patron Support", - "level": CoreConfiguration.SYS_ADMIN_ONLY, - }, - { - "key": COPYRIGHT_DESIGNATED_AGENT_EMAIL, - "label": _("Copyright designated agent email"), - "description": _( - "Patrons of this library should use this email address to send a DMCA notification (or other copyright complaint) to the library.
If no value is specified here, the general patron support address will be used." - ), - "format": "email", - "category": "Patron Support", - "level": CoreConfiguration.SYS_ADMIN_OR_MANAGER, - }, - { - "key": CONFIGURATION_CONTACT_EMAIL, - "label": _( - "A point of contact for the organization reponsible for configuring this library" - ), - "description": _( - "This email address will be shared as part of integrations that you set up through this interface. It will not be shared with the general public. This gives the administrator of the remote integration a way to contact you about problems with this library's use of that integration.
If no value is specified here, the general patron support address will be used." - ), - "format": "email", - "category": "Patron Support", - "level": CoreConfiguration.SYS_ADMIN_OR_MANAGER, - }, - { - "key": DEFAULT_NOTIFICATION_EMAIL_ADDRESS, - "label": _("Write-only email address for vendor hold notifications"), - "description": _( - 'This address must trash all email sent to it. Vendor hold notifications contain sensitive patron information, but cannot be forwarded to patrons because they contain vendor-specific instructions.
The default address will work, but for greater security, set up your own address that trashes all incoming email.' - ), - "default": STANDARD_NOREPLY_EMAIL_ADDRESS, - "required": True, - "format": "email", - "level": CoreConfiguration.SYS_ADMIN_ONLY, - }, - { - "key": COLOR_SCHEME, - "label": _("Mobile color scheme"), - "description": _( - "This tells mobile applications what color scheme to use when rendering this library's OPDS feed." - ), - "options": [ - dict(key="amber", label=_("Amber")), - dict(key="black", label=_("Black")), - dict(key="blue", label=_("Blue")), - dict(key="bluegray", label=_("Blue Gray")), - dict(key="brown", label=_("Brown")), - dict(key="cyan", label=_("Cyan")), - dict(key="darkorange", label=_("Dark Orange")), - dict(key="darkpurple", label=_("Dark Purple")), - dict(key="green", label=_("Green")), - dict(key="gray", label=_("Gray")), - dict(key="indigo", label=_("Indigo")), - dict(key="lightblue", label=_("Light Blue")), - dict(key="orange", label=_("Orange")), - dict(key="pink", label=_("Pink")), - dict(key="purple", label=_("Purple")), - dict(key="red", label=_("Red")), - dict(key="teal", label=_("Teal")), - ], - "type": "select", - "default": DEFAULT_COLOR_SCHEME, - "category": "Client Interface Customization", - "level": CoreConfiguration.SYS_ADMIN_OR_MANAGER, - }, - { - "key": WEB_PRIMARY_COLOR, - "label": _("Web primary color"), - "description": _( - "This is the brand primary color for the web application. Must have sufficient contrast with white." - ), - "type": "color-picker", - "default": DEFAULT_WEB_PRIMARY_COLOR, - "category": "Client Interface Customization", - "level": CoreConfiguration.SYS_ADMIN_OR_MANAGER, - }, - { - "key": WEB_SECONDARY_COLOR, - "label": _("Web secondary color"), - "description": _( - "This is the brand secondary color for the web application. Must have sufficient contrast with white." - ), - "type": "color-picker", - "default": DEFAULT_WEB_SECONDARY_COLOR, - "category": "Client Interface Customization", - "level": CoreConfiguration.SYS_ADMIN_OR_MANAGER, - }, - { - "key": WEB_CSS_FILE, - "label": _("Custom CSS file for web"), - "description": _( - "Give web applications a CSS file to customize the catalog display." - ), - "format": "url", - "category": "Client Interface Customization", - "level": CoreConfiguration.SYS_ADMIN_ONLY, - }, - { - "key": WEB_HEADER_LINKS, - "label": _("Web header links"), - "description": _( - "This gives web applications a list of links to display in the header. Specify labels for each link in the same order under 'Web header labels'." - ), - "type": "list", - "category": "Client Interface Customization", - "level": CoreConfiguration.SYS_ADMIN_OR_MANAGER, - }, - { - "key": WEB_HEADER_LABELS, - "label": _("Web header labels"), - "description": _("Labels for each link under 'Web header links'."), - "type": "list", - "category": "Client Interface Customization", - "level": CoreConfiguration.SYS_ADMIN_OR_MANAGER, - }, - { - "key": LOGO, - "label": _("Logo image"), - "type": "image", - "description": _( - "The image should be in GIF, PNG, or JPG format, approximately square, no larger than " - f"{LOGO_MAX_DIMENSION}x{LOGO_MAX_DIMENSION} pixels, " - "and look good on a light or dark mode background. " - "Larger images will be accepted, but scaled down (maintaining aspect ratio) such that " - f"the longest dimension does not exceed {LOGO_MAX_DIMENSION} pixels." - ), - "category": "Client Interface Customization", - "level": CoreConfiguration.ALL_ACCESS, - }, - { - "key": HIDDEN_CONTENT_TYPES, - "label": _("Hidden content types"), - "type": "text", - "description": _( - 'A list of content types to hide from all clients, e.g. ["application/pdf"]. This can be left blank except to solve specific problems.' - ), - "category": "Client Interface Customization", - "level": CoreConfiguration.SYS_ADMIN_ONLY, - }, - { - "key": LIBRARY_FOCUS_AREA, - "label": _("Focus area"), - "type": "list", - "description": _( - "The library focuses on serving patrons in this geographic area. In most cases this will be a city name like Springfield, OR." - ), - "category": "Geographic Areas", - "format": "geographic", - "instructions": AREA_INPUT_INSTRUCTIONS, - "capitalize": True, - "level": CoreConfiguration.ALL_ACCESS, - }, - { - "key": LIBRARY_SERVICE_AREA, - "label": _("Service area"), - "type": "list", - "description": _( - "The full geographic area served by this library. In most cases this is the same as the focus area and can be left blank, but it may be a larger area such as a US state (which should be indicated by its abbreviation, like OR)." - ), - "category": "Geographic Areas", - "format": "geographic", - "instructions": AREA_INPUT_INSTRUCTIONS, - "capitalize": True, - "level": CoreConfiguration.ALL_ACCESS, - }, - { - "key": MAX_OUTSTANDING_FINES, - "label": _( - "Maximum amount in fines a patron can have before losing lending privileges" - ), - "type": "number", - "category": "Loans, Holds, & Fines", - "level": CoreConfiguration.ALL_ACCESS, - }, - { - "key": LOAN_LIMIT, - "label": _("Maximum number of books a patron can have on loan at once"), - "description": _( - "(Note: depending on distributor settings, a patron may be able to exceed the limit by checking out books directly from a distributor's app. They may also get a limit exceeded error before they reach these limits if a distributor has a smaller limit.)" - ), - "type": "number", - "category": "Loans, Holds, & Fines", - "level": CoreConfiguration.ALL_ACCESS, - }, - { - "key": HOLD_LIMIT, - "label": _("Maximum number of books a patron can have on hold at once"), - "description": _( - "(Note: depending on distributor settings, a patron may be able to exceed the limit by checking out books directly from a distributor's app. They may also get a limit exceeded error before they reach these limits if a distributor has a smaller limit.)" - ), - "type": "number", - "category": "Loans, Holds, & Fines", - "level": CoreConfiguration.ALL_ACCESS, - }, - { - "key": TERMS_OF_SERVICE, - "label": _("Terms of Service URL"), - "format": "url", - "category": "Links", - "level": CoreConfiguration.ALL_ACCESS, - }, - { - "key": PRIVACY_POLICY, - "label": _("Privacy Policy URL"), - "format": "url", - "category": "Links", - "level": CoreConfiguration.ALL_ACCESS, - }, - { - "key": COPYRIGHT, - "label": _("Copyright URL"), - "format": "url", - "category": "Links", - "level": CoreConfiguration.SYS_ADMIN_OR_MANAGER, - }, - { - "key": ABOUT, - "label": _("About URL"), - "format": "url", - "category": "Links", - "level": CoreConfiguration.ALL_ACCESS, - }, - { - "key": LICENSE, - "label": _("License URL"), - "format": "url", - "category": "Links", - "level": CoreConfiguration.SYS_ADMIN_OR_MANAGER, - }, - { - "key": REGISTER, - "label": _("Patron registration URL"), - "description": _( - "A URL where someone who doesn't have a library card yet can sign up for one." - ), - "format": "url", - "category": "Patron Support", - "allowed": ["nypl.card-creator:https://patrons.librarysimplified.org/"], - "level": CoreConfiguration.ALL_ACCESS, - }, - { - "key": LinkRelations.PATRON_PASSWORD_RESET, - "label": _("Password Reset Link"), - "description": _( - "A link to a web page where a user can reset their virtual library card password" - ), - "format": "url", - "category": "Patron Support", - "level": CoreConfiguration.SYS_ADMIN_ONLY, - }, - { - "key": LARGE_COLLECTION_LANGUAGES, - "label": _( - "The primary languages represented in this library's collection" - ), - "type": "list", - "format": "language-code", - "description": LANGUAGE_DESCRIPTION, - "optional": True, - "category": "Languages", - "level": CoreConfiguration.ALL_ACCESS, - }, - { - "key": SMALL_COLLECTION_LANGUAGES, - "label": _( - "Other major languages represented in this library's collection" - ), - "type": "list", - "format": "language-code", - "description": LANGUAGE_DESCRIPTION, - "optional": True, - "category": "Languages", - "level": CoreConfiguration.ALL_ACCESS, - }, - { - "key": TINY_COLLECTION_LANGUAGES, - "label": _("Other languages in this library's collection"), - "type": "list", - "format": "language-code", - "description": LANGUAGE_DESCRIPTION, - "optional": True, - "category": "Languages", - "level": CoreConfiguration.ALL_ACCESS, - }, - ] - ANNOUNCEMENT_SETTINGS = [ { "key": ANNOUNCEMENT_SETTING_NAME, @@ -568,53 +131,48 @@ class Configuration(CoreConfiguration): ), "category": "Announcements", "type": "announcements", - "level": CoreConfiguration.ALL_ACCESS, }, ] @classmethod - def _collection_languages(cls, library, key): - """Look up a list of languages in a library configuration. - - If the value is not set, estimate a value (and all related - values) by looking at the library's collection. - """ - setting = ConfigurationSetting.for_library(key, library) - value = None - try: - value = setting.json_value - if not isinstance(value, list): - value = None - except (TypeError, ValueError): - pass - - if value is None: - # We have no value or a bad value. Estimate a better value. + def estimate_language_collections_when_unset(cls, library: Library) -> None: + settings = library.settings + if ( + settings.large_collection_languages is None + and settings.small_collection_languages is None + and settings.tiny_collection_languages is None + ): cls.estimate_language_collections_for_library(library) - value = setting.json_value - return value @classmethod - def large_collection_languages(cls, library): - return cls._collection_languages(library, cls.LARGE_COLLECTION_LANGUAGES) + def large_collection_languages(cls, library: Library) -> List[str]: + cls.estimate_language_collections_when_unset(library) + if library.settings.large_collection_languages is None: + return [] + return library.settings.large_collection_languages @classmethod - def small_collection_languages(cls, library): - return cls._collection_languages(library, cls.SMALL_COLLECTION_LANGUAGES) + def small_collection_languages(cls, library: Library) -> List[str]: + cls.estimate_language_collections_when_unset(library) + if library.settings.small_collection_languages is None: + return [] + return library.settings.small_collection_languages @classmethod - def tiny_collection_languages(cls, library): - return cls._collection_languages(library, cls.TINY_COLLECTION_LANGUAGES) + def tiny_collection_languages(cls, library: Library) -> List[str]: + cls.estimate_language_collections_when_unset(library) + if library.settings.tiny_collection_languages is None: + return [] + return library.settings.tiny_collection_languages @classmethod - def max_outstanding_fines(cls, library): - max_fines = ConfigurationSetting.for_library(cls.MAX_OUTSTANDING_FINES, library) - if max_fines.value is None: + def max_outstanding_fines(cls, library: Library) -> Optional[Money]: + if library.settings.max_outstanding_fines is None: return None - return MoneyUtility.parse(max_fines.value) + return MoneyUtility.parse(library.settings.max_outstanding_fines) @classmethod - def estimate_language_collections_for_library(cls, library): + def estimate_language_collections_for_library(cls, library: Library) -> None: """Guess at appropriate values for the given library for LARGE_COLLECTION_LANGUAGES, SMALL_COLLECTION_LANGUAGES, and TINY_COLLECTION_LANGUAGES. Set configuration values @@ -622,12 +180,12 @@ def estimate_language_collections_for_library(cls, library): """ holdings = library.estimated_holdings_by_language() large, small, tiny = cls.classify_holdings(holdings) - for setting, value in ( - (cls.LARGE_COLLECTION_LANGUAGES, large), - (cls.SMALL_COLLECTION_LANGUAGES, small), - (cls.TINY_COLLECTION_LANGUAGES, tiny), - ): - ConfigurationSetting.for_library(setting, library).value = json.dumps(value) + settings = LibrarySettings.construct( + large_collection_languages=large, + small_collection_languages=small, + tiny_collection_languages=tiny, + ) + library.update_settings(settings) @classmethod def classify_holdings(cls, works_by_language): @@ -680,49 +238,38 @@ def _as_mailto(cls, value): return "mailto:%s" % value @classmethod - def help_uris(cls, library): + def help_uris(cls, library: Library) -> Iterable[Tuple[Optional[str], str]]: """Find all the URIs that might help patrons get help from this library. :yield: A sequence of 2-tuples (media type, URL) """ - for name in cls.HELP_LINKS: - setting = ConfigurationSetting.for_library(name, library) - value = setting.value - if not value: - continue - type = None - if name == cls.HELP_EMAIL: - value = cls._as_mailto(value) - if name == cls.HELP_WEB: - type = "text/html" - yield type, value + if library.settings.help_email: + yield None, cls._as_mailto(library.settings.help_email) + if library.settings.help_web: + yield "text/html", library.settings.help_web @classmethod - def _email_uri_with_fallback(cls, library, key): - """Try to find a certain email address configured for the given - purpose. If not available, use the general patron support - address. + def copyright_designated_agent_uri(cls, library: Library) -> Optional[str]: + if library.settings.copyright_designated_agent_email_address: + email = library.settings.copyright_designated_agent_email_address + elif library.settings.help_email: + email = library.settings.help_email + else: + return None - :param key: The specific email address to look for. - """ - for setting in [key, Configuration.HELP_EMAIL]: - value = ConfigurationSetting.for_library(setting, library).value - if not value: - continue - return cls._as_mailto(value) + return cls._as_mailto(email) @classmethod - def copyright_designated_agent_uri(cls, library): - return cls._email_uri_with_fallback( - library, Configuration.COPYRIGHT_DESIGNATED_AGENT_EMAIL - ) + def configuration_contact_uri(cls, library: Library) -> Optional[str]: + if library.settings.configuration_contact_email_address: + email = library.settings.configuration_contact_email_address + elif library.settings.help_email: + email = library.settings.help_email + else: + return None - @classmethod - def configuration_contact_uri(cls, library): - return cls._email_uri_with_fallback( - library, Configuration.CONFIGURATION_CONTACT_EMAIL - ) + return cls._as_mailto(email) @classmethod def cipher(cls, key: bytes) -> PKCS1OAEP_Cipher: diff --git a/api/controller.py b/api/controller.py index 63405dc513..c40ab48324 100644 --- a/api/controller.py +++ b/api/controller.py @@ -770,7 +770,7 @@ def apply_borrowing_policy(self, patron, license_pool): return NOT_AGE_APPROPRIATE if ( - not patron.library.allow_holds + not patron.library.settings.allow_holds and license_pool.licenses_available == 0 and not license_pool.open_access and not license_pool.unlimited_access @@ -897,7 +897,7 @@ def groups(self, lane_identifier, feed_class=AcquisitionFeed): return self.feed(lane_identifier, feed_class) facet_class_kwargs = dict( - minimum_featured_quality=library.minimum_featured_quality, + minimum_featured_quality=library.settings.minimum_featured_quality, ) facets = self.manager.load_facets_from_request( worklist=lane, @@ -989,7 +989,7 @@ def navigation(self, lane_identifier): title = lane.display_name facet_class_kwargs = dict( - minimum_featured_quality=library.minimum_featured_quality, + minimum_featured_quality=library.settings.minimum_featured_quality, ) facets = self.manager.load_facets_from_request( worklist=lane, @@ -2191,7 +2191,7 @@ def related( worklist=lane, base_class=FeaturedFacets, base_class_constructor_kwargs=dict( - minimum_featured_quality=library.minimum_featured_quality + minimum_featured_quality=library.settings.minimum_featured_quality ), ) if isinstance(facets, ProblemDetail): diff --git a/api/opds.py b/api/opds.py index 3cac5fa5a9..e7d8e46787 100644 --- a/api/opds.py +++ b/api/opds.py @@ -22,10 +22,10 @@ from core.model import ( CirculationEvent, Collection, - ConfigurationSetting, DeliveryMechanism, Edition, Hold, + Library, LicensePool, LicensePoolDeliveryMechanism, Loan, @@ -556,27 +556,6 @@ def _single_entry_response( class LibraryAnnotator(CirculationManagerAnnotator): - TERMS_OF_SERVICE = Configuration.TERMS_OF_SERVICE - PRIVACY_POLICY = Configuration.PRIVACY_POLICY - COPYRIGHT = Configuration.COPYRIGHT - ABOUT = Configuration.ABOUT - LICENSE = Configuration.LICENSE - REGISTER = Configuration.REGISTER - - CONFIGURATION_LINKS = [ - TERMS_OF_SERVICE, - PRIVACY_POLICY, - COPYRIGHT, - ABOUT, - LICENSE, - ] - - HELP_LINKS = [ - Configuration.HELP_EMAIL, - Configuration.HELP_WEB, - Configuration.HELP_URI, - ] - def __init__( self, circulation, @@ -611,11 +590,11 @@ def __init__( active_loans_by_work=active_loans_by_work, active_holds_by_work=active_holds_by_work, active_fulfillments_by_work=active_fulfillments_by_work, - hidden_content_types=self._hidden_content_types(library), + hidden_content_types=library.settings.hidden_content_types, test_mode=test_mode, ) self.circulation = circulation - self.library = library + self.library: Library = library self.patron = patron self.lanes_by_work = defaultdict(list) self.facet_view = facet_view @@ -624,30 +603,6 @@ def __init__( self.identifies_patrons = library_identifies_patrons self.facets = facets or None - @classmethod - def _hidden_content_types(self, library): - """Find all content types which this library should not be - presenting to patrons. - - This is stored as a per-library setting. - """ - if not library: - # This shouldn't happen, but we shouldn't crash if it does. - return [] - setting = library.setting(Configuration.HIDDEN_CONTENT_TYPES) - if not setting or not setting.value: - return [] - try: - hidden_types = setting.json_value - except ValueError: - hidden_types = setting.value - hidden_types = hidden_types or [] - if isinstance(hidden_types, str): - hidden_types = [hidden_types] - elif not isinstance(hidden_types, list): - hidden_types = list(hidden_types) - return hidden_types - def top_level_title(self): return self._top_level_title @@ -1040,28 +995,63 @@ def _add_link(l): link = OPDSFeed.link(**l) feed.append(link) - for rel in self.CONFIGURATION_LINKS: - setting = ConfigurationSetting.for_library(rel, self.library) - if setting.value: - d = dict(href=setting.value, type="text/html", rel=rel) - _add_link(d) - - navigation_urls = ConfigurationSetting.for_library( - Configuration.WEB_HEADER_LINKS, self.library - ).json_value - if navigation_urls: - navigation_labels = ConfigurationSetting.for_library( - Configuration.WEB_HEADER_LABELS, self.library - ).json_value - for url, label in zip(navigation_urls, navigation_labels): - d = dict( - href=url, - title=label, + library = self.library + if library.settings.terms_of_service: + _add_link( + dict( + rel="terms-of-service", + href=library.settings.terms_of_service, type="text/html", - rel="related", - role="navigation", ) - _add_link(d) + ) + + if library.settings.privacy_policy: + _add_link( + dict( + rel="privacy-policy", + href=library.settings.privacy_policy, + type="text/html", + ) + ) + + if library.settings.copyright: + _add_link( + dict( + rel="copyright", + href=library.settings.copyright, + type="text/html", + ) + ) + + if library.settings.about: + _add_link( + dict( + rel="about", + href=library.settings.about, + type="text/html", + ) + ) + + if library.settings.license: + _add_link( + dict( + rel="license", + href=library.settings.license, + type="text/html", + ) + ) + + navigation_urls = self.library.settings.web_header_links + navigation_labels = self.library.settings.web_header_labels + for url, label in zip(navigation_urls, navigation_labels): + d = dict( + href=url, + title=label, + type="text/html", + rel="related", + role="navigation", + ) + _add_link(d) for type, value in Configuration.help_uris(self.library): d = dict(href=value, rel="help") @@ -1136,7 +1126,7 @@ def acquisition_links( active_fulfillment, feed, identifier, - can_hold=self.library.allow_holds, + can_hold=self.library.settings.allow_holds, can_revoke_hold=( active_hold and ( diff --git a/core/config.py b/core/config.py index 808cad395b..689d55a5e6 100644 --- a/core/config.py +++ b/core/config.py @@ -11,8 +11,6 @@ # from this module, alongside CannotLoadConfiguration. from core.exceptions import IntegrationException -from .entrypoint import EntryPoint -from .facets import FacetConstants from .util import LanguageCodes from .util.datetime_helpers import to_utc, utc_now @@ -30,19 +28,6 @@ class CannotLoadConfiguration(IntegrationException): class ConfigurationConstants: - # Each facet group has two associated per-library keys: one - # configuring which facets are enabled for that facet group, and - # one configuring which facet is the default. - ENABLED_FACETS_KEY_PREFIX = "facets_enabled_" - DEFAULT_FACET_KEY_PREFIX = "facets_default_" - - # The "level" property determines which admins will be able to modify the setting. Level 1 settings can be modified by anyone. - # Level 2 settings can be modified only by library managers and system admins (i.e. not by librarians). Level 3 settings can be changed only by system admins. - # If no level is specified, the setting will be treated as Level 1 by default. - ALL_ACCESS = 1 - SYS_ADMIN_OR_MANAGER = 2 - SYS_ADMIN_ONLY = 3 - TRUE = "true" FALSE = "false" @@ -84,23 +69,8 @@ class Configuration(ConfigurationConstants): URL = "url" INTEGRATIONS = "integrations" - # The name of the per-library configuration policy that controls whether - # books may be put on hold. - ALLOW_HOLDS = "allow_holds" - - # Each library may set a minimum quality for the books that show - # up in the 'featured' lanes that show up on the front page. - MINIMUM_FEATURED_QUALITY = "minimum_featured_quality" DEFAULT_MINIMUM_FEATURED_QUALITY = 0.65 - # Each library may configure the maximum number of books in the - # 'featured' lanes. - FEATURED_LANE_SIZE = "featured_lane_size" - - WEBSITE_URL = "website" - NAME = "name" - SHORT_NAME = "short_name" - DEBUG = "DEBUG" INFO = "INFO" WARN = "WARN" @@ -190,132 +160,6 @@ class Configuration(ConfigurationConstants): }, ] - LIBRARY_SETTINGS = ( - [ - { - "key": NAME, - "label": _("Name"), - "description": _("The human-readable name of this library."), - "category": "Basic Information", - "level": ConfigurationConstants.SYS_ADMIN_ONLY, - "required": True, - }, - { - "key": SHORT_NAME, - "label": _("Short name"), - "description": _( - "A short name of this library, to use when identifying it in scripts or URLs, e.g. 'NYPL'." - ), - "category": "Basic Information", - "level": ConfigurationConstants.SYS_ADMIN_ONLY, - "required": True, - }, - { - "key": WEBSITE_URL, - "label": _("URL of the library's website"), - "description": _( - "The library's main website, e.g. \"https://www.nypl.org/\" (not this Circulation Manager's URL)." - ), - "required": True, - "format": "url", - "level": ConfigurationConstants.SYS_ADMIN_ONLY, - "category": "Basic Information", - }, - { - "key": ALLOW_HOLDS, - "label": _("Allow books to be put on hold"), - "type": "select", - "options": [ - {"key": "true", "label": _("Allow holds")}, - {"key": "false", "label": _("Disable holds")}, - ], - "default": "true", - "category": "Loans, Holds, & Fines", - "level": ConfigurationConstants.SYS_ADMIN_ONLY, - }, - { - "key": EntryPoint.ENABLED_SETTING, - "label": _("Enabled entry points"), - "description": _( - "Patrons will see the selected entry points at the top level and in search results.

Currently supported audiobook vendors: Bibliotheca, Axis 360" - ), - "type": "list", - "options": [ - { - "key": entrypoint.INTERNAL_NAME, - "label": EntryPoint.DISPLAY_TITLES.get(entrypoint), - } - for entrypoint in EntryPoint.ENTRY_POINTS - ], - "default": [x.INTERNAL_NAME for x in EntryPoint.DEFAULT_ENABLED], - "category": "Lanes & Filters", - # Renders a component with options that get narrowed down as the user makes selections. - "format": "narrow", - # Renders an input field that cannot be edited. - "readOnly": True, - "level": ConfigurationConstants.SYS_ADMIN_ONLY, - }, - { - "key": FEATURED_LANE_SIZE, - "label": _("Maximum number of books in the 'featured' lanes"), - "type": "number", - "default": 15, - "category": "Lanes & Filters", - "level": ConfigurationConstants.ALL_ACCESS, - }, - { - "key": MINIMUM_FEATURED_QUALITY, - "label": _( - "Minimum quality for books that show up in 'featured' lanes" - ), - "description": _("Between 0 and 1."), - "type": "number", - "max": 1, - "default": DEFAULT_MINIMUM_FEATURED_QUALITY, - "category": "Lanes & Filters", - "level": ConfigurationConstants.ALL_ACCESS, - }, - ] - + [ - { - "key": ConfigurationConstants.ENABLED_FACETS_KEY_PREFIX + group, - "label": description, - "type": "list", - "options": [ - { - "key": facet, - "label": FacetConstants.FACET_DISPLAY_TITLES.get(facet), - } - for facet in FacetConstants.FACETS_BY_GROUP.get(group, []) - ], - "default": FacetConstants.FACETS_BY_GROUP.get(group), - "category": "Lanes & Filters", - # Tells the front end that each of these settings is related to the corresponding default setting. - "paired": ConfigurationConstants.DEFAULT_FACET_KEY_PREFIX + group, - "level": ConfigurationConstants.SYS_ADMIN_OR_MANAGER, - } - for group, description in FacetConstants.GROUP_DESCRIPTIONS.items() - ] - + [ - { - "key": ConfigurationConstants.DEFAULT_FACET_KEY_PREFIX + group, - "label": _("Default %(group)s", group=display_name), - "type": "select", - "options": [ - { - "key": facet, - "label": FacetConstants.FACET_DISPLAY_TITLES.get(facet), - } - for facet in FacetConstants.FACETS_BY_GROUP.get(group, []) - ], - "default": FacetConstants.DEFAULT_FACET.get(group), - "category": "Lanes & Filters", - "skip": True, - } - for group, display_name in FacetConstants.GROUP_DISPLAY_TITLES.items() - ] - ) - @classmethod def database_url(cls): """Find the database URL configured for this site. diff --git a/core/configuration/library.py b/core/configuration/library.py new file mode 100644 index 0000000000..f5330e27de --- /dev/null +++ b/core/configuration/library.py @@ -0,0 +1,649 @@ +from __future__ import annotations + +from dataclasses import dataclass +from enum import IntEnum +from typing import Any, Dict, List, Optional, Tuple + +import wcag_contrast_ratio +from pydantic import ( + ConstrainedFloat, + EmailStr, + HttpUrl, + PositiveFloat, + PositiveInt, + root_validator, + validator, +) +from pydantic.fields import ModelField +from sqlalchemy.orm import Session + +from api.admin.problem_details import ( + INCOMPLETE_CONFIGURATION, + INVALID_CONFIGURATION_OPTION, + UNKNOWN_LANGUAGE, +) +from core.config import Configuration +from core.entrypoint import EntryPoint +from core.facets import FacetConstants +from core.integration.settings import ( + BaseSettings, + ConfigurationFormItem, + ConfigurationFormItemType, + FormField, + SettingsValidationError, +) +from core.util import LanguageCodes + + +class PercentFloat(ConstrainedFloat): + ge = 0 + le = 1 + + +# The "level" property determines which admins will be able to modify the +# setting. Level 1 settings can be modified by anyone. Level 2 settings can be +# modified only by library managers and system admins (i.e. not by librarians). +# Level 3 settings can be changed only by system admins. If no level is +# specified, the setting will be treated as Level 1 by default. +class Level(IntEnum): + ALL_ACCESS = 1 + SYS_ADMIN_OR_MANAGER = 2 + SYS_ADMIN_ONLY = 3 + + +@dataclass(frozen=True) +class LibraryConfFormItem(ConfigurationFormItem): + category: str = "Basic Information" + level: Level = Level.ALL_ACCESS + read_only: Optional[bool] = None + skip: Optional[bool] = None + paired: Optional[str] = None + + def to_dict( + self, db: Session, key: str, required: bool = False, default: Any = None + ) -> Tuple[int, Dict[str, Any]]: + """Serialize additional form items specific to library settings.""" + weight, item = super().to_dict(db, key, required, default) + item["category"] = self.category + item["level"] = self.level + if self.read_only is not None: + item["readOnly"] = self.read_only + if self.skip is not None: + item["skip"] = self.skip + if self.paired is not None: + item["paired"] = self.paired + + if ( + "default" in item + and isinstance(item["default"], list) + and len(item["default"]) == 0 + ): + del item["default"] + + return weight, item + + +class LibrarySettings(BaseSettings): + _additional_form_fields = { + "name": LibraryConfFormItem( + label="Name", + description="The human-readable name of this library.", + category="Basic Information", + level=Level.SYS_ADMIN_ONLY, + required=True, + weight=-1, + ), + "short_name": LibraryConfFormItem( + label="Short name", + description="A short name of this library, to use when identifying it " + "in scripts or URLs, e.g. 'NYPL'.", + category="Basic Information", + level=Level.SYS_ADMIN_ONLY, + required=True, + weight=-1, + ), + "logo": LibraryConfFormItem( + label="Logo image", + description="The image should be in GIF, PNG, or JPG format, approximately square, no larger than " + "135x135 pixels, and look good on a light or dark mode background. " + "Larger images will be accepted, but scaled down (maintaining aspect ratio) such that " + "the longest dimension does not exceed 135 pixels.", + category="Client Interface Customization", + type=ConfigurationFormItemType.IMAGE, + level=Level.ALL_ACCESS, + ), + "announcements": LibraryConfFormItem( + label="Scheduled announcements", + description="Announcements will be displayed to authenticated patrons.", + type=ConfigurationFormItemType.ANNOUNCEMENTS, + category="Announcements", + level=Level.ALL_ACCESS, + ), + } + + website: HttpUrl = FormField( + ..., + form=LibraryConfFormItem( + label="URL of the library's website", + description='The library\'s main website, e.g. "https://www.nypl.org/" ' + "(not this Circulation Manager's URL).", + category="Basic Information", + level=Level.SYS_ADMIN_ONLY, + ), + ) + allow_holds: bool = FormField( + True, + form=LibraryConfFormItem( + label="Allow books to be put on hold", + type=ConfigurationFormItemType.SELECT, + options={ + "true": "Allow holds", + "false": "Disable holds", + }, + category="Loans, Holds, & Fines", + level=Level.SYS_ADMIN_ONLY, + ), + ) + enabled_entry_points: List[str] = FormField( + [x.INTERNAL_NAME for x in EntryPoint.DEFAULT_ENABLED], + form=LibraryConfFormItem( + label="Enabled entry points", + description="Patrons will see the selected entry points at the " + "top level and in search results.

Currently supported " + "audiobook vendors: Bibliotheca, Axis 360", + type=ConfigurationFormItemType.MENU, + options={ + entrypoint.INTERNAL_NAME: EntryPoint.DISPLAY_TITLES[entrypoint] + for entrypoint in EntryPoint.ENTRY_POINTS + }, + category="Lanes & Filters", + format="narrow", + read_only=True, + level=Level.SYS_ADMIN_ONLY, + ), + ) + featured_lane_size: PositiveInt = FormField( + 15, + form=LibraryConfFormItem( + label="Maximum number of books in the 'featured' lanes", + category="Lanes & Filters", + level=Level.ALL_ACCESS, + ), + ) + minimum_featured_quality: PercentFloat = FormField( + Configuration.DEFAULT_MINIMUM_FEATURED_QUALITY, + form=LibraryConfFormItem( + label="Minimum quality for books that show up in 'featured' lanes", + description="Between 0 and 1.", + category="Lanes & Filters", + level=Level.ALL_ACCESS, + ), + ) + facets_enabled_order: List[str] = FormField( + FacetConstants.DEFAULT_ENABLED_FACETS[FacetConstants.ORDER_FACET_GROUP_NAME], + form=LibraryConfFormItem( + label="Allow patrons to sort by", + type=ConfigurationFormItemType.MENU, + options={ + facet: FacetConstants.FACET_DISPLAY_TITLES[facet] + for facet in FacetConstants.ORDER_FACETS + }, + category="Lanes & Filters", + paired="facets_default_order", + level=Level.SYS_ADMIN_OR_MANAGER, + ), + ) + facets_default_order: str = FormField( + FacetConstants.ORDER_AUTHOR, + form=LibraryConfFormItem( + label="Default Sort by", + type=ConfigurationFormItemType.SELECT, + options={ + facet: FacetConstants.FACET_DISPLAY_TITLES[facet] + for facet in FacetConstants.ORDER_FACETS + }, + category="Lanes & Filters", + skip=True, + ), + ) + facets_enabled_available: List[str] = FormField( + FacetConstants.DEFAULT_ENABLED_FACETS[ + FacetConstants.AVAILABILITY_FACET_GROUP_NAME + ], + form=LibraryConfFormItem( + label="Allow patrons to filter availability to", + type=ConfigurationFormItemType.MENU, + options={ + facet: FacetConstants.FACET_DISPLAY_TITLES[facet] + for facet in FacetConstants.AVAILABILITY_FACETS + }, + category="Lanes & Filters", + paired="facets_default_available", + level=Level.SYS_ADMIN_OR_MANAGER, + ), + ) + facets_default_available: str = FormField( + FacetConstants.AVAILABLE_ALL, + form=LibraryConfFormItem( + label="Default Availability", + type=ConfigurationFormItemType.SELECT, + options={ + facet: FacetConstants.FACET_DISPLAY_TITLES[facet] + for facet in FacetConstants.AVAILABILITY_FACETS + }, + category="Lanes & Filters", + skip=True, + ), + ) + facets_enabled_collection: List[str] = FormField( + FacetConstants.DEFAULT_ENABLED_FACETS[ + FacetConstants.COLLECTION_FACET_GROUP_NAME + ], + form=LibraryConfFormItem( + label="Allow patrons to filter collection to", + type=ConfigurationFormItemType.MENU, + options={ + facet: FacetConstants.FACET_DISPLAY_TITLES[facet] + for facet in FacetConstants.COLLECTION_FACETS + }, + category="Lanes & Filters", + paired="facets_default_collection", + level=Level.SYS_ADMIN_OR_MANAGER, + ), + ) + facets_default_collection: str = FormField( + FacetConstants.COLLECTION_FULL, + form=LibraryConfFormItem( + label="Default Collection", + type=ConfigurationFormItemType.SELECT, + options={ + facet: FacetConstants.FACET_DISPLAY_TITLES[facet] + for facet in FacetConstants.COLLECTION_FACETS + }, + category="Lanes & Filters", + skip=True, + ), + ) + library_description: Optional[str] = FormField( + None, + form=LibraryConfFormItem( + label="A short description of this library", + description="This will be shown to people who aren't sure they've chosen the right library.", + category="Basic Information", + level=Level.SYS_ADMIN_ONLY, + ), + ) + help_email: Optional[EmailStr] = FormField( + None, + form=LibraryConfFormItem( + label="Patron support email address", + description="An email address a patron can use if they need help, " + "e.g. 'palacehelp@yourlibrary.org'.", + category="Basic Information", + level=Level.ALL_ACCESS, + ), + alias="help-email", + ) + help_web: Optional[HttpUrl] = FormField( + None, + form=LibraryConfFormItem( + label="Patron support website", + description="A URL for patrons to get help. Either this field or " + "patron support email address must be provided.", + category="Basic Information", + level=Level.ALL_ACCESS, + ), + alias="help-web", + ) + copyright_designated_agent_email_address: Optional[EmailStr] = FormField( + None, + form=LibraryConfFormItem( + label="Copyright designated agent email", + description="Patrons of this library should use this email " + "address to send a DMCA notification (or other copyright " + "complaint) to the library.
If no value is specified here, " + "the general patron support address will be used.", + category="Patron Support", + level=Level.SYS_ADMIN_OR_MANAGER, + ), + ) + configuration_contact_email_address: Optional[EmailStr] = FormField( + None, + form=LibraryConfFormItem( + label="A point of contact for the organization responsible for configuring this library", + description="This email address will be shared as part of " + "integrations that you set up through this interface. It will not " + "be shared with the general public. This gives the administrator " + "of the remote integration a way to contact you about problems with " + "this library's use of that integration.
If no value is specified here, " + "the general patron support address will be used.", + category="Patron Support", + level=Level.SYS_ADMIN_OR_MANAGER, + ), + ) + default_notification_email_address: EmailStr = FormField( + "noreply@thepalaceproject.org", + form=LibraryConfFormItem( + label="Write-only email address for vendor hold notifications", + description="This address must trash all email sent to it. Vendor hold notifications " + "contain sensitive patron information, but " + '' + "cannot be forwarded to patrons because they contain vendor-specific instructions." + "
The default address will work, but for greater security, set up your own address that " + "trashes all incoming email.", + level=Level.SYS_ADMIN_OR_MANAGER, + ), + ) + color_scheme: str = FormField( + "blue", + form=LibraryConfFormItem( + label="Mobile color scheme", + description="This tells mobile applications what color scheme to use when rendering " + "this library's OPDS feed.", + type=ConfigurationFormItemType.SELECT, + options={ + "amber": "Amber", + "black": "Black", + "blue": "Blue", + "bluegray": "Blue Gray", + "brown": "Brown", + "cyan": "Cyan", + "darkorange": "Dark Orange", + "darkpurple": "Dark Purple", + "green": "Green", + "gray": "Gray", + "indigo": "Indigo", + "lightblue": "Light Blue", + "orange": "Orange", + "pink": "Pink", + "purple": "Purple", + "red": "Red", + "teal": "Teal", + }, + category="Client Interface Customization", + level=Level.SYS_ADMIN_OR_MANAGER, + ), + ) + web_primary_color: str = FormField( + "#377F8B", + form=LibraryConfFormItem( + label="Web primary color", + description="This is the brand primary color for the web application. " + "Must have sufficient contrast with white.", + category="Client Interface Customization", + type=ConfigurationFormItemType.COLOR, + level=Level.SYS_ADMIN_OR_MANAGER, + ), + alias="web-primary-color", + ) + web_secondary_color: str = FormField( + "#D53F34", + form=LibraryConfFormItem( + label="Web secondary color", + description="This is the brand secondary color for the web application. " + "Must have sufficient contrast with white.", + category="Client Interface Customization", + type=ConfigurationFormItemType.COLOR, + level=Level.SYS_ADMIN_OR_MANAGER, + ), + alias="web-secondary-color", + ) + web_css_file: Optional[HttpUrl] = FormField( + None, + form=LibraryConfFormItem( + label="Custom CSS file for web", + description="Give web applications a CSS file to customize the catalog display.", + category="Client Interface Customization", + level=Level.SYS_ADMIN_ONLY, + ), + alias="web-css-file", + ) + web_header_links: List[str] = FormField( + [], + form=LibraryConfFormItem( + label="Web header links", + description="This gives web applications a list of links to display in the header. " + "Specify labels for each link in the same order under 'Web header labels'.", + category="Client Interface Customization", + type=ConfigurationFormItemType.LIST, + level=Level.SYS_ADMIN_OR_MANAGER, + ), + alias="web-header-links", + ) + web_header_labels: List[str] = FormField( + [], + form=LibraryConfFormItem( + label="Web header labels", + description="Labels for each link under 'Web header links'.", + category="Client Interface Customization", + type=ConfigurationFormItemType.LIST, + level=Level.SYS_ADMIN_OR_MANAGER, + ), + alias="web-header-labels", + ) + hidden_content_types: List[str] = FormField( + [], + form=LibraryConfFormItem( + label="Hidden content types", + description="A list of content types to hide from all clients, e.g. " + "application/pdf. This can be left blank except to " + "solve specific problems.", + category="Client Interface Customization", + type=ConfigurationFormItemType.LIST, + level=Level.SYS_ADMIN_ONLY, + ), + ) + max_outstanding_fines: Optional[PositiveFloat] = FormField( + None, + form=LibraryConfFormItem( + label="Maximum amount in fines a patron can have before losing lending privileges", + category="Loans, Holds, & Fines", + level=Level.ALL_ACCESS, + ), + ) + loan_limit: Optional[PositiveInt] = FormField( + None, + form=LibraryConfFormItem( + label="Maximum number of books a patron can have on loan at once", + description="Note: depending on distributor settings, a patron may be able to exceed " + "the limit by checking out books directly from a distributor's app. They may also get " + "a limit exceeded error before they reach these limits if a distributor has a smaller limit.", + category="Loans, Holds, & Fines", + level=Level.ALL_ACCESS, + ), + ) + hold_limit: Optional[PositiveInt] = FormField( + None, + form=LibraryConfFormItem( + label="Maximum number of books a patron can have on hold at once", + description="Note: depending on distributor settings, a patron may be able to exceed " + "the limit by placing holds directly from a distributor's app. They may also get " + "a limit exceeded error before they reach these limits if a distributor has a smaller limit.", + category="Loans, Holds, & Fines", + level=Level.ALL_ACCESS, + ), + ) + terms_of_service: Optional[HttpUrl] = FormField( + None, + form=LibraryConfFormItem( + label="Terms of service URL", + category="Links", + level=Level.ALL_ACCESS, + ), + alias="terms-of-service", + ) + privacy_policy: Optional[HttpUrl] = FormField( + None, + form=LibraryConfFormItem( + label="Privacy policy URL", + category="Links", + level=Level.ALL_ACCESS, + ), + alias="privacy-policy", + ) + copyright: Optional[HttpUrl] = FormField( + None, + form=LibraryConfFormItem( + label="Copyright URL", + category="Links", + level=Level.SYS_ADMIN_OR_MANAGER, + ), + ) + about: Optional[HttpUrl] = FormField( + None, + form=LibraryConfFormItem( + label="About URL", + category="Links", + level=Level.ALL_ACCESS, + ), + ) + license: Optional[HttpUrl] = FormField( + None, + form=LibraryConfFormItem( + label="License URL", + category="Links", + level=Level.SYS_ADMIN_OR_MANAGER, + ), + ) + registration_url: Optional[HttpUrl] = FormField( + None, + form=LibraryConfFormItem( + label="Patron registration URL", + description="A URL where someone who doesn't have a library card yet can sign up for one.", + category="Patron Support", + level=Level.ALL_ACCESS, + ), + alias="register", + ) + patron_password_reset: Optional[HttpUrl] = FormField( + None, + form=LibraryConfFormItem( + label="Password Reset Link", + description="A link to a web page where a user can reset their virtual library card password", + category="Patron Support", + level=Level.SYS_ADMIN_ONLY, + ), + alias="http://librarysimplified.org/terms/rel/patron-password-reset", + ) + large_collection_languages: Optional[List[str]] = FormField( + None, + form=LibraryConfFormItem( + label="The primary languages represented in this library's collection", + type=ConfigurationFormItemType.LIST, + format="language-code", + description="Each value can be either the full name of a language or an " + '' + "ISO-639-2 language code.", + category="Languages", + level=Level.ALL_ACCESS, + ), + alias="large_collections", + ) + small_collection_languages: Optional[List[str]] = FormField( + None, + form=LibraryConfFormItem( + label="Other major languages represented in this library's collection", + type=ConfigurationFormItemType.LIST, + format="language-code", + description="Each value can be either the full name of a language or an " + '' + "ISO-639-2 language code.", + category="Languages", + level=Level.ALL_ACCESS, + ), + alias="small_collections", + ) + tiny_collection_languages: Optional[List[str]] = FormField( + None, + form=LibraryConfFormItem( + label="Other languages in this library's collection", + type=ConfigurationFormItemType.LIST, + format="language-code", + description="Each value can be either the full name of a language or an " + '' + "ISO-639-2 language code.", + category="Languages", + level=Level.ALL_ACCESS, + ), + alias="tiny_collections", + ) + + @root_validator + def validate_require_help_email_or_website( + cls, values: Dict[str, Any] + ) -> Dict[str, Any]: + if not values.get("help_email") and not values.get("help_web"): + help_email_label = cls.get_form_field_label("help_email") + help_website_label = cls.get_form_field_label("help_web") + raise SettingsValidationError( + problem_detail=INCOMPLETE_CONFIGURATION.detailed( + f"You must provide either '{help_email_label}' or '{help_website_label}'." + ) + ) + + return values + + @root_validator + def validate_header_links(cls, values: Dict[str, Any]) -> Dict[str, Any]: + """Verify that header links and labels are the same length.""" + header_links = values.get("web_header_links") + header_labels = values.get("web_header_labels") + if header_links and header_labels and len(header_links) != len(header_labels): + raise SettingsValidationError( + problem_detail=INVALID_CONFIGURATION_OPTION.detailed( + "There must be the same number of web header links and web header labels." + ) + ) + return values + + @validator("web_primary_color", "web_secondary_color") + def validate_web_color_contrast(cls, value: str, field: ModelField) -> str: + """ + Verify that the web primary and secondary color both contrast + well on white, as these colors will serve as button backgrounds with + white test, as well as text color on white backgrounds. + """ + + def hex_to_rgb(hex: str) -> Tuple[float, ...]: + hex = hex.lstrip("#") + return tuple(int(hex[i : i + 2], 16) / 255.0 for i in (0, 2, 4)) + + passes = wcag_contrast_ratio.passes_AA( + wcag_contrast_ratio.rgb(hex_to_rgb(value), hex_to_rgb("#ffffff")) + ) + if not passes: + check_url = ( + "https://contrast-ratio.com/#%23" + + value[1:] + + "-on-%23" + + "#ffffff"[1:] + ) + field_label = cls.get_form_field_label(field.name) + raise SettingsValidationError( + problem_detail=INVALID_CONFIGURATION_OPTION.detailed( + f"The {field_label} doesn't have enough contrast to pass the WCAG 2.0 AA guidelines and " + f"will be difficult for some patrons to read. Check contrast here." + ) + ) + return value + + @validator( + "large_collection_languages", + "small_collection_languages", + "tiny_collection_languages", + ) + def validate_language_codes( + cls, value: Optional[List[str]], field: ModelField + ) -> Optional[List[str]]: + """Verify that collection languages are valid.""" + if value is not None: + for language in value: + if not LanguageCodes.string_to_alpha_3(language): + field_label = cls.get_form_field_label(field.name) + raise SettingsValidationError( + problem_detail=UNKNOWN_LANGUAGE.detailed( + f'"{field_label}": "{language}" is not a valid language code.' + ) + ) + return value diff --git a/core/entrypoint.py b/core/entrypoint.py index 326b6fa4f8..0ddd499ccf 100644 --- a/core/entrypoint.py +++ b/core/entrypoint.py @@ -20,10 +20,6 @@ class by calling EntryPoint.register. renders entry points as a set of tabs. """ - # The name of the per-library setting that controls which entry points are - # enabled. - ENABLED_SETTING = "enabled_entry_points" - ENTRY_POINTS: list[type[EntryPoint]] = [] DEFAULT_ENABLED: list[type[EntryPoint]] = [] DISPLAY_TITLES: dict[type[EntryPoint], str] = {} diff --git a/core/external_search.py b/core/external_search.py index 64b11d9533..6273928499 100644 --- a/core/external_search.py +++ b/core/external_search.py @@ -2513,7 +2513,7 @@ def from_worklist(cls, _db, worklist, facets): if library is None: allow_holds = True else: - allow_holds = library.allow_holds + allow_holds = library.settings.allow_holds return cls( collections, media, diff --git a/core/integration/settings.py b/core/integration/settings.py index 05555f6d71..15f22c099e 100644 --- a/core/integration/settings.py +++ b/core/integration/settings.py @@ -22,7 +22,7 @@ ValidationError, root_validator, ) -from pydantic.fields import FieldInfo, ModelField, NoArgAnyCallable, Undefined +from pydantic.fields import FieldInfo, NoArgAnyCallable, Undefined from sqlalchemy.orm import Session from api.admin.problem_details import ( @@ -150,6 +150,9 @@ class ConfigurationFormItemType(Enum): LIST = "list" MENU = "menu" NUMBER = "number" + ANNOUNCEMENTS = "announcements" + COLOR = "color-picker" + IMAGE = "image" @classmethod def options_from_enum(cls, enum_: Type[Enum]) -> Dict[Enum | str, str]: @@ -207,7 +210,9 @@ def get_form_value(value: Any) -> Any: return str(value) return value - def to_dict(self, db: Session, field: ModelField) -> Tuple[int, Dict[str, Any]]: + def to_dict( + self, db: Session, key: str, required: bool = False, default: Any = None + ) -> Tuple[int, Dict[str, Any]]: """ Convert the ConfigurationFormItem to a dictionary @@ -215,11 +220,11 @@ def to_dict(self, db: Session, field: ModelField) -> Tuple[int, Dict[str, Any]]: """ form_entry: Dict[str, Any] = { "label": self.label, - "key": field.name, - "required": field.required or self.required, + "key": key, + "required": required or self.required, } - if field.default is not None: - form_entry["default"] = self.get_form_value(field.default) + if default is not None: + form_entry["default"] = self.get_form_value(default) if self.type.value is not None: form_entry["type"] = self.type.value if self.description is not None: @@ -319,7 +324,13 @@ def configuration_form(cls, db: Session) -> List[Dict[str, Any]]: f"{field.name} was not initialized with FormField, skipping." ) continue - config.append(field.field_info.form.to_dict(db, field)) + required = field.required if isinstance(field.required, bool) else False + config.append( + field.field_info.form.to_dict(db, field.name, required, field.default) + ) + + for key, additional_field in cls._additional_form_fields.items(): + config.append(additional_field.to_dict(db, key)) # Sort by weight then return only the settings config.sort(key=lambda x: x[0]) @@ -327,7 +338,34 @@ def configuration_form(cls, db: Session) -> List[Dict[str, Any]]: def dict(self, *args: Any, **kwargs: Any) -> Dict[str, Any]: """Override the dict method to remove the default values""" - return super().dict(exclude_defaults=True, *args, **kwargs) + if "exclude_defaults" not in kwargs: + kwargs["exclude_defaults"] = True + return super().dict(*args, **kwargs) + + @classmethod + def get_form_field_label(cls, field_name: str) -> str: + item = cls.__fields__.get(field_name) + if item is None: + # Try to lookup field_name by alias instead + for field in cls.__fields__.values(): + if field.alias == field_name: + item = field + break + if item is not None and isinstance(item.field_info, FormFieldInfo): + return item.field_info.form.label + else: + return field_name + + # If your settings class needs additional form fields that are not + # defined on the model, you can add them here. This is useful if you + # need to add a custom form field, but don't want the data in the field + # to be stored on the model in the database. For example, if you want + # to add a custom form field that allows the user to upload an image, but + # want to store that image data outside the settings model. + # + # The key for the dictionary should be the field name, and the value + # should be a ConfigurationFormItem object that defines the form field. + _additional_form_fields: Dict[str, ConfigurationFormItem] = {} def __init__(self, **data: Any): """ @@ -342,11 +380,7 @@ def __init__(self, **data: Any): except ValidationError as e: error = e.errors()[0] error_location = str(error["loc"][0]) - item = self.__fields__.get(error_location) - if item is not None and isinstance(item.field_info, FormFieldInfo): - item_label = item.field_info.form.label - else: - item_label = error_location + item_label = self.get_form_field_label(error_location) if ( error["type"] == "value_error.problem_detail" diff --git a/core/lane.py b/core/lane.py index f4c1e4e649..481620be33 100644 --- a/core/lane.py +++ b/core/lane.py @@ -584,7 +584,7 @@ def __init__( if ( availability == self.AVAILABLE_ALL - and (library and not library.allow_holds) + and (library and not library.settings.allow_holds) and ( self.AVAILABLE_NOW in self.available_facets(library, self.AVAILABILITY_FACET_GROUP_NAME) @@ -755,7 +755,9 @@ def modify_search_filter(self, filter): super().modify_search_filter(filter) if self.library: - filter.minimum_featured_quality = self.library.minimum_featured_quality + filter.minimum_featured_quality = ( + self.library.settings.minimum_featured_quality + ) filter.availability = self.availability filter.subcollection = self.collection @@ -832,7 +834,9 @@ def modify_database_query(self, _db, qu): elif self.collection == self.COLLECTION_FEATURED: # Exclude books with a quality of less than the library's # minimum featured quality. - qu = qu.filter(Work.quality >= self.library.minimum_featured_quality) + qu = qu.filter( + Work.quality >= self.library.settings.minimum_featured_quality + ) return qu @@ -990,7 +994,7 @@ def default(cls, lane, **kwargs): library = lane.library if library: - quality = library.minimum_featured_quality + quality = library.settings.minimum_featured_quality else: quality = Configuration.DEFAULT_MINIMUM_FEATURED_QUALITY return cls(quality, **kwargs) @@ -2108,7 +2112,7 @@ def _groups_for_lanes( # the featured lane size, but we'll ask for a few extra # works for each lane, to reduce the risk that we end up # reusing a book in two different lanes. - target_size = library.featured_lane_size + target_size = library.settings.featured_lane_size # We ask for a few extra works for each lane, to reduce the # risk that we'll end up reusing a book in two different @@ -2688,7 +2692,7 @@ class Lane(Base, DatabaseBackedWorkList, HierarchyWorkList): __tablename__ = "lanes" id = Column(Integer, primary_key=True) library_id = Column(Integer, ForeignKey("libraries.id"), index=True, nullable=False) - library: Mapped[Library] = relationship("Library", back_populates="lanes") + library: Mapped[Library] = relationship(Library, back_populates="lanes") parent_id = Column(Integer, ForeignKey("lanes.id"), index=True, nullable=True) priority = Column(Integer, index=True, nullable=False, default=0) @@ -3268,10 +3272,6 @@ def groups( works each child of this WorkList may contribute. :param facets: A FeaturedFacets object. """ - clauses = [] - library = self.get_library(_db) - target_size = library.featured_lane_size - if self.include_self_in_grouped_feed: relevant_lanes = [self] else: diff --git a/core/model/configuration.py b/core/model/configuration.py index 4b3d664e1d..227276b4bb 100644 --- a/core/model/configuration.py +++ b/core/model/configuration.py @@ -586,8 +586,9 @@ class ConfigurationSetting(Base, HasSessionCache): ) library_id = Column(Integer, ForeignKey("libraries.id"), index=True) - library: Mapped[Library] = relationship("Library", back_populates="settings") - + library: Mapped[Library] = relationship( + "Library", back_populates="external_integration_settings" + ) key = Column(Unicode) _value = Column("value", Unicode) @@ -676,12 +677,6 @@ def sitewide(cls, _db, key): """Find or create a sitewide ConfigurationSetting.""" return cls.for_library_and_externalintegration(_db, key, None, None) - @classmethod - def for_library(cls, key, library): - """Find or create a ConfigurationSetting for the given Library.""" - _db = Session.object_session(library) - return cls.for_library_and_externalintegration(_db, key, library, None) - @classmethod def for_externalintegration(cls, key, externalintegration): """Find or create a ConfigurationSetting for the given diff --git a/core/model/constants.py b/core/model/constants.py index 16eb8f15ea..3e7e09a775 100644 --- a/core/model/constants.py +++ b/core/model/constants.py @@ -235,11 +235,6 @@ class LinkRelations: # The rel for a link we feed to clients for samples/previews. CLIENT_SAMPLE = "preview" - # A uri rel type for authentication documents with a vendor specific "link" - PATRON_PASSWORD_RESET = ( - "http://librarysimplified.org/terms/rel/patron-password-reset" - ) - # opds/opds2 auth token rel TOKEN_AUTH = "token_endpoint" diff --git a/core/model/datasource.py b/core/model/datasource.py index 77dbae4a33..13a178363c 100644 --- a/core/model/datasource.py +++ b/core/model/datasource.py @@ -18,8 +18,8 @@ if TYPE_CHECKING: # This is needed during type checking so we have the # types of related models. - from core.lane import Lane - from core.model import ( # noqa: autoflake + from api.lanes import Lane + from core.model import ( Classification, CoverageRecord, Credential, @@ -110,16 +110,16 @@ class DataSource(Base, HasSessionCache, DataSourceConstants): foreign_keys=lambda: [LicensePoolDeliveryMechanism.data_source_id], ) - list_lanes: Mapped[List[Lane]] = relationship( + license_lanes: Mapped[List[Lane]] = relationship( "Lane", - back_populates="_list_datasource", - primaryjoin="DataSource.id==Lane._list_datasource_id", + back_populates="license_datasource", + foreign_keys="Lane.license_datasource_id", ) - license_lanes: Mapped[List[Lane]] = relationship( + list_lanes: Mapped[List[Lane]] = relationship( "Lane", - back_populates="license_datasource", - primaryjoin="DataSource.id==Lane.license_datasource_id", + back_populates="_list_datasource", + foreign_keys="Lane._list_datasource_id", ) def __repr__(self): diff --git a/core/model/library.py b/core/model/library.py index 50af7b6379..fa83c601a0 100644 --- a/core/model/library.py +++ b/core/model/library.py @@ -3,7 +3,17 @@ import logging from collections import Counter -from typing import TYPE_CHECKING, List, Tuple +from typing import ( + TYPE_CHECKING, + Any, + Dict, + Generator, + List, + Optional, + Tuple, + Type, + Union, +) from Crypto.PublicKey import RSA from expiringdict import ExpiringDict @@ -17,13 +27,15 @@ Unicode, UniqueConstraint, ) -from sqlalchemy.orm import Mapped, relationship +from sqlalchemy.dialects.postgresql import JSONB +from sqlalchemy.orm import Mapped, Query, relationship +from sqlalchemy.orm.attributes import flag_modified from sqlalchemy.orm.session import Session from sqlalchemy.sql.functions import func from core.model.hybrid import hybrid_property -from ..config import Configuration +from ..configuration.library import LibrarySettings from ..entrypoint import EntryPoint from ..facets import FacetConstants from . import Base, get_one @@ -48,6 +60,8 @@ Patron, ) + from ..lane import Lane + class Library(Base, HasSessionCache): """A library that uses this circulation manager to authenticate @@ -133,15 +147,19 @@ class Library(Base, HasSessionCache): back_populates="libraries", ) - # Any additional configuration information is stored as - # ConfigurationSettings. - settings: Mapped[List[ConfigurationSetting]] = relationship( + # This parameter is deprecated, and will be removed once all of our integrations + # are updated to use IntegrationSettings. New code shouldn't use it. + # TODO: Remove this column. + external_integration_settings: Mapped[List[ConfigurationSetting]] = relationship( "ConfigurationSetting", back_populates="library", lazy="joined", cascade="all, delete", ) + # Any additional configuration information is stored as JSON on this column. + settings_dict: Dict[str, Any] = Column(JSONB, nullable=False, default=dict) + # A Library may have many CirculationEvents circulation_events: Mapped[List[CirculationEvent]] = relationship( "CirculationEvent", backref="library", cascade="all, delete-orphan" @@ -156,11 +174,15 @@ class Library(Base, HasSessionCache): # A class-wide cache mapping library ID to the calculated value # used for Library.has_root_lane. This is invalidated whenever # Lane configuration changes, and it will also expire on its own. - _has_root_lane_cache = ExpiringDict(max_len=1000, max_age_seconds=3600) + _has_root_lane_cache: Dict[Union[int, None], bool] = ExpiringDict( + max_len=1000, max_age_seconds=3600 + ) + # A Library can have many lanes lanes: Mapped[List[Lane]] = relationship( "Lane", back_populates="library", + foreign_keys="Lane.library_id", cascade="all, delete-orphan", ) @@ -182,20 +204,23 @@ class Library(Base, HasSessionCache): # Typing specific collections: List[Collection] - def __repr__(self): + # Cache of the libraries loaded settings object + _settings: Optional[LibrarySettings] + + def __repr__(self) -> str: return ( '' % (self.name, self.short_name, self.uuid, self.library_registry_short_name) ) - def cache_key(self): + def cache_key(self) -> Optional[str]: return self.short_name @classmethod - def lookup(cls, _db, short_name): + def lookup(cls, _db: Session, short_name: Optional[str]) -> Optional[Library]: """Look up a library by short name.""" - def _lookup(): + def _lookup() -> Tuple[Optional[Library], bool]: library = get_one(_db, Library, short_name=short_name) return library, False @@ -203,13 +228,13 @@ def _lookup(): return library @classmethod - def default(cls, _db): + def default(cls, _db: Session) -> Optional[Library]: """Find the default Library.""" # If for some reason there are multiple default libraries in # the database, they're not actually interchangeable, but # raising an error here might make it impossible to fix the # problem. - defaults = ( + defaults: List[Library] = ( _db.query(Library) .filter(Library._is_default == True) .order_by(Library.id.asc()) @@ -219,7 +244,6 @@ def default(cls, _db): # This is the normal case. return defaults[0] - default_library = None if not defaults: # There is no current default. Find the library with the # lowest ID and make it the default. @@ -242,7 +266,7 @@ def default(cls, _db): % (default_library.short_name) ) default_library.is_default = True - return default_library + return default_library # type: ignore[no-any-return] @classmethod def generate_keypair(cls) -> Tuple[str, bytes]: @@ -254,12 +278,12 @@ def generate_keypair(cls) -> Tuple[str, bytes]: return public_key_str, private_key_bytes @hybrid_property - def library_registry_short_name(self): + def library_registry_short_name(self) -> Optional[str]: """Gets library_registry_short_name from database""" return self._library_registry_short_name @library_registry_short_name.setter - def library_registry_short_name(self, value): + def library_registry_short_name(self, value: Optional[str]) -> None: """Uppercase the library registry short name on the way in.""" if value: value = value.upper() @@ -270,88 +294,42 @@ def library_registry_short_name(self, value): value = str(value) self._library_registry_short_name = value - def setting(self, key): - """Find or create a ConfigurationSetting on this Library. - :param key: Name of the setting. - :return: A ConfigurationSetting - """ - from .configuration import ConfigurationSetting + @property + def settings(self) -> LibrarySettings: + """Get the settings for this integration""" + settings = getattr(self, "_settings", None) + if settings is None: + if not isinstance(self.settings_dict, dict): + raise ValueError( + "settings_dict for library %s is not a dict: %r" + % (self.short_name, self.settings_dict) + ) + settings = LibrarySettings.construct(**self.settings_dict) + self._settings = settings + return settings - return ConfigurationSetting.for_library(key, self) + def update_settings(self, new_settings: LibrarySettings) -> None: + """Update the settings for this integration""" + self._settings = None + self.settings_dict.update(new_settings.dict()) + flag_modified(self, "settings_dict") @property - def all_collections(self): + def all_collections(self) -> Generator[Collection, None, None]: for collection in self.collections: yield collection yield from collection.parents - # Some specific per-library configuration settings. - - # The name of the per-library regular expression used to derive a patron's - # external_type from their authorization_identifier. - EXTERNAL_TYPE_REGULAR_EXPRESSION = "external_type_regular_expression" - - # The name of the per-library configuration policy that controls whether - # books may be put on hold. - ALLOW_HOLDS = Configuration.ALLOW_HOLDS - - # Each facet group has two associated per-library keys: one - # configuring which facets are enabled for that facet group, and - # one configuring which facet is the default. - ENABLED_FACETS_KEY_PREFIX = Configuration.ENABLED_FACETS_KEY_PREFIX - DEFAULT_FACET_KEY_PREFIX = Configuration.DEFAULT_FACET_KEY_PREFIX - - # Each library may set a minimum quality for the books that show - # up in the 'featured' lanes that show up on the front page. - MINIMUM_FEATURED_QUALITY = Configuration.MINIMUM_FEATURED_QUALITY - - # Each library may configure the maximum number of books in the - # 'featured' lanes. - FEATURED_LANE_SIZE = Configuration.FEATURED_LANE_SIZE - - @property - def allow_holds(self): - """Does this library allow patrons to put items on hold?""" - value = self.setting(self.ALLOW_HOLDS).bool_value - if value is None: - # If the library has not set a value for this setting, - # holds are allowed. - value = True - return value - @property - def minimum_featured_quality(self): - """The minimum quality a book must have to be 'featured'.""" - value = self.setting(self.MINIMUM_FEATURED_QUALITY).float_value - if value is None: - value = 0.65 - return value - - @property - def featured_lane_size(self): - """The minimum quality a book must have to be 'featured'.""" - value = self.setting(self.FEATURED_LANE_SIZE).int_value - if value is None: - value = 15 - return value - - @property - def entrypoints(self): + def entrypoints(self) -> Generator[Optional[Type[EntryPoint]], None, None]: """The EntryPoints enabled for this library.""" - values = self.setting(EntryPoint.ENABLED_SETTING).json_value - if values is None: - # No decision has been made about enabled EntryPoints. - for cls in EntryPoint.DEFAULT_ENABLED: + values = self.settings.enabled_entry_points + for v in values: + cls = EntryPoint.BY_INTERNAL_NAME.get(v) + if cls: yield cls - else: - # It's okay for `values` to be an empty list--that means - # the library wants to only use lanes, no entry points. - for v in values: - cls = EntryPoint.BY_INTERNAL_NAME.get(v) - if cls: - yield cls - - def enabled_facets(self, group_name): + + def enabled_facets(self, group_name: str) -> List[str]: """Look up the enabled facets for a given facet group.""" if group_name == FacetConstants.DISTRIBUTOR_FACETS_GROUP_NAME: enabled = [] @@ -366,25 +344,10 @@ def enabled_facets(self, group_name): enabled.append(collection.name) return enabled - setting = self.enabled_facets_setting(group_name) - value = None - - try: - value = setting.json_value - except ValueError as e: - logging.error( - "Invalid list of enabled facets for %s: %s", group_name, setting.value - ) - if value is None: - value = list(FacetConstants.DEFAULT_ENABLED_FACETS.get(group_name, [])) - return value - - def enabled_facets_setting(self, group_name): - key = self.ENABLED_FACETS_KEY_PREFIX + group_name - return self.setting(key) + return getattr(self.settings, f"facets_enabled_{group_name}") # type: ignore[no-any-return] @property - def has_root_lanes(self): + def has_root_lanes(self) -> bool: """Does this library have any lanes that act as the root lane for a certain patron type? @@ -416,10 +379,10 @@ def has_root_lanes(self): def restrict_to_ready_deliverable_works( self, - query, - collection_ids=None, - show_suppressed=False, - ): + query: Query[Work], + collection_ids: Optional[List[int]] = None, + show_suppressed: bool = False, + ) -> Query[Work]: """Restrict a query to show only presentation-ready works present in an appropriate collection which the default client can fulfill. @@ -433,15 +396,19 @@ def restrict_to_ready_deliverable_works( """ from .collection import Collection - collection_ids = collection_ids or [x.id for x in self.all_collections] - return Collection.restrict_to_ready_deliverable_works( + collection_ids = collection_ids or [ + x.id for x in self.all_collections if x.id is not None + ] + return Collection.restrict_to_ready_deliverable_works( # type: ignore[no-any-return] query, collection_ids=collection_ids, show_suppressed=show_suppressed, - allow_holds=self.allow_holds, + allow_holds=self.settings.allow_holds, ) - def estimated_holdings_by_language(self, include_open_access=True): + def estimated_holdings_by_language( + self, include_open_access: bool = True + ) -> Counter[str]: """Estimate how many titles this library has in various languages. The estimate is pretty good but should not be relied upon as exact. @@ -460,23 +427,22 @@ def estimated_holdings_by_language(self, include_open_access=True): qu = self.restrict_to_ready_deliverable_works(qu) if not include_open_access: qu = qu.filter(LicensePool.open_access == False) - counter = Counter() - for language, count in qu: - counter[language] = count + counter: Counter[str] = Counter() + for language, count in qu: # type: ignore[misc] + counter[language] = count # type: ignore[has-type] return counter - def default_facet(self, group_name): - """Look up the default facet for a given facet group.""" - value = self.default_facet_setting(group_name).value - if not value: - value = FacetConstants.DEFAULT_FACET.get(group_name) - return value + def default_facet(self, group_name: str) -> str: + if ( + group_name == FacetConstants.DISTRIBUTOR_FACETS_GROUP_NAME + or group_name == FacetConstants.COLLECTION_NAME_FACETS_GROUP_NAME + ): + return FacetConstants.DEFAULT_FACET[group_name] - def default_facet_setting(self, group_name): - key = self.DEFAULT_FACET_KEY_PREFIX + group_name - return self.setting(key) + """Look up the default facet for a given facet group.""" + return getattr(self.settings, "facets_default_" + group_name) # type: ignore[no-any-return] - def explain(self, include_secrets=False): + def explain(self, include_secrets: bool = False) -> List[str]: """Create a series of human-readable strings to explain a library's settings. @@ -503,16 +469,13 @@ def explain(self, include_secrets=False): % self.library_registry_shared_secret ) - # Find all ConfigurationSettings that are set on the library - # itself and are not on the library + an external integration. - settings = [x for x in self.settings if not x.external_integration] - if settings: - lines.append("") - lines.append("Configuration settings:") - lines.append("-----------------------") - for setting in settings: - if (include_secrets or not setting.is_secret) and setting.value is not None: - lines.append(f"{setting.key}='{setting.value}'") + # Find all settings that are set on the library + lines.append("") + lines.append("Configuration settings:") + lines.append("-----------------------") + for key, value in self.settings.dict(exclude_defaults=False).items(): + if value is not None: + lines.append(f"{key}='{value}'") integrations = list(self.integrations) if integrations: @@ -525,11 +488,11 @@ def explain(self, include_secrets=False): return lines @property - def is_default(self): + def is_default(self) -> Optional[bool]: return self._is_default @is_default.setter - def is_default(self, new_is_default): + def is_default(self, new_is_default: bool) -> None: """Set this library, and only this library, as the default.""" if self._is_default and not new_is_default: raise ValueError( diff --git a/core/model/licensing.py b/core/model/licensing.py index bb2ed88407..783ccede46 100644 --- a/core/model/licensing.py +++ b/core/model/licensing.py @@ -1036,7 +1036,7 @@ def on_hold_to( _db = Session.object_session(patron_or_client) if ( isinstance(patron_or_client, Patron) - and not patron_or_client.library.allow_holds + and not patron_or_client.library.settings.allow_holds ): raise PolicyException("Holds are disabled for this library.") start = start or utc_now() diff --git a/core/model/listeners.py b/core/model/listeners.py index c8b22d6b43..b3e4e5e7ed 100644 --- a/core/model/listeners.py +++ b/core/model/listeners.py @@ -115,8 +115,6 @@ def _site_configuration_has_changed(_db, cooldown=1): @event.listens_for(ExternalIntegration.settings, "remove") @event.listens_for(Library.integrations, "append") @event.listens_for(Library.integrations, "remove") -@event.listens_for(Library.settings, "append") -@event.listens_for(Library.settings, "remove") def configuration_relevant_collection_change(target, value, initiator): site_configuration_has_changed(target) diff --git a/core/scripts.py b/core/scripts.py index 29e453c6d2..3ba35c5a0f 100644 --- a/core/scripts.py +++ b/core/scripts.py @@ -14,6 +14,7 @@ from sqlalchemy import and_, exists, tuple_ from sqlalchemy.orm import Query, Session, defer +from sqlalchemy.orm.attributes import flag_modified from sqlalchemy.orm.exc import MultipleResultsFound, NoResultFound from core.model.classification import Classification @@ -1039,6 +1040,17 @@ def arg_parser(cls): ) return parser + def apply_settings(self, settings, library): + """Treat `settings` as a list of command-line argument settings, + and apply each one to `obj`. + """ + if not settings: + return None + for setting in settings: + key, value = self._parse_setting(setting) + library.settings_dict[key] = value + flag_modified(library, "settings_dict") + def do_run(self, _db=None, cmd_args=None, output=sys.stdout): _db = _db or self._db args = self.parse_command_line(_db, cmd_args=cmd_args) diff --git a/core/util/authentication_for_opds.py b/core/util/authentication_for_opds.py index b9d10c2232..334bee3775 100644 --- a/core/util/authentication_for_opds.py +++ b/core/util/authentication_for_opds.py @@ -1,7 +1,7 @@ from __future__ import annotations from abc import ABC, abstractmethod -from typing import Any, Dict, List +from typing import Any, Dict, List, Optional from sqlalchemy.orm import Session @@ -45,7 +45,7 @@ def __init__( id: str | None = None, title: str | None = None, authentication_flows: List[OPDSAuthenticationFlow] | None = None, - links: List[Dict[str, str]] | None = None, + links: List[Dict[str, Optional[str]]] | None = None, ): """Initialize an Authentication For OPDS document. diff --git a/data/uszipcode/README.md b/data/uszipcode/README.md deleted file mode 100644 index a56fba7126..0000000000 --- a/data/uszipcode/README.md +++ /dev/null @@ -1,5 +0,0 @@ -# uszipcode - -This files source is [datahub.io](https://datahub.io/machu-gwu/uszipcode-0.2.0-simple_db/r/simple_db.sqlite). - -Its mirrored here for the reasons outlined in `api/admin/geographic_validator.py`. diff --git a/data/uszipcode/simple_db.sqlite b/data/uszipcode/simple_db.sqlite deleted file mode 100644 index 1710d1fb37..0000000000 Binary files a/data/uszipcode/simple_db.sqlite and /dev/null differ diff --git a/poetry.lock b/poetry.lock index 9acfff8797..f60e0fb6f6 100644 --- a/poetry.lock +++ b/poetry.lock @@ -22,17 +22,6 @@ typing-extensions = ">=4" [package.extras] tz = ["python-dateutil"] -[[package]] -name = "atomicwrites" -version = "1.4.0" -description = "Atomic file writes." -optional = false -python-versions = ">=2.7, !=3.0.*, !=3.1.*, !=3.2.*, !=3.3.*" -files = [ - {file = "atomicwrites-1.4.0-py2.py3-none-any.whl", hash = "sha256:6d1784dea7c0c8d4a5172b6c620f40b6e4cbfdf96d783691f2e1302a7b88e197"}, - {file = "atomicwrites-1.4.0.tar.gz", hash = "sha256:ae70396ad1a434f9c7046fd2dd196fc04b12f9e91ffb859164193be8b6168a7a"}, -] - [[package]] name = "attrs" version = "21.4.0" @@ -50,21 +39,6 @@ docs = ["furo", "sphinx", "sphinx-notfound-page", "zope.interface"] tests = ["cloudpickle", "coverage[toml] (>=5.0.2)", "hypothesis", "mypy", "pympler", "pytest (>=4.3.0)", "pytest-mypy-plugins", "six", "zope.interface"] tests-no-zope = ["cloudpickle", "coverage[toml] (>=5.0.2)", "hypothesis", "mypy", "pympler", "pytest (>=4.3.0)", "pytest-mypy-plugins", "six"] -[[package]] -name = "autopep8" -version = "1.6.0" -description = "A tool that automatically formats Python code to conform to the PEP 8 style guide" -optional = false -python-versions = "*" -files = [ - {file = "autopep8-1.6.0-py2.py3-none-any.whl", hash = "sha256:ed77137193bbac52d029a52c59bec1b0629b5a186c495f1eb21b126ac466083f"}, - {file = "autopep8-1.6.0.tar.gz", hash = "sha256:44f0932855039d2c15c4510d6df665e4730f2b8582704fa48f9c55bd3e17d979"}, -] - -[package.dependencies] -pycodestyle = ">=2.8.0" -toml = "*" - [[package]] name = "aws-xray-sdk" version = "2.12.0" @@ -905,6 +879,26 @@ files = [ {file = "distlib-0.3.6.tar.gz", hash = "sha256:14bad2d9b04d3a36127ac97f30b12a19268f211063d8f8ee4f47108896e11b46"}, ] +[[package]] +name = "dnspython" +version = "2.3.0" +description = "DNS toolkit" +optional = false +python-versions = ">=3.7,<4.0" +files = [ + {file = "dnspython-2.3.0-py3-none-any.whl", hash = "sha256:89141536394f909066cabd112e3e1a37e4e654db00a25308b0f130bc3152eb46"}, + {file = "dnspython-2.3.0.tar.gz", hash = "sha256:224e32b03eb46be70e12ef6d64e0be123a64e621ab4c0822ff6d450d52a540b9"}, +] + +[package.extras] +curio = ["curio (>=1.2,<2.0)", "sniffio (>=1.1,<2.0)"] +dnssec = ["cryptography (>=2.6,<40.0)"] +doh = ["h2 (>=4.1.0)", "httpx (>=0.21.1)", "requests (>=2.23.0,<3.0.0)", "requests-toolbelt (>=0.9.1,<0.11.0)"] +doq = ["aioquic (>=0.9.20)"] +idna = ["idna (>=2.1,<4.0)"] +trio = ["trio (>=0.14,<0.23)"] +wmi = ["wmi (>=1.5.1,<2.0.0)"] + [[package]] name = "docker" version = "4.2.2" @@ -940,6 +934,21 @@ files = [ [package.dependencies] packaging = ">=20.9" +[[package]] +name = "email-validator" +version = "2.0.0.post2" +description = "A robust email address syntax and deliverability validation library." +optional = false +python-versions = ">=3.7" +files = [ + {file = "email_validator-2.0.0.post2-py3-none-any.whl", hash = "sha256:2466ba57cda361fb7309fd3d5a225723c788ca4bbad32a0ebd5373b99730285c"}, + {file = "email_validator-2.0.0.post2.tar.gz", hash = "sha256:1ff6e86044200c56ae23595695c54e9614f4a9551e0e393614f764860b3d7900"}, +] + +[package.dependencies] +dnspython = ">=2.0.0" +idna = ">=2.0.0" + [[package]] name = "exceptiongroup" version = "1.0.4" @@ -2449,26 +2458,6 @@ requests = ">=2.27.1,<3.0.0" rfc3987 = ">=1.3.8,<2.0.0" uritemplate = ">=3.0.1,<5.0.0" -[[package]] -name = "pathlib-mate" -version = "1.0.3" -description = "An extended and more powerful pathlib." -optional = false -python-versions = "*" -files = [ - {file = "pathlib_mate-1.0.3-py2.py3-none-any.whl", hash = "sha256:a589e597e55e41b5fb8d91b0a38f862e9a0b74667ebd3c495c311acfcdb8382b"}, - {file = "pathlib_mate-1.0.3.tar.gz", hash = "sha256:8241c3ca06bc007e2cc3f33d275d59ec2da2cd66af911abf46b5d0cbcbb1e918"}, -] - -[package.dependencies] -atomicwrites = "*" -autopep8 = "*" -six = "*" - -[package.extras] -docs = ["docfly (==1.0.2)", "furo (==2021.8.31)", "rstobj (==0.0.7)", "sphinx (==4.3.0)", "sphinx-copybutton (==0.4.0)", "sphinx-inline-tabs (==2021.8.17b10)", "sphinx-jinja (==1.1.1)"] -tests = ["pytest", "pytest-cov", "tox"] - [[package]] name = "pillow" version = "10.0.0" @@ -2741,17 +2730,6 @@ files = [ [package.dependencies] pyasn1 = ">=0.4.6,<0.6.0" -[[package]] -name = "pycodestyle" -version = "2.8.0" -description = "Python style guide checker" -optional = false -python-versions = ">=2.7, !=3.0.*, !=3.1.*, !=3.2.*, !=3.3.*, !=3.4.*" -files = [ - {file = "pycodestyle-2.8.0-py2.py3-none-any.whl", hash = "sha256:720f8b39dde8b293825e7ff02c475f3077124006db4f440dcbc9a20b76548a20"}, - {file = "pycodestyle-2.8.0.tar.gz", hash = "sha256:eddd5847ef438ea1c7870ca7eb78a9d47ce0cdb4851a5523949f2601d0cbbe7f"}, -] - [[package]] name = "pycparser" version = "2.21" @@ -2850,6 +2828,7 @@ files = [ ] [package.dependencies] +email-validator = {version = ">=1.0.3", optional = true, markers = "extra == \"email\""} typing-extensions = ">=4.2.0" [package.extras] @@ -3027,20 +3006,6 @@ files = [ [package.dependencies] pywin32 = ">=223" -[[package]] -name = "pypostalcode" -version = "0.4.1" -description = "Radius searches on Canadian postal codes, location data" -optional = false -python-versions = "*" -files = [ - {file = "pypostalcode-0.4.1-py3-none-any.whl", hash = "sha256:76c6ccff42b58604a485b568a34f296a15df05be3c6fc6ee7edc945bba45377b"}, - {file = "pypostalcode-0.4.1.tar.gz", hash = "sha256:5706b37c4bad5859723140ae1d9ef7be0dd094a3f68e27c64f8df20668e4b955"}, -] - -[package.extras] -dev = ["pytz", "timezonefinder"] - [[package]] name = "pyproject-api" version = "1.5.2" @@ -3734,17 +3699,6 @@ files = [ [package.dependencies] nltk = {version = ">=3.1", markers = "python_version >= \"3\""} -[[package]] -name = "toml" -version = "0.10.2" -description = "Python Library for Tom's Obvious, Minimal Language" -optional = false -python-versions = ">=2.6, !=3.0.*, !=3.1.*, !=3.2.*" -files = [ - {file = "toml-0.10.2-py2.py3-none-any.whl", hash = "sha256:806143ae5bfb6a3c6e736a764057db0e6a0e05e338b5630894a5f779cabb4f9b"}, - {file = "toml-0.10.2.tar.gz", hash = "sha256:b3bda1d108d5dd99f4a20d24d9c348e91c4db7ab1b749200bded2f839ccbe68f"}, -] - [[package]] name = "tomli" version = "2.0.1" @@ -4036,27 +3990,6 @@ brotli = ["brotli (>=1.0.9)", "brotlicffi (>=0.8.0)", "brotlipy (>=0.6.0)"] secure = ["certifi", "cryptography (>=1.3.4)", "idna (>=2.0.0)", "ipaddress", "pyOpenSSL (>=0.14)", "urllib3-secure-extra"] socks = ["PySocks (>=1.5.6,!=1.5.7,<2.0)"] -[[package]] -name = "uszipcode" -version = "0.2.6" -description = "USA zipcode programmable database, includes up-to-date census and geometry information." -optional = false -python-versions = "*" -files = [ - {file = "uszipcode-0.2.6-py2.py3-none-any.whl", hash = "sha256:f1b30034d1d634bdf440f62ef61dbe2d3d6d9021c939ff9499b15e0a779fec18"}, - {file = "uszipcode-0.2.6.tar.gz", hash = "sha256:80072754f17675d7667345ef9f01b0195e783acbf4f43e3f1e69cfd632c64b28"}, -] - -[package.dependencies] -attrs = "*" -pathlib-mate = "*" -requests = "*" -SQLAlchemy = "*" - -[package.extras] -docs = ["docfly (>=0.0.17)", "pygments", "rstobj (>=0.0.5)", "sphinx (==1.8.1)", "sphinx-copybutton", "sphinx-jinja", "sphinx-rtd-theme"] -tests = ["pytest (==3.2.3)", "pytest-cov (==2.5.1)", "python-Levenshtein"] - [[package]] name = "uwsgi" version = "2.0.21" @@ -4261,4 +4194,4 @@ testing = ["flake8 (<5)", "func-timeout", "jaraco.functools", "jaraco.itertools" [metadata] lock-version = "2.0" python-versions = ">=3.8,<4" -content-hash = "5487a9982b95b75a129da6394036a1b202710108584f8f7b8319005c359340c7" +content-hash = "31f3da1d714f3330146f9423d35b0712fa9099b84e1b991fff439956f529cd04" diff --git a/pyproject.toml b/pyproject.toml index 523901584b..fe461586f0 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -57,6 +57,7 @@ disallow_untyped_decorators = true disallow_untyped_defs = true module = [ "api.admin.announcement_list_validator", + "api.admin.controller.library_settings", "api.admin.controller.patron_auth_services", "api.admin.form_data", "api.admin.model.dashboard_statistics", @@ -66,7 +67,9 @@ module = [ "core.model.announcements", "core.model.hassessioncache", "core.model.integration", + "core.model.library", "core.selftest", + "core.settings.*", "core.util.authentication_for_opds", "core.util.cache", "tests.migration.*", @@ -118,12 +121,10 @@ module = [ "pyld", "pymarc", "pyparsing", - "pypostalcode", "spellchecker", "textblob.*", "unicodecsv", "uritemplate", - "uszipcode", "watchtower", "wcag_contrast_ratio", "webpub_manifest_parser.*", @@ -171,14 +172,13 @@ opensearch-py = "~1.1" palace-webpub-manifest-parser = "~3.0.1" pillow = "^10.0" pycryptodome = "^3.18" -pydantic = "^1.10.9" +pydantic = {version = "^1.10.9", extras = ["email"]} pyinstrument = "<4.6" PyJWT = "2.6.0" PyLD = "2.0.3" pymarc = "5.1.0" pyOpenSSL = "^23.1.0" pyparsing = "3.1.0" -pypostalcode = "0.4.1" pyspellchecker = "0.7.2" python = ">=3.8,<4" python-dateutil = "2.8.2" @@ -198,7 +198,6 @@ typing_extensions = {version = "^4.5.0", python = "<3.11"} unicodecsv = "0.14.1" # this is used, but can probably be removed on py3 uritemplate = "4.1.1" urllib3 = "~1.26.14" -uszipcode = "0.2.6" uWSGI = "~2.0.21" watchtower = "3.0.1" # watchtower is for Cloudwatch logging integration wcag-contrast-ratio = "0.9" diff --git a/scripts.py b/scripts.py index fc88afe458..c623564a69 100644 --- a/scripts.py +++ b/scripts.py @@ -58,6 +58,7 @@ Hold, Hyperlink, Identifier, + Library, LicensePool, Loan, Patron, @@ -548,7 +549,7 @@ def facets(self, lane): no way to override this. """ top_level = lane.parent is None - library = lane.get_library(self._db) + library: Library = lane.get_library(self._db) # If the WorkList has explicitly defined EntryPoints, we want to # create a grouped feed for each EntryPoint. Otherwise, we want @@ -563,7 +564,7 @@ def facets(self, lane): default_entrypoint = entrypoints[0] for entrypoint in entrypoints: facets = FeaturedFacets( - minimum_featured_quality=library.minimum_featured_quality, + minimum_featured_quality=library.settings.minimum_featured_quality, uses_customlists=lane.uses_customlists, entrypoint=entrypoint, entrypoint_is_default=(top_level and entrypoint is default_entrypoint), diff --git a/tests/api/admin/controller/test_controller.py b/tests/api/admin/controller/test_controller.py index c9c707f74d..7b2788c046 100644 --- a/tests/api/admin/controller/test_controller.py +++ b/tests/api/admin/controller/test_controller.py @@ -10,7 +10,7 @@ import flask import pytest from attrs import define -from werkzeug.datastructures import FileStorage, ImmutableMultiDict +from werkzeug.datastructures import ImmutableMultiDict from werkzeug.http import dump_cookie from werkzeug.wrappers import Response as WerkzeugResponse @@ -25,7 +25,6 @@ PasswordAdminAuthenticationProvider, ) from api.admin.problem_details import * -from api.admin.validator import Validator from api.adobe_vendor_id import AuthdataUtility from api.authentication.base import PatronData from api.config import Configuration @@ -3246,56 +3245,6 @@ def test_create_integration(self, settings_ctrl_fixture: SettingsControllerFixtu assert False == is_new2 assert DUPLICATE_INTEGRATION == i2 - def test_validate_formats(self, settings_ctrl_fixture: SettingsControllerFixture): - class MockValidator(Validator): - def __init__(self): - self.was_called = False - self.args = [] - - def validate(self, settings, content): - self.was_called = True - self.args.append(settings) - self.args.append(content) - - def validate_error(self, settings, content): - return INVALID_EMAIL - - validator = MockValidator() - - with settings_ctrl_fixture.request_context_with_admin("/", method="POST"): - flask.request.form = ImmutableMultiDict( - [ - ("name", "The New York Public Library"), - ("short_name", "nypl"), - (Configuration.WEBSITE_URL, "https://library.library/"), - ( - Configuration.DEFAULT_NOTIFICATION_EMAIL_ADDRESS, - "email@example.com", - ), - (Configuration.HELP_EMAIL, "help@example.com"), - ] - ) - flask.request.files = ImmutableMultiDict( - [(Configuration.LOGO, FileStorage())] - ) - response = settings_ctrl_fixture.manager.admin_settings_controller.validate_formats( - Configuration.LIBRARY_SETTINGS, validator - ) - assert response == None - assert validator.was_called == True - assert validator.args[0] == Configuration.LIBRARY_SETTINGS - assert validator.args[1] == { - "files": flask.request.files, - "form": flask.request.form, - } - - setattr(validator, "validate", validator.validate_error) - # If the validator returns an problem detail, validate_formats returns it. - response = settings_ctrl_fixture.manager.admin_settings_controller.validate_formats( - Configuration.LIBRARY_SETTINGS, validator - ) - assert response == INVALID_EMAIL - def test__mirror_integration_settings( self, settings_ctrl_fixture: SettingsControllerFixture ): diff --git a/tests/api/admin/controller/test_library.py b/tests/api/admin/controller/test_library.py index 778c9ed4d0..95f8ce824e 100644 --- a/tests/api/admin/controller/test_library.py +++ b/tests/api/admin/controller/test_library.py @@ -1,7 +1,11 @@ +from __future__ import annotations + import base64 import datetime import json from io import BytesIO +from typing import Dict, List +from unittest.mock import MagicMock import flask import pytest @@ -12,14 +16,16 @@ from api.admin.controller.library_settings import LibrarySettingsController from api.admin.exceptions import * -from api.admin.geographic_validator import GeographicValidator from api.config import Configuration from core.facets import FacetConstants -from core.model import AdminRole, ConfigurationSetting, Library, get_one +from core.model import AdminRole, Library, get_one from core.model.announcements import SETTING_NAME as ANNOUNCEMENTS_SETTING_NAME from core.model.announcements import Announcement, AnnouncementData -from core.util.problem_detail import ProblemDetail +from core.model.library import LibraryLogo +from core.util.problem_detail import ProblemDetail, ProblemError from tests.fixtures.announcements import AnnouncementFixture +from tests.fixtures.api_controller import ControllerFixture +from tests.fixtures.library import LibraryFixture class TestLibrarySettings: @@ -38,18 +44,29 @@ def logo_properties(self): "image": image, } - def library_form(self, library, fields={}): - - defaults = { - "uuid": library.uuid, + def library_form( + self, library: Library, fields: Dict[str, str | List[str]] | None = None + ): + fields = fields or {} + defaults: Dict[str, str | List[str]] = { + "uuid": str(library.uuid), "name": "The New York Public Library", - "short_name": library.short_name, - Configuration.WEBSITE_URL: "https://library.library/", - Configuration.HELP_EMAIL: "help@example.com", - Configuration.DEFAULT_NOTIFICATION_EMAIL_ADDRESS: "email@example.com", + "short_name": str(library.short_name), + "website": "https://library.library/", + "help_email": "help@example.com", + "default_notification_email": "email@example.com", } defaults.update(fields) - form = ImmutableMultiDict(list(defaults.items())) + + form_data = [] + for k, v in defaults.items(): + if isinstance(v, list): + for value in v: + form_data.append((k, value)) + else: + form_data.append((k, v)) + + form = ImmutableMultiDict(form_data) return form def test_libraries_get_with_no_libraries(self, settings_ctrl_fixture): @@ -62,35 +79,7 @@ def test_libraries_get_with_no_libraries(self, settings_ctrl_fixture): response = ( settings_ctrl_fixture.manager.admin_library_settings_controller.process_get() ) - assert response.get("libraries") == [] - - def test_libraries_get_with_geographic_info(self, settings_ctrl_fixture): - # Delete any existing library created by the controller test setup. - library = get_one(settings_ctrl_fixture.ctrl.db.session, Library) - if library: - settings_ctrl_fixture.ctrl.db.session.delete(library) - - test_library = settings_ctrl_fixture.ctrl.db.library("Library 1", "L1") - ConfigurationSetting.for_library( - Configuration.LIBRARY_FOCUS_AREA, test_library - ).value = '{"CA": ["N3L"], "US": ["11235"]}' - ConfigurationSetting.for_library( - Configuration.LIBRARY_SERVICE_AREA, test_library - ).value = '{"CA": ["J2S"], "US": ["31415"]}' - - with settings_ctrl_fixture.request_context_with_admin("/"): - response = ( - settings_ctrl_fixture.manager.admin_library_settings_controller.process_get() - ) - library_settings = response.get("libraries")[0].get("settings") - assert library_settings.get("focus_area") == { - "CA": [{"N3L": "Paris, Ontario"}], - "US": [{"11235": "Brooklyn, NY"}], - } - assert library_settings.get("service_area") == { - "CA": [{"J2S": "Saint-Hyacinthe Southwest, Quebec"}], - "US": [{"31415": "Savannah, GA"}], - } + assert response.json.get("libraries") == [] def test_libraries_get_with_announcements( self, settings_ctrl_fixture, announcement_fixture: AnnouncementFixture @@ -112,7 +101,7 @@ def test_libraries_get_with_announcements( response = ( settings_ctrl_fixture.manager.admin_library_settings_controller.process_get() ) - library_settings = response.get("libraries")[0].get("settings") + library_settings = response.json.get("libraries")[0].get("settings") # We find out about the library's announcements. announcements = library_settings.get(ANNOUNCEMENTS_SETTING_NAME) @@ -135,30 +124,50 @@ def test_libraries_get_with_announcements( datetime.date, ) - def test_libraries_get_with_multiple_libraries(self, settings_ctrl_fixture): + def test_libraries_get_with_logo(self, settings_ctrl_fixture, logo_properties): + db = settings_ctrl_fixture.ctrl.db + + library = db.default_library() + + # Give the library a logo + library.logo = LibraryLogo(content=logo_properties["base64_bytes"]) + + # When we request information about this library... + with settings_ctrl_fixture.request_context_with_admin("/"): + response = ( + settings_ctrl_fixture.manager.admin_library_settings_controller.process_get() + ) + + libraries = response.json.get("libraries") + assert len(libraries) == 1 + library_settings = libraries[0].get("settings") + + assert "logo" in library_settings + assert library_settings["logo"] == logo_properties["data_url"] + + def test_libraries_get_with_multiple_libraries( + self, settings_ctrl_fixture, library_fixture: LibraryFixture + ): # Delete any existing library created by the controller test setup. library = get_one(settings_ctrl_fixture.ctrl.db.session, Library) if library: settings_ctrl_fixture.ctrl.db.session.delete(library) - l1 = settings_ctrl_fixture.ctrl.db.library("Library 1", "L1") - l2 = settings_ctrl_fixture.ctrl.db.library("Library 2", "L2") - l3 = settings_ctrl_fixture.ctrl.db.library("Library 3", "L3") + l1 = library_fixture.library("Library 1", "L1") + l2 = library_fixture.library("Library 2", "L2") + l3 = library_fixture.library("Library 3", "L3") + # L2 has some additional library-wide settings. - ConfigurationSetting.for_library(Configuration.FEATURED_LANE_SIZE, l2).value = 5 - ConfigurationSetting.for_library( - Configuration.DEFAULT_FACET_KEY_PREFIX - + FacetConstants.ORDER_FACET_GROUP_NAME, - l2, - ).value = FacetConstants.ORDER_TITLE - ConfigurationSetting.for_library( - Configuration.ENABLED_FACETS_KEY_PREFIX - + FacetConstants.ORDER_FACET_GROUP_NAME, - l2, - ).value = json.dumps([FacetConstants.ORDER_TITLE, FacetConstants.ORDER_AUTHOR]) - ConfigurationSetting.for_library( - Configuration.LARGE_COLLECTION_LANGUAGES, l2 - ).value = json.dumps(["French"]) + settings = library_fixture.settings(l2) + settings.featured_lane_size = 5 + settings.facets_default_order = FacetConstants.ORDER_TITLE + settings.facets_enabled_order = [ + FacetConstants.ORDER_TITLE, + FacetConstants.ORDER_AUTHOR, + ] + settings.large_collection_languages = ["French"] + l2.update_settings(settings) + # The admin only has access to L1 and L2. settings_ctrl_fixture.admin.remove_role(AdminRole.SYSTEM_ADMIN) settings_ctrl_fixture.admin.add_role(AdminRole.LIBRARIAN, l1) @@ -168,7 +177,7 @@ def test_libraries_get_with_multiple_libraries(self, settings_ctrl_fixture): response = ( settings_ctrl_fixture.manager.admin_library_settings_controller.process_get() ) - libraries = response.get("libraries") + libraries = response.json.get("libraries") assert 2 == len(libraries) assert l1.uuid == libraries[0].get("uuid") @@ -180,42 +189,53 @@ def test_libraries_get_with_multiple_libraries(self, settings_ctrl_fixture): assert l1.short_name == libraries[0].get("short_name") assert l2.short_name == libraries[1].get("short_name") - assert {} == libraries[0].get("settings") - assert 4 == len(libraries[1].get("settings").keys()) - settings = libraries[1].get("settings") - assert "5" == settings.get(Configuration.FEATURED_LANE_SIZE) - assert FacetConstants.ORDER_TITLE == settings.get( - Configuration.DEFAULT_FACET_KEY_PREFIX - + FacetConstants.ORDER_FACET_GROUP_NAME + assert { + "website": "http://library.com", + "help_web": "http://library.com/support", + } == libraries[0].get("settings") + assert 6 == len(libraries[1].get("settings").keys()) + settings_dict = libraries[1].get("settings") + assert 5 == settings_dict.get("featured_lane_size") + assert FacetConstants.ORDER_TITLE == settings_dict.get( + "facets_default_order" ) assert [ FacetConstants.ORDER_TITLE, FacetConstants.ORDER_AUTHOR, - ] == settings.get( - Configuration.ENABLED_FACETS_KEY_PREFIX - + FacetConstants.ORDER_FACET_GROUP_NAME - ) - assert ["French"] == settings.get(Configuration.LARGE_COLLECTION_LANGUAGES) + ] == settings_dict.get("facets_enabled_order") + assert ["French"] == settings_dict.get("large_collection_languages") def test_libraries_post_errors(self, settings_ctrl_fixture): + with settings_ctrl_fixture.request_context_with_admin("/", method="POST"): + flask.request.form = ImmutableMultiDict([]) + with pytest.raises(ProblemError) as excinfo: + settings_ctrl_fixture.manager.admin_library_settings_controller.process_post() + assert excinfo.value.problem_detail.uri == INCOMPLETE_CONFIGURATION.uri + assert ( + "Required field 'Name' is missing." + == excinfo.value.problem_detail.detail + ) + with settings_ctrl_fixture.request_context_with_admin("/", method="POST"): flask.request.form = ImmutableMultiDict( [ ("name", "Brooklyn Public Library"), ] ) - response = ( + with pytest.raises(ProblemError) as excinfo: settings_ctrl_fixture.manager.admin_library_settings_controller.process_post() + assert excinfo.value.problem_detail.uri == INCOMPLETE_CONFIGURATION.uri + assert ( + "Required field 'Short name' is missing." + == excinfo.value.problem_detail.detail ) - assert response == MISSING_LIBRARY_SHORT_NAME library = settings_ctrl_fixture.ctrl.db.library() with settings_ctrl_fixture.request_context_with_admin("/", method="POST"): flask.request.form = self.library_form(library, {"uuid": "1234"}) - response = ( + with pytest.raises(ProblemError) as excinfo: settings_ctrl_fixture.manager.admin_library_settings_controller.process_post() - ) - assert response.uri == LIBRARY_NOT_FOUND.uri + assert excinfo.value.problem_detail.uri == LIBRARY_NOT_FOUND.uri with settings_ctrl_fixture.request_context_with_admin("/", method="POST"): flask.request.form = ImmutableMultiDict( @@ -224,10 +244,10 @@ def test_libraries_post_errors(self, settings_ctrl_fixture): ("short_name", library.short_name), ] ) - response = ( + with pytest.raises(ProblemError) as excinfo: settings_ctrl_fixture.manager.admin_library_settings_controller.process_post() - ) - assert response == LIBRARY_SHORT_NAME_ALREADY_IN_USE + + assert excinfo.value.problem_detail == LIBRARY_SHORT_NAME_ALREADY_IN_USE bpl = settings_ctrl_fixture.ctrl.db.library(short_name="bpl") with settings_ctrl_fixture.request_context_with_admin("/", method="POST"): @@ -238,10 +258,9 @@ def test_libraries_post_errors(self, settings_ctrl_fixture): ("short_name", library.short_name), ] ) - response = ( + with pytest.raises(ProblemError) as excinfo: settings_ctrl_fixture.manager.admin_library_settings_controller.process_post() - ) - assert response == LIBRARY_SHORT_NAME_ALREADY_IN_USE + assert excinfo.value.problem_detail == LIBRARY_SHORT_NAME_ALREADY_IN_USE with settings_ctrl_fixture.request_context_with_admin("/", method="POST"): flask.request.form = ImmutableMultiDict( @@ -251,10 +270,9 @@ def test_libraries_post_errors(self, settings_ctrl_fixture): ("short_name", library.short_name), ] ) - response = ( + with pytest.raises(ProblemError) as excinfo: settings_ctrl_fixture.manager.admin_library_settings_controller.process_post() - ) - assert response.uri == INCOMPLETE_CONFIGURATION.uri + assert excinfo.value.problem_detail.uri == INCOMPLETE_CONFIGURATION.uri # Either patron support email or website MUST be present with settings_ctrl_fixture.request_context_with_admin("/", method="POST"): @@ -266,13 +284,12 @@ def test_libraries_post_errors(self, settings_ctrl_fixture): ("default_notification_email_address", "email@example.org"), ] ) - response = ( + with pytest.raises(ProblemError) as excinfo: settings_ctrl_fixture.manager.admin_library_settings_controller.process_post() - ) - assert response.uri == INCOMPLETE_CONFIGURATION.uri + assert excinfo.value.problem_detail.uri == INCOMPLETE_CONFIGURATION.uri assert ( - "Patron support email address or Patron support web site" - in response.detail + "'Patron support email address' or 'Patron support website'" + in excinfo.value.problem_detail.detail ) # Test a web primary and secondary color that doesn't contrast @@ -281,41 +298,90 @@ def test_libraries_post_errors(self, settings_ctrl_fixture): flask.request.form = self.library_form( library, { - Configuration.WEB_PRIMARY_COLOR: "#000000", - Configuration.WEB_SECONDARY_COLOR: "#e0e0e0", + "web_primary_color": "#000000", + "web_secondary_color": "#e0e0e0", }, ) - response = ( + with pytest.raises(ProblemError) as excinfo: settings_ctrl_fixture.manager.admin_library_settings_controller.process_post() + assert excinfo.value.problem_detail.uri == INVALID_CONFIGURATION_OPTION.uri + assert ( + "contrast-ratio.com/#%23e0e0e0-on-%23ffffff" + in excinfo.value.problem_detail.detail + ) + assert ( + "contrast-ratio.com/#%23e0e0e0-on-%23ffffff" + in excinfo.value.problem_detail.detail ) - assert response.uri == INVALID_CONFIGURATION_OPTION.uri - assert "contrast-ratio.com/#%23e0e0e0-on-%23ffffff" in response.detail - assert "contrast-ratio.com/#%23e0e0e0-on-%23ffffff" in response.detail # Test a list of web header links and a list of labels that # aren't the same length. - library = settings_ctrl_fixture.ctrl.db.library() with settings_ctrl_fixture.request_context_with_admin("/", method="POST"): - flask.request.form = ImmutableMultiDict( - [ - ("uuid", library.uuid), - ("name", "The New York Public Library"), - ("short_name", library.short_name), - (Configuration.WEBSITE_URL, "https://library.library/"), - ( - Configuration.DEFAULT_NOTIFICATION_EMAIL_ADDRESS, - "email@example.com", - ), - (Configuration.HELP_EMAIL, "help@example.com"), - (Configuration.WEB_HEADER_LINKS, "http://library.com/1"), - (Configuration.WEB_HEADER_LINKS, "http://library.com/2"), - (Configuration.WEB_HEADER_LABELS, "One"), - ] + flask.request.form = self.library_form( + library, + { + "web_header_links": [ + "http://library.com/1", + "http://library.com/2", + ], + "web_header_labels": "One", + }, ) - response = ( + with pytest.raises(ProblemError) as excinfo: + settings_ctrl_fixture.manager.admin_library_settings_controller.process_post() + assert excinfo.value.problem_detail.uri == INVALID_CONFIGURATION_OPTION.uri + + # Test bad language code + with settings_ctrl_fixture.request_context_with_admin("/", method="POST"): + flask.request.form = self.library_form( + library, {"tiny_collection_languages": "zzz"} + ) + with pytest.raises(ProblemError) as excinfo: + settings_ctrl_fixture.manager.admin_library_settings_controller.process_post() + assert excinfo.value.problem_detail.uri == UNKNOWN_LANGUAGE.uri + assert ( + '"zzz" is not a valid language code' + in excinfo.value.problem_detail.detail + ) + + # Test uploading a logo that is in the wrong format. + with settings_ctrl_fixture.request_context_with_admin("/", method="POST"): + flask.request.form = self.library_form(library) + flask.request.files = ImmutableMultiDict( + { + "logo": FileStorage( + stream=BytesIO(b"not a png"), + content_type="application/pdf", + filename="logo.png", + ) + } + ) + with pytest.raises(ProblemError) as excinfo: settings_ctrl_fixture.manager.admin_library_settings_controller.process_post() + assert excinfo.value.problem_detail.uri == INVALID_CONFIGURATION_OPTION.uri + assert ( + "Image upload must be in GIF, PNG, or JPG format." + in excinfo.value.problem_detail.detail + ) + + # Test uploading a logo that we can't open to resize. + with settings_ctrl_fixture.request_context_with_admin("/", method="POST"): + flask.request.form = self.library_form(library) + flask.request.files = ImmutableMultiDict( + { + "logo": FileStorage( + stream=BytesIO(b"not a png"), + content_type="image/png", + filename="logo.png", + ) + } + ) + with pytest.raises(ProblemError) as excinfo: + settings_ctrl_fixture.manager.admin_library_settings_controller.process_post() + assert excinfo.value.problem_detail.uri == INVALID_CONFIGURATION_OPTION.uri + assert ( + "Unable to open uploaded image" in excinfo.value.problem_detail.detail ) - assert response.uri == INVALID_CONFIGURATION_OPTION.uri def test__process_image(self, logo_properties, settings_ctrl_fixture): image, expected_encoded_image = ( @@ -351,32 +417,14 @@ def test_libraries_post_create( # a mismatch between the expected data URL and the one configured. assert max(*image.size) <= Configuration.LOGO_MAX_DIMENSION - original_geographic_validate = GeographicValidator().validate_geographic_areas - - class MockGeographicValidator(GeographicValidator): - def __init__(self): - self.was_called = False - - def validate_geographic_areas(self, values, db): - self.was_called = True - return original_geographic_validate(values, db) - with settings_ctrl_fixture.request_context_with_admin("/", method="POST"): flask.request.form = ImmutableMultiDict( [ ("name", "The New York Public Library"), ("short_name", "nypl"), ("library_description", "Short description of library"), - (Configuration.WEBSITE_URL, "https://library.library/"), - (Configuration.TINY_COLLECTION_LANGUAGES, ["ger"]), # type: ignore[list-item] - ( - Configuration.LIBRARY_SERVICE_AREA, - ["06759", "everywhere", "MD", "Boston, MA"], # type: ignore[list-item] - ), - ( - Configuration.LIBRARY_FOCUS_AREA, - ["Manitoba", "Broward County, FL", "QC"], # type: ignore[list-item] - ), + ("website", "https://library.library/"), + ("tiny_collection_languages", "ger"), ( ANNOUNCEMENTS_SETTING_NAME, json.dumps( @@ -395,47 +443,36 @@ def validate_geographic_areas(self, values, db): ), ), ( - Configuration.DEFAULT_NOTIFICATION_EMAIL_ADDRESS, + "default_notification_email_address", "email@example.com", ), - (Configuration.HELP_EMAIL, "help@example.com"), - (Configuration.FEATURED_LANE_SIZE, "5"), + ("help_email", "help@example.com"), + ("featured_lane_size", "5"), ( - Configuration.DEFAULT_FACET_KEY_PREFIX - + FacetConstants.ORDER_FACET_GROUP_NAME, + "facets_default_order", FacetConstants.ORDER_RANDOM, ), ( - Configuration.ENABLED_FACETS_KEY_PREFIX - + FacetConstants.ORDER_FACET_GROUP_NAME - + "_" - + FacetConstants.ORDER_TITLE, + "facets_enabled_order" + "_" + FacetConstants.ORDER_TITLE, "", ), ( - Configuration.ENABLED_FACETS_KEY_PREFIX - + FacetConstants.ORDER_FACET_GROUP_NAME - + "_" - + FacetConstants.ORDER_RANDOM, + "facets_enabled_order" + "_" + FacetConstants.ORDER_RANDOM, "", ), ] ) flask.request.files = ImmutableMultiDict( { - Configuration.LOGO: FileStorage( + "logo": FileStorage( stream=BytesIO(image_data), content_type="image/png", filename="logo.png", ) } ) - geographic_validator = MockGeographicValidator() - validators = dict( - geographic=geographic_validator, - ) - response = settings_ctrl_fixture.manager.admin_library_settings_controller.process_post( - validators + response = ( + settings_ctrl_fixture.manager.admin_library_settings_controller.process_post() ) assert response.status_code == 201 @@ -444,43 +481,14 @@ def validate_geographic_areas(self, values, db): assert library.uuid == response.get_data(as_text=True) assert library.name == "The New York Public Library" assert library.short_name == "nypl" - assert ( - "5" - == ConfigurationSetting.for_library( - Configuration.FEATURED_LANE_SIZE, library - ).value - ) - assert ( - FacetConstants.ORDER_RANDOM - == ConfigurationSetting.for_library( - Configuration.DEFAULT_FACET_KEY_PREFIX - + FacetConstants.ORDER_FACET_GROUP_NAME, - library, - ).value - ) - assert ( - json.dumps([FacetConstants.ORDER_TITLE]) - == ConfigurationSetting.for_library( - Configuration.ENABLED_FACETS_KEY_PREFIX - + FacetConstants.ORDER_FACET_GROUP_NAME, - library, - ).value - ) + assert library.settings.featured_lane_size == 5 + assert library.settings.facets_default_order == FacetConstants.ORDER_RANDOM + assert library.settings.facets_enabled_order == [ + FacetConstants.ORDER_TITLE, + FacetConstants.ORDER_RANDOM, + ] assert library.logo is not None assert expected_logo_data_url == library.logo.data_url - assert geographic_validator.was_called == True - assert ( - '{"US": ["06759", "everywhere", "MD", "Boston, MA"], "CA": []}' - == ConfigurationSetting.for_library( - Configuration.LIBRARY_SERVICE_AREA, library - ).value - ) - assert ( - '{"US": ["Broward County, FL"], "CA": ["Manitoba", "Quebec"]}' - == ConfigurationSetting.for_library( - Configuration.LIBRARY_FOCUS_AREA, library - ).value - ) # Make sure public and private key were generated and stored. assert library.private_key is not None @@ -517,60 +525,47 @@ def validate_geographic_areas(self, values, db): assert other_languages == german.parent assert ["ger"] == german.languages - def test_libraries_post_edit(self, settings_ctrl_fixture): + def test_libraries_post_edit( + self, settings_ctrl_fixture, library_fixture: LibraryFixture + ): # A library already exists. - library = settings_ctrl_fixture.ctrl.db.library( - "New York Public Library", "nypl" + settings = library_fixture.mock_settings() + settings.featured_lane_size = 5 + settings.facets_default_order = FacetConstants.ORDER_RANDOM + settings.facets_enabled_order = [ + FacetConstants.ORDER_TITLE, + FacetConstants.ORDER_RANDOM, + ] + library_to_edit = library_fixture.library( + "New York Public Library", "nypl", settings ) - - ConfigurationSetting.for_library( - Configuration.FEATURED_LANE_SIZE, library - ).value = 5 - ConfigurationSetting.for_library( - Configuration.DEFAULT_FACET_KEY_PREFIX - + FacetConstants.ORDER_FACET_GROUP_NAME, - library, - ).value = FacetConstants.ORDER_RANDOM - ConfigurationSetting.for_library( - Configuration.ENABLED_FACETS_KEY_PREFIX - + FacetConstants.ORDER_FACET_GROUP_NAME, - library, - ).value = json.dumps([FacetConstants.ORDER_TITLE, FacetConstants.ORDER_RANDOM]) - ConfigurationSetting.for_library( - Configuration.LOGO, library - ).value = "A tiny image" + library_to_edit.logo = LibraryLogo(content=b"A tiny image") + library_fixture.reset_settings_cache(library_to_edit) with settings_ctrl_fixture.request_context_with_admin("/", method="POST"): flask.request.form = ImmutableMultiDict( [ - ("uuid", library.uuid), + ("uuid", str(library_to_edit.uuid)), ("name", "The New York Public Library"), ("short_name", "nypl"), - (Configuration.FEATURED_LANE_SIZE, "20"), - (Configuration.MINIMUM_FEATURED_QUALITY, "0.9"), - (Configuration.WEBSITE_URL, "https://library.library/"), + ("featured_lane_size", "20"), + ("minimum_featured_quality", "0.9"), + ("website", "https://library.library/"), ( - Configuration.DEFAULT_NOTIFICATION_EMAIL_ADDRESS, + "default_notification_email_address", "email@example.com", ), - (Configuration.HELP_EMAIL, "help@example.com"), + ("help_email", "help@example.com"), ( - Configuration.DEFAULT_FACET_KEY_PREFIX - + FacetConstants.ORDER_FACET_GROUP_NAME, + "facets_default_order", FacetConstants.ORDER_AUTHOR, ), ( - Configuration.ENABLED_FACETS_KEY_PREFIX - + FacetConstants.ORDER_FACET_GROUP_NAME - + "_" - + FacetConstants.ORDER_AUTHOR, + "facets_enabled_order" + "_" + FacetConstants.ORDER_AUTHOR, "", ), ( - Configuration.ENABLED_FACETS_KEY_PREFIX - + FacetConstants.ORDER_FACET_GROUP_NAME - + "_" - + FacetConstants.ORDER_RANDOM, + "facets_enabled_order" + "_" + FacetConstants.ORDER_RANDOM, "", ), ] @@ -582,56 +577,50 @@ def test_libraries_post_edit(self, settings_ctrl_fixture): assert response.status_code == 200 library = get_one( - settings_ctrl_fixture.ctrl.db.session, Library, uuid=library.uuid + settings_ctrl_fixture.ctrl.db.session, Library, uuid=library_to_edit.uuid ) + assert library is not None assert library.uuid == response.get_data(as_text=True) assert library.name == "The New York Public Library" assert library.short_name == "nypl" # The library-wide settings were updated. - def val(x): - return ConfigurationSetting.for_library(x, library).value - - assert "https://library.library/" == val(Configuration.WEBSITE_URL) - assert "email@example.com" == val( - Configuration.DEFAULT_NOTIFICATION_EMAIL_ADDRESS - ) - assert "help@example.com" == val(Configuration.HELP_EMAIL) - assert "20" == val(Configuration.FEATURED_LANE_SIZE) - assert "0.9" == val(Configuration.MINIMUM_FEATURED_QUALITY) - assert FacetConstants.ORDER_AUTHOR == val( - Configuration.DEFAULT_FACET_KEY_PREFIX - + FacetConstants.ORDER_FACET_GROUP_NAME - ) - assert json.dumps([FacetConstants.ORDER_AUTHOR]) == val( - Configuration.ENABLED_FACETS_KEY_PREFIX - + FacetConstants.ORDER_FACET_GROUP_NAME + assert library.settings.website == "https://library.library/" + assert ( + library.settings.default_notification_email_address == "email@example.com" ) + assert library.settings.help_email == "help@example.com" + assert library.settings.featured_lane_size == 20 + assert library.settings.minimum_featured_quality == 0.9 + assert library.settings.facets_default_order == FacetConstants.ORDER_AUTHOR + assert library.settings.facets_enabled_order == [ + FacetConstants.ORDER_AUTHOR, + FacetConstants.ORDER_RANDOM, + ] # The library-wide logo was not updated and has been left alone. - assert ( - "A tiny image" - == ConfigurationSetting.for_library(Configuration.LOGO, library).value - ) + assert library.logo.content == b"A tiny image" - def test_library_post_empty_values_edit(self, settings_ctrl_fixture): - library = settings_ctrl_fixture.ctrl.db.library( - "New York Public Library", "nypl" + def test_library_post_empty_values_edit( + self, settings_ctrl_fixture, library_fixture: LibraryFixture + ): + settings = library_fixture.mock_settings() + settings.library_description = "description" + library_to_edit = library_fixture.library( + "New York Public Library", "nypl", settings ) - ConfigurationSetting.for_library( - Configuration.LIBRARY_DESCRIPTION, library - ).value = "description" + library_fixture.reset_settings_cache(library_to_edit) with settings_ctrl_fixture.request_context_with_admin("/", method="POST"): flask.request.form = ImmutableMultiDict( [ - ("uuid", library.uuid), + ("uuid", str(library_to_edit.uuid)), ("name", "The New York Public Library"), ("short_name", "nypl"), - (Configuration.LIBRARY_DESCRIPTION, ""), # empty value - (Configuration.WEBSITE_URL, "https://library.library/"), - (Configuration.HELP_EMAIL, "help@example.com"), + ("library_description", ""), # empty value + ("website", "https://library.library/"), + ("help_email", "help@example.com"), ] ) response = ( @@ -639,19 +628,11 @@ def test_library_post_empty_values_edit(self, settings_ctrl_fixture): ) assert response.status_code == 200 - assert ( - ConfigurationSetting.for_library( - Configuration.LIBRARY_DESCRIPTION, library - )._value - == "" - ) - # ConfigurationSetting.value property.getter sets "" to None - assert ( - ConfigurationSetting.for_library( - Configuration.LIBRARY_DESCRIPTION, library - ).value - == None + library = get_one( + settings_ctrl_fixture.ctrl.db.session, Library, uuid=library_to_edit.uuid ) + assert library is not None + assert library.settings.library_description is None def test_library_post_empty_values_create(self, settings_ctrl_fixture): with settings_ctrl_fixture.request_context_with_admin("/", method="POST"): @@ -659,9 +640,9 @@ def test_library_post_empty_values_create(self, settings_ctrl_fixture): [ ("name", "The New York Public Library"), ("short_name", "nypl"), - (Configuration.LIBRARY_DESCRIPTION, ""), # empty value - (Configuration.WEBSITE_URL, "https://library.library/"), - (Configuration.HELP_EMAIL, "help@example.com"), + ("library_description", ""), # empty value + ("website", "https://library.library/"), + ("help_email", "help@example.com"), ] ) response: Response = ( @@ -671,19 +652,7 @@ def test_library_post_empty_values_create(self, settings_ctrl_fixture): uuid = response.get_data(as_text=True) library = get_one(settings_ctrl_fixture.ctrl.db.session, Library, uuid=uuid) - assert ( - ConfigurationSetting.for_library( - Configuration.LIBRARY_DESCRIPTION, library - )._value - == "" - ) - # ConfigurationSetting.value property.getter sets "" to None - assert ( - ConfigurationSetting.for_library( - Configuration.LIBRARY_DESCRIPTION, library - ).value - == None - ) + assert library.settings.library_description is None def test_library_delete(self, settings_ctrl_fixture): library = settings_ctrl_fixture.ctrl.db.library() @@ -707,236 +676,44 @@ def test_library_delete(self, settings_ctrl_fixture): ) assert None == library - def test_library_configuration_settings(self, settings_ctrl_fixture): - # Verify that library_configuration_settings validates and updates every - # setting for a library. - settings = [ - dict(key="setting1", format="format1"), - dict(key="setting2", format="format2"), - ] - - # format1 has a custom validation class; format2 does not. - class MockValidator: - def format_as_string(self, value): - self.format_as_string_called_with = value - return value + ", formatted for storage" - - validator1 = MockValidator() - validators = dict(format1=validator1) - - class MockController(LibrarySettingsController): - succeed = True - _validate_setting_calls = [] - - def _validate_setting(self, library, setting, validator): - self._validate_setting_calls.append((library, setting, validator)) - if self.succeed: - return "validated %s" % setting["key"] - else: - return INVALID_INPUT.detailed("invalid!") - - # Run library_configuration_settings in a situation where all validations succeed. - controller = MockController(settings_ctrl_fixture.manager) - library = settings_ctrl_fixture.ctrl.db.default_library() - result = controller.library_configuration_settings( - library, validators, settings + def test_process_libraries(self, controller_fixture: ControllerFixture): + manager = MagicMock() + controller = LibrarySettingsController(manager) + controller.process_get = MagicMock() # type: ignore[method-assign] + controller.process_post = MagicMock() # type: ignore[method-assign] + + # Make sure we call process_get for a get request + with controller_fixture.request_context_with_library("/", method="GET"): + controller.process_libraries() + + controller.process_get.assert_called_once() + controller.process_post.assert_not_called() + controller.process_get.reset_mock() + controller.process_post.reset_mock() + + # Make sure we call process_post for a post request + with controller_fixture.request_context_with_library("/", method="POST"): + controller.process_libraries() + + controller.process_get.assert_not_called() + controller.process_post.assert_called_once() + controller.process_get.reset_mock() + controller.process_post.reset_mock() + + # For any other request, make sure we return a ProblemDetail + with controller_fixture.request_context_with_library("/", method="PUT"): + resp = controller.process_libraries() + + controller.process_get.assert_not_called() + controller.process_post.assert_not_called() + assert isinstance(resp, ProblemDetail) + + # Make sure that if process_get or process_post raises a ProblemError, + # we return the problem detail. + controller.process_get.side_effect = ProblemError( + problem_detail=INCOMPLETE_CONFIGURATION.detailed("test") ) - - # No problem detail was returned -- the 'request' can continue. - assert None == result - - # _validate_setting was called twice... - [c1, c2] = controller._validate_setting_calls - - # ...once for each item in `settings`. One of the settings was - # of a type with a known validator, so the validator was - # passed in. - assert (library, settings[0], validator1) == c1 - assert (library, settings[1], None) == c2 - - # The 'validated' value from the MockValidator was then formatted - # for storage using the format() method. - assert ( - "validated %s" % settings[0]["key"] - == validator1.format_as_string_called_with - ) - - # Each (validated and formatted) value was written to the - # database. - setting1, setting2 = (library.setting(x["key"]) for x in settings) - assert "validated %s, formatted for storage" % setting1.key == setting1.value - assert "validated %s" % setting2.key == setting2.value - - # Try again in a situation where there are validation failures. - setting1.value = None - setting2.value = None - controller.succeed = False - controller._validate_setting_calls = [] - result = controller.library_configuration_settings( - settings_ctrl_fixture.ctrl.db.default_library(), validators, settings - ) - - # _validate_setting was only called once. - assert [ - (library, settings[0], validator1) - ] == controller._validate_setting_calls - - # When it returned a ProblemDetail, that ProblemDetail - # was propagated outwards. - assert isinstance(result, ProblemDetail) - assert "invalid!" == result.detail - - # No new values were written to the database. - for x in settings: - assert None == library.setting(x["key"]).value - - def test__validate_setting(self, settings_ctrl_fixture): - # Verify the rules for validating different kinds of settings, - # one simulated setting at a time. - - library = settings_ctrl_fixture.ctrl.db.default_library() - - class MockController(LibrarySettingsController): - - # Mock the functions that pull various values out of the - # 'current request' or the 'database' so we don't need an - # actual current request or actual database settings. - def scalar_setting(self, setting): - return self.scalar_form_values.get(setting["key"]) - - def list_setting(self, setting, json_objects=False): - value = self.list_form_values.get(setting["key"]) - if json_objects: - value = [json.loads(x) for x in value] - return json.dumps(value) - - def image_setting(self, setting): - return self.image_form_values.get(setting["key"]) - - def current_value(self, setting, _library): - # While we're here, make sure the right Library - # object was passed in. - assert _library == library - return self.current_values.get(setting["key"]) - - # Now insert mock data into the 'form submission' and - # the 'database'. - - # Simulate list values in a form submission. The geographic values - # go in as normal strings; the announcements go in as strings that are - # JSON-encoded data structures. - announcement_list = [ - {"content": "announcement1"}, - {"content": "announcement2"}, - ] - list_form_values = dict( - geographic_setting=["geographic values"], - announcement_list=[json.dumps(x) for x in announcement_list], - language_codes=["English", "fr"], - list_value=["a list"], - ) - - # Simulate scalar values in a form submission. - scalar_form_values = dict(string_value="a scalar value") - - # Simulate uploaded images in a form submission. - image_form_values = dict(image_setting="some image data") - - # Simulate values present in the database but not present - # in the form submission. - current_values = dict( - value_not_present_in_request="a database value", - previously_uploaded_image="an old image", - ) - - # First test some simple cases: scalar values. - controller = MockController(settings_ctrl_fixture.manager) - m = controller._validate_setting - - # The incoming request has a value for this setting. - assert "a scalar value" == m(library, dict(key="string_value")) - - # But not for this setting: we end up going to the database - # instead. - assert "a database value" == m( - library, dict(key="value_not_present_in_request") - ) - - # And not for this setting either: there is no database value, - # so we have to use the default associated with the setting configuration. - assert "a default value" == m( - library, dict(key="some_other_value", default="a default value") - ) - - # There are some lists which are more complex, but a normal list is - # simple: the return value is the JSON-encoded list. - assert json.dumps(["a list"]) == m(library, dict(key="list_value", type="list")) - - # Now let's look at the more complex lists. - - # A list of language codes. - assert json.dumps(["eng", "fre"]) == m( - library, dict(key="language_codes", format="language-code", type="list") - ) - - # A list of geographic places - class MockGeographicValidator: - value = "validated value" - - def validate_geographic_areas(self, value, _db): - self.called_with = (value, _db) - return self.value - - validator = MockGeographicValidator() - - # The validator was consulted and its response was used as the - # value. - assert "validated value" == m( - library, dict(key="geographic_setting", format="geographic"), validator - ) - assert ( - json.dumps(["geographic values"]), - settings_ctrl_fixture.ctrl.db.session, - ) == validator.called_with - - # Just to be explicit, let's also test the case where the 'response' sent from the - # validator is a ProblemDetail. - validator.value = INVALID_INPUT - assert INVALID_INPUT == m( - library, dict(key="geographic_setting", format="geographic"), validator - ) - - # A list of announcements. - class MockAnnouncementValidator: - value = "validated value" - - def validate_announcements(self, value): - self.called_with = value - return self.value - - validator = MockAnnouncementValidator() - - assert "validated value" == m( - library, dict(key="announcement_list", type="announcements"), validator - ) - assert json.dumps(controller.announcement_list) == validator.called_with - - def test__format_validated_value(self, settings_ctrl_fixture): - - m = LibrarySettingsController._format_validated_value - - # When there is no validator, the incoming value is used as the formatted value, - # unchanged. - value = object() - assert value == m(value, validator=None) - - # When there is a validator, its format_as_string method is - # called, and its return value is used as the formatted value. - class MockValidator: - def format_as_string(self, value): - self.called_with = value - return "formatted value" - - validator = MockValidator() - assert "formatted value" == m(value, validator=validator) - assert value == validator.called_with + with controller_fixture.request_context_with_library("/", method="GET"): + resp = controller.process_libraries() + assert isinstance(resp, ProblemDetail) + assert resp.detail == "test" diff --git a/tests/api/admin/test_geographic_validator.py b/tests/api/admin/test_geographic_validator.py deleted file mode 100644 index 8487d7bb2e..0000000000 --- a/tests/api/admin/test_geographic_validator.py +++ /dev/null @@ -1,350 +0,0 @@ -import json -import urllib.error -import urllib.parse -import urllib.request - -import pypostalcode -import uszipcode - -from api.admin.geographic_validator import GeographicValidator -from api.admin.problem_details import * -from api.registration.registry import RemoteRegistry -from core.model import ExternalIntegration, create -from tests.core.mock import MockRequestsResponse - - -class TestGeographicValidator: - def test_validate_geographic_areas(self, settings_ctrl_fixture): - db = settings_ctrl_fixture.ctrl.db.session - - class Mock(GeographicValidator): - def __init__(self): - self._db = db - self.value = None - - def mock_find_location_through_registry(self, value, db): - self.value = value - - def mock_find_location_through_registry_with_error(self, value, db): - self.value = value - return REMOTE_INTEGRATION_FAILED - - def mock_find_location_through_registry_success(self, value, db): - self.value = value - return "CA" - - mock = Mock() - mock.find_location_through_registry = mock.mock_find_location_through_registry - - # Invalid US zipcode - response = mock.validate_geographic_areas('["00000"]', db) - assert response.uri == UNKNOWN_LOCATION.uri - assert response.detail == '"00000" is not a valid U.S. zipcode.' - assert response.status_code == 400 - # The validator should have returned the problem detail without bothering to ask the registry. - assert mock.value == None - - # Invalid Canadian zipcode - response = mock.validate_geographic_areas('["X1Y"]', db) - assert response.uri == UNKNOWN_LOCATION.uri - assert response.detail == '"X1Y" is not a valid Canadian zipcode.' - # The validator should have returned the problem detail without bothering to ask the registry. - assert mock.value == None - - # Invalid 2-letter abbreviation - response = mock.validate_geographic_areas('["ZZ"]', db) - assert response.uri == UNKNOWN_LOCATION.uri - assert ( - response.detail - == '"ZZ" is not a valid U.S. state or Canadian province abbreviation.' - ) - # The validator should have returned the problem detail without bothering to ask the registry. - assert mock.value == None - - # Validator converts Canadian 2-letter abbreviations into province names, without needing to ask the registry. - response = mock.validate_geographic_areas('["NL"]', db) - assert response == {"CA": ["Newfoundland and Labrador"], "US": []} - assert mock.value == None - - # County with wrong state - response = mock.validate_geographic_areas('["Fairfield County, FL"]', db) - assert response.uri == UNKNOWN_LOCATION.uri - assert response.detail == 'Unable to locate "Fairfield County, FL".' - # The validator should go ahead and call find_location_through_registry - assert mock.value == "Fairfield County, FL" - - # City with wrong state - response = mock.validate_geographic_areas('["Albany, NJ"]', db) - assert response.uri == UNKNOWN_LOCATION.uri - assert response.detail == 'Unable to locate "Albany, NJ".' - # The validator should go ahead and call find_location_through_registry - assert mock.value == "Albany, NJ" - - # The Canadian zip code is valid, but it corresponds to a place too small for the registry to know about it. - response = mock.validate_geographic_areas('["J5J"]', db) - assert response.uri == UNKNOWN_LOCATION.uri - assert ( - response.detail - == 'Unable to locate "J5J" (Saint-Sophie, Quebec). Try entering the name of a larger area.' - ) - assert mock.value == "Saint-Sophie, Quebec" - - # Can't connect to registry - mock.find_location_through_registry = ( - mock.mock_find_location_through_registry_with_error - ) - response = mock.validate_geographic_areas('["Victoria, BC"]', db) - # The controller goes ahead and calls find_location_through_registry, but it can't connect to the registry. - assert response.uri == REMOTE_INTEGRATION_FAILED.uri - - # The registry successfully finds the place - mock.find_location_through_registry = ( - mock.mock_find_location_through_registry_success - ) - response = mock.validate_geographic_areas('["Victoria, BC"]', db) - assert response == {"CA": ["Victoria, BC"], "US": []} - - def test_format_as_string(self): - # GeographicValidator.format_as_string just turns its output into JSON. - value = {"CA": ["Victoria, BC"], "US": []} - as_string = GeographicValidator().format_as_string(value) - assert as_string == json.dumps(value) - - def test_find_location_through_registry(self, settings_ctrl_fixture): - db = settings_ctrl_fixture.ctrl.db.session - get = settings_ctrl_fixture.do_request - original_ask_registry = GeographicValidator().ask_registry - - class Mock(GeographicValidator): - called_with = [] - - def mock_ask_registry(self, service_area_object, db): - places = {"US": ["Chicago"], "CA": ["Victoria, BC"]} - service_area_info = json.loads( - urllib.parse.unquote(service_area_object) - ) - nation = list(service_area_info.keys())[0] - city_or_county = list(service_area_info.values())[0] - if city_or_county == "ERROR": - settings_ctrl_fixture.responses.append(MockRequestsResponse(502)) - elif city_or_county in places[nation]: - self.called_with.append(service_area_info) - settings_ctrl_fixture.responses.append( - MockRequestsResponse( - 200, content=json.dumps(dict(unknown=None, ambiguous=None)) - ) - ) - else: - self.called_with.append(service_area_info) - settings_ctrl_fixture.responses.append( - MockRequestsResponse( - 200, content=json.dumps(dict(unknown=[city_or_county])) - ) - ) - return original_ask_registry(service_area_object, db, get) - - mock = Mock() - mock.ask_registry = mock.mock_ask_registry - - self._registry("https://registry_url", db) - - us_response = mock.find_location_through_registry("Chicago", db) - assert len(mock.called_with) == 1 - assert {"US": "Chicago"} == mock.called_with[0] - assert us_response == "US" - - mock.called_with = [] - - ca_response = mock.find_location_through_registry("Victoria, BC", db) - assert len(mock.called_with) == 2 - assert {"US": "Victoria, BC"} == mock.called_with[0] - assert {"CA": "Victoria, BC"} == mock.called_with[1] - assert ca_response == "CA" - - mock.called_with = [] - - nowhere_response = mock.find_location_through_registry("Not a real place", db) - assert len(mock.called_with) == 2 - assert {"US": "Not a real place"} == mock.called_with[0] - assert {"CA": "Not a real place"} == mock.called_with[1] - assert nowhere_response == None - - error_response = mock.find_location_through_registry("ERROR", db) - assert ( - error_response.detail - == "Unable to contact the registry at https://registry_url." - ) - assert error_response.status_code == 502 - - def test_ask_registry(self, monkeypatch, settings_ctrl_fixture): - validator = GeographicValidator() - db = settings_ctrl_fixture.ctrl.db.session - get = settings_ctrl_fixture.do_request - - registry_1 = "https://registry_1_url" - registry_2 = "https://registry_2_url" - registry_3 = "https://registry_3_url" - registries = self._registries( - [registry_1, registry_2, registry_3], monkeypatch, db - ) - - true_response = MockRequestsResponse(200, content="{}") - unknown_response = MockRequestsResponse(200, content='{"unknown": "place"}') - ambiguous_response = MockRequestsResponse(200, content='{"ambiguous": "place"}') - problem_response = MockRequestsResponse(404) - - # Registry 1 knows about the place - settings_ctrl_fixture.responses.append(true_response) - response_1 = validator.ask_registry(json.dumps({"CA": "Victoria, BC"}), db, get) - assert response_1 == True - assert len(settings_ctrl_fixture.requests) == 1 - request_1 = settings_ctrl_fixture.requests.pop() - assert ( - request_1[0] - == 'https://registry_1_url/coverage?coverage={"CA": "Victoria, BC"}' - ) - - # Registry 1 says the place is unknown, but Registry 2 finds it. - settings_ctrl_fixture.responses.append(true_response) - settings_ctrl_fixture.responses.append(unknown_response) - response_2 = validator.ask_registry(json.dumps({"CA": "Victoria, BC"}), db, get) - assert response_2 == True - assert len(settings_ctrl_fixture.requests) == 2 - request_2 = settings_ctrl_fixture.requests.pop() - assert ( - request_2[0] - == 'https://registry_2_url/coverage?coverage={"CA": "Victoria, BC"}' - ) - request_1 = settings_ctrl_fixture.requests.pop() - assert ( - request_1[0] - == 'https://registry_1_url/coverage?coverage={"CA": "Victoria, BC"}' - ) - - # Registry_1 says the place is ambiguous and Registry_2 says it's unknown, but Registry_3 finds it. - settings_ctrl_fixture.responses.append(true_response) - settings_ctrl_fixture.responses.append(unknown_response) - settings_ctrl_fixture.responses.append(ambiguous_response) - response_3 = validator.ask_registry(json.dumps({"CA": "Victoria, BC"}), db, get) - assert response_3 == True - assert len(settings_ctrl_fixture.requests) == 3 - request_3 = settings_ctrl_fixture.requests.pop() - assert ( - request_3[0] - == 'https://registry_3_url/coverage?coverage={"CA": "Victoria, BC"}' - ) - request_2 = settings_ctrl_fixture.requests.pop() - assert ( - request_2[0] - == 'https://registry_2_url/coverage?coverage={"CA": "Victoria, BC"}' - ) - request_1 = settings_ctrl_fixture.requests.pop() - assert ( - request_1[0] - == 'https://registry_1_url/coverage?coverage={"CA": "Victoria, BC"}' - ) - - # Registry 1 returns a problem detail, but Registry 2 finds the place - settings_ctrl_fixture.responses.append(true_response) - settings_ctrl_fixture.responses.append(problem_response) - response_4 = validator.ask_registry(json.dumps({"CA": "Victoria, BC"}), db, get) - assert response_4 == True - assert len(settings_ctrl_fixture.requests) == 2 - request_2 = settings_ctrl_fixture.requests.pop() - assert ( - request_2[0] - == 'https://registry_2_url/coverage?coverage={"CA": "Victoria, BC"}' - ) - request_1 = settings_ctrl_fixture.requests.pop() - assert ( - request_1[0] - == 'https://registry_1_url/coverage?coverage={"CA": "Victoria, BC"}' - ) - - # Registry 1 returns a problem detail and the other two registries can't find the place - settings_ctrl_fixture.responses.append(unknown_response) - settings_ctrl_fixture.responses.append(ambiguous_response) - settings_ctrl_fixture.responses.append(problem_response) - response_5 = validator.ask_registry(json.dumps({"CA": "Victoria, BC"}), db, get) - assert response_5.status_code == 502 - assert ( - response_5.detail - == "Unable to contact the registry at https://registry_1_url." - ) - assert len(settings_ctrl_fixture.requests) == 3 - request_3 = settings_ctrl_fixture.requests.pop() - assert ( - request_3[0] - == 'https://registry_3_url/coverage?coverage={"CA": "Victoria, BC"}' - ) - request_2 = settings_ctrl_fixture.requests.pop() - assert ( - request_2[0] - == 'https://registry_2_url/coverage?coverage={"CA": "Victoria, BC"}' - ) - request_1 = settings_ctrl_fixture.requests.pop() - assert ( - request_1[0] - == 'https://registry_1_url/coverage?coverage={"CA": "Victoria, BC"}' - ) - - def _registry(self, url, db_session): - integration, is_new = create( - db_session, - ExternalIntegration, - protocol=ExternalIntegration.OPDS_REGISTRATION, - goal=ExternalIntegration.DISCOVERY_GOAL, - ) - integration.url = url - return RemoteRegistry(integration) - - def _registries(self, urls, monkeypatch, db_session): - """Create and mock the `for_protocol_and_goal` function from - RemoteRegistry. Instead of relying on getting the newly created - integrations from the database in a specific order, we return them - in the order they were created. - """ - integrations = [] - for url in urls: - integration, is_new = create( - db_session, - ExternalIntegration, - protocol=ExternalIntegration.OPDS_REGISTRATION, - goal=ExternalIntegration.DISCOVERY_GOAL, - ) - integration.url = url - integrations.append(integration) - - def mock_for_protocol_and_goal(_db, protocol, goal): - for integration in integrations: - yield RemoteRegistry(integration) - - monkeypatch.setattr( - RemoteRegistry, "for_protocol_and_goal", mock_for_protocol_and_goal - ) - - def test_is_zip(self): - validator = GeographicValidator() - assert validator.is_zip("06759", "US") == True - assert validator.is_zip("J2S", "US") == False - assert validator.is_zip("1234", "US") == False - assert validator.is_zip("1a234", "US") == False - - assert validator.is_zip("J2S", "CA") == True - assert validator.is_zip("06759", "CA") == False - assert validator.is_zip("12S", "CA") == False - # "J2S 0A1" is a legit Canadian zipcode, but pypostalcode, which we use for looking up Canadian zipcodes, - # only takes the FSA (the first three characters). - assert validator.is_zip("J2S 0A1", "CA") == False - - def test_look_up_zip(self): - validator = GeographicValidator() - us_zip_unformatted = validator.look_up_zip("06759", "US") - assert isinstance(us_zip_unformatted, uszipcode.SimpleZipcode) - us_zip_formatted = validator.look_up_zip("06759", "US", True) - assert us_zip_formatted == {"06759": "Litchfield, CT"} - - ca_zip_unformatted = validator.look_up_zip("R2V", "CA") - assert isinstance(ca_zip_unformatted, pypostalcode.PostalCode) - ca_zip_formatted = validator.look_up_zip("R2V", "CA", True) - assert ca_zip_formatted == {"R2V": "Winnipeg (Seven Oaks East), Manitoba"} diff --git a/tests/api/admin/test_validator.py b/tests/api/admin/test_validator.py index 0f6782b1c8..63e021ebd0 100644 --- a/tests/api/admin/test_validator.py +++ b/tests/api/admin/test_validator.py @@ -1,10 +1,7 @@ -from io import StringIO - import pytest from werkzeug.datastructures import MultiDict from api.admin.validator import Validator -from api.config import Configuration from tests.api.admin.dummy_validator.dummy_validator import ( DummyAuthenticationProviderValidator, ) @@ -25,21 +22,28 @@ class MockValidations: class TestValidator: def test_validate_email(self): + settings_form = [ + { + "key": "help-email", + "format": "email", + }, + { + "key": "configuration_contact_email_address", + "format": "email", + }, + ] + valid = "valid_format@email.com" invalid = "invalid_format" # One valid input from form form = MultiDict([("help-email", valid)]) - response = Validator().validate_email( - Configuration.LIBRARY_SETTINGS, {"form": form} - ) + response = Validator().validate_email(settings_form, {"form": form}) assert response == None # One invalid input from form form = MultiDict([("help-email", invalid)]) - response = Validator().validate_email( - Configuration.LIBRARY_SETTINGS, {"form": form} - ) + response = Validator().validate_email(settings_form, {"form": form}) assert response.detail == '"invalid_format" is not a valid email address.' assert response.status_code == 400 @@ -47,9 +51,7 @@ def test_validate_email(self): form = MultiDict( [("help-email", valid), ("configuration_contact_email_address", invalid)] ) - response = Validator().validate_email( - Configuration.LIBRARY_SETTINGS, {"form": form} - ) + response = Validator().validate_email(settings_form, {"form": form}) assert response.detail == '"invalid_format" is not a valid email address.' assert response.status_code == 400 @@ -64,23 +66,17 @@ def test_validate_email(self): # Two valid in a list form = MultiDict([("help-email", valid), ("help-email", "valid2@email.com")]) - response = Validator().validate_email( - Configuration.LIBRARY_SETTINGS, {"form": form} - ) + response = Validator().validate_email(settings_form, {"form": form}) assert response == None # One valid and one empty in a list form = MultiDict([("help-email", valid), ("help-email", "")]) - response = Validator().validate_email( - Configuration.LIBRARY_SETTINGS, {"form": form} - ) + response = Validator().validate_email(settings_form, {"form": form}) assert response == None # One valid and one invalid in a list form = MultiDict([("help-email", valid), ("help-email", invalid)]) - response = Validator().validate_email( - Configuration.LIBRARY_SETTINGS, {"form": form} - ) + response = Validator().validate_email(settings_form, {"form": form}) assert response.detail == '"invalid_format" is not a valid email address.' assert response.status_code == 400 @@ -88,26 +84,31 @@ def test_validate_url(self): valid = "https://valid_url.com" invalid = "invalid_url" + settings_form = [ + { + "key": "help-web", + "format": "url", + }, + { + "key": "terms-of-service", + "format": "url", + }, + ] + # Valid form = MultiDict([("help-web", valid)]) - response = Validator().validate_url( - Configuration.LIBRARY_SETTINGS, {"form": form} - ) + response = Validator().validate_url(settings_form, {"form": form}) assert response == None # Invalid form = MultiDict([("help-web", invalid)]) - response = Validator().validate_url( - Configuration.LIBRARY_SETTINGS, {"form": form} - ) + response = Validator().validate_url(settings_form, {"form": form}) assert response.detail == '"invalid_url" is not a valid URL.' assert response.status_code == 400 # One valid, one invalid form = MultiDict([("help-web", valid), ("terms-of-service", invalid)]) - response = Validator().validate_url( - Configuration.LIBRARY_SETTINGS, {"form": form} - ) + response = Validator().validate_url(settings_form, {"form": form}) assert response.detail == '"invalid_url" is not a valid URL.' assert response.status_code == 400 @@ -143,139 +144,77 @@ def test_validate_url(self): assert response.status_code == 400 def test_validate_number(self): + settings_form = [ + { + "key": "hold_limit", + "type": "number", + }, + { + "key": "loan_limit", + "type": "number", + }, + { + "key": "minimum_featured_quality", + "max": 1, + "type": "number", + }, + ] + valid = "10" invalid = "ten" # Valid form = MultiDict([("hold_limit", valid)]) - response = Validator().validate_number( - Configuration.LIBRARY_SETTINGS, {"form": form} - ) + response = Validator().validate_number(settings_form, {"form": form}) assert response == None # Invalid form = MultiDict([("hold_limit", invalid)]) - response = Validator().validate_number( - Configuration.LIBRARY_SETTINGS, {"form": form} - ) + response = Validator().validate_number(settings_form, {"form": form}) assert response.detail == '"ten" is not a number.' assert response.status_code == 400 # One valid, one invalid form = MultiDict([("hold_limit", valid), ("loan_limit", invalid)]) - response = Validator().validate_number( - Configuration.LIBRARY_SETTINGS, {"form": form} - ) + response = Validator().validate_number(settings_form, {"form": form}) assert response.detail == '"ten" is not a number.' assert response.status_code == 400 # Invalid: below minimum form = MultiDict([("hold_limit", -5)]) - response = Validator().validate_number( - Configuration.LIBRARY_SETTINGS, {"form": form} - ) - assert ( - response.detail - == "Maximum number of books a patron can have on hold at once must be greater than 0." - ) + response = Validator().validate_number(settings_form, {"form": form}) + assert "must be greater than 0." in response.detail assert response.status_code == 400 # Valid: below maximum form = MultiDict([("minimum_featured_quality", ".9")]) - response = Validator().validate_number( - Configuration.LIBRARY_SETTINGS, {"form": form} - ) + response = Validator().validate_number(settings_form, {"form": form}) assert response == None # Invalid: above maximum form = MultiDict([("minimum_featured_quality", "2")]) - response = Validator().validate_number( - Configuration.LIBRARY_SETTINGS, {"form": form} - ) - assert ( - response.detail - == "Minimum quality for books that show up in 'featured' lanes cannot be greater than 1." - ) + response = Validator().validate_number(settings_form, {"form": form}) + assert "cannot be greater than 1." in response.detail assert response.status_code == 400 - def test_validate_language_code(self): - all_valid = ["eng", "spa", "ita"] - all_invalid = ["abc", "def", "ghi"] - mixed = ["eng", "abc", "spa"] - - form = MultiDict([("large_collections", all_valid)]) - response = Validator().validate_language_code( - Configuration.LIBRARY_SETTINGS, {"form": form} - ) - assert response == None - - form = MultiDict([("large_collections", all_invalid)]) - response = Validator().validate_language_code( - Configuration.LIBRARY_SETTINGS, {"form": form} - ) - assert response.detail == '"abc" is not a valid language code.' - assert response.status_code == 400 - - form = MultiDict([("large_collections", mixed)]) - response = Validator().validate_language_code( - Configuration.LIBRARY_SETTINGS, {"form": form} - ) - assert response.detail == '"abc" is not a valid language code.' - assert response.status_code == 400 - - form = MultiDict( - [ - ("large_collections", all_valid), - ("small_collections", all_valid), - ("tiny_collections", mixed), - ] - ) - response = Validator().validate_language_code( - Configuration.LIBRARY_SETTINGS, {"form": form} - ) - assert response.detail == '"abc" is not a valid language code.' - assert response.status_code == 400 - - def test_validate_image(self): - def create_image_file(format_string): - image_data = "\x89PNG\r\n\x1a\n\x00\x00\x00\rIHDR\x00\x00\x00\x01\x00\x00\x00\x01\x01\x03\x00\x00\x00%\xdbV\xca\x00\x00\x00\x06PLTE\xffM\x00\x01\x01\x01\x8e\x1e\xe5\x1b\x00\x00\x00\x01tRNS\xcc\xd24V\xfd\x00\x00\x00\nIDATx\x9cc`\x00\x00\x00\x02\x00\x01H\xaf\xa4q\x00\x00\x00\x00IEND\xaeB`\x82" - - class TestImageFile(StringIO): - headers = {"Content-Type": "image/" + format_string} - - return TestImageFile(image_data) - return result - - [png, jpeg, gif, invalid] = [ - MultiDict([(Configuration.LOGO, create_image_file(x))]) - for x in ["png", "jpeg", "gif", "abc"] - ] - - png_response = Validator().validate_image( - Configuration.LIBRARY_SETTINGS, {"files": png} - ) - assert png_response == None - jpeg_response = Validator().validate_image( - Configuration.LIBRARY_SETTINGS, {"files": jpeg} - ) - assert jpeg_response == None - gif_response = Validator().validate_image( - Configuration.LIBRARY_SETTINGS, {"files": gif} - ) - assert gif_response == None - - abc_response = Validator().validate_image( - Configuration.LIBRARY_SETTINGS, {"files": invalid} - ) - assert ( - abc_response.detail - == "Upload for Logo image must be in GIF, PNG, or JPG format. (Upload was image/abc.)" - ) - assert abc_response.status_code == 400 - def test_validate(self): called = [] + settings_form = [ + { + "key": "hold_limit", + "type": "number", + }, + { + "key": "help-web", + "format": "url", + }, + { + "key": "configuration_contact_email_address", + "format": "email", + }, + ] + class Mock(Validator): def validate_email(self, settings, content): called.append("validate_email") @@ -286,19 +225,11 @@ def validate_url(self, settings, content): def validate_number(self, settings, content): called.append("validate_number") - def validate_language_code(self, settings, content): - called.append("validate_language_code") - - def validate_image(self, settings, content): - called.append("validate_image") - - Mock().validate(Configuration.LIBRARY_SETTINGS, {}) + Mock().validate(settings_form, {}) assert called == [ "validate_email", "validate_url", "validate_number", - "validate_language_code", - "validate_image", ] def test__is_url(self): diff --git a/tests/api/conftest.py b/tests/api/conftest.py index 15e528a2e8..fc1309236c 100644 --- a/tests/api/conftest.py +++ b/tests/api/conftest.py @@ -30,6 +30,7 @@ "tests.fixtures.database", "tests.fixtures.files", "tests.fixtures.flask", + "tests.fixtures.library", "tests.fixtures.marc_files", "tests.fixtures.odl", "tests.fixtures.opds2_files", diff --git a/tests/api/test_adobe_vendor_id.py b/tests/api/test_adobe_vendor_id.py index 895af7a0d8..5eccce5523 100644 --- a/tests/api/test_adobe_vendor_id.py +++ b/tests/api/test_adobe_vendor_id.py @@ -6,12 +6,12 @@ from jwt import DecodeError, ExpiredSignatureError, InvalidIssuedAtError from api.adobe_vendor_id import AuthdataUtility -from api.config import Configuration from api.registration.constants import RegistrationConstants from core.config import CannotLoadConfiguration from core.model import ConfigurationSetting, ExternalIntegration from core.util.datetime_helpers import datetime_utc, utc_now from tests.fixtures.database import DatabaseTransactionFixture +from tests.fixtures.library import LibraryFixture from tests.fixtures.vendor_id import VendorIDFixture @@ -63,11 +63,14 @@ def test_eligible_authdata_vendor_id_integrations( assert isinstance(utility, authdata_utility_type) def test_from_config( - self, authdata: AuthdataUtility, vendor_id_fixture: VendorIDFixture + self, + authdata: AuthdataUtility, + vendor_id_fixture: VendorIDFixture, + library_fixture: LibraryFixture, ): - library = vendor_id_fixture.db.default_library() + library = library_fixture.library() vendor_id_fixture.initialize_adobe(library) - library_url = library.setting(Configuration.WEBSITE_URL).value + library_url = library.settings.website utility = AuthdataUtility.from_config(library) assert utility is not None @@ -133,11 +136,11 @@ def test_from_config( pytest.raises(CannotLoadConfiguration, AuthdataUtility.from_config, library) setting.value = old_short_name - setting = library.setting(Configuration.WEBSITE_URL) - old_value = setting.value - setting.value = None + library_settings = library_fixture.settings(library) + old_website = library_settings.website + library_settings.website = None # type: ignore[assignment] pytest.raises(CannotLoadConfiguration, AuthdataUtility.from_config, library) - setting.value = old_value + library_settings.website = old_website setting = ConfigurationSetting.for_library_and_externalintegration( vendor_id_fixture.db.session, diff --git a/tests/api/test_authenticator.py b/tests/api/test_authenticator.py index 4efde02159..b2352e0899 100644 --- a/tests/api/test_authenticator.py +++ b/tests/api/test_authenticator.py @@ -9,7 +9,7 @@ import re from decimal import Decimal from functools import partial -from typing import TYPE_CHECKING, Callable, Literal, Optional, Tuple, cast +from typing import TYPE_CHECKING, Callable, Literal, Tuple, cast from unittest.mock import MagicMock, PropertyMock, patch import flask @@ -40,7 +40,6 @@ from api.custom_patron_catalog import CustomPatronCatalog from api.integration.registry.patron_auth import PatronAuthRegistry from api.millenium_patron import MilleniumPatronAPI -from api.opds import LibraryAnnotator from api.problem_details import * from api.problem_details import PATRON_OF_ANOTHER_LIBRARY from api.simple_authentication import SimpleAuthenticationProvider @@ -51,7 +50,6 @@ from core.integration.registry import IntegrationRegistry from core.mock_analytics_provider import MockAnalyticsProvider from core.model import CirculationEvent, ConfigurationSetting, Library, Patron -from core.model.constants import LinkRelations from core.model.integration import ( IntegrationConfiguration, IntegrationLibraryConfiguration, @@ -65,6 +63,7 @@ from core.util.problem_detail import ProblemDetail from ..fixtures.announcements import AnnouncementFixture +from ..fixtures.library import LibraryFixture if TYPE_CHECKING: from ..fixtures.api_controller import ControllerFixture @@ -958,6 +957,7 @@ def test_create_authentication_document( db: DatabaseTransactionFixture, mock_basic: MockBasicFixture, announcement_fixture: AnnouncementFixture, + library_fixture: LibraryFixture, ): class MockAuthenticator(LibraryAuthenticator): """Mock the _geographic_areas method.""" @@ -968,7 +968,8 @@ class MockAuthenticator(LibraryAuthenticator): def _geographic_areas(cls, library): return cls.AREAS - library = db.default_library() + library = library_fixture.library() + library_settings = library_fixture.settings(library) basic = mock_basic() library.name = "A Fabulous Library" authenticator = MockAuthenticator( @@ -996,54 +997,32 @@ def annotate_authentication_document(library, doc, url_for): del os.environ["AUTOINITIALIZE"] # Set up configuration settings for links. - link_config = { - LibraryAnnotator.TERMS_OF_SERVICE: "http://terms", - LibraryAnnotator.PRIVACY_POLICY: "http://privacy", - LibraryAnnotator.COPYRIGHT: "http://copyright", - LibraryAnnotator.ABOUT: "http://about", - LibraryAnnotator.LICENSE: "http://license/", - LibraryAnnotator.REGISTER: "custom-registration-hook://library/", - LinkRelations.PATRON_PASSWORD_RESET: "https://example.org/reset", - Configuration.WEB_CSS_FILE: "http://style.css", - } - - for rel, value in link_config.items(): - ConfigurationSetting.for_library(rel, db.default_library()).value = value - - db.default_library().logo = LibraryLogo(content=b"image data") - - ConfigurationSetting.for_library( - Configuration.LIBRARY_DESCRIPTION, library - ).value = "Just the best." + library_settings.terms_of_service = "http://terms.com" # type: ignore[assignment] + library_settings.privacy_policy = "http://privacy.com" # type: ignore[assignment] + library_settings.copyright = "http://copyright.com" # type: ignore[assignment] + library_settings.license = "http://license.ca/" # type: ignore[assignment] + library_settings.about = "http://about.io" # type: ignore[assignment] + library_settings.registration_url = "https://library.org/register" # type: ignore[assignment] + library_settings.patron_password_reset = "https://example.org/reset" # type: ignore[assignment] + library_settings.web_css_file = "http://style.css" # type: ignore[assignment] + + library.logo = LibraryLogo(content=b"image data") + + library_settings.library_description = "Just the best." # Set the URL to the library's web page. - ConfigurationSetting.for_library( - Configuration.WEBSITE_URL, library - ).value = "http://library/" + library_settings.website = "http://library.org/" # type: ignore[assignment] # Set the color scheme a mobile client should use. - ConfigurationSetting.for_library( - Configuration.COLOR_SCHEME, library - ).value = "plaid" + library_settings.color_scheme = "plaid" # Set the colors a web client should use. - ConfigurationSetting.for_library( - Configuration.WEB_PRIMARY_COLOR, library - ).value = "#012345" - ConfigurationSetting.for_library( - Configuration.WEB_SECONDARY_COLOR, library - ).value = "#abcdef" + library_settings.web_primary_color = "#012345" + library_settings.web_secondary_color = "#abcdef" # Configure the various ways a patron can get help. - ConfigurationSetting.for_library( - Configuration.HELP_EMAIL, library - ).value = "help@library" - ConfigurationSetting.for_library( - Configuration.HELP_WEB, library - ).value = "http://library.help/" - ConfigurationSetting.for_library( - Configuration.HELP_URI, library - ).value = "custom:uri" + library_settings.help_email = "help@library.org" # type: ignore[assignment] + library_settings.help_web = "http://library.help/" # type: ignore[assignment] base_url = ConfigurationSetting.sitewide(db.session, Configuration.BASE_URL_KEY) base_url.value = "http://circulation-manager/" @@ -1109,18 +1088,12 @@ def annotate_authentication_document(library, doc, url_for): assert "#012345" == doc["web_color_scheme"]["primary"] assert "#abcdef" == doc["web_color_scheme"]["secondary"] - # _geographic_areas was called and provided the library's - # focus area and service area. - assert "focus area" == doc["focus_area"] - assert "service area" == doc["service_area"] - # We also need to test that the links got pulled in # from the configuration. ( about, alternate, copyright, - help_uri, help_web, help_email, copyright_agent, @@ -1135,11 +1108,11 @@ def annotate_authentication_document(library, doc, url_for): stylesheet, terms_of_service, ) = sorted(doc["links"], key=lambda x: (x["rel"], x["href"])) - assert "http://terms" == terms_of_service["href"] - assert "http://privacy" == privacy_policy["href"] - assert "http://copyright" == copyright["href"] - assert "http://about" == about["href"] - assert "http://license/" == license["href"] + assert "http://terms.com" == terms_of_service["href"] + assert "http://privacy.com" == privacy_policy["href"] + assert "http://copyright.com" == copyright["href"] + assert "http://about.io" == about["href"] + assert "http://license.ca/" == license["href"] assert "data:image/png;base64,image data" == logo["href"] assert "http://style.css" == stylesheet["href"] @@ -1153,7 +1126,7 @@ def annotate_authentication_document(library, doc, url_for): expect_start = url_for( "index", - library_short_name=db.default_library().short_name, + library_short_name=library.short_name, _external=True, ) assert expect_start == start["href"] @@ -1164,21 +1137,18 @@ def annotate_authentication_document(library, doc, url_for): # Most of the other links have type='text/html' assert "text/html" == about["type"] - # The registration link doesn't have a type, because it - # uses a non-HTTP URI scheme. - assert "type" not in register - assert "custom-registration-hook://library/" == register["href"] + # The registration link + assert "https://library.org/register" == register["href"] assert "https://example.org/reset" == reset_link["href"] # The logo link has type "image/png". assert "image/png" == logo["type"] - # We have three help links. - assert "custom:uri" == help_uri["href"] + # We have two help links. assert "http://library.help/" == help_web["href"] assert "text/html" == help_web["type"] - assert "mailto:help@library" == help_email["href"] + assert "mailto:help@library.org" == help_email["href"] # Since no special address was given for the copyright # designated agent, the help address was reused. @@ -1186,7 +1156,7 @@ def annotate_authentication_document(library, doc, url_for): "http://librarysimplified.org/rel/designated-agent/copyright" ) assert copyright_rel == copyright_agent["rel"] - assert "mailto:help@library" == copyright_agent["href"] + assert "mailto:help@library.org" == copyright_agent["href"] # The public key is correct. assert authenticator.library is not None @@ -1197,7 +1167,7 @@ def annotate_authentication_document(library, doc, url_for): # The library's web page shows up as an HTML alternate # to the OPDS server. assert ( - dict(rel="alternate", type="text/html", href="http://library/") + dict(rel="alternate", type="text/html", href="http://library.org/") == alternate ) @@ -1216,9 +1186,7 @@ def annotate_authentication_document(library, doc, url_for): # If a separate copyright designated agent is configured, # that email address is used instead of the default # patron support address. - ConfigurationSetting.for_library( - Configuration.COPYRIGHT_DESIGNATED_AGENT_EMAIL, library - ).value = "mailto:dmca@library.org" + library_settings.copyright_designated_agent_email_address = "dmca@library.org" # type: ignore[assignment] doc = json.loads(authenticator.create_authentication_document()) [agent] = [x for x in doc["links"] if x["rel"] == copyright_rel] assert "mailto:dmca@library.org" == agent["href"] @@ -1278,65 +1246,6 @@ def annotate_authentication_document(library, doc, url_for): headers = real_authenticator.create_authentication_headers() assert "WWW-Authenticate" not in headers - def test__geographic_areas(self, db: DatabaseTransactionFixture): - """Test the _geographic_areas helper method.""" - - class Mock(LibraryAuthenticator): - called_with: Optional[Library] = None - - values = { - Configuration.LIBRARY_FOCUS_AREA: "focus", - Configuration.LIBRARY_SERVICE_AREA: "service", - } - - @classmethod - def _geographic_area(cls, key, library): - cls.called_with = library - return cls.values.get(key) - - # _geographic_areas calls _geographic_area twice and - # returns the results in a 2-tuple. - m = Mock._geographic_areas - library = object() - assert ("focus", "service") == m(library) # type: ignore - assert library == Mock.called_with - - # If only one value is provided, the same value is given for both - # areas. - del Mock.values[Configuration.LIBRARY_FOCUS_AREA] - assert ("service", "service") == m(library) # type: ignore - - Mock.values[Configuration.LIBRARY_FOCUS_AREA] = "focus" - del Mock.values[Configuration.LIBRARY_SERVICE_AREA] - assert ("focus", "focus") == m(library) # type: ignore - - def test__geographic_area(self, db: DatabaseTransactionFixture): - """Test the _geographic_area helper method.""" - library = db.default_library() - key = "a key" - setting = ConfigurationSetting.for_library(key, library) - - def m(): - return LibraryAuthenticator._geographic_area(key, library) - - # A missing value is returned as None. - assert m() is None - - # The literal string "everywhere" is returned as is. - setting.value = "everywhere" - assert "everywhere" == m() - - # A string that makes sense as JSON is returned as its JSON - # equivalent. - two_states = ["NY", "NJ"] - setting.value = json.dumps(two_states) - assert two_states == m() - - # A string that does not make sense as JSON is put in a - # single-element list. - setting.value = "Arvin, CA" - assert ["Arvin, CA"] == m() - class TestBasicAuthenticationProvider: diff --git a/tests/api/test_axis.py b/tests/api/test_axis.py index edc9b5c4a7..ab1a5717d4 100644 --- a/tests/api/test_axis.py +++ b/tests/api/test_axis.py @@ -32,7 +32,6 @@ ) from api.circulation import FulfillmentInfo, HoldInfo, LoanInfo from api.circulation_exceptions import * -from api.config import Configuration from api.web_publication_manifest import FindawayManifest, SpineItem from core.coverage import CoverageFailure from core.metadata_layer import ( @@ -45,7 +44,6 @@ ) from core.mock_analytics_provider import MockAnalyticsProvider from core.model import ( - ConfigurationSetting, Contributor, DataSource, DeliveryMechanism, @@ -66,6 +64,8 @@ from core.util.problem_detail import ProblemDetail, ProblemError from tests.api.mockapi.axis import MockAxis360API +from ..fixtures.library import LibraryFixture + if TYPE_CHECKING: from ..fixtures.api_axis_files import AxisFilesFixture from ..fixtures.authenticator import AuthProviderFixture @@ -390,7 +390,7 @@ def test_checkin_failure(self, axis360: Axis360Fixture): patron.authorization_identifier = axis360.db.fresh_str() pytest.raises(NotFoundOnRemote, axis360.api.checkin, patron, "pin", pool) - def test_place_hold(self, axis360: Axis360Fixture): + def test_place_hold(self, axis360: Axis360Fixture, library_fixture: LibraryFixture): edition, pool = axis360.db.edition( identifier_type=Identifier.AXIS_360_ID, data_source_name=DataSource.AXIS_360, @@ -398,11 +398,13 @@ def test_place_hold(self, axis360: Axis360Fixture): ) data = axis360.sample_data("place_hold_success.xml") axis360.api.queue_response(200, content=data) - patron = axis360.db.patron() - ConfigurationSetting.for_library( - Configuration.DEFAULT_NOTIFICATION_EMAIL_ADDRESS, - axis360.db.default_library(), - ).value = "notifications@example.com" + library = library_fixture.library() + library_settings = library_fixture.settings(library) + patron = axis360.db.patron(library=library) + library_settings.default_notification_email_address = ( + "notifications@example.com" # type: ignore[assignment] + ) + response = axis360.api.place_hold(patron, "pin", pool, None) assert 1 == response.hold_position assert response.identifier_type == pool.identifier.type diff --git a/tests/api/test_circulation_exceptions.py b/tests/api/test_circulation_exceptions.py index c7acb95965..ffe9479a8a 100644 --- a/tests/api/test_circulation_exceptions.py +++ b/tests/api/test_circulation_exceptions.py @@ -1,7 +1,6 @@ from flask_babel import lazy_gettext as _ from api.circulation_exceptions import * -from api.config import Configuration from api.problem_details import * from core.util.problem_detail import ProblemDetail from tests.fixtures.database import DatabaseTransactionFixture @@ -40,47 +39,19 @@ def test_as_problem_detail_document(self, db: DatabaseTransactionFixture): "You exceeded the limit, but I don't know what the limit was." ) pd = ProblemDetail("http://uri/", 403, _("Limit exceeded."), generic_message) - setting = "some setting" class Mock(LimitReached): BASE_DOC = pd - SETTING_NAME = setting MESSAGE_WITH_LIMIT = _("The limit was %(limit)d.") # No limit -> generic message. - ex = Mock(library=db.default_library()) - pd = ex.as_problem_detail_document() - assert None == ex.limit - assert generic_message == pd.detail - - # Limit but no library -> generic message. - db.default_library().setting(setting).value = 14 ex = Mock() - assert None == ex.limit pd = ex.as_problem_detail_document() + assert ex.limit is None assert generic_message == pd.detail - # Limit and library -> specific message. - ex = Mock(library=db.default_library()) + # Limit -> specific message. + ex = Mock(limit=14) assert 14 == ex.limit pd = ex.as_problem_detail_document() assert "The limit was 14." == pd.detail - - def test_subclasses(self, db: DatabaseTransactionFixture): - # Use end-to-end tests to verify that the subclasses of - # LimitReached define the right constants. - library = db.default_library() - - library.setting(Configuration.LOAN_LIMIT).value = 2 - pd = PatronLoanLimitReached(library=library).as_problem_detail_document() - assert ( - "You have reached your loan limit of 2. You cannot borrow anything further until you return something." - == pd.detail - ) - - library.setting(Configuration.HOLD_LIMIT).value = 3 - pd = PatronHoldLimitReached(library=library).as_problem_detail_document() - assert ( - "You have reached your hold limit of 3. You cannot place another item on hold until you borrow something or remove a hold." - == pd.detail - ) diff --git a/tests/api/test_circulationapi.py b/tests/api/test_circulationapi.py index 1f3ad990b5..cb8f020911 100644 --- a/tests/api/test_circulationapi.py +++ b/tests/api/test_circulationapi.py @@ -24,7 +24,6 @@ from core.mock_analytics_provider import MockAnalyticsProvider from core.model import ( CirculationEvent, - ConfigurationSetting, DataSource, DeliveryMechanism, ExternalIntegration, @@ -43,6 +42,7 @@ from ..fixtures.api_bibliotheca_files import BibliothecaFilesFixture from ..fixtures.database import DatabaseTransactionFixture +from ..fixtures.library import LibraryFixture class CirculationAPIFixture: @@ -531,7 +531,7 @@ def test_borrow_with_expired_card_fails( circulation_api.patron.authorization_expires = old_expires def test_borrow_with_outstanding_fines( - self, circulation_api: CirculationAPIFixture + self, circulation_api: CirculationAPIFixture, library_fixture: LibraryFixture ): # This checkout would succeed... now = utc_now() @@ -548,19 +548,18 @@ def test_borrow_with_outstanding_fines( # ...except the patron has too many fines. old_fines = circulation_api.patron.fines circulation_api.patron.fines = 1000 - setting = ConfigurationSetting.for_library( - Configuration.MAX_OUTSTANDING_FINES, circulation_api.db.default_library() - ) - setting.value = "$0.50" + library = circulation_api.db.default_library() + library_settings = library_fixture.settings(library) + library_settings.max_outstanding_fines = 0.50 pytest.raises(OutstandingFines, lambda: self.borrow(circulation_api)) - # Test the case where any amount of fines are too much. - setting.value = "$0" + # Test the case where any amount of fines is too much. + library_settings.max_outstanding_fines = 0 pytest.raises(OutstandingFines, lambda: self.borrow(circulation_api)) # Remove the fine policy, and borrow succeeds. - setting.value = None + library_settings.max_outstanding_fines = None loan, i1, i2 = self.borrow(circulation_api) assert isinstance(loan, Loan) @@ -641,9 +640,11 @@ def api_for_license_pool(self, pool): (circulation_api.patron, circulation_api.pool) ] == circulation_api.circulation.enforce_limits_calls # type: ignore[attr-defined] - def test_patron_at_loan_limit(self, circulation_api: CirculationAPIFixture): + def test_patron_at_loan_limit( + self, circulation_api: CirculationAPIFixture, library_fixture: LibraryFixture + ): # The loan limit is a per-library setting. - setting = circulation_api.patron.library.setting(Configuration.LOAN_LIMIT) + settings = library_fixture.settings(circulation_api.patron.library) future = utc_now() + timedelta(hours=1) @@ -671,28 +672,31 @@ def test_patron_at_loan_limit(self, circulation_api: CirculationAPIFixture): # patron_at_loan_limit returns True if your number of relevant # loans equals or exceeds the limit. m = circulation_api.circulation.patron_at_loan_limit - assert None == setting.value - assert False == m(patron) + assert settings.loan_limit is None + assert m(patron) is False - setting.value = 1 - assert True == m(patron) - setting.value = 2 - assert True == m(patron) - setting.value = 3 - assert False == m(patron) + settings.loan_limit = 1 + assert m(patron) is True + settings.loan_limit = 2 + assert m(patron) is True + settings.loan_limit = 3 + assert m(patron) is False # Setting the loan limit to 0 is treated the same as disabling it. - setting.value = 0 - assert False == m(patron) + settings.loan_limit = 0 + assert m(patron) is False # Another library's setting doesn't affect your limit. other_library = circulation_api.db.library() - other_library.setting(Configuration.LOAN_LIMIT).value = 1 - assert False == m(patron) + library_fixture.settings(other_library).loan_limit = 1 + assert False is m(patron) - def test_patron_at_hold_limit(self, circulation_api: CirculationAPIFixture): + def test_patron_at_hold_limit( + self, circulation_api: CirculationAPIFixture, library_fixture: LibraryFixture + ): # The hold limit is a per-library setting. - setting = circulation_api.patron.library.setting(Configuration.HOLD_LIMIT) + library = circulation_api.patron.library + library_settings = library_fixture.settings(library) # Unlike the loan limit, it's pretty simple -- every hold counts towards your limit. patron = circulation_api.patron @@ -707,26 +711,28 @@ def test_patron_at_hold_limit(self, circulation_api: CirculationAPIFixture): # patron_at_hold_limit returns True if your number of holds # equals or exceeds the limit. m = circulation_api.circulation.patron_at_hold_limit - assert None == setting.value - assert False == m(patron) + assert library.settings.hold_limit == None + assert m(patron) is False - setting.value = 1 - assert True == m(patron) - setting.value = 2 - assert True == m(patron) - setting.value = 3 - assert False == m(patron) + library_settings.hold_limit = 1 + assert m(patron) is True + library_settings.hold_limit = 2 + assert m(patron) is True + library_settings.hold_limit = 3 + assert m(patron) is False # Setting the hold limit to 0 is treated the same as disabling it. - setting.value = 0 - assert False == m(patron) + library_settings.hold_limit = 0 + assert m(patron) is False # Another library's setting doesn't affect your limit. - other_library = circulation_api.db.library() - other_library.setting(Configuration.HOLD_LIMIT).value = 1 - assert False == m(patron) + other_library = library_fixture.library() + library_fixture.settings(other_library).hold_limit = 1 + assert m(patron) is False - def test_enforce_limits(self, circulation_api: CirculationAPIFixture): + def test_enforce_limits( + self, circulation_api: CirculationAPIFixture, library_fixture: LibraryFixture + ): # Verify that enforce_limits works whether the patron is at one, both, # or neither of their loan limits. @@ -745,13 +751,11 @@ def update_availability(self, pool): # # Both limits are set to the same value for the sake of # convenience in testing. - circulation_api.db.default_library().setting( - Configuration.LOAN_LIMIT - ).value = 12 - circulation_api.db.default_library().setting( - Configuration.HOLD_LIMIT - ).value = 12 + mock_settings = library_fixture.mock_settings() + mock_settings.loan_limit = 12 + mock_settings.hold_limit = 12 + library = library_fixture.library(settings=mock_settings) api = MockVendorAPI() class Mock(MockCirculationAPI): @@ -781,13 +785,11 @@ def patron_at_hold_limit(self, patron): self.patron_at_hold_limit_calls.append(patron) return self.at_hold_limit - circulation = Mock( - circulation_api.db.session, circulation_api.db.default_library() - ) + circulation = Mock(circulation_api.db.session, library) # Sub-test 1: patron has reached neither limit. # - patron = circulation_api.patron + patron = circulation_api.db.patron(library=library) pool = object() circulation.at_loan_limit = False circulation.at_hold_limit = False @@ -809,24 +811,16 @@ def patron_at_hold_limit(self, patron): circulation.at_loan_limit = True circulation.at_hold_limit = True - # We can't use assert_raises here because we need to examine the - # exception object to make sure it was properly instantiated. - def assert_enforce_limits_raises(expected_exception): - try: - circulation.enforce_limits(patron, pool) - raise Exception("Expected a %r" % expected_exception) - except Exception as e: - assert isinstance(e, expected_exception) - # If .limit is set it means we were able to find a - # specific limit in the database, which means the - # exception was instantiated correctly. - # - # The presence of .limit will let us give a more specific - # error message when the exception is converted to a - # problem detail document. - assert 12 == e.limit - - assert_enforce_limits_raises(PatronLoanLimitReached) + with pytest.raises(PatronLoanLimitReached) as excinfo: + circulation.enforce_limits(patron, pool) + # If .limit is set it means we were able to find a + # specific limit, which means the exception was instantiated + # correctly. + # + # The presence of .limit will let us give a more specific + # error message when the exception is converted to a + # problem detail document. + assert 12 == excinfo.value.limit # We were able to deduce that the patron can't do anything # with this book, without having to ask the API about @@ -845,7 +839,9 @@ def assert_enforce_limits_raises(expected_exception): # If the book is available, we get PatronLoanLimitReached pool.licenses_available = 1 # type: ignore - assert_enforce_limits_raises(PatronLoanLimitReached) + with pytest.raises(PatronLoanLimitReached) as loan_limit_info: + circulation.enforce_limits(patron, pool) + assert 12 == loan_limit_info.value.limit # Reaching this conclusion required checking both patron # limits and asking the remote API for updated availability @@ -869,7 +865,9 @@ def assert_enforce_limits_raises(expected_exception): # If the book is not available, we get PatronHoldLimitReached pool.licenses_available = 0 # type: ignore - assert_enforce_limits_raises(PatronHoldLimitReached) + with pytest.raises(PatronHoldLimitReached) as hold_limit_info: + circulation.enforce_limits(patron, pool) + assert 12 == hold_limit_info.value.limit # Reaching this conclusion required checking both patron # limits and asking the remote API for updated availability @@ -885,14 +883,16 @@ def assert_enforce_limits_raises(expected_exception): assert patron == circulation.patron_at_hold_limit_calls.pop() assert pool == api.availability_updated.pop() - def test_borrow_hold_limit_reached(self, circulation_api: CirculationAPIFixture): + def test_borrow_hold_limit_reached( + self, circulation_api: CirculationAPIFixture, library_fixture: LibraryFixture + ): # Verify that you can't place a hold on an unavailable book # if you're at your hold limit. # # NOTE: This is redundant except as an end-to-end test. # The hold limit is 1, and the patron has a previous hold. - circulation_api.patron.library.setting(Configuration.HOLD_LIMIT).value = 1 + library_fixture.settings(circulation_api.patron.library).hold_limit = 1 other_pool = circulation_api.db.licensepool(None) other_pool.open_access = False other_pool.on_hold_to(circulation_api.patron) @@ -908,7 +908,7 @@ def test_borrow_hold_limit_reached(self, circulation_api: CirculationAPIFixture) assert 1 == e.limit # If we increase the limit, borrow succeeds. - circulation_api.patron.library.setting(Configuration.HOLD_LIMIT).value = 2 + library_fixture.settings(circulation_api.patron.library).hold_limit = 2 circulation_api.remote.queue_checkout(NoAvailableCopies()) now = utc_now() holdinfo = HoldInfo( @@ -1611,17 +1611,20 @@ def can_fulfill_without_loan(self, patron, pool, lpdm): class TestBaseCirculationAPI: - def test_default_notification_email_address(self, db: DatabaseTransactionFixture): + def test_default_notification_email_address( + self, db: DatabaseTransactionFixture, library_fixture: LibraryFixture + ): # Test the ability to get the default notification email address # for a patron or a library. - db.default_library().setting( - Configuration.DEFAULT_NOTIFICATION_EMAIL_ADDRESS - ).value = "help@library" + settings = library_fixture.mock_settings() + settings.default_notification_email_address = "help@library" # type: ignore[assignment] + library = library_fixture.library(settings=settings) + patron = db.patron(library=library) m = BaseCirculationAPI.default_notification_email_address - assert "help@library" == m(db.default_library(), None) - assert "help@library" == m(db.patron(), None) - other_library = db.library() - assert None == m(other_library, None) + assert "help@library" == m(library, "") + assert "help@library" == m(patron, "") + other_library = library_fixture.library() + assert "noreply@thepalaceproject.org" == m(other_library, "") def test_can_fulfill_without_loan(self, db: DatabaseTransactionFixture): """By default, there is a blanket prohibition on fulfilling a title diff --git a/tests/api/test_config.py b/tests/api/test_config.py index af24717359..76a00f3bb5 100644 --- a/tests/api/test_config.py +++ b/tests/api/test_config.py @@ -10,9 +10,10 @@ from api.config import Configuration from core.config import CannotLoadConfiguration from core.config import Configuration as CoreConfiguration -from core.model import ConfigurationSetting +from core.configuration.library import LibrarySettings from tests.fixtures.database import DatabaseTransactionFixture from tests.fixtures.files import FilesFixture +from tests.fixtures.library import LibraryFixture @pytest.fixture() @@ -49,88 +50,47 @@ def test_collection_language_method_performs_estimate( library = db.default_library() # We haven't set any of these values. - for key in [ - C.LARGE_COLLECTION_LANGUAGES, - C.SMALL_COLLECTION_LANGUAGES, - C.TINY_COLLECTION_LANGUAGES, - ]: - assert None == ConfigurationSetting.for_library(key, library).value + assert library.settings.large_collection_languages is None + assert library.settings.small_collection_languages is None + assert library.settings.tiny_collection_languages is None # So how does this happen? - assert ["eng"] == C.large_collection_languages(library) - assert [] == C.small_collection_languages(library) - assert [] == C.tiny_collection_languages(library) + assert C.large_collection_languages(library) == ["eng"] + assert C.small_collection_languages(library) == [] + assert C.tiny_collection_languages(library) == [] # It happens because the first time we call one of those # *_collection_languages, it estimates values for all three # configuration settings, based on the library's current # holdings. - large_setting = ConfigurationSetting.for_library( - C.LARGE_COLLECTION_LANGUAGES, library - ) - assert ["eng"] == large_setting.json_value - assert ( - [] - == ConfigurationSetting.for_library( - C.SMALL_COLLECTION_LANGUAGES, library - ).json_value - ) - assert ( - [] - == ConfigurationSetting.for_library( - C.TINY_COLLECTION_LANGUAGES, library - ).json_value - ) + assert library.settings.large_collection_languages == ["eng"] + assert library.settings.small_collection_languages == [] + assert library.settings.tiny_collection_languages == [] # We can change these values. - large_setting.value = json.dumps(["spa", "jpn"]) - assert ["spa", "jpn"] == C.large_collection_languages(library) - - # If we enter an invalid value, or a value that's not a list, - # the estimate is re-calculated the next time we look. - large_setting.value = "this isn't json" - assert ["eng"] == C.large_collection_languages(library) - - large_setting.value = '"this is json but it\'s not a list"' - assert ["eng"] == C.large_collection_languages(library) + library.update_settings( + LibrarySettings.construct(large_collection_languages=["spa", "jpn"]) + ) + assert C.large_collection_languages(library) == ["spa", "jpn"] def test_estimate_language_collection_for_library( - self, db: DatabaseTransactionFixture + self, db: DatabaseTransactionFixture, library_fixture: LibraryFixture ): - library = db.default_library() - # We thought we'd have big collections. - old_settings = { - Configuration.LARGE_COLLECTION_LANGUAGES: ["spa", "fre"], - Configuration.SMALL_COLLECTION_LANGUAGES: ["chi"], - Configuration.TINY_COLLECTION_LANGUAGES: ["rus"], - } - - for key, value in list(old_settings.items()): - ConfigurationSetting.for_library(key, library).value = json.dumps(value) + settings = library_fixture.mock_settings() + settings.large_collection_languages = ["spa", "fre"] + settings.small_collection_languages = ["chi"] + settings.tiny_collection_languages = ["rus"] + library = library_fixture.library(settings=settings) # But there's nothing in our database, so when we call # Configuration.estimate_language_collections_for_library... Configuration.estimate_language_collections_for_library(library) # ...it gets reset to the default. - assert ["eng"] == ConfigurationSetting.for_library( - Configuration.LARGE_COLLECTION_LANGUAGES, library - ).json_value - - assert ( - [] - == ConfigurationSetting.for_library( - Configuration.SMALL_COLLECTION_LANGUAGES, library - ).json_value - ) - - assert ( - [] - == ConfigurationSetting.for_library( - Configuration.TINY_COLLECTION_LANGUAGES, library - ).json_value - ) + assert library.settings.large_collection_languages == ["eng"] + assert library.settings.small_collection_languages == [] + assert library.settings.tiny_collection_languages == [] def test_classify_holdings(self, db: DatabaseTransactionFixture): m = Configuration.classify_holdings @@ -152,26 +112,27 @@ def test_classify_holdings(self, db: DatabaseTransactionFixture): ) assert [["fre", "jpn"], ["spa", "ukr", "ira"], ["nav"]] == m(different_sizes) - def test_max_outstanding_fines(self, db: DatabaseTransactionFixture): + def test_max_outstanding_fines( + self, db: DatabaseTransactionFixture, library_fixture: LibraryFixture + ): m = Configuration.max_outstanding_fines - # By default, fines are not enforced. - assert None == m(db.default_library()) + library = library_fixture.library() + settings = library_fixture.settings(library) - # The maximum fine value is determined by this - # ConfigurationSetting. - setting = ConfigurationSetting.for_library( - Configuration.MAX_OUTSTANDING_FINES, db.default_library() - ) + # By default, fines are not enforced. + assert m(library) is None # Any amount of fines is too much. - setting.value = "$0" - max_fines = m(db.default_library()) + settings.max_outstanding_fines = 0 + max_fines = m(library) + assert max_fines is not None assert 0 == max_fines.amount # A more lenient approach. - setting.value = "100" - max_fines = m(db.default_library()) + settings.max_outstanding_fines = 100.0 + max_fines = m(library) + assert max_fines is not None assert 100 == max_fines.amount def test_default_opds_format(self): diff --git a/tests/api/test_controller_base.py b/tests/api/test_controller_base.py index 0e3088a2a1..ce01f2e699 100644 --- a/tests/api/test_controller_base.py +++ b/tests/api/test_controller_base.py @@ -27,6 +27,7 @@ # TODO: we can drop this when we drop support for Python 3.6 and 3.7 from tests.fixtures.api_controller import CirculationControllerFixture +from tests.fixtures.library import LibraryFixture class TestBaseController: @@ -496,7 +497,9 @@ def test_apply_borrowing_policy_succeeds_for_self_hosted_books( assert problem is None def test_apply_borrowing_policy_when_holds_prohibited( - self, circulation_fixture: CirculationControllerFixture + self, + circulation_fixture: CirculationControllerFixture, + library_fixture: LibraryFixture, ): with circulation_fixture.request_context_with_library("/"): patron = circulation_fixture.controller.authenticated_patron( @@ -504,7 +507,7 @@ def test_apply_borrowing_policy_when_holds_prohibited( ) # This library does not allow holds. library = circulation_fixture.db.default_library() - library.setting(library.ALLOW_HOLDS).value = "False" + library_fixture.settings(library).allow_holds = False # This is an open-access work. work = circulation_fixture.db.work( diff --git a/tests/api/test_controller_loan.py b/tests/api/test_controller_loan.py index cd1c6db1fc..ee33abdcd8 100644 --- a/tests/api/test_controller_loan.py +++ b/tests/api/test_controller_loan.py @@ -20,7 +20,6 @@ NotFoundOnRemote, PatronHoldLimitReached, ) -from api.config import Configuration from api.problem_details import ( BAD_DELIVERY_MECHANISM, CANNOT_RELEASE_HOLD, @@ -31,7 +30,6 @@ ) from core.model import ( Collection, - ConfigurationSetting, DataSource, DeliveryMechanism, ExternalIntegration, @@ -56,6 +54,7 @@ from tests.core.mock import DummyHTTPClient from tests.fixtures.api_controller import CirculationControllerFixture from tests.fixtures.database import DatabaseTransactionFixture +from tests.fixtures.library import LibraryFixture class LoanFixture(CirculationControllerFixture): @@ -1045,7 +1044,9 @@ def test_hold_fails_when_patron_is_at_hold_limit(self, loan_fixture: LoanFixture assert isinstance(response, ProblemDetail) assert HOLD_LIMIT_REACHED.uri == response.uri - def test_borrow_fails_with_outstanding_fines(self, loan_fixture: LoanFixture): + def test_borrow_fails_with_outstanding_fines( + self, loan_fixture: LoanFixture, library_fixture: LibraryFixture + ): threem_edition, pool = loan_fixture.db.edition( with_open_access_download=False, data_source_name=DataSource.THREEM, @@ -1057,9 +1058,10 @@ def test_borrow_fails_with_outstanding_fines(self, loan_fixture: LoanFixture): ) pool.open_access = False - ConfigurationSetting.for_library( - Configuration.MAX_OUTSTANDING_FINES, loan_fixture.db.default_library() - ).value = "$0.50" + library = loan_fixture.db.default_library() + settings = library_fixture.settings(library) + + settings.max_outstanding_fines = 0.50 with loan_fixture.request_context_with_library( "/", headers=dict(Authorization=loan_fixture.valid_auth) ): diff --git a/tests/api/test_controller_opdsfeed.py b/tests/api/test_controller_opdsfeed.py index 964eae280d..4b0a1ab806 100644 --- a/tests/api/test_controller_opdsfeed.py +++ b/tests/api/test_controller_opdsfeed.py @@ -11,13 +11,14 @@ from api.opds import LibraryAnnotator from api.problem_details import REMOTE_INTEGRATION_FAILED from core.app_server import load_facets_from_request -from core.entrypoint import AudiobooksEntryPoint, EntryPoint, EverythingEntryPoint +from core.entrypoint import AudiobooksEntryPoint, EverythingEntryPoint from core.external_search import SortKeyPagination from core.lane import Facets, FeaturedFacets, Lane, Pagination, SearchFacets, WorkList -from core.model import CachedFeed, ConfigurationSetting, Edition +from core.model import CachedFeed, Edition from core.opds import AcquisitionFeed, NavigationFacets, NavigationFeed from core.util.flask_util import Response from tests.fixtures.api_controller import CirculationControllerFixture, WorkSpec +from tests.fixtures.library import LibraryFixture class TestOPDSFeedController: @@ -36,7 +37,11 @@ class TestOPDSFeedController: page_called_with: Any called_with: Any - def test_feed(self, circulation_fixture: CirculationControllerFixture): + def test_feed( + self, + circulation_fixture: CirculationControllerFixture, + library_fixture: LibraryFixture, + ): circulation_fixture.add_works(self._EXTRA_BOOKS) # Test the feed() method. @@ -80,13 +85,11 @@ def test_feed(self, circulation_fixture: CirculationControllerFixture): # Set up configuration settings for links and entry points library = circulation_fixture.db.default_library() - for rel, value in [ - (LibraryAnnotator.TERMS_OF_SERVICE, "a"), - (LibraryAnnotator.PRIVACY_POLICY, "b"), - (LibraryAnnotator.COPYRIGHT, "c"), - (LibraryAnnotator.ABOUT, "d"), - ]: - ConfigurationSetting.for_library(rel, library).value = value + settings = library_fixture.settings(library) + settings.terms_of_service = "a" # type: ignore[assignment] + settings.privacy_policy = "b" # type: ignore[assignment] + settings.copyright = "c" # type: ignore[assignment] + settings.about = "d" # type: ignore[assignment] # Make a real OPDS feed and poke at it. with circulation_fixture.request_context_with_library( @@ -127,10 +130,10 @@ def test_feed(self, circulation_fixture: CirculationControllerFixture): else: by_rel[i["rel"]] = i["href"] - assert "a" == by_rel[LibraryAnnotator.TERMS_OF_SERVICE] - assert "b" == by_rel[LibraryAnnotator.PRIVACY_POLICY] - assert "c" == by_rel[LibraryAnnotator.COPYRIGHT] - assert "d" == by_rel[LibraryAnnotator.ABOUT] + assert "a" == by_rel["terms-of-service"] + assert "b" == by_rel["privacy-policy"] + assert "c" == by_rel["copyright"] + assert "d" == by_rel["about"] next_link = by_rel["next"] lane_str = str(lane_id) @@ -231,7 +234,11 @@ def page(cls, **kwargs): # No other arguments were passed into page(). assert {} == kwargs - def test_groups(self, circulation_fixture: CirculationControllerFixture): + def test_groups( + self, + circulation_fixture: CirculationControllerFixture, + library_fixture: LibraryFixture, + ): circulation_fixture.add_works(self._EXTRA_BOOKS) # AcquisitionFeed.groups is tested in core/test_opds.py, and a @@ -239,8 +246,9 @@ def test_groups(self, circulation_fixture: CirculationControllerFixture): # index, so we're just going to test that groups() (or, in one # case, page()) is called properly. library = circulation_fixture.db.default_library() - library.setting(library.MINIMUM_FEATURED_QUALITY).value = 0.15 - library.setting(library.FEATURED_LANE_SIZE).value = 2 + settings = library_fixture.settings(library) + settings.minimum_featured_quality = 0.15 # type: ignore[assignment] + settings.featured_lane_size = 2 # Patron with root lane -> redirect to root lane lane = circulation_fixture.db.lane() @@ -453,7 +461,11 @@ def test_search_document(self, circulation_fixture: CirculationControllerFixture ) assert "OpenSearchDescription" in response.get_data(as_text=True) - def test_search(self, circulation_fixture: CirculationControllerFixture): + def test_search( + self, + circulation_fixture: CirculationControllerFixture, + library_fixture: LibraryFixture, + ): circulation_fixture.add_works(self._EXTRA_BOOKS) # Test the search() controller method. @@ -579,9 +591,9 @@ def search(cls, **kwargs): # When only a single entry point is enabled, it's used as the # default. - library.setting(EntryPoint.ENABLED_SETTING).value = json.dumps( - [AudiobooksEntryPoint.INTERNAL_NAME] - ) + library_fixture.settings(library).enabled_entry_points = [ + AudiobooksEntryPoint.INTERNAL_NAME + ] with circulation_fixture.request_context_with_library("/?q=t"): response = circulation_fixture.manager.opds_feeds.search( None, feed_class=Mock diff --git a/tests/api/test_controller_scopedsession.py b/tests/api/test_controller_scopedsession.py index 60ba5ca760..1f80649d48 100644 --- a/tests/api/test_controller_scopedsession.py +++ b/tests/api/test_controller_scopedsession.py @@ -36,7 +36,15 @@ def make_default_libraries(self, session: Session): for i in range(2): name = self.fresh_id() + " (library for scoped session)" library, ignore = create( - session, Library, short_name=name, public_key="x", private_key=b"y" + session, + Library, + short_name=name, + public_key="x", + private_key=b"y", + settings_dict={ + "website": "https://library.com", + "help_web": "https://library.com/help", + }, ) libraries.append(library) return libraries diff --git a/tests/api/test_lanes.py b/tests/api/test_lanes.py index e17aae0369..0004fc0668 100644 --- a/tests/api/test_lanes.py +++ b/tests/api/test_lanes.py @@ -1,10 +1,8 @@ -import json from collections import Counter from unittest.mock import MagicMock, patch import pytest -from api.config import Configuration from api.lanes import ( ContributorFacets, ContributorLane, @@ -42,6 +40,7 @@ create, ) from tests.fixtures.database import DatabaseTransactionFixture +from tests.fixtures.library import LibraryFixture class TestLaneCreation: @@ -260,21 +259,16 @@ def test_lane_for_tiny_collection(self, db: DatabaseTransactionFixture): lane = db.session.query(Lane).filter(Lane.parent == new_parent) assert lane.count() == 0 - def test_create_default_lanes(self, db: DatabaseTransactionFixture): - library = db.default_library() - library.setting(Configuration.LARGE_COLLECTION_LANGUAGES).value = json.dumps( - ["eng"] - ) - - library.setting(Configuration.SMALL_COLLECTION_LANGUAGES).value = json.dumps( - ["spa", "chi"] - ) - - library.setting(Configuration.TINY_COLLECTION_LANGUAGES).value = json.dumps( - ["ger", "fre", "ita"] - ) + def test_create_default_lanes( + self, db: DatabaseTransactionFixture, library_fixture: LibraryFixture + ): + settings = library_fixture.mock_settings() + settings.large_collection_languages = ["eng"] + settings.small_collection_languages = ["spa", "chi"] + settings.tiny_collection_languages = ["ger", "fre", "ita"] + library = library_fixture.library(settings=settings) - create_default_lanes(db.session, db.default_library()) + create_default_lanes(db.session, library) lanes = ( db.session.query(Lane) .filter(Lane.library == library) @@ -312,19 +306,12 @@ def test_create_default_lanes(self, db: DatabaseTransactionFixture): assert Classifier.AUDIENCE_CHILDREN == audiences[0] def test_create_default_when_more_than_one_large_language_is_configured( - self, db: DatabaseTransactionFixture + self, db: DatabaseTransactionFixture, library_fixture: LibraryFixture ): - library = db.default_library() - library.setting(Configuration.LARGE_COLLECTION_LANGUAGES).value = json.dumps( - [ - "eng", - "fre", - ] - ) - - library.setting(Configuration.SMALL_COLLECTION_LANGUAGES).value = json.dumps([]) + settings = library_fixture.mock_settings() + settings.large_collection_languages = ["eng", "fre"] + library = library_fixture.library(settings=settings) - library.setting(Configuration.TINY_COLLECTION_LANGUAGES).value = json.dumps([]) session = db.session create_default_lanes(session, library) lanes = ( @@ -348,9 +335,6 @@ def test_create_default_when_more_than_one_large_language_is_returned_by_estimat self, db: DatabaseTransactionFixture ): library = db.default_library() - library.setting(Configuration.LARGE_COLLECTION_LANGUAGES).value = json.dumps([]) - library.setting(Configuration.SMALL_COLLECTION_LANGUAGES).value = json.dumps([]) - library.setting(Configuration.TINY_COLLECTION_LANGUAGES).value = json.dumps([]) session = db.session with patch( "api.lanes._lane_configuration_from_collection_sizes" diff --git a/tests/api/test_opds.py b/tests/api/test_opds.py index be7b2995a3..b9b0e454a6 100644 --- a/tests/api/test_opds.py +++ b/tests/api/test_opds.py @@ -1,8 +1,7 @@ import datetime -import json import re from collections import defaultdict -from typing import Any, Dict, List +from typing import Any, List from unittest.mock import MagicMock, create_autospec import dateutil @@ -13,7 +12,6 @@ from api.adobe_vendor_id import AuthdataUtility from api.circulation import BaseCirculationAPI, CirculationAPI, FulfillmentInfo -from api.config import Configuration from api.lanes import ContributorLane from api.novelist import NoveListAPI from api.opds import ( @@ -59,6 +57,7 @@ from core.util.flask_util import OPDSEntryResponse, OPDSFeedResponse from core.util.opds_writer import AtomFeed, OPDSFeed from tests.fixtures.database import DatabaseTransactionFixture +from tests.fixtures.library import LibraryFixture from tests.fixtures.vendor_id import VendorIDFixture _strftime = AtomFeed._strftime @@ -388,81 +387,52 @@ def annotator_fixture(db: DatabaseTransactionFixture) -> LibraryAnnotatorFixture class TestLibraryAnnotator: - def test__hidden_content_types(self, annotator_fixture: LibraryAnnotatorFixture): - def f(value): - """Set the default library's HIDDEN_CONTENT_TYPES setting - to a specific value and see what _hidden_content_types - says. - """ - library = annotator_fixture.db.default_library() - library.setting(Configuration.HIDDEN_CONTENT_TYPES).value = value - return LibraryAnnotator._hidden_content_types(library) - - # When the value is not set at all, no content types are hidden. - assert [] == list( - LibraryAnnotator._hidden_content_types( - annotator_fixture.db.default_library() - ) - ) - - # Now set various values and see what happens. - assert [] == f(None) - assert [] == f("") - assert [] == f(json.dumps([])) - assert ["text/html"] == f("text/html") - assert ["text/html"] == f(json.dumps("text/html")) - assert ["text/html"] == f(json.dumps({"text/html": "some value"})) - assert ["text/html", "text/plain"] == f(json.dumps(["text/html", "text/plain"])) - - def test_add_configuration_links(self, annotator_fixture: LibraryAnnotatorFixture): + def test_add_configuration_links( + self, + annotator_fixture: LibraryAnnotatorFixture, + library_fixture: LibraryFixture, + ): mock_feed: List[Any] = [] - link_config = { - LibraryAnnotator.TERMS_OF_SERVICE: "http://terms/", - LibraryAnnotator.PRIVACY_POLICY: "http://privacy/", - LibraryAnnotator.COPYRIGHT: "http://copyright/", - LibraryAnnotator.ABOUT: "http://about/", - LibraryAnnotator.LICENSE: "http://license/", - Configuration.HELP_EMAIL: "help@me", - Configuration.HELP_WEB: "http://help/", - Configuration.HELP_URI: "uri:help", - } # Set up configuration settings for links. - for rel, value in link_config.items(): - ConfigurationSetting.for_library( - rel, annotator_fixture.db.default_library() - ).value = value + library = annotator_fixture.db.default_library() + settings = library_fixture.settings(library) + settings.terms_of_service = "http://terms/" # type: ignore[assignment] + settings.privacy_policy = "http://privacy/" # type: ignore[assignment] + settings.copyright = "http://copyright/" # type: ignore[assignment] + settings.about = "http://about/" # type: ignore[assignment] + settings.license = "http://license/" # type: ignore[assignment] + settings.help_email = "help@me" # type: ignore[assignment] + settings.help_web = "http://help/" # type: ignore[assignment] # Set up settings for navigation links. - ConfigurationSetting.for_library( - Configuration.WEB_HEADER_LINKS, annotator_fixture.db.default_library() - ).value = json.dumps(["http://example.com/1", "http://example.com/2"]) - ConfigurationSetting.for_library( - Configuration.WEB_HEADER_LABELS, annotator_fixture.db.default_library() - ).value = json.dumps(["one", "two"]) + settings.web_header_links = ["http://example.com/1", "http://example.com/2"] + settings.web_header_labels = ["one", "two"] annotator_fixture.annotator.add_configuration_links(mock_feed) - # Ten links were added to the "feed" - assert 10 == len(mock_feed) + assert 9 == len(mock_feed) + + mock_feed_links = sorted(mock_feed, key=lambda x: x.attrib["rel"]) + expected_links = [ + (link.attrib["href"], link.attrib.get("type")) + for link in mock_feed_links + if link.attrib["rel"] != "related" + ] # They are the links we'd expect. - links: Dict[str, Any] = {} - for link in mock_feed: - rel = link.attrib["rel"] - href = link.attrib["href"] - if rel == "help" or rel == "related": - continue # Tested below - # Check that the configuration value made it into the link. - assert href == link_config[rel] - assert "text/html" == link.attrib["type"] - - # There are three help links using different protocols. - help_links = [x.attrib["href"] for x in mock_feed if x.attrib["rel"] == "help"] - assert {"mailto:help@me", "http://help/", "uri:help"} == set(help_links) + assert [ + ("http://about/", "text/html"), + ("http://copyright/", "text/html"), + ("mailto:help@me", None), + ("http://help/", "text/html"), + ("http://license/", "text/html"), + ("http://privacy/", "text/html"), + ("http://terms/", "text/html"), + ] == expected_links # There are two navigation links. - navigation_links = [x for x in mock_feed if x.attrib["rel"] == "related"] + navigation_links = [x for x in mock_feed_links if x.attrib["rel"] == "related"] assert {"navigation"} == {x.attrib["role"] for x in navigation_links} assert {"http://example.com/1", "http://example.com/2"} == { x.attrib["href"] for x in navigation_links @@ -665,9 +635,7 @@ def test_adobe_id_tags_when_vendor_id_configured( authdata = AuthdataUtility.from_config(library) assert authdata is not None decoded = authdata.decode_short_client_token(token) - expected_url = ConfigurationSetting.for_library( - Configuration.WEBSITE_URL, library - ).value + expected_url = library.settings.website assert (expected_url, patron_identifier) == decoded # If we call adobe_id_tags again we'll get a distinct tag @@ -1442,7 +1410,9 @@ def test_acquisition_feed_includes_license_information( assert copies_re.search(u) is not None def test_loans_feed_includes_fulfill_links( - self, annotator_fixture: LibraryAnnotatorFixture + self, + annotator_fixture: LibraryAnnotatorFixture, + library_fixture: LibraryFixture, ): patron = annotator_fixture.db.patron() @@ -1492,18 +1462,15 @@ def test_loans_feed_includes_fulfill_links( # If one of the content types is hidden, the corresponding # delivery mechanism does not have a link. - setting = annotator_fixture.db.default_library().setting( - Configuration.HIDDEN_CONTENT_TYPES - ) - setting.value = json.dumps([mech1.delivery_mechanism.content_type]) - feed_obj = LibraryLoanAndHoldAnnotator.active_loans_for( - None, patron, test_mode=True - ) + library = annotator_fixture.db.default_library() + settings = library_fixture.settings(library) + settings.hidden_content_types = [mech1.delivery_mechanism.content_type] + LibraryLoanAndHoldAnnotator.active_loans_for(None, patron, test_mode=True) assert { mech2.delivery_mechanism.drm_scheme_media_type, OPDSFeed.ENTRY_TYPE, } == {link["type"] for link in fulfill_links} - setting.value = None + settings.hidden_content_types = [] # When the loan is fulfilled, there are only fulfill links for that mechanism # and the streaming mechanism. @@ -1828,7 +1795,11 @@ def annotated_links(lane, annotator): assert "/crawlable_list_feed" in crawlable assert str(list1.name) in crawlable - def test_acquisition_links(self, annotator_fixture: LibraryAnnotatorFixture): + def test_acquisition_links( + self, + annotator_fixture: LibraryAnnotatorFixture, + library_fixture: LibraryFixture, + ): annotator = LibraryLoanAndHoldAnnotator( None, None, annotator_fixture.db.default_library(), test_mode=True ) @@ -1923,14 +1894,13 @@ def test_acquisition_links(self, annotator_fixture: LibraryAnnotatorFixture): # If a book is ready to be fulfilled, but the library has # hidden all of its available content types, the fulfill link does # not show up -- only the revoke link. - hidden = annotator_fixture.db.default_library().setting( - Configuration.HIDDEN_CONTENT_TYPES - ) + library = annotator_fixture.db.default_library() + settings = library_fixture.settings(library) available_types = [ lpdm.delivery_mechanism.content_type for lpdm in loan2.license_pool.delivery_mechanisms ] - hidden.value = json.dumps(available_types) + settings.hidden_content_types = available_types # The list of hidden content types is stored in the Annotator # constructor, so this particular test needs a fresh Annotator. @@ -1945,7 +1915,7 @@ def test_acquisition_links(self, annotator_fixture: LibraryAnnotatorFixture): "rel" ) # Un-hide the content types so the test can continue. - hidden.value = None + settings.hidden_content_types = [] hold_links = annotator.acquisition_links( hold.license_pool, None, hold, None, feed, hold.license_pool.identifier @@ -2041,7 +2011,9 @@ def test_acquisition_links(self, annotator_fixture: LibraryAnnotatorFixture): assert [] == hold_links def test_acquisition_links_multiple_links( - self, annotator_fixture: LibraryAnnotatorFixture + self, + annotator_fixture: LibraryAnnotatorFixture, + library_fixture: LibraryFixture, ): annotator = LibraryLoanAndHoldAnnotator( None, None, annotator_fixture.db.default_library(), test_mode=True @@ -2113,9 +2085,9 @@ class MockAPI: # If we configure the library to hide one of the content types, # we end up with only one link -- the one for the delivery # mechanism that's not hidden. - annotator_fixture.db.default_library().setting( - Configuration.HIDDEN_CONTENT_TYPES - ).value = json.dumps([mech1.delivery_mechanism.content_type]) + library = annotator_fixture.db.default_library() + settings = library_fixture.settings(library) + settings.hidden_content_types = [mech1.delivery_mechanism.content_type] annotator = LibraryLoanAndHoldAnnotator( None, None, annotator_fixture.db.default_library(), test_mode=True ) diff --git a/tests/api/test_overdrive.py b/tests/api/test_overdrive.py index f5712cb708..21c0cf210b 100644 --- a/tests/api/test_overdrive.py +++ b/tests/api/test_overdrive.py @@ -27,7 +27,6 @@ from core.config import CannotLoadConfiguration from core.metadata_layer import TimestampData from core.model import ( - ConfigurationSetting, DataSource, DeliveryMechanism, Edition, @@ -44,6 +43,7 @@ from tests.core.mock import DummyHTTPClient, MockRequestsResponse from ..fixtures.database import DatabaseTransactionFixture +from ..fixtures.library import LibraryFixture if TYPE_CHECKING: from ..fixtures.api_overdrive_files import OverdriveAPIFilesFixture @@ -243,7 +243,9 @@ def explode(*args, **kwargs): assert "Failure!" == str(check_creds.exception) def test_default_notification_email_address( - self, overdrive_api_fixture: OverdriveAPIFixture + self, + overdrive_api_fixture: OverdriveAPIFixture, + library_fixture: LibraryFixture, ): """Test the ability of the Overdrive API to detect an email address previously given by the patron to Overdrive for the purpose of @@ -255,13 +257,12 @@ def test_default_notification_email_address( "patron_info.json" ) overdrive_api_fixture.api.queue_response(200, content=patron_with_email) - patron = db.patron() + settings = library_fixture.mock_settings() + library = library_fixture.library(settings=settings) + patron = db.patron(library=library) # The site default for notification emails will never be used. - configuration_setting = ConfigurationSetting.for_library( - Configuration.DEFAULT_NOTIFICATION_EMAIL_ADDRESS, db.default_library() - ) - configuration_setting.value = "notifications@example.com" + settings.default_notification_email_address = "notifications@example.com" # type: ignore[assignment] # If the patron has used a particular email address to put # books on hold, use that email address, not the site default. @@ -276,7 +277,7 @@ def test_default_notification_email_address( # the site default, it is ignored. This can only happen if # this patron placed a hold using an older version of the # circulation manager. - patron_with_email["lastHoldEmail"] = configuration_setting.value + patron_with_email["lastHoldEmail"] = settings.default_notification_email_address overdrive_api_fixture.api.queue_response(200, content=patron_with_email) assert None == overdrive_api_fixture.api.default_notification_email_address( patron, "pin" diff --git a/tests/api/test_patron_utility.py b/tests/api/test_patron_utility.py index 0a308c6fef..d2241aaf9b 100644 --- a/tests/api/test_patron_utility.py +++ b/tests/api/test_patron_utility.py @@ -6,12 +6,11 @@ from api.authentication.base import PatronData from api.circulation_exceptions import * -from api.config import Configuration from api.util.patron import Patron, PatronUtility -from core.model import ConfigurationSetting from core.util import MoneyUtility from core.util.datetime_helpers import utc_now from tests.fixtures.database import DatabaseTransactionFixture +from tests.fixtures.library import LibraryFixture class TestPatronUtility: @@ -119,15 +118,15 @@ def has_excess_fines(cls, patron): patron.block_reason = None assert True == PatronUtility.has_borrowing_privileges(patron) - def test_has_excess_fines(self, db: DatabaseTransactionFixture): + def test_has_excess_fines( + self, db: DatabaseTransactionFixture, library_fixture: LibraryFixture + ): # Test the has_excess_fines method. - patron = db.patron() + library = library_fixture.library() + patron = db.patron(library=library) + settings = library_fixture.settings(library) # If you accrue excessive fines you lose borrowing privileges. - setting = ConfigurationSetting.for_library( - Configuration.MAX_OUTSTANDING_FINES, db.default_library() - ) - # Verify that all these tests work no matter what data type has been stored in # patron.fines. for patron_fines in ("1", "0.75", 1, 1.0, Decimal(1), MoneyUtility.parse("1")): @@ -135,29 +134,20 @@ def test_has_excess_fines(self, db: DatabaseTransactionFixture): # Test cases where the patron's fines exceed a well-defined limit, # or when any amount of fines is too much. - for max_fines in ["$0.50", "0.5", 0.5] + [ # well-defined limit - "$0", - "$0.00", - "0", - 0, - ]: # any fines is too much - setting.value = max_fines - assert True == PatronUtility.has_excess_fines(patron) + for max_fines_exceed in [0.5, 0]: + settings.max_outstanding_fines = max_fines_exceed + assert PatronUtility.has_excess_fines(patron) is True # Test cases where the patron's fines are below a # well-defined limit, or where fines are ignored # altogether. - for max_fines in ["$100", 100] + [ # well-defined-limit - None, - "", - ]: # fines ignored - setting.value = max_fines - assert False == PatronUtility.has_excess_fines(patron) + for max_fines_below in [100, None]: # fines ignored + settings.max_outstanding_fines = max_fines_below + assert PatronUtility.has_excess_fines(patron) is False # Test various cases where fines in any amount deny borrowing # privileges, but the patron has no fines. for patron_fines in ("0", "$0", 0, None, MoneyUtility.parse("$0")): patron.fines = patron_fines - for max_fines in ["$0", "$0.00", "0", 0]: - setting.value = max_fines - assert False == PatronUtility.has_excess_fines(patron) + settings.max_outstanding_fines = 0 + assert PatronUtility.has_excess_fines(patron) is False diff --git a/tests/api/test_registry.py b/tests/api/test_registry.py index a3a0df52a5..4024adf645 100644 --- a/tests/api/test_registry.py +++ b/tests/api/test_registry.py @@ -21,6 +21,7 @@ from tests.api.mockapi.circulation import MockCirculationManager from tests.core.mock import DummyHTTPClient, MockRequestsResponse from tests.fixtures.database import DatabaseTransactionFixture +from tests.fixtures.library import LibraryFixture class RemoteRegistryFixture: @@ -635,7 +636,7 @@ def fail3(*args, **kwargs): assert "could not fetch catalog" == problem.detail def test__create_registration_payload( - self, registration_fixture: RegistrationFixture + self, registration_fixture: RegistrationFixture, library_fixture: LibraryFixture ): m = registration_fixture.registration._create_registration_payload @@ -655,10 +656,8 @@ def url_for(controller, library_short_name, **kwargs): # If a contact is configured, it shows up in the payload. contact = "mailto:ohno@library.org" - ConfigurationSetting.for_library( - Configuration.CONFIGURATION_CONTACT_EMAIL, - registration_fixture.registration.library, - ).value = contact + settings = library_fixture.settings(registration_fixture.registration.library) + settings.configuration_contact_email_address = contact # type: ignore[assignment] expect_payload["contact"] = contact assert expect_payload == m(url_for, stage) diff --git a/tests/api/test_scripts.py b/tests/api/test_scripts.py index d7ccdfc084..7b78f973df 100644 --- a/tests/api/test_scripts.py +++ b/tests/api/test_scripts.py @@ -2,7 +2,6 @@ import contextlib import datetime -import json from io import StringIO from pathlib import Path from typing import TYPE_CHECKING, Callable @@ -14,7 +13,7 @@ from api.config import Configuration from api.marc import LibraryAnnotator as MARCLibraryAnnotator from api.novelist import NoveListAPI -from core.entrypoint import AudiobooksEntryPoint, EbooksEntryPoint, EntryPoint +from core.entrypoint import AudiobooksEntryPoint, EbooksEntryPoint from core.external_search import ( ExternalSearchIndex, MockExternalSearchIndex, @@ -61,6 +60,7 @@ NovelistSnapshotScript, ) from tests.api.mockapi.circulation import MockCirculationManager +from tests.fixtures.library import LibraryFixture if TYPE_CHECKING: from tests.fixtures.authenticator import AuthProviderFixture @@ -114,25 +114,25 @@ def set_value(credential): class LaneScriptFixture: - def __init__(self, db: DatabaseTransactionFixture): + def __init__(self, db: DatabaseTransactionFixture, library_fixture: LibraryFixture): self.db = db base_url_setting = ConfigurationSetting.sitewide( self.db.session, Configuration.BASE_URL_KEY ) base_url_setting.value = "http://test-circulation-manager/" - for k, v in [ - (Configuration.LARGE_COLLECTION_LANGUAGES, []), - (Configuration.SMALL_COLLECTION_LANGUAGES, []), - (Configuration.TINY_COLLECTION_LANGUAGES, ["eng", "fre"]), - ]: - ConfigurationSetting.for_library( - k, self.db.default_library() - ).value = json.dumps(v) + library = db.default_library() + settings = library_fixture.mock_settings() + settings.large_collection_languages = [] + settings.small_collection_languages = [] + settings.tiny_collection_languages = ["eng", "fre"] + library.update_settings(settings) @pytest.fixture(scope="function") -def lane_script_fixture(db: DatabaseTransactionFixture) -> LaneScriptFixture: - return LaneScriptFixture(db) +def lane_script_fixture( + db: DatabaseTransactionFixture, library_fixture: LibraryFixture +) -> LaneScriptFixture: + return LaneScriptFixture(db, library_fixture) class TestCacheRepresentationPerLane: @@ -271,7 +271,9 @@ def test_arguments(self, lane_script_fixture: LaneScriptFixture): script = CacheFacetListsPerLane(db.session, ["--pages=1"], manager=object()) assert 1 == script.pages - def test_facets(self, lane_script_fixture: LaneScriptFixture): + def test_facets( + self, lane_script_fixture: LaneScriptFixture, library_fixture: LibraryFixture + ): db = lane_script_fixture.db # Verify that CacheFacetListsPerLane.facets combines the items # found in the attributes created by command-line parsing. @@ -285,12 +287,15 @@ def test_facets(self, lane_script_fixture: LaneScriptFixture): script.availabilities = [Facets.AVAILABLE_NOW, "nonsense"] script.collections = [Facets.COLLECTION_FULL, "nonsense"] + library = library_fixture.library() + # EbooksEntryPoint is normally a valid entry point, but we're # going to disable it for this library. - setting = db.default_library().setting(EntryPoint.ENABLED_SETTING) - setting.value = json.dumps([AudiobooksEntryPoint.INTERNAL_NAME]) + settings = library_fixture.mock_settings() + settings.enabled_entry_points = [AudiobooksEntryPoint.INTERNAL_NAME] + library.update_settings(settings) - lane = db.lane() + lane = db.lane(library=library) # We get one Facets object for every valid combination # of parameters. Here there are 2*1*1*1 combinations. @@ -313,7 +318,7 @@ def test_facets(self, lane_script_fixture: LaneScriptFixture): # that have no parent. When the WorkList has a parent, the selected # entry point is treated as an explicit choice -- navigating downward # in the lane hierarchy ratifies the default value. - sublane = db.lane(parent=lane) + sublane = db.lane(parent=lane, library=library) f1, f2 = script.facets(sublane) for f in f1, f2: assert False == f.entrypoint_is_default @@ -456,16 +461,20 @@ def groups(cls, **kwargs): assert AcquisitionFeed.ACQUISITION_FEED_TYPE == response.content_type assert response.get_data(as_text=True).startswith(" str: return str(self.fresh_id()) def library( - self, name: Optional[str] = None, short_name: Optional[str] = None + self, + name: Optional[str] = None, + short_name: Optional[str] = None, + settings: Optional[LibrarySettings] = None, ) -> Library: # Just a dummy key used for testing. key_string = """\ @@ -274,6 +278,14 @@ def library( name = name or self.fresh_str() short_name = short_name or self.fresh_str() + settings_dict = settings.dict() if settings else {} + + # Make sure we have defaults for settings that are required + if "website" not in settings_dict: + settings_dict["website"] = "http://library.com" + if "help_web" not in settings_dict and "help_email" not in settings_dict: + settings_dict["help_web"] = "http://library.com/support" + library, ignore = get_one_or_create( self.session, Library, @@ -283,6 +295,7 @@ def library( uuid=str(uuid.uuid4()), public_key=public_key.export_key("PEM").decode("utf-8"), private_key=private_key.export_key("DER"), + settings_dict=settings_dict, ), ) return library @@ -1012,11 +1025,6 @@ def db( tr.close() -@pytest.fixture -def default_library(db: DatabaseTransactionFixture) -> Library: - return db.default_library() - - @pytest.fixture def create_integration_configuration( db: DatabaseTransactionFixture, diff --git a/tests/fixtures/library.py b/tests/fixtures/library.py new file mode 100644 index 0000000000..c17f599669 --- /dev/null +++ b/tests/fixtures/library.py @@ -0,0 +1,70 @@ +from __future__ import annotations + +from typing import TYPE_CHECKING, Optional + +import pytest + +from core.configuration.library import LibrarySettings +from core.model import Library + +if TYPE_CHECKING: + from tests.fixtures.database import DatabaseTransactionFixture + + +@pytest.fixture +def default_library(db: DatabaseTransactionFixture) -> Library: + return db.default_library() + + +class MockLibrarySettings(LibrarySettings): + """ + A mock LibrarySettings object that can be used in tests, the + only change is that it allows mutation of the settings. + """ + + class Config(LibrarySettings.Config): + allow_mutation = True + + +class LibraryFixture: + """ + A mock Library object that can be used in tests, it returns + a MockLibrarySettings object for its settings. This allows us + to write tests that change the settings of a library. + """ + + def __init__(self, db: DatabaseTransactionFixture) -> None: + self.db = db + + def library( + self, + name: Optional[str] = None, + short_name: Optional[str] = None, + settings: Optional[LibrarySettings] = None, + ) -> Library: + library = self.db.library(name=name, short_name=short_name, settings=settings) + if isinstance(settings, MockLibrarySettings): + self.set_mock_on_library(library, settings) + return library + + def settings(self, library: Library) -> MockLibrarySettings: + settings_dict = library.settings_dict + settings = MockLibrarySettings(**settings_dict) + self.set_mock_on_library(library, settings) + return settings + + def mock_settings(self) -> MockLibrarySettings: + return MockLibrarySettings.construct() + + def set_mock_on_library( + self, library: Library, settings: MockLibrarySettings + ) -> None: + library._settings = settings + + def reset_settings_cache(self, library: Library) -> None: + library._settings = None + + +@pytest.fixture +def library_fixture(db: DatabaseTransactionFixture) -> LibraryFixture: + return LibraryFixture(db) diff --git a/tests/fixtures/vendor_id.py b/tests/fixtures/vendor_id.py index d5de8150b7..8dbb6c84e9 100644 --- a/tests/fixtures/vendor_id.py +++ b/tests/fixtures/vendor_id.py @@ -3,7 +3,6 @@ import pytest from api.adobe_vendor_id import AuthdataUtility -from api.config import Configuration from api.registration.constants import RegistrationConstants from core.model import ConfigurationSetting, ExternalIntegration, Library from tests.fixtures.database import DatabaseTransactionFixture @@ -37,7 +36,6 @@ def initialize_adobe( # The library given to this fixture will be setup to be able to generate # Short Client Tokens. - library_uri = self.db.fresh_url() assert vendor_id_library.short_name is not None short_name = vendor_id_library.short_name + "token" secret = vendor_id_library.short_name + " token secret" @@ -60,8 +58,6 @@ def initialize_adobe( self.registry, ).value = RegistrationConstants.SUCCESS_STATUS - vendor_id_library.setting(Configuration.WEBSITE_URL).value = library_uri - def __init__(self, db: DatabaseTransactionFixture): assert isinstance(db, DatabaseTransactionFixture) self.db = db diff --git a/tests/migration/conftest.py b/tests/migration/conftest.py index 12683092b3..99bef013a2 100644 --- a/tests/migration/conftest.py +++ b/tests/migration/conftest.py @@ -11,6 +11,7 @@ from pytest_alembic.config import Config from sqlalchemy import inspect +from core.model import json_serializer from tests.fixtures.database import ApplicationFixture, DatabaseFixture if TYPE_CHECKING: @@ -111,23 +112,27 @@ def fixture( if short_name is None: short_name = random_name() - # See if we need to include public and private keys inspector = inspect(connection) - columns = inspector.get_columns("libraries") - if "public_key" in [column["name"] for column in columns]: - include_keys = True - else: - include_keys = False + columns = [column["name"] for column in inspector.get_columns("libraries")] args = { "name": name, "short_name": short_name, } - if include_keys: + # See if we need to include public and private keys + if "public_key" in columns: args["public_key"] = random_name() args["private_key"] = random_name() + # See if we need to include a settings dict + if "settings_dict" in columns: + settings_dict = { + "website": "http://library.com", + "help_web": "http://library.com/support", + } + args["settings_dict"] = json_serializer(settings_dict) + keys = ",".join(args.keys()) values = ",".join([f"'{value}'" for value in args.values()]) library = connection.execute( diff --git a/tests/migration/test_20230719_b3749bac3e55.py b/tests/migration/test_20230719_b3749bac3e55.py new file mode 100644 index 0000000000..f25f860070 --- /dev/null +++ b/tests/migration/test_20230719_b3749bac3e55.py @@ -0,0 +1,64 @@ +import json + +from pytest_alembic import MigrationContext +from sqlalchemy import inspect +from sqlalchemy.engine import Engine + +from tests.migration.conftest import CreateConfigSetting, CreateLibrary + + +def column_exists(engine: Engine, table_name: str, column_name: str) -> bool: + inspector = inspect(engine) + columns = [column["name"] for column in inspector.get_columns(table_name)] + return column_name in columns + + +def test_migration( + alembic_runner: MigrationContext, + alembic_engine: Engine, + create_config_setting: CreateConfigSetting, + create_library: CreateLibrary, +) -> None: + alembic_runner.migrate_down_to("b3749bac3e55") + + # Make sure settings column exists + assert column_exists(alembic_engine, "libraries", "settings_dict") + + # Test down migration, make sure settings column is dropped + alembic_runner.migrate_down_one() + assert not column_exists(alembic_engine, "libraries", "settings_dict") + + # Create a library with some configuration settings + with alembic_engine.connect() as connection: + library = create_library(connection) + create_config_setting( + connection, "website", "https://foo.bar", library_id=library + ) + create_config_setting( + connection, "help_web", "https://foo.bar/helpme", library_id=library + ) + create_config_setting( + connection, "logo", "https://foo.bar/logo.png", library_id=library + ) + create_config_setting(connection, "key-pair", "foo", library_id=library) + create_config_setting(connection, "foo", "foo", library_id=library) + create_config_setting( + connection, + "enabled_entry_points", + json.dumps(["xyz", "abc"]), + library_id=library, + ) + + # Run the up migration, and make sure settings column is added + alembic_runner.migrate_up_one() + assert column_exists(alembic_engine, "libraries", "settings_dict") + + # Make sure settings are migrated into table correctly + with alembic_engine.connect() as connection: + result = connection.execute("select settings_dict from libraries").fetchone() + assert result is not None + settings_dict = result.settings_dict + assert len(settings_dict) == 3 + assert settings_dict["website"] == "https://foo.bar" + assert settings_dict["help_web"] == "https://foo.bar/helpme" + assert settings_dict["enabled_entry_points"] == ["xyz", "abc"]