feat: migrating storages #10
No reviewers
Labels
No Label
No Milestone
No project
No Assignees
2 Participants
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: TouchInstinct/LeadKit#10
Loading…
Reference in New Issue
No description provided.
Delete Branch "feature/migrating_storage"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
MigratingStorage
При необходимости мигрировать с одного keychain на другой можно воспользоваться классом
BaseMigratingSingleValueStorage. При создании "мигрирующего" хранилища необходимо будет указать:KeychainMigratingCodableBackingStore
@ -0,0 +38,4 @@return value}if case let .success(value) = container.sourceStorage.codableObject(forKey: key, decoder: decoder) {может через цепочку?
что-то такое:
@ -0,0 +74,4 @@return value}if case let .success(value) = container.sourceStorage.codableObject(forKey: key, decoder: decoder) {копипаста. можем что-то с этим сделать?
@ -0,0 +42,4 @@open func hasStoredValue() -> Bool {if targetStorage.hasStoredValue() {let _ = sourceStorage.deleteValue()не думаю, что это тут требуется
скорее просто
@ -0,0 +51,4 @@}open func store(value: ValueType) -> Result<Void, StorageError> {targetStorage.store(value: value)так как миграция у нас ленивая, то может быть кейс когда мы сначала будем записывать значение в storage, а потом пробовать читать и тогда в getValue отработает скорей всего неверно
давай тут удаление их sourceStorage сделаем перед записью в targetStorage
@ -0,0 +62,4 @@let result = sourceStorage.getValue()if case let .success(value) = result, case .success = store(value: value) {let _ = sourceStorage.deleteValue()let обязательно?
@ -0,0 +69,4 @@}open func deleteValue() -> Result<Void, StorageError> {if targetStorage.hasStoredValue() {да можно и не проверять
@ -0,0 +75,4 @@return targetStorage.deleteValue()}return sourceStorage.deleteValue()?
@ -0,0 +24,4 @@import KeychainAccessimport TISwiftUtilspublic typealias KeychainMigratingCodableBackingStore<T: Codable> = MigratingBackingStore<Keychain,лишний пробел
@ -0,0 +23,4 @@import TIFoundationUtilsimport KeychainAccessopen class BaseMigratingSingleValueKeychainStorage<ValueType>: BaseMigratingSingleValueStorage<ValueType,я не понимаю
BaseMigratingSingleValueStorage<SourceStorage: SingleValueStorage,
TargetStorage: SingleValueStorage>
2 generic параметра типа SingleValueStorage, а тут передаётся ValueType и Container
@ -0,0 +29,4 @@open func codableObject<Value: Decodable>(forKey key: StorageKey<Value>,decoder: CodableKeyValueDecoder) -> Result<Value, StorageError> {if case let .success(value) = targetStorage.codableObject(forKey: key, decoder: decoder) {а тут не должно быть кода с миграцией? в чем суть storage container'а?
@ -0,0 +44,4 @@forKey key: StorageKey<Value>,encoder: CodableKeyValueEncoder) -> Result<Void, StorageError> {targetStorage.set(encodableObject: encodableObject, forKey: key, encoder: encoder)и тут удалить из старого
@ -0,0 +48,4 @@}open func removeCodableValue<Value: Codable>(forKey key: StorageKey<Value>) -> Result<Void, StorageError> {if case .success = sourceStorage.hasCodableValue(forKey: key) {проверка на has лишняя кмк
@ -0,0 +60,4 @@}open func hasCodableValue<Value: Decodable>(forKey key: StorageKey<Value>) -> Result<Bool, StorageError> {if case .success = targetStorage.hasCodableValue(forKey: key) {тоже через flatMapError как в комменте к codableObject(forKey:)
@ -0,0 +44,4 @@return codableResult}return container.getStringValue(forKey: storageKey)это какой-то compatibility?)
@ -33,6 +33,8 @@ open class BaseSingleValueStorage<ValueType, StorageType>: SingleValueStorage {public let getValueClosure: GetValueClosurepublic let storeValueClosure: StoreValueClosurepublic var migrationStorage: StorageType?я не понимаю зачем так делать. мы не должны эту сущности связывать
@ -0,0 +59,4 @@}.flatMapError { _ insourceStorage.getValue().flatMap { value in? можно даже без self навер
Вроде не можем, т.к.
store(value:)возвращаяетResult<Void, Error>, на нам нуженResult<Value, Error>@ -0,0 +65,4 @@return value}.mapError { .unableToExtractData(underlyingError: $0) }и это не потребуется наверное
@ -0,0 +45,4 @@forKey key: StorageKey<Value>,encoder: CodableKeyValueEncoder) -> Result<Void, StorageError> {removeSourceValue(forKey: key)даже если мы не пробрасываем результат выше, то надо залогировать (см. пример в TINetworking)
@ -0,0 +53,4 @@open func removeCodableValue<Value: Codable>(forKey key: StorageKey<Value>) -> Result<Void, StorageError> {targetStorage.removeCodableValue(forKey: key).flatMap {if let value = try? sourceStorage.hasCodableValue(forKey: key).get(), value {в случае codable, вызов hasCodableValue будет равносилен получению codableObject (чтение + декодирование в объект), поэтому предлагаю сразу вызвать removeCodableValue и сделать flatMapError с проверкой на valueNotFound
@ -0,0 +70,4 @@// MARK: - Open methodsopen func removeSourceValue<Value>(forKey key: StorageKey<Value>) {тут не надо вернуть
Result<Void, StorageError>?@ -0,0 +25,4 @@public final class KeychainMigratingStorageContainer: BaseCodableMigratingStorageContainer<Keychain, Keychain> {public override func removeSourceValue<Value>(forKey key: StorageKey<Value>) {а зачем на этот метод отдельно? вроде бы мы storage'ы закрываем протоколами и там есть функция для этого
Проблема в том, что из методов, в которых мы удаляем значение из хранилища, Value ограничен протоколом Decodable. А стандартный метод удаления требует, чтобы Value был Decodable. Нашел вот такой способ обойти ограничение без изменения сигнатуры метода удаления
@ -18,3 +13,1 @@s.source_files = s.name + '/' + sourcess.exclude_files = s.name + '/*.app'ends.source_files = s.name + '/Sources/**/*'смержилось неверно
@ -0,0 +41,4 @@let oldProfile = Profile(name: "some", age: 0)let sourceStorage = MockMigratingStorageContainer.defaultContainer.sourceStoragelet _ = sourceStorage.set(encodableObject: oldProfile, forKey: .profile, encoder: encoder)результат тоже надо через XCTAssert проверять
@ -0,0 +46,4 @@XCTAssertEqual(profile, oldProfile)}func testGetValueFromTarget() throws {в итоге, тут протестирована просто работа targetSource? если да, то этот тест надо в отдельный набор тестов надо вынести. так как тут тесты про миграцию
@ -0,0 +53,4 @@let _ = targetStorage.set(encodableObject: newProfile, forKey: .profile, encoder: encoder)XCTAssertEqual(profile, newProfile)XCTAssertEqual(try? targetStorage.codableObject(forKey: .profile, decoder: decoder).get(), newProfile)optional try тут вроде не нужен, так как функция декларирует throws
@ -0,0 +63,4 @@profile = newProfileXCTAssertEqual(profile, newProfile)XCTAssertEqual(try? targetStorage.codableObject(forKey: .profile, decoder: decoder).get(), newProfile)аналогично, про optional try
@ -0,0 +50,4 @@}open func store(value: ValueType) -> Result<Void, StorageError> {let _ = sourceStorage.deleteValue()@ -0,0 +63,4 @@open func getValue() -> Result<ValueType, StorageError> {targetStorage.getValue().flatMap {let _ = sourceStorage.deleteValue()аналогично, так как возвращаемое значение не участвует в дальнейшей цепочке
@ -0,0 +75,4 @@.flatMapError { _ insourceStorage.getValue().flatMap { value inResult {вроде так логичней:
Так не, вывод у
store(value:)не тот, который ожидается отgetValue()а, ну верно. тогда:
?
@ -0,0 +97,4 @@.flatMapError {errorLogger.log(error: $0, file: #file, line: #line)return sourceStorage.deleteValue()вот тут странно. получается, если мы не смогли удалить значение из source несколькими строками выше, то тоже попадём сюда и попробуем еще раз?
@ -0,0 +28,4 @@CodableKeyValueStoragewhere SourceStorage: CodableKeyValueStorage, TargetStorage: CodableKeyValueStorage {public enum CodableTypeError: Error {unused
@ -0,0 +48,4 @@targetStorage.codableObject(forKey: key, decoder: decoder).flatMap {let _ = removeSourceValue(forKey: key)if case let
@ -0,0 +58,4 @@return .success($0)}.flatMapError { _ insourceStorage.codableObject(forKey: key, decoder: decoder)вот тут можно ещё залогировать, если ошибка не .valueNotFound
@ -0,0 +66,4 @@forKey key: StorageKey<Value>,encoder: CodableKeyValueEncoder) -> Result<Void, StorageError> {let _ = removeSourceValue(forKey: key)if case let
@ -0,0 +83,4 @@.flatMapError { _ in .success(value) }}.flatMapError {errorLogger.log(error: $0, file: #file, line: #line)логируем если не .valueNotFound
@ -0,0 +34,4 @@override func setUp() {profile = nillet _ = MockMigratingStorageContainer.defaultContainer.sourceStorage.removeValue(forKey: .profile)let _ = MockMigratingStorageContainer.defaultContainer.targetStorage.removeValue(forKey: .profile)profile = nil не затрёт в source и target?
Затрет, поправил
@ -0,0 +1,26 @@//// Copyright (c) 2020 Touch Instinct2023
@ -0,0 +32,4 @@where StoreContent == Value? {let getClosure: GetClosure = { container in// If source storage has value and target doesn't we can't migrate it from storate containerstorate ?
ну и вообще коммент не понятен
Сделал коммент подробнее. Контейнер не может мигрировать значение из-за того, что в методе
codableObject(forKey:decoder)нет доступа к encoder'у и Value ограничен только Decodable@ -0,0 +35,4 @@// If source storage has value and target doesn't we can't migrate it from storate containertry? container.codableObject(forKey: key, decoder: decoder).flatMap {let _ = container.set(encodableObject: $0, forKey: key, encoder: encoder)видимо коммент должен был прояснить зачем мы сначала получаем значение а потом его обратно записываем.
разве мигация не происходит внутри контейнера?
окей, а можем это вынести в отдельный (статический?) метод, чтобы не дублировать логину ниже
@ -0,0 +57,4 @@where StoreContent == Value? {let getClosure: GetClosure = { container in// If source storage has value and target doesn't we can't migrate it from storate containerаналогично
@ -0,0 +60,4 @@// If source storage has value and target doesn't we can't migrate it from storate containertry? container.codableObject(forKey: key, decoder: decoder).flatMap {let _ = container.set(encodableObject: $0, forKey: key, encoder: encoder)аналогично
@ -0,0 +22,4 @@import TILoggingopen class BaseMigratingSingleValueStorage<SourceStorage: SingleValueStorage,не вижу наследований. может сделать final? потенциально могут вообще быть кейсы, когда нам надо будет это переопределять?
@ -0,0 +89,4 @@return sourceStorage.deleteValue()}.flatMap {if sourceStorage.hasStoredValue() {кажется, тут лучше разорвать цепочку и сделать примерно так:
@ -0,0 +71,4 @@}open func removeCodableValue<Value: Codable>(forKey key: StorageKey<Value>) -> Result<Void, StorageError> {targetStorage.removeCodableValue(forKey: key)тут вроде аналогичная логика как и выше с двумя результатами
up
@ -0,0 +21,4 @@//import XCTest@testable import TIFoundationUtilsтут правда нужен именно testable import? расскажи плз до чего нужен доступ
@ -0,0 +43,4 @@let oldProfile = Profile(name: "old", age: 0)let newProfile = Profile(name: "new", age: 0)let sourceStorage = MockMigratingStorageContainer.defaultContainer.sourceStoragelet targetStorage = MockMigratingStorageContainer.defaultContainer.targetStorageможет вынести их в property TestCase'а? а то получается лишнее дублирование двух строк в каждом кейсе
@ -0,0 +74,4 @@targetStorage.removeCodableValue(forKey: key).flatMap { value insourceStorage.removeCodableValue(forKey: key).flatMapError { _ in .success(value) }залогировать если не .valueNotFound?
@ -0,0 +81,4 @@errorLogger.log(error: $0, file: #file, line: #line)}return removeSourceValue(forKey: key)я не понимаю.
как мне представляется, текущий код работает так:
а нужно:
@ -0,0 +88,4 @@open func hasCodableValue<Value: Decodable>(forKey key: StorageKey<Value>) -> Result<Bool, StorageError> {targetStorage.hasCodableValue(forKey: key).flatMapError { _ insourceStorage.hasCodableValue(forKey: key)тут тоже надо отдать приоритет ошибке из target (если она не .valueNotFound)
@ -0,0 +102,4 @@return .failure(error)case let (.failure, .failure(error)):в этом случае я бы отдал приоритет error из targetSource
@ -0,0 +36,4 @@public init(sourceStorage: SourceStorage,targetStorage: TargetStorage,errorLogger: ErrorLogger = TIFoundationLogger(category: "BaseMigratingSingleValueStorage")) {надо переименовать и в доках тоже
@ -0,0 +50,4 @@}public func store(value: ValueType) -> Result<Void, StorageError> {if case let .failure(error) = sourceStorage.deleteValue() {сначала надо записать в target и если получилось уже удалять из source
@ -0,0 +69,4 @@.flatMapError { _ insourceStorage.getValue().flatMap { value inResult {не?
@ -0,0 +114,4 @@public extension SingleValueStorage where ErrorType == StorageError {func migrating<S: SingleValueStorage>(to targetStorage: S) -> MigratingSingleValueStorage<Self, S>можно ещё добавить .migrating(from:)