From f4ae62cc4882412e742a597aabf2930791ffe52a Mon Sep 17 00:00:00 2001 From: "David A. Velasco" Date: Thu, 29 Oct 2015 16:05:14 +0100 Subject: [PATCH] Simplified handling of life cycle in ShareActivity to grant nice operation while navigating and rotating --- .../android/ui/activity/FileActivity.java | 2 +- .../ui/activity/FileDisplayActivity.java | 78 ++++++++--- .../android/ui/activity/ShareActivity.java | 128 ++++++++---------- .../ui/fragment/SearchShareesFragment.java | 40 ++++-- .../ui/fragment/ShareFileFragment.java | 38 ++++-- 5 files changed, 169 insertions(+), 117 deletions(-) diff --git a/src/com/owncloud/android/ui/activity/FileActivity.java b/src/com/owncloud/android/ui/activity/FileActivity.java index e062acc098..249ae8dfd9 100644 --- a/src/com/owncloud/android/ui/activity/FileActivity.java +++ b/src/com/owncloud/android/ui/activity/FileActivity.java @@ -107,7 +107,7 @@ public class FileActivity extends AppCompatActivity private static final String KEY_TRY_SHARE_AGAIN = "TRY_SHARE_AGAIN"; private static final String KEY_ACTION_BAR_TITLE = "ACTION_BAR_TITLE"; - protected static final long DELAY_TO_REQUEST_OPERATION_ON_ACTIVITY_RESULTS = 200; + protected static final long DELAY_TO_REQUEST_OPERATIONS_LATER = 200; /** OwnCloud {@link Account} where the main {@link OCFile} handled by the activity is located.*/ diff --git a/src/com/owncloud/android/ui/activity/FileDisplayActivity.java b/src/com/owncloud/android/ui/activity/FileDisplayActivity.java index 95e7fad9ac..27d55c7bea 100644 --- a/src/com/owncloud/android/ui/activity/FileDisplayActivity.java +++ b/src/com/owncloud/android/ui/activity/FileDisplayActivity.java @@ -225,6 +225,13 @@ public class FileDisplayActivity extends HookActivity Log_OC.v(TAG, "onStart() end"); } + @Override + protected void onStop() { + Log_OC.v(TAG, "onStop() start"); + super.onStop(); + Log_OC.v(TAG, "onStop() end"); + } + @Override protected void onDestroy() { Log_OC.v(TAG, "onDestroy() start"); @@ -635,7 +642,7 @@ public class FileDisplayActivity extends HookActivity requestMoveOperation(fData, fResultCode); } }, - DELAY_TO_REQUEST_OPERATION_ON_ACTIVITY_RESULTS + DELAY_TO_REQUEST_OPERATIONS_LATER ); } else if (requestCode == ACTION_COPY_FILES && resultCode == RESULT_OK) { @@ -649,7 +656,7 @@ public class FileDisplayActivity extends HookActivity requestCopyOperation(fData, fResultCode); } }, - DELAY_TO_REQUEST_OPERATION_ON_ACTIVITY_RESULTS + DELAY_TO_REQUEST_OPERATIONS_LATER ); } else { @@ -982,6 +989,7 @@ public class FileDisplayActivity extends HookActivity } } + } removeStickyBroadcast(intent); Log_OC.d(TAG, "Setting progress visibility to " + mSyncInProgress); @@ -1634,26 +1642,60 @@ public class FileDisplayActivity extends HookActivity return null; } - public void startSyncFolderOperation(OCFile folder, boolean ignoreETag) { - long currentSyncTime = System.currentTimeMillis(); + /** + * Starts an operation to refresh the requested folder. + * + * The operation is run in a new background thread created on the fly. + * + * The refresh updates is a "light sync": properties of regular files in folder are updated (including + * associated shares), but not their contents. Only the contents of files marked to be kept-in-sync are + * synchronized too. + * + * @param folder Folder to refresh. + * @param ignoreETag If 'true', the data from the server will be fetched and sync'ed even if the eTag + * didn't change. + */ + public void startSyncFolderOperation(final OCFile folder, final boolean ignoreETag) { - mSyncInProgress = true; + // the execution is slightly delayed to allow the activity get the window focus if it's being started + // or if the method is called from a dialog that is being dismissed + getHandler().postDelayed( + new Runnable() { + @Override + public void run() { + if (hasWindowFocus()) { + long currentSyncTime = System.currentTimeMillis(); + mSyncInProgress = true; - // perform folder synchronization - RemoteOperation synchFolderOp = new RefreshFolderOperation( folder, - currentSyncTime, - false, - getFileOperationsHelper().isSharedSupported(), - ignoreETag, - getStorageManager(), - getAccount(), - getApplicationContext() + // perform folder synchronization + RemoteOperation synchFolderOp = new RefreshFolderOperation(folder, + currentSyncTime, + false, + getFileOperationsHelper().isSharedSupported(), + ignoreETag, + getStorageManager(), + getAccount(), + getApplicationContext() + ); + synchFolderOp.execute( + getAccount(), + MainApp.getAppContext(), + FileDisplayActivity.this, + null, + null + ); + + mProgressBar.setIndeterminate(true); + + setBackgroundText(); + + } // else: NOTHING ; lets' not refresh when the user rotates the device but there is + // another window floating over + } + }, + DELAY_TO_REQUEST_OPERATIONS_LATER ); - synchFolderOp.execute(getAccount(), MainApp.getAppContext(), this, null, null); - mProgressBar.setIndeterminate(true); - - setBackgroundText(); } /** diff --git a/src/com/owncloud/android/ui/activity/ShareActivity.java b/src/com/owncloud/android/ui/activity/ShareActivity.java index 9d4a769e1e..180b2a06f2 100644 --- a/src/com/owncloud/android/ui/activity/ShareActivity.java +++ b/src/com/owncloud/android/ui/activity/ShareActivity.java @@ -25,6 +25,7 @@ import android.app.SearchManager; import android.content.Intent; import android.net.Uri; import android.os.Bundle; +import android.support.v4.app.Fragment; import android.support.v4.app.FragmentTransaction; import com.owncloud.android.R; @@ -36,12 +37,10 @@ import com.owncloud.android.lib.common.operations.RemoteOperationResult; import com.owncloud.android.datamodel.OCFile; import com.owncloud.android.lib.resources.shares.OCShare; import com.owncloud.android.lib.resources.shares.ShareType; -import com.owncloud.android.operations.CreateShareWithShareeOperation; import com.owncloud.android.ui.fragment.SearchShareesFragment; import com.owncloud.android.ui.fragment.ShareFileFragment; import com.owncloud.android.utils.GetShareWithUsersAsyncTask; -import java.util.ArrayList; /** * Activity for sharing files @@ -56,60 +55,38 @@ public class ShareActivity extends FileActivity private static final String TAG_SHARE_FRAGMENT = "SHARE_FRAGMENT"; private static final String TAG_SEARCH_FRAGMENT = "SEARCH_USER_AND_GROUPS_FRAGMENT"; - private static final String DIALOG_WAIT_LOAD_DATA = "DIALOG_WAIT_LOAD_DATA"; - - private ShareFileFragment mShareFileFragment; - private SearchShareesFragment mSearchFragment; @Override protected void onCreate(Bundle savedInstanceState) { super.onCreate(savedInstanceState); - onAccountSet(false); setContentView(R.layout.share_activity); FragmentTransaction ft = getSupportFragmentManager().beginTransaction(); - if (savedInstanceState != null) { - - mShareFileFragment = (ShareFileFragment) getSupportFragmentManager(). - getFragment(savedInstanceState, TAG_SHARE_FRAGMENT); - mSearchFragment = (SearchShareesFragment) getSupportFragmentManager(). - getFragment(savedInstanceState, TAG_SEARCH_FRAGMENT); - - if (mShareFileFragment != null){ - ft.replace(R.id.share_fragment_container, mShareFileFragment, TAG_SHARE_FRAGMENT); - - if (mSearchFragment != null){ - ft.hide(mShareFileFragment); - ft.add(R.id.share_fragment_container, mSearchFragment, TAG_SEARCH_FRAGMENT); - } - ft.commit(); - } - - } else { - // Add Share fragment - mShareFileFragment = ShareFileFragment.newInstance(getFile(), getAccount()); - ft.replace(R.id.share_fragment_container, mShareFileFragment, TAG_SHARE_FRAGMENT); + if (savedInstanceState == null) { + // Add Share fragment on first creation + Fragment fragment = ShareFileFragment.newInstance(getFile(), getAccount()); + ft.replace(R.id.share_fragment_container, fragment, TAG_SHARE_FRAGMENT); ft.commit(); - - mSearchFragment = null; } - handleIntent(getIntent()); + } + protected void onAccountSet(boolean stateWasRecovered) { + super.onAccountSet(stateWasRecovered); + // Load data into the list + Log_OC.d(TAG, "Refreshing lists on account set"); + refreshUsersInLists(); + + // Request for a refresh of the data through the server (starts an Async Task) + refreshUsersOrGroupsListFromServer(); } @Override protected void onNewIntent(Intent intent) { - setIntent(intent); - handleIntent(intent); - } - - - private void handleIntent(Intent intent) { // Verify the action and get the query if (Intent.ACTION_SEARCH.equals(intent.getAction())) { String query = intent.getStringExtra(SearchManager.QUERY); @@ -135,30 +112,19 @@ public class ShareActivity extends FileActivity ); } - @Override - protected void onSaveInstanceState(Bundle outState) { - super.onSaveInstanceState(outState); - //Save the fragment's instance - getSupportFragmentManager().putFragment(outState, TAG_SHARE_FRAGMENT, mShareFileFragment); - if (mSearchFragment != null) { - getSupportFragmentManager().putFragment(outState, TAG_SEARCH_FRAGMENT, mSearchFragment); - } - - } - @Override public void showSearchUsersAndGroups() { + // replace ShareFragment with SearchFragment on demand FragmentTransaction ft = getSupportFragmentManager().beginTransaction(); - mSearchFragment = SearchShareesFragment.newInstance(getFile(), getAccount()); - ft.hide(mShareFileFragment); - ft.add(R.id.share_fragment_container, mSearchFragment, TAG_SEARCH_FRAGMENT); - ft.addToBackStack(TAG_SEARCH_FRAGMENT); + Fragment searchFragment = SearchShareesFragment.newInstance(getFile(), getAccount()); + ft.replace(R.id.share_fragment_container, searchFragment, TAG_SEARCH_FRAGMENT); + ft.addToBackStack(null); // BACK button will recover the ShareFragment ft.commit(); } @Override // Call to Unshare operation - public void unshareWith(OCShare share){ + public void unshareWith(OCShare share) { OCFile file = getFile(); getFileOperationsHelper().unshareFileWithUserOrGroup(file, share.getShareType(), share.getShareWith()); } @@ -167,25 +133,15 @@ public class ShareActivity extends FileActivity * Get users and groups from the server to fill in the "share with" list */ @Override - public void refreshUsersOrGroupsListFromServer(){ + public void refreshUsersOrGroupsListFromServer() { // Show loading showLoadingDialog(getString(R.string.common_loading)); // Get Users and Groups GetShareWithUsersAsyncTask getTask = new GetShareWithUsersAsyncTask(this); - Object[] params = { getFile(), getAccount(), getStorageManager()}; + Object[] params = {getFile(), getAccount(), getStorageManager()}; getTask.execute(params); } - @Override - public void onBackPressed() { - super.onBackPressed(); - if (mSearchFragment != null){ - mSearchFragment = null; - getSupportFragmentManager().popBackStackImmediate(); - mShareFileFragment.refreshUsersOrGroupsListFromDB(); - } - } - /** * Updates the view associated to the activity after the finish of some operation over files * in the current account. @@ -198,22 +154,44 @@ public class ShareActivity extends FileActivity super.onRemoteOperationFinish(operation, result); if (result.isSuccess()) { + Log_OC.d(TAG, "Refreshing lists on successful sync"); refreshUsersInLists(); - if (operation instanceof CreateShareWithShareeOperation) { - // Clean action - getIntent().setAction(null); + } + + } + + private void refreshUsersInLists() { + ShareFileFragment shareFileFragment = getShareFileFragment(); + if (shareFileFragment != null) { // only if added to the view hierarchy!! + if (shareFileFragment.isAdded()) { + shareFileFragment.refreshUsersOrGroupsListFromDB(); } } + SearchShareesFragment searchShareesFragment = getSearchFragment(); + if (searchShareesFragment != null) { + if (searchShareesFragment.isAdded()) { // only if added to the view hierarchy!! + searchShareesFragment.refreshUsersOrGroupsListFromDB(); + } + } } - private void refreshUsersInLists(){ - if (mShareFileFragment != null){ - mShareFileFragment.refreshUsersOrGroupsListFromDB(); - } - if (mSearchFragment != null) { - mSearchFragment.refreshUsersOrGroupsListFromDB(); - } + /** + * Shortcut to get access to the {@link ShareFileFragment} instance, if any + * + * @return A {@link ShareFileFragment} instance, or null + */ + private ShareFileFragment getShareFileFragment() { + return (ShareFileFragment) getSupportFragmentManager().findFragmentByTag(TAG_SHARE_FRAGMENT); + } + + /** + * Shortcut to get access to the {@link SearchShareesFragment} instance, if any + * + * @return A {@link SearchShareesFragment} instance, or null + */ + private SearchShareesFragment getSearchFragment() { + return (SearchShareesFragment) getSupportFragmentManager().findFragmentByTag(TAG_SEARCH_FRAGMENT); } } diff --git a/src/com/owncloud/android/ui/fragment/SearchShareesFragment.java b/src/com/owncloud/android/ui/fragment/SearchShareesFragment.java index 01c4728700..0059b3f9d0 100644 --- a/src/com/owncloud/android/ui/fragment/SearchShareesFragment.java +++ b/src/com/owncloud/android/ui/fragment/SearchShareesFragment.java @@ -39,18 +39,21 @@ import com.owncloud.android.R; import com.owncloud.android.datamodel.OCFile; import com.owncloud.android.lib.common.utils.Log_OC; import com.owncloud.android.lib.resources.shares.OCShare; +import com.owncloud.android.ui.activity.FileActivity; import com.owncloud.android.ui.activity.ShareActivity; import com.owncloud.android.ui.adapter.ShareUserListAdapter; import java.util.ArrayList; /** - * Fragment for Searching users and groups + * Fragment for Searching sharees (users and groups) * * A simple {@link Fragment} subclass. + * * Activities that contain this fragment must implement the * {@link SearchShareesFragment.OnSearchFragmentInteractionListener} interface * to handle interaction events. + * * Use the {@link SearchShareesFragment#newInstance} factory method to * create an instance of this fragment. */ @@ -65,16 +68,17 @@ public class SearchShareesFragment extends Fragment implements ShareUserListAdap private OCFile mFile; private Account mAccount; + // other members private ArrayList mShares; private ShareUserListAdapter mUserGroupsAdapter = null; - private OnSearchFragmentInteractionListener mListener; + /** * Public factory method to create new SearchShareesFragment instances. * * @param fileToShare An {@link OCFile} to be shared - * @param account An ownCloud account + * @param account The ownCloud account containing fileToShare * @return A new instance of fragment SearchShareesFragment. */ public static SearchShareesFragment newInstance(OCFile fileToShare, Account account) { @@ -90,6 +94,9 @@ public class SearchShareesFragment extends Fragment implements ShareUserListAdap // Required empty public constructor } + /** + * {@inheritDoc} + */ @Override public void onCreate(Bundle savedInstanceState) { super.onCreate(savedInstanceState); @@ -100,6 +107,9 @@ public class SearchShareesFragment extends Fragment implements ShareUserListAdap } + /** + * {@inheritDoc} + */ @Override public View onCreateView(LayoutInflater inflater, ViewGroup container, Bundle savedInstanceState) { @@ -144,21 +154,31 @@ public class SearchShareesFragment extends Fragment implements ShareUserListAdap /** - * Get users and groups fromn the DB to fill in the "share with" list + * Get users and groups from the DB to fill in the "share with" list + * + * Depends on the parent Activity provides a {@link com.owncloud.android.datamodel.FileDataStorageManager} + * instance ready to use. If not ready, does nothing. */ public void refreshUsersOrGroupsListFromDB (){ // Get Users and Groups - mShares = ((ShareActivity) mListener).getStorageManager().getSharesWithForAFile(mFile.getRemotePath(), - mAccount.name); + if (((FileActivity) mListener).getStorageManager() != null) { + mShares = ((FileActivity) mListener).getStorageManager().getSharesWithForAFile( + mFile.getRemotePath(), + mAccount.name + ); - // Update list of users/groups - updateListOfUserGroups(); + // Update list of users/groups + updateListOfUserGroups(); + } } private void updateListOfUserGroups() { // Update list of users/groups - mUserGroupsAdapter = new ShareUserListAdapter(getActivity().getApplicationContext(), - R.layout.share_user_item, mShares, this); + // TODO Refactoring: create a new {@link ShareUserListAdapter} instance with every call should not be needed + mUserGroupsAdapter = new ShareUserListAdapter( + getActivity().getApplicationContext(), + R.layout.share_user_item, mShares, this + ); // Show data ListView usersList = (ListView) getView().findViewById(R.id.searchUsersListView); diff --git a/src/com/owncloud/android/ui/fragment/ShareFileFragment.java b/src/com/owncloud/android/ui/fragment/ShareFileFragment.java index 5ee2a87ca5..a0296533cf 100644 --- a/src/com/owncloud/android/ui/fragment/ShareFileFragment.java +++ b/src/com/owncloud/android/ui/fragment/ShareFileFragment.java @@ -41,6 +41,7 @@ import com.owncloud.android.datamodel.OCFile; import com.owncloud.android.datamodel.ThumbnailsCacheManager; import com.owncloud.android.lib.common.utils.Log_OC; import com.owncloud.android.lib.resources.shares.OCShare; +import com.owncloud.android.ui.activity.FileActivity; import com.owncloud.android.ui.activity.ShareActivity; import com.owncloud.android.ui.adapter.ShareUserListAdapter; import com.owncloud.android.utils.DisplayUtils; @@ -49,12 +50,14 @@ import com.owncloud.android.utils.MimetypeIconUtil; import java.util.ArrayList; /** - * Fragment for Sharing a file with users + * Fragment for Sharing a file with sharees (users or groups) * * A simple {@link Fragment} subclass. + * * Activities that contain this fragment must implement the * {@link ShareFileFragment.OnShareFragmentInteractionListener} interface * to handle interaction events. + * * Use the {@link ShareFileFragment#newInstance} factory method to * create an instance of this fragment. */ @@ -71,9 +74,9 @@ public class ShareFileFragment extends Fragment private OCFile mFile; private Account mAccount; + // other members private ArrayList mShares; private ShareUserListAdapter mUserGroupsAdapter = null; - private OnShareFragmentInteractionListener mListener; /** @@ -96,6 +99,9 @@ public class ShareFileFragment extends Fragment // Required empty public constructor } + /** + * {@inheritDoc} + */ @Override public void onCreate(Bundle savedInstanceState) { super.onCreate(savedInstanceState); @@ -105,6 +111,9 @@ public class ShareFileFragment extends Fragment } } + /** + * {@inheritDoc} + */ @Override public View onCreateView(LayoutInflater inflater, ViewGroup container, Bundle savedInstanceState) { @@ -160,11 +169,6 @@ public class ShareFileFragment extends Fragment // Load data into the list refreshUsersOrGroupsListFromDB(); - - // Request for a refresh of the data through the server (starts an Async Task) - if (mListener != null) { - mListener.refreshUsersOrGroupsListFromServer(); - } } @Override @@ -185,19 +189,27 @@ public class ShareFileFragment extends Fragment } /** - * Get users and groups fromn the DB to fill in the "share with" list + * Get users and groups from the DB to fill in the "share with" list + * + * Depends on the parent Activity provides a {@link com.owncloud.android.datamodel.FileDataStorageManager} + * instance ready to use. If not ready, does nothing. */ public void refreshUsersOrGroupsListFromDB (){ - // Get Users and Groups - mShares = ((ShareActivity) mListener).getStorageManager().getSharesWithForAFile(mFile.getRemotePath(), - mAccount.name); + if (((FileActivity) mListener).getStorageManager() != null) { + // Get Users and Groups + mShares = ((FileActivity) mListener).getStorageManager().getSharesWithForAFile( + mFile.getRemotePath(), + mAccount.name + ); - // Update list of users/groups - updateListOfUserGroups(); + // Update list of users/groups + updateListOfUserGroups(); + } } private void updateListOfUserGroups() { // Update list of users/groups + // TODO Refactoring: create a new {@link ShareUserListAdapter} instance with every call should not be needed mUserGroupsAdapter = new ShareUserListAdapter( getActivity(), R.layout.share_user_item,