Code quality assurance completed

hub/persistence/__init__.py

@@ -2,11 +2,3 @@
 Persistence package
 """
 
-from .repository import Repository
-from .repositories.city import City
-from .repositories.application import Application
-from .repositories.simulation_results import SimulationResults
-from .repositories.city_object import CityObject
-from .db_setup import DBSetup
-from .repositories.user import User
-from .models.user import UserRoles

hub/persistence/db_control.py

@@ -5,15 +5,15 @@ Copyright © 2022 Concordia CERC group
 Project CoderPeter Yefi peteryefi@gmail.com
 """
 import json
-from typing import Union, Dict
+from typing import Dict
 
 from hub.city_model_structure.city import City
-from hub.persistence import Application
+from hub.persistence.repositories.application import Application
-from hub.persistence import City as CityRepository
+from hub.persistence.repositories.city import City as CityRepository
-from hub.persistence import CityObject
+from hub.persistence.repositories.city_object import CityObject
-from hub.persistence import SimulationResults
+from hub.persistence.repositories.simulation_results import SimulationResults
-from hub.persistence import User
+from hub.persistence.repositories.user import User
-from hub.persistence import UserRoles
+from hub.persistence.repositories.user import UserRoles
 
 
 class DBControl:
@@ -28,31 +28,34 @@ class DBControl:
     self._city_object = CityObject(db_name=db_name, app_env=app_env, dotenv_path=dotenv_path)
     self._simulation_results = SimulationResults(db_name=db_name, dotenv_path=dotenv_path, app_env=app_env)
 
-  def application_info(self, application_uuid) -> Union[Application, None]:
+  @property
+  def identity(self):
+    return self._identity
+  def application_info(self, application_uuid) -> Application:
     """
     Retrieve the application info for the given uuid
     :param application_uuid: the uuid for the application
-    :return: Application or None
+    :return: Application
     """
     return self._application.get_by_uuid(application_uuid)
 
-  def user_info(self, name, password, application_id):
+  def user_info(self, name, password, application_id) -> User:
     """
     Retrieve the user info for the given name and password and application_id
     :param name: the username
     :param password: the user password
     :param application_id: the application id
-    :return: User or None
+    :return: User
     """
     return self._user.get_by_name_application_id_and_password(name, password, application_id)
 
-  def user_login(self, name, password, application_uuid):
+  def user_login(self, name, password, application_uuid) -> User:
     """
     Retrieve the user info
     :param name: the username
     :param password: the user password
     :param application_uuid: the application uuid
-    :return: User or None
+    :return: User
     """
     return self._user.get_by_name_application_uuid_and_password(name, password, application_uuid)
 
@@ -65,12 +68,12 @@ class DBControl:
     """
     return self._city_repository.get_by_user_id_and_application_id(user_id, application_id)
 
-  def building_info(self, name, city_id) -> Union[CityObject, None]:
+  def building_info(self, name, city_id) -> CityObject:
     """
     Retrieve the building info
     :param name: Building name
     :param city_id: City ID
-    :return: CityObject or None
+    :return: CityObject
     """
     return self._city_object.get_by_name_and_city(name, city_id)
 
@@ -114,8 +117,10 @@ class DBControl:
     :param pickle_path: Path to save the pickle file
     :param application_id: Application id owning this city
     :param user_id: User who create the city
+    return identity_id
     """
-    return self._city_repository.insert(city, pickle_path, application_id, user_id)
+    self._city_repository.insert(city, pickle_path, application_id, user_id)
+    return
 
   def update_city(self, city_id, city):
     """

hub/persistence/db_setup.py

@@ -13,8 +13,8 @@ from hub.persistence.models import CityObject
 from hub.persistence.models import User
 from hub.persistence.models import UserRoles
 from hub.persistence.models import SimulationResults
-from hub.persistence.repositories import User as UserRepository
+from hub.persistence.repositories.user import User as UserRepository
-from hub.persistence.repositories import Application as ApplicationRepository
+from hub.persistence.repositories.application import Application as ApplicationRepository
 
 
 class DBSetup:

hub/persistence/repositories/__init__.py

@@ -1,5 +1,3 @@
 """
 Repositories Package
 """
-from .user import User
-from .application import Application

hub/persistence/repositories/application.py

@@ -11,7 +11,7 @@ import logging
 from sqlalchemy import select
 from sqlalchemy.exc import SQLAlchemyError
 
-from hub.persistence import Repository
+from hub.persistence.repository import Repository
 from hub.persistence.models import Application as Model
 
 
@@ -38,18 +38,23 @@ class Application(Repository):
     :param name: Application name
     :param description: Application description
     :param application_uuid: Unique identifier for the application
-    :return: None
+    :return: Identity id
     """
