#Unmarshalling of ORM associations

1 messages · Page 1 of 1 (latest)

cloud patio
#

Sort of following up from https://discord.com/channels/919193495116337154/1255535200318914660 I would like to reframe the issue in the subject.
Currently, the unmarshalling of classes derived from SQLAlchemy base class ignores non Mapped fields, resulting in incomplete data being passed to Controllers.
This is counterintuitive as the non Mapped fields can be used normally, outside of Litestar unmarshalling, and unmarshalling of, say, dataclasses happens for all the fields.
This is regardless of the type of the non mapped field.

The current behaviour impedes the usage of a single class for all the marshalling and unmarshalling in cases where the API exposes database entities but needs to treat certain data differently (like in relationship management, see previous topic linked above).

In my opinion this is not related to deserialization of relationships, which can still be not supported but could be managed by DTOs and proper code. This would actually enable to handle deserialization of relationships without having to resort to companion dataclasses and copy operations.

Thoughts?

crimson sandBOT
#
Notes for Unmarshalling of non ORM-backed fields
At your assistance

@cloud patio

No Response?

If no response in a reasonable time, ping @Member.

Closing

To close, type !solve or byte solve.

MCVE

Please include an MCVE so that we can reproduce your issue locally.

jade bough
#

Can you provide a very concise MCVE that illustrates the problem you have? I'm having a hard time following the ask now based on the previous assistance.

cloud patio
#

will do

cloud patio
#
curl -s -X POST  http://127.0.0.1:8000/entities -d@functionapi/app/mvce_unmarshalling_entity1.json 
{"name":"entity1","id":"8019da6c-2238-4dca-8b31-9db93ad3ee91"}

taglist never makes it to the @post method in line 170.
debug prints from the @post method:

Creating with name entity1
Creating with taglist []
#

relevant sections

class Entity(AsyncAttrs, UUIDAuditBase):
    __allow_unmapped__ = True
    name: Mapped[str] = mapped_column(String(191))
    taglist: List[str] = []
#
    @post(dto=EntityCreateDTO)
    async def create_entity(self, data: Entity, service: Service) -> Entity:
        print(f"Creating with name {data.name}")
        print(f"Creating with taglist {data.taglist}")
        return await service.create(data, data.taglist)
#

now of course I don't expect taglist to be parsed as part of the ORM, but it would be nice for it to make thru the unmarshalling and get to the @post method

jade bough
#

i'll be honest, this doesn't feel like a valid example

#

this should be declared some other way

#
taglist: List[str] = []
#

this feels more like a hybrid property or association proxy

#

Do you have a valid example where you've implemented logic for taglist?

cloud patio
#

I get what you mean. the valid example is the oither thread (implementing tags for entities) and in code everything works (supplying tags as list of string and converting them to proper tag object etc, like in fullstack). the thing that's lacking is how to get something that is not ORM mapped as an argument of a @post

#

or part of an argument

jade bough
#

sorry for being obtuse on this, but is this not what update_instance does? If you've added some custom method to the model, why (or how) can the DTO possible know how to merge that.

halcyon spindleBOT
#

advanced_alchemy/extensions/litestar/dto.py line 141

@handle_orm_descriptor.register(AssociationProxyExtensionType)
cloud patio
#

it's me not able to explain myself... the point is how I get the tag list from the client

jade bough
#

you want a list of strings to populate a many to many on tags

cloud patio
#

I can't get the list

#

that's my point

#

without using a dataclass model that I then need to convert to the ORM backed object

#

(which is what fullstack does)

jade bough
#

here's sqlalchemy

#
class User(Base):
    ...

    # use Keyword(keyword=kw) on append() events
    keywords: AssociationProxy[List[str]] = association_proxy(
        "kw", "keyword", creator=lambda kw: Keyword(keyword=kw)
    )
cloud patio
#

logic isn't the issue, but litestar "loses" the taglist element of the json

jade bough
#

it doesn't lose anything. You have to merge that into your instance with update_instance

cloud patio
#

no, it's not there in the incoming argument

#

in my application I have this

    tags: Mapped[List[Tag]] = relationship(secondary=lambda: tag_association_table, lazy="immediate")
    taglist: AssociationProxy[List[str]] = association_proxy("tags","name")

but any "taglist" in the JSON passed to the post POST doesn't get to the @post method

jade bough
#

Can you share a MCVE of this exact behavior?

#

the example you sent above doens't have this

cloud patio
#

of the unmarshalling behaviour?

#

it does

#

or you want a different model?

jade bough
#

it doesn't ahve any relationship

#

