From 3a20994cb40f4cf6ee2bcc1aee66b9443efe3857 Mon Sep 17 00:00:00 2001 From: Maxwell Mapako Date: Mon, 25 Mar 2024 22:49:15 +0200 Subject: [PATCH] refactor: request rate reduction (#646) * refactor: reduce off screen page limit to reduce preemptive loading * refactor: add last sync marker for syncing local - remote user state * chore: minor refactoring and optimisations * chore: add documentation to public APIs * refactor: add `getModel` to ActivityBase --- app/src/main/assets/changelog.md | 2 +- .../base/custom/activity/ActivityBase.java | 11 +++++++++-- .../base/custom/fragment/FragmentBase.java | 4 +++- .../custom/view/image/AvatarIndicatorView.kt | 3 --- .../base/custom/viewmodel/ViewModelBase.kt | 5 +++++ .../anitrend/presenter/base/BasePresenter.kt | 17 +++++++++++++++++ .../main/java/com/mxt/anitrend/util/Settings.kt | 9 +++++++++ .../anitrend/util/graphql/AniGraphErrorUtil.kt | 14 +++++++------- .../view/activity/detail/CharacterActivity.java | 8 ++++---- .../view/activity/detail/MediaActivity.java | 14 ++++++++++---- .../view/activity/detail/MediaListActivity.java | 2 +- .../view/activity/detail/ProfileActivity.java | 12 ++++++++---- .../view/activity/detail/StaffActivity.java | 10 +++++----- .../view/activity/index/MainActivity.kt | 16 ++++++++-------- .../mxt/anitrend/worker/NotificationWorker.kt | 10 ++++------ 15 files changed, 91 insertions(+), 46 deletions(-) diff --git a/app/src/main/assets/changelog.md b/app/src/main/assets/changelog.md index 7ebb79cd6..ff2ef4e0e 100644 --- a/app/src/main/assets/changelog.md +++ b/app/src/main/assets/changelog.md @@ -6,7 +6,7 @@ Read the **FAQ** for issues regarding NSFW and notifications. Goto **Options** - ## What's Changed ### Enhancements -- Reduce the number of items loaded per page, hopefully will result in less request timeouts +- Additional optimizations to reduce the number of requests should result in slightly lower rate limit issues ### Current Issues - Gifs may show artifacts if more than one is playing at a given moment (with experimental markdown support in settings) diff --git a/app/src/main/java/com/mxt/anitrend/base/custom/activity/ActivityBase.java b/app/src/main/java/com/mxt/anitrend/base/custom/activity/ActivityBase.java index 906887ed5..95479d72c 100644 --- a/app/src/main/java/com/mxt/anitrend/base/custom/activity/ActivityBase.java +++ b/app/src/main/java/com/mxt/anitrend/base/custom/activity/ActivityBase.java @@ -21,6 +21,7 @@ import androidx.core.app.ActivityCompat; import androidx.core.content.ContextCompat; import androidx.lifecycle.Observer; +import androidx.lifecycle.ViewModelProvider; import androidx.lifecycle.ViewModelProviders; import com.miguelcatalan.materialsearchview.MaterialSearchView; @@ -73,7 +74,7 @@ public abstract class ActivityBase extends AppComp protected long id; - protected int offScreenLimit = 3; + protected int offScreenLimit = 2; protected boolean disableNavigationStyle; protected static final int REQUEST_PERMISSION = 102; @@ -366,7 +367,8 @@ protected ViewModelBase getViewModel() { @SuppressWarnings("unchecked") protected void setViewModel(boolean stateSupported) { if(viewModel == null) { - viewModel = ViewModelProviders.of(this).get(ViewModelBase.class); + ViewModelProvider provider = new ViewModelProvider(this); + viewModel = provider.get(ViewModelBase.class); viewModel.setContext(this); if(!viewModel.getModel().hasActiveObservers()) viewModel.getModel().observe(this, this); @@ -375,6 +377,11 @@ protected void setViewModel(boolean stateSupported) { } } + @Nullable + protected M getModel() { + return viewModel != null ? viewModel.snapshot() : null; + } + /** * Called when the model state is changed. * diff --git a/app/src/main/java/com/mxt/anitrend/base/custom/fragment/FragmentBase.java b/app/src/main/java/com/mxt/anitrend/base/custom/fragment/FragmentBase.java index 05990123d..26ce3bae9 100644 --- a/app/src/main/java/com/mxt/anitrend/base/custom/fragment/FragmentBase.java +++ b/app/src/main/java/com/mxt/anitrend/base/custom/fragment/FragmentBase.java @@ -19,6 +19,7 @@ import androidx.fragment.app.Fragment; import androidx.lifecycle.Lifecycle; import androidx.lifecycle.Observer; +import androidx.lifecycle.ViewModelProvider; import androidx.lifecycle.ViewModelProviders; import com.annimon.stream.IntPair; @@ -229,7 +230,8 @@ public ViewModelBase getViewModel() { @SuppressWarnings("unchecked") protected void setViewModel(boolean stateSupported) { if(viewModel == null) { - viewModel = ViewModelProviders.of(this).get(ViewModelBase.class); + ViewModelProvider provider = new ViewModelProvider(this); + viewModel = provider.get(ViewModelBase.class); viewModel.setContext(getContext()); if(!viewModel.getModel().hasActiveObservers()) viewModel.getModel().observe(this, this); diff --git a/app/src/main/java/com/mxt/anitrend/base/custom/view/image/AvatarIndicatorView.kt b/app/src/main/java/com/mxt/anitrend/base/custom/view/image/AvatarIndicatorView.kt index b2dc75df8..7577ff5cb 100644 --- a/app/src/main/java/com/mxt/anitrend/base/custom/view/image/AvatarIndicatorView.kt +++ b/app/src/main/java/com/mxt/anitrend/base/custom/view/image/AvatarIndicatorView.kt @@ -47,7 +47,6 @@ class AvatarIndicatorView : FrameLayout, CustomView, View.OnClickListener, BaseC get() = presenter.database?.currentUser private lateinit var binding: WidgetAvatarIndicatorBinding - private var mLastSynced: Long = 0 override fun onInit() { binding = WidgetAvatarIndicatorBinding.inflate(context.getLayoutInflater(), this, true) @@ -76,8 +75,6 @@ class AvatarIndicatorView : FrameLayout, CustomView, View.OnClickListener, BaseC @Subscribe(threadMode = ThreadMode.MAIN_ORDERED) override fun onModelChanged(consumer: BaseConsumer) { if (consumer.requestMode == KeyUtil.USER_CURRENT_REQ) { - if (DateUtil.timeDifferenceSatisfied(KeyUtil.TIME_UNIT_MINUTES, mLastSynced, 15)) - mLastSynced = System.currentTimeMillis() checkLastSyncTime() } } diff --git a/app/src/main/java/com/mxt/anitrend/base/custom/viewmodel/ViewModelBase.kt b/app/src/main/java/com/mxt/anitrend/base/custom/viewmodel/ViewModelBase.kt index 19cd4d00d..e429bf20d 100644 --- a/app/src/main/java/com/mxt/anitrend/base/custom/viewmodel/ViewModelBase.kt +++ b/app/src/main/java/com/mxt/anitrend/base/custom/viewmodel/ViewModelBase.kt @@ -33,6 +33,11 @@ class ViewModelBase: ViewModel(), RetroCallback { val params = Bundle() + /** + * @return A live data snapshot value [T]? at the time this function is invoked + */ + fun snapshot(): T? = model.value + fun setContext(context: Context?) { context?.apply { emptyMessage = getString(R.string.layout_empty_response) diff --git a/app/src/main/java/com/mxt/anitrend/presenter/base/BasePresenter.kt b/app/src/main/java/com/mxt/anitrend/presenter/base/BasePresenter.kt index edac7ae90..28a4cacea 100644 --- a/app/src/main/java/com/mxt/anitrend/presenter/base/BasePresenter.kt +++ b/app/src/main/java/com/mxt/anitrend/presenter/base/BasePresenter.kt @@ -35,6 +35,23 @@ open class BasePresenter(context: Context?) : CommonPresenter(context) { private var favouriteYears: List? = null private var favouriteFormats: List? = null + /** + * Avoids running a delegate if the specified interval time has not elapsed since the last + * call of this function + * + * @param intervalInMinutes time interval in minutes + * @param action delegate to invoke if the the time interval has elapsed since the last run + * + * @see KeyUtil.TIME_UNIT_MINUTES + */ + inline fun updateUserLastSyncTimeStampIf(intervalInMinutes: Int = 15, action: () -> Unit) { + val lastSyncedAt = settings.lastUserSyncTime + if (DateUtil.timeDifferenceSatisfied(KeyUtil.TIME_UNIT_MINUTES, lastSyncedAt, intervalInMinutes)) { + action() + settings.lastUserSyncTime = System.currentTimeMillis() + } + } + @IdRes fun getNavigationItem(): Int { return when (settings.startupPage) { diff --git a/app/src/main/java/com/mxt/anitrend/util/Settings.kt b/app/src/main/java/com/mxt/anitrend/util/Settings.kt index 6e364890f..7fc041c20 100644 --- a/app/src/main/java/com/mxt/anitrend/util/Settings.kt +++ b/app/src/main/java/com/mxt/anitrend/util/Settings.kt @@ -341,6 +341,14 @@ class Settings( } } + var lastUserSyncTime: Long + get() = getLong(_lastUserSyncTime, 0) + set(value) { + edit { + putLong(_lastUserSyncTime, value) + } + } + fun saveSeasonYear(year: Int) { edit { putInt(KeyUtil.arg_seasonYear, year) @@ -391,5 +399,6 @@ class Settings( private const val _reviewSort = "_reviewSort" private const val _staffSort = "_staffSort" private const val _experimentalMarkdown = "_experimentalMarkdown" + private const val _lastUserSyncTime = "_lastUserSyncTime" } } diff --git a/app/src/main/java/com/mxt/anitrend/util/graphql/AniGraphErrorUtil.kt b/app/src/main/java/com/mxt/anitrend/util/graphql/AniGraphErrorUtil.kt index 6469c34ff..e7cedbbc2 100644 --- a/app/src/main/java/com/mxt/anitrend/util/graphql/AniGraphErrorUtil.kt +++ b/app/src/main/java/com/mxt/anitrend/util/graphql/AniGraphErrorUtil.kt @@ -13,8 +13,6 @@ private const val HTTP_LIMIT_REACHED = 429 private const val TAG = "ErrorUtil" private const val Retry_After = "Retry-After" -private const val RateLimit_Limit = "X-RateLimit-Limit" -private const val RateLimit_Remaining = "X-RateLimit-Remaining" /** * Converts the response error response into an object. @@ -28,16 +26,18 @@ fun Response<*>?.apiError(): String { val headers = headers() val errors = getError() return if (code() != HTTP_LIMIT_REACHED) { - errors?.firstOrNull()?.message ?: "Unable to provide information regarding error!" - } else - "${headers.get(RateLimit_Remaining)} of ${headers.get(RateLimit_Limit)} requests remaining, please retry after ${headers.get(Retry_After)} seconds" + errors?.firstOrNull()?.message ?: "Unexpected HTTP/${code()} from server" + } else { + val waitPeriod = headers.get(Retry_After) ?: 60 + "Too many requests, please retry after $waitPeriod seconds" + } } } catch (ex: Exception) { ex.printStackTrace() Timber.tag(TAG).e(ex) - return "Unexpected error encountered" + return "Unable to recover from encountered error" } - return "Unable to provide information regarding error!" + return "Unable to provide information regarding error" } diff --git a/app/src/main/java/com/mxt/anitrend/view/activity/detail/CharacterActivity.java b/app/src/main/java/com/mxt/anitrend/view/activity/detail/CharacterActivity.java index 9c2fad007..3a8434c8e 100644 --- a/app/src/main/java/com/mxt/anitrend/view/activity/detail/CharacterActivity.java +++ b/app/src/main/java/com/mxt/anitrend/view/activity/detail/CharacterActivity.java @@ -40,8 +40,6 @@ public class CharacterActivity extends ActivityBase implements View.OnClickListener { private ActivitySeriesBinding binding; - private MediaBase model; private @KeyUtil.MediaType String mediaType; @@ -87,6 +86,7 @@ public boolean onCreateOptionsMenu(Menu menu) { getMenuInflater().inflate(R.menu.media_base_menu, menu); menu.findItem(R.id.action_favourite).setVisible(isAuth); + MediaBase model = getModel(); malMenuItem = menu.findItem(R.id.action_mal); malMenuItem.setVisible(model != null && model.getIdMal() > 0); @@ -104,6 +104,7 @@ public boolean onCreateOptionsMenu(Menu menu) { @Override public boolean onOptionsItemSelected(MenuItem item) { + MediaBase model = getModel(); if(model != null) { switch (item.getItemId()) { case R.id.action_manage: @@ -153,7 +154,7 @@ protected void onActivityReady() { @Override protected void onResume() { super.onResume(); - if(model == null) + if(getModel() == null) makeRequest(); else updateUI(); @@ -161,6 +162,7 @@ protected void onResume() { @Override protected void updateUI() { + MediaBase model = getModel(); if(model != null) { binding.setModel(model); binding.setOnClickListener(this); @@ -196,7 +198,6 @@ protected void makeRequest() { @Override public void onChanged(@Nullable MediaBase model) { super.onChanged(model); - this.model = model; updateUI(); } @@ -204,7 +205,10 @@ public void onChanged(@Nullable MediaBase model) { public void onClick(View view) { switch (view.getId()) { case R.id.series_banner: - CompatUtil.INSTANCE.imagePreview(view, model.getBannerImage(), R.string.image_preview_error_series_banner); + MediaBase model = getModel(); + if (model != null) { + CompatUtil.INSTANCE.imagePreview(view, model.getBannerImage(), R.string.image_preview_error_series_banner); + } break; } } @@ -217,6 +221,7 @@ protected void onDestroy() { } private void setMenuItemIcons() { + MediaBase model = getModel(); if (model != null) { if (model.getMediaListEntry() != null && manageMenuItem != null) manageMenuItem.setIcon(CompatUtil.INSTANCE.getDrawable(this, R.drawable.ic_mode_edit_white_24dp)); @@ -226,6 +231,7 @@ private void setMenuItemIcons() { } private void setFavouriteWidgetMenuItemIcon() { + MediaBase model = getModel(); if(model != null && favouriteWidget != null) favouriteWidget.setModel(model); } diff --git a/app/src/main/java/com/mxt/anitrend/view/activity/detail/MediaListActivity.java b/app/src/main/java/com/mxt/anitrend/view/activity/detail/MediaListActivity.java index ef49b20f1..4da5530b7 100644 --- a/app/src/main/java/com/mxt/anitrend/view/activity/detail/MediaListActivity.java +++ b/app/src/main/java/com/mxt/anitrend/view/activity/detail/MediaListActivity.java @@ -86,7 +86,7 @@ protected void onActivityReady() { @Override protected void updateUI() { viewPager.setAdapter(pageAdapter); - viewPager.setOffscreenPageLimit(offScreenLimit + 2); + viewPager.setOffscreenPageLimit(offScreenLimit); smartTabLayout.setViewPager(viewPager); } diff --git a/app/src/main/java/com/mxt/anitrend/view/activity/detail/ProfileActivity.java b/app/src/main/java/com/mxt/anitrend/view/activity/detail/ProfileActivity.java index 2a2c9b37b..d7936c60d 100644 --- a/app/src/main/java/com/mxt/anitrend/view/activity/detail/ProfileActivity.java +++ b/app/src/main/java/com/mxt/anitrend/view/activity/detail/ProfileActivity.java @@ -18,6 +18,7 @@ import com.mxt.anitrend.base.custom.consumer.BaseConsumer; import com.mxt.anitrend.base.custom.view.image.WideImageView; import com.mxt.anitrend.databinding.ActivityProfileBinding; +import com.mxt.anitrend.model.entity.base.StaffBase; import com.mxt.anitrend.model.entity.base.UserBase; import com.mxt.anitrend.presenter.base.BasePresenter; import com.mxt.anitrend.util.CompatUtil; @@ -47,7 +48,6 @@ public class ProfileActivity extends ActivityBase imple private ActivityProfileBinding binding; private String userName; - private UserBase model; @Override protected void onCreate(@Nullable Bundle savedInstanceState) { @@ -84,7 +84,7 @@ protected void onPostCreate(@Nullable Bundle savedInstanceState) { @Override protected void onPostResume() { super.onPostResume(); - if(model == null) + if(getModel() == null) onActivityReady(); else updateUI(); @@ -100,6 +100,7 @@ public boolean onCreateOptionsMenu(Menu menu) { @Override public boolean onOptionsItemSelected(MenuItem item) { + UserBase model = getModel(); switch (item.getItemId()) { case R.id.action_notification: startActivity(new Intent(ProfileActivity.this, NotificationActivity.class)); @@ -150,6 +151,7 @@ protected void onActivityReady() { protected void updateUI() { binding.setOnClickListener(this); binding.profileStatsWidget.setParams(getIntent().getExtras()); + UserBase model = getModel(); WideImageView.setImage(binding.profileBanner, model.getBannerImage()); if(getPresenter().isCurrentUser(model.getId())) { new TutorialUtil().setContext(this) @@ -191,7 +193,6 @@ public void onChanged(@Nullable UserBase model) { super.onChanged(model); if(model != null) { this.id = model.getId(); - this.model = model; updateUI(); } else NotifyUtil.INSTANCE.createAlerter(this, R.string.text_user_model, R.string.layout_empty_response, R.drawable.ic_warning_white_18dp, R.color.colorStateRed); @@ -201,7 +202,10 @@ public void onChanged(@Nullable UserBase model) { public void onClick(View view) { switch (view.getId()) { case R.id.profile_banner: - CompatUtil.INSTANCE.imagePreview(view, model.getBannerImage(), R.string.image_preview_error_profile_banner); + UserBase model = getModel(); + if (model != null) { + CompatUtil.INSTANCE.imagePreview(view, model.getBannerImage(), R.string.image_preview_error_profile_banner); + } break; } } diff --git a/app/src/main/java/com/mxt/anitrend/view/activity/detail/StaffActivity.java b/app/src/main/java/com/mxt/anitrend/view/activity/detail/StaffActivity.java index da36afac4..d18eca84d 100644 --- a/app/src/main/java/com/mxt/anitrend/view/activity/detail/StaffActivity.java +++ b/app/src/main/java/com/mxt/anitrend/view/activity/detail/StaffActivity.java @@ -45,8 +45,6 @@ public class StaffActivity extends ActivityBase { private Boolean onList; - private StaffBase model; - private FavouriteToolbarWidget favouriteWidget; @Override @@ -78,6 +76,7 @@ public boolean onCreateOptionsMenu(Menu menu) { if(isAuth) { MenuItem favouriteMenuItem = menu.findItem(R.id.action_favourite); favouriteWidget = (FavouriteToolbarWidget) favouriteMenuItem.getActionView(); + StaffBase model = getModel(); if(model != null) favouriteWidget.setModel(model); } @@ -86,6 +85,7 @@ public boolean onCreateOptionsMenu(Menu menu) { @Override public boolean onOptionsItemSelected(MenuItem item) { + StaffBase model = getModel(); if(model != null) { switch (item.getItemId()) { case R.id.action_share: @@ -132,14 +132,14 @@ protected void onActivityReady() { StaffPageAdapter pageAdapter = new StaffPageAdapter(getSupportFragmentManager(), getApplicationContext()); pageAdapter.setParams(getViewModel().getParams()); viewPager.setAdapter(pageAdapter); - viewPager.setOffscreenPageLimit(offScreenLimit + 1); + viewPager.setOffscreenPageLimit(offScreenLimit); smartTabLayout.setViewPager(viewPager); } @Override protected void onResume() { super.onResume(); - if(model == null) + if(getModel() == null) makeRequest(); else updateUI(); @@ -147,6 +147,7 @@ protected void onResume() { @Override protected void updateUI() { + StaffBase model = getModel(); if(model != null) if(favouriteWidget != null) favouriteWidget.setModel(model); @@ -168,7 +169,6 @@ protected void makeRequest() { @Override public void onChanged(@Nullable StaffBase model) { super.onChanged(model); - this.model = model; updateUI(); } diff --git a/app/src/main/java/com/mxt/anitrend/view/activity/index/MainActivity.kt b/app/src/main/java/com/mxt/anitrend/view/activity/index/MainActivity.kt index 38b0e9d82..95fbe5efe 100644 --- a/app/src/main/java/com/mxt/anitrend/view/activity/index/MainActivity.kt +++ b/app/src/main/java/com/mxt/anitrend/view/activity/index/MainActivity.kt @@ -124,8 +124,7 @@ class MainActivity : ActivityBase(), View.OnClickListener, setViewModel(true) if (savedInstanceState == null) redirectShortcut = intent.getIntExtra(KeyUtil.arg_redirect, 0) - if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.LOLLIPOP) - mNavigationView.itemBackground = getCompatDrawable(R.drawable.nav_background) + mNavigationView.itemBackground = getCompatDrawable(R.drawable.nav_background) mNavigationView.setNavigationItemSelectedListener(this) mViewPager.offscreenPageLimit = offScreenLimit mPageIndex = DateUtil.menuSelect @@ -408,13 +407,14 @@ class MainActivity : ActivityBase(), View.OnClickListener, } private fun requestCurrentUser() { - // Sync local current user data with remote if (presenter.settings.isAuthenticated) { - viewModel.params.putParcelable( - KeyUtil.arg_graph_params, - GraphUtil.getDefaultQuery(false) - ) - viewModel.requestData(KeyUtil.USER_CURRENT_REQ, this) + presenter.updateUserLastSyncTimeStampIf(intervalInMinutes = 5) { + viewModel.params.putParcelable( + KeyUtil.arg_graph_params, + GraphUtil.getDefaultQuery(false) + ) + viewModel.requestData(KeyUtil.USER_CURRENT_REQ, this) + } } } diff --git a/app/src/main/java/com/mxt/anitrend/worker/NotificationWorker.kt b/app/src/main/java/com/mxt/anitrend/worker/NotificationWorker.kt index e99bb4382..54049f91a 100644 --- a/app/src/main/java/com/mxt/anitrend/worker/NotificationWorker.kt +++ b/app/src/main/java/com/mxt/anitrend/worker/NotificationWorker.kt @@ -72,12 +72,10 @@ class NotificationWorker( private fun requestUser(): User? { val userGraphContainer = userEndpoint.getCurrentUser( GraphUtil.getDefaultQuery(false) - ).execute().body() as User? + ).execute().body() as? User? - return (userGraphContainer).let { - it?.also { user -> - presenter.database.currentUser = user - } + return userGraphContainer?.let { + presenter.database.currentUser = it it } } @@ -85,7 +83,7 @@ class NotificationWorker( private fun requestNotifications(user: User) { val notificationsContainer = userEndpoint.getUserNotifications( GraphUtil.getDefaultQuery(false) - ).execute().body() as PageContainer? + ).execute().body() as? PageContainer? if (user.unreadNotificationCount > 0 && notificationsContainer != null) notificationUtil.createNotification(user, notificationsContainer)