Skip to content

Commit

Permalink
Feedback upload refactor
Browse files Browse the repository at this point in the history
- Update psiphon-tunnel-core to 3a44f293 which provides improved
  feedback upload API that is asynchronous
- Update PsiphonClientCommonLibrary to 1194280 for feedback packaging
  changes
- Create FeedbackReducer for serializing feedback upload attempts
- Only perform feedback upload when network is available and the tunnel
  is disconnected or connected
  • Loading branch information
mirokuratczyk committed Sep 25, 2020
1 parent 37e175f commit c141e3f
Show file tree
Hide file tree
Showing 42 changed files with 1,035 additions and 496 deletions.
2 changes: 1 addition & 1 deletion Podfile
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ target 'Psiphon' do
pod "InAppSettingsKit", :git => "https://github.com/Psiphon-Inc/InAppSettingsKit.git", :commit => '8bd203c'
#pod "InAppSettingsKit", :path => "../InAppSettingsKit"
#pod "PsiphonClientCommonLibrary", :path => "../psiphon-ios-client-common-library"
pod 'PsiphonClientCommonLibrary', :git => "https://github.com/Psiphon-Inc/psiphon-ios-client-common-library.git", :commit => '4d90586'
pod 'PsiphonClientCommonLibrary', :git => "https://github.com/Psiphon-Inc/psiphon-ios-client-common-library.git", :commit => '1194280'

# Swift dependencies
pod 'ReactiveObjC', :git => "https://github.com/Psiphon-Inc/ReactiveObjC.git", :commit => '8bbf9dd'
Expand Down
12 changes: 6 additions & 6 deletions Podfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ PODS:
- InAppSettingsKit (2.15)
- MBProgressHUD (1.1.0)
- PersonalizedAdConsent (1.0.5)
- PsiphonClientCommonLibrary (2.0.11):
- PsiphonClientCommonLibrary (3.0.0):
- InAppSettingsKit (= 2.15)
- ReactiveObjC (3.1.0)
- SVProgressHUD (2.2.5)
Expand All @@ -13,7 +13,7 @@ DEPENDENCIES:
- InAppSettingsKit (from `https://github.com/Psiphon-Inc/InAppSettingsKit.git`, commit `8bd203c`)
- MBProgressHUD (~> 1.1.0)
- PersonalizedAdConsent (~> 1.0)
- PsiphonClientCommonLibrary (from `https://github.com/Psiphon-Inc/psiphon-ios-client-common-library.git`, commit `4d90586`)
- PsiphonClientCommonLibrary (from `https://github.com/Psiphon-Inc/psiphon-ios-client-common-library.git`, commit `1194280`)
- ReactiveObjC (from `https://github.com/Psiphon-Inc/ReactiveObjC.git`, commit `8bbf9dd`)
- SVProgressHUD (~> 2.2.5)

Expand All @@ -29,7 +29,7 @@ EXTERNAL SOURCES:
:commit: 8bd203c
:git: https://github.com/Psiphon-Inc/InAppSettingsKit.git
PsiphonClientCommonLibrary:
:commit: 4d90586
:commit: '1194280'
:git: https://github.com/Psiphon-Inc/psiphon-ios-client-common-library.git
ReactiveObjC:
:commit: 8bbf9dd
Expand All @@ -40,7 +40,7 @@ CHECKOUT OPTIONS:
:commit: 8bd203c
:git: https://github.com/Psiphon-Inc/InAppSettingsKit.git
PsiphonClientCommonLibrary:
:commit: 4d90586
:commit: '1194280'
:git: https://github.com/Psiphon-Inc/psiphon-ios-client-common-library.git
ReactiveObjC:
:commit: 8bbf9dd
Expand All @@ -51,10 +51,10 @@ SPEC CHECKSUMS:
InAppSettingsKit: 4890a44f9df7a1742c632920f0ea939cbeb29ea2
MBProgressHUD: e7baa36a220447d8aeb12769bf0585582f3866d9
PersonalizedAdConsent: dbecabb3467df967c16d9cebc2ef4a8890e4bbd8
PsiphonClientCommonLibrary: 788ef3cc1d7aa9ce1e88ed1e30a77ec181d89910
PsiphonClientCommonLibrary: bbef27228ea9756933a189d7ab5088df35c5889a
ReactiveObjC: 2a38ea15335de4119d8b17caf1db1484f61db902
SVProgressHUD: 1428aafac632c1f86f62aa4243ec12008d7a51d6

