#1167 Cache Retrofit APIs

They use reflection and should therefore be cached.

8 files changed, 155 insertions, 117 deletions

diff --git a/app/src/main/java/it/niedermann/owncloud/notes/importaccount/ImportAccountActivity.java b/app/src/main/java/it/niedermann/owncloud/notes/importaccount/ImportAccountActivity.java
index 6d03327a..c942b345 100644
--- a/app/src/main/java/it/niedermann/owncloud/notes/importaccount/ImportAccountActivity.java
+++ b/app/src/main/java/it/niedermann/owncloud/notes/importaccount/ImportAccountActivity.java
@@ -27,7 +27,7 @@ import it.niedermann.owncloud.notes.databinding.ActivityImportAccountBinding;
 import it.niedermann.owncloud.notes.exception.ExceptionDialogFragment;
 import it.niedermann.owncloud.notes.exception.ExceptionHandler;
 import it.niedermann.owncloud.notes.persistence.CapabilitiesClient;
-import it.niedermann.owncloud.notes.persistence.SSOClient;
+import it.niedermann.owncloud.notes.persistence.ApiProvider;
 import it.niedermann.owncloud.notes.persistence.entity.Account;
 import it.niedermann.owncloud.notes.shared.model.Capabilities;
 import it.niedermann.owncloud.notes.shared.model.IResponseCallback;
