Smack-omemo rework #177 - TrustCallback can only be set once

Smack Smack Dev
1

The OmemoManager#setTrustCallback can only be executed once. Presently this requires an instance of OmemoManager to use this method. In aTalk implementation, this method is called after connection is connected. Hence it will get call again during auto re-connection or manual re-connection. Would you consider making getTrustCallback() public so the app can check before making call to setTrustCallback().

Alternative, setTrustCallback() just ignores the call if trustCallback is not null, instead of throwing an error. (see below source)

I do think of making a local flag to keep track the state but do not think this is a good idea.

    /**
     * Set a TrustCallback for this particular OmemoManager.
     * TrustCallbacks are used to query and modify trust decisions.
     *
     * @param callback trustCallback.
     */
    public void setTrustCallback(OmemoTrustCallback callback) {
        if (trustCallback == null) {
            trustCallback = callback;
        }
    }

    /**
     * Return the TrustCallback of this manager.
     * @return
     */
    public OmemoTrustCallback getTrustCallback() {
        return trustCallback;
    }
2

I will not make the getter public, as it would make the API too complex. I suggest calling setTrustCallback before the connection gets connected. Otherwise race conditions might occur.

3

Changing where to setTrustCallback does not really help in aTalk. aTalk manual reconnect process goes through the same path. It may help only the ReconnectManager triggers re-connection.

In aTalk, setTrustCallback and OmemoInit happen some time apart, so race condition does not happen in aTalk.

But is it necessary to throws exception in setter? can’t the setter just ignore it as proposed.
Right now aTalk just catches the exception and ignores it.

4

It is desirable not to change the callback during operation, thats why I throw an exception. If catching the exception works in your case, than I’d say its a good solution :slight_smile: