From f5e5a70d2104d1c4d0b827afd65b8c5666110386 Mon Sep 17 00:00:00 2001 From: Andrea Torlaschi Date: Sun, 21 Aug 2016 17:19:42 +0200 Subject: [PATCH] ImportKeys: Refactoring --- .../processing/ImportKeysListCloudLoader.java | 28 +++---- .../processing/ImportKeysListLoader.java | 31 ++------ .../processing/ImportKeysListener.java | 6 +- .../keychain/ui/ImportKeysActivity.java | 15 ++-- .../keychain/ui/ImportKeysListFragment.java | 78 ++++--------------- .../ui/adapter/ImportKeysAdapter.java | 2 +- .../keychain/util/ParcelableFileCache.java | 4 - 7 files changed, 49 insertions(+), 115 deletions(-) diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/keyimport/processing/ImportKeysListCloudLoader.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/keyimport/processing/ImportKeysListCloudLoader.java index b06489d60..34d2682ae 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/keyimport/processing/ImportKeysListCloudLoader.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/keyimport/processing/ImportKeysListCloudLoader.java @@ -41,9 +41,7 @@ public class ImportKeysListCloudLoader extends AsyncTaskLoader>> { private Context mContext; - - private String mServerQuery; - private Preferences.CloudSearchPrefs mCloudPrefs; + private CloudLoaderState mState; private ParcelableProxy mParcelableProxy; private ArrayList mEntryList = new ArrayList<>(); @@ -52,20 +50,18 @@ public class ImportKeysListCloudLoader /** * Searches a keyserver as specified in cloudPrefs, using an explicit proxy if passed * - * @param serverQuery string to search on servers for. If is a fingerprint, - * will enforce fingerprint check - * @param cloudPrefs contains keyserver to search on, whether to search on the keyserver, - * and whether to search keybase.io + * @param loaderState state containing the string to search on servers for (if it is a + * fingerprint, will enforce fingerprint check) and the keyserver to + * search on (whether to search on the keyserver, and whether to search + * keybase.io) * @param parcelableProxy explicit proxy to use. If null, will retrieve from preferences */ - public ImportKeysListCloudLoader(Context context, String serverQuery, - Preferences.CloudSearchPrefs cloudPrefs, + public ImportKeysListCloudLoader(Context context, CloudLoaderState loaderState, @Nullable ParcelableProxy parcelableProxy) { super(context); mContext = context; - mServerQuery = serverQuery; - mCloudPrefs = cloudPrefs; + mState = loaderState; mParcelableProxy = parcelableProxy; } @@ -73,12 +69,12 @@ public class ImportKeysListCloudLoader public AsyncTaskResultWrapper> loadInBackground() { mEntryListWrapper = new AsyncTaskResultWrapper<>(mEntryList, null); - if (mServerQuery == null) { + if (mState.mServerQuery == null) { Log.e(Constants.TAG, "mServerQuery is null!"); return mEntryListWrapper; } - if (mServerQuery.startsWith("0x") && mServerQuery.length() == 42) { + if (mState.mServerQuery.startsWith("0x") && mState.mServerQuery.length() == 42) { Log.d(Constants.TAG, "This search is based on a unique fingerprint. Enforce a fingerprint check!"); queryServer(true); } else { @@ -143,15 +139,15 @@ public class ImportKeysListCloudLoader try { ArrayList searchResult = CloudSearch.search( - mServerQuery, - mCloudPrefs, + mState.mServerQuery, + mState.mCloudPrefs, parcelableProxy.getProxy() ); mEntryList.clear(); // add result to data if (enforceFingerprint) { - String fingerprint = mServerQuery.substring(2); + String fingerprint = mState.mServerQuery.substring(2); Log.d(Constants.TAG, "fingerprint: " + fingerprint); // query must return only one result! if (searchResult.size() == 1) { diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/keyimport/processing/ImportKeysListLoader.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/keyimport/processing/ImportKeysListLoader.java index 717dbe511..f4afecc09 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/keyimport/processing/ImportKeysListLoader.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/keyimport/processing/ImportKeysListLoader.java @@ -24,7 +24,6 @@ import android.support.v4.content.AsyncTaskLoader; import org.sufficientlysecure.keychain.Constants; import org.sufficientlysecure.keychain.keyimport.ImportKeysListEntry; -import org.sufficientlysecure.keychain.keyimport.ParcelableKeyRing; import org.sufficientlysecure.keychain.operations.results.GetKeyResult; import org.sufficientlysecure.keychain.operations.results.OperationResult; import org.sufficientlysecure.keychain.operations.results.OperationResult.LogType; @@ -42,22 +41,20 @@ import java.io.FileNotFoundException; import java.io.IOException; import java.io.InputStream; import java.util.ArrayList; -import java.util.List; public class ImportKeysListLoader extends AsyncTaskLoader>> { - private final Context mContext; - private final BytesLoaderState mLoaderState; + private Context mContext; + private BytesLoaderState mState; private ArrayList mData = new ArrayList<>(); - private ArrayList mParcelableRings = new ArrayList<>(); private AsyncTaskResultWrapper> mEntryListWrapper; - public ImportKeysListLoader(Context context, BytesLoaderState inputData) { + public ImportKeysListLoader(Context context, BytesLoaderState loaderState) { super(context); - this.mContext = context; - this.mLoaderState = inputData; + mContext = context; + mState = loaderState; } @Override @@ -72,13 +69,13 @@ public class ImportKeysListLoader mEntryListWrapper = new AsyncTaskResultWrapper<>(mData, getKeyResult); } - if (mLoaderState == null) { + if (mState == null) { Log.e(Constants.TAG, "Input data is null!"); return mEntryListWrapper; } try { - InputData inputData = getInputData(mLoaderState); + InputData inputData = getInputData(mState); generateListOfKeyrings(inputData); } catch (FileNotFoundException e) { OperationLog log = new OperationLog(); @@ -108,15 +105,6 @@ public class ImportKeysListLoader super.cancelLoad(); } - @Override - public void deliverResult(AsyncTaskResultWrapper> data) { - super.deliverResult(data); - } - - public List getParcelableRings() { - return mParcelableRings; - } - /** * Reads all PGPKeyRing objects from the bytes of an InputData object. */ @@ -132,10 +120,7 @@ public class ImportKeysListLoader // parse all keyrings IteratorWithIOThrow it = UncachedKeyRing.fromStream(bufferedInput); while (it.hasNext()) { - UncachedKeyRing ring = it.next(); - ImportKeysListEntry item = new ImportKeysListEntry(mContext, ring); - mData.add(item); - mParcelableRings.add(item.getParcelableKeyRing()); + mData.add(new ImportKeysListEntry(mContext, it.next())); } } catch (IOException e) { Log.e(Constants.TAG, "IOException on parsing key file! Return NoValidKeysException!", e); diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/keyimport/processing/ImportKeysListener.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/keyimport/processing/ImportKeysListener.java index bef3156a5..55b20c561 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/keyimport/processing/ImportKeysListener.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/keyimport/processing/ImportKeysListener.java @@ -1,9 +1,13 @@ package org.sufficientlysecure.keychain.keyimport.processing; +import org.sufficientlysecure.keychain.keyimport.ImportKeysListEntry; + +import java.util.List; + public interface ImportKeysListener extends ImportKeysResultListener { void loadKeys(LoaderState loaderState); - void importKeys(); + void importKeys(List entries); } diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/ImportKeysActivity.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/ImportKeysActivity.java index cd34f57f8..a2fbb3e00 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/ImportKeysActivity.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/ImportKeysActivity.java @@ -30,6 +30,7 @@ import org.sufficientlysecure.keychain.Constants; import org.sufficientlysecure.keychain.R; import org.sufficientlysecure.keychain.intents.OpenKeychainIntents; import org.sufficientlysecure.keychain.keyimport.FacebookKeyserver; +import org.sufficientlysecure.keychain.keyimport.ImportKeysListEntry; import org.sufficientlysecure.keychain.keyimport.ParcelableKeyRing; import org.sufficientlysecure.keychain.keyimport.processing.ImportKeysListener; import org.sufficientlysecure.keychain.keyimport.processing.ImportKeysOperationCallback; @@ -46,6 +47,8 @@ import org.sufficientlysecure.keychain.util.ParcelableFileCache; import org.sufficientlysecure.keychain.util.Preferences; import java.io.IOException; +import java.util.ArrayList; +import java.util.List; public class ImportKeysActivity extends BaseActivity implements ImportKeysListener { @@ -317,11 +320,11 @@ public class ImportKeysActivity extends BaseActivity implements ImportKeysListen } @Override - public void importKeys() { - FragmentManager fM = getSupportFragmentManager(); - ImportKeysListFragment listFragment = (ImportKeysListFragment) fM.findFragmentByTag(TAG_FRAG_LIST); - - Log.d(Constants.TAG, "importKeys started"); + public void importKeys(List entries) { + List keyRings = new ArrayList<>(); + for (ImportKeysListEntry e : entries) { + keyRings.add(e.getParcelableKeyRing()); + } // instead of giving the entries by Intent extra, cache them into a // file to prevent Java Binder problems on heavy imports // read FileImportCache for more info. @@ -330,7 +333,7 @@ public class ImportKeysActivity extends BaseActivity implements ImportKeysListen // display here, we should be able to import. ParcelableFileCache cache = new ParcelableFileCache<>(this, ImportOperation.CACHE_FILE_NAME); - cache.writeCache(listFragment.getData()); + cache.writeCache(entries.size(), keyRings.iterator()); } catch (IOException e) { Log.e(Constants.TAG, "Problem writing cache file", e); Notify.create(this, "Problem writing cache file!", Notify.Style.ERROR).show(); diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/ImportKeysListFragment.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/ImportKeysListFragment.java index ac9489e53..08c2116c6 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/ImportKeysListFragment.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/ImportKeysListFragment.java @@ -31,13 +31,13 @@ import android.support.v4.content.Loader; import android.support.v7.widget.LinearLayoutManager; import android.view.LayoutInflater; import android.view.View; +import android.view.View.OnClickListener; import android.view.ViewGroup; import org.sufficientlysecure.keychain.R; import org.sufficientlysecure.keychain.databinding.ImportKeysListFragmentBinding; import org.sufficientlysecure.keychain.databinding.ImportKeysListItemBasicBinding; import org.sufficientlysecure.keychain.keyimport.ImportKeysListEntry; -import org.sufficientlysecure.keychain.keyimport.ParcelableKeyRing; import org.sufficientlysecure.keychain.keyimport.processing.AsyncTaskResultWrapper; import org.sufficientlysecure.keychain.keyimport.processing.BytesLoaderState; import org.sufficientlysecure.keychain.keyimport.processing.CloudLoaderState; @@ -49,15 +49,12 @@ import org.sufficientlysecure.keychain.operations.results.GetKeyResult; import org.sufficientlysecure.keychain.service.input.RequiredInputParcel; import org.sufficientlysecure.keychain.ui.adapter.ImportKeysAdapter; import org.sufficientlysecure.keychain.ui.util.PermissionsUtil; -import org.sufficientlysecure.keychain.util.IteratorWithSize; import org.sufficientlysecure.keychain.util.ParcelableProxy; import org.sufficientlysecure.keychain.util.Preferences; import org.sufficientlysecure.keychain.util.Preferences.CloudSearchPrefs; import org.sufficientlysecure.keychain.util.orbot.OrbotHelper; import java.util.ArrayList; -import java.util.Iterator; -import java.util.List; public class ImportKeysListFragment extends Fragment implements LoaderManager.LoaderCallbacks>> { @@ -69,13 +66,12 @@ public class ImportKeysListFragment extends Fragment implements public static final String ARG_CLOUD_SEARCH_PREFS = "cloud_search_prefs"; private FragmentActivity mActivity; - private ImportKeysListener mCallback; + private ImportKeysListener mListener; private ImportKeysListFragmentBinding mBinding; private ImportKeysListItemBasicBinding mBindingBasic; private ParcelableProxy mParcelableProxy; - private List mKeyData; private ImportKeysAdapter mAdapter; private LoaderState mLoaderState; @@ -154,7 +150,7 @@ public class ImportKeysListFragment extends Fragment implements mBindingBasic.setNonInteractive(nonInteractive); // Create an empty adapter we will use to display the loaded data. - mAdapter = new ImportKeysAdapter(mActivity, mCallback, nonInteractive); + mAdapter = new ImportKeysAdapter(mActivity, mListener, nonInteractive); mBinding.recyclerView.setAdapter(mAdapter); mBinding.recyclerView.setLayoutManager(new LinearLayoutManager(mActivity)); @@ -173,13 +169,13 @@ public class ImportKeysListFragment extends Fragment implements restartLoaders(); } - mBinding.basic.importKeys.setOnClickListener(new View.OnClickListener() { + mBinding.basic.importKeys.setOnClickListener(new OnClickListener() { @Override public void onClick(View view) { - mCallback.importKeys(); + mListener.importKeys(mAdapter.getEntries()); } }); - mBinding.basic.listKeys.setOnClickListener(new View.OnClickListener() { + mBinding.basic.listKeys.setOnClickListener(new OnClickListener() { @Override public void onClick(View view) { mBinding.setAdvanced(true); @@ -194,7 +190,7 @@ public class ImportKeysListFragment extends Fragment implements super.onAttach(activity); try { - mCallback = (ImportKeysListener) activity; + mListener = (ImportKeysListener) activity; } catch (ClassCastException e) { throw new ClassCastException(activity.toString() + " must implement ImportKeysListener"); @@ -214,10 +210,6 @@ public class ImportKeysListFragment extends Fragment implements } } - public LoaderState getState() { - return mLoaderState; - } - public void loadState(LoaderState loaderState) { mLoaderState = loaderState; @@ -255,9 +247,8 @@ public class ImportKeysListFragment extends Fragment implements break; } case LOADER_ID_CLOUD: { - CloudLoaderState ls = (CloudLoaderState) mLoaderState; - loader = new ImportKeysListCloudLoader(mActivity, ls.mServerQuery, - ls.mCloudPrefs, mParcelableProxy); + loader = new ImportKeysListCloudLoader(mActivity, (CloudLoaderState) mLoaderState, + mParcelableProxy); break; } } @@ -274,30 +265,22 @@ public class ImportKeysListFragment extends Fragment implements Loader>> loader, AsyncTaskResultWrapper> data ) { - ArrayList result = data.getResult(); - mKeyData = null; - mAdapter.setData(result); + mAdapter.setData(data.getResult()); + int size = mAdapter.getItemCount(); - int size = result.size(); mBindingBasic.setNumber(size); mBinding.setStatus(size > 0 ? STATUS_LOADED : STATUS_EMPTY); GetKeyResult getKeyResult = (GetKeyResult) data.getOperationResult(); switch (loader.getId()) { case LOADER_ID_BYTES: - if (getKeyResult.success()) { - // No error - mKeyData = ((ImportKeysListLoader) loader).getParcelableRings(); - } else { + if (!getKeyResult.success()) { getKeyResult.createNotify(mActivity).show(); } break; - case LOADER_ID_CLOUD: - if (getKeyResult.success()) { - // No error - } else if (getKeyResult.isPending()) { + if (getKeyResult.isPending()) { if (getKeyResult.getRequiredInputParcel().mType == RequiredInputParcel.RequiredInputType.ENABLE_ORBOT) { if (mShowingOrbotDialog) { @@ -342,11 +325,10 @@ public class ImportKeysListFragment extends Fragment implements new Handler().post(showOrbotDialog); mShowingOrbotDialog = true; } - } else { + } else if (!getKeyResult.success()) { getKeyResult.createNotify(mActivity).show(); } break; - default: break; } @@ -359,36 +341,4 @@ public class ImportKeysListFragment extends Fragment implements mAdapter.clearData(); } - /** - * Returns an Iterator (with size) of the selected data items. - * This iterator is sort of a tradeoff, it's slightly more complex than an - * ArrayList would have been, but we save some memory by just returning - * relevant elements on demand. - */ - public IteratorWithSize getData() { - final Iterator it = mKeyData.iterator(); - return new IteratorWithSize() { - - @Override - public int getSize() { - return mKeyData.size(); - } - - @Override - public boolean hasNext() { - return it.hasNext(); - } - - @Override - public ParcelableKeyRing next() { - return it.next(); - } - - @Override - public void remove() { - it.remove(); - } - }; - } - } diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/adapter/ImportKeysAdapter.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/adapter/ImportKeysAdapter.java index 3ed25fd55..c262a313a 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/adapter/ImportKeysAdapter.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/adapter/ImportKeysAdapter.java @@ -117,7 +117,7 @@ public class ImportKeysAdapter extends RecyclerView.Adapter { mFilename = filename; } - public void writeCache(IteratorWithSize it) throws IOException { - writeCache(it.getSize(), it); - } - public void writeCache(int numEntries, Iterator it) throws IOException { DataOutputStream oos = getOutputStream();