-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Used cosmos as db #1
base: master
Are you sure you want to change the base?
Changes from 4 commits
6fa55e4
d8a8cb2
c35e265
3796753
0863ec1
1158785
d5e183b
8023c90
75fc439
476098e
60604a0
d2a11d5
4fe77ea
a8b1407
6ccc15f
f27e7dc
fb646a2
9834e34
166b2d6
144a8cc
5291854
fbf02c6
7d1f785
bd86c40
efe01c5
b422d27
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,15 +1,25 @@ | ||
from flask import Flask | ||
from flask_bcrypt import Bcrypt | ||
from flask_sqlalchemy import SQLAlchemy | ||
from azure.cosmos import errors, CosmosClient | ||
|
||
|
||
import os | ||
|
||
APP_DIR = os.path.abspath(os.path.dirname(__file__)) | ||
STATIC_FOLDER = os.path.join(APP_DIR, '../static/build/static') # Where your webpack build output folder is | ||
TEMPLATE_FOLDER = os.path.join(APP_DIR, '../static/build') # Where your index.html file is located | ||
STATIC_FOLDER = os.path.join( | ||
APP_DIR, "../static/build/static" | ||
) # Where your webpack build output folder is | ||
TEMPLATE_FOLDER = os.path.join( | ||
APP_DIR, "../static/build" | ||
) # Where your index.html file is located | ||
|
||
app = Flask(__name__, static_folder=STATIC_FOLDER, template_folder=TEMPLATE_FOLDER) | ||
app.config.from_object('app.config.ProductionConfig') | ||
app.config.from_object("app.config.ProductionConfig") | ||
|
||
key = app.config["SECRET_KEY"] | ||
uri = app.config["COSMOS_DB_URI"] | ||
|
||
client = CosmosClient(uri, credential=key) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not crazy about having the cosmos client as a global variable for the app. I would like all the "cosmos" stuff to be encapsulated so the app is only coupled to that encapsulation versus directly to cosmos. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done, I think. Added a db.py in /utils and kept cosmos things there. Imported the db in models.py to use. I thought this way, if in the future, we want to use other db, we can keep all db things there and just import the appropriate dbs to use in models.py or other scripts. |
||
bcrypt = Bcrypt(app) | ||
|
||
db = SQLAlchemy(app) | ||
bcrypt = Bcrypt(app) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,34 +1,55 @@ | ||
from datetime import datetime, timedelta | ||
from sqlalchemy.exc import IntegrityError | ||
from app import db, bcrypt | ||
from app import client, bcrypt | ||
from random import randint | ||
import json, time | ||
from azure.cosmos import CosmosClient, PartitionKey | ||
|
||
|
||
class User(db.Model): | ||
id = db.Column(db.Integer(), primary_key=True) | ||
email = db.Column(db.String(255), unique=True) | ||
password = db.Column(db.String(255)) | ||
first_name = db.Column(db.String(255)) | ||
last_name = db.Column(db.String(255)) | ||
|
||
# for task id | ||
_MIN = 1 | ||
_MAX = 1000000000 | ||
|
||
user_container_name = "users" | ||
task_container_name = "tasks" | ||
db_name = "team_standup" | ||
|
||
|
||
# get container by walking down resource hierarchy | ||
# client -> db -> container | ||
db = client.get_database_client(db_name) | ||
user_container = db.get_container_client(user_container_name) | ||
task_container = db.get_container_client(task_container_name) | ||
|
||
|
||
class User: | ||
def __init__(self, first_name, last_name, email, password): | ||
self.id = email | ||
self.first_name = first_name | ||
self.last_name = last_name | ||
self.email = email | ||
self.password = User.hashed_password(password) | ||
|
||
@staticmethod | ||
def create_user(payload): | ||
user = User( | ||
email=payload["email"], | ||
password=payload["password"], | ||
first_name=payload["first_name"], | ||
last_name=payload["last_name"], | ||
) | ||
self.password = password | ||
|
||
@staticmethod | ||
def create_user(input): | ||
try: | ||
db.session.add(user) | ||
db.session.commit() | ||
user = User( | ||
first_name=input["first_name"], | ||
last_name=input["last_name"], | ||
email=input["email"], | ||
password=input["password"], | ||
) | ||
|
||
user_container.upsert_item( | ||
{ | ||
"id": user.id, | ||
"first_name": user.first_name, | ||
"last_name": user.last_name, | ||
"email": user.email, | ||
"password": user.password, | ||
} | ||
) | ||
|
||
return True | ||
except IntegrityError: | ||
return False | ||
|
@@ -39,102 +60,144 @@ def hashed_password(password): | |
|
||
@staticmethod | ||
def get_user_by_id(user_id): | ||
user = User.query.filter_by(id=user_id).first() | ||
query = "SELECT * FROM users u where u.id=" + user_id | ||
users = user_container.query_items( | ||
query=query, enable_cross_partition_query=True | ||
) | ||
user = list(users)[0] | ||
|
||
return user | ||
|
||
@staticmethod | ||
def get_user_by_email(user_email): | ||
query = "SELECT * FROM users u where u.email='" + user_email + "'" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Cosmos SQL is subject to SQL injection. Read this: https://en.wikipedia.org/wiki/SQL_injection and this https://michaelhowardsecure.blog/2019/03/05/cosmosdb-and-sql-injection/ There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done. |
||
users = user_container.query_items( | ||
query=query, enable_cross_partition_query=True | ||
) | ||
user = list(users)[0] | ||
|
||
return user | ||
|
||
@staticmethod | ||
def get_user_with_email_and_password(email, password): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should be using OAuth to manage users and permissions. Having a method with email and password basically means that your app controls user access, permissions, etc and storing of credentials in the database. Typically you avoid with this access tokens and OAuth. It's a bigger more complicated implementation that you should learn, but we can discuss later. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, I'd love to have a discussion on this. Right now, one problem I'm seeing is that when a user is logged in, he/she can edit other people's tasks, instead of just his/her own. So basically, a logged in user has access to the whole database, instead of just part of it that stores his/her own info. I don't know if this can be solved by OAuth, but the way this works now is not ideal. Also, can we discuss the create-react-app too, like how to make it integrate with Flask? I didn't change the structure of the code at all. I think the original app is based on this boilerplate that combines Flask and React, which I guess is not ideally structured. |
||
user = User.query.filter_by(email=email).first() | ||
if user and bcrypt.check_password_hash(user.password, password): | ||
return user | ||
|
||
query = ( | ||
"SELECT * FROM users u where u.email='" | ||
+ "'" | ||
+ " and u.password='" | ||
+ password | ||
+ "'" | ||
) | ||
users = user_container.query_items( | ||
query=query, enable_cross_partition_query=True | ||
) | ||
|
||
if users: | ||
user = list(users) | ||
return user[0] | ||
else: | ||
return None | ||
|
||
|
||
class Task(db.Model): | ||
class Task: | ||
class STATUS: | ||
COMPLETED = 'COMPLETED' | ||
IN_PROGRESS = 'IN_PROGRESS' | ||
|
||
id = db.Column(db.Integer(), primary_key=True) | ||
date = db.Column(db.DateTime()) | ||
task = db.Column(db.String(255)) | ||
user_id = db.Column(db.String(255)) | ||
status = db.Column(db.String(255)) | ||
|
||
COMPLETED = "COMPLETED" | ||
IN_PROGRESS = "IN_PROGRESS" | ||
|
||
def __init__(self, task, user_id, status): | ||
self.date = datetime.utcnow().date() | ||
self.id = str(randint(_MIN, _MAX)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would think Cosmos has a way to generate random identifiers for documents - what happens if you just don't assign an id? I would think it would auto generate one. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also, randint doesn't guarantee unique across all items, so please switch over to using Guid for ids. If you don't provide an "id" field, then Cosmos should auto create an id field and insert a guid. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You have to add an id to each item you insert into the container according to the docs here. I'll try to use Guid. |
||
self.date = str(datetime.utcnow().date()) | ||
self.task = task | ||
self.user_id = user_id | ||
self.status = status | ||
|
||
@staticmethod | ||
def add_task(task, user_id, status): | ||
task = Task( | ||
task=task, | ||
user_id=user_id, | ||
status=status | ||
) | ||
|
||
db.session.add(task) | ||
try: | ||
db.session.commit() | ||
task = Task(task=task, user_id=user_id, status=status) | ||
task_container.upsert_item( | ||
{ | ||
"id": task.id, | ||
"date": task.date, | ||
"task": task.task, | ||
"user_id": task.user_id, #user_id is email here | ||
"status": task.status, | ||
} | ||
) | ||
|
||
return True, task.id | ||
except IntegrityError: | ||
return False, None | ||
|
||
@staticmethod | ||
def get_latest_tasks(): | ||
user_to_task = {} | ||
|
||
result = db.engine.execute( | ||
"""SELECT t.id, t.date, t.task, t.user_id, t.status, u.first_name, u.last_name | ||
from task t | ||
INNER JOIN "user" u | ||
on t.user_id = u.email""") # join with users table | ||
|
||
for t in result: | ||
if t.user_id in user_to_task: | ||
user_to_task.get(t.user_id).append(dict(t)) | ||
else: | ||
user_to_task[t.user_id] = [dict(t)] | ||
query = "SELECT * FROM users" | ||
users = user_container.query_items( | ||
query=query, enable_cross_partition_query=True | ||
) | ||
|
||
cnt = 0 | ||
for u in users: | ||
cnt+=1 | ||
task_query = "SELECT * FROM tasks t where t.user_id='" + u["id"] + "'" | ||
tasks = task_container.query_items( | ||
query=task_query, enable_cross_partition_query=True | ||
) | ||
for t in tasks: | ||
t["first_name"] = u["first_name"] | ||
t["last_name"] = u["last_name"] | ||
if t["user_id"] in user_to_task: | ||
user_to_task.get(t["user_id"]).append(t) | ||
else: | ||
user_to_task[t["user_id"]] = [t] | ||
|
||
return user_to_task | ||
|
||
@staticmethod | ||
def get_tasks_for_user(user_id): | ||
return Task.query.filter_by(user_id=user_id) | ||
query = "SELECT * FROM tasks t where t.user_id='" + user_id + "'" | ||
tasks = task_container.query_items( | ||
query=query, enable_cross_partition_query=True | ||
) | ||
return tasks | ||
|
||
@staticmethod | ||
def delete_task(task_id): | ||
task_to_delete = Task.query.filter_by(id=task_id).first() | ||
db.session.delete(task_to_delete) | ||
|
||
try: | ||
db.session.commit() | ||
query = "SELECT * FROM tasks t where t.id='" + task_id + "'" | ||
tasks = task_container.query_items( | ||
query=query, enable_cross_partition_query=True | ||
) | ||
task = list(tasks)[0] | ||
task_container.delete_item(task, partition_key=task_id) | ||
|
||
return True | ||
except IntegrityError: | ||
return False | ||
|
||
@staticmethod | ||
def edit_task(task_id, task, status): | ||
task_to_edit = Task.query.filter_by(id=task_id).first() | ||
task_to_edit.task = task | ||
task_to_edit.status = status | ||
|
||
try: | ||
db.session.commit() | ||
task_to_edit = task_container.read_item(task_id, partition_key=task_id) | ||
task_to_edit["task"] = task | ||
task_to_edit["status"] = status | ||
|
||
task_container.upsert_item(task_to_edit) | ||
|
||
return True | ||
except IntegrityError: | ||
return False | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's do some research on how we want to handle error messages from the model. Typically the UI doesn't want to show the error details, but something to consider, versus just returning false. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh the UI wouldn't just show 'False.' It'll have an error message that's appropriate for the specific situation. Please take a look at the routes.py script. |
||
|
||
@property | ||
def serialize(self): | ||
"""Return object data in easily serializeable format""" | ||
return { | ||
'id' : self.id, | ||
'date' : self.date.strftime("%Y-%m-%d"), | ||
'task' : self.task, | ||
'user_id' : self.user_id, | ||
'status' : self.status, | ||
} | ||
"""Return object data in easily serializeable format""" | ||
return { | ||
"id": self.id, | ||
"date": self.date.strftime("%Y-%m-%d"), | ||
"task": self.task, | ||
"user_id": self.user_id, | ||
"status": self.status, | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like this would be a different sample. So let's chat before you attempt to merge back into this repo. We'll likely create a new one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay. I'll keep it as is for now and change after we chat.