From 4c25a81004252d8d154fa73c903cd2a673a54ae9 Mon Sep 17 00:00:00 2001 From: James Moffet Date: Tue, 26 Jul 2022 11:17:43 -0700 Subject: [PATCH] disable cache until we can fix on cloud deployment --- app/config.py | 4 ++- app/main/views/platform_admin.py | 26 ++++++++------- app/main/views/send.py | 32 +++++++++++++++++-- app/notify_client/job_api_client.py | 12 ++++--- app/notify_client/organisations_api_client.py | 11 ++++--- app/notify_client/service_api_client.py | 15 ++++++--- .../template_folder_api_client.py | 2 +- app/s3_client/s3_csv_client.py | 8 +++-- app/s3_client/s3_logo_client.py | 6 +++- app/utils/user.py | 3 ++ manifest.yml | 2 +- 11 files changed, 88 insertions(+), 33 deletions(-) diff --git a/app/config.py b/app/config.py index ecb4e3a2b..305663ba7 100644 --- a/app/config.py +++ b/app/config.py @@ -11,6 +11,8 @@ if os.environ.get('VCAP_APPLICATION'): class Config(object): + NOTIFY_ADMIN_API_CACHE_ENABLED = False + ADMIN_CLIENT_SECRET = os.environ.get('ADMIN_CLIENT_SECRET') ADMIN_CLIENT_USER_NAME = os.environ.get('ADMIN_CLIENT_USERNAME') API_HOST_NAME = os.environ.get('API_HOST_NAME', 'localhost') @@ -41,7 +43,7 @@ class Config(object): HEADER_COLOUR = '#81878b' # mix(govuk-colour("dark-grey"), govuk-colour("mid-grey")) HTTP_PROTOCOL = 'http' NOTIFY_APP_NAME = 'admin' - NOTIFY_LOG_LEVEL = os.environ.get('NOTIFY_LOG_LEVEL', 'DEBUG') + NOTIFY_LOG_LEVEL = os.environ.get('NOTIFY_LOG_LEVEL', 'INFO') PERMANENT_SESSION_LIFETIME = 20 * 60 * 60 # 20 hours SEND_FILE_MAX_AGE_DEFAULT = 365 * 24 * 60 * 60 # 1 year SESSION_COOKIE_HTTPONLY = True diff --git a/app/main/views/platform_admin.py b/app/main/views/platform_admin.py index 260a2e486..71d82bebf 100644 --- a/app/main/views/platform_admin.py +++ b/app/main/views/platform_admin.py @@ -3,7 +3,7 @@ import re from collections import OrderedDict from datetime import datetime -from flask import abort, flash, redirect, render_template, request, url_for +from flask import abort, flash, redirect, render_template, request, url_for, current_app from notifications_python_client.errors import HTTPError from app import ( @@ -460,12 +460,13 @@ def platform_admin_returned_letters(): try: letter_jobs_client.submit_returned_letters(references) - redis_client.delete_by_pattern( - 'service-????????-????-????-????-????????????-returned-letters-statistics' - ) - redis_client.delete_by_pattern( - 'service-????????-????-????-????-????????????-returned-letters-summary' - ) + if current_app.config['NOTIFY_ADMIN_API_CACHE_ENABLED']: + redis_client.delete_by_pattern( + 'service-????????-????-????-????-????????????-returned-letters-statistics' + ) + redis_client.delete_by_pattern( + 'service-????????-????-????-????-????????????-returned-letters-summary' + ) except HTTPError as error: if error.status_code == 400: error_references = [ @@ -538,10 +539,13 @@ def clear_cache(): groups = map(CACHE_KEYS.get, group_keys) patterns = list(itertools.chain(*groups)) - num_deleted = sum( - redis_client.delete_by_pattern(pattern) - for pattern in patterns - ) + if current_app.config['NOTIFY_ADMIN_API_CACHE_ENABLED']: + num_deleted = sum( + redis_client.delete_by_pattern(pattern) + for pattern in patterns + ) + else: + num_deleted = 0 msg = ( f'Removed {num_deleted} objects ' diff --git a/app/main/views/send.py b/app/main/views/send.py index 200bfbc4b..ebe42d742 100644 --- a/app/main/views/send.py +++ b/app/main/views/send.py @@ -598,12 +598,14 @@ def send_from_contact_list(service_id, template_id, contact_list_id): def _check_messages(service_id, template_id, upload_id, preview_row, letters_as_pdf=False): - + current_app.logger.info('Running _check_messages') try: # The happy path is that the job doesn’t already exist, so the # API will return a 404 and the client will raise HTTPError. + current_app.logger.info('Requesting job from api with upload_id/job_id: {}'.format(upload_id)) job_api_client.get_job(service_id, upload_id) - + + current_app.logger.info('Job already exists for upload_id/job_id: {}, sending 302'.format(upload_id)) # the job exists already - so go back to the templates page # If we just return a `redirect` (302) object here, we'll get # errors when we try and unpack in the check_messages route. @@ -614,15 +616,25 @@ def _check_messages(service_id, template_id, upload_id, preview_row, letters_as_ template_id=template_id )) except HTTPError as e: + current_app.logger.info('Job does not exist for upload_id/job_id: {}'.format(upload_id)) if e.status_code != 404: + current_app.logger.info('Expected 404 when fetching job with upload_id/job_id: {}, instead got: {}'.format(upload_id, e.status_code)) raise + current_app.logger.info('_check_messages is now evaluating uploaded file') + notification_count = service_api_client.get_notification_count(service_id) + current_app.logger.info('_check_messages notification_count is {}'.format(notification_count)) remaining_messages = (current_service.message_limit - notification_count) + current_app.logger.info('_check_messages remaining_messages is {}'.format(remaining_messages)) contents = s3download(service_id, upload_id) + + current_app.logger.info('_check_messages obtained file contents' ) db_template = current_service.get_template_with_user_permission_or_403(template_id, current_user) + + current_app.logger.info('_check_messages got db template with type: {}'.format(db_template['template_type'])) email_reply_to = None sms_sender = None @@ -631,6 +643,8 @@ def _check_messages(service_id, template_id, upload_id, preview_row, letters_as_ elif db_template['template_type'] == 'sms': sms_sender = get_sms_sender_from_session() + current_app.logger.info('_check_messages creating template from db_template') + template = get_template( db_template, current_service, @@ -650,6 +664,7 @@ def _check_messages(service_id, template_id, upload_id, preview_row, letters_as_ # recalculate the page count once we have the values page_count=get_page_count_for_letter(db_template), ) + current_app.logger.info('_check_messages creating recipients from file contents') recipients = RecipientCSV( contents, template=template, @@ -681,8 +696,11 @@ def _check_messages(service_id, template_id, upload_id, preview_row, letters_as_ page_count = get_page_count_for_letter(db_template, template.values) template.page_count = page_count + + current_app.logger.info('_check_messages getting csv metadata') original_file_name = get_csv_metadata(service_id, upload_id).get('original_file_name', '') + current_app.logger.info('_check_messages returning dict') return dict( recipients=recipients, template=template, @@ -720,7 +738,7 @@ def _check_messages(service_id, template_id, upload_id, preview_row, letters_as_ ) @user_has_permissions('send_messages', restrict_admin_usage=True) def check_messages(service_id, template_id, upload_id, row_index=2): - + current_app.logger.info('Check messages, getting data from upload with id {}'.format(upload_id)) data = _check_messages(service_id, template_id, upload_id, row_index) data['allowed_file_extensions'] = Spreadsheet.ALLOWED_FILE_EXTENSIONS @@ -732,17 +750,21 @@ def check_messages(service_id, template_id, upload_id, row_index=2): or data['recipients'].missing_column_headers or data['sent_previously'] ): + current_app.logger.info('Found column errors in upload') return render_template('views/check/column-errors.html', **data) if data['row_errors']: + current_app.logger.info('Found row errors in upload') return render_template('views/check/row-errors.html', **data) if ( data['errors'] or data['trying_to_send_letters_in_trial_mode'] ): + current_app.logger.info('Found other errors in upload') return render_template('views/check/column-errors.html', **data) + current_app.logger.info('Writing upload metadata') metadata_kwargs = { 'notification_count': data['count_of_recipients'], 'template_id': template_id, @@ -754,8 +776,10 @@ def check_messages(service_id, template_id, upload_id, row_index=2): # sender_id is not an option for sending letters. metadata_kwargs['sender_id'] = session['sender_id'] + current_app.logger.info('Setting upload metadata') set_metadata_on_csv_upload(service_id, upload_id, **metadata_kwargs) + current_app.logger.info('Returning 200 from check messages') return render_template('views/check/ok.html', **data) @@ -769,11 +793,13 @@ def check_messages(service_id, template_id, upload_id, row_index=2): ) @user_has_permissions('send_messages') def check_messages_preview(service_id, template_id, upload_id, filetype, row_index=2): + current_app.logger.info('Check messages preview, checking filetype') if filetype == 'pdf': page = None elif filetype == 'png': page = request.args.get('page', 1) else: + current_app.logger.info('Check messages preview, filetype is neither pdf nor png, so we return 404') abort(404) template = _check_messages( diff --git a/app/notify_client/job_api_client.py b/app/notify_client/job_api_client.py index 60a2d295e..5d4d1735b 100644 --- a/app/notify_client/job_api_client.py +++ b/app/notify_client/job_api_client.py @@ -1,5 +1,6 @@ from app.extensions import redis_client from app.notify_client import NotifyAdminAPIClient, _attach_current_user, cache +from flask import current_app class JobApiClient(NotifyAdminAPIClient): @@ -100,11 +101,12 @@ class JobApiClient(NotifyAdminAPIClient): data = _attach_current_user(data) job = self.post(url='/service/{}/job'.format(service_id), data=data) - redis_client.set( - 'has_jobs-{}'.format(service_id), - b'true', - ex=int(cache.DEFAULT_TTL), - ) + if current_app.config['NOTIFY_ADMIN_API_CACHE_ENABLED']: + redis_client.set( + 'has_jobs-{}'.format(service_id), + b'true', + ex=int(cache.DEFAULT_TTL), + ) return job diff --git a/app/notify_client/organisations_api_client.py b/app/notify_client/organisations_api_client.py index 140fe9411..badbcda5b 100644 --- a/app/notify_client/organisations_api_client.py +++ b/app/notify_client/organisations_api_client.py @@ -5,6 +5,8 @@ from notifications_python_client.errors import HTTPError from app.extensions import redis_client from app.notify_client import NotifyAdminAPIClient, cache +from flask import current_app + class OrganisationsClient(NotifyAdminAPIClient): @@ -53,11 +55,12 @@ class OrganisationsClient(NotifyAdminAPIClient): def update_organisation(self, org_id, cached_service_ids=None, **kwargs): api_response = self.post(url="/organisations/{}".format(org_id), data=kwargs) - if cached_service_ids: - redis_client.delete(*map('service-{}'.format, cached_service_ids)) + if current_app.config['NOTIFY_ADMIN_API_CACHE_ENABLED']: + if cached_service_ids: + redis_client.delete(*map('service-{}'.format, cached_service_ids)) - if 'name' in kwargs: - redis_client.delete(f'organisation-{org_id}-name') + if 'name' in kwargs: + redis_client.delete(f'organisation-{org_id}-name') return api_response diff --git a/app/notify_client/service_api_client.py b/app/notify_client/service_api_client.py index 06c9f7d73..955c2be6b 100644 --- a/app/notify_client/service_api_client.py +++ b/app/notify_client/service_api_client.py @@ -1,5 +1,5 @@ from datetime import datetime - +from flask import current_app from notifications_utils.clients.redis import daily_limit_cache_key from app.extensions import redis_client @@ -138,7 +138,7 @@ class ServiceAPIClient(NotifyAdminAPIClient): # @cache.delete('service-{service_id}-templates') # @cache.delete_by_pattern('service-{service_id}-template-*') def archive_service(self, service_id, cached_service_user_ids): - if cached_service_user_ids: + if cached_service_user_ids and current_app.config['NOTIFY_ADMIN_API_CACHE_ENABLED']: redis_client.delete(*map('user-{}'.format, cached_service_user_ids)) return self.post('/service/{}/archive'.format(service_id), data=None) @@ -595,7 +595,7 @@ class ServiceAPIClient(NotifyAdminAPIClient): broadcast channel is one of "operator", "test", "severe", "government" provider_restriction is one of "all", "three", "o2", "vodafone", "ee" """ - if cached_service_user_ids: + if cached_service_user_ids and current_app.config['NOTIFY_ADMIN_API_CACHE_ENABLED']: redis_client.delete(*map('user-{}'.format, cached_service_user_ids)) data = { @@ -608,7 +608,14 @@ class ServiceAPIClient(NotifyAdminAPIClient): def get_notification_count(self, service_id): # if cache is not set return 0 - count = redis_client.get(daily_limit_cache_key(service_id)) or 0 + current_app.logger.info("Pinging redis for daily_limit_cache_key(service_id): {}".format(daily_limit_cache_key(service_id))) + current_app.logger.info("Redis url is: {}".format( current_app.config['REDIS_URL'] )) + current_app.logger.info("Redis enabled is: {}".format( current_app.config['REDIS_ENABLED'] )) + + if current_app.config['NOTIFY_ADMIN_API_CACHE_ENABLED']: + count = redis_client.get(daily_limit_cache_key(service_id)) or 0 + else: + count = 0 return int(count) diff --git a/app/notify_client/template_folder_api_client.py b/app/notify_client/template_folder_api_client.py index c0646665b..f3d8b9c8b 100644 --- a/app/notify_client/template_folder_api_client.py +++ b/app/notify_client/template_folder_api_client.py @@ -48,7 +48,7 @@ class TemplateFolderAPIClient(NotifyAdminAPIClient): 'folders': list(folder_ids), }) - if template_ids: + if template_ids and current_app.config['NOTIFY_ADMIN_API_CACHE_ENABLED']: redis_client.delete(*(f'service-{service_id}-template-{id}-version-None' for id in template_ids)) # @cache.delete('service-{service_id}-template-folders') diff --git a/app/s3_client/s3_csv_client.py b/app/s3_client/s3_csv_client.py index f7217ef25..d78148db8 100644 --- a/app/s3_client/s3_csv_client.py +++ b/app/s3_client/s3_csv_client.py @@ -17,7 +17,8 @@ def get_csv_location(service_id, upload_id, bucket=None): def get_csv_upload(service_id, upload_id, bucket=None): - return get_s3_object(*get_csv_location(service_id, upload_id, bucket)) + s3_object = get_s3_object(*get_csv_location(service_id, upload_id, bucket)) + return s3_object def s3upload(service_id, filedata, region, bucket=None): @@ -45,7 +46,9 @@ def s3download(service_id, upload_id, bucket=None): def set_metadata_on_csv_upload(service_id, upload_id, bucket=None, **kwargs): - get_csv_upload( + current_app.logger.info('set_metadata_on_csv_upload, service_id: {} upload_id: {} bucket: {}'.format(service_id, upload_id, bucket)) + current_app.logger.info('csv location to copy from is: {}/{}'.format(*get_csv_location(service_id, upload_id, bucket=bucket))) + copy_from_object_result = get_csv_upload( service_id, upload_id, bucket=bucket ).copy_from( CopySource='{}/{}'.format(*get_csv_location(service_id, upload_id, bucket=bucket)), @@ -55,6 +58,7 @@ def set_metadata_on_csv_upload(service_id, upload_id, bucket=None, **kwargs): }, MetadataDirective='REPLACE', ) + return copy_from_object_result def get_csv_metadata(service_id, upload_id, bucket=None): diff --git a/app/s3_client/s3_logo_client.py b/app/s3_client/s3_logo_client.py index 3e280bc25..4c70579f8 100644 --- a/app/s3_client/s3_logo_client.py +++ b/app/s3_client/s3_logo_client.py @@ -13,7 +13,11 @@ LETTER_TEMP_LOGO_LOCATION = 'letters/static/images/letter-template/temp-{user_id def get_s3_object(bucket_name, filename): s3 = resource('s3') - return s3.Object(bucket_name, filename) + current_app.logger.info('Requesting s3 object from bucket: {} with key: {}'.format(bucket_name, filename)) + obj = s3.Object(bucket_name, filename) + string_body = obj.get()['Body'].read().decode('utf-8') + current_app.logger.info('s3 Object string body is: {}'.format(string_body)) + return obj def delete_s3_object(filename): diff --git a/app/utils/user.py b/app/utils/user.py index 23a12ccb1..5658a471c 100644 --- a/app/utils/user.py +++ b/app/utils/user.py @@ -19,9 +19,12 @@ def user_has_permissions(*permissions, **permission_kwargs): def wrap(func): @wraps(func) def wrap_func(*args, **kwargs): + current_app.logger.info('Checking user permissions') if not current_user.is_authenticated: + current_app.logger.info('User is not authenticated') return current_app.login_manager.unauthorized() if not current_user.has_permissions(*permissions, **permission_kwargs): + current_app.logger.info('Authenticated user does not have appropriate permissions') abort(403) return func(*args, **kwargs) return wrap_func diff --git a/manifest.yml b/manifest.yml index 68080ef65..ba66d920f 100644 --- a/manifest.yml +++ b/manifest.yml @@ -18,7 +18,7 @@ applications: env: NOTIFY_APP_NAME: admin NOTIFY_LOG_PATH: /home/vcap/logs/app.log - NOTIFY_LOG_LEVEL: DEBUG + NOTIFY_LOG_LEVEL: INFO FLASK_APP: application.py FLASK_ENV: production REDIS_ENABLED: ((REDIS_ENABLED))