and there's only a single model

cloud patio
#

the other thread has that

#

exactly that

#

but

#

let me add the behavior to this mcve

#

and rename all the mvce to mcve 😄

jade bough
#

thank you. There are quite a bit of different threads that i work through. It's much easier to have it all in a single copy/paste-able format

cloud patio
#

well no, thanks to you!

#

output is still

Creating with name entity1
Creating with taglist []
jade bough
#

I think we need to take a step back. How does something like this work without a DTO? I think this is where there's confusion.

Let's start with a set of passing tests and add to it

#

Take this script. Both tests currently pass

#

add this test in

#
        product_3 = Product(name="Product 3", tags=["second tag"])
        db_session.add(product_3)
        tags = db_session.execute(select(Tag)).fetchall()
        assert len(tags) == 3
#

and let me know what you'd do to fix this?

#

the async equivalent is just below it

#

which may have additional hurdles

cloud patio
#

Lots to take in, job for tomorrow. Thanks

jade bough
#

np, let me know if you have questions on the tests. Feel free to edit the script inside and send back. They both run independently with no extras added

#

we'll add these as official tests if needed

cloud patio
#

I took a look at the code and while pytest reports a number of problems it doesn't cover the issue.
creating a new Entity with tags in the code works, my problem is that I can't get the taglist from the json in the request body.

looking at your code, I modified the Entity to specify more things about the relantioship and associationproxy

class Entity(AsyncAttrs, UUIDAuditBase):
    __allow_unmapped__ = True
    name: Mapped[str] = mapped_column(String(191))
    tags: Mapped[List[Tag]] = relationship(secondary=lambda: tag_association_table, lazy="noload", cascade="all, delete",
            passive_deletes=True)
    taglist: AssociationProxy[list[str]] = association_proxy("tags","name",creator=lambda name: Tag(name=name))

(I can't back populate tags as tags in my application are meant to be related to multiple other tables)

this changed nothing, data.taglist is still empty at the beginning of the EntityController.create_entity method

jade bough
#

I am not asking you to include anything Litestar related. Stay focused and solve the problem I’ve asked you.

#

Use the code I sent you.

#

Otherwise, close this issue.

#

I think I’ve been patient to try and get you to reason through this yourself, but you don’t seem to be willing to go through the discovery process I’m asking.

cloud patio
#

Apologies, I did not understand I was being put to test. Evidently there's a gap I don't seem to be able to grasp.
that said, if I run the original file with
pytest test.py
I get pytest errors

PytestUnknownMarkWarning: Unknown pytest.mark.xdist_group - is this a typo?  You can register custom marks to avoid this warning - for details, see https://docs.pytest.org/en/stable/how-to/mark.html

I also get another error about asyncio

PytestUnhandledCoroutineWarning: async def functions are not natively supported and have been skipped.
  You need to install a suitable plugin for your async framework, for example:
    - anyio
    - pytest-asyncio
    - pytest-tornasync
    - pytest-trio
    - pytest-twisted
    warnings.warn(PytestUnhandledCoroutineWarning(msg.format(nodeid)))

which disappears if I comment out the second part with asyncio (which I don't think it's necessary at this stage)
Therefore I don't have the base to work with.

What I think I understand is that there is no way to associate tags to an entity by simply passing the tag name if it exists and hope SQLalchemy magic put in place the association.

#

I did run the Product 3 test and I got an integrity error, but I was expecting that.

#

the solution is to create the Product with the references to the tags, not their names

jade bough
#

But this is the whole excersie. How you can you expect us to automatically do something when it doesn't work normally?

cloud patio
#

which I do in my application, in the same way fullstack does

#

but I'm not asking for that

jade bough
#

If you want my help, you will need to solve what i'm asking

#

sorry

cloud patio
#

isn't this the answer?
the solution is to create the Product with the references to the tags, not their names
to expand, with a code like this

        newtags =   [ 
                        await Tag.as_unique_async(self.repository.session, name=tag_text)
                        for tag_text in taglist or []
                    ]
        data.tags.extend(newtags)
        
        return await self.repository.add(data)

Otherwise, I don't understand what you're asking me...

jade bough
jade bough
jade bough
cloud patio
#

by using an __init__ in Product that takes care of retrieving existing tags and use their objects instead of the string names

jade bough
#

This is why i'd like for you to solve the problem without any controller or repository, Because that is just noise in this case

cloud patio
#

another way would be (akin to what fullstack does) have a service create the Product from a product without tags and a tag list passed separately)

#

the service can have access to the session and retrieve the tags

#

