Respond to CR.

This commit is contained in:
Matthew Chen 2019-04-19 10:03:28 -04:00
parent d02ec04031
commit 06968cc05c
16 changed files with 77 additions and 38 deletions

View File

@ -879,7 +879,7 @@ extension %s {
// Fetches a single model by "unique id".
@objc
public class func anyFetch(withUniqueId uniqueId: String,
public class func anyFetch(uniqueId: String,
transaction: SDSAnyReadTransaction) -> %s? {
assert(uniqueId.count > 0)

View File

@ -242,7 +242,7 @@ extension TSThread {
// Fetches a single model by "unique id".
@objc
public class func anyFetch(withUniqueId uniqueId: String,
public class func anyFetch(uniqueId: String,
transaction: SDSAnyReadTransaction) -> TSThread? {
assert(uniqueId.count > 0)

View File

@ -315,7 +315,7 @@ extension TSAttachment {
// Fetches a single model by "unique id".
@objc
public class func anyFetch(withUniqueId uniqueId: String,
public class func anyFetch(uniqueId: String,
transaction: SDSAnyReadTransaction) -> TSAttachment? {
assert(uniqueId.count > 0)

View File

@ -1063,7 +1063,7 @@ extension TSInteraction {
// Fetches a single model by "unique id".
@objc
public class func anyFetch(withUniqueId uniqueId: String,
public class func anyFetch(uniqueId: String,
transaction: SDSAnyReadTransaction) -> TSInteraction? {
assert(uniqueId.count > 0)

View File

@ -158,7 +158,7 @@ extension OWSMessageContentJob {
// Fetches a single model by "unique id".
@objc
public class func anyFetch(withUniqueId uniqueId: String,
public class func anyFetch(uniqueId: String,
transaction: SDSAnyReadTransaction) -> OWSMessageContentJob? {
assert(uniqueId.count > 0)

View File

@ -150,7 +150,7 @@ extension OWSMessageDecryptJob {
// Fetches a single model by "unique id".
@objc
public class func anyFetch(withUniqueId uniqueId: String,
public class func anyFetch(uniqueId: String,
transaction: SDSAnyReadTransaction) -> OWSMessageDecryptJob? {
assert(uniqueId.count > 0)

View File

@ -151,7 +151,7 @@ extension TSRecipientReadReceipt {
// Fetches a single model by "unique id".
@objc
public class func anyFetch(withUniqueId uniqueId: String,
public class func anyFetch(uniqueId: String,
transaction: SDSAnyReadTransaction) -> TSRecipientReadReceipt? {
assert(uniqueId.count > 0)

View File

@ -37,14 +37,14 @@ class DownloadStickerOperation: OWSOperation {
return OWSSignalService.sharedInstance().cdnSessionManager
}
var firstAttempt = true
override public func run() {
if InstalledStickers.isStickerInstalled(packId: packId,
stickerId: stickerId) {
Logger.verbose("Skipping redundant operation.")
return didFail(error: StickerError.redundantOperation)
var error = StickerError.redundantOperation
error.isRetryable = false
return reportError(error)
}
// https://cdn.signal.org/stickers/<pack_id>/full/<sticker_id>
@ -69,24 +69,25 @@ class DownloadStickerOperation: OWSOperation {
Logger.verbose("Decryption succeeded.")
self.success(plaintext)
self.didSucceed()
self.reportSuccess()
} catch let error as NSError {
owsFailDebug("Decryption failed: \(error)")
// Fail immediately; do not retry.
self.didFail(error: error)
error.isRetryable = false
return self.reportError(error)
}
}) { [weak self] (_, error) in
},
failure: { [weak self] (_, error) in
guard let self = self else {
return
}
Logger.error("Download failed: \(error)")
self.failureCount += 1
// TODO: We need to discriminate retry-able errors from
// 404s, etc. We might want to abort on all 4xx and 5xx.
self.reportError(error)
}
})
}
override public func didFail(error: Error) {
@ -95,8 +96,6 @@ class DownloadStickerOperation: OWSOperation {
failure(error)
}
private var failureCount: UInt = 0
override public func retryInterval() -> TimeInterval {
// Arbitrary backoff factor...
// With backOffFactor of 1.9
@ -109,7 +108,7 @@ class DownloadStickerOperation: OWSOperation {
let backoffFactor = 1.9
let maxBackoff = kHourInterval
let seconds = 0.1 * min(maxBackoff, pow(backoffFactor, Double(self.failureCount)))
let seconds = 0.1 * min(maxBackoff, pow(backoffFactor, Double(self.errorCount)))
return seconds
}
}

View File

@ -34,13 +34,13 @@ class DownloadStickerPackOperation: OWSOperation {
return OWSSignalService.sharedInstance().cdnSessionManager
}
var firstAttempt = true
override public func run() {
if InstalledStickers.isStickerPackInstalled(packId: packId) {
Logger.verbose("Skipping redundant operation.")
return didFail(error: StickerError.redundantOperation)
var error = StickerError.redundantOperation
error.isRetryable = false
return reportError(error)
}
// https://cdn.signal.org/stickers/<pack_id>/manifest.proto
@ -65,24 +65,25 @@ class DownloadStickerPackOperation: OWSOperation {
Logger.verbose("Decryption succeeded.")
self.success(plaintext)
self.didSucceed()
self.reportSuccess()
} catch let error as NSError {
owsFailDebug("Decryption failed: \(error)")
// Fail immediately; do not retry.
self.didFail(error: error)
error.isRetryable = false
return self.reportError(error)
}
}) { [weak self] (_, error) in
},
failure: { [weak self] (_, error) in
guard let self = self else {
return
}
Logger.error("Download failed: \(error)")
self.failureCount += 1
// TODO: We need to discriminate retry-able errors from
// 404s, etc. We might want to abort on all 4xx and 5xx.
self.reportError(error)
}
})
}
override public func didFail(error: Error) {
@ -91,8 +92,6 @@ class DownloadStickerPackOperation: OWSOperation {
failure(error)
}
private var failureCount: UInt = 0
override public func retryInterval() -> TimeInterval {
// Arbitrary backoff factor...
// With backOffFactor of 1.9
@ -105,7 +104,7 @@ class DownloadStickerPackOperation: OWSOperation {
let backoffFactor = 1.9
let maxBackoff = kHourInterval
let seconds = 0.1 * min(maxBackoff, pow(backoffFactor, Double(self.failureCount)))
let seconds = 0.1 * min(maxBackoff, pow(backoffFactor, Double(self.errorCount)))
return seconds
}
}

View File

@ -154,7 +154,7 @@ extension InstalledSticker {
// Fetches a single model by "unique id".
@objc
public class func anyFetch(withUniqueId uniqueId: String,
public class func anyFetch(uniqueId: String,
transaction: SDSAnyReadTransaction) -> InstalledSticker? {
assert(uniqueId.count > 0)

View File

@ -168,7 +168,7 @@ extension InstalledStickerPack {
// Fetches a single model by "unique id".
@objc
public class func anyFetch(withUniqueId uniqueId: String,
public class func anyFetch(uniqueId: String,
transaction: SDSAnyReadTransaction) -> InstalledStickerPack? {
assert(uniqueId.count > 0)

View File

@ -73,7 +73,7 @@ public class InstalledStickers: NSObject {
let uniqueId = InstalledStickerPack.uniqueId(forPackId: packId)
return InstalledStickerPack.anyFetch(withUniqueId: uniqueId, transaction: transaction)
return InstalledStickerPack.anyFetch(uniqueId: uniqueId, transaction: transaction)
}
@objc
@ -217,7 +217,7 @@ public class InstalledStickers: NSObject {
let uniqueId = InstalledSticker.uniqueId(forPackId: packId, stickerId: stickerId)
return InstalledSticker.anyFetch(withUniqueId: uniqueId, transaction: transaction)
return InstalledSticker.anyFetch(uniqueId: uniqueId, transaction: transaction)
}
@objc

View File

@ -1,5 +1,5 @@
//
// Copyright (c) 2018 Open Whisper Systems. All rights reserved.
// Copyright (c) 2019 Open Whisper Systems. All rights reserved.
//
import Foundation
@ -22,7 +22,12 @@ import Foundation
public extension Error {
var isRetryable: Bool {
return (self as NSError).isRetryable
get {
return (self as NSError).isRetryable
}
set {
(self as NSError).isRetryable = newValue
}
}
}

View File

@ -1,5 +1,5 @@
//
// Copyright (c) 2018 Open Whisper Systems. All rights reserved.
// Copyright (c) 2019 Open Whisper Systems. All rights reserved.
//
#import <Foundation/Foundation.h>
@ -24,7 +24,9 @@ typedef NS_ENUM(NSInteger, OWSOperationState) {
// any of the errors were fatal. Fatal errors trump retryable errors.
@interface OWSOperation : NSOperation
@property (readonly, nullable) NSError *failingError;
@property (nonatomic, readonly, nullable) NSError *failingError;
@property (nonatomic, readonly) NSUInteger errorCount;
// Defaults to 0, set to greater than 0 in init if you'd like the operation to be retryable.
@property NSUInteger remainingRetries;

View File

@ -15,13 +15,15 @@ NSString *const OWSOperationKeyIsFinished = @"isFinished";
@interface OWSOperation ()
@property (nullable) NSError *failingError;
@property (nonatomic, nullable) NSError *failingError;
@property (atomic) OWSOperationState operationState;
@property (nonatomic) OWSBackgroundTask *backgroundTask;
// This property should only be accessed on the main queue.
@property (nonatomic) NSTimer *_Nullable retryTimer;
@property (nonatomic) NSUInteger errorCount;
@end
@implementation OWSOperation
@ -172,6 +174,8 @@ NSString *const OWSOperationKeyIsFinished = @"isFinished";
error.isRetryable,
(unsigned long)self.remainingRetries);
self.errorCount += 1;
[self didReportError:error];
if (error.isFatal) {

View File

@ -81,4 +81,34 @@ class SDSKeyValueStoreTest: SSKBaseTestSwift {
XCTAssertNil(store.getString("stringA"))
XCTAssertEqual("valueC", store.getString("stringB"))
}
func test_data() {
let store = SDSKeyValueStore(collection: "test")
let bytesA = Randomness.generateRandomBytes(32)
let bytesB = Randomness.generateRandomBytes(32)
XCTAssertNil(store.getData("dataA"))
XCTAssertNil(store.getData("dataB"))
store.setData(bytesA, key: "dataA")
XCTAssertEqual(bytesA, store.getData("dataA"))
XCTAssertNil(store.getData("dataB"))
store.setData(bytesB, key: "dataA")
XCTAssertEqual(bytesB, store.getData("dataA"))
XCTAssertNil(store.getData("dataB"))
store.setData("valueC", key: "dataB")
XCTAssertEqual(bytesB, store.getData("dataA"))
XCTAssertEqual("valueC", store.getData("dataB"))
store.setData(nil, key: "dataA")
XCTAssertNil(store.getData("dataA"))
XCTAssertEqual("valueC", store.getData("dataB"))
}
}