c2corg / v6_api

REST API for https://www.camptocamp.org
GNU Affero General Public License v3.0
22 stars 26 forks source link

Constraint error with the feed when deleting an image #634

Open asaunier opened 7 years ago

asaunier commented 7 years ago

Not sure the changes done in the feed entries when deleting an image https://github.com/c2corg/v6_api/blob/master/c2corg_api/views/document_delete.py#L323-L346 are sufficient: while trying to remove an image in my instance I got the following error message:

sqlalchemy.exc.IntegrityError: (psycopg2.IntegrityError) update or delete on table "images" violates foreign key constraint "feed_document_changes_image1_id_fkey" on table "feed_document_changes" DETAIL: Key (document_id)=(842105) is still referenced from table "feed_document_changes". [SQL: 'DELETE FROM guidebook.images WHERE guidebook.images.document_id = %(document_id_1)s'] [parameters: {'document_id_1': (842105,)}]

tsauerwein commented 7 years ago

Far guess but it could be that or_ only takes 2 arguments instead of 3: https://github.com/c2corg/v6_api/blob/d9c24c9cda7d4298c26483e011cbf8f251aa4992/c2corg_api/views/document_delete.py#L327

asaunier commented 7 years ago

Far guess but it could be that or_ only takes 2 arguments instead of 3

Are you sure only 2 arguments are supported? Why SqlAlchemy does not output an explicit error then? The doc says sqlalchemy.sql.expression.or_(*clauses) http://docs.sqlalchemy.org/en/latest/core/sqlelement.html?highlight=or_#sqlalchemy.sql.expression.or_ Doesn't *clauses mean "whatever number of clauses"?

In this discussion, people give example of or_ with >2 arguments http://stackoverflow.com/questions/7942547/using-or-in-sqlalchemy For instance

filter(or_(User.name == v for v in ('Alice', 'Bob', 'Carl')))
tsauerwein commented 7 years ago

Yes, you are right. The query is generated correctly.

tsauerwein commented 7 years ago

What does the feed entry look like? Could it be that image1_id and image2_id both are 842105?

asaunier commented 7 years ago

I think the feed entry data are:

 change_id |             time              | user_id | change_type | document_id | document_type |  activities  |             area_ids             | user_ids | image1_id | image2_id | image3_id | more_images 
-----------+-------------------------------+---------+-------------+-------------+---------------+--------------+----------------------------------+----------+-----------+-----------+-----------+-------------
     82450 | 2017-01-19 16:13:15.247604+01 |     377 | updated     |      734063 | o             | {skitouring} | {14274,14295,14361,14407,252522} | {377}    |    842105 |           |           | f
tsauerwein commented 7 years ago

Thanks! The function _remove_image_from_feed should work correctly in that case. I am wondering if the DBSession.flush(), that I told you to remove, is needed after all. :) Because you are making changes to objects inside the session, but then you are deleting rows outside the session. So the changes to the objects inside the session might not have been synchronized yet with the database.

Could you try with a DBSession.flush() at the end of _remove_image_from_feed?

asaunier commented 7 years ago

Could you try with a DBSession.flush() at the end of _remove_image_from_feed?

It does not work either. By the way does a DBSession.flush() results in a COMMIT? If so couldn't it neutralize the rollback we expect if further requests fail? I guess not since my image document is still available after the last crash despite the flush. But just want to make sure :P

tsauerwein commented 7 years ago

flush only synchronizes the changes made to objects with the database. For example if you have changed an attribute of an object, SQLAlchemy issues an update statement. The transaction is not committed at that point.