mirrors/nextcloud-notes-android
1
0
Fork 0
mirror of https://github.com/nextcloud/notes-android.git synced 2024-11-23 05:16:15 +03:00
Code Issues Projects Releases Packages Wiki Activity

Enhance testability by using Singleton pattern for ApiProvider

Browse source
This commit is contained in:
Stefan Niedermann 2021-05-17 17:07:13 +02:00 committed by Niedermann IT-Dienstleistungen
parent fbaf93865d
commit 25250701c6
10 changed files with 61 additions and 69 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
app/build.gradle
View file

@ -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'

2
app/src/main/java/it/niedermann/owncloud/notes/importaccount/ImportAccountActivity.java
View file

@ -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();

2
app/src/main/java/it/niedermann/owncloud/notes/main/MainActivity.java
View file

@ -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.");

33
app/src/main/java/it/niedermann/owncloud/notes/persistence/ApiProvider.java
View file

@ -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)) {

4
app/src/main/java/it/niedermann/owncloud/notes/persistence/CapabilitiesClient.java
View file

@ -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()) {

14
app/src/main/java/it/niedermann/owncloud/notes/persistence/NotesRepository.java
View file

@ -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);

11
app/src/main/java/it/niedermann/owncloud/notes/persistence/NotesServerSyncTask.java
View file

@ -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);

2
app/src/main/java/it/niedermann/owncloud/notes/persistence/migration/Migration_22_23.java
View file

@ -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

4
app/src/test/java/it/niedermann/owncloud/notes/persistence/NotesRepositoryTest.java
View file

@ -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

54
app/src/test/java/it/niedermann/owncloud/notes/persistence/NotesServerSyncTaskTest.java
View file

@ -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());
}
}