Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rename IntegrationConfiguration settings property to settings_dict #1303

Merged
merged 2 commits into from
Aug 9, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions api/admin/controller/collection_settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ def process_get(self):
collection_dict["libraries"] = libraries
collection_dict[
"settings"
] = collection_object.integration_configuration.settings
] = collection_object.integration_configuration.settings_dict
self.load_settings(
protocol["settings"], collection_object, collection_dict["settings"]
)
Expand Down Expand Up @@ -145,7 +145,7 @@ def load_libraries(self, collection_object: Collection, user: Admin) -> List[Dic
# Find and update the library settings if they exist
for config in integration.library_configurations:
if library.id == config.library_id:
library_info.update(config.settings)
library_info.update(config.settings_dict)
break
libraries.append(library_info)

Expand Down Expand Up @@ -343,7 +343,7 @@ def process_settings(self, settings, collection):
settings_class(**collection_settings)
except ProblemError as ex:
return ex.problem_detail
collection.integration_configuration.settings = collection_settings
collection.integration_configuration.settings_dict = collection_settings

def _set_external_integration_link(
self,
Expand Down
10 changes: 5 additions & 5 deletions api/admin/controller/patron_auth_service_self_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ def get_info(patron_auth_service: IntegrationConfiguration):
name=patron_auth_service.name,
protocol=patron_auth_service.protocol,
goal=patron_auth_service.goal,
settings=patron_auth_service.settings,
settings=patron_auth_service.settings_dict,
)
return info

Expand All @@ -122,8 +122,8 @@ def run_tests(self, integration: IntegrationConfiguration) -> Dict[str, Any]:
)
)

