diff options
10 files changed, 61 insertions, 69 deletions
diff --git a/app/build.gradle b/app/build.gradle index 8d8c30e6..226bbb8b 100644 --- a/app/build.gradle +++ b/app/build.gradle @@ -112,10 +112,6 @@ dependencies { // Testing testImplementation 'junit:junit:4.13.2' testImplementation 'org.mockito:mockito-core:3.10.0' - def powermockVersion = "2.0.9" - testImplementation "org.powermock:powermock-core:$powermockVersion" - testImplementation "org.powermock:powermock-module-junit4:$powermockVersion" - testImplementation "org.powermock:powermock-api-mockito2:$powermockVersion" testImplementation 'org.robolectric:robolectric:4.5.1' testImplementation 'androidx.test:core:1.3.0' testImplementation 'androidx.test.ext:junit:1.1.2' 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 37a0bde6..994c652c 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 @@ -121,7 +121,7 @@ public class ImportAccountActivity extends AppCompatActivity { }); } catch (Throwable t) { t.printStackTrace(); - ApiProvider.invalidateAPICache(ssoAccount); + ApiProvider.getInstance().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 26ae3548..ef4ae2fc 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 @@ -674,7 +674,7 @@ public class MainActivity extends LockedActivity implements NoteClickListener, A } }); } catch (Throwable e) { - ApiProvider.invalidateAPICache(ssoAccount); + ApiProvider.getInstance().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 index b1bca215..242dc83f 100644 --- a/app/src/main/java/it/niedermann/owncloud/notes/persistence/ApiProvider.java +++ b/app/src/main/java/it/niedermann/owncloud/notes/persistence/ApiProvider.java @@ -34,19 +34,32 @@ import retrofit2.Retrofit; @WorkerThread public class ApiProvider { - private static final String TAG = ApiProvider.class.getSimpleName(); + private static ApiProvider instance; - private static final String API_ENDPOINT_OCS = "/ocs/v2.php/cloud/"; + private final static String TAG = ApiProvider.class.getSimpleName(); - private static final Map<String, NextcloudAPI> API_CACHE = new HashMap<>(); + private final static String API_ENDPOINT_OCS = "/ocs/v2.php/cloud/"; - private static final Map<String, OcsAPI> API_CACHE_OCS = new HashMap<>(); - private static final Map<String, NotesAPI> API_CACHE_NOTES = new HashMap<>(); + private final Map<String, NextcloudAPI> API_CACHE = new HashMap<>(); + + private final Map<String, OcsAPI> API_CACHE_OCS = new HashMap<>(); + private final Map<String, NotesAPI> API_CACHE_NOTES = new HashMap<>(); + + public static synchronized ApiProvider getInstance() { + if (instance == null) { + instance = new ApiProvider(); + } + return instance; + } + + private ApiProvider() { + // Singleton + } /** * 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) { + public synchronized OcsAPI getOcsAPI(@NonNull Context context, @NonNull SingleSignOnAccount ssoAccount) { if (API_CACHE_OCS.containsKey(ssoAccount.name)) { return API_CACHE_OCS.get(ssoAccount.name); } @@ -58,7 +71,7 @@ public class ApiProvider { /** * 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) { + public 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); } @@ -67,7 +80,7 @@ public class ApiProvider { return notesAPI; } - private static synchronized NextcloudAPI getNextcloudAPI(@NonNull Context context, @NonNull SingleSignOnAccount ssoAccount) { + private synchronized NextcloudAPI getNextcloudAPI(@NonNull Context context, @NonNull SingleSignOnAccount ssoAccount) { if (API_CACHE.containsKey(ssoAccount.name)) { return API_CACHE.get(ssoAccount.name); } else { @@ -104,7 +117,7 @@ public class ApiProvider { * * @param ssoAccount the ssoAccount for which the API cache should be cleared. */ - public static synchronized void invalidateAPICache(@NonNull SingleSignOnAccount ssoAccount) { + public 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); @@ -120,7 +133,7 @@ public class ApiProvider { /** * Invalidates the whole API cache for all accounts */ - public static synchronized void invalidateAPICache() { + public synchronized void invalidateAPICache() { for (String key : API_CACHE.keySet()) { Log.v(TAG, "Invalidating API cache for " + key); if (API_CACHE.containsKey(key)) { 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 b2dd051b..3f27b820 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 @@ -27,7 +27,7 @@ public class CapabilitiesClient { @WorkerThread public static Capabilities getCapabilities(@NonNull Context context, @NonNull SingleSignOnAccount ssoAccount, @Nullable String lastETag) throws Throwable { - final OcsAPI ocsAPI = ApiProvider.getOcsAPI(context, ssoAccount); + final OcsAPI ocsAPI = ApiProvider.getInstance().getOcsAPI(context, ssoAccount); try { final ParsedResponse<OcsResponse<Capabilities>> response = ocsAPI.getCapabilities(lastETag).blockingSingle(); final Capabilities capabilities = response.getResponse().ocs.data; @@ -51,7 +51,7 @@ public class CapabilitiesClient { @WorkerThread @Nullable public static String getDisplayName(@NonNull Context context, @NonNull SingleSignOnAccount ssoAccount) { - final OcsAPI ocsAPI = ApiProvider.getOcsAPI(context, ssoAccount); + final OcsAPI ocsAPI = ApiProvider.getInstance().getOcsAPI(context, ssoAccount); try { final Response<OcsResponse<OcsUser>> userResponse = ocsAPI.getUser(ssoAccount.userId).execute(); if (userResponse.isSuccessful()) { 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 9e553be1..c9598ba6 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 @@ -81,6 +81,7 @@ public class NotesRepository { private static NotesRepository instance; + private final ApiProvider apiProvider; private final ExecutorService executor; private final Context context; private final NotesDatabase db; @@ -134,15 +135,16 @@ public class NotesRepository { public static synchronized NotesRepository getInstance(@NonNull Context context) { if (instance == null) { - instance = new NotesRepository(context, NotesDatabase.getInstance(context.getApplicationContext()), Executors.newCachedThreadPool()); + instance = new NotesRepository(context, NotesDatabase.getInstance(context.getApplicationContext()), Executors.newCachedThreadPool(), ApiProvider.getInstance()); } return instance; } - private NotesRepository(@NonNull final Context context, @NonNull final NotesDatabase db, @NonNull final ExecutorService executor) { + private NotesRepository(@NonNull final Context context, @NonNull final NotesDatabase db, @NonNull final ExecutorService executor, @NonNull ApiProvider apiProvider) { this.context = context.getApplicationContext(); this.db = db; this.executor = executor; + this.apiProvider = apiProvider; this.defaultNonEmptyTitle = NoteUtil.generateNonEmptyNoteTitle("", this.context); this.syncOnlyOnWifiKey = context.getApplicationContext().getResources().getString(R.string.pref_key_wifi_only); @@ -177,10 +179,10 @@ public class NotesRepository { @WorkerThread public void deleteAccount(@NonNull Account account) { try { - ApiProvider.invalidateAPICache(AccountImporter.getSingleSignOnAccount(context, account.getAccountName())); + apiProvider.invalidateAPICache(AccountImporter.getSingleSignOnAccount(context, account.getAccountName())); } catch (NextcloudFilesAppAccountNotFoundException e) { e.printStackTrace(); - ApiProvider.invalidateAPICache(); + apiProvider.invalidateAPICache(); } db.getAccountDao().deleteAccount(account); @@ -582,7 +584,7 @@ public class NotesRepository { Log.d(TAG, "ApiVersion not updated, because it did not change"); } else if (updatedRows == 1) { Log.i(TAG, "Updated apiVersion to \"" + raw + "\" for accountId = " + accountId); - ApiProvider.invalidateAPICache(); + apiProvider.invalidateAPICache(); } else { Log.w(TAG, "Updated " + updatedRows + " but expected only 1 for accountId = " + accountId + " and apiVersion = \"" + raw + "\""); } @@ -779,7 +781,7 @@ public class NotesRepository { syncActive.put(account.getId(), true); try { Log.d(TAG, "... starting now"); - final NotesServerSyncTask syncTask = new NotesServerSyncTask(context, this, account, onlyLocalChanges) { + final NotesServerSyncTask syncTask = new NotesServerSyncTask(context, this, account, onlyLocalChanges, apiProvider) { @Override void onPreExecute() { syncStatus.postValue(true); 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 2a1855ce..4b572bc8 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 @@ -52,6 +52,8 @@ abstract class NotesServerSyncTask extends Thread { private NotesAPI notesAPI; @NonNull + private final ApiProvider apiProvider; + @NonNull private final Context context; @NonNull private final NotesRepository repo; @@ -65,13 +67,14 @@ abstract class NotesServerSyncTask extends Thread { @NonNull protected final ArrayList<Throwable> exceptions = new ArrayList<>(); - NotesServerSyncTask(@NonNull Context context, @NonNull NotesRepository repo, @NonNull Account localAccount, boolean onlyLocalChanges) throws NextcloudFilesAppAccountNotFoundException { + NotesServerSyncTask(@NonNull Context context, @NonNull NotesRepository repo, @NonNull Account localAccount, boolean onlyLocalChanges, @NonNull ApiProvider apiProvider) throws NextcloudFilesAppAccountNotFoundException { super(TAG); this.context = context; this.repo = repo; this.localAccount = localAccount; this.ssoAccount = AccountImporter.getSingleSignOnAccount(context, localAccount.getAccountName()); this.onlyLocalChanges = onlyLocalChanges; + this.apiProvider = apiProvider; } void addCallbacks(Account account, List<ISyncCallback> callbacks) { @@ -82,7 +85,7 @@ abstract class NotesServerSyncTask extends Thread { public void run() { onPreExecute(); - notesAPI = ApiProvider.getNotesAPI(context, ssoAccount, ApiVersionUtil.getPreferredApiVersion(localAccount.getApiVersion())); + notesAPI = apiProvider.getNotesAPI(context, ssoAccount, ApiVersionUtil.getPreferredApiVersion(localAccount.getApiVersion())); Log.i(TAG, "STARTING SYNCHRONIZATION"); @@ -176,7 +179,7 @@ abstract class NotesServerSyncTask extends Thread { } } catch (Exception e) { if (e instanceof TokenMismatchException) { - ApiProvider.invalidateAPICache(ssoAccount); + apiProvider.invalidateAPICache(ssoAccount); } exceptions.add(e); success = false; @@ -267,7 +270,7 @@ abstract class NotesServerSyncTask extends Thread { return true; } } else if (cause.getClass() == NextcloudApiNotRespondingException.class || cause instanceof NextcloudApiNotRespondingException) { - ApiProvider.invalidateAPICache(ssoAccount); + apiProvider.invalidateAPICache(ssoAccount); } } exceptions.add(t); diff --git a/app/src/main/java/it/niedermann/owncloud/notes/persistence/migration/Migration_22_23.java b/app/src/main/java/it/niedermann/owncloud/notes/persistence/migration/Migration_22_23.java index 4addcf29..b6a7494b 100644 --- a/app/src/main/java/it/niedermann/owncloud/notes/persistence/migration/Migration_22_23.java +++ b/app/src/main/java/it/niedermann/owncloud/notes/persistence/migration/Migration_22_23.java @@ -57,7 +57,7 @@ public class Migration_22_23 extends Migration { db.update("ACCOUNT", OnConflictStrategy.REPLACE, values, "ID = ?", new String[]{String.valueOf(cursor.getLong(COLUMN_POSITION_ID))}); } cursor.close(); - ApiProvider.invalidateAPICache(); + ApiProvider.getInstance().invalidateAPICache(); } @Nullable diff --git a/app/src/test/java/it/niedermann/owncloud/notes/persistence/NotesRepositoryTest.java b/app/src/test/java/it/niedermann/owncloud/notes/persistence/NotesRepositoryTest.java index 3611a875..0f48f5ed 100644 --- a/app/src/test/java/it/niedermann/owncloud/notes/persistence/NotesRepositoryTest.java +++ b/app/src/test/java/it/niedermann/owncloud/notes/persistence/NotesRepositoryTest.java @@ -68,9 +68,9 @@ public class NotesRepositoryTest { .allowMainThreadQueries() .build(); - final Constructor<NotesRepository> constructor = NotesRepository.class.getDeclaredConstructor(Context.class, NotesDatabase.class, ExecutorService.class); + final Constructor<NotesRepository> constructor = NotesRepository.class.getDeclaredConstructor(Context.class, NotesDatabase.class, ExecutorService.class, ApiProvider.class); constructor.setAccessible(true); - repo = constructor.newInstance(context, db, MoreExecutors.newDirectExecutorService()); + repo = constructor.newInstance(context, db, MoreExecutors.newDirectExecutorService(), ApiProvider.getInstance()); repo.addAccount("https://äöüß.example.com", "彼得", "彼得@äöüß.example.com", new Capabilities(), null, new IResponseCallback<Account>() { @Override diff --git a/app/src/test/java/it/niedermann/owncloud/notes/persistence/NotesServerSyncTaskTest.java b/app/src/test/java/it/niedermann/owncloud/notes/persistence/NotesServerSyncTaskTest.java index 4c7a2b26..1a108366 100644 --- a/app/src/test/java/it/niedermann/owncloud/notes/persistence/NotesServerSyncTaskTest.java +++ b/app/src/test/java/it/niedermann/owncloud/notes/persistence/NotesServerSyncTaskTest.java @@ -1,16 +1,10 @@ package it.niedermann.owncloud.notes.persistence; import android.content.Context; -import android.graphics.Color; -import android.text.SpannedString; -import android.text.TextUtils; -import android.util.Log; +import android.os.Build; -import androidx.annotation.NonNull; import androidx.arch.core.executor.testing.InstantTaskExecutorRule; -import androidx.core.text.HtmlCompat; -import com.nextcloud.android.sso.AccountImporter; import com.nextcloud.android.sso.api.ParsedResponse; import com.nextcloud.android.sso.exceptions.NextcloudFilesAppAccountNotFoundException; import com.nextcloud.android.sso.model.SingleSignOnAccount; @@ -19,14 +13,12 @@ import org.junit.Before; import org.junit.Rule; import org.junit.Test; import org.junit.runner.RunWith; -import org.mockito.ArgumentMatchers; -import org.powermock.api.mockito.PowerMockito; -import org.powermock.core.classloader.annotations.PrepareForTest; -import org.powermock.modules.junit4.PowerMockRunner; +import org.robolectric.RobolectricTestRunner; +import org.robolectric.annotation.Config; +import java.io.IOException; import java.util.Arrays; import java.util.Calendar; -import java.util.List; import java.util.Map; import io.reactivex.Observable; @@ -34,24 +26,21 @@ import it.niedermann.owncloud.notes.persistence.entity.Account; import it.niedermann.owncloud.notes.persistence.entity.Note; import it.niedermann.owncloud.notes.persistence.sync.NotesAPI; import it.niedermann.owncloud.notes.shared.model.SyncResultStatus; -import it.niedermann.owncloud.notes.shared.util.ApiVersionUtil; import static it.niedermann.owncloud.notes.shared.model.DBStatus.LOCAL_EDITED; import static it.niedermann.owncloud.notes.shared.model.DBStatus.VOID; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyBoolean; -import static org.mockito.ArgumentMatchers.anyInt; import static org.mockito.ArgumentMatchers.anyLong; import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.ArgumentMatchers.argThat; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; -import static org.powermock.api.mockito.PowerMockito.mockStatic; @SuppressWarnings("CallToThreadRun") -@RunWith(PowerMockRunner.class) -@PrepareForTest({ApiProvider.class, AccountImporter.class, TextUtils.class, Log.class, Color.class, ApiVersionUtil.class, HtmlCompat.class}) +@RunWith(RobolectricTestRunner.class) +@Config(sdk = {Build.VERSION_CODES.P}) public class NotesServerSyncTaskTest { @Rule @@ -62,19 +51,13 @@ public class NotesServerSyncTaskTest { private final Account account = mock(Account.class); private final NotesRepository repo = mock(NotesRepository.class); private final NotesAPI notesAPI = mock(NotesAPI.class); + private final ApiProvider apiProvider = mock(ApiProvider.class); @Before - public void setup() throws NextcloudFilesAppAccountNotFoundException { - mockStatic(ApiProvider.class, invocation -> notesAPI); - mockStatic(AccountImporter.class, invocation -> mock(SingleSignOnAccount.class)); - mockStatic(Log.class); - mockStatic(TextUtils.class); - PowerMockito.when(TextUtils.join(anyString(), any(Object[].class))).thenReturn(""); - mockStatic(HtmlCompat.class); - PowerMockito.when(HtmlCompat.fromHtml(anyString(), anyInt())).thenReturn(mock(SpannedString.class)); - mockStatic(ApiVersionUtil.class); - mockStatic(Color.class); - this.task = new NotesServerSyncTask(mock(Context.class), repo, account, false) { + public void setup() throws NextcloudFilesAppAccountNotFoundException, IOException { + when(apiProvider.getNotesAPI(any(), any(), any())).thenReturn(notesAPI); + NotesTestingUtil.mockSingleSignOn(new SingleSignOnAccount(account.getAccountName(), account.getUserName(), "", account.getUrl(), "")); + this.task = new NotesServerSyncTask(mock(Context.class), repo, account, false, apiProvider) { @Override void onPreExecute() { @@ -105,19 +88,14 @@ public class NotesServerSyncTaskTest { when(repo.getAccountById(anyLong())).thenReturn(account); when(repo.getIdMap(anyLong())).thenReturn(Map.of(1000L, 1L, 2000L, 2L)); when(repo.updateIfNotModifiedLocallyAndAnyRemoteColumnHasChanged(anyLong(), anyLong(), anyString(), anyBoolean(), anyString(), anyString(), anyString(), anyString())).thenReturn(1); - mockStatic(ApiProvider.class, invocation -> new NotesAPI(any(), any()) { - @Override - public Observable<ParsedResponse<List<Note>>> getNotes(@NonNull Calendar a, String b) { - return Observable.just(ParsedResponse.of(Arrays.asList( - new Note(0, 1000L, Calendar.getInstance(), "RemoteId is in the idMap, therefore", "This note should be updated locally", "", false, "1", VOID, 0, "", 0), - new Note(0, 3000L, Calendar.getInstance(), "Is a new RemoteId, therefore", "This note should be created locally", "", false, "1", VOID, 0, "", 0) - ))); - } - }); + when(notesAPI.getNotes(any(), any())).thenReturn(Observable.just(ParsedResponse.of(Arrays.asList( + new Note(0, 1000L, Calendar.getInstance(), "RemoteId is in the idMap, therefore", "This note should be updated locally", "", false, "1", VOID, 0, "", 0), + new Note(0, 3000L, Calendar.getInstance(), "Is a new RemoteId, therefore", "This note should be created locally", "", false, "1", VOID, 0, "", 0) + )))); this.task.run(); - verify(repo).addNote(ArgumentMatchers.anyLong(), argThat(argument -> "This note should be created locally".equals(argument.getContent()))); + verify(repo).addNote(anyLong(), argThat(argument -> "This note should be created locally".equals(argument.getContent()))); verify(repo).updateIfNotModifiedLocallyAndAnyRemoteColumnHasChanged(anyLong(), anyLong(), anyString(), anyBoolean(), anyString(), anyString(), argThat("This note should be updated locally"::equals), anyString()); } }
\ No newline at end of file |