From 55e34d4e4d3928f03f26f49054452706e8c3ae3f Mon Sep 17 00:00:00 2001 From: femalves Date: Tue, 13 Jan 2026 17:32:52 -0500 Subject: [PATCH 01/11] bumping to python 3.10 --- .github/workflows/python_actions.yml | 7 +- .gitignore | 6 + biblib/app.py | 4 + biblib/cli.py | 145 +++++++++++++++++ config.py => biblib/config.py | 0 biblib/manage.py | 147 ------------------ biblib/models.py | 1 + biblib/tests/base.py | 12 +- .../test_bb_and_classic_user_epic.py | 2 +- .../test_classic_user_epic.py | 2 +- biblib/tests/unit_tests/test_views.py | 18 ++- dev-requirements.txt | 14 -- pyproject.toml | 59 +++++++ pytest.ini | 3 - requirements.txt | 12 -- scripts/cronjob.sh | 6 +- 16 files changed, 247 insertions(+), 191 deletions(-) create mode 100644 biblib/cli.py rename config.py => biblib/config.py (100%) delete mode 100644 biblib/manage.py delete mode 100644 dev-requirements.txt create mode 100644 pyproject.toml delete mode 100644 pytest.ini delete mode 100644 requirements.txt diff --git a/.github/workflows/python_actions.yml b/.github/workflows/python_actions.yml index 9ca637d..4ad655f 100644 --- a/.github/workflows/python_actions.yml +++ b/.github/workflows/python_actions.yml @@ -11,13 +11,12 @@ jobs: - uses: actions/checkout@v2 - uses: actions/setup-python@v2 with: - python-version: 3.8 + python-version: '3.10' - name: Install dependencies run: | - python -m pip install --upgrade wheel setuptools==57 pip - pip install -U -r requirements.txt - pip install -U -r dev-requirements.txt + python -m pip install --upgrade pip setuptools==57.5.0 wheel + pip install ".[dev]" - name: Test with pytest run: | diff --git a/.gitignore b/.gitignore index cdda8fe..24598b8 100644 --- a/.gitignore +++ b/.gitignore @@ -20,3 +20,9 @@ python docker-compose* Dockerfile scripts/pytest/* +build/ +*.egg-info/ +__pycache__/ +.pytest_cache/ +.tox/ +dist/ diff --git a/biblib/app.py b/biblib/app.py index c513433..bb1c0fa 100644 --- a/biblib/app.py +++ b/biblib/app.py @@ -70,6 +70,10 @@ def create_app(**config): methods=['GET'] ) + # Register CLI commands + from biblib.cli import biblib + app.cli.add_command(biblib) + return app diff --git a/biblib/cli.py b/biblib/cli.py new file mode 100644 index 0000000..8b8910b --- /dev/null +++ b/biblib/cli.py @@ -0,0 +1,145 @@ +import click +from flask import current_app +from flask.cli import with_appcontext +from biblib.models import User, Permissions, Library, Notes +from datetime import datetime +from dateutil.relativedelta import relativedelta +import sqlalchemy_continuum + +@click.group() +def biblib(): + """Commands for the biblib service""" + pass + +@biblib.command() +@with_appcontext +def syncdb(): + """ + Compares the users that exist within the API to those within the + microservice and deletes any stale users that no longer exist. The logic + also takes care of the associated permissions and libraries depending on + the cascade that has been implemented. + """ + with current_app.session_scope() as session: + # Obtain the list of API users + postgres_search_text = 'SELECT id FROM users;' + result = session.execute(postgres_search_text).fetchall() + list_of_api_users = [int(r[0]) for r in result] + + # Loop through every use in the service database + removal_list = [] + for service_user in session.query(User).all(): + if service_user.absolute_uid not in list_of_api_users: + try: + # Obtain the libraries that should be deleted + permissions = session.query(Permissions).filter( + Permissions.user_id == service_user.id + ).all() + + libraries = [ + session.query(Library).filter(Library.id == p.library_id).one() + for p in permissions if p.permissions['owner'] + ] + + # Delete all the libraries found + # By cascade this should delete all the permissions + for library in libraries: + session.delete(library) + for permission in permissions: + session.delete(permission) + + session.delete(service_user) + session.commit() + + d_len = len(libraries) + current_app.logger.info('Removed stale user: {} and {} libraries'.format(service_user, d_len)) + + removal_list.append(service_user) + + except Exception as error: + current_app.logger.info('Problem with database, could not remove user {}: {}' + .format(service_user, error)) + session.rollback() + + current_app.logger.info('Deleted {} stale users: {}'.format(len(removal_list), removal_list)) + +@biblib.command(name='clean_versions_time') +@click.option('--years', default=None, type=int, help='Number of years to keep') +@with_appcontext +def clean_versions_time(years): + """ + Clears obsolete library and notes versions older than chosen time. + """ + if not years: + years = current_app.config.get('REVISION_TIME', 7) + + with current_app.session_scope() as session: + # Obtain a list of all versions older than chosen time. + LibraryVersion = sqlalchemy_continuum.version_class(Library) + NotesVersion = sqlalchemy_continuum.version_class(Notes) + + current_date = datetime.now() + current_offset = current_date - relativedelta(years=years) + + try: + library_results = session.query(LibraryVersion).filter( + LibraryVersion.date_last_modified < current_offset + ).all() + notes_results = session.query(NotesVersion).filter( + NotesVersion.date_last_modified < current_offset + ).all() + + for revision in library_results: + session.delete(revision) + for revision in notes_results: + session.delete(revision) + + session.commit() + + current_app.logger.info('Removed {} obsolete library revisions'.format(len(library_results))) + current_app.logger.info('Removed {} obsolete notes revisions'.format(len(notes_results))) + + except Exception as error: + current_app.logger.info('Problem with database, could not remove revisions: {}' + .format(error)) + session.rollback() + +@click.option('--revisions', default=None, type=int, help='Maximum revisions to keep') +@biblib.command(name='clean_versions_number') +@with_appcontext +def clean_versions_number(revisions): + """ + Limits number of revisions saved per entity to n_revisions. + """ + if not revisions: + revisions = current_app.config.get('NUMBER_REVISIONS', 7) + + def limit_revisions(session, entity_class, n_revisions): + VersionClass = sqlalchemy_continuum.version_class(entity_class) + entities = session.query(entity_class).all() + + for entity in entities: + try: + revs = session.query(VersionClass).filter_by(id=entity.id).order_by( + VersionClass.date_last_modified.asc() + ).all() + + current_app.logger.debug('Found {} revisions for entity: {}'.format(len(revs), entity.id)) + + if len(revs) > n_revisions: + obsolete = revs[:-n_revisions] + for r in obsolete: + session.delete(r) + + session.commit() + + current_app.logger.info('Removed {} obsolete revisions for entity: {}'.format(len(obsolete), entity.id)) + + except Exception as error: + current_app.logger.info('Problem with the database, could not remove revisions for entity {}: {}' + .format(entity, error)) + session.rollback() + + with current_app.session_scope() as session: + limit_revisions(session, Library, revisions) + limit_revisions(session, Notes, revisions) diff --git a/config.py b/biblib/config.py similarity index 100% rename from config.py rename to biblib/config.py diff --git a/biblib/manage.py b/biblib/manage.py deleted file mode 100644 index e7f9d44..0000000 --- a/biblib/manage.py +++ /dev/null @@ -1,147 +0,0 @@ -""" -Alembic migration management file -""" -from datetime import datetime -from dateutil.relativedelta import relativedelta -import os -import sys -PROJECT_HOME = os.path.abspath( - os.path.join(os.path.dirname(__file__), '..')) -sys.path.append(PROJECT_HOME) -from flask import current_app -from flask_script import Manager, Command, Option -from flask_migrate import Migrate, MigrateCommand -from biblib.models import Base, User, Permissions, Library, Notes -from biblib.app import create_app -from sqlalchemy import create_engine, desc -from sqlalchemy.orm import sessionmaker, scoped_session -import sqlalchemy_continuum - -# Load the app with the factory -app = create_app() - -class DeleteStaleUsers(Command): - """ - Compares the users that exist within the API to those within the - microservice and deletes any stale users that no longer exist. The logic - also takes care of the associated permissions and libraries depending on - the cascade that has been implemented. - """ - @staticmethod - def run(app=app): - """ - Carries out the deletion of the stale content - """ - with app.app_context(): - with current_app.session_scope() as session: - # Obtain the list of API users - postgres_search_text = 'SELECT id FROM users;' - result = session.execute(postgres_search_text).fetchall() - list_of_api_users = [int(r[0]) for r in result] - - # Loop through every use in the service database - removal_list = [] - for service_user in session.query(User).all(): - if service_user.absolute_uid not in list_of_api_users: - try: - # Obtain the libraries that should be deleted - permissions = session.query(Permissions).filter(Permissions.user_id == service_user.id).all() - libraries = [session.query(Library).filter(Library.id == permission.library_id).one() for permission in permissions if permission.permissions['owner']] - - # Delete all the libraries found - # By cascade this should delete all the permissions - d = [session.delete(library) for library in libraries] - p = [session.delete(permission) for permission in permissions] - d_len = len(d) - - session.delete(service_user) - session.commit() - current_app.logger.info('Removed stale user: {} and {} libraries'.format(service_user, d_len)) - removal_list.append(service_user) - - except Exception as error: - current_app.logger.info('Problem with database, could not remove user {}: {}' - .format(service_user, error)) - session.rollback() - current_app.logger.info('Deleted {} stale users: {}'.format(len(removal_list), removal_list)) - -class DeleteObsoleteVersionsTime(Command): - """ - Clears obsolete library and notes versions older than chosen time. - """ - @staticmethod - def run(app=app, n_years=None): - """ - Carries out the deletion of older versions - """ - with app.app_context(): - - if not n_years: n_years = current_app.config.get('REVISION_TIME', 7) - - with current_app.session_scope() as session: - # Obtain a list of all versions older than 1 year. - LibraryVersion = sqlalchemy_continuum.version_class(Library) - NotesVersion = sqlalchemy_continuum.version_class(Notes) - current_date = datetime.now() - current_offset = current_date - relativedelta(years=n_years) - try: - library_results = session.query(LibraryVersion).filter(LibraryVersion.date_last_modified=3.2,<4", + "ppsetuptools==2.0.2" +] +build-backend = "flit_core.buildapi" + +[tool.pytest.ini_options] +addopts = "--cov=biblib --cov-report=term-missing" +testpaths = ["biblib/tests"] + +[tool.black] +line-length = 88 +target-version = ['py310'] + +[tool.isort] +profile = "black" diff --git a/pytest.ini b/pytest.ini deleted file mode 100644 index 6400285..0000000 --- a/pytest.ini +++ /dev/null @@ -1,3 +0,0 @@ -[pytest] -addopts = --cov=biblib --cov-report=term-missing -testpaths = biblib/tests \ No newline at end of file diff --git a/requirements.txt b/requirements.txt deleted file mode 100644 index 1411b05..0000000 --- a/requirements.txt +++ /dev/null @@ -1,12 +0,0 @@ -git+https://github.com/adsabs/ADSMicroserviceUtils.git@v1.1.9 -Flask-Migrate==2.0.2 -Flask-Script==2.0.5 -alembic==1.5.3 -psycopg2==2.8.6 -sqlalchemy-continuum==1.3.12 -Flask-Mail==0.9.1 -Flask-Email==1.4.4 -Jinja2==2.11.3 -markupsafe<=2.0.1 -itsdangerous<=2.0.1 -werkzeug<=2.0.3 diff --git a/scripts/cronjob.sh b/scripts/cronjob.sh index 52a9723..923bbd3 100644 --- a/scripts/cronjob.sh +++ b/scripts/cronjob.sh @@ -1,3 +1,3 @@ -* 1 * * * /usr/bin/python /biblib/biblib/manage.py syncdb >> /tmp/biblib_delete_stale_users.log -* 1 * * * /usr/bin/python /biblib/biblib/manage.py clean_versions_number >> /tmp/biblib_revision_deletion.log -* 1 * * * /usr/bin/python /biblib/biblib/manage.py clean_versions_time >> /tmp/biblib_revision_deletion.log \ No newline at end of file +* 1 * * * cd /biblib && export FLASK_APP=biblib/app.py && flask biblib syncdb >> /tmp/biblib_delete_stale_users.log +* 1 * * * cd /biblib && export FLASK_APP=biblib/app.py && flask biblib clean_versions_number >> /tmp/biblib_revision_deletion.log +* 1 * * * cd /biblib && export FLASK_APP=biblib/app.py && flask biblib clean_versions_time >> /tmp/biblib_revision_deletion.log From c8e4196e1c41a0b982ba75015acd20c42b6e2582 Mon Sep 17 00:00:00 2001 From: femalves Date: Tue, 13 Jan 2026 17:43:27 -0500 Subject: [PATCH 02/11] adding setup.py --- pyproject.toml | 4 +++- setup.py | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 1 deletion(-) create mode 100644 setup.py diff --git a/pyproject.toml b/pyproject.toml index 5e46912..fe7ee27 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -5,6 +5,7 @@ description = "ADS Library Service" authors = [{ name = "ADS Team", email = "adshelp@cfa.harvard.edu" }] license = { text = "MIT" } readme = "README.md" +packages = ["biblib"] dependencies = [ "adsmutils @ git+https://github.com/adsabs/ADSMicroserviceUtils.git@v1.3.0", "alembic==1.12.0", @@ -43,7 +44,8 @@ requires = [ "setuptools", "wheel", "flit_core >=3.2,<4", - "ppsetuptools==2.0.2" + "ppsetuptools==2.0.2", + "toml" ] build-backend = "flit_core.buildapi" diff --git a/setup.py b/setup.py new file mode 100644 index 0000000..dfa7e12 --- /dev/null +++ b/setup.py @@ -0,0 +1,52 @@ +# mimic presence of 'setupy.py' - by reading config from pyproject.toml + +# we are using ppsetuptools to read values and pass them to setup() +# however we also have to deal with idiosyncracies of ppsetuptools +# and of flint (which doesn't want to allow any entry inside +# [project.entry-points.console_scripts]) + +import inspect +import os + +# important to import before importing ppsetuptools +import setuptools +import toml + +orig_setup = setuptools.setup + + +def monkey(*args, **kwargs): + del kwargs["license_files"] + del kwargs["keywords"] + + try: + caller_directory = os.path.abspath(os.path.dirname(inspect.stack()[1].filename)) + if not os.path.exists(os.path.join(caller_directory, "pyproject.toml")): + raise + except: # noqa: E722 + caller_directory = "." + + with open(os.path.join(caller_directory, "pyproject.toml"), "r") as pptoml: + pyproject_toml = pptoml.read() + if isinstance(pyproject_toml, bytes): + pyproject_toml = pyproject_toml.decode("utf-8") + + data = toml.loads(pyproject_toml) + + if "xsetup" in data: + for key, value in data["xsetup"].items(): + if key not in kwargs or not kwargs[key]: + kwargs[key] = value + + print("monkey patched setuptools, going to call setup() with those kwargs:") + print("\n".join([str(x) for x in sorted(kwargs.items())])) + + orig_setup(*args, **kwargs) + # raise ("To see values; for testing purposes") + + +setuptools.setup = monkey +import ppsetuptools # noqa: E402 + +ppsetuptools.setup() + From d9da40fc1733a8be26409d1b6c03acaeddc50a9d Mon Sep 17 00:00:00 2001 From: femalves Date: Tue, 13 Jan 2026 17:59:20 -0500 Subject: [PATCH 03/11] downgrading pip --- .github/workflows/python_actions.yml | 2 +- pyproject.toml | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/.github/workflows/python_actions.yml b/.github/workflows/python_actions.yml index 4ad655f..dcd992d 100644 --- a/.github/workflows/python_actions.yml +++ b/.github/workflows/python_actions.yml @@ -15,7 +15,7 @@ jobs: - name: Install dependencies run: | - python -m pip install --upgrade pip setuptools==57.5.0 wheel + python -m pip install "pip<23.1" setuptools==57.5.0 wheel pip install ".[dev]" - name: Test with pytest diff --git a/pyproject.toml b/pyproject.toml index fe7ee27..759abd6 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -44,8 +44,7 @@ requires = [ "setuptools", "wheel", "flit_core >=3.2,<4", - "ppsetuptools==2.0.2", - "toml" + "ppsetuptools==2.0.2" ] build-backend = "flit_core.buildapi" From b9a2d88fab99bd03c221eda4092095567e66fc0f Mon Sep 17 00:00:00 2001 From: femalves Date: Tue, 13 Jan 2026 18:03:22 -0500 Subject: [PATCH 04/11] removing SQLAlchemy pin --- pyproject.toml | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 759abd6..f5d5619 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -16,8 +16,7 @@ dependencies = [ "Jinja2==3.1.2", "markupsafe==2.1.3", "itsdangerous==2.1.2", - "werkzeug==2.3.8", - "SQLAlchemy==1.3.24" + "werkzeug==2.3.8" ] [project.optional-dependencies] From ad26ab2bfc99de03df60d2abbeb900a94d4f159d Mon Sep 17 00:00:00 2001 From: femalves Date: Tue, 13 Jan 2026 18:09:00 -0500 Subject: [PATCH 05/11] remove legacy manage.py references --- .../{test_manage.py => test_cli.py} | 47 +++++++++++-------- 1 file changed, 27 insertions(+), 20 deletions(-) rename biblib/tests/unit_tests/{test_manage.py => test_cli.py} (94%) diff --git a/biblib/tests/unit_tests/test_manage.py b/biblib/tests/unit_tests/test_cli.py similarity index 94% rename from biblib/tests/unit_tests/test_manage.py rename to biblib/tests/unit_tests/test_cli.py index f1f7d0c..10c2098 100644 --- a/biblib/tests/unit_tests/test_manage.py +++ b/biblib/tests/unit_tests/test_cli.py @@ -1,9 +1,9 @@ """ -Tests the methods within the flask-script file manage.py +Tests the methods within the flask cli file cli.py """ import unittest -from biblib.manage import DeleteObsoleteVersionsNumber, DeleteStaleUsers, DeleteObsoleteVersionsTime +from biblib.cli import syncdb, clean_versions_time, clean_versions_number from biblib.models import User, Library, Permissions, Notes from sqlalchemy.orm.exc import NoResultFound from biblib.tests.base import TestCaseDatabase @@ -16,12 +16,9 @@ from biblib.tests.stubdata.stub_data import LibraryShop from biblib.views import DocumentView -class TestManagePy(TestCaseDatabase): +class TestCli(TestCaseDatabase): """ - Class for testing the behaviour of the custom manage scripts - """ - """ - Base test class for when databases are being used. + Class for testing the behaviour of the custom cli scripts """ def __init__(self, *args, **kwargs): """ @@ -32,7 +29,7 @@ def __init__(self, *args, **kwargs): :return: no return """ - super(TestManagePy, self).__init__(*args, **kwargs) + super(TestCli, self).__init__(*args, **kwargs) self.document_view = DocumentView @@ -45,7 +42,7 @@ def __init__(self, *args, **kwargs): def test_delete_stale_users(self): """ - Tests that the DeleteStaleUsers action that propogates the deletion of + Tests that the syncdb action that propogates the deletion of users from the API database to that of the microservice. :return: no return @@ -112,8 +109,10 @@ def test_delete_stale_users(self): library_1_id = library_1.id library_2_id = library_2.id - # Now run the stale deletion - DeleteStaleUsers().run(app=self.app) + # Now run the stale deletion using the CLI runner + runner = self.app.test_cli_runner() + result = runner.invoke(syncdb) + self.assertEqual(result.exit_code, 0) # Check the state of users, libraries and permissions # User 2 @@ -176,7 +175,7 @@ def test_delete_stale_users(self): def test_delete_obsolete_versions_number(self): """ - Tests that the DeleteObsoleteVersionsNumber action that removes + Tests that the clean_versions_number action that removes LibraryVersions older than a given number of years. :return: no return @@ -301,12 +300,16 @@ def test_delete_obsolete_versions_number(self): NotesVersion = sqlalchemy_continuum.version_class(Notes) notes = session.query(Notes).all() notes_revision_lengths = [] - for notes in notes: - revisions = session.query(NotesVersion).filter_by(id=notes.id).all() + for n in notes: + revisions = session.query(NotesVersion).filter_by(id=n.id).all() notes_revision_lengths.append(len(revisions)) self.assertEqual(notes_revision_lengths, [2, 2]) - # Now run the obsolete deletion - DeleteObsoleteVersionsNumber().run(app=self.app, n_revisions=self.n_revisions) + + # Now run the obsolete deletion using the CLI runner + runner = self.app.test_cli_runner() + result = runner.invoke(clean_versions_number, ['--revisions', self.n_revisions]) + self.assertEqual(result.exit_code, 0) + service_user = user_1_id permissions = session.query(Permissions).filter(Permissions.user_id == service_user).all() libraries = [session.query(Library).filter(Library.id == permission.library_id).one() for permission in permissions if permission.permissions['owner']] @@ -346,7 +349,7 @@ def test_delete_obsolete_versions_number(self): def test_delete_obsolete_versions_time(self): """ - Tests that the DeleteObsoleteVersionsTime action that removes + Tests that the clean_versions_time action that removes LibraryVersions older than a given number of years. :return: no return @@ -482,8 +485,11 @@ def test_delete_obsolete_versions_time(self): # Now run the obsolete deletion acting as if we are 1 year in the future. # Libraries and notes should persist because n_years is 2 current_offset = datetime.now() + relativedelta(years=1) + runner = self.app.test_cli_runner() with freezegun.freeze_time(current_offset): - DeleteObsoleteVersionsTime().run(app=self.app, n_years=self.n_years) + result = runner.invoke(clean_versions_time, ['--years', self.n_years]) + self.assertEqual(result.exit_code, 0) + service_user = user_1_id permissions = session.query(Permissions).filter(Permissions.user_id == service_user).all() libraries = [session.query(Library).filter(Library.id == permission.library_id).one() for permission in permissions if permission.permissions['owner']] @@ -509,7 +515,9 @@ def test_delete_obsolete_versions_time(self): # Libraries and notes should be deleted current_offset = datetime.now() + relativedelta(years=2, days=1) with freezegun.freeze_time(current_offset): - DeleteObsoleteVersionsTime().run(app=self.app, n_years=self.n_years) + result = runner.invoke(clean_versions_time, ['--years', self.n_years]) + self.assertEqual(result.exit_code, 0) + service_user = user_1_id permissions = session.query(Permissions).filter(Permissions.user_id == service_user).all() libraries = [session.query(Library).filter(Library.id == permission.library_id).one() for permission in permissions if permission.permissions['owner']] @@ -542,4 +550,3 @@ def test_delete_obsolete_versions_time(self): if __name__ == '__main__': unittest.main(verbosity=2) - From 90ee463beef05a0037b8d4d4d0afc6912415f102 Mon Sep 17 00:00:00 2001 From: femalves Date: Tue, 13 Jan 2026 18:13:06 -0500 Subject: [PATCH 06/11] Fix mock scoping in webservices tests to resolve Python 3.10 failures --- biblib/tests/unit_tests/test_webservices.py | 25 +++++++++++---------- 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/biblib/tests/unit_tests/test_webservices.py b/biblib/tests/unit_tests/test_webservices.py index f66303c..dff5637 100644 --- a/biblib/tests/unit_tests/test_webservices.py +++ b/biblib/tests/unit_tests/test_webservices.py @@ -191,7 +191,7 @@ def test_create_library_resource_and_add_bibcodes(self): url = url_for('libraryview', library=library_id) with MockSolrBigqueryService( - canonical_bibcode=stub_library.bibcode) as BQ, \ + canonical_bibcode=full_bibcodes) as BQ, \ MockEmailService(stub_user, end_type='uid') as ES: response = self.client.get( url, @@ -1131,7 +1131,7 @@ def test_add_document_to_library(self): # Check the library was created and documents exist url = url_for('libraryview', library=library_id) with MockSolrBigqueryService( - canonical_bibcode=stub_library.bibcode) as BQ, \ + canonical_bibcode=full_bibcodes) as BQ, \ MockEmailService(stub_user, end_type='uid') as ES: response = self.client.get( url, @@ -1179,10 +1179,10 @@ def test_add_invalid_document_to_library(self): self.assertEqual(response.status_code, 400) - # Check the library was created and documents exist + # Check the library was created and documents do not exist (it should be empty) url = url_for('libraryview', library=library_id) with MockSolrBigqueryService( - canonical_bibcode=stub_library.bibcode) as BQ, \ + canonical_bibcode=[]) as BQ, \ MockEmailService(stub_user, end_type='uid') as ES: response = self.client.get( url, @@ -1190,8 +1190,7 @@ def test_add_invalid_document_to_library(self): ) self.assertEqual(response.status_code, 200, response) - self.assertNotEqual(stub_library.get_bibcodes(), - response.json['documents']) + self.assertEqual(response.json['documents'], []) def test_add_some_invalid_documents_to_library(self): """ @@ -1236,15 +1235,17 @@ def test_add_some_invalid_documents_to_library(self): # Check the library was created and documents exist url = url_for('libraryview', library=library_id) + # We only expect the second bibcode to be in the library + valid_bibcodes = [json.loads(stub_library.document_view_post_data_json('add')).get('bibcode')[1]] with MockSolrBigqueryService( - canonical_bibcode=stub_library.bibcode) as BQ, \ + canonical_bibcode=valid_bibcodes) as BQ, \ MockEmailService(stub_user, end_type='uid') as ES: response = self.client.get( url, headers=stub_user.headers ) #Check that the expected bibcode and only the expected bibcode is in the libary. - self.assertIn(json.loads(stub_library.document_view_post_data_json('add')).get('bibcode')[1], response.json['documents']) + self.assertIn(valid_bibcodes[0], response.json['documents']) self.assertNotIn(json.loads(stub_library.document_view_post_data_json('add')).get('bibcode')[0], response.json['documents']) #Check that the library makes sense. @@ -1417,7 +1418,7 @@ def test_timestamp_sort_returns_correct_order(self): url = url_for('libraryview', library=library_id) with MockSolrBigqueryService( - canonical_bibcode=stub_library.bibcode) as BQ, \ + canonical_bibcode=full_bibcodes) as BQ, \ MockEmailService(stub_user, end_type='uid') as ES: response = self.client.get( url, @@ -1433,7 +1434,7 @@ def test_timestamp_sort_returns_correct_order(self): response.json['documents']) with MockSolrBigqueryService( - canonical_bibcode=stub_library.bibcode) as BQ, \ + canonical_bibcode=full_bibcodes) as BQ, \ MockEmailService(stub_user, end_type='uid') as ES: response = self.client.get( url, @@ -1489,7 +1490,7 @@ def test_add_query_to_library(self): # Check the library was created and documents exist url = url_for('libraryview', library=library_id) with MockSolrBigqueryService( - canonical_bibcode=stub_library.bibcode) as BQ, \ + canonical_bibcode=full_bibcodes) as BQ, \ MockEmailService(stub_user, end_type='uid') as ES: response = self.client.get( url, @@ -1651,7 +1652,7 @@ def test_add_query_to_library_get(self): # Check the library was created and documents exist url = url_for('libraryview', library=library_id) with MockSolrBigqueryService( - canonical_bibcode=stub_library.bibcode) as BQ, \ + canonical_bibcode=full_bibcodes) as BQ, \ MockEmailService(stub_user, end_type='uid') as ES: response = self.client.get( url, From 3fd7d0673504ad03a67bcfd51bf606481133e02a Mon Sep 17 00:00:00 2001 From: femalves Date: Tue, 13 Jan 2026 18:21:05 -0500 Subject: [PATCH 07/11] Updating README --- README.md | 61 ++++++++++++++++++++++++-------------------- biblib/tests/base.py | 8 +++--- 2 files changed, 38 insertions(+), 31 deletions(-) diff --git a/README.md b/README.md index 411db06..265b8e8 100644 --- a/README.md +++ b/README.md @@ -19,54 +19,59 @@ To load and enter the VM: `vagrant up && vagrant ssh` ### tests -Run the tests using `py.test`: +Run the tests using `pytest`: ```bash -docker run --name some-postgres -e POSTGRES_USER="postgres" POSTGRES_PASSWORD="postgres" -p 5432:5432 --name postgres -virtualenv python +# Setup postgres (using docker for example) +docker run -d --name postgres -e POSTGRES_USER="postgres" -e POSTGRES_PASSWORD="postgres" -p 5432:5432 postgres:12.6 + +# Setup environment +python3 -m venv python source python/bin/activate -pip install -r requirements.txt -pip install -r dev-requirements.txt -py.tests biblib/tests/ + +# Install with legacy build support (requires pip < 23.1 and specific setuptools) +python -m pip install "pip<23.1" setuptools==57.5.0 wheel +pip install -e ".[dev]" + +# Run tests +pytest ``` ### Layout -Tests are split into three (excessive) stages: - 1. Functional tests (*biblib/tests/functional_tests/*): this tests a *workflow*, for example, a user adding a library and then trying to delete it - and testing everything behaves as expected via the REST work flow - 2. Unit tests, level 1 (*biblib/tests/unit_tests/test_webservices.py*): this tests the above *workflow* on the REST end points - 3. Unit tests, level 2 (*biblib/tests/unit_tests/test_views.py*): this tests the logic of functions that are used within views (end points), and is usually the most fine grained testing - 4. Other unit testing: other tests that are testing other parts, such as *manage* scripts are in their own unit tests, e.g., *biblib/tests/unit_tests/test_manage.py*. +Tests are split into three stages: + 1. Functional tests (`biblib/tests/functional_tests/`): this tests a *workflow*, for example, a user adding a library and then trying to delete it - and testing everything behaves as expected via the REST work flow + 2. Unit tests, level 1 (`biblib/tests/unit_tests/test_webservices.py`): this tests the above *workflow* on the REST end points + 3. Unit tests, level 2 (`biblib/tests/unit_tests/test_views.py`): this tests the logic of functions that are used within views (end points), and is usually the most fine grained testing + 4. Maintenance CLI tests: tests for the `flask biblib` commands are in `biblib/tests/unit_tests/test_cli.py`. All tests have been written top down, or in a Test-Driven Development approach, so keep this in mind when reading the tests. All the logic has been built based on these tests, so if you were to add something, I'd advise you first create a test for your expected behaviour, and build the logic until it works. ### Running Biblib Locally To run a version of Biblib locally, a postgres database needs to be created and properly formatted for use with Biblib. This can be done with a local postgres instance or in a docker container using the following commands. -`config.py` must also be copied to `local_config.py` and the environment variables must be adjusted to reflect the local environment. -```bash -docker run -d -e POSTGRES_USER="postgres" -e POSTGRES_PASSWORD="postgres" -p 5432:5432 --name postgres postgres:12.6 -docker exec -it postgres bash -c "psql -c \"CREATE ROLE biblib_service WITH LOGIN PASSWORD 'biblib_service';\"" -docker exec -it postgres bash -c "psql -c \"CREATE DATABASE biblib_service;\"" -docker exec -it postgres bash -c "psql -c \"GRANT CREATE ON DATABASE biblib_service TO biblib_service;\"" -``` +`local_config.py` should be created in `biblib/` and the environment variables must be adjusted to reflect the local environment. -Once the database has been created, `alembic` can be used to upgrade the database to the correct alembic revision ```bash -#In order for alembic to have access to the models metadata, the biblib-service directory must be added to the PYTHONPATH -export PYTHONPATH=$(pwd):$PYTHONPATH +# Setup database +docker run -d -e POSTGRES_USER="postgres" -e POSTGRES_PASSWORD="postgres" -p 5432:5432 --name postgres postgres:12.6 +docker exec -it postgres psql -U postgres -c "CREATE ROLE biblib_service WITH LOGIN PASSWORD 'biblib_service';" +docker exec -it postgres psql -U postgres -c "CREATE DATABASE biblib_service;" +docker exec -it postgres psql -U postgres -c "GRANT ALL PRIVILEGES ON DATABASE biblib_service TO biblib_service;" + +# Run migrations +export FLASK_APP=biblib/app.py +flask biblib syncdb # This will sync users and can be used to initialize schema via alembic indirectly or directly: alembic upgrade head ``` -A new revision can be created by doing the following: +A test version of the microservice can then be deployed using: ```bash -#In order for alembic to have access to the models metadata, the biblib-service directory must be added to the PYTHONPATH -export PYTHONPATH=$(pwd):$PYTHONPATH -alembic revision -m "" --autogenerate +export FLASK_APP=biblib/app.py +flask run --port 4000 ``` - -A test version of the microservice can then be deployed using +or via the legacy entrypoint: ```bash -python3 wsgi.py +python wsgi.py ``` ## deployment diff --git a/biblib/tests/base.py b/biblib/tests/base.py index 7791f80..5c52786 100644 --- a/biblib/tests/base.py +++ b/biblib/tests/base.py @@ -301,12 +301,14 @@ def request_callback(request, uri, headers): } else: - if self.kwargs.get('canonical_bibcode'): + if 'canonical_bibcode' in self.kwargs: docs = [] canonical_bibcodes = self.kwargs.get('canonical_bibcode') - #Sets all odd indexed bibcodes as valid, all other bibcodes are invalid. + if isinstance(canonical_bibcodes, dict): + canonical_bibcodes = list(canonical_bibcodes.keys()) + #Sets all odd indexed bibcodes as valid, all other bibcodes are invalid. for i in range(len(canonical_bibcodes)): - if i%2-1 == 0: + if i % 2 == 1: docs.append({'bibcode': canonical_bibcodes[i]}) input_query ="identifier:("+" OR ".join(canonical_bibcodes)+")" params = { From eb4ef48b3179c8f19f68357142829b27edc1bc02 Mon Sep 17 00:00:00 2001 From: femalves Date: Tue, 13 Jan 2026 18:31:53 -0500 Subject: [PATCH 08/11] Updating pip --- .github/workflows/python_actions.yml | 2 +- README.md | 5 ++--- biblib/tests/base.py | 20 ++++++++++---------- biblib/tests/unit_tests/test_views.py | 7 +++++-- biblib/tests/unit_tests/test_webservices.py | 8 ++++---- 5 files changed, 22 insertions(+), 20 deletions(-) diff --git a/.github/workflows/python_actions.yml b/.github/workflows/python_actions.yml index dcd992d..e54b4d8 100644 --- a/.github/workflows/python_actions.yml +++ b/.github/workflows/python_actions.yml @@ -15,7 +15,7 @@ jobs: - name: Install dependencies run: | - python -m pip install "pip<23.1" setuptools==57.5.0 wheel + python -m pip install "pip==24" setuptools==57.5.0 wheel pip install ".[dev]" - name: Test with pytest diff --git a/README.md b/README.md index 265b8e8..0123c94 100644 --- a/README.md +++ b/README.md @@ -21,15 +21,14 @@ To load and enter the VM: `vagrant up && vagrant ssh` Run the tests using `pytest`: ```bash -# Setup postgres (using docker for example) docker run -d --name postgres -e POSTGRES_USER="postgres" -e POSTGRES_PASSWORD="postgres" -p 5432:5432 postgres:12.6 # Setup environment python3 -m venv python source python/bin/activate -# Install with legacy build support (requires pip < 23.1 and specific setuptools) -python -m pip install "pip<23.1" setuptools==57.5.0 wheel +# Install with legacy build support (requires pip 24 and specific setuptools) +python -m pip install "pip==24" setuptools==57.5.0 wheel pip install -e ".[dev]" # Run tests diff --git a/biblib/tests/base.py b/biblib/tests/base.py index 5c52786..5ee4208 100644 --- a/biblib/tests/base.py +++ b/biblib/tests/base.py @@ -177,16 +177,16 @@ def request_callback(request, uri, headers): if not self.kwargs.get('invalid'): docs = [] canonical_bibcodes = self.kwargs.get('canonical_bibcode') - if isinstance(canonical_bibcodes, dict): - canonical_bibcodes = list(canonical_bibcodes.keys()) + if not isinstance(canonical_bibcodes, list): + canonical_bibcodes = list(canonical_bibcodes) for i in range(self.page*self.page_size, min(len(canonical_bibcodes), (self.page + 1)*self.page_size)): docs.append({'bibcode': canonical_bibcodes[i]}) else: #This treats every other odd bibcode as valid. docs = [] canonical_bibcodes = self.kwargs.get('canonical_bibcode') - if isinstance(canonical_bibcodes, dict): - canonical_bibcodes = list(canonical_bibcodes.keys()) + if not isinstance(canonical_bibcodes, list): + canonical_bibcodes = list(canonical_bibcodes) i = self.page*self.page_size while len(docs) < min(len(canonical_bibcodes), (self.page + 1)*self.page_size) and i < len(canonical_bibcodes): if i%4-1 == 0: @@ -244,10 +244,10 @@ def __exit__(self, etype, value, traceback): :param traceback: the traceback for the exit :return: no return """ - #adding this allows for checking pagination calls. - return self.page HTTPretty.reset() HTTPretty.disable() + #adding this allows for checking pagination calls. + return self.page class MockSolrQueryService(MockADSWSAPI): @@ -280,8 +280,8 @@ def request_callback(request, uri, headers): if 'canonical_bibcode' in self.kwargs: docs = [] canonical_bibcodes = self.kwargs.get('canonical_bibcode') - if isinstance(canonical_bibcodes, dict): - canonical_bibcodes = list(canonical_bibcodes.keys()) + if not isinstance(canonical_bibcodes, list): + canonical_bibcodes = list(canonical_bibcodes) for i in range(len(canonical_bibcodes)): docs.append({'bibcode': canonical_bibcodes[i]}) input_query ="identifier:("+" OR ".join(canonical_bibcodes)+")" @@ -304,8 +304,8 @@ def request_callback(request, uri, headers): if 'canonical_bibcode' in self.kwargs: docs = [] canonical_bibcodes = self.kwargs.get('canonical_bibcode') - if isinstance(canonical_bibcodes, dict): - canonical_bibcodes = list(canonical_bibcodes.keys()) + if not isinstance(canonical_bibcodes, list): + canonical_bibcodes = list(canonical_bibcodes) #Sets all odd indexed bibcodes as valid, all other bibcodes are invalid. for i in range(len(canonical_bibcodes)): if i % 2 == 1: diff --git a/biblib/tests/unit_tests/test_views.py b/biblib/tests/unit_tests/test_views.py index 42f634d..1238ca7 100644 --- a/biblib/tests/unit_tests/test_views.py +++ b/biblib/tests/unit_tests/test_views.py @@ -479,15 +479,18 @@ def test_user_can_retrieve_all_libraries_by_paging(self): libraries = [] total_libraries = 0 with MockEmailService(self.stub_user, end_type='uid'): - for start in range(number_of_libs): + for start in range(0, number_of_libs, 10): curr_libraries = self.user_view.get_libraries( service_uid=user.id, - absolute_uid=user.absolute_uid, start=start*10, + absolute_uid=user.absolute_uid, start=start, rows=10 ) libraries += curr_libraries['libraries'] total_libraries = curr_libraries['count'] self.assertEqual(total_libraries, 100) + # Sort by id to ensure order-independent comparison + libraries_full.sort(key=lambda x: x['id']) + libraries.sort(key=lambda x: x['id']) self.assertEqual(libraries_full, libraries) def test_user_can_retrieve_library_when_uid_does_not_exist(self): diff --git a/biblib/tests/unit_tests/test_webservices.py b/biblib/tests/unit_tests/test_webservices.py index dff5637..3343a2f 100644 --- a/biblib/tests/unit_tests/test_webservices.py +++ b/biblib/tests/unit_tests/test_webservices.py @@ -191,7 +191,7 @@ def test_create_library_resource_and_add_bibcodes(self): url = url_for('libraryview', library=library_id) with MockSolrBigqueryService( - canonical_bibcode=full_bibcodes) as BQ, \ + canonical_bibcode=stub_library.bibcode) as BQ, \ MockEmailService(stub_user, end_type='uid') as ES: response = self.client.get( url, @@ -1131,7 +1131,7 @@ def test_add_document_to_library(self): # Check the library was created and documents exist url = url_for('libraryview', library=library_id) with MockSolrBigqueryService( - canonical_bibcode=full_bibcodes) as BQ, \ + canonical_bibcode=stub_library.bibcode) as BQ, \ MockEmailService(stub_user, end_type='uid') as ES: response = self.client.get( url, @@ -1490,7 +1490,7 @@ def test_add_query_to_library(self): # Check the library was created and documents exist url = url_for('libraryview', library=library_id) with MockSolrBigqueryService( - canonical_bibcode=full_bibcodes) as BQ, \ + canonical_bibcode=stub_library.bibcode) as BQ, \ MockEmailService(stub_user, end_type='uid') as ES: response = self.client.get( url, @@ -1652,7 +1652,7 @@ def test_add_query_to_library_get(self): # Check the library was created and documents exist url = url_for('libraryview', library=library_id) with MockSolrBigqueryService( - canonical_bibcode=full_bibcodes) as BQ, \ + canonical_bibcode=stub_library.bibcode) as BQ, \ MockEmailService(stub_user, end_type='uid') as ES: response = self.client.get( url, From 8231d763ccaaace07ed5858adfd594c6023bf263 Mon Sep 17 00:00:00 2001 From: femalves Date: Wed, 14 Jan 2026 18:12:36 -0500 Subject: [PATCH 09/11] Reverting code for compatibility --- biblib/app.py | 4 - biblib/cli.py | 145 --------------- biblib/manage.py | 169 ++++++++++++++++++ .../{test_cli.py => test_manage.py} | 40 ++--- pyproject.toml | 3 +- scripts/__init__.py | 0 scripts/cronjob.sh | 6 +- 7 files changed, 189 insertions(+), 178 deletions(-) delete mode 100644 biblib/cli.py create mode 100644 biblib/manage.py rename biblib/tests/unit_tests/{test_cli.py => test_manage.py} (94%) create mode 100644 scripts/__init__.py diff --git a/biblib/app.py b/biblib/app.py index bb1c0fa..c513433 100644 --- a/biblib/app.py +++ b/biblib/app.py @@ -70,10 +70,6 @@ def create_app(**config): methods=['GET'] ) - # Register CLI commands - from biblib.cli import biblib - app.cli.add_command(biblib) - return app diff --git a/biblib/cli.py b/biblib/cli.py deleted file mode 100644 index 8b8910b..0000000 --- a/biblib/cli.py +++ /dev/null @@ -1,145 +0,0 @@ -import click -from flask import current_app -from flask.cli import with_appcontext -from biblib.models import User, Permissions, Library, Notes -from datetime import datetime -from dateutil.relativedelta import relativedelta -import sqlalchemy_continuum - -@click.group() -def biblib(): - """Commands for the biblib service""" - pass - -@biblib.command() -@with_appcontext -def syncdb(): - """ - Compares the users that exist within the API to those within the - microservice and deletes any stale users that no longer exist. The logic - also takes care of the associated permissions and libraries depending on - the cascade that has been implemented. - """ - with current_app.session_scope() as session: - # Obtain the list of API users - postgres_search_text = 'SELECT id FROM users;' - result = session.execute(postgres_search_text).fetchall() - list_of_api_users = [int(r[0]) for r in result] - - # Loop through every use in the service database - removal_list = [] - for service_user in session.query(User).all(): - if service_user.absolute_uid not in list_of_api_users: - try: - # Obtain the libraries that should be deleted - permissions = session.query(Permissions).filter( - Permissions.user_id == service_user.id - ).all() - - libraries = [ - session.query(Library).filter(Library.id == p.library_id).one() - for p in permissions if p.permissions['owner'] - ] - - # Delete all the libraries found - # By cascade this should delete all the permissions - for library in libraries: - session.delete(library) - for permission in permissions: - session.delete(permission) - - session.delete(service_user) - session.commit() - - d_len = len(libraries) - current_app.logger.info('Removed stale user: {} and {} libraries'.format(service_user, d_len)) - - removal_list.append(service_user) - - except Exception as error: - current_app.logger.info('Problem with database, could not remove user {}: {}' - .format(service_user, error)) - session.rollback() - - current_app.logger.info('Deleted {} stale users: {}'.format(len(removal_list), removal_list)) - -@biblib.command(name='clean_versions_time') -@click.option('--years', default=None, type=int, help='Number of years to keep') -@with_appcontext -def clean_versions_time(years): - """ - Clears obsolete library and notes versions older than chosen time. - """ - if not years: - years = current_app.config.get('REVISION_TIME', 7) - - with current_app.session_scope() as session: - # Obtain a list of all versions older than chosen time. - LibraryVersion = sqlalchemy_continuum.version_class(Library) - NotesVersion = sqlalchemy_continuum.version_class(Notes) - - current_date = datetime.now() - current_offset = current_date - relativedelta(years=years) - - try: - library_results = session.query(LibraryVersion).filter( - LibraryVersion.date_last_modified < current_offset - ).all() - notes_results = session.query(NotesVersion).filter( - NotesVersion.date_last_modified < current_offset - ).all() - - for revision in library_results: - session.delete(revision) - for revision in notes_results: - session.delete(revision) - - session.commit() - - current_app.logger.info('Removed {} obsolete library revisions'.format(len(library_results))) - current_app.logger.info('Removed {} obsolete notes revisions'.format(len(notes_results))) - - except Exception as error: - current_app.logger.info('Problem with database, could not remove revisions: {}' - .format(error)) - session.rollback() - -@click.option('--revisions', default=None, type=int, help='Maximum revisions to keep') -@biblib.command(name='clean_versions_number') -@with_appcontext -def clean_versions_number(revisions): - """ - Limits number of revisions saved per entity to n_revisions. - """ - if not revisions: - revisions = current_app.config.get('NUMBER_REVISIONS', 7) - - def limit_revisions(session, entity_class, n_revisions): - VersionClass = sqlalchemy_continuum.version_class(entity_class) - entities = session.query(entity_class).all() - - for entity in entities: - try: - revs = session.query(VersionClass).filter_by(id=entity.id).order_by( - VersionClass.date_last_modified.asc() - ).all() - - current_app.logger.debug('Found {} revisions for entity: {}'.format(len(revs), entity.id)) - - if len(revs) > n_revisions: - obsolete = revs[:-n_revisions] - for r in obsolete: - session.delete(r) - - session.commit() - - current_app.logger.info('Removed {} obsolete revisions for entity: {}'.format(len(obsolete), entity.id)) - - except Exception as error: - current_app.logger.info('Problem with the database, could not remove revisions for entity {}: {}' - .format(entity, error)) - session.rollback() - - with current_app.session_scope() as session: - limit_revisions(session, Library, revisions) - limit_revisions(session, Notes, revisions) diff --git a/biblib/manage.py b/biblib/manage.py new file mode 100644 index 0000000..9e36f96 --- /dev/null +++ b/biblib/manage.py @@ -0,0 +1,169 @@ +import os +import sys +import click +from biblib.app import create_app +from biblib.models import User, Permissions, Library, Notes +from flask import current_app +from datetime import datetime +from dateutil.relativedelta import relativedelta +import sqlalchemy_continuum + +class DeleteStaleUsers: + """ + Compares the users that exist within the API to those within the + microservice and deletes any stale users that no longer exist. + """ + def run(self, app=None): + if app is None: + app = create_app() + with app.app_context(): + with current_app.session_scope() as session: + # Obtain the list of API users + postgres_search_text = 'SELECT id FROM users;' + result = session.execute(postgres_search_text).fetchall() + list_of_api_users = [int(r[0]) for r in result] + + # Loop through every user in the service database + removal_list = [] + for service_user in session.query(User).all(): + if service_user.absolute_uid not in list_of_api_users: + try: + # Obtain the libraries that should be deleted + permissions = session.query(Permissions).filter( + Permissions.user_id == service_user.id + ).all() + + libraries = [ + session.query(Library).filter(Library.id == p.library_id).one() + for p in permissions if p.permissions['owner'] + ] + + # Delete all the libraries found + # By cascade this should delete all the permissions + for library in libraries: + session.delete(library) + for permission in permissions: + session.delete(permission) + + session.delete(service_user) + session.commit() + + d_len = len(libraries) + current_app.logger.info('Removed stale user: {} and {} libraries'.format(service_user, d_len)) + + removal_list.append(service_user) + + except Exception as error: + current_app.logger.info('Problem with database, could not remove user {}: {}' + .format(service_user, error)) + session.rollback() + + current_app.logger.info('Deleted {} stale users: {}'.format(len(removal_list), removal_list)) + +class DeleteObsoleteVersionsTime: + """ + Clears obsolete library and notes versions older than chosen time. + """ + def run(self, years=None, app=None): + if app is None: + app = create_app() + with app.app_context(): + if not years: + years = current_app.config.get('REVISION_TIME', 7) + + with current_app.session_scope() as session: + # Obtain a list of all versions older than chosen time. + LibraryVersion = sqlalchemy_continuum.version_class(Library) + NotesVersion = sqlalchemy_continuum.version_class(Notes) + + current_date = datetime.now() + current_offset = current_date - relativedelta(years=years) + + try: + library_results = session.query(LibraryVersion).filter( + LibraryVersion.date_last_modified < current_offset + ).all() + notes_results = session.query(NotesVersion).filter( + NotesVersion.date_last_modified < current_offset + ).all() + + for revision in library_results: + session.delete(revision) + for revision in notes_results: + session.delete(revision) + + session.commit() + + current_app.logger.info('Removed {} obsolete library revisions'.format(len(library_results))) + current_app.logger.info('Removed {} obsolete notes revisions'.format(len(notes_results))) + + except Exception as error: + current_app.logger.info('Problem with database, could not remove revisions: {}' + .format(error)) + session.rollback() + +class DeleteObsoleteVersionsNumber: + """ + Limits number of revisions saved per entity to n_revisions. + """ + def run(self, n_revisions=None, app=None): + if app is None: + app = create_app() + with app.app_context(): + if not n_revisions: + n_revisions = current_app.config.get('NUMBER_REVISIONS', 7) + + def limit_revisions(session, entity_class, n_revs): + VersionClass = sqlalchemy_continuum.version_class(entity_class) + entities = session.query(entity_class).all() + + for entity in entities: + try: + revs = session.query(VersionClass).filter_by(id=entity.id).order_by( + VersionClass.date_last_modified.asc() + ).all() + + if len(revs) > n_revs: + obsolete = revs[:-n_revs] + for r in obsolete: + session.delete(r) + + session.commit() + + current_app.logger.info('Removed {} obsolete revisions for entity: {}'.format(len(obsolete), entity.id)) + + except Exception as error: + current_app.logger.info('Problem with the database, could not remove revisions for entity {}: {}' + .format(entity, error)) + session.rollback() + + with current_app.session_scope() as session: + limit_revisions(session, Library, n_revisions) + limit_revisions(session, Notes, n_revisions) + +# CLI part for backward compatibility running as script +@click.group() +def manager(): + """Management script for the Biblib service.""" + pass + +@manager.command() +def syncdb(): + """Compares microservice users to API users and deletes stale users.""" + DeleteStaleUsers().run() + +@manager.command(name='clean_versions_time') +@click.option('--years', default=None, type=int, help='Number of years to keep') +def clean_versions_time(years): + """Clears obsolete revisions older than chosen time.""" + DeleteObsoleteVersionsTime().run(years=years) + +@manager.command(name='clean_versions_number') +@click.option('--revisions', default=None, type=int, help='Maximum revisions to keep') +def clean_versions_number(revisions): + """Limits number of revisions saved per entity.""" + DeleteObsoleteVersionsNumber().run(n_revisions=revisions) + +if __name__ == '__main__': + manager() + diff --git a/biblib/tests/unit_tests/test_cli.py b/biblib/tests/unit_tests/test_manage.py similarity index 94% rename from biblib/tests/unit_tests/test_cli.py rename to biblib/tests/unit_tests/test_manage.py index 10c2098..b99c0e5 100644 --- a/biblib/tests/unit_tests/test_cli.py +++ b/biblib/tests/unit_tests/test_manage.py @@ -1,9 +1,9 @@ """ -Tests the methods within the flask cli file cli.py +Tests the methods within the flask-script file manage.py """ import unittest -from biblib.cli import syncdb, clean_versions_time, clean_versions_number +from biblib.manage import DeleteObsoleteVersionsNumber, DeleteStaleUsers, DeleteObsoleteVersionsTime from biblib.models import User, Library, Permissions, Notes from sqlalchemy.orm.exc import NoResultFound from biblib.tests.base import TestCaseDatabase @@ -16,9 +16,9 @@ from biblib.tests.stubdata.stub_data import LibraryShop from biblib.views import DocumentView -class TestCli(TestCaseDatabase): +class TestManagePy(TestCaseDatabase): """ - Class for testing the behaviour of the custom cli scripts + Class for testing the behaviour of the custom manage scripts """ def __init__(self, *args, **kwargs): """ @@ -29,7 +29,7 @@ def __init__(self, *args, **kwargs): :return: no return """ - super(TestCli, self).__init__(*args, **kwargs) + super(TestManagePy, self).__init__(*args, **kwargs) self.document_view = DocumentView @@ -42,7 +42,7 @@ def __init__(self, *args, **kwargs): def test_delete_stale_users(self): """ - Tests that the syncdb action that propogates the deletion of + Tests that the DeleteStaleUsers action that propogates the deletion of users from the API database to that of the microservice. :return: no return @@ -109,10 +109,8 @@ def test_delete_stale_users(self): library_1_id = library_1.id library_2_id = library_2.id - # Now run the stale deletion using the CLI runner - runner = self.app.test_cli_runner() - result = runner.invoke(syncdb) - self.assertEqual(result.exit_code, 0) + # Now run the stale deletion + DeleteStaleUsers().run(app=self.app) # Check the state of users, libraries and permissions # User 2 @@ -175,7 +173,7 @@ def test_delete_stale_users(self): def test_delete_obsolete_versions_number(self): """ - Tests that the clean_versions_number action that removes + Tests that the DeleteObsoleteVersionsNumber action that removes LibraryVersions older than a given number of years. :return: no return @@ -300,15 +298,13 @@ def test_delete_obsolete_versions_number(self): NotesVersion = sqlalchemy_continuum.version_class(Notes) notes = session.query(Notes).all() notes_revision_lengths = [] - for n in notes: - revisions = session.query(NotesVersion).filter_by(id=n.id).all() + for note in notes: + revisions = session.query(NotesVersion).filter_by(id=note.id).all() notes_revision_lengths.append(len(revisions)) self.assertEqual(notes_revision_lengths, [2, 2]) - # Now run the obsolete deletion using the CLI runner - runner = self.app.test_cli_runner() - result = runner.invoke(clean_versions_number, ['--revisions', self.n_revisions]) - self.assertEqual(result.exit_code, 0) + # Now run the obsolete deletion + DeleteObsoleteVersionsNumber().run(app=self.app, n_revisions=self.n_revisions) service_user = user_1_id permissions = session.query(Permissions).filter(Permissions.user_id == service_user).all() @@ -349,7 +345,7 @@ def test_delete_obsolete_versions_number(self): def test_delete_obsolete_versions_time(self): """ - Tests that the clean_versions_time action that removes + Tests that the DeleteObsoleteVersionsTime action that removes LibraryVersions older than a given number of years. :return: no return @@ -485,10 +481,8 @@ def test_delete_obsolete_versions_time(self): # Now run the obsolete deletion acting as if we are 1 year in the future. # Libraries and notes should persist because n_years is 2 current_offset = datetime.now() + relativedelta(years=1) - runner = self.app.test_cli_runner() with freezegun.freeze_time(current_offset): - result = runner.invoke(clean_versions_time, ['--years', self.n_years]) - self.assertEqual(result.exit_code, 0) + DeleteObsoleteVersionsTime().run(app=self.app, years=self.n_years) service_user = user_1_id permissions = session.query(Permissions).filter(Permissions.user_id == service_user).all() @@ -515,8 +509,7 @@ def test_delete_obsolete_versions_time(self): # Libraries and notes should be deleted current_offset = datetime.now() + relativedelta(years=2, days=1) with freezegun.freeze_time(current_offset): - result = runner.invoke(clean_versions_time, ['--years', self.n_years]) - self.assertEqual(result.exit_code, 0) + DeleteObsoleteVersionsTime().run(app=self.app, years=self.n_years) service_user = user_1_id permissions = session.query(Permissions).filter(Permissions.user_id == service_user).all() @@ -549,4 +542,3 @@ def test_delete_obsolete_versions_time(self): if __name__ == '__main__': unittest.main(verbosity=2) - diff --git a/pyproject.toml b/pyproject.toml index f5d5619..33e898d 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -21,7 +21,6 @@ dependencies = [ [project.optional-dependencies] dev = [ - "setuptools==57.5.0", "Flask-Testing==0.8.1", "httpretty==1.1.4", "testing.postgresql==1.3.0", @@ -40,7 +39,7 @@ dev = [ [build-system] requires = [ - "setuptools", + "setuptools==57.5.0", "wheel", "flit_core >=3.2,<4", "ppsetuptools==2.0.2" diff --git a/scripts/__init__.py b/scripts/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/scripts/cronjob.sh b/scripts/cronjob.sh index 923bbd3..a2ce491 100644 --- a/scripts/cronjob.sh +++ b/scripts/cronjob.sh @@ -1,3 +1,3 @@ -* 1 * * * cd /biblib && export FLASK_APP=biblib/app.py && flask biblib syncdb >> /tmp/biblib_delete_stale_users.log -* 1 * * * cd /biblib && export FLASK_APP=biblib/app.py && flask biblib clean_versions_number >> /tmp/biblib_revision_deletion.log -* 1 * * * cd /biblib && export FLASK_APP=biblib/app.py && flask biblib clean_versions_time >> /tmp/biblib_revision_deletion.log +* 1 * * * /usr/bin/python /biblib/biblib/manage.py syncdb >> /tmp/biblib_delete_stale_users.log +* 1 * * * /usr/bin/python /biblib/biblib/manage.py clean_versions_number >> /tmp/biblib_revision_deletion.log +* 1 * * * /usr/bin/python /biblib/biblib/manage.py clean_versions_time >> /tmp/biblib_revision_deletion.log From ebae44c894daca113701cb18ea8ae8466912e0bc Mon Sep 17 00:00:00 2001 From: femalves Date: Wed, 21 Jan 2026 13:43:25 -0500 Subject: [PATCH 10/11] Reviewed Readme, test_manage and cronjobs --- README.md | 33 ++++++++++++++++++++------ biblib/tests/unit_tests/test_manage.py | 10 ++++---- scripts/cronjob.sh | 1 + 3 files changed, 31 insertions(+), 13 deletions(-) diff --git a/README.md b/README.md index 0123c94..843b45f 100644 --- a/README.md +++ b/README.md @@ -37,11 +37,11 @@ pytest ### Layout -Tests are split into three stages: - 1. Functional tests (`biblib/tests/functional_tests/`): this tests a *workflow*, for example, a user adding a library and then trying to delete it - and testing everything behaves as expected via the REST work flow - 2. Unit tests, level 1 (`biblib/tests/unit_tests/test_webservices.py`): this tests the above *workflow* on the REST end points - 3. Unit tests, level 2 (`biblib/tests/unit_tests/test_views.py`): this tests the logic of functions that are used within views (end points), and is usually the most fine grained testing - 4. Maintenance CLI tests: tests for the `flask biblib` commands are in `biblib/tests/unit_tests/test_cli.py`. +Tests are split into three (excessive) stages: + 1. Functional tests (*biblib/tests/functional_tests/*): this tests a *workflow*, for example, a user adding a library and then trying to delete it - and testing everything behaves as expected via the REST work flow + 2. Unit tests, level 1 (*biblib/tests/unit_tests/test_webservices.py*): this tests the above *workflow* on the REST end points + 3. Unit tests, level 2 (*biblib/tests/unit_tests/test_views.py*): this tests the logic of functions that are used within views (end points), and is usually the most fine grained testing + 4. Other unit testing: other tests that are testing other parts, such as *manage* scripts are in their own unit tests, e.g., *biblib/tests/unit_tests/test_manage.py*. All tests have been written top down, or in a Test-Driven Development approach, so keep this in mind when reading the tests. All the logic has been built based on these tests, so if you were to add something, I'd advise you first create a test for your expected behaviour, and build the logic until it works. @@ -56,10 +56,12 @@ docker run -d -e POSTGRES_USER="postgres" -e POSTGRES_PASSWORD="postgres" -p 543 docker exec -it postgres psql -U postgres -c "CREATE ROLE biblib_service WITH LOGIN PASSWORD 'biblib_service';" docker exec -it postgres psql -U postgres -c "CREATE DATABASE biblib_service;" docker exec -it postgres psql -U postgres -c "GRANT ALL PRIVILEGES ON DATABASE biblib_service TO biblib_service;" +``` # Run migrations -export FLASK_APP=biblib/app.py -flask biblib syncdb # This will sync users and can be used to initialize schema via alembic indirectly or directly: +# In order for alembic to have access to the models metadata, the biblib-service directory must be added to the PYTHONPATH +export PYTHONPATH=$(pwd):$PYTHONPATH +python biblib/manage.py syncdb # This will sync users and can be used to initialize schema via alembic indirectly or directly: alembic upgrade head ``` @@ -73,6 +75,23 @@ or via the legacy entrypoint: python wsgi.py ``` +### Database versioning + +Database versioning is managed using Alembic. You can upgrade to the latest revision or downgrade to a previous one using the following commands: + +```bash +# Upgrade to latest revision +alembic upgrade head + +# Downgrade revision +alembic downgrade + +# Create a new revision +alembic revision --autogenerate -m "revision description" +``` + +New revisions of libraries and notes are created automatically by `sqlalchemy-continuum` whenever a record is updated and committed to the database. + ## deployment The only thing to take care of when making a deployment is the migration of the backend database. Libraries uses specific features of PostgreSQL, such as `UUID` and `JSON`-store, so you should think carefully if you wish to change the backend. **The use of `flask-migrate` for database migrations has been replaced by directly calling `alembic`.** diff --git a/biblib/tests/unit_tests/test_manage.py b/biblib/tests/unit_tests/test_manage.py index b99c0e5..3065685 100644 --- a/biblib/tests/unit_tests/test_manage.py +++ b/biblib/tests/unit_tests/test_manage.py @@ -20,6 +20,9 @@ class TestManagePy(TestCaseDatabase): """ Class for testing the behaviour of the custom manage scripts """ + """ + Base test class for when databases are being used. + """ def __init__(self, *args, **kwargs): """ Constructor of the class @@ -302,17 +305,14 @@ def test_delete_obsolete_versions_number(self): revisions = session.query(NotesVersion).filter_by(id=note.id).all() notes_revision_lengths.append(len(revisions)) self.assertEqual(notes_revision_lengths, [2, 2]) - # Now run the obsolete deletion DeleteObsoleteVersionsNumber().run(app=self.app, n_revisions=self.n_revisions) - service_user = user_1_id permissions = session.query(Permissions).filter(Permissions.user_id == service_user).all() libraries = [session.query(Library).filter(Library.id == permission.library_id).one() for permission in permissions if permission.permissions['owner']] LibraryVersion = sqlalchemy_continuum.version_class(Library) updated_revision_lengths = [] - - + #confirm most recent remaining revision matches current state of library for library in libraries: updated_revisions = session.query(LibraryVersion).filter_by(id=library.id).all() @@ -483,7 +483,6 @@ def test_delete_obsolete_versions_time(self): current_offset = datetime.now() + relativedelta(years=1) with freezegun.freeze_time(current_offset): DeleteObsoleteVersionsTime().run(app=self.app, years=self.n_years) - service_user = user_1_id permissions = session.query(Permissions).filter(Permissions.user_id == service_user).all() libraries = [session.query(Library).filter(Library.id == permission.library_id).one() for permission in permissions if permission.permissions['owner']] @@ -510,7 +509,6 @@ def test_delete_obsolete_versions_time(self): current_offset = datetime.now() + relativedelta(years=2, days=1) with freezegun.freeze_time(current_offset): DeleteObsoleteVersionsTime().run(app=self.app, years=self.n_years) - service_user = user_1_id permissions = session.query(Permissions).filter(Permissions.user_id == service_user).all() libraries = [session.query(Library).filter(Library.id == permission.library_id).one() for permission in permissions if permission.permissions['owner']] diff --git a/scripts/cronjob.sh b/scripts/cronjob.sh index a2ce491..eeeca09 100644 --- a/scripts/cronjob.sh +++ b/scripts/cronjob.sh @@ -1,3 +1,4 @@ * 1 * * * /usr/bin/python /biblib/biblib/manage.py syncdb >> /tmp/biblib_delete_stale_users.log * 1 * * * /usr/bin/python /biblib/biblib/manage.py clean_versions_number >> /tmp/biblib_revision_deletion.log * 1 * * * /usr/bin/python /biblib/biblib/manage.py clean_versions_time >> /tmp/biblib_revision_deletion.log + From cff9e658f6422f48069ca68ff272caa4bf2d021e Mon Sep 17 00:00:00 2001 From: femalves Date: Wed, 21 Jan 2026 14:33:00 -0500 Subject: [PATCH 11/11] Reviewed manage.py --- biblib/manage.py | 24 +++++++++++++----------- biblib/tests/unit_tests/test_manage.py | 4 ++-- scripts/cronjob.sh | 1 - 3 files changed, 15 insertions(+), 14 deletions(-) diff --git a/biblib/manage.py b/biblib/manage.py index 9e36f96..206c629 100644 --- a/biblib/manage.py +++ b/biblib/manage.py @@ -34,8 +34,8 @@ def run(self, app=None): ).all() libraries = [ - session.query(Library).filter(Library.id == p.library_id).one() - for p in permissions if p.permissions['owner'] + session.query(Library).filter(Library.id == permission.library_id).one() + for permission in permissions if permission.permissions['owner'] ] # Delete all the libraries found @@ -64,12 +64,12 @@ class DeleteObsoleteVersionsTime: """ Clears obsolete library and notes versions older than chosen time. """ - def run(self, years=None, app=None): + def run(self, n_years=None, app=None): if app is None: app = create_app() with app.app_context(): - if not years: - years = current_app.config.get('REVISION_TIME', 7) + if not n_years: + n_years = current_app.config.get('REVISION_TIME', 7) with current_app.session_scope() as session: # Obtain a list of all versions older than chosen time. @@ -77,7 +77,7 @@ def run(self, years=None, app=None): NotesVersion = sqlalchemy_continuum.version_class(Notes) current_date = datetime.now() - current_offset = current_date - relativedelta(years=years) + current_offset = current_date - relativedelta(years=n_years) try: library_results = session.query(LibraryVersion).filter( @@ -113,18 +113,20 @@ def run(self, n_revisions=None, app=None): if not n_revisions: n_revisions = current_app.config.get('NUMBER_REVISIONS', 7) - def limit_revisions(session, entity_class, n_revs): + def limit_revisions(session, entity_class, n_revisions): VersionClass = sqlalchemy_continuum.version_class(entity_class) entities = session.query(entity_class).all() for entity in entities: try: - revs = session.query(VersionClass).filter_by(id=entity.id).order_by( + revisions = session.query(VersionClass).filter_by(id=entity.id).order_by( VersionClass.date_last_modified.asc() ).all() - if len(revs) > n_revs: - obsolete = revs[:-n_revs] + current_app.logger.debug('Found {} revisions for entity: {}'.format(len(revisions), entity.id)) + + if len(revisions) > n_revisions: + obsolete = revisions[:-n_revisions] for r in obsolete: session.delete(r) @@ -156,7 +158,7 @@ def syncdb(): @click.option('--years', default=None, type=int, help='Number of years to keep') def clean_versions_time(years): """Clears obsolete revisions older than chosen time.""" - DeleteObsoleteVersionsTime().run(years=years) + DeleteObsoleteVersionsTime().run(n_years=years) @manager.command(name='clean_versions_number') @click.option('--revisions', default=None, type=int, help='Maximum revisions to keep') diff --git a/biblib/tests/unit_tests/test_manage.py b/biblib/tests/unit_tests/test_manage.py index 3065685..c47a00f 100644 --- a/biblib/tests/unit_tests/test_manage.py +++ b/biblib/tests/unit_tests/test_manage.py @@ -482,7 +482,7 @@ def test_delete_obsolete_versions_time(self): # Libraries and notes should persist because n_years is 2 current_offset = datetime.now() + relativedelta(years=1) with freezegun.freeze_time(current_offset): - DeleteObsoleteVersionsTime().run(app=self.app, years=self.n_years) + DeleteObsoleteVersionsTime().run(app=self.app, n_years=self.n_years) service_user = user_1_id permissions = session.query(Permissions).filter(Permissions.user_id == service_user).all() libraries = [session.query(Library).filter(Library.id == permission.library_id).one() for permission in permissions if permission.permissions['owner']] @@ -508,7 +508,7 @@ def test_delete_obsolete_versions_time(self): # Libraries and notes should be deleted current_offset = datetime.now() + relativedelta(years=2, days=1) with freezegun.freeze_time(current_offset): - DeleteObsoleteVersionsTime().run(app=self.app, years=self.n_years) + DeleteObsoleteVersionsTime().run(app=self.app, n_years=self.n_years) service_user = user_1_id permissions = session.query(Permissions).filter(Permissions.user_id == service_user).all() libraries = [session.query(Library).filter(Library.id == permission.library_id).one() for permission in permissions if permission.permissions['owner']] diff --git a/scripts/cronjob.sh b/scripts/cronjob.sh index eeeca09..a2ce491 100644 --- a/scripts/cronjob.sh +++ b/scripts/cronjob.sh @@ -1,4 +1,3 @@ * 1 * * * /usr/bin/python /biblib/biblib/manage.py syncdb >> /tmp/biblib_delete_stale_users.log * 1 * * * /usr/bin/python /biblib/biblib/manage.py clean_versions_number >> /tmp/biblib_revision_deletion.log * 1 * * * /usr/bin/python /biblib/biblib/manage.py clean_versions_time >> /tmp/biblib_revision_deletion.log -