Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update to RxJava2 #239

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion app/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,6 @@ dependencies {
annotationProcessor libraries.daggerCompiler

implementation libraries.rxJava
releaseImplementation libraries.rxJavaProguardRules
implementation libraries.rxLint

implementation libraries.okHttp
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,19 +3,17 @@
import com.artemzin.qualitymatters.QualityMattersIntegrationRobolectricTestRunner;
import com.artemzin.qualitymatters.api.QualityMattersRestApi;
import com.artemzin.qualitymatters.api.entities.Item;

import okhttp3.mockwebserver.MockResponse;
import okhttp3.mockwebserver.MockWebServer;
import org.junit.After;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
import retrofit2.HttpException;

import java.io.IOException;
import java.util.List;

import okhttp3.mockwebserver.MockResponse;
import okhttp3.mockwebserver.MockWebServer;
import retrofit2.adapter.rxjava.HttpException;

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.fail;

Expand Down Expand Up @@ -58,7 +56,7 @@ public void items_shouldHandleCorrectResponse() {
+ "]"));

// Get items from the API
List<Item> items = qualityMattersRestApi.items().toBlocking().value();
List<Item> items = qualityMattersRestApi.items().blockingGet();

assertThat(items).hasSize(3);

Expand Down Expand Up @@ -88,10 +86,9 @@ public void items_shouldThrowExceptionIfWebServerRespondError() {
mockWebServer.enqueue(new MockResponse().setStatus("HTTP/1.1 " + errorCode + " Not today"));

try {
qualityMattersRestApi.items().toBlocking().value();
qualityMattersRestApi.items().blockingGet();
fail("HttpException should be thrown for error code: " + errorCode);
} catch (RuntimeException expected) {
HttpException httpException = (HttpException) expected.getCause();
} catch (HttpException httpException ) {
assertThat(httpException.code()).isEqualTo(errorCode);
assertThat(httpException.message()).isEqualTo("Not today");
}
Expand Down
12 changes: 5 additions & 7 deletions app/src/main/java/com/artemzin/qualitymatters/api/ApiModule.java
Original file line number Diff line number Diff line change
@@ -1,19 +1,17 @@
package com.artemzin.qualitymatters.api;

import android.support.annotation.NonNull;

import com.artemzin.qualitymatters.BuildConfig;
import com.google.gson.Gson;

import javax.inject.Singleton;

import dagger.Module;
import dagger.Provides;
import okhttp3.OkHttpClient;
import retrofit2.Retrofit;
import retrofit2.adapter.rxjava.RxJavaCallAdapterFactory;
import retrofit2.adapter.rxjava2.RxJava2CallAdapterFactory;
import retrofit2.converter.gson.GsonConverterFactory;

import javax.inject.Singleton;

@Module
public class ApiModule {

Expand All @@ -32,10 +30,10 @@ public ChangeableBaseUrl provideChangeableBaseUrl() {
@Provides @NonNull @Singleton
public QualityMattersRestApi provideQualityMattersApi(@NonNull OkHttpClient okHttpClient, @NonNull Gson gson, @NonNull ChangeableBaseUrl changeableBaseUrl) {
return new Retrofit.Builder()
.baseUrl(changeableBaseUrl)
.baseUrl(changeableBaseUrl.url())
.client(okHttpClient)
.addConverterFactory(GsonConverterFactory.create(gson))
.addCallAdapterFactory(RxJavaCallAdapterFactory.create())
.addCallAdapterFactory(RxJava2CallAdapterFactory.create())
.validateEagerly(BuildConfig.DEBUG) // Fail early: check Retrofit configuration at creation time in Debug build.
.build()
.create(QualityMattersRestApi.class);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,16 +1,14 @@
package com.artemzin.qualitymatters.api;

import android.support.annotation.NonNull;
import okhttp3.HttpUrl;

import java.util.concurrent.atomic.AtomicReference;

import okhttp3.HttpUrl;
import retrofit2.BaseUrl;

/**
* Such implementation allows us easily change base url in the integration and functional tests!
*/
public class ChangeableBaseUrl implements BaseUrl {
public class ChangeableBaseUrl {

@NonNull
private final AtomicReference<HttpUrl> baseUrl;
Expand All @@ -23,7 +21,6 @@ public void setBaseUrl(@NonNull String baseUrl) {
this.baseUrl.set(HttpUrl.parse(baseUrl));
}

@Override @NonNull
public HttpUrl url() {
return baseUrl.get();
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,14 +1,12 @@
package com.artemzin.qualitymatters.api;

import android.support.annotation.NonNull;

import com.artemzin.qualitymatters.api.entities.Item;
import io.reactivex.Single;
import retrofit2.http.GET;

import java.util.List;

import retrofit2.http.GET;
import rx.Single;

public interface QualityMattersRestApi {

@GET("items") @NonNull
Expand Down
Original file line number Diff line number Diff line change
@@ -1,13 +1,12 @@
package com.artemzin.qualitymatters.models;

import android.support.annotation.NonNull;

import com.artemzin.qualitymatters.api.QualityMattersRestApi;
import com.artemzin.qualitymatters.api.entities.Item;
import io.reactivex.Single;

import java.util.List;

import rx.Single;

/**
* Model is not an entity. It's a container for business logic code. M(VC), M(VP), M(VVM).
Expand Down
Original file line number Diff line number Diff line change
@@ -1,33 +1,36 @@
package com.artemzin.qualitymatters.other;

import android.support.annotation.NonNull;
import io.reactivex.disposables.Disposable;
import io.reactivex.functions.Action;

import java.util.concurrent.atomic.AtomicBoolean;

import rx.Subscription;
import rx.functions.Action0;

public class DisposableSubscription implements Subscription {
public class DisposableSubscription implements Disposable {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like this could be solved with Disposables.from instead


@NonNull
private final AtomicBoolean unsubscribed = new AtomicBoolean(false);
private final AtomicBoolean disposed = new AtomicBoolean(false);

@NonNull
private final Action0 disposeAction;
private final Action disposeAction;

public DisposableSubscription(@NonNull Action0 disposeAction) {
public DisposableSubscription(@NonNull Action disposeAction) {
this.disposeAction = disposeAction;
}

@Override
public boolean isUnsubscribed() {
return unsubscribed.get();
public void dispose() {
if (disposed.compareAndSet(false, true)) {
try {
disposeAction.run();
} catch (Exception e) {
throw new RuntimeException(e); // RxJava2 Action throws an exception. Pass it on
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we rethrow here or use the RxJavaPlugins?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please check out the updated implementation which converts FinishAsyncJobSubscription into FinishAsyncJobAction and passes it through to Disposables.fromAction(finishAsyncJobAction)

}
}
}

@Override
public void unsubscribe() {
if (unsubscribed.compareAndSet(false, true)) {
disposeAction.call();
}
public boolean isDisposed() {
return disposed.get();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,10 @@
import android.view.LayoutInflater;
import android.view.View;
import android.view.ViewGroup;

import butterknife.BindView;
import butterknife.ButterKnife;
import butterknife.OnClick;
import butterknife.Unbinder;
import com.artemzin.qualitymatters.QualityMattersApp;
import com.artemzin.qualitymatters.R;
import com.artemzin.qualitymatters.api.entities.Item;
Expand All @@ -22,19 +25,13 @@
import com.artemzin.qualitymatters.ui.presenters.ItemsPresenter;
import com.artemzin.qualitymatters.ui.presenters.ItemsPresenterConfiguration;
import com.artemzin.qualitymatters.ui.views.ItemsView;

import java.util.List;

import javax.inject.Inject;

import butterknife.BindView;
import butterknife.ButterKnife;
import butterknife.OnClick;
import butterknife.Unbinder;
import dagger.Module;
import dagger.Provides;
import dagger.Subcomponent;
import rx.schedulers.Schedulers;
import io.reactivex.schedulers.Schedulers;

import javax.inject.Inject;
import java.util.List;

import static android.support.v7.widget.LinearLayoutManager.VERTICAL;
import static android.view.View.GONE;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
import com.artemzin.qualitymatters.performance.AsyncJobsObserver;
import com.artemzin.qualitymatters.ui.views.ItemsView;

import rx.Subscription;
import io.reactivex.disposables.Disposable;

public class ItemsPresenter extends Presenter<ItemsView> {

Expand Down Expand Up @@ -47,7 +47,7 @@ public void reloadData() {

final AsyncJob asyncJob = asyncJobsObserver.asyncJobStarted("Reload data in ItemsPresenter");

final Subscription subscription = itemsModel
final Disposable disposable = itemsModel
.getItems()
.subscribeOn(presenterConfiguration.ioScheduler())
.subscribe(
Expand Down Expand Up @@ -76,6 +76,6 @@ public void reloadData() {
);

// Prevent memory leak.
unsubscribeOnUnbindView(subscription, new FinishAsyncJobSubscription(asyncJobsObserver, asyncJob));
unsubscribeOnUnbindView(disposable, new FinishAsyncJobSubscription(asyncJobsObserver, asyncJob));
}
}
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
package com.artemzin.qualitymatters.ui.presenters;

import android.support.annotation.NonNull;

import com.google.auto.value.AutoValue;
import io.reactivex.Scheduler;

import rx.Scheduler;

// Such approach allows us configure presenter in runtime without hardcoded values.
// Also, it allows us easily change some parts of the presenter configuration for tests.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,8 @@
import android.support.annotation.CallSuper;
import android.support.annotation.NonNull;
import android.support.annotation.Nullable;

import rx.Subscription;
import rx.subscriptions.CompositeSubscription;
import io.reactivex.disposables.CompositeDisposable;
import io.reactivex.disposables.Disposable;

/**
* Base presenter implementation.
Expand All @@ -15,7 +14,7 @@
public class Presenter<V> {

@NonNull
private final CompositeSubscription subscriptionsToUnsubscribeOnUnbindView = new CompositeSubscription();
private final CompositeDisposable disposablesToDisposeOnUnbindView = new CompositeDisposable();

@Nullable
private volatile V view;
Expand All @@ -36,11 +35,11 @@ protected V view() {
return view;
}

protected final void unsubscribeOnUnbindView(@NonNull Subscription subscription, @NonNull Subscription... subscriptions) {
subscriptionsToUnsubscribeOnUnbindView.add(subscription);
protected final void unsubscribeOnUnbindView(@NonNull Disposable disposable, @NonNull Disposable... disposables) {
disposablesToDisposeOnUnbindView.add(disposable);

for (Subscription s : subscriptions) {
subscriptionsToUnsubscribeOnUnbindView.add(s);
for (Disposable d : disposables) {
disposablesToDisposeOnUnbindView.add(d);
}
}

Expand All @@ -56,6 +55,6 @@ public void unbindView(@NonNull V view) {
}

// Unsubscribe all subscriptions that need to be unsubscribed in this lifecycle state.
subscriptionsToUnsubscribeOnUnbindView.clear();
disposablesToDisposeOnUnbindView.clear();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,12 @@

import com.artemzin.qualitymatters.api.QualityMattersRestApi;
import com.artemzin.qualitymatters.api.entities.Item;

import io.reactivex.Single;
import org.junit.Before;
import org.junit.Test;

import java.util.List;

import rx.Single;

import static java.util.Arrays.asList;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.failBecauseExceptionWasNotThrown;
Expand All @@ -31,7 +29,7 @@ public void getItems_shouldReturnItemsFromQualityMattersRestApi() {
List<Item> items = asList(mock(Item.class), mock(Item.class));
when(qualityMattersRestApi.items()).thenReturn(Single.just(items));

assertThat(itemsModel.getItems().toBlocking().value()).containsExactlyElementsOf(items);
assertThat(itemsModel.getItems().blockingGet()).containsExactlyElementsOf(items);
}

@Test
Expand All @@ -40,7 +38,7 @@ public void getItems_shouldReturnErrorFromQualityMattersRestApi() {
when(qualityMattersRestApi.items()).thenReturn(Single.error(error));

try {
itemsModel.getItems().toBlocking().value();
itemsModel.getItems().blockingGet();
failBecauseExceptionWasNotThrown(RuntimeException.class);
} catch (Exception expected) {
assertThat(expected).isSameAs(error);
Expand Down
Loading