AllocationAttribute clean method exception handling

coldfront/core/allocation/models.py

@@ -1,10 +1,8 @@
 import datetime
-import importlib
 import logging
 from ast import literal_eval
 from enum import Enum
 
-from django.conf import settings
 from django.contrib.auth.models import User
 from django.core.exceptions import ValidationError
 from django.db import models
@@ -16,7 +14,6 @@
 from coldfront.core.project.models import Project, ProjectPermission
 from coldfront.core.resource.models import Resource
 from coldfront.core.utils.common import import_from_settings
-import coldfront.core.attribute_expansion as attribute_expansion
 
 logger = logging.getLogger(__name__)
 
@@ -35,8 +32,8 @@ class AllocationPermission(Enum):
  MANAGER = 'MANAGER'
 
 class AllocationStatusChoice(TimeStampedModel):
- """ A project status choice indicates the status of the project. Examples include Active, Archived, and New. 
- 
+ """ A project status choice indicates the status of the project. Examples include Active, Archived, and New.
+
  Attributes:
  name (str): name of project status choice
  """
@@ -57,8 +54,8 @@ def natural_key(self):
  return (self.name,)
 
 class Allocation(TimeStampedModel):
- """ An allocation provides users access to a resource. 
- 
+ """ An allocation provides users access to a resource.
+
  Attributes:
  project (Project): links the project the allocation falls under
  resources (Resource): links resources that this allocation allocates
@@ -134,7 +131,7 @@ def save(self, *args, **kwargs):
 
  @property
  def expires_in(self):
- """ 
+ """
  Returns:
  int: the number of days until the allocation expires
  """
@@ -143,7 +140,7 @@ def expires_in(self):
 
  @property
  def get_information(self):
- """ 
+ """
  Returns:
  str: the allocation's attribute type, usage out of total value, and usage out of total value as a percentage
  """
@@ -344,7 +341,7 @@ def __str__(self):
 
 class AllocationAdminNote(TimeStampedModel):
  """ An allocation admin note is a note that an admin makes on an allocation.
- 
+
  Attributes:
  allocation (Allocation): links the allocation to the note
  author (User): represents the User class of the admin who authored the note
@@ -360,7 +357,7 @@ def __str__(self):
 
 class AllocationUserNote(TimeStampedModel):
  """ An allocation user note is a note that an user makes on an allocation.
- 
+
  Attributes:
  allocation (Allocation): links the allocation to the note
  author (User): represents the User class of the user who authored the note
@@ -377,8 +374,8 @@ def __str__(self):
  return self.note
 
 class AttributeType(TimeStampedModel):
- """ An attribute type indicates the data type of the attribute. Examples include Date, Float, Int, Text, and Yes/No. 
- 
+ """ An attribute type indicates the data type of the attribute. Examples include Date, Float, Int, Text, and Yes/No.
+
  Attributes:
  name (str): name of attribute data type
  """
@@ -392,8 +389,8 @@ class Meta:
  ordering = ['name', ]
 
 class AllocationAttributeType(TimeStampedModel):
- """ An allocation attribute type indicates the type of the attribute. Examples include Cloud Account Name and Core Usage (Hours). 
- 
+ """ An allocation attribute type indicates the type of the attribute. Examples include Cloud Account Name and Core Usage (Hours).
+
  Attributes:
  attribute_type (AttributeType): indicates the data type of the attribute
  name (str): name of allocation attribute type
@@ -420,8 +417,8 @@ class Meta:
  ordering = ['name', ]
 
 class AllocationAttribute(TimeStampedModel):
- """ An allocation attribute class links an allocation attribute type and an allocation. 
- 
+ """ An allocation attribute class links an allocation attribute type and an allocation.
+
  Attributes:
  allocation_attribute_type (AllocationAttributeType): attribute type to link
  allocation (Allocation): allocation to link
@@ -450,22 +447,30 @@ def clean(self):
  self.allocation_attribute_type))
 
  expected_value_type = self.allocation_attribute_type.attribute_type.name.strip()
