4
\$\begingroup\$

So I have just started to work with SQL and I have tested my script and is working very well when it comes to my database.

The database is used for shopping. What I mean by that is that I scrape a website that I love (Flowers etc) and then I collect each item that has been updated on the website and add it to my database if the product has not already been added in the database before (Reason why I have a checkIfExists function)

I also have different get Links, get Keywords functions and the reason I have

getLinks = Is to see if there has been added new page in the website that is not in my DB

getBlackList = In case they re-add same product and I don't want it to be too spammy with my logs then I can blacklist it inside the database.

getAllKeywords = Sometimes I want to filter for specific flowers where I have a function that checks whats the name of the product and then compare if those keywords matches from my database

The code I have written is working as I want and very proud of it and I do feel like I have done some steps abit too much. What I mean by that is that for every function I call cur = self.database.cursor() and cur.close() which maybe is not important or there is a newer way to handle this better. In my eyes I have a feeling I am missing something more.

I would appreciate all kind of help, even if I might have asked wrong question in Code review (its my first time and tried to read the description on how to write good) and as I mentioned before... I appreciate all kind of help! :)

#!/usr/bin/python3 # -*- coding: utf-8 -*- from datetime import datetime import psycopg2 class DataBaseAPI: def __init__(self): self.database = psycopg2.connect(host="test", database="test", user="test", password="test" ) def checkIfExists(self, store, link): try: cur = self.database.cursor() sql = f"SELECT DISTINCT link FROM public.store_items WHERE store='{store.title()}' AND visible='yes' AND link='{link}';" cur.execute(sql) cur.close() except (Exception, psycopg2.DatabaseError) as error: print(error) return False if cur.rowcount: return True else: return False def addProduct(self, product): if self.checkIfExists(store=product["store"], link=product["url"]) is False: link = product["url"], store = product["store"], name = product["name"], image = product["image"] visible = "yes" cur = self.database.cursor() sql = f"INSERT INTO public.store_items (store, name, link, image, visible, added_date) VALUES ('{store}', '{name}', '{link}', '{image}', '{visible}', '{datetime.utcnow().strftime('%Y-%m-%d %H:%M:%S')}');" cur.execute(sql) # # Commit the changes to the database self.database.commit() # Close communication with the PostgreSQL database cur.close() return True else: return False def getAllLinks(self, store): cur = self.database.cursor() sql = f"SELECT DISTINCT link FROM public.store_items WHERE store='{store.title()}' AND visible='yes';" cur.execute(sql) cur.close() return [link[0] for link in cur.fetchall()] def getBlackList(self, store): cur = self.database.cursor() sql = f"SELECT DISTINCT link FROM public.store_items WHERE store='{store.title()}' AND visible='no';" cur.execute(sql) cur.close() return [link[0] for link in cur.fetchall()] def getAllKeywords(self, filtered_or_unfiltered): cur = self.database.cursor() sql = f"SELECT DISTINCT keyword FROM public.keywords WHERE filter_type='{filtered_or_unfiltered}';" cur.execute(sql) cur.close() return [keyword[0] for keyword in cur.fetchall()] 

I think thats pretty much it. Just a simple Add and Get database :)

\$\endgroup\$
0

    1 Answer 1

    3
    \$\begingroup\$

    Before anything, read on SQL parameterization and avoid concatenating or interpolating variables directly in SQL statements. Otherwise you open your application to SQL injection and inefficiencies. The module, psycopg2, has much support for parameterizations as their docs indicate.

    Secondly, save cur as another self. attribute and handle cursor and connection closings in a __del__ method. Also public. is the default schema in PostgreSQL and hence can be omitted from table names.

    from datetime import datetime import psycopg2 class DataBaseAPI: def __init__(self): self.database = psycopg2.connect(host="test", database="test", user="test", password="test") self.cur = self.database.cursor() def checkIfExists(self, store, link): try: # PREPARED STATEMENT WITH %s PLACEHOLDERS sql = """SELECT DISTINCT link FROM store_items WHERE store=%s AND visible=%s AND link=%s; """ # EXECUTE WITH BINDED PARAMS cur.execute(sql, (store.title(), 'yes', link)) except (Exception, psycopg2.DatabaseError) as error: print(error) return False if cur.rowcount: return True else: return False def addProduct(self, product): if self.checkIfExists(store=product["store"], link=product["url"]) is False: link = product["url"], store = product["store"], name = product["name"], image = product["image"] visible = "yes" # PREPARED STATEMENT WITH %s PLACEHOLDERS sql = """INSERT INTO store_items (store, name, link, image, visible, added_date) VALUES (%s, %s, %s, %s, %s, %s);""" # EXECUTE WITH BINDED PARAMS cur.execute(sql, (store, name, link, image, visible, datetime.utcnow().strftime('%Y-%m-%d %H:%M:%S'))) # Commit the changes to the database self.database.commit() return True else: return False def getAllLinks(self, store): # PREPARED STATEMENT WITH %s PLACEHOLDERS sql = """SELECT DISTINCT link FROM store_items WHERE store=%s AND visible=%s; """ # EXECUTE WITH BINDED PARAMS cur.execute(sql, (store.title(), 'yes')) return [link[0] for link in cur.fetchall()] def getBlackList(self, store): # PREPARED STATEMENT WITH %s PLACEHOLDERS sql = """SELECT DISTINCT link FROM store_items WHERE store=%s AND visible=%s; """ # EXECUTE WITH BINDED PARAMS cur.execute(sql, (store.title(), 'no')) return [link[0] for link in cur.fetchall()] def getAllKeywords(self, filtered_or_unfiltered): # PREPARED STATEMENT WITH %s PLACEHOLDERS sql = """SELECT DISTINCT keyword FROM keywords WHERE filter_type=%s; """ # EXECUTE WITH BINDED PARAMS # (INNER PARENS + COMMA IS NOT A TYPO BUT A ONE-ITEM TUPLE) cur.execute(sql, (filtered_or_unfiltered,)) return [keyword[0] for keyword in cur.fetchall()] def __del__(self): # CLOSE OBJECTS self.cur.close() self.database.close() 
    \$\endgroup\$
    0

      Start asking to get answers

      Find the answer to your question by asking.

      Ask question

      Explore related questions

      See similar questions with these tags.