-    application = self.get_by_uuid(application_uuid)
+    try:
-    if application is not None:
+      application = self.get_by_uuid(application_uuid)
-      raise SQLAlchemyError('application already exists')
+      if application is not None:
+        raise SQLAlchemyError('application already exists')
+    except TypeError:
+      pass
     try:
       application = Model(name=name, description=description, application_uuid=application_uuid)
       self.session.add(application)
       self.session.commit()
+      self.session.refresh(application)
+      return application.id
     except SQLAlchemyError as err:
       logging.error('An error occurred while creating application %s', err)
-      raise SQLAlchemyError(err)
+      raise SQLAlchemyError from err
 
   def update(self, application_uuid: str, name: str, description: str):
     """
@@ -66,7 +71,7 @@ class Application(Repository):
       self.session.commit()
     except SQLAlchemyError as err:
       logging.error('Error while updating application %s', err)
-      raise SQLAlchemyError(err)
+      raise SQLAlchemyError from err
 
   def delete(self, application_uuid: str):
     """
@@ -80,7 +85,7 @@ class Application(Repository):
       self.session.commit()
     except SQLAlchemyError as err:
       logging.error('Error while deleting application %s', err)
-      raise SQLAlchemyError(err)
+      raise SQLAlchemyError from err
 
   def get_by_uuid(self, application_uuid: str) -> Model:
     """
@@ -95,7 +100,7 @@ class Application(Repository):
       return result_set[0]
     except SQLAlchemyError as err:
       logging.error('Error while fetching application by application_uuid %s', err)
-      raise SQLAlchemyError(err)
+      raise SQLAlchemyError from err
-    except IndexError as err:
+    except TypeError as err:
       logging.error('Error while fetching application, empty result %s', err)
-      raise IndexError(err)
+      raise TypeError from err

hub/persistence/repositories/city.py

@@ -11,7 +11,7 @@ from sqlalchemy import select
 from sqlalchemy.exc import SQLAlchemyError
 
 from hub.city_model_structure.city import City as CityHub
-from hub.persistence import Repository
+from hub.persistence.repository import Repository
 from hub.persistence.models import City as Model
 from hub.persistence.models import CityObject
 from hub.version import __version__
@@ -41,7 +41,7 @@ class City(Repository):
     :param pickle_path: Path to the pickle
     :param application_id: Application id owning the instance
     :param user_id: User id owning the instance
-    :return: None
+    :return: Identity id
     """
     city.save_compressed(pickle_path)
     try:
@@ -63,9 +63,11 @@ class City(Repository):
         self.session.add(db_city_object)
         self.session.flush()
       self.session.commit()
+      self.session.refresh(db_city)
+      return db_city.id
     except SQLAlchemyError as err:
       logging.error('An error occurred while creating a city %s', err)
-      raise SQLAlchemyError(err)
+      raise SQLAlchemyError from err
 
   def update(self, city_id: int, city: CityHub):
     """
@@ -80,7 +82,7 @@ class City(Repository):
       self.session.commit()
     except SQLAlchemyError as err:
       logging.error('Error while updating city %s', err)
-      raise SQLAlchemyError(err)
+      raise SQLAlchemyError from err
 
   def delete(self, city_id: int):
     """
@@ -94,7 +96,7 @@ class City(Repository):
       self.session.commit()
     except SQLAlchemyError as err:
       logging.error('Error while fetching city %s', err)
