From 8fd287a2b36f1d28ee1c1ca67687abaab4e95f2f Mon Sep 17 00:00:00 2001 From: Damyan Ivanov Date: Fri, 29 Mar 2019 19:53:43 +0200 Subject: [PATCH] single observer instances, single place for reloading account/transaction lists fixes observer leak which also multiplied the reloads with each screen rotation (and activity restart) --- .../net/ktnx/mobileledger/model/Data.java | 11 +- .../AccountSummaryFragment.java | 1 - .../ui/activity/MainActivity.java | 152 ++++++++++------ .../TransactionListFragment.java | 1 - .../TransactionListViewModel.java | 1 + .../net/ktnx/mobileledger/utils/Colors.java | 167 +++++++++--------- 6 files changed, 189 insertions(+), 144 deletions(-) diff --git a/app/src/main/java/net/ktnx/mobileledger/model/Data.java b/app/src/main/java/net/ktnx/mobileledger/model/Data.java index 7380d613..2559fd86 100644 --- a/app/src/main/java/net/ktnx/mobileledger/model/Data.java +++ b/app/src/main/java/net/ktnx/mobileledger/model/Data.java @@ -28,7 +28,6 @@ import net.ktnx.mobileledger.utils.ObservableValue; import java.util.ArrayList; import java.util.Date; -import java.util.List; public final class Data { public static ObservableList transactions = new ObservableList<>(new ArrayList<>()); @@ -53,6 +52,16 @@ public final class Data { return -1; } } + public static int getProfileIndex(String profileUUID) { + try (LockHolder lh = profiles.lockForReading()) { + for (int i = 0; i < profiles.size(); i++) { + MobileLedgerProfile p = profiles.get(i); + if (p.getUuid().equals(profileUUID)) return i; + } + + return -1; + } + } public static int retrieveCurrentThemeIdFromDb() { String profileUUID = MLDB.getOption(MLDB.OPT_PROFILE_UUID, null); if (profileUUID == null) return -1; diff --git a/app/src/main/java/net/ktnx/mobileledger/ui/account_summary/AccountSummaryFragment.java b/app/src/main/java/net/ktnx/mobileledger/ui/account_summary/AccountSummaryFragment.java index 14ec0209..93975b05 100644 --- a/app/src/main/java/net/ktnx/mobileledger/ui/account_summary/AccountSummaryFragment.java +++ b/app/src/main/java/net/ktnx/mobileledger/ui/account_summary/AccountSummaryFragment.java @@ -167,7 +167,6 @@ public class AccountSummaryFragment extends MobileLedgerListFragment { Data.accounts.addObserver( (o, arg) -> mActivity.runOnUiThread(() -> modelAdapter.notifyDataSetChanged())); - update_account_table(); } private void update_account_table() { if (this.getContext() == null) return; diff --git a/app/src/main/java/net/ktnx/mobileledger/ui/activity/MainActivity.java b/app/src/main/java/net/ktnx/mobileledger/ui/activity/MainActivity.java index ec348098..7373d0d8 100644 --- a/app/src/main/java/net/ktnx/mobileledger/ui/activity/MainActivity.java +++ b/app/src/main/java/net/ktnx/mobileledger/ui/activity/MainActivity.java @@ -158,64 +158,81 @@ public class MainActivity extends ProfileThemedActivity { Toolbar toolbar = findViewById(R.id.toolbar); setSupportActionBar(toolbar); - profileObserver = (o, arg) -> { - MobileLedgerProfile profile = Data.profile.get(); - MainActivity.this.runOnUiThread(() -> { - if (profile == null) MainActivity.this.setTitle(R.string.app_name); - else MainActivity.this.setTitle(profile.getName()); - MainActivity.this.updateLastUpdateTextFromDB(); - if (profile.isPostingPermitted()) { - toolbar.setSubtitle(null); - fab.show(); - } - else { - toolbar.setSubtitle(R.string.profile_subitlte_read_only); - fab.hide(); - } - - int old_index = -1; - int new_index = -1; - if (arg != null) { - MobileLedgerProfile old = (MobileLedgerProfile) arg; - old_index = Data.getProfileIndex(old); - new_index = Data.getProfileIndex(profile); - } - - if ((old_index != -1) && (new_index != -1)) { - mProfileListAdapter.notifyItemChanged(old_index); - mProfileListAdapter.notifyItemChanged(new_index); - } - else mProfileListAdapter.notifyDataSetChanged(); - - MainActivity.this.collapseProfileList(); - - int newProfileTheme = profile.getThemeId(); - if (newProfileTheme != Colors.profileThemeId) { - Log.d("profiles", String.format("profile theme %d → %d", Colors.profileThemeId, - newProfileTheme)); - MainActivity.this.profileThemeChanged(); - Colors.profileThemeId = newProfileTheme; - } - else drawer.closeDrawers(); + if (profileObserver == null) { + profileObserver = (o, arg) -> { + MobileLedgerProfile profile = Data.profile.get(); + MainActivity.this.runOnUiThread(() -> { + + Data.transactions.clear(); + Log.d("transactions", "requesting list reload"); + TransactionListViewModel.scheduleTransactionListReload(); + + Data.accounts.clear(); + AccountSummaryViewModel.scheduleAccountListReload(); + + if (profile == null) MainActivity.this.setTitle(R.string.app_name); + else MainActivity.this.setTitle(profile.getName()); + MainActivity.this.updateLastUpdateTextFromDB(); + int old_index = -1; + int new_index = -1; + if (arg != null) { + MobileLedgerProfile old = (MobileLedgerProfile) arg; + old_index = Data.getProfileIndex(old); + new_index = Data.getProfileIndex(profile); + } - Log.d("transactions", "requesting list reload"); - TransactionListViewModel.scheduleTransactionListReload(); + if ((old_index != -1) && (new_index != -1)) { + mProfileListAdapter.notifyItemChanged(old_index); + mProfileListAdapter.notifyItemChanged(new_index); + } + else mProfileListAdapter.notifyDataSetChanged(); + + MainActivity.this.collapseProfileList(); + + int newProfileTheme = (profile == null) ? -1 : profile.getThemeId(); + if (newProfileTheme != Colors.profileThemeId) { + Log.d("profiles", + String.format("profile theme %d → %d", Colors.profileThemeId, + newProfileTheme)); + MainActivity.this.profileThemeChanged(); + Colors.profileThemeId = newProfileTheme; + // profileThemeChanged would restart the activity, so no need to reload the + // data sets below + return; + } + drawer.closeDrawers(); - AccountSummaryViewModel.scheduleAccountListReload(); + if (profile == null) { + toolbar.setSubtitle(null); + fab.hide(); + } + else { + if (profile.isPostingPermitted()) { + toolbar.setSubtitle(null); + fab.show(); + } + else { + toolbar.setSubtitle(R.string.profile_subitlte_read_only); + fab.hide(); + } + } + }); + }; + Data.profile.addObserver(profileObserver); + } - }); - }; - Data.profile.addObserver(profileObserver); - profilesObserver = (o, arg) -> { - findViewById(R.id.nav_profile_list).setMinimumHeight( - (int) (getResources().getDimension(R.dimen.thumb_row_height) * - Data.profiles.size())); - - Log.d("profiles", "profile list changed"); - if (arg == null) mProfileListAdapter.notifyDataSetChanged(); - else mProfileListAdapter.notifyItemChanged((int) arg); - }; - Data.profiles.addObserver(profilesObserver); + if (profilesObserver == null) { + profilesObserver = (o, arg) -> { + findViewById(R.id.nav_profile_list).setMinimumHeight( + (int) (getResources().getDimension(R.dimen.thumb_row_height) * + Data.profiles.size())); + + Log.d("profiles", "profile list changed"); + if (arg == null) mProfileListAdapter.notifyDataSetChanged(); + else mProfileListAdapter.notifyItemChanged((int) arg); + }; + Data.profiles.addObserver(profilesObserver); + } ActionBarDrawerToggle toggle = new ActionBarDrawerToggle(this, drawer, toolbar, R.string.navigation_drawer_open, @@ -356,6 +373,20 @@ public class MainActivity extends ProfileThemedActivity { Log.d("main", String.format("Date formatted: %s", text)); } } + @Override + public void finish() { + if (profilesObserver != null) { + Data.profiles.deleteObserver(profilesObserver); + profilesObserver = null; + } + + if (profileObserver != null) { + Data.profile.deleteObserver(profileObserver); + profileObserver = null; + } + + super.finish(); + } private void profileThemeChanged() { setupProfileColors(); @@ -381,7 +412,16 @@ public class MainActivity extends ProfileThemedActivity { String profileUUID = MLDB.getOption(MLDB.OPT_PROFILE_UUID, null); MobileLedgerProfile profile; - profile = MobileLedgerProfile.loadAllFromDB(profileUUID); + if (Data.profiles.isEmpty()) { + profile = MobileLedgerProfile.loadAllFromDB(profileUUID); + } + else { + try(LockHolder lh = Data.profiles.lockForReading()) { + int i = Data.getProfileIndex(profileUUID); + if (i == -1 ) i = 0; + profile = Data.profiles.get(i); + } + } if (Data.profiles.isEmpty()) { findViewById(R.id.no_profiles_layout).setVisibility(View.VISIBLE); diff --git a/app/src/main/java/net/ktnx/mobileledger/ui/transaction_list/TransactionListFragment.java b/app/src/main/java/net/ktnx/mobileledger/ui/transaction_list/TransactionListFragment.java index 631205f2..bb0355ce 100644 --- a/app/src/main/java/net/ktnx/mobileledger/ui/transaction_list/TransactionListFragment.java +++ b/app/src/main/java/net/ktnx/mobileledger/ui/transaction_list/TransactionListFragment.java @@ -198,7 +198,6 @@ public class TransactionListFragment extends MobileLedgerListFragment { accountFilter.addObserver(accountFilterObserver); } - TransactionListViewModel.scheduleTransactionListReload(); TransactionListViewModel.updating.addObserver( (o, arg) -> swiper.setRefreshing(TransactionListViewModel.updating.get())); TransactionListViewModel.updateError.addObserver(new Observer() { diff --git a/app/src/main/java/net/ktnx/mobileledger/ui/transaction_list/TransactionListViewModel.java b/app/src/main/java/net/ktnx/mobileledger/ui/transaction_list/TransactionListViewModel.java index 6e20f652..9e7f77c5 100644 --- a/app/src/main/java/net/ktnx/mobileledger/ui/transaction_list/TransactionListViewModel.java +++ b/app/src/main/java/net/ktnx/mobileledger/ui/transaction_list/TransactionListViewModel.java @@ -22,6 +22,7 @@ import android.os.AsyncTask; import net.ktnx.mobileledger.async.UpdateTransactionsTask; import net.ktnx.mobileledger.model.Data; import net.ktnx.mobileledger.model.TransactionListItem; +import net.ktnx.mobileledger.utils.LockHolder; import net.ktnx.mobileledger.utils.ObservableValue; import androidx.lifecycle.ViewModel; diff --git a/app/src/main/java/net/ktnx/mobileledger/utils/Colors.java b/app/src/main/java/net/ktnx/mobileledger/utils/Colors.java index 8e526c1f..acfea2f4 100644 --- a/app/src/main/java/net/ktnx/mobileledger/utils/Colors.java +++ b/app/src/main/java/net/ktnx/mobileledger/utils/Colors.java @@ -161,91 +161,88 @@ public class Colors { setupTheme(activity, profile); } public static void setupTheme(Activity activity, MobileLedgerProfile profile) { - if (profile != null) { - final int themeId = profile.getThemeId(); - switch (themeId) { - case 0: - case 360: - activity.setTheme(R.style.AppTheme_NoActionBar_0); - break; - case 15: - activity.setTheme(R.style.AppTheme_NoActionBar_15); - break; - case 30: - activity.setTheme(R.style.AppTheme_NoActionBar_30); - break; - case 45: - activity.setTheme(R.style.AppTheme_NoActionBar_45); - break; - case 60: - activity.setTheme(R.style.AppTheme_NoActionBar_60); - break; - case 75: - activity.setTheme(R.style.AppTheme_NoActionBar_75); - break; - case 90: - activity.setTheme(R.style.AppTheme_NoActionBar_90); - break; - case 105: - activity.setTheme(R.style.AppTheme_NoActionBar_105); - break; - case 120: - activity.setTheme(R.style.AppTheme_NoActionBar_120); - break; - case 135: - activity.setTheme(R.style.AppTheme_NoActionBar_135); - break; - case 150: - activity.setTheme(R.style.AppTheme_NoActionBar_150); - break; - case 165: - activity.setTheme(R.style.AppTheme_NoActionBar_165); - break; - case 180: - activity.setTheme(R.style.AppTheme_NoActionBar_180); - break; - case 195: - activity.setTheme(R.style.AppTheme_NoActionBar_195); - break; - case 210: - activity.setTheme(R.style.AppTheme_NoActionBar_210); - break; - case 225: - activity.setTheme(R.style.AppTheme_NoActionBar_225); - break; - case 240: - activity.setTheme(R.style.AppTheme_NoActionBar_240); - break; - case 255: - activity.setTheme(R.style.AppTheme_NoActionBar_255); - break; - case 270: - activity.setTheme(R.style.AppTheme_NoActionBar_270); - break; - case 285: - activity.setTheme(R.style.AppTheme_NoActionBar_285); - break; - case 300: - activity.setTheme(R.style.AppTheme_NoActionBar_300); - break; - case 315: - activity.setTheme(R.style.AppTheme_NoActionBar_315); - break; - case 330: - activity.setTheme(R.style.AppTheme_NoActionBar_330); - break; - case 345: - activity.setTheme(R.style.AppTheme_NoActionBar_345); - break; - default: - activity.setTheme(R.style.AppTheme_NoActionBar); - Log.d("profiles", String.format("Theme hue %d not supported, using the default", - themeId)); - } - } - else { - Log.d("profiles", "No profile given, using default theme"); - activity.setTheme(R.style.AppTheme_NoActionBar); + final int themeId = (profile == null) ? -1 : profile.getThemeId(); + setupTheme(activity, themeId); + } + public static void setupTheme(Activity activity, int themeId) { + switch (themeId) { + case 0: + case 360: + activity.setTheme(R.style.AppTheme_NoActionBar_0); + break; + case 15: + activity.setTheme(R.style.AppTheme_NoActionBar_15); + break; + case 30: + activity.setTheme(R.style.AppTheme_NoActionBar_30); + break; + case 45: + activity.setTheme(R.style.AppTheme_NoActionBar_45); + break; + case 60: + activity.setTheme(R.style.AppTheme_NoActionBar_60); + break; + case 75: + activity.setTheme(R.style.AppTheme_NoActionBar_75); + break; + case 90: + activity.setTheme(R.style.AppTheme_NoActionBar_90); + break; + case 105: + activity.setTheme(R.style.AppTheme_NoActionBar_105); + break; + case 120: + activity.setTheme(R.style.AppTheme_NoActionBar_120); + break; + case 135: + activity.setTheme(R.style.AppTheme_NoActionBar_135); + break; + case 150: + activity.setTheme(R.style.AppTheme_NoActionBar_150); + break; + case 165: + activity.setTheme(R.style.AppTheme_NoActionBar_165); + break; + case 180: + activity.setTheme(R.style.AppTheme_NoActionBar_180); + break; + case 195: + activity.setTheme(R.style.AppTheme_NoActionBar_195); + break; + case 210: + activity.setTheme(R.style.AppTheme_NoActionBar_210); + break; + case 225: + activity.setTheme(R.style.AppTheme_NoActionBar_225); + break; + case 240: + activity.setTheme(R.style.AppTheme_NoActionBar_240); + break; + case 255: + activity.setTheme(R.style.AppTheme_NoActionBar_255); + break; + case 270: + activity.setTheme(R.style.AppTheme_NoActionBar_270); + break; + case 285: + activity.setTheme(R.style.AppTheme_NoActionBar_285); + break; + case 300: + activity.setTheme(R.style.AppTheme_NoActionBar_300); + break; + case 315: + activity.setTheme(R.style.AppTheme_NoActionBar_315); + break; + case 330: + activity.setTheme(R.style.AppTheme_NoActionBar_330); + break; + case 345: + activity.setTheme(R.style.AppTheme_NoActionBar_345); + break; + default: + activity.setTheme(R.style.AppTheme_NoActionBar); + Log.d("profiles", + String.format("Theme hue %d not supported, using the default", themeId)); } refreshColors(activity.getTheme()); -- 2.39.5