PODFILE CHECKSUM: 06556bef42d645309ddab07c5601a3daaf9810a7
PODFILE CHECKSUM: be5b26e8186c186db4e5779eb1f52b17a629c901

COCOAPODS: 1.8.4
8 changes: 4 additions & 4 deletions PsiApi/Sources/AppStoreIAP/IAPReducer.swift
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ public struct IAPEnvironment {
var tunnelStatusSignal: SignalProducer<TunnelProviderVPNStatus, Never>
var tunnelConnectionRefSignal: SignalProducer<TunnelConnection?, Never>
var psiCashEffects: PsiCashEffects
var clientMetaData: () -> ClientMetaData
var appInfo: () -> AppInfoProvider
var paymentQueue: PaymentQueue
var psiCashPersistedValues: PsiCashPersistedValues
var isSupportedProduct: (ProductID) -> AppStoreProductType?
Expand All @@ -89,7 +89,7 @@ public struct IAPEnvironment {
tunnelStatusSignal: SignalProducer<TunnelProviderVPNStatus, Never>,
tunnelConnectionRefSignal: SignalProducer<TunnelConnection?, Never>,
psiCashEffects: PsiCashEffects,
clientMetaData: @escaping () -> ClientMetaData,
appInfo: @escaping () -> AppInfoProvider,
paymentQueue: PaymentQueue,
psiCashPersistedValues: PsiCashPersistedValues,
isSupportedProduct: @escaping (ProductID) -> AppStoreProductType?,
Expand All @@ -102,7 +102,7 @@ public struct IAPEnvironment {
self.tunnelStatusSignal = tunnelStatusSignal
self.tunnelConnectionRefSignal = tunnelConnectionRefSignal
self.psiCashEffects = psiCashEffects
self.clientMetaData = clientMetaData
self.appInfo = appInfo
self.paymentQueue = paymentQueue
self.psiCashPersistedValues = psiCashPersistedValues
self.isSupportedProduct = isSupportedProduct
Expand Down Expand Up @@ -256,7 +256,7 @@ public func iapReducer(
receipt: receiptData,
customData: customData
),
clientMetaData: environment.clientMetaData()
clientMetaData: ClientMetaData(environment.appInfo())
)

let psiCashVerifyRequest = RetriableTunneledHttpRequest(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ public struct SubscriptionAuthStateReducerEnvironment {
public let sharedDB: SharedDBContainer
public let tunnelStatusSignal: SignalProducer<TunnelProviderVPNStatus, Never>
public let tunnelConnectionRefSignal: SignalProducer<TunnelConnection?, Never>
public let clientMetaData: () -> ClientMetaData
public let appInfo: () -> AppInfoProvider
public let getCurrentTime: () -> Date
public let compareDates: (Date, Date, Calendar.Component) -> ComparisonResult

Expand All @@ -109,7 +109,7 @@ public struct SubscriptionAuthStateReducerEnvironment {
sharedDB: SharedDBContainer,
tunnelStatusSignal: SignalProducer<TunnelProviderVPNStatus, Never>,
tunnelConnectionRefSignal: SignalProducer<TunnelConnection?, Never>,
clientMetaData: @escaping () -> ClientMetaData,
appInfo: @escaping () -> AppInfoProvider,
getCurrentTime: @escaping () -> Date,
compareDates: @escaping (Date, Date, Calendar.Component) -> ComparisonResult) {

Expand All @@ -122,7 +122,7 @@ public struct SubscriptionAuthStateReducerEnvironment {
self.sharedDB = sharedDB
self.tunnelStatusSignal = tunnelStatusSignal
self.tunnelConnectionRefSignal = tunnelConnectionRefSignal
self.clientMetaData = clientMetaData
self.appInfo = appInfo
self.getCurrentTime = getCurrentTime
self.compareDates = compareDates
}
Expand Down Expand Up @@ -339,7 +339,7 @@ public func subscriptionAuthStateReducer(
productID: purchaseWithLatestExpiry.purchase.productID,
receipt: receiptData
),
clientMetaData: environment.clientMetaData()
clientMetaData: ClientMetaData(environment.appInfo())
)

let authRequest = RetriableTunneledHttpRequest(
Expand Down
2 changes: 1 addition & 1 deletion PsiApi/Sources/PsiApi/InternetReachability.swift
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import Foundation
import Utilities
import ReactiveSwift

@objc public enum ReachabilityStatus: Int {
@objc public enum ReachabilityStatus: Int {
case notReachable
case viaWiFi
case viaWWAN
Expand Down
21 changes: 19 additions & 2 deletions PsiApi/Sources/PsiApi/Logging/FeedbackLogging.swift
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@ public protocol FeedbackLogHandler {
func fatalError(type: String, message: String)

func feedbackLog(level: NonFatalLogLevel, type: String, message: String)

func feedbackLogNotice(type: String, message: String, timestamp: String)

}

Expand All @@ -49,6 +51,10 @@ public struct StdoutFeedbackLogger: FeedbackLogHandler {
public func feedbackLog(level: NonFatalLogLevel, type: String, message: String) {
print("[\(String(describing: level))] type: '\(type)' message: '\(message)'")
}

public func feedbackLogNotice(type: String, message: String, timestamp: String) {
print("[Notice] type: '\(type)' message: '\(message)' timestamp: '\(timestamp)'")
}

}

Expand All @@ -58,16 +64,21 @@ final class ArrayFeedbackLogHandler: FeedbackLogHandler {
let level: LogLevel
let type: String
let message: String
let timestamp: String?
}

var logs = [Log]()

func fatalError(type: String, message: String) {
logs.append(Log(level: .fatal, type: type, message: message))
logs.append(Log(level: .fatal, type: type, message: message, timestamp: .none))
}

func feedbackLog(level: NonFatalLogLevel, type: String, message: String) {
logs.append(Log(level: .nonFatal(level), type: type, message: message))
logs.append(Log(level: .nonFatal(level), type: type, message: message, timestamp: .none))
}

public func feedbackLogNotice(type: String, message: String, timestamp: String) {
logs.append(Log(level: .nonFatal(.info), type: type, message: message, timestamp: .some(timestamp)))
}

}
Expand Down Expand Up @@ -158,6 +169,12 @@ public struct FeedbackLogger {
}
}

public func logNotice(type: String, value: String, timestamp: String) -> Effect<Never> {
.fireAndForget {
self.handler.feedbackLogNotice(type: type, message: value, timestamp: timestamp)
}
}

public func immediate(
_ level: NonFatalLogLevel, _ value: LogMessage, file: String = #file, line: UInt = #line
) {
Expand Down
39 changes: 38 additions & 1 deletion PsiApi/Sources/PsiApi/ReactiveSwift+Additions.swift
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,44 @@ public struct Combined<Value> {
}
}

/// A value that represents either the next value in a stream of values
/// or that the signal should be completed. Allows for an inclusive
/// `take(while:)` operator, e.g.:
///
/// .flatMap(.latest) { (done: Bool) -> SignalProducer<SignalTermination<String>, Never> in
/// if done {
/// // End the stream.
/// return SignalProducer(value:.terminate).prefix(value:.value("last value emitted"))
/// } else {
/// return SignalProducer(value:.value("next value emitted"))
/// }
/// }
/// .take(while: { (signalTermination: SignalTermination<String>) -> Bool in
/// // Forwards values while the `.terminate` value has not been emitted.
/// guard case .value(_) = signalTermination else {
/// return false
/// }
/// return true
/// })
/// .map { (signalTermination: SignalTermination<String>) -> String in
/// guard case let .value(x) = signalTermination else {
/// // Will never happen.
/// fatalError()
/// }
/// return x
/// }
///
/// - Note: can be replaced by an inclusive `take(while:)` operator.
public enum SignalTermination<Value> {

/// The next value in the stream.
case value(Value)

/// Value which indicates that the signal should be completed.
case terminate

}

extension Signal where Error == Never {

public func observe<A>(store: Store<A, Value>) -> Disposable? {
Expand Down Expand Up @@ -114,4 +152,3 @@ extension SignalProducer where Value: Collection, Error == Never {
}

}

17 changes: 5 additions & 12 deletions PsiApi/Sources/PsiApi/http/Requests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -319,14 +319,7 @@ public struct RetriableTunneledHttpRequest<Response: RetriableHTTPResponse>: Equ
/// Request error due to response indicating a retry is needed.
case responseRetryError(Response.Failure)
}

/// `SignalTermination` represents whether the authorization request signal has completed,
/// and that the signal should be completed.
private enum SignalTermination: Equatable {
case value(RequestResult)
case terminate
}


let request: HTTPRequest<Response>
let retryCount: Int
let retryInterval: DispatchTimeInterval
Expand All @@ -351,7 +344,7 @@ public struct RetriableTunneledHttpRequest<Response: RetriableHTTPResponse>: Equ
return lhs.0 == rhs.0 && lhs.1 == rhs.1
})
.flatMap(.latest) { (value: (TunnelProviderVPNStatus, TunnelConnection?))
-> SignalProducer<SignalTermination, ErrorEvent<Response.Failure>> in
-> SignalProducer<SignalTermination<RequestResult>, ErrorEvent<Response.Failure>> in

let vpnStatus = Debugging.ignoreTunneledChecks ? .connected : value.0
guard case .connected = vpnStatus else {
Expand All @@ -376,7 +369,7 @@ public struct RetriableTunneledHttpRequest<Response: RetriableHTTPResponse>: Equ
tunnelErrorEvent.map { .tunnelError($0) }
}
.flatMap(.latest) { (response: Response)
-> SignalProducer<SignalTermination, ErrorEvent<RetryError>> in
-> SignalProducer<SignalTermination<RequestResult>, ErrorEvent<RetryError>> in

// Determines if the request needs to be retried.
let (result, maybeRetryDueToError) = response.unpackRetriableResultError()
Expand All @@ -394,7 +387,7 @@ public struct RetriableTunneledHttpRequest<Response: RetriableHTTPResponse>: Equ
}
}
.flatMapError { (requestRetryErrorEvent: ErrorEvent<RetryError>)
-> SignalProducer<SignalTermination, ErrorEvent<Response.Failure>> in
-> SignalProducer<SignalTermination<RequestResult>, ErrorEvent<Response.Failure>> in

// If the tunnel error request is not retriable, then the signal is completed
// and no further retries are carried out.
Expand Down Expand Up @@ -422,7 +415,7 @@ public struct RetriableTunneledHttpRequest<Response: RetriableHTTPResponse>: Equ
on: QueueScheduler(qos: .default, name: "RetryScheduler"))
}
.flatMapError { (responseError: ErrorEvent<Response.Failure>)
-> Effect<SignalTermination> in
-> Effect<SignalTermination<RequestResult>> in
// Maps failure response error after all retries from a signal failure
// to a signal value event.
return SignalProducer(value: .value(.failed(responseError)))
Expand Down
Loading

0 comments on commit c141e3f

Please sign in to comment.