Differential D18672

Properly clean up scopes when deregistering a subscriber.
ClosedPublic
Actions

Authored by dfaure on Feb 2 2019, 3:18 PM.

Details

Reviewers

dvratil

Commits

R165:a4ff4ce80087: Properly clean up scopes when deregistering a subscriber.

Summary

It was done "by hand", for only a few setters; use apply() to do this
consistently for all of them.

New unittest notificationmanagertest
This requires adding a special mode for unittests in AkThread, to be
able to call methods in NotificationManager from the main thread.

Test Plan

bin/notificationmanagertest

Diff Detail

Repository

R165 Akonadi

Lint

Automatic diff as part of commit; lint not applicable.

Unit

Automatic diff as part of commit; unit tests not applicable.

dfaure requested review of this revision.Feb 2 2019, 3:18 PM

dfaure created this revision.

Harbormaster completed remote builds in B7782: Diff 50719.Feb 2 2019, 3:18 PM

Looks good, just one small change regarding the thread/no-thread handling.

src/server/notificationmanager.cpp
56	Probably would be safer to move the`if` into `AkThread::quitThread()` and make it a no-op if `startMode` is `NoThread`.

This revision now requires changes to proceed.Feb 2 2019, 5:02 PM

dfaure added inline comments.Feb 2 2019, 5:12 PM

src/server/notificationmanager.cpp
56	Yeah, I thought about that. I didn't do it because it requires making `startMode` a member variable there, but indeed this seems like the cleanest solution, I'll do that.

Add m_startMode to AkThread to make quitThread() a no-op if NoThread.

Restricted Application added a project: KDE PIM. · View Herald TranscriptFeb 2 2019, 5:22 PM

Harbormaster completed remote builds in B7790: Diff 50732.Feb 2 2019, 5:22 PM

Thanks!

This revision is now accepted and ready to land.Feb 5 2019, 12:01 PM

Closed by commit R165:a4ff4ce80087: Properly clean up scopes when deregistering a subscriber. (authored by dfaure). · Explain WhyFeb 5 2019, 8:24 PM

This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

		Path
M		autotests/server/CMakeLists.txt (1 line)
A	M	autotests/server/notificationmanagertest.cpp (157 lines)
M		src/server/akthread.h (5 lines)
M		src/server/akthread.cpp (19 lines)
M		src/server/notificationmanager.h (2 lines)
M		src/server/notificationmanager.cpp (5 lines)
M		src/server/notificationsubscriber.cpp (23 lines)

Diff	ID	Base	Description	Created	Lint	Unit
Base			Base
Diff 1	50719	7a564c6		Feb 2 2019, 3:18 PM	★	★
Diff 2	50732	a646904	Add m_startMode to AkThread to make quitThread() a no-op if NoThread.	Feb 2 2019, 5:22 PM	★	★
Diff 3	50984	87a155b	R165:a4ff4ce80087933b6b5ca27d81c0d0de45e63d58	Feb 5 2019, 8:24 PM	★	★