Skip to content

Commit

Permalink
style: manual reformatting to comply with ruff
Browse files Browse the repository at this point in the history
refs: ATV-201
  • Loading branch information
voneiden committed Jan 3, 2025
1 parent ab62c23 commit 7c60de8
Show file tree
Hide file tree
Showing 18 changed files with 401 additions and 233 deletions.
8 changes: 5 additions & 3 deletions atv/authentication.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,11 @@ def authenticate(self, request):
if not service_key.user:
raise exceptions.AuthenticationFailed("Permissions missing from API key.")

# Verified key is cached for 5 minutes. This improves subsequent requests' response times significantly.
# Default cache uses local memory caching. Keys aren't accessible from console or to other threads or pods
# TODO: if caching is refactored to use Redis or Memcached this needs to be reconsidered
# Verified key is cached for 5 minutes. This improves subsequent requests'
# response times significantly. Default cache uses local memory caching. Keys
# aren't accessible from console or to other threads or pods
# TODO: if caching is refactored to use Redis or Memcached this needs to be
# reconsidered
cache.set(key, service_key, timeout=60 * 5)

return service_key.user, service_key
5 changes: 3 additions & 2 deletions atv/decorators.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,9 @@ def permission_checker(request):

def staff_required(required_permission=ServicePermissions.VIEW_DOCUMENTS):
"""
Returns a decorator that checks if the user has staff permission on resources for the service
specified in the request. Required permission can be defined as an argument, defaults to 'view'.
Returns a decorator that checks if the user has staff permission on resources for
the service specified in the request. Required permission can be defined as an
argument, defaults to 'view'.
"""

if not isinstance(required_permission, ServicePermissions):
Expand Down
6 changes: 4 additions & 2 deletions audit_log/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@ def send_audit_log_entries():

if not (settings.ELASTIC_HOST and settings.ELASTIC_AUDIT_LOG_INDEX):
logger.warning(
"Trying to send audit logs to Elasticsearch without proper configuration, process skipped"
"Trying to send audit logs to Elasticsearch without proper configuration,"
" process skipped"
)
return

