storage code cleanup

apiserver/main.py

+import logging
 from datetime import timedelta
 from enum import Enum
-from typing import Dict, Optional
 
-from fastapi import FastAPI, HTTPException, status, Request
+from fastapi import FastAPI, HTTPException, Request, status
 from fastapi.param_functions import Depends
 from fastapi.responses import JSONResponse
 from fastapi.security import OAuth2PasswordBearer, OAuth2PasswordRequestForm
 
 from .config import ApiserverSettings
-from .security import (ACCESS_TOKEN_EXPIRES_MINUTES, AbstractDBInterface,
-                       JsonDBInterface, Token, User, authenticate_user,
-                       create_access_token, get_current_user)
-from .storage import (AbstractLocationDataStorageAdapter,
-                      JsonFileStorageAdapter, LocationData, LocationDataType)
+from .security import (ACCESS_TOKEN_EXPIRES_MINUTES, JsonDBInterface, Token,
+                       User, authenticate_user, create_access_token,
+                       get_current_user)
+from .storage import JsonFileStorageAdapter, LocationData, LocationDataType
 
 
 class ReservedPaths(str, Enum):
@@ -102,7 +101,6 @@ async def update_specific_dataset(location_data_type: LocationDataType,
                                   dataset_id: str, dataset: LocationData,
                                   user: User = Depends(my_user)):
     # update the information about a specific dataset, identified by id
-   
     return adapter.update_details(location_data_type, dataset_id, dataset, user.username)
 
 
