port to bugzilla REST API
ClosedPublic

Authored by sitter on Mar 8 2019, 2:43 PM.

Details

Summary

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

the old API is deprecated. the new API is RESTy and much easier to use
plus also has some efficiency boosts.

this commit introduces a new supporting library to wrap the REST API
for easy consumption in drkonqi.

  • new internal lib wrapping the REST API
  • unit testing of the lib
  • 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
  • duplicate querying is pretty inefficient, needs porting to pagination, but is not worse than what it was before. in fact, it's faster than before out of the box!

the lib api is split in three pieces:

  1. the connection containing protocol logic
  2. the clients containing routing logic

2.1. command models that are basically POSIX-style argument structs which can serialize into json

  1. the models deserializing the json API replies back into cpp

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.
the uniform representation of errors on the REST side thus always has
uniform representation on the c++ side.

Diff Detail

Repository
R871 DrKonqi
Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 9333
Build 9351: 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

sitter updated this revision to Diff 60058.Jun 19 2019, 2:45 PM
  • refactor
  • move clients to subdir
  • catch all exceptions
  • don't incorrectly init the static
  • cleanup includes
  • more tests and less asserts
  • litter--
  • split commands
  • throw out a whole bunch of warnings and bring up coverage
sitter updated this revision to Diff 60059.Jun 19 2019, 2:53 PM

clean up diff maybe?

sitter updated this revision to Diff 60060.Jun 19 2019, 2:55 PM

diff cleanup try 2

sitter updated this revision to Diff 60061.Jun 19 2019, 2:56 PM

cleanup try 3

apol accepted this revision.Jun 20 2019, 10:39 AM
apol added a subscriber: apol.

Looks good!

This revision is now accepted and ready to land.Jun 20 2019, 10:39 AM
sitter retitled this revision from RFC port to bugzilla REST API to port to bugzilla REST API.Jun 25 2019, 12:25 PM
sitter edited the summary of this revision. (Show Details)
sitter updated this revision to Diff 60693.Jun 26 2019, 3:01 PM

lots of warnings fixed & polish & move away from cmake hacks & test updates & new qdebug category for api

This revision was automatically updated to reflect the committed changes.