Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Using a String as primary key will override the value with item_id by insert statement #46

Closed
EspenAlbert opened this issue Nov 19, 2020 · 1 comment
Labels
bug Something isn't working

Comments

@EspenAlbert
Copy link
Contributor

EspenAlbert commented Nov 19, 2020

In the code sample below the test fails since the instance receives the item_id

import databases
import ormar
import sqlalchemy
from typing import Optional, List, Union
from ormar import String, Float, Boolean, ManyToMany, Integer, ForeignKey
from models_metadata.sql import MainMeta, DbModel



DATABASE_URL = "sqlite:///test.db"
database = databases.Database(DATABASE_URL)
metadata = sqlalchemy.MetaData()


class MainMeta(ormar.ModelMeta):
    metadata = metadata
    database = database


class DbModel(ormar.Model):
    pass


class PositionOrm(DbModel):
    class Meta(MainMeta):
        pass

    name: str = String(primary_key=True, max_length=50)
    x: float = Float()
    y: float = Float()
    degrees: float = Float()


@pytest.fixture(autouse=True, scope="module")
def create_test_database():
    engine = create_engine(DATABASE_URL)
    metadata.create_all(engine)
    yield
    metadata.drop_all(engine)


@pytest.fixture(scope="function")
async def cleanup():
    yield
    async with database:
        await PositionOrm.objects.delete(each=True)


@pytest.mark.asyncio
async def test_creating_a_position(cleanup):
    async with database:
        instance = PositionOrm(
            name="my_pos",
            x=1.0,
            y=2.0,
            degrees=3.0,
        )
        await instance.save()
        assert instance.saved
        assert instance.name == "my_pos"

Error message:

1 != my_pos

Expected :my_pos
Actual   :1

BTW: Awesome library. I am looking forward to start using it :)

@collerek
Copy link
Owner

Hi, thanks for catching that up - fix that in Model.object.create() but didn't in Model.save() before.

Should be fixed now -> please update to latest version (0.5.1) and give it a try.

Also please notice that in your example the DbModel class is redundant.

  • If there is no Meta class declared on an ormar model it's treated as normal pydantic model and in theory you can inherit from it, but ormar redeclares the fields so there is no much use for this
  • If you declare a Meta and not declare any fields on model - ormar will ad id = Integer(primary_key=True) field for you, as all models requires a pk, and this feature populates columns on through model in ManyToMany relations
  • For now ormar does not support model inheritance - so you cannot get multi-table models etc. For now each ormar.Model is a separate table in db.
  • Therefore all ormar models should inherit directly from ormar.Model and declare an internal Meta class with required params

So something like:

DATABASE_URL = "sqlite:///test.db"
database = databases.Database(DATABASE_URL)
metadata = sqlalchemy.MetaData()


class MainMeta(ormar.ModelMeta):
    metadata = metadata
    database = database

# not that db models should inherit from ormar.Model directly
class PositionOrm(ormar.Model):
    class Meta(MainMeta):
        pass

    name: str = String(primary_key=True, max_length=50)
    x: float = Float()
    y: float = Float()
    degrees: float = Float()


@pytest.fixture(autouse=True, scope="module")
def create_test_database():
    engine = create_engine(DATABASE_URL)
    metadata.create_all(engine)
    yield
    metadata.drop_all(engine)


@pytest.fixture(scope="function")
async def cleanup():
    yield
    async with database:
        await PositionOrm.objects.delete(each=True)


@pytest.mark.asyncio
async def test_creating_a_position(cleanup):
    async with database:
        instance = PositionOrm(
            name="my_pos",
            x=1.0,
            y=2.0,
            degrees=3.0,
        )
        await instance.save()
        assert instance.saved
        assert instance.name == "my_pos"

Now it should pass.

BTW. Thanks for the nice words! 😄 If you like the package consider starring it and spreading the news around - I am trying to get some traction and build a community around it to quicker catch bugs like this one (thanks again for catching!) and maybe also get some contributors for further development.

@collerek collerek added the bug Something isn't working label Nov 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants