From 05b7fce917f90712a33f624458186ade0dd42389 Mon Sep 17 00:00:00 2001 From: James Robinson Date: Tue, 24 Sep 2024 12:31:52 +0100 Subject: [PATCH 01/39] :sparkles: Add pep8-naming checks --- guacamole_user_sync/ldap/ldap_client.py | 10 ++--- guacamole_user_sync/models/__init__.py | 6 +-- guacamole_user_sync/models/exceptions.py | 4 +- guacamole_user_sync/postgresql/orm.py | 4 +- .../postgresql/postgresql_client.py | 30 +++++++------- pyproject.toml | 4 +- synchronise.py | 6 +-- tests/conftest.py | 24 +++++------ tests/mocks.py | 4 +- tests/test_ldap.py | 10 ++--- tests/test_postgresql.py | 40 +++++++++---------- 11 files changed, 72 insertions(+), 70 deletions(-) diff --git a/guacamole_user_sync/ldap/ldap_client.py b/guacamole_user_sync/ldap/ldap_client.py index c233c14..b9f1563 100644 --- a/guacamole_user_sync/ldap/ldap_client.py +++ b/guacamole_user_sync/ldap/ldap_client.py @@ -5,7 +5,7 @@ from ldap.ldapobject import LDAPObject from guacamole_user_sync.models import ( - LDAPException, + LDAPError, LDAPGroup, LDAPQuery, LDAPSearchResult, @@ -37,7 +37,7 @@ def connect(self) -> LDAPObject: self.cnxn.simple_bind_s(self.bind_dn, self.bind_password) except ldap.INVALID_CREDENTIALS as exc: logger.warning("Connection credentials were incorrect.") - raise LDAPException from exc + raise LDAPError from exc return self.cnxn def search_groups(self, query: LDAPQuery) -> list[LDAPGroup]: @@ -94,10 +94,10 @@ def search(self, query: LDAPQuery) -> LDAPSearchResult: return results except ldap.NO_SUCH_OBJECT as exc: logger.warning("Server returned no results.") - raise LDAPException from exc + raise LDAPError from exc except ldap.SERVER_DOWN as exc: logger.warning("Server could not be reached.") - raise LDAPException from exc + raise LDAPError from exc except ldap.SIZELIMIT_EXCEEDED as exc: logger.warning("Server-side size limit exceeded.") - raise LDAPException from exc + raise LDAPError from exc diff --git a/guacamole_user_sync/models/__init__.py b/guacamole_user_sync/models/__init__.py index e0ca516..c20a684 100644 --- a/guacamole_user_sync/models/__init__.py +++ b/guacamole_user_sync/models/__init__.py @@ -1,14 +1,14 @@ -from .exceptions import LDAPException, PostgreSQLException +from .exceptions import LDAPError, PostgreSQLError from .ldap_objects import LDAPGroup, LDAPUser from .ldap_query import LDAPQuery LDAPSearchResult = list[tuple[int, tuple[str, dict[str, list[bytes]]]]] __all__ = [ - "LDAPException", + "LDAPError", "LDAPGroup", "LDAPQuery", "LDAPSearchResult", "LDAPUser", - "PostgreSQLException", + "PostgreSQLError", ] diff --git a/guacamole_user_sync/models/exceptions.py b/guacamole_user_sync/models/exceptions.py index ac1da8f..1114159 100644 --- a/guacamole_user_sync/models/exceptions.py +++ b/guacamole_user_sync/models/exceptions.py @@ -1,6 +1,6 @@ -class LDAPException(Exception): +class LDAPError(Exception): pass -class PostgreSQLException(Exception): +class PostgreSQLError(Exception): pass diff --git a/guacamole_user_sync/postgresql/orm.py b/guacamole_user_sync/postgresql/orm.py index dfa2c58..b3b46c4 100644 --- a/guacamole_user_sync/postgresql/orm.py +++ b/guacamole_user_sync/postgresql/orm.py @@ -10,7 +10,7 @@ ) -class guacamole_entity_type(enum.Enum): +class GuacamoleEntityType(enum.Enum): USER = "USER" USER_GROUP = "USER_GROUP" @@ -24,7 +24,7 @@ class GuacamoleEntity(GuacamoleBase): entity_id: Mapped[int] = mapped_column(Integer, primary_key=True) name: Mapped[str] = mapped_column(String(128)) - type: Mapped[guacamole_entity_type] = mapped_column(Enum(guacamole_entity_type)) + type: Mapped[GuacamoleEntityType] = mapped_column(Enum(GuacamoleEntityType)) class GuacamoleUser(GuacamoleBase): diff --git a/guacamole_user_sync/postgresql/postgresql_client.py b/guacamole_user_sync/postgresql/postgresql_client.py index 7b17803..dc72ec2 100644 --- a/guacamole_user_sync/postgresql/postgresql_client.py +++ b/guacamole_user_sync/postgresql/postgresql_client.py @@ -7,15 +7,15 @@ from guacamole_user_sync.models import ( LDAPGroup, LDAPUser, - PostgreSQLException, + PostgreSQLError, ) from .orm import ( GuacamoleEntity, + GuacamoleEntityType, GuacamoleUser, GuacamoleUserGroup, GuacamoleUserGroupMember, - guacamole_entity_type, ) from .postgresql_backend import PostgreSQLBackend from .sql import GuacamoleSchema, SchemaVersion @@ -58,7 +58,7 @@ def assign_users_to_groups( for item in self.backend.query( GuacamoleEntity, name=group.name, - type=guacamole_entity_type.USER_GROUP, + type=GuacamoleEntityType.USER_GROUP, ) ][0] user_group_id = [ @@ -89,7 +89,7 @@ def assign_users_to_groups( for item in self.backend.query( GuacamoleEntity, name=user.name, - type=guacamole_entity_type.USER, + type=GuacamoleEntityType.USER, ) ][0] logger.debug( @@ -115,7 +115,7 @@ def ensure_schema(self, schema_version: SchemaVersion) -> None: self.backend.execute_commands(GuacamoleSchema.commands(schema_version)) except OperationalError as exc: logger.warning("Unable to connect to the PostgreSQL server.") - raise PostgreSQLException("Unable to ensure PostgreSQL schema.") from exc + raise PostgreSQLError("Unable to ensure PostgreSQL schema.") from exc def update(self, *, groups: list[LDAPGroup], users: list[LDAPUser]) -> None: """Update the relevant tables to match lists of LDAP users and groups""" @@ -133,7 +133,7 @@ def update_groups(self, groups: list[LDAPGroup]) -> None: current_group_names = [ item.name for item in self.backend.query( - GuacamoleEntity, type=guacamole_entity_type.USER_GROUP + GuacamoleEntity, type=GuacamoleEntityType.USER_GROUP ) ] # Add groups @@ -148,7 +148,7 @@ def update_groups(self, groups: list[LDAPGroup]) -> None: logger.debug(f"... {len(group_names_to_add)} group(s) will be added") self.backend.add_all( [ - GuacamoleEntity(name=group_name, type=guacamole_entity_type.USER_GROUP) + GuacamoleEntity(name=group_name, type=GuacamoleEntityType.USER_GROUP) for group_name in group_names_to_add ] ) @@ -163,7 +163,7 @@ def update_groups(self, groups: list[LDAPGroup]) -> None: self.backend.delete( GuacamoleEntity, GuacamoleEntity.name == group_name, - GuacamoleEntity.type == guacamole_entity_type.USER_GROUP, + GuacamoleEntity.type == GuacamoleEntityType.USER_GROUP, ) def update_group_entities(self) -> None: @@ -178,7 +178,7 @@ def update_group_entities(self) -> None: new_group_entity_ids = [ group.entity_id for group in self.backend.query( - GuacamoleEntity, type=guacamole_entity_type.USER_GROUP + GuacamoleEntity, type=GuacamoleEntityType.USER_GROUP ) if group.entity_id not in current_user_group_entity_ids ] @@ -197,7 +197,7 @@ def update_group_entities(self) -> None: valid_entity_ids = [ group.entity_id for group in self.backend.query( - GuacamoleEntity, type=guacamole_entity_type.USER_GROUP + GuacamoleEntity, type=GuacamoleEntityType.USER_GROUP ) ] logger.debug(f"There are {len(valid_entity_ids)} valid user group entit(y|ies)") @@ -213,7 +213,7 @@ def update_users(self, users: list[LDAPUser]) -> None: current_usernames = [ user.name for user in self.backend.query( - GuacamoleEntity, type=guacamole_entity_type.USER + GuacamoleEntity, type=GuacamoleEntityType.USER ) ] # Add users @@ -226,7 +226,7 @@ def update_users(self, users: list[LDAPUser]) -> None: logger.debug(f"... {len(usernames_to_add)} user(s) will be added") self.backend.add_all( [ - GuacamoleEntity(name=username, type=guacamole_entity_type.USER) + GuacamoleEntity(name=username, type=GuacamoleEntityType.USER) for username in usernames_to_add ] ) @@ -241,7 +241,7 @@ def update_users(self, users: list[LDAPUser]) -> None: self.backend.delete( GuacamoleEntity, GuacamoleEntity.name == username, - GuacamoleEntity.type == guacamole_entity_type.USER, + GuacamoleEntity.type == GuacamoleEntityType.USER, ) def update_user_entities(self, users: list[LDAPUser]) -> None: @@ -256,7 +256,7 @@ def update_user_entities(self, users: list[LDAPUser]) -> None: new_user_tuples: list[tuple[int, LDAPUser]] = [ (user.entity_id, [u for u in users if u.name == user.name][0]) for user in self.backend.query( - GuacamoleEntity, type=guacamole_entity_type.USER + GuacamoleEntity, type=GuacamoleEntityType.USER ) if user.entity_id not in current_user_entity_ids ] @@ -279,7 +279,7 @@ def update_user_entities(self, users: list[LDAPUser]) -> None: valid_entity_ids = [ user.entity_id for user in self.backend.query( - GuacamoleEntity, type=guacamole_entity_type.USER + GuacamoleEntity, type=GuacamoleEntityType.USER ) ] logger.debug(f"There are {len(valid_entity_ids)} valid user entit(y|ies)") diff --git a/pyproject.toml b/pyproject.toml index f7bf290..677ee05 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -101,9 +101,11 @@ ignore_missing_imports = true [tool.ruff.lint] select = [ - # See https://beta.ruff.rs/docs/rules/ + "A", # flake8-builtins + "C90", # McCabe complexity "E", # pycodestyle errors "F", # pyflakes "I", # isort + "N", # pep8-naming "W", # pycodestyle warnings ] diff --git a/synchronise.py b/synchronise.py index 626e4a8..3afaa90 100644 --- a/synchronise.py +++ b/synchronise.py @@ -4,7 +4,7 @@ import time from guacamole_user_sync.ldap import LDAPClient -from guacamole_user_sync.models import LDAPException, LDAPQuery, PostgreSQLException +from guacamole_user_sync.models import LDAPError, LDAPQuery, PostgreSQLError from guacamole_user_sync.postgresql import PostgreSQLClient, SchemaVersion @@ -76,14 +76,14 @@ def synchronise( try: ldap_groups = ldap_client.search_groups(ldap_group_query) ldap_users = ldap_client.search_users(ldap_user_query) - except LDAPException: + except LDAPError: logger.warning("LDAP server query failed") return try: postgresql_client.ensure_schema(SchemaVersion.v1_5_5) postgresql_client.update(groups=ldap_groups, users=ldap_users) - except PostgreSQLException: + except PostgreSQLError: logger.warning("PostgreSQL update failed") return diff --git a/tests/conftest.py b/tests/conftest.py index 8a339f1..1b39502 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -5,9 +5,9 @@ from guacamole_user_sync.models import LDAPGroup, LDAPQuery, LDAPSearchResult, LDAPUser from guacamole_user_sync.postgresql.orm import ( GuacamoleEntity, + GuacamoleEntityType, GuacamoleUser, GuacamoleUserGroup, - guacamole_entity_type, ) @@ -138,42 +138,42 @@ def ldap_response_users_fixture() -> LDAPSearchResult: @pytest.fixture -def postgresql_model_guacamoleentity_USER_GROUP_fixture() -> list[GuacamoleEntity]: +def postgresql_model_guacamoleentity_user_group_fixture() -> list[GuacamoleEntity]: return [ GuacamoleEntity( - entity_id=1, name="defendants", type=guacamole_entity_type.USER_GROUP + entity_id=1, name="defendants", type=GuacamoleEntityType.USER_GROUP ), GuacamoleEntity( - entity_id=2, name="everyone", type=guacamole_entity_type.USER_GROUP + entity_id=2, name="everyone", type=GuacamoleEntityType.USER_GROUP ), GuacamoleEntity( - entity_id=3, name="plaintiffs", type=guacamole_entity_type.USER_GROUP + entity_id=3, name="plaintiffs", type=GuacamoleEntityType.USER_GROUP ), ] @pytest.fixture -def postgresql_model_guacamoleentity_USER_fixture() -> list[GuacamoleEntity]: +def postgresql_model_guacamoleentity_user_fixture() -> list[GuacamoleEntity]: return [ GuacamoleEntity( - entity_id=4, name="aulus.agerius@rome.la", type=guacamole_entity_type.USER + entity_id=4, name="aulus.agerius@rome.la", type=GuacamoleEntityType.USER ), GuacamoleEntity( entity_id=5, name="numerius.negidius@rome.la", - type=guacamole_entity_type.USER, + type=GuacamoleEntityType.USER, ), ] @pytest.fixture def postgresql_model_guacamoleentity_fixture( - postgresql_model_guacamoleentity_USER_GROUP_fixture: list[GuacamoleEntity], - postgresql_model_guacamoleentity_USER_fixture: list[GuacamoleEntity], + postgresql_model_guacamoleentity_user_group_fixture: list[GuacamoleEntity], + postgresql_model_guacamoleentity_user_fixture: list[GuacamoleEntity], ) -> list[GuacamoleEntity]: return ( - postgresql_model_guacamoleentity_USER_GROUP_fixture - + postgresql_model_guacamoleentity_USER_fixture + postgresql_model_guacamoleentity_user_group_fixture + + postgresql_model_guacamoleentity_user_fixture ) diff --git a/tests/mocks.py b/tests/mocks.py index 7721ce7..d79fd16 100644 --- a/tests/mocks.py +++ b/tests/mocks.py @@ -26,10 +26,10 @@ def __init__( self.allResults = results self.partial = partial - def startSearch(self, *args: Any, **kwargs: Any) -> None: + def startSearch(self, *args: Any, **kwargs: Any) -> None: # noqa: N802 pass - def processResults(self, *args: Any, **kwargs: Any) -> bool: + def processResults(self, *args: Any, **kwargs: Any) -> bool: # noqa: N802 return self.partial diff --git a/tests/test_ldap.py b/tests/test_ldap.py index 61371d5..7881af0 100644 --- a/tests/test_ldap.py +++ b/tests/test_ldap.py @@ -7,7 +7,7 @@ from guacamole_user_sync.ldap import LDAPClient from guacamole_user_sync.models import ( - LDAPException, + LDAPError, LDAPGroup, LDAPQuery, LDAPSearchResult, @@ -60,7 +60,7 @@ def mock_initialize(uri: str) -> MockLDAPObject: client = LDAPClient( hostname="test-host", bind_dn="bind-dn", bind_password="incorrect-password" ) - with pytest.raises(LDAPException): + with pytest.raises(LDAPError): client.connect() assert "Connection credentials were incorrect." in caplog.text @@ -72,7 +72,7 @@ def mock_raise_server_down(*args: Any) -> None: ldap.asyncsearch.List, "startSearch", mock_raise_server_down ) client = LDAPClient(hostname="test-host") - with pytest.raises(LDAPException): + with pytest.raises(LDAPError): client.search(query=LDAPQuery(base_dn="", filter="", id_attr="")) assert "Server could not be reached." in caplog.text @@ -86,7 +86,7 @@ def mock_raise_sizelimit_exceeded(*args: Any) -> None: ldap.asyncsearch.List, "startSearch", mock_raise_sizelimit_exceeded ) client = LDAPClient(hostname="test-host") - with pytest.raises(LDAPException): + with pytest.raises(LDAPError): client.search(query=LDAPQuery(base_dn="", filter="", id_attr="")) assert "Server-side size limit exceeded." in caplog.text @@ -117,7 +117,7 @@ def mock_raise_no_results(*args: Any) -> None: monkeypatch.setattr(ldap.asyncsearch.List, "startSearch", mock_raise_no_results) client = LDAPClient(hostname="test-host") - with pytest.raises(LDAPException): + with pytest.raises(LDAPError): client.search(query=LDAPQuery(base_dn="", filter="", id_attr="")) assert "Server returned no results." in caplog.text diff --git a/tests/test_postgresql.py b/tests/test_postgresql.py index bb6eb9f..63b979d 100644 --- a/tests/test_postgresql.py +++ b/tests/test_postgresql.py @@ -11,13 +11,13 @@ from sqlalchemy.pool.impl import QueuePool from sqlalchemy.sql.elements import BinaryExpression -from guacamole_user_sync.models import LDAPGroup, LDAPUser, PostgreSQLException +from guacamole_user_sync.models import LDAPGroup, LDAPUser, PostgreSQLError from guacamole_user_sync.postgresql import PostgreSQLBackend, PostgreSQLClient from guacamole_user_sync.postgresql.orm import ( GuacamoleEntity, + GuacamoleEntityType, GuacamoleUser, GuacamoleUserGroup, - guacamole_entity_type, ) from guacamole_user_sync.postgresql.sql import SchemaVersion @@ -106,7 +106,7 @@ def test_delete_with_filter( ) -> None: backend, session = self.mock_backend() backend.delete( - GuacamoleEntity, GuacamoleEntity.type == guacamole_entity_type.USER + GuacamoleEntity, GuacamoleEntity.type == GuacamoleEntityType.USER ) # Check method calls @@ -154,7 +154,7 @@ def test_query_with_filter( self, ) -> None: backend, session = self.mock_backend() - backend.query(GuacamoleEntity, type=guacamole_entity_type.USER) + backend.query(GuacamoleEntity, type=GuacamoleEntityType.USER) # Check method calls print(session.mock_calls) @@ -169,7 +169,7 @@ def test_query_with_filter( filter_by_kwargs = session.filter_by.call_args.kwargs assert len(filter_by_kwargs) == 1 assert "type" in filter_by_kwargs - assert filter_by_kwargs["type"] == guacamole_entity_type.USER + assert filter_by_kwargs["type"] == GuacamoleEntityType.USER class TestPostgreSQLClient: @@ -260,14 +260,14 @@ def test_assign_users_to_groups_missing_user_entity( caplog: Any, ldap_model_groups_fixture: list[LDAPGroup], ldap_model_users_fixture: list[LDAPUser], - postgresql_model_guacamoleentity_USER_fixture: list[GuacamoleEntity], - postgresql_model_guacamoleentity_USER_GROUP_fixture: list[GuacamoleEntity], + postgresql_model_guacamoleentity_user_fixture: list[GuacamoleEntity], + postgresql_model_guacamoleentity_user_group_fixture: list[GuacamoleEntity], postgresql_model_guacamoleusergroup_fixture: list[GuacamoleUserGroup], ) -> None: # Create a mock backend mock_backend = MockPostgreSQLBackend( # type: ignore - postgresql_model_guacamoleentity_USER_fixture[1:], - postgresql_model_guacamoleentity_USER_GROUP_fixture, + postgresql_model_guacamoleentity_user_fixture[1:], + postgresql_model_guacamoleentity_user_group_fixture, postgresql_model_guacamoleusergroup_fixture, ) @@ -430,7 +430,7 @@ def execute_commands_exception(commands: Any) -> None: client = PostgreSQLClient(**self.client_kwargs) with pytest.raises( - PostgreSQLException, match="Unable to ensure PostgreSQL schema." + PostgreSQLError, match="Unable to ensure PostgreSQL schema." ): client.ensure_schema(SchemaVersion.v1_5_5) @@ -489,12 +489,12 @@ def test_update( def test_update_group_entities( self, caplog: Any, - postgresql_model_guacamoleentity_USER_GROUP_fixture: list[GuacamoleEntity], + postgresql_model_guacamoleentity_user_group_fixture: list[GuacamoleEntity], postgresql_model_guacamoleusergroup_fixture: list[GuacamoleUserGroup], ) -> None: # Create a mock backend mock_backend = MockPostgreSQLBackend( # type: ignore - postgresql_model_guacamoleentity_USER_GROUP_fixture, + postgresql_model_guacamoleentity_user_group_fixture, postgresql_model_guacamoleusergroup_fixture[0:1], ) @@ -520,16 +520,16 @@ def test_update_groups( self, caplog: Any, ldap_model_groups_fixture: list[LDAPGroup], - postgresql_model_guacamoleentity_USER_GROUP_fixture: list[GuacamoleEntity], + postgresql_model_guacamoleentity_user_group_fixture: list[GuacamoleEntity], ) -> None: # Create a mock backend mock_backend = MockPostgreSQLBackend( # type: ignore - postgresql_model_guacamoleentity_USER_GROUP_fixture[0:1], + postgresql_model_guacamoleentity_user_group_fixture[0:1], [ GuacamoleEntity( entity_id=99, name="to-be-deleted", - type=guacamole_entity_type.USER_GROUP, + type=GuacamoleEntityType.USER_GROUP, ) ], ) @@ -555,11 +555,11 @@ def test_update_user_entities( caplog: Any, ldap_model_users_fixture: list[LDAPUser], postgresql_model_guacamoleuser_fixture: list[GuacamoleUser], - postgresql_model_guacamoleentity_USER_fixture: list[GuacamoleEntity], + postgresql_model_guacamoleentity_user_fixture: list[GuacamoleEntity], ) -> None: # Create a mock backend mock_backend = MockPostgreSQLBackend( # type: ignore - postgresql_model_guacamoleentity_USER_fixture, + postgresql_model_guacamoleentity_user_fixture, postgresql_model_guacamoleuser_fixture[0:1], ) @@ -585,14 +585,14 @@ def test_update_users( self, caplog: Any, ldap_model_users_fixture: list[LDAPUser], - postgresql_model_guacamoleentity_USER_fixture: list[GuacamoleEntity], + postgresql_model_guacamoleentity_user_fixture: list[GuacamoleEntity], ) -> None: # Create a mock backend mock_backend = MockPostgreSQLBackend( # type: ignore - postgresql_model_guacamoleentity_USER_fixture[0:1], + postgresql_model_guacamoleentity_user_fixture[0:1], [ GuacamoleEntity( - entity_id=99, name="to-be-deleted", type=guacamole_entity_type.USER + entity_id=99, name="to-be-deleted", type=GuacamoleEntityType.USER ) ], ) From c1732dea2f726b92f78758c11519b8f00add20ff Mon Sep 17 00:00:00 2001 From: James Robinson Date: Tue, 24 Sep 2024 16:00:53 +0100 Subject: [PATCH 02/39] :white_check_mark: Add pydocstyle validation --- guacamole_user_sync/ldap/ldap_client.py | 2 ++ guacamole_user_sync/models/exceptions.py | 4 ++++ guacamole_user_sync/postgresql/orm.py | 12 ++++++++++++ guacamole_user_sync/postgresql/postgresql_backend.py | 2 ++ guacamole_user_sync/postgresql/postgresql_client.py | 12 +++++++----- guacamole_user_sync/postgresql/sql.py | 3 +++ pyproject.toml | 12 ++++++++++++ tests/mocks.py | 9 +++++++++ tests/test_about.py | 7 +++++-- tests/test_ldap.py | 2 ++ tests/test_postgresql.py | 4 ++++ 11 files changed, 62 insertions(+), 7 deletions(-) diff --git a/guacamole_user_sync/ldap/ldap_client.py b/guacamole_user_sync/ldap/ldap_client.py index b9f1563..9b1ad96 100644 --- a/guacamole_user_sync/ldap/ldap_client.py +++ b/guacamole_user_sync/ldap/ldap_client.py @@ -16,6 +16,8 @@ class LDAPClient: + """Client for connecting to an LDAP server.""" + def __init__( self, hostname: str, diff --git a/guacamole_user_sync/models/exceptions.py b/guacamole_user_sync/models/exceptions.py index 1114159..a56cce6 100644 --- a/guacamole_user_sync/models/exceptions.py +++ b/guacamole_user_sync/models/exceptions.py @@ -1,6 +1,10 @@ class LDAPError(Exception): + """LDAP error.""" + pass class PostgreSQLError(Exception): + """PostgreSQL error.""" + pass diff --git a/guacamole_user_sync/postgresql/orm.py b/guacamole_user_sync/postgresql/orm.py index b3b46c4..ec94a28 100644 --- a/guacamole_user_sync/postgresql/orm.py +++ b/guacamole_user_sync/postgresql/orm.py @@ -11,15 +11,21 @@ class GuacamoleEntityType(enum.Enum): + """Guacamole entity enum.""" + USER = "USER" USER_GROUP = "USER_GROUP" class GuacamoleBase(DeclarativeBase): # type:ignore + """Guacamole database base table.""" + pass class GuacamoleEntity(GuacamoleBase): + """Guacamole database GuacamoleEntity table.""" + __tablename__ = "guacamole_entity" entity_id: Mapped[int] = mapped_column(Integer, primary_key=True) @@ -28,6 +34,8 @@ class GuacamoleEntity(GuacamoleBase): class GuacamoleUser(GuacamoleBase): + """Guacamole database GuacamoleUser table.""" + __tablename__ = "guacamole_user" user_id: Mapped[int] = mapped_column(Integer, primary_key=True) @@ -39,6 +47,8 @@ class GuacamoleUser(GuacamoleBase): class GuacamoleUserGroup(GuacamoleBase): + """Guacamole database GuacamoleUserGroup table.""" + __tablename__ = "guacamole_user_group" user_group_id: Mapped[int] = mapped_column(Integer, primary_key=True) @@ -46,6 +56,8 @@ class GuacamoleUserGroup(GuacamoleBase): class GuacamoleUserGroupMember(GuacamoleBase): + """Guacamole database GuacamoleUserGroupMember table.""" + __tablename__ = "guacamole_user_group_member" user_group_id: Mapped[int] = mapped_column(Integer, primary_key=True) diff --git a/guacamole_user_sync/postgresql/postgresql_backend.py b/guacamole_user_sync/postgresql/postgresql_backend.py index 5639643..754f0e9 100644 --- a/guacamole_user_sync/postgresql/postgresql_backend.py +++ b/guacamole_user_sync/postgresql/postgresql_backend.py @@ -12,6 +12,8 @@ class PostgreSQLBackend: + """Backend for connecting to a PostgreSQL database.""" + def __init__( self, *, diff --git a/guacamole_user_sync/postgresql/postgresql_client.py b/guacamole_user_sync/postgresql/postgresql_client.py index dc72ec2..aacbf91 100644 --- a/guacamole_user_sync/postgresql/postgresql_client.py +++ b/guacamole_user_sync/postgresql/postgresql_client.py @@ -24,6 +24,8 @@ class PostgreSQLClient: + """Client for connecting to a PostgreSQL database.""" + def __init__( self, *, @@ -118,7 +120,7 @@ def ensure_schema(self, schema_version: SchemaVersion) -> None: raise PostgreSQLError("Unable to ensure PostgreSQL schema.") from exc def update(self, *, groups: list[LDAPGroup], users: list[LDAPUser]) -> None: - """Update the relevant tables to match lists of LDAP users and groups""" + """Update the relevant tables to match lists of LDAP users and groups.""" self.update_groups(groups) self.update_users(users) self.update_group_entities() @@ -126,7 +128,7 @@ def update(self, *, groups: list[LDAPGroup], users: list[LDAPUser]) -> None: self.assign_users_to_groups(groups, users) def update_groups(self, groups: list[LDAPGroup]) -> None: - """Update the entities table with desired groups""" + """Update the entities table with desired groups.""" # Set groups to desired list logger.info(f"Ensuring that {len(groups)} group(s) are registered") desired_group_names = [group.name for group in groups] @@ -167,7 +169,7 @@ def update_groups(self, groups: list[LDAPGroup]) -> None: ) def update_group_entities(self) -> None: - """Add group entities to the groups table""" + """Add group entities to the groups table.""" current_user_group_entity_ids = [ group.entity_id for group in self.backend.query(GuacamoleUserGroup) ] @@ -206,7 +208,7 @@ def update_group_entities(self) -> None: ) def update_users(self, users: list[LDAPUser]) -> None: - """Update the entities table with desired users""" + """Update the entities table with desired users.""" # Set users to desired list logger.info(f"Ensuring that {len(users)} user(s) are registered") desired_usernames = [user.name for user in users] @@ -245,7 +247,7 @@ def update_users(self, users: list[LDAPUser]) -> None: ) def update_user_entities(self, users: list[LDAPUser]) -> None: - """Add user entities to the users table""" + """Add user entities to the users table.""" current_user_entity_ids = [ user.entity_id for user in self.backend.query(GuacamoleUser) ] diff --git a/guacamole_user_sync/postgresql/sql.py b/guacamole_user_sync/postgresql/sql.py index 6523440..d0a9a54 100644 --- a/guacamole_user_sync/postgresql/sql.py +++ b/guacamole_user_sync/postgresql/sql.py @@ -10,10 +10,13 @@ class SchemaVersion(StrEnum): + """Version for Guacamole database schema.""" + v1_5_5 = "1.5.5" class GuacamoleSchema: + """Schema for Guacamole database.""" @classmethod def commands(cls, schema_version: SchemaVersion) -> list[TextClause]: diff --git a/pyproject.toml b/pyproject.toml index 677ee05..db8e26a 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -103,9 +103,21 @@ ignore_missing_imports = true select = [ "A", # flake8-builtins "C90", # McCabe complexity + "D", # pydocstyle "E", # pycodestyle errors "F", # pyflakes "I", # isort "N", # pep8-naming "W", # pycodestyle warnings ] +ignore = [ + "D100", # undocumented-public-module + "D102", # undocumented-public-method + "D103", # undocumented-public-function + "D104", # undocumented-public-package + "D105", # undocumented-magic-method + "D107", # undocumented-public-init + "D203", # one-blank-line-before-class [due to conflict with D211] + "D213", # multi-line-summary-second-line [due to conflict with D212] + "D400", # ends-in-period [due to conflict with D415] +] \ No newline at end of file diff --git a/tests/mocks.py b/tests/mocks.py index d79fd16..992c5ef 100644 --- a/tests/mocks.py +++ b/tests/mocks.py @@ -7,6 +7,8 @@ class MockLDAPObject: + """Mock LDAPObject.""" + def __init__(self, uri: str) -> None: self.uri = uri self.bind_dn = "" @@ -20,6 +22,8 @@ def simple_bind_s(self, bind_dn: str, bind_password: str) -> None: class MockAsyncSearchList: + """Mock AsyncSearchList.""" + def __init__( self, partial: bool, results: LDAPSearchResult, *args: Any, **kwargs: Any ) -> None: @@ -34,11 +38,15 @@ def processResults(self, *args: Any, **kwargs: Any) -> bool: # noqa: N802 class MockAsyncSearchListFullResults(MockAsyncSearchList): + """Mock AsyncSearchList with full results.""" + def __init__(self, results: LDAPSearchResult) -> None: super().__init__(results=results, partial=False) class MockAsyncSearchListPartialResults(MockAsyncSearchList): + """Mock AsyncSearchList with partial results.""" + def __init__(self, results: LDAPSearchResult) -> None: super().__init__(results=results, partial=True) @@ -47,6 +55,7 @@ def __init__(self, results: LDAPSearchResult) -> None: class MockPostgreSQLBackend(Generic[T]): + """Mock PostgreSQLBackend.""" def __init__(self, *data_lists: Any, **kwargs: Any) -> None: self.contents: dict[Type[T], list[T]] = {} diff --git a/tests/test_about.py b/tests/test_about.py index d3ffaa1..e87cf36 100644 --- a/tests/test_about.py +++ b/tests/test_about.py @@ -1,5 +1,8 @@ from guacamole_user_sync import version -def test_about() -> None: - assert version == "0.6.0" +class TestAbout: + """Test about.py.""" + + def test_version(self) -> None: + assert version == "0.6.0" diff --git a/tests/test_ldap.py b/tests/test_ldap.py index 7881af0..1a9fb9f 100644 --- a/tests/test_ldap.py +++ b/tests/test_ldap.py @@ -22,6 +22,8 @@ class TestLDAPClient: + """Test LDAPClient.""" + def test_constructor(self) -> None: client = LDAPClient(hostname="test-host") assert client.hostname == "test-host" diff --git a/tests/test_postgresql.py b/tests/test_postgresql.py index 63b979d..c3d18fd 100644 --- a/tests/test_postgresql.py +++ b/tests/test_postgresql.py @@ -25,6 +25,8 @@ class TestPostgreSQLBackend: + """Test PostgreSQLBackend.""" + def mock_backend( self, test_session: bool = False ) -> tuple[PostgreSQLBackend, mock.MagicMock]: @@ -173,6 +175,8 @@ def test_query_with_filter( class TestPostgreSQLClient: + """Test PostgreSQLClient.""" + client_kwargs: ClassVar[dict[str, Any]] = { "database_name": "database_name", "host_name": "host_name", From 73d3a8e70c71bc4d72d5a38f23536a7851360915 Mon Sep 17 00:00:00 2001 From: James Robinson Date: Tue, 24 Sep 2024 16:02:54 +0100 Subject: [PATCH 03/39] :white_check_mark: Add pyupgrade validation --- pyproject.toml | 1 + 1 file changed, 1 insertion(+) diff --git a/pyproject.toml b/pyproject.toml index db8e26a..0596c96 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -108,6 +108,7 @@ select = [ "F", # pyflakes "I", # isort "N", # pep8-naming + "UP", # pyupgrade "W", # pycodestyle warnings ] ignore = [ From 845dabad56ca1290cb76420f9a37105ba1b531a9 Mon Sep 17 00:00:00 2001 From: James Robinson Date: Tue, 24 Sep 2024 16:04:25 +0100 Subject: [PATCH 04/39] :white_check_mark: Add flake8-2020 validation --- guacamole_user_sync/postgresql/postgresql_backend.py | 6 +++--- guacamole_user_sync/postgresql/sql.py | 2 +- pyproject.toml | 1 + tests/mocks.py | 6 +++--- 4 files changed, 8 insertions(+), 7 deletions(-) diff --git a/guacamole_user_sync/postgresql/postgresql_backend.py b/guacamole_user_sync/postgresql/postgresql_backend.py index 754f0e9..0ebd172 100644 --- a/guacamole_user_sync/postgresql/postgresql_backend.py +++ b/guacamole_user_sync/postgresql/postgresql_backend.py @@ -1,5 +1,5 @@ import logging -from typing import Any, Type, TypeVar +from typing import Any, TypeVar from sqlalchemy import create_engine from sqlalchemy.engine import URL, Engine # type:ignore @@ -56,7 +56,7 @@ def add_all(self, items: list[T]) -> None: with session.begin(): session.add_all(items) - def delete(self, table: Type[T], *filter_args: Any) -> None: + def delete(self, table: type[T], *filter_args: Any) -> None: with self.session() as session: # type:ignore with session.begin(): if filter_args: @@ -70,7 +70,7 @@ def execute_commands(self, commands: list[TextClause]) -> None: for command in commands: session.execute(command) - def query(self, table: Type[T], **filter_kwargs: Any) -> list[T]: + def query(self, table: type[T], **filter_kwargs: Any) -> list[T]: with self.session() as session: # type:ignore with session.begin(): if filter_kwargs: diff --git a/guacamole_user_sync/postgresql/sql.py b/guacamole_user_sync/postgresql/sql.py index d0a9a54..4708375 100644 --- a/guacamole_user_sync/postgresql/sql.py +++ b/guacamole_user_sync/postgresql/sql.py @@ -25,7 +25,7 @@ def commands(cls, schema_version: SchemaVersion) -> list[TextClause]: sql_file_path = Path(__file__).with_name( f"guacamole_schema.{schema_version.value}.sql" ) - with open(sql_file_path, "r") as f_sql: + with open(sql_file_path) as f_sql: statements = sqlparse.split(f_sql.read()) for statement in statements: # Extract the first comment if there is one diff --git a/pyproject.toml b/pyproject.toml index 0596c96..a673fa4 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -110,6 +110,7 @@ select = [ "N", # pep8-naming "UP", # pyupgrade "W", # pycodestyle warnings + "YTT", # flake8-2020 ] ignore = [ "D100", # undocumented-public-module diff --git a/tests/mocks.py b/tests/mocks.py index 992c5ef..fb5c52c 100644 --- a/tests/mocks.py +++ b/tests/mocks.py @@ -1,4 +1,4 @@ -from typing import Any, Generic, Type, TypeVar +from typing import Any, Generic, TypeVar import ldap from sqlalchemy.sql.expression import TextClause @@ -58,7 +58,7 @@ class MockPostgreSQLBackend(Generic[T]): """Mock PostgreSQLBackend.""" def __init__(self, *data_lists: Any, **kwargs: Any) -> None: - self.contents: dict[Type[T], list[T]] = {} + self.contents: dict[type[T], list[T]] = {} for data_list in data_lists: self.add_all(data_list) @@ -75,7 +75,7 @@ def execute_commands(self, commands: list[TextClause]) -> None: for command in commands: print(f"Executing {command}") - def query(self, table: Type[T], **filter_kwargs: Any) -> Any: + def query(self, table: type[T], **filter_kwargs: Any) -> Any: if table not in self.contents: self.contents[table] = [] results = [item for item in self.contents[table]] From 8a2d26e3a07a9162a8791bf1d7a922e136c5693f Mon Sep 17 00:00:00 2001 From: James Robinson Date: Tue, 24 Sep 2024 16:15:44 +0100 Subject: [PATCH 05/39] :white_check_mark: Add flake8-annotations validation --- .../postgresql/postgresql_backend.py | 6 ++-- .../postgresql/postgresql_client.py | 2 +- pyproject.toml | 9 ++++-- tests/mocks.py | 16 ++++++---- tests/test_ldap.py | 30 +++++++++++-------- tests/test_postgresql.py | 25 ++++++++-------- 6 files changed, 50 insertions(+), 38 deletions(-) diff --git a/guacamole_user_sync/postgresql/postgresql_backend.py b/guacamole_user_sync/postgresql/postgresql_backend.py index 0ebd172..b1648cd 100644 --- a/guacamole_user_sync/postgresql/postgresql_backend.py +++ b/guacamole_user_sync/postgresql/postgresql_backend.py @@ -23,7 +23,7 @@ def __init__( user_name: str, user_password: str, session: Session | None = None, - ): + ) -> None: self.database_name = database_name self.host_name = host_name self.port = port @@ -56,7 +56,7 @@ def add_all(self, items: list[T]) -> None: with session.begin(): session.add_all(items) - def delete(self, table: type[T], *filter_args: Any) -> None: + def delete(self, table: type[T], *filter_args: Any) -> None: # noqa: ANN401 with self.session() as session: # type:ignore with session.begin(): if filter_args: @@ -70,7 +70,7 @@ def execute_commands(self, commands: list[TextClause]) -> None: for command in commands: session.execute(command) - def query(self, table: type[T], **filter_kwargs: Any) -> list[T]: + def query(self, table: type[T], **filter_kwargs: Any) -> list[T]: # noqa: ANN401 with self.session() as session: # type:ignore with session.begin(): if filter_kwargs: diff --git a/guacamole_user_sync/postgresql/postgresql_client.py b/guacamole_user_sync/postgresql/postgresql_client.py index aacbf91..443297c 100644 --- a/guacamole_user_sync/postgresql/postgresql_client.py +++ b/guacamole_user_sync/postgresql/postgresql_client.py @@ -34,7 +34,7 @@ def __init__( port: int, user_name: str, user_password: str, - ): + ) -> None: self.backend = PostgreSQLBackend( database_name=database_name, host_name=host_name, diff --git a/pyproject.toml b/pyproject.toml index a673fa4..8602fe1 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -102,6 +102,7 @@ ignore_missing_imports = true [tool.ruff.lint] select = [ "A", # flake8-builtins + "ANN", # flake8-annotations "C90", # McCabe complexity "D", # pydocstyle "E", # pycodestyle errors @@ -113,13 +114,15 @@ select = [ "YTT", # flake8-2020 ] ignore = [ + "ANN101", # missing-type-self [deprecated] + "ANN102", # missing-type-cls [deprecated] "D100", # undocumented-public-module "D102", # undocumented-public-method "D103", # undocumented-public-function "D104", # undocumented-public-package "D105", # undocumented-magic-method "D107", # undocumented-public-init - "D203", # one-blank-line-before-class [due to conflict with D211] - "D213", # multi-line-summary-second-line [due to conflict with D212] - "D400", # ends-in-period [due to conflict with D415] + "D203", # one-blank-line-before-class [conflicts with D211] + "D213", # multi-line-summary-second-line [conflicts with D212] + "D400", # ends-in-period [conflicts with D415] ] \ No newline at end of file diff --git a/tests/mocks.py b/tests/mocks.py index fb5c52c..cf3c582 100644 --- a/tests/mocks.py +++ b/tests/mocks.py @@ -25,15 +25,19 @@ class MockAsyncSearchList: """Mock AsyncSearchList.""" def __init__( - self, partial: bool, results: LDAPSearchResult, *args: Any, **kwargs: Any + self, + partial: bool, + results: LDAPSearchResult, + *args: Any, # noqa: ANN401 + **kwargs: Any, # noqa: ANN401 ) -> None: self.allResults = results self.partial = partial - def startSearch(self, *args: Any, **kwargs: Any) -> None: # noqa: N802 + def startSearch(self, *args: Any, **kwargs: Any) -> None: # noqa: ANN401, N802 pass - def processResults(self, *args: Any, **kwargs: Any) -> bool: # noqa: N802 + def processResults(self, *args: Any, **kwargs: Any) -> bool: # noqa: ANN401, N802 return self.partial @@ -57,7 +61,7 @@ def __init__(self, results: LDAPSearchResult) -> None: class MockPostgreSQLBackend(Generic[T]): """Mock PostgreSQLBackend.""" - def __init__(self, *data_lists: Any, **kwargs: Any) -> None: + def __init__(self, *data_lists: Any, **kwargs: Any) -> None: # noqa: ANN401 self.contents: dict[type[T], list[T]] = {} for data_list in data_lists: self.add_all(data_list) @@ -68,14 +72,14 @@ def add_all(self, items: list[T]) -> None: self.contents[cls] = [] self.contents[cls] += items - def delete(self, *args: Any, **kwargs: Any) -> None: + def delete(self, *args: Any, **kwargs: Any) -> None: # noqa: ANN401 pass def execute_commands(self, commands: list[TextClause]) -> None: for command in commands: print(f"Executing {command}") - def query(self, table: type[T], **filter_kwargs: Any) -> Any: + def query(self, table: type[T], **filter_kwargs: Any) -> list[T]: # noqa: ANN401 if table not in self.contents: self.contents[table] = [] results = [item for item in self.contents[table]] diff --git a/tests/test_ldap.py b/tests/test_ldap.py index 1a9fb9f..84f6f8a 100644 --- a/tests/test_ldap.py +++ b/tests/test_ldap.py @@ -28,7 +28,7 @@ def test_constructor(self) -> None: client = LDAPClient(hostname="test-host") assert client.hostname == "test-host" - def test_connect(self, monkeypatch: Any) -> None: + def test_connect(self, monkeypatch: pytest.MonkeyPatch) -> None: def mock_initialize(uri: str) -> MockLDAPObject: return MockLDAPObject(uri) @@ -39,7 +39,7 @@ def mock_initialize(uri: str) -> MockLDAPObject: assert isinstance(cnxn, MockLDAPObject) assert cnxn.uri == "ldap://test-host" - def test_connect_with_bind(self, monkeypatch: Any) -> None: + def test_connect_with_bind(self, monkeypatch: pytest.MonkeyPatch) -> None: def mock_initialize(uri: str) -> MockLDAPObject: return MockLDAPObject(uri) @@ -53,7 +53,9 @@ def mock_initialize(uri: str) -> MockLDAPObject: assert cnxn.bind_dn == "bind-dn" assert cnxn.bind_password == "bind_password" - def test_connect_with_failed_bind(self, monkeypatch: Any, caplog: Any) -> None: + def test_connect_with_failed_bind( + self, monkeypatch: pytest.MonkeyPatch, caplog: pytest.LogCaptureFixture + ) -> None: def mock_initialize(uri: str) -> MockLDAPObject: return MockLDAPObject(uri) @@ -66,8 +68,10 @@ def mock_initialize(uri: str) -> MockLDAPObject: client.connect() assert "Connection credentials were incorrect." in caplog.text - def test_search_exception_server_down(self, monkeypatch: Any, caplog: Any) -> None: - def mock_raise_server_down(*args: Any) -> None: + def test_search_exception_server_down( + self, monkeypatch: pytest.MonkeyPatch, caplog: pytest.LogCaptureFixture + ) -> None: + def mock_raise_server_down(*args: Any) -> None: # noqa: ANN401 raise ldap.SERVER_DOWN monkeypatch.setattr( @@ -79,9 +83,9 @@ def mock_raise_server_down(*args: Any) -> None: assert "Server could not be reached." in caplog.text def test_search_exception_sizelimit_exceeded( - self, monkeypatch: Any, caplog: Any + self, monkeypatch: pytest.MonkeyPatch, caplog: pytest.LogCaptureFixture ) -> None: - def mock_raise_sizelimit_exceeded(*args: Any) -> None: + def mock_raise_sizelimit_exceeded(*args: Any) -> None: # noqa: ANN401 raise ldap.SIZELIMIT_EXCEEDED monkeypatch.setattr( @@ -94,7 +98,7 @@ def mock_raise_sizelimit_exceeded(*args: Any) -> None: def test_search_failure_partial( self, - caplog: Any, + caplog: pytest.LogCaptureFixture, ldap_response_groups_fixture: LDAPSearchResult, ) -> None: caplog.set_level(logging.DEBUG) @@ -111,10 +115,10 @@ def test_search_failure_partial( def test_search_no_results( self, - monkeypatch: Any, - caplog: Any, + monkeypatch: pytest.MonkeyPatch, + caplog: pytest.LogCaptureFixture, ) -> None: - def mock_raise_no_results(*args: Any) -> None: + def mock_raise_no_results(*args: Any) -> None: # noqa: ANN401 raise ldap.NO_SUCH_OBJECT monkeypatch.setattr(ldap.asyncsearch.List, "startSearch", mock_raise_no_results) @@ -125,7 +129,7 @@ def mock_raise_no_results(*args: Any) -> None: def test_search_groups( self, - caplog: Any, + caplog: pytest.LogCaptureFixture, ldap_query_groups_fixture: LDAPQuery, ldap_response_groups_fixture: LDAPSearchResult, ldap_model_groups_fixture: list[LDAPGroup], @@ -146,7 +150,7 @@ def test_search_groups( def test_search_users( self, - caplog: Any, + caplog: pytest.LogCaptureFixture, ldap_query_users_fixture: LDAPQuery, ldap_response_users_fixture: LDAPSearchResult, ldap_model_users_fixture: list[LDAPUser], diff --git a/tests/test_postgresql.py b/tests/test_postgresql.py index c3d18fd..b9fab03 100644 --- a/tests/test_postgresql.py +++ b/tests/test_postgresql.py @@ -10,6 +10,7 @@ from sqlalchemy.orm import Session from sqlalchemy.pool.impl import QueuePool from sqlalchemy.sql.elements import BinaryExpression +from sqlalchemy.sql.expression import TextClause from guacamole_user_sync.models import LDAPGroup, LDAPUser, PostgreSQLError from guacamole_user_sync.postgresql import PostgreSQLBackend, PostgreSQLClient @@ -192,7 +193,7 @@ def test_constructor(self) -> None: def test_assign_users_to_groups( self, - caplog: Any, + caplog: pytest.LogCaptureFixture, ldap_model_groups_fixture: list[LDAPGroup], ldap_model_users_fixture: list[LDAPUser], postgresql_model_guacamoleentity_fixture: list[GuacamoleEntity], @@ -227,7 +228,7 @@ def test_assign_users_to_groups( def test_assign_users_to_groups_missing_ldap_user( self, - caplog: Any, + caplog: pytest.LogCaptureFixture, ldap_model_groups_fixture: list[LDAPGroup], ldap_model_users_fixture: list[LDAPUser], postgresql_model_guacamoleentity_fixture: list[GuacamoleEntity], @@ -261,7 +262,7 @@ def test_assign_users_to_groups_missing_ldap_user( def test_assign_users_to_groups_missing_user_entity( self, - caplog: Any, + caplog: pytest.LogCaptureFixture, ldap_model_groups_fixture: list[LDAPGroup], ldap_model_users_fixture: list[LDAPUser], postgresql_model_guacamoleentity_user_fixture: list[GuacamoleEntity], @@ -296,7 +297,7 @@ def test_assign_users_to_groups_missing_user_entity( def test_assign_users_to_groups_missing_usergroup( self, - caplog: Any, + caplog: pytest.LogCaptureFixture, ldap_model_groups_fixture: list[LDAPGroup], ldap_model_users_fixture: list[LDAPUser], postgresql_model_guacamoleentity_fixture: list[GuacamoleEntity], @@ -329,7 +330,7 @@ def test_assign_users_to_groups_missing_usergroup( ): assert output_line in caplog.text - def test_ensure_schema(self, capsys: Any) -> None: + def test_ensure_schema(self, capsys: pytest.CaptureFixture) -> None: # Create a mock backend mock_backend = MockPostgreSQLBackend() # type: ignore @@ -418,9 +419,9 @@ def test_ensure_schema(self, capsys: Any) -> None: f"Executing CREATE INDEX IF NOT EXISTS {index_name}" in captured.out ) - def test_ensure_schema_exception(self, capsys: Any) -> None: + def test_ensure_schema_exception(self, capsys: pytest.CaptureFixture) -> None: # Create a mock backend - def execute_commands_exception(commands: Any) -> None: + def execute_commands_exception(commands: list[TextClause]) -> None: raise OperationalError(statement="statement", params=None, orig=None) mock_backend = MockPostgreSQLBackend() # type: ignore @@ -440,7 +441,7 @@ def execute_commands_exception(commands: Any) -> None: def test_update( self, - caplog: Any, + caplog: pytest.LogCaptureFixture, ldap_model_groups_fixture: list[LDAPGroup], ldap_model_users_fixture: list[LDAPUser], ) -> None: @@ -492,7 +493,7 @@ def test_update( def test_update_group_entities( self, - caplog: Any, + caplog: pytest.LogCaptureFixture, postgresql_model_guacamoleentity_user_group_fixture: list[GuacamoleEntity], postgresql_model_guacamoleusergroup_fixture: list[GuacamoleUserGroup], ) -> None: @@ -522,7 +523,7 @@ def test_update_group_entities( def test_update_groups( self, - caplog: Any, + caplog: pytest.LogCaptureFixture, ldap_model_groups_fixture: list[LDAPGroup], postgresql_model_guacamoleentity_user_group_fixture: list[GuacamoleEntity], ) -> None: @@ -556,7 +557,7 @@ def test_update_groups( def test_update_user_entities( self, - caplog: Any, + caplog: pytest.LogCaptureFixture, ldap_model_users_fixture: list[LDAPUser], postgresql_model_guacamoleuser_fixture: list[GuacamoleUser], postgresql_model_guacamoleentity_user_fixture: list[GuacamoleEntity], @@ -587,7 +588,7 @@ def test_update_user_entities( def test_update_users( self, - caplog: Any, + caplog: pytest.LogCaptureFixture, ldap_model_users_fixture: list[LDAPUser], postgresql_model_guacamoleentity_user_fixture: list[GuacamoleEntity], ) -> None: From 8b6e4f682470c877927dd09e2b8a1fca58699594 Mon Sep 17 00:00:00 2001 From: James Robinson Date: Tue, 24 Sep 2024 16:16:35 +0100 Subject: [PATCH 06/39] :white_check_mark: Add flake8-bandit validation --- pyproject.toml | 2 ++ tests/mocks.py | 2 +- tests/test_ldap.py | 10 +++++++--- tests/test_postgresql.py | 4 ++-- 4 files changed, 12 insertions(+), 6 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 8602fe1..8024831 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -109,6 +109,7 @@ select = [ "F", # pyflakes "I", # isort "N", # pep8-naming + "S", # flake8-bandit "UP", # pyupgrade "W", # pycodestyle warnings "YTT", # flake8-2020 @@ -125,4 +126,5 @@ ignore = [ "D203", # one-blank-line-before-class [conflicts with D211] "D213", # multi-line-summary-second-line [conflicts with D212] "D400", # ends-in-period [conflicts with D415] + "S101", # assert [conflicts with pytest] ] \ No newline at end of file diff --git a/tests/mocks.py b/tests/mocks.py index cf3c582..028a951 100644 --- a/tests/mocks.py +++ b/tests/mocks.py @@ -15,7 +15,7 @@ def __init__(self, uri: str) -> None: self.bind_password = "" def simple_bind_s(self, bind_dn: str, bind_password: str) -> None: - if bind_password == "incorrect-password": + if bind_password == "incorrect-password": # noqa: S105 raise ldap.INVALID_CREDENTIALS self.bind_dn = bind_dn self.bind_password = bind_password diff --git a/tests/test_ldap.py b/tests/test_ldap.py index 84f6f8a..816fa80 100644 --- a/tests/test_ldap.py +++ b/tests/test_ldap.py @@ -46,12 +46,14 @@ def mock_initialize(uri: str) -> MockLDAPObject: monkeypatch.setattr(ldap, "initialize", mock_initialize) client = LDAPClient( - hostname="test-host", bind_dn="bind-dn", bind_password="bind_password" + hostname="test-host", + bind_dn="bind-dn", + bind_password="bind_password", # noqa: S106 ) cnxn = client.connect() assert isinstance(cnxn, MockLDAPObject) assert cnxn.bind_dn == "bind-dn" - assert cnxn.bind_password == "bind_password" + assert cnxn.bind_password == "bind_password" # noqa: S105 def test_connect_with_failed_bind( self, monkeypatch: pytest.MonkeyPatch, caplog: pytest.LogCaptureFixture @@ -62,7 +64,9 @@ def mock_initialize(uri: str) -> MockLDAPObject: monkeypatch.setattr(ldap, "initialize", mock_initialize) client = LDAPClient( - hostname="test-host", bind_dn="bind-dn", bind_password="incorrect-password" + hostname="test-host", + bind_dn="bind-dn", + bind_password="incorrect-password", # noqa: S106 ) with pytest.raises(LDAPError): client.connect() diff --git a/tests/test_postgresql.py b/tests/test_postgresql.py index b9fab03..fc5117b 100644 --- a/tests/test_postgresql.py +++ b/tests/test_postgresql.py @@ -40,7 +40,7 @@ def mock_backend( host_name="host_name", port=1234, user_name="user_name", - user_password="user_password", + user_password="user_password", # noqa: S106 session=mock_session, ) if test_session: @@ -54,7 +54,7 @@ def test_constructor(self) -> None: assert backend.host_name == "host_name" assert backend.port == 1234 assert backend.user_name == "user_name" - assert backend.user_password == "user_password" + assert backend.user_password == "user_password" # noqa: S105 def test_engine(self) -> None: backend, _ = self.mock_backend() From c34e1d490005999bf9302cae3a3ed6470f9da6f3 Mon Sep 17 00:00:00 2001 From: James Robinson Date: Tue, 24 Sep 2024 16:24:57 +0100 Subject: [PATCH 07/39] :white_check_mark: Add flake8-boolean-trap validation --- pyproject.toml | 1 + tests/mocks.py | 2 +- tests/test_postgresql.py | 70 +++++++++++++++++----------------------- 3 files changed, 32 insertions(+), 41 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 8024831..5317b56 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -107,6 +107,7 @@ select = [ "D", # pydocstyle "E", # pycodestyle errors "F", # pyflakes + "FBT", # flake8-boolean-trap "I", # isort "N", # pep8-naming "S", # flake8-bandit diff --git a/tests/mocks.py b/tests/mocks.py index 028a951..189d58f 100644 --- a/tests/mocks.py +++ b/tests/mocks.py @@ -26,7 +26,7 @@ class MockAsyncSearchList: def __init__( self, - partial: bool, + partial: bool, # noqa: FBT001 results: LDAPSearchResult, *args: Any, # noqa: ANN401 **kwargs: Any, # noqa: ANN401 diff --git a/tests/test_postgresql.py b/tests/test_postgresql.py index fc5117b..2041986 100644 --- a/tests/test_postgresql.py +++ b/tests/test_postgresql.py @@ -28,27 +28,25 @@ class TestPostgreSQLBackend: """Test PostgreSQLBackend.""" - def mock_backend( - self, test_session: bool = False - ) -> tuple[PostgreSQLBackend, mock.MagicMock]: - mock_session = mock.MagicMock() - mock_session.__enter__.return_value = mock_session - mock_session.filter.return_value = mock_session - mock_session.query.return_value = mock_session - backend = PostgreSQLBackend( + def mock_backend(self, session: Session | None = None) -> PostgreSQLBackend: + return PostgreSQLBackend( database_name="database_name", host_name="host_name", port=1234, user_name="user_name", user_password="user_password", # noqa: S106 - session=mock_session, + session=session, ) - if test_session: - backend._session = None - return (backend, mock_session) + + def mock_session(self) -> mock.MagicMock: + mock_session = mock.MagicMock() + mock_session.__enter__.return_value = mock_session + mock_session.filter.return_value = mock_session + mock_session.query.return_value = mock_session + return mock_session def test_constructor(self) -> None: - backend, _ = self.mock_backend() + backend = self.mock_backend() assert isinstance(backend, PostgreSQLBackend) assert backend.database_name == "database_name" assert backend.host_name == "host_name" @@ -57,7 +55,7 @@ def test_constructor(self) -> None: assert backend.user_password == "user_password" # noqa: S105 def test_engine(self) -> None: - backend, _ = self.mock_backend() + backend = self.mock_backend() assert isinstance(backend.engine, Engine) assert isinstance(backend.engine.pool, QueuePool) assert isinstance(backend.engine.dialect, PGDialect_psycopg) @@ -67,14 +65,15 @@ def test_engine(self) -> None: assert not backend.engine.hide_parameters # type: ignore def test_session(self) -> None: - backend, _ = self.mock_backend(test_session=True) + backend = self.mock_backend() assert isinstance(backend.session(), Session) def test_add_all( self, postgresql_model_guacamoleentity_fixture: list[GuacamoleEntity], ) -> None: - backend, session = self.mock_backend() + session = self.mock_session() + backend = self.mock_backend(session=session) backend.add_all(postgresql_model_guacamoleentity_fixture) # Check method calls @@ -86,14 +85,12 @@ def test_add_all( assert len(execute_args) == 1 assert len(execute_args[0]) == len(postgresql_model_guacamoleentity_fixture) - def test_delete( - self, - ) -> None: - backend, session = self.mock_backend() + def test_delete(self) -> None: + session = self.mock_session() + backend = self.mock_backend(session=session) backend.delete(GuacamoleEntity) # Check method calls - print(session.mock_calls) session.query.assert_called_once() session.filter.assert_not_called() session.delete.assert_called_once() @@ -104,16 +101,14 @@ def test_delete( assert len(query_args) == 1 assert isinstance(query_args[0], type(GuacamoleEntity)) - def test_delete_with_filter( - self, - ) -> None: - backend, session = self.mock_backend() + def test_delete_with_filter(self) -> None: + session = self.mock_session() + backend = self.mock_backend(session=session) backend.delete( GuacamoleEntity, GuacamoleEntity.type == GuacamoleEntityType.USER ) # Check method calls - print(session.mock_calls) session.query.assert_called_once() session.filter.assert_called_once() session.delete.assert_called_once() @@ -127,22 +122,19 @@ def test_delete_with_filter( assert len(filter_args) == 1 assert isinstance(filter_args[0], BinaryExpression) - def test_execute_commands( - self, - ) -> None: + def test_execute_commands(self) -> None: command = text("SELECT * FROM guacamole_entity;") - backend, session = self.mock_backend() + session = self.mock_session() + backend = self.mock_backend(session=session) backend.execute_commands([command]) session.execute.assert_called_once_with(command) - def test_query( - self, - ) -> None: - backend, session = self.mock_backend() + def test_query(self) -> None: + session = self.mock_session() + backend = self.mock_backend(session=session) backend.query(GuacamoleEntity) # Check method calls - print(session.mock_calls) session.query.assert_called_once() session.filter.assert_not_called() session.delete.assert_not_called() @@ -153,14 +145,12 @@ def test_query( assert len(query_args) == 1 assert isinstance(query_args[0], type(GuacamoleEntity)) - def test_query_with_filter( - self, - ) -> None: - backend, session = self.mock_backend() + def test_query_with_filter(self) -> None: + session = self.mock_session() + backend = self.mock_backend(session=session) backend.query(GuacamoleEntity, type=GuacamoleEntityType.USER) # Check method calls - print(session.mock_calls) session.query.assert_called_once() session.filter_by.assert_called_once() session.__exit__.assert_called_once() From e13ecf9dc849b584dbc76c849a109ab19d3f2e43 Mon Sep 17 00:00:00 2001 From: James Robinson Date: Tue, 24 Sep 2024 16:29:58 +0100 Subject: [PATCH 08/39] :white_check_mark: Add flake8-bugbear validation --- pyproject.toml | 1 + 1 file changed, 1 insertion(+) diff --git a/pyproject.toml b/pyproject.toml index 5317b56..54d3438 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -103,6 +103,7 @@ ignore_missing_imports = true select = [ "A", # flake8-builtins "ANN", # flake8-annotations + "B", # flake8-bugbear "C90", # McCabe complexity "D", # pydocstyle "E", # pycodestyle errors From 9f8978c80c1a04e00b74fc025ec642d8df9ef358 Mon Sep 17 00:00:00 2001 From: James Robinson Date: Tue, 24 Sep 2024 16:31:17 +0100 Subject: [PATCH 09/39] :white_check_mark: Add flake8-comma validation --- guacamole_user_sync/ldap/ldap_client.py | 4 +- .../postgresql/postgresql_client.py | 56 +++++++++++-------- guacamole_user_sync/postgresql/sql.py | 4 +- pyproject.toml | 1 + tests/conftest.py | 16 ++++-- tests/test_ldap.py | 32 +++++++---- tests/test_postgresql.py | 51 ++++++++++------- 7 files changed, 101 insertions(+), 63 deletions(-) diff --git a/guacamole_user_sync/ldap/ldap_client.py b/guacamole_user_sync/ldap/ldap_client.py index 9b1ad96..a5aa19e 100644 --- a/guacamole_user_sync/ldap/ldap_client.py +++ b/guacamole_user_sync/ldap/ldap_client.py @@ -55,7 +55,7 @@ def search_groups(self, query: LDAPQuery) -> list[LDAPGroup]: group.decode("utf-8") for group in attr_dict["memberUid"] ], name=attr_dict[query.id_attr][0].decode("utf-8"), - ) + ), ) logger.debug(f"Loaded {len(output)} LDAP groups") return output @@ -72,7 +72,7 @@ def search_users(self, query: LDAPQuery) -> list[LDAPUser]: ], name=attr_dict[query.id_attr][0].decode("utf-8"), uid=attr_dict["uid"][0].decode("utf-8"), - ) + ), ) logger.debug(f"Loaded {len(output)} LDAP users") return output diff --git a/guacamole_user_sync/postgresql/postgresql_client.py b/guacamole_user_sync/postgresql/postgresql_client.py index 443297c..0328546 100644 --- a/guacamole_user_sync/postgresql/postgresql_client.py +++ b/guacamole_user_sync/postgresql/postgresql_client.py @@ -44,11 +44,13 @@ def __init__( ) def assign_users_to_groups( - self, groups: list[LDAPGroup], users: list[LDAPUser] + self, + groups: list[LDAPGroup], + users: list[LDAPUser], ) -> None: logger.info( f"Ensuring that {len(users)} user(s)" - f" are correctly assigned among {len(groups)} group(s)" + f" are correctly assigned among {len(groups)} group(s)", ) user_group_members = [] for group in groups: @@ -71,11 +73,11 @@ def assign_users_to_groups( ) ][0] logger.debug( - f"-> entity_id: {group_entity_id}; user_group_id: {user_group_id}" + f"-> entity_id: {group_entity_id}; user_group_id: {user_group_id}", ) except IndexError: logger.debug( - f"Could not determine user_group_id for group '{group.name}'." + f"Could not determine user_group_id for group '{group.name}'.", ) continue # Get the user_entity_id for each user belonging to this group @@ -95,7 +97,7 @@ def assign_users_to_groups( ) ][0] logger.debug( - f"... group member '{user}' has entity_id '{user_entity_id}'" + f"... group member '{user}' has entity_id '{user_entity_id}'", ) except IndexError: logger.debug(f"Could not find entity ID for LDAP user {user_uid}") @@ -105,7 +107,7 @@ def assign_users_to_groups( GuacamoleUserGroupMember( user_group_id=user_group_id, member_entity_id=user_entity_id, - ) + ), ) # Clear existing assignments then reassign logger.debug(f"... creating {len(user_group_members)} user/group assignments.") @@ -135,12 +137,13 @@ def update_groups(self, groups: list[LDAPGroup]) -> None: current_group_names = [ item.name for item in self.backend.query( - GuacamoleEntity, type=GuacamoleEntityType.USER_GROUP + GuacamoleEntity, + type=GuacamoleEntityType.USER_GROUP, ) ] # Add groups logger.debug( - f"There are {len(current_group_names)} group(s) currently registered" + f"There are {len(current_group_names)} group(s) currently registered", ) group_names_to_add = [ group_name @@ -152,7 +155,7 @@ def update_groups(self, groups: list[LDAPGroup]) -> None: [ GuacamoleEntity(name=group_name, type=GuacamoleEntityType.USER_GROUP) for group_name in group_names_to_add - ] + ], ) # Remove groups group_names_to_remove = [ @@ -175,17 +178,18 @@ def update_group_entities(self) -> None: ] logger.debug( f"There are {len(current_user_group_entity_ids)}" - " user group entit(y|ies) currently registered" + " user group entit(y|ies) currently registered", ) new_group_entity_ids = [ group.entity_id for group in self.backend.query( - GuacamoleEntity, type=GuacamoleEntityType.USER_GROUP + GuacamoleEntity, + type=GuacamoleEntityType.USER_GROUP, ) if group.entity_id not in current_user_group_entity_ids ] logger.debug( - f"... {len(new_group_entity_ids)} user group entit(y|ies) will be added" + f"... {len(new_group_entity_ids)} user group entit(y|ies) will be added", ) self.backend.add_all( [ @@ -193,18 +197,20 @@ def update_group_entities(self) -> None: entity_id=group_entity_id, ) for group_entity_id in new_group_entity_ids - ] + ], ) # Clean up any unused entries valid_entity_ids = [ group.entity_id for group in self.backend.query( - GuacamoleEntity, type=GuacamoleEntityType.USER_GROUP + GuacamoleEntity, + type=GuacamoleEntityType.USER_GROUP, ) ] logger.debug(f"There are {len(valid_entity_ids)} valid user group entit(y|ies)") self.backend.delete( - GuacamoleUserGroup, GuacamoleUserGroup.entity_id.not_in(valid_entity_ids) + GuacamoleUserGroup, + GuacamoleUserGroup.entity_id.not_in(valid_entity_ids), ) def update_users(self, users: list[LDAPUser]) -> None: @@ -215,7 +221,8 @@ def update_users(self, users: list[LDAPUser]) -> None: current_usernames = [ user.name for user in self.backend.query( - GuacamoleEntity, type=GuacamoleEntityType.USER + GuacamoleEntity, + type=GuacamoleEntityType.USER, ) ] # Add users @@ -230,7 +237,7 @@ def update_users(self, users: list[LDAPUser]) -> None: [ GuacamoleEntity(name=username, type=GuacamoleEntityType.USER) for username in usernames_to_add - ] + ], ) # Remove users usernames_to_remove = [ @@ -253,17 +260,18 @@ def update_user_entities(self, users: list[LDAPUser]) -> None: ] logger.debug( f"There are {len(current_user_entity_ids)} " - "user entit(y|ies) currently registered" + "user entit(y|ies) currently registered", ) new_user_tuples: list[tuple[int, LDAPUser]] = [ (user.entity_id, [u for u in users if u.name == user.name][0]) for user in self.backend.query( - GuacamoleEntity, type=GuacamoleEntityType.USER + GuacamoleEntity, + type=GuacamoleEntityType.USER, ) if user.entity_id not in current_user_entity_ids ] logger.debug( - f"... {len(current_user_entity_ids)} user entit(y|ies) will be added" + f"... {len(current_user_entity_ids)} user entit(y|ies) will be added", ) self.backend.add_all( [ @@ -275,16 +283,18 @@ def update_user_entities(self, users: list[LDAPUser]) -> None: password_salt=secrets.token_bytes(32), ) for user_tuple in new_user_tuples - ] + ], ) # Clean up any unused entries valid_entity_ids = [ user.entity_id for user in self.backend.query( - GuacamoleEntity, type=GuacamoleEntityType.USER + GuacamoleEntity, + type=GuacamoleEntityType.USER, ) ] logger.debug(f"There are {len(valid_entity_ids)} valid user entit(y|ies)") self.backend.delete( - GuacamoleUser, GuacamoleUser.entity_id.not_in(valid_entity_ids) + GuacamoleUser, + GuacamoleUser.entity_id.not_in(valid_entity_ids), ) diff --git a/guacamole_user_sync/postgresql/sql.py b/guacamole_user_sync/postgresql/sql.py index 4708375..2584001 100644 --- a/guacamole_user_sync/postgresql/sql.py +++ b/guacamole_user_sync/postgresql/sql.py @@ -23,7 +23,7 @@ def commands(cls, schema_version: SchemaVersion) -> list[TextClause]: logger.info(f"Ensuring correct schema for Guacamole {schema_version.value}") commands = [] sql_file_path = Path(__file__).with_name( - f"guacamole_schema.{schema_version.value}.sql" + f"guacamole_schema.{schema_version.value}.sql", ) with open(sql_file_path) as f_sql: statements = sqlparse.split(f_sql.read()) @@ -40,6 +40,6 @@ def commands(cls, schema_version: SchemaVersion) -> list[TextClause]: logger.debug(f"... {first_comment}") # Extract the command commands.append( - text(sqlparse.format(statement, strip_comments=True, compact=True)) + text(sqlparse.format(statement, strip_comments=True, compact=True)), ) return commands diff --git a/pyproject.toml b/pyproject.toml index 54d3438..c2451c5 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -105,6 +105,7 @@ select = [ "ANN", # flake8-annotations "B", # flake8-bugbear "C90", # McCabe complexity + "COM", # flake8-commas "D", # pydocstyle "E", # pycodestyle errors "F", # pyflakes diff --git a/tests/conftest.py b/tests/conftest.py index 1b39502..e83d27b 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -141,13 +141,19 @@ def ldap_response_users_fixture() -> LDAPSearchResult: def postgresql_model_guacamoleentity_user_group_fixture() -> list[GuacamoleEntity]: return [ GuacamoleEntity( - entity_id=1, name="defendants", type=GuacamoleEntityType.USER_GROUP + entity_id=1, + name="defendants", + type=GuacamoleEntityType.USER_GROUP, ), GuacamoleEntity( - entity_id=2, name="everyone", type=GuacamoleEntityType.USER_GROUP + entity_id=2, + name="everyone", + type=GuacamoleEntityType.USER_GROUP, ), GuacamoleEntity( - entity_id=3, name="plaintiffs", type=GuacamoleEntityType.USER_GROUP + entity_id=3, + name="plaintiffs", + type=GuacamoleEntityType.USER_GROUP, ), ] @@ -156,7 +162,9 @@ def postgresql_model_guacamoleentity_user_group_fixture() -> list[GuacamoleEntit def postgresql_model_guacamoleentity_user_fixture() -> list[GuacamoleEntity]: return [ GuacamoleEntity( - entity_id=4, name="aulus.agerius@rome.la", type=GuacamoleEntityType.USER + entity_id=4, + name="aulus.agerius@rome.la", + type=GuacamoleEntityType.USER, ), GuacamoleEntity( entity_id=5, diff --git a/tests/test_ldap.py b/tests/test_ldap.py index 816fa80..15f8c86 100644 --- a/tests/test_ldap.py +++ b/tests/test_ldap.py @@ -56,7 +56,9 @@ def mock_initialize(uri: str) -> MockLDAPObject: assert cnxn.bind_password == "bind_password" # noqa: S105 def test_connect_with_failed_bind( - self, monkeypatch: pytest.MonkeyPatch, caplog: pytest.LogCaptureFixture + self, + monkeypatch: pytest.MonkeyPatch, + caplog: pytest.LogCaptureFixture, ) -> None: def mock_initialize(uri: str) -> MockLDAPObject: return MockLDAPObject(uri) @@ -73,13 +75,17 @@ def mock_initialize(uri: str) -> MockLDAPObject: assert "Connection credentials were incorrect." in caplog.text def test_search_exception_server_down( - self, monkeypatch: pytest.MonkeyPatch, caplog: pytest.LogCaptureFixture + self, + monkeypatch: pytest.MonkeyPatch, + caplog: pytest.LogCaptureFixture, ) -> None: def mock_raise_server_down(*args: Any) -> None: # noqa: ANN401 raise ldap.SERVER_DOWN monkeypatch.setattr( - ldap.asyncsearch.List, "startSearch", mock_raise_server_down + ldap.asyncsearch.List, + "startSearch", + mock_raise_server_down, ) client = LDAPClient(hostname="test-host") with pytest.raises(LDAPError): @@ -87,13 +93,17 @@ def mock_raise_server_down(*args: Any) -> None: # noqa: ANN401 assert "Server could not be reached." in caplog.text def test_search_exception_sizelimit_exceeded( - self, monkeypatch: pytest.MonkeyPatch, caplog: pytest.LogCaptureFixture + self, + monkeypatch: pytest.MonkeyPatch, + caplog: pytest.LogCaptureFixture, ) -> None: def mock_raise_sizelimit_exceeded(*args: Any) -> None: # noqa: ANN401 raise ldap.SIZELIMIT_EXCEEDED monkeypatch.setattr( - ldap.asyncsearch.List, "startSearch", mock_raise_sizelimit_exceeded + ldap.asyncsearch.List, + "startSearch", + mock_raise_sizelimit_exceeded, ) client = LDAPClient(hostname="test-host") with pytest.raises(LDAPError): @@ -107,10 +117,10 @@ def test_search_failure_partial( ) -> None: caplog.set_level(logging.DEBUG) with mock.patch( - "guacamole_user_sync.ldap.ldap_client.AsyncSearchList" + "guacamole_user_sync.ldap.ldap_client.AsyncSearchList", ) as mock_async_search_list: mock_async_search_list.return_value = MockAsyncSearchListPartialResults( - results=ldap_response_groups_fixture[0:1] + results=ldap_response_groups_fixture[0:1], ) client = LDAPClient(hostname="test-host") client.search(query=LDAPQuery(base_dn="", filter="", id_attr="")) @@ -140,10 +150,10 @@ def test_search_groups( ) -> None: caplog.set_level(logging.DEBUG) with mock.patch( - "guacamole_user_sync.ldap.ldap_client.AsyncSearchList" + "guacamole_user_sync.ldap.ldap_client.AsyncSearchList", ) as mock_async_search_list: mock_async_search_list.return_value = MockAsyncSearchListFullResults( - results=ldap_response_groups_fixture + results=ldap_response_groups_fixture, ) client = LDAPClient(hostname="test-host") users = client.search_groups(query=ldap_query_groups_fixture) @@ -161,10 +171,10 @@ def test_search_users( ) -> None: caplog.set_level(logging.DEBUG) with mock.patch( - "guacamole_user_sync.ldap.ldap_client.AsyncSearchList" + "guacamole_user_sync.ldap.ldap_client.AsyncSearchList", ) as mock_async_search_list: mock_async_search_list.return_value = MockAsyncSearchListFullResults( - results=ldap_response_users_fixture + results=ldap_response_users_fixture, ) client = LDAPClient(hostname="test-host") users = client.search_users(query=ldap_query_users_fixture) diff --git a/tests/test_postgresql.py b/tests/test_postgresql.py index 2041986..142f4fd 100644 --- a/tests/test_postgresql.py +++ b/tests/test_postgresql.py @@ -105,7 +105,8 @@ def test_delete_with_filter(self) -> None: session = self.mock_session() backend = self.mock_backend(session=session) backend.delete( - GuacamoleEntity, GuacamoleEntity.type == GuacamoleEntityType.USER + GuacamoleEntity, + GuacamoleEntity.type == GuacamoleEntityType.USER, ) # Check method calls @@ -200,13 +201,14 @@ def test_assign_users_to_groups( # Patch PostgreSQLBackend with mock.patch( - "guacamole_user_sync.postgresql.postgresql_client.PostgreSQLBackend" + "guacamole_user_sync.postgresql.postgresql_client.PostgreSQLBackend", ) as mock_postgresql_backend: mock_postgresql_backend.return_value = mock_backend client = PostgreSQLClient(**self.client_kwargs) client.assign_users_to_groups( - ldap_model_groups_fixture, ldap_model_users_fixture + ldap_model_groups_fixture, + ldap_model_users_fixture, ) for output_line in ( "Ensuring that 2 user(s) are correctly assigned among 3 group(s)", @@ -235,13 +237,14 @@ def test_assign_users_to_groups_missing_ldap_user( # Patch PostgreSQLBackend with mock.patch( - "guacamole_user_sync.postgresql.postgresql_client.PostgreSQLBackend" + "guacamole_user_sync.postgresql.postgresql_client.PostgreSQLBackend", ) as mock_postgresql_backend: mock_postgresql_backend.return_value = mock_backend client = PostgreSQLClient(**self.client_kwargs) client.assign_users_to_groups( - ldap_model_groups_fixture, ldap_model_users_fixture[0:1] + ldap_model_groups_fixture, + ldap_model_users_fixture[0:1], ) for output_line in ( "Ensuring that 1 user(s) are correctly assigned among 3 group(s)", @@ -271,13 +274,14 @@ def test_assign_users_to_groups_missing_user_entity( # Patch PostgreSQLBackend with mock.patch( - "guacamole_user_sync.postgresql.postgresql_client.PostgreSQLBackend" + "guacamole_user_sync.postgresql.postgresql_client.PostgreSQLBackend", ) as mock_postgresql_backend: mock_postgresql_backend.return_value = mock_backend client = PostgreSQLClient(**self.client_kwargs) client.assign_users_to_groups( - ldap_model_groups_fixture, ldap_model_users_fixture + ldap_model_groups_fixture, + ldap_model_users_fixture, ) for output_line in ( @@ -304,13 +308,14 @@ def test_assign_users_to_groups_missing_usergroup( # Patch PostgreSQLBackend with mock.patch( - "guacamole_user_sync.postgresql.postgresql_client.PostgreSQLBackend" + "guacamole_user_sync.postgresql.postgresql_client.PostgreSQLBackend", ) as mock_postgresql_backend: mock_postgresql_backend.return_value = mock_backend client = PostgreSQLClient(**self.client_kwargs) client.assign_users_to_groups( - ldap_model_groups_fixture, ldap_model_users_fixture + ldap_model_groups_fixture, + ldap_model_users_fixture, ) for output_line in ( @@ -326,7 +331,7 @@ def test_ensure_schema(self, capsys: pytest.CaptureFixture) -> None: # Patch PostgreSQLBackend with mock.patch( - "guacamole_user_sync.postgresql.postgresql_client.PostgreSQLBackend" + "guacamole_user_sync.postgresql.postgresql_client.PostgreSQLBackend", ) as mock_postgresql_backend: mock_postgresql_backend.return_value = mock_backend @@ -419,13 +424,14 @@ def execute_commands_exception(commands: list[TextClause]) -> None: # Patch PostgreSQLBackend with mock.patch( - "guacamole_user_sync.postgresql.postgresql_client.PostgreSQLBackend" + "guacamole_user_sync.postgresql.postgresql_client.PostgreSQLBackend", ) as mock_postgresql_backend: mock_postgresql_backend.return_value = mock_backend client = PostgreSQLClient(**self.client_kwargs) with pytest.raises( - PostgreSQLError, match="Unable to ensure PostgreSQL schema." + PostgreSQLError, + match="Unable to ensure PostgreSQL schema.", ): client.ensure_schema(SchemaVersion.v1_5_5) @@ -443,13 +449,14 @@ def test_update( # Patch PostgreSQLBackend with mock.patch( - "guacamole_user_sync.postgresql.postgresql_client.PostgreSQLBackend" + "guacamole_user_sync.postgresql.postgresql_client.PostgreSQLBackend", ) as mock_postgresql_backend: mock_postgresql_backend.return_value = mock_backend client = PostgreSQLClient(**self.client_kwargs) client.update( - groups=ldap_model_groups_fixture, users=ldap_model_users_fixture + groups=ldap_model_groups_fixture, + users=ldap_model_users_fixture, ) for output_line in ( "Ensuring that 3 group(s) are registered", @@ -498,7 +505,7 @@ def test_update_group_entities( # Patch PostgreSQLBackend with mock.patch( - "guacamole_user_sync.postgresql.postgresql_client.PostgreSQLBackend" + "guacamole_user_sync.postgresql.postgresql_client.PostgreSQLBackend", ) as mock_postgresql_backend: mock_postgresql_backend.return_value = mock_backend @@ -525,7 +532,7 @@ def test_update_groups( entity_id=99, name="to-be-deleted", type=GuacamoleEntityType.USER_GROUP, - ) + ), ], ) @@ -534,7 +541,7 @@ def test_update_groups( # Patch PostgreSQLBackend with mock.patch( - "guacamole_user_sync.postgresql.postgresql_client.PostgreSQLBackend" + "guacamole_user_sync.postgresql.postgresql_client.PostgreSQLBackend", ) as mock_postgresql_backend: mock_postgresql_backend.return_value = mock_backend @@ -563,7 +570,7 @@ def test_update_user_entities( # Patch PostgreSQLBackend with mock.patch( - "guacamole_user_sync.postgresql.postgresql_client.PostgreSQLBackend" + "guacamole_user_sync.postgresql.postgresql_client.PostgreSQLBackend", ) as mock_postgresql_backend: mock_postgresql_backend.return_value = mock_backend @@ -587,8 +594,10 @@ def test_update_users( postgresql_model_guacamoleentity_user_fixture[0:1], [ GuacamoleEntity( - entity_id=99, name="to-be-deleted", type=GuacamoleEntityType.USER - ) + entity_id=99, + name="to-be-deleted", + type=GuacamoleEntityType.USER, + ), ], ) @@ -597,7 +606,7 @@ def test_update_users( # Patch PostgreSQLBackend with mock.patch( - "guacamole_user_sync.postgresql.postgresql_client.PostgreSQLBackend" + "guacamole_user_sync.postgresql.postgresql_client.PostgreSQLBackend", ) as mock_postgresql_backend: mock_postgresql_backend.return_value = mock_backend From f9630f1af81f8c70b02c62c699a9de678a5d9cd8 Mon Sep 17 00:00:00 2001 From: James Robinson Date: Tue, 24 Sep 2024 17:15:55 +0100 Subject: [PATCH 10/39] :white_check_mark: Add flake8-comprehension validation --- guacamole_user_sync/postgresql/postgresql_backend.py | 2 +- pyproject.toml | 1 + tests/mocks.py | 2 +- 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/guacamole_user_sync/postgresql/postgresql_backend.py b/guacamole_user_sync/postgresql/postgresql_backend.py index b1648cd..ded6ce2 100644 --- a/guacamole_user_sync/postgresql/postgresql_backend.py +++ b/guacamole_user_sync/postgresql/postgresql_backend.py @@ -77,4 +77,4 @@ def query(self, table: type[T], **filter_kwargs: Any) -> list[T]: # noqa: ANN40 result = session.query(table).filter_by(**filter_kwargs) else: result = session.query(table) - return [item for item in result] + return list(result) diff --git a/pyproject.toml b/pyproject.toml index c2451c5..2f5826b 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -104,6 +104,7 @@ select = [ "A", # flake8-builtins "ANN", # flake8-annotations "B", # flake8-bugbear + "C4", # flake8-comprehensions "C90", # McCabe complexity "COM", # flake8-commas "D", # pydocstyle diff --git a/tests/mocks.py b/tests/mocks.py index 189d58f..28e7496 100644 --- a/tests/mocks.py +++ b/tests/mocks.py @@ -82,7 +82,7 @@ def execute_commands(self, commands: list[TextClause]) -> None: def query(self, table: type[T], **filter_kwargs: Any) -> list[T]: # noqa: ANN401 if table not in self.contents: self.contents[table] = [] - results = [item for item in self.contents[table]] + results = list(self.contents[table]) if "entity_id" in filter_kwargs: results = [ From 2180f7ab067ca9c30b7b4bb98d8431639cea3670 Mon Sep 17 00:00:00 2001 From: James Robinson Date: Tue, 24 Sep 2024 17:23:45 +0100 Subject: [PATCH 11/39] :white_check_mark: Add flake8-datetimez validation --- guacamole_user_sync/postgresql/postgresql_client.py | 4 ++-- pyproject.toml | 1 + tests/conftest.py | 6 +++--- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/guacamole_user_sync/postgresql/postgresql_client.py b/guacamole_user_sync/postgresql/postgresql_client.py index 0328546..125b634 100644 --- a/guacamole_user_sync/postgresql/postgresql_client.py +++ b/guacamole_user_sync/postgresql/postgresql_client.py @@ -1,6 +1,6 @@ -import datetime import logging import secrets +from datetime import UTC, datetime from sqlalchemy.exc import OperationalError @@ -278,7 +278,7 @@ def update_user_entities(self, users: list[LDAPUser]) -> None: GuacamoleUser( entity_id=user_tuple[0], full_name=user_tuple[1].display_name, - password_date=datetime.datetime.now(), + password_date=datetime.now(tz=UTC), password_hash=secrets.token_bytes(32), password_salt=secrets.token_bytes(32), ) diff --git a/pyproject.toml b/pyproject.toml index 2f5826b..4fc4aa4 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -108,6 +108,7 @@ select = [ "C90", # McCabe complexity "COM", # flake8-commas "D", # pydocstyle + "DTZ", # flake8-datetimez "E", # pycodestyle errors "F", # pyflakes "FBT", # flake8-boolean-trap diff --git a/tests/conftest.py b/tests/conftest.py index e83d27b..12afadf 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,4 +1,4 @@ -from datetime import datetime +from datetime import UTC, datetime import pytest @@ -194,7 +194,7 @@ def postgresql_model_guacamoleuser_fixture() -> list[GuacamoleUser]: full_name="Aulus Agerius", password_hash=b"PASSWORD_HASH", password_salt=b"PASSWORD_SALT", - password_date=datetime(1, 1, 1), + password_date=datetime(1, 1, 1, tzinfo=UTC), ), GuacamoleUser( user_id=2, @@ -202,7 +202,7 @@ def postgresql_model_guacamoleuser_fixture() -> list[GuacamoleUser]: full_name="Numerius Negidius", password_hash=b"PASSWORD_HASH", password_salt=b"PASSWORD_SALT", - password_date=datetime(1, 1, 1), + password_date=datetime(1, 1, 1, tzinfo=UTC), ), ] From 66d3360d54521532c62a6c36d79d57d73f48415c Mon Sep 17 00:00:00 2001 From: James Robinson Date: Tue, 24 Sep 2024 21:02:05 +0100 Subject: [PATCH 12/39] :white_check_mark: Add flake8-errmsg validation --- .../postgresql/postgresql_backend.py | 13 ++++++---- .../postgresql/postgresql_client.py | 8 +++---- pyproject.toml | 1 + synchronise.py | 24 ++++++++++++------- tests/test_postgresql.py | 16 +++++++++++++ 5 files changed, 46 insertions(+), 16 deletions(-) diff --git a/guacamole_user_sync/postgresql/postgresql_backend.py b/guacamole_user_sync/postgresql/postgresql_backend.py index ded6ce2..b3d7736 100644 --- a/guacamole_user_sync/postgresql/postgresql_backend.py +++ b/guacamole_user_sync/postgresql/postgresql_backend.py @@ -3,6 +3,7 @@ from sqlalchemy import create_engine from sqlalchemy.engine import URL, Engine # type:ignore +from sqlalchemy.exc import SQLAlchemyError from sqlalchemy.orm import Session from sqlalchemy.sql.expression import TextClause @@ -65,10 +66,14 @@ def delete(self, table: type[T], *filter_args: Any) -> None: # noqa: ANN401 session.query(table).delete() def execute_commands(self, commands: list[TextClause]) -> None: - with self.session() as session: # type:ignore - with session.begin(): - for command in commands: - session.execute(command) + try: + with self.session() as session: # type:ignore + with session.begin(): + for command in commands: + session.execute(command) + except SQLAlchemyError: + logger.warning("Unable to execute PostgreSQL commands.") + raise def query(self, table: type[T], **filter_kwargs: Any) -> list[T]: # noqa: ANN401 with self.session() as session: # type:ignore diff --git a/guacamole_user_sync/postgresql/postgresql_client.py b/guacamole_user_sync/postgresql/postgresql_client.py index 125b634..dd9e3e9 100644 --- a/guacamole_user_sync/postgresql/postgresql_client.py +++ b/guacamole_user_sync/postgresql/postgresql_client.py @@ -2,7 +2,7 @@ import secrets from datetime import UTC, datetime -from sqlalchemy.exc import OperationalError +from sqlalchemy.exc import SQLAlchemyError from guacamole_user_sync.models import ( LDAPGroup, @@ -117,9 +117,9 @@ def assign_users_to_groups( def ensure_schema(self, schema_version: SchemaVersion) -> None: try: self.backend.execute_commands(GuacamoleSchema.commands(schema_version)) - except OperationalError as exc: - logger.warning("Unable to connect to the PostgreSQL server.") - raise PostgreSQLError("Unable to ensure PostgreSQL schema.") from exc + except SQLAlchemyError as exc: + msg = "Unable to ensure PostgreSQL schema." + raise PostgreSQLError(msg) from exc def update(self, *, groups: list[LDAPGroup], users: list[LDAPUser]) -> None: """Update the relevant tables to match lists of LDAP users and groups.""" diff --git a/pyproject.toml b/pyproject.toml index 4fc4aa4..4bf66b0 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -110,6 +110,7 @@ select = [ "D", # pydocstyle "DTZ", # flake8-datetimez "E", # pycodestyle errors + "EM", # flake8-errmsg "F", # pyflakes "FBT", # flake8-boolean-trap "I", # isort diff --git a/synchronise.py b/synchronise.py index 3afaa90..4a8c157 100644 --- a/synchronise.py +++ b/synchronise.py @@ -90,23 +90,31 @@ def synchronise( if __name__ == "__main__": if not (ldap_host := os.getenv("LDAP_HOST", None)): - raise ValueError("LDAP_HOST is not defined") + msg = "LDAP_HOST is not defined" + raise ValueError(msg) if not (ldap_group_base_dn := os.getenv("LDAP_GROUP_BASE_DN", None)): - raise ValueError("LDAP_GROUP_BASE_DN is not defined") + msg = "LDAP_GROUP_BASE_DN is not defined" + raise ValueError(msg) if not (ldap_group_filter := os.getenv("LDAP_GROUP_FILTER", None)): - raise ValueError("LDAP_GROUP_FILTER is not defined") + msg = "LDAP_GROUP_FILTER is not defined" + raise ValueError(msg) if not (ldap_user_base_dn := os.getenv("LDAP_USER_BASE_DN", None)): - raise ValueError("LDAP_USER_BASE_DN is not defined") + msg = "LDAP_USER_BASE_DN is not defined" + raise ValueError(msg) if not (ldap_user_filter := os.getenv("LDAP_USER_FILTER", None)): - raise ValueError("LDAP_USER_FILTER is not defined") + msg = "LDAP_USER_FILTER is not defined" + raise ValueError(msg) if not (postgresql_host_name := os.getenv("POSTGRESQL_HOST", None)): - raise ValueError("POSTGRESQL_HOST is not defined") + msg = "POSTGRESQL_HOST is not defined" + raise ValueError(msg) if not (postgresql_password := os.getenv("POSTGRESQL_PASSWORD", None)): - raise ValueError("POSTGRESQL_PASSWORD is not defined") + msg = "POSTGRESQL_PASSWORD is not defined" + raise ValueError(msg) if not (postgresql_user_name := os.getenv("POSTGRESQL_USERNAME", None)): - raise ValueError("POSTGRESQL_USERNAME is not defined") + msg = "POSTGRESQL_USERNAME is not defined" + raise ValueError(msg) logging.basicConfig( level=( diff --git a/tests/test_postgresql.py b/tests/test_postgresql.py index 142f4fd..c86d721 100644 --- a/tests/test_postgresql.py +++ b/tests/test_postgresql.py @@ -130,6 +130,22 @@ def test_execute_commands(self) -> None: backend.execute_commands([command]) session.execute.assert_called_once_with(command) + def test_execute_commands_exception( + self, + caplog: pytest.LogCaptureFixture, + ) -> None: + command = text("SELECT * FROM guacamole_entity;") + session = self.mock_session() + session.execute.side_effect = OperationalError( + statement="exception reason", + params=None, + orig=None, + ) + backend = self.mock_backend(session=session) + with pytest.raises(OperationalError, match="SQL: exception reason"): + backend.execute_commands([command]) + assert "Unable to execute PostgreSQL commands." in caplog.text + def test_query(self) -> None: session = self.mock_session() backend = self.mock_backend(session=session) From f3afa8789b7be52b2e9217e9ed0611ff47ee5d5c Mon Sep 17 00:00:00 2001 From: James Robinson Date: Thu, 26 Sep 2024 09:30:55 +0100 Subject: [PATCH 13/39] :white_check_mark: Add flake8-future-annotations validation --- pyproject.toml | 1 + 1 file changed, 1 insertion(+) diff --git a/pyproject.toml b/pyproject.toml index 4bf66b0..9a6f240 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -112,6 +112,7 @@ select = [ "E", # pycodestyle errors "EM", # flake8-errmsg "F", # pyflakes + "FA", # flake8-future-annotations "FBT", # flake8-boolean-trap "I", # isort "N", # pep8-naming From 3e53d99eaac58644783d76e6abeb01f804ed38b2 Mon Sep 17 00:00:00 2001 From: James Robinson Date: Thu, 26 Sep 2024 09:31:52 +0100 Subject: [PATCH 14/39] :white_check_mark: Add flake8-implicit-str-concat validation --- pyproject.toml | 1 + 1 file changed, 1 insertion(+) diff --git a/pyproject.toml b/pyproject.toml index 9a6f240..f52f51a 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -115,6 +115,7 @@ select = [ "FA", # flake8-future-annotations "FBT", # flake8-boolean-trap "I", # isort + "ISC", # flake8-implicit-str-concat "N", # pep8-naming "S", # flake8-bandit "UP", # pyupgrade From 9d4f0b9a637b479054b151b1b1a02ace189cff85 Mon Sep 17 00:00:00 2001 From: James Robinson Date: Thu, 26 Sep 2024 09:32:27 +0100 Subject: [PATCH 15/39] :white_check_mark: Add flake8-import-conventions validation --- pyproject.toml | 1 + 1 file changed, 1 insertion(+) diff --git a/pyproject.toml b/pyproject.toml index f52f51a..671ec4d 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -115,6 +115,7 @@ select = [ "FA", # flake8-future-annotations "FBT", # flake8-boolean-trap "I", # isort + "ICN", # flake8-import-conventions "ISC", # flake8-implicit-str-concat "N", # pep8-naming "S", # flake8-bandit From 631d9942c4e9a90b3b27203346a7f0e83ccde4f5 Mon Sep 17 00:00:00 2001 From: James Robinson Date: Thu, 26 Sep 2024 09:34:50 +0100 Subject: [PATCH 16/39] :white_check_mark: Add flake8-logging validation --- pyproject.toml | 1 + 1 file changed, 1 insertion(+) diff --git a/pyproject.toml b/pyproject.toml index 671ec4d..fd33c4a 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -117,6 +117,7 @@ select = [ "I", # isort "ICN", # flake8-import-conventions "ISC", # flake8-implicit-str-concat + "LOG", # flake8-logging "N", # pep8-naming "S", # flake8-bandit "UP", # pyupgrade From 67654d1b0ab69c98d2cafda14636de22e5a5218f Mon Sep 17 00:00:00 2001 From: James Robinson Date: Thu, 26 Sep 2024 09:55:58 +0100 Subject: [PATCH 17/39] :white_check_mark: Add flake8-logging-format validation --- guacamole_user_sync/ldap/ldap_client.py | 12 ++-- .../postgresql/postgresql_client.py | 71 ++++++++++++------- guacamole_user_sync/postgresql/sql.py | 4 +- pyproject.toml | 1 + synchronise.py | 2 +- tests/test_postgresql.py | 2 +- 6 files changed, 57 insertions(+), 35 deletions(-) diff --git a/guacamole_user_sync/ldap/ldap_client.py b/guacamole_user_sync/ldap/ldap_client.py index a5aa19e..56279ac 100644 --- a/guacamole_user_sync/ldap/ldap_client.py +++ b/guacamole_user_sync/ldap/ldap_client.py @@ -32,7 +32,7 @@ def __init__( def connect(self) -> LDAPObject: if not self.cnxn: - logger.info(f"Initialising connection to LDAP host at {self.hostname}") + logger.info("Initialising connection to LDAP host at %s", self.hostname) self.cnxn = ldap.initialize(f"ldap://{self.hostname}") if self.bind_dn: try: @@ -57,7 +57,7 @@ def search_groups(self, query: LDAPQuery) -> list[LDAPGroup]: name=attr_dict[query.id_attr][0].decode("utf-8"), ), ) - logger.debug(f"Loaded {len(output)} LDAP groups") + logger.debug("Loaded %s LDAP groups", len(output)) return output def search_users(self, query: LDAPQuery) -> list[LDAPUser]: @@ -74,14 +74,14 @@ def search_users(self, query: LDAPQuery) -> list[LDAPUser]: uid=attr_dict["uid"][0].decode("utf-8"), ), ) - logger.debug(f"Loaded {len(output)} LDAP users") + logger.debug("Loaded %s LDAP users", len(output)) return output def search(self, query: LDAPQuery) -> LDAPSearchResult: results: LDAPSearchResult = [] logger.info("Querying LDAP host with:") - logger.info(f"... base DN: {query.base_dn}") - logger.info(f"... filter: {query.filter}") + logger.info("... base DN: %s", query.base_dn) + logger.info("... filter: %s", query.filter) searcher = AsyncSearchList(self.connect()) try: searcher.startSearch( @@ -92,7 +92,7 @@ def search(self, query: LDAPQuery) -> LDAPSearchResult: if searcher.processResults() != 0: logger.warning("Only partial results received.") results = searcher.allResults - logger.debug(f"Server returned {len(results)} results.") + logger.debug("Server returned %s results.", len(results)) return results except ldap.NO_SUCH_OBJECT as exc: logger.warning("Server returned no results.") diff --git a/guacamole_user_sync/postgresql/postgresql_client.py b/guacamole_user_sync/postgresql/postgresql_client.py index dd9e3e9..2f7faf8 100644 --- a/guacamole_user_sync/postgresql/postgresql_client.py +++ b/guacamole_user_sync/postgresql/postgresql_client.py @@ -49,12 +49,13 @@ def assign_users_to_groups( users: list[LDAPUser], ) -> None: logger.info( - f"Ensuring that {len(users)} user(s)" - f" are correctly assigned among {len(groups)} group(s)", + "Ensuring that %s user(s) are correctly assigned among %s group(s)", + len(users), + len(groups), ) user_group_members = [] for group in groups: - logger.debug(f"Working on group '{group.name}'") + logger.debug("Working on group '%s'", group.name) # Get the user_group_id for each group (via looking up the entity_id) try: group_entity_id = [ @@ -73,11 +74,14 @@ def assign_users_to_groups( ) ][0] logger.debug( - f"-> entity_id: {group_entity_id}; user_group_id: {user_group_id}", + "-> entity_id: %s; user_group_id: %s", + group_entity_id, + user_group_id, ) except IndexError: logger.debug( - f"Could not determine user_group_id for group '{group.name}'.", + "Could not determine user_group_id for group '%s'.", + group.name, ) continue # Get the user_entity_id for each user belonging to this group @@ -85,7 +89,7 @@ def assign_users_to_groups( try: user = next(filter(lambda u: u.uid == user_uid, users)) except StopIteration: - logger.debug(f"Could not find LDAP user with UID {user_uid}") + logger.debug("Could not find LDAP user with UID %s", user_uid) continue try: user_entity_id = [ @@ -97,10 +101,15 @@ def assign_users_to_groups( ) ][0] logger.debug( - f"... group member '{user}' has entity_id '{user_entity_id}'", + "... group member '%s' has entity_id '%s'", + user, + user_entity_id, ) except IndexError: - logger.debug(f"Could not find entity ID for LDAP user {user_uid}") + logger.debug( + "Could not find entity ID for LDAP user '%s'", + user_uid, + ) continue # Create an entry in the user group member table user_group_members.append( @@ -110,7 +119,10 @@ def assign_users_to_groups( ), ) # Clear existing assignments then reassign - logger.debug(f"... creating {len(user_group_members)} user/group assignments.") + logger.debug( + "... creating %s user/group assignments.", + len(user_group_members), + ) self.backend.delete(GuacamoleUserGroupMember) self.backend.add_all(user_group_members) @@ -132,7 +144,7 @@ def update(self, *, groups: list[LDAPGroup], users: list[LDAPUser]) -> None: def update_groups(self, groups: list[LDAPGroup]) -> None: """Update the entities table with desired groups.""" # Set groups to desired list - logger.info(f"Ensuring that {len(groups)} group(s) are registered") + logger.info("Ensuring that %s group(s) are registered", len(groups)) desired_group_names = [group.name for group in groups] current_group_names = [ item.name @@ -143,14 +155,15 @@ def update_groups(self, groups: list[LDAPGroup]) -> None: ] # Add groups logger.debug( - f"There are {len(current_group_names)} group(s) currently registered", + "There are %s group(s) currently registered", + len(current_group_names), ) group_names_to_add = [ group_name for group_name in desired_group_names if group_name not in current_group_names ] - logger.debug(f"... {len(group_names_to_add)} group(s) will be added") + logger.debug("... %s group(s) will be added", len(group_names_to_add)) self.backend.add_all( [ GuacamoleEntity(name=group_name, type=GuacamoleEntityType.USER_GROUP) @@ -163,7 +176,7 @@ def update_groups(self, groups: list[LDAPGroup]) -> None: for group_name in current_group_names if group_name not in desired_group_names ] - logger.debug(f"... {len(group_names_to_remove)} group(s) will be removed") + logger.debug("... %s group(s) will be removed", len(group_names_to_remove)) for group_name in group_names_to_remove: self.backend.delete( GuacamoleEntity, @@ -177,8 +190,8 @@ def update_group_entities(self) -> None: group.entity_id for group in self.backend.query(GuacamoleUserGroup) ] logger.debug( - f"There are {len(current_user_group_entity_ids)}" - " user group entit(y|ies) currently registered", + "There are %s user group entit(y|ies) currently registered", + len(current_user_group_entity_ids), ) new_group_entity_ids = [ group.entity_id @@ -189,7 +202,8 @@ def update_group_entities(self) -> None: if group.entity_id not in current_user_group_entity_ids ] logger.debug( - f"... {len(new_group_entity_ids)} user group entit(y|ies) will be added", + "... %s user group entit(y|ies) will be added", + len(new_group_entity_ids), ) self.backend.add_all( [ @@ -207,7 +221,10 @@ def update_group_entities(self) -> None: type=GuacamoleEntityType.USER_GROUP, ) ] - logger.debug(f"There are {len(valid_entity_ids)} valid user group entit(y|ies)") + logger.debug( + "There are %s valid user group entit(y|ies)", + len(valid_entity_ids), + ) self.backend.delete( GuacamoleUserGroup, GuacamoleUserGroup.entity_id.not_in(valid_entity_ids), @@ -216,7 +233,7 @@ def update_group_entities(self) -> None: def update_users(self, users: list[LDAPUser]) -> None: """Update the entities table with desired users.""" # Set users to desired list - logger.info(f"Ensuring that {len(users)} user(s) are registered") + logger.info("Ensuring that %s user(s) are registered", len(users)) desired_usernames = [user.name for user in users] current_usernames = [ user.name @@ -226,13 +243,16 @@ def update_users(self, users: list[LDAPUser]) -> None: ) ] # Add users - logger.debug(f"There are {len(current_usernames)} user(s) currently registered") + logger.debug( + "There are %s user(s) currently registered", + len(current_usernames), + ) usernames_to_add = [ username for username in desired_usernames if username not in current_usernames ] - logger.debug(f"... {len(usernames_to_add)} user(s) will be added") + logger.debug("... %s user(s) will be added", len(usernames_to_add)) self.backend.add_all( [ GuacamoleEntity(name=username, type=GuacamoleEntityType.USER) @@ -245,7 +265,7 @@ def update_users(self, users: list[LDAPUser]) -> None: for username in current_usernames if username not in desired_usernames ] - logger.debug(f"... {len(usernames_to_remove)} user(s) will be removed") + logger.debug("... %s user(s) will be removed", len(usernames_to_remove)) for username in usernames_to_remove: self.backend.delete( GuacamoleEntity, @@ -259,8 +279,8 @@ def update_user_entities(self, users: list[LDAPUser]) -> None: user.entity_id for user in self.backend.query(GuacamoleUser) ] logger.debug( - f"There are {len(current_user_entity_ids)} " - "user entit(y|ies) currently registered", + "There are %s user entit(y|ies) currently registered", + len(current_user_entity_ids), ) new_user_tuples: list[tuple[int, LDAPUser]] = [ (user.entity_id, [u for u in users if u.name == user.name][0]) @@ -271,7 +291,8 @@ def update_user_entities(self, users: list[LDAPUser]) -> None: if user.entity_id not in current_user_entity_ids ] logger.debug( - f"... {len(current_user_entity_ids)} user entit(y|ies) will be added", + "... %s user entit(y|ies) will be added", + len(current_user_entity_ids), ) self.backend.add_all( [ @@ -293,7 +314,7 @@ def update_user_entities(self, users: list[LDAPUser]) -> None: type=GuacamoleEntityType.USER, ) ] - logger.debug(f"There are {len(valid_entity_ids)} valid user entit(y|ies)") + logger.debug("There are %s valid user entit(y|ies)", len(valid_entity_ids)) self.backend.delete( GuacamoleUser, GuacamoleUser.entity_id.not_in(valid_entity_ids), diff --git a/guacamole_user_sync/postgresql/sql.py b/guacamole_user_sync/postgresql/sql.py index 2584001..3f6b80d 100644 --- a/guacamole_user_sync/postgresql/sql.py +++ b/guacamole_user_sync/postgresql/sql.py @@ -20,7 +20,7 @@ class GuacamoleSchema: @classmethod def commands(cls, schema_version: SchemaVersion) -> list[TextClause]: - logger.info(f"Ensuring correct schema for Guacamole {schema_version.value}") + logger.info("Ensuring correct schema for Guacamole %s", schema_version.value) commands = [] sql_file_path = Path(__file__).with_name( f"guacamole_schema.{schema_version.value}.sql", @@ -37,7 +37,7 @@ def commands(cls, schema_version: SchemaVersion) -> list[TextClause]: if isinstance(token, sqlparse.sql.Comment) ] if first_comment := next(filter(lambda item: item, comment_lines), None): - logger.debug(f"... {first_comment}") + logger.debug("... %s", first_comment) # Extract the command commands.append( text(sqlparse.format(statement, strip_comments=True, compact=True)), diff --git a/pyproject.toml b/pyproject.toml index fd33c4a..c95391b 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -114,6 +114,7 @@ select = [ "F", # pyflakes "FA", # flake8-future-annotations "FBT", # flake8-boolean-trap + "G", # flake8-logging-format "I", # isort "ICN", # flake8-import-conventions "ISC", # flake8-implicit-str-concat diff --git a/synchronise.py b/synchronise.py index 4a8c157..fb4c34e 100644 --- a/synchronise.py +++ b/synchronise.py @@ -61,7 +61,7 @@ def main( ) # Wait before repeating - logger.info(f"Waiting {repeat_interval} seconds.") + logger.info("Waiting %s seconds.", repeat_interval) time.sleep(repeat_interval) diff --git a/tests/test_postgresql.py b/tests/test_postgresql.py index c86d721..e629d99 100644 --- a/tests/test_postgresql.py +++ b/tests/test_postgresql.py @@ -301,7 +301,7 @@ def test_assign_users_to_groups_missing_user_entity( ) for output_line in ( - "Could not find entity ID for LDAP user aulus.agerius", + "Could not find entity ID for LDAP user 'aulus.agerius'", ): assert output_line in caplog.text From 6324fe54b48c85505e42e884eef427459450ac21 Mon Sep 17 00:00:00 2001 From: James Robinson Date: Thu, 26 Sep 2024 09:57:01 +0100 Subject: [PATCH 18/39] :white_check_mark: Add flake8-no-pep420 validation --- pyproject.toml | 1 + 1 file changed, 1 insertion(+) diff --git a/pyproject.toml b/pyproject.toml index c95391b..49f2eba 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -117,6 +117,7 @@ select = [ "G", # flake8-logging-format "I", # isort "ICN", # flake8-import-conventions + "INP", # flake8-no-pep420 "ISC", # flake8-implicit-str-concat "LOG", # flake8-logging "N", # pep8-naming From 3cf98a3b88184390c09d72a04f516f8a28e265ca Mon Sep 17 00:00:00 2001 From: James Robinson Date: Thu, 26 Sep 2024 09:58:55 +0100 Subject: [PATCH 19/39] :white_check_mark: Add flake8-pie validation --- guacamole_user_sync/models/exceptions.py | 4 ---- guacamole_user_sync/postgresql/orm.py | 2 -- pyproject.toml | 1 + 3 files changed, 1 insertion(+), 6 deletions(-) diff --git a/guacamole_user_sync/models/exceptions.py b/guacamole_user_sync/models/exceptions.py index a56cce6..dc589c7 100644 --- a/guacamole_user_sync/models/exceptions.py +++ b/guacamole_user_sync/models/exceptions.py @@ -1,10 +1,6 @@ class LDAPError(Exception): """LDAP error.""" - pass - class PostgreSQLError(Exception): """PostgreSQL error.""" - - pass diff --git a/guacamole_user_sync/postgresql/orm.py b/guacamole_user_sync/postgresql/orm.py index ec94a28..9558d77 100644 --- a/guacamole_user_sync/postgresql/orm.py +++ b/guacamole_user_sync/postgresql/orm.py @@ -20,8 +20,6 @@ class GuacamoleEntityType(enum.Enum): class GuacamoleBase(DeclarativeBase): # type:ignore """Guacamole database base table.""" - pass - class GuacamoleEntity(GuacamoleBase): """Guacamole database GuacamoleEntity table.""" diff --git a/pyproject.toml b/pyproject.toml index 49f2eba..56f5771 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -121,6 +121,7 @@ select = [ "ISC", # flake8-implicit-str-concat "LOG", # flake8-logging "N", # pep8-naming + "PIE", # flake8-pie "S", # flake8-bandit "UP", # pyupgrade "W", # pycodestyle warnings From 1b81e748a7261ac2a02a90767faee70bac5b28c2 Mon Sep 17 00:00:00 2001 From: James Robinson Date: Thu, 26 Sep 2024 10:04:49 +0100 Subject: [PATCH 20/39] :white_check_mark: Add flake8-print validation --- pyproject.toml | 1 + tests/mocks.py | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index 56f5771..d055a3d 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -123,6 +123,7 @@ select = [ "N", # pep8-naming "PIE", # flake8-pie "S", # flake8-bandit + "T20", # flake8-print "UP", # pyupgrade "W", # pycodestyle warnings "YTT", # flake8-2020 diff --git a/tests/mocks.py b/tests/mocks.py index 28e7496..ca9564d 100644 --- a/tests/mocks.py +++ b/tests/mocks.py @@ -77,7 +77,7 @@ def delete(self, *args: Any, **kwargs: Any) -> None: # noqa: ANN401 def execute_commands(self, commands: list[TextClause]) -> None: for command in commands: - print(f"Executing {command}") + print(f"Executing {command}") # noqa: T201 def query(self, table: type[T], **filter_kwargs: Any) -> list[T]: # noqa: ANN401 if table not in self.contents: From 83f006081f170ddc22b3dfeafe19fdf1699985b8 Mon Sep 17 00:00:00 2001 From: James Robinson Date: Thu, 26 Sep 2024 10:05:30 +0100 Subject: [PATCH 21/39] :white_check_mark: Add flake8-pyi validation --- pyproject.toml | 1 + 1 file changed, 1 insertion(+) diff --git a/pyproject.toml b/pyproject.toml index d055a3d..5afc572 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -122,6 +122,7 @@ select = [ "LOG", # flake8-logging "N", # pep8-naming "PIE", # flake8-pie + "PYI", # flake8-pyi "S", # flake8-bandit "T20", # flake8-print "UP", # pyupgrade From b9ec91dee61d3e5c2fa1c86429e9ee4bfca7eb62 Mon Sep 17 00:00:00 2001 From: James Robinson Date: Thu, 26 Sep 2024 10:06:15 +0100 Subject: [PATCH 22/39] :white_check_mark: Add flake8-pytest-style validation --- pyproject.toml | 1 + 1 file changed, 1 insertion(+) diff --git a/pyproject.toml b/pyproject.toml index 5afc572..6eb5d06 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -122,6 +122,7 @@ select = [ "LOG", # flake8-logging "N", # pep8-naming "PIE", # flake8-pie + "PT", # flake8-pytest-style "PYI", # flake8-pyi "S", # flake8-bandit "T20", # flake8-print From c9a43f8f350bcdfb2163f5a740a893d40e555f65 Mon Sep 17 00:00:00 2001 From: James Robinson Date: Thu, 26 Sep 2024 10:07:46 +0100 Subject: [PATCH 23/39] :white_check_mark: Add flake8-quotes validation --- pyproject.toml | 1 + 1 file changed, 1 insertion(+) diff --git a/pyproject.toml b/pyproject.toml index 6eb5d06..060eb97 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -124,6 +124,7 @@ select = [ "PIE", # flake8-pie "PT", # flake8-pytest-style "PYI", # flake8-pyi + "Q", # flake8-quotes "S", # flake8-bandit "T20", # flake8-print "UP", # pyupgrade From 6b75e8387ffd801844efad4687dc261fc138ae82 Mon Sep 17 00:00:00 2001 From: James Robinson Date: Thu, 26 Sep 2024 10:08:25 +0100 Subject: [PATCH 24/39] :white_check_mark: Add flake8-raise validation --- pyproject.toml | 1 + 1 file changed, 1 insertion(+) diff --git a/pyproject.toml b/pyproject.toml index 060eb97..866a610 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -125,6 +125,7 @@ select = [ "PT", # flake8-pytest-style "PYI", # flake8-pyi "Q", # flake8-quotes + "RSE", # flake8-rse "S", # flake8-bandit "T20", # flake8-print "UP", # pyupgrade From b6434284f4d53b37662a68c3f9215343f1791d73 Mon Sep 17 00:00:00 2001 From: James Robinson Date: Thu, 26 Sep 2024 10:08:50 +0100 Subject: [PATCH 25/39] :white_check_mark: Add flake8-return validation --- pyproject.toml | 1 + 1 file changed, 1 insertion(+) diff --git a/pyproject.toml b/pyproject.toml index 866a610..5cb81c4 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -125,6 +125,7 @@ select = [ "PT", # flake8-pytest-style "PYI", # flake8-pyi "Q", # flake8-quotes + "RET", # flake8-return "RSE", # flake8-rse "S", # flake8-bandit "T20", # flake8-print From 79bf7c0f179c808d415fb9d8fb9fa6b1ee0a0727 Mon Sep 17 00:00:00 2001 From: James Robinson Date: Thu, 26 Sep 2024 10:09:19 +0100 Subject: [PATCH 26/39] :white_check_mark: Add flake8-self validation --- pyproject.toml | 1 + 1 file changed, 1 insertion(+) diff --git a/pyproject.toml b/pyproject.toml index 5cb81c4..36c2557 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -128,6 +128,7 @@ select = [ "RET", # flake8-return "RSE", # flake8-rse "S", # flake8-bandit + "SLF", # flake8-self "T20", # flake8-print "UP", # pyupgrade "W", # pycodestyle warnings From 04c1834df8a480dbe2e15f3a3775e7e714a7b176 Mon Sep 17 00:00:00 2001 From: James Robinson Date: Thu, 26 Sep 2024 10:12:11 +0100 Subject: [PATCH 27/39] :white_check_mark: Add flake8-simplify validation --- .../postgresql/postgresql_backend.py | 34 ++++++++----------- pyproject.toml | 1 + 2 files changed, 16 insertions(+), 19 deletions(-) diff --git a/guacamole_user_sync/postgresql/postgresql_backend.py b/guacamole_user_sync/postgresql/postgresql_backend.py index b3d7736..a5e58da 100644 --- a/guacamole_user_sync/postgresql/postgresql_backend.py +++ b/guacamole_user_sync/postgresql/postgresql_backend.py @@ -53,33 +53,29 @@ def session(self) -> Session: return Session(self.engine) def add_all(self, items: list[T]) -> None: - with self.session() as session: # type:ignore - with session.begin(): - session.add_all(items) + with self.session() as session, session.begin(): # type:ignore + session.add_all(items) def delete(self, table: type[T], *filter_args: Any) -> None: # noqa: ANN401 - with self.session() as session: # type:ignore - with session.begin(): - if filter_args: - session.query(table).filter(*filter_args).delete() - else: - session.query(table).delete() + with self.session() as session, session.begin(): # type:ignore + if filter_args: + session.query(table).filter(*filter_args).delete() + else: + session.query(table).delete() def execute_commands(self, commands: list[TextClause]) -> None: try: - with self.session() as session: # type:ignore - with session.begin(): - for command in commands: - session.execute(command) + with self.session() as session, session.begin(): # type:ignore + for command in commands: + session.execute(command) except SQLAlchemyError: logger.warning("Unable to execute PostgreSQL commands.") raise def query(self, table: type[T], **filter_kwargs: Any) -> list[T]: # noqa: ANN401 - with self.session() as session: # type:ignore - with session.begin(): - if filter_kwargs: - result = session.query(table).filter_by(**filter_kwargs) - else: - result = session.query(table) + with self.session() as session, session.begin(): # type:ignore + if filter_kwargs: + result = session.query(table).filter_by(**filter_kwargs) + else: + result = session.query(table) return list(result) diff --git a/pyproject.toml b/pyproject.toml index 36c2557..65802d2 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -128,6 +128,7 @@ select = [ "RET", # flake8-return "RSE", # flake8-rse "S", # flake8-bandit + "SIM", # flake8-simplify "SLF", # flake8-self "T20", # flake8-print "UP", # pyupgrade From 79640d3b037310e2ce5ab449097c801334faf115 Mon Sep 17 00:00:00 2001 From: James Robinson Date: Thu, 26 Sep 2024 10:12:50 +0100 Subject: [PATCH 28/39] :white_check_mark: Add flake8-tidy-imports validation --- pyproject.toml | 1 + 1 file changed, 1 insertion(+) diff --git a/pyproject.toml b/pyproject.toml index 65802d2..0385ded 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -131,6 +131,7 @@ select = [ "SIM", # flake8-simplify "SLF", # flake8-self "T20", # flake8-print + "TID", # flake8-tidy-imports "UP", # pyupgrade "W", # pycodestyle warnings "YTT", # flake8-2020 From db71231a9e383d934051b9ea207269032965a417 Mon Sep 17 00:00:00 2001 From: James Robinson Date: Thu, 26 Sep 2024 10:13:24 +0100 Subject: [PATCH 29/39] :white_check_mark: Add flake8-type-checking validation --- pyproject.toml | 1 + 1 file changed, 1 insertion(+) diff --git a/pyproject.toml b/pyproject.toml index 0385ded..3164120 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -131,6 +131,7 @@ select = [ "SIM", # flake8-simplify "SLF", # flake8-self "T20", # flake8-print + "TCH", # flake8-type-checking "TID", # flake8-tidy-imports "UP", # pyupgrade "W", # pycodestyle warnings From 2d43bb1cf01104af56d60fed1a1ae78cc2453d8f Mon Sep 17 00:00:00 2001 From: James Robinson Date: Thu, 26 Sep 2024 10:14:04 +0100 Subject: [PATCH 30/39] :white_check_mark: Add flake8-gettext validation --- pyproject.toml | 1 + 1 file changed, 1 insertion(+) diff --git a/pyproject.toml b/pyproject.toml index 3164120..2e834f1 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -118,6 +118,7 @@ select = [ "I", # isort "ICN", # flake8-import-conventions "INP", # flake8-no-pep420 + "INT", # flake8-gettext "ISC", # flake8-implicit-str-concat "LOG", # flake8-logging "N", # pep8-naming From 596185866d5b59daab306506b38915718a1ed837 Mon Sep 17 00:00:00 2001 From: James Robinson Date: Thu, 26 Sep 2024 10:57:46 +0100 Subject: [PATCH 31/39] :white_check_mark: Add flake8-unused-arguments validation --- pyproject.toml | 1 + tests/mocks.py | 44 ++++++++++++++++++++++++---------------- tests/test_ldap.py | 6 +++--- tests/test_postgresql.py | 28 +++++++++++++------------ 4 files changed, 46 insertions(+), 33 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 2e834f1..6c6be91 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -103,6 +103,7 @@ ignore_missing_imports = true select = [ "A", # flake8-builtins "ANN", # flake8-annotations + "ARG", # flake8-unused-arguments "B", # flake8-bugbear "C4", # flake8-comprehensions "C90", # McCabe complexity diff --git a/tests/mocks.py b/tests/mocks.py index ca9564d..6eca753 100644 --- a/tests/mocks.py +++ b/tests/mocks.py @@ -1,9 +1,10 @@ -from typing import Any, Generic, TypeVar +from typing import Any import ldap from sqlalchemy.sql.expression import TextClause from guacamole_user_sync.models import LDAPSearchResult +from guacamole_user_sync.postgresql.orm import GuacamoleBase class MockLDAPObject: @@ -28,16 +29,24 @@ def __init__( self, partial: bool, # noqa: FBT001 results: LDAPSearchResult, - *args: Any, # noqa: ANN401 - **kwargs: Any, # noqa: ANN401 + *args: Any, # noqa: ANN401, ARG002 + **kwargs: Any, # noqa: ANN401, ARG002 ) -> None: self.allResults = results self.partial = partial - def startSearch(self, *args: Any, **kwargs: Any) -> None: # noqa: ANN401, N802 + def startSearch( # noqa: N802 + self, + *args: Any, # noqa: ANN401, ARG002 + **kwargs: Any, # noqa: ANN401, ARG002 + ) -> None: pass - def processResults(self, *args: Any, **kwargs: Any) -> bool: # noqa: ANN401, N802 + def processResults( # noqa: N802 + self, + *args: Any, # noqa: ANN401, ARG002 + **kwargs: Any, # noqa: ANN401, ARG002 + ) -> bool: return self.partial @@ -55,44 +64,45 @@ def __init__(self, results: LDAPSearchResult) -> None: super().__init__(results=results, partial=True) -T = TypeVar("T") - - -class MockPostgreSQLBackend(Generic[T]): +class MockPostgreSQLBackend: """Mock PostgreSQLBackend.""" - def __init__(self, *data_lists: Any, **kwargs: Any) -> None: # noqa: ANN401 - self.contents: dict[type[T], list[T]] = {} + def __init__(self, *data_lists: Any, **kwargs: Any) -> None: # noqa: ANN401, ARG002 + self.contents: dict[type[GuacamoleBase], list[GuacamoleBase]] = {} for data_list in data_lists: self.add_all(data_list) - def add_all(self, items: list[T]) -> None: + def add_all(self, items: list[GuacamoleBase]) -> None: cls = type(items[0]) if cls not in self.contents: self.contents[cls] = [] self.contents[cls] += items - def delete(self, *args: Any, **kwargs: Any) -> None: # noqa: ANN401 + def delete(self, *args: Any, **kwargs: Any) -> None: # noqa: ANN401, ARG001 pass def execute_commands(self, commands: list[TextClause]) -> None: for command in commands: print(f"Executing {command}") # noqa: T201 - def query(self, table: type[T], **filter_kwargs: Any) -> list[T]: # noqa: ANN401 + def query( + self, + table: type[GuacamoleBase], + **filter_kwargs: Any, # noqa: ANN401 + ) -> list[GuacamoleBase]: if table not in self.contents: self.contents[table] = [] results = list(self.contents[table]) if "entity_id" in filter_kwargs: results = [ - item for item in results if item.entity_id == filter_kwargs["entity_id"] # type: ignore + item for item in results if item.entity_id == filter_kwargs["entity_id"] ] if "name" in filter_kwargs: - results = [item for item in results if item.name == filter_kwargs["name"]] # type: ignore + results = [item for item in results if item.name == filter_kwargs["name"]] if "type" in filter_kwargs: - results = [item for item in results if item.type == filter_kwargs["type"]] # type: ignore + results = [item for item in results if item.type == filter_kwargs["type"]] return results diff --git a/tests/test_ldap.py b/tests/test_ldap.py index 15f8c86..af61854 100644 --- a/tests/test_ldap.py +++ b/tests/test_ldap.py @@ -79,7 +79,7 @@ def test_search_exception_server_down( monkeypatch: pytest.MonkeyPatch, caplog: pytest.LogCaptureFixture, ) -> None: - def mock_raise_server_down(*args: Any) -> None: # noqa: ANN401 + def mock_raise_server_down(*args: Any) -> None: # noqa: ANN401, ARG001 raise ldap.SERVER_DOWN monkeypatch.setattr( @@ -97,7 +97,7 @@ def test_search_exception_sizelimit_exceeded( monkeypatch: pytest.MonkeyPatch, caplog: pytest.LogCaptureFixture, ) -> None: - def mock_raise_sizelimit_exceeded(*args: Any) -> None: # noqa: ANN401 + def mock_raise_sizelimit_exceeded(*args: Any) -> None: # noqa: ANN401, ARG001 raise ldap.SIZELIMIT_EXCEEDED monkeypatch.setattr( @@ -132,7 +132,7 @@ def test_search_no_results( monkeypatch: pytest.MonkeyPatch, caplog: pytest.LogCaptureFixture, ) -> None: - def mock_raise_no_results(*args: Any) -> None: # noqa: ANN401 + def mock_raise_no_results(*args: Any) -> None: # noqa: ANN401, ARG001 raise ldap.NO_SUCH_OBJECT monkeypatch.setattr(ldap.asyncsearch.List, "startSearch", mock_raise_no_results) diff --git a/tests/test_postgresql.py b/tests/test_postgresql.py index e629d99..bf798d1 100644 --- a/tests/test_postgresql.py +++ b/tests/test_postgresql.py @@ -207,7 +207,7 @@ def test_assign_users_to_groups( postgresql_model_guacamoleusergroup_fixture: list[GuacamoleUserGroup], ) -> None: # Create a mock backend - mock_backend = MockPostgreSQLBackend( # type: ignore + mock_backend = MockPostgreSQLBackend( postgresql_model_guacamoleentity_fixture, postgresql_model_guacamoleusergroup_fixture, ) @@ -243,7 +243,7 @@ def test_assign_users_to_groups_missing_ldap_user( postgresql_model_guacamoleusergroup_fixture: list[GuacamoleUserGroup], ) -> None: # Create a mock backend - mock_backend = MockPostgreSQLBackend( # type: ignore + mock_backend = MockPostgreSQLBackend( postgresql_model_guacamoleentity_fixture, postgresql_model_guacamoleusergroup_fixture, ) @@ -279,7 +279,7 @@ def test_assign_users_to_groups_missing_user_entity( postgresql_model_guacamoleusergroup_fixture: list[GuacamoleUserGroup], ) -> None: # Create a mock backend - mock_backend = MockPostgreSQLBackend( # type: ignore + mock_backend = MockPostgreSQLBackend( postgresql_model_guacamoleentity_user_fixture[1:], postgresql_model_guacamoleentity_user_group_fixture, postgresql_model_guacamoleusergroup_fixture, @@ -314,7 +314,7 @@ def test_assign_users_to_groups_missing_usergroup( postgresql_model_guacamoleusergroup_fixture: list[GuacamoleUserGroup], ) -> None: # Create a mock backend - mock_backend = MockPostgreSQLBackend( # type: ignore + mock_backend = MockPostgreSQLBackend( postgresql_model_guacamoleentity_fixture, postgresql_model_guacamoleusergroup_fixture[1:], ) @@ -343,7 +343,7 @@ def test_assign_users_to_groups_missing_usergroup( def test_ensure_schema(self, capsys: pytest.CaptureFixture) -> None: # Create a mock backend - mock_backend = MockPostgreSQLBackend() # type: ignore + mock_backend = MockPostgreSQLBackend() # Patch PostgreSQLBackend with mock.patch( @@ -430,12 +430,14 @@ def test_ensure_schema(self, capsys: pytest.CaptureFixture) -> None: f"Executing CREATE INDEX IF NOT EXISTS {index_name}" in captured.out ) - def test_ensure_schema_exception(self, capsys: pytest.CaptureFixture) -> None: + def test_ensure_schema_exception(self) -> None: # Create a mock backend - def execute_commands_exception(commands: list[TextClause]) -> None: + def execute_commands_exception( + commands: list[TextClause], # noqa: ARG001 + ) -> None: raise OperationalError(statement="statement", params=None, orig=None) - mock_backend = MockPostgreSQLBackend() # type: ignore + mock_backend = MockPostgreSQLBackend() mock_backend.execute_commands = execute_commands_exception # type: ignore # Patch PostgreSQLBackend @@ -458,7 +460,7 @@ def test_update( ldap_model_users_fixture: list[LDAPUser], ) -> None: # Create a mock backend - mock_backend = MockPostgreSQLBackend() # type: ignore + mock_backend = MockPostgreSQLBackend() # Capture logs at debug level and above caplog.set_level(logging.DEBUG) @@ -511,7 +513,7 @@ def test_update_group_entities( postgresql_model_guacamoleusergroup_fixture: list[GuacamoleUserGroup], ) -> None: # Create a mock backend - mock_backend = MockPostgreSQLBackend( # type: ignore + mock_backend = MockPostgreSQLBackend( postgresql_model_guacamoleentity_user_group_fixture, postgresql_model_guacamoleusergroup_fixture[0:1], ) @@ -541,7 +543,7 @@ def test_update_groups( postgresql_model_guacamoleentity_user_group_fixture: list[GuacamoleEntity], ) -> None: # Create a mock backend - mock_backend = MockPostgreSQLBackend( # type: ignore + mock_backend = MockPostgreSQLBackend( postgresql_model_guacamoleentity_user_group_fixture[0:1], [ GuacamoleEntity( @@ -576,7 +578,7 @@ def test_update_user_entities( postgresql_model_guacamoleentity_user_fixture: list[GuacamoleEntity], ) -> None: # Create a mock backend - mock_backend = MockPostgreSQLBackend( # type: ignore + mock_backend = MockPostgreSQLBackend( postgresql_model_guacamoleentity_user_fixture, postgresql_model_guacamoleuser_fixture[0:1], ) @@ -606,7 +608,7 @@ def test_update_users( postgresql_model_guacamoleentity_user_fixture: list[GuacamoleEntity], ) -> None: # Create a mock backend - mock_backend = MockPostgreSQLBackend( # type: ignore + mock_backend = MockPostgreSQLBackend( postgresql_model_guacamoleentity_user_fixture[0:1], [ GuacamoleEntity( From ad911618b429102cbf6c9136c0f99961eb04886c Mon Sep 17 00:00:00 2001 From: James Robinson Date: Thu, 26 Sep 2024 11:20:16 +0100 Subject: [PATCH 32/39] :white_check_mark: Add flake8-use-pathlib validation --- guacamole_user_sync/postgresql/sql.py | 2 +- pyproject.toml | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/guacamole_user_sync/postgresql/sql.py b/guacamole_user_sync/postgresql/sql.py index 3f6b80d..a4d4c9b 100644 --- a/guacamole_user_sync/postgresql/sql.py +++ b/guacamole_user_sync/postgresql/sql.py @@ -25,7 +25,7 @@ def commands(cls, schema_version: SchemaVersion) -> list[TextClause]: sql_file_path = Path(__file__).with_name( f"guacamole_schema.{schema_version.value}.sql", ) - with open(sql_file_path) as f_sql: + with Path.open(sql_file_path) as f_sql: statements = sqlparse.split(f_sql.read()) for statement in statements: # Extract the first comment if there is one diff --git a/pyproject.toml b/pyproject.toml index 6c6be91..57e61f5 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -125,6 +125,7 @@ select = [ "N", # pep8-naming "PIE", # flake8-pie "PT", # flake8-pytest-style + "PTH", # flake8-use-pathlib "PYI", # flake8-pyi "Q", # flake8-quotes "RET", # flake8-return From 2e6822cebab026c8aa889a96818cc9c767ddc1be Mon Sep 17 00:00:00 2001 From: James Robinson Date: Thu, 26 Sep 2024 12:37:51 +0100 Subject: [PATCH 33/39] :wrench: Update mypy sqlalchemy checking --- guacamole_user_sync/postgresql/orm.py | 15 +++------ .../postgresql/postgresql_backend.py | 32 +++++++++++-------- guacamole_user_sync/postgresql/sql.py | 3 +- pyproject.toml | 3 +- tests/mocks.py | 2 +- tests/test_postgresql.py | 12 +++---- 6 files changed, 32 insertions(+), 35 deletions(-) diff --git a/guacamole_user_sync/postgresql/orm.py b/guacamole_user_sync/postgresql/orm.py index 9558d77..3fece71 100644 --- a/guacamole_user_sync/postgresql/orm.py +++ b/guacamole_user_sync/postgresql/orm.py @@ -1,13 +1,8 @@ import enum from datetime import datetime -from sqlalchemy import DateTime, Enum, Integer, String -from sqlalchemy.dialects.postgresql import BYTEA -from sqlalchemy.orm import ( # type:ignore - DeclarativeBase, - Mapped, - mapped_column, -) +from sqlalchemy import DateTime, Enum, Integer, LargeBinary, String +from sqlalchemy.orm import DeclarativeBase, Mapped, mapped_column class GuacamoleEntityType(enum.Enum): @@ -17,7 +12,7 @@ class GuacamoleEntityType(enum.Enum): USER_GROUP = "USER_GROUP" -class GuacamoleBase(DeclarativeBase): # type:ignore +class GuacamoleBase(DeclarativeBase): # type: ignore[misc] """Guacamole database base table.""" @@ -39,8 +34,8 @@ class GuacamoleUser(GuacamoleBase): user_id: Mapped[int] = mapped_column(Integer, primary_key=True) entity_id: Mapped[int] = mapped_column(Integer) full_name: Mapped[str] = mapped_column(String(256)) - password_hash: Mapped[bytes] = mapped_column(BYTEA) - password_salt: Mapped[bytes] = mapped_column(BYTEA) + password_hash: Mapped[bytes] = mapped_column(LargeBinary) + password_salt: Mapped[bytes] = mapped_column(LargeBinary) password_date: Mapped[datetime] = mapped_column(DateTime(timezone=True)) diff --git a/guacamole_user_sync/postgresql/postgresql_backend.py b/guacamole_user_sync/postgresql/postgresql_backend.py index a5e58da..d0ebbc5 100644 --- a/guacamole_user_sync/postgresql/postgresql_backend.py +++ b/guacamole_user_sync/postgresql/postgresql_backend.py @@ -1,16 +1,12 @@ import logging -from typing import Any, TypeVar +from typing import Any -from sqlalchemy import create_engine -from sqlalchemy.engine import URL, Engine # type:ignore +from sqlalchemy import URL, Engine, TextClause, create_engine from sqlalchemy.exc import SQLAlchemyError -from sqlalchemy.orm import Session -from sqlalchemy.sql.expression import TextClause +from sqlalchemy.orm import DeclarativeBase, Session logger = logging.getLogger("guacamole_user_sync") -T = TypeVar("T") - class PostgreSQLBackend: """Backend for connecting to a PostgreSQL database.""" @@ -52,12 +48,16 @@ def session(self) -> Session: return self._session return Session(self.engine) - def add_all(self, items: list[T]) -> None: - with self.session() as session, session.begin(): # type:ignore + def add_all(self, items: list[DeclarativeBase]) -> None: + with self.session() as session, session.begin(): session.add_all(items) - def delete(self, table: type[T], *filter_args: Any) -> None: # noqa: ANN401 - with self.session() as session, session.begin(): # type:ignore + def delete( + self, + table: type[DeclarativeBase], + *filter_args: Any, # noqa: ANN401 + ) -> None: + with self.session() as session, session.begin(): if filter_args: session.query(table).filter(*filter_args).delete() else: @@ -65,15 +65,19 @@ def delete(self, table: type[T], *filter_args: Any) -> None: # noqa: ANN401 def execute_commands(self, commands: list[TextClause]) -> None: try: - with self.session() as session, session.begin(): # type:ignore + with self.session() as session, session.begin(): for command in commands: session.execute(command) except SQLAlchemyError: logger.warning("Unable to execute PostgreSQL commands.") raise - def query(self, table: type[T], **filter_kwargs: Any) -> list[T]: # noqa: ANN401 - with self.session() as session, session.begin(): # type:ignore + def query( + self, + table: type[DeclarativeBase], + **filter_kwargs: Any, # noqa: ANN401 + ) -> list[DeclarativeBase]: + with self.session() as session, session.begin(): if filter_kwargs: result = session.query(table).filter_by(**filter_kwargs) else: diff --git a/guacamole_user_sync/postgresql/sql.py b/guacamole_user_sync/postgresql/sql.py index a4d4c9b..c4df5bc 100644 --- a/guacamole_user_sync/postgresql/sql.py +++ b/guacamole_user_sync/postgresql/sql.py @@ -3,8 +3,7 @@ from pathlib import Path import sqlparse -from sqlalchemy import text -from sqlalchemy.sql.expression import TextClause +from sqlalchemy import TextClause, text logger = logging.getLogger("guacamole_user_sync") diff --git a/pyproject.toml b/pyproject.toml index 57e61f5..bcb82c6 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -37,7 +37,6 @@ lint = [ "black==24.8.0", "mypy==1.11.2", "ruff==0.6.2", - "types-SQLAlchemy==1.4.53.38", ] test = [ "coverage[toml]==7.6.1", @@ -95,6 +94,7 @@ strict = true # enable all optional error checking flags module = [ "ldap.*", "pytest.*", + "sqlalchemy.*", "sqlparse.*", ] ignore_missing_imports = true @@ -123,6 +123,7 @@ select = [ "ISC", # flake8-implicit-str-concat "LOG", # flake8-logging "N", # pep8-naming + "PGH", # pygrep-hooks "PIE", # flake8-pie "PT", # flake8-pytest-style "PTH", # flake8-use-pathlib diff --git a/tests/mocks.py b/tests/mocks.py index 6eca753..e6ba6be 100644 --- a/tests/mocks.py +++ b/tests/mocks.py @@ -1,7 +1,7 @@ from typing import Any import ldap -from sqlalchemy.sql.expression import TextClause +from sqlalchemy import TextClause from guacamole_user_sync.models import LDAPSearchResult from guacamole_user_sync.postgresql.orm import GuacamoleBase diff --git a/tests/test_postgresql.py b/tests/test_postgresql.py index bf798d1..e34b43f 100644 --- a/tests/test_postgresql.py +++ b/tests/test_postgresql.py @@ -3,14 +3,12 @@ from unittest import mock import pytest -from sqlalchemy import text +from sqlalchemy import URL, Engine, text from sqlalchemy.dialects.postgresql.psycopg import PGDialect_psycopg -from sqlalchemy.engine import URL, Engine # type: ignore from sqlalchemy.exc import OperationalError from sqlalchemy.orm import Session -from sqlalchemy.pool.impl import QueuePool -from sqlalchemy.sql.elements import BinaryExpression -from sqlalchemy.sql.expression import TextClause +from sqlalchemy.pool import QueuePool +from sqlalchemy.sql.elements import BinaryExpression, TextClause from guacamole_user_sync.models import LDAPGroup, LDAPUser, PostgreSQLError from guacamole_user_sync.postgresql import PostgreSQLBackend, PostgreSQLClient @@ -62,7 +60,7 @@ def test_engine(self) -> None: assert isinstance(backend.engine.url, URL) assert backend.engine.logging_name is None assert not backend.engine.echo - assert not backend.engine.hide_parameters # type: ignore + assert not backend.engine.hide_parameters def test_session(self) -> None: backend = self.mock_backend() @@ -438,7 +436,7 @@ def execute_commands_exception( raise OperationalError(statement="statement", params=None, orig=None) mock_backend = MockPostgreSQLBackend() - mock_backend.execute_commands = execute_commands_exception # type: ignore + mock_backend.execute_commands = execute_commands_exception # type: ignore[method-assign] # Patch PostgreSQLBackend with mock.patch( From 100a9b625fc5983da3d70c22ebb26d336ae6d566 Mon Sep 17 00:00:00 2001 From: James Robinson Date: Thu, 26 Sep 2024 12:44:31 +0100 Subject: [PATCH 34/39] :white_check_mark: Add pylint validation --- guacamole_user_sync/postgresql/__init__.py | 3 +- .../postgresql/postgresql_backend.py | 34 +++++++++++-------- .../postgresql/postgresql_client.py | 14 ++++---- pyproject.toml | 1 + synchronise.py | 8 ++--- tests/test_postgresql.py | 31 ++++++++++------- 6 files changed, 53 insertions(+), 38 deletions(-) diff --git a/guacamole_user_sync/postgresql/__init__.py b/guacamole_user_sync/postgresql/__init__.py index 6ff288d..c5c4081 100644 --- a/guacamole_user_sync/postgresql/__init__.py +++ b/guacamole_user_sync/postgresql/__init__.py @@ -1,9 +1,10 @@ -from .postgresql_backend import PostgreSQLBackend +from .postgresql_backend import PostgreSQLBackend, PostgreSQLConnectionDetails from .postgresql_client import PostgreSQLClient from .sql import SchemaVersion __all__ = [ "PostgreSQLBackend", + "PostgreSQLConnectionDetails", "PostgreSQLClient", "SchemaVersion", ] diff --git a/guacamole_user_sync/postgresql/postgresql_backend.py b/guacamole_user_sync/postgresql/postgresql_backend.py index d0ebbc5..542dd97 100644 --- a/guacamole_user_sync/postgresql/postgresql_backend.py +++ b/guacamole_user_sync/postgresql/postgresql_backend.py @@ -1,4 +1,5 @@ import logging +from dataclasses import dataclass from typing import Any from sqlalchemy import URL, Engine, TextClause, create_engine @@ -8,24 +9,27 @@ logger = logging.getLogger("guacamole_user_sync") +@dataclass +class PostgreSQLConnectionDetails: + """Dataclass for holding PostgreSQL connection details.""" + + database_name: str + host_name: str + port: int + user_name: str + user_password: str + + class PostgreSQLBackend: """Backend for connecting to a PostgreSQL database.""" def __init__( self, *, - database_name: str, - host_name: str, - port: int, - user_name: str, - user_password: str, + connection_details: PostgreSQLConnectionDetails, session: Session | None = None, ) -> None: - self.database_name = database_name - self.host_name = host_name - self.port = port - self.user_name = user_name - self.user_password = user_password + self.connection_details = connection_details self._engine: Engine | None = None self._session = session @@ -34,11 +38,11 @@ def engine(self) -> Engine: if not self._engine: url_object = URL.create( "postgresql+psycopg", - username=self.user_name, - password=self.user_password, - host=self.host_name, - port=self.port, - database=self.database_name, + username=self.connection_details.user_name, + password=self.connection_details.user_password, + host=self.connection_details.host_name, + port=self.connection_details.port, + database=self.connection_details.database_name, ) self._engine = create_engine(url_object, echo=False) return self._engine diff --git a/guacamole_user_sync/postgresql/postgresql_client.py b/guacamole_user_sync/postgresql/postgresql_client.py index 2f7faf8..ce7295a 100644 --- a/guacamole_user_sync/postgresql/postgresql_client.py +++ b/guacamole_user_sync/postgresql/postgresql_client.py @@ -17,7 +17,7 @@ GuacamoleUserGroup, GuacamoleUserGroupMember, ) -from .postgresql_backend import PostgreSQLBackend +from .postgresql_backend import PostgreSQLBackend, PostgreSQLConnectionDetails from .sql import GuacamoleSchema, SchemaVersion logger = logging.getLogger("guacamole_user_sync") @@ -36,11 +36,13 @@ def __init__( user_password: str, ) -> None: self.backend = PostgreSQLBackend( - database_name=database_name, - host_name=host_name, - port=port, - user_name=user_name, - user_password=user_password, + connection_details=PostgreSQLConnectionDetails( + database_name=database_name, + host_name=host_name, + port=port, + user_name=user_name, + user_password=user_password, + ), ) def assign_users_to_groups( diff --git a/pyproject.toml b/pyproject.toml index bcb82c6..9c64a57 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -125,6 +125,7 @@ select = [ "N", # pep8-naming "PGH", # pygrep-hooks "PIE", # flake8-pie + "PL", # pylint "PT", # flake8-pytest-style "PTH", # flake8-use-pathlib "PYI", # flake8-pyi diff --git a/synchronise.py b/synchronise.py index fb4c34e..eb89731 100644 --- a/synchronise.py +++ b/synchronise.py @@ -8,7 +8,7 @@ from guacamole_user_sync.postgresql import PostgreSQLClient, SchemaVersion -def main( +def main( # noqa: PLR0913 ldap_bind_dn: str | None, ldap_bind_password: str | None, ldap_group_base_dn: str, @@ -134,14 +134,14 @@ def synchronise( ldap_group_filter=ldap_group_filter, ldap_group_name_attr=os.getenv("LDAP_GROUP_NAME_ATTR", "cn"), ldap_host=ldap_host, - ldap_port=int(os.getenv("LDAP_PORT", 389)), + ldap_port=int(os.getenv("LDAP_PORT", "389")), ldap_user_base_dn=ldap_user_base_dn, ldap_user_filter=ldap_user_filter, ldap_user_name_attr=os.getenv("LDAP_USER_NAME_ATTR", "userPrincipalName"), postgresql_database_name=os.getenv("POSTGRESQL_DB_NAME", "guacamole"), postgresql_host_name=postgresql_host_name, postgresql_password=postgresql_password, - postgresql_port=int(os.getenv("POSTGRESQL_PORT", 5432)), + postgresql_port=int(os.getenv("POSTGRESQL_PORT", "5432")), postgresql_user_name=postgresql_user_name, - repeat_interval=int(os.getenv("REPEAT_INTERVAL", 300)), + repeat_interval=int(os.getenv("REPEAT_INTERVAL", "300")), ) diff --git a/tests/test_postgresql.py b/tests/test_postgresql.py index e34b43f..989b2e5 100644 --- a/tests/test_postgresql.py +++ b/tests/test_postgresql.py @@ -11,7 +11,11 @@ from sqlalchemy.sql.elements import BinaryExpression, TextClause from guacamole_user_sync.models import LDAPGroup, LDAPUser, PostgreSQLError -from guacamole_user_sync.postgresql import PostgreSQLBackend, PostgreSQLClient +from guacamole_user_sync.postgresql import ( + PostgreSQLBackend, + PostgreSQLClient, + PostgreSQLConnectionDetails, +) from guacamole_user_sync.postgresql.orm import ( GuacamoleEntity, GuacamoleEntityType, @@ -28,11 +32,13 @@ class TestPostgreSQLBackend: def mock_backend(self, session: Session | None = None) -> PostgreSQLBackend: return PostgreSQLBackend( - database_name="database_name", - host_name="host_name", - port=1234, - user_name="user_name", - user_password="user_password", # noqa: S106 + connection_details=PostgreSQLConnectionDetails( + database_name="database_name", + host_name="host_name", + port=1234, + user_name="user_name", + user_password="user_password", # noqa: S106 + ), session=session, ) @@ -46,11 +52,12 @@ def mock_session(self) -> mock.MagicMock: def test_constructor(self) -> None: backend = self.mock_backend() assert isinstance(backend, PostgreSQLBackend) - assert backend.database_name == "database_name" - assert backend.host_name == "host_name" - assert backend.port == 1234 - assert backend.user_name == "user_name" - assert backend.user_password == "user_password" # noqa: S105 + assert isinstance(backend.connection_details, PostgreSQLConnectionDetails) + assert backend.connection_details.database_name == "database_name" + assert backend.connection_details.host_name == "host_name" + assert backend.connection_details.port == 1234 # noqa: PLR2004 + assert backend.connection_details.user_name == "user_name" + assert backend.connection_details.user_password == "user_password" # noqa: S105 def test_engine(self) -> None: backend = self.mock_backend() @@ -267,7 +274,7 @@ def test_assign_users_to_groups_missing_ldap_user( ): assert output_line in caplog.text - def test_assign_users_to_groups_missing_user_entity( + def test_assign_users_to_groups_missing_user_entity( # noqa: PLR0913 self, caplog: pytest.LogCaptureFixture, ldap_model_groups_fixture: list[LDAPGroup], From d3fa803d2d7c3eec9e93b1007a08184c85016d8a Mon Sep 17 00:00:00 2001 From: James Robinson Date: Thu, 26 Sep 2024 12:46:52 +0100 Subject: [PATCH 35/39] :white_check_mark: Add tryceratops validation --- guacamole_user_sync/ldap/ldap_client.py | 7 ++++--- pyproject.toml | 1 + 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/guacamole_user_sync/ldap/ldap_client.py b/guacamole_user_sync/ldap/ldap_client.py index 56279ac..c5400aa 100644 --- a/guacamole_user_sync/ldap/ldap_client.py +++ b/guacamole_user_sync/ldap/ldap_client.py @@ -91,9 +91,6 @@ def search(self, query: LDAPQuery) -> LDAPSearchResult: ) if searcher.processResults() != 0: logger.warning("Only partial results received.") - results = searcher.allResults - logger.debug("Server returned %s results.", len(results)) - return results except ldap.NO_SUCH_OBJECT as exc: logger.warning("Server returned no results.") raise LDAPError from exc @@ -103,3 +100,7 @@ def search(self, query: LDAPQuery) -> LDAPSearchResult: except ldap.SIZELIMIT_EXCEEDED as exc: logger.warning("Server-side size limit exceeded.") raise LDAPError from exc + else: + results = searcher.allResults + logger.debug("Server returned %s results.", len(results)) + return results diff --git a/pyproject.toml b/pyproject.toml index 9c64a57..8025143 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -138,6 +138,7 @@ select = [ "T20", # flake8-print "TCH", # flake8-type-checking "TID", # flake8-tidy-imports + "TRY", # tryceratops "UP", # pyupgrade "W", # pycodestyle warnings "YTT", # flake8-2020 From 085bdbd48f49d7011b5f052bfea689472d35388e Mon Sep 17 00:00:00 2001 From: James Robinson Date: Thu, 26 Sep 2024 12:47:43 +0100 Subject: [PATCH 36/39] :white_check_mark: Add flynt validation --- pyproject.toml | 1 + 1 file changed, 1 insertion(+) diff --git a/pyproject.toml b/pyproject.toml index 8025143..83f6fca 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -115,6 +115,7 @@ select = [ "F", # pyflakes "FA", # flake8-future-annotations "FBT", # flake8-boolean-trap + "FLY", # flynt "G", # flake8-logging-format "I", # isort "ICN", # flake8-import-conventions From 7d548d8c0309c1e8e5fa3a171c932348bb05b086 Mon Sep 17 00:00:00 2001 From: James Robinson Date: Thu, 26 Sep 2024 12:48:23 +0100 Subject: [PATCH 37/39] :white_check_mark: Add perflint validation --- pyproject.toml | 85 +++++++++++++++++++++++++------------------------- 1 file changed, 43 insertions(+), 42 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 83f6fca..87fcbe5 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -101,48 +101,49 @@ ignore_missing_imports = true [tool.ruff.lint] select = [ - "A", # flake8-builtins - "ANN", # flake8-annotations - "ARG", # flake8-unused-arguments - "B", # flake8-bugbear - "C4", # flake8-comprehensions - "C90", # McCabe complexity - "COM", # flake8-commas - "D", # pydocstyle - "DTZ", # flake8-datetimez - "E", # pycodestyle errors - "EM", # flake8-errmsg - "F", # pyflakes - "FA", # flake8-future-annotations - "FBT", # flake8-boolean-trap - "FLY", # flynt - "G", # flake8-logging-format - "I", # isort - "ICN", # flake8-import-conventions - "INP", # flake8-no-pep420 - "INT", # flake8-gettext - "ISC", # flake8-implicit-str-concat - "LOG", # flake8-logging - "N", # pep8-naming - "PGH", # pygrep-hooks - "PIE", # flake8-pie - "PL", # pylint - "PT", # flake8-pytest-style - "PTH", # flake8-use-pathlib - "PYI", # flake8-pyi - "Q", # flake8-quotes - "RET", # flake8-return - "RSE", # flake8-rse - "S", # flake8-bandit - "SIM", # flake8-simplify - "SLF", # flake8-self - "T20", # flake8-print - "TCH", # flake8-type-checking - "TID", # flake8-tidy-imports - "TRY", # tryceratops - "UP", # pyupgrade - "W", # pycodestyle warnings - "YTT", # flake8-2020 + "A", # flake8-builtins + "ANN", # flake8-annotations + "ARG", # flake8-unused-arguments + "B", # flake8-bugbear + "C4", # flake8-comprehensions + "C90", # McCabe complexity + "COM", # flake8-commas + "D", # pydocstyle + "DTZ", # flake8-datetimez + "E", # pycodestyle errors + "EM", # flake8-errmsg + "F", # pyflakes + "FA", # flake8-future-annotations + "FBT", # flake8-boolean-trap + "FLY", # flynt + "G", # flake8-logging-format + "I", # isort + "ICN", # flake8-import-conventions + "INP", # flake8-no-pep420 + "INT", # flake8-gettext + "ISC", # flake8-implicit-str-concat + "LOG", # flake8-logging + "N", # pep8-naming + "PERF", # perflint + "PGH", # pygrep-hooks + "PIE", # flake8-pie + "PL", # pylint + "PT", # flake8-pytest-style + "PTH", # flake8-use-pathlib + "PYI", # flake8-pyi + "Q", # flake8-quotes + "RET", # flake8-return + "RSE", # flake8-rse + "S", # flake8-bandit + "SIM", # flake8-simplify + "SLF", # flake8-self + "T20", # flake8-print + "TCH", # flake8-type-checking + "TID", # flake8-tidy-imports + "TRY", # tryceratops + "UP", # pyupgrade + "W", # pycodestyle warnings + "YTT", # flake8-2020 ] ignore = [ "ANN101", # missing-type-self [deprecated] From 90e05dee975a4f59f6e55881eb790f3a697bfd5b Mon Sep 17 00:00:00 2001 From: James Robinson Date: Thu, 26 Sep 2024 12:48:57 +0100 Subject: [PATCH 38/39] :white_check_mark: Add refurb validation --- pyproject.toml | 1 + 1 file changed, 1 insertion(+) diff --git a/pyproject.toml b/pyproject.toml index 87fcbe5..11f5a07 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -116,6 +116,7 @@ select = [ "FA", # flake8-future-annotations "FBT", # flake8-boolean-trap "FLY", # flynt + "FURB", # refurb "G", # flake8-logging-format "I", # isort "ICN", # flake8-import-conventions From 885ad8ac0a0e0ba511988cffc6c812fb74ca2221 Mon Sep 17 00:00:00 2001 From: James Robinson Date: Thu, 26 Sep 2024 14:16:22 +0100 Subject: [PATCH 39/39] :bug: Fix possible exception in update_user_entities --- guacamole_user_sync/models/__init__.py | 2 + guacamole_user_sync/models/guacamole.py | 10 ++++ .../postgresql/postgresql_backend.py | 13 +++-- .../postgresql/postgresql_client.py | 48 +++++++++++-------- pyproject.toml | 1 + tests/mocks.py | 6 +-- tests/test_postgresql.py | 2 +- 7 files changed, 52 insertions(+), 30 deletions(-) create mode 100644 guacamole_user_sync/models/guacamole.py diff --git a/guacamole_user_sync/models/__init__.py b/guacamole_user_sync/models/__init__.py index c20a684..9695664 100644 --- a/guacamole_user_sync/models/__init__.py +++ b/guacamole_user_sync/models/__init__.py @@ -1,10 +1,12 @@ from .exceptions import LDAPError, PostgreSQLError +from .guacamole import GuacamoleUserDetails from .ldap_objects import LDAPGroup, LDAPUser from .ldap_query import LDAPQuery LDAPSearchResult = list[tuple[int, tuple[str, dict[str, list[bytes]]]]] __all__ = [ + "GuacamoleUserDetails", "LDAPError", "LDAPGroup", "LDAPQuery", diff --git a/guacamole_user_sync/models/guacamole.py b/guacamole_user_sync/models/guacamole.py new file mode 100644 index 0000000..a1771ba --- /dev/null +++ b/guacamole_user_sync/models/guacamole.py @@ -0,0 +1,10 @@ +from dataclasses import dataclass + + +@dataclass +class GuacamoleUserDetails: + """A Guacamole user with required attributes only.""" + + entity_id: int + full_name: str + name: str diff --git a/guacamole_user_sync/postgresql/postgresql_backend.py b/guacamole_user_sync/postgresql/postgresql_backend.py index 542dd97..f939166 100644 --- a/guacamole_user_sync/postgresql/postgresql_backend.py +++ b/guacamole_user_sync/postgresql/postgresql_backend.py @@ -1,6 +1,6 @@ import logging from dataclasses import dataclass -from typing import Any +from typing import Any, TypeVar from sqlalchemy import URL, Engine, TextClause, create_engine from sqlalchemy.exc import SQLAlchemyError @@ -20,6 +20,9 @@ class PostgreSQLConnectionDetails: user_password: str +T = TypeVar("T", bound=DeclarativeBase) + + class PostgreSQLBackend: """Backend for connecting to a PostgreSQL database.""" @@ -52,13 +55,13 @@ def session(self) -> Session: return self._session return Session(self.engine) - def add_all(self, items: list[DeclarativeBase]) -> None: + def add_all(self, items: list[T]) -> None: with self.session() as session, session.begin(): session.add_all(items) def delete( self, - table: type[DeclarativeBase], + table: type[T], *filter_args: Any, # noqa: ANN401 ) -> None: with self.session() as session, session.begin(): @@ -78,9 +81,9 @@ def execute_commands(self, commands: list[TextClause]) -> None: def query( self, - table: type[DeclarativeBase], + table: type[T], **filter_kwargs: Any, # noqa: ANN401 - ) -> list[DeclarativeBase]: + ) -> list[T]: with self.session() as session, session.begin(): if filter_kwargs: result = session.query(table).filter_by(**filter_kwargs) diff --git a/guacamole_user_sync/postgresql/postgresql_client.py b/guacamole_user_sync/postgresql/postgresql_client.py index ce7295a..3086e7e 100644 --- a/guacamole_user_sync/postgresql/postgresql_client.py +++ b/guacamole_user_sync/postgresql/postgresql_client.py @@ -5,6 +5,7 @@ from sqlalchemy.exc import SQLAlchemyError from guacamole_user_sync.models import ( + GuacamoleUserDetails, LDAPGroup, LDAPUser, PostgreSQLError, @@ -60,27 +61,27 @@ def assign_users_to_groups( logger.debug("Working on group '%s'", group.name) # Get the user_group_id for each group (via looking up the entity_id) try: - group_entity_id = [ + group_entity_id = next( item.entity_id for item in self.backend.query( GuacamoleEntity, name=group.name, type=GuacamoleEntityType.USER_GROUP, ) - ][0] - user_group_id = [ + ) + user_group_id = next( item.user_group_id for item in self.backend.query( GuacamoleUserGroup, entity_id=group_entity_id, ) - ][0] + ) logger.debug( "-> entity_id: %s; user_group_id: %s", group_entity_id, user_group_id, ) - except IndexError: + except StopIteration: logger.debug( "Could not determine user_group_id for group '%s'.", group.name, @@ -94,20 +95,20 @@ def assign_users_to_groups( logger.debug("Could not find LDAP user with UID %s", user_uid) continue try: - user_entity_id = [ + user_entity_id = next( item.entity_id for item in self.backend.query( GuacamoleEntity, name=user.name, type=GuacamoleEntityType.USER, ) - ][0] + ) logger.debug( "... group member '%s' has entity_id '%s'", user, user_entity_id, ) - except IndexError: + except StopIteration: logger.debug( "Could not find entity ID for LDAP user '%s'", user_uid, @@ -284,28 +285,33 @@ def update_user_entities(self, users: list[LDAPUser]) -> None: "There are %s user entit(y|ies) currently registered", len(current_user_entity_ids), ) - new_user_tuples: list[tuple[int, LDAPUser]] = [ - (user.entity_id, [u for u in users if u.name == user.name][0]) - for user in self.backend.query( - GuacamoleEntity, - type=GuacamoleEntityType.USER, + user_entities = self.backend.query( + GuacamoleEntity, + type=GuacamoleEntityType.USER, + ) + new_users = [ + GuacamoleUserDetails( + entity_id=entity.entity_id, + full_name=user.display_name, + name=user.name, ) - if user.entity_id not in current_user_entity_ids + for user in users + for entity in user_entities + if entity.name == user.name + and entity.entity_id not in current_user_entity_ids ] - logger.debug( - "... %s user entit(y|ies) will be added", - len(current_user_entity_ids), - ) + logger.debug("... %s user entit(y|ies) will be added", len(new_users)) + self.backend.add_all( [ GuacamoleUser( - entity_id=user_tuple[0], - full_name=user_tuple[1].display_name, + entity_id=new_user.entity_id, + full_name=new_user.full_name, password_date=datetime.now(tz=UTC), password_hash=secrets.token_bytes(32), password_salt=secrets.token_bytes(32), ) - for user_tuple in new_user_tuples + for new_user in new_users ], ) # Clean up any unused entries diff --git a/pyproject.toml b/pyproject.toml index 11f5a07..cdfeecb 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -135,6 +135,7 @@ select = [ "Q", # flake8-quotes "RET", # flake8-return "RSE", # flake8-rse + "RUF", # Ruff-specific rules "S", # flake8-bandit "SIM", # flake8-simplify "SLF", # flake8-self diff --git a/tests/mocks.py b/tests/mocks.py index e6ba6be..02a7b24 100644 --- a/tests/mocks.py +++ b/tests/mocks.py @@ -37,8 +37,8 @@ def __init__( def startSearch( # noqa: N802 self, - *args: Any, # noqa: ANN401, ARG002 - **kwargs: Any, # noqa: ANN401, ARG002 + *args: Any, # noqa: ANN401 + **kwargs: Any, # noqa: ANN401 ) -> None: pass @@ -78,7 +78,7 @@ def add_all(self, items: list[GuacamoleBase]) -> None: self.contents[cls] = [] self.contents[cls] += items - def delete(self, *args: Any, **kwargs: Any) -> None: # noqa: ANN401, ARG001 + def delete(self, *args: Any, **kwargs: Any) -> None: # noqa: ANN401 pass def execute_commands(self, commands: list[TextClause]) -> None: diff --git a/tests/test_postgresql.py b/tests/test_postgresql.py index 989b2e5..3c8f6e4 100644 --- a/tests/test_postgresql.py +++ b/tests/test_postgresql.py @@ -494,7 +494,7 @@ def test_update( "... 3 user group entit(y|ies) will be added", "There are 3 valid user group entit(y|ies)", "There are 0 user entit(y|ies) currently registered", - "... 0 user entit(y|ies) will be added", + "... 2 user entit(y|ies) will be added", "There are 2 valid user entit(y|ies)", "Ensuring that 2 user(s) are correctly assigned among 3 group(s)", "Working on group 'defendants'",