[Bug 261417] New: duplicate image counter is not adjusted when removing a duplicate

[bugzilla_noreply]

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

--- Comment #17 from [hidden email] ---
The DBJob for duplicates search album must be called through DuplicatesFinder
maintenance tool, as i commented here :

https://bugs.kde.org/show_bug.cgi?id=372435#c2

This must automatically start a progress manager task, with progress info and
results. It's simple...

Gilles Caulier

--
You are receiving this mail because:
You are the assignee for the bug.

[bugzilla_noreply]

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

--- Comment #18 from Mario Frank <[hidden email]> ---
Hey,

I found all that spots when I extended the duplicates search to the interval
variant. And I did see the contextmenuhelper, too.

And I traced the signal-slot-connections. The image preview view signals the
stacked view that signals the digikam view. This one then signals either the
table or image view  which then delete the selected images.
In both cases, an update of the modified albums is necessary. Since now
multiple SAlbums can be selected and one image can be part of multiple SAlbums,
I can have n SAlbums for update.

Thus, I first have to find out which albums need to be updated. And since I can
jump from an SAlbum with the context menu to a physical album and delete the
duplicate image there, I am not even in the context of the duplicates search.

Am I overseeing something? Is there a good spot to catch which SAlbums were
modified?

--
You are receiving this mail because:
You are the assignee for the bug.

[bugzilla_noreply]

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

--- Comment #19 from [hidden email] ---
>Thus, I first have to find out which albums need to be updated. And since I can >jump from an SAlbum with the context menu to a physical album and delete the >duplicate image there, I am not even in the context of the duplicates search.

Well, it's simple. If one image can be present in more than one SAlbum, all
relevant SAlbum must be updated through DuplicatesFinder tool.

CoreDb class can be your friend here. But as i can see the relevant method is
missing about search album.

Another question : In avanced search tool, we can show items relevant of
complex DB queries. What's happen when i delete an item from a SAlbum in this
view ?

Gilles Caulier

--
You are receiving this mail because:
You are the assignee for the bug.

[bugzilla_noreply]

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