if not isinstance(integration.settings, dict) or not isinstance(
library_configuration.settings, dict
if not isinstance(integration.settings_dict, dict) or not isinstance(
library_configuration.settings_dict, dict
):
raise ProblemError(
problem_detail=FAILED_TO_RUN_SELF_TESTS.detailed(
Expand All @@ -132,9 +132,9 @@ def run_tests(self, integration: IntegrationConfiguration) -> Dict[str, Any]:
)

protocol_class = self.get_protocol_class(integration)
settings = protocol_class.settings_class()(**integration.settings)
settings = protocol_class.settings_class()(**integration.settings_dict)
library_settings = protocol_class.library_settings_class()(
**library_configuration.settings
**library_configuration.settings_dict
)

value, _ = protocol_class.run_self_tests(
Expand Down
8 changes: 4 additions & 4 deletions api/admin/controller/patron_auth_services.py
Original file line number Diff line number Diff line change
Expand Up @@ -91,14 +91,14 @@ def configured_services(self) -> List[Dict[str, Any]]:
libraries = []
for library_settings in service.library_configurations:
library_info = {"short_name": library_settings.library.short_name}
library_info.update(library_settings.settings)
library_info.update(library_settings.settings_dict)
libraries.append(library_info)

service_info = {
"id": service.id,
"name": service.name,
"protocol": service.protocol,
"settings": service.settings,
"settings": service.settings_dict,
"libraries": libraries,
}
configured_services.append(service_info)
Expand Down Expand Up @@ -240,7 +240,7 @@ def process_libraries(
# Update new and existing libraries settings
for integration, settings in chain(new, updated):
validated_settings = settings_class(**settings)
integration.settings = validated_settings.dict()
integration.settings_dict = validated_settings.dict()
# Make sure library doesn't have multiple auth basic auth services
self.check_library_integrations(integration.library)

Expand Down Expand Up @@ -276,7 +276,7 @@ def process_post(self) -> Union[Response, ProblemDetail]:
impl_cls = self.registry[protocol]
settings_class = impl_cls.settings_class()
validated_settings = ProcessFormData.get_settings(settings_class, form_data)
auth_service.settings = validated_settings.dict()
auth_service.settings_dict = validated_settings.dict()

# Update library settings
if libraries_data:
Expand Down
2 changes: 1 addition & 1 deletion api/admin/controller/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -372,7 +372,7 @@ def _set_configuration_library(
protocol_class.library_settings_class()(**info_copy)
# Attach the configuration
config = configuration.for_library(cast(int, library.id), create=True)
config.settings = info_copy
config.settings_dict = info_copy
return config

def _set_integration_library(self, integration, library_info, protocol):
Expand Down
10 changes: 6 additions & 4 deletions api/authenticator.py
Original file line number Diff line number Diff line change
Expand Up @@ -352,18 +352,20 @@ def register_provider(
f"Implementation class {impl_cls} is not an AuthenticationProvider."
)
try:
if not isinstance(integration.parent.settings, dict):
if not isinstance(integration.parent.settings_dict, dict):
raise CannotLoadConfiguration(
f"Settings for {impl_cls.__name__} authentication provider for "
f"library {self.library_short_name} are not a dictionary."
)
if not isinstance(integration.settings, dict):
if not isinstance(integration.settings_dict, dict):
raise CannotLoadConfiguration(
f"Library settings for {impl_cls.__name__} authentication provider for "
f"library {self.library_short_name} are not a dictionary."
)
settings = impl_cls.settings_class()(**integration.parent.settings)
library_settings = impl_cls.library_settings_class()(**integration.settings)
settings = impl_cls.settings_class()(**integration.parent.settings_dict)
library_settings = impl_cls.library_settings_class()(
**integration.settings_dict
)
provider = impl_cls(
self.library_id, # type: ignore[arg-type]
integration.parent_id, # type: ignore[arg-type]
Expand Down
4 changes: 2 additions & 2 deletions api/circulation.py
Original file line number Diff line number Diff line change
Expand Up @@ -492,12 +492,12 @@ def integration_configuration(self) -> IntegrationConfiguration:
def library_configuration(self, library_id) -> LibrarySettingsType | None:
libconfig = self.integration_configuration().for_library(library_id=library_id)
if libconfig:
config = self.library_settings_class()(**libconfig.settings)
config = self.library_settings_class()(**libconfig.settings_dict)
return config # type: ignore [return-value]
return None

def configuration(self) -> SettingsType:
return self.settings_class()(**self.integration_configuration().settings) # type: ignore [return-value]
return self.settings_class()(**self.integration_configuration().settings_dict) # type: ignore [return-value]


class BaseCirculationAPIProtocol(BaseCirculationAPIIntegrationProtocol):
Expand Down
10 changes: 7 additions & 3 deletions api/lcp/encrypt.py
Original file line number Diff line number Diff line change
Expand Up @@ -299,13 +299,15 @@ def __init__(

:param configuration: IntegrationConfiguration instance
"""
self._lcpencrypt_location = configuration.settings.get(
self._lcpencrypt_location = configuration.settings_dict.get(
"lcpencrypt_location"
)
self._input_file_path = str(file_path)
self._content_id = str(identifier)

output_directory = configuration.settings.get("lcpencrypt_output_directory")
output_directory = configuration.settings_dict.get(
"lcpencrypt_output_directory"
)

self._output_file_path = None

Expand Down Expand Up @@ -388,7 +390,9 @@ def __init__(self, configuration: IntegrationConfiguration):

def _lcpencrypt_exists_locally(self):
"""Returns a Boolean value indicating whether lcpencrypt exists locally"""
return os.path.isfile(self.configuration.settings.get("lcpencrypt_location"))
return os.path.isfile(
self.configuration.settings_dict.get("lcpencrypt_location")
)

def _parse_output(self, output):
"""Parses lcpencrypt's output
Expand Down
2 changes: 1 addition & 1 deletion api/lcp/mirror.py
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ def _create_lcp_importer(self, collection):
credential_factory = LCPCredentialFactory()
lcp_encryptor = LCPEncryptor(configuration)
lcp_server = LCPServer(
lambda: LCPServerSettings(**configuration.settings),
lambda: LCPServerSettings(**configuration.settings_dict),
hasher_factory,
credential_factory,
)
Expand Down
7 changes: 4 additions & 3 deletions api/opds.py
Original file line number Diff line number Diff line change
Expand Up @@ -193,11 +193,12 @@ def _prioritized_formats_for_pool(
# information _might_ contain a set of prioritized DRM schemes and
# content types.
prioritized_drm_schemes: list[str] = (
config.settings.get(FormatPriorities.PRIORITIZED_DRM_SCHEMES_KEY) or []
config.settings_dict.get(FormatPriorities.PRIORITIZED_DRM_SCHEMES_KEY) or []
)

content_setting: List[str] = (
config.settings.get(FormatPriorities.PRIORITIZED_CONTENT_TYPES_KEY) or []
config.settings_dict.get(FormatPriorities.PRIORITIZED_CONTENT_TYPES_KEY)
or []
)
return prioritized_drm_schemes, content_setting

Expand All @@ -214,7 +215,7 @@ def _deprioritized_lcp_content(
# LCP content. By default, if no configuration value is specified, then
# the priority of LCP content will be left completely unchanged.

_prioritize: bool = config.settings.get(
_prioritize: bool = config.settings_dict.get(
FormatPriorities.DEPRIORITIZE_LCP_NON_EPUBS_KEY, False
)
return _prioritize
Expand Down
2 changes: 1 addition & 1 deletion api/opds2.py
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ def fulfill(
if "authentication_token" not in templated.variable_names:
return fulfillment

token_auth = licensepool.collection.integration_configuration.settings.get(
token_auth = licensepool.collection.integration_configuration.settings_dict.get(
ExternalIntegration.TOKEN_AUTH
)
if token_auth is None:
Expand Down
2 changes: 1 addition & 1 deletion api/saml/wayfless.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ def __init__(self, collection: Collection) -> None:
external: ExternalIntegration = collection.external_integration
self._wayfless_url_template: Optional[
str
] = collection.integration_configuration.settings.get(
] = collection.integration_configuration.settings_dict.get(
SAMLWAYFlessConstants.WAYFLESS_URL_TEMPLATE_KEY
)

Expand Down
4 changes: 2 additions & 2 deletions api/shared_collection.py
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ def register(self, collection, auth_document_url, do_get=HTTP.get_with_timeout):
)

external_library_urls = (
collection.integration_configuration.settings.get(
collection.integration_configuration.settings_dict.get(
BaseSharedCollectionAPI.EXTERNAL_LIBRARY_URLS
)
or []
Expand Down Expand Up @@ -173,7 +173,7 @@ def register(self, collection, auth_document_url, do_get=HTTP.get_with_timeout):

def check_client_authorization(self, collection, client):
"""Verify that an IntegrationClient is whitelisted for access to the collection."""
external_library_urls = collection.integration_configuration.settings.get(
external_library_urls = collection.integration_configuration.settings_dict.get(
BaseSharedCollectionAPI.EXTERNAL_LIBRARY_URLS, []
)
if client.url not in [
Expand Down
6 changes: 3 additions & 3 deletions core/configuration/ignored_identifier.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ def _get_ignored_identifier_types(
:return: Set of ignored identifier types
"""
if self._ignored_identifier_types is None:
self._ignored_identifier_types = configuration.settings.get(
self._ignored_identifier_types = configuration.settings_dict.get(
"ignored_identifier_types", []
)

Expand Down Expand Up @@ -83,7 +83,7 @@ def set_ignored_identifier_types(
"Argument 'value' must contain string or IdentifierType enumeration's items only"
)

settings = configuration.settings.copy()
settings = configuration.settings_dict.copy()
settings["ignored_identifier_types"] = ignored_identifier_types
configuration.settings = settings
configuration.settings_dict = settings
self._ignored_identifier_types = None
2 changes: 1 addition & 1 deletion core/marc.py
Original file line number Diff line number Diff line change
Expand Up @@ -616,7 +616,7 @@ def get_storage_settings(cls, _db):
# Only add an integration to choose from if it has a
# MARC File Bucket field in its settings.
configuration_settings = [
s for s in integration.settings if s.key == "marc_bucket"
s for s in integration.settings_dict if s.key == "marc_bucket"
]

if configuration_settings:
Expand Down
29 changes: 15 additions & 14 deletions core/model/collection.py
Original file line number Diff line number Diff line change
Expand Up @@ -264,7 +264,7 @@ def by_datasource(cls, _db, data_source):
cls.integration_configuration_id == IntegrationConfiguration.id,
)
.filter(
IntegrationConfiguration.settings[
IntegrationConfiguration.settings_dict[
Collection.DATA_SOURCE_NAME_SETTING
].astext
== data_source
Expand Down Expand Up @@ -297,17 +297,17 @@ def protocol(self, new_protocol):
@hybrid_property
def primary_identifier_source(self):
"""Identify if should try to use another identifier than <id>"""
return self.integration_configuration.settings.get(
return self.integration_configuration.settings_dict.get(
ExternalIntegration.PRIMARY_IDENTIFIER_SOURCE
)

@primary_identifier_source.setter
def primary_identifier_source(self, new_primary_identifier_source):
"""Modify the primary identifier source in use by this Collection."""
self.integration_configuration.settings = (
self.integration_configuration.settings.copy()
self.integration_configuration.settings_dict = (
self.integration_configuration.settings_dict.copy()
)
self.integration_configuration.settings[
self.integration_configuration.settings_dict[
ExternalIntegration.PRIMARY_IDENTIFIER_SOURCE
] = new_primary_identifier_source

Expand Down Expand Up @@ -354,15 +354,15 @@ def default_loan_period_setting(
config = self.integration_configuration

if config:
return config.settings.get(key)
return config.settings_dict.get(key)

DEFAULT_RESERVATION_PERIOD_KEY = "default_reservation_period"
STANDARD_DEFAULT_RESERVATION_PERIOD = 3

def _set_settings(self, **kwargs):
settings = self.integration_configuration.settings.copy()
settings.update(kwargs)
self.integration_configuration.settings = settings
settings_dict = self.integration_configuration.settings_dict.copy()
settings_dict.update(kwargs)
self.integration_configuration.settings_dict = settings_dict

@hybrid_property
def default_reservation_period(self):
Expand All @@ -371,7 +371,7 @@ def default_reservation_period(self):
check it out before it goes to the next person in line.
"""
return (
self.integration_configuration.settings.get(
self.integration_configuration.settings_dict.get(
self.DEFAULT_RESERVATION_PERIOD_KEY
)
or self.STANDARD_DEFAULT_RESERVATION_PERIOD
Expand All @@ -395,7 +395,8 @@ def default_audience(self) -> str:
:return: Default audience
"""
return (
self.integration_configuration.settings.get(self.DEFAULT_AUDIENCE_KEY) or ""
self.integration_configuration.settings_dict.get(self.DEFAULT_AUDIENCE_KEY)
or ""
)

@default_audience.setter
Expand Down Expand Up @@ -515,7 +516,7 @@ def data_source(self):
data_source = None
name = ExternalIntegration.DATA_SOURCE_FOR_LICENSE_PROTOCOL.get(self.protocol)
if not name:
name = self.integration_configuration.settings.get(
name = self.integration_configuration.settings_dict.get(
Collection.DATA_SOURCE_NAME_SETTING
)
_db = Session.object_session(self)
Expand Down Expand Up @@ -696,8 +697,8 @@ def explain(self, include_secrets=False):
lines.append('Used by library: "%s"' % library.short_name)
if self.external_account_id:
lines.append('External account ID: "%s"' % self.external_account_id)
for name in sorted(integration.settings):
value = integration.settings[name]
for name in sorted(integration.settings_dict):
value = integration.settings_dict[name]
if (
include_secrets or not ConfigurationSetting._is_secret(name)
) and value is not None:
Expand Down
8 changes: 6 additions & 2 deletions core/model/integration.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,9 @@ class IntegrationConfiguration(Base):
name = Column(Unicode, nullable=False, unique=True)

# The configuration settings for this integration. Stored as json.
settings: Mapped[Dict[str, Any]] = Column(JSONB, nullable=False, default=dict)
settings_dict: Mapped[Dict[str, Any]] = Column(
"settings", JSONB, nullable=False, default=dict
)

# Self test results, stored as json.
self_test_results = Column(JSONB, nullable=False, default=dict)
Expand Down Expand Up @@ -134,7 +136,9 @@ class IntegrationLibraryConfiguration(Base):
library: Mapped[Library] = relationship("Library")

# The configuration settings for this integration. Stored as json.
settings: Mapped[Dict[str, Any]] = Column(JSONB, nullable=False, default=dict)
settings_dict: Mapped[Dict[str, Any]] = Column(
"settings", JSONB, nullable=False, default=dict
)

def __repr__(self) -> str:
return (
Expand Down
4 changes: 2 additions & 2 deletions core/opds2_import.py
Original file line number Diff line number Diff line change
Expand Up @@ -953,9 +953,9 @@ def _parse_feed_links(self, links: list[core_ast.Link]) -> None:
if first_or_default(link.rels) == Hyperlink.TOKEN_AUTH:
# Save the collection-wide token authentication endpoint
config = self.integration_configuration()
settings = config.settings.copy()
settings = config.settings_dict.copy()
settings[ExternalIntegration.TOKEN_AUTH] = link.href
config.settings = settings
config.settings_dict = settings
Comment on lines +956 to +958
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor: In some cases we're using the variable settings and in others (e.g., core/model/collection.py ll. 363-365), settings_dict. Would be nice to keep these consistent throughout.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This has been hanging out there for awhile, so I'd like to get it merged. I'll try to address this comment in a follow up PR.


def extract_feed_data(
self, feed: str | opds2_ast.OPDS2Feed, feed_url: str | None = None
Expand Down
Loading