diff --git a/Makefile b/Makefile index ae2148f2..f212b546 100644 --- a/Makefile +++ b/Makefile @@ -20,6 +20,11 @@ stop: restart-workers: ## Restart workers docker compose -f devops/docker-compose.local.yml --env-file .env restart workers +.PHONY: restart-nginx +restart-nginx: ## Restart nginx + docker compose -f devops/docker-compose.local.yml --env-file .env restart nginx + + .PHONY: build build: make geoapi && make workers diff --git a/README.md b/README.md index 50b8b86d..21268b0b 100644 --- a/README.md +++ b/README.md @@ -70,7 +70,8 @@ Then, create migrations: ``` docker exec -it geoapi /bin/bash -alembic revision --autogenerate +# determine a description for the migration like 'add_user_email_column' +alembic revision --autogenerate -m "add_user_email_column" # Then: # - remove drop table commands for postgis # - add/commit migrations diff --git a/devops/geoapi-services/nginx.conf b/devops/geoapi-services/nginx.conf index 1ccbe74a..77823823 100644 --- a/devops/geoapi-services/nginx.conf +++ b/devops/geoapi-services/nginx.conf @@ -38,15 +38,15 @@ http { large_client_header_buffers 2 50k; location / { - add_header "Access-Control-Allow-Origin" *; + add_header 'Access-Control-Allow-Origin' '*' always; # Preflighted requests - if ($request_method = OPTIONS ) { - add_header "Access-Control-Allow-Origin" *; - add_header "Access-Control-Allow-Methods" "GET, POST, OPTIONS, HEAD, PUT, DELETE"; - add_header "Access-Control-Allow-Headers" "*"; - add_header 'Access-Control-Max-Age' 1728000; - add_header 'Content-Length' 0; + if ($request_method = 'OPTIONS') { + add_header 'Access-Control-Allow-Origin' '*' always; + add_header 'Access-Control-Allow-Methods' 'GET, POST, OPTIONS, HEAD, PUT, DELETE' always; + add_header 'Access-Control-Allow-Headers' '*' always; + add_header 'Access-Control-Max-Age' 86400 always; + add_header 'Content-Length' 0 always; return 204; } rewrite ^/api(.*) /$1 break; @@ -70,7 +70,7 @@ http { add_header "Access-Control-Allow-Origin" *; add_header "Access-Control-Allow-Methods" "GET, POST, OPTIONS, HEAD, PUT, DELETE"; add_header "Access-Control-Allow-Headers" "*"; - add_header 'Access-Control-Max-Age' 1728000; + add_header 'Access-Control-Max-Age' 86400; add_header 'Content-Length' 0; return 204; } diff --git a/devops/local_conf/nginx.conf b/devops/local_conf/nginx.conf index e4069d17..0507dda3 100644 --- a/devops/local_conf/nginx.conf +++ b/devops/local_conf/nginx.conf @@ -13,18 +13,20 @@ http { server { include /etc/nginx/mime.types; client_max_body_size 10g; + location / { - add_header "Access-Control-Allow-Origin" *; + add_header 'Access-Control-Allow-Origin' '*' always; # Preflighted requests if ($request_method = OPTIONS ) { - add_header "Access-Control-Allow-Origin" *; - add_header "Access-Control-Allow-Methods" "GET, POST, OPTIONS, HEAD, PUT, DELETE"; - add_header "Access-Control-Allow-Headers" "*"; - add_header 'Access-Control-Max-Age' 1728000; - add_header 'Content-Length' 0; + add_header 'Access-Control-Allow-Origin' '*' always; + add_header 'Access-Control-Allow-Methods' 'GET, POST, OPTIONS, HEAD, PUT, DELETE' always; + add_header 'Access-Control-Allow-Headers' '*' always; + add_header 'Access-Control-Max-Age' 86400 always; + add_header 'Content-Length' 0 always; return 204; } + rewrite ^/api(.*) /$1 break; proxy_pass http://geoapi:8000; proxy_http_version 1.1; @@ -39,13 +41,14 @@ http { location /assets { max_ranges 0; expires 30d; - add_header "Access-Control-Allow-Origin" *; + add_header 'Access-Control-Allow-Origin' '*'; + # Preflighted requests if ($request_method = OPTIONS ) { - add_header "Access-Control-Allow-Origin" *; - add_header "Access-Control-Allow-Methods" "GET, POST, OPTIONS, HEAD, PUT, DELETE"; - add_header "Access-Control-Allow-Headers" "*"; - add_header 'Access-Control-Max-Age' 1728000; + add_header 'Access-Control-Allow-Origin' '*'; + add_header 'Access-Control-Allow-Methods' 'GET, POST, OPTIONS, HEAD, PUT, DELETE'; + add_header 'Access-Control-Allow-Headers' '*'; + add_header 'Access-Control-Max-Age' 86400; add_header 'Content-Length' 0; return 204; } diff --git a/geoapi/alembic.ini b/geoapi/alembic.ini index 696b650c..3672ed15 100644 --- a/geoapi/alembic.ini +++ b/geoapi/alembic.ini @@ -6,6 +6,7 @@ script_location = migrations # template used to generate migration files # file_template = %%(rev)s_%%(slug)s +file_template = %%(year)d%%(month).2d%%(day).2d_%%(hour).2d%%(minute).2d-%%(rev)s_%%(slug)s # timezone to use when rendering the date # within the migration file as well as the filename. diff --git a/geoapi/app.py b/geoapi/app.py index f0ba2413..d8c05d48 100644 --- a/geoapi/app.py +++ b/geoapi/app.py @@ -4,7 +4,7 @@ from geoapi.settings import settings as app_settings from geoapi.db import db_session from geoapi.exceptions import (InvalidGeoJSON, InvalidEXIFData, InvalidCoordinateReferenceSystem, - ObservableProjectAlreadyExists, ApiException, StreetviewAuthException, + ProjectSystemPathWatchFilesAlreadyExists, ApiException, StreetviewAuthException, StreetviewLimitException, AuthenticationIssue) import logging @@ -48,9 +48,9 @@ def handle_coordinate_reference_system_exception(error: Exception): return {'message': 'Invalid data, coordinate reference system could not be found'}, 400 -@api.errorhandler(ObservableProjectAlreadyExists) -def handle_observable_project_already_exists_exception(error: Exception): - return {'message': 'Conflict, a project for this storage system/path already exists'}, 409 +@api.errorhandler(ProjectSystemPathWatchFilesAlreadyExists) +def handle_project_system_path_watch_files_already_exists_exception(error: Exception): + return {'message': 'Conflict, a project watching files for this storage system/path already exists'}, 409 @api.errorhandler(StreetviewAuthException) diff --git a/geoapi/celery_app.py b/geoapi/celery_app.py index c7fd9826..4732f42b 100644 --- a/geoapi/celery_app.py +++ b/geoapi/celery_app.py @@ -1,7 +1,8 @@ from celery import Celery -from celery.schedules import crontab +from datetime import timedelta from geoapi.settings import settings + CELERY_CONNECTION_STRING = "amqp://{user}:{pwd}@{hostname}/{vhost}".format( user=settings.RABBITMQ_USERNAME, pwd=settings.RABBITMQ_PASSWD, @@ -15,8 +16,12 @@ include=['geoapi.tasks']) app.conf.beat_schedule = { - 'refresh_observable_projects': { - 'task': 'geoapi.tasks.external_data.refresh_observable_projects', - 'schedule': crontab(hour='*', minute='0') + 'refresh_projects_watch_content': { + 'task': 'geoapi.tasks.external_data.refresh_projects_watch_content', + 'schedule': timedelta(hours=1) + }, + 'refresh_projects_watch_users': { + 'task': 'geoapi.tasks.external_data.refresh_projects_watch_users', + 'schedule': timedelta(minutes=30) } } diff --git a/geoapi/exceptions.py b/geoapi/exceptions.py index 41b345d4..6449c710 100644 --- a/geoapi/exceptions.py +++ b/geoapi/exceptions.py @@ -14,8 +14,8 @@ class InvalidCoordinateReferenceSystem(Exception): pass -class ObservableProjectAlreadyExists(Exception): - """ Observable Project already exists for this path""" +class ProjectSystemPathWatchFilesAlreadyExists(Exception): + """ Project with watch_files True already exists for this system path""" pass diff --git a/geoapi/migrations/versions/20240916_1855-968f358e102a_add_watch_variables_to_project.py b/geoapi/migrations/versions/20240916_1855-968f358e102a_add_watch_variables_to_project.py new file mode 100644 index 00000000..63afcae3 --- /dev/null +++ b/geoapi/migrations/versions/20240916_1855-968f358e102a_add_watch_variables_to_project.py @@ -0,0 +1,68 @@ +"""add_watch_variables_to_project + +Revision ID: 968f358e102a +Revises: 4eeeeea72dbc +Create Date: 2024-09-16 18:55:13.685590 + +""" +from alembic import op +import sqlalchemy as sa +from sqlalchemy.orm import Session +from geoapi.log import logger + + +# revision identifiers, used by Alembic. +revision = '968f358e102a' +down_revision = '4eeeeea72dbc' +branch_labels = None +depends_on = None + + +def upgrade(): + # Add new columns + op.add_column('projects', sa.Column('watch_content', sa.Boolean(), nullable=True)) + op.add_column('projects', sa.Column('watch_users', sa.Boolean(), nullable=True)) + + bind = op.get_bind() + session = Session(bind=bind) + + try: + # Query all projects and their related observable data projects in order + # to set the watch_content and watch_users + projects_query = sa.text(""" + SELECT p.id, odp.id as odp_id, odp.watch_content + FROM projects p + LEFT JOIN observable_data_projects odp ON p.id = odp.project_id + """) + results = session.execute(projects_query) + + # Update projects based on the query results + for project_id, odp_id, odp_watch_content in results: + update_query = sa.text(""" + UPDATE projects + SET watch_content = :watch_content, watch_users = :watch_users + WHERE id = :project_id + """) + session.execute(update_query, { + 'watch_content': odp_watch_content if odp_id is not None else False, + 'watch_users': odp_id is not None, + 'project_id': project_id + }) + session.commit() + logger.info(f"Data migration of project/observable completed successfully") + + except Exception as e: + session.rollback() + logger.exception(f"An error occurred during project/observable data migration: {str(e)}") + raise + finally: + session.close() + + # Drop the unique constraint + op.drop_constraint('observable_data_projects_system_id_path_key', 'observable_data_projects') + + +def downgrade(): + op.drop_column('projects', 'watch_users') + op.drop_column('projects', 'watch_content') + op.create_unique_constraint('observable_data_projects_system_id_path_key', 'observable_data_projects', ['system_id', 'path']) diff --git a/geoapi/models/observable_data.py b/geoapi/models/observable_data.py index 8e6a0e1f..64a1e58e 100644 --- a/geoapi/models/observable_data.py +++ b/geoapi/models/observable_data.py @@ -1,17 +1,15 @@ from sqlalchemy import ( Column, Integer, String, Boolean, - ForeignKey, DateTime, UniqueConstraint + ForeignKey, DateTime ) from sqlalchemy.orm import relationship from sqlalchemy.sql import func from geoapi.db import Base +# Deprecated; Replaced by watch_user and watch_content of Project table. See https://tacc-main.atlassian.net/browse/WG-377 class ObservableDataProject(Base): __tablename__ = 'observable_data_projects' - __table_args__ = ( - UniqueConstraint('system_id', 'path', name="unique_system_id_path"), - ) id = Column(Integer, primary_key=True) project_id = Column(ForeignKey('projects.id', ondelete="CASCADE", onupdate="CASCADE"), index=True) created_date = Column(DateTime(timezone=True), server_default=func.now()) diff --git a/geoapi/models/project.py b/geoapi/models/project.py index 6732aac5..283104c9 100644 --- a/geoapi/models/project.py +++ b/geoapi/models/project.py @@ -29,10 +29,11 @@ class Project(Base): id = Column(Integer, primary_key=True) uuid = Column(UUID(as_uuid=True), default=uuid.uuid4, nullable=False) tenant_id = Column(String, nullable=False) - # Project system_id/system_path really not used except for analytics. - # This could be improved; see https://jira.tacc.utexas.edu/browse/WG-185 + # associated tapis system id system_id = Column(String, nullable=True) + # associated tapis system path system_path = Column(String, nullable=True) + # associated tapis system file system_file = Column(String, nullable=True) name = Column(String, nullable=False) description = Column(String) @@ -47,5 +48,16 @@ class Project(Base): overlaps="project,project_users") point_clouds = relationship('PointCloud', cascade="all, delete-orphan") + # watch content of tapis directory location (system_id and system_path) + watch_content = Column(Boolean, default=False) + + # watch user of tapis system (system_id) + watch_users = Column(Boolean, default=False) + def __repr__(self): - return ''.format(self.id) + return f"" diff --git a/geoapi/routes/projects.py b/geoapi/routes/projects.py index c999a2e3..e0e1cb94 100644 --- a/geoapi/routes/projects.py +++ b/geoapi/routes/projects.py @@ -60,6 +60,12 @@ 'watch_users': fields.Boolean() }) +project_update_payload = api.model('Project', { + 'name': fields.String(), + 'description': fields.String(), + 'public': fields.Boolean(), +}) + project_response = api.model('ProjectResponse', { 'id': fields.Integer(), 'uuid': fields.String(), @@ -225,6 +231,7 @@ def delete(self, projectId: int): @api.doc(id="updateProject", description="Update metadata about a project") + @api.expect(project_update_payload) @api.marshal_with(project_response) @project_permissions def put(self, projectId: int): @@ -232,7 +239,6 @@ def put(self, projectId: int): logger.info("Update project:{} for user:{}".format(projectId, u.username)) return ProjectsService.update(db_session, - user=u, projectId=projectId, data=api.payload) diff --git a/geoapi/services/projects.py b/geoapi/services/projects.py index e56ce2d0..d496e081 100644 --- a/geoapi/services/projects.py +++ b/geoapi/services/projects.py @@ -1,17 +1,15 @@ -from pathlib import Path from typing import List, Optional -from sqlalchemy import desc -from geoapi.models import Project, ProjectUser, User, ObservableDataProject +from sqlalchemy import desc, exists +from geoapi.models import Project, ProjectUser, User from sqlalchemy.sql import select, text -from sqlalchemy.exc import IntegrityError from geoapi.services.users import UserService from geoapi.utils.agave import AgaveUtils, get_system_users from geoapi.utils.users import is_anonymous from geoapi.tasks.external_data import import_from_agave from geoapi.tasks.projects import remove_project_assets from geoapi.log import logger -from geoapi.exceptions import ApiException, ObservableProjectAlreadyExists, GetUsersForProjectNotSupported +from geoapi.exceptions import ApiException, GetUsersForProjectNotSupported, ProjectSystemPathWatchFilesAlreadyExists from geoapi.custom import custom_on_project_creation, custom_on_project_deletion @@ -28,28 +26,30 @@ def create(database_session, data: dict, user: User) -> Project: :param user: User :return: Project """ - watch_content = data.pop("watch_content", False) - watch_users = data.pop("watch_users", False) + # Check that a storage system is there + if data.get('system_id'): + AgaveUtils(database_session, user).systemsGet(data.get('system_id')) - project = Project(**data) + system_id = data.get("system_id") + system_path = data.get("system_path") + + # Check that there is no other matching projects if watch_content is True + if data.get("watch_content") and ProjectsService.is_project_watching_content_on_system_path(database_session, + system_id, + system_path): + logger.exception(f"User:{user.username} tried to create a project with watch_content although there is " + f"a project for that system_id={system_id} and system_path={system_path}") + raise ProjectSystemPathWatchFilesAlreadyExists(f"'{system_id}/{system_path}' project already exists") + project = Project(**data) project.tenant_id = user.tenant_id project.users.append(user) - if watch_users or watch_content: - try: - ProjectsService.makeObservable(database_session, - project, - user, - watch_content) - except Exception as e: - logger.exception("{}".format(e)) - raise e - database_session.add(project) database_session.commit() - # set the user to be the creator + # Now after the above commit, we have project_users, + # and we can now mark the creator. for project_user in project.project_users: if project_user.user_id == user.id: project_user.creator = True @@ -62,65 +62,32 @@ def create(database_session, data: dict, user: User) -> Project: if user.tenant_id.upper() in custom_on_project_creation: custom_on_project_creation[user.tenant_id.upper()](database_session, user, project) + if project.system_id: + try: + system_users = get_system_users(database_session, user, project.system_id) + logger.info(f"Initial update of project_id:{project.id} system_id:{project.system_id} " + f"system_path:{project.system_path} to have the following users: {system_users}") + users = [UserService.getOrCreateUser(database_session, user.username, project.tenant_id) for user in system_users] + project.users = users + + if system_users: + # Initialize the admin status + users_dict = {user.username: user for user in system_users} + for project_user in project.project_users: + project_user.admin = users_dict[project_user.user.username].admin + database_session.add(project) + database_session.commit() + except GetUsersForProjectNotSupported: + logger.info(f"Not getting users for project_id:{project.id} system:{project.system_id}") + + if project.watch_content: + import_from_agave.apply_async(args=[project.tenant_id, + user.id, project.system_id, + project.system_path, + project.id]) setattr(project, 'deletable', True) return project - @staticmethod - def makeObservable(database_session, - proj: Project, - user: User, - watch_content: bool): - """ - Makes a project an observable project - Requires project's system_path, system_id, tenant_id to exist - :param proj: Project - :param user: User - :param watch_content: bool - :return: None - """ - folder_name = Path(proj.system_path).name - name = proj.system_id + '/' + folder_name - - # TODO: Handle no storage system found - AgaveUtils(database_session, user).systemsGet(proj.system_id) - - obs = ObservableDataProject( - system_id=proj.system_id, - path=proj.system_path, - watch_content=watch_content - ) - - system_users = None - try: - system_users = get_system_users(database_session, user, proj.system_id) - logger.info("Initial update of project:{} to have the following users: {}".format(name, system_users)) - users = [UserService.getOrCreateUser(database_session, u.username, tenant=proj.tenant_id) for u in system_users] - proj.users = users - except GetUsersForProjectNotSupported: - logger.info("Not getting users for system:{proj.system_id}") - - obs.project = proj - - try: - database_session.add(obs) - database_session.commit() - except IntegrityError as e: - database_session.rollback() - if 'unique_system_id_path' in str(e.orig): - logger.exception("User:{} tried to create an observable project that already exists: '{}'".format(user.username, name)) - raise ObservableProjectAlreadyExists("'{}' project already exists".format(name)) - else: - raise e - - if system_users: - # Initialize the admin status - users_dict = {u.username: u for u in system_users} - for u in obs.project.project_users: - u.admin = users_dict[u.user.username].admin - - if watch_content: - import_from_agave.apply_async(args=[obs.project.tenant_id, user.id, obs.system_id, obs.path, obs.project_id]) - @staticmethod def list(database_session, user: User) -> List[Project]: """ @@ -133,7 +100,6 @@ def list(database_session, user: User) -> List[Project]: .filter(ProjectUser.user_id == user.id) \ .order_by(desc(Project.created)) \ .all() - for p, u in projects_and_project_user: setattr(p, 'deletable', u.admin or u.creator) @@ -273,22 +239,25 @@ def getFeatures(database_session, projectId: int, query: dict = None) -> object: return out.geojson @staticmethod - def update(database_session, user: User, projectId: int, data: dict) -> Project: + def update(database_session, projectId: int, data: dict) -> Project: """ - Update the metadata associated with a project + Update the metadata associated with a project. + + Only `name`, `description` and `public` can be updated. + :param projectId: int :param data: dict :return: Project """ - proj = ProjectsService.get(database_session=database_session, project_id=projectId) + project = ProjectsService.get(database_session=database_session, project_id=projectId) - proj.name = data.get('name', proj.name) - proj.description = data.get('description', proj.description) - proj.public = data.get('public', proj.public) + project.name = data.get('name', project.name) + project.description = data.get('description', project.description) + project.public = data.get('public', project.public) database_session.commit() - return proj + return project @staticmethod def delete(database_session, user: User, projectId: int) -> dict: @@ -320,9 +289,9 @@ def addUserToProject(database_session, projectId: int, username: str, admin: boo :return: """ - proj = database_session.query(Project).get(projectId) - user = UserService.getOrCreateUser(database_session, username, proj.tenant_id) - proj.users.append(user) + project = database_session.query(Project).get(projectId) + user = UserService.getOrCreateUser(database_session, username, project.tenant_id) + project.users.append(user) database_session.commit() project_user = database_session.query(ProjectUser)\ @@ -333,8 +302,8 @@ def addUserToProject(database_session, projectId: int, username: str, admin: boo @staticmethod def getUsers(database_session, projectId: int) -> List[User]: - proj = database_session.query(Project).get(projectId) - return proj.users + project = database_session.query(Project).get(projectId) + return project.users @staticmethod def getUser(database_session, projectId: int, username: str) -> User: @@ -350,21 +319,30 @@ def removeUserFromProject(database_session, projectId: int, username: str) -> No :param username: str :return: None """ - proj = database_session.query(Project).get(projectId) + project = database_session.query(Project).get(projectId) user = database_session.query(User).filter(User.username == username).first() - observable_project = database_session.query(ObservableDataProject) \ - .filter(ObservableDataProject.id == projectId).first() - if user not in proj.users: + if user not in project.users: raise ApiException("User is not in project") - if len(proj.users) == 1: + if len(project.users) == 1: raise ApiException("Unable to remove last user of project") - if observable_project: - number_of_potential_observers = len([u for u in proj.users if u.jwt]) + if project.watch_content or project.watch_users: + number_of_potential_observers = len([user for user in project.users if user.jwt]) if user.jwt and number_of_potential_observers == 1: - raise ApiException("Unable to remove last user of observable project who can observe file system") + raise ApiException("Unable to remove last user of project who can observe file system or users") - proj.users.remove(user) + project.users.remove(user) database_session.commit() + + @staticmethod + def is_project_watching_content_on_system_path(database_session, system_id, system_path) -> bool: + """ + Check if any project is watching content at the specified system path. + """ + return database_session.query(exists().where( + Project.system_id == system_id, + Project.system_path == system_path, + Project.watch_content + )).scalar() diff --git a/geoapi/services/streetview.py b/geoapi/services/streetview.py index 25013e77..604b9b82 100644 --- a/geoapi/services/streetview.py +++ b/geoapi/services/streetview.py @@ -216,14 +216,14 @@ def createInstance(database_session, streetview_id: int, system_id: str, path: s @staticmethod def getInstances(database_session, projectId: int) -> List[StreetviewInstance]: - proj = database_session.query(Project).get(projectId) - return proj.streetview_instances + project = database_session.query(Project).get(projectId) + return project.streetview_instances @staticmethod def addInstanceToProject(database_session, projectId: int, streetview_instance_id: int) -> None: - proj = database_session.query(Project).get(projectId) + project = database_session.query(Project).get(projectId) streetview_instance = database_session.query(StreetviewInstance).get(streetview_instance_id) - proj.streetview_instances.append(streetview_instance) + project.streetview_instances.append(streetview_instance) database_session.commit() @staticmethod diff --git a/geoapi/tasks/__init__.py b/geoapi/tasks/__init__.py index c08be1b2..7c60fb10 100644 --- a/geoapi/tasks/__init__.py +++ b/geoapi/tasks/__init__.py @@ -1,4 +1,4 @@ from geoapi.tasks.lidar import convert_to_potree -from geoapi.tasks.external_data import import_file_from_agave, import_from_agave, refresh_observable_projects +from geoapi.tasks.external_data import import_file_from_agave, import_from_agave, refresh_projects_watch_content from geoapi.tasks.streetview import publish, from_tapis_to_streetview, process_streetview_sequences from geoapi.tasks.projects import remove_project_assets \ No newline at end of file diff --git a/geoapi/tasks/external_data.py b/geoapi/tasks/external_data.py index 29c73df8..debcbf27 100644 --- a/geoapi/tasks/external_data.py +++ b/geoapi/tasks/external_data.py @@ -7,10 +7,11 @@ import datetime from celery import uuid as celery_uuid import json +from sqlalchemy import true from geoapi.celery_app import app from geoapi.exceptions import InvalidCoordinateReferenceSystem, GetUsersForProjectNotSupported -from geoapi.models import User, ProjectUser, ObservableDataProject, Task +from geoapi.models import Project, User, ProjectUser, Task from geoapi.utils.agave import (AgaveUtils, SystemUser, get_system_users, get_metadata, AgaveFileGetError, AgaveListingError) from geoapi.utils import features as features_util @@ -255,8 +256,8 @@ def import_from_agave(tenant_id: str, userId: int, systemId: str, path: str, pro """ Recursively import files from a system/path. - This method is called regularly refresh_observable_projects() and once when a project is created - where watch content is true + For projects where watch_content is True, this method is called periodically by refresh_projects_watch_content() + and once when the project is initially created (if watch_content is True) """ with create_task_session() as session: import_files_recursively_from_path(session, tenant_id, userId, systemId, path, projectId) @@ -274,7 +275,7 @@ def import_files_recursively_from_path(session, tenant_id: str, userId: int, sys contained in specific-file-format metadata (e.g. exif for images) but instead the location is stored in Tapis metadata. - This method is called by refresh_observable_projects() via import_from_agave + This method is called by refresh_projects_watch_content() via import_from_agave """ user = session.query(User).get(userId) logger.info("Importing for project:{} directory:{}/{} for user:{}".format(projectId, @@ -289,7 +290,6 @@ def import_files_recursively_from_path(session, tenant_id: str, userId: int, sys logger.exception(f"Unable to perform file listing on {systemId}/{path} when importing for project:{projectId}") NotificationsService.create(session, user, "error", f"Error importing as unable to access {systemId}/{path}") return - filenames_in_directory = [str(f.path) for f in listing] for item in listing: if item.type == "dir" and not str(item.path).endswith(".Trash"): @@ -375,90 +375,142 @@ def import_files_recursively_from_path(session, tenant_id: str, userId: int, sys raise +def _get_user_with_valid_token(project): + """ Return a user with valid token + + Returns None if no such user exists. + """ + importing_user = next( + (user for user in project.users if user.has_unexpired_refresh_token() or user.has_valid_token()), None) + return importing_user + + @app.task() -def refresh_observable_projects(): +def refresh_projects_watch_content(): """ - Refresh all observable projects + Refresh users for all projects where watch_content is True """ + start_time = time.time() + with create_task_session() as session: + try: + logger.info("Starting to refresh all projects where watch_content is True") + projects_with_watch_content = session.query(Project).filter(Project.watch_content.is_(true())).all() + for i, project in enumerate(projects_with_watch_content): + try: + importing_user = _get_user_with_valid_token(project) + + if importing_user is None: + logger.error(f"Unable to watch content of project" + f" ({i}/{len(projects_with_watch_content)}): observer:{importing_user} " + f"system:{project.system_id} path:{project.system_path} project:{project.id} " + f"watch_content:{project.watch_content}: No user with an active token found. " + f"So we are skipping (i.e. no update of users or importing of watched " + f"content)") + continue + + # perform the importing + if project.watch_content: + logger.info(f"Refreshing content of project ({i}/{len(projects_with_watch_content)}): " + f"observer:{importing_user} system:{project.system_id} path:{project.system_path} " + f"project:{project.id} watch_content:{project.watch_content}") + import_files_recursively_from_path(session, project.tenant_id, + importing_user.id, project.system_id, + project.system_path, project.id) + except Exception: # noqa: E722 + logger.exception(f"Unhandled exception when importing for project:{project.id}. " + "Performing rollback of current database transaction") + session.rollback() + total_time = time.time() - start_time + logger.info("refresh_projects_watch_content completed. " + "Elapsed time {}".format(datetime.timedelta(seconds=total_time))) + except Exception: # noqa: E722 + logger.error("Error when trying to get list of projects where watch_content is True; " + "this is unexpected and should be reported " + "(i.e. https://jira.tacc.utexas.edu/browse/WG-131).") + raise + - # TODO refactor to consider scaling issues; see https://tacc-main.atlassian.net/browse/WG-47 +@app.task() +def refresh_projects_watch_users(): + """ + Refresh users for all projects where watch_users is True + """ start_time = time.time() with create_task_session() as session: try: - logger.info("Starting to refresh all observable projects") - obs = session.query(ObservableDataProject).all() - for i, o in enumerate(obs): - # TODO_TAPISv3 refactored into a command (used here and by ProjectService) or just put into its own method for clarity? + logger.info("Starting to refresh all projects where watch_content is True") + projects_with_watch_users = session.query(Project).filter(Project.watch_users.is_(true())).all() + for i, project in enumerate(projects_with_watch_users): + # TODO_TAPISv3 refactored into a command (used here and by ProjectService) + # or just put into its own method for clarity? try: - try: - # we need a user with a valid Tapis token for importing files or updating users - importing_user = next( - (u for u in o.project.users if u.has_unexpired_refresh_token() or u.has_valid_token()), - None) - - if importing_user is None: - logger.error(f"Unable to refresh observable project ({i}/{len(obs)}): observer:{importing_user} " - f"system:{o.system_id} path:{o.path} project:{o.project.id} " - f"watch_content:{o.watch_content}: No user with an active token found. So we " - f"are skipping (i.e. no update of users or importing of watched content)") - continue - - logger.info(f"Refreshing observable project ({i}/{len(obs)}): observer:{importing_user} " - f"system:{o.system_id} path:{o.path} project:{o.project.id} watch_content:{o.watch_content}") - - # we need to add any users who have been added to the project/system or update if their admin-status - # has changed - current_users = set([SystemUser(username=u.user.username, admin=u.admin) - for u in o.project.project_users]) - updated_users = set(get_system_users(session, importing_user, o.system_id)) - - current_creator = session.query(ProjectUser)\ - .filter(ProjectUser.project_id == o.id)\ - .filter(ProjectUser.creator is True).one_or_none() - - if current_users != updated_users: - logger.info("Updating users from:{} to:{}".format(current_users, updated_users)) - - # set project users - o.project.users = [UserService.getOrCreateUser(session, u.username, tenant=o.project.tenant_id) - for u in updated_users] - session.add(o) - session.commit() - - updated_users_to_admin_status = {u.username: u for u in updated_users} - logger.info("current_users_to_admin_status:{}".format(updated_users_to_admin_status)) - for u in o.project.project_users: - u.admin = updated_users_to_admin_status[u.user.username].admin - session.add(u) - session.commit() + # we need a user with a valid Tapis token for importing files or updating users + importing_user = next( + (user for user in project.users + if user.has_unexpired_refresh_token() or user.has_valid_token()), None) + + if importing_user is None: + logger.error(f"Unable to watch users of project" + f" ({i}/{len(projects_with_watch_users)}): observer:{importing_user} " + f"system:{project.system_id} path:{project.system_path} project:{project.id} " + f"watch_content:{project.watch_content}: No user with an active token found. " + f"So we are skipping (i.e. no update of users or importing of watched " + f"content)") + continue + logger.info(f"Refreshing users of project ({i}/{len(projects_with_watch_users)}): " + f"observer:{importing_user} system:{project.system_id} path:{project.system_path} " + f"project:{project.id} watch_content:{project.watch_content}") + + # we need to add any users who have been added to the project/system or update + # if their admin-status has changed + current_users = set([SystemUser(username=project_user.user.username, admin=project_user.admin) + for project_user in project.project_users]) + updated_users = set(get_system_users(session, importing_user, project.system_id)) + + current_creator = session.query(ProjectUser)\ + .filter(ProjectUser.project_id == project.id)\ + .filter(ProjectUser.creator is True).one_or_none() + if current_users != updated_users: + logger.info("Updating users from:{} to:{}".format(current_users, updated_users)) + + # set project users + project.users = [UserService.getOrCreateUser(session, user.username, tenant=project.tenant_id) + for user in updated_users] + session.add(project) + session.commit() + + updated_users_to_admin_status = {user.username: user for user in updated_users} + logger.info("current_users_to_admin_status:{}".format(updated_users_to_admin_status)) + for project_user in project.project_users: + project_user.admin = updated_users_to_admin_status[project_user.user.username].admin + session.add(project_user) + session.commit() + + if current_creator: + # reset the creator by finding that updated user again and updating it. + current_creator = session.query(ProjectUser)\ + .filter(ProjectUser.project_id == project.id)\ + .filter(ProjectUser.user_id == current_creator.user_id)\ + .one_or_none() if current_creator: - # reset the creator by finding that updated user again and updating it. - current_creator = session.query(ProjectUser)\ - .filter(ProjectUser.project_id == o.id)\ - .filter(ProjectUser.user_id == current_creator.user_id)\ - .one_or_none() - if current_creator: - current_creator.creator = True - session.add(current_creator) - session.commit() - except GetUsersForProjectNotSupported: - logger.info(f"Not updating users for project:{o.project.id} system_id:{o.system_id}") - pass - - # perform the importing - if o.watch_content: - import_files_recursively_from_path(session, o.project.tenant_id, - importing_user.id, o.system_id, o.path, o.project.id) + current_creator.creator = True + session.add(current_creator) + session.commit() + except GetUsersForProjectNotSupported: + logger.info(f"Not updating users for project:{project.id} " + f"system_id:{project.system_id}") + pass except Exception: # noqa: E722 - logger.exception(f"Unhandled exception when importing observable project:{o.project.id}. " + logger.exception(f"Unhandled exception when updating users for project:{project.id}. " "Performing rollback of current database transaction") session.rollback() total_time = time.time() - start_time - logger.info("refresh_observable_projects completed. " + logger.info("refresh_projects_watch_users completed. " "Elapsed time {}".format(datetime.timedelta(seconds=total_time))) except Exception: # noqa: E722 - logger.error("Error when trying to get list of observable projects; this is unexpected and should be reported" + logger.error("Error when trying to get list of projects where watch_users is True; " + "this is unexpected and should be reported " "(i.e. https://jira.tacc.utexas.edu/browse/WG-131).") raise diff --git a/geoapi/tests/api_tests/test_projects_routes.py b/geoapi/tests/api_tests/test_projects_routes.py index 393f3370..276928e2 100644 --- a/geoapi/tests/api_tests/test_projects_routes.py +++ b/geoapi/tests/api_tests/test_projects_routes.py @@ -63,16 +63,6 @@ def test_get_projects_using_single_uuid(test_client, projects_fixture, projects_ assert data[0]["deletable"] is True -def test_get_projects_using_single_uuid_observable_project(test_client, observable_projects_fixture, user1): - resp = test_client.get('/projects/', - query_string='uuid={}'.format(observable_projects_fixture.project.uuid), - headers={'X-Tapis-Token': user1.jwt}) - data = resp.get_json() - assert resp.status_code == 200 - assert len(data) == 1 - assert data[0]["uuid"] == str(observable_projects_fixture.project.uuid) - - def test_get_projects_using_single_uuid_that_is_wrong(test_client, user1): resp = test_client.get('/projects/', query_string='uuid={}'.format(uuid.uuid4()), @@ -408,21 +398,20 @@ def test_update_project_unauthorized_guest(test_client, public_projects_fixture) assert resp.status_code == 403 -def test_create_observable_project_already_exists(test_client, - projects_fixture, - get_system_users_mock, - observable_projects_fixture, - import_from_agave_mock, - agave_utils_with_geojson_file_mock, - user1): +def test_create_project_watch_content_already_exists(test_client, + watch_content_users_projects_fixture, + get_system_users_mock, + import_from_agave_mock, + agave_utils_with_geojson_file_mock, + user1): data = { - 'name': 'Observable name', - 'description': 'Observable description', - 'system_id': observable_projects_fixture.system_id, - 'system_path': observable_projects_fixture.path, + 'name': 'Project name', + 'description': 'Project description', + 'system_id': watch_content_users_projects_fixture.system_id, + 'system_path': watch_content_users_projects_fixture.system_path, 'system_file': 'testFilename', 'watch_users': True, - 'watch_content': False + 'watch_content': True } resp = test_client.post( @@ -432,23 +421,22 @@ def test_create_observable_project_already_exists(test_client, ) assert resp.status_code == 409 - assert "Conflict, a project for this storage system/path already exists" in resp.json['message'] + assert "Conflict, a project watching files for this storage system/path already exists" in resp.json['message'] -def test_create_observable_project(test_client, - get_system_users_mock, - import_from_agave_mock, - agave_utils_with_geojson_file_mock, user1): +def test_create_project_with_watch_content_watch_users(test_client, + get_system_users_mock, + import_from_agave_mock, + agave_utils_with_geojson_file_mock, user1): data = { - 'name': 'Observable name', - 'description': 'Observable description', + 'name': 'Project name', + 'description': 'Project description', 'system_id': 'testSystem', 'system_path': 'testPath', 'system_file': 'testFilename', 'watch_users': True, 'watch_content': False } - resp = test_client.post('/projects/', json=data, headers={'X-Tapis-Token': user1.jwt}) @@ -456,16 +444,16 @@ def test_create_observable_project(test_client, assert resp.status_code == 200 data = resp.get_json() assert data["deletable"] is True - assert data["name"] == "Observable name" + assert data["name"] == "Project name" proj = db_session.query(Project).get(1) - assert proj.name == "Observable name" + assert proj.name == "Project name" -def test_create_observable_project_unauthorized(test_client): +def test_create_project_unauthorized(test_client): data = { - 'name': 'Observable name', - 'description': 'Observable description', + 'name': 'Project name', + 'description': 'Project description', 'system_id': 'testSystem', 'system_path': 'testPath', 'system_file': 'testFilename', diff --git a/geoapi/tests/api_tests/test_projects_service.py b/geoapi/tests/api_tests/test_projects_service.py index e81d4fce..6b7b6e0e 100644 --- a/geoapi/tests/api_tests/test_projects_service.py +++ b/geoapi/tests/api_tests/test_projects_service.py @@ -5,8 +5,7 @@ from geoapi.services.projects import ProjectsService from geoapi.models import User from geoapi.models.project import Project, ProjectUser -from geoapi.models.observable_data import ObservableDataProject -from geoapi.exceptions import ObservableProjectAlreadyExists +from geoapi.exceptions import ProjectSystemPathWatchFilesAlreadyExists def test_create_project(user1): @@ -14,16 +13,18 @@ def test_create_project(user1): 'name': "test name", 'description': "test description", } - proj = ProjectsService.create(db_session, data, user1) - assert proj.id is not None - assert len(proj.users) == 1 - assert proj.name == "test name" - assert proj.description == "test description" - assert proj.deletable - - assert not db_session.query(ObservableDataProject).all() - matching_project_user = db_session.query(ProjectUser).filter(ProjectUser.user_id == user1.id).one() - assert matching_project_user.creator + project = ProjectsService.create(db_session, data, user1) + assert project.id is not None + assert len(project.users) == 1 + assert project.name == "test name" + assert project.description == "test description" + assert project.deletable + assert not project.public + assert not project.watch_users + assert not project.watch_content + assert project.project_users[0].user_id == user1.id + assert project.project_users[0].creator + assert not project.project_users[0].admin def test_delete_project(projects_fixture, remove_project_assets_mock, user1): @@ -32,10 +33,10 @@ def test_delete_project(projects_fixture, remove_project_assets_mock, user1): assert projects == [] -def test_create_observable_project(user1, - get_system_users_mock, - agave_utils_with_geojson_file_mock, - import_from_agave_mock): +def test_create_watch_users_watch_content_project(user1, + get_system_users_mock, + agave_utils_with_geojson_file_mock, + import_from_agave_mock): data = { 'name': 'test name', 'description': 'test description', @@ -45,9 +46,16 @@ def test_create_observable_project(user1, 'watch_users': True, 'watch_content': True } - proj = ProjectsService.create(db_session, data, user1) + project = ProjectsService.create(db_session, data, user1) + assert project.id is not None + assert len(project.users) == 2 + assert project.name == "test name" + assert project.description == "test description" + assert project.deletable + assert not project.public + assert project.watch_users + assert project.watch_content - assert len(proj.users) == 2 creator_user = db_session.query(ProjectUser).filter(ProjectUser.user_id == user1.id).one() assert creator_user.creator assert creator_user.admin @@ -56,27 +64,23 @@ def test_create_observable_project(user1, assert not other_user.creator assert not other_user.admin - observable_project = db_session.query(ObservableDataProject).one() - assert observable_project.project_id == proj.id - assert observable_project.watch_content - -def test_create_observable_project_already_exists(observable_projects_fixture, - agave_utils_with_geojson_file_mock, - import_from_agave_mock, - get_system_users_mock): +def test_create_watch_content_project_already_exists(watch_content_users_projects_fixture, + agave_utils_with_geojson_file_mock, + import_from_agave_mock, + get_system_users_mock): user = db_session.query(User).get(1) data = { 'name': 'test name', 'description': 'test description', - 'system_id': observable_projects_fixture.system_id, - 'system_path': observable_projects_fixture.path, + 'system_id': watch_content_users_projects_fixture.system_id, + 'system_path': watch_content_users_projects_fixture.system_path, 'system_file': 'file_name', 'watch_users': True, 'watch_content': True } - with pytest.raises(ObservableProjectAlreadyExists): + with pytest.raises(ProjectSystemPathWatchFilesAlreadyExists): ProjectsService.create(db_session, data, user) @@ -116,11 +120,10 @@ def test_get_features_filter_type(projects_fixture, def test_update_project(projects_fixture): - user = db_session.query(User).get(1) data = { 'name': 'new name', 'description': 'new description' } - proj = ProjectsService.update(db_session, user, projects_fixture.id, data) + proj = ProjectsService.update(db_session, projects_fixture.id, data) assert proj.name == "new name" assert proj.description == "new description" diff --git a/geoapi/tests/api_tests/test_users_service.py b/geoapi/tests/api_tests/test_users_service.py index 33054759..b1616b2e 100644 --- a/geoapi/tests/api_tests/test_users_service.py +++ b/geoapi/tests/api_tests/test_users_service.py @@ -45,7 +45,10 @@ def test_add_new_user_to_project(user1): def test_add_existing_user_to_project(user1, user2, projects_fixture): assert not UserService.canAccess(db_session, user2, projects_fixture.id) - ProjectsService.addUserToProject(database_session=db_session, projectId=projects_fixture.id, username=user2.username, admin=False) + ProjectsService.addUserToProject(database_session=db_session, + projectId=projects_fixture.id, + username=user2.username, + admin=False) assert UserService.canAccess(db_session, user2, projects_fixture.id) project_user = db_session.query(ProjectUser).filter(ProjectUser.project_id == projects_fixture.id) \ .filter(ProjectUser.user_id == user2.id).one_or_none() @@ -56,7 +59,10 @@ def test_add_existing_user_to_project(user1, user2, projects_fixture): def test_add_existing_user_to_project_as_admin(user1, user2, projects_fixture): assert not UserService.canAccess(db_session, user2, projects_fixture.id) - ProjectsService.addUserToProject(database_session=db_session, projectId=projects_fixture.id, username=user2.username, admin=True) + ProjectsService.addUserToProject(database_session=db_session, + projectId=projects_fixture.id, + username=user2.username, + admin=True) assert UserService.canAccess(db_session, user2, projects_fixture.id) project_user = db_session.query(ProjectUser).filter(ProjectUser.project_id == projects_fixture.id) \ .filter(ProjectUser.user_id == user2.id).one_or_none() @@ -86,19 +92,9 @@ def test_remove_user_with_only_one_user_failure(projects_fixture): assert len(projects_fixture.users) == 1 -def test_remove_user_from_observable_project(observable_projects_fixture): - project = observable_projects_fixture.project - - ProjectsService.addUserToProject(db_session, project.id, "newUser", admin=False) - assert len(project.users) == 2 - ProjectsService.removeUserFromProject(db_session, project.id, "newUser") - assert len(project.users) == 1 - - -def test_remove_last_user_with_jwt_from_observable_project_failure( - observable_projects_fixture): - project = observable_projects_fixture.project - +def test_remove_last_user_with_jwt_from_watch_users_project_failure( + watch_content_users_projects_fixture): + project = watch_content_users_projects_fixture ProjectsService.addUserToProject(db_session, project.id, "newUser", admin=False) assert len(project.users) == 2 with pytest.raises(ApiException): diff --git a/geoapi/tests/conftest.py b/geoapi/tests/conftest.py index 33ee244e..e9aab917 100644 --- a/geoapi/tests/conftest.py +++ b/geoapi/tests/conftest.py @@ -10,7 +10,6 @@ from geoapi.db import Base, engine, db_session from geoapi.models.users import User from geoapi.models.project import Project, ProjectUser -from geoapi.models.observable_data import ObservableDataProject from geoapi.models.feature import Feature from geoapi.models.task import Task from geoapi.services.point_cloud import PointCloudService @@ -72,44 +71,47 @@ def user2(userdata): @pytest.fixture(scope="function") def projects_fixture(): """ Project with 1 user and test1 is an admin""" - proj = Project(name="test", description="description") + project = Project(name="test", description="description") u1 = db_session.query(User).filter(User.username == "test1").first() - proj.users.append(u1) + project.users.append(u1) - proj.tenant_id = u1.tenant_id - db_session.add(proj) + project.tenant_id = u1.tenant_id + db_session.add(project) db_session.commit() - project_user1 = db_session.query(ProjectUser).filter(ProjectUser.project_id == proj.id).first() + project_user1 = db_session.query(ProjectUser).filter(ProjectUser.project_id == project.id).first() project_user1.admin = True db_session.add(project_user1) db_session.commit() - yield proj + yield project - shutil.rmtree(get_project_asset_dir(proj.id), ignore_errors=True) + shutil.rmtree(get_project_asset_dir(project.id), ignore_errors=True) @pytest.fixture(scope="function") def projects_fixture2(user1, user2): """ Project with 2 users and test1 is creator""" "" - proj = Project(name="test2", description="description2") - proj.users.append(user1) - proj.users.append(user2) - proj.tenant_id = user1.tenant_id - db_session.add(proj) + project = Project(name="test2", description="description2") + project.users.append(user1) + project.users.append(user2) + project.tenant_id = user1.tenant_id + db_session.add(project) db_session.commit() - project_user1 = db_session.query(ProjectUser).filter(ProjectUser.project_id == proj.id).filter(ProjectUser.user_id == user1.id).first() + project_user1 = db_session.query(ProjectUser) \ + .filter(ProjectUser.project_id == project.id) \ + .filter(ProjectUser.user_id == user1.id) \ + .first() project_user1.creator = True db_session.add(project_user1) db_session.commit() - yield proj + yield project - shutil.rmtree(get_project_asset_dir(proj.id), ignore_errors=True) + shutil.rmtree(get_project_asset_dir(project.id), ignore_errors=True) @pytest.fixture(scope="function") @@ -121,35 +123,24 @@ def public_projects_fixture(projects_fixture): @pytest.fixture(scope="function") -def observable_projects_fixture(): +def watch_content_users_projects_fixture(): u1 = db_session.query(User).filter(User.username == "test1").first() - proj = Project(name="test_observable", - description="description", - tenant_id=u1.tenant_id, - system_id="project-1234", - system_path="/testPath", - system_file="system_file") # system_file.hazmapper - obs = ObservableDataProject( - system_id="project-1234", - path="/testPath", - watch_content=True - ) - - # Project system_id/system_path really not used except for analytics. - # This could be improved; see https://jira.tacc.utexas.edu/browse/WG-185 - proj.system_id = obs.system_id - proj.system_path = obs.path - - obs.project = proj - proj.users.append(u1) - db_session.add(obs) - db_session.add(proj) + project = Project(name="test_observable", + description="description", + tenant_id=u1.tenant_id, + system_id="project-1234", + system_path="/testPath", + system_file="system_file", # system_file.hazmapper + watch_content=True, + watch_users=True) + project.users.append(u1) + db_session.add(project) db_session.commit() - proj.project_users[0].creator = True + project.project_users[0].creator = True db_session.commit() - yield obs + yield project - shutil.rmtree(get_project_asset_dir(proj.id), ignore_errors=True) + shutil.rmtree(get_project_asset_dir(project.id), ignore_errors=True) @pytest.fixture(scope="function") diff --git a/geoapi/tests/custom/designsafe/test_project.py b/geoapi/tests/custom/designsafe/test_project.py index 28448a00..b0340c87 100644 --- a/geoapi/tests/custom/designsafe/test_project.py +++ b/geoapi/tests/custom/designsafe/test_project.py @@ -7,8 +7,8 @@ import os -def test_on_project_creation(tapis_url, requests_mock, user1, observable_projects_fixture): - project = observable_projects_fixture.project +def test_on_project_creation(tapis_url, requests_mock, user1, watch_content_users_projects_fixture): + project = watch_content_users_projects_fixture create_file_url = tapis_url + quote(f"/files/media/system/{project.system_id}{project.system_path}/") requests_mock.post(create_file_url) @@ -39,8 +39,8 @@ def test_on_project_creation(tapis_url, requests_mock, user1, observable_project } -def test_on_project_deletion(tapis_url, requests_mock, user1, observable_projects_fixture): - project = observable_projects_fixture.project +def test_on_project_deletion(tapis_url, requests_mock, user1, watch_content_users_projects_fixture): + project = watch_content_users_projects_fixture file_path = f"{project.system_path}/{project.system_file}.hazmapper" delete_file_url = tapis_url + quote(f"/files/media/system/{project.system_id}{file_path}") diff --git a/geoapi/tests/external_data_tests/test_external_data.py b/geoapi/tests/external_data_tests/test_external_data.py index 4f0fa2e4..aed315cf 100644 --- a/geoapi/tests/external_data_tests/test_external_data.py +++ b/geoapi/tests/external_data_tests/test_external_data.py @@ -7,7 +7,8 @@ from geoapi.db import db_session, create_task_session from geoapi.tasks.external_data import (import_from_agave, import_point_clouds_from_agave, - refresh_observable_projects, + refresh_projects_watch_content, + refresh_projects_watch_users, get_additional_files) from geoapi.utils.features import is_member_of_rapp_project_folder from geoapi.utils.agave import AgaveFileListing, SystemUser @@ -178,7 +179,7 @@ def test_external_data_good_files(metadata_geolocation_30long_20lat_fixture, use agave_utils_with_geojson_file.reset_mock() - # run import again (to mimic the periodically scheduled refresh_observable_projects) + # run import again (to mimic the periodically scheduled refresh_projects_watch_content) import_from_agave(projects_fixture.tenant_id, user1.id, "testSystem", "/testPath", projects_fixture.id) # This should only have been called once, since there is only # one FILE in the listing @@ -197,10 +198,10 @@ def test_external_data_bad_files(metadata_none_fixture, user1, projects_fixture, agave_utils_with_bad_image_file.client_in_external_data.reset_mock() - # run import again (to mimic the periodically scheduled refresh_observable_projects) + # run import again (to mimic the periodically scheduled refresh_projects_watch_content) import_from_agave(projects_fixture.tenant_id, user1.id, "testSystem", "/testPath", projects_fixture.id) # Getting the file should only have been called once, since there is only - # one FILE in the listing and we already attempted to import it in the first call + # one FILE in the listing, and we already attempted to import it in the first call # to import_from_agave agave_utils_with_bad_image_file.client_in_external_data.getFile.assert_not_called() @@ -363,38 +364,52 @@ def test_import_from_agave_failed_dbsession_rollback(agave_utils_with_geojson_fi @pytest.mark.worker -def test_refresh_observable_projects(metadata_but_no_geolocation_fixture, - user1, - user2, - agave_utils_with_geojson_file, - observable_projects_fixture, - get_system_users_mock, - caplog): - assert len(observable_projects_fixture.project.project_users) == 1 +def test_refresh_projects_watch_users(metadata_but_no_geolocation_fixture, + user1, + user2, + agave_utils_with_geojson_file, + watch_content_users_projects_fixture, + get_system_users_mock, + caplog): + assert len(watch_content_users_projects_fixture.project_users) == 1 # single user with no admin but is creator assert [(user1.username, False, True)] == [(u.user.username, u.admin, u.creator) - for u in observable_projects_fixture.project.project_users] + for u in watch_content_users_projects_fixture.project_users] - refresh_observable_projects() + refresh_projects_watch_users() - db_session.refresh(observable_projects_fixture.project) + db_session.refresh(watch_content_users_projects_fixture) + + assert "rollback" not in caplog.text + + # now two users with one being the admin and creator + assert ([(user1.username, True, True), (user2.username, False, False)] == + [(u.user.username, u.admin, u.creator) for u in watch_content_users_projects_fixture.project_users]) + + +@pytest.mark.worker +def test_refresh_projects_watch_content(metadata_but_no_geolocation_fixture, + agave_utils_with_geojson_file, + watch_content_users_projects_fixture, + get_system_users_mock, + caplog): + refresh_projects_watch_content() + + db_session.refresh(watch_content_users_projects_fixture) assert "rollback" not in caplog.text features = db_session.query(Feature).all() # the test geojson has 3 features in it assert len(features) == 3 - # now two users with one being the admin and creator - assert [(user1.username, True, True), (user2.username, False, False)] == [(u.user.username, u.admin, u.creator) - for u in observable_projects_fixture.project.project_users] @pytest.mark.worker -def test_refresh_observable_projects_dbsession_rollback(agave_utils_with_geojson_file, - observable_projects_fixture, - get_system_users_mock, - task_session_commit_throws_exception, - caplog): - refresh_observable_projects() +def test_refresh_projects_watch_content_dbsession_rollback(agave_utils_with_geojson_file, + watch_content_users_projects_fixture, + get_system_users_mock, + task_session_commit_throws_exception, + caplog): + refresh_projects_watch_content() assert "rollback" in caplog.text diff --git a/geoapi/tools/check_observable_projects.py b/geoapi/tools/check_observable_projects.py new file mode 100644 index 00000000..32ed3c3a --- /dev/null +++ b/geoapi/tools/check_observable_projects.py @@ -0,0 +1,30 @@ +from geoapi.models import ObservableDataProject, Project +from geoapi.db import create_task_session +from geoapi.log import logger + +# Check that migrations worked out as expected +# Remove this file with https://tacc-main.atlassian.net/browse/WG-377 + +with create_task_session() as session: + observable_projects = session.query(ObservableDataProject).all() + projects = session.query(Project).all() + + logger.info(f"Checking observable_projects ({len(observable_projects)}) and projects ({len(projects)})") + + issues = 0 + for obs in observable_projects: + try: + if not obs.project.watch_users or obs.watch_content != obs.project.watch_content: + issues += 1 + logger.info(f"Issue with observable project (id:{obs.id} project_id:{obs.project_id}):\n" + f" obs.project.watch_users:{obs.project.watch_users}\n" + f" obs.project.watch_content:{obs.project.watch_content}\n" + f" obs.project.watch_content:{obs.watch_content}\n") + except Exception: + issues += 1 + logger.exception(f"Something happened when checking observable project" + f" (id:{obs.id} project_id:{obs.project_id})") + + logger.info("\n\n\nDone checking.\n\nNote some discrepancies could occur (i.e. projects are deleted but then deprecated" + " observable project is not deleted) \n\n") + logger.info(f"{issues} issues found.")