Expand Down Expand Up @@ -63,5 +64,6 @@ def clear_audit_log_entries(days_to_keep=30):
if count := sent_entries.count():
sent_entries.delete()
logger.info(
f"Cleared {count} sent audit logs, which were older than {days_to_keep} days."
f"Cleared {count} sent audit logs, which were older than {days_to_keep}"
" days."
)
3 changes: 2 additions & 1 deletion audit_log/viewsets.py
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,8 @@ def _get_ip_address(self) -> str:
]
for ip in forwarded_for:
try:
# This regexp matches IPv4 addresses without including the port number
# This regexp matches IPv4 addresses without including the port
# number
regexp_for_ipv4 = re.match(
r"(^((25[0-5]|(2[0-4]|1\d|[1-9]|)\d)\.?\b){4})", ip
)
Expand Down
512 changes: 321 additions & 191 deletions documents/api/docs.py

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions documents/api/filtersets.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,8 @@ def filter(self, qs, value):
raise ValidationError(
detail={
"Invalid Query": [
"Enter query in format 'key:value' without quotes."
" You can have multiple key and value pairs, separated by comma"
"Enter query in format 'key:value' without quotes. You can"
" have multiple key and value pairs, separated by comma"
]
}
)
Expand Down
3 changes: 2 additions & 1 deletion documents/api/querysets.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,8 @@ def get_document_metadata_queryset(
user: User, api_key: ServiceAPIKey = None
) -> QuerySet:
"""
Superusers and staff(API key) are allowed to see document metadata of all services for all users.
Superusers and staff(API key) are allowed to see document metadata of all services
for all users.
Token users can only see their own documents metadata from across all services.
"""
queryset = (
Expand Down
6 changes: 4 additions & 2 deletions documents/api/viewsets.py
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,8 @@ def get_queryset(self):
service_api_key = get_service_api_key_from_request(self.request)
return get_document_metadata_queryset(user, service_api_key)

# Use retrieve to allow using user__uuid as a lookup_field to list documents of single user
# Use retrieve to allow using user__uuid as a lookup_field to list documents of
# single user
def retrieve(self, request, *args, **kwargs):
with self.record_action():
service_api_key = get_service_api_key_from_request(request)
Expand Down Expand Up @@ -502,7 +503,8 @@ def create(self, request, *args, **kwargs):

serializer = CreateAnonymousDocumentSerializer(data=data)

# If the data is not valid, it will raise a ValidationError and return Bad Request
# If the data is not valid, it will raise a ValidationError and return Bad
# Request
serializer.is_valid(raise_exception=True)

with self.record_action():
Expand Down
5 changes: 4 additions & 1 deletion documents/management/commands/delete_expired_documents.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,10 @@


class Command(BaseCommand):
help = "Delete documents and related objects and files that have reached their deletion date"
help = (
"Delete documents and related objects and files that have reached their"
" deletion date"
)

def handle(self, *args, **kwargs):
delete_expired_documents()
3 changes: 2 additions & 1 deletion documents/management/commands/remove_outdated_files.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@ class Command(BaseCommand):
files_deleted = 0

def remove_document_outdated_files(self, document: Document, path, dry_run):
"""For a given document, remove the files that don't have an associated Attachment"""
"""For a given document, remove the files that don't have an associated
Attachment"""
document_path = get_document_attachment_directory_path(document)

for filename in os.listdir(path):
Expand Down
19 changes: 12 additions & 7 deletions documents/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,8 @@ class Activity(models.Model):
default=dict,
blank=True,
help_text=_(
"Structure with links related to activity with display text in multiple languages"
"Structure with links related to activity with display text in multiple"
" languages"
),
)
show_to_user = models.BooleanField(
Expand Down Expand Up @@ -155,17 +156,19 @@ class Document(UUIDModel, TimestampedModel):
verbose_name=_("TOS function ID"),
help_text=_(
"UUID without dashes. Should correspond with a Function instance "
"(e.g. the id from https://api.hel.fi/helerm/v1/function/eb30af1d9d654ebc98287ca25f231bf6/) "
"which is applied to the stored document when considering storage time."
"(e.g. the id from"
" https://api.hel.fi/helerm/v1/function/eb30af1d9d654ebc98287ca25f231bf6/"
") which is applied to the stored document when considering storage time."
),
)
tos_record_id = models.CharField(
max_length=32,
verbose_name=_("TOS record ID"),
help_text=_(
"UUID without dashes. Should correspond to a record ID "
"(e.g. records[].id from https://api.hel.fi/helerm/v1/function/eb30af1d9d654ebc98287ca25f231bf6/) "
"within a Function instance which is applied to the stored document when "
"(e.g. records[].id from"
" https://api.hel.fi/helerm/v1/function/eb30af1d9d654ebc98287ca25f231bf6/"
") within a Function instance which is applied to the stored document when "
"considering storage time."
),
)
Expand Down Expand Up @@ -224,7 +227,8 @@ class Document(UUIDModel, TimestampedModel):
default=timezone.now,
verbose_name=_("status_timestamp"),
help_text=_(
"Date and time when document status was last changed. Field is automatically set."
"Date and time when document status was last changed. Field is"
" automatically set."
),
)
type = models.CharField(
Expand Down Expand Up @@ -264,7 +268,8 @@ class Document(UUIDModel, TimestampedModel):
delete_after = models.DateField(
null=True,
help_text=_(
"Date which after the document and related attachments are permanently deleted"
"Date which after the document and related attachments are permanently"
" deleted"
),
)

Expand Down
21 changes: 13 additions & 8 deletions documents/serializers/document.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,8 @@ class DocumentStatisticsSerializer(serializers.ModelSerializer):
source="service.name", required=False, read_only=True
)
attachments = AttachmentNameSerializer(many=True)
# Attachment count included here just for clarity. Field is added to response body in to_representation
# Attachment count included here just for clarity. Field is added to
# response body in to_representation
attachment_count = serializers.HiddenField(default=0)
user_id = serializers.UUIDField(source="user.uuid", read_only=True)

Expand All @@ -69,7 +70,8 @@ class Meta:

def to_representation(self, instance):
representation = super().to_representation(instance)
# Calculate attachment count here instead of aggregating it to queryset for performance reasons
# Calculate attachment count here instead of aggregating it to queryset
# for performance reasons
representation["attachment_count"] = len(representation["attachments"])
return representation

Expand Down Expand Up @@ -142,7 +144,7 @@ def to_representation(self, instance):


class DocumentSerializer(serializers.ModelSerializer):
"""Basic "read" serializer for the Document model"""
"""Basic "read" serializer for the Document model."""

