I have a function whose responsibility is to send a push notification to a user. I call this function from a view in flask framework.
I have read multiple articles around refactoring and good designs but I want to know from those who have been doing this for a while what I can improve.
Here is the function.
def send_push(api_key, cid, username,request_signature):
## ---> DB CALLS
master_db = get_db('master_db')
cname = master_db.companies.find_one({'company_id': cid})['company_name']
company_db = get_db(cname)
application = company_db.applications.find_one({'app_config.api_key':api_key})
if not application:
#error_log_writer(cid=cid, error='API Key is invalid')
raise InvalidApiKey('The API Key is incorrect.')
appname = application['app_name']
# application = company_db.applications.find_one({'app_name': appname})
username_appname = username + '_' + appname
redis_db = get_redis()
## DB CALLS
if redis_db.hgetall(username_appname):
# error_log_writer(cid=cid, error='The PUSH request is already in process', appname=appname)
raise SomeCustomException(' The request is already in process.')
auth_id = generate_random_alphanumeric(10)
from pyfcm import FCMNotification
push_service = FCMNotification(
api_key="xxxxx"
)
## DB CALLS
user = company_db.users.find_one({"_id": username})
if not user:
raise UserNotFound
## DB CALLS
registration_id = company_db.users.find_one({"_id": username})['device_token']
data_message = {
"Nick" : "Mario",
"body" : "great match!",
"app_name": appname,
"cid": cid,
"username":username,
"auth_id" : auth_id,
"api_key": api_key,
"request_signature":request_signature
}
message_body = "authenticate app?"
username_appname = username + '_' + appname
username_appname_response = username_appname + 'response'
## DB CALLS
redis_db.setex(name=username_appname_response, value='waiting', time=180)
redis_data = {'auth_id': auth_id, 'ts': datetime.datetime.now().strftime('%s')}
redis_db.hmset(username_appname, redis_data)
redis_db.expire(username_appname, 180)
result = push_service.notify_single_device(registration_id=registration_id, data_message=data_message)
I think this has many code smells. Ideally, I would like to refactor it so that I get
- Easy unit testing
- Avoiding mocks for unit testing where necessary
- Minimal side effects
- Readable code.
Current issues that I think are there:
- Lots of database calls scattered around the core business logic.
- Unit testing is difficult as lots of db calls requires mocking
them. This tightly couples the test to my implementation rather than behaviour of the function. - Lots of indirection to logic is introduced because of scattered calls to mongo db & redis.
Possible solutions:
- pass db objects to function rather than creating them inside the function that handles business logic(but this will just defer the object creation to a higher level so db calls will have to be mocked anyways)
How would you refactor this function considering best practices?
Is it a general practice while developing CRUD app to have multiple mocks on average for every function?