diff --git a/docs/API/V1/server_config.md b/docs/API/V1/server_settings.md similarity index 53% rename from docs/API/V1/server_config.md rename to docs/API/V1/server_settings.md index a6d8dec0c5..b9b39b3fbb 100644 --- a/docs/API/V1/server_config.md +++ b/docs/API/V1/server_settings.md @@ -1,9 +1,9 @@ -# `GET /api/v1/server/configuration[/][{key}]` +# `GET /api/v1/server/settings[/][{key}]` This API returns an `application/json` document describing the Pbench Server -configuration settings. When the `{key}` parameter is specified, the API will -return the specific named server configuration setting. When `{key}` is omitted, -all server configuration settings will be returned. +settings. When the `{key}` parameter is specified, the API will return the +specific named server setting. When `{key}` is omitted, all server settings will +be returned. ## Query parameters @@ -13,35 +13,34 @@ None. `authorization: bearer` token [_optional_] \ *Bearer* schema authorization may be specified, but is not required to `GET` -server configuration settings. +server settings. ## Response headers `content-type: application/json` \ -The return is a serialized JSON object with the requested server configuration -settings. +The return is a serialized JSON object with the requested server settings. ## Response status `400` **BAD REQUEST** \ -The specified `{key}` value (see [settings](#server-configuration-settings)) +The specified `{key}` value (see [settings](#server-settings)) is unknown. ## Response body The `application/json` response body is a JSON document containing the requested -server configuration setting key and value or, if no `{key}` was specified, all -supported server configuration settings. +server setting key and value or, if no `{key}` was specified, all supported +server settings. ### Examples ``` -GET /api/v1/server/configuration/dataset-lifetime +GET /api/v1/server/settings/dataset-lifetime { "dataset-lifetime": "4" } -GET /api/v1/server/configuration/ +GET /api/v1/server/settings/ { "dataset-lifetime": "4", "server-banner": { @@ -55,29 +54,28 @@ GET /api/v1/server/configuration/ } ``` -# `PUT /api/v1/server/configuration[/][{key}]` +# `PUT /api/v1/server/settings[/][{key}]` -This API allows a user holding the `ADMIN` role to modify server configuration -settings. When the `{key}` parameter is specified, the API will modify a single -named configuration setting. When `{key}` is omitted, the `application/json` -request body can be used to modify multiple configuration settings at once. +This API allows a user holding the `ADMIN` role to modify server settings. When +the `{key}` parameter is specified, the API will modify a single named setting. +When `{key}` is omitted, the `application/json` request body can be used to +modify multiple server settings at once. ## Query parameters `value` string \ -When a single configuration setting is specified with `{key}` in the URI, you -can specify a string value for the parameter using this query parameter without -an `application/json` request body. For example, -`PUT /api/v1/server/configuration/key?value=1`. +When a single server setting is specified with `{key}` in the URI, you can +specify a string value for the parameter using this query parameter without an +`application/json` request body. For example, `PUT /api/v1/server/settings/key?value=1`. -You cannot specify complex JSON configuration settings this way. Instead, use -the `value` field in the `application/json` request body. +You cannot specify complex JSON server settings this way. Instead, use the +`value` field in the `application/json` request body. ## Request body -When specifying a complex JSON value for a server configuration setting, or when -specifying multiple configuration settings, the data to be set is specified in -an `application/json` request body. +When specifying a complex JSON value for a server setting, or when specifying +multiple server settings, the data to be set is specified in an +`application/json` request body. You can specify a single `{key}` in the URI and then specify the value using a `value` field in the `application/json` request body instead of using @@ -86,19 +84,18 @@ string, although it's more useful when you need to specify a JSON object value. For example, ``` -PUT /api/v1/server/configuration/server-state +PUT /api/v1/server/settings/server-state { "value": {"status": "enabled"} } ``` -If you omit the `{key}` value from the URI, specify all configuration settings -you wish to change in the `application/json` request body. You can specify a -single server configuration setting, or any group of server configuration -settings at once. For example, +If you omit the `{key}` value from the URI, specify all server settings you wish +to change in the `application/json` request body. You can specify a single +server setting, or any group of server settings at once. For example, ``` -PUT /api/v1/server/configuration/ +PUT /api/v1/server/settings/ { "server-state": {"status": "disabled", "message": "down for maintenance"}, "server-banner": {"message": "Days of 100% uptime: 0"} @@ -108,14 +105,13 @@ PUT /api/v1/server/configuration/ ## Request headers `authorization: bearer` token \ -*Bearer* schema authorization is required to change any server configuration -settings. The authenticated user must have `ADMIN` role. +*Bearer* schema authorization is required to change any server settings. The +authenticated user must have `ADMIN` role. ## Response headers `content-type: application/json` \ -The response body is a serialized JSON object with the selected server -configuration settings. +The response body is a serialized JSON object with the selected server settings. ## Response status @@ -129,13 +125,12 @@ role. ## Response body The `application/json` response body for `PUT` is exactly the same as for -[`GET`](#response-body) when the same server configuration settings are -requested, showing only the server configuration settings that were changed -in the `PUT`. +[`GET`](#response-body) when the same server settings are requested, showing +only the server settings that were changed in the `PUT`. _This request:_ ``` -PUT /api/v1/server/configuration/dataset-lifetime?value=4 +PUT /api/v1/server/settings/dataset-lifetime?value=4 ``` _returns this response:_ @@ -148,7 +143,7 @@ _returns this response:_ _And this request:_ ``` -PUT /api/v1/server/configuration +PUT /api/v1/server/settings { "dataset-lifetime": "4 days", "server-state": {"status": "enabled"} @@ -163,18 +158,17 @@ _returns this response:_ } ``` -## Server configuration settings +## Server settings ### dataset-lifetime -The value for the `dataset-lifetime` server configuration setting is the *maximum* -number of days a dataset can be retained on the server. When each dataset is -uploaded to the server, a "deletion date" represented by the dataset -[metadata](../metadata.md) key `server.deletion` is calculated based on this -value and user preferences (which may specify a shorter lifetime, but not a -longer lifetime). When a dataset has remained on the server past the -`server.deletion` date, it may be removed automatically by the server to -conserve space. +The value for the `dataset-lifetime` server setting is the *maximum* number of +days a dataset can be retained on the server. When each dataset is uploaded to +the server, a "deletion date" represented by the dataset [metadata](../metadata.md) +key `server.deletion` is calculated based on this value and user preferences +(which may specify a shorter lifetime, but not a longer lifetime). When a +dataset has remained on the server past the `server.deletion` date, it may be +removed automatically by the server to conserve space. The number of days is specified as an string representing an integer, optionally followed by a space and `day` or `days`. For example, "4" or "4 days" or "4 day" @@ -188,14 +182,14 @@ are equivalent. ### server-banner -This server configuration setting allows a server administrator to set an -informational message that can be retrieved and displayed by any client, for -example as a banner on a UI. The value is a JSON object, containing at least -a `message` field. +This server setting allows a server administrator to set an informational +message that can be retrieved and displayed by any client, for example as a +banner on a UI. The value is a JSON object, containing at least a `message` +field. Any additional JSON data may be provided. The server will store the entire JSON object and return it when a client requests it with -`GET /api/v1/server/configuration/server-banner`. The server will not interpret +`GET /api/v1/server/settings/server-banner`. The server will not interpret any information in this JSON object. For example, the following are examples of valid banners: @@ -228,11 +222,10 @@ For example, the following are examples of valid banners: ### server-state -This server configuration setting allows a server administrator to control -the operating state of the server remotely. As for -[`server-banner`](#server-banner), the value is a JSON object, and any JSON -fields passed in to the server will be returned to a client. The -following fields have special meaning: +This server setting allows a server administrator to control the operating state +of the server remotely. As for [`server-banner`](#server-banner), the value is a +JSON object, and any JSON fields passed in to the server will be returned to a +client. The following fields have special meaning: **`status`** \ The operating status of the server. @@ -240,13 +233,15 @@ The operating status of the server. * `enabled`: Normal operation. * `disabled`: Most server API endpoints will fail with the **503** (service unavailable) HTTP status. However a few endpoints are always allowed: - * [endpoints](./endpoints.md) for server configuration information; + * [endpoints](./endpoints.md) for server settings information; * [login](./login.md) because only an authenticated user with `ADMIN` role - can modify server configuration settings; + can modify server settings; * [logout](./logout.md) for consistency; - * [server_configuration](./server_config.md) to allow re-enabling the server + * [server_settings](./server_settings.md) to allow re-enabling the server or modifying the banner. -* `readonly`: The server will respond to `GET` requests for information, but will return **503** (service unavailable) for any attempt to modify server state. (With the same exceptions as listed above for `disabled`.) +* `readonly`: The server will respond to `GET` requests for information, but +will return **503** (service unavailable) for any attempt to modify server +state. (With the same exceptions as listed above for `disabled`.) **`message`** \ A message to explain downtime. This is required when the `status` is `disabled` @@ -261,8 +256,7 @@ will display this as an explanation for the failure. The client will also have access to any additional information provided in the `server-state` JSON object. Note that you can set a `message` when the `status` is `enabled` but it won't -be reported to a client unless a client asks for the `server-state` -configuration setting. The `server-state` message is intended to explain server -downtime when an API call fails. The `server-banner` configuration setting -is generally more appropriate to provide client information under normal -operating conditions. +be reported to a client unless a client asks for the `server-state` setting. The +`server-state` message is intended to explain server downtime when an API call +fails. The `server-banner` setting is generally more appropriate to provide +client information under normal operating conditions. diff --git a/lib/pbench/server/api/__init__.py b/lib/pbench/server/api/__init__.py index 8d0d04a7aa..5766bd6e56 100644 --- a/lib/pbench/server/api/__init__.py +++ b/lib/pbench/server/api/__init__.py @@ -36,7 +36,7 @@ from pbench.server.api.resources.query_apis.datasets_publish import DatasetsPublish from pbench.server.api.resources.query_apis.datasets_search import DatasetsSearch from pbench.server.api.resources.server_audit import ServerAudit -from pbench.server.api.resources.server_configuration import ServerConfiguration +from pbench.server.api.resources.server_settings import ServerSettings from pbench.server.api.resources.upload_api import Upload from pbench.server.api.resources.users_api import Login, Logout, RegisterUser, UserAPI from pbench.server.auth.auth import Auth @@ -163,11 +163,11 @@ def register_endpoints(api, app, config): resource_class_args=(config,), ) api.add_resource( - ServerConfiguration, - f"{base_uri}/server/configuration", - f"{base_uri}/server/configuration/", - f"{base_uri}/server/configuration/", - endpoint="server_configuration", + ServerSettings, + f"{base_uri}/server/settings", + f"{base_uri}/server/settings/", + f"{base_uri}/server/settings/", + endpoint="server_settings", resource_class_args=(config,), ) api.add_resource( diff --git a/lib/pbench/server/api/resources/__init__.py b/lib/pbench/server/api/resources/__init__.py index 9d913b5d4d..c65fbe724d 100644 --- a/lib/pbench/server/api/resources/__init__.py +++ b/lib/pbench/server/api/resources/__init__.py @@ -27,7 +27,7 @@ MetadataBadKey, MetadataError, ) -from pbench.server.database.models.server_config import ServerConfig +from pbench.server.database.models.server_settings import ServerSetting from pbench.server.database.models.users import User @@ -1556,7 +1556,7 @@ def _dispatch( schema = self.schemas[method] if not self.always_enabled: readonly = schema.operation == OperationCode.READ - disabled = ServerConfig.get_disabled(readonly=readonly) + disabled = ServerSetting.get_disabled(readonly=readonly) if disabled: abort(HTTPStatus.SERVICE_UNAVAILABLE, **disabled) diff --git a/lib/pbench/server/api/resources/query_apis/__init__.py b/lib/pbench/server/api/resources/query_apis/__init__.py index 1b639f6544..c1b162df10 100644 --- a/lib/pbench/server/api/resources/query_apis/__init__.py +++ b/lib/pbench/server/api/resources/query_apis/__init__.py @@ -32,7 +32,7 @@ from pbench.server.auth.auth import Auth from pbench.server.database.models.audit import AuditReason, AuditStatus from pbench.server.database.models.datasets import Dataset, Metadata, States -from pbench.server.database.models.template import Template +from pbench.server.database.models.templates import Template from pbench.server.database.models.users import User diff --git a/lib/pbench/server/api/resources/query_apis/datasets/__init__.py b/lib/pbench/server/api/resources/query_apis/datasets/__init__.py index 3269ac0a78..738bc09275 100644 --- a/lib/pbench/server/api/resources/query_apis/datasets/__init__.py +++ b/lib/pbench/server/api/resources/query_apis/datasets/__init__.py @@ -14,7 +14,7 @@ ) from pbench.server.api.resources.query_apis import ElasticBase from pbench.server.database.models.datasets import Dataset, Metadata, MetadataError -from pbench.server.database.models.template import Template +from pbench.server.database.models.templates import Template class MissingDatasetNameParameter(SchemaError): diff --git a/lib/pbench/server/api/resources/query_apis/datasets/datasets_mappings.py b/lib/pbench/server/api/resources/query_apis/datasets/datasets_mappings.py index aadbec4eb5..f6da2c01e9 100644 --- a/lib/pbench/server/api/resources/query_apis/datasets/datasets_mappings.py +++ b/lib/pbench/server/api/resources/query_apis/datasets/datasets_mappings.py @@ -15,7 +15,7 @@ Schema, ) from pbench.server.api.resources.query_apis.datasets import IndexMapBase -from pbench.server.database.models.template import TemplateNotFound +from pbench.server.database.models.templates import TemplateNotFound class DatasetsMappings(ApiBase): diff --git a/lib/pbench/server/api/resources/query_apis/datasets/namespace_and_rows.py b/lib/pbench/server/api/resources/query_apis/datasets/namespace_and_rows.py index 1cff94d0f3..261c4cbcc2 100644 --- a/lib/pbench/server/api/resources/query_apis/datasets/namespace_and_rows.py +++ b/lib/pbench/server/api/resources/query_apis/datasets/namespace_and_rows.py @@ -16,7 +16,7 @@ from pbench.server.api.resources.query_apis import ApiContext, PostprocessError from pbench.server.api.resources.query_apis.datasets import IndexMapBase from pbench.server.database.models.datasets import Dataset -from pbench.server.database.models.template import TemplateNotFound +from pbench.server.database.models.templates import TemplateNotFound class SampleNamespace(IndexMapBase): diff --git a/lib/pbench/server/api/resources/server_configuration.py b/lib/pbench/server/api/resources/server_settings.py similarity index 66% rename from lib/pbench/server/api/resources/server_configuration.py rename to lib/pbench/server/api/resources/server_settings.py index 5a7eaf7a8f..94126a8b92 100644 --- a/lib/pbench/server/api/resources/server_configuration.py +++ b/lib/pbench/server/api/resources/server_settings.py @@ -19,16 +19,16 @@ Schema, ) from pbench.server.database.models.audit import AuditType -from pbench.server.database.models.server_config import ( - ServerConfig, - ServerConfigBadValue, - ServerConfigError, +from pbench.server.database.models.server_settings import ( + ServerSetting, + ServerSettingBadValue, + ServerSettingError, ) -class ServerConfiguration(ApiBase): +class ServerSettings(ApiBase): """ - API class to retrieve and mutate server configuration settings. + API class to retrieve and mutate server settings. """ def __init__(self, config: PbenchServerConfig): @@ -38,19 +38,19 @@ def __init__(self, config: PbenchServerConfig): ApiMethod.PUT, OperationCode.UPDATE, uri_schema=Schema( - Parameter("key", ParamType.KEYWORD, keywords=ServerConfig.KEYS) + Parameter("key", ParamType.KEYWORD, keywords=ServerSetting.KEYS) ), - # We aspire to be flexible about how a config option is - # specified on a PUT. The value of a single config option can - # be set using the "value" query parameter or the "value" JSON - # body parameter. You can also specify one config option or - # multiple config options by omitting the key name from the URI - # and specifying the names and values in a JSON request body: + # We aspire to be flexible about how a setting is specified on a + # PUT. The value of a single setting can be set using the + # "value" query parameter or the "value" JSON body parameter. + # You can also specify one setting or multiple settings by + # omitting the key name from the URI and specifying the names + # and values in a JSON request body: # - # PUT /server/configuration/dataset-lifetime?value=2y - # PUT /server/configuration/dataset-lifetime + # PUT /server/settings/dataset-lifetime?value=2y + # PUT /server/settings/dataset-lifetime # {"value": "2y"} - # PUT /server/configuration + # PUT /server/settings # {"dataset-lifetime": "2y"} query_schema=Schema(Parameter("value", ParamType.STRING)), body_schema=Schema( @@ -64,7 +64,7 @@ def __init__(self, config: PbenchServerConfig): ApiMethod.GET, OperationCode.READ, uri_schema=Schema( - Parameter("key", ParamType.KEYWORD, keywords=ServerConfig.KEYS) + Parameter("key", ParamType.KEYWORD, keywords=ServerSetting.KEYS) ), authorization=ApiAuthorizationType.NONE, ), @@ -75,15 +75,15 @@ def _get( self, params: ApiParams, request: Request, context: ApiContext ) -> Response: """ - Get the values of server configuration parameters. + Get the values of server settings. - GET /api/v1/server/configuration/{key} - return the value of a single configuration parameter + GET /api/v1/server/settings/{key} + return the value of a single server setting or - GET /api/v1/server/configuration - return all configuration parameters + GET /api/v1/server/settings + return all server settings Args: params: API parameters @@ -97,21 +97,21 @@ def _get( key = params.uri.get("key") try: if key: - s = ServerConfig.get(key) + s = ServerSetting.get(key) return jsonify({key: s.value if s else None}) else: - return jsonify(ServerConfig.get_all()) - except ServerConfigError as e: - raise APIInternalError(f"Error reading server configuration {key}") from e + return jsonify(ServerSetting.get_all()) + except ServerSettingError as e: + raise APIInternalError(f"Error reading server setting {key}") from e def _put_key(self, params: ApiParams, context: ApiContext) -> Response: """ - Implement the PUT operation when a system configuration setting key is - specified on the URI as /system/configuration/{key}. + Implement the PUT operation when a server setting key is specified on + the URI as /server/settings/{key}. - A single system config setting is set by naming the config key in the - URI and specifying a value using either the "value" query parameter or - a "value" key in a JSON request body. + A single server setting is set by naming the key in the URI and + specifying a value using either the "value" query parameter or a "value" + key in a JSON request body. We'll complain about JSON request body parameters that are "shadowed" by the "value" query parameter and might represent client confusion. @@ -152,23 +152,23 @@ def _put_key(self, params: ApiParams, context: ApiContext) -> Response: if not value: raise APIAbort( HTTPStatus.BAD_REQUEST, - f"No value found for key system configuration key {key!r}", + f"No value found for server settings key {key!r}", ) context["auditing"]["attributes"] = {"updated": {key: value}} try: - ServerConfig.set(key=key, value=value) - except ServerConfigBadValue as e: + ServerSetting.set(key=key, value=value) + except ServerSettingBadValue as e: raise APIAbort(HTTPStatus.BAD_REQUEST, str(e)) from e - except ServerConfigError as e: - raise APIInternalError(f"Error setting server configuration {key}") from e + except ServerSettingError as e: + raise APIInternalError(f"Error setting server setting {key}") from e return jsonify({key: value}) def _put_body(self, params: ApiParams, context: ApiContext) -> Response: """ - Allow setting the value of multiple system configuration settings with - a single PUT by specifying a JSON request body with key/value pairs. + Allow setting the value of multiple server settings with a single PUT by + specifying a JSON request body with key/value pairs. Args: params: API parameters @@ -179,13 +179,13 @@ def _put_body(self, params: ApiParams, context: ApiContext) -> Response: """ badkeys = [] for k, v in params.body.items(): - if k not in ServerConfig.KEYS: + if k not in ServerSetting.KEYS: badkeys.append(k) if badkeys: raise APIAbort( HTTPStatus.BAD_REQUEST, - f"Unrecognized configuration parameters {sorted(badkeys)!r} specified: valid parameters are {sorted(ServerConfig.KEYS)!r}", + f"Unrecognized server setting {sorted(badkeys)!r} specified: valid settings are {sorted(ServerSetting.KEYS)!r}", ) context["auditing"]["attributes"] = {"updated": params.body} @@ -194,13 +194,13 @@ def _put_body(self, params: ApiParams, context: ApiContext) -> Response: response = {} for k, v in params.body.items(): try: - c = ServerConfig.set(key=k, value=v) + c = ServerSetting.set(key=k, value=v) response[c.key] = c.value - except ServerConfigBadValue as e: + except ServerSettingBadValue as e: failures.append(str(e)) except Exception as e: current_app.logger.warning("{}", e) - raise APIInternalError(f"Error setting server configuration {k}") + raise APIInternalError(f"Error setting server setting {k}") if failures: raise APIAbort(HTTPStatus.BAD_REQUEST, message=", ".join(failures)) return jsonify(response) @@ -209,17 +209,17 @@ def _put( self, params: ApiParams, request: Request, context: ApiContext ) -> Response: """ - Set or modify the values of server configuration keys. + Set or modify the values of server setting keys. - PUT /api/v1/server/configuration + PUT /api/v1/server/settings { "dataset-lifetime": 10, "server-state": "running" } - PUT /api/v1/server/configuration/dataset-lifetime?value=10 + PUT /api/v1/server/settings/dataset-lifetime?value=10 - PUT /api/v1/server/configuration/dataset-lifetime + PUT /api/v1/server/settings/dataset-lifetime { "value": "10" } diff --git a/lib/pbench/server/api/resources/users_api.py b/lib/pbench/server/api/resources/users_api.py index f503350b3e..95b038240f 100644 --- a/lib/pbench/server/api/resources/users_api.py +++ b/lib/pbench/server/api/resources/users_api.py @@ -10,8 +10,8 @@ from sqlalchemy.exc import IntegrityError, SQLAlchemyError from pbench.server.auth.auth import Auth -from pbench.server.database.models.active_tokens import ActiveTokens -from pbench.server.database.models.server_config import ServerConfig +from pbench.server.database.models.auth_tokens import AuthToken +from pbench.server.database.models.server_settings import ServerSetting from pbench.server.database.models.users import User @@ -48,7 +48,7 @@ def post(self): } To get the auth token user has to perform the login action """ - disabled = ServerConfig.get_disabled() + disabled = ServerSetting.get_disabled() if disabled: abort(HTTPStatus.SERVICE_UNAVAILABLE, **disabled) @@ -233,9 +233,8 @@ def post(self): # Add the new auth token to the database for later access try: - token = ActiveTokens(auth_token=auth_token) # TODO: Decide on the auth token limit per user - user.update(auth_tokens=token) + user.update(auth_token=AuthToken(auth_token=auth_token)) current_app.logger.info( "New auth token registered for user {}", user.username @@ -294,7 +293,7 @@ def post(self): if user: try: - ActiveTokens.delete(auth_token) + AuthToken.delete(auth_token) except Exception: current_app.logger.exception( "Exception occurred deleting an auth token" @@ -391,7 +390,7 @@ def get(self, target_username): "message": "failure message" } """ - disabled = ServerConfig.get_disabled(readonly=True) + disabled = ServerSetting.get_disabled(readonly=True) if disabled: abort(HTTPStatus.SERVICE_UNAVAILABLE, **disabled) @@ -434,7 +433,7 @@ def put(self, target_username): "message": "failure message" } """ - disabled = ServerConfig.get_disabled() + disabled = ServerSetting.get_disabled() if disabled: abort(HTTPStatus.SERVICE_UNAVAILABLE, **disabled) @@ -511,7 +510,7 @@ def delete(self, target_username): "message": "failure message" } """ - disabled = ServerConfig.get_disabled() + disabled = ServerSetting.get_disabled() if disabled: abort(HTTPStatus.SERVICE_UNAVAILABLE, **disabled) diff --git a/lib/pbench/server/auth/auth.py b/lib/pbench/server/auth/auth.py index 6bbde028af..26ac1afae3 100644 --- a/lib/pbench/server/auth/auth.py +++ b/lib/pbench/server/auth/auth.py @@ -11,7 +11,7 @@ from pbench.server import PbenchServerConfig from pbench.server.auth import OpenIDClient, OpenIDClientError -from pbench.server.database.models.active_tokens import ActiveTokens +from pbench.server.database.models.auth_tokens import AuthToken from pbench.server.database.models.users import User @@ -199,12 +199,12 @@ def verify_auth(auth_token: str) -> Optional[Union[User, InternalUser]]: }, ) user_id = payload["sub"] - if ActiveTokens.valid(auth_token): + if AuthToken.valid(auth_token): user = User.query(id=user_id) return user except jwt.ExpiredSignatureError: try: - ActiveTokens.delete(auth_token) + AuthToken.delete(auth_token) except Exception as e: Auth.logger.error( "User passed expired token but we could not delete the" diff --git a/lib/pbench/server/database/__init__.py b/lib/pbench/server/database/__init__.py index 00a71115f7..ed60a41bbd 100644 --- a/lib/pbench/server/database/__init__.py +++ b/lib/pbench/server/database/__init__.py @@ -4,11 +4,11 @@ of the same is required here. """ from pbench.server.database.database import Database -from pbench.server.database.models.active_tokens import ActiveTokens # noqa F401 from pbench.server.database.models.audit import Audit # noqa F401 +from pbench.server.database.models.auth_tokens import AuthToken # noqa F401 from pbench.server.database.models.datasets import Dataset, Metadata # noqa F401 -from pbench.server.database.models.server_config import ServerConfig # noqa F401 -from pbench.server.database.models.template import Template # noqa F401 +from pbench.server.database.models.server_settings import ServerSetting # noqa F401 +from pbench.server.database.models.templates import Template # noqa F401 from pbench.server.database.models.users import User # noqa F401 diff --git a/lib/pbench/server/database/alembic/versions/e6b44fb7c065_rework_database_models_and_class_names.py b/lib/pbench/server/database/alembic/versions/e6b44fb7c065_rework_database_models_and_class_names.py new file mode 100644 index 0000000000..dc26859846 --- /dev/null +++ b/lib/pbench/server/database/alembic/versions/e6b44fb7c065_rework_database_models_and_class_names.py @@ -0,0 +1,96 @@ +"""Rework database modules and class names + +Revision ID: e6b44fb7c065 +Revises: fa12f45a2a5a +Create Date: 2023-01-23 20:44:32.238138 +""" +from alembic import op + +# revision identifiers, used by Alembic. +revision = "e6b44fb7c065" +down_revision = "fa12f45a2a5a" +branch_labels = None +depends_on = None + + +def rename_table(old: str, new: str): + """Rename a table from the given old name to the new name. + + It also renames the primary key index and the associated sequence for the ID + field. + + Args: + old : exiting table name + new : new table name + """ + # Rename table "serverconfig" to "server_settings", updating the primary key + # index and the sequence for IDs. + op.rename_table(old, new) + op.execute(f"ALTER INDEX {old}_pkey RENAME TO {new}_pkey") + op.execute(f"ALTER SEQUENCE {old}_id_seq RENAME TO {new}_id_seq") + + +def rename_index(column: str, old: str, new: str): + """Rename an index from an existing table to a new table where the column + name does not change. + + Args: + column : column name of associated index to rename + old : existing table name + new : new table name + """ + op.execute(f"ALTER INDEX ix_{old}_{column} RENAME TO ix_{new}_{column}") + + +def rename_fkey(column: str, old: str, new: str): + """Rename a foreign key constraint for an existing table to a new table + where the column name does not change. + + Args: + column : column name of associated foreign key constraint to rename + old : existing table name + new : new table name + """ + op.execute( + f'ALTER TABLE {new} RENAME CONSTRAINT "{old}_{column}_fkey" TO "{new}_{column}_fkey"' + ) + + +def rename_column(old_table: str, new_table: str, old_column: str, new_column: str): + """Rename a column in a new table and update the existing index to the new + table and column name. + + This MUST following a `rename_table()` operation. + + Args: + old_table : existing table name + new_table : new table name where the column lives + old_column : existing column name + new_column : new column name + """ + op.execute( + f'ALTER TABLE {new_table} RENAME COLUMN "{old_column}" TO "{new_column}"' + ) + op.execute( + f"ALTER INDEX ix_{old_table}_{old_column} RENAME TO ix_{new_table}_{new_column}" + ) + + +def upgrade() -> None: + # Rename table "serverconfig" to "server_settings", updating the index names + # and sequence for IDs. + rename_table("serverconfig", "server_settings") + rename_index("key", "serverconfig", "server_settings") + + rename_table("active_tokens", "auth_tokens") + rename_fkey("user_id", "active_tokens", "auth_tokens") + rename_column("active_tokens", "auth_tokens", "token", "auth_token") + + +def downgrade() -> None: + rename_table("server_settings", "serverconfig") + rename_index("key", "server_settings", "serverconfig") + + rename_table("auth_tokens", "active_tokens") + rename_fkey("user_id", "auth_tokens", "active_tokens") + rename_column("auth_tokens", "active_tokens", "auth_token", "token") diff --git a/lib/pbench/server/database/models/active_tokens.py b/lib/pbench/server/database/models/auth_tokens.py similarity index 65% rename from lib/pbench/server/database/models/active_tokens.py rename to lib/pbench/server/database/models/auth_tokens.py index 6191cd6845..8e5c285a98 100644 --- a/lib/pbench/server/database/models/active_tokens.py +++ b/lib/pbench/server/database/models/auth_tokens.py @@ -6,12 +6,13 @@ from pbench.server.database.database import Database -class ActiveTokens(Database.Base): +class AuthToken(Database.Base): """Model for storing the active auth tokens associated with a user.""" - __tablename__ = "active_tokens" + # Table name is plural so that SQL statements read easier. + __tablename__ = "auth_tokens" id = Column(Integer, primary_key=True, autoincrement=True) - token = Column(String(500), unique=True, nullable=False, index=True) + auth_token = Column(String(500), unique=True, nullable=False, index=True) created = Column(DateTime, nullable=False) user_id = Column( Integer, @@ -21,21 +22,23 @@ class ActiveTokens(Database.Base): ) def __init__(self, auth_token: str): - self.token = auth_token + self.auth_token = auth_token self.created = datetime.datetime.now() @staticmethod - def query(auth_token: str) -> Optional["ActiveTokens"]: + def query(auth_token: str) -> Optional["AuthToken"]: """Find the given auth token in the database. Returns: - An ActiveTokens object if found, otherwise None + An AuthToken object if found, otherwise None """ # We currently only query token database with given token - active_token = ( - Database.db_session.query(ActiveTokens).filter_by(token=auth_token).first() + auth_token_o = ( + Database.db_session.query(AuthToken) + .filter_by(auth_token=auth_token) + .first() ) - return active_token + return auth_token_o @staticmethod def delete(auth_token: str): @@ -45,7 +48,9 @@ def delete(auth_token: str): auth_token : the auth token to delete """ try: - Database.db_session.query(ActiveTokens).filter_by(token=auth_token).delete() + Database.db_session.query(AuthToken).filter_by( + auth_token=auth_token + ).delete() Database.db_session.commit() except Exception: Database.db_session.rollback() @@ -58,4 +63,4 @@ def valid(auth_token: str) -> bool: Returns: True if valid (token is found in the database), False otherwise. """ - return bool(ActiveTokens.query(auth_token)) + return bool(AuthToken.query(auth_token)) diff --git a/lib/pbench/server/database/models/datasets.py b/lib/pbench/server/database/models/datasets.py index a451c6a648..40aabf1a79 100644 --- a/lib/pbench/server/database/models/datasets.py +++ b/lib/pbench/server/database/models/datasets.py @@ -12,9 +12,9 @@ from pbench.server.database.database import Database from pbench.server.database.models import TZDateTime -from pbench.server.database.models.server_config import ( +from pbench.server.database.models.server_settings import ( OPTION_DATASET_LIFETIME, - ServerConfig, + ServerSetting, ) @@ -322,6 +322,7 @@ class Dataset(Database.Base): transition The timestamp of the last state transition """ + # Table name is plural so that SQL statements read easier. __tablename__ = "datasets" # This dict defines the allowed dataset state transitions through its @@ -666,6 +667,8 @@ class Metadata(Database.Base): value Metadata value string """ + # SQLAlchemy reserves the name `metadata` for its own use so we add the + # "dataset_" prefix to avoid the conflict. __tablename__ = "dataset_metadata" # +++ Standard Metadata keys: @@ -979,7 +982,7 @@ def validate(dataset: Dataset, key: str, value: Any) -> Any: and we fail otherwise. We store only the UTC date, as we don't guarantee that deletion will occur at any specific time of day. The maximum retention period (from upload) is defined by the server - configuration dataset-lifetime property. + setting dataset-lifetime property. For any other key value, there's no required format. @@ -1010,7 +1013,7 @@ def validate(dataset: Dataset, key: str, value: Any) -> Any: except date_parser.ParserError as p: raise MetadataBadValue(dataset, key, value, "date/time") from p - max_retention = ServerConfig.get(key=OPTION_DATASET_LIFETIME) + max_retention = ServerSetting.get(key=OPTION_DATASET_LIFETIME) maximum = dataset.uploaded + datetime.timedelta( days=int(max_retention.value) ) diff --git a/lib/pbench/server/database/models/server_config.py b/lib/pbench/server/database/models/server_settings.py similarity index 65% rename from lib/pbench/server/database/models/server_config.py rename to lib/pbench/server/database/models/server_settings.py index bc43e27e44..5bf4da0d7a 100644 --- a/lib/pbench/server/database/models/server_config.py +++ b/lib/pbench/server/database/models/server_settings.py @@ -1,3 +1,4 @@ +"""A framework to manage Pbench configuration settings.""" import re from typing import Optional @@ -9,17 +10,17 @@ from pbench.server.database.database import Database -class ServerConfigError(Exception): - """A base class for errors reported by the ServerConfig class. +class ServerSettingError(Exception): + """A base class for errors reported by the ServerSetting class. It is never raised directly, but may be used in "except" clauses. """ -class ServerConfigSqlError(ServerConfigError): - """SQLAlchemy errors reported through ServerConfig operations. +class ServerSettingSqlError(ServerSettingError): + """SQLAlchemy errors reported through ServerSetting operations. - The exception will identify the operation being performed and the config + The exception will identify the operation being performed and the setting key; the cause will specify the original SQLAlchemy exception. """ @@ -32,19 +33,19 @@ def __str__(self) -> str: return f"Error {self.operation} index {self.name!r}: {self.cause}" -class ServerConfigDuplicate(ServerConfigError): - """Attempt to commit a duplicate ServerConfig.""" +class ServerSettingDuplicate(ServerSettingError): + """Attempt to commit a duplicate ServerSetting.""" def __init__(self, name: str, cause: str): self.name = name self.cause = cause def __str__(self) -> str: - return f"Duplicate config setting {self.name!r}: {self.cause}" + return f"Duplicate server setting {self.name!r}: {self.cause}" -class ServerConfigNullKey(ServerConfigError): - """Attempt to commit a ServerConfig with an empty key.""" +class ServerSettingNullKey(ServerSettingError): + """Attempt to commit a ServerSetting with an empty key.""" def __init__(self, name: str, cause: str): self.name = name @@ -54,32 +55,34 @@ def __str__(self) -> str: return f"Missing key value in {self.name!r}: {self.cause}" -class ServerConfigMissingKey(ServerConfigError): - """Attempt to set a ServerConfig with a missing key.""" +class ServerSettingMissingKey(ServerSettingError): + """Attempt to set a ServerSetting with a missing key.""" def __str__(self) -> str: - return "Missing key name" + return "Missing server setting key name" -class ServerConfigBadKey(ServerConfigError): - """Attempt to commit a ServerConfig with a bad key.""" +class ServerSettingBadKey(ServerSettingError): + """Attempt to commit a ServerSetting with a bad key.""" def __init__(self, key: str): self.key = key def __str__(self) -> str: - return f"Configuration key {self.key!r} is unknown" + return f"Server setting key {self.key!r} is unknown" -class ServerConfigBadValue(ServerConfigError): - """Attempt to assign a bad value to a server configuration option.""" +class ServerSettingBadValue(ServerSettingError): + """Attempt to assign a bad value to a server setting option.""" def __init__(self, key: str, value: JSONVALUE): self.key = key self.value = value def __str__(self) -> str: - return f"Unsupported value for configuration key {self.key!r} ({self.value!r})" + return ( + f"Unsupported value for server settings key {self.key!r} ({self.value!r})" + ) # Formal timedelta syntax is "[D day[s], ][H]H:MM:SS[.UUUUUU]"; without an @@ -95,7 +98,7 @@ def validate_lifetime(key: str, value: JSONVALUE) -> JSONVALUE: if check: days = check.group("days") else: - raise ServerConfigBadValue(key, value) + raise ServerSettingBadValue(key, value) return days @@ -134,14 +137,14 @@ def validate_server_state(key: str, value: JSONVALUE) -> JSONVALUE: try: status = value[STATE_STATUS_KEY].lower() except (KeyError, SyntaxError, TypeError) as e: - raise ServerConfigBadValue(key, value) from e + raise ServerSettingBadValue(key, value) from e if status not in STATE_STATUS_KEYWORDS: - raise ServerConfigBadValue(key, value) + raise ServerSettingBadValue(key, value) # canonicalize the status value by lowercasing it value[STATE_STATUS_KEY] = status if status != STATE_STATUS_KEYWORD_ENABLED and STATE_MESSAGE_KEY not in value: - raise ServerConfigBadValue(key, value) + raise ServerSettingBadValue(key, value) return value @@ -153,7 +156,7 @@ def validate_server_state(key: str, value: JSONVALUE) -> JSONVALUE: def validate_server_banner(key: str, value: JSONVALUE) -> JSONVALUE: if not isinstance(value, dict) or BANNER_MESSAGE_KEY not in value: - raise ServerConfigBadValue(key, value) + raise ServerSettingBadValue(key, value) return value @@ -161,10 +164,10 @@ def validate_server_banner(key: str, value: JSONVALUE) -> JSONVALUE: OPTION_SERVER_BANNER = "server-banner" OPTION_SERVER_STATE = "server-state" -SERVER_CONFIGURATION_OPTIONS = { +SERVER_SETTINGS_OPTIONS = { OPTION_DATASET_LIFETIME: { "validator": validate_lifetime, - "default": lambda: str(ServerConfig.config.max_retention_period), + "default": lambda: str(ServerSetting.config.max_retention_period), }, OPTION_SERVER_BANNER: { "validator": validate_server_banner, @@ -177,21 +180,18 @@ def validate_server_banner(key: str, value: JSONVALUE) -> JSONVALUE: } -class ServerConfig(Database.Base): - """A framework to manage Pbench configuration settings. - - This is a simple key-value store that can be used flexibly to manage - runtime configuration. +class ServerSetting(Database.Base): + """A simple key-value store used to manage runtime settings. Columns: id Generated unique ID of table row - key Configuration key name - value Configuration key value + key Setting key name + value Setting key value """ - KEYS = sorted([s for s in SERVER_CONFIGURATION_OPTIONS.keys()]) + KEYS = sorted([s for s in SERVER_SETTINGS_OPTIONS.keys()]) - __tablename__ = "serverconfig" + __tablename__ = "server_settings" id = Column(Integer, primary_key=True, autoincrement=True) key = Column(String(255), unique=True, index=True, nullable=False) @@ -200,92 +200,93 @@ class ServerConfig(Database.Base): @staticmethod def _default(key: str) -> JSONVALUE: try: - config = SERVER_CONFIGURATION_OPTIONS[key] + setting = SERVER_SETTINGS_OPTIONS[key] except KeyError as e: if key: - raise ServerConfigBadKey(key) from e + raise ServerSettingBadKey(key) from e else: - raise ServerConfigMissingKey() from e - return config["default"]() + raise ServerSettingMissingKey() from e + return setting["default"]() @staticmethod def _validate(key: str, value: JSONVALUE) -> JSONVALUE: try: - config = SERVER_CONFIGURATION_OPTIONS[key] + setting = SERVER_SETTINGS_OPTIONS[key] except KeyError as e: if key: - raise ServerConfigBadKey(key) from e + raise ServerSettingBadKey(key) from e else: - raise ServerConfigMissingKey() from e - return config["validator"](key, value) + raise ServerSettingMissingKey() from e + return setting["validator"](key, value) @staticmethod - def create(key: str, value: JSONVALUE) -> "ServerConfig": - """A simple factory method to construct a new ServerConfig setting and + def create(key: str, value: JSONVALUE) -> "ServerSetting": + """A simple factory method to construct a new ServerSetting setting and add it to the database. Args: - key : config setting key - value : config setting value + key : server setting key + value : server setting value Returns: - A new ServerConfig object initialized with the parameters and added + A new ServerSetting object initialized with the settings and added to the database. """ v = __class__._validate(key, value) - config = ServerConfig(key=key, value=v) - config.add() - return config + setting = ServerSetting(key=key, value=v) + setting.add() + return setting @staticmethod - def get(key: str, use_default: bool = True) -> "ServerConfig": - """Return a ServerConfig object with the specified configuration key - setting. + def get(key: str, use_default: bool = True) -> "ServerSetting": + """Return a ServerSetting object with the specified key setting. - For example, ServerConfig.get("dataset-lifetime"). + For example, ServerSetting.get("dataset-lifetime"). If the setting has no definition, a default value will optionally be provided. Args: - key : System configuration setting name + key : System setting name use_default : If the DB value is None, return a default Raises: - ServerConfigSqlError : problem interacting with Database + ServerSettingSqlError : problem interacting with Database Returns: - ServerConfig object with the specified key name or None + ServerSetting object with the specified key name or None """ try: - config = Database.db_session.query(ServerConfig).filter_by(key=key).first() - if config is None and use_default: - config = ServerConfig(key=key, value=__class__._default(key)) + setting = ( + Database.db_session.query(ServerSetting).filter_by(key=key).first() + ) + if setting is None and use_default: + setting = ServerSetting(key=key, value=__class__._default(key)) except SQLAlchemyError as e: - raise ServerConfigSqlError("finding", key, str(e)) from e - return config + raise ServerSettingSqlError("finding", key, str(e)) from e + return setting @staticmethod - def set(key: str, value: JSONVALUE) -> "ServerConfig": - """Update a ServerConfig key with the specified value. + def set(key: str, value: JSONVALUE) -> "ServerSetting": + """Update a ServerSetting key with the specified value. - For example, ServerConfig.set("dataset-lifetime"). + For example, ServerSetting.set("dataset-lifetime"). Args: key : Configuration setting name Returns: - ServerConfig object with the specified key name + ServerSetting object with the specified key name """ v = __class__._validate(key, value) - config = __class__.get(key, use_default=False) - if config: - config.value = v - config.update() + setting = __class__.get(key, use_default=False) + if setting: + setting.value = v + setting.update() else: - config = __class__.create(key=key, value=v) - return config + setting = __class__.create(key=key, value=v) + return setting @staticmethod def get_disabled(readonly: bool = False) -> Optional[JSONOBJECT]: @@ -313,9 +314,9 @@ def get_disabled(readonly: bool = False) -> Optional[JSONOBJECT]: @staticmethod def get_all() -> JSONOBJECT: - """Return all server config settings as a JSON object.""" - configs = Database.db_session.query(ServerConfig).all() - db = {c.key: c.value for c in configs} + """Return all server settings as a JSON object.""" + settings = Database.db_session.query(ServerSetting).all() + db = {c.key: c.value for c in settings} return {k: db[k] if k in db else __class__._default(k) for k in __class__.KEYS} def __str__(self) -> str: @@ -342,13 +343,13 @@ def _decode(self, exception: IntegrityError) -> Exception: # returns (message); so always take the last element. cause = exception.orig.args[-1] if cause.find("UNIQUE constraint") != -1: - return ServerConfigDuplicate(self.key, cause) + return ServerSettingDuplicate(self.key, cause) elif cause.find("NOT NULL constraint") != -1: - return ServerConfigNullKey(self.key, cause) + return ServerSettingNullKey(self.key, cause) return exception def add(self): - """Add the ServerConfig object to the database.""" + """Add the ServerSetting object to the database.""" try: Database.db_session.add(self) Database.db_session.commit() @@ -356,11 +357,11 @@ def add(self): Database.db_session.rollback() if isinstance(e, IntegrityError): raise self._decode(e) from e - raise ServerConfigSqlError("adding", self.key, str(e)) from e + raise ServerSettingSqlError("adding", self.key, str(e)) from e def update(self): """Update the database row with the modified version of the - ServerConfig object. + ServerSetting object. """ try: Database.db_session.commit() @@ -368,4 +369,4 @@ def update(self): Database.db_session.rollback() if isinstance(e, IntegrityError): raise self._decode(e) from e - raise ServerConfigSqlError("updating", self.key, str(e)) from e + raise ServerSettingSqlError("updating", self.key, str(e)) from e diff --git a/lib/pbench/server/database/models/template.py b/lib/pbench/server/database/models/templates.py similarity index 99% rename from lib/pbench/server/database/models/template.py rename to lib/pbench/server/database/models/templates.py index bd530eee4b..f85125ff42 100644 --- a/lib/pbench/server/database/models/template.py +++ b/lib/pbench/server/database/models/templates.py @@ -91,6 +91,7 @@ class Template(Database.Base): version The template version metadata """ + # Table name is plural so that SQL statements read easier. __tablename__ = "templates" id = Column(Integer, primary_key=True, autoincrement=True) diff --git a/lib/pbench/server/database/models/users.py b/lib/pbench/server/database/models/users.py index 8a4db8eabb..e18183b6dc 100644 --- a/lib/pbench/server/database/models/users.py +++ b/lib/pbench/server/database/models/users.py @@ -18,6 +18,7 @@ class Roles(enum.Enum): class User(Database.Base): """User Model for storing user related details.""" + # Table name is plural so that SQL statements read easier. __tablename__ = "users" id = Column(Integer, primary_key=True, autoincrement=True) @@ -28,7 +29,7 @@ class User(Database.Base): registered_on = Column(DateTime, nullable=False, default=datetime.datetime.now()) email = Column(String(255), unique=True, nullable=False) role = Column(Enum(Roles), unique=False, nullable=True) - auth_tokens = relationship("ActiveTokens", backref="users") + auth_tokens = relationship("AuthToken", backref="users") def __str__(self): return f"User, id: {self.id}, username: {self.username}" @@ -105,7 +106,7 @@ def update(self, **kwargs): """Update the current user object with given keyword arguments.""" try: for key, value in kwargs.items(): - if key == "auth_tokens": + if key == "auth_token": # Insert the auth token self.auth_tokens.append(value) Database.db_session.add(value) diff --git a/lib/pbench/server/templates.py b/lib/pbench/server/templates.py index 8387be42bc..8f080126a2 100644 --- a/lib/pbench/server/templates.py +++ b/lib/pbench/server/templates.py @@ -19,7 +19,7 @@ ) from pbench.server import JSONOBJECT, OperationCode, tstos from pbench.server.database.models.audit import Audit, AuditStatus, AuditType -from pbench.server.database.models.template import ( +from pbench.server.database.models.templates import ( Template, TemplateDuplicate, TemplateNotFound, diff --git a/lib/pbench/test/unit/server/conftest.py b/lib/pbench/test/unit/server/conftest.py index 0a17787f15..593f82c94d 100644 --- a/lib/pbench/test/unit/server/conftest.py +++ b/lib/pbench/test/unit/server/conftest.py @@ -26,7 +26,7 @@ from pbench.server.database import init_db from pbench.server.database.database import Database from pbench.server.database.models.datasets import Dataset, Metadata, States -from pbench.server.database.models.template import Template +from pbench.server.database.models.templates import Template from pbench.server.database.models.users import User from pbench.test import on_disk_config from pbench.test.unit.server.headertypes import HeaderTypes diff --git a/lib/pbench/test/unit/server/database/test_server_config_db.py b/lib/pbench/test/unit/server/database/test_server_settings.py similarity index 61% rename from lib/pbench/test/unit/server/database/test_server_config_db.py rename to lib/pbench/test/unit/server/database/test_server_settings.py index a91012b891..7323bb4137 100644 --- a/lib/pbench/test/unit/server/database/test_server_config_db.py +++ b/lib/pbench/test/unit/server/database/test_server_settings.py @@ -2,18 +2,18 @@ from sqlalchemy.exc import IntegrityError from pbench.server.database.database import Database -from pbench.server.database.models.server_config import ( - ServerConfig, - ServerConfigBadKey, - ServerConfigBadValue, - ServerConfigDuplicate, - ServerConfigMissingKey, - ServerConfigNullKey, +from pbench.server.database.models.server_settings import ( + ServerSetting, + ServerSettingBadKey, + ServerSettingBadValue, + ServerSettingDuplicate, + ServerSettingMissingKey, + ServerSettingNullKey, ) from pbench.test.unit.server.database import FakeDBOrig, FakeRow, FakeSession -class TestServerConfig: +class TestServerSetting: session = None @pytest.fixture(autouse=True, scope="function") @@ -25,125 +25,122 @@ def fake_db(self, monkeypatch, server_config): server configuration object directly on the Database.Base (normally done during DB initialization) because that can't be monkeypatched. """ - self.session = FakeSession(ServerConfig) + self.session = FakeSession(ServerSetting) monkeypatch.setattr(Database, "db_session", self.session) Database.Base.config = server_config yield def test_bad_key(self): - """Test server config parameter key validation""" - with pytest.raises(ServerConfigBadKey) as exc: - ServerConfig.create(key="not-a-key", value="no-no") - assert str(exc.value) == "Configuration key 'not-a-key' is unknown" + """Test server setting key validation""" + with pytest.raises(ServerSettingBadKey) as exc: + ServerSetting.create(key="not-a-key", value="no-no") + assert str(exc.value) == "Server setting key 'not-a-key' is unknown" def test_construct(self): - """Test server config parameter constructor""" - config = ServerConfig(key="dataset-lifetime", value="2") - assert config.key == "dataset-lifetime" - assert config.value == "2" - assert str(config) == "dataset-lifetime: '2'" + """Test server setting constructor""" + setting = ServerSetting(key="dataset-lifetime", value="2") + assert setting.key == "dataset-lifetime" + assert setting.value == "2" + assert str(setting) == "dataset-lifetime: '2'" self.session.check_session() def test_create(self): - """Test server config parameter creation""" - config = ServerConfig.create(key="dataset-lifetime", value="2") - assert config.key == "dataset-lifetime" - assert config.value == "2" - assert str(config) == "dataset-lifetime: '2'" + """Test server setting creation""" + setting = ServerSetting.create(key="dataset-lifetime", value="2") + assert setting.key == "dataset-lifetime" + assert setting.value == "2" + assert str(setting) == "dataset-lifetime: '2'" self.session.check_session( committed=[ - FakeRow(cls=ServerConfig, id=1, key="dataset-lifetime", value="2") + FakeRow(cls=ServerSetting, id=1, key="dataset-lifetime", value="2") ] ) def test_construct_duplicate(self): - """Test server config parameter constructor""" - ServerConfig.create(key="dataset-lifetime", value=1) + """Test server setting constructor""" + ServerSetting.create(key="dataset-lifetime", value=1) self.session.check_session( committed=[ - FakeRow(cls=ServerConfig, id=1, key="dataset-lifetime", value="1") + FakeRow(cls=ServerSetting, id=1, key="dataset-lifetime", value="1") ] ) - with pytest.raises(ServerConfigDuplicate) as e: + with pytest.raises(ServerSettingDuplicate) as e: self.session.raise_on_commit = IntegrityError( statement="", params="", orig=FakeDBOrig("UNIQUE constraint") ) - ServerConfig.create(key="dataset-lifetime", value=2) + ServerSetting.create(key="dataset-lifetime", value=2) assert str(e).find("dataset-lifetime") != -1 self.session.check_session( committed=[ - FakeRow(cls=ServerConfig, id=1, key="dataset-lifetime", value="1") + FakeRow(cls=ServerSetting, id=1, key="dataset-lifetime", value="1") ], rolledback=1, ) def test_construct_null(self): - """Test server config parameter constructor""" - with pytest.raises(ServerConfigNullKey): + """Test server setting constructor""" + with pytest.raises(ServerSettingNullKey): self.session.raise_on_commit = IntegrityError( statement="", params="", orig=FakeDBOrig("NOT NULL constraint") ) - ServerConfig.create(key="dataset-lifetime", value=2) + ServerSetting.create(key="dataset-lifetime", value=2) self.session.check_session( rolledback=1, ) def test_get(self): """Test that we can retrieve what we set""" - config = ServerConfig.create(key="dataset-lifetime", value="2") - result = ServerConfig.get(key="dataset-lifetime") - assert config == result + setting = ServerSetting.create(key="dataset-lifetime", value="2") + result = ServerSetting.get(key="dataset-lifetime") + assert setting == result self.session.check_session( committed=[ - FakeRow(cls=ServerConfig, id=1, key="dataset-lifetime", value="2") + FakeRow(cls=ServerSetting, id=1, key="dataset-lifetime", value="2") ], queries=1, filters=["key=dataset-lifetime"], ) def test_update(self): - """Test that we can update an existing configuration setting""" - config = ServerConfig.create(key="dataset-lifetime", value="2") + """Test that we can update an existing server setting""" + setting = ServerSetting.create(key="dataset-lifetime", value="2") self.session.check_session( committed=[ - FakeRow(cls=ServerConfig, id=1, key="dataset-lifetime", value="2") + FakeRow(cls=ServerSetting, id=1, key="dataset-lifetime", value="2") ] ) - config.value = "5" - config.update() + setting.value = "5" + setting.update() self.session.check_session( committed=[ - FakeRow(cls=ServerConfig, id=1, key="dataset-lifetime", value="5") + FakeRow(cls=ServerSetting, id=1, key="dataset-lifetime", value="5") ] ) - result = ServerConfig.get(key="dataset-lifetime") - assert config == result + result = ServerSetting.get(key="dataset-lifetime") + assert setting == result self.session.check_session( committed=[ - FakeRow(cls=ServerConfig, id=1, key="dataset-lifetime", value="5") + FakeRow(cls=ServerSetting, id=1, key="dataset-lifetime", value="5") ], queries=1, filters=["key=dataset-lifetime"], ) def test_set(self): - """ - Test that we can use set both to create and update configuration - settings - """ - config = ServerConfig.set(key="dataset-lifetime", value="40 days") - result = ServerConfig.get(key="dataset-lifetime") - assert config == result - assert config.value == "40" - ServerConfig.set(key="dataset-lifetime", value=120) - result = ServerConfig.get(key="dataset-lifetime") + """Test that we can use set to both create and update server settings""" + setting = ServerSetting.set(key="dataset-lifetime", value="40 days") + result = ServerSetting.get(key="dataset-lifetime") + assert setting == result + assert setting.value == "40" + ServerSetting.set(key="dataset-lifetime", value=120) + result = ServerSetting.get(key="dataset-lifetime") assert result.value == "120" # NOTE: each `get` does a query, plus each `set` checks whether the # key already exists: 2 get + 2 set == 4 queries self.session.check_session( committed=[ - FakeRow(cls=ServerConfig, id=1, key="dataset-lifetime", value="120") + FakeRow(cls=ServerSetting, id=1, key="dataset-lifetime", value="120") ], queries=4, filters=[ @@ -156,15 +153,15 @@ def test_set(self): def test_missing(self): """Check that 'create' complains about a missing key""" - with pytest.raises(ServerConfigMissingKey): - ServerConfig.create(key=None, value=None) + with pytest.raises(ServerSettingMissingKey): + ServerSetting.create(key=None, value=None) def test_get_all_default(self): """ Test get_all when no values have been set. It should return all the defined keys, but with None values. """ - assert ServerConfig.get_all() == { + assert ServerSetting.get_all() == { "dataset-lifetime": "3650", "server-state": {"status": "enabled"}, "server-banner": None, @@ -174,10 +171,10 @@ def test_get_all(self): """ Set values and make sure they're all reported correctly """ - c1 = ServerConfig.create(key="dataset-lifetime", value="2") - c2 = ServerConfig.create(key="server-state", value={"status": "enabled"}) - c3 = ServerConfig.create(key="server-banner", value={"message": "Mine"}) - assert ServerConfig.get_all() == { + c1 = ServerSetting.create(key="dataset-lifetime", value="2") + c2 = ServerSetting.create(key="server-state", value={"status": "enabled"}) + c3 = ServerSetting.create(key="server-banner", value={"message": "Mine"}) + assert ServerSetting.get_all() == { "dataset-lifetime": "2", "server-state": {"status": "enabled"}, "server-banner": {"message": "Mine"}, @@ -204,8 +201,8 @@ def test_lifetimes(self, value, expected): """ Test some lifetimes that should be OK """ - config = ServerConfig.create(key="dataset-lifetime", value=value) - assert config.value == expected + setting = ServerSetting.create(key="dataset-lifetime", value=value) + assert setting.value == expected @pytest.mark.parametrize( "value", @@ -225,8 +222,8 @@ def test_bad_lifetimes(self, value): """ Test some lifetime values that aren't legal """ - with pytest.raises(ServerConfigBadValue) as exc: - ServerConfig.create(key="dataset-lifetime", value=value) + with pytest.raises(ServerSettingBadValue) as exc: + ServerSetting.create(key="dataset-lifetime", value=value) assert exc.value.value == value @pytest.mark.parametrize( @@ -262,9 +259,9 @@ def test_bad_lifetimes(self, value): ) def test_create_state_and_banner(self, key, value): """Test server state and banner setting""" - config = ServerConfig.create(key=key, value=value) - assert config.key == key - assert config.value == value + setting = ServerSetting.create(key=key, value=value) + assert setting.key == key + assert setting.value == value @pytest.mark.parametrize( "value", @@ -285,8 +282,8 @@ def test_bad_state(self, value): A "server-state" value must include at least "status" and if the value isn't "enabled" must also contain "message" """ - with pytest.raises(ServerConfigBadValue) as exc: - ServerConfig.create(key="server-state", value=value) + with pytest.raises(ServerSettingBadValue) as exc: + ServerSetting.create(key="server-state", value=value) assert exc.value.value == value @pytest.mark.parametrize("value", [1, True, "yes", ["a", "b"], {"banner": "xyzzy"}]) @@ -294,6 +291,6 @@ def test_bad_banner(self, value): """ A "server-banner" value must include at least "message" """ - with pytest.raises(ServerConfigBadValue) as exc: - ServerConfig.create(key="server-banner", value=value) + with pytest.raises(ServerSettingBadValue) as exc: + ServerSetting.create(key="server-banner", value=value) assert exc.value.value == value diff --git a/lib/pbench/test/unit/server/database/test_template_db.py b/lib/pbench/test/unit/server/database/test_template_db.py index bd0ce0d80f..366d713b9a 100644 --- a/lib/pbench/test/unit/server/database/test_template_db.py +++ b/lib/pbench/test/unit/server/database/test_template_db.py @@ -2,7 +2,7 @@ import pytest -from pbench.server.database.models.template import ( +from pbench.server.database.models.templates import ( Template, TemplateDuplicate, TemplateFileMissing, diff --git a/lib/pbench/test/unit/server/test_api_base.py b/lib/pbench/test/unit/server/test_api_base.py index f9ed3a074f..4314dbf0f5 100644 --- a/lib/pbench/test/unit/server/test_api_base.py +++ b/lib/pbench/test/unit/server/test_api_base.py @@ -13,7 +13,7 @@ ApiSchema, ) from pbench.server.auth.auth import Auth -from pbench.server.database.models.server_config import ServerConfig +from pbench.server.database.models.server_settings import ServerSetting class OnlyGet(ApiBase): @@ -125,7 +125,7 @@ def test_method_validation( def mock_get_disabled(readonly: bool = False) -> Optional[JSONOBJECT]: return None - monkeypatch.setattr(ServerConfig, "get_disabled", mock_get_disabled) + monkeypatch.setattr(ServerSetting, "get_disabled", mock_get_disabled) # Verify all allowed APIs response = client.get("/api/v1/onlyget") diff --git a/lib/pbench/test/unit/server/test_endpoint_configure.py b/lib/pbench/test/unit/server/test_endpoint_configure.py index d2ed5a643f..032ff3aa73 100644 --- a/lib/pbench/test/unit/server/test_endpoint_configure.py +++ b/lib/pbench/test/unit/server/test_endpoint_configure.py @@ -68,7 +68,7 @@ def check_config(self, client, server_config, host, my_headers={}): "logout": f"{uri}/logout", "register": f"{uri}/register", "server_audit": f"{uri}/server/audit", - "server_configuration": f"{uri}/server/configuration", + "server_settings": f"{uri}/server/settings", "upload": f"{uri}/upload", "user": f"{uri}/user", }, @@ -132,8 +132,8 @@ def check_config(self, client, server_config, host, my_headers={}): "logout": {"template": f"{uri}/logout", "params": {}}, "register": {"template": f"{uri}/register", "params": {}}, "server_audit": {"template": f"{uri}/server/audit", "params": {}}, - "server_configuration": { - "template": f"{uri}/server/configuration/{{key}}", + "server_settings": { + "template": f"{uri}/server/settings/{{key}}", "params": {"key": {"type": "string"}}, }, "upload": { diff --git a/lib/pbench/test/unit/server/test_server_configuration.py b/lib/pbench/test/unit/server/test_server_settings.py similarity index 94% rename from lib/pbench/test/unit/server/test_server_configuration.py rename to lib/pbench/test/unit/server/test_server_settings.py index b46c6e7f9c..0141646644 100644 --- a/lib/pbench/test/unit/server/test_server_configuration.py +++ b/lib/pbench/test/unit/server/test_server_settings.py @@ -5,11 +5,11 @@ from pbench.server import OperationCode from pbench.server.api.resources import APIAbort, ApiParams -from pbench.server.api.resources.server_configuration import ServerConfiguration +from pbench.server.api.resources.server_settings import ServerSettings from pbench.server.database.models.audit import Audit, AuditStatus, AuditType -class TestServerConfiguration: +class TestServerSettings: @pytest.fixture() def query_get(self, client, server_config): """ @@ -25,7 +25,7 @@ def query_api( key: str, expected_status: HTTPStatus = HTTPStatus.OK ) -> requests.Response: k = "" if key is None else f"/{key}" - response = client.get(f"{server_config.rest_uri}/server/configuration{k}") + response = client.get(f"{server_config.rest_uri}/server/settings{k}") assert response.status_code == expected_status return response @@ -51,7 +51,7 @@ def query_api( ) -> requests.Response: k = f"/{key}" if key else "" response = client.put( - f"{server_config.rest_uri}/server/configuration{k}", + f"{server_config.rest_uri}/server/settings{k}", headers={"authorization": f"bearer {token}"}, **kwargs, ) @@ -74,8 +74,8 @@ def test_get1(self, query_get): def test_get_all(self, query_get, key): """ We use a trailing-slash-insensitive URI mapping so that both - /server/configuration and /server/configuration/ should be mapped to - "get all"; test that both paths work. + /server/settings and /server/settings/ should be mapped to "get all"; + test that both paths work. """ response = query_get(key) assert response.json == { @@ -90,7 +90,7 @@ def test_put_bad_uri_key(self, server_config): handling of a condition that ought to be impossible through Flask routing. """ - put = ServerConfiguration(server_config) + put = ServerSettings(server_config) with pytest.raises(APIAbort, match="Missing parameter 'key'"): put._put_key(ApiParams(uri={"plugh": "xyzzy", "foo": "bar"}), context=None) @@ -103,7 +103,7 @@ def test_put_missing_value(self, query_put): ) assert ( response.json.get("message") - == "No value found for key system configuration key 'dataset-lifetime'" + == "No value found for server settings key 'dataset-lifetime'" ) def test_put_bad_key(self, query_put): @@ -119,7 +119,7 @@ def test_put_bad_keys(self, query_put): expected_status=HTTPStatus.BAD_REQUEST, ) assert response.json == { - "message": "Unrecognized configuration parameters ['fookey'] specified: valid parameters are ['dataset-lifetime', 'server-banner', 'server-state']" + "message": "Unrecognized server setting ['fookey'] specified: valid settings are ['dataset-lifetime', 'server-banner', 'server-state']" } @pytest.mark.parametrize( @@ -136,7 +136,7 @@ def test_bad_value(self, query_put, key, value): key=key, expected_status=HTTPStatus.BAD_REQUEST, json={"value": value} ) assert ( - f"Unsupported value for configuration key '{key}'" + f"Unsupported value for server settings key '{key}'" in response.json["message"] ) diff --git a/lib/pbench/test/unit/server/test_user_auth.py b/lib/pbench/test/unit/server/test_user_auth.py index e839744cbe..3b1ccd1203 100644 --- a/lib/pbench/test/unit/server/test_user_auth.py +++ b/lib/pbench/test/unit/server/test_user_auth.py @@ -3,7 +3,7 @@ import time from pbench.server.database.database import Database -from pbench.server.database.models.active_tokens import ActiveTokens +from pbench.server.database.models.auth_tokens import AuthToken from pbench.server.database.models.users import User from pbench.test.unit.server.conftest import admin_username @@ -373,8 +373,8 @@ def test_valid_logout(client, server_config): assert response.status_code == HTTPStatus.OK # Check if the token has been successfully removed from the database assert ( - not Database.db_session.query(ActiveTokens) - .filter_by(token=data_login["auth_token"]) + not Database.db_session.query(AuthToken) + .filter_by(auth_token=data_login["auth_token"]) .first() ) assert response.status_code == HTTPStatus.OK