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

Generic assignment (Auto Coercion) #98

Open
lelit opened this issue Mar 12, 2018 · 7 comments
Open

Generic assignment (Auto Coercion) #98

lelit opened this issue Mar 12, 2018 · 7 comments
Assignees
Milestone

Comments

@lelit
Copy link
Contributor

lelit commented Mar 12, 2018

First of all I'm sorry about the title, couldn't figure out a better phrase... second, sorry again for the length of this!

My use case is the following: I have different SA classes containing different Image kinds (that is, with different constraints such as max_length or pre_processors), for example:

from sqlalchemy_media import Image, ImageProcessor, KB, WandAnalyzer

class IconImage(Image):
    __auto_coercion__ = True
    __max_length__ = 256*KB
    __pre_processors__ = (
        WandAnalyzer(),
        ImageValidator(
            minimum=(16, 16),
            maximum=(800, 800),
            min_aspect_ratio=0.9,
            max_aspect_ratio=1.1,
            content_types=('image/png', 'image/jpeg', 'image/gif')),
        ImageProcessor(fmt='png', width=80),        
    )

class ProfileImage(Image):
    __auto_coercion__ = True
    __max_length__ = 2048*KB
    __pre_processors__ = (
        WandAnalyzer(),
        ImageValidator(
            minimum=(80, 80),
            maximum=(3840, 3840),
            min_aspect_ratio=0.9,
            max_aspect_ratio=1.1,
            content_types=('image/png', 'image/jpeg', 'image/gif')),
        ImageProcessor(fmt='png', width=80),        
    )

class Person(Base):
    __tablename__ = 'persons'

    id = Column(Integer, primary_key=True)
    name = Column(Unicode(100))
    image = Column(ProfileImage.as_mutable(Json))

    def __repr__(self):
        return "<%s id=%s>" % (self.name, self.id)

class Tool(Base):
    __tablename__ = 'tools'

    id = Column(Integer, primary_key=True)
    name = Column(Unicode(100))
    image = Column(IconImage.as_mutable(Json))

    def __repr__(self):
        return "<%s id=%s>" % (self.name, self.id)

I have then a generic function that, given the class of an entity and basically a dictionary coming from a data entry form, creates a new instance and initialize its content, greatly simplified as this:

def create_instance(class_, data):
    instance = class_()
    for key, value in data.items():
        setattr(instance, key, value)

For the image attribute, the value is actually a cgi.FieldStorage dictionary, and thus I have to handle it in a special way, and here are my two problems:

  1. I didn't find a way to find out the specific custom Image class (i.e. given a Tool find out that its image column is actually an IconImage), so that I could write:
def create_instance(class_, data):
    instance = class_()
    for key, value in data.items():
        if "is the image attribute":
            custom_image_class = find_custom_image_class(class_, key)
            setattr(instance, key, custom_image_class.create_from(value))
        else:
            setattr(instance, key, value)

This is because the actual class is buried very deep in the SA Mutable machinery, and thus class_.image.property.expression gives you Json, not IconImage...

  1. As a workaround, I tried this:
def create_instance(class_, data):
    instance = class_()
    for key, value in data.items():
        if "is the image attribute":
            setattr(instance, key, (value['fp'], value['mimetype'], value['filename']))
        else:
            setattr(instance, key, value)

to delegate the logic to the Attachment.coerce() class method, but that fails too, due to the way it forwards the argument to Attachment.create_from(): IMHO it should handle the case when value is a tuple, and when that's the case call cls.create_from(*value)... and indeed the following monkey patch seems to satisfy my needs:

def coerce(cls, key, value) -> 'Attachment':
    """
    Converts plain dictionary to instance of this class.

    .. seealso:: :meth:`sqlalchemy.ext.mutable.MutableDict.coerce`

    """
    if value is not None and not isinstance(value, dict):
        try:
            cls._assert_type(value)
        except TypeError:
            if cls.__auto_coercion__:
                if isinstance(value, tuple):
                    return cls.create_from(*value)
                else:
                    return cls.create_from(value)
            raise

    return super(SAMAttachment, cls).coerce(key, value)


SAMAttachment.coerce = classmethod(coerce)

What do you think? Am I either missing something or abusing the API, or is there a better way to accomplish the above?

Thank you in advance for any hint!

@pylover
Copy link
Owner

pylover commented Mar 17, 2018

Thanks for the nice description, and sorry for late.

I'll check it asap.

@pylover
Copy link
Owner

pylover commented Mar 24, 2018

Please accept my apologies because of the late.

Yes, you are right, but the coerce classmethod is the only thing you need to override.

In the coerce classmethod you can define your very own behavior to align with the HTTP framework you are using.

At the other side, I think the idea of setting the file-like object directly to a model attribute is awesome, and I couldn't find any test to ensure this feature is already implemented or not.

So, I'll add some tests inside a new branch named genericassignment.

After reading this, I found the title: Generic assignment perfect.

Many thanks for the contribution.

@lelit
Copy link
Contributor Author

lelit commented Mar 24, 2018

No need to apologize, really! Thanks a lot for this handy add-on!

But I'm not sure I understand what you are suggesting: are you going to tweak the Attachment.coerce classmethod, or should I keep doing the mentioned monkey-patch?

@pylover
Copy link
Owner

pylover commented Mar 24, 2018

Currently, I'm working on that to get rid of this situation.

After fixing this issue, These lines should be working:

person1.cv = filelike
person1.cv = filelike, mimetype
person1.cv = filelike, mimetype, filename

Any suggestion?

@pylover pylover changed the title Generic assignment Generic assignment (Auto Coercion) Mar 24, 2018
pylover added a commit that referenced this issue Mar 24, 2018
pylover added a commit that referenced this issue Mar 24, 2018
@pylover pylover reopened this Mar 24, 2018
@pylover
Copy link
Owner

pylover commented Mar 24, 2018

Please take a look at this test case:

class AutoCoerceFile(File):
    __auto_coercion__ = True


class AutoCoercionTestCase(TempStoreTestCase):

    def test_file_assignment(self):

        class Person(self.Base):
            __tablename__ = 'person'
            id = Column(Integer, primary_key=True)
            cv = Column(AutoCoerceFile.as_mutable(Json))

        session = self.create_all_and_get_session()
        person1 = Person()
        resume = io.BytesIO(b'This is my resume')
        with StoreManager(session):
            person1.cv = resume
            self.assertIsNone(person1.cv.content_type)
            self.assertIsNone(person1.cv.extension)
            self.assertTrue(exists(join(self.temp_path, person1.cv.path)))

            person1.cv = resume, 'text/plain'
            self.assertEqual(person1.cv.content_type, 'text/plain')
            self.assertEqual(person1.cv.extension, '.txt')
            self.assertTrue(exists(join(self.temp_path, person1.cv.path)))

            person1.cv = resume, 'text/plain', 'myfile.note'
            self.assertEqual(person1.cv.content_type, 'text/plain')
            self.assertEqual(person1.cv.extension, '.note')
            self.assertTrue(exists(join(self.temp_path, person1.cv.path

The __auto_coercion__ is what you want.

@lelit
Copy link
Contributor Author

lelit commented Mar 24, 2018

Great, thank you! I cannot try it out immediately, but of course I will do it as soon as possible.

@pylover
Copy link
Owner

pylover commented Mar 24, 2018

Pending on #101

@pylover pylover added this to the v0.18 milestone Aug 2, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants