Remove view logic from catalogue presenter and improve catalogue fragment

app/src/main/java/eu/kanade/mangafeed/ui/catalogue/CatalogueFragment.java

@@ -3,6 +3,7 @@ package eu.kanade.mangafeed.ui.catalogue;
 import android.content.Intent;
 import android.os.Bundle;
 import android.support.v7.widget.SearchView;
+import android.text.TextUtils;
 import android.view.LayoutInflater;
 import android.view.Menu;
 import android.view.MenuInflater;
@@ -14,6 +15,7 @@ import android.widget.ImageView;
 import android.widget.ProgressBar;
 
 import java.util.List;
+import java.util.concurrent.TimeUnit;
 
 import butterknife.Bind;
 import butterknife.ButterKnife;
@@ -24,7 +26,13 @@ import eu.kanade.mangafeed.ui.base.fragment.BaseRxFragment;
 import eu.kanade.mangafeed.ui.manga.MangaActivity;
 import eu.kanade.mangafeed.util.PageBundle;
 import eu.kanade.mangafeed.widget.EndlessScrollListener;
+import icepick.Icepick;
+import icepick.State;
 import nucleus.factory.RequiresPresenter;
+import rx.Subscription;
+import rx.android.schedulers.AndroidSchedulers;
+import rx.schedulers.Schedulers;
+import rx.subjects.PublishSubject;
 
 @RequiresPresenter(CataloguePresenter.class)
 public class CatalogueFragment extends BaseRxFragment<CataloguePresenter> {
@@ -35,7 +43,12 @@ public class CatalogueFragment extends BaseRxFragment<CataloguePresenter> {
 
     private CatalogueAdapter adapter;
     private EndlessScrollListener scrollListener;
-    private String search;
+
+    @State String query = "";
+    private final int SEARCH_TIMEOUT = 1000;
+
+    private PublishSubject<String> queryDebouncerSubject;
+    private Subscription queryDebouncerSubscription;
 
     public final static String SOURCE_ID = "source_id";
 
@@ -48,63 +61,132 @@ public class CatalogueFragment extends BaseRxFragment<CataloguePresenter> {
     }
 
     @Override
-    public void onCreate(Bundle savedInstanceState) {
+    public void onCreate(Bundle savedState) {
-        super.onCreate(savedInstanceState);
+        super.onCreate(savedState);
+        Icepick.restoreInstanceState(this, savedState);
         setHasOptionsMenu(true);
     }
 
     @Override
-    public View onCreateView(LayoutInflater inflater, ViewGroup container,
+    public View onCreateView(LayoutInflater inflater, ViewGroup container, Bundle savedState) {
-                             Bundle savedInstanceState) {
         // Inflate the layout for this fragment
         View view = inflater.inflate(R.layout.fragment_catalogue, container, false);
         ButterKnife.bind(this, view);
 
-        initializeAdapter();
+        // Initialize adapter and scroll listener
-        initializeScrollListener();
+        adapter = new CatalogueAdapter(this);
+        scrollListener = new EndlessScrollListener(this::requestNextPage);
+        gridView.setAdapter(adapter);
+        gridView.setOnScrollListener(scrollListener);
 
-        int source_id = getArguments().getInt(SOURCE_ID, -1);
+        int sourceId = getArguments().getInt(SOURCE_ID, -1);
 
         showProgressBar();
+        if (savedState == null)
+            getPresenter().startRequesting(sourceId);
 
-        if (savedInstanceState == null)
+        setToolbarTitle(getPresenter().getSource().getName());
-            getPresenter().startRequesting(source_id);
-
         return view;
     }
 
     @Override
     public void onCreateOptionsMenu(Menu menu, MenuInflater inflater) {
         inflater.inflate(R.menu.catalogue_list, menu);
-        initializeSearch(menu);
-    }
 
-    private void initializeSearch(Menu menu) {
+        // Initialize search menu
         MenuItem searchItem = menu.findItem(R.id.action_search);
-        final SearchView sv = (SearchView) searchItem.getActionView();
+        final SearchView searchView = (SearchView) searchItem.getActionView();
-        sv.setOnQueryTextListener(new SearchView.OnQueryTextListener() {
+
+        if (!TextUtils.isEmpty(query)) {
+            searchItem.expandActionView();
+            searchView.setQuery(query, true);
+            searchView.clearFocus();
+        }
+        searchView.setOnQueryTextListener(new SearchView.OnQueryTextListener() {
             @Override
             public boolean onQueryTextSubmit(String query) {
-                getPresenter().onSearchEvent(query, true);
+                onSearchEvent(query, true);
                 return true;
             }
 
             @Override
             public boolean onQueryTextChange(String newText) {
-                getPresenter().onSearchEvent(newText, false);
+                onSearchEvent(newText, false);
                 return true;
             }
         });
-        if (search != null && !search.equals("")) {
+    }
-            searchItem.expandActionView();
+
-            sv.setQuery(search, true);
+    @Override
-            sv.clearFocus();
+    public void onStart() {
+        super.onStart();
+        initializeSearchSubscription();
+    }
+
+    @Override
+    public void onStop() {
+        destroySearchSubscription();
+        super.onStop();
+    }
+
+    @Override
+    public void onSaveInstanceState(Bundle outState) {
+        Icepick.saveInstanceState(this, outState);
+        super.onSaveInstanceState(outState);
+    }
+
+    private void initializeSearchSubscription() {
+        queryDebouncerSubject = PublishSubject.create();
+        queryDebouncerSubscription = queryDebouncerSubject
+                .debounce(SEARCH_TIMEOUT, TimeUnit.MILLISECONDS)
+                .subscribeOn(Schedulers.io())
+                .observeOn(AndroidSchedulers.mainThread())
+                .subscribe(this::restartRequest);
+    }
+
+    private void destroySearchSubscription() {
+        queryDebouncerSubscription.unsubscribe();
+    }
+
+    private void onSearchEvent(String query, boolean now) {
+        // If the query is not debounced, resolve it instantly
+        if (now)
+            restartRequest(query);
+        else if (queryDebouncerSubject != null)
+            queryDebouncerSubject.onNext(query);
+    }
+
+    private void restartRequest(String newQuery) {
+        // If text didn't change, do nothing
+        if (query.equals(newQuery)) return;
+
+        query = newQuery;
+        showProgressBar();
+        // Set adapter again for scrolling to top: http://stackoverflow.com/a/17577981/3263582
+        gridView.setAdapter(adapter);
+        gridView.setSelection(0);
+
+        getPresenter().restartRequest(query);
+    }
+
+    private void requestNextPage() {
+        if (getPresenter().hasNextPage()) {
+            showGridProgressBar();
+            getPresenter().requestNext();
         }
     }
 
-    public void initializeAdapter() {
+    public void onAddPage(PageBundle<List<Manga>> page) {
-        adapter = new CatalogueAdapter(this);
+        hideProgressBar();
-        gridView.setAdapter(adapter);
+        if (page.page == 0) {
+            adapter.clear();
+            scrollListener.resetScroll();
+        }
+        adapter.addAll(page.data);
+    }
+
+    public void onAddPageError() {
+        hideProgressBar();
     }
 
     @OnItemClick(R.id.gridView)
@@ -116,37 +198,23 @@ public class CatalogueFragment extends BaseRxFragment<CataloguePresenter> {
         startActivity(intent);
     }
 
-    public void initializeScrollListener() {
+    public void updateImage(Manga manga) {
-        scrollListener = new EndlessScrollListener(this::requestNext);
+        ImageView imageView = getImageView(getMangaIndex(manga));
-        gridView.setOnScrollListener(scrollListener);
+        if (imageView != null && manga.thumbnail_url != null) {
-    }
+            getPresenter().coverCache.loadFromNetwork(imageView, manga.thumbnail_url,
-
+                    getPresenter().getSource().getGlideHeaders());
-    public void requestNext() {
-        if (getPresenter().requestNext())
-            showGridProgressBar();
-    }
-
-    public void showProgressBar() {
-        progress.setVisibility(ProgressBar.VISIBLE);
-    }
-
-    public void showGridProgressBar() {
-        progressGrid.setVisibility(ProgressBar.VISIBLE);
-    }
-
-    public void hideProgressBar() {
-        progress.setVisibility(ProgressBar.GONE);
-        progressGrid.setVisibility(ProgressBar.GONE);
-    }
-
-    public void onAddPage(PageBundle<List<Manga>> page) {
-        hideProgressBar();
-        if (page.page == 0) {
-            gridView.setSelection(0);
-            adapter.clear();
-            scrollListener.resetScroll();
         }
-        adapter.addAll(page.data);
+    }
+
+    private ImageView getImageView(int position) {
+        if (position == -1) return null;
+
+        View v = gridView.getChildAt(position -
+                gridView.getFirstVisiblePosition());
+
+        if (v == null) return null;
+
+        return (ImageView) v.findViewById(R.id.thumbnail);
     }
 
     private int getMangaIndex(Manga manga) {
@@ -158,28 +226,17 @@ public class CatalogueFragment extends BaseRxFragment<CataloguePresenter> {
         return -1;
     }
 
-    private ImageView getImageView(int position) {
+    private void showProgressBar() {
-        if (position == -1)
+        progress.setVisibility(ProgressBar.VISIBLE);
-            return null;
-
-        View v = gridView.getChildAt(position -
-                gridView.getFirstVisiblePosition());
-
-        if(v == null)
-            return null;
-
-        return (ImageView) v.findViewById(R.id.thumbnail);
     }
 
-    public void updateImage(Manga manga) {
+    private void showGridProgressBar() {
-        ImageView imageView = getImageView(getMangaIndex(manga));
+        progressGrid.setVisibility(ProgressBar.VISIBLE);
-        if (imageView != null && manga.thumbnail_url != null) {
-            getPresenter().coverCache.loadFromNetwork(imageView, manga.thumbnail_url,
-                    getPresenter().getSource().getGlideHeaders());
-        }
     }
 
-    public void restoreSearch(String mSearchName) {
+    private void hideProgressBar() {
-        search = mSearchName;
+        progress.setVisibility(ProgressBar.GONE);
+        progressGrid.setVisibility(ProgressBar.GONE);
     }
+
 }

app/src/main/java/eu/kanade/mangafeed/ui/catalogue/CataloguePresenter.java

@@ -1,11 +1,11 @@
 package eu.kanade.mangafeed.ui.catalogue;
 
 import android.os.Bundle;
+import android.text.TextUtils;
 
 import com.pushtorefresh.storio.sqlite.operations.put.PutResult;
 
 import java.util.List;
-import java.util.concurrent.TimeUnit;
 
 import javax.inject.Inject;
 
@@ -18,9 +18,7 @@ import eu.kanade.mangafeed.data.source.model.MangasPage;
 import eu.kanade.mangafeed.ui.base.presenter.BasePresenter;
 import eu.kanade.mangafeed.util.PageBundle;
 import eu.kanade.mangafeed.util.RxPager;
-import icepick.State;
 import rx.Observable;
-import rx.Subscription;
 import rx.android.schedulers.AndroidSchedulers;
 import rx.schedulers.Schedulers;
 import rx.subjects.PublishSubject;
@@ -32,18 +30,14 @@ public class CataloguePresenter extends BasePresenter<CatalogueFragment> {
     @Inject DatabaseHelper db;
     @Inject CoverCache coverCache;
 
-    private Source selectedSource;
+    private Source source;
 
-    @State protected String searchName;
+    private String query;
-    @State protected boolean searchMode;
-    private final int SEARCH_TIMEOUT = 1000;
 
     private int currentPage;
     private RxPager pager;
     private MangasPage lastMangasPage;
 
-    private Subscription queryDebouncerSubscription;
-    private PublishSubject<String> queryDebouncerSubject;
     private PublishSubject<List<Manga>> mangaDetailSubject;
 
     private static final int GET_MANGA_LIST = 1;
@@ -65,7 +59,10 @@ public class CataloguePresenter extends BasePresenter<CatalogueFragment> {
                     if (mangaDetailSubject != null)
                         mangaDetailSubject.onNext(page.data);
                 },
-                (view, error) -> Timber.e(error.fillInStackTrace(), error.getMessage()));
+                (view, error) -> {
+                    view.onAddPageError();
+                    Timber.e(error.getMessage());
+                });
 
         restartableLatestCache(GET_MANGA_DETAIL,
                 () -> mangaDetailSubject
@@ -77,46 +74,28 @@ public class CataloguePresenter extends BasePresenter<CatalogueFragment> {
                         .filter(manga -> manga.initialized)
                         .onBackpressureBuffer()
                         .observeOn(AndroidSchedulers.mainThread()),
-                (view, manga) -> {
+                (view, manga) -> view.updateImage(manga),
-                    view.updateImage(manga);
+                (view, error) -> Timber.e(error.getMessage()));
-                },
-                (view, error) -> Timber.e(error.fillInStackTrace(), error.getMessage()));
-
-        initializeSearch();
-    }
-
-    @Override
-    protected void onTakeView(CatalogueFragment view) {
-        super.onTakeView(view);
-
-        view.setToolbarTitle(selectedSource.getName());
-
-        if (searchMode)
-            view.restoreSearch(searchName);
     }
 
     public void startRequesting(int sourceId) {
-        selectedSource = sourceManager.get(sourceId);
+        source = sourceManager.get(sourceId);
-        restartRequest();
+        restartRequest(null);
     }
 
-    private void restartRequest() {
+    public void restartRequest(String query) {
+        this.query = query;
         stop(GET_MANGA_LIST);
         currentPage = 1;
         pager = new RxPager();
-        if (getView() != null)
-            getView().showProgressBar();
 
         start(GET_MANGA_DETAIL);
         start(GET_MANGA_LIST);
     }
 
-    public boolean requestNext() {
+    public void requestNext() {
-        if (lastMangasPage.nextPageUrl == null)
+        if (hasNextPage())
-            return false;
+            pager.requestNext(++currentPage);
-
-        pager.requestNext(++currentPage);
-        return true;
     }
 
     private Observable<List<Manga>> getMangaObs(int page) {
@@ -125,11 +104,9 @@ public class CataloguePresenter extends BasePresenter<CatalogueFragment> {
             nextMangasPage.url = lastMangasPage.nextPageUrl;
         }
 
-        Observable<MangasPage> obs;
+        Observable<MangasPage> obs = !TextUtils.isEmpty(query) ?
-        if (searchMode)
+            source.searchMangasFromNetwork(nextMangasPage, query) :
-            obs = selectedSource.searchMangasFromNetwork(nextMangasPage, searchName);
+            source.pullPopularMangasFromNetwork(nextMangasPage);
-        else
-            obs = selectedSource.pullPopularMangasFromNetwork(nextMangasPage);
 
         return obs.subscribeOn(Schedulers.io())
                 .doOnNext(mangasPage -> lastMangasPage = mangasPage)
@@ -139,7 +116,7 @@ public class CataloguePresenter extends BasePresenter<CatalogueFragment> {
     }
 
     private Manga networkToLocalManga(Manga networkManga) {
-        List<Manga> dbResult = db.getManga(networkManga.url, selectedSource.getSourceId()).executeAsBlocking();
+        List<Manga> dbResult = db.getManga(networkManga.url, source.getSourceId()).executeAsBlocking();
         Manga localManga = !dbResult.isEmpty() ? dbResult.get(0) : null;
         if (localManga == null) {
             PutResult result = db.insertManga(networkManga).executeAsBlocking();
@@ -149,62 +126,23 @@ public class CataloguePresenter extends BasePresenter<CatalogueFragment> {
         return localManga;
     }
 
-    private void initializeSearch() {
-        if (queryDebouncerSubscription != null)
-            return;
-
-        searchName = "";
-        searchMode = false;
-        queryDebouncerSubject = PublishSubject.create();
-
-        add(queryDebouncerSubscription = queryDebouncerSubject
-                .debounce(SEARCH_TIMEOUT, TimeUnit.MILLISECONDS)
-                .subscribeOn(Schedulers.io())
-                .observeOn(AndroidSchedulers.mainThread())
-                .subscribe(this::queryFromSearch));
-    }
-
     private Observable<Manga> getMangaDetails(final Manga manga) {
-        return selectedSource.pullMangaFromNetwork(manga.url)
+        return source.pullMangaFromNetwork(manga.url)
                 .subscribeOn(Schedulers.io())
                 .flatMap(networkManga -> {
                     Manga.copyFromNetwork(manga, networkManga);
                     db.insertManga(manga).executeAsBlocking();
                     return Observable.just(manga);
                 })
-                .onErrorResumeNext(error -> {
+                .onErrorResumeNext(error -> Observable.just(manga));
-                    return Observable.just(manga);
-                });
-    }
-
-    public void onSearchEvent(String query, boolean now) {
-        // If the query is empty or not debounced, resolve it instantly
-        if (now || query.equals(""))
-            queryFromSearch(query);
-        else if (queryDebouncerSubject != null)
-            queryDebouncerSubject.onNext(query);
-    }
-
-    private void queryFromSearch(String query) {
-        // If text didn't change, do nothing
-        if (searchName.equals(query)) {
-            return;
-        }
-        // If going to search mode
-        else if (searchName.equals("") && !query.equals("")) {
-            searchMode = true;
-        }
-        // If going to normal mode
-        else if (!searchName.equals("") && query.equals("")) {
-            searchMode = false;
-        }
-
-        searchName = query;
-        restartRequest();
     }
 
     public Source getSource() {
-        return selectedSource;
+        return source;
+    }
+
+    public boolean hasNextPage() {
+        return lastMangasPage != null && lastMangasPage.nextPageUrl != null;
     }
 
 }

app/src/main/java/eu/kanade/mangafeed/ui/manga/chapter/ChaptersPresenter.java

@@ -133,8 +133,8 @@ public class ChaptersPresenter extends BasePresenter<ChaptersFragment> {
             observable = observable.filter(chapter -> chapter.status == Download.DOWNLOADED);
         }
         return observable.toSortedList((chapter, chapter2) -> sortOrderAToZ ?
-                Float.compare(chapter.chapter_number, chapter2.chapter_number) :
+                Float.compare(chapter2.chapter_number, chapter.chapter_number) :
-                Float.compare(chapter2.chapter_number, chapter.chapter_number));
+                Float.compare(chapter.chapter_number, chapter2.chapter_number));
     }
 
     private void setChapterStatus(Chapter chapter) {