[digikam] [Bug 363697] Album visible even after deletion [patch]

[Maik Qualmann via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=363697

--- Comment #28 from Maik Qualmann  ---
Yes, the FOREIGN_KEY_CHECKS patch is included into git/master.

Maik

-- 
You are receiving this mail because:
You are watching all bug changes.

[digikam] [Bug 363697] Album visible even after deletion [patch]

[via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=363697

--- Comment #27 from swatilodh...@gmail.com ---
Maik,

Could your last patch be included before 5.0.0 ?

-- 
You are receiving this mail because:
You are watching all bug changes.

[digikam] [Bug 363697] Album visible even after deletion [patch]

[Maik Qualmann via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=363697

--- Comment #26 from Maik Qualmann  ---
I think that this solution is better at the moment. I have set this problem on
my todo list, to find a clean solution for the DB.

Maik

-- 
You are receiving this mail because:
You are watching all bug changes.

[digikam] [Bug 363697] Album visible even after deletion [patch]

[Maik Qualmann via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=363697

Maik Qualmann  changed:

   What|Removed |Added

 Status|UNCONFIRMED |RESOLVED
   Version Fixed In||5.0.0
  Latest Commit||http://commits.kde.org/digi
   ||kam/adc7a5868a10f8f21ac3b36
   ||edecdd9f53f1efec4
 Resolution|--- |FIXED

--- Comment #25 from Maik Qualmann  ---
Git commit adc7a5868a10f8f21ac3b36edecdd9f53f1efec4 by Maik Qualmann.
Committed on 29/06/2016 at 20:45.
Pushed by mqualmann into branch 'master'.

disable temporary Foreign_Key_Checks
FIXED-IN: 5.0.0

M  +2-2NEWS
M  +2-0libs/database/coredb/coredb.cpp
M  +17   -0libs/database/engine/dbenginebackend.cpp
M  +6-0libs/database/engine/dbenginebackend.h

http://commits.kde.org/digikam/adc7a5868a10f8f21ac3b36edecdd9f53f1efec4

-- 
You are receiving this mail because:
You are watching all bug changes.

[digikam] [Bug 363697] Album visible even after deletion [patch]

[Maik Qualmann via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=363697

Maik Qualmann  changed:

   What|Removed |Added

  Attachment #99765|0   |1
is obsolete||

-- 
You are receiving this mail because:
You are watching all bug changes.

[digikam] [Bug 363697] Album visible even after deletion [patch]

[Maik Qualmann via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=363697

--- Comment #24 from Maik Qualmann  ---
No, the entry does not exist here in the DB. Strange.

Maik

-- 
You are receiving this mail because:
You are watching all bug changes.

[digikam] [Bug 363697] Album visible even after deletion [patch]

[via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=363697

--- Comment #23 from swatilodh...@gmail.com ---
(In reply to Maik Qualmann from comment #22)
> Created attachment 99765 [details]
> removeStaleAlbum.patch
> 
> This patch removes stale album function. Swati, you can test it please.

The deleted album is no more visible in DK, but entry still exists in DB.
Is it the same for you?

-- 
You are receiving this mail because:
You are watching all bug changes.

[digikam] [Bug 363697] Album visible even after deletion [patch]

[Maik Qualmann via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=363697

--- Comment #22 from Maik Qualmann  ---
Created attachment 99765
  --> https://bugs.kde.org/attachment.cgi?id=99765=edit
removeStaleAlbum.patch

This patch removes stale album function. Swati, you can test it please.

Maik

-- 
You are receiving this mail because:
You are watching all bug changes.

[digikam] [Bug 363697] Album visible even after deletion [patch]

[Maik Qualmann via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=363697

Maik Qualmann  changed:

   What|Removed |Added

  Attachment #99742|0   |1
is obsolete||

-- 
You are receiving this mail because:
You are watching all bug changes.

[digikam] [Bug 363697] Album visible even after deletion [patch]

[Maik Qualmann via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=363697

--- Comment #21 from Maik Qualmann  ---
I wonder just what need the stale album function. Perhaps Marcel can give an
indication. I have in the CollectionScanner this function replaced with
CoreDB::deleteAlbum(). It seems to work without problems. I create tonight a
patch.

Maik

-- 
You are receiving this mail because:
You are watching all bug changes.

[digikam] [Bug 363697] Album visible even after deletion [patch]

[Richard Mortimer via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=363697

--- Comment #20 from Richard Mortimer  ---
I like the look of Maik's patch. It does seem to be working in a portable way
that does not rely on any magic database primary key values. A deleted
AlbumRoot is easy to identify too.

A few comments on the patch:

the magic relativePath value "*deleted*" should really be a constant that is
used throughout the application. That way it is easier to ensure consistency
and find places where the value is used.

I haven't looked but does addAlbumRoot cope gracefully with situations where
there is already an existing albumRoot for deleted? If not then the code needs
to check for presence of a deleted album root before trying to add one.

-- 
You are receiving this mail because:
You are watching all bug changes.

[digikam] [Bug 363697] Album visible even after deletion [patch]

[via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=363697

--- Comment #19 from swatilodh...@gmail.com ---
(In reply to Maik Qualmann from comment #18)
> Created attachment 99742 [details]
> albumRoot.patch
> 
> A first new patch, it works. The question is, is correct and can solved this
> problem?

Well, it did work for me. After deletion of album, it was neither visible, nor
present in DB.
And this patch solves the issue in right way I think.

-- 
You are receiving this mail because:
You are watching all bug changes.

[digikam] [Bug 363697] Album visible even after deletion [patch]

[Maik Qualmann via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=363697

--- Comment #18 from Maik Qualmann  ---
Created attachment 99742
  --> https://bugs.kde.org/attachment.cgi?id=99742=edit
albumRoot.patch

A first new patch, it works. The question is, is correct and can solved this
problem?

Maik

-- 
You are receiving this mail because:
You are watching all bug changes.

[digikam] [Bug 363697] Album visible even after deletion [patch]

[Maik Qualmann via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=363697

Maik Qualmann  changed:

   What|Removed |Added

  Attachment #99669|0   |1
is obsolete||

-- 
You are receiving this mail because:
You are watching all bug changes.

[digikam] [Bug 363697] Album visible even after deletion [patch]

[via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=363697

--- Comment #17 from swatilodh...@gmail.com ---
Richard,

Could you please provide your suggestions here?

Thank you.

-- 
You are receiving this mail because:
You are watching all bug changes.

[digikam] [Bug 363697] Album visible even after deletion [patch]

[Johannes Hirte via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=363697

--- Comment #16 from Johannes Hirte  ---
(In reply to swatilodha27 from comment #15)
> Is it possible to already have a "Fake Album" row when the DB is initially
> created? 
> In the DB schema file, adding a query:
> //
> INSERT INTO AlbumRoots values(0, "Fake Album Root", 0, 0, NULL, NULL);
> //
> 
> This way the current UPDATE query in the makeStaleAlbum() would execute
> without foreign key constraint failure.

Did this manually and it seems to work. But I don't know if other parts of the
code may get confused about this fake album.

> Also, when deleteStaleAlbum() would be called, all those Albums would be
> deleted from the DB.
> It just need to be take care to include this function in a way that it gets
> executed each time when digiKam is started.

I don't really understand the mechanism of hints for now. Just calling
deleteStaleAlbums() unconditionally will do, what you're suggesting. Maybe this
is overkill and we can hook this in another place better.

-- 
You are receiving this mail because:
You are watching all bug changes.

[digikam] [Bug 363697] Album visible even after deletion [patch]

[Richard Mortimer via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=363697

Richard Mortimer  changed:

   What|Removed |Added

 CC||richm+...@oldelvet.org.uk

--- Comment #14 from Richard Mortimer  ---
Disabling foreign key checks is pretty much the same as not using referential
integrity. It allows the database to get into "illegal" states. It is also
highly non-portable between database engines.

I just tried a quick test with MySQL 5.5 and simulated a crash halfway through
deleting an album. That left the database with an albumRoot set to a
non-existant value in the database.

mysql> set foreign_key_checks = 0;
Query OK, 0 rows affected (0.00 sec)

mysql> update Albums set albumRoot = 0 where id = 1;
Query OK, 1 row affected (0.19 sec)
Rows matched: 1  Changed: 1  Warnings: 0

mysql> set foreign_key_checks = 1;
Query OK, 0 rows affected (0.00 sec)

mysql> select * from Albums;
++---+--++-++--+
| id | albumRoot | relativePath | date   | caption | collection | icon |
++---+--++-++--+
|  1 | 0 | /| 2015-11-25 | NULL| NULL   | NULL |
|  2 | 1 | /Capture | 2015-11-25 | NULL| NULL   | NULL |
|  3 | 1 | /Test| 2015-11-25 | NULL| NULL   | NULL |
|  4 | 1 | /Webcam  | 2015-11-03 | NULL| NULL   | NULL |
++---+--++-++--+
4 rows in set (0.00 sec)

I really do think that the fake, placeholder, album route is the correct way to
go. It is similar to dropping foreign key checks but it does not leave the
database in an inconsistent state.

-- 
You are receiving this mail because:
You are watching all bug changes.

[digikam] [Bug 363697] Album visible even after deletion [patch]

[Johannes Hirte via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=363697

--- Comment #13 from Johannes Hirte  ---
Patch works for me with MySQL/MariaDB.

-- 
You are receiving this mail because:
You are watching all bug changes.

[digikam] [Bug 363697] Album visible even after deletion [patch]

[Maik Qualmann via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=363697

Maik Qualmann  changed:

   What|Removed |Added

Summary|Album visible even after|Album visible even after
   |deletion|deletion [patch]

-- 
You are receiving this mail because:
You are watching all bug changes.