feat: migrating storages #10

Merged
nikita.semenov merged 13 commits from feature/migrating_storage into master 2023-07-09 22:33:52 +03:00
Member

MigratingStorage

При необходимости мигрировать с одного keychain на другой можно воспользоваться классом BaseMigratingSingleValueStorage. При создании "мигрирующего" хранилища необходимо будет указать:

  • source storage: хранилище с которого мигрируем
  • target storage: хранилище на которое мигрируем
let groupKeychain = Keychain(service: "app.group.identifier")
let targetApiTokenKeychainStorage = StringValueKeychainStorage(keychain: groupKeychain, storageKey: .apiToken)

let migratingApiTokenKeychainStorage = apiTokenKeychainStorage.migrating(to: targetApiTokenKeychainStorage)

let token = migratingApiTokenKeychainStorage.getValue()

KeychainMigratingCodableBackingStore

import TIFoundationUtils
import TIKeychainUtils
import KeychainAccess

extension StorageKey {
    static var knownPins: StorageKey<[String: Set<String>]> {
        .init(rawValue: "knownPins")
    }
}

extension Keychain {
    static var keychain: Keychain {
        .init()
    }

    static var groupKeychain: Keychain {
        .init(service: "app.group.identifier")
    }
}

extension KeychainMigratingStorageContainer {
    static var defaultContainer: KeychainMigratingStorageContainer {
        .init(sourceStorage: .keychain, targetStorage: .groupKeychain)
    }
}

struct PinningManager {