--- Comment #20 from Mario Frank <[hidden email]> ---
(In reply to caulier.gilles from comment #19)
>
> Well, it's simple. If one image can be present in more than one SAlbum, all
> relevant SAlbum must be updated through DuplicatesFinder tool.
>
> CoreDb class can be your friend here. But as i can see the relevant method
> is missing about search album.
>
Okay, I'll try to get that done.

> Another question : In avanced search tool, we can show items relevant of
> complex DB queries. What's happen when i delete an item from a SAlbum in
> this view ?

I will have to look at the code.

Thanks for the hints.

--
You are receiving this mail because:
You are the assignee for the bug.

[bugzilla_noreply]

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

--- Comment #21 from Mario Frank <[hidden email]> ---
(In reply to Mario Frank from comment #20)

> (In reply to caulier.gilles from comment #19)
> >
> > Well, it's simple. If one image can be present in more than one SAlbum, all
> > relevant SAlbum must be updated through DuplicatesFinder tool.
> >
> > CoreDb class can be your friend here. But as i can see the relevant method
> > is missing about search album.
> >
> Okay, I'll try to get that done.
>
> > Another question : In avanced search tool, we can show items relevant of
> > complex DB queries. What's happen when i delete an item from a SAlbum in
> > this view ?
>
> I will have to look at the code.
>
> Thanks for the hints.

Hey Gilles,

The queries of advanced searches do not contain any image ids, the search
should not have to be updated. Was this what you wanted to know?

Nevertheless, I crawled through the source code and have a working solution.
Since I am not sure if it is the best way to solve that point, I will describe
the approach more closely to make my thoughts clear.

If some image (or some images) is deleted, I have to find out whether this
image is part of a duplicates search. I will come to the way back later.
The easiest way is to get all SAlbums that have DuplicateSearch as type.
Filtering them out is easy. Since I wanted to create reusable code, I created a
function that is able to filter all SAlbums by type. I could do that
in-database. But since the SAlbums are already present in the AlbumManager, the
functionality is located best there. Then I get all SAlbums that represent the
duplicate search, get all SAlbums that contain the deleted image(s) (by using
SearchXmlReader) and buffer the ids of the images contained in those albums
(subtracting the deleted ones).

In the next step, I delete those SAlbums. I do that in order to have clean
SAlbums after rescanning for duplicates. Normally, if a duplicates search is
finished, all SAlbums for duplicates are deleted. But I want to delete only the
ones that were updated. And here comes the important spot. If an image is
deleted which reduces a SAlbum to size 1, this SAlbum is not created and
following this, the old SAlbum would not be deleted and would stay in the
duplicates view.

Since I have now all images that need to be rescanned, I can communicate those
images to the DuplicatesFinder. A new search is triggered and the updated
SAlbums are created.

My solution works for every variant for deleting images (context menu, main
menu and delete button). Also, the duplicates search does not need to be in
focus. If it is not, the duplicates search runs in background.

Now back to the part on getting notified about the deleted image. After
crawling through the code, I found out, that all deletion methods use the
imageViewUtilities. There, I have the information which images are deleted.
Thus I send a signal to the AlbumManager to check whether some SAlbums become
invalid. Also, I wire the AlbumManager with a signal to the FindDuplicatesView
that triggers the DuplicatesSearch on the set of images.

Is this a more fitting way to solve that?

Cheers,
Mario

--
You are receiving this mail because:
You are the assignee for the bug.

[bugzilla_noreply]

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

--- Comment #22 from Mario Frank <[hidden email]> ---
Okay, I looked for other ways to signal the AlbumManager about the deleted
images and I did not find performant ways to do this in another way.

Thus, I will upload the patch. Here is the commit message:
"[PATCH] This commit fixes bug 261417. When images are deleted, the
 ImageViewUtilities give the ImageInfos to the DIO. In the ImageViewUtilities,
 the ids of the deleted images are buffered in a list and given to the
 AlbumManager via Signal-Slot-communication. I do that since in database
 layer, only the URIs of the images are present and getting the image ids is
 unnecessary effort. Since the AlbumManager already has all SAlbums, it is
 easy to first get all SAlbums that represent DuplicateSearches (I implemented
 a method to get all SAlbums by type) and then get all those albums that
 contain the deleted images (SearchXmlReader is needed here).

In order to have a good performance and do no unnecessary work, the
AlbumManager unites
all image ids from the SAlbums to update and subtracts the ids of the deleted
images (set operations).
This way, I am able to communicate the set of images to rescan to the
FindDuplicatesView.
Before notifying the FindDuplicatesView, I delete all SAlbums that should be
updated.
The reason: Consider an SAlbum that contained two duplicates - the original
image and the now deleted
image. If I start a normal duplicates search, no SAlbum will be created for the
original image and
the SAlbum will thus not be updated and should not be in the final result set.

Since I now know the ids of the images to rescan, I notify the
FindDuplicatesView.
But now I do not have albums to search in - and in fact this would not really
be performant since
uninvolved images would be scanned. Thus, I extended the DuplicatesFinder with
a new constructor
for searching for duplicates in a set of images. This could be applicable in
other situations, too.
To make this work, I extended the DB-Jobinfo with a switch for update of
duplicates where the
set of images is and not the albums and tags is used for duplicates search.
This way, I can set the
image ids and the update flag in the job info. The DBJob triggers a new variant
of rebuildDuplicates
in haariface where the image ids are taken directly.

With this approach, only the SAlbums that need to be updated are updated.
SAlbums are only lost,
if no duplicates exist due to deletion which is appropriate. Previously not
existing SAlbums cannot be created
since no new images are involved.
"

By the way, I found some flaw that can lead to an unexpected set of SAlbums.
I'll open another file soon.

Cheers,
Mario

--
You are receiving this mail because:
You are the assignee for the bug.

[bugzilla_noreply]

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

Mario Frank <[hidden email]> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #102157|0                           |1
        is obsolete|                            |

--- Comment #23 from Mario Frank <[hidden email]> ---
Created attachment 102381
  --> https://bugs.kde.org/attachment.cgi?id=102381&action=edit
The patch that triggers an update of involved SAlbums on deletion of images.

--
You are receiving this mail because:
You are the assignee for the bug.

[bugzilla_noreply]

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

--- Comment #24 from [hidden email] ---
Mario,

The analysis is excelent and the way to implement the solution sound good.

I will review your patch.

Gilles Caulier

--
You are receiving this mail because:
You are the assignee for the bug.

[bugzilla_noreply]

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

[hidden email] changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|CONFIRMED                   |RESOLVED
   Version Fixed In|                            |5.4.0
      Latest Commit|                            |http://commits.kde.org/digi
                   |                            |kam/2183d3849f857ef1c04cf76
                   |                            |5fa357a211a8757d8
         Resolution|---                         |FIXED

--- Comment #25 from [hidden email] ---
Git commit 2183d3849f857ef1c04cf765fa357a211a8757d8 by Gilles Caulier.
Committed on 22/11/2016 at 12:01.
Pushed by cgilles into branch 'master'.

Next stage to improve FuzzySearch management to apply patch #102381 from Mario
Frank

When images are deleted, the ImageViewUtilities give
the ImageInfos to the DIO. In the ImageViewUtilities,
the ids of the deleted images are buffered in a list and given to the
AlbumManager via Signal-Slot-communication. I do that since in database
layer, only the URIs of the images are present and getting the image ids is
unnecessary effort. Since the AlbumManager already has all SAlbums, it is
easy to first get all SAlbums that represent DuplicateSearches (I implemented
a method to get all SAlbums by type) and then get all those albums that
contain the deleted images (SearchXmlReader is needed here).

In order to have a good performance and do no unnecessary work, the
AlbumManager unites
all image ids from the SAlbums to update and subtracts the ids of the deleted
images (set operations).
This way, I am able to communicate the set of images to rescan to the
FindDuplicatesView.
Before notifying the FindDuplicatesView, I delete all SAlbums that should be
updated.
The reason: Consider an SAlbum that contained two duplicates - the original
image and the now deleted
image. If I start a normal duplicates search, no SAlbum will be created for the
original image and
the SAlbum will thus not be updated and should not be in the final result set.

Since I now know the ids of the images to rescan, I notify the
FindDuplicatesView.
But now I do not have albums to search in - and in fact this would not really
be performant since
uninvolved images would be scanned. Thus, I extended the DuplicatesFinder with
a new constructor
for searching for duplicates in a set of images. This could be applicable in
other situations, too.
To make this work, I extended the DB-Jobinfo with a switch for update of
duplicates where the
set of images is and not the albums and tags is used for duplicates search.
This way, I can set the
image ids and the update flag in the job info. The DBJob triggers a new variant
of rebuildDuplicates
in haariface where the image ids are taken directly.

With this approach, only the SAlbums that need to be updated are updated.
SAlbums are only lost,
if no duplicates exist due to deletion which is appropriate. Previously not
existing SAlbums cannot be created
since no new images are involved.
FIXED-IN: 5.4.0

M  +19   -0    app/items/imageviewutilities.cpp
M  +1    -0    app/items/imageviewutilities.h
M  +63   -0    libs/album/albummanager.cpp
M  +10   -0    libs/album/albummanager.h
M  +17   -7    libs/database/dbjobs/dbjob.cpp
M  +21   -0    libs/database/dbjobs/dbjobinfo.cpp
M  +14   -6    libs/database/dbjobs/dbjobinfo.h
M  +36   -6    libs/database/haar/haariface.cpp
M  +16   -0    libs/database/haar/haariface.h
M  +17   -0    utilities/fuzzysearch/findduplicatesview.cpp
M  +1    -0    utilities/fuzzysearch/findduplicatesview.h
M  +20   -2    utilities/maintenance/duplicatesfinder.cpp
M  +3    -0    utilities/maintenance/duplicatesfinder.h

http://commits.kde.org/digikam/2183d3849f857ef1c04cf765fa357a211a8757d8

--
You are receiving this mail because:
You are the assignee for the bug.

[bugzilla_noreply]

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

[hidden email] changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|Duplicate image counter is  |Duplicate image counter is
                   |not adjusted when removing  |not adjusted when removing
                   |a duplicate                 |a duplicate [patch]

--
You are receiving this mail because:
You are the assignee for the bug.