codemation/pydbantic

GET function does not work with Unique columns (core.py)

Closed this issue · 2 comments

Function of interest (core.py):

@classmethod
    async def get(
        cls: Type[T],
        *p_key_condition: Tuple[DataBaseModelCondition],
        backward_refs=True,
        **primary_key_input,
    ) -> T:
        if not p_key_condition:
            for k in primary_key_input:
                primary_key = cls.__metadata__.tables[cls.__name__]["primary_key"]
                if k != cls.__metadata__.tables[cls.__name__]["primary_key"]:
                    raise Exception(f"Expected primary key {primary_key}=<value>")
                p_key_condition = [getattr(cls, primary_key) == primary_key_input[k]]

        result = await cls.filter(*p_key_condition, backward_refs=backward_refs)
        return result[0] if result else None

Function is used with a code similar to the one below (Taken from easyauth package)

from pydbantic import DataBaseModel

class BaseUser(DataBaseModel):
.....

class Users(BaseUser):
.....

await Users.get(username=username)

So given a database again taken from easyauth package with small modifications:

CREATE TABLE IF NOT EXISTS "Users" (
        username VARCHAR(60) NOT NULL,
        account_type BLOB,
        password VARCHAR(60),
        email VARCHAR(60),
        full_name VARCHAR(60),
        PRIMARY KEY (username)

);
CREATE UNIQUE INDEX email_unique on users(email);

Users.get is only able to retrieve elements that have PRIMARY KEY, to ensure that it is unique. So in this case I have added UNIQUE INDEX on column email, which should also ensure that all emails will be unique in the same way as username. But function mentioned above only checks if searchable column has PRIMARY KEY.

From my short analysis, I could not find a way to include columns that have UNIQUE INDEX as cls.__metadata__.tables[cls.__name__] does not hold information about indices.

So my quick and dirty solution is to allow retrieval of elements using any column.

    @classmethod
    async def get(
        cls: Type[T],
        *p_key_condition: Tuple[DataBaseModelCondition],
        backward_refs=True,
        **primary_key_input,
    ) -> DataBaseModel:
        # if not p_key_condition:
        #     for k in p_key:
        #         primary_key = cls.__metadata__.tables[cls.__name__]['primary_key']
        #         if k != cls.__metadata__.tables[cls.__name__]['primary_key']:
        #             raise Exception(f"Expected primary key {primary_key}=<value>")
        #         p_key_condition = [getattr(cls, primary_key)  == p_key[k]]          

        # result = await cls.filter(*p_key_condition, backward_refs=backward_refs)
        # return result[0] if result else None

        if not p_key_condition:
            p_key_condition = []
            for k, v in p_key.items():
                print(cls.__metadata__.tables[cls.__name__])
                p_key_condition.append(getattr(cls, k) == v) 

        result = await cls.filter(*p_key_condition, backward_refs=backward_refs)

        return result[0] if result else None

Current implementation allows the use of DataBaseModelCondition. This would be the cleanest approach without requiring any internal changes.

user = await Users.get(DataBaseModelCondition(f"Email equals {email}", Users.email == email, None))

Although it feels weird to allow such string interpolation with f-string instead of using integrated DB mechanisms. (More info on f-string vulnerability arogozhnikov/python3_with_pleasure#3)

I would suggest to include Unique Index data in table metadata, and improve the function to filter columns that have either primary key or unique index. Also to expose Unique (by Index) in a similar way as PrimaryKey to use in applications. This could also be used to allow login using email in easyauth application.

Hey @IUChimes,

Did you encounter an issue with the following syntax:

user = await Users.get(Users.email == email)

Typically, you should not be explicitly creating DataBaseModelConditions, as these are generated by using operators with DataBaseModel attributes, or other DataBaseModelAttribute comparison methods:

# this syntax should create a DataBaseModelCondition
Users.email == email 

Good catch, I missed that part where I could pass it simply like that. Thank you.
This seems less of a problem now with this "workaround". I guess it would be nice to have it work the same way as the primary key column since it is unique, but it may cost too many hours for only philosophical improvements.

Feel free to close this, as I am satisfied with your provided solution.