From 500c219fa0ac511145232cb98ba643827d9627f3 Mon Sep 17 00:00:00 2001 From: Vincent Breitmoser Date: Fri, 22 Jun 2018 17:10:02 +0200 Subject: [PATCH] Extract remaining user id loading from KeychainProvider --- .../keychain/provider/KeyRepository.java | 4 +- .../keychain/provider/KeychainContract.java | 5 - .../keychain/provider/KeychainDatabase.java | 2 +- .../keychain/provider/KeychainProvider.java | 42 ----- .../keychain/ui/MultiUserIdsFragment.java | 143 ++++++------------ .../ui/keyview/loader/IdentityDao.java | 2 +- .../keychain/UserPackets.sq | 9 +- 7 files changed, 53 insertions(+), 154 deletions(-) diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/provider/KeyRepository.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/provider/KeyRepository.java index 6c278f2c3..43a92f4e0 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/provider/KeyRepository.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/provider/KeyRepository.java @@ -241,8 +241,8 @@ public class KeyRepository extends AbstractDao { } } - public List getUserIds(long masterKeyId) { - SqlDelightQuery query = UserPacket.FACTORY.selectUserIdsByMasterKeyId(masterKeyId); + public List getUserIds(long... masterKeyIds) { + SqlDelightQuery query = UserPacket.FACTORY.selectUserIdsByMasterKeyId(masterKeyIds); return mapAllRows(query, UserPacket.USER_ID_MAPPER::map); } diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/provider/KeychainContract.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/provider/KeychainContract.java index 4d3db4a13..21edee446 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/provider/KeychainContract.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/provider/KeychainContract.java @@ -229,10 +229,6 @@ public class KeychainContract { public static final Uri CONTENT_URI = BASE_CONTENT_URI_INTERNAL.buildUpon() .appendPath(BASE_KEY_RINGS).build(); - public static Uri buildUserIdsUri() { - return CONTENT_URI.buildUpon().appendPath(PATH_USER_IDS).build(); - } - public static Uri buildUserIdsUri(long masterKeyId) { return CONTENT_URI.buildUpon().appendPath(Long.toString(masterKeyId)).appendPath(PATH_USER_IDS).build(); } @@ -243,7 +239,6 @@ public class KeychainContract { public static final String NAME = UserPacketsColumns.NAME; public static final String EMAIL = UserPacketsColumns.EMAIL; public static final String COMMENT = UserPacketsColumns.COMMENT; - public static final String SIGNER_UID = "signer_user_id"; public static final int UNVERIFIED = 0; public static final int VERIFIED_SECRET = 1; diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/provider/KeychainDatabase.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/provider/KeychainDatabase.java index e8a6c7472..5e0e8e2b6 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/provider/KeychainDatabase.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/provider/KeychainDatabase.java @@ -152,7 +152,7 @@ public class KeychainDatabase { + ")"; public KeychainDatabase(Context context) { - this.context = context; + this.context = context.getApplicationContext(); supportSQLiteOpenHelper = new FrameworkSQLiteOpenHelperFactory() .create(Configuration.builder(context).name(DATABASE_NAME).callback( diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/provider/KeychainProvider.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/provider/KeychainProvider.java index d5481f5ea..48efb2781 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/provider/KeychainProvider.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/provider/KeychainProvider.java @@ -55,7 +55,6 @@ import static android.database.DatabaseUtils.dumpCursorToString; public class KeychainProvider extends ContentProvider implements SimpleContentResolverInterface { private static final int KEY_RINGS_UNIFIED = 101; - private static final int KEY_RINGS_USER_IDS = 104; private static final int KEY_RING_UNIFIED = 200; private static final int KEY_RING_KEYS = 201; @@ -92,9 +91,6 @@ public class KeychainProvider extends ContentProvider implements SimpleContentRe matcher.addURI(authority, KeychainContract.BASE_KEY_RINGS + "/" + KeychainContract.PATH_UNIFIED, KEY_RINGS_UNIFIED); - matcher.addURI(authority, KeychainContract.BASE_KEY_RINGS - + "/" + KeychainContract.PATH_USER_IDS, - KEY_RINGS_USER_IDS); /* * find by criteria other than master key id @@ -463,44 +459,6 @@ public class KeychainProvider extends ContentProvider implements SimpleContentRe break; } - case KEY_RINGS_USER_IDS: { - HashMap projectionMap = new HashMap<>(); - projectionMap.put(UserPackets._ID, Tables.USER_PACKETS + ".oid AS _id"); - projectionMap.put(UserPackets.MASTER_KEY_ID, Tables.USER_PACKETS + "." + UserPackets.MASTER_KEY_ID); - projectionMap.put(UserPackets.TYPE, Tables.USER_PACKETS + "." + UserPackets.TYPE); - projectionMap.put(UserPackets.USER_ID, Tables.USER_PACKETS + "." + UserPackets.USER_ID); - projectionMap.put(UserPackets.NAME, Tables.USER_PACKETS + "." + UserPackets.NAME); - projectionMap.put(UserPackets.EMAIL, Tables.USER_PACKETS + "." + UserPackets.EMAIL); - projectionMap.put(UserPackets.COMMENT, Tables.USER_PACKETS + "." + UserPackets.COMMENT); - projectionMap.put(UserPackets.ATTRIBUTE_DATA, Tables.USER_PACKETS + "." + UserPackets.ATTRIBUTE_DATA); - projectionMap.put(UserPackets.RANK, Tables.USER_PACKETS + "." + UserPackets.RANK); - projectionMap.put(UserPackets.IS_PRIMARY, Tables.USER_PACKETS + "." + UserPackets.IS_PRIMARY); - projectionMap.put(UserPackets.IS_REVOKED, Tables.USER_PACKETS + "." + UserPackets.IS_REVOKED); - // we take the minimum (>0) here, where "1" is "verified by known secret key" - projectionMap.put(UserPackets.VERIFIED, "MIN(" + Certs.VERIFIED + ") AS " + UserPackets.VERIFIED); - qb.setProjectionMap(projectionMap); - - qb.setTables(Tables.USER_PACKETS - + " LEFT JOIN " + Tables.CERTS + " ON (" - + Tables.USER_PACKETS + "." + UserPackets.MASTER_KEY_ID + " = " - + Tables.CERTS + "." + Certs.MASTER_KEY_ID - + " AND " + Tables.USER_PACKETS + "." + UserPackets.RANK + " = " - + Tables.CERTS + "." + Certs.RANK - + " AND " + Tables.CERTS + "." + Certs.VERIFIED + " > 0" - + ")"); - groupBy = Tables.USER_PACKETS + "." + UserPackets.MASTER_KEY_ID - + ", " + Tables.USER_PACKETS + "." + UserPackets.RANK; - - qb.appendWhere(Tables.USER_PACKETS + "." + UserPackets.TYPE + " IS NULL"); - - if (TextUtils.isEmpty(sortOrder)) { - sortOrder = Tables.USER_PACKETS + "." + UserPackets.MASTER_KEY_ID + " ASC" - + "," + Tables.USER_PACKETS + "." + UserPackets.RANK + " ASC"; - } - - break; - } - default: { throw new IllegalArgumentException("Unknown URI " + uri + " (" + match + ")"); } diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/MultiUserIdsFragment.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/MultiUserIdsFragment.java index dc4a4de3f..6cabeb2a8 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/MultiUserIdsFragment.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/MultiUserIdsFragment.java @@ -17,67 +17,47 @@ package org.sufficientlysecure.keychain.ui; -import android.database.Cursor; + +import java.util.ArrayList; +import java.util.List; + +import android.arch.lifecycle.LiveData; import android.database.MatrixCursor; -import android.net.Uri; import android.os.Bundle; import android.os.Parcel; +import android.support.annotation.NonNull; import android.support.annotation.Nullable; import android.support.v4.app.Fragment; -import android.support.v4.app.LoaderManager; -import android.support.v4.content.CursorLoader; -import android.support.v4.content.Loader; +import android.support.v4.app.FragmentActivity; import android.view.LayoutInflater; import android.view.View; import android.view.ViewGroup; import android.widget.ListView; import org.sufficientlysecure.keychain.R; -import org.sufficientlysecure.keychain.provider.KeychainContract; -import org.sufficientlysecure.keychain.provider.KeychainDatabase; +import org.sufficientlysecure.keychain.livedata.GenericLiveData; +import org.sufficientlysecure.keychain.model.UserPacket.UserId; +import org.sufficientlysecure.keychain.provider.KeyRepository; import org.sufficientlysecure.keychain.service.CertifyActionsParcel; import org.sufficientlysecure.keychain.ui.adapter.MultiUserIdsAdapter; import timber.log.Timber; -import java.util.ArrayList; -public class MultiUserIdsFragment extends Fragment implements LoaderManager.LoaderCallbacks{ +public class MultiUserIdsFragment extends Fragment { public static final String ARG_CHECK_STATES = "check_states"; public static final String EXTRA_KEY_IDS = "extra_key_ids"; private boolean checkboxVisibility = true; - ListView mUserIds; - private MultiUserIdsAdapter mUserIdsAdapter; + ListView userIds; + private MultiUserIdsAdapter userIdsAdapter; - private long[] mPubMasterKeyIds; - - public static final String[] USER_IDS_PROJECTION = new String[]{ - KeychainContract.UserPackets._ID, - KeychainContract.UserPackets.MASTER_KEY_ID, - KeychainContract.UserPackets.USER_ID, - KeychainContract.UserPackets.IS_PRIMARY, - KeychainContract.UserPackets.IS_REVOKED, - KeychainContract.UserPackets.NAME, - KeychainContract.UserPackets.EMAIL, - KeychainContract.UserPackets.COMMENT, - }; - private static final int INDEX_MASTER_KEY_ID = 1; - private static final int INDEX_USER_ID = 2; - @SuppressWarnings("unused") - private static final int INDEX_IS_PRIMARY = 3; - @SuppressWarnings("unused") - private static final int INDEX_IS_REVOKED = 4; - private static final int INDEX_NAME = 5; - private static final int INDEX_EMAIL = 6; - private static final int INDEX_COMMENT = 7; + private long[] pubMasterKeyIds; @Nullable @Override - public View onCreateView(LayoutInflater inflater, ViewGroup container, Bundle savedInstanceState) { - View view = inflater.inflate(R.layout.multi_user_ids_fragment, null); - - mUserIds = view.findViewById(R.id.view_key_user_ids); - + public View onCreateView(@NonNull LayoutInflater inflater, ViewGroup container, Bundle savedInstanceState) { + View view = inflater.inflate(R.layout.multi_user_ids_fragment, container, false); + userIds = view.findViewById(R.id.view_key_user_ids); return view; } @@ -85,10 +65,11 @@ public class MultiUserIdsFragment extends Fragment implements LoaderManager.Load public void onActivityCreated(@Nullable Bundle savedInstanceState) { super.onActivityCreated(savedInstanceState); - mPubMasterKeyIds = getActivity().getIntent().getLongArrayExtra(EXTRA_KEY_IDS); - if (mPubMasterKeyIds == null) { + FragmentActivity activity = requireActivity(); + pubMasterKeyIds = activity.getIntent().getLongArrayExtra(EXTRA_KEY_IDS); + if (pubMasterKeyIds == null) { Timber.e("List of key ids to certify missing!"); - getActivity().finish(); + activity.finish(); return; } @@ -97,18 +78,22 @@ public class MultiUserIdsFragment extends Fragment implements LoaderManager.Load checkedStates = (ArrayList) savedInstanceState.getSerializable(ARG_CHECK_STATES); } - mUserIdsAdapter = new MultiUserIdsAdapter(getActivity(), null, 0, checkedStates, checkboxVisibility); - mUserIds.setAdapter(mUserIdsAdapter); - mUserIds.setDividerHeight(0); + userIdsAdapter = new MultiUserIdsAdapter(activity, null, 0, checkedStates, checkboxVisibility); + userIds.setAdapter(userIdsAdapter); + userIds.setDividerHeight(0); + + KeyRepository keyRepository = KeyRepository.create(activity); + LiveData> userIdLiveData = + new GenericLiveData<>(getContext(), null, () -> keyRepository.getUserIds(pubMasterKeyIds)); + userIdLiveData.observe(this, this::onUserIdsLoaded); - getLoaderManager().initLoader(0, null, this); } @Override - public void onSaveInstanceState(Bundle outState) { + public void onSaveInstanceState(@NonNull Bundle outState) { super.onSaveInstanceState(outState); - ArrayList states = mUserIdsAdapter.getCheckStates(); + ArrayList states = userIdsAdapter.getCheckStates(); // no proper parceling method available :( outState.putSerializable(ARG_CHECK_STATES, states); } @@ -118,40 +103,10 @@ public class MultiUserIdsFragment extends Fragment implements LoaderManager.Load throw new AssertionError("Item selection not allowed"); } - return mUserIdsAdapter.getSelectedCertifyActions(); + return userIdsAdapter.getSelectedCertifyActions(); } - @Override - public Loader onCreateLoader(int id, Bundle args) { - Uri uri = KeychainContract.UserPackets.buildUserIdsUri(); - - String selection, ids[]; - { - // generate placeholders and string selection args - ids = new String[mPubMasterKeyIds.length]; - StringBuilder placeholders = new StringBuilder("?"); - for (int i = 0; i < mPubMasterKeyIds.length; i++) { - ids[i] = Long.toString(mPubMasterKeyIds[i]); - if (i != 0) { - placeholders.append(",?"); - } - } - // put together selection string - selection = KeychainContract.UserPackets.IS_REVOKED + " = 0" + " AND " - + KeychainDatabase.Tables.USER_PACKETS + "." + KeychainContract.UserPackets.MASTER_KEY_ID - + " IN (" + placeholders + ")"; - } - - return new CursorLoader(getActivity(), uri, - USER_IDS_PROJECTION, selection, ids, - KeychainDatabase.Tables.USER_PACKETS + "." + KeychainContract.UserPackets.MASTER_KEY_ID + " ASC" - + ", " + KeychainDatabase.Tables.USER_PACKETS + "." + KeychainContract.UserPackets.USER_ID + " ASC" - ); - } - - @Override - public void onLoadFinished(Loader loader, Cursor data) { - + private void onUserIdsLoaded(List userIds) { MatrixCursor matrix = new MatrixCursor(new String[]{ "_id", "user_data", "grouped" }) { @@ -160,28 +115,25 @@ public class MultiUserIdsFragment extends Fragment implements LoaderManager.Load return super.getBlob(column); } }; - data.moveToFirst(); long lastMasterKeyId = 0; String lastName = ""; ArrayList uids = new ArrayList<>(); boolean header = true; + boolean isFirst = true; // Iterate over all rows - while (!data.isAfterLast()) { - long masterKeyId = data.getLong(INDEX_MASTER_KEY_ID); - String userId = data.getString(INDEX_USER_ID); - String name = data.getString(INDEX_NAME); - + for (UserId userId : userIds) { // Two cases: - boolean grouped = masterKeyId == lastMasterKeyId; - boolean subGrouped = data.isFirst() || grouped && lastName != null && lastName.equals(name); + boolean grouped = userId.master_key_id() == lastMasterKeyId; + boolean subGrouped = isFirst || grouped && lastName != null && lastName.equals(userId.name()); + isFirst = false; // Remember for next loop - lastName = name; + lastName = userId.name(); - Timber.d(Long.toString(masterKeyId, 16) + (grouped ? "grouped" : "not grouped")); + Timber.d(Long.toString(userId.master_key_id(), 16) + (grouped ? "grouped" : "not grouped")); if (!subGrouped) { // 1. This name should NOT be grouped with the previous, so we flush the buffer @@ -203,17 +155,13 @@ public class MultiUserIdsFragment extends Fragment implements LoaderManager.Load } // 2. This name should be grouped with the previous, just add to buffer - uids.add(userId); - lastMasterKeyId = masterKeyId; + uids.add(userId.user_id()); + lastMasterKeyId = userId.master_key_id(); // If this one wasn't grouped, the next one's gotta be a header if (!grouped) { header = true; } - - // Regardless of the outcome, move to next entry - data.moveToNext(); - } // If there is anything left in the buffer, flush it one last time @@ -230,12 +178,7 @@ public class MultiUserIdsFragment extends Fragment implements LoaderManager.Load } - mUserIdsAdapter.swapCursor(matrix); - } - - @Override - public void onLoaderReset(Loader loader) { - mUserIdsAdapter.swapCursor(null); + userIdsAdapter.swapCursor(matrix); } public void setCheckboxVisibility(boolean checkboxVisibility) { diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/keyview/loader/IdentityDao.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/keyview/loader/IdentityDao.java index f3a39cccf..0d7c06596 100644 --- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/keyview/loader/IdentityDao.java +++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/keyview/loader/IdentityDao.java @@ -172,7 +172,7 @@ public class IdentityDao { return null; } - private void loadUserIds(ArrayList identities, long masterKeyId) { + private void loadUserIds(ArrayList identities, long... masterKeyId) { SqlDelightQuery query = UserPacket.FACTORY.selectUserIdsByMasterKeyId(masterKeyId); try (Cursor cursor = db.query(query)) { while (cursor.moveToNext()) { diff --git a/OpenKeychain/src/main/sqldelight/org/sufficientlysecure/keychain/UserPackets.sq b/OpenKeychain/src/main/sqldelight/org/sufficientlysecure/keychain/UserPackets.sq index 42bca407d..85f8551e0 100644 --- a/OpenKeychain/src/main/sqldelight/org/sufficientlysecure/keychain/UserPackets.sq +++ b/OpenKeychain/src/main/sqldelight/org/sufficientlysecure/keychain/UserPackets.sq @@ -20,19 +20,22 @@ selectUserIdsByMasterKeyId: SELECT user_packets.master_key_id, user_packets.rank, user_id, name, email, comment, is_primary, is_revoked, certs.verified FROM user_packets LEFT JOIN certs ON ( user_packets.master_key_id = certs.master_key_id AND user_packets.rank = certs.rank AND certs.verified > 0 ) - WHERE user_packets.type IS NULL AND user_packets.is_revoked = 0 AND user_packets.master_key_id = ?; + WHERE user_packets.type IS NULL AND user_packets.is_revoked = 0 AND user_packets.master_key_id IN ? + ORDER BY user_packets.master_key_id ASC,user_packets.rank ASC; selectUserIdsByMasterKeyIdAndVerification: SELECT user_packets.master_key_id, user_packets.rank, user_id, name, email, comment, is_primary, is_revoked, certs.verified FROM user_packets LEFT JOIN certs ON ( user_packets.master_key_id = certs.master_key_id AND user_packets.rank = certs.rank AND certs.verified > 0 ) - WHERE user_packets.type IS NULL AND user_packets.is_revoked = 0 AND user_packets.master_key_id = ? AND certs.verified = ?; + WHERE user_packets.type IS NULL AND user_packets.is_revoked = 0 AND user_packets.master_key_id = ? AND certs.verified = ? + ORDER BY user_packets.rank ASC; selectUserAttributesByTypeAndMasterKeyId: SELECT user_packets.master_key_id, user_packets.rank, attribute_data, is_primary, is_revoked, certs.verified FROM user_packets LEFT JOIN certs ON ( user_packets.master_key_id = certs.master_key_id AND user_packets.rank = certs.rank AND certs.verified > 0 ) - WHERE user_packets.type = ? AND user_packets.is_revoked = 0 AND user_packets.master_key_id = ?; + WHERE user_packets.type = ? AND user_packets.is_revoked = 0 AND user_packets.master_key_id = ? + ORDER BY user_packets.rank ASC; selectSpecificUserAttribute: SELECT user_packets.master_key_id, user_packets.rank, attribute_data, is_primary, is_revoked, certs.verified