-      raise SQLAlchemyError(err)
+      raise SQLAlchemyError from err
 
   def get_by_user_id_application_id_and_name(self, user_id, application_id, city_name) -> Model:
     """
@@ -114,7 +116,7 @@ class City(Repository):
       return result_set
     except SQLAlchemyError as err:
       logging.error('Error while fetching city by name %s', err)
-      raise SQLAlchemyError(err)
+      raise SQLAlchemyError from err
 
   def get_by_user_id_and_application_id(self, user_id, application_id) -> [Model]:
     """
@@ -130,4 +132,4 @@ class City(Repository):
       return [r[0] for r in result_set]
     except SQLAlchemyError as err:
       logging.error('Error while fetching city by name %s', err)
-      raise SQLAlchemyError(err)
+      raise SQLAlchemyError from err

hub/persistence/repositories/city_object.py

@@ -4,19 +4,21 @@ SPDX - License - Identifier: LGPL - 3.0 - or -later
 Copyright © 2022 Concordia CERC group
 Project Coder Guille Gutierrez Guillermo.GutierrezMorote@concordia.ca
 """
-import logging
 import datetime
-from typing import Union, Dict
+import logging
 
 from sqlalchemy import select
 from sqlalchemy.exc import SQLAlchemyError
 
-from hub.persistence import Repository
-from hub.persistence.models import CityObject as Model
 from hub.city_model_structure.building import Building
+from hub.persistence.repository import Repository
+from hub.persistence.models import CityObject as Model
 
 
 class CityObject(Repository):
+  """
+  City object repository
+  """
   _instance = None
 
   def __init__(self, db_name: str, dotenv_path: str, app_env: str):
@@ -35,7 +37,7 @@ class CityObject(Repository):
     Inserts a new city object
     :param city_id: city id for the city owning this city object
     :param building: the city object (only building for now) to be inserted
-    return None
+    return Identity id
     """
     city_object = self.get_by_name_and_city(building.name, city_id)
     if city_object is not None:
@@ -46,11 +48,11 @@ class CityObject(Repository):
       self.session.add(city_object)
       self.session.flush()
       self.session.commit()
-
+      self.session.refresh(city_object)
+      return city_object.id
     except SQLAlchemyError as err:
-      error_message = f'An error occurred while creating city_object {err}'
+      logging.error('An error occurred while creating city_object %s', err)
-      logging.error(error_message)
+      raise SQLAlchemyError from err
-      raise SQLAlchemyError(error_message)
 
   def update(self, city_id: int, building: Building):
     """
@@ -77,9 +79,8 @@ class CityObject(Repository):
          'updated': datetime.datetime.utcnow()})
       self.session.commit()
     except SQLAlchemyError as err:
-      error_message = f'Error while updating city object {err}'
+      logging.error('Error while updating city object %s', err)
-      logging.error(error_message)
+      raise SQLAlchemyError from err
-      raise SQLAlchemyError(error_message)
 
   def delete(self, city_id: int, name: str):
     """
@@ -92,9 +93,8 @@ class CityObject(Repository):
       self.session.query(Model).filter(Model.city_id == city_id, Model.name == name).delete()
       self.session.commit()
     except SQLAlchemyError as err:
-      error_message = f'Error while deleting application {err}'
+      logging.error('Error while deleting application %s', err)
-      logging.error(error_message)
+      raise SQLAlchemyError from err
-      raise SQLAlchemyError(error_message)
 
   def get_by_name_and_city(self, name, city_id) -> Model:
     """
@@ -111,9 +111,7 @@ class CityObject(Repository):
       return _city_object[0]
     except SQLAlchemyError as err:
       logging.error('Error while fetching city object by name and city: %s', err)
-      raise SQLAlchemyError(err)
+      raise SQLAlchemyError from err
     except IndexError as err:
       logging.error('Error while fetching city object by name and city, empty result %s', err)
-      raise IndexError(err)
+      raise IndexError from err
-
-

hub/persistence/repositories/simulation_results.py

@@ -11,7 +11,7 @@ from sqlalchemy import or_
 from sqlalchemy import select
 from sqlalchemy.exc import SQLAlchemyError
 
-from hub.persistence import Repository
+from hub.persistence.repository import Repository
 from hub.persistence.models import City
 from hub.persistence.models import CityObject
 from hub.persistence.models import SimulationResults as Model
@@ -41,7 +41,7 @@ class SimulationResults(Repository):
     :param values: the simulation results in json format
     :param city_id: optional city id
     :param city_object_id: optional city object id
-    :return: None
+    :return: Identity id
     """
     if city_id is not None:
       _ = self._get_city(city_id)