user_id = serializers.UUIDField(
source="user.uuid", required=False, default=None, read_only=True
Expand Down Expand Up @@ -189,7 +191,8 @@ def update(self, document, validated_data):
# If the document has been locked, no further updates are allowed
if document.locked_after and document.locked_after <= now():
raise DocumentLockedException()
# Deletable field can be changed from True to False but not the other way
# Deletable field can be changed from True to False but not the other
# way
if validated_data.get("deletable") is True and not document.deletable:
raise PermissionDenied(
detail="Field 'deletable' can't be changed if set to False"
Expand Down Expand Up @@ -218,10 +221,11 @@ def to_representation(self, instance):


class CreateAnonymousDocumentSerializer(serializers.ModelSerializer):
"""Create a Document with Attachment for an anonymous user submitting the document
through a Service authorized with an API key.
"""Create a Document with Attachment for an anonymous user submitting the
document through a Service authorized with an API key.
Also handles the creation of the associated Attachments through `CreateAttachmentSerializer`.
Also handles the creation of the associated Attachments through
`CreateAttachmentSerializer`.
"""

user_id = serializers.UUIDField(source="user.uuid", required=False, default=None)
Expand Down Expand Up @@ -254,7 +258,8 @@ class Meta:
)

def validate(self, attrs):
# Validate that no additional fields are being passed (to sanitize the input)
# Validate that no additional fields are being passed (to sanitize the
# input)
if hasattr(self, "initial_data"):
invalid_keys = set(self.initial_data.keys()) - set(self.fields.keys())
if invalid_keys:
Expand Down
3 changes: 2 additions & 1 deletion documents/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@ def delete_expired_documents():
total, by_type_dict = documents_to_delete_qs.delete()
if total != 0:
logger.info(
f"Deleted {total} objects: {', '.join([f'{i[1]} {i[0]}' for i in by_type_dict.items()])}."
f"Deleted {total} objects:"
f" {', '.join([f'{i[1]} {i[0]}' for i in by_type_dict.items()])}."
)
else:
logger.info("Nothing to delete.")
15 changes: 9 additions & 6 deletions documents/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@


def get_attachment_file_path(instance, filename):
"""File will be uploaded to MEDIA_ROOT/ATTACHMENT_MEDIA_DIR/<document_id>/<filename>"""
"""File will be uploaded to
MEDIA_ROOT/ATTACHMENT_MEDIA_DIR/<document_id>/<filename>"""
return f"{settings.ATTACHMENT_MEDIA_DIR}/{instance.document.id}/{filename}"


Expand All @@ -24,8 +25,8 @@ def get_document_attachment_directory_path(instance):

def get_decrypted_file(file, file_name):
nonce = file[:16]
# Perform same nonce checks here as Pycryptodome, so we can raise a more user
# friendly error message
# Perform same nonce checks here as Pycryptodome, so we can raise a more
# user-friendly error message
if not isinstance(nonce, (bytes, bytearray, memoryview)) or len(nonce) != 16:
raise ValueError("Data is corrupted.")
tag = file[16:32]
Expand All @@ -45,9 +46,11 @@ def get_decrypted_file(file, file_name):
raise ValueError("AES Key incorrect or data is corrupted")


# TODO: Consider scanning files on download as well to improve chance of catching most recent threats to protect users
# if clamav virus databases didn't include the virus' profile at the time of upload
# Note this function is mocked in testing because there is no clamav connection during pipeline testing
# TODO: Consider scanning files on download as well to improve chance of catching most
# recent threats to protect users if clamav virus databases didn't include the virus'
# profile at the time of upload
# Note this function is mocked in testing because there is no clamav connection during
# pipeline testing
def virus_scan_attachment_file(file_data):
cd = pyclamd.ClamdNetworkSocket(host=settings.CLAMAV_HOST)
if cd.scan_stream(file_data) is not None:
Expand Down
3 changes: 2 additions & 1 deletion services/signals.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,8 @@ def create_service_api_key_user(
instance.user = user
instance.save()
logger.debug(
f"Created user {user} for ServiceAPIKey {instance} permission group for service: {instance.name}"
f"Created user {user} for ServiceAPIKey {instance} permission group for "
f"service: {instance.name}"
)
for perm in Service._meta.permissions:
assign_perm(f"{perm[0]}", user, instance.service)
Expand Down
3 changes: 2 additions & 1 deletion users/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@ def __str__(self):

def clean(self):
super().clean()
# Prevent personal details from being saved to ATV user model, ATV doesn't need to know anything else than uuid
# Prevent personal details from being saved to ATV user model, ATV doesn't need
# to know anything else than uuid
self.first_name = self.last_name = self.email = ""

class Meta:
Expand Down
3 changes: 2 additions & 1 deletion utils/commands.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@ def add_arguments(self, parser):
def setup_logging(self, verbosity):
"""
The values passed by the command are:
Verbosity level; 0=minimal output, 1=normal output, 2=verbose output, 3=very verbose output
Verbosity level; 0=minimal output, 1=normal output, 2=verbose output,
3=very verbose output
So output of 2 or 3 (i.e. > 1) will be verbose
Defaults to 1
Expand Down
12 changes: 10 additions & 2 deletions utils/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,13 +37,21 @@ def custom_exception_handler(exc, _context=None) -> Response:
# Validation errors require a special processing to use the same format
# as with the spec
if isinstance(exc, ValidationError):

def exception_value_to_str(value):
if isinstance(value, list):
return value[0]
return f"Required fields: {[key for key in value.keys()]}"

response = Response(
status=status.HTTP_400_BAD_REQUEST,
data=_get_error_wrapper(
list( # TODO: Refactor this to produce cleaner error message for nested errors
list(
# TODO: Refactor this to produce cleaner error message for nested
# errors
_get_error_detail(
"INVALID_FIELD",
f"{k}: {v[0] if isinstance(v, list) else f'Required fields: {[key for key in v.keys()]}'}",
f"{k}: {exception_value_to_str(v)}",
)
for k, v in exc.detail.items()
)
Expand Down

0 comments on commit 7c60de8

Please sign in to comment.