peepr_backend.md

Generally

I think all features has been implemented in rushed, thus there too many coding standard has been violated

Python code style

not follow pep8 standard for python code

• using tabs intention instead of spaces, spaces is recommended for replacing tabs long ago.
• no space after : and , in dict declaration
• import statements is not grouped and sorted
• too many code duplication, eg: generating unique id in models
• module level definition (class, module's method) is separated by only 1 empty line instead of 2 empty lines
• Some classes start with a lower case character like allVideos, deleteImage, some variable is in camel-case format instead of underline format like allowComments, dateCreated

Git problems

• Email credentials has been committed into git repos at peepr/email_info.py, you need to change that password immediately and change password of all account that are using same password as this. Event if this one is a private repo, we must keep credentials information out of the git repos. Because when github has problem, your secret info will be reveal to others.
• Some files is not program's code. All *.pyc is is not necessary to be appeared in git repos. htmlcov folder is not necessary for code to be run. This also may cause problem if somehow your server allow to view *.html files.
• There is not .gitignore in this repo, mostly because the programmer use a text-editor to code. We use .gitignore to prevent things like *.pem, *.pyc, etc to be committed to git history.

Django problem

• There is no admin in account and videos mean it's really hard to control the behavior of the system, also it's hard for QA testing too
• Re-implementing id field for models for preventing id from guessing. I think we can use normal auto-increment number id with help of hashids package to make it safe. This reduce the complexity of code while keeping id safety from hacker.
• All view have been implemented in only 1 file views, which is really hard for adding new features later, also it increases the changes of having code conflict when have multiple engineers work on same file.
• The JSON response is not well defined. Sometime I see JSONResponse({"d"}, status=400), sometime JSONResponse("Unknown Video", status=400)
• Since we use djangorestframework, we can use the defined statuses in that package, instead of using raw number response code
• Test is mostly check for the response status code, not the expected result and behavior of the system, we can add more assertion to make tests have more meaningful.
• videos/tests.py is commented out, this means we will need about 2 weeks to make it work again.
• Some part of code are not necessary, like this one

from email_info import *
EMAIL_USE_TLS = EMAIL_USE_TLS
EMAIL_HOST = EMAIL_HOST
EMAIL_HOST_USER = EMAIL_HOST_USER
EMAIL_HOST_PASSWORD = EMAIL_HOST_PASSWORD
EMAIL_PORT = EMAIL_PORT

The last 5 lines is not necessary

• Some code is not necessary because there's existing code for that purpose. For example:

def checkem(self,em):
	try:
		return PUser.objects.get(email=em)
	except PUser.DoesNotExist:
		return None

You can use

PUser.objects.filter(email=em).exists()

This was documented at Django Document. Or

class JSONResponse(HttpResponse):

You can use

from rest_framework.response import JsonResponse

And this is a joke payload = Payload(custom={'fuck1':'you1','cum1':'on1'}) , account/views.py line 157 =)).

• There is only 1 config for all environment, which means this code is not ready for production. We need at least turn off the DEBUG mode, separate debug settings like DEBUG_TOOLBAR_CONFIG, DEBUG_TOOLBAR_PANELS, etc... from production settings.
• There is no document for APIs, I think we should at least implement swagger document for rest_framework, this one is not difficult, it just take time.
• The code is not ready for cloud deployment, I think it's mostly suitable for dedicate server. The class deleteImage in videos/views.py is trying to delete a file from server.
• Many un-well defined response codes are used at ConnectSocialMedia, short code like a, d, x1, etc will increase the debugging time when error happen.
• Too complex logic is defined in views, we can divide it to other level like services, helpers, models to make the code easier to understand and maintenance.
• in videos/views.py line 366, the code executes a command inside a http request which may take more than 1 seconds, this may be a good target for DOS-attack, we can use celery to make this request lighter. At first when I see the command is generated from video.title, I think it's a security bug but it's turn out not like that. The video's title is actually a unique generated string from uuid4 and current timestamp which is mostly safe.

Summary

We should refactor this code before putting it to production. If we have some kind of human testing for iOS, Android or Web, and if 1 of them work well, we can start with improving tests first.

Written with StackEdit.