dahlia / sqlalchemy-imageattach

SQLAlchemy extension for attaching images to entities.
https://sqlalchemy-imageattach.readthedocs.io/
MIT License
116 stars 25 forks source link

generate_thumbnail() doesn't work right when using Multiple Image Sets #44

Open rogeriobastos opened 6 years ago

rogeriobastos commented 6 years ago

When using Multiple Image Sets generate_thumbnail() only create a thumbnail to the first image. This is happening because generate_thumbnail() thinks a thumbnail was already generated and return the thumbnail of the first image.

I'm using Python (2.7.13), SQLAlchemy (1.1.14) and SQLAlchemy-ImageAttach (1.1.0).

and-semakin commented 6 years ago

I can confirm the issue on Python 3.6.4, SQLAlchemy 2.3.2 and sqlalchemy-imageattach 1.1.0. I think it happens because generate_thumbnail() does not consider our 'extra discriminating primary key columns' and just search for matches though all the thumbnails.

That's what I see via debugger (entity.py, line 759):

SELECT level_scheme.width AS level_scheme_width, level_scheme.height AS level_scheme_height, level_scheme.mimetype AS level_scheme_mimetype, level_scheme.original AS level_scheme_original, level_scheme.created_at AS level_scheme_created_at, level_scheme.level_id AS level_scheme_level_id, level_scheme.order_index AS level_scheme_order_index 
FROM level_scheme 
WHERE %(param_1)s = level_scheme.level_id AND level_scheme.width = %(width_1)s

That's just about what I expected to see, because I have additional column order_index in the primary key:

SELECT level_scheme.width AS level_scheme_width, level_scheme.height AS level_scheme_height, level_scheme.mimetype AS level_scheme_mimetype, level_scheme.original AS level_scheme_original, level_scheme.created_at AS level_scheme_created_at, level_scheme.level_id AS level_scheme_level_id, level_scheme.order_index AS level_scheme_order_index 
FROM level_scheme 
WHERE %(param_1)s = level_scheme.level_id AND level_scheme.width = %(width_1)s AND level_scheme.order_index = %(order_index_1)s
and-semakin commented 6 years ago

It seems to me that author forget to apply filter_by(**self.identity_map) in the ImageSubset.__init__() constructor. As I can see filter_by() function is applied later in ImageSubset.count(). I think that is the trick. Will check it out.

Edit. Hmm, no, it was not so easy.

and-semakin commented 6 years ago

@dahlia, your code is beautiful and well-documented but it's very difficult to unravel crazy genealogy of classes and mixins :fearful: uselist=True mode seems to be very buggy: at least I get different exceptions if I add list of images to existing object or not.