@@ -55,9 +55,11 @@ class SimulationResults(Repository):
       self.session.add(simulation_result)
       self.session.flush()
       self.session.commit()
+      self.session.refresh(simulation_result)
+      return simulation_result.id
     except SQLAlchemyError as err:
       logging.error('An error occurred while creating city_object %s', err)
-      SQLAlchemyError(err)
+      raise SQLAlchemyError from err
 
   def update(self, name: str, values: str, city_id=None, city_object_id=None):
     """
@@ -87,7 +89,7 @@ class SimulationResults(Repository):
         raise NotImplementedError('Missing either city_id or city_object_id')
     except SQLAlchemyError as err:
       logging.error('Error while updating city object %s', err)
-      SQLAlchemyError(err)
+      raise SQLAlchemyError from err
 
   def delete(self, name: str, city_id=None, city_object_id=None):
     """
@@ -108,7 +110,7 @@ class SimulationResults(Repository):
         raise NotImplementedError('Missing either city_id or city_object_id')
     except SQLAlchemyError as err:
       logging.error('Error while deleting application: %s', err)
-      raise SQLAlchemyError(err)
+      raise SQLAlchemyError from err
 
   def _get_city(self, city_id) -> City:
     """
@@ -120,7 +122,7 @@ class SimulationResults(Repository):
       return self.session.execute(select(City).where(City.id == city_id)).first()
     except SQLAlchemyError as err:
       logging.error('Error while fetching city by city_id: %s', err)
-      raise SQLAlchemyError(err)
+      raise SQLAlchemyError from err
 
   def _get_city_object(self, city_object_id) -> [CityObject]:
     """
@@ -132,7 +134,7 @@ class SimulationResults(Repository):
       return self.session.execute(select(CityObject).where(CityObject.id == city_object_id)).first()
     except SQLAlchemyError as err:
       logging.error('Error while fetching city by city_id: %s', err)
-      SQLAlchemyError(err)
+      raise SQLAlchemyError from err
 
   def get_simulation_results_by_city_id_city_object_id_and_names(self, city_id, city_object_id, result_names=None) -> [Model]:
     """
@@ -157,4 +159,4 @@ class SimulationResults(Repository):
       return filtered_results
     except SQLAlchemyError as err:
       logging.error('Error while fetching city by city_id: %s', err)
-    return None
+      raise SQLAlchemyError from err

hub/persistence/repositories/user.py

@@ -4,15 +4,15 @@ SPDX - License - Identifier: LGPL - 3.0 - or -later
 Copyright © 2022 Concordia CERC group
 Project Coder Peter Yefi peteryefi@gmail.com
 """
-import logging
-from typing import Union, Dict
 import datetime
-from sqlalchemy.exc import SQLAlchemyError
+import logging
-from sqlalchemy import select
-from hub.persistence import Repository
-from hub.persistence.models import User as Model, Application as ApplicationModel, UserRoles
-from hub.helpers.auth import Auth
 
+from sqlalchemy import select
+from sqlalchemy.exc import SQLAlchemyError
+
+from hub.helpers.auth import Auth
+from hub.persistence.repository import Repository
+from hub.persistence.models import User as Model, Application as ApplicationModel, UserRoles
 
 
 class User(Repository):
@@ -32,36 +32,40 @@ class User(Repository):
       cls._instance = super(User, cls).__new__(cls)
     return cls._instance
 