@@ -116,7 +114,8 @@ async def delete_specific_dataset(location_data_type: LocationDataType,
 
 
 @app.exception_handler(FileNotFoundError)
-async def not_found_handler(request: Request, exc: FileNotFoundError):
+async def not_found_handler(request: Request, ex: FileNotFoundError):
     oid=request.path_params.get('dataset_id', '')
+    logging.error("File not found translated %s", ex)
     return JSONResponse(status_code=status.HTTP_404_NOT_FOUND,
                         content={'message':f"Object {oid} does not exist"})

apiserver/security/user.py

+import logging
 import abc
 import json
 import os
@@ -5,7 +6,7 @@ import warnings
 from datetime import datetime, timedelta
 from typing import List, Optional
 
-from fastapi import Depends, HTTPException, status
+from fastapi import HTTPException, status
 from passlib.context import CryptContext
 from pydantic import BaseModel
 
@@ -58,9 +59,9 @@ class AbstractDBInterface(metaclass=abc.ABCMeta):
 class JsonDBInterface(AbstractDBInterface):
 
     def __init__(self, settings: ApiserverSettings):
-        print(f"Recreating userdb {settings}")
-        self.filePath = settings.userdb_path
-        if not (os.path.exists(self.filePath) and os.path.isfile(self.filePath)):
+        logging.info("Recreating userdb %s", settings)
+        self.file_path = settings.userdb_path
+        if not (os.path.exists(self.file_path) and os.path.isfile(self.file_path)):
             # create empty json
             self.__save_all({})
         else:
@@ -68,11 +69,11 @@ class JsonDBInterface(AbstractDBInterface):
             _ = self.__read_all()
 
     def __read_all(self):
-        with open(self.filePath, 'r') as f:
+        with open(self.file_path, 'r') as f:
             return json.load(f)
 
     def __save_all(self, data):
-        with open(self.filePath, 'w') as f:
+        with open(self.file_path, 'w') as f:
             json.dump(data, f)
 
     def list(self):
@@ -133,6 +134,7 @@ credentials_exception = HTTPException(
     headers={"WWW-Authenticate": "Bearer"},
 )
 
+
 def get_current_user(token: str, userdb: AbstractDBInterface):
     try:
         payload = jwt.decode(token, SECRET_KEY, algorithms=[ALGORITHM])
@@ -142,4 +144,4 @@ def get_current_user(token: str, userdb: AbstractDBInterface):
 
         return user
     except JWTError:
-        raise credentials_exception
+        raise credentials_exception from JWTError

apiserver/storage/JsonFileStorageAdapter.py

@@ -15,6 +15,14 @@ class StoredData(BaseModel):
     actualData: LocationData
     users: List[str]
 
+def load_object(path):
+    return StoredData.parse_file(path)
+
+def get_unique_id(path: str) -> str:
+    oid = str(uuid.uuid4())
+    while os.path.exists(os.path.join(path, oid)):
+        oid = str(uuid.uuid4())
+    return oid
 
 class JsonFileStorageAdapter(AbstractLocationDataStorageAdapter):
     """ This stores LocationData via the StoredData Object as json files
@@ -36,7 +44,7 @@ class JsonFileStorageAdapter(AbstractLocationDataStorageAdapter):
 
     def __setup_path(self, value: str) -> str:
         localpath = os.path.join(self.data_dir, value)
-        if not (os.path.isdir(localpath)):
+        if not os.path.isdir(localpath):
             os.mkdir(localpath)
         return localpath
 
@@ -48,43 +56,34 @@ class JsonFileStorageAdapter(AbstractLocationDataStorageAdapter):
                 f"The requested object ({oid}) does not exist.")
         return fullpath
 
-    def __load_object(self, path):
-        return StoredData.parse_file(path)
-
-    def __get_unique_id(self, path: str) -> str:
-        oid = str(uuid.uuid4())
-        while (os.path.exists(os.path.join(path, oid))):
-            oid = str(uuid.uuid4())
-        return oid
-
     def get_list(self, n_type: LocationDataType) -> List:
         local_path = self.__setup_path(n_type.value)
-        retList = []
+        ret = []
         for f in os.listdir(local_path):
             p = os.path.join(local_path, f)
             if not os.path.isfile(p):
                 continue
-            data = self.__load_object(p)
-            retList.append((data.actualData.name, f))
-        return retList
+            data = load_object(p)
+            ret.append((data.actualData.name, f))
+        return ret
 
     def add_new(self, n_type: LocationDataType, data: LocationData, user_name: str):
         localpath = self.__setup_path(value=n_type.value)
-        oid = self.__get_unique_id(path=localpath)
-        toStore = StoredData(users=[user_name], actualData=data)
+        oid = get_unique_id(path=localpath)
+        to_store = StoredData(users=[user_name], actualData=data)
         with open(os.path.join(localpath, oid), 'w') as json_file:
-            json.dump(toStore.dict(), json_file)
+            json.dump(to_store.dict(), json_file)
         return (oid, data)
 
     def get_details(self, n_type: LocationDataType, oid: str):
         full_path = self.__get_object_path(value=n_type.value, oid=oid)
-        obj = self.__load_object(path=full_path)
+        obj = load_object(path=full_path)
         return obj.actualData
 
     def update_details(self, n_type: LocationDataType, oid: str, data: LocationData, usr: str):
         # TODO: usr is ignored here?
         full_path = self.__get_object_path(value=n_type.value, oid=oid)
-        obj = self.__load_object(path=full_path)
+        obj = load_object(path=full_path)
         obj.actualData = data
 
         with open(full_path, 'w') as f:
@@ -93,17 +92,19 @@ class JsonFileStorageAdapter(AbstractLocationDataStorageAdapter):
         return (oid, data)
 
     def delete(self, n_type: LocationDataType, oid: str, usr: str):
-        fullpath = self.__get_object_path(value=n_type.value, oid=oid)
-        os.remove(fullpath)
+        full_path = self.__get_object_path(value=n_type.value, oid=oid)
+        os.remove(full_path)
 
-    def get_owner(self, type: LocationDataType, oid: str):
+    def get_owner(self, n_type: LocationDataType, oid: str):
         raise NotImplementedError()
 
-    def check_perm(self, type: LocationDataType, oid: str, usr: str):
+    def check_perm(self, n_type: LocationDataType, oid: str, usr: str):
         raise NotImplementedError()
 
-    def add_perm(self, type: LocationDataType, oid: str, authUsr: str, newUser: str):
+    def add_perm(self, n_type: LocationDataType, oid: str, usr: str):
+        """add user to file perm"""
         raise NotImplementedError()
 
-    def rm_perm(self, type: LocationDataType, oid: str, usr: str, rmUser: str):
+    def rm_perm(self, n_type: LocationDataType, oid: str, usr: str):
+        """remove user from file perm"""
         raise NotImplementedError()

apiserver/storage/LocationStorage.py

@@ -17,7 +17,7 @@ class LocationData(BaseModel):
 
 
 class AbstractLocationDataStorageAdapter:
-    '''
+    """
     This is an abstract storage adapter for storing information about datasets,
     storage targets and similar things. It can easily be expanded to also store
     other data (that has roughly similar metadata), just by expanding
@@ -34,41 +34,48 @@ class AbstractLocationDataStorageAdapter:
     verified email or a user name. The management of authentication etc. is
     done by the caller, this adapter assumes that the user id fulfills the criteria.
     Permissions are stored as a list of user ids, and every id is authorized for full access.
-    '''
+    """
 
     def get_list(self, n_type: LocationDataType) -> List:
-        # get a list of all LocationData Elements with the provided type, as pairs of {name : id}
+        """Get a list of all LocationData Elements with the provided type, as pairs of {name : id}"""
+
         raise NotImplementedError()
     
-    def add_new(self, n_type: LocationDataType, data: LocationData, usr: str):
-        # add a new element of the provided type, assign and return the id and 
-        # the new data as {id : LocationData}
+    def add_new(self, n_type: LocationDataType, data: LocationData, user_name: str):
+        """
+        add a new element of the provided type, assign and return the id and
+        the new data as {id : LocationData}
+        """
+
         raise NotImplementedError()
 
     def get_details(self, n_type: LocationDataType, oid: str):
-        # return the LocationData of the requested object (identified by oid and type)
+        """ return the LocationData of the requested object (identified by oid and type)"""
         raise NotImplementedError()
 
     def update_details(self, n_type: LocationDataType, oid: str, data: LocationData, usr: str):
-        # change the details of the requested object, return {oid : newData}
+        """ change the details of the requested object, return {oid : newData}"""
         raise NotImplementedError()
 
     def delete(self, n_type: LocationDataType, oid: str, usr: str):
+        """ deletes given resource"""
         raise NotImplementedError()
 
     def get_owner(self, n_type: LocationDataType, oid: str):
-        # return the owner of the requested object; if multiple owners are set, 
-        # return them is a list
+        """
+        return the owner of the requested object; if multiple owners are set,
+        return them is a list
+        """
         raise NotImplementedError()
 
     def check_perm(self, n_type: LocationDataType, oid: str, usr: str):
-        # check if the given user has permission to change the given object
+        """ check if the given user has permission to change the given object"""
         raise NotImplementedError()
 
     def add_perm(self, n_type: LocationDataType, oid: str, usr: str):
-        # add user to file perm
+        """add user to file perm"""
         raise NotImplementedError()
 
     def rm_perm(self, n_type: LocationDataType, oid: str, usr: str):
-        # remove user from file perm
+        """remove user from file perm"""
         raise NotImplementedError()