Fix ConcurrentModificationException on saving a book

app/src/main/java/org/kiwix/kiwixmobile/data/DataSource.java

@@ -17,7 +17,11 @@ import io.reactivex.Single;
 public interface DataSource {
   Single<List<LibraryNetworkEntity.Book>> getLanguageCategorizedBooks();
 
-  void saveBooks(List<LibraryNetworkEntity.Book> book);
+  Completable saveBook(LibraryNetworkEntity.Book book);
+
+  Completable saveBooks(List<LibraryNetworkEntity.Book> book);
+
+  Completable deleteBook(LibraryNetworkEntity.Book book);
 
   Completable saveLanguages(List<Language> languages);
 

app/src/main/java/org/kiwix/kiwixmobile/data/Repository.java

@@ -83,8 +83,21 @@ public class Repository implements DataSource {
   }
 
   @Override
-  public void saveBooks(List<LibraryNetworkEntity.Book> books) {
+  public Completable saveBooks(List<LibraryNetworkEntity.Book> books) {
-    bookDao.saveBooks((ArrayList<LibraryNetworkEntity.Book>) books);
+    return Completable.fromAction(() -> bookDao.saveBooks((ArrayList<LibraryNetworkEntity.Book>) books))
+        .subscribeOn(io);
+  }
+
+  @Override
+  public Completable saveBook(LibraryNetworkEntity.Book book) {
+    return Completable.fromAction(() -> bookDao.saveBook(book))
+        .subscribeOn(io);
+  }
+
+  @Override
+  public Completable deleteBook(LibraryNetworkEntity.Book book) {
+    return Completable.fromAction(() -> bookDao.deleteBook(book.getId()))
+        .subscribeOn(io);
   }
 
   @Override

app/src/main/java/org/kiwix/kiwixmobile/downloader/DownloadService.java

@@ -38,7 +38,7 @@ import android.widget.Toast;
 
 import org.kiwix.kiwixmobile.KiwixApplication;
 import org.kiwix.kiwixmobile.R;
-import org.kiwix.kiwixmobile.data.local.dao.BookDao;
+import org.kiwix.kiwixmobile.data.DataSource;
 import org.kiwix.kiwixmobile.data.remote.KiwixService;
 import org.kiwix.kiwixmobile.library.entity.LibraryNetworkEntity;
 import org.kiwix.kiwixmobile.main.MainActivity;
@@ -61,8 +61,11 @@ import java.util.concurrent.TimeUnit;
 
 import javax.inject.Inject;
 
+import io.reactivex.CompletableObserver;
 import io.reactivex.Observable;
+import io.reactivex.Observer;
 import io.reactivex.android.schedulers.AndroidSchedulers;
+import io.reactivex.disposables.Disposable;
 import okhttp3.OkHttpClient;
 import okhttp3.Request;
 import okhttp3.Response;
@@ -77,15 +80,6 @@ import static org.kiwix.kiwixmobile.utils.files.FileUtils.getCurrentSize;
 
 public class DownloadService extends Service {
 
-  @Inject KiwixService kiwixService;
-  @Inject OkHttpClient httpClient;
-  @Inject NotificationManager notificationManager;
-
-  private static String SD_CARD;
-  // 1024 / 100
-  private static final double BOOK_SIZE_OFFSET = 10.24;
-  private static final String KIWIX_TAG = "kiwixdownloadservice";
-  public static String KIWIX_ROOT;
   public static final int PLAY = 1;
   public static final int PAUSE = 2;
   public static final int FINISH = 3;
@@ -94,22 +88,33 @@ public class DownloadService extends Service {
   public static final String ACTION_STOP = "STOP";
   public static final String ACTION_NO_WIFI = "NO_WIFI";
   public static final String NOTIFICATION_ID = "NOTIFICATION_ID";
+  public static final Object pauseLock = new Object();
+  // 1024 / 100
+  private static final double BOOK_SIZE_OFFSET = 10.24;
+  private static final String KIWIX_TAG = "kiwixdownloadservice";
+  public static String KIWIX_ROOT;
   public static ArrayList<String> notifications = new ArrayList<>();
+  private static String SD_CARD;
+  private static DownloadFragment downloadFragment;
+  private final IBinder mBinder = new LocalBinder();
   public String notificationTitle;
-
-  private SparseArray<NotificationCompat.Builder> notification = new SparseArray<>();
   public SparseIntArray downloadStatus = new SparseIntArray();
   public SparseIntArray downloadProgress = new SparseIntArray();
   public SparseIntArray timeRemaining = new SparseIntArray();
-  public static final Object pauseLock = new Object();
+  @Inject
-  private static DownloadFragment downloadFragment;
+  KiwixService kiwixService;
+  @Inject
+  OkHttpClient httpClient;
+  @Inject
+  NotificationManager notificationManager;
   Handler handler = new Handler(Looper.getMainLooper());
 
   @Inject
   SharedPreferenceUtil sharedPreferenceUtil;
 
   @Inject
-  BookDao bookDao;
+  DataSource dataSource;
+  private SparseArray<NotificationCompat.Builder> notification = new SparseArray<>();
 
   public static void setDownloadFragment(DownloadFragment dFragment) {
     downloadFragment = dFragment;
@@ -303,19 +308,42 @@ public class DownloadService extends Service {
         .flatMap(pair -> Observable.fromIterable(ChunkUtils.getChunks(pair.first, pair.second, notificationID)))
         .concatMap(this::downloadChunk)
         .distinctUntilChanged().doOnComplete(() -> updateDownloadFragmentComplete(notificationID))
-        .subscribe(progress -> {
+        .subscribe(new Observer<Integer>() {
+          @Override
+          public void onSubscribe(Disposable d) {
+
+          }
+
+          @Override
+          public void onNext(Integer progress) {
             if (progress == 100) {
               notification.get(notificationID).setOngoing(false);
               notification.get(notificationID).setContentTitle(notificationTitle + " " + getResources().getString(R.string.zim_file_downloaded));
               notification.get(notificationID).setContentText(getString(R.string.zim_file_downloaded));
-            final Intent target = new Intent(this, MainActivity.class);
+              final Intent target = new Intent(DownloadService.this, MainActivity.class);
               target.putExtra(EXTRA_ZIM_FILE, KIWIX_ROOT + StorageUtils.getFileNameFromUrl(book.getUrl()));
               target.putExtra(EXTRA_NOTIFICATION_ID, notificationID);
               PendingIntent pendingIntent = PendingIntent.getActivity
                   (getBaseContext(), 0,
                       target, PendingIntent.FLAG_CANCEL_CURRENT);
               book.downloaded = true;
-            bookDao.deleteBook(book.id);
+              dataSource.deleteBook(book)
+                  .subscribe(new CompletableObserver() {
+                    @Override
+                    public void onSubscribe(Disposable d) {
+
+                    }
+
+                    @Override
+                    public void onComplete() {
+
+                    }
+
+                    @Override
+                    public void onError(Throwable e) {
+                      Log.e("DownloadService", "Unable to delete book", e);
+                    }
+                  });
               notification.get(notificationID).setContentIntent(pendingIntent);
               notification.get(notificationID).mActions.clear();
               TestingUtils.unbindResource(DownloadService.class);
@@ -332,7 +360,18 @@ public class DownloadService extends Service {
             if (progress == 100) {
               stopSelf();
             }
-        }, Throwable::printStackTrace);
+          }
+
+          @Override
+          public void onError(Throwable e) {
+
+          }
+
+          @Override
+          public void onComplete() {
+
+          }
+        });
   }
 
   private void updateDownloadFragmentProgress(int progress, int notificationID) {
@@ -436,7 +475,23 @@ public class DownloadService extends Service {
                 .get(chunk.getNotificationID());
             book.remoteUrl = book.getUrl();
             book.file = fullFile;
-            bookDao.saveBook(book);
+            dataSource.saveBook(book)
+                .subscribe(new CompletableObserver() {
+                  @Override
+                  public void onSubscribe(Disposable d) {
+
+                  }
+
+                  @Override
+                  public void onComplete() {
+
+                  }
+
+                  @Override
+                  public void onError(Throwable e) {
+                    Log.e("DownloadService", "Unable to save book", e);
+                  }
+                });
           }
           downloadStatus.put(chunk.getNotificationID(), PLAY);
           downloadProgress.put(chunk.getNotificationID(), 0);
@@ -578,7 +633,10 @@ public class DownloadService extends Service {
     }
   }
 
-  private final IBinder mBinder = new LocalBinder();
+  @Override
+  public IBinder onBind(Intent intent) {
+    return mBinder;
+  }
 
   /**
    * Class used for the client Binder.  Because we know this service always
@@ -590,10 +648,4 @@ public class DownloadService extends Service {
       return DownloadService.this;
     }
   }
-
-  @Override
-  public IBinder onBind(Intent intent) {
-    return mBinder;
-  }
-
 }

app/src/main/java/org/kiwix/kiwixmobile/main/MainPresenter.java

@@ -24,6 +24,7 @@ import io.reactivex.disposables.Disposable;
 @PerActivity
 class MainPresenter extends BasePresenter<MainContract.View> implements MainContract.Presenter {
 
+  private static final String TAG = "MainPresenter";
   private final DataSource dataSource;
 
   @Inject
@@ -47,17 +48,32 @@ class MainPresenter extends BasePresenter<MainContract.View> implements MainCont
 
           @Override
           public void onError(Throwable e) {
-            Log.d("MainPresenter", e.toString());
+            Log.e(TAG, "Unable to load books", e);
           }
         });
   }
 
   @Override
   public void saveBooks(List<LibraryNetworkEntity.Book> book) {
-    dataSource.saveBooks(book);
+    dataSource.saveBooks(book)
+        .subscribe(new CompletableObserver() {
+          @Override
+          public void onSubscribe(Disposable d) {
+
+          }
+
+          @Override
+          public void onComplete() {
             showHome();
           }
 
+          @Override
+          public void onError(Throwable e) {
+            Log.e(TAG, "Unable to save books", e);
+          }
+        });
+  }
+
   @Override
   public void saveHistory(History history) {
     dataSource.saveHistory(history)
@@ -74,7 +90,7 @@ class MainPresenter extends BasePresenter<MainContract.View> implements MainCont
 
           @Override
           public void onError(Throwable e) {
-            Log.e("MainPresenter", e.toString());
+            Log.e(TAG, "Unable to save history", e);
           }
         });
   }
@@ -82,7 +98,7 @@ class MainPresenter extends BasePresenter<MainContract.View> implements MainCont
   @Override
   public void loadCurrentZimBookmarksUrl() {
     compositeDisposable.add(dataSource.getCurrentZimBookmarksUrl()
-        .subscribe(view::refreshBookmarksUrl, e -> Log.e("MainPresenter", e.toString())));
+        .subscribe(view::refreshBookmarksUrl, e -> Log.e(TAG, "Unable to load current ZIM urls", e)));
   }
 
   @Override
@@ -101,7 +117,7 @@ class MainPresenter extends BasePresenter<MainContract.View> implements MainCont
 
           @Override
           public void onError(Throwable e) {
-            Log.e("MainPresenter", e.toString());
+            Log.e(TAG, "Unable to save bookmark", e);
           }
         });
   }
@@ -122,7 +138,7 @@ class MainPresenter extends BasePresenter<MainContract.View> implements MainCont
 
           @Override
           public void onError(Throwable e) {
-            Log.e("MainPresenter", e.toString());
+            Log.e(TAG, "Unable to delete bookmark", e);
           }
         });
   }

app/src/main/java/org/kiwix/kiwixmobile/zim_manager/fileselect_view/ZimFileSelectFragment.java

@@ -24,6 +24,7 @@ import android.content.Context;
 import android.content.pm.PackageManager;
 import android.os.Build;
 import android.os.Bundle;
+import android.support.annotation.NonNull;
 import android.support.v4.content.ContextCompat;
 import android.support.v4.widget.SwipeRefreshLayout;
 import android.support.v7.app.AlertDialog;
@@ -72,7 +73,14 @@ public class ZimFileSelectFragment extends BaseFragment
 
   public RelativeLayout llLayout;
   public SwipeRefreshLayout swipeRefreshLayout;
-
+  @Inject
+  ZimFileSelectPresenter presenter;
+  @Inject
+  BookUtils bookUtils;
+  @Inject
+  SharedPreferenceUtil sharedPreferenceUtil;
+  @Inject
+  BookDao bookDao;
   private ZimManageActivity zimManageActivity;
   private RescanDataAdapter mRescanAdapter;
   private ArrayList<LibraryNetworkEntity.Book> mFiles;
@@ -80,20 +88,14 @@ public class ZimFileSelectFragment extends BaseFragment
   private TextView mFileMessage;
   private boolean mHasRefresh;
 
-  @Inject ZimFileSelectPresenter presenter;
-  @Inject BookUtils bookUtils;
-  @Inject SharedPreferenceUtil sharedPreferenceUtil;
-  @Inject
-  BookDao bookDao;
-
   @Override
-  public View onCreateView(LayoutInflater inflater, ViewGroup container, Bundle savedInstanceState) {
+  public View onCreateView(@NonNull LayoutInflater inflater, ViewGroup container, Bundle savedInstanceState) {
     KiwixApplication.getApplicationComponent().inject(this);
     zimManageActivity = (ZimManageActivity) super.getActivity();
     presenter.attachView(this);
     // Replace LinearLayout by the type of the root element of the layout you're trying to load
     llLayout = (RelativeLayout) inflater.inflate(R.layout.zim_list, container, false);
-    new LanguageUtils(super.getActivity()).changeFont(super.getActivity().getLayoutInflater(), sharedPreferenceUtil);
+    new LanguageUtils(zimManageActivity).changeFont(zimManageActivity.getLayoutInflater(), sharedPreferenceUtil);
 
     mFileMessage = llLayout.findViewById(R.id.file_management_no_files);
     mZimFileList = llLayout.findViewById(R.id.zimfilelist);
@@ -154,20 +156,13 @@ public class ZimFileSelectFragment extends BaseFragment
     if (book != null) {
       mFiles.add(book);
       mRescanAdapter.notifyDataSetChanged();
-      bookDao.saveBooks(mFiles);
+      presenter.saveBooks(mFiles);
       checkEmpty();
     }
   }
 
-  private class FileComparator implements Comparator<LibraryNetworkEntity.Book> {
-    @Override
-    public int compare(LibraryNetworkEntity.Book b1, LibraryNetworkEntity.Book b2) {
-      return b1.getTitle().compareTo(b2.getTitle());
-    }
-  }
-
   public void checkPermissions() {
-    if (ContextCompat.checkSelfPermission(super.getActivity(),
+    if (ContextCompat.checkSelfPermission(zimManageActivity,
         Manifest.permission.READ_EXTERNAL_STORAGE)
         != PackageManager.PERMISSION_GRANTED && Build.VERSION.SDK_INT > 18) {
       Toast.makeText(super.getActivity(), getResources().getString(R.string.request_storage), Toast.LENGTH_LONG)
@@ -225,7 +220,7 @@ public class ZimFileSelectFragment extends BaseFragment
         // Save the current list of books
         zimManageActivity.runOnUiThread(() -> {
           mRescanAdapter.notifyDataSetChanged();
-          bookDao.saveBooks(mFiles);
+          presenter.saveBooks(mFiles);
           checkEmpty();
           TestingUtils.unbindResource(ZimFileSelectFragment.class);
 
@@ -238,7 +233,7 @@ public class ZimFileSelectFragment extends BaseFragment
 
   @Override
   public void onRequestPermissionsResult(int requestCode,
-                                         String permissions[], int[] grantResults) {
+                                         @NonNull String permissions[], @NonNull int[] grantResults) {
     switch (requestCode) {
       case REQUEST_STORAGE_PERMISSION: {
         if (grantResults.length > 0
@@ -298,7 +293,7 @@ public class ZimFileSelectFragment extends BaseFragment
     if (file.exists()) {
       return false;
     }
-    bookDao.deleteBook(mFiles.get(position).getId());
+    presenter.deleteBook(mFiles.get(position));
     mFiles.remove(position);
     mRescanAdapter.notifyDataSetChanged();
     checkEmpty();
@@ -315,15 +310,23 @@ public class ZimFileSelectFragment extends BaseFragment
       mFileMessage.setVisibility(View.GONE);
   }
 
+  private class FileComparator implements Comparator<LibraryNetworkEntity.Book> {
+    @Override
+    public int compare(LibraryNetworkEntity.Book b1, LibraryNetworkEntity.Book b2) {
+      return b1.getTitle().compareTo(b2.getTitle());
+    }
+  }
+
   // The Adapter for the ListView for when the ListView is populated with the rescanned files
   private class RescanDataAdapter extends ArrayAdapter<LibraryNetworkEntity.Book> {
 
-    public RescanDataAdapter(Context context, int textViewResourceId, List<LibraryNetworkEntity.Book> objects) {
+    RescanDataAdapter(Context context, int textViewResourceId, List<LibraryNetworkEntity.Book> objects) {
       super(context, textViewResourceId, objects);
     }
 
+    @NonNull
     @Override
-    public View getView(int position, View convertView, ViewGroup parent) {
+    public View getView(int position, View convertView, @NonNull ViewGroup parent) {
 
       ViewHolder holder;
       LibraryNetworkEntity.Book book = getItem(position);

app/src/main/java/org/kiwix/kiwixmobile/zim_manager/fileselect_view/ZimFileSelectPresenter.java

@@ -17,7 +17,10 @@
  */
 package org.kiwix.kiwixmobile.zim_manager.fileselect_view;
 
+import android.util.Log;
+
 import org.kiwix.kiwixmobile.base.BasePresenter;
+import org.kiwix.kiwixmobile.data.DataSource;
 import org.kiwix.kiwixmobile.data.local.dao.BookDao;
 import org.kiwix.kiwixmobile.library.entity.LibraryNetworkEntity;
 
@@ -25,16 +28,23 @@ import java.util.ArrayList;
 
 import javax.inject.Inject;
 
+import io.reactivex.CompletableObserver;
+import io.reactivex.disposables.Disposable;
+
 /**
  * Created by EladKeyshawn on 06/04/2017.
  */
 public class ZimFileSelectPresenter extends BasePresenter<ZimFileSelectViewCallback> {
 
+  private static final String TAG = "ZimFileSelectPresenter";
+  private final DataSource dataSource;
+
   @Inject
   BookDao bookDao;
 
   @Inject
-  ZimFileSelectPresenter() {
+  ZimFileSelectPresenter(DataSource dataSource) {
+    this.dataSource = dataSource;
   }
 
   @Override
@@ -47,4 +57,43 @@ public class ZimFileSelectPresenter extends BasePresenter<ZimFileSelectViewCallb
     view.showFiles(books);
   }
 
+  void saveBooks(ArrayList<LibraryNetworkEntity.Book> books) {
+    dataSource.saveBooks(books)
+        .subscribe(new CompletableObserver() {
+          @Override
+          public void onSubscribe(Disposable d) {
+
+          }
+
+          @Override
+          public void onComplete() {
+
+          }
+
+          @Override
+          public void onError(Throwable e) {
+            Log.e(TAG, "Unable to save books", e);
+          }
+        });
+  }
+
+  public void deleteBook(LibraryNetworkEntity.Book book) {
+    dataSource.deleteBook(book)
+        .subscribe(new CompletableObserver() {
+          @Override
+          public void onSubscribe(Disposable d) {
+
+          }
+
+          @Override
+          public void onComplete() {
+
+          }
+
+          @Override
+          public void onError(Throwable e) {
+            Log.e(TAG, "Unable to delete book", e);
+          }
+        });
+  }
 }