use throwing var methods for realm references to other objects

[tomholub]

@tomholub ability to throw from var variable was added to the Swift recently:

var activeUser: UserRealmObject {
    get throws {
        guard let user = user else { throw AppErr... }
        return user
    }
  }

This looks good. Probably for another issue/PR?

The advantage is that an exception can be captured and handled (in the general error handling somewhere in VC, wherever user action was initiated) while a forced unwrap cannot.

Originally posted by @tomholub in #1030 (comment)

[ivan-ushakov]

@tomholub
Now we use optional reference in Realm objects for linked objects, as documentation says. What we need to do in this issue?

[tomholub]

I'm thinking, when converting from RealmObject to a general model, throw if these cross-referenced objects are missing.

Then general models would not have optional objects, if we meant the objects to be required.

For example, a private key is meaningless if it doesn't belong to a particular user. So a key with a nil user on it makes no sense: should throw when we convert that from PrivateKeyRealmObject to PrivateKey (or similar - didn't check the actual names)

[ivan-ushakov]

We have KeyInfoRealmObject but when we create it, we provide user object like this:

convenience init(_ keyDetails: KeyDetails, passphrase: String?, source: KeySource, user: UserRealmObject) throws

and in KeyInfo type we don't have user property. All other places are same. We don't construct objects with optional links (the only exception is UserRealmObject with optional SessionRealmObject).

[tomholub]

Yes, but look at this:

extension KeyInfoRealmObject {
    /// associated user email
    var account: String? {
        user?.email
    }
}

user should never be nil, therefore there should be guard and throw, and it should return a String that's not optional. Usages of .account which is undesirably optional:

Same problem here:

extension ClientConfigurationRealmObject: CachedRealmObject {
    var identifier: String { userEmail ?? "" }

    var activeUser: UserRealmObject? { user }
}

actually there are two problems above:

1. activeUser is badly named, should be just "user". There is nothing active about it. And it should not be nilable, so guard and throw?
2. identifier should not be defaulted to empty string!!! If I saw this in newly added code, I'd be chopping heads :-) I must have overlooked it earlier. What exactly is an empty string for identifier? Need to guard and throw.

And so on

[ivan-ushakov]

@tomholub
In one case we could have nil active user:

extension RecipientRealmObject: CachedRealmObject {
    // Contacts can be shared between accounts
    // https://github.com/FlowCrypt/flowcrypt-ios/issues/269
    var activeUser: UserRealmObject? { nil }

    var identifier: String { email }
}

How to deal with this case?

[ivan-ushakov]

activeUser is badly named, should be just "user". There is nothing active about it. And it should not be nilable, so guard and throw?

This will overlap with user property in object. We have different semantic here: in object we could have optional user link because Realm requires it, but in cache protocol we should have user.

protocol CachedRealmObject: Object {
    associatedtype Identifier: Equatable
    var identifier: Identifier { get }
    var activeUser: UserRealmObject { get }
}

[tomholub]

@tomholub In one case we could have nil active user:

extension RecipientRealmObject: CachedRealmObject {
    // Contacts can be shared between accounts
    // https://github.com/FlowCrypt/flowcrypt-ios/issues/269
    var activeUser: UserRealmObject? { nil }

    var identifier: String { email }
}

How to deal with this case?

There is no reason for RecipientRealmObject to inherit of CachedRealmObject. It should be separated in any case.

Then that will also solve this problem.

[tomholub]

activeUser is badly named, should be just "user". There is nothing active about it. And it should not be nilable, so guard and throw?

This will overlap with user property in object. We have different semantic here: in object we could have optional user link because Realm requires it, but in cache protocol we should have user.

protocol CachedRealmObject: Object {
    associatedtype Identifier: Equatable
    var identifier: Identifier { get }
    var activeUser: UserRealmObject { get }
}

Then we can call it owner maybe?

[ivan-ushakov]

@Kharchevskyi

Could you please explain what was the idea of EncryptedCacheService? Is it the same as EncryptedStorage?

For example if we want to remove CachedRealmObject conformance from RecipientRealmObject, we can not use this:

struct LocalContactsProvider {
    private let localContactsCache: EncryptedCacheService<RecipientRealmObject>
    let core: Core

    init(
        encryptedStorage: EncryptedStorageType = EncryptedStorage(),
        core: Core = .shared
    ) {
        self.localContactsCache = EncryptedCacheService<RecipientRealmObject>(encryptedStorage: encryptedStorage)
        self.core = core
    }
}

[ivan-ushakov]

@tomholub

I think it makes sense to wait for #1117.