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

Enhancement: DTOs: support fields defined in mixins and declared_attr fields while excluding implicit fields #163

Open
abdulhaq-e opened this issue Apr 24, 2024 · 2 comments
Labels
enhancement New feature or request

Comments

@abdulhaq-e
Copy link
Member

abdulhaq-e commented Apr 24, 2024

Summary

SQLAlchemyDTOConfig has a field named include_implicit_fields, it is either True, False or 'hybrid-only.

The first issue here is that there is another type of mapping that can be defined using declared_attr (see https://docs.sqlalchemy.org/en/20/orm/mapping_api.html#sqlalchemy.orm.declared_attr). Currently the only way to show this mapping is to set include_implicit_fields=True.

The use case is to enable showing declared_attr fields while still hiding implicitly mapped columns.

The second issue is fields defined in a mixin. They will also not appear unless include_implicit_fields=True

Basic Example

Here is a small example based o SQLAlchemy docs:

from sqlalchemy import ForeignKey, Table
from sqlalchemy.orm import declared_attr
from sqlalchemy.orm import DeclarativeBase
from sqlalchemy.orm import Mapped
from sqlalchemy.orm import mapped_column
from sqlalchemy.orm import relationship
from litestar.contrib.sqlalchemy.dto import SQLAlchemyDTO, SQLAlchemyDTOConfig

my_model_table = Table(
   "my_model_table",
    metadata,
    Column("id", Integer, primary_key=True),
    Column("log_record_id", Integer), # this is a foreign key
    Column("age", Integer),
    Column("grade", Integer),
    Column("name", String),
    Column("occupation", String),
)

class Base(DeclarativeBase):
    pass


class CommonMixin:
    """define a series of common elements that may be applied to mapped
    classes using this class as a mixin class."""

    @declared_attr.directive
    def __tablename__(cls) -> str:
        return cls.__name__.lower()

    __table_args__ = {"mysql_engine": "InnoDB"}
    __mapper_args__ = {"eager_defaults": True}

    id: Mapped[int] = mapped_column(primary_key=True)


class HasLogRecord:
    """mark classes that have a many-to-one relationship to the
    ``LogRecord`` class."""

    log_record_id: Mapped[int] = mapped_column(ForeignKey("logrecord.id"))

    @declared_attr
    def log_record(self) -> Mapped["LogRecord"]:
        return relationship("LogRecord")


class LogRecord(CommonMixin, Base):
    log_info: Mapped[str]


class MyModelView(CommonMixin, HasLogRecord, Base):
    __table__ = my_model_table

    name: Mapped[str]

# this dto will show all fields including the relationship defined in the mixin
class DTO1(SQLAlchemyDTO[V]):
    config = SQLAlchemyDTOConfig(
        include_implicit_fields=True)
    )

# this dto will only show the `name` field
class DTO2(SQLAlchemyDTO[V]):
    config = SQLAlchemyDTOConfig(
        include_implicit_fields=False)
    )

# this will behave like DTO2 since there isn't any hybrid field
class DTO3(SQLAlchemyDTO[V]):
    config = SQLAlchemyDTOConfig(
        include_implicit_fields='hybrid-only')
    )

Drawbacks and Impact

No response

Unresolved questions

No response


Funding

  • If you would like to see an issue prioritized, make a pledge towards it!
  • We receive the pledge once the issue is completed & verified
Fund with Polar
@abdulhaq-e abdulhaq-e added the enhancement New feature or request label Apr 24, 2024
@abdulhaq-e
Copy link
Member Author

I'm quiet familiar with that part of the code base so I will attempt a solution when I get sometime. Currently I was either forced to include implicit fields or stop using mixins. I stopped using mixins which isn't nice, too many repeated definitions.

@cofin cofin changed the title Enhancement: DTOs: support fields defined in mixins and declerad_attr fields while excluding implicit fields Enhancement: DTOs: support fields defined in mixins and declared_attr fields while excluding implicit fields May 3, 2024
@Seemone
Copy link

Seemone commented Jul 11, 2024

I'm not seeing declared_attrs being picked up by the DTO regardless of include_implicit_field.
by digging further in the issue (more details on discord: https://discord.com/channels/919193495116337154/1260856394937270282/1260856394937270282)
I think the root cause is sqlalchemy/sqlalchemy#10008

in short, DTOData code doesn't look in every place it should when it's gathering annotations to map the model.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants