From 179a3c9d5a9d83797aa9bdce8243f8fd9bc006cd Mon Sep 17 00:00:00 2001 From: martin1tab Date: Sun, 15 Nov 2020 18:12:18 +0100 Subject: [PATCH 01/19] [enhancement] Changing query filters from list of tuples to dict --- src/app/api/crud.py | 16 ++++++++-------- src/app/api/deps.py | 4 ++-- src/app/api/routes/devices.py | 2 +- src/app/api/routing.py | 6 +++--- src/tests/test_devices.py | 2 +- 5 files changed, 15 insertions(+), 15 deletions(-) diff --git a/src/app/api/crud.py b/src/app/api/crud.py index 774f73d6..ae41610b 100644 --- a/src/app/api/crud.py +++ b/src/app/api/crud.py @@ -1,4 +1,4 @@ -from typing import Optional, Tuple, Any, List +from typing import Optional, Tuple, Any, List, Dict from app.db import database from sqlalchemy import Table from pydantic import BaseModel @@ -14,18 +14,18 @@ async def get(entry_id: int, table: Table): return await database.fetch_one(query=query) -async def fetch_all(table: Table, query_filters: Optional[List[Tuple[str, Any]]] = None): +async def fetch_all(table: Table, query_filters: Optional[Dict[str, Any]] = None): query = table.select() - if isinstance(query_filters, list): - for query_filter in query_filters: - query = query.where(getattr(table.c, query_filter[0]) == query_filter[1]) + if isinstance(query_filters, dict): + for queryFilterKey, queryFilterValue in query_filters: + query = query.where(getattr(table.c, queryFilterKey) == queryFilterValue) return await database.fetch_all(query=query) -async def fetch_one(table: Table, query_filters: List[Tuple[str, Any]]): +async def fetch_one(table: Table, query_filters: Dict[str, Any]): query = table.select() - for query_filter in query_filters: - query = query.where(getattr(table.c, query_filter[0]) == query_filter[1]) + for queryFilterKey, queryFilterValue in query_filters: + query = query.where(getattr(table.c, queryFilterKey) == queryFilterValue) return await database.fetch_one(query=query) diff --git a/src/app/api/deps.py b/src/app/api/deps.py index 103edcc4..cdf85756 100644 --- a/src/app/api/deps.py +++ b/src/app/api/deps.py @@ -60,7 +60,7 @@ async def get_current_access(security_scopes: SecurityScopes, token: str = Depen async def get_current_user(access=Depends(get_current_access)): - user = await crud.fetch_one(users, [('access_id', access.id)]) + user = await crud.fetch_one(users, {'access_id': access.id}) if user is None: # Could be a "permission denied error as well" @@ -70,7 +70,7 @@ async def get_current_user(access=Depends(get_current_access)): async def get_current_device(access=Depends(get_current_access)): - device = await crud.fetch_one(devices, [('access_id', access.id)]) + device = await crud.fetch_one(devices, {'access_id': access.id}) if device is None: # Could be a "permission denied error as well" raise HTTPException(status_code=400, detail="Non existing device") diff --git a/src/app/api/routes/devices.py b/src/app/api/routes/devices.py index 3836ec17..3457c013 100644 --- a/src/app/api/routes/devices.py +++ b/src/app/api/routes/devices.py @@ -36,7 +36,7 @@ async def delete_device(device_id: int = Path(..., gt=0), _=Security(get_current @router.get("/my-devices", response_model=List[DeviceOut]) async def fetch_my_devices(me: UserRead = Security(get_current_user, scopes=["me"])): - return await routing.fetch_entries(devices, ("owner_id", me.id)) + return await routing.fetch_entries(devices, {"owner_id": me.id}) @router.post("/heartbeat", response_model=HeartbeatOut) diff --git a/src/app/api/routing.py b/src/app/api/routing.py index 4968b629..bd77a4c8 100644 --- a/src/app/api/routing.py +++ b/src/app/api/routing.py @@ -1,6 +1,6 @@ from sqlalchemy import Table from pydantic import BaseModel -from typing import Optional, Tuple, Any, List +from typing import Optional, Tuple, Any, List, Dict from fastapi import HTTPException, Path from datetime import datetime from app.api import crud, security @@ -23,11 +23,11 @@ async def get_entry(table: Table, entry_id: int = Path(..., gt=0)): return entry -async def fetch_entries(table: Table, query_filter: Optional[List[Tuple[str, Any]]] = None): +async def fetch_entries(table: Table, query_filter: Optional[Dict[str, Any]] = None): return await crud.fetch_all(table, query_filter) -async def fetch_entry(table: Table, query_filter: List[Tuple[str, Any]]): +async def fetch_entry(table: Table, query_filter: Dict[str, Any]): return await crud.fetch_one(table, query_filter) diff --git a/src/tests/test_devices.py b/src/tests/test_devices.py index 0bbff1e2..0d877bd4 100644 --- a/src/tests/test_devices.py +++ b/src/tests/test_devices.py @@ -23,7 +23,7 @@ def test_fetch_my_devices(test_app, monkeypatch): test_data = [{"id": did, **REPLY_PAYLOAD, "owner_id": 99 if did <= 4 else 1} for did in range(1, 7)] async def mock_fetch_by_owner(table, query_filter): - return [entry for entry in test_data if entry[query_filter[0]] == query_filter[1]] + return [entry for entry in test_data if entry["owner_id"] == query_filter["owner_id"]] monkeypatch.setattr(crud, "fetch_all", mock_fetch_by_owner) From 3116e7d2a0dabc7482968bf87759026240b739a7 Mon Sep 17 00:00:00 2001 From: martin1tab Date: Mon, 16 Nov 2020 19:10:03 +0100 Subject: [PATCH 02/19] Add test for getCurrentUser --- requirements.txt | 1 + src/app/api/crud.py | 2 +- src/app/api/routing.py | 2 +- src/tests/conftest.py | 4 ++-- src/tests/test_deps.py | 28 ++++++++++++++++++++++++++++ 5 files changed, 33 insertions(+), 4 deletions(-) create mode 100644 src/tests/test_deps.py diff --git a/requirements.txt b/requirements.txt index a891fbab..0f22807c 100644 --- a/requirements.txt +++ b/requirements.txt @@ -8,6 +8,7 @@ python-multipart==0.0.5 # dev pytest>=5.3.2 +pytest-asyncio>=0.14.0 requests>=2.22.0 asyncpg>=0.20.0 coverage>=4.5.4 diff --git a/src/app/api/crud.py b/src/app/api/crud.py index ae41610b..5d37d680 100644 --- a/src/app/api/crud.py +++ b/src/app/api/crud.py @@ -1,4 +1,4 @@ -from typing import Optional, Tuple, Any, List, Dict +from typing import Optional, Any, List, Dict from app.db import database from sqlalchemy import Table from pydantic import BaseModel diff --git a/src/app/api/routing.py b/src/app/api/routing.py index bd77a4c8..afdc0772 100644 --- a/src/app/api/routing.py +++ b/src/app/api/routing.py @@ -1,6 +1,6 @@ from sqlalchemy import Table from pydantic import BaseModel -from typing import Optional, Tuple, Any, List, Dict +from typing import Optional, Any, List, Dict from fastapi import HTTPException, Path from datetime import datetime from app.api import crud, security diff --git a/src/tests/conftest.py b/src/tests/conftest.py index 01d09b6e..e5f0eaab 100644 --- a/src/tests/conftest.py +++ b/src/tests/conftest.py @@ -4,7 +4,7 @@ from app.main import app from app.api.schemas import UserRead, DeviceOut -from app.api.deps import get_current_user, get_current_device +from app.api.deps import get_current_user, get_current_device, get_current_access async def mock_current_user(): @@ -14,7 +14,6 @@ async def mock_current_user(): async def mock_current_device(): return DeviceOut(id=99, owner_id=1, specs="raspberry", name="connected_device", created_at=dt.now()) - @pytest.fixture(scope="module") def test_app(): @@ -25,3 +24,4 @@ def test_app(): yield client # testing happens here app.dependency_overrides = {} + diff --git a/src/tests/test_deps.py b/src/tests/test_deps.py new file mode 100644 index 00000000..3c9b8f0b --- /dev/null +++ b/src/tests/test_deps.py @@ -0,0 +1,28 @@ +import json +import pytest + +from app.api import crud +from app.api.deps import get_current_access, get_current_user +from app.api.schemas import AccessRead, UserRead + +@pytest.mark.asyncio +async def testGetCurrentUser(test_app,monkeypatch): + + test_user_data = [ + {"username": "JohnDoe", "id": 1, "access_id":1}, + ] + + async def mock_fetch_one(table, query_filters): + for entry in test_user_data: + for queryFilterKey, queryFilterValue in query_filters.items(): + valid_entry = True + if entry[queryFilterKey] != queryFilterValue: + valid_entry = False + if valid_entry: + return entry + + monkeypatch.setattr(crud, "fetch_one", mock_fetch_one) + + response = await get_current_user(AccessRead(id=1, login= "JohnDoe", scopes="me")) + assert response == UserRead(**test_user_data[0]) + From 0aedd8ba3262039a9b936e70ab87f62c67a7b898 Mon Sep 17 00:00:00 2001 From: martin1tab Date: Mon, 16 Nov 2020 19:10:03 +0100 Subject: [PATCH 03/19] Add test for getCurrentUser --- requirements.txt | 1 + src/app/api/crud.py | 2 +- src/app/api/routing.py | 2 +- src/tests/conftest.py | 4 ++-- src/tests/test_deps.py | 28 ++++++++++++++++++++++++++++ 5 files changed, 33 insertions(+), 4 deletions(-) create mode 100644 src/tests/test_deps.py diff --git a/requirements.txt b/requirements.txt index a891fbab..0f22807c 100644 --- a/requirements.txt +++ b/requirements.txt @@ -8,6 +8,7 @@ python-multipart==0.0.5 # dev pytest>=5.3.2 +pytest-asyncio>=0.14.0 requests>=2.22.0 asyncpg>=0.20.0 coverage>=4.5.4 diff --git a/src/app/api/crud.py b/src/app/api/crud.py index ae41610b..5d37d680 100644 --- a/src/app/api/crud.py +++ b/src/app/api/crud.py @@ -1,4 +1,4 @@ -from typing import Optional, Tuple, Any, List, Dict +from typing import Optional, Any, List, Dict from app.db import database from sqlalchemy import Table from pydantic import BaseModel diff --git a/src/app/api/routing.py b/src/app/api/routing.py index bd77a4c8..afdc0772 100644 --- a/src/app/api/routing.py +++ b/src/app/api/routing.py @@ -1,6 +1,6 @@ from sqlalchemy import Table from pydantic import BaseModel -from typing import Optional, Tuple, Any, List, Dict +from typing import Optional, Any, List, Dict from fastapi import HTTPException, Path from datetime import datetime from app.api import crud, security diff --git a/src/tests/conftest.py b/src/tests/conftest.py index 01d09b6e..c5fc054b 100644 --- a/src/tests/conftest.py +++ b/src/tests/conftest.py @@ -4,7 +4,7 @@ from app.main import app from app.api.schemas import UserRead, DeviceOut -from app.api.deps import get_current_user, get_current_device +from app.api.deps import get_current_user, get_current_device, get_current_access async def mock_current_user(): @@ -24,4 +24,4 @@ def test_app(): client = TestClient(app) yield client # testing happens here - app.dependency_overrides = {} + app.dependency_overrides = {} \ No newline at end of file diff --git a/src/tests/test_deps.py b/src/tests/test_deps.py new file mode 100644 index 00000000..b7951b15 --- /dev/null +++ b/src/tests/test_deps.py @@ -0,0 +1,28 @@ +import json +import pytest + +from app.api import crud +from app.api.deps import get_current_access, get_current_user +from app.api.schemas import AccessRead, UserRead + + +@pytest.mark.asyncio +async def testGetCurrentUser(test_app, monkeypatch): + + test_user_data = [ + {"username": "JohnDoe", "id": 1, "access_id":1}, + ] + + async def mock_fetch_one(table, query_filters): + for entry in test_user_data: + for queryFilterKey, queryFilterValue in query_filters.items(): + valid_entry = True + if entry[queryFilterKey] != queryFilterValue: + valid_entry = False + if valid_entry: + return entry + + monkeypatch.setattr(crud, "fetch_one", mock_fetch_one) + + response = await get_current_user(AccessRead(id=1, login="JohnDoe", scopes="me")) + assert response == UserRead(**test_user_data[0]) \ No newline at end of file From f12c75427edc781b73ecd18924e37e871d20bf93 Mon Sep 17 00:00:00 2001 From: martin1tab Date: Mon, 16 Nov 2020 19:32:57 +0100 Subject: [PATCH 04/19] Fix some issues --- src/tests/test_deps.py | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/tests/test_deps.py b/src/tests/test_deps.py index 89979359..194e09d2 100644 --- a/src/tests/test_deps.py +++ b/src/tests/test_deps.py @@ -5,14 +5,9 @@ from app.api.deps import get_current_access, get_current_user from app.api.schemas import AccessRead, UserRead -<<<<<<< HEAD @pytest.mark.asyncio async def testGetCurrentUser(test_app, monkeypatch): -======= -@pytest.mark.asyncio -async def testGetCurrentUser(test_app,monkeypatch): ->>>>>>> 3116e7d2a0dabc7482968bf87759026240b739a7 test_user_data = [ {"username": "JohnDoe", "id": 1, "access_id":1}, From faa9c386fd799f9747ff643185d003aace67a7c2 Mon Sep 17 00:00:00 2001 From: martin1tab Date: Mon, 16 Nov 2020 19:38:37 +0100 Subject: [PATCH 05/19] Fix Flake8 issues --- src/tests/conftest.py | 3 ++- src/tests/test_deps.py | 4 ++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/tests/conftest.py b/src/tests/conftest.py index 79a9ff64..fb2ef71b 100644 --- a/src/tests/conftest.py +++ b/src/tests/conftest.py @@ -14,6 +14,7 @@ async def mock_current_user(): async def mock_current_device(): return DeviceOut(id=99, owner_id=1, specs="raspberry", name="connected_device", created_at=dt.now()) + @pytest.fixture(scope="module") def test_app(): @@ -23,4 +24,4 @@ def test_app(): client = TestClient(app) yield client # testing happens here - app.dependency_overrides = {} \ No newline at end of file + app.dependency_overrides = {} diff --git a/src/tests/test_deps.py b/src/tests/test_deps.py index 194e09d2..53fde64e 100644 --- a/src/tests/test_deps.py +++ b/src/tests/test_deps.py @@ -7,10 +7,10 @@ @pytest.mark.asyncio -async def testGetCurrentUser(test_app, monkeypatch): +async def testGetCurrentUser(test_app, monkeypatch): test_user_data = [ - {"username": "JohnDoe", "id": 1, "access_id":1}, + {"username": "JohnDoe", "id": 1, "access_id": 1}, ] async def mock_fetch_one(table, query_filters): From 103c84a74104e489f126cf70aa34d1df2b605684 Mon Sep 17 00:00:00 2001 From: martin1tab Date: Mon, 16 Nov 2020 19:10:03 +0100 Subject: [PATCH 06/19] Add test for getCurrentUser --- src/tests/conftest.py | 3 +-- src/tests/test_deps.py | 13 ++++++++++++- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/src/tests/conftest.py b/src/tests/conftest.py index c5fc054b..c10cf170 100644 --- a/src/tests/conftest.py +++ b/src/tests/conftest.py @@ -14,7 +14,6 @@ async def mock_current_user(): async def mock_current_device(): return DeviceOut(id=99, owner_id=1, specs="raspberry", name="connected_device", created_at=dt.now()) - @pytest.fixture(scope="module") def test_app(): @@ -24,4 +23,4 @@ def test_app(): client = TestClient(app) yield client # testing happens here - app.dependency_overrides = {} \ No newline at end of file + app.dependency_overrides = {} diff --git a/src/tests/test_deps.py b/src/tests/test_deps.py index b7951b15..e2348b41 100644 --- a/src/tests/test_deps.py +++ b/src/tests/test_deps.py @@ -5,9 +5,14 @@ from app.api.deps import get_current_access, get_current_user from app.api.schemas import AccessRead, UserRead +<<<<<<< HEAD @pytest.mark.asyncio async def testGetCurrentUser(test_app, monkeypatch): +======= +@pytest.mark.asyncio +async def testGetCurrentUser(test_app,monkeypatch): +>>>>>>> 3116e7d... Add test for getCurrentUser test_user_data = [ {"username": "JohnDoe", "id": 1, "access_id":1}, @@ -24,5 +29,11 @@ async def mock_fetch_one(table, query_filters): monkeypatch.setattr(crud, "fetch_one", mock_fetch_one) +<<<<<<< HEAD response = await get_current_user(AccessRead(id=1, login="JohnDoe", scopes="me")) - assert response == UserRead(**test_user_data[0]) \ No newline at end of file + assert response == UserRead(**test_user_data[0]) +======= + response = await get_current_user(AccessRead(id=1, login= "JohnDoe", scopes="me")) + assert response == UserRead(**test_user_data[0]) + +>>>>>>> 3116e7d... Add test for getCurrentUser From 08f55543336bb0b9d5d441472e9f2b0ce2b78b28 Mon Sep 17 00:00:00 2001 From: martin1tab Date: Mon, 16 Nov 2020 19:32:57 +0100 Subject: [PATCH 07/19] Fix some issues --- src/tests/test_deps.py | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/src/tests/test_deps.py b/src/tests/test_deps.py index e2348b41..194e09d2 100644 --- a/src/tests/test_deps.py +++ b/src/tests/test_deps.py @@ -5,14 +5,9 @@ from app.api.deps import get_current_access, get_current_user from app.api.schemas import AccessRead, UserRead -<<<<<<< HEAD @pytest.mark.asyncio async def testGetCurrentUser(test_app, monkeypatch): -======= -@pytest.mark.asyncio -async def testGetCurrentUser(test_app,monkeypatch): ->>>>>>> 3116e7d... Add test for getCurrentUser test_user_data = [ {"username": "JohnDoe", "id": 1, "access_id":1}, @@ -29,11 +24,5 @@ async def mock_fetch_one(table, query_filters): monkeypatch.setattr(crud, "fetch_one", mock_fetch_one) -<<<<<<< HEAD response = await get_current_user(AccessRead(id=1, login="JohnDoe", scopes="me")) assert response == UserRead(**test_user_data[0]) -======= - response = await get_current_user(AccessRead(id=1, login= "JohnDoe", scopes="me")) - assert response == UserRead(**test_user_data[0]) - ->>>>>>> 3116e7d... Add test for getCurrentUser From 0ec8dbd2e528f421bf926bae57fd2848748112b3 Mon Sep 17 00:00:00 2001 From: martin1tab Date: Mon, 16 Nov 2020 19:38:37 +0100 Subject: [PATCH 08/19] Fix Flake8 issues --- src/tests/conftest.py | 1 + src/tests/test_deps.py | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/tests/conftest.py b/src/tests/conftest.py index c10cf170..fb2ef71b 100644 --- a/src/tests/conftest.py +++ b/src/tests/conftest.py @@ -14,6 +14,7 @@ async def mock_current_user(): async def mock_current_device(): return DeviceOut(id=99, owner_id=1, specs="raspberry", name="connected_device", created_at=dt.now()) + @pytest.fixture(scope="module") def test_app(): diff --git a/src/tests/test_deps.py b/src/tests/test_deps.py index 194e09d2..53fde64e 100644 --- a/src/tests/test_deps.py +++ b/src/tests/test_deps.py @@ -7,10 +7,10 @@ @pytest.mark.asyncio -async def testGetCurrentUser(test_app, monkeypatch): +async def testGetCurrentUser(test_app, monkeypatch): test_user_data = [ - {"username": "JohnDoe", "id": 1, "access_id":1}, + {"username": "JohnDoe", "id": 1, "access_id": 1}, ] async def mock_fetch_one(table, query_filters): From eaa3a6d2f52bb7479a119ba602d518fdb3d73624 Mon Sep 17 00:00:00 2001 From: martin1tab Date: Mon, 16 Nov 2020 20:15:59 +0100 Subject: [PATCH 09/19] Fix static code analysis issues --- src/app/api/crud.py | 2 +- src/app/api/routing.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/app/api/crud.py b/src/app/api/crud.py index 5d37d680..e9fb29bf 100644 --- a/src/app/api/crud.py +++ b/src/app/api/crud.py @@ -1,4 +1,4 @@ -from typing import Optional, Any, List, Dict +from typing import Optional, Any, Dict from app.db import database from sqlalchemy import Table from pydantic import BaseModel diff --git a/src/app/api/routing.py b/src/app/api/routing.py index afdc0772..12d5a577 100644 --- a/src/app/api/routing.py +++ b/src/app/api/routing.py @@ -1,6 +1,6 @@ from sqlalchemy import Table from pydantic import BaseModel -from typing import Optional, Any, List, Dict +from typing import Optional, Any, Dict from fastapi import HTTPException, Path from datetime import datetime from app.api import crud, security From 4e5500dd18b496cd9205b7ccf7e8f3bf805874f4 Mon Sep 17 00:00:00 2001 From: martin1tab Date: Mon, 16 Nov 2020 20:24:50 +0100 Subject: [PATCH 10/19] Fix static code checks issues --- src/tests/test_deps.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/tests/test_deps.py b/src/tests/test_deps.py index 53fde64e..91d5a42f 100644 --- a/src/tests/test_deps.py +++ b/src/tests/test_deps.py @@ -1,8 +1,7 @@ -import json import pytest from app.api import crud -from app.api.deps import get_current_access, get_current_user +from app.api.deps import get_current_user from app.api.schemas import AccessRead, UserRead From 1a8c773a06b396067e57f0454304ffe98a8dc707 Mon Sep 17 00:00:00 2001 From: martin1tab Date: Mon, 16 Nov 2020 20:30:35 +0100 Subject: [PATCH 11/19] Some more static checks errors --- src/tests/conftest.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tests/conftest.py b/src/tests/conftest.py index fb2ef71b..01d09b6e 100644 --- a/src/tests/conftest.py +++ b/src/tests/conftest.py @@ -4,7 +4,7 @@ from app.main import app from app.api.schemas import UserRead, DeviceOut -from app.api.deps import get_current_user, get_current_device, get_current_access +from app.api.deps import get_current_user, get_current_device async def mock_current_user(): From 761fa29e51f09bb876ecd3eb6d44802561c50913 Mon Sep 17 00:00:00 2001 From: martin1tab Date: Mon, 16 Nov 2020 20:42:44 +0100 Subject: [PATCH 12/19] Add test get_current_device --- src/tests/test_deps.py | 36 ++++++++++++++++++++++++++++++++++-- 1 file changed, 34 insertions(+), 2 deletions(-) diff --git a/src/tests/test_deps.py b/src/tests/test_deps.py index 91d5a42f..942d2089 100644 --- a/src/tests/test_deps.py +++ b/src/tests/test_deps.py @@ -1,8 +1,8 @@ import pytest from app.api import crud -from app.api.deps import get_current_user -from app.api.schemas import AccessRead, UserRead +from app.api.deps import get_current_user, get_current_device +from app.api.schemas import AccessRead, UserRead, DeviceOut @pytest.mark.asyncio @@ -25,3 +25,35 @@ async def mock_fetch_one(table, query_filters): response = await get_current_user(AccessRead(id=1, login="JohnDoe", scopes="me")) assert response == UserRead(**test_user_data[0]) + + +@pytest.mark.asyncio +async def testGetCurrentDevice(test_app, monkeypatch): + + MIN_PAYLOAD = {"name": "my_device", "owner_id": 1, "specs": "my_specs", "password": "my_password"} + FULL_PAYLOAD = { + **MIN_PAYLOAD, + "lat": None, + "lon": None, + "elevation": None, + "yaw": None, + "pitch": None, + "last_ping": None, + } + test_device_data = [ + {"id": 1, **FULL_PAYLOAD, "access_id": 1} + ] + + async def mock_fetch_one(table, query_filters): + for entry in test_device_data: + for queryFilterKey, queryFilterValue in query_filters.items(): + valid_entry = True + if entry[queryFilterKey] != queryFilterValue: + valid_entry = False + if valid_entry: + return entry + + monkeypatch.setattr(crud, "fetch_one", mock_fetch_one) + + response = await get_current_device(AccessRead(id=1, login="JohnDoe", scopes="me")) + assert response == DeviceOut(**test_device_data[0]) From d9cf2611aae8064a379032330251aea1f80f5795 Mon Sep 17 00:00:00 2001 From: martin1tab Date: Wed, 18 Nov 2020 17:39:04 +0100 Subject: [PATCH 13/19] Fix merge conflicts --- src/app/api/crud.py | 8 ++++---- src/app/api/deps.py | 3 ++- src/app/api/routes/devices.py | 4 ++-- src/tests/conftest.py | 2 +- 4 files changed, 9 insertions(+), 8 deletions(-) diff --git a/src/app/api/crud.py b/src/app/api/crud.py index 09e555ec..3dba9218 100644 --- a/src/app/api/crud.py +++ b/src/app/api/crud.py @@ -18,15 +18,15 @@ async def get(entry_id: int, table: Table) -> Dict[str, Any]: async def fetch_all(table: Table, query_filters: Optional[Dict[str, Any]] = None): query = table.select() if isinstance(query_filters, dict): - for queryFilterKey, queryFilterValue in query_filters: - query = query.where(getattr(table.c, queryFilterKey) == queryFilterValue) + for query_filter_key, query_filter_value in query_filters.items(): + query = query.where(getattr(table.c, query_filter_key) == query_filter_value) return await database.fetch_all(query=query) async def fetch_one(table: Table, query_filters: Dict[str, Any]): query = table.select() - for queryFilterKey, queryFilterValue in query_filters: - query = query.where(getattr(table.c, queryFilterKey) == queryFilterValue) + for query_filter_key, query_filter_value in query_filters.items(): + query = query.where(getattr(table.c, query_filter_key) == query_filter_value) return await database.fetch_one(query=query) diff --git a/src/app/api/deps.py b/src/app/api/deps.py index b7e9327c..0b43df98 100644 --- a/src/app/api/deps.py +++ b/src/app/api/deps.py @@ -29,12 +29,13 @@ def unauthorized_exception(detail: str, authenticate_value: str) -> HTTPExceptio async def get_current_access(security_scopes: SecurityScopes, token: str = Depends(reusable_oauth2)) -> AccessRead: - """Dependency to use as fastapi.security.Security with scopes. + """ Dependency to use as fastapi.security.Security with scopes. >>> @app.get("/users/me") >>> async def read_users_me(current_user: User = Security(get_current_access, scopes=["me"])): >>> return current_user """ + if security_scopes.scopes: authenticate_value = f'Bearer scope="{security_scopes.scope_str}"' diff --git a/src/app/api/routes/devices.py b/src/app/api/routes/devices.py index 5fc9eed3..9ff5ccfe 100644 --- a/src/app/api/routes/devices.py +++ b/src/app/api/routes/devices.py @@ -40,8 +40,8 @@ async def delete_device(device_id: int = Path(..., gt=0), _=Security(get_current @router.get("/my-devices", response_model=List[DeviceOut]) async def fetch_my_devices(me: UserRead = Security(get_current_user, scopes=["me"])): - return await crud.fetch_all(devices, {"owner_id", me.id}) - + return await crud.fetch_all(devices, {"owner_id": me.id}) + @router.put("/heartbeat", response_model=DeviceOut) async def heartbeat(device: DeviceOut = Security(get_current_device, scopes=["device"])): diff --git a/src/tests/conftest.py b/src/tests/conftest.py index 91ad6b9f..6eadbddd 100644 --- a/src/tests/conftest.py +++ b/src/tests/conftest.py @@ -28,7 +28,7 @@ async def mock_fetch_all(table, query_filters=None): return table response = [] for entry in table: - if all(entry[k] == v for k, v in query_filters): + if all(entry[k] == v for k, v in query_filters.items()): response.append(entry) return response From 4922ee0a58b2a12664227470099ee488aed385ef Mon Sep 17 00:00:00 2001 From: martin1tab Date: Wed, 18 Nov 2020 17:40:49 +0100 Subject: [PATCH 14/19] Fix Flake8 issues --- src/app/api/deps.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/app/api/deps.py b/src/app/api/deps.py index 0b43df98..b46923cd 100644 --- a/src/app/api/deps.py +++ b/src/app/api/deps.py @@ -35,7 +35,6 @@ async def get_current_access(security_scopes: SecurityScopes, token: str = Depen >>> async def read_users_me(current_user: User = Security(get_current_access, scopes=["me"])): >>> return current_user """ - if security_scopes.scopes: authenticate_value = f'Bearer scope="{security_scopes.scope_str}"' From 0d1f4697a6dc4700d2af766459909f2542e65020 Mon Sep 17 00:00:00 2001 From: martin1tab Date: Wed, 18 Nov 2020 17:43:48 +0100 Subject: [PATCH 15/19] Fix merge conflicts --- src/app/api/routing.py | 112 ----------------------------------------- 1 file changed, 112 deletions(-) delete mode 100644 src/app/api/routing.py diff --git a/src/app/api/routing.py b/src/app/api/routing.py deleted file mode 100644 index 12d5a577..00000000 --- a/src/app/api/routing.py +++ /dev/null @@ -1,112 +0,0 @@ -from sqlalchemy import Table -from pydantic import BaseModel -from typing import Optional, Any, Dict -from fastapi import HTTPException, Path -from datetime import datetime -from app.api import crud, security -from app.api.schemas import (UserAuth, UserCreation, UserRead, CredHash, Cred, - AccessCreation, AccessRead, AccessAuth, - DeviceAuth, DeviceCreation, DeviceOut, - HeartbeatOut, DefaultPosition) -from app.db import accesses as access_table - - -async def create_entry(table: Table, payload: BaseModel): - entry_id = await crud.post(payload, table) - return {**payload.dict(), "id": entry_id} - - -async def get_entry(table: Table, entry_id: int = Path(..., gt=0)): - entry = await crud.get(entry_id, table) - if not entry: - raise HTTPException(status_code=404, detail="Entry not found") - return entry - - -async def fetch_entries(table: Table, query_filter: Optional[Dict[str, Any]] = None): - return await crud.fetch_all(table, query_filter) - - -async def fetch_entry(table: Table, query_filter: Dict[str, Any]): - return await crud.fetch_one(table, query_filter) - - -async def update_entry(table: Table, payload: BaseModel, entry_id: int = Path(..., gt=0)): - await get_entry(table, entry_id) - entry_id = await crud.put(entry_id, payload, table) - - return {**payload.dict(), "id": entry_id} - - -async def delete_entry(table: Table, entry_id: int = Path(..., gt=0)): - entry = await get_entry(table, entry_id) - await crud.delete(entry_id, table) - - return entry - - -async def _create_access(login: str, password: str, scopes: str) -> AccessRead: - # Check that username does not already exist - if await fetch_entry(access_table, [('login', login)]) is not None: - raise HTTPException( - status_code=400, - detail=f"An entry with login='{login}' already exists.", - ) - # Hash the password - pwd = await security.hash_password(password) - access = AccessCreation(login=login, hashed_password=pwd, scopes=scopes) - access_entry = AccessRead(** await create_entry(access_table, access)) - return access_entry - - -async def create_access(payload: AccessAuth) -> AccessRead: - return await _create_access(payload.login, payload.password, scopes=payload.scopes) - - -async def create_user(user_table: Table, payload: UserAuth) -> UserRead: - access_entry = await _create_access(payload.username, payload.password, scopes=payload.scopes) - user = UserCreation(username=payload.username, access_id=access_entry.id) - return await create_entry(user_table, user) - - -async def create_device(device_table: Table, payload: DeviceAuth) -> DeviceOut: - access_entry = await _create_access(payload.name, payload.password, scopes=payload.scopes) - payload = DeviceCreation(**payload.dict(), access_id=access_entry.id) - return await create_entry(device_table, payload) - - -async def update_access_pwd(payload: Cred, entry_id: int = Path(..., gt=0)): - entry = await get_entry(access_table, entry_id) - # Hash the password - pwd = await security.hash_password(payload.password) - # Update the password - payload = CredHash(hashed_password=pwd) - await crud.put(entry_id, payload, access_table) - # Return non-sensitive information - return {"login": entry["login"]} - - -async def update_pwd(table: Table, payload: Cred, entry_id: int = Path(..., gt=0)): - entry = await get_entry(table, entry_id) - await update_access_pwd(payload, entry["access_id"]) - return entry - - -async def heartbeat(device_table: Table, device: DeviceOut) -> HeartbeatOut: - device.last_ping = datetime.utcnow() - await update_entry(device_table, device, device.id) - return device - - -async def update_location(device_table: Table, payload: DefaultPosition, device_id: int, user_id: int): - user_owns_device = bool(await fetch_entry(device_table, [("id", device_id), ("owner_id", user_id)])) - if not user_owns_device: - raise HTTPException( - status_code=400, - detail="Permission denied" - ) - device = (await get_entry(device_id, device_table)) - device.update(payload.dict()) - device = DeviceOut(**device) - await update_entry(device_table, device, device.id) - return device From 031fd0959dd1296b62c8e63d1c28b86466df642b Mon Sep 17 00:00:00 2001 From: martin1tab Date: Wed, 18 Nov 2020 18:24:22 +0100 Subject: [PATCH 16/19] Fix Merge conflicts --- src/tests/test_deps.py | 49 ++++++++++++++++++++---------------------- 1 file changed, 23 insertions(+), 26 deletions(-) diff --git a/src/tests/test_deps.py b/src/tests/test_deps.py index 942d2089..5d5cc1d8 100644 --- a/src/tests/test_deps.py +++ b/src/tests/test_deps.py @@ -4,19 +4,30 @@ from app.api.deps import get_current_user, get_current_device from app.api.schemas import AccessRead, UserRead, DeviceOut +USER_TABLE = [ + {"id": 1, "login": "first_user", "access_id": 1, "created_at": "2020-10-13T08:18:45.447773"}, + {"id": 99, "login": "connected_user", "access_id": 2, "created_at": "2020-11-13T08:18:45.447773"}, +] + +DEVICE_TABLE = [ + {"id": 1, "login": "first_device", "owner_id": 1, "access_id": 1, "specs": "v0.1", "elevation": None, "lat": None, + "lon": None, "yaw": None, "pitch": None, "last_ping": None, "created_at": "2020-10-13T08:18:45.447773"}, + {"id": 2, "login": "second_device", "owner_id": 99, "access_id": 2, "specs": "v0.1", "elevation": None, "lat": None, + "lon": None, "yaw": None, "pitch": None, "last_ping": None, "created_at": "2020-10-13T08:18:45.447773"}, + {"id": 99, "login": "connected_device", "owner_id": 1, "access_id": 3, "specs": "raspberry", "elevation": None, + "lat": None, "lon": None, "yaw": None, "pitch": None, "last_ping": None, + "created_at": "2020-10-13T08:18:45.447773"}, +] + @pytest.mark.asyncio async def testGetCurrentUser(test_app, monkeypatch): - test_user_data = [ - {"username": "JohnDoe", "id": 1, "access_id": 1}, - ] - async def mock_fetch_one(table, query_filters): - for entry in test_user_data: - for queryFilterKey, queryFilterValue in query_filters.items(): + for entry in USER_TABLE: + for query_filter_key, query_filter_value in query_filters.items(): valid_entry = True - if entry[queryFilterKey] != queryFilterValue: + if entry[query_filter_key] != query_filter_value: valid_entry = False if valid_entry: return entry @@ -24,31 +35,17 @@ async def mock_fetch_one(table, query_filters): monkeypatch.setattr(crud, "fetch_one", mock_fetch_one) response = await get_current_user(AccessRead(id=1, login="JohnDoe", scopes="me")) - assert response == UserRead(**test_user_data[0]) + assert response == UserRead(**USER_TABLE[0]) @pytest.mark.asyncio async def testGetCurrentDevice(test_app, monkeypatch): - MIN_PAYLOAD = {"name": "my_device", "owner_id": 1, "specs": "my_specs", "password": "my_password"} - FULL_PAYLOAD = { - **MIN_PAYLOAD, - "lat": None, - "lon": None, - "elevation": None, - "yaw": None, - "pitch": None, - "last_ping": None, - } - test_device_data = [ - {"id": 1, **FULL_PAYLOAD, "access_id": 1} - ] - async def mock_fetch_one(table, query_filters): - for entry in test_device_data: - for queryFilterKey, queryFilterValue in query_filters.items(): + for entry in DEVICE_TABLE: + for query_filter_key, query_filter_value in query_filters.items(): valid_entry = True - if entry[queryFilterKey] != queryFilterValue: + if entry[query_filter_key] != query_filter_value: valid_entry = False if valid_entry: return entry @@ -56,4 +53,4 @@ async def mock_fetch_one(table, query_filters): monkeypatch.setattr(crud, "fetch_one", mock_fetch_one) response = await get_current_device(AccessRead(id=1, login="JohnDoe", scopes="me")) - assert response == DeviceOut(**test_device_data[0]) + assert response == DeviceOut(**DEVICE_TABLE[0]) From 1da7ccbe005d969e4ee32557bc71c7e5c771d7e1 Mon Sep 17 00:00:00 2001 From: martin1tab Date: Wed, 18 Nov 2020 18:37:48 +0100 Subject: [PATCH 17/19] Handle comments --- src/app/api/deps.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/app/api/deps.py b/src/app/api/deps.py index b46923cd..f69e31c0 100644 --- a/src/app/api/deps.py +++ b/src/app/api/deps.py @@ -62,7 +62,7 @@ async def get_current_access(security_scopes: SecurityScopes, token: str = Depen return AccessRead(**entry) -async def get_current_user(access=Depends(get_current_access)): +async def get_current_user(access: AccessRead = Depends(get_current_access)) -> UserRead: user = await crud.fetch_one(users, {'access_id': access.id}) if user is None: raise HTTPException(status_code=400, detail="Permission denied") @@ -70,7 +70,7 @@ async def get_current_user(access=Depends(get_current_access)): return UserRead(**user) -async def get_current_device(access=Depends(get_current_access)): +async def get_current_device(access: AccessRead = Depends(get_current_access)) -> DeviceOut: device = await crud.fetch_one(devices, {'access_id': access.id}) if device is None: raise HTTPException(status_code=400, detail="Permission denied") From 4f831b8626e0c7b76e8722ec7d63dd802aab08f7 Mon Sep 17 00:00:00 2001 From: martin1tab Date: Fri, 20 Nov 2020 14:02:47 +0100 Subject: [PATCH 18/19] Refactor tests_deps --- src/app/api/routes/accesses.py | 2 +- src/app/api/routes/devices.py | 2 +- src/app/api/routes/login.py | 2 +- src/tests/conftest.py | 2 +- src/tests/test_deps.py | 50 +++++++++++++++------------------- 5 files changed, 26 insertions(+), 32 deletions(-) diff --git a/src/app/api/routes/accesses.py b/src/app/api/routes/accesses.py index d554a51d..cc463e6d 100644 --- a/src/app/api/routes/accesses.py +++ b/src/app/api/routes/accesses.py @@ -10,7 +10,7 @@ async def post_access(login: str, password: str, scopes: str) -> AccessRead: # Check that the login does not already exist - if await crud.fetch_one(accesses, [('login', login)]) is not None: + if await crud.fetch_one(accesses, {'login': login}) is not None: raise HTTPException( status_code=400, detail=f"An entry with login='{login}' already exists.", diff --git a/src/app/api/routes/devices.py b/src/app/api/routes/devices.py index 305904a8..729a0c43 100644 --- a/src/app/api/routes/devices.py +++ b/src/app/api/routes/devices.py @@ -57,7 +57,7 @@ async def update_device_location( user: UserRead = Security(get_current_user, scopes=["me"]) ): # Check that device is accessible to this user - entry = await crud.fetch_one(devices, [("id", device_id), ("owner_id", user.id)]) + entry = await crud.fetch_one(devices, {"id": device_id, "owner_id": user.id}) if entry is None: raise HTTPException( status_code=400, diff --git a/src/app/api/routes/login.py b/src/app/api/routes/login.py index cd0d170c..4a150a61 100644 --- a/src/app/api/routes/login.py +++ b/src/app/api/routes/login.py @@ -13,7 +13,7 @@ @router.post("/access-token", response_model=Token) async def create_access_token(form_data: OAuth2PasswordRequestForm = Depends()): # Verify credentials - entry = await crud.fetch_one(accesses, [('login', form_data.username)]) + entry = await crud.fetch_one(accesses, {'login': form_data.username}) if entry is None or not await security.verify_password(form_data.password, entry['hashed_password']): raise HTTPException(status_code=400, detail="Invalid credentials") # create access token using user user_id/user_scopes diff --git a/src/tests/conftest.py b/src/tests/conftest.py index add3821d..e96b77d1 100644 --- a/src/tests/conftest.py +++ b/src/tests/conftest.py @@ -35,7 +35,7 @@ async def mock_fetch_all(table, query_filters=None): async def mock_fetch_one(table, query_filters=None): for entry in table: - if all(entry[k] == v for k, v in query_filters): + if all(entry[k] == v for k, v in query_filters.items()): return entry return None diff --git a/src/tests/test_deps.py b/src/tests/test_deps.py index 5d5cc1d8..3ffd86a4 100644 --- a/src/tests/test_deps.py +++ b/src/tests/test_deps.py @@ -1,8 +1,8 @@ import pytest -from app.api import crud -from app.api.deps import get_current_user, get_current_device +from app.api import crud, deps from app.api.schemas import AccessRead, UserRead, DeviceOut +from copy import deepcopy USER_TABLE = [ {"id": 1, "login": "first_user", "access_id": 1, "created_at": "2020-10-13T08:18:45.447773"}, @@ -20,37 +20,31 @@ ] -@pytest.mark.asyncio -async def testGetCurrentUser(test_app, monkeypatch): +def _patch_session(monkeypatch, mock_user_table=None, mock_device_table=None): + # DB patching + if mock_user_table is not None: + monkeypatch.setattr(deps, "users", mock_user_table) + if mock_device_table is not None: + monkeypatch.setattr(deps, "devices", mock_device_table) + # Sterilize all DB interactions through CRUD override + monkeypatch.setattr(crud, "fetch_one", pytest.mock_fetch_one) + - async def mock_fetch_one(table, query_filters): - for entry in USER_TABLE: - for query_filter_key, query_filter_value in query_filters.items(): - valid_entry = True - if entry[query_filter_key] != query_filter_value: - valid_entry = False - if valid_entry: - return entry +@pytest.mark.asyncio +async def test_get_current_user(test_app, monkeypatch): - monkeypatch.setattr(crud, "fetch_one", mock_fetch_one) + mock_user_table = deepcopy(USER_TABLE) + _patch_session(monkeypatch, mock_user_table, None) - response = await get_current_user(AccessRead(id=1, login="JohnDoe", scopes="me")) - assert response == UserRead(**USER_TABLE[0]) + response = await deps.get_current_user(AccessRead(id=1, login="JohnDoe", scopes="me")) + assert response == UserRead(**mock_user_table[0]) @pytest.mark.asyncio -async def testGetCurrentDevice(test_app, monkeypatch): - - async def mock_fetch_one(table, query_filters): - for entry in DEVICE_TABLE: - for query_filter_key, query_filter_value in query_filters.items(): - valid_entry = True - if entry[query_filter_key] != query_filter_value: - valid_entry = False - if valid_entry: - return entry +async def test_get_current_device(test_app, monkeypatch): - monkeypatch.setattr(crud, "fetch_one", mock_fetch_one) + mock_device_table = deepcopy(DEVICE_TABLE) + _patch_session(monkeypatch, None, mock_device_table) - response = await get_current_device(AccessRead(id=1, login="JohnDoe", scopes="me")) - assert response == DeviceOut(**DEVICE_TABLE[0]) + response = await deps.get_current_device(AccessRead(id=1, login="JohnDoe", scopes="me")) + assert response == DeviceOut(**mock_device_table[0]) From 9192c4d69928fce8eaf115309b27168fd1a60dae Mon Sep 17 00:00:00 2001 From: martin1tab Date: Fri, 20 Nov 2020 14:09:51 +0100 Subject: [PATCH 19/19] add output type: ' --- src/app/api/crud.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/app/api/crud.py b/src/app/api/crud.py index 3dba9218..a143fced 100644 --- a/src/app/api/crud.py +++ b/src/app/api/crud.py @@ -15,7 +15,7 @@ async def get(entry_id: int, table: Table) -> Dict[str, Any]: return await database.fetch_one(query=query) -async def fetch_all(table: Table, query_filters: Optional[Dict[str, Any]] = None): +async def fetch_all(table: Table, query_filters: Optional[Dict[str, Any]] = None) -> List[Dict[str, Any]]: query = table.select() if isinstance(query_filters, dict): for query_filter_key, query_filter_value in query_filters.items(): @@ -23,7 +23,7 @@ async def fetch_all(table: Table, query_filters: Optional[Dict[str, Any]] = None return await database.fetch_all(query=query) -async def fetch_one(table: Table, query_filters: Dict[str, Any]): +async def fetch_one(table: Table, query_filters: Dict[str, Any]) -> Dict[str, Any]: query = table.select() for query_filter_key, query_filter_value in query_filters.items(): query = query.where(getattr(table.c, query_filter_key) == query_filter_value)