From 6191943d3f7612051e6957bf2a90f8c7416413af Mon Sep 17 00:00:00 2001 From: Bradlee Speice Date: Sat, 24 May 2014 22:13:23 -0400 Subject: [PATCH] Downloading now shows progress! Nasty bug where you can double register to the bus because it's not actually destroyed, need to fix that. --- .../downloader/ActivityDownloaderModule.java | 17 +-- .../activities/downloader/BookItemHolder.java | 23 ++-- .../downloader/BookListFragment.java | 6 +- .../manager/BookDownloadManager.java | 45 +++++-- .../manager/BookDownloadThread.java | 21 ++-- ...rogressEvent.java => DLProgressEvent.java} | 12 +- .../downloader/manager/DownloadManager.java | 115 +----------------- .../downloader/manager/RefreshManager.java | 84 +++++++++++++ 8 files changed, 163 insertions(+), 160 deletions(-) rename MinimalBible/src/main/java/org/bspeice/minimalbible/activities/downloader/manager/{DownloadProgressEvent.java => DLProgressEvent.java} (69%) create mode 100644 MinimalBible/src/main/java/org/bspeice/minimalbible/activities/downloader/manager/RefreshManager.java diff --git a/MinimalBible/src/main/java/org/bspeice/minimalbible/activities/downloader/ActivityDownloaderModule.java b/MinimalBible/src/main/java/org/bspeice/minimalbible/activities/downloader/ActivityDownloaderModule.java index d6e1efe..ecdc5e5 100644 --- a/MinimalBible/src/main/java/org/bspeice/minimalbible/activities/downloader/ActivityDownloaderModule.java +++ b/MinimalBible/src/main/java/org/bspeice/minimalbible/activities/downloader/ActivityDownloaderModule.java @@ -5,8 +5,11 @@ import org.bspeice.minimalbible.activities.downloader.manager.BookDownloadManage import org.bspeice.minimalbible.activities.downloader.manager.BookDownloadThread; import org.bspeice.minimalbible.activities.downloader.manager.BookRefreshTask; import org.bspeice.minimalbible.activities.downloader.manager.DownloadManager; +import org.bspeice.minimalbible.activities.downloader.manager.RefreshManager; import org.crosswire.common.progress.JobManager; +import java.sql.Ref; + import javax.inject.Singleton; import dagger.Module; @@ -24,22 +27,12 @@ import de.greenrobot.event.EventBus; BookRefreshTask.class, BookItemHolder.class, BookDownloadManager.class, - BookDownloadThread.class + BookDownloadThread.class, + RefreshManager.class } ) public class ActivityDownloaderModule { - /** - * Provide a Singleton DownloadManager for injection - * Note that we need to annotate Singleton here, only annotating on the - * DownloadManager itself is not enough. - * @return Global DownloadManager instance - */ - @Provides @Singleton - DownloadManager provideDownloadManager() { - return new DownloadManager(); - } - @Provides EventBus provideBus() { return new EventBus(); diff --git a/MinimalBible/src/main/java/org/bspeice/minimalbible/activities/downloader/BookItemHolder.java b/MinimalBible/src/main/java/org/bspeice/minimalbible/activities/downloader/BookItemHolder.java index 26ab5e2..18225f8 100644 --- a/MinimalBible/src/main/java/org/bspeice/minimalbible/activities/downloader/BookItemHolder.java +++ b/MinimalBible/src/main/java/org/bspeice/minimalbible/activities/downloader/BookItemHolder.java @@ -1,5 +1,6 @@ package org.bspeice.minimalbible.activities.downloader; +import android.util.Log; import android.view.View; import android.widget.ImageButton; import android.widget.RelativeLayout; @@ -9,8 +10,9 @@ import com.todddavies.components.progressbar.ProgressWheel; import org.bspeice.minimalbible.MinimalBible; import org.bspeice.minimalbible.R; +import org.bspeice.minimalbible.activities.downloader.manager.BookDownloadManager; +import org.bspeice.minimalbible.activities.downloader.manager.DLProgressEvent; import org.bspeice.minimalbible.activities.downloader.manager.DownloadManager; -import org.bspeice.minimalbible.activities.downloader.manager.DownloadProgressEvent; import org.crosswire.jsword.book.Book; import javax.inject.Inject; @@ -18,7 +20,6 @@ import javax.inject.Inject; import butterknife.ButterKnife; import butterknife.InjectView; import butterknife.OnClick; -import de.greenrobot.event.EventBus; /** * Created by bspeice on 5/20/14. @@ -34,6 +35,7 @@ public class BookItemHolder { @InjectView(R.id.download_prg_download) ProgressWheel downloadProgress; @Inject DownloadManager downloadManager; + @Inject BookDownloadManager bookDownloadManager; Book b; @@ -46,21 +48,21 @@ public class BookItemHolder { public void bindHolder() { acronym.setText(b.getInitials()); itemName.setText(b.getName()); - DownloadProgressEvent downloadProgressEvent = downloadManager.getInProgressDownloadProgress(b); - if (downloadProgressEvent != null) { - displayProgress((int) downloadProgressEvent.toCircular()); + DLProgressEvent dlProgressEvent = bookDownloadManager.getInProgressDownloadProgress(b); + if (dlProgressEvent != null) { + displayProgress((int) dlProgressEvent.toCircular()); } // TODO: Display a remove icon if the book has been downloaded. } @OnClick(R.id.download_ibtn_download) public void onDownloadItem(View v) { - downloadManager.getRefreshBus().register(this); - downloadManager.installBook(this.b); + downloadManager.getDownloadBus().register(this); + bookDownloadManager.installBook(this.b); } - public void onEventMainThread(DownloadProgressEvent event) { - if (event.getB().equals(b)) { + public void onEventMainThread(DLProgressEvent event) { + if (event.getB().getName().equals(b.getName())) { displayProgress((int) event.toCircular()); } } @@ -72,7 +74,7 @@ public class BookItemHolder { private void displayProgress(int progress) { - if (progress == DownloadProgressEvent.PROGRESS_BEGINNING) { + if (progress == DLProgressEvent.PROGRESS_BEGINNING) { // Download starting RelativeLayout.LayoutParams acronymParams = (RelativeLayout.LayoutParams)acronym.getLayoutParams(); @@ -99,6 +101,7 @@ public class BookItemHolder { isDownloaded.setVisibility(View.GONE); downloadProgress.setVisibility(View.VISIBLE); + downloadProgress.stopSpinning(); downloadProgress.setProgress(progress); } else { // Download complete diff --git a/MinimalBible/src/main/java/org/bspeice/minimalbible/activities/downloader/BookListFragment.java b/MinimalBible/src/main/java/org/bspeice/minimalbible/activities/downloader/BookListFragment.java index 5405c0b..67b399c 100644 --- a/MinimalBible/src/main/java/org/bspeice/minimalbible/activities/downloader/BookListFragment.java +++ b/MinimalBible/src/main/java/org/bspeice/minimalbible/activities/downloader/BookListFragment.java @@ -17,6 +17,7 @@ import org.bspeice.minimalbible.R; import org.bspeice.minimalbible.activities.BaseFragment; import org.bspeice.minimalbible.activities.downloader.manager.DownloadManager; import org.bspeice.minimalbible.activities.downloader.manager.EventBookList; +import org.bspeice.minimalbible.activities.downloader.manager.RefreshManager; import org.crosswire.jsword.book.Book; import org.crosswire.jsword.book.BookCategory; import org.crosswire.jsword.book.BookComparators; @@ -48,6 +49,7 @@ public class BookListFragment extends BaseFragment { ListView downloadsAvailable; @Inject DownloadManager downloadManager; + @Inject RefreshManager refreshManager; @Inject DownloadPrefs downloadPrefs; @@ -116,10 +118,10 @@ public class BookListFragment extends BaseFragment { */ private void refreshModules() { // Check if the downloadManager has already refreshed everything - List bookList = downloadManager.getBookList(); + List bookList = refreshManager.getBookList(); if (bookList == null) { // downloadManager is in progress of refreshing - downloadManager.getRefreshBus().register(this); + downloadManager.getDownloadBus().register(this); refreshDialog = new ProgressDialog(getActivity()); refreshDialog.setMessage("Refreshing available modules..."); refreshDialog.setCancelable(false); diff --git a/MinimalBible/src/main/java/org/bspeice/minimalbible/activities/downloader/manager/BookDownloadManager.java b/MinimalBible/src/main/java/org/bspeice/minimalbible/activities/downloader/manager/BookDownloadManager.java index e4733ab..cf9610b 100644 --- a/MinimalBible/src/main/java/org/bspeice/minimalbible/activities/downloader/manager/BookDownloadManager.java +++ b/MinimalBible/src/main/java/org/bspeice/minimalbible/activities/downloader/manager/BookDownloadManager.java @@ -29,21 +29,24 @@ public class BookDownloadManager implements WorkListener{ */ private Map bookMappings; + /** + * Cached copy of downloads in progress so views displaying this info can get it quickly. + */ + private Map inProgressDownloads; + @Inject Provider dlThreadProvider; - /* Going to fix this in the next commit, right now it's circular - @Inject - */ - DownloadManager downloadManager; + @Inject DownloadManager downloadManager; public BookDownloadManager() { bookMappings = new HashMap(); + inProgressDownloads = new HashMap(); JobManager.addWorkListener(this); MinimalBible.getApplication().inject(this); } - public void downloadBook(Book b) { + public void installBook(Book b) { BookDownloadThread dlThread = dlThreadProvider.get(); dlThread.downloadBook(b); addJob(BookDownloadThread.getJobId(b), b); @@ -55,12 +58,35 @@ public class BookDownloadManager implements WorkListener{ @Override public void workProgressed(WorkEvent ev) { - Log.d("BookDownloadManager", ev.toString()); Progress job = ev.getJob(); + EventBus downloadBus = downloadManager.getDownloadBus(); if (bookMappings.containsKey(job.getJobID())) { - downloadManager.getRefreshBus() - .post(new DownloadProgressEvent(job.getTotalWork(), job.getWorkDone(), - bookMappings.get(job.getJobID()))); + Book b = bookMappings.get(job.getJobID()); + + if (job.getWorkDone() == job.getTotalWork()) { + // Download is complete + inProgressDownloads.remove(bookMappings.get(job.getJobID())); + downloadBus.post(new DLProgressEvent(DLProgressEvent.PROGRESS_COMPLETE, b)); + } else { + // Track the ongoing download + DLProgressEvent event = new DLProgressEvent(job.getWorkDone(), + job.getTotalWork(), b); + inProgressDownloads.put(b, event); + downloadBus.post(event); + } + } + } + + /** + * Check the status of a book download in progress. + * @param b + * @return The most recent DownloadProgressEvent for the book, or null if not downloading + */ + public DLProgressEvent getInProgressDownloadProgress(Book b) { + if (inProgressDownloads.containsKey(b)) { + return inProgressDownloads.get(b); + } else { + return null; } } @@ -68,5 +94,4 @@ public class BookDownloadManager implements WorkListener{ public void workStateChanged(WorkEvent ev) { Log.d("BookDownloadManager", ev.toString()); } - } diff --git a/MinimalBible/src/main/java/org/bspeice/minimalbible/activities/downloader/manager/BookDownloadThread.java b/MinimalBible/src/main/java/org/bspeice/minimalbible/activities/downloader/manager/BookDownloadThread.java index 7ab7e5a..cf4ac3b 100644 --- a/MinimalBible/src/main/java/org/bspeice/minimalbible/activities/downloader/manager/BookDownloadThread.java +++ b/MinimalBible/src/main/java/org/bspeice/minimalbible/activities/downloader/manager/BookDownloadThread.java @@ -4,18 +4,12 @@ import android.util.Log; import org.bspeice.minimalbible.MinimalBible; import org.crosswire.common.progress.JobManager; -import org.crosswire.common.progress.WorkEvent; -import org.crosswire.common.progress.WorkListener; import org.crosswire.jsword.book.Book; import org.crosswire.jsword.book.install.InstallException; import org.crosswire.jsword.book.install.Installer; -import java.util.UUID; - import javax.inject.Inject; -import de.greenrobot.event.EventBus; - /** * Created by bspeice on 5/12/14. */ @@ -25,6 +19,7 @@ public class BookDownloadThread { @Inject DownloadManager downloadManager; + @Inject RefreshManager refreshManager; public BookDownloadThread() { MinimalBible.getApplication().inject(this); @@ -34,7 +29,7 @@ public class BookDownloadThread { // So, the JobManager can't be injected, but we'll make do // First, look up where the Book came from - final Installer i = downloadManager.installerFromBook(b); + final Installer i = refreshManager.installerFromBook(b); final Thread worker = new Thread() { @Override @@ -47,17 +42,19 @@ public class BookDownloadThread { } }; worker.start(); + // The worker automatically communicates with the JobManager for its progress. - JobManager.createJob(getJobId(b), b.getName(), worker); - downloadManager.getRefreshBus().post(new DownloadProgressEvent(DownloadProgressEvent.PROGRESS_BEGINNING, b)); + downloadManager.getDownloadBus().post(new DLProgressEvent(DLProgressEvent.PROGRESS_BEGINNING, b)); } /** - * Return a job ID for a given book. Must be consistent per book. + * Build what the installer creates the job name as. + * Likely prone to be brittle. + * TODO: Make sure to test that this is an accurate job name * @param b - * @return A string representing this job IDs + * @return */ public static String getJobId(Book b) { - return b.toString(); + return "INSTALL_BOOK-" + b.getInitials().toUpperCase(); } } diff --git a/MinimalBible/src/main/java/org/bspeice/minimalbible/activities/downloader/manager/DownloadProgressEvent.java b/MinimalBible/src/main/java/org/bspeice/minimalbible/activities/downloader/manager/DLProgressEvent.java similarity index 69% rename from MinimalBible/src/main/java/org/bspeice/minimalbible/activities/downloader/manager/DownloadProgressEvent.java rename to MinimalBible/src/main/java/org/bspeice/minimalbible/activities/downloader/manager/DLProgressEvent.java index 45d109c..7c3f1f1 100644 --- a/MinimalBible/src/main/java/org/bspeice/minimalbible/activities/downloader/manager/DownloadProgressEvent.java +++ b/MinimalBible/src/main/java/org/bspeice/minimalbible/activities/downloader/manager/DLProgressEvent.java @@ -5,19 +5,23 @@ import org.crosswire.jsword.book.Book; /** * Used for notifying that a book's download progress is ongoing */ -public class DownloadProgressEvent { +public class DLProgressEvent { private int progress; private Book b; public static final int PROGRESS_COMPLETE = 100; public static final int PROGRESS_BEGINNING = 0; - public DownloadProgressEvent(int workDone, int totalWork, Book b) { - this.progress = workDone / totalWork; + public DLProgressEvent(int workDone, int totalWork, Book b) { + if (totalWork == 0) { + this.progress = 0; + } else { + this.progress = (int)((float) workDone / totalWork * 100); + } this.b = b; } - public DownloadProgressEvent(int workDone, Book b) { + public DLProgressEvent(int workDone, Book b) { this.progress = workDone; this.b = b; } diff --git a/MinimalBible/src/main/java/org/bspeice/minimalbible/activities/downloader/manager/DownloadManager.java b/MinimalBible/src/main/java/org/bspeice/minimalbible/activities/downloader/manager/DownloadManager.java index b665005..151a6ef 100644 --- a/MinimalBible/src/main/java/org/bspeice/minimalbible/activities/downloader/manager/DownloadManager.java +++ b/MinimalBible/src/main/java/org/bspeice/minimalbible/activities/downloader/manager/DownloadManager.java @@ -3,17 +3,12 @@ package org.bspeice.minimalbible.activities.downloader.manager; import android.util.Log; import org.bspeice.minimalbible.MinimalBible; -import org.crosswire.common.util.CWProject; -import org.crosswire.jsword.book.Book; import org.crosswire.jsword.book.BookCategory; import org.crosswire.jsword.book.install.InstallManager; import org.crosswire.jsword.book.install.Installer; import org.crosswire.jsword.book.sword.SwordBookPath; import java.io.File; -import java.util.ArrayList; -import java.util.HashMap; -import java.util.List; import java.util.Map; import javax.inject.Inject; @@ -27,20 +22,8 @@ public class DownloadManager { private final String TAG = "DownloadManager"; - /** - * Cached copy of modules that are available so we don't refresh for everyone who requests it. - */ - private Map> availableModules; - - /** - * Cached copy of downloads in progress so views displaying this info can get it quickly. - */ - private Map inProgressDownloads; - @Inject - protected EventBus refreshBus; - - @Inject BookDownloadManager bookDownloadManager; + protected EventBus downloadBus; public static final BookCategory[] VALID_CATEGORIES = { BookCategory.BIBLE, BookCategory.COMMENTARY, BookCategory.DICTIONARY, @@ -52,11 +35,6 @@ public class DownloadManager { public DownloadManager() { MinimalBible.getApplication().inject(this); setDownloadDir(); - - availableModules = new HashMap>(); - refreshModules(); - - inProgressDownloads = new HashMap(); } /** @@ -93,93 +71,10 @@ public class DownloadManager { } /** - * Do the work of kicking off the AsyncTask to refresh books, and make sure we know - * when it's done. + * Get the current download bus + * Used to broker refresh events, and ongoing download events */ - private void refreshModules() { - refreshBus.register(this); - new BookRefreshTask(refreshBus).execute(getInstallersArray()); + public EventBus getDownloadBus() { + return this.downloadBus; } - - /** - * When book refresh is done, cache the list so we can give that to someone else - * @param event A POJO wrapper around the Book list - */ - @SuppressWarnings("unused") - public void onEvent(EventBookList event) { - this.availableModules = event.getInstallerMapping(); - } - - /** - * Get the cached book list - * @return The cached book list, or null - */ - public List getBookList() { - if (availableModules.values().size() == 0) { - return null; - } else { - List bookList = new ArrayList(); - for (List l : availableModules.values()) { - bookList.addAll(l); - } - return bookList; - } - } - - /** - * Get the current download bus if you want to know when refresh is done. - * Please note that you will not be notified if the book refresh has already - * been completed, make sure to check {@link #getBookList()} first. - * @return The EventBus the DownloadManager is using - */ - public EventBus getRefreshBus() { - return this.refreshBus; - } - - // TODO: All code below should be migrated to BookDownloadManager - - /** - * Handle a book download progress event. - * Mostly responsible for caching the in progress status to check on it easily - * @param event - */ - public void onEvent(DownloadProgressEvent event) { - if (event.isComplete() && inProgressDownloads.containsKey(event.getB())) { - inProgressDownloads.remove(event.getB()); - } else { - inProgressDownloads.put(event.getB(), event); - } - } - - /** - * Check the status of a book download in progress. - * @param b - * @return The most recent DownloadProgressEvent for the book, or null if not downloading - */ - public DownloadProgressEvent getInProgressDownloadProgress(Book b) { - if (inProgressDownloads.containsKey(b)) { - return inProgressDownloads.get(b); - } else { - return null; - } - } - - /** - * Find the installer that a Book comes from. - * @param b The book to search for - * @return The Installer that should be used for this book. - */ - public Installer installerFromBook(Book b) { - for (Map.Entry> entry : availableModules.entrySet()) { - if (entry.getValue().contains(b)) { - return entry.getKey(); - } - } - return null; - } - - public void installBook(Book b) { - bookDownloadManager.downloadBook(b); - } - } diff --git a/MinimalBible/src/main/java/org/bspeice/minimalbible/activities/downloader/manager/RefreshManager.java b/MinimalBible/src/main/java/org/bspeice/minimalbible/activities/downloader/manager/RefreshManager.java new file mode 100644 index 0000000..95897a0 --- /dev/null +++ b/MinimalBible/src/main/java/org/bspeice/minimalbible/activities/downloader/manager/RefreshManager.java @@ -0,0 +1,84 @@ +package org.bspeice.minimalbible.activities.downloader.manager; + +import org.bspeice.minimalbible.MinimalBible; +import org.crosswire.jsword.book.Book; +import org.crosswire.jsword.book.install.Installer; + +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +import javax.inject.Inject; +import javax.inject.Singleton; + +import de.greenrobot.event.EventBus; + +/** + * Created by bspeice on 5/24/14. + */ +@Singleton +public class RefreshManager { + + @Inject DownloadManager downloadManager; + + /** + * Cached copy of modules that are available so we don't refresh for everyone who requests it. + */ + private Map> availableModules; + + public RefreshManager() { + MinimalBible.getApplication().inject(this); + availableModules = new HashMap>(); + refreshModules(); + } + + /** + * Do the work of kicking off the AsyncTask to refresh books, and make sure we know + * when it's done. + */ + private void refreshModules() { + EventBus refreshBus = downloadManager.getDownloadBus(); + refreshBus.register(this); + new BookRefreshTask(refreshBus).execute(downloadManager.getInstallersArray()); + } + + /** + * When book refresh is done, cache the list so we can give that to someone else + * @param event A POJO wrapper around the Book list + */ + @SuppressWarnings("unused") + public void onEvent(EventBookList event) { + this.availableModules = event.getInstallerMapping(); + } + + /** + * Get the cached book list + * @return The cached book list, or null + */ + public List getBookList() { + if (availableModules.values().size() == 0) { + return null; + } else { + List bookList = new ArrayList(); + for (List l : availableModules.values()) { + bookList.addAll(l); + } + return bookList; + } + } + + /** + * Find the installer that a Book comes from. + * @param b The book to search for + * @return The Installer that should be used for this book. + */ + public Installer installerFromBook(Book b) { + for (Map.Entry> entry : availableModules.entrySet()) { + if (entry.getValue().contains(b)) { + return entry.getKey(); + } + } + return null; + } +}