    // Migration from keychain to groupKeychain
    // @KeychainCodableBackingStore(key: .knownPins, codableKeyValueStorage: .keychain)
    @KeychainMigratingCodableBackingStore(key: .knownPins, storageContainer: .defaultContainer)
    var knownPins = [String: Set<String>]()
}
### MigratingStorage При необходимости мигрировать с одного keychain на другой можно воспользоваться классом `BaseMigratingSingleValueStorage`. При создании "мигрирующего" хранилища необходимо будет указать: - source storage: хранилище с которого мигрируем - target storage: хранилище на которое мигрируем ```swift let groupKeychain = Keychain(service: "app.group.identifier") let targetApiTokenKeychainStorage = StringValueKeychainStorage(keychain: groupKeychain, storageKey: .apiToken) let migratingApiTokenKeychainStorage = apiTokenKeychainStorage.migrating(to: targetApiTokenKeychainStorage) let token = migratingApiTokenKeychainStorage.getValue() ``` ### KeychainMigratingCodableBackingStore ```swift import TIFoundationUtils import TIKeychainUtils import KeychainAccess extension StorageKey { static var knownPins: StorageKey<[String: Set<String>]> { .init(rawValue: "knownPins") } } extension Keychain { static var keychain: Keychain { .init() } static var groupKeychain: Keychain { .init(service: "app.group.identifier") } } extension KeychainMigratingStorageContainer { static var defaultContainer: KeychainMigratingStorageContainer { .init(sourceStorage: .keychain, targetStorage: .groupKeychain) } } struct PinningManager { // Migration from keychain to groupKeychain // @KeychainCodableBackingStore(key: .knownPins, codableKeyValueStorage: .keychain) @KeychainMigratingCodableBackingStore(key: .knownPins, storageContainer: .defaultContainer) var knownPins = [String: Set<String>]() } ```
nikita.semenov added 1 commit 2023-06-25 20:16:46 +03:00
nikita.semenov added 1 commit 2023-06-28 09:26:20 +03:00
ivan.smolin requested changes 2023-06-30 17:10:51 +03:00
@ -0,0 +38,4 @@
return value
}
if case let .success(value) = container.sourceStorage.codableObject(forKey: key, decoder: decoder) {
Member

может через цепочку?

что-то такое:

container.sourceStorage.codableObject(forKey: key, decoder: decoder)
.flatMap {
    container.sourceStorage.removeCodableValue(forKey: key)
}
.flatMap {
    container.targetStorage.set(encodableObject: value, forKey: key, encoder: encoder)
}
может через цепочку? что-то такое: ```swift container.sourceStorage.codableObject(forKey: key, decoder: decoder) .flatMap { container.sourceStorage.removeCodableValue(forKey: key) } .flatMap { container.targetStorage.set(encodableObject: value, forKey: key, encoder: encoder) } ```
ivan.smolin marked this conversation as resolved
@ -0,0 +74,4 @@
return value
}
if case let .success(value) = container.sourceStorage.codableObject(forKey: key, decoder: decoder) {
Member

копипаста. можем что-то с этим сделать?

копипаста. можем что-то с этим сделать?
ivan.smolin marked this conversation as resolved
@ -0,0 +42,4 @@
open func hasStoredValue() -> Bool {
if targetStorage.hasStoredValue() {
let _ = sourceStorage.deleteValue()
Member

не думаю, что это тут требуется

скорее просто

targetStorage.hasStoredValue() || sourceStorage.hasStoredValue()

не думаю, что это тут требуется скорее просто ```swift targetStorage.hasStoredValue() || sourceStorage.hasStoredValue() ```
ivan.smolin marked this conversation as resolved
@ -0,0 +51,4 @@
}
open func store(value: ValueType) -> Result<Void, StorageError> {
targetStorage.store(value: value)
Member

так как миграция у нас ленивая, то может быть кейс когда мы сначала будем записывать значение в storage, а потом пробовать читать и тогда в getValue отработает скорей всего неверно

давай тут удаление их sourceStorage сделаем перед записью в targetStorage

так как миграция у нас ленивая, то может быть кейс когда мы сначала будем записывать значение в storage, а потом пробовать читать и тогда в getValue отработает скорей всего неверно давай тут удаление их sourceStorage сделаем перед записью в targetStorage
ivan.smolin marked this conversation as resolved
@ -0,0 +62,4 @@
let result = sourceStorage.getValue()
if case let .success(value) = result, case .success = store(value: value) {
let _ = sourceStorage.deleteValue()
Member

let обязательно?

let обязательно?
ivan.smolin marked this conversation as resolved
@ -0,0 +69,4 @@
}
open func deleteValue() -> Result<Void, StorageError> {
if targetStorage.hasStoredValue() {
Member

да можно и не проверять

да можно и не проверять
ivan.smolin marked this conversation as resolved
@ -0,0 +75,4 @@
return targetStorage.deleteValue()
}
return sourceStorage.deleteValue()
Member
targetStorage.deleteValue()
.flatMap {
    sourceStorage.deleteValue()
}

?

```swift targetStorage.deleteValue() .flatMap { sourceStorage.deleteValue() } ``` ?
ivan.smolin marked this conversation as resolved
@ -0,0 +24,4 @@
import KeychainAccess
import TISwiftUtils
public typealias KeychainMigratingCodableBackingStore<T: Codable> = MigratingBackingStore<Keychain,
Member

лишний пробел

лишний пробел
ivan.smolin marked this conversation as resolved
@ -0,0 +23,4 @@
import TIFoundationUtils
import KeychainAccess
open class BaseMigratingSingleValueKeychainStorage<ValueType>: BaseMigratingSingleValueStorage<ValueType,
Member

я не понимаю

BaseMigratingSingleValueStorage<SourceStorage: SingleValueStorage,
TargetStorage: SingleValueStorage>

2 generic параметра типа SingleValueStorage, а тут передаётся ValueType и Container

я не понимаю BaseMigratingSingleValueStorage<SourceStorage: SingleValueStorage, TargetStorage: SingleValueStorage> 2 generic параметра типа SingleValueStorage, а тут передаётся ValueType и Container
ivan.smolin marked this conversation as resolved
@ -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) {
Member
targetStorage.codableObject(forKey: key, decoder: decoder)
    .flatMapError {
        if case .valueNotFound = $0 {
            return sourceStorage.codableObject(forKey: key, decoder: decoder)
        } else {
            return $0
        }
    }

а тут не должно быть кода с миграцией? в чем суть storage container'а?

```swift targetStorage.codableObject(forKey: key, decoder: decoder) .flatMapError { if case .valueNotFound = $0 { return sourceStorage.codableObject(forKey: key, decoder: decoder) } else { return $0 } } ``` а тут не должно быть кода с миграцией? в чем суть storage container'а?
ivan.smolin marked this conversation as resolved
@ -0,0 +44,4 @@
forKey key: StorageKey<Value>,
encoder: CodableKeyValueEncoder) -> Result<Void, StorageError> {
targetStorage.set(encodableObject: encodableObject, forKey: key, encoder: encoder)
Member

и тут удалить из старого

и тут удалить из старого
ivan.smolin marked this conversation as resolved
@ -0,0 +48,4 @@
}
open func removeCodableValue<Value: Codable>(forKey key: StorageKey<Value>) -> Result<Void, StorageError> {
if case .success = sourceStorage.hasCodableValue(forKey: key) {
Member

проверка на has лишняя кмк

проверка на has лишняя кмк
ivan.smolin marked this conversation as resolved
@ -0,0 +60,4 @@
}
open func hasCodableValue<Value: Decodable>(forKey key: StorageKey<Value>) -> Result<Bool, StorageError> {
if case .success = targetStorage.hasCodableValue(forKey: key) {
Member

тоже через flatMapError как в комменте к codableObject(forKey:)

тоже через flatMapError как в комменте к codableObject(forKey:)
ivan.smolin marked this conversation as resolved
@ -0,0 +44,4 @@
return codableResult
}
return container.getStringValue(forKey: storageKey)
Member

это какой-то compatibility?)

это какой-то compatibility?)
ivan.smolin marked this conversation as resolved
nikita.semenov added 1 commit 2023-06-30 22:03:43 +03:00
0776c99e38 Merge branch 'master' into feature/migrating_storage
# Conflicts:
#	CHANGELOG.md
#	TIAppleMapUtils/TIAppleMapUtils.podspec
#	TIAuth/TIAuth.podspec
#	TIDeeplink/TIDeeplink.podspec
#	TIDeveloperUtils/TIDeveloperUtils.podspec
#	TIEcommerce/TIEcommerce.podspec
#	TIFoundationUtils/TIFoundationUtils.podspec
#	TIGoogleMapUtils/TIGoogleMapUtils.podspec
#	TIKeychainUtils/TIKeychainUtils.app/Contents/MacOS/TIKeychainUtils.playground/Pages/SingleValueStorage.xcplaygroundpage/Contents.swift
#	TIKeychainUtils/TIKeychainUtils.podspec
#	TILogging/TILogging.podspec
#	TIMapUtils/TIMapUtils.podspec
#	TIMoyaNetworking/TIMoyaNetworking.podspec
#	TINetworking/TINetworking.podspec
#	TINetworkingCache/TINetworkingCache.podspec
#	TIPagination/TIPagination.podspec
#	TISwiftUICore/TISwiftUICore.podspec
#	TISwiftUtils/TISwiftUtils.podspec
#	TITableKitUtils/TITableKitUtils.podspec
#	TITextProcessing/TITextProcessing.podspec
#	TIUIElements/TIUIElements.podspec
#	TIUIKitCore/TIUIKitCore.podspec
#	TIWebView/TIWebView.podspec
#	TIYandexMapUtils/TIYandexMapUtils.podspec
#	docs/tikeychainutils/singlevaluestorage.md
nikita.semenov added 1 commit 2023-07-02 18:00:45 +03:00
ivan.smolin reviewed 2023-07-03 10:23:43 +03:00
ivan.smolin reviewed 2023-07-03 13:25:41 +03:00
@ -33,6 +33,8 @@ open class BaseSingleValueStorage<ValueType, StorageType>: SingleValueStorage {
public let getValueClosure: GetValueClosure
public let storeValueClosure: StoreValueClosure
public var migrationStorage: StorageType?
Member

я не понимаю зачем так делать. мы не должны эту сущности связывать

я не понимаю зачем так делать. мы не должны эту сущности связывать
ivan.smolin marked this conversation as resolved
@ -0,0 +59,4 @@
}
.flatMapError { _ in
sourceStorage.getValue()
.flatMap { value in
Member
.flatMap(self.store)

? можно даже без self навер

```swift .flatMap(self.store) ``` ? можно даже без self навер
Author
Member

Вроде не можем, т.к. store(value:) возвращаяет Result<Void, Error>, на нам нужен Result<Value, Error>

Вроде не можем, т.к. `store(value:)` возвращаяет `Result<Void, Error>`, на нам нужен `Result<Value, Error>`
ivan.smolin marked this conversation as resolved
@ -0,0 +65,4 @@
return value
}
.mapError { .unableToExtractData(underlyingError: $0) }
Member

и это не потребуется наверное

и это не потребуется наверное
ivan.smolin marked this conversation as resolved
@ -0,0 +45,4 @@
forKey key: StorageKey<Value>,
encoder: CodableKeyValueEncoder) -> Result<Void, StorageError> {
removeSourceValue(forKey: key)
Member

даже если мы не пробрасываем результат выше, то надо залогировать (см. пример в TINetworking)

даже если мы не пробрасываем результат выше, то надо залогировать (см. пример в TINetworking)
ivan.smolin marked this conversation as resolved
@ -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 {
Member

в случае codable, вызов hasCodableValue будет равносилен получению codableObject (чтение + декодирование в объект), поэтому предлагаю сразу вызвать removeCodableValue и сделать flatMapError с проверкой на valueNotFound

в случае codable, вызов hasCodableValue будет равносилен получению codableObject (чтение + декодирование в объект), поэтому предлагаю сразу вызвать removeCodableValue и сделать flatMapError с проверкой на valueNotFound
ivan.smolin marked this conversation as resolved
@ -0,0 +70,4 @@
// MARK: - Open methods
open func removeSourceValue<Value>(forKey key: StorageKey<Value>) {
Member

тут не надо вернуть Result<Void, StorageError>?

тут не надо вернуть `Result<Void, StorageError>`?
ivan.smolin marked this conversation as resolved
@ -0,0 +25,4 @@
public final class KeychainMigratingStorageContainer: BaseCodableMigratingStorageContainer<Keychain, Keychain> {
public override func removeSourceValue<Value>(forKey key: StorageKey<Value>) {
Member

а зачем на этот метод отдельно? вроде бы мы storage'ы закрываем протоколами и там есть функция для этого

а зачем на этот метод отдельно? вроде бы мы storage'ы закрываем протоколами и там есть функция для этого
Author
Member

Проблема в том, что из методов, в которых мы удаляем значение из хранилища, Value ограничен протоколом Decodable. А стандартный метод удаления требует, чтобы Value был Decodable. Нашел вот такой способ обойти ограничение без изменения сигнатуры метода удаления

Проблема в том, что из методов, в которых мы удаляем значение из хранилища, Value ограничен протоколом Decodable. А стандартный метод удаления требует, чтобы Value был Decodable. Нашел вот такой способ обойти ограничение без изменения сигнатуры метода удаления
ivan.smolin marked this conversation as resolved
@ -18,3 +13,1 @@
s.source_files = s.name + '/' + sources
s.exclude_files = s.name + '/*.app'
end
s.source_files = s.name + '/Sources/**/*'
Member

смержилось неверно

смержилось неверно
ivan.smolin marked this conversation as resolved
@ -0,0 +41,4 @@
let oldProfile = Profile(name: "some", age: 0)
let sourceStorage = MockMigratingStorageContainer.defaultContainer.sourceStorage
let _ = sourceStorage.set(encodableObject: oldProfile, forKey: .profile, encoder: encoder)
Member

результат тоже надо через XCTAssert проверять

результат тоже надо через XCTAssert проверять
ivan.smolin marked this conversation as resolved
@ -0,0 +46,4 @@
XCTAssertEqual(profile, oldProfile)
}
func testGetValueFromTarget() throws {
Member

в итоге, тут протестирована просто работа targetSource? если да, то этот тест надо в отдельный набор тестов надо вынести. так как тут тесты про миграцию

в итоге, тут протестирована просто работа targetSource? если да, то этот тест надо в отдельный набор тестов надо вынести. так как тут тесты про миграцию
ivan.smolin marked this conversation as resolved
@ -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)
Member

optional try тут вроде не нужен, так как функция декларирует throws

optional try тут вроде не нужен, так как функция декларирует throws
ivan.smolin marked this conversation as resolved
@ -0,0 +63,4 @@
profile = newProfile
XCTAssertEqual(profile, newProfile)
XCTAssertEqual(try? targetStorage.codableObject(forKey: .profile, decoder: decoder).get(), newProfile)
Member

аналогично, про optional try

аналогично, про optional try
ivan.smolin marked this conversation as resolved
nikita.semenov added 1 commit 2023-07-06 12:34:53 +03:00
ivan.smolin reviewed 2023-07-06 13:22:39 +03:00
@ -0,0 +50,4 @@
}
open func store(value: ValueType) -> Result<Void, StorageError> {
let _ = sourceStorage.deleteValue()
Member
if case let .failure(error) = sourceStorage.deleteValue() {
    errorLogger.log(error: error, file: #file, line: #line)
}
```swift if case let .failure(error) = sourceStorage.deleteValue() { errorLogger.log(error: error, file: #file, line: #line) } ```
ivan.smolin marked this conversation as resolved
@ -0,0 +63,4 @@
open func getValue() -> Result<ValueType, StorageError> {
targetStorage.getValue()
.flatMap {
let _ = sourceStorage.deleteValue()
Member

аналогично, так как возвращаемое значение не участвует в дальнейшей цепочке

аналогично, так как возвращаемое значение не участвует в дальнейшей цепочке
ivan.smolin marked this conversation as resolved
@ -0,0 +75,4 @@
.flatMapError { _ in
sourceStorage.getValue()
.flatMap { value in
Result {
Member

вроде так логичней:

.flatMap {
    store(value: $0)
}
.mapError { .unableToExtractData(underlyingError: $0) }
вроде так логичней: ```swift .flatMap { store(value: $0) } .mapError { .unableToExtractData(underlyingError: $0) } ```
Author
Member

Так не, вывод у store(value:) не тот, который ожидается от getValue()

Cannot convert value of type '(SourceStorage.ValueType) -> Result<Void, StorageError>' to expected argument type '(SourceStorage.ValueType) -> Result<BaseMigratingSingleValueStorage<SourceStorage, TargetStorage>.ValueType, StorageError>' (aka '(SourceStorage.ValueType) -> Result<SourceStorage.ValueType, StorageError>')
Так не, вывод у `store(value:)` не тот, который ожидается от `getValue()` ``` Cannot convert value of type '(SourceStorage.ValueType) -> Result<Void, StorageError>' to expected argument type '(SourceStorage.ValueType) -> Result<BaseMigratingSingleValueStorage<SourceStorage, TargetStorage>.ValueType, StorageError>' (aka '(SourceStorage.ValueType) -> Result<SourceStorage.ValueType, StorageError>') ```
Member

а, ну верно. тогда:

.flatMap { value in
    store(value: value)
        .map { _ in value}
}
.mapError { .unableToExtractData(underlyingError: $0) }

?

а, ну верно. тогда: ```swift .flatMap { value in store(value: value) .map { _ in value} } .mapError { .unableToExtractData(underlyingError: $0) } ``` ?
ivan.smolin marked this conversation as resolved
@ -0,0 +97,4 @@
.flatMapError {
errorLogger.log(error: $0, file: #file, line: #line)
return sourceStorage.deleteValue()
Member

вот тут странно. получается, если мы не смогли удалить значение из source несколькими строками выше, то тоже попадём сюда и попробуем еще раз?

вот тут странно. получается, если мы не смогли удалить значение из source несколькими строками выше, то тоже попадём сюда и попробуем еще раз?
ivan.smolin marked this conversation as resolved
@ -0,0 +28,4 @@
CodableKeyValueStorage
where SourceStorage: CodableKeyValueStorage, TargetStorage: CodableKeyValueStorage {
public enum CodableTypeError: Error {
Member

unused

unused
ivan.smolin marked this conversation as resolved
@ -0,0 +48,4 @@
targetStorage.codableObject(forKey: key, decoder: decoder)
.flatMap {
let _ = removeSourceValue(forKey: key)
Member

if case let

if case let
ivan.smolin marked this conversation as resolved
@ -0,0 +58,4 @@
return .success($0)
}
.flatMapError { _ in
sourceStorage.codableObject(forKey: key, decoder: decoder)
Member

вот тут можно ещё залогировать, если ошибка не .valueNotFound

вот тут можно ещё залогировать, если ошибка не .valueNotFound
ivan.smolin marked this conversation as resolved
@ -0,0 +66,4 @@
forKey key: StorageKey<Value>,
encoder: CodableKeyValueEncoder) -> Result<Void, StorageError> {
let _ = removeSourceValue(forKey: key)
Member

if case let

if case let
ivan.smolin marked this conversation as resolved
@ -0,0 +83,4 @@
.flatMapError { _ in .success(value) }
}
.flatMapError {
errorLogger.log(error: $0, file: #file, line: #line)
Member

логируем если не .valueNotFound

логируем если не .valueNotFound
ivan.smolin marked this conversation as resolved
@ -0,0 +34,4 @@
override func setUp() {
profile = nil
let _ = MockMigratingStorageContainer.defaultContainer.sourceStorage.removeValue(forKey: .profile)
let _ = MockMigratingStorageContainer.defaultContainer.targetStorage.removeValue(forKey: .profile)
Member

profile = nil не затрёт в source и target?

profile = nil не затрёт в source и target?
Author
Member

Затрет, поправил

Затрет, поправил
ivan.smolin marked this conversation as resolved
@ -0,0 +1,26 @@
//
// Copyright (c) 2020 Touch Instinct
Member

2023

2023
ivan.smolin marked this conversation as resolved
nikita.semenov added 1 commit 2023-07-06 18:42:23 +03:00
ivan.smolin reviewed 2023-07-06 19:48:34 +03:00
ivan.smolin reviewed 2023-07-06 21:04:54 +03:00
@ -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 container
Member

storate ?

storate ?
Member

ну и вообще коммент не понятен

ну и вообще коммент не понятен
Author
Member

Сделал коммент подробнее. Контейнер не может мигрировать значение из-за того, что в методе codableObject(forKey:decoder) нет доступа к encoder'у и Value ограничен только Decodable

Сделал коммент подробнее. Контейнер не может мигрировать значение из-за того, что в методе `codableObject(forKey:decoder)` нет доступа к encoder'у и Value ограничен только Decodable
ivan.smolin marked this conversation as resolved
@ -0,0 +35,4 @@
// If source storage has value and target doesn't we can't migrate it from storate container
try? container.codableObject(forKey: key, decoder: decoder)
.flatMap {
let _ = container.set(encodableObject: $0, forKey: key, encoder: encoder)
Member

видимо коммент должен был прояснить зачем мы сначала получаем значение а потом его обратно записываем.
разве мигация не происходит внутри контейнера?

видимо коммент должен был прояснить зачем мы сначала получаем значение а потом его обратно записываем. разве мигация не происходит внутри контейнера?
Member

окей, а можем это вынести в отдельный (статический?) метод, чтобы не дублировать логину ниже

окей, а можем это вынести в отдельный (статический?) метод, чтобы не дублировать логину ниже
ivan.smolin marked this conversation as resolved
@ -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
Member

аналогично

аналогично
ivan.smolin marked this conversation as resolved
@ -0,0 +60,4 @@
// If source storage has value and target doesn't we can't migrate it from storate container
try? container.codableObject(forKey: key, decoder: decoder)
.flatMap {
let _ = container.set(encodableObject: $0, forKey: key, encoder: encoder)
Member

аналогично

аналогично
ivan.smolin marked this conversation as resolved
@ -0,0 +22,4 @@
import TILogging
open class BaseMigratingSingleValueStorage<SourceStorage: SingleValueStorage,
Member

не вижу наследований. может сделать final? потенциально могут вообще быть кейсы, когда нам надо будет это переопределять?

не вижу наследований. может сделать final? потенциально могут вообще быть кейсы, когда нам надо будет это переопределять?
ivan.smolin marked this conversation as resolved
@ -0,0 +89,4 @@
return sourceStorage.deleteValue()
}
.flatMap {
if sourceStorage.hasStoredValue() {
Member

кажется, тут лучше разорвать цепочку и сделать примерно так:

switch (targetStorage.deleteValue(), sourceStorage.deleteValue()) {
case .success, .success:
    return .success(())
    
case let .success, .failure(error):
    if error.isValueNotFound {
        return .success(())
    }
    
    return .failure(error)
    
// ...
}
кажется, тут лучше разорвать цепочку и сделать примерно так: ```swift switch (targetStorage.deleteValue(), sourceStorage.deleteValue()) { case .success, .success: return .success(()) case let .success, .failure(error): if error.isValueNotFound { return .success(()) } return .failure(error) // ... } ```
ivan.smolin marked this conversation as resolved
@ -0,0 +71,4 @@
}
open func removeCodableValue<Value: Codable>(forKey key: StorageKey<Value>) -> Result<Void, StorageError> {
targetStorage.removeCodableValue(forKey: key)
Member

тут вроде аналогичная логика как и выше с двумя результатами

тут вроде аналогичная логика как и выше с двумя результатами
Member

up

up
ivan.smolin marked this conversation as resolved
@ -0,0 +21,4 @@
//
import XCTest
@testable import TIFoundationUtils
Member

тут правда нужен именно testable import? расскажи плз до чего нужен доступ

тут правда нужен именно testable import? расскажи плз до чего нужен доступ
ivan.smolin marked this conversation as resolved
@ -0,0 +43,4 @@
let oldProfile = Profile(name: "old", age: 0)
let newProfile = Profile(name: "new", age: 0)
let sourceStorage = MockMigratingStorageContainer.defaultContainer.sourceStorage
let targetStorage = MockMigratingStorageContainer.defaultContainer.targetStorage
Member

может вынести их в property TestCase'а? а то получается лишнее дублирование двух строк в каждом кейсе

может вынести их в property TestCase'а? а то получается лишнее дублирование двух строк в каждом кейсе
ivan.smolin marked this conversation as resolved
nikita.semenov added 1 commit 2023-07-07 09:01:26 +03:00
ivan.smolin reviewed 2023-07-07 11:29:43 +03:00
ivan.smolin reviewed 2023-07-07 11:50:59 +03:00
@ -0,0 +74,4 @@
targetStorage.removeCodableValue(forKey: key)
.flatMap { value in
sourceStorage.removeCodableValue(forKey: key)
.flatMapError { _ in .success(value) }
Member

залогировать если не .valueNotFound?

залогировать если не .valueNotFound?
ivan.smolin marked this conversation as resolved
@ -0,0 +81,4 @@
errorLogger.log(error: $0, file: #file, line: #line)
}
return removeSourceValue(forKey: key)
Member

я не понимаю.

как мне представляется, текущий код работает так:

  • удаляем из target
  • удаляем из source
  • ошибку source игнорим
  • ошибку из target логируем
  • удаляем из source
  • ошибку из source прокидываем

а нужно:

  • удалить из target
  • удалить из source
  • ошибку из target прокинуть
  • ошибку из source залогировать (если не .valueNotFound)
я не понимаю. как мне представляется, текущий код работает так: - удаляем из target - удаляем из source - ошибку source игнорим - ошибку из target логируем - удаляем из source - ошибку из source прокидываем а нужно: - удалить из target - удалить из source - ошибку из target прокинуть - ошибку из source залогировать (если не .valueNotFound)
ivan.smolin marked this conversation as resolved
@ -0,0 +88,4 @@
open func hasCodableValue<Value: Decodable>(forKey key: StorageKey<Value>) -> Result<Bool, StorageError> {
targetStorage.hasCodableValue(forKey: key)
.flatMapError { _ in
sourceStorage.hasCodableValue(forKey: key)
Member

тут тоже надо отдать приоритет ошибке из target (если она не .valueNotFound)

тут тоже надо отдать приоритет ошибке из target (если она не .valueNotFound)
ivan.smolin marked this conversation as resolved
@ -0,0 +102,4 @@
return .failure(error)
case let (.failure, .failure(error)):
Member

в этом случае я бы отдал приоритет error из targetSource

в этом случае я бы отдал приоритет error из targetSource
ivan.smolin marked this conversation as resolved
ivan.smolin reviewed 2023-07-07 12:02:48 +03:00
@ -0,0 +36,4 @@
public init(sourceStorage: SourceStorage,
targetStorage: TargetStorage,
errorLogger: ErrorLogger = TIFoundationLogger(category: "BaseMigratingSingleValueStorage")) {
Member

надо переименовать и в доках тоже

надо переименовать и в доках тоже
ivan.smolin marked this conversation as resolved
nikita.semenov added 1 commit 2023-07-07 14:00:48 +03:00
ivan.smolin reviewed 2023-07-07 14:54:10 +03:00
ivan.smolin reviewed 2023-07-07 14:58:02 +03:00
@ -0,0 +50,4 @@
}
public func store(value: ValueType) -> Result<Void, StorageError> {
if case let .failure(error) = sourceStorage.deleteValue() {
Member

сначала надо записать в target и если получилось уже удалять из source

сначала надо записать в target и если получилось уже удалять из source
ivan.smolin marked this conversation as resolved
@ -0,0 +69,4 @@
.flatMapError { _ in
sourceStorage.getValue()
.flatMap { value in
Result {
Member
store(value: value)
    .map { _ in value }

не?

```swift store(value: value) .map { _ in value } ``` не?
ivan.smolin marked this conversation as resolved
@ -0,0 +114,4 @@
public extension SingleValueStorage where ErrorType == StorageError {
func migrating<S: SingleValueStorage>(to targetStorage: S) -> MigratingSingleValueStorage<Self, S>
Member

можно ещё добавить .migrating(from:)

можно ещё добавить .migrating(from:)
ivan.smolin marked this conversation as resolved
nikita.semenov added 1 commit 2023-07-07 15:17:58 +03:00
ivan.smolin approved these changes 2023-07-07 15:20:19 +03:00
nikita.semenov added 1 commit 2023-07-07 15:39:17 +03:00
nikita.semenov added 1 commit 2023-07-09 21:51:49 +03:00
nikita.semenov added 1 commit 2023-07-09 22:10:41 +03:00
nikita.semenov added 1 commit 2023-07-09 22:15:40 +03:00
nikita.semenov merged commit 83655d2bac into master 2023-07-09 22:33:52 +03:00
nikita.semenov deleted branch feature/migrating_storage 2023-07-09 22:34:00 +03:00
Sign in to join this conversation.
No reviewers
No Label
No Milestone
No project
No Assignees
2 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: TouchInstinct/LeadKit#10
No description provided.