-  def insert(self, name: str, password: str, role: UserRoles, application_id: int) -> Union[Model, Dict]:
+  def insert(self, name: str, password: str, role: UserRoles, application_id: int):
     """
     Inserts a new user
     :param name: username
     :param password: user password
     :param role: user rol [Admin or Hub_Reader]
     :param application_id: user application id
-    :return: [User, Dictionary]
+    :return: Identity id
     """
-    user = self.get_by_name_and_application(name, application_id)
+    try:
-    if user is None:
+      user = self.get_by_name_and_application(name, application_id)
-      try:
+      if user is not None:
-        user = Model(name=name, password=Auth.hash_password(password), role=role, application_id=application_id)
+        raise SQLAlchemyError(f'A user named {user.name} already exists for that application')
-        self.session.add(user)
+    except TypeError:
-        self.session.flush()
+      pass
-        self.session.commit()
+    try:
-        return user
+      user = Model(name=name, password=Auth.hash_password(password), role=role, application_id=application_id)
-      except SQLAlchemyError as err:
+      self.session.add(user)
-        error_message = f'An error occurred while creating user: {err}'
+      self.session.flush()
-        logging.error(error_message)
+      self.session.commit()
-    return {'message': f'error creating user {name}'}
+      self.session.refresh(user)
+      return user.id
+    except SQLAlchemyError as err:
+      logging.error('An error occurred while creating user %s', err)
+      raise SQLAlchemyError from err
 
