Untitled

 avatar
unknown
python
a year ago
6.3 kB
4
Indexable
"""
Core:
This code snippet has few problems:
Fat model: Model is ~200 lines of code.
Separation of concern: Model has schema, business logic, cache operations, database operations
Cyclomatic complexity: Function create_or_update_experiment is fat and has multiple if-else statements
Low Testability: Almost all the functions has side-effects


Goal is to refactor the code, making it modular such that it solves all the above 4 problems.

Extra:
Write the Unit Tests (only signature and not implementation) for the completed code.
How will you design an API to create an experiment? Focus is on the request data validation, authorisation, response format, request orchestration (different service/backed layer it will go to).
"""


class DBConStore:
   redis = RedisUtils()
   redis_shard_utils = RedisShardingUtils(shard_range=EXP_SHARD_RANGE)
   cache_ops_utils = CustomRedisCache()




class Experiment(models.Model):
   experiment_id = models.CharField(primary_key=True, max_length=60)
   title = models.CharField(null=False, max_length=100)
   description = models.CharField(max_length=100, default=None)
   props = JSONField()
   type = models.CharField(max_length=20)
   is_disabled = models.IntegerField(default=0)
   create_time = models.DateTimeField(auto_now_add=True, editable=True)
   start_time = models.DateTimeField(auto_now_add=True)
   end_time = models.DateTimeField(default=None)
   is_existing_alc_disabled = models.IntegerField(default=0)
   app_name = EnumField(choices=['pocket_fm', 'pocket_fm_lite'], default="pocket_fm")


   class Meta:
       managed = False
       db_table = "experiments"


   DEFAULT_CACHE_TIME = 3600


   def __init__(self, *args, **kwargs):
       super().__init__(*args, **kwargs)
       self.output_fields = get_model_fields(Experiment)
  
   @staticmethod
   def get_shard_idx_for_details():
       """
       Get shard for enabling local store for sharded experiment detail keys
       """
       cached_data = get_request_info().get('cached_data')
       store_key = 'exp_shard'
       if cached_data.get(store_key):
           result = cached_data[store_key]
           return result


       return set_shard_num_to_local_store(shard_range=EXP_SHARD_RANGE, store_key=store_key)


   def create_or_update_experiment(self, payload):
       """
       Create or Update an experiment
       :param payload: Data to insert/update
       :type payload: dict
        - Experiment_ID : (optional) Experiment id in case of update
        - Title : (required) Title of experiment
        - Props : (optional) Properties of experiment
        - is_disabled : (optional, default 0) If the experiment should be disabled or not
        - end_time : (optional) End time of experiment
        - update_props : (optional, default 1) If the props should be updated or not
       :return: Created/Updated Data, If created or updated
       :rtype: dict, int
       """
       """ Break this class into three different layers: 
           1. Controller
           2. Domain logic 
           3. Repository classes 
           In repo class also make 2 different classes : 
               1. For Entity 
               2. Repository 
               The repository class would be using entity and saving it into the DB """
        """Rename update to append and use either true or false as values as it would be easier to read"""
       update = int(payload.get("update_props", 1)) # Move this to get from the already extracted payload in the next line. Perform the check for empty payload before getting this 
       payload = filter_dict_keys(payload, self.output_fields)

        """There are two checks that return a bad request: 
            1. if payload is empty 
            2. if both experiment id and titale is empty
            We can combine both of this into one separate method called validatePayload and return the reason from it"""
       if not len(payload):
           return {"status_code": 400}, None


       title = payload.get("title") 
       experiment_id = payload.get("experiment_id")
        
        
       if title is None and experiment_id is None:
           return {"status_code": 400}, None
       elif experiment_id is None:
           """Put the below lines in a separate method as create new Experiment """
           key = title + str(time.time())
           experiment_id = get_hash_key(key)
           payload["experiment_id"] = experiment_id
           props = {}
       else:
           """Use conditional instead of try-catch to populate props. This is not a valid use-case of using try-catch"""
           """Put the entire below logic in a method called as modify existing experiment """
           try:
               props = Experiment.objects.get(experiment_id=experiment_id).props
           except:
               props = {}
               if title is None:
                   return {"status_code": 400}, None
           if isinstance(props, str):
               props = json.loads(props)

        """Move the below logic to a separate method called updateProps"""
       if "props" in payload:
           if update == 1:
               props.update(payload["props"])
           else:
               props = payload["props"]
       payload["props"] = props

        """ Add logic to validate that if experiment id is coming in the call is bogus. If so throw and error rather than creating it""""
       experiment_obj, is_created = Experiment.objects.update_or_create(
           experiment_id=experiment_id, defaults=payload
       )


       if not is_created:
           # Invalidate all shards
           """Make a sepratae private method to invalidate all shards a it is getting used at multiple places"""
           key, _ = RedisKeySchema.get_experiment_detail_key(experiment_id)
           DBConStore.redis_shard_utils.delete_shard_keys(key)
           LocalStore.delete_cache_key(key=key)

        """Mske this as a separate method called serialize ouput. Return the final output from it"""
       experiment_json = model_to_dict(experiment_obj)
       experiment_json["create_time"] = experiment_obj.create_time
       if type(experiment_json["props"]) is str:
           experiment_json["props"] = json.loads(experiment_json["props"])


       return experiment_json, is_created


  
Editor is loading...
Leave a Comment