TableModel Partial updates

[warvariuc]

spyne.model.complex.ComplexModelBase

    def __init__(self, **kwargs):
        cls = self.__class__
        fti = cls.get_flat_type_info(cls)
        for k,v in fti.items():
            if k in kwargs:
                setattr(self, k, kwargs[k])
            elif not k in self.__dict__:
                a = v.Attributes
                if a.default is not None:
                    setattr(self, k, v.Attributes.default)
                elif a.max_occurs > 1 or issubclass(v, Array):
                    try:
                        setattr(self, k, None)
                    except TypeError: # SQLAlchemy does this
                        setattr(self, k, [])
                else:
                    setattr(self, k, None)

I removed in our project lines (actually i replaced TableModel.__init__):

                else:
                    setattr(self, k, None)

because they cause the following behavior:

If a SOAP request comes with some optional fields omitted, those lines assign to skipped fields None overwriting with None current field values from the database.

[warvariuc]

Actually, i think these

                elif a.max_occurs > 1 or issubclass(v, Array):
                    try:
                        setattr(self, k, None)
                    except TypeError: # SQLAlchemy does this
                        setattr(self, k, [])

should also be removed for TableModels

[plq]

Hi,

I agree that the default ComplexModelBase.init is problematic. The default ComplexModel constructor changed a lot. Can you look at the new one and tell me what you think?

I added a "read_only" attribute and the code that enforces it, maybe that'll be a nice workaround.

Thanks!

[warvariuc]

Hi, i see this code:

    def __init__(self, *args, **kwargs):
        cls = self.__class__
        fti = cls.get_flat_type_info(cls)
        xtba_key, xtba_type = cls.Attributes._xml_tag_body_as

        if xtba_key is not None and len(args) == 1:
            self._safe_set(xtba_key, args[0], xtba_type)
        elif len(args) > 0:
            raise TypeError("No XmlData field found.")

        for k,v in fti.items():
            if k in kwargs:
                self._safe_set(k, kwargs[k], v)
            elif not k in self.__dict__:
                a = v.Attributes; d=a.default
                if d is not None:
                    setattr(self, k, d)
                elif a.max_occurs > 1 or issubclass(v, Array):
                    try:
                        setattr(self, k, d)
                    except TypeError: # SQLAlchemy does this
                        setattr(self, k, [])
                else:
                    setattr(self, k, None)

...
    def _safe_set(self, key, value, t):
        if t.Attributes.read_only:
            pass
        else:
            setattr(self, key, value)

I don't think marking an attribute of a TableModel as read-only is a solution:

1. You have to manually mark attributes as read-only - a lot of manual work, especially as TableModel fields are extracted automatically from SQLAlchemy model.
2. I am not talking about read-only attributes (fields), but about fields whose values didn't come in a SOAP request at all. I.e. they are writable, but if they are omitted an a request, they should not be set to None

[plq]

I mostly agree with your points here.

I think the best thing to do here is to override ComplexModelBase.__init__ in TableModel.

[warvariuc]

Yes, that was my point:

I removed in our project lines (actually i replaced TableModel.__init__):

[plq]

ok, if you can send a pull request, we'll continue from there.

[warvariuc]

I've been thinking that it's not very good that in ComplexModel (not TableModel) all fields are set to None if there are no default values for them, too. Because you cannot determine if a field was set to None because nil came in the SOAP request or because the field didn't come at all. I mean this is important not only for TableModel where it can lead to data corruption in database, but also in other cases, where you want to know if a value for a field came at all and take corrsponding action (e.g. do partial update to other backend, not just SQLAlchemy). So if we disable this behavior, you would check if a field value came in a SOAP request by checking CompexModels instance __dict__ or catching AttributeError exception. But I suppose this is a legacy behavior, and many tests and existing applications rely on it. Am I correct?

[plq]

BTW, you must be wondering, I still haven't made my mind up about this issue. I'll spill my guts the moment I got something to say, as I think this is a very valid concern about the way Spyne handles incoming data.

[warvariuc]

I am not wondering, i know it's a complicated issue. I would like to share what i did in our project. In ComplexModels i introduced support for a new attribute __model__ which maps to ComplexModel.Attributes.model. It can be any callable: a function, a class, an SQLAlchemy model. And when a request comes with as complex object data, i get the data and pass it a kwargs to that callable. The object is then passed to a service method. This way:

1. I solve the issue of not having Nones for attributes not passed in the request
2. A can support any model, not just SQLAlchemy models - for example Django models.