From 499cfa002e90b5e652ba195da959245527cec3f8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?dni=20=E2=9A=A1?= Date: Tue, 30 Jul 2024 10:11:59 +0200 Subject: [PATCH] remove pyproject artifacts import conv fix postgres url fix migration fixup timestamp postgresql fixup more timestamp fix migrations fix migrations fix statement add psycopg2 for conv tests expiry timestamp flaku fix expiry more wait parse to int remove time from filters --- .github/actions/prepare/action.yml | 5 +- .github/workflows/ci.yml | 6 +- Makefile | 4 +- lnbits/core/crud.py | 58 +++++++++--------- lnbits/core/migrations.py | 97 ++++++++++++++---------------- lnbits/db.py | 30 +++++---- pyproject.toml | 2 - tests/api/test_api.py | 4 +- tests/conftest.py | 15 +++-- tests/unit/test_db.py | 4 +- tools/conv.py | 2 +- 11 files changed, 114 insertions(+), 113 deletions(-) diff --git a/.github/actions/prepare/action.yml b/.github/actions/prepare/action.yml index 7ebee9b04..02a35a15a 100644 --- a/.github/actions/prepare/action.yml +++ b/.github/actions/prepare/action.yml @@ -46,7 +46,10 @@ runs: - name: Install the project dependencies shell: bash - run: poetry install + run: | + poetry install + # needed for conv tests + poetry add psycopg2-binary - name: Use Node.js ${{ inputs.node-version }} if: ${{ (inputs.npm == 'true') }} diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index fe3c2c23a..bc48ed271 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -17,7 +17,7 @@ jobs: strategy: matrix: python-version: ["3.9", "3.10"] - db-url: ["", "postgres://lnbits:lnbits@0.0.0.0:5432/lnbits"] + db-url: ["", "postgresql+asyncpg://lnbits:lnbits@0.0.0.0:5432/lnbits"] uses: ./.github/workflows/tests.yml with: custom-pytest: "poetry run pytest tests/api" @@ -31,7 +31,7 @@ jobs: strategy: matrix: python-version: ["3.9", "3.10"] - db-url: ["", "postgres://lnbits:lnbits@0.0.0.0:5432/lnbits"] + db-url: ["", "postgresql+asyncpg://lnbits:lnbits@0.0.0.0:5432/lnbits"] uses: ./.github/workflows/tests.yml with: custom-pytest: "poetry run pytest tests/wallets" @@ -45,7 +45,7 @@ jobs: strategy: matrix: python-version: ["3.9", "3.10"] - db-url: ["", "postgres://lnbits:lnbits@0.0.0.0:5432/lnbits"] + db-url: ["", "postgresql+asyncpg://lnbits:lnbits@0.0.0.0:5432/lnbits"] uses: ./.github/workflows/tests.yml with: custom-pytest: "poetry run pytest tests/unit" diff --git a/Makefile b/Makefile index 1bcc0c9c2..7c2dc5645 100644 --- a/Makefile +++ b/Makefile @@ -74,10 +74,10 @@ test-migration: timeout 5s poetry run lnbits --host 0.0.0.0 --port 5002 || code=$?; if [[ $code -ne 124 && $code -ne 0 ]]; then exit $code; fi HOST=0.0.0.0 \ PORT=5002 \ - LNBITS_DATABASE_URL="postgres://lnbits:lnbits@localhost:5432/migration" \ + LNBITS_DATABASE_URL="postgresql+asyncpg://lnbits:lnbits@localhost:5432/migration" \ timeout 5s poetry run lnbits --host 0.0.0.0 --port 5002 || code=$?; if [[ $code -ne 124 && $code -ne 0 ]]; then exit $code; fi LNBITS_DATA_FOLDER="./tests/data" \ - LNBITS_DATABASE_URL="postgres://lnbits:lnbits@localhost:5432/migration" \ + LNBITS_DATABASE_URL="postgresql+asyncpg://lnbits:lnbits@localhost:5432/migration" \ poetry run python tools/conv.py migration: diff --git a/lnbits/core/crud.py b/lnbits/core/crud.py index 721f84bf0..643645f7f 100644 --- a/lnbits/core/crud.py +++ b/lnbits/core/crud.py @@ -53,10 +53,11 @@ async def create_account( user_id = user_id or uuid4().hex extra = json.dumps(dict(user_config)) if user_config else "{}" now = int(time()) + now_ph = db.timestamp_placeholder("now") await (conn or db).execute( - """ + f""" INSERT INTO accounts (id, username, pass, email, extra, created_at, updated_at) - VALUES (:user, :username, :password, :email, :extra, :now, :now) + VALUES (:user, :username, :password, :email, :extra, {now_ph}, {now_ph}) """, { "user": user_id, @@ -98,10 +99,11 @@ async def update_account( extra = user_config or user.config now = int(time()) + now_ph = db.timestamp_placeholder("now") await db.execute( - """ + f""" UPDATE accounts SET (username, email, extra, updated_at) = - (:username, :email, :extra, :now) + (:username, :email, :extra, {now_ph}) WHERE id = :user """, { @@ -181,13 +183,13 @@ async def delete_accounts_no_wallets( ) -> None: delta = int(time()) - time_delta await (conn or db).execute( - """ + f""" DELETE FROM accounts WHERE NOT EXISTS ( SELECT wallets.id FROM wallets WHERE wallets.user = accounts.id ) AND ( (updated_at is null AND created_at < :delta) - OR updated_at < :delta + OR updated_at < {db.timestamp_placeholder("delta")} ) """, {"delta": delta}, @@ -225,9 +227,10 @@ async def update_user_password(data: UpdateUserPassword) -> Optional[User]: pwd_context = CryptContext(schemes=["bcrypt"], deprecated="auto") now = int(time()) + now_ph = db.timestamp_placeholder("now") await db.execute( - """ - UPDATE accounts SET pass = :pass, updated_at = :now + f""" + UPDATE accounts SET pass = :pass, updated_at = {now_ph} WHERE id = :user """, { @@ -517,10 +520,11 @@ async def create_wallet( ) -> Wallet: wallet_id = uuid4().hex now = int(time()) + now_ph = db.timestamp_placeholder("now") await (conn or db).execute( - """ + f""" INSERT INTO wallets (id, name, "user", adminkey, inkey, created_at, updated_at) - VALUES (:wallet, :name, :user, :adminkey, :inkey, :now, :now) + VALUES (:wallet, :name, :user, :adminkey, :inkey, {now_ph}, {now_ph}) """, { "wallet": wallet_id, @@ -545,7 +549,7 @@ async def update_wallet( conn: Optional[Connection] = None, ) -> Optional[Wallet]: set_clause = [] - set_clause.append("updated_at = :now") + set_clause.append(f"updated_at = {db.timestamp_placeholder('now')}") values: dict = { "wallet": wallet_id, "now": int(time()), @@ -576,9 +580,9 @@ async def delete_wallet( ) -> None: now = int(time()) await (conn or db).execute( - """ + f""" UPDATE wallets - SET deleted = :deleted, updated_at = :now + SET deleted = :deleted, updated_at = {db.timestamp_placeholder('now')} WHERE id = :wallet AND "user" = :user """, {"wallet": wallet_id, "user": user_id, "deleted": deleted, "now": now}, @@ -599,9 +603,9 @@ async def delete_wallet_by_id( ) -> Optional[int]: now = int(time()) result = await (conn or db).execute( - """ + f""" UPDATE wallets - SET deleted = true, updated_at = :now + SET deleted = true, updated_at = {db.timestamp_placeholder('now')} WHERE id = :wallet """, {"wallet": wallet_id, "now": now}, @@ -771,7 +775,7 @@ async def get_payments_paginated( clause: List[str] = [] if since is not None: - clause.append("time > :time") + clause.append(f"time > {db.timestamp_placeholder('time')}") if wallet_id: clause.append("wallet = :wallet") @@ -858,7 +862,7 @@ async def delete_expired_invoices( f""" DELETE FROM apipayments WHERE status = '{PaymentState.PENDING}' AND amount > 0 - AND time < :delta + AND time < {db.timestamp_placeholder("delta")} """, {"delta": int(time() - 2592000)}, ) @@ -867,7 +871,7 @@ async def delete_expired_invoices( f""" DELETE FROM apipayments WHERE status = '{PaymentState.PENDING}' AND amount > 0 - AND expiry < :now + AND expiry < {db.timestamp_placeholder("now")} """, {"now": int(time())}, ) @@ -898,13 +902,14 @@ async def create_payment( previous_payment = await get_standalone_payment(checking_id, conn=conn) assert previous_payment is None, "Payment already exists" + expiry_ph = db.timestamp_placeholder("expiry") await (conn or db).execute( - """ + f""" INSERT INTO apipayments (wallet, checking_id, bolt11, hash, preimage, amount, status, memo, fee, extra, webhook, expiry, pending) VALUES (:wallet, :checking_id, :bolt11, :hash, :preimage, - :amount, :status, :memo, :fee, :extra, :webhook, :expiry, :pending) + :amount, :status, :memo, :fee, :extra, :webhook, {expiry_ph}, :pending) """, { "wallet": wallet_id, @@ -922,7 +927,7 @@ async def create_payment( else None ), "webhook": webhook, - "expiry": db.datetime_to_timestamp(expiry) if expiry else None, + "expiry": expiry if expiry else None, "pending": False, # TODO: remove this in next release }, ) @@ -1021,12 +1026,6 @@ async def get_payments_history( ) -> List[PaymentHistoryPoint]: if not filters: filters = Filters() - where = [f"(status = '{PaymentState.SUCCESS}' OR amount < 0)"] - values: dict = { - "wallet": wallet_id, - } - if wallet_id: - where.append("wallet = :wallet") if DB_TYPE == SQLITE and group in sqlite_formats: date_trunc = f"strftime('{sqlite_formats[group]}', time, 'unixepoch')" @@ -1041,11 +1040,12 @@ async def get_payments_history( SUM(CASE WHEN amount > 0 THEN amount ELSE 0 END) income, SUM(CASE WHEN amount < 0 THEN abs(amount) + abs(fee) ELSE 0 END) spending FROM apipayments - {filters.where(where)} + WHERE wallet = :wallet AND (status = '{PaymentState.SUCCESS}' OR amount < 0) GROUP BY date ORDER BY date DESC """, - filters.values(values), + {"wallet": wallet_id}, + # filters.values(values), ) if wallet_id: wallet = await get_wallet(wallet_id) diff --git a/lnbits/core/migrations.py b/lnbits/core/migrations.py index a357b9afe..d3ca75f74 100644 --- a/lnbits/core/migrations.py +++ b/lnbits/core/migrations.py @@ -1,4 +1,3 @@ -import datetime from time import time from loguru import logger @@ -102,7 +101,7 @@ async def m002_add_fields_to_apipayments(db): import json - rows = await (await db.execute("SELECT * FROM apipayments")).fetchall() + rows = await db.fetchall("SELECT * FROM apipayments") for row in rows: if not row["memo"] or not row["memo"].startswith("#"): continue @@ -113,15 +112,15 @@ async def m002_add_fields_to_apipayments(db): new = row["memo"][len(prefix) :] await db.execute( """ - UPDATE apipayments SET extra = ?, memo = ? - WHERE checking_id = ? AND memo = ? + UPDATE apipayments SET extra = :extra, memo = :memo1 + WHERE checking_id = :checking_id AND memo = :memo2 """, - ( - json.dumps({"tag": ext}), - new, - row["checking_id"], - row["memo"], - ), + { + "extra": json.dumps({"tag": ext}), + "memo1": new, + "checking_id": row["checking_id"], + "memo2": row["memo"], + }, ) break except OperationalError: @@ -212,19 +211,17 @@ async def m007_set_invoice_expiries(db): Precomputes invoice expiry for existing pending incoming payments. """ try: - rows = await ( - await db.execute( - f""" - SELECT bolt11, checking_id - FROM apipayments - WHERE pending = true - AND amount > 0 - AND bolt11 IS NOT NULL - AND expiry IS NULL - AND time < {db.timestamp_now} - """ - ) - ).fetchall() + rows = await db.fetchall( + f""" + SELECT bolt11, checking_id + FROM apipayments + WHERE pending = true + AND amount > 0 + AND bolt11 IS NOT NULL + AND expiry IS NULL + AND time < {db.timestamp_now} + """ + ) if len(rows): logger.info(f"Migration: Checking expiry of {len(rows)} invoices") for i, ( @@ -236,22 +233,17 @@ async def m007_set_invoice_expiries(db): if invoice.expiry is None: continue - expiration_date = datetime.datetime.fromtimestamp( - invoice.date + invoice.expiry - ) + expiration_date = invoice.date + invoice.expiry logger.info( f"Migration: {i+1}/{len(rows)} setting expiry of invoice" f" {invoice.payment_hash} to {expiration_date}" ) await db.execute( - """ - UPDATE apipayments SET expiry = ? - WHERE checking_id = ? AND amount > 0 + f""" + UPDATE apipayments SET expiry = {db.timestamp_placeholder('expiry')} + WHERE checking_id = :checking_id AND amount > 0 """, - ( - db.datetime_to_timestamp(expiration_date), - checking_id, - ), + {"expiry": expiration_date, "checking_id": checking_id}, ) except Exception: continue @@ -347,17 +339,15 @@ async def m014_set_deleted_wallets(db): Sets deleted column to wallets. """ try: - rows = await ( - await db.execute( - """ - SELECT * - FROM wallets - WHERE user LIKE 'del:%' - AND adminkey LIKE 'del:%' - AND inkey LIKE 'del:%' - """ - ) - ).fetchall() + rows = await db.fetchall( + """ + SELECT * + FROM wallets + WHERE user LIKE 'del:%' + AND adminkey LIKE 'del:%' + AND inkey LIKE 'del:%' + """ + ) for row in rows: try: @@ -367,10 +357,15 @@ async def m014_set_deleted_wallets(db): await db.execute( """ UPDATE wallets SET - "user" = ?, adminkey = ?, inkey = ?, deleted = true - WHERE id = ? + "user" = :user, adminkey = :adminkey, inkey = :inkey, deleted = true + WHERE id = :wallet """, - (user, adminkey, inkey, row[0]), + { + "user": user, + "adminkey": adminkey, + "inkey": inkey, + "wallet": row.get("id"), + }, ) except Exception: continue @@ -456,17 +451,17 @@ async def m017_add_timestamp_columns_to_accounts_and_wallets(db): now = int(time()) await db.execute( f""" - UPDATE wallets SET created_at = {db.timestamp_placeholder} + UPDATE wallets SET created_at = {db.timestamp_placeholder('now')} WHERE created_at IS NULL """, - (now,), + {"now": now}, ) await db.execute( f""" - UPDATE accounts SET created_at = {db.timestamp_placeholder} + UPDATE accounts SET created_at = {db.timestamp_placeholder('now')} WHERE created_at IS NULL """, - (now,), + {"now": now}, ) except OperationalError as exc: diff --git a/lnbits/db.py b/lnbits/db.py index 26cdb57b8..3bed014a9 100644 --- a/lnbits/db.py +++ b/lnbits/db.py @@ -40,13 +40,13 @@ else: DB_TYPE = SQLITE -def compat_timestamp_placeholder(): +def compat_timestamp_placeholder(key: str): if DB_TYPE == POSTGRES: - return "to_timestamp(?)" + return f"to_timestamp(:{key})" elif DB_TYPE == COCKROACH: - return "cast(? AS timestamp)" + return f"cast(:{key} AS timestamp)" else: - return "?" + return f":{key}" def get_placeholder(model: Any, field: str) -> str: @@ -111,9 +111,8 @@ class Compat: return "BIGINT" return "INT" - @property - def timestamp_placeholder(self) -> str: - return compat_timestamp_placeholder() + def timestamp_placeholder(self, key: str) -> str: + return compat_timestamp_placeholder(key) class Connection(Compat): @@ -257,7 +256,9 @@ class Database(Compat): f = "%Y-%m-%d %H:%M:%S.%f" if "." not in value: f = "%Y-%m-%d %H:%M:%S" - return time.mktime(datetime.datetime.strptime(value, f).timetuple()) + return int( + time.mktime(datetime.datetime.strptime(value, f).timetuple()) + ) dbapi_connection.run_async( lambda connection: connection.set_type_codec( @@ -420,7 +421,7 @@ class Filter(BaseModel, Generic[TFilterModel]): validated, errors = compare_field.validate(raw_value, {}, loc="none") if errors: raise ValidationError(errors=[errors], model=model) - values[field](validated) + values[field] = validated else: raise ValueError("Unknown filter field") @@ -428,9 +429,14 @@ class Filter(BaseModel, Generic[TFilterModel]): @property def statement(self): - if self.op in (Operator.INCLUDE, Operator.EXCLUDE): - placeholders = ", ".join([]) - stmt = [f"{self.field} {self.op.as_sql} ({placeholders})"] + if self.op in (Operator.INCLUDE, Operator.EXCLUDE) and self.values: + placeholders = [] + for key in self.values.keys(): + if self.model and self.model.__fields__[key].type_ == datetime.datetime: + placeholders.append(compat_timestamp_placeholder(self.field)) + else: + placeholders.append(f":{key}") + stmt = [f"{self.field} {self.op.as_sql} ({', '.join(placeholders)})"] else: stmt = [f"{self.field} {self.op.as_sql} :{self.field}"] return " OR ".join(stmt) diff --git a/pyproject.toml b/pyproject.toml index e58390ea6..2afc6036d 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -127,7 +127,6 @@ module = [ "secp256k1.*", "uvicorn.*", "sqlalchemy.*", - "sqlalchemy_aio.*", "websocket.*", "websockets.*", "pyqrcode.*", @@ -137,7 +136,6 @@ module = [ "bolt11.*", "bitstring.*", "ecdsa.*", - "psycopg2.*", "pyngrok.*", "pyln.client.*", "py_vapid.*", diff --git a/tests/api/test_api.py b/tests/api/test_api.py index 7c5b5a3ec..e4e6ced14 100644 --- a/tests/api/test_api.py +++ b/tests/api/test_api.py @@ -368,10 +368,10 @@ async def test_get_payments_history(client, adminkey_headers_from, fake_payments data = response.json() assert len(data) == 1 assert data[0]["spending"] == sum( - payment.amount * 1000 for payment in fake_data if payment.out + int(payment.amount * 1000) for payment in fake_data if payment.out ) assert data[0]["income"] == sum( - payment.amount * 1000 for payment in fake_data if not payment.out + int(payment.amount * 1000) for payment in fake_data if not payment.out ) response = await client.get( diff --git a/tests/conftest.py b/tests/conftest.py index b1955b917..070d03008 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -22,7 +22,7 @@ from lnbits.core.crud import ( from lnbits.core.models import CreateInvoice, PaymentState from lnbits.core.services import update_wallet_balance from lnbits.core.views.payment_api import api_payments_create_invoice -from lnbits.db import DB_TYPE, SQLITE, Database +from lnbits.db import Database from lnbits.settings import settings from tests.helpers import ( get_random_invoice_data, @@ -182,9 +182,8 @@ async def invoice(to_wallet): async def fake_payments(client, adminkey_headers_from): # Because sqlite only stores timestamps with milliseconds # we have to wait a second to ensure a different timestamp than previous invoices - if DB_TYPE == SQLITE: - await asyncio.sleep(1) - ts = time() + await asyncio.sleep(1) + ts = int(time()) fake_data = [ CreateInvoice(amount=10, memo="aaaa", out=False), @@ -197,9 +196,9 @@ async def fake_payments(client, adminkey_headers_from): "/api/v1/payments", headers=adminkey_headers_from, json=invoice.dict() ) assert response.is_success - await update_payment_status( - response.json()["checking_id"], status=PaymentState.SUCCESS - ) + data = response.json() + assert data["checking_id"] + await update_payment_status(data["checking_id"], status=PaymentState.SUCCESS) - params = {"time[ge]": ts, "time[le]": time()} + params = {"time[ge]": ts, "time[le]": int(time())} return fake_data, params diff --git a/tests/unit/test_db.py b/tests/unit/test_db.py index 97cebb22e..e0ffc1b36 100644 --- a/tests/unit/test_db.py +++ b/tests/unit/test_db.py @@ -14,8 +14,8 @@ from lnbits.db import POSTGRES @pytest.mark.asyncio async def test_date_conversion(db): if db.type == POSTGRES: - row = await db.fetchone("SELECT now()::date") - assert row and isinstance(row[0], date) + row = await db.fetchone("SELECT now()::date as now") + assert row and isinstance(row.get("now"), date) # make test to create wallet and delete wallet diff --git a/tools/conv.py b/tools/conv.py index ff18936ab..59cc7ac92 100644 --- a/tools/conv.py +++ b/tools/conv.py @@ -12,7 +12,7 @@ from typing import List, Optional from lnbits.settings import settings try: - import psycopg2 + import psycopg2 # type: ignore except ImportError: print("Please install psycopg2") sys.exit(1)