-
- if expected_value_type == "Int" and not isinstance(literal_eval(self.value), int):
- raise ValidationError(
- 'Invalid Value "%s" for "%s". Value must be an integer.' % (self.value, self.allocation_attribute_type.name))
- elif expected_value_type == "Float" and not (isinstance(literal_eval(self.value), float) or isinstance(literal_eval(self.value), int)):
- raise ValidationError(
- 'Invalid Value "%s" for "%s". Value must be a float.' % (self.value, self.allocation_attribute_type.name))
+ error = None
+ if expected_value_type in ['Float', 'Int']:
+ try:
+ literal_val = literal_eval(self.value)
+ except SyntaxError as exc:
+ error = 'Value must be entirely numeric. Please remove any non-numeric characters.'
+ raise ValidationError(
+ f'Invalid Value "{self.value}" for "{self.allocation_attribute_type.name}". {error}'
+ ) from exc
+ if expected_value_type == 'Float' and not isinstance(literal_val, (float,int)):
+ error = 'Value must be a float.'
+ elif expected_value_type == 'Int' and not isinstance(literal_val, int):
+ error = 'Value must be an integer.'
  elif expected_value_type == "Yes/No" and self.value not in ["Yes", "No"]:
- raise ValidationError(
- 'Invalid Value "%s" for "%s". Allowed inputs are "Yes" or "No".' % (self.value, self.allocation_attribute_type.name))
+ error = 'Allowed inputs are "Yes" or "No".'
  elif expected_value_type == "Date":
  try:
  datetime.datetime.strptime(self.value.strip(), "%Y-%m-%d")
  except ValueError:
- raise ValidationError(
- 'Invalid Value "%s" for "%s". Date must be in format YYYY-MM-DD' % (self.value, self.allocation_attribute_type.name))
+ error = 'Date must be in format YYYY-MM-DD'
+ if error:
+ raise ValidationError(
+ f'Invalid Value "{self.value}" for "{self.allocation_attribute_type.name}". {error}'
+ )
 
  def __str__(self):
  return '%s' % (self.allocation_attribute_type.name)
@@ -480,8 +485,8 @@ def typed_value(self):
  atype_name = self.allocation_attribute_type.attribute_type.name
  return attribute_expansion.convert_type(
  value=raw_value, type_name=atype_name)
- 
- 
+
+
  def expanded_value(self, extra_allocations=[], typed=True):
  """
  Params:
@@ -491,7 +496,7 @@ def expanded_value(self, extra_allocations=[], typed=True):
  Returns:
  int, float, str: the value of the attribute after attribute expansion
 
- For attributes with attribute type of 'Attribute Expanded Text' we look for an attribute with same name suffixed with '_attriblist' (this should be ResourceAttribute of the Resource associated with the attribute). If the attriblist attribute is found, we use it to generate a dictionary to use to expand the attribute value, and the expanded value is returned. 
+ For attributes with attribute type of 'Attribute Expanded Text' we look for an attribute with same name suffixed with '_attriblist' (this should be ResourceAttribute of the Resource associated with the attribute). If the attriblist attribute is found, we use it to generate a dictionary to use to expand the attribute value, and the expanded value is returned.
 
  If the expansion fails, or if no attriblist attribute is found, or if the attribute type is not 'Attribute Expanded Text', we just return the raw value.
  """
@@ -526,10 +531,10 @@ def expanded_value(self, extra_allocations=[], typed=True):
  resources = resources,
  allocations = allocs)
  return expanded
- 
+
 class AllocationAttributeUsage(TimeStampedModel):
- """ Allocation attribute usage indicates the usage of an allocation attribute. 
- 
+ """ Allocation attribute usage indicates the usage of an allocation attribute.
+
  Attributes:
  allocation_attribute (AllocationAttribute): links the usage to its allocation attribute
  value (float): usage value of the allocation attribute
@@ -545,7 +550,7 @@ def __str__(self):
 
 class AllocationUserStatusChoice(TimeStampedModel):
  """ An allocation user status choice indicates the status of an allocation user. Examples include Active, Error, and Removed.
- 
+
  Attributes:
  name (str): name of the allocation user status choice
  """
