Untitled
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