@@ -110,7 +110,7 @@ public class ImportAccountActivity extends AppCompatActivity {
                         });
                     } catch (Throwable t) {
                         t.printStackTrace();
-                        SSOClient.invalidateAPICache(ssoAccount);
+                        ApiProvider.invalidateAPICache(ssoAccount);
                         SingleAccountHelper.setCurrentAccount(this, null);
                         runOnUiThread(() -> {
                             restoreCleanState();
diff --git a/app/src/main/java/it/niedermann/owncloud/notes/main/MainActivity.java b/app/src/main/java/it/niedermann/owncloud/notes/main/MainActivity.java
index 47dfca95..d889689d 100644
--- a/app/src/main/java/it/niedermann/owncloud/notes/main/MainActivity.java
+++ b/app/src/main/java/it/niedermann/owncloud/notes/main/MainActivity.java
@@ -75,7 +75,7 @@ import it.niedermann.owncloud.notes.main.navigation.NavigationClickListener;
 import it.niedermann.owncloud.notes.main.navigation.NavigationItem;
 import it.niedermann.owncloud.notes.persistence.CapabilitiesClient;
 import it.niedermann.owncloud.notes.persistence.CapabilitiesWorker;
-import it.niedermann.owncloud.notes.persistence.SSOClient;
+import it.niedermann.owncloud.notes.persistence.ApiProvider;
 import it.niedermann.owncloud.notes.persistence.entity.Account;
 import it.niedermann.owncloud.notes.persistence.entity.Note;
 import it.niedermann.owncloud.notes.shared.model.Capabilities;
@@ -673,7 +673,7 @@ public class MainActivity extends LockedActivity implements NoteClickListener, A
                                     }
                                 });
                             } catch (Throwable e) {
-                                SSOClient.invalidateAPICache(ssoAccount);
+                                ApiProvider.invalidateAPICache(ssoAccount);
                                 // Happens when importing an already existing account the second time
                                 if (e instanceof TokenMismatchException && mainViewModel.getLocalAccountByAccountName(ssoAccount.name) != null) {
                                     Log.w(TAG, "Received " + TokenMismatchException.class.getSimpleName() + " and the given ssoAccount.name (" + ssoAccount.name + ") does already exist in the database. Assume that this account has already been imported.");
diff --git a/app/src/main/java/it/niedermann/owncloud/notes/persistence/ApiProvider.java b/app/src/main/java/it/niedermann/owncloud/notes/persistence/ApiProvider.java
new file mode 100644
index 00000000..b1bca215
--- /dev/null
+++ b/app/src/main/java/it/niedermann/owncloud/notes/persistence/ApiProvider.java
@@ -0,0 +1,137 @@
+package it.niedermann.owncloud.notes.persistence;
+
+import android.content.Context;
+import android.util.Log;
+
+import androidx.annotation.NonNull;
+import androidx.annotation.Nullable;
+import androidx.annotation.WorkerThread;
+
+import com.google.gson.Gson;
+import com.google.gson.GsonBuilder;
+import com.google.gson.JsonDeserializer;
+import com.google.gson.JsonPrimitive;
+import com.google.gson.JsonSerializer;
+import com.nextcloud.android.sso.api.NextcloudAPI;
+import com.nextcloud.android.sso.model.SingleSignOnAccount;
+
+import java.util.Calendar;
+import java.util.HashMap;
+import java.util.Map;
+
+import it.niedermann.owncloud.notes.persistence.sync.CapabilitiesDeserializer;
+import it.niedermann.owncloud.notes.persistence.sync.NotesAPI;
+import it.niedermann.owncloud.notes.persistence.sync.OcsAPI;
+import it.niedermann.owncloud.notes.shared.model.ApiVersion;
+import it.niedermann.owncloud.notes.shared.model.Capabilities;
+import retrofit2.NextcloudRetrofitApiBuilder;
+import retrofit2.Retrofit;
+
+/**
+ * Since creating APIs via {@link Retrofit} uses reflection and {@link NextcloudAPI} <a href="https://github.com/nextcloud/Android-SingleSignOn/issues/120#issuecomment-540069990">is supposed to stay alive as long as possible</a>, those artifacts are going to be cached.
+ * They can be invalidated by using either {@link #invalidateAPICache()} for all or {@link #invalidateAPICache(SingleSignOnAccount)} for a specific {@link SingleSignOnAccount} and will be recreated when they are queried the next time.
+ */
+@WorkerThread
+public class ApiProvider {
+
+    private static final String TAG = ApiProvider.class.getSimpleName();
+
+    private static final String API_ENDPOINT_OCS = "/ocs/v2.php/cloud/";
+
+    private static final Map<String, NextcloudAPI> API_CACHE = new HashMap<>();
+
+    private static final Map<String, OcsAPI> API_CACHE_OCS = new HashMap<>();
+    private static final Map<String, NotesAPI> API_CACHE_NOTES = new HashMap<>();
+
+    /**
+     * An {@link OcsAPI} currently shares the {@link Gson} configuration with the {@link NotesAPI} and therefore divides all {@link Calendar} milliseconds by 1000 while serializing and multiplies values by 1000 during deserialization.
+     */
+    public static synchronized OcsAPI getOcsAPI(@NonNull Context context, @NonNull SingleSignOnAccount ssoAccount) {
+        if (API_CACHE_OCS.containsKey(ssoAccount.name)) {
+            return API_CACHE_OCS.get(ssoAccount.name);
+        }
+        final OcsAPI ocsAPI = new NextcloudRetrofitApiBuilder(getNextcloudAPI(context, ssoAccount), API_ENDPOINT_OCS).create(OcsAPI.class);
+        API_CACHE_OCS.put(ssoAccount.name, ocsAPI);
+        return ocsAPI;
+    }
+
+    /**
+     * In case the {@param preferredApiVersion} changes, call {@link #invalidateAPICache(SingleSignOnAccount)} or {@link #invalidateAPICache()} to make sure that this call returns a {@link NotesAPI} that uses the correct compatibility layer.
+     */
+    public static synchronized NotesAPI getNotesAPI(@NonNull Context context, @NonNull SingleSignOnAccount ssoAccount, @Nullable ApiVersion preferredApiVersion) {
+        if (API_CACHE_NOTES.containsKey(ssoAccount.name)) {
+            return API_CACHE_NOTES.get(ssoAccount.name);
+        }
+        final NotesAPI notesAPI = new NotesAPI(getNextcloudAPI(context, ssoAccount), preferredApiVersion);
+        API_CACHE_NOTES.put(ssoAccount.name, notesAPI);
+        return notesAPI;
+    }
+
+    private static synchronized NextcloudAPI getNextcloudAPI(@NonNull Context context, @NonNull SingleSignOnAccount ssoAccount) {
+        if (API_CACHE.containsKey(ssoAccount.name)) {
+            return API_CACHE.get(ssoAccount.name);
+        } else {
+            Log.v(TAG, "NextcloudRequest account: " + ssoAccount.name);
+            final NextcloudAPI nextcloudAPI = new NextcloudAPI(context.getApplicationContext(), ssoAccount,
+                    new GsonBuilder()
+                            .excludeFieldsWithoutExposeAnnotation()
+                            .registerTypeHierarchyAdapter(Calendar.class, (JsonSerializer<Calendar>) (src, typeOfSrc, ctx) -> new JsonPrimitive(src.getTimeInMillis() / 1_000))
+                            .registerTypeHierarchyAdapter(Calendar.class, (JsonDeserializer<Calendar>) (src, typeOfSrc, ctx) -> {
+                                final Calendar calendar = Calendar.getInstance();
+                                calendar.setTimeInMillis(src.getAsLong() * 1_000);
+                                return calendar;
+                            })
+                            .registerTypeAdapter(Capabilities.class, new CapabilitiesDeserializer())
+                            .create(), new NextcloudAPI.ApiConnectedListener() {
+                @Override
+                public void onConnected() {
+                    Log.i(TAG, "SSO API connected for " + ssoAccount);
+                }
+
+                @Override
+                public void onError(Exception ex) {
+                    ex.printStackTrace();
+                    invalidateAPICache(ssoAccount);
+                }
+            });
+            API_CACHE.put(ssoAccount.name, nextcloudAPI);
+            return nextcloudAPI;
+        }
+    }
+
+    /**
+     * Invalidates the API cache for the given {@param ssoAccount}
+     *
+     * @param ssoAccount the ssoAccount for which the API cache should be cleared.
+     */
+    public static synchronized void invalidateAPICache(@NonNull SingleSignOnAccount ssoAccount) {
+        Log.v(TAG, "Invalidating API cache for " + ssoAccount.name);
+        if (API_CACHE.containsKey(ssoAccount.name)) {
+            final NextcloudAPI nextcloudAPI = API_CACHE.get(ssoAccount.name);
+            if (nextcloudAPI != null) {
+                nextcloudAPI.stop();
+            }
+            API_CACHE.remove(ssoAccount.name);
+        }
+        API_CACHE_NOTES.remove(ssoAccount.name);
+        API_CACHE_OCS.remove(ssoAccount.name);
+    }
+
+    /**
+     * Invalidates the whole API cache for all accounts
+     */
+    public static synchronized void invalidateAPICache() {
+        for (String key : API_CACHE.keySet()) {
+            Log.v(TAG, "Invalidating API cache for " + key);
+            if (API_CACHE.containsKey(key)) {
+                final NextcloudAPI nextcloudAPI = API_CACHE.get(key);
+                if (nextcloudAPI != null) {
+                    nextcloudAPI.stop();
+                }
+                API_CACHE.remove(key);
+            }
+        }
+        API_CACHE_NOTES.clear();
+        API_CACHE_OCS.clear();
+    }
+}
diff --git a/app/src/main/java/it/niedermann/owncloud/notes/persistence/CapabilitiesClient.java b/app/src/main/java/it/niedermann/owncloud/notes/persistence/CapabilitiesClient.java
index 206195c8..8afc64b8 100644
--- a/app/src/main/java/it/niedermann/owncloud/notes/persistence/CapabilitiesClient.java
+++ b/app/src/main/java/it/niedermann/owncloud/notes/persistence/CapabilitiesClient.java
@@ -7,29 +7,23 @@ import androidx.annotation.NonNull;
 import androidx.annotation.Nullable;
 import androidx.annotation.WorkerThread;
 
-import com.nextcloud.android.sso.api.NextcloudAPI;
 import com.nextcloud.android.sso.api.ParsedResponse;
-import com.nextcloud.android.sso.exceptions.NextcloudHttpRequestFailedException;
 import com.nextcloud.android.sso.model.SingleSignOnAccount;
 
-import java.io.IOException;
 import java.util.Map;
 
 import it.niedermann.owncloud.notes.persistence.sync.OcsAPI;
 import it.niedermann.owncloud.notes.shared.model.Capabilities;
-import retrofit2.NextcloudRetrofitApiBuilder;
 
 @WorkerThread
 public class CapabilitiesClient {
 
     private static final String TAG = CapabilitiesClient.class.getSimpleName();
 
-    private static final String API_ENDPOINT_OCS = "/ocs/v2.php/cloud/";
     private static final String HEADER_KEY_ETAG = "ETag";
 
     public static Capabilities getCapabilities(@NonNull Context context, @NonNull SingleSignOnAccount ssoAccount, @Nullable String lastETag) throws Throwable {
-        final NextcloudAPI nextcloudAPI = SSOClient.getNextcloudAPI(context.getApplicationContext(), ssoAccount);
-        final OcsAPI ocsAPI = new NextcloudRetrofitApiBuilder(nextcloudAPI, API_ENDPOINT_OCS).create(OcsAPI.class);
+        final OcsAPI ocsAPI = ApiProvider.getOcsAPI(context, ssoAccount);
         try {
             final ParsedResponse<Capabilities> response = ocsAPI.getCapabilities(lastETag).blockingSingle();
             final Capabilities capabilities = response.getResponse();
diff --git a/app/src/main/java/it/niedermann/owncloud/notes/persistence/NotesRepository.java b/app/src/main/java/it/niedermann/owncloud/notes/persistence/NotesRepository.java
index aa5fe39d..e52b4df4 100644
--- a/app/src/main/java/it/niedermann/owncloud/notes/persistence/NotesRepository.java
+++ b/app/src/main/java/it/niedermann/owncloud/notes/persistence/NotesRepository.java
@@ -178,10 +178,10 @@ public class NotesRepository {
     @WorkerThread
     public void deleteAccount(@NonNull Account account) {
         try {
-            SSOClient.invalidateAPICache(AccountImporter.getSingleSignOnAccount(context, account.getAccountName()));
+            ApiProvider.invalidateAPICache(AccountImporter.getSingleSignOnAccount(context, account.getAccountName()));
         } catch (NextcloudFilesAppAccountNotFoundException e) {
             e.printStackTrace();
-            SSOClient.invalidateAPICache();
+            ApiProvider.invalidateAPICache();
         }
 
         db.getAccountDao().deleteAccount(account);
@@ -582,10 +582,13 @@ public class NotesRepository {
                 }
                 if (apiVersions.length() > 0) {
                     final int updatedRows = db.getAccountDao().updateApiVersion(accountId, apiVersion);
-                    if (updatedRows == 1) {
+                    if (updatedRows == 0) {
+                        Log.d(TAG, "ApiVersion not updated, because it did not change");
+                    } else if (updatedRows == 1) {
                         Log.i(TAG, "Updated apiVersion to \"" + apiVersion + "\" for accountId = " + accountId);
+                        ApiProvider.invalidateAPICache();
                     } else {
-                        Log.e(TAG, "Updated " + updatedRows + " but expected only 1 for accountId = " + accountId + " and apiVersion = \"" + apiVersion + "\"");
+                        Log.w(TAG, "Updated " + updatedRows + " but expected only 1 for accountId = " + accountId + " and apiVersion = \"" + apiVersion + "\"");
                     }
                     return true;
                 } else {
@@ -899,7 +902,7 @@ public class NotesRepository {
                 Log.d(TAG, "No network connection.");
             }
         } catch (NetworkErrorException e) {
-            e.printStackTrace();
+            Log.i(TAG, e.getMessage());
             networkConnected = false;
             isSyncPossible = false;
         }
diff --git a/app/src/main/java/it/niedermann/owncloud/notes/persistence/NotesServerSyncTask.java b/app/src/main/java/it/niedermann/owncloud/notes/persistence/NotesServerSyncTask.java
index 1059e622..88fb44f3 100644
--- a/app/src/main/java/it/niedermann/owncloud/notes/persistence/NotesServerSyncTask.java
+++ b/app/src/main/java/it/niedermann/owncloud/notes/persistence/NotesServerSyncTask.java
@@ -6,7 +6,6 @@ import android.util.Log;
 import androidx.annotation.NonNull;
 
 import com.nextcloud.android.sso.AccountImporter;
-import com.nextcloud.android.sso.api.NextcloudAPI;
 import com.nextcloud.android.sso.api.ParsedResponse;
 import com.nextcloud.android.sso.exceptions.NextcloudFilesAppAccountNotFoundException;
 import com.nextcloud.android.sso.exceptions.NextcloudHttpRequestFailedException;
@@ -49,11 +48,11 @@ abstract class NotesServerSyncTask extends Thread {
     private static final String HEADER_KEY_X_NOTES_API_VERSIONS = "X-Notes-API-Versions";
     private static final String HEADER_KEY_ETAG = "ETag";
     private static final String HEADER_KEY_LAST_MODIFIED = "Last-Modified";
+
+    private NotesAPI notesAPI;
     @NonNull
     private final Context context;
     @NonNull
-    private NotesAPI notesAPI;
-    @NonNull
     private final NotesRepository repo;
     @NonNull
     protected final Account localAccount;
@@ -82,8 +81,7 @@ abstract class NotesServerSyncTask extends Thread {
     public void run() {
         onPreExecute();
 
-        final NextcloudAPI nextcloudAPI = SSOClient.getNextcloudAPI(context.getApplicationContext(), ssoAccount);
-        notesAPI = new NotesAPI(nextcloudAPI, localAccount.getPreferredApiVersion());
+        notesAPI = ApiProvider.getNotesAPI(context, ssoAccount, localAccount.getPreferredApiVersion());
 
         Log.i(TAG, "STARTING SYNCHRONIZATION");
 
@@ -177,7 +175,7 @@ abstract class NotesServerSyncTask extends Thread {
                 }
             } catch (Exception e) {
                 if (e instanceof TokenMismatchException) {
-                    SSOClient.invalidateAPICache(ssoAccount);
+                    ApiProvider.invalidateAPICache(ssoAccount);
                 }
                 exceptions.add(e);
                 success = false;
diff --git a/app/src/main/java/it/niedermann/owncloud/notes/persistence/SSOClient.java b/app/src/main/java/it/niedermann/owncloud/notes/persistence/SSOClient.java
deleted file mode 100644
index d110c8a0..00000000
--- a/app/src/main/java/it/niedermann/owncloud/notes/persistence/SSOClient.java
+++ /dev/null
@@ -1,94 +0,0 @@
-package it.niedermann.owncloud.notes.persistence;
-
-import android.content.Context;
-import android.util.Log;
-
-import androidx.annotation.NonNull;
-import androidx.annotation.WorkerThread;
-
-import com.google.gson.GsonBuilder;
-import com.google.gson.JsonDeserializer;
-import com.google.gson.JsonPrimitive;
-import com.google.gson.JsonSerializer;
-import com.nextcloud.android.sso.api.NextcloudAPI;
-import com.nextcloud.android.sso.model.SingleSignOnAccount;
-
-import java.util.Calendar;
-import java.util.HashMap;
-import java.util.Map;
-
-import it.niedermann.owncloud.notes.persistence.sync.CapabilitiesDeserializer;
-import it.niedermann.owncloud.notes.shared.model.Capabilities;
-
-@SuppressWarnings("WeakerAccess")
-@WorkerThread
-public class SSOClient {
-
-    private static final String TAG = SSOClient.class.getSimpleName();
-
-    private static final Map<String, NextcloudAPI> mNextcloudAPIs = new HashMap<>();
-
-    public static synchronized NextcloudAPI getNextcloudAPI(@NonNull Context appContext, @NonNull SingleSignOnAccount ssoAccount) {
-        if (mNextcloudAPIs.containsKey(ssoAccount.name)) {
-            return mNextcloudAPIs.get(ssoAccount.name);
-        } else {
-            Log.v(TAG, "NextcloudRequest account: " + ssoAccount.name);
-            final NextcloudAPI nextcloudAPI = new NextcloudAPI(appContext, ssoAccount,
-                    new GsonBuilder()
-                            .excludeFieldsWithoutExposeAnnotation()
-                            .registerTypeHierarchyAdapter(Calendar.class, (JsonSerializer<Calendar>) (src, typeOfSrc, context) -> new JsonPrimitive(src.getTimeInMillis() / 1_000))
-                            .registerTypeHierarchyAdapter(Calendar.class, (JsonDeserializer<Calendar>) (src, typeOfSrc, context) -> {
-                                final Calendar calendar = Calendar.getInstance();
-                                calendar.setTimeInMillis(src.getAsLong() * 1_000);
-                                return calendar;
-                            })
-                            .registerTypeAdapter(Capabilities.class, new CapabilitiesDeserializer())
-                            .create(), new NextcloudAPI.ApiConnectedListener() {
-                @Override
-                public void onConnected() {
-                    Log.i(TAG, "SSO API connected for " + ssoAccount);
-                }
-
-                @Override
-                public void onError(Exception ex) {
-                    ex.printStackTrace();
-                    SSOClient.invalidateAPICache(ssoAccount);
-                }
-            });
-            mNextcloudAPIs.put(ssoAccount.name, nextcloudAPI);
-            return nextcloudAPI;
-        }
-    }
-
-    /**
-     * Invalidates thes API cache for the given ssoAccount
-     *
-     * @param ssoAccount the ssoAccount for which the API cache should be cleared.
-     */
-    public static void invalidateAPICache(@NonNull SingleSignOnAccount ssoAccount) {
-        Log.v(TAG, "Invalidating API cache for " + ssoAccount.name);
-        if (mNextcloudAPIs.containsKey(ssoAccount.name)) {
-            final NextcloudAPI nextcloudAPI = mNextcloudAPIs.get(ssoAccount.name);
-            if (nextcloudAPI != null) {
-                nextcloudAPI.stop();
-            }
-            mNextcloudAPIs.remove(ssoAccount.name);
-        }
-    }
-
-    /**
-     * Invalidates the whole API cache for all accounts
-     */
-    public static void invalidateAPICache() {
-        for (String key : mNextcloudAPIs.keySet()) {
-            Log.v(TAG, "Invalidating API cache for " + key);
-            if (mNextcloudAPIs.containsKey(key)) {
-                final NextcloudAPI nextcloudAPI = mNextcloudAPIs.get(key);
-                if (nextcloudAPI != null) {
-                    nextcloudAPI.stop();
-                }
-                mNextcloudAPIs.remove(key);
-            }
-        }
-    }
-}
diff --git a/app/src/main/java/it/niedermann/owncloud/notes/persistence/dao/AccountDao.java b/app/src/main/java/it/niedermann/owncloud/notes/persistence/dao/AccountDao.java
index b0828717..9751805b 100644
--- a/app/src/main/java/it/niedermann/owncloud/notes/persistence/dao/AccountDao.java
+++ b/app/src/main/java/it/niedermann/owncloud/notes/persistence/dao/AccountDao.java
@@ -53,6 +53,6 @@ public interface AccountDao {
     @Query("UPDATE Account SET MODIFIED = :modified WHERE id = :id")
     void updateModified(long id, long modified);
 
-    @Query("UPDATE Account SET APIVERSION = :apiVersion WHERE id = :id")
+    @Query("UPDATE Account SET APIVERSION = :apiVersion WHERE id = :id AND APIVERSION != :apiVersion")
     int updateApiVersion(Long id, String apiVersion);
 }