I guess you're telling me Litestar can't give me an object with a taglist because it can't safely resolve the relationship without having a session, which is not available before getting into the @post

jade bough
#

If you can show me how to do this in SQLAlchemy normally, we can see if there is a pattern

#

but, I think if you'd stick with the tests i asked you to try, you'd realize that it doesn't work in SQLAlchemy

#

and it's not just a "simple" pattern here

cloud patio
#

I get that

#

but that's only half of the problem.

#

the issue could be circumvented by having a non Mapped field parsed from the input json

#

like _taglist

jade bough
#

I completely disagree

cloud patio
#

it's ugly I know

jade bough
#

it's not an acceptable solution

#

There's still something in the docs you've not found yet though

cloud patio
#

but without it the only way is to have dataclasses with the same fields as the various DTO views of the ORM class and write code to copy their fields to the ORM Class and viceversa

cloud patio
jade bough
#

This function ```python
def _unique(session, cls, hashfunc, queryfunc, constructor, arg, kw):
cache = getattr(session, '_unique_cache', None)
if cache is None:
session._unique_cache = cache = {}

key = (cls, hashfunc(*arg, **kw))
if key in cache:
    return cache[key]
else:
    with session.no_autoflush:
        q = session.query(cls)
        q = queryfunc(q, *arg, **kw)
        obj = q.first()
        if not obj:
            obj = constructor(*arg, **kw)
            session.add(obj)
    cache[key] = obj
    return obj
Is that unique mixin you referenced above
#

There’s an actual discussion where zeek says this specific problem doesn’t have a great solution. And that the links above are the patterns to do what you are trying to do.

cloud patio
#

the second solution is the one used by fullstack if I'm not mistaken

jade bough
#

Nope, the first one

#

sorry, by first one

#

i meant this Explicit classmethod and session

#

this implements the unique recipe listed in the first section of the doc

cloud patio
#

yes, sorry I meant the second code block

jade bough
#

the 2 following it are the other methods

#

I don't think the scope session one is a good approach

#

but the listener may work

#

After this, you'd just use the normal association proxy

cloud patio
#

now I think I fully get why if I user an ORM backed class as argument of a @post I can't get associated data parsed

#

but I should be able to use DTOData[ORMClass]

#

and get the data in the JSON, then do what I need to

jade bough
#

100%, and if you get the event listening working, you should be able to use the DTO like you originally hade it with the association proxy

cloud patio
#

why would I need to involve SQLAlchemy? isn't DTOdata unrelated?

#

(but yes, that recipe looks interesting, reading it now)

jade bough
#

I'm saying you wouldn't need DTOData at all (no need to call update instance with the taglist). Because the listener would intercept the add at the SQLAlchemy level and mkae the tag list unique

cloud patio
#

(I assume you're referring to the last in the page)

jade bough
#

yeah. I wasn't able to quickly get it working last night, fwiw. It's not as trivial as i'd hoped (at least not with an async session)

#

but, i do think it will work

jade bough
cloud patio
#

what about DTOData not containing associated fields?

jade bough
#

So, if you are using an accepted SQLAlchemy mapping construct, it should be on the field. I validated that it's serialized out last night. I'll verify it's on the post request now

#

what won't work is if you just add any arbitrary property or attribute to the model

cloud patio
#

I read that recipe and it requires specifiy schema knowledge at global Session level, a bit ugly

jade bough
cloud patio
cloud patio
jade bough
halcyon spindleBOT
#

advanced_alchemy/extensions/litestar/dto.py line 141

@handle_orm_descriptor.register(AssociationProxyExtensionType)
jade bough
#

I'd have to go review the dataclass, but i'm wondering if it's a dataclass field that it's looking for

#

@tawdry orchid may know this one

cloud patio
#

understood

jade bough
halcyon spindleBOT
#

litestar/dto/dataclass_dto.py line 29

def generate_field_definitions(
jade bough
#

@cloud patio, @tawdry orchid just pointed out a property that i hadn't seen before. include_implicit_fields in the SQLAlchemyDTO config

#

can you set this to true and see if your field is added to the DTO?

#

this may not work, but let's rule it out now if so

#

@tawdry orchid answer here!

cloud patio
#

include_implicit_fields: bool | Literal["hybrid-only"] = True

#

default is true tho

jade bough
#

ok, so the default is already true

#

I need to chat with peter to see if that's what he intended for this method

tawdry orchid
#

i dont think peter wrote this

cloud patio
#

did you manage to get the DTOData with the taglist in the @post? because I couldn't

jade bough
#

Who wrote it? This is like the one piece in AA that didn't have my hand

cloud patio
#

my MCVE is all about this (line 211)

jade bough
#

if it's a unmapped property I think it doesn't work like you've identified

cloud patio
#
    taglist: AssociationProxy[list[str]] = association_proxy("tags","name",creator=lambda name: Tag(name=name))
#

yeah I get the unmapped property thing now

jade bough
#

ok, this specific case feels like something that should work. I'll finish creating a few additional tests for this today. If it doesn't, it can be fixed.

cloud patio
#

Implicit Private Fields

Fields that are named with a leading underscore are considered “private” by default. This means that they will not be parsed from client data, and will not be serialized into return data.

#

LOL there goes my tests with _taglist

jade bough
#

good catch

cloud patio
#

wouldn't have worked anyway on an sqlalchemy orm backed class

#

because of what you explained above

#

yep, just confimed with taglist2: list[str]

cloud patio
#

so I suppose it's the parser

tawdry orchid
#

can you link me to a snippet that has this issue?

#

there is a lot to read here

cloud patio
#
class Entity(AsyncAttrs, UUIDAuditBase):
    __allow_unmapped__ = True
    name: Mapped[str] = mapped_column(String(191))
    tags: Mapped[List[Tag]] = relationship(secondary=lambda: tag_association_table, lazy="noload", cascade="all, delete",
            passive_deletes=True)
    taglist: AssociationProxy[list[str]] = association_proxy("tags","name",creator=lambda name: Tag(name=name))
    @post(dto=EntityCreateDTO)
    async def create_entity(self, data: DTOData[Entity], service: Service) -> Entity:
        b = data.as_builtins()
        print(type(b))
        print(f"Creating with name {b["name"]}")
        print(f"Creating with taglist {b["taglist"]}")
        return await service.create(data, data.taglist)

problem: b has no taglist element

#
{
    "name": "entity1",
    "taglist": [
        "tag1",
        "tag2",
        "tag3"
    ]
}

when passed this

tawdry orchid
#

thanks

cloud patio
#

I'm trying to navigate the execution stack to find where taglist is lost but it's a maze of callbacks I'm not familiar with

jade bough
#

It does look like there's a bug on the incoming parse with the minimal MVCE that @tawdry orchid has for the AssociationProxy mapped type

#

it's on the outgoing response

#

so it's generated/parsing the data, but there's something different there

tawdry orchid
#

@cloud patio can you try this

#
from litestar.dto import DTOField
taglist: AssociationProxy[list[str]] = association_proxy("tags","name",creator=lambda name: Tag(name=name), info={"__dto__": DTOField()})
#

@jade bough FYI

tawdry orchid
cloud patio
#

On school run atm. asap!

cloud patio
#
    async def create_entity(self, data: DTOData[Entity], service: Service) -> Entity:
        b = data.as_builtins()
        print(type(b))
        print(f"Creating with name {b["name"]}")
        print(f"Creating with taglist {b["taglist"]}")

results in

Creating with name entity1
Creating with taglist ['tag1', 'tag2', 'tag3']

Victory!
Thanks a lot guys!

one last comment: with proper annotation or similar technique I think it would be possible for advanced_alchemy to install an event listener as described above that can be made generic and solve the "association presented as a list" problem. This is beyond my ability (as of now, at least) therefore I'm not going to try and submit a PR for it, just throwing the idea here to see if it's theoretically feasible

jade bough
#

Yes, potentially. Or we can offer a helper so that you can initialize one more easily.

#

There’s a bit to work through, but we already use event listeners for the create/updated audit column mixin

tawdry orchid
cloud patio
#

yes

#
class EntityService(Service[Entity]):
    repository_type = EntityRepository
    async def create(
        self,
        data: Entity,
        taglist: List[str] = None
    ) -> Entity:
        data.taglist = []
        data.tags = []
        newtags =   [ 
                        await Tag.as_unique_async(self.repository.session, name=tag_text)
                        for tag_text in taglist or []
                    ]
        data.tags.extend(newtags)
        
        return await self.repository.add(data)
    @post(dto=EntityCreateDTO)
    async def create_entity(self, data: DTOData[Entity], service: Service) -> Entity:
        b = data.as_builtins()
        tl=b["taglist"]
        return await service.create(data.create_instance(), tl)
#
class Entity(AsyncAttrs, UUIDAuditBase):
    __allow_unmapped__ = True
    name: Mapped[str] = mapped_column(String(191))
    tags: Mapped[List[Tag]] = relationship(secondary=lambda: tag_association_table, lazy="noload", cascade="all, delete",
            passive_deletes=True)
    taglist: AssociationProxy[list[str]] = association_proxy("tags","name",creator=lambda name: Tag(name=name), info={"__dto__": DTOField()})
#

here all the relevant pieces of code to make it work (for create)

#

I wonder what happens if I add an unmapped attribute and annotate it with the same technique..

tawdry orchid
#

I think we are done here with respect to your original issue

#

can this be closed?

cloud patio
#

yes, thank you

#

I'll change the title to make it more relevant to the problem

#

ah, seems I can't...

tawdry orchid
#

what do you want to name it as

cloud patio
#

Unmarshalling of ORM associations?

#

because that's what the problem was in hindsight

tawdry orchid
#

not sure who would search here tbh

#

the ones who do are mostly likely reading this

cloud patio
#

well others might have the same problem

tawdry orchid
#

alr, thanks

#

Unmarshalling of ORM associations

jade bough
#

woohoo, so there were a lot of moving pieces in this one

cloud patio
#

the current title is misleading because that's not possible by design

jade bough
#

so it sounds like the only thing missing is the unmapped fields support

tawdry orchid
#

i want to ask, why would you have unmapped attributes in a sqla model

jade bough
#

(and the SQLAlchemy fix for merging into the many-to-many)

cloud patio
#

@tawdry orchid to be able to use the same model in controllers for marshalling and unmarshalling

#

calculated properties etc

tawdry orchid
cloud patio
#

that's another way to solve it, but say you want to bring that unmapped attribute around in the code...

jade bough
jade bough
#

i'm still not sure if a use case that couldn't be an associative array or hybrid property, but I am probably only seeing things a certain way at this point

#

Or, maybe it's something that can be accomplished by changing the model to use MappedAsDataclass

#

and use the dataclass DTO instead

#

Anyway, I think the end goal in this case is to be able to post a list of strings (or whatever structure it may be) and have it load into the many-to-many objects.

#

Glad you stuck with us on this one @cloud patio

cloud patio
#

well sorry to have tested your patience, I know it can be frustrating when who asks doesn't "get" the bigger picture. In hindsight I asked stupid questions

jade bough
#

One more link that i read but couldn't find earlier

cloud patio
#

this works

class Entity(AsyncAttrs, UUIDAuditBase):
    __allow_unmapped__ = True
    name: Mapped[str] = mapped_column(String(191))
    tags: Mapped[List[Tag]] = relationship(secondary=lambda: tag_association_table, lazy="noload", cascade="all, delete",
            passive_deletes=True)
    taglist: AssociationProxy[list[str]] = association_proxy("tags","name",creator=lambda name: Tag(name=name), info={"__dto__": DTOField()})
    
    @hybrid_property
    def extrafield(self) -> str:
        return self.extrafield_
    
    @extrafield.setter
    def extrafield(self, value: str) -> None:
        self.extrafield_ = value

    extrafield_: str
#

I got extrafield in the DTOData

jade bough
cloud patio
#

There's no way to do this case without doing a database lookup, so if I were using this pattern myself, since I am a big proponent of the transational context being explicit, I'd make the creator here raise an error and only use the assocaition proxy for viewing. manipulation of items would be via special methods that accept the Session so that it can look up an existing Keyword.

#

ditto

cloud patio
#

I spoke too soon. when I get to the object creation

TypeError: 'extrafield' is an invalid keyword argument for Entity
#

strange, it fails in sqlalchemy.orm.decl_base.py

def _declarative_constructor(self: Any, **kwargs: Any) -> None:
    """A simple constructor that allows initialization from kwargs.

    Sets attributes on the constructed instance using the names and
    values in ``kwargs``.

    Only keys that are present as
    attributes of the instance's class are allowed. These could be,
    for example, any mapped columns or relationships.
    """
    cls_ = type(self)
    for k in kwargs:
        if not hasattr(cls_, k):
            raise TypeError(
                "%r is an invalid keyword argument for %s" % (k, cls_.__name__)
            )
        setattr(self, k, kwargs[k])

the hasattr doesn't find "extrafield" and raises the exception

#

as there is no way of removing a builtin value from a DTO this cannot be used as a cheap way to get an extra non mapped attribute to the controller to, say, alter its behaviour

#

which I think it's a use case. Another use case is when the external representation is different from the internal one

jade bough
#

MappedAsADataclass and init=False may be the way forward

#

really, just guesings here though. I'd need to test set up some unit tests for just this to be sure

cloud patio
#

As in something that should work now? That page provides a few interesting examples

jade bough
#

Yeah, I don’t see why this wouldn’t work.