You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@taverna.apache.org by Hiteshgautam01 <gi...@git.apache.org> on 2018/02/19 10:32:38 UTC
[GitHub] incubator-taverna-mobile pull request #56: Migration RxJava 1 -> RxJava2
GitHub user Hiteshgautam01 opened a pull request:
https://github.com/apache/incubator-taverna-mobile/pull/56
Migration RxJava 1 -> RxJava2
Please make sure these boxes are checked before submitting your pull request - thanks!
- [ ] Apply the `AndroidStyle.xml` style template to your code in Android Studio.
- [ ] Run the checks with `./gradlew check` to make sure you didn't break anything
- [ ] If you have multiple commits please combine them into one commit by squashing them.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/Hiteshgautam01/incubator-taverna-mobile RxJava2
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-taverna-mobile/pull/56.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #56
----
commit 573cc4a4721bb09797bced075f3b60c9a41111ec
Author: Hitesh Gautam <ga...@...>
Date: 2018-02-15T19:52:07Z
Adding RxJava 2.0 dependencies
commit 3e921aca92dee1f69adcd82d43484bf6255ddd7a
Author: Hitesh Gautam <ga...@...>
Date: 2018-02-17T13:38:26Z
Feat: Migrating to RxJava2
commit 9ae9186588297ba90b6697588b6c5862eaed6697
Author: Hitesh Gautam <ga...@...>
Date: 2018-02-19T10:28:18Z
fix: Announcement test
----
---
[GitHub] incubator-taverna-mobile pull request #56: Migration RxJava 1 -> RxJava2
Posted by therajanmaurya <gi...@git.apache.org>.
Github user therajanmaurya commented on a diff in the pull request:
https://github.com/apache/incubator-taverna-mobile/pull/56#discussion_r169136686
--- Diff: app/src/main/java/org/apache/taverna/mobile/ui/playerlogin/PlayerLoginPresenter.java ---
@@ -25,24 +25,24 @@
import org.apache.taverna.mobile.data.DataManager;
import org.apache.taverna.mobile.ui.base.BasePresenter;
+import io.reactivex.android.schedulers.AndroidSchedulers;
+import io.reactivex.disposables.CompositeDisposable;
+import io.reactivex.observers.DisposableObserver;
+import io.reactivex.schedulers.Schedulers;
import okhttp3.ResponseBody;
-import retrofit2.adapter.rxjava.HttpException;
-import rx.Observer;
-import rx.Subscription;
-import rx.android.schedulers.AndroidSchedulers;
-import rx.schedulers.Schedulers;
-
+import retrofit2.HttpException;
public class PlayerLoginPresenter extends BasePresenter<PlayerLoginMvpView> {
private static final String TAG = PlayerLoginPresenter.class.getSimpleName();
private DataManager mDataManager;
--- End diff --
Remove blank line.
---
[GitHub] incubator-taverna-mobile pull request #56: Migration RxJava 1 -> RxJava2
Posted by therajanmaurya <gi...@git.apache.org>.
Github user therajanmaurya commented on a diff in the pull request:
https://github.com/apache/incubator-taverna-mobile/pull/56#discussion_r169134086
--- Diff: app/src/main/java/org/apache/taverna/mobile/ui/login/LoginPresenter.java ---
@@ -25,21 +25,23 @@
import org.apache.taverna.mobile.data.model.User;
import org.apache.taverna.mobile.ui.base.BasePresenter;
-import rx.Observer;
-import rx.Subscription;
-import rx.android.schedulers.AndroidSchedulers;
-import rx.schedulers.Schedulers;
+import io.reactivex.android.schedulers.AndroidSchedulers;
+import io.reactivex.disposables.CompositeDisposable;
+import io.reactivex.observers.DisposableObserver;
+import io.reactivex.schedulers.Schedulers;
+
public class LoginPresenter extends BasePresenter<LoginMvpView> {
public final String LOG_TAG = getClass().getSimpleName();
private DataManager mDataManager;
--- End diff --
Remove this blank like.
---
[GitHub] incubator-taverna-mobile pull request #56: Migration RxJava 1 -> RxJava2
Posted by therajanmaurya <gi...@git.apache.org>.
Github user therajanmaurya commented on a diff in the pull request:
https://github.com/apache/incubator-taverna-mobile/pull/56#discussion_r169134000
--- Diff: app/src/main/java/org/apache/taverna/mobile/ui/favouriteworkflowdetail/FavouriteWorkflowDetailPresenter.java ---
@@ -135,22 +130,26 @@ public void onError(Throwable e) {
}
@Override
- public void onNext(License license) {
- getMvpView().showLicense(license);
+ public void onComplete() {
+ getMvpView().showLicenseProgress(false);
}
}));
}
public void setFavourite(String id) {
-
-
- mCompositeSubscription.add(mDataManager.setFavoriteWorkflow(id)
+ checkViewAttached();
+ compositeDisposable.add(mDataManager.setFavoriteWorkflow(id)
.observeOn(AndroidSchedulers.mainThread())
.subscribeOn(Schedulers.io())
- .subscribe(new Observer<Boolean>() {
+ .subscribeWith(new DisposableObserver<Boolean>() {
@Override
- public void onCompleted() {
-
+ public void onNext(Boolean b) {
--- End diff --
This Boolean `b` name should be `favoriteStatus`
---
[GitHub] incubator-taverna-mobile pull request #56: Migration RxJava 1 -> RxJava2
Posted by therajanmaurya <gi...@git.apache.org>.
Github user therajanmaurya commented on a diff in the pull request:
https://github.com/apache/incubator-taverna-mobile/pull/56#discussion_r169137816
--- Diff: app/src/main/java/org/apache/taverna/mobile/ui/workflowrun/WorkflowRunPresenter.java ---
@@ -31,50 +31,48 @@
import java.io.IOException;
import java.io.InputStreamReader;
+import io.reactivex.Observable;
+import io.reactivex.ObservableSource;
+import io.reactivex.android.schedulers.AndroidSchedulers;
+import io.reactivex.disposables.CompositeDisposable;
+import io.reactivex.functions.Function;
+import io.reactivex.observers.DisposableObserver;
+import io.reactivex.schedulers.Schedulers;
import okhttp3.MediaType;
import okhttp3.RequestBody;
import okhttp3.ResponseBody;
-import rx.Observable;
-import rx.Observer;
-import rx.Subscription;
-import rx.android.schedulers.AndroidSchedulers;
-import rx.functions.Func1;
-import rx.schedulers.Schedulers;
public class WorkflowRunPresenter extends BasePresenter<WorkflowRunMvpView> {
private static final String TAG = WorkflowRunPresenter.class.getSimpleName();
- private final DataManager mDataManager;
- private Subscription mSubscriptions;
+ private final DataManager mDataManager;
+ private CompositeDisposable compositeDisposable;
public WorkflowRunPresenter(DataManager dataManager) {
mDataManager = dataManager;
-
+ compositeDisposable = new CompositeDisposable();
}
@Override
public void attachView(WorkflowRunMvpView mvpView) {
-
super.attachView(mvpView);
-
}
@Override
public void detachView() {
super.detachView();
- if (mSubscriptions != null) mSubscriptions.unsubscribe();
+ compositeDisposable.clear();
}
-
public void runWorkflow(String contentURL) {
- if (mSubscriptions != null) mSubscriptions.unsubscribe();
+ compositeDisposable.clear();
--- End diff --
don't need to clear the Disposable, as we are using `CompositeDisposable` It manages every subscription we don't have to worry about, any subscription will drop or mix. clear only in detachView.
---
[GitHub] incubator-taverna-mobile pull request #56: Migration RxJava 1 -> RxJava2
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/incubator-taverna-mobile/pull/56
---
[GitHub] incubator-taverna-mobile pull request #56: Migration RxJava 1 -> RxJava2
Posted by sagar15795 <gi...@git.apache.org>.
Github user sagar15795 commented on a diff in the pull request:
https://github.com/apache/incubator-taverna-mobile/pull/56#discussion_r169214143
--- Diff: app/src/main/java/org/apache/taverna/mobile/ui/workflowdetail/WorkflowDetailPresenter.java ---
@@ -155,28 +155,21 @@ public void onError(Throwable e) {
}
@Override
- public void onNext(Boolean b) {
- if (b) {
- getMvpView().setFavouriteIcon();
- } else {
- getMvpView().showErrorSnackBar("Something went wrong please try after" +
- "sometime");
- }
+ public void onComplete() {
}
}));
}
public void getFavourite(String id) {
-
-
- mCompositeSubscription.add(mDataManager.getFavoriteWorkflow(id)
+ checkViewAttached();
+ compositeDisposable.add(mDataManager.getFavoriteWorkflow(id)
.observeOn(AndroidSchedulers.mainThread())
.subscribeOn(Schedulers.io())
- .subscribe(new Observer<Boolean>() {
+ .subscribeWith(new DisposableObserver<Boolean>() {
@Override
- public void onCompleted() {
-
+ public void onNext(Boolean b) {
--- End diff --
This Boolean `b` name should be `favoriteStatus`
---
[GitHub] incubator-taverna-mobile pull request #56: Migration RxJava 1 -> RxJava2
Posted by therajanmaurya <gi...@git.apache.org>.
Github user therajanmaurya commented on a diff in the pull request:
https://github.com/apache/incubator-taverna-mobile/pull/56#discussion_r169136620
--- Diff: app/src/main/java/org/apache/taverna/mobile/ui/myworkflows/MyWorkflowPresenter.java ---
@@ -102,23 +101,20 @@ public void onError(Throwable e) {
}
@Override
- public void onNext(Workflow workflow) {
- getMvpView().showWorkflow(workflow);
+ public void onComplete() {
+ getMvpView().showProgressbar(false);
+ getMvpView().checkWorkflowSize();
--- End diff --
Move these two lines in onNext
---
[GitHub] incubator-taverna-mobile pull request #56: Migration RxJava 1 -> RxJava2
Posted by therajanmaurya <gi...@git.apache.org>.
Github user therajanmaurya commented on a diff in the pull request:
https://github.com/apache/incubator-taverna-mobile/pull/56#discussion_r169132417
--- Diff: app/src/main/java/org/apache/taverna/mobile/data/local/PreferencesHelper.java ---
@@ -155,9 +158,15 @@ private void setUserAvatar(String userAvatar) {
}
public Observable<User> saveUserDetail(final User user) {
- return Observable.defer(new Func0<Observable<User>>() {
+ return Observable.defer(new Callable<ObservableSource<? extends User>>() {
+ /**
+ * Computes a result, or throws an exception if unable to do so.
+ *
+ * @return computed result
+ * @throws Exception if unable to compute a result
+ */
--- End diff --
Please change the return type in the comment.
---
[GitHub] incubator-taverna-mobile pull request #56: Migration RxJava 1 -> RxJava2
Posted by therajanmaurya <gi...@git.apache.org>.
Github user therajanmaurya commented on a diff in the pull request:
https://github.com/apache/incubator-taverna-mobile/pull/56#discussion_r169131194
--- Diff: app/build.gradle ---
@@ -82,11 +88,8 @@ dependencies {
compile "com.jakewharton:butterknife:$rootProject.butterKnifeVersion"
annotationProcessor "com.jakewharton:butterknife-compiler:$rootProject.butterKnifeVersion"
-
- compile "io.reactivex:rxandroid:1.2.0"
-// Because RxAndroid releases are few and far between, it is recommended you also
-// explicitly depend on RxJava's latest version for bug fixes and new features.
- compile "io.reactivex:rxjava:1.1.5"
+ compile "io.reactivex.rxjava2:rxjava:2.0.1"
+ compile "io.reactivex.rxjava2:rxandroid:2.0.1"
--- End diff --
Please make `rxJavaVersion = 2.0.1` and `rxAndroidVersion = 2.0.1` in project build.gradle and use here as we already using.
---
[GitHub] incubator-taverna-mobile pull request #56: Migration RxJava 1 -> RxJava2
Posted by therajanmaurya <gi...@git.apache.org>.
Github user therajanmaurya commented on a diff in the pull request:
https://github.com/apache/incubator-taverna-mobile/pull/56#discussion_r169137166
--- Diff: app/src/main/java/org/apache/taverna/mobile/ui/workflow/WorkflowPresenter.java ---
@@ -111,33 +111,36 @@ public void onError(Throwable e) {
}
@Override
- public void onNext(String s) {
- getMvpView().performSearch(s);
- if (!TextUtils.isEmpty(s)) {
- searchWorkflow(1, s);
- }
+ public void onComplete() {
+
}
- });
- mSubscriptions.add(mSearchViewSubscription);
+ }));
+ compositeDisposable.add(compositeDisposable);
--- End diff --
As we are using `CompositeDisposable`, Please remove this line. this line is making no sense.
---
[GitHub] incubator-taverna-mobile pull request #56: Migration RxJava 1 -> RxJava2
Posted by therajanmaurya <gi...@git.apache.org>.
Github user therajanmaurya commented on a diff in the pull request:
https://github.com/apache/incubator-taverna-mobile/pull/56#discussion_r169138137
--- Diff: app/src/test/java/org/apache/taverna/mobile/utils/RxSchedulersOverrideRule.java ---
@@ -4,52 +4,55 @@
import org.junit.runner.Description;
import org.junit.runners.model.Statement;
-import rx.Scheduler;
-import rx.android.plugins.RxAndroidPlugins;
-import rx.android.plugins.RxAndroidSchedulersHook;
-import rx.plugins.RxJavaPlugins;
-import rx.plugins.RxJavaSchedulersHook;
-import rx.schedulers.Schedulers;
+import java.util.concurrent.Callable;
+import io.reactivex.Scheduler;
+import io.reactivex.android.plugins.RxAndroidPlugins;
+import io.reactivex.functions.Function;
+import io.reactivex.plugins.RxJavaPlugins;
+import io.reactivex.schedulers.Schedulers;
-public class RxSchedulersOverrideRule implements TestRule {
-
- private final RxJavaSchedulersHook mRxJavaSchedulersHook = new RxJavaSchedulersHook() {
- @Override
- public Scheduler getIOScheduler() {
- return Schedulers.immediate();
- }
-
- @Override
- public Scheduler getNewThreadScheduler() {
- return Schedulers.immediate();
- }
- };
- private final RxAndroidSchedulersHook mRxAndroidSchedulersHook = new RxAndroidSchedulersHook() {
- @Override
- public Scheduler getMainThreadScheduler() {
- return Schedulers.immediate();
- }
- };
+public class RxSchedulersOverrideRule implements TestRule {
+ private final Function<Callable<Scheduler>, Scheduler> mRxAndroidSchedulersHook =
--- End diff --
it's great you started working on Unit test too and thanks for fixing the previous test with Rxjava 2.
---
[GitHub] incubator-taverna-mobile pull request #56: Migration RxJava 1 -> RxJava2
Posted by therajanmaurya <gi...@git.apache.org>.
Github user therajanmaurya commented on a diff in the pull request:
https://github.com/apache/incubator-taverna-mobile/pull/56#discussion_r169133578
--- Diff: app/src/main/java/org/apache/taverna/mobile/ui/favouriteworkflowdetail/FavouriteWorkflowDetailPresenter.java ---
@@ -39,14 +39,12 @@
private DataManager mDataManager;
--- End diff --
Remove this blank line
---
[GitHub] incubator-taverna-mobile pull request #56: Migration RxJava 1 -> RxJava2
Posted by therajanmaurya <gi...@git.apache.org>.
Github user therajanmaurya commented on a diff in the pull request:
https://github.com/apache/incubator-taverna-mobile/pull/56#discussion_r169131553
--- Diff: app/src/main/java/org/apache/taverna/mobile/data/local/DBHelper.java ---
@@ -46,23 +46,21 @@ public DBHelper() {
@Nullable
public Observable<Workflows> syncWorkflows(final Workflows workflows) {
- return Observable.create(new Observable.OnSubscribe<Workflows>() {
+ return Observable.defer(new Callable<ObservableSource<? extends Workflows>>() {
--- End diff --
Good work for using `Observable.defer(...)`
---
[GitHub] incubator-taverna-mobile pull request #56: Migration RxJava 1 -> RxJava2
Posted by therajanmaurya <gi...@git.apache.org>.
Github user therajanmaurya commented on a diff in the pull request:
https://github.com/apache/incubator-taverna-mobile/pull/56#discussion_r169133509
--- Diff: app/src/main/java/org/apache/taverna/mobile/ui/favouriteworkflow/FavouriteWorkflowsPresenter.java ---
@@ -84,19 +84,18 @@ public void onNext(List<Workflow> workflowList) {
getMvpView().showEmptyWorkflow();
}
}
- });
-
+ }));
}
-
public void attachSearchHandler(SearchView searchView) {
+ checkViewAttached();
RxSearch.fromSearchView(searchView)
.debounce(300, TimeUnit.MILLISECONDS)
.observeOn(AndroidSchedulers.mainThread())
- .subscribe(new Subscriber<String>() {
+ .subscribeWith(new DisposableObserver<String>() {
@Override
- public void onCompleted() {
-
+ public void onNext(String s) {
--- End diff --
Please change `s` with specific naming convention, I am sure It is search query so It's name can be `searchQuery`
---
[GitHub] incubator-taverna-mobile pull request #56: Migration RxJava 1 -> RxJava2
Posted by therajanmaurya <gi...@git.apache.org>.
Github user therajanmaurya commented on a diff in the pull request:
https://github.com/apache/incubator-taverna-mobile/pull/56#discussion_r169132847
--- Diff: app/src/main/java/org/apache/taverna/mobile/ui/anouncements/AnnouncementPresenter.java ---
@@ -26,23 +26,21 @@
import org.apache.taverna.mobile.data.model.DetailAnnouncement;
import org.apache.taverna.mobile.ui.base.BasePresenter;
-import rx.Observer;
-import rx.android.schedulers.AndroidSchedulers;
-import rx.schedulers.Schedulers;
-import rx.subscriptions.CompositeSubscription;
-
+import io.reactivex.android.schedulers.AndroidSchedulers;
+import io.reactivex.observers.DisposableObserver;
+import io.reactivex.schedulers.Schedulers;
+import io.reactivex.disposables.CompositeDisposable;
public class AnnouncementPresenter extends BasePresenter<AnnouncementMvpView> {
public final String LOG_TAG = getClass().getSimpleName();
- private DataManager mDataManager;
- private CompositeSubscription mSubscriptions;
+ private DataManager mDataManager;
+ private CompositeDisposable compositeDisposable;
public AnnouncementPresenter(DataManager dataManager) {
mDataManager = dataManager;
-
- mSubscriptions = new CompositeSubscription();
+ compositeDisposable = new CompositeDisposable();
--- End diff --
Good work for using the `CompositeDisposable`, It has so many advantages that why we exceptionally lose subscription because of our logic.
---
[GitHub] incubator-taverna-mobile pull request #56: Migration RxJava 1 -> RxJava2
Posted by Hiteshgautam01 <gi...@git.apache.org>.
Github user Hiteshgautam01 commented on a diff in the pull request:
https://github.com/apache/incubator-taverna-mobile/pull/56#discussion_r169177839
--- Diff: app/src/main/java/org/apache/taverna/mobile/ui/login/LoginPresenter.java ---
@@ -76,16 +75,15 @@ public void onError(Throwable e) {
}
@Override
- public void onNext(User user) {
-
+ public void onComplete() {
+ getMvpView().showDashboardActivity();
+ getMvpView().showProgressDialog(false);
--- End diff --
When i am moving these to OnNext then it showing the activity.Its keep loading.
---
[GitHub] incubator-taverna-mobile pull request #56: Migration RxJava 1 -> RxJava2
Posted by sagar15795 <gi...@git.apache.org>.
Github user sagar15795 commented on a diff in the pull request:
https://github.com/apache/incubator-taverna-mobile/pull/56#discussion_r169214069
--- Diff: app/src/main/java/org/apache/taverna/mobile/ui/workflow/WorkflowPresenter.java ---
@@ -84,25 +82,27 @@ public void onError(Throwable e) {
}
@Override
- public void onNext(Workflows workflows) {
- getMvpView().showProgressbar(false);
- getMvpView().removeLoadMoreProgressbar();
- getMvpView().showWorkflows(workflows);
+ public void onComplete() {
+
}
}));
}
public void attachSearchHandler(final SearchView searchView) {
- mSearchViewSubscription = RxSearch.fromSearchView(searchView)
+ checkViewAttached();
+ compositeDisposable.add(RxSearch.fromSearchView(searchView)
.distinctUntilChanged()
.debounce(300, TimeUnit.MILLISECONDS)
.observeOn(AndroidSchedulers.mainThread())
.subscribeOn(Schedulers.io())
- .subscribe(new Subscriber<String>() {
+ .subscribeWith(new DisposableObserver<String>() {
@Override
- public void onCompleted() {
-
+ public void onNext(String s) {
--- End diff --
This String `s` name should be `searchText`
---
[GitHub] incubator-taverna-mobile pull request #56: Migration RxJava 1 -> RxJava2
Posted by therajanmaurya <gi...@git.apache.org>.
Github user therajanmaurya commented on a diff in the pull request:
https://github.com/apache/incubator-taverna-mobile/pull/56#discussion_r169136376
--- Diff: app/src/main/java/org/apache/taverna/mobile/ui/login/LoginPresenter.java ---
@@ -76,16 +75,15 @@ public void onError(Throwable e) {
}
@Override
- public void onNext(User user) {
-
+ public void onComplete() {
+ getMvpView().showDashboardActivity();
+ getMvpView().showProgressDialog(false);
--- End diff --
`onComplete()` get called in the last of the cycle so basically, we use this when we make list of request and want to check all of them went well or not. Please move
`
getMvpView().showDashboardActivity();
getMvpView().showProgressDialog(false);
`
in onNext.
---