RFC port to bugzilla REST API
Needs ReviewPublic

Authored by sitter on Mar 8 2019, 2:43 PM.
This revision needs review, but there are no reviewers specified.

Details

Reviewers
None
Summary

I'd love some feedback on the lib api especially. this is the first REST
wrapper I wrote using cpp, so it feels all a bit clunky to me.

https://bugzilla.readthedocs.io/en/5.0/api/

  • new internal lib wrapping the REST API
  • some testing on the lib, should probably be a bit more comprehensive
  • xmlrpc use gone
  • BugzillaManager continues to be a singleton manager for the API
  • ancient cookie system for login is entirely gone, this has been deprecated (or perhaps even removed?) for years

this is functional, but still requires todo cleanups at the very least.
also duplicate querying is pretty inefficient, needs porting to pagination
really.

some of the files are also overloaded with stuff (connection.cpp brrr)

the lib api is split in three pieces:

  1. the connection containing protocol logic
  2. the clients containing routing logic
  3. the models modeling the json blobs into cpp

the clients especially feel a bit awkward, but I think separating endpoint
handling from the models is a worthwhile thing.

clients get a request, give back a job and the intended use is for
the consumer to then connect to the finished signal of the job to then
ask the clients for the final return value of the job.
this is a bit 'meh'. what I really wanted was a Future to be returned
and the caller to connect to the future. unfortunately qfuture is fairly
limited and since qtbase has no Promise implementation I'm rather opting
for the double-function-approach than doing it like we did in the previous
decade and have a gazillion signals.
this way the handling of the job is always uniform and all specific
"handling code" is inside the connected slot rather than the actual
connection.

error handling has been put entirely into exceptions which get raised
when trying to access a finished job. in BugzillaManager they get rescued
and turned into the already existing signals. this simplifies error
handling a fair amount and also makes the error handling code
look uniform across the various API calls we have.

Diff Detail

Repository
R871 DrKonqi
Branch
webservice
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 9624
Build 9642: arc lint + arc unit
sitter created this revision.Mar 8 2019, 2:43 PM
Restricted Application added a project: Plasma. · View Herald TranscriptMar 8 2019, 2:43 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
sitter requested review of this revision.Mar 8 2019, 2:43 PM
sitter updated this revision to Diff 53669.Mar 11 2019, 3:40 PM
  • repair dupe detection - was running on empty list
  • refactor bug fetching to not include comment fetching and port all users of it
  • stop holding all comments of all bugs in memory. we have no use for like 99% of them after parsing
  • rebase on master

considering there's a default connection and the report dialog is bugzilla specific anyway I am kinda leaning towards doing away with bugzillalib entirely. we have little to no advantage from its singletonness

sitter updated this revision to Diff 53888.Mar 14 2019, 2:59 PM

search now searches long description so only get somewhat pertinent bugs come back as results. also implement pagniation of results (ie. only 25 results at a time are queried, this is both much faster to query and to parse). boths somewhat reliant on undocumented features of the webservice api (they are however intentional features since they have explicit code in the webservice code)

sitter updated this revision to Diff 53889.Mar 14 2019, 3:02 PM

redo diff so it maybe doesn't include nonesense

sitter updated this revision to Diff 53953.Mar 15 2019, 1:22 PM

competing api design on commentsclient::getfrombug2

this has some more cruft but looks nicer from a usage POV IMO

it's basically lambda chaining originating in a signal...
instead of return a kjob the getter returns an APIReply.
APIReply is a templated, not-moc'd QObject which wraps an APIJob.
it's a QObject so we can easily place it as a chield under the "context"
object (i.e. the bugzillamanager instance).
the api getter sets a constructor function which knows how to marshal
kjob data into the intended return type, returns the reply to the
consumer and the consumer sets a handler which will get the marshalled
concrete data from the constructor.
the APIReply then connects to the kjob as per usual.
when the job finishes an internal lambda is called which calls the
constructor to get the concrete type and then calls the handler with that

while that looks lovely I am not sure this actually is going to work.
specifically with how exception handling works this would currently issue
the exception in the slot and that is not safe to do AFAIK