@@ -567,7 +572,7 @@ def natural_key(self):
 
 class AllocationUser(TimeStampedModel):
  """ An allocation user represents a user on the allocation.
- 
+
  Attributes:
  allocation (Allocation): links user to its allocation
  user (User): represents the User object of the allocation user
@@ -588,9 +593,9 @@ class Meta:
  unique_together = ('user', 'allocation')
 
 class AllocationAccount(TimeStampedModel):
- """ An allocation account 
+ """ An allocation account
  #come back to
- 
+
  Attributes:
  user (User): represents the User object of the project user
  name (str):
@@ -607,7 +612,7 @@ class Meta:
 
 class AllocationChangeStatusChoice(TimeStampedModel):
  """ An allocation change status choice represents statuses displayed when a user changes their allocation status (for allocations that have their is_changeable attribute set to True). Examples include Expired and Payment Pending.
- 
+
  Attributes:
  name (str): status name
  """
@@ -622,7 +627,7 @@ class Meta:
 
 class AllocationChangeRequest(TimeStampedModel):
  """ An allocation change request represents a request from a PI or manager to change their allocation.
- 
+
  Attributes:
  allocation (Allocation): represents the allocation to change
  status (AllocationStatusChoice): represents the allocation status of the changed allocation
@@ -656,11 +661,11 @@ def __str__(self):
 
 class AllocationAttributeChangeRequest(TimeStampedModel):
  """ An allocation attribute change request represents a request from a PI/ manager to change their allocation attribute.
- 
+
  Attributes:
  allocation_change_request (AllocationChangeRequest): links the change request from which this attribute change is derived
  allocation_attribute (AllocationAttribute): represents the allocation_attribute to change
- new_value (str): new value of allocation attribute 
+ new_value (str): new value of allocation attribute
  """
 
  allocation_change_request = models.ForeignKey(AllocationChangeRequest, on_delete=models.CASCADE)

coldfront/core/allocation/test_models.py

@@ -1,16 +1,22 @@
 """Unit tests for the allocation models"""
 
 from django.test import TestCase
+from django.core.exceptions import ValidationError
 
-from coldfront.core.test_helpers.factories import AllocationFactory, ResourceFactory
+from coldfront.core.test_helpers.factories import (
+ AllocationFactory,
+ ResourceFactory,
+ AllocationAttributeTypeFactory,
+ AllocationAttributeFactory,
+)
 
 
 class AllocationModelTests(TestCase):
  """tests for Allocation model"""
 
  @classmethod
  def setUpTestData(cls):
- """Set up project to test model properties and methods"""
+ """Set up allocation to test model properties and methods"""
  cls.allocation = AllocationFactory()
  cls.allocation.resources.add(ResourceFactory(name='holylfs07/tier1'))
 
@@ -21,3 +27,42 @@ def test_allocation_str(self):
  self.allocation.project.pi
  )
  self.assertEqual(str(self.allocation), allocation_str)
+
+
+class AllocationAttributeModelTests(TestCase):
+ """Tests for allocationattribute models"""
+
+ @classmethod
+ def setUpTestData(cls):
+ """Set up allocationattribute to test model properties and methods"""
+ cls.allocation = AllocationFactory()
+ cls.allocation.resources.add(ResourceFactory(name='holylfs07/tier1'))
+ cls.allocationattribute = AllocationAttributeFactory(
+ allocation=cls.allocation,
+ value = 100,
+ allocation_attribute_type=AllocationAttributeTypeFactory(
+ name='Storage Quota (TB)'
+ ),
+ )
+
+ def test_allocationattribute_clean_no_error(self):
+ """cleaning numeric value for numeric AllocationAttributeType gives no error
+ """
+ self.allocationattribute.value = "1000"
+ self.allocationattribute.clean()
+
+ def test_allocationattribute_clean_nonnumeric_error(self):
+ """cleaning non-numeric value for numeric AllocationAttributeType gives useful error message
+ """
+ self.allocationattribute.value = "1000TB"
+ error = 'Value must be entirely numeric. Please remove any non-numeric characters.'
+ with self.assertRaisesMessage(ValidationError, error):
+ self.allocationattribute.clean()
+
+ def test_allocationattribute_clean_nonnumeric_error2(self):
+ """cleaning non-numeric value for numeric AllocationAttributeType gives useful error message
+ """
+ self.allocationattribute.value = "150%"
+ error = 'Value must be entirely numeric. Please remove any non-numeric characters.'
+ with self.assertRaisesMessage(ValidationError, error):
+ self.allocationattribute.clean()