Questions regarding code design choices for Akonadi resource

[Daniel Vrátil]

On Monday, 29 June 2020 13:03:58 CEST Shashwat Jolly wrote:
> Hey everyone!
> 
> As you may know, I'm working on a new resource for Akonadi that would
> enable syncing EteSync contacts, calendar and tasks to Kontact. I have
> implemented contacts/address books sync [0], and will now start working to
> extend functionality to calendars and tasks. I have some questions to ask
> before I start working on that. Here they are:
> 
> 1. As this is a network backend resource, should *all* network calls be in
> async KJobs?

It is preferred, yes. On one hand, Akonadi resources /can/ block, but it is 
better when they don't. Jobs provides better encapsulation of state.

> Currently, I have two async jobs, each of which may have to fetch a lot of
> data (retrieveCollections() and retrieveItems()). I understand that network
> calls may take indefinite time to complete, and so async jobs make sense.
> However, I have a lot of one-line network function calls in my code too
> (Example: see [1]). Should I implement jobs for those too? I'm not able to
> decide on the trade-off between a convoluted implementation (with jobs),
> and the current in-line calls, which I'm not sure cause trouble or not.

However, note that your jobs are not really asynchronous. As an example, let's 
take JournalsFetchJob: when start() is called, it starts a QTimer, which will 
invoke `JournalsFetchJob::fetchJournals`. This is indeed asynchronous in the 
sense that the `start()` method will return immediatelly and `fetchJournals()` 
will be called from the main event loop. However this function then performs a 
synchronous `etesync_journal_manager_list` call to network, which will block 
the event loop, so in fact this is /blocking/.

If you want it to be truly asynchronous (that is, the main event loop is never 
blocked), you need to run the network requests in a thread. The question is, 
whether the Rust bindings/API of the etesync library is thread safe. That's a 
question at Tom.

I think for version 1, doing the blocking calls is fine. Right now I wouldn't 
waste time on trying to get threading right.

I still think that wrapping the logic into more threads is the preferred way 
to go.

> 2. Is there such a thing as too much abstraction? 

Absolutely :-)

> I sometimes have the
> choice between making interfaces for function calls simple, but passing
> extra information to functions vs only taking relevant parameters as input
> to the functions. This is again a tradeoff between a simplified interface
> and specificity. 

C++ gives you another tool to balance the tradeoff: inheritance. Put common 
code into EteSyncBaseJob and then derive your jobs from that, pushing shared 
code into the base class.

Alternatively you can use a more functional programming approach by using 
higher-order functions.

> The extra information is passed by reference, otherwise
> the choice would be easy. For example, I could make all job interfaces
> simple by only passing a reference to the EteSyncClientState object, but it
> has a lot of extra information and it hides the parameters the job actually
> needs.

The purpose of the job/function should be clear from its name, rather than 
arguments it takes. 

It's OK to pass EteSyncClientState to all the jobs. After all, its the Job's 
job (hehe) to know what information it needs from EteSyncClientState in order 
to do what it is supposed to do, rather than the caller needing to know what 
the Job needs. 

/Dan

> It would be great to have opinions on these queries.
> 
> [0]
> https://invent.kde.org/sjolly/kdepim-runtime/-/tree/etesyncResource/resource
> s/etesync [1]
> https://invent.kde.org/sjolly/kdepim-runtime/-/blob/etesyncResource/resource
> s/etesync/etesyncresource.cpp#L181
> 
> Thanks


-- 
Daniel Vrátil
www.dvratil.cz | dvratil at kde.org
IRC: dvratil on Freenode (#kde, #kontact, #akonadi, #fedora-kde)

GPG Key: 0x4D69557AECB13683
Fingerprint: 0ABD FA55 A4E6 BEA9 9A83 EA97 4D69 557A ECB1 3683
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: This is a digitally signed message part.
URL: <http://mail.kde.org/pipermail/kde-pim/attachments/20200629/67deb2d0/attachment.sig>