baloo-widgets: Emit metaDataRequestFinished once per request
ClosedPublic

Authored by michaelh on Jan 26 2018, 11:18 AM.

Details

Summary

Add signal 'dataAvailable' to allow chunkwise data processing
Emit metaDataRequestFinished when request processing is finished
Adapted execution order to ensure loadingFinished is signalled last

CCBUG: 388583

Test Plan

Visual inspection
make test

Diff Detail

Repository
R824 Baloo Widgets
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
michaelh requested review of this revision.Jan 26 2018, 11:18 AM
michaelh created this revision.
michaelh retitled this revision from Emit metaDataRequestFinished once per request to baloo-widgets: Emit metaDataRequestFinished once per request.Jan 26 2018, 11:38 AM
dhaumann added inline comments.
src/filemetadataprovider.h
112

Could you add API documentation there what this signal indicates and when to use it? Typically, if you find code that is documented it is good practice to do the same :-)

michaelh updated this revision to Diff 26071.Jan 27 2018, 4:33 PM

Update signal descriptions

michaelh marked an inline comment as done.Jan 27 2018, 4:38 PM
michaelh added inline comments.
src/filemetadataprovider.cpp
327

Is it guaranteed, that this is emitted before IndexedDataRetriever job is finished?

+1, looks good to me.

src/filemetadataprovider.cpp
327

I don't think so, because IndexedDataRetriever::start() does not block.

michaelh added inline comments.Jan 27 2018, 6:34 PM
src/filemetadataprovider.cpp
327

In that case it might be safer to do

       IndexedDataRetriever *ret = new IndexedDataRetriever(filePath, this);
	​connect(ret, SIGNAL(finished(KJob*)), this, SLOT(slotLoadingFinished(KJob*)));       
       
        insertBasicData();
	​insertEditableData();
	​emit dataAvailable();

​        ret->start();

Because once loadingFinished() is signalled it's over. dataAvailable(); won't get processed anymore.

michaelh updated this revision to Diff 26117.Jan 28 2018, 11:47 AM

Apply suggested changes

michaelh updated this revision to Diff 26120.Jan 28 2018, 12:37 PM

Revert incorrect diff update

michaelh updated this revision to Diff 26129.Jan 28 2018, 2:25 PM

Add Q_ASSERT

michaelh updated this revision to Diff 26131.Jan 28 2018, 2:38 PM
michaelh updated this revision to Diff 26135.Jan 28 2018, 3:03 PM

Correct yet another update error

michaelh updated this revision to Diff 26149.EditedJan 28 2018, 7:54 PM

Change execution order in setFileItem

All tests pass

michaelh planned changes to this revision.Jan 29 2018, 2:09 PM

Needs rebasing

michaelh updated this revision to Diff 26183.Jan 29 2018, 4:56 PM

Merge branch 'master' of git://anongit.kde.org/baloo-widgets
Change execution order in setFileItems

src/filemetadataprovider.cpp
327

Sounds good, but please explain why we are moving this signal in the commit message. This will make easier to git blame this code in the future.

michaelh edited the summary of this revision. (Show Details)Jan 30 2018, 8:25 AM

Changed description
Or did you mean the commit messages of 1e3aa22d15d8 and 0f362a3e0de0?
Also, I've committed the accepted revisions to master not Applications/17.1. Correct?

michaelh updated this revision to Diff 26218.Jan 30 2018, 5:20 PM

Added test for empty file list
Added test for file list with empty url
Simplified path construction

This comment was removed by michaelh.
michaelh updated this revision to Diff 26222.Jan 30 2018, 7:29 PM

Change commit message

michaelh added inline comments.Jan 30 2018, 8:34 PM
src/filemetadatawidget.cpp
119

test with empty items(): This code is reached, but the moc doesn't signal.

michaelh marked 3 inline comments as done.Jan 31 2018, 12:22 AM
This revision is now accepted and ready to land.Feb 3 2018, 5:48 PM
ngraham accepted this revision.Feb 3 2018, 7:40 PM
This revision was automatically updated to reflect the committed changes.