simplify progress for key import operations

This commit is contained in:
Vincent Breitmoser
2017-05-19 16:59:18 +02:00
parent f34edae294
commit c91b7e377a
13 changed files with 74 additions and 92 deletions

View File

@@ -42,7 +42,6 @@ import org.sufficientlysecure.keychain.provider.KeyWritableRepository;
import org.sufficientlysecure.keychain.provider.KeychainDatabase;
import org.sufficientlysecure.keychain.service.PassphraseCacheService;
import org.sufficientlysecure.keychain.ui.util.Notify.Style;
import org.sufficientlysecure.keychain.util.ProgressScaler;
import static android.support.test.InstrumentationRegistry.getInstrumentation;
import static android.support.test.espresso.Espresso.onView;
@@ -101,7 +100,7 @@ public class TestHelpers {
while(stream.hasNext()) {
UncachedKeyRing ring = stream.next();
if (ring.isSecret()) {
helper.saveSecretKeyRing(ring, new ProgressScaler());
helper.saveSecretKeyRing(ring);
} else {
helper.savePublicKeyRing(ring);
}

View File

@@ -90,8 +90,8 @@ public class ChangeUnlockOperation extends BaseReadWriteOperation<ChangeUnlockPa
// It's a success, so this must be non-null now
UncachedKeyRing ring = modifyResult.getRing();
SaveKeyringResult saveResult = mKeyWritableRepository
.saveSecretKeyRing(ring, new ProgressScaler(mProgressable, 70, 95, 100));
updateProgress(R.string.progress_saving, 80, 100);
SaveKeyringResult saveResult = mKeyWritableRepository.saveSecretKeyRing(ring);
log.add(saveResult, 1);
// If the save operation didn't succeed, exit here

View File

@@ -146,8 +146,8 @@ public class EditKeyOperation extends BaseReadWriteOperation<SaveKeyringParcel>
UploadKeyringParcel exportKeyringParcel =
new UploadKeyringParcel(saveParcel.getUploadKeyserver(), keyringBytes);
UploadResult uploadResult =
new UploadOperation(mContext, mKeyRepository, mProgressable, mCancelled)
UploadResult uploadResult = new UploadOperation(
mContext, mKeyRepository, new ProgressScaler(mProgressable, 60, 80, 100), mCancelled)
.execute(exportKeyringParcel, cryptoInput);
log.add(uploadResult, 2);
@@ -161,8 +161,8 @@ public class EditKeyOperation extends BaseReadWriteOperation<SaveKeyringParcel>
}
// Save the new keyring.
SaveKeyringResult saveResult = mKeyWritableRepository
.saveSecretKeyRing(ring, new ProgressScaler(mProgressable, 60, 95, 100));
updateProgress(R.string.progress_saving, 90, 100);
SaveKeyringResult saveResult = mKeyWritableRepository.saveSecretKeyRing(ring);
log.add(saveResult, 1);
// If the save operation didn't succeed, exit here

View File

@@ -129,13 +129,13 @@ public class ImportOperation extends BaseReadWriteOperation<ImportKeyringParcel>
* contact-to-key sync i.e ContactSyncAdapterService.requestContactsSync()
*
* @param entries keys to import
* @param num number of keys to import
* @param numTotalKeys number of keys to import
* @param hkpKeyserver contains uri of keyserver to import from, if it is an import from cloud
* @param progressable Allows multi-threaded import to supply a progressable that ignores the
* progress of a single key being imported
*/
@NonNull
private ImportKeyResult serialKeyRingImport(Iterator<ParcelableKeyRing> entries, int num,
private ImportKeyResult serialKeyRingImport(Iterator<ParcelableKeyRing> entries, int numTotalKeys,
ParcelableHkpKeyserver hkpKeyserver, Progressable progressable,
@NonNull ParcelableProxy proxy, boolean skipSave) {
if (progressable != null) {
@@ -143,7 +143,7 @@ public class ImportOperation extends BaseReadWriteOperation<ImportKeyringParcel>
}
OperationLog log = new OperationLog();
log.add(LogType.MSG_IMPORT, 0, num);
log.add(LogType.MSG_IMPORT, 0, numTotalKeys);
// If there aren't even any keys, do nothing here.
if (entries == null || !entries.hasNext()) {
@@ -156,8 +156,7 @@ public class ImportOperation extends BaseReadWriteOperation<ImportKeyringParcel>
ArrayList<CanonicalizedKeyRing> canKeyRings = new ArrayList<>();
boolean cancelled = false;
int position = 0;
double progSteps = 100.0 / num;
int keyImportsFinished = 0;
KeybaseKeyserver keybaseServer = null;
FacebookKeyserver facebookServer = null;
@@ -316,14 +315,10 @@ public class ImportOperation extends BaseReadWriteOperation<ImportKeyringParcel>
// and https://github.com/open-keychain/open-keychain/issues/1480
synchronized (mKeyRepository) {
mKeyRepository.clearLog();
ProgressScaler progressScaler = new ProgressScaler(progressable, (int) (position * progSteps),
(int) ((position + 1) * progSteps), 100);
if (key.isSecret()) {
result = mKeyWritableRepository.saveSecretKeyRing(key, progressScaler,
canKeyRings, skipSave);
result = mKeyWritableRepository.saveSecretKeyRing(key, canKeyRings, skipSave);
} else {
result = mKeyWritableRepository.savePublicKeyRing(key, progressScaler,
entry.mExpectedFingerprint, canKeyRings, skipSave);
result = mKeyWritableRepository.savePublicKeyRing(key, entry.mExpectedFingerprint, canKeyRings, skipSave);
}
}
if (!result.success()) {
@@ -354,8 +349,10 @@ public class ImportOperation extends BaseReadWriteOperation<ImportKeyringParcel>
Log.e(Constants.TAG, "Encountered bad key on import!", e);
++badKeys;
}
// update progress
position++;
keyImportsFinished += 1;
progressable.setProgress(keyImportsFinished, numTotalKeys);
}
// Special: consolidate on secret key import (cannot be cancelled!)

View File

@@ -113,8 +113,8 @@ public class PromoteKeyOperation extends BaseReadWriteOperation<PromoteKeyringPa
setPreventCancel();
// Save the new keyring.
SaveKeyringResult saveResult = mKeyWritableRepository
.saveSecretKeyRing(promotedRing, new ProgressScaler(mProgressable, 60, 95, 100));
updateProgress(R.string.progress_saving, 80, 100);
SaveKeyringResult saveResult = mKeyWritableRepository.saveSecretKeyRing(promotedRing);
log.add(saveResult, 1);
// If the save operation didn't succeed, exit here

View File

@@ -75,7 +75,6 @@ import org.sufficientlysecure.keychain.util.Log;
import org.sufficientlysecure.keychain.util.ParcelableFileCache;
import org.sufficientlysecure.keychain.util.Preferences;
import org.sufficientlysecure.keychain.util.ProgressFixedScaler;
import org.sufficientlysecure.keychain.util.ProgressScaler;
import org.sufficientlysecure.keychain.util.Utf8Util;
/**
@@ -179,8 +178,7 @@ public class KeyWritableRepository extends KeyRepository {
* and need to be saved externally to be preserved past the operation.
*/
@SuppressWarnings("unchecked")
private int saveCanonicalizedPublicKeyRing(CanonicalizedPublicKeyRing keyRing,
Progressable progress, boolean selfCertsAreTrusted) {
private int saveCanonicalizedPublicKeyRing(CanonicalizedPublicKeyRing keyRing, boolean selfCertsAreTrusted) {
// start with ok result
int result = SaveKeyringResult.SAVED_PUBLIC;
@@ -206,7 +204,6 @@ public class KeyWritableRepository extends KeyRepository {
}
log(LogType.MSG_IP_INSERT_SUBKEYS);
progress.setProgress(LogType.MSG_IP_INSERT_SUBKEYS.getMsgId(), 40, 100);
mIndent += 1;
{ // insert subkeys
Uri uri = Keys.buildKeysUri(masterKeyId);
@@ -475,7 +472,6 @@ public class KeyWritableRepository extends KeyRepository {
}
mIndent -= 1;
progress.setProgress(LogType.MSG_IP_UID_REORDER.getMsgId(), 65, 100);
log(LogType.MSG_IP_UID_REORDER);
// primary before regular before revoked (see UserIdItem.compareTo)
// this is a stable sort, so the order of keys is otherwise preserved.
@@ -563,11 +559,9 @@ public class KeyWritableRepository extends KeyRepository {
}
log(LogType.MSG_IP_APPLY_BATCH);
progress.setProgress(LogType.MSG_IP_APPLY_BATCH.getMsgId(), 75, 100);
mContentResolver.applyBatch(KeychainContract.CONTENT_AUTHORITY, operations);
log(LogType.MSG_IP_SUCCESS);
progress.setProgress(LogType.MSG_IP_SUCCESS.getMsgId(), 90, 100);
return result;
} catch (RemoteException e) {
@@ -762,7 +756,7 @@ public class KeyWritableRepository extends KeyRepository {
* <p>
* If you want to merge keys in-memory only and not save in database set skipSave=true.
*/
public SaveKeyringResult savePublicKeyRing(UncachedKeyRing publicRing, Progressable progress,
public SaveKeyringResult savePublicKeyRing(UncachedKeyRing publicRing,
String expectedFingerprint,
ArrayList<CanonicalizedKeyRing> canKeyRings,
boolean skipSave) {
@@ -858,13 +852,11 @@ public class KeyWritableRepository extends KeyRepository {
result = SaveKeyringResult.SAVED_PUBLIC
| (alreadyExists ? SaveKeyringResult.UPDATED : 0);
} else {
result = saveCanonicalizedPublicKeyRing(canPublicRing, progress, canSecretRing != null);
result = saveCanonicalizedPublicKeyRing(canPublicRing, canSecretRing != null);
}
// Save the saved keyring (if any)
if (canSecretRing != null) {
progress.setProgress(LogType.MSG_IP_REINSERT_SECRET.getMsgId(), 90, 100);
int secretResult;
if (skipSave) {
// skip save method, set fixed result
@@ -887,16 +879,15 @@ public class KeyWritableRepository extends KeyRepository {
}
}
public SaveKeyringResult savePublicKeyRing(UncachedKeyRing publicRing, Progressable progress,
String expectedFingerprint) {
return savePublicKeyRing(publicRing, progress, expectedFingerprint, null, false);
public SaveKeyringResult savePublicKeyRing(UncachedKeyRing publicRing, String expectedFingerprint) {
return savePublicKeyRing(publicRing, expectedFingerprint, null, false);
}
public SaveKeyringResult savePublicKeyRing(UncachedKeyRing keyRing) {
return savePublicKeyRing(keyRing, new ProgressScaler(), null);
return savePublicKeyRing(keyRing, null);
}
public SaveKeyringResult saveSecretKeyRing(UncachedKeyRing secretRing, Progressable progress,
public SaveKeyringResult saveSecretKeyRing(UncachedKeyRing secretRing,
ArrayList<CanonicalizedKeyRing> canKeyRings,
boolean skipSave) {
@@ -996,15 +987,13 @@ public class KeyWritableRepository extends KeyRepository {
// skip save method, set fixed result
publicResult = SaveKeyringResult.SAVED_PUBLIC;
} else {
publicResult = saveCanonicalizedPublicKeyRing(canPublicRing, progress, true);
publicResult = saveCanonicalizedPublicKeyRing(canPublicRing, true);
}
if ((publicResult & SaveKeyringResult.RESULT_ERROR) == SaveKeyringResult.RESULT_ERROR) {
return new SaveKeyringResult(SaveKeyringResult.RESULT_ERROR, mLog, null);
}
progress.setProgress(LogType.MSG_IP_REINSERT_SECRET.getMsgId(), 90, 100);
int result;
if (skipSave) {
// skip save method, set fixed result
@@ -1023,8 +1012,8 @@ public class KeyWritableRepository extends KeyRepository {
}
}
public SaveKeyringResult saveSecretKeyRing(UncachedKeyRing secretRing, Progressable progress) {
return saveSecretKeyRing(secretRing, progress, null, false);
public SaveKeyringResult saveSecretKeyRing(UncachedKeyRing secretRing) {
return saveSecretKeyRing(secretRing, null, false);
}
@NonNull

View File

@@ -18,6 +18,14 @@
package org.sufficientlysecure.keychain.operations;
import java.io.ByteArrayInputStream;
import java.io.ByteArrayOutputStream;
import java.io.PipedInputStream;
import java.io.PipedOutputStream;
import java.io.PrintStream;
import java.security.Security;
import java.util.Iterator;
import android.app.Application;
import android.content.ContentResolver;
import android.content.ContentValues;
@@ -53,17 +61,8 @@ import org.sufficientlysecure.keychain.service.SaveKeyringParcel.Algorithm;
import org.sufficientlysecure.keychain.service.input.CryptoInputParcel;
import org.sufficientlysecure.keychain.ui.util.KeyFormattingUtils;
import org.sufficientlysecure.keychain.util.Passphrase;
import org.sufficientlysecure.keychain.util.ProgressScaler;
import org.sufficientlysecure.keychain.util.TestingUtils;
import java.io.ByteArrayInputStream;
import java.io.ByteArrayOutputStream;
import java.io.PipedInputStream;
import java.io.PipedOutputStream;
import java.io.PrintStream;
import java.security.Security;
import java.util.Iterator;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
@@ -141,8 +140,8 @@ public class BackupOperationTest {
// don't log verbosely here, we're not here to test imports
ShadowLog.stream = oldShadowStream;
databaseInteractor.saveSecretKeyRing(mStaticRing1, new ProgressScaler());
databaseInteractor.saveSecretKeyRing(mStaticRing2, new ProgressScaler());
databaseInteractor.saveSecretKeyRing(mStaticRing1);
databaseInteractor.saveSecretKeyRing(mStaticRing2);
// ok NOW log verbosely!
ShadowLog.stream = System.out;

View File

@@ -17,6 +17,13 @@
package org.sufficientlysecure.keychain.operations;
import java.io.PrintStream;
import java.security.Security;
import java.util.ArrayList;
import java.util.Date;
import java.util.Random;
import org.bouncycastle.bcpg.sig.KeyFlags;
import org.bouncycastle.jce.provider.BouncyCastleProvider;
import org.junit.Assert;
@@ -43,15 +50,8 @@ import org.sufficientlysecure.keychain.service.SaveKeyringParcel;
import org.sufficientlysecure.keychain.service.SaveKeyringParcel.Algorithm;
import org.sufficientlysecure.keychain.service.input.CryptoInputParcel;
import org.sufficientlysecure.keychain.util.Passphrase;
import org.sufficientlysecure.keychain.util.ProgressScaler;
import org.sufficientlysecure.keychain.util.TestingUtils;
import java.io.PrintStream;
import java.security.Security;
import java.util.ArrayList;
import java.util.Date;
import java.util.Random;
@RunWith(KeychainTestRunner.class)
public class CertifyOperationTest {
@@ -123,8 +123,8 @@ public class CertifyOperationTest {
// don't log verbosely here, we're not here to test imports
ShadowLog.stream = oldShadowStream;
databaseInteractor.saveSecretKeyRing(mStaticRing1, new ProgressScaler());
databaseInteractor.savePublicKeyRing(mStaticRing2.extractPublicKeyRing(), new ProgressScaler(), null);
databaseInteractor.saveSecretKeyRing(mStaticRing1);
databaseInteractor.savePublicKeyRing(mStaticRing2.extractPublicKeyRing(), null);
// ok NOW log verbosely!
ShadowLog.stream = System.out;

View File

@@ -18,6 +18,10 @@
package org.sufficientlysecure.keychain.operations;
import java.io.PrintStream;
import java.security.Security;
import java.util.Iterator;
import org.bouncycastle.bcpg.sig.KeyFlags;
import org.bouncycastle.jce.provider.BouncyCastleProvider;
import org.bouncycastle.util.encoders.Hex;
@@ -45,13 +49,8 @@ import org.sufficientlysecure.keychain.service.SaveKeyringParcel;
import org.sufficientlysecure.keychain.service.SaveKeyringParcel.Algorithm;
import org.sufficientlysecure.keychain.support.KeyringTestingHelper;
import org.sufficientlysecure.keychain.util.Passphrase;
import org.sufficientlysecure.keychain.util.ProgressScaler;
import org.sufficientlysecure.keychain.util.TestingUtils;
import java.io.PrintStream;
import java.security.Security;
import java.util.Iterator;
@RunWith(KeychainTestRunner.class)
public class PromoteKeyOperationTest {
@@ -96,7 +95,7 @@ public class PromoteKeyOperationTest {
// don't log verbosely here, we're not here to test imports
ShadowLog.stream = oldShadowStream;
databaseInteractor.savePublicKeyRing(mStaticRing.extractPublicKeyRing(), new ProgressScaler(), null);
databaseInteractor.savePublicKeyRing(mStaticRing.extractPublicKeyRing(), null);
// ok NOW log verbosely!
ShadowLog.stream = System.out;

View File

@@ -64,7 +64,6 @@ import org.sufficientlysecure.keychain.support.KeyringTestingHelper;
import org.sufficientlysecure.keychain.support.KeyringTestingHelper.RawPacket;
import org.sufficientlysecure.keychain.util.InputData;
import org.sufficientlysecure.keychain.util.Passphrase;
import org.sufficientlysecure.keychain.util.ProgressScaler;
import org.sufficientlysecure.keychain.util.TestingUtils;
import static org.hamcrest.core.AnyOf.anyOf;
@@ -156,9 +155,9 @@ public class PgpEncryptDecryptTest {
// don't log verbosely here, we're not here to test imports
ShadowLog.stream = oldShadowStream;
databaseInteractor.saveSecretKeyRing(mStaticRing1, new ProgressScaler());
databaseInteractor.saveSecretKeyRing(mStaticRing2, new ProgressScaler());
databaseInteractor.saveSecretKeyRing(mStaticRingInsecure, new ProgressScaler());
databaseInteractor.saveSecretKeyRing(mStaticRing1);
databaseInteractor.saveSecretKeyRing(mStaticRing2);
databaseInteractor.saveSecretKeyRing(mStaticRingInsecure);
// ok NOW log verbosely!
ShadowLog.stream = System.out;
@@ -631,7 +630,7 @@ public class PgpEncryptDecryptTest {
KeyWritableRepository databaseInteractor =
KeyWritableRepository.createDatabaseReadWriteInteractor(RuntimeEnvironment.application);
databaseInteractor.saveSecretKeyRing(modified, new ProgressScaler());
databaseInteractor.saveSecretKeyRing(modified);
PgpDecryptVerifyOperation op = new PgpDecryptVerifyOperation(RuntimeEnvironment.application,
KeyWritableRepository.createDatabaseReadWriteInteractor(RuntimeEnvironment.application), null);
@@ -654,7 +653,7 @@ public class PgpEncryptDecryptTest {
KeyWritableRepository databaseInteractor =
KeyWritableRepository.createDatabaseReadWriteInteractor(RuntimeEnvironment.application);
databaseInteractor.saveSecretKeyRing(modified, new ProgressScaler());
databaseInteractor.saveSecretKeyRing(modified);
PgpDecryptVerifyOperation op = new PgpDecryptVerifyOperation(RuntimeEnvironment.application,
KeyWritableRepository.createDatabaseReadWriteInteractor(RuntimeEnvironment.application), null);
@@ -682,7 +681,7 @@ public class PgpEncryptDecryptTest {
KeyWritableRepository databaseInteractor =
KeyWritableRepository.createDatabaseReadWriteInteractor(RuntimeEnvironment.application);
databaseInteractor.saveSecretKeyRing(modified, new ProgressScaler());
databaseInteractor.saveSecretKeyRing(modified);
}
{ // encrypt to this keyring, make sure it's not encrypted to the revoked subkey

View File

@@ -92,14 +92,14 @@ public class KeyRepositorySaveTest {
// insert secret, this should fail because of missing self-cert
result = KeyWritableRepository.createDatabaseReadWriteInteractor(RuntimeEnvironment.application)
.saveSecretKeyRing(seckey, new ProgressScaler());
.saveSecretKeyRing(seckey);
Assert.assertFalse("secret keyring import before pubring import should fail", result.success());
// insert pubkey, then seckey - both should succeed
result = KeyWritableRepository.createDatabaseReadWriteInteractor(RuntimeEnvironment.application).savePublicKeyRing(pubkey);
Assert.assertTrue("public keyring import should succeed", result.success());
result = KeyWritableRepository.createDatabaseReadWriteInteractor(RuntimeEnvironment.application)
.saveSecretKeyRing(seckey, new ProgressScaler());
.saveSecretKeyRing(seckey);
Assert.assertTrue("secret keyring import after pubring import should succeed", result.success());
}
@@ -134,7 +134,7 @@ public class KeyRepositorySaveTest {
SaveKeyringResult result;
result = mDatabaseInteractor.saveSecretKeyRing(sec, new ProgressScaler());
result = mDatabaseInteractor.saveSecretKeyRing(sec);
Assert.assertTrue("import of secret keyring should succeed", result.success());
// make sure both the CanonicalizedSecretKeyRing as well as the CachedPublicKeyRing correctly
@@ -198,7 +198,7 @@ public class KeyRepositorySaveTest {
SaveKeyringResult result;
result = mDatabaseInteractor.savePublicKeyRing(key, new ProgressScaler(), null);
result = mDatabaseInteractor.savePublicKeyRing(key, null);
Assert.assertTrue("import of keyring should succeed", result.success());
CanonicalizedPublicKeyRing ring = mDatabaseInteractor.getCanonicalizedPublicKeyRing(keyId);
@@ -225,7 +225,7 @@ public class KeyRepositorySaveTest {
SaveKeyringResult result;
result = mDatabaseInteractor.saveSecretKeyRing(key, new ProgressScaler());
result = mDatabaseInteractor.saveSecretKeyRing(key);
Assert.assertTrue("import of keyring should succeed", result.success());
long signId;

View File

@@ -211,7 +211,7 @@ public class KeychainExternalProviderTest {
private void insertSecretKeyringFrom(String filename) throws Exception {
UncachedKeyRing ring = readRingFromResource(filename);
SaveKeyringResult saveKeyringResult = databaseInteractor.saveSecretKeyRing(ring, new ProgressScaler());
SaveKeyringResult saveKeyringResult = databaseInteractor.saveSecretKeyRing(ring);
assertTrue(saveKeyringResult.success());
}

View File

@@ -18,15 +18,6 @@
package org.sufficientlysecure.keychain.support;
import android.content.Context;
import org.bouncycastle.util.Arrays;
import org.sufficientlysecure.keychain.pgp.UncachedKeyRing;
import org.sufficientlysecure.keychain.pgp.exception.PgpGeneralException;
import org.sufficientlysecure.keychain.provider.KeyRepository;
import org.sufficientlysecure.keychain.operations.results.SaveKeyringResult;
import org.sufficientlysecure.keychain.provider.KeyWritableRepository;
import org.sufficientlysecure.keychain.util.ProgressScaler;
import java.io.ByteArrayInputStream;
import java.io.ByteArrayOutputStream;
@@ -40,6 +31,15 @@ import java.util.HashSet;
import java.util.Iterator;
import java.util.List;
import android.content.Context;
import org.bouncycastle.util.Arrays;
import org.sufficientlysecure.keychain.operations.results.SaveKeyringResult;
import org.sufficientlysecure.keychain.pgp.UncachedKeyRing;
import org.sufficientlysecure.keychain.pgp.exception.PgpGeneralException;
import org.sufficientlysecure.keychain.provider.KeyRepository;
import org.sufficientlysecure.keychain.provider.KeyWritableRepository;
/** Helper methods for keyring tests. */
public class KeyringTestingHelper {
@@ -61,7 +61,7 @@ public class KeyringTestingHelper {
// Should throw an exception; key is not yet saved
retrieveKeyAndExpectNotFound(databaseInteractor, masterKeyId);
SaveKeyringResult saveKeyringResult = databaseInteractor.savePublicKeyRing(ring, new ProgressScaler(), null);
SaveKeyringResult saveKeyringResult = databaseInteractor.savePublicKeyRing(ring, null);
boolean saveSuccess = saveKeyringResult.success();