Ticket #1563 (closed defect: fixed)

Opened 8 months ago

Last modified 8 months ago

Deleting last lang content doesn't set metadata_deleted=1 but instead throws the _i row away leaving torso object with metadata_deleted=0 behind

Reported by: jval Assigned to: piotras
Priority: critical Milestone: 8.09.8 Ragnaroek
Component: Midgard Core Version: 8.09 Ragnaroek
Keywords: Cc:

Description

If lang is something else than 0, delete of last lang content doesn't set metadata_deleted to 1 but instead throws the last _i row away.

This leaves the database into inconsistent state because the actual main table row is still there with metadata_deleted=0 but all _i rows are all gone. The object is basically gone (last instance of it was deleted after all) but its table still has a row claiming the object exists (when it actually was deleted). Clearly that's not correct. When object is gone, metadata_deleted should also be set to 1.

This also affects the end user: Asgard's trash can can't be used to restore the delete. (Which is expected as now single lang object deletes can't be restored if lang is something else than 0.)

I think I know how this should be fixed. See:

http://trac.midgard-project.org/browser/branches/ragnaroek/midgard/core/midgard/src/types.c?rev=24638#L2822

The skipping of lang0 rows when counting the _i table rows should be removed. Then the _i row should be deleted if there are multiple rows. See:

http://trac.midgard-project.org/browser/branches/ragnaroek/midgard/core/midgard/src/types.c?rev=24638#L2841

"lang_i > 0" should be "lang_i > 1" then.

If done that way, the delete for last lang content would result _i row to stay there and metadata_deleted to be set to 1 instead. Which would be the correct way to handle this I think. Clearly metadata_deleted should be 1 when object is gone.

I'm setting this to critical because the database inconsistency effect makes this a bit ugly. It is hard to clean the database afterwards...

Change History

01/07/10 18:55:25 changed by jval

Basically don't delete langX content from the _i table if it's the only row for the object. And to get that effect all rows need to be counted (lang0 rows shouldn't be excluded from the row count) and the row count check should then be "> 1" (like it was previously) to get the effect of skipping the delete if there's only one row.

Code will get simpler even - and correct. :)

(follow-up: ↓ 3 ) 01/08/10 11:11:22 changed by piotras

  • status changed from new to assigned.

Your proposal triggers #1522. This is logical bug. There's assumption lang0 content always exists.

(in reply to: ↑ 2 ) 01/08/10 11:56:30 changed by jval

Replying to piotras:

Your proposal triggers #1522. This is logical bug. There's assumption lang0 content always exists.

No it doesn't trigger it. According to my proposal when the *last* lang content is deleted the whole object would be marked as deleted instead. Which would be *correct* behavior.

#1522 was about situations where the langX _i table row is *not* the last one. In such cases the whole object must not be marked as deleted. But my proposal would not affect those cases because in those cases there are multiple _i table rows (e.g. in example given in #1522 there are lang0 row and langX row - if my proposal would be implemented, in the #1522 example case langX row would be deleted --> #1522 would not be triggered).

01/11/10 19:18:21 changed by piotras

  • status changed from assigned to closed.
  • resolution set to fixed.

(In [24648]) midgard_object_delete. Deletes multilang content if there are more languages. Lang0 is not considered special lang in this case. Fix #1563

01/13/10 00:26:44 changed by jval

Above commit introduced #1579 for 8.09.7.99.

It is fixed in newer 8.09.7.99, so no action is needed here. I just wanted to cross reference these tickets for better understanding of the changes.