-  def update(self, user_id: int, name: str, password: str, role: UserRoles) -> Union[Dict, None]:
+  def update(self, user_id: int, name: str, password: str, role: UserRoles):
     """
     Updates a user
     :param user_id: the id of the user to be updated
     :param name: the name of the user
     :param password: the password of the user
     :param role: the role of the user
-    :return: None, Dictionary
+    :return: None
     """
     try:
       self.session.query(Model).filter(Model.id == user_id).update({
@@ -73,8 +77,7 @@ class User(Repository):
       self.session.commit()
     except SQLAlchemyError as err:
       logging.error('Error while updating user: %s', err)
-      return {'err_msg': 'Error occurred while updated user'}
+      raise SQLAlchemyError from err
-    return None
 
   def delete(self, user_id: int):
     """
@@ -87,28 +90,30 @@ class User(Repository):
       self.session.commit()
     except SQLAlchemyError as err:
       logging.error('Error while fetching user: %s', err)
+      raise SQLAlchemyError from err
 
-  def get_by_name_and_application(self, name: str, application_id: int) -> Union[Model, None]:
+  def get_by_name_and_application(self, name: str, application_id: int) -> Model:
     """
     Fetch user based on the email address
     :param name: Username
     :param application_id: User application name
     :return: User matching the search criteria or None
     """
-    user = None
     try:
       user = self.session.execute(
         select(Model).where(Model.name == name, Model.application_id == application_id)
       ).first()
-      if user is not None:
+      return user[0]
-        user = user[0]
     except SQLAlchemyError as err:
       logging.error('Error while fetching user by name and application: %s', err)
-    return user
+      raise SQLAlchemyError from err
+    except TypeError as err:
+      logging.error('Error while fetching user, empty result %s', err)
+      raise TypeError from err
 
-  def get_by_name_application_id_and_password(self, name: str, password: str, application_id: int) -> Union[Model, None]:
+  def get_by_name_application_id_and_password(self, name: str, password: str, application_id: int) -> Model:
     """
-    Fetch user based on the email and password
+    Fetch user based on the name, password and application id
     :param name: Username
     :param password: User password
     :param application_id: Application id
@@ -122,10 +127,11 @@ class User(Repository):
         if Auth.check_password(password, user[0].password):
           return user[0]
     except SQLAlchemyError as err:
-      logging.error('Error while fetching user by email: %s', err)
+      logging.error('Error while fetching user by name: %s', err)
-    return None
+      raise SQLAlchemyError from err
+    raise ValueError('Unauthorized')
 
-  def get_by_name_application_uuid_and_password(self, name: str, password: str, application_uuid: str) -> Union[Model, None]:
+  def get_by_name_application_uuid_and_password(self, name: str, password: str, application_uuid: str) -> Model:
     """
     Fetch user based on the email and password
     :param name: Username
@@ -133,11 +139,13 @@ class User(Repository):
     :param application_uuid: Application uuid
     :return: User
     """
-    application = None
     try:
       application = self.session.execute(
         select(ApplicationModel).where(ApplicationModel.application_uuid == application_uuid)
       ).first()
+      return self.get_by_name_application_id_and_password(name, password, application[0].id)
     except SQLAlchemyError as err:
       logging.error('Error while fetching user by name: %s', err)
-    return self.get_by_name_application_id_and_password(name, password, application[0].id)
+      raise SQLAlchemyError from err
+    except ValueError as err:
+      raise ValueError from err

hub/persistence/repository.py

@@ -10,7 +10,6 @@ from sqlalchemy.orm import Session
 from hub.persistence.configuration import Configuration
 
 
-
 class Repository:
   """
   Base repository class to establish db connection

hub/unittests/test_db_factory.py

@@ -73,8 +73,8 @@ class Control:
       dotenv_path=dotenv_path)
 
     self._unique_id = str(uuid.uuid4())
-    self._application = self._database.persist_application("test", "test application", self.unique_id)
+    self._application_id = self._database.persist_application("test", "test application", self.unique_id)
-    self._user = self._database.create_user("Admin", self.application.id, "Admin@123", UserRoles.Admin)
+    self._user_id = self._database.create_user("Admin", self._application_id, "Admin@123", UserRoles.Admin)
     self._pickle_path = 'tests_data/pickle_path.bz2'
 
   @property
@@ -86,12 +86,12 @@ class Control:
     return self._unique_id
 
   @property
-  def application(self):
+  def application_id(self):
-    return self._application
+    return self._application_id
 
   @property
-  def user(self):
+  def user_id(self):
-    return self._user
+    return self._user_id
 
   @property
   def skip_test(self):
@@ -122,39 +122,30 @@ class TestDBFactory(TestCase):
 TestDBFactory
 """
 
-  @unittest.skipIf(control.skip_test, control.skip_reason)
-  def test_save_application(self):
-    self.assertEqual(control.application.name, "test")
-    self.assertEqual(control.application.description, "test application")
-    self.assertEqual(str(control.application.application_uuid), control.unique_id)
-
   @unittest.skipIf(control.skip_test, control.skip_reason)
   def test_save_city(self):
     control.city.name = "Montreal"
-    city = control.database.persist_city(
+    city_id = control.database.persist_city(
       control.city,
       control.pickle_path,
-      control.application.id,
+      control.application_id,
-      control.user.id)
+      control.user_id)
-    self.assertEqual(city.name, 'Montreal')
+    control.database.delete_city(city_id)
-    self.assertEqual(city.pickle_path, control.pickle_path)
-    self.assertEqual(city.level_of_detail, control.city.level_of_detail.geometry)
-    control.database.delete_city(city.id)
     os.unlink(control.pickle_path)
 
   @unittest.skipIf(control.skip_test, control.skip_reason)
   def test_get_update_city(self):
-    city = control.database.persist_city(control.city,
+    city_id = control.database.persist_city(control.city,
                                          control.pickle_path,
-                                         control.application.id,
+                                         control.application_id,
-                                         control._user.id)
+                                         control.user_id)
-    city.name = "Ottawa"
+    control.city.name = "Ottawa"
-    control.database.update_city(city.id, control.city)
+    control.database.update_city(city_id, control.city)
     cities = control.database.cities_by_user_and_application(
-      control.user.id,
+      control.user_id,
-      control.application.id)
+      control.application_id)
     for updated_city in cities:
-      if updated_city.id == city.id:
+      if updated_city.id == city_id:
-        self.assertEqual(updated_city.name, city.name)
+        self.assertEqual(updated_city.name,control.city.name)
         break
-    control.database.delete_city(city.id)
+    control.database.delete_city(city_id)