Skip to content

Rewrite FMD Server API in Kotlin

This MR rewrites the communication with FMD Server in Kotlin.

Motivation

The current FMDServerService mixes two responsibilities:

  1. communicating with FMD Server and
  2. being a JobService executing in the background.

We should separate these two. 1. should be handled by a dedicated FMDServerApiRepository that implements the REST API. Thus the idea to do a refactor to pull this out.

Other improvements

Other improvements to the readability/maintainability/robustness of the networking code, while we are at it:

  • The old code used RestHandler.DEFAULT_METHOD and RestHandler.DEFAULT_RESP_METHOD. This has grown historically, and DEFAULT_RESP_METHOD seems to be used whenever an access token is sent along. The name RESP(ONSE) doesn't really convey that though. Thus this naming is confusing to a newcomer skimming the code (it was for me).
    • This MR instead directly uses Method.GET and Method.PUT. This is much clearer to read, making it easier to e.g. implement the API in other languages.
  • The old code doesn't consistently expose the ErrorListener to the UI. The PostListener returns only a simple boolean true/false, instead of the full error (sometimes it also does printStacktrace(), though). This leads to cases where network errors are silently discarded.
    • The new FMDServerApiRepository in this MR requires callers to always pass an onError callback. This makes it much clearer in the UI code whether a network error is handled or intentionally ignored.
  • ATHandler:
    • Naming: I took me until I read the FMD Server code to realise that ATstands for Access Token. => This MR uses the explicit, full name "Access Token".
    • Inconsistency: Sometimes ATHandler is instantiated directly, sometimes indirectly via RestHandler.runWithAT. Sometimes RestHandler is used manually instead of ATHandler. => Streamline all of that.
  • General readability.
    • E.g. compare the old loginOnServer with the new login. Shorter, easier to skim. Also note that the old loginOnServer has inconsistent error handling. The saltHandler doesn't have any error listener (this demonstrates the point above).
    • Everything is now in one single file. Previously there were different Handlers and different Listeners. All this abstraction adds complexity, making the code harder to review, harder to understand, harder to add new feature, harder to maintain.

I completely understand that this has grown historically :) But I do want to clean it up, for my own sake, who has to wants to work on this app.

And since these changes are already quite big and tedious to review, we might as well rewrite to